Issue 5109 - Create stylelintrc for websites and ui modules

stylelint-config-eyeo/index.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-"use strict"
+"use strict";
 
 module.exports = {
-  "extends": "stylelint-config-recommended",
-  "rules": {
+  extends: "stylelint-config-recommended",
+  rules: {
+    // Opening braces go on their own line
+    "block-opening-brace-newline-before": "always-multi-line",
+    "block-opening-brace-newline-after": "always-multi-line",
+    "block-closing-brace-newline-before": "always-multi-line",
+    "block-closing-brace-newline-after": "always",
+    "block-closing-brace-empty-line-before": "never",
 
-    // Opening braces go on their own line
-    "block-closing-brace-empty-line-before": "never",
-    "block-closing-brace-newline-after": "always",
-    "block-closing-brace-newline-before": "always-multi-line",
+    // Use a space between the last selector and the declaration block
+    "block-opening-brace-space-before": "always-single-line",
     "block-closing-brace-space-after": "always-single-line",
-    "block-closing-brace-space-before": "always-single-line",

[Thomas Greiner, 2017/09/13 18:12:52]

Which part of the coding style does this rule refer to?

Same applies to "block-opening-brace-space-after"

[ire, 2017/09/14 08:33:00]

We link to the Google HTML/CSS Style guide, which has rules on having a space
before opening braces -
https://google.github.io/styleguide/htmlcssguide.html#Declaration_Block_Separ...

[Thomas Greiner, 2017/09/14 10:52:15]

This rule is not about the opening but the closing brace.

[ire, 2017/09/15 08:54:11]

Under 4.2.5 Declaration Block Separation, it says:

    "Always use a single space between the last selector and the opening brace
that begins the declaration block."

[Thomas Greiner, 2017/09/15 16:58:29]

There seems to be a misunderstanding - sorry about that - so I'll try to clarify
it.

The text you mention applies to this space:

  selector { name: value; }
          ^

The rule I'm referring to in my initial comment (i.e.
"block-closing-brace-space-before") applies to this space, however:

  selector { name: value; }
                         ^

The other rule in that comment (i.e. "block-opening-brace-space-after") applies
to this space:

  selector { name: value; }
            ^

[ire, 2017/09/18 07:36:20]

Ah right! Yes seems I was mixing the two up. 

Two questions:
1. Do you disagree with the rule as I implemented it here? i.e. is the issue
that I mislabelled the rule,
 or that the rule is incorrect
2. Do we actually have a rule about closing braces anywhere? 

[Thomas Greiner, 2017/11/14 13:18:30]

I can't find any mentions of it being prohibited in our coding style so it
should be fine. All I could find are other uses of braces in single lines where
we consistently use them without inner spaces.

e.g.
const {foo} = require("bar");
`foo${bar}baz`
let foo = {bar: "baz"};

So I'd suggest either enforcing it without inner spaces or leaving it up to the
developers to choose whichever variant they prefer.

[ire, 2017/11/15 06:38:32]

Okay. I will remove two rules:
"block-opening-brace-space-after" and "block-closing-brace-space-before"

[ire, 2017/11/15 06:38:32]

Done.

-    "block-opening-brace-newline-after": "always-multi-line",
-    "block-opening-brace-newline-before": "always-multi-line",

[Thomas Greiner, 2017/09/13 18:12:52]

Allowing single-line rules here sounds incompatible with the statement "Opening
braces go on their own line".

If it indeed turns out that we don't allow single-line rules, it would
furthermore render rules with the value "always-single-line" redundant.

[ire, 2017/09/14 08:33:00]

I think I assumed that single-line rules were okay when it's just one
declaration. I can't seem to find any rule for/against this though, is this
documented somewhere? (If not, can we decide now?)

[Thomas Greiner, 2017/09/14 10:52:15]

It's the "Opening braces go on their own line" guideline that I mentioned but I
guess you're right because

.foo
{bar: baz;}

would still be considered valid with that rule. I'm not sure whether stylelint
considers that to be a single line though.
We can always agree on changing the rules. :) Personally, I'd be happy with
allowing blocks with a single selector and a single CSS property to be written
in a single line.

Anyway, I created https://issues.adblockplus.org/ticket/5676 to discuss what
we'd like to have and to adapt the coding style accordingly.

[ire, 2017/09/15 08:54:11]

Stylelint will consider that a single line (I believe they count the line as
[opening brace, property, value, closing brace]). See this example -
https://stylelint.io/user-guide/rules/block-opening-brace-newline-before/#alw...

I think the example you gave is something we may not be able to catch with
styelint unless we can to disallow single-line declarations completely.
Ack, thanks!

[Thomas Greiner, 2017/09/15 16:58:29]

True, seems like it.

-    "block-opening-brace-space-after": "always-single-line",
-    "block-opening-brace-space-before": "always-single-line",
+
+    // Use a space after a property name’s colon
+    "declaration-colon-space-after": "always",
+
+    // Selectors and declarations should be on their own line
+    "selector-list-comma-newline-after": "always",
+    "declaration-block-semicolon-newline-after": "always-multi-line",
+
+    // Separate rules by an empty line
+    "rule-empty-line-before": ["always", {
+      "ignore": ["after-comment", "first-nested"]
+    }],
 
     // Use double over single quotation marks
     "string-quotes": "double",
 
     // CSS color values should be specified in hexadecimal where possible

[ire, 2017/09/11 09:26:38]

I couldn't find stylelint rules for these. Not sure if I should leave the
comments anyway?

[Thomas Greiner, 2017/09/13 18:12:52]

It's more of a guideline than a strict rule. Some more info on that (from what I
remember):
- We use `rgba()` instead of `#RRGGBBAA` for compatibility.
- Under some circumstances using `hsl()` or even `rgb()` could be more
appropriate than a hex value so prohibiting those outright is likely not a good
idea.

