Skip to content

Commit 5b2bcff

Browse files
Lightning00BladeDevtools-frontend LUCI CQ
authored andcommitted
[lint] Improvements and fixes to linting
Rerun all files if `lit-analyzer` rules change. Add support for EsLint cache - `npm run lint` delta first run vs second of over 2 min. Don't run EsLint if all files are ignored. Bug: none Change-Id: I042ede01cf0fe6de63b066723e1dc61cae3f9dcc Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6213930 Auto-Submit: Nikolay Vitkov <[email protected]> Reviewed-by: Benedikt Meurer <[email protected]> Commit-Queue: Simon Zünd <[email protected]> Reviewed-by: Simon Zünd <[email protected]>
1 parent 07df73f commit 5b2bcff

File tree

5 files changed

+55
-35
lines changed

5 files changed

+55
-35
lines changed

.gitignore

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,3 +48,7 @@ test/perf/.generated
4848
/.vscode/settings.json
4949
/.vscode/tasks.json
5050
/.vscode/launch.json
51+
52+
# Linters caches
53+
.eslintcache
54+
.stylelintcache

PRESUBMIT.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,9 @@ def _CheckDevToolsLint(input_api, output_api):
292292
'.stylelintrc.json'),
293293
input_api.os_path.join(input_api.PresubmitLocalPath(),
294294
'.stylelintignore'),
295+
# This file includes the LitAnalyzer rules
296+
input_api.os_path.join(input_api.PresubmitLocalPath(),
297+
'tsconfig.json'),
295298
input_api.os_path.join(scripts_directory, 'test',
296299
'run_lint_check.mjs'),
297300
]

eslint.config.mjs

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
/* eslint-disable import/no-default-export */
56
import typescriptPlugin from '@typescript-eslint/eslint-plugin';
67
import mochaPlugin from 'eslint-plugin-mocha';
78
import rulesdirPlugin from 'eslint-plugin-rulesdir';
@@ -26,6 +27,10 @@ export default [
2627
{
2728
name: 'Ignore list',
2829
ignores: [
30+
// Git submodules that are not in third_party
31+
'build/',
32+
'buildtools/',
33+
2934
'front_end/diff/diff_match_patch.jD',
3035
'front_end/models/javascript_metadata/NativeFunctions.js',
3136
// All of these scripts are auto-generated so don't lint them.
@@ -84,6 +89,10 @@ export default [
8489
},
8590
},
8691

92+
linterOptions: {
93+
reportUnusedDisableDirectives: 'error',
94+
},
95+
8796
rules: {
8897
// syntax preferences
8998
'@stylistic/quotes': [
@@ -272,11 +281,11 @@ export default [
272281
parserOptions: {
273282
allowAutomaticSingleRunInference: true,
274283
project: join(
275-
import.meta.dirname,
276-
'config',
277-
'typescript',
278-
'tsconfig.eslint.json',
279-
),
284+
import.meta.dirname,
285+
'config',
286+
'typescript',
287+
'tsconfig.eslint.json',
288+
),
280289
},
281290
},
282291

@@ -465,12 +474,12 @@ export default [
465474
{
466475
// Enforce that any import of models/trace/trace.js names the import Trace.
467476
modulePath: join(
468-
import.meta.dirname,
469-
'front_end',
470-
'models',
471-
'trace',
472-
'trace.js',
473-
),
477+
import.meta.dirname,
478+
'front_end',
479+
'models',
480+
'trace',
481+
'trace.js',
482+
),
474483
importName: 'Trace',
475484
},
476485
],

scripts/test/run_lint_check.mjs

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5-
import { loadESLint } from 'eslint';
5+
import { ESLint } from 'eslint';
66
import stylelint from 'stylelint';
77

88
import { extname, join } from 'path';
@@ -44,25 +44,35 @@ if (!flags.fix) {
4444
console.log('[lint]: fix is disabled; no errors will be autofixed.');
4545
}
4646

47-
async function runESLint(files) {
48-
const EsLintFlat = await loadESLint({ useFlatConfig: true });
49-
const cli = new EsLintFlat({
47+
async function runESLint(scriptFiles) {
48+
const cli = new ESLint({
5049
cwd: join(import.meta.dirname, '..', '..'),
5150
fix: flags.fix,
51+
cache: true,
5252
});
5353

54-
// We filter out certain files in the `.eslintignore`. However, ESLint produces warnings
54+
// We filter out certain files in the `eslint.config.mjs` `Ignore list` entry.
55+
// However, ESLint produces warnings
5556
// when you include a particular file that is ignored. This means that if you edit a file
56-
// that is directly ignored in the `.eslintignore`, ESLint would report a failure.
57+
// that is directly ignored. ESLint would report a failure.
5758
// This was originally reported in https://github.com/eslint/eslint/issues/9977
5859
// The suggested workaround is to use the CLIEngine to preemptively filter out these
5960
// problematic paths.
60-
files = await Promise.all(
61-
files.map(async file => {
62-
return (await cli.isPathIgnored(file)) ? null : file;
63-
}),
64-
);
65-
files = files.filter(file => file !== null);
61+
const files = (
62+
await Promise.all(
63+
scriptFiles.map(async file => {
64+
return (await cli.isPathIgnored(file)) ? null : file;
65+
}),
66+
)
67+
).filter(file => file !== null);
68+
69+
if (files.length === 0) {
70+
// When an empty array is pass lint CWD
71+
// This can happen only if we pass things that will
72+
// be ignored by the above filter
73+
// https://github.com/eslint/eslint/pull/17644
74+
return true;
75+
}
6676

6777
const results = await cli.lintFiles(files);
6878

@@ -79,7 +89,7 @@ async function runESLint(files) {
7989
}
8090

8191
if (flags.fix) {
82-
await EsLintFlat.outputFixes(results);
92+
await ESLint.outputFixes(results);
8393
}
8494

8595
const formatter = await cli.loadFormatter('stylish');
@@ -98,6 +108,8 @@ async function runStylelint(files) {
98108
fix: flags.fix,
99109
files,
100110
formatter: 'string',
111+
cache: true,
112+
allowEmptyInput: true,
101113
});
102114

103115
if (report) {

third_party/i18n/collect-strings.js

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#!/usr/bin/env node
2-
/* eslint-disable no-console, max-len, rulesdir/check-license-header */
2+
/* eslint-disable no-console, rulesdir/check-license-header */
33

44
/**
55
* @license Copyright 2018 The Lighthouse Authors. All Rights Reserved.
@@ -604,12 +604,6 @@ function parseUIStrings(sourceStr) {
604604
/** @type {Map<string, string>} */
605605
const seenStrings = new Map();
606606

607-
/** @type {number} */
608-
let collisions = 0;
609-
610-
/** @type {Array<string>} */
611-
const collisionStrings = [];
612-
613607
/** @type {Record<string, CtcMessage>} */
614608
const strings = {};
615609

@@ -623,7 +617,7 @@ function collectAllStringsInDir(directory) {
623617
// If CWD is contains "third_party" (e.g. in a full Chromium checkout) then
624618
// *all* paths will be ignored. To avoid this, make the directory relative to
625619
// CWD.
626-
directory = path.relative(process.cwd(), directory)
620+
directory = path.relative(process.cwd(), directory);
627621
const files = glob.sync('**/*.{js,ts}', {
628622
cwd: directory,
629623
ignore: IGNORED_PATH_COMPONENTS,
@@ -685,10 +679,7 @@ function collectAllStringsInDir(directory) {
685679
if (seenId) {
686680
if (!strings[seenId].meaning) {
687681
strings[seenId].meaning = strings[seenId].meaning ?? strings[seenId].description;
688-
collisions++;
689682
}
690-
collisionStrings.push(ctc.message);
691-
collisions++;
692683
}
693684
}
694685
seenStrings.set(ctc.message, messageKey);
@@ -721,6 +712,7 @@ if (require.main === module) {
721712
throw new Error('Provide at least one directory from which to collect strings!');
722713
}
723714
const directories = process.argv.slice(2);
715+
/** @type {Record<string, CtcMessage>} */
724716
let collectedStrings = {};
725717
for (const directory of directories) {
726718
collectedStrings = {

0 commit comments

Comments
 (0)