Add url secondary option to any rules

[emmacharp]

What is the problem you're trying to solve?

Some generic rules (e.g. rule-selector-property-disallowed-list) can be used multiple times, for different reasons, but can link to only one URL. As it is, rule authors cannot point to the relevant documentation for each different use of the rule.

What solution would you like to see?

Enabling the use of custom URLs in the config file, like with messages.

[ybiquitous]

@emmacharp Thanks for the proposal. Sounds interesting to me.

Like the message secondary option, a new option (e.g. url) seems to be as follows:

const config = {
  rules: {
    'custom-property-pattern': [/^[a-z]+$/, {
      url: 'https://your.custom.rule/resource', // as a string
    }],
    'color-no-hex': [true, {
      url: (rule) => `https://your.custom.rule/${rule}`, // as a function
    }]
  }
};

Does the example above satisfy your requirements?

[emmacharp]

Yes, it seems like it does!

Thanks a bunch for the quick response. Looking forward to put that into action!

[ybiquitous]

Got it. Let's wait for other member's feedback.

[ybiquitous]

There seem no objections, so let's transition to "ready to implement".

Rule URLs are used only by a few formatters (for reporting). We can change the logic. E.g.

stylelint/lib/formatters/verboseFormatter.mjs

Lines 142 to 148 in 5e2eff7

	function ruleLink(rule, metadata) {
	if (metadata && metadata.url) {
	return terminalLink(rule, metadata.url);
	}
	return rule;
	}

if (typeof ruleConfig.secondaryOptions.url === 'function') {
  // `ruleConfig` can be retrieved from `returnValue.results[0]?._postcssResult?.stylelint.config?.rules`
  const url = ruleConfig.secondaryOptions.url({ name: rule, config: ruleConfig, message: reportedRuleMessage, ... });
  // ...
}

[github-actions]

This issue is older than one month. Please ask before opening a pull request, as it may no longer be relevant.

[emmacharp]

@ybiquitous, I'm having a try at implementing this and can't seem to get anything in the results array you specified above...

Maybe I'm being dense but if a try and log it in the verbose formatter, it always return undefined.
Am I using it wrong?

Thanks for the help!

[ybiquitous]

@emmacharp Can you share your branch?

[emmacharp]

You are fast, @ybiquitous! Hehe. Sorry for the delay. I didn't commit anything since I was only experimenting.

I tried this:

function ruleLink(rule, metadata, returnValue) {
	console.log(returnValue.results[0]);

	if (metadata && metadata.url) {
		return terminalLink(rule, metadata.url);
	}

	return rule;
}

and then added returnValue in verboseFormatter function:

output += dim(`  ${ruleLink(rule, meta, returnValue)}: ${list.length}${additional}\n`);

I also tried logging directly in the verboseFormatter function with the same undefined result.

If this message is not clear I can push this code to my fork and share it...

[ybiquitous]

Ah, I understand my explanation is not enough. 😅

Probably, we may have to save url properties in secondary options like message or severity properties.

stylelint/lib/lintPostcssResult.mjs

Lines 102 to 105 in a5965a0

	postcssResult.stylelint.ruleSeverities[ruleName] =
	(secondaryOptions && secondaryOptions.severity) || defaultSeverity;
	postcssResult.stylelint.customMessages[ruleName] = secondaryOptions && secondaryOptions.message;
	postcssResult.stylelint.ruleMetadata[ruleName] = ruleFunction.meta || {};

Users cannot change a rule metadata.

[emmacharp]

I see. I'll have a look there. But maybe it's above my pay grade. hehe.

But then, about my previous question, shouldn't I be able to access returnValue.results[0]in the verboseFormatter function?

For instance, ant the bottom of the function like this:

        ...
	console.log(returnValue.results[0]);
	return `${output}\n`;
}

Sorry for the inconvenience. If I can't get it to work, I'll leave this to professionals and leave you be!

[ybiquitous]

@emmacharp Ah, I understand what you mean in the questions above. When a formatter function is called, lint results are not yet set to returnValue.results. See the code:

stylelint/lib/prepareReturnValue.mjs

Lines 67 to 69 in a5965a0

	returnValue.report = formatter(stylelintResults, returnValue);
	returnValue._output = returnValue.report; // TODO: Deprecated. Remove in the next major version.
	returnValue.results = stylelintResults;

I forgot why, but I guess we cannot easily change the formatter function signature for backward compatibility. Formatter authors can use the first argument of a formatter function to access lint problems reported.

[emmacharp]

Thanks for heads up, @ybiquitous! I'll see if I can make progress and I'll report back. Any pointers as to where/how I should try and implement this (if you think of anything) will be welcome.

[emmacharp]

Reporting back, @ybiquitous !

Here are the changes (in progress) made: 301bec8.
I have two questions as of now:

1. I can access the url through the rules property mentioned above (not sure I did right though) but I wonder if url should be treated as the message in the Warning object. Any thoughts?
2. To make it work, I had to add url in the possible options in a rule... but if a custom url is possible on any rule, I guess there would be a better way to go about it?

Not sure it's done right but hope it is. Hehe.
Thanks!

[ybiquitous]

@emmacharp The commit looks good. Can you open a PR if you're ready?

Rather than the message property, adding a new url property to the Warning object is better. E.g.

{
  "line": 1,
  "column": 1,
  "endLine": 10,
  "endColumn": 2,
  "rule": "unit-disallowed-list"
  "severity": "error",
  "text": "Unexpected unit \"px\" (unit-disallowed-list)"
+ "url": "https://stylelint.io/user-guide/rules/declaration-property-unit-disallowed-list"
}

