Deprecate no-unused-variable rule for typescript@next; also fix runti…

…me error (#3919) * fix no-unused-variable-rule for typescript@next * fix ignore pattern test file as well * fix tests * deprecate noUnusedVariableRule for TS 2.9+ * use semver for cleaner comparison check
palantir · Jun 11, 2018 · fa86eaa · fa86eaa
1 parent 175a586
commit fa86eaa
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 7 deletions.
diff --git a/src/rules/noUnusedVariableRule.ts b/src/rules/noUnusedVariableRule.ts
@@ -15,6 +15,7 @@
  * limitations under the License.
  */
 
+import * as semver from "semver";
 import * as utils from "tsutils";
 import * as ts from "typescript";
 
@@ -72,6 +73,9 @@ export class Rule extends Lint.Rules.TypedRule {
         type: "functionality",
         typescriptOnly: true,
         requiresTypeInfo: true,
+        deprecationMessage: semver.gte(ts.version, "2.9.0-dev.0")
+            ? "Since TypeScript 2.9. Please use the built-in compiler checks instead."
+            : undefined,
     };
     /* tslint:enable:object-literal-sort-keys */
 
@@ -119,7 +123,8 @@ function walk(ctx: Lint.WalkContext<Options>, program: ts.Program): void {
 
         const failure = ts.flattenDiagnosticMessageText(diag.messageText, "\n");
 
-        if (ignorePattern !== undefined) {
+        // BUG: this means imports / destructures with all (2+) unused variables don't respect ignore pattern
+        if (ignorePattern !== undefined && kind !== UnusedKind.DECLARATION && kind !== UnusedKind.ALL_DESTRUCTURES) {
             const varName = /'(.*)'/.exec(failure)![1];
             if (ignorePattern.test(varName)) {
                 continue;
@@ -411,6 +416,7 @@ const enum UnusedKind {
     VARIABLE_OR_PARAMETER,
     PROPERTY,
     DECLARATION, // Introduced in TS 2.8
+    ALL_DESTRUCTURES, // introduced in TS 2.9
 }
 function getUnusedDiagnostic(diag: ts.Diagnostic): UnusedKind | undefined  {
     // https://github.com/Microsoft/TypeScript/blob/master/src/compiler/diagnosticMessages.json
@@ -423,6 +429,8 @@ function getUnusedDiagnostic(diag: ts.Diagnostic): UnusedKind | undefined  {
             return UnusedKind.PROPERTY; // "Property '{0}' is declared but never used."
         case 6192:
             return UnusedKind.DECLARATION; // "All imports in import declaration are unused."
+        case 6198:
+            return UnusedKind.ALL_DESTRUCTURES; // "All destructured elements are unused."
         default:
             return undefined;
     }

diff --git a/test/rules/no-unused-variable/default/var.ts.lint b/test/rules/no-unused-variable/default/var.ts.lint
@@ -40,8 +40,12 @@ export function testDestructuring() {
     var [c] = [3];
 
     var {d, e} = { d: 1, e: 2 };
+#if typescript >= 2.9.0
+        ~~~~~~                   [All destructured elements are unused.]
+#else
          ~                       [err % ('d')]
             ~                    [err % ('e')]
+#endif
     var {f} = { f: 3 };
 
     return c * f;
@@ -67,9 +71,13 @@ for(let e of [1,2,3]) {
 export function testRenamingDestructure() {
   var a = 2;
   let {a: b} = {a: 4};
-#if typescript >= 2.8.0
+#if typescript >= 2.9.0
+      ~~~~~~           [err % ('b')]
+#endif
+#if typescript >= 2.8.0 < 2.9.0
        ~~~~            [err % ('b')]
-#else
+#endif
+#if typescript < 2.8.0
           ~            [err % ('b')]
 #endif
   let {x: y} = {x: 7}; // false positive

diff --git a/test/rules/no-unused-variable/ignore-pattern/var.ts.lint b/test/rules/no-unused-variable/ignore-pattern/var.ts.lint
@@ -44,9 +44,17 @@ export function testDestructuring() {
     var [c] = [3];
 
     var {d, e} = { d: 1, e: 2 };
+#if typescript >= 2.9.0
+        ~~~~~~                   [All destructured elements are unused.]
+#else
          ~                       [err % ('d')]
             ~                    [err % ('e')]
+#endif
     var {d: _d, e: _e} = { d: 1, e: 2 };
+#if typescript >= 2.9.0
+        ~~~~~~~~~~~~~~           [All destructured elements are unused.]
+#endif
+    // this above is incorrect, unfortunately w/ TS 2.9+, ignore pattern doesn't work for this sort of error
     var {f} = { f: 3 };
 
     return c * f;
@@ -77,9 +85,13 @@ for(let _e of [1,2,3]) {
 export function testRenamingDestructure() {
   var a = 2;
   let {a: b} = {a: 4};
-#if typescript >= 2.8.0
+#if typescript >= 2.9.0
+      ~~~~~~           [err % ('b')]
+#endif
+#if typescript >= 2.8.0 < 2.9.0
        ~~~~            [err % ('b')]
-#else
+#endif
+#if typescript < 2.8.0
           ~            [err % ('b')]
 #endif
   let {a: _b} = {a: 4};

diff --git a/test/rules/no-unused-variable/type-checked/destructuring.ts.lint b/test/rules/no-unused-variable/type-checked/destructuring.ts.lint
@@ -1,9 +1,13 @@
 import { SomeClass, someVar } from "./some.test";
 const person = { name: "alice" };
 const { name } = person;
-#if typescript >= 2.6.0
+#if typescript >= 2.9.0
+      ~~~~~~~~ ['name' is declared but its value is never read.]
+#endif
+#if typescript >= 2.6.0 < 2.9.0
         ~~~~ ['name' is declared but its value is never read.]
-#else
+#endif
+#if typescript < 2.6.0
         ~~~~ ['name' is declared but never used.]
 #endif