CSSLint / csslint

Automated linting of Cascading Stylesheets
http://csslint.net
Other
4.76k stars 483 forks source link

CSSLint + Stylelint #668

Open ai opened 8 years ago

ai commented 8 years ago

I think if we will merge CSSLint and Stylelint everyone will win — users will get more features, and we will have more free time. We already have great examples of ESLint + JSCS, Babel + esnext, Rails + Merb.

@XhmikosR @frvge @davidtheclark @jeddy3 what do you think?

Merging Plan Example

  1. CSSLint developers will get a membership in Stylelint core team.
  2. We check Stylelint rules and rewrite missed CSSLint plugin in Stylelint.
  3. CSSLint 2.0 will use Stylelint as backend, but work with same old CSSLint config (it will convert CSSLint config to Stylelint config inside)
  4. We provide some migration tool to convert CSSLint config to Stylelint config.

    Benefits for CSSLint Developers

  5. More developers will work for one tool — so less time for boring support tasks and more for developing new smart rules.
  6. You will be a bigger project maintainer — more glory and respect. After merging, you will increase user base to 70%. Facebook, Github, US gov 18F, Wikipedia uses Stylelint. Bootstrap is in the migration to Stylelint.
  7. Stylelint uses PostCSS — you will get many benefits from PostCSS ecosystem: SCSS, Less, Broken CSS and SugarSS parsers. There are useful development tools for PostCSS: rules benchmark and AST explorer. Autoprefixer brings a huge user base to be sure in CSS parser.
  8. With PostCSS AST you could fix lint issues in CSS like eslint --fix does in JS. Stylelint already could do it for some rules with Stylefmt.
  9. PostCSS parser is DOM-like, instead of SAX-like ParserLib. But with postcss-selector-parser, postcss-value-parser, and postcss-media-query-parser it will have same abilities for you.

    Benefits for CSSLint Users

  10. They will get more rules. Stylelint has around 150 rules including very smart: no-unsupported-browser-features, no-indistinguishable-colors, no-browser-hacks, no-descending-specificity
  11. They will get SCSS and Less support.
  12. With Stylefmt they will be able to fix CSS automatically according to linter rules.
  13. They will be able to parse CSS once for Autoprefixer and linter. Parsing is the longest part of CSS tools.
  14. They will have postcss-browser-reporter.
  15. Also people like modular architecture for linters — ESLint is the great example. Stylelint based on same ESLint modular philosophy.
  16. Users love when two awesome projects merge, so they are free from choosing what is best — for example, look at the reaction on ESLint + JSCS merge.
XhmikosR commented 8 years ago

Hey there.

All this seems nice, but personally I have very limited time at this point.

If there are developers willing to do this not just for a week, I'm totally in favor of this.

Unfortunately we lack of manpower. I tried getting out a stable release but I'm all alone and with limited time for CSS Lint.

On Jul 22, 2016 17:10, "Andrey Sitnik" notifications@github.com wrote:

I think if we will merge CSSLint and Stylelint http://stylelint.io/ everyone will win — users will get more features, and we will have more free time. We already have great examples of ESLint + JSCS http://eslint.org/blog/2016/04/welcoming-jscs-to-eslint, Babel + esnext https://babeljs.io/blog/2015/01/12/6to5-esnext, Rails + Merb http://yehudakatz.com/2008/12/23/rails-and-merb-merge/.

@XhmikosR https://github.com/XhmikosR @frvge https://github.com/frvge @davidtheclark https://github.com/davidtheclark @jeddy3 https://github.com/jeddy3 what do you think? Merging Plan Example

  1. CSSLint developers will get a membership in Stylelint core team.
  2. We check Stylelint rules and rewrite missed CSSLint plugin in Stylelint.
  3. CSSLint 2.0 will use Stylelint as backend, but work with same old CSSLint config (it will convert CSSLint config to Stylelint config inside)
  4. We provide some migration tool to convert CSSLint config to Stylelint config.

