Merge pull request #2143 from courajs/quotes-fixer

README.md

@@ -172,7 +172,7 @@ Each rule has emojis denoting:
 | [no-with](./docs/rule/no-with.md)                                                                         | ✅  |     |     |     |
 | [no-yield-only](./docs/rule/no-yield-only.md)                                                             | ✅  |     |     |     |
 | [no-yield-to-default](./docs/rule/no-yield-to-default.md)                                                 | ✅  |     |     |     |
-| [quotes](./docs/rule/quotes.md)                                                                           |     | 💅  |     |     |
+| [quotes](./docs/rule/quotes.md)                                                                           |     | 💅  |     | 🔧  |
 | [require-button-type](./docs/rule/require-button-type.md)                                                 | ✅  |     |     | 🔧  |
 | [require-context-role](./docs/rule/require-context-role.md)                                               | ✅  |     | ⌨️  |     |
 | [require-each-key](./docs/rule/require-each-key.md)                                                       |     |     |     |     |

docs/rule/quotes.md

@@ -2,6 +2,8 @@
 
 💅 The `extends: 'stylistic'` property in a configuration file enables this rule.
 
+🔧 The `--fix` option on the command line can automatically fix some of the problems reported by this rule.
+
 Enforce the consistent use of either double or single quotes.
 
 ## Examples

lib/rules/quotes.js

