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.

Sign up for GitHub

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

Enforce ruff/tryceratops rules (TRY) #266

Merged
merged 4 commits into from
Aug 26, 2024
Merged

Enforce ruff/tryceratops rules (TRY) #266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bb5f936
Apply ruff/tryceratops rule TRY300
DimitriPapadopoulos Jun 29, 2024
51dc58b
Apply ruff/tryceratops rule TRY301
DimitriPapadopoulos Jun 29, 2024
e166477
Enforce ruff/tryceratops rules (TRY)
DimitriPapadopoulos Jun 29, 2024
d0e3de1
Merge branch 'main' into TRY
jaraco Aug 26, 2024
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
6 changes: 3 additions & 3 deletions distutils/command/install_lib.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ def finalize_options(self):
if not isinstance(self.optimize, int):
try:
self.optimize = int(self.optimize)
if self.optimize not in (0, 1, 2):
raise AssertionError
except (ValueError, AssertionError):
except ValueError:
pass
if self.optimize not in (0, 1, 2):
raise DistutilsOptionError("optimize must be 0, 1, or 2")

def run(self):
Expand Down
5 changes: 3 additions & 2 deletions distutils/cygwinccompiler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,9 @@ def check_config_h():
fn = sysconfig.get_config_h_filename()
try:
config_h = pathlib.Path(fn).read_text(encoding='utf-8')
except OSError as exc:
return (CONFIG_H_UNCERTAIN, f"couldn't read '{fn}': {exc.strerror}")
else:
substring = '__GNUC__'
if substring in config_h:
code = CONFIG_H_OK
Expand All @@ -316,8 +319,6 @@ def check_config_h():
code = CONFIG_H_NOTOK
mention_inflected = 'does not mention'
return code, f"{fn!r} {mention_inflected} {substring!r}"
except OSError as exc:
return (CONFIG_H_UNCERTAIN, f"couldn't read '{fn}': {exc.strerror}")


def is_cygwincc(cc):
Expand Down
2 changes: 1 addition & 1 deletion distutils/extension.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ def __init__(
**kw, # To catch unknown keywords
):
if not isinstance(name, str):
raise AssertionError("'name' must be a string")
raise AssertionError("'name' must be a string") # noqa: TRY004
if not (
isinstance(sources, list)
and all(isinstance(v, (str, os.PathLike)) for v in sources)
Expand Down
3 changes: 2 additions & 1 deletion distutils/file_util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,13 @@ def copy_file( # noqa: C901
if not (os.path.exists(dst) and os.path.samefile(src, dst)):
try:
os.link(src, dst)
return (dst, 1)
except OSError:
# If hard linking fails, fall back on copying file
# (some special filesystems don't support hard linking
# even under Unix, see issue #8876).
pass
else:
return (dst, 1)
Comment on lines +148 to +149
Copy link
Member

Choose a reason for hiding this comment

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

Honestly, I feel like TRY is overreaching here. There's no way that return (dst, 1) is going to cause an OSError, and re-writing with the else clause just distracts from the typical control flow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In general, this rule might help identify cases where it's not clear which part of the code may throw. The usefulness of the TRY300 rule depends on the context: number of lines and complexity of the code under try.

In this specific case, it is quite clear that the return line won't raise en exception. On the other hand, I do not find the else clause less readable. The typical control flow is important of course, but so is clarity about what is tested under try. Additionally, the error control flow feels important too, enough to have 3 lines of comments.

elif link == 'sym':
if not (os.path.exists(dst) and os.path.samefile(src, dst)):
os.symlink(src, dst)
Expand Down
2 changes: 1 addition & 1 deletion distutils/msvc9compiler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ def load_macros(self, version):
if version >= 8.0:
self.set_macro("FrameworkSDKDir", NET_BASE, "sdkinstallrootv2.0")
else:
raise KeyError("sdkinstallrootv2.0")
raise KeyError("sdkinstallrootv2.0") # noqa: TRY301
except KeyError:
raise DistutilsPlatformError(
"""Python was built with Visual Studio 2008;
Expand Down
4 changes: 3 additions & 1 deletion ruff.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,15 @@ extend-select = [
"PERF",
"RUF010",
"RUF100",
"TRY",
"UP",
]
ignore = [
# local
"PERF203",
"TRY003",

# https://docs.astral.sh/ruff/formatter/#conflicting-lint-rules
# https://docs.astral.sh/ruff/formatter/#conflicting-lint-rules
"W191",
"E111",
"E114",
Expand Down
Loading