From f7808c4e8dca9a97c5f250524a2ef57445952374 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Wed, 18 Dec 2019 14:53:17 -0800 Subject: [PATCH 1/4] Add @readonly The rule for @readonly on this-assignments in the constructor is wrong. See failing tests. --- src/compiler/parser.ts | 4 +++ src/compiler/types.ts | 5 ++++ src/compiler/utilities.ts | 3 +- src/compiler/utilitiesPublic.ts | 9 ++++++ .../cases/conformance/jsdoc/jsdocReadonly.ts | 29 +++++++++++++++++++ .../jsdoc/jsdocReadonlyDeclarations.ts | 20 +++++++++++++ 6 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 tests/cases/conformance/jsdoc/jsdocReadonly.ts create mode 100644 tests/cases/conformance/jsdoc/jsdocReadonlyDeclarations.ts diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 6e4e168e2ca77..1ed276dddc8a4 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -509,6 +509,7 @@ namespace ts { case SyntaxKind.JSDocPublicTag: case SyntaxKind.JSDocPrivateTag: case SyntaxKind.JSDocProtectedTag: + case SyntaxKind.JSDocReadonlyTag: return visitNode(cbNode, (node as JSDocTag).tagName); case SyntaxKind.PartiallyEmittedExpression: return visitNode(cbNode, (node).expression); @@ -6845,6 +6846,9 @@ namespace ts { case "protected": tag = parseSimpleTag(start, SyntaxKind.JSDocProtectedTag, tagName); break; + case "readonly": + tag = parseSimpleTag(start, SyntaxKind.JSDocReadonlyTag, tagName); + break; case "this": tag = parseThisTag(start, tagName); break; diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 393e48695a2a6..2817e4dfd68b5 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -471,6 +471,7 @@ namespace ts { JSDocPublicTag, JSDocPrivateTag, JSDocProtectedTag, + JSDocReadonlyTag, JSDocCallbackTag, JSDocEnumTag, JSDocParameterTag, @@ -2632,6 +2633,10 @@ namespace ts { kind: SyntaxKind.JSDocProtectedTag; } + export interface JSDocReadonlyTag extends JSDocTag { + kind: SyntaxKind.JSDocReadonlyTag; + } + export interface JSDocEnumTag extends JSDocTag, Declaration { parent: JSDoc; kind: SyntaxKind.JSDocEnumTag; diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 0ed664b2703d7..d095541d4ca6c 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -4130,7 +4130,8 @@ namespace ts { // or when !(node.flags & NodeFlags.Synthesized) && node.kind !== SyntaxKind.SourceFile) const tags = (getJSDocPublicTag(node) ? ModifierFlags.Public : ModifierFlags.None) | (getJSDocPrivateTag(node) ? ModifierFlags.Private : ModifierFlags.None) - | (getJSDocProtectedTag(node) ? ModifierFlags.Protected : ModifierFlags.None); + | (getJSDocProtectedTag(node) ? ModifierFlags.Protected : ModifierFlags.None) + | (getJSDocReadonlyTag(node) ? ModifierFlags.Readonly : ModifierFlags.None); flags |= tags; } diff --git a/src/compiler/utilitiesPublic.ts b/src/compiler/utilitiesPublic.ts index 4f27c261938c3..cf5f444260c65 100644 --- a/src/compiler/utilitiesPublic.ts +++ b/src/compiler/utilitiesPublic.ts @@ -688,6 +688,11 @@ namespace ts { return getFirstJSDocTag(node, isJSDocProtectedTag); } + /** Gets the JSDoc protected tag for the node if present */ + export function getJSDocReadonlyTag(node: Node): JSDocReadonlyTag | undefined { + return getFirstJSDocTag(node, isJSDocReadonlyTag); + } + /** Gets the JSDoc enum tag for the node if present */ export function getJSDocEnumTag(node: Node): JSDocEnumTag | undefined { return getFirstJSDocTag(node, isJSDocEnumTag); @@ -1574,6 +1579,10 @@ namespace ts { return node.kind === SyntaxKind.JSDocProtectedTag; } + export function isJSDocReadonlyTag(node: Node): node is JSDocReadonlyTag { + return node.kind === SyntaxKind.JSDocReadonlyTag; + } + export function isJSDocEnumTag(node: Node): node is JSDocEnumTag { return node.kind === SyntaxKind.JSDocEnumTag; } diff --git a/tests/cases/conformance/jsdoc/jsdocReadonly.ts b/tests/cases/conformance/jsdoc/jsdocReadonly.ts new file mode 100644 index 0000000000000..15ead236fbe82 --- /dev/null +++ b/tests/cases/conformance/jsdoc/jsdocReadonly.ts @@ -0,0 +1,29 @@ +// @allowJs: true +// @checkJs: true +// @target: esnext +// @noEmit: true +// @Filename: jsdocReadonly.js + +class LOL { + /** + * @readonly + * @private + * @type {number} + * Order rules do not apply to JSDoc + */ + x = 1 + /** @readonly */ + y = 2 + /** @readonly Definitely not here */ + static z = 3 + /** @readonly This is OK too */ + constructor() { + /** ok */ + this.y = 2 + /** @readonly ok */ + this.ka = 2 + } +} + +var l = new LOL() +l.y = 12 diff --git a/tests/cases/conformance/jsdoc/jsdocReadonlyDeclarations.ts b/tests/cases/conformance/jsdoc/jsdocReadonlyDeclarations.ts new file mode 100644 index 0000000000000..d62aa19d52ce3 --- /dev/null +++ b/tests/cases/conformance/jsdoc/jsdocReadonlyDeclarations.ts @@ -0,0 +1,20 @@ +// @allowJs: true +// @checkJs: true +// @target: esnext +// @out: foo.js +// @declaration: true +// @Filename: jsdocReadonlyDeclarations.js +class C { + /** @readonly */ + x = 6 + /** @readonly */ + constructor(n) { + this.x = n + /** + * @readonly + * @type {number} + */ + this.y = n + } +} +new C().x From 8974fb56e7b6fc058abaa6814bbcdfceba971fc3 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Thu, 19 Dec 2019 14:17:54 -0800 Subject: [PATCH 2/4] In-progress Add ctor function test Add some notes and rename variable --- src/compiler/checker.ts | 11 ++++++++--- .../conformance/jsdoc/jsdocReadonlyDeclarations.ts | 5 +++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 4095b0b4b5c7a..3dd4229e957d7 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -26339,14 +26339,19 @@ namespace ts { (expr.kind === SyntaxKind.PropertyAccessExpression || expr.kind === SyntaxKind.ElementAccessExpression) && (expr as AccessExpression).expression.kind === SyntaxKind.ThisKeyword) { // Look for if this is the constructor for the class that `symbol` is a property of. - const func = getContainingFunction(expr); - if (!(func && func.kind === SyntaxKind.Constructor)) { + const ctor = getContainingFunction(expr); + if (!(ctor && ctor.kind === SyntaxKind.Constructor)) { return true; } + // 1. no valueDeclaration -- some kind of synthetic property, don't allow the write + // 2. normal property declaration -- its parent and the ctor's parent should both be a class + // 3. parameter property declaration -- its parent should BE the ctor itself + // 4. this.property assignment in a class -- it should be a binaryexpression, and ctor.parent should be the symbol.parent.valueDeclaration + // 5. this.property assignment in a ctor func -- it should be a binaryexp, and ctor should be the symbol.parent.valueDeclaration // If func.parent is a class and symbol is a (readonly) property of that class, or // if func is a constructor and symbol is a (readonly) parameter property declared in it, // then symbol is writeable here. - return !symbol.valueDeclaration || !(func.parent === symbol.valueDeclaration.parent || func === symbol.valueDeclaration.parent); + return !symbol.valueDeclaration || !(ctor.parent === symbol.valueDeclaration.parent || ctor === symbol.valueDeclaration.parent); } return true; } diff --git a/tests/cases/conformance/jsdoc/jsdocReadonlyDeclarations.ts b/tests/cases/conformance/jsdoc/jsdocReadonlyDeclarations.ts index d62aa19d52ce3..a9a3f0d16e00b 100644 --- a/tests/cases/conformance/jsdoc/jsdocReadonlyDeclarations.ts +++ b/tests/cases/conformance/jsdoc/jsdocReadonlyDeclarations.ts @@ -18,3 +18,8 @@ class C { } } new C().x + +function F() { + /** @readonly */ + this.z = 1 +} From 55b48d4d61aa6ffb93acbfda4b49ff1441c37fee Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Thu, 19 Dec 2019 16:01:17 -0800 Subject: [PATCH 3/4] Done except for cleanup and fix 1 bug --- src/compiler/checker.ts | 31 ++++++---- .../reference/api/tsserverlibrary.d.ts | 45 ++++++++------ tests/baselines/reference/api/typescript.d.ts | 45 ++++++++------ .../reference/jsDeclarationsClasses.js | 4 +- .../reference/jsdocReadonly.errors.txt | 30 ++++++++++ .../baselines/reference/jsdocReadonly.symbols | 46 ++++++++++++++ tests/baselines/reference/jsdocReadonly.types | 56 +++++++++++++++++ .../reference/jsdocReadonlyDeclarations.js | 60 +++++++++++++++++++ .../jsdocReadonlyDeclarations.symbols | 42 +++++++++++++ .../reference/jsdocReadonlyDeclarations.types | 50 ++++++++++++++++ 10 files changed, 357 insertions(+), 52 deletions(-) create mode 100644 tests/baselines/reference/jsdocReadonly.errors.txt create mode 100644 tests/baselines/reference/jsdocReadonly.symbols create mode 100644 tests/baselines/reference/jsdocReadonly.types create mode 100644 tests/baselines/reference/jsdocReadonlyDeclarations.js create mode 100644 tests/baselines/reference/jsdocReadonlyDeclarations.symbols create mode 100644 tests/baselines/reference/jsdocReadonlyDeclarations.types diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 3dd4229e957d7..6151c167db8c3 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -11964,7 +11964,7 @@ namespace ts { if (prop) { if (accessExpression) { markPropertyAsReferenced(prop, accessExpression, /*isThisAccess*/ accessExpression.expression.kind === SyntaxKind.ThisKeyword); - if (isAssignmentTarget(accessExpression) && (isReferenceToReadonlyEntity(accessExpression, prop) || isReferenceThroughNamespaceImport(accessExpression))) { + if (isAssignmentToReadonlyEntity(accessExpression, prop, getAssignmentTargetKind(accessExpression))) { error(accessExpression.argumentExpression, Diagnostics.Cannot_assign_to_0_because_it_is_a_read_only_property, symbolToString(prop)); return undefined; } @@ -23046,11 +23046,9 @@ namespace ts { markPropertyAsReferenced(prop, node, left.kind === SyntaxKind.ThisKeyword); getNodeLinks(node).resolvedSymbol = prop; checkPropertyAccessibility(node, left.kind === SyntaxKind.SuperKeyword, apparentType, prop); - if (assignmentKind) { - if (isReferenceToReadonlyEntity(node, prop) || isReferenceThroughNamespaceImport(node)) { - error(right, Diagnostics.Cannot_assign_to_0_because_it_is_a_read_only_property, idText(right)); - return errorType; - } + if (isAssignmentToReadonlyEntity(node as Expression, prop, assignmentKind)) { + error(right, Diagnostics.Cannot_assign_to_0_because_it_is_a_read_only_property, idText(right)); + return errorType; } propType = getConstraintForLocation(getTypeOfSymbol(prop), node); } @@ -26332,7 +26330,11 @@ namespace ts { ); } - function isReferenceToReadonlyEntity(expr: Expression, symbol: Symbol): boolean { + function isAssignmentToReadonlyEntity(expr: Expression, symbol: Symbol, assignmentKind: AssignmentKind) { + if (assignmentKind === AssignmentKind.None) { + // no assigment means it doesn't matter whether the entity is readonly + return false; + } if (isReadonlySymbol(symbol)) { // Allow assignments to readonly properties within constructors of the same class declaration. if (symbol.flags & SymbolFlags.Property && @@ -26351,15 +26353,20 @@ namespace ts { // If func.parent is a class and symbol is a (readonly) property of that class, or // if func is a constructor and symbol is a (readonly) parameter property declared in it, // then symbol is writeable here. - return !symbol.valueDeclaration || !(ctor.parent === symbol.valueDeclaration.parent || ctor === symbol.valueDeclaration.parent); + if (symbol.valueDeclaration) { + const isLocalParameterProperty = ctor === symbol.valueDeclaration.parent; + const isLocalPropertyDeclaration = ctor.parent === symbol.valueDeclaration.parent; + const isLocalThisPropertyAssignment = symbol.parent && symbol.parent.valueDeclaration === ctor.parent; // TODO: Make sure that symbol.parent is a function and class + const isLocalThisPropertyAssignmentConstructorFunction = symbol.parent && symbol.parent.valueDeclaration === ctor; // TODO: Make sure that symbol.parent is a function and class + const isLocalProperty = isLocalPropertyDeclaration || isLocalParameterProperty || isLocalThisPropertyAssignment || isLocalThisPropertyAssignmentConstructorFunction ; + return !isLocalProperty; + } + return false; } return true; } - return false; - } - - function isReferenceThroughNamespaceImport(expr: Expression): boolean { if (expr.kind === SyntaxKind.PropertyAccessExpression || expr.kind === SyntaxKind.ElementAccessExpression) { + // references through namespace import should be readonly const node = skipParentheses((expr as AccessExpression).expression); if (node.kind === SyntaxKind.Identifier) { const symbol = getNodeLinks(node).resolvedSymbol!; diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 923a9a4257b0f..bb9a42a783705 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -386,23 +386,24 @@ declare namespace ts { JSDocPublicTag = 308, JSDocPrivateTag = 309, JSDocProtectedTag = 310, - JSDocCallbackTag = 311, - JSDocEnumTag = 312, - JSDocParameterTag = 313, - JSDocReturnTag = 314, - JSDocThisTag = 315, - JSDocTypeTag = 316, - JSDocTemplateTag = 317, - JSDocTypedefTag = 318, - JSDocPropertyTag = 319, - SyntaxList = 320, - NotEmittedStatement = 321, - PartiallyEmittedExpression = 322, - CommaListExpression = 323, - MergeDeclarationMarker = 324, - EndOfDeclarationMarker = 325, - SyntheticReferenceExpression = 326, - Count = 327, + JSDocReadonlyTag = 311, + JSDocCallbackTag = 312, + JSDocEnumTag = 313, + JSDocParameterTag = 314, + JSDocReturnTag = 315, + JSDocThisTag = 316, + JSDocTypeTag = 317, + JSDocTemplateTag = 318, + JSDocTypedefTag = 319, + JSDocPropertyTag = 320, + SyntaxList = 321, + NotEmittedStatement = 322, + PartiallyEmittedExpression = 323, + CommaListExpression = 324, + MergeDeclarationMarker = 325, + EndOfDeclarationMarker = 326, + SyntheticReferenceExpression = 327, + Count = 328, FirstAssignment = 62, LastAssignment = 74, FirstCompoundAssignment = 63, @@ -431,9 +432,9 @@ declare namespace ts { LastStatement = 240, FirstNode = 152, FirstJSDocNode = 292, - LastJSDocNode = 319, + LastJSDocNode = 320, FirstJSDocTagNode = 304, - LastJSDocTagNode = 319, + LastJSDocTagNode = 320, } export enum NodeFlags { None = 0, @@ -1632,6 +1633,9 @@ declare namespace ts { export interface JSDocProtectedTag extends JSDocTag { kind: SyntaxKind.JSDocProtectedTag; } + export interface JSDocReadonlyTag extends JSDocTag { + kind: SyntaxKind.JSDocReadonlyTag; + } export interface JSDocEnumTag extends JSDocTag, Declaration { parent: JSDoc; kind: SyntaxKind.JSDocEnumTag; @@ -3472,6 +3476,8 @@ declare namespace ts { function getJSDocPrivateTag(node: Node): JSDocPrivateTag | undefined; /** Gets the JSDoc protected tag for the node if present */ function getJSDocProtectedTag(node: Node): JSDocProtectedTag | undefined; + /** Gets the JSDoc protected tag for the node if present */ + function getJSDocReadonlyTag(node: Node): JSDocReadonlyTag | undefined; /** Gets the JSDoc enum tag for the node if present */ function getJSDocEnumTag(node: Node): JSDocEnumTag | undefined; /** Gets the JSDoc this tag for the node if present */ @@ -3679,6 +3685,7 @@ declare namespace ts { function isJSDocPublicTag(node: Node): node is JSDocPublicTag; function isJSDocPrivateTag(node: Node): node is JSDocPrivateTag; function isJSDocProtectedTag(node: Node): node is JSDocProtectedTag; + function isJSDocReadonlyTag(node: Node): node is JSDocReadonlyTag; function isJSDocEnumTag(node: Node): node is JSDocEnumTag; function isJSDocThisTag(node: Node): node is JSDocThisTag; function isJSDocParameterTag(node: Node): node is JSDocParameterTag; diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 2d934840e7304..5ec4c0010fad4 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -386,23 +386,24 @@ declare namespace ts { JSDocPublicTag = 308, JSDocPrivateTag = 309, JSDocProtectedTag = 310, - JSDocCallbackTag = 311, - JSDocEnumTag = 312, - JSDocParameterTag = 313, - JSDocReturnTag = 314, - JSDocThisTag = 315, - JSDocTypeTag = 316, - JSDocTemplateTag = 317, - JSDocTypedefTag = 318, - JSDocPropertyTag = 319, - SyntaxList = 320, - NotEmittedStatement = 321, - PartiallyEmittedExpression = 322, - CommaListExpression = 323, - MergeDeclarationMarker = 324, - EndOfDeclarationMarker = 325, - SyntheticReferenceExpression = 326, - Count = 327, + JSDocReadonlyTag = 311, + JSDocCallbackTag = 312, + JSDocEnumTag = 313, + JSDocParameterTag = 314, + JSDocReturnTag = 315, + JSDocThisTag = 316, + JSDocTypeTag = 317, + JSDocTemplateTag = 318, + JSDocTypedefTag = 319, + JSDocPropertyTag = 320, + SyntaxList = 321, + NotEmittedStatement = 322, + PartiallyEmittedExpression = 323, + CommaListExpression = 324, + MergeDeclarationMarker = 325, + EndOfDeclarationMarker = 326, + SyntheticReferenceExpression = 327, + Count = 328, FirstAssignment = 62, LastAssignment = 74, FirstCompoundAssignment = 63, @@ -431,9 +432,9 @@ declare namespace ts { LastStatement = 240, FirstNode = 152, FirstJSDocNode = 292, - LastJSDocNode = 319, + LastJSDocNode = 320, FirstJSDocTagNode = 304, - LastJSDocTagNode = 319, + LastJSDocTagNode = 320, } export enum NodeFlags { None = 0, @@ -1632,6 +1633,9 @@ declare namespace ts { export interface JSDocProtectedTag extends JSDocTag { kind: SyntaxKind.JSDocProtectedTag; } + export interface JSDocReadonlyTag extends JSDocTag { + kind: SyntaxKind.JSDocReadonlyTag; + } export interface JSDocEnumTag extends JSDocTag, Declaration { parent: JSDoc; kind: SyntaxKind.JSDocEnumTag; @@ -3472,6 +3476,8 @@ declare namespace ts { function getJSDocPrivateTag(node: Node): JSDocPrivateTag | undefined; /** Gets the JSDoc protected tag for the node if present */ function getJSDocProtectedTag(node: Node): JSDocProtectedTag | undefined; + /** Gets the JSDoc protected tag for the node if present */ + function getJSDocReadonlyTag(node: Node): JSDocReadonlyTag | undefined; /** Gets the JSDoc enum tag for the node if present */ function getJSDocEnumTag(node: Node): JSDocEnumTag | undefined; /** Gets the JSDoc this tag for the node if present */ @@ -3679,6 +3685,7 @@ declare namespace ts { function isJSDocPublicTag(node: Node): node is JSDocPublicTag; function isJSDocPrivateTag(node: Node): node is JSDocPrivateTag; function isJSDocProtectedTag(node: Node): node is JSDocProtectedTag; + function isJSDocReadonlyTag(node: Node): node is JSDocReadonlyTag; function isJSDocEnumTag(node: Node): node is JSDocEnumTag; function isJSDocThisTag(node: Node): node is JSDocThisTag; function isJSDocParameterTag(node: Node): node is JSDocParameterTag; diff --git a/tests/baselines/reference/jsDeclarationsClasses.js b/tests/baselines/reference/jsDeclarationsClasses.js index 88e5991b09f95..7cf3145119595 100644 --- a/tests/baselines/reference/jsDeclarationsClasses.js +++ b/tests/baselines/reference/jsDeclarationsClasses.js @@ -477,7 +477,7 @@ export class E { * @type {string} * @readonly */ - static staticReadonlyField: string; + static readonly staticReadonlyField: string; static staticInitializedField: number; /** * @param {string} _p @@ -508,7 +508,7 @@ export class E { * @type {T & U} * @readonly */ - readonlyField: T & U; + readonly readonlyField: T & U; initializedField: number; /** * @param {U} _p diff --git a/tests/baselines/reference/jsdocReadonly.errors.txt b/tests/baselines/reference/jsdocReadonly.errors.txt new file mode 100644 index 0000000000000..316775fc1b207 --- /dev/null +++ b/tests/baselines/reference/jsdocReadonly.errors.txt @@ -0,0 +1,30 @@ +tests/cases/conformance/jsdoc/jsdocReadonly.js(23,3): error TS2540: Cannot assign to 'y' because it is a read-only property. + + +==== tests/cases/conformance/jsdoc/jsdocReadonly.js (1 errors) ==== + class LOL { + /** + * @readonly + * @private + * @type {number} + * Order rules do not apply to JSDoc + */ + x = 1 + /** @readonly */ + y = 2 + /** @readonly Definitely not here */ + static z = 3 + /** @readonly This is OK too */ + constructor() { + /** ok */ + this.y = 2 + /** @readonly ok */ + this.ka = 2 + } + } + + var l = new LOL() + l.y = 12 + ~ +!!! error TS2540: Cannot assign to 'y' because it is a read-only property. + \ No newline at end of file diff --git a/tests/baselines/reference/jsdocReadonly.symbols b/tests/baselines/reference/jsdocReadonly.symbols new file mode 100644 index 0000000000000..05fc062422263 --- /dev/null +++ b/tests/baselines/reference/jsdocReadonly.symbols @@ -0,0 +1,46 @@ +=== tests/cases/conformance/jsdoc/jsdocReadonly.js === +class LOL { +>LOL : Symbol(LOL, Decl(jsdocReadonly.js, 0, 0)) + + /** + * @readonly + * @private + * @type {number} + * Order rules do not apply to JSDoc + */ + x = 1 +>x : Symbol(LOL.x, Decl(jsdocReadonly.js, 0, 11)) + + /** @readonly */ + y = 2 +>y : Symbol(LOL.y, Decl(jsdocReadonly.js, 7, 9)) + + /** @readonly Definitely not here */ + static z = 3 +>z : Symbol(LOL.z, Decl(jsdocReadonly.js, 9, 9)) + + /** @readonly This is OK too */ + constructor() { + /** ok */ + this.y = 2 +>this.y : Symbol(LOL.y, Decl(jsdocReadonly.js, 7, 9)) +>this : Symbol(LOL, Decl(jsdocReadonly.js, 0, 0)) +>y : Symbol(LOL.y, Decl(jsdocReadonly.js, 7, 9)) + + /** @readonly ok */ + this.ka = 2 +>this.ka : Symbol(LOL.ka, Decl(jsdocReadonly.js, 15, 18)) +>this : Symbol(LOL, Decl(jsdocReadonly.js, 0, 0)) +>ka : Symbol(LOL.ka, Decl(jsdocReadonly.js, 15, 18)) + } +} + +var l = new LOL() +>l : Symbol(l, Decl(jsdocReadonly.js, 21, 3), Decl(jsdocReadonly.js, 21, 17)) +>LOL : Symbol(LOL, Decl(jsdocReadonly.js, 0, 0)) + +l.y = 12 +>l.y : Symbol(LOL.y, Decl(jsdocReadonly.js, 7, 9)) +>l : Symbol(l, Decl(jsdocReadonly.js, 21, 3), Decl(jsdocReadonly.js, 21, 17)) +>y : Symbol(LOL.y, Decl(jsdocReadonly.js, 7, 9)) + diff --git a/tests/baselines/reference/jsdocReadonly.types b/tests/baselines/reference/jsdocReadonly.types new file mode 100644 index 0000000000000..8a0f0806a2497 --- /dev/null +++ b/tests/baselines/reference/jsdocReadonly.types @@ -0,0 +1,56 @@ +=== tests/cases/conformance/jsdoc/jsdocReadonly.js === +class LOL { +>LOL : LOL + + /** + * @readonly + * @private + * @type {number} + * Order rules do not apply to JSDoc + */ + x = 1 +>x : number +>1 : 1 + + /** @readonly */ + y = 2 +>y : 2 +>2 : 2 + + /** @readonly Definitely not here */ + static z = 3 +>z : 3 +>3 : 3 + + /** @readonly This is OK too */ + constructor() { + /** ok */ + this.y = 2 +>this.y = 2 : 2 +>this.y : 2 +>this : this +>y : 2 +>2 : 2 + + /** @readonly ok */ + this.ka = 2 +>this.ka = 2 : 2 +>this.ka : number +>this : this +>ka : number +>2 : 2 + } +} + +var l = new LOL() +>l : LOL +>new LOL() : LOL +>LOL : typeof LOL + +l.y = 12 +>l.y = 12 : 12 +>l.y : any +>l : LOL +>y : any +>12 : 12 + diff --git a/tests/baselines/reference/jsdocReadonlyDeclarations.js b/tests/baselines/reference/jsdocReadonlyDeclarations.js new file mode 100644 index 0000000000000..09568ef6b196f --- /dev/null +++ b/tests/baselines/reference/jsdocReadonlyDeclarations.js @@ -0,0 +1,60 @@ +//// [jsdocReadonlyDeclarations.js] +class C { + /** @readonly */ + x = 6 + /** @readonly */ + constructor(n) { + this.x = n + /** + * @readonly + * @type {number} + */ + this.y = n + } +} +new C().x + +function F() { + /** @readonly */ + this.z = 1 +} + + +//// [foo.js] +class C { + /** @readonly */ + constructor(n) { + /** @readonly */ + this.x = 6; + this.x = n; + /** + * @readonly + * @type {number} + */ + this.y = n; + } +} +new C().x; +function F() { + /** @readonly */ + this.z = 1; +} + + +//// [foo.d.ts] +declare function F(): void; +declare class F { + /** @readonly */ + readonly z: number; +} +declare class C { + /** @readonly */ + constructor(n: any); + /** @readonly */ + readonly x: 6; + /** + * @readonly + * @type {number} + */ + readonly y: number; +} diff --git a/tests/baselines/reference/jsdocReadonlyDeclarations.symbols b/tests/baselines/reference/jsdocReadonlyDeclarations.symbols new file mode 100644 index 0000000000000..29ac14063dc86 --- /dev/null +++ b/tests/baselines/reference/jsdocReadonlyDeclarations.symbols @@ -0,0 +1,42 @@ +=== tests/cases/conformance/jsdoc/jsdocReadonlyDeclarations.js === +class C { +>C : Symbol(C, Decl(jsdocReadonlyDeclarations.js, 0, 0)) + + /** @readonly */ + x = 6 +>x : Symbol(C.x, Decl(jsdocReadonlyDeclarations.js, 0, 9)) + + /** @readonly */ + constructor(n) { +>n : Symbol(n, Decl(jsdocReadonlyDeclarations.js, 4, 16)) + + this.x = n +>this.x : Symbol(C.x, Decl(jsdocReadonlyDeclarations.js, 0, 9)) +>this : Symbol(C, Decl(jsdocReadonlyDeclarations.js, 0, 0)) +>x : Symbol(C.x, Decl(jsdocReadonlyDeclarations.js, 0, 9)) +>n : Symbol(n, Decl(jsdocReadonlyDeclarations.js, 4, 16)) + + /** + * @readonly + * @type {number} + */ + this.y = n +>this.y : Symbol(C.y, Decl(jsdocReadonlyDeclarations.js, 5, 18)) +>this : Symbol(C, Decl(jsdocReadonlyDeclarations.js, 0, 0)) +>y : Symbol(C.y, Decl(jsdocReadonlyDeclarations.js, 5, 18)) +>n : Symbol(n, Decl(jsdocReadonlyDeclarations.js, 4, 16)) + } +} +new C().x +>new C().x : Symbol(C.x, Decl(jsdocReadonlyDeclarations.js, 0, 9)) +>C : Symbol(C, Decl(jsdocReadonlyDeclarations.js, 0, 0)) +>x : Symbol(C.x, Decl(jsdocReadonlyDeclarations.js, 0, 9)) + +function F() { +>F : Symbol(F, Decl(jsdocReadonlyDeclarations.js, 13, 9)) + + /** @readonly */ + this.z = 1 +>z : Symbol(F.z, Decl(jsdocReadonlyDeclarations.js, 15, 14)) +} + diff --git a/tests/baselines/reference/jsdocReadonlyDeclarations.types b/tests/baselines/reference/jsdocReadonlyDeclarations.types new file mode 100644 index 0000000000000..ebb5fd00ff14c --- /dev/null +++ b/tests/baselines/reference/jsdocReadonlyDeclarations.types @@ -0,0 +1,50 @@ +=== tests/cases/conformance/jsdoc/jsdocReadonlyDeclarations.js === +class C { +>C : C + + /** @readonly */ + x = 6 +>x : 6 +>6 : 6 + + /** @readonly */ + constructor(n) { +>n : any + + this.x = n +>this.x = n : any +>this.x : 6 +>this : this +>x : 6 +>n : any + + /** + * @readonly + * @type {number} + */ + this.y = n +>this.y = n : any +>this.y : number +>this : this +>y : number +>n : any + } +} +new C().x +>new C().x : 6 +>new C() : C +>C : typeof C +>x : 6 + +function F() { +>F : typeof F + + /** @readonly */ + this.z = 1 +>this.z = 1 : 1 +>this.z : any +>this : any +>z : any +>1 : 1 +} + From a1d3a3e388155122d8f414986a11af83eb5c1a1f Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Thu, 19 Dec 2019 16:11:23 -0800 Subject: [PATCH 4/4] Fix last test and clean up --- src/compiler/checker.ts | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 6151c167db8c3..28a4d61c5464a 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -26345,23 +26345,19 @@ namespace ts { if (!(ctor && ctor.kind === SyntaxKind.Constructor)) { return true; } - // 1. no valueDeclaration -- some kind of synthetic property, don't allow the write - // 2. normal property declaration -- its parent and the ctor's parent should both be a class - // 3. parameter property declaration -- its parent should BE the ctor itself - // 4. this.property assignment in a class -- it should be a binaryexpression, and ctor.parent should be the symbol.parent.valueDeclaration - // 5. this.property assignment in a ctor func -- it should be a binaryexp, and ctor should be the symbol.parent.valueDeclaration - // If func.parent is a class and symbol is a (readonly) property of that class, or - // if func is a constructor and symbol is a (readonly) parameter property declared in it, - // then symbol is writeable here. if (symbol.valueDeclaration) { - const isLocalParameterProperty = ctor === symbol.valueDeclaration.parent; + const isAssignmentDeclaration = isBinaryExpression(symbol.valueDeclaration); const isLocalPropertyDeclaration = ctor.parent === symbol.valueDeclaration.parent; - const isLocalThisPropertyAssignment = symbol.parent && symbol.parent.valueDeclaration === ctor.parent; // TODO: Make sure that symbol.parent is a function and class - const isLocalThisPropertyAssignmentConstructorFunction = symbol.parent && symbol.parent.valueDeclaration === ctor; // TODO: Make sure that symbol.parent is a function and class - const isLocalProperty = isLocalPropertyDeclaration || isLocalParameterProperty || isLocalThisPropertyAssignment || isLocalThisPropertyAssignmentConstructorFunction ; - return !isLocalProperty; + const isLocalParameterProperty = ctor === symbol.valueDeclaration.parent; + const isLocalThisPropertyAssignment = isAssignmentDeclaration && symbol.parent?.valueDeclaration === ctor.parent; + const isLocalThisPropertyAssignmentConstructorFunction = isAssignmentDeclaration && symbol.parent?.valueDeclaration === ctor; + const isWriteableSymbol = + isLocalPropertyDeclaration + || isLocalParameterProperty + || isLocalThisPropertyAssignment + || isLocalThisPropertyAssignmentConstructorFunction; + return !isWriteableSymbol; } - return false; } return true; }