@@ -36,40 +36,78 @@ module.exports = class Quotes extends Rule {
 
   visitor() {
     let badChar;
+    let goodChar;
     let message;
     switch (this.config) {
       case 'double':
         badChar = "'";
+        goodChar = '"';
         message = 'you must use double quotes in templates';
         break;
       case 'single':
         badChar = '"';
+        goodChar = "'";
         message = 'you must use single quotes in templates';
         break;
     }
 
     return {
       AttrNode(node) {
         if (!node.isValueless && node.quoteType === badChar) {
-          return this.log({
-            message,
-            node,
-          });
+          if (attrValueHasChar(node.value, goodChar)) {
+            // TODO: Autofix blocked on: https://github.com/ember-template-lint/ember-template-recast/issues/698
+            return this.log({
+              message,
+              node,
+            });
+          }
+          if (this.mode === 'fix') {
+            node.quoteType = goodChar;
+          } else {
+            return this.log({
+              message,
+              node,
+              isFixable: true,
+            });
+          }
         }
       },
 
       StringLiteral(node, path) {
-        let source = this.sourceForNode(node);
         let errorSource = this.sourceForNode(path.parentNode);
 
-        if (source[0] === badChar) {
-          return this.log({
-            message,
-            node,
-            source: errorSource,
-          });
+        if (node.quoteType === badChar) {
+          if (node.value.includes(goodChar)) {
+            // TODO: Autofix blocked on: https://github.com/ember-template-lint/ember-template-recast/issues/698
+            return this.log({
+              message,
+              node,
+              source: errorSource,
+            });
+          }
+          if (this.mode === 'fix') {
+            node.quoteType = goodChar;
+          } else {
+            return this.log({
+              message,
+              node,
+              source: errorSource,
+              isFixable: true,
+            });
+          }
         }
       },
     };
   }
 };
+
+function attrValueHasChar(node, ch) {
+  if (node.type === 'TextNode') {
+    return node.chars.includes(ch);
+  } else if (node.type === 'ConcatStatement') {
+    return node.parts.some((n) => {
+      return n.type === 'TextNode' && n.chars.includes(ch);
+    });
+  }
+  return false;
+}

test/acceptance/public-api-test.js

@@ -286,31 +286,54 @@ describe('public api', function () {
     });
 
     it('returns whether the source has been fixed + an array of remaining issues with the provided template', async function () {
+      project.write({
+        app: {
+          templates: {
+            'application.hbs': '<div>FORBIDDEN</div>',
+          },
+        },
+      });
+      linter = new Linter({
+        console: mockConsole,
+        config: {
+          plugins: [require('../helpers/failure-plugin')],
+          rules: {
+            'fail-on-word': 'FORBIDDEN',
+          },
+        },
+      });
+
       let templatePath = project.path('app/templates/application.hbs');
       let templateContents = fs.readFileSync(templatePath, { encoding: 'utf8' });
-      let expected = [
-        {
-          column: 7,
-          line: 1,
-          endColumn: 18,
-          endLine: 1,
-          message: 'you must use double quotes in templates',
-          filePath: templatePath,
-          rule: 'quotes',
-          severity: 2,
-          source: "class='mb4'",
-        },
-      ];
 
       let result = await linter.verifyAndFix({
         source: templateContents,
         filePath: templatePath,
         moduleId: templatePath.slice(0, -4),
       });
 
-      expect(result.messages).toEqual(expected);
-      expect(result.output).toEqual(templateContents);
-      expect(result.isFixed).toEqual(false);
+      expect(result).toMatchInlineSnapshot(
+        { messages: [{ filePath: expect.any(String) }] },
+        `
+        Object {
+          "isFixed": false,
+          "messages": Array [
+            Object {
+              "column": 5,
+              "endColumn": 14,
+              "endLine": 1,
+              "filePath": Any<String>,
+              "line": 1,
+              "message": "The string \\"FORBIDDEN\\" is forbidden in templates",
+              "rule": "fail-on-word",
+              "severity": 2,
+              "source": "FORBIDDEN",
+            },
+          ],
+          "output": "<div>FORBIDDEN</div>",
+        }
+      `
+      );
     });
 
     it('ensures template parsing errors are only reported once (not once per-rule)', async function () {
@@ -1196,31 +1219,54 @@ describe('public api', function () {
     });
 
     it('[.html] returns whether the source has been fixed + an array of remaining issues with the provided template', async function () {
+      project.write({
+        app: {
+          templates: {
+            'application.html': '<div>FORBIDDEN</div>',
+          },
+        },
+      });
+      linter = new Linter({
+        console: mockConsole,
+        config: {
+          plugins: [require('../helpers/failure-plugin')],
+          rules: {
+            'fail-on-word': 'FORBIDDEN',
+          },
+        },
+      });
+
       let templatePath = project.path('app/templates/application.html');
       let templateContents = fs.readFileSync(templatePath, { encoding: 'utf8' });
-      let expected = [
-        {
-          column: 7,
-          line: 1,
-          endColumn: 18,
-          endLine: 1,
-          message: 'you must use double quotes in templates',
-          filePath: templatePath,
-          rule: 'quotes',
-          severity: 2,
-          source: "class='mb4'",
-        },
-      ];
 
       let result = await linter.verifyAndFix({
         source: templateContents,
         filePath: templatePath,
         moduleId: templatePath.slice(0, -4),
       });
 
-      expect(result.messages).toEqual(expected);
-      expect(result.output).toEqual(templateContents);
-      expect(result.isFixed).toEqual(false);
+      expect(result).toMatchInlineSnapshot(
+        { messages: [{ filePath: expect.any(String) }] },
+        `
+        Object {
+          "isFixed": false,
+          "messages": Array [
+            Object {
+              "column": 5,
+              "endColumn": 14,
+              "endLine": 1,
+              "filePath": Any<String>,
+              "line": 1,
+              "message": "The string \\"FORBIDDEN\\" is forbidden in templates",
+              "rule": "fail-on-word",
+              "severity": 2,
+              "source": "FORBIDDEN",
+            },
+          ],
+          "output": "<div>FORBIDDEN</div>",
+        }
+      `
+      );
     });
 
     it('[.html] ensures template parsing errors are only reported once (not once per-rule)', async function () {

test/helpers/failure-plugin.js

@@ -0,0 +1,25 @@
+const Rule = require('../..').Rule;
+
+// Easily controllable failure for acceptance tests
+class FailOnWord extends Rule {
+  visitor() {
+    return {
+      TextNode(node) {
+        const bad = this.config;
+        if (node.chars.includes(bad)) {
+          this.log({
+            message: `The string "${bad}" is forbidden in templates`,
+            node,
+          });
+        }
+      },
+    };
+  }
+}
+
+module.exports = {
+  name: 'test-helper-failure-plugin',
+  rules: {
+    'fail-on-word': FailOnWord,
+  },
+};