From 79da510066e4806e8519d8428ab2dc582ef66c35 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Wed, 25 Oct 2023 18:24:53 -0400 Subject: [PATCH] Fix incorrect flags in modifierFlagsCache (#56198) --- src/compiler/types.ts | 58 +++++++++++++------ src/compiler/utilities.ts | 34 ++++++++--- src/services/codefixes/fixSpelling.ts | 5 +- src/testRunner/unittests/publicApi.ts | 31 ++++++++++ tests/baselines/reference/api/typescript.d.ts | 50 ++++++++-------- 5 files changed, 122 insertions(+), 56 deletions(-) diff --git a/src/compiler/types.ts b/src/compiler/types.ts index bb1505131fa85..927720e580da8 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -851,25 +851,45 @@ export const enum NodeFlags { // dprint-ignore export const enum ModifierFlags { None = 0, - Export = 1 << 0, // Declarations - Ambient = 1 << 1, // Declarations - Public = 1 << 2, // Property/Method - Private = 1 << 3, // Property/Method - Protected = 1 << 4, // Property/Method - Static = 1 << 5, // Property/Method - Readonly = 1 << 6, // Property/Method - Accessor = 1 << 7, // Property - Abstract = 1 << 8, // Class/Method/ConstructSignature - Async = 1 << 9, // Property/Method/Function - Default = 1 << 10, // Function/Class (export default declaration) - Const = 1 << 11, // Const enum - HasComputedJSDocModifiers = 1 << 12, // Indicates the computed modifier flags include modifiers from JSDoc. - - Deprecated = 1 << 13, // Deprecated tag. - Override = 1 << 14, // Override method. - In = 1 << 15, // Contravariance modifier - Out = 1 << 16, // Covariance modifier - Decorator = 1 << 17, // Contains a decorator. + + // Syntactic/JSDoc modifiers + Public = 1 << 0, // Property/Method + Private = 1 << 1, // Property/Method + Protected = 1 << 2, // Property/Method + Readonly = 1 << 3, // Property/Method + Override = 1 << 4, // Override method. + + // Syntactic-only modifiers + Export = 1 << 5, // Declarations + Abstract = 1 << 6, // Class/Method/ConstructSignature + Ambient = 1 << 7, // Declarations + Static = 1 << 8, // Property/Method + Accessor = 1 << 9, // Property + Async = 1 << 10, // Property/Method/Function + Default = 1 << 11, // Function/Class (export default declaration) + Const = 1 << 12, // Const enum + In = 1 << 13, // Contravariance modifier + Out = 1 << 14, // Covariance modifier + Decorator = 1 << 15, // Contains a decorator. + + // JSDoc-only modifiers + Deprecated = 1 << 16, // Deprecated tag. + + // Cache-only JSDoc-modifiers. Should match order of Syntactic/JSDoc modifiers, above. + /** @internal */ JSDocPublic = 1 << 23, // if this value changes, `selectEffectiveModifierFlags` must change accordingly + /** @internal */ JSDocPrivate = 1 << 24, + /** @internal */ JSDocProtected = 1 << 25, + /** @internal */ JSDocReadonly = 1 << 26, + /** @internal */ JSDocOverride = 1 << 27, + + /** @internal */ SyntacticOrJSDocModifiers = Public | Private | Protected | Readonly | Override, + /** @internal */ SyntacticOnlyModifiers = Export | Ambient | Abstract | Static | Accessor | Async | Default | Const | In | Out | Decorator, + /** @internal */ SyntacticModifiers = SyntacticOrJSDocModifiers | SyntacticOnlyModifiers, + /** @internal */ JSDocCacheOnlyModifiers = JSDocPublic | JSDocPrivate | JSDocProtected | JSDocReadonly | JSDocOverride, + /** @internal */ JSDocOnlyModifiers = Deprecated, + /** @internal */ NonCacheOnlyModifiers = SyntacticOrJSDocModifiers | SyntacticOnlyModifiers | JSDocOnlyModifiers, + + HasComputedJSDocModifiers = 1 << 28, // Indicates the computed modifier flags include modifiers from JSDoc. HasComputedFlags = 1 << 29, // Modifier flags have been computed AccessibilityModifier = Public | Private | Protected, diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 0d1e4a4ac6836..ac4e6cb95d575 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -6928,11 +6928,14 @@ function getModifierFlagsWorker(node: Node, includeJSDoc: boolean, alwaysInclude node.modifierFlagsCache = getSyntacticModifierFlagsNoCache(node) | ModifierFlags.HasComputedFlags; } - if (includeJSDoc && !(node.modifierFlagsCache & ModifierFlags.HasComputedJSDocModifiers) && (alwaysIncludeJSDoc || isInJSFile(node)) && node.parent) { - node.modifierFlagsCache |= getJSDocModifierFlagsNoCache(node) | ModifierFlags.HasComputedJSDocModifiers; + if (alwaysIncludeJSDoc || includeJSDoc && isInJSFile(node)) { + if (!(node.modifierFlagsCache & ModifierFlags.HasComputedJSDocModifiers) && node.parent) { + node.modifierFlagsCache |= getRawJSDocModifierFlagsNoCache(node) | ModifierFlags.HasComputedJSDocModifiers; + } + return selectEffectiveModifierFlags(node.modifierFlagsCache); } - return node.modifierFlagsCache & ~(ModifierFlags.HasComputedFlags | ModifierFlags.HasComputedJSDocModifiers); + return selectSyntacticModifierFlags(node.modifierFlagsCache); } /** @@ -6962,15 +6965,15 @@ export function getSyntacticModifierFlags(node: Node): ModifierFlags { return getModifierFlagsWorker(node, /*includeJSDoc*/ false); } -function getJSDocModifierFlagsNoCache(node: Node): ModifierFlags { +function getRawJSDocModifierFlagsNoCache(node: Node): ModifierFlags { let flags = ModifierFlags.None; if (!!node.parent && !isParameter(node)) { if (isInJSFile(node)) { - if (getJSDocPublicTagNoCache(node)) flags |= ModifierFlags.Public; - if (getJSDocPrivateTagNoCache(node)) flags |= ModifierFlags.Private; - if (getJSDocProtectedTagNoCache(node)) flags |= ModifierFlags.Protected; - if (getJSDocReadonlyTagNoCache(node)) flags |= ModifierFlags.Readonly; - if (getJSDocOverrideTagNoCache(node)) flags |= ModifierFlags.Override; + if (getJSDocPublicTagNoCache(node)) flags |= ModifierFlags.JSDocPublic; + if (getJSDocPrivateTagNoCache(node)) flags |= ModifierFlags.JSDocPrivate; + if (getJSDocProtectedTagNoCache(node)) flags |= ModifierFlags.JSDocProtected; + if (getJSDocReadonlyTagNoCache(node)) flags |= ModifierFlags.JSDocReadonly; + if (getJSDocOverrideTagNoCache(node)) flags |= ModifierFlags.JSDocOverride; } if (getJSDocDeprecatedTagNoCache(node)) flags |= ModifierFlags.Deprecated; } @@ -6978,6 +6981,19 @@ function getJSDocModifierFlagsNoCache(node: Node): ModifierFlags { return flags; } +function selectSyntacticModifierFlags(flags: ModifierFlags) { + return flags & ModifierFlags.SyntacticModifiers; +} + +function selectEffectiveModifierFlags(flags: ModifierFlags) { + return (flags & ModifierFlags.NonCacheOnlyModifiers) | + ((flags & ModifierFlags.JSDocCacheOnlyModifiers) >>> 23); // shift ModifierFlags.JSDoc* to match ModifierFlags.* +} + +function getJSDocModifierFlagsNoCache(node: Node): ModifierFlags { + return selectEffectiveModifierFlags(getRawJSDocModifierFlagsNoCache(node)); +} + /** * Gets the effective ModifierFlags for the provided node, including JSDoc modifiers. The modifier flags cache on the node is ignored. * diff --git a/src/services/codefixes/fixSpelling.ts b/src/services/codefixes/fixSpelling.ts index 687c16cbb9915..ede869cfea944 100644 --- a/src/services/codefixes/fixSpelling.ts +++ b/src/services/codefixes/fixSpelling.ts @@ -10,7 +10,7 @@ import { getModeForUsageLocation, getTextOfNode, getTokenAtPosition, - hasSyntacticModifier, + hasOverrideModifier, ImportDeclaration, isBinaryExpression, isClassElement, @@ -27,7 +27,6 @@ import { isPropertyAccessExpression, isQualifiedName, isStringLiteralLike, - ModifierFlags, Node, NodeFlags, ScriptTarget, @@ -131,7 +130,7 @@ function getInfo(sourceFile: SourceFile, pos: number, context: CodeFixContextBas const props = checker.getContextualTypeForArgumentAtIndex(tag, 0); suggestedSymbol = checker.getSuggestedSymbolForNonexistentJSXAttribute(node, props!); } - else if (hasSyntacticModifier(parent, ModifierFlags.Override) && isClassElement(parent) && parent.name === node) { + else if (hasOverrideModifier(parent) && isClassElement(parent) && parent.name === node) { const baseDeclaration = findAncestor(node, isClassLike); const baseTypeNode = baseDeclaration ? getEffectiveBaseTypeNode(baseDeclaration) : undefined; const baseType = baseTypeNode ? checker.getTypeAtLocation(baseTypeNode) : undefined; diff --git a/src/testRunner/unittests/publicApi.ts b/src/testRunner/unittests/publicApi.ts index 7fb4fb096c122..1b5ab93f0a187 100644 --- a/src/testRunner/unittests/publicApi.ts +++ b/src/testRunner/unittests/publicApi.ts @@ -231,3 +231,34 @@ describe("unittests:: Public APIs:: getChild* methods on EndOfFileToken with JSD assert.notEqual(endOfFileToken.getChildAt(0), /*expected*/ undefined); }); }); + +describe("unittests:: Public APIs:: get syntactic and effective modifiers", () => { + it("caches and reports correct flags in TS file", () => { + // https://github.com/microsoft/TypeScript/issues/42189 + const content = ` +class C { + /** @private */ + prop = 1; +}`; + const sourceFile = ts.createSourceFile("/file.ts", content, ts.ScriptTarget.ESNext, /*setParentNodes*/ true); + const classNode = sourceFile.statements[0] as ts.ClassDeclaration; + const propNode = classNode.members[0] as ts.PropertyDeclaration; + assert.equal(ts.ModifierFlags.None, ts.getSyntacticModifierFlags(propNode)); + assert.equal(ts.ModifierFlags.None, ts.getEffectiveModifierFlags(propNode)); + assert.equal(ts.ModifierFlags.None, ts.getSyntacticModifierFlags(propNode)); + }); + it("caches and reports correct flags in JS file", () => { + // https://github.com/microsoft/TypeScript/issues/42189 + const content = ` +class C { + /** @private */ + prop = 1; +}`; + const sourceFile = ts.createSourceFile("/file.js", content, ts.ScriptTarget.ESNext, /*setParentNodes*/ true); + const classNode = sourceFile.statements[0] as ts.ClassDeclaration; + const propNode = classNode.members[0] as ts.PropertyDeclaration; + assert.equal(ts.ModifierFlags.None, ts.getSyntacticModifierFlags(propNode)); + assert.equal(ts.ModifierFlags.Private, ts.getEffectiveModifierFlags(propNode)); + assert.equal(ts.ModifierFlags.None, ts.getSyntacticModifierFlags(propNode)); + }); +}); diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index d3028ed4d6065..5ee1d5258cb01 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -4752,32 +4752,32 @@ declare namespace ts { } enum ModifierFlags { None = 0, - Export = 1, - Ambient = 2, - Public = 4, - Private = 8, - Protected = 16, - Static = 32, - Readonly = 64, - Accessor = 128, - Abstract = 256, - Async = 512, - Default = 1024, - Const = 2048, - HasComputedJSDocModifiers = 4096, - Deprecated = 8192, - Override = 16384, - In = 32768, - Out = 65536, - Decorator = 131072, + Public = 1, + Private = 2, + Protected = 4, + Readonly = 8, + Override = 16, + Export = 32, + Abstract = 64, + Ambient = 128, + Static = 256, + Accessor = 512, + Async = 1024, + Default = 2048, + Const = 4096, + In = 8192, + Out = 16384, + Decorator = 32768, + Deprecated = 65536, + HasComputedJSDocModifiers = 268435456, HasComputedFlags = 536870912, - AccessibilityModifier = 28, - ParameterPropertyModifier = 16476, - NonPublicAccessibilityModifier = 24, - TypeScriptModifier = 117086, - ExportDefault = 1025, - All = 258047, - Modifier = 126975, + AccessibilityModifier = 7, + ParameterPropertyModifier = 31, + NonPublicAccessibilityModifier = 6, + TypeScriptModifier = 28895, + ExportDefault = 2080, + All = 131071, + Modifier = 98303, } enum JsxFlags { None = 0,