Skip to content

Commit 05aca83

Browse files
danilsomsikovDevtools-frontend LUCI CQ
authored andcommitted
Migrate no-imperative-dom-api to TS
Bug: 397586315 Change-Id: I85023b65eee29f6254cdafb8b18cbe52bdc0568d Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6429853 Auto-Submit: Danil Somsikov <[email protected]> Reviewed-by: Nikolay Vitkov <[email protected]> Commit-Queue: Danil Somsikov <[email protected]>
1 parent 310d89e commit 05aca83

File tree

11 files changed

+233
-350
lines changed

11 files changed

+233
-350
lines changed

scripts/eslint_rules/lib/no-imperative-dom-api.js renamed to scripts/eslint_rules/lib/no-imperative-dom-api.ts

Lines changed: 72 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -3,62 +3,69 @@
33
// found in the LICENSE file.
44
/**
55
* @fileoverview Rule to identify and templatize manually constructed DOM.
6-
*
7-
* To check types, run
8-
* $ npx tsc --noEmit --allowJS --checkJS --downlevelIteration scripts/eslint_rules/lib/no-imperative-dom-api.js
96
*/
10-
'use strict';
117

12-
const adorner = require('./no-imperative-dom-api/adorner.js');
13-
const {isIdentifier, getEnclosingExpression} = require('./no-imperative-dom-api/ast.js');
14-
const {ClassMember} = require('./no-imperative-dom-api/class-member.js');
15-
const domApiDevtoolsExtensions = require('./no-imperative-dom-api/dom-api-devtools-extensions.js');
16-
const domApi = require('./no-imperative-dom-api/dom-api.js');
17-
const {DomFragment} = require('./no-imperative-dom-api/dom-fragment.js');
18-
const toolbar = require('./no-imperative-dom-api/toolbar.js');
19-
const widget = require('./no-imperative-dom-api/widget.js');
8+
import type {TSESTree} from '@typescript-eslint/utils';
209

21-
/** @typedef {import('estree').Node} Node */
22-
/** @typedef {import('eslint').Rule.Node} EsLintNode */
23-
/** @typedef {import('eslint').AST.SourceLocation} SourceLocation */
24-
/** @typedef {import('eslint').Scope.Variable} Variable */
25-
/** @typedef {import('eslint').Scope.Reference} Reference*/
10+
import {adorner} from './no-imperative-dom-api/adorner.ts';
11+
import {getEnclosingExpression, isIdentifier} from './no-imperative-dom-api/ast.ts';
12+
import {ClassMember} from './no-imperative-dom-api/class-member.ts';
13+
import {domApiDevtoolsExtensions} from './no-imperative-dom-api/dom-api-devtools-extensions.ts';
14+
import {domApi} from './no-imperative-dom-api/dom-api.ts';
15+
import {DomFragment} from './no-imperative-dom-api/dom-fragment.ts';
16+
import {toolbar} from './no-imperative-dom-api/toolbar.ts';
17+
import {widget} from './no-imperative-dom-api/widget.ts';
18+
import {createRule} from './tsUtils.ts';
19+
type CallExpression = TSESTree.CallExpression;
20+
type Identifier = TSESTree.Identifier;
21+
type MemberExpression = TSESTree.MemberExpression;
22+
type NewExpression = TSESTree.NewExpression;
23+
type Node = TSESTree.Node;
24+
type Range = TSESTree.Range;
2625

