When --noUnusedLocals/--noUnusedParameters is disabled, add suggestio…

…ns instead of errors (#22361) * When --noUnusedLocals/--noUnusedParameters is disabled, add suggestions instead of errors * Improve performance: do not add unused suggestion diagnostics unless asking for a suggestion * Add "unused" flag to diagnostics * Code review * reportsUnused -> reportsUnnecessary * Fix test
microsoft · Apr 5, 2018 · 24842b4 · omril1 · Apr 18, 2018 · 24842b4
1 parent f61f126
commit 24842b4
Show file tree

Hide file tree

Showing 67 changed files with 335 additions and 200 deletions.
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
diff --git a/src/compiler/core.ts b/src/compiler/core.ts
@@ -1606,6 +1606,7 @@ namespace ts {
             messageText: text,
             category: message.category,
             code: message.code,
+            reportsUnnecessary: message.unused,
         };
     }
 
@@ -1635,7 +1636,8 @@ namespace ts {
 
             messageText: text,
             category: message.category,
-            code: message.code
+            code: message.code,
+            reportsUnnecessary: message.unused,
         };
     }
 
@@ -1647,7 +1649,7 @@ namespace ts {
 
             code: chain.code,
             category: chain.category,
-            messageText: chain.next ? chain : chain.messageText
+            messageText: chain.next ? chain : chain.messageText,
         };
     }
 

diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json
@@ -3282,7 +3282,8 @@
     },
     "'{0}' is declared but its value is never read.": {
         "category": "Error",
-        "code": 6133
+        "code": 6133,
+        "unused": true
     },
     "Report errors on unused locals.": {
         "category": "Message",
@@ -3302,7 +3303,8 @@
     },
     "Property '{0}' is declared but its value is never read.": {
         "category": "Error",
-        "code": 6138
+        "code": 6138,
+        "unused": true
     },
     "Import emit helpers from 'tslib'.": {
         "category": "Message",
@@ -3514,7 +3516,8 @@
     },
     "All imports in import declaration are unused.": {
         "category": "Error",
-        "code": 6192
+        "code": 6192,
+        "unused": true
     },
     "Found 1 error.": {
         "category": "Message",
@@ -3602,7 +3605,8 @@
     },
     "Unused label.": {
         "category": "Error",
-        "code": 7028
+        "code": 7028,
+        "unused": true
     },
     "Fallthrough case in switch.": {
         "category": "Error",

diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -4069,6 +4069,7 @@ namespace ts {
         category: DiagnosticCategory;
         code: number;
         message: string;
+        unused?: {};
     }
 
     /**
@@ -4090,6 +4091,8 @@ namespace ts {
         length: number | undefined;
         messageText: string | DiagnosticMessageChain;
         category: DiagnosticCategory;
+        /** May store more in future. For now, this will simply be `true` to indicate when a diagnostic is an unused-identifier diagnostic. */
+        reportsUnnecessary?: {};
         code: number;
         source?: string;
     }

diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts
@@ -2577,7 +2577,7 @@ Actual: ${stringify(fullActual)}`);
             }
             const range = ts.first(ranges);
 
-            const codeFixes = this.getCodeFixes(fileName, errorCode, preferences);
+            const codeFixes = this.getCodeFixes(fileName, errorCode, preferences).filter(f => f.fixId === undefined); // TODO: GH#20315 filter out those that use the import fix ID;
 
             if (codeFixes.length === 0) {
                 if (expectedTextArray.length !== 0) {

diff --git a/src/harness/unittests/matchFiles.ts b/src/harness/unittests/matchFiles.ts
@@ -123,13 +123,14 @@ namespace ts {
         }
         {
             const actual = parseJsonConfigFileContent(json, host, basePath, existingOptions, configFileName, resolutionStack);
-            expected.errors = expected.errors.map<Diagnostic>(error => ({
+            expected.errors = expected.errors.map((error): Diagnostic => ({
                 category: error.category,
                 code: error.code,
                 file: undefined,
                 length: undefined,
                 messageText: error.messageText,
                 start: undefined,
+                reportsUnnecessary: undefined,
             }));
             assertParsed(actual, expected);
         }

diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts
@@ -4068,7 +4068,7 @@ namespace ts.projectSystem {
             const folderPath = "/a/b/projects/temp";
             const file1: FileOrFolder = {
                 path: `${folderPath}/a.ts`,
-                content: 'import f = require("pad")'
+                content: 'import f = require("pad"); f;'
             };
             const files = [file1, libFile];
             const host = createServerHost(files);

diff --git a/src/server/client.ts b/src/server/client.ts
@@ -353,11 +353,11 @@ namespace ts.server {
             return this.getDiagnostics(file, CommandNames.SuggestionDiagnosticsSync);
         }
 
-        private getDiagnostics(file: string, command: CommandNames) {
+        private getDiagnostics(file: string, command: CommandNames): Diagnostic[] {
             const request = this.processRequest<protocol.SyntacticDiagnosticsSyncRequest | protocol.SemanticDiagnosticsSyncRequest | protocol.SuggestionDiagnosticsSyncRequest>(command, { file, includeLinePosition: true });
             const response = this.processResponse<protocol.SyntacticDiagnosticsSyncResponse | protocol.SemanticDiagnosticsSyncResponse | protocol.SuggestionDiagnosticsSyncResponse>(request);
 
-            return (<protocol.DiagnosticWithLinePosition[]>response.body).map(entry => {
+            return (<protocol.DiagnosticWithLinePosition[]>response.body).map((entry): Diagnostic => {
                 const category = firstDefined(Object.keys(DiagnosticCategory), id =>
                     isString(id) && entry.category === id.toLowerCase() ? (<any>DiagnosticCategory)[id] : undefined);
                 return {
@@ -366,7 +366,8 @@ namespace ts.server {
                     length: entry.length,
                     messageText: entry.message,
                     category: Debug.assertDefined(category, "convertDiagnostic: category should not be undefined"),
-                    code: entry.code
+                    code: entry.code,
+                    reportsUnnecessary: entry.reportsUnnecessary,
                 };
             });
         }

diff --git a/src/server/protocol.ts b/src/server/protocol.ts
@@ -455,6 +455,8 @@ namespace ts.server.protocol {
         endLocation: Location;
         category: string;
         code: number;
+        /** May store more in future. For now, this will simply be `true` to indicate when a diagnostic is an unused-identifier diagnostic. */
+        reportsUnnecessary?: {};
     }
 
     /**

diff --git a/src/services/shims.ts b/src/services/shims.ts
@@ -588,6 +588,7 @@ namespace ts {
         length: number;
         category: string;
         code: number;
+        unused?: {};
     }
     export function realizeDiagnostics(diagnostics: ReadonlyArray<Diagnostic>, newLine: string): RealizedDiagnostic[] {
         return diagnostics.map(d => realizeDiagnostic(d, newLine));

diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts
@@ -2271,6 +2271,7 @@ declare namespace ts {
         category: DiagnosticCategory;
         code: number;
         message: string;
+        unused?: {};
     }
     /**
      * A linked list of formatted diagnostic messages to be used as part of a multiline message.
@@ -2290,6 +2291,8 @@ declare namespace ts {
         length: number | undefined;
         messageText: string | DiagnosticMessageChain;
         category: DiagnosticCategory;
+        /** May store more in future. For now, this will simply be `true` to indicate when a diagnostic is an unused-identifier diagnostic. */
+        reportsUnnecessary?: {};
         code: number;
         source?: string;
     }
@@ -5398,6 +5401,8 @@ declare namespace ts.server.protocol {
         endLocation: Location;
         category: string;
         code: number;
+        /** May store more in future. For now, this will simply be `true` to indicate when a diagnostic is an unused-identifier diagnostic. */
+        reportsUnnecessary?: {};
     }
     /**
      * Response message for "projectInfo" request

diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts
@@ -2271,6 +2271,7 @@ declare namespace ts {
         category: DiagnosticCategory;
         code: number;
         message: string;
+        unused?: {};
     }
     /**
      * A linked list of formatted diagnostic messages to be used as part of a multiline message.
@@ -2290,6 +2291,8 @@ declare namespace ts {
         length: number | undefined;
         messageText: string | DiagnosticMessageChain;
         category: DiagnosticCategory;
+        /** May store more in future. For now, this will simply be `true` to indicate when a diagnostic is an unused-identifier diagnostic. */
+        reportsUnnecessary?: {};
         code: number;
         source?: string;
     }

diff --git a/tests/cases/fourslash/annotateWithTypeFromJSDoc12.ts b/tests/cases/fourslash/annotateWithTypeFromJSDoc12.ts
@@ -5,6 +5,7 @@
 ////     * @return {...*}
 ////     */
 ////    m(x) {
+////        return [x];
 ////    }
 ////}
 
@@ -16,6 +17,7 @@ verify.codeFix({
      * @return {...*}
      */
     m(x): any[] {
+        return [x];
     }
 }`,
 });
diff --git a/tests/cases/fourslash/annotateWithTypeFromJSDoc15.ts b/tests/cases/fourslash/annotateWithTypeFromJSDoc15.ts
@@ -12,6 +12,7 @@
 //// * @param {promise<String>} zeta
 //// */
 ////function f(x, y, z, alpha, beta, gamma, delta, epsilon, zeta) {
