Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change Request: Support additional rule metadata for deprecations #18061

Open
1 task done
bmish opened this issue Jan 31, 2024 · 10 comments
Open
1 task done

Change Request: Support additional rule metadata for deprecations #18061

bmish opened this issue Jan 31, 2024 · 10 comments
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint

Comments

@bmish
Copy link
Sponsor Member

bmish commented Jan 31, 2024

ESLint version

8.56.0

What problem do you want to solve?

There are long-time rule properties meta.deprecated and meta.replacedBy that have been intended to document when rules are deprecated and what their replacement rule(s) are. For the most part, usage would look something like this:

module.exports = { meta: { deprecated: true, replacedBy: ['replacement-rule-name'] } };

These properties are often used for generating plugin/rule documentation websites and in documentation tooling like eslint-doc-generator.

But there are some limitations to this current format.

  • Simply providing the replacement rule name as a string doesn't yield much context/explanation of the replacement/deprecation.
  • Some rules provide the replacement rule name with the plugin prefix as in prefix/rule-name while others just provide it as rule-name, which can result in ambiguity about whether the replacement rule is in the same plugin, a different third-party plugin, or ESLint core. For third-party plugins, there's no easy way to automatically determine where their documentation is located or how to link to them.

What do you think is the correct solution?

I would like to propose an extended meta.deprecated / meta.replacedBy rule property schema to reduce ambiguity and allow additional key details to be represented in it, described below as a TypeScript type for clarity:

type RuleMeta = {
  deprecated:
    | boolean // Existing boolean option, backwards compatible.
    | string // General deprecation message, such as why the deprecation occurred. Any truthy value implies deprecated.
    | {
        message?: string; // General deprecation message, such as why the deprecation occurred.
        url?: string; // URL to more information about this deprecation in general.
      };
  replacedBy:
    | readonly string[] // Existing string array option of rule names, backwards compatible.
    | readonly {
        /**
         * Plugin containing the replacement.
         * Use "eslint" if the replacement is an ESLint core rule.
         * Omit if the replacement is in the same plugin.
         */
        plugin?:
          | string // Plugin name i.e. "eslint-plugin-example" that contains the replacement rule.
          | {
              name?: string; // Plugin name i.e. "eslint-plugin-example" that contains the replacement rule.
              url?: string; // URL to plugin documentation.
            };
        rule:
          | string // Replacement rule name (without plugin prefix).
          | {
              name?: string; // Replacement rule name (without plugin prefix).
              url?: string; // URL to rule documentation.
            };
        deprecation?: {
          message?: string; // Message about this specific replacement, such as how to use/configure the replacement rule to achieve the same results as the rule being replaced.
          url?: string; // URL to more information about this specific deprecation/replacement.
        };
      }[]
    | undefined;
};

Real-world example of how this could be used based on the situation in #18053:

// lib/rules/semi.js
module.exports = {
  meta: {
    deprecated: {
      message: 'Stylistic rules are being moved out of ESLint core.',
      url: 'https://eslint.org/blog/2023/10/deprecating-formatting-rules/',
    },
    replacedBy: [
      {
        plugin: {
          name: '@stylistic/js',
          url: 'https://eslint.style/',
        },
        rule: {
          name: 'semi',
          url: 'https://eslint.style/rules/js/semi',
        },
      },
    ],
  },
};

This data could be used by documentation websites and tooling like eslint-doc-generator to generate notices like:

semi (deprecated)
Replaced by semi from @stylistic/js.
Stylistic rules are being moved out of ESLint core. Read more.

We can also support the same meta.deprecated and meta.replacedBy properties on configurations and processors (the other kinds of objects exported by ESLint plugins), replacing rule with config or processor as needed.

In terms of actual changes needed for this:

This proposal is inspired by:

Related:

Participation

  • I am willing to submit a pull request for this change.

Additional comments

No response

@Rec0iL99
Copy link
Member

Rec0iL99 commented Feb 5, 2024

ping @eslint/eslint-team

@Rec0iL99
Copy link
Member

Rec0iL99 commented Feb 5, 2024

@kecrily please tag @team when moving an issue across the board. I think folks did not get a notification for this.

@nzakas
Copy link
Member

nzakas commented Feb 5, 2024

Yes, that's true, and "Evaluating" is meant to be when you, personally, are evaluating it. That means you should leave a comment indicating that's what is happening.

If instead your intent was to ask other team members for their opinion, then you should also leave a comment and move to "Feedback Needed."

In any event, please don't just move the issue into a different column without comment. :)

@nzakas
Copy link
Member

nzakas commented Feb 5, 2024

My opinion: This is worth exploring, but we should do it through an RFC to make sure it gets the appropriate amount of attention and exploration.

Would like some other opinions.

@JoshuaKGoldberg
Copy link
Contributor

Is there a strong motivation to have two properties here? As in, is there a time when a rule would reasonably have replacedBy but not deprecated? I don't think I've ever seen that in the wild.

If the deprecated property is getting more metadata and becoming an object, would that RFC be a good place to discuss making replacedBy a property of that object?

@bmish
Copy link
Sponsor Member Author

bmish commented Feb 5, 2024

If we were designing this from scratch, then yeah I'd likely put everything inside the deprecated object. I'm leaning toward keeping the two existing properties for backwards-compatibility, but this potential redesign is a good call out that I could mention in the "Alternatives" section of the RFC.

Would be good to hear any other preferred design choices before I write up the RFC.

@bmish
Copy link
Sponsor Member Author

bmish commented Feb 5, 2024

Thinking about it some more, perhaps I can just leave replacedBy alone as a string array for backwards-compat, possibly deprecating it, while adding the more sophisticated object array version of it as a new replacedBy property inside the deprecated object for the purpose of consolidating things.

@JoshuaKGoldberg
Copy link
Contributor

The only other point that comes to mind for me is being able to specify options in the new rule. https://github.com/typescript-eslint/tslint-to-eslint-config -> https://github.com/typescript-eslint/tslint-to-eslint-config/blob/470d44de20beb7c7366de993edb8898d0766b8aa/docs/Architecture/Linters.md might be a good reference. Deprecated rules could map to multiple new ones, including specifying options and changed or missing functionality.

Not suggesting rules should include a transformer function themselves - just posting as examples of what information users might want to know.

@nzakas nzakas added the accepted There is consensus among the team that this change meets the criteria for inclusion label Feb 6, 2024
@nzakas
Copy link
Member

nzakas commented Feb 6, 2024

It seems like there's interest in pursuing further, so marking as accepted and moving to "Waiting for RFC".

@bmish
Copy link
Sponsor Member Author

bmish commented Feb 21, 2024

I put together an RFC!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint
Projects
Status: RFC Opened
Development

No branches or pull requests

4 participants