Remove the CodeOracle

[mhammond]

This is now purely a local consideration for bindings, there's no need for a shared trait. This removes a lot of unnecessary code.

I also snuck in some Kotlin improvements which are a good example of why not having the shared trait makes sense. It's probably easiest/best to look at the 2 commits individually.

[jplatte]

Haven't looked at the diff, but this sounds great! I previously found the amount of indirection / abstraction hard to grok, and jump-to-definition bringing me to a trait method declaration instead of an implementation was a big part of that.

[bendk]

I'm mixed on this one. The nice thing about the shared code was that it kept the backends more consistent. For example, the Kotlin backend is now going to handle the enums differently from all the other ones. OTOH, I do agree that the indirection was hard to follow. Also, it can be easier to evolve the code if you don't have to change all backends at once.

Overall I think I'm for, but I'd like to hear from @jhugman who understands the vision more.

[mhammond]

The nice thing about the shared code was that it kept the backends more consistent.

I think it's worth pointing out that the current code is setup such that the CodeOracle never actually escapes the binding (ie, no implementations outside the bindings ever take an Oracle as an arg etc).

So now, when a binding has a new requirement, it has 2 choices:

• Add a new method to CodeOracle which is specific to the binding, but force all other bindings to learn about it, reinforcing the fact that the CodeOracle is really just a union of requirements from all bindings.
• Ignore it completely, just add a new method to the impl of the code oracle and call it directly instead of by the trait

(which is exactly what I've done in #1592)

IMO, artificially trying to force bindings to use a trait that's entirely unused, unnecessary and which adds significant complexity, all in the interests of consistency seems the wrong tradeoff.

Also, it can be easier to evolve the code if you don't have to change all backends at once.

Yeah, this is the big win IMO, especially for external bindings. Ruby never used the code oracle, which is the reason it hasn't needed as much churn as the other bindings recently.

[jhugman]

The point of the oracle was to reduce the mega-matches over the Type enum, by putting the lookup to an object that could do the lift/lower for each variant.

I think the mistake was to put the CodeType methods (e.g. type_label) onto the oracle. This seemed to be a cause of too much indirection, and over-coupled the oracle to CodeType.

This patch seems to move the lookup functionality of the oracle into the AsCodeType trait, and correct that mistake.

So, all in all, this PR looks good to me.

Should there be a different or the same CodeType trait per language? Does it drive forward all the backends at a time? Is it the cause of the dreaded churn? I don't know. I'm not sure.

I think consistency of implementation in the binding backends should be prized: adding the same thing to Kotlin as you've just added to Swift has been a lot easier because they look the same.

[mhammond]

Thanks James.

Should there be a different or the same CodeType trait per language? Does it drive forward all the backends at a time? Is it the cause of the dreaded churn? I don't know. I'm not sure.

That's a great question I don't really know the answer to either. By sharing the implementation it has the nice side-effect of meaning some backends are quite similar, but has the bad side-effect of being a kind of "chilling effect" on innovation of implementation in particular back-ends - eg, Swift can't do something very interesting with its implementation if it means describing something Swift specific in the CodeType (or worse, it means adding something Swift specific to the CodeType :).

I think consistency of implementation in the binding backends should be prized

I'm also not 100% sure about this any more TBH. We've been steering contributors towards having their bindings out of the tree, so I strongly suspect the 4 bindings we have in-tree will be the max we will ever see, and ultimately we should probably move them somewhere else for "parity" with what we are asking external contributors to do. And of those 4, Ruby doesn't use an Oracle, and Python hasn't really kept up with Swift and Kotlin.

So at some point, I suspect we want to let Swift and Kotlin part so that they can express concepts naturally instead of expressing them as some awkward union of language capabilities. Eg, the handling of _Nonnull in Swift is already quite awkward, which I suspect could be made much more natural if Swift didn't have to also think about Kotlin or Python.

But that's all academic at the moment - I've no intention of moving the CodeType out, but I do think it's an interesting question we should keep in mind as future changes happen in the bindings.