From 730e6b2b4ca664db109700daa10fc9915677243c Mon Sep 17 00:00:00 2001 From: konsti Date: Tue, 18 Jul 2023 13:40:15 +0200 Subject: [PATCH] Refactor `StmtIf`: Formatter and Linter (#5459) ## Summary Previously, `StmtIf` was defined recursively as ```rust pub struct StmtIf { pub range: TextRange, pub test: Box, pub body: Vec, pub orelse: Vec, } ``` Every `elif` was represented as an `orelse` with a single `StmtIf`. This means that this representation couldn't differentiate between ```python if cond1: x = 1 else: if cond2: x = 2 ``` and ```python if cond1: x = 1 elif cond2: x = 2 ``` It also makes many checks harder than they need to be because we have to recurse just to iterate over an entire if-elif-else and because we're lacking nodes and ranges on the `elif` and `else` branches. We change the representation to a flat ```rust pub struct StmtIf { pub range: TextRange, pub test: Box, pub body: Vec, pub elif_else_clauses: Vec, } pub struct ElifElseClause { pub range: TextRange, pub test: Option, pub body: Vec, } ``` where `test: Some(_)` represents an `elif` and `test: None` an else. This representation is different tradeoff, e.g. we need to allocate the `Vec`, the `elif`s are now different than the `if`s (which matters in rules where want to check both `if`s and `elif`s) and the type system doesn't guarantee that the `test: None` else is actually last. We're also now a bit more inconsistent since all other `else`, those from `for`, `while` and `try`, still don't have nodes. With the new representation some things became easier, e.g. finding the `elif` token (we can use the start of the `ElifElseClause`) and formatting comments for if-elif-else (no more dangling comments splitting, we only have to insert the dangling comment after the colon manually and set `leading_alternate_branch_comments`, everything else is taken of by having nodes for each branch and the usual placement.rs fixups). ## Merge Plan This PR requires coordination between the parser repo and the main ruff repo. I've split the ruff part, into two stacked PRs which have to be merged together (only the second one fixes all tests), the first for the formatter to be reviewed by @michareiser and the second for the linter to be reviewed by @charliermarsh. * MH: Review and merge https://github.com/astral-sh/RustPython-Parser/pull/20 * MH: Review and merge or move later in stack https://github.com/astral-sh/RustPython-Parser/pull/21 * MH: Review and approve https://github.com/astral-sh/RustPython-Parser/pull/22 * MH: Review and approve formatter PR https://github.com/astral-sh/ruff/pull/5459 * CM: Review and approve linter PR https://github.com/astral-sh/ruff/pull/5460 * Merge linter PR in formatter PR, fix ecosystem checks (ecosystem checks can't run on the formatter PR and won't run on the linter PR, so we need to merge them first) * Merge https://github.com/astral-sh/RustPython-Parser/pull/22 * Create tag in the parser, update linter+formatter PR * Merge linter+formatter PR https://github.com/astral-sh/ruff/pull/5459 --------- Co-authored-by: Micha Reiser --- Cargo.lock | 12 +- Cargo.toml | 12 +- .../test/fixtures/flake8_bugbear/B007.py | 7 + .../test/fixtures/flake8_simplify/SIM102.py | 8 + .../test/fixtures/flake8_simplify/SIM108.py | 2 +- .../test/fixtures/flake8_simplify/SIM114.py | 7 + .../test/fixtures/flake8_simplify/SIM116.py | 12 + .../test/fixtures/flake8_simplify/SIM401.py | 18 +- .../resources/test/fixtures/pyflakes/F634.py | 5 + .../test/fixtures/pyflakes/F811_25.py | 15 + .../fixtures/pylint/collapsible_else_if.py | 14 + .../test/fixtures/pyupgrade/UP036_4.py | 8 +- .../test/fixtures/tryceratops/TRY004.py | 20 + crates/ruff/src/autofix/edits.rs | 16 +- crates/ruff/src/checkers/ast/mod.rs | 70 +- .../rules/reuse_of_groupby_generator.rs | 63 +- .../src/rules/flake8_return/rules/function.rs | 63 +- ...lake8_return__tests__RET508_RET508.py.snap | 9 + .../ruff/src/rules/flake8_return/visitor.rs | 36 +- .../src/rules/flake8_simplify/rules/ast_if.rs | 631 ++++++++---------- .../flake8_simplify/rules/ast_unary_op.rs | 8 +- .../src/rules/flake8_simplify/rules/fix_if.rs | 17 +- .../rules/reimplemented_builtin.rs | 8 +- ...ke8_simplify__tests__SIM102_SIM102.py.snap | 87 ++- ...ke8_simplify__tests__SIM108_SIM108.py.snap | 26 + ...ke8_simplify__tests__SIM114_SIM114.py.snap | 5 +- ...ke8_simplify__tests__SIM116_SIM116.py.snap | 2 + ...ke8_simplify__tests__SIM401_SIM401.py.snap | 34 +- crates/ruff/src/rules/isort/block.rs | 10 +- .../mccabe/rules/function_is_too_complex.rs | 13 +- .../rules/manual_list_comprehension.rs | 7 +- .../pycodestyle/rules/lambda_assignment.rs | 2 + crates/ruff/src/rules/pyflakes/mod.rs | 1 + .../ruff/src/rules/pyflakes/rules/if_tuple.rs | 19 +- ..._rules__pyflakes__tests__F634_F634.py.snap | 36 +- ...les__pyflakes__tests__F811_F811_25.py.snap | 4 + .../rules/pylint/rules/collapsible_else_if.rs | 27 +- .../rules/pylint/rules/continue_in_finally.rs | 13 +- .../rules/pylint/rules/too_many_branches.rs | 133 ++-- .../rules/pylint/rules/too_many_statements.rs | 18 +- .../pylint/rules/useless_else_on_loop.rs | 11 +- ...tests__PLR5501_collapsible_else_if.py.snap | 37 +- ...convert_named_tuple_functional_to_class.rs | 1 + .../convert_typed_dict_functional_to_class.rs | 1 + .../pyupgrade/rules/outdated_version_block.rs | 386 +++++------ ...__rules__pyupgrade__tests__UP036_0.py.snap | 423 +++++------- ...__rules__pyupgrade__tests__UP036_1.py.snap | 212 +++--- ...__rules__pyupgrade__tests__UP036_2.py.snap | 194 +++--- ...__rules__pyupgrade__tests__UP036_3.py.snap | 53 +- ...__rules__pyupgrade__tests__UP036_4.py.snap | 150 ++--- ...__rules__pyupgrade__tests__UP036_5.py.snap | 51 +- ...__rules__unreachable__tests__if.py.md.snap | 80 ++- .../ruff/src/rules/ruff/rules/unreachable.rs | 62 +- .../rules/type_check_without_type_error.rs | 51 +- ...pe-check-without-type-error_TRY004.py.snap | 48 +- crates/ruff_python_ast/src/comparable.rs | 36 +- crates/ruff_python_ast/src/helpers.rs | 79 +-- crates/ruff_python_ast/src/lib.rs | 1 + crates/ruff_python_ast/src/node.rs | 86 ++- .../src/source_code/comment_ranges.rs | 16 + .../src/source_code/generator.rs | 42 +- .../src/source_code/indexer.rs | 12 + .../ruff_python_ast/src/statement_visitor.rs | 21 +- crates/ruff_python_ast/src/stmt_if.rs | 87 +++ crates/ruff_python_ast/src/token_kind.rs | 2 + crates/ruff_python_ast/src/visitor.rs | 24 +- .../ruff_python_ast/src/visitor/preorder.rs | 23 +- .../test/fixtures/ruff/statement/if.py | 37 +- .../src/comments/placement.rs | 224 +++++-- ...formatter__comments__tests__base_test.snap | 4 +- ...omments__tests__if_elif_else_comments.snap | 27 +- ...ents__tests__if_elif_if_else_comments.snap | 30 +- .../src/comments/visitor.rs | 11 +- crates/ruff_python_formatter/src/generated.rs | 40 ++ .../src/statement/mod.rs | 1 + .../src/statement/stmt_class_def.rs | 1 + .../src/statement/stmt_if.rs | 181 +++-- .../format@statement__function.py.snap | 2 +- .../snapshots/format@statement__if.py.snap | 74 +- .../src/analyze/branch_detection.rs | 13 +- crates/ruff_wasm/tests/api.rs | 6 +- fuzz/Cargo.toml | 4 +- 82 files changed, 2333 insertions(+), 2009 deletions(-) create mode 100644 crates/ruff/resources/test/fixtures/pyflakes/F811_25.py create mode 100644 crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F811_F811_25.py.snap create mode 100644 crates/ruff_python_ast/src/stmt_if.rs diff --git a/Cargo.lock b/Cargo.lock index 7b3a67a298..b58fdb4ab8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2226,7 +2226,7 @@ dependencies = [ [[package]] name = "ruff_text_size" version = "0.0.0" -source = "git+https://github.com/astral-sh/RustPython-Parser.git?rev=b996b21ffca562ecb2086f632a6a0b05c245c24a#b996b21ffca562ecb2086f632a6a0b05c245c24a" +source = "git+https://github.com/astral-sh/RustPython-Parser.git?rev=db04fd415774032e1e2ceb03bcbf5305e0d22c8c#db04fd415774032e1e2ceb03bcbf5305e0d22c8c" dependencies = [ "schemars", "serde", @@ -2327,7 +2327,7 @@ dependencies = [ [[package]] name = "rustpython-ast" version = "0.2.0" -source = "git+https://github.com/astral-sh/RustPython-Parser.git?rev=b996b21ffca562ecb2086f632a6a0b05c245c24a#b996b21ffca562ecb2086f632a6a0b05c245c24a" +source = "git+https://github.com/astral-sh/RustPython-Parser.git?rev=db04fd415774032e1e2ceb03bcbf5305e0d22c8c#db04fd415774032e1e2ceb03bcbf5305e0d22c8c" dependencies = [ "is-macro", "num-bigint", @@ -2338,7 +2338,7 @@ dependencies = [ [[package]] name = "rustpython-format" version = "0.2.0" -source = "git+https://github.com/astral-sh/RustPython-Parser.git?rev=b996b21ffca562ecb2086f632a6a0b05c245c24a#b996b21ffca562ecb2086f632a6a0b05c245c24a" +source = "git+https://github.com/astral-sh/RustPython-Parser.git?rev=db04fd415774032e1e2ceb03bcbf5305e0d22c8c#db04fd415774032e1e2ceb03bcbf5305e0d22c8c" dependencies = [ "bitflags 2.3.3", "itertools", @@ -2350,7 +2350,7 @@ dependencies = [ [[package]] name = "rustpython-literal" version = "0.2.0" -source = "git+https://github.com/astral-sh/RustPython-Parser.git?rev=b996b21ffca562ecb2086f632a6a0b05c245c24a#b996b21ffca562ecb2086f632a6a0b05c245c24a" +source = "git+https://github.com/astral-sh/RustPython-Parser.git?rev=db04fd415774032e1e2ceb03bcbf5305e0d22c8c#db04fd415774032e1e2ceb03bcbf5305e0d22c8c" dependencies = [ "hexf-parse", "is-macro", @@ -2362,7 +2362,7 @@ dependencies = [ [[package]] name = "rustpython-parser" version = "0.2.0" -source = "git+https://github.com/astral-sh/RustPython-Parser.git?rev=b996b21ffca562ecb2086f632a6a0b05c245c24a#b996b21ffca562ecb2086f632a6a0b05c245c24a" +source = "git+https://github.com/astral-sh/RustPython-Parser.git?rev=db04fd415774032e1e2ceb03bcbf5305e0d22c8c#db04fd415774032e1e2ceb03bcbf5305e0d22c8c" dependencies = [ "anyhow", "is-macro", @@ -2385,7 +2385,7 @@ dependencies = [ [[package]] name = "rustpython-parser-core" version = "0.2.0" -source = "git+https://github.com/astral-sh/RustPython-Parser.git?rev=b996b21ffca562ecb2086f632a6a0b05c245c24a#b996b21ffca562ecb2086f632a6a0b05c245c24a" +source = "git+https://github.com/astral-sh/RustPython-Parser.git?rev=db04fd415774032e1e2ceb03bcbf5305e0d22c8c#db04fd415774032e1e2ceb03bcbf5305e0d22c8c" dependencies = [ "is-macro", "memchr", diff --git a/Cargo.toml b/Cargo.toml index 04360b6360..a96cd6f742 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -54,12 +54,12 @@ libcst = { git = "https://github.com/Instagram/LibCST.git", rev = "3cacca1a1029f # Please tag the RustPython version every time you update its revision here and in fuzz/Cargo.toml # Tagging the version ensures that older ruff versions continue to build from source even when we rebase our RustPython fork. -# Current tag: v0.0.7 -ruff_text_size = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "b996b21ffca562ecb2086f632a6a0b05c245c24a" } -rustpython-ast = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "b996b21ffca562ecb2086f632a6a0b05c245c24a" , default-features = false, features = ["num-bigint"]} -rustpython-format = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "b996b21ffca562ecb2086f632a6a0b05c245c24a", default-features = false, features = ["num-bigint"] } -rustpython-literal = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "b996b21ffca562ecb2086f632a6a0b05c245c24a", default-features = false } -rustpython-parser = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "b996b21ffca562ecb2086f632a6a0b05c245c24a" , default-features = false, features = ["full-lexer", "num-bigint"] } +# Current tag: v0.0.9 +ruff_text_size = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "db04fd415774032e1e2ceb03bcbf5305e0d22c8c" } +rustpython-ast = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "db04fd415774032e1e2ceb03bcbf5305e0d22c8c" , default-features = false, features = ["num-bigint"]} +rustpython-format = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "db04fd415774032e1e2ceb03bcbf5305e0d22c8c", default-features = false, features = ["num-bigint"] } +rustpython-literal = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "db04fd415774032e1e2ceb03bcbf5305e0d22c8c", default-features = false } +rustpython-parser = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "db04fd415774032e1e2ceb03bcbf5305e0d22c8c" , default-features = false, features = ["full-lexer", "num-bigint"] } [profile.release] lto = "fat" diff --git a/crates/ruff/resources/test/fixtures/flake8_bugbear/B007.py b/crates/ruff/resources/test/fixtures/flake8_bugbear/B007.py index 6c855a7989..4e82accaaa 100644 --- a/crates/ruff/resources/test/fixtures/flake8_bugbear/B007.py +++ b/crates/ruff/resources/test/fixtures/flake8_bugbear/B007.py @@ -97,3 +97,10 @@ def f(): # variable name). for line_ in range(self.header_lines): fp.readline() + +# Regression test: visitor didn't walk the elif test +for key, value in current_crawler_tags.items(): + if key: + pass + elif wanted_tag_value != value: + pass diff --git a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM102.py b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM102.py index 3b47a3e335..a0c3656550 100644 --- a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM102.py +++ b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM102.py @@ -100,6 +100,14 @@ if node.module0123456789: ): print("Bad module!") +# SIM102 +# Regression test for https://github.com/apache/airflow/blob/145b16caaa43f0c42bffd97344df916c602cddde/airflow/configuration.py#L1161 +if a: + if b: + if c: + print("if") +elif d: + print("elif") # OK if a: diff --git a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM108.py b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM108.py index 94b14f911a..956caff2b3 100644 --- a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM108.py +++ b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM108.py @@ -23,7 +23,7 @@ elif a: else: b = 2 -# OK (false negative) +# SIM108 if True: pass else: diff --git a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM114.py b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM114.py index 2a4f155468..d18bb76d3e 100644 --- a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM114.py +++ b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM114.py @@ -94,3 +94,10 @@ if result.eofs == "F": errors = 1 else: errors = 1 + +if a: + # Ignore branches with diverging comments because it means we're repeating + # the bodies because we have different reasons for each branch + x = 1 +elif c: + x = 1 diff --git a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM116.py b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM116.py index d5ddf88107..155c3fccc4 100644 --- a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM116.py +++ b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM116.py @@ -84,3 +84,15 @@ elif func_name == "remove": return "D" elif func_name == "move": return "MV" + +# OK +def no_return_in_else(platform): + if platform == "linux": + return "auditwheel repair -w {dest_dir} {wheel}" + elif platform == "macos": + return "delocate-wheel --require-archs {delocate_archs} -w {dest_dir} -v {wheel}" + elif platform == "windows": + return "" + else: + msg = f"Unknown platform: {platform!r}" + raise ValueError(msg) diff --git a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM401.py b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM401.py index 487f15a5f7..79b48a57dd 100644 --- a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM401.py +++ b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM401.py @@ -38,6 +38,15 @@ if key in a_dict: else: vars[idx] = "defaultß9πŸ’£2ℝ6789ß9πŸ’£2ℝ6789ß9πŸ’£2ℝ6789ß9πŸ’£2ℝ6789ß9πŸ’£2ℝ6789" +# SIM401 +if foo(): + pass +else: + if key in a_dict: + vars[idx] = a_dict[key] + else: + vars[idx] = "default" + ### # Negative cases ### @@ -105,12 +114,3 @@ elif key in a_dict: vars[idx] = a_dict[key] else: vars[idx] = "default" - -# OK (false negative for nested else) -if foo(): - pass -else: - if key in a_dict: - vars[idx] = a_dict[key] - else: - vars[idx] = "default" diff --git a/crates/ruff/resources/test/fixtures/pyflakes/F634.py b/crates/ruff/resources/test/fixtures/pyflakes/F634.py index f23ac85a36..022e334a7a 100644 --- a/crates/ruff/resources/test/fixtures/pyflakes/F634.py +++ b/crates/ruff/resources/test/fixtures/pyflakes/F634.py @@ -1,6 +1,11 @@ if (1, 2): pass +if (3, 4): + pass +elif foo: + pass + for _ in range(5): if True: pass diff --git a/crates/ruff/resources/test/fixtures/pyflakes/F811_25.py b/crates/ruff/resources/test/fixtures/pyflakes/F811_25.py new file mode 100644 index 0000000000..c33acf4b4f --- /dev/null +++ b/crates/ruff/resources/test/fixtures/pyflakes/F811_25.py @@ -0,0 +1,15 @@ +# Regression test for branch detection from +# https://github.com/pypa/build/blob/5800521541e5e749d4429617420d1ef8cdb40b46/src/build/_importlib.py +import sys + +if sys.version_info < (3, 8): + import importlib_metadata as metadata +elif sys.version_info < (3, 9, 10) or (3, 10, 0) <= sys.version_info < (3, 10, 2): + try: + import importlib_metadata as metadata + except ModuleNotFoundError: + from importlib import metadata +else: + from importlib import metadata + +__all__ = ["metadata"] diff --git a/crates/ruff/resources/test/fixtures/pylint/collapsible_else_if.py b/crates/ruff/resources/test/fixtures/pylint/collapsible_else_if.py index 2d12216144..fb00f5d329 100644 --- a/crates/ruff/resources/test/fixtures/pylint/collapsible_else_if.py +++ b/crates/ruff/resources/test/fixtures/pylint/collapsible_else_if.py @@ -47,3 +47,17 @@ def not_ok1(): pass else: pass + + +# Regression test for https://github.com/apache/airflow/blob/f1e1cdcc3b2826e68ba133f350300b5065bbca33/airflow/models/dag.py#L1737 +def not_ok2(): + if True: + print(1) + elif True: + print(2) + else: + if True: + print(3) + else: + print(4) + diff --git a/crates/ruff/resources/test/fixtures/pyupgrade/UP036_4.py b/crates/ruff/resources/test/fixtures/pyupgrade/UP036_4.py index e3eead1251..57184c9c9c 100644 --- a/crates/ruff/resources/test/fixtures/pyupgrade/UP036_4.py +++ b/crates/ruff/resources/test/fixtures/pyupgrade/UP036_4.py @@ -7,20 +7,20 @@ if True: if True: if foo: - pass + print() elif sys.version_info < (3, 3): cmd = [sys.executable, "-m", "test.regrtest"] if True: if foo: - pass + print() elif sys.version_info < (3, 3): cmd = [sys.executable, "-m", "test.regrtest"] elif foo: cmd = [sys.executable, "-m", "test", "-j0"] if foo: - pass + print() elif sys.version_info < (3, 3): cmd = [sys.executable, "-m", "test.regrtest"] @@ -28,7 +28,7 @@ if True: cmd = [sys.executable, "-m", "test.regrtest"] if foo: - pass + print() elif sys.version_info < (3, 3): cmd = [sys.executable, "-m", "test.regrtest"] else: diff --git a/crates/ruff/resources/test/fixtures/tryceratops/TRY004.py b/crates/ruff/resources/test/fixtures/tryceratops/TRY004.py index 2c228e8929..8825737059 100644 --- a/crates/ruff/resources/test/fixtures/tryceratops/TRY004.py +++ b/crates/ruff/resources/test/fixtures/tryceratops/TRY004.py @@ -230,6 +230,15 @@ def incorrect_multi_conditional(arg1, arg2): raise Exception("...") # should be typeerror +def multiple_is_instance_checks(some_arg): + if isinstance(some_arg, str): + pass + elif isinstance(some_arg, int): + pass + else: + raise Exception("...") # should be typeerror + + class MyCustomTypeValidation(Exception): pass @@ -296,6 +305,17 @@ def multiple_ifs(some_args): pass +def else_body(obj): + if isinstance(obj, datetime.timedelta): + return "TimeDelta" + elif isinstance(obj, relativedelta.relativedelta): + return "RelativeDelta" + elif isinstance(obj, CronExpression): + return "CronExpression" + else: + raise Exception(f"Unknown object type: {obj.__class__.__name__}") + + def early_return(): if isinstance(this, some_type): if x in this: diff --git a/crates/ruff/src/autofix/edits.rs b/crates/ruff/src/autofix/edits.rs index 8526815d2a..5bdff428c9 100644 --- a/crates/ruff/src/autofix/edits.rs +++ b/crates/ruff/src/autofix/edits.rs @@ -190,12 +190,24 @@ fn is_lone_child(child: &Stmt, parent: &Stmt) -> bool { } Stmt::For(ast::StmtFor { body, orelse, .. }) | Stmt::AsyncFor(ast::StmtAsyncFor { body, orelse, .. }) - | Stmt::While(ast::StmtWhile { body, orelse, .. }) - | Stmt::If(ast::StmtIf { body, orelse, .. }) => { + | Stmt::While(ast::StmtWhile { body, orelse, .. }) => { if is_only(body, child) || is_only(orelse, child) { return true; } } + Stmt::If(ast::StmtIf { + body, + elif_else_clauses, + .. + }) => { + if is_only(body, child) + || elif_else_clauses + .iter() + .any(|ast::ElifElseClause { body, .. }| is_only(body, child)) + { + return true; + } + } Stmt::Try(ast::StmtTry { body, handlers, diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 61f09c6950..1c39c784a4 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -5,8 +5,8 @@ use log::error; use ruff_text_size::{TextRange, TextSize}; use rustpython_format::cformat::{CFormatError, CFormatErrorType}; use rustpython_parser::ast::{ - self, Arg, ArgWithDefault, Arguments, Comprehension, Constant, ExceptHandler, Expr, - ExprContext, Keyword, Operator, Pattern, Ranged, Stmt, Suite, UnaryOp, + self, Arg, ArgWithDefault, Arguments, Comprehension, Constant, ElifElseClause, ExceptHandler, + Expr, ExprContext, Keyword, Operator, Pattern, Ranged, Stmt, Suite, UnaryOp, }; use ruff_diagnostics::{Diagnostic, Fix, IsolationLevel}; @@ -588,6 +588,7 @@ where name, bases, keywords, + type_params: _, decorator_list, body, range: _, @@ -1159,9 +1160,8 @@ where Stmt::If( stmt_if @ ast::StmtIf { test, - body, - orelse, - range: _, + elif_else_clauses, + .. }, ) => { if self.enabled(Rule::EmptyTypeCheckingBlock) { @@ -1170,70 +1170,42 @@ where } } if self.enabled(Rule::IfTuple) { - pyflakes::rules::if_tuple(self, stmt, test); + pyflakes::rules::if_tuple(self, stmt_if); } if self.enabled(Rule::CollapsibleIf) { flake8_simplify::rules::nested_if_statements( self, - stmt, - test, - body, - orelse, + stmt_if, self.semantic.stmt_parent(), ); } if self.enabled(Rule::IfWithSameArms) { - flake8_simplify::rules::if_with_same_arms( - self, - stmt, - self.semantic.stmt_parent(), - ); + flake8_simplify::rules::if_with_same_arms(self, self.locator, stmt_if); } if self.enabled(Rule::NeedlessBool) { flake8_simplify::rules::needless_bool(self, stmt); } if self.enabled(Rule::IfElseBlockInsteadOfDictLookup) { - flake8_simplify::rules::manual_dict_lookup( - self, - stmt, - test, - body, - orelse, - self.semantic.stmt_parent(), - ); + flake8_simplify::rules::manual_dict_lookup(self, stmt_if); } if self.enabled(Rule::IfElseBlockInsteadOfIfExp) { - flake8_simplify::rules::use_ternary_operator( - self, - stmt, - self.semantic.stmt_parent(), - ); + flake8_simplify::rules::use_ternary_operator(self, stmt); } if self.enabled(Rule::IfElseBlockInsteadOfDictGet) { - flake8_simplify::rules::use_dict_get_with_default( - self, - stmt, - test, - body, - orelse, - self.semantic.stmt_parent(), - ); + flake8_simplify::rules::use_dict_get_with_default(self, stmt_if); } if self.enabled(Rule::TypeCheckWithoutTypeError) { tryceratops::rules::type_check_without_type_error( self, - body, - test, - orelse, + stmt_if, self.semantic.stmt_parent(), ); } if self.enabled(Rule::OutdatedVersionBlock) { - pyupgrade::rules::outdated_version_block(self, stmt, test, body, orelse); + pyupgrade::rules::outdated_version_block(self, stmt_if); } if self.enabled(Rule::CollapsibleElseIf) { - if let Some(diagnostic) = - pylint::rules::collapsible_else_if(orelse, self.locator) + if let Some(diagnostic) = pylint::rules::collapsible_else_if(elif_else_clauses) { self.diagnostics.push(diagnostic); } @@ -2053,7 +2025,7 @@ where stmt_if @ ast::StmtIf { test, body, - orelse, + elif_else_clauses, range: _, }, ) => { @@ -2068,7 +2040,9 @@ where self.visit_body(body); } - self.visit_body(orelse); + for clause in elif_else_clauses { + self.visit_elif_else_clause(clause); + } } _ => visitor::walk_stmt(self, stmt), }; @@ -4344,6 +4318,14 @@ impl<'a> Checker<'a> { self.semantic.flags = snapshot; } + /// Visit an [`ElifElseClause`] + fn visit_elif_else_clause(&mut self, clause: &'a ElifElseClause) { + if let Some(test) = &clause.test { + self.visit_boolean_test(test); + } + self.visit_body(&clause.body); + } + /// Add a [`Binding`] to the current scope, bound to the given name. fn add_binding( &mut self, diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/reuse_of_groupby_generator.rs b/crates/ruff/src/rules/flake8_bugbear/rules/reuse_of_groupby_generator.rs index 1618f0f9b2..736470b3b9 100644 --- a/crates/ruff/src/rules/flake8_bugbear/rules/reuse_of_groupby_generator.rs +++ b/crates/ruff/src/rules/flake8_bugbear/rules/reuse_of_groupby_generator.rs @@ -55,8 +55,6 @@ struct GroupNameFinder<'a> { /// A flag indicating that the `group_name` variable has been overridden /// during the visit. overridden: bool, - /// A stack of `if` statements. - parent_ifs: Vec<&'a Stmt>, /// A stack of counters where each counter is itself a list of usage count. /// This is used specifically for mutually exclusive statements such as an /// `if` or `match`. @@ -77,7 +75,6 @@ impl<'a> GroupNameFinder<'a> { usage_count: 0, nested: false, overridden: false, - parent_ifs: Vec::new(), counter_stack: Vec::new(), exprs: Vec::new(), } @@ -146,56 +143,28 @@ where Stmt::If(ast::StmtIf { test, body, - orelse, + elif_else_clauses, range: _, }) => { - // Determine whether we're on an `if` arm (as opposed to an `elif`). - let is_if_arm = !self.parent_ifs.iter().any(|parent| { - if let Stmt::If(ast::StmtIf { orelse, .. }) = parent { - orelse.len() == 1 && &orelse[0] == stmt - } else { - false - } - }); + // base if plus branches + let mut if_stack = Vec::with_capacity(1 + elif_else_clauses.len()); + // Initialize the vector with the count for the if branch. + if_stack.push(0); + self.counter_stack.push(if_stack); - if is_if_arm { - // Initialize the vector with the count for current branch. - self.counter_stack.push(vec![0]); - } else { - // SAFETY: `unwrap` is safe because we're either in `elif` or - // `else` branch which can come only after an `if` branch. - // When inside an `if` branch, a new vector will be pushed - // onto the stack. + self.visit_expr(test); + self.visit_body(body); + + for clause in elif_else_clauses { self.counter_stack.last_mut().unwrap().push(0); + self.visit_elif_else_clause(clause); } - let has_else = orelse - .first() - .map_or(false, |expr| !matches!(expr, Stmt::If(_))); - - self.parent_ifs.push(stmt); - if has_else { - // There's no `Stmt::Else`; instead, the `else` contents are directly on - // the `orelse` of the `Stmt::If` node. We want to add a new counter for - // the `orelse` branch, but first, we need to visit the `if` body manually. - self.visit_expr(test); - self.visit_body(body); - - // Now, we're in an `else` block. - self.counter_stack.last_mut().unwrap().push(0); - self.visit_body(orelse); - } else { - visitor::walk_stmt(self, stmt); - } - self.parent_ifs.pop(); - - if is_if_arm { - if let Some(last) = self.counter_stack.pop() { - // This is the max number of group usage from all the - // branches of this `if` statement. - let max_count = last.into_iter().max().unwrap_or(0); - self.increment_usage_count(max_count); - } + if let Some(last) = self.counter_stack.pop() { + // This is the max number of group usage from all the + // branches of this `if` statement. + let max_count = last.into_iter().max().unwrap_or(0); + self.increment_usage_count(max_count); } } Stmt::Match(ast::StmtMatch { diff --git a/crates/ruff/src/rules/flake8_return/rules/function.rs b/crates/ruff/src/rules/flake8_return/rules/function.rs index 01d27636d3..c55088654a 100644 --- a/crates/ruff/src/rules/flake8_return/rules/function.rs +++ b/crates/ruff/src/rules/flake8_return/rules/function.rs @@ -1,13 +1,14 @@ use std::ops::Add; use ruff_text_size::{TextRange, TextSize}; -use rustpython_parser::ast::{self, Expr, Ranged, Stmt}; +use rustpython_parser::ast::{self, ElifElseClause, Expr, Ranged, Stmt}; use ruff_diagnostics::{AlwaysAutofixableViolation, Violation}; use ruff_diagnostics::{Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::helpers::is_const_none; -use ruff_python_ast::helpers::{elif_else_range, is_const_false, is_const_true}; +use ruff_python_ast::helpers::{is_const_false, is_const_true}; +use ruff_python_ast::stmt_if::elif_else_range; use ruff_python_ast::visitor::Visitor; use ruff_python_ast::whitespace::indentation; use ruff_python_semantic::SemanticModel; @@ -387,13 +388,25 @@ fn is_noreturn_func(func: &Expr, semantic: &SemanticModel) -> bool { /// RET503 fn implicit_return(checker: &mut Checker, stmt: &Stmt) { match stmt { - Stmt::If(ast::StmtIf { body, orelse, .. }) => { + Stmt::If(ast::StmtIf { + body, + elif_else_clauses, + .. + }) => { if let Some(last_stmt) = body.last() { implicit_return(checker, last_stmt); } - if let Some(last_stmt) = orelse.last() { - implicit_return(checker, last_stmt); - } else { + for clause in elif_else_clauses { + if let Some(last_stmt) = clause.body.last() { + implicit_return(checker, last_stmt); + } + } + + // Check if we don't have an else clause + if matches!( + elif_else_clauses.last(), + None | Some(ast::ElifElseClause { test: Some(_), .. }) + ) { let mut diagnostic = Diagnostic::new(ImplicitReturn, stmt.range()); if checker.patch(diagnostic.kind.rule()) { if let Some(indent) = indentation(checker.locator, stmt) { @@ -564,13 +577,21 @@ fn unnecessary_assign(checker: &mut Checker, stack: &Stack) { } /// RET505, RET506, RET507, RET508 -fn superfluous_else_node(checker: &mut Checker, stmt: &ast::StmtIf, branch: Branch) -> bool { - let ast::StmtIf { body, .. } = stmt; - for child in body { +fn superfluous_else_node( + checker: &mut Checker, + if_elif_body: &[Stmt], + elif_else: &ElifElseClause, +) -> bool { + let branch = if elif_else.test.is_some() { + Branch::Elif + } else { + Branch::Else + }; + for child in if_elif_body { if child.is_return_stmt() { let diagnostic = Diagnostic::new( SuperfluousElseReturn { branch }, - elif_else_range(stmt, checker.locator).unwrap_or_else(|| stmt.range()), + elif_else_range(elif_else, checker.locator).unwrap_or_else(|| elif_else.range()), ); if checker.enabled(diagnostic.kind.rule()) { checker.diagnostics.push(diagnostic); @@ -579,7 +600,7 @@ fn superfluous_else_node(checker: &mut Checker, stmt: &ast::StmtIf, branch: Bran } else if child.is_break_stmt() { let diagnostic = Diagnostic::new( SuperfluousElseBreak { branch }, - elif_else_range(stmt, checker.locator).unwrap_or_else(|| stmt.range()), + elif_else_range(elif_else, checker.locator).unwrap_or_else(|| elif_else.range()), ); if checker.enabled(diagnostic.kind.rule()) { checker.diagnostics.push(diagnostic); @@ -588,7 +609,7 @@ fn superfluous_else_node(checker: &mut Checker, stmt: &ast::StmtIf, branch: Bran } else if child.is_raise_stmt() { let diagnostic = Diagnostic::new( SuperfluousElseRaise { branch }, - elif_else_range(stmt, checker.locator).unwrap_or_else(|| stmt.range()), + elif_else_range(elif_else, checker.locator).unwrap_or_else(|| elif_else.range()), ); if checker.enabled(diagnostic.kind.rule()) { checker.diagnostics.push(diagnostic); @@ -597,7 +618,7 @@ fn superfluous_else_node(checker: &mut Checker, stmt: &ast::StmtIf, branch: Bran } else if child.is_continue_stmt() { let diagnostic = Diagnostic::new( SuperfluousElseContinue { branch }, - elif_else_range(stmt, checker.locator).unwrap_or_else(|| stmt.range()), + elif_else_range(elif_else, checker.locator).unwrap_or_else(|| elif_else.range()), ); if checker.enabled(diagnostic.kind.rule()) { checker.diagnostics.push(diagnostic); @@ -609,16 +630,9 @@ fn superfluous_else_node(checker: &mut Checker, stmt: &ast::StmtIf, branch: Bran } /// RET505, RET506, RET507, RET508 -fn superfluous_elif(checker: &mut Checker, stack: &Stack) { - for stmt in &stack.elifs { - superfluous_else_node(checker, stmt, Branch::Elif); - } -} - -/// RET505, RET506, RET507, RET508 -fn superfluous_else(checker: &mut Checker, stack: &Stack) { - for stmt in &stack.elses { - superfluous_else_node(checker, stmt, Branch::Else); +fn superfluous_elif_else(checker: &mut Checker, stack: &Stack) { + for (if_elif_body, elif_else) in &stack.elifs_elses { + superfluous_else_node(checker, if_elif_body, elif_else); } } @@ -655,8 +669,7 @@ pub(crate) fn function(checker: &mut Checker, body: &[Stmt], returns: Option<&Ex Rule::SuperfluousElseContinue, Rule::SuperfluousElseBreak, ]) { - superfluous_elif(checker, &stack); - superfluous_else(checker, &stack); + superfluous_elif_else(checker, &stack); } // Skip any functions without return statements. diff --git a/crates/ruff/src/rules/flake8_return/snapshots/ruff__rules__flake8_return__tests__RET508_RET508.py.snap b/crates/ruff/src/rules/flake8_return/snapshots/ruff__rules__flake8_return__tests__RET508_RET508.py.snap index 9ae299e91b..5154b39429 100644 --- a/crates/ruff/src/rules/flake8_return/snapshots/ruff__rules__flake8_return__tests__RET508_RET508.py.snap +++ b/crates/ruff/src/rules/flake8_return/snapshots/ruff__rules__flake8_return__tests__RET508_RET508.py.snap @@ -70,4 +70,13 @@ RET508.py:82:9: RET508 Unnecessary `else` after `break` statement 84 | return | +RET508.py:158:13: RET508 Unnecessary `else` after `break` statement + | +156 | if i > w: +157 | break +158 | else: + | ^^^^ RET508 +159 | a = z + | + diff --git a/crates/ruff/src/rules/flake8_return/visitor.rs b/crates/ruff/src/rules/flake8_return/visitor.rs index 32b7daa93d..9d157f0e6e 100644 --- a/crates/ruff/src/rules/flake8_return/visitor.rs +++ b/crates/ruff/src/rules/flake8_return/visitor.rs @@ -1,5 +1,5 @@ use rustc_hash::FxHashSet; -use rustpython_parser::ast::{self, Expr, Identifier, Stmt}; +use rustpython_parser::ast::{self, ElifElseClause, Expr, Identifier, Stmt}; use ruff_python_ast::visitor; use ruff_python_ast::visitor::Visitor; @@ -8,10 +8,8 @@ use ruff_python_ast::visitor::Visitor; pub(super) struct Stack<'a> { /// The `return` statements in the current function. pub(super) returns: Vec<&'a ast::StmtReturn>, - /// The `else` statements in the current function. - pub(super) elses: Vec<&'a ast::StmtIf>, - /// The `elif` statements in the current function. - pub(super) elifs: Vec<&'a ast::StmtIf>, + /// The `elif` or `else` statements in the current function. + pub(super) elifs_elses: Vec<(&'a [Stmt], &'a ElifElseClause)>, /// The non-local variables in the current function. pub(super) non_locals: FxHashSet<&'a str>, /// Whether the current function is a generator. @@ -117,27 +115,13 @@ impl<'a> Visitor<'a> for ReturnVisitor<'a> { self.stack.returns.push(stmt_return); } - Stmt::If(stmt_if) => { - let is_elif_arm = self.parents.iter().any(|parent| { - if let Stmt::If(ast::StmtIf { orelse, .. }) = parent { - orelse.len() == 1 && &orelse[0] == stmt - } else { - false - } - }); - - if !is_elif_arm { - let has_elif = - stmt_if.orelse.len() == 1 && stmt_if.orelse.first().unwrap().is_if_stmt(); - let has_else = !stmt_if.orelse.is_empty(); - - if has_elif { - // `stmt` is an `if` block followed by an `elif` clause. - self.stack.elifs.push(stmt_if); - } else if has_else { - // `stmt` is an `if` block followed by an `else` clause. - self.stack.elses.push(stmt_if); - } + Stmt::If(ast::StmtIf { + body, + elif_else_clauses, + .. + }) => { + if let Some(first) = elif_else_clauses.first() { + self.stack.elifs_elses.push((body, first)); } } _ => {} diff --git a/crates/ruff/src/rules/flake8_simplify/rules/ast_if.rs b/crates/ruff/src/rules/flake8_simplify/rules/ast_if.rs index 7c181d1405..ffa33185cf 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/ast_if.rs +++ b/crates/ruff/src/rules/flake8_simplify/rules/ast_if.rs @@ -1,12 +1,16 @@ use log::error; use ruff_text_size::TextRange; use rustc_hash::FxHashSet; -use rustpython_parser::ast::{self, CmpOp, Constant, Expr, ExprContext, Identifier, Ranged, Stmt}; +use rustpython_parser::ast::{ + self, CmpOp, Constant, ElifElseClause, Expr, ExprContext, Identifier, Ranged, Stmt, StmtIf, +}; use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::comparable::{ComparableConstant, ComparableExpr, ComparableStmt}; use ruff_python_ast::helpers::{any_over_expr, contains_effect, first_colon_range, has_comments}; +use ruff_python_ast::source_code::Locator; +use ruff_python_ast::stmt_if::if_elif_branches; use ruff_python_semantic::SemanticModel; use ruff_python_whitespace::UniversalNewlines; @@ -23,16 +27,6 @@ fn compare_stmt(stmt1: &ComparableStmt, stmt2: &ComparableStmt) -> bool { stmt1.eq(stmt2) } -fn compare_body(body1: &[Stmt], body2: &[Stmt]) -> bool { - if body1.len() != body2.len() { - return false; - } - body1 - .iter() - .zip(body2.iter()) - .all(|(stmt1, stmt2)| compare_stmt(&stmt1.into(), &stmt2.into())) -} - /// ## What it does /// Checks for nested `if` statements that can be collapsed into a single `if` /// statement. @@ -287,7 +281,7 @@ fn is_main_check(expr: &Expr) -> bool { } /// Find the last nested if statement and return the test expression and the -/// first statement. +/// last statement. /// /// ```python /// if xxx: @@ -301,13 +295,13 @@ fn find_last_nested_if(body: &[Stmt]) -> Option<(&Expr, &Stmt)> { let [Stmt::If(ast::StmtIf { test, body: inner_body, - orelse, + elif_else_clauses, .. })] = body else { return None; }; - if !orelse.is_empty() { + if !elif_else_clauses.is_empty() { return None; } find_last_nested_if(inner_body).or_else(|| { @@ -318,30 +312,36 @@ fn find_last_nested_if(body: &[Stmt]) -> Option<(&Expr, &Stmt)> { }) } -/// SIM102 -pub(crate) fn nested_if_statements( - checker: &mut Checker, - stmt: &Stmt, - test: &Expr, - body: &[Stmt], - orelse: &[Stmt], - parent: Option<&Stmt>, -) { - // If the parent could contain a nested if-statement, abort. - if let Some(Stmt::If(ast::StmtIf { body, orelse, .. })) = parent { - if orelse.is_empty() && body.len() == 1 { - return; - } - } +fn nested_if_body(stmt_if: &StmtIf) -> Option<(&[Stmt], TextRange)> { + let StmtIf { + test, + body, + elif_else_clauses, + .. + } = stmt_if; - // If this if-statement has an else clause, or more than one child, abort. - if !(orelse.is_empty() && body.len() == 1) { - return; + // It must be the last condition, otherwise there could be another `elif` or `else` that only + // depends on the outer of the two conditions + let (test, body, range) = if let Some(clause) = elif_else_clauses.last() { + if let Some(test) = &clause.test { + (test, &clause.body, clause.range()) + } else { + // The last condition is an `else` (different rule) + return None; + } + } else { + (test.as_ref(), body, stmt_if.range()) + }; + + // The nested if must be the only child, otherwise there is at least one more statement that + // only depends on the outer condition + if body.len() > 1 { + return None; } // Allow `if __name__ == "__main__":` statements. if is_main_check(test) { - return; + return None; } // Allow `if True:` and `if False:` statements. @@ -352,9 +352,18 @@ pub(crate) fn nested_if_statements( .. }) ) { - return; + return None; } + Some((body, range)) +} + +/// SIM102 +pub(crate) fn nested_if_statements(checker: &mut Checker, stmt_if: &StmtIf, parent: Option<&Stmt>) { + let Some((body, range)) = nested_if_body(stmt_if) else { + return; + }; + // Find the deepest nested if-statement, to inform the range. let Some((test, first_stmt)) = find_last_nested_if(body) else { return; @@ -365,12 +374,22 @@ pub(crate) fn nested_if_statements( checker.locator, ); + // Check if the parent is already emitting a larger diagnostic including this if statement + if let Some(Stmt::If(stmt_if)) = parent { + if let Some((body, _range)) = nested_if_body(stmt_if) { + // In addition to repeating the `nested_if_body` and `find_last_nested_if` check, we + // also need to be the first child in the parent + if matches!(&body[0], Stmt::If(inner) if inner == stmt_if) + && find_last_nested_if(body).is_some() + { + return; + } + } + } + let mut diagnostic = Diagnostic::new( CollapsibleIf, - colon.map_or_else( - || stmt.range(), - |colon| TextRange::new(stmt.start(), colon.end()), - ), + colon.map_or(range, |colon| TextRange::new(range.start(), colon.end())), ); if checker.patch(diagnostic.kind.rule()) { // The fixer preserves comments in the nested body, but removes comments between @@ -379,9 +398,9 @@ pub(crate) fn nested_if_statements( if !checker .indexer .comment_ranges() - .intersects(TextRange::new(stmt.start(), nested_if.start())) + .intersects(TextRange::new(range.start(), nested_if.start())) { - match fix_if::fix_nested_if_statements(checker.locator, checker.stylist, stmt) { + match fix_if::fix_nested_if_statements(checker.locator, checker.stylist, range) { Ok(edit) => { if edit .content() @@ -437,17 +456,43 @@ fn is_one_line_return_bool(stmts: &[Stmt]) -> Option { /// SIM103 pub(crate) fn needless_bool(checker: &mut Checker, stmt: &Stmt) { let Stmt::If(ast::StmtIf { - test, - body, - orelse, + test: if_test, + body: if_body, + elif_else_clauses, range: _, }) = stmt else { return; }; + // Extract an `if` or `elif` (that returns) followed by an else (that returns the same value) + let (if_test, if_body, else_body, range) = match elif_else_clauses.as_slice() { + // if-else case + [ElifElseClause { + body: else_body, + test: None, + .. + }] => (if_test.as_ref(), if_body, else_body, stmt.range()), + // elif-else case + [.., ElifElseClause { + body: elif_body, + test: Some(elif_test), + range: elif_range, + }, ElifElseClause { + body: else_body, + test: None, + range: else_range, + }] => ( + elif_test, + elif_body, + else_body, + TextRange::new(elif_range.start(), else_range.end()), + ), + _ => return, + }; + let (Some(if_return), Some(else_return)) = ( - is_one_line_return_bool(body), - is_one_line_return_bool(orelse), + is_one_line_return_bool(if_body), + is_one_line_return_bool(else_body), ) else { return; }; @@ -458,23 +503,23 @@ pub(crate) fn needless_bool(checker: &mut Checker, stmt: &Stmt) { return; } - let condition = checker.generator().expr(test); - let mut diagnostic = Diagnostic::new(NeedlessBool { condition }, stmt.range()); + let condition = checker.generator().expr(if_test); + let mut diagnostic = Diagnostic::new(NeedlessBool { condition }, range); if checker.patch(diagnostic.kind.rule()) { if matches!(if_return, Bool::True) && matches!(else_return, Bool::False) - && !has_comments(stmt, checker.locator, checker.indexer) - && (test.is_compare_expr() || checker.semantic().is_builtin("bool")) + && !has_comments(&range, checker.locator, checker.indexer) + && (if_test.is_compare_expr() || checker.semantic().is_builtin("bool")) { - if test.is_compare_expr() { + if if_test.is_compare_expr() { // If the condition is a comparison, we can replace it with the condition. let node = ast::StmtReturn { - value: Some(test.clone()), + value: Some(Box::new(if_test.clone())), range: TextRange::default(), }; diagnostic.set_fix(Fix::suggested(Edit::range_replacement( checker.generator().stmt(&node.into()), - stmt.range(), + range, ))); } else { // Otherwise, we need to wrap the condition in a call to `bool`. (We've already @@ -486,7 +531,7 @@ pub(crate) fn needless_bool(checker: &mut Checker, stmt: &Stmt) { }; let node1 = ast::ExprCall { func: Box::new(node.into()), - args: vec![(**test).clone()], + args: vec![if_test.clone()], keywords: vec![], range: TextRange::default(), }; @@ -496,7 +541,7 @@ pub(crate) fn needless_bool(checker: &mut Checker, stmt: &Stmt) { }; diagnostic.set_fix(Fix::suggested(Edit::range_replacement( checker.generator().stmt(&node2.into()), - stmt.range(), + range, ))); }; } @@ -520,99 +565,71 @@ fn ternary(target_var: &Expr, body_value: &Expr, test: &Expr, orelse_value: &Exp node1.into() } -/// Return `true` if the `Expr` contains a reference to `${module}.${target}`. -fn contains_call_path(expr: &Expr, target: &[&str], semantic: &SemanticModel) -> bool { +/// Return `true` if the `Expr` contains a reference to any of the given `${module}.${target}`. +fn contains_call_path(expr: &Expr, targets: &[&[&str]], semantic: &SemanticModel) -> bool { any_over_expr(expr, &|expr| { - semantic - .resolve_call_path(expr) - .map_or(false, |call_path| call_path.as_slice() == target) + semantic.resolve_call_path(expr).map_or(false, |call_path| { + targets.iter().any(|target| &call_path.as_slice() == target) + }) }) } /// SIM108 -pub(crate) fn use_ternary_operator(checker: &mut Checker, stmt: &Stmt, parent: Option<&Stmt>) { +pub(crate) fn use_ternary_operator(checker: &mut Checker, stmt: &Stmt) { let Stmt::If(ast::StmtIf { test, body, - orelse, + elif_else_clauses, range: _, }) = stmt else { return; }; - if body.len() != 1 || orelse.len() != 1 { + // `test: None` to only match an `else` clause + let [ElifElseClause { + body: else_body, + test: None, + .. + }] = elif_else_clauses.as_slice() + else { return; - } - let Stmt::Assign(ast::StmtAssign { + }; + let [Stmt::Assign(ast::StmtAssign { targets: body_targets, value: body_value, .. - }) = &body[0] + })] = body.as_slice() else { return; }; - let Stmt::Assign(ast::StmtAssign { - targets: orelse_targets, - value: orelse_value, + let [Stmt::Assign(ast::StmtAssign { + targets: else_targets, + value: else_value, .. - }) = &orelse[0] + })] = else_body.as_slice() else { return; }; - if body_targets.len() != 1 || orelse_targets.len() != 1 { - return; - } - let Expr::Name(ast::ExprName { id: body_id, .. }) = &body_targets[0] else { + let ([body_target], [else_target]) = (body_targets.as_slice(), else_targets.as_slice()) else { return; }; - let Expr::Name(ast::ExprName { id: orelse_id, .. }) = &orelse_targets[0] else { + let Expr::Name(ast::ExprName { id: body_id, .. }) = body_target else { return; }; - if body_id != orelse_id { + let Expr::Name(ast::ExprName { id: else_id, .. }) = else_target else { + return; + }; + if body_id != else_id { return; } - // Avoid suggesting ternary for `if sys.version_info >= ...`-style checks. - if contains_call_path(test, &["sys", "version_info"], checker.semantic()) { + // Avoid suggesting ternary for `if sys.version_info >= ...`-style and + // `if sys.platform.startswith("...")`-style checks. + let ignored_call_paths: &[&[&str]] = &[&["sys", "version_info"], &["sys", "platform"]]; + if contains_call_path(test, ignored_call_paths, checker.semantic()) { return; } - // Avoid suggesting ternary for `if sys.platform.startswith("...")`-style - // checks. - if contains_call_path(test, &["sys", "platform"], checker.semantic()) { - return; - } - - // It's part of a bigger if-elif block: - // https://github.com/MartinThoma/flake8-simplify/issues/115 - if let Some(Stmt::If(ast::StmtIf { - orelse: parent_orelse, - .. - })) = parent - { - if parent_orelse.len() == 1 && stmt == &parent_orelse[0] { - // TODO(charlie): These two cases have the same AST: - // - // if True: - // pass - // elif a: - // b = 1 - // else: - // b = 2 - // - // if True: - // pass - // else: - // if a: - // b = 1 - // else: - // b = 2 - // - // We want to flag the latter, but not the former. Right now, we flag neither. - return; - } - } - // Avoid suggesting ternary for `if (yield ...)`-style checks. // TODO(charlie): Fix precedence handling for yields in generator. if matches!( @@ -622,14 +639,14 @@ pub(crate) fn use_ternary_operator(checker: &mut Checker, stmt: &Stmt, parent: O return; } if matches!( - orelse_value.as_ref(), + else_value.as_ref(), Expr::Yield(_) | Expr::YieldFrom(_) | Expr::Await(_) ) { return; } - let target_var = &body_targets[0]; - let ternary = ternary(target_var, body_value, test, orelse_value); + let target_var = &body_target; + let ternary = ternary(target_var, body_value, test, else_value); let contents = checker.generator().stmt(&ternary); // Don't flag if the resulting expression would exceed the maximum line length. @@ -659,135 +676,85 @@ pub(crate) fn use_ternary_operator(checker: &mut Checker, stmt: &Stmt, parent: O checker.diagnostics.push(diagnostic); } -fn get_if_body_pairs<'a>( - test: &'a Expr, - body: &'a [Stmt], - orelse: &'a [Stmt], -) -> Vec<(&'a Expr, &'a [Stmt])> { - let mut pairs = vec![(test, body)]; - let mut orelse = orelse; - loop { - if orelse.len() != 1 { - break; - } - let Stmt::If(ast::StmtIf { - test, - body, - orelse: orelse_orelse, - range: _, - }) = &orelse[0] - else { - break; - }; - pairs.push((test, body)); - orelse = orelse_orelse; - } - pairs -} - /// SIM114 -pub(crate) fn if_with_same_arms(checker: &mut Checker, stmt: &Stmt, parent: Option<&Stmt>) { - let Stmt::If(ast::StmtIf { - test, - body, - orelse, - range: _, - }) = stmt - else { - return; - }; +pub(crate) fn if_with_same_arms(checker: &mut Checker, locator: &Locator, stmt_if: &StmtIf) { + let mut branches_iter = if_elif_branches(stmt_if).peekable(); + while let Some(current_branch) = branches_iter.next() { + let Some(following_branch) = branches_iter.peek() else { + continue; + }; - // It's part of a bigger if-elif block: - // https://github.com/MartinThoma/flake8-simplify/issues/115 - if let Some(Stmt::If(ast::StmtIf { - orelse: parent_orelse, - .. - })) = parent - { - if parent_orelse.len() == 1 && stmt == &parent_orelse[0] { - // TODO(charlie): These two cases have the same AST: - // - // if True: - // pass - // elif a: - // b = 1 - // else: - // b = 2 - // - // if True: - // pass - // else: - // if a: - // b = 1 - // else: - // b = 2 - // - // We want to flag the latter, but not the former. Right now, we flag neither. - return; + // The bodies must have the same code ... + if current_branch.body.len() != following_branch.body.len() { + continue; + } + if !current_branch + .body + .iter() + .zip(following_branch.body.iter()) + .all(|(stmt1, stmt2)| compare_stmt(&stmt1.into(), &stmt2.into())) + { + continue; } - } - let if_body_pairs = get_if_body_pairs(test, body, orelse); - for i in 0..(if_body_pairs.len() - 1) { - let (test, body) = &if_body_pairs[i]; - let (.., next_body) = &if_body_pairs[i + 1]; - if compare_body(body, next_body) { - checker.diagnostics.push(Diagnostic::new( - IfWithSameArms, - TextRange::new( - if i == 0 { stmt.start() } else { test.start() }, - next_body.last().unwrap().end(), - ), - )); + // ...and the same comments + let first_comments: Vec<_> = checker + .indexer + .comments_in_range(current_branch.range, locator) + .collect(); + let second_comments: Vec<_> = checker + .indexer + .comments_in_range(following_branch.range, locator) + .collect(); + if first_comments != second_comments { + continue; } + + checker.diagnostics.push(Diagnostic::new( + IfWithSameArms, + TextRange::new( + current_branch.range.start(), + following_branch.body.last().unwrap().end(), + ), + )); } } /// SIM116 -pub(crate) fn manual_dict_lookup( - checker: &mut Checker, - stmt: &Stmt, - test: &Expr, - body: &[Stmt], - orelse: &[Stmt], - parent: Option<&Stmt>, -) { +pub(crate) fn manual_dict_lookup(checker: &mut Checker, stmt_if: &StmtIf) { // Throughout this rule: - // * Each if-statement's test must consist of a constant equality check with the same variable. - // * Each if-statement's body must consist of a single `return`. - // * Each if-statement's orelse must be either another if-statement or empty. - // * The final if-statement's orelse must be empty, or a single `return`. + // * Each if or elif statement's test must consist of a constant equality check with the same variable. + // * Each if or elif statement's body must consist of a single `return`. + // * The else clause must be empty, or a single `return`. + let StmtIf { + body, + test, + elif_else_clauses, + .. + } = stmt_if; + let Expr::Compare(ast::ExprCompare { left, ops, comparators, range: _, - }) = &test + }) = test.as_ref() else { return; }; let Expr::Name(ast::ExprName { id: target, .. }) = left.as_ref() else { return; }; - if body.len() != 1 { + if ops != &[CmpOp::Eq] { return; } - if orelse.len() != 1 { - return; - } - if !(ops.len() == 1 && ops[0] == CmpOp::Eq) { - return; - } - if comparators.len() != 1 { - return; - } - let Expr::Constant(ast::ExprConstant { + let [Expr::Constant(ast::ExprConstant { value: constant, .. - }) = &comparators[0] + })] = comparators.as_slice() else { return; }; - let Stmt::Return(ast::StmtReturn { value, range: _ }) = &body[0] else { + let [Stmt::Return(ast::StmtReturn { value, range: _ })] = body.as_slice() else { return; }; if value.as_ref().map_or(false, |value| { @@ -796,99 +763,60 @@ pub(crate) fn manual_dict_lookup( return; } - // It's part of a bigger if-elif block: - // https://github.com/MartinThoma/flake8-simplify/issues/115 - if let Some(Stmt::If(ast::StmtIf { - orelse: parent_orelse, - .. - })) = parent - { - if parent_orelse.len() == 1 && stmt == &parent_orelse[0] { - // TODO(charlie): These two cases have the same AST: - // - // if True: - // pass - // elif a: - // b = 1 - // else: - // b = 2 - // - // if True: - // pass - // else: - // if a: - // b = 1 - // else: - // b = 2 - // - // We want to flag the latter, but not the former. Right now, we flag neither. - return; - } - } - let mut constants: FxHashSet = FxHashSet::default(); constants.insert(constant.into()); - let mut child: Option<&Stmt> = orelse.get(0); - while let Some(current) = child.take() { - let Stmt::If(ast::StmtIf { - test, - body, - orelse, - range: _, - }) = ¤t - else { - return; - }; - if body.len() != 1 { - return; - } - if orelse.len() > 1 { - return; - } - let Expr::Compare(ast::ExprCompare { - left, - ops, - comparators, - range: _, - }) = test.as_ref() - else { - return; - }; - let Expr::Name(ast::ExprName { id, .. }) = left.as_ref() else { - return; - }; - if !(id == target && matches!(ops.as_slice(), [CmpOp::Eq])) { - return; - } - let [Expr::Constant(ast::ExprConstant { - value: constant, .. - })] = comparators.as_slice() - else { - return; - }; - let Stmt::Return(ast::StmtReturn { value, range: _ }) = &body[0] else { - return; - }; - if value.as_ref().map_or(false, |value| { - contains_effect(value, |id| checker.semantic().is_builtin(id)) - }) { + for clause in elif_else_clauses { + let ElifElseClause { test, body, .. } = clause; + let [Stmt::Return(ast::StmtReturn { value, range: _ })] = body.as_slice() else { return; }; - constants.insert(constant.into()); - if let Some(orelse) = orelse.first() { - match orelse { - Stmt::If(_) => { - child = Some(orelse); - } - Stmt::Return(_) => { - child = None; - } - _ => return, + match test.as_ref() { + // `else` + None => { + // The else must also be a single effect-free return statement + let [Stmt::Return(ast::StmtReturn { value, range: _ })] = body.as_slice() else { + return; + }; + if value.as_ref().map_or(false, |value| { + contains_effect(value, |id| checker.semantic().is_builtin(id)) + }) { + return; + }; + } + // `elif` + Some(Expr::Compare(ast::ExprCompare { + left, + ops, + comparators, + range: _, + })) => { + let Expr::Name(ast::ExprName { id, .. }) = left.as_ref() else { + return; + }; + if id != target || ops != &[CmpOp::Eq] { + return; + } + let [Expr::Constant(ast::ExprConstant { + value: constant, .. + })] = comparators.as_slice() + else { + return; + }; + + if value.as_ref().map_or(false, |value| { + contains_effect(value, |id| checker.semantic().is_builtin(id)) + }) { + return; + }; + + constants.insert(constant.into()); + } + // Different `elif` + _ => { + return; } - } else { - child = None; } } @@ -898,27 +826,38 @@ pub(crate) fn manual_dict_lookup( checker.diagnostics.push(Diagnostic::new( IfElseBlockInsteadOfDictLookup, - stmt.range(), + stmt_if.range(), )); } /// SIM401 -pub(crate) fn use_dict_get_with_default( - checker: &mut Checker, - stmt: &Stmt, - test: &Expr, - body: &[Stmt], - orelse: &[Stmt], - parent: Option<&Stmt>, -) { - if body.len() != 1 || orelse.len() != 1 { +pub(crate) fn use_dict_get_with_default(checker: &mut Checker, stmt_if: &StmtIf) { + let StmtIf { + test, + body, + elif_else_clauses, + .. + } = stmt_if; + + let [body_stmt] = body.as_slice() else { return; - } + }; + let [ElifElseClause { + body: else_body, + test: None, + .. + }] = elif_else_clauses.as_slice() + else { + return; + }; + let [else_body_stmt] = else_body.as_slice() else { + return; + }; let Stmt::Assign(ast::StmtAssign { targets: body_var, value: body_value, .. - }) = &body[0] + }) = &body_stmt else { return; }; @@ -929,7 +868,7 @@ pub(crate) fn use_dict_get_with_default( targets: orelse_var, value: orelse_value, .. - }) = &orelse[0] + }) = &else_body_stmt else { return; }; @@ -941,7 +880,7 @@ pub(crate) fn use_dict_get_with_default( ops, comparators: test_dict, range: _, - }) = &test + }) = test.as_ref() else { return; }; @@ -949,8 +888,18 @@ pub(crate) fn use_dict_get_with_default( return; } let (expected_var, expected_value, default_var, default_value) = match ops[..] { - [CmpOp::In] => (&body_var[0], body_value, &orelse_var[0], orelse_value), - [CmpOp::NotIn] => (&orelse_var[0], orelse_value, &body_var[0], body_value), + [CmpOp::In] => ( + &body_var[0], + body_value, + &orelse_var[0], + orelse_value.as_ref(), + ), + [CmpOp::NotIn] => ( + &orelse_var[0], + orelse_value, + &body_var[0], + body_value.as_ref(), + ), _ => { return; } @@ -979,37 +928,7 @@ pub(crate) fn use_dict_get_with_default( return; } - // It's part of a bigger if-elif block: - // https://github.com/MartinThoma/flake8-simplify/issues/115 - if let Some(Stmt::If(ast::StmtIf { - orelse: parent_orelse, - .. - })) = parent - { - if parent_orelse.len() == 1 && stmt == &parent_orelse[0] { - // TODO(charlie): These two cases have the same AST: - // - // if True: - // pass - // elif a: - // b = 1 - // else: - // b = 2 - // - // if True: - // pass - // else: - // if a: - // b = 1 - // else: - // b = 2 - // - // We want to flag the latter, but not the former. Right now, we flag neither. - return; - } - } - - let node = *default_value.clone(); + let node = default_value.clone(); let node1 = *test_key.clone(); let node2 = ast::ExprAttribute { value: expected_subscript.clone(), @@ -1033,9 +952,9 @@ pub(crate) fn use_dict_get_with_default( let contents = checker.generator().stmt(&node5.into()); // Don't flag if the resulting expression would exceed the maximum line length. - let line_start = checker.locator.line_start(stmt.start()); + let line_start = checker.locator.line_start(stmt_if.start()); if LineWidth::new(checker.settings.tab_size) - .add_str(&checker.locator.contents()[TextRange::new(line_start, stmt.start())]) + .add_str(&checker.locator.contents()[TextRange::new(line_start, stmt_if.start())]) .add_str(&contents) > checker.settings.line_length { @@ -1046,13 +965,13 @@ pub(crate) fn use_dict_get_with_default( IfElseBlockInsteadOfDictGet { contents: contents.clone(), }, - stmt.range(), + stmt_if.range(), ); if checker.patch(diagnostic.kind.rule()) { - if !has_comments(stmt, checker.locator, checker.indexer) { + if !has_comments(stmt_if, checker.locator, checker.indexer) { diagnostic.set_fix(Fix::suggested(Edit::range_replacement( contents, - stmt.range(), + stmt_if.range(), ))); } } diff --git a/crates/ruff/src/rules/flake8_simplify/rules/ast_unary_op.rs b/crates/ruff/src/rules/flake8_simplify/rules/ast_unary_op.rs index b8a4246c2f..9079807755 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/ast_unary_op.rs +++ b/crates/ruff/src/rules/flake8_simplify/rules/ast_unary_op.rs @@ -127,13 +127,7 @@ fn is_dunder_method(name: &str) -> bool { } fn is_exception_check(stmt: &Stmt) -> bool { - let Stmt::If(ast::StmtIf { - test: _, - body, - orelse: _, - range: _, - }) = stmt - else { + let Stmt::If(ast::StmtIf { body, .. }) = stmt else { return false; }; matches!(body.as_slice(), [Stmt::Raise(_)]) diff --git a/crates/ruff/src/rules/flake8_simplify/rules/fix_if.rs b/crates/ruff/src/rules/flake8_simplify/rules/fix_if.rs index 3199c6c509..3b21d0539d 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/fix_if.rs +++ b/crates/ruff/src/rules/flake8_simplify/rules/fix_if.rs @@ -5,14 +5,13 @@ use libcst_native::{ BooleanOp, BooleanOperation, CompoundStatement, Expression, If, LeftParen, ParenthesizableWhitespace, ParenthesizedNode, RightParen, SimpleWhitespace, Statement, Suite, }; -use rustpython_parser::ast::Ranged; +use ruff_text_size::TextRange; -use crate::autofix::codemods::CodegenStylist; use ruff_diagnostics::Edit; use ruff_python_ast::source_code::{Locator, Stylist}; use ruff_python_ast::whitespace; -use ruff_python_whitespace::PythonWhitespace; +use crate::autofix::codemods::CodegenStylist; use crate::cst::matchers::{match_function_def, match_if, match_indented_block, match_statement}; fn parenthesize_and_operand(expr: Expression) -> Expression { @@ -34,21 +33,19 @@ fn parenthesize_and_operand(expr: Expression) -> Expression { pub(crate) fn fix_nested_if_statements( locator: &Locator, stylist: &Stylist, - stmt: &rustpython_parser::ast::Stmt, + range: TextRange, ) -> Result { // Infer the indentation of the outer block. - let Some(outer_indent) = whitespace::indentation(locator, stmt) else { + let Some(outer_indent) = whitespace::indentation(locator, &range) else { bail!("Unable to fix multiline statement"); }; // Extract the module text. - let contents = locator.lines(stmt.range()); - - // Handle `elif` blocks differently; detect them upfront. - let is_elif = contents.trim_whitespace_start().starts_with("elif"); + let contents = locator.lines(range); // If this is an `elif`, we have to remove the `elif` keyword for now. (We'll // restore the `el` later on.) + let is_elif = contents.starts_with("elif"); let module_text = if is_elif { Cow::Owned(contents.replacen("elif", "if", 1)) } else { @@ -128,6 +125,6 @@ pub(crate) fn fix_nested_if_statements( Cow::Borrowed(module_text) }; - let range = locator.lines_range(stmt.range()); + let range = locator.lines_range(range); Ok(Edit::range_replacement(contents.to_string(), range)) } diff --git a/crates/ruff/src/rules/flake8_simplify/rules/reimplemented_builtin.rs b/crates/ruff/src/rules/flake8_simplify/rules/reimplemented_builtin.rs index f0e757b111..4a30d94859 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/reimplemented_builtin.rs +++ b/crates/ruff/src/rules/flake8_simplify/rules/reimplemented_builtin.rs @@ -237,7 +237,7 @@ fn return_values_for_else(stmt: &Stmt) -> Option { let Stmt::If(ast::StmtIf { body: nested_body, test: nested_test, - orelse: nested_orelse, + elif_else_clauses: nested_elif_else_clauses, range: _, }) = &body[0] else { @@ -246,7 +246,7 @@ fn return_values_for_else(stmt: &Stmt) -> Option { if nested_body.len() != 1 { return None; } - if !nested_orelse.is_empty() { + if !nested_elif_else_clauses.is_empty() { return None; } let Stmt::Return(ast::StmtReturn { value, range: _ }) = &nested_body[0] else { @@ -317,7 +317,7 @@ fn return_values_for_siblings<'a>(stmt: &'a Stmt, sibling: &'a Stmt) -> Option(stmt: &'a Stmt, sibling: &'a Stmt) -> Option 0 else -x` instead of `if`-`else`-block | 57 | # SIM108 (without fix due to comments) diff --git a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM114_SIM114.py.snap b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM114_SIM114.py.snap index e9a21dee80..ed2db659c5 100644 --- a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM114_SIM114.py.snap +++ b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM114_SIM114.py.snap @@ -127,12 +127,11 @@ SIM114.py:38:1: SIM114 Combine `if` branches using logical `or` operator 58 | if result.eofs == "O": | -SIM114.py:62:6: SIM114 Combine `if` branches using logical `or` operator +SIM114.py:62:1: SIM114 Combine `if` branches using logical `or` operator | 60 | elif result.eofs == "S": 61 | skipped = 1 -62 | elif result.eofs == "F": - | ______^ +62 | / elif result.eofs == "F": 63 | | errors = 1 64 | | elif result.eofs == "E": 65 | | errors = 1 diff --git a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM116_SIM116.py.snap b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM116_SIM116.py.snap index 0b151c3d72..d2ef5ee577 100644 --- a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM116_SIM116.py.snap +++ b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM116_SIM116.py.snap @@ -105,6 +105,8 @@ SIM116.py:79:1: SIM116 Use a dictionary instead of consecutive `if` statements 85 | | elif func_name == "move": 86 | | return "MV" | |_______________^ SIM116 +87 | +88 | # OK | diff --git a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM401_SIM401.py.snap b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM401_SIM401.py.snap index 9ad99c7484..9408fda39e 100644 --- a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM401_SIM401.py.snap +++ b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM401_SIM401.py.snap @@ -114,7 +114,7 @@ SIM401.py:36:1: SIM401 [*] Use `vars[idx] = a_dict.get(key, "defaultß9πŸ’£2ℝ6 39 | | vars[idx] = "defaultß9πŸ’£2ℝ6789ß9πŸ’£2ℝ6789ß9πŸ’£2ℝ6789ß9πŸ’£2ℝ6789ß9πŸ’£2ℝ6789" | |___________________________________________________________________________^ SIM401 40 | -41 | ### +41 | # SIM401 | = help: Replace with `vars[idx] = a_dict.get(key, "defaultß9πŸ’£2ℝ6789ß9πŸ’£2ℝ6789ß9πŸ’£2ℝ6789ß9πŸ’£2ℝ6789ß9πŸ’£2ℝ6789")` @@ -128,7 +128,35 @@ SIM401.py:36:1: SIM401 [*] Use `vars[idx] = a_dict.get(key, "defaultß9πŸ’£2ℝ6 39 |- vars[idx] = "defaultß9πŸ’£2ℝ6789ß9πŸ’£2ℝ6789ß9πŸ’£2ℝ6789ß9πŸ’£2ℝ6789ß9πŸ’£2ℝ6789" 36 |+vars[idx] = a_dict.get(key, "defaultß9πŸ’£2ℝ6789ß9πŸ’£2ℝ6789ß9πŸ’£2ℝ6789ß9πŸ’£2ℝ6789ß9πŸ’£2ℝ6789") 40 37 | -41 38 | ### -42 39 | # Negative cases +41 38 | # SIM401 +42 39 | if foo(): + +SIM401.py:45:5: SIM401 [*] Use `vars[idx] = a_dict.get(key, "default")` instead of an `if` block + | +43 | pass +44 | else: +45 | if key in a_dict: + | _____^ +46 | | vars[idx] = a_dict[key] +47 | | else: +48 | | vars[idx] = "default" + | |_____________________________^ SIM401 +49 | +50 | ### + | + = help: Replace with `vars[idx] = a_dict.get(key, "default")` + +β„Ή Suggested fix +42 42 | if foo(): +43 43 | pass +44 44 | else: +45 |- if key in a_dict: +46 |- vars[idx] = a_dict[key] +47 |- else: +48 |- vars[idx] = "default" + 45 |+ vars[idx] = a_dict.get(key, "default") +49 46 | +50 47 | ### +51 48 | # Negative cases diff --git a/crates/ruff/src/rules/isort/block.rs b/crates/ruff/src/rules/isort/block.rs index e0eea8ae35..1f5d4f56c1 100644 --- a/crates/ruff/src/rules/isort/block.rs +++ b/crates/ruff/src/rules/isort/block.rs @@ -241,14 +241,18 @@ where } self.finalize(None); } - Stmt::If(ast::StmtIf { body, orelse, .. }) => { + Stmt::If(ast::StmtIf { + body, + elif_else_clauses, + .. + }) => { for stmt in body { self.visit_stmt(stmt); } self.finalize(None); - for stmt in orelse { - self.visit_stmt(stmt); + for clause in elif_else_clauses { + self.visit_elif_else_clause(clause); } self.finalize(None); } diff --git a/crates/ruff/src/rules/mccabe/rules/function_is_too_complex.rs b/crates/ruff/src/rules/mccabe/rules/function_is_too_complex.rs index d67de8efc1..e5242e69e6 100644 --- a/crates/ruff/src/rules/mccabe/rules/function_is_too_complex.rs +++ b/crates/ruff/src/rules/mccabe/rules/function_is_too_complex.rs @@ -68,10 +68,19 @@ fn get_complexity_number(stmts: &[Stmt]) -> usize { let mut complexity = 0; for stmt in stmts { match stmt { - Stmt::If(ast::StmtIf { body, orelse, .. }) => { + Stmt::If(ast::StmtIf { + body, + elif_else_clauses, + .. + }) => { complexity += 1; complexity += get_complexity_number(body); - complexity += get_complexity_number(orelse); + for clause in elif_else_clauses { + if clause.test.is_some() { + complexity += 1; + } + complexity += get_complexity_number(&clause.body); + } } Stmt::For(ast::StmtFor { body, orelse, .. }) | Stmt::AsyncFor(ast::StmtAsyncFor { body, orelse, .. }) => { diff --git a/crates/ruff/src/rules/perflint/rules/manual_list_comprehension.rs b/crates/ruff/src/rules/perflint/rules/manual_list_comprehension.rs index b5bf5ea1ae..73b6ee60be 100644 --- a/crates/ruff/src/rules/perflint/rules/manual_list_comprehension.rs +++ b/crates/ruff/src/rules/perflint/rules/manual_list_comprehension.rs @@ -64,9 +64,12 @@ pub(crate) fn manual_list_comprehension(checker: &mut Checker, target: &Expr, bo // filtered.append(x) // ``` [Stmt::If(ast::StmtIf { - body, orelse, test, .. + body, + elif_else_clauses, + test, + .. })] => { - if !orelse.is_empty() { + if !elif_else_clauses.is_empty() { return; } let [stmt] = body.as_slice() else { diff --git a/crates/ruff/src/rules/pycodestyle/rules/lambda_assignment.rs b/crates/ruff/src/rules/pycodestyle/rules/lambda_assignment.rs index 8e3f4930e2..0bb158482b 100644 --- a/crates/ruff/src/rules/pycodestyle/rules/lambda_assignment.rs +++ b/crates/ruff/src/rules/pycodestyle/rules/lambda_assignment.rs @@ -224,6 +224,7 @@ fn function( body: vec![body], decorator_list: vec![], returns: Some(Box::new(return_type)), + type_params: vec![], type_comment: None, range: TextRange::default(), }); @@ -236,6 +237,7 @@ fn function( body: vec![body], decorator_list: vec![], returns: None, + type_params: vec![], type_comment: None, range: TextRange::default(), }); diff --git a/crates/ruff/src/rules/pyflakes/mod.rs b/crates/ruff/src/rules/pyflakes/mod.rs index e796ac4563..1f13edf973 100644 --- a/crates/ruff/src/rules/pyflakes/mod.rs +++ b/crates/ruff/src/rules/pyflakes/mod.rs @@ -107,6 +107,7 @@ mod tests { #[test_case(Rule::RedefinedWhileUnused, Path::new("F811_22.py"))] #[test_case(Rule::RedefinedWhileUnused, Path::new("F811_23.py"))] #[test_case(Rule::RedefinedWhileUnused, Path::new("F811_24.py"))] + #[test_case(Rule::RedefinedWhileUnused, Path::new("F811_25.py"))] #[test_case(Rule::UndefinedName, Path::new("F821_0.py"))] #[test_case(Rule::UndefinedName, Path::new("F821_1.py"))] #[test_case(Rule::UndefinedName, Path::new("F821_2.py"))] diff --git a/crates/ruff/src/rules/pyflakes/rules/if_tuple.rs b/crates/ruff/src/rules/pyflakes/rules/if_tuple.rs index 98ce068c20..0fe40fd842 100644 --- a/crates/ruff/src/rules/pyflakes/rules/if_tuple.rs +++ b/crates/ruff/src/rules/pyflakes/rules/if_tuple.rs @@ -1,7 +1,8 @@ -use rustpython_parser::ast::{self, Expr, Ranged, Stmt}; +use rustpython_parser::ast::{self, Expr, Ranged, StmtIf}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::stmt_if::if_elif_branches; use crate::checkers::ast::Checker; @@ -37,12 +38,16 @@ impl Violation for IfTuple { } /// F634 -pub(crate) fn if_tuple(checker: &mut Checker, stmt: &Stmt, test: &Expr) { - if let Expr::Tuple(ast::ExprTuple { elts, .. }) = &test { - if !elts.is_empty() { - checker - .diagnostics - .push(Diagnostic::new(IfTuple, stmt.range())); +pub(crate) fn if_tuple(checker: &mut Checker, stmt_if: &StmtIf) { + for branch in if_elif_branches(stmt_if) { + let Expr::Tuple(ast::ExprTuple { elts, .. }) = &branch.test else { + continue; + }; + if elts.is_empty() { + continue; } + checker + .diagnostics + .push(Diagnostic::new(IfTuple, branch.test.range())); } } diff --git a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F634_F634.py.snap b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F634_F634.py.snap index 7f3a889816..d753d6d96e 100644 --- a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F634_F634.py.snap +++ b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F634_F634.py.snap @@ -1,25 +1,31 @@ --- source: crates/ruff/src/rules/pyflakes/mod.rs --- -F634.py:1:1: F634 If test is a tuple, which is always `True` +F634.py:1:4: F634 If test is a tuple, which is always `True` | -1 | / if (1, 2): -2 | | pass - | |________^ F634 -3 | -4 | for _ in range(5): +1 | if (1, 2): + | ^^^^^^ F634 +2 | pass | -F634.py:7:5: F634 If test is a tuple, which is always `True` +F634.py:4:4: F634 If test is a tuple, which is always `True` + | +2 | pass +3 | +4 | if (3, 4): + | ^^^^^^ F634 +5 | pass +6 | elif foo: + | + +F634.py:12:10: F634 If test is a tuple, which is always `True` | - 5 | if True: - 6 | pass - 7 | elif (3, 4): - | _____^ - 8 | | pass - 9 | | elif (): -10 | | pass - | |____________^ F634 +10 | if True: +11 | pass +12 | elif (3, 4): + | ^^^^^^ F634 +13 | pass +14 | elif (): | diff --git a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F811_F811_25.py.snap b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F811_F811_25.py.snap new file mode 100644 index 0000000000..1976c4331d --- /dev/null +++ b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F811_F811_25.py.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff/src/rules/pyflakes/mod.rs +--- + diff --git a/crates/ruff/src/rules/pylint/rules/collapsible_else_if.rs b/crates/ruff/src/rules/pylint/rules/collapsible_else_if.rs index 57a1bf8405..1ad81b4cf7 100644 --- a/crates/ruff/src/rules/pylint/rules/collapsible_else_if.rs +++ b/crates/ruff/src/rules/pylint/rules/collapsible_else_if.rs @@ -1,8 +1,8 @@ -use rustpython_parser::ast::{Ranged, Stmt}; +use ruff_text_size::TextRange; +use rustpython_parser::ast::{ElifElseClause, Ranged, Stmt}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::source_code::Locator; /// ## What it does /// Checks for `else` blocks that consist of a single `if` statement. @@ -47,15 +47,20 @@ impl Violation for CollapsibleElseIf { } /// PLR5501 -pub(crate) fn collapsible_else_if(orelse: &[Stmt], locator: &Locator) -> Option { - if orelse.len() == 1 { - let first = &orelse[0]; - if matches!(first, Stmt::If(_)) { - // Determine whether this is an `elif`, or an `if` in an `else` block. - if locator.slice(first.range()).starts_with("if") { - return Some(Diagnostic::new(CollapsibleElseIf, first.range())); - } - } +pub(crate) fn collapsible_else_if(elif_else_clauses: &[ElifElseClause]) -> Option { + let Some(ElifElseClause { + body, + test: None, + range, + }) = elif_else_clauses.last() + else { + return None; + }; + if let [first @ Stmt::If(_)] = body.as_slice() { + return Some(Diagnostic::new( + CollapsibleElseIf, + TextRange::new(range.start(), first.start()), + )); } None } diff --git a/crates/ruff/src/rules/pylint/rules/continue_in_finally.rs b/crates/ruff/src/rules/pylint/rules/continue_in_finally.rs index 600ee4306e..84a553c929 100644 --- a/crates/ruff/src/rules/pylint/rules/continue_in_finally.rs +++ b/crates/ruff/src/rules/pylint/rules/continue_in_finally.rs @@ -54,8 +54,17 @@ fn traverse_body(checker: &mut Checker, body: &[Stmt]) { } match stmt { - Stmt::If(ast::StmtIf { body, orelse, .. }) - | Stmt::Try(ast::StmtTry { body, orelse, .. }) + Stmt::If(ast::StmtIf { + body, + elif_else_clauses, + .. + }) => { + traverse_body(checker, body); + for clause in elif_else_clauses { + traverse_body(checker, &clause.body); + } + } + Stmt::Try(ast::StmtTry { body, orelse, .. }) | Stmt::TryStar(ast::StmtTryStar { body, orelse, .. }) => { traverse_body(checker, body); traverse_body(checker, orelse); diff --git a/crates/ruff/src/rules/pylint/rules/too_many_branches.rs b/crates/ruff/src/rules/pylint/rules/too_many_branches.rs index 5d997ab843..32e2cf728b 100644 --- a/crates/ruff/src/rules/pylint/rules/too_many_branches.rs +++ b/crates/ruff/src/rules/pylint/rules/too_many_branches.rs @@ -88,74 +88,74 @@ impl Violation for TooManyBranches { fn num_branches(stmts: &[Stmt]) -> usize { stmts .iter() - .map(|stmt| { - match stmt { - Stmt::If(ast::StmtIf { body, orelse, .. }) => { - 1 + num_branches(body) - + (if let Some(stmt) = orelse.first() { - // `elif:` and `else: if:` have the same AST representation. - // Avoid treating `elif:` as two statements. - usize::from(!stmt.is_if_stmt()) - } else { - 0 - }) - + num_branches(orelse) - } - Stmt::Match(ast::StmtMatch { cases, .. }) => { - 1 + cases + .map(|stmt| match stmt { + Stmt::If(ast::StmtIf { + body, + elif_else_clauses, + .. + }) => { + 1 + num_branches(body) + + elif_else_clauses.len() + + elif_else_clauses .iter() - .map(|case| num_branches(&case.body)) + .map(|clause| num_branches(&clause.body)) .sum::() - } - Stmt::For(ast::StmtFor { body, orelse, .. }) - | Stmt::AsyncFor(ast::StmtAsyncFor { body, orelse, .. }) - | Stmt::While(ast::StmtWhile { body, orelse, .. }) => { - 1 + num_branches(body) - + (if orelse.is_empty() { - 0 - } else { - 1 + num_branches(orelse) - }) - } - Stmt::Try(ast::StmtTry { - body, - handlers, - orelse, - finalbody, - range: _, - }) - | Stmt::TryStar(ast::StmtTryStar { - body, - handlers, - orelse, - finalbody, - range: _, - }) => { - 1 + num_branches(body) - + (if orelse.is_empty() { - 0 - } else { - 1 + num_branches(orelse) - }) - + (if finalbody.is_empty() { - 0 - } else { - 1 + num_branches(finalbody) - }) - + handlers - .iter() - .map(|handler| { - 1 + { - let ExceptHandler::ExceptHandler( - ast::ExceptHandlerExceptHandler { body, .. }, - ) = handler; - num_branches(body) - } - }) - .sum::() - } - _ => 0, } + Stmt::Match(ast::StmtMatch { cases, .. }) => { + 1 + cases + .iter() + .map(|case| num_branches(&case.body)) + .sum::() + } + Stmt::For(ast::StmtFor { body, orelse, .. }) + | Stmt::AsyncFor(ast::StmtAsyncFor { body, orelse, .. }) + | Stmt::While(ast::StmtWhile { body, orelse, .. }) => { + 1 + num_branches(body) + + (if orelse.is_empty() { + 0 + } else { + 1 + num_branches(orelse) + }) + } + Stmt::Try(ast::StmtTry { + body, + handlers, + orelse, + finalbody, + range: _, + }) + | Stmt::TryStar(ast::StmtTryStar { + body, + handlers, + orelse, + finalbody, + range: _, + }) => { + 1 + num_branches(body) + + (if orelse.is_empty() { + 0 + } else { + 1 + num_branches(orelse) + }) + + (if finalbody.is_empty() { + 0 + } else { + 1 + num_branches(finalbody) + }) + + handlers + .iter() + .map(|handler| { + 1 + { + let ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler { + body, + .. + }) = handler; + num_branches(body) + } + }) + .sum::() + } + _ => 0, }) .sum() } @@ -205,8 +205,7 @@ else: else: pass "#; - - test_helper(source, 3)?; + test_helper(source, 4)?; Ok(()) } diff --git a/crates/ruff/src/rules/pylint/rules/too_many_statements.rs b/crates/ruff/src/rules/pylint/rules/too_many_statements.rs index b22bffdbd8..0832b55c83 100644 --- a/crates/ruff/src/rules/pylint/rules/too_many_statements.rs +++ b/crates/ruff/src/rules/pylint/rules/too_many_statements.rs @@ -66,16 +66,16 @@ fn num_statements(stmts: &[Stmt]) -> usize { let mut count = 0; for stmt in stmts { match stmt { - Stmt::If(ast::StmtIf { body, orelse, .. }) => { + Stmt::If(ast::StmtIf { + body, + elif_else_clauses, + .. + }) => { count += 1; count += num_statements(body); - if let Some(stmt) = orelse.first() { - // `elif:` and `else: if:` have the same AST representation. - // Avoid treating `elif:` as two statements. - if !stmt.is_if_stmt() { - count += 1; - } - count += num_statements(orelse); + for clause in elif_else_clauses { + count += 1; + count += num_statements(&clause.body); } } Stmt::For(ast::StmtFor { body, orelse, .. }) @@ -207,7 +207,7 @@ def f(): print() "#; let stmts = Suite::parse(source, "")?; - assert_eq!(num_statements(&stmts), 5); + assert_eq!(num_statements(&stmts), 6); Ok(()) } diff --git a/crates/ruff/src/rules/pylint/rules/useless_else_on_loop.rs b/crates/ruff/src/rules/pylint/rules/useless_else_on_loop.rs index 32fb1b8b59..2005541405 100644 --- a/crates/ruff/src/rules/pylint/rules/useless_else_on_loop.rs +++ b/crates/ruff/src/rules/pylint/rules/useless_else_on_loop.rs @@ -53,8 +53,15 @@ impl Violation for UselessElseOnLoop { fn loop_exits_early(body: &[Stmt]) -> bool { body.iter().any(|stmt| match stmt { - Stmt::If(ast::StmtIf { body, orelse, .. }) => { - loop_exits_early(body) || loop_exits_early(orelse) + Stmt::If(ast::StmtIf { + body, + elif_else_clauses, + .. + }) => { + loop_exits_early(body) + || elif_else_clauses + .iter() + .any(|clause| loop_exits_early(&clause.body)) } Stmt::With(ast::StmtWith { body, .. }) | Stmt::AsyncWith(ast::StmtAsyncWith { body, .. }) => loop_exits_early(body), diff --git a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR5501_collapsible_else_if.py.snap b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR5501_collapsible_else_if.py.snap index 426513185c..cfb1bb48fe 100644 --- a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR5501_collapsible_else_if.py.snap +++ b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR5501_collapsible_else_if.py.snap @@ -1,26 +1,39 @@ --- source: crates/ruff/src/rules/pylint/mod.rs --- -collapsible_else_if.py:38:9: PLR5501 Use `elif` instead of `else` then `if`, to reduce indentation +collapsible_else_if.py:37:5: PLR5501 Use `elif` instead of `else` then `if`, to reduce indentation | +35 | if 1: 36 | pass 37 | else: -38 | if 2: - | _________^ -39 | | pass - | |________________^ PLR5501 + | _____^ +38 | | if 2: + | |________^ PLR5501 +39 | pass | -collapsible_else_if.py:46:9: PLR5501 Use `elif` instead of `else` then `if`, to reduce indentation +collapsible_else_if.py:45:5: PLR5501 Use `elif` instead of `else` then `if`, to reduce indentation | +43 | if 1: 44 | pass 45 | else: -46 | if 2: - | _________^ -47 | | pass -48 | | else: -49 | | pass - | |________________^ PLR5501 + | _____^ +46 | | if 2: + | |________^ PLR5501 +47 | pass +48 | else: + | + +collapsible_else_if.py:58:5: PLR5501 Use `elif` instead of `else` then `if`, to reduce indentation + | +56 | elif True: +57 | print(2) +58 | else: + | _____^ +59 | | if True: + | |________^ PLR5501 +60 | print(3) +61 | else: | diff --git a/crates/ruff/src/rules/pyupgrade/rules/convert_named_tuple_functional_to_class.rs b/crates/ruff/src/rules/pyupgrade/rules/convert_named_tuple_functional_to_class.rs index 2d59a3df80..857871708f 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/convert_named_tuple_functional_to_class.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/convert_named_tuple_functional_to_class.rs @@ -170,6 +170,7 @@ fn create_class_def_stmt(typename: &str, body: Vec, base_class: &Expr) -> bases: vec![base_class.clone()], keywords: vec![], body, + type_params: vec![], decorator_list: vec![], range: TextRange::default(), } diff --git a/crates/ruff/src/rules/pyupgrade/rules/convert_typed_dict_functional_to_class.rs b/crates/ruff/src/rules/pyupgrade/rules/convert_typed_dict_functional_to_class.rs index afce256896..8e3b541ec9 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/convert_typed_dict_functional_to_class.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/convert_typed_dict_functional_to_class.rs @@ -127,6 +127,7 @@ fn create_class_def_stmt( bases: vec![base_class.clone()], keywords, body, + type_params: vec![], decorator_list: vec![], range: TextRange::default(), } diff --git a/crates/ruff/src/rules/pyupgrade/rules/outdated_version_block.rs b/crates/ruff/src/rules/pyupgrade/rules/outdated_version_block.rs index e84e0ab64b..cde8398e0e 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/outdated_version_block.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/outdated_version_block.rs @@ -1,13 +1,12 @@ use std::cmp::Ordering; use num_bigint::{BigInt, Sign}; -use ruff_text_size::{TextRange, TextSize}; -use rustpython_parser::ast::{self, CmpOp, Constant, Expr, Ranged, Stmt}; -use rustpython_parser::{lexer, Mode, Tok}; +use ruff_text_size::{TextLen, TextRange}; +use rustpython_parser::ast::{self, CmpOp, Constant, ElifElseClause, Expr, Ranged, StmtIf}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::source_code::Locator; +use ruff_python_ast::stmt_if::{if_elif_branches, BranchKind, IfElifBranch}; use ruff_python_ast::whitespace::indentation; use crate::autofix::edits::delete_stmt; @@ -61,98 +60,6 @@ impl AlwaysAutofixableViolation for OutdatedVersionBlock { } } -/// The metadata for a version-comparison block. -#[derive(Debug)] -struct BlockMetadata { - /// The first `if` or `elif` token in the block, used to signal the start of the - /// version-comparison block. - leading_token: StartToken, - /// The first `elif` or `else` token following the start token, if any, used to signal the end - /// of the version-comparison block. - trailing_token: Option, -} - -/// The set of tokens that can start a block, i.e., the first token in an `if` statement. -#[derive(Debug)] -enum StartTok { - If, - Elif, -} - -impl StartTok { - fn from_tok(tok: &Tok) -> Option { - match tok { - Tok::If => Some(Self::If), - Tok::Elif => Some(Self::Elif), - _ => None, - } - } -} - -#[derive(Debug)] -struct StartToken { - tok: StartTok, - range: TextRange, -} - -/// The set of tokens that can end a block, i.e., the first token in the subsequent `elif` or `else` -/// branch that follows an `if` or `elif` statement. -#[derive(Debug)] -enum EndTok { - Elif, - Else, -} - -impl EndTok { - fn from_tok(tok: &Tok) -> Option { - match tok { - Tok::Elif => Some(Self::Elif), - Tok::Else => Some(Self::Else), - _ => None, - } - } -} - -#[derive(Debug)] -struct EndToken { - tok: EndTok, - range: TextRange, -} - -fn metadata(locator: &Locator, located: &T, body: &[Stmt]) -> Option -where - T: Ranged, -{ - indentation(locator, located)?; - - let mut iter = lexer::lex_starts_at( - locator.slice(located.range()), - Mode::Module, - located.start(), - ) - .flatten(); - - // First the leading `if` or `elif` token. - let (tok, range) = iter.next()?; - let leading_token = StartToken { - tok: StartTok::from_tok(&tok)?, - range, - }; - - // Skip any tokens until we reach the end of the `if` body. - let body_end = body.last()?.range().end(); - - // Find the trailing `elif` or `else` token, if any. - let trailing_token = iter - .skip_while(|(_, range)| range.start() < body_end) - .find_map(|(tok, range)| EndTok::from_tok(&tok).map(|tok| EndToken { tok, range })); - - Some(BlockMetadata { - leading_token, - trailing_token, - }) -} - /// Converts a `BigInt` to a `u32`. If the number is negative, it will return 0. fn bigint_to_u32(number: &BigInt) -> u32 { let the_number = number.to_u32_digits(); @@ -207,96 +114,110 @@ fn compare_version(if_version: &[u32], py_version: PythonVersion, or_equal: bool } } -/// Convert a [`Stmt::If`], retaining the `else`. -fn fix_py2_block( - checker: &Checker, - stmt: &Stmt, - orelse: &[Stmt], - block: &BlockMetadata, -) -> Option { - let leading_token = &block.leading_token; - let Some(trailing_token) = &block.trailing_token else { - // Delete the entire statement. If this is an `elif`, know it's the only child - // of its parent, so avoid passing in the parent at all. Otherwise, - // `delete_stmt` will erroneously include a `pass`. - let stmt = checker.semantic().stmt(); - let parent = checker.semantic().stmt_parent(); - let edit = delete_stmt( - stmt, - if matches!(block.leading_token.tok, StartTok::If) { - parent - } else { - None - }, - checker.locator, - checker.indexer, - ); - return Some(Fix::suggested(edit)); - }; - - match (&leading_token.tok, &trailing_token.tok) { - // If we only have an `if` and an `else`, dedent the `else` block. - (StartTok::If, EndTok::Else) => { - let start = orelse.first()?; - let end = orelse.last()?; - if indentation(checker.locator, start).is_none() { - // Inline `else` block (e.g., `else: x = 1`). - Some(Fix::suggested(Edit::range_replacement( - checker - .locator - .slice(TextRange::new(start.start(), end.end())) - .to_string(), - stmt.range(), - ))) - } else { - indentation(checker.locator, stmt) - .and_then(|indentation| { - adjust_indentation( - TextRange::new(checker.locator.line_start(start.start()), end.end()), - indentation, - checker.locator, - checker.stylist, - ) - .ok() - }) - .map(|contents| { - Fix::suggested(Edit::replacement( - contents, - checker.locator.line_start(stmt.start()), - stmt.end(), - )) - }) +/// For fixing, we have 4 cases: +/// * Just an if: delete as statement (insert pass in parent if required) +/// * If with an elif: delete, turn elif into if +/// * If with an else: delete, dedent else +/// * Just an elif: delete, `elif False` can always be removed +fn fix_py2_block(checker: &Checker, stmt_if: &StmtIf, branch: &IfElifBranch) -> Option { + match branch.kind { + BranchKind::If => match stmt_if.elif_else_clauses.first() { + // If we have a lone `if`, delete as statement (insert pass in parent if required) + None => { + let stmt = checker.semantic().stmt(); + let parent = checker.semantic().stmt_parent(); + let edit = delete_stmt(stmt, parent, checker.locator, checker.indexer); + Some(Fix::suggested(edit)) } - } - (StartTok::If, EndTok::Elif) => { - // If we have an `if` and an `elif`, turn the `elif` into an `if`. - let start_location = leading_token.range.start(); - let end_location = trailing_token.range.start() + TextSize::from(2); - Some(Fix::suggested(Edit::deletion(start_location, end_location))) - } - (StartTok::Elif, _) => { - // If we have an `elif`, delete up to the `else` or the end of the statement. - let start_location = leading_token.range.start(); - let end_location = trailing_token.range.start(); - Some(Fix::suggested(Edit::deletion(start_location, end_location))) + // If we have an `if` and an `elif`, turn the `elif` into an `if` + Some(ElifElseClause { + test: Some(_), + range, + .. + }) => { + debug_assert!( + &checker.locator.contents()[TextRange::at(range.start(), "elif".text_len())] + == "elif" + ); + let end_location = range.start() + ("elif".text_len() - "if".text_len()); + Some(Fix::suggested(Edit::deletion( + stmt_if.start(), + end_location, + ))) + } + // If we only have an `if` and an `else`, dedent the `else` block + Some(ElifElseClause { + body, test: None, .. + }) => { + let start = body.first()?; + let end = body.last()?; + if indentation(checker.locator, start).is_none() { + // Inline `else` block (e.g., `else: x = 1`). + Some(Fix::suggested(Edit::range_replacement( + checker + .locator + .slice(TextRange::new(start.start(), end.end())) + .to_string(), + stmt_if.range(), + ))) + } else { + indentation(checker.locator, stmt_if) + .and_then(|indentation| { + adjust_indentation( + TextRange::new( + checker.locator.line_start(start.start()), + end.end(), + ), + indentation, + checker.locator, + checker.stylist, + ) + .ok() + }) + .map(|contents| { + Fix::suggested(Edit::replacement( + contents, + checker.locator.line_start(stmt_if.start()), + stmt_if.end(), + )) + }) + } + } + }, + BranchKind::Elif => { + // The range of the `ElifElseClause` ends in the line of the last statement. To avoid + // inserting an empty line between the end of `if` branch and the beginning `elif` or + // `else` branch after the deleted branch we find the next branch after the current, if + // any, and delete to its start. + // ```python + // if cond: + // x = 1 + // elif sys.version < (3.0): + // delete from here ... ^ x = 2 + // else: + // ... to here (exclusive) ^ x = 3 + // ``` + let next_start = stmt_if + .elif_else_clauses + .iter() + .map(Ranged::start) + .find(|start| *start > branch.range.start()); + Some(Fix::suggested(Edit::deletion( + branch.range.start(), + next_start.unwrap_or(branch.range.end()), + ))) } } } /// Convert a [`Stmt::If`], removing the `else` block. -fn fix_py3_block( - checker: &mut Checker, - stmt: &Stmt, - test: &Expr, - body: &[Stmt], - block: &BlockMetadata, -) -> Option { - match block.leading_token.tok { - StartTok::If => { - // If the first statement is an if, use the body of this statement, and ignore +fn fix_py3_block(checker: &mut Checker, stmt_if: &StmtIf, branch: &IfElifBranch) -> Option { + match branch.kind { + BranchKind::If => { + // If the first statement is an `if`, use the body of this statement, and ignore // the rest. - let start = body.first()?; - let end = body.last()?; + let start = branch.body.first()?; + let end = branch.body.last()?; if indentation(checker.locator, start).is_none() { // Inline `if` block (e.g., `if ...: x = 1`). Some(Fix::suggested(Edit::range_replacement( @@ -304,10 +225,10 @@ fn fix_py3_block( .locator .slice(TextRange::new(start.start(), end.end())) .to_string(), - stmt.range(), + stmt_if.range, ))) } else { - indentation(checker.locator, stmt) + indentation(checker.locator, &stmt_if) .and_then(|indentation| { adjust_indentation( TextRange::new(checker.locator.line_start(start.start()), end.end()), @@ -320,81 +241,76 @@ fn fix_py3_block( .map(|contents| { Fix::suggested(Edit::replacement( contents, - checker.locator.line_start(stmt.start()), - stmt.end(), + checker.locator.line_start(stmt_if.start()), + stmt_if.end(), )) }) } } - StartTok::Elif => { - // Replace the `elif` with an `else, preserve the body of the elif, and remove + BranchKind::Elif => { + // Replace the `elif` with an `else`, preserve the body of the elif, and remove // the rest. - let end = body.last()?; - let text = checker.locator.slice(TextRange::new(test.end(), end.end())); + let end = branch.body.last()?; + let text = checker + .locator + .slice(TextRange::new(branch.test.end(), end.end())); Some(Fix::suggested(Edit::range_replacement( format!("else{text}"), - stmt.range(), + TextRange::new(branch.range.start(), stmt_if.end()), ))) } } } /// UP036 -pub(crate) fn outdated_version_block( - checker: &mut Checker, - stmt: &Stmt, - test: &Expr, - body: &[Stmt], - orelse: &[Stmt], -) { - let Expr::Compare(ast::ExprCompare { - left, - ops, - comparators, - range: _, - }) = &test - else { - return; - }; +pub(crate) fn outdated_version_block(checker: &mut Checker, stmt_if: &StmtIf) { + for branch in if_elif_branches(stmt_if) { + let Expr::Compare(ast::ExprCompare { + left, + ops, + comparators, + range: _, + }) = &branch.test + else { + continue; + }; - if !checker - .semantic() - .resolve_call_path(left) - .map_or(false, |call_path| { - matches!(call_path.as_slice(), ["sys", "version_info"]) - }) - { - return; - } + let ([op], [comparison]) = (ops.as_slice(), comparators.as_slice()) else { + continue; + }; + + if !checker + .semantic() + .resolve_call_path(left) + .map_or(false, |call_path| { + matches!(call_path.as_slice(), ["sys", "version_info"]) + }) + { + continue; + } - if ops.len() == 1 && comparators.len() == 1 { - let comparison = &comparators[0]; - let op = &ops[0]; match comparison { Expr::Tuple(ast::ExprTuple { elts, .. }) => { let version = extract_version(elts); let target = checker.settings.target_version; if op == &CmpOp::Lt || op == &CmpOp::LtE { if compare_version(&version, target, op == &CmpOp::LtE) { - let mut diagnostic = Diagnostic::new(OutdatedVersionBlock, stmt.range()); + let mut diagnostic = + Diagnostic::new(OutdatedVersionBlock, branch.test.range()); if checker.patch(diagnostic.kind.rule()) { - if let Some(block) = metadata(checker.locator, stmt, body) { - if let Some(fix) = fix_py2_block(checker, stmt, orelse, &block) { - diagnostic.set_fix(fix); - } + if let Some(fix) = fix_py2_block(checker, stmt_if, &branch) { + diagnostic.set_fix(fix); } } checker.diagnostics.push(diagnostic); } } else if op == &CmpOp::Gt || op == &CmpOp::GtE { if compare_version(&version, target, op == &CmpOp::GtE) { - let mut diagnostic = Diagnostic::new(OutdatedVersionBlock, stmt.range()); + let mut diagnostic = + Diagnostic::new(OutdatedVersionBlock, branch.test.range()); if checker.patch(diagnostic.kind.rule()) { - if let Some(block) = metadata(checker.locator, stmt, body) { - if let Some(fix) = fix_py3_block(checker, stmt, test, body, &block) - { - diagnostic.set_fix(fix); - } + if let Some(fix) = fix_py3_block(checker, stmt_if, &branch) { + diagnostic.set_fix(fix); } } checker.diagnostics.push(diagnostic); @@ -407,22 +323,18 @@ pub(crate) fn outdated_version_block( }) => { let version_number = bigint_to_u32(number); if version_number == 2 && op == &CmpOp::Eq { - let mut diagnostic = Diagnostic::new(OutdatedVersionBlock, stmt.range()); + let mut diagnostic = Diagnostic::new(OutdatedVersionBlock, branch.test.range()); if checker.patch(diagnostic.kind.rule()) { - if let Some(block) = metadata(checker.locator, stmt, body) { - if let Some(fix) = fix_py2_block(checker, stmt, orelse, &block) { - diagnostic.set_fix(fix); - } + if let Some(fix) = fix_py2_block(checker, stmt_if, &branch) { + diagnostic.set_fix(fix); } } checker.diagnostics.push(diagnostic); } else if version_number == 3 && op == &CmpOp::Eq { - let mut diagnostic = Diagnostic::new(OutdatedVersionBlock, stmt.range()); + let mut diagnostic = Diagnostic::new(OutdatedVersionBlock, branch.test.range()); if checker.patch(diagnostic.kind.rule()) { - if let Some(block) = metadata(checker.locator, stmt, body) { - if let Some(fix) = fix_py3_block(checker, stmt, test, body, &block) { - diagnostic.set_fix(fix); - } + if let Some(fix) = fix_py3_block(checker, stmt_if, &branch) { + diagnostic.set_fix(fix); } } checker.diagnostics.push(diagnostic); diff --git a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP036_0.py.snap b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP036_0.py.snap index bb8998dbcc..4fccfbe7b3 100644 --- a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP036_0.py.snap +++ b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP036_0.py.snap @@ -1,17 +1,14 @@ --- source: crates/ruff/src/rules/pyupgrade/mod.rs --- -UP036_0.py:3:1: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:3:4: UP036 [*] Version block is outdated for minimum Python version | -1 | import sys -2 | -3 | / if sys.version_info < (3,0): -4 | | print("py2") -5 | | else: -6 | | print("py3") - | |________________^ UP036 -7 | -8 | if sys.version_info < (3,0): +1 | import sys +2 | +3 | if sys.version_info < (3,0): + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +4 | print("py2") +5 | else: | = help: Remove outdated version block @@ -27,20 +24,14 @@ UP036_0.py:3:1: UP036 [*] Version block is outdated for minimum Python version 8 5 | if sys.version_info < (3,0): 9 6 | if True: -UP036_0.py:8:1: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:8:4: UP036 [*] Version block is outdated for minimum Python version | - 6 | print("py3") - 7 | - 8 | / if sys.version_info < (3,0): - 9 | | if True: -10 | | print("py2!") -11 | | else: -12 | | print("???") -13 | | else: -14 | | print("py3") - | |________________^ UP036 -15 | -16 | if sys.version_info < (3,0): print("PY2!") + 6 | print("py3") + 7 | + 8 | if sys.version_info < (3,0): + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 + 9 | if True: +10 | print("py2!") | = help: Remove outdated version block @@ -60,15 +51,13 @@ UP036_0.py:8:1: UP036 [*] Version block is outdated for minimum Python version 16 10 | if sys.version_info < (3,0): print("PY2!") 17 11 | else: print("PY3!") -UP036_0.py:16:1: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:16:4: UP036 [*] Version block is outdated for minimum Python version | -14 | print("py3") -15 | -16 | / if sys.version_info < (3,0): print("PY2!") -17 | | else: print("PY3!") - | |___________________^ UP036 -18 | -19 | if True: +14 | print("py3") +15 | +16 | if sys.version_info < (3,0): print("PY2!") + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +17 | else: print("PY3!") | = help: Remove outdated version block @@ -83,17 +72,13 @@ UP036_0.py:16:1: UP036 [*] Version block is outdated for minimum Python version 19 18 | if True: 20 19 | if sys.version_info < (3,0): -UP036_0.py:20:5: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:20:8: UP036 [*] Version block is outdated for minimum Python version | -19 | if True: -20 | if sys.version_info < (3,0): - | _____^ -21 | | print("PY2") -22 | | else: -23 | | print("PY3") - | |____________________^ UP036 -24 | -25 | if sys.version_info < (3,0): print(1 if True else 3) +19 | if True: +20 | if sys.version_info < (3,0): + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +21 | print("PY2") +22 | else: | = help: Remove outdated version block @@ -110,16 +95,14 @@ UP036_0.py:20:5: UP036 [*] Version block is outdated for minimum Python version 25 22 | if sys.version_info < (3,0): print(1 if True else 3) 26 23 | else: -UP036_0.py:25:1: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:25:4: UP036 [*] Version block is outdated for minimum Python version | -23 | print("PY3") -24 | -25 | / if sys.version_info < (3,0): print(1 if True else 3) -26 | | else: -27 | | print("py3") - | |________________^ UP036 -28 | -29 | if sys.version_info < (3,0): +23 | print("PY3") +24 | +25 | if sys.version_info < (3,0): print(1 if True else 3) + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +26 | else: +27 | print("py3") | = help: Remove outdated version block @@ -135,20 +118,14 @@ UP036_0.py:25:1: UP036 [*] Version block is outdated for minimum Python version 29 27 | if sys.version_info < (3,0): 30 28 | def f(): -UP036_0.py:29:1: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:29:4: UP036 [*] Version block is outdated for minimum Python version | -27 | print("py3") -28 | -29 | / if sys.version_info < (3,0): -30 | | def f(): -31 | | print("py2") -32 | | else: -33 | | def f(): -34 | | print("py3") -35 | | print("This the next") - | |______________________________^ UP036 -36 | -37 | if sys.version_info > (3,0): +27 | print("py3") +28 | +29 | if sys.version_info < (3,0): + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +30 | def f(): +31 | print("py2") | = help: Remove outdated version block @@ -170,15 +147,14 @@ UP036_0.py:29:1: UP036 [*] Version block is outdated for minimum Python version 37 33 | if sys.version_info > (3,0): 38 34 | print("py3") -UP036_0.py:37:1: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:37:4: UP036 [*] Version block is outdated for minimum Python version | -35 | print("This the next") -36 | -37 | / if sys.version_info > (3,0): -38 | | print("py3") -39 | | else: -40 | | print("py2") - | |________________^ UP036 +35 | print("This the next") +36 | +37 | if sys.version_info > (3,0): + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +38 | print("py3") +39 | else: | = help: Remove outdated version block @@ -195,16 +171,14 @@ UP036_0.py:37:1: UP036 [*] Version block is outdated for minimum Python version 42 39 | 43 40 | x = 1 -UP036_0.py:45:1: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:45:4: UP036 [*] Version block is outdated for minimum Python version | -43 | x = 1 -44 | -45 | / if sys.version_info > (3,0): -46 | | print("py3") -47 | | else: -48 | | print("py2") - | |________________^ UP036 -49 | # ohai +43 | x = 1 +44 | +45 | if sys.version_info > (3,0): + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +46 | print("py3") +47 | else: | = help: Remove outdated version block @@ -221,15 +195,13 @@ UP036_0.py:45:1: UP036 [*] Version block is outdated for minimum Python version 50 47 | 51 48 | x = 1 -UP036_0.py:53:1: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:53:4: UP036 [*] Version block is outdated for minimum Python version | -51 | x = 1 -52 | -53 | / if sys.version_info > (3,0): print("py3") -54 | | else: print("py2") - | |__________________^ UP036 -55 | -56 | if sys.version_info > (3,): +51 | x = 1 +52 | +53 | if sys.version_info > (3,0): print("py3") + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +54 | else: print("py2") | = help: Remove outdated version block @@ -244,17 +216,14 @@ UP036_0.py:53:1: UP036 [*] Version block is outdated for minimum Python version 56 55 | if sys.version_info > (3,): 57 56 | print("py3") -UP036_0.py:56:1: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:56:4: UP036 [*] Version block is outdated for minimum Python version | -54 | else: print("py2") -55 | -56 | / if sys.version_info > (3,): -57 | | print("py3") -58 | | else: -59 | | print("py2") - | |________________^ UP036 -60 | -61 | if True: +54 | else: print("py2") +55 | +56 | if sys.version_info > (3,): + | ^^^^^^^^^^^^^^^^^^^^^^^ UP036 +57 | print("py3") +58 | else: | = help: Remove outdated version block @@ -271,17 +240,13 @@ UP036_0.py:56:1: UP036 [*] Version block is outdated for minimum Python version 61 58 | if True: 62 59 | if sys.version_info > (3,): -UP036_0.py:62:5: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:62:8: UP036 [*] Version block is outdated for minimum Python version | -61 | if True: -62 | if sys.version_info > (3,): - | _____^ -63 | | print("py3") -64 | | else: -65 | | print("py2") - | |____________________^ UP036 -66 | -67 | if sys.version_info < (3,): +61 | if True: +62 | if sys.version_info > (3,): + | ^^^^^^^^^^^^^^^^^^^^^^^ UP036 +63 | print("py3") +64 | else: | = help: Remove outdated version block @@ -298,17 +263,14 @@ UP036_0.py:62:5: UP036 [*] Version block is outdated for minimum Python version 67 64 | if sys.version_info < (3,): 68 65 | print("py2") -UP036_0.py:67:1: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:67:4: UP036 [*] Version block is outdated for minimum Python version | -65 | print("py2") -66 | -67 | / if sys.version_info < (3,): -68 | | print("py2") -69 | | else: -70 | | print("py3") - | |________________^ UP036 -71 | -72 | def f(): +65 | print("py2") +66 | +67 | if sys.version_info < (3,): + | ^^^^^^^^^^^^^^^^^^^^^^^ UP036 +68 | print("py2") +69 | else: | = help: Remove outdated version block @@ -325,18 +287,13 @@ UP036_0.py:67:1: UP036 [*] Version block is outdated for minimum Python version 72 69 | def f(): 73 70 | if sys.version_info < (3,0): -UP036_0.py:73:5: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:73:8: UP036 [*] Version block is outdated for minimum Python version | -72 | def f(): -73 | if sys.version_info < (3,0): - | _____^ -74 | | try: -75 | | yield -76 | | finally: -77 | | pass -78 | | else: -79 | | yield - | |_____________^ UP036 +72 | def f(): +73 | if sys.version_info < (3,0): + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +74 | try: +75 | yield | = help: Remove outdated version block @@ -356,20 +313,14 @@ UP036_0.py:73:5: UP036 [*] Version block is outdated for minimum Python version 81 75 | 82 76 | class C: -UP036_0.py:86:5: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:86:8: UP036 [*] Version block is outdated for minimum Python version | -84 | pass -85 | -86 | if sys.version_info < (3,0): - | _____^ -87 | | def f(py2): -88 | | pass -89 | | else: -90 | | def f(py3): -91 | | pass - | |________________^ UP036 -92 | -93 | def h(): +84 | pass +85 | +86 | if sys.version_info < (3,0): + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +87 | def f(py2): +88 | pass | = help: Remove outdated version block @@ -389,19 +340,15 @@ UP036_0.py:86:5: UP036 [*] Version block is outdated for minimum Python version 93 89 | def h(): 94 90 | pass -UP036_0.py:97:5: UP036 [*] Version block is outdated for minimum Python version - | - 96 | if True: - 97 | if sys.version_info < (3,0): - | _____^ - 98 | | 2 - 99 | | else: -100 | | 3 - | |_________^ UP036 -101 | -102 | # comment - | - = help: Remove outdated version block +UP036_0.py:97:8: UP036 [*] Version block is outdated for minimum Python version + | +96 | if True: +97 | if sys.version_info < (3,0): + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +98 | 2 +99 | else: + | + = help: Remove outdated version block β„Ή Suggested fix 94 94 | pass @@ -416,23 +363,14 @@ UP036_0.py:97:5: UP036 [*] Version block is outdated for minimum Python version 102 99 | # comment 103 100 | -UP036_0.py:104:1: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:104:4: UP036 [*] Version block is outdated for minimum Python version | -102 | # comment -103 | -104 | / if sys.version_info < (3,0): -105 | | def f(): -106 | | print("py2") -107 | | def g(): -108 | | print("py2") -109 | | else: -110 | | def f(): -111 | | print("py3") -112 | | def g(): -113 | | print("py3") - | |____________________^ UP036 -114 | -115 | if True: +102 | # comment +103 | +104 | if sys.version_info < (3,0): + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +105 | def f(): +106 | print("py2") | = help: Remove outdated version block @@ -458,15 +396,13 @@ UP036_0.py:104:1: UP036 [*] Version block is outdated for minimum Python version 115 109 | if True: 116 110 | if sys.version_info > (3,): -UP036_0.py:116:5: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:116:8: UP036 [*] Version block is outdated for minimum Python version | -115 | if True: -116 | if sys.version_info > (3,): - | _____^ -117 | | print(3) - | |________________^ UP036 -118 | # comment -119 | print(2+3) +115 | if True: +116 | if sys.version_info > (3,): + | ^^^^^^^^^^^^^^^^^^^^^^^ UP036 +117 | print(3) +118 | # comment | = help: Remove outdated version block @@ -481,11 +417,11 @@ UP036_0.py:116:5: UP036 [*] Version block is outdated for minimum Python version 119 118 | print(2+3) 120 119 | -UP036_0.py:122:5: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:122:8: UP036 [*] Version block is outdated for minimum Python version | 121 | if True: 122 | if sys.version_info > (3,): print(3) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 + | ^^^^^^^^^^^^^^^^^^^^^^^ UP036 123 | 124 | if True: | @@ -501,13 +437,12 @@ UP036_0.py:122:5: UP036 [*] Version block is outdated for minimum Python version 124 124 | if True: 125 125 | if sys.version_info > (3,): -UP036_0.py:125:5: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:125:8: UP036 [*] Version block is outdated for minimum Python version | -124 | if True: -125 | if sys.version_info > (3,): - | _____^ -126 | | print(3) - | |________________^ UP036 +124 | if True: +125 | if sys.version_info > (3,): + | ^^^^^^^^^^^^^^^^^^^^^^^ UP036 +126 | print(3) | = help: Remove outdated version block @@ -522,19 +457,13 @@ UP036_0.py:125:5: UP036 [*] Version block is outdated for minimum Python version 128 127 | 129 128 | if True: -UP036_0.py:130:5: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:130:8: UP036 [*] Version block is outdated for minimum Python version | -129 | if True: -130 | if sys.version_info <= (3, 0): - | _____^ -131 | | expected_error = [] -132 | | else: -133 | | expected_error = [ -134 | | ":1:5: Generator expression must be parenthesized", -135 | | "max(1 for i in range(10), key=lambda x: x+1)", -136 | | " ^", -137 | | ] - | |_________^ UP036 +129 | if True: +130 | if sys.version_info <= (3, 0): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +131 | expected_error = [] +132 | else: | = help: Remove outdated version block @@ -556,17 +485,12 @@ UP036_0.py:130:5: UP036 [*] Version block is outdated for minimum Python version 139 136 | 140 137 | if sys.version_info <= (3, 0): -UP036_0.py:140:1: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:140:4: UP036 [*] Version block is outdated for minimum Python version | -140 | / if sys.version_info <= (3, 0): -141 | | expected_error = [] -142 | | else: -143 | | expected_error = [ -144 | | ":1:5: Generator expression must be parenthesized", -145 | | "max(1 for i in range(10), key=lambda x: x+1)", -146 | | " ^", -147 | | ] - | |_____^ UP036 +140 | if sys.version_info <= (3, 0): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +141 | expected_error = [] +142 | else: | = help: Remove outdated version block @@ -588,23 +512,12 @@ UP036_0.py:140:1: UP036 [*] Version block is outdated for minimum Python version 149 146 | 150 147 | if sys.version_info > (3,0): -UP036_0.py:150:1: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:150:4: UP036 [*] Version block is outdated for minimum Python version | -150 | / if sys.version_info > (3,0): -151 | | """this -152 | | is valid""" -153 | | -154 | | """the indentation on -155 | | this line is significant""" -156 | | -157 | | "this is" \ -158 | | "allowed too" -159 | | -160 | | ("so is" -161 | | "this for some reason") - | |____________________________^ UP036 -162 | -163 | if sys.version_info > (3, 0): expected_error = \ +150 | if sys.version_info > (3,0): + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +151 | """this +152 | is valid""" | = help: Remove outdated version block @@ -633,15 +546,13 @@ UP036_0.py:150:1: UP036 [*] Version block is outdated for minimum Python version 163 162 | if sys.version_info > (3, 0): expected_error = \ 164 163 | [] -UP036_0.py:163:1: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:163:4: UP036 [*] Version block is outdated for minimum Python version | -161 | "this for some reason") -162 | -163 | / if sys.version_info > (3, 0): expected_error = \ -164 | | [] - | |______^ UP036 -165 | -166 | if sys.version_info > (3, 0): expected_error = [] +161 | "this for some reason") +162 | +163 | if sys.version_info > (3, 0): expected_error = \ + | ^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +164 | [] | = help: Remove outdated version block @@ -655,12 +566,12 @@ UP036_0.py:163:1: UP036 [*] Version block is outdated for minimum Python version 165 165 | 166 166 | if sys.version_info > (3, 0): expected_error = [] -UP036_0.py:166:1: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:166:4: UP036 [*] Version block is outdated for minimum Python version | 164 | [] 165 | 166 | if sys.version_info > (3, 0): expected_error = [] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 + | ^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 167 | 168 | if sys.version_info > (3, 0): \ | @@ -676,15 +587,13 @@ UP036_0.py:166:1: UP036 [*] Version block is outdated for minimum Python version 168 168 | if sys.version_info > (3, 0): \ 169 169 | expected_error = [] -UP036_0.py:168:1: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:168:4: UP036 [*] Version block is outdated for minimum Python version | -166 | if sys.version_info > (3, 0): expected_error = [] -167 | -168 | / if sys.version_info > (3, 0): \ -169 | | expected_error = [] - | |_______________________^ UP036 -170 | -171 | if True: +166 | if sys.version_info > (3, 0): expected_error = [] +167 | +168 | if sys.version_info > (3, 0): \ + | ^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +169 | expected_error = [] | = help: Remove outdated version block @@ -699,15 +608,12 @@ UP036_0.py:168:1: UP036 [*] Version block is outdated for minimum Python version 171 170 | if True: 172 171 | if sys.version_info > (3, 0): expected_error = \ -UP036_0.py:172:5: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:172:8: UP036 [*] Version block is outdated for minimum Python version | -171 | if True: -172 | if sys.version_info > (3, 0): expected_error = \ - | _____^ -173 | | [] - | |______^ UP036 -174 | -175 | if True: +171 | if True: +172 | if sys.version_info > (3, 0): expected_error = \ + | ^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +173 | [] | = help: Remove outdated version block @@ -721,11 +627,11 @@ UP036_0.py:172:5: UP036 [*] Version block is outdated for minimum Python version 174 174 | 175 175 | if True: -UP036_0.py:176:5: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:176:8: UP036 [*] Version block is outdated for minimum Python version | 175 | if True: 176 | if sys.version_info > (3, 0): expected_error = [] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 + | ^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 177 | 178 | if True: | @@ -741,13 +647,12 @@ UP036_0.py:176:5: UP036 [*] Version block is outdated for minimum Python version 178 178 | if True: 179 179 | if sys.version_info > (3, 0): \ -UP036_0.py:179:5: UP036 [*] Version block is outdated for minimum Python version +UP036_0.py:179:8: UP036 [*] Version block is outdated for minimum Python version | -178 | if True: -179 | if sys.version_info > (3, 0): \ - | _____^ -180 | | expected_error = [] - | |_______________________^ UP036 +178 | if True: +179 | if sys.version_info > (3, 0): \ + | ^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +180 | expected_error = [] | = help: Remove outdated version block diff --git a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP036_1.py.snap b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP036_1.py.snap index e64022388a..5d385f538d 100644 --- a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP036_1.py.snap +++ b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP036_1.py.snap @@ -1,17 +1,14 @@ --- source: crates/ruff/src/rules/pyupgrade/mod.rs --- -UP036_1.py:3:1: UP036 [*] Version block is outdated for minimum Python version +UP036_1.py:3:4: UP036 [*] Version block is outdated for minimum Python version | -1 | import sys -2 | -3 | / if sys.version_info == 2: -4 | | 2 -5 | | else: -6 | | 3 - | |_____^ UP036 -7 | -8 | if sys.version_info < (3,): +1 | import sys +2 | +3 | if sys.version_info == 2: + | ^^^^^^^^^^^^^^^^^^^^^ UP036 +4 | 2 +5 | else: | = help: Remove outdated version block @@ -27,17 +24,14 @@ UP036_1.py:3:1: UP036 [*] Version block is outdated for minimum Python version 8 5 | if sys.version_info < (3,): 9 6 | 2 -UP036_1.py:8:1: UP036 [*] Version block is outdated for minimum Python version +UP036_1.py:8:4: UP036 [*] Version block is outdated for minimum Python version | - 6 | 3 - 7 | - 8 | / if sys.version_info < (3,): - 9 | | 2 -10 | | else: -11 | | 3 - | |_____^ UP036 -12 | -13 | if sys.version_info < (3,0): + 6 | 3 + 7 | + 8 | if sys.version_info < (3,): + | ^^^^^^^^^^^^^^^^^^^^^^^ UP036 + 9 | 2 +10 | else: | = help: Remove outdated version block @@ -54,17 +48,14 @@ UP036_1.py:8:1: UP036 [*] Version block is outdated for minimum Python version 13 10 | if sys.version_info < (3,0): 14 11 | 2 -UP036_1.py:13:1: UP036 [*] Version block is outdated for minimum Python version +UP036_1.py:13:4: UP036 [*] Version block is outdated for minimum Python version | -11 | 3 -12 | -13 | / if sys.version_info < (3,0): -14 | | 2 -15 | | else: -16 | | 3 - | |_____^ UP036 -17 | -18 | if sys.version_info == 3: +11 | 3 +12 | +13 | if sys.version_info < (3,0): + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +14 | 2 +15 | else: | = help: Remove outdated version block @@ -81,17 +72,14 @@ UP036_1.py:13:1: UP036 [*] Version block is outdated for minimum Python version 18 15 | if sys.version_info == 3: 19 16 | 3 -UP036_1.py:18:1: UP036 [*] Version block is outdated for minimum Python version +UP036_1.py:18:4: UP036 [*] Version block is outdated for minimum Python version | -16 | 3 -17 | -18 | / if sys.version_info == 3: -19 | | 3 -20 | | else: -21 | | 2 - | |_____^ UP036 -22 | -23 | if sys.version_info > (3,): +16 | 3 +17 | +18 | if sys.version_info == 3: + | ^^^^^^^^^^^^^^^^^^^^^ UP036 +19 | 3 +20 | else: | = help: Remove outdated version block @@ -108,17 +96,14 @@ UP036_1.py:18:1: UP036 [*] Version block is outdated for minimum Python version 23 20 | if sys.version_info > (3,): 24 21 | 3 -UP036_1.py:23:1: UP036 [*] Version block is outdated for minimum Python version +UP036_1.py:23:4: UP036 [*] Version block is outdated for minimum Python version | -21 | 2 -22 | -23 | / if sys.version_info > (3,): -24 | | 3 -25 | | else: -26 | | 2 - | |_____^ UP036 -27 | -28 | if sys.version_info >= (3,): +21 | 2 +22 | +23 | if sys.version_info > (3,): + | ^^^^^^^^^^^^^^^^^^^^^^^ UP036 +24 | 3 +25 | else: | = help: Remove outdated version block @@ -135,17 +120,14 @@ UP036_1.py:23:1: UP036 [*] Version block is outdated for minimum Python version 28 25 | if sys.version_info >= (3,): 29 26 | 3 -UP036_1.py:28:1: UP036 [*] Version block is outdated for minimum Python version +UP036_1.py:28:4: UP036 [*] Version block is outdated for minimum Python version | -26 | 2 -27 | -28 | / if sys.version_info >= (3,): -29 | | 3 -30 | | else: -31 | | 2 - | |_____^ UP036 -32 | -33 | from sys import version_info +26 | 2 +27 | +28 | if sys.version_info >= (3,): + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +29 | 3 +30 | else: | = help: Remove outdated version block @@ -162,17 +144,14 @@ UP036_1.py:28:1: UP036 [*] Version block is outdated for minimum Python version 33 30 | from sys import version_info 34 31 | -UP036_1.py:35:1: UP036 [*] Version block is outdated for minimum Python version +UP036_1.py:35:4: UP036 [*] Version block is outdated for minimum Python version | -33 | from sys import version_info -34 | -35 | / if version_info > (3,): -36 | | 3 -37 | | else: -38 | | 2 - | |_____^ UP036 -39 | -40 | if True: +33 | from sys import version_info +34 | +35 | if version_info > (3,): + | ^^^^^^^^^^^^^^^^^^^ UP036 +36 | 3 +37 | else: | = help: Remove outdated version block @@ -189,17 +168,14 @@ UP036_1.py:35:1: UP036 [*] Version block is outdated for minimum Python version 40 37 | if True: 41 38 | print(1) -UP036_1.py:42:1: UP036 [*] Version block is outdated for minimum Python version +UP036_1.py:42:6: UP036 [*] Version block is outdated for minimum Python version | -40 | if True: -41 | print(1) -42 | / elif sys.version_info < (3,0): -43 | | print(2) -44 | | else: -45 | | print(3) - | |____________^ UP036 -46 | -47 | if True: +40 | if True: +41 | print(1) +42 | elif sys.version_info < (3,0): + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +43 | print(2) +44 | else: | = help: Remove outdated version block @@ -213,17 +189,14 @@ UP036_1.py:42:1: UP036 [*] Version block is outdated for minimum Python version 45 43 | print(3) 46 44 | -UP036_1.py:49:1: UP036 [*] Version block is outdated for minimum Python version +UP036_1.py:49:6: UP036 [*] Version block is outdated for minimum Python version | -47 | if True: -48 | print(1) -49 | / elif sys.version_info > (3,): -50 | | print(3) -51 | | else: -52 | | print(2) - | |____________^ UP036 -53 | -54 | if True: +47 | if True: +48 | print(1) +49 | elif sys.version_info > (3,): + | ^^^^^^^^^^^^^^^^^^^^^^^ UP036 +50 | print(3) +51 | else: | = help: Remove outdated version block @@ -240,15 +213,13 @@ UP036_1.py:49:1: UP036 [*] Version block is outdated for minimum Python version 54 52 | if True: 55 53 | print(1) -UP036_1.py:56:1: UP036 [*] Version block is outdated for minimum Python version +UP036_1.py:56:6: UP036 [*] Version block is outdated for minimum Python version | -54 | if True: -55 | print(1) -56 | / elif sys.version_info > (3,): -57 | | print(3) - | |____________^ UP036 -58 | -59 | def f(): +54 | if True: +55 | print(1) +56 | elif sys.version_info > (3,): + | ^^^^^^^^^^^^^^^^^^^^^^^ UP036 +57 | print(3) | = help: Remove outdated version block @@ -262,16 +233,13 @@ UP036_1.py:56:1: UP036 [*] Version block is outdated for minimum Python version 58 58 | 59 59 | def f(): -UP036_1.py:62:5: UP036 [*] Version block is outdated for minimum Python version +UP036_1.py:62:10: UP036 [*] Version block is outdated for minimum Python version | -60 | if True: -61 | print(1) -62 | elif sys.version_info > (3,): - | _____^ -63 | | print(3) - | |________________^ UP036 -64 | -65 | if True: +60 | if True: +61 | print(1) +62 | elif sys.version_info > (3,): + | ^^^^^^^^^^^^^^^^^^^^^^^ UP036 +63 | print(3) | = help: Remove outdated version block @@ -285,17 +253,14 @@ UP036_1.py:62:5: UP036 [*] Version block is outdated for minimum Python version 64 64 | 65 65 | if True: -UP036_1.py:67:1: UP036 [*] Version block is outdated for minimum Python version +UP036_1.py:67:6: UP036 [*] Version block is outdated for minimum Python version | -65 | if True: -66 | print(1) -67 | / elif sys.version_info < (3,0): -68 | | print(2) -69 | | else: -70 | | print(3) - | |____________^ UP036 -71 | -72 | def f(): +65 | if True: +66 | print(1) +67 | elif sys.version_info < (3,0): + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +68 | print(2) +69 | else: | = help: Remove outdated version block @@ -309,14 +274,13 @@ UP036_1.py:67:1: UP036 [*] Version block is outdated for minimum Python version 70 68 | print(3) 71 69 | -UP036_1.py:75:5: UP036 [*] Version block is outdated for minimum Python version +UP036_1.py:75:10: UP036 [*] Version block is outdated for minimum Python version | -73 | if True: -74 | print(1) -75 | elif sys.version_info > (3,): - | _____^ -76 | | print(3) - | |________________^ UP036 +73 | if True: +74 | print(1) +75 | elif sys.version_info > (3,): + | ^^^^^^^^^^^^^^^^^^^^^^^ UP036 +76 | print(3) | = help: Remove outdated version block diff --git a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP036_2.py.snap b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP036_2.py.snap index 773502a44f..9ca87c972a 100644 --- a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP036_2.py.snap +++ b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP036_2.py.snap @@ -1,17 +1,14 @@ --- source: crates/ruff/src/rules/pyupgrade/mod.rs --- -UP036_2.py:4:1: UP036 [*] Version block is outdated for minimum Python version +UP036_2.py:4:4: UP036 [*] Version block is outdated for minimum Python version | -2 | from sys import version_info -3 | -4 | / if sys.version_info > (3, 5): -5 | | 3+6 -6 | | else: -7 | | 3-5 - | |_______^ UP036 -8 | -9 | if version_info > (3, 5): +2 | from sys import version_info +3 | +4 | if sys.version_info > (3, 5): + | ^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +5 | 3+6 +6 | else: | = help: Remove outdated version block @@ -28,17 +25,14 @@ UP036_2.py:4:1: UP036 [*] Version block is outdated for minimum Python version 9 6 | if version_info > (3, 5): 10 7 | 3+6 -UP036_2.py:9:1: UP036 [*] Version block is outdated for minimum Python version +UP036_2.py:9:4: UP036 [*] Version block is outdated for minimum Python version | - 7 | 3-5 - 8 | - 9 | / if version_info > (3, 5): -10 | | 3+6 -11 | | else: -12 | | 3-5 - | |_______^ UP036 -13 | -14 | if sys.version_info >= (3,6): + 7 | 3-5 + 8 | + 9 | if version_info > (3, 5): + | ^^^^^^^^^^^^^^^^^^^^^ UP036 +10 | 3+6 +11 | else: | = help: Remove outdated version block @@ -55,17 +49,14 @@ UP036_2.py:9:1: UP036 [*] Version block is outdated for minimum Python version 14 11 | if sys.version_info >= (3,6): 15 12 | 3+6 -UP036_2.py:14:1: UP036 [*] Version block is outdated for minimum Python version +UP036_2.py:14:4: UP036 [*] Version block is outdated for minimum Python version | -12 | 3-5 -13 | -14 | / if sys.version_info >= (3,6): -15 | | 3+6 -16 | | else: -17 | | 3-5 - | |_______^ UP036 -18 | -19 | if version_info >= (3,6): +12 | 3-5 +13 | +14 | if sys.version_info >= (3,6): + | ^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +15 | 3+6 +16 | else: | = help: Remove outdated version block @@ -82,17 +73,14 @@ UP036_2.py:14:1: UP036 [*] Version block is outdated for minimum Python version 19 16 | if version_info >= (3,6): 20 17 | 3+6 -UP036_2.py:19:1: UP036 [*] Version block is outdated for minimum Python version +UP036_2.py:19:4: UP036 [*] Version block is outdated for minimum Python version | -17 | 3-5 -18 | -19 | / if version_info >= (3,6): -20 | | 3+6 -21 | | else: -22 | | 3-5 - | |_______^ UP036 -23 | -24 | if sys.version_info < (3,6): +17 | 3-5 +18 | +19 | if version_info >= (3,6): + | ^^^^^^^^^^^^^^^^^^^^^ UP036 +20 | 3+6 +21 | else: | = help: Remove outdated version block @@ -109,17 +97,14 @@ UP036_2.py:19:1: UP036 [*] Version block is outdated for minimum Python version 24 21 | if sys.version_info < (3,6): 25 22 | 3-5 -UP036_2.py:24:1: UP036 [*] Version block is outdated for minimum Python version +UP036_2.py:24:4: UP036 [*] Version block is outdated for minimum Python version | -22 | 3-5 -23 | -24 | / if sys.version_info < (3,6): -25 | | 3-5 -26 | | else: -27 | | 3+6 - | |_______^ UP036 -28 | -29 | if sys.version_info <= (3,5): +22 | 3-5 +23 | +24 | if sys.version_info < (3,6): + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +25 | 3-5 +26 | else: | = help: Remove outdated version block @@ -136,17 +121,14 @@ UP036_2.py:24:1: UP036 [*] Version block is outdated for minimum Python version 29 26 | if sys.version_info <= (3,5): 30 27 | 3-5 -UP036_2.py:29:1: UP036 [*] Version block is outdated for minimum Python version +UP036_2.py:29:4: UP036 [*] Version block is outdated for minimum Python version | -27 | 3+6 -28 | -29 | / if sys.version_info <= (3,5): -30 | | 3-5 -31 | | else: -32 | | 3+6 - | |_______^ UP036 -33 | -34 | if sys.version_info <= (3, 5): +27 | 3+6 +28 | +29 | if sys.version_info <= (3,5): + | ^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +30 | 3-5 +31 | else: | = help: Remove outdated version block @@ -163,17 +145,14 @@ UP036_2.py:29:1: UP036 [*] Version block is outdated for minimum Python version 34 31 | if sys.version_info <= (3, 5): 35 32 | 3-5 -UP036_2.py:34:1: UP036 [*] Version block is outdated for minimum Python version +UP036_2.py:34:4: UP036 [*] Version block is outdated for minimum Python version | -32 | 3+6 -33 | -34 | / if sys.version_info <= (3, 5): -35 | | 3-5 -36 | | else: -37 | | 3+6 - | |_______^ UP036 -38 | -39 | if sys.version_info >= (3, 5): +32 | 3+6 +33 | +34 | if sys.version_info <= (3, 5): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +35 | 3-5 +36 | else: | = help: Remove outdated version block @@ -190,15 +169,13 @@ UP036_2.py:34:1: UP036 [*] Version block is outdated for minimum Python version 39 36 | if sys.version_info >= (3, 5): 40 37 | pass -UP036_2.py:39:1: UP036 [*] Version block is outdated for minimum Python version +UP036_2.py:39:4: UP036 [*] Version block is outdated for minimum Python version | -37 | 3+6 -38 | -39 | / if sys.version_info >= (3, 5): -40 | | pass - | |________^ UP036 -41 | -42 | if sys.version_info < (3,0): +37 | 3+6 +38 | +39 | if sys.version_info >= (3, 5): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +40 | pass | = help: Remove outdated version block @@ -213,15 +190,13 @@ UP036_2.py:39:1: UP036 [*] Version block is outdated for minimum Python version 42 41 | if sys.version_info < (3,0): 43 42 | pass -UP036_2.py:42:1: UP036 [*] Version block is outdated for minimum Python version +UP036_2.py:42:4: UP036 [*] Version block is outdated for minimum Python version | -40 | pass -41 | -42 | / if sys.version_info < (3,0): -43 | | pass - | |________^ UP036 -44 | -45 | if True: +40 | pass +41 | +42 | if sys.version_info < (3,0): + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +43 | pass | = help: Remove outdated version block @@ -235,15 +210,12 @@ UP036_2.py:42:1: UP036 [*] Version block is outdated for minimum Python version 45 43 | if True: 46 44 | if sys.version_info < (3,0): -UP036_2.py:46:5: UP036 [*] Version block is outdated for minimum Python version +UP036_2.py:46:8: UP036 [*] Version block is outdated for minimum Python version | -45 | if True: -46 | if sys.version_info < (3,0): - | _____^ -47 | | pass - | |____________^ UP036 -48 | -49 | if sys.version_info < (3,0): +45 | if True: +46 | if sys.version_info < (3,0): + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +47 | pass | = help: Remove outdated version block @@ -258,17 +230,14 @@ UP036_2.py:46:5: UP036 [*] Version block is outdated for minimum Python version 49 48 | if sys.version_info < (3,0): 50 49 | pass -UP036_2.py:49:1: UP036 [*] Version block is outdated for minimum Python version +UP036_2.py:49:4: UP036 [*] Version block is outdated for minimum Python version | -47 | pass -48 | -49 | / if sys.version_info < (3,0): -50 | | pass -51 | | elif False: -52 | | pass - | |________^ UP036 -53 | -54 | if sys.version_info > (3,): +47 | pass +48 | +49 | if sys.version_info < (3,0): + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +50 | pass +51 | elif False: | = help: Remove outdated version block @@ -284,17 +253,14 @@ UP036_2.py:49:1: UP036 [*] Version block is outdated for minimum Python version 53 51 | 54 52 | if sys.version_info > (3,): -UP036_2.py:54:1: UP036 [*] Version block is outdated for minimum Python version +UP036_2.py:54:4: UP036 [*] Version block is outdated for minimum Python version | -52 | pass -53 | -54 | / if sys.version_info > (3,): -55 | | pass -56 | | elif False: -57 | | pass - | |________^ UP036 -58 | -59 | if sys.version_info[0] > "2": +52 | pass +53 | +54 | if sys.version_info > (3,): + | ^^^^^^^^^^^^^^^^^^^^^^^ UP036 +55 | pass +56 | elif False: | = help: Remove outdated version block diff --git a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP036_3.py.snap b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP036_3.py.snap index f7365f6174..d82d1aba25 100644 --- a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP036_3.py.snap +++ b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP036_3.py.snap @@ -1,23 +1,16 @@ --- source: crates/ruff/src/rules/pyupgrade/mod.rs --- -UP036_3.py:3:1: UP036 [*] Version block is outdated for minimum Python version - | - 1 | import sys - 2 | - 3 | / if sys.version_info < (3,0): - 4 | | print("py2") - 5 | | for item in range(10): - 6 | | print(f"PY2-{item}") - 7 | | else : - 8 | | print("py3") - 9 | | for item in range(10): -10 | | print(f"PY3-{item}") - | |____________________________^ UP036 -11 | -12 | if False: - | - = help: Remove outdated version block +UP036_3.py:3:15: UP036 [*] Version block is outdated for minimum Python version + | +1 | import sys +2 | +3 | if sys.version_info < (3,0): + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +4 | print("py2") +5 | for item in range(10): + | + = help: Remove outdated version block β„Ή Suggested fix 1 1 | import sys @@ -37,19 +30,13 @@ UP036_3.py:3:1: UP036 [*] Version block is outdated for minimum Python version 12 7 | if False: 13 8 | if sys.version_info < (3,0): -UP036_3.py:13:5: UP036 [*] Version block is outdated for minimum Python version +UP036_3.py:13:19: UP036 [*] Version block is outdated for minimum Python version | -12 | if False: -13 | if sys.version_info < (3,0): - | _____^ -14 | | print("py2") -15 | | for item in range(10): -16 | | print(f"PY2-{item}") -17 | | else : -18 | | print("py3") -19 | | for item in range(10): -20 | | print(f"PY3-{item}") - | |________________________________^ UP036 +12 | if False: +13 | if sys.version_info < (3,0): + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +14 | print("py2") +15 | for item in range(10): | = help: Remove outdated version block @@ -72,11 +59,11 @@ UP036_3.py:13:5: UP036 [*] Version block is outdated for minimum Python version 22 17 | 23 18 | if sys.version_info < (3,0): print("PY2!") -UP036_3.py:23:1: UP036 [*] Version block is outdated for minimum Python version +UP036_3.py:23:15: UP036 [*] Version block is outdated for minimum Python version | -23 | / if sys.version_info < (3,0): print("PY2!") -24 | | else : print("PY3!") - | |__________________________________________________^ UP036 +23 | if sys.version_info < (3,0): print("PY2!") + | ^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +24 | else : print("PY3!") | = help: Remove outdated version block diff --git a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP036_4.py.snap b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP036_4.py.snap index 9ddd273855..a0bacbe7af 100644 --- a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP036_4.py.snap +++ b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP036_4.py.snap @@ -1,13 +1,12 @@ --- source: crates/ruff/src/rules/pyupgrade/mod.rs --- -UP036_4.py:4:5: UP036 [*] Version block is outdated for minimum Python version +UP036_4.py:4:8: UP036 [*] Version block is outdated for minimum Python version | -3 | if True: -4 | if sys.version_info < (3, 3): - | _____^ -5 | | cmd = [sys.executable, "-m", "test.regrtest"] - | |_____________________________________________________^ UP036 +3 | if True: +4 | if sys.version_info < (3, 3): + | ^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +5 | cmd = [sys.executable, "-m", "test.regrtest"] | = help: Remove outdated version block @@ -22,87 +21,76 @@ UP036_4.py:4:5: UP036 [*] Version block is outdated for minimum Python version 7 6 | 8 7 | if True: -UP036_4.py:11:5: UP036 [*] Version block is outdated for minimum Python version +UP036_4.py:11:10: UP036 [*] Version block is outdated for minimum Python version | - 9 | if foo: -10 | pass -11 | elif sys.version_info < (3, 3): - | _____^ -12 | | cmd = [sys.executable, "-m", "test.regrtest"] - | |_____________________________________________________^ UP036 -13 | -14 | if True: + 9 | if foo: +10 | print() +11 | elif sys.version_info < (3, 3): + | ^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +12 | cmd = [sys.executable, "-m", "test.regrtest"] | = help: Remove outdated version block β„Ή Suggested fix 8 8 | if True: 9 9 | if foo: -10 10 | pass +10 10 | print() 11 |- elif sys.version_info < (3, 3): 12 |- cmd = [sys.executable, "-m", "test.regrtest"] -13 11 | -14 12 | if True: -15 13 | if foo: + 11 |+ +13 12 | +14 13 | if True: +15 14 | if foo: -UP036_4.py:17:5: UP036 [*] Version block is outdated for minimum Python version +UP036_4.py:17:10: UP036 [*] Version block is outdated for minimum Python version | -15 | if foo: -16 | pass -17 | elif sys.version_info < (3, 3): - | _____^ -18 | | cmd = [sys.executable, "-m", "test.regrtest"] -19 | | elif foo: -20 | | cmd = [sys.executable, "-m", "test", "-j0"] - | |___________________________________________________^ UP036 -21 | -22 | if foo: +15 | if foo: +16 | print() +17 | elif sys.version_info < (3, 3): + | ^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +18 | cmd = [sys.executable, "-m", "test.regrtest"] +19 | elif foo: | = help: Remove outdated version block β„Ή Suggested fix 14 14 | if True: 15 15 | if foo: -16 16 | pass +16 16 | print() 17 |- elif sys.version_info < (3, 3): 18 |- cmd = [sys.executable, "-m", "test.regrtest"] 19 17 | elif foo: 20 18 | cmd = [sys.executable, "-m", "test", "-j0"] 21 19 | -UP036_4.py:24:5: UP036 [*] Version block is outdated for minimum Python version +UP036_4.py:24:10: UP036 [*] Version block is outdated for minimum Python version | -22 | if foo: -23 | pass -24 | elif sys.version_info < (3, 3): - | _____^ -25 | | cmd = [sys.executable, "-m", "test.regrtest"] - | |_____________________________________________________^ UP036 -26 | -27 | if sys.version_info < (3, 3): +22 | if foo: +23 | print() +24 | elif sys.version_info < (3, 3): + | ^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +25 | cmd = [sys.executable, "-m", "test.regrtest"] | = help: Remove outdated version block β„Ή Suggested fix 21 21 | 22 22 | if foo: -23 23 | pass +23 23 | print() 24 |- elif sys.version_info < (3, 3): 25 |- cmd = [sys.executable, "-m", "test.regrtest"] -26 24 | -27 25 | if sys.version_info < (3, 3): -28 26 | cmd = [sys.executable, "-m", "test.regrtest"] + 24 |+ +26 25 | +27 26 | if sys.version_info < (3, 3): +28 27 | cmd = [sys.executable, "-m", "test.regrtest"] -UP036_4.py:27:5: UP036 [*] Version block is outdated for minimum Python version +UP036_4.py:27:8: UP036 [*] Version block is outdated for minimum Python version | -25 | cmd = [sys.executable, "-m", "test.regrtest"] -26 | -27 | if sys.version_info < (3, 3): - | _____^ -28 | | cmd = [sys.executable, "-m", "test.regrtest"] - | |_____________________________________________________^ UP036 -29 | -30 | if foo: +25 | cmd = [sys.executable, "-m", "test.regrtest"] +26 | +27 | if sys.version_info < (3, 3): + | ^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +28 | cmd = [sys.executable, "-m", "test.regrtest"] | = help: Remove outdated version block @@ -114,45 +102,37 @@ UP036_4.py:27:5: UP036 [*] Version block is outdated for minimum Python version 28 |- cmd = [sys.executable, "-m", "test.regrtest"] 29 27 | 30 28 | if foo: -31 29 | pass +31 29 | print() -UP036_4.py:32:5: UP036 [*] Version block is outdated for minimum Python version +UP036_4.py:32:10: UP036 [*] Version block is outdated for minimum Python version | -30 | if foo: -31 | pass -32 | elif sys.version_info < (3, 3): - | _____^ -33 | | cmd = [sys.executable, "-m", "test.regrtest"] -34 | | else: -35 | | cmd = [sys.executable, "-m", "test", "-j0"] - | |___________________________________________________^ UP036 -36 | -37 | if sys.version_info < (3, 3): +30 | if foo: +31 | print() +32 | elif sys.version_info < (3, 3): + | ^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +33 | cmd = [sys.executable, "-m", "test.regrtest"] +34 | else: | = help: Remove outdated version block β„Ή Suggested fix 29 29 | 30 30 | if foo: -31 31 | pass +31 31 | print() 32 |- elif sys.version_info < (3, 3): 33 |- cmd = [sys.executable, "-m", "test.regrtest"] 34 32 | else: 35 33 | cmd = [sys.executable, "-m", "test", "-j0"] 36 34 | -UP036_4.py:37:5: UP036 [*] Version block is outdated for minimum Python version +UP036_4.py:37:8: UP036 [*] Version block is outdated for minimum Python version | -35 | cmd = [sys.executable, "-m", "test", "-j0"] -36 | -37 | if sys.version_info < (3, 3): - | _____^ -38 | | cmd = [sys.executable, "-m", "test.regrtest"] -39 | | else: -40 | | cmd = [sys.executable, "-m", "test", "-j0"] - | |___________________________________________________^ UP036 -41 | -42 | if sys.version_info < (3, 3): +35 | cmd = [sys.executable, "-m", "test", "-j0"] +36 | +37 | if sys.version_info < (3, 3): + | ^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +38 | cmd = [sys.executable, "-m", "test.regrtest"] +39 | else: | = help: Remove outdated version block @@ -169,16 +149,14 @@ UP036_4.py:37:5: UP036 [*] Version block is outdated for minimum Python version 42 39 | if sys.version_info < (3, 3): 43 40 | cmd = [sys.executable, "-m", "test.regrtest"] -UP036_4.py:42:5: UP036 [*] Version block is outdated for minimum Python version +UP036_4.py:42:8: UP036 [*] Version block is outdated for minimum Python version | -40 | cmd = [sys.executable, "-m", "test", "-j0"] -41 | -42 | if sys.version_info < (3, 3): - | _____^ -43 | | cmd = [sys.executable, "-m", "test.regrtest"] -44 | | elif foo: -45 | | cmd = [sys.executable, "-m", "test", "-j0"] - | |___________________________________________________^ UP036 +40 | cmd = [sys.executable, "-m", "test", "-j0"] +41 | +42 | if sys.version_info < (3, 3): + | ^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +43 | cmd = [sys.executable, "-m", "test.regrtest"] +44 | elif foo: | = help: Remove outdated version block diff --git a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP036_5.py.snap b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP036_5.py.snap index 59edd620b5..3bc04c32cb 100644 --- a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP036_5.py.snap +++ b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP036_5.py.snap @@ -1,24 +1,16 @@ --- source: crates/ruff/src/rules/pyupgrade/mod.rs --- -UP036_5.py:3:1: UP036 [*] Version block is outdated for minimum Python version - | - 1 | import sys - 2 | - 3 | / if sys.version_info < (3, 8): - 4 | | - 5 | | def a(): - 6 | | if b: - 7 | | print(1) - 8 | | elif c: - 9 | | print(2) -10 | | return None -11 | | -12 | | else: -13 | | pass - | |________^ UP036 - | - = help: Remove outdated version block +UP036_5.py:3:4: UP036 [*] Version block is outdated for minimum Python version + | +1 | import sys +2 | +3 | if sys.version_info < (3, 8): + | ^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +4 | +5 | def a(): + | + = help: Remove outdated version block β„Ή Suggested fix 1 1 | import sys @@ -39,24 +31,13 @@ UP036_5.py:3:1: UP036 [*] Version block is outdated for minimum Python version 15 5 | 16 6 | import sys -UP036_5.py:18:1: UP036 [*] Version block is outdated for minimum Python version +UP036_5.py:18:4: UP036 [*] Version block is outdated for minimum Python version | -16 | import sys -17 | -18 | / if sys.version_info < (3, 8): -19 | | pass -20 | | -21 | | else: -22 | | -23 | | def a(): -24 | | if b: -25 | | print(1) -26 | | elif c: -27 | | print(2) -28 | | else: -29 | | print(3) -30 | | return None - | |___________________^ UP036 +16 | import sys +17 | +18 | if sys.version_info < (3, 8): + | ^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +19 | pass | = help: Remove outdated version block diff --git a/crates/ruff/src/rules/ruff/rules/snapshots/ruff__rules__ruff__rules__unreachable__tests__if.py.md.snap b/crates/ruff/src/rules/ruff/rules/snapshots/ruff__rules__ruff__rules__unreachable__tests__if.py.md.snap index 1ab3a11c7d..9294427514 100644 --- a/crates/ruff/src/rules/ruff/rules/snapshots/ruff__rules__ruff__rules__unreachable__tests__if.py.md.snap +++ b/crates/ruff/src/rules/ruff/rules/snapshots/ruff__rules__ruff__rules__unreachable__tests__if.py.md.snap @@ -286,9 +286,11 @@ flowchart TD start(("Start")) return(("End")) block0["return 1\n"] - block1["return 2\n"] - block2["return 0\n"] - block3["elif False: + block1["return 0\n"] + block2["return 2\n"] + block3["if True: + return 1 + elif False: return 2 else: return 0\n"] @@ -302,8 +304,8 @@ flowchart TD start --> block4 block4 -- "True" --> block0 block4 -- "else" --> block3 - block3 -- "False" --> block1 - block3 -- "else" --> block2 + block3 -- "False" --> block2 + block3 -- "else" --> block1 block2 --> return block1 --> return block0 --> return @@ -327,9 +329,11 @@ flowchart TD start(("Start")) return(("End")) block0["return 1\n"] - block1["return 2\n"] - block2["return 0\n"] - block3["elif True: + block1["return 0\n"] + block2["return 2\n"] + block3["if False: + return 1 + elif True: return 2 else: return 0\n"] @@ -343,8 +347,8 @@ flowchart TD start --> block4 block4 -- "False" --> block0 block4 -- "else" --> block3 - block3 -- "True" --> block1 - block3 -- "else" --> block2 + block3 -- "True" --> block2 + block3 -- "else" --> block1 block2 --> return block1 --> return block0 --> return @@ -377,9 +381,11 @@ flowchart TD block0["return 6\n"] block1["return 3\n"] block2["return 0\n"] - block3["return 1\n"] - block4["return 2\n"] - block5["elif True: + block3["return 2\n"] + block4["return 1\n"] + block5["if False: + return 0 + elif True: return 1 else: return 2\n"] @@ -389,9 +395,17 @@ flowchart TD return 1 else: return 2\n"] - block7["return 4\n"] - block8["return 5\n"] - block9["elif True: + block7["return 5\n"] + block8["return 4\n"] + block9["if True: + if False: + return 0 + elif True: + return 1 + else: + return 2 + return 3 + elif True: return 4 else: return 5\n"] @@ -411,14 +425,14 @@ flowchart TD start --> block10 block10 -- "True" --> block6 block10 -- "else" --> block9 - block9 -- "True" --> block7 - block9 -- "else" --> block8 + block9 -- "True" --> block8 + block9 -- "else" --> block7 block8 --> return block7 --> return block6 -- "False" --> block2 block6 -- "else" --> block5 - block5 -- "True" --> block3 - block5 -- "else" --> block4 + block5 -- "True" --> block4 + block5 -- "else" --> block3 block4 --> return block3 --> return block2 --> return @@ -445,7 +459,9 @@ flowchart TD block0["return #quot;reached#quot;\n"] block1["return #quot;unreached#quot;\n"] block2["return #quot;also unreached#quot;\n"] - block3["elif False: + block3["if False: + return #quot;unreached#quot; + elif False: return #quot;also unreached#quot;\n"] block4["if False: return #quot;unreached#quot; @@ -490,14 +506,16 @@ flowchart TD return(("End")) block0["return buffer.data\n"] block1["return base64.b64decode(data)\n"] - block2["buffer = data\n"] - block3["buffer = self._buffers[id]\n"] - block4["self.error(f#quot;can't resolve buffer '{id}'#quot;)\n"] - block5["id = data[#quot;id#quot;]\nif id in self._buffers: + block2["buffer = self._buffers[id]\n"] + block3["self.error(f#quot;can't resolve buffer '{id}'#quot;)\n"] + block4["id = data[#quot;id#quot;]\nif id in self._buffers: buffer = self._buffers[id] else: self.error(f#quot;can't resolve buffer '{id}'#quot;)\n"] - block6["elif isinstance(data, Buffer): + block5["buffer = data\n"] + block6["if isinstance(data, str): + return base64.b64decode(data) + elif isinstance(data, Buffer): buffer = data else: id = data[#quot;id#quot;] @@ -521,11 +539,11 @@ flowchart TD start --> block7 block7 -- "isinstance(data, str)" --> block1 block7 -- "else" --> block6 - block6 -- "isinstance(data, Buffer)" --> block2 - block6 -- "else" --> block5 - block5 -- "id in self._buffers" --> block3 - block5 -- "else" --> block4 - block4 --> block0 + block6 -- "isinstance(data, Buffer)" --> block5 + block6 -- "else" --> block4 + block5 --> block0 + block4 -- "id in self._buffers" --> block2 + block4 -- "else" --> block3 block3 --> block0 block2 --> block0 block1 --> return diff --git a/crates/ruff/src/rules/ruff/rules/unreachable.rs b/crates/ruff/src/rules/ruff/rules/unreachable.rs index 5c81acd9be..872eb50c88 100644 --- a/crates/ruff/src/rules/ruff/rules/unreachable.rs +++ b/crates/ruff/src/rules/ruff/rules/unreachable.rs @@ -484,17 +484,44 @@ impl<'stmt> BasicBlocksBuilder<'stmt> { self.unconditional_next_block(after) } // Statements that (can) divert the control flow. - Stmt::If(stmt) => { - let next_after_block = - self.maybe_next_block_index(after, || needs_next_block(&stmt.body)); - let orelse_after_block = - self.maybe_next_block_index(after, || needs_next_block(&stmt.orelse)); - let next = self.append_blocks_if_not_empty(&stmt.body, next_after_block); - let orelse = self.append_blocks_if_not_empty(&stmt.orelse, orelse_after_block); + Stmt::If(stmt_if) => { + let after_consequent_block = + self.maybe_next_block_index(after, || needs_next_block(&stmt_if.body)); + let after_alternate_block = self.maybe_next_block_index(after, || { + stmt_if + .elif_else_clauses + .last() + .map_or(true, |clause| needs_next_block(&clause.body)) + }); + + let consequent = + self.append_blocks_if_not_empty(&stmt_if.body, after_consequent_block); + + // Block ID of the next elif or else clause. + let mut next_branch = after_alternate_block; + + for clause in stmt_if.elif_else_clauses.iter().rev() { + let consequent = + self.append_blocks_if_not_empty(&clause.body, after_consequent_block); + + next_branch = if let Some(test) = &clause.test { + let next = NextBlock::If { + condition: Condition::Test(test), + next: consequent, + orelse: next_branch, + }; + let stmts = std::slice::from_ref(stmt); + let block = BasicBlock { stmts, next }; + self.blocks.push(block) + } else { + consequent + }; + } + NextBlock::If { - condition: Condition::Test(&stmt.test), - next, - orelse, + condition: Condition::Test(&stmt_if.test), + next: consequent, + orelse: next_branch, } } Stmt::While(StmtWhile { @@ -648,6 +675,7 @@ impl<'stmt> BasicBlocksBuilder<'stmt> { } // The tough branches are done, here is an easy one. Stmt::Return(_) => NextBlock::Terminate, + Stmt::TypeAlias(_) => todo!(), }; // Include any statements in the block that don't divert the control flow. @@ -867,7 +895,7 @@ fn needs_next_block(stmts: &[Stmt]) -> bool { match last { Stmt::Return(_) | Stmt::Raise(_) => false, - Stmt::If(stmt) => needs_next_block(&stmt.body) || needs_next_block(&stmt.orelse), + Stmt::If(stmt) => needs_next_block(&stmt.body) || stmt.elif_else_clauses.last().map_or(true, |clause| needs_next_block(&clause.body)), Stmt::FunctionDef(_) | Stmt::AsyncFunctionDef(_) | Stmt::Import(_) @@ -893,6 +921,7 @@ fn needs_next_block(stmts: &[Stmt]) -> bool { | Stmt::Try(_) | Stmt::TryStar(_) | Stmt::Assert(_) => true, + Stmt::TypeAlias(_) => todo!(), } } @@ -927,6 +956,7 @@ fn is_control_flow_stmt(stmt: &Stmt) -> bool { | Stmt::Assert(_) | Stmt::Break(_) | Stmt::Continue(_) => true, + Stmt::TypeAlias(_) => todo!(), } } @@ -1063,6 +1093,11 @@ mod tests { "first block should always terminate" ); + let got_mermaid = MermaidGraph { + graph: &got, + source: &source, + }; + // All block index should be valid. let valid = BlockIndex::from_usize(got.blocks.len()); for block in &got.blocks { @@ -1076,11 +1111,6 @@ mod tests { } } - let got_mermaid = MermaidGraph { - graph: &got, - source: &source, - }; - writeln!( output, "## Function {i}\n### Source\n```python\n{}\n```\n\n### Control Flow Graph\n```mermaid\n{}```\n", diff --git a/crates/ruff/src/rules/tryceratops/rules/type_check_without_type_error.rs b/crates/ruff/src/rules/tryceratops/rules/type_check_without_type_error.rs index 193da3a5c9..a38078120d 100644 --- a/crates/ruff/src/rules/tryceratops/rules/type_check_without_type_error.rs +++ b/crates/ruff/src/rules/tryceratops/rules/type_check_without_type_error.rs @@ -1,8 +1,7 @@ -use rustpython_parser::ast::{self, Expr, Ranged, Stmt}; - use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::statement_visitor::{walk_stmt, StatementVisitor}; +use rustpython_parser::ast::{self, Expr, Ranged, Stmt, StmtIf}; use crate::checkers::ast::Checker; @@ -164,34 +163,18 @@ fn check_body(checker: &mut Checker, body: &[Stmt]) { } } -/// Search the orelse of an if-condition for raises. -fn check_orelse(checker: &mut Checker, body: &[Stmt]) { - for item in body { - if has_control_flow(item) { - return; - } - match item { - Stmt::If(ast::StmtIf { test, .. }) => { - if !check_type_check_test(checker, test) { - return; - } - } - Stmt::Raise(ast::StmtRaise { exc: Some(exc), .. }) => { - check_raise(checker, exc, item); - } - _ => {} - } - } -} - /// TRY004 pub(crate) fn type_check_without_type_error( checker: &mut Checker, - body: &[Stmt], - test: &Expr, - orelse: &[Stmt], + stmt_if: &StmtIf, parent: Option<&Stmt>, ) { + let StmtIf { + body, + test, + elif_else_clauses, + .. + } = stmt_if; if let Some(Stmt::If(ast::StmtIf { test, .. })) = parent { if !check_type_check_test(checker, test) { return; @@ -199,8 +182,20 @@ pub(crate) fn type_check_without_type_error( } // Only consider the body when the `if` condition is all type-related - if check_type_check_test(checker, test) { - check_body(checker, body); - check_orelse(checker, orelse); + if !check_type_check_test(checker, test) { + return; + } + check_body(checker, body); + + for clause in elif_else_clauses { + if let Some(test) = &clause.test { + // If there are any `elif`, they must all also be type-related + if !check_type_check_test(checker, test) { + return; + } + } + + // The `elif` or `else` body raises the wrong exception + check_body(checker, &clause.body); } } diff --git a/crates/ruff/src/rules/tryceratops/snapshots/ruff__rules__tryceratops__tests__type-check-without-type-error_TRY004.py.snap b/crates/ruff/src/rules/tryceratops/snapshots/ruff__rules__tryceratops__tests__type-check-without-type-error_TRY004.py.snap index 0d2f2202ef..be351bfdc4 100644 --- a/crates/ruff/src/rules/tryceratops/snapshots/ruff__rules__tryceratops__tests__type-check-without-type-error_TRY004.py.snap +++ b/crates/ruff/src/rules/tryceratops/snapshots/ruff__rules__tryceratops__tests__type-check-without-type-error_TRY004.py.snap @@ -252,32 +252,48 @@ TRY004.py:230:9: TRY004 Prefer `TypeError` exception for invalid type | ^^^^^^^^^^^^^^^^^^^^^^ TRY004 | -TRY004.py:267:9: TRY004 Prefer `TypeError` exception for invalid type +TRY004.py:239:9: TRY004 Prefer `TypeError` exception for invalid type | -265 | def check_body(some_args): -266 | if isinstance(some_args, int): -267 | raise ValueError("...") # should be typeerror +237 | pass +238 | else: +239 | raise Exception("...") # should be typeerror + | ^^^^^^^^^^^^^^^^^^^^^^ TRY004 + | + +TRY004.py:276:9: TRY004 Prefer `TypeError` exception for invalid type + | +274 | def check_body(some_args): +275 | if isinstance(some_args, int): +276 | raise ValueError("...") # should be typeerror | ^^^^^^^^^^^^^^^^^^^^^^^ TRY004 | -TRY004.py:277:9: TRY004 Prefer `TypeError` exception for invalid type +TRY004.py:286:9: TRY004 Prefer `TypeError` exception for invalid type | -275 | def multiple_elifs(some_args): -276 | if not isinstance(some_args, int): -277 | raise ValueError("...") # should be typerror +284 | def multiple_elifs(some_args): +285 | if not isinstance(some_args, int): +286 | raise ValueError("...") # should be typerror | ^^^^^^^^^^^^^^^^^^^^^^^ TRY004 -278 | elif some_args < 3: -279 | raise ValueError("...") # this is ok +287 | elif some_args < 3: +288 | raise ValueError("...") # this is ok | -TRY004.py:288:9: TRY004 Prefer `TypeError` exception for invalid type +TRY004.py:297:9: TRY004 Prefer `TypeError` exception for invalid type | -286 | def multiple_ifs(some_args): -287 | if not isinstance(some_args, int): -288 | raise ValueError("...") # should be typerror +295 | def multiple_ifs(some_args): +296 | if not isinstance(some_args, int): +297 | raise ValueError("...") # should be typerror | ^^^^^^^^^^^^^^^^^^^^^^^ TRY004 -289 | else: -290 | if some_args < 3: +298 | else: +299 | if some_args < 3: + | + +TRY004.py:316:9: TRY004 Prefer `TypeError` exception for invalid type + | +314 | return "CronExpression" +315 | else: +316 | raise Exception(f"Unknown object type: {obj.__class__.__name__}") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY004 | diff --git a/crates/ruff_python_ast/src/comparable.rs b/crates/ruff_python_ast/src/comparable.rs index cfda94d4b5..5ebc6468ba 100644 --- a/crates/ruff_python_ast/src/comparable.rs +++ b/crates/ruff_python_ast/src/comparable.rs @@ -466,6 +466,26 @@ impl<'a> From<&'a ast::ExceptHandler> for ComparableExceptHandler<'a> { } } +#[derive(Debug, PartialEq, Eq, Hash)] +pub struct ComparableElifElseClause<'a> { + test: Option>, + body: Vec>, +} + +impl<'a> From<&'a ast::ElifElseClause> for ComparableElifElseClause<'a> { + fn from(elif_else_clause: &'a ast::ElifElseClause) -> Self { + let ast::ElifElseClause { + range: _, + test, + body, + } = elif_else_clause; + Self { + test: test.as_ref().map(Into::into), + body: body.iter().map(Into::into).collect(), + } + } +} + #[derive(Debug, PartialEq, Eq, Hash)] pub struct ExprBoolOp<'a> { op: ComparableBoolOp, @@ -999,7 +1019,7 @@ pub struct StmtWhile<'a> { pub struct StmtIf<'a> { test: ComparableExpr<'a>, body: Vec>, - orelse: Vec>, + elif_else_clauses: Vec>, } #[derive(Debug, PartialEq, Eq, Hash)] @@ -1118,7 +1138,8 @@ impl<'a> From<&'a ast::Stmt> for ComparableStmt<'a> { decorator_list, returns, type_comment, - range: _range, + range: _, + type_params: _, }) => Self::FunctionDef(StmtFunctionDef { name: name.as_str(), args: args.into(), @@ -1134,7 +1155,8 @@ impl<'a> From<&'a ast::Stmt> for ComparableStmt<'a> { decorator_list, returns, type_comment, - range: _range, + range: _, + type_params: _, }) => Self::AsyncFunctionDef(StmtAsyncFunctionDef { name: name.as_str(), args: args.into(), @@ -1149,7 +1171,8 @@ impl<'a> From<&'a ast::Stmt> for ComparableStmt<'a> { keywords, body, decorator_list, - range: _range, + range: _, + type_params: _, }) => Self::ClassDef(StmtClassDef { name: name.as_str(), bases: bases.iter().map(Into::into).collect(), @@ -1242,12 +1265,12 @@ impl<'a> From<&'a ast::Stmt> for ComparableStmt<'a> { ast::Stmt::If(ast::StmtIf { test, body, - orelse, + elif_else_clauses, range: _range, }) => Self::If(StmtIf { test: test.into(), body: body.iter().map(Into::into).collect(), - orelse: orelse.iter().map(Into::into).collect(), + elif_else_clauses: elif_else_clauses.iter().map(Into::into).collect(), }), ast::Stmt::With(ast::StmtWith { items, @@ -1354,6 +1377,7 @@ impl<'a> From<&'a ast::Stmt> for ComparableStmt<'a> { ast::Stmt::Pass(_) => Self::Pass, ast::Stmt::Break(_) => Self::Break, ast::Stmt::Continue(_) => Self::Continue, + ast::Stmt::TypeAlias(_) => todo!(), } } } diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index b946a17e5c..0dfad2d123 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -437,9 +437,19 @@ where Stmt::If(ast::StmtIf { test, body, - orelse, + elif_else_clauses, range: _range, - }) => any_over_expr(test, func) || any_over_body(body, func) || any_over_body(orelse, func), + }) => { + any_over_expr(test, func) + || any_over_body(body, func) + || elif_else_clauses.iter().any(|clause| { + clause + .test + .as_ref() + .map_or(false, |test| any_over_expr(test, func)) + || any_over_body(&clause.body, func) + }) + } Stmt::With(ast::StmtWith { items, body, .. }) | Stmt::AsyncWith(ast::StmtAsyncWith { items, body, .. }) => { items.iter().any(|with_item| { @@ -529,6 +539,7 @@ where range: _range, }) => any_over_expr(value, func), Stmt::Pass(_) | Stmt::Break(_) | Stmt::Continue(_) => false, + Stmt::TypeAlias(_) => todo!(), } } @@ -944,9 +955,15 @@ where | Stmt::AsyncFunctionDef(_) | Stmt::Try(_) | Stmt::TryStar(_) => {} - Stmt::If(ast::StmtIf { body, orelse, .. }) => { + Stmt::If(ast::StmtIf { + body, + elif_else_clauses, + .. + }) => { walk_body(self, body); - walk_body(self, orelse); + for clause in elif_else_clauses { + self.visit_elif_else_clause(clause); + } } Stmt::While(ast::StmtWhile { body, .. }) | Stmt::With(ast::StmtWith { body, .. }) @@ -1063,25 +1080,6 @@ pub fn first_colon_range(range: TextRange, locator: &Locator) -> Option Option { - let ast::StmtIf { body, orelse, .. } = stmt; - - let start = body.last().expect("Expected body to be non-empty").end(); - - let end = match &orelse[..] { - [Stmt::If(ast::StmtIf { test, .. })] => test.start(), - [stmt, ..] => stmt.start(), - _ => return None, - }; - - let contents = &locator.contents()[TextRange::new(start, end)]; - lexer::lex_starts_at(contents, Mode::Module, start) - .flatten() - .find(|(kind, _)| matches!(kind, Tok::Elif | Tok::Else)) - .map(|(_, range)| range) -} - /// Given an offset at the end of a line (including newlines), return the offset of the /// continuation at the end of that line. fn find_continuation(offset: TextSize, locator: &Locator, indexer: &Indexer) -> Option { @@ -1568,13 +1566,13 @@ mod tests { use anyhow::Result; use ruff_text_size::{TextLen, TextRange, TextSize}; - use rustpython_ast::{CmpOp, Expr, Ranged, Stmt}; + use rustpython_ast::{CmpOp, Expr, Ranged}; use rustpython_parser::ast::Suite; use rustpython_parser::Parse; use crate::helpers::{ - elif_else_range, first_colon_range, has_trailing_content, locate_cmp_ops, - resolve_imported_module_path, LocatedCmpOp, + first_colon_range, has_trailing_content, locate_cmp_ops, resolve_imported_module_path, + LocatedCmpOp, }; use crate::source_code::Locator; @@ -1667,35 +1665,6 @@ y = 2 assert_eq!(range, TextRange::new(TextSize::from(6), TextSize::from(7))); } - #[test] - fn extract_elif_else_range() -> Result<()> { - let contents = "if a: - ... -elif b: - ... -"; - let stmt = Stmt::parse(contents, "")?; - let stmt = Stmt::as_if_stmt(&stmt).unwrap(); - let locator = Locator::new(contents); - let range = elif_else_range(stmt, &locator).unwrap(); - assert_eq!(range.start(), TextSize::from(14)); - assert_eq!(range.end(), TextSize::from(18)); - - let contents = "if a: - ... -else: - ... -"; - let stmt = Stmt::parse(contents, "")?; - let stmt = Stmt::as_if_stmt(&stmt).unwrap(); - let locator = Locator::new(contents); - let range = elif_else_range(stmt, &locator).unwrap(); - assert_eq!(range.start(), TextSize::from(14)); - assert_eq!(range.end(), TextSize::from(18)); - - Ok(()) - } - #[test] fn extract_cmp_op_location() -> Result<()> { let contents = "x == 1"; diff --git a/crates/ruff_python_ast/src/lib.rs b/crates/ruff_python_ast/src/lib.rs index 72928ec9e9..424e1be6ff 100644 --- a/crates/ruff_python_ast/src/lib.rs +++ b/crates/ruff_python_ast/src/lib.rs @@ -12,6 +12,7 @@ pub mod node; pub mod relocate; pub mod source_code; pub mod statement_visitor; +pub mod stmt_if; pub mod str; pub mod token_kind; pub mod types; diff --git a/crates/ruff_python_ast/src/node.rs b/crates/ruff_python_ast/src/node.rs index ccbc44c723..ef3e6b4309 100644 --- a/crates/ruff_python_ast/src/node.rs +++ b/crates/ruff_python_ast/src/node.rs @@ -98,6 +98,7 @@ pub enum AnyNode { WithItem(WithItem), MatchCase(MatchCase), Decorator(Decorator), + ElifElseClause(ast::ElifElseClause), } impl AnyNode { @@ -180,7 +181,8 @@ impl AnyNode { | AnyNode::Alias(_) | AnyNode::WithItem(_) | AnyNode::MatchCase(_) - | AnyNode::Decorator(_) => None, + | AnyNode::Decorator(_) + | AnyNode::ElifElseClause(_) => None, } } @@ -263,7 +265,8 @@ impl AnyNode { | AnyNode::Alias(_) | AnyNode::WithItem(_) | AnyNode::MatchCase(_) - | AnyNode::Decorator(_) => None, + | AnyNode::Decorator(_) + | AnyNode::ElifElseClause(_) => None, } } @@ -346,7 +349,8 @@ impl AnyNode { | AnyNode::Alias(_) | AnyNode::WithItem(_) | AnyNode::MatchCase(_) - | AnyNode::Decorator(_) => None, + | AnyNode::Decorator(_) + | AnyNode::ElifElseClause(_) => None, } } @@ -429,7 +433,8 @@ impl AnyNode { | AnyNode::Alias(_) | AnyNode::WithItem(_) | AnyNode::MatchCase(_) - | AnyNode::Decorator(_) => None, + | AnyNode::Decorator(_) + | AnyNode::ElifElseClause(_) => None, } } @@ -512,7 +517,8 @@ impl AnyNode { | AnyNode::Alias(_) | AnyNode::WithItem(_) | AnyNode::MatchCase(_) - | AnyNode::Decorator(_) => None, + | AnyNode::Decorator(_) + | AnyNode::ElifElseClause(_) => None, } } @@ -595,7 +601,8 @@ impl AnyNode { | AnyNode::Alias(_) | AnyNode::WithItem(_) | AnyNode::MatchCase(_) - | AnyNode::Decorator(_) => None, + | AnyNode::Decorator(_) + | AnyNode::ElifElseClause(_) => None, } } @@ -702,6 +709,7 @@ impl AnyNode { Self::WithItem(node) => AnyNodeRef::WithItem(node), Self::MatchCase(node) => AnyNodeRef::MatchCase(node), Self::Decorator(node) => AnyNodeRef::Decorator(node), + Self::ElifElseClause(node) => AnyNodeRef::ElifElseClause(node), } } @@ -1159,6 +1167,34 @@ impl AstNode for ast::StmtIf { AnyNode::from(self) } } +impl AstNode for ast::ElifElseClause { + fn cast(kind: AnyNode) -> Option + where + Self: Sized, + { + if let AnyNode::ElifElseClause(node) = kind { + Some(node) + } else { + None + } + } + + fn cast_ref(kind: AnyNodeRef) -> Option<&Self> { + if let AnyNodeRef::ElifElseClause(node) = kind { + Some(node) + } else { + None + } + } + + fn as_any_node_ref(&self) -> AnyNodeRef { + AnyNodeRef::from(self) + } + + fn into_any_node(self) -> AnyNode { + AnyNode::from(self) + } +} impl AstNode for ast::StmtWith { fn cast(kind: AnyNode) -> Option where @@ -2900,6 +2936,7 @@ impl From for AnyNode { Stmt::Pass(node) => AnyNode::StmtPass(node), Stmt::Break(node) => AnyNode::StmtBreak(node), Stmt::Continue(node) => AnyNode::StmtContinue(node), + Stmt::TypeAlias(_) => todo!(), } } } @@ -3076,6 +3113,12 @@ impl From for AnyNode { } } +impl From for AnyNode { + fn from(node: ast::ElifElseClause) -> Self { + AnyNode::ElifElseClause(node) + } +} + impl From for AnyNode { fn from(node: ast::StmtWith) -> Self { AnyNode::StmtWith(node) @@ -3514,6 +3557,7 @@ impl Ranged for AnyNode { AnyNode::WithItem(node) => node.range(), AnyNode::MatchCase(node) => node.range(), AnyNode::Decorator(node) => node.range(), + AnyNode::ElifElseClause(node) => node.range(), } } } @@ -3597,6 +3641,7 @@ pub enum AnyNodeRef<'a> { WithItem(&'a WithItem), MatchCase(&'a MatchCase), Decorator(&'a Decorator), + ElifElseClause(&'a ast::ElifElseClause), } impl AnyNodeRef<'_> { @@ -3679,6 +3724,7 @@ impl AnyNodeRef<'_> { AnyNodeRef::WithItem(node) => NonNull::from(*node).cast(), AnyNodeRef::MatchCase(node) => NonNull::from(*node).cast(), AnyNodeRef::Decorator(node) => NonNull::from(*node).cast(), + AnyNodeRef::ElifElseClause(node) => NonNull::from(*node).cast(), } } @@ -3767,6 +3813,7 @@ impl AnyNodeRef<'_> { AnyNodeRef::WithItem(_) => NodeKind::WithItem, AnyNodeRef::MatchCase(_) => NodeKind::MatchCase, AnyNodeRef::Decorator(_) => NodeKind::Decorator, + AnyNodeRef::ElifElseClause(_) => NodeKind::ElifElseClause, } } @@ -3849,7 +3896,8 @@ impl AnyNodeRef<'_> { | AnyNodeRef::Alias(_) | AnyNodeRef::WithItem(_) | AnyNodeRef::MatchCase(_) - | AnyNodeRef::Decorator(_) => false, + | AnyNodeRef::Decorator(_) + | AnyNodeRef::ElifElseClause(_) => false, } } @@ -3932,7 +3980,8 @@ impl AnyNodeRef<'_> { | AnyNodeRef::Alias(_) | AnyNodeRef::WithItem(_) | AnyNodeRef::MatchCase(_) - | AnyNodeRef::Decorator(_) => false, + | AnyNodeRef::Decorator(_) + | AnyNodeRef::ElifElseClause(_) => false, } } @@ -4015,7 +4064,8 @@ impl AnyNodeRef<'_> { | AnyNodeRef::Alias(_) | AnyNodeRef::WithItem(_) | AnyNodeRef::MatchCase(_) - | AnyNodeRef::Decorator(_) => false, + | AnyNodeRef::Decorator(_) + | AnyNodeRef::ElifElseClause(_) => false, } } @@ -4098,7 +4148,8 @@ impl AnyNodeRef<'_> { | AnyNodeRef::Alias(_) | AnyNodeRef::WithItem(_) | AnyNodeRef::MatchCase(_) - | AnyNodeRef::Decorator(_) => false, + | AnyNodeRef::Decorator(_) + | AnyNodeRef::ElifElseClause(_) => false, } } @@ -4181,7 +4232,8 @@ impl AnyNodeRef<'_> { | AnyNodeRef::Alias(_) | AnyNodeRef::WithItem(_) | AnyNodeRef::MatchCase(_) - | AnyNodeRef::Decorator(_) => false, + | AnyNodeRef::Decorator(_) + | AnyNodeRef::ElifElseClause(_) => false, } } @@ -4264,7 +4316,8 @@ impl AnyNodeRef<'_> { | AnyNodeRef::Alias(_) | AnyNodeRef::WithItem(_) | AnyNodeRef::MatchCase(_) - | AnyNodeRef::Decorator(_) => false, + | AnyNodeRef::Decorator(_) + | AnyNodeRef::ElifElseClause(_) => false, } } @@ -4383,6 +4436,12 @@ impl<'a> From<&'a ast::StmtIf> for AnyNodeRef<'a> { } } +impl<'a> From<&'a ast::ElifElseClause> for AnyNodeRef<'a> { + fn from(node: &'a ast::ElifElseClause) -> Self { + AnyNodeRef::ElifElseClause(node) + } +} + impl<'a> From<&'a ast::StmtWith> for AnyNodeRef<'a> { fn from(node: &'a ast::StmtWith) -> Self { AnyNodeRef::StmtWith(node) @@ -4731,6 +4790,7 @@ impl<'a> From<&'a Stmt> for AnyNodeRef<'a> { Stmt::Pass(node) => AnyNodeRef::StmtPass(node), Stmt::Break(node) => AnyNodeRef::StmtBreak(node), Stmt::Continue(node) => AnyNodeRef::StmtContinue(node), + Stmt::TypeAlias(_) => todo!(), } } } @@ -4934,6 +4994,7 @@ impl Ranged for AnyNodeRef<'_> { AnyNodeRef::WithItem(node) => node.range(), AnyNodeRef::MatchCase(node) => node.range(), AnyNodeRef::Decorator(node) => node.range(), + AnyNodeRef::ElifElseClause(node) => node.range(), } } } @@ -5017,4 +5078,5 @@ pub enum NodeKind { WithItem, MatchCase, Decorator, + ElifElseClause, } diff --git a/crates/ruff_python_ast/src/source_code/comment_ranges.rs b/crates/ruff_python_ast/src/source_code/comment_ranges.rs index 977bc83b0d..1c296666eb 100644 --- a/crates/ruff_python_ast/src/source_code/comment_ranges.rs +++ b/crates/ruff_python_ast/src/source_code/comment_ranges.rs @@ -1,3 +1,4 @@ +use itertools::Itertools; use std::fmt::{Debug, Formatter}; use std::ops::Deref; @@ -25,6 +26,21 @@ impl CommentRanges { }) .is_ok() } + + /// Returns the comments who are within the range + pub fn comments_in_range(&self, range: TextRange) -> &[TextRange] { + let start = self + .raw + .partition_point(|comment| comment.start() < range.start()); + // We expect there are few comments, so switching to find should be faster + match self.raw[start..] + .iter() + .find_position(|comment| comment.end() > range.end()) + { + Some((in_range, _element)) => &self.raw[start..start + in_range], + None => &self.raw[start..], + } + } } impl Deref for CommentRanges { diff --git a/crates/ruff_python_ast/src/source_code/generator.rs b/crates/ruff_python_ast/src/source_code/generator.rs index 5c34362bee..412b897eed 100644 --- a/crates/ruff_python_ast/src/source_code/generator.rs +++ b/crates/ruff_python_ast/src/source_code/generator.rs @@ -271,7 +271,8 @@ impl<'a> Generator<'a> { keywords, body, decorator_list, - range: _range, + range: _, + type_params: _, }) => { self.newlines(if self.indent_depth == 0 { 2 } else { 1 }); for decorator in decorator_list { @@ -457,7 +458,7 @@ impl<'a> Generator<'a> { Stmt::If(ast::StmtIf { test, body, - orelse, + elif_else_clauses, range: _range, }) => { statement!({ @@ -467,33 +468,19 @@ impl<'a> Generator<'a> { }); self.body(body); - let mut orelse_: &[Stmt] = orelse; - loop { - if orelse_.len() == 1 && matches!(orelse_[0], Stmt::If(_)) { - if let Stmt::If(ast::StmtIf { - body, - test, - orelse, - range: _range, - }) = &orelse_[0] - { - statement!({ - self.p("elif "); - self.unparse_expr(test, precedence::IF); - self.p(":"); - }); - self.body(body); - orelse_ = orelse; - } + for clause in elif_else_clauses { + if let Some(test) = &clause.test { + statement!({ + self.p("elif "); + self.unparse_expr(test, precedence::IF); + self.p(":"); + }); } else { - if !orelse_.is_empty() { - statement!({ - self.p("else:"); - }); - self.body(orelse_); - } - break; + statement!({ + self.p("else:"); + }); } + self.body(&clause.body); } } Stmt::With(ast::StmtWith { items, body, .. }) => { @@ -715,6 +702,7 @@ impl<'a> Generator<'a> { self.p("continue"); }); } + Stmt::TypeAlias(_) => todo!(), } } diff --git a/crates/ruff_python_ast/src/source_code/indexer.rs b/crates/ruff_python_ast/src/source_code/indexer.rs index c70e8378fc..2a60e64c86 100644 --- a/crates/ruff_python_ast/src/source_code/indexer.rs +++ b/crates/ruff_python_ast/src/source_code/indexer.rs @@ -97,6 +97,18 @@ impl Indexer { &self.comment_ranges } + /// Returns the comments in the given range as source code slices + pub fn comments_in_range<'a>( + &'a self, + range: TextRange, + locator: &'a Locator, + ) -> impl Iterator { + self.comment_ranges + .comments_in_range(range) + .iter() + .map(move |comment_range| locator.slice(*comment_range)) + } + /// Returns the line start positions of continuations (backslash). pub fn continuation_line_starts(&self) -> &[TextSize] { &self.continuation_lines diff --git a/crates/ruff_python_ast/src/statement_visitor.rs b/crates/ruff_python_ast/src/statement_visitor.rs index df35b6bb2e..bda9fa8f64 100644 --- a/crates/ruff_python_ast/src/statement_visitor.rs +++ b/crates/ruff_python_ast/src/statement_visitor.rs @@ -1,5 +1,6 @@ //! Specialized AST visitor trait and walk functions that only visit statements. +use rustpython_ast::ElifElseClause; use rustpython_parser::ast::{self, ExceptHandler, MatchCase, Stmt}; /// A trait for AST visitors that only need to visit statements. @@ -13,6 +14,9 @@ pub trait StatementVisitor<'a> { fn visit_except_handler(&mut self, except_handler: &'a ExceptHandler) { walk_except_handler(self, except_handler); } + fn visit_elif_else_clause(&mut self, elif_else_clause: &'a ElifElseClause) { + walk_elif_else_clause(self, elif_else_clause); + } fn visit_match_case(&mut self, match_case: &'a MatchCase) { walk_match_case(self, match_case); } @@ -47,9 +51,15 @@ pub fn walk_stmt<'a, V: StatementVisitor<'a> + ?Sized>(visitor: &mut V, stmt: &' visitor.visit_body(body); visitor.visit_body(orelse); } - Stmt::If(ast::StmtIf { body, orelse, .. }) => { + Stmt::If(ast::StmtIf { + body, + elif_else_clauses, + .. + }) => { visitor.visit_body(body); - visitor.visit_body(orelse); + for clause in elif_else_clauses { + visitor.visit_elif_else_clause(clause); + } } Stmt::With(ast::StmtWith { body, .. }) => { visitor.visit_body(body); @@ -105,6 +115,13 @@ pub fn walk_except_handler<'a, V: StatementVisitor<'a> + ?Sized>( } } +pub fn walk_elif_else_clause<'a, V: StatementVisitor<'a> + ?Sized>( + visitor: &mut V, + elif_else_clause: &'a ElifElseClause, +) { + visitor.visit_body(&elif_else_clause.body); +} + pub fn walk_match_case<'a, V: StatementVisitor<'a> + ?Sized>( visitor: &mut V, match_case: &'a MatchCase, diff --git a/crates/ruff_python_ast/src/stmt_if.rs b/crates/ruff_python_ast/src/stmt_if.rs new file mode 100644 index 0000000000..1ad617e1d8 --- /dev/null +++ b/crates/ruff_python_ast/src/stmt_if.rs @@ -0,0 +1,87 @@ +use crate::source_code::Locator; +use ruff_text_size::TextRange; +use rustpython_ast::{ElifElseClause, Expr, Ranged, Stmt, StmtIf}; +use rustpython_parser::{lexer, Mode, Tok}; +use std::iter; + +/// Return the `Range` of the first `Elif` or `Else` token in an `If` statement. +pub fn elif_else_range(clause: &ElifElseClause, locator: &Locator) -> Option { + let contents = &locator.contents()[clause.range]; + let token = lexer::lex_starts_at(contents, Mode::Module, clause.range.start()) + .flatten() + .next()?; + if matches!(token.0, Tok::Elif | Tok::Else) { + Some(token.1) + } else { + None + } +} + +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub enum BranchKind { + If, + Elif, +} + +pub struct IfElifBranch<'a> { + pub kind: BranchKind, + pub test: &'a Expr, + pub body: &'a [Stmt], + pub range: TextRange, +} + +pub fn if_elif_branches(stmt_if: &StmtIf) -> impl Iterator { + iter::once(IfElifBranch { + kind: BranchKind::If, + test: stmt_if.test.as_ref(), + body: stmt_if.body.as_slice(), + range: TextRange::new(stmt_if.start(), stmt_if.body.last().unwrap().end()), + }) + .chain(stmt_if.elif_else_clauses.iter().filter_map(|clause| { + Some(IfElifBranch { + kind: BranchKind::Elif, + test: clause.test.as_ref()?, + body: clause.body.as_slice(), + range: clause.range, + }) + })) +} + +#[cfg(test)] +mod test { + use crate::source_code::Locator; + use crate::stmt_if::elif_else_range; + use anyhow::Result; + use ruff_text_size::TextSize; + use rustpython_ast::Stmt; + use rustpython_parser::Parse; + + #[test] + fn extract_elif_else_range() -> Result<()> { + let contents = "if a: + ... +elif b: + ... +"; + let stmt = Stmt::parse(contents, "")?; + let stmt = Stmt::as_if_stmt(&stmt).unwrap(); + let locator = Locator::new(contents); + let range = elif_else_range(&stmt.elif_else_clauses[0], &locator).unwrap(); + assert_eq!(range.start(), TextSize::from(14)); + assert_eq!(range.end(), TextSize::from(18)); + + let contents = "if a: + ... +else: + ... +"; + let stmt = Stmt::parse(contents, "")?; + let stmt = Stmt::as_if_stmt(&stmt).unwrap(); + let locator = Locator::new(contents); + let range = elif_else_range(&stmt.elif_else_clauses[0], &locator).unwrap(); + assert_eq!(range.start(), TextSize::from(14)); + assert_eq!(range.end(), TextSize::from(18)); + + Ok(()) + } +} diff --git a/crates/ruff_python_ast/src/token_kind.rs b/crates/ruff_python_ast/src/token_kind.rs index 7b460eba20..89b88c7fcc 100644 --- a/crates/ruff_python_ast/src/token_kind.rs +++ b/crates/ruff_python_ast/src/token_kind.rs @@ -431,6 +431,8 @@ impl TokenKind { Tok::StartModule => TokenKind::StartModule, Tok::StartInteractive => TokenKind::StartInteractive, Tok::StartExpression => TokenKind::StartExpression, + Tok::MagicCommand { .. } => todo!(), + Tok::Type => todo!(), } } } diff --git a/crates/ruff_python_ast/src/visitor.rs b/crates/ruff_python_ast/src/visitor.rs index 9d90036cf8..bf2f87ac92 100644 --- a/crates/ruff_python_ast/src/visitor.rs +++ b/crates/ruff_python_ast/src/visitor.rs @@ -2,6 +2,7 @@ pub mod preorder; +use rustpython_ast::ElifElseClause; use rustpython_parser::ast::{ self, Alias, Arg, Arguments, BoolOp, CmpOp, Comprehension, Decorator, ExceptHandler, Expr, ExprContext, Keyword, MatchCase, Operator, Pattern, Stmt, UnaryOp, WithItem, @@ -75,6 +76,9 @@ pub trait Visitor<'a> { fn visit_body(&mut self, body: &'a [Stmt]) { walk_body(self, body); } + fn visit_elif_else_clause(&mut self, elif_else_clause: &'a ElifElseClause) { + walk_elif_else_clause(self, elif_else_clause); + } } pub fn walk_body<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, body: &'a [Stmt]) { @@ -83,6 +87,16 @@ pub fn walk_body<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, body: &'a [Stmt]) } } +pub fn walk_elif_else_clause<'a, V: Visitor<'a> + ?Sized>( + visitor: &mut V, + elif_else_clause: &'a ElifElseClause, +) { + if let Some(test) = &elif_else_clause.test { + visitor.visit_expr(test); + } + visitor.visit_body(&elif_else_clause.body); +} + pub fn walk_stmt<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, stmt: &'a Stmt) { match stmt { Stmt::FunctionDef(ast::StmtFunctionDef { @@ -216,12 +230,17 @@ pub fn walk_stmt<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, stmt: &'a Stmt) { Stmt::If(ast::StmtIf { test, body, - orelse, + elif_else_clauses, range: _range, }) => { visitor.visit_expr(test); visitor.visit_body(body); - visitor.visit_body(orelse); + for clause in elif_else_clauses { + if let Some(test) = &clause.test { + visitor.visit_expr(test); + } + walk_elif_else_clause(visitor, clause); + } } Stmt::With(ast::StmtWith { items, body, .. }) => { for with_item in items { @@ -315,6 +334,7 @@ pub fn walk_stmt<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, stmt: &'a Stmt) { range: _range, }) => visitor.visit_expr(value), Stmt::Pass(_) | Stmt::Break(_) | Stmt::Continue(_) => {} + Stmt::TypeAlias(_) => todo!(), } } diff --git a/crates/ruff_python_ast/src/visitor/preorder.rs b/crates/ruff_python_ast/src/visitor/preorder.rs index 5e83d1e782..ae9209d21f 100644 --- a/crates/ruff_python_ast/src/visitor/preorder.rs +++ b/crates/ruff_python_ast/src/visitor/preorder.rs @@ -1,4 +1,4 @@ -use rustpython_ast::{ArgWithDefault, Mod, TypeIgnore}; +use rustpython_ast::{ArgWithDefault, ElifElseClause, Mod, TypeIgnore}; use rustpython_parser::ast::{ self, Alias, Arg, Arguments, BoolOp, CmpOp, Comprehension, Constant, Decorator, ExceptHandler, Expr, Keyword, MatchCase, Operator, Pattern, Stmt, UnaryOp, WithItem, @@ -95,6 +95,10 @@ pub trait PreorderVisitor<'a> { fn visit_type_ignore(&mut self, type_ignore: &'a TypeIgnore) { walk_type_ignore(self, type_ignore); } + + fn visit_elif_else_clause(&mut self, elif_else_clause: &'a ElifElseClause) { + walk_elif_else_clause(self, elif_else_clause); + } } pub fn walk_module<'a, V>(visitor: &mut V, module: &'a Mod) @@ -286,12 +290,14 @@ where Stmt::If(ast::StmtIf { test, body, - orelse, + elif_else_clauses, range: _range, }) => { visitor.visit_expr(test); visitor.visit_body(body); - visitor.visit_body(orelse); + for clause in elif_else_clauses { + visitor.visit_elif_else_clause(clause); + } } Stmt::With(ast::StmtWith { @@ -394,6 +400,7 @@ where | Stmt::Continue(_) | Stmt::Global(_) | Stmt::Nonlocal(_) => {} + Stmt::TypeAlias(_) => todo!(), } } @@ -703,6 +710,16 @@ where } } +pub fn walk_elif_else_clause<'a, V>(visitor: &mut V, elif_else_clause: &'a ElifElseClause) +where + V: PreorderVisitor<'a> + ?Sized, +{ + if let Some(test) = &elif_else_clause.test { + visitor.visit_expr(test); + } + visitor.visit_body(&elif_else_clause.body); +} + pub fn walk_except_handler<'a, V>(visitor: &mut V, except_handler: &'a ExceptHandler) where V: PreorderVisitor<'a> + ?Sized, diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/if.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/if.py index e737ebbab1..e332e221df 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/if.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/if.py @@ -1,16 +1,20 @@ -if x == y: # trailing if condition - pass # trailing `pass` comment - # Root `if` trailing comment +# 1 leading if comment +if x == y: # 2 trailing if condition + # 3 leading pass + pass # 4 end-of-line trailing `pass` comment + # 5 Root `if` trailing comment -# Leading elif comment -elif x < y: # trailing elif condition - pass - # `elif` trailing comment +# 6 Leading elif comment +elif x < y: # 7 trailing elif condition + # 8 leading pass + pass # 9 end-of-line trailing `pass` comment + # 10 `elif` trailing comment -# Leading else comment -else: # trailing else condition - pass - # `else` trailing comment +# 11 Leading else comment +else: # 12 trailing else condition + # 13 leading pass + pass # 14 end-of-line trailing `pass` comment + # 15 `else` trailing comment if x == y: @@ -71,3 +75,14 @@ else: # Comment if False: pass pass + + +# Regression test for `last_child_in_body` special casing of `StmtIf` +# https://github.com/python/cpython/blob/aecf6aca515a203a823a87c711f15cbb82097c8b/Lib/test/test_pty.py#L260-L275 +def f(): + if True: + pass + else: + pass + + # comment diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index 6effa9e3f5..e4d7ec9d82 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -292,12 +292,26 @@ fn handle_in_between_bodies_own_line_comment<'a>( // if x == y: // pass // # I'm a leading comment of the `elif` statement. - // elif: + // elif True: // print("nooop") // ``` - if following.is_stmt_if() || following.is_except_handler() { - // The `elif` or except handlers have their own body to which we can attach the leading comment + if following.is_except_handler() { + // The except handlers have their own body to which we can attach the leading comment CommentPlacement::leading(following, comment) + } else if let AnyNodeRef::StmtIf(stmt_if) = comment.enclosing_node() { + if let Some(clause) = stmt_if + .elif_else_clauses + .iter() + .find(|clause| are_same_optional(following, clause.test.as_ref())) + { + CommentPlacement::leading(clause.into(), comment) + } else { + // Since we know we're between bodies and we know that the following node is + // not the condition of any `elif`, we know the next node must be the `else` + let else_clause = stmt_if.elif_else_clauses.last().unwrap(); + debug_assert!(else_clause.test.is_none()); + CommentPlacement::leading(else_clause.into(), comment) + } } else { // There are no bodies for the "else" branch and other bodies that are represented as a `Vec`. // This means, there's no good place to attach the comments to. @@ -356,42 +370,42 @@ fn handle_in_between_bodies_end_of_line_comment<'a>( } if locator.contains_line_break(TextRange::new(preceding.end(), comment.slice().start())) { - // The `elif` or except handlers have their own body to which we can attach the trailing comment + // The except handlers have their own body to which we can attach the trailing comment // ```python - // if test: - // a - // elif c: # comment - // b + // try: + // f() # comment + // except RuntimeError: + // raise // ``` if following.is_except_handler() { return CommentPlacement::trailing(following, comment); - } else if following.is_stmt_if() { - // We have to exclude for following if statements that are not elif by checking the - // indentation - // ```python - // if True: - // pass - // else: # Comment - // if False: - // pass - // pass - // ``` - let base_if_indent = - whitespace::indentation_at_offset(locator, following.range().start()); - let maybe_elif_indent = whitespace::indentation_at_offset( - locator, - comment.enclosing_node().range().start(), - ); - if base_if_indent == maybe_elif_indent { - return CommentPlacement::trailing(following, comment); + } + + // Handle the `else` of an `if`. It is special because we don't have a test but unlike other + // `else` (e.g. for `while`), we have a dedicated node. + // ```python + // if x == y: + // pass + // elif x < y: + // pass + // else: # 12 trailing else condition + // pass + // ``` + if let AnyNodeRef::StmtIf(stmt_if) = comment.enclosing_node() { + if let Some(else_clause) = stmt_if.elif_else_clauses.last() { + if else_clause.test.is_none() + && following.ptr_eq(else_clause.body.first().unwrap().into()) + { + return CommentPlacement::dangling(else_clause.into(), comment); + } } } - // There are no bodies for the "else" branch and other bodies that are represented as a `Vec`. - // This means, there's no good place to attach the comments to. - // Make this a dangling comments and manually format the comment in - // in the enclosing node's formatting logic. For `try`, it's the formatters responsibility - // to correctly identify the comments for the `finally` and `orelse` block by looking - // at the comment's range. + + // There are no bodies for the "else" branch (only `Vec`) expect for StmtIf, so + // we make this a dangling comments of the node containing the alternate branch and + // manually format the comment in that node's formatting logic. For `try`, it's the + // formatters responsibility to correctly identify the comments for the `finally` and + // `orelse` block by looking at the comment's range. // // ```python // while x == y: @@ -425,6 +439,64 @@ fn handle_in_between_bodies_end_of_line_comment<'a>( } } +/// Without the `StmtIf` special, this function would just be the following: +/// ```ignore +/// if let Some(preceding_node) = comment.preceding_node() { +/// Some((preceding_node, last_child_in_body(preceding_node)?)) +/// } else { +/// None +/// } +/// ``` +/// We handle two special cases here: +/// ```python +/// if True: +/// pass +/// # Comment between if and elif/else clause, needs to be manually attached to the `StmtIf` +/// else: +/// pass +/// # Comment after the `StmtIf`, needs to be manually attached to the ElifElseClause +/// ``` +/// The problem is that `StmtIf` spans the whole range (there is no "inner if" node), so the first +/// comment doesn't see it as preceding node, and the second comment takes the entire `StmtIf` when +/// it should only take the `ElifElseClause` +fn find_preceding_and_handle_stmt_if_special_cases<'a>( + comment: &DecoratedComment<'a>, +) -> Option<(AnyNodeRef<'a>, AnyNodeRef<'a>)> { + if let (stmt_if @ AnyNodeRef::StmtIf(stmt_if_inner), Some(AnyNodeRef::ElifElseClause(..))) = + (comment.enclosing_node(), comment.following_node()) + { + if let Some(preceding_node @ AnyNodeRef::ElifElseClause(..)) = comment.preceding_node() { + // We're already after and elif or else, defaults work + Some((preceding_node, last_child_in_body(preceding_node)?)) + } else { + // Special case 1: The comment is between if body and an elif/else clause. We have + // to handle this separately since StmtIf spans the entire range, so it's not the + // preceding node + Some(( + stmt_if, + AnyNodeRef::from(stmt_if_inner.body.last().unwrap()), + )) + } + } else if let Some(preceding_node @ AnyNodeRef::StmtIf(stmt_if_inner)) = + comment.preceding_node() + { + if let Some(clause) = stmt_if_inner.elif_else_clauses.last() { + // Special case 2: We're after an if statement and need to narrow the preceding + // down to the elif/else clause + Some((clause.into(), last_child_in_body(clause.into())?)) + } else { + // After an if without any elif/else, defaults work + Some((preceding_node, last_child_in_body(preceding_node)?)) + } + } else if let Some(preceding_node) = comment.preceding_node() { + // The normal case + Some((preceding_node, last_child_in_body(preceding_node)?)) + } else { + // Only do something if the preceding node has a body (has indented statements). + None + } +} + /// Handles trailing comments at the end of a body block (or any other block that is indented). /// ```python /// def test(): @@ -442,12 +514,9 @@ fn handle_trailing_body_comment<'a>( return CommentPlacement::Default(comment); } - // Only do something if the preceding node has a body (has indented statements). - let Some(preceding_node) = comment.preceding_node() else { - return CommentPlacement::Default(comment); - }; - - let Some(last_child) = last_child_in_body(preceding_node) else { + let Some((preceding_node, last_child)) = + find_preceding_and_handle_stmt_if_special_cases(&comment) + else { return CommentPlacement::Default(comment); }; @@ -566,6 +635,22 @@ fn handle_trailing_end_of_line_body_comment<'a>( return CommentPlacement::Default(comment); }; + // Handle the StmtIf special case + // ```python + // if True: + // pass + // elif True: + // pass # 14 end-of-line trailing `pass` comment, set preceding to the ElifElseClause + // ``` + let preceding = if let AnyNodeRef::StmtIf(stmt_if) = preceding { + stmt_if + .elif_else_clauses + .last() + .map_or(preceding, AnyNodeRef::from) + } else { + preceding + }; + // Recursively get the last child of statements with a body. let last_children = std::iter::successors(last_child_in_body(preceding), |parent| { last_child_in_body(*parent) @@ -600,20 +685,40 @@ fn handle_trailing_end_of_line_condition_comment<'a>( return CommentPlacement::Default(comment); } + // We handle trailing else comments separately because we the preceding node is None for their + // case + // ```python + // if True: + // pass + // else: # 12 trailing else condition + // pass + // ``` + if let AnyNodeRef::ElifElseClause(ast::ElifElseClause { + body, test: None, .. + }) = comment.enclosing_node() + { + if comment.start() < body.first().unwrap().start() { + return CommentPlacement::dangling(comment.enclosing_node(), comment); + } + } + // Must be between the condition expression and the first body element let (Some(preceding), Some(following)) = (comment.preceding_node(), comment.following_node()) else { return CommentPlacement::Default(comment); }; - let expression_before_colon = match comment.enclosing_node() { + let enclosing_node = comment.enclosing_node(); + let expression_before_colon = match enclosing_node { + AnyNodeRef::ElifElseClause(ast::ElifElseClause { + test: Some(expr), .. + }) => Some(AnyNodeRef::from(expr)), AnyNodeRef::StmtIf(ast::StmtIf { test: expr, .. }) | AnyNodeRef::StmtWhile(ast::StmtWhile { test: expr, .. }) | AnyNodeRef::StmtFor(ast::StmtFor { iter: expr, .. }) | AnyNodeRef::StmtAsyncFor(ast::StmtAsyncFor { iter: expr, .. }) => { Some(AnyNodeRef::from(expr.as_ref())) } - AnyNodeRef::StmtWith(ast::StmtWith { items, .. }) | AnyNodeRef::StmtAsyncWith(ast::StmtAsyncWith { items, .. }) => { items.last().map(AnyNodeRef::from) @@ -656,7 +761,7 @@ fn handle_trailing_end_of_line_condition_comment<'a>( // while a: # comment // ... // ``` - return CommentPlacement::dangling(comment.enclosing_node(), comment); + return CommentPlacement::dangling(enclosing_node, comment); } // Comment comes before the colon @@ -1439,10 +1544,15 @@ fn last_child_in_body(node: AnyNodeRef) -> Option { | AnyNodeRef::MatchCase(ast::MatchCase { body, .. }) | AnyNodeRef::ExceptHandlerExceptHandler(ast::ExceptHandlerExceptHandler { body, .. - }) => body, + }) + | AnyNodeRef::ElifElseClause(ast::ElifElseClause { body, .. }) => body, + AnyNodeRef::StmtIf(ast::StmtIf { + body, + elif_else_clauses, + .. + }) => elif_else_clauses.last().map_or(body, |clause| &clause.body), - AnyNodeRef::StmtIf(ast::StmtIf { body, orelse, .. }) - | AnyNodeRef::StmtFor(ast::StmtFor { body, orelse, .. }) + AnyNodeRef::StmtFor(ast::StmtFor { body, orelse, .. }) | AnyNodeRef::StmtAsyncFor(ast::StmtAsyncFor { body, orelse, .. }) | AnyNodeRef::StmtWhile(ast::StmtWhile { body, orelse, .. }) => { if orelse.is_empty() { @@ -1453,7 +1563,7 @@ fn last_child_in_body(node: AnyNodeRef) -> Option { } AnyNodeRef::StmtMatch(ast::StmtMatch { cases, .. }) => { - return cases.last().map(AnyNodeRef::from) + return cases.last().map(AnyNodeRef::from); } AnyNodeRef::StmtTry(ast::StmtTry { @@ -1498,8 +1608,26 @@ fn is_first_statement_in_enclosing_alternate_body( enclosing: AnyNodeRef, ) -> bool { match enclosing { - AnyNodeRef::StmtIf(ast::StmtIf { orelse, .. }) - | AnyNodeRef::StmtFor(ast::StmtFor { orelse, .. }) + AnyNodeRef::StmtIf(ast::StmtIf { + elif_else_clauses, .. + }) => { + for clause in elif_else_clauses { + if let Some(test) = &clause.test { + // `elif`, the following node is the test + if following.ptr_eq(test.into()) { + return true; + } + } else { + // `else`, there is no test and the following node is the first entry in the + // body + if following.ptr_eq(clause.body.first().unwrap().into()) { + return true; + } + } + } + false + } + AnyNodeRef::StmtFor(ast::StmtFor { orelse, .. }) | AnyNodeRef::StmtAsyncFor(ast::StmtAsyncFor { orelse, .. }) | AnyNodeRef::StmtWhile(ast::StmtWhile { orelse, .. }) => { are_same_optional(following, orelse.first()) diff --git a/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__base_test.snap b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__base_test.snap index ef24b1dbef..3ace969501 100644 --- a/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__base_test.snap +++ b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__base_test.snap @@ -34,8 +34,8 @@ expression: comments.debug(test_case.source_code) "trailing": [], }, Node { - kind: StmtIf, - range: 144..212, + kind: ElifElseClause, + range: 144..177, source: `elif x < y:⏎`, }: { "leading": [ diff --git a/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__if_elif_else_comments.snap b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__if_elif_else_comments.snap index af56bdb4b9..99d380cc70 100644 --- a/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__if_elif_else_comments.snap +++ b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__if_elif_else_comments.snap @@ -24,8 +24,8 @@ expression: comments.debug(test_case.source_code) ], }, Node { - kind: StmtIf, - range: 104..192, + kind: ElifElseClause, + range: 104..124, source: `elif x < y:⏎`, }: { "leading": [ @@ -35,13 +35,7 @@ expression: comments.debug(test_case.source_code) formatted: false, }, ], - "dangling": [ - SourceComment { - text: "# Leading else comment", - position: OwnLine, - formatted: false, - }, - ], + "dangling": [], "trailing": [], }, Node { @@ -59,6 +53,21 @@ expression: comments.debug(test_case.source_code) }, ], }, + Node { + kind: ElifElseClause, + range: 178..192, + source: `else:⏎`, + }: { + "leading": [ + SourceComment { + text: "# Leading else comment", + position: OwnLine, + formatted: false, + }, + ], + "dangling": [], + "trailing": [], + }, Node { kind: StmtPass, range: 188..192, diff --git a/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__if_elif_if_else_comments.snap b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__if_elif_if_else_comments.snap index 1aa79f4f4e..1cd1f74ac2 100644 --- a/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__if_elif_if_else_comments.snap +++ b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__if_elif_if_else_comments.snap @@ -3,21 +3,6 @@ source: crates/ruff_python_formatter/src/comments/mod.rs expression: comments.debug(test_case.source_code) --- { - Node { - kind: StmtIf, - range: 21..128, - source: `elif x < y:⏎`, - }: { - "leading": [], - "dangling": [ - SourceComment { - text: "# Leading else comment", - position: OwnLine, - formatted: false, - }, - ], - "trailing": [], - }, Node { kind: StmtIf, range: 37..60, @@ -33,4 +18,19 @@ expression: comments.debug(test_case.source_code) }, ], }, + Node { + kind: ElifElseClause, + range: 114..128, + source: `else:⏎`, + }: { + "leading": [ + SourceComment { + text: "# Leading else comment", + position: OwnLine, + formatted: false, + }, + ], + "dangling": [], + "trailing": [], + }, } diff --git a/crates/ruff_python_formatter/src/comments/visitor.rs b/crates/ruff_python_formatter/src/comments/visitor.rs index 947dc4b31b..28218bb985 100644 --- a/crates/ruff_python_formatter/src/comments/visitor.rs +++ b/crates/ruff_python_formatter/src/comments/visitor.rs @@ -2,8 +2,8 @@ use std::iter::Peekable; use ruff_text_size::{TextRange, TextSize}; use rustpython_parser::ast::{ - Alias, Arg, ArgWithDefault, Arguments, Comprehension, Decorator, ExceptHandler, Expr, Keyword, - MatchCase, Mod, Pattern, Ranged, Stmt, WithItem, + Alias, Arg, ArgWithDefault, Arguments, Comprehension, Decorator, ElifElseClause, ExceptHandler, + Expr, Keyword, MatchCase, Mod, Pattern, Ranged, Stmt, WithItem, }; use ruff_formatter::{SourceCode, SourceCodeSlice}; @@ -284,6 +284,13 @@ impl<'ast> PreorderVisitor<'ast> for CommentsVisitor<'ast> { } self.finish_node(pattern); } + + fn visit_elif_else_clause(&mut self, elif_else_clause: &'ast ElifElseClause) { + if self.start_node(elif_else_clause).is_traverse() { + walk_elif_else_clause(self, elif_else_clause); + } + self.finish_node(elif_else_clause); + } } fn text_position(comment_range: TextRange, source_code: SourceCode) -> CommentLinePosition { diff --git a/crates/ruff_python_formatter/src/generated.rs b/crates/ruff_python_formatter/src/generated.rs index c9bb761e30..e83c2746fb 100644 --- a/crates/ruff_python_formatter/src/generated.rs +++ b/crates/ruff_python_formatter/src/generated.rs @@ -617,6 +617,46 @@ impl<'ast> IntoFormat> for ast::StmtIf { } } +impl FormatRule> + for crate::statement::stmt_if::FormatElifElseClause +{ + #[inline] + fn fmt( + &self, + node: &ast::ElifElseClause, + f: &mut Formatter>, + ) -> FormatResult<()> { + FormatNodeRule::::fmt(self, node, f) + } +} +impl<'ast> AsFormat> for ast::ElifElseClause { + type Format<'a> = FormatRefWithRule< + 'a, + ast::ElifElseClause, + crate::statement::stmt_if::FormatElifElseClause, + PyFormatContext<'ast>, + >; + fn format(&self) -> Self::Format<'_> { + FormatRefWithRule::new( + self, + crate::statement::stmt_if::FormatElifElseClause::default(), + ) + } +} +impl<'ast> IntoFormat> for ast::ElifElseClause { + type Format = FormatOwnedWithRule< + ast::ElifElseClause, + crate::statement::stmt_if::FormatElifElseClause, + PyFormatContext<'ast>, + >; + fn into_format(self) -> Self::Format { + FormatOwnedWithRule::new( + self, + crate::statement::stmt_if::FormatElifElseClause::default(), + ) + } +} + impl FormatRule> for crate::statement::stmt_with::FormatStmtWith { diff --git a/crates/ruff_python_formatter/src/statement/mod.rs b/crates/ruff_python_formatter/src/statement/mod.rs index 4d5804a847..d0e0130da9 100644 --- a/crates/ruff_python_formatter/src/statement/mod.rs +++ b/crates/ruff_python_formatter/src/statement/mod.rs @@ -64,6 +64,7 @@ impl FormatRule> for FormatStmt { Stmt::Pass(x) => x.format().fmt(f), Stmt::Break(x) => x.format().fmt(f), Stmt::Continue(x) => x.format().fmt(f), + Stmt::TypeAlias(_) => todo!(), } } } diff --git a/crates/ruff_python_formatter/src/statement/stmt_class_def.rs b/crates/ruff_python_formatter/src/statement/stmt_class_def.rs index 4f14e9df45..86c7688284 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_class_def.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_class_def.rs @@ -18,6 +18,7 @@ impl FormatNodeRule for FormatStmtClassDef { bases, keywords, body, + type_params: _, decorator_list, } = item; diff --git a/crates/ruff_python_formatter/src/statement/stmt_if.rs b/crates/ruff_python_formatter/src/statement/stmt_if.rs index ab249f228e..4e1391cbe6 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_if.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_if.rs @@ -1,91 +1,43 @@ -use crate::comments::{leading_alternate_branch_comments, trailing_comments, SourceComment}; +use crate::comments::{leading_alternate_branch_comments, trailing_comments}; use crate::expression::maybe_parenthesize_expression; use crate::expression::parentheses::Parenthesize; use crate::prelude::*; use crate::FormatNodeRule; -use ruff_formatter::{write, FormatError}; -use rustpython_parser::ast::{Ranged, Stmt, StmtIf, Suite}; +use ruff_formatter::write; +use ruff_python_ast::node::AnyNodeRef; +use rustpython_parser::ast::{ElifElseClause, StmtIf}; #[derive(Default)] pub struct FormatStmtIf; impl FormatNodeRule for FormatStmtIf { fn fmt_fields(&self, item: &StmtIf, f: &mut PyFormatter) -> FormatResult<()> { + let StmtIf { + range: _, + test, + body, + elif_else_clauses, + } = item; + let comments = f.context().comments().clone(); + let trailing_colon_comment = comments.dangling_comments(item); - let mut current = IfOrElIf::If(item); - let mut else_comments: &[SourceComment]; - let mut last_node_of_previous_body = None; + write!( + f, + [ + text("if"), + space(), + maybe_parenthesize_expression(test, item, Parenthesize::IfBreaks), + text(":"), + trailing_comments(trailing_colon_comment), + block_indent(&body.format()) + ] + )?; - loop { - let current_statement = current.statement(); - let StmtIf { - test, body, orelse, .. - } = current_statement; - - let first_statement = body.first().ok_or(FormatError::SyntaxError)?; - let trailing = comments.dangling_comments(current_statement); - - let trailing_if_comments_end = trailing - .partition_point(|comment| comment.slice().start() < first_statement.start()); - - let (if_trailing_comments, trailing_alternate_comments) = - trailing.split_at(trailing_if_comments_end); - - if current.is_elif() { - let elif_leading = comments.leading_comments(current_statement); - // Manually format the leading comments because the formatting bypasses `NodeRule::fmt` - write!( - f, - [ - leading_alternate_branch_comments(elif_leading, last_node_of_previous_body), - source_position(current_statement.start()) - ] - )?; - } - - write!( - f, - [ - text(current.keyword()), - space(), - maybe_parenthesize_expression(test, current_statement, Parenthesize::IfBreaks), - text(":"), - trailing_comments(if_trailing_comments), - block_indent(&body.format()) - ] - )?; - - // RustPython models `elif` by setting the body to a single `if` statement. The `orelse` - // of the most inner `if` statement then becomes the `else` of the whole `if` chain. - // That's why it's necessary to take the comments here from the most inner `elif`. - else_comments = trailing_alternate_comments; - last_node_of_previous_body = body.last(); - - if let Some(elif) = else_if(orelse) { - current = elif; - } else { - break; - } - } - - let orelse = ¤t.statement().orelse; - - if !orelse.is_empty() { - // Leading comments are always own line comments - let leading_else_comments_end = - else_comments.partition_point(|comment| comment.line_position().is_own_line()); - let (else_leading, else_trailing) = else_comments.split_at(leading_else_comments_end); - - write!( - f, - [ - leading_alternate_branch_comments(else_leading, last_node_of_previous_body), - text("else:"), - trailing_comments(else_trailing), - block_indent(&orelse.format()) - ] - )?; + let mut last_node = body.last().unwrap().into(); + for clause in elif_else_clauses { + format_elif_else_clause(clause, f, Some(last_node))?; + last_node = clause.body.last().unwrap().into(); } Ok(()) @@ -97,35 +49,56 @@ impl FormatNodeRule for FormatStmtIf { } } -fn else_if(or_else: &Suite) -> Option { - if let [Stmt::If(if_stmt)] = or_else.as_slice() { - Some(IfOrElIf::ElIf(if_stmt)) +/// Note that this implementation misses the leading newlines before the leading comments because +/// it does not have access to the last node of the previous branch. The `StmtIf` therefore doesn't +/// call this but `format_elif_else_clause` directly. +#[derive(Default)] +pub struct FormatElifElseClause; + +impl FormatNodeRule for FormatElifElseClause { + fn fmt_fields(&self, item: &ElifElseClause, f: &mut PyFormatter) -> FormatResult<()> { + format_elif_else_clause(item, f, None) + } +} + +/// Extracted so we can implement `FormatElifElseClause` but also pass in `last_node` from +/// `FormatStmtIf` +fn format_elif_else_clause( + item: &ElifElseClause, + f: &mut PyFormatter, + last_node: Option, +) -> FormatResult<()> { + let ElifElseClause { + range: _, + test, + body, + } = item; + + let comments = f.context().comments().clone(); + let trailing_colon_comment = comments.dangling_comments(item); + let leading_comments = comments.leading_comments(item); + + leading_alternate_branch_comments(leading_comments, last_node).fmt(f)?; + + if let Some(test) = test { + write!( + f, + [ + text("elif"), + space(), + maybe_parenthesize_expression(test, item, Parenthesize::IfBreaks), + ] + )?; } else { - None - } -} - -enum IfOrElIf<'a> { - If(&'a StmtIf), - ElIf(&'a StmtIf), -} - -impl<'a> IfOrElIf<'a> { - const fn statement(&self) -> &'a StmtIf { - match self { - IfOrElIf::If(statement) => statement, - IfOrElIf::ElIf(statement) => statement, - } - } - - const fn keyword(&self) -> &'static str { - match self { - IfOrElIf::If(_) => "if", - IfOrElIf::ElIf(_) => "elif", - } - } - - const fn is_elif(&self) -> bool { - matches!(self, IfOrElIf::ElIf(_)) + text("else").fmt(f)?; } + + write!( + f, + [ + text(":"), + trailing_comments(trailing_colon_comment), + block_indent(&body.format()) + ] + ) } diff --git a/crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap index 341956733f..31226f41f7 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap @@ -353,7 +353,7 @@ else: if True: def f2(): pass - # a + # a else: pass diff --git a/crates/ruff_python_formatter/tests/snapshots/format@statement__if.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@statement__if.py.snap index 9bbe6803f2..a695caa4e1 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@statement__if.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@statement__if.py.snap @@ -4,19 +4,23 @@ input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/ --- ## Input ```py -if x == y: # trailing if condition - pass # trailing `pass` comment - # Root `if` trailing comment +# 1 leading if comment +if x == y: # 2 trailing if condition + # 3 leading pass + pass # 4 end-of-line trailing `pass` comment + # 5 Root `if` trailing comment -# Leading elif comment -elif x < y: # trailing elif condition - pass - # `elif` trailing comment +# 6 Leading elif comment +elif x < y: # 7 trailing elif condition + # 8 leading pass + pass # 9 end-of-line trailing `pass` comment + # 10 `elif` trailing comment -# Leading else comment -else: # trailing else condition - pass - # `else` trailing comment +# 11 Leading else comment +else: # 12 trailing else condition + # 13 leading pass + pass # 14 end-of-line trailing `pass` comment + # 15 `else` trailing comment if x == y: @@ -77,23 +81,38 @@ else: # Comment if False: pass pass + + +# Regression test for `last_child_in_body` special casing of `StmtIf` +# https://github.com/python/cpython/blob/aecf6aca515a203a823a87c711f15cbb82097c8b/Lib/test/test_pty.py#L260-L275 +def f(): + if True: + pass + else: + pass + + # comment ``` ## Output ```py -if x == y: # trailing if condition - pass # trailing `pass` comment - # Root `if` trailing comment +# 1 leading if comment +if x == y: # 2 trailing if condition + # 3 leading pass + pass # 4 end-of-line trailing `pass` comment + # 5 Root `if` trailing comment -# Leading elif comment -elif x < y: # trailing elif condition - pass - # `elif` trailing comment +# 6 Leading elif comment +elif x < y: # 7 trailing elif condition + # 8 leading pass + pass # 9 end-of-line trailing `pass` comment + # 10 `elif` trailing comment -# Leading else comment -else: # trailing else condition - pass - # `else` trailing comment +# 11 Leading else comment +else: # 12 trailing else condition + # 13 leading pass + pass # 14 end-of-line trailing `pass` comment + # 15 `else` trailing comment if x == y: @@ -153,6 +172,17 @@ else: # Comment if False: pass pass + + +# Regression test for `last_child_in_body` special casing of `StmtIf` +# https://github.com/python/cpython/blob/aecf6aca515a203a823a87c711f15cbb82097c8b/Lib/test/test_pty.py#L260-L275 +def f(): + if True: + pass + else: + pass + + # comment ``` diff --git a/crates/ruff_python_semantic/src/analyze/branch_detection.rs b/crates/ruff_python_semantic/src/analyze/branch_detection.rs index 62ffb8e116..5314f2dce5 100644 --- a/crates/ruff_python_semantic/src/analyze/branch_detection.rs +++ b/crates/ruff_python_semantic/src/analyze/branch_detection.rs @@ -1,4 +1,5 @@ use std::cmp::Ordering; +use std::iter; use rustpython_parser::ast::{self, ExceptHandler, Stmt}; @@ -42,7 +43,17 @@ fn common_ancestor( /// Return the alternative branches for a given node. fn alternatives(stmt: &Stmt) -> Vec> { match stmt { - Stmt::If(ast::StmtIf { body, .. }) => vec![body.iter().collect()], + Stmt::If(ast::StmtIf { + body, + elif_else_clauses, + .. + }) => iter::once(body.iter().collect()) + .chain( + elif_else_clauses + .iter() + .map(|clause| clause.body.iter().collect()), + ) + .collect(), Stmt::Try(ast::StmtTry { body, handlers, diff --git a/crates/ruff_wasm/tests/api.rs b/crates/ruff_wasm/tests/api.rs index fd2a70c4ff..35c72dfc10 100644 --- a/crates/ruff_wasm/tests/api.rs +++ b/crates/ruff_wasm/tests/api.rs @@ -29,11 +29,11 @@ fn empty_config() { message: "If test is a tuple, which is always `True`".to_string(), location: SourceLocation { row: OneIndexed::from_zero_indexed(0), - column: OneIndexed::from_zero_indexed(0) + column: OneIndexed::from_zero_indexed(3) }, end_location: SourceLocation { - row: OneIndexed::from_zero_indexed(1), - column: OneIndexed::from_zero_indexed(8) + row: OneIndexed::from_zero_indexed(0), + column: OneIndexed::from_zero_indexed(9) }, fix: None, }] diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index 27f3f16930..7e5c73aff0 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -24,8 +24,8 @@ ruff_python_ast = { path = "../crates/ruff_python_ast" } ruff_python_formatter = { path = "../crates/ruff_python_formatter" } similar = { version = "2.2.1" } -# Current tag: v0.0.7 -rustpython-parser = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "b996b21ffca562ecb2086f632a6a0b05c245c24a" , default-features = false, features = ["full-lexer", "num-bigint"] } +# Current tag: v0.0.9 +rustpython-parser = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "db04fd415774032e1e2ceb03bcbf5305e0d22c8c" , default-features = false, features = ["full-lexer", "num-bigint"] } # Prevent this from interfering with workspaces [workspace]