Skip to content

Fix 10438 - Remove parameters of private methods and constructors in declaration files #11030

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Fix 10438 - Remove parameters of private methods and constructors in declaration files #11030

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b1ace49
Remove emitting parameters of private methods in declaration files.
Igorbek Sep 21, 2016
fb1129d
Accept new baselines.
Igorbek Sep 21, 2016
a871af6
Add a baseline test of private members emitting in declaration files.
Igorbek Sep 21, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 35 additions & 7 deletions src/compiler/declarationEmitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ namespace ts {
const emitJsDocComments = compilerOptions.removeComments ? function (declaration: Node) { } : writeJsDocComments;
const emit = compilerOptions.stripInternal ? stripInternal : emitNode;
let noDeclare: boolean;
let enclosingDeclarationEmittedPrivateInstanceMembers: Map<boolean>;
let enclosingDeclarationEmittedPrivateStaticMembers: Map<boolean>;

let moduleElementDeclarationEmitInfo: ModuleElementDeclarationEmitInfo[] = [];
let asynchronousSubModuleDeclarationEmitInfo: ModuleElementDeclarationEmitInfo[];
Expand Down Expand Up @@ -1073,6 +1075,10 @@ namespace ts {
writeTextOfNode(currentText, node.name);
const prevEnclosingDeclaration = enclosingDeclaration;
enclosingDeclaration = node;
const prevEnclosingDeclarationEmittedPrivateInstanceMembers = enclosingDeclarationEmittedPrivateInstanceMembers;
enclosingDeclarationEmittedPrivateInstanceMembers = ts.createMap<boolean>();
const prevEnclosingDeclarationEmittedPrivateStaticMembers = enclosingDeclarationEmittedPrivateStaticMembers;
enclosingDeclarationEmittedPrivateStaticMembers = ts.createMap<boolean>();
emitTypeParameters(node.typeParameters);
const baseTypeNode = getClassExtendsHeritageClauseElement(node);
if (baseTypeNode) {
Expand All @@ -1088,6 +1094,8 @@ namespace ts {
write("}");
writeLine();
enclosingDeclaration = prevEnclosingDeclaration;
enclosingDeclarationEmittedPrivateInstanceMembers = prevEnclosingDeclarationEmittedPrivateInstanceMembers;
enclosingDeclarationEmittedPrivateStaticMembers = prevEnclosingDeclarationEmittedPrivateStaticMembers;
}

function writeInterfaceDeclaration(node: InterfaceDeclaration) {
Expand Down Expand Up @@ -1352,6 +1360,15 @@ namespace ts {
// If we are emitting Method/Constructor it isn't moduleElement and hence already determined to be emitting
// so no need to verify if the declaration is visible
if (!resolver.isImplementationOfOverload(node)) {
// If we are emitting a private method or constructor it might be already emitted with an overloaded signature
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems incorrect way of handling it.
You could just check in emitSignatureDeclaration if this is first signature and skip rest for private methods.
Map of name is not needed and if needed you can ask resolver for the question instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, I'll change

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, writeFunctionDeclaration first writes declaration flags and function name and then calls emitSignatureDeclaration. So it seems here's the proper place for checking.
Do you suggest to add a new question to resolver? Because I didn't find any that matches that need.

if ((node.kind === SyntaxKind.MethodDeclaration || node.kind === SyntaxKind.Constructor) && hasModifier(node, ModifierFlags.Private)) {
const emitted = hasModifier(node, ModifierFlags.Static) ? enclosingDeclarationEmittedPrivateStaticMembers : enclosingDeclarationEmittedPrivateInstanceMembers;
const name = node.kind === SyntaxKind.Constructor ? "constructor" : getTextOfNode(node.name);
if (emitted[name] === true)
return;
emitted[name] = true;
}

emitJsDocComments(node);
if (node.kind === SyntaxKind.FunctionDeclaration) {
emitModuleElementDeclarationFlags(node);
Expand Down Expand Up @@ -1385,6 +1402,7 @@ namespace ts {
const prevEnclosingDeclaration = enclosingDeclaration;
enclosingDeclaration = node;
let closeParenthesizedFunctionType = false;
let emitParameters = true;

if (node.kind === SyntaxKind.IndexSignature) {
// Index signature can have readonly modifier
Expand All @@ -1406,18 +1424,28 @@ namespace ts {
write("(");
}
}
emitTypeParameters(node.typeParameters);
write("(");
if (hasModifier(node, ModifierFlags.Private)) {
emitParameters = false;
}
else {
emitTypeParameters(node.typeParameters);
write("(");
}
}

// Parameters
emitCommaList(node.parameters, emitParameterDeclaration);
if (emitParameters) {
emitCommaList(node.parameters, emitParameterDeclaration);

if (node.kind === SyntaxKind.IndexSignature) {
write("]");
if (node.kind === SyntaxKind.IndexSignature) {
write("]");
}
else {
write(")");
}
}
else {
write(")");
else if (node.kind == SyntaxKind.Constructor) {
write("()");
}

// If this is not a constructor and is not private, emit the return type
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/classConstructorAccessibility.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ declare class C {
}
declare class D {
x: number;
private constructor(x);
private constructor();
}
declare class E {
x: number;
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/classConstructorAccessibility2.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ declare class BaseB {
}
declare class BaseC {
x: number;
private constructor(x);
private constructor();
createInstance(): void;
static staticInstance(): void;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/classConstructorAccessibility3.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ declare class Baz {
}
declare class Qux {
x: number;
private constructor(x);
private constructor();
}
declare let a: typeof Foo;
declare let b: typeof Baz;
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/classConstructorOverloadsAccessibility.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ var D = (function () {
declare class A {
constructor(a: boolean);
protected constructor(a: number);
private constructor(a);
private constructor();
}
declare class B {
protected constructor(a: number);
Expand Down
13 changes: 5 additions & 8 deletions tests/baselines/reference/classdecl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,11 +215,10 @@ declare class a {
x: number;
y: number;
};
private static d2();
private static d2;
private static readonly p3;
private pv3;
private foo(n);
private foo(s);
private foo;
}
declare class b extends a {
}
Expand Down Expand Up @@ -254,13 +253,11 @@ declare class aAmbient {
static d2(): any;
static p3: any;
private pv3;
private foo(s);
private foo;
}
declare class d {
private foo(n);
private foo(s);
private foo;
}
declare class e {
private foo(s);
private foo(n);
private foo;
}
8 changes: 4 additions & 4 deletions tests/baselines/reference/commentsClassMembers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ declare class c1 {
/** pp1 is property of c1*/
private pp1;
/** sum with property*/
private pp2(/** number to add*/ b);
private pp2;
/** getter property*/
/** setter property*/
private pp3;
Expand All @@ -509,7 +509,7 @@ declare class c1 {
nc_p2(b: number): number;
nc_p3: number;
private nc_pp1;
private nc_pp2(b);
private nc_pp2;
private nc_pp3;
static nc_s1: number;
static nc_s2(b: number): number;
Expand All @@ -518,7 +518,7 @@ declare class c1 {
a_p2(b: number): number;
a_p3: number;
private a_pp1;
private a_pp2(b);
private a_pp2;
private a_pp3;
static a_s1: number;
static a_s2(b: number): number;
Expand All @@ -533,7 +533,7 @@ declare class c1 {
/** pp1 is property of c1 */
private b_pp1;
/** sum with property */
private b_pp2(b);
private b_pp2;
/** getter property */
/** setter property */
private b_pp3;
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/commentsTypeParameters.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function compare(a, b) {
declare class C</**docComment for type parameter*/ T> {
method</**docComment of method type parameter */ U extends T>(a: U): void;
static staticmethod</**docComment of method type parameter */ U>(a: U): void;
private privatemethod</**docComment of method type parameter */ U>(a);
private static privatestaticmethod</**docComment of method type parameter */ U>(a);
private privatemethod;
private static privatestaticmethod;
}
declare function compare</**type*/ T>(a: T, b: T): boolean;
3 changes: 1 addition & 2 deletions tests/baselines/reference/declFileForClassWithPrivateOverloadedFunction.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,5 @@ var C = (function () {

//// [declFileForClassWithPrivateOverloadedFunction.d.ts]
declare class C {
private foo(x);
private foo(x);
private foo;
}
44 changes: 16 additions & 28 deletions tests/baselines/reference/declFileMethods.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,14 +370,11 @@ export declare class c1 {
fooWithOverloads(a: string): string;
fooWithOverloads(a: number): number;
/** This comment should appear for privateFoo*/
private privateFoo();
private privateFoo;
/** This is comment for function signature*/
private privateFooWithParameters(/** this is comment about a*/ a,
/** this is comment for b*/
b);
private privateFooWithRestParameters(a, ...rests);
private privateFooWithOverloads(a);
private privateFooWithOverloads(a);
private privateFooWithParameters;
private privateFooWithRestParameters;
private privateFooWithOverloads;
/** This comment should appear for static foo*/
static staticFoo(): void;
/** This is comment for function signature*/
Expand All @@ -388,14 +385,11 @@ export declare class c1 {
static staticFooWithOverloads(a: string): string;
static staticFooWithOverloads(a: number): number;
/** This comment should appear for privateStaticFoo*/
private static privateStaticFoo();
private static privateStaticFoo;
/** This is comment for function signature*/
private static privateStaticFooWithParameters(/** this is comment about a*/ a,
/** this is comment for b*/
b);
private static privateStaticFooWithRestParameters(a, ...rests);
private static privateStaticFooWithOverloads(a);
private static privateStaticFooWithOverloads(a);
private static privateStaticFooWithParameters;
private static privateStaticFooWithRestParameters;
private static privateStaticFooWithOverloads;
}
export interface I1 {
/** This comment should appear for foo*/
Expand All @@ -420,14 +414,11 @@ declare class c2 {
fooWithOverloads(a: string): string;
fooWithOverloads(a: number): number;
/** This comment should appear for privateFoo*/
private privateFoo();
private privateFoo;
/** This is comment for function signature*/
private privateFooWithParameters(/** this is comment about a*/ a,
/** this is comment for b*/
b);
private privateFooWithRestParameters(a, ...rests);
private privateFooWithOverloads(a);
private privateFooWithOverloads(a);
private privateFooWithParameters;
private privateFooWithRestParameters;
private privateFooWithOverloads;
/** This comment should appear for static foo*/
static staticFoo(): void;
/** This is comment for function signature*/
Expand All @@ -438,14 +429,11 @@ declare class c2 {
static staticFooWithOverloads(a: string): string;
static staticFooWithOverloads(a: number): number;
/** This comment should appear for privateStaticFoo*/
private static privateStaticFoo();
private static privateStaticFoo;
/** This is comment for function signature*/
private static privateStaticFooWithParameters(/** this is comment about a*/ a,
/** this is comment for b*/
b);
private static privateStaticFooWithRestParameters(a, ...rests);
private static privateStaticFooWithOverloads(a);
private static privateStaticFooWithOverloads(a);
private static privateStaticFooWithParameters;
private static privateStaticFooWithRestParameters;
private static privateStaticFooWithOverloads;
}
interface I2 {
/** This comment should appear for foo*/
Expand Down
11 changes: 4 additions & 7 deletions tests/baselines/reference/declFilePrivateMethodOverloads.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,10 @@ interface IContext {
someMethod(): any;
}
declare class c1 {
private _forEachBindingContext(bindingContext, fn);
private _forEachBindingContext(bindingContextArray, fn);
private overloadWithArityDifference(bindingContext);
private overloadWithArityDifference(bindingContextArray, fn);
private _forEachBindingContext;
private overloadWithArityDifference;
}
declare class c2 {
private overload1(context, fn);
private overload2(context);
private overload2(context, fn);
private overload1;
private overload2;
}
2 changes: 1 addition & 1 deletion tests/baselines/reference/declFilePrivateStatic.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ C.y = 1;
declare class C {
private static x;
static y: number;
private static a();
private static a;
static b(): void;
private static readonly c;
static readonly d: number;
Expand Down
Loading