Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

false positive for no-string-based-set-interval rule only when type checking is enabled #374

Closed
ChrisMBarr opened this issue May 17, 2017 · 2 comments
Labels
Difficulty: Medium People with non-trivial experience in TSLint should be able to send a pull request for this issue. 💀 R.I.P. 💀 Requires Type Checker Must be implemented with a "typed" rule that uses a TypeScript program. Type: Bug

Comments

@ChrisMBarr
Copy link
Contributor

typescript version 2.2.2
ts-lint version 5.2.0
running via gulp-tslint 8.0.0


When type checlking is enabled the following code fails with ERROR: 84:32 no-string-based-set-interval Forbidden setInterval string parameter: process.

const process = () => {
    for (let i = 0; i < queue.length; i++) {
        const qi = queue.splice(0, 1)[0];
        if (qi) {
            sendRequest(qi);
        } else {
            break;
        }
    }
};

const intervalLoop = window.setInterval(process, 100);  // <-- failure reported here

This only fails when rule type checking is enabled though, even though this rule says it does not require type checking! No failures here are produced when type checking is not enabled.

However, writing it like this does not produce a failure:

const intervalLoop = window.setInterval(() => {
    for (let i = 0; i < queue.length; i++) {
        const qi = queue.splice(0, 1)[0];
        if (qi) {
            sendRequest(qi);
        } else {
            break;
        }
    }
}, 100);

I'm not sure why... I ended up keeping the re-written version since it's simpler, but this feels like a bug.

@JoshuaKGoldberg
Copy link

The rule doesn't require type checking, but it will take advantage of a TypeScript program if provided one. This must be a bug specifically in that optional logic.

Scanning through the code, noStringBasedSetIntervalRule.ts uses a NoStringParameterToFunctionCallWalker, which uses its isExpressionEvaluatingToFunction. I'd check there for logic that requires this.typeChecker.

@JoshuaKGoldberg JoshuaKGoldberg added Type: Bug Requires Type Checker Must be implemented with a "typed" rule that uses a TypeScript program. Status: Accepting PRs Difficulty: Medium People with non-trivial experience in TSLint should be able to send a pull request for this issue. labels Jul 6, 2018
@JoshuaKGoldberg
Copy link

💀 It's time! 💀

TSLint is deprecated and no longer accepting pull requests other than security fixes. See #876. ☠️
We recommend you instead use typescript-eslint to lint your TypeScript code with ESLint. ✅

👋 It was a pleasure open sourcing with you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Difficulty: Medium People with non-trivial experience in TSLint should be able to send a pull request for this issue. 💀 R.I.P. 💀 Requires Type Checker Must be implemented with a "typed" rule that uses a TypeScript program. Type: Bug
Projects
None yet
Development

No branches or pull requests

2 participants