diff --git a/lib/rules/template-no-implicit-this.js b/lib/rules/template-no-implicit-this.js index 299a265bef..00f08aab03 100644 --- a/lib/rules/template-no-implicit-this.js +++ b/lib/rules/template-no-implicit-this.js @@ -31,40 +31,65 @@ const BUILT_INS = new Set([ 'rootURL', ]); -// Control-flow built-ins whose params should not be flagged -const CONTROL_FLOW_HELPERS = new Set([ - 'if', - 'unless', - 'each', - 'let', - 'with', - 'each-in', - 'concat', - 'get', - 'array', - 'hash', - 'log', +// Node types that have a `path` property pointing to a callee PathExpression +const CALLEE_PARENT_TYPES = new Set([ + 'GlimmerMustacheStatement', + 'GlimmerSubExpression', + 'GlimmerBlockStatement', + 'GlimmerElementModifierStatement', ]); -function isMustacheCalleeWithArgs(node) { +// Callees are always valid for SubExpression/Block/Modifier; for Mustache, +// only when the mustache has args (bare {{foo}} is still ambiguous). +function isCalleePosition(node) { const parent = node.parent; - if (parent.path !== node) { + if (!parent || !CALLEE_PARENT_TYPES.has(parent.type) || parent.path !== node) { return false; } - if (parent.params && parent.params.length > 0) { + if (parent.type !== 'GlimmerMustacheStatement') { return true; } - return Boolean(parent.hash && parent.hash.pairs && parent.hash.pairs.length > 0); + const hasParams = parent.params && parent.params.length > 0; + const hasHash = parent.hash && parent.hash.pairs && parent.hash.pairs.length > 0; + return hasParams || hasHash; } -function isControlFlowParam(node) { - const callee = node.parent.path?.original; - return CONTROL_FLOW_HELPERS.has(callee) && node.parent.params?.includes(node); +// Returns true if the path root resolves to a JS binding (import, const, +// param, etc.). Walks scope.variables by name so it catches Glimmer built-in +// names (e.g. log, outlet) that don't surface in scope.references. +function isJsScopeVariable(node, sourceCode) { + if (!sourceCode || !node.original) { + return false; + } + const name = node.original.split('.')[0]; + try { + let scope = sourceCode.getScope(node); + while (scope) { + if (scope.variables.some((v) => v.name === name)) { + return true; + } + scope = scope.upper; + } + } catch { + // sourceCode.getScope may not be available in .hbs-only mode; ignore. + } + return false; } -function isBlockParamPath(node, path) { - const blockParams = node.parent.program?.blockParams || []; - return blockParams.includes(path.split('.')[0]); +// Walks ancestors collecting block params from GlimmerBlockStatement nodes. +function isLocalBlockParam(node, pathRoot) { + let current = node.parent; + while (current) { + // GlimmerBlockStatement nodes carry block params in program.blockParams + if (current.type === 'GlimmerBlockStatement') { + const blockParams = current.program?.blockParams || current.blockParams || []; + if (blockParams.includes(pathRoot)) { + return true; + } + } + current = current.parent; + } + return false; } /** @type {import('eslint').Rule.RuleModule} */ @@ -104,13 +129,14 @@ module.exports = { create(context) { const allowList = context.options[0]?.allow || []; + const sourceCode = context.sourceCode; return { GlimmerPathExpression(node) { const path = node.original; // Skip if path starts with @ (named arg) or this. (explicit) - if (path.startsWith('@') || path.startsWith('this.')) { + if (path.startsWith('@') || path.startsWith('this.') || path === 'this') { return; } @@ -124,38 +150,34 @@ module.exports = { return; } - // Skip single identifiers that are the callee of a helper-like MustacheStatement - if (node.parent && node.parent.type === 'GlimmerMustacheStatement') { - if (isMustacheCalleeWithArgs(node)) { - return; - } - if (isControlFlowParam(node)) { - return; - } + // Skip if it looks like a component (PascalCase) + const firstPart = path.split('.')[0]; + if (firstPart[0] === firstPart[0].toUpperCase()) { + return; } - // Skip paths that are part of block params - if (node.parent && node.parent.type === 'GlimmerBlockStatement') { - if (isBlockParamPath(node, path)) { - return; - } + // Skip callees of call-like expressions (SubExpression, BlockStatement, + // ElementModifierStatement always; MustacheStatement only with args) + if (isCalleePosition(node)) { + return; } - // Report ambiguous paths that should use this. or @ - if (!path.includes('.') || !path.startsWith('this.')) { - const firstPart = path.split('.')[0]; - - // Skip if it looks like a component (PascalCase) - if (firstPart[0] === firstPart[0].toUpperCase()) { - return; - } + // Skip paths whose root is a JS scope binding (import/const/param) — + // this is how GJS/GTS references external helpers, components, values. + if (isJsScopeVariable(node, sourceCode)) { + return; + } - context.report({ - node, - messageId: 'noImplicitThis', - data: { path }, - }); + // Skip paths whose root is an in-scope block param + if (isLocalBlockParam(node, firstPart)) { + return; } + + context.report({ + node, + messageId: 'noImplicitThis', + data: { path }, + }); }, }; }, diff --git a/tests/lib/rules/template-no-implicit-this.js b/tests/lib/rules/template-no-implicit-this.js index b3d083cb98..0871f76cdc 100644 --- a/tests/lib/rules/template-no-implicit-this.js +++ b/tests/lib/rules/template-no-implicit-this.js @@ -23,9 +23,28 @@ ruleTester.run('template-no-implicit-this', rule, { '', '', - // Helpers with params - '', - '', + // Named-argument control-flow helpers not flagged + '', + '', + + // SubExpression, modifier, block callees not flagged + '', + '', + '', + + // Bare {{this}} is not ambiguous + '', + + // Block params in nested scopes + '', + + // JS scope bindings (imports, const, let, params) are valid references in GJS/GTS + `const condition = false; + export default ;`, + `import helper from './my-helper'; + export default ;`, + `const items = [1, 2, 3]; + export default ;`, // Components (PascalCase) '', @@ -52,6 +71,12 @@ ruleTester.run('template-no-implicit-this', rule, { output: null, errors: [{ messageId: 'noImplicitThis' }], }, + // Control-flow helper args with no JS binding are still ambiguous + { + code: '', + output: null, + errors: [{ messageId: 'noImplicitThis' }], + }, { code: '', output: null, @@ -130,6 +155,7 @@ hbsRuleTester.run('template-no-implicit-this', rule, { '{{@book.author}}', // Explicit this + '{{this}}', '{{this.book}}', '{{this.book.author}}', @@ -140,6 +166,24 @@ hbsRuleTester.run('template-no-implicit-this', rule, { // Helpers invoked with positional arguments (callee is not flagged) '', + // SubExpression callees should not be flagged + '{{echo (my-helper @arg)}}', + '{{echo (some-util "value")}}', + + // ElementModifierStatement callees should not be flagged + '
', + '
', + + // BlockStatement callees should not be flagged + '{{#my-component}}{{/my-component}}', + '{{#some-layout title="Hi"}}content{{/some-layout}}', + + // Block params should be recognized in nested scopes + '{{#each @items as |item|}}{{item.name}}{{/each}}', + '{{#each @items as |item|}}{{item}}{{/each}}', + '{{#let @foo as |bar|}}{{bar.baz}}{{/let}}', + '{{#each @items as |item|}}{{#each item.children as |child|}}{{child.name}}{{/each}}{{/each}}', + // PascalCase components '', '',