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

mocha-avoid-only only checking for string literals #436

Closed
mmilleruva opened this issue Jun 14, 2018 · 3 comments
Closed

mocha-avoid-only only checking for string literals #436

mmilleruva opened this issue Jun 14, 2018 · 3 comments
Labels
Difficulty: Medium People with non-trivial experience in TSLint should be able to send a pull request for this issue. Requires Type Checker Must be implemented with a "typed" rule that uses a TypeScript program. Status: Accepting PRs Type: Rule Feature Adding a feature to an existing rule.

Comments

@mmilleruva
Copy link

mmilleruva commented Jun 14, 2018

I noticed mochaAvoidOnlyRule.ts was missing cases where the first argument was not a string literal. So the following case would be missed:

['x', 'y', 'z'].forEach((val) =>  {
   describe.only(val, () => { ...}) 
})

I saw that there were test cases specifically written not to catch this case. I believe it is to avoid false positives where .only is not from mocha. It might be better to be more aggressive with reporting the issue and people can use https://palantir.github.io/tslint/usage/rule-flags/ to disable false positives.

Perhaps the best approach would be to make this rule a typed rule and check if the return of .only is one of ExclusiveSuiteFunction or ExclusiveTestFunction

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/mocha/index.d.ts#L392

@JoshuaKGoldberg
Copy link

+1 to using type information to fix this false negative! Having it as an optionally typed rule would be best so we still get the existing behavior if no project is provided.

@JoshuaKGoldberg JoshuaKGoldberg added 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. Type: Rule Feature Adding a feature to an existing rule. labels Jul 4, 2018
@reduckted
Copy link
Contributor

Another case where you can get a false negative is Angular unit tests that use fakeAsync. The code looks like this:

it.only('something', fakeAsync(() => {
}));

Because the second parameter isn't an arrow function, the rule doesn't consider it to be mocha's it.only() function. Making the rule typed would fix this.

@JoshuaKGoldberg
Copy link

☠️ It's time! ☠️

Per #876, this repository is no longer accepting feature pull requests. TSLint is being deprecated and we recommend you switch to https://typescript-eslint.io.

Thanks for open sourcing with us, everyone!

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. Requires Type Checker Must be implemented with a "typed" rule that uses a TypeScript program. Status: Accepting PRs Type: Rule Feature Adding a feature to an existing rule.
Projects
None yet
Development

No branches or pull requests

3 participants