ts-unused-exports icon indicating copy to clipboard operation
ts-unused-exports copied to clipboard

TSLint or ESLint rule

Open felixfbecker opened this issue 6 years ago • 13 comments

I was confused that this is not implemented as a tslint rule. It would be great to have the editor integration, line/column data for errors, shareable configuration and autofixing of tslint for this.

felixfbecker avatar Jun 13 '18 04:06 felixfbecker

AFAIK, tslint operates on one file at a time.

vsviridov avatar Jan 09 '19 22:01 vsviridov

Yes tslint rules only work on 1 file at a time.

So I don't see how this could be implemented.

Closing.

mrseanryan avatar Oct 19 '19 12:10 mrseanryan

Don’t tslint/typescript-eslint rules have access to the type checker which has the whole project loaded? Can that bot be used to find references?

felixfbecker avatar Oct 20 '19 00:10 felixfbecker

We should definitely explore the linter possibility. Specially now that we have TS support in ESLint.

In raw TS, with createProgram we can access the TypeChecker. I'm not sure if that's enough to catch all usage instances but we could look into it.

pzavolinsky avatar Oct 20 '19 03:10 pzavolinsky

(re-opened)

With tslint my understanding is, it works on 1 file at a time, and will have access to whatever types that file uses. I don't think it works by first opening the whole project. So, I don't see how a tslint rule can determine if an export is unused, without false positives ...

However - eslint (eslint-typescript) is replacing tslint.

I don't know that much about how eslint works.

There is a Visitor pattern, and a rule can implement one or more of the visiting methods.

For example, this is a simple rule, that accepts visits from Node:

https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/src/rules/camelcase.ts

This is the same general pattern used by tslint rules.


brainstorming:

Is is possible that eslint-typescript can load a whole project, before evaluating each file?

OR if it's possible to have static state across 'rule runs', then it could be possible to check exports by sharing a 'used exports' state across runs. But this would require 2-passes: a collection pass, and then an evaluation pass.

A concern could be performance ...

mrseanryan avatar Oct 20 '19 08:10 mrseanryan

There is a discussion here at typescript-eslint

mrseanryan avatar Oct 20 '19 14:10 mrseanryan

Yes tslint rules only work on 1 file at a time.

File boundaries seem convenient for linter implementation but otherwise a fairly arbitrary compartmentalization of linting, because file bounds have relatively little coincidence with data flow, calls etc.

monfera avatar Oct 21 '19 14:10 monfera

I was reading a little bit about ESLint rules/plugins yesterday and I don't think we can access multiple files inside a rule. More specifically I found this: image

That being said, if we completely ignore that :point_up: then we might get away with something. If we (pre-)generate the analysis for the whole project and we could incrementally update/replace everything related to the current file, we might get away with a workable prototype.

Given that we start the POC by blatantly ignoring everything that is considered a good practice from the ESLint POV this might not be an ideal solution but it just might work.

A brutal first approach would be to run a a full ts-unused-exports analysis for each file. Performance will be laughable (specially in big projects) but we could quickly check if this as a viable solution. If it is, we could start thinking on incremental analysis (something that will be required to get this done).

pzavolinsky avatar Nov 07 '19 17:11 pzavolinsky

I'd recommend looking to integrate with typescript-eslint. typescript-eslint has many rules that use type information, which means it does have access to the whole project, because otherwise it wouldn't be able to make use of full type information (which needs resolving types from other files). This also means there is a way for rules to get a reference to the type checker/project, and hopefully that would be enough to implement this rule.

felixfbecker avatar Nov 07 '19 21:11 felixfbecker

Hi all, will the new rule offer something better than the approach described here (https://github.com/typescript-eslint/typescript-eslint/issues/371#issuecomment-500927802). I understand that approach requires some configuration but apart from that, does this new rule seem to offer any other benefits?


Edit: Upon testing, that approach was catching unused-exports of functions and variables but not the interfaces.

maneetgoyal avatar Mar 08 '20 23:03 maneetgoyal

@maneetgoyal the approach in that comment seems more suited to eslint architecture. (Parser and Resolver).

But either way sounds like a rewrite

mrseanryan avatar Mar 09 '20 17:03 mrseanryan

You might try this https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-unused-modules.md

It worked like a charm for me. The extends config with "plugin:import/typescript" was essential for me, otherwise the lint would take forever and return nothing. If you do any absolute webpack config, check their docs for the resolvers.

cscleison avatar Apr 26 '21 11:04 cscleison

Thanks @cscleison, that rule is working perfectly for me!

RE: "plugin:import/typescript" For people who are using the Airbnb TypeScript Config, then you don't have to do anything extra, it just works.

Zamiell avatar Jun 14 '21 05:06 Zamiell