Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python lint: Use flake8 --extend-ignore instead of --ignore #1498

Merged
merged 1 commit into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 8 additions & 51 deletions .flake8
Original file line number Diff line number Diff line change
@@ -1,55 +1,6 @@
[flake8]
# E111: Indentation is not a multiple of four
# E114: Indentation is not a multiple of four (comment)
# E121: Continuation line under-indented for hanging indent
# E122: continuation line missing indentation or outdented
# E126: continuation line over-indented for hanging indent
# E127: continuation line over-indented for visual indent
# E128: continuation line under-indented for visual indent
# E201: whitespace after '['
# E202: whitespace before ']'
# E203: whitespace before ':'
# E211: whitespace before '('
# E221: multiple spaces before operator
# E226: missing whitespace around arithmetic operator
# E228: missing whitespace around modulo operator
# E231: missing whitespace after ','
# E241: multiple spaces after ','
# E251: unexpected spaces around keyword / parameter equals
# E261: at least two spaces before inline comment
# E262: inline comment should start with '# '
# E266: too many leading '#' for block comment
# E271: multiple spaces after keyword
# E301: expected 1 blank line, found 0
# E302: expected 2 blank lines, found 1
# E303: too many blank lines (2)
# E305: expected 2 blank lines after class or function definition, found 1
# E306: expected 1 blank line before a nested definition, found 0
# E402: module level import not at top of file
# E501: Line too long
# E704: multiple statements on one line (def)
# E713: test for membership should be 'not in'
# E721: do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`
# E722: bare excepts
# E731: do not assign a lambda expression, use a def
# E741: Variable names such as 'l', 'O', or 'I'
# F401: 'pip._internal.cli.index_command.SessionCommandMixin' imported but unused
# F403: 'from .core import *' used; unable to detect undefined names
# F405: 'encode' may be undefined, or defined from star imports: .codec, .core
# F541: f-string is missing placeholders
# F811: redefinition of unused 'Console' from line 8
# F821: undefined name 'basestring'
# F841: local variable 'foos' is assigned to but never used
# W291: trailing whitespace
# W391: blank line at end of file
# W503: line break before binary operator
# W504: line break after binary operator
ignore = E111, E114, E121, E122, E126, E127, E128, E201, E202, E203, E211, E221, E226,
E228, E231, E241, E251, E261, E262, E266, E271, E301, E302, E303, E305, E306, E402,
E501, E704, E713, E721, E722, E731, E741, F401, F403, F405, F541, F811, F821, F841,
W291, W391, W503, W504

exclude =
extend-exclude =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wasn't needed before?

./.*,
./binaryen,
./clang,
./crunch,
Expand All @@ -65,3 +16,9 @@ exclude =
./releases,
./temp,
./upstream,

# E111: Indentation is not a multiple of four
# E114: Indentation is not a multiple of four (comment)
extend-ignore = E111, E114

max-line-length = 326
2 changes: 1 addition & 1 deletion bazel/emscripten_toolchain/link_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
# Only argument should be @path/to/parameter/file
assert sys.argv[1][0] == '@', sys.argv
param_filename = sys.argv[1][1:]
param_file_args = [l.strip() for l in open(param_filename, 'r').readlines()]
param_file_args = [line.strip() for line in open(param_filename, 'r').readlines()]

# Re-write response file if needed.
if any(' ' in a for a in param_file_args):
Expand Down
16 changes: 8 additions & 8 deletions emsdk.py
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,7 @@ def GIT(must_succeed=True):
if ret == 0:
cached_git_executable = git
return git
except:
except Exception:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we are trying to catch everything, why not use the shorter form?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

% ruff rule E722

Why is this bad?

A bare except catches BaseException which includes KeyboardInterrupt, SystemExit, Exception, and others. Catching BaseException can make it hard to interrupt the program (e.g., with Ctrl-C) and can disguise other problems.


https://peps.python.org/pep-0008/#programming-recommendations

When catching exceptions, mention specific exceptions whenever possible instead of using a bare except: clause:

try:
    import platform_specific_module
except ImportError:
    platform_specific_module = None

A bare except: clause will catch SystemExit and KeyboardInterrupt exceptions, making it harder to interrupt a program with Control-C, and can disguise other problems. If you want to catch all exceptions that signal program errors, use except Exception: (bare except is equivalent to except BaseException:).

A good rule of thumb is to limit use of bare ‘except’ clauses to two cases:

  • If the exception handler will be printing out or logging the traceback; at least the user will be aware that an error has occurred.
  • If the code needs to do some cleanup work, but then lets the exception propagate upwards with raise. try...finally can be a better way to handle this case.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes we don't want to catch KeyboardInterrupt here I guess

