Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep track of already linted/autocorrected files. #868

Closed
nhgrif opened this issue Nov 7, 2016 · 7 comments · Fixed by #1073
Closed

Keep track of already linted/autocorrected files. #868

nhgrif opened this issue Nov 7, 2016 · 7 comments · Fixed by #1073

Comments

@nhgrif
Copy link

nhgrif commented Nov 7, 2016

I'm using SwiftLint with a code base of around 30,000 lines of code (and growing). We have set up a build phase to run SwiftLint (and SwiftLint autocorrect) on every build. Some devs on the project are reporting that this takes a significantly long amount of time even when they have not modified any files.

On every run, SwiftLint could keep track of which files it checked or corrected and when. Then, on a future run, SwiftLint will determine if the .swiftlint.yml or the Swift file has been modified since the last run. If neither has been modified, SwiftLint will skip the file.

On a version update from SwiftLint, it should clear out these records so the first run of a new version of SwiftLint, it re-checks every file.

@scottrhoyt
Copy link
Contributor

scottrhoyt commented Nov 7, 2016

This sounds a bit out of scope. If you just don't lint files that have been modified from the last linting, then you will loose any warnings/errors generated by those. Simply running swiftlint twice in a row would therefore always not emit any errors or warnings. You could solve this by tracking only files that have linted successfully, but my point is that I think this gets complicated quickly--particularly if you want to clear the linting status of every project on swiftlint configuration changes and upgrades.

There are a few other solutions you can consider:

  • If the codebase contains any 3rd party libraries or files/directories that seldom change, consider excluding them in your .swiftlint.yml.
  • Run swiftlint lint --benchmark to generate benchmark files. You can use these files to determine if there are any rules you can disable or files you can exclude to speed up the process.
  • Some users have tied in swiftlint to a pre-commit hook. Doing this would have the desired result of linting only changed files. However, you lose the feedback in your build stage. This also would not re-lint every file if your configuration changed or swiftlint was upgraded. I believe if you search the issues hear you will find some notes on that approach.

@scottrhoyt
Copy link
Contributor

If you run swiftlint lint --benchmark, files will be generated informing you how much time was spent per-rule and per-file. Have a look at the rule benchmarks and see if there are any rules that are taking a significant amount of time. If you decide you want to not run that rule, you can add it to the disabled_rules key in your configuration if it is a default rule or remove it from opt_in_rules if it is an opt-in rule.

@nhgrif
Copy link
Author

nhgrif commented Nov 7, 2016

Will the --benchmark command also work with autocorrect?

@scottrhoyt
Copy link
Contributor

No. But autocorrect generally shouldn't be run automatically anyway.

For reference, you can run swiftlint help and swiftlint help <action> to get command line usage options.

@nhgrif
Copy link
Author

nhgrif commented Nov 7, 2016

When you run --benchmark and find stuff that should've been caught in a PR...

@jpsim
Copy link
Collaborator

jpsim commented Nov 25, 2016

What you're requesting is akin to a compiler cache, and probably very useful for large projects.

I'd certainly welcome a PR to SwiftLint that added this type of caching.

In the meantime, some users are using modified files in git to only lint changed files: #785 (comment). That may be an option for you too.

@marcelofabri
Copy link
Collaborator

I was thinking about this since we now have more rules and linting takes more time. On my MBP 15", https://github.com/kickstarter/ios-oss takes almost 5s and I believe that would prevent me for running SwiftLint on each build if I were working on that project.

Here're some random thoughts:

  • The cache should store the SwiftLint version used to lint. If it has changed, discard the cache.
  • The cache should store the configuration (probably its hash) used to lint. If it has changed, discard the cache.
  • I was thinking we should keep the cache in a SQLite file. We'd have two tables:

File

- id
- file_path <- this would be indexed
- last_lint_date
- contents_hash 

Violation

- file_id <- this would be indexed
- line
- character
- severity
- rule_id
- rule_name
- reason
  • Alternatively, we could store the cache in a JSON file, with file_path being the keys of a dictionary. This has the advantage of being easier to implement, but the whole cache would be read into memory.
  • I think it'd be easier to keep the file in the same folder as the linted project, but it might be committed accidentally.
  • We should support a parameter to ignore the cache when linting. I'm not sure it should overwrite the cache in this case (probably yes).
  • We should support a custom cache path.
  • Should we remove non-linted files from the cache? This might happen if a file was moved/deleted but also if it wasn't specified on a particular execution (linting only files that were modified according to git for example).
  • The cache shouldn't be used in autocorrect.
  • We could make it an experimental feature at first (disabled by default).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants