typescript-eslint/typescript-eslintPublic

NotificationsYou must be signed in to change notification settings
Fork2.8k
Star16k

tests(ast-spec): make`PunctuatorTokenToText` more type-safe#3529

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

bradzacher merged 1 commit intotypescript-eslint:masterfromMichaelDeBoey:patch-16

Jun 21, 2021

Merged

tests(ast-spec): make`PunctuatorTokenToText` more type-safe#3529

bradzacher merged 1 commit intotypescript-eslint:masterfromMichaelDeBoey:patch-16

Jun 21, 2021

Conversation

Copy link

Contributor

MichaelDeBoey commentedJun 14, 2021•
edited
Loading

Follow-up of#3496

We should somehow test that all possible values ofPunctuationSyntaxKind are definitely included.
If not, this should error out somehow.

I first tried doing

exportconstPunctuatorTokenToText:Record<PunctuationSyntaxKind,string>={// ...}

and this errored out (as expected) becauseSyntaxKind.BacktickToken isn't includes, but is included inPunctuationSyntaxKind.
Not including the type makes it also of typestring again, which will fail#3461 &#3462.

This however made it a value instead of a type and had the implication thatSyntaxKind.BarBarEqualsToken,SyntaxKind.AmpersandAmpersandEqualsToken &SyntaxKind.QuestionQuestionEqualsToken couldn't be added as they'll only be added toPunctuationSyntaxKind in TS 4.4 (they're currently included onmain).

In TS 4.4 the newSyntaxKind.HashToken should also be included, so I think it would be nice to have it somehow error out if we don't include that when updating our TS version.

@bradzacher @JamesHenry Any idea how I can make this error out when some values aren't included or how I should write a test that can fail when a new value isn't included here?

Copy link

Contributor

typescript-eslintbot commentedJun 14, 2021

Thanks for the PR,@MichaelDeBoey!

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. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitorsper day.

Copy link

nx-cloudbot commentedJun 14, 2021•
edited
Loading

Nx Cloud Report

CI ran the following commands for commit5ef6efb. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status	Command
	`nx run-many --target=build --all --parallel`
	`nx run-many --target=typecheck --all --parallel`

Sent with 💌 fromNxCloud.

MichaelDeBoey force-pushed thepatch-16 branch from6969166 tod5f0734Compare

June 17, 2021 08:24

Copy link

ContributorAuthor

MichaelDeBoey commentedJun 20, 2021

@bradzacher @JamesHenry Do you have any idea how I can make this error out as explained in the above message?

Would love to work on this one further, but don't see a solution right away.

MichaelDeBoey force-pushed thepatch-16 branch fromd5f0734 to6852617Compare

June 20, 2021 14:50

bradzacher added the repo maintenancethings to do with maintenance of the repo, and not with code/docs label

Jun 20, 2021

Copy link

Member

bradzacher commentedJun 20, 2021

can you do something similar to what I did in this?

typescript-eslint/packages/ast-spec/tests/ast-node-types.test.ts

Lines 1 to 16 ine2e76a7

	importtype{AST_NODE_TYPES}from'../src/ast-node-types';
	importtype{Node}from'../src/unions/Node';

	typeGetKeys<TextendsAST_NODE_TYPES>=keyofExtract<Node,{type:T}>;

	typeAllKeys={
	readonly[TinAST_NODE_TYPES]:GetKeys<T>;
	};

	typeTakesString<TextendsRecord<string,string>>=T;

	//@ts-expect-error: purposely unused
	type_Test=
	// forcing the test onto a new line so it isn't covered by the expect error
	// If there are any enum members that don't have a corresponding TSESTree.Node, then this line will error with "Type 'string \| number \| symbol' is not assignable to type 'string'."
	TakesString<AllKeys>\|void;

Copy link

codecovbot commentedJun 20, 2021•
edited
Loading

Codecov Report

Merging#3529 (e2e76a7) intomaster (44f9c07) willdecrease coverage by0.00%.
The diff coverage isn/a.

❗ Current heade2e76a7 differs from pull request most recent head5ef6efb. Consider uploading reports for the commit5ef6efb to get more accurate results

@@            Coverage Diff             @@##           master    #3529      +/-   ##==========================================- Coverage   92.64%   92.63%   -0.01%==========================================  Files         326      325       -1       Lines       11253    11236      -17       Branches     3171     3168       -3     ==========================================- Hits        10425    10409      -16+ Misses        368      367       -1  Partials      460      460

Flag	Coverage Δ
unittest	`92.63% <0.00%> (-0.01%)`	⬇️

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

Impacted Files	Coverage Δ
...ackages/experimental-utils/src/ts-eslint/Linter.ts	`100.00% <0.00%> (ø)`
...ages/eslint-plugin/src/rules/prefer-regexp-exec.ts	`100.00% <0.00%> (ø)`
...ges/experimental-utils/src/ts-eslint/RuleTester.ts	`100.00% <0.00%> (ø)`
packages/eslint-plugin/src/util/escapeRegExp.ts

Copy link

ContributorAuthor

MichaelDeBoey commentedJun 20, 2021

@bradzacher I don't see directly how I can use the mentioned test in my use-case tbh. 🤔

A bit more context (or even the test if you like) would be welcome.

Copy link

Member

bradzacher commentedJun 20, 2021

//@ts-expect-error: purposely unusedtype_AllKeys={// no error as all PunctuationSyntaxKind are coveredreadonly[TinPunctuationSyntaxKind]:PunctuatorTokenToText[T];};// example showing if we miss one of the PunctuationSyntaxKindinterfaceAccidentallyIncomplete{[SyntaxKind.CaretEqualsToken]:'^=';}//@ts-expect-error: purposely unusedtype_AllKeysError={readonly[TinPunctuationSyntaxKind]:AccidentallyIncomplete[T];//                                       ^^^^^^^^^^^^^^^^^^^^^^^^^ error: Type 'T' cannot be used to index type 'AccidentallyIncomplete'.}