Skip to content

Commit b00393c

Browse files
Lightning00BladeDevtools-frontend LUCI CQ
authored andcommitted
[eslint] Fix type checking in custom EsLint rules
Either fix or suppress the errors for now Bug: 407085691 Change-Id: I45c5bd4c3644571ddaebcba45914c209ebfa46f5 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6415909 Auto-Submit: Nikolay Vitkov <[email protected]> Commit-Queue: Nikolay Vitkov <[email protected]> Reviewed-by: Danil Somsikov <[email protected]> Commit-Queue: Danil Somsikov <[email protected]>
1 parent 935df52 commit b00393c

File tree

7 files changed

+110
-52
lines changed

7 files changed

+110
-52
lines changed

scripts/eslint_rules/lib/canvas-context-tracking.js

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,15 @@ module.exports = {
1818
messages: {
1919
saveNotRestored:
2020
'Found a block that has more context.save() calls than context.restore() calls',
21-
uselessRestore: 'Found a context.restore() call with no context.save() prior to it',
21+
uselessRestore:
22+
'Found a context.restore() call with no context.save() prior to it',
2223
},
2324
schema: [], // no options
2425
},
2526
create: function (context) {
2627
// To track canvas calls across scopes we keep a stack which we push nodes on with every new scope that we find.
2728
// When we then leave a scope, we can check all the calls in that scope and see if they align or not.
28-
/** @type {Array<ASTNode>} */
29+
/** @type {Array<import('estree').Node>} */
2930
let stack = [];
3031

3132
// The key is a node's range as a string. The value is a stack of
@@ -48,8 +49,12 @@ module.exports = {
4849
*/
4950
function exitScope() {
5051
const lastScope = stack.pop();
52+
if (!lastScope) {
53+
return;
54+
}
55+
5156
const stackForCurrentScope = scopeToCanvasCalls.get(
52-
nodeToKeyForMap(lastScope)
57+
nodeToKeyForMap(lastScope),
5358
);
5459

5560
// We have no issues to report if:
@@ -60,10 +65,10 @@ module.exports = {
6065
}
6166

6267
// If we got here it means the stack for the scope has items in, which means that it is unbalanced.
63-
context.report({
64-
node: lastScope,
65-
messageId: 'saveNotRestored',
66-
});
68+
context.report({
69+
node: lastScope,
70+
messageId: 'saveNotRestored',
71+
});
6772
}
6873

6974
/**
@@ -72,32 +77,34 @@ module.exports = {
7277
**/
7378
function trackContextCall(methodName) {
7479
const currentScopeNode = stack.at(-1);
75-
const stackForCurrentScope = scopeToCanvasCalls.get(
76-
nodeToKeyForMap(currentScopeNode)
77-
) || [];
80+
if (!currentScopeNode) {
81+
return;
82+
}
83+
const stackForCurrentScope =
84+
scopeToCanvasCalls.get(nodeToKeyForMap(currentScopeNode)) || [];
7885

79-
if(methodName === 'save') {
86+
if (methodName === 'save') {
8087
stackForCurrentScope.push('save');
81-
} else if(methodName === 'restore') {
88+
} else if (methodName === 'restore') {
8289
// If we get a restore() call but the stack is empty, this means that
8390
// we have nothing to restore as we did not save anything in this
8491
// scope. Either the user has forgotten a save() call, or this
8592
// restore() has been accidentally left behind after a refactor and
8693
// shold be removed.
87-
if(stackForCurrentScope.length === 0) {
94+
if (stackForCurrentScope.length === 0) {
8895
context.report({
8996
messageId: 'uselessRestore',
9097
node: currentScopeNode,
9198
});
9299
} else {
93100
// Pop the stack, so that the last save() is accounted for.
94-
stackForCurrentScope.pop();
101+
stackForCurrentScope.pop();
95102
}
96103
}
97104

98105
scopeToCanvasCalls.set(
99106
nodeToKeyForMap(currentScopeNode),
100-
stackForCurrentScope
107+
stackForCurrentScope,
101108
);
102109
}
103110

@@ -108,9 +115,12 @@ module.exports = {
108115
MemberExpression(node) {
109116
const methodCallsToTrack = ['save', 'restore'];
110117
if (
118+
node.object.type === 'Identifier' &&
111119
node.object?.name === 'context' &&
120+
node.property.type === 'Identifier' &&
112121
methodCallsToTrack.includes(node.property?.name)
113122
) {
123+
// @ts-expect-error the methodCallsToTrack needs `as const`
114124
trackContextCall(node.property.name);
115125
}
116126
},

scripts/eslint_rules/lib/check-enumerated-histograms.js

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,22 +14,33 @@ module.exports = {
1414
category: 'Possible Errors',
1515
},
1616
fixable: 'code',
17-
schema: [] // no options
17+
schema: [], // no options
1818
},
19-
create: function(context) {
19+
create: function (context) {
2020
return {
2121
CallExpression(node) {
22-
if (node?.callee?.object?.name === 'InspectorFrontendHostInstance' &&
23-
node.callee.property.name === 'recordEnumeratedHistogram') {
24-
if (node?.arguments[2]?.property?.name !== 'MAX_VALUE') {
22+
if (
23+
node.callee.type === 'MemberExpression' &&
24+
node.callee.object.type === 'Identifier' &&
25+
node.callee.object.name === 'InspectorFrontendHostInstance' &&
26+
node.callee.property.type === 'Identifier' &&
27+
node.callee.property.name === 'recordEnumeratedHistogram'
28+
) {
29+
const argumentNode = node.arguments[2];
30+
31+
if (
32+
argumentNode.type !== 'MemberExpression' ||
33+
argumentNode.property.type !== 'Identifier' ||
34+
argumentNode.property.name !== 'MAX_VALUE'
35+
) {
2536
context.report({
2637
node,
2738
message:
28-
'When calling \'recordEnumeratedHistogram\' the third argument should be of the form \'SomeEnum.MAX_VALUE\'.'
39+
'When calling \'recordEnumeratedHistogram\' the third argument should be of the form \'SomeEnum.MAX_VALUE\'.',
2940
});
3041
}
3142
}
32-
}
43+
},
3344
};
34-
}
45+
},
3546
};

scripts/eslint_rules/lib/check-license-header.js

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -175,11 +175,9 @@ module.exports = {
175175

176176
const comments = sourceCode.getCommentsBefore(node.body[0]);
177177

178-
if (
179-
!comments ||
180-
comments.length === 0 ||
181-
(comments.length === 1 && comments[0].type === 'Shebang')
182-
) {
178+
if (!comments || comments.length === 0 ||
179+
// @ts-expect-error Shebang is not on the types
180+
(comments.length === 1 && comments[0].type === 'Shebang')) {
183181
context.report({
184182
node,
185183
message: 'Missing license header',
@@ -194,6 +192,7 @@ module.exports = {
194192
let commentsToCheck = comments;
195193
let firstCommentToCheck = comments[0];
196194

195+
// @ts-expect-error Shebang is not on the types
197196
if (comments[0].type === 'Shebang') {
198197
commentsToCheck = comments.slice(1);
199198
firstCommentToCheck = commentsToCheck[0];

scripts/eslint_rules/lib/l10n-no-locked-or-placeholder-only-phrase.js

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,36 +17,49 @@ module.exports = {
1717
type: 'problem',
1818
docs: {
1919
description:
20-
'UIStrings object literals are not allowed to have phrases that are fully locked, or consist only of a single placeholder.',
20+
'UIStrings object literals are not allowed to have phrases that are fully locked, or consist only of a single placeholder.',
2121
category: 'Possible Errors',
2222
},
23-
schema: [] // no options
23+
schema: [], // no options
2424
},
25-
create: function(context) {
25+
create: function (context) {
2626
return {
2727
VariableDeclarator(variableDeclarator) {
28-
if (!l10nHelper.isUIStringsVariableDeclarator(context, variableDeclarator)) {
28+
if (
29+
!l10nHelper.isUIStringsVariableDeclarator(context, variableDeclarator)
30+
) {
2931
return;
3032
}
3133

34+
// @ts-ignore needs TypeScript types
35+
if (variableDeclarator.init?.type !== 'TSAsExpression') {
36+
return;
37+
}
38+
39+
// @ts-ignore needs TypeScript types
3240
for (const property of variableDeclarator.init.expression.properties) {
33-
if (property.type !== 'Property' || property.value.type !== 'Literal') {
41+
if (
42+
property.type !== 'Property' ||
43+
property.value.type !== 'Literal'
44+
) {
3445
continue;
3546
}
3647

3748
if (FULLY_LOCKED_PHRASE_REGEX.test(property.value.value)) {
3849
context.report({
3950
node: property.value,
40-
message: 'Locking whole phrases is not allowed. Use i18n.i18n.lockedString instead.',
51+
message:
52+
'Locking whole phrases is not allowed. Use i18n.i18n.lockedString instead.',
4153
});
4254
} else if (SINGLE_PLACEHOLDER_REGEX.test(property.value.value)) {
4355
context.report({
4456
node: property.value,
45-
message: 'Single placeholder-only phrases are not allowed. Use i18n.i18n.lockedString instead.',
57+
message:
58+
'Single placeholder-only phrases are not allowed. Use i18n.i18n.lockedString instead.',
4659
});
4760
}
4861
}
4962
},
5063
};
51-
}
64+
},
5265
};

scripts/eslint_rules/lib/l10n-no-unused-message.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,9 @@ module.exports = {
5555
// Are there comments in front of the property?
5656
// Move the line we want to remove to the line of the first comment.
5757
const comments = source.getCommentsBefore(property);
58-
if (comments.length > 0) {
59-
lineToRemoveStart = source.getLocFromIndex(comments[0].range[0]).line;
58+
const firstComment = comments[0];
59+
if (firstComment && firstComment.range) {
60+
lineToRemoveStart = source.getLocFromIndex(firstComment.range[0]).line;
6061
}
6162

6263
const removeStart = source.getIndexFromLoc({
@@ -86,6 +87,7 @@ module.exports = {
8687
return;
8788
}
8889

90+
// @ts-expect-error the function above is not typed
8991
for (const property of variableDeclarator.init.expression.properties) {
9092
if (
9193
property.type !== 'Property' ||
@@ -100,6 +102,7 @@ module.exports = {
100102
if (!isStandardUIStringsMemberExpression(memberExpression)) {
101103
return;
102104
}
105+
// @ts-expect-error the function above is not typed
103106
usedUIStringsKeys.add(memberExpression.property.name);
104107
},
105108
'Program:exit': function () {

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ function getEnclosingVariable(estreeNode, sourceCode) {
205205
if (variable) {
206206
return variable;
207207
}
208+
// @ts-expect-error need the above to be typed to undefined
208209
scope = scope.upper;
209210
}
210211
}

scripts/eslint_rules/lib/set-data-type-reference.js

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,51 +12,72 @@ module.exports = {
1212
type: 'problem',
1313

1414
docs: {
15-
description: 'check data setters have an explicit type reference for their parameter',
15+
description:
16+
'check data setters have an explicit type reference for their parameter',
1617
category: 'Possible Errors',
1718
},
1819
fixable: 'code',
19-
schema: [] // no options
20+
schema: [], // no options
2021
},
21-
create: function(context) {
22+
create: function (context) {
2223
return {
2324
ClassDeclaration(node) {
2425
// Only enforce this rule for custom elements
25-
if (!node.superClass || node.superClass.type !== 'Identifier' || node.superClass.name !== 'HTMLElement') {
26+
if (
27+
!node.superClass ||
28+
node.superClass.type !== 'Identifier' ||
29+
node.superClass.name !== 'HTMLElement'
30+
) {
2631
return;
2732
}
2833

2934
const dataSetterDefinition = node.body.body.find(methodDefinition => {
3035
return (
31-
'kind' in methodDefinition && methodDefinition.kind === 'set' && methodDefinition.key.name === 'data');
36+
'kind' in methodDefinition &&
37+
methodDefinition.kind === 'set' &&
38+
'key' in methodDefinition &&
39+
methodDefinition.key.type === 'Identifier' &&
40+
methodDefinition.key.name === 'data'
41+
);
3242
});
3343

34-
if (!dataSetterDefinition || dataSetterDefinition.type === 'StaticBlock') {
44+
if (
45+
!dataSetterDefinition ||
46+
dataSetterDefinition.type === 'StaticBlock'
47+
) {
3548
return;
3649
}
37-
50+
// @ts-expect-error needs proper check of eslint.type
3851
const dataSetterParam = dataSetterDefinition.value?.params?.[0];
3952
if (!dataSetterParam) {
40-
context.report(
41-
{node: dataSetterDefinition, message: 'A data setter must take a parameter that is explicitly typed.'});
53+
context.report({
54+
node: dataSetterDefinition,
55+
message:
56+
'A data setter must take a parameter that is explicitly typed.',
57+
});
4258
return;
4359
}
4460

4561
if (!dataSetterParam.typeAnnotation) {
4662
context.report({
4763
node: dataSetterDefinition,
48-
message: 'The type of a parameter in a data setter must be explicitly defined.'
64+
message:
65+
'The type of a parameter in a data setter must be explicitly defined.',
4966
});
5067
return;
5168
}
5269

53-
if (dataSetterParam.typeAnnotation.typeAnnotation.type !== 'TSTypeReference') {
70+
if (
71+
dataSetterParam.typeAnnotation.typeAnnotation.type !==
72+
'TSTypeReference'
73+
) {
5474
context.report({
5575
node: dataSetterDefinition,
56-
message: 'A data setter parameter\'s type must be a type reference, not a literal type defined inline.'
76+
message:
77+
'A data setter parameter\'s type must be a type reference, not a literal type defined inline.',
5778
});
5879
}
59-
}
80+
},
6081
};
61-
}
82+
},
6283
};

0 commit comments

Comments
 (0)