Skip to content
Permalink
Browse files

fix(eslint-plugin): [no-unnecessary-condition] correct regression wit…

…h unary negations (#2422)

Fixes #2421
  • Loading branch information
bradzacher committed Aug 25, 2020
1 parent 797a133 commit d1f08879338c825a1a20406fe47c051a287d6519
@@ -165,13 +165,16 @@ export default createRule<Options, MessageId>({
* Checks if a conditional node is necessary:
* if the type of the node is always true or always false, it's not necessary.
*/
function checkNode(node: TSESTree.Expression): void {
function checkNode(
node: TSESTree.Expression,
isUnaryNotArgument = false,
): void {
// Check if the node is Unary Negation expression and handle it
if (
node.type === AST_NODE_TYPES.UnaryExpression &&
node.operator === '!'
) {
return checkIfUnaryNegationExpressionIsNecessaryConditional(node);
return checkNode(node.argument, true);
}

// Since typescript array index signature types don't represent the
@@ -208,41 +211,19 @@ export default createRule<Options, MessageId>({
) {
return;
}
const messageId = isTypeFlagSet(type, ts.TypeFlags.Never)
? 'never'
: !isPossiblyTruthy(type)
? 'alwaysFalsy'
: !isPossiblyFalsy(type)
? 'alwaysTruthy'
: undefined;

if (messageId) {
context.report({ node, messageId });
let messageId: MessageId | null = null;

if (isTypeFlagSet(type, ts.TypeFlags.Never)) {
messageId = 'never';
} else if (!isPossiblyTruthy(type)) {
messageId = !isUnaryNotArgument ? 'alwaysFalsy' : 'alwaysTruthy';
} else if (!isPossiblyFalsy(type)) {
messageId = !isUnaryNotArgument ? 'alwaysTruthy' : 'alwaysFalsy';
}
}

/**
* If it is Unary Negation Expression, check the argument for alwaysTruthy / alwaysFalsy
*/
function checkIfUnaryNegationExpressionIsNecessaryConditional(
node: TSESTree.UnaryExpression,
): void {
if (isArrayIndexExpression(node.argument)) {
return;
}
const type = getNodeType(node.argument);
const messageId = isTypeFlagSet(type, ts.TypeFlags.Never)
? 'never'
: isPossiblyTruthy(type)
? 'alwaysFalsy'
: isPossiblyFalsy(type)
? 'alwaysTruthy'
: undefined;

if (messageId) {
context.report({ node, messageId });
}
return;
}

function checkNodeForNullish(node: TSESTree.Expression): void {
@@ -257,13 +238,15 @@ export default createRule<Options, MessageId>({
if (isTypeAnyType(type) || isTypeUnknownType(type)) {
return;
}
const messageId = isTypeFlagSet(type, ts.TypeFlags.Never)
? 'never'
: !isPossiblyNullish(type)
? 'neverNullish'
: isAlwaysNullish(type)
? 'alwaysNullish'
: undefined;

let messageId: MessageId | null = null;
if (isTypeFlagSet(type, ts.TypeFlags.Never)) {
messageId = 'never';
} else if (!isPossiblyNullish(type)) {
messageId = 'neverNullish';
} else if (isAlwaysNullish(type)) {
messageId = 'alwaysNullish';
}

if (messageId) {
context.report({ node, messageId });
@@ -472,37 +472,27 @@ function test(testVal?: boolean) {
}
}
`,
],
invalid: [
// Ensure that it's checking in all the right places
{
code: `
const a = null;
if (!a) {
}
`,
errors: [ruleError(3, 5, 'alwaysTruthy')],
},
{
code: `
const a = true;
if (!a) {
`
declare const x: string[];
if (!x[0]) {
}
`,
errors: [ruleError(3, 5, 'alwaysFalsy')],
},
{
code: `
function sayHi(): void {
console.log('Hi!');
`,
// https://github.com/typescript-eslint/typescript-eslint/issues/2421
`
const isEven = (val: number) => val % 2 === 0;
if (!isEven(1)) {
}
`,
`
declare const booleanTyped: boolean;
declare const unknownTyped: unknown;
let speech: never = sayHi();
if (!speech) {
if (!(booleanTyped || unknownTyped)) {
}
`,
errors: [ruleError(7, 5, 'never')],
},
`,
],
invalid: [
// Ensure that it's checking in all the right places
{
code: `
const b1 = true;
@@ -1442,5 +1432,34 @@ function test(testVal?: true) {
},
],
},
// https://github.com/typescript-eslint/typescript-eslint/issues/2255
{
code: `
const a = null;
if (!a) {
}
`,
errors: [ruleError(3, 6, 'alwaysTruthy')],
},
{
code: `
const a = true;
if (!a) {
}
`,
errors: [ruleError(3, 6, 'alwaysFalsy')],
},
{
code: `
function sayHi(): void {
console.log('Hi!');
}
let speech: never = sayHi();
if (!speech) {
}
`,
errors: [ruleError(7, 6, 'never')],
},
],
});

0 comments on commit d1f0887

Please sign in to comment.