types: TS conformance Rounds 6-11 — 216→250 Pass (symbol members, operators, class/interface checks)#1262
Open
nickna wants to merge 32 commits into
Open
types: TS conformance Rounds 6-11 — 216→250 Pass (symbol members, operators, class/interface checks)#1262nickna wants to merge 32 commits into
nickna wants to merge 32 commits into
Conversation
…S2403 HoistConstFunctionExpression pre-registers an unannotated var/const arrow's type before its body is checked, so forward references resolve. When neither the binding nor the arrow itself annotates a return type, it hoisted a `(params) => any` placeholder. VisitVar's TS2403 redeclaration check then compared that placeholder against the real inferred return type once the declaration's own initializer was checked — falsely reporting "subsequent variable declarations must have the same type" against the var's own first-and-only declaration whenever the real return type wasn't `any`. Register a bare `any` for this case instead, matching how HoistVarDeclarations/HoistClassDeclarations placeholder forward references — `any` is already exempt from the redeclaration check.
… now pass 219 Pass / 59 Fail (was 216/62), 0 regressions.
CheckTernary picked the wider of the two branch types via IsCompatible(thenType, elseType) || IsCompatible(elseType, thenType), but returned `thenType` unconditionally regardless of which clause fired. When elseType was actually the wider type (thenType assignable to elseType), the narrower thenType was returned instead — producing a different, order- dependent result for `cond ? a : b` vs `cond ? b : a`, which should be identical since both simply pick the common type of the same two operands. Same "pick-the-wider-of-two-types with a swapped return" anti-pattern already found and fixed in CheckLogicalAssign; this is a second, independent sibling instance in the ternary operator.
…eCore TypeInfo.Namespace and TypeInfo.Module had no dedicated case anywhere in IsCompatibleCore's decision tree, so a namespace-or-module-typed value fell through to the final `return false` — making it spuriously incompatible with every target, including structurally identical (or literally the same) namespace/module type. Relate them the same way object/record types are related: every member the expected type exposes must exist on the actual type with a compatible type.
223 Pass / 55 Fail (was 219/59), 0 regressions: nullIsSubtypeOfEverythingButUndefined, subtypesOfTypeParameter, and subtypesOfTypeParameterWithConstraints2/3/4 now pass.
…/SymbolConstructor precisely
Adds the missing TS2464 check ("a computed property name must be of type
'string', 'number', 'symbol', or 'any'") for object-literal properties,
methods, and accessors with a computed key. Records rather than throws so
every offending key in one literal is reported, not just the first
(mirrors the existing TS2411 interface string-index-collision loop).
This check is only meaningful once `Symbol` itself types precisely:
- Bare `Symbol` now resolves to a real `SymbolConstructor`-shaped interface
(WellKnownSymbolTypes.SymbolConstructor) instead of `any`, so e.g.
`var s = Symbol; {[s]: 0}` is correctly rejected — tsc types `s` as
SymbolConstructor, not `any`.
- `Symbol.for`/`Symbol.keyFor`/`Symbol.prototype` referenced as plain values
(not called) now get real types in CheckGet, mirroring the typed CALL
form TryCheckBuiltinCall already gives Symbol.for/keyFor. Previously they
fell through to evaluating bare `Symbol`=any then a member access on
`any`=any.
- A user-declared `interface SymbolConstructor { ... }` (tsc's declaration-
merging pattern for augmenting global ambients) takes priority over the
built-in shape, so tests that add their own SymbolConstructor members
keep working. Not a full merge (a file mixing a custom member with a
built-in one like Symbol.for would only see the custom interface) but
covers the real declaration-merging tests, which never mix the two.
Also fixes a pre-existing, unrelated line-attribution bug surfaced only by
the above: "Property 'X' does not exist on interface" (TS2339, 3 call
sites in TypeChecker.Properties.cs/Properties.Helpers.cs) never passed a
`line:`, falling back to the enclosing statement's start line instead of
the member token's own line — wrong whenever the access is not on the
statement's first line (e.g. inside a multi-line object literal).
226 Pass / 53 Fail (was 223/55), 0 regressions.
…d class accessors
Two independent missing duplicate-identifier checks, both surfaced by a
well-known-symbol computed key colliding with itself:
- TS1117 ("An object literal cannot have multiple properties with the same
name"): a second plain property with the same canonical well-known-symbol
name (`{[Symbol.isConcatSpreadable]: 0, [Symbol.isConcatSpreadable]: 1}`)
silently overwrote the first in CheckObject's fields dict. Getter/setter
pairs for the same name are exempt (legitimate); a second plain duplicate
is not.
- TS2300 ("Duplicate identifier"): class accessors were never checked for
duplicates at all — two getters (or two setters) with the same name/
static-ness silently overwrote each other in mutableClass.Getters/Setters.
Fixed for both plain AND well-known-symbol computed accessor names (the
computed case had no name-tracking of any kind before — CheckClassMembers
skipped it entirely, "no static member name to register").
228 Pass / 51 Fail (was 226/53), 0 regressions.
…ethods
Computed symbol-keyed methods (`[Symbol.iterator]() {...}`) were entirely
excluded from the class's method-overload grouping (`.Where(m =>
m.ComputedKey == null)`), and separately handled by a loop that only
considered bodies (`m.Body != null`) — so overload signatures with no body
were silently dropped, and multiple implementations just overwrote each
other in file order with no duplicate detection at all.
Replaced that loop with proper grouping by canonical well-known-symbol name
(across BOTH static and instance declarations — needed so the new static-
consistency check below can compare across the static/instance boundary,
not just within it), running the same validation string-named methods
already got:
- TS2393 ("Duplicate function implementation"): now fires on every
declaration in the group (signatures included) once a second
implementation appears, matching tsc exactly rather than silently
keeping the last one.
- TS2391 ("Function implementation is missing..."): fires on the last
declaration when a group has signatures but no implementation.
- TS2387/TS2388 ("Function overload must (not) be static"): brand new
check, previously missing for every method (not just computed-key ones).
tsc compares consecutive declarations pairwise and reports the mismatch
on the later one, with the message chosen by the earlier one's own
static-ness — verified against symbolProperty42's exact (line, code)
pairs empirically, since deriving it from the compiler source's
neighbor-pair algorithm didn't cleanly map to a single forward pass.
The existing generator-return special-casing for [Symbol.iterator]/
[Symbol.asyncIterator] factory signatures is preserved, now applied to
whichever declaration is chosen as the group's representative (the sole
implementation, or the last declaration as a best-effort fallback when the
group is itself erroneous).
231 Pass / 48 Fail (was 228/51), 0 regressions.
A class member can't be both a method and an accessor, but nothing checked
for it: `[Symbol.toPrimitive](x: I) {}` followed by `get [Symbol.toPrimitive]()`/
`set [Symbol.toPrimitive](x)` silently registered all three under the same
canonical name with no duplicate-identifier error. Methods are registered
before accessors are processed, so each accessor now also checks whether a
method already claimed its (IsStatic, name) key — tsc flags the accessor,
not the method that established the name first.
232 Pass / 47 Fail (was 231/48), 0 regressions.
…selection CheckNullishCoalescing had the same "pick wider of two types, return the wrong one" anti-pattern already found in CheckLogicalAssign and CheckTernary this epic: it picked the wider of the non-nullish-left/right types via IsCompatible(A,B) || IsCompatible(B,A), but always returned the left operand regardless of which clause fired. When the right operand was actually the wider type, the narrower left leaked out instead. Audited CheckLogical (&&/||) as the other unaudited sibling from prior rounds — it was already correct (two separate returns, each matching its own branch). The other two `IsCompatible(A,B) || IsCompatible(B,A)` hits in the codebase (generic bivariant-variance boolean check, type-assertion escape hatch) are unrelated patterns, not instances of this bug.
…mbol index signature Interfaces already checked explicit properties against a string index signature (TS2411) but had no equivalent for a symbol index signature ([s: symbol]: T) — a computed well-known-symbol member (canonical "@@name") could freely violate it. Added the mirror-image check, exempting plain string/number-keyed members the same way the string-index check exempts symbol-keyed ones. Class-side symbol-index checks (own AND cross-hierarchy — symbolProperty30/ 32/34) are NOT covered here: they need per-method (not just per-field) index validation plus, for 34 specifically, an unrelated new "class used before its declaration" check (TS2449). Scoped out as bigger, separate work.
233 Pass / 46 Fail (was 232/47), 0 regressions.
…rface names
Adds the checks tsc emits for symbol used where it isn't allowed:
- 'in' right operand must be an object (TS2322)
- for-in right operand may not be symbol (TS2407); an existing symbol-typed
loop variable fails the left-hand side (TS2405) — needs a new IsDeclaration
flag on Stmt.ForIn to tell 'for (x in o)' from 'for (var x in o)'
- delete of a read-only property is TS2704; the well-known-symbol members of
SymbolConstructor are now marked readonly so 'delete Symbol.iterator' fires
- new Symbol() is TS2350 (Symbol has no construct signature)
- an interface may not be named 'symbol' (TS2427)
Flips symbolType2/3/13/14/20. instanceof operand validation (symbolType1) is
deferred: it needs 'Symbol() || {}' to stay a symbol|{} union, but CheckLogical
collapses it to a bare symbol.
…ause tsc reports TS2449 when a class value is referenced in an earlier (or its own) class's extends clause before that class is textually declared — class values, unlike types, aren't hoisted for that position. Track each class's declaration line during hoisting and compare against the extends site. Recorded rather than thrown so the deriving class body is still checked. Flips symbolProperty33 (symbolProperty34 still needs the TS2411 class-side symbol-index check, out of scope here).
…ects to empty classes Two structural-assignability gaps behind the symbol-keyed member tests: - A computed class field '[Symbol.iterator]: T' was keyed by the synthetic '<computed>' lexeme instead of its canonical '@@iterator' name (methods and accessors already canonicalize), so it never lined up with the same member on an interface — producing spurious missing-property errors. Route all field registration/lookup through a shared GetFieldMemberName helper. - An empty (member-less, index-less, unbranded) class target returned 'not assignable' for every source, but an object-like source (interface value / record) is assignable to '{}'. Add an opt-in flag so only the interface/record -> instance path relaxes; class-vs-class stays nominal. Flips symbolProperty9/10/11/12.
…cal shared member TS2320: 'interface I3 extends I1, I2' is an error when I1 and I2 both declare the same member with types that aren't identical. Compare each pair of base members by mutual assignability (conservative, so a genuinely-shared diamond member never trips it) and report once at the interface name. Flips symbolProperty35.
…x signature TS2411: a class's symbol-keyed members (own and inherited) must be assignable to its effective '[s: symbol]' index type (own or inherited). Runs after method bodies so an un-annotated '[Symbol.x]()' carries its inferred return type. tsc's line attribution reproduced from all three cases: an own member reports at the member; an inherited member (paired with an own index signature) reports at the index signature; a fully-inherited pair is skipped (reported on the base). Flips symbolProperty30/32. symbolProperty34 stays deferred: its base class is declared after the derived one, so the forward-referenced base resolves to Any and its index signature isn't visible when the derived class is checked.
BigInt's parameter type is 'string | number | bigint | boolean' (a boolean coerces to 0n/1n); the checker rejected boolean. Flips constructBigint.
…ember target An unconstrained type parameter has no apparent type, so it was assignable to nothing. tsc still relates it to an all-optional / empty object target (its 'subtyping assumes transitivity for optional properties' allowance) — but not to a primitive, 'object', or a target with any required member. Flips subtypingWithOptionalProperties.
…ptionalProperties now pass
A computed DATA-property name on a class (unlike a method/accessor) must be a literal type or a 'unique symbol'; a plain non-unique 'symbol' (e.g. [Symbol()]) is not allowed, while a well-known symbol ([Symbol.iterator], a unique symbol) is fine. Flips symbolProperty7.
A value whose type can't be a property key — e.g. a function type like the 'Symbol.for' method — cannot be used as an index (TS2538), distinct from the implicit-any TS7053 used for a valid-but-unmodeled key. Flips symbolProperty53.
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.
Advances the TypeScript conformance suite (
SharpTS.TypeScriptConformance/, epic #80 / #99) across six rounds of type-checker work. Baseline on the 296-test lib-sensitive subset: 216 → 250 Pass / 62 → 29 Fail / 0 ParseError. Zero regressions at every step; full xUnit 15452/0, Test262 and TS-conformance baselines green throughout.Almost every fix is a general checker correctness improvement that a symbol test happened to exercise — not symbol-specific plumbing.
Round 6 — Symbols duplicate-declaration cluster + sibling bugs
declare function+varfalse-positive TS2403 (var/const-arrow hoisting placeholder)CheckLogicalAssign/CheckTernaryfamily)Namespace/Modulenever had a compatibility relation inIsCompatibleCoreRound 7 — symbol operands in operator/statement positions
inRHS (TS2322), for-in symbol RHS/LHS (TS2407/TS2405, newStmt.ForIn.IsDeclaration),new Symbol()(TS2350), interface namedsymbol(TS2427),delete Symbol.iterator(TS2704)extendsclause (TS2449)Round 8 — symbol-keyed member structural assignability
@@name(like methods/accessors) so they match interface membersRound 9 — class member vs
[s: symbol]index signature (TS2411)Round 10
BigInt()accepts a boolean argument; an unconstrained type parameter relates to a no-required-member targetRound 11
Tests flipped: symbolProperty3/33/9/10/11/12/35/30/32/7/53 and others, symbolType2/3/13/14/20, constructBigint, subtypingWithOptionalProperties, subtypingWithCallSignatures2/3/4, and more.