fixable property only necessary when meta is present

[brettz9]

What did you expect to happen?

Per the docs:

Important: Without the fixable property, ESLint does not apply fixes even if the rule implements fix functions. Omit the fixable property if the rule is not fixable.

Based on this, I would expect a rule without any meta at all to also not be fixable.

What actually happened? Please include the actual, raw output from ESLint.

A rule without meta is successfully fixed when it has a fixer.

The docs are true, however, if there is a meta object of some sort (including an empty object); in such a case, an error will be reported that fixable is necessary if the rule attempts to add a fixer. But if there is no meta object at all, fixable is not necessary for fixes to be applied for the rule.

I am not expecting breaking changes to start preventing fixers without meta; I'd just expect the docs to mention that rules still without the recommended meta property at all are currently still fixable.

[kaicataldo]

@brettz9 Can you fill out an issue template? It's not clear to me what the desired outcome of this issue is. Thanks!

[brettz9]

I've updated to include those two headings relevant to a doc bug.

[kaicataldo]

Thank you for clarifying. Agreed, and PRs to improve documentation are always welcome!

[anikethsaha]

I am not expecting breaking changes to start preventing fixers without meta;

agreed. but shouldn't it?

IMO, the fixer shouldnt work or throw an error if meta is not present.
cause in rule authoring docs, it is mentioned that we can omit docs but it is not mentioned about the meta. (or maybe I am missing.)

thoughts?
otherwise I think bug label can be removed then ?

[brettz9]

Re: it not mentioning meta, yeah, the Working with Rules page doesn't mention that meta can be omitted.

As far as whether it should be a breaking change to require meta.fixable always, I'd personally even be in favor of always requiring meta (specifically so as to require meta.type even for non-fixable rules--though that is not required now and would perhaps also need an "other" type; without getting too off track, in my eslint badge-maker, I felt the need to let users override the built-in type's as they are not very specific (e.g., as to whether the "problem" is for security, preventing obtrusiveness, etc); I could perhaps use docs.category for that instead, but these are not enumerated in the docs nor encouraged for third-party use).

However, I have come across quite a number of plugins using the legacy format or not using meta if they are, so I would expect people might worry about breaking too many old plugins. Might be good to mention it may become required in a future version, both to encourage meta (and meta.type) in case a breaking change is ever seen as feasible and desirable.

[sovoid]

Hey guys I am looking to contribute to ESLint. I would like to work on this issue however based on what I read, I am not sure if this requires just a documentation fix. Can anyone appropriately guide me to work on it. I have already set up ESLint on my machine.

[kaicataldo]

This is marked as accepted for a documentation update, though I do think this is unclear behavior.

[ajkovar]

Hi. I'm interested in taking this one if no one minds. Seems a pretty straight forward documentation update.

[matthenschke]

Is anyone working on this issue? If not, I would not mind working on it.

[btmills]

I have come across quite a number of plugins using the legacy format or not using meta

@brettz9 do you happen to know of any examples that currently rely on this buggy behavior by providing fixers without including meta?

I see three options to resolve this:

1. Fix the bug and require meta.
2. Document the existing behavior with a warning that we'll fix it in a future major version.
3. Document the existing behavior and live with it even though it's unintended.

Which path I prefer would depend on how much fixing the buggy behavior would break.

[brettz9]

Authors I've encountered have thankfully been open to updates, e.g., recently with SonarSource/eslint-plugin-sonarjs#161 . (In the cases where the lack of meta.fixable is due to using the old format where there is just a function exported and no create as well as no meta (i.e., when the old format is in use), authors might see this less as "buggy" and more as merely "deprecated" since those rules predated meta entirely, but I get your gist, esp. with the docs urging this now.)

I am waiting on one PR, Turbo87/eslint-plugin-chai-expect#99 , to introduce the new rule format, so I know at least one plugins still using the old format, but in this case, the rules do not happen to be fixable.

If you were only going to fix the case of requiring meta.fixable when create was present, I would think you might avoid a major bump (assuming the docs when the new rule format was introduced had from the beginning added mention of this need).

However, I would think no. 1 on your list is not an option for a major bump when the fixing occurs with the deprecated rule format since that format (which did not even allow meta at all despite supporting fixing) is still mentioned as supported (though deprecated) at https://eslint.org/docs/developer-guide/working-with-rules (beginning of page).

