From e6e71978df30e407d51c6c6412c0841fe2e3a6d4 Mon Sep 17 00:00:00 2001 From: Markus Wolf Date: Fri, 19 Oct 2018 15:55:34 +0200 Subject: [PATCH 1/6] Correct codefix by removing private modifier In case of private attribute and private constructor parameter with assignment in the constructor body, the parameter is flagged as unused. This is caused by the private modifier which is shadowed by the explicity assignment in the body. This commit updates the codefix to just remove the private modifier in this cases. Closes #24931 --- src/services/codefixes/fixUnusedIdentifier.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/services/codefixes/fixUnusedIdentifier.ts b/src/services/codefixes/fixUnusedIdentifier.ts index 2905f0b131b..d31fe739d2b 100644 --- a/src/services/codefixes/fixUnusedIdentifier.ts +++ b/src/services/codefixes/fixUnusedIdentifier.ts @@ -200,8 +200,14 @@ namespace ts.codefix { function tryDeleteParameter(changes: textChanges.ChangeTracker, sourceFile: SourceFile, p: ParameterDeclaration, checker: TypeChecker, sourceFiles: ReadonlyArray, isFixAll: boolean): void { if (mayDeleteParameter(p, checker, isFixAll)) { - changes.delete(sourceFile, p); - deleteUnusedArguments(changes, sourceFile, p, sourceFiles, checker); + const privateModifier = ts.findModifier(p, ts.SyntaxKind.PrivateKeyword); + if (privateModifier) { + changes.deleteModifier(sourceFile, p.modifiers![0]); + } + else { + changes.delete(sourceFile, p); + deleteUnusedArguments(changes, sourceFile, p, sourceFiles, checker); + } } } From de7faa1b7e1f0d48a7f7ec79cf2ec747e91c6097 Mon Sep 17 00:00:00 2001 From: Markus Wolf Date: Fri, 19 Oct 2018 16:32:19 +0200 Subject: [PATCH 2/6] Remove obsolte ts namespace --- src/services/codefixes/fixUnusedIdentifier.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/codefixes/fixUnusedIdentifier.ts b/src/services/codefixes/fixUnusedIdentifier.ts index d31fe739d2b..5b59f4baedd 100644 --- a/src/services/codefixes/fixUnusedIdentifier.ts +++ b/src/services/codefixes/fixUnusedIdentifier.ts @@ -200,7 +200,7 @@ namespace ts.codefix { function tryDeleteParameter(changes: textChanges.ChangeTracker, sourceFile: SourceFile, p: ParameterDeclaration, checker: TypeChecker, sourceFiles: ReadonlyArray, isFixAll: boolean): void { if (mayDeleteParameter(p, checker, isFixAll)) { - const privateModifier = ts.findModifier(p, ts.SyntaxKind.PrivateKeyword); + const privateModifier = findModifier(p, SyntaxKind.PrivateKeyword); if (privateModifier) { changes.deleteModifier(sourceFile, p.modifiers![0]); } From d411fa34a7b771b9682ff6b39bec96f61ff16588 Mon Sep 17 00:00:00 2001 From: Markus Wolf Date: Fri, 19 Oct 2018 17:17:45 +0200 Subject: [PATCH 3/6] Add test case for codeFix --- ...eFixUnusedIdentifier_parameter_modifier.ts | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 tests/cases/fourslash/codeFixUnusedIdentifier_parameter_modifier.ts diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_parameter_modifier.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_parameter_modifier.ts new file mode 100644 index 00000000000..cc9f37e61b0 --- /dev/null +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_parameter_modifier.ts @@ -0,0 +1,22 @@ +/// + +// @noUnusedLocals: true +// @noUnusedParameters: true + +////export class Example { +//// prop: any; +//// constructor(private arg: any) { +//// this.prop = arg; +//// } +////} + +verify.codeFix({ + description: "Remove declaration for: 'arg'", + newFileContent: +`export class Example { + prop: any; + constructor(arg: any) { + this.prop = arg; + } +}`, +}); From 13e85ac3a949183b0ffff54b0e9250887c91828b Mon Sep 17 00:00:00 2001 From: Markus Wolf Date: Fri, 2 Nov 2018 15:05:04 +0100 Subject: [PATCH 4/6] add more modifiers --- src/services/codefixes/fixUnusedIdentifier.ts | 9 ++++++--- .../codeFixUnusedIdentifier_parameter_modifier.ts | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/services/codefixes/fixUnusedIdentifier.ts b/src/services/codefixes/fixUnusedIdentifier.ts index 5b59f4baedd..b41c5f52ee0 100644 --- a/src/services/codefixes/fixUnusedIdentifier.ts +++ b/src/services/codefixes/fixUnusedIdentifier.ts @@ -200,9 +200,12 @@ namespace ts.codefix { function tryDeleteParameter(changes: textChanges.ChangeTracker, sourceFile: SourceFile, p: ParameterDeclaration, checker: TypeChecker, sourceFiles: ReadonlyArray, isFixAll: boolean): void { if (mayDeleteParameter(p, checker, isFixAll)) { - const privateModifier = findModifier(p, SyntaxKind.PrivateKeyword); - if (privateModifier) { - changes.deleteModifier(sourceFile, p.modifiers![0]); + const modifiers: Modifier["kind"][] = [SyntaxKind.PrivateKeyword, SyntaxKind.ProtectedKeyword, SyntaxKind.PublicKeyword, SyntaxKind.ReadonlyKeyword]; + const foundModifiers = modifiers.map(modifier => findModifier(p, modifier)).filter(modifier => modifier !== undefined); + if (foundModifiers.length > 0) { + foundModifiers.forEach(modifier => { + changes.deleteModifier(sourceFile, modifier!); + }); } else { changes.delete(sourceFile, p); diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_parameter_modifier.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_parameter_modifier.ts index cc9f37e61b0..5f544df5c2e 100644 --- a/tests/cases/fourslash/codeFixUnusedIdentifier_parameter_modifier.ts +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_parameter_modifier.ts @@ -5,7 +5,7 @@ ////export class Example { //// prop: any; -//// constructor(private arg: any) { +//// constructor(private readonly arg: any) { //// this.prop = arg; //// } ////} From 6bd298b8841a67a297e083be3e73e99443f22460 Mon Sep 17 00:00:00 2001 From: Markus Wolf Date: Mon, 5 Nov 2018 18:07:29 +0100 Subject: [PATCH 5/6] add test for remove modifier and parameter --- src/services/codefixes/fixUnusedIdentifier.ts | 9 ++++----- ...sedIdentifier_parameter_modifier_and_arg.ts | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 5 deletions(-) create mode 100644 tests/cases/fourslash/codeFixUnusedIdentifier_parameter_modifier_and_arg.ts diff --git a/src/services/codefixes/fixUnusedIdentifier.ts b/src/services/codefixes/fixUnusedIdentifier.ts index b41c5f52ee0..d30e8509b07 100644 --- a/src/services/codefixes/fixUnusedIdentifier.ts +++ b/src/services/codefixes/fixUnusedIdentifier.ts @@ -200,11 +200,10 @@ namespace ts.codefix { function tryDeleteParameter(changes: textChanges.ChangeTracker, sourceFile: SourceFile, p: ParameterDeclaration, checker: TypeChecker, sourceFiles: ReadonlyArray, isFixAll: boolean): void { if (mayDeleteParameter(p, checker, isFixAll)) { - const modifiers: Modifier["kind"][] = [SyntaxKind.PrivateKeyword, SyntaxKind.ProtectedKeyword, SyntaxKind.PublicKeyword, SyntaxKind.ReadonlyKeyword]; - const foundModifiers = modifiers.map(modifier => findModifier(p, modifier)).filter(modifier => modifier !== undefined); - if (foundModifiers.length > 0) { - foundModifiers.forEach(modifier => { - changes.deleteModifier(sourceFile, modifier!); + if (p.modifiers && p.modifiers.length > 0 + && (!isIdentifier(p.name) || FindAllReferences.Core.isSymbolReferencedInFile(p.name, checker, sourceFile))) { + p.modifiers.forEach(modifier => { + changes.deleteModifier(sourceFile, modifier); }); } else { diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_parameter_modifier_and_arg.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_parameter_modifier_and_arg.ts new file mode 100644 index 00000000000..0f33db06dfa --- /dev/null +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_parameter_modifier_and_arg.ts @@ -0,0 +1,18 @@ +/// + +// @noUnusedLocals: true +// @noUnusedParameters: true + +////export class Example { +//// constructor(private readonly arg: any) { +//// } +////} + +verify.codeFix({ + description: "Remove declaration for: 'arg'", + newFileContent: +`export class Example { + constructor() { + } +}`, +}); From 499bed540bafd675472aebad63ce1ae016680100 Mon Sep 17 00:00:00 2001 From: Markus Wolf Date: Fri, 9 Nov 2018 09:45:20 +0100 Subject: [PATCH 6/6] Better reference usage detection --- src/services/codefixes/fixUnusedIdentifier.ts | 6 +++--- src/services/findAllReferences.ts | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/services/codefixes/fixUnusedIdentifier.ts b/src/services/codefixes/fixUnusedIdentifier.ts index d30e8509b07..de071ded24c 100644 --- a/src/services/codefixes/fixUnusedIdentifier.ts +++ b/src/services/codefixes/fixUnusedIdentifier.ts @@ -181,8 +181,8 @@ namespace ts.codefix { function deleteAssignments(changes: textChanges.ChangeTracker, sourceFile: SourceFile, token: Identifier, checker: TypeChecker) { FindAllReferences.Core.eachSymbolReferenceInFile(token, checker, sourceFile, (ref: Node) => { - if (ref.parent.kind === SyntaxKind.PropertyAccessExpression) ref = ref.parent; - if (ref.parent.kind === SyntaxKind.BinaryExpression && ref.parent.parent.kind === SyntaxKind.ExpressionStatement) { + if (isPropertyAccessExpression(ref.parent) && ref.parent.name === ref) ref = ref.parent; + if (isBinaryExpression(ref.parent) && isExpressionStatement(ref.parent.parent) && ref.parent.left === ref) { changes.delete(sourceFile, ref.parent.parent); } }); @@ -201,7 +201,7 @@ namespace ts.codefix { function tryDeleteParameter(changes: textChanges.ChangeTracker, sourceFile: SourceFile, p: ParameterDeclaration, checker: TypeChecker, sourceFiles: ReadonlyArray, isFixAll: boolean): void { if (mayDeleteParameter(p, checker, isFixAll)) { if (p.modifiers && p.modifiers.length > 0 - && (!isIdentifier(p.name) || FindAllReferences.Core.isSymbolReferencedInFile(p.name, checker, sourceFile))) { + && (!isIdentifier(p.name) || FindAllReferences.Core.isSymbolReferencedInFile(p.name, checker, sourceFile))) { p.modifiers.forEach(modifier => { changes.deleteModifier(sourceFile, modifier); }); diff --git a/src/services/findAllReferences.ts b/src/services/findAllReferences.ts index b7ded40c28f..30b69c7ce19 100644 --- a/src/services/findAllReferences.ts +++ b/src/services/findAllReferences.ts @@ -839,7 +839,9 @@ namespace ts.FindAllReferences.Core { } export function eachSymbolReferenceInFile(definition: Identifier, checker: TypeChecker, sourceFile: SourceFile, cb: (token: Identifier) => T): T | undefined { - const symbol = checker.getSymbolAtLocation(definition); + const symbol = isParameterPropertyDeclaration(definition.parent) + ? first(checker.getSymbolsOfParameterPropertyDeclaration(definition.parent, definition.text)) + : checker.getSymbolAtLocation(definition); if (!symbol) return undefined; for (const token of getPossibleSymbolReferenceNodes(sourceFile, symbol.name)) { if (!isIdentifier(token) || token === definition || token.escapedText !== definition.escapedText) continue;