Merge 6163fd2 into 9d9fc3f

.eslintrc.yml

@@ -2,6 +2,8 @@ env:
   es2017: true
   node: true
   browser: true
+parserOptions:
+  ecmaVersion: 2020
 extends:
   - 'eslint:recommended'
   - 'plugin:node/recommended'

.github/workflows/tests.yml

@@ -17,7 +17,6 @@ jobs:
       fail-fast: false
       matrix:
         node-version:
-        - '12.x'
         - '14.x'
         - '15.x'
         - '16.x'

lib/css-analyzer.d.ts

@@ -4,7 +4,8 @@
  * See https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html
  */
 
-import { ParserError, StyleRules, Stylesheet } from "css";
+import { StyleRules, Stylesheet, Position } from "css";
+import { Selector } from "css-what";
 import {
   EventsNames,
   MetricsNames,
@@ -25,9 +26,9 @@ declare class CSSAnalyzer {
   public addOffender(
     metricName: MetricsNames,
     msg: string,
-    position: any | undefined
+    position: Position | undefined
   ): void;
-  public setCurrentPosition(position: any): void;
+  public setCurrentPosition(position: Position): void;
 
   // types based on the event
   public on(ev: EventsNames, fn: any): void;
@@ -38,10 +39,10 @@ declare class CSSAnalyzer {
     ev: "declaration",
     fn: (rule: CSSRule, property: string, value: string) => void
   ): void;
-  public on(ev: "error", fn: (err: ParserError) => void): void;
+  public on(ev: "error", fn: (err: Error) => void): void;
   public on(
     ev: "expression",
-    fn: (selector: any, expression: any) => void
+    fn: (selector: string, expression: Selector) => void
   ): void;
   public on(ev: "font-face", fn: (rule: CSSRule) => void): void;
   public on(ev: "import", fn: (url: string) => void): void;
@@ -57,7 +58,7 @@ declare class CSSAnalyzer {
   public on(ev: "rule", fn: (rule: CSSRule) => void): void;
   public on(
     ev: "selector",
-    fn: (rule: CSSRule, selector: string, expressions: any) => void
+    fn: (rule: CSSRule, selector: string, expressions: Selector[]) => void
   ): void;
   public on(ev: "stylesheet", fn: (stylesheet: StyleRules) => void): void;
 

lib/css-analyzer.js

@@ -2,7 +2,7 @@ const analyze = require("."),
   cssParser = require("css").parse,
   debug = require("debug")("analyze-css:analyzer"),
   fs = require("fs"),
-  slickParse = require("slick").parse;
+  CSSwhat = require("css-what");
 
 function error(msg, code) {
   var err = new Error(msg);
@@ -128,7 +128,7 @@ class CSSAnalyzer {
     const debug = require("debug")("analyze-css:parseRules");
 
     rules.forEach(function (rule) {
-      debug("%j", rule);
+      debug("rule: %j", rule);
 
       // store the default current position
       //
@@ -168,37 +168,38 @@ class CSSAnalyzer {
 
           // analyze each selector and declaration
           rule.selectors.forEach(function (selector) {
-            var parsedSelector,
-              expressions = [],
-              i,
-              len;
-
-            // "#features > div:first-child" will become two expressions:
-            //  {"combinator":" ","tag":"*","id":"features"}
-            //  {"combinator":">","tag":"div","pseudos":[{"key":"first-child","value":null}]}
-            parsedSelector = slickParse(selector)[0];
-
-            if (typeof parsedSelector === "undefined") {
-              var positionDump =
-                "Rule position start @ " +
-                rule.position.start.line +
-                ":" +
-                rule.position.start.column +
-                ", end @ " +
-                rule.position.end.line +
-                ":" +
-                rule.position.end.column;
-              throw error(
-                'Unable to parse "' + selector + '" selector. ' + positionDump,
-                analyze.EXIT_PARSING_FAILED
-              );
+            // https://github.com/fb55/css-what#example
+            // "#features > div:first-child" will become:
+            // {"type":"attribute","name":"id","action":"equals","value":"features","namespace":null,"ignoreCase":false}
+            // {"type":"child"}
+            // {"type":"tag","name":"div","namespace":null}
+            // {"type":"pseudo","name":"first-child","data":null}
+            debug("selector: %s", selector);
+
+            let parsedSelector;
+
+            try {
+              parsedSelector = CSSwhat.parse(selector);
+            } catch (err) {
+              /**
+               * > require("css-what").parse('foo { color= red; }');
+                Uncaught Error: Unmatched selector: { color= red; }
+                    at Object.parse (node_modules/css-what/lib/parse.js:139:15)
+              */
+
+              debug("selector parsing failed: %s", err.message);
+              this.emit("error", err);
+              return;
             }
 
             // convert object with keys to array with numeric index
-            for (i = 0, len = parsedSelector.length; i < len; i++) {
-              expressions.push(parsedSelector[i]);
+            let expressions = [];
+
+            for (let i = 0, len = parsedSelector[0].length; i < len; i++) {
+              expressions.push(parsedSelector[0][i]);
             }
 
+            debug("selector expressions: %j", expressions);
             this.emit("selector", rule, selector, expressions);
 
             expressions.forEach(function (expression) {
@@ -252,22 +253,6 @@ class CSSAnalyzer {
 
     this.emit("stylesheet", stylesheet);
 
-    // check for parsing errors (#84)
-    stylesheet.parsingErrors.forEach(function (err) {
-      debug("error: %j", err);
-
-      const pos = {
-        line: err.line,
-        column: err.column,
-      };
-      this.setCurrentPosition({
-        start: pos,
-        end: pos,
-      });
-
-      this.emit("error", err);
-    }, this);
-
     this.parseRules(rules);
   }
 
@@ -292,11 +277,7 @@ class CSSAnalyzer {
     this.emit("css", css);
 
     // now go through parsed CSS tree and emit events for rules
-    try {
-      this.run();
-    } catch (ex) {
-      return ex;
-    }
+    this.run();
 
     this.emit("report");
 

lib/types.d.ts

@@ -69,7 +69,13 @@ export type Metrics = { [metric in MetricsNames]: number };
 /**
  * Encapsulates a set of offenders
  */
-export type Offenders = { [metric in MetricsNames]: Array<Object> };
+import { Position } from "css";
+
+export interface Offender {
+  message: string;
+  position: Position;
+}
+export type Offenders = { [metric in MetricsNames]: Array<Offender> };
 
 /**
  * A CSS rule taken from "css" package

package.json

@@ -18,20 +18,20 @@
   ],
   "license": "BSD-2-Clause",
   "engines": {
-    "node": ">=10.0"
+    "node": ">=14.0"
   },
   "dependencies": {
     "cli": "^1.0.1",
     "commander": "^8.0.0",
     "css": "^3.0.0",
     "css-shorthand-properties": "^1.1.1",
+    "css-what": "^5.0.1",
     "debug": "^4.1.1",
     "fast-stats": "0.0.6",
     "glob": "^7.1.6",
     "http-proxy-agent": "^4.0.1",
     "node-fetch": "^2.6.0",
     "onecolor": "^3.1.0",
-    "slick": "~1.12.1",
     "specificity": "^0.4.1"
   },
   "devDependencies": {
@@ -44,7 +44,7 @@
     "eslint-plugin-node": "^11.1.0",
     "jest": "^27.0.4",
     "nyc": "^15.1.0",
-    "postcss": "^8.2.6",
+    "postcss": "^8.3.6",
     "prettier": "2.3.2"
   },
   "optionalDependencies": {

rules/bodySelectors.js

@@ -1,39 +1,124 @@
 "use strict";
+/**
+ * @typedef { import("css-what").AttributeSelector[] } AttributeSelectors
+ */
+
+/**
+ * @param { AttributeSelectors } expressions
+ * @returns { number }
+ */
+function getBodyIndex(expressions) {
+  let idx = 0;
+
+  // body.foo h1 -> 0
+  // .foo body -> 1
+  // html.css body -> 1
+
+  for (let i = 0; i < expressions.length; i++) {
+    switch (expressions[i].type) {
+      case "tag":
+        if (expressions[i].name === "body") {
+          return idx;
+        }
+        break;
+
+      case "child":
+      case "descendant":
+        idx++;
+    }
+  }
+
+  return -1;
+}
+
+/**
+ * @param { AttributeSelectors } expressions
+ * @returns {boolean}
+ */
+function firstSelectorHasClass(expressions) {
+  // remove any non-class selectors
+  return expressions[0].type === "tag"
+    ? // h1.foo
+      expressions[1].type === "attribute" && expressions[1].name === "class"
+    : // .foo
+      expressions[0].type === "attribute" && expressions[0].name === "class";
+}
+
+/**
+ * @param { AttributeSelectors } expressions
+ * @returns {number}
+ */
+function getDescendantCombinatorIndex(expressions) {
+  // body > .foo
+  // {"type":"child"}
+  return expressions
+    .filter((item) => {
+      return !["tag", "attribute", "pseudo"].includes(item.type);
+    })
+    .map((item) => {
+      return item.type;
+    })
+    .indexOf("child");
+}
 
 /**
  * @param { import("../lib/css-analyzer") } analyzer
  */
 function rule(analyzer) {
+  const debug = require("debug")("analyze-css:bodySelectors");
+
   analyzer.setMetric("redundantBodySelectors");
 
-  analyzer.on("selector", function (rule, selector, expressions) {
-    var noExpressions = expressions.length;
+  analyzer.on("selector", function (_, selector, expressions) {
+    const noExpressions = expressions.length;
 
     // check more complex selectors only
     if (noExpressions < 2) {
       return;
     }
 
-    var firstTag = expressions[0].tag,
-      firstHasClass = !!expressions[0].classList,
-      isDescendantCombinator = expressions[1].combinator === ">",
-      isShortExpression = noExpressions === 2,
-      isRedundant = true; // always expect the worst ;)
+    const firstTag = expressions[0].type === "tag" && expressions[0].name;
+
+    const firstHasClass = firstSelectorHasClass(expressions);
+
+    const isDescendantCombinator =
+      getDescendantCombinatorIndex(expressions) === 0;
+
+    // there only a single descendant / child selector
+    // e.g. "body > foo" or "html h1"
+    const isShortExpression =
+      expressions.filter((item) => {
+        return ["child", "descendant"].includes(item.type);
+      }).length === 1;
+
+    let isRedundant = true; // always expect the worst ;)
 
     // first, let's find the body tag selector in the expression
-    var bodyIndex = expressions
-      .map(function (item) {
-        return item.tag;
-      })
-      .indexOf("body");
+    const bodyIndex = getBodyIndex(expressions);
+
+    debug("selector: %s %j", selector, {
+      firstTag,
+      firstHasClass,
+      isDescendantCombinator,
+      isShortExpression,
+      bodyIndex,
+    });
 
     // body selector not found - skip the rules that follow
     if (bodyIndex < 0) {
       return;
     }
 
     // matches "html > body"
+    // {"type":"tag","name":"html","namespace":null}
+    // {"type":"child"}
+    // {"type":"tag","name":"body","namespace":null}
+    //
     // matches "html.modal-popup-mode body" (issue #44)
+    // {"type":"tag","name":"html","namespace":null}
+    // {"type":"attribute","name":"class","action":"element","value":"modal-popup-mode","namespace":null,"ignoreCase":false}
+    // {"type":"descendant"}
+    // {"type":"tag","name":"body","namespace":null}
     if (
       firstTag === "html" &&
       bodyIndex === 1 &&
@@ -56,6 +141,8 @@ function rule(analyzer) {
 
     // report he redundant body selector
     if (isRedundant) {
+      debug("selector %s - is redundant", selector);
+
       analyzer.incrMetric("redundantBodySelectors");
       analyzer.addOffender("redundantBodySelectors", selector);
     }