Skip to content

Commit bd27296

Browse files
Kingwlweswigham
Kingwl
authored and
weswigham
committed
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
1 parent b15e64f commit bd27296

File tree

6 files changed

+297
-5
lines changed

6 files changed

+297
-5
lines changed

‎src/compiler/transformers/declarations.ts

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,31 @@ namespace ts {
99
return result.diagnostics;
1010
}
1111

12+
function hasInternalAnnotation(range: CommentRange, currentSourceFile: SourceFile) {
13+
const comment = currentSourceFile.text.substring(range.pos, range.end);
14+
return stringContains(comment, "@internal");
15+
}
16+
1217
export function isInternalDeclaration(node: Node, currentSourceFile: SourceFile) {
1318
const parseTreeNode = getParseTreeNode(node);
19+
if (parseTreeNode && parseTreeNode.kind === SyntaxKind.Parameter) {
20+
const paramIdx = (parseTreeNode.parent as FunctionLike).parameters.indexOf(parseTreeNode as ParameterDeclaration);
21+
const previousSibling = paramIdx > 0 ? (parseTreeNode.parent as FunctionLike).parameters[paramIdx - 1] : undefined;
22+
const text = currentSourceFile.text;
23+
const commentRanges = previousSibling
24+
? concatenate(
25+
// to handle
26+
// ... parameters, /* @internal */
27+
// public param: string
28+
getTrailingCommentRanges(text, skipTrivia(text, previousSibling.end + 1, /* stopAfterLineBreak */ false, /* stopAtComments */ true)),
29+
getLeadingCommentRanges(text, node.pos)
30+
)
31+
: getTrailingCommentRanges(text, skipTrivia(text, node.pos, /* stopAfterLineBreak */ false, /* stopAtComments */ true));
32+
return commentRanges && commentRanges.length && hasInternalAnnotation(last(commentRanges), currentSourceFile);
33+
}
1434
const leadingCommentRanges = parseTreeNode && getLeadingCommentRangesOfNode(parseTreeNode, currentSourceFile);
1535
return !!forEach(leadingCommentRanges, range => {
16-
const comment = currentSourceFile.text.substring(range.pos, range.end);
17-
return stringContains(comment, "@internal");
36+
return hasInternalAnnotation(range, currentSourceFile);
1837
});
1938
}
2039

