feat: remove partial type-information program #6066

bradzacher · 2022-11-23T05:53:53Z

BREAKING CHANGE:
[utils] sets program = null for getParserServices(context, true).
[typescript-estree] disallows errorOnTypeScriptSyntacticAndSemanticIssues if project not provided

PR Checklist

Addresses an existing open issue: fixes Enhancement: Remove 2 year old backwards compatibility check from getParserServices #5508
That issue was marked as accepting prs
Steps in CONTRIBUTING.md were taken

Overview

A few years ago I experimented with leveraging TS's unused variable diagnostics to power a lint rule. As this diagnostic is "single-file", I added the ability for the parser to generate a program in "isolated" mode (i.e. no parserOptions.project required and types/diagnostics only produced for the single file) as well as the ability for plugins to interact with this program.

However this approach had some major drawbacks:

calculating diagnostics in TS is slow - so the lint rule was slow as there's no way to tell TS to calculate specific diagnostics.
the isolated types are completely useless because in most modules so many types are seeded from imports.

We never bothered going back to cleanup this work (until now).

This PR:

updates the parser so that it only works with ts.SourceFiles if we're not doing type-aware linting.
- this should have the added bonus of speeding the parser up for non-type-aware runs as we're not going through the heavy process of creating a program!
updates the getParserServices util as appropriate
- Note: I left the ability to consume parser services without type info because the node maps can be useful in some situations, or sometimes it's useful to consume compiler options if provided.
removes the semantic-diagnostics-enabled test because it's a test that we don't get any value out of - it's just emitting TS errors which might change from version to version
only allows errorOnTypeScriptSyntacticAndSemanticIssues if we have full type info.
- This is because TS doesn't compute a lot of diagnostics without a program!

typescript-eslint · 2022-11-23T05:53:55Z

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.

nx-cloud · 2022-11-23T05:53:56Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 526c1d1. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 47 targets

Sent with 💌 from NxCloud.

codecov · 2022-12-02T02:30:54Z

Codecov Report

Merging #6066 (526c1d1) into v6 (dde6861) will decrease coverage by 0.02%.
The diff coverage is 70.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##               v6    #6066      +/-   ##
==========================================
- Coverage   91.66%   91.64%   -0.02%     
==========================================
  Files         354      354              
  Lines       12199    12200       +1     
  Branches     3581     3582       +1     
==========================================
- Hits        11182    11181       -1     
- Misses        721      722       +1     
- Partials      296      297       +1

