Add experimental support for custom message arguments

[ybiquitous]

Which issue, if any, is this issue related to?

Closes #4117

Is there anything in the PR that needs further explanation?

This PR is still a PoC. I appreciate any comments or suggestions.

Summary:

• stylelint.utils.report() can receive a message function and an array of messageArgs, in addition to just a message string
• Only JS config file support (e.g. .stylelintrc.js)
• For JSON (or YAML), the message property can receive parameters like %s for string interpolation. The Node.js util.format() API is internally used.

Considerations:

• Should we support other formats like JSON, YAML, etc.? If doing so, another method than a message function might be necessary; for example, some template syntax like message: "a={a}, b={b}".
• It's necessary to change report() call in all the rules in this PR. This would be a tedious job. Is there a better API instead of adding messageArgs?
  • Now, we can proceed gradually to add messageArgs to each rule implementation.

Example:

.stylelintrc.js:

module.exports = {
  rules: {
    'custom-property-pattern': [
      '^([a-z][a-z0-9]*)(-[a-z0-9]+)*$',
      {
        message: (pattern) => `Expected custom property name to be kebab-case; pattern=/${pattern}/`,
      },
    ],
  },
};

Or .stylelintrc.json:

{
  "message": "Expected custom property name to be kebab-case; pattern=/%s/"
}

a.css:

a {
  --CustomProperty: 1;
}

Run:

$ stylelint a.css

a.css
 2:3  ✖  Expected custom property name to be kebab-case; pattern=/^([a-z][a-z0-9]*)(-[a-z0-9]+)*$/  custom-property-pattern

1 problem (1 error, 0 warnings)

[changeset-bot]

🦋 Changeset detected

Latest commit: fb47cf7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
stylelint	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Mouvedia]

Only JS config file support (e.g. .stylelintrc.js)

I am using .stylelintrc in my projects; will it be supported?

Should we support other formats like JSON, YAML, etc.?

The minimum for the PoC would be JSON IMO.

If doing so, another method than a message function might be necessary

What about

message: '(pattern) => `Expected custom property name to be kebab-case, but actual "${pattern}"`'

?
i.e. using the result of calling toString on the function
which can be reversed by

eval('(pattern) => `Expected custom property name to be kebab-case, but actual "${pattern}"`')

[ybiquitous]

@Mouvedia Thanks for the feedback. JSON support is still essential.

Perhaps, using eval may cause some security problems. So we need to get a better idea, not using a just JS function.

[Mouvedia]

Perhaps, using eval may cause some security problems. So we need to get a better idea, not using a just JS function.

message: [
	'pattern',
	'return `Expected custom property name to be kebab-case, but actual "${pattern}"`'
]

i.e. the last element is always the body (exactly like Function)

for the reversal:

Function('pattern', 'return `Expected custom property name to be kebab-case, but actual "${pattern}"`')

i.e. you pass all the elements in the same order
e.g. Function.apply(null, message);

[ybiquitous]

Using Function() sounds interesting, but it also may cause problems like eval(). For example, when using Content-Security-Policy, such unsafe expressions may be disallowed.

See also:

• CSP: script-src - HTTP | MDN
• no-new-func - ESLint - Pluggable JavaScript Linter

[Mouvedia]

when using Content-Security-Policy

Is stylelint being run in browser context?

[ybiquitous]

