Skip to content

Commit 02606bd

Browse files
Lightning00BladeDevtools-frontend LUCI CQ
authored andcommitted
[eslint] Fix type assertions in custom Rules
Last batch, all other issue are on the `no-imperative-dom-api` Bug: 407085691 Change-Id: I25c70e66d3749978faed3e602f3524e656171f16 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6416090 Commit-Queue: Nikolay Vitkov <[email protected]> Reviewed-by: Danil Somsikov <[email protected]> Commit-Queue: Danil Somsikov <[email protected]> Auto-Submit: Nikolay Vitkov <[email protected]>
1 parent c698f45 commit 02606bd

8 files changed

+265
-152
lines changed

scripts/eslint_rules/lib/enforce-bound-render-for-schedule-render.js

Lines changed: 47 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,9 @@ function isPropertyDefinitionViaBindCallToThis(propertyDefinition) {
3636
return false;
3737
}
3838

39-
const isCalleeObjectThis = isMemberExpressionOnThis(propertyDefinition.value.callee);
39+
const isCalleeObjectThis = isMemberExpressionOnThis(
40+
propertyDefinition.value.callee,
41+
);
4042
// Whether the CallExpression is on a property of `this` (this.xxx.yyy.bind)
4143
if (!isCalleeObjectThis) {
4244
return false;
@@ -69,45 +71,74 @@ module.exports = {
6971
meta: {
7072
type: 'problem',
7173
docs: {
72-
description: 'Enforce render method to be bound while calling scheduleRender',
74+
description:
75+
'Enforce render method to be bound while calling scheduleRender',
7376
category: 'Possible Errors',
7477
},
7578
fixable: 'code',
76-
schema: [] // no options
79+
schema: [], // no options
7780
},
78-
create: function(context) {
81+
create: function (context) {
7982
return {
8083
CallExpression(node) {
8184
// Calls in the form of `ScheduledRender.scheduleRender`
82-
const isScheduleRenderCall = node.callee.type === 'MemberExpression' &&
83-
node.callee.object?.property?.name === 'ScheduledRender' && node.callee.property?.name === 'scheduleRender';
84-
if (!isScheduleRenderCall) {
85+
if (
86+
node.callee.type !== 'MemberExpression' ||
87+
node.callee.object.type !== 'MemberExpression' ||
88+
node.callee.object.property.type !== 'Identifier' ||
89+
node.callee.object.property?.name !== 'ScheduledRender' ||
90+
node.callee.property.type !== 'Identifier' ||
91+
node.callee.property.name !== 'scheduleRender'
92+
) {
8593
return;
8694
}
8795

8896
const callbackArgument = node.arguments[1];
8997
// Whether the second argument points to a property of `this`
9098
// like `ScheduledRender.scheduleRender(<any>, this.<any>)
91-
if (callbackArgument.type !== 'MemberExpression' || callbackArgument.object.type !== 'ThisExpression') {
99+
if (
100+
callbackArgument.type !== 'MemberExpression' ||
101+
callbackArgument.object.type !== 'ThisExpression'
102+
) {
92103
return;
93104
}
94105

95106
const containingClassForTheCall = goToClassDeclaration(node);
96107
// Only care about the calls in custom components
97-
if (!containingClassForTheCall.superClass || containingClassForTheCall.superClass.name !== 'HTMLElement') {
108+
if (
109+
!containingClassForTheCall.superClass ||
110+
containingClassForTheCall.superClass.name !== 'HTMLElement'
111+
) {
98112
return;
99113
}
100114

101115
const calledMethod = callbackArgument.property;
116+
if (
117+
calledMethod.type !== 'Identifier' &&
118+
calledMethod.type !== 'PrivateIdentifier'
119+
) {
120+
return;
121+
}
122+
102123
// Check whether the called method is bound (it should be 'PropertyDefinition')
103124
const propertyDefinition = containingClassForTheCall.body.body.find(
104-
bodyNode => bodyNode.type === 'PropertyDefinition' && bodyNode.key.name === calledMethod.name);
105-
if (!propertyDefinition ||
106-
(!isPropertyDefinitionViaArrowFunction(propertyDefinition) &&
107-
!isPropertyDefinitionViaBindCallToThis(propertyDefinition))) {
108-
context.report({node, message: 'Bind `render` method of `scheduleRender` to `this` in components'});
125+
bodyNode =>
126+
bodyNode.type === 'PropertyDefinition' &&
127+
'name' in bodyNode.key &&
128+
bodyNode.key.name === calledMethod.name,
129+
);
130+
if (
131+
!propertyDefinition ||
132+
(!isPropertyDefinitionViaArrowFunction(propertyDefinition) &&
133+
!isPropertyDefinitionViaBindCallToThis(propertyDefinition))
134+
) {
135+
context.report({
136+
node,
137+
message:
138+
'Bind `render` method of `scheduleRender` to `this` in components',
139+
});
109140
}
110-
}
141+
},
111142
};
112-
}
143+
},
113144
};

scripts/eslint_rules/lib/enforce-custom-element-definitions-location.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ module.exports = {
6565

6666
return {
6767
ClassDeclaration(node) {
68-
if (node.superClass?.name !== 'HTMLElement') {
68+
if (node.superClass?.type !== 'Identifier' || node.superClass?.name !== 'HTMLElement') {
6969
return;
7070
}
7171

scripts/eslint_rules/lib/enforce-custom-event-names.js

Lines changed: 86 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -20,83 +20,93 @@ module.exports = {
2020
},
2121
fixable: 'code',
2222
messages: {
23-
invalidEventName: 'Custom events must be named in all lower case with no punctuation.',
24-
invalidEventNameReference: 'When referencing a custom event name, it must be accessed as ClassName.eventName.'
23+
invalidEventName:
24+
'Custom events must be named in all lower case with no punctuation.',
25+
invalidEventNameReference:
26+
'When referencing a custom event name, it must be accessed as ClassName.eventName.',
2527
},
26-
schema: [] // no options
28+
schema: [], // no options
2729
},
28-
create: function(context) {
30+
create: function (context) {
2931
let foundLocalEventClassDeclaration = false;
3032
const classDeclarationsToLint = [];
3133

3234
function lintClassNode(node) {
33-
34-
const constructor =
35-
node.body.body.find(node => node.type === 'MethodDefinition' && node.kind === 'constructor');
36-
if (!constructor) {
37-
return;
38-
}
39-
const superCall = constructor.value.body.body.find(bodyNode => {
40-
return bodyNode.type === 'ExpressionStatement' && bodyNode.expression.type === 'CallExpression' &&
41-
bodyNode.expression.callee.type === 'Super';
42-
});
43-
if (!superCall) {
44-
return;
45-
}
46-
47-
const firstArgToSuper = superCall.expression.arguments[0];
48-
if (!firstArgToSuper) {
49-
// This is invalid, but TypeScript will catch this for us so no need to
50-
// error in ESLint land as well.
51-
return;
52-
}
53-
if (firstArgToSuper.type === 'Literal') {
54-
const firstArgLiteralValue = firstArgToSuper.value;
55-
if (!firstArgLiteralValue.match(VALID_EVENT_NAME_REGEX)) {
56-
context.report({node, messageId: 'invalidEventName'});
57-
}
58-
return;
59-
}
60-
61-
if (firstArgToSuper.type !== 'MemberExpression') {
62-
// This means it's a variable but not of the form ClassName.eventName, which we do not allow.
63-
context.report({node, messageId: 'invalidEventNameReference'});
64-
return;
65-
}
66-
67-
// the name of the custom event class we're looking at
68-
const eventClassName = node.id.name;
69-
const objectName = firstArgToSuper.object.name;
70-
const propertyName = firstArgToSuper.property.name;
71-
72-
if (objectName !== eventClassName || propertyName !== 'eventName') {
73-
context.report({node, messageId: 'invalidEventNameReference'});
74-
return;
75-
}
76-
77-
// If the reference is right, let's find the value of the static eventName property and make sure it is valid.
78-
const eventNameProperty = node.body.body.find(classBodyPart => {
79-
return classBodyPart.type === 'PropertyDefinition' && classBodyPart.key.name === 'eventName';
80-
});
81-
82-
// This should always exist because we checked for its existence
83-
// previously, no error loudly as this is a bug in the lint rule.
84-
if (!eventNameProperty) {
85-
throw new Error(`Could not find static eventName property for ${eventClassName}.`);
86-
}
87-
88-
// We don't let people use static eventName = SOME_VAR;
89-
if (eventNameProperty.value.type !== 'Literal') {
90-
context.report({node, messageId: 'invalidEventNameReference'});
91-
return;
92-
}
93-
94-
// Grab the value of static eventName and confirm it follows the
95-
// required conventions.
96-
const valueOfEventName = eventNameProperty.value.value;
97-
if (!valueOfEventName.match(VALID_EVENT_NAME_REGEX)) {
98-
context.report({node, messageId: 'invalidEventName'});
35+
const constructor = node.body.body.find(
36+
node => node.type === 'MethodDefinition' && node.kind === 'constructor',
37+
);
38+
if (!constructor) {
39+
return;
40+
}
41+
const superCall = constructor.value.body.body.find(bodyNode => {
42+
return (
43+
bodyNode.type === 'ExpressionStatement' &&
44+
bodyNode.expression.type === 'CallExpression' &&
45+
bodyNode.expression.callee.type === 'Super'
46+
);
47+
});
48+
if (!superCall) {
49+
return;
50+
}
51+
52+
const firstArgToSuper = superCall.expression.arguments[0];
53+
if (!firstArgToSuper) {
54+
// This is invalid, but TypeScript will catch this for us so no need to
55+
// error in ESLint land as well.
56+
return;
57+
}
58+
if (firstArgToSuper.type === 'Literal') {
59+
const firstArgLiteralValue = firstArgToSuper.value;
60+
if (!firstArgLiteralValue.match(VALID_EVENT_NAME_REGEX)) {
61+
context.report({ node, messageId: 'invalidEventName' });
9962
}
63+
return;
64+
}
65+
66+
if (firstArgToSuper.type !== 'MemberExpression') {
67+
// This means it's a variable but not of the form ClassName.eventName, which we do not allow.
68+
context.report({ node, messageId: 'invalidEventNameReference' });
69+
return;
70+
}
71+
72+
// the name of the custom event class we're looking at
73+
const eventClassName = node.id.name;
74+
const objectName = firstArgToSuper.object.name;
75+
const propertyName = firstArgToSuper.property.name;
76+
77+
if (objectName !== eventClassName || propertyName !== 'eventName') {
78+
context.report({ node, messageId: 'invalidEventNameReference' });
79+
return;
80+
}
81+
82+
// If the reference is right, let's find the value of the static eventName property and make sure it is valid.
83+
const eventNameProperty = node.body.body.find(classBodyPart => {
84+
return (
85+
classBodyPart.type === 'PropertyDefinition' &&
86+
classBodyPart.key.name === 'eventName'
87+
);
88+
});
89+
90+
// This should always exist because we checked for its existence
91+
// previously, no error loudly as this is a bug in the lint rule.
92+
if (!eventNameProperty) {
93+
throw new Error(
94+
`Could not find static eventName property for ${eventClassName}.`,
95+
);
96+
}
97+
98+
// We don't let people use static eventName = SOME_VAR;
99+
if (eventNameProperty.value.type !== 'Literal') {
100+
context.report({ node, messageId: 'invalidEventNameReference' });
101+
return;
102+
}
103+
104+
// Grab the value of static eventName and confirm it follows the
105+
// required conventions.
106+
const valueOfEventName = eventNameProperty.value.value;
107+
if (!valueOfEventName.match(VALID_EVENT_NAME_REGEX)) {
108+
context.report({ node, messageId: 'invalidEventName' });
109+
}
100110
}
101111

102112
return {
@@ -110,7 +120,11 @@ module.exports = {
110120
return;
111121
}
112122

113-
if (!node.superClass || node.superClass.name !== 'Event') {
123+
if (
124+
!node.superClass ||
125+
node.superClass.type !== 'Identifier' ||
126+
node.superClass.name !== 'Event'
127+
) {
114128
return;
115129
}
116130

@@ -126,5 +140,5 @@ module.exports = {
126140
});
127141
},
128142
};
129-
}
143+
},
130144
};

scripts/eslint_rules/lib/enforce-ui-strings-as-const.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
/**
66
* @fileoverview Rule to check ES import usage
7-
* @author ergunsh@
7+
* @author Ergün Erdoğmuş
88
*/
99
'use strict';
1010

@@ -44,7 +44,8 @@ module.exports = {
4444
const [declaration] = node.declarations;
4545
// We look for `startsWith` because we want to capture other variations as well
4646
// such as `UIStringsNotTranslate` from the AIAssistancePanel.
47-
const isIdentifierUIStrings = declaration.id.type === 'Identifier' && declaration.id.name.startsWith('UIStrings');
47+
const isIdentifierUIStrings =
48+
declaration.id.type === 'Identifier' && declaration.id.name.startsWith('UIStrings');
4849
const isAValidObjectExpression = declaration.init?.type === 'ObjectExpression';
4950
if (!isIdentifierUIStrings || !isAValidObjectExpression) {
5051
return;

scripts/eslint_rules/lib/html-tagged-template.js

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,36 +25,42 @@ module.exports = {
2525
lastImport = node;
2626
},
2727
VariableDeclarator(node) {
28-
if (
29-
(sourceCode.getScope ? sourceCode.getScope(node) : context.getScope())
30-
.type !== 'module'
31-
) {
28+
const scope = sourceCode.getScope ? sourceCode.getScope(node) : context.getScope();
29+
if (scope.type !== 'module') {
3230
return;
3331
}
34-
if (node.id.name === 'html') {
32+
if (node.id.type === 'Identifier' && node.id.name === 'html') {
3533
shorthandDefined = true;
3634
}
35+
36+
if (node.id.type !== 'ObjectPattern') {
37+
return;
38+
}
39+
3740
for (const property of node.id.properties || []) {
38-
if (property.key.name === 'html') {
41+
if (property.type === 'Property' && property.key.type === 'Identifier' && property.key.name === 'html') {
3942
shorthandDefined = true;
4043
}
4144
}
4245
},
4346
TaggedTemplateExpression(node) {
4447
const tag = node.tag;
45-
if (
46-
tag.type === 'MemberExpression' &&
47-
tag.object.name === 'Lit' &&
48-
tag.property.name === 'html'
49-
) {
48+
if (tag.type === 'MemberExpression' && tag.object.type === 'Identifier' && tag.object.name === 'Lit' &&
49+
tag.property.type === 'Identifier' && tag.property.name === 'html') {
5050
context.report({
5151
node,
52-
message:
53-
'Use unqualified html tagged template for compatibility with lit-analyzer',
52+
message: 'Use unqualified html tagged template for compatibility with lit-analyzer',
5453
fix(fixer) {
55-
const result = [
56-
fixer.removeRange([tag.object.range[0], tag.property.range[0]]),
57-
];
54+
const result = [];
55+
if (tag.object?.range?.[0] && tag.property?.range?.[0]) {
56+
result.push(
57+
fixer.removeRange([
58+
tag.object.range[0],
59+
tag.property.range[0],
60+
]),
61+
);
62+
}
63+
5864
if (lastImport && !shorthandDefined) {
5965
result.push(
6066
fixer.insertTextAfter(lastImport, '\n\nconst {html} = Lit;'),

0 commit comments

Comments
 (0)