Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
feat(eslint-plugin): [prefer-nullish-coalescing] addignorePrimitives option#6487
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
Uh oh!
There was an error while loading.Please reload this page.
Changes fromall commits
b427e6b8e016d309f207a24fb9c57aec9faed7d5256954d626be0360a5760958dd233f25a0852e66ee9b5c4ab383b1e0122e579a6e38a2a97d30fe57907cf540c1839d085f342dff212bea946b6d4aa01357fd59d3f6ee0e34fb0a419a38381ada9853bbfbad1b0347ef2998237477964b6c03f7207e055a18ee3File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -131,6 +131,29 @@ a ?? (b && c && d); | ||
| **_NOTE:_** Errors for this specific case will be presented as suggestions (see below), instead of fixes. This is because it is not always safe to automatically convert `||` to `??` within a mixed logical expression, as we cannot tell the intended precedence of the operator. Note that by design, `??` requires parentheses when used with `&&` or `||` in the same expression. | ||
| ### `ignorePrimitives` | ||
omril1 marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. [Docs] Could you add some examples please? We generally try to -at least for newly added rule options- so users can see exactly how the code looks. ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. I tried to look at the style for examples with options, this rule seems unique in how the examples are presented but decided to keep it the same since I've noticed bradzacher did the same Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Heh yeah we'll eventually clean all these up... eventually. Thanks! | ||
| If you would like to ignore certain primitive types that can be falsy then you may pass an object containing a boolean value for each primitive: | ||
| - `string: true`, ignores `null` or `undefined` unions with `string` (default: false). | ||
| - `number: true`, ignores `null` or `undefined` unions with `number` (default: false). | ||
JoshuaKGoldberg marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| - `bigint: true`, ignores `null` or `undefined` unions with `bigint` (default: false). | ||
| - `boolean: true`, ignores `null` or `undefined` unions with `boolean` (default: false). | ||
JoshuaKGoldberg marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| Incorrect code for `ignorePrimitives: { string: true }`, and correct code for `ignorePrimitives: { string: false }`: | ||
| ```ts | ||
| const foo: string | undefined = 'bar'; | ||
| foo || 'a string'; | ||
| ``` | ||
| Correct code for `ignorePrimitives: { string: true }`: | ||
| ```ts | ||
| const foo: string | undefined = 'bar'; | ||
| foo ?? 'a string'; | ||
| ``` | ||
| ## When Not To Use It | ||
| If you are not using TypeScript 3.7 (or greater), then you will not be able to use this rule, as the operator is not supported. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -7,10 +7,16 @@ import * as util from '../util'; | ||
| export type Options = [ | ||
| { | ||
| allowRuleToRunWithoutStrictNullChecksIKnowWhatIAmDoing?: boolean; | ||
| ignoreConditionalTests?: boolean; | ||
| ignoreMixedLogicalExpressions?: boolean; | ||
| ignorePrimitives?: { | ||
| bigint?: boolean; | ||
| boolean?: boolean; | ||
| number?: boolean; | ||
| string?: boolean; | ||
| }; | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. [Non-actionable] In theory we could allow this to be a ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Good idea, leaving it for now. ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Do I need to open an issue for it or can I reference the same one? Member
| ||
| ignoreTernaryTests?: boolean; | ||
| }, | ||
| ]; | ||
| @@ -44,16 +50,25 @@ export default util.createRule<Options, MessageIds>({ | ||
| { | ||
| type: 'object', | ||
| properties: { | ||
| allowRuleToRunWithoutStrictNullChecksIKnowWhatIAmDoing: { | ||
| type: 'boolean', | ||
| }, | ||
| ignoreConditionalTests: { | ||
| type: 'boolean', | ||
| }, | ||
| ignoreMixedLogicalExpressions: { | ||
| type: 'boolean', | ||
| }, | ||
| ignorePrimitives: { | ||
| type: 'object', | ||
| properties: { | ||
| bigint: { type: 'boolean' }, | ||
| boolean: { type: 'boolean' }, | ||
| number: { type: 'boolean' }, | ||
| string: { type: 'boolean' }, | ||
| }, | ||
| }, | ||
| ignoreTernaryTests: { | ||
| type: 'boolean', | ||
| }, | ||
| }, | ||
| @@ -63,20 +78,27 @@ export default util.createRule<Options, MessageIds>({ | ||
| }, | ||
| defaultOptions: [ | ||
| { | ||
| allowRuleToRunWithoutStrictNullChecksIKnowWhatIAmDoing: false, | ||
| ignoreConditionalTests: true, | ||
| ignoreTernaryTests: true, | ||
| ignoreMixedLogicalExpressions: true, | ||
| ignorePrimitives: { | ||
| bigint: false, | ||
| boolean: false, | ||
| number: false, | ||
| string: false, | ||
| }, | ||
| }, | ||
| ], | ||
| create( | ||
| context, | ||
| [ | ||
| { | ||
| allowRuleToRunWithoutStrictNullChecksIKnowWhatIAmDoing, | ||
| ignoreConditionalTests, | ||
| ignoreMixedLogicalExpressions, | ||
| ignorePrimitives, | ||
| ignoreTernaryTests, | ||
| }, | ||
| ], | ||
| ) { | ||
| @@ -279,6 +301,22 @@ export default util.createRule<Options, MessageIds>({ | ||
| return; | ||
| } | ||
| const ignorableFlags = [ | ||
| ignorePrimitives!.bigint && ts.TypeFlags.BigInt, | ||
| ignorePrimitives!.boolean && ts.TypeFlags.BooleanLiteral, | ||
| ignorePrimitives!.number && ts.TypeFlags.Number, | ||
| ignorePrimitives!.string && ts.TypeFlags.String, | ||
| ] | ||
| .filter((flag): flag is number => flag !== undefined) | ||
| .reduce((previous, flag) => previous | flag, 0); | ||
| if ( | ||
| (type as ts.UnionOrIntersectionType).types.some(t => | ||
| tsutils.isTypeFlagSet(t, ignorableFlags), | ||
| ) | ||
| ) { | ||
| return; | ||
| } | ||
| const barBarOperator = util.nullThrows( | ||
| sourceCode.getTokenAfter( | ||
| node.left, | ||
Uh oh!
There was an error while loading.Please reload this page.