Rules #1
Comments
|
Some rules idea (most of them are based on @necolas postcss-bem-linter)
|
|
Obviously we might take some stuff from csslint https://github.com/CSSLint/csslint/wiki/Rules |
|
My main headache is a lot of nesting rules when peoples use preprocessors. .welcome .to .css > .hell ul > li .etc{}Css linter with source maps support with this checker will be very usefull for me. |
|
I'd take a look at everything available in SCSS-Lint: https://github.com/causes/scss-lint/blob/master/lib/scss_lint/linter/README.md Lots of good stuff there. |
|
Use ColorKeyword is better! Good color keyword
color: green;
Bad: hexadecimal color
color: #0f0; |
|
why reimplement another existing module (bem linter)? |
|
@necolas the idea behind this is to make an eslint like for css (csslint seems dead & not likely to have a second life..). I want something really configurable that might also have the role of a code style checker (like jscs). |
|
FYI, I've added a (pretty long) list in the description of the issue. |
|
Great list! A couple of others (drawn from SCSS-Lint) that I'd be interested in using (trying to match your naming pattern):
And for a couple of the rules you mentioned (and vendor-prefixes) I think we'd probably want to be able to have a |
|
I've added those 3 rules as
Good idea for some presets. |
|
there's already csscomb too. maybe contribute to that first |
|
I have already checked csscomb, but I don't really like the parser (gonzales "pe"). |
|
plus csscomb also looks like it isn't being developed anymore. for example, i opened this issue 6 months ago - csscomb/csscomb.js#245 - and had to write a module to provide reporting output. i assume they don't have time to maintain it, so probably worth writing a new tool that is easier for the community to patch / extend. some feedback:
blankline? newline is not the same. or you could it could also accept an integer where the default is
what are these exactly?
presumably only for 0 values that can be unitless in css?
what is this? including support for 'excludes' (and maybe 'severity') would also be useful, like: https://github.com/causes/scss-lint |
|
Indeed blankline is not newline. So I changed
.oneline {(space)prop: value(space)}
.class {}
.class:hover {} /* indented */
.class::before {} /* indented */
.selector {
background: red;
background: linear-gradient(...)
}
.Block {}
.Block--modifier {}
.Block-element {}
.Block-element--modifier {}This should be exploded into several rules like Maybe all indentation related rules should start with
|
|
ok thanks
Could that not be handled by the options for space before property and space before closing brace? Probably don't need special options.
the example you gave is a duplicate property, not value |
|
You are probably right for the rule with the name too long to type. |
|
List updated. I made some change based on http://apps.workflower.fi/vocabs/css/en Also
For most of the rules, I've tried to separate them according to their scope (statements, rules, properties, values...) except for indentation. Not sure if it's a good thing. Any thoughts ? |
|
This all looks great, thanks! Is it possible to make the
Perhaps take a page out of the JSCS book and use the
Note how all three rules now have And the other boolean rules would look something like:
Note again how everything now defaults to As the indentation rules can take a string as well as a bool, the
How does that look? The linter is now permissive by default and becomes more restrictive as rules are enabled. |
|
Yeah sounds good this way (I'm using jscs everywhere). I was thinking about that this morning when I see trailing-semicolon rule :) Everything disabled by default seems a good idea, but I'm sure some lazy people would like to get predefined configuration. How can we handle that? I like something simpler like eslint with no-* (disallow) or * (require). More short & easier to spot. Don't you think? I you see some That could give us something like
|
Looks good to me.
So, perhaps using "no-* (disallow) or * (require)" for the rule names, and using "require" and "disallow" in the documentation is the best of both worlds e.g.
Perhaps in the same way as JSCS does by providing presets? Two for CSS that spring to mind are: You could bundle a companion stylelint preset, but I think it should be separate and as comprehensively documented as the two guides above. The rules within Style Guides tend to be very subjective and distancing the linter from this subjectiveness might help with the clarity and longevity of the project. The linter documentation can then be narrowly focused on describing what the rule does without getting sucked into the murky world of rationalising why (dis)enabling a rule is subjectively better than not. Does that make sense, I can elaborate on it if I've done a terrible job explaining what I mean? :) |
|
Indeed managing presets will represent an amount of work I don't want to handle in a near future. So we will skip that for now. Probably just some example + links to some others presets. I just updated a part of the list to match the "no-* (disallow) or * (require)" & also deleted & renamed some rules to make it easier to read. |
|
The rules are starting to come together nicely! I really like how they are grouped by things like selector, rule and value.
I understand what you mean now. I agree that allowing an int as well as a string seems to be a bad idea as it feels unnecessarily confusing. A string offers all the flexibility needed and with a consistent interface. The user can then throw anything they want in there, be it:
Is that what you were thinking? I thought I’d try to mock-up a Style Guide preset to help us sanity-check the rules so far. I thought of mocking up the Idiomatic CSS styleguide first as it’s very well-known, but I went with the SUITCSS one instead as it is a stricter subset of the Idiomatic one. Doing the mock-up uncovered a few things that were not as clear as they could be with the rule names… Again, I really like how the rules are grouped. Some of the terms on that vocabs site were new to me, so I had to look at syntax part of the spec to get my head around it. It uncovered a few things that might be mis-grouped or named:
What do you think? A few others queries:
.selector {
background-image:
linear-gradient(#fff, #ccc),
linear-gradient(#f3c, #4ec);
box-shadow: 1px 1px 1px #000, 2px 2px 1px 1px #ccc inset;
}Perhaps by allowing an array of strings for the declaration-coma-before: ""; // no space
declaration-coma-after: [" ", "/n"]; // space or new line
So, with these things in mind, here's the mock-up of the SUITCSS preset: newline-eof: false,
no-end-of-line-white-space: true,
root-no-standard-properties: true
rule-single-line-declaration-before: " ",
rule-single-line-declaration-after: " ",
selector-attribute-quotes: '"',
selector-combinator-before: " ",
selector-combinator-after: " ",
selector-delimiter-before: "",
selector-delimiter-after: "/n",
block-brace-opening-before: " ",
block-brace-opening-after: [" ", "/n"], // space (for single-line rule-set) or newline (for normal)?
block-brace-closing-before: [" ", ""], // space (for single-line rule-set) or no space (for normal)?
block-brace-closing-after: "", // is this where separation should be handled?
declaration-block-properties-order: "alphabetical",
declaration-block-semicolon-before: "",
declaration-block-semicolon-after: [" ", "/n"] // space (for single-line rule-set) or newline (for normal)?
declaration-block-trailing-semicolon: true,
declaration-colon-before: "", // no space`,
declaration-colon-after: [" ", "/n"], // space or new line`
declaration-coma-before: "", // no space`
declaration-coma-after: [" ", "/n"], // space or new line`
value-string-quotes: '"',
value-url-quotes: '"',
value-comma-before: "",
value-comma-after: " ",
value-bang-before: " ",
value-bang-after: "",
value-unitless-zero: true,I haven't added any indentation or separation rules yet, as I'm finding them a little confusing.
I was struggling to get my head around what are the rules needed for something like?: /** @define Excerpt; use strict */
@import "suitcss-utils-layout";
@import "./Button";
/**
* Content excerpts. Agnostic of image size, and with a clear call to action.
*/
:root {
--Excerpt-padding: 20px;
--Excerpt-color: orange;
}
.Excerpt {
padding: var(--Excerpt-padding);
}
@media (--mq-wide) {
.Excerpt {
color: var(--Excerpt-color);
}
}i.e. where The |
|
FWIW, if it's too much work to get the idiomatic-css or suitcss styles, i'd be happy to remove some of the more idiosyncratic aspects of those style guides. I'd rather have something easily enforceable with tools than make tool authors bend-over-backwards to support every little detail of a style. |
|
Follow up to @jeddy3 and other comments above:
|
|
Btw, generic quotes should be just Also, @jeddy3 the problem for -before/after with custom string will be general indentation of the code .A { /* {: before " ", after "\n" */
} { /* }: before "\n", after "\n\n" */
.A-b { { /* {: before " ", after "\n" */
} { /* }: before "\n" + INDENTATION, after "\n" */Here you might see a problem. Should we accept that some rules might rely on some others? (In this case ...brace-before, using indentation(-*). Ok for:
For
For the point about comma & multiples lines
In you example, for background-image there is coma in the function with no \n & coma to separate gradient with \n. Not easy to spot. Maybe we can postpone this issue for later? Seems not a big deal to me.
I was thinking about that but didn't take time to write this up :) Also I think we can just get ride of that
We should get something like /* eg with declaration-block-single-line-declaration-length === 2 */
/* valid */
a {
b: c;
d: e;
}
/* valid */
a { b: c; d: e }
/* INVALID */
a { b: c; d: e; f: g } |
|
All those
Is having general rules that can be overridden by more specific ones a good idea? One of ESLint’s philosophies is: “Every rule is standalone”, and it looks like the “Working with Rules” section of the ESLint Developer Guide goes into a bit more detail about it. I think they do it because keeping each rule discrete helps to keep the linter pluggable and the rule explicitly understandable to the user?
So, using Firstly, having general rules does not seem keep rules explicitly understandable: Given: quote: '"'Am I right in thinking that this will enforce the following rules?:
Is that too much stuff hidden behind a fairly harmless looking rule? Secondly, and perhaps more importantly, does overriding something create a dependency between to rules?: Given: quote: '"' // general
selector-attribute-quote: "" // specific overridei.e.
Doesn't the .selector[type=text] {
a: b;
}Wouldn’t that mean, if general rules were allowed, the linter would need to track which rules are dependent on which other rules? Thirdly, even if all the rules are standalone, the linter won't need to manage conflicts between rules?: Given: property-blacklist: ["color", "width"]
property-whitelist: ["color", "height"]And .selector {
color: blue;
height: 10px;
width: 5px;
}The following errors would be returned: file.css:2:3 - "color" is blacklisted // thrown by the property-blacklist rule
file.css:4:3 - "width" is not whitelisted // thrown by the property-whitelist rule
file.css:4:3 - "width" is blacklisted // thrown by the property-blacklist rule Which is good right? As it’s then down the user to correct their choice of rules so they no longer clash. The linter doesn’t need to get involved in sorting out clashing rules even if they are standalone. Finally, keeping rules discrete/standalone could be advantageous to the rule developer. For example, a standalone, narrowly-focused rule like
I think it might be possible to get great mileage out of starting with this standalone philosophy and only tackling the interdependent rules later. Here is everything I can think of that is definitely a standalone rule: // File rules
eof-newline
eol-no-whitespace
// Rule-set rules
rule-set-no-empty
// At-rule rules
at-rule-no-vendor-prefix
// Custom media rules
custom-media-pattern
// :root rules
root-no-standard-properties: true
// Selector rules
selector-attribute-quotes
selector-no-vendor-prefix
selector-pattern
selector-no-id
selector-no-type
selector-no-universal
selector-no-attribute
selector-no-qualified
selector-no-pseudo-class
selector-no-pseudo-element
selector-no-combinator
selector-no-delimiter
selector-no-descendant
selector-max-length
selector-max-specificity
// Custom selector rules
custom-selector-pattern
// Block rules
block-indent // I think this is standalone...
// Declaration block rules
declaration-block-properties-order
declaration-block-trailing-semicolon
declaration-block-no-duplicate-properties
// Declaration rules
declaration-no-important // previously "value-no-important"
// Property rules
property-no-vendor-prefix
property-blacklist
property-whitelist
// Custom property rules
custom-property-root-restriction
custom-property-pattern
// Value rules
value-string-quotes
value-url-quotes
value-unitless-zero
value-no-leading-zero
value-no-trailing-zero
value-color-keywords-prefered
value-validate-hexa
value-unit-blacklist
value-unit-whitelist
value-body-backgroundThat’s a lot of stuff and, it looks like, none of it needs the complexity of interdependent rules. (All the So, keeping rules focused and standalone to begin with seems like a win-win to me. Rules are explicitly understandable to the user, and the Linter doesn’t need to keep track of what rules rely on other rules, or manage conflicts. Does that make sense, or have I got myself confused thinking about this too much? :) |
|
Indeed, we should make each rule standalone. Simpler to develop, simpler for testing, simpler to understand. I'm dropping this as an idea: {
indentation: {
"default": 1,
"char": "\t",
"selector-class-bem-element": true
"selector-class-bem-modifier": false
"selector-pseudo-classes": 2,
"selector-pseudo-element": true
"rule": true
}
} |
|
I really like the idea here, great stuff! I currently use esformatter for JS and SCSS linter for SCSS. Just one thing, I think that naming conventions such as BEM, SMACSS, and stuff like Compass or Bourbon could be "pluggable". This way Stylelinter would have plugins to handle non-CSS stuff (which makes more sense to me compared to presets?). |
|
@davidtheclark I think you would be best to use parser.eachInside(function (selector) {
if (selector.type === 'universal' && opts.noUniversal) {
// ... your logic for warning the user
}
});I'm going to rework this iterator to be safe for removing nodes whilst iterating, like PostCSS does, but the parser itself is pretty solid right now. Just need more tools to work with the AST I reckon. 😄 |
|
@ben-eb Awesome. Just let us know when you think it's finalized enough for us to work with. @MoOx I guess I don't understand why you would not use delimiters because you're using classes. Couldn't you have something like: .my-component__thing--odd,
.my-component__otherpart {
color: pink;
}
.my-component__thing--even {
background: oragen;
}I don't recall seeing a rule that you must have only one selector per rule -- maybe I missed it? |
|
@davidtheclark I just published |
|
@davidtheclark Current API: https://github.com/postcss/postcss-selector-parser/blob/master/API.md - if you have any suggestions, please let me know. postcss/postcss-selector-parser#5 |
|
Thanks @ben-eb. At some point here I'll have a few minutes and I'll try to write some selector linting rules to give it a shot. |
|
Updated list to reflect decisions in #178 |
jeddy3
added
PR: review accepted
type: enhancement
|
@jeddy3 : Because of insupportable length of this issue, I suggest that we close it and open new issues for any of the rules listed way up above that we definitely want to implement or urge fantasy contributors to implement. What do you think? |
|
@davidtheclark Makes sense to me. I admit I'll be a little sad to see it go though as it's been with us since the start. Kind of like saying good bye to a comfy sofa that you've had for years, but has out-lived its usefulness :) I'll pick out the ones I'd like to see. Then, if you can do the same then close the issue. |
|
I'm done :) FYI, I replaced |
|
Thanks. |
Vocabulary reference http://apps.workflower.fi/vocabs/css/en
Updated list
Rules
General (tied to the stylesheet itself)
no-missing-eof-newline: Disallow missing end-of-file newlineno-multiple-empty-lines: Disallow multiple empty white linesno-eol-no-whitespace: Disallow whitespace at the end of lineindentation: Specify whitespace for indentationmax-line-length: Specify maximum line lengthString (ref)
string-quotes:"double"|"single"Specify quotes around strings"double"- strings must be doubled quoted"single"- strings must be single quotedNumber (ref)
number-leading-zero:"always"|"never"Require or disallow a leading zero before number"always"- a number must have a leading zero"never"- a number must not have a leading zeronumber-no-trailing-zeros: Disallow trailing zeros (e.g.5.0px)number-zero-length-no-unit: Disallow units for zero length valuesnumber-max-precision:intMaximum number of digits after the "." in a numberFunction (ref)
function-comma-space-after:"always"|"never"function-comma-space-before:"always"|"never"function-parentheses-inside-space:"always"|"never"Specify space inside parentheses (after opening, before closing)function-space-after:"always"|"never"Specify space between the closing parenthesis of a function and the next valuefunction-token-no-space: Disallow space between the function's name and its opening parenthesisfunction-calc-no-unspaced-operator: Disallow operators without space on both sides incalc()function-url-quotes:"double","single","none"Specify quotes around URLs (URLs, unlike strings, can be unquoted)"double"- URLs must be doubled quoted"single"- URLs must be single quoted"none"- URLs must be unquotedColor (ref)
color-hex-length:"short"|"long"color-hex-case:"lower"|"upper"color-no-invalid-hex: Disallow invalid hexcolor-no-named: Disallow named colorscolor-no-hex: Disallow hex colorscolor-function-blacklist:arrayDisallowed color functions e.g.["rgba", "hsl", "hwb"]color-function-whitelist:arrayOnly allowed color functions e.g.["gray", "color", "device-cmyk"]:root
root-no-standard-properties: Disallow use of standard properties in :rootRule
rule-no-single-line: Disallow single line rule-setsrule-properties-order:1 dimension array,string(preset) -"alphabetical"Order of properties within a rulerule-trailing-semicolon:"always"|"never"Trailing semicolon at the end of a rule"always"- there must be a trailing semicolon"never"- there must not be a trailing semicolonrule-no-duplicate-properties: Disallow duplicate properties within a rulerule-nested-empty-line-before: Require or disallow an empty line before nested rulesrule-single-line-max-length:intThe maximum length of a single line rulerule-single-line-max-declarations:intThe maximum number of declaration within a single line ruleAt-rule rules
at-rule-no-vendor-prefix: Disallow vendor prefixes in @rulesat-rule-empty-line-before: Require or disallow an empty line before at-rulesCustom media (ref)
custom-media-pattern:stringPreset or regex pattern to use to check names are matching a given patternMedia query
media-query-parenthesis-inside-space:"always"|"never"media-query-list-comma-space-after:"always"|"never"media-query-list-comma-space-before:"always"|"never"media-query-list-comma-newline-after:"always"|"never"media-query-list-comma-newline-before:"always"|"never"Media feature
media-feature-colon-space-after:"always"|"never"media-feature-colon-space-before:"always"|"never"media-feature-range-operator-space-after:"always"|"never"media-feature-range-operator-space-before:"always"|"never"Media feature
media-feature-colon-space-after:"always"|"never"media-feature-colon-space-before:"always"|"never"media-feature-range-operator-space-after:"always"|"never"media-feature-range-operator-space-before:"always"|"never"Media feature name
media-feature-name-no-vendor-prefix:"always"|"never"Disallow vendor prefixes for media feature namesSelector
selector-combinator-space-before:stringSpecify what space should be used before and after combinatorselector-combinator-space-after:stringSpecify what space should be used before and after combinatorselector-delimiter-space-after:"always"|"never"selector-delimiter-space-before:"always"|"never"selector-delimiter-newline-after:"always"|"never"selector-delimiter-newline-before:"always"|"never"selector-no-vendor-prefix: Disallow vendor prefixes in selectorsselector-pseudo-element-notation: Specify that pseudo element selectors need one or twoselector-root-no-composition: Disallow use of:rootselector with others (in list, complex, or compound selectors)selector-no-id: Disallow use of id in selectors e.g:#idselector-no-type: Disallow use of type in selectors e.g:divselector-no-universal: Disallow use of universal selector in selectors e.g:*selector-no-attribute: Disallow use of attributes in selectors e.g:[attr]selector-no-combinator: Disallow use of combinator in selectorsselector-no-qualified: Disallow use of qualified selectors e.g:div#idselector-no-pseudo-class: Disallow use of pseudo-class in selectorsselector-no-pseudo-element: Disallow use of pseudo-element in selectorsselector-no-delimiter: Disallow use of delimiter in selectorsselector-pattern:stringregex (eg:/\.([a-z]+-)?[A-Z][a-z]+(-[a-z][a-zA-Z]+)(--[a-z][a-zA-Z]+)/to match .org-Block-eleMent--modifierCustom selector (ref)
custom-selector-pattern:stringRegex pattern to use to check names are matching a given patternBlock
block-no-empty: Disallow empty blocksblock-opening-brace-space-after:"always"|"never"|"always-single-line"|"never-single-line"|"always-multi-line"|"never-multi-line"block-opening-brace-space-before:"always"|"never"|"always-single-line"|"never-single-line"|"always-multi-line"|"never-multi-line"block-opening-brace-newline-after:"always"|"never"|"always-multi-line"|"never-multi-line"block-opening-brace-newline-before:"always"|"never"|"always-single-line"|"never-single-line"|"always-multi-line"|"never-multi-line"block-closing-brace-space-after:"always"|"never"|"always-single-line"block-closing-brace-space-before:"always"|"never"|"always-single-line"|"never-single-line"|"always-multi-line"|"never-multi-line"block-closing-brace-newline-after:"always"|"never"|"always-single-line"|"never-single-line"|"always-multi-line"|"never-multi-line"block-closing-brace-newline-before:"always"|"never"|"always-multi-line"|"never-multi-line"Nesting Block
nesting-block-opening-brace-space-before:"always"|"never"|"always-single-line"|"never-single-line"|"always-multi-line"|"never-multi-line"nesting-block-opening-brace-newline-before:"always"|"never"|"always-single-line"|"never-single-line"|"always-multi-line"|"never-multi-line"Declaration rules
declaration-no-important: Disallow the use of !importantdeclaration-bang-space-after:"always"|"never"declaration-bang-space-before:"always"|"never"declaration-colon-space-after:"always"|"never"declaration-colon-space-before:"always"|"never"declaration-semicolon-space-after:"always"|"never"|"always-single-line"|"never-single-line"declaration-semicolon-space-before:"always"|"never"|"always-single-line"|"never-single-line"declaration-semicolon-newline-after:"always"|"never"|"always-multi-line"|"never-multi-line"declaration-semicolon-newline-before:"always"|"never"|"always-multi-line"|"never-multi-line"Unit
unit-blacklist:arrayDisallowed units e.g.["px", "pt", "cm"]unit-whitelist:arrayOnly allowed units e.g.["%", "rem", "em"]Property
property-no-vendor-prefix: Disallow vendor prefixes in propertiesproperty-blacklist:arrayDisallowed properties e.g.["tranform", "background-size"]property-whitelist:arrayOnly allowed properties e.g.["height", "width", "font-size"]property-unit-blacklist:objectDisallowed units for specific properties e.g.{"width": ["%"], "height": ["%"]}property-unit-whitelist:objectOnly allowed units for specific properties e.g.{"font-size": ["em", "rem"]}Custom property
custom-property-no-outside-root: Disallow custom properties outside of:rootcustom-property-pattern:stringRegex pattern to use to check names are matching a given patternValue
value-no-vendor-prefix: Disallow vendor prefixes in valuesValue list
value-list-comma-space-after:"always"|"never"|"always-single-line"|"never-single-line"value-list-comma-space-before:"always"|"never"|"always-single-line"|"never-single-line"value-list-comma-newline-after:"always"|"never"|"always-multi-line"|"never-multi-line"value-list-comma-newline-before:"always"|"never"|"always-multi-line"|"never-multi-line"Comment
comment-space-inside:string- `"always"|"never"comment-empty-line-before: Require or disallow an empty line before commentsTo think about Mistakes
The text was updated successfully, but these errors were encountered: