From 2d10fe025905b09daa4a7f2f63a53f5d483ec0d6 Mon Sep 17 00:00:00 2001
From: patrick <pa.trickjlp@gmail.com>
Date: Tue, 22 Aug 2023 11:01:05 +0200
Subject: [PATCH 1/2] add template-no-let-reference rule

---
 .eslintignore                                |  1 +
 README.md                                    |  1 +
 docs/rules/template-no-let-reference.md      | 58 ++++++++++++++++++
 lib/parsers/gjs-gts-parser.js                | 28 ++++-----
 lib/rules/template-no-let-reference.js       | 44 ++++++++++++++
 tests/lib/rules/template-no-let-reference.js | 64 ++++++++++++++++++++
 6 files changed, 182 insertions(+), 14 deletions(-)
 create mode 100644 docs/rules/template-no-let-reference.md
 create mode 100644 lib/rules/template-no-let-reference.js
 create mode 100644 tests/lib/rules/template-no-let-reference.js

diff --git a/.eslintignore b/.eslintignore
index 071bd5627c..d67e8df837 100644
--- a/.eslintignore
+++ b/.eslintignore
@@ -6,3 +6,4 @@ lib/recommended-rules-gts.js
 
 # Contains <template> in js markdown
 docs/rules/no-empty-glimmer-component-classes.md
+docs/rules/template-no-let-reference.md
diff --git a/README.md b/README.md
index 89313f5df0..b3580e3099 100644
--- a/README.md
+++ b/README.md
@@ -240,6 +240,7 @@ rules in templates can be disabled with eslint directives with mustache or html
 | [no-ember-super-in-es-classes](docs/rules/no-ember-super-in-es-classes.md)                 | disallow use of `this._super` in ES class methods                                                              | ✅  | 🔧 |    |
 | [no-empty-glimmer-component-classes](docs/rules/no-empty-glimmer-component-classes.md)     | disallow empty backing classes for Glimmer components                                                          | ✅  |    |    |
 | [no-tracked-properties-from-args](docs/rules/no-tracked-properties-from-args.md)           | disallow creating @tracked properties from this.args                                                           | ✅  |    |    |
+| [template-no-let-reference](docs/rules/template-no-let-reference.md)                       | disallow referencing let variables in \<template\>                                                             |    |    |    |
 
 ### jQuery
 
diff --git a/docs/rules/template-no-let-reference.md b/docs/rules/template-no-let-reference.md
new file mode 100644
index 0000000000..1f25a1176a
--- /dev/null
+++ b/docs/rules/template-no-let-reference.md
@@ -0,0 +1,58 @@
+# ember/template-no-let-reference
+
+<!-- end auto-generated rule header -->
+
+Disallows referencing let/var variables in templates.
+
+```js
+// app/components/foo.gjs
+let foo = 1;
+
+function increment() {
+foo++;
+}
+
+export default <template>{{foo}}</template>;
+```
+
+This does not "work" – it doesn't error, but it will essentially capture and compile in the value of the foo variable at some arbitrary point and never update again. Even if the component is torn down and a new instance is created/rendered, it will probably still hold on to the old value when the template was initially compiled.
+
+So, generally speaking, one should avoid referencing let variables from within &lt;template&gt; and instead prefer to use const bindings.
+
+## Rule Detail
+
+Use `const` variables instead of `let`.
+
+## Examples
+
+Examples of **incorrect** code for this rule:
+
+```js
+let x = 1;
+<template>
+  {{x}}
+</template>
+```
+
+```js
+let Comp = x; // SomeComponent
+<template>
+  <Comp />
+</template>
+```
+
+Examples of **correct** code for this rule:
+
+```js
+const x = 1;
+<template>
+  {{x}}
+</template>
+```
+
+```js
+const Comp = x; // SomeComponent
+<template>
+  <Comp />
+</template>
+```
diff --git a/lib/parsers/gjs-gts-parser.js b/lib/parsers/gjs-gts-parser.js
index ab449c5cb1..1d437cc1d3 100644
--- a/lib/parsers/gjs-gts-parser.js
+++ b/lib/parsers/gjs-gts-parser.js
@@ -377,20 +377,6 @@ function convertAst(result, preprocessedResult, visitorKeys) {
       Object.assign(node, ast);
     }
 
-    if ('blockParams' in node) {
-      const upperScope = findParentScope(result.scopeManager, path);
-      const scope = result.isTypescript
-        ? new TypescriptScope.BlockScope(result.scopeManager, upperScope, node)
-        : new Scope(result.scopeManager, 'block', upperScope, node);
-      for (const [i, b] of node.params.entries()) {
-        const v = new Variable(b.name, scope);
-        v.identifiers.push(b);
-        v.defs.push(new Definition('Parameter', b, node, node, i, 'Block Param'));
-        scope.variables.push(v);
-        scope.set.set(b.name, v);
-      }
-    }
-
     if (node.type === 'GlimmerPathExpression' && node.head.type === 'VarHead') {
       const name = node.head.name;
       if (glimmer.isKeyword(name)) {
@@ -409,6 +395,20 @@ function convertAst(result, preprocessedResult, visitorKeys) {
         registerNodeInScope(node, scope, variable);
       }
     }
+
+    if ('blockParams' in node) {
+      const upperScope = findParentScope(result.scopeManager, path);
+      const scope = result.isTypescript
+        ? new TypescriptScope.BlockScope(result.scopeManager, upperScope, node)
+        : new Scope(result.scopeManager, 'block', upperScope, node);
+      for (const [i, b] of node.params.entries()) {
+        const v = new Variable(b.name, scope);
+        v.identifiers.push(b);
+        v.defs.push(new Definition('Parameter', b, node, node, i, 'Block Param'));
+        scope.variables.push(v);
+        scope.set.set(b.name, v);
+      }
+    }
     return null;
   });
 
diff --git a/lib/rules/template-no-let-reference.js b/lib/rules/template-no-let-reference.js
new file mode 100644
index 0000000000..8f32c59ff7
--- /dev/null
+++ b/lib/rules/template-no-let-reference.js
@@ -0,0 +1,44 @@
+const ERROR_MESSAGE =
+  'update-able variables are not supported in templates, reference a const variable';
+
+/** @type {import('eslint').Rule.RuleModule} */
+module.exports = {
+  ERROR_MESSAGE,
+  meta: {
+    type: 'suggestion',
+    docs: {
+      description: 'disallow referencing let variables in \\<template\\>',
+      category: 'Ember Octane',
+      recommended: false,
+      url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/template-no-let-reference.md',
+    },
+    fixable: null,
+    schema: [],
+  },
+
+  create: (context) => {
+    const sourceCode = context.sourceCode;
+
+    function checkIfWritableReferences(node, scope) {
+      const ref = scope.references.find((v) => v.identifier === node);
+      if (!ref) {
+        return;
+      }
+      if (ref.resolved?.identifiers.some((i) => ['let', 'var'].includes(i.parent.parent.kind))) {
+        context.report({ node, message: ERROR_MESSAGE });
+      }
+    }
+
+    return {
+      GlimmerPathExpression(node) {
+        checkIfWritableReferences(node.head, sourceCode.getScope(node));
+      },
+
+      GlimmerElementNode(node) {
+        // glimmer element is in its own scope, need tp get upper scope
+        const scope = sourceCode.getScope(node.parent);
+        checkIfWritableReferences(node, scope);
+      },
+    };
+  },
+};
diff --git a/tests/lib/rules/template-no-let-reference.js b/tests/lib/rules/template-no-let-reference.js
new file mode 100644
index 0000000000..d0eb2312af
--- /dev/null
+++ b/tests/lib/rules/template-no-let-reference.js
@@ -0,0 +1,64 @@
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const rule = require('../../../lib/rules/template-no-let-reference');
+const RuleTester = require('eslint').RuleTester;
+
+const { ERROR_MESSAGE } = rule;
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+const ruleTester = new RuleTester({
+  parser: require.resolve('../../../lib/parsers/gjs-gts-parser.js'),
+  parserOptions: { ecmaVersion: 2020, sourceType: 'module' },
+});
+ruleTester.run('template-no-let-reference', rule, {
+  valid: [
+    `
+      const a = '';
+      <template>
+      {{a}}
+      </template>
+    `,
+    `
+      const a = '';
+      <template>
+      <a></a>
+      </template>
+    `,
+    // make sure rule does not error out on missing references
+    `
+      const a = '';
+      <template>
+      {{ab}}
+      <ab></ab>
+      </template>
+    `,
+  ],
+
+  invalid: [
+    {
+      code: `
+      let a = '';
+      <template>
+      {{a}}
+      </template>
+      `,
+      output: null,
+      errors: [{ type: 'VarHead', message: ERROR_MESSAGE }],
+    },
+    {
+      code: `
+      var a = '';
+      <template>
+      <a></a>
+      </template>
+      `,
+      output: null,
+      errors: [{ type: 'GlimmerElementNode', message: ERROR_MESSAGE }],
+    },
+  ],
+});