Flag	Coverage Δ
unittest	`91.64% <70.00%> (-0.02%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...-estree/src/create-program/createDefaultProgram.ts	`78.26% <ø> (ø)`
...estree/src/create-program/createIsolatedProgram.ts	`64.00% <ø> (-12.00%)`	⬇️
...-estree/src/create-program/createProjectProgram.ts	`93.47% <ø> (ø)`
...ges/typescript-estree/src/create-program/shared.ts	`83.33% <ø> (ø)`
...t-estree/src/create-program/useProvidedPrograms.ts	`84.84% <ø> (ø)`
...ckages/utils/src/eslint-utils/getParserServices.ts	`10.00% <0.00%> (ø)`
...escript-estree/src/semantic-or-syntactic-errors.ts	`86.66% <75.00%> (ø)`
...eslint-plugin/src/rules/consistent-type-exports.ts	`97.70% <100.00%> (ø)`
...kages/eslint-plugin/src/rules/naming-convention.ts	`81.86% <100.00%> (ø)`
...ript-estree/src/create-program/createSourceFile.ts	`90.00% <100.00%> (+12.22%)`	⬆️
... and 2 more

JoshuaKGoldberg

🔥 Mostly looks fantastic - just requesting changes on the // backwards compatibility check comment.

.husky/pre-commit

packages/typescript-estree/src/parser.ts

packages/eslint-plugin/src/rules/consistent-type-exports.ts

packages/utils/src/eslint-utils/getParserServices.ts

packages/typescript-estree/tests/lib/test-utils/expectToHaveParserServices.ts

packages/typescript-estree/src/parser.ts

packages/typescript-estree/src/parser-options.ts

packages/typescript-estree/src/create-program/createSourceFile.ts

JoshuaKGoldberg

No remaining blockers, and a lot of goodness in this PR. Let's get this merged!! 🙌

JoshuaKGoldberg · 2022-12-18T01:23:40Z

Btw @bradzacher the imports all available rule modules unit test failure is fixed in the v6 branch.

bradzacher · 2023-01-23T12:46:20Z

For reference I just did a really quick perf test of the before-and-after for this.

$ TSESTREE_SINGLE_RUN=true node --cpu-prof ./node_modules/.bin/eslint --no-eslintrc --parser="@typescript-eslint/parser" --ext=".ts,.tsx" .

Loading the profiles into https://www.speedscope.app/ I can see that getProgramAndAST is about 100ms faster on our codebase.

It doesn't seem like much, but our codebase is relatively small - ~2.6k files for ~207k SLOC in total. For a larger code base I'd expect more of an impact, though it'll likely still be pretty small overall.

For reference the CPU profiles: PROFILES.zip

bradzacher added the breaking change This change will require a new major version to be released label Nov 23, 2022

bradzacher added this to the 6.0.0 milestone Nov 23, 2022

bradzacher force-pushed the remove-partial-type-info-parse branch from fc7e58f to d82bec2 Compare November 23, 2022 06:05

bradzacher force-pushed the v6 branch from e649ea0 to 36dc7ce Compare November 23, 2022 06:13

bradzacher linked an issue Nov 23, 2022 that may be closed by this pull request

Enhancement: Remove 2 year old backwards compatibility check from getParserServices #5508

Closed

4 tasks

feat: remove partial type-information program

5ab70f1

bradzacher force-pushed the remove-partial-type-info-parse branch from f66f1e1 to 5ab70f1 Compare November 30, 2022 06:54

fix docs test to handle naming-convention weirdness

3182b1c

bradzacher marked this pull request as ready for review December 2, 2022 02:14

bradzacher requested a review from JoshuaKGoldberg December 2, 2022 04:10

JoshuaKGoldberg requested changes Dec 2, 2022

View reviewed changes

JoshuaKGoldberg added the awaiting response Issues waiting for a reply from the OP or another party label Dec 2, 2022

bradzacher added 4 commits December 15, 2022 10:08

Merge branch 'v6' into remove-partial-type-info-parse

7c9d865

review comments

a984658

fuck

932fbc3

silence useless lint logs

2a80442

bradzacher removed the awaiting response Issues waiting for a reply from the OP or another party label Dec 15, 2022

bradzacher requested a review from JoshuaKGoldberg December 15, 2022 01:44

JoshuaKGoldberg approved these changes Dec 15, 2022

View reviewed changes

This was referenced Dec 24, 2022

feat(typescript-estree): allow erroring on TypeScript syntactic diagn… #6271

Closed

Parsing: strictly enforce the produced AST matches the spec and enforce most "error recovery" parsing errors #1852

Closed

Merge branch 'v6' into remove-partial-type-info-parse

526c1d1

bradzacher merged commit 7fc062a into v6 Jan 23, 2023

bradzacher deleted the remove-partial-type-info-parse branch January 23, 2023 08:03

github-actions bot locked as resolved and limited conversation to collaborators Jan 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: remove partial type-information program #6066

feat: remove partial type-information program #6066

bradzacher commented Nov 23, 2022

typescript-eslint bot commented Nov 23, 2022

nx-cloud bot commented Nov 23, 2022 •

edited

Loading

codecov bot commented Dec 2, 2022 •

edited

Loading

JoshuaKGoldberg left a comment

JoshuaKGoldberg left a comment

JoshuaKGoldberg commented Dec 18, 2022

bradzacher commented Jan 23, 2023

feat: remove partial type-information program #6066

feat: remove partial type-information program #6066

Conversation

bradzacher commented Nov 23, 2022

PR Checklist

Overview

typescript-eslint bot commented Nov 23, 2022

nx-cloud bot commented Nov 23, 2022 • edited Loading

☁️ Nx Cloud Report

codecov bot commented Dec 2, 2022 • edited Loading

Codecov Report

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

JoshuaKGoldberg commented Dec 18, 2022

bradzacher commented Jan 23, 2023

nx-cloud bot commented Nov 23, 2022 •

edited

Loading

codecov bot commented Dec 2, 2022 •

edited

Loading