Skip to content

Commit aea47bb

Browse files
committed
Change IdWithExtraneousSelector to IdSelector
Rename and modify behavior of linter to always report IDs any time they appear in a selector, rather than only when they appear concatenated with a type/class etc. Change-Id: Ie2bbc54cab25faaef6715c340e8d3798d07bd051 Reviewed-on: http://gerrit.causes.com/44913 Tested-by: jenkins <[email protected]> Reviewed-by: Shane da Silva <[email protected]>
1 parent 5a41a6f commit aea47bb

File tree

7 files changed

+86
-179
lines changed

7 files changed

+86
-179
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
as they are always hyphenated lowercase
1717
* Fix `Indentation` to not crash on `@at-root` directives with comments inside
1818
* Add support for disabling/enabling linters via inline comments
19+
* Change `IdWithExtraneousSelector` to `IdSelector` and modify behavior to
20+
always report a lint when an ID appears as part of a selector.
1921

2022
## 0.30.0
2123

config/default.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ linters:
5252
HexValidation:
5353
enabled: true
5454

55-
IdWithExtraneousSelector:
55+
IdSelector:
5656
enabled: true
5757

5858
ImportPath:

lib/scss_lint/linter/README.md

+11-19
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ Below is a list of linters supported by `scss-lint`, ordered alphabetically.
1717
* [HexLength](#hexlength)
1818
* [HexNotation](#hexnotation)
1919
* [HexValidation](#hexvalidation)
20-
* [IdWithExtraneousSelector](#idwithextraneousselector)
20+
* [IdSelector](#idselector)
2121
* [ImportPath](#importpath)
2222
* [Indentation](#indentation)
2323
* [LeadingZero](#leadingzero)
@@ -362,37 +362,29 @@ p {
362362
}
363363
```
364364

365-
## IdWithExtraneousSelector
365+
## IdSelector
366366

367-
Don't combine additional selectors with an ID selector.
367+
Avoid using ID selectors.
368368

369-
**Bad: `.button` class is unnecessary**
369+
**Bad: highly-specific styling for a single element via ID**
370370
```scss
371-
#submit-button.button {
371+
#submit-button {
372372
...
373373
}
374374
```
375375

376-
**Good: standalone ID selector**
376+
**Good: reusable class**
377377
```scss
378-
#submit-button {
378+
.submit-button {
379379
...
380380
}
381381
```
382382

383383
While the CSS specification allows for multiple elements with the same ID to
384-
appear in a single document, in practice this is a smell. When
385-
reasoning about IDs (including selector specificity), it should suffice to
386-
style an element with a particular ID based solely on the ID.
387-
388-
Another possible pattern is to modify the style of an element with a given
389-
ID based on the class it has. This is also a smell, as the purpose of a CSS
390-
class is to be reusable and composable, and thus redefining it for a specific
391-
ID is a violation of those principles.
392-
393-
Even better would be to
394-
[never use IDs](http://screwlewse.com/2010/07/dont-use-id-selectors-in-css/)
395-
in the first place.
384+
appear in a single document, in practice this is a smell. [ID selectors should
385+
never be used](http://screwlewse.com/2010/07/dont-use-id-selectors-in-css/) for
386+
the purposes of styling an element, as it leads to overly specific styles that
387+
aren't easily shared with other elements.
396388

397389
## ImportPath
398390

lib/scss_lint/linter/id_selector.rb

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
module SCSSLint
2+
# Checks for the use of an ID selector.
3+
class Linter::IdSelector < Linter
4+
include LinterRegistry
5+
6+
def visit_id(id)
7+
add_lint(id, 'Avoid using id selectors')
8+
end
9+
end
10+
end

lib/scss_lint/linter/id_with_extraneous_selector.rb

-20
This file was deleted.
+62
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
require 'spec_helper'
2+
3+
describe SCSSLint::Linter::IdSelector do
4+
context 'when rule is a type' do
5+
let(:css) { 'p {}' }
6+
7+
it { should_not report_lint }
8+
end
9+
10+
context 'when rule is an ID' do
11+
let(:css) { '#identifier {}' }
12+
13+
it { should report_lint line: 1 }
14+
end
15+
16+
context 'when rule is a class' do
17+
let(:css) { '.class {}' }
18+
19+
it { should_not report_lint }
20+
end
21+
22+
context 'when rule is a type with a class' do
23+
let(:css) { 'a.class {}' }
24+
25+
it { should_not report_lint }
26+
end
27+
28+
context 'when rule is a type with an ID' do
29+
let(:css) { 'a#identifier {}' }
30+
31+
it { should report_lint line: 1 }
32+
end
33+
34+
context 'when rule is an ID with a pseudo-selector' do
35+
let(:css) { '#identifier:active {}' }
36+
37+
it { should report_lint line: 1 }
38+
end
39+
40+
context 'when rule contains a nested rule with type and ID' do
41+
let(:css) { <<-CSS }
42+
p {
43+
a#identifier {}
44+
}
45+
CSS
46+
47+
it { should report_lint line: 2 }
48+
end
49+
50+
context 'when rule contains multiple selectors' do
51+
context 'when all of the selectors are just IDs, classes, or types' do
52+
let(:css) { <<-CSS }
53+
#identifier,
54+
.class,
55+
a {
56+
}
57+
CSS
58+
59+
it { should report_lint line: 1 }
60+
end
61+
end
62+
end

spec/scss_lint/linter/id_with_extraneous_selector_spec.rb

-139
This file was deleted.

0 commit comments

Comments
 (0)