Not now, but it may be realized in the future (see #4796).

[ybiquitous]

Here is an example of a template syntax (in JSON), using {} characters for interpolation. The idea of specifying an array is inspired by @Mouvedia:

{
  "message": [
    "pattern", // message arguments
    "Expected custom property name to be kebab-case, but actual \"{pattern}\"" // message template
  ]
}

---

EDIT: People can specify arbitrary argument names like this:

{
  "message": [
    "foo", // message arguments
    "Expected custom property name to be kebab-case, but actual \"{foo}\"" // message template
  ]
}

[ybiquitous]

Examples of interpolation characters:

• {value}
• {{value}}
• #{value}
• %{value}
• etc.

[Mouvedia]

Let's keep it simple:

// 0 argument
{
  "message": "Expected …"
  // equivalent of
  "message": ["Expected …"]
}

// 1 argument
{
  "message": [
    "foo",
    "Expected … \"${foo}\""
  ]
}

// n arguments
{
  "message": [
    ["foo", "bar", "qux"],
    "Expected … \"${foo}\"  \"${bar}\"  \"${qux}\""
  ]
}

[ybiquitous]

Oh, I meant to indicate the candidates in #6312 (comment).

Also, I think it's better to avoid ${value} because it's easy to confuse with JS template literals. See the ESLint no-template-curly-in-string rule and the following example:

{
  message: [
    'pattern',
    'Expected custom property name to be kebab-case, but actual "${pattern}"',
                                                              /* ^^^^^^^^^^ not template literal */
  ]
}

The idea of 0 arguments, 1 argument and n arguments in #6312 (comment) sounds good. 👍🏼

[ybiquitous]

I find a more straightforward solution; to use the notation (%s) of util.format(). Here is an example:

{
  "message": "Expected custom property name to be kebab-case, but actual \"%s\""
}

%s is also used by console.log.

[Mouvedia]

%s is also used by console.log.

For the PoC, yes. We should add the support of %d and %f in the backlog.
String support will be enough for now.

[ybiquitous]

The eddbb4b commit adds JSON support using util.format(), and I've updated the PR description. It is now ready to review.

[ybiquitous]

To-dos

• Update the usage document in this PR
• [] Add messageArgs to each rule implementation (but it's not required in this PR)

[Mouvedia]

related: #5170

[jeddy3]

This is fantastic work, thank you!

Update the usage document in this PR

Let's classify this as an experimental feature (like we did for `--fix) so that we can treat any issues that may emerge as non-breaking. It's possible this will happen when we add support to more rules or plugin authors start to make use of the feature.

As with exposing our reference data, we should only add support to a rule when a user requests it as we'll need to discuss and lockdown the message format.

I suggest we add support to color-no-hex, rather than custom-property-pattern, in this pull request as it was one of the rules originally requested in the issue.

Let's then add the following to the color-no-hex rule README:

"The message secondary option can accept the arguments of this rule."

It'll be the last item in the extended description, i.e. below the following if it's present:

"The fix option can automatically fix all of the problems reported by this rule.".

We can also extend the description of the message option to say something along the lines of:

"Experimental feature: some rules support message arguments. For example, when configuring the color-no-hex rule the hex color can be used in message string:

.stylelintrc.js:

{
  'color-no-hex': [true, {
    message: (hex) => `Don't use hex colors like "${hex}"`,
  }]
}

.stylelintrc.json:

{
  "color-no-hex": [true, {
    "message": "Don't use hex colors like '${%s/'"
  }]
}

[ybiquitous]

@jeddy3 Thanks for the feedback. Your suggestion sounds good, so I'll implement it.

By the way, I chose custom-property-pattern because the rule is used in scripts/visual-config.json, and it's easy to try the new behavior in the scripts/ directory. No problem to start with any rules to me. 👍🏼

[ybiquitous]

[note] I think a description of %s is necessary. Please let me know if you have a better expression.

See also the util.format on the Node.js documentation.

[ybiquitous]

[note] Unless the disabling comment, Prettier would format the snippet like this:

{
  "color-no-hex": [
    true,
    {
      "message": "Don't use hex colors like \"%s\""
    }
  ]
}

Compared to the snippet above for .stylelintrc.js, it would not be easier to read.

[ybiquitous]

@jeddy3 I've done updating the doc. I'd appreciate your review when you have time.

[Mouvedia]

remark

If I understand correctly, the order will be arbitrary.
e.g. if you don't use --fix but you want to use the fixed value in the message (this rule is not a good example because it's not fixable)

i.e.

• we would generate the value but not edit the file in place
• the user will need to know that the bad value is the first element and the good value is the second element

related

• ESLint --fix-dry-run
• --fix-dry-run option #4005

[ybiquitous]

@Mouvedia Thanks for the share. Other problems might be found than you pointed out, but this feature is still experimental. We would try considering it based on reactions or feedback later.

[jeddy3]

LGTM, thank you!