Give ComplexType and UniqueType a shared Type base class - #91
Draft
apiology wants to merge 14 commits into
Draft
Conversation
Let's see if we can make space for a first-class intersection type
Pin::Base#typify now declares [Type], and Source::Chain::Call calls #expand on its result. Type listed every other method its subclasses share but not that one, so the call did not resolve. Both ComplexType and ComplexType::UniqueType define #expand, so the omission was an incomplete interface declaration rather than a real gap. Declaring it also gives the two implementations their @PARAM and @return tags by inheritance, which resolves four pre-existing missing tag reports and one unresolved Hash lookup in UniqueType#expand.
Both are inherited from Type, which derives them from #name. A ComplexType has no #name, so it forwarded to @Items.first and a multi-member union answered as if it were its first member: the assert added earlier fires 9,556 times in one self-typecheck, all from these two. ComplexType#conforms_to? gates duck-type matching on expected.duck_type?, so a union like "#foo, String" entered duck_types_match?, which skips non-duck members -- the String was never checked against anything. Answering true only when every member is a duck type sends mixed unions down the ordinary member-wise conformance path instead. Empty is handled explicitly because Array#all? is true for no elements, which would make an empty type claim to be a duck type and then conform to everything.
Chain::If#resolve builds one type from the branch results with
types.map(&:tag).uniq.join(', '), but #tag names a single type. On a
branch that inferred a union, ComplexType forwarded #tag to its first
member, so the rest of that branch's type was dropped before the join.
#tags is the whole-type form and is already defined on both
ComplexType and UniqueType, returning the same string as #tag for a
single type. A union has no single tag, so this is fixed here rather
than by giving ComplexType a #tag.
BaseVariable#probe asked the assigned type for a single #rooted_name to decide whether it was list-like, then read #all_params. A ComplexType has neither, so both forwarded to @Items.first: for (Array<String>, Array<Integer>) the second member was dropped, and for a union whose first member happened to be an Array the whole union was treated as one. Neither question has a single answer over a union, so ask each member its own and collect the results. That also widens the outcome correctly, since destructuring (Array<String>, Array<Integer>) yields String, Integer.
ProxyType.anonymous derives an enclosing namespace from context.namespace to build the closure it synthesizes. A ComplexType forwarded that to @Items.first, so a union got the gates of whichever member happened to be first. Members of a union can sit in different namespaces and none of them speaks for the rest, so use the namespace only where every member agrees on it, and the root otherwise. Types whose namespace has a single segment already resolved to the root here, which is why this changes nothing for most of them.
UniqueType#resolve_param_generics_from_context matches its own type parameters positionally against the context type's, reading them with #key_types and #subtypes. A ComplexType forwarded both to @Items.first, so a generic could be resolved from one member of a union while the rest were ignored. A union holds one parameter list per member -- Hash{String => Integer}, Hash{Symbol => Float} has two key types in one position -- so there is nothing to match positionally against. Leave the generic unresolved instead of resolving it from an arbitrary member.
The block form ends in @items.each and so returns the array, not void as declared. The mismatch was invisible while callers of the branches that reach it inferred less precisely; it reports as a return type problem now that they do not.
subtypes.first can be nil, so the typechecker was right to reject subtypes.first.name; the fix is the check rather than a marker. The emptiness test moves into the safe navigation, and the `|| name` supplies the fallback the old ternary reached through its else branch. Measured equivalent: both forms were evaluated side by side through a full self-typecheck and the whole spec suite, and disagreed on nothing. An empty parameter list still falls back to `name`, and an empty-string name is returned as before, being truthy. They part on one value only, an empty ComplexType among the subtypes of a Class or Module, whose #name is nil. Nothing parses to it and no caller in lib builds one there; it has to be constructed by hand. On it the old form returned nil, which its own @return [String] forbids.
Closed
RuboCop's Lint/UnreachableCode fired on this line only because moving type_methods.rb's content into a new file, type.rb, reclassified the whole file as added, so a diff-scoped reviewdog check that ignores unmodified lines on master started reporting it. The line was already dead on master: an unconditional `return false` guards it, and #tuple? has no other implementation. @tuple has no other reader.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR was written by Claude Code on behalf of @apiology.
Problem:
ComplexType's predicate methods forward to@items.firstfor anything not explicitly overridden, so a multi-member union answers using only its first member:The same gap affects
void?,tag,rooted_name,all_params,namespace,key_types, andsubtypes. There was also no single place declaring which methodsComplexTypeandUniqueTypeare required to implement — that contract lived only in a flatTypeMethodsmixin included by both.Solution:
Introduces
Solargraph::Type, an abstract base class declaring the shared interface;ComplexTypeandUniqueTypeinherit from it instead of mixing inTypeMethods(deleted).ComplexTypegets real union-awareduck_type?/void?(true only when every member is); the handful of call sites that read a single-member field off a union (Chain::If#resolve,BaseVariable#probe,ProxyType.anonymous,UniqueType#resolve_param_generics_from_context) are updated to consider every member instead. Supersedes the draft #29, which carried this refactor on a stale January base with the same information-loss bug still present.