chore(utils): use `Extract` generic for `ast-utils`' `predicates`' helper functions #4545

MichaelDeBoey · 2022-02-12T15:32:03Z

Thanks to @farskid & @Beraliv for the TILs! 💪

https://twitter.com/farzad_yz/status/1492127144423694338
https://twitter.com/beraliv/status/1425767234878812165

nx-cloud · 2022-02-12T15:32:05Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 8def652. 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.

typescript-eslint · 2022-02-12T15:32:07Z

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 visitors per day.

netlify · 2022-02-12T15:32:10Z

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	`8def652`
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/630381704f3d8e0009407aa6
😎 Deploy Preview	https://deploy-preview-4545--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

JoshuaKGoldberg

Extract is indeed neat, but this is a few extra characters per line for... what is the benefit here?

Happy to approve if there's some reason for it that can be shown. 🙂

MichaelDeBoey · 2022-02-24T00:36:28Z

what is the benefit here?

@JoshuaKGoldberg As seen in the mentioned tweets, this wil result in a nicer type & cleaner error messages

type A = { name: 'A', lowercase: 'a' };
type B = { name: 'B', lowercase: 'b' };

type AOrB = A | B;

type AWithExact = Exact<AOrB, { name: 'A' }>;
// type A

type AWithoutExact = AOrB & { name: 'A' };
// { name: 'A', lowercase: 'a' } & { name: 'A' } | { name: 'B', lowercase: 'b' } & { name: 'A' }
// which results in { name: 'A', lowercase: 'a' } & { name: 'A' } | never
// or just { name: 'A', lowercase: 'a' } & { name: 'A' }
// which is the same as { name: 'A', lowercase: 'a' }
// which is the same as type A

JoshuaKGoldberg

Cool, thanks - that makes sense!

Maybe we should include a lint rule asking for this preference?

MichaelDeBoey · 2022-02-24T00:39:26Z

Maybe we should include a lint rule asking for this preference?

@JoshuaKGoldberg I think this can indeed be a great addition to this package 👍

MichaelDeBoey · 2022-02-24T00:40:47Z

It seems like type checks are failing, but I can't figure out why exactly tbh.

@farskid @Beraliv Do one of you guys have any pointers for me please?

CC/ @bradzacher