@@ -1075,8 +1094,8 @@ namespace ts {
10751094
let parameterProperties: ReadonlyArray<PropertyDeclaration> | undefined;
10761095
if (ctor) {
10771096
const oldDiag = getSymbolAccessibilityDiagnostic;
1078-
parameterProperties = compact(flatMap(ctor.parameters, param => {
1079-
if (!hasModifier(param, ModifierFlags.ParameterPropertyModifier)) return;
1097+
parameterProperties = compact(flatMap(ctor.parameters, (param) => {
1098+
if (!hasModifier(param, ModifierFlags.ParameterPropertyModifier) || shouldStripInternal(param)) return;
10801099
getSymbolAccessibilityDiagnostic = createGetSymbolAccessibilityDiagnosticForNode(param);
10811100
if (param.name.kind === SyntaxKind.Identifier) {
10821101
return preserveJsDoc(createProperty(

‎src/server/project.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ namespace ts.server {
221221

222222
/*@internal*/
223223
constructor(
224-
readonly projectName: string,
224+
/*@internal*/ readonly projectName: string,
225225
readonly projectKind: ProjectKind,
226226
readonly projectService: ProjectService,
227227
private documentRegistry: DocumentRegistry,

‎tests/baselines/reference/declarationEmitWorkWithInlineComments.js

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
//// [declarationEmitWorkWithInlineComments.ts]
2+
export class Foo {
3+
constructor(
4+
/** @internal */
5+
public isInternal1: string,
6+
/** @internal */ public isInternal2: string, /** @internal */
7+
public isInternal3: string,
8+
// @internal
9+
public isInternal4: string,
10+
// nothing
11+
/** @internal */
12+
public isInternal5: string,
13+
/* @internal */ public isInternal6: string /* trailing */,
14+
/* @internal */ public isInternal7: string, /** @internal */
15+
// not work
16+
public notInternal1: string,
17+
// @internal
18+
/* not work */
19+
public notInternal2: string,
20+
/* not work */
21+
// @internal
22+
/* not work */
23+
public notInternal3: string,
24+
) { }
25+
}
26+
27+
export class Bar {
28+
constructor(/* @internal */ public isInternal1: string) {}
29+
}
30+
31+
export class Baz {
32+
constructor(/* @internal */
33+
public isInternal: string
34+
) {}
35+
}
36+
37+
//// [declarationEmitWorkWithInlineComments.js]
38+
"use strict";
39+
exports.__esModule = true;
40+
var Foo = /** @class */ (function () {
41+
function Foo(
42+
/** @internal */
43+
isInternal1,
44+
/** @internal */ isInternal2, /** @internal */ isInternal3,
45+
// @internal
46+
isInternal4,
47+
// nothing
48+
/** @internal */
49+
isInternal5,
50+
/* @internal */ isInternal6 /* trailing */,
51+
/* @internal */ isInternal7, /** @internal */
52+
// not work
53+
notInternal1,
54+
// @internal
55+
/* not work */
56+
notInternal2,
57+
/* not work */
58+
// @internal
59+
/* not work */
60+
notInternal3) {
61+
this.isInternal1 = isInternal1;
62+
this.isInternal2 = isInternal2;
63+
this.isInternal3 = isInternal3;
64+
this.isInternal4 = isInternal4;
65+
this.isInternal5 = isInternal5;
66+
this.isInternal6 = isInternal6;
67+
this.isInternal7 = isInternal7;
68+
this.notInternal1 = notInternal1;
69+
this.notInternal2 = notInternal2;
70+
this.notInternal3 = notInternal3;
71+
}
72+
return Foo;
73+
}());
74+
exports.Foo = Foo;
75+
var Bar = /** @class */ (function () {
76+
function Bar(/* @internal */ isInternal1) {
77+
this.isInternal1 = isInternal1;
78+
}
79+
return Bar;
80+
}());
81+
exports.Bar = Bar;
82+
var Baz = /** @class */ (function () {
83+
function Baz(/* @internal */ isInternal) {
84+
this.isInternal = isInternal;
85+
}
86+
return Baz;
87+
}());
88+
exports.Baz = Baz;
89+
90+
91+
//// [declarationEmitWorkWithInlineComments.d.ts]
92+
export declare class Foo {
93+
notInternal1: string;
94+
notInternal2: string;
95+
notInternal3: string;
96+
constructor(
97+
/** @internal */
98+
isInternal1: string,
99+
/** @internal */ isInternal2: string, /** @internal */ isInternal3: string, isInternal4: string,
100+
/** @internal */
101+
isInternal5: string, isInternal6: string, isInternal7: string, /** @internal */ notInternal1: string, notInternal2: string, notInternal3: string);
102+
}
103+
export declare class Bar {
104+
constructor(/* @internal */ isInternal1: string);
105+
}
106+
export declare class Baz {
107+
constructor(/* @internal */ isInternal: string);
108+
}

‎tests/baselines/reference/declarationEmitWorkWithInlineComments.symbols

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
=== tests/cases/conformance/declarationEmit/declarationEmitWorkWithInlineComments.ts ===
2+
export class Foo {
3+
>Foo : Symbol(Foo, Decl(declarationEmitWorkWithInlineComments.ts, 0, 0))
4+
5+
constructor(
6+
/** @internal */
7+
public isInternal1: string,
8+
>isInternal1 : Symbol(Foo.isInternal1, Decl(declarationEmitWorkWithInlineComments.ts, 1, 14))
9+
10+
/** @internal */ public isInternal2: string, /** @internal */
11+
>isInternal2 : Symbol(Foo.isInternal2, Decl(declarationEmitWorkWithInlineComments.ts, 3, 31))
12+
13+
public isInternal3: string,
14+
>isInternal3 : Symbol(Foo.isInternal3, Decl(declarationEmitWorkWithInlineComments.ts, 4, 48))
15+
16+
// @internal
17+
public isInternal4: string,
18+
>isInternal4 : Symbol(Foo.isInternal4, Decl(declarationEmitWorkWithInlineComments.ts, 5, 31))
19+
20+
// nothing
21+
/** @internal */
22+
public isInternal5: string,
23+
>isInternal5 : Symbol(Foo.isInternal5, Decl(declarationEmitWorkWithInlineComments.ts, 7, 31))
24+
25+
/* @internal */ public isInternal6: string /* trailing */,
26+
>isInternal6 : Symbol(Foo.isInternal6, Decl(declarationEmitWorkWithInlineComments.ts, 10, 31))
27+
28+
/* @internal */ public isInternal7: string, /** @internal */
29+
>isInternal7 : Symbol(Foo.isInternal7, Decl(declarationEmitWorkWithInlineComments.ts, 11, 62))
30+
31+
// not work
32+
public notInternal1: string,
33+
>notInternal1 : Symbol(Foo.notInternal1, Decl(declarationEmitWorkWithInlineComments.ts, 12, 47))
34+
35+
// @internal
36+
/* not work */
37+
public notInternal2: string,
38+
>notInternal2 : Symbol(Foo.notInternal2, Decl(declarationEmitWorkWithInlineComments.ts, 14, 32))
39+
40+
/* not work */
41+
// @internal
42+
/* not work */
43+
public notInternal3: string,
44+
>notInternal3 : Symbol(Foo.notInternal3, Decl(declarationEmitWorkWithInlineComments.ts, 17, 32))
45+
46+
) { }
47+
}
48+
49+
export class Bar {
50+
>Bar : Symbol(Bar, Decl(declarationEmitWorkWithInlineComments.ts, 23, 1))
51+
52+
constructor(/* @internal */ public isInternal1: string) {}
53+
>isInternal1 : Symbol(Bar.isInternal1, Decl(declarationEmitWorkWithInlineComments.ts, 26, 14))
54+
}
55+
56+
export class Baz {
57+
>Baz : Symbol(Baz, Decl(declarationEmitWorkWithInlineComments.ts, 27, 1))
58+
59+
constructor(/* @internal */
60+
public isInternal: string
61+
>isInternal : Symbol(Baz.isInternal, Decl(declarationEmitWorkWithInlineComments.ts, 30, 14))
62+
63+
) {}
64+
}

‎tests/baselines/reference/declarationEmitWorkWithInlineComments.types

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
=== tests/cases/conformance/declarationEmit/declarationEmitWorkWithInlineComments.ts ===
2+
export class Foo {
3+
>Foo : Foo
4+
5+
constructor(
6+
/** @internal */
7+
public isInternal1: string,
8+
>isInternal1 : string
9+
10+
/** @internal */ public isInternal2: string, /** @internal */
11+
>isInternal2 : string
12+
13+
public isInternal3: string,
14+
>isInternal3 : string
15+
16+
// @internal
17+
public isInternal4: string,
18+
>isInternal4 : string
19+
20+
// nothing
21+
/** @internal */
22+
public isInternal5: string,
23+
>isInternal5 : string
24+
25+
/* @internal */ public isInternal6: string /* trailing */,
26+
>isInternal6 : string
27+
28+
/* @internal */ public isInternal7: string, /** @internal */
29+
>isInternal7 : string
30+
31+
// not work
32+
public notInternal1: string,
33+
>notInternal1 : string
34+
35+
// @internal
36+
/* not work */
37+
public notInternal2: string,
38+
>notInternal2 : string
39+
40+
/* not work */
41+
// @internal
42+
/* not work */
43+
public notInternal3: string,
44+
>notInternal3 : string
45+
46+
) { }
47+
}
48+
49+
export class Bar {
50+
>Bar : Bar
51+
52+
constructor(/* @internal */ public isInternal1: string) {}
53+
>isInternal1 : string
54+
}
55+
56+
export class Baz {
57+
>Baz : Baz
58+
59+
constructor(/* @internal */
60+
public isInternal: string
61+
>isInternal : string
62+
63+
) {}
64+
}

‎tests/cases/conformance/declarationEmit/declarationEmitWorkWithInlineComments.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// @declaration: true
2+
// @stripInternal:true
3+
4+
export class Foo {
5+
constructor(
6+
/** @internal */
7+
public isInternal1: string,
8+
/** @internal */ public isInternal2: string, /** @internal */
9+
public isInternal3: string,
10+
// @internal
11+
public isInternal4: string,
12+
// nothing
13+
/** @internal */
14+
public isInternal5: string,
15+
/* @internal */ public isInternal6: string /* trailing */,
16+
/* @internal */ public isInternal7: string, /** @internal */
17+
// not work
18+
public notInternal1: string,
19+
// @internal
20+
/* not work */
21+
public notInternal2: string,
22+
/* not work */
23+
// @internal
24+
/* not work */
25+
public notInternal3: string,
26+
) { }
27+
}
28+
29+
export class Bar {
30+
constructor(/* @internal */ public isInternal1: string) {}
31+
}
32+
33+
export class Baz {
34+
constructor(/* @internal */
35+
public isInternal: string
36+
) {}
37+
}

0 commit comments

Comments
 (0)