From 45fb3732a4378dd3d8120d3b5c13c7694bc02ea2 Mon Sep 17 00:00:00 2001 From: Phong Do <45266517+phongddo@users.noreply.github.com> Date: Mon, 8 Dec 2025 20:00:05 +0100 Subject: [PATCH 1/6] [`pydocstyle`] Suppress `D417` for parameters with `Unpack` annotations (#21816) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes https://github.com/astral-sh/ruff/issues/8774 This PR fixes `pydocstyle` incorrectly flagging missing argument for arguments with `Unpack` type annotation by extracting the `kwarg` `D417` suppression logic into a helper function for future rules as needed. ## Problem Statement The below example was incorrectly triggering `D417` error for missing `**kwargs` doc. ```python class User(TypedDict): id: int name: str def do_something(some_arg: str, **kwargs: Unpack[User]): """Some doc Args: some_arg: Some argument """ ``` image `**kwargs: Unpack[User]` indicates the function expects keyword arguments that will be unpacked. Ideally, the individual fields of the User `TypedDict` should be documented, not in the `**kwargs` itself. The `**kwargs` parameter acts as a semantic grouping rather than a parameter requiring documentation. ## Solution As discussed in the linked issue, it makes sense to suppress the `D417` for parameters with `Unpack` annotation. I extract a helper function to solely check `D417` should be suppressed with `**kwarg: Unpack[T]` parameter, this function can also be unit tested independently and reduce complexity of current `missing_args` check function. This also makes it easier to add additional rules in the future. _✏️ Note:_ This is my first PR in this repo, as I've learned a ton from it, please call out anything that could be improved. Thanks for making this excellent tool 👏 ## Test Plan Add 2 test cases in `D417.py` and update snapshots. --------- Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com> --- .../test/fixtures/pydocstyle/D417.py | 23 +++++++++++++++++++ .../src/rules/pydocstyle/rules/sections.rs | 19 ++++++++++++++- ...rules__pydocstyle__tests__d417_google.snap | 10 ++++++++ ...ts__d417_google_ignore_var_parameters.snap | 10 ++++++++ ...__pydocstyle__tests__d417_unspecified.snap | 10 ++++++++ ...417_unspecified_ignore_var_parameters.snap | 10 ++++++++ 6 files changed, 81 insertions(+), 1 deletion(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pydocstyle/D417.py b/crates/ruff_linter/resources/test/fixtures/pydocstyle/D417.py index 7c5a1f538c856..510bf77bf8588 100644 --- a/crates/ruff_linter/resources/test/fixtures/pydocstyle/D417.py +++ b/crates/ruff_linter/resources/test/fixtures/pydocstyle/D417.py @@ -218,3 +218,26 @@ def should_not_fail(payload, Args): Args: The other arguments. """ + + +# Test cases for Unpack[TypedDict] kwargs +from typing import TypedDict +from typing_extensions import Unpack + +class User(TypedDict): + id: int + name: str + +def function_with_unpack_args_should_not_fail(query: str, **kwargs: Unpack[User]): + """Function with Unpack kwargs. + + Args: + query: some arg + """ + +def function_with_unpack_and_missing_arg_doc_should_fail(query: str, **kwargs: Unpack[User]): + """Function with Unpack kwargs but missing query arg documentation. + + Args: + **kwargs: keyword arguments + """ diff --git a/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs b/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs index 7b9fc80ba8adb..bf84ea85fcd5c 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs +++ b/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs @@ -4,7 +4,9 @@ use rustc_hash::FxHashSet; use std::sync::LazyLock; use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::Parameter; use ruff_python_ast::docstrings::{clean_space, leading_space}; +use ruff_python_ast::helpers::map_subscript; use ruff_python_ast::identifier::Identifier; use ruff_python_semantic::analyze::visibility::is_staticmethod; use ruff_python_trivia::textwrap::dedent; @@ -1184,6 +1186,9 @@ impl AlwaysFixableViolation for MissingSectionNameColon { /// This rule is enabled when using the `google` convention, and disabled when /// using the `pep257` and `numpy` conventions. /// +/// Parameters annotated with `typing.Unpack` are exempt from this rule. +/// This follows the Python typing specification for unpacking keyword arguments. +/// /// ## Example /// ```python /// def calculate_speed(distance: float, time: float) -> float: @@ -1233,6 +1238,7 @@ impl AlwaysFixableViolation for MissingSectionNameColon { /// - [PEP 257 – Docstring Conventions](https://peps.python.org/pep-0257/) /// - [PEP 287 – reStructuredText Docstring Format](https://peps.python.org/pep-0287/) /// - [Google Python Style Guide - Docstrings](https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings) +/// - [Python - Unpack for keyword arguments](https://typing.python.org/en/latest/spec/callables.html#unpack-kwargs) #[derive(ViolationMetadata)] #[violation_metadata(stable_since = "v0.0.73")] pub(crate) struct UndocumentedParam { @@ -1808,7 +1814,9 @@ fn missing_args(checker: &Checker, docstring: &Docstring, docstrings_args: &FxHa missing_arg_names.insert(starred_arg_name); } } - if let Some(arg) = function.parameters.kwarg.as_ref() { + if let Some(arg) = function.parameters.kwarg.as_ref() + && !has_unpack_annotation(checker, arg) + { let arg_name = arg.name.as_str(); let starred_arg_name = format!("**{arg_name}"); if !arg_name.starts_with('_') @@ -1834,6 +1842,15 @@ fn missing_args(checker: &Checker, docstring: &Docstring, docstrings_args: &FxHa } } +/// Returns `true` if the parameter is annotated with `typing.Unpack` +fn has_unpack_annotation(checker: &Checker, parameter: &Parameter) -> bool { + parameter.annotation.as_ref().is_some_and(|annotation| { + checker + .semantic() + .match_typing_expr(map_subscript(annotation), "Unpack") + }) +} + // See: `GOOGLE_ARGS_REGEX` in `pydocstyle/checker.py`. static GOOGLE_ARGS_REGEX: LazyLock = LazyLock::new(|| Regex::new(r"^\s*(\*?\*?\w+)\s*(\(.*?\))?\s*:(\r\n|\n)?\s*.+").unwrap()); diff --git a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_google.snap b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_google.snap index 63b975feb9ec6..44b20a8c6b0b1 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_google.snap +++ b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_google.snap @@ -101,3 +101,13 @@ D417 Missing argument description in the docstring for `should_fail`: `Args` 200 | """ 201 | Send a message. | + +D417 Missing argument description in the docstring for `function_with_unpack_and_missing_arg_doc_should_fail`: `query` + --> D417.py:238:5 + | +236 | """ +237 | +238 | def function_with_unpack_and_missing_arg_doc_should_fail(query: str, **kwargs: Unpack[User]): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +239 | """Function with Unpack kwargs but missing query arg documentation. + | diff --git a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_google_ignore_var_parameters.snap b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_google_ignore_var_parameters.snap index f645ff960e08e..7ff0f72bf03ba 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_google_ignore_var_parameters.snap +++ b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_google_ignore_var_parameters.snap @@ -83,3 +83,13 @@ D417 Missing argument description in the docstring for `should_fail`: `Args` 200 | """ 201 | Send a message. | + +D417 Missing argument description in the docstring for `function_with_unpack_and_missing_arg_doc_should_fail`: `query` + --> D417.py:238:5 + | +236 | """ +237 | +238 | def function_with_unpack_and_missing_arg_doc_should_fail(query: str, **kwargs: Unpack[User]): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +239 | """Function with Unpack kwargs but missing query arg documentation. + | diff --git a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_unspecified.snap b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_unspecified.snap index 63b975feb9ec6..44b20a8c6b0b1 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_unspecified.snap +++ b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_unspecified.snap @@ -101,3 +101,13 @@ D417 Missing argument description in the docstring for `should_fail`: `Args` 200 | """ 201 | Send a message. | + +D417 Missing argument description in the docstring for `function_with_unpack_and_missing_arg_doc_should_fail`: `query` + --> D417.py:238:5 + | +236 | """ +237 | +238 | def function_with_unpack_and_missing_arg_doc_should_fail(query: str, **kwargs: Unpack[User]): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +239 | """Function with Unpack kwargs but missing query arg documentation. + | diff --git a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_unspecified_ignore_var_parameters.snap b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_unspecified_ignore_var_parameters.snap index 63b975feb9ec6..44b20a8c6b0b1 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_unspecified_ignore_var_parameters.snap +++ b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__d417_unspecified_ignore_var_parameters.snap @@ -101,3 +101,13 @@ D417 Missing argument description in the docstring for `should_fail`: `Args` 200 | """ 201 | Send a message. | + +D417 Missing argument description in the docstring for `function_with_unpack_and_missing_arg_doc_should_fail`: `query` + --> D417.py:238:5 + | +236 | """ +237 | +238 | def function_with_unpack_and_missing_arg_doc_should_fail(query: str, **kwargs: Unpack[User]): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +239 | """Function with Unpack kwargs but missing query arg documentation. + | From 2d3466eccf66a2049d95542224d5985d79bcd1ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Riegel?= <96702577+LoicRiegel@users.noreply.github.com> Date: Mon, 8 Dec 2025 20:00:43 +0100 Subject: [PATCH 2/6] [`flake8-bugbear`] Accept immutable slice default arguments (`B008`) (#21823) Closes issue #21565 ## Summary As pointed out in the issue, slices are currently flagged by B008 but this behavior is incorrect because slices are immutable. ## Test Plan Added a test case in the "B006_B008.py" fixture. Sorry for the diff in the snapshots, the only thing that changes in those flies is the line numbers, though. You can also test this manually with this file: ```py # test_slice.py def c(d=slice(0, 3)): ... ``` ```sh > target/debug/ruff check tmp/test_slice.py --no-cache --select B008 All checks passed! ``` --- .../test/fixtures/flake8_bugbear/B006_B008.py | 3 + ...ke8_bugbear__tests__B006_B006_B008.py.snap | 258 +++++++++--------- ...ke8_bugbear__tests__B008_B006_B008.py.snap | 36 +-- ...ar__tests__preview__B006_B006_B008.py.snap | 258 +++++++++--------- crates/ruff_python_stdlib/src/typing.rs | 10 +- 5 files changed, 288 insertions(+), 277 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B006_B008.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B006_B008.py index 77ab80b7ee336..8e55d5b34000f 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B006_B008.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B006_B008.py @@ -199,6 +199,9 @@ def bytes_okay(value=bytes(1)): def int_okay(value=int("12")): pass +# Allow immutable slice() +def slice_okay(value=slice(1,2)): + pass # Allow immutable complex() value def complex_okay(value=complex(1,2)): diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B006_B006_B008.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B006_B006_B008.py.snap index e3f42e2da7a8b..f4113617b5279 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B006_B006_B008.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B006_B006_B008.py.snap @@ -236,227 +236,227 @@ help: Replace with `None`; initialize within function note: This is an unsafe fix and may change runtime behavior B006 [*] Do not use mutable data structures for argument defaults - --> B006_B008.py:239:20 + --> B006_B008.py:242:20 | -237 | # B006 and B008 -238 | # We should handle arbitrary nesting of these B008. -239 | def nested_combo(a=[float(3), dt.datetime.now()]): +240 | # B006 and B008 +241 | # We should handle arbitrary nesting of these B008. +242 | def nested_combo(a=[float(3), dt.datetime.now()]): | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -240 | pass +243 | pass | help: Replace with `None`; initialize within function -236 | -237 | # B006 and B008 -238 | # We should handle arbitrary nesting of these B008. +239 | +240 | # B006 and B008 +241 | # We should handle arbitrary nesting of these B008. - def nested_combo(a=[float(3), dt.datetime.now()]): -239 + def nested_combo(a=None): -240 | pass -241 | -242 | +242 + def nested_combo(a=None): +243 | pass +244 | +245 | note: This is an unsafe fix and may change runtime behavior B006 [*] Do not use mutable data structures for argument defaults - --> B006_B008.py:276:27 + --> B006_B008.py:279:27 | -275 | def mutable_annotations( -276 | a: list[int] | None = [], +278 | def mutable_annotations( +279 | a: list[int] | None = [], | ^^ -277 | b: Optional[Dict[int, int]] = {}, -278 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +280 | b: Optional[Dict[int, int]] = {}, +281 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), | help: Replace with `None`; initialize within function -273 | -274 | -275 | def mutable_annotations( +276 | +277 | +278 | def mutable_annotations( - a: list[int] | None = [], -276 + a: list[int] | None = None, -277 | b: Optional[Dict[int, int]] = {}, -278 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), -279 | d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +279 + a: list[int] | None = None, +280 | b: Optional[Dict[int, int]] = {}, +281 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +282 | d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), note: This is an unsafe fix and may change runtime behavior B006 [*] Do not use mutable data structures for argument defaults - --> B006_B008.py:277:35 + --> B006_B008.py:280:35 | -275 | def mutable_annotations( -276 | a: list[int] | None = [], -277 | b: Optional[Dict[int, int]] = {}, +278 | def mutable_annotations( +279 | a: list[int] | None = [], +280 | b: Optional[Dict[int, int]] = {}, | ^^ -278 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), -279 | d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +281 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +282 | d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), | help: Replace with `None`; initialize within function -274 | -275 | def mutable_annotations( -276 | a: list[int] | None = [], +277 | +278 | def mutable_annotations( +279 | a: list[int] | None = [], - b: Optional[Dict[int, int]] = {}, -277 + b: Optional[Dict[int, int]] = None, -278 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), -279 | d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), -280 | ): +280 + b: Optional[Dict[int, int]] = None, +281 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +282 | d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +283 | ): note: This is an unsafe fix and may change runtime behavior B006 [*] Do not use mutable data structures for argument defaults - --> B006_B008.py:278:62 + --> B006_B008.py:281:62 | -276 | a: list[int] | None = [], -277 | b: Optional[Dict[int, int]] = {}, -278 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +279 | a: list[int] | None = [], +280 | b: Optional[Dict[int, int]] = {}, +281 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), | ^^^^^ -279 | d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), -280 | ): +282 | d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +283 | ): | help: Replace with `None`; initialize within function -275 | def mutable_annotations( -276 | a: list[int] | None = [], -277 | b: Optional[Dict[int, int]] = {}, +278 | def mutable_annotations( +279 | a: list[int] | None = [], +280 | b: Optional[Dict[int, int]] = {}, - c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), -278 + c: Annotated[Union[Set[str], abc.Sized], "annotation"] = None, -279 | d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), -280 | ): -281 | pass +281 + c: Annotated[Union[Set[str], abc.Sized], "annotation"] = None, +282 | d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +283 | ): +284 | pass note: This is an unsafe fix and may change runtime behavior B006 [*] Do not use mutable data structures for argument defaults - --> B006_B008.py:279:80 + --> B006_B008.py:282:80 | -277 | b: Optional[Dict[int, int]] = {}, -278 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), -279 | d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +280 | b: Optional[Dict[int, int]] = {}, +281 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +282 | d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), | ^^^^^ -280 | ): -281 | pass +283 | ): +284 | pass | help: Replace with `None`; initialize within function -276 | a: list[int] | None = [], -277 | b: Optional[Dict[int, int]] = {}, -278 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +279 | a: list[int] | None = [], +280 | b: Optional[Dict[int, int]] = {}, +281 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), - d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), -279 + d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = None, -280 | ): -281 | pass -282 | +282 + d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = None, +283 | ): +284 | pass +285 | note: This is an unsafe fix and may change runtime behavior B006 [*] Do not use mutable data structures for argument defaults - --> B006_B008.py:284:52 + --> B006_B008.py:287:52 | -284 | def single_line_func_wrong(value: dict[str, str] = {}): +287 | def single_line_func_wrong(value: dict[str, str] = {}): | ^^ -285 | """Docstring""" +288 | """Docstring""" | help: Replace with `None`; initialize within function -281 | pass -282 | -283 | - - def single_line_func_wrong(value: dict[str, str] = {}): -284 + def single_line_func_wrong(value: dict[str, str] = None): -285 | """Docstring""" +284 | pass +285 | 286 | -287 | + - def single_line_func_wrong(value: dict[str, str] = {}): +287 + def single_line_func_wrong(value: dict[str, str] = None): +288 | """Docstring""" +289 | +290 | note: This is an unsafe fix and may change runtime behavior B006 [*] Do not use mutable data structures for argument defaults - --> B006_B008.py:288:52 + --> B006_B008.py:291:52 | -288 | def single_line_func_wrong(value: dict[str, str] = {}): +291 | def single_line_func_wrong(value: dict[str, str] = {}): | ^^ -289 | """Docstring""" -290 | ... +292 | """Docstring""" +293 | ... | help: Replace with `None`; initialize within function -285 | """Docstring""" -286 | -287 | +288 | """Docstring""" +289 | +290 | - def single_line_func_wrong(value: dict[str, str] = {}): -288 + def single_line_func_wrong(value: dict[str, str] = None): -289 | """Docstring""" -290 | ... -291 | +291 + def single_line_func_wrong(value: dict[str, str] = None): +292 | """Docstring""" +293 | ... +294 | note: This is an unsafe fix and may change runtime behavior B006 [*] Do not use mutable data structures for argument defaults - --> B006_B008.py:293:52 + --> B006_B008.py:296:52 | -293 | def single_line_func_wrong(value: dict[str, str] = {}): +296 | def single_line_func_wrong(value: dict[str, str] = {}): | ^^ -294 | """Docstring"""; ... +297 | """Docstring"""; ... | help: Replace with `None`; initialize within function -290 | ... -291 | -292 | - - def single_line_func_wrong(value: dict[str, str] = {}): -293 + def single_line_func_wrong(value: dict[str, str] = None): -294 | """Docstring"""; ... +293 | ... +294 | 295 | -296 | + - def single_line_func_wrong(value: dict[str, str] = {}): +296 + def single_line_func_wrong(value: dict[str, str] = None): +297 | """Docstring"""; ... +298 | +299 | note: This is an unsafe fix and may change runtime behavior B006 [*] Do not use mutable data structures for argument defaults - --> B006_B008.py:297:52 + --> B006_B008.py:300:52 | -297 | def single_line_func_wrong(value: dict[str, str] = {}): +300 | def single_line_func_wrong(value: dict[str, str] = {}): | ^^ -298 | """Docstring"""; \ -299 | ... +301 | """Docstring"""; \ +302 | ... | help: Replace with `None`; initialize within function -294 | """Docstring"""; ... -295 | -296 | +297 | """Docstring"""; ... +298 | +299 | - def single_line_func_wrong(value: dict[str, str] = {}): -297 + def single_line_func_wrong(value: dict[str, str] = None): -298 | """Docstring"""; \ -299 | ... -300 | +300 + def single_line_func_wrong(value: dict[str, str] = None): +301 | """Docstring"""; \ +302 | ... +303 | note: This is an unsafe fix and may change runtime behavior B006 [*] Do not use mutable data structures for argument defaults - --> B006_B008.py:302:52 + --> B006_B008.py:305:52 | -302 | def single_line_func_wrong(value: dict[str, str] = { +305 | def single_line_func_wrong(value: dict[str, str] = { | ____________________________________________________^ -303 | | # This is a comment -304 | | }): +306 | | # This is a comment +307 | | }): | |_^ -305 | """Docstring""" +308 | """Docstring""" | help: Replace with `None`; initialize within function -299 | ... -300 | -301 | +302 | ... +303 | +304 | - def single_line_func_wrong(value: dict[str, str] = { - # This is a comment - }): -302 + def single_line_func_wrong(value: dict[str, str] = None): -303 | """Docstring""" -304 | -305 | +305 + def single_line_func_wrong(value: dict[str, str] = None): +306 | """Docstring""" +307 | +308 | note: This is an unsafe fix and may change runtime behavior B006 Do not use mutable data structures for argument defaults - --> B006_B008.py:308:52 + --> B006_B008.py:311:52 | -308 | def single_line_func_wrong(value: dict[str, str] = {}) \ +311 | def single_line_func_wrong(value: dict[str, str] = {}) \ | ^^ -309 | : \ -310 | """Docstring""" +312 | : \ +313 | """Docstring""" | help: Replace with `None`; initialize within function B006 [*] Do not use mutable data structures for argument defaults - --> B006_B008.py:313:52 + --> B006_B008.py:316:52 | -313 | def single_line_func_wrong(value: dict[str, str] = {}): +316 | def single_line_func_wrong(value: dict[str, str] = {}): | ^^ -314 | """Docstring without newline""" +317 | """Docstring without newline""" | help: Replace with `None`; initialize within function -310 | """Docstring""" -311 | -312 | +313 | """Docstring""" +314 | +315 | - def single_line_func_wrong(value: dict[str, str] = {}): -313 + def single_line_func_wrong(value: dict[str, str] = None): -314 | """Docstring without newline""" +316 + def single_line_func_wrong(value: dict[str, str] = None): +317 | """Docstring without newline""" note: This is an unsafe fix and may change runtime behavior diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B008_B006_B008.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B008_B006_B008.py.snap index 49da306103c93..edaaadb94439b 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B008_B006_B008.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B008_B006_B008.py.snap @@ -53,39 +53,39 @@ B008 Do not perform function call in argument defaults; instead, perform the cal | B008 Do not perform function call `dt.datetime.now` in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable - --> B006_B008.py:239:31 + --> B006_B008.py:242:31 | -237 | # B006 and B008 -238 | # We should handle arbitrary nesting of these B008. -239 | def nested_combo(a=[float(3), dt.datetime.now()]): +240 | # B006 and B008 +241 | # We should handle arbitrary nesting of these B008. +242 | def nested_combo(a=[float(3), dt.datetime.now()]): | ^^^^^^^^^^^^^^^^^ -240 | pass +243 | pass | B008 Do not perform function call `map` in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable - --> B006_B008.py:245:22 + --> B006_B008.py:248:22 | -243 | # Don't flag nested B006 since we can't guarantee that -244 | # it isn't made mutable by the outer operation. -245 | def no_nested_b006(a=map(lambda s: s.upper(), ["a", "b", "c"])): +246 | # Don't flag nested B006 since we can't guarantee that +247 | # it isn't made mutable by the outer operation. +248 | def no_nested_b006(a=map(lambda s: s.upper(), ["a", "b", "c"])): | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -246 | pass +249 | pass | B008 Do not perform function call `random.randint` in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable - --> B006_B008.py:250:19 + --> B006_B008.py:253:19 | -249 | # B008-ception. -250 | def nested_b008(a=random.randint(0, dt.datetime.now().year)): +252 | # B008-ception. +253 | def nested_b008(a=random.randint(0, dt.datetime.now().year)): | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -251 | pass +254 | pass | B008 Do not perform function call `dt.datetime.now` in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable - --> B006_B008.py:250:37 + --> B006_B008.py:253:37 | -249 | # B008-ception. -250 | def nested_b008(a=random.randint(0, dt.datetime.now().year)): +252 | # B008-ception. +253 | def nested_b008(a=random.randint(0, dt.datetime.now().year)): | ^^^^^^^^^^^^^^^^^ -251 | pass +254 | pass | diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__preview__B006_B006_B008.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__preview__B006_B006_B008.py.snap index e3f42e2da7a8b..f4113617b5279 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__preview__B006_B006_B008.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__preview__B006_B006_B008.py.snap @@ -236,227 +236,227 @@ help: Replace with `None`; initialize within function note: This is an unsafe fix and may change runtime behavior B006 [*] Do not use mutable data structures for argument defaults - --> B006_B008.py:239:20 + --> B006_B008.py:242:20 | -237 | # B006 and B008 -238 | # We should handle arbitrary nesting of these B008. -239 | def nested_combo(a=[float(3), dt.datetime.now()]): +240 | # B006 and B008 +241 | # We should handle arbitrary nesting of these B008. +242 | def nested_combo(a=[float(3), dt.datetime.now()]): | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -240 | pass +243 | pass | help: Replace with `None`; initialize within function -236 | -237 | # B006 and B008 -238 | # We should handle arbitrary nesting of these B008. +239 | +240 | # B006 and B008 +241 | # We should handle arbitrary nesting of these B008. - def nested_combo(a=[float(3), dt.datetime.now()]): -239 + def nested_combo(a=None): -240 | pass -241 | -242 | +242 + def nested_combo(a=None): +243 | pass +244 | +245 | note: This is an unsafe fix and may change runtime behavior B006 [*] Do not use mutable data structures for argument defaults - --> B006_B008.py:276:27 + --> B006_B008.py:279:27 | -275 | def mutable_annotations( -276 | a: list[int] | None = [], +278 | def mutable_annotations( +279 | a: list[int] | None = [], | ^^ -277 | b: Optional[Dict[int, int]] = {}, -278 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +280 | b: Optional[Dict[int, int]] = {}, +281 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), | help: Replace with `None`; initialize within function -273 | -274 | -275 | def mutable_annotations( +276 | +277 | +278 | def mutable_annotations( - a: list[int] | None = [], -276 + a: list[int] | None = None, -277 | b: Optional[Dict[int, int]] = {}, -278 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), -279 | d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +279 + a: list[int] | None = None, +280 | b: Optional[Dict[int, int]] = {}, +281 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +282 | d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), note: This is an unsafe fix and may change runtime behavior B006 [*] Do not use mutable data structures for argument defaults - --> B006_B008.py:277:35 + --> B006_B008.py:280:35 | -275 | def mutable_annotations( -276 | a: list[int] | None = [], -277 | b: Optional[Dict[int, int]] = {}, +278 | def mutable_annotations( +279 | a: list[int] | None = [], +280 | b: Optional[Dict[int, int]] = {}, | ^^ -278 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), -279 | d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +281 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +282 | d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), | help: Replace with `None`; initialize within function -274 | -275 | def mutable_annotations( -276 | a: list[int] | None = [], +277 | +278 | def mutable_annotations( +279 | a: list[int] | None = [], - b: Optional[Dict[int, int]] = {}, -277 + b: Optional[Dict[int, int]] = None, -278 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), -279 | d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), -280 | ): +280 + b: Optional[Dict[int, int]] = None, +281 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +282 | d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +283 | ): note: This is an unsafe fix and may change runtime behavior B006 [*] Do not use mutable data structures for argument defaults - --> B006_B008.py:278:62 + --> B006_B008.py:281:62 | -276 | a: list[int] | None = [], -277 | b: Optional[Dict[int, int]] = {}, -278 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +279 | a: list[int] | None = [], +280 | b: Optional[Dict[int, int]] = {}, +281 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), | ^^^^^ -279 | d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), -280 | ): +282 | d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +283 | ): | help: Replace with `None`; initialize within function -275 | def mutable_annotations( -276 | a: list[int] | None = [], -277 | b: Optional[Dict[int, int]] = {}, +278 | def mutable_annotations( +279 | a: list[int] | None = [], +280 | b: Optional[Dict[int, int]] = {}, - c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), -278 + c: Annotated[Union[Set[str], abc.Sized], "annotation"] = None, -279 | d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), -280 | ): -281 | pass +281 + c: Annotated[Union[Set[str], abc.Sized], "annotation"] = None, +282 | d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +283 | ): +284 | pass note: This is an unsafe fix and may change runtime behavior B006 [*] Do not use mutable data structures for argument defaults - --> B006_B008.py:279:80 + --> B006_B008.py:282:80 | -277 | b: Optional[Dict[int, int]] = {}, -278 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), -279 | d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +280 | b: Optional[Dict[int, int]] = {}, +281 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +282 | d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), | ^^^^^ -280 | ): -281 | pass +283 | ): +284 | pass | help: Replace with `None`; initialize within function -276 | a: list[int] | None = [], -277 | b: Optional[Dict[int, int]] = {}, -278 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), +279 | a: list[int] | None = [], +280 | b: Optional[Dict[int, int]] = {}, +281 | c: Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), - d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = set(), -279 + d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = None, -280 | ): -281 | pass -282 | +282 + d: typing_extensions.Annotated[Union[Set[str], abc.Sized], "annotation"] = None, +283 | ): +284 | pass +285 | note: This is an unsafe fix and may change runtime behavior B006 [*] Do not use mutable data structures for argument defaults - --> B006_B008.py:284:52 + --> B006_B008.py:287:52 | -284 | def single_line_func_wrong(value: dict[str, str] = {}): +287 | def single_line_func_wrong(value: dict[str, str] = {}): | ^^ -285 | """Docstring""" +288 | """Docstring""" | help: Replace with `None`; initialize within function -281 | pass -282 | -283 | - - def single_line_func_wrong(value: dict[str, str] = {}): -284 + def single_line_func_wrong(value: dict[str, str] = None): -285 | """Docstring""" +284 | pass +285 | 286 | -287 | + - def single_line_func_wrong(value: dict[str, str] = {}): +287 + def single_line_func_wrong(value: dict[str, str] = None): +288 | """Docstring""" +289 | +290 | note: This is an unsafe fix and may change runtime behavior B006 [*] Do not use mutable data structures for argument defaults - --> B006_B008.py:288:52 + --> B006_B008.py:291:52 | -288 | def single_line_func_wrong(value: dict[str, str] = {}): +291 | def single_line_func_wrong(value: dict[str, str] = {}): | ^^ -289 | """Docstring""" -290 | ... +292 | """Docstring""" +293 | ... | help: Replace with `None`; initialize within function -285 | """Docstring""" -286 | -287 | +288 | """Docstring""" +289 | +290 | - def single_line_func_wrong(value: dict[str, str] = {}): -288 + def single_line_func_wrong(value: dict[str, str] = None): -289 | """Docstring""" -290 | ... -291 | +291 + def single_line_func_wrong(value: dict[str, str] = None): +292 | """Docstring""" +293 | ... +294 | note: This is an unsafe fix and may change runtime behavior B006 [*] Do not use mutable data structures for argument defaults - --> B006_B008.py:293:52 + --> B006_B008.py:296:52 | -293 | def single_line_func_wrong(value: dict[str, str] = {}): +296 | def single_line_func_wrong(value: dict[str, str] = {}): | ^^ -294 | """Docstring"""; ... +297 | """Docstring"""; ... | help: Replace with `None`; initialize within function -290 | ... -291 | -292 | - - def single_line_func_wrong(value: dict[str, str] = {}): -293 + def single_line_func_wrong(value: dict[str, str] = None): -294 | """Docstring"""; ... +293 | ... +294 | 295 | -296 | + - def single_line_func_wrong(value: dict[str, str] = {}): +296 + def single_line_func_wrong(value: dict[str, str] = None): +297 | """Docstring"""; ... +298 | +299 | note: This is an unsafe fix and may change runtime behavior B006 [*] Do not use mutable data structures for argument defaults - --> B006_B008.py:297:52 + --> B006_B008.py:300:52 | -297 | def single_line_func_wrong(value: dict[str, str] = {}): +300 | def single_line_func_wrong(value: dict[str, str] = {}): | ^^ -298 | """Docstring"""; \ -299 | ... +301 | """Docstring"""; \ +302 | ... | help: Replace with `None`; initialize within function -294 | """Docstring"""; ... -295 | -296 | +297 | """Docstring"""; ... +298 | +299 | - def single_line_func_wrong(value: dict[str, str] = {}): -297 + def single_line_func_wrong(value: dict[str, str] = None): -298 | """Docstring"""; \ -299 | ... -300 | +300 + def single_line_func_wrong(value: dict[str, str] = None): +301 | """Docstring"""; \ +302 | ... +303 | note: This is an unsafe fix and may change runtime behavior B006 [*] Do not use mutable data structures for argument defaults - --> B006_B008.py:302:52 + --> B006_B008.py:305:52 | -302 | def single_line_func_wrong(value: dict[str, str] = { +305 | def single_line_func_wrong(value: dict[str, str] = { | ____________________________________________________^ -303 | | # This is a comment -304 | | }): +306 | | # This is a comment +307 | | }): | |_^ -305 | """Docstring""" +308 | """Docstring""" | help: Replace with `None`; initialize within function -299 | ... -300 | -301 | +302 | ... +303 | +304 | - def single_line_func_wrong(value: dict[str, str] = { - # This is a comment - }): -302 + def single_line_func_wrong(value: dict[str, str] = None): -303 | """Docstring""" -304 | -305 | +305 + def single_line_func_wrong(value: dict[str, str] = None): +306 | """Docstring""" +307 | +308 | note: This is an unsafe fix and may change runtime behavior B006 Do not use mutable data structures for argument defaults - --> B006_B008.py:308:52 + --> B006_B008.py:311:52 | -308 | def single_line_func_wrong(value: dict[str, str] = {}) \ +311 | def single_line_func_wrong(value: dict[str, str] = {}) \ | ^^ -309 | : \ -310 | """Docstring""" +312 | : \ +313 | """Docstring""" | help: Replace with `None`; initialize within function B006 [*] Do not use mutable data structures for argument defaults - --> B006_B008.py:313:52 + --> B006_B008.py:316:52 | -313 | def single_line_func_wrong(value: dict[str, str] = {}): +316 | def single_line_func_wrong(value: dict[str, str] = {}): | ^^ -314 | """Docstring without newline""" +317 | """Docstring without newline""" | help: Replace with `None`; initialize within function -310 | """Docstring""" -311 | -312 | +313 | """Docstring""" +314 | +315 | - def single_line_func_wrong(value: dict[str, str] = {}): -313 + def single_line_func_wrong(value: dict[str, str] = None): -314 | """Docstring without newline""" +316 + def single_line_func_wrong(value: dict[str, str] = None): +317 | """Docstring without newline""" note: This is an unsafe fix and may change runtime behavior diff --git a/crates/ruff_python_stdlib/src/typing.rs b/crates/ruff_python_stdlib/src/typing.rs index 63d7ccf32ed77..7f4abd367cf17 100644 --- a/crates/ruff_python_stdlib/src/typing.rs +++ b/crates/ruff_python_stdlib/src/typing.rs @@ -326,7 +326,15 @@ pub fn is_immutable_return_type(qualified_name: &[&str]) -> bool { | ["re", "compile"] | [ "", - "bool" | "bytes" | "complex" | "float" | "frozenset" | "int" | "str" | "tuple" + "bool" + | "bytes" + | "complex" + | "float" + | "frozenset" + | "int" + | "str" + | "tuple" + | "slice" ] ) } From eac8a90cc4ca0cc7b85a2129ec5c7bc067e6d7d6 Mon Sep 17 00:00:00 2001 From: Rasmus Nygren Date: Wed, 3 Dec 2025 21:58:01 +0100 Subject: [PATCH 3/6] [ty] Add autocomplete suggestions for function arguments This adds autocomplete suggestions for function arguments. For example, `okay` in: ```python def foo(okay=None): foo(o ``` This also ensures that we don't suggest a keyword argument if it has already been used. Closes astral-sh/issues#1550 --- crates/ty_ide/src/completion.rs | 254 +++++++++++++++++- crates/ty_ide/src/signature_help.rs | 10 + crates/ty_python_semantic/src/types.rs | 1 + .../src/types/ide_support.rs | 11 +- .../src/types/signatures.rs | 2 +- 5 files changed, 268 insertions(+), 10 deletions(-) diff --git a/crates/ty_ide/src/completion.rs b/crates/ty_ide/src/completion.rs index 70505ac4c85d9..445dccb6155ce 100644 --- a/crates/ty_ide/src/completion.rs +++ b/crates/ty_ide/src/completion.rs @@ -9,6 +9,7 @@ use ruff_python_ast::token::{Token, TokenAt, TokenKind, Tokens}; use ruff_python_ast::{self as ast, AnyNodeRef}; use ruff_python_codegen::Stylist; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; +use rustc_hash::FxHashSet; use ty_python_semantic::types::UnionType; use ty_python_semantic::{ Completion as SemanticCompletion, KnownModule, ModuleName, NameKind, SemanticModel, @@ -20,7 +21,7 @@ use crate::find_node::covering_node; use crate::goto::Definitions; use crate::importer::{ImportRequest, Importer}; use crate::symbols::QueryPattern; -use crate::{Db, all_symbols}; +use crate::{Db, all_symbols, signature_help}; /// A collection of completions built up from various sources. #[derive(Clone)] @@ -436,6 +437,10 @@ pub fn completion<'db>( ); } } + + if let Some(arg_completions) = detect_function_arg_completions(db, file, &parsed, offset) { + completions.extend(arg_completions); + } } if is_raising_exception(tokens) { @@ -451,10 +456,89 @@ pub fn completion<'db>( !ty.is_notimplemented(db) }); } - completions.into_completions() } +/// Detect and construct completions for unset function arguments. +/// +/// Suggestions are only provided if the cursor is currently inside a +/// function call and the function arguments have not 1) already been +/// set and 2) been defined as positional-only. +fn detect_function_arg_completions<'db>( + db: &'db dyn Db, + file: File, + parsed: &ParsedModuleRef, + offset: TextSize, +) -> Option>> { + let sig_help = signature_help(db, file, offset)?; + let set_function_args = detect_set_function_args(parsed, offset); + + let completions = sig_help + .signatures + .iter() + .flat_map(|sig| &sig.parameters) + .filter(|p| !p.is_positional_only && !set_function_args.contains(&p.name.as_str())) + .map(|p| { + let name = Name::new(&p.name); + let documentation = p + .documentation + .as_ref() + .map(|d| Docstring::new(d.to_owned())); + let insert = Some(format!("{name}=").into_boxed_str()); + Completion { + name, + qualified: None, + insert, + ty: None, + kind: Some(CompletionKind::Variable), + module_name: None, + import: None, + builtin: false, + is_type_check_only: false, + is_definitively_raisable: false, + documentation, + } + }) + .collect(); + Some(completions) +} + +/// Returns function arguments that have already been set. +/// +/// If `offset` is inside an arguments node, this returns +/// the list of argument names that are already set. +/// +/// For example, given: +/// +/// ```python +/// def abc(foo, bar, baz): ... +/// abc(foo=1, bar=2, b) +/// ``` +/// +/// the resulting value is `["foo", "bar"]` +/// +/// This is useful to be able to exclude autocomplete suggestions +/// for arguments that have already been set to some value. +/// +/// If the parent node is not an arguments node, the return value +/// is an empty Vec. +fn detect_set_function_args(parsed: &ParsedModuleRef, offset: TextSize) -> FxHashSet<&str> { + let range = TextRange::empty(offset); + covering_node(parsed.syntax().into(), range) + .parent() + .and_then(|node| match node { + ast::AnyNodeRef::Arguments(args) => Some(args), + _ => None, + }) + .map(|args| { + args.keywords + .iter() + .filter_map(|kw| kw.arg.as_ref().map(|ident| ident.id.as_str())) + .collect() + }) + .unwrap_or_default() +} + pub(crate) struct ImportEdit { pub label: String, pub edit: Edit, @@ -2386,10 +2470,11 @@ def frob(): ... ", ); - // FIXME: Should include `foo`. assert_snapshot!( builder.skip_keywords().skip_builtins().build().snapshot(), - @"", + @r" + foo + ", ); } @@ -2401,10 +2486,11 @@ def frob(): ... ", ); - // FIXME: Should include `foo`. assert_snapshot!( builder.skip_keywords().skip_builtins().build().snapshot(), - @"", + @r" + foo + ", ); } @@ -3039,7 +3125,6 @@ quux. "); } - // We don't yet take function parameters into account. #[test] fn call_prefix1() { let builder = completion_test_builder( @@ -3052,7 +3137,159 @@ bar(o ", ); - assert_snapshot!(builder.skip_keywords().skip_builtins().build().snapshot(), @"foo"); + assert_snapshot!( + builder.skip_keywords().skip_builtins().build().snapshot(), + @r" + foo + okay + " + ); + } + + #[test] + fn call_keyword_only_argument() { + let builder = completion_test_builder( + "\ +def bar(*, okay): ... + +foo = 1 + +bar(o +", + ); + + assert_snapshot!( + builder.skip_keywords().skip_builtins().build().snapshot(), + @r" + foo + okay + " + ); + } + + #[test] + fn call_multiple_keyword_arguments() { + let builder = completion_test_builder( + "\ +def foo(bar, baz, barbaz): ... + +foo(b +", + ); + + assert_snapshot!( + builder.skip_keywords().skip_builtins().build().snapshot(), + @r" + bar + barbaz + baz + " + ); + } + + #[test] + fn call_multiple_keyword_arguments_some_set() { + let builder = completion_test_builder( + "\ +def foo(bar, baz): ... + +foo(bar=1, b +", + ); + + assert_snapshot!( + builder.skip_keywords().skip_builtins().build().snapshot(), + @r" + baz + " + ); + } + + #[test] + fn call_arguments_multi_def() { + let builder = completion_test_builder( + "\ +def abc(okay, x): ... +def bar(not_okay, y): ... +def baz(foobarbaz, z): ... + +abc(o +", + ); + + assert_snapshot!( + builder.skip_keywords().skip_builtins().build().snapshot(), + @r" + okay + " + ); + } + + #[test] + fn call_arguments_cursor_middle() { + let builder = completion_test_builder( + "\ +def abc(okay, foo, bar, baz): ... + +abc(okay=1, ba baz=5 +", + ); + + assert_snapshot!( + builder.skip_keywords().skip_builtins().build().snapshot(), + @r" + bar + " + ); + } + + + + #[test] + fn call_positional_only_argument() { + // If the parameter is positional only we don't + // want to suggest it as specifying by name + // is not valid. + let builder = completion_test_builder( + "\ +def bar(okay, /): ... + +foo = 1 + +bar(o +", + ); + + assert_snapshot!( + builder.skip_keywords().skip_builtins().build().snapshot(), + @"foo" + ); + } + + #[test] + fn call_positional_only_keyword_only_argument_mix() { + // If the parameter is positional only we don't + // want to suggest it as specifying by name + // is not valid. + let builder = completion_test_builder( + "\ +def bar(not_okay, no, /, okay, *, okay_abc, okay_okay): ... + +foo = 1 + +bar(o +", + ); + + assert_snapshot!( + builder.skip_keywords().skip_builtins().build().snapshot(), + @r" + foo + okay + okay_abc + okay_okay + " + ); } #[test] @@ -3070,6 +3307,7 @@ bar( assert_snapshot!(builder.skip_keywords().skip_builtins().build().snapshot(), @r" bar foo + okay "); } diff --git a/crates/ty_ide/src/signature_help.rs b/crates/ty_ide/src/signature_help.rs index d79f298dd6abd..14b374e8989c9 100644 --- a/crates/ty_ide/src/signature_help.rs +++ b/crates/ty_ide/src/signature_help.rs @@ -17,6 +17,7 @@ use ruff_text_size::{Ranged, TextRange, TextSize}; use ty_python_semantic::ResolvedDefinition; use ty_python_semantic::SemanticModel; use ty_python_semantic::semantic_index::definition::Definition; +use ty_python_semantic::types::ParameterKind; use ty_python_semantic::types::ide_support::{ CallSignatureDetails, call_signature_details, find_active_signature_from_details, }; @@ -35,6 +36,8 @@ pub struct ParameterDetails { /// Documentation specific to the parameter, typically extracted from the /// function's docstring pub documentation: Option, + /// True if the parameter is positional-only. + pub is_positional_only: bool, } /// Information about a function signature @@ -200,6 +203,7 @@ fn create_signature_details_from_call_signature_details( &signature_label, documentation.as_ref(), &details.parameter_names, + &details.parameter_kinds, ); SignatureDetails { label: signature_label, @@ -223,6 +227,7 @@ fn create_parameters_from_offsets( signature_label: &str, docstring: Option<&Docstring>, parameter_names: &[String], + parameter_kinds: &[ParameterKind], ) -> Vec { // Extract parameter documentation from the function's docstring if available. let param_docs = if let Some(docstring) = docstring { @@ -245,11 +250,16 @@ fn create_parameters_from_offsets( // Get the parameter name for documentation lookup. let param_name = parameter_names.get(i).map(String::as_str).unwrap_or(""); + let is_positional_only = matches!( + parameter_kinds.get(i), + Some(ParameterKind::PositionalOnly { .. }) + ); ParameterDetails { name: param_name.to_string(), label, documentation: param_docs.get(param_name).cloned(), + is_positional_only, } }) .collect() diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index 23f7a53f79ac2..dfc932bc662be 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -30,6 +30,7 @@ pub(crate) use self::infer::{ TypeContext, infer_deferred_types, infer_definition_types, infer_expression_type, infer_expression_types, infer_scope_types, static_expression_truthiness, }; +pub use self::signatures::ParameterKind; pub(crate) use self::signatures::{CallableSignature, Signature}; pub(crate) use self::subclass_of::{SubclassOfInner, SubclassOfType}; pub use crate::diagnostic::add_inferred_python_version_hint_to_diagnostic; diff --git a/crates/ty_python_semantic/src/types/ide_support.rs b/crates/ty_python_semantic/src/types/ide_support.rs index a74cc82f7e3a5..974500b75af40 100644 --- a/crates/ty_python_semantic/src/types/ide_support.rs +++ b/crates/ty_python_semantic/src/types/ide_support.rs @@ -6,7 +6,7 @@ use crate::semantic_index::definition::Definition; use crate::semantic_index::definition::DefinitionKind; use crate::semantic_index::{attribute_scopes, global_scope, semantic_index, use_def_map}; use crate::types::call::{CallArguments, MatchedArgument}; -use crate::types::signatures::Signature; +use crate::types::signatures::{ParameterKind, Signature}; use crate::types::{CallDunderError, UnionType}; use crate::types::{CallableTypes, ClassBase, KnownClass, Type, TypeContext}; use crate::{Db, DisplaySettings, HasType, SemanticModel}; @@ -459,6 +459,9 @@ pub struct CallSignatureDetails<'db> { /// This provides easy access to parameter names for documentation lookup. pub parameter_names: Vec, + /// Parameter kinds, useful to determine correct autocomplete suggestions. + pub parameter_kinds: Vec>, + /// The definition where this callable was originally defined (useful for /// extracting docstrings). pub definition: Option>, @@ -517,6 +520,11 @@ pub fn call_signature_details<'db>( let display_details = signature.display(model.db()).to_string_parts(); let parameter_label_offsets = display_details.parameter_ranges; let parameter_names = display_details.parameter_names; + let parameter_kinds = signature + .parameters() + .iter() + .map(|param| param.kind().clone()) + .collect(); CallSignatureDetails { definition: signature.definition(), @@ -524,6 +532,7 @@ pub fn call_signature_details<'db>( label: display_details.label, parameter_label_offsets, parameter_names, + parameter_kinds, argument_to_parameter_mapping, } }) diff --git a/crates/ty_python_semantic/src/types/signatures.rs b/crates/ty_python_semantic/src/types/signatures.rs index ffc224c8d5c93..f5406798a5dc9 100644 --- a/crates/ty_python_semantic/src/types/signatures.rs +++ b/crates/ty_python_semantic/src/types/signatures.rs @@ -2292,7 +2292,7 @@ impl<'db> Parameter<'db> { } #[derive(Clone, Debug, PartialEq, Eq, Hash, salsa::Update, get_size2::GetSize)] -pub(crate) enum ParameterKind<'db> { +pub enum ParameterKind<'db> { /// Positional-only parameter, e.g. `def f(x, /): ...` PositionalOnly { /// Parameter name. From e548ce1ca90edaab7df571ad5e8f059fb8f2e312 Mon Sep 17 00:00:00 2001 From: Rasmus Nygren Date: Fri, 5 Dec 2025 19:37:36 +0100 Subject: [PATCH 4/6] [ty] Enrich function argument auto-complete suggestions with annotated types --- crates/ty_ide/src/completion.rs | 4 +-- crates/ty_ide/src/signature_help.rs | 32 +++++++++++-------- .../src/types/ide_support.rs | 15 ++++++--- 3 files changed, 30 insertions(+), 21 deletions(-) diff --git a/crates/ty_ide/src/completion.rs b/crates/ty_ide/src/completion.rs index 445dccb6155ce..3fc0f95235b6f 100644 --- a/crates/ty_ide/src/completion.rs +++ b/crates/ty_ide/src/completion.rs @@ -489,7 +489,7 @@ fn detect_function_arg_completions<'db>( name, qualified: None, insert, - ty: None, + ty: p.ty, kind: Some(CompletionKind::Variable), module_name: None, import: None, @@ -3243,8 +3243,6 @@ abc(okay=1, ba baz=5 ); } - - #[test] fn call_positional_only_argument() { // If the parameter is positional only we don't diff --git a/crates/ty_ide/src/signature_help.rs b/crates/ty_ide/src/signature_help.rs index 14b374e8989c9..a4746f1563373 100644 --- a/crates/ty_ide/src/signature_help.rs +++ b/crates/ty_ide/src/signature_help.rs @@ -17,10 +17,10 @@ use ruff_text_size::{Ranged, TextRange, TextSize}; use ty_python_semantic::ResolvedDefinition; use ty_python_semantic::SemanticModel; use ty_python_semantic::semantic_index::definition::Definition; -use ty_python_semantic::types::ParameterKind; use ty_python_semantic::types::ide_support::{ CallSignatureDetails, call_signature_details, find_active_signature_from_details, }; +use ty_python_semantic::types::{ParameterKind, Type}; // TODO: We may want to add special-case handling for calls to constructors // so the class docstring is used in place of (or inaddition to) any docstring @@ -28,11 +28,13 @@ use ty_python_semantic::types::ide_support::{ /// Information about a function parameter #[derive(Debug, Clone, PartialEq, Eq)] -pub struct ParameterDetails { +pub struct ParameterDetails<'db> { /// The parameter name (e.g., "param1") pub name: String, /// The parameter label in the signature (e.g., "param1: str") pub label: String, + /// The annotated type of the parameter, if any + pub ty: Option>, /// Documentation specific to the parameter, typically extracted from the /// function's docstring pub documentation: Option, @@ -42,13 +44,13 @@ pub struct ParameterDetails { /// Information about a function signature #[derive(Debug, Clone, PartialEq, Eq)] -pub struct SignatureDetails { +pub struct SignatureDetails<'db> { /// Text representation of the full signature (including input parameters and return type). pub label: String, /// Documentation for the signature, typically from the function's docstring. pub documentation: Option, /// Information about each of the parameters in left-to-right order. - pub parameters: Vec, + pub parameters: Vec>, /// Index of the parameter that corresponds to the argument where the /// user's cursor is currently positioned. pub active_parameter: Option, @@ -56,18 +58,18 @@ pub struct SignatureDetails { /// Signature help information for function calls #[derive(Debug, Clone, PartialEq, Eq)] -pub struct SignatureHelpInfo { +pub struct SignatureHelpInfo<'db> { /// Information about each of the signatures for the function call. We /// need to handle multiple because of unions, overloads, and composite /// calls like constructors (which invoke both __new__ and __init__). - pub signatures: Vec, + pub signatures: Vec>, /// Index of the "active signature" which is the first signature where /// all arguments that are currently present in the code map to parameters. pub active_signature: Option, } /// Signature help information for function calls at the given position -pub fn signature_help(db: &dyn Db, file: File, offset: TextSize) -> Option { +pub fn signature_help(db: &dyn Db, file: File, offset: TextSize) -> Option> { let parsed = parsed_module(db, file).load(db); // Get the call expression at the given position. @@ -169,11 +171,11 @@ fn get_argument_index(call_expr: &ast::ExprCall, offset: TextSize) -> usize { } /// Create signature details from `CallSignatureDetails`. -fn create_signature_details_from_call_signature_details( +fn create_signature_details_from_call_signature_details<'db>( db: &dyn crate::Db, - details: &CallSignatureDetails, + details: &CallSignatureDetails<'db>, current_arg_index: usize, -) -> SignatureDetails { +) -> SignatureDetails<'db> { let signature_label = details.label.clone(); let documentation = get_callable_documentation(db, details.definition); @@ -204,6 +206,7 @@ fn create_signature_details_from_call_signature_details( documentation.as_ref(), &details.parameter_names, &details.parameter_kinds, + &details.parameter_types, ); SignatureDetails { label: signature_label, @@ -222,13 +225,14 @@ fn get_callable_documentation( } /// Create `ParameterDetails` objects from parameter label offsets. -fn create_parameters_from_offsets( +fn create_parameters_from_offsets<'db>( parameter_offsets: &[TextRange], signature_label: &str, docstring: Option<&Docstring>, parameter_names: &[String], parameter_kinds: &[ParameterKind], -) -> Vec { + parameter_types: &[Option>], +) -> Vec> { // Extract parameter documentation from the function's docstring if available. let param_docs = if let Some(docstring) = docstring { docstring.parameter_documentation() @@ -254,10 +258,12 @@ fn create_parameters_from_offsets( parameter_kinds.get(i), Some(ParameterKind::PositionalOnly { .. }) ); + let ty = parameter_types.get(i).copied().flatten(); ParameterDetails { name: param_name.to_string(), label, + ty, documentation: param_docs.get(param_name).cloned(), is_positional_only, } @@ -1183,7 +1189,7 @@ def ab(a: int, *, c: int): } impl CursorTest { - fn signature_help(&self) -> Option { + fn signature_help(&self) -> Option> { crate::signature_help::signature_help(&self.db, self.cursor.file, self.cursor.offset) } diff --git a/crates/ty_python_semantic/src/types/ide_support.rs b/crates/ty_python_semantic/src/types/ide_support.rs index 974500b75af40..d32eba2ace43c 100644 --- a/crates/ty_python_semantic/src/types/ide_support.rs +++ b/crates/ty_python_semantic/src/types/ide_support.rs @@ -462,6 +462,9 @@ pub struct CallSignatureDetails<'db> { /// Parameter kinds, useful to determine correct autocomplete suggestions. pub parameter_kinds: Vec>, + /// Parameter kinds, useful to determine correct autocomplete suggestions. + pub parameter_types: Vec>>, + /// The definition where this callable was originally defined (useful for /// extracting docstrings). pub definition: Option>, @@ -520,11 +523,12 @@ pub fn call_signature_details<'db>( let display_details = signature.display(model.db()).to_string_parts(); let parameter_label_offsets = display_details.parameter_ranges; let parameter_names = display_details.parameter_names; - let parameter_kinds = signature - .parameters() - .iter() - .map(|param| param.kind().clone()) - .collect(); + let (parameter_kinds, parameter_types): (Vec, Vec>) = + signature + .parameters() + .iter() + .map(|param| (param.kind().clone(), param.annotated_type())) + .unzip(); CallSignatureDetails { definition: signature.definition(), @@ -533,6 +537,7 @@ pub fn call_signature_details<'db>( parameter_label_offsets, parameter_names, parameter_kinds, + parameter_types, argument_to_parameter_mapping, } }) From 8ea18966cfcb6c6f634afdd0c9503ac2ef1b97b3 Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Mon, 8 Dec 2025 17:44:17 -0500 Subject: [PATCH 5/6] [ty] followup: add-import action for `reveal_type` too (#21668) --- crates/ty_ide/src/code_action.rs | 10 +- crates/ty_server/tests/e2e/code_actions.rs | 39 +++++++- ...ns__code_action_undefined_reveal_type.snap | 98 +++++++++++++++++++ 3 files changed, 142 insertions(+), 5 deletions(-) create mode 100644 crates/ty_server/tests/e2e/snapshots/e2e__code_actions__code_action_undefined_reveal_type.snap diff --git a/crates/ty_ide/src/code_action.rs b/crates/ty_ide/src/code_action.rs index 1a02389735153..8826dfce061fe 100644 --- a/crates/ty_ide/src/code_action.rs +++ b/crates/ty_ide/src/code_action.rs @@ -5,7 +5,8 @@ use ruff_diagnostics::Edit; use ruff_text_size::TextRange; use ty_project::Db; use ty_python_semantic::create_suppression_fix; -use ty_python_semantic::types::UNRESOLVED_REFERENCE; +use ty_python_semantic::lint::LintId; +use ty_python_semantic::types::{UNDEFINED_REVEAL, UNRESOLVED_REFERENCE}; /// A `QuickFix` Code Action #[derive(Debug, Clone)] @@ -28,12 +29,17 @@ pub fn code_actions( let mut actions = Vec::new(); - if lint_id.name() == UNRESOLVED_REFERENCE.name() + // Suggest imports for unresolved references (often ideal) + // TODO: suggest qualifying with an already imported symbol + let is_unresolved_reference = + lint_id == LintId::of(&UNRESOLVED_REFERENCE) || lint_id == LintId::of(&UNDEFINED_REVEAL); + if is_unresolved_reference && let Some(import_quick_fix) = create_import_symbol_quick_fix(db, file, diagnostic_range) { actions.extend(import_quick_fix); } + // Suggest just suppressing the lint (always a valid option, but never ideal) actions.push(QuickFix { title: format!("Ignore '{}' for this line", lint_id.name()), edits: create_suppression_fix(db, file, lint_id, diagnostic_range).into_edits(), diff --git a/crates/ty_server/tests/e2e/code_actions.rs b/crates/ty_server/tests/e2e/code_actions.rs index d60d9ad302566..d3d50c5fb9592 100644 --- a/crates/ty_server/tests/e2e/code_actions.rs +++ b/crates/ty_server/tests/e2e/code_actions.rs @@ -132,11 +132,44 @@ x: Literal[1] = 1 "; let ty_toml = SystemPath::new("ty.toml"); - let ty_toml_content = "\ -[rules] -unused-ignore-comment = \"warn\" + let ty_toml_content = ""; + + let mut server = TestServerBuilder::new()? + .with_workspace(workspace_root, None)? + .with_file(ty_toml, ty_toml_content)? + .with_file(foo, foo_content)? + .enable_pull_diagnostics(true) + .build() + .wait_until_workspaces_are_initialized(); + + server.open_text_document(foo, foo_content, 1); + + // Wait for diagnostics to be computed. + let diagnostics = server.document_diagnostic_request(foo, None); + let range = full_range(foo_content); + let code_action_params = code_actions_at(&server, diagnostics, foo, range); + + // Get code actions + let code_action_id = server.send_request::(code_action_params); + let code_actions = server.await_response::(&code_action_id); + + insta::assert_json_snapshot!(code_actions); + + Ok(()) +} + +// `Literal` is available from two places so we should suggest two possible imports +#[test] +fn code_action_undefined_reveal_type() -> Result<()> { + let workspace_root = SystemPath::new("src"); + let foo = SystemPath::new("src/foo.py"); + let foo_content = "\ +reveal_type(1) "; + let ty_toml = SystemPath::new("ty.toml"); + let ty_toml_content = ""; + let mut server = TestServerBuilder::new()? .with_workspace(workspace_root, None)? .with_file(ty_toml, ty_toml_content)? diff --git a/crates/ty_server/tests/e2e/snapshots/e2e__code_actions__code_action_undefined_reveal_type.snap b/crates/ty_server/tests/e2e/snapshots/e2e__code_actions__code_action_undefined_reveal_type.snap new file mode 100644 index 0000000000000..aace2bc04200e --- /dev/null +++ b/crates/ty_server/tests/e2e/snapshots/e2e__code_actions__code_action_undefined_reveal_type.snap @@ -0,0 +1,98 @@ +--- +source: crates/ty_server/tests/e2e/code_actions.rs +expression: code_actions +--- +[ + { + "title": "import typing.reveal_type", + "kind": "quickfix", + "diagnostics": [ + { + "range": { + "start": { + "line": 0, + "character": 0 + }, + "end": { + "line": 0, + "character": 11 + } + }, + "severity": 2, + "code": "undefined-reveal", + "codeDescription": { + "href": "https://ty.dev/rules#undefined-reveal" + }, + "source": "ty", + "message": "`reveal_type` used without importing it", + "relatedInformation": [] + } + ], + "edit": { + "changes": { + "file:///src/foo.py": [ + { + "range": { + "start": { + "line": 0, + "character": 0 + }, + "end": { + "line": 0, + "character": 0 + } + }, + "newText": "from typing import reveal_type\n" + } + ] + } + }, + "isPreferred": true + }, + { + "title": "Ignore 'undefined-reveal' for this line", + "kind": "quickfix", + "diagnostics": [ + { + "range": { + "start": { + "line": 0, + "character": 0 + }, + "end": { + "line": 0, + "character": 11 + } + }, + "severity": 2, + "code": "undefined-reveal", + "codeDescription": { + "href": "https://ty.dev/rules#undefined-reveal" + }, + "source": "ty", + "message": "`reveal_type` used without importing it", + "relatedInformation": [] + } + ], + "edit": { + "changes": { + "file:///src/foo.py": [ + { + "range": { + "start": { + "line": 0, + "character": 14 + }, + "end": { + "line": 0, + "character": 14 + } + }, + "newText": " # ty:ignore[undefined-reveal]" + } + ] + } + }, + "isPreferred": false + } +] From 4e67a219bbb7b66db315a010e44e67fec895be04 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Mon, 8 Dec 2025 16:11:59 -0800 Subject: [PATCH 6/6] apply range suppressions to filter diagnostics (#21623) Builds on range suppressions from https://github.com/astral-sh/ruff/pull/21441 Filters diagnostics based on parsed valid range suppressions. Issue: #3711 --- crates/ruff/tests/cli/lint.rs | 166 +++++++++++++++++ .../test/fixtures/ruff/suppressions.py | 56 ++++++ crates/ruff_linter/src/checkers/noqa.rs | 14 +- crates/ruff_linter/src/linter.rs | 19 ++ crates/ruff_linter/src/noqa.rs | 40 ++++- crates/ruff_linter/src/preview.rs | 5 + crates/ruff_linter/src/rules/pyflakes/mod.rs | 4 + crates/ruff_linter/src/rules/ruff/mod.rs | 19 ++ ...ules__ruff__tests__range_suppressions.snap | 168 ++++++++++++++++++ crates/ruff_linter/src/settings/mod.rs | 6 + crates/ruff_linter/src/suppression.rs | 61 ++++++- crates/ruff_linter/src/test.rs | 6 + crates/ruff_server/src/lint.rs | 7 + crates/ruff_wasm/src/lib.rs | 5 + 14 files changed, 564 insertions(+), 12 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__range_suppressions.snap diff --git a/crates/ruff/tests/cli/lint.rs b/crates/ruff/tests/cli/lint.rs index 25500ed346c4f..a86d8e81be889 100644 --- a/crates/ruff/tests/cli/lint.rs +++ b/crates/ruff/tests/cli/lint.rs @@ -1440,6 +1440,78 @@ def function(): Ok(()) } +#[test] +fn ignore_noqa() -> Result<()> { + let fixture = CliTest::new()?; + fixture.write_file( + "ruff.toml", + r#" +[lint] +select = ["F401"] +"#, + )?; + + fixture.write_file( + "noqa.py", + r#" +import os # noqa: F401 + +# ruff: disable[F401] +import sys +"#, + )?; + + // without --ignore-noqa + assert_cmd_snapshot!(fixture + .check_command() + .args(["--config", "ruff.toml"]) + .arg("noqa.py"), + @r" + success: false + exit_code: 1 + ----- stdout ----- + noqa.py:5:8: F401 [*] `sys` imported but unused + Found 1 error. + [*] 1 fixable with the `--fix` option. + + ----- stderr ----- + "); + + assert_cmd_snapshot!(fixture + .check_command() + .args(["--config", "ruff.toml"]) + .arg("noqa.py") + .args(["--preview"]), + @r" + success: true + exit_code: 0 + ----- stdout ----- + All checks passed! + + ----- stderr ----- + "); + + // with --ignore-noqa --preview + assert_cmd_snapshot!(fixture + .check_command() + .args(["--config", "ruff.toml"]) + .arg("noqa.py") + .args(["--ignore-noqa", "--preview"]), + @r" + success: false + exit_code: 1 + ----- stdout ----- + noqa.py:2:8: F401 [*] `os` imported but unused + noqa.py:5:8: F401 [*] `sys` imported but unused + Found 2 errors. + [*] 2 fixable with the `--fix` option. + + ----- stderr ----- + "); + + Ok(()) +} + #[test] fn add_noqa() -> Result<()> { let fixture = CliTest::new()?; @@ -1632,6 +1704,100 @@ def unused(x): # noqa: ANN001, ARG001, D103 Ok(()) } +#[test] +fn add_noqa_existing_file_level_noqa() -> Result<()> { + let fixture = CliTest::new()?; + fixture.write_file( + "ruff.toml", + r#" +[lint] +select = ["F401"] +"#, + )?; + + fixture.write_file( + "noqa.py", + r#" +# ruff: noqa F401 +import os +"#, + )?; + + assert_cmd_snapshot!(fixture + .check_command() + .args(["--config", "ruff.toml"]) + .arg("noqa.py") + .arg("--preview") + .args(["--add-noqa"]) + .arg("-") + .pass_stdin(r#" + +"#), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + "); + + let test_code = + fs::read_to_string(fixture.root().join("noqa.py")).expect("should read test file"); + + insta::assert_snapshot!(test_code, @r" + # ruff: noqa F401 + import os + "); + + Ok(()) +} + +#[test] +fn add_noqa_existing_range_suppression() -> Result<()> { + let fixture = CliTest::new()?; + fixture.write_file( + "ruff.toml", + r#" +[lint] +select = ["F401"] +"#, + )?; + + fixture.write_file( + "noqa.py", + r#" +# ruff: disable[F401] +import os +"#, + )?; + + assert_cmd_snapshot!(fixture + .check_command() + .args(["--config", "ruff.toml"]) + .arg("noqa.py") + .arg("--preview") + .args(["--add-noqa"]) + .arg("-") + .pass_stdin(r#" + +"#), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + "); + + let test_code = + fs::read_to_string(fixture.root().join("noqa.py")).expect("should read test file"); + + insta::assert_snapshot!(test_code, @r" + # ruff: disable[F401] + import os + "); + + Ok(()) +} + #[test] fn add_noqa_multiline_comment() -> Result<()> { let fixture = CliTest::new()?; diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py b/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py new file mode 100644 index 0000000000000..7a70c4d548b0c --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py @@ -0,0 +1,56 @@ +def f(): + # These should both be ignored by the range suppression. + # ruff: disable[E741, F841] + I = 1 + # ruff: enable[E741, F841] + + +def f(): + # These should both be ignored by the implicit range suppression. + # Should also generate an "unmatched suppression" warning. + # ruff:disable[E741,F841] + I = 1 + + +def f(): + # Neither warning is ignored, and an "unmatched suppression" + # should be generated. + I = 1 + # ruff: enable[E741, F841] + + +def f(): + # One should be ignored by the range suppression, and + # the other logged to the user. + # ruff: disable[E741] + I = 1 + # ruff: enable[E741] + + +def f(): + # Test interleaved range suppressions. The first and last + # lines should each log a different warning, while the + # middle line should be completely silenced. + # ruff: disable[E741] + l = 0 + # ruff: disable[F841] + O = 1 + # ruff: enable[E741] + I = 2 + # ruff: enable[F841] + + +def f(): + # Neither of these are ignored and warnings are + # logged to user + # ruff: disable[E501] + I = 1 + # ruff: enable[E501] + + +def f(): + # These should both be ignored by the range suppression, + # and an unusued noqa diagnostic should be logged. + # ruff:disable[E741,F841] + I = 1 # noqa: E741,F841 + # ruff:enable[E741,F841] diff --git a/crates/ruff_linter/src/checkers/noqa.rs b/crates/ruff_linter/src/checkers/noqa.rs index 7cf58a5def343..2602adeeeef6a 100644 --- a/crates/ruff_linter/src/checkers/noqa.rs +++ b/crates/ruff_linter/src/checkers/noqa.rs @@ -12,17 +12,20 @@ use crate::fix::edits::delete_comment; use crate::noqa::{ Code, Directive, FileExemption, FileNoqaDirectives, NoqaDirectives, NoqaMapping, }; +use crate::preview::is_range_suppressions_enabled; use crate::registry::Rule; use crate::rule_redirects::get_redirect_target; use crate::rules::pygrep_hooks; use crate::rules::ruff; use crate::rules::ruff::rules::{UnusedCodes, UnusedNOQA}; use crate::settings::LinterSettings; +use crate::suppression::Suppressions; use crate::{Edit, Fix, Locator}; use super::ast::LintContext; /// RUF100 +#[expect(clippy::too_many_arguments)] pub(crate) fn check_noqa( context: &mut LintContext, path: &Path, @@ -31,6 +34,7 @@ pub(crate) fn check_noqa( noqa_line_for: &NoqaMapping, analyze_directives: bool, settings: &LinterSettings, + suppressions: &Suppressions, ) -> Vec { // Identify any codes that are globally exempted (within the current file). let file_noqa_directives = @@ -40,7 +44,7 @@ pub(crate) fn check_noqa( let mut noqa_directives = NoqaDirectives::from_commented_ranges(comment_ranges, &settings.external, path, locator); - if file_noqa_directives.is_empty() && noqa_directives.is_empty() { + if file_noqa_directives.is_empty() && noqa_directives.is_empty() && suppressions.is_empty() { return Vec::new(); } @@ -60,11 +64,19 @@ pub(crate) fn check_noqa( continue; } + // Apply file-level suppressions first if exemption.contains_secondary_code(code) { ignored_diagnostics.push(index); continue; } + // Apply ranged suppressions next + if is_range_suppressions_enabled(settings) && suppressions.check_diagnostic(diagnostic) { + ignored_diagnostics.push(index); + continue; + } + + // Apply end-of-line noqa suppressions last let noqa_offsets = diagnostic .parent() .into_iter() diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 719d5ac9c5332..08c0417020b02 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -32,6 +32,7 @@ use crate::rules::ruff::rules::test_rules::{self, TEST_RULES, TestRule}; use crate::settings::types::UnsafeFixes; use crate::settings::{LinterSettings, TargetVersion, flags}; use crate::source_kind::SourceKind; +use crate::suppression::Suppressions; use crate::{Locator, directives, fs}; pub(crate) mod float; @@ -128,6 +129,7 @@ pub fn check_path( source_type: PySourceType, parsed: &Parsed, target_version: TargetVersion, + suppressions: &Suppressions, ) -> Vec { // Aggregate all diagnostics. let mut context = LintContext::new(path, locator.contents(), settings); @@ -339,6 +341,7 @@ pub fn check_path( &directives.noqa_line_for, parsed.has_valid_syntax(), settings, + suppressions, ); if noqa.is_enabled() { for index in ignored.iter().rev() { @@ -400,6 +403,9 @@ pub fn add_noqa_to_path( &indexer, ); + // Parse range suppression comments + let suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); + // Generate diagnostics, ignoring any existing `noqa` directives. let diagnostics = check_path( path, @@ -414,6 +420,7 @@ pub fn add_noqa_to_path( source_type, &parsed, target_version, + &suppressions, ); // Add any missing `# noqa` pragmas. @@ -427,6 +434,7 @@ pub fn add_noqa_to_path( &directives.noqa_line_for, stylist.line_ending(), reason, + &suppressions, ) } @@ -461,6 +469,9 @@ pub fn lint_only( &indexer, ); + // Parse range suppression comments + let suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); + // Generate diagnostics. let diagnostics = check_path( path, @@ -475,6 +486,7 @@ pub fn lint_only( source_type, &parsed, target_version, + &suppressions, ); LinterResult { @@ -566,6 +578,9 @@ pub fn lint_fix<'a>( &indexer, ); + // Parse range suppression comments + let suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); + // Generate diagnostics. let diagnostics = check_path( path, @@ -580,6 +595,7 @@ pub fn lint_fix<'a>( source_type, &parsed, target_version, + &suppressions, ); if iterations == 0 { @@ -769,6 +785,7 @@ mod tests { use crate::registry::Rule; use crate::settings::LinterSettings; use crate::source_kind::SourceKind; + use crate::suppression::Suppressions; use crate::test::{TestedNotebook, assert_notebook_path, test_contents, test_snippet}; use crate::{Locator, assert_diagnostics, directives, settings}; @@ -944,6 +961,7 @@ mod tests { &locator, &indexer, ); + let suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); let mut diagnostics = check_path( path, None, @@ -957,6 +975,7 @@ mod tests { source_type, &parsed, target_version, + &suppressions, ); diagnostics.sort_by(Diagnostic::ruff_start_ordering); diagnostics diff --git a/crates/ruff_linter/src/noqa.rs b/crates/ruff_linter/src/noqa.rs index da9535817ed07..e8c3ada650de4 100644 --- a/crates/ruff_linter/src/noqa.rs +++ b/crates/ruff_linter/src/noqa.rs @@ -20,12 +20,14 @@ use crate::Locator; use crate::fs::relativize_path; use crate::registry::Rule; use crate::rule_redirects::get_redirect_target; +use crate::suppression::Suppressions; /// Generates an array of edits that matches the length of `messages`. /// Each potential edit in the array is paired, in order, with the associated diagnostic. /// Each edit will add a `noqa` comment to the appropriate line in the source to hide /// the diagnostic. These edits may conflict with each other and should not be applied /// simultaneously. +#[expect(clippy::too_many_arguments)] pub fn generate_noqa_edits( path: &Path, diagnostics: &[Diagnostic], @@ -34,11 +36,19 @@ pub fn generate_noqa_edits( external: &[String], noqa_line_for: &NoqaMapping, line_ending: LineEnding, + suppressions: &Suppressions, ) -> Vec> { let file_directives = FileNoqaDirectives::extract(locator, comment_ranges, external, path); let exemption = FileExemption::from(&file_directives); let directives = NoqaDirectives::from_commented_ranges(comment_ranges, external, path, locator); - let comments = find_noqa_comments(diagnostics, locator, &exemption, &directives, noqa_line_for); + let comments = find_noqa_comments( + diagnostics, + locator, + &exemption, + &directives, + noqa_line_for, + suppressions, + ); build_noqa_edits_by_diagnostic(comments, locator, line_ending, None) } @@ -725,6 +735,7 @@ pub(crate) fn add_noqa( noqa_line_for: &NoqaMapping, line_ending: LineEnding, reason: Option<&str>, + suppressions: &Suppressions, ) -> Result { let (count, output) = add_noqa_inner( path, @@ -735,6 +746,7 @@ pub(crate) fn add_noqa( noqa_line_for, line_ending, reason, + suppressions, ); fs::write(path, output)?; @@ -751,6 +763,7 @@ fn add_noqa_inner( noqa_line_for: &NoqaMapping, line_ending: LineEnding, reason: Option<&str>, + suppressions: &Suppressions, ) -> (usize, String) { let mut count = 0; @@ -760,7 +773,14 @@ fn add_noqa_inner( let directives = NoqaDirectives::from_commented_ranges(comment_ranges, external, path, locator); - let comments = find_noqa_comments(diagnostics, locator, &exemption, &directives, noqa_line_for); + let comments = find_noqa_comments( + diagnostics, + locator, + &exemption, + &directives, + noqa_line_for, + suppressions, + ); let edits = build_noqa_edits_by_line(comments, locator, line_ending, reason); @@ -859,6 +879,7 @@ fn find_noqa_comments<'a>( exemption: &'a FileExemption, directives: &'a NoqaDirectives, noqa_line_for: &NoqaMapping, + suppressions: &Suppressions, ) -> Vec>> { // List of noqa comments, ordered to match up with `messages` let mut comments_by_line: Vec>> = vec![]; @@ -875,6 +896,12 @@ fn find_noqa_comments<'a>( continue; } + // Apply ranged suppressions next + if suppressions.check_diagnostic(message) { + comments_by_line.push(None); + continue; + } + // Is the violation ignored by a `noqa` directive on the parent line? if let Some(parent) = message.parent() { if let Some(directive_line) = @@ -1253,6 +1280,7 @@ mod tests { use crate::rules::pycodestyle::rules::{AmbiguousVariableName, UselessSemicolon}; use crate::rules::pyflakes::rules::UnusedVariable; use crate::rules::pyupgrade::rules::PrintfStringFormatting; + use crate::suppression::Suppressions; use crate::{Edit, Violation}; use crate::{Locator, generate_noqa_edits}; @@ -2848,6 +2876,7 @@ mod tests { &noqa_line_for, LineEnding::Lf, None, + &Suppressions::default(), ); assert_eq!(count, 0); assert_eq!(output, format!("{contents}")); @@ -2872,6 +2901,7 @@ mod tests { &noqa_line_for, LineEnding::Lf, None, + &Suppressions::default(), ); assert_eq!(count, 1); assert_eq!(output, "x = 1 # noqa: F841\n"); @@ -2903,6 +2933,7 @@ mod tests { &noqa_line_for, LineEnding::Lf, None, + &Suppressions::default(), ); assert_eq!(count, 1); assert_eq!(output, "x = 1 # noqa: E741, F841\n"); @@ -2934,6 +2965,7 @@ mod tests { &noqa_line_for, LineEnding::Lf, None, + &Suppressions::default(), ); assert_eq!(count, 0); assert_eq!(output, "x = 1 # noqa"); @@ -2956,6 +2988,7 @@ print( let messages = [PrintfStringFormatting .into_diagnostic(TextRange::new(12.into(), 79.into()), &source_file)]; let comment_ranges = CommentRanges::default(); + let suppressions = Suppressions::default(); let edits = generate_noqa_edits( path, &messages, @@ -2964,6 +2997,7 @@ print( &[], &noqa_line_for, LineEnding::Lf, + &suppressions, ); assert_eq!( edits, @@ -2987,6 +3021,7 @@ bar = [UselessSemicolon.into_diagnostic(TextRange::new(4.into(), 5.into()), &source_file)]; let noqa_line_for = NoqaMapping::default(); let comment_ranges = CommentRanges::default(); + let suppressions = Suppressions::default(); let edits = generate_noqa_edits( path, &messages, @@ -2995,6 +3030,7 @@ bar = &[], &noqa_line_for, LineEnding::Lf, + &suppressions, ); assert_eq!( edits, diff --git a/crates/ruff_linter/src/preview.rs b/crates/ruff_linter/src/preview.rs index 52be7305454f8..93a49e63a049a 100644 --- a/crates/ruff_linter/src/preview.rs +++ b/crates/ruff_linter/src/preview.rs @@ -286,3 +286,8 @@ pub(crate) const fn is_s310_resolve_string_literal_bindings_enabled( ) -> bool { settings.preview.is_enabled() } + +// https://github.com/astral-sh/ruff/pull/21623 +pub(crate) const fn is_range_suppressions_enabled(settings: &LinterSettings) -> bool { + settings.preview.is_enabled() +} diff --git a/crates/ruff_linter/src/rules/pyflakes/mod.rs b/crates/ruff_linter/src/rules/pyflakes/mod.rs index d290ed38c5efd..02cd5158a8ac4 100644 --- a/crates/ruff_linter/src/rules/pyflakes/mod.rs +++ b/crates/ruff_linter/src/rules/pyflakes/mod.rs @@ -28,6 +28,7 @@ mod tests { use crate::settings::types::PreviewMode; use crate::settings::{LinterSettings, flags}; use crate::source_kind::SourceKind; + use crate::suppression::Suppressions; use crate::test::{test_contents, test_path, test_snippet}; use crate::{Locator, assert_diagnostics, assert_diagnostics_diff, directives}; @@ -955,6 +956,8 @@ mod tests { &locator, &indexer, ); + let suppressions = + Suppressions::from_tokens(&settings, locator.contents(), parsed.tokens()); let mut messages = check_path( Path::new(""), None, @@ -968,6 +971,7 @@ mod tests { source_type, &parsed, target_version, + &suppressions, ); messages.sort_by(Diagnostic::ruff_start_ordering); let actual = messages diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index 5f06ffdb9f7b4..c2d03fb1ae6be 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -305,6 +305,25 @@ mod tests { Ok(()) } + #[test] + fn range_suppressions() -> Result<()> { + assert_diagnostics_diff!( + Path::new("ruff/suppressions.py"), + &settings::LinterSettings::for_rules(vec![ + Rule::UnusedVariable, + Rule::AmbiguousVariableName, + Rule::UnusedNOQA, + ]), + &settings::LinterSettings::for_rules(vec![ + Rule::UnusedVariable, + Rule::AmbiguousVariableName, + Rule::UnusedNOQA, + ]) + .with_preview_mode(), + ); + Ok(()) + } + #[test] fn ruf100_0() -> Result<()> { let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__range_suppressions.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__range_suppressions.snap new file mode 100644 index 0000000000000..4e095074822bb --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__range_suppressions.snap @@ -0,0 +1,168 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 9 +Added: 1 + +--- Removed --- +E741 Ambiguous variable name: `I` + --> suppressions.py:4:5 + | +2 | # These should both be ignored by the range suppression. +3 | # ruff: disable[E741, F841] +4 | I = 1 + | ^ +5 | # ruff: enable[E741, F841] + | + + +F841 [*] Local variable `I` is assigned to but never used + --> suppressions.py:4:5 + | +2 | # These should both be ignored by the range suppression. +3 | # ruff: disable[E741, F841] +4 | I = 1 + | ^ +5 | # ruff: enable[E741, F841] + | +help: Remove assignment to unused variable `I` +1 | def f(): +2 | # These should both be ignored by the range suppression. +3 | # ruff: disable[E741, F841] + - I = 1 +4 + pass +5 | # ruff: enable[E741, F841] +6 | +7 | +note: This is an unsafe fix and may change runtime behavior + + +E741 Ambiguous variable name: `I` + --> suppressions.py:12:5 + | +10 | # Should also generate an "unmatched suppression" warning. +11 | # ruff:disable[E741,F841] +12 | I = 1 + | ^ + | + + +F841 [*] Local variable `I` is assigned to but never used + --> suppressions.py:12:5 + | +10 | # Should also generate an "unmatched suppression" warning. +11 | # ruff:disable[E741,F841] +12 | I = 1 + | ^ + | +help: Remove assignment to unused variable `I` +9 | # These should both be ignored by the implicit range suppression. +10 | # Should also generate an "unmatched suppression" warning. +11 | # ruff:disable[E741,F841] + - I = 1 +12 + pass +13 | +14 | +15 | def f(): +note: This is an unsafe fix and may change runtime behavior + + +E741 Ambiguous variable name: `I` + --> suppressions.py:26:5 + | +24 | # the other logged to the user. +25 | # ruff: disable[E741] +26 | I = 1 + | ^ +27 | # ruff: enable[E741] + | + + +E741 Ambiguous variable name: `l` + --> suppressions.py:35:5 + | +33 | # middle line should be completely silenced. +34 | # ruff: disable[E741] +35 | l = 0 + | ^ +36 | # ruff: disable[F841] +37 | O = 1 + | + + +E741 Ambiguous variable name: `O` + --> suppressions.py:37:5 + | +35 | l = 0 +36 | # ruff: disable[F841] +37 | O = 1 + | ^ +38 | # ruff: enable[E741] +39 | I = 2 + | + + +F841 [*] Local variable `O` is assigned to but never used + --> suppressions.py:37:5 + | +35 | l = 0 +36 | # ruff: disable[F841] +37 | O = 1 + | ^ +38 | # ruff: enable[E741] +39 | I = 2 + | +help: Remove assignment to unused variable `O` +34 | # ruff: disable[E741] +35 | l = 0 +36 | # ruff: disable[F841] + - O = 1 +37 | # ruff: enable[E741] +38 | I = 2 +39 | # ruff: enable[F841] +note: This is an unsafe fix and may change runtime behavior + + +F841 [*] Local variable `I` is assigned to but never used + --> suppressions.py:39:5 + | +37 | O = 1 +38 | # ruff: enable[E741] +39 | I = 2 + | ^ +40 | # ruff: enable[F841] + | +help: Remove assignment to unused variable `I` +36 | # ruff: disable[F841] +37 | O = 1 +38 | # ruff: enable[E741] + - I = 2 +39 | # ruff: enable[F841] +40 | +41 | +note: This is an unsafe fix and may change runtime behavior + + + +--- Added --- +RUF100 [*] Unused `noqa` directive (unused: `E741`, `F841`) + --> suppressions.py:55:12 + | +53 | # and an unusued noqa diagnostic should be logged. +54 | # ruff:disable[E741,F841] +55 | I = 1 # noqa: E741,F841 + | ^^^^^^^^^^^^^^^^^ +56 | # ruff:enable[E741,F841] + | +help: Remove unused `noqa` directive +52 | # These should both be ignored by the range suppression, +53 | # and an unusued noqa diagnostic should be logged. +54 | # ruff:disable[E741,F841] + - I = 1 # noqa: E741,F841 +55 + I = 1 +56 | # ruff:enable[E741,F841] diff --git a/crates/ruff_linter/src/settings/mod.rs b/crates/ruff_linter/src/settings/mod.rs index b94e4edafbc29..5d5e35aa8d0a0 100644 --- a/crates/ruff_linter/src/settings/mod.rs +++ b/crates/ruff_linter/src/settings/mod.rs @@ -465,6 +465,12 @@ impl LinterSettings { self } + #[must_use] + pub fn with_preview_mode(mut self) -> Self { + self.preview = PreviewMode::Enabled; + self + } + /// Resolve the [`TargetVersion`] to use for linting. /// /// This method respects the per-file version overrides in diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 66ad98d25e304..3c1a2f57abfb4 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -1,5 +1,6 @@ use compact_str::CompactString; use core::fmt; +use ruff_db::diagnostic::Diagnostic; use ruff_python_ast::token::{TokenKind, Tokens}; use ruff_python_ast::whitespace::indentation; use std::{error::Error, fmt::Formatter}; @@ -9,6 +10,9 @@ use ruff_python_trivia::Cursor; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize, TextSlice}; use smallvec::{SmallVec, smallvec}; +use crate::preview::is_range_suppressions_enabled; +use crate::settings::LinterSettings; + #[allow(unused)] #[derive(Clone, Debug, Eq, PartialEq)] enum SuppressionAction { @@ -98,8 +102,8 @@ pub(crate) struct InvalidSuppression { } #[allow(unused)] -#[derive(Debug)] -pub(crate) struct Suppressions { +#[derive(Debug, Default)] +pub struct Suppressions { /// Valid suppression ranges with associated comments valid: Vec, @@ -112,9 +116,41 @@ pub(crate) struct Suppressions { #[allow(unused)] impl Suppressions { - pub(crate) fn from_tokens(source: &str, tokens: &Tokens) -> Suppressions { - let builder = SuppressionsBuilder::new(source); - builder.load_from_tokens(tokens) + pub fn from_tokens(settings: &LinterSettings, source: &str, tokens: &Tokens) -> Suppressions { + if is_range_suppressions_enabled(settings) { + let builder = SuppressionsBuilder::new(source); + builder.load_from_tokens(tokens) + } else { + Suppressions::default() + } + } + + pub(crate) fn is_empty(&self) -> bool { + self.valid.is_empty() + } + + /// Check if a diagnostic is suppressed by any known range suppressions + pub(crate) fn check_diagnostic(&self, diagnostic: &Diagnostic) -> bool { + if self.valid.is_empty() { + return false; + } + + let Some(code) = diagnostic.secondary_code() else { + return false; + }; + let Some(span) = diagnostic.primary_span() else { + return false; + }; + let Some(range) = span.range() else { + return false; + }; + + for suppression in &self.valid { + if *code == suppression.code.as_str() && suppression.range.contains_range(range) { + return true; + } + } + false } } @@ -457,9 +493,12 @@ mod tests { use ruff_text_size::{TextRange, TextSize}; use similar::DiffableStr; - use crate::suppression::{ - InvalidSuppression, ParseError, Suppression, SuppressionAction, SuppressionComment, - SuppressionParser, Suppressions, + use crate::{ + settings::LinterSettings, + suppression::{ + InvalidSuppression, ParseError, Suppression, SuppressionAction, SuppressionComment, + SuppressionParser, Suppressions, + }, }; #[test] @@ -1376,7 +1415,11 @@ def bar(): /// Parse all suppressions and errors in a module for testing fn debug(source: &'_ str) -> DebugSuppressions<'_> { let parsed = parse(source, ParseOptions::from(Mode::Module)).unwrap(); - let suppressions = Suppressions::from_tokens(source, parsed.tokens()); + let suppressions = Suppressions::from_tokens( + &LinterSettings::default().with_preview_mode(), + source, + parsed.tokens(), + ); DebugSuppressions { source, suppressions, diff --git a/crates/ruff_linter/src/test.rs b/crates/ruff_linter/src/test.rs index 67a6728404345..344c921890105 100644 --- a/crates/ruff_linter/src/test.rs +++ b/crates/ruff_linter/src/test.rs @@ -32,6 +32,7 @@ use crate::packaging::detect_package_root; use crate::settings::types::UnsafeFixes; use crate::settings::{LinterSettings, flags}; use crate::source_kind::SourceKind; +use crate::suppression::Suppressions; use crate::{Applicability, FixAvailability}; use crate::{Locator, directives}; @@ -234,6 +235,7 @@ pub(crate) fn test_contents<'a>( &locator, &indexer, ); + let suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); let messages = check_path( path, path.parent() @@ -249,6 +251,7 @@ pub(crate) fn test_contents<'a>( source_type, &parsed, target_version, + &suppressions, ); let source_has_errors = parsed.has_invalid_syntax(); @@ -299,6 +302,8 @@ pub(crate) fn test_contents<'a>( &indexer, ); + let suppressions = + Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); let fixed_messages = check_path( path, None, @@ -312,6 +317,7 @@ pub(crate) fn test_contents<'a>( source_type, &parsed, target_version, + &suppressions, ); if parsed.has_invalid_syntax() && !source_has_errors { diff --git a/crates/ruff_server/src/lint.rs b/crates/ruff_server/src/lint.rs index c9d0d76becd63..db3f9ce4d85d7 100644 --- a/crates/ruff_server/src/lint.rs +++ b/crates/ruff_server/src/lint.rs @@ -20,6 +20,7 @@ use ruff_linter::{ packaging::detect_package_root, settings::flags, source_kind::SourceKind, + suppression::Suppressions, }; use ruff_notebook::Notebook; use ruff_python_codegen::Stylist; @@ -118,6 +119,10 @@ pub(crate) fn check( // Extract the `# noqa` and `# isort: skip` directives from the source. let directives = extract_directives(parsed.tokens(), Flags::all(), &locator, &indexer); + // Parse range suppression comments + let suppressions = + Suppressions::from_tokens(&settings.linter, locator.contents(), parsed.tokens()); + // Generate checks. let diagnostics = check_path( &document_path, @@ -132,6 +137,7 @@ pub(crate) fn check( source_type, &parsed, target_version, + &suppressions, ); let noqa_edits = generate_noqa_edits( @@ -142,6 +148,7 @@ pub(crate) fn check( &settings.linter.external, &directives.noqa_line_for, stylist.line_ending(), + &suppressions, ); let mut diagnostics_map = DiagnosticsMap::default(); diff --git a/crates/ruff_wasm/src/lib.rs b/crates/ruff_wasm/src/lib.rs index 8c18111d167e3..6dd49a15bf44c 100644 --- a/crates/ruff_wasm/src/lib.rs +++ b/crates/ruff_wasm/src/lib.rs @@ -2,6 +2,7 @@ use std::path::Path; use js_sys::Error; use ruff_linter::settings::types::PythonVersion; +use ruff_linter::suppression::Suppressions; use serde::{Deserialize, Serialize}; use wasm_bindgen::prelude::*; @@ -212,6 +213,9 @@ impl Workspace { &indexer, ); + let suppressions = + Suppressions::from_tokens(&self.settings.linter, locator.contents(), parsed.tokens()); + // Generate checks. let diagnostics = check_path( Path::new(""), @@ -226,6 +230,7 @@ impl Workspace { source_type, &parsed, target_version, + &suppressions, ); let source_code = locator.to_source_code();