prefer-math-min-max: Reduce false positives in TypeScript (#2527)

Author: axetroy

rules/prefer-math-min-max.js

@@ -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 */
@@ -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 === '<=';
@@ -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,

test/prefer-math-min-max.mjs

@@ -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);
 
@@ -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()`
@@ -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;
+		`,
+	],
+});