diff --git a/.markdownlint.yaml b/.markdownlint.yaml index 637134b0f9373..15656efe736ec 100644 --- a/.markdownlint.yaml +++ b/.markdownlint.yaml @@ -17,4 +17,4 @@ MD013: false # MD024/no-duplicate-heading MD024: # Allow when nested under different parents e.g. CHANGELOG.md - allow_different_nesting: true + siblings_only: true diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8759c0002625e..3b65abf3510b7 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -13,7 +13,7 @@ exclude: | repos: - repo: https://github.com/abravalheri/validate-pyproject - rev: v0.15 + rev: v0.16 hooks: - id: validate-pyproject @@ -31,7 +31,7 @@ repos: )$ - repo: https://github.com/igorshubovych/markdownlint-cli - rev: v0.37.0 + rev: v0.39.0 hooks: - id: markdownlint-fix exclude: | @@ -41,7 +41,7 @@ repos: )$ - repo: https://github.com/crate-ci/typos - rev: v1.16.22 + rev: v1.20.4 hooks: - id: typos @@ -55,7 +55,7 @@ repos: pass_filenames: false # This makes it a lot faster - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.1.4 + rev: v0.3.5 hooks: - id: ruff-format - id: ruff @@ -70,7 +70,7 @@ repos: # Prettier - repo: https://github.com/pre-commit/mirrors-prettier - rev: v3.0.3 + rev: v3.1.0 hooks: - id: prettier types: [yaml] diff --git a/Cargo.lock b/Cargo.lock index d3a0680b71b3c..a3480feb0542d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -365,7 +365,7 @@ dependencies = [ "heck 0.5.0", "proc-macro2", "quote", - "syn 2.0.57", + "syn 2.0.58", ] [[package]] @@ -596,7 +596,7 @@ dependencies = [ "proc-macro2", "quote", "strsim 0.10.0", - "syn 2.0.57", + "syn 2.0.58", ] [[package]] @@ -607,7 +607,7 @@ checksum = "a668eda54683121533a393014d8692171709ff57a7d61f187b6e782719f8933f" dependencies = [ "darling_core", "quote", - "syn 2.0.57", + "syn 2.0.58", ] [[package]] @@ -1108,7 +1108,7 @@ dependencies = [ "Inflector", "proc-macro2", "quote", - "syn 2.0.57", + "syn 2.0.58", ] [[package]] @@ -1271,9 +1271,9 @@ checksum = "9c198f91728a82281a64e1f4f9eeb25d82cb32a5de251c6bd1b5154d63a8e7bd" [[package]] name = "libcst" -version = "1.2.0" +version = "1.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "890ee958b936e712c6f1c184f208176973e73c2e4f8d3cf499f94eb112f647f9" +checksum = "6f1e25d1b119ab5c2f15a6e081bb94a8d547c5c2ad065f5fd0dbb683f31ced91" dependencies = [ "chic", "libcst_derive", @@ -1286,12 +1286,12 @@ dependencies = [ [[package]] name = "libcst_derive" -version = "1.2.0" +version = "1.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1dbd2f3cd9346422ebdc3a614aed6969d4e0b3e9c10517f33b30326acf894c11" +checksum = "4a5011f2d59093de14a4a90e01b9d85dee9276e58a25f0107dcee167dd601be0" dependencies = [ "quote", - "syn 2.0.57", + "syn 2.0.58", ] [[package]] @@ -1437,12 +1437,6 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e4a24736216ec316047a1fc4252e27dabb04218aa4a3f37c6e7ddbf1f9782b54" -[[package]] -name = "nextest-workspace-hack" -version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d906846a98739ed9d73d66e62c2641eef8321f1734b7a1156ab045a0248fb2b3" - [[package]] name = "nix" version = "0.26.4" @@ -1757,7 +1751,7 @@ checksum = "52a40bc70c2c58040d2d8b167ba9a5ff59fc9dab7ad44771cfde3dcfde7a09c6" dependencies = [ "proc-macro2", "quote", - "syn 2.0.57", + "syn 2.0.58", ] [[package]] @@ -1812,13 +1806,12 @@ dependencies = [ [[package]] name = "quick-junit" -version = "0.3.5" +version = "0.3.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1b9599bffc2cd7511355996e0cfd979266b2cfa3f3ff5247d07a3a6e1ded6158" +checksum = "d1a341ae463320e9f8f34adda49c8a85d81d4e8f34cce4397fb0350481552224" dependencies = [ "chrono", "indexmap", - "nextest-workspace-hack", "quick-xml", "strip-ansi-escapes", "thiserror", @@ -1975,7 +1968,7 @@ dependencies = [ "pmutil", "proc-macro2", "quote", - "syn 2.0.57", + "syn 2.0.58", ] [[package]] @@ -2225,7 +2218,7 @@ dependencies = [ "proc-macro2", "quote", "ruff_python_trivia", - "syn 2.0.57", + "syn 2.0.58", ] [[package]] @@ -2671,7 +2664,7 @@ checksum = "7eb0b34b42edc17f6b7cac84a52a1c5f0e1bb2227e997ca9011ea3dd34e8610b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.57", + "syn 2.0.58", ] [[package]] @@ -2704,7 +2697,7 @@ checksum = "0b2e6b945e9d3df726b65d6ee24060aff8e3533d431f677a9695db04eff9dfdb" dependencies = [ "proc-macro2", "quote", - "syn 2.0.57", + "syn 2.0.58", ] [[package]] @@ -2745,7 +2738,7 @@ dependencies = [ "darling", "proc-macro2", "quote", - "syn 2.0.57", + "syn 2.0.58", ] [[package]] @@ -2855,7 +2848,7 @@ dependencies = [ "proc-macro2", "quote", "rustversion", - "syn 2.0.57", + "syn 2.0.58", ] [[package]] @@ -2877,9 +2870,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.57" +version = "2.0.58" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "11a6ae1e52eb25aab8f3fb9fca13be982a373b8f1157ca14b897a825ba4a2d35" +checksum = "44cfb93f38070beee36b3fef7d4f5a16f27751d94b187b666a5cc5e9b0d30687" dependencies = [ "proc-macro2", "quote", @@ -2950,7 +2943,7 @@ dependencies = [ "cfg-if", "proc-macro2", "quote", - "syn 2.0.57", + "syn 2.0.58", ] [[package]] @@ -2961,7 +2954,7 @@ checksum = "5c89e72a01ed4c579669add59014b9a524d609c0c88c6a585ce37485879f6ffb" dependencies = [ "proc-macro2", "quote", - "syn 2.0.57", + "syn 2.0.58", "test-case-core", ] @@ -2982,7 +2975,7 @@ checksum = "c61f3ba182994efc43764a46c018c347bc492c79f024e705f46567b418f6d4f7" dependencies = [ "proc-macro2", "quote", - "syn 2.0.57", + "syn 2.0.58", ] [[package]] @@ -3103,7 +3096,7 @@ checksum = "34704c8d6ebcbc939824180af020566b01a7c01f80641264eba0999f6c2b6be7" dependencies = [ "proc-macro2", "quote", - "syn 2.0.57", + "syn 2.0.58", ] [[package]] @@ -3339,7 +3332,7 @@ checksum = "9881bea7cbe687e36c9ab3b778c36cd0487402e270304e8b1296d5085303c1a2" dependencies = [ "proc-macro2", "quote", - "syn 2.0.57", + "syn 2.0.58", ] [[package]] @@ -3424,7 +3417,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn 2.0.57", + "syn 2.0.58", "wasm-bindgen-shared", ] @@ -3458,7 +3451,7 @@ checksum = "e94f17b526d0a461a191c78ea52bbce64071ed5c04c9ffe424dcb38f74171bb7" dependencies = [ "proc-macro2", "quote", - "syn 2.0.57", + "syn 2.0.58", "wasm-bindgen-backend", "wasm-bindgen-shared", ] @@ -3491,7 +3484,7 @@ checksum = "b7f89739351a2e03cb94beb799d47fb2cac01759b40ec441f7de39b00cbf7ef0" dependencies = [ "proc-macro2", "quote", - "syn 2.0.57", + "syn 2.0.58", ] [[package]] @@ -3747,7 +3740,7 @@ checksum = "9ce1b18ccd8e73a9321186f97e46f9f04b778851177567b1975109d26a08d2a6" dependencies = [ "proc-macro2", "quote", - "syn 2.0.57", + "syn 2.0.58", ] [[package]] diff --git a/_typos.toml b/_typos.toml index 1987a214db5d4..f239540e3af36 100644 --- a/_typos.toml +++ b/_typos.toml @@ -3,9 +3,11 @@ extend-exclude = ["**/resources/**/*", "**/snapshots/**/*"] [default.extend-words] +"arange" = "arange" # e.g. `numpy.arange` hel = "hel" whos = "whos" spawnve = "spawnve" ned = "ned" +pn = "pn" # `import panel as pd` is a thing poit = "poit" BA = "BA" # acronym for "Bad Allowed", used in testing. diff --git a/crates/ruff/src/lib.rs b/crates/ruff/src/lib.rs index 1a9a333d232b4..42d0e61185861 100644 --- a/crates/ruff/src/lib.rs +++ b/crates/ruff/src/lib.rs @@ -149,6 +149,13 @@ pub fn run( #[cfg(windows)] assert!(colored::control::set_virtual_terminal(true).is_ok()); + // support FORCE_COLOR env var + if let Some(force_color) = std::env::var_os("FORCE_COLOR") { + if force_color.len() > 0 { + colored::control::set_override(true); + } + } + set_up_logging(global_options.log_level())?; if let Some(deprecated_alias_warning) = deprecated_alias_warning { diff --git a/crates/ruff/tests/lint.rs b/crates/ruff/tests/lint.rs index a32c46aeed4c3..d02b112f934dc 100644 --- a/crates/ruff/tests/lint.rs +++ b/crates/ruff/tests/lint.rs @@ -1168,3 +1168,83 @@ def func(): Ok(()) } + +/// Per-file selects via ! negation in per-file-ignores +#[test] +fn negated_per_file_ignores() -> Result<()> { + let tempdir = TempDir::new()?; + let ruff_toml = tempdir.path().join("ruff.toml"); + fs::write( + &ruff_toml, + r#" +[lint.per-file-ignores] +"!selected.py" = ["RUF"] +"#, + )?; + let selected = tempdir.path().join("selected.py"); + fs::write(selected, "")?; + let ignored = tempdir.path().join("ignored.py"); + fs::write(ignored, "")?; + + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .arg("--config") + .arg(&ruff_toml) + .arg("--select") + .arg("RUF901") + .current_dir(&tempdir) + , @r###" + success: false + exit_code: 1 + ----- stdout ----- + selected.py:1:1: RUF901 [*] Hey this is a stable test rule with a safe fix. + Found 1 error. + [*] 1 fixable with the `--fix` option. + + ----- stderr ----- + "###); + Ok(()) +} + +#[test] +fn negated_per_file_ignores_absolute() -> Result<()> { + let tempdir = TempDir::new()?; + let ruff_toml = tempdir.path().join("ruff.toml"); + fs::write( + &ruff_toml, + r#" +[lint.per-file-ignores] +"!src/**.py" = ["RUF"] +"#, + )?; + let src_dir = tempdir.path().join("src"); + fs::create_dir(&src_dir)?; + let selected = src_dir.join("selected.py"); + fs::write(selected, "")?; + let ignored = tempdir.path().join("ignored.py"); + fs::write(ignored, "")?; + + insta::with_settings!({filters => vec![ + // Replace windows paths + (r"\\", "/"), + ]}, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .arg("--config") + .arg(&ruff_toml) + .arg("--select") + .arg("RUF901") + .current_dir(&tempdir) + , @r###" + success: false + exit_code: 1 + ----- stdout ----- + src/selected.py:1:1: RUF901 [*] Hey this is a stable test rule with a safe fix. + Found 1 error. + [*] 1 fixable with the `--fix` option. + + ----- stderr ----- + "###); + }); + Ok(()) +} diff --git a/crates/ruff/tests/snapshots/show_settings__display_default_settings.snap b/crates/ruff/tests/snapshots/show_settings__display_default_settings.snap index 1c0402e16305b..adff7742ce4b8 100644 --- a/crates/ruff/tests/snapshots/show_settings__display_default_settings.snap +++ b/crates/ruff/tests/snapshots/show_settings__display_default_settings.snap @@ -50,6 +50,7 @@ file_resolver.exclude = [ "venv", ] file_resolver.extend_exclude = [ + "crates/ruff/resources/", "crates/ruff_linter/resources/", "crates/ruff_python_formatter/resources/", ] diff --git a/crates/ruff_diagnostics/src/diagnostic.rs b/crates/ruff_diagnostics/src/diagnostic.rs index 006df346365fd..84da5f3904607 100644 --- a/crates/ruff_diagnostics/src/diagnostic.rs +++ b/crates/ruff_diagnostics/src/diagnostic.rs @@ -71,6 +71,14 @@ impl Diagnostic { } } + /// Consumes `self` and returns a new `Diagnostic` with the given parent node. + #[inline] + #[must_use] + pub fn with_parent(mut self, parent: TextSize) -> Self { + self.set_parent(parent); + self + } + /// Set the location of the diagnostic's parent node. #[inline] pub fn set_parent(&mut self, parent: TextSize) { diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI034.py b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI034.py index cb806763284b0..90257717e8c14 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI034.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI034.py @@ -195,6 +195,13 @@ class BadAsyncIterator(collections.abc.AsyncIterator[str]): def __aiter__(self) -> typing.AsyncIterator[str]: ... # Y034 "__aiter__" methods in classes like "BadAsyncIterator" usually return "self" at runtime. Consider using "typing_extensions.Self" in "BadAsyncIterator.__aiter__", e.g. "def __aiter__(self) -> Self: ..." # Y022 Use "collections.abc.AsyncIterator[T]" instead of "typing.AsyncIterator[T]" (PEP 585 syntax) +class SubclassOfBadIterator3(BadIterator3): + def __iter__(self) -> Iterator[int]: # Y034 + ... + +class SubclassOfBadAsyncIterator(BadAsyncIterator): + def __aiter__(self) -> collections.abc.AsyncIterator[str]: # Y034 + ... class AsyncIteratorReturningAsyncIterable: def __aiter__(self) -> AsyncIterable[str]: @@ -225,6 +232,11 @@ def __enter__(self) -> MetaclassInWhichSelfCannotBeUsed4: ... async def __aenter__(self) -> MetaclassInWhichSelfCannotBeUsed4: ... def __isub__(self, other: MetaclassInWhichSelfCannotBeUsed4) -> MetaclassInWhichSelfCannotBeUsed4: ... +class SubclassOfMetaclassInWhichSelfCannotBeUsed(MetaclassInWhichSelfCannotBeUsed4): + def __new__(cls) -> SubclassOfMetaclassInWhichSelfCannotBeUsed: ... + def __enter__(self) -> SubclassOfMetaclassInWhichSelfCannotBeUsed: ... + async def __aenter__(self) -> SubclassOfMetaclassInWhichSelfCannotBeUsed: ... + def __isub__(self, other: SubclassOfMetaclassInWhichSelfCannotBeUsed) -> SubclassOfMetaclassInWhichSelfCannotBeUsed: ... class Abstract(Iterator[str]): @abstractmethod diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT007.py b/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT007.py index 4a4d9731c0c5b..d6a9a934e4cad 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT007.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT007.py @@ -80,5 +80,13 @@ def test_single_list_of_lists(param): @pytest.mark.parametrize("a", [1, 2]) @pytest.mark.parametrize(("b", "c"), ((3, 4), (5, 6))) @pytest.mark.parametrize("d", [3,]) -def test_multiple_decorators(a, b, c): +@pytest.mark.parametrize( + "d", + [("3", "4")], +) +@pytest.mark.parametrize( + "e", + [("3", "4"),], +) +def test_multiple_decorators(a, b, c, d, e): pass diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM103.py b/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM103.py index 85f00aec21530..e1b868888df86 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM103.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM103.py @@ -53,7 +53,7 @@ def f(): def f(): - # SIM103 (but not fixable) + # SIM103 if a: return False else: @@ -77,7 +77,7 @@ def f(): def f(): - # OK + # SIM103 (but not fixable) def bool(): return False if a: @@ -86,6 +86,14 @@ def bool(): return False +def f(): + # SIM103 + if keys is not None and notice.key not in keys: + return False + else: + return True + + ### # Positive cases (preview) ### diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_slots/SLOT002.py b/crates/ruff_linter/resources/test/fixtures/flake8_slots/SLOT002.py index 59a9ba3f5579b..11f2782b77ac2 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_slots/SLOT002.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_slots/SLOT002.py @@ -6,6 +6,14 @@ class Bad(namedtuple("foo", ["str", "int"])): # SLOT002 pass +class UnusualButStillBad(NamedTuple("foo", [("x", int, "y", int)])): # SLOT002 + pass + + +class UnusualButOkay(NamedTuple("foo", [("x", int, "y", int)])): + __slots__ = () + + class Good(namedtuple("foo", ["str", "int"])): # OK __slots__ = ("foo",) diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F821_26.pyi b/crates/ruff_linter/resources/test/fixtures/pyflakes/F821_26.pyi index f87819ef8004b..73b2b01a0b19f 100644 --- a/crates/ruff_linter/resources/test/fixtures/pyflakes/F821_26.pyi +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F821_26.pyi @@ -1,6 +1,6 @@ """Tests for constructs allowed in `.pyi` stub files but not at runtime""" -from typing import Optional, TypeAlias, Union +from typing import Generic, NewType, Optional, TypeAlias, TypeVar, Union __version__: str __author__: str @@ -33,6 +33,19 @@ class Leaf: ... class Tree(list[Tree | Leaf]): ... # valid in a `.pyi` stub file, not in a `.py` runtime file class Tree2(list["Tree | Leaf"]): ... # always okay +# Generic bases can have forward references in stubs +class Foo(Generic[T]): ... +T = TypeVar("T") +class Bar(Foo[Baz]): ... +class Baz: ... + +# bases in general can be forward references in stubs +class Eggs(Spam): ... +class Spam: ... + +# NewType can have forward references +MyNew = NewType("MyNew", MyClass) + # Annotations are treated as assignments in .pyi files, but not in .py files class MyClass: foo: int @@ -42,3 +55,6 @@ class MyClass: baz: MyClass eggs = baz # valid in a `.pyi` stub file, not in a `.py` runtime file eggs = "baz" # always okay + +class Blah: + class Blah2(Blah): ... diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F822_3.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F822_3.py new file mode 100644 index 0000000000000..682b69597223e --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F822_3.py @@ -0,0 +1,13 @@ +"""Respect `# noqa` directives on `__all__` definitions.""" + +__all__ = [ # noqa: F822 + "Bernoulli", + "Beta", + "Binomial", +] + + +__all__ += [ + "ContinuousBernoulli", # noqa: F822 + "ExponentialFamily", +] diff --git a/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH004_0.py b/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH004_0.py index c35f8e8824b68..2e7ae46fba766 100644 --- a/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH004_0.py +++ b/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH004_0.py @@ -9,3 +9,22 @@ x = 1 # noqa: F401, W203 # noqa: F401 # noqa: F401, W203 + +# OK +x = 2 # noqa: X100 +x = 2 # noqa:X100 + +# PGH004 +x = 2 # noqa X100 + +# PGH004 +x = 2 # noqa X100, X200 + +# PGH004 +x = 2 # noqa : X300 + +# PGH004 +x = 2 # noqa : X400 + +# PGH004 +x = 2 # noqa :X500 diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/if_stmt_min_max.py b/crates/ruff_linter/resources/test/fixtures/pylint/if_stmt_min_max.py new file mode 100644 index 0000000000000..e27adfda6db49 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pylint/if_stmt_min_max.py @@ -0,0 +1,136 @@ +# pylint: disable=missing-docstring, invalid-name, too-few-public-methods, redefined-outer-name + +value = 10 +value2 = 0 +value3 = 3 + +# Positive +if value < 10: # [max-instead-of-if] + value = 10 + +if value <= 10: # [max-instead-of-if] + value = 10 + +if value < value2: # [max-instead-of-if] + value = value2 + +if value > 10: # [min-instead-of-if] + value = 10 + +if value >= 10: # [min-instead-of-if] + value = 10 + +if value > value2: # [min-instead-of-if] + value = value2 + + +class A: + def __init__(self): + self.value = 13 + + +A1 = A() +if A1.value < 10: # [max-instead-of-if] + A1.value = 10 + +if A1.value > 10: # [min-instead-of-if] + A1.value = 10 + + +class AA: + def __init__(self, value): + self.value = value + + def __gt__(self, b): + return self.value > b + + def __ge__(self, b): + return self.value >= b + + def __lt__(self, b): + return self.value < b + + def __le__(self, b): + return self.value <= b + + +A1 = AA(0) +A2 = AA(3) + +if A2 < A1: # [max-instead-of-if] + A2 = A1 + +if A2 <= A1: # [max-instead-of-if] + A2 = A1 + +if A2 > A1: # [min-instead-of-if] + A2 = A1 + +if A2 >= A1: # [min-instead-of-if] + A2 = A1 + +# Negative +if value < 10: + value = 2 + +if value <= 3: + value = 5 + +if value < 10: + value = 2 + value2 = 3 + +if value < value2: + value = value3 + +if value < 5: + value = value3 + +if 2 < value <= 3: + value = 1 + +if value < 10: + value = 10 +else: + value = 3 + +if value <= 3: + value = 5 +elif value == 3: + value = 2 + +if value > 10: + value = 2 + +if value >= 3: + value = 5 + +if value > 10: + value = 2 + value2 = 3 + +if value > value2: + value = value3 + +if value > 5: + value = value3 + +if 2 > value >= 3: + value = 1 + +if value > 10: + value = 10 +else: + value = 3 + +if value >= 3: + value = 5 +elif value == 3: + value = 2 + +# Parenthesized expressions +if value.attr > 3: + ( + value. + attr + ) = 3 diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP042.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP042.py new file mode 100644 index 0000000000000..392cf0ff5eee0 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP042.py @@ -0,0 +1,13 @@ +from enum import Enum + + +class A(str, Enum): ... + + +class B(Enum, str): ... + + +class D(int, str, Enum): ... + + +class E(str, int, Enum): ... diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB101.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB101.py index 2f0276a09a826..31b1ccd341e24 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB101.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB101.py @@ -28,10 +28,6 @@ def bar(x): with open("file.txt", errors="ignore") as f: x = f.read() -# FURB101 -with open("file.txt", errors="ignore", mode="rb") as f: - x = f.read() - # FURB101 with open("file.txt", mode="r") as f: # noqa: FURB120 x = f.read() @@ -60,6 +56,11 @@ def bar(x): # Non-errors. +# Path.read_bytes does not support any kwargs +with open("file.txt", errors="ignore", mode="rb") as f: + x = f.read() + + f2 = open("file2.txt") with open("file.txt") as f: x = f2.read() diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB110.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB110.py new file mode 100644 index 0000000000000..706d78a4f0103 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB110.py @@ -0,0 +1,40 @@ +z = x if x else y # FURB110 + +z = x \ + if x else y # FURB110 + +z = x if x \ + else \ + y # FURB110 + +z = x() if x() else y() # FURB110 + +# FURB110 +z = x if ( + # Test for x. + x +) else ( + # Test for y. + y +) + +# FURB110 +z = ( + x if ( + # Test for x. + x + ) else ( + # Test for y. + y + ) +) + +# FURB110 +z = ( + x if + # If true, use x. + x + # Otherwise, use y. + else + y +) diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB118.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB118.py index 51c136f976558..6e8c3d7676091 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB118.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB118.py @@ -10,7 +10,7 @@ op_matmutl = lambda x, y: x @ y op_truediv = lambda x, y: x / y op_mod = lambda x, y: x % y -op_pow = lambda x, y: x ** y +op_pow = lambda x, y: x**y op_lshift = lambda x, y: x << y op_rshift = lambda x, y: x >> y op_bitor = lambda x, y: x | y @@ -27,6 +27,10 @@ op_is = lambda x, y: x is y op_isnot = lambda x, y: x is not y op_in = lambda x, y: y in x +op_itemgetter = lambda x: x[0] +op_itemgetter = lambda x: (x[0], x[1], x[2]) +op_itemgetter = lambda x: (x[1:], x[2]) +op_itemgetter = lambda x: x[:] def op_not2(x): @@ -41,21 +45,30 @@ class Adder: def add(x, y): return x + y + # OK. -op_add3 = lambda x, y = 1: x + y +op_add3 = lambda x, y=1: x + y op_neg2 = lambda x, y: y - x op_notin = lambda x, y: y not in x op_and = lambda x, y: y and x op_or = lambda x, y: y or x op_in = lambda x, y: x in y +op_itemgetter = lambda x: (1, x[1], x[2]) +op_itemgetter = lambda x: (x.y, x[1], x[2]) +op_itemgetter = lambda x, y: (x[0], y[0]) +op_itemgetter = lambda x, y: (x[0], y[0]) +op_itemgetter = lambda x: () +op_itemgetter = lambda x: (*x[0], x[1]) def op_neg3(x, y): return y - x -def op_add4(x, y = 1): + +def op_add4(x, y=1): return x + y + def op_add5(x, y): print("op_add5") return x + y diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index b3fc11d7f7e15..72a293d3acc52 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -32,10 +32,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if let Some(operator) = typing::to_pep604_operator(value, slice, &checker.semantic) { if checker.enabled(Rule::FutureRewritableTypeAnnotation) { - if !checker.source_type.is_stub() + if !checker.semantic.future_annotations_or_stub() && checker.settings.target_version < PythonVersion::Py310 && checker.settings.target_version >= PythonVersion::Py37 - && !checker.semantic.future_annotations() && checker.semantic.in_annotation() && !checker.settings.pyupgrade.keep_runtime_typing { @@ -48,7 +47,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.source_type.is_stub() || checker.settings.target_version >= PythonVersion::Py310 || (checker.settings.target_version >= PythonVersion::Py37 - && checker.semantic.future_annotations() + && checker.semantic.future_annotations_or_stub() && checker.semantic.in_annotation() && !checker.settings.pyupgrade.keep_runtime_typing) { @@ -60,9 +59,8 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { // Ex) list[...] if checker.enabled(Rule::FutureRequiredTypeAnnotation) { - if !checker.source_type.is_stub() + if !checker.semantic.future_annotations_or_stub() && checker.settings.target_version < PythonVersion::Py39 - && !checker.semantic.future_annotations() && checker.semantic.in_annotation() && typing::is_pep585_generic(value, &checker.semantic) { @@ -186,10 +184,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { typing::to_pep585_generic(expr, &checker.semantic) { if checker.enabled(Rule::FutureRewritableTypeAnnotation) { - if !checker.source_type.is_stub() + if !checker.semantic.future_annotations_or_stub() && checker.settings.target_version < PythonVersion::Py39 && checker.settings.target_version >= PythonVersion::Py37 - && !checker.semantic.future_annotations() && checker.semantic.in_annotation() && !checker.settings.pyupgrade.keep_runtime_typing { @@ -200,7 +197,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.source_type.is_stub() || checker.settings.target_version >= PythonVersion::Py39 || (checker.settings.target_version >= PythonVersion::Py37 - && checker.semantic.future_annotations() + && checker.semantic.future_annotations_or_stub() && checker.semantic.in_annotation() && !checker.settings.pyupgrade.keep_runtime_typing) { @@ -270,10 +267,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { ]) { if let Some(replacement) = typing::to_pep585_generic(expr, &checker.semantic) { if checker.enabled(Rule::FutureRewritableTypeAnnotation) { - if !checker.source_type.is_stub() + if !checker.semantic.future_annotations_or_stub() && checker.settings.target_version < PythonVersion::Py39 && checker.settings.target_version >= PythonVersion::Py37 - && !checker.semantic.future_annotations() && checker.semantic.in_annotation() && !checker.settings.pyupgrade.keep_runtime_typing { @@ -286,7 +282,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.source_type.is_stub() || checker.settings.target_version >= PythonVersion::Py39 || (checker.settings.target_version >= PythonVersion::Py37 - && checker.semantic.future_annotations() + && checker.semantic.future_annotations_or_stub() && checker.semantic.in_annotation() && !checker.settings.pyupgrade.keep_runtime_typing) { @@ -1176,9 +1172,8 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { }) => { // Ex) `str | None` if checker.enabled(Rule::FutureRequiredTypeAnnotation) { - if !checker.source_type.is_stub() + if !checker.semantic.future_annotations_or_stub() && checker.settings.target_version < PythonVersion::Py310 - && !checker.semantic.future_annotations() && checker.semantic.in_annotation() { flake8_future_annotations::rules::future_required_type_annotation( @@ -1363,6 +1358,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::IfExprMinMax) { refurb::rules::if_expr_min_max(checker, if_exp); } + if checker.enabled(Rule::IfExpInsteadOfOrOperator) { + refurb::rules::if_exp_instead_of_or_operator(checker, if_exp); + } } Expr::ListComp( comp @ ast::ExprListComp { diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index bff7e5658b6fb..141a0ec9e96ef 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -406,6 +406,11 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.enabled(Rule::UselessObjectInheritance) { pyupgrade::rules::useless_object_inheritance(checker, class_def); } + if checker.enabled(Rule::ReplaceStrEnum) { + if checker.settings.target_version >= PythonVersion::Py311 { + pyupgrade::rules::replace_str_enum(checker, class_def); + } + } if checker.enabled(Rule::UnnecessaryClassParentheses) { pyupgrade::rules::unnecessary_class_parentheses(checker, class_def); } @@ -1112,6 +1117,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.enabled(Rule::TooManyBooleanExpressions) { pylint::rules::too_many_boolean_expressions(checker, if_); } + if checker.enabled(Rule::IfStmtMinMax) { + pylint::rules::if_stmt_min_max(checker, if_); + } if checker.source_type.is_stub() { if checker.any_enabled(&[ Rule::UnrecognizedVersionInfoCheck, diff --git a/crates/ruff_linter/src/checkers/ast/annotation.rs b/crates/ruff_linter/src/checkers/ast/annotation.rs index 1421361a92a9c..86d1ba50f8d81 100644 --- a/crates/ruff_linter/src/checkers/ast/annotation.rs +++ b/crates/ruff_linter/src/checkers/ast/annotation.rs @@ -56,9 +56,10 @@ impl AnnotationContext { _ => {} } - // If `__future__` annotations are enabled, then annotations are never evaluated - // at runtime, so we can treat them as typing-only. - if semantic.future_annotations() { + // If `__future__` annotations are enabled or it's a stub file, + // then annotations are never evaluated at runtime, + // so we can treat them as typing-only. + if semantic.future_annotations_or_stub() { return Self::TypingOnly; } @@ -87,7 +88,7 @@ impl AnnotationContext { semantic, ) { Self::RuntimeRequired - } else if semantic.future_annotations() { + } else if semantic.future_annotations_or_stub() { Self::TypingOnly } else { Self::RuntimeEvaluated diff --git a/crates/ruff_linter/src/checkers/ast/deferred.rs b/crates/ruff_linter/src/checkers/ast/deferred.rs index e1ca4a8656375..7f390e7afd577 100644 --- a/crates/ruff_linter/src/checkers/ast/deferred.rs +++ b/crates/ruff_linter/src/checkers/ast/deferred.rs @@ -12,6 +12,8 @@ pub(crate) struct Visit<'a> { pub(crate) type_param_definitions: Vec<(&'a Expr, Snapshot)>, pub(crate) functions: Vec, pub(crate) lambdas: Vec, + /// N.B. This field should always be empty unless it's a stub file + pub(crate) class_bases: Vec<(&'a Expr, Snapshot)>, } impl Visit<'_> { @@ -22,6 +24,7 @@ impl Visit<'_> { && self.type_param_definitions.is_empty() && self.functions.is_empty() && self.lambdas.is_empty() + && self.class_bases.is_empty() } } diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 6af274cb8f331..86b4f54b6782f 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -31,15 +31,14 @@ use std::path::Path; use itertools::Itertools; use log::debug; use ruff_python_ast::{ - self as ast, all::DunderAllName, Comprehension, ElifElseClause, ExceptHandler, Expr, - ExprContext, FStringElement, Keyword, MatchCase, Parameter, ParameterWithDefault, Parameters, - Pattern, Stmt, Suite, UnaryOp, + self as ast, Comprehension, ElifElseClause, ExceptHandler, Expr, ExprContext, FStringElement, + Keyword, MatchCase, Parameter, ParameterWithDefault, Parameters, Pattern, Stmt, Suite, UnaryOp, }; use ruff_text_size::{Ranged, TextRange, TextSize}; use ruff_diagnostics::{Diagnostic, IsolationLevel}; use ruff_notebook::{CellOffsets, NotebookIndex}; -use ruff_python_ast::all::{extract_all_names, DunderAllFlags}; +use ruff_python_ast::all::{extract_all_names, DunderAllDefinition, DunderAllFlags}; use ruff_python_ast::helpers::{ collect_import_from_member, extract_handled_exceptions, is_docstring_stmt, to_module_path, }; @@ -572,6 +571,7 @@ impl<'a> Visitor<'a> for Checker<'a> { match stmt { Stmt::FunctionDef( function_def @ ast::StmtFunctionDef { + name, body, parameters, decorator_list, @@ -691,9 +691,21 @@ impl<'a> Visitor<'a> for Checker<'a> { if let Some(globals) = Globals::from_body(body) { self.semantic.set_globals(globals); } + let scope_id = self.semantic.scope_id; + self.analyze.scopes.push(scope_id); + self.semantic.pop_scope(); // Function scope + self.semantic.pop_definition(); + self.semantic.pop_scope(); // Type parameter scope + self.add_binding( + name, + stmt.identifier(), + BindingKind::FunctionDefinition(scope_id), + BindingFlags::empty(), + ); } Stmt::ClassDef( class_def @ ast::StmtClassDef { + name, body, arguments, decorator_list, @@ -712,7 +724,9 @@ impl<'a> Visitor<'a> for Checker<'a> { } if let Some(arguments) = arguments { + self.semantic.flags |= SemanticModelFlags::CLASS_BASE; self.visit_arguments(arguments); + self.semantic.flags -= SemanticModelFlags::CLASS_BASE; } let definition = docstrings::extraction::extract_definition( @@ -732,6 +746,18 @@ impl<'a> Visitor<'a> for Checker<'a> { // Set the docstring state before visiting the class body. self.docstring_state = DocstringState::Expected; self.visit_body(body); + + let scope_id = self.semantic.scope_id; + self.analyze.scopes.push(scope_id); + self.semantic.pop_scope(); // Class scope + self.semantic.pop_definition(); + self.semantic.pop_scope(); // Type parameter scope + self.add_binding( + name, + stmt.identifier(), + BindingKind::ClassDefinition(scope_id), + BindingFlags::empty(), + ); } Stmt::TypeAlias(ast::StmtTypeAlias { range: _, @@ -888,35 +914,6 @@ impl<'a> Visitor<'a> for Checker<'a> { }; // Step 3: Clean-up - match stmt { - Stmt::FunctionDef(ast::StmtFunctionDef { name, .. }) => { - let scope_id = self.semantic.scope_id; - self.analyze.scopes.push(scope_id); - self.semantic.pop_scope(); // Function scope - self.semantic.pop_definition(); - self.semantic.pop_scope(); // Type parameter scope - self.add_binding( - name, - stmt.identifier(), - BindingKind::FunctionDefinition(scope_id), - BindingFlags::empty(), - ); - } - Stmt::ClassDef(ast::StmtClassDef { name, .. }) => { - let scope_id = self.semantic.scope_id; - self.analyze.scopes.push(scope_id); - self.semantic.pop_scope(); // Class scope - self.semantic.pop_definition(); - self.semantic.pop_scope(); // Type parameter scope - self.add_binding( - name, - stmt.identifier(), - BindingKind::ClassDefinition(scope_id), - BindingFlags::empty(), - ); - } - _ => {} - } // Step 4: Analysis analyze::statement(stmt, self); @@ -935,10 +932,23 @@ impl<'a> Visitor<'a> for Checker<'a> { fn visit_expr(&mut self, expr: &'a Expr) { // Step 0: Pre-processing + if self.source_type.is_stub() + && self.semantic.in_class_base() + && !self.semantic.in_deferred_class_base() + { + self.visit + .class_bases + .push((expr, self.semantic.snapshot())); + return; + } + if !self.semantic.in_typing_literal() + // `in_deferred_type_definition()` will only be `true` if we're now visiting the deferred nodes + // after having already traversed the source tree once. If we're now visiting the deferred nodes, + // we can't defer again, or we'll infinitely recurse! && !self.semantic.in_deferred_type_definition() && self.semantic.in_type_definition() - && self.semantic.future_annotations() + && self.semantic.future_annotations_or_stub() && (self.semantic.in_annotation() || self.source_type.is_stub()) { if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = expr { @@ -1965,6 +1975,52 @@ impl<'a> Checker<'a> { scope.add(id, binding_id); } + /// After initial traversal of the AST, visit all class bases that were deferred. + /// + /// This method should only be relevant in stub files, where forward references are + /// legal in class bases. For other kinds of Python files, using a forward reference + /// in a class base is never legal, so `self.visit.class_bases` should always be empty. + /// + /// For example, in a stub file: + /// ```python + /// class Foo(list[Bar]): ... # <-- `Bar` is a forward reference in a class base + /// class Bar: ... + /// ``` + fn visit_deferred_class_bases(&mut self) { + let snapshot = self.semantic.snapshot(); + let deferred_bases = std::mem::take(&mut self.visit.class_bases); + debug_assert!( + self.source_type.is_stub() || deferred_bases.is_empty(), + "Class bases should never be deferred outside of stub files" + ); + for (expr, snapshot) in deferred_bases { + self.semantic.restore(snapshot); + // Set this flag to avoid infinite recursion, or we'll just defer it again: + self.semantic.flags |= SemanticModelFlags::DEFERRED_CLASS_BASE; + self.visit_expr(expr); + } + self.semantic.restore(snapshot); + } + + /// After initial traversal of the AST, visit all "future type definitions". + /// + /// A "future type definition" is a type definition where [PEP 563] semantics + /// apply (i.e., an annotation in a module that has `from __future__ import annotations` + /// at the top of the file, or an annotation in a stub file). These type definitions + /// support forward references, so they are deferred on initial traversal + /// of the source tree. + /// + /// For example: + /// ```python + /// from __future__ import annotations + /// + /// def foo() -> Bar: # <-- return annotation is a "future type definition" + /// return Bar() + /// + /// class Bar: pass + /// ``` + /// + /// [PEP 563]: https://peps.python.org/pep-0563/ fn visit_deferred_future_type_definitions(&mut self) { let snapshot = self.semantic.snapshot(); while !self.visit.future_type_definitions.is_empty() { @@ -1972,6 +2028,14 @@ impl<'a> Checker<'a> { for (expr, snapshot) in type_definitions { self.semantic.restore(snapshot); + // Type definitions should only be considered "`__future__` type definitions" + // if they are annotations in a module where `from __future__ import + // annotations` is active, or they are type definitions in a stub file. + debug_assert!( + self.semantic.future_annotations_or_stub() + && (self.source_type.is_stub() || self.semantic.in_annotation()) + ); + self.semantic.flags |= SemanticModelFlags::TYPE_DEFINITION | SemanticModelFlags::FUTURE_TYPE_DEFINITION; self.visit_expr(expr); @@ -1980,6 +2044,19 @@ impl<'a> Checker<'a> { self.semantic.restore(snapshot); } + /// After initial traversal of the AST, visit all [type parameter definitions]. + /// + /// Type parameters natively support forward references, + /// so are always deferred during initial traversal of the source tree. + /// + /// For example: + /// ```python + /// class Foo[T: Bar]: pass # <-- Forward reference used in definition of type parameter `T` + /// type X[T: Bar] = Foo[T] # <-- Ditto + /// class Bar: pass + /// ``` + /// + /// [type parameter definitions]: https://docs.python.org/3/reference/executionmodel.html#annotation-scopes fn visit_deferred_type_param_definitions(&mut self) { let snapshot = self.semantic.snapshot(); while !self.visit.type_param_definitions.is_empty() { @@ -1995,6 +2072,17 @@ impl<'a> Checker<'a> { self.semantic.restore(snapshot); } + /// After initial traversal of the AST, visit all "string type definitions", + /// i.e., type definitions that are enclosed within quotes so as to allow + /// the type definition to use forward references. + /// + /// For example: + /// ```python + /// def foo() -> "Bar": # <-- return annotation is a "string type definition" + /// return Bar() + /// + /// class Bar: pass + /// ``` fn visit_deferred_string_type_definitions(&mut self, allocator: &'a typed_arena::Arena) { let snapshot = self.semantic.snapshot(); while !self.visit.string_type_definitions.is_empty() { @@ -2007,7 +2095,7 @@ impl<'a> Checker<'a> { self.semantic.restore(snapshot); - if self.semantic.in_annotation() && self.semantic.future_annotations() { + if self.semantic.in_annotation() && self.semantic.future_annotations_or_stub() { if self.enabled(Rule::QuotedAnnotation) { pyupgrade::rules::quoted_annotation(self, value, range); } @@ -2043,6 +2131,11 @@ impl<'a> Checker<'a> { self.semantic.restore(snapshot); } + /// After initial traversal of the AST, visit all function bodies. + /// + /// Function bodies are always deferred on initial traversal of the source tree, + /// as the body of a function may validly contain references to global-scope symbols + /// that were not yet defined at the point when the function was defined. fn visit_deferred_functions(&mut self) { let snapshot = self.semantic.snapshot(); while !self.visit.functions.is_empty() { @@ -2066,8 +2159,9 @@ impl<'a> Checker<'a> { self.semantic.restore(snapshot); } - /// Visit all deferred lambdas. Returns a list of snapshots, such that the caller can restore - /// the semantic model to the state it was in before visiting the deferred lambdas. + /// After initial traversal of the source tree has been completed, + /// visit all lambdas. Lambdas are deferred during the initial traversal + /// for the same reason as function bodies. fn visit_deferred_lambdas(&mut self) { let snapshot = self.semantic.snapshot(); while !self.visit.lambdas.is_empty() { @@ -2093,10 +2187,12 @@ impl<'a> Checker<'a> { self.semantic.restore(snapshot); } - /// Recursively visit all deferred AST nodes, including lambdas, functions, and type - /// annotations. + /// After initial traversal of the source tree has been completed, + /// recursively visit all AST nodes that were deferred on the first pass. + /// This includes lambdas, functions, type parameters, and type annotations. fn visit_deferred(&mut self, allocator: &'a typed_arena::Arena) { while !self.visit.is_empty() { + self.visit_deferred_class_bases(); self.visit_deferred_functions(); self.visit_deferred_type_param_definitions(); self.visit_deferred_lambdas(); @@ -2109,45 +2205,54 @@ impl<'a> Checker<'a> { fn visit_exports(&mut self) { let snapshot = self.semantic.snapshot(); - let exports: Vec = self + let definitions: Vec = self .semantic .global_scope() .get_all("__all__") .map(|binding_id| &self.semantic.bindings[binding_id]) .filter_map(|binding| match &binding.kind { - BindingKind::Export(Export { names }) => Some(names.iter().copied()), + BindingKind::Export(Export { names }) => { + Some(DunderAllDefinition::new(binding.range(), names.to_vec())) + } _ => None, }) - .flatten() .collect(); - for export in exports { - let (name, range) = (export.name(), export.range()); - if let Some(binding_id) = self.semantic.global_scope().get(name) { - self.semantic.flags |= SemanticModelFlags::DUNDER_ALL_DEFINITION; - // Mark anything referenced in `__all__` as used. - self.semantic - .add_global_reference(binding_id, ExprContext::Load, range); - self.semantic.flags -= SemanticModelFlags::DUNDER_ALL_DEFINITION; - } else { - if self.semantic.global_scope().uses_star_imports() { - if self.enabled(Rule::UndefinedLocalWithImportStarUsage) { - self.diagnostics.push(Diagnostic::new( - pyflakes::rules::UndefinedLocalWithImportStarUsage { - name: name.to_string(), - }, - range, - )); - } + for definition in definitions { + for export in definition.names() { + let (name, range) = (export.name(), export.range()); + if let Some(binding_id) = self.semantic.global_scope().get(name) { + self.semantic.flags |= SemanticModelFlags::DUNDER_ALL_DEFINITION; + // Mark anything referenced in `__all__` as used. + self.semantic + .add_global_reference(binding_id, ExprContext::Load, range); + self.semantic.flags -= SemanticModelFlags::DUNDER_ALL_DEFINITION; } else { - if self.enabled(Rule::UndefinedExport) { - if !self.path.ends_with("__init__.py") { - self.diagnostics.push(Diagnostic::new( - pyflakes::rules::UndefinedExport { - name: name.to_string(), - }, - range, - )); + if self.semantic.global_scope().uses_star_imports() { + if self.enabled(Rule::UndefinedLocalWithImportStarUsage) { + self.diagnostics.push( + Diagnostic::new( + pyflakes::rules::UndefinedLocalWithImportStarUsage { + name: name.to_string(), + }, + range, + ) + .with_parent(definition.start()), + ); + } + } else { + if self.enabled(Rule::UndefinedExport) { + if !self.path.ends_with("__init__.py") { + self.diagnostics.push( + Diagnostic::new( + pyflakes::rules::UndefinedExport { + name: name.to_string(), + }, + range, + ) + .with_parent(definition.start()), + ); + } } } } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index c83ee76b3bef9..988f0118b1329 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -287,6 +287,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pylint, "R1711") => (RuleGroup::Stable, rules::pylint::rules::UselessReturn), (Pylint, "R1714") => (RuleGroup::Stable, rules::pylint::rules::RepeatedEqualityComparison), (Pylint, "R1722") => (RuleGroup::Stable, rules::pylint::rules::SysExitAlias), + (Pylint, "R1730") => (RuleGroup::Preview, rules::pylint::rules::IfStmtMinMax), (Pylint, "R1733") => (RuleGroup::Preview, rules::pylint::rules::UnnecessaryDictIndexLookup), (Pylint, "R1736") => (RuleGroup::Preview, rules::pylint::rules::UnnecessaryListIndexLookup), (Pylint, "R2004") => (RuleGroup::Stable, rules::pylint::rules::MagicValueComparison), @@ -547,6 +548,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pyupgrade, "039") => (RuleGroup::Stable, rules::pyupgrade::rules::UnnecessaryClassParentheses), (Pyupgrade, "040") => (RuleGroup::Stable, rules::pyupgrade::rules::NonPEP695TypeAlias), (Pyupgrade, "041") => (RuleGroup::Stable, rules::pyupgrade::rules::TimeoutErrorAlias), + (Pyupgrade, "042") => (RuleGroup::Preview, rules::pyupgrade::rules::ReplaceStrEnum), // pydocstyle (Pydocstyle, "100") => (RuleGroup::Stable, rules::pydocstyle::rules::UndocumentedPublicModule), @@ -1037,6 +1039,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Refurb, "101") => (RuleGroup::Preview, rules::refurb::rules::ReadWholeFile), (Refurb, "103") => (RuleGroup::Preview, rules::refurb::rules::WriteWholeFile), (Refurb, "105") => (RuleGroup::Preview, rules::refurb::rules::PrintEmptyString), + (Refurb, "110") => (RuleGroup::Preview, rules::refurb::rules::IfExpInsteadOfOrOperator), #[allow(deprecated)] (Refurb, "113") => (RuleGroup::Nursery, rules::refurb::rules::RepeatedAppend), (Refurb, "118") => (RuleGroup::Preview, rules::refurb::rules::ReimplementedOperator), diff --git a/crates/ruff_linter/src/fs.rs b/crates/ruff_linter/src/fs.rs index 2a617113fe2ee..aa230ab28788e 100644 --- a/crates/ruff_linter/src/fs.rs +++ b/crates/ruff_linter/src/fs.rs @@ -9,24 +9,37 @@ use crate::registry::RuleSet; /// Create a set with codes matching the pattern/code pairs. pub(crate) fn ignores_from_path( path: &Path, - pattern_code_pairs: &[(GlobMatcher, GlobMatcher, RuleSet)], + pattern_code_pairs: &[(GlobMatcher, GlobMatcher, bool, RuleSet)], ) -> RuleSet { let file_name = path.file_name().expect("Unable to parse filename"); pattern_code_pairs .iter() - .filter_map(|(absolute, basename, rules)| { + .filter_map(|(absolute, basename, negated, rules)| { if basename.is_match(file_name) { - debug!( - "Adding per-file ignores for {:?} due to basename match on {:?}: {:?}", - path, - basename.glob().regex(), - rules - ); - Some(rules) + if *negated { None } else { + debug!( + "Adding per-file ignores for {:?} due to basename match on {:?}: {:?}", + path, + basename.glob().regex(), + rules + ); + Some(rules) + } } else if absolute.is_match(path) { + if *negated { None } else { + debug!( + "Adding per-file ignores for {:?} due to absolute match on {:?}: {:?}", + path, + absolute.glob().regex(), + rules + ); + Some(rules) + } + } else if *negated { debug!( - "Adding per-file ignores for {:?} due to absolute match on {:?}: {:?}", + "Adding per-file ignores for {:?} due to negated pattern matching neither {:?} nor {:?}: {:?}", path, + basename.glob().regex(), absolute.glob().regex(), rules ); diff --git a/crates/ruff_linter/src/noqa.rs b/crates/ruff_linter/src/noqa.rs index d50c3b1757038..3af740e819193 100644 --- a/crates/ruff_linter/src/noqa.rs +++ b/crates/ruff_linter/src/noqa.rs @@ -147,7 +147,7 @@ impl<'a> Directive<'a> { /// Lex an individual rule code (e.g., `F401`). #[inline] - fn lex_code(line: &str) -> Option<&str> { + pub(crate) fn lex_code(line: &str) -> Option<&str> { // Extract, e.g., the `F` in `F401`. let prefix = line.chars().take_while(char::is_ascii_uppercase).count(); // Extract, e.g., the `401` in `F401`. diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/non_self_return_type.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/non_self_return_type.rs index 739d4ac61175b..d2a28ae2e8d1c 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/non_self_return_type.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/non_self_return_type.rs @@ -1,9 +1,10 @@ -use ruff_python_ast::{self as ast, Arguments, Decorator, Expr, Parameters, Stmt}; +use ruff_python_ast::{self as ast, Decorator, Expr, Parameters, Stmt}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::helpers::map_subscript; use ruff_python_ast::identifier::Identifier; +use ruff_python_semantic::analyze; use ruff_python_semantic::analyze::visibility::{is_abstract, is_final, is_overload}; use ruff_python_semantic::{ScopeKind, SemanticModel}; @@ -119,7 +120,9 @@ pub(crate) fn non_self_return_type( returns: Option<&Expr>, parameters: &Parameters, ) { - let ScopeKind::Class(class_def) = checker.semantic().current_scope().kind else { + let semantic = checker.semantic(); + + let ScopeKind::Class(class_def) = semantic.current_scope().kind else { return; }; @@ -132,21 +135,19 @@ pub(crate) fn non_self_return_type( }; // PEP 673 forbids the use of `typing(_extensions).Self` in metaclasses. - if is_metaclass(class_def, checker.semantic()) { + if is_metaclass(class_def, semantic) { return; } // Skip any abstract or overloaded methods. - if is_abstract(decorator_list, checker.semantic()) - || is_overload(decorator_list, checker.semantic()) - { + if is_abstract(decorator_list, semantic) || is_overload(decorator_list, semantic) { return; } if is_async { if name == "__aenter__" && is_name(returns, &class_def.name) - && !is_final(&class_def.decorator_list, checker.semantic()) + && !is_final(&class_def.decorator_list, semantic) { checker.diagnostics.push(Diagnostic::new( NonSelfReturnType { @@ -161,7 +162,7 @@ pub(crate) fn non_self_return_type( // In-place methods that are expected to return `Self`. if is_inplace_bin_op(name) { - if !is_self(returns, checker.semantic()) { + if !is_self(returns, semantic) { checker.diagnostics.push(Diagnostic::new( NonSelfReturnType { class_name: class_def.name.to_string(), @@ -174,8 +175,7 @@ pub(crate) fn non_self_return_type( } if is_name(returns, &class_def.name) { - if matches!(name, "__enter__" | "__new__") - && !is_final(&class_def.decorator_list, checker.semantic()) + if matches!(name, "__enter__" | "__new__") && !is_final(&class_def.decorator_list, semantic) { checker.diagnostics.push(Diagnostic::new( NonSelfReturnType { @@ -190,8 +190,8 @@ pub(crate) fn non_self_return_type( match name { "__iter__" => { - if is_iterable(returns, checker.semantic()) - && is_iterator(class_def.arguments.as_deref(), checker.semantic()) + if is_iterable_or_iterator(returns, semantic) + && subclasses_iterator(class_def, semantic) { checker.diagnostics.push(Diagnostic::new( NonSelfReturnType { @@ -203,8 +203,8 @@ pub(crate) fn non_self_return_type( } } "__aiter__" => { - if is_async_iterable(returns, checker.semantic()) - && is_async_iterator(class_def.arguments.as_deref(), checker.semantic()) + if is_async_iterable_or_iterator(returns, semantic) + && subclasses_async_iterator(class_def, semantic) { checker.diagnostics.push(Diagnostic::new( NonSelfReturnType { @@ -221,26 +221,14 @@ pub(crate) fn non_self_return_type( /// Returns `true` if the given class is a metaclass. fn is_metaclass(class_def: &ast::StmtClassDef, semantic: &SemanticModel) -> bool { - class_def.arguments.as_ref().is_some_and(|arguments| { - arguments - .args - .iter() - .any(|expr| is_metaclass_base(expr, semantic)) + analyze::class::any_qualified_name(class_def, semantic, &|qualified_name| { + matches!( + qualified_name.segments(), + ["" | "builtins", "type"] | ["abc", "ABCMeta"] | ["enum", "EnumMeta" | "EnumType"] + ) }) } -/// Returns `true` if the given expression resolves to a metaclass. -fn is_metaclass_base(base: &Expr, semantic: &SemanticModel) -> bool { - semantic - .resolve_qualified_name(base) - .is_some_and(|qualified_name| { - matches!( - qualified_name.segments(), - ["" | "builtins", "type"] | ["abc", "ABCMeta"] | ["enum", "EnumMeta" | "EnumType"] - ) - }) -} - /// Returns `true` if the method is an in-place binary operator. fn is_inplace_bin_op(name: &str) -> bool { matches!( @@ -275,24 +263,17 @@ fn is_self(expr: &Expr, semantic: &SemanticModel) -> bool { } /// Return `true` if the given class extends `collections.abc.Iterator`. -fn is_iterator(arguments: Option<&Arguments>, semantic: &SemanticModel) -> bool { - let Some(Arguments { args: bases, .. }) = arguments else { - return false; - }; - bases.iter().any(|expr| { - semantic - .resolve_qualified_name(map_subscript(expr)) - .is_some_and(|qualified_name| { - matches!( - qualified_name.segments(), - ["typing", "Iterator"] | ["collections", "abc", "Iterator"] - ) - }) +fn subclasses_iterator(class_def: &ast::StmtClassDef, semantic: &SemanticModel) -> bool { + analyze::class::any_qualified_name(class_def, semantic, &|qualified_name| { + matches!( + qualified_name.segments(), + ["typing", "Iterator"] | ["collections", "abc", "Iterator"] + ) }) } -/// Return `true` if the given expression resolves to `collections.abc.Iterable`. -fn is_iterable(expr: &Expr, semantic: &SemanticModel) -> bool { +/// Return `true` if the given expression resolves to `collections.abc.Iterable` or `collections.abc.Iterator`. +fn is_iterable_or_iterator(expr: &Expr, semantic: &SemanticModel) -> bool { semantic .resolve_qualified_name(map_subscript(expr)) .is_some_and(|qualified_name| { @@ -305,24 +286,17 @@ fn is_iterable(expr: &Expr, semantic: &SemanticModel) -> bool { } /// Return `true` if the given class extends `collections.abc.AsyncIterator`. -fn is_async_iterator(arguments: Option<&Arguments>, semantic: &SemanticModel) -> bool { - let Some(Arguments { args: bases, .. }) = arguments else { - return false; - }; - bases.iter().any(|expr| { - semantic - .resolve_qualified_name(map_subscript(expr)) - .is_some_and(|qualified_name| { - matches!( - qualified_name.segments(), - ["typing", "AsyncIterator"] | ["collections", "abc", "AsyncIterator"] - ) - }) +fn subclasses_async_iterator(class_def: &ast::StmtClassDef, semantic: &SemanticModel) -> bool { + analyze::class::any_qualified_name(class_def, semantic, &|qualified_name| { + matches!( + qualified_name.segments(), + ["typing", "AsyncIterator"] | ["collections", "abc", "AsyncIterator"] + ) }) } -/// Return `true` if the given expression resolves to `collections.abc.AsyncIterable`. -fn is_async_iterable(expr: &Expr, semantic: &SemanticModel) -> bool { +/// Return `true` if the given expression resolves to `collections.abc.AsyncIterable` or `collections.abc.AsyncIterator`. +fn is_async_iterable_or_iterator(expr: &Expr, semantic: &SemanticModel) -> bool { semantic .resolve_qualified_name(map_subscript(expr)) .is_some_and(|qualified_name| { diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI034_PYI034.py.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI034_PYI034.py.snap index 93d267a767f36..d1d2bb3c95f06 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI034_PYI034.py.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI034_PYI034.py.snap @@ -89,4 +89,20 @@ PYI034.py:195:9: PYI034 `__aiter__` methods in classes like `BadAsyncIterator` u | = help: Consider using `typing_extensions.Self` as return type +PYI034.py:199:9: PYI034 `__iter__` methods in classes like `SubclassOfBadIterator3` usually return `self` at runtime + | +198 | class SubclassOfBadIterator3(BadIterator3): +199 | def __iter__(self) -> Iterator[int]: # Y034 + | ^^^^^^^^ PYI034 +200 | ... + | + = help: Consider using `typing_extensions.Self` as return type +PYI034.py:203:9: PYI034 `__aiter__` methods in classes like `SubclassOfBadAsyncIterator` usually return `self` at runtime + | +202 | class SubclassOfBadAsyncIterator(BadAsyncIterator): +203 | def __aiter__(self) -> collections.abc.AsyncIterator[str]: # Y034 + | ^^^^^^^^^ PYI034 +204 | ... + | + = help: Consider using `typing_extensions.Self` as return type diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/parametrize.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/parametrize.rs index ae9760c7e9a8d..4f7cd1c4b4dd6 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/parametrize.rs +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/parametrize.rs @@ -567,7 +567,7 @@ fn check_values(checker: &mut Checker, names: &Expr, values: &Expr) { // Replace `]` with `)` or `,)`. let values_end = Edit::replacement( if needs_trailing_comma { - "),".into() + ",)".into() } else { ")".into() }, diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT007_list_of_lists.snap b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT007_list_of_lists.snap index 1e61a32cbac51..1feb8b1d245d0 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT007_list_of_lists.snap +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT007_list_of_lists.snap @@ -168,7 +168,7 @@ PT007.py:81:38: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expect 81 | @pytest.mark.parametrize(("b", "c"), ((3, 4), (5, 6))) | ^^^^^^^^^^^^^^^^ PT007 82 | @pytest.mark.parametrize("d", [3,]) -83 | def test_multiple_decorators(a, b, c): +83 | @pytest.mark.parametrize( | = help: Use `list` of `list` for parameter values @@ -179,8 +179,8 @@ PT007.py:81:38: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expect 81 |-@pytest.mark.parametrize(("b", "c"), ((3, 4), (5, 6))) 81 |+@pytest.mark.parametrize(("b", "c"), [(3, 4), (5, 6)]) 82 82 | @pytest.mark.parametrize("d", [3,]) -83 83 | def test_multiple_decorators(a, b, c): -84 84 | pass +83 83 | @pytest.mark.parametrize( +84 84 | "d", PT007.py:81:39: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expected `list` of `list` | @@ -188,7 +188,7 @@ PT007.py:81:39: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expect 81 | @pytest.mark.parametrize(("b", "c"), ((3, 4), (5, 6))) | ^^^^^^ PT007 82 | @pytest.mark.parametrize("d", [3,]) -83 | def test_multiple_decorators(a, b, c): +83 | @pytest.mark.parametrize( | = help: Use `list` of `list` for parameter values @@ -199,8 +199,8 @@ PT007.py:81:39: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expect 81 |-@pytest.mark.parametrize(("b", "c"), ((3, 4), (5, 6))) 81 |+@pytest.mark.parametrize(("b", "c"), ([3, 4], (5, 6))) 82 82 | @pytest.mark.parametrize("d", [3,]) -83 83 | def test_multiple_decorators(a, b, c): -84 84 | pass +83 83 | @pytest.mark.parametrize( +84 84 | "d", PT007.py:81:47: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expected `list` of `list` | @@ -208,7 +208,7 @@ PT007.py:81:47: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expect 81 | @pytest.mark.parametrize(("b", "c"), ((3, 4), (5, 6))) | ^^^^^^ PT007 82 | @pytest.mark.parametrize("d", [3,]) -83 | def test_multiple_decorators(a, b, c): +83 | @pytest.mark.parametrize( | = help: Use `list` of `list` for parameter values @@ -219,5 +219,5 @@ PT007.py:81:47: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expect 81 |-@pytest.mark.parametrize(("b", "c"), ((3, 4), (5, 6))) 81 |+@pytest.mark.parametrize(("b", "c"), ((3, 4), [5, 6])) 82 82 | @pytest.mark.parametrize("d", [3,]) -83 83 | def test_multiple_decorators(a, b, c): -84 84 | pass +83 83 | @pytest.mark.parametrize( +84 84 | "d", diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT007_list_of_tuples.snap b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT007_list_of_tuples.snap index 3f208a2d6ce52..2ee341b846203 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT007_list_of_tuples.snap +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT007_list_of_tuples.snap @@ -210,7 +210,7 @@ PT007.py:81:38: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expect 81 | @pytest.mark.parametrize(("b", "c"), ((3, 4), (5, 6))) | ^^^^^^^^^^^^^^^^ PT007 82 | @pytest.mark.parametrize("d", [3,]) -83 | def test_multiple_decorators(a, b, c): +83 | @pytest.mark.parametrize( | = help: Use `list` of `tuple` for parameter values @@ -221,5 +221,5 @@ PT007.py:81:38: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expect 81 |-@pytest.mark.parametrize(("b", "c"), ((3, 4), (5, 6))) 81 |+@pytest.mark.parametrize(("b", "c"), [(3, 4), (5, 6)]) 82 82 | @pytest.mark.parametrize("d", [3,]) -83 83 | def test_multiple_decorators(a, b, c): -84 84 | pass +83 83 | @pytest.mark.parametrize( +84 84 | "d", diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT007_tuple_of_lists.snap b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT007_tuple_of_lists.snap index 55aea05664b54..93a0b2954c463 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT007_tuple_of_lists.snap +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT007_tuple_of_lists.snap @@ -237,7 +237,7 @@ PT007.py:80:31: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expect 80 |+@pytest.mark.parametrize("a", (1, 2)) 81 81 | @pytest.mark.parametrize(("b", "c"), ((3, 4), (5, 6))) 82 82 | @pytest.mark.parametrize("d", [3,]) -83 83 | def test_multiple_decorators(a, b, c): +83 83 | @pytest.mark.parametrize( PT007.py:81:39: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expected `tuple` of `list` | @@ -245,7 +245,7 @@ PT007.py:81:39: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expect 81 | @pytest.mark.parametrize(("b", "c"), ((3, 4), (5, 6))) | ^^^^^^ PT007 82 | @pytest.mark.parametrize("d", [3,]) -83 | def test_multiple_decorators(a, b, c): +83 | @pytest.mark.parametrize( | = help: Use `tuple` of `list` for parameter values @@ -256,8 +256,8 @@ PT007.py:81:39: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expect 81 |-@pytest.mark.parametrize(("b", "c"), ((3, 4), (5, 6))) 81 |+@pytest.mark.parametrize(("b", "c"), ([3, 4], (5, 6))) 82 82 | @pytest.mark.parametrize("d", [3,]) -83 83 | def test_multiple_decorators(a, b, c): -84 84 | pass +83 83 | @pytest.mark.parametrize( +84 84 | "d", PT007.py:81:47: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expected `tuple` of `list` | @@ -265,7 +265,7 @@ PT007.py:81:47: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expect 81 | @pytest.mark.parametrize(("b", "c"), ((3, 4), (5, 6))) | ^^^^^^ PT007 82 | @pytest.mark.parametrize("d", [3,]) -83 | def test_multiple_decorators(a, b, c): +83 | @pytest.mark.parametrize( | = help: Use `tuple` of `list` for parameter values @@ -276,8 +276,8 @@ PT007.py:81:47: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expect 81 |-@pytest.mark.parametrize(("b", "c"), ((3, 4), (5, 6))) 81 |+@pytest.mark.parametrize(("b", "c"), ((3, 4), [5, 6])) 82 82 | @pytest.mark.parametrize("d", [3,]) -83 83 | def test_multiple_decorators(a, b, c): -84 84 | pass +83 83 | @pytest.mark.parametrize( +84 84 | "d", PT007.py:82:31: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expected `tuple` of `list` | @@ -285,8 +285,8 @@ PT007.py:82:31: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expect 81 | @pytest.mark.parametrize(("b", "c"), ((3, 4), (5, 6))) 82 | @pytest.mark.parametrize("d", [3,]) | ^^^^ PT007 -83 | def test_multiple_decorators(a, b, c): -84 | pass +83 | @pytest.mark.parametrize( +84 | "d", | = help: Use `tuple` of `list` for parameter values @@ -296,5 +296,48 @@ PT007.py:82:31: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expect 81 81 | @pytest.mark.parametrize(("b", "c"), ((3, 4), (5, 6))) 82 |-@pytest.mark.parametrize("d", [3,]) 82 |+@pytest.mark.parametrize("d", (3,)) -83 83 | def test_multiple_decorators(a, b, c): -84 84 | pass +83 83 | @pytest.mark.parametrize( +84 84 | "d", +85 85 | [("3", "4")], + +PT007.py:85:5: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expected `tuple` of `list` + | +83 | @pytest.mark.parametrize( +84 | "d", +85 | [("3", "4")], + | ^^^^^^^^^^^^ PT007 +86 | ) +87 | @pytest.mark.parametrize( + | + = help: Use `tuple` of `list` for parameter values + +ℹ Unsafe fix +82 82 | @pytest.mark.parametrize("d", [3,]) +83 83 | @pytest.mark.parametrize( +84 84 | "d", +85 |- [("3", "4")], + 85 |+ (("3", "4"),), +86 86 | ) +87 87 | @pytest.mark.parametrize( +88 88 | "e", + +PT007.py:89:5: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expected `tuple` of `list` + | +87 | @pytest.mark.parametrize( +88 | "e", +89 | [("3", "4"),], + | ^^^^^^^^^^^^^ PT007 +90 | ) +91 | def test_multiple_decorators(a, b, c, d, e): + | + = help: Use `tuple` of `list` for parameter values + +ℹ Unsafe fix +86 86 | ) +87 87 | @pytest.mark.parametrize( +88 88 | "e", +89 |- [("3", "4"),], + 89 |+ (("3", "4"),), +90 90 | ) +91 91 | def test_multiple_decorators(a, b, c, d, e): +92 92 | pass diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT007_tuple_of_tuples.snap b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT007_tuple_of_tuples.snap index d38c58b152a24..3d90a13b2eb9c 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT007_tuple_of_tuples.snap +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT007_tuple_of_tuples.snap @@ -279,7 +279,7 @@ PT007.py:80:31: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expect 80 |+@pytest.mark.parametrize("a", (1, 2)) 81 81 | @pytest.mark.parametrize(("b", "c"), ((3, 4), (5, 6))) 82 82 | @pytest.mark.parametrize("d", [3,]) -83 83 | def test_multiple_decorators(a, b, c): +83 83 | @pytest.mark.parametrize( PT007.py:82:31: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expected `tuple` of `tuple` | @@ -287,8 +287,8 @@ PT007.py:82:31: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expect 81 | @pytest.mark.parametrize(("b", "c"), ((3, 4), (5, 6))) 82 | @pytest.mark.parametrize("d", [3,]) | ^^^^ PT007 -83 | def test_multiple_decorators(a, b, c): -84 | pass +83 | @pytest.mark.parametrize( +84 | "d", | = help: Use `tuple` of `tuple` for parameter values @@ -298,5 +298,48 @@ PT007.py:82:31: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expect 81 81 | @pytest.mark.parametrize(("b", "c"), ((3, 4), (5, 6))) 82 |-@pytest.mark.parametrize("d", [3,]) 82 |+@pytest.mark.parametrize("d", (3,)) -83 83 | def test_multiple_decorators(a, b, c): -84 84 | pass +83 83 | @pytest.mark.parametrize( +84 84 | "d", +85 85 | [("3", "4")], + +PT007.py:85:5: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expected `tuple` of `tuple` + | +83 | @pytest.mark.parametrize( +84 | "d", +85 | [("3", "4")], + | ^^^^^^^^^^^^ PT007 +86 | ) +87 | @pytest.mark.parametrize( + | + = help: Use `tuple` of `tuple` for parameter values + +ℹ Unsafe fix +82 82 | @pytest.mark.parametrize("d", [3,]) +83 83 | @pytest.mark.parametrize( +84 84 | "d", +85 |- [("3", "4")], + 85 |+ (("3", "4"),), +86 86 | ) +87 87 | @pytest.mark.parametrize( +88 88 | "e", + +PT007.py:89:5: PT007 [*] Wrong values type in `@pytest.mark.parametrize` expected `tuple` of `tuple` + | +87 | @pytest.mark.parametrize( +88 | "e", +89 | [("3", "4"),], + | ^^^^^^^^^^^^^ PT007 +90 | ) +91 | def test_multiple_decorators(a, b, c, d, e): + | + = help: Use `tuple` of `tuple` for parameter values + +ℹ Unsafe fix +86 86 | ) +87 87 | @pytest.mark.parametrize( +88 88 | "e", +89 |- [("3", "4"),], + 89 |+ (("3", "4"),), +90 90 | ) +91 91 | def test_multiple_decorators(a, b, c, d, e): +92 92 | pass diff --git a/crates/ruff_linter/src/rules/flake8_simplify/rules/needless_bool.rs b/crates/ruff_linter/src/rules/flake8_simplify/rules/needless_bool.rs index 93f533de3f88e..0ab4856483ba4 100644 --- a/crates/ruff_linter/src/rules/flake8_simplify/rules/needless_bool.rs +++ b/crates/ruff_linter/src/rules/flake8_simplify/rules/needless_bool.rs @@ -41,8 +41,8 @@ use crate::fix::snippet::SourceCodeSnippet; /// [preview]: https://docs.astral.sh/ruff/preview/ #[violation] pub struct NeedlessBool { - condition: SourceCodeSnippet, - replacement: Option, + condition: Option, + negate: bool, } impl Violation for NeedlessBool { @@ -50,21 +50,22 @@ impl Violation for NeedlessBool { #[derive_message_formats] fn message(&self) -> String { - let NeedlessBool { condition, .. } = self; - if let Some(condition) = condition.full_display() { + let NeedlessBool { condition, negate } = self; + + if let Some(condition) = condition.as_ref().and_then(SourceCodeSnippet::full_display) { format!("Return the condition `{condition}` directly") + } else if *negate { + format!("Return the negated condition directly") } else { format!("Return the condition directly") } } fn fix_title(&self) -> Option { - let NeedlessBool { replacement, .. } = self; - if let Some(replacement) = replacement - .as_ref() - .and_then(SourceCodeSnippet::full_display) - { - Some(format!("Replace with `{replacement}`")) + let NeedlessBool { condition, .. } = self; + + if let Some(condition) = condition.as_ref().and_then(SourceCodeSnippet::full_display) { + Some(format!("Replace with `return {condition}`")) } else { Some(format!("Inline condition")) } @@ -191,29 +192,21 @@ pub(crate) fn needless_bool(checker: &mut Checker, stmt: &Stmt) { return; } - let condition = checker.locator().slice(if_test); - let replacement = if checker.indexer().has_comments(&range, checker.locator()) { + // Generate the replacement condition. + let condition = if checker.indexer().has_comments(&range, checker.locator()) { None } else { // If the return values are inverted, wrap the condition in a `not`. if inverted { - let node = ast::StmtReturn { - value: Some(Box::new(Expr::UnaryOp(ast::ExprUnaryOp { - op: ast::UnaryOp::Not, - operand: Box::new(if_test.clone()), - range: TextRange::default(), - }))), + Some(Expr::UnaryOp(ast::ExprUnaryOp { + op: ast::UnaryOp::Not, + operand: Box::new(if_test.clone()), range: TextRange::default(), - }; - Some(checker.generator().stmt(&node.into())) + })) } else if if_test.is_compare_expr() { // If the condition is a comparison, we can replace it with the condition, since we // know it's a boolean. - let node = ast::StmtReturn { - value: Some(Box::new(if_test.clone())), - range: TextRange::default(), - }; - Some(checker.generator().stmt(&node.into())) + Some(if_test.clone()) } else if checker.semantic().is_builtin("bool") { // Otherwise, we need to wrap the condition in a call to `bool`. let func_node = ast::ExprName { @@ -221,7 +214,7 @@ pub(crate) fn needless_bool(checker: &mut Checker, stmt: &Stmt) { ctx: ExprContext::Load, range: TextRange::default(), }; - let value_node = ast::ExprCall { + let call_node = ast::ExprCall { func: Box::new(func_node.into()), arguments: Arguments { args: Box::from([if_test.clone()]), @@ -230,20 +223,32 @@ pub(crate) fn needless_bool(checker: &mut Checker, stmt: &Stmt) { }, range: TextRange::default(), }; - let return_node = ast::StmtReturn { - value: Some(Box::new(value_node.into())), - range: TextRange::default(), - }; - Some(checker.generator().stmt(&return_node.into())) + Some(Expr::Call(call_node)) } else { None } }; + // Generate the replacement `return` statement. + let replacement = condition.as_ref().map(|expr| { + Stmt::Return(ast::StmtReturn { + value: Some(Box::new(expr.clone())), + range: TextRange::default(), + }) + }); + + // Generate source code. + let replacement = replacement + .as_ref() + .map(|stmt| checker.generator().stmt(stmt)); + let condition = condition + .as_ref() + .map(|expr| checker.generator().expr(expr)); + let mut diagnostic = Diagnostic::new( NeedlessBool { - condition: SourceCodeSnippet::from_str(condition), - replacement: replacement.clone().map(SourceCodeSnippet::new), + condition: condition.map(SourceCodeSnippet::new), + negate: inverted, }, range, ); diff --git a/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM103_SIM103.py.snap b/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM103_SIM103.py.snap index 2ed80d20b0880..b656f423713b2 100644 --- a/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM103_SIM103.py.snap +++ b/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM103_SIM103.py.snap @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/flake8_simplify/mod.rs --- -SIM103.py:3:5: SIM103 [*] Return the condition `a` directly +SIM103.py:3:5: SIM103 [*] Return the condition `bool(a)` directly | 1 | def f(): 2 | # SIM103 @@ -52,7 +52,7 @@ SIM103.py:11:5: SIM103 [*] Return the condition `a == b` directly 16 13 | 17 14 | def f(): -SIM103.py:21:5: SIM103 [*] Return the condition `b` directly +SIM103.py:21:5: SIM103 [*] Return the condition `bool(b)` directly | 19 | if a: 20 | return 1 @@ -78,7 +78,7 @@ SIM103.py:21:5: SIM103 [*] Return the condition `b` directly 26 23 | 27 24 | def f(): -SIM103.py:32:9: SIM103 [*] Return the condition `b` directly +SIM103.py:32:9: SIM103 [*] Return the condition `bool(b)` directly | 30 | return 1 31 | else: @@ -104,10 +104,10 @@ SIM103.py:32:9: SIM103 [*] Return the condition `b` directly 37 34 | 38 35 | def f(): -SIM103.py:57:5: SIM103 [*] Return the condition `a` directly +SIM103.py:57:5: SIM103 [*] Return the condition `not a` directly | 55 | def f(): -56 | # SIM103 (but not fixable) +56 | # SIM103 57 | if a: | _____^ 58 | | return False @@ -120,7 +120,7 @@ SIM103.py:57:5: SIM103 [*] Return the condition `a` directly ℹ Unsafe fix 54 54 | 55 55 | def f(): -56 56 | # SIM103 (but not fixable) +56 56 | # SIM103 57 |- if a: 58 |- return False 59 |- else: @@ -130,7 +130,7 @@ SIM103.py:57:5: SIM103 [*] Return the condition `a` directly 62 59 | 63 60 | def f(): -SIM103.py:83:5: SIM103 Return the condition `a` directly +SIM103.py:83:5: SIM103 Return the condition directly | 81 | def bool(): 82 | return False @@ -142,3 +142,29 @@ SIM103.py:83:5: SIM103 Return the condition `a` directly | |____________________^ SIM103 | = help: Inline condition + +SIM103.py:91:5: SIM103 [*] Return the condition `not (keys is not None and notice.key not in keys)` directly + | +89 | def f(): +90 | # SIM103 +91 | if keys is not None and notice.key not in keys: + | _____^ +92 | | return False +93 | | else: +94 | | return True + | |___________________^ SIM103 + | + = help: Replace with `return not (keys is not None and notice.key not in keys)` + +ℹ Unsafe fix +88 88 | +89 89 | def f(): +90 90 | # SIM103 +91 |- if keys is not None and notice.key not in keys: +92 |- return False +93 |- else: +94 |- return True + 91 |+ return not (keys is not None and notice.key not in keys) +95 92 | +96 93 | +97 94 | ### diff --git a/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__preview__SIM103_SIM103.py.snap b/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__preview__SIM103_SIM103.py.snap index 868129a6d16bf..a71b6622c25cf 100644 --- a/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__preview__SIM103_SIM103.py.snap +++ b/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__preview__SIM103_SIM103.py.snap @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/flake8_simplify/mod.rs --- -SIM103.py:3:5: SIM103 [*] Return the condition `a` directly +SIM103.py:3:5: SIM103 [*] Return the condition `bool(a)` directly | 1 | def f(): 2 | # SIM103 @@ -52,7 +52,7 @@ SIM103.py:11:5: SIM103 [*] Return the condition `a == b` directly 16 13 | 17 14 | def f(): -SIM103.py:21:5: SIM103 [*] Return the condition `b` directly +SIM103.py:21:5: SIM103 [*] Return the condition `bool(b)` directly | 19 | if a: 20 | return 1 @@ -78,7 +78,7 @@ SIM103.py:21:5: SIM103 [*] Return the condition `b` directly 26 23 | 27 24 | def f(): -SIM103.py:32:9: SIM103 [*] Return the condition `b` directly +SIM103.py:32:9: SIM103 [*] Return the condition `bool(b)` directly | 30 | return 1 31 | else: @@ -104,10 +104,10 @@ SIM103.py:32:9: SIM103 [*] Return the condition `b` directly 37 34 | 38 35 | def f(): -SIM103.py:57:5: SIM103 [*] Return the condition `a` directly +SIM103.py:57:5: SIM103 [*] Return the condition `not a` directly | 55 | def f(): -56 | # SIM103 (but not fixable) +56 | # SIM103 57 | if a: | _____^ 58 | | return False @@ -120,7 +120,7 @@ SIM103.py:57:5: SIM103 [*] Return the condition `a` directly ℹ Unsafe fix 54 54 | 55 55 | def f(): -56 56 | # SIM103 (but not fixable) +56 56 | # SIM103 57 |- if a: 58 |- return False 59 |- else: @@ -130,7 +130,7 @@ SIM103.py:57:5: SIM103 [*] Return the condition `a` directly 62 59 | 63 60 | def f(): -SIM103.py:83:5: SIM103 Return the condition `a` directly +SIM103.py:83:5: SIM103 Return the condition directly | 81 | def bool(): 82 | return False @@ -143,47 +143,73 @@ SIM103.py:83:5: SIM103 Return the condition `a` directly | = help: Inline condition -SIM103.py:96:5: SIM103 [*] Return the condition `a` directly +SIM103.py:91:5: SIM103 [*] Return the condition `not (keys is not None and notice.key not in keys)` directly | -94 | def f(): -95 | # SIM103 -96 | if a: +89 | def f(): +90 | # SIM103 +91 | if keys is not None and notice.key not in keys: | _____^ -97 | | return True -98 | | return False - | |________________^ SIM103 +92 | | return False +93 | | else: +94 | | return True + | |___________________^ SIM103 | - = help: Replace with `return bool(a)` + = help: Replace with `return not (keys is not None and notice.key not in keys)` + +ℹ Unsafe fix +88 88 | +89 89 | def f(): +90 90 | # SIM103 +91 |- if keys is not None and notice.key not in keys: +92 |- return False +93 |- else: +94 |- return True + 91 |+ return not (keys is not None and notice.key not in keys) +95 92 | +96 93 | +97 94 | ### + +SIM103.py:104:5: SIM103 [*] Return the condition `bool(a)` directly + | +102 | def f(): +103 | # SIM103 +104 | if a: + | _____^ +105 | | return True +106 | | return False + | |________________^ SIM103 + | + = help: Replace with `return bool(a)` ℹ Unsafe fix -93 93 | -94 94 | def f(): -95 95 | # SIM103 -96 |- if a: -97 |- return True -98 |- return False - 96 |+ return bool(a) -99 97 | -100 98 | -101 99 | def f(): - -SIM103.py:103:5: SIM103 [*] Return the condition `a` directly +101 101 | +102 102 | def f(): +103 103 | # SIM103 +104 |- if a: +105 |- return True +106 |- return False + 104 |+ return bool(a) +107 105 | +108 106 | +109 107 | def f(): + +SIM103.py:111:5: SIM103 [*] Return the condition `not a` directly | -101 | def f(): -102 | # SIM103 -103 | if a: +109 | def f(): +110 | # SIM103 +111 | if a: | _____^ -104 | | return False -105 | | return True +112 | | return False +113 | | return True | |_______________^ SIM103 | = help: Replace with `return not a` ℹ Unsafe fix -100 100 | -101 101 | def f(): -102 102 | # SIM103 -103 |- if a: -104 |- return False -105 |- return True - 103 |+ return not a +108 108 | +109 109 | def f(): +110 110 | # SIM103 +111 |- if a: +112 |- return False +113 |- return True + 111 |+ return not a diff --git a/crates/ruff_linter/src/rules/flake8_slots/rules/no_slots_in_namedtuple_subclass.rs b/crates/ruff_linter/src/rules/flake8_slots/rules/no_slots_in_namedtuple_subclass.rs index 187e0bcdf9062..fdbf87efcc74d 100644 --- a/crates/ruff_linter/src/rules/flake8_slots/rules/no_slots_in_namedtuple_subclass.rs +++ b/crates/ruff_linter/src/rules/flake8_slots/rules/no_slots_in_namedtuple_subclass.rs @@ -1,17 +1,16 @@ -use ruff_python_ast as ast; -use ruff_python_ast::{Arguments, Expr, StmtClassDef}; +use std::fmt; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::identifier::Identifier; -use ruff_python_ast::Stmt; +use ruff_python_ast::{self as ast, identifier::Identifier, Arguments, Expr, Stmt, StmtClassDef}; +use ruff_python_semantic::SemanticModel; use crate::checkers::ast::Checker; use crate::rules::flake8_slots::rules::helpers::has_slots; /// ## What it does -/// Checks for subclasses of `collections.namedtuple` that lack a `__slots__` -/// definition. +/// Checks for subclasses of `collections.namedtuple` or `typing.NamedTuple` +/// that lack a `__slots__` definition. /// /// ## Why is this bad? /// In Python, the `__slots__` attribute allows you to explicitly define the @@ -48,12 +47,28 @@ use crate::rules::flake8_slots::rules::helpers::has_slots; /// ## References /// - [Python documentation: `__slots__`](https://docs.python.org/3/reference/datamodel.html#slots) #[violation] -pub struct NoSlotsInNamedtupleSubclass; +pub struct NoSlotsInNamedtupleSubclass(NamedTupleKind); impl Violation for NoSlotsInNamedtupleSubclass { #[derive_message_formats] fn message(&self) -> String { - format!("Subclasses of `collections.namedtuple()` should define `__slots__`") + let NoSlotsInNamedtupleSubclass(namedtuple_kind) = self; + format!("Subclasses of {namedtuple_kind} should define `__slots__`") + } +} + +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +enum NamedTupleKind { + Collections, + Typing, +} + +impl fmt::Display for NamedTupleKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(match self { + Self::Collections => "`collections.namedtuple()`", + Self::Typing => "call-based `typing.NamedTuple()`", + }) } } @@ -67,22 +82,33 @@ pub(crate) fn no_slots_in_namedtuple_subclass( return; }; - if bases.iter().any(|base| { - let Expr::Call(ast::ExprCall { func, .. }) = base else { - return false; - }; - checker - .semantic() - .resolve_qualified_name(func) - .is_some_and(|qualified_name| { - matches!(qualified_name.segments(), ["collections", "namedtuple"]) - }) - }) { + if let Some(namedtuple_kind) = namedtuple_base(bases, checker.semantic()) { if !has_slots(&class.body) { checker.diagnostics.push(Diagnostic::new( - NoSlotsInNamedtupleSubclass, + NoSlotsInNamedtupleSubclass(namedtuple_kind), stmt.identifier(), )); } } } + +/// If the class has a call-based namedtuple in its bases, +/// return the kind of namedtuple it is +/// (either `collections.namedtuple()`, or `typing.NamedTuple()`). +/// Else, return `None`. +fn namedtuple_base(bases: &[Expr], semantic: &SemanticModel) -> Option { + for base in bases { + let Expr::Call(ast::ExprCall { func, .. }) = base else { + continue; + }; + let Some(qualified_name) = semantic.resolve_qualified_name(func) else { + continue; + }; + match qualified_name.segments() { + ["collections", "namedtuple"] => return Some(NamedTupleKind::Collections), + ["typing", "NamedTuple"] => return Some(NamedTupleKind::Typing), + _ => continue, + } + } + None +} diff --git a/crates/ruff_linter/src/rules/flake8_slots/snapshots/ruff_linter__rules__flake8_slots__tests__SLOT002_SLOT002.py.snap b/crates/ruff_linter/src/rules/flake8_slots/snapshots/ruff_linter__rules__flake8_slots__tests__SLOT002_SLOT002.py.snap index 4ee1ad407bf3d..d7670abd56fe8 100644 --- a/crates/ruff_linter/src/rules/flake8_slots/snapshots/ruff_linter__rules__flake8_slots__tests__SLOT002_SLOT002.py.snap +++ b/crates/ruff_linter/src/rules/flake8_slots/snapshots/ruff_linter__rules__flake8_slots__tests__SLOT002_SLOT002.py.snap @@ -8,4 +8,9 @@ SLOT002.py:5:7: SLOT002 Subclasses of `collections.namedtuple()` should define ` 6 | pass | - +SLOT002.py:9:7: SLOT002 Subclasses of call-based `typing.NamedTuple()` should define `__slots__` + | + 9 | class UnusualButStillBad(NamedTuple("foo", [("x", int, "y", int)])): # SLOT002 + | ^^^^^^^^^^^^^^^^^^ SLOT002 +10 | pass + | diff --git a/crates/ruff_linter/src/rules/pyflakes/mod.rs b/crates/ruff_linter/src/rules/pyflakes/mod.rs index 503690624cac2..6f5673b1ecc39 100644 --- a/crates/ruff_linter/src/rules/pyflakes/mod.rs +++ b/crates/ruff_linter/src/rules/pyflakes/mod.rs @@ -162,6 +162,7 @@ mod tests { #[test_case(Rule::UndefinedExport, Path::new("F822_0.pyi"))] #[test_case(Rule::UndefinedExport, Path::new("F822_1.py"))] #[test_case(Rule::UndefinedExport, Path::new("F822_2.py"))] + #[test_case(Rule::UndefinedExport, Path::new("F822_3.py"))] #[test_case(Rule::UndefinedLocal, Path::new("F823.py"))] #[test_case(Rule::UnusedVariable, Path::new("F841_0.py"))] #[test_case(Rule::UnusedVariable, Path::new("F841_1.py"))] diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F822_F822_3.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F822_F822_3.py.snap new file mode 100644 index 0000000000000..5d572ea096297 --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F822_F822_3.py.snap @@ -0,0 +1,11 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +F822_3.py:12:5: F822 Undefined name `ExponentialFamily` in `__all__` + | +10 | __all__ += [ +11 | "ContinuousBernoulli", # noqa: F822 +12 | "ExponentialFamily", + | ^^^^^^^^^^^^^^^^^^^ F822 +13 | ] + | diff --git a/crates/ruff_linter/src/rules/pygrep_hooks/rules/blanket_noqa.rs b/crates/ruff_linter/src/rules/pygrep_hooks/rules/blanket_noqa.rs index 7bbf4fef771d9..5d6fc4ff0eeb0 100644 --- a/crates/ruff_linter/src/rules/pygrep_hooks/rules/blanket_noqa.rs +++ b/crates/ruff_linter/src/rules/pygrep_hooks/rules/blanket_noqa.rs @@ -1,8 +1,9 @@ -use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_index::Indexer; +use ruff_python_trivia::Cursor; use ruff_source_file::Locator; -use ruff_text_size::Ranged; +use ruff_text_size::{Ranged, TextRange, TextSize}; use crate::noqa::Directive; @@ -27,15 +28,56 @@ use crate::noqa::Directive; /// from .base import * # noqa: F403 /// ``` /// +/// ## Fix safety +/// This rule will attempt to fix blanket `noqa` annotations that appear to +/// be unintentional. For example, given `# noqa F401`, the rule will suggest +/// inserting a colon, as in `# noqa: F401`. +/// +/// While modifying `noqa` comments is generally safe, doing so may introduce +/// additional diagnostics. +/// /// ## References /// - [Ruff documentation](https://docs.astral.sh/ruff/configuration/#error-suppression) #[violation] -pub struct BlanketNOQA; +pub struct BlanketNOQA { + missing_colon: bool, + space_before_colon: bool, +} impl Violation for BlanketNOQA { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + #[derive_message_formats] fn message(&self) -> String { - format!("Use specific rule codes when using `noqa`") + let BlanketNOQA { + missing_colon, + space_before_colon, + } = self; + + // This awkward branching is necessary to ensure that the generic message is picked up by + // `derive_message_formats`. + if !missing_colon && !space_before_colon { + format!("Use specific rule codes when using `noqa`") + } else if *missing_colon { + format!("Use a colon when specifying `noqa` rule codes") + } else { + format!("Do not add spaces between `noqa` and its colon") + } + } + + fn fix_title(&self) -> Option { + let BlanketNOQA { + missing_colon, + space_before_colon, + } = self; + + if *missing_colon { + Some("Add missing colon".to_string()) + } else if *space_before_colon { + Some("Remove space(s) before colon".to_string()) + } else { + None + } } } @@ -47,8 +89,54 @@ pub(crate) fn blanket_noqa( ) { for range in indexer.comment_ranges() { let line = locator.slice(*range); - if let Ok(Some(Directive::All(all))) = Directive::try_extract(line, range.start()) { - diagnostics.push(Diagnostic::new(BlanketNOQA, all.range())); + let offset = range.start(); + if let Ok(Some(Directive::All(all))) = Directive::try_extract(line, TextSize::new(0)) { + // The `all` range is that of the `noqa` directive in, e.g., `# noqa` or `# noqa F401`. + let noqa_start = offset + all.start(); + let noqa_end = offset + all.end(); + + // Skip the `# noqa`, plus any trailing whitespace. + let mut cursor = Cursor::new(&line[all.end().to_usize()..]); + cursor.eat_while(char::is_whitespace); + + // Check for extraneous spaces before the colon. + // Ex) `# noqa : F401` + if cursor.first() == ':' { + let start = offset + all.end(); + let end = start + cursor.token_len(); + let mut diagnostic = Diagnostic::new( + BlanketNOQA { + missing_colon: false, + space_before_colon: true, + }, + TextRange::new(noqa_start, end), + ); + diagnostic.set_fix(Fix::unsafe_edit(Edit::deletion(start, end))); + diagnostics.push(diagnostic); + } else if Directive::lex_code(cursor.chars().as_str()).is_some() { + // Check for a missing colon. + // Ex) `# noqa F401` + let start = offset + all.end(); + let end = start + TextSize::new(1); + let mut diagnostic = Diagnostic::new( + BlanketNOQA { + missing_colon: true, + space_before_colon: false, + }, + TextRange::new(noqa_start, end), + ); + diagnostic.set_fix(Fix::unsafe_edit(Edit::insertion(':'.to_string(), start))); + diagnostics.push(diagnostic); + } else { + // Otherwise, it looks like an intentional blanket `noqa` annotation. + diagnostics.push(Diagnostic::new( + BlanketNOQA { + missing_colon: false, + space_before_colon: false, + }, + TextRange::new(noqa_start, noqa_end), + )); + } } } } diff --git a/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH004_PGH004_0.py.snap b/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH004_PGH004_0.py.snap index 45a79a3e85d34..5c4e0772479d1 100644 --- a/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH004_PGH004_0.py.snap +++ b/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH004_PGH004_0.py.snap @@ -29,4 +29,97 @@ PGH004_0.py:4:1: PGH004 Use specific rule codes when using `noqa` 6 | # noqa:F401,W203 | +PGH004_0.py:18:8: PGH004 [*] Use a colon when specifying `noqa` rule codes + | +17 | # PGH004 +18 | x = 2 # noqa X100 + | ^^^^^^^ PGH004 +19 | +20 | # PGH004 + | + = help: Add missing colon +ℹ Unsafe fix +15 15 | x = 2 # noqa:X100 +16 16 | +17 17 | # PGH004 +18 |-x = 2 # noqa X100 + 18 |+x = 2 # noqa: X100 +19 19 | +20 20 | # PGH004 +21 21 | x = 2 # noqa X100, X200 + +PGH004_0.py:21:8: PGH004 [*] Use a colon when specifying `noqa` rule codes + | +20 | # PGH004 +21 | x = 2 # noqa X100, X200 + | ^^^^^^^ PGH004 +22 | +23 | # PGH004 + | + = help: Add missing colon + +ℹ Unsafe fix +18 18 | x = 2 # noqa X100 +19 19 | +20 20 | # PGH004 +21 |-x = 2 # noqa X100, X200 + 21 |+x = 2 # noqa: X100, X200 +22 22 | +23 23 | # PGH004 +24 24 | x = 2 # noqa : X300 + +PGH004_0.py:24:8: PGH004 [*] Do not add spaces between `noqa` and its colon + | +23 | # PGH004 +24 | x = 2 # noqa : X300 + | ^^^^^^^ PGH004 +25 | +26 | # PGH004 + | + = help: Remove space(s) before colon + +ℹ Unsafe fix +21 21 | x = 2 # noqa X100, X200 +22 22 | +23 23 | # PGH004 +24 |-x = 2 # noqa : X300 + 24 |+x = 2 # noqa: X300 +25 25 | +26 26 | # PGH004 +27 27 | x = 2 # noqa : X400 + +PGH004_0.py:27:8: PGH004 [*] Do not add spaces between `noqa` and its colon + | +26 | # PGH004 +27 | x = 2 # noqa : X400 + | ^^^^^^^^ PGH004 +28 | +29 | # PGH004 + | + = help: Remove space(s) before colon + +ℹ Unsafe fix +24 24 | x = 2 # noqa : X300 +25 25 | +26 26 | # PGH004 +27 |-x = 2 # noqa : X400 + 27 |+x = 2 # noqa: X400 +28 28 | +29 29 | # PGH004 +30 30 | x = 2 # noqa :X500 + +PGH004_0.py:30:8: PGH004 [*] Do not add spaces between `noqa` and its colon + | +29 | # PGH004 +30 | x = 2 # noqa :X500 + | ^^^^^^^ PGH004 + | + = help: Remove space(s) before colon + +ℹ Unsafe fix +27 27 | x = 2 # noqa : X400 +28 28 | +29 29 | # PGH004 +30 |-x = 2 # noqa :X500 + 30 |+x = 2 # noqa:X500 diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs index 867e6dfc9596b..ed77947eb8b62 100644 --- a/crates/ruff_linter/src/rules/pylint/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/mod.rs @@ -47,6 +47,7 @@ mod tests { #[test_case(Rule::EqWithoutHash, Path::new("eq_without_hash.py"))] #[test_case(Rule::EmptyComment, Path::new("empty_comment.py"))] #[test_case(Rule::ManualFromImport, Path::new("import_aliasing.py"))] + #[test_case(Rule::IfStmtMinMax, Path::new("if_stmt_min_max.py"))] #[test_case(Rule::SingleStringSlots, Path::new("single_string_slots.py"))] #[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_0.py"))] #[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_1.py"))] diff --git a/crates/ruff_linter/src/rules/pylint/rules/if_stmt_min_max.rs b/crates/ruff_linter/src/rules/pylint/rules/if_stmt_min_max.rs new file mode 100644 index 0000000000000..dfbca11d10aff --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/rules/if_stmt_min_max.rs @@ -0,0 +1,196 @@ +use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::comparable::ComparableExpr; +use ruff_python_ast::parenthesize::parenthesized_range; +use ruff_python_ast::{self as ast, CmpOp, Stmt}; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; +use crate::fix::snippet::SourceCodeSnippet; + +/// ## What it does +/// Checks for `if` statements that can be replaced with `min()` or `max()` +/// calls. +/// +/// ## Why is this bad? +/// An `if` statement that selects the lesser or greater of two sub-expressions +/// can be replaced with a `min()` or `max()` call respectively. When possible, +/// prefer `min()` and `max()`, as they're more concise and readable than the +/// equivalent `if` statements. +/// +/// ## Example +/// ```python +/// if score > highest_score: +/// highest_score = score +/// ``` +/// +/// Use instead: +/// ```python +/// highest_score = max(highest_score, score) +/// ``` +/// +/// ## References +/// - [Python documentation: max function](https://docs.python.org/3/library/functions.html#max) +/// - [Python documentation: min function](https://docs.python.org/3/library/functions.html#min) +#[violation] +pub struct IfStmtMinMax { + min_max: MinMax, + replacement: SourceCodeSnippet, +} + +impl Violation for IfStmtMinMax { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + + #[derive_message_formats] + fn message(&self) -> String { + let Self { + min_max, + replacement, + } = self; + if let Some(replacement) = replacement.full_display() { + format!("Replace `if` statement with `{replacement}`") + } else { + format!("Replace `if` statement with `{min_max}` call") + } + } + + fn fix_title(&self) -> Option { + let Self { + min_max, + replacement, + } = self; + if let Some(replacement) = replacement.full_display() { + Some(format!("Replace with `{replacement}`")) + } else { + Some(format!("Replace with `{min_max}` call")) + } + } +} + +/// R1730, R1731 +pub(crate) fn if_stmt_min_max(checker: &mut Checker, stmt_if: &ast::StmtIf) { + let ast::StmtIf { + test, + body, + elif_else_clauses, + range: _, + } = stmt_if; + + if !elif_else_clauses.is_empty() { + return; + } + + let [body @ Stmt::Assign(ast::StmtAssign { + targets: body_targets, + value: body_value, + .. + })] = body.as_slice() + else { + return; + }; + let [body_target] = body_targets.as_slice() else { + return; + }; + + let Some(ast::ExprCompare { + ops, + left, + comparators, + .. + }) = test.as_compare_expr() + else { + return; + }; + + // Ignore, e.g., `foo < bar < baz`. + let [op] = &**ops else { + return; + }; + + // Determine whether to use `min()` or `max()`, and whether to flip the + // order of the arguments, which is relevant for breaking ties. + let (min_max, flip_args) = match op { + CmpOp::Gt => (MinMax::Max, true), + CmpOp::GtE => (MinMax::Max, false), + CmpOp::Lt => (MinMax::Min, true), + CmpOp::LtE => (MinMax::Min, false), + _ => return, + }; + + let [right] = &**comparators else { + return; + }; + + let _min_or_max = match op { + CmpOp::Gt | CmpOp::GtE => MinMax::Min, + CmpOp::Lt | CmpOp::LtE => MinMax::Max, + _ => return, + }; + + let left_cmp = ComparableExpr::from(left); + let body_target_cmp = ComparableExpr::from(body_target); + let right_statement_cmp = ComparableExpr::from(right); + let body_value_cmp = ComparableExpr::from(body_value); + if left_cmp != body_target_cmp || right_statement_cmp != body_value_cmp { + return; + } + + let (arg1, arg2) = if flip_args { + (left.as_ref(), right) + } else { + (right, left.as_ref()) + }; + + let replacement = format!( + "{} = {min_max}({}, {})", + checker.locator().slice( + parenthesized_range( + body_target.into(), + body.into(), + checker.indexer().comment_ranges(), + checker.locator().contents() + ) + .unwrap_or(body_target.range()) + ), + checker.locator().slice(arg1), + checker.locator().slice(arg2), + ); + + let mut diagnostic = Diagnostic::new( + IfStmtMinMax { + min_max, + replacement: SourceCodeSnippet::from_str(replacement.as_str()), + }, + stmt_if.range(), + ); + + if checker.semantic().is_builtin(min_max.as_str()) { + diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( + replacement, + stmt_if.range(), + ))); + } + + checker.diagnostics.push(diagnostic); +} + +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +enum MinMax { + Min, + Max, +} + +impl MinMax { + fn as_str(self) -> &'static str { + match self { + Self::Min => "min", + Self::Max => "max", + } + } +} + +impl std::fmt::Display for MinMax { + fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(fmt, "{}", self.as_str()) + } +} diff --git a/crates/ruff_linter/src/rules/pylint/rules/mod.rs b/crates/ruff_linter/src/rules/pylint/rules/mod.rs index 93f89e1f6cec0..56cb0edd97f9b 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/mod.rs @@ -21,6 +21,7 @@ pub(crate) use eq_without_hash::*; pub(crate) use global_at_module_level::*; pub(crate) use global_statement::*; pub(crate) use global_variable_not_assigned::*; +pub(crate) use if_stmt_min_max::*; pub(crate) use import_outside_top_level::*; pub(crate) use import_private_name::*; pub(crate) use import_self::*; @@ -116,6 +117,7 @@ mod eq_without_hash; mod global_at_module_level; mod global_statement; mod global_variable_not_assigned; +mod if_stmt_min_max; mod import_outside_top_level; mod import_private_name; mod import_self; diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1730_if_stmt_min_max.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1730_if_stmt_min_max.py.snap new file mode 100644 index 0000000000000..70ff72b378ae6 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1730_if_stmt_min_max.py.snap @@ -0,0 +1,296 @@ +--- +source: crates/ruff_linter/src/rules/pylint/mod.rs +--- +if_stmt_min_max.py:8:1: PLR1730 [*] Replace `if` statement with `value = min(value, 10)` + | + 7 | # Positive + 8 | / if value < 10: # [max-instead-of-if] + 9 | | value = 10 + | |______________^ PLR1730 +10 | +11 | if value <= 10: # [max-instead-of-if] + | + = help: Replace with `value = min(value, 10)` + +ℹ Safe fix +5 5 | value3 = 3 +6 6 | +7 7 | # Positive +8 |-if value < 10: # [max-instead-of-if] +9 |- value = 10 + 8 |+value = min(value, 10) +10 9 | +11 10 | if value <= 10: # [max-instead-of-if] +12 11 | value = 10 + +if_stmt_min_max.py:11:1: PLR1730 [*] Replace `if` statement with `value = min(10, value)` + | + 9 | value = 10 +10 | +11 | / if value <= 10: # [max-instead-of-if] +12 | | value = 10 + | |______________^ PLR1730 +13 | +14 | if value < value2: # [max-instead-of-if] + | + = help: Replace with `value = min(10, value)` + +ℹ Safe fix +8 8 | if value < 10: # [max-instead-of-if] +9 9 | value = 10 +10 10 | +11 |-if value <= 10: # [max-instead-of-if] +12 |- value = 10 + 11 |+value = min(10, value) +13 12 | +14 13 | if value < value2: # [max-instead-of-if] +15 14 | value = value2 + +if_stmt_min_max.py:14:1: PLR1730 [*] Replace `if` statement with `value = min(value, value2)` + | +12 | value = 10 +13 | +14 | / if value < value2: # [max-instead-of-if] +15 | | value = value2 + | |__________________^ PLR1730 +16 | +17 | if value > 10: # [min-instead-of-if] + | + = help: Replace with `value = min(value, value2)` + +ℹ Safe fix +11 11 | if value <= 10: # [max-instead-of-if] +12 12 | value = 10 +13 13 | +14 |-if value < value2: # [max-instead-of-if] +15 |- value = value2 + 14 |+value = min(value, value2) +16 15 | +17 16 | if value > 10: # [min-instead-of-if] +18 17 | value = 10 + +if_stmt_min_max.py:17:1: PLR1730 [*] Replace `if` statement with `value = max(value, 10)` + | +15 | value = value2 +16 | +17 | / if value > 10: # [min-instead-of-if] +18 | | value = 10 + | |______________^ PLR1730 +19 | +20 | if value >= 10: # [min-instead-of-if] + | + = help: Replace with `value = max(value, 10)` + +ℹ Safe fix +14 14 | if value < value2: # [max-instead-of-if] +15 15 | value = value2 +16 16 | +17 |-if value > 10: # [min-instead-of-if] +18 |- value = 10 + 17 |+value = max(value, 10) +19 18 | +20 19 | if value >= 10: # [min-instead-of-if] +21 20 | value = 10 + +if_stmt_min_max.py:20:1: PLR1730 [*] Replace `if` statement with `value = max(10, value)` + | +18 | value = 10 +19 | +20 | / if value >= 10: # [min-instead-of-if] +21 | | value = 10 + | |______________^ PLR1730 +22 | +23 | if value > value2: # [min-instead-of-if] + | + = help: Replace with `value = max(10, value)` + +ℹ Safe fix +17 17 | if value > 10: # [min-instead-of-if] +18 18 | value = 10 +19 19 | +20 |-if value >= 10: # [min-instead-of-if] +21 |- value = 10 + 20 |+value = max(10, value) +22 21 | +23 22 | if value > value2: # [min-instead-of-if] +24 23 | value = value2 + +if_stmt_min_max.py:23:1: PLR1730 [*] Replace `if` statement with `value = max(value, value2)` + | +21 | value = 10 +22 | +23 | / if value > value2: # [min-instead-of-if] +24 | | value = value2 + | |__________________^ PLR1730 + | + = help: Replace with `value = max(value, value2)` + +ℹ Safe fix +20 20 | if value >= 10: # [min-instead-of-if] +21 21 | value = 10 +22 22 | +23 |-if value > value2: # [min-instead-of-if] +24 |- value = value2 + 23 |+value = max(value, value2) +25 24 | +26 25 | +27 26 | class A: + +if_stmt_min_max.py:33:1: PLR1730 [*] Replace `if` statement with `A1.value = min(A1.value, 10)` + | +32 | A1 = A() +33 | / if A1.value < 10: # [max-instead-of-if] +34 | | A1.value = 10 + | |_________________^ PLR1730 +35 | +36 | if A1.value > 10: # [min-instead-of-if] + | + = help: Replace with `A1.value = min(A1.value, 10)` + +ℹ Safe fix +30 30 | +31 31 | +32 32 | A1 = A() +33 |-if A1.value < 10: # [max-instead-of-if] +34 |- A1.value = 10 + 33 |+A1.value = min(A1.value, 10) +35 34 | +36 35 | if A1.value > 10: # [min-instead-of-if] +37 36 | A1.value = 10 + +if_stmt_min_max.py:36:1: PLR1730 [*] Replace `if` statement with `A1.value = max(A1.value, 10)` + | +34 | A1.value = 10 +35 | +36 | / if A1.value > 10: # [min-instead-of-if] +37 | | A1.value = 10 + | |_________________^ PLR1730 + | + = help: Replace with `A1.value = max(A1.value, 10)` + +ℹ Safe fix +33 33 | if A1.value < 10: # [max-instead-of-if] +34 34 | A1.value = 10 +35 35 | +36 |-if A1.value > 10: # [min-instead-of-if] +37 |- A1.value = 10 + 36 |+A1.value = max(A1.value, 10) +38 37 | +39 38 | +40 39 | class AA: + +if_stmt_min_max.py:60:1: PLR1730 [*] Replace `if` statement with `A2 = min(A2, A1)` + | +58 | A2 = AA(3) +59 | +60 | / if A2 < A1: # [max-instead-of-if] +61 | | A2 = A1 + | |___________^ PLR1730 +62 | +63 | if A2 <= A1: # [max-instead-of-if] + | + = help: Replace with `A2 = min(A2, A1)` + +ℹ Safe fix +57 57 | A1 = AA(0) +58 58 | A2 = AA(3) +59 59 | +60 |-if A2 < A1: # [max-instead-of-if] +61 |- A2 = A1 + 60 |+A2 = min(A2, A1) +62 61 | +63 62 | if A2 <= A1: # [max-instead-of-if] +64 63 | A2 = A1 + +if_stmt_min_max.py:63:1: PLR1730 [*] Replace `if` statement with `A2 = min(A1, A2)` + | +61 | A2 = A1 +62 | +63 | / if A2 <= A1: # [max-instead-of-if] +64 | | A2 = A1 + | |___________^ PLR1730 +65 | +66 | if A2 > A1: # [min-instead-of-if] + | + = help: Replace with `A2 = min(A1, A2)` + +ℹ Safe fix +60 60 | if A2 < A1: # [max-instead-of-if] +61 61 | A2 = A1 +62 62 | +63 |-if A2 <= A1: # [max-instead-of-if] +64 |- A2 = A1 + 63 |+A2 = min(A1, A2) +65 64 | +66 65 | if A2 > A1: # [min-instead-of-if] +67 66 | A2 = A1 + +if_stmt_min_max.py:66:1: PLR1730 [*] Replace `if` statement with `A2 = max(A2, A1)` + | +64 | A2 = A1 +65 | +66 | / if A2 > A1: # [min-instead-of-if] +67 | | A2 = A1 + | |___________^ PLR1730 +68 | +69 | if A2 >= A1: # [min-instead-of-if] + | + = help: Replace with `A2 = max(A2, A1)` + +ℹ Safe fix +63 63 | if A2 <= A1: # [max-instead-of-if] +64 64 | A2 = A1 +65 65 | +66 |-if A2 > A1: # [min-instead-of-if] +67 |- A2 = A1 + 66 |+A2 = max(A2, A1) +68 67 | +69 68 | if A2 >= A1: # [min-instead-of-if] +70 69 | A2 = A1 + +if_stmt_min_max.py:69:1: PLR1730 [*] Replace `if` statement with `A2 = max(A1, A2)` + | +67 | A2 = A1 +68 | +69 | / if A2 >= A1: # [min-instead-of-if] +70 | | A2 = A1 + | |___________^ PLR1730 +71 | +72 | # Negative + | + = help: Replace with `A2 = max(A1, A2)` + +ℹ Safe fix +66 66 | if A2 > A1: # [min-instead-of-if] +67 67 | A2 = A1 +68 68 | +69 |-if A2 >= A1: # [min-instead-of-if] +70 |- A2 = A1 + 69 |+A2 = max(A1, A2) +71 70 | +72 71 | # Negative +73 72 | if value < 10: + +if_stmt_min_max.py:132:1: PLR1730 [*] Replace `if` statement with `max` call + | +131 | # Parenthesized expressions +132 | / if value.attr > 3: +133 | | ( +134 | | value. +135 | | attr +136 | | ) = 3 + | |_________^ PLR1730 + | + = help: Replace with `max` call + +ℹ Safe fix +129 129 | value = 2 +130 130 | +131 131 | # Parenthesized expressions +132 |-if value.attr > 3: +133 |- ( + 132 |+( +134 133 | value. +135 134 | attr +136 |- ) = 3 + 135 |+ ) = max(value.attr, 3) diff --git a/crates/ruff_linter/src/rules/pyupgrade/mod.rs b/crates/ruff_linter/src/rules/pyupgrade/mod.rs index 10fae94973062..427b448d765b1 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/mod.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/mod.rs @@ -61,6 +61,7 @@ mod tests { #[test_case(Rule::ReplaceUniversalNewlines, Path::new("UP021.py"))] #[test_case(Rule::SuperCallWithParameters, Path::new("UP008.py"))] #[test_case(Rule::TimeoutErrorAlias, Path::new("UP041.py"))] + #[test_case(Rule::ReplaceStrEnum, Path::new("UP042.py"))] #[test_case(Rule::TypeOfPrimitive, Path::new("UP003.py"))] #[test_case(Rule::TypingTextStrAlias, Path::new("UP019.py"))] #[test_case(Rule::UTF8EncodingDeclaration, Path::new("UP009_0.py"))] diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/mod.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/mod.rs index aa6a60732fbed..3b7928f6e9020 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/mod.rs @@ -18,6 +18,7 @@ pub(crate) use printf_string_formatting::*; pub(crate) use quoted_annotation::*; pub(crate) use redundant_open_modes::*; pub(crate) use replace_stdout_stderr::*; +pub(crate) use replace_str_enum::*; pub(crate) use replace_universal_newlines::*; pub(crate) use super_call_with_parameters::*; pub(crate) use timeout_error_alias::*; @@ -58,6 +59,7 @@ mod printf_string_formatting; mod quoted_annotation; mod redundant_open_modes; mod replace_stdout_stderr; +mod replace_str_enum; mod replace_universal_newlines; mod super_call_with_parameters; mod timeout_error_alias; diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/replace_str_enum.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/replace_str_enum.rs new file mode 100644 index 0000000000000..4717ce2a5347f --- /dev/null +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/replace_str_enum.rs @@ -0,0 +1,160 @@ +use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast as ast; +use ruff_python_ast::identifier::Identifier; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; +use crate::importer::ImportRequest; + +/// ## What it does +/// Checks for classes that inherit from both `str` and `enum.Enum`. +/// +/// ## Why is this bad? +/// Python 3.11 introduced `enum.StrEnum`, which is preferred over inheriting +/// from both `str` and `enum.Enum`. +/// +/// ## Example +/// +/// ```python +/// import enum +/// +/// +/// class Foo(str, enum.Enum): +/// ... +/// ``` +/// +/// Use instead: +/// +/// ```python +/// import enum +/// +/// +/// class Foo(enum.StrEnum): +/// ... +/// ``` +/// +/// ## Fix safety +/// +/// Python 3.11 introduced a [breaking change] for enums that inherit from both +/// `str` and `enum.Enum`. Consider the following enum: +/// +/// ```python +/// from enum import Enum +/// +/// +/// class Foo(str, Enum): +/// BAR = "bar" +/// ``` +/// +/// In Python 3.11, the formatted representation of `Foo.BAR` changed as +/// follows: +/// +/// ```python +/// # Python 3.10 +/// f"{Foo.BAR}" # > bar +/// # Python 3.11 +/// f"{Foo.BAR}" # > Foo.BAR +/// ``` +/// +/// Migrating from `str` and `enum.Enum` to `enum.StrEnum` will restore the +/// previous behavior, such that: +/// +/// ```python +/// from enum import StrEnum +/// +/// +/// class Foo(StrEnum): +/// BAR = "bar" +/// +/// +/// f"{Foo.BAR}" # > bar +/// ``` +/// +/// As such, migrating to `enum.StrEnum` will introduce a behavior change for +/// code that relies on the Python 3.11 behavior. +/// +/// ## References +/// - [enum.StrEnum](https://docs.python.org/3/library/enum.html#enum.StrEnum) +/// +/// [breaking change]: https://blog.pecar.me/python-enum + +#[violation] +pub struct ReplaceStrEnum { + name: String, +} + +impl Violation for ReplaceStrEnum { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + + #[derive_message_formats] + fn message(&self) -> String { + let ReplaceStrEnum { name } = self; + format!("Class {name} inherits from both `str` and `enum.Enum`") + } + + fn fix_title(&self) -> Option { + Some("Inherit from `enum.StrEnum`".to_string()) + } +} + +/// UP042 +pub(crate) fn replace_str_enum(checker: &mut Checker, class_def: &ast::StmtClassDef) { + let Some(arguments) = class_def.arguments.as_deref() else { + // class does not inherit anything, exit early + return; + }; + + // Determine whether the class inherits from both `str` and `enum.Enum`. + let mut inherits_str = false; + let mut inherits_enum = false; + for base in arguments.args.iter() { + if let Some(qualified_name) = checker.semantic().resolve_qualified_name(base) { + match qualified_name.segments() { + ["", "str"] => inherits_str = true, + ["enum", "Enum"] => inherits_enum = true, + _ => {} + } + } + + // Short-circuit if both `str` and `enum.Enum` are found. + if inherits_str && inherits_enum { + break; + } + } + + // If the class does not inherit both `str` and `enum.Enum`, exit early. + if !inherits_str || !inherits_enum { + return; + }; + + let mut diagnostic = Diagnostic::new( + ReplaceStrEnum { + name: class_def.name.to_string(), + }, + class_def.identifier(), + ); + + // If the base classes are _exactly_ `str` and `enum.Enum`, apply a fix. + // TODO(charlie): As an alternative, we could remove both arguments, and replace one of the two + // with `StrEnum`. However, `remove_argument` can't be applied multiple times within a single + // fix; doing so leads to a syntax error. + if arguments.len() == 2 { + diagnostic.try_set_fix(|| { + let (import_edit, binding) = checker.importer().get_or_import_symbol( + &ImportRequest::import("enum", "StrEnum"), + class_def.start(), + checker.semantic(), + )?; + Ok(Fix::unsafe_edits( + import_edit, + [Edit::range_replacement( + format!("({binding})"), + arguments.range(), + )], + )) + }); + } + + checker.diagnostics.push(diagnostic); +} diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP042.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP042.py.snap new file mode 100644 index 0000000000000..e7bff6e26969f --- /dev/null +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP042.py.snap @@ -0,0 +1,55 @@ +--- +source: crates/ruff_linter/src/rules/pyupgrade/mod.rs +--- +UP042.py:4:7: UP042 [*] Class A inherits from both `str` and `enum.Enum` + | +4 | class A(str, Enum): ... + | ^ UP042 + | + = help: Inherit from `enum.StrEnum` + +ℹ Unsafe fix +1 |-from enum import Enum + 1 |+from enum import Enum, StrEnum +2 2 | +3 3 | +4 |-class A(str, Enum): ... + 4 |+class A(StrEnum): ... +5 5 | +6 6 | +7 7 | class B(Enum, str): ... + +UP042.py:7:7: UP042 [*] Class B inherits from both `str` and `enum.Enum` + | +7 | class B(Enum, str): ... + | ^ UP042 + | + = help: Inherit from `enum.StrEnum` + +ℹ Unsafe fix +1 |-from enum import Enum + 1 |+from enum import Enum, StrEnum +2 2 | +3 3 | +4 4 | class A(str, Enum): ... +5 5 | +6 6 | +7 |-class B(Enum, str): ... + 7 |+class B(StrEnum): ... +8 8 | +9 9 | +10 10 | class D(int, str, Enum): ... + +UP042.py:10:7: UP042 Class D inherits from both `str` and `enum.Enum` + | +10 | class D(int, str, Enum): ... + | ^ UP042 + | + = help: Inherit from `enum.StrEnum` + +UP042.py:13:7: UP042 Class E inherits from both `str` and `enum.Enum` + | +13 | class E(str, int, Enum): ... + | ^ UP042 + | + = help: Inherit from `enum.StrEnum` diff --git a/crates/ruff_linter/src/rules/refurb/mod.rs b/crates/ruff_linter/src/rules/refurb/mod.rs index 37a91ca51dfbf..812e2f30309dd 100644 --- a/crates/ruff_linter/src/rules/refurb/mod.rs +++ b/crates/ruff_linter/src/rules/refurb/mod.rs @@ -16,6 +16,7 @@ mod tests { #[test_case(Rule::ReadWholeFile, Path::new("FURB101.py"))] #[test_case(Rule::RepeatedAppend, Path::new("FURB113.py"))] + #[test_case(Rule::IfExpInsteadOfOrOperator, Path::new("FURB110.py"))] #[test_case(Rule::ReimplementedOperator, Path::new("FURB118.py"))] #[test_case(Rule::ReadlinesInFor, Path::new("FURB129.py"))] #[test_case(Rule::DeleteFullSlice, Path::new("FURB131.py"))] diff --git a/crates/ruff_linter/src/rules/refurb/rules/if_exp_instead_of_or_operator.rs b/crates/ruff_linter/src/rules/refurb/rules/if_exp_instead_of_or_operator.rs new file mode 100644 index 0000000000000..3096f9a62b0b7 --- /dev/null +++ b/crates/ruff_linter/src/rules/refurb/rules/if_exp_instead_of_or_operator.rs @@ -0,0 +1,101 @@ +use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast as ast; +use ruff_python_ast::comparable::ComparableExpr; +use ruff_python_ast::helpers::contains_effect; +use ruff_python_ast::parenthesize::parenthesized_range; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for ternary `if` expressions that can be replaced with the `or` +/// operator. +/// +/// ## Why is this bad? +/// Ternary `if` expressions are more verbose than `or` expressions while +/// providing the same functionality. +/// +/// ## Example +/// ```python +/// z = x if x else y +/// ``` +/// +/// Use instead: +/// ```python +/// z = x or y +/// ``` +/// +/// ## Fix safety +/// This rule's fix is marked as unsafe in the event that the body of the +/// `if` expression contains side effects. +/// +/// For example, `foo` will be called twice in `foo() if foo() else bar()` +/// (assuming `foo()` returns a truthy value), but only once in +/// `foo() or bar()`. +#[violation] +pub struct IfExpInsteadOfOrOperator; + +impl Violation for IfExpInsteadOfOrOperator { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + + #[derive_message_formats] + fn message(&self) -> String { + format!("Replace ternary `if` expression with `or` operator") + } + + fn fix_title(&self) -> Option { + Some(format!("Replace with `or` operator")) + } +} + +/// FURB110 +pub(crate) fn if_exp_instead_of_or_operator(checker: &mut Checker, if_expr: &ast::ExprIf) { + let ast::ExprIf { + test, + body, + orelse, + range, + } = if_expr; + + if ComparableExpr::from(test) != ComparableExpr::from(body) { + return; + } + + let mut diagnostic = Diagnostic::new(IfExpInsteadOfOrOperator, *range); + + // Grab the range of the `test` and `orelse` expressions. + let left = parenthesized_range( + test.into(), + if_expr.into(), + checker.indexer().comment_ranges(), + checker.locator().contents(), + ) + .unwrap_or(test.range()); + let right = parenthesized_range( + orelse.into(), + if_expr.into(), + checker.indexer().comment_ranges(), + checker.locator().contents(), + ) + .unwrap_or(orelse.range()); + + // Replace with `{test} or {orelse}`. + diagnostic.set_fix(Fix::applicable_edit( + Edit::range_replacement( + format!( + "{} or {}", + checker.locator().slice(left), + checker.locator().slice(right), + ), + if_expr.range(), + ), + if contains_effect(body, |id| checker.semantic().is_builtin(id)) { + Applicability::Unsafe + } else { + Applicability::Safe + }, + )); + + checker.diagnostics.push(diagnostic); +} diff --git a/crates/ruff_linter/src/rules/refurb/rules/mod.rs b/crates/ruff_linter/src/rules/refurb/rules/mod.rs index 34d760b48dd05..6f86341e7a8ec 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/mod.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/mod.rs @@ -3,6 +3,7 @@ pub(crate) use check_and_remove_from_set::*; pub(crate) use delete_full_slice::*; pub(crate) use for_loop_set_mutations::*; pub(crate) use hashlib_digest_hex::*; +pub(crate) use if_exp_instead_of_or_operator::*; pub(crate) use if_expr_min_max::*; pub(crate) use implicit_cwd::*; pub(crate) use int_on_sliced_str::*; @@ -31,6 +32,7 @@ mod check_and_remove_from_set; mod delete_full_slice; mod for_loop_set_mutations; mod hashlib_digest_hex; +mod if_exp_instead_of_or_operator; mod if_expr_min_max; mod implicit_cwd; mod int_on_sliced_str; diff --git a/crates/ruff_linter/src/rules/refurb/rules/read_whole_file.rs b/crates/ruff_linter/src/rules/refurb/rules/read_whole_file.rs index c6e1f31cde00f..dd653ffcf154d 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/read_whole_file.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/read_whole_file.rs @@ -142,6 +142,10 @@ fn find_file_open<'a>( return None; } + if matches!(mode, helpers::OpenMode::ReadBytes) && !keywords.is_empty() { + return None; + } + // Now we need to find what is this variable bound to... let scope = semantic.current_scope(); let bindings: Vec = scope.get_all(var.id.as_str()).collect(); diff --git a/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs b/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs index 7f5f0ea75973f..c13a385563ef5 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs @@ -1,9 +1,13 @@ +use std::borrow::Cow; +use std::fmt::{Debug, Display, Formatter}; + use anyhow::Result; use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::{self as ast, Expr, Stmt}; +use ruff_python_ast::{self as ast, Expr, ExprSlice, ExprSubscript, ExprTuple, Parameters, Stmt}; use ruff_python_semantic::SemanticModel; +use ruff_source_file::Locator; use ruff_text_size::{Ranged, TextRange}; use crate::checkers::ast::Checker; @@ -39,7 +43,7 @@ use crate::importer::{ImportRequest, Importer}; /// ## References #[violation] pub struct ReimplementedOperator { - operator: &'static str, + operator: Operator, target: FunctionLikeKind, } @@ -71,9 +75,10 @@ pub(crate) fn reimplemented_operator(checker: &mut Checker, target: &FunctionLik return; }; let Some(body) = target.body() else { return }; - let Some(operator) = get_operator(body, params) else { + let Some(operator) = get_operator(body, params, checker.locator()) else { return; }; + let fix = target.try_fix(&operator, checker.importer(), checker.semantic()); let mut diagnostic = Diagnostic::new( ReimplementedOperator { operator, @@ -81,8 +86,7 @@ pub(crate) fn reimplemented_operator(checker: &mut Checker, target: &FunctionLik }, target.range(), ); - diagnostic - .try_set_optional_fix(|| target.try_fix(operator, checker.importer(), checker.semantic())); + diagnostic.try_set_optional_fix(|| fix); checker.diagnostics.push(diagnostic); } @@ -115,8 +119,8 @@ impl Ranged for FunctionLike<'_> { } impl FunctionLike<'_> { - /// Return the [`ast::Parameters`] of the function-like node. - fn parameters(&self) -> Option<&ast::Parameters> { + /// Return the [`Parameters`] of the function-like node. + fn parameters(&self) -> Option<&Parameters> { match self { Self::Lambda(expr) => expr.parameters.as_deref(), Self::Function(stmt) => Some(&stmt.parameters), @@ -149,19 +153,24 @@ impl FunctionLike<'_> { /// function from `operator` module. fn try_fix( &self, - operator: &'static str, + operator: &Operator, importer: &Importer, semantic: &SemanticModel, ) -> Result> { match self { Self::Lambda(_) => { let (edit, binding) = importer.get_or_import_symbol( - &ImportRequest::import("operator", operator), + &ImportRequest::import("operator", operator.name), self.start(), semantic, )?; + let content = if operator.args.is_empty() { + binding + } else { + format!("{binding}({})", operator.args.join(", ")) + }; Ok(Some(Fix::safe_edits( - Edit::range_replacement(binding, self.range()), + Edit::range_replacement(content, self.range()), [edit], ))) } @@ -170,12 +179,112 @@ impl FunctionLike<'_> { } } -/// Return the name of the `operator` implemented by the given expression. -fn get_operator(expr: &Expr, params: &ast::Parameters) -> Option<&'static str> { +/// Convert the slice expression to the string representation of `slice` call. +/// For example, expression `1:2` will be `slice(1, 2)`, and `:` will be `slice(None)`. +fn slice_expr_to_slice_call(slice: &ExprSlice, locator: &Locator) -> String { + let stringify = |expr: Option<&Expr>| expr.map_or("None", |expr| locator.slice(expr)); + match ( + slice.lower.as_deref(), + slice.upper.as_deref(), + slice.step.as_deref(), + ) { + (lower, upper, step @ Some(_)) => format!( + "slice({}, {}, {})", + stringify(lower), + stringify(upper), + stringify(step) + ), + (None, upper, None) => format!("slice({})", stringify(upper)), + (lower @ Some(_), upper, None) => { + format!("slice({}, {})", stringify(lower), stringify(upper)) + } + } +} + +/// Convert the given expression to a string representation, suitable to be a function argument. +fn subscript_slice_to_string<'a>(expr: &Expr, locator: &Locator<'a>) -> Cow<'a, str> { + if let Expr::Slice(expr_slice) = expr { + Cow::Owned(slice_expr_to_slice_call(expr_slice, locator)) + } else { + Cow::Borrowed(locator.slice(expr)) + } +} + +/// Return the `operator` implemented by given subscript expression. +fn itemgetter_op(expr: &ExprSubscript, params: &Parameters, locator: &Locator) -> Option { + let [arg] = params.args.as_slice() else { + return None; + }; + if !is_same_expression(arg, &expr.value) { + return None; + }; + Some(Operator { + name: "itemgetter", + args: vec![subscript_slice_to_string(expr.slice.as_ref(), locator).to_string()], + }) +} + +/// Return the `operator` implemented by given tuple expression. +fn itemgetter_op_tuple( + expr: &ExprTuple, + params: &Parameters, + locator: &Locator, +) -> Option { + let [arg] = params.args.as_slice() else { + return None; + }; + if expr.elts.is_empty() { + return None; + } + Some(Operator { + name: "itemgetter", + args: expr + .elts + .iter() + .map(|expr| { + expr.as_subscript_expr() + .filter(|expr| is_same_expression(arg, &expr.value)) + .map(|expr| expr.slice.as_ref()) + .map(|slice| subscript_slice_to_string(slice, locator).to_string()) + }) + .collect::>>()?, + }) +} + +#[derive(Eq, PartialEq, Debug)] +struct Operator { + name: &'static str, + args: Vec, +} + +impl From<&'static str> for Operator { + fn from(value: &'static str) -> Self { + Self { + name: value, + args: vec![], + } + } +} + +impl Display for Operator { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.name)?; + if self.args.is_empty() { + Ok(()) + } else { + write!(f, "({})", self.args.join(", ")) + } + } +} + +/// Return the `operator` implemented by the given expression. +fn get_operator(expr: &Expr, params: &Parameters, locator: &Locator) -> Option { match expr { - Expr::UnaryOp(expr) => unary_op(expr, params), - Expr::BinOp(expr) => bin_op(expr, params), - Expr::Compare(expr) => cmp_op(expr, params), + Expr::UnaryOp(expr) => unary_op(expr, params).map(Operator::from), + Expr::BinOp(expr) => bin_op(expr, params).map(Operator::from), + Expr::Compare(expr) => cmp_op(expr, params).map(Operator::from), + Expr::Subscript(expr) => itemgetter_op(expr, params, locator), + Expr::Tuple(expr) => itemgetter_op_tuple(expr, params, locator), _ => None, } } @@ -187,7 +296,7 @@ enum FunctionLikeKind { } /// Return the name of the `operator` implemented by the given unary expression. -fn unary_op(expr: &ast::ExprUnaryOp, params: &ast::Parameters) -> Option<&'static str> { +fn unary_op(expr: &ast::ExprUnaryOp, params: &Parameters) -> Option<&'static str> { let [arg] = params.args.as_slice() else { return None; }; @@ -203,7 +312,7 @@ fn unary_op(expr: &ast::ExprUnaryOp, params: &ast::Parameters) -> Option<&'stati } /// Return the name of the `operator` implemented by the given binary expression. -fn bin_op(expr: &ast::ExprBinOp, params: &ast::Parameters) -> Option<&'static str> { +fn bin_op(expr: &ast::ExprBinOp, params: &Parameters) -> Option<&'static str> { let [arg1, arg2] = params.args.as_slice() else { return None; }; @@ -228,7 +337,7 @@ fn bin_op(expr: &ast::ExprBinOp, params: &ast::Parameters) -> Option<&'static st } /// Return the name of the `operator` implemented by the given comparison expression. -fn cmp_op(expr: &ast::ExprCompare, params: &ast::Parameters) -> Option<&'static str> { +fn cmp_op(expr: &ast::ExprCompare, params: &Parameters) -> Option<&'static str> { let [arg1, arg2] = params.args.as_slice() else { return None; }; @@ -240,71 +349,19 @@ fn cmp_op(expr: &ast::ExprCompare, params: &ast::Parameters) -> Option<&'static }; match op { - ast::CmpOp::Eq => { - if match_arguments(arg1, arg2, &expr.left, right) { - Some("eq") - } else { - None - } - } - ast::CmpOp::NotEq => { - if match_arguments(arg1, arg2, &expr.left, right) { - Some("ne") - } else { - None - } - } - ast::CmpOp::Lt => { - if match_arguments(arg1, arg2, &expr.left, right) { - Some("lt") - } else { - None - } - } - ast::CmpOp::LtE => { - if match_arguments(arg1, arg2, &expr.left, right) { - Some("le") - } else { - None - } - } - ast::CmpOp::Gt => { - if match_arguments(arg1, arg2, &expr.left, right) { - Some("gt") - } else { - None - } - } - ast::CmpOp::GtE => { - if match_arguments(arg1, arg2, &expr.left, right) { - Some("ge") - } else { - None - } - } - ast::CmpOp::Is => { - if match_arguments(arg1, arg2, &expr.left, right) { - Some("is_") - } else { - None - } - } - ast::CmpOp::IsNot => { - if match_arguments(arg1, arg2, &expr.left, right) { - Some("is_not") - } else { - None - } - } + ast::CmpOp::Eq => match_arguments(arg1, arg2, &expr.left, right).then_some("eq"), + ast::CmpOp::NotEq => match_arguments(arg1, arg2, &expr.left, right).then_some("ne"), + ast::CmpOp::Lt => match_arguments(arg1, arg2, &expr.left, right).then_some("lt"), + ast::CmpOp::LtE => match_arguments(arg1, arg2, &expr.left, right).then_some("le"), + ast::CmpOp::Gt => match_arguments(arg1, arg2, &expr.left, right).then_some("gt"), + ast::CmpOp::GtE => match_arguments(arg1, arg2, &expr.left, right).then_some("ge"), + ast::CmpOp::Is => match_arguments(arg1, arg2, &expr.left, right).then_some("is_"), + ast::CmpOp::IsNot => match_arguments(arg1, arg2, &expr.left, right).then_some("is_not"), ast::CmpOp::In => { // Note: `operator.contains` reverses the order of arguments. That is: // `operator.contains` is equivalent to `lambda x, y: y in x`, rather than // `lambda x, y: x in y`. - if match_arguments(arg1, arg2, right, &expr.left) { - Some("contains") - } else { - None - } + match_arguments(arg1, arg2, right, &expr.left).then_some("contains") } ast::CmpOp::NotIn => None, } diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB101_FURB101.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB101_FURB101.py.snap index 6b3595549841c..b3db05568d87f 100644 --- a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB101_FURB101.py.snap +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB101_FURB101.py.snap @@ -41,56 +41,46 @@ FURB101.py:28:6: FURB101 `open` and `read` should be replaced by `Path("file.txt 29 | x = f.read() | -FURB101.py:32:6: FURB101 `open` and `read` should be replaced by `Path("file.txt").read_bytes(errors="ignore")` +FURB101.py:32:6: FURB101 `open` and `read` should be replaced by `Path("file.txt").read_text()` | 31 | # FURB101 -32 | with open("file.txt", errors="ignore", mode="rb") as f: - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB101 +32 | with open("file.txt", mode="r") as f: # noqa: FURB120 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB101 33 | x = f.read() | -FURB101.py:36:6: FURB101 `open` and `read` should be replaced by `Path("file.txt").read_text()` +FURB101.py:36:6: FURB101 `open` and `read` should be replaced by `Path(foo()).read_bytes()` | 35 | # FURB101 -36 | with open("file.txt", mode="r") as f: # noqa: FURB120 - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB101 -37 | x = f.read() - | - -FURB101.py:40:6: FURB101 `open` and `read` should be replaced by `Path(foo()).read_bytes()` - | -39 | # FURB101 -40 | with open(foo(), "rb") as f: +36 | with open(foo(), "rb") as f: | ^^^^^^^^^^^^^^^^^^^^^^ FURB101 -41 | # The body of `with` is non-trivial, but the recommendation holds. -42 | bar("pre") +37 | # The body of `with` is non-trivial, but the recommendation holds. +38 | bar("pre") | -FURB101.py:48:6: FURB101 `open` and `read` should be replaced by `Path("a.txt").read_text()` +FURB101.py:44:6: FURB101 `open` and `read` should be replaced by `Path("a.txt").read_text()` | -47 | # FURB101 -48 | with open("a.txt") as a, open("b.txt", "rb") as b: +43 | # FURB101 +44 | with open("a.txt") as a, open("b.txt", "rb") as b: | ^^^^^^^^^^^^^^^^^^ FURB101 -49 | x = a.read() -50 | y = b.read() +45 | x = a.read() +46 | y = b.read() | -FURB101.py:48:26: FURB101 `open` and `read` should be replaced by `Path("b.txt").read_bytes()` +FURB101.py:44:26: FURB101 `open` and `read` should be replaced by `Path("b.txt").read_bytes()` | -47 | # FURB101 -48 | with open("a.txt") as a, open("b.txt", "rb") as b: +43 | # FURB101 +44 | with open("a.txt") as a, open("b.txt", "rb") as b: | ^^^^^^^^^^^^^^^^^^^^^^^^ FURB101 -49 | x = a.read() -50 | y = b.read() +45 | x = a.read() +46 | y = b.read() | -FURB101.py:53:18: FURB101 `open` and `read` should be replaced by `Path("file.txt").read_text()` +FURB101.py:49:18: FURB101 `open` and `read` should be replaced by `Path("file.txt").read_text()` | -52 | # FURB101 -53 | with foo() as a, open("file.txt") as b, foo() as c: +48 | # FURB101 +49 | with foo() as a, open("file.txt") as b, foo() as c: | ^^^^^^^^^^^^^^^^^^^^^ FURB101 -54 | # We have other things in here, multiple with items, but -55 | # the user reads the whole file and that bit they can replace. +50 | # We have other things in here, multiple with items, but +51 | # the user reads the whole file and that bit they can replace. | - - diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB110_FURB110.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB110_FURB110.py.snap new file mode 100644 index 0000000000000..69ae14f9401c5 --- /dev/null +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB110_FURB110.py.snap @@ -0,0 +1,179 @@ +--- +source: crates/ruff_linter/src/rules/refurb/mod.rs +--- +FURB110.py:1:5: FURB110 [*] Replace ternary `if` expression with `or` operator + | +1 | z = x if x else y # FURB110 + | ^^^^^^^^^^^^^ FURB110 +2 | +3 | z = x \ + | + = help: Replace with `or` operator + +ℹ Safe fix +1 |-z = x if x else y # FURB110 + 1 |+z = x or y # FURB110 +2 2 | +3 3 | z = x \ +4 4 | if x else y # FURB110 + +FURB110.py:3:5: FURB110 [*] Replace ternary `if` expression with `or` operator + | +1 | z = x if x else y # FURB110 +2 | +3 | z = x \ + | _____^ +4 | | if x else y # FURB110 + | |_______________^ FURB110 +5 | +6 | z = x if x \ + | + = help: Replace with `or` operator + +ℹ Safe fix +1 1 | z = x if x else y # FURB110 +2 2 | +3 |-z = x \ +4 |- if x else y # FURB110 + 3 |+z = x or y # FURB110 +5 4 | +6 5 | z = x if x \ +7 6 | else \ + +FURB110.py:6:5: FURB110 [*] Replace ternary `if` expression with `or` operator + | + 4 | if x else y # FURB110 + 5 | + 6 | z = x if x \ + | _____^ + 7 | | else \ + 8 | | y # FURB110 + | |_________^ FURB110 + 9 | +10 | z = x() if x() else y() # FURB110 + | + = help: Replace with `or` operator + +ℹ Safe fix +3 3 | z = x \ +4 4 | if x else y # FURB110 +5 5 | +6 |-z = x if x \ +7 |- else \ +8 |- y # FURB110 + 6 |+z = x or y # FURB110 +9 7 | +10 8 | z = x() if x() else y() # FURB110 +11 9 | + +FURB110.py:10:5: FURB110 [*] Replace ternary `if` expression with `or` operator + | + 8 | y # FURB110 + 9 | +10 | z = x() if x() else y() # FURB110 + | ^^^^^^^^^^^^^^^^^^^ FURB110 +11 | +12 | # FURB110 + | + = help: Replace with `or` operator + +ℹ Unsafe fix +7 7 | else \ +8 8 | y # FURB110 +9 9 | +10 |-z = x() if x() else y() # FURB110 + 10 |+z = x() or y() # FURB110 +11 11 | +12 12 | # FURB110 +13 13 | z = x if ( + +FURB110.py:13:5: FURB110 [*] Replace ternary `if` expression with `or` operator + | +12 | # FURB110 +13 | z = x if ( + | _____^ +14 | | # Test for x. +15 | | x +16 | | ) else ( +17 | | # Test for y. +18 | | y +19 | | ) + | |_^ FURB110 +20 | +21 | # FURB110 + | + = help: Replace with `or` operator + +ℹ Safe fix +10 10 | z = x() if x() else y() # FURB110 +11 11 | +12 12 | # FURB110 +13 |-z = x if ( + 13 |+z = ( +14 14 | # Test for x. +15 15 | x +16 |-) else ( + 16 |+) or ( +17 17 | # Test for y. +18 18 | y +19 19 | ) + +FURB110.py:23:5: FURB110 [*] Replace ternary `if` expression with `or` operator + | +21 | # FURB110 +22 | z = ( +23 | x if ( + | _____^ +24 | | # Test for x. +25 | | x +26 | | ) else ( +27 | | # Test for y. +28 | | y +29 | | ) + | |_____^ FURB110 +30 | ) + | + = help: Replace with `or` operator + +ℹ Safe fix +20 20 | +21 21 | # FURB110 +22 22 | z = ( +23 |- x if ( + 23 |+ ( +24 24 | # Test for x. +25 25 | x +26 |- ) else ( + 26 |+ ) or ( +27 27 | # Test for y. +28 28 | y +29 29 | ) + +FURB110.py:34:5: FURB110 [*] Replace ternary `if` expression with `or` operator + | +32 | # FURB110 +33 | z = ( +34 | x if + | _____^ +35 | | # If true, use x. +36 | | x +37 | | # Otherwise, use y. +38 | | else +39 | | y + | |_____^ FURB110 +40 | ) + | + = help: Replace with `or` operator + +ℹ Safe fix +31 31 | +32 32 | # FURB110 +33 33 | z = ( +34 |- x if +35 |- # If true, use x. +36 |- x +37 |- # Otherwise, use y. +38 |- else +39 |- y + 34 |+ x or y +40 35 | ) diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB118_FURB118.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB118_FURB118.py.snap index 828a95ee3b5ea..6c208bcd9530d 100644 --- a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB118_FURB118.py.snap +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB118_FURB118.py.snap @@ -187,7 +187,7 @@ FURB118.py:10:14: FURB118 [*] Use `operator.matmul` instead of defining a lambda 11 |+op_matmutl = operator.matmul 11 12 | op_truediv = lambda x, y: x / y 12 13 | op_mod = lambda x, y: x % y -13 14 | op_pow = lambda x, y: x ** y +13 14 | op_pow = lambda x, y: x**y FURB118.py:11:14: FURB118 [*] Use `operator.truediv` instead of defining a lambda | @@ -196,7 +196,7 @@ FURB118.py:11:14: FURB118 [*] Use `operator.truediv` instead of defining a lambd 11 | op_truediv = lambda x, y: x / y | ^^^^^^^^^^^^^^^^^^ FURB118 12 | op_mod = lambda x, y: x % y -13 | op_pow = lambda x, y: x ** y +13 | op_pow = lambda x, y: x**y | = help: Replace with `operator.truediv` @@ -213,7 +213,7 @@ FURB118.py:11:14: FURB118 [*] Use `operator.truediv` instead of defining a lambd 11 |-op_truediv = lambda x, y: x / y 12 |+op_truediv = operator.truediv 12 13 | op_mod = lambda x, y: x % y -13 14 | op_pow = lambda x, y: x ** y +13 14 | op_pow = lambda x, y: x**y 14 15 | op_lshift = lambda x, y: x << y FURB118.py:12:10: FURB118 [*] Use `operator.mod` instead of defining a lambda @@ -222,7 +222,7 @@ FURB118.py:12:10: FURB118 [*] Use `operator.mod` instead of defining a lambda 11 | op_truediv = lambda x, y: x / y 12 | op_mod = lambda x, y: x % y | ^^^^^^^^^^^^^^^^^^ FURB118 -13 | op_pow = lambda x, y: x ** y +13 | op_pow = lambda x, y: x**y 14 | op_lshift = lambda x, y: x << y | = help: Replace with `operator.mod` @@ -239,7 +239,7 @@ FURB118.py:12:10: FURB118 [*] Use `operator.mod` instead of defining a lambda 11 12 | op_truediv = lambda x, y: x / y 12 |-op_mod = lambda x, y: x % y 13 |+op_mod = operator.mod -13 14 | op_pow = lambda x, y: x ** y +13 14 | op_pow = lambda x, y: x**y 14 15 | op_lshift = lambda x, y: x << y 15 16 | op_rshift = lambda x, y: x >> y @@ -247,8 +247,8 @@ FURB118.py:13:10: FURB118 [*] Use `operator.pow` instead of defining a lambda | 11 | op_truediv = lambda x, y: x / y 12 | op_mod = lambda x, y: x % y -13 | op_pow = lambda x, y: x ** y - | ^^^^^^^^^^^^^^^^^^^ FURB118 +13 | op_pow = lambda x, y: x**y + | ^^^^^^^^^^^^^^^^^ FURB118 14 | op_lshift = lambda x, y: x << y 15 | op_rshift = lambda x, y: x >> y | @@ -264,7 +264,7 @@ FURB118.py:13:10: FURB118 [*] Use `operator.pow` instead of defining a lambda 10 11 | op_matmutl = lambda x, y: x @ y 11 12 | op_truediv = lambda x, y: x / y 12 13 | op_mod = lambda x, y: x % y -13 |-op_pow = lambda x, y: x ** y +13 |-op_pow = lambda x, y: x**y 14 |+op_pow = operator.pow 14 15 | op_lshift = lambda x, y: x << y 15 16 | op_rshift = lambda x, y: x >> y @@ -273,7 +273,7 @@ FURB118.py:13:10: FURB118 [*] Use `operator.pow` instead of defining a lambda FURB118.py:14:13: FURB118 [*] Use `operator.lshift` instead of defining a lambda | 12 | op_mod = lambda x, y: x % y -13 | op_pow = lambda x, y: x ** y +13 | op_pow = lambda x, y: x**y 14 | op_lshift = lambda x, y: x << y | ^^^^^^^^^^^^^^^^^^^ FURB118 15 | op_rshift = lambda x, y: x >> y @@ -290,7 +290,7 @@ FURB118.py:14:13: FURB118 [*] Use `operator.lshift` instead of defining a lambda -------------------------------------------------------------------------------- 11 12 | op_truediv = lambda x, y: x / y 12 13 | op_mod = lambda x, y: x % y -13 14 | op_pow = lambda x, y: x ** y +13 14 | op_pow = lambda x, y: x**y 14 |-op_lshift = lambda x, y: x << y 15 |+op_lshift = operator.lshift 15 16 | op_rshift = lambda x, y: x >> y @@ -299,7 +299,7 @@ FURB118.py:14:13: FURB118 [*] Use `operator.lshift` instead of defining a lambda FURB118.py:15:13: FURB118 [*] Use `operator.rshift` instead of defining a lambda | -13 | op_pow = lambda x, y: x ** y +13 | op_pow = lambda x, y: x**y 14 | op_lshift = lambda x, y: x << y 15 | op_rshift = lambda x, y: x >> y | ^^^^^^^^^^^^^^^^^^^ FURB118 @@ -316,7 +316,7 @@ FURB118.py:15:13: FURB118 [*] Use `operator.rshift` instead of defining a lambda 4 5 | op_pos = lambda x: +x -------------------------------------------------------------------------------- 12 13 | op_mod = lambda x, y: x % y -13 14 | op_pow = lambda x, y: x ** y +13 14 | op_pow = lambda x, y: x**y 14 15 | op_lshift = lambda x, y: x << y 15 |-op_rshift = lambda x, y: x >> y 16 |+op_rshift = operator.rshift @@ -342,7 +342,7 @@ FURB118.py:16:12: FURB118 [*] Use `operator.or_` instead of defining a lambda 3 4 | op_not = lambda x: not x 4 5 | op_pos = lambda x: +x -------------------------------------------------------------------------------- -13 14 | op_pow = lambda x, y: x ** y +13 14 | op_pow = lambda x, y: x**y 14 15 | op_lshift = lambda x, y: x << y 15 16 | op_rshift = lambda x, y: x >> y 16 |-op_bitor = lambda x, y: x | y @@ -617,7 +617,7 @@ FURB118.py:27:9: FURB118 [*] Use `operator.is_` instead of defining a lambda 28 |+op_is = operator.is_ 28 29 | op_isnot = lambda x, y: x is not y 29 30 | op_in = lambda x, y: y in x -30 31 | +30 31 | op_itemgetter = lambda x: x[0] FURB118.py:28:12: FURB118 [*] Use `operator.is_not` instead of defining a lambda | @@ -626,6 +626,7 @@ FURB118.py:28:12: FURB118 [*] Use `operator.is_not` instead of defining a lambda 28 | op_isnot = lambda x, y: x is not y | ^^^^^^^^^^^^^^^^^^^^^^^ FURB118 29 | op_in = lambda x, y: y in x +30 | op_itemgetter = lambda x: x[0] | = help: Replace with `operator.is_not` @@ -642,8 +643,8 @@ FURB118.py:28:12: FURB118 [*] Use `operator.is_not` instead of defining a lambda 28 |-op_isnot = lambda x, y: x is not y 29 |+op_isnot = operator.is_not 29 30 | op_in = lambda x, y: y in x -30 31 | -31 32 | +30 31 | op_itemgetter = lambda x: x[0] +31 32 | op_itemgetter = lambda x: (x[0], x[1], x[2]) FURB118.py:29:9: FURB118 [*] Use `operator.contains` instead of defining a lambda | @@ -651,6 +652,8 @@ FURB118.py:29:9: FURB118 [*] Use `operator.contains` instead of defining a lambd 28 | op_isnot = lambda x, y: x is not y 29 | op_in = lambda x, y: y in x | ^^^^^^^^^^^^^^^^^^^ FURB118 +30 | op_itemgetter = lambda x: x[0] +31 | op_itemgetter = lambda x: (x[0], x[1], x[2]) | = help: Replace with `operator.contains` @@ -666,36 +669,137 @@ FURB118.py:29:9: FURB118 [*] Use `operator.contains` instead of defining a lambd 28 29 | op_isnot = lambda x, y: x is not y 29 |-op_in = lambda x, y: y in x 30 |+op_in = operator.contains -30 31 | -31 32 | -32 33 | def op_not2(x): +30 31 | op_itemgetter = lambda x: x[0] +31 32 | op_itemgetter = lambda x: (x[0], x[1], x[2]) +32 33 | op_itemgetter = lambda x: (x[1:], x[2]) + +FURB118.py:30:17: FURB118 [*] Use `operator.itemgetter(0)` instead of defining a lambda + | +28 | op_isnot = lambda x, y: x is not y +29 | op_in = lambda x, y: y in x +30 | op_itemgetter = lambda x: x[0] + | ^^^^^^^^^^^^^^ FURB118 +31 | op_itemgetter = lambda x: (x[0], x[1], x[2]) +32 | op_itemgetter = lambda x: (x[1:], x[2]) + | + = help: Replace with `operator.itemgetter(0)` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +27 28 | op_is = lambda x, y: x is y +28 29 | op_isnot = lambda x, y: x is not y +29 30 | op_in = lambda x, y: y in x +30 |-op_itemgetter = lambda x: x[0] + 31 |+op_itemgetter = operator.itemgetter(0) +31 32 | op_itemgetter = lambda x: (x[0], x[1], x[2]) +32 33 | op_itemgetter = lambda x: (x[1:], x[2]) +33 34 | op_itemgetter = lambda x: x[:] + +FURB118.py:31:17: FURB118 [*] Use `operator.itemgetter(0, 1, 2)` instead of defining a lambda + | +29 | op_in = lambda x, y: y in x +30 | op_itemgetter = lambda x: x[0] +31 | op_itemgetter = lambda x: (x[0], x[1], x[2]) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB118 +32 | op_itemgetter = lambda x: (x[1:], x[2]) +33 | op_itemgetter = lambda x: x[:] + | + = help: Replace with `operator.itemgetter(0, 1, 2)` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +28 29 | op_isnot = lambda x, y: x is not y +29 30 | op_in = lambda x, y: y in x +30 31 | op_itemgetter = lambda x: x[0] +31 |-op_itemgetter = lambda x: (x[0], x[1], x[2]) + 32 |+op_itemgetter = operator.itemgetter(0, 1, 2) +32 33 | op_itemgetter = lambda x: (x[1:], x[2]) +33 34 | op_itemgetter = lambda x: x[:] +34 35 | -FURB118.py:32:1: FURB118 Use `operator.not_` instead of defining a function +FURB118.py:32:17: FURB118 [*] Use `operator.itemgetter(slice(1, None), 2)` instead of defining a lambda | -32 | / def op_not2(x): -33 | | return not x +30 | op_itemgetter = lambda x: x[0] +31 | op_itemgetter = lambda x: (x[0], x[1], x[2]) +32 | op_itemgetter = lambda x: (x[1:], x[2]) + | ^^^^^^^^^^^^^^^^^^^^^^^ FURB118 +33 | op_itemgetter = lambda x: x[:] + | + = help: Replace with `operator.itemgetter(slice(1, None), 2)` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +29 30 | op_in = lambda x, y: y in x +30 31 | op_itemgetter = lambda x: x[0] +31 32 | op_itemgetter = lambda x: (x[0], x[1], x[2]) +32 |-op_itemgetter = lambda x: (x[1:], x[2]) + 33 |+op_itemgetter = operator.itemgetter(slice(1, None), 2) +33 34 | op_itemgetter = lambda x: x[:] +34 35 | +35 36 | + +FURB118.py:33:17: FURB118 [*] Use `operator.itemgetter(slice(None))` instead of defining a lambda + | +31 | op_itemgetter = lambda x: (x[0], x[1], x[2]) +32 | op_itemgetter = lambda x: (x[1:], x[2]) +33 | op_itemgetter = lambda x: x[:] + | ^^^^^^^^^^^^^^ FURB118 + | + = help: Replace with `operator.itemgetter(slice(None))` + +ℹ Safe fix +1 1 | # Errors. + 2 |+import operator +2 3 | op_bitnot = lambda x: ~x +3 4 | op_not = lambda x: not x +4 5 | op_pos = lambda x: +x +-------------------------------------------------------------------------------- +30 31 | op_itemgetter = lambda x: x[0] +31 32 | op_itemgetter = lambda x: (x[0], x[1], x[2]) +32 33 | op_itemgetter = lambda x: (x[1:], x[2]) +33 |-op_itemgetter = lambda x: x[:] + 34 |+op_itemgetter = operator.itemgetter(slice(None)) +34 35 | +35 36 | +36 37 | def op_not2(x): + +FURB118.py:36:1: FURB118 Use `operator.not_` instead of defining a function + | +36 | / def op_not2(x): +37 | | return not x | |________________^ FURB118 | = help: Replace with `operator.not_` -FURB118.py:36:1: FURB118 Use `operator.add` instead of defining a function +FURB118.py:40:1: FURB118 Use `operator.add` instead of defining a function | -36 | / def op_add2(x, y): -37 | | return x + y +40 | / def op_add2(x, y): +41 | | return x + y | |________________^ FURB118 | = help: Replace with `operator.add` -FURB118.py:41:5: FURB118 Use `operator.add` instead of defining a function +FURB118.py:45:5: FURB118 Use `operator.add` instead of defining a function | -40 | class Adder: -41 | def add(x, y): +44 | class Adder: +45 | def add(x, y): | _____^ -42 | | return x + y +46 | | return x + y | |____________________^ FURB118 -43 | -44 | # OK. | = help: Replace with `operator.add` - - diff --git a/crates/ruff_linter/src/rules/ruff/rules/asyncio_dangling_task.rs b/crates/ruff_linter/src/rules/ruff/rules/asyncio_dangling_task.rs index 4c04fc0d43af1..8613f44e250bc 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/asyncio_dangling_task.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/asyncio_dangling_task.rs @@ -139,8 +139,7 @@ pub(crate) fn asyncio_dangling_binding( // else: // task = asyncio.create_task(make_request()) // ``` - for binding_id in - std::iter::successors(Some(binding_id), |id| semantic.shadowed_binding(*id)) + for binding_id in std::iter::successors(Some(binding_id), |id| scope.shadowed_binding(*id)) { let binding = semantic.binding(binding_id); if binding.is_used() diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF006_RUF006.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF006_RUF006.py.snap index 46fa193ae9e43..884e920dcb16c 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF006_RUF006.py.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF006_RUF006.py.snap @@ -41,6 +41,16 @@ RUF006.py:97:5: RUF006 Store a reference to the return value of `loop.create_tas | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF006 | +RUF006.py:152:13: RUF006 Store a reference to the return value of `asyncio.create_task` + | +150 | async def f(x: bool): +151 | if x: +152 | t = asyncio.create_task(asyncio.sleep(1)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF006 +153 | else: +154 | t = None + | + RUF006.py:170:5: RUF006 Store a reference to the return value of `loop.create_task` | 168 | def f(): @@ -60,5 +70,3 @@ RUF006.py:175:5: RUF006 Store a reference to the return value of `loop.create_ta 176 | 177 | # OK | - - diff --git a/crates/ruff_linter/src/settings/types.rs b/crates/ruff_linter/src/settings/types.rs index 99aec6740cab2..a06bb2ecaeb54 100644 --- a/crates/ruff_linter/src/settings/types.rs +++ b/crates/ruff_linter/src/settings/types.rs @@ -296,13 +296,22 @@ impl CacheKey for FilePatternSet { pub struct PerFileIgnore { pub(crate) basename: String, pub(crate) absolute: PathBuf, + pub(crate) negated: bool, pub(crate) rules: RuleSet, } impl PerFileIgnore { - pub fn new(pattern: String, prefixes: &[RuleSelector], project_root: Option<&Path>) -> Self { + pub fn new( + mut pattern: String, + prefixes: &[RuleSelector], + project_root: Option<&Path>, + ) -> Self { // Rules in preview are included here even if preview mode is disabled; it's safe to ignore disabled rules let rules: RuleSet = prefixes.iter().flat_map(RuleSelector::all_rules).collect(); + let negated = pattern.starts_with('!'); + if negated { + pattern.drain(..1); + } let path = Path::new(&pattern); let absolute = match project_root { Some(project_root) => fs::normalize_path_to(path, project_root), @@ -312,6 +321,7 @@ impl PerFileIgnore { Self { basename: pattern, absolute, + negated, rules, } } @@ -593,7 +603,7 @@ pub type IdentifierPattern = glob::Pattern; #[derive(Debug, Clone, CacheKey, Default)] pub struct PerFileIgnores { // Ordered as (absolute path matcher, basename matcher, rules) - ignores: Vec<(GlobMatcher, GlobMatcher, RuleSet)>, + ignores: Vec<(GlobMatcher, GlobMatcher, bool, RuleSet)>, } impl PerFileIgnores { @@ -609,7 +619,12 @@ impl PerFileIgnores { // Construct basename matcher. let basename = Glob::new(&per_file_ignore.basename)?.compile_matcher(); - Ok((absolute, basename, per_file_ignore.rules)) + Ok(( + absolute, + basename, + per_file_ignore.negated, + per_file_ignore.rules, + )) }) .collect(); Ok(Self { ignores: ignores? }) @@ -622,10 +637,10 @@ impl Display for PerFileIgnores { write!(f, "{{}}")?; } else { writeln!(f, "{{")?; - for (absolute, basename, rules) in &self.ignores { + for (absolute, basename, negated, rules) in &self.ignores { writeln!( f, - "\t{{ absolute = {absolute:#?}, basename = {basename:#?}, rules = {rules} }}," + "\t{{ absolute = {absolute:#?}, basename = {basename:#?}, negated = {negated:#?}, rules = {rules} }}," )?; } write!(f, "}}")?; @@ -635,7 +650,7 @@ impl Display for PerFileIgnores { } impl Deref for PerFileIgnores { - type Target = Vec<(GlobMatcher, GlobMatcher, RuleSet)>; + type Target = Vec<(GlobMatcher, GlobMatcher, bool, RuleSet)>; fn deref(&self) -> &Self::Target { &self.ignores diff --git a/crates/ruff_python_ast/src/all.rs b/crates/ruff_python_ast/src/all.rs index 71348cdc105eb..4a5899ce172e6 100644 --- a/crates/ruff_python_ast/src/all.rs +++ b/crates/ruff_python_ast/src/all.rs @@ -39,6 +39,34 @@ impl Ranged for DunderAllName<'_> { } } +/// Abstraction for a collection of names inside an `__all__` definition, +/// e.g. `["foo", "bar"]` in `__all__ = ["foo", "bar"]` +#[derive(Debug, Clone)] +pub struct DunderAllDefinition<'a> { + /// The range of the `__all__` identifier. + range: TextRange, + /// The names inside the `__all__` definition. + names: Vec>, +} + +impl<'a> DunderAllDefinition<'a> { + /// Initialize a new [`DunderAllDefinition`] instance. + pub fn new(range: TextRange, names: Vec>) -> Self { + Self { range, names } + } + + /// The names inside the `__all__` definition. + pub fn names(&self) -> &[DunderAllName<'a>] { + &self.names + } +} + +impl Ranged for DunderAllDefinition<'_> { + fn range(&self) -> TextRange { + self.range + } +} + /// Extract the names bound to a given __all__ assignment. /// /// Accepts a closure that determines whether a given name (e.g., `"list"`) is a Python builtin. diff --git a/crates/ruff_python_ast/tests/preorder.rs b/crates/ruff_python_ast/tests/preorder.rs index b3b1f457f8207..21a159b424a3a 100644 --- a/crates/ruff_python_ast/tests/preorder.rs +++ b/crates/ruff_python_ast/tests/preorder.rs @@ -158,7 +158,7 @@ fn trace_preorder_visitation(source: &str) -> String { } /// Emits a `tree` with a node for every visited AST node (labelled by the AST node's kind) -/// and leafs for attributes. +/// and leaves for attributes. #[derive(Default)] struct RecordVisitor { depth: usize, diff --git a/crates/ruff_python_ast/tests/visitor.rs b/crates/ruff_python_ast/tests/visitor.rs index 1097d699a993f..e039e78d96ad6 100644 --- a/crates/ruff_python_ast/tests/visitor.rs +++ b/crates/ruff_python_ast/tests/visitor.rs @@ -181,7 +181,7 @@ where } /// Emits a `tree` with a node for every visited AST node (labelled by the AST node's kind) -/// and leafs for attributes. +/// and leaves for attributes. #[derive(Default)] struct RecordVisitor { depth: usize, diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index ac735d97acd29..ffd407ad34996 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -1465,31 +1465,36 @@ impl<'a> SemanticModel<'a> { self.flags.intersects(SemanticModelFlags::TYPE_DEFINITION) } - /// Return `true` if the model is in a string type definition. + /// Return `true` if the model is visiting a "string type definition" + /// that was previously deferred when initially traversing the AST pub const fn in_string_type_definition(&self) -> bool { self.flags .intersects(SemanticModelFlags::STRING_TYPE_DEFINITION) } - /// Return `true` if the model is in a "simple" string type definition. + /// Return `true` if the model is visiting a "simple string type definition" + /// that was previously deferred when initially traversing the AST pub const fn in_simple_string_type_definition(&self) -> bool { self.flags .intersects(SemanticModelFlags::SIMPLE_STRING_TYPE_DEFINITION) } - /// Return `true` if the model is in a "complex" string type definition. + /// Return `true` if the model is visiting a "complex string type definition" + /// that was previously deferred when initially traversing the AST pub const fn in_complex_string_type_definition(&self) -> bool { self.flags .intersects(SemanticModelFlags::COMPLEX_STRING_TYPE_DEFINITION) } - /// Return `true` if the model is in a `__future__` type definition. + /// Return `true` if the model is visiting a "`__future__` type definition" + /// that was previously deferred when initially traversing the AST pub const fn in_future_type_definition(&self) -> bool { self.flags .intersects(SemanticModelFlags::FUTURE_TYPE_DEFINITION) } - /// Return `true` if the model is in any kind of deferred type definition. + /// Return `true` if the model is visiting any kind of type definition + /// that was previously deferred when initially traversing the AST pub const fn in_deferred_type_definition(&self) -> bool { self.flags .intersects(SemanticModelFlags::DEFERRED_TYPE_DEFINITION) @@ -1574,9 +1579,9 @@ impl<'a> SemanticModel<'a> { } /// Return `true` if `__future__`-style type annotations are enabled. - pub const fn future_annotations(&self) -> bool { + pub const fn future_annotations_or_stub(&self) -> bool { self.flags - .intersects(SemanticModelFlags::FUTURE_ANNOTATIONS) + .intersects(SemanticModelFlags::FUTURE_ANNOTATIONS_OR_STUB) } /// Return `true` if the model is in a stub file (i.e., a file with a `.pyi` extension). @@ -1603,6 +1608,20 @@ impl<'a> SemanticModel<'a> { .intersects(SemanticModelFlags::DUNDER_ALL_DEFINITION) } + /// Return `true` if the model is visiting an item in a class's bases tuple + /// (e.g. `Foo` in `class Bar(Foo): ...`) + pub const fn in_class_base(&self) -> bool { + self.flags.intersects(SemanticModelFlags::CLASS_BASE) + } + + /// Return `true` if the model is visiting an item in a class's bases tuple + /// that was initially deferred while traversing the AST. + /// (This only happens in stub files.) + pub const fn in_deferred_class_base(&self) -> bool { + self.flags + .intersects(SemanticModelFlags::DEFERRED_CLASS_BASE) + } + /// Return an iterator over all bindings shadowed by the given [`BindingId`], within the /// containing scope, and across scopes. pub fn shadowed_bindings( @@ -1770,6 +1789,9 @@ bitflags! { /// /// "Simple" string type definitions are those that consist of a single string literal, /// as opposed to an implicitly concatenated string literal. + /// + /// Note that this flag is only set when we are actually *visiting* the deferred definition, + /// not when we "pass by" it when initially traversing the source tree. const SIMPLE_STRING_TYPE_DEFINITION = 1 << 4; /// The model is in a (deferred) "complex" string type definition. @@ -1781,6 +1803,9 @@ bitflags! { /// /// "Complex" string type definitions are those that consist of a implicitly concatenated /// string literals. These are uncommon but valid. + /// + /// Note that this flag is only set when we are actually *visiting* the deferred definition, + /// not when we "pass by" it when initially traversing the source tree. const COMPLEX_STRING_TYPE_DEFINITION = 1 << 5; /// The model is in a (deferred) `__future__` type definition. @@ -1794,6 +1819,20 @@ bitflags! { /// /// `__future__`-style type annotations are only enabled if the `annotations` feature /// is enabled via `from __future__ import annotations`. + /// + /// This flag should only be set in contexts where PEP-563 semantics are relevant to + /// resolution of the type definition. For example, the flag should not be set + /// in the following context, because the type definition is not inside a type annotation, + /// so whether or not `from __future__ import annotations` is active has no relevance: + /// ```python + /// from __future__ import annotations + /// from typing import TypeAlias + /// + /// X: TypeAlias = list[int] + /// ``` + /// + /// Note also that this flag is only set when we are actually *visiting* the deferred definition, + /// not when we "pass by" it when initially traversing the source tree. const FUTURE_TYPE_DEFINITION = 1 << 6; /// The model is in an exception handler. @@ -1884,7 +1923,8 @@ bitflags! { /// any other non-`__future__`-importing statements. const FUTURES_BOUNDARY = 1 << 14; - /// `__future__`-style type annotations are enabled in this model. + /// The model is in a file that has `from __future__ import annotations` + /// at the top of the module. /// /// For example, the model could be visiting `x` in: /// ```python @@ -1899,6 +1939,12 @@ bitflags! { /// The model is in a Python stub file (i.e., a `.pyi` file). const STUB_FILE = 1 << 16; + /// `__future__`-style type annotations are enabled in this model. + /// That could be because it's a stub file, + /// or it could be because it's a non-stub file that has `from __future__ import annotations` + /// a the top of the module. + const FUTURE_ANNOTATIONS_OR_STUB = Self::FUTURE_ANNOTATIONS.bits() | Self::STUB_FILE.bits(); + /// The model has traversed past the module docstring. /// /// For example, the model could be visiting `x` in: @@ -1909,14 +1955,28 @@ bitflags! { /// ``` const MODULE_DOCSTRING_BOUNDARY = 1 << 17; - /// The model is in a type parameter definition. + /// The model is in a (deferred) [type parameter definition]. + /// + /// For example, the model could be visiting `T`, `P` or `Ts` in: + /// ```python + /// class Foo[T, *Ts, **P]: pass + /// ``` + /// + /// Note that this flag is *not* set for "pre-PEP-695" TypeVars, ParamSpecs or TypeVarTuples. + /// None of the following would lead to the flag being set: /// - /// For example, the model could be visiting `Record` in: /// ```python - /// from typing import TypeVar + /// from typing import TypeVar, ParamSpec, TypeVarTuple + /// + /// T = TypeVar("T") + /// P = ParamSpec("P") + /// Ts = TypeVarTuple("Ts") + /// ``` /// - /// Record = TypeVar("Record") + /// Note also that this flag is only set when we are actually *visiting* the deferred definition, + /// not when we "pass by" it when initially traversing the source tree. /// + /// [type parameter definition]: https://docs.python.org/3/reference/executionmodel.html#annotation-scopes const TYPE_PARAM_DEFINITION = 1 << 18; /// The model is in a named expression assignment. @@ -1975,6 +2035,20 @@ bitflags! { /// ``` const F_STRING_REPLACEMENT_FIELD = 1 << 23; + /// The model is visiting the bases tuple of a class. + /// + /// For example, the model could be visiting `Foo` or `Bar` in: + /// + /// ```python + /// class Baz(Foo, Bar): + /// pass + /// ``` + const CLASS_BASE = 1 << 24; + + /// The model is visiting a class base that was initially deferred + /// while traversing the AST. (This only happens in stub files.) + const DEFERRED_CLASS_BASE = 1 << 25; + /// The context is in any type annotation. const ANNOTATION = Self::TYPING_ONLY_ANNOTATION.bits() | Self::RUNTIME_EVALUATED_ANNOTATION.bits() | Self::RUNTIME_REQUIRED_ANNOTATION.bits(); @@ -1996,12 +2070,11 @@ bitflags! { impl SemanticModelFlags { pub fn new(path: &Path) -> Self { - let mut flags = Self::default(); if is_python_stub_file(path) { - flags |= Self::STUB_FILE; - flags |= Self::FUTURE_ANNOTATIONS; + Self::STUB_FILE + } else { + Self::default() } - flags } } diff --git a/crates/ruff_python_semantic/src/scope.rs b/crates/ruff_python_semantic/src/scope.rs index 0b2637258f5e6..997a6a0252723 100644 --- a/crates/ruff_python_semantic/src/scope.rs +++ b/crates/ruff_python_semantic/src/scope.rs @@ -185,6 +185,7 @@ pub enum ScopeKind<'a> { Function(&'a ast::StmtFunctionDef), Generator, Module, + /// A Python 3.12+ ["annotation scope"](https://docs.python.org/3/reference/executionmodel.html#annotation-scopes) Type, Lambda(&'a ast::ExprLambda), } diff --git a/crates/ruff_server/src/edit.rs b/crates/ruff_server/src/edit.rs index 9af598eaebcac..ec41b22a94479 100644 --- a/crates/ruff_server/src/edit.rs +++ b/crates/ruff_server/src/edit.rs @@ -4,12 +4,16 @@ mod document; mod range; mod replacement; +use std::collections::HashMap; + pub use document::Document; pub(crate) use document::DocumentVersion; use lsp_types::PositionEncodingKind; pub(crate) use range::{RangeExt, ToRangeExt}; pub(crate) use replacement::Replacement; +use crate::session::ResolvedClientCapabilities; + /// A convenient enumeration for supported text encodings. Can be converted to [`lsp_types::PositionEncodingKind`]. // Please maintain the order from least to greatest priority for the derived `Ord` impl. #[derive(Default, Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] @@ -25,6 +29,14 @@ pub enum PositionEncoding { UTF8, } +/// Tracks multi-document edits to eventually merge into a `WorkspaceEdit`. +/// Compatible with clients that don't support `workspace.workspaceEdit.documentChanges`. +#[derive(Debug)] +pub(crate) enum WorkspaceEditTracker { + DocumentChanges(Vec), + Changes(HashMap>), +} + impl From for lsp_types::PositionEncodingKind { fn from(value: PositionEncoding) -> Self { match value { @@ -50,3 +62,70 @@ impl TryFrom<&lsp_types::PositionEncodingKind> for PositionEncoding { }) } } + +impl WorkspaceEditTracker { + pub(crate) fn new(client_capabilities: &ResolvedClientCapabilities) -> Self { + if client_capabilities.document_changes { + Self::DocumentChanges(Vec::default()) + } else { + Self::Changes(HashMap::default()) + } + } + + /// Sets the edits made to a specific document. This should only be called + /// once for each document `uri`, and will fail if this is called for the same `uri` + /// multiple times. + pub(crate) fn set_edits_for_document( + &mut self, + uri: lsp_types::Url, + version: DocumentVersion, + edits: Vec, + ) -> crate::Result<()> { + match self { + Self::DocumentChanges(document_edits) => { + if document_edits + .iter() + .any(|document| document.text_document.uri == uri) + { + return Err(anyhow::anyhow!( + "Attempted to add edits for a document that was already edited" + )); + } + document_edits.push(lsp_types::TextDocumentEdit { + text_document: lsp_types::OptionalVersionedTextDocumentIdentifier { + uri, + version: Some(version), + }, + edits: edits.into_iter().map(lsp_types::OneOf::Left).collect(), + }); + Ok(()) + } + Self::Changes(changes) => { + if changes.get(&uri).is_some() { + return Err(anyhow::anyhow!( + "Attempted to add edits for a document that was already edited" + )); + } + changes.insert(uri, edits); + Ok(()) + } + } + } + + pub(crate) fn is_empty(&self) -> bool { + match self { + Self::DocumentChanges(document_edits) => document_edits.is_empty(), + Self::Changes(changes) => changes.is_empty(), + } + } + + pub(crate) fn into_workspace_edit(self) -> lsp_types::WorkspaceEdit { + match self { + Self::DocumentChanges(document_edits) => lsp_types::WorkspaceEdit { + document_changes: Some(lsp_types::DocumentChanges::Edits(document_edits)), + ..Default::default() + }, + Self::Changes(changes) => lsp_types::WorkspaceEdit::new(changes), + } + } +} diff --git a/crates/ruff_server/src/lint.rs b/crates/ruff_server/src/lint.rs index ba8bef1e26825..355460913f1fb 100644 --- a/crates/ruff_server/src/lint.rs +++ b/crates/ruff_server/src/lint.rs @@ -35,7 +35,7 @@ pub(crate) struct DiagnosticFix { pub(crate) fixed_diagnostic: lsp_types::Diagnostic, pub(crate) title: String, pub(crate) code: String, - pub(crate) document_edits: Vec, + pub(crate) edits: Vec, } pub(crate) fn check( @@ -90,11 +90,9 @@ pub(crate) fn check( .collect() } -pub(crate) fn fixes_for_diagnostics<'d>( - document: &'d crate::edit::Document, - url: &'d lsp_types::Url, +pub(crate) fn fixes_for_diagnostics( + document: &crate::edit::Document, encoding: PositionEncoding, - version: crate::edit::DocumentVersion, diagnostics: Vec, ) -> crate::Result> { diagnostics @@ -118,14 +116,6 @@ pub(crate) fn fixes_for_diagnostics<'d>( .to_range(document.contents(), document.index(), encoding), new_text: edit.content().unwrap_or_default().to_string(), }); - - let document_edits = vec![lsp_types::TextDocumentEdit { - text_document: lsp_types::OptionalVersionedTextDocumentIdentifier::new( - url.clone(), - version, - ), - edits: edits.map(lsp_types::OneOf::Left).collect(), - }]; Ok(Some(DiagnosticFix { fixed_diagnostic, code: associated_data.code, @@ -133,7 +123,7 @@ pub(crate) fn fixes_for_diagnostics<'d>( .kind .suggestion .unwrap_or(associated_data.kind.name), - document_edits, + edits: edits.collect(), })) }) .filter_map(crate::Result::transpose) diff --git a/crates/ruff_server/src/server/api.rs b/crates/ruff_server/src/server/api.rs index 90cf543a29fac..d649379d48317 100644 --- a/crates/ruff_server/src/server/api.rs +++ b/crates/ruff_server/src/server/api.rs @@ -41,6 +41,7 @@ pub(super) fn request<'a>(req: server::Request) -> Task<'a> { BackgroundSchedule::LatencySensitive, ) } + request::ExecuteCommand::METHOD => local_request_task::(req), request::Format::METHOD => { background_request_task::(req, BackgroundSchedule::Fmt) } @@ -87,13 +88,12 @@ pub(super) fn notification<'a>(notif: server::Notification) -> Task<'a> { }) } -#[allow(dead_code)] fn local_request_task<'a, R: traits::SyncRequestHandler>( req: server::Request, ) -> super::Result> { let (id, params) = cast_request::(req)?; - Ok(Task::local(|session, notifier, responder| { - let result = R::run(session, notifier, params); + Ok(Task::local(|session, notifier, requester, responder| { + let result = R::run(session, notifier, requester, params); respond::(id, result, &responder); })) } @@ -119,7 +119,7 @@ fn local_notification_task<'a, N: traits::SyncNotificationHandler>( notif: server::Notification, ) -> super::Result> { let (id, params) = cast_notification::(notif)?; - Ok(Task::local(move |session, notifier, _| { + Ok(Task::local(move |session, notifier, _, _| { if let Err(err) = N::run(session, notifier, params) { tracing::error!("An error occurred while running {id}: {err}"); } diff --git a/crates/ruff_server/src/server/api/requests.rs b/crates/ruff_server/src/server/api/requests.rs index 3883c009bdec4..3713ef536f592 100644 --- a/crates/ruff_server/src/server/api/requests.rs +++ b/crates/ruff_server/src/server/api/requests.rs @@ -1,16 +1,18 @@ mod code_action; mod code_action_resolve; mod diagnostic; +mod execute_command; mod format; mod format_range; use super::{ define_document_url, - traits::{BackgroundDocumentRequestHandler, RequestHandler}, + traits::{BackgroundDocumentRequestHandler, RequestHandler, SyncRequestHandler}, }; pub(super) use code_action::CodeActions; pub(super) use code_action_resolve::CodeActionResolve; pub(super) use diagnostic::DocumentDiagnostic; +pub(super) use execute_command::ExecuteCommand; pub(super) use format::Format; pub(super) use format_range::FormatRange; diff --git a/crates/ruff_server/src/server/api/requests/code_action.rs b/crates/ruff_server/src/server/api/requests/code_action.rs index e9bc8d1842d85..0d697b0e59196 100644 --- a/crates/ruff_server/src/server/api/requests/code_action.rs +++ b/crates/ruff_server/src/server/api/requests/code_action.rs @@ -1,3 +1,4 @@ +use crate::edit::WorkspaceEditTracker; use crate::lint::fixes_for_diagnostics; use crate::server::api::LSPResult; use crate::server::SupportedCodeAction; @@ -28,18 +29,24 @@ impl super::BackgroundDocumentRequestHandler for CodeActions { let supported_code_actions = supported_code_actions(params.context.only.clone()); - if supported_code_actions.contains(&SupportedCodeAction::QuickFix) { + if snapshot.client_settings().fix_violation() + && supported_code_actions.contains(&SupportedCodeAction::QuickFix) + { response.extend( quick_fix(&snapshot, params.context.diagnostics.clone()) .with_failure_code(ErrorCode::InternalError)?, ); } - if supported_code_actions.contains(&SupportedCodeAction::SourceFixAll) { + if snapshot.client_settings().fix_all() + && supported_code_actions.contains(&SupportedCodeAction::SourceFixAll) + { response.push(fix_all(&snapshot).with_failure_code(ErrorCode::InternalError)?); } - if supported_code_actions.contains(&SupportedCodeAction::SourceOrganizeImports) { + if snapshot.client_settings().organize_imports() + && supported_code_actions.contains(&SupportedCodeAction::SourceOrganizeImports) + { response.push(organize_imports(&snapshot).with_failure_code(ErrorCode::InternalError)?); } @@ -50,30 +57,34 @@ impl super::BackgroundDocumentRequestHandler for CodeActions { fn quick_fix( snapshot: &DocumentSnapshot, diagnostics: Vec, -) -> crate::Result + '_> { +) -> crate::Result> { let document = snapshot.document(); - let fixes = fixes_for_diagnostics( - document, - snapshot.url(), - snapshot.encoding(), - document.version(), - diagnostics, - )?; - - Ok(fixes.into_iter().map(|fix| { - types::CodeActionOrCommand::CodeAction(types::CodeAction { - title: format!("{DIAGNOSTIC_NAME} ({}): {}", fix.code, fix.title), - kind: Some(types::CodeActionKind::QUICKFIX), - edit: Some(types::WorkspaceEdit { - document_changes: Some(types::DocumentChanges::Edits(fix.document_edits.clone())), + let fixes = fixes_for_diagnostics(document, snapshot.encoding(), diagnostics)?; + + fixes + .into_iter() + .map(|fix| { + let mut tracker = WorkspaceEditTracker::new(snapshot.resolved_client_capabilities()); + + tracker.set_edits_for_document( + snapshot.url().clone(), + document.version(), + fix.edits, + )?; + + Ok(types::CodeActionOrCommand::CodeAction(types::CodeAction { + title: format!("{DIAGNOSTIC_NAME} ({}): {}", fix.code, fix.title), + kind: Some(types::CodeActionKind::QUICKFIX), + edit: Some(tracker.into_workspace_edit()), + diagnostics: Some(vec![fix.fixed_diagnostic.clone()]), + data: Some( + serde_json::to_value(snapshot.url()).expect("document url to serialize"), + ), ..Default::default() - }), - diagnostics: Some(vec![fix.fixed_diagnostic.clone()]), - data: Some(serde_json::to_value(snapshot.url()).expect("document url to serialize")), - ..Default::default() + })) }) - })) + .collect() } fn fix_all(snapshot: &DocumentSnapshot) -> crate::Result { @@ -92,9 +103,11 @@ fn fix_all(snapshot: &DocumentSnapshot) -> crate::Result { ( Some(resolve_edit_for_fix_all( document, + snapshot.resolved_client_capabilities(), snapshot.url(), &snapshot.configuration().linter, snapshot.encoding(), + document.version(), )?), None, ) @@ -125,9 +138,11 @@ fn organize_imports(snapshot: &DocumentSnapshot) -> crate::Result Some( resolve_edit_for_fix_all( document, + snapshot.resolved_client_capabilities(), snapshot.url(), &snapshot.configuration().linter, snapshot.encoding(), + document.version(), ) .with_failure_code(ErrorCode::InternalError)?, ), SupportedCodeAction::SourceOrganizeImports => Some( resolve_edit_for_organize_imports( document, + snapshot.resolved_client_capabilities(), snapshot.url(), &snapshot.configuration().linter, snapshot.encoding(), + document.version(), ) .with_failure_code(ErrorCode::InternalError)?, ), @@ -71,29 +76,51 @@ impl super::BackgroundDocumentRequestHandler for CodeActionResolve { pub(super) fn resolve_edit_for_fix_all( document: &crate::edit::Document, + client_capabilities: &ResolvedClientCapabilities, url: &types::Url, linter_settings: &LinterSettings, encoding: PositionEncoding, + version: DocumentVersion, ) -> crate::Result { - Ok(types::WorkspaceEdit { - changes: Some( - [( - url.clone(), - crate::fix::fix_all(document, linter_settings, encoding)?, - )] - .into_iter() - .collect(), - ), - ..Default::default() - }) + let mut tracker = WorkspaceEditTracker::new(client_capabilities); + tracker.set_edits_for_document( + url.clone(), + version, + fix_all_edit(document, linter_settings, encoding)?, + )?; + Ok(tracker.into_workspace_edit()) +} + +pub(super) fn fix_all_edit( + document: &crate::edit::Document, + linter_settings: &LinterSettings, + encoding: PositionEncoding, +) -> crate::Result> { + crate::fix::fix_all(document, linter_settings, encoding) } pub(super) fn resolve_edit_for_organize_imports( document: &crate::edit::Document, + client_capabilities: &ResolvedClientCapabilities, url: &types::Url, linter_settings: &ruff_linter::settings::LinterSettings, encoding: PositionEncoding, + version: DocumentVersion, ) -> crate::Result { + let mut tracker = WorkspaceEditTracker::new(client_capabilities); + tracker.set_edits_for_document( + url.clone(), + version, + organize_imports_edit(document, linter_settings, encoding)?, + )?; + Ok(tracker.into_workspace_edit()) +} + +pub(super) fn organize_imports_edit( + document: &crate::edit::Document, + linter_settings: &LinterSettings, + encoding: PositionEncoding, +) -> crate::Result> { let mut linter_settings = linter_settings.clone(); linter_settings.rules = [ Rule::UnsortedImports, // I001 @@ -102,15 +129,5 @@ pub(super) fn resolve_edit_for_organize_imports( .into_iter() .collect(); - Ok(types::WorkspaceEdit { - changes: Some( - [( - url.clone(), - crate::fix::fix_all(document, &linter_settings, encoding)?, - )] - .into_iter() - .collect(), - ), - ..Default::default() - }) + crate::fix::fix_all(document, &linter_settings, encoding) } diff --git a/crates/ruff_server/src/server/api/requests/diagnostic.rs b/crates/ruff_server/src/server/api/requests/diagnostic.rs index 634f111f86d02..e72245f2015f0 100644 --- a/crates/ruff_server/src/server/api/requests/diagnostic.rs +++ b/crates/ruff_server/src/server/api/requests/diagnostic.rs @@ -19,11 +19,15 @@ impl super::BackgroundDocumentRequestHandler for DocumentDiagnostic { _notifier: Notifier, _params: types::DocumentDiagnosticParams, ) -> Result { - let diagnostics = crate::lint::check( - snapshot.document(), - &snapshot.configuration().linter, - snapshot.encoding(), - ); + let diagnostics = if snapshot.client_settings().lint() { + crate::lint::check( + snapshot.document(), + &snapshot.configuration().linter, + snapshot.encoding(), + ) + } else { + vec![] + }; Ok(DocumentDiagnosticReportResult::Report( types::DocumentDiagnosticReport::Full(RelatedFullDocumentDiagnosticReport { diff --git a/crates/ruff_server/src/server/api/requests/execute_command.rs b/crates/ruff_server/src/server/api/requests/execute_command.rs new file mode 100644 index 0000000000000..c8cdb7fdec05a --- /dev/null +++ b/crates/ruff_server/src/server/api/requests/execute_command.rs @@ -0,0 +1,153 @@ +use std::str::FromStr; + +use crate::edit::WorkspaceEditTracker; +use crate::server::api::LSPResult; +use crate::server::client; +use crate::server::schedule::Task; +use crate::session::Session; +use crate::DIAGNOSTIC_NAME; +use crate::{edit::DocumentVersion, server}; +use lsp_server::ErrorCode; +use lsp_types::{self as types, request as req}; +use serde::Deserialize; + +#[derive(Debug)] +enum Command { + Format, + FixAll, + OrganizeImports, +} + +pub(crate) struct ExecuteCommand; + +#[derive(Deserialize)] +struct TextDocumentArgument { + uri: types::Url, + version: DocumentVersion, +} + +impl super::RequestHandler for ExecuteCommand { + type RequestType = req::ExecuteCommand; +} + +impl super::SyncRequestHandler for ExecuteCommand { + fn run( + session: &mut Session, + _notifier: client::Notifier, + requester: &mut client::Requester, + params: types::ExecuteCommandParams, + ) -> server::Result> { + let command = + Command::from_str(¶ms.command).with_failure_code(ErrorCode::InvalidParams)?; + + // check if we can apply a workspace edit + if !session.resolved_client_capabilities().apply_edit { + return Err(anyhow::anyhow!("Cannot execute the '{}' command: the client does not support `workspace/applyEdit`", command.label())).with_failure_code(ErrorCode::InternalError); + } + + let mut arguments: Vec = params + .arguments + .into_iter() + .map(|value| serde_json::from_value(value).with_failure_code(ErrorCode::InvalidParams)) + .collect::>()?; + + arguments.sort_by(|a, b| a.uri.cmp(&b.uri)); + arguments.dedup_by(|a, b| a.uri == b.uri); + + let mut edit_tracker = WorkspaceEditTracker::new(session.resolved_client_capabilities()); + for TextDocumentArgument { uri, version } in arguments { + let snapshot = session + .take_snapshot(&uri) + .ok_or(anyhow::anyhow!("Document snapshot not available for {uri}",)) + .with_failure_code(ErrorCode::InternalError)?; + match command { + Command::FixAll => { + let edits = super::code_action_resolve::fix_all_edit( + snapshot.document(), + &snapshot.configuration().linter, + snapshot.encoding(), + ) + .with_failure_code(ErrorCode::InternalError)?; + edit_tracker + .set_edits_for_document(uri, version, edits) + .with_failure_code(ErrorCode::InternalError)?; + } + Command::Format => { + let response = super::format::format_document(&snapshot)?; + if let Some(edits) = response { + edit_tracker + .set_edits_for_document(uri, version, edits) + .with_failure_code(ErrorCode::InternalError)?; + } + } + Command::OrganizeImports => { + let edits = super::code_action_resolve::organize_imports_edit( + snapshot.document(), + &snapshot.configuration().linter, + snapshot.encoding(), + ) + .with_failure_code(ErrorCode::InternalError)?; + edit_tracker + .set_edits_for_document(uri, version, edits) + .with_failure_code(ErrorCode::InternalError)?; + } + } + } + + if !edit_tracker.is_empty() { + apply_edit( + requester, + command.label(), + edit_tracker.into_workspace_edit(), + ) + .with_failure_code(ErrorCode::InternalError)?; + } + + Ok(None) + } +} + +impl Command { + fn label(&self) -> &str { + match self { + Self::FixAll => "Fix all auto-fixable problems", + Self::Format => "Format document", + Self::OrganizeImports => "Format imports", + } + } +} + +impl FromStr for Command { + type Err = anyhow::Error; + + fn from_str(name: &str) -> Result { + Ok(match name { + "ruff.applyAutofix" => Self::FixAll, + "ruff.applyFormat" => Self::Format, + "ruff.applyOrganizeImports" => Self::OrganizeImports, + _ => return Err(anyhow::anyhow!("Invalid command `{name}`")), + }) + } +} + +fn apply_edit( + requester: &mut client::Requester, + label: &str, + edit: types::WorkspaceEdit, +) -> crate::Result<()> { + requester.request::( + types::ApplyWorkspaceEditParams { + label: Some(format!("{DIAGNOSTIC_NAME}: {label}")), + edit, + }, + |response| { + if !response.applied { + let reason = response + .failure_reason + .unwrap_or_else(|| String::from("unspecified reason")); + tracing::error!("Failed to apply workspace edit: {}", reason); + } + Task::nothing() + }, + ) +} diff --git a/crates/ruff_server/src/server/api/requests/format.rs b/crates/ruff_server/src/server/api/requests/format.rs index 566e3609054bf..05a4485f594e7 100644 --- a/crates/ruff_server/src/server/api/requests/format.rs +++ b/crates/ruff_server/src/server/api/requests/format.rs @@ -19,31 +19,35 @@ impl super::BackgroundDocumentRequestHandler for Format { _notifier: Notifier, _params: types::DocumentFormattingParams, ) -> Result { - let doc = snapshot.document(); - let source = doc.contents(); - let formatted = crate::format::format(doc, &snapshot.configuration().formatter) - .with_failure_code(lsp_server::ErrorCode::InternalError)?; - // fast path - if the code is the same, return early - if formatted == source { - return Ok(None); - } - let formatted_index: LineIndex = LineIndex::from_source_text(&formatted); + format_document(&snapshot) + } +} - let unformatted_index = doc.index(); +pub(super) fn format_document(snapshot: &DocumentSnapshot) -> Result { + let doc = snapshot.document(); + let source = doc.contents(); + let formatted = crate::format::format(doc, &snapshot.configuration().formatter) + .with_failure_code(lsp_server::ErrorCode::InternalError)?; + // fast path - if the code is the same, return early + if formatted == source { + return Ok(None); + } + let formatted_index: LineIndex = LineIndex::from_source_text(&formatted); - let Replacement { - source_range, - modified_range: formatted_range, - } = Replacement::between( - source, - unformatted_index.line_starts(), - &formatted, - formatted_index.line_starts(), - ); + let unformatted_index = doc.index(); - Ok(Some(vec![TextEdit { - range: source_range.to_range(source, unformatted_index, snapshot.encoding()), - new_text: formatted[formatted_range].to_owned(), - }])) - } + let Replacement { + source_range, + modified_range: formatted_range, + } = Replacement::between( + source, + unformatted_index.line_starts(), + &formatted, + formatted_index.line_starts(), + ); + + Ok(Some(vec![TextEdit { + range: source_range.to_range(source, unformatted_index, snapshot.encoding()), + new_text: formatted[formatted_range].to_owned(), + }])) } diff --git a/crates/ruff_server/src/server/api/traits.rs b/crates/ruff_server/src/server/api/traits.rs index 7a980ebfca8e7..59da1624e3924 100644 --- a/crates/ruff_server/src/server/api/traits.rs +++ b/crates/ruff_server/src/server/api/traits.rs @@ -1,6 +1,6 @@ //! A stateful LSP implementation that calls into the Ruff API. -use crate::server::client::Notifier; +use crate::server::client::{Notifier, Requester}; use crate::session::{DocumentSnapshot, Session}; use lsp_types::notification::Notification as LSPNotification; @@ -20,6 +20,7 @@ pub(super) trait SyncRequestHandler: RequestHandler { fn run( session: &mut Session, notifier: Notifier, + requester: &mut Requester, params: <::RequestType as Request>::Params, ) -> super::Result<<::RequestType as Request>::Result>; } diff --git a/crates/ruff_server/src/server/schedule.rs b/crates/ruff_server/src/server/schedule.rs index 3e5ecbd35a781..fe8cc5c18c4e0 100644 --- a/crates/ruff_server/src/server/schedule.rs +++ b/crates/ruff_server/src/server/schedule.rs @@ -80,10 +80,13 @@ impl<'s> Scheduler<'s> { pub(super) fn dispatch(&mut self, task: task::Task<'s>) { match task { Task::Sync(SyncTask { func }) => { + let notifier = self.client.notifier(); + let responder = self.client.responder(); func( self.session, - self.client.notifier(), - self.client.responder(), + notifier, + &mut self.client.requester, + responder, ); } Task::Background(BackgroundTaskBuilder { diff --git a/crates/ruff_server/src/server/schedule/task.rs b/crates/ruff_server/src/server/schedule/task.rs index b4de2d8c97b0a..fdba5e3991d9a 100644 --- a/crates/ruff_server/src/server/schedule/task.rs +++ b/crates/ruff_server/src/server/schedule/task.rs @@ -2,11 +2,11 @@ use lsp_server::RequestId; use serde::Serialize; use crate::{ - server::client::{Notifier, Responder}, + server::client::{Notifier, Requester, Responder}, session::Session, }; -type LocalFn<'s> = Box; +type LocalFn<'s> = Box; type BackgroundFn = Box; @@ -68,7 +68,9 @@ impl<'s> Task<'s> { }) } /// Creates a new local task. - pub(crate) fn local(func: impl FnOnce(&mut Session, Notifier, Responder) + 's) -> Self { + pub(crate) fn local( + func: impl FnOnce(&mut Session, Notifier, &mut Requester, Responder) + 's, + ) -> Self { Self::Sync(SyncTask { func: Box::new(func), }) @@ -79,14 +81,15 @@ impl<'s> Task<'s> { where R: Serialize + Send + 'static, { - Self::local(move |_, _, responder| { + Self::local(move |_, _, _, responder| { if let Err(err) = responder.respond(id, result) { tracing::error!("Unable to send immediate response: {err}"); } }) } + /// Creates a local task that does nothing. pub(crate) fn nothing() -> Self { - Self::local(move |_, _, _| {}) + Self::local(move |_, _, _, _| {}) } } diff --git a/crates/ruff_server/src/session.rs b/crates/ruff_server/src/session.rs index 208ad923fd5cf..72580c3a3022b 100644 --- a/crates/ruff_server/src/session.rs +++ b/crates/ruff_server/src/session.rs @@ -15,7 +15,7 @@ use rustc_hash::FxHashMap; use crate::edit::{Document, DocumentVersion}; use crate::PositionEncoding; -use self::capabilities::ResolvedClientCapabilities; +pub(crate) use self::capabilities::ResolvedClientCapabilities; use self::settings::ResolvedClientSettings; pub(crate) use self::settings::{AllSettings, ClientSettings}; @@ -36,7 +36,6 @@ pub(crate) struct Session { pub(crate) struct DocumentSnapshot { configuration: Arc, resolved_client_capabilities: Arc, - #[allow(dead_code)] client_settings: settings::ResolvedClientSettings, document_ref: DocumentRef, position_encoding: PositionEncoding, @@ -96,12 +95,10 @@ impl Session { } pub(crate) fn take_snapshot(&self, url: &Url) -> Option { - let resolved_settings = self.workspaces.client_settings(url, &self.global_settings); - tracing::info!("Resolved settings for document {url}: {resolved_settings:?}"); Some(DocumentSnapshot { configuration: self.workspaces.configuration(url)?.clone(), resolved_client_capabilities: self.resolved_client_capabilities.clone(), - client_settings: resolved_settings, + client_settings: self.workspaces.client_settings(url, &self.global_settings), document_ref: self.workspaces.snapshot(url)?, position_encoding: self.position_encoding, url: url.clone(), @@ -140,6 +137,10 @@ impl Session { Ok(()) } + pub(crate) fn resolved_client_capabilities(&self) -> &ResolvedClientCapabilities { + &self.resolved_client_capabilities + } + pub(crate) fn encoding(&self) -> PositionEncoding { self.position_encoding } @@ -215,6 +216,10 @@ impl DocumentSnapshot { &self.resolved_client_capabilities } + pub(crate) fn client_settings(&self) -> &ResolvedClientSettings { + &self.client_settings + } + pub(crate) fn document(&self) -> &DocumentRef { &self.document_ref } diff --git a/crates/ruff_server/src/session/capabilities.rs b/crates/ruff_server/src/session/capabilities.rs index f415aa7541169..563737542cf51 100644 --- a/crates/ruff_server/src/session/capabilities.rs +++ b/crates/ruff_server/src/session/capabilities.rs @@ -3,6 +3,8 @@ use lsp_types::ClientCapabilities; #[derive(Debug, Clone, PartialEq, Eq, Default)] pub(crate) struct ResolvedClientCapabilities { pub(crate) code_action_deferred_edit_resolution: bool, + pub(crate) apply_edit: bool, + pub(crate) document_changes: bool, } impl ResolvedClientCapabilities { @@ -17,9 +19,25 @@ impl ResolvedClientCapabilities { let code_action_edit_resolution = code_action_settings .and_then(|code_action_settings| code_action_settings.resolve_support.as_ref()) .is_some_and(|resolve_support| resolve_support.properties.contains(&"edit".into())); + + let apply_edit = client_capabilities + .workspace + .as_ref() + .and_then(|workspace| workspace.apply_edit) + .unwrap_or_default(); + + let document_changes = client_capabilities + .workspace + .as_ref() + .and_then(|workspace| workspace.workspace_edit.as_ref()) + .and_then(|workspace_edit| workspace_edit.document_changes) + .unwrap_or_default(); + Self { code_action_deferred_edit_resolution: code_action_data_support && code_action_edit_resolution, + apply_edit, + document_changes, } } } diff --git a/crates/ruff_server/src/session/settings.rs b/crates/ruff_server/src/session/settings.rs index b6e831b1d05ed..a29692a63c419 100644 --- a/crates/ruff_server/src/session/settings.rs +++ b/crates/ruff_server/src/session/settings.rs @@ -12,12 +12,13 @@ pub(crate) type WorkspaceSettingsMap = FxHashMap; /// sends them. #[derive(Debug)] #[cfg_attr(test, derive(PartialEq, Eq))] -// TODO(jane): Remove dead code warning -#[allow(dead_code, clippy::struct_excessive_bools)] +#[allow(clippy::struct_excessive_bools)] pub(crate) struct ResolvedClientSettings { fix_all: bool, organize_imports: bool, lint_enable: bool, + // TODO(jane): Remove once noqa auto-fix is implemented + #[allow(dead_code)] disable_rule_comment_enable: bool, fix_violation_enable: bool, } @@ -196,6 +197,24 @@ impl ResolvedClientSettings { } } +impl ResolvedClientSettings { + pub(crate) fn fix_all(&self) -> bool { + self.fix_all + } + + pub(crate) fn organize_imports(&self) -> bool { + self.organize_imports + } + + pub(crate) fn lint(&self) -> bool { + self.lint_enable + } + + pub(crate) fn fix_violation(&self) -> bool { + self.fix_violation_enable + } +} + impl Default for InitializationOptions { fn default() -> Self { Self::GlobalOnly { settings: None } diff --git a/crates/ruff_workspace/src/options.rs b/crates/ruff_workspace/src/options.rs index fefffa3c1defb..2553290464f3e 100644 --- a/crates/ruff_workspace/src/options.rs +++ b/crates/ruff_workspace/src/options.rs @@ -905,7 +905,8 @@ pub struct LintCommonOptions { // Tables are required to go last. /// A list of mappings from file pattern to rule codes or prefixes to - /// exclude, when considering any matching files. + /// exclude, when considering any matching files. An initial '!' negates + /// the file pattern. #[option( default = "{}", value_type = "dict[str, list[RuleSelector]]", @@ -914,6 +915,8 @@ pub struct LintCommonOptions { # Ignore `E402` (import violations) in all `__init__.py` files, and in `path/to/file.py`. "__init__.py" = ["E402"] "path/to/file.py" = ["E402"] + # Ignore `D` rules everywhere except for the `src/` directory. + "!src/**.py" = ["F401"] "# )] pub per_file_ignores: Option>>, diff --git a/docs/faq.md b/docs/faq.md index f6fff988df650..02d2ff1663585 100644 --- a/docs/faq.md +++ b/docs/faq.md @@ -628,11 +628,12 @@ Even still, given the dynamic nature of Python, it's difficult to have _complete making changes to code, even for seemingly trivial fixes. If a "safe" fix breaks your code, please [file an Issue](https://github.com/astral-sh/ruff/issues/new). -## How can I disable Ruff's color output? +## How can I disable/force Ruff's color output? Ruff's color output is powered by the [`colored`](https://crates.io/crates/colored) crate, which attempts to automatically detect whether the output stream supports color. However, you can force -colors off by setting the `NO_COLOR` environment variable to any value (e.g., `NO_COLOR=1`). +colors off by setting the `NO_COLOR` environment variable to any value (e.g., `NO_COLOR=1`), or +force colors on by setting `FORCE_COLOR` to any non-empty value (e.g. `FORCE_COLOR=1`). [`colored`](https://crates.io/crates/colored) also supports the `CLICOLOR` and `CLICOLOR_FORCE` environment variables (see the [spec](https://bixense.com/clicolors/)). diff --git a/playground/api/package-lock.json b/playground/api/package-lock.json index 293caa2cdfd25..4ac44e2d41a07 100644 --- a/playground/api/package-lock.json +++ b/playground/api/package-lock.json @@ -16,7 +16,7 @@ "@cloudflare/workers-types": "^4.20230801.0", "miniflare": "^3.20230801.1", "typescript": "^5.1.6", - "wrangler": "3.42.0" + "wrangler": "3.48.0" } }, "node_modules/@cloudflare/kv-asset-handler": { @@ -29,9 +29,9 @@ } }, "node_modules/@cloudflare/workerd-darwin-64": { - "version": "1.20240329.0", - "resolved": "https://registry.npmjs.org/@cloudflare/workerd-darwin-64/-/workerd-darwin-64-1.20240329.0.tgz", - "integrity": "sha512-/raHmsHrYjoC5am84wqyiZIDCRrrYN6YDFb4zchwWQzJ0ZHleUeY6IzNdjujrS/gYey/+Db9oyl2PD1xAZt4gA==", + "version": "1.20240404.0", + "resolved": "https://registry.npmjs.org/@cloudflare/workerd-darwin-64/-/workerd-darwin-64-1.20240404.0.tgz", + "integrity": "sha512-rc/ov3I9GwgKRtUnkShNW3TIoZEPHzExrMRNlHq1VpXQRBSchHdMw8meMn54+oqgxW1AKLmPWj/c0A7EnYAsIw==", "cpu": [ "x64" ], @@ -45,9 +45,9 @@ } }, "node_modules/@cloudflare/workerd-darwin-arm64": { - "version": "1.20240329.0", - "resolved": "https://registry.npmjs.org/@cloudflare/workerd-darwin-arm64/-/workerd-darwin-arm64-1.20240329.0.tgz", - "integrity": "sha512-3wnwVdfFDt+JUhlA6NWW+093ryGNF0HMuBmkOh0PG6j4GMRH8Y+EDsqzqrzT3ZoGGXbI9x1H7k15VKb3LAN/KA==", + "version": "1.20240404.0", + "resolved": "https://registry.npmjs.org/@cloudflare/workerd-darwin-arm64/-/workerd-darwin-arm64-1.20240404.0.tgz", + "integrity": "sha512-V9oPjeC2PYBCoAYnjbO2bsjT7PtzxfUHnh780FUi1r59Hwxd7FNlojwsIidA0nS/1WV5UKeJusIdrYlQbsketA==", "cpu": [ "arm64" ], @@ -61,9 +61,9 @@ } }, "node_modules/@cloudflare/workerd-linux-64": { - "version": "1.20240329.0", - "resolved": "https://registry.npmjs.org/@cloudflare/workerd-linux-64/-/workerd-linux-64-1.20240329.0.tgz", - "integrity": "sha512-E909ZIXgjdr2iuq5bF/vq02elizDlPQoYRiKojdvODC7w8rbnpwnuptajS4xK5kmKh4XBiU2o9NDhut/W1kfyw==", + "version": "1.20240404.0", + "resolved": "https://registry.npmjs.org/@cloudflare/workerd-linux-64/-/workerd-linux-64-1.20240404.0.tgz", + "integrity": "sha512-ndO7q6G2X8uYd5byGFzow4SWPqINQcmJ7pKKATNa+9vh/YMO0of2ihPntnm9uv577l8nRiAwRkHbnsWoEI33qQ==", "cpu": [ "x64" ], @@ -77,9 +77,9 @@ } }, "node_modules/@cloudflare/workerd-linux-arm64": { - "version": "1.20240329.0", - "resolved": "https://registry.npmjs.org/@cloudflare/workerd-linux-arm64/-/workerd-linux-arm64-1.20240329.0.tgz", - "integrity": "sha512-PELA3FVW75pKchsSI5o40oiClFY2Uiq+KUx/f/srwz2pIJoM5YWLmFrv+s8feKoEwuabxIGSzHxy7QA++HyprQ==", + "version": "1.20240404.0", + "resolved": "https://registry.npmjs.org/@cloudflare/workerd-linux-arm64/-/workerd-linux-arm64-1.20240404.0.tgz", + "integrity": "sha512-hto5pjKYFqFu2Rvxnh84TzgDwalBRXQSwOVHltcgqo48en9TJDCN48ZtLj2G7QTGUOMW88h+nqdbj8+P7S/GXQ==", "cpu": [ "arm64" ], @@ -93,9 +93,9 @@ } }, "node_modules/@cloudflare/workerd-windows-64": { - "version": "1.20240329.0", - "resolved": "https://registry.npmjs.org/@cloudflare/workerd-windows-64/-/workerd-windows-64-1.20240329.0.tgz", - "integrity": "sha512-/T+AcjVqTuqAeGBQmjAF4TOTm8sv3BSO/NtUPa1ghCvsp1sb03L6/c3wFc9ZonSdRYeBb0XDX7PnenGCvjr/Tw==", + "version": "1.20240404.0", + "resolved": "https://registry.npmjs.org/@cloudflare/workerd-windows-64/-/workerd-windows-64-1.20240404.0.tgz", + "integrity": "sha512-DpCLvNkOeFinKGJwv9qbyT7RLZ1168dhPx85IHSqAYVWZIszNSmNOkEDqklvoJoab01AqETrrEhwBdmjCA7qfA==", "cpu": [ "x64" ], @@ -109,9 +109,9 @@ } }, "node_modules/@cloudflare/workers-types": { - "version": "4.20240329.0", - "resolved": "https://registry.npmjs.org/@cloudflare/workers-types/-/workers-types-4.20240329.0.tgz", - "integrity": "sha512-AbzgvSQjG8Nci4xxQEcjTTVjiWXgOQnFIbIHtEZXteHiMGDXMWGegjWBo5JHGsZCq+U5V/SD5EnlypQnUQEoig==", + "version": "4.20240405.0", + "resolved": "https://registry.npmjs.org/@cloudflare/workers-types/-/workers-types-4.20240405.0.tgz", + "integrity": "sha512-sEVOhyOgXUwfLkgHqbLZa/sfkSYrh7/zLmI6EZNibPaVPvAnAcItbNNl3SAlLyLKuwf8m4wAIAgu9meKWCvXjg==", "dev": true }, "node_modules/@cspotcode/source-map-support": { @@ -1067,9 +1067,9 @@ } }, "node_modules/miniflare": { - "version": "3.20240329.0", - "resolved": "https://registry.npmjs.org/miniflare/-/miniflare-3.20240329.0.tgz", - "integrity": "sha512-kdHlMwhV241kck5oh8uyKPIhCusP1BL4+iOSeJZgcJ46EATA6crWtYqlARNU9t/iYXhzKhXOlOPJjjlCJuOgTA==", + "version": "3.20240404.0", + "resolved": "https://registry.npmjs.org/miniflare/-/miniflare-3.20240404.0.tgz", + "integrity": "sha512-+FOTcztPMW3akmucX4vE0PWMNvP4JBwl4s9ieA84fcOaDtTbtfU1rHXpcacj16klpUpvSnD6xd8Sjsn6SJXPfg==", "dev": true, "dependencies": { "@cspotcode/source-map-support": "0.8.1", @@ -1080,7 +1080,7 @@ "glob-to-regexp": "^0.4.1", "stoppable": "^1.1.0", "undici": "^5.28.2", - "workerd": "1.20240329.0", + "workerd": "1.20240404.0", "ws": "^8.11.0", "youch": "^3.2.2", "zod": "^3.20.6" @@ -1431,9 +1431,9 @@ "dev": true }, "node_modules/typescript": { - "version": "5.4.3", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.4.3.tgz", - "integrity": "sha512-KrPd3PKaCLr78MalgiwJnA25Nm8HAmdwN3mYUYZgG/wizIo9EainNVQI9/yDavtVFRN2h3k8uf3GLHuhDMgEHg==", + "version": "5.4.4", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.4.4.tgz", + "integrity": "sha512-dGE2Vv8cpVvw28v8HCPqyb08EzbBURxDpuhJvTrusShUfGnhHBafDsLdS1EhhxyL6BJQE+2cT3dDPAv+MQ6oLw==", "dev": true, "bin": { "tsc": "bin/tsc", @@ -1493,9 +1493,9 @@ } }, "node_modules/workerd": { - "version": "1.20240329.0", - "resolved": "https://registry.npmjs.org/workerd/-/workerd-1.20240329.0.tgz", - "integrity": "sha512-6wWuMOwWsp3K6447XsI/MZYFq0KlpV2zVbbNFEkv3N7UgJJKaHGwL/hilr6RlS4UFLU4co8nrF2lc5uR781HKg==", + "version": "1.20240404.0", + "resolved": "https://registry.npmjs.org/workerd/-/workerd-1.20240404.0.tgz", + "integrity": "sha512-U4tfnvBcPMsv7pmRGuF0J5UnoZi6tbc64tXNfyijI74r6w6Vlb2+a6eibdQL8g0g46+4vjuTKME9G5RvSvdc8g==", "dev": true, "hasInstallScript": true, "bin": { @@ -1505,17 +1505,17 @@ "node": ">=16" }, "optionalDependencies": { - "@cloudflare/workerd-darwin-64": "1.20240329.0", - "@cloudflare/workerd-darwin-arm64": "1.20240329.0", - "@cloudflare/workerd-linux-64": "1.20240329.0", - "@cloudflare/workerd-linux-arm64": "1.20240329.0", - "@cloudflare/workerd-windows-64": "1.20240329.0" + "@cloudflare/workerd-darwin-64": "1.20240404.0", + "@cloudflare/workerd-darwin-arm64": "1.20240404.0", + "@cloudflare/workerd-linux-64": "1.20240404.0", + "@cloudflare/workerd-linux-arm64": "1.20240404.0", + "@cloudflare/workerd-windows-64": "1.20240404.0" } }, "node_modules/wrangler": { - "version": "3.42.0", - "resolved": "https://registry.npmjs.org/wrangler/-/wrangler-3.42.0.tgz", - "integrity": "sha512-t/Fq80aG5RrCyN118aV9oG1Tt66QEatz0tarKzpy0cuUMUf3T3yJAuYb6kmYIy6+beQJNoGWSAjjhAQnOn2MCQ==", + "version": "3.48.0", + "resolved": "https://registry.npmjs.org/wrangler/-/wrangler-3.48.0.tgz", + "integrity": "sha512-Wv7JS6FyX1j9HkaM6WL3fmTzBMAYc4hPSyZCuxuH55hkJDX/7ts+YAgsaN1U8rKoDrV3FVSgBfI9TyqP9iuM8Q==", "dev": true, "dependencies": { "@cloudflare/kv-asset-handler": "0.3.1", @@ -1524,7 +1524,7 @@ "blake3-wasm": "^2.1.5", "chokidar": "^3.5.3", "esbuild": "0.17.19", - "miniflare": "3.20240329.0", + "miniflare": "3.20240404.0", "nanoid": "^3.3.3", "path-to-regexp": "^6.2.0", "resolve": "^1.22.8", @@ -1544,7 +1544,7 @@ "fsevents": "~2.3.2" }, "peerDependencies": { - "@cloudflare/workers-types": "^4.20240320.1" + "@cloudflare/workers-types": "^4.20240404.0" }, "peerDependenciesMeta": { "@cloudflare/workers-types": { diff --git a/playground/api/package.json b/playground/api/package.json index 107cdc7d91090..99db13e82fe5a 100644 --- a/playground/api/package.json +++ b/playground/api/package.json @@ -5,7 +5,7 @@ "@cloudflare/workers-types": "^4.20230801.0", "miniflare": "^3.20230801.1", "typescript": "^5.1.6", - "wrangler": "3.42.0" + "wrangler": "3.48.0" }, "private": true, "scripts": { diff --git a/playground/package.json b/playground/package.json index 274a5fe6eb3a3..f6835efa8bec9 100644 --- a/playground/package.json +++ b/playground/package.json @@ -30,7 +30,7 @@ "@typescript-eslint/parser": "^7.0.0", "@vitejs/plugin-react-swc": "^3.0.0", "autoprefixer": "^10.4.13", - "eslint": "^8.30.0", + "eslint": "^9.0.0", "eslint-config-prettier": "^9.0.0", "eslint-plugin-import": "^2.26.0", "eslint-plugin-prettier": "^5.0.0", diff --git a/playground/src/Editor/Editor.tsx b/playground/src/Editor/Editor.tsx index a2696dabc6fe8..5bfc5bc1cf28c 100644 --- a/playground/src/Editor/Editor.tsx +++ b/playground/src/Editor/Editor.tsx @@ -7,7 +7,7 @@ import { } from "react"; import { Panel, PanelGroup } from "react-resizable-panels"; import { DEFAULT_PYTHON_SOURCE } from "../constants"; -import init, { Diagnostic, Workspace } from "../pkg"; +import init, { Diagnostic, Workspace } from "../pkg/ruff_wasm"; import { ErrorMessage } from "./ErrorMessage"; import Header from "./Header"; import PrimarySideBar from "./PrimarySideBar"; diff --git a/playground/src/index.css b/playground/src/index.css index 54b6cf59b2feb..b644d5add34e7 100644 --- a/playground/src/index.css +++ b/playground/src/index.css @@ -28,7 +28,8 @@ html, @font-face { font-family: "Alliance Text"; - src: url("../fonts/Alliance-TextRegular.woff2") format("woff2"), + src: + url("../fonts/Alliance-TextRegular.woff2") format("woff2"), url("../fonts/Alliance-TextRegular.woff") format("woff"); font-weight: normal; font-style: normal; @@ -37,7 +38,8 @@ html, @font-face { font-family: "Alliance Text"; - src: url("../fonts/Alliance-TextMedium.woff2") format("woff2"), + src: + url("../fonts/Alliance-TextMedium.woff2") format("woff2"), url("../fonts/Alliance-TextMedium.woff") format("woff"); font-weight: 500; font-style: normal; @@ -46,7 +48,8 @@ html, @font-face { font-family: "Alliance Platt"; - src: url("../fonts/Alliance-PlattMedium.woff2") format("woff2"), + src: + url("../fonts/Alliance-PlattMedium.woff2") format("woff2"), url("../fonts/Alliance-PlattMedium.woff") format("woff"); font-weight: 500; font-style: normal; @@ -55,7 +58,8 @@ html, @font-face { font-family: "Alliance Platt"; - src: url("../fonts/Alliance-PlattRegular.woff2") format("woff2"), + src: + url("../fonts/Alliance-PlattRegular.woff2") format("woff2"), url("../fonts/Alliance-PlattRegular.woff") format("woff"); font-weight: normal; font-style: normal; diff --git a/pyproject.toml b/pyproject.toml index c1db0a858587e..fdcf414dbc41e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -58,10 +58,17 @@ include = [ [tool.ruff] extend-exclude = [ + "crates/ruff/resources/", "crates/ruff_linter/resources/", "crates/ruff_python_formatter/resources/" ] +[tool.ruff.lint] +ignore = [ + # Conflicts with the formatter + "COM812", "ISC001" +] + [tool.black] force-exclude = ''' /( diff --git a/python/ruff-ecosystem/ruff_ecosystem/check.py b/python/ruff-ecosystem/ruff_ecosystem/check.py index 983133b9a2bf0..bebbe97a62953 100644 --- a/python/ruff-ecosystem/ruff_ecosystem/check.py +++ b/python/ruff-ecosystem/ruff_ecosystem/check.py @@ -1,6 +1,7 @@ """ Execution, comparison, and summary of `ruff check` ecosystem checks. """ + from __future__ import annotations import asyncio diff --git a/python/ruff-ecosystem/ruff_ecosystem/defaults.py b/python/ruff-ecosystem/ruff_ecosystem/defaults.py index bf096046fb937..e6d130d015d4d 100644 --- a/python/ruff-ecosystem/ruff_ecosystem/defaults.py +++ b/python/ruff-ecosystem/ruff_ecosystem/defaults.py @@ -1,6 +1,7 @@ """ Default projects for ecosystem checks """ + from ruff_ecosystem.projects import ( CheckOptions, FormatOptions, diff --git a/ruff.schema.json b/ruff.schema.json index ab68b675b3bd1..a7c2d7cea8986 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -554,7 +554,7 @@ ] }, "per-file-ignores": { - "description": "A list of mappings from file pattern to rule codes or prefixes to exclude, when considering any matching files.", + "description": "A list of mappings from file pattern to rule codes or prefixes to exclude, when considering any matching files. An initial '!' negates the file pattern.", "deprecated": true, "type": [ "object", @@ -2168,7 +2168,7 @@ ] }, "per-file-ignores": { - "description": "A list of mappings from file pattern to rule codes or prefixes to exclude, when considering any matching files.", + "description": "A list of mappings from file pattern to rule codes or prefixes to exclude, when considering any matching files. An initial '!' negates the file pattern.", "type": [ "object", "null" @@ -3044,6 +3044,7 @@ "FURB103", "FURB105", "FURB11", + "FURB110", "FURB113", "FURB118", "FURB12", @@ -3355,6 +3356,7 @@ "PLR172", "PLR1722", "PLR173", + "PLR1730", "PLR1733", "PLR1736", "PLR2", @@ -3868,6 +3870,7 @@ "UP04", "UP040", "UP041", + "UP042", "W", "W1", "W19", diff --git a/scripts/add_plugin.py b/scripts/add_plugin.py index e930398c65bcf..1e12e47e6b178 100755 --- a/scripts/add_plugin.py +++ b/scripts/add_plugin.py @@ -8,6 +8,7 @@ --url https://pypi.org/project/flake8-pie/ \ --prefix PIE """ + from __future__ import annotations import argparse diff --git a/scripts/check_docs_formatted.py b/scripts/check_docs_formatted.py index c25e3d1929ca2..96f6ec3a7476a 100755 --- a/scripts/check_docs_formatted.py +++ b/scripts/check_docs_formatted.py @@ -1,5 +1,6 @@ #!/usr/bin/env python3 """Check code snippets in docs are formatted by black.""" + from __future__ import annotations import argparse diff --git a/scripts/check_ecosystem.py b/scripts/check_ecosystem.py index f13d623391b93..cc8206673f289 100755 --- a/scripts/check_ecosystem.py +++ b/scripts/check_ecosystem.py @@ -9,6 +9,7 @@ scripts/check_ecosystem.py """ + from __future__ import annotations import argparse diff --git a/scripts/ecosystem_all_check.py b/scripts/ecosystem_all_check.py index 8882142e85f14..7d3c2e5cfe51f 100644 --- a/scripts/ecosystem_all_check.py +++ b/scripts/ecosystem_all_check.py @@ -3,6 +3,7 @@ It's a less elaborate, more hacky version of check_ecosystem.py """ + from __future__ import annotations import json diff --git a/scripts/generate_known_standard_library.py b/scripts/generate_known_standard_library.py index f35a714d0e656..559663b16da88 100644 --- a/scripts/generate_known_standard_library.py +++ b/scripts/generate_known_standard_library.py @@ -5,6 +5,7 @@ Only the generation of the file has been modified for use in this project. """ + from __future__ import annotations from pathlib import Path diff --git a/scripts/generate_mkdocs.py b/scripts/generate_mkdocs.py index 2b61daec3e7e6..5085f4400b82e 100644 --- a/scripts/generate_mkdocs.py +++ b/scripts/generate_mkdocs.py @@ -1,4 +1,5 @@ """Generate an MkDocs-compatible `docs` and `mkdocs.yml` from the README.md.""" + from __future__ import annotations import argparse diff --git a/scripts/transform_readme.py b/scripts/transform_readme.py index 7170eef20f1f3..63f5c002676bf 100644 --- a/scripts/transform_readme.py +++ b/scripts/transform_readme.py @@ -4,6 +4,7 @@ targets have different strategies for rendering light- and dark-mode images. This script adjusts the images in the README.md to support the given target. """ + from __future__ import annotations import argparse diff --git a/scripts/update_ambiguous_characters.py b/scripts/update_ambiguous_characters.py index 17347009ce966..24143adaab15d 100644 --- a/scripts/update_ambiguous_characters.py +++ b/scripts/update_ambiguous_characters.py @@ -1,4 +1,5 @@ """Generate the confusables.rs file from the VS Code ambiguous.json file.""" + from __future__ import annotations import json diff --git a/scripts/update_schemastore.py b/scripts/update_schemastore.py index 7f45f9b331eed..967ca7b75b819 100644 --- a/scripts/update_schemastore.py +++ b/scripts/update_schemastore.py @@ -4,6 +4,7 @@ to a new branch tagged with the ruff git hash. You should see a URL to create the PR to schemastore in the CLI. """ + from __future__ import annotations import json