+////    x; y; z; alpha; beta; gamma; delta; epsilon; zeta;
 ////}
 
 verify.codeFix({
@@ -29,5 +30,6 @@ verify.codeFix({
  * @param {promise<String>} zeta
  */
 function f(x: boolean, y: string, z: number, alpha: object, beta: Date, gamma: Promise<any>, delta: Array<any>, epsilon: Array<number>, zeta: Promise<string>) {
+    x; y; z; alpha; beta; gamma; delta; epsilon; zeta;
 }`,
 });
diff --git a/tests/cases/fourslash/annotateWithTypeFromJSDoc16.ts b/tests/cases/fourslash/annotateWithTypeFromJSDoc16.ts
@@ -1,12 +1,12 @@
 /// <reference path='fourslash.ts' />
 // @strict: true
 /////** @type {function(*, ...number, ...boolean): void} */
-////var x = (x, ys, ...zs) => { };
+////var x = (x, ys, ...zs) => { x; ys; zs; };
 
 verify.codeFix({
     description: "Annotate with type from JSDoc",
     index: 0,
     newFileContent:
 `/** @type {function(*, ...number, ...boolean): void} */
-var x: (arg0: any, arg1: number[], ...rest: boolean[]) => void = (x, ys, ...zs) => { };`,
+var x: (arg0: any, arg1: number[], ...rest: boolean[]) => void = (x, ys, ...zs) => { x; ys; zs; };`,
 });
diff --git a/tests/cases/fourslash/annotateWithTypeFromJSDoc17.ts b/tests/cases/fourslash/annotateWithTypeFromJSDoc17.ts
@@ -3,7 +3,7 @@
 ////    /**
 ////     * @param {number} x - the first parameter
 ////     */
-////    constructor(x) {
+////    constructor(readonly x) {
 ////    }
 ////}
 
@@ -14,7 +14,7 @@ verify.codeFix({
     /**
      * @param {number} x - the first parameter
      */
-    constructor(x: number) {
+    constructor(readonly x: number) {
     }
 }`,
 });
diff --git a/tests/cases/fourslash/annotateWithTypeFromJSDoc18.ts b/tests/cases/fourslash/annotateWithTypeFromJSDoc18.ts
@@ -1,14 +1,14 @@
 /// <reference path='fourslash.ts' />
 ////class C {
 ////    /** @param {number} value */
-////    set c(value) { return 12 }
+////    set c(value) { return value }
 ////}
 
 verify.codeFix({
     description: "Annotate with type from JSDoc",
     newFileContent:
 `class C {
     /** @param {number} value */
-    set c(value: number) { return 12 }
+    set c(value: number) { return value }
 }`,
 });
diff --git a/tests/cases/fourslash/annotateWithTypeFromJSDoc19.ts b/tests/cases/fourslash/annotateWithTypeFromJSDoc19.ts
@@ -6,6 +6,7 @@
 //// * @param {T} b
 //// */
 ////function f(a, b) {
+////    return a || b;
 ////}
 
 verify.codeFix({
@@ -17,5 +18,6 @@ verify.codeFix({
  * @param {T} b
  */
 function f<T>(a: number, b: T) {
+    return a || b;
 }`,
 });
diff --git a/tests/cases/fourslash/annotateWithTypeFromJSDoc20.ts b/tests/cases/fourslash/annotateWithTypeFromJSDoc20.ts
@@ -4,11 +4,12 @@
 //// * @param {number} a
 //// * @param {T} b
 //// */
-////function /*1*/f<T>(a, b) {
+////function f<T>(a, b) {
 ////}
 
 verify.codeFix({
     description: "Annotate with type from JSDoc",
+    errorCode: 80004, // ignore 'unused T'
     newFileContent:
 `/**
  * @param {number} a

diff --git a/tests/cases/fourslash/annotateWithTypeFromJSDoc21.ts b/tests/cases/fourslash/annotateWithTypeFromJSDoc21.ts
@@ -4,33 +4,35 @@
 //// * @return {number}
 //// */
 ////function [|f|](x, y) {
+////    return x + y;
 ////}
 ////
 /////**
 //// * @return {number}
 //// */
 ////function g(x, y): number {
-////    return 0;
+////    return x + y;
 ////}
 /////**
 //// * @param {number} x
 //// */
 ////function h(x: number, y): number {
-////    return 0;
+////    return x + y;
 ////}
 ////
 /////**
 //// * @param {number} x
 //// * @param {string} y
 //// */
 ////function i(x: number, y: string) {
+////    return x + y;
 ////}
 /////**
 //// * @param {number} x
 //// * @return {boolean}
 //// */
 ////function j(x: number, y): boolean {
-////    return true;
+////    return x < y;
 ////}
 
 // Only first location triggers a suggestion
@@ -41,37 +43,40 @@ verify.getSuggestionDiagnostics([{
 
 verify.codeFix({
     description: "Annotate with type from JSDoc",
-   newFileContent:
+    errorCode: 80004,
+    newFileContent:
 `/**
  * @return {number}
  */
 function f(x, y): number {
+    return x + y;
 }
 
 /**
  * @return {number}
  */
 function g(x, y): number {
-    return 0;
+    return x + y;
 }
 /**
  * @param {number} x
  */
 function h(x: number, y): number {
-    return 0;
+    return x + y;
 }
 
 /**
  * @param {number} x
  * @param {string} y
  */
 function i(x: number, y: string) {
+    return x + y;
 }
 /**
  * @param {number} x
  * @return {boolean}
  */
 function j(x: number, y): boolean {
-    return true;
+    return x < y;
 }`,
 });