Benefits for CSSLint Developers

  1. More developers will work for one tool — so less time for boring support tasks and more for developing new smart rules.
  2. You will be a bigger project maintainer — more glory and respect. After merging, you will increase user base to 70%. Facebook https://code.facebook.com/posts/879890885467584/improving-css-quality-at-facebook-and-beyond/, Github https://github.com/primer/stylelint-config-primer, US gov 18F https://github.com/18F/stylelint-rules, Wikipedia https://github.com/wikimedia/stylelint-config-wikimedia uses Stylelint. Bootstrap is in the migration https://github.com/ntwb/stylelint-config-bootstrap to Stylelint.
  3. Stylelint uses PostCSS https://github.com/postcss/postcss — you will get many benefits from PostCSS ecosystem: SCSS https://github.com/postcss/postcss-scss, Less https://github.com/webschik/postcss-less, Broken CSS https://github.com/postcss/postcss-safe-parser and SugarSS https://github.com/postcss/sugarss parsers. There are useful development tools for PostCSS: rules benchmark https://github.com/postcss/postcss-devtools and AST explorer http://astexplorer.net/#/np0DfVT78g/1. Autoprefixer brings a huge user base to be sure in CSS parser.
  4. With PostCSS AST you could fix lint issues in CSS like eslint --fix does in JS. Stylelint already could do it for some rules with Stylefmt https://github.com/morishitter/stylefmt.
  5. PostCSS parser is DOM-like, instead of SAX-like ParserLib. But with postcss-selector-parser https://github.com/postcss/postcss-selector-parser, postcss-value-parser https://github.com/TrySound/postcss-value-parser, and postcss-media-query-parser https://github.com/dryoma/postcss-media-query-parser it will have same abilities for you.

Benefits for CSSLint Users

  1. They will get more rules. Stylelint has around 150 rules http://stylelint.io/user-guide/rules/ including very smart: no-unsupported-browser-features http://stylelint.io/user-guide/rules/no-unsupported-browser-features/, no-indistinguishable-colors http://stylelint.io/user-guide/rules/no-indistinguishable-colors/, no-browser-hacks http://stylelint.io/user-guide/rules/no-browser-hacks/, no-descending-specificity http://stylelint.io/user-guide/rules/no-descending-specificity/
  2. They will get SCSS https://github.com/postcss/postcss-scss and Less https://github.com/webschik/postcss-less support.
  3. With Stylefmt https://github.com/morishitter/stylefmt they will be able to fix CSS automatically according to linter rules.
  4. They will be able to parse CSS once for Autoprefixer and linter. Parsing is the longest part of CSS tools.
  5. They will have postcss-browser-reporter https://github.com/postcss/postcss-browser-reporter.
  6. Also people like modular architecture for linters — ESLint is the great example. Stylelint based on same ESLint modular philosophy.
  7. Users love when two awesome projects merge, so they are free from choosing what is best — for example, look at the reaction on ESLint + JSCS merge.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/CSSLint/csslint/issues/668, or mute the thread https://github.com/notifications/unsubscribe-auth/AAVVtWwFHkRPNoMR0U-0A7i1653Gxl7Wks5qYM9VgaJpZM4JSy1b .

ai commented 8 years ago

@XhmikosR sure, maintaining is hard. But this merging could dramatically help you with lack of manpower ;).

I will ask my followers to help.

I think the first task will be to compare CSSLint and Stylelint rules.

maraisr commented 8 years ago

I'm happy to help! So use me ☝🏻️

Xaviju commented 8 years ago

I'll be happy to help as well. Keep me informed!

yepninja commented 8 years ago

I am ready to help, too.

mjovel commented 8 years ago

Happy to help!

ai commented 8 years ago

@mjovel @alanev @Xaviju @maraisr Awesome! Also I have Kirbaba developer from Russia, who want to have.

