-
Notifications
You must be signed in to change notification settings - Fork 887
ban: Refactor and add new option format #2547
Conversation
test/rules/ban/tslint.json
Outdated
["xit"], | ||
["fit"], | ||
"xit", | ||
{"name": "fit"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add test for message
in object
src/rules/banRule.ts
Outdated
if (typeof arg === "string") { | ||
functions.push({name: arg}); | ||
} else if (Array.isArray(arg)) { | ||
if (arg.length === 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handle arg.length == 0
src/rules/banRule.ts
Outdated
"eval", | ||
{name: "$", message: "please don't"}, | ||
["describe", "only"], | ||
{object: "it", name: "only", message: "don't fucus tests"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"fucus" -> "focus"?
what if someone actually wanted to ban obj.foo.bar()? |
That's really prone to errors, because it would also ban {"object": ["obj", "foo"], "name": "bar"} |
@adidahiya I actually did that and messed up a rebase recently ... will fix that |
While thinking about #2821 I realized that there shouldn't be an {"name": ["chai", "assert", "equal"], "message": "Use strictEqual instead of equal"} Any thoughts about adding a wildcard to address @nchen63's comment #2547 (comment) to ban a property globally? {"name": ["*", "forEach"], "message": "Use a regular for loop instead"}
// bans [].forEach(...) as well as foo.forEach(...), foo.bar.forEach(...) and foo.bar.baz.forEach(...) |
sgtm 👍
also seems fine, as the user is making it explicit that they are engaging in potentially wild behavior 😂 |
PR updated to allow banning nested methods as requested in #2821 and added a wildcard for method bans |
PR checklist
Overview of change:
Refactor and simplify banRule.
[enhancement]
ban
new options format: allows to specify an optional explanation message for function bans, banning nested methods and using a wildcard for object of a method banFixes: #1940
Fixes: #2821
Is there anything you'd like reviewers to focus on?
This removes some ancient rather unintentional behavior, where banning
foo.bar()
would also banobj.foo.bar()
and evenfn().foo.bar()
.I looked through the git history and PRs for the rule and it seems adding this "feature" was unintended.
CHANGELOG.md entry: