Repository: rwjblue/ember-template-lint
Pull request #262 (2017-09-06 23:55:43)

Adding rule to check for template-length
Hi there, I'd love to be able to add a linting rule (ideally as a plugin) to have the linter fail when a template is over a certain size (assuming it should be broken up into pieces) or BELOW a certain size (assuming it should be inlined into a component).

~For some reason, disabling tests through instruction comments does
not seem to work and unit tests failing. If anyone has any idea what might be wrong here, I'd certainly appreciate the assistance. I wonder if we are not getting the configuration change because I'm using the `Program` hook to look for template-length?~

rwjblue: Personal opinion time:

I totally agree that we _should_ strive to use logical separation of concerns which in many cases will naturally lead to this rule being satisfied, but I am not personally in favor of _using_ this rule in projects that I work on. In my experience the idea that a given file is "bad" because it is long and to satisfy linting you have to split things up leads to very hard to reason about codebases. In my experience, splitting up a single logical unit into multiple files simply because it is "long" roughly means that to find where some HTML snippet is you have to solve a murder mystery. 😝 

---

That personal thought aside, I am totally in favor of _having_ this rule in the codebase.  Thank you for working on it!
psbanka: @rwjblue – you're welcome, and thanks for all the help 😃 ! I think I caught all the items you mentioned in review and I added documentation (including adding a note as to why you might *not* want to use this check, drawing heavily from your "personal opinion." 

I hope this looks good ow; glad to fix anything else you see.

Patch:
diff --git a/docs/rule/template-length.md b/docs/rule/template-length.md
new file mode 100644
index 0000000..542c355
--- /dev/null
+++ b/docs/rule/template-length.md
@@ -0,0 +1,24 @@
+#### template-length
+
+Some ember shops try to put some guard-rails on the length of templates for the following reasons:
+
+- Templates that are extremely long are often a sign that code should be
+  factored into several different components. Be aware that sometimes breaking
+  a template up just because it's long in order to satisfy linting-errors can
+  potentially make the code harder to reason about: splitting up a single
+  logical unit into multiple files simply because it is "long" roughly means that
+  to find where some HTML snippet is you have to solve a murder mystery. So while
+  this check is helpful to help with sanity-checking, `{{! template-disable template-length }}` 
+  might be the right answer if a long template does in fact represent a logical unit.
+
+- Templates that are extremely short might be better off inlined into the
+  component itself rather than existing as a separate `.hbs` file.
+
+This rule is configured with a boolean, or a string value:
+
+* boolean -- `true` for enabled (defaults of max: 200 / min: 5) / `false` for disabling this rule
+* object --
+  * max {number} - the longest a template should be without failing a test (assuming the
+    right thing to do would be to split the template up into pieces).
+  * min {number} - the shortest a template should be without failing a test (assuming the
+    right thing to do would be to inline the template.
diff --git a/docs/rules.md b/docs/rules.md
index 4682e30..00c4b56 100644
--- a/docs/rules.md
+++ b/docs/rules.md
@@ -16,6 +16,7 @@
 * [self-closing-void-elements](rule/self-closing-void-elements.md)
 * [simple-unless](rule/simple-unless.md)
 * [style-concatenation](rule/style-concatenation.md)
+* [template-length](rule/template-length.md)
 * [triple-curlies](rule/triple-curlies.md)
 * [unused-block-params](rule/unused-block-params.md)
 
diff --git a/lib/rules/index.js b/lib/rules/index.js
index 716ceff..6ef9db6 100644
--- a/lib/rules/index.js
+++ b/lib/rules/index.js
@@ -22,4 +22,5 @@ module.exports = {
   'no-duplicate-attributes': require('./lint-duplicate-attributes'),
   'linebreak-style': require('./lint-linebreak-style'),
   'attribute-indentation': require('./lint-attribute-indentation'),
+  'template-length': require('./lint-template-length')
 };
diff --git a/lib/rules/lint-template-length.js b/lib/rules/lint-template-length.js
new file mode 100644
index 0000000..86167f9
--- /dev/null
+++ b/lib/rules/lint-template-length.js
@@ -0,0 +1,96 @@
+'use strict';
+
+/*
+ Enforce template size constraints
+
+ The following values are valid configuration:
+
+   * boolean -- `true` for enabled (defaults of max: 200 / min: 5) / `false` for disabling this rule
+   * object --
+     * max {number} - the longest a template should be without failing a test (assuming the
+       right thing to do would be to split the template up into pieces).
+     * min {number} - the shortest a template should be without failing a test (assuming the
+       right thing to do would be to inline the template).
+ */
+
+const Rule = require('./base');
+
+const DEFAULT_MAX_LENGTH = 200;
+const DEFAULT_MIN_LENGTH = 5;
+
+const DEFAULT_CONFIG = {
+  max: DEFAULT_MAX_LENGTH,
+  min: DEFAULT_MIN_LENGTH
+};
+
+function isValidConfigObjectFormat(config) {
+  for (let key in config) {
+    let value = config[key];
+    let valueIsInteger = Number.isInteger(value);
+
+    if (key === 'min' && !valueIsInteger) {
+      return false;
+    } else if (key === 'max' && !valueIsInteger) {
+      return false;
+    }
+  }
+  return true;
+}
+
+module.exports = class LengthValidation extends Rule {
+  parseConfig(config) {
+    let configType = typeof config;
+
+    let errorMessage = 'The template-length rule accepts one of the following values.\n ' +
+      '  * boolean - `true` to enable / `false` to disable\n' +
+      '  * object -- An object with the following keys:' +
+      '    * `min` -- Minimum length of a template ' +
+      '    * `max` -- Maximum length of a template. ' +
+      '\nYou specified `' + JSON.stringify(config) + '`';
+
+    switch (configType) {
+    case 'boolean':
+      // if `true` use `DEFAULT_CONFIG`
+      return config ? DEFAULT_CONFIG : {};
+    case 'object':
+      if (isValidConfigObjectFormat(config)) {
+        return config;
+      } else {
+        throw new Error(errorMessage);
+      }
+    case 'undefined':
+      return {};
+    default:
+      throw new Error(errorMessage);
+    }
+  }
+
+  visitor() {
+    let seenOuterProgram = false;
+    return {
+      Program: {
+
+        // implementation goes here in exit(): in the exit handler, the rule will not
+        // be called if it has been disabled by any inline comments within the file.
+
+        exit(node) {
+          if (seenOuterProgram === true) { return; }
+          if (this.config.max && this.source.length > this.config.max) {
+            this.log({
+              message: `Template length of ${this.source.length} exceeds ${this.config.max}`,
+              line: node.loc.start.line,
+              column: node.loc.start.column
+            });
+          } else if (this.config.min && this.source.length < this.config.min) {
+            this.log({
+              message: `Template length of ${this.source.length} is smaller than ${this.config.min}`,
+              line: node.loc.start.line,
+              column: node.loc.start.column
+            });
+          }
+          seenOuterProgram = true;
+        }
+      },
+    };
+  }
+};


Test patch:
diff --git a/test/unit/rules/lint-template-length-test.js b/test/unit/rules/lint-template-length-test.js
new file mode 100644
index 0000000..6a5a4eb
--- /dev/null
+++ b/test/unit/rules/lint-template-length-test.js
@@ -0,0 +1,52 @@
+'use strict';
+
+const generateRuleTests = require('../../helpers/rule-test-harness');
+
+generateRuleTests({
+  name: 'template-length',
+
+  config: true,
+
+  good: [
+    'testing this\nand\nthis\nand\this',
+    {
+      config: {max: 10},
+      template: 'testing\nthis\n'
+    },
+    {
+      config: {min: 1},
+      template: 'testing\nthis\nand\this\n'
+    },
+    {
+      config: {min: 1, max: 5},
+      template: 'testing\nthis\nandthis\n'
+    },
+  ],
+
+  bad: [
+    {
+      config: {min: 10},
+      template: 'testing\ntoo-short template\n',
+
+      result: {
+        rule: 'template-length',
+        moduleId: 'layout.hbs',
+        message: 'Template length of 3 is smaller than 10',
+        line: 1,
+        column: 0,
+      }
+    },
+    {
+      config: {max: 3},
+      template: 'test\nthis\nand\nthis\n',
+
+      result: {
+        rule: 'template-length',
+        moduleId: 'layout.hbs',
+        message: 'Template length of 5 exceeds 3',
+        line: 1,
+        column: 0,
+      }
+    }
+  ]
+});