First step is to check CSSLint rules — what we can already implement in Stylelint.

Here is CSSLint rules: https://github.com/CSSLint/csslint/wiki/Rules. I count 31 rules. We have 5 developers. Let’s everyone take 6 rules:

  1. Kirbaba: box-model, display-property-grouping, duplicate-properties, empty-rules, known-properties, adjoining-classes
  2. @maraisr: box-sizing, compatible-vendor-prefixes, gradients, text-indent, vendor-prefix, fallback-colors
  3. @Xaviju: star-property-hack, underscore-property-hack, bulletproof-font-face, font-faces, import, regex-selectors
  4. @alanev: universal-selector, unqualified-attributes, zero-units, overqualified-elements, shorthand, duplicate-background-images
  5. @mjovel: floats, font-sizes, ids, important, qualified-headings, unique-headings

I need you to check every rule and mark it with:

davidtheclark commented 8 years ago

It's awesome that all of you stepped forward to help with this!

One of the things I imagine will need to happen during the course of development is the creation of a stylelint-csslint plugin collection. See stylelint-scss for a good existing example. A number of the CSSLint rules look like they'll end up in that plugin collection rather than in stylelint's core rule set (e.g. floats, unique-headings, etc.). We can talk about each individually, of course, but just clueing you all in to a part of the process I anticipate — something for someone to take charge of if interested.

maraisr commented 8 years ago

@davidtheclark yeah mate - I think its so amazing too!! My two cents are, I think that @ai wanted to iron out the discrepancy between having 2 toolsets that do roughly the exact same thing, and merge them into 1 toolchain, like JSCS and ESlint did.

davidtheclark commented 8 years ago

@maraisr Yep, sounds right! I'm not suggesting that the merge wouldn't involve enhancements to stylelint, if it sounded like that. Just suggesting that I think to full compatibility with CSSLint's rule set will involve a combination of enhancements to existing rules, new rules, and a stylelint-csslint plugin collection.

ai commented 8 years ago

@davidtheclark sure, we could put extra rules in different place if they are should not be stylelint core.

Also where we can find good third-party rules list (for CSSLint rules checking)?

davidtheclark commented 8 years ago

I don't think anyone's compiled a comprehensive list of third party rules. @jeddy3 might know more than me here. But I've relied just on npm searches to see what's out there --- kind of weak.

maraisr commented 8 years ago

@davidtheclark yeah - will be interesting! In my mind I see a "polymorphic" style approach, where by you can load both your long standing csslint, and or stylelint config file, with a general move toward a standard config.

jeddy3 commented 8 years ago

It's awesome that all of you stepped forward to help with this!

Agreed. Thanks everyone!

Just suggesting that I think to full compatibility with CSSLint's rule set will involve a combination of enhancements to existing rules, new rules, and a stylelint-csslint plugin collection.

Agreed. To expand on what @davidtheclark said, rules are only included in core if they meet these criteria. This is to ensure development of the linter remains sustainable.

@jeddy3 might know more than me here. But I've relied just on npm searches to see what's out there

Plugin authors are asked to include the stylelint-plugin keyword in their package.json. Plugins that currently do so are listed here.

