Skip to content

Commit bc494ff

Browse files
clydinalexeagle
authored andcommitted
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 bazel-contrib#346 PiperOrigin-RevId: 225020913
1 parent 012ceb0 commit bc494ff

File tree

5 files changed

+51
-56
lines changed

5 files changed

+51
-56
lines changed

third_party/github.com/bazelbuild/rules_typescript/internal/tsetse/checker.ts

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -47,36 +47,8 @@ export class Checker {
4747
* `nodeKind` node in `nodeHandlersMap` map. After all rules register their
4848
* handlers, the source file AST will be traversed.
4949
*/
50-
on(nodeKind: ts.SyntaxKind.BinaryExpression,
51-
handlerFunction: (checker: Checker, node: ts.BinaryExpression) => void,
52-
code: number): void;
53-
on(nodeKind: ts.SyntaxKind.ConditionalExpression,
54-
handlerFunction:
55-
(checker: Checker, node: ts.ConditionalExpression) => void,
56-
code: number): void;
57-
on(nodeKind: ts.SyntaxKind.WhileStatement,
58-
handlerFunction: (checker: Checker, node: ts.WhileStatement) => void,
59-
code: number): void;
60-
on(nodeKind: ts.SyntaxKind.IfStatement,
61-
handlerFunction: (checker: Checker, node: ts.IfStatement) => void,
62-
code: number): void;
63-
on(nodeKind: ts.SyntaxKind.CallExpression,
64-
handlerFunction: (checker: Checker, node: ts.CallExpression) => void,
65-
code: number): void;
66-
on(nodeKind: ts.SyntaxKind.PropertyDeclaration,
67-
handlerFunction: (checker: Checker, node: ts.PropertyDeclaration) => void,
68-
code: number): void;
69-
on(nodeKind: ts.SyntaxKind.ElementAccessExpression,
70-
handlerFunction: (checker: Checker, node: ts.ElementAccessExpression) => void,
71-
code: number): void;
72-
on(nodeKind: ts.SyntaxKind.ImportDeclaration,
73-
handlerFunction: (checker: Checker, node: ts.ImportDeclaration) => void,
74-
code: number): void;
75-
on(nodeKind: ts.SyntaxKind,
76-
handlerFunction: (checker: Checker, node: ts.Node) => void,
77-
code: number): void;
7850
on<T extends ts.Node>(
79-
nodeKind: ts.SyntaxKind,
51+
nodeKind: T['kind'],
8052
handlerFunction: (checker: Checker, node: T) => void, code: number) {
8153
const newHandler: Handler = {handlerFunction, code};
8254
const registeredHandlers: Handler[]|undefined =

third_party/github.com/bazelbuild/rules_typescript/internal/tsetse/rules/ban_expect_truthy_promise_rule.ts

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,23 +21,27 @@ export class Rule extends AbstractRule {
2121
}
2222
}
2323

24-
function checkForTruthy(checker: Checker, node: ts.Node) {
25-
const tc = checker.typeChecker;
24+
function checkForTruthy(checker: Checker, node: ts.PropertyAccessExpression) {
25+
if (node.name.text !== 'toBeTruthy') {
26+
return;
27+
}
2628

27-
if (!tsutils.isPropertyAccessExpression(node)) {
29+
const expectCallNode = getLeftmostNode(node);
30+
if (!ts.isCallExpression(expectCallNode)) {
2831
return;
2932
}
3033

31-
if (node.name.getText() !== 'toBeTruthy') {
34+
if (!ts.isIdentifier(expectCallNode.expression) || expectCallNode.expression.text !== 'expect') {
3235
return;
3336
}
3437

35-
const expectCallNode = getLeftmostNode(tc, node);
36-
if (!ts.isCallExpression(expectCallNode)) {
38+
if (expectCallNode.arguments.length === 0 || ts.isAwaitExpression(expectCallNode.arguments[0])) {
3739
return;
3840
}
3941

40-
const signature = checker.typeChecker.getResolvedSignature(expectCallNode);
42+
const tc = checker.typeChecker;
43+
44+
const signature = tc.getResolvedSignature(expectCallNode);
4145
if (signature === undefined) {
4246
return;
4347
}
@@ -48,13 +52,11 @@ function checkForTruthy(checker: Checker, node: ts.Node) {
4852
}
4953

5054
// Only look for methods named expect that return a Matchers
51-
if (!((symbol.name === 'Matchers') &&
52-
expectCallNode.expression.getText() === 'expect')) {
55+
if (symbol.name !== 'Matchers') {
5356
return;
5457
}
5558

56-
if (!tsutils.isThenableType(tc, expectCallNode.arguments[0]) ||
57-
tsutils.isAwaitExpression(expectCallNode.arguments[0])) {
59+
if (!tsutils.isThenableType(tc, expectCallNode.arguments[0])) {
5860
return;
5961
}
6062

@@ -67,8 +69,7 @@ function checkForTruthy(checker: Checker, node: ts.Node) {
6769
`\n\tSee http://tsetse.info/ban-expect-truthy-promise`);
6870
}
6971

70-
function getLeftmostNode(
71-
tc: ts.TypeChecker, node: ts.PropertyAccessExpression) {
72+
function getLeftmostNode(node: ts.PropertyAccessExpression) {
7273
let current: ts.LeftHandSideExpression|undefined = node;
7374
while (ts.isPropertyAccessExpression(current)) {
7475
current = current.expression;

third_party/github.com/bazelbuild/rules_typescript/internal/tsetse/rules/equals_nan_rule.ts

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,41 @@ export class Rule extends AbstractRule {
2020
}
2121

2222
function checkBinaryExpression(checker: Checker, node: ts.BinaryExpression) {
23-
if (node.left.getText() === 'NaN' || node.right.getText() === 'NaN') {
24-
const operator = node.operatorToken;
25-
if (operator.kind == ts.SyntaxKind.EqualsEqualsToken ||
26-
operator.kind === ts.SyntaxKind.EqualsEqualsEqualsToken) {
23+
const isLeftNaN = ts.isIdentifier(node.left) && node.left.text === 'NaN';
24+
const isRightNaN = ts.isIdentifier(node.right) && node.right.text === 'NaN';
25+
if (!isLeftNaN && !isRightNaN) {
26+
return;
27+
}
28+
29+
// We avoid calling getText() on the node.operatorToken because it's slow.
30+
// Instead, manually map back from the kind to the string form of the operator
31+
switch (node.operatorToken.kind) {
32+
case ts.SyntaxKind.EqualsEqualsToken:
33+
checker.addFailureAtNode(
34+
node,
35+
`x == NaN is always false; use isNaN(x) instead`,
36+
);
37+
break;
38+
case ts.SyntaxKind.EqualsEqualsEqualsToken:
39+
checker.addFailureAtNode(
40+
node,
41+
`x === NaN is always false; use isNaN(x) instead`,
42+
);
43+
break;
44+
case ts.SyntaxKind.ExclamationEqualsToken:
2745
checker.addFailureAtNode(
28-
node,
29-
`x ${operator.getText()} NaN is always false; use isNaN(x) instead`);
30-
}
31-
if (operator.kind === ts.SyntaxKind.ExclamationEqualsEqualsToken ||
32-
operator.kind === ts.SyntaxKind.ExclamationEqualsToken) {
46+
node,
47+
`x != NaN is always true; use !isNaN(x) instead`,
48+
);
49+
break;
50+
case ts.SyntaxKind.ExclamationEqualsEqualsToken:
3351
checker.addFailureAtNode(
34-
node,
35-
`x ${operator.getText()} NaN is always true; use !isNaN(x) instead`);
36-
}
52+
node,
53+
`x !== NaN is always true; use !isNaN(x) instead`,
54+
);
55+
break;
56+
default:
57+
// We don't care about other operators acting on NaN
58+
break;
3759
}
3860
}

third_party/github.com/bazelbuild/rules_typescript/package.bzl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
2424
# It will be automatically synced via the npm "version" script
2525
# that is run when running `npm version` during the release
2626
# process. See `Releasing` section in README.md.
27-
VERSION = "0.22.0"
27+
VERSION = "0.21.0"
2828

2929
def rules_typescript_dependencies():
3030
"""

third_party/github.com/bazelbuild/rules_typescript/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"description": "Monorepo for TypeScript Bazel support. Published packages are in subdirectories under internal/",
33
"private": true,
4-
"version": "0.22.0",
4+
"version": "0.21.0",
55
"dependencies": {
66
"jasmine-core": "2.8.0",
77
"karma": "alexeagle/karma#fa1a84ac881485b5657cb669e9b4e5da77b79f0a",

0 commit comments

Comments
 (0)