diff --git a/.circleci/config.yml b/.circleci/config.yml index be4aed00b67..f4aaeddef2f 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -63,6 +63,28 @@ jobs: key: dependency-cache-{{ checksum "yarn.lock" }} - run: yarn add typescript@2.4 - run: yarn test + test2.7: + docker: + - image: circleci/node:6 + steps: + - checkout + - attach_workspace: + at: '.' + - restore_cache: + key: dependency-cache-{{ checksum "yarn.lock" }} + - run: yarn add typescript@2.7 + - run: yarn test + test2.8: + docker: + - image: circleci/node:6 + steps: + - checkout + - attach_workspace: + at: '.' + - restore_cache: + key: dependency-cache-{{ checksum "yarn.lock" }} + - run: yarn add typescript@2.8 + - run: yarn test testNext: docker: - image: circleci/node:latest @@ -92,6 +114,12 @@ workflows: - test2.4: requires: - build + - test2.7: + requires: + - build + - test2.8: + requires: + - build - testNext: requires: - build diff --git a/package.json b/package.json index da8cf42731d..9c0a72a9c6f 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,7 @@ "tsutils": "^2.12.1" }, "peerDependencies": { - "typescript": ">=2.1.0 || >=2.1.0-dev || >=2.2.0-dev || >=2.3.0-dev || >=2.4.0-dev || >=2.5.0-dev || >=2.6.0-dev || >=2.7.0-dev || >=2.8.0-dev" + "typescript": ">=2.1.0 || >=2.1.0-dev || >=2.2.0-dev || >=2.3.0-dev || >=2.4.0-dev || >=2.5.0-dev || >=2.6.0-dev || >=2.7.0-dev || >=2.8.0-dev || >=2.9.0-dev" }, "devDependencies": { "@types/babel-code-frame": "^6.20.0", @@ -75,7 +75,7 @@ "ts-node": "^3.3.0", "tslint": "^5.8.0", "tslint-test-config-non-relative": "file:test/external/tslint-test-config-non-relative", - "typescript": "~2.7.2" + "typescript": "~2.8.3" }, "license": "Apache-2.0", "engines": { diff --git a/src/rules/noUnusedVariableRule.ts b/src/rules/noUnusedVariableRule.ts index 5a0aa8e093c..908a80a1fd8 100644 --- a/src/rules/noUnusedVariableRule.ts +++ b/src/rules/noUnusedVariableRule.ts @@ -126,17 +126,19 @@ function walk(ctx: Lint.WalkContext, program: ts.Program): void { } } - if (kind === UnusedKind.VARIABLE_OR_PARAMETER) { - const importName = findImport(diag.start, sourceFile); - if (importName !== undefined) { - if (declaration && isImportUsed(importName, sourceFile, checker)) { - continue; - } + if (kind === UnusedKind.VARIABLE_OR_PARAMETER || kind === UnusedKind.DECLARATION) { + const importNames = findImports(diag.start, sourceFile, kind); + if (importNames.length > 0) { + for (const importName of importNames) { + if (declaration && isImportUsed(importName, sourceFile, checker)) { + continue; + } - if (importSpecifierFailures.has(importName)) { - throw new Error("Should not get 2 errors for the same import."); + if (importSpecifierFailures.has(importName)) { + throw new Error("Should not get 2 errors for the same import."); + } + importSpecifierFailures.set(importName, failure); } - importSpecifierFailures.set(importName, failure); continue; } } @@ -337,12 +339,14 @@ function forEachImport(sourceFile: ts.SourceFile, f: (i: ImportLike) => T | u }); } -function findImport(pos: number, sourceFile: ts.SourceFile): ts.Identifier | undefined { - return forEachImport(sourceFile, (i) => { +function findImports(pos: number, sourceFile: ts.SourceFile, kind: UnusedKind): ReadonlyArray { + const imports = forEachImport(sourceFile, (i) => { + if (!isInRange(i, pos)) { + return undefined; + } + if (i.kind === ts.SyntaxKind.ImportEqualsDeclaration) { - if (i.name.getStart() === pos) { - return i.name; - } + return [i.name]; } else { if (i.importClause === undefined) { // Error node @@ -351,37 +355,74 @@ function findImport(pos: number, sourceFile: ts.SourceFile): ts.Identifier | und const { name: defaultName, namedBindings } = i.importClause; if (namedBindings !== undefined && namedBindings.kind === ts.SyntaxKind.NamespaceImport) { - const { name } = namedBindings; - if (name.getStart() === pos) { - return name; - } - return undefined; + return [namedBindings.name]; } - if (defaultName !== undefined && defaultName.getStart() === pos) { - return defaultName; + // Starting from TS2.8, when all imports in an import node are not used, + // TS emits only 1 diagnostic object for the whole line as opposed + // to the previous behavior of outputting a diagnostic with kind == 6192 + // (UnusedKind.VARIABLE_OR_PARAMETER) for every unused import. + // From TS2.8, in the case of none of the imports in a line being used, + // the single diagnostic TS outputs are different between the 1 import + // and 2+ imports cases: + // - 1 import in node: + // - diagnostic has kind == 6133 (UnusedKind.VARIABLE_OR_PARAMETER) + // - the text range is the whole node (`import { ... } from "..."`) + // whereas pre-TS2.8, the text range was for the import node. so + // `name.getStart()` won't equal `pos` like in pre-TS2.8 + // - 2+ imports in node: + // - diagnostic has kind == 6192 (UnusedKind.DECLARATION) + // - we know that all of these are unused + if (kind === UnusedKind.DECLARATION) { + const imp: ts.Identifier[] = []; + if (defaultName !== undefined) { + imp.push(defaultName); + } + if (namedBindings !== undefined) { + imp.push(...namedBindings.elements.map((el) => el.name)); + } + return imp.length > 0 ? imp : undefined; + } else if (defaultName !== undefined && ( + isInRange(defaultName, pos) || namedBindings === undefined // defaultName is the only option + )) { + return [defaultName]; } else if (namedBindings !== undefined) { - for (const { name } of namedBindings.elements) { - if (name.getStart() === pos) { - return name; + if (namedBindings.elements.length === 1) { + return [namedBindings.elements[0].name]; + } + + for (const element of namedBindings.elements) { + if (isInRange(element, pos)) { + return [element.name]; } } } } return undefined; }); + return imports !== undefined ? imports : []; +} + +function isInRange(range: ts.TextRange, pos: number): boolean { + return range.pos <= pos && range.end >= pos; } const enum UnusedKind { VARIABLE_OR_PARAMETER, PROPERTY, + DECLARATION, // Introduced in TS 2.8 } function getUnusedDiagnostic(diag: ts.Diagnostic): UnusedKind | undefined { + // https://github.com/Microsoft/TypeScript/blob/master/src/compiler/diagnosticMessages.json switch (diag.code) { - case 6133: - return UnusedKind.VARIABLE_OR_PARAMETER; // "'{0}' is declared but never used. + case 6133: // Pre TS 2.9 "'{0}' is declared but never used. + // TS 2.9+ "'{0}' is declared but its value is never read." + case 6196: // TS 2.9+ "'{0}' is declared but never used." + return UnusedKind.VARIABLE_OR_PARAMETER; case 6138: return UnusedKind.PROPERTY; // "Property '{0}' is declared but never used." + case 6192: + return UnusedKind.DECLARATION; // "All imports in import declaration are unused." default: return undefined; } diff --git a/src/runner.ts b/src/runner.ts index 581d2438621..1e65dcd348b 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -269,6 +269,7 @@ async function doLinting(options: Options, files: string[], program: ts.Program } else { contents = await tryReadFile(file, logger); } + if (contents !== undefined) { linter.lint(file, contents, configFile); } diff --git a/test/rules/completed-docs/defaults/test.ts.lint b/test/rules/completed-docs/defaults/test.ts.lint index 63ba0b5757b..aca3361617f 100644 --- a/test/rules/completed-docs/defaults/test.ts.lint +++ b/test/rules/completed-docs/defaults/test.ts.lint @@ -12,9 +12,12 @@ export class Aaa { public set prop(value) { this.bbb = value; } ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for public properties.] - // TODO: TypeScript API doesn't give us a symbol for this, so we must ignore it. + // TypeScript API doesn't give a symbol for this before version 2.8.0 // https://github.com/Microsoft/TypeScript/issues/14257 [Symbol.iterator]() {} +#if typescript >= 2.8.0 + ~~~~~~~~~~~~~~~~~~~~~~ [Documentation must exist for methods.] +#endif } export enum Ddd { } diff --git a/test/rules/no-unnecessary-type-assertion/test.ts.fix b/test/rules/no-unnecessary-type-assertion/test.ts.fix index 6bc4afd88e5..17726c9bc0c 100644 --- a/test/rules/no-unnecessary-type-assertion/test.ts.fix +++ b/test/rules/no-unnecessary-type-assertion/test.ts.fix @@ -95,8 +95,6 @@ interface NotATuple { declare const notATuple: NotATuple; notATuple; -unknownName; - function foo() { let xx: 1 | 2 = 1; const f = () => xx = 2; diff --git a/test/rules/no-unnecessary-type-assertion/test.ts.lint b/test/rules/no-unnecessary-type-assertion/test.ts.lint index a0068aebfb5..0bac9d59232 100644 --- a/test/rules/no-unnecessary-type-assertion/test.ts.lint +++ b/test/rules/no-unnecessary-type-assertion/test.ts.lint @@ -114,9 +114,6 @@ declare const notATuple: NotATuple; notATuple; ~~~~~~~~~~~~~~~~~~~~ [0] -unknownName!; -~~~~~~~~~~~~ [0] - function foo() { let xx: 1 | 2 = 1; const f = () => xx = 2; diff --git a/test/rules/no-unused-variable/check-parameters/test.ts.lint b/test/rules/no-unused-variable/check-parameters/test.ts.lint index a2298e7f238..de0c022dabb 100644 --- a/test/rules/no-unused-variable/check-parameters/test.ts.lint +++ b/test/rules/no-unused-variable/check-parameters/test.ts.lint @@ -66,7 +66,15 @@ export class DestructuringTests { } abstract class AbstractTest { +#if typescript >= 2.9.0 +~~~~~~~~~~~~~~~~~~~~~~~~~~~ ['AbstractTest' is declared but never used.] +#endif +#if typescript >= 2.8.0 < 2.9.0 +~~~~~~~~~~~~~~~~~~~~~~~~~~~ [err % ('AbstractTest')] +#endif +#if typescript < 2.8.0 ~~~~~~~~~~~~ [err % ('AbstractTest')] +#endif abstract foo(x); } diff --git a/test/rules/no-unused-variable/default/false-positives.ts.lint b/test/rules/no-unused-variable/default/false-positives.ts.lint index 5a80e6dc984..63724222835 100644 --- a/test/rules/no-unused-variable/default/false-positives.ts.lint +++ b/test/rules/no-unused-variable/default/false-positives.ts.lint @@ -2,7 +2,11 @@ const fs = require("fs"); module Foo { - ~~~ [err % ('Foo')] +#if typescript >= 2.8.0 +~~~~~~~~~~ [err % ('Foo')] +#else + ~~~ [err % ('Foo')] +#endif const path = require("path"); console.log(fs); @@ -24,7 +28,11 @@ hello.sayHello(); import Bar = whatever.that.Foo; module some.module.blah { - ~~~~ [err % ('some')] +#if typescript >= 2.8.0 +~~~~~~~~~~~ [err % ('some')] +#else + ~~~~ [err % ('some')] +#endif export class bar { private bar: Bar; constructor() { diff --git a/test/rules/no-unused-variable/default/function.ts.lint b/test/rules/no-unused-variable/default/function.ts.lint index 3799d63faad..e10a28b6239 100644 --- a/test/rules/no-unused-variable/default/function.ts.lint +++ b/test/rules/no-unused-variable/default/function.ts.lint @@ -3,12 +3,16 @@ function func1(x: number, y: number) { } var func2 = () => { - ~~~~~ [err % ('func2')] + ~~~~~ [err % ('func2')] // }; function func3() { - ~~~~~ [err % ('func3')] +#if typescript >= 2.8.0 +~~~~~~~~~~~~~~ [err % ('func3')] +#else + ~~~~~ [err % ('func3')] +#endif return func1(1, 2); } @@ -17,7 +21,11 @@ export function func4() { } declare function func5(): any; +#if typescript >= 2.8.0 +~~~~~~~~~~~~~~~~~~~~~~ [err % ('func5')] +#else ~~~~~ [err % ('func5')] +#endif export default function () { return 0; diff --git a/test/rules/no-unused-variable/default/import.ts.fix b/test/rules/no-unused-variable/default/import.ts.fix index f9537b1224b..961e3f72bad 100644 --- a/test/rules/no-unused-variable/default/import.ts.fix +++ b/test/rules/no-unused-variable/default/import.ts.fix @@ -50,3 +50,4 @@ import abc = require('a'); import def = abc.someVar; console.log(def); + diff --git a/test/rules/no-unused-variable/default/import.ts.lint b/test/rules/no-unused-variable/default/import.ts.lint index 01d47860887..cae8d224aa6 100644 --- a/test/rules/no-unused-variable/default/import.ts.lint +++ b/test/rules/no-unused-variable/default/import.ts.lint @@ -14,9 +14,9 @@ import {a3 as aa3, a4 as aa4} from "a"; aa3; // This import statement is unused and will be deleted along with this comment. import {a5, a6} from "a"; -~~~~~~~~~~~~~~~~~~~~~~~~~ [All imports on this line are unused.] +~~~~~~~~~~~~~~~~~~~~~~~~~ [allErr] import {a7} from "a"; -~~~~~~~~~~~~~~~~~~~~~ [All imports on this line are unused.] +~~~~~~~~~~~~~~~~~~~~~ [allErr] import {a8, a9, a10} from "a"; ~~ [err % ('a9')] ~~~ [err % ('a10')] @@ -32,7 +32,7 @@ a16; // Leading comment will not be deleted import {unusedFunction} from "legacyDependency"; // Import unusedFunction because .... -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [All imports on this line are unused.] +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [allErr] // Next comment will not be deleted export import a = require("a"); @@ -42,7 +42,11 @@ $(_.xyz()); /// module S { +#if typescript >= 2.8.0 +~~~~~~~~ [err % ('S')] +#else ~ [err % ('S')] +#endif var template = ""; ~~~~~~~~ [err % ('template')] } @@ -54,17 +58,17 @@ import baz from "a"; import defaultExport, { namedExport } from "a"; ~~~~~~~~~~~~~ [err % ('defaultExport')] import d1, { d2 } from "a"; -~~~~~~~~~~~~~~~~~~~~~~~~~~~ [All imports on this line are unused.] +~~~~~~~~~~~~~~~~~~~~~~~~~~~ [allErr] import d3, { d4 } from "a"; ~~~~~~ [All named bindings are unused.] d3; d3;import d5 from "a"; - ~~~~~~~~~~~~~~~~~~~ [All imports on this line are unused.] + ~~~~~~~~~~~~~~~~~~~ [allErr] import d6 from "a";d3; -~~~~~~~~~~~~~~~~~~~ [All imports on this line are unused.] +~~~~~~~~~~~~~~~~~~~ [allErr] d3;import d7 from "a";d3; - ~~~~~~~~~~~~~~~~~~~ [All imports on this line are unused.] + ~~~~~~~~~~~~~~~~~~~ [allErr] bar.someFunc(); baz(); @@ -81,3 +85,6 @@ console.log(def); #else [err]: '%s' is declared but never used. #endif + +[allErr]: All imports on this line are unused. +[allErrDecl]: All imports in import declaration are unused. diff --git a/test/rules/no-unused-variable/default/var.ts.lint b/test/rules/no-unused-variable/default/var.ts.lint index 04079d7c34f..24cf08ae0cb 100644 --- a/test/rules/no-unused-variable/default/var.ts.lint +++ b/test/rules/no-unused-variable/default/var.ts.lint @@ -10,7 +10,15 @@ var z; export var abcd = 3; class ABCD { +#if typescript >= 2.9.0 +~~~~~~~~~~ ['ABCD' is declared but never used.] +#endif +#if typescript >= 2.8.0 < 2.9.0 +~~~~~~~~~~ [err % ('ABCD')] +#endif +#if typescript < 2.8.0 ~~~~ [err % ('ABCD')] +#endif constructor() { z = 3; } @@ -59,7 +67,11 @@ for(let e of [1,2,3]) { export function testRenamingDestructure() { var a = 2; let {a: b} = {a: 4}; +#if typescript >= 2.8.0 + ~~~~ [err % ('b')] +#else ~ [err % ('b')] +#endif let {x: y} = {x: 7}; // false positive return a + y; } diff --git a/test/rules/no-unused-variable/ignore-pattern/var.ts.lint b/test/rules/no-unused-variable/ignore-pattern/var.ts.lint index fca1919552c..54f52c0decc 100644 --- a/test/rules/no-unused-variable/ignore-pattern/var.ts.lint +++ b/test/rules/no-unused-variable/ignore-pattern/var.ts.lint @@ -13,7 +13,15 @@ var z; export var abcd = 3; class ABCD { - ~~~~ [err % ('ABCD')] +#if typescript >= 2.9.0 +~~~~~~~~~~ ['ABCD' is declared but never used.] +#endif +#if typescript >= 2.8.0 < 2.9.0 +~~~~~~~~~~ [err % ('ABCD')] +#endif +#if typescript < 2.8.0 + ~~~~ [err % ('ABCD')] +#endif constructor() { z = 3; } @@ -69,7 +77,11 @@ for(let _e of [1,2,3]) { export function testRenamingDestructure() { var a = 2; let {a: b} = {a: 4}; +#if typescript >= 2.8.0 + ~~~~ [err % ('b')] +#else ~ [err % ('b')] +#endif let {a: _b} = {a: 4}; let {x: y} = {x: 7}; // false positive return a + y; diff --git a/yarn.lock b/yarn.lock index 485b8d0a7f6..1ff0c581ecf 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1601,9 +1601,9 @@ type-detect@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/type-detect/-/type-detect-1.0.0.tgz#762217cc06db258ec48908a1298e8b95121e8ea2" -typescript@~2.7.2: - version "2.7.2" - resolved "https://registry.yarnpkg.com/typescript/-/typescript-2.7.2.tgz#2d615a1ef4aee4f574425cdff7026edf81919836" +typescript@~2.8.3: + version "2.8.3" + resolved "https://registry.yarnpkg.com/typescript/-/typescript-2.8.3.tgz#5d817f9b6f31bb871835f4edf0089f21abe6c170" uglify-js@^2.6: version "2.8.28"