Skip to content

Commit

Permalink
feat(eslint-plugin): [prefer-null-coal] opt for suggestion fixer (#1272)
Browse files Browse the repository at this point in the history
  • Loading branch information
bradzacher committed Nov 30, 2019
1 parent dc73510 commit f84eb96
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 25 deletions.
12 changes: 10 additions & 2 deletions packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.md
Expand Up @@ -46,13 +46,15 @@ type Options = [
{
ignoreConditionalTests?: boolean;
ignoreMixedLogicalExpressions?: boolean;
forceSuggestionFixer?: boolean;
},
];

const defaultOptions = [
{
ignoreConditionalTests: true,
ignoreMixedLogicalExpressions: true;
ignoreMixedLogicalExpressions: true,
forceSuggestionFixer: false,
},
];
```
Expand Down Expand Up @@ -129,7 +131,13 @@ a ?? (b && c) ?? d;
a ?? (b && c && d);
```

**_NOTE:_** Errors for this specific case will be presented as suggestions, 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.
**_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.

### forceSuggestionFixer

Setting this option to `true` will cause the rule to use ESLint's "suggested fix" mode for all fixes. _This option is provided as to aid in transitioning your codebase onto this rule_.

Suggestion fixes cannot be automatically applied via the `--fix` CLI command, but can be _manually_ chosen to be applied one at a time via an IDE or similar. This makes it safe to run autofixers on an existing codebase without worrying about potential runtime behaviour changes from this rule's fixer.

## When Not To Use It

Expand Down
64 changes: 41 additions & 23 deletions packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts
Expand Up @@ -11,6 +11,7 @@ export type Options = [
{
ignoreConditionalTests?: boolean;
ignoreMixedLogicalExpressions?: boolean;
forceSuggestionFixer?: boolean;
},
];
export type MessageIds = 'preferNullish';
Expand Down Expand Up @@ -41,6 +42,9 @@ export default util.createRule<Options, MessageIds>({
ignoreMixedLogicalExpressions: {
type: 'boolean',
},
forceSuggestionFixer: {
type: 'boolean',
},
},
additionalProperties: false,
},
Expand All @@ -50,9 +54,19 @@ export default util.createRule<Options, MessageIds>({
{
ignoreConditionalTests: true,
ignoreMixedLogicalExpressions: true,
forceSuggestionFixer: false,
},
],
create(context, [{ ignoreConditionalTests, ignoreMixedLogicalExpressions }]) {
create(
context,
[
{
ignoreConditionalTests,
ignoreMixedLogicalExpressions,
forceSuggestionFixer,
},
],
) {
const parserServices = util.getParserServices(context);
const sourceCode = context.getSourceCode();
const checker = parserServices.program.getTypeChecker();
Expand All @@ -79,30 +93,34 @@ export default util.createRule<Options, MessageIds>({
return;
}

const barBarOperator = sourceCode.getTokenAfter(
node.left,
token =>
token.type === AST_TOKEN_TYPES.Punctuator &&
token.value === node.operator,
)!; // there _must_ be an operator

const fixer = isMixedLogical
? // suggestion instead for cases where we aren't sure if the fixer is completely safe
({
suggest: [
{
messageId: 'preferNullish',
fix(fixer: TSESLint.RuleFixer): TSESLint.RuleFix {
return fixer.replaceText(barBarOperator, '??');
const barBarOperator = util.nullThrows(
sourceCode.getTokenAfter(
node.left,
token =>
token.type === AST_TOKEN_TYPES.Punctuator &&
token.value === node.operator,
),
util.NullThrowsReasons.MissingToken('operator', node.type),
);

const fixer =
isMixedLogical || forceSuggestionFixer
? // suggestion instead for cases where we aren't sure if the fixer is completely safe
({
suggest: [
{
messageId: 'preferNullish',
fix(fixer: TSESLint.RuleFixer): TSESLint.RuleFix {
return fixer.replaceText(barBarOperator, '??');
},
},
],
} as const)
: {
fix(fixer: TSESLint.RuleFixer): TSESLint.RuleFix {
return fixer.replaceText(barBarOperator, '??');
},
],
} as const)
: {
fix(fixer: TSESLint.RuleFixer): TSESLint.RuleFix {
return fixer.replaceText(barBarOperator, '??');
},
};
};

context.report({
node: barBarOperator,
Expand Down
Expand Up @@ -435,5 +435,33 @@ if (function werid() { return x ?? 'foo' }) {}
},
],
})),

// testing the suggestion fixer option
{
code: `
declare const x: string | null;
x || 'foo';
`.trimRight(),
output: null,
options: [{ forceSuggestionFixer: true }],
errors: [
{
messageId: 'preferNullish',
line: 3,
column: 3,
endLine: 3,
endColumn: 5,
suggestions: [
{
messageId: 'preferNullish',
output: `
declare const x: string | null;
x ?? 'foo';
`.trimRight(),
},
],
},
],
},
],
});

0 comments on commit f84eb96

Please sign in to comment.