V4 Planned Changes

[bmish]

I wanted to put together a list of the changes I'm hoping to see in the next major version. What do you think?

• Upgrade to eslint-utils v3 (Chore: Update eslint-utils to v3 #148)
• Drop Node 10 support and other old versions (Node 10 end-of-life 2021-04 and Node 15 end-of-life 2021-06-01) (Breaking: Drop support for Node 10, 13, 15 #151 Breaking: Drop support for old minor versions of Node 12 and 14 #212)
• Switch to pure ES modules? Probably not practical yet.
• Support ESM rules support esm #157
• Support TS rules TS support #176
• Stricter detection of function-style rules Breaking: Reduce false positives by only detecting function-style rules when the function returns an object #211 Fix: Only consider functions with a single argument as function-style rules #213
• Upgrade dev-dependencies Update all dev-dependencies ahead of v4 release #188
• Update rule options:
  • Update consistent-output rule option default to always since it's always best practice to assert the output (Breaking: Rename consistent-output rule to require-test-output #150)
  • Add meta.hasSuggestions property to meta-property-ordering rule (Breaking: Add meta.hasSuggestions property to meta-property-ordering rule #119)
• Update existing rules to handle suggestions functions
  • Breaking: fixer-return should be updated to apply to suggestion fixer functions too #190
  • Breaking: Message rules should be updated to apply to suggestion messages #191
  • Breaking: report-message-format should apply to suggestion messages #192
  • Breaking: prefer-replace-text should be updated to apply to suggestion fixer functions too #193
• Add fixer-return to rules preset (Breaking: Add fixer-return to rules preset #199)
• Add new recommended rules (Breaking: Enable additional recommended rules for v4 #152):
  • I would actually add ALL existing rules. I've used all of this plugin's rules successfully in a number of my eslint plugins, and I find them pretty much all useful and agreeable. Only about 1/3 of existing rules are enabled as recommended right now, and I see this as a missed opportunity, since most people only bother to enable the recommended rules. Enabling more rules means more exposure/battle-hardening of the rules and more benefit to users.
  • consistent-output
  • no-deprecated-context-methods
  • no-only-tests
  • prefer-object-rule
  • require-meta-has-suggestions
  • require-meta-schema
  • require-meta-type
• I'll open PRs for all these changes.
• Check for any other high-priority rule proposals we should implement and enable.
• Before the major release, let's release any non-breaking changes in a patch/minor version first.

[bradzacher]

I would actually add ALL existing rules. I've used all of this plugin's rules successfully in a number of my eslint plugins, and I find them pretty much all useful and agreeable.

The problem you've got is "pretty much".
The recommended set needs to be unopinionated.
The last thing you want is for someone to disable a rule that's recommended.

Examples of rules that are opinionated: require-meta-type, report-message-format, test-case-property-ordering, test-case-shorthand-strings.

Speaking from experience with @typescript-eslint - you want the recommended set to just be an unopinionated set of rules that catch problems.

[aladdin-add]

there is already a preset so you can extend like:

extends: ["plugin:eslint-plugin/all"]

[bmish]

@bradzacher if the goal is to leave "opinionated" or "stylistic" rules (i.e. about the ordering of properties) out of the recommended config, I'd be fine with that.

Here are the rules I would specifically advocate for being added to the recommended config because they improve rule usability or facilitate eslint plugin development:

Rule	Why it could make sense in the recommended config
consistent-output	Asserting output is required by ESLint 7 when there's an autofix, and even when there's no autofix, it's still valuable to assert that there's no autofix.
no-deprecated-context-methods	We should help people avoid deprecated APIs.
prefer-object-rule	The old rule style is deprecated.
prefer-output-null	It's significantly easier to distinguish if a test case has an autofix or not when using output: null.
prefer-placeholders	See rule doc about benefits.
prefer-replace-text	Improved readability.
require-meta-docs-description	It's easier for users to understand rules when they have descriptions. But this rule currently requires descriptions to begin with enforce, require, or disallow which might make it too opinionated for the recommended config if people often use other description formats, so we could enable it with pattern: '.+' to allow any description.
require-meta-docs-url	It's easier for users to understand rules when they provide a link to their documentation.
require-meta-has-suggestions	This new property will be required for suggestable rules in ESLint 8.
require-meta-schema	Enforcing rule schemas reduces the risk of users configuring a rule incorrectly, and even rules that have no options should provide schema: [] so that eslint can validate that no options are passed to them.
require-meta-type	This enables users to choose what type of rule they want to run (based on the three types used by eslint core: problem, suggestion, or layout). However, if users commonly use types other than these, then I agree this rule could be considered too opinionated to include in the recommended config.

Here are rules which could be considered too opinionated/stylistic to include in the recommended config.

Rule	Why it may be too opinionated / stylistic to include in the recommended config
meta-property-ordering	Enforces an opinionated ordering of properties.
prefer-message-format	Enforces an opinionated (albeit pretty minimal) format (starts with capital letter, ends of period) for error messages.
test-case-property-ordering	Enforces an opinionated ordering of properties.
test-case-shorthand-strings	Enforces using a string instead of object to make basic test cases more concise. This could be considered an opinionated/stylistic preference at odds with test case consistency.

[aladdin-add]

agreed with @bradzacher .

eslint has set a very high bar to eslint:recommended (eslint/eslint#14673). I think better to adopt the similar policy in the plugin. I'm 👍 to the following rules:

• consistent-output
• no-deprecated-context-methods
• prefer-object-rule
• require-meta-has-suggestions
• require-meta-type

[bmish]

Sounds good. Can I ask why you would exclude these ones:

• require-meta-docs-url
• require-meta-schema

I consider these two especially high impact in that they both improve external-facing rule usability by helping developers understand lint rules (by providing an IDE documentation link) and ensuring developers configure rules correctly.

[aladdin-add]

yes, it is a best practice, and should be enabled in most cases, but there are some exceptions, e.g. internal-rules: https://github.com/eslint/eslint/tree/master/tools/internal-rules

[bradzacher]

Example:
None of the ~400 internal rules at Facebook have a schema defined, because they take no options.
Nor do they have a docs url defined, because we have our own infra for dealing with rule docs based solely on the rule ID.

For the wider world - docs url assumes the author has written and maintained docs (not always true).

And yeah whilst technically it's best if you define a schema even without options. It's certainly not a huge deal if you don't when you have no options.

[bmish]

Having no rule options is totally fine. The require-meta-schema rule doc explains that schema: [] should be specified for rules that do not have options in order to enforce that no options are accidentally passed to such rules, and rules that specify schema: [] are allowed by this rule.

[bmish]

Thanks to both of you for bringing up some use cases where a plugin might not want to specify rule URLs. Those are helpful.

[bmish]

I have an update to share regarding the require-meta-schema rule. I have recommended to the ESLint team that schemas should be required for rules with options and they are interested in this idea so I'll be opening an RFC for it targeting ESLint 9.

With this change, rules without options will no longer need to specify schema: [], as that will be the default behavior. That means we can update the require-meta-schema rule to only require the schema property for rules that actually have options (which is the same as how require-meta-fixable only requires the fixable property for rules with fixers).

As a result of this change, we should be able to enable the require-meta-schema rule as recommended in eslint-plugin-eslint-plugin v4.

[aladdin-add]

As eslint v8.0.0 will be released in a few weeks, we can start merging these PRs, and made a prerelease version. thoughts? @bmish

[bmish]

@aladdin-add I believe all PRs for the next release are ready, except for #179 which I can try to get to soon. A prerelease version is a good idea.

[bmish]

Two more notes:

1. We should release a minor version first with these two non-breaking changes: Fix: Remove erroneous schema from require-meta-schema rule #178 and New: Add requireSchemaPropertyWhenOptionless option to require-meta-schema rule #180.
2. We should also consider including TypeScript support (TS support #176) in this release. TS support will build on top of ESM rule support (Breaking: Support ESM rules #177) and I could give it a try, but Breaking: Support ESM rules #177 needs to be merged first for me to do so.

[bmish]

Open question: I have added a requireSchemaPropertyWhenOptionless option (default true to maintain current behavior) to require-meta-schema to address the above concerns. We have a few paths going forward:

1. Leave option enabled for v4 release. Disable it once ESLint changes their default behavior (possibly in ESLint v9) to require schemas for rules with options (since then it will no longer be necessary to specify schema: [] for rules without options).
2. Disable the option in v4 release.

[aladdin-add]

re requireSchemaPropertyWhenOptionless: I'm fine either way. which one do you prefer?

[bmish]

I personally think we should enforce the more strict behavior (keep requireSchemaPropertyWhenOptionless enabled by default on require-meta-schema) for now until it becomes no longer necessary (as a result of a potential future ESLint change).

[bmish]

I realized that a lot of our existing rules need to be updated to handle suggestions. I think it's pretty important that we correct this gap in coverage in this major release, as the suggestions feature has been available for nearly 2 years now. I've opened issues to track these gaps, and I will try to open PRs to address these in the coming days. The changes required should not be that large.

• Breaking: fixer-return should be updated to apply to suggestion fixer functions too #190
• Breaking: Message rules should be updated to apply to suggestion messages #191
• Breaking: report-message-format should apply to suggestion messages #192
• Breaking: prefer-replace-text should be updated to apply to suggestion fixer functions too #193

In the meantime, we should continue to work on getting all the other changes for the major release merged.

[aladdin-add]

👍 nice catch!

[bmish]

@aladdin-add thanks for releasing the prerelease version. I'll spend a few days now testing it and checking for false positives/negatives before the final release.

[aladdin-add]

@bmish I've tested it in a few plugins - all working fine, an excellent job! 🎉

just pushed 4.0.0 - let's wait and see if need patch releases. 😄

[SimenB]

🎉

I opened up a couple of bug reports (#214, #215). Not reduced, can do if needed (won't be until next week, tho)

[bmish]

Release completed! https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/releases/tag/v4.0.0

Will address any bugs separately.