Code review

Author: Andy Hanson

src/compiler/checker.ts

@@ -314,16 +314,16 @@ namespace ts {
             getSuggestionDiagnostics: file => {
                 return (suggestionDiagnostics.get(file.fileName) || emptyArray).concat(getUnusedDiagnostics());
                 function getUnusedDiagnostics(): ReadonlyArray<Diagnostic> {
+                    if (file.isDeclarationFile) return emptyArray;
+
                     checkSourceFile(file);
                     const diagnostics: Diagnostic[] = [];
                     Debug.assert(!!(getNodeLinks(file).flags & NodeCheckFlags.TypeChecked));
-                    if (!file.isDeclarationFile) {
-                        checkUnusedIdentifiers(allPotentiallyUnusedIdentifiers.get(file.fileName)!, (kind, diag) => {
-                            if (!unusedIsError(kind)) {
-                                diagnostics.push({ ...diag, category: DiagnosticCategory.Suggestion });
-                            }
-                        });
-                    }
+                    checkUnusedIdentifiers(allPotentiallyUnusedIdentifiers.get(file.fileName)!, (kind, diag) => {
+                        if (!unusedIsError(kind)) {
+                            diagnostics.push({ ...diag, category: DiagnosticCategory.Suggestion });
+                        }
+                    });
                     return diagnostics;
                 }
             },

src/compiler/core.ts

@@ -1623,7 +1623,7 @@ namespace ts {
             messageText: text,
             category: message.category,
             code: message.code,
-            unused: message.unused,
+            reportsUnused: message.unused,
         };
     }
 
@@ -1654,7 +1654,7 @@ namespace ts {
             messageText: text,
             category: message.category,
             code: message.code,
-            unused: message.unused,
+            reportsUnused: message.unused,
         };
     }
 

src/compiler/types.ts

@@ -4061,7 +4061,7 @@ namespace ts {
         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. */
-        unused?: {};
+        reportsUnused?: {};
         code: number;
         source?: string;
     }

src/harness/unittests/matchFiles.ts

@@ -123,14 +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,
-                unused: undefined,
+                reportsUnused: undefined,
             }));
             assertParsed(actual, expected);
         }

src/server/client.ts

@@ -357,7 +357,7 @@ namespace ts.server {
             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<Diagnostic>(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 {
@@ -367,7 +367,7 @@ namespace ts.server {
                     messageText: entry.message,
                     category: Debug.assertDefined(category, "convertDiagnostic: category should not be undefined"),
                     code: entry.code,
-                    unused: entry.unused,
+                    reportsUnused: entry.reportsUnused,
                 };
             });
         }

src/server/protocol.ts

@@ -456,7 +456,7 @@ namespace ts.server.protocol {
         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. */
-        unused?: {};
+        reportsUnused?: {};
     }
 
     /**

tests/baselines/reference/api/tsserverlibrary.d.ts

@@ -2279,7 +2279,7 @@ declare namespace ts {
         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. */
-        unused?: {};
+        reportsUnused?: {};
         code: number;
         source?: string;
     }

tests/baselines/reference/api/typescript.d.ts

@@ -2279,7 +2279,7 @@ declare namespace ts {
         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. */
-        unused?: {};
+        reportsUnused?: {};
         code: number;
         source?: string;
     }

tests/cases/fourslash/convertFunctionToEs6ClassJsDoc.ts

@@ -17,7 +17,7 @@
 /////**
 ////  * This is a cool function!
 ////*/
-////fn.prototype.bar = function (x, y, z) {
+////fn.prototype.bar = function (y) {
 ////    this.x = y;
 ////};
 
@@ -38,7 +38,7 @@ verify.codeFix({
     /**
       * This is a cool function!
     */
-    bar(x, y, z) {
+    bar(y) {
         this.x = y;
     }
 }

tests/cases/fourslash/refactorConvertToEs6Module_export_referenced.ts

@@ -12,12 +12,13 @@
 ////
 ////exports.z = 2;
 ////exports.f = function(z) {
-////    exports.z; z;
+////    z;
 ////}
 
+// TODO: GH#22492 Should be a able access `exports.z` inside `exports.f`
+
 verify.codeFix({
     description: "Convert to ES6 module",
-    // TODO: GH#22492
     newFileContent:
 `export const x = 0;
 x;
@@ -27,9 +28,8 @@ const _y = y;
 export { _y as y };
 _y;
 
-const _z = 2;
-export { _z as z };
+export const z = 2;
 export function f(z) {
-        _z  z;
+    z;
 }`,
 });