Fix incorrect violation with embedded templates in `no-trailing-space…

…s` rule (#2833) * Add `columnOffset` to template infos for use in embedded templates * Fix incorrect `no-trailing-spaces` in embedded templates
ember-template-lint · Mar 15, 2023 · 027c7dd · 027c7dd
1 parent d5c258d
commit 027c7dd
Show file tree

Hide file tree

Showing 6 changed files with 112 additions and 5 deletions.
diff --git a/lib/extract-templates.js b/lib/extract-templates.js
@@ -2,7 +2,7 @@ import { parseTemplates } from 'ember-template-imports/lib/parse-templates.js';
 import * as util from 'ember-template-imports/src/util.js';
 
 export const SUPPORTED_EXTENSIONS = ['.js', '.ts', '.gjs', '.gts'];
-const LOCATION_START = Object.freeze({ line: 0, column: 0, start: 0, end: 0 });
+const LOCATION_START = Object.freeze({ line: 0, column: 0, start: 0, end: 0, columnOffset: 0 });
 /**
  * Processes results and corrects for template location offsets.
  *
@@ -90,12 +90,18 @@ export function extractTemplates(moduleSource, relativePath) {
  *
  * @returns {TemplateInfo}
  */
-function makeTemplateInfo({ line, column, start, end }, template, templateInfo, isEmbedded) {
+function makeTemplateInfo(
+  { line, column, start, end, columnOffset },
+  template,
+  templateInfo,
+  isEmbedded
+) {
   return {
     line,
     start,
     end,
     column,
+    columnOffset,
     template,
     isEmbedded,
     isStrictMode: !isEmbedded || isStrictMode(templateInfo),
@@ -122,11 +128,13 @@ export function isSupportedScriptFileExtension(filePath = '') {
 
 export function coordinatesOf(text, offset, start, end) {
   const contentBeforeTemplateStart = text.slice(0, offset).split('\n');
+  const lineBeforeTemplateStart = contentBeforeTemplateStart[contentBeforeTemplateStart.length - 1];
   return {
     line: contentBeforeTemplateStart.length,
-    column: contentBeforeTemplateStart[contentBeforeTemplateStart.length - 1].length,
+    column: lineBeforeTemplateStart.length,
     start,
     end,
+    columnOffset: lineBeforeTemplateStart.length - lineBeforeTemplateStart.trimStart().length,
   };
 }
 

diff --git a/lib/linter.js b/lib/linter.js
@@ -240,6 +240,7 @@ export default class Linter {
         let rule = this._buildRule(ruleName, {
           shouldFix: true,
           filePath: options.filePath,
+          columnOffset: templateInfo.columnOffset,
           rawSource: templateInfo.template,
           fileConfig,
         });
@@ -367,6 +368,7 @@ export default class Linter {
         fileConfig,
         filePath: options.filePath,
         configResolver: options.configResolver,
+        columnOffset: templateInfo.columnOffset,
         rawSource: templateInfo.template,
         isStrictMode: templateInfo.isStrictMode,
       });

diff --git a/lib/rules/_base.js b/lib/rules/_base.js
@@ -66,6 +66,8 @@ export default class {
     this[MODULE_NAME] = options.moduleName;
     this._rawSource = options.rawSource;
 
+    this.columnOffset = options.columnOffset;
+
     // split into a source array (allow windows and posix line endings)
     this.source = options.rawSource.match(reLines);
   }

diff --git a/lib/rules/no-trailing-spaces.js b/lib/rules/no-trailing-spaces.js
@@ -9,10 +9,13 @@ export default class NoTrailingSpaces extends Rule {
 
         exit(node) {
           let source = this.sourceForNode(node);
+          let lines = source.split('\n');
 
-          for (const [i, line] of source.split('\n').entries()) {
+          for (const [i, line] of lines.entries()) {
             let column = line.length - 1;
-            if (line[column] === ' ') {
+            let isLastLine = i === lines.length - 1;
+
+            if (line[column] === ' ' && (!isLastLine || column > this.columnOffset)) {
               this.log({
                 message: 'line cannot end with space',
                 node,

diff --git a/test/unit/extract-templates-test.js b/test/unit/extract-templates-test.js
@@ -25,6 +25,7 @@ describe('extractTemplates', function () {
         [
           {
             "column": 0,
+            "columnOffset": 0,
             "end": 0,
             "isEmbedded": undefined,
             "isStrictMode": true,
@@ -41,6 +42,7 @@ describe('extractTemplates', function () {
         [
           {
             "column": 60,
+            "columnOffset": 6,
             "end": 238,
             "isEmbedded": true,
             "isStrictMode": false,
@@ -71,6 +73,7 @@ describe('extractTemplates', function () {
         [
           {
             "column": 60,
+            "columnOffset": 6,
             "end": 242,
             "isEmbedded": true,
             "isStrictMode": true,
@@ -109,6 +112,7 @@ describe('extractTemplates', function () {
         [
           {
             "column": 63,
+            "columnOffset": 6,
             "end": 255,
             "isEmbedded": true,
             "isStrictMode": true,
@@ -139,6 +143,7 @@ describe('extractTemplates', function () {
         [
           {
             "column": 39,
+            "columnOffset": 0,
             "end": 58,
             "isEmbedded": true,
             "isStrictMode": true,
@@ -174,6 +179,7 @@ describe('extractTemplates', function () {
         [
           {
             "column": 0,
+            "columnOffset": 0,
             "end": 0,
             "isEmbedded": undefined,
             "isStrictMode": true,
@@ -190,6 +196,7 @@ describe('extractTemplates', function () {
         [
           {
             "column": 39,
+            "columnOffset": 0,
             "end": 58,
             "isEmbedded": true,
             "isStrictMode": true,
@@ -237,6 +244,7 @@ describe('calculate template coordinates', function () {
     expect(coordinatesOf(typescript, 228)).toEqual({
       line: 8,
       column: 12,
+      columnOffset: 2,
     });
   });
 });

diff --git a/test/unit/rules/no-trailing-spaces-test.js b/test/unit/rules/no-trailing-spaces-test.js
@@ -11,6 +11,22 @@ generateRuleTests({
     'test\n' + '\n',
     // test the re-entering of yielded content
     '{{#my-component}}\n' + '  test\n' + '{{/my-component}}',
+    {
+      template: [
+        "import { hbs } from 'ember-cli-htmlbars';",
+        '',
+        "test('it renders', async (assert) => {",
+        '  await render(hbs`',
+        '    <div class="parent">',
+        '      <div class="child"></div>',
+        '    </div>',
+        '  `);',
+        ');',
+      ].join('\n'),
+      meta: {
+        filePath: 'layout.js',
+      },
+    },
   ],
 
   bad: [
@@ -100,5 +116,73 @@ generateRuleTests({
         `);
       },
     },
+    {
+      template: [
+        "import { hbs } from 'ember-cli-htmlbars';",
+        '',
+        "test('it renders', async (assert) => {",
+        '  await render(hbs`  ',
+        '    <div class="parent">',
+        '      <div class="child"></div>',
+        '    </div>',
+        '  `);',
+        ');',
+      ].join('\n'),
+      meta: {
+        filePath: 'layout.js',
+      },
+
+      verifyResults(results) {
+        expect(results).toMatchInlineSnapshot(`
+          [
+            {
+              "column": 20,
+              "endColumn": 21,
+              "endLine": 8,
+              "filePath": "layout.js",
+              "line": 4,
+              "message": "line cannot end with space",
+              "rule": "no-trailing-spaces",
+              "severity": 2,
+              "source": "  ",
+            },
+          ]
+        `);
+      },
+    },
+    {
+      template: [
+        "import { hbs } from 'ember-cli-htmlbars';",
+        '',
+        "test('it renders', async (assert) => {",
+        '  await render(hbs`',
+        '    <div></div>',
+        '  ',
+        '    <div></div>',
+        '  `);',
+        ');',
+      ].join('\n'),
+      meta: {
+        filePath: 'layout.js',
+      },
+
+      verifyResults(results) {
+        expect(results).toMatchInlineSnapshot(`
+          [
+            {
+              "column": 1,
+              "endColumn": 2,
+              "endLine": 8,
+              "filePath": "layout.js",
+              "line": 6,
+              "message": "line cannot end with space",
+              "rule": "no-trailing-spaces",
+              "severity": 2,
+              "source": "  ",
+            },
+          ]
+        `);
+      },
+    },
   ],
 });