Bare in mind that around three quarters of these plugins are deprecated (or have pending deprecate issues) as they can now be handled by core rules e.g. the deprecated plugin stylelint-known-property → the core rule property-no-unknown. I believe the only viable plugins (that aren't already on our curated list of plugins) are:

In my mind I see a "polymorphic" style approach, where by you can load both your long standing csslint, and or stylelint config file, with a general move toward a standard config.

Would a simple migration guide from CSSLint to stylelint (in the same manner as this one for scss-lint) not be the simplest option here? If someone feels adventurous they could create a standalone tool (and/or website) that converts a CSSLint config into a stylelint one.

Either way, I agree with @ai's first step. Once we have a better idea of what CSSLint rules are already achievable using stylelint, we'll be in a better position to decide what approach is best.

@maraisr @Xaviju @alanev @mjovel FYI, you'll find the complete list of stylelint's rules here. If you're not already familiar with stylelint you might find this guide about rules useful, as it explains the naming convention and how rules are designed to work together. There is also this part of the Developer Guide that details the option conventions we use. Good luck and please don't hesitate to holler if you've any questions about the workings and conventions of stylelint :)

delorge commented 8 years ago

I'm a bit late but I want to help as well :) guess I will join later

Kirbaba commented 8 years ago

1. The same Stylelint rules "empty-rules" is same as the "block-no-empty"

2. Similar rules in Stylelint, but we need to add some features or options: Stylelint "declaration-block-no-ignored-properties" is same as CSSLint "display-property-grouping" but

there is no rule for combination of inline-block displaying with float.

"duplicate-properties" is looks like "declaration-block-no-duplicate-properties" but there is nothing about checking properties which containing the same value. For example: CSSLint considered warnings for:

.mybox {
    border: 1px solid black;
    border: 1px solid black;
}

3. There is no same rules in Stylelint The CSSLint rules "adjoining-classes" and "box-model" doesn’t have the Stylelint analogs.

"known-properties" in the CSSLint is checking a value and ignoring all vendor-prefixes. Stylelint "property-no-unknown" is checking only property and isn't ignoring a prefixes.

ai commented 8 years ago

@jeddy3 awesome migration guide. But it doesn’t really work.

  1. User should learn that there is Stylelint.
  2. User should find this guide (I didn’t saw it, before you mention it).
  3. User should find working time to do all steps manually.

As result we will have few % of users in the end of this funnel.

Of course for users will be much easy to just change "csslint": "^1.0.0" to "csslint": "^2.0.0" or run something npm install stylelint-csslint stylelint; stylelint-csslint convert ./.

nschonni commented 8 years ago

I'm more 👎 on this, but since I've been pretty dormant I'll go with what the other maintainers go with. Few points:

I will probably use Stylelint in new projects projects where it makes sense, just like I've transitioned some to ESLint where it makes sense. You folks are doing awesome stuff, and I've enjoyed the Autoprefixer for along time.

ai commented 8 years ago

@nschonni

  1. CSSLint and Stylelint are both functional linting. Am I right?
  2. Yeap, npm size is always problem right now. But I think most of users prefer more rules, than less file size.
  3. PostCSS and Stylelint definitely support node.js and browser. I supported Java/Rhino, but have no good tests right now. Could you recommend Travis CI solution, to test PostCSS/Stylelint with Rhino? What is WSH?
ai commented 8 years ago

@delorge feel free to take someone rules task if they will not get review in this evening :)

davidtheclark commented 8 years ago

@nschonni @ai I don't know what "functional linting" refers to, and don't see a definition on a quick search. But I agree that there's a difference in approach. I wouldn't say that CSSLint is more like JSHint than stylelint, in trying to catch real code errors (both do that); and I also wouldn't say that stylelint is restricted to "style-only" rules (there are plenty of non-style rules). A distinction that does exist, I think --- and maybe this is what you're referring to --- is that CSSLint includes rules like "too many floats", which are more like provisional suggestions that you may override in code review rather than auto-enforceable rules. (Those are in fact the kind of rules that I think would make for better stylelint plugins than core rules.)

The reason for the merge would not be to stop enforcement of those CSSLint-specific rules (plugin collection would work), but to share infrastructure and maintenance, and possibly even extend those CSSLint-specific rules to a wider audience (e.g. SCSS users).

(Why are people worried about filesize for npm modules? Is it just download time when running npm install?)

Kirbaba commented 8 years ago

2. Similar rules in Stylelint, but we need to add some features or options:

Stylelint rule "no-browser-hacks" can be extended to include the CSSLint rules "start-property-hack" and "underscore-property-hack"

3.There is no same rules in Stylelint

ai commented 8 years ago