bradzacher · 2022-02-24T23:19:57Z

  Overload 1 of 2, '(predicate: (value: ObjectEntry<Extract<BooleanToken, { type: TokenType; }> | Extract<BlockComment, { type: TokenType; }> | Extract<LineComment, { ...; }> | ... 9 more ... | Extract<...>>, index: number, array: ObjectEntry<...>[]) => value is ObjectEntry<...>, thisArg?: any): this is ObjectEntry<...>[]', gave the following error.
    Argument of type '([key, value]: ObjectEntry<Extract<BooleanToken, { type: TokenType; }> | Extract<BlockComment, { type: TokenType; }> | Extract<LineComment, { ...; }> | ... 9 more ... | Extract<...>>) => boolean' is not assignable to parameter of type '(value: ObjectEntry<Extract<BooleanToken, { type: TokenType; }> | Extract<BlockComment, { type: TokenType; }> | Extract<LineComment, { ...; }> | ... 9 more ... | Extract<...>>, index: number, array: ObjectEntry<...>[]) => value is ObjectEntry<...>'.
      Signature '([key, value]: ObjectEntry<Extract<BooleanToken, { type: TokenType; }> | Extract<BlockComment, { type: TokenType; }> | Extract<LineComment, { ...; }> | ... 9 more ... | Extract<...>>): boolean' must be a type predicate.
  Overload 2 of 2, '(predicate: (value: ObjectEntry<Extract<BooleanToken, { type: TokenType; }> | Extract<BlockComment, { type: TokenType; }> | Extract<LineComment, { ...; }> | ... 9 more ... | Extract<...>>, index: number, array: ObjectEntry<...>[]) => unknown, thisArg?: any): boolean', gave the following error.
    Argument of type '([key, value]: ObjectEntry<Extract<BooleanToken, { type: TokenType; }> | Extract<BlockComment, { type: TokenType; }> | Extract<LineComment, { ...; }> | ... 9 more ... | Extract<...>>) => boolean' is not assignable to parameter of type '(value: ObjectEntry<Extract<BooleanToken, { type: TokenType; }> | Extract<BlockComment, { type: TokenType; }> | Extract<LineComment, { ...; }> | ... 9 more ... | Extract<...>>, index: number, array: ObjectEntry<...>[]) => unknown'.
      Types of parameters '__0' and 'value' are incompatible.
        Type 'ObjectEntry<Extract<import("/home/runner/work/typescript-eslint/typescript-eslint/packages/types/dist/ast-spec").BooleanToken, { type: TokenType; }> | Extract<import("/home/runner/work/typescript-eslint/typescript-eslint/packages/types/dist/ast-spec").BlockComment, { ...; }> | ... 10 more ... | Extract<...>>' is not assignable to type 'ObjectEntry<Extract<import("/home/runner/work/typescript-eslint/typescript-eslint/packages/types/dist/ast-spec").BooleanToken, { type: TokenType; }> | Extract<import("/home/runner/work/typescript-eslint/typescript-eslint/packages/types/dist/ast-spec").BlockComment, { ...; }> | ... 10 more ... | Extract<...>>'. Two different types with this name exist, but they are unrelated.
          Type at position 1 in source is not compatible with type at position 1 in target.
            Type 'Extract<BooleanToken, { type: TokenType; }>[keyof Extract<BooleanToken, { type: TokenType; }> & keyof Extract<...> & ... 10 more ... & keyof Extract<...>] | ... 11 more ... | Extract<...>[keyof Extract<...> & ... 11 more ... & keyof Extract<...>]' is not assignable to type 'Extract<BooleanToken, { type: TokenType; }>[keyof Extract<BooleanToken, { type: TokenType; }> & keyof Extract<...> & ... 10 more ... & keyof Extract<...>] & ... 11 more ... & Extract<...>[keyof Extract<...> & ... 11 more ... & keyof Extract<...>]'.
              Type 'Extract<BooleanToken, { type: TokenType; }>[keyof Extract<BooleanToken, { type: TokenType; }> & keyof Extract<...> & ... 10 more ... & keyof Extract<...>]' is not assignable to type 'Extract<BooleanToken, { type: TokenType; }>[keyof Extract<BooleanToken, { type: TokenType; }> & keyof Extract<...> & ... 10 more ... & keyof Extract<...>] & ... 11 more ... & Extract<...>[keyof Extract<...> & ... 11 more ... & keyof Extract<...>]'.
                Type 'Extract<BooleanToken, { type: TokenType; }>[string] | Extract<BooleanToken, { type: TokenType; }>[number] | Extract<...>[symbol]' is not assignable to type 'Extract<BooleanToken, { type: TokenType; }>[keyof Extract<BooleanToken, { type: TokenType; }> & keyof Extract<...> & ... 10 more ... & keyof Extract<...>] & ... 11 more ... & Extract<...>[keyof Extract<...> & ... 11 more ... & keyof Extract<...>]'.
                  Type 'Extract<BooleanToken, { type: TokenType; }>[string]' is not assignable to type 'Extract<BooleanToken, { type: TokenType; }>[keyof Extract<BooleanToken, { type: TokenType; }> & keyof Extract<...> & ... 10 more ... & keyof Extract<...>] & ... 11 more ... & Extract<...>[keyof Extract<...> & ... 11 more ... & keyof Extract<...>]'.
                    Type 'Extract<BooleanToken, { type: TokenType; }>[string]' is not assignable to type 'Extract<BooleanToken, { type: TokenType; }>[keyof Extract<BooleanToken, { type: TokenType; }> & keyof Extract<...> & ... 10 more ... & keyof Extract<...>]'.
                      Type 'string' is not assignable to type 'keyof Extract<BooleanToken, { type: TokenType; }> & keyof Extract<BlockComment, { type: TokenType; }> & ... 10 more ... & keyof Extract<...>'.
                        Type 'string' is not assignable to type 'keyof Extract<BooleanToken, { type: TokenType; }>'.
                          Type 'string' is not assignable to type 'keyof BooleanToken'.
                            Type 'Extract<BooleanToken, { type: TokenType; }>[keyof Extract<BooleanToken, { type: TokenType; }> & keyof Extract<...> & ... 10 more ... & keyof Extract<...>]' is not assignable to type 'Extract<BlockComment, { type: TokenType; }>[keyof Extract<BooleanToken, { type: TokenType; }> & keyof Extract<...> & ... 10 more ... & keyof Extract<...>]'.
                              Type 'Extract<BooleanToken, { type: TokenType; }>' is not assignable to type 'Extract<BlockComment, { type: TokenType; }>'.
                                Type '{ type: TokenType; } & BooleanToken' is not assignable to type 'Extract<BlockComment, { type: TokenType; }>'.
Error: src/ast-utils/helpers.ts(57,37): error TS2536: Type 'keyof Extract<BooleanToken, { type: TokenType; }> & keyof Extract<BlockComment, { type: TokenT