While I haven't done any exhaustive study (e.g., of https://github.com/dustinspecker/awesome-eslint/ or those on npm), I would think therefore that with no. 1 excluded, no. 2 could be chosen over no. 3 as:

1. At least the big ones I have used or needed to use with other projects, "standard", "prettier", "promise", "import", "jsdoc", "sonarjs" and other ones of interest to me (e.g., "mocha", "mocha-cleanup", various chai ones) are pretty much all on board already.
2. It's not too big of a hassle to just add a create method and if needed add a fixable.
3. The blog post introducing the new rule format (at https://eslint.org/blog/2016/07/eslint-new-rule-format ) mentions "knowing that a rule is fixable ahead of time allows us to optimize the autofixing process as well as call this out in documentation" so it should be nice to be able to enforce this benefit (even if there may be some plugins or custom rules in the wild that have yet to make the change).

IMO, it would be nice to also require meta.type (and meta.docs (url and/or description) too if the latter were worded less as being ESLint-project-specific). Even for non-fixable rules, a type can be used by formatters, so that tools could more readily make use of them. Authors I have encountered have been open to adding the former or have at least not objected for the few PRs I have still waiting merging (I haven't made PRs for docs). Don't want to distract with this suggestion, but just mentioning if a breaking change is being made anyways and there were interest.

[nzakas]

Because we don't guarantee fixes to be applied, I think we can fix this behavior to work as it was originally intended. The result of using a rule with a legacy fixer is that it will only warn and not fix, which I wouldn't consider a breaking change.

The bigger question is why RuleTester isn't testing for this already. That's one of our primary ways of enforcing rule standards and I'm surprised there's not a check in there.

Here's what I would suggest as a path forward:

1. Make an announcement about this explaining that it is a bug and we know it will affect people.
2. Update RuleTester to check that meta.fixable is present whenever a fix is provided. Release this first without changing how rules actually work.
3. Fix the bug with how rule fixes are being applied in a subsequent release.

[mdjermanovic]

If there is consensus on these steps, I could start working on RuleTester.

[btmills]

3 of the other 4 TSC members left a 👍 on @nzakas’s comment, and nobody’s objected, so I’d say you’re good to go.

[mdjermanovic]

I think the documentation is still incorrect, even if there's meta without fixable:

Important: Without the fixable property, ESLint does not apply fixes even if the rule implements fix functions. Omit the fixable property if the rule is not fixable.

Important: Unless the rule exports the meta.fixable property, ESLint does not apply fixes even if the rule implements fix functions.

It sounds like ESLint will just not apply fixes, but it actually throws when this happens. This was an intended change (#6043), so perhaps we should update the docs to be more accurate about what will actually happen.

If we want the same behavior in the case when rule that provides fixes doesn't have meta (which is what this issue is about), then ESLint should throw in that case as well. Maybe we should add this as step 4 and implement it in the next major version?

[brettz9]

Also to confirm: this doesn't apply to legacy-format rules (those that export a function and thus don't have meta)?

Per @nzakas in #13349 (comment)

...The result of using a rule with a legacy fixer is that it will only warn and not fix, which I wouldn't consider a breaking change.
The bigger question is why RuleTester isn't testing for this already...

While this is speaking of only warning, it seems to me from this indication of (no longer) fixing, that the idea is to start treating legacy format rules the same, i.e., that all your steps should apply to both the legacy format (functions) and to missing meta on objects (as with an object missing meta.fixable) and also that another step should be added, presumably preceding your current step 3, that avoids fixing for legacy (function) format.

[mdjermanovic]

I interpreted this comment differently since it wasn't originally intended to disallow fixable legacy-format rules.

I'm not sure whether we should now break legacy-format rules that are following the spec for that format (working-with-rules-deprecated).

@nzakas can you clarify this? I think we all agree to disallow new-format rules that produce fixes but don't export meta. The question is what to do with legacy-format rules.

[mdjermanovic]

As clarified in today's TSC meeting, steps 1-3 will apply to legacy (function) format as well.

Meaning that, as of v8.0.0, only rules that export meta.fixable (which implies that it's a new-format rule, and that it exports meta) can be fixable. In v8.0.0, an error will be thrown during linting whenever a rule that doesn't satisfy this condition produces a fix.

[mdjermanovic]

and also that another step should be added, presumably preceding your current step 3, that avoids fixing for legacy (function) format.

I think this isn't necessary, similar to how it seems unnecessary for new-format rules that don't export meta.

[mdjermanovic]

Added "breaking" label for the remaining step 3.