improve stripInternal with inline comments (#23611)

* improve stripInternal with inline comments

* fix lint

* stash

* simptify StripInternal

* fix internal type declaration

* fix internal type declaration again

* accept baseline

* refactor inline

* simply prev check

* remove getTrailingCommentRangesOfNode

* Merge implementation with new isInternalDeclaration method, accept lkg-based baseline

src/compiler/transformers/declarations.ts

@@ -9,12 +9,31 @@ namespace ts {
         return result.diagnostics;
     }
 
+    function hasInternalAnnotation(range: CommentRange, currentSourceFile: SourceFile) {
+        const comment = currentSourceFile.text.substring(range.pos, range.end);
+        return stringContains(comment, "@internal");
+    }
+
     export function isInternalDeclaration(node: Node, currentSourceFile: SourceFile) {
         const parseTreeNode = getParseTreeNode(node);
+        if (parseTreeNode && parseTreeNode.kind === SyntaxKind.Parameter) {
+            const paramIdx = (parseTreeNode.parent as FunctionLike).parameters.indexOf(parseTreeNode as ParameterDeclaration);
+            const previousSibling = paramIdx > 0 ? (parseTreeNode.parent as FunctionLike).parameters[paramIdx - 1] : undefined;
+            const text = currentSourceFile.text;
+            const commentRanges = previousSibling
+                ? concatenate(
+                    // to handle
+                    // ... parameters, /* @internal */
+                    // public param: string
+                    getTrailingCommentRanges(text, skipTrivia(text, previousSibling.end + 1, /* stopAfterLineBreak */ false, /* stopAtComments */ true)),
+                    getLeadingCommentRanges(text, node.pos)
+                )
+                : getTrailingCommentRanges(text, skipTrivia(text, node.pos, /* stopAfterLineBreak */ false, /* stopAtComments */ true));
+            return commentRanges && commentRanges.length && hasInternalAnnotation(last(commentRanges), currentSourceFile);
+        }
         const leadingCommentRanges = parseTreeNode && getLeadingCommentRangesOfNode(parseTreeNode, currentSourceFile);
         return !!forEach(leadingCommentRanges, range => {
-            const comment = currentSourceFile.text.substring(range.pos, range.end);
-            return stringContains(comment, "@internal");
+            return hasInternalAnnotation(range, currentSourceFile);
         });
     }
 
@@ -1075,8 +1094,8 @@ namespace ts {
                     let parameterProperties: ReadonlyArray<PropertyDeclaration> | undefined;
                     if (ctor) {
                         const oldDiag = getSymbolAccessibilityDiagnostic;
-                        parameterProperties = compact(flatMap(ctor.parameters, param => {
-                            if (!hasModifier(param, ModifierFlags.ParameterPropertyModifier)) return;
+                        parameterProperties = compact(flatMap(ctor.parameters, (param) => {
+                            if (!hasModifier(param, ModifierFlags.ParameterPropertyModifier) || shouldStripInternal(param)) return;
                             getSymbolAccessibilityDiagnostic = createGetSymbolAccessibilityDiagnosticForNode(param);
                             if (param.name.kind === SyntaxKind.Identifier) {
                                 return preserveJsDoc(createProperty(

src/server/project.ts

@@ -221,7 +221,7 @@ namespace ts.server {
 
         /*@internal*/
         constructor(
-            readonly projectName: string,
+            /*@internal*/ readonly projectName: string,
             readonly projectKind: ProjectKind,
             readonly projectService: ProjectService,
             private documentRegistry: DocumentRegistry,

tests/baselines/reference/declarationEmitWorkWithInlineComments.js

@@ -0,0 +1,108 @@
+//// [declarationEmitWorkWithInlineComments.ts]
+export class Foo {
+  constructor(
+    /** @internal */
+    public isInternal1: string,
+    /** @internal */ public isInternal2: string, /** @internal */
+    public isInternal3: string,
+    // @internal
+    public isInternal4: string,
+    // nothing
+    /** @internal */
+    public isInternal5: string,
+    /* @internal */ public isInternal6: string /* trailing */,
+    /* @internal */ public isInternal7: string, /** @internal */
+    // not work
+    public notInternal1: string,
+    // @internal
+    /* not work */
+    public notInternal2: string,
+    /* not work */
+    // @internal
+    /* not work */
+    public notInternal3: string,
+  ) { }
+}
+
+export class Bar {
+  constructor(/* @internal */ public isInternal1: string) {}
+}
+
+export class Baz {
+  constructor(/* @internal */
+    public isInternal: string
+  ) {}
+}
+
+//// [declarationEmitWorkWithInlineComments.js]
+"use strict";
+exports.__esModule = true;
+var Foo = /** @class */ (function () {
+    function Foo(
+    /** @internal */
+    isInternal1, 
+    /** @internal */ isInternal2, /** @internal */ isInternal3, 
+    // @internal
+    isInternal4, 
+    // nothing
+    /** @internal */
+    isInternal5, 
+    /* @internal */ isInternal6 /* trailing */, 
+    /* @internal */ isInternal7, /** @internal */ 
+    // not work
+    notInternal1, 
+    // @internal
+    /* not work */
+    notInternal2, 
+    /* not work */
+    // @internal
+    /* not work */
+    notInternal3) {
+        this.isInternal1 = isInternal1;
+        this.isInternal2 = isInternal2;
+        this.isInternal3 = isInternal3;
+        this.isInternal4 = isInternal4;
+        this.isInternal5 = isInternal5;
+        this.isInternal6 = isInternal6;
+        this.isInternal7 = isInternal7;
+        this.notInternal1 = notInternal1;
+        this.notInternal2 = notInternal2;
+        this.notInternal3 = notInternal3;
+    }
+    return Foo;
+}());
+exports.Foo = Foo;
+var Bar = /** @class */ (function () {
+    function Bar(/* @internal */ isInternal1) {
+        this.isInternal1 = isInternal1;
+    }
+    return Bar;
+}());
+exports.Bar = Bar;
+var Baz = /** @class */ (function () {
+    function Baz(/* @internal */ isInternal) {
+        this.isInternal = isInternal;
+    }
+    return Baz;
+}());
+exports.Baz = Baz;
+
+
+//// [declarationEmitWorkWithInlineComments.d.ts]
+export declare class Foo {
+    notInternal1: string;
+    notInternal2: string;
+    notInternal3: string;
+    constructor(
+    /** @internal */
+    isInternal1: string, 
+    /** @internal */ isInternal2: string, /** @internal */ isInternal3: string, isInternal4: string, 
+    /** @internal */
+    isInternal5: string, isInternal6: string, isInternal7: string, /** @internal */ notInternal1: string, notInternal2: string, notInternal3: string);
+}
+export declare class Bar {
+    constructor(/* @internal */ isInternal1: string);
+}
+export declare class Baz {
+    constructor(/* @internal */ isInternal: string);
+}

tests/baselines/reference/declarationEmitWorkWithInlineComments.symbols

@@ -0,0 +1,64 @@
+=== tests/cases/conformance/declarationEmit/declarationEmitWorkWithInlineComments.ts ===
+export class Foo {
+>Foo : Symbol(Foo, Decl(declarationEmitWorkWithInlineComments.ts, 0, 0))
+
+  constructor(
+    /** @internal */
+    public isInternal1: string,
+>isInternal1 : Symbol(Foo.isInternal1, Decl(declarationEmitWorkWithInlineComments.ts, 1, 14))
+
+    /** @internal */ public isInternal2: string, /** @internal */
+>isInternal2 : Symbol(Foo.isInternal2, Decl(declarationEmitWorkWithInlineComments.ts, 3, 31))
+
+    public isInternal3: string,
+>isInternal3 : Symbol(Foo.isInternal3, Decl(declarationEmitWorkWithInlineComments.ts, 4, 48))
+
+    // @internal
+    public isInternal4: string,
+>isInternal4 : Symbol(Foo.isInternal4, Decl(declarationEmitWorkWithInlineComments.ts, 5, 31))
+
+    // nothing
+    /** @internal */
+    public isInternal5: string,
+>isInternal5 : Symbol(Foo.isInternal5, Decl(declarationEmitWorkWithInlineComments.ts, 7, 31))
+
+    /* @internal */ public isInternal6: string /* trailing */,
+>isInternal6 : Symbol(Foo.isInternal6, Decl(declarationEmitWorkWithInlineComments.ts, 10, 31))
+
+    /* @internal */ public isInternal7: string, /** @internal */
+>isInternal7 : Symbol(Foo.isInternal7, Decl(declarationEmitWorkWithInlineComments.ts, 11, 62))
+
+    // not work
+    public notInternal1: string,
+>notInternal1 : Symbol(Foo.notInternal1, Decl(declarationEmitWorkWithInlineComments.ts, 12, 47))
+
+    // @internal
+    /* not work */
+    public notInternal2: string,
+>notInternal2 : Symbol(Foo.notInternal2, Decl(declarationEmitWorkWithInlineComments.ts, 14, 32))
+
+    /* not work */
+    // @internal
+    /* not work */
+    public notInternal3: string,
+>notInternal3 : Symbol(Foo.notInternal3, Decl(declarationEmitWorkWithInlineComments.ts, 17, 32))
+
+  ) { }
+}
+
+export class Bar {
+>Bar : Symbol(Bar, Decl(declarationEmitWorkWithInlineComments.ts, 23, 1))
+
+  constructor(/* @internal */ public isInternal1: string) {}
+>isInternal1 : Symbol(Bar.isInternal1, Decl(declarationEmitWorkWithInlineComments.ts, 26, 14))
+}
+
+export class Baz {
+>Baz : Symbol(Baz, Decl(declarationEmitWorkWithInlineComments.ts, 27, 1))
+
+  constructor(/* @internal */
+    public isInternal: string
+>isInternal : Symbol(Baz.isInternal, Decl(declarationEmitWorkWithInlineComments.ts, 30, 14))
+
+  ) {}
+}

tests/baselines/reference/declarationEmitWorkWithInlineComments.types

@@ -0,0 +1,64 @@
+=== tests/cases/conformance/declarationEmit/declarationEmitWorkWithInlineComments.ts ===
+export class Foo {
+>Foo : Foo
+
+  constructor(
+    /** @internal */
+    public isInternal1: string,
+>isInternal1 : string
+
+    /** @internal */ public isInternal2: string, /** @internal */
+>isInternal2 : string
+
+    public isInternal3: string,
+>isInternal3 : string
+
+    // @internal
+    public isInternal4: string,
+>isInternal4 : string
+
+    // nothing
+    /** @internal */
+    public isInternal5: string,
+>isInternal5 : string
+
+    /* @internal */ public isInternal6: string /* trailing */,
+>isInternal6 : string
+
+    /* @internal */ public isInternal7: string, /** @internal */
+>isInternal7 : string
+
+    // not work
+    public notInternal1: string,
+>notInternal1 : string
+
+    // @internal
+    /* not work */
+    public notInternal2: string,
+>notInternal2 : string
+
+    /* not work */
+    // @internal
+    /* not work */
+    public notInternal3: string,
+>notInternal3 : string
+
+  ) { }
+}
+
+export class Bar {
+>Bar : Bar
+
+  constructor(/* @internal */ public isInternal1: string) {}
+>isInternal1 : string
+}
+
+export class Baz {
+>Baz : Baz
+
+  constructor(/* @internal */
+    public isInternal: string
+>isInternal : string
+
+  ) {}
+}

tests/cases/conformance/declarationEmit/declarationEmitWorkWithInlineComments.ts

@@ -0,0 +1,37 @@
+// @declaration: true
+// @stripInternal:true
+
+export class Foo {
+  constructor(
+    /** @internal */
+    public isInternal1: string,
+    /** @internal */ public isInternal2: string, /** @internal */
+    public isInternal3: string,
+    // @internal
+    public isInternal4: string,
+    // nothing
+    /** @internal */
+    public isInternal5: string,
+    /* @internal */ public isInternal6: string /* trailing */,
+    /* @internal */ public isInternal7: string, /** @internal */
+    // not work
+    public notInternal1: string,
+    // @internal
+    /* not work */
+    public notInternal2: string,
+    /* not work */
+    // @internal
+    /* not work */
+    public notInternal3: string,
+  ) { }
+}
+
+export class Bar {
+  constructor(/* @internal */ public isInternal1: string) {}
+}
+
+export class Baz {
+  constructor(/* @internal */
+    public isInternal: string
+  ) {}
+}