diff --git a/PRESUBMIT.py b/PRESUBMIT.py index 037fd0291e9..997f6d349c9 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -30,34 +30,53 @@ def is_dart_file(path): return path.endswith('.dart') -def _CheckFormat(input_api, - identification, - extension, - windows, - hasFormatErrors: Callable[[str, str], bool], - should_skip=lambda path: False): +def get_old_contents(input_api, path): local_root = input_api.change.RepositoryRoot() upstream = input_api.change._upstream - unformatted_files = [] + return scm.GIT.Capture(['show', upstream + ':' + path], + cwd=local_root, + strip_out=False) + + +def files_to_check_for_format(input_api, extension, exclude_folders): + files = [] + exclude_folders += [ + "pkg/front_end/testcases/", "pkg/front_end/parser_testcases/" + ] for git_file in input_api.AffectedTextFiles(): - if git_file.LocalPath().startswith("pkg/front_end/testcases/"): + local_path = git_file.LocalPath() + if not local_path.endswith(extension): continue - if git_file.LocalPath().startswith("pkg/front_end/parser_testcases/"): - continue - if should_skip(git_file.LocalPath()): + if any([local_path.startswith(f) for f in exclude_folders]): continue + files.append(git_file) + return files + + +def _CheckFormat(input_api, identification, extension, windows, + hasFormatErrors: Callable[[str, list, str], + bool], exclude_folders): + files = files_to_check_for_format(input_api, extension, exclude_folders) + if not files: + return [] + + # Check for formatting errors in bulk first. This is orders of magnitude + # faster than checking file-by-file on large changes with hundreds of files. + if not hasFormatErrors(filenames=[f.AbsoluteLocalPath() for f in files]): + return [] + + print("Formatting errors found, comparing against old versions.") + unformatted_files = [] + for git_file in files: filename = git_file.AbsoluteLocalPath() - if filename.endswith(extension) and hasFormatErrors(filename=filename): + if hasFormatErrors(filename=filename): old_version_has_errors = False try: path = git_file.LocalPath() if windows: # Git expects a linux style path. path = path.replace(os.sep, '/') - old_contents = scm.GIT.Capture(['show', upstream + ':' + path], - cwd=local_root, - strip_out=False) - if hasFormatErrors(contents=old_contents): + if hasFormatErrors(contents=get_old_contents(input_api, path)): old_version_has_errors = True except subprocess.CalledProcessError as e: old_version_has_errors = False @@ -102,15 +121,22 @@ def _CheckDartFormat(input_api, output_api): '--fix-named-default-separator', ] - def HasFormatErrors(filename: str = None, contents: str = None): + def HasFormatErrors(filename: str = None, + filenames: list = None, + contents: str = None): # Don't look for formatting errors in multitests. Since those are very # sensitive to whitespace, many cannot be reformatted without breaking # them. - if filename and filename.endswith('_test.dart'): - with open(filename) as f: - contents = f.read() - if '//#' in contents: - return False + def skip_file(path): + if path.endswith('_test.dart'): + with open(path) as f: + contents = f.read() + if '//#' in contents: + return True + return False + + if filename and skip_file(filename): + return False args = [ dart, @@ -138,6 +164,9 @@ def _CheckDartFormat(input_api, output_api): os.unlink(f.name) elif contents: process = subprocess.run(args, input=contents, text=True) + elif filenames: + args += [f for f in filenames if not skip_file(f)] + process = subprocess.run(args) else: args.append(filename) process = subprocess.run(args) @@ -149,7 +178,7 @@ def _CheckDartFormat(input_api, output_api): return process.returncode == 1 unformatted_files = _CheckFormat(input_api, "dart format", ".dart", windows, - HasFormatErrors) + HasFormatErrors, []) if unformatted_files: lineSep = " \\\n" @@ -185,17 +214,22 @@ def _CheckStatusFiles(input_api, output_api): print('WARNING: Status file linter not found: %s' % lint) return [] - def HasFormatErrors(filename=None, contents=None): + def HasFormatErrors(filename=None, filenames=None, contents=None): + if filenames: + # The status file linter doesn't support checking files in bulk. + # Returning `True` causes `_CheckFormat` to fallback to check + # formatting file by file below. + return True args = [dart, lint] + (['-t'] if contents else [filename]) process = subprocess.run(args, input=contents, text=True) return process.returncode != 0 - def should_skip(path): - return (path.startswith("pkg/status_file/test/data/") or - path.startswith("pkg/front_end/")) - + exclude_folders = [ + "pkg/status_file/test/data/", + "pkg/front_end/", + ] unformatted_files = _CheckFormat(input_api, "status file", ".status", - windows, HasFormatErrors, should_skip) + windows, HasFormatErrors, exclude_folders) if unformatted_files: normalize = os.path.join(local_root, 'pkg', 'status_file', 'bin',