eslint / generator-eslint

A Yeoman generator to help with ESLint development
Other
227 stars 51 forks source link

Missing meta.messages property #181

Closed MikeMcC399 closed 3 months ago

MikeMcC399 commented 3 months ago

Suggestion

Support the recommended rule eslint-plugin/prefer-message-ids by providing a meta.messages property using a MessageId. Currently, running yo eslint:rule does not add this property to the generated lib/rules/<rule-id>.js rule.

Background

When reporting a rule violation, it's preferred to provide the violation message with the messageId property instead of the message property.

Documentation

nzakas commented 3 months ago

Thanks for the suggestion. Would you like to submit a PR to add this?

MikeMcC399 commented 3 months ago

@nzakas

Would you like to submit a PR to add this?

I'm very new to ESLint and at this time I don't feel that I have the skills and experience to implement the suggestion. Sorry!

I just noticed the issue as I was working on resolving a lot of eslint-plugin-cypress upgrade issues which I started on one month ago.

bmish commented 3 months ago

So you want to add messages: {} with an empty object I assume? I'm open to this if there's interest, since most new rules should be using this property for at least one violation message.

The template is here: https://github.com/eslint/generator-eslint/blob/92a44afb6bb04bd7b247111aa9d0f2c2b07558b2/rule/templates/_rule.js#L13-L22

MikeMcC399 commented 3 months ago

@bmish

So you want to add messages: {} with an empty object I assume?

That would be a minimum and just adding that to the template would be easy to implement.

What I wasn't sure about is how far this should be extended, and if it should be expanded to a dialog which asks for a messageID and a message or assumes some default messageIDand adds a dummy message. That's where my lack of experience in this area breaks down!

bmish commented 3 months ago

I don't think the generator needs to fill in any actual messages. The rule creator probably doesn't have specific messages ready yet when they're just trying to create the barebones rule skeleton.

MikeMcC399 commented 3 months ago

@bmish

So I suggest to add:

    messages: {} // Add messageId and message

to the template. The linting message will stay the same, however adding the above line into the template means that rule creators should be able to find the place faster, where they need to add a message. No additional information is requested at rule creation time with yo eslint:rule.

Linting error:

error meta.messages must contain at least one violation message eslint-plugin/prefer-message-ids

If that sounds good, then I will submit a PR for this change.

bmish commented 3 months ago

That sounds reasonable.

MikeMcC399 commented 3 months ago

@bmish

Should this be a chore (no release), fix (patch release) or feat (minor release)?

It doesn't change any error message, so from that point of view it could be a chore. On the other hand it won't get used unless there is a release.

bmish commented 3 months ago

feat should be fine. It's not fixing anything, but it is a possible usability improvement.

MikeMcC399 commented 3 months ago