@alanev did his part too, but create for it separated issue https://github.com/CSSLint/csslint/issues/669

ai commented 8 years ago

We need to check only @Xaviju and @mjovel rules pack and then we are ready to go to next step

delorge commented 8 years ago

@ai neither @Xaviju nor @mjovel showed up, I'll take @mjovel tasks

davidtheclark commented 8 years ago

@ai Thanks for organizing! What do you envision as the next step? One possibility would be to open issues for all the CSSLint rules the do not have already have direct substitutes so we can discuss how to handle each one individually.

Xaviju commented 8 years ago

I'm working on my task, it should be ready today, sorry :+1:

delorge commented 8 years ago

Existing Stylelint rules

  1. ids -> selector-no-id
  2. important -> declaration-no-important

CSSLint unique rules

  1. floats
  2. font-sizes

CSSLint rules I'm not sure about

  1. qualified-headings
  2. unique-headings

There are no such rules in stylelint, but selector-no-type dissalows

h3 {
    font-weight: normal;
}

and has an option to allow descendant type selectors which may cause conflict with this dissalowed declaration:

.box h3 {
    font-weight: bold;
}

I guess we need separate rules for these ones, but I don't know how to handle possible conflicts.

@ai @davidtheclark @jeddy3 any thoughts?

Xaviju commented 8 years ago

Disallow star hack

x Similar to no-browser-hacks

Stylelint disallow browser hacks that are irrelevant to the browsers you are targeting. Disallow star hack only points to this specific IE hack but stylelint option is more powerful.

underscore-property-hack

x Similar to no-browser-hacks

Same as above

bulletproof-font-face

n We have no same rules in Stylelint.

This rule is aimed at preventing 404 errors in Internet Explorer 8 and earlier due to a bug in how web font URLs are parsed.

font-faces

n We have no same rules in Stylelint.

Don't use too many web fonts

import

x Similar to At rule blacklist

This rule warns when @import is being used in CSS. In stylelint you can specify a blacklist of disallowed at-rules instead of a specific rule for import at-rule. Is more flexible.

regex-selectors

x Similar to selector-attribute-operator-blacklist

Disallow selectors that look like regular expressions In stylelint you can specify a list of disallowed attribute operators. If we leave it as it is in Stylelint, we will miss the useful recommendations of CSSlint on what are good and bad performance uses of this selectors.

mjovel commented 8 years ago

sorry. yesterday was a travel day. Thanks for picking my set up @delorge

ai commented 8 years ago

Step 2 — we need to collect data into 2 lists:

  1. CSSLint Rules that we need to write.
  2. Stylelint rules that didn’t match exactly — we will think what Stylelint rules we need to extend or what rules we need to rewrite.

Sorry, today I will prepare my slides for talk, so I need help. @delorge @Kirbaba @Xaviju ?

ai commented 8 years ago

@XhmikosR what is the reason behind gradient, compatible-vendor-prefixes, vendor-prefix rules? I think this is bad practice, because of Autoprefixer. Maybe we could miss them in next release? What do you think?

Also right now we don’t need prefixes for gradients in most of browsers http://caniuse.com/#search=gradients

XhmikosR commented 8 years ago

Well when that rule was added Autoprefixer didn't even exist ;)

Nowadays it doesn't make a lot of sense but I'm not sure about the IE case. Other than that maybe we should show a warning or have a target browser config similar to Autoprefixer and display errors then.

On Jul 26, 2016 23:10, "Andrey Sitnik" notifications@github.com wrote:

@XhmikosR https://github.com/XhmikosR what is the reason behind gradient, compatible-vendor-prefixes, vendor-prefix rules? I think this is bad practice, because of Autoprefixer. Maybe we could miss them in next release? What do you think?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/CSSLint/csslint/issues/668#issuecomment-235389319, or mute the thread https://github.com/notifications/unsubscribe-auth/AAVVtSEEuMk16rXoIY9AFB44jb8dycJIks5qZmmfgaJpZM4JSy1b .

