Skip to content

Commit

Permalink
fix(no-this-assign-in-render): explicit left hand side traversal (#185)
Browse files Browse the repository at this point in the history
The esquery we were using was too wide in that it would match a `.left`
at _any_ level.

This meant the following code would match:

```ts
render() {
  x = this.foo || 123;
}
```

As a `LogicalExpression` has a left just like an `AssignmentExpression`.

Since esquery doesn't support relative direct child selection (i.e.
using `:has(> foo)`), we are now selecting the left side node and
reporting on the parent instead.
  • Loading branch information
43081j committed Oct 24, 2023
1 parent 2c2a456 commit 8f307c5
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 7 deletions.
14 changes: 7 additions & 7 deletions src/rules/no-this-assign-in-render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,18 +88,18 @@ const rule: Rule.RuleModule = {
}

/**
* Assignment expression entered
* Left side of an assignment expr found
*
* @param {ESTree.AssignmentExpression} node Node entered
* @param {Rule.Node} node Node entered
* @return {void}
*/
function assignmentFound(node: ESTree.AssignmentExpression): void {
function assignmentFound(node: Rule.Node): void {
if (!inRender) {
return;
}

context.report({
node,
node: node.parent,
messageId: 'noThis'
});
}
Expand All @@ -114,9 +114,9 @@ const rule: Rule.RuleModule = {
MethodDefinition: (node: ESTree.Node): void =>
methodEnter(node as ESTree.MethodDefinition),
'MethodDefinition:exit': methodExit,
'AssignmentExpression:has(.left ThisExpression)': (
node: ESTree.Node
): void => assignmentFound(node as ESTree.AssignmentExpression)
'AssignmentExpression > .left:has(ThisExpression)': (
node: Rule.Node
): void => assignmentFound(node)
};
}
};
Expand Down
6 changes: 6 additions & 0 deletions src/test/rules/no-this-assign-in-render_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ ruleTester.run('no-this-assign-in-render', rule, {
let x;
x = 5;
}
}`,
`class Foo extends LitElement {
render() {
let x;
x = this.foo || 123;
}
}`
],

Expand Down

0 comments on commit 8f307c5

Please sign in to comment.