-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Description
Before You File a Proposal Please Confirm You Have Done The Following...
- I have searched for related issues and found none that match my proposal.
- I have searched the current rule list and found no rules that match my proposal.
- I have read the FAQ and my problem is not listed.
My proposal is suitable for this project
- My proposal specifically checks TypeScript syntax, or it proposes a check that requires type information to be accurate.
- My proposal is not a "formatting rule"; meaning it does not just enforce how code is formatted (whitespace, brace placement, etc).
- I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).
Description
My proposal is similar to this proposal: #5075), (which is already covered by: https://typescript-eslint.io/rules/no-unnecessary-boolean-literal-compare). But my proposal goes further. I have actually already written the rule, and it is presently in use in our codebase in Office.
Rule name: no-unnecessary-falsy-condition (the name is open to debate)
Summary: This rule leverages the TypeChecker to identify when explicit checks against falsy values are not needed, and provides an autofixer to shorten them.
Motivation: Varies -- for some people, they simply prefer the "leaner" code style. But for us, this has a meaningful improvement on final bundle size. Running this rule across one of our repos shaved off 20Kb. Existing JS minifiers cannot safely do this because they lack type information. This rule is what I would more broadly categorize as "type-aware minification"
More formally, the rule identifies the following as eligible for shortening:
- A binary expression involving either strict equals (
===) or strict not equals (!==) - One side of that expression is an explicit falsy value:
null,undefined,false,0, or''(NaNis excluded because it never equal to anything, not even itself) - The other side of the expression is a Type that accepts exactly one falsy value (the one on the other side).
Then the condition is eligible for shortening. A simple example will help (with many many more examples provided in the Pass and Fail cases below):
let foo: string[] | undefined = functionThatReturnsSomething();
if (foo === undefined) {
}The rule would flag the above code and autofix it to:
if (!foo) {
}We know that foo is either string[] or undefined. Besides undefined, there is no other falsy value that foo could be. TS already knows this, and will balk if you attempt to assign null, false, '', or 0 to foo.
However, something like:
let foo: number | undefined;
if (foo === undefined) {
}Would not be eligible for shortening. Since foo could be a number or undefined, shortening it to if (!foo) is ambiguous and therefore not safe.
NOTE: This rule is similar to no-unnecessary-condition, but it is not the same. The former would detect when there is no overlap between types on each side of the expression. My proposed rule identifies when there is a specific kind of overlap such that it is safe to shorten the code. My rule is capable of detecting the conditions that no-unnecessary-condition reports on, but I explicitly chose not to do that, since I did not want the rules to overlap.
The rule is configurable. If you only want to shorten expressions involving say, null, undefined, and false that is easy to specify. By default, it would shorten for all falsy values: null, undefined, false, 0, and ''
As I have already written the rule, I have many Pass and Fail test cases. I will share them all below
Fail Cases
// Examples that require coercion to boolean
{
// ReturnStatement:
// `y` needs to be coerced to a boolean to satisfy the return type of `fn`
code: `
function fn(y: number): boolean {
return y !== 0;
}`,
errors: [{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'y', value: '0' } }],
output: `
function fn(y: number): boolean {
return !!y;
}`
},
{
// ReturnStatement:
// `y` and `x` both need to be coerced to booleans to satisfy the return type of `fn`
code: `
function fn(x: string[] | undefined, y: number): boolean {
return (y !== 0 || undefined !== x);
}`,
errors: [
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'y', value: '0' } },
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'x', value: 'undefined' } }
],
output: `
function fn(x: string[] | undefined, y: number): boolean {
return (!!y || !!x);
}`
},
{
// CallExpression:
// `iLoveTheAvettBrothers` needs to be coerced to a boolean to satisfy the boolean argument to `someFnCall`
code: `
let iLoveTheAvettBrothers: string[] | undefined;
if (true) {
someFnCall(iLoveTheAvettBrothers !== undefined)
}`,
errors: [
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'iLoveTheAvettBrothers', value: 'undefined' } }
],
output: `
let iLoveTheAvettBrothers: string[] | undefined;
if (true) {
someFnCall(!!iLoveTheAvettBrothers)
}`
},
{
// CallExpression:
// `fname` and `age` both need to be coerced to boolean to satisfy the boolean argument to `fn`
code: `
function fn(x: boolean) {};
let fname: string;
let age: number;
const y = fn(fname !== '' && age === 0);`,
errors: [
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'fname', value: '""' } },
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'age', value: '0' } }
],
output: `
function fn(x: boolean) {};
let fname: string;
let age: number;
const y = fn(!!fname && !age);`
},
{
// VariableDeclarator:
// `x` needs to be coerced to boolean (y does not; it's already a boolean) to preserve the inferred type of `z` (boolean)
code: `
let x: object | null;
let y: boolean;
let z = x !== null && y !== false`,
errors: [
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'x', value: 'null' } },
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'y', value: 'false' } }
],
output: `
let x: object | null;
let y: boolean;
let z = !!x && y`
},
{
// VariableDeclarator:
// `x` and `y` both need to be coerced to boolean to preserve the inferred type of `z` (boolean)
code: `
let x: object | null;
let y: 'scott avett' | 'seth avett' | true | undefined;
let z = x !== null && y !== undefined;`,
errors: [
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'x', value: 'null' } },
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'y', value: 'undefined' } }
],
output: `
let x: object | null;
let y: 'scott avett' | 'seth avett' | true | undefined;
let z = !!x && !!y;`
},
{
// VariableDeclarator:
// `iLoveTheAvettBrothers` is declared as a boolean; `bestBand`, `avett`, and `seenInConcertNTimes` all must be
// coerced to boolean to satisfy the type of `iLoveTheAvettBrothers`
code: `
let avett: string;
let bestBand: 'Jimmy Eat World' | 'The Avett Brothers' | 'Of Monsters and Men' | null;
let seenInConcertNTimes = 14;
const iLoveTheAvettBrothers: boolean =
avett === '' ||
(bestBand !== null && avett !== '') || seenInConcertNTimes !== 0;`,
errors: [
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'avett', value: '""' } },
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'bestBand', value: 'null' } },
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'avett', value: '""' } },
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'seenInConcertNTimes', value: '0' } }
],
output: `
let avett: string;
let bestBand: 'Jimmy Eat World' | 'The Avett Brothers' | 'Of Monsters and Men' | null;
let seenInConcertNTimes = 14;
const iLoveTheAvettBrothers: boolean =
!avett ||
(!!bestBand && !!avett) || !!seenInConcertNTimes;`
},
{
// AssignmentExpression:
// `theAvettBrothers` needs to be coerced to a boolean, as per its declared type
code: `
let theAvettBrothers: string[] | undefined;
let theAvettsAreHere: boolean;
theAvettsAreHere = theAvettBrothers !== undefined;`,
errors: [{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'theAvettBrothers', value: 'undefined' } }],
output: `
let theAvettBrothers: string[] | undefined;
let theAvettsAreHere: boolean;
theAvettsAreHere = !!theAvettBrothers;`
},
{
// AssignmentExpression:
// `bar = foo()` needs to remain wrapped in parens
code: `
function foo(): string[] | undefined {}
let bar = undefined;
if ((bar = foo()) !== undefined) {}`,
errors: [{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'bar = foo()', value: 'undefined' } }],
output: `
function foo(): string[] | undefined {}
let bar = undefined;
if ((bar = foo())) {}`
},
{
// AssignmentExpression:
// `bar = foo()` needs to remain wrapped in parens AND coerced to boolean to satisfy the `bit` arg to the function `flops`
code: `
function foo(): string[] | undefined {}
function flop(bit: boolean) {}
let bar = undefined;
flop((bar = foo()) !== undefined);`,
errors: [{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'bar = foo()', value: 'undefined' } }],
output: `
function foo(): string[] | undefined {}
function flop(bit: boolean) {}
let bar = undefined;
flop(!!(bar = foo()));`
},
{
// SwitchStatement:
// `avett` needs to be coerced to boolean to preserve the original type that was given to the SwitchStatement
code: `
let avett: string;
switch(avett !== '') {
}`,
errors: [{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'avett', value: '""' } }],
output: `
let avett: string;
switch(!!avett) {
}`
},
{
// ArrowFunctionExpression:
// `space` must be coerced to boolean b/c it is used as implicit return in an ArrowFunctionExpression
code: `const perfect = (space: string) => space !== '';`,
errors: [{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'space', value: '""' } }],
output: `const perfect = (space: string) => !!space;`
},
{
// ArrowFunctionExpression:
// `me` and `tune` are part of the implicit arrow function return statement, which involves logical expressions, so it requires coercion to boolean
code: `const hand = (me: string, down: number, tune: string[] | undefined) => () => me !== '' || (down === 0 || tune !== undefined);`,
errors: [
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'me', value: '""' } },
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'down', value: '0' } },
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'tune', value: 'undefined' } }
],
output: `const hand = (me: string, down: number, tune: string[] | undefined) => () => !!me || (!down || !!tune);`
},
// Examples that do not require coercion to boolean
{
// BlockStatement:
// The body of the forEach is a BlockStatement. Coercion not required, unless within it, there is a ReturnStatement, CallExpression, VariableDeclarator, VariableAssignment, or SwitchStatement
code: `
const n = [ 1, 2, 3, 4, 5, 6, 7 ];
n.forEach(n => {
if (n !== 0) {}
});`,
errors: [{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'n', value: '0' } }],
output: `
const n = [ 1, 2, 3, 4, 5, 6, 7 ];
n.forEach(n => {
if (n) {}
});`
},
{
// ConditionalExpression:
// Ternary expressions should also be flagged in the same way
// Note that ternary expressions are similar to if-statements, and as such, `x` doesn't require coercion
code: `
let x: string[] | undefined;
const y = x !== undefined ? [ 'avett', 'brothers' ] : [ 'sorry', 'no', 'avetts', 'for', 'you' ];`,
errors: [{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'x', value: 'undefined' } }],
output: `
let x: string[] | undefined;
const y = x ? [ 'avett', 'brothers' ] : [ 'sorry', 'no', 'avetts', 'for', 'you' ];`
},
{
// ConditionalExpression:
// Ternary expressions that involves type narrowing
// Note that ternary expressions are similar to if-statements, and as such, `x` doesn't require coercion
code: `
let x: string[] | number | undefined;
const y = typeof x !== 'number' ? x !== undefined ? [ 'avett', 'brothers' ] : [ 'sorry', 'no', 'avetts', 'for', 'you' ] : [ 'scott', 'seth', 'bob', 'joe' ];`,
errors: [{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'x', value: 'undefined' } }],
output: `
let x: string[] | number | undefined;
const y = typeof x !== 'number' ? x ? [ 'avett', 'brothers' ] : [ 'sorry', 'no', 'avetts', 'for', 'you' ] : [ 'scott', 'seth', 'bob', 'joe' ];`
},
{
// Only trigger the rule for null, undefined, and false (not 0 or '')
options: [null, undefined, false],
code: `
let x: number;
let y: string;
let z: object | undefined;
let p: string[] | null;
let q: boolean;
if (x === 0) {};
if (y === '') {};
if (z === undefined) {};
if (p === null) {};
if (q === false) {};`,
errors: [
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'z', value: 'undefined' } },
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'p', value: 'null' } },
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'q', value: 'false' } }
],
output: `
let x: number;
let y: string;
let z: object | undefined;
let p: string[] | null;
let q: boolean;
if (x === 0) {};
if (y === '') {};
if (!z) {};
if (!p) {};
if (!q) {};`
},
{
// Only trigger the rule for 0 and '' (not null, undefined, or false)
options: ['', 0],
// `x` is used in an if statement, so it doesn't require coercion to boolean
code: `
let x: number;
let y: string;
let z: object | undefined;
let p: string[] | null;
let q: boolean;
if (x !== 0) {};
if (y === '') {};
if (z === undefined) {};
if (p === null) {};
if (q === false) {};`,
errors: [
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'x', value: '0' } },
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'y', value: '""' } }
],
output: `
let x: number;
let y: string;
let z: object | undefined;
let p: string[] | null;
let q: boolean;
if (x) {};
if (!y) {};
if (z === undefined) {};
if (p === null) {};
if (q === false) {};`
},
{
// CallExpression:
// `x` is used in a call expression, but `someFnCall` doesn't specify that `thing` is a boolean, so coercion not required
code: `
function someFnCall(thing) {}
let x: boolean;
someFnCall(x !== false);`,
errors: [{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'x', value: 'false' } }],
output: `
function someFnCall(thing) {}
let x: boolean;
someFnCall(x);`
},
{
// x's only possible falsy value is undefined; check not necessary
code: `
let x: string[] | undefined;
if (undefined === x) {}`,
errors: [{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'x', value: 'undefined' } }],
output: `
let x: string[] | undefined;
if (!x) {}`
},
{
// x's only possible falsy value is ''; check not necessary
code: `
let x: string[] | '';
if (x === '') {}`,
errors: [{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'x', value: '""' } }],
output: `
let x: string[] | '';
if (!x) {}`
},
{
// x's only possible falsy value is 0; check not necessary
code: `
let x: { a: number, b: string } | 0;
if (0 === x) {}`,
errors: [{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'x', value: '0' } }],
output: `
let x: { a: number, b: string } | 0;
if (!x) {}`
},
{
// x's only possibly falsy value is null; check not necessary
code: `
let x: { a: number, b: string } | { MP: 'OY' } | null;
if (x === null) {}`,
errors: [{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'x', value: 'null' } }],
output: `
let x: { a: number, b: string } | { MP: 'OY' } | null;
if (!x) {}`
},
{
// x's only possibly falsy value is null; check not necessary
code: `
let x: boolean;
if (x === false) {}`,
errors: [{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'x', value: 'false' } }],
output: `
let x: boolean;
if (!x) {}`
},
{
// ArrowFunctionExpression:
// Coercion of `space` not explicitly needed; happens naturally through negation
code: `const perfect = (space: string) => space === '';`,
errors: [{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'space', value: '""' } }],
output: `const perfect = (space: string) => !space;`
},
{
// `x` and `y` can only accept falsy values of undefined and 0 respectively;
// They don't require coercion when used in a while condition
code: `
let x: string[] | undefined;
let y: number;
while (x !== undefined && y !== 0) {}`,
errors: [
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'x', value: 'undefined' } },
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'y', value: '0' } }
],
output: `
let x: string[] | undefined;
let y: number;
while (x && y) {}`
},
{
// `rects` can only accept a single falsy value of undefined, `rects.length` being a number, can only accept a single falsy value of 0
// Neither requires coercion when used in an if statement
code: `
let rects: object[] | undefined;
if (rects === undefined || rects.length === 0) {}`,
errors: [
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'rects', value: 'undefined' } },
{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'rects.length', value: '0' } }
],
output: `
let rects: object[] | undefined;
if (!rects || !rects.length) {}`
},
{
// `node`'s only possible falsy value is undefined.
// It need not be coerced to a boolean when used as part of a for loop
code: `
interface node {
next: node | undefined;
}
let x: node | undefined;
for (let y = x; y !== undefined; y = y.next) {}`,
errors: [{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'y', value: 'undefined' } }],
output: `
interface node {
next: node | undefined;
}
let x: node | undefined;
for (let y = x; y; y = y.next) {}`
},
{
// Type narrowing test
code: `
let x: number | undefined;
if (typeof x !== 'number') {
if (x !== undefined) {}
}`,
errors: [{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'x', value: 'undefined' } }],
output: `
let x: number | undefined;
if (typeof x !== 'number') {
if (x) {}
}`
},
{
// Test to ensure that -0 is caught the same way 0 would be
code: `
let x: number;
if (x === -0) {}`,
errors: [{ messageId: 'unnecessaryExplicitFalsyCheck', data: { type: 'x', value: '0' } }],
output: `
let x: number;
if (!x) {}`
},Pass Cases
{
code: `
let n: number | boolean;
if (n > 7) {}`
},
{
code: `
let x: { toUpperCase: () => string; } | undefined;
if (x === undefined) {}`
},
{
code: `
let x: any;
if (x === undefined) {}`
},
{
code: `
let x: number | null;
if (x === 'The Avett brothers') {}`
},
{
code: `
let x: number | null
if (x === null) {}`
},
{
code: `
let x: Foo | null | undefined
if (x === null) {}`
},
{
code: `
interface Avett {};
type AvettOrBust = Avett | undefined | false;
if (x === false) {}`
},
{
code: `
let x: boolean | undefined;
if (x === undefined) {}`
},
{
code: `
let x: boolean | undefined;
if (x === false) {}`
},
{
code: `
let p: { toUpperCase: () => string; };
if (p === 5) {}`
},
{
code: `
class Foo {}
function getFooMaybe(): Foo | null | undefined { return new Foo(); };
if (getFooMaybe() === undefined) {}`
},
{
code: `
class Foo {}
function getFooMaybe(): Foo | null | undefined { return new Foo(); };
if (getFooMaybe() === '') {}`
},
{
code: `
let x: { a: number, b: string } | {} | null | undefined;
if (x === null) {}`
},
{
code: `
let x: {};
if (x === '') {}`
},
{
code: `
let x: { toFixed: (n: number) => string; };
if (x === 777) {}`
},
{
code: `
let x: { a: number, b: string } | {} | null;
if (x === null) {}`
},
{
code: `
let x: { toLocaleString: () => string; };
if (x === false) {}`
},
{
code: `
let x: { toLocaleString: () => string; };
if (x === '') {}`
},
{
code: `
let x: string | boolean;
if (x === '') {}`
},
{
code: `
let x: false | undefined;
if (x === false) {}`
},
{
options: [null, undefined, false],
code: `
enum Avett {
Stephen = 0,
Scott = 1,
Seth = 2,
Bob = 3
}
const isAvett = (a: Avett | undefined): boolean => {
return (
a !== undefined &&
a !== Avett.Stephen
);
};`
}(edit 9/4 by @JoshuaKGoldberg: made this a toggle list to spare my scrollbar)
Additional Info
No response