Skip to content

Commit 1675d4d

Browse files
danilsomsikovDevtools-frontend LUCI CQ
authored andcommitted
An abstraction to track class members and distinguish the class they belong to
Bug: 400353541 Change-Id: I5f8c6577407299d2c91980b40e1cc93ca74ef043 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6403237 Reviewed-by: Philip Pfaffe <[email protected]> Commit-Queue: Danil Somsikov <[email protected]>
1 parent 32caf01 commit 1675d4d

File tree

5 files changed

+135
-16
lines changed

5 files changed

+135
-16
lines changed

scripts/eslint_rules/lib/no-imperative-dom-api.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
const adorner = require('./no-imperative-dom-api/adorner.js');
1313
const {isIdentifier, getEnclosingExpression} = require('./no-imperative-dom-api/ast.js');
14+
const {ClassMember} = require('./no-imperative-dom-api/class-member.js');
1415
const domApiDevtoolsExtensions = require('./no-imperative-dom-api/dom-api-devtools-extensions.js');
1516
const domApi = require('./no-imperative-dom-api/dom-api.js');
1617
const {DomFragment} = require('./no-imperative-dom-api/dom-fragment.js');
@@ -175,7 +176,7 @@ export const DEFAULT_VIEW = (input, _output, target) => {
175176
return {
176177
MemberExpression(node) {
177178
if (node.object.type === 'ThisExpression') {
178-
DomFragment.getOrCreate(node, sourceCode);
179+
ClassMember.getOrCreate(node, sourceCode);
179180
}
180181
for (const rule of subrules) {
181182
if ('MemberExpression' in rule) {

scripts/eslint_rules/lib/no-imperative-dom-api/ast.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ function getEnclosingExpression(estreeNode) {
4242
/** @param {Node} estreeNode */
4343
function getEnclosingProperty(estreeNode) {
4444
const node = /** @type {EsLintNode} */ (estreeNode);
45+
if (isMemberExpression(node, n => n.type === 'ThisExpression', n => n.type === 'Identifier')) {
46+
return node;
47+
}
4548
if (node.parent.type === 'AssignmentExpression' && node.parent.right === node &&
4649
isMemberExpression(node.parent.left, n => n.type === 'ThisExpression', n => n.type === 'Identifier')) {
4750
return node.parent.left;
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// Copyright 2025 The Chromium Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
/**
5+
* @fileoverview A library to associate class members with their parent class.
6+
*/
7+
'use strict';
8+
9+
const {getEnclosingClassDeclaration} = require('./ast.js');
10+
11+
/** @typedef {import('estree').Node} Node */
12+
/** @typedef {import('eslint').SourceCode} SourceCode */
13+
14+
/** @type {Map<Node, Map<string, ClassMember>>} */
15+
const classes = new Map();
16+
17+
class ClassMember {
18+
/** @type {Set<Node>} */
19+
references = new Set();
20+
21+
/** @type {Node} */
22+
classDeclaration;
23+
24+
/**
25+
* @param {Node} node
26+
* @param {SourceCode} sourceCode
27+
* @return {ClassMember}
28+
*/
29+
static getOrCreate(node, sourceCode) {
30+
const classDeclaration = getEnclosingClassDeclaration(node);
31+
if (!classDeclaration) {
32+
return null;
33+
}
34+
let classMembers = classes.get(classDeclaration);
35+
if (!classMembers) {
36+
classMembers = new Map();
37+
classes.set(classDeclaration, classMembers);
38+
}
39+
const memberName = sourceCode.getText(node);
40+
let classMember = classMembers.get(memberName);
41+
if (!classMember) {
42+
classMember = new ClassMember();
43+
classMember.classDeclaration = classDeclaration;
44+
classMembers.set(memberName, classMember);
45+
}
46+
classMember.references.add(node);
47+
return classMember;
48+
}
49+
}
50+
51+
exports.ClassMember = ClassMember;

scripts/eslint_rules/lib/no-imperative-dom-api/dom-fragment.js

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,15 @@
66
*/
77
'use strict';
88

9-
const {getEnclosingProperty, getEnclosingClassDeclaration} = require('./ast.js');
9+
const {getEnclosingProperty} = require('./ast.js');
10+
const {ClassMember} = require('./class-member.js');
1011

1112
/** @typedef {import('estree').Node} Node */
1213
/** @typedef {import('eslint').Rule.Node} EsLintNode */
1314
/** @typedef {import('eslint').Scope.Variable} Variable */
1415
/** @typedef {import('eslint').SourceCode} SourceCode */
1516

16-
/** @type {Map<string|Variable, DomFragment>} */
17+
/** @type {Map<EsLintNode|ClassMember|Variable, DomFragment>} */
1718
const domFragments = new Map();
1819

1920
class DomFragment {
@@ -37,26 +38,25 @@ class DomFragment {
3738
*/
3839
static getOrCreate(estreeNode, sourceCode) {
3940
const node = /** @type {EsLintNode} */ (estreeNode);
40-
const variable = getEnclosingVariable(node, sourceCode);
41-
const key = variable ?? sourceCode.getText(getEnclosingProperty(node) ?? node);
41+
const key = getKey(node, sourceCode);
4242

4343
let result = domFragments.get(key);
4444
if (!result) {
4545
result = new DomFragment();
4646
domFragments.set(key, result);
47-
if (variable) {
48-
result.references = variable.references.map(r => (/** @type {EsLintNode} */ (r.identifier)));
49-
result.references.push(/** @type {EsLintNode} */ (variable.identifiers[0]));
50-
} else {
47+
if ('parent' in key) {
48+
result.expression = sourceCode.getText(node);
49+
result.references.push(node);
50+
} else if (key instanceof ClassMember) {
51+
result.replacementLocation = /** @type {EsLintNode} */ (key.classDeclaration);
5152
result.expression = sourceCode.getText(node);
52-
const classDeclaration = getEnclosingClassDeclaration(node);
53-
if (classDeclaration) {
54-
result.replacementLocation = classDeclaration;
55-
}
53+
} else {
54+
result.references = key.references.map(r => (/** @type {EsLintNode} */ (r.identifier)));
55+
result.references.push(/** @type {EsLintNode} */ (key.identifiers[0]));
5656
}
5757
}
58-
if (!variable && !result.references.includes(node)) {
59-
result.references.push(node);
58+
if (key instanceof ClassMember) {
59+
result.references = /** @type {EsLintNode[]} */ ([...key.references]);
6060
}
6161
return result;
6262
}
@@ -183,4 +183,20 @@ function attributeValue(outputString) {
183183
return '"' + outputString + '"';
184184
}
185185

186+
/**
187+
* @param {EsLintNode} node
188+
* @param {SourceCode} sourceCode
189+
*/
190+
function getKey(node, sourceCode) {
191+
const variable = getEnclosingVariable(node, sourceCode);
192+
if (variable) {
193+
return variable;
194+
}
195+
const property = getEnclosingProperty(node);
196+
if (property) {
197+
return ClassMember.getOrCreate(property, sourceCode);
198+
}
199+
return node;
200+
}
201+
186202
exports.DomFragment = DomFragment;

scripts/eslint_rules/tests/no-imperative-dom-api.test.js

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ class SomeWidget extends UI.Widget.Widget {
8888
export const DEFAULT_VIEW = (input, _output, target) => {
8989
render(html\`
9090
<div>
91-
<div class="container some-class" @click=\${this.onClick.bind(this)}></div>
91+
<div class="some-class container" @click=\${this.onClick.bind(this)}></div>
9292
</div>\`,
9393
target, {host: input});
9494
};
@@ -315,5 +315,53 @@ class SomeWidget extends UI.Widget.Widget {
315315
}`,
316316
errors: [{messageId: 'preferTemplateLiterals'}],
317317
},
318+
{
319+
filename: 'front_end/ui/components/component/file.ts',
320+
code: `
321+
class Widget1 extends UI.Widget.Widget {
322+
constructor() {
323+
super();
324+
this.contentElement.createChild('div', 'widget1');
325+
}
326+
}
327+
328+
class Widget2 extends UI.Widget.Widget {
329+
constructor() {
330+
super();
331+
this.contentElement.createChild('div', 'widget2');
332+
}
333+
}`,
334+
output: `
335+
336+
export const DEFAULT_VIEW = (input, _output, target) => {
337+
render(html\`
338+
<div>
339+
<div class="widget1"></div>
340+
</div>\`,
341+
target, {host: input});
342+
};
343+
344+
class Widget1 extends UI.Widget.Widget {
345+
constructor() {
346+
super();
347+
}
348+
}
349+
350+
351+
export const DEFAULT_VIEW = (input, _output, target) => {
352+
render(html\`
353+
<div>
354+
<div class="widget2"></div>
355+
</div>\`,
356+
target, {host: input});
357+
};
358+
359+
class Widget2 extends UI.Widget.Widget {
360+
constructor() {
361+
super();
362+
}
363+
}`,
364+
errors: [{messageId: 'preferTemplateLiterals'}, {messageId: 'preferTemplateLiterals'}],
365+
},
318366
],
319367
});

0 commit comments

Comments
 (0)