From 1b2c5e463ebeeeb9dfd950e74c94a95225521c02 Mon Sep 17 00:00:00 2001 From: Mathias Bynens Date: Thu, 18 Jun 2020 08:29:21 +0200 Subject: [PATCH] Enable stylelint on presubmit Bug: chromium:1083142 Change-Id: I74136f2ba5b74d4385760e157e2495d706cb5c35 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2248184 Reviewed-by: Tim van der Lippe Reviewed-by: Alex Rudenko Reviewed-by: Liviu Rau Reviewed-by: Changhao Han Commit-Queue: Mathias Bynens --- .stylelintrc.json | 1 - PRESUBMIT.py | 130 ++++++++++++++---- package.json | 2 +- scripts/deps/manage_node_deps.py | 2 +- scripts/test/run_lint_check.py | 9 +- scripts/test/run_lint_check_css.py | 47 +++++++ ...run_lint_check.js => run_lint_check_js.js} | 0 scripts/test/run_lint_check_js.py | 32 +++++ 8 files changed, 189 insertions(+), 34 deletions(-) create mode 100755 scripts/test/run_lint_check_css.py rename scripts/test/{run_lint_check.js => run_lint_check_js.js} (100%) create mode 100755 scripts/test/run_lint_check_js.py diff --git a/.stylelintrc.json b/.stylelintrc.json index 9a1f5e1b9e..9b2eaf9c63 100644 --- a/.stylelintrc.json +++ b/.stylelintrc.json @@ -1,5 +1,4 @@ { - "extends": "stylelint-config-standard", "rules": { "alpha-value-notation": "percentage", "color-function-notation": "modern", diff --git a/PRESUBMIT.py b/PRESUBMIT.py index 943d81366c..a198286e98 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -145,48 +145,97 @@ def _CheckDevtoolsLocalization(input_api, output_api, check_all_files=False): # return _ExecuteSubProcess(input_api, output_api, script_path, args, results) -def _CheckDevtoolsStyle(input_api, output_api): - results = [output_api.PresubmitNotifyResult('Running Devtools Style Check:')] - lint_path = input_api.os_path.join(input_api.PresubmitLocalPath(), 'scripts', 'test', 'run_lint_check.js') +def _CheckDevToolsStyleJS(input_api, output_api): + results = [ + output_api.PresubmitNotifyResult('Running DevTools JS style check:') + ] + lint_path = input_api.os_path.join(input_api.PresubmitLocalPath(), + 'scripts', 'test', + 'run_lint_check_js.js') - front_end_directory = input_api.os_path.join(input_api.PresubmitLocalPath(), 'front_end') - test_directory = input_api.os_path.join(input_api.PresubmitLocalPath(), 'test') - scripts_directory = input_api.os_path.join(input_api.PresubmitLocalPath(), 'scripts') + front_end_directory = input_api.os_path.join( + input_api.PresubmitLocalPath(), 'front_end') + test_directory = input_api.os_path.join(input_api.PresubmitLocalPath(), + 'test') + scripts_directory = input_api.os_path.join(input_api.PresubmitLocalPath(), + 'scripts') - default_linted_directories = [front_end_directory, test_directory, scripts_directory] + default_linted_directories = [ + front_end_directory, test_directory, scripts_directory + ] eslint_related_files = [ - input_api.os_path.join(input_api.PresubmitLocalPath(), 'node_modules', 'eslint'), + input_api.os_path.join(input_api.PresubmitLocalPath(), 'node_modules', + 'eslint'), input_api.os_path.join(input_api.PresubmitLocalPath(), '.eslintrc.js'), - input_api.os_path.join(input_api.PresubmitLocalPath(), '.eslintignore'), - input_api.os_path.join(scripts_directory, 'test', 'run_lint_check.py'), - input_api.os_path.join(scripts_directory, 'test', 'run_lint_check.js'), + input_api.os_path.join(input_api.PresubmitLocalPath(), + '.eslintignore'), + input_api.os_path.join(scripts_directory, 'test', + 'run_lint_check_js.py'), + input_api.os_path.join(scripts_directory, 'test', + 'run_lint_check_js.js'), input_api.os_path.join(scripts_directory, '.eslintrc.js'), input_api.os_path.join(scripts_directory, 'eslint_rules'), ] - affected_files = _getAffectedFiles(input_api, eslint_related_files, [], ['.js', '.py', '.eslintignore']) + lint_config_files = _getAffectedFiles(input_api, eslint_related_files, [], + ['.js', '.py', '.eslintignore']) - # We are changing the ESLint configuration, make sure to run the full check - if len(affected_files) is not 0: - results.append(output_api.PresubmitNotifyResult('Running full ESLint check')) - affected_files = default_linted_directories - else: - # Only run ESLint on files that are relevant, to save PRESUBMIT time - affected_files = _getAffectedFiles(input_api, default_linted_directories, ['D'], ['.js', '.ts']) + files_to_lint = _getFilesToLint(input_api, output_api, lint_config_files, + default_linted_directories, ['.js', '.ts'], + results) + if len(files_to_lint) is 0: + return results - # If we have not changed any lintable files, then we should bail out. - # Otherwise, `run_lint_check.js` will lint *all* files. - if len(affected_files) is 0: - results.append(output_api.PresubmitNotifyResult('No affected files for ESLint check')) - return results - - results.extend(_checkWithNodeScript(input_api, output_api, lint_path, affected_files)) + results.extend( + _checkWithNodeScript(input_api, output_api, lint_path, files_to_lint)) return results +def _CheckDevToolsStyleCSS(input_api, output_api): + results = [ + output_api.PresubmitNotifyResult('Running DevTools CSS style check:') + ] + lint_path = input_api.os_path.join(input_api.PresubmitLocalPath(), + 'scripts', 'test', + 'run_lint_check_css.py') + + front_end_directory = input_api.os_path.join( + input_api.PresubmitLocalPath(), 'front_end') + default_linted_directories = [front_end_directory] + + scripts_directory = input_api.os_path.join(input_api.PresubmitLocalPath(), + 'scripts') + + stylelint_related_files = [ + input_api.os_path.join(input_api.PresubmitLocalPath(), 'node_modules', + 'stylelint'), + input_api.os_path.join(input_api.PresubmitLocalPath(), + '.stylelintrc.json'), + input_api.os_path.join(input_api.PresubmitLocalPath(), + '.stylelintignore'), + input_api.os_path.join(scripts_directory, 'test', + 'run_lint_check_css.py'), + ] + + lint_config_files = _getAffectedFiles(input_api, stylelint_related_files, + [], + ['.json', '.py', '.stylelintignore']) + + files_to_lint = _getFilesToLint(input_api, output_api, lint_config_files, + default_linted_directories, ['.css'], + results) + if len(files_to_lint) is 0: + return results + + return _ExecuteSubProcess(input_api, output_api, lint_path, files_to_lint, + results) + + def _CheckOptimizeSVGHashes(input_api, output_api): - results = [output_api.PresubmitNotifyResult('Running SVG Optimization Check:')] + results = [ + output_api.PresubmitNotifyResult('Running SVG optimization check:') + ] if not input_api.platform.startswith('linux'): return results @@ -309,7 +358,8 @@ def _CommonChecks(input_api, output_api): results.extend(_CheckBuildGN(input_api, output_api)) results.extend(_CheckGeneratedFiles(input_api, output_api)) results.extend(_CheckJSON(input_api, output_api)) - results.extend(_CheckDevtoolsStyle(input_api, output_api)) + results.extend(_CheckDevToolsStyleJS(input_api, output_api)) + results.extend(_CheckDevToolsStyleCSS(input_api, output_api)) results.extend(_CheckFormat(input_api, output_api)) results.extend(_CheckOptimizeSVGHashes(input_api, output_api)) results.extend(_CheckChangesAreExclusiveToDirectory(input_api, output_api)) @@ -356,3 +406,27 @@ def _checkWithNodeScript(input_api, output_api, script_path, script_arguments=[] sys.path = original_sys_path return _ExecuteSubProcess(input_api, output_api, [devtools_paths.node_path(), script_path], script_arguments, []) + + +def _getFilesToLint(input_api, output_api, lint_config_files, + default_linted_directories, accepted_endings, results): + files_to_lint = [] + + # We are changing the lint configuration; run the full check. + if len(lint_config_files) is not 0: + results.append( + output_api.PresubmitNotifyResult('Running full lint check')) + else: + # Only run the linter on files that are relevant, to save PRESUBMIT time. + files_to_lint = _getAffectedFiles(input_api, + default_linted_directories, ['D'], + accepted_endings) + + if len(files_to_lint) is 0: + results.append( + output_api.PresubmitNotifyResult( + 'No affected files for lint check')) + + # Callers should check len(files_to_lint) and bail out if it's 0, + # otherwise all files get linted. + return files_to_lint diff --git a/package.json b/package.json index 2f4a30d22f..5da1c9d12d 100644 --- a/package.json +++ b/package.json @@ -24,7 +24,7 @@ "check-gn": "third_party/node/node.py scripts/check_gn.js", "check-grdp": "third_party/node/node.py scripts/localization/check_localizable_resources.js", "check-json": "third_party/node/node.py scripts/json_validator/validate_module_json.js", - "check-lint": "third_party/node/node.py scripts/test/run_lint_check.js", + "check-lint": "third_party/node/node.py scripts/test/run_lint_check_js.js && python scripts/test/run_lint_check_css.py", "check-loc": "python scripts/test/run_localization_check.py", "check-type": "echo \"If you want to typecheck with TypeScript, run \\\"autoninja -C out/X\\\". If you want to typecheck with Closure, run \\\"npm run check-type-closure\\\"\"", "check-type-closure": "python scripts/test/run_type_check.py", diff --git a/scripts/deps/manage_node_deps.py b/scripts/deps/manage_node_deps.py index ca92c3a53f..37d9b3ee18 100755 --- a/scripts/deps/manage_node_deps.py +++ b/scripts/deps/manage_node_deps.py @@ -61,7 +61,7 @@ DEPS = { "rollup-plugin-terser": "5.3.0", "stylelint": "13.5.0", "typescript": "3.9.3", - "yargs": "15.3.1" + "yargs": "15.3.1", } def exec_command(cmd): diff --git a/scripts/test/run_lint_check.py b/scripts/test/run_lint_check.py index 9cc502f07e..5fda7b9bf9 100755 --- a/scripts/test/run_lint_check.py +++ b/scripts/test/run_lint_check.py @@ -4,6 +4,9 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +# TODO(1083142): remove this file in favor of run_lint_check_js.py once +# infra has been updated. + import sys from os import path from subprocess import Popen @@ -13,13 +16,13 @@ sys.path.append(scripts_path) import devtools_paths CURRENT_DIRECTORY = path.dirname(path.abspath(__file__)) -ROOT_DIRECTORY = path.join(CURRENT_DIRECTORY, '..', '..') +ROOT_DIRECTORY = path.normpath(path.join(CURRENT_DIRECTORY, '..', '..')) def main(): exec_command = [ devtools_paths.node_path(), - path.join(CURRENT_DIRECTORY, 'run_lint_check.js'), + path.join(CURRENT_DIRECTORY, 'run_lint_check_js.js'), ] eslint_proc = Popen(exec_command, cwd=ROOT_DIRECTORY) @@ -27,6 +30,6 @@ def main(): sys.exit(eslint_proc.returncode) -# Run + if __name__ == '__main__': main() diff --git a/scripts/test/run_lint_check_css.py b/scripts/test/run_lint_check_css.py new file mode 100755 index 0000000000..655feba843 --- /dev/null +++ b/scripts/test/run_lint_check_css.py @@ -0,0 +1,47 @@ +#!/usr/bin/env python +# +# Copyright 2020 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +import pathlib +import sys +from os import path +from subprocess import Popen + +scripts_path = path.dirname(path.dirname(path.abspath(__file__))) +sys.path.append(scripts_path) +import devtools_paths + +CURRENT_DIRECTORY = path.dirname(path.abspath(__file__)) +ROOT_DIRECTORY = path.normpath(path.join(CURRENT_DIRECTORY, '..', '..')) +FRONT_END_DIRECTORY = path.join(ROOT_DIRECTORY, 'front_end') +# Note: stylelint requires POSIX-formatted paths, even on Windows. +DEFAULT_GLOB = pathlib.PurePath(path.join(FRONT_END_DIRECTORY, '**', + '*.css')).as_posix + + +def get_css_files_or_glob(): + files = sys.argv[1:] + if len(files): + return files + return [DEFAULT_GLOB] + + +def main(): + exec_command = [ + devtools_paths.node_path(), + path.join(ROOT_DIRECTORY, 'node_modules', 'stylelint', 'bin', + 'stylelint.js'), + ] + exec_command.extend(get_css_files_or_glob()) + exec_command.append('--fix') + + stylelint_proc = Popen(exec_command, cwd=ROOT_DIRECTORY) + stylelint_proc.communicate() + + sys.exit(stylelint_proc.returncode) + + +if __name__ == '__main__': + main() diff --git a/scripts/test/run_lint_check.js b/scripts/test/run_lint_check_js.js similarity index 100% rename from scripts/test/run_lint_check.js rename to scripts/test/run_lint_check_js.js diff --git a/scripts/test/run_lint_check_js.py b/scripts/test/run_lint_check_js.py new file mode 100755 index 0000000000..17694fab09 --- /dev/null +++ b/scripts/test/run_lint_check_js.py @@ -0,0 +1,32 @@ +#!/usr/bin/env python +# +# Copyright 2016 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +import sys +from os import path +from subprocess import Popen + +scripts_path = path.dirname(path.dirname(path.abspath(__file__))) +sys.path.append(scripts_path) +import devtools_paths + +CURRENT_DIRECTORY = path.dirname(path.abspath(__file__)) +ROOT_DIRECTORY = path.normpath(path.join(CURRENT_DIRECTORY, '..', '..')) + + +def main(): + exec_command = [ + devtools_paths.node_path(), + path.join(CURRENT_DIRECTORY, 'run_lint_check_js.js'), + ] + + eslint_proc = Popen(exec_command, cwd=ROOT_DIRECTORY) + eslint_proc.communicate() + + sys.exit(eslint_proc.returncode) + + +if __name__ == '__main__': + main()