Skip to content

improve stripInternal with inline comments #23611

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Mar 12, 2019
27 changes: 23 additions & 4 deletions src/compiler/transformers/declarations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}

Expand Down Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion src/server/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
108 changes: 108 additions & 0 deletions tests/baselines/reference/declarationEmitWorkWithInlineComments.js
Original file line number Diff line number Diff line change
@@ -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);
}
Original file line number Diff line number Diff line change
@@ -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))

) {}
}
Original file line number Diff line number Diff line change
@@ -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

) {}
}
Original file line number Diff line number Diff line change
@@ -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
) {}
}