Skip to content

Commit e86e10d

Browse files
RyanCavanaughRyanCavanaugh
committed
Refactor to move write type to SymbolLinks
1 parent 6f09705 commit e86e10d

File tree

5 files changed

+69
-54
lines changed

5 files changed

+69
-54
lines changed

‎src/compiler/checker.ts

Lines changed: 59 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -8723,7 +8723,7 @@ namespace ts {
87238723
return links.type;
87248724
}
87258725

8726-
function getTypeOfVariableOrParameterOrPropertyWorker(symbol: Symbol) {
8726+
function getTypeOfVariableOrParameterOrPropertyWorker(symbol: Symbol): Type {
87278727
// Handle prototype property
87288728
if (symbol.flags & SymbolFlags.Prototype) {
87298729
return getTypeOfPrototypeProperty(symbol);
@@ -8763,7 +8763,7 @@ namespace ts {
87638763
}
87648764
return reportCircularityError(symbol);
87658765
}
8766-
let type: Type | undefined;
8766+
let type: Type;
87678767
if (declaration.kind === SyntaxKind.ExportAssignment) {
87688768
type = widenTypeForVariableLikeDeclaration(checkExpressionCached((<ExportAssignment>declaration).expression), declaration);
87698769
}
@@ -8820,7 +8820,7 @@ namespace ts {
88208820
type = getTypeOfEnumMember(symbol);
88218821
}
88228822
else if (isAccessor(declaration)) {
8823-
type = resolveTypeOfAccessors(symbol);
8823+
type = resolveTypeOfAccessors(symbol) || Debug.fail("Non-write accessor resolution must always produce a type");
88248824
}
88258825
else {
88268826
return Debug.fail("Unhandled declaration kind! " + Debug.formatSyntaxKind(declaration.kind) + " for " + Debug.formatSymbol(symbol));
@@ -8866,15 +8866,20 @@ namespace ts {
88668866

88678867
function getTypeOfAccessors(symbol: Symbol): Type {
88688868
const links = getSymbolLinks(symbol);
8869-
return links.type || (links.type = getTypeOfAccessorsWorker(symbol));
8869+
return links.type || (links.type = getTypeOfAccessorsWorker(symbol) || Debug.fail("Read type of accessor must always produce a type"));
8870+
}
8871+
8872+
function getTypeOfSetAccessor(symbol: Symbol): Type | undefined {
8873+
const links = getSymbolLinks(symbol);
8874+
return links.writeType || (links.writeType = getTypeOfAccessorsWorker(symbol, /*isWrite*/ true));
88708875
}
88718876

8872-
function getTypeOfAccessorsWorker(symbol: Symbol): Type {
8877+
function getTypeOfAccessorsWorker(symbol: Symbol, writing = false): Type | undefined {
88738878
if (!pushTypeResolution(symbol, TypeSystemPropertyName.Type)) {
88748879
return errorType;
88758880
}
88768881

8877-
let type = resolveTypeOfAccessors(symbol);
8882+
let type = resolveTypeOfAccessors(symbol, writing);
88788883

88798884
if (!popTypeResolution()) {
88808885
type = anyType;
@@ -8886,49 +8891,58 @@ namespace ts {
88868891
return type;
88878892
}
88888893

8889-
function resolveTypeOfAccessors(symbol: Symbol) {
8894+
function resolveTypeOfAccessors(symbol: Symbol, writing = false) {
88908895
const getter = getDeclarationOfKind<AccessorDeclaration>(symbol, SyntaxKind.GetAccessor);
88918896
const setter = getDeclarationOfKind<AccessorDeclaration>(symbol, SyntaxKind.SetAccessor);
88928897

8898+
// For write operations, prioritize type annotations on the setter
8899+
if (writing) {
8900+
const setterParameterType = getAnnotatedAccessorType(setter);
8901+
if (setterParameterType) {
8902+
return setterParameterType;
8903+
}
8904+
}
8905+
// Else defer to the getter type
8906+
88938907
if (getter && isInJSFile(getter)) {
88948908
const jsDocType = getTypeForDeclarationFromJSDocComment(getter);
88958909
if (jsDocType) {
88968910
return jsDocType;
88978911
}
88988912
}
8899-
// First try to see if the user specified a return type on the get-accessor.
8913+
8914+
// Try to see if the user specified a return type on the get-accessor.
89008915
const getterReturnType = getAnnotatedAccessorType(getter);
89018916
if (getterReturnType) {
89028917
return getterReturnType;
89038918
}
8904-
else {
8905-
// If the user didn't specify a return type, try to use the set-accessor's parameter type.
8906-
const setterParameterType = getAnnotatedAccessorType(setter);
8907-
if (setterParameterType) {
8908-
return setterParameterType;
8919+
8920+
// If the user didn't specify a return type, try to use the set-accessor's parameter type.
8921+
const setterParameterType = getAnnotatedAccessorType(setter);
8922+
if (setterParameterType) {
8923+
return setterParameterType;
8924+
}
8925+
8926+
// If there are no specified types, try to infer it from the body of the get accessor if it exists.
8927+
if (getter && getter.body) {
8928+
return getReturnTypeFromBody(getter);
8929+
}
8930+
8931+
// Otherwise, fall back to 'any'.
8932+
if (setter) {
8933+
if (!isPrivateWithinAmbient(setter)) {
8934+
errorOrSuggestion(noImplicitAny, setter, Diagnostics.Property_0_implicitly_has_type_any_because_its_set_accessor_lacks_a_parameter_type_annotation, symbolToString(symbol));
89098935
}
8910-
else {
8911-
// If there are no specified types, try to infer it from the body of the get accessor if it exists.
8912-
if (getter && getter.body) {
8913-
return getReturnTypeFromBody(getter);
8914-
}
8915-
// Otherwise, fall back to 'any'.
8916-
else {
8917-
if (setter) {
8918-
if (!isPrivateWithinAmbient(setter)) {
8919-
errorOrSuggestion(noImplicitAny, setter, Diagnostics.Property_0_implicitly_has_type_any_because_its_set_accessor_lacks_a_parameter_type_annotation, symbolToString(symbol));
8920-
}
8921-
}
8922-
else {
8923-
Debug.assert(!!getter, "there must exist a getter as we are current checking either setter or getter in this function");
8924-
if (!isPrivateWithinAmbient(getter)) {
8925-
errorOrSuggestion(noImplicitAny, getter, Diagnostics.Property_0_implicitly_has_type_any_because_its_get_accessor_lacks_a_return_type_annotation, symbolToString(symbol));
8926-
}
8927-
}
8928-
return anyType;
8929-
}
8936+
return anyType;
8937+
}
8938+
else if (getter) {
8939+
Debug.assert(!!getter, "there must exist a getter as we are current checking either setter or getter in this function");
8940+
if (!isPrivateWithinAmbient(getter)) {
8941+
errorOrSuggestion(noImplicitAny, getter, Diagnostics.Property_0_implicitly_has_type_any_because_its_get_accessor_lacks_a_return_type_annotation, symbolToString(symbol));
89308942
}
8943+
return anyType;
89318944
}
8945+
return undefined;
89328946
}
89338947

89348948
function getBaseTypeVariableOfClass(symbol: Symbol) {
@@ -9055,6 +9069,16 @@ namespace ts {
90559069
return links.type;
90569070
}
90579071

9072+
function getWriteTypeOfSymbol(symbol: Symbol): Type {
9073+
if (symbol.flags & SymbolFlags.Accessor) {
9074+
const type = getTypeOfSetAccessor(symbol);
9075+
if (type) {
9076+
return type;
9077+
}
9078+
}
9079+
return getTypeOfSymbol(symbol);
9080+
}
9081+
90589082
function getTypeOfSymbol(symbol: Symbol): Type {
90599083
const checkFlags = getCheckFlags(symbol);
90609084
if (checkFlags & CheckFlags.DeferredType) {
@@ -25968,17 +25992,9 @@ namespace ts {
2596825992
function checkPropertyAccessibility(
2596925993
node: PropertyAccessExpression | QualifiedName | PropertyAccessExpression | VariableDeclaration | ParameterDeclaration | ImportTypeNode | PropertyAssignment | ShorthandPropertyAssignment | BindingElement,
2597025994
isSuper: boolean, type: Type, prop: Symbol, isWrite = false): boolean {
25971-
let flags = getDeclarationModifierFlagsFromSymbol(prop);
25995+
const flags = getDeclarationModifierFlagsFromSymbol(prop, isWrite);
2597225996
const errorNode = node.kind === SyntaxKind.QualifiedName ? node.right : node.kind === SyntaxKind.ImportType ? node : node.name;
2597325997

25974-
// Writes use the visibility modifier of the set accessor, if one is declared
25975-
if (isWrite) {
25976-
const setter = forEach(prop.declarations, p => p.kind === SyntaxKind.SetAccessor && p);
25977-
if (setter) {
25978-
flags = (flags & ~ModifierFlags.AccessibilityModifier) | getEffectiveDeclarationFlags(setter, ModifierFlags.AccessibilityModifier);
25979-
}
25980-
}
25981-
2598225998
if (isSuper) {
2598325999
// TS 1.0 spec (April 2014): 4.8.2
2598426000
// - In a constructor, instance member function, instance member accessor, or
@@ -26336,15 +26352,9 @@ namespace ts {
2633626352
error(right, Diagnostics.Cannot_assign_to_0_because_it_is_a_read_only_property, idText(right));
2633726353
return errorType;
2633826354
}
26339-
if (isWrite) {
26340-
const setter = forEach(prop.declarations, p => (p.kind === SyntaxKind.SetAccessor && p));
26341-
if (setter) {
26342-
propType = getAnnotatedAccessorType(setter as SetAccessorDeclaration);
26343-
}
26344-
}
2634526355

2634626356
if (propType === undefined) {
26347-
propType = isThisPropertyAccessInConstructor(node, prop) ? autoType : getConstraintForLocation(getTypeOfSymbol(prop), node);
26357+
propType = isThisPropertyAccessInConstructor(node, prop) ? autoType : getConstraintForLocation(isWrite ? getWriteTypeOfSymbol(prop) : getTypeOfSymbol(prop), node);
2634826358
}
2634926359
}
2635026360
// For writes to properties declared as accessors, use the 'set' type

‎src/compiler/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4739,6 +4739,7 @@ namespace ts {
47394739
immediateTarget?: Symbol; // Immediate target of an alias. May be another alias. Do not access directly, use `checker.getImmediateAliasedSymbol` instead.
47404740
target?: Symbol; // Resolved (non-alias) target of an alias
47414741
type?: Type; // Type of value symbol
4742+
writeType?: Type; // Type of value symbol in write contexts
47424743
nameType?: Type; // Type associated with a late-bound symbol
47434744
uniqueESSymbolType?: Type; // UniqueESSymbol type for a symbol
47444745
declaredType?: Type; // Type of class, interface, enum, type alias, or type parameter

‎src/compiler/utilities.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5262,9 +5262,10 @@ namespace ts {
52625262
return symbol.flags & SymbolFlags.Transient ? (<TransientSymbol>symbol).checkFlags : 0;
52635263
}
52645264

5265-
export function getDeclarationModifierFlagsFromSymbol(s: Symbol): ModifierFlags {
5265+
export function getDeclarationModifierFlagsFromSymbol(s: Symbol, isWrite = false): ModifierFlags {
52665266
if (s.valueDeclaration) {
5267-
const flags = getCombinedModifierFlags(s.valueDeclaration);
5267+
const declaration = (isWrite && forEach(s.declarations, d => d.kind === SyntaxKind.SetAccessor && d)) || s.valueDeclaration;
5268+
const flags = getCombinedModifierFlags(declaration);
52685269
return s.parent && s.parent.flags & SymbolFlags.Class ? flags : flags & ~ModifierFlags.AccessibilityModifier;
52695270
}
52705271
if (getCheckFlags(s) & CheckFlags.Synthetic) {

‎tests/baselines/reference/objectSpreadNegative.errors.txt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,12 @@ tests/cases/conformance/types/spread/objectSpreadNegative.ts(52,20): error TS269
2222
tests/cases/conformance/types/spread/objectSpreadNegative.ts(54,19): error TS2698: Spread types may only be created from object types.
2323
tests/cases/conformance/types/spread/objectSpreadNegative.ts(59,1): error TS2349: This expression is not callable.
2424
Type '{}' has no call signatures.
25+
tests/cases/conformance/types/spread/objectSpreadNegative.ts(63,1): error TS2322: Type '12' is not assignable to type 'undefined'.
2526
tests/cases/conformance/types/spread/objectSpreadNegative.ts(69,9): error TS2339: Property 'm' does not exist on type '{ p: number; }'.
2627
tests/cases/conformance/types/spread/objectSpreadNegative.ts(74,11): error TS2339: Property 'a' does not exist on type '{}'.
2728

2829

29-
==== tests/cases/conformance/types/spread/objectSpreadNegative.ts (22 errors) ====
30+
==== tests/cases/conformance/types/spread/objectSpreadNegative.ts (23 errors) ====
3031
let o = { a: 1, b: 'no' }
3132

3233
/// private propagates
@@ -144,6 +145,8 @@ tests/cases/conformance/types/spread/objectSpreadNegative.ts(74,11): error TS233
144145
// write-only properties get skipped
145146
let setterOnly = { ...{ set b (bad: number) { } } };
146147
setterOnly.b = 12; // error, 'b' does not exist
148+
~~~~~~~~~~~~
149+
!!! error TS2322: Type '12' is not assignable to type 'undefined'.
147150

148151
// methods are skipped because they aren't enumerable
149152
class C { p = 1; m() { } }

‎tests/baselines/reference/objectSpreadNegative.types

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,9 +288,9 @@ let setterOnly = { ...{ set b (bad: number) { } } };
288288

289289
setterOnly.b = 12; // error, 'b' does not exist
290290
>setterOnly.b = 12 : 12
291-
>setterOnly.b : number
291+
>setterOnly.b : undefined
292292
>setterOnly : { b: undefined; }
293-
>b : number
293+
>b : undefined
294294
>12 : 12
295295

296296
// methods are skipped because they aren't enumerable

0 commit comments

Comments
 (0)