-
-
Notifications
You must be signed in to change notification settings - Fork 988
Fix declaration-block-no-redundant-longhand-properties false negatives for shorthand-then-longhand
#8892
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix declaration-block-no-redundant-longhand-properties false negatives for shorthand-then-longhand
#8892
Conversation
🦋 Changeset detectedLatest commit: 0173032 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
9306b73 to
922a26a
Compare
ybiquitous
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nathannewyen Thanks for the pull request.
We've started working on the v17 branch, not main, toward the next major version, so can you please recreate the feature branch from v17 and change the base branch of this PR?
922a26a to
a5ba767
Compare
…s for shorthand-then-longhand This commit fixes issue stylelint#8349 by detecting redundant longhand properties when they follow a shorthand property with the same value. For example, the following CSS now triggers a warning: ```css a { background: red; background-color: red; /* Redundant - flagged now */ } ``` Changes: - Add tracking of declared shorthand properties and their values - Check if longhand properties have a covering shorthand with the same value - Add new message type `redundantLonghand` for clear error reporting - Add auto-fix support to remove redundant longhands - Add test cases for both accept and reject scenarios Closes stylelint#8349
a5ba767 to
68bdba7
Compare
|
I just rebased onto v17 and updated the base branch, can you double check? |
|
This PR is packaged and the instant preview is available (68bdba7). View the demo website. Install it locally: npm i -D https://pkg.pr.new/stylelint@68bdba7 |
ybiquitous
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I've left review comments, so can you address them?
[suggestion] Can you add two examples to the README?
- a problem example
- a non-problem example
lib/rules/declaration-block-no-redundant-longhand-properties/README.md
lib/rules/declaration-block-no-redundant-longhand-properties/index.mjs
Outdated
Show resolved
Hide resolved
lib/rules/declaration-block-no-redundant-longhand-properties/index.mjs
Outdated
Show resolved
Hide resolved
lib/rules/declaration-block-no-redundant-longhand-properties/index.mjs
Outdated
Show resolved
Hide resolved
lib/rules/declaration-block-no-redundant-longhand-properties/index.mjs
Outdated
Show resolved
Hide resolved
lib/rules/declaration-block-no-redundant-longhand-properties/index.mjs
Outdated
Show resolved
Hide resolved
lib/rules/declaration-block-no-redundant-longhand-properties/index.mjs
Outdated
Show resolved
Hide resolved
lib/rules/declaration-block-no-redundant-longhand-properties/__tests__/index.mjs
Outdated
Show resolved
Hide resolved
lib/rules/declaration-block-no-redundant-longhand-properties/__tests__/index.mjs
Show resolved
Hide resolved
lib/rules/declaration-block-no-redundant-longhand-properties/__tests__/index.mjs
Show resolved
Hide resolved
lib/rules/declaration-block-no-redundant-longhand-properties/index.mjs
Outdated
Show resolved
Hide resolved
|
Hi sorry for taking a bit long, can you check again. Thanks alot! |
ybiquitous
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. The implementation code looks good. 👍🏼
Additionally, can you fix the test failures and take a look at my reviews on the doc and test?
lib/rules/declaration-block-no-redundant-longhand-properties/README.md
Outdated
Show resolved
Hide resolved
lib/rules/declaration-block-no-redundant-longhand-properties/README.md
Outdated
Show resolved
Hide resolved
lib/rules/declaration-block-no-redundant-longhand-properties/__tests__/index.mjs
Show resolved
Hide resolved
…EADME.md Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
…EADME.md Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
Update test expectations to match actual values from the new redundant longhand detection feature: - Add fix objects to warnings for multiple redundant longhands - Update column positions (35->36, 47->48) for margin-right - Update fix range from [34, 54] to [35, 54]
ybiquitous
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jeddy3
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nathannewyen LGTM, thank you!
@ybiquitous Thank you for reviewing and helping shape the PR 🙇
Shouldn't the fixes go to |
| datasource | package | from | to | | ---------- | --------- | ------- | ------ | | npm | stylelint | 16.26.1 | 17.0.0 | ## [v17.0.0](https://github.com/stylelint/stylelint/blob/HEAD/CHANGELOG.md#1700---2026-01-15) It contains 14 breaking changes, which we've detailed in the [migrating to `17.0.0` guide](docs/migration-guide/to-17.md). Additionally, it adds 3 options to the rules and fixes 9 bugs. We've also released compatible versions of our [shared config](https://www.npmjs.com/package/stylelint-config-standard), [Visual Studio Code extension](https://marketplace.visualstudio.com/items?itemName=stylelint.vscode-stylelint), [Node.js Rule Tester](https://www.npmjs.com/package/stylelint-test-rule-node) and [Jest preset](https://www.npmjs.com/package/jest-preset-stylelint). - Removed: CommonJS Node.js API ([#8859](stylelint/stylelint#8859)) ([@jeddy3](https://github.com/jeddy3)). - Removed: `output` property in the Node.js API returned resolved object ([#8878](stylelint/stylelint#8878)) ([@jeddy3](https://github.com/jeddy3)). - Removed: support for Node.js less than 20.19.0 ([#8867](stylelint/stylelint#8867)) ([@jeddy3](https://github.com/jeddy3)). - Removed: GitHub formatter ([#8888](stylelint/stylelint#8888)) ([@jeddy3](https://github.com/jeddy3)). - Removed: `resolveNestedSelectors` option from `selector-class-pattern` ([#8931](stylelint/stylelint#8931)) ([@jeddy3](https://github.com/jeddy3)). - Removed: `checkContextFunctionalPseudoClasses` option from `selector-max-id` ([#8913](stylelint/stylelint#8913)) ([@jeddy3](https://github.com/jeddy3)). - Changed: default `fix` mode to `strict` ([#8889](stylelint/stylelint#8889)) ([@jeddy3](https://github.com/jeddy3)). - Changed: `report` to be consistent and predictable in how it handles the provided position arguments ([#8217](stylelint/stylelint#8217)) ([@romainmenke](https://github.com/romainmenke)). - Changed: `selector-max-*` syntax rules for standard CSS nesting and modern functional pseudo-classes ([#8913](stylelint/stylelint#8913)) ([@jeddy3](https://github.com/jeddy3)). - Changed: `*-specificity` semantic rules for standard CSS nesting ([#8913](stylelint/stylelint#8913)) ([@jeddy3](https://github.com/jeddy3)). - Changed: `no-duplicate-selectors` and `selector-no-qualifying-type` for standard CSS nesting ([#8913](stylelint/stylelint#8913)) ([@jeddy3](https://github.com/jeddy3)). - Changed: `*-list` rules to have consistent behaviour for vendor prefixes and case ([#8912](stylelint/stylelint#8912)) ([@jeddy3](https://github.com/jeddy3)). - Changed: `*-no-vendor-prefix` rules to have consistent behaviour for their `ignore*: []` secondary options ([#8924](stylelint/stylelint#8924)) ([@jeddy3](https://github.com/jeddy3)). - Changed: `declaration-property-max-values` rule to have consistent behaviour for vendor prefixes ([#8926](stylelint/stylelint#8926)) ([@jeddy3](https://github.com/jeddy3)). - Added: `except: ["after-block"]` to `custom-property-empty-line-before` ([#8921](stylelint/stylelint#8921)) ([@kovsu](https://github.com/kovsu)). - Added: `except: ["after-block"]` to `declaration-empty-line-before` ([#8910](stylelint/stylelint#8910)) ([@kovsu](https://github.com/kovsu)). - Added: `ignoreSelectors: []` to `no-duplicate-selectors` ([#8883](stylelint/stylelint#8883)) ([@kovsu](https://github.com/kovsu)). - Fixed: Windows drive letter casing inconsistencies when matching patterns against file paths ([#8941](stylelint/stylelint#8941)) ([@adalinesimonian](https://github.com/adalinesimonian)). - Fixed: CLI help to include TypeScript config files ([#8908](stylelint/stylelint#8908)) ([@kovsu](https://github.com/kovsu)). - Fixed: `at-rule-descriptor-no-unknown` false positives for declarations within feature-value-blocks ([#8868](stylelint/stylelint#8868)) ([@kovsu](https://github.com/kovsu)). - Fixed: `declaration-block-no-redundant-longhand-properties` false negatives for short and long combinations ([#8892](stylelint/stylelint#8892)) ([@nathannewyen](https://github.com/nathannewyen)). - Fixed: `media-feature-name-no-unknown` false positives for namespaced dollar variables and range context queries ([#8890](stylelint/stylelint#8890)) ([@kovsu](https://github.com/kovsu)). - Fixed: `nesting-selector-no-missing-scoping-root` false positives for CSS-in-JS ([#8905](stylelint/stylelint#8905)) ([@kovsu](https://github.com/kovsu)). - Fixed: `no-invalid-position-declaration` false negatives for embedded blocks ([#8907](stylelint/stylelint#8907)) ([@kovsu](https://github.com/kovsu)). - Fixed: `selector-no-qualifying-type` false negatives for `:is/where()` ([#8940](stylelint/stylelint#8940)) ([@romainmenke](https://github.com/romainmenke)). - Fixed: `selector-type-no-unknown` false positives for MathML 4 tags ([#8874](stylelint/stylelint#8874)) ([@jeddy3](https://github.com/jeddy3)).
Summary
Fixes #8349
This PR detects redundant longhand properties when they follow a shorthand property with the same value.
Before
After
Changes
redundantLonghandfor clear error reportingTest Plan