fix(typescript-estree): don't consider a cached program unless it's specified in the current parserOptions.project config
#5999
Conversation
…pecified in the current `parserOptions.project` config
|
Thanks for the PR, @bradzacher! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. |
✅ Deploy Preview for typescript-eslint ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
| @@ -1,14 +1,11 @@ | |||
| import path from 'path'; | |||
There was a problem hiding this comment.
the changes here were extracted from #5916 as an easy test for this behaviour.
could add a regression test against this, but I didn't bother because ultimately it's such a rare case.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #5999 +/- ##
=======================================
Coverage 91.42% 91.42%
=======================================
Files 365 365
Lines 12339 12342 +3
Branches 3609 3610 +1
=======================================
+ Hits 11281 11284 +3
Misses 753 753
Partials 305 305
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| // so we don't want to consider it for caching purposes. | ||
| // | ||
| // if we did consider it we might return a program for a project | ||
| // that wasn't specified in the current parser run (which is obv bad!). |
There was a problem hiding this comment.
To you, maybe 😛
| // that wasn't specified in the current parser run (which is obv bad!). | |
| // that wasn't specified in the current parser run (which is bad!). |
PR Checklist
Overview
Discovered this while working on #5916
Previously we would consider any cached program as fair game during parsing.
Most of the time this isn't a problem because users always have the same config for an entire lint run.
However if the user has a different value for
parserOptions.projectaround the place, then we would behave incorrectly - we would iterate through our cached programs and return a matching program for a file even if its tsconfig wasn't included in the currentparserOptions.project! This is really bad because it means we'll lint a file with probably incorrect types!!!I discovered this in the above PR because I defined two separate test runs with different tsconfigs in the same file.
Because our rule tester uses the exact same filename, the parser checked the cache, found the old program matched, and returned it - meaning we never used the new tsconfig (causing the test to fail!)
The fix is simply to enforce that we only consider a program from the cache if and only if its tsconfig exists in the current
parserOptions.projectset.