Merge pull request #27988 from KnisterPeter/fix24931

Correct codefix by removing private modifier

Author: sheetalkamat

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);
             }
         });
@@ -200,8 +200,16 @@ namespace ts.codefix {
 
     function tryDeleteParameter(changes: textChanges.ChangeTracker, sourceFile: SourceFile, p: ParameterDeclaration, checker: TypeChecker, sourceFiles: ReadonlyArray<SourceFile>, isFixAll: boolean): void {
         if (mayDeleteParameter(p, checker, isFixAll)) {
-            changes.delete(sourceFile, p);
-            deleteUnusedArguments(changes, sourceFile, p, sourceFiles, checker);
+            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 {
+                changes.delete(sourceFile, p);
+                deleteUnusedArguments(changes, sourceFile, p, sourceFiles, checker);
+            }
         }
     }
 

src/services/findAllReferences.ts

@@ -839,7 +839,9 @@ namespace ts.FindAllReferences.Core {
     }
 
     export function eachSymbolReferenceInFile<T>(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;

tests/cases/fourslash/codeFixUnusedIdentifier_parameter_modifier.ts

@@ -0,0 +1,22 @@
+/// <reference path='fourslash.ts' />
+
+// @noUnusedLocals: true
+// @noUnusedParameters: true
+
+////export class Example {
+////    prop: any;
+////    constructor(private readonly arg: any) {
+////        this.prop = arg;
+////    }
+////}
+
+verify.codeFix({
+    description: "Remove declaration for: 'arg'",
+    newFileContent:
+`export class Example {
+    prop: any;
+    constructor(arg: any) {
+        this.prop = arg;
+    }
+}`,
+});

tests/cases/fourslash/codeFixUnusedIdentifier_parameter_modifier_and_arg.ts

@@ -0,0 +1,18 @@
+/// <reference path='fourslash.ts' />
+
+// @noUnusedLocals: true
+// @noUnusedParameters: true
+
+////export class Example {
+////    constructor(private readonly arg: any) {
+////    }
+////}
+
+verify.codeFix({
+    description: "Remove declaration for: 'arg'",
+    newFileContent:
+`export class Example {
+    constructor() {
+    }
+}`,
+});