Add deprecated rule warnings #6561

ybiquitous · 2023-01-05T16:33:53Z

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

Is there anything in the PR that needs further explanation?

To notify deprecation and make the migration easier.
Such warnings are added to a linting result and reporting output.

For example of a report:

$ cd scripts
$ ../bin/stylelint.js visual.css --config=visual-config.json

Deprecation Warning: The "string-quotes" rule is deprecated and will be removed in the next major release.
Deprecation Warning: The "unit-case" rule is deprecated and will be removed in the next major release.

visual.css
 1:9   ✖  Expected "url("foo.css")" to be ""foo.css""       import-notation
 1:13  ✖  Expected single quotes                            string-quotes
 4:9   ✖  Unexpected invalid hex color "#4f"                color-no-invalid-hex
 6:1   ✖  Expected ".foo.bar" to have no more than 1 class  selector-max-class
 7:10  ✖  Unexpected unit "px"                              unit-disallowed-list
 8:3   ⚠  Expected custom property name to be kebab-case    custom-property-pattern

6 problems (5 errors, 1 warning)

To notify deprecation and make the migration easier. Such warnings are added to a linting result and reporting output. For example of a report: ```console $ cd scripts $ ../bin/stylelint.js visual.css --config=visual-config.json Deprecation Warning: The "string-quotes" rule is deprecated and will be removed in the next major release. Deprecation Warning: The "unit-case" rule is deprecated and will be removed in the next major release. visual.css 1:9 ✖ Expected "url("foo.css")" to be ""foo.css"" import-notation 1:13 ✖ Expected single quotes string-quotes 4:9 ✖ Unexpected invalid hex color "#4f" color-no-invalid-hex 6:1 ✖ Expected ".foo.bar" to have no more than 1 class selector-max-class 7:10 ✖ Unexpected unit "px" unit-disallowed-list 8:3 ⚠ Expected custom property name to be kebab-case custom-property-pattern 6 problems (5 errors, 1 warning) ```

changeset-bot · 2023-01-05T16:33:56Z

🦋 Changeset detected

Latest commit: 54a2305

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

jeddy3

@ybiquitous I've made one minor suggestion, otherwise LGTM.

jeddy3 · 2023-01-12T11:17:11Z

lib/lintPostcssResult.js

