Fix autofix replacements & rework test pipeline (#14)

* Track dummy replacements & revert them at the end

* Also use hash for JS preprocessor

* Replace "index" with "id"

* Mark variable as unused

* Get rid of wrong offsetmap lookup

* Update ESLint VSCode settings

* Replace cumbersome unit tests by integration tests

* Add another fixture: multiple properties on object

* Strip unit tests to most essential stuff

* Outsource run test method

* Add first HTML test

* Improve HTML rules & add one more HTML test

* Add test to test set

* Replace unnecessary map by array

Author: Splines

.vscode/settings.json

@@ -2,17 +2,22 @@
     //////////////////////////////////////
     // JS (ESLint)
     //////////////////////////////////////
-    "eslint.format.enable": true,
-    "eslint.experimental.useFlatConfig": true,
     "[javascript]": {
         "editor.formatOnSave": false, // to avoid formatting twice (ESLint + VSCode)
         "editor.defaultFormatter": "dbaeumer.vscode-eslint"
     },
     "editor.codeActionsOnSave": {
         "source.fixAll.eslint": "explicit"
     },
-    // this disables VSCode built-int formatter (instead we want to use ESLint)
+    "eslint.format.enable": true,
+    // this disables VSCode built-in formatter (instead we want to use ESLint)
     "javascript.validate.enable": false,
+    "eslint.options": {
+        "overrideConfigFile": "./eslint.config.js"
+    },
+    "eslint.validate": [
+        "javascript"
+    ],
     //////////////////////////////////////
     // Git
     //////////////////////////////////////

lib/autofix.js

@@ -1,9 +1,10 @@
 const { coordinatesToIndex, indexToColumn } = require("./file_coordinates.js");
 const { calculateOffset } = require("./offset_calculation.js");
 
-function transformFix(message, originalText, lintableText, offsetMap) {
+function transformFix(message, originalText, lintableText, offsetMap, dummyReplacementsMap) {
   const newRange = calculateNewRange(message, originalText, lintableText, offsetMap);
-  const newFixText = transformFixText(message, newRange, originalText);
+  const newFixText = transformFixText(message, newRange, originalText,
+    offsetMap, dummyReplacementsMap);
 
   return {
     range: newRange,
@@ -35,19 +36,9 @@ function calculateNewRange(message, originalText, lintableText, offsetMap) {
   });
 }
 
-function transformFixText(message, newRange, originalText) {
+function transformFixText(message, newRange, originalText, offsetMap, dummyReplacementsMap) {
   const fixText = message.fix.text;
-
-  switch (message.ruleId) {
-    case "@stylistic/quotes": {
-      const quoteChar = fixText[0];
-      const originalTextWithoutQuoteChars = originalText
-        .slice(newRange[0] + 1, newRange[1] - 1);
-      return `${quoteChar}${originalTextWithoutQuoteChars}${quoteChar}`;
-    }
-    default:
-      return fixText;
-  }
+  return dummyReplacementsMap.apply(fixText);
 }
 
 module.exports = { transformFix };

lib/cache.js

@@ -4,8 +4,8 @@ class Cache {
     this.cache = new Map();
   }
 
-  add(filename, originalText, lintableText, offsetMap) {
-    this.cache.set(filename, { originalText, lintableText, offsetMap });
+  add(filename, originalText, lintableText, offsetMap, dummyReplacementsMap) {
+    this.cache.set(filename, { originalText, lintableText, offsetMap, dummyReplacementsMap });
   }
 
   get(filename) {

lib/dummy_replacements_map.js

@@ -0,0 +1,30 @@
+/**
+ * Array that tracks the replacements that have been made in the processed text,
+ * so that we can reverse them later.
+ */
+class DummyReplacementsMap {
+  constructor() {
+    this.replacements = [];
+  }
+
+  addMapping(originalText, dummyText) {
+    this.replacements.push([originalText, dummyText]);
+  }
+
+  /**
+   * Tries to apply all stored replacements to the given text.
+   * @param {string} text - The text to apply the replacements to.
+   * @returns {string} - The text with the replacements applied.
+   */
+  apply(text) {
+    let lintableText = text;
+    for (const [originalText, dummyText] of this.replacements) {
+      lintableText = lintableText.replace(dummyText, originalText);
+    }
+    return lintableText;
+  }
+}
+
+module.exports = {
+  DummyReplacementsMap,
+};

lib/postprocess.js

@@ -13,10 +13,10 @@ const { transformFix } = require("./autofix.js");
  * @returns {Message[]} A flattened array of messages with mapped locations.
  */
 function postprocess(messages, filename) {
-  const { originalText, lintableText, offsetMap } = cache.get(filename);
+  const { originalText, lintableText, offsetMap, dummyReplacementsMap } = cache.get(filename);
 
   const transformedMessages = messages[0].map((message) => {
-    return adjustMessage(message, originalText, lintableText, offsetMap);
+    return adjustMessage(message, originalText, lintableText, offsetMap, dummyReplacementsMap);
   });
   cache.delete(filename);
 
@@ -28,7 +28,7 @@ function postprocess(messages, filename) {
  * @param {Message} message message form ESLint
  * @returns {Message} same message, but adjusted to the correct location
  */
-function adjustMessage(message, originalText, lintableText, offsetMap) {
+function adjustMessage(message, originalText, lintableText, offsetMap, dummyReplacementsMap) {
   if (!Number.isInteger(message.line)) {
     return {
       ...message,
@@ -62,11 +62,10 @@ function adjustMessage(message, originalText, lintableText, offsetMap) {
   // Autofixes
   const adjustedFix = {};
   if (message.fix) {
-    adjustedFix.fix = transformFix(message, originalText, lintableText, offsetMap);
+    adjustedFix.fix = transformFix(message, originalText, lintableText,
+      offsetMap, dummyReplacementsMap);
   }
 
-  // TODO: Implement suggestion ranges
-
   return { ...message, ...newCoordinates, ...adjustedFix };
 }
 

lib/preprocess.js

@@ -7,8 +7,10 @@ const { indexToColumn } = require("./file_coordinates.js");
 
 // how annoying is that kind of import in JS ?!
 var OffsetMap = require("./offset_map.js").OffsetMap;
+var DummyReplacementsMap = require("./dummy_replacements_map.js").DummyReplacementsMap;
 
 const ERB_REGEX = /<%[\s\S]*?%>/g;
+const HASH = "566513c5d83ac26e15414f2754"; // to avoid collisions with user code
 
 /**
  * Transforms the given text into lintable text. We do this by stripping out
@@ -17,8 +19,10 @@ const ERB_REGEX = /<%[\s\S]*?%>/g;
  * location of messages in the postprocess step later.
  * @param {string} text text of the file
  * @param {string} filename filename of the file
- * @param {string} dummyString dummy string to replace ERB tags with
- *                             (this is language-specific)
+ * @param {(id) => string} dummyString dummy string to replace ERB tags with
+ *                                     (this is language-specific). Should be
+ *                                     a function that takes an id and returns
+ *                                     a UNIQUE dummy string.
  * @returns {Array<{ filename: string, text: string }>} source code blocks to lint
  */
 function preprocess(text, filename, dummyString) {
@@ -28,7 +32,9 @@ function preprocess(text, filename, dummyString) {
   let numAddLines = 0;
   let numDiffChars = 0;
   const offsetMap = new OffsetMap();
+  const dummyReplacementsMap = new DummyReplacementsMap();
 
+  let matchedId = 0;
   while ((match = ERB_REGEX.exec(text)) !== null) {
     // Match information
     const startIndex = match.index;
@@ -41,23 +47,28 @@ function preprocess(text, filename, dummyString) {
     numAddLines += matchLines.length - 1;
 
     // Columns
+    const dummy = dummyString(matchedId);
     const coordStartIndex = indexToColumn(text, startIndex);
-    const endColumnAfter = coordStartIndex.column + dummyString.length;
+    const endColumnAfter = coordStartIndex.column + dummy.length;
     const coordEndIndex = indexToColumn(text, endIndex);
     const endColumnBefore = coordEndIndex.column;
     const numAddColumns = endColumnBefore - endColumnAfter;
+    replaceTextWithDummy(lintableTextArr, startIndex, matchLength - 1, dummy);
 
-    replaceTextWithDummy(lintableTextArr, startIndex, matchLength - 1, dummyString);
+    const textWithErbSyntax = text.slice(startIndex, endIndex);
+    dummyReplacementsMap.addMapping(textWithErbSyntax, dummy);
 
     // Store in map
     const lineAfter = coordEndIndex.line - numAddLines;
-    numDiffChars += dummyString.length - matchLength;
+    numDiffChars += dummy.length - matchLength;
     const endIndexAfter = endIndex + numDiffChars;
     offsetMap.addMapping(endIndexAfter, lineAfter, numAddLines, numAddColumns);
+
+    matchedId += 1;
   }
 
   const lintableText = lintableTextArr.join("");
-  cache.add(filename, text, lintableText, offsetMap);
+  cache.add(filename, text, lintableText, offsetMap, dummyReplacementsMap);
   return [lintableText];
 }
 
@@ -69,20 +80,24 @@ function preprocess(text, filename, dummyString) {
  * For the length of the match, subsequent characters are replaced with empty
  * strings in the array.
  */
-function replaceTextWithDummy(lintableTextArr, startIndex, length, dummyString) {
-  lintableTextArr[startIndex] = dummyString;
+function replaceTextWithDummy(lintableTextArr, startIndex, length, dummy) {
+  lintableTextArr[startIndex] = dummy;
   const replaceArgs = Array(length).join(".").split(".");
   // -> results in ['', '', '', '', ...]
   lintableTextArr.splice(startIndex + 1, length, ...replaceArgs);
 }
 
 function preprocessJs(text, filename) {
-  const dummyString = "/* eslint-disable */{}/* eslint-enable */";
+  function dummyString(id) {
+    return `/* eslint-disable */{}/* ${HASH} ${id} *//* eslint-enable */`;
+  }
   return preprocess(text, filename, dummyString);
 }
 
 function preprocessHtml(text, filename) {
-  const dummyString = "<!-- -->";
+  function dummyString(id) {
+    return `<!-- ${HASH} ${id} -->`;
+  }
   return preprocess(text, filename, dummyString);
 }
 

package.json

@@ -31,9 +31,11 @@
     "lib"
   ],
   "devDependencies": {
+    "@html-eslint/eslint-plugin": "^0.31.1",
+    "@html-eslint/parser": "^0.31.0",
     "@stylistic/eslint-plugin": "^1.3.3",
     "chai": "^4.3.10",
-    "eslint": "^9.0.0-alpha.0",
+    "eslint": "^9.17.0",
     "globals": "^13.24.0",
     "mocha": "^10.2.0"
   },

tests/eslint.test.config.js

@@ -0,0 +1,82 @@
+const js = require("@eslint/js");
+const stylistic = require("@stylistic/eslint-plugin");
+const globals = require("globals");
+const html = require("@html-eslint/eslint-plugin");
+
+const plugin = require("../lib/index.js");
+
+const customizedStylistic = stylistic.configs.customize({
+  "indent": 2,
+  "jsx": false,
+  "quote-props": "always",
+  "semi": "always",
+  "brace-style": "1tbs",
+});
+
+module.exports = [
+  js.configs.recommended,
+  {
+    processor: plugin.processors["processorJs"],
+    files: ["**/*.js", "**/*.js.erb"],
+  },
+  {
+    plugins: {
+      "@stylistic": stylistic,
+    },
+    rules: {
+      ...customizedStylistic.rules,
+      "no-unused-vars": ["warn", { argsIgnorePattern: "^_" }],
+      "@stylistic/quotes": ["error", "double", { avoidEscape: true }],
+    },
+    languageOptions: {
+      ecmaVersion: 2024,
+      sourceType: "module",
+      globals: {
+        ...globals.node,
+        ...globals.mocha,
+      },
+    },
+    ignores: ["**/*.html**"],
+  },
+  {
+    // HTML linting (aside from erb_lint)
+    files: ["**/*.html", "**/*.html.erb"],
+    processor: plugin.processors["processorHtml"],
+    ...html.configs["flat/recommended"],
+    plugins: {
+      "@html-eslint": html,
+      "@stylistic": stylistic,
+    },
+    rules: {
+      "@stylistic/eol-last": ["error", "always"],
+      "@stylistic/no-trailing-spaces": "error",
+      "@stylistic/no-multiple-empty-lines": ["error", { max: 1, maxEOF: 0 }],
+      ...html.configs["flat/recommended"].rules,
+      // 🎈 Best Practices
+      "@html-eslint/no-extra-spacing-text": "error",
+      "@html-eslint/no-script-style-type": "error",
+      "@html-eslint/no-target-blank": "error",
+      // 🎈 Accessibility
+      "@html-eslint/no-abstract-roles": "error",
+      "@html-eslint/no-accesskey-attrs": "error",
+      "@html-eslint/no-aria-hidden-body": "error",
+      "@html-eslint/no-non-scalable-viewport": "error",
+      "@html-eslint/no-positive-tabindex": "error",
+      "@html-eslint/no-skip-heading-levels": "error",
+      // 🎈 Styles
+      "@html-eslint/attrs-newline": ["error", {
+        closeStyle: "newline",
+        ifAttrsMoreThan: 5,
+      }],
+      "@html-eslint/id-naming-convention": ["error", "kebab-case"],
+      "@html-eslint/indent": ["error", 2],
+      "@html-eslint/sort-attrs": "error",
+      "@html-eslint/no-extra-spacing-attrs": ["error", {
+        enforceBeforeSelfClose: true,
+        disallowMissing: true,
+        disallowTabs: true,
+        disallowInAssignment: true,
+      }],
+    },
+  },
+];

tests/fixtures/common.expected.js

@@ -0,0 +1,12 @@
+if (<%= @success %>) {
+  console.log("Success 🎉");
+  $("#yeah").html("<%= j render partial: 'yeah',
+                          locals: { cool: @cool } %>");
+}
+else {
+  console.log("Sad noises");
+}
+
+<% if you_feel_lucky %>
+console.log("You are lucky 🍀");
+<% end %>