Skip to content

Commit

Permalink
Flag JS Literals and ignore assignments/accesses to invalid props, in…
Browse files Browse the repository at this point in the history
…stead of adding an index (#25996)

* Remove index signatures from js literals, use an object flag to indicate errors should be ignored instead

* Add focused test on the keyof problem

* Fix fourslash test

* Reenable errors with noImplicitAny flag

* Also disable excess property checks outside of noImplicitAny mode for js literals

* Edit and move comments
  • Loading branch information
weswigham authored Aug 2, 2018
1 parent eb763f0 commit fefc47f
Show file tree
Hide file tree
Showing 65 changed files with 526 additions and 369 deletions.
63 changes: 56 additions & 7 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -4809,13 +4808,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) {
Expand Down Expand Up @@ -5123,7 +5124,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;
}
}

Expand Down Expand Up @@ -9074,6 +9077,34 @@ namespace ts {
return type;
}

/**
* Returns if a type is or consists of a JSLiteral object type
* 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 possibly existing properties.
* This mirrors the behavior of the index signature propagation, to which this behaves similarly (but doesn't affect assignability or inference).
*/
function isJSLiteralType(type: Type): boolean {
if (noImplicitAny) {
return false; // Flag is meaningless under `noImplicitAny` mode
}
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) :
Expand Down Expand Up @@ -9122,6 +9153,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)) {
Expand All @@ -9142,6 +9176,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)) {
Expand Down Expand Up @@ -11217,6 +11254,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) &&
Expand Down Expand Up @@ -12705,7 +12745,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;
}
Expand Down Expand Up @@ -12784,9 +12824,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) {
Expand Down Expand Up @@ -16791,12 +16833,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;
}
Expand Down Expand Up @@ -17872,6 +17917,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);
}
Expand Down Expand Up @@ -20009,7 +20057,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;
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3824,6 +3824,7 @@ namespace ts {
ReverseMapped = 1 << 11, // Object contains a property from a reverse-mapped type
JsxAttributes = 1 << 12, // Jsx attributes type
MarkerType = 1 << 13, // Marker type used for variance probing
JSLiteral = 1 << 14, // Object type declared in JS - disables errors on read/write of nonexisting members
ClassOrInterface = Class | Interface
}

Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2231,6 +2231,7 @@ declare namespace ts {
ReverseMapped = 2048,
JsxAttributes = 4096,
MarkerType = 8192,
JSLiteral = 16384,
ClassOrInterface = 3
}
interface ObjectType extends Type {
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2231,6 +2231,7 @@ declare namespace ts {
ReverseMapped = 2048,
JsxAttributes = 4096,
MarkerType = 8192,
JSLiteral = 16384,
ClassOrInterface = 3
}
interface ObjectType extends Type {
Expand Down
26 changes: 13 additions & 13 deletions tests/baselines/reference/chainedPrototypeAssignment.types
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,30 @@ var mod = require('./mod');
>'./mod' : "./mod"

var a = new mod.A()
>a : A & { [x: string]: any; m(n: number): number; }
>new mod.A() : A & { [x: string]: any; m(n: number): number; }
>a : A & { m(n: number): number; }
>new mod.A() : A & { m(n: number): number; }
>mod.A : typeof A
>mod : typeof import("tests/cases/conformance/salsa/mod")
>A : typeof A

var b = new mod.B()
>b : B & { [x: string]: any; m(n: number): number; }
>new mod.B() : B & { [x: string]: any; m(n: number): number; }
>b : B & { m(n: number): number; }
>new mod.B() : B & { m(n: number): number; }
>mod.B : typeof B
>mod : typeof import("tests/cases/conformance/salsa/mod")
>B : typeof B

a.m('nope')
>a.m('nope') : number
>a.m : (n: number) => number
>a : A & { [x: string]: any; m(n: number): number; }
>a : A & { m(n: number): number; }
>m : (n: number) => number
>'nope' : "nope"

b.m('not really')
>b.m('not really') : number
>b.m : (n: number) => number
>b : B & { [x: string]: any; m(n: number): number; }
>b : B & { m(n: number): number; }
>m : (n: number) => number
>'not really' : "not really"

Expand Down Expand Up @@ -81,15 +81,15 @@ exports.B = B
>B : typeof B

A.prototype = B.prototype = {
>A.prototype = B.prototype = { /** @param {number} n */ m(n) { return n + 1 }} : { [x: string]: any; m(n: number): number; }
>A.prototype : { [x: string]: any; m(n: number): number; }
>A.prototype = B.prototype = { /** @param {number} n */ m(n) { return n + 1 }} : { m(n: number): number; }
>A.prototype : { m(n: number): number; }
>A : typeof A
>prototype : { [x: string]: any; m(n: number): number; }
>B.prototype = { /** @param {number} n */ m(n) { return n + 1 }} : { [x: string]: any; m(n: number): number; }
>B.prototype : { [x: string]: any; m(n: number): number; }
>prototype : { m(n: number): number; }
>B.prototype = { /** @param {number} n */ m(n) { return n + 1 }} : { m(n: number): number; }
>B.prototype : { m(n: number): number; }
>B : typeof B
>prototype : { [x: string]: any; m(n: number): number; }
>{ /** @param {number} n */ m(n) { return n + 1 }} : { [x: string]: any; m(n: number): number; }
>prototype : { m(n: number): number; }
>{ /** @param {number} n */ m(n) { return n + 1 }} : { m(n: number): number; }

/** @param {number} n */
m(n) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
=== tests/cases/compiler/bug25434.js ===
// should not crash while checking
function Test({ b = '' } = {}) {}
>Test : ({ b }?: { [x: string]: any; }) => void
>Test : ({ b }?: {}) => void
>b : string
>'' : ""
>{} : { b?: string; }

Test(({ b = '5' } = {}));
>Test(({ b = '5' } = {})) : void
>Test : ({ b }?: { [x: string]: any; }) => void
>Test : ({ b }?: {}) => void
>({ b = '5' } = {}) : { b?: any; }
>{ b = '5' } = {} : { b?: any; }
>{ b = '5' } : { [x: string]: any; b?: any; }
>{ b = '5' } : { b?: any; }
>b : any
>'5' : "5"
>{} : { b?: any; }
Expand Down
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"'.

25 changes: 25 additions & 0 deletions tests/baselines/reference/checkJsObjectLiteralHasCheckedKeyof.js
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"

Loading

0 comments on commit fefc47f

Please sign in to comment.