Request: Option to only apply superfluous-else-* on multi-statement blocks (RET505, RET506, RET507, RET508) #15167
Description
I'm really into if-guards, and superfluous-else-* rules help enable that pattern.
# RET505 will catch this
def foo(bar):
if do_early_validation(bar):
return # or throw
else:
print(bar)
# some longer block of code
# ...
# ...
# ...
# ...
return bar
# and turn it into that
def foo(bar):
if do_early_validation(bar):
return # or throw
# my mind will mostly ignore this part when quickly reading the method to understand what it does
print(bar)
# some longer block of code
# ...
# ...
# ...
# ...
return barHowever, I find that for single statements (a simple "if do this else do that") it doesn't work as well and the else keyword could actually help with the conditional flow readability.
Here's two concrete example from typeshed:
def tests_path(distribution_name: str) -> Path:
if distribution_name == "stdlib":
return STDLIB_PATH / TESTS_DIR
else:
return STUBS_PATH / distribution_name / TESTS_DIR
def allowlists(distribution_name: str) -> list[str]:
prefix = "" if distribution_name == "stdlib" else "stubtest_allowlist_"
version_id = f"py{sys.version_info.major}{sys.version_info.minor}"
platform_allowlist = f"{prefix}{sys.platform}.txt"
version_allowlist = f"{prefix}{version_id}.txt"
combined_allowlist = f"{prefix}{sys.platform}-{version_id}.txt"
local_version_allowlist = version_allowlist + ".local"
if distribution_name == "stdlib":
return ["common.txt", platform_allowlist, version_allowlist, combined_allowlist, local_version_allowlist]
else:
return ["stubtest_allowlist.txt", platform_allowlist]vs
def tests_path(distribution_name: str) -> Path:
if distribution_name == "stdlib":
return STDLIB_PATH / TESTS_DIR
return STUBS_PATH / distribution_name / TESTS_DIR
def allowlists(distribution_name: str) -> list[str]:
prefix = "" if distribution_name == "stdlib" else "stubtest_allowlist_"
version_id = f"py{sys.version_info.major}{sys.version_info.minor}"
platform_allowlist = f"{prefix}{sys.platform}.txt"
version_allowlist = f"{prefix}{version_id}.txt"
combined_allowlist = f"{prefix}{sys.platform}-{version_id}.txt"
local_version_allowlist = version_allowlist + ".local"
if distribution_name == "stdlib":
return ["common.txt", platform_allowlist, version_allowlist, combined_allowlist, local_version_allowlist]
return ["stubtest_allowlist.txt", platform_allowlist]So I'm suggesting that these rule could be only applied to cases where the else block is more than a single statement.
Moreover, users of ternary conditions (like if-else-block-instead-of-if-exp (SIM108)) may prefer turning the single-statement cases into something like this anyway:
def tests_path(distribution_name: str) -> Path:
return STDLIB_PATH / TESTS_DIR if distribution_name == "stdlib" else STUBS_PATH / distribution_name / TESTS_DIR
def allowlists(distribution_name: str) -> list[str]:
return (
["common.txt", platform_allowlist, version_allowlist, combined_allowlist, local_version_allowlist]
if distribution_name == "stdlib"
else ["stubtest_allowlist.txt", platform_allowlist]
)Granted SIM108 doesn't currently cover that, it'd probably be a different rule (#13898 (comment)) but this is getting out of topic.
More keywords: superfluous-else-return , superfluous-else-raise, superfluous-else-continue, superfluous-else-break
Ruff: 0.8.4