27-
/**
28-
* @type {import('eslint').Rule.RuleModule}
29-
*/
30-
module.exports = {
26+
type Subrule = Partial<{
27+
getEvent(event: Node): string | null,
28+
propertyAssignment(property: Identifier, propertyValue: Node, domFragment: DomFragment): boolean,
29+
methodCall(property: Identifier, firstArg: Node, secondArg: Node, domFragment: DomFragment, call: CallExpression):
30+
boolean,
31+
propertyMethodCall(property: Identifier, method: Node, firstArg: Node, domFragment: DomFragment): boolean,
32+
subpropertyAssignment(
33+
property: Identifier, subproperty: Identifier, subpropertyValue: Node, domFragment: DomFragment): boolean,
34+
// eslint-disable-next-line @typescript-eslint/naming-convention
35+
MemberExpression: (node: MemberExpression) => void,
36+
// eslint-disable-next-line @typescript-eslint/naming-convention
37+
NewExpression: (node: NewExpression) => void,
38+
}>;
39+
40+
export default createRule({
41+
name: 'no-imperative-dom-api',
3142
meta: {
3243
type: 'problem',
3344
docs: {
3445
description: 'Prefer template literals over imperative DOM API calls',
3546
category: 'Possible Errors',
3647
},
3748
messages: {
38-
preferTemplateLiterals:
39-
'Prefer template literals over imperative DOM API calls',
49+
preferTemplateLiterals: 'Prefer template literals over imperative DOM API calls',
4050
},
4151
fixable: 'code',
42-
schema: [], // no options
52+
schema: [], // no options
4353
},
44-
create : function(context) {
54+
defaultOptions: [],
55+
create: function(context) {
4556
const sourceCode = context.getSourceCode();
4657

47-
const subrules = [
58+
const subrules: Subrule[] = [
4859
adorner.create(context),
4960
domApi.create(context),
5061
domApiDevtoolsExtensions.create(context),
5162
toolbar.create(context),
5263
widget.create(context),
5364
];
5465

55-
/**
56-
* @param {Node} event
57-
* @return {string|null}
58-
*/
59-
function getEvent(event) {
66+
function getEvent(event: Node): string|null {
6067
for (const rule of subrules) {
61-
const result = rule.getEvent?.(event);
68+
const result = 'getEvent' in rule ? rule.getEvent?.(event) : null;
6269
if (result) {
6370
return result;
6471
}
@@ -69,35 +76,37 @@ module.exports = {
6976
return null;
7077
}
7178

72-
/**
73-
* @param {EsLintNode} reference
74-
* @param {DomFragment} domFragment
75-
* @return {boolean}
76-
*/
77-
function processReference(reference, domFragment) {
79+
function processReference(reference: Node, domFragment: DomFragment): boolean {
7880
const parent = reference.parent;
81+
if (!parent) {
82+
return false;
83+
}
7984
const isAccessed = parent.type === 'MemberExpression' && parent.object === reference;
8085
if (!isAccessed) {
8186
return false;
8287
}
8388
const property = parent.property;
89+
if (property.type !== 'Identifier') {
90+
return false;
91+
}
8492
const grandParent = parent.parent;
8593
const isPropertyAssignment = grandParent.type === 'AssignmentExpression' && grandParent.left === parent;
8694
const propertyValue = isPropertyAssignment ? grandParent.right : null;
8795
const isMethodCall = grandParent.type === 'CallExpression' && grandParent.callee === parent;
8896
const grandGrandParent = grandParent.parent;
8997
const isPropertyMethodCall = grandParent.type === 'MemberExpression' && grandParent.object === parent &&
90-
grandGrandParent.type === 'CallExpression' && grandGrandParent.callee === grandParent;
98+
grandGrandParent?.type === 'CallExpression' && grandGrandParent?.callee === grandParent &&
99+
grandParent.property.type === 'Identifier';
91100
const propertyMethodArgument = isPropertyMethodCall ? grandGrandParent.arguments[0] : null;
92101
const isSubpropertyAssignment = grandParent.type === 'MemberExpression' && grandParent.object === parent &&
93-
grandParent.property.type === 'Identifier' && grandGrandParent.type === 'AssignmentExpression' &&
94-
grandGrandParent.left === grandParent;
102+
grandParent.property.type === 'Identifier' && grandGrandParent?.type === 'AssignmentExpression' &&
103+
grandGrandParent?.left === grandParent;
95104
const subproperty =
96105
isSubpropertyAssignment && grandParent.property.type === 'Identifier' ? grandParent.property : null;
97106
const subpropertyValue = isSubpropertyAssignment ? grandGrandParent.right : null;
98107
for (const rule of subrules) {
99-
if (isPropertyAssignment) {
100-
if (rule.propertyAssignment?.(property, propertyValue, domFragment)) {
108+
if (isPropertyAssignment && propertyValue) {
109+
if ('propertyAssignment' in rule && rule.propertyAssignment?.(property, propertyValue, domFragment)) {
101110
return true;
102111
}
103112
} else if (isMethodCall) {
@@ -111,28 +120,26 @@ module.exports = {
111120
}
112121
return true;
113122
}
114-
if (rule.methodCall?.(property, firstArg, secondArg, domFragment, grandParent)) {
123+
if ('methodCall' in rule && rule.methodCall?.(property, firstArg, secondArg, domFragment, grandParent)) {
115124
return true;
116125
}
117-
} else if (isPropertyMethodCall) {
118-
if (rule.propertyMethodCall?.(property, grandParent.property, propertyMethodArgument, domFragment)) {
126+
} else if (isPropertyMethodCall && propertyMethodArgument) {
127+
if ('propertyMethodCall' in rule &&
128+
rule.propertyMethodCall?.(property, grandParent.property, propertyMethodArgument, domFragment)) {
119129
return true;
120130
}
121-
} else if (isSubpropertyAssignment) {
122-
if (rule.subpropertyAssignment?.(property, subproperty, subpropertyValue, domFragment)) {
131+
} else if (isSubpropertyAssignment && subproperty && subpropertyValue) {
132+
if ('subpropertyAssignment' in rule &&
133+
rule.subpropertyAssignment?.(property, subproperty, subpropertyValue, domFragment)) {
123134
return true;
124135
}
125136
}
126137
}
127138
return false;
128139
}
129140

130-
/**
131-
* @param {DomFragment} domFragment
132-
*/
133-
function getRangesToRemove(domFragment) {
134-
/** @type {[number, number][]} */
135-
const ranges = [];
141+
function getRangesToRemove(domFragment: DomFragment): Range[] {
142+
const ranges: Range[] = [];
136143
for (const reference of domFragment.references) {
137144
if (!reference.processed) {
138145
continue;
@@ -168,32 +175,28 @@ module.exports = {
168175
return ranges.filter(r => r[0] < r[1]);
169176
}
170177

171-
/**
172-
* @param {DomFragment} domFragment
173-
*/
174-
function maybeReportDomFragment(domFragment) {
175-
if (!domFragment.replacementLocation || domFragment.parent || !domFragment.tagName ||
178+
function maybeReportDomFragment(domFragment: DomFragment): void {
179+
const replacementLocation = domFragment.replacementLocation?.parent?.type === 'ExportNamedDeclaration' ?
180+
domFragment.replacementLocation.parent :
181+
domFragment.replacementLocation;
182+
if (!replacementLocation || domFragment.parent || !domFragment.tagName ||
176183
domFragment.references.every(r => !r.processed)) {
177184
return;
178185
}
179186
context.report({
180-
node: domFragment.replacementLocation,
187+
node: replacementLocation,
181188
messageId: 'preferTemplateLiterals',
182189
fix(fixer) {
183190
const template = 'html`' + domFragment.toTemplateLiteral(sourceCode).join('') + '`';
184-
let replacementLocation = domFragment.replacementLocation;
185191

186-
if (replacementLocation?.type === 'VariableDeclarator') {
192+
if (replacementLocation.type === 'VariableDeclarator' && replacementLocation.init) {
187193
domFragment.initializer = undefined;
188194
return [
189195
fixer.replaceText(replacementLocation.init, template),
190196
...getRangesToRemove(domFragment).map(range => fixer.removeRange(range)),
191197
];
192198
}
193199

194-
if (replacementLocation?.parent?.type === 'ExportNamedDeclaration') {
195-
replacementLocation = replacementLocation.parent;
196-
}
197200
const text = `
198201
export const DEFAULT_VIEW = (input, _output, target) => {
199202
render(${template},
@@ -210,20 +213,20 @@ export const DEFAULT_VIEW = (input, _output, target) => {
210213
}
211214

212215
return {
213-
MemberExpression(node) {
216+
MemberExpression(node: MemberExpression) {
214217
if (node.object.type === 'ThisExpression') {
215218
ClassMember.getOrCreate(node, sourceCode);
216219
}
217220
for (const rule of subrules) {
218221
if ('MemberExpression' in rule) {
219-
rule.MemberExpression(node);
222+
rule.MemberExpression?.(node);
220223
}
221224
}
222225
},
223226
NewExpression(node) {
224227
for (const rule of subrules) {
225228
if ('NewExpression' in rule) {
226-
rule.NewExpression(node);
229+
rule.NewExpression?.(node);
227230
}
228231
}
229232
},
@@ -254,4 +257,4 @@ export const DEFAULT_VIEW = (input, _output, target) => {
254257
}
255258
};
256259
}
257-
};
260+
});

scripts/eslint_rules/lib/no-imperative-dom-api/adorner.js renamed to scripts/eslint_rules/lib/no-imperative-dom-api/adorner.ts

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,23 +4,19 @@
44
/**
55
* @fileoverview A library to identify and templatize manually constructed Adorner.
66
*/
7-
'use strict';
87

9-
const {isIdentifier, isMemberExpression} = require('./ast.js');
10-
const {DomFragment} = require('./dom-fragment.js');
8+
import type {TSESTree} from '@typescript-eslint/utils';
119

12-
/** @typedef {import('eslint').Rule.Node} Node */
10+
import {isIdentifier, isMemberExpression} from './ast.ts';
11+
import {DomFragment} from './dom-fragment.ts';
12+
type Identifier = TSESTree.Identifier;
13+
type Node = TSESTree.Node;
1314

14-
module.exports = {
15+
export const adorner = {
1516
create(context) {
1617
const sourceCode = context.getSourceCode();
1718
return {
18-
/**
19-
* @param {Node} property
20-
* @param {Node} propertyValue
21-
* @param {DomFragment} domFragment
22-
*/
23-
propertyAssignment(property, propertyValue, domFragment) {
19+
propertyAssignment(property: Identifier, propertyValue: Node, domFragment: DomFragment) {
2420
if (domFragment.tagName === 'devtools-adorner' && isIdentifier(property, 'data') &&
2521
propertyValue.type === 'ObjectExpression') {
2622
for (const property of propertyValue.properties) {

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

Lines changed: 0 additions & 78 deletions
This file was deleted.

0 commit comments

Comments
 (0)