that is a truly gnarly error message lol

bradzacher · 2022-02-24T23:38:05Z

From the error...

Type 'ObjectEntry<Extract<import("/home/runner/work/typescript-eslint/typescript-eslint/packages/types/dist/ast-spec").BooleanToken, { type: TokenType; }> | Extract<import("/home/runner/work/typescript-eslint/typescript-eslint/packages/types/dist/ast-spec").BlockComment, { ...; }> | ... 10 more ... | Extract<...>>' is not assignable to type 'ObjectEntry<Extract<import("/home/runner/work/typescript-eslint/typescript-eslint/packages/types/dist/ast-spec").BooleanToken, { type: TokenType; }> | Extract<import("/home/runner/work/typescript-eslint/typescript-eslint/packages/types/dist/ast-spec").BlockComment, { ...; }> | ... 10 more ... | Extract<...>>'. Two different types with this name exist, but they are unrelated.

is TS doing something weird here? why are there duplicate named types it's referencing?

MichaelDeBoey · 2022-02-25T00:03:29Z

@bradzacher I've reverted all the changes except for isNodeOfType (for now), so we'll have less possible errors

bradzacher · 2022-03-01T04:22:12Z

Just one error now which is nowhere near as gnarly!

src/ast-utils/helpers.ts(6,14): error TS4023: Exported variable 'isNodeOfType' has or is using name 'TSIntrinsicKeyword' from external module "/home/runner/work/typescript-eslint/typescript-eslint/packages/types/dist/ast-spec" but cannot be named.

MichaelDeBoey · 2022-03-05T18:19:26Z

@bradzacher Do you happen to know how I could fix it?
I really have no clue tbh 😕

bradzacher · 2022-03-07T01:03:28Z

From what I can tell - the issue is that TSIntrinsicKeyword is not exported from the ast-spec package, so it's not a publically available type - so TS doesn't like it.

Make the following two changes:

Rename this file packages/ast-spec/src/type/TSIntrinsicType/spec.ts to packages/ast-spec/src/type/TSIntrinsicKeyword/spec.ts
make sure it's exported from packages/ast-spec/src/type/spec.ts (i.e. export * from './TSIntrinsicKeyword/spec')

A TODO here would be for us to add a test to ensure that all types named within our Node type are exported from ast-spec. We would probably have to use the TS compiler API to properly verify this.

MichaelDeBoey · 2022-03-07T03:05:40Z

@bradzacher Those changes seem to be fixing the issues indeed
https://github.com/typescript-eslint/typescript-eslint/runs/5442617641

I'll update all other helper functions again, so we can see if that was the only problem or not.

A TODO here would be for us to add a test to ensure that all types named within our Node type are exported from ast-spec. We would probably have to use the TS compiler API to properly verify this.

I have the feeling that with the changes in this PR in place, CI will automatically fail again whenever we don't export everything correctly.
But a specific test for it won't hurt & will probably point into the right direction sooner.

bradzacher

lgtm - just one comment

packages/eslint-plugin/src/rules/sort-type-union-intersection-members.ts

…lper functions

MichaelDeBoey force-pushed the patch-52 branch from f9ad3e2 to a94e466 Compare February 13, 2022 01:11

bradzacher added the refactor PRs that refactor code only label Feb 14, 2022

JoshuaKGoldberg requested changes Feb 23, 2022

View reviewed changes

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

MichaelDeBoey force-pushed the patch-52 branch from a94e466 to 73f74b2 Compare February 24, 2022 00:26

JoshuaKGoldberg previously approved these changes Feb 24, 2022

View reviewed changes

MichaelDeBoey force-pushed the patch-52 branch from 73f74b2 to 3a4e29e Compare February 24, 2022 09:20

JoshuaKGoldberg dismissed their stale review via 3a4e29e February 24, 2022 15:42

MichaelDeBoey force-pushed the patch-52 branch from 3a4e29e to 1879833 Compare February 24, 2022 17:59

MichaelDeBoey force-pushed the patch-52 branch from 1879833 to 9d81ce3 Compare February 24, 2022 23:36

MichaelDeBoey force-pushed the patch-52 branch from 9d81ce3 to d33209a Compare February 25, 2022 00:02

MichaelDeBoey force-pushed the patch-52 branch from d33209a to 66fe5ba Compare March 5, 2022 18:18

MichaelDeBoey force-pushed the patch-52 branch from 66fe5ba to 134f9d1 Compare March 7, 2022 02:52

MichaelDeBoey force-pushed the patch-52 branch from 134f9d1 to 614f5a2 Compare March 7, 2022 03:10

