palantir / tslint

:vertical_traffic_light: An extensible linter for the TypeScript language
http://palantir.github.io/tslint/
Apache License 2.0
5.91k stars 889 forks source link

no-unused-variable: All imports unused #2577

Closed jasekiw closed 6 years ago

jasekiw commented 7 years ago

Bug Report

TypeScript code being linted

import {Injectable} from "@angular/core";
import {RoutesFactoryContract} from "./RoutesFactoryContract";
/**
 * An environment helper to get fully qualified urls
 */
@Injectable()
export class MockRoutesFactory implements RoutesFactoryContract {

  /**
   * Generates a url given from the environment that is a api url meaning it will contain the api base url.
   * @param {string} url The url with placeholder templates in it. placeholder templates look like this: {id}
   * @param {Object} parameters key to value mapping of placeholder name to the real value. The keys shall not include the {}
   * @return string The rendered url
   */
  public getApiUrl(url: string, parameters?: Object): string {
    return url;
  }

  /**
   * Generates a url given from the environment.
   * @param {string} url The url with placeholder templates in it. placeholder templates look like this: {id}
   * @param {Object} parameters key to value mapping of placeholder name to the real value. The keys shall not include the {}
   * @return string The rendered url
   */
  public getUrl(url: string, parameters?: Object): string {
    return url;
  }

}

with tslint.json configuration:

{
  "rules": {
    "no-duplicate-variable": true,
    "no-constant-condition": true,
    "no-unused-variable": [
      true
    ]
  },
  "extends": [
    "tslint-eslint-rules"
  ]
}

Background:

Ionic 2 project

cli command: tslint --type-check -c tslint.json --project tsconfig.json

Actual behavior

no-unused-variable reports All imports are unused. example: ERROR: src/providers/routes/MockRoutesFactory.ts[2, 1]: All imports are unused.

Expected behavior

No errors should be found for this file.

andy-hanson commented 7 years ago

I can't reproduce this in a sample project. Can you try getting an isolated repro?

jasekiw commented 7 years ago

I am about to post a sample repo to github for this issue.

jasekiw commented 7 years ago

Here is an example project that exercises the issue.

https://github.com/jasekiw/ionic2-tslint-issue2577

andy-hanson commented 7 years ago

Here's the problem:

declare module '*';

TypeScript has an issue when using an imported type from this (presumably because no such type was ever declared). I've filed Microsoft/TypeScript#15207. Until then, you could just remove that.

jasekiw commented 7 years ago

Hi andy,

I removed that code and it still seems to be throwing "All imports are unused". Is there anything else that could be causing it?

andy-hanson commented 7 years ago

Are you getting:

ERROR: src/providers/routes/RoutesFactory.ts[4, 1]: All imports are unused.

Because that one (importing Platform on line 4) is actually unused...

jasekiw commented 7 years ago

Right, I put that in there to show what goes wrong.

There is one unused import, the other three however are used. that means

ERROR: src/providers/routes/RoutesFactory.ts[4, 1]: All imports are unused.

is a false statement.

andy-hanson commented 7 years ago

Would you prefer:

src/providers/routes/RoutesFactory.ts[4, 1]: All imports on this line are unused.
jasekiw commented 7 years ago

Interesting it looks like the wording was just changed between versions 3.15.1 and 5.1.0. I didn't realize that and thought it was throwing the same error as before.

in 3.15.1 it would throw this error instead

src/providers/routes/RoutesFactory.ts[4, 24]: Unused import: 'Platform'

I believe this error now is fine it's just not as verbose.

jasekiw commented 7 years ago

However this does break support for WebStorm. There is no longer linking in the results window. I shall open an issue with them.

amatiasq commented 7 years ago

I'm having this issue too but can't isolate it: ERROR: src/pathfinding/pathfinding.spec.ts[7, 1]: All imports are unused.

These are the first 7 lines of the offending file:

import { assert } from 'chai';
import { Vector3D } from '../core/vector3d';
import { AStar } from './a-star';
import { Area } from './area';
import { INode } from './node';
import { Pathfinding } from './pathfinding';
import { ITile } from './tile';

All imports are used, I tried removing all the imports and adding back only the ones used but it still complaining.

On a different file I have import * as sinon from 'sinon'; at line 4 which is throwing the same error ERROR: src/pathfinding/area.spec.ts[5, 1]: All imports are unused.. I can understand this one but not the previous one.

My tsconfig.json file

{
  "include": [ "src/**/*.ts" ],
  "exclude": [ "node_modules" ],

  "compilerOptions": {
    "noImplicitAny": true,
    "module": "commonjs",
    "target": "es6"
  }
}
andy-hanson commented 7 years ago

@amatiasq Can't help without knowing the rest of the file. It may be another case of Microsoft/TypeScript#14953.

amatiasq commented 7 years ago

@andy-hanson I've finally isolated the issue and it's not easy to catch: https://gist.github.com/amatiasq/7352f52d9290311589e3df5bc2afc1fe

Let me copy the readme description from the gist:

Bug in TSLint no-unused-variable rule.

How to reproduce

If you run tslint -c tslint.json --type-check --project tsconfig.json '*.ts' in this folder you'll see:

$ tslint -c tslint.json --type-check --project tsconfig.json '*.ts'
ERROR: main.ts[1, 1]: All imports are unused.

Conditions

TSLint version

5.2.0

andy-hanson commented 7 years ago

@amatiasq Fixed in typescript@next. Looks like case https://github.com/Microsoft/TypeScript/issues/14953#issuecomment-302101264.

amatiasq commented 7 years ago

@andy-hanson Are you sure? all imports in the gist are valid :\

andy-hanson commented 7 years ago

The imports may not be valid from within the no-unused-variable rule's perspective because it uses a fake project. I tested it, tell me if you're able to reproduce it using the latest tslint and typescript.

amatiasq commented 7 years ago

What do you mean with fake project? This started as a real project, I just removed the code than didn't make the bug disappear. Anyway with the last version of tslint it doesn't happen anymore.

andy-hanson commented 7 years ago

I'm referring to the implementation.

csvn commented 7 years ago

I had the same issue when a type/interface was only used inside a generic (e.g. Resolve<User>). https://github.com/Microsoft/TypeScript/issues/14953#issuecomment-302101264

Upgrading to typescript@next (>=v2.4) made the problem go away for me.

adidahiya commented 7 years ago

@csvn that's a duplicate of https://github.com/palantir/tslint/issues/2470

corydeppen commented 7 years ago

I'm running into the same issue using tslint 5.4.3 and tsc 2.4.1.

aneilbaboo commented 6 years ago

I'm seeing a similar issue with a file containing only exports:

export {foo} from './foo';
export {bar} from './bar';

Should not throw errors, but generates TS6192: All imports in import declaration are unused.

tsc --version = 3.1.3 tslint version = 5.11.0

JoshuaKGoldberg commented 6 years ago

@aneilbaboo @amatiasq I'm not able to reproduce these errors locally from the gist or what's posted in this thread. no-unused-variable is deprecated for TypeScript>=2.9. Closing this issue as non-actionable on our end; if you do still have errors around unused variables and imports/exports, please file them on TypeScript.

JoshuaKGoldberg commented 4 years ago

πŸ€– Beep boop! πŸ‘‰ TSLint is deprecated πŸ‘ˆ and you should switch to typescript-eslint! πŸ€–

πŸ”’ This issue is being locked to prevent further unnecessary discussions. Thank you! πŸ‘‹