Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(eslint-plugin): [no-unnecessary-boolean-literal-compare] simplify fixer and add support for double negation #6620

Merged
merged 2 commits into from
Mar 21, 2023
Merged

fix(eslint-plugin): [no-unnecessary-boolean-literal-compare] simplify fixer and add support for double negation #6620

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
018b28b
fix: [no-unnecessary-boolean-literal-compare] simplify fixer and add …
armano2 Mar 13, 2023
0c94193
fix: correct comment simplify condition
armano2 Mar 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
65 changes: 29 additions & 36 deletions packages/eslint-plugin/src/rules/no-unnecessary-boolean-literal-compare.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ type Options = [
interface BooleanComparison {
expression: TSESTree.Expression | TSESTree.PrivateIdentifier;
literalBooleanInComparison: boolean;
forTruthy: boolean;
negated: boolean;
}

Expand Down Expand Up @@ -182,7 +181,6 @@ export default util.createRule<Options, MessageIds>({

return {
literalBooleanInComparison,
forTruthy: literalBooleanInComparison ? !negated : negated,
expression,
negated,
};
Expand Down Expand Up @@ -223,48 +221,43 @@ export default util.createRule<Options, MessageIds>({

context.report({
fix: function* (fixer) {
// 1. isUnaryNegation - parent negation
// 2. literalBooleanInComparison - is compared to literal boolean
// 3. negated - is expression negated

const isUnaryNegation =
node.parent != null && nodeIsUnaryNegation(node.parent);

const shouldNegate =
comparison.negated !== comparison.literalBooleanInComparison;

const mutatedNode = isUnaryNegation ? node.parent! : node;

yield fixer.replaceText(
node,
mutatedNode,
sourceCode.getText(comparison.expression),
);

// if the expression `exp` isn't nullable, or we're comparing to `true`,
// we can just replace the entire comparison with `exp` or `!exp`
if (
!comparison.expressionIsNullableBoolean ||
comparison.literalBooleanInComparison
) {
if (!comparison.forTruthy) {
yield fixer.insertTextBefore(node, '!');
if (!util.isStrongPrecedenceNode(comparison.expression)) {
yield fixer.insertTextBefore(node, '(');
yield fixer.insertTextAfter(node, ')');
}
}
return;
}
// if `isUnaryNegation === literalBooleanInComparison === !negated` is true - negate the expression
if (shouldNegate === isUnaryNegation) {
yield fixer.insertTextBefore(mutatedNode, '!');

// if we're here, then the expression is a nullable boolean and we're
// comparing to a literal `false`

// if we're doing `== false` or `=== false`, then we need to negate the expression
if (!comparison.negated) {
const { parent } = node;
// if the parent is a negation, we can instead just get rid of the parent's negation.
// i.e. instead of resulting in `!(!(exp))`, we can just result in `exp`
if (parent != null && nodeIsUnaryNegation(parent)) {
// remove from the beginning of the parent to the beginning of this node
yield fixer.removeRange([parent.range[0], node.range[0]]);
// remove from the end of the node to the end of the parent
yield fixer.removeRange([node.range[1], parent.range[1]]);
} else {
yield fixer.insertTextBefore(node, '!');
// if the expression `exp` is not a strong precedence node, wrap it in parentheses
if (!util.isStrongPrecedenceNode(comparison.expression)) {
yield fixer.insertTextBefore(mutatedNode, '(');
yield fixer.insertTextAfter(mutatedNode, ')');
}
}

// provide the default `true`
yield fixer.insertTextBefore(node, '(');
yield fixer.insertTextAfter(node, ' ?? true)');
// if the expression `exp` is nullable, and we're not comparing to `true`, insert `?? true`
if (
comparison.expressionIsNullableBoolean &&
!comparison.literalBooleanInComparison
) {
// provide the default `true`
yield fixer.insertTextBefore(mutatedNode, '(');
yield fixer.insertTextAfter(mutatedNode, ' ?? true)');
}
},
messageId: comparison.expressionIsNullableBoolean
? comparison.literalBooleanInComparison
Expand Down
119 changes: 119 additions & 0 deletions packages/eslint-plugin/tests/rules/no-unnecessary-boolean-literal-compare.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,5 +360,124 @@ ruleTester.run('no-unnecessary-boolean-literal-compare', rule, {
}
`,
},
{
code: `
declare const varBoolean: boolean;
if (!(varBoolean !== false)) {
}
`,
errors: [
{
messageId: 'negated',
},
],
output: `
declare const varBoolean: boolean;
if (!varBoolean) {
}
`,
},
{
code: `
declare const varBoolean: boolean;
if (!(varBoolean === false)) {
}
`,
errors: [
{
messageId: 'direct',
},
],
output: `
declare const varBoolean: boolean;
if (varBoolean) {
}
`,
},
{
code: `
declare const varBoolean: boolean;
if (!(varBoolean instanceof Event == false)) {
}
`,
errors: [
{
messageId: 'direct',
},
],
output: `
declare const varBoolean: boolean;
if (varBoolean instanceof Event) {
}
`,
},
{
code: `
declare const varBoolean: boolean;
if (varBoolean instanceof Event == false) {
}
`,
errors: [
{
messageId: 'direct',
},
],
output: `
declare const varBoolean: boolean;
if (!(varBoolean instanceof Event)) {
}
`,
},
{
code: `
declare const varBoolean: boolean;
if (!((varBoolean ?? false) !== false)) {
}
`,
errors: [
{
messageId: 'negated',
},
],
output: `
declare const varBoolean: boolean;
if (!(varBoolean ?? false)) {
}
`,
},
{
code: `
declare const varBoolean: boolean;
if (!((varBoolean ?? false) === false)) {
}
`,
errors: [
{
messageId: 'direct',
},
],
output: `
declare const varBoolean: boolean;
if (varBoolean ?? false) {
}
`,
},
{
code: `
declare const varBoolean: boolean;
if (!((varBoolean ?? true) !== false)) {
}
`,
errors: [
{
messageId: 'negated',
},
],
output: `
declare const varBoolean: boolean;
if (!(varBoolean ?? true)) {
}
`,
},
],
});