-
Notifications
You must be signed in to change notification settings - Fork 13.2k
Flag JS Literals and ignore assignments/accesses to invalid props, instead of adding an index #25996
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Flag JS Literals and ignore assignments/accesses to invalid props, instead of adding an index #25996
Changes from 8 commits
2389eac
b3096e7
3503b1f
3623084
943c5ac
ce8bff9
8d41b63
9ed6bed
ff4a711
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -425,7 +425,6 @@ namespace ts { | |
| const resolvingSignaturesArray = [resolvingSignature]; | ||
|
|
||
| const enumNumberIndexInfo = createIndexInfo(stringType, /*isReadonly*/ true); | ||
| const jsObjectLiteralIndexInfo = createIndexInfo(anyType, /*isReadonly*/ false); | ||
|
|
||
| const globals = createSymbolTable(); | ||
| let amalgamatedDuplicates: Map<{ firstFile: SourceFile, secondFile: SourceFile, firstFileInstances: Map<{ instances: Node[], blockScoped: boolean }>, secondFileInstances: Map<{ instances: Node[], blockScoped: boolean }> }> | undefined; | ||
|
|
@@ -4807,13 +4806,15 @@ namespace ts { | |
| members.set(name, s); | ||
| } | ||
| }); | ||
| return createAnonymousType( | ||
| const result = createAnonymousType( | ||
| exportedType.symbol, | ||
| members, | ||
| exportedType.callSignatures, | ||
| exportedType.constructSignatures, | ||
| exportedType.stringIndexInfo, | ||
| exportedType.numberIndexInfo); | ||
| result.objectFlags |= (getObjectFlags(type) & ObjectFlags.JSLiteral); // Propagate JSLiteral flag | ||
| return result; | ||
| } | ||
| if (isEmptyArrayLiteralType(type)) { | ||
| if (noImplicitAny) { | ||
|
|
@@ -5121,7 +5122,9 @@ namespace ts { | |
| if (s && hasEntries(s.exports)) { | ||
| mergeSymbolTable(exports, s.exports); | ||
| } | ||
| return createAnonymousType(symbol, exports, emptyArray, emptyArray, jsObjectLiteralIndexInfo, undefined); | ||
| const type = createAnonymousType(symbol, exports, emptyArray, emptyArray, undefined, undefined); | ||
| type.objectFlags |= ObjectFlags.JSLiteral; | ||
| return type; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -9072,6 +9075,34 @@ namespace ts { | |
| return type; | ||
| } | ||
|
|
||
| /** | ||
| * Returns if a type is or consists of a JSLiteral object type | ||
| */ | ||
| function isJSLiteralType(type: Type): boolean { | ||
| if (noImplicitAny) { | ||
| return false; // Flag is meaningless under `noImplicitAny` mode | ||
| } | ||
| // In addition to objects which are directly literals, | ||
| // * unions where every element is a jsliteral | ||
| // * intersections where at least one element is a jsliteral | ||
| // * and instantiable types constrained to a jsliteral | ||
| // Should all count as literals and not print errors on access or assignment of imaginary properties. | ||
|
||
| // This mirrors the behavior of the index signature propagation in the old implementation which utilized one. | ||
| if (getObjectFlags(type) & ObjectFlags.JSLiteral) { | ||
| return true; | ||
| } | ||
| if (type.flags & TypeFlags.Union) { | ||
| return every((type as UnionType).types, isJSLiteralType); | ||
| } | ||
| if (type.flags & TypeFlags.Intersection) { | ||
| return some((type as IntersectionType).types, isJSLiteralType); | ||
| } | ||
| if (type.flags & TypeFlags.Instantiable) { | ||
| return isJSLiteralType(getResolvedBaseConstraint(type)); | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| function getPropertyTypeForIndexType(objectType: Type, indexType: Type, accessNode: ElementAccessExpression | IndexedAccessTypeNode | undefined, cacheSymbol: boolean) { | ||
| const accessExpression = accessNode && accessNode.kind === SyntaxKind.ElementAccessExpression ? accessNode : undefined; | ||
| const propName = isTypeUsableAsLateBoundName(indexType) ? getLateBoundNameFromType(indexType) : | ||
|
|
@@ -9120,6 +9151,9 @@ namespace ts { | |
| if (indexType.flags & TypeFlags.Never) { | ||
| return neverType; | ||
| } | ||
| if (isJSLiteralType(objectType)) { | ||
| return anyType; | ||
| } | ||
| if (accessExpression && !isConstEnumObjectType(objectType)) { | ||
| if (noImplicitAny && !compilerOptions.suppressImplicitAnyIndexErrors) { | ||
| if (getIndexTypeOfType(objectType, IndexKind.Number)) { | ||
|
|
@@ -9140,6 +9174,9 @@ namespace ts { | |
| return anyType; | ||
| } | ||
| } | ||
| if (isJSLiteralType(objectType)) { | ||
| return anyType; | ||
| } | ||
| if (accessNode) { | ||
| const indexNode = accessNode.kind === SyntaxKind.ElementAccessExpression ? accessNode.argumentExpression : accessNode.indexType; | ||
| if (indexType.flags & (TypeFlags.StringLiteral | TypeFlags.NumberLiteral)) { | ||
|
|
@@ -11215,6 +11252,9 @@ namespace ts { | |
| } | ||
|
|
||
| function hasExcessProperties(source: FreshObjectLiteralType, target: Type, discriminant: Type | undefined, reportErrors: boolean): boolean { | ||
| if (!noImplicitAny && getObjectFlags(target) & ObjectFlags.JSLiteral) { | ||
| return false; // Disable excess property checks on JS literals to simulate having an implicit "index signature" - but only outside of noImplicitAny | ||
| } | ||
| if (maybeTypeOfKind(target, TypeFlags.Object) && !(getObjectFlags(target) & ObjectFlags.ObjectLiteralPatternWithComputedProperties)) { | ||
| const isComparingJsxAttributes = !!(getObjectFlags(source) & ObjectFlags.JsxAttributes); | ||
| if ((relation === assignableRelation || relation === definitelyAssignableRelation || relation === comparableRelation) && | ||
|
|
@@ -12703,7 +12743,7 @@ namespace ts { | |
| resolved.stringIndexInfo, | ||
| resolved.numberIndexInfo); | ||
| regularNew.flags = resolved.flags & ~TypeFlags.FreshLiteral; | ||
| regularNew.objectFlags |= ObjectFlags.ObjectLiteral; | ||
| regularNew.objectFlags |= ObjectFlags.ObjectLiteral | (getObjectFlags(resolved) & ObjectFlags.JSLiteral); | ||
| (<FreshObjectLiteralType>type).regularType = regularNew; | ||
| return regularNew; | ||
| } | ||
|
|
@@ -12782,9 +12822,11 @@ namespace ts { | |
| } | ||
| const stringIndexInfo = getIndexInfoOfType(type, IndexKind.String); | ||
| const numberIndexInfo = getIndexInfoOfType(type, IndexKind.Number); | ||
| return createAnonymousType(type.symbol, members, emptyArray, emptyArray, | ||
| const result = createAnonymousType(type.symbol, members, emptyArray, emptyArray, | ||
| stringIndexInfo && createIndexInfo(getWidenedType(stringIndexInfo.type), stringIndexInfo.isReadonly), | ||
| numberIndexInfo && createIndexInfo(getWidenedType(numberIndexInfo.type), numberIndexInfo.isReadonly)); | ||
| result.objectFlags |= (getObjectFlags(type) & ObjectFlags.JSLiteral); // Retain js literal flag through widening | ||
| return result; | ||
| } | ||
|
|
||
| function getWidenedType(type: Type) { | ||
|
|
@@ -16789,12 +16831,15 @@ namespace ts { | |
| return createObjectLiteralType(); | ||
|
|
||
| function createObjectLiteralType() { | ||
| const stringIndexInfo = isJSObjectLiteral ? jsObjectLiteralIndexInfo : hasComputedStringProperty ? getObjectLiteralIndexInfo(node.properties, offset, propertiesArray, IndexKind.String) : undefined; | ||
| const stringIndexInfo = hasComputedStringProperty ? getObjectLiteralIndexInfo(node.properties, offset, propertiesArray, IndexKind.String) : undefined; | ||
| const numberIndexInfo = hasComputedNumberProperty && !isJSObjectLiteral ? getObjectLiteralIndexInfo(node.properties, offset, propertiesArray, IndexKind.Number) : undefined; | ||
| const result = createAnonymousType(node.symbol, propertiesTable, emptyArray, emptyArray, stringIndexInfo, numberIndexInfo); | ||
| const freshObjectLiteralFlag = compilerOptions.suppressExcessPropertyErrors ? 0 : TypeFlags.FreshLiteral; | ||
| result.flags |= TypeFlags.ContainsObjectLiteral | freshObjectLiteralFlag | (typeFlags & TypeFlags.PropagatingFlags); | ||
| result.objectFlags |= ObjectFlags.ObjectLiteral; | ||
| if (isJSObjectLiteral) { | ||
| result.objectFlags |= ObjectFlags.JSLiteral; | ||
| } | ||
| if (patternWithComputedProperties) { | ||
| result.objectFlags |= ObjectFlags.ObjectLiteralPatternWithComputedProperties; | ||
| } | ||
|
|
@@ -17870,6 +17915,9 @@ namespace ts { | |
| if (!prop) { | ||
| const indexInfo = getIndexInfoOfType(apparentType, IndexKind.String); | ||
| if (!(indexInfo && indexInfo.type)) { | ||
| if (isJSLiteralType(leftType)) { | ||
| return anyType; | ||
| } | ||
| if (right.escapedText && !checkAndReportErrorForExtendingInterface(node)) { | ||
| reportNonexistentProperty(right, leftType.flags & TypeFlags.TypeParameter && (leftType as TypeParameter).isThisType ? apparentType : leftType); | ||
| } | ||
|
|
@@ -20007,7 +20055,8 @@ namespace ts { | |
| if (decl) { | ||
| const jsSymbol = getSymbolOfNode(decl); | ||
| if (jsSymbol && hasEntries(jsSymbol.exports)) { | ||
| jsAssignmentType = createAnonymousType(jsSymbol, jsSymbol.exports, emptyArray, emptyArray, jsObjectLiteralIndexInfo, undefined); | ||
| jsAssignmentType = createAnonymousType(jsSymbol, jsSymbol.exports, emptyArray, emptyArray, undefined, undefined); | ||
| (jsAssignmentType as ObjectType).objectFlags |= ObjectFlags.JSLiteral; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| tests/cases/compiler/file.js(11,1): error TS2322: Type '"z"' is not assignable to type '"x" | "y"'. | ||
|
|
||
|
|
||
| ==== tests/cases/compiler/file.js (1 errors) ==== | ||
| // @ts-check | ||
| const obj = { | ||
| x: 1, | ||
| y: 2 | ||
| }; | ||
|
|
||
| /** | ||
| * @type {keyof typeof obj} | ||
| */ | ||
| let selected = "x"; | ||
| selected = "z"; // should fail | ||
| ~~~~~~~~ | ||
| !!! error TS2322: Type '"z"' is not assignable to type '"x" | "y"'. | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| //// [file.js] | ||
| // @ts-check | ||
| const obj = { | ||
| x: 1, | ||
| y: 2 | ||
| }; | ||
|
|
||
| /** | ||
| * @type {keyof typeof obj} | ||
| */ | ||
| let selected = "x"; | ||
| selected = "z"; // should fail | ||
|
|
||
|
|
||
| //// [file.js] | ||
| // @ts-check | ||
| var obj = { | ||
| x: 1, | ||
| y: 2 | ||
| }; | ||
| /** | ||
| * @type {keyof typeof obj} | ||
| */ | ||
| var selected = "x"; | ||
| selected = "z"; // should fail |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| === tests/cases/compiler/file.js === | ||
| // @ts-check | ||
| const obj = { | ||
| >obj : Symbol(obj, Decl(file.js, 1, 5)) | ||
|
|
||
| x: 1, | ||
| >x : Symbol(x, Decl(file.js, 1, 13)) | ||
|
|
||
| y: 2 | ||
| >y : Symbol(y, Decl(file.js, 2, 9)) | ||
|
|
||
| }; | ||
|
|
||
| /** | ||
| * @type {keyof typeof obj} | ||
| */ | ||
| let selected = "x"; | ||
| >selected : Symbol(selected, Decl(file.js, 9, 3)) | ||
|
|
||
| selected = "z"; // should fail | ||
| >selected : Symbol(selected, Decl(file.js, 9, 3)) | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| === tests/cases/compiler/file.js === | ||
| // @ts-check | ||
| const obj = { | ||
| >obj : { x: number; y: number; } | ||
| >{ x: 1, y: 2} : { x: number; y: number; } | ||
|
|
||
| x: 1, | ||
| >x : number | ||
| >1 : 1 | ||
|
|
||
| y: 2 | ||
| >y : number | ||
| >2 : 2 | ||
|
|
||
| }; | ||
|
|
||
| /** | ||
| * @type {keyof typeof obj} | ||
| */ | ||
| let selected = "x"; | ||
| >selected : "x" | "y" | ||
| >"x" : "x" | ||
|
|
||
| selected = "z"; // should fail | ||
| >selected = "z" : "z" | ||
| >selected : "x" | "y" | ||
| >"z" : "z" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would move the comment block up here.