Store information for files with lint warnings and errors in cache

[galvarez421]

The version of ESLint you are using.
4.15.0

The problem you want to solve.
Make cache feature (and its corresponding documentation) more useful and intuitive. Currently, the cache feature does not seem as useful or intuitive as it might be in that only information for files that pass linting with no warnings or errors is saved in the cache. I wonder if the cache feature could offer even more of a performance benefit if the cache were to store information for files with lint warnings or errors as well such that they don't have to run through a linting process if they haven't been changed since they were last linted with the cache feature enabled?

Your take on the correct solution to problem.
Perhaps we can extend the usage of the cache such that information for files with lint warnings or errors is also saved to the cache? @IanVS, following up on this comment

[ilyavolodin]

That sounds like a reasonable enhancement. I would be a bit worried about files with lining errors/warning with --fix flag and caching, since I think caching will happen before autofixes are applied.

[not-an-aardvark]

I'm concerned that this would make cache files quite large, because formatters need to be provided with the original source text of a file. If a cache file needed to maintain that information, it could effectively be the same size as the all of the javascript files combined. (However, maybe this could be mitigated by omitting the source text of a file from the results object, since we can just reread it from the filesystem if we know it hasn't changed).

[platinumazure]

However, maybe this could be mitigated by omitting the source text of a file from the results object, since we can just reread it from the filesystem if we know it hasn't changed

Agreed. We can still get a huge win here if we replace the full linting process with a read-and-report process for unchanged files with errors.

Full lint process (done on any file with errors even after linting once):

1. Read file from filesystem
2. Parse file contents
3. Prepare to lint (build selector events, set up scope analysis and visitor keys, etc.)
4. Traverse file and invoke visitors to lint file
5. Calculate output by applying fixes
6. Write output to filesystem
7. Cache results
8. Call formatter

Reading cached file results and reporting errors (assuming file mtime is earlier than cache write time so no need to re-lint):

1. Read cache to see that errors exist
2. Read file from filesystem
3. Call formatter with cached errors

My feeling is that this could be a massive benefit to someone doing a large refactor or introducing a new linting rule into their codebase. And it would be a small benefit to people who lint the codebase frequently even if they're only changing one file at a time.

[galvarez421]

@ilyavolodin If auto fix occurs after results for a file have been cached, wouldn't that cause the file to be considered newer than the cache during the next lint and thereby cause the cache to be ignored for that file? In other words, wouldn't the cache process still work as expected (although maybe the timing of the autofix relative to the caching wouldn't)? Or are you concerned not about caching not working as expected but rather about the caching not being as effective as it could be due to the timing of autofixes? Based on the description by @platinumazure of the lint process, it seems that autofixes are applied before the results are outputted and cached, so maybe this is a non-issue.

[platinumazure]

Apologies, I don't know if caching is done before or after autofix. I was just thinking it should be afterwards, ideally speaking. If a lint run results in fixable errors only, then we could apply fixes and store a cache entry saying all went well.

…

On Feb 6, 2018 6:49 AM, "Gerry Alvarez" ***@***.***> wrote: @ilyavolodin <https://github.com/ilyavolodin> If auto fix occurs after results for a file have been cached, wouldn't that cause the file to be considered newer than the cache during the next lint and thereby cause the cache to be ignored for that file? In other words, wouldn't the cache process still work as expected (although maybe the timing of the autofix relative to the caching wouldn't)? Or are you concerned not about caching not working as expected but rather about the caching not being as effective as it could be due to the timing of autofixes? Based on the description by @platinumazure <https://github.com/platinumazure> of the lint process, it seems that autofixes are applied before the results are outputted and cached, so maybe this is a non-issue. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9948 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AARWei7gycw8VShGWqK6gZJARuCzS8oPks5tSEpMgaJpZM4R6UGq> .

[platinumazure]

I'm finally digging into this. At this point, it does look like we write to the cache after the file is linted and autofixed, so I think we should be able to cache files which failed linting (though we may need to remove the full source text from the file entry cache, as discussed above).

[platinumazure]

Relabeling as "cli" based on previous CLIEngine issues (which don't have to do with linting/fixing) being labeled as "cli".

[platinumazure]

TSC Summary:

eslint --cache currently only caches files that passed linting. This is useful for users who have fully integrated ESLint into their codebase, so most of their files should be passing at any given time. However, for users that have not yet fully integrated ESLint (i.e., users who are still cleaning up lint errors), the cache becomes useless because most files are not passing at any given time, and we currently don't cache files which failed linting. It was suggested that files which failed linting could be cached, as long as we check that they haven't been modified between the cache time and the next lint run (in other words, following the same logic we follow for successfully linted files in the cache).

One challenge is that we currently include the entire source code of files with linting problems in the lint results object. This could result in the cache file growing significantly if there are a lot of files which failed linting. A possible approach to avoid this is to remove the source property when caching the lint results, and then when retrieving results from the cache, simply read the file from the filesystem again. This will slow the overall cache performance for files that failed linting, but still be much faster than reading, parsing, and linting the file again. (PR #10571 follows this approach.)

TSC Question:

Should we add files which failed linting to the lint results cache?

[platinumazure]

This issue was accepted in today's TSC meeting.