maraisr commented 8 years ago

@ai I'll be on tonight (AEST, GMT+10) to complete my list, I've been reading the thread.

yepninja commented 8 years ago

CSSLint rule: universal-selector Type: y Stylelint rule: selector-no-universal: true


CSSLint rule: unqualified-attributes Type: n There is no rule in stylelint that analyze attribute selectors. There is rule selector-no-attribute, but it disallow all selectors with attribute.


CSSLint rule: zero-units Type: y Stylelint rule: length-zero-no-unit: true


CSSLint rule: overqualified-elements Type: x Stylelint rule: selector-no-qualifying-type Stylelint disallows all qualifying selectors. CSSLint allows qualifying selectors if elements are different. The difference is not big.


CSSLint rule: shorthand Type: n


CSSLint rule: duplicate-background-images Type: n There is no rules in styleslint that analyze urls at all.

hkrutzer commented 8 years ago

@ai WSH is most likely Windows Scripting Host

delorge commented 8 years ago

Alright, let's sum everything up

Existing StyleLint rules empty-rules -> block-no-empty ids -> selector-no-id important -> declaration-no-important universal-selector -> selector-no-universal zero-units -> length-zero-no-unit

StyleLint rules that should be reworked/ updated display-property-grouping -> declaration-block-no-ignored-properties duplicate-properties -> declaration-block-no-duplicate-properties qualified-headings, unique-headings -> selector-no-type help wanted star-property-hack, underscore-property-hack -> no-browser-hacks overqualified-elements -> selector-no-qualifying-type box-sizing -> property-blacklist

Unique CSSLint rules that should be written adjoining-classes box-model known-properties help wanted text-indent fallback-colors bulletproof-font-face font-faces unqualified-attributes shorthand duplicate-background-images floats font-sizes

Should be deprecated gradient compatible-vendor-prefixes vendor-prefix gradients import -> at-rule-blacklist regex-selectors -> selector-attribute-operator-blacklist

Rules that are confusing or reported not clear enough I marked as help wanted .

If you found any mistakes, wrong links, etc, please let me know.

frvge commented 8 years ago

I'm mostly just helping out with basic stuff/triage/quick-fixes. I leave the decisions up to the others.

maraisr commented 8 years ago

@delorge thank you for the overview! We're getting close guys!

alexander-akait commented 8 years ago

@delorge

alexander-akait commented 8 years ago

@delorge also i don't understand what need reworked/updated in rules:

davidtheclark commented 8 years ago

I suggest as a next step to open individual issues in stylelint for each rule or option that you think should be added. We could also open issues for individual plugins, either there or here, now or later. That way we can discuss each case in its own individual issue. I think that will be more effective than trying to address them all here in this one thread.

alexander-akait commented 8 years ago

@davidtheclark maybe open one issue CCSLint compatibility with all rules CSSLint rules and step by step open issue on rules?

maraisr commented 8 years ago

@davidtheclark i tend to agree, the issues will allow us to in effect keep track of progress!

Kirbaba commented 8 years ago

1. Need to write:

2. Need to extend or rewrite:

delorge commented 8 years ago

@evilebottnawi I didn't examine most of the rules, I just put everything in order :)

import -> at-rule-blacklist regex-selectors -> selector-attribute-operator-blacklist You better ask @Xaviju about that.

delorge commented 8 years ago

@evilebottnawi agreed with box-sizing, moving this rule to update category.

Xaviju commented 8 years ago

@delorge @evilebottnawi I don't think they need to be rewritten. Both at-rule-blacklist and selector-attribute-operator-blacklist are more powerful and already include the same options than CSSLint. I just pointed out that are not exactly similar.

delorge commented 8 years ago

@Xaviju alright, moving this to deprecated category

alexander-akait commented 8 years ago

Move to https://github.com/stylelint/stylelint/issues/1739, let's go