Fixes #3205, partial 3.0 math regression #1880 (#3228)

* Fixes Mixin call args not being visited * Add ability to use ES6 in tests * Fixes #3205 and partial 3.0 math regression #1880
less · Jun 27, 2018 · 566f428 · 566f428
1 parent b8140d4
commit 566f428
Show file tree

Hide file tree

Showing 15 changed files with 175 additions and 51 deletions.
diff --git a/.eslintrc.json b/.eslintrc.json
@@ -3,11 +3,8 @@
     "node": true
   },
   "globals": {},
+
   "rules": {
-    "quotes": [
-      1,
-      "single"
-    ],
     "no-eval": 2,
     "no-use-before-define": [
       2,

diff --git a/Gruntfile.js b/Gruntfile.js
@@ -295,7 +295,7 @@ module.exports = function (grunt) {
             main: {
                 // src is used to build list of less files to compile
                 src: [
-                    'test/less/*.less',
+                    'test/less/plugin.less',
                     // Don't test NPM import, obviously
                     '!test/less/plugin-module.less',
                     '!test/less/import-module.less',

diff --git a/lib/less/parser/parser-input.js b/lib/less/parser/parser-input.js
@@ -239,7 +239,6 @@ module.exports = function() {
                     case '/':
                         if (input.charAt(i + 1) === '*') {
                             i++;
-                            console.log(input.substr(lastPos, i - lastPos));
                             inComment = true;
                             blockDepth++;
                         }
@@ -324,6 +323,10 @@ module.exports = function() {
         return input.charAt(parserInput.i);
     };
 
+    parserInput.prevChar = function() {
+        return input.charAt(parserInput.i - 1);
+    };
+
     parserInput.getInput = function() {
         return input;
     };

diff --git a/lib/less/parser/parser.js b/lib/less/parser/parser.js
@@ -1302,7 +1302,7 @@ var Parser = function Parser(context, imports, fileInfo) {
 
                         // Custom property values get permissive parsing
                         if (name[0].value && name[0].value.slice(0, 2) === '--') {
-                            value = this.permissiveValue(';');
+                            value = this.permissiveValue();
                         }
                         // Try to store values as anonymous
                         // If we need the value later we'll re-parse it in ruleset.parseValue
@@ -1318,13 +1318,12 @@ var Parser = function Parser(context, imports, fileInfo) {
                         if (!value) {
                             value = this.value();
                         }
-
-                        important = this.important();
-
-                        // As a last resort, let a variable try to be parsed as a permissive value
+                        // As a last resort, try permissiveValue
                         if (!value && isVariable) {
-                            value = this.permissiveValue(';');
+                            value = this.permissiveValue();
                         }
+
+                        important = this.important();
                     }
 
                     if (value && this.end()) {
@@ -1346,27 +1345,76 @@ var Parser = function Parser(context, imports, fileInfo) {
                 }
             },
             /**
-             * Used for custom properties and custom at-rules
+             * Used for custom properties, at-rules, and variables (as fallback)
              * Parses almost anything inside of {} [] () "" blocks
              * until it reaches outer-most tokens.
+             * 
+             * First, it will try to parse comments and entities to reach
+             * the end. This is mostly like the Expression parser except no
+             * math is allowed.
              */
             permissiveValue: function (untilTokens) {
-                var i, index = parserInput.i,
-                    value = parserInput.$parseUntil(untilTokens);
+                var i, e, done, value, 
+                    tok = untilTokens || ';',
+                    index = parserInput.i, result = [];
+
+                function testCurrentChar() {
+                    var char = parserInput.currentChar();
+                    if (typeof tok === 'string') {
+                        return char === tok;
+                    } else {
+                        return tok.test(char);
+                    }
+                }
+                if (testCurrentChar()) {
+                    return;
+                }
+                value = [];
+                do {
+                    e = this.comment();
+                    if (e) {
+                        value.push(e);
+                        continue;
+                    }
+                    e = this.entity();
+                    if (e) {
+                        value.push(e);
+                    }
+                } while (e);
+
+                done = testCurrentChar();
+
+                if (value.length > 0) {
+                    value = new(tree.Expression)(value);
+                    if (done) {
+                        return value;
+                    }
+                    else {
+                        result.push(value);
+                    }
+                    // Preserve space before $parseUntil as it will not
+                    if (parserInput.prevChar() === ' ') {
+                        result.push(new tree.Anonymous(' ', index));
+                    }
+                }
+                parserInput.save();
+
+                value = parserInput.$parseUntil(tok);
 
                 if (value) {
                     if (typeof value === 'string') {
                         error('Expected \'' + value + '\'', 'Parse');
                     }
                     if (value.length === 1 && value[0] === ' ') {
+                        parserInput.forget();
                         return new tree.Anonymous('', index);
                     }
-                    var item, args = [];
+                    var item;
                     for (i = 0; i < value.length; i++) {
                         item = value[i];
                         if (Array.isArray(item)) {
                             // Treat actual quotes as normal quoted values
-                            args.push(new tree.Quoted(item[0], item[1], true, index, fileInfo));
+                            result.push(new tree.Quoted(item[0], item[1], true, index, fileInfo));
                         }
                         else {
                             if (i === value.length - 1) {
@@ -1376,12 +1424,13 @@ var Parser = function Parser(context, imports, fileInfo) {
                             var quote = new tree.Quoted('\'', item, true, index, fileInfo);
                             quote.variableRegex = /@([\w-]+)/g;
                             quote.propRegex = /\$([\w-]+)/g;
-                            quote.reparse = true;
-                            args.push(quote);
+                            result.push(quote);
                         }
                     }
-                    return new tree.Expression(args, true);
+                    parserInput.forget();
+                    return new tree.Expression(result, true);
                 }
+                parserInput.restore();
             },
 
             //
@@ -1463,7 +1512,7 @@ var Parser = function Parser(context, imports, fileInfo) {
                         nodes.push(e);
                     } else if (parserInput.$char('(')) {
                         p = this.property();
-                        e = this.value();
+                        e = this.permissiveValue(')');
                         if (parserInput.$char(')')) {
                             if (p && e) {
                                 nodes.push(new(tree.Paren)(new(tree.Declaration)(p, e, null, null, parserInput.i, fileInfo, true)));

diff --git a/lib/less/transform-tree.js b/lib/less/transform-tree.js
@@ -41,29 +41,49 @@ module.exports = function(root, options) {
             new visitor.MarkVisibleSelectorsVisitor(true),
             new visitor.ExtendVisitor(),
             new visitor.ToCSSVisitor({compress: Boolean(options.compress)})
-        ], v, visitorIterator;
+        ], preEvalVisitors = [], v, visitorIterator;
 
-    // first() / get() allows visitors to be added while visiting
+    /**
+     * first() / get() allows visitors to be added while visiting
+     * 
+     * @todo Add scoping for visitors just like functions for @plugin; right now they're global
+     */
     if (options.pluginManager) {
         visitorIterator = options.pluginManager.visitor();
-        visitorIterator.first();
-        while ((v = visitorIterator.get())) {
-            if (v.isPreEvalVisitor) {
-                v.run(root);
+        for (var i = 0; i < 2; i++) {
+            visitorIterator.first();
+            while ((v = visitorIterator.get())) {
+                if (v.isPreEvalVisitor) {
+                    if (i === 0 || preEvalVisitors.indexOf(v) === -1) {
+                        preEvalVisitors.push(v);
+                        v.run(root);
+                    }
+                }
+                else {
+                    if (i === 0 || visitors.indexOf(v) === -1) {
+                        if (v.isPreVisitor) {
+                            visitors.unshift(v);
+                        }
+                        else {
+                            visitors.push(v);
+                        }
+                    }
+                }
             }
         }
     }
-
+    
     evaldRoot = root.eval(evalEnv);
 
     for (var i = 0; i < visitors.length; i++) {
         visitors[i].run(evaldRoot);
     }
 
+    // Run any remaining visitors added after eval pass
     if (options.pluginManager) {
         visitorIterator.first();
         while ((v = visitorIterator.get())) {
-            if (!v.isPreEvalVisitor) {
+            if (visitors.indexOf(v) === -1 && preEvalVisitors.indexOf(v) === -1) {
                 v.run(evaldRoot);
             }
         }

diff --git a/lib/less/tree/expression.js b/lib/less/tree/expression.js
@@ -24,6 +24,9 @@ Expression.prototype.eval = function (context) {
     }
     if (this.value.length > 1) {
         returnValue = new Expression(this.value.map(function (e) {
+            if (!e.eval) {
+                return e;
+            }
             return e.eval(context);
         }), this.noSpacing);
     } else if (this.value.length === 1) {

diff --git a/lib/less/visitors/visitor.js b/lib/less/visitors/visitor.js
@@ -32,7 +32,8 @@ function indexNodeTypes(parent, ticker) {
 
 var Visitor = function(implementation) {
     this._implementation = implementation;
-    this._visitFnCache = [];
+    this._visitInCache = {};
+    this._visitOutCache = {};
 
     if (!_hasIndexed) {
         indexNodeTypes(tree, 1);
@@ -48,15 +49,16 @@ Visitor.prototype = {
 
         var nodeTypeIndex = node.typeIndex;
         if (!nodeTypeIndex) {
+            // MixinCall args aren't a node type?
+            if (node.value && node.value.typeIndex) {
+                this.visit(node.value);
+            }
             return node;
         }
 
-        var visitFnCache = this._visitFnCache,
-            impl = this._implementation,
-            aryIndx = nodeTypeIndex << 1,
-            outAryIndex = aryIndx | 1,
-            func = visitFnCache[aryIndx],
-            funcOut = visitFnCache[outAryIndex],
+        var impl = this._implementation,
+            func = this._visitInCache[nodeTypeIndex],
+            funcOut = this._visitOutCache[nodeTypeIndex],
             visitArgs = _visitArgs,
             fnName;
 
@@ -66,13 +68,13 @@ Visitor.prototype = {
             fnName = 'visit' + node.type;
             func = impl[fnName] || _noop;
             funcOut = impl[fnName + 'Out'] || _noop;
-            visitFnCache[aryIndx] = func;
-            visitFnCache[outAryIndex] = funcOut;
+            this._visitInCache[nodeTypeIndex] = func;
+            this._visitOutCache[nodeTypeIndex] = funcOut;
         }
 
         if (func !== _noop) {
             var newNode = func.call(impl, node, visitArgs);
-            if (impl.isReplacing) {
+            if (node && impl.isReplacing) {
                 node = newNode;
             }
         }

diff --git a/test/.eslintrc.json b/test/.eslintrc.json
@@ -0,0 +1,9 @@
+{
+  "env": {
+    "node": true,
+    "browser": true
+  },
+  "parserOptions": { 
+    "ecmaVersion": 6
+  }
+}
diff --git a/test/css/permissive-parse.css b/test/css/permissive-parse.css
@@ -12,8 +12,8 @@
     basically anything until final semi-colon;
     even other stuff; // i\'m serious;
   };
-  --custom-color: #ff3333;
-  custom-color: #ff3333;
+  --custom-color: #ff3333 #ff3333;
+  custom-color: #ff3333 #ff3333;
 }
 .var {
   --fortran: read (*, *, iostat=1) radius, height;
@@ -32,7 +32,6 @@ foo[attr="blah"] {
   }
 }
 .test-comment {
-  --value: ;
+  --value: a /* { ; } */;
   --comment-within: ( /* okay?; comment; */ );
-  --empty: ;
 }
diff --git a/test/css/plugin-preeval.css b/test/css/plugin-preeval.css
@@ -0,0 +1,3 @@
+:root.two .one {
+  --foo: bar !important;
+}
diff --git a/test/less/errors/at-rules-unmatching-block.txt b/test/less/errors/at-rules-unmatching-block.txt
@@ -1,4 +1,3 @@
-SyntaxError: @unknown rule is missing block or ending semi-colon in {path}at-rules-unmatching-block.less on line 2, column 10:
-1 
-2 @unknown url( {
-3     50% {width: 20px;}
+SyntaxError: expected ')' got '' in {path}at-rules-unmatching-block.less on line 5, column 1:
+4 }
+5 
diff --git a/test/less/media.less b/test/less/media.less
@@ -23,7 +23,7 @@
 @ratio_large: 16;
 @ratio_small: 9;
 
-@media all and (device-aspect-ratio: ~"@{ratio_large} / @{ratio_small}") {
+@media all and (device-aspect-ratio: @ratio_large / @ratio_small) {
    body { max-width: 800px; }
 }
 

diff --git a/test/less/permissive-parse.less b/test/less/permissive-parse.less
@@ -17,7 +17,7 @@
   };
   --that: @this;
   @red: lighten(red, 10%);
-  --custom-color: @red;
+  --custom-color: @red lighten(red, 10%);
   custom-color: $--custom-color;
 }
 
@@ -45,7 +45,6 @@
 }
 // @todo - fix comment absorption after property
 .test-comment {
-  --value: /* { ; } */;
+  --value: a/* { ; } */;
   --comment-within: ( /* okay?; comment; */ );
-  --empty: ;
 }
diff --git a/test/less/plugin-preeval.less b/test/less/plugin-preeval.less
@@ -0,0 +1,15 @@
+@plugin "./plugin/plugin-preeval";
+
+.two(@rules: {}) {
+    :root.two & {
+        @rules();
+    }
+}
+
+.one {
+    .two({
+        --foo: @replace !important;
+    });
+}
+
+@stop: end;