pass
if must_succeed:
if WINDOWS:
Expand Down Expand Up @@ -853,7 +853,7 @@ def git_pull(repo_path, branch_or_tag):
if ret != 0:
return False
run([GIT(), 'submodule', 'update', '--init'], repo_path, quiet=True)
except:
except Exception:
errlog('git operation failed!')
return False
print("Successfully updated and checked out branch/tag '" + branch_or_tag + "' on repository '" + repo_path + "'")
Expand Down Expand Up @@ -1041,7 +1041,7 @@ def xcode_sdk_version():
if sys.version_info >= (3,):
output = output.decode('utf8')
return output.strip().split('.')
except:
except Exception:
return subprocess.checkplatform.mac_ver()[0].split('.')


Expand Down Expand Up @@ -1490,14 +1490,14 @@ def load_em_config():
lines = []
try:
lines = open(EM_CONFIG_PATH, "r").read().split('\n')
except:
except Exception:
pass
for line in lines:
try:
key, value = parse_key_value(line)
if value != '':
EM_CONFIG_DICT[key] = value
except:
except Exception:
pass


Expand Down Expand Up @@ -2127,13 +2127,13 @@ def make_url(ext):
make_url('tar.xz' if not WINDOWS else 'zip')
try:
urlopen(make_url('tar.xz' if not WINDOWS else 'zip'))
except:
except Exception:
if not WINDOWS:
# Try the old `.tbz2` name
# TODO:remove this once tot builds are all using xz
try:
urlopen(make_url('tbz2'))
except:
except Exception:
continue
else:
continue
Expand Down Expand Up @@ -2918,7 +2918,7 @@ def extract_string_arg(name):
build_type_index = [x.lower() for x in build_types].index(build_type.lower())
CMAKE_BUILD_TYPE_OVERRIDE = build_types[build_type_index]
args[i] = ''
except:
except Exception:
errlog('Unknown CMake build type "' + build_type + '" specified! Please specify one of ' + str(build_types))
return 1
else:
Expand Down
4 changes: 2 additions & 2 deletions scripts/create_release.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ def main(args):

branch_name = 'version_' + new_version

if is_github_runner: # For GitHub Actions workflows
if is_github_runner: # For GitHub Actions workflows
with open(os.environ['GITHUB_ENV'], 'a') as f:
f.write(f'RELEASE_VERSION={new_version}')
else: # Local use
else: # Local use
# Create a new git branch
subprocess.check_call(['git', 'checkout', '-b', branch_name, 'origin/main'], cwd=root_dir)

Expand Down
3 changes: 3 additions & 0 deletions scripts/get_emscripten_revision_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@
EMSCRIPTEN_RELEASES_GIT = 'https://chromium.googlesource.com/emscripten-releases'
TAGFILE = 'emscripten-releases-tags.json'


def get_latest_hash(tagfile):
with open(tagfile) as f:
versions = json.load(f)
latest = versions['aliases']['latest']
return versions['releases'][latest]


def get_latest_emscripten(tagfile):
latest = get_latest_hash(tagfile)
if not os.path.isdir('emscripten-releases'):
Expand All @@ -30,6 +32,7 @@ def get_latest_emscripten(tagfile):
if len(tokens) and tokens[0] == 'emscripten':
return tokens[2]


if __name__ == '__main__':
emscripten_hash = get_latest_emscripten(TAGFILE)
print('Emscripten revision ' + emscripten_hash)
Expand Down
2 changes: 1 addition & 1 deletion scripts/get_release_info.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
#!/usr/bin/env python3

import json
import os
import sys


def get_latest(tagfile):
with open(tagfile) as f:
versions = json.load(f)
Expand Down
2 changes: 0 additions & 2 deletions scripts/update_python.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@
upload_base = 'gs://webassembly/emscripten-releases-builds/deps/'




def make_python_patch():
pywin32_filename = 'pywin32-%s.win-amd64-py%s.exe' % (pywin32_version, major_minor_version)
filename = 'python-%s-amd64.zip' % (version)
Expand Down
1 change: 1 addition & 0 deletions scripts/zip.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import os


def unzip_cmd():
# Use 7-Zip if available (https://www.7-zip.org/)
sevenzip = os.path.join(os.getenv('ProgramFiles', ''), '7-Zip', '7z.exe')
Expand Down
6 changes: 3 additions & 3 deletions test/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@


def listify(x):
if type(x) == list or type(x) == tuple:
if type(x) in {list, tuple}:
return x
return [x]


def check_call(cmd, **args):
if type(cmd) != list:
if type(cmd) is not list:
cmd = cmd.split()
print('running: %s' % cmd)
args['universal_newlines'] = True
Expand Down Expand Up @@ -115,7 +115,7 @@ def do_build(args, is_expected=None):


def run_emsdk(cmd):
if type(cmd) != list:
if type(cmd) is not list:
cmd = cmd.split()
check_call([emsdk] + cmd)

Expand Down