Skip to content

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Suggestion: standardize against prefixing rule names with "no-" #6022

JoshuaKGoldberg started this conversation in Technical Discussions
Discussion options

Many ESLint and TSLint rules start with "no-" to indicate a preference against some behavior. Some behaviors are always bad (e.g. no-eval), but others eventually are requested to have a preference towards. Having "no-" starting the rule name then makes it weird to add an option to encourage the behavior. For example:

Having only some rules start with no- also makes them a little less discoverable. There are a few more characters to scan through, and alphabetical lists make a little less sense.

Since it's already a conversion to go from TSLint to ESLint, can there be a preference set for not starting rule names with "no-"? In the two cases above, it'd be better if they were parameter-properties and type-alias.

You must be logged in to vote

Replies: 16 comments · 4 replies

Comment options

no- prefix in eslint is used for rules which disallow / disable some functionality.

Changing names of rules is considered breaking change and can be done only in major release

I don't disagree that some of rules should not be prefixed with no-, if options allow to inverse ( enforce) functionality.

https://eslint.org/docs/rules/

You must be logged in to vote
0 replies
Comment options

Changing names of rules is considered breaking change

How about rule aliases? As in, having a no-parameter-properties file that just re-exports parameter-properties? At the next major version it could log a deprecation warning, and a major version after that, log an error, and one major version later, stop existing.

Just to clarify: for rules that aren't yet ported, would removing the no- be acceptable?

You must be logged in to vote
0 replies
Comment options

eslint provides api for deprecation and replacing rules

  • deprecated (boolean) indicates whether the rule has been deprecated. You may omit the deprecated property if the rule has not been deprecated.
  • replacedBy (array) in the case of a deprecated rule, specifies replacement rule(s)

https://eslint.org/docs/developer-guide/working-with-rules#rule-basics

You must be logged in to vote
0 replies
Comment options

I don't have "final" answer for you now about usage of prefix.

my opinion is that we should follow eslint rule naming convention

https://eslint.org/docs/developer-guide/working-with-rules#rule-naming-conventions

The rule naming conventions for ESLint are fairly simple:

  • If your rule is disallowing something, prefix it with no- such as no-eval for disallowing eval() and no-debugger for disallowing debugger.
  • If your rule is enforcing the inclusion of something, use a short name without a special prefix.
  • Use dashes between words.

@typescript-eslint/core-team what is your opinion on this?

You must be logged in to vote
0 replies
Comment options

Yeah we need to knuckle out some guidance around this. Most of our no- rules were built because someone had an opinion about a ts feature and nobody had an opinion about ever wanting that feature on!

Now that we are much bigger, we should definitely nut out some guidance docs around this. no- rules only work in rare cases where it's bad to use a feature, or it makes no sense to have an inverse check (as in no-eval or no-explicit-any).

We can deprecate and redirect users to new rules to replace some of the no- rules without doing a major bump, but removing them obviously is breaking.

You must be logged in to vote
0 replies
Comment options

Note also that you can't just rename the rule from no- to "no no-". You also have to flip what "always" / "never" / et al mean.

Not saying it wouldn't be a good change, it's just not simple to do, and can't really be done with aliases unless there is some refactoring done.

You must be logged in to vote
0 replies
Comment options

Just I mention about the deprecation policy in ESLint core.

  • Deprecating a rule happens in a minor version.
  • Deprecated rules are frozen. People can continue to use the deprecated rules, but they don't change the deprecated rules even if bugs exist.
  • They make renaming a rule in a minor version if it's needed because renaming is a combination of deprecating a rule and adding a new rule. (it's not alias.)
  • Some precedents exist in core. For example, no-comma-dangle had been renamed to comma-dangle because it's a rule that enforces the style about trailing commas rather than that disallows trailing commas.

Related references:

You must be logged in to vote
0 replies
Comment options

Should we do a 2.0 and get rid of all the no- rules? Since we don’t have the support guarantees of ESLint core, we should be OK to just remove them.

You must be logged in to vote
0 replies
Comment options

A number of the no- rules make sense as things that won't be turned on and have no inverse, so a blanket rule against no- prefixes is a bad idea.
I agree though that it makes sense for us to advise against that naming if it's feasible that we will add inverse functionality to a rule.