From 18a5ea34e9a383e6fdc1cc429c15f6ae426a5f9a Mon Sep 17 00:00:00 2001
From: patrick <pa.trickjlp@gmail.com>
Date: Thu, 2 Nov 2023 16:33:50 +0100
Subject: [PATCH 2/2] use message id

---
 lib/rules/template-no-let-reference.js       | 9 ++++-----
 tests/lib/rules/template-no-let-reference.js | 6 ++----
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/lib/rules/template-no-let-reference.js b/lib/rules/template-no-let-reference.js
index 8f32c59ff7..9cdea62d82 100644
--- a/lib/rules/template-no-let-reference.js
+++ b/lib/rules/template-no-let-reference.js
@@ -1,9 +1,5 @@
-const ERROR_MESSAGE =
-  'update-able variables are not supported in templates, reference a const variable';
-
 /** @type {import('eslint').Rule.RuleModule} */
 module.exports = {
-  ERROR_MESSAGE,
   meta: {
     type: 'suggestion',
     docs: {
@@ -14,6 +10,9 @@ module.exports = {
     },
     fixable: null,
     schema: [],
+    messages: {
+      'no-let': 'update-able variables are not supported in templates, reference a const variable',
+    },
   },
 
   create: (context) => {
@@ -25,7 +24,7 @@ module.exports = {
         return;
       }
       if (ref.resolved?.identifiers.some((i) => ['let', 'var'].includes(i.parent.parent.kind))) {
-        context.report({ node, message: ERROR_MESSAGE });
+        context.report({ node, messageId: 'no-let' });
       }
     }
 
diff --git a/tests/lib/rules/template-no-let-reference.js b/tests/lib/rules/template-no-let-reference.js
index d0eb2312af..d26c81c917 100644
--- a/tests/lib/rules/template-no-let-reference.js
+++ b/tests/lib/rules/template-no-let-reference.js
@@ -5,8 +5,6 @@
 const rule = require('../../../lib/rules/template-no-let-reference');
 const RuleTester = require('eslint').RuleTester;
 
-const { ERROR_MESSAGE } = rule;
-
 //------------------------------------------------------------------------------
 // Tests
 //------------------------------------------------------------------------------
@@ -48,7 +46,7 @@ ruleTester.run('template-no-let-reference', rule, {
       </template>
       `,
       output: null,
-      errors: [{ type: 'VarHead', message: ERROR_MESSAGE }],
+      errors: [{ type: 'VarHead', message: rule.meta.messages['no-let'] }],
     },
     {
       code: `
@@ -58,7 +56,7 @@ ruleTester.run('template-no-let-reference', rule, {
       </template>
       `,
       output: null,
-      errors: [{ type: 'GlimmerElementNode', message: ERROR_MESSAGE }],
+      errors: [{ type: 'GlimmerElementNode', message: rule.meta.messages['no-let'] }],
     },
   ],
 });