This addition will benefit for the string and json formatters, too.

[emmacharp]

@ybiquitous, yes that's what I thought about.

But I'm not sure how/where to set the value of a url property in the Warning object from the config . Can you give me a pointer?

And do we need to add the url secondary option in every rule or is there a more efficient way to do it (like the custom messages, for instance) ?

I'll gladly open a PR afterwards!

[ybiquitous]

@emmacharp You can set url to warningProperties as customMessages:

stylelint/lib/utils/report.mjs

Lines 119 to 125 in a5965a0

	const { customMessages } = result.stylelint;
	const warningMessage = buildWarningMessage(
	(customMessages && customMessages[ruleName]) || message,
	messageArgs,
	);
	result.warn(warningMessage, warningProperties);

[emmacharp]

Super, I'll have a look at this. Thanks!

[emmacharp]

Hi @ybiquitous !

It's been a while but I've come back to this and I think I understand what's going on here. Still, I wonder: customMessagesand the result of warn() are strings. If I want to include an url there, I'll have to change the type to an array, for instance, changing the definition of warn(). It seems to be quite risky, is it not?

I don't see how to do it another way but maybe I'm mistaken.

Would be glad to have your thoughts on this!

[ybiquitous]

@emmacharp It'd be great if you could come back! My explanation might be wrong, so let me explain again. My rough idea on this feature is as follows:

• Add customUrls (like customMessages) to StylelintPostcssResult. This will store url secondary option values in users' configuration.

  • type StylelintPostcssResult = {
      customUrls: { [ruleName: string]: string }; // to keep it simple, only string is given.
    };

  • stylelint/types/stylelint/index.d.ts

    Lines 156 to 159 in e361561

    	export type StylelintPostcssResult = {
    	ruleSeverities: { [ruleName: string]: RuleSeverity };
    	customMessages: { [ruleName: string]: RuleMessage };
    	ruleMetadata: { [ruleName: string]: Partial<RuleMeta> };

  • stylelint/lib/lintPostcssResult.mjs

    Lines 100 to 103 in e361561

    	postcssResult.stylelint.ruleSeverities[ruleName] =
    	(secondaryOptions && secondaryOptions.severity) || defaultSeverity;
    	postcssResult.stylelint.customMessages[ruleName] = secondaryOptions && secondaryOptions.message;
    	postcssResult.stylelint.ruleMetadata[ruleName] = ruleFunction.meta || {};

• Add url? to WarningOptions.

  • type WarningOptions = {
      url?: string;
    };

  • stylelint/types/stylelint/index.d.ts

    Lines 171 to 175 in e361561

    	export type WarningOptions = PostCSS.WarningOptions & {
    	stylelintType?: string;
    	severity?: Severity;
    	rule?: string;
    	};

• Add url? to Warning.

  • type Warning = {
      url?: string;
    };

  • stylelint/types/stylelint/index.d.ts

    Lines 480 to 501 in e361561

    	export type Warning = {
    	/**
    	* The line of the inclusive start position of the warning.
    	*/
    	line: number;
    	/**
    	* The column of the inclusive start position of the warning.
    	*/
    	column: number;
    	/**
    	* The line of the exclusive end position of the warning.
    	*/
    	endLine?: number;
    	/**
    	* The column of the exclusive end position of the warning.
    	*/
    	endColumn?: number;
    	rule: string;
    	severity: Severity;
    	text: string;
    	stylelintType?: string;
    	};

• In report(), set a custom URL to warn() as a warning option.

  • const { customUrls } = result.stylelint;
    if (customUrls.[ruleName]) {
      warningProperties.url = customUrls[ruleName];
    }
    result.warn(warningMessage, warningProperties);

  • stylelint/lib/utils/report.mjs

    Lines 115 to 121 in e361561

    	const { customMessages } = result.stylelint;
    	const warningMessage = buildWarningMessage(
    	(customMessages && customMessages[ruleName]) || message,
    	messageArgs,
    	);
    	result.warn(warningMessage, warningProperties);

• In createPartialStylelintResult(), copy a URL from warnings to messages.

  • stylelint/lib/createPartialStylelintResult.mjs

    Line 64 in e361561

    return {

Please feel free to comment if you still have any questions. 😃

[emmacharp]

Oh wow. You pretty much did the work for me... I'm kind of sorry if you felt you had to. Heh.

I'll look into this later in the day.
Thank you for the support, as always!

[emmacharp]

It works!

I must admit to being confused by "messages" which seems to mean two different things if I'm not mistaken (first the warning message as a whole and then the text message inside of it)?

Now, I think I may have to add the url option to every rule, is that right? Or is there a more efficient way?

I'll make a pull request afterwards.
Thanks, you're a boss!

[ybiquitous]

Mｙimplementation idea would add the support for the url option, but there seem to be no problem. 👍

[emmacharp]

Ok, just to make sure: your implementation does not "implicitly" support an url option, right?

I have to add url to validOptions in every rule, like this:

{
	actual: secondaryOptions,
	possible: {
		ignore: ['comments'],
		url: [isString],
	},
	optional: true,
}

It does not seem to work without it.

[ybiquitous]

No, we don't have to check url in every rule. 🤔

If you are ready for PR, I recommend opening it. We can discuss code on that PR in details.

[emmacharp]

Done! #7743