Skip to content

Commit 1cbc561

Browse files
axetroyaxetroy
authoredJan 18, 2025··
prefer-math-min-max: Reduce false positives in TypeScript (#2527)
·
v58.0.0v57.0.0
1 parent 4e539b4 commit 1cbc561

4 files changed

+366
-2
lines changed
 

‎rules/prefer-math-min-max.js

+108-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,32 @@ const messages = {
77
[MESSAGE_ID]: 'Prefer `Math.{{method}}()` to simplify ternary expressions.',
88
};
99

10+
const isNumberTypeAnnotation = typeAnnotation => {
11+
if (typeAnnotation.type === 'TSNumberKeyword') {
12+
return true;
13+
}
14+
15+
if (typeAnnotation.type === 'TSTypeAnnotation' && typeAnnotation.typeAnnotation.type === 'TSNumberKeyword') {
16+
return true;
17+
}
18+
19+
if (typeAnnotation.type === 'TSTypeReference' && typeAnnotation.typeName.name === 'Number') {
20+
return true;
21+
}
22+
23+
return false;
24+
};
25+
26+
const getExpressionText = (node, sourceCode) => {
27+
const expressionNode = node.type === 'TSAsExpression' ? node.expression : node;
28+
29+
if (node.type === 'TSAsExpression') {
30+
return getExpressionText(expressionNode, sourceCode);
31+
}
32+
33+
return sourceCode.getText(expressionNode);
34+
};
35+
1036
/** @param {import('eslint').Rule.RuleContext} context */
1137
const create = context => ({
1238
/** @param {import('estree').ConditionalExpression} conditionalExpression */
@@ -33,7 +59,7 @@ const create = context => ({
3359
return;
3460
}
3561

36-
const [leftText, rightText, alternateText, consequentText] = [left, right, alternate, consequent].map(node => context.sourceCode.getText(node));
62+
const [leftText, rightText, alternateText, consequentText] = [left, right, alternate, consequent].map(node => getExpressionText(node, context.sourceCode));
3763

3864
const isGreaterOrEqual = operator === '>' || operator === '>=';
3965
const isLessOrEqual = operator === '<' || operator === '<=';
@@ -61,6 +87,87 @@ const create = context => ({
6187
return;
6288
}
6389

90+
for (const node of [left, right]) {
91+
let expressionNode = node;
92+
93+
if (expressionNode.typeAnnotation && expressionNode.type === 'TSAsExpression') {
94+
// Ignore if the test is not a number comparison operator
95+
if (!isNumberTypeAnnotation(expressionNode.typeAnnotation)) {
96+
return;
97+
}
98+
99+
expressionNode = expressionNode.expression;
100+
}
101+
102+
// Find variable declaration
103+
if (expressionNode.type === 'Identifier') {
104+
const variable = context.sourceCode.getScope(expressionNode).variables.find(variable => variable.name === expressionNode.name);
105+
106+
for (const definition of variable?.defs ?? []) {
107+
switch (definition.type) {
108+
case 'Parameter': {
109+
const identifier = definition.name;
110+
111+
/**
112+
Capture the following statement
113+
114+
```js
115+
function foo(a: number) {}
116+
```
117+
*/
118+
if (identifier.typeAnnotation?.type === 'TSTypeAnnotation' && !isNumberTypeAnnotation(identifier.typeAnnotation)) {
119+
return;
120+
}
121+
122+
/**
123+
Capture the following statement
124+
125+
```js
126+
function foo(a = 10) {}
127+
```
128+
*/
129+
if (identifier.parent.type === 'AssignmentPattern' && identifier.parent.right.type === 'Literal' && typeof identifier.parent.right.value !== 'number') {
130+
return;
131+
}
132+
133+
break;
134+
}
135+
136+
case 'Variable': {
137+
/** @type {import('estree').VariableDeclarator} */
138+
const variableDeclarator = definition.node;
139+
140+
/**
141+
Capture the following statement
142+
143+
```js
144+
var foo: number
145+
```
146+
*/
147+
if (variableDeclarator.id.typeAnnotation?.type === 'TSTypeAnnotation' && !isNumberTypeAnnotation(variableDeclarator.id.typeAnnotation)) {
148+
return;
149+
}
150+
151+
/**
152+
Capture the following statement
153+
154+
```js
155+
var foo = 10
156+
```
157+
*/
158+
if (variableDeclarator.init?.type === 'Literal' && typeof variableDeclarator.init.value !== 'number') {
159+
return;
160+
}
161+
162+
break;
163+
}
164+
165+
default:
166+
}
167+
}
168+
}
169+
}
170+
64171
return {
65172
node: conditionalExpression,
66173
messageId: MESSAGE_ID,

‎test/prefer-math-min-max.mjs

+90-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {outdent} from 'outdent';
2-
import {getTester} from './utils/test.mjs';
2+
import {getTester, parsers} from './utils/test.mjs';
33

44
const {test} = getTester(import.meta);
55

@@ -12,6 +12,13 @@ test.snapshot({
1212
// Ignore bigint
1313
'foo > 10n ? 10n : foo',
1414
'foo > BigInt(10) ? BigInt(10) : foo',
15+
16+
// Ignore when you know it is a string
17+
outdent`
18+
function foo(a = 'string', b) {
19+
return a > b ? a : b;
20+
}
21+
`,
1522
],
1623
invalid: [
1724
// Prefer `Math.min()`
@@ -78,3 +85,85 @@ test.snapshot({
7885
'foo.length > bar.length ? bar.length : foo.length',
7986
],
8087
});
88+
89+
test.snapshot({
90+
testerOptions: {
91+
languageOptions: {
92+
parser: parsers.typescript,
93+
},
94+
},
95+
valid: [
96+
outdent`
97+
function foo(a, b) {
98+
return (a as bigint) > b ? a : b;
99+
}
100+
`,
101+
outdent`
102+
function foo(a, b) {
103+
return (a as string) > b ? a : b;
104+
}
105+
`,
106+
outdent`
107+
function foo(a: string, b) {
108+
return a > b ? a : b;
109+
}
110+
`,
111+
outdent`
112+
function foo(a, b: string) {
113+
return a > b ? a : b;
114+
}
115+
`,
116+
outdent`
117+
function foo(a: bigint, b: bigint) {
118+
return a > b ? a : b;
119+
}
120+
`,
121+
outdent`
122+
var foo = 10;
123+
var bar = '20';
124+
125+
var value = foo > bar ? bar : foo;
126+
`,
127+
outdent`
128+
var foo = 10;
129+
var bar: string;
130+
131+
var value = foo > bar ? bar : foo;
132+
`,
133+
],
134+
invalid: [
135+
outdent`
136+
function foo(a, b) {
137+
return (a as number) > b ? a : b;
138+
}
139+
`,
140+
outdent`
141+
function foo(a, b) {
142+
return (a as number) > b ? a : b;
143+
}
144+
`,
145+
outdent`
146+
function foo(a, b) {
147+
return (a as unknown as number) > b ? a : b;
148+
}
149+
`,
150+
151+
outdent`
152+
var foo = 10;
153+
154+
var value = foo > bar ? bar : foo;
155+
`,
156+
outdent`
157+
var foo = 10;
158+
var bar = 20;
159+
160+
var value = foo > bar ? bar : foo;
161+
`,
162+
outdent`
163+
var foo: number;
164+
var bar: number;
165+
166+
var value = foo > bar ? bar : foo;
167+
`,
168+
],
169+
});

‎test/snapshots/prefer-math-min-max.mjs.md

+168
Original file line numberDiff line numberDiff line change
@@ -543,3 +543,171 @@ Generated by [AVA](https://avajs.dev).
543543
> 1 | foo.length > bar.length ? bar.length : foo.length␊
544544
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.min()\` to simplify ternary expressions.␊
545545
`
546+
547+
## invalid(1): function foo(a, b) { return (a as number) > b ? a : b; }
548+
549+
> Input
550+
551+
`␊
552+
1 | function foo(a, b) {␊
553+
2 | return (a as number) > b ? a : b;␊
554+
3 | }␊
555+
`
556+
557+
> Output
558+
559+
`␊
560+
1 | function foo(a, b) {␊
561+
2 | return Math.max(a as number, b);␊
562+
3 | }␊
563+
`
564+
565+
> Error 1/1
566+
567+
`␊
568+
1 | function foo(a, b) {␊
569+
> 2 | return (a as number) > b ? a : b;␊
570+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.max()\` to simplify ternary expressions.␊
571+
3 | }␊
572+
`
573+
574+
## invalid(2): function foo(a, b) { return (a as number) > b ? a : b; }
575+
576+
> Input
577+
578+
`␊
579+
1 | function foo(a, b) {␊
580+
2 | return (a as number) > b ? a : b;␊
581+
3 | }␊
582+
`
583+
584+
> Output
585+
586+
`␊
587+
1 | function foo(a, b) {␊
588+
2 | return Math.max(a as number, b);␊
589+
3 | }␊
590+
`
591+
592+
> Error 1/1
593+
594+
`␊
595+
1 | function foo(a, b) {␊
596+
> 2 | return (a as number) > b ? a : b;␊
597+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.max()\` to simplify ternary expressions.␊
598+
3 | }␊
599+
`
600+
601+
## invalid(3): function foo(a, b) { return (a as unknown as number) > b ? a : b; }
602+
603+
> Input
604+
605+
`␊
606+
1 | function foo(a, b) {␊
607+
2 | return (a as unknown as number) > b ? a : b;␊
608+
3 | }␊
609+
`
610+
611+
> Output
612+
613+
`␊
614+
1 | function foo(a, b) {␊
615+
2 | return Math.max(a as unknown as number, b);␊
616+
3 | }␊
617+
`
618+
619+
> Error 1/1
620+
621+
`␊
622+
1 | function foo(a, b) {␊
623+
> 2 | return (a as unknown as number) > b ? a : b;␊
624+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.max()\` to simplify ternary expressions.␊
625+
3 | }␊
626+
`
627+
628+
## invalid(4): var foo = 10; var value = foo > bar ? bar : foo;
629+
630+
> Input
631+
632+
`␊
633+
1 | var foo = 10;␊
634+
2 |␊
635+
3 | var value = foo > bar ? bar : foo;␊
636+
`
637+
638+
> Output
639+
640+
`␊
641+
1 | var foo = 10;␊
642+
2 |␊
643+
3 | var value = Math.min(foo, bar);␊
644+
`
645+
646+
> Error 1/1
647+
648+
`␊
649+
1 | var foo = 10;␊
650+
2 |␊
651+
> 3 | var value = foo > bar ? bar : foo;␊
652+
| ^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.min()\` to simplify ternary expressions.␊
653+
`
654+
655+
## invalid(5): var foo = 10; var bar = 20; var value = foo > bar ? bar : foo;
656+
657+
> Input
658+
659+
`␊
660+
1 | var foo = 10;␊
661+
2 | var bar = 20;␊
662+
3 |␊
663+
4 | var value = foo > bar ? bar : foo;␊
664+
`
665+
666+
> Output
667+
668+
`␊
669+
1 | var foo = 10;␊
670+
2 | var bar = 20;␊
671+
3 |␊
672+
4 | var value = Math.min(foo, bar);␊
673+
`
674+
675+
> Error 1/1
676+
677+
`␊
678+
1 | var foo = 10;␊
679+
2 | var bar = 20;␊
680+
3 |␊
681+
> 4 | var value = foo > bar ? bar : foo;␊
682+
| ^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.min()\` to simplify ternary expressions.␊
683+
`
684+
685+
## invalid(6): var foo: number; var bar: number; var value = foo > bar ? bar : foo;
686+
687+
> Input
688+
689+
`␊
690+
1 | var foo: number;␊
691+
2 | var bar: number;␊
692+
3 |␊
693+
4 | var value = foo > bar ? bar : foo;␊
694+
`
695+
696+
> Output
697+
698+
`␊
699+
1 | var foo: number;␊
700+
2 | var bar: number;␊
701+
3 |␊
702+
4 | var value = Math.min(foo, bar);␊
703+
`
704+
705+
> Error 1/1
706+
707+
`␊
708+
1 | var foo: number;␊
709+
2 | var bar: number;␊
710+
3 |␊
711+
> 4 | var value = foo > bar ? bar : foo;␊
712+
| ^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.min()\` to simplify ternary expressions.␊
713+
`

‎test/snapshots/prefer-math-min-max.mjs.snap

332 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)
Please sign in to comment.