Proposal: support custom message for all no-restricted-* rules and possibly others

[dwelle]

The basic idea is to allow custom message for rules which may not be self-evident and may demand a description of why they exist.

Semi-duplicate of #8315, but I'm opening a new issue to suggest this for (IMO) all the rules that really need this, and to discuss what other rules we could implement this for.

I think all no-restricted* rules need this. no-restricted-syntax and no-restricted-properties rules already have this, but these don't:

• no-restricted-modules
• no-restricted-globals
• no-restricted-imports

Syntax is comparative to the rules where it's already implemented, but the naming of the keys is up for bikeshed. Proposals:

• no-restricted-globals - either a name, variable or identifier for the main key:

  /*
   * eslint no-restricted-globals: [
   *     "error",
   *     {
   *         "name": "self",
   *         "message": "some explanation"
   *     }
   * ]
   */
  

• no-restricted-modules && no-restricted-imports

  /*
   * eslint no-restricted-modules: [
   *     "error",
   *     {
   *         "name": "myModule",
   *         "message": "some explanation"
   *     }
   * ]
   */
  
  // and
  
  /*
   * eslint no-restricted-modules: [
   *     "error",
   *     {
   *         "paths": [{ "path": "myModule", "message": "some message" }],
   *         "patterns": [{ "pattern": "import2/*", "message": "some message" }]
   *     }
   * ]
   */
  

It may be a good idea to implement custom messages for other rules, as well. Thoughts?

[not-an-aardvark]

Thanks for the proposal. This seems like a good idea to me. 👍

[not-an-aardvark]

@eslint/eslint-team This issue has three 👍s -- it just needs a champion to be accepted.

[vitorbal]

Should #8315 be closed in favor of this once it gets accepted?

[not-an-aardvark]

Yes, I think that sounds reasonable.

[kaicataldo]

There's already a PR for this #8315: #8932

I think it's totally fine to do this in multiple PRs, just wanted to raise awareness so that we can ensure all the APIs match for clarity.

[platinumazure]

I'll champion. Marking as accepted.

I think I like a name key better for no-restricted-globals, so I'll update that in my PR at some point.

[platinumazure]

Okay, finally coming back around to this.

Current status:

• no-restricted-imports still needs to be augmented to support custom messages
• no-restricted-modules still needs to be augmented to support custom messages

It's not immediately clear how we should add custom message support for patterns. @ljharb Any ideas on that?

PRs would be very welcome!

[not-an-aardvark]

It's not immediately clear how we should add custom message support for patterns.

What do you mean by "patterns"?

[ljharb]

@platinumazure the OP seems to have that answered for patterns; can you elaborate on the confusion?

[dwelle]

Not sure if I can elaborate on the patterns. Is this a question for implem or API?

[platinumazure]

I'm having trouble with it conceptually since patterns allow for negation which represent patterns that should specifically not be flagged by the rule.

Take this example: no-restricted-imports: ["error", { patterns: ["lib/vendor/**", "!lib/vendor/conf.json", "foo/**"] }]

Seems the first two patterns are related and should have one message between them, ideally (the second pattern is sort of a modification of the first, really). The last gets its own message. Or, if we really want to do one message per pattern, the second one doesn't need a message because it's not actually forbidding anything.

So when implementing a { pattern, message } block in the schema, should that actually be { patterns, message } so we can encourage users to group their patterns that are logically connected? Or can we have one pattern string with multiple patterns (e.g., using comma delimiting)? I'm just not sure what the best way to handle it is.

Similar questions arise for no-restricted-modules.

[ljharb]

That's a good point. I'd say grouping patterns with a message in an object, and allowing multiple objects, makes sense.

[dwelle]

patterns: [{ patterns, message }, { patterns, message }] seems good to me.

So, for it to not be a breaking change, we need to allow mixed objects and strings in the top-level patterns, I suppose: (patterns: [{ patterns, message }, "path/to/module"].

[platinumazure]

@dwelle Thanks, I agree. I think I'll support a top-level paths and patterns array (similar to current schema) and then each item in the array can be a single string pattern/path (with no message), or an object with paths/patterns and a message... I'll work on a PR today and we'll see what people think.

[kaicataldo]

Support for custom messages in paths has landed as of #9413. We still need to implement custom messages for patterns for no-restricted-modules and no-restricted-imports.

[platinumazure]

I'm going to close this and create a new issue for whether we need to support custom messages for patterns in no-restricted-modules and no-restricted-imports. If users don't seem to need them, then we can regard this as solved (until users start needing them).