False positive for declarationless rules in no-duplicate-selectors

[subzey]

Describe the issue. Is it a bug or a feature request (new rule, new option, etc.)?

A feature request. Add a secondary option ignoreEmpty to no-duplicate-selectors, so blocks with no content won't count.

The use case:

.some-block {
  & {
    color: black;
  }

  &--error {
    color: red;
  }
}

This LESS code is linted as

.some-block {}
.some-block {
  color: black;
}
.some-block--error {
  color: red;
}

triggering a no-duplicate-selectors error/warning. ignoreEmpty would allow using such a style, yet catching the "real" errors, like this:

.some-block {
  color: black;

  & { // ERROR!
    color: green;
  }
}

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

no-duplicate-selectors

What CSS is needed to reproduce this issue?

e.g.

.some-block {
  & {
    color: black;
  }

  &--error {
    color: red;
  }
}

What stylelint configuration is needed to reproduce this issue?

e.g.

{
  "rules": {
    "no-duplicate-selectors": true
  }
}

Which version of stylelint are you using?

7.6.0

How are you running stylelint: CLI, PostCSS plugin, Node API?

CLI, as a git hook and with SublimeLinter-contrib-stylelint

Does your issue relate to non-standard syntax (e.g. SCSS, nesting, etc.)?

Yes, it's about LESS nesting

What did you expect to happen?

& within a rule without properties is not treated as a duplicate block

What actually happened (e.g. what warnings or errors you are getting)?

& within a rule without properties is treated as a duplicate block

[alexander-akait]

@subzey I think it is bug, not feature, we should ignore selectors which contains only nesting

[alexander-akait]

it should be easy fix, add isContainsOnlyNesting utils (maybe best name) and skip this selectors.

[jeddy3]

it should be easy fix, add isContainsOnlyNesting utils (maybe best name) and skip this selectors.

SGTM. Let's go withhasOnlyNestedRules to be in line with our other has* utils like hasEmptyBlock.

[hudochenkov]

This rule fails if a CSS rule has a nested rule with selector &. I think #2199 fails because of this too.

When I started to figure out a solution for this problem and edge cases, I wrote following text based on the discussion above:

---

What hasOnlyNestedRules would check? And where it will be applied?

This shouldn't show warnings but shows now:

.foo {
	& {}
}
.foo {
	&--bar {}
	& {}
}

This should show warnings, and it's ok:

.foo {
	color: red;
	& {
		color: blue;
	}
}

How about this:

.foo {
	/* comment 1 */
	& {}

	/* comment 2 */
	&--bar {}
}

Should it fails, if there are nested rules and comments? Processed CSS would have two .foo rule-sets: one with comments only, and one with content of & {}.

There are few more edge cases for non-standard syntax:

.foo {
	@include icon;

	& {
		color: red;
	}
}

.foo {
	@at-root & {
		color: red;
	}
}

.foo {
	@media (min-width: 300px) {
		& {
			color: pink;
		}
	}
}

---

Now I re-read no-duplicate-selectors description and where it applies. I believe we were thinking in a wrong direction. The rule is about selectors only, not about rules content or how preprocessors will compile the code.

We shouldn't check rule content because we can't predict what preprocessors will compile.

Following code:

.foo {
	& {}
}

Can be compiled as .foo {} or as .foo {} .foo {} depending on preprocessor and its settings.

We don't know how it will compile. But in most cases, it will compile as .foo {} .foo {}, because usually there is declarations or other content in parent rule and nested rules.

To sum up, I think it's a correct behavior for no-duplicate-selectors to show warnings for this code:

.foo {
	& {}
}

[alexander-akait]

@hudochenkov can you send failed test PR? so it will be much easier to solve the problem

[hudochenkov]

@evilebottnawi how failed test should be written in a test code? Should it really fail tests?

I believe this rule should throw warnings in this particular case, so I can add tests in reject array.

[jeddy3]

@hudochenkov Thanks for doing the investigation.

To sum up, I think it's a correct behavior for no-duplicate-selectors to show warnings for this code:

I agree with your assessment. Does that means this issue can be closed?

[hudochenkov]

Yeah, after I'll add tests, so we won't forget about correct behaviour :)

[hudochenkov]

Closed because it works as expected. Explanation in a comment above.

[hudochenkov]

@7iomka you have duplicate selector on line 13.

[LuXDAmore]

@hudochenkov , What do you think about this .scss file?

  .class {
  
    &, /* <-- no-duplicate-selectors error here */
    > .content {
  
         display: block;
  
    }
  
  }

The error still in &, but i think should be something allowed, because i prefer to not duplicate code in that case.