MichaelDeBoey requested a review from JoshuaKGoldberg March 7, 2022 03:13

MichaelDeBoey force-pushed the patch-52 branch 2 times, most recently from a9ea0b6 to 928a2d8 Compare March 7, 2022 03:20

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

bradzacher requested changes Aug 21, 2022

View reviewed changes

packages/eslint-plugin/src/rules/sort-type-union-intersection-members.ts Outdated Show resolved Hide resolved

bradzacher added the awaiting response Issues waiting for a reply from the OP or another party label Aug 21, 2022

MichaelDeBoey added 10 commits August 22, 2022 15:14

chore(utils): use Extract generic for ast-utils' predicates' he…

0dc5052

…lper functions

chore: enable for isNodeOfTypes

04da286

chore: move TS keywords to Node

8921bd4

chore: enable for isNodeOfTypeWithConditions

98b4df0

chore: enable for isTokenOfTypeWithConditions

328a82f

chore: enable for isNotTokenOfTypeWithConditions

52db9d8

fix: fix Conditions type for isNodeOfTypeWithConditions

d3f4b5d

fix: fix entries.every condition in isNodeOfTypeWithConditions

5b52f9b

chore: extract ExtractedNode & ExtractedToken generics

3da4a94

fix: fix getGroup

8def652

MichaelDeBoey force-pushed the patch-52 branch from 9f820a9 to 8def652 Compare August 22, 2022 13:15

MichaelDeBoey requested review from bradzacher, Beraliv and Josh-Cena and removed request for JoshuaKGoldberg, bradzacher, Beraliv and Josh-Cena August 22, 2022 14:07

bradzacher merged commit 4483a4b into typescript-eslint:main Aug 25, 2022

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

MichaelDeBoey deleted the patch-52 branch August 25, 2022 20:00

github-actions bot locked as resolved and limited conversation to collaborators Sep 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(utils): use `Extract` generic for `ast-utils`' `predicates`' helper functions #4545

chore(utils): use `Extract` generic for `ast-utils`' `predicates`' helper functions #4545

MichaelDeBoey commented Feb 12, 2022 •

edited

Loading

nx-cloud bot commented Feb 12, 2022 •

edited

Loading

typescript-eslint bot commented Feb 12, 2022

netlify bot commented Feb 12, 2022 •

edited

Loading

JoshuaKGoldberg left a comment •

edited

Loading

MichaelDeBoey commented Feb 24, 2022

JoshuaKGoldberg left a comment

MichaelDeBoey commented Feb 24, 2022

MichaelDeBoey commented Feb 24, 2022

bradzacher commented Feb 24, 2022

bradzacher commented Feb 24, 2022

MichaelDeBoey commented Feb 25, 2022

bradzacher commented Mar 1, 2022

MichaelDeBoey commented Mar 5, 2022

bradzacher commented Mar 7, 2022 •

edited

Loading

MichaelDeBoey commented Mar 7, 2022

bradzacher left a comment

chore(utils): use Extract generic for ast-utils' predicates' helper functions #4545

chore(utils): use Extract generic for ast-utils' predicates' helper functions #4545

Conversation

MichaelDeBoey commented Feb 12, 2022 • edited Loading

nx-cloud bot commented Feb 12, 2022 • edited Loading

☁️ Nx Cloud Report

typescript-eslint bot commented Feb 12, 2022

netlify bot commented Feb 12, 2022 • edited Loading

✅ Deploy Preview for typescript-eslint ready!

JoshuaKGoldberg left a comment • edited Loading

Choose a reason for hiding this comment

MichaelDeBoey commented Feb 24, 2022

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

MichaelDeBoey commented Feb 24, 2022

MichaelDeBoey commented Feb 24, 2022

bradzacher commented Feb 24, 2022

bradzacher commented Feb 24, 2022

MichaelDeBoey commented Feb 25, 2022

bradzacher commented Mar 1, 2022

MichaelDeBoey commented Mar 5, 2022

bradzacher commented Mar 7, 2022 • edited Loading

MichaelDeBoey commented Mar 7, 2022

bradzacher left a comment

Choose a reason for hiding this comment

chore(utils): use `Extract` generic for `ast-utils`' `predicates`' helper functions #4545

chore(utils): use `Extract` generic for `ast-utils`' `predicates`' helper functions #4545

MichaelDeBoey commented Feb 12, 2022 •

edited

Loading

nx-cloud bot commented Feb 12, 2022 •

edited

Loading

netlify bot commented Feb 12, 2022 •

edited

Loading

JoshuaKGoldberg left a comment •

edited

Loading

bradzacher commented Mar 7, 2022 •

edited

Loading