refactor(tsetse): optimize equals-nan rule
`getText()` should be avoid as it as an expensive call; favor node 'isXYZ' checks and direct `text` property access.
simplify rule registration method overloads
uses type property directly to remove the need for specialized overloads for each TS node type
cleanup ban-expect-truthy-promise rule
Closes #346
PiperOrigin-RevId: 225020913
diff --git a/internal/tsetse/checker.ts b/internal/tsetse/checker.ts
index 5e8b6d3..72fdbaa 100644
--- a/internal/tsetse/checker.ts
+++ b/internal/tsetse/checker.ts
@@ -47,36 +47,8 @@
* `nodeKind` node in `nodeHandlersMap` map. After all rules register their
* handlers, the source file AST will be traversed.
*/
- on(nodeKind: ts.SyntaxKind.BinaryExpression,
- handlerFunction: (checker: Checker, node: ts.BinaryExpression) => void,
- code: number): void;
- on(nodeKind: ts.SyntaxKind.ConditionalExpression,
- handlerFunction:
- (checker: Checker, node: ts.ConditionalExpression) => void,
- code: number): void;
- on(nodeKind: ts.SyntaxKind.WhileStatement,
- handlerFunction: (checker: Checker, node: ts.WhileStatement) => void,
- code: number): void;
- on(nodeKind: ts.SyntaxKind.IfStatement,
- handlerFunction: (checker: Checker, node: ts.IfStatement) => void,
- code: number): void;
- on(nodeKind: ts.SyntaxKind.CallExpression,
- handlerFunction: (checker: Checker, node: ts.CallExpression) => void,
- code: number): void;
- on(nodeKind: ts.SyntaxKind.PropertyDeclaration,
- handlerFunction: (checker: Checker, node: ts.PropertyDeclaration) => void,
- code: number): void;
- on(nodeKind: ts.SyntaxKind.ElementAccessExpression,
- handlerFunction: (checker: Checker, node: ts.ElementAccessExpression) => void,
- code: number): void;
- on(nodeKind: ts.SyntaxKind.ImportDeclaration,
- handlerFunction: (checker: Checker, node: ts.ImportDeclaration) => void,
- code: number): void;
- on(nodeKind: ts.SyntaxKind,
- handlerFunction: (checker: Checker, node: ts.Node) => void,
- code: number): void;
on<T extends ts.Node>(
- nodeKind: ts.SyntaxKind,
+ nodeKind: T['kind'],
handlerFunction: (checker: Checker, node: T) => void, code: number) {
const newHandler: Handler = {handlerFunction, code};
const registeredHandlers: Handler[]|undefined =
diff --git a/internal/tsetse/rules/ban_expect_truthy_promise_rule.ts b/internal/tsetse/rules/ban_expect_truthy_promise_rule.ts
index 6f852ae..fbbefd9 100644
--- a/internal/tsetse/rules/ban_expect_truthy_promise_rule.ts
+++ b/internal/tsetse/rules/ban_expect_truthy_promise_rule.ts
@@ -21,23 +21,27 @@
}
}
-function checkForTruthy(checker: Checker, node: ts.Node) {
- const tc = checker.typeChecker;
-
- if (!tsutils.isPropertyAccessExpression(node)) {
+function checkForTruthy(checker: Checker, node: ts.PropertyAccessExpression) {
+ if (node.name.text !== 'toBeTruthy') {
return;
}
- if (node.name.getText() !== 'toBeTruthy') {
- return;
- }
-
- const expectCallNode = getLeftmostNode(tc, node);
+ const expectCallNode = getLeftmostNode(node);
if (!ts.isCallExpression(expectCallNode)) {
return;
}
- const signature = checker.typeChecker.getResolvedSignature(expectCallNode);
+ if (!ts.isIdentifier(expectCallNode.expression) || expectCallNode.expression.text !== 'expect') {
+ return;
+ }
+
+ if (expectCallNode.arguments.length === 0 || ts.isAwaitExpression(expectCallNode.arguments[0])) {
+ return;
+ }
+
+ const tc = checker.typeChecker;
+
+ const signature = tc.getResolvedSignature(expectCallNode);
if (signature === undefined) {
return;
}
@@ -48,13 +52,11 @@
}
// Only look for methods named expect that return a Matchers
- if (!((symbol.name === 'Matchers') &&
- expectCallNode.expression.getText() === 'expect')) {
+ if (symbol.name !== 'Matchers') {
return;
}
- if (!tsutils.isThenableType(tc, expectCallNode.arguments[0]) ||
- tsutils.isAwaitExpression(expectCallNode.arguments[0])) {
+ if (!tsutils.isThenableType(tc, expectCallNode.arguments[0])) {
return;
}
@@ -67,8 +69,7 @@
`\n\tSee http://tsetse.info/ban-expect-truthy-promise`);
}
-function getLeftmostNode(
- tc: ts.TypeChecker, node: ts.PropertyAccessExpression) {
+function getLeftmostNode(node: ts.PropertyAccessExpression) {
let current: ts.LeftHandSideExpression|undefined = node;
while (ts.isPropertyAccessExpression(current)) {
current = current.expression;
diff --git a/internal/tsetse/rules/equals_nan_rule.ts b/internal/tsetse/rules/equals_nan_rule.ts
index da8f7fe..38b01d5 100644
--- a/internal/tsetse/rules/equals_nan_rule.ts
+++ b/internal/tsetse/rules/equals_nan_rule.ts
@@ -20,19 +20,41 @@
}
function checkBinaryExpression(checker: Checker, node: ts.BinaryExpression) {
- if (node.left.getText() === 'NaN' || node.right.getText() === 'NaN') {
- const operator = node.operatorToken;
- if (operator.kind == ts.SyntaxKind.EqualsEqualsToken ||
- operator.kind === ts.SyntaxKind.EqualsEqualsEqualsToken) {
+ const isLeftNaN = ts.isIdentifier(node.left) && node.left.text === 'NaN';
+ const isRightNaN = ts.isIdentifier(node.right) && node.right.text === 'NaN';
+ if (!isLeftNaN && !isRightNaN) {
+ return;
+ }
+
+ // We avoid calling getText() on the node.operatorToken because it's slow.
+ // Instead, manually map back from the kind to the string form of the operator
+ switch (node.operatorToken.kind) {
+ case ts.SyntaxKind.EqualsEqualsToken:
checker.addFailureAtNode(
- node,
- `x ${operator.getText()} NaN is always false; use isNaN(x) instead`);
- }
- if (operator.kind === ts.SyntaxKind.ExclamationEqualsEqualsToken ||
- operator.kind === ts.SyntaxKind.ExclamationEqualsToken) {
+ node,
+ `x == NaN is always false; use isNaN(x) instead`,
+ );
+ break;
+ case ts.SyntaxKind.EqualsEqualsEqualsToken:
checker.addFailureAtNode(
- node,
- `x ${operator.getText()} NaN is always true; use !isNaN(x) instead`);
- }
+ node,
+ `x === NaN is always false; use isNaN(x) instead`,
+ );
+ break;
+ case ts.SyntaxKind.ExclamationEqualsToken:
+ checker.addFailureAtNode(
+ node,
+ `x != NaN is always true; use !isNaN(x) instead`,
+ );
+ break;
+ case ts.SyntaxKind.ExclamationEqualsEqualsToken:
+ checker.addFailureAtNode(
+ node,
+ `x !== NaN is always true; use !isNaN(x) instead`,
+ );
+ break;
+ default:
+ // We don't care about other operators acting on NaN
+ break;
}
}
