Auto merge of #27005 - saschanaz:py3-tidy, r=jdm

Do not raise StopIteration: PEP 479

<!-- Please describe your changes on the following line: -->

This fixes `test-tidy` Py3 compatibility, as [PEP 479](https://www.python.org/dev/peps/pep-0479/) says `return` must be used to stop iteration inside generator instead of `raise StopIteration`.

This introduces subtle behavior change where `FileList()` constructor now won't implicitly stop the caller generator when the list is empty. `wpt_lint.py` is modified to explicitly stop when empty, to match the change.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
This commit is contained in:
bors-servo 2020-06-21 02:19:55 -04:00 committed by GitHub
commit fde47f8937
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 24 additions and 29 deletions

View file

@ -141,19 +141,9 @@ class FileList(object):
def __init__(self, directory, only_changed_files=False, exclude_dirs=[], progress=True):
self.directory = directory
self.excluded = exclude_dirs
iterator = self._filter_excluded() if exclude_dirs else self._default_walk()
self.generator = self._filter_excluded() if exclude_dirs else self._default_walk()
if only_changed_files:
try:
# Fall back if git doesn't work
newiter = self._git_changed_files()
obj = next(newiter)
iterator = itertools.chain((obj,), newiter)
except subprocess.CalledProcessError:
pass
# Raise `StopIteration` if the iterator is empty
obj = next(iterator)
self.generator = itertools.chain((obj,), iterator)
self.generator = self._git_changed_files()
if progress:
self.generator = progress_wrapper(self.generator)
@ -165,6 +155,9 @@ class FileList(object):
def _git_changed_files(self):
args = ["git", "log", "-n1", "--merges", "--format=%H"]
last_merge = subprocess.check_output(args, universal_newlines=True).strip()
if not last_merge:
return
args = ["git", "diff", "--name-only", last_merge, self.directory]
file_list = normilize_paths(subprocess.check_output(args, universal_newlines=True).splitlines())
@ -198,7 +191,7 @@ def filter_file(file_name):
def filter_files(start_dir, only_changed_files, progress):
file_iter = FileList(start_dir, only_changed_files=only_changed_files,
exclude_dirs=config["ignore"]["directories"], progress=progress)
# always yield Cargo.lock so that the correctness of transitive dependacies is checked
# always yield Cargo.lock so that the correctness of transitive dependencies is checked
yield "./Cargo.lock"
for file_name in iter(file_iter):
@ -226,7 +219,7 @@ def is_apache_licensed(header):
def check_license(file_name, lines):
if any(file_name.endswith(ext) for ext in (".yml", ".toml", ".lock", ".json", ".html")) or \
config["skip-check-licenses"]:
raise StopIteration
return
if lines[0].startswith(b"#!") and lines[1].strip():
yield (1, "missing blank line after shebang")
@ -264,7 +257,7 @@ def check_modeline(file_name, lines):
def check_length(file_name, idx, line):
if any(file_name.endswith(ext) for ext in (".yml", ".lock", ".json", ".html", ".toml")) or \
config["skip-check-length"]:
raise StopIteration
return
# Prefer shorter lines when shell scripting.
max_length = 80 if file_name.endswith(".sh") else 120
@ -330,7 +323,7 @@ def check_by_line(file_name, lines):
def check_flake8(file_name, contents):
if not file_name.endswith(".py"):
raise StopIteration
return
ignore = {
"W291", # trailing whitespace; the standard tidy process will enforce no trailing whitespace
@ -358,7 +351,7 @@ def check_lock(file_name, contents):
yield package["name"], package["version"], dependency
if not file_name.endswith(".lock"):
raise StopIteration
return
# Package names to be neglected (as named by cargo)
exceptions = config["ignore"]["packages"]
@ -432,11 +425,11 @@ def check_lock(file_name, contents):
def check_toml(file_name, lines):
if not file_name.endswith("Cargo.toml"):
raise StopIteration
return
ok_licensed = False
for idx, line in enumerate(map(lambda line: line.decode("utf-8"), lines)):
if idx == 0 and "[workspace]" in line:
raise StopIteration
return
line_without_comment, _, _ = line.partition("#")
if line_without_comment.find("*") != -1:
yield (idx + 1, "found asterisk instead of minimum version number")
@ -448,7 +441,7 @@ def check_toml(file_name, lines):
def check_shell(file_name, lines):
if not file_name.endswith(".sh"):
raise StopIteration
return
shebang = "#!/usr/bin/env bash"
required_options = ["set -o errexit", "set -o nounset", "set -o pipefail"]
@ -530,7 +523,7 @@ def check_rust(file_name, lines):
file_name.endswith(".mako.rs") or \
file_name.endswith(os.path.join("style", "build.rs")) or \
file_name.endswith(os.path.join("unit", "style", "stylesheets.rs")):
raise StopIteration
return
comment_depth = 0
merged_lines = ''
@ -745,11 +738,11 @@ def check_webidl_spec(file_name, contents):
# }
if not file_name.endswith(".webidl"):
raise StopIteration
return
for i in WEBIDL_STANDARDS:
if contents.find(i) != -1:
raise StopIteration
return
yield (0, "No specification link found.")
@ -792,7 +785,7 @@ class SafeYamlLoader(yaml.SafeLoader):
def check_yaml(file_name, contents):
if not file_name.endswith("buildbot_steps.yml"):
raise StopIteration
return
# YAML specification doesn't explicitly disallow
# duplicate keys, but they shouldn't be allowed in
@ -840,7 +833,7 @@ def check_json_requirements(filename):
def check_json(filename, contents):
if not filename.endswith(".json"):
raise StopIteration
return
try:
json.loads(contents, object_pairs_hook=check_json_requirements(filename))
@ -854,7 +847,7 @@ def check_json(filename, contents):
def check_spec(file_name, lines):
if SPEC_BASE_PATH not in file_name:
raise StopIteration
return
file_name = os.path.relpath(os.path.splitext(file_name)[0], SPEC_BASE_PATH)
patt = re.compile("^\s*\/\/.+")
@ -1017,7 +1010,7 @@ We only expect files with {ext} extensions in {dir_name}'''.format(**details)
def collect_errors_for_files(files_to_check, checking_functions, line_checking_functions, print_text=True):
(has_element, files_to_check) = is_iter_empty(files_to_check)
if not has_element:
raise StopIteration
return
if print_text:
print('\rChecking files for tidiness...')