Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

--fix is very slow if there are multiple issues #3786

Closed
ljani opened this issue Mar 21, 2018 · 9 comments
Closed

--fix is very slow if there are multiple issues #3786

ljani opened this issue Mar 21, 2018 · 9 comments

Comments

@ljani
Copy link

ljani commented Mar 21, 2018

Bug Report

  • TSLint version: 5.9.1
  • TypeScript version: 2.7.2
  • Running TSLint via: CLI

--fix is very slow if there are hundreds or thousands (957 in my case) of linting issues, which are fixable automatically. For example adding a bunch of new rules or changing your code style can introduce such cases.

This issue seems to be calling createProgram in a loop. Each call may take up to a second or two to finish (at least on Windows) and if it's called a thousand times, the program takes 15 minutes to finish.

This issue was probably introduced by #2864, which seems to be an important fix though.

The actual linting process is fast though.

I can share you a profiler dump privately, if needed.

TypeScript code being linted

Any project with a thousand linting issues, which are fixable automatically.

Actual behavior

Running the linter takes like 15 minutes to finish.

Expected behavior

The linter should finish very well within a minute.

@ajafff
Copy link
Contributor

ajafff commented Mar 21, 2018

That's a known issue. If you have many style issues, you can run tslint without --project to fix those failures first. That should be way faster. Afterwards you can fix the remaining failures that require type information.

You can also use my alternative linter runtime to execute TSLint rules. It's optimized for speed. It can probably fix your whole project in a couple of seconds.
You can find all necessary instructions in the description: https://www.npmjs.com/package/@fimbul/valtyr

@ljani
Copy link
Author

ljani commented Mar 21, 2018

@ajafff Thanks. I tried searching the issues for it, but didn't find anything. Is there an issue I could subscribe to and close this one?

@ajafff
Copy link
Contributor

ajafff commented Mar 21, 2018

It turns out there is no other issue tracking this performance problem. I only answered a similar question on Gitter. So this was only a known issue for me.

Unfortunately there's not much that can be done to fix this in the current implementation. There's no separation of concerns so the linter cannot know how to update the program (it already assumes too much implementation details to be 100% correct). Therefore it needs to read (and parse) everything from disk over and over again.
Incremental parsing mutates the original SourceFile object which could/will break API users.

@ljani
Copy link
Author

ljani commented Mar 21, 2018

Thanks for the insight. I'll leave the issue open then.

The Fimbullinter project seems interesting, I'll give it a go.

@bfricka
Copy link

bfricka commented Jul 10, 2019

That's a known issue. If you have many style issues, you can run tslint without --project to fix those failures first.

That's somewhat true. However, using this, e.g. w/ tslint -c and globs, there is a bug where tslint running on macOS will miss a ton of things (even after verifying that the files being missed are indeed part of the glob). These errors will then show up, for example, in a Linux CI environment.

Edit: I should clarify that this is only an issue when running through npm scripts / yarn.

@adidahiya
Copy link
Contributor

@bfricka if you're using globs, make sure to quote the glob strings. that way it uses node glob to expand them. otherwise, without quotes, the glob expansion is done by the shell, which can vary across different environments

@bfricka
Copy link

bfricka commented Jul 10, 2019

Holy smokes. That worked. I had no idea it would use node-glob by forcing a string (instead of glob expansion). All the examples I've seen don't quote. Thanks!

@JoshuaKGoldberg
Copy link
Contributor

💀 It's time! 💀

TSLint is being deprecated and no longer accepting pull requests for major new changes or features. See #4534. 😱

If you'd like to see this change implemented, you have two choices:

  • Recommended: Check if this is available in ESLint + typescript-eslint
  • Not Recommended: Fork TSLint locally 🤷‍♂️

👋 It was a pleasure open sourcing with you!

If you believe this message was posted here in error, please comment so we can re-open the issue!

@JoshuaKGoldberg
Copy link
Contributor

🤖 Beep boop! 👉 TSLint is deprecated 👈 (#4534) and you should switch to typescript-eslint! 🤖

🔒 This issue is being locked to prevent further unnecessary discussions. Thank you! 👋

@palantir palantir locked and limited conversation to collaborators Mar 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants