Skip to content

Commit

Permalink
prefer-math-min-max: Reduce false positives in TypeScript (#2527)
Browse files Browse the repository at this point in the history
  • Loading branch information
axetroy authored Jan 18, 2025
1 parent 4e539b4 commit 1cbc561
Show file tree
Hide file tree
Showing 4 changed files with 366 additions and 2 deletions.
109 changes: 108 additions & 1 deletion rules/prefer-math-min-max.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,32 @@ const messages = {
[MESSAGE_ID]: 'Prefer `Math.{{method}}()` to simplify ternary expressions.',
};

const isNumberTypeAnnotation = typeAnnotation => {
if (typeAnnotation.type === 'TSNumberKeyword') {
return true;
}

if (typeAnnotation.type === 'TSTypeAnnotation' && typeAnnotation.typeAnnotation.type === 'TSNumberKeyword') {
return true;
}

if (typeAnnotation.type === 'TSTypeReference' && typeAnnotation.typeName.name === 'Number') {
return true;
}

return false;
};

const getExpressionText = (node, sourceCode) => {
const expressionNode = node.type === 'TSAsExpression' ? node.expression : node;

if (node.type === 'TSAsExpression') {
return getExpressionText(expressionNode, sourceCode);
}

return sourceCode.getText(expressionNode);
};

/** @param {import('eslint').Rule.RuleContext} context */
const create = context => ({
/** @param {import('estree').ConditionalExpression} conditionalExpression */
Expand All @@ -33,7 +59,7 @@ const create = context => ({
return;
}

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

const isGreaterOrEqual = operator === '>' || operator === '>=';
const isLessOrEqual = operator === '<' || operator === '<=';
Expand Down Expand Up @@ -61,6 +87,87 @@ const create = context => ({
return;
}

for (const node of [left, right]) {
let expressionNode = node;

if (expressionNode.typeAnnotation && expressionNode.type === 'TSAsExpression') {
// Ignore if the test is not a number comparison operator
if (!isNumberTypeAnnotation(expressionNode.typeAnnotation)) {
return;
}

expressionNode = expressionNode.expression;
}

// Find variable declaration
if (expressionNode.type === 'Identifier') {
const variable = context.sourceCode.getScope(expressionNode).variables.find(variable => variable.name === expressionNode.name);

for (const definition of variable?.defs ?? []) {
switch (definition.type) {
case 'Parameter': {
const identifier = definition.name;

/**
Capture the following statement
```js
function foo(a: number) {}
```
*/
if (identifier.typeAnnotation?.type === 'TSTypeAnnotation' && !isNumberTypeAnnotation(identifier.typeAnnotation)) {
return;
}

/**
Capture the following statement
```js
function foo(a = 10) {}
```
*/
if (identifier.parent.type === 'AssignmentPattern' && identifier.parent.right.type === 'Literal' && typeof identifier.parent.right.value !== 'number') {
return;
}

break;
}

case 'Variable': {
/** @type {import('estree').VariableDeclarator} */
const variableDeclarator = definition.node;

/**
Capture the following statement
```js
var foo: number
```
*/
if (variableDeclarator.id.typeAnnotation?.type === 'TSTypeAnnotation' && !isNumberTypeAnnotation(variableDeclarator.id.typeAnnotation)) {
return;
}

/**
Capture the following statement
```js
var foo = 10
```
*/
if (variableDeclarator.init?.type === 'Literal' && typeof variableDeclarator.init.value !== 'number') {
return;
}

break;
}

default:
}
}
}
}

return {
node: conditionalExpression,
messageId: MESSAGE_ID,
Expand Down
91 changes: 90 additions & 1 deletion test/prefer-math-min-max.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {outdent} from 'outdent';
import {getTester} from './utils/test.mjs';
import {getTester, parsers} from './utils/test.mjs';

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

Expand All @@ -12,6 +12,13 @@ test.snapshot({
// Ignore bigint
'foo > 10n ? 10n : foo',
'foo > BigInt(10) ? BigInt(10) : foo',

// Ignore when you know it is a string
outdent`
function foo(a = 'string', b) {
return a > b ? a : b;
}
`,
],
invalid: [
// Prefer `Math.min()`
Expand Down Expand Up @@ -78,3 +85,85 @@ test.snapshot({
'foo.length > bar.length ? bar.length : foo.length',
],
});

test.snapshot({
testerOptions: {
languageOptions: {
parser: parsers.typescript,
},
},
valid: [
outdent`
function foo(a, b) {
return (a as bigint) > b ? a : b;
}
`,
outdent`
function foo(a, b) {
return (a as string) > b ? a : b;
}
`,
outdent`
function foo(a: string, b) {
return a > b ? a : b;
}
`,
outdent`
function foo(a, b: string) {
return a > b ? a : b;
}
`,
outdent`
function foo(a: bigint, b: bigint) {
return a > b ? a : b;
}
`,
outdent`
var foo = 10;
var bar = '20';
var value = foo > bar ? bar : foo;
`,
outdent`
var foo = 10;
var bar: string;
var value = foo > bar ? bar : foo;
`,
],
invalid: [
outdent`
function foo(a, b) {
return (a as number) > b ? a : b;
}
`,
outdent`
function foo(a, b) {
return (a as number) > b ? a : b;
}
`,
outdent`
function foo(a, b) {
return (a as unknown as number) > b ? a : b;
}
`,

outdent`
var foo = 10;
var value = foo > bar ? bar : foo;
`,
outdent`
var foo = 10;
var bar = 20;
var value = foo > bar ? bar : foo;
`,
outdent`
var foo: number;
var bar: number;
var value = foo > bar ? bar : foo;
`,
],
});
Loading

0 comments on commit 1cbc561

Please sign in to comment.