Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix(eslint-plugin): [prefer-null-coal] fixer w/ mixed logicals (#1326)
  • Loading branch information
dimabory authored and bradzacher committed Dec 16, 2019
1 parent 3923a09 commit f9a9fbf
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 12 deletions.
28 changes: 20 additions & 8 deletions packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts
Expand Up @@ -103,24 +103,36 @@ export default util.createRule<Options, MessageIds>({
util.NullThrowsReasons.MissingToken('operator', node.type),
);

function* fix(
fixer: TSESLint.RuleFixer,
): IterableIterator<TSESLint.RuleFix> {
if (node.parent && util.isLogicalOrOperator(node.parent)) {
// '&&' and '??' operations cannot be mixed without parentheses (e.g. a && b ?? c)
if (
node.left.type === AST_NODE_TYPES.LogicalExpression &&
!util.isLogicalOrOperator(node.left.left)
) {
yield fixer.insertTextBefore(node.left.right, '(');
} else {
yield fixer.insertTextBefore(node.left, '(');
}
yield fixer.insertTextAfter(node.right, ')');
}
yield fixer.replaceText(barBarOperator, '??');
}

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, '??');
},
fix,
},
],
} as const)
: {
fix(fixer: TSESLint.RuleFixer): TSESLint.RuleFix {
return fixer.replaceText(barBarOperator, '??');
},
};
: { fix };

context.report({
node: barBarOperator,
Expand Down
14 changes: 12 additions & 2 deletions packages/eslint-plugin/src/util/astUtils.ts
@@ -1,7 +1,7 @@
import {
TSESTree,
AST_TOKEN_TYPES,
AST_NODE_TYPES,
AST_TOKEN_TYPES,
TSESTree,
} from '@typescript-eslint/experimental-utils';

const LINEBREAK_MATCHER = /\r\n|[\r\n\u2028\u2029]/;
Expand Down Expand Up @@ -42,6 +42,15 @@ function isOptionalOptionalChain(
);
}

/**
* Returns true if and only if the node represents logical OR
*/
function isLogicalOrOperator(node: TSESTree.Node): boolean {
return (
node.type === AST_NODE_TYPES.LogicalExpression && node.operator === '||'
);
}

/**
* Determines whether two adjacent tokens are on the same line
*/
Expand All @@ -59,5 +68,6 @@ export {
isOptionalChainPunctuator,
isOptionalOptionalChain,
isTokenOnSameLine,
isLogicalOrOperator,
LINEBREAK_MATCHER,
};
Expand Up @@ -317,7 +317,7 @@ declare const a: ${type} | ${nullish};
declare const b: ${type} | ${nullish};
declare const c: ${type} | ${nullish};
declare const d: ${type} | ${nullish};
a ?? b || c && d;
(a ?? b) || c && d;
`.trimRight(),
},
],
Expand Down Expand Up @@ -367,7 +367,7 @@ declare const a: ${type} | ${nullish};
declare const b: ${type} | ${nullish};
declare const c: ${type} | ${nullish};
declare const d: ${type} | ${nullish};
a && b ?? c || d;
a && (b ?? c) || d;
`.trimRight(),
},
],
Expand Down Expand Up @@ -463,5 +463,30 @@ x ?? 'foo';
},
],
},

// https://github.com/typescript-eslint/typescript-eslint/issues/1290
...nullishTypeInvalidTest((nullish, type) => ({
code: `
declare const a: ${type} | ${nullish};
declare const b: ${type};
declare const c: ${type};
a || b || c;
`.trimRight(),
output: `
declare const a: ${type} | ${nullish};
declare const b: ${type};
declare const c: ${type};
(a ?? b) || c;
`.trimRight(),
errors: [
{
messageId: 'preferNullish',
line: 5,
column: 3,
endLine: 5,
endColumn: 5,
},
],
})),
],
});

0 comments on commit f9a9fbf

Please sign in to comment.