Give JSDoc-provided modifiers their own entries in ModifierFlags

Author: rbuckton

src/compiler/types.ts

@@ -851,26 +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
-    Deprecated =         1 << 12, // Deprecated tag.
-    Override =           1 << 13, // Override method.
-    In =                 1 << 14, // Contravariance modifier
-    Out =                1 << 15, // Covariance modifier
-    Decorator =          1 << 16, // Contains a decorator.
-
-    HasComputedJSDocModifiers = 1 << 27, // Indicates the computed modifier flags include modifiers from JSDoc.
-    HasExcessJDocModifiers = 1 << 28, // Indicates that there are JSDoc modifiers that are not in the cache and must be recalculated. For use with nodes in TS files only.
+
+    // 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, `adjustJSDocModifierFlags` 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,
@@ -879,7 +898,6 @@ export const enum ModifierFlags {
     NonPublicAccessibilityModifier = Private | Protected,
 
     TypeScriptModifier = Ambient | Public | Private | Protected | Readonly | Abstract | Const | Override | In | Out,
-    JSDocOnlyModifier = Deprecated,
     ExportDefault = Export | Default,
     All = Export | Ambient | Public | Private | Protected | Static | Readonly | Abstract | Accessor | Async | Default | Const | Deprecated | Override | In | Out | Decorator,
     Modifier = All & ~Decorator,

src/compiler/utilities.ts

@@ -6924,45 +6924,18 @@ function getModifierFlagsWorker(node: Node, includeJSDoc: boolean, alwaysInclude
         return ModifierFlags.None;
     }
 
-    // Calculate the modifier flags for a given node, and cache them based on the file containing the node.
-    // For a TypeScript file, the cache will only contain syntactic modifiers and potentially the `Deprecated` JSDoc modifier.
-    // For a JavaScript file, the cache will contain both syntactic and JSDoc modifiers.
-
     if (!(node.modifierFlagsCache & ModifierFlags.HasComputedFlags)) {
         node.modifierFlagsCache = getSyntacticModifierFlagsNoCache(node) | ModifierFlags.HasComputedFlags;
     }
 
-    let jsdocModifierFlags: ModifierFlags | undefined;
-    if (includeJSDoc && !(node.modifierFlagsCache & ModifierFlags.HasComputedJSDocModifiers) && node.parent) {
-        jsdocModifierFlags = getJSDocModifierFlagsNoCache(node);
-        if (isInJSFile(node)) {
-            node.modifierFlagsCache |= jsdocModifierFlags | ModifierFlags.HasComputedJSDocModifiers;
-        }
-        else {
-            node.modifierFlagsCache |= (jsdocModifierFlags & ModifierFlags.Deprecated) | ModifierFlags.HasComputedJSDocModifiers;
-            if (jsdocModifierFlags & ~node.modifierFlagsCache & ~ModifierFlags.Deprecated) {
-                // If there are other JSDoc modifiers aside from `@deprecated` that aren't also in the set of syntactic modifiers for the node,
-                // we can add a flag indicating we need to recompute JSDoc modifiers when `alwaysIncludeJSDoc` is true.
-                node.modifierFlagsCache |= ModifierFlags.HasExcessJDocModifiers;
-            }
+    if (alwaysIncludeJSDoc || includeJSDoc && isInJSFile(node)) {
+        if (!(node.modifierFlagsCache & ModifierFlags.HasComputedJSDocModifiers) && node.parent) {
+            node.modifierFlagsCache |= getRawJSDocModifierFlagsNoCache(node) | ModifierFlags.HasComputedJSDocModifiers;
         }
+        return selectEffectiveModifierFlags(node.modifierFlagsCache);
     }
 
-    const modifiers = node.modifierFlagsCache & ~(ModifierFlags.HasComputedFlags | ModifierFlags.HasComputedJSDocModifiers | ModifierFlags.HasExcessJDocModifiers);
-    if (includeJSDoc || alwaysIncludeJSDoc) {
-        if (!isInJSFile(node) && alwaysIncludeJSDoc && node.modifierFlagsCache & ModifierFlags.HasExcessJDocModifiers) {
-            // If we are requesting JSDoc modifiers in a TS file, we only need to recalculate additional JSDoc modifiers if the cache indicates there are any to find.
-            // We recalculate the JSDoc modifiers since they may overlap with the syntactic modifiers in the cache. While this isn't as efficient as caching the modifiers,
-            // it is rare to use JSDoc for modifiers like `@private` in a TS file.
-            jsdocModifierFlags ??= getJSDocModifierFlagsNoCache(node);
-            return modifiers | jsdocModifierFlags;
-        }
-        return modifiers;
-    }
-    else {
-        // We are requesting only syntactic modifiers. filter out any JSDoc modifiers in the cache.
-        return (isInJSFile(node) ? modifiers & ~ModifierFlags.TypeScriptModifier : modifiers) & ~ModifierFlags.JSDocOnlyModifier;
-    }
+    return selectSyntacticModifierFlags(node.modifierFlagsCache);
 }
 
 /**
@@ -6992,22 +6965,35 @@ 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;
     }
 
     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.
  *

tests/baselines/reference/api/typescript.d.ts

@@ -4752,34 +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,
-        Deprecated = 4096,
-        Override = 8192,
-        In = 16384,
-        Out = 32768,
-        Decorator = 65536,
-        HasComputedJSDocModifiers = 134217728,
-        HasExcessJDocModifiers = 268435456,
+        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 = 8284,
-        NonPublicAccessibilityModifier = 24,
-        TypeScriptModifier = 59742,
-        JSDocOnlyModifier = 4096,
-        ExportDefault = 1025,
+        AccessibilityModifier = 7,
+        ParameterPropertyModifier = 31,
+        NonPublicAccessibilityModifier = 6,
+        TypeScriptModifier = 28895,
+        ExportDefault = 2080,
         All = 131071,
-        Modifier = 65535,
+        Modifier = 98303,
     }
     enum JsxFlags {
         None = 0,

tests/baselines/reference/jsFileCompilationPublicParameterModifier.symbols

@@ -3,5 +3,5 @@
 === a.js ===
 class C { constructor(public x) { }}
 >C : Symbol(C, Decl(a.js, 0, 0))
->x : Symbol(x, Decl(a.js, 0, 22))
+>x : Symbol(C.x, Decl(a.js, 0, 22))