Going through the list (checkmark = imo should be renamed):

  • no-angle-bracket-type-assertion
    • IMO shouldn't have an inverse because the <cast>value syntax is essentially deprecated.
  • no-array-constructor
    • has no inverse
    • maybe should be renamed though because it sounds like you shouldn't use any array constructors, when it's only enforcing correct usage.
  • no-empty-interface
    • has no inverse
  • no-explicit-any
    • has no inverse
  • no-extraneous-class
    • I doubt anyone would ever want the inverse of this unless they absolutely love java.
    • would be good to rename this so it's clearer though (no-class-as-namespace?)
  • no-inferrable-types
    • could feasibly have an inverse but I don't think it makes sense to have one
  • no-misused-new
    • has no inverse
  • no-namespace
    • would people want an inverse for this? I haven't ever seen namespace used outside of definition files.
  • no-non-null-assertion
    • could have an inverse? (would require typechecking though)
  • no-object-literal-type-assertion
    • inverse seems weird here due to the type-unsoundness which as X introduces?
  • no-parameter-properties
    • needs inverse
  • no-this-alias
    • has no inverse
  • no-triple-slash-reference
    • has no inverse (because /// <ref /> is essentially deprecated)
  • no-type-alias
    • needs inverse
  • no-unnecessary-type-assertion
    • has no inverse
  • no-unused-vars
    • has no inverse
  • no-use-before-define
    • has no inverse
  • no-useless-constructor
    • has no inverse (well technically it does - force every class to have at least a useless constructor, but that seems like a really poor practice).
  • no-var-requires
    • has no inverse that makes sense in TS.
You must be logged in to vote
0 replies
Comment options

i don't see point in rules like:

  • no-non-null-assertion: inverted -> what it will check? that you need null assertion?
  • no-type-alias: inverted -> there should no be inverse, we should have 2nd rule: prefer-type-alias/no-simple-interfaces or something like that

it's better to keep rules simple

You must be logged in to vote
0 replies
Comment options

the problem with two rules is now you can have a state where you're both not allowed to use interfaces and not allowed to use type aliases.

we can have a rule type-definition-style which has an option to pick either interface or type-alias. It's still simple and prevents invalid states.

You must be logged in to vote
0 replies
Comment options

Regarding the change to add options to explicit-member-visibility in #214 I guess the better name for the rule would be member-visibility I assume that this would however be a breaking change.
What's the process/plan for implementing these changes as a whole?

You must be logged in to vote
1 reply
@JoshuaKGoldberg
Comment options

@gavinbarron I was hoping someone would file a specific issue for this one 😄 but nobody ended up filing it. If you (or anybody else!) wants to, feel free to file one - it's a bit outside this discussion but I can see why you'd want it.

Though, to be honest, I wonder if the current name is fine? explicit- is not a no-. The rule is about whether to be explicit... 🤔 not sure.

Comment options

Marking as accepting breaking change PRs per the list in #203 (comment).

You must be logged in to vote
0 replies
Comment options

would like to add that some "no-" rules' names don't lend themselves well to understanding; for example, "no-inferrable-types" can (and has multiple times on my team) give the impression that it does the opposite of what it does. It has been floated that "force-type-inference" would be a good disambiguation.

You must be logged in to vote
2 replies
@JoshuaKGoldberg
Comment options

Hmm, interesting. I'm not on the same page for this one. But feel free to file a new issue if you'd like as I'm closing this discussion.

@bradzacher
Comment options

perhaps a better name would be no-easily-interable-type-annotations
verbose but super clear.

Comment options

Can we at least keep the no- prefix for extension rules? I don't think the benefits of simplifying the APIs apply to rules that are already in ESLint, and this makes migrating from ESLint rules more confusing.

You must be logged in to vote
1 reply
@JoshuaKGoldberg
Comment options

This is reasonable, yes. ✔️

Comment options

For the no- rules that might want to be renamed, filed:

For no-type-alias, https://typescript-eslint.io/rules/consistent-type-definitions exists now. I filed #6036 to consider deprecating no-type-alias.

You must be logged in to vote
0 replies
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
9 participants
Converted from issue

This discussion was converted from issue #203 on November 17, 2022 15:54.

Morty Proxy This is a proxified and sanitized view of the page, visit original site.