-
-
Notifications
You must be signed in to change notification settings - Fork 662
Add linting to validate types specified via twoslash (//=>) inside JSDoc codeblocks
#1309
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add linting to validate types specified via twoslash (//=>) inside JSDoc codeblocks
#1309
Conversation
1099024 to
95ce69a
Compare
b0ff62d to
f5a80f8
Compare
82f96ec to
0289f95
Compare
0289f95 to
71dde37
Compare
71dde37 to
218534f
Compare
| type OctalInteger = Integer<0o10>; | ||
| //=> 0o10 | ||
| //=> 8 | ||
| type BinaryInteger = Integer<0b10>; | ||
| //=> 0b10 | ||
| //=> 2 | ||
| type HexadecimalInteger = Integer<0x10>; | ||
| //=> 0x10 | ||
| //=> 16 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a short code comment on why |
source/int-closed-range.d.ts
Outdated
| type Age = IntClosedRange<0, 120>; | ||
| //=> 0 | 1 | 2 | ... | 119 | 120 | ||
| type Age = IntClosedRange<0, 20>; | ||
| //=> 0 | 1 | 20 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12 | 13 | 14 | 15 | 16 | 17 | 18 | 19 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not ideal, and I bet users will wonder about this and open issues.
Some ideas:
- Sort the output
- Some kind of code comment to disable linting for the code block or certain lines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, let me look into this then, I had implemented a sorting logic earlier, but it was fairly complicated, so I removed it. I’ll try to add it back with proper tests.
ac41e8e to
480f7f9
Compare
Umm...not sure if a code comment is really necessary here, since |
Up to you. Not important. Just thought something like:
|
3daf895 to
a5a6a22
Compare
c767fae to
8f5e261
Compare
8f5e261 to
5265b61
Compare
|
Looks great! Really nice to be able to enforce this. Merry Xmas! |
|
Merry Christmas and happy holidays! |
| type E = Subtract<PositiveInfinity, 9999>; | ||
| //=> PositiveInfinity | ||
| //=> Infinity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice improvements. Interestingly it seems like there is no Infinity type. Only a detail that might sound misleading.
Reference:
Lines 8 to 19 in fe88ad8
| /** | |
| Matches the hidden `Infinity` type. | |
| Please upvote [this issue](https://github.com/microsoft/TypeScript/issues/32277) if you want to have this type as a built-in in TypeScript. | |
| @see {@link NegativeInfinity} | |
| @category Numeric | |
| */ | |
| // See https://github.com/microsoft/TypeScript/issues/31752 | |
| // eslint-disable-next-line no-loss-of-precision | |
| export type PositiveInfinity = 1e999; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mrazauskas Yeah, but I think showing PositiveInfinity is not great either because the quickinfo tooltip doesn't show that.
So, I think neither option is perfect, I just went with the one that actually shows up in the tooltip. Not sure why quickinfo decides to show Infinity when it doesn’t exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for clarifying. Now I think this could be reported as bug to TypeScript. Only a detail, but it is rather odd to see quickinfo with types that do not exist. Feels like the internals are leaking here. Seems like this should be flag as a limitation of TypeScript.



This PR validates the types specified via twoslash arrow comments (
//=>) inside JSDoc codeblocks using thegetQuickInfoAtPositionAPI, with automatic fixes for incorrect types.It also improves the DX, since we no longer have to manually write the twoslash types, we can simply start the twoslash comment and rely on autofix to fill in the type.
twoslash-autofix-demo.mov
Surprisingly, there were only a few types that were incorrect:
Should be
booleaninstead offalse.type-fest/source/all-extend.d.ts
Lines 87 to 89 in b3ecd07
Should be
trueinstead ofunknown.Return type is not
number.type-fest/source/is-literal.d.ts
Lines 244 to 245 in b3ecd07
The resulting type is
Infinityand notPositiveInfinityorNegativeInfinity.Working:
Twoslash arrow comments (
//=>) inside JSDoc codeblocks are identified, and the first position on the previous line where quickinfo is available is retrieved using thegetQuickInfoAtPositionAPI.The type portion is extracted from the retrieved quickinfo. For example, if the quickinfo is
type T1 = string | number;, the extracted type isstring | number.The extracted type is then normalized to match our linting style. This includes converting double-quoted string literals to single-quoted ones and replacing four-space indentation with tabs.
If the normalized type does not match the specified twoslash comment, an autofixable lint error is reported at the appropriate location.
Notes:
Adds support for overwriting default compiler options, like
type-fest/source/all-extend.d.ts
Lines 80 to 98 in 71dde37
This is similar to how we'd do it in TS playground.
Catches formatting issues like use of spaces instead of tabs, use of double quotes etc.
For object types, type annotations were previously specified inconsistently, sometimes on a single line and sometimes across multiple lines. This PR makes this behaviour deterministic. If the quickinfo type is within a defined length threshold, it must be written in a single line, otherwise, it must be written just as it appears in the quickinfo.
Twoslash arrow comments (
//=>) should now only be used to specify types. All non-type usages (such as error comments) have been converted to plain comments.Future improvements:
The order of unions in quickinfo is sometimes messed up, which leads to changes like these:I'll look into this separately if this can be improved.Fixed by #1320.