+ */
+function warnDeprecatedRule(result, ruleName) {
+	result.warn(
+		`The "${ruleName}" rule is deprecated and will be removed in the next major release.`,


Suggested change

`The "${ruleName}" rule is deprecated and will be removed in the next major release.`,

`The "${ruleName}" rule is deprecated.`,

Let's keep it succinct in the terminal to avoid a wall of text.

What about having just one sentence with a list of the matched rules instead of multiple sentences?
e.g. The "string-quotes", "unit-case" and "foo-bar" rules are deprecated and will be removed in the next major release.

Thanks for the advice. I've simplified warning messages via ff83ba8. Here is a screenshot:

I keep multiple sentences because each message may have a reference, and it's hard to include reference(s) in one sentence. See the code below:

stylelint/lib/formatters/stringFormatter.js

Lines 59 to 61 in ff83ba8

if (reference) {

line += dim(` See: ${underline(reference)}`);

}

Instead, I've avoided repeated "Deprecation Warning" messages in each line and appended a " - " prefix to each line.

What about this change?

[suggestion]

Let's take stylelint config standard as the baseline.
Let's say a user resets about 19% of the deprecated rules; that clocks at around 12 lines out of potentially 64.
In that scenario I would prefer a counter and the usage of … or etc. for more than 3.
e.g.

`${count} deprecation warnings for rules ${rules[0]}, ${rules[1]}, ${rules[2]}, etc.`

What about this change?

SGTM, thank you.

@Mouvedia I understand your concern. But, if reference is specified with a rule deprecation, how should we display it with … or etc.?　I believe we need to consider also plugin rules, not only the built-in ones.

Alright, as long as we have an ENV variable—or something similar—that hides those warnings it should be OK.
@ybiquitous Does the --quiet option hide these warnings?

Does the --quiet option hide these warnings?

I'm afraid the answer is no because these warnings are not related to a warning severity produced by lint rules. We currently don't have a way to suppress this kind of warning, except for using a custom formatter.

types/stylelint/index.d.ts

lib/formatters/stringFormatter.js

ybiquitous · 2023-01-14T03:15:07Z

Maybe off-topic, but I found the following description in our developer guide:

stylelint/docs/developer-guide/rules.md

Line 278 in f129f3b

    
           1. Point the `stylelintReference` link to the specific version of the rule README on the GitHub website, so that it is always accessible.

Indeed, the stylelintReference property seems not to be used:

$ git grep -w 'stylelintReference' lib/
lib/createPartialStylelintResult.js:32:                reference: deprecationMessage.stylelintReference,

Should we follow the guideline still? If doing that, this PR change would show a large amount of text for users.

jeddy3

LGTM, thank you!

Should we follow the guideline still?

Narh, as we're making these deprecations as part of a major release. People will find the general migration guide. This advice was for when we'd deprecate the occasional rule in a minor release, and we'd need to direct people to migration steps in each rule's README.

ollie-iterators · 2023-01-17T14:49:35Z

When will this be getting merged? Also, once this PR gets merged, will it cause #6573 to be able to be merged so that stylelint can be updated to version 15.0.0?

jeddy3 · 2023-01-19T16:27:54Z

When will this be getting merged?

The original author usually merges once a pull request has one approval. So, it'll be merged whenever @ybiquitous has time. We're all volunteers so things happen when they happen.

Also, once this PR gets merged, will it cause #6573 to be able to be merged so that stylelint can be updated to version 15.0.0?

Yes, that's likely. I'll update #6409 to bring things together.

ybiquitous · 2023-01-25T13:25:03Z

Because we will deprecate many rules in v15.0.0, some may see many deprecation warnings and feel annoyed. But I believe showing warnings should be better than showing nothing.

Also, if people update stylelint and stylelint-config-{recommended,standard} at the same time, they will not see the warnings, since the configs will remove deprecation rules.

We can consider adding an option to turn off deprecation warnings if people need it.

If there is no other feedback, I will merge this PR within a few days.

jeddy3 · 2023-01-25T14:19:19Z

Also, if people update stylelint and stylelint-config-{recommended,standard} at the same time, they will not see the warnings, since the configs will remove deprecation rules.

Yep, exactly.

If there is no other feedback, I will merge this PR within a few days.

Merging SGTM.

Mouvedia · 2023-01-25T16:15:41Z

We can consider adding an option to turn off deprecation warnings if people need it.

I will for the 15.1.0 version.

ybiquitous · 2023-01-26T12:46:08Z

I will for the 15.1.0 version.

Let's open an issue when it becomes necessary.

lib/lintPostcssResult.js

ybiquitous marked this pull request as ready for review January 5, 2023 16:38

jeddy3 requested changes Jan 12, 2023

View reviewed changes

ybiquitous added 2 commits January 13, 2023 09:57

Merge branch 'v15' into report-deprecated-rules

e1bb607

Simplify deprecation warning messages in console

ff83ba8

ybiquitous commented Jan 13, 2023

View reviewed changes

types/stylelint/index.d.ts Outdated Show resolved Hide resolved

jeddy3 requested changes Jan 13, 2023

View reviewed changes

lib/formatters/stringFormatter.js Outdated Show resolved Hide resolved

ybiquitous added 2 commits January 14, 2023 11:50

Fix LintResult.deprecations[].reference type

d6108a9

Improve warning text in console

bec7cdc

jeddy3 approved these changes Jan 14, 2023

View reviewed changes

jeddy3 mentioned this pull request Jan 19, 2023

Prepare 15.0.0 #6409

Closed

Merge branch 'v15' into report-deprecated-rules

54a2305

ybiquitous merged commit 005ed45 into v15 Jan 26, 2023

ybiquitous deleted the report-deprecated-rules branch January 26, 2023 12:56

jeddy3 mentioned this pull request Jan 28, 2023

Add declaration-property-value-no-unknown #6511

Merged

mattdell mentioned this pull request Feb 9, 2023

Loads of deprecation warnings? stylelint-scss/stylelint-config-standard-scss#59

Closed

Mouvedia mentioned this pull request Feb 21, 2023

Add --quiet-deprecation-warnings flag #6677

Closed

Mouvedia reviewed Jul 15, 2024

View reviewed changes

lib/lintPostcssResult.js Show resolved Hide resolved

Mouvedia mentioned this pull request Jul 16, 2024

Use emitDeprecationWarning for rule deprecations #7845

Closed

Mouvedia mentioned this pull request Aug 19, 2024

Honour --no-deprecation flag for deprecated rules #7943

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add deprecated rule warnings #6561

Add deprecated rule warnings #6561

ybiquitous commented Jan 5, 2023

changeset-bot bot commented Jan 5, 2023 •

edited

Loading

jeddy3 left a comment

jeddy3 Jan 12, 2023

Mouvedia Jan 12, 2023 •

edited

Loading

ybiquitous Jan 13, 2023

Mouvedia Jan 13, 2023 •

edited

Loading

jeddy3 Jan 13, 2023

ybiquitous Jan 14, 2023

Mouvedia Jan 14, 2023 •

edited

Loading

ybiquitous Jan 22, 2023

ybiquitous commented Jan 14, 2023

jeddy3 left a comment

ollie-iterators commented Jan 17, 2023 •

edited

Loading

jeddy3 commented Jan 19, 2023

ybiquitous commented Jan 25, 2023

jeddy3 commented Jan 25, 2023

Mouvedia commented Jan 25, 2023

ybiquitous commented Jan 26, 2023

	`The "${ruleName}" rule is deprecated and will be removed in the next major release.`,
	`The "${ruleName}" rule is deprecated.`,

	if (reference) {
	line += dim(` See: ${underline(reference)}`);
	}

Add deprecated rule warnings #6561

Add deprecated rule warnings #6561

Conversation

ybiquitous commented Jan 5, 2023

changeset-bot bot commented Jan 5, 2023 • edited Loading

🦋 Changeset detected

jeddy3 left a comment

Choose a reason for hiding this comment

jeddy3 Jan 12, 2023

Choose a reason for hiding this comment

Mouvedia Jan 12, 2023 • edited Loading

Choose a reason for hiding this comment

ybiquitous Jan 13, 2023

Choose a reason for hiding this comment

Mouvedia Jan 13, 2023 • edited Loading

Choose a reason for hiding this comment

jeddy3 Jan 13, 2023

Choose a reason for hiding this comment

ybiquitous Jan 14, 2023

Choose a reason for hiding this comment

Mouvedia Jan 14, 2023 • edited Loading

Choose a reason for hiding this comment

ybiquitous Jan 22, 2023

Choose a reason for hiding this comment

ybiquitous commented Jan 14, 2023

jeddy3 left a comment

Choose a reason for hiding this comment

ollie-iterators commented Jan 17, 2023 • edited Loading

jeddy3 commented Jan 19, 2023

ybiquitous commented Jan 25, 2023

jeddy3 commented Jan 25, 2023

Mouvedia commented Jan 25, 2023

ybiquitous commented Jan 26, 2023

changeset-bot bot commented Jan 5, 2023 •

edited

Loading

Mouvedia Jan 12, 2023 •

edited

Loading

Mouvedia Jan 13, 2023 •

edited

Loading

Mouvedia Jan 14, 2023 •

edited

Loading

ollie-iterators commented Jan 17, 2023 •

edited

Loading