fix faulty source map generation with variables in selectors (less#3761)

* fix faulty source map generation * alternative solution * add test
clewarne14 · Apr 2, 2023 · 2702322 · 2702322
1 parent 9b37be7
commit 2702322
Show file tree

Hide file tree

Showing 8 changed files with 113 additions and 89 deletions.
diff --git a/packages/less/src/less/parser/parser.js b/packages/less/src/less/parser/parser.js
diff --git a/packages/less/src/less/tree/ruleset.js b/packages/less/src/less/tree/ruleset.js
@@ -11,6 +11,7 @@ import globalFunctionRegistry from '../functions/function-registry';
 import defaultFunc from '../functions/default';
 import getDebugInfo from './debug-info';
 import * as utils from '../utils';
+import Parser from '../parser/parser';
 
 const Ruleset = function(selectors, rules, strictImports, visibilityInfo) {
     this.selectors = selectors;
@@ -79,11 +80,11 @@ Ruleset.prototype = Object.assign(new Node(), {
                     selector = selectors[i];
                     toParseSelectors[i] = selector.toCSS(context);
                 }
-                this.parse.parseNode(
+                const startingIndex = selectors[0].getIndex();
+                const selectorFileInfo = selectors[0].fileInfo();
+                new Parser(context, this.parse.importManager, selectorFileInfo, startingIndex).parseNode(
                     toParseSelectors.join(','),
-                    ["selectors"], 
-                    selectors[0].getIndex(), 
-                    selectors[0].fileInfo(), 
+                    ["selectors"],
                     function(err, result) {
                         if (result) {
                             selectors = utils.flattenArray(result);
@@ -354,11 +355,9 @@ Ruleset.prototype = Object.assign(new Node(), {
         function transformDeclaration(decl) {
             if (decl.value instanceof Anonymous && !decl.parsed) {
                 if (typeof decl.value.value === 'string') {
-                    this.parse.parseNode(
+                    new Parser(this.parse.context, this.parse.importManager, decl.fileInfo(), decl.value.getIndex()).parseNode(
                         decl.value.value,
-                        ['value', 'important'], 
-                        decl.value.getIndex(), 
-                        decl.fileInfo(), 
+                        ['value', 'important'],
                         function(err, result) {
                             if (err) {
                                 decl.parsed = true;

diff --git a/packages/less/src/less/tree/selector.js b/packages/less/src/less/tree/selector.js
@@ -2,6 +2,7 @@ import Node from './node';
 import Element from './element';
 import LessError from '../less-error';
 import * as utils from '../utils';
+import Parser from '../parser/parser';
 
 const Selector = function(elements, extendList, condition, index, currentFileInfo, visibilityInfo) {
     this.extendList = extendList;
@@ -44,11 +45,9 @@ Selector.prototype = Object.assign(new Node(), {
             return [new Element('', '&', false, this._index, this._fileInfo)];
         }
         if (typeof els === 'string') {
-            this.parse.parseNode(
-                els, 
+            new Parser(this.parse.context, this.parse.importManager, this._fileInfo, this._index).parseNode(
+                els,
                 ['selector'],
-                this._index, 
-                this._fileInfo, 
                 function(err, result) {
                     if (err) {
                         throw new LessError({

diff --git a/packages/less/test/index.js b/packages/less/test/index.js
@@ -63,6 +63,8 @@ var testMap = [
         'sourcemaps-empty/', lessTester.testEmptySourcemap],
     [{math: 'strict', strictUnits: true, sourceMap: {disableSourcemapAnnotation: true}},
         'sourcemaps-disable-annotation/', lessTester.testSourcemapWithoutUrlAnnotation],
+    [{math: 'strict', strictUnits: true, sourceMap: true},
+        'sourcemaps-variable-selector/', lessTester.testSourcemapWithVariableInSelector],
     [{globalVars: true, banner: '/**\n  * Test\n  */\n'}, 'globalVars/',
         null, null, null, function(name, type, baseFolder) { return path.join(baseFolder, name) + '.json'; }],
     [{modifyVars: true}, 'modifyVars/',

diff --git a/packages/less/test/less-test.js b/packages/less/test/less-test.js
@@ -169,6 +169,29 @@ module.exports = function() {
         }
     }
 
+    function testSourcemapWithVariableInSelector(name, err, compiledLess, doReplacements, sourcemap, baseFolder) {
+        if (err) {
+            fail('ERROR: ' + (err && err.message));
+            return;
+        }
+
+        // Even if annotation is not necessary, the map file should be there.
+        fs.readFile(path.join('test/', name) + '.json', 'utf8', function (e, expectedSourcemap) {
+            process.stdout.write('- ' + path.join(baseFolder, name) + ': ');
+            if (sourcemap === expectedSourcemap) {
+                ok('OK');
+            } else if (err) {
+                fail('ERROR: ' + (err && err.message));
+                if (isVerbose) {
+                    process.stdout.write('\n');
+                    process.stdout.write(err.stack + '\n');
+                }
+            } else {
+                difference('FAIL', expectedSourcemap, sourcemap);
+            }
+        });
+    }
+
     function testImports(name, err, compiledLess, doReplacements, sourcemap, baseFolder, imports) {
         if (err) {
             fail('ERROR: ' + (err && err.message));
@@ -228,7 +251,7 @@ module.exports = function() {
 
     // To fix ci fail about error format change in upstream v8 project
     // https://github.com/v8/v8/commit/c0fd89c3c089e888c4f4e8582e56db7066fa779b
-    // Node 16.9.0+ include this change via https://github.com/nodejs/node/pull/39947 
+    // Node 16.9.0+ include this change via https://github.com/nodejs/node/pull/39947
     function testTypeErrors(name, err, compiledLess, doReplacements, sourcemap, baseFolder) {
         const fileSuffix = semver.gte(process.version, 'v16.9.0') ? '-2.txt' : '.txt';
         fs.readFile(path.join(baseFolder, name) + fileSuffix, 'utf8', function (e, expectedErr) {
@@ -254,7 +277,7 @@ module.exports = function() {
     // https://github.com/less/less.js/issues/3112
     function testJSImport() {
         process.stdout.write('- Testing root function registry');
-        less.functions.functionRegistry.add('ext', function() { 
+        less.functions.functionRegistry.add('ext', function() {
             return new less.tree.Anonymous('file');
         });
         var expected = '@charset "utf-8";\n';
@@ -282,7 +305,7 @@ module.exports = function() {
             .replace(/\{pathhref\}/g, '')
             .replace(/\{404status\}/g, '')
             .replace(/\{nodepath\}/g, path.join(process.cwd(), 'node_modules', '/'))
-            .replace(/\{pathrel\}/g, path.join(path.relative(lessFolder, p), '/')) 
+            .replace(/\{pathrel\}/g, path.join(path.relative(lessFolder, p), '/'))
             .replace(/\{pathesc\}/g, pathesc)
             .replace(/\{pathimport\}/g, pathimport)
             .replace(/\{pathimportesc\}/g, pathimportesc)
@@ -327,7 +350,7 @@ module.exports = function() {
 
     function runTestSetInternal(baseFolder, opts, foldername, verifyFunction, nameModifier, doReplacements, getFilename) {
         foldername = foldername || '';
-        
+
         var originalOptions = opts || {};
 
         if (!doReplacements) {
@@ -497,10 +520,10 @@ module.exports = function() {
     }
 
     /**
-     * 
-     * @param {Object} options 
-     * @param {string} filePath 
-     * @param {Function} callback 
+     *
+     * @param {Object} options
+     * @param {string} filePath
+     * @param {Function} callback
      */
     function toCSS(options, filePath, callback) {
         options = options || {};
@@ -577,7 +600,7 @@ module.exports = function() {
                 }
                 ok(stylize('OK\n', 'green'));
             }
-        );      
+        );
     }
 
     return {
@@ -588,6 +611,7 @@ module.exports = function() {
         testTypeErrors: testTypeErrors,
         testSourcemap: testSourcemap,
         testSourcemapWithoutUrlAnnotation: testSourcemapWithoutUrlAnnotation,
+        testSourcemapWithVariableInSelector: testSourcemapWithVariableInSelector,
         testImports: testImports,
         testImportRedirect: testImportRedirect,
         testEmptySourcemap: testEmptySourcemap,

diff --git a/packages/less/test/sourcemaps-variable-selector/basic.json b/packages/less/test/sourcemaps-variable-selector/basic.json
@@ -0,0 +1 @@
+{"version":3,"sources":["testweb/sourcemaps-variable-selector/basic.less"],"names":[],"mappings":"AAEC;EACG,eAAA","file":"sourcemaps-variable-selector/basic.css"}
diff --git a/packages/test-data/less/sourcemaps-variable-selector/basic.less b/packages/test-data/less/sourcemaps-variable-selector/basic.less
@@ -0,0 +1,5 @@
+@import (reference) "./vars.less";
+
+.@{hello}-class {
+    font-size: @font-size;
+}
diff --git a/packages/test-data/less/sourcemaps-variable-selector/vars.less b/packages/test-data/less/sourcemaps-variable-selector/vars.less
@@ -0,0 +1,3 @@
+@foo: bar;
+@font-size: 12px;
+@hello: world;
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		{"version":3,"sources":["testweb/sourcemaps-variable-selector/basic.less"],"names":[],"mappings":"AAEC;EACG,eAAA","file":"sourcemaps-variable-selector/basic.css"}