Skip to content

Commit

Permalink
Merge pull request #1992 from patricklx/patch-8
Browse files Browse the repository at this point in the history
fix references for tags with a dot & non standard html tags
  • Loading branch information
NullVoxPopuli authored Nov 9, 2023
2 parents 20e6c34 + b7ab046 commit 59ea472
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 16 deletions.
47 changes: 43 additions & 4 deletions lib/parsers/gjs-gts-parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const typescriptParser = require('@typescript-eslint/parser');
const TypescriptScope = require('@typescript-eslint/scope-manager');
const { Reference, Scope, Variable, Definition } = require('eslint-scope');
const { registerParsedFile } = require('../preprocessors/noop');
const htmlTags = require('html-tags');

/**
* finds the nearest node scope
Expand Down Expand Up @@ -260,6 +261,34 @@ function preprocessGlimmerTemplates(info, code) {
n.loc.start = codeLines.offsetToPosition(tpl.templateRange[0]);
n.loc.end = codeLines.offsetToPosition(tpl.templateRange[1]);
}
// split up element node into sub nodes to be able to reference tag name
// parts <Foo.Bar /> -> nodes for `Foo` and `Bar`
if (n.type === 'ElementNode') {
n.name = n.tag;
n.parts = [];
let start = n.range[0];
let codeSlice = code.slice(...n.range);
for (const part of n.tag.split('.')) {
const regex = new RegExp(`\\b${part}\\b`);
const match = codeSlice.match(regex);
const range = [start + match.index, 0];
range[1] = range[0] + part.length;
codeSlice = code.slice(range[1], n.range[1]);
start = range[1];
n.parts.push({
type: 'GlimmerElementNodePart',
name: part,
range,
parent: n,
loc: {
start: codeLines.offsetToPosition(range[0]),
end: codeLines.offsetToPosition(range[1]),
},
});
}
}
// block params do not have location information
// add our own nodes so we can reference them
if ('blockParams' in n) {
n.params = [];
}
Expand Down Expand Up @@ -353,6 +382,7 @@ function convertAst(result, preprocessedResult, visitorKeys) {
result.ast.tokens.splice(firstIdx, lastIdx - firstIdx + 1, ...ti.ast.tokens);
}

// eslint-disable-next-line complexity
traverse(visitorKeys, result.ast, (path) => {
const node = path.node;
if (
Expand Down Expand Up @@ -389,10 +419,19 @@ function convertAst(result, preprocessedResult, visitorKeys) {
}
}
if (node.type === 'GlimmerElementNode') {
node.name = node.tag;
const { scope, variable } = findVarInParentScopes(result.scopeManager, path, node.tag) || {};
if (scope && (variable || isUpperCase(node.tag[0]))) {
registerNodeInScope(node, scope, variable);
// always reference first part of tag name, this also has the advantage
// that errors regarding this tag will only mark the tag name instead of
// the whole tag + children
const n = node.parts[0];
const { scope, variable } = findVarInParentScopes(result.scopeManager, path, n.name) || {};
if (
scope &&
(variable ||
isUpperCase(n.name[0]) ||
node.name.includes('.') ||
!htmlTags.includes(node.name))
) {
registerNodeInScope(n, scope, variable);
}
}

Expand Down
5 changes: 3 additions & 2 deletions lib/rules/template-no-let-reference.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,10 @@ module.exports = {
},

GlimmerElementNode(node) {
// glimmer element is in its own scope, need tp get upper scope
// glimmer element is in its own scope, need to get upper scope
const scope = sourceCode.getScope(node.parent);
checkIfWritableReferences(node, scope);
// GlimmerElementNode is not referenced, instead use tag name parts[0]
checkIfWritableReferences(node.parts[0], scope);
},
};
},
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
"eslint-scope": "^7.2.2",
"eslint-utils": "^3.0.0",
"estraverse": "^5.3.0",
"html-tags": "^3.3.1",
"lodash.camelcase": "^4.3.0",
"lodash.kebabcase": "^4.1.1",
"requireindex": "^1.2.0",
Expand Down
57 changes: 48 additions & 9 deletions tests/lib/rules-preprocessor/gjs-gts-parser-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,13 @@ const valid = [
filename: 'my-component.gjs',
code: `
const Foo = <template>hi</template>;
const Bar = 'x';
<template>
<Foo />
<Foo as |Abc Xyz|>
<Abc.x />
{{Xyz.x}}
</Foo>
<Bar.x />
</template>
`,
},
Expand Down Expand Up @@ -253,6 +257,8 @@ const invalid = [
{{#let 'x' as |noop notUsed usedEl|}}
{{noop}}
<usedEl />
<undef.x />
<non-std-html-tag />
{{/let}}
</template>
`,
Expand All @@ -268,21 +274,54 @@ const invalid = [
ruleId: 'no-unused-vars',
severity: 2,
},
{
column: 10,
endColumn: 15,
endLine: 6,
line: 6,
message: "'undef' is not defined.",
messageId: 'undef',
nodeType: 'GlimmerElementNodePart',
ruleId: 'no-undef',
severity: 2,
},
{
column: 10,
endColumn: 26,
endLine: 7,
line: 7,
message: "'non-std-html-tag' is not defined.",
messageId: 'undef',
nodeType: 'GlimmerElementNodePart',
ruleId: 'no-undef',
severity: 2,
},
],
},
{
filename: 'my-component.gjs',
code: `
<template>
<Foo />
<Bar>
<div></div>
</Bar>
</template>
`,
errors: [
{
message: "'Foo' is not defined.",
line: 3,
column: 9,
endColumn: 16,
endLine: 3,
column: 10,
endColumn: 13,
},
{
message: "'Bar' is not defined.",
line: 4,
endLine: 4,
column: 10,
endColumn: 13,
},
],
},
Expand All @@ -297,8 +336,8 @@ const invalid = [
{
message: "'F_0_O' is not defined.",
line: 3,
column: 9,
endColumn: 18,
column: 10,
endColumn: 15,
},
],
},
Expand Down Expand Up @@ -677,13 +716,13 @@ describe('multiple tokens in same file', () => {
});

expect(resultErrors[1]).toStrictEqual({
column: 30,
endColumn: 37,
column: 31,
endColumn: 34,
endLine: 4,
line: 4,
message: "'Bad' is not defined.",
messageId: 'undef',
nodeType: 'GlimmerElementNode',
nodeType: 'GlimmerElementNodePart',
ruleId: 'no-undef',
severity: 2,
});
Expand Down
2 changes: 1 addition & 1 deletion tests/lib/rules/template-no-let-reference.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ ruleTester.run('template-no-let-reference', rule, {
</template>
`,
output: null,
errors: [{ type: 'GlimmerElementNode', message: rule.meta.messages['no-let'] }],
errors: [{ type: 'GlimmerElementNodePart', message: rule.meta.messages['no-let'] }],
},
],
});
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3471,6 +3471,11 @@ html-escaper@^2.0.0:
resolved "https://registry.yarnpkg.com/html-escaper/-/html-escaper-2.0.2.tgz#dfd60027da36a36dfcbe236262c00a5822681453"
integrity sha512-H2iMtd0I4Mt5eYiapRdIDjp+XzelXQ0tFE4JS7YFwFevXXMmOp9myNrUvCg0D6ws8iqkRPBfKHgbwig1SmlLfg==

html-tags@^3.3.1:
version "3.3.1"
resolved "https://registry.yarnpkg.com/html-tags/-/html-tags-3.3.1.tgz#a04026a18c882e4bba8a01a3d39cfe465d40b5ce"
integrity sha512-ztqyC3kLto0e9WbNp0aeP+M3kTt+nbaIveGmUxAtZa+8iFgKLUOD4YKM5j+f3QD89bra7UeumolZHKuOXnTmeQ==

http-cache-semantics@^4.1.0, http-cache-semantics@^4.1.1:
version "4.1.1"
resolved "https://registry.yarnpkg.com/http-cache-semantics/-/http-cache-semantics-4.1.1.tgz#abe02fcb2985460bf0323be664436ec3476a6d5a"
Expand Down

0 comments on commit 59ea472

Please sign in to comment.