Therefore `"color-named": "never"` should be sufficient for now.

[ire, 2017/09/14 08:33:00]

Okay thanks. Done.

+    "color-named": "never",
 
-    // CSS shorthand properties usage is optional
-
-    // CSS rule declaration order should follow the WordPress CSS Coding Standards

[ire, 2017/09/11 09:26:38]

The declaration order is no longer part of stylelint. If we really want this to
be part of our stylelint, I think we can use this plugin
(https://www.npmjs.com/package/stylelint-order). 

[Thomas Greiner, 2017/09/13 18:12:52]

Given that this rule can be quite confusing for some people it may be worth
adding unless we want to open the discussion about declaraction order again.

[ire, 2017/09/14 08:33:00]

Okay. I think we should add it in a separate commit/issue though, just so the
initial config isn't held up by this (unless it is essential)

[Thomas Greiner, 2017/09/14 10:52:15]

I totally agree.

[ire, 2017/09/15 08:54:12]

Great. Created the issue here
https://issues.adblockplus.org/ticket/5689#comment:1

[Thomas Greiner, 2017/09/15 16:58:29]

Awesome, thanks.

-
-    // CSS number values should specify units where possible
-    "unit-no-unknown": true,

[Thomas Greiner, 2017/09/13 18:12:52]

This is not what "should specify units where possible" refers to. It's about
cases like `width: 0;` where no unit is provided.

I don't think there's a strong reason to keep it though, in case we want to
abandon it.

[ire, 2017/09/14 08:33:00]

Seems I misunderstood the "unit-no-unknown" rule :/

Stylelint doesn't have a rule for what we want. They have the opposite rule
("length-zero-no-unit"), which requires that zeros should not have a unit. 
I personally prefer to not specify the units when its a 0, so if it's up for
discussion my vote is we abandon that rule.

[Thomas Greiner, 2017/09/14 10:52:15]

I included this in https://issues.adblockplus.org/ticket/5676 so that we can
discuss this and adapt the coding style.

[ire, 2017/09/15 08:54:11]

Acknowledged.

+    // Use short hexadecimal notation where possible
+    "color-hex-length": "short",
 
     // Don't omit the optional leading 0 for decimal numbers
     "number-leading-zero": "always",
     "number-no-trailing-zeros": true,
 
-    // Indent by 2 spaces
+    // Two spaces per logic level
     "indentation": 2,

[ire, 2017/09/11 09:26:38]

This isn't actually documented in https://adblockplus.org/en/coding-style, but
is a rule we seem to follow

[Thomas Greiner, 2017/09/13 18:12:52]

Under "General" we link to
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style...
which states "Two spaces per logic level, or four spaces in Python code."

[ire, 2017/09/14 08:33:00]

Acknowledged.

-    
-  },
-}
+
+    // Line length should be 80 characters or less
+    "max-line-length": 80,
+
+    // Avoid qualifying ID and class names with type selectors
+    "selector-no-qualifying-type": [true, {
+      "ignore": ["attribute"]
+    }]
+  }
+};