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

Caching lint results #370

Closed
ajafff opened this issue Aug 6, 2018 · 4 comments · Fixed by #752
Closed

Caching lint results #370

ajafff opened this issue Aug 6, 2018 · 4 comments · Fixed by #752

Comments

@ajafff
Copy link
Member

ajafff commented Aug 6, 2018

It would be nice to support caching to speed up consecutive linter runs with only little changes to the codebase.

Requirements:

  • modification time of file
    • needs special handling for in-memory updates during autofixing?
  • hash of config for file
  • hash of compilerOptions?
  • needs some distinction of rule versions to invalidate cache on updates
  • distinction on purely AST-based rules and type checking rules
    • AST-based rules only work on one file -> only needs to be executed when file changes
    • type checking rules work on the whole project -> needs to be executed when any file in the project changes
  • TBD: is it worth to store a dependency graph for each module?
    • changes in non-modules could affect all files
    • changes in external modules invalidate files that (transitively) reference them through /// <reference />, import, export and import('') (and require and JSDoc import() in JS files)
    • special handling for @types, wildcard ambient modules, global augmentations, module agumentations, export as default namespace
    • do we need to support ///<amd-dependency>?
  • determine modification time of every file in the project upfront to avoid linting anything if nothing changed?
    • probably not possible as it needs to know about every config as well
  • should rules be allowed to disable caching? (i.e. rules that access other files on the file system - even though that's discouraged)
  • for a processed file, use the modification timestamps of all files it consists of
  • how does this work with plugin modules (e.g. running with and without -m @fimbul/valtyr)
    • API users need to provide an ID for the current setup
    • CLI could just use a hash of the parsed options merged with any unknown option specified in .fimbullinter.yaml
  • make cache file configurable?
    • if yes, it needs to contain the CWD as this is important for TypeScripts lookup of declaration files
  • how/when to remove old entries from cache?
    • override the whole cache with the new content
    • store all files included in the project
    • have a special marker for files that were not linted
@ajafff
Copy link
Member Author

ajafff commented Mar 4, 2019

Some more recent thoughts on this:

  • it would be useful to reuse the cache on other machines and CI
  • rather than using the modification timestamp it could make sense to just use the hashed content
  • store all paths relative to the current directory / the project directory

@ajafff
Copy link
Member Author

ajafff commented Apr 6, 2019

More things to consider:

  • UMD globals are only available in global scripts (unless they add a flag)
  • Files with only ambient modules don't affect global scope.
  • Files with module augmentations or ambient modules depend on all other files affecting the same module name.
  • Handle skip(Default)LibCheck?
  • Findings might depend on the uses of a declaration. Therefore we also need to consider downstream consumers in addition to upstream dependencies (make rules have Metadata to tell this?)
  • Findings could depend on completely unrelated files (e.g. duplicate function implementation). That doesn't work with caching at all.

@ajafff
Copy link
Member Author

ajafff commented Apr 9, 2019

  • Findings might depend on the uses of a declaration. Therefore we also need to consider downstream consumers in addition to upstream dependencies (make rules have Metadata to tell this?)
  • Findings could depend on completely unrelated files (e.g. duplicate function implementation). That doesn't work with caching at all.

Possible solution: rules should call a method on RuleContext with the file name of each file they inspect other than the current one. It's not necessary to report files that are (transitive) dependencies of the current file. That means following Symbol/Signature.declaration is fine. You only need to add something special if you use Program#getSourceFiles

@ajafff
Copy link
Member Author

ajafff commented Feb 19, 2021

🎉 This issue has been resolved in version v0.24.0 🎉

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.

1 participant