From bbfcf6e111648052783bdb68ec71706fc69b714f Mon Sep 17 00:00:00 2001 From: David Peter Date: Mon, 1 Sep 2025 11:22:19 +0200 Subject: [PATCH 01/20] [ty] `__class_getitem__` is a classmethod (#20192) ## Summary `__class_getitem__` is [implicitly a classmethod](https://docs.python.org/3/reference/datamodel.html#object.__class_getitem__). ## Test Plan Added regression test. --- .../ty_python_semantic/resources/mdtest/subscript/class.md | 6 ++++++ crates/ty_python_semantic/src/types/function.rs | 5 ++++- crates/ty_python_semantic/src/types/infer.rs | 2 +- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/crates/ty_python_semantic/resources/mdtest/subscript/class.md b/crates/ty_python_semantic/resources/mdtest/subscript/class.md index 86de205086..947f7fbbaf 100644 --- a/crates/ty_python_semantic/resources/mdtest/subscript/class.md +++ b/crates/ty_python_semantic/resources/mdtest/subscript/class.md @@ -19,6 +19,12 @@ class Identity: reveal_type(Identity[0]) # revealed: str ``` +`__class_getitem__` is implicitly a classmethod, so it can be called like this: + +```py +reveal_type(Identity.__class_getitem__(0)) # revealed: str +``` + ## Class getitem union ```py diff --git a/crates/ty_python_semantic/src/types/function.rs b/crates/ty_python_semantic/src/types/function.rs index 6b3c2902c8..3730439017 100644 --- a/crates/ty_python_semantic/src/types/function.rs +++ b/crates/ty_python_semantic/src/types/function.rs @@ -725,7 +725,10 @@ impl<'db> FunctionType<'db> { /// classmethod. pub(crate) fn is_classmethod(self, db: &'db dyn Db) -> bool { self.has_known_decorator(db, FunctionDecorators::CLASSMETHOD) - || self.name(db) == "__init_subclass__" + || matches!( + self.name(db).as_str(), + "__init_subclass__" | "__class_getitem__" + ) } /// If the implementation of this function is deprecated, returns the `@warnings.deprecated`. diff --git a/crates/ty_python_semantic/src/types/infer.rs b/crates/ty_python_semantic/src/types/infer.rs index 7259f9565d..7916fa1fc4 100644 --- a/crates/ty_python_semantic/src/types/infer.rs +++ b/crates/ty_python_semantic/src/types/infer.rs @@ -9203,7 +9203,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } } - match ty.try_call(db, &CallArguments::positional([value_ty, slice_ty])) { + match ty.try_call(db, &CallArguments::positional([slice_ty])) { Ok(bindings) => return bindings.return_type(db), Err(CallError(_, bindings)) => { if let Some(builder) = From f40a0b38000c73fcc09c3a675e5dca95c209fc4e Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Tue, 2 Sep 2025 10:38:26 -0400 Subject: [PATCH 02/20] [ty] add more lsp tests for overloads (#20148) I decided to split out the addition of these tests from other PRs so that it's easier to follow changes to the LSP's function call handling. I'm not particularly concerned with whether the results produced by these tests are "good" or "bad" in this PR, I'm just establishing a baseline. --- crates/ty_ide/src/goto_declaration.rs | 456 ++++++++++++++++++++++++++ crates/ty_ide/src/goto_definition.rs | 312 ++++++++++++++++++ crates/ty_ide/src/hover.rs | 450 ++++++++++++++++++++++++- crates/ty_ide/src/signature_help.rs | 440 +++++++++++++++++++++++++ 4 files changed, 1656 insertions(+), 2 deletions(-) diff --git a/crates/ty_ide/src/goto_declaration.rs b/crates/ty_ide/src/goto_declaration.rs index cdb2cd31ed..2e25862f59 100644 --- a/crates/ty_ide/src/goto_declaration.rs +++ b/crates/ty_ide/src/goto_declaration.rs @@ -1355,6 +1355,462 @@ class MyClass: "#); } + #[test] + fn goto_declaration_overload_type_disambiguated1() { + let test = CursorTest::builder() + .source( + "main.py", + " +from mymodule import ab + +ab(1) +", + ) + .source( + "mymodule.py", + r#" +def ab(a): + """the real implementation!""" +"#, + ) + .source( + "mymodule.pyi", + r#" +from typing import overload + +@overload +def ab(a: int): ... + +@overload +def ab(a: str): ... +"#, + ) + .build(); + + assert_snapshot!(test.goto_declaration(), @r" + info[goto-declaration]: Declaration + --> mymodule.pyi:5:5 + | + 4 | @overload + 5 | def ab(a: int): ... + | ^^ + 6 | + 7 | @overload + | + info: Source + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab(1) + | ^^ + | + + info[goto-declaration]: Declaration + --> mymodule.pyi:8:5 + | + 7 | @overload + 8 | def ab(a: str): ... + | ^^ + | + info: Source + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab(1) + | ^^ + | + "); + } + + #[test] + fn goto_declaration_overload_type_disambiguated2() { + let test = CursorTest::builder() + .source( + "main.py", + r#" +from mymodule import ab + +ab("hello") +"#, + ) + .source( + "mymodule.py", + r#" +def ab(a): + """the real implementation!""" +"#, + ) + .source( + "mymodule.pyi", + r#" +from typing import overload + +@overload +def ab(a: int): ... + +@overload +def ab(a: str): ... +"#, + ) + .build(); + + assert_snapshot!(test.goto_declaration(), @r#" + info[goto-declaration]: Declaration + --> mymodule.pyi:5:5 + | + 4 | @overload + 5 | def ab(a: int): ... + | ^^ + 6 | + 7 | @overload + | + info: Source + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab("hello") + | ^^ + | + + info[goto-declaration]: Declaration + --> mymodule.pyi:8:5 + | + 7 | @overload + 8 | def ab(a: str): ... + | ^^ + | + info: Source + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab("hello") + | ^^ + | + "#); + } + + #[test] + fn goto_declaration_overload_arity_disambiguated1() { + let test = CursorTest::builder() + .source( + "main.py", + " +from mymodule import ab + +ab(1, 2) +", + ) + .source( + "mymodule.py", + r#" +def ab(a, b = None): + """the real implementation!""" +"#, + ) + .source( + "mymodule.pyi", + r#" +from typing import overload + +@overload +def ab(a: int, b: int): ... + +@overload +def ab(a: int): ... +"#, + ) + .build(); + + assert_snapshot!(test.goto_declaration(), @r" + info[goto-declaration]: Declaration + --> mymodule.pyi:5:5 + | + 4 | @overload + 5 | def ab(a: int, b: int): ... + | ^^ + 6 | + 7 | @overload + | + info: Source + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab(1, 2) + | ^^ + | + + info[goto-declaration]: Declaration + --> mymodule.pyi:8:5 + | + 7 | @overload + 8 | def ab(a: int): ... + | ^^ + | + info: Source + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab(1, 2) + | ^^ + | + "); + } + + #[test] + fn goto_declaration_overload_arity_disambiguated2() { + let test = CursorTest::builder() + .source( + "main.py", + " +from mymodule import ab + +ab(1) +", + ) + .source( + "mymodule.py", + r#" +def ab(a, b = None): + """the real implementation!""" +"#, + ) + .source( + "mymodule.pyi", + r#" +from typing import overload + +@overload +def ab(a: int, b: int): ... + +@overload +def ab(a: int): ... +"#, + ) + .build(); + + assert_snapshot!(test.goto_declaration(), @r" + info[goto-declaration]: Declaration + --> mymodule.pyi:5:5 + | + 4 | @overload + 5 | def ab(a: int, b: int): ... + | ^^ + 6 | + 7 | @overload + | + info: Source + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab(1) + | ^^ + | + + info[goto-declaration]: Declaration + --> mymodule.pyi:8:5 + | + 7 | @overload + 8 | def ab(a: int): ... + | ^^ + | + info: Source + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab(1) + | ^^ + | + "); + } + + #[test] + fn goto_declaration_overload_keyword_disambiguated1() { + let test = CursorTest::builder() + .source( + "main.py", + " +from mymodule import ab + +ab(1, b=2) +", + ) + .source( + "mymodule.py", + r#" +def ab(a, *, b = None, c = None): + """the real implementation!""" +"#, + ) + .source( + "mymodule.pyi", + r#" +from typing import overload + +@overload +def ab(a: int): ... + +@overload +def ab(a: int, *, b: int): ... + +@overload +def ab(a: int, *, c: int): ... +"#, + ) + .build(); + + assert_snapshot!(test.goto_declaration(), @r" + info[goto-declaration]: Declaration + --> mymodule.pyi:5:5 + | + 4 | @overload + 5 | def ab(a: int): ... + | ^^ + 6 | + 7 | @overload + | + info: Source + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab(1, b=2) + | ^^ + | + + info[goto-declaration]: Declaration + --> mymodule.pyi:8:5 + | + 7 | @overload + 8 | def ab(a: int, *, b: int): ... + | ^^ + 9 | + 10 | @overload + | + info: Source + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab(1, b=2) + | ^^ + | + + info[goto-declaration]: Declaration + --> mymodule.pyi:11:5 + | + 10 | @overload + 11 | def ab(a: int, *, c: int): ... + | ^^ + | + info: Source + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab(1, b=2) + | ^^ + | + "); + } + + #[test] + fn goto_declaration_overload_keyword_disambiguated2() { + let test = CursorTest::builder() + .source( + "main.py", + " +from mymodule import ab + +ab(1, c=2) +", + ) + .source( + "mymodule.py", + r#" +def ab(a, *, b = None, c = None): + """the real implementation!""" +"#, + ) + .source( + "mymodule.pyi", + r#" +from typing import overload + +@overload +def ab(a: int): ... + +@overload +def ab(a: int, *, b: int): ... + +@overload +def ab(a: int, *, c: int): ... +"#, + ) + .build(); + + assert_snapshot!(test.goto_declaration(), @r" + info[goto-declaration]: Declaration + --> mymodule.pyi:5:5 + | + 4 | @overload + 5 | def ab(a: int): ... + | ^^ + 6 | + 7 | @overload + | + info: Source + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab(1, c=2) + | ^^ + | + + info[goto-declaration]: Declaration + --> mymodule.pyi:8:5 + | + 7 | @overload + 8 | def ab(a: int, *, b: int): ... + | ^^ + 9 | + 10 | @overload + | + info: Source + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab(1, c=2) + | ^^ + | + + info[goto-declaration]: Declaration + --> mymodule.pyi:11:5 + | + 10 | @overload + 11 | def ab(a: int, *, c: int): ... + | ^^ + | + info: Source + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab(1, c=2) + | ^^ + | + "); + } + impl CursorTest { fn goto_declaration(&self) -> String { let Some(targets) = goto_declaration(&self.db, self.cursor.file, self.cursor.offset) diff --git a/crates/ty_ide/src/goto_definition.rs b/crates/ty_ide/src/goto_definition.rs index 3c5edac5ec..55156488fc 100644 --- a/crates/ty_ide/src/goto_definition.rs +++ b/crates/ty_ide/src/goto_definition.rs @@ -802,6 +802,318 @@ my_func(my_other_func(ab=5, y=2), 0) } } + #[test] + fn goto_definition_overload_type_disambiguated1() { + let test = CursorTest::builder() + .source( + "main.py", + " +from mymodule import ab + +ab(1) +", + ) + .source( + "mymodule.py", + r#" +def ab(a): + """the real implementation!""" +"#, + ) + .source( + "mymodule.pyi", + r#" +from typing import overload + +@overload +def ab(a: int): ... + +@overload +def ab(a: str): ... +"#, + ) + .build(); + + assert_snapshot!(test.goto_definition(), @r#" + info[goto-definition]: Definition + --> mymodule.py:2:5 + | + 2 | def ab(a): + | ^^ + 3 | """the real implementation!""" + | + info: Source + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab(1) + | ^^ + | + "#); + } + + #[test] + fn goto_definition_overload_type_disambiguated2() { + let test = CursorTest::builder() + .source( + "main.py", + r#" +from mymodule import ab + +ab("hello") +"#, + ) + .source( + "mymodule.py", + r#" +def ab(a): + """the real implementation!""" +"#, + ) + .source( + "mymodule.pyi", + r#" +from typing import overload + +@overload +def ab(a: int): ... + +@overload +def ab(a: str): ... +"#, + ) + .build(); + + assert_snapshot!(test.goto_definition(), @r#" + info[goto-definition]: Definition + --> mymodule.py:2:5 + | + 2 | def ab(a): + | ^^ + 3 | """the real implementation!""" + | + info: Source + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab("hello") + | ^^ + | + "#); + } + + #[test] + fn goto_definition_overload_arity_disambiguated1() { + let test = CursorTest::builder() + .source( + "main.py", + " +from mymodule import ab + +ab(1, 2) +", + ) + .source( + "mymodule.py", + r#" +def ab(a, b = None): + """the real implementation!""" +"#, + ) + .source( + "mymodule.pyi", + r#" +from typing import overload + +@overload +def ab(a: int, b: int): ... + +@overload +def ab(a: int): ... +"#, + ) + .build(); + + assert_snapshot!(test.goto_definition(), @r#" + info[goto-definition]: Definition + --> mymodule.py:2:5 + | + 2 | def ab(a, b = None): + | ^^ + 3 | """the real implementation!""" + | + info: Source + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab(1, 2) + | ^^ + | + "#); + } + + #[test] + fn goto_definition_overload_arity_disambiguated2() { + let test = CursorTest::builder() + .source( + "main.py", + " +from mymodule import ab + +ab(1) +", + ) + .source( + "mymodule.py", + r#" +def ab(a, b = None): + """the real implementation!""" +"#, + ) + .source( + "mymodule.pyi", + r#" +from typing import overload + +@overload +def ab(a: int, b: int): ... + +@overload +def ab(a: int): ... +"#, + ) + .build(); + + assert_snapshot!(test.goto_definition(), @r#" + info[goto-definition]: Definition + --> mymodule.py:2:5 + | + 2 | def ab(a, b = None): + | ^^ + 3 | """the real implementation!""" + | + info: Source + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab(1) + | ^^ + | + "#); + } + + #[test] + fn goto_definition_overload_keyword_disambiguated1() { + let test = CursorTest::builder() + .source( + "main.py", + " +from mymodule import ab + +ab(1, b=2) +", + ) + .source( + "mymodule.py", + r#" +def ab(a, *, b = None, c = None): + """the real implementation!""" +"#, + ) + .source( + "mymodule.pyi", + r#" +from typing import overload + +@overload +def ab(a: int): ... + +@overload +def ab(a: int, *, b: int): ... + +@overload +def ab(a: int, *, c: int): ... +"#, + ) + .build(); + + assert_snapshot!(test.goto_definition(), @r#" + info[goto-definition]: Definition + --> mymodule.py:2:5 + | + 2 | def ab(a, *, b = None, c = None): + | ^^ + 3 | """the real implementation!""" + | + info: Source + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab(1, b=2) + | ^^ + | + "#); + } + + #[test] + fn goto_definition_overload_keyword_disambiguated2() { + let test = CursorTest::builder() + .source( + "main.py", + " +from mymodule import ab + +ab(1, c=2) +", + ) + .source( + "mymodule.py", + r#" +def ab(a, *, b = None, c = None): + """the real implementation!""" +"#, + ) + .source( + "mymodule.pyi", + r#" +from typing import overload + +@overload +def ab(a: int): ... + +@overload +def ab(a: int, *, b: int): ... + +@overload +def ab(a: int, *, c: int): ... +"#, + ) + .build(); + + assert_snapshot!(test.goto_definition(), @r#" + info[goto-definition]: Definition + --> mymodule.py:2:5 + | + 2 | def ab(a, *, b = None, c = None): + | ^^ + 3 | """the real implementation!""" + | + info: Source + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab(1, c=2) + | ^^ + | + "#); + } + struct GotoDefinitionDiagnostic { source: FileRange, target: FileRange, diff --git a/crates/ty_ide/src/hover.rs b/crates/ty_ide/src/hover.rs index 90b64d911e..fbfc57041e 100644 --- a/crates/ty_ide/src/hover.rs +++ b/crates/ty_ide/src/hover.rs @@ -791,7 +791,453 @@ mod tests { } #[test] - fn hover_overload() { + fn hover_overload_type_disambiguated1() { + let test = CursorTest::builder() + .source( + "main.py", + " +from mymodule import ab + +ab(1) +", + ) + .source( + "mymodule.py", + r#" +def ab(a): + """the real implementation!""" +"#, + ) + .source( + "mymodule.pyi", + r#" +from typing import overload + +@overload +def ab(a: int): + """the int overload""" + +@overload +def ab(a: str): ... + """the str overload""" +"#, + ) + .build(); + + assert_snapshot!(test.hover(), @r" + (a: int) -> Unknown + (a: str) -> Unknown + --------------------------------------------- + the int overload + + --------------------------------------------- + ```python + (a: int) -> Unknown + (a: str) -> Unknown + ``` + --- + ```text + the int overload + + ``` + --------------------------------------------- + info[hover]: Hovered content is + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab(1) + | ^- + | || + | |Cursor offset + | source + | + "); + } + + #[test] + fn hover_overload_type_disambiguated2() { + let test = CursorTest::builder() + .source( + "main.py", + r#" +from mymodule import ab + +ab("hello") +"#, + ) + .source( + "mymodule.py", + r#" +def ab(a): + """the real implementation!""" +"#, + ) + .source( + "mymodule.pyi", + r#" +from typing import overload + +@overload +def ab(a: int): + """the int overload""" + +@overload +def ab(a: str): + """the str overload""" +"#, + ) + .build(); + + assert_snapshot!(test.hover(), @r#" + (a: int) -> Unknown + (a: str) -> Unknown + --------------------------------------------- + the int overload + + --------------------------------------------- + ```python + (a: int) -> Unknown + (a: str) -> Unknown + ``` + --- + ```text + the int overload + + ``` + --------------------------------------------- + info[hover]: Hovered content is + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab("hello") + | ^- + | || + | |Cursor offset + | source + | + "#); + } + + #[test] + fn hover_overload_arity_disambiguated1() { + let test = CursorTest::builder() + .source( + "main.py", + " +from mymodule import ab + +ab(1, 2) +", + ) + .source( + "mymodule.py", + r#" +def ab(a, b = None): + """the real implementation!""" +"#, + ) + .source( + "mymodule.pyi", + r#" +from typing import overload + +@overload +def ab(a: int, b: int): + """the two arg overload""" + +@overload +def ab(a: int): + """the one arg overload""" +"#, + ) + .build(); + + assert_snapshot!(test.hover(), @r" + ( + a: int, + b: int + ) -> Unknown + (a: int) -> Unknown + --------------------------------------------- + the two arg overload + + --------------------------------------------- + ```python + ( + a: int, + b: int + ) -> Unknown + (a: int) -> Unknown + ``` + --- + ```text + the two arg overload + + ``` + --------------------------------------------- + info[hover]: Hovered content is + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab(1, 2) + | ^- + | || + | |Cursor offset + | source + | + "); + } + + #[test] + fn hover_overload_arity_disambiguated2() { + let test = CursorTest::builder() + .source( + "main.py", + " +from mymodule import ab + +ab(1) +", + ) + .source( + "mymodule.py", + r#" +def ab(a, b = None): + """the real implementation!""" +"#, + ) + .source( + "mymodule.pyi", + r#" +from typing import overload + +@overload +def ab(a: int, b: int): + """the two arg overload""" + +@overload +def ab(a: int): + """the one arg overload""" +"#, + ) + .build(); + + assert_snapshot!(test.hover(), @r" + ( + a: int, + b: int + ) -> Unknown + (a: int) -> Unknown + --------------------------------------------- + the two arg overload + + --------------------------------------------- + ```python + ( + a: int, + b: int + ) -> Unknown + (a: int) -> Unknown + ``` + --- + ```text + the two arg overload + + ``` + --------------------------------------------- + info[hover]: Hovered content is + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab(1) + | ^- + | || + | |Cursor offset + | source + | + "); + } + + #[test] + fn hover_overload_keyword_disambiguated1() { + let test = CursorTest::builder() + .source( + "main.py", + " +from mymodule import ab + +ab(1, b=2) +", + ) + .source( + "mymodule.py", + r#" +def ab(a, *, b = None, c = None): + """the real implementation!""" +"#, + ) + .source( + "mymodule.pyi", + r#" +from typing import overload + +@overload +def ab(a: int): + """keywordless overload""" + +@overload +def ab(a: int, *, b: int): + """b overload""" + +@overload +def ab(a: int, *, c: int): + """c overload""" +"#, + ) + .build(); + + assert_snapshot!(test.hover(), @r" + (a: int) -> Unknown + ( + a: int, + *, + b: int + ) -> Unknown + ( + a: int, + *, + c: int + ) -> Unknown + --------------------------------------------- + keywordless overload + + --------------------------------------------- + ```python + (a: int) -> Unknown + ( + a: int, + *, + b: int + ) -> Unknown + ( + a: int, + *, + c: int + ) -> Unknown + ``` + --- + ```text + keywordless overload + + ``` + --------------------------------------------- + info[hover]: Hovered content is + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab(1, b=2) + | ^- + | || + | |Cursor offset + | source + | + "); + } + + #[test] + fn hover_overload_keyword_disambiguated2() { + let test = CursorTest::builder() + .source( + "main.py", + " +from mymodule import ab + +ab(1, c=2) +", + ) + .source( + "mymodule.py", + r#" +def ab(a, *, b = None, c = None): + """the real implementation!""" +"#, + ) + .source( + "mymodule.pyi", + r#" +from typing import overload + +@overload +def ab(a: int): + """keywordless overload""" + +@overload +def ab(a: int, *, b: int): + """b overload""" + +@overload +def ab(a: int, *, c: int): + """c overload""" +"#, + ) + .build(); + + assert_snapshot!(test.hover(), @r" + (a: int) -> Unknown + ( + a: int, + *, + b: int + ) -> Unknown + ( + a: int, + *, + c: int + ) -> Unknown + --------------------------------------------- + keywordless overload + + --------------------------------------------- + ```python + (a: int) -> Unknown + ( + a: int, + *, + b: int + ) -> Unknown + ( + a: int, + *, + c: int + ) -> Unknown + ``` + --- + ```text + keywordless overload + + ``` + --------------------------------------------- + info[hover]: Hovered content is + --> main.py:4:1 + | + 2 | from mymodule import ab + 3 | + 4 | ab(1, c=2) + | ^- + | || + | |Cursor offset + | source + | + "); + } + + #[test] + fn hover_overload_ambiguous() { let test = cursor_test( r#" from typing import overload @@ -858,7 +1304,7 @@ mod tests { } #[test] - fn hover_overload_compact() { + fn hover_overload_ambiguous_compact() { let test = cursor_test( r#" from typing import overload diff --git a/crates/ty_ide/src/signature_help.rs b/crates/ty_ide/src/signature_help.rs index e9f775e6d6..bbcb676c69 100644 --- a/crates/ty_ide/src/signature_help.rs +++ b/crates/ty_ide/src/signature_help.rs @@ -258,6 +258,9 @@ fn create_parameters_from_offsets( #[cfg(test)] mod tests { + use insta::assert_snapshot; + + use crate::MarkupKind; use crate::docstring::Docstring; use crate::signature_help::SignatureHelpInfo; use crate::tests::{CursorTest, cursor_test}; @@ -470,6 +473,354 @@ mod tests { assert_eq!(param2.name, "value"); } + #[test] + fn signature_help_overload_type_disambiguated1() { + let test = CursorTest::builder() + .source( + "main.py", + " +from mymodule import ab + +ab(1) +", + ) + .source( + "mymodule.py", + r#" +def ab(a): + """the real implementation!""" +"#, + ) + .source( + "mymodule.pyi", + r#" +from typing import overload + +@overload +def ab(a: int): + """the int overload""" + +@overload +def ab(a: str): ... + """the str overload""" +"#, + ) + .build(); + + assert_snapshot!(test.signature_help_render(), @r" + ============== active signature ============= + (a: int) -> Unknown + --------------------------------------------- + the int overload + + -------------- active parameter ------------- + a: int + --------------------------------------------- + + =============== other signature ============= + (a: str) -> Unknown + --------------------------------------------- + the real implementation! + + -------------- active parameter ------------- + a: str + --------------------------------------------- + "); + } + + #[test] + fn signature_help_overload_type_disambiguated2() { + let test = CursorTest::builder() + .source( + "main.py", + r#" +from mymodule import ab + +ab("hello") +"#, + ) + .source( + "mymodule.py", + r#" +def ab(a): + """the real implementation!""" +"#, + ) + .source( + "mymodule.pyi", + r#" +from typing import overload + +@overload +def ab(a: int): + """the int overload""" + +@overload +def ab(a: str): + """the str overload""" +"#, + ) + .build(); + + assert_snapshot!(test.signature_help_render(), @r" + ============== active signature ============= + (a: int) -> Unknown + --------------------------------------------- + the int overload + + -------------- active parameter ------------- + a: int + --------------------------------------------- + + =============== other signature ============= + (a: str) -> Unknown + --------------------------------------------- + the str overload + + -------------- active parameter ------------- + a: str + --------------------------------------------- + "); + } + + #[test] + fn signature_help_overload_arity_disambiguated1() { + let test = CursorTest::builder() + .source( + "main.py", + " +from mymodule import ab + +ab(1, 2) +", + ) + .source( + "mymodule.py", + r#" +def ab(a, b = None): + """the real implementation!""" +"#, + ) + .source( + "mymodule.pyi", + r#" +from typing import overload + +@overload +def ab(a: int, b: int): + """the two arg overload""" + +@overload +def ab(a: int): + """the one arg overload""" +"#, + ) + .build(); + + assert_snapshot!(test.signature_help_render(), @r" + ============== active signature ============= + (a: int, b: int) -> Unknown + --------------------------------------------- + the two arg overload + + -------------- active parameter ------------- + b: int + --------------------------------------------- + + =============== other signature ============= + (a: int) -> Unknown + --------------------------------------------- + the one arg overload + + (no active parameter specified) + "); + } + + #[test] + fn signature_help_overload_arity_disambiguated2() { + let test = CursorTest::builder() + .source( + "main.py", + " +from mymodule import ab + +ab(1) +", + ) + .source( + "mymodule.py", + r#" +def ab(a, b = None): + """the real implementation!""" +"#, + ) + .source( + "mymodule.pyi", + r#" +from typing import overload + +@overload +def ab(a: int, b: int): + """the two arg overload""" + +@overload +def ab(a: int): + """the one arg overload""" +"#, + ) + .build(); + + assert_snapshot!(test.signature_help_render(), @r" + ============== active signature ============= + (a: int, b: int) -> Unknown + --------------------------------------------- + the two arg overload + + -------------- active parameter ------------- + a: int + --------------------------------------------- + + =============== other signature ============= + (a: int) -> Unknown + --------------------------------------------- + the one arg overload + + -------------- active parameter ------------- + a: int + --------------------------------------------- + "); + } + + #[test] + fn signature_help_overload_keyword_disambiguated1() { + let test = CursorTest::builder() + .source( + "main.py", + " +from mymodule import ab + +ab(1, b=2) +", + ) + .source( + "mymodule.py", + r#" +def ab(a, *, b = None, c = None): + """the real implementation!""" +"#, + ) + .source( + "mymodule.pyi", + r#" +from typing import overload + +@overload +def ab(a: int): + """keywordless overload""" + +@overload +def ab(a: int, *, b: int): + """b overload""" + +@overload +def ab(a: int, *, c: int): + """c overload""" +"#, + ) + .build(); + + assert_snapshot!(test.signature_help_render(), @r" + ============== active signature ============= + (a: int, *, b: int) -> Unknown + --------------------------------------------- + b overload + + -------------- active parameter ------------- + b: int + --------------------------------------------- + + =============== other signature ============= + (a: int) -> Unknown + --------------------------------------------- + keywordless overload + + (no active parameter specified) + =============== other signature ============= + (a: int, *, c: int) -> Unknown + --------------------------------------------- + c overload + + -------------- active parameter ------------- + c: int + --------------------------------------------- + "); + } + + #[test] + fn signature_help_overload_keyword_disambiguated2() { + let test = CursorTest::builder() + .source( + "main.py", + " +from mymodule import ab + +ab(1, c=2) +", + ) + .source( + "mymodule.py", + r#" +def ab(a, *, b = None, c = None): + """the real implementation!""" +"#, + ) + .source( + "mymodule.pyi", + r#" +from typing import overload + +@overload +def ab(a: int): + """keywordless overload""" + +@overload +def ab(a: int, *, b: int): + """b overload""" + +@overload +def ab(a: int, *, c: int): + """c overload""" +"#, + ) + .build(); + + assert_snapshot!(test.signature_help_render(), @r" + ============== active signature ============= + (a: int, *, c: int) -> Unknown + --------------------------------------------- + c overload + + -------------- active parameter ------------- + c: int + --------------------------------------------- + + =============== other signature ============= + (a: int) -> Unknown + --------------------------------------------- + keywordless overload + + (no active parameter specified) + =============== other signature ============= + (a: int, *, b: int) -> Unknown + --------------------------------------------- + b overload + + -------------- active parameter ------------- + b: int + --------------------------------------------- + "); + } + #[test] fn signature_help_class_constructor() { let test = cursor_test( @@ -826,5 +1177,94 @@ mod tests { fn signature_help(&self) -> Option { crate::signature_help::signature_help(&self.db, self.cursor.file, self.cursor.offset) } + + fn signature_help_render(&self) -> String { + use std::fmt::Write; + + let Some(signature_help) = self.signature_help() else { + return "Signature help found no signatures".to_string(); + }; + let active_sig_heading = "\n============== active signature =============\n"; + let second_sig_heading = "\n=============== other signature =============\n"; + let active_arg_heading = "\n-------------- active parameter -------------\n"; + + let mut buf = String::new(); + if let Some(active_signature) = signature_help.active_signature { + let signature = signature_help + .signatures + .get(active_signature) + .expect("failed to find active signature!"); + write!( + &mut buf, + "{heading}{label}{line}{docs}", + heading = active_sig_heading, + label = signature.label, + line = MarkupKind::PlainText.horizontal_line(), + docs = signature + .documentation + .as_ref() + .map(Docstring::render_plaintext) + .unwrap_or_default(), + ) + .unwrap(); + if let Some(active_parameter) = signature.active_parameter { + let parameter = signature + .parameters + .get(active_parameter) + .expect("failed to find active parameter!"); + write!( + &mut buf, + "{heading}{label}{line}{docs}", + heading = active_arg_heading, + label = parameter.label, + line = MarkupKind::PlainText.horizontal_line(), + docs = parameter.documentation.as_deref().unwrap_or_default(), + ) + .unwrap(); + } else { + writeln!(&mut buf, "\n(no active parameter specified)").unwrap(); + } + } else { + writeln!(&mut buf, "\n(no active signature specified)").unwrap(); + } + + for (idx, signature) in signature_help.signatures.iter().enumerate() { + if Some(idx) == signature_help.active_signature { + continue; + } + write!( + &mut buf, + "{heading}{label}{line}{docs}", + heading = second_sig_heading, + label = signature.label, + line = MarkupKind::PlainText.horizontal_line(), + docs = signature + .documentation + .as_ref() + .map(Docstring::render_plaintext) + .unwrap_or_default(), + ) + .unwrap(); + if let Some(active_parameter) = signature.active_parameter { + let parameter = signature + .parameters + .get(active_parameter) + .expect("failed to find active parameter!"); + write!( + &mut buf, + "{heading}{label}{line}{docs}", + heading = active_arg_heading, + label = parameter.label, + line = MarkupKind::PlainText.horizontal_line(), + docs = parameter.documentation.as_deref().unwrap_or_default(), + ) + .unwrap(); + } else { + write!(&mut buf, "\n(no active parameter specified)").unwrap(); + } + } + + buf + } } } From d5e48a0f8055942ad64ec9a45f2b3f71e88d1384 Mon Sep 17 00:00:00 2001 From: chiri Date: Tue, 2 Sep 2025 18:55:24 +0300 Subject: [PATCH 03/20] [`flake8-use-pathlib`] Make `PTH119` and `PTH120` fixes unsafe because they can change behavior (#20118) ## Summary Fixes https://github.com/astral-sh/ruff/issues/20112 ## Test Plan `cargo nextest run flake8_use_pathlib` --------- Co-authored-by: dylwil3 --- .../src/rules/flake8_use_pathlib/helpers.rs | 59 +++++++++++-------- .../rules/os_path_basename.rs | 19 +++++- .../rules/os_path_dirname.rs | 19 +++++- .../rules/os_path_exists.rs | 7 ++- .../rules/os_path_expanduser.rs | 7 ++- .../rules/os_path_getatime.rs | 7 ++- .../rules/os_path_getctime.rs | 7 ++- .../rules/os_path_getmtime.rs | 7 ++- .../rules/os_path_getsize.rs | 7 ++- .../flake8_use_pathlib/rules/os_path_isabs.rs | 7 ++- .../flake8_use_pathlib/rules/os_path_isdir.rs | 6 +- .../rules/os_path_isfile.rs | 6 +- .../rules/os_path_islink.rs | 6 +- .../flake8_use_pathlib/rules/os_readlink.rs | 6 +- .../flake8_use_pathlib/rules/os_remove.rs | 6 +- .../flake8_use_pathlib/rules/os_rmdir.rs | 6 +- .../flake8_use_pathlib/rules/os_unlink.rs | 6 +- ..._pathlib__tests__preview_full_name.py.snap | 2 + ..._pathlib__tests__preview_import_as.py.snap | 2 + ...athlib__tests__preview_import_from.py.snap | 2 + ...lib__tests__preview_import_from_as.py.snap | 2 + 21 files changed, 136 insertions(+), 60 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs index 19a00b26b0..937b6e587b 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/helpers.rs @@ -60,6 +60,7 @@ pub(crate) fn check_os_pathlib_single_arg_calls( fn_argument: &str, fix_enabled: bool, violation: impl Violation, + applicability: Option, ) { if call.arguments.len() != 1 { return; @@ -74,33 +75,39 @@ pub(crate) fn check_os_pathlib_single_arg_calls( let mut diagnostic = checker.report_diagnostic(violation, call.func.range()); - if fix_enabled { - diagnostic.try_set_fix(|| { - let (import_edit, binding) = checker.importer().get_or_import_symbol( - &ImportRequest::import("pathlib", "Path"), - call.start(), - checker.semantic(), - )?; - - let applicability = if checker.comment_ranges().intersects(range) { - Applicability::Unsafe - } else { - Applicability::Safe - }; - - let replacement = if is_pathlib_path_call(checker, arg) { - format!("{arg_code}.{attr}") - } else { - format!("{binding}({arg_code}).{attr}") - }; - - Ok(Fix::applicable_edits( - Edit::range_replacement(replacement, range), - [import_edit], - applicability, - )) - }); + if !fix_enabled { + return; } + + diagnostic.try_set_fix(|| { + let (import_edit, binding) = checker.importer().get_or_import_symbol( + &ImportRequest::import("pathlib", "Path"), + call.start(), + checker.semantic(), + )?; + + let replacement = if is_pathlib_path_call(checker, arg) { + format!("{arg_code}.{attr}") + } else { + format!("{binding}({arg_code}).{attr}") + }; + + let edit = Edit::range_replacement(replacement, range); + + let fix = match applicability { + Some(Applicability::Unsafe) => Fix::unsafe_edits(edit, [import_edit]), + _ => { + let applicability = if checker.comment_ranges().intersects(range) { + Applicability::Unsafe + } else { + Applicability::Safe + }; + Fix::applicable_edits(edit, [import_edit], applicability) + } + }; + + Ok(fix) + }); } pub(crate) fn get_name_expr(expr: &Expr) -> Option<&ast::ExprName> { diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_basename.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_basename.rs index a2c98821a5..b517d00a7b 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_basename.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_basename.rs @@ -1,9 +1,11 @@ +use ruff_diagnostics::Applicability; +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::ExprCall; + use crate::checkers::ast::Checker; use crate::preview::is_fix_os_path_basename_enabled; use crate::rules::flake8_use_pathlib::helpers::check_os_pathlib_single_arg_calls; use crate::{FixAvailability, Violation}; -use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::ExprCall; /// ## What it does /// Checks for uses of `os.path.basename`. @@ -34,7 +36,16 @@ use ruff_python_ast::ExprCall; /// especially on older versions of Python. /// /// ## Fix Safety -/// This rule's fix is marked as unsafe if the replacement would remove comments attached to the original expression. +/// This rule's fix is always marked as unsafe because the replacement is not always semantically +/// equivalent to the original code. In particular, `pathlib` performs path normalization, +/// which can alter the result compared to `os.path.basename`. For example, this normalization: +/// +/// - Collapses consecutive slashes (e.g., `"a//b"` → `"a/b"`). +/// - Removes trailing slashes (e.g., `"a/b/"` → `"a/b"`). +/// - Eliminates `"."` (e.g., `"a/./b"` → `"a/b"`). +/// +/// As a result, code relying on the exact string returned by `os.path.basename` +/// may behave differently after the fix. /// /// ## References /// - [Python documentation: `PurePath.name`](https://docs.python.org/3/library/pathlib.html#pathlib.PurePath.name) @@ -62,6 +73,7 @@ pub(crate) fn os_path_basename(checker: &Checker, call: &ExprCall, segments: &[& if segments != ["os", "path", "basename"] { return; } + check_os_pathlib_single_arg_calls( checker, call, @@ -69,5 +81,6 @@ pub(crate) fn os_path_basename(checker: &Checker, call: &ExprCall, segments: &[& "p", is_fix_os_path_basename_enabled(checker.settings()), OsPathBasename, + Some(Applicability::Unsafe), ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_dirname.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_dirname.rs index 0ba27908af..e8628647a5 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_dirname.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_dirname.rs @@ -1,9 +1,11 @@ +use ruff_diagnostics::Applicability; +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::ExprCall; + use crate::checkers::ast::Checker; use crate::preview::is_fix_os_path_dirname_enabled; use crate::rules::flake8_use_pathlib::helpers::check_os_pathlib_single_arg_calls; use crate::{FixAvailability, Violation}; -use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::ExprCall; /// ## What it does /// Checks for uses of `os.path.dirname`. @@ -29,7 +31,16 @@ use ruff_python_ast::ExprCall; /// ``` /// /// ## Fix Safety -/// This rule's fix is marked as unsafe if the replacement would remove comments attached to the original expression. +/// This rule's fix is always marked as unsafe because the replacement is not always semantically +/// equivalent to the original code. In particular, `pathlib` performs path normalization, +/// which can alter the result compared to `os.path.dirname`. For example, this normalization: +/// +/// - Collapses consecutive slashes (e.g., `"a//b"` → `"a/b"`). +/// - Removes trailing slashes (e.g., `"a/b/"` → `"a/b"`). +/// - Eliminates `"."` (e.g., `"a/./b"` → `"a/b"`). +/// +/// As a result, code relying on the exact string returned by `os.path.dirname` +/// may behave differently after the fix. /// /// ## Known issues /// While using `pathlib` can improve the readability and type safety of your code, @@ -62,6 +73,7 @@ pub(crate) fn os_path_dirname(checker: &Checker, call: &ExprCall, segments: &[&s if segments != ["os", "path", "dirname"] { return; } + check_os_pathlib_single_arg_calls( checker, call, @@ -69,5 +81,6 @@ pub(crate) fn os_path_dirname(checker: &Checker, call: &ExprCall, segments: &[&s "p", is_fix_os_path_dirname_enabled(checker.settings()), OsPathDirname, + Some(Applicability::Unsafe), ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_exists.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_exists.rs index 3e880805e3..53e441883d 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_exists.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_exists.rs @@ -1,9 +1,10 @@ +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::ExprCall; + use crate::checkers::ast::Checker; use crate::preview::is_fix_os_path_exists_enabled; use crate::rules::flake8_use_pathlib::helpers::check_os_pathlib_single_arg_calls; use crate::{FixAvailability, Violation}; -use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::ExprCall; /// ## What it does /// Checks for uses of `os.path.exists`. @@ -62,6 +63,7 @@ pub(crate) fn os_path_exists(checker: &Checker, call: &ExprCall, segments: &[&st if segments != ["os", "path", "exists"] { return; } + check_os_pathlib_single_arg_calls( checker, call, @@ -69,5 +71,6 @@ pub(crate) fn os_path_exists(checker: &Checker, call: &ExprCall, segments: &[&st "path", is_fix_os_path_exists_enabled(checker.settings()), OsPathExists, + None, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_expanduser.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_expanduser.rs index aa6f7cae71..14efef8c90 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_expanduser.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_expanduser.rs @@ -1,9 +1,10 @@ +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::ExprCall; + use crate::checkers::ast::Checker; use crate::preview::is_fix_os_path_expanduser_enabled; use crate::rules::flake8_use_pathlib::helpers::check_os_pathlib_single_arg_calls; use crate::{FixAvailability, Violation}; -use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::ExprCall; /// ## What it does /// Checks for uses of `os.path.expanduser`. @@ -62,6 +63,7 @@ pub(crate) fn os_path_expanduser(checker: &Checker, call: &ExprCall, segments: & if segments != ["os", "path", "expanduser"] { return; } + check_os_pathlib_single_arg_calls( checker, call, @@ -69,5 +71,6 @@ pub(crate) fn os_path_expanduser(checker: &Checker, call: &ExprCall, segments: & "path", is_fix_os_path_expanduser_enabled(checker.settings()), OsPathExpanduser, + None, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getatime.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getatime.rs index a54e79d339..7797ea5745 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getatime.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getatime.rs @@ -1,9 +1,10 @@ +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::ExprCall; + use crate::checkers::ast::Checker; use crate::preview::is_fix_os_path_getatime_enabled; use crate::rules::flake8_use_pathlib::helpers::check_os_pathlib_single_arg_calls; use crate::{FixAvailability, Violation}; -use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::ExprCall; /// ## What it does /// Checks for uses of `os.path.getatime`. @@ -65,6 +66,7 @@ pub(crate) fn os_path_getatime(checker: &Checker, call: &ExprCall, segments: &[& if segments != ["os", "path", "getatime"] { return; } + check_os_pathlib_single_arg_calls( checker, call, @@ -72,5 +74,6 @@ pub(crate) fn os_path_getatime(checker: &Checker, call: &ExprCall, segments: &[& "filename", is_fix_os_path_getatime_enabled(checker.settings()), OsPathGetatime, + None, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getctime.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getctime.rs index 165c6eae94..873a229865 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getctime.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getctime.rs @@ -1,9 +1,10 @@ +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::ExprCall; + use crate::checkers::ast::Checker; use crate::preview::is_fix_os_path_getctime_enabled; use crate::rules::flake8_use_pathlib::helpers::check_os_pathlib_single_arg_calls; use crate::{FixAvailability, Violation}; -use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::ExprCall; /// ## What it does /// Checks for uses of `os.path.getctime`. @@ -66,6 +67,7 @@ pub(crate) fn os_path_getctime(checker: &Checker, call: &ExprCall, segments: &[& if segments != ["os", "path", "getctime"] { return; } + check_os_pathlib_single_arg_calls( checker, call, @@ -73,5 +75,6 @@ pub(crate) fn os_path_getctime(checker: &Checker, call: &ExprCall, segments: &[& "filename", is_fix_os_path_getctime_enabled(checker.settings()), OsPathGetctime, + None, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getmtime.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getmtime.rs index 0eec96ee2a..0d3cda75cd 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getmtime.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getmtime.rs @@ -1,9 +1,10 @@ +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::ExprCall; + use crate::checkers::ast::Checker; use crate::preview::is_fix_os_path_getmtime_enabled; use crate::rules::flake8_use_pathlib::helpers::check_os_pathlib_single_arg_calls; use crate::{FixAvailability, Violation}; -use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::ExprCall; /// ## What it does /// Checks for uses of `os.path.getmtime`. @@ -66,6 +67,7 @@ pub(crate) fn os_path_getmtime(checker: &Checker, call: &ExprCall, segments: &[& if segments != ["os", "path", "getmtime"] { return; } + check_os_pathlib_single_arg_calls( checker, call, @@ -73,5 +75,6 @@ pub(crate) fn os_path_getmtime(checker: &Checker, call: &ExprCall, segments: &[& "filename", is_fix_os_path_getmtime_enabled(checker.settings()), OsPathGetmtime, + None, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getsize.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getsize.rs index ea1cbfd372..fe3baf4241 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getsize.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_getsize.rs @@ -1,9 +1,10 @@ +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::ExprCall; + use crate::checkers::ast::Checker; use crate::preview::is_fix_os_path_getsize_enabled; use crate::rules::flake8_use_pathlib::helpers::check_os_pathlib_single_arg_calls; use crate::{FixAvailability, Violation}; -use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::ExprCall; /// ## What it does /// Checks for uses of `os.path.getsize`. @@ -66,6 +67,7 @@ pub(crate) fn os_path_getsize(checker: &Checker, call: &ExprCall, segments: &[&s if segments != ["os", "path", "getsize"] { return; } + check_os_pathlib_single_arg_calls( checker, call, @@ -73,5 +75,6 @@ pub(crate) fn os_path_getsize(checker: &Checker, call: &ExprCall, segments: &[&s "filename", is_fix_os_path_getsize_enabled(checker.settings()), OsPathGetsize, + None, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isabs.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isabs.rs index 69ccb55e86..355c6987a4 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isabs.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isabs.rs @@ -1,9 +1,10 @@ +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::ExprCall; + use crate::checkers::ast::Checker; use crate::preview::is_fix_os_path_isabs_enabled; use crate::rules::flake8_use_pathlib::helpers::check_os_pathlib_single_arg_calls; use crate::{FixAvailability, Violation}; -use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::ExprCall; /// ## What it does /// Checks for uses of `os.path.isabs`. @@ -61,6 +62,7 @@ pub(crate) fn os_path_isabs(checker: &Checker, call: &ExprCall, segments: &[&str if segments != ["os", "path", "isabs"] { return; } + check_os_pathlib_single_arg_calls( checker, call, @@ -68,5 +70,6 @@ pub(crate) fn os_path_isabs(checker: &Checker, call: &ExprCall, segments: &[&str "s", is_fix_os_path_isabs_enabled(checker.settings()), OsPathIsabs, + None, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isdir.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isdir.rs index 454415fabc..3c8ee3f7a6 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isdir.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isdir.rs @@ -1,9 +1,10 @@ +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::ExprCall; + use crate::checkers::ast::Checker; use crate::preview::is_fix_os_path_isdir_enabled; use crate::rules::flake8_use_pathlib::helpers::check_os_pathlib_single_arg_calls; use crate::{FixAvailability, Violation}; -use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::ExprCall; /// ## What it does /// Checks for uses of `os.path.isdir`. @@ -71,5 +72,6 @@ pub(crate) fn os_path_isdir(checker: &Checker, call: &ExprCall, segments: &[&str "s", is_fix_os_path_isdir_enabled(checker.settings()), OsPathIsdir, + None, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isfile.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isfile.rs index adfabd61c9..a1ed2a5601 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isfile.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_isfile.rs @@ -1,9 +1,10 @@ +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::ExprCall; + use crate::checkers::ast::Checker; use crate::preview::is_fix_os_path_isfile_enabled; use crate::rules::flake8_use_pathlib::helpers::check_os_pathlib_single_arg_calls; use crate::{FixAvailability, Violation}; -use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::ExprCall; /// ## What it does /// Checks for uses of `os.path.isfile`. @@ -71,5 +72,6 @@ pub(crate) fn os_path_isfile(checker: &Checker, call: &ExprCall, segments: &[&st "path", is_fix_os_path_isfile_enabled(checker.settings()), OsPathIsfile, + None, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_islink.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_islink.rs index bfd96e4b4b..5b6c879de9 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_islink.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_path_islink.rs @@ -1,9 +1,10 @@ +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::ExprCall; + use crate::checkers::ast::Checker; use crate::preview::is_fix_os_path_islink_enabled; use crate::rules::flake8_use_pathlib::helpers::check_os_pathlib_single_arg_calls; use crate::{FixAvailability, Violation}; -use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::ExprCall; /// ## What it does /// Checks for uses of `os.path.islink`. @@ -71,5 +72,6 @@ pub(crate) fn os_path_islink(checker: &Checker, call: &ExprCall, segments: &[&st "path", is_fix_os_path_islink_enabled(checker.settings()), OsPathIslink, + None, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_readlink.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_readlink.rs index c70636d558..3cf990b035 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_readlink.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_readlink.rs @@ -1,11 +1,12 @@ +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::{ExprCall, PythonVersion}; + use crate::checkers::ast::Checker; use crate::preview::is_fix_os_readlink_enabled; use crate::rules::flake8_use_pathlib::helpers::{ check_os_pathlib_single_arg_calls, is_keyword_only_argument_non_default, }; use crate::{FixAvailability, Violation}; -use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::{ExprCall, PythonVersion}; /// ## What it does /// Checks for uses of `os.readlink`. @@ -87,5 +88,6 @@ pub(crate) fn os_readlink(checker: &Checker, call: &ExprCall, segments: &[&str]) "path", is_fix_os_readlink_enabled(checker.settings()), OsReadlink, + None, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_remove.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_remove.rs index 63ef37c411..56975ddc3d 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_remove.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_remove.rs @@ -1,11 +1,12 @@ +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::ExprCall; + use crate::checkers::ast::Checker; use crate::preview::is_fix_os_remove_enabled; use crate::rules::flake8_use_pathlib::helpers::{ check_os_pathlib_single_arg_calls, is_keyword_only_argument_non_default, }; use crate::{FixAvailability, Violation}; -use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::ExprCall; /// ## What it does /// Checks for uses of `os.remove`. @@ -82,5 +83,6 @@ pub(crate) fn os_remove(checker: &Checker, call: &ExprCall, segments: &[&str]) { "path", is_fix_os_remove_enabled(checker.settings()), OsRemove, + None, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_rmdir.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_rmdir.rs index 6cc0eef0de..0e0320b6eb 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_rmdir.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_rmdir.rs @@ -1,11 +1,12 @@ +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::ExprCall; + use crate::checkers::ast::Checker; use crate::preview::is_fix_os_rmdir_enabled; use crate::rules::flake8_use_pathlib::helpers::{ check_os_pathlib_single_arg_calls, is_keyword_only_argument_non_default, }; use crate::{FixAvailability, Violation}; -use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::ExprCall; /// ## What it does /// Checks for uses of `os.rmdir`. @@ -82,5 +83,6 @@ pub(crate) fn os_rmdir(checker: &Checker, call: &ExprCall, segments: &[&str]) { "path", is_fix_os_rmdir_enabled(checker.settings()), OsRmdir, + None, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_unlink.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_unlink.rs index 18fbce929c..d071b3cebc 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_unlink.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_unlink.rs @@ -1,11 +1,12 @@ +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::ExprCall; + use crate::checkers::ast::Checker; use crate::preview::is_fix_os_unlink_enabled; use crate::rules::flake8_use_pathlib::helpers::{ check_os_pathlib_single_arg_calls, is_keyword_only_argument_non_default, }; use crate::{FixAvailability, Violation}; -use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::ExprCall; /// ## What it does /// Checks for uses of `os.unlink`. @@ -82,5 +83,6 @@ pub(crate) fn os_unlink(checker: &Checker, call: &ExprCall, segments: &[&str]) { "path", is_fix_os_unlink_enabled(checker.settings()), OsUnlink, + None, ); } diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_full_name.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_full_name.py.snap index 3761d0eded..d5fe727f4a 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_full_name.py.snap +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_full_name.py.snap @@ -466,6 +466,7 @@ help: Replace with `Path(...).name` 30 | os.path.dirname(p) 31 | os.path.samefile(p) 32 | os.path.splitext(p) +note: This is an unsafe fix and may change runtime behavior PTH120 [*] `os.path.dirname()` should be replaced by `Path.parent` --> full_name.py:29:1 @@ -493,6 +494,7 @@ help: Replace with `Path(...).parent` 31 | os.path.samefile(p) 32 | os.path.splitext(p) 33 | with open(p) as fp: +note: This is an unsafe fix and may change runtime behavior PTH121 `os.path.samefile()` should be replaced by `Path.samefile()` --> full_name.py:30:1 diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_as.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_as.py.snap index 8f240afbab..00d6ccde82 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_as.py.snap +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_as.py.snap @@ -466,6 +466,7 @@ help: Replace with `Path(...).name` 30 | foo_p.dirname(p) 31 | foo_p.samefile(p) 32 | foo_p.splitext(p) +note: This is an unsafe fix and may change runtime behavior PTH120 [*] `os.path.dirname()` should be replaced by `Path.parent` --> import_as.py:29:1 @@ -492,6 +493,7 @@ help: Replace with `Path(...).parent` 30 + pathlib.Path(p).parent 31 | foo_p.samefile(p) 32 | foo_p.splitext(p) +note: This is an unsafe fix and may change runtime behavior PTH121 `os.path.samefile()` should be replaced by `Path.samefile()` --> import_as.py:30:1 diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from.py.snap index bacf13158a..db73ba81ee 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from.py.snap +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from.py.snap @@ -480,6 +480,7 @@ help: Replace with `Path(...).name` 32 | dirname(p) 33 | samefile(p) 34 | splitext(p) +note: This is an unsafe fix and may change runtime behavior PTH120 [*] `os.path.dirname()` should be replaced by `Path.parent` --> import_from.py:31:1 @@ -508,6 +509,7 @@ help: Replace with `Path(...).parent` 33 | samefile(p) 34 | splitext(p) 35 | with open(p) as fp: +note: This is an unsafe fix and may change runtime behavior PTH121 `os.path.samefile()` should be replaced by `Path.samefile()` --> import_from.py:32:1 diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from_as.py.snap b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from_as.py.snap index 0722ac76b1..23e111498c 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from_as.py.snap +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/snapshots/ruff_linter__rules__flake8_use_pathlib__tests__preview_import_from_as.py.snap @@ -480,6 +480,7 @@ help: Replace with `Path(...).name` 37 | xdirname(p) 38 | xsamefile(p) 39 | xsplitext(p) +note: This is an unsafe fix and may change runtime behavior PTH120 [*] `os.path.dirname()` should be replaced by `Path.parent` --> import_from_as.py:36:1 @@ -507,6 +508,7 @@ help: Replace with `Path(...).parent` 37 + pathlib.Path(p).parent 38 | xsamefile(p) 39 | xsplitext(p) +note: This is an unsafe fix and may change runtime behavior PTH121 `os.path.samefile()` should be replaced by `Path.samefile()` --> import_from_as.py:37:1 From ec5584219e7c3863d68c473964de4c649ce5aedf Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Tue, 2 Sep 2025 14:49:14 -0400 Subject: [PATCH 04/20] [ty] Make initializer calls GotoTargets (#20014) This introduces `GotoTarget::Call` that represents the kind of ambiguous/overloaded click of a callable-being-called: ```py x = mymodule.MyClass(1, 2) ^^^^^^^ ``` This is equivalent to `GotoTarget::Expression` for the same span but enriched with information about the actual callable implementation. That is, if you click on `MyClass` in `MyClass()` it is *both* a reference to the class and to the initializer of the class. Therefore it would be ideal for goto-* and docstrings to be some intelligent merging of both the class and the initializer. In particular the callable-implementation (initializer) is prioritized over the callable-itself (class) so when showing docstrings we will preferentially show the docs of the initializer if it exists, and then fallback to the docs of the class. For goto-definition/goto-declaration we will yield both the class and the initializer, requiring you to pick which you want (this is perhaps needlessly pedantic but...). Fixes https://github.com/astral-sh/ty/issues/898 Fixes https://github.com/astral-sh/ty/issues/1010 --- crates/ty_ide/src/goto.rs | 122 +++++++++++++++++++++---- crates/ty_ide/src/goto_declaration.rs | 17 ++++ crates/ty_ide/src/goto_definition.rs | 16 ++++ crates/ty_ide/src/hover.rs | 125 +++++++++++++++++++++++++- 4 files changed, 261 insertions(+), 19 deletions(-) diff --git a/crates/ty_ide/src/goto.rs b/crates/ty_ide/src/goto.rs index 52af38896b..bb085e3c3c 100644 --- a/crates/ty_ide/src/goto.rs +++ b/crates/ty_ide/src/goto.rs @@ -8,14 +8,15 @@ use std::borrow::Cow; use crate::find_node::covering_node; use crate::stub_mapping::StubMapper; use ruff_db::parsed::ParsedModuleRef; +use ruff_python_ast::ExprCall; use ruff_python_ast::{self as ast, AnyNodeRef}; use ruff_python_parser::TokenKind; use ruff_text_size::{Ranged, TextRange, TextSize}; use ty_python_semantic::HasDefinition; use ty_python_semantic::ImportAliasResolution; use ty_python_semantic::ResolvedDefinition; -use ty_python_semantic::types::Type; use ty_python_semantic::types::definitions_for_keyword_argument; +use ty_python_semantic::types::{Type, call_signature_details}; use ty_python_semantic::{ HasType, SemanticModel, definitions_for_imported_symbol, definitions_for_name, }; @@ -145,6 +146,26 @@ pub(crate) enum GotoTarget<'a> { Globals { identifier: &'a ast::Identifier, }, + /// Go to on the invocation of a callable + /// + /// ```py + /// x = mymodule.MyClass(1, 2) + /// ^^^^^^^ + /// ``` + /// + /// This is equivalent to `GotoTarget::Expression(callable)` but enriched + /// with information about the actual callable implementation. + /// + /// That is, if you click on `MyClass` in `MyClass()` it is *both* a + /// reference to the class and to the initializer of the class. Therefore + /// it would be ideal for goto-* and docstrings to be some intelligent + /// merging of both the class and the initializer. + Call { + /// The callable that can actually be selected by a cursor + callable: ast::ExprRef<'a>, + /// The call of the callable + call: &'a ExprCall, + }, } /// The resolved definitions for a `GotoTarget` @@ -258,6 +279,9 @@ impl GotoTarget<'_> { GotoTarget::ImportModuleAlias { alias } => alias.inferred_type(model), GotoTarget::ExceptVariable(except) => except.inferred_type(model), GotoTarget::KeywordArgument { keyword, .. } => keyword.value.inferred_type(model), + // When asking the type of a callable, usually you want the callable itself? + // (i.e. the type of `MyClass` in `MyClass()` is `` and not `() -> MyClass`) + GotoTarget::Call { callable, .. } => callable.inferred_type(model), // TODO: Support identifier targets GotoTarget::PatternMatchRest(_) | GotoTarget::PatternKeywordArgument(_) @@ -293,18 +317,10 @@ impl GotoTarget<'_> { alias_resolution: ImportAliasResolution, ) -> Option> { use crate::NavigationTarget; - use ruff_python_ast as ast; match self { - GotoTarget::Expression(expression) => match expression { - ast::ExprRef::Name(name) => Some(DefinitionsOrTargets::Definitions( - definitions_for_name(db, file, name), - )), - ast::ExprRef::Attribute(attribute) => Some(DefinitionsOrTargets::Definitions( - ty_python_semantic::definitions_for_attribute(db, file, attribute), - )), - _ => None, - }, + GotoTarget::Expression(expression) => definitions_for_expression(db, file, expression) + .map(DefinitionsOrTargets::Definitions), // For already-defined symbols, they are their own definitions GotoTarget::FunctionDef(function) => { @@ -417,6 +433,22 @@ impl GotoTarget<'_> { } } + // For callables, both the definition of the callable and the actual function impl are relevant. + // + // Prefer the function impl over the callable so that its docstrings win if defined. + GotoTarget::Call { callable, call } => { + let mut definitions = definitions_for_callable(db, file, call); + let expr_definitions = + definitions_for_expression(db, file, callable).unwrap_or_default(); + definitions.extend(expr_definitions); + + if definitions.is_empty() { + None + } else { + Some(DefinitionsOrTargets::Definitions(definitions)) + } + } + _ => None, } } @@ -427,7 +459,11 @@ impl GotoTarget<'_> { /// to this goto target. pub(crate) fn to_string(&self) -> Option> { match self { - GotoTarget::Expression(expression) => match expression { + GotoTarget::Call { + callable: expression, + .. + } + | GotoTarget::Expression(expression) => match expression { ast::ExprRef::Name(name) => Some(Cow::Borrowed(name.id.as_str())), ast::ExprRef::Attribute(attr) => Some(Cow::Borrowed(attr.attr.as_str())), _ => None, @@ -627,7 +663,18 @@ impl GotoTarget<'_> { Some(GotoTarget::TypeParamTypeVarTupleName(var_tuple)) } Some(AnyNodeRef::ExprAttribute(attribute)) => { - Some(GotoTarget::Expression(attribute.into())) + // Check if this is seemingly a callable being invoked (the `y` in `x.y(...)`) + let grandparent_expr = covering_node.ancestors().nth(2); + let attribute_expr = attribute.into(); + if let Some(AnyNodeRef::ExprCall(call)) = grandparent_expr { + if ruff_python_ast::ExprRef::from(&call.func) == attribute_expr { + return Some(GotoTarget::Call { + call, + callable: attribute_expr, + }); + } + } + Some(GotoTarget::Expression(attribute_expr)) } Some(AnyNodeRef::StmtNonlocal(_)) => Some(GotoTarget::NonLocal { identifier }), Some(AnyNodeRef::StmtGlobal(_)) => Some(GotoTarget::Globals { identifier }), @@ -641,7 +688,19 @@ impl GotoTarget<'_> { } }, - node => node.as_expr_ref().map(GotoTarget::Expression), + node => { + // Check if this is seemingly a callable being invoked (the `x` in `x(...)`) + let parent = covering_node.parent(); + if let (Some(AnyNodeRef::ExprCall(call)), AnyNodeRef::ExprName(name)) = + (parent, node) + { + return Some(GotoTarget::Call { + call, + callable: name.into(), + }); + } + node.as_expr_ref().map(GotoTarget::Expression) + } } } } @@ -649,7 +708,11 @@ impl GotoTarget<'_> { impl Ranged for GotoTarget<'_> { fn range(&self) -> TextRange { match self { - GotoTarget::Expression(expression) => match expression { + GotoTarget::Call { + callable: expression, + .. + } + | GotoTarget::Expression(expression) => match expression { ast::ExprRef::Attribute(attribute) => attribute.attr.range, _ => expression.range(), }, @@ -711,6 +774,35 @@ fn convert_resolved_definitions_to_targets( .collect() } +/// Shared helper to get definitions for an expr (that is presumably a name/attr) +fn definitions_for_expression<'db>( + db: &'db dyn crate::Db, + file: ruff_db::files::File, + expression: &ruff_python_ast::ExprRef<'_>, +) -> Option>> { + match expression { + ast::ExprRef::Name(name) => Some(definitions_for_name(db, file, name)), + ast::ExprRef::Attribute(attribute) => Some(ty_python_semantic::definitions_for_attribute( + db, file, attribute, + )), + _ => None, + } +} + +fn definitions_for_callable<'db>( + db: &'db dyn crate::Db, + file: ruff_db::files::File, + call: &ExprCall, +) -> Vec> { + let model = SemanticModel::new(db, file); + // Attempt to refine to a specific call + let signature_info = call_signature_details(db, &model, call); + signature_info + .into_iter() + .filter_map(|signature| signature.definition.map(ResolvedDefinition::Definition)) + .collect() +} + /// Shared helper to map and convert resolved definitions into navigation targets. fn definitions_to_navigation_targets<'db>( db: &dyn crate::Db, diff --git a/crates/ty_ide/src/goto_declaration.rs b/crates/ty_ide/src/goto_declaration.rs index 2e25862f59..bd67246b03 100644 --- a/crates/ty_ide/src/goto_declaration.rs +++ b/crates/ty_ide/src/goto_declaration.rs @@ -123,6 +123,23 @@ mod tests { | 4 | pass 5 | + 6 | instance = MyClass() + | ^^^^^^^ + | + + info[goto-declaration]: Declaration + --> main.py:3:9 + | + 2 | class MyClass: + 3 | def __init__(self): + | ^^^^^^^^ + 4 | pass + | + info: Source + --> main.py:6:12 + | + 4 | pass + 5 | 6 | instance = MyClass() | ^^^^^^^ | diff --git a/crates/ty_ide/src/goto_definition.rs b/crates/ty_ide/src/goto_definition.rs index 55156488fc..fb165f61a0 100644 --- a/crates/ty_ide/src/goto_definition.rs +++ b/crates/ty_ide/src/goto_definition.rs @@ -466,6 +466,22 @@ class MyOtherClass: --> main.py:3:5 | 2 | from mymodule import MyClass + 3 | x = MyClass(0) + | ^^^^^^^ + | + + info[goto-definition]: Definition + --> mymodule.py:3:9 + | + 2 | class MyClass: + 3 | def __init__(self, val): + | ^^^^^^^^ + 4 | self.val = val + | + info: Source + --> main.py:3:5 + | + 2 | from mymodule import MyClass 3 | x = MyClass(0) | ^^^^^^^ | diff --git a/crates/ty_ide/src/hover.rs b/crates/ty_ide/src/hover.rs index fbfc57041e..0867983e3a 100644 --- a/crates/ty_ide/src/hover.rs +++ b/crates/ty_ide/src/hover.rs @@ -465,6 +465,123 @@ mod tests { "#, ); + assert_snapshot!(test.hover(), @r" + + --------------------------------------------- + initializes MyClass (perfectly) + + --------------------------------------------- + ```python + + ``` + --- + ```text + initializes MyClass (perfectly) + + ``` + --------------------------------------------- + info[hover]: Hovered content is + --> main.py:24:5 + | + 22 | return 0 + 23 | + 24 | x = MyClass(0) + | ^^^^^-^ + | | | + | | Cursor offset + | source + | + "); + } + + #[test] + fn hover_class_init_attr() { + let test = CursorTest::builder() + .source( + "mymod.py", + r#" + class MyClass: + ''' + This is such a great class!! + + Don't you know? + + Everyone loves my class!! + + ''' + def __init__(self, val): + """initializes MyClass (perfectly)""" + self.val = val + "#, + ) + .source( + "main.py", + r#" + import mymod + + x = mymod.MyClass(0) + "#, + ) + .build(); + + assert_snapshot!(test.hover(), @r" + + --------------------------------------------- + initializes MyClass (perfectly) + + --------------------------------------------- + ```python + + ``` + --- + ```text + initializes MyClass (perfectly) + + ``` + --------------------------------------------- + info[hover]: Hovered content is + --> main.py:4:11 + | + 2 | import mymod + 3 | + 4 | x = mymod.MyClass(0) + | ^^^^^-^ + | | | + | | Cursor offset + | source + | + "); + } + + #[test] + fn hover_class_init_no_init_docs() { + let test = cursor_test( + r#" + class MyClass: + ''' + This is such a great class!! + + Don't you know? + + Everyone loves my class!! + + ''' + def __init__(self, val): + self.val = val + + def my_method(self, a, b): + '''This is such a great func!! + + Args: + a: first for a reason + b: coming for `a`'s title + ''' + return 0 + + x = MyClass(0) + "#, + ); + assert_snapshot!(test.hover(), @r" --------------------------------------------- @@ -489,11 +606,11 @@ mod tests { ``` --------------------------------------------- info[hover]: Hovered content is - --> main.py:24:5 + --> main.py:23:5 | - 22 | return 0 - 23 | - 24 | x = MyClass(0) + 21 | return 0 + 22 | + 23 | x = MyClass(0) | ^^^^^-^ | | | | | Cursor offset From 00214fc60c770d6e3011b05030d84d8452ca40d6 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 3 Sep 2025 12:28:34 +0200 Subject: [PATCH 05/20] [ty] Update ecosystem-analyzer (#20209) This pulls in some new changes, like the possibility to run on failing projects (timeouts, abnormal exits). --- .github/workflows/ty-ecosystem-analyzer.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ty-ecosystem-analyzer.yaml b/.github/workflows/ty-ecosystem-analyzer.yaml index 615c5e0997..9c954bf774 100644 --- a/.github/workflows/ty-ecosystem-analyzer.yaml +++ b/.github/workflows/ty-ecosystem-analyzer.yaml @@ -64,7 +64,7 @@ jobs: cd .. - uv tool install "git+https://github.com/astral-sh/ecosystem-analyzer@27dd66d9e397d986ef9c631119ee09556eab8af9" + uv tool install "git+https://github.com/astral-sh/ecosystem-analyzer@1f560d07d672effae250e3d271da53d96c5260ff" ecosystem-analyzer \ --repository ruff \ From 4e97b97a76734f345c2a0e9408f04cc8ed1d5473 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 3 Sep 2025 12:51:30 +0200 Subject: [PATCH 06/20] [ty] Run ecosystem-analyzer in release mode (#20210) ## Summary We already have `mypy_primer` running in debug mode, so this should provide some additional coverage, and allows us produce reasonable timing results. ## Test Plan CI run on this PR --- .github/workflows/ty-ecosystem-analyzer.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ty-ecosystem-analyzer.yaml b/.github/workflows/ty-ecosystem-analyzer.yaml index 9c954bf774..9f3d8b7db8 100644 --- a/.github/workflows/ty-ecosystem-analyzer.yaml +++ b/.github/workflows/ty-ecosystem-analyzer.yaml @@ -69,6 +69,7 @@ jobs: ecosystem-analyzer \ --repository ruff \ diff \ + --profile=release \ --projects-old ruff/projects_old.txt \ --projects-new ruff/projects_new.txt \ --old old_commit \ From aee9350df13a230276a6dfebe488485f82476a48 Mon Sep 17 00:00:00 2001 From: Brent Westbrook <36778786+ntBre@users.noreply.github.com> Date: Wed, 3 Sep 2025 09:08:12 -0400 Subject: [PATCH 07/20] [ty] Add GitLab output format (#20155) ## Summary This wires up the GitLab output format moved into `ruff_db` in https://github.com/astral-sh/ruff/pull/20117 to the ty CLI. While I was here, I made one unrelated change to the CLI docs. Clap was rendering the escapes around the `\[default\]` brackets for the `full` output, so I just switched those to parentheses: ``` --output-format The format to use for printing diagnostic messages Possible values: - full: Print diagnostics verbosely, with context and helpful hints \[default\] - concise: Print diagnostics concisely, one per line - gitlab: Print diagnostics in the JSON format expected by GitLab Code Quality reports ``` ## Test Plan New CLI test, and a manual test with `--config 'terminal.output-format = "gitlab"'` to make sure this works as a configuration option too. I also tried piping the output through jq to make sure it's at least valid JSON --- crates/ruff_db/src/diagnostic/mod.rs | 2 +- crates/ty/docs/cli.md | 3 +- crates/ty/src/args.rs | 6 ++- crates/ty/src/lib.rs | 51 +++++++++++------- crates/ty/tests/cli/main.rs | 65 +++++++++++++++++++++++ crates/ty_project/src/metadata/options.rs | 16 ++++++ ty.schema.json | 7 +++ 7 files changed, 127 insertions(+), 23 deletions(-) diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index 774b341f8b..c44dce2116 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -1444,7 +1444,7 @@ pub enum DiagnosticFormat { Junit, /// Print diagnostics in the JSON format used by GitLab [Code Quality] reports. /// - /// [Code Quality]: https://docs.gitlab.com/ee/ci/testing/code_quality.html#implement-a-custom-tool + /// [Code Quality]: https://docs.gitlab.com/ci/testing/code_quality/#code-quality-report-format #[cfg(feature = "serde")] Gitlab, } diff --git a/crates/ty/docs/cli.md b/crates/ty/docs/cli.md index 9fff1e8876..c02bb6ea4b 100644 --- a/crates/ty/docs/cli.md +++ b/crates/ty/docs/cli.md @@ -60,8 +60,9 @@ over all configuration files.

--output-format output-format

The format to use for printing diagnostic messages

Possible values:

    -
  • full: Print diagnostics verbosely, with context and helpful hints [default]
    • +
  • full: Print diagnostics verbosely, with context and helpful hints (default)
  • concise: Print diagnostics concisely, one per line
    • +
  • gitlab: Print diagnostics in the JSON format expected by GitLab Code Quality reports
--project project

Run the command within the given project directory.

All pyproject.toml files will be discovered by walking up the directory tree from the given project directory, as will the project's virtual environment (.venv) unless the venv-path option is set.

Other command-line arguments (such as relative paths) will be resolved relative to the current working directory.

diff --git a/crates/ty/src/args.rs b/crates/ty/src/args.rs index f518e028db..1a2eabf5f3 100644 --- a/crates/ty/src/args.rs +++ b/crates/ty/src/args.rs @@ -306,7 +306,7 @@ impl clap::Args for RulesArg { /// The diagnostic output format. #[derive(Copy, Clone, Hash, Debug, PartialEq, Eq, PartialOrd, Ord, Default, clap::ValueEnum)] pub enum OutputFormat { - /// Print diagnostics verbosely, with context and helpful hints \[default\]. + /// Print diagnostics verbosely, with context and helpful hints (default). /// /// Diagnostic messages may include additional context and /// annotations on the input to help understand the message. @@ -321,6 +321,9 @@ pub enum OutputFormat { /// dropped. #[value(name = "concise")] Concise, + /// Print diagnostics in the JSON format expected by GitLab Code Quality reports. + #[value(name = "gitlab")] + Gitlab, } impl From for ty_project::metadata::options::OutputFormat { @@ -328,6 +331,7 @@ impl From for ty_project::metadata::options::OutputFormat { match format { OutputFormat::Full => Self::Full, OutputFormat::Concise => Self::Concise, + OutputFormat::Gitlab => Self::Gitlab, } } } diff --git a/crates/ty/src/lib.rs b/crates/ty/src/lib.rs index 3e11a3bedf..4e3c5993d0 100644 --- a/crates/ty/src/lib.rs +++ b/crates/ty/src/lib.rs @@ -21,7 +21,7 @@ use clap::{CommandFactory, Parser}; use colored::Colorize; use crossbeam::channel as crossbeam_channel; use rayon::ThreadPoolBuilder; -use ruff_db::diagnostic::{Diagnostic, DisplayDiagnosticConfig, Severity}; +use ruff_db::diagnostic::{Diagnostic, DisplayDiagnosticConfig, DisplayDiagnostics, Severity}; use ruff_db::files::File; use ruff_db::max_parallelism; use ruff_db::system::{OsSystem, SystemPath, SystemPathBuf}; @@ -319,37 +319,48 @@ impl MainLoop { return Ok(ExitStatus::Success); } + let is_human_readable = terminal_settings.output_format.is_human_readable(); + if result.is_empty() { - writeln!( - self.printer.stream_for_success_summary(), - "{}", - "All checks passed!".green().bold() - )?; + if is_human_readable { + writeln!( + self.printer.stream_for_success_summary(), + "{}", + "All checks passed!".green().bold() + )?; + } if self.watcher.is_none() { return Ok(ExitStatus::Success); } } else { - let mut max_severity = Severity::Info; let diagnostics_count = result.len(); let mut stdout = self.printer.stream_for_details().lock(); - for diagnostic in result { - // Only render diagnostics if they're going to be displayed, since doing - // so is expensive. - if stdout.is_enabled() { - write!(stdout, "{}", diagnostic.display(db, &display_config))?; - } + let max_severity = result + .iter() + .map(Diagnostic::severity) + .max() + .unwrap_or(Severity::Info); - max_severity = max_severity.max(diagnostic.severity()); + // Only render diagnostics if they're going to be displayed, since doing + // so is expensive. + if stdout.is_enabled() { + write!( + stdout, + "{}", + DisplayDiagnostics::new(db, &display_config, &result) + )?; } - writeln!( - self.printer.stream_for_failure_summary(), - "Found {} diagnostic{}", - diagnostics_count, - if diagnostics_count > 1 { "s" } else { "" } - )?; + if is_human_readable { + writeln!( + self.printer.stream_for_failure_summary(), + "Found {} diagnostic{}", + diagnostics_count, + if diagnostics_count > 1 { "s" } else { "" } + )?; + } if max_severity.is_fatal() { tracing::warn!( diff --git a/crates/ty/tests/cli/main.rs b/crates/ty/tests/cli/main.rs index f85f294842..70d2859f8d 100644 --- a/crates/ty/tests/cli/main.rs +++ b/crates/ty/tests/cli/main.rs @@ -618,6 +618,71 @@ fn concise_diagnostics() -> anyhow::Result<()> { Ok(()) } +#[test] +fn gitlab_diagnostics() -> anyhow::Result<()> { + let case = CliTest::with_file( + "test.py", + r#" + print(x) # [unresolved-reference] + print(4[1]) # [non-subscriptable] + "#, + )?; + + let mut settings = insta::Settings::clone_current(); + settings.add_filter(r#"("fingerprint": ")[a-z0-9]+(",)"#, "$1[FINGERPRINT]$2"); + let _s = settings.bind_to_scope(); + + assert_cmd_snapshot!(case.command().arg("--output-format=gitlab").arg("--warn").arg("unresolved-reference"), @r#" + success: false + exit_code: 1 + ----- stdout ----- + [ + { + "check_name": "unresolved-reference", + "description": "unresolved-reference: Name `x` used when not defined", + "severity": "minor", + "fingerprint": "[FINGERPRINT]", + "location": { + "path": "test.py", + "positions": { + "begin": { + "line": 2, + "column": 7 + }, + "end": { + "line": 2, + "column": 8 + } + } + } + }, + { + "check_name": "non-subscriptable", + "description": "non-subscriptable: Cannot subscript object of type `Literal[4]` with no `__getitem__` method", + "severity": "major", + "fingerprint": "[FINGERPRINT]", + "location": { + "path": "test.py", + "positions": { + "begin": { + "line": 3, + "column": 7 + }, + "end": { + "line": 3, + "column": 8 + } + } + } + } + ] + ----- stderr ----- + WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors. + "#); + + Ok(()) +} + /// This tests the diagnostic format for revealed type. /// /// This test was introduced because changes were made to diff --git a/crates/ty_project/src/metadata/options.rs b/crates/ty_project/src/metadata/options.rs index 6e5d614de4..1ec0bb2edb 100644 --- a/crates/ty_project/src/metadata/options.rs +++ b/crates/ty_project/src/metadata/options.rs @@ -1055,6 +1055,21 @@ pub enum OutputFormat { /// /// This may use color when printing to a `tty`. Concise, + /// Print diagnostics in the JSON format expected by GitLab [Code Quality] reports. + /// + /// [Code Quality]: https://docs.gitlab.com/ci/testing/code_quality/#code-quality-report-format + Gitlab, +} + +impl OutputFormat { + /// Returns `true` if this format is intended for users to read directly, in contrast to + /// machine-readable or structured formats. + /// + /// This can be used to check whether information beyond the diagnostics, such as a header or + /// `Found N diagnostics` footer, should be included. + pub const fn is_human_readable(&self) -> bool { + matches!(self, OutputFormat::Full | OutputFormat::Concise) + } } impl From for DiagnosticFormat { @@ -1062,6 +1077,7 @@ impl From for DiagnosticFormat { match value { OutputFormat::Full => Self::Full, OutputFormat::Concise => Self::Concise, + OutputFormat::Gitlab => Self::Gitlab, } } } diff --git a/ty.schema.json b/ty.schema.json index a9261dfefb..45cc92b023 100644 --- a/ty.schema.json +++ b/ty.schema.json @@ -164,6 +164,13 @@ "enum": [ "concise" ] + }, + { + "description": "Print diagnostics in the JSON format expected by GitLab [Code Quality] reports.\n\n[Code Quality]: https://docs.gitlab.com/ci/testing/code_quality/#code-quality-report-format", + "type": "string", + "enum": [ + "gitlab" + ] } ] }, From 3b913ce6524ed3ccd30d9d8fe3763737fc819e69 Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Wed, 3 Sep 2025 21:22:56 +0800 Subject: [PATCH 08/20] [`airflow`] Improve the `AIR002` error message (#20173) ## Summary ### What Change the message from "DAG should have an explicit `schedule` argument" to "`DAG` or `@dag` should have an explicit `schedule` argument" ### Why We're trying to get rid of the idea that DAG in airflow was Directed acyclic graph. Thus, change it to refer to the class `DAG` or the decorator `@dag` might help a bit. ## Test Plan update the test fixtures accordly --- .../src/rules/airflow/rules/dag_schedule_argument.rs | 2 +- .../ruff_linter__rules__airflow__tests__AIR002_AIR002.py.snap | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/ruff_linter/src/rules/airflow/rules/dag_schedule_argument.rs b/crates/ruff_linter/src/rules/airflow/rules/dag_schedule_argument.rs index c89071f120..efb7a69f13 100644 --- a/crates/ruff_linter/src/rules/airflow/rules/dag_schedule_argument.rs +++ b/crates/ruff_linter/src/rules/airflow/rules/dag_schedule_argument.rs @@ -46,7 +46,7 @@ pub(crate) struct AirflowDagNoScheduleArgument; impl Violation for AirflowDagNoScheduleArgument { #[derive_message_formats] fn message(&self) -> String { - "DAG should have an explicit `schedule` argument".to_string() + "`DAG` or `@dag` should have an explicit `schedule` argument".to_string() } } diff --git a/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR002_AIR002.py.snap b/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR002_AIR002.py.snap index 2ca71e8bfa..9b13f0b310 100644 --- a/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR002_AIR002.py.snap +++ b/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR002_AIR002.py.snap @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/airflow/mod.rs --- -AIR002 DAG should have an explicit `schedule` argument +AIR002 `DAG` or `@dag` should have an explicit `schedule` argument --> AIR002.py:4:1 | 2 | from airflow.timetables.simple import NullTimetable @@ -12,7 +12,7 @@ AIR002 DAG should have an explicit `schedule` argument 6 | DAG(dag_id="class_schedule", schedule="@hourly") | -AIR002 DAG should have an explicit `schedule` argument +AIR002 `DAG` or `@dag` should have an explicit `schedule` argument --> AIR002.py:13:2 | 13 | @dag() From 9cea75293469530afa6416d70bce9b933b8eea9b Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Tue, 26 Aug 2025 14:55:19 -0400 Subject: [PATCH 09/20] [ty] Require that we can find a root when listing sub-modules This is similar to a change made in the "list top-level modules" implementation that had been masked by poor Salsa failure modes. Basically, if we can't find a root here, it *must* be a bug. And if we just silently skip over it, we risk voiding Salsa's purity contract, leading to more difficult to debug panics. This did cause one test to fail, but only because the test wasn't properly setting up roots. --- Cargo.lock | 1 + crates/ty_ide/Cargo.toml | 2 +- crates/ty_ide/src/lib.rs | 17 ++++++++++++++++- .../src/module_resolver/module.rs | 8 +++++--- 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1a1af3dcb1..27e1808bb4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4241,6 +4241,7 @@ name = "ty_ide" version = "0.0.0" dependencies = [ "bitflags 2.9.3", + "camino", "get-size2", "insta", "itertools 0.14.0", diff --git a/crates/ty_ide/Cargo.toml b/crates/ty_ide/Cargo.toml index e00b8db200..08fbb7eedb 100644 --- a/crates/ty_ide/Cargo.toml +++ b/crates/ty_ide/Cargo.toml @@ -33,7 +33,7 @@ smallvec = { workspace = true } tracing = { workspace = true } [dev-dependencies] - +camino = { workspace = true } insta = { workspace = true, features = ["filters"] } [lints] diff --git a/crates/ty_ide/src/lib.rs b/crates/ty_ide/src/lib.rs index 6baea75a8f..b695ee5804 100644 --- a/crates/ty_ide/src/lib.rs +++ b/crates/ty_ide/src/lib.rs @@ -286,10 +286,12 @@ impl HasNavigationTargets for TypeDefinition<'_> { #[cfg(test)] mod tests { + use camino::Utf8Component; use insta::internals::SettingsBindDropGuard; + use ruff_db::Db; use ruff_db::diagnostic::{Diagnostic, DiagnosticFormat, DisplayDiagnosticConfig}; - use ruff_db::files::{File, system_path_to_file}; + use ruff_db::files::{File, FileRootKind, system_path_to_file}; use ruff_db::system::{DbWithWritableSystem, SystemPath, SystemPathBuf}; use ruff_python_trivia::textwrap::dedent; use ruff_text_size::TextSize; @@ -378,6 +380,19 @@ mod tests { db.write_file(path, contents) .expect("write to memory file system to be successful"); + // Add a root for the top-most component. + let top = path.components().find_map(|c| match c { + Utf8Component::Normal(c) => Some(c), + _ => None, + }); + if let Some(top) = top { + let top = SystemPath::new(top); + if db.system().is_directory(top) { + db.files() + .try_add_root(&db, top, FileRootKind::LibrarySearchPath); + } + } + let file = system_path_to_file(&db, path).expect("newly written file to existing"); if let Some(offset) = cursor_offset { diff --git a/crates/ty_python_semantic/src/module_resolver/module.rs b/crates/ty_python_semantic/src/module_resolver/module.rs index 3586f52933..fdafa64323 100644 --- a/crates/ty_python_semantic/src/module_resolver/module.rs +++ b/crates/ty_python_semantic/src/module_resolver/module.rs @@ -161,9 +161,11 @@ fn all_submodule_names_for_package(db: &dyn Db, file: File) -> Option> // tree. When the revision gets bumped, the cache // that Salsa creates does for this routine will be // invalidated. - if let Some(root) = db.files().root(db, parent_directory) { - let _ = root.revision(db); - } + let root = db + .files() + .root(db, parent_directory) + .expect("System search path should have a registered root"); + let _ = root.revision(db); db.system() .read_directory(parent_directory) From 046893c1861ef6d77e81b7f341a90e0fa34572b4 Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Thu, 28 Aug 2025 10:53:53 -0400 Subject: [PATCH 10/20] [ty] Make `Module::all_submodules` return `Module` instead of `Name` This is to facilitate recursive traversal of all modules in an environment. This way, we can keep asking for submodules. This also simplifies how this is used in completions, and probably makes it faster. Namely, since we return the `Module` itself, callers don't need to invoke the full module resolver just to get the module type. Note that this doesn't include namespace packages. (Which were previously not supported in `Module::all_submodules`.) Given how they can be spread out across multiple search paths, they will likely require special consideration here. --- crates/ty/tests/file_watching.rs | 2 +- .../src/module_resolver/module.rs | 94 ++++++++++++++----- .../ty_python_semantic/src/semantic_model.rs | 15 +-- 3 files changed, 74 insertions(+), 37 deletions(-) diff --git a/crates/ty/tests/file_watching.rs b/crates/ty/tests/file_watching.rs index e40722d395..ba16bc8c3f 100644 --- a/crates/ty/tests/file_watching.rs +++ b/crates/ty/tests/file_watching.rs @@ -240,7 +240,7 @@ impl TestCase { .module(parent_module_name) .all_submodules(self.db()) .iter() - .map(|name| name.as_str().to_string()) + .map(|submodule| submodule.name(self.db()).to_string()) .collect::>(); names.sort(); names diff --git a/crates/ty_python_semantic/src/module_resolver/module.rs b/crates/ty_python_semantic/src/module_resolver/module.rs index fdafa64323..7d61e65818 100644 --- a/crates/ty_python_semantic/src/module_resolver/module.rs +++ b/crates/ty_python_semantic/src/module_resolver/module.rs @@ -1,9 +1,9 @@ use std::fmt::Formatter; use std::str::FromStr; -use ruff_db::files::File; -use ruff_python_ast::name::Name; -use ruff_python_stdlib::identifiers::is_identifier; +use ruff_db::files::{File, system_path_to_file, vendored_path_to_file}; +use ruff_db::system::SystemPath; +use ruff_db::vendored::VendoredPath; use salsa::Database; use salsa::plumbing::AsId; @@ -97,23 +97,10 @@ impl<'db> Module<'db> { /// /// The names returned correspond to the "base" name of the module. /// That is, `{self.name}.{basename}` should give the full module name. - pub fn all_submodules(self, db: &'db dyn Db) -> &'db [Name] { - self.all_submodules_inner(db).unwrap_or_default() - } - - fn all_submodules_inner(self, db: &'db dyn Db) -> Option<&'db [Name]> { - // It would be complex and expensive to compute all submodules for - // namespace packages, since a namespace package doesn't correspond - // to a single file; it can span multiple directories across multiple - // search paths. For now, we only compute submodules for traditional - // packages that exist in a single directory on a single search path. - let Module::File(module) = self else { - return None; - }; - if !matches!(module.kind(db), ModuleKind::Package) { - return None; - } - all_submodule_names_for_package(db, module.file(db)).as_deref() + pub fn all_submodules(self, db: &'db dyn Db) -> &'db [Module<'db>] { + all_submodule_names_for_package(db, self) + .as_deref() + .unwrap_or_default() } } @@ -134,7 +121,10 @@ impl std::fmt::Debug for Module<'_> { #[allow(clippy::ref_option)] #[salsa::tracked(returns(ref))] -fn all_submodule_names_for_package(db: &dyn Db, file: File) -> Option> { +fn all_submodule_names_for_package<'db>( + db: &'db dyn Db, + module: Module<'db>, +) -> Option>> { fn is_submodule( is_dir: bool, is_file: bool, @@ -147,7 +137,31 @@ fn all_submodule_names_for_package(db: &dyn Db, file: File) -> Option> && !matches!(basename, Some("__init__.py" | "__init__.pyi"))) } - let path = SystemOrVendoredPathRef::try_from_file(db, file)?; + fn find_package_init_system(db: &dyn Db, dir: &SystemPath) -> Option { + system_path_to_file(db, dir.join("__init__.pyi")) + .or_else(|_| system_path_to_file(db, dir.join("__init__.py"))) + .ok() + } + + fn find_package_init_vendored(db: &dyn Db, dir: &VendoredPath) -> Option { + vendored_path_to_file(db, dir.join("__init__.pyi")) + .or_else(|_| vendored_path_to_file(db, dir.join("__init__.py"))) + .ok() + } + + // It would be complex and expensive to compute all submodules for + // namespace packages, since a namespace package doesn't correspond + // to a single file; it can span multiple directories across multiple + // search paths. For now, we only compute submodules for traditional + // packages that exist in a single directory on a single search path. + let Module::File(module) = module else { + return None; + }; + if !matches!(module.kind(db), ModuleKind::Package) { + return None; + } + + let path = SystemOrVendoredPathRef::try_from_file(db, module.file(db))?; debug_assert!( matches!(path.file_name(), Some("__init__.py" | "__init__.pyi")), "expected package file `{:?}` to be `__init__.py` or `__init__.pyi`", @@ -189,7 +203,23 @@ fn all_submodule_names_for_package(db: &dyn Db, file: File) -> Option> }) .filter_map(|entry| { let stem = entry.path().file_stem()?; - is_identifier(stem).then(|| Name::from(stem)) + let name = ModuleName::new(stem)?; + let (kind, file) = if entry.file_type().is_directory() { + ( + ModuleKind::Package, + find_package_init_system(db, entry.path())?, + ) + } else { + let file = system_path_to_file(db, entry.path()).ok()?; + (ModuleKind::Module, file) + }; + Some(Module::file_module( + db, + name, + kind, + module.search_path(db).clone(), + file, + )) }) .collect() } @@ -209,7 +239,23 @@ fn all_submodule_names_for_package(db: &dyn Db, file: File) -> Option> }) .filter_map(|entry| { let stem = entry.path().file_stem()?; - is_identifier(stem).then(|| Name::from(stem)) + let name = ModuleName::new(stem)?; + let (kind, file) = if entry.file_type().is_directory() { + ( + ModuleKind::Package, + find_package_init_vendored(db, entry.path())?, + ) + } else { + let file = vendored_path_to_file(db, entry.path()).ok()?; + (ModuleKind::Module, file) + }; + Some(Module::file_module( + db, + name, + kind, + module.search_path(db).clone(), + file, + )) }) .collect(), }) diff --git a/crates/ty_python_semantic/src/semantic_model.rs b/crates/ty_python_semantic/src/semantic_model.rs index 47eacbb3a5..ba633b5d22 100644 --- a/crates/ty_python_semantic/src/semantic_model.rs +++ b/crates/ty_python_semantic/src/semantic_model.rs @@ -173,19 +173,10 @@ impl<'db> SemanticModel<'db> { let builtin = module.is_known(self.db, KnownModule::Builtins); let mut completions = vec![]; - for submodule_basename in module.all_submodules(self.db) { - let Some(basename) = ModuleName::new(submodule_basename.as_str()) else { - continue; - }; - let mut submodule_name = module.name(self.db).clone(); - submodule_name.extend(&basename); - - let Some(submodule) = resolve_module(self.db, &submodule_name) else { - continue; - }; - let ty = Type::module_literal(self.db, self.file, submodule); + for submodule in module.all_submodules(self.db) { + let ty = Type::module_literal(self.db, self.file, *submodule); completions.push(Completion { - name: submodule_basename.clone(), + name: Name::new(submodule.name(self.db).as_str()), ty, builtin, }); From 78db56e36242e0f7a66897712c68ee3de108ec21 Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Tue, 26 Aug 2025 14:22:57 -0400 Subject: [PATCH 11/20] [ty] Add base for "all symbols" implementation This just copies the existing "workspace symbols" implementation and re-names some things. --- crates/ty_ide/src/all_symbols.rs | 177 +++++++++++++++++++++++++++++++ crates/ty_ide/src/lib.rs | 2 + 2 files changed, 179 insertions(+) create mode 100644 crates/ty_ide/src/all_symbols.rs diff --git a/crates/ty_ide/src/all_symbols.rs b/crates/ty_ide/src/all_symbols.rs new file mode 100644 index 0000000000..6b78a8c367 --- /dev/null +++ b/crates/ty_ide/src/all_symbols.rs @@ -0,0 +1,177 @@ +use crate::symbols::{QueryPattern, SymbolInfo, symbols_for_file_global_only}; +use ruff_db::files::File; +use ty_project::Db; + +/// Get all symbols matching the query string. +/// +/// Returns symbols from all files in the workspace and dependencies, filtered +/// by the query. +pub fn all_symbols(db: &dyn Db, query: &str) -> Vec { + // If the query is empty, return immediately to avoid expensive file scanning + if query.is_empty() { + return Vec::new(); + } + + let project = db.project(); + + let query = QueryPattern::new(query); + let files = project.files(db); + let results = std::sync::Mutex::new(Vec::new()); + { + let db = db.dyn_clone(); + let files = &files; + let results = &results; + let query = &query; + + rayon::scope(move |s| { + // For each file, extract symbols and add them to results + for file in files.iter() { + let db = db.dyn_clone(); + s.spawn(move |_| { + for (_, symbol) in symbols_for_file_global_only(&*db, *file).search(query) { + // It seems like we could do better here than + // locking `results` for every single symbol, + // but this works pretty well as it is. + results.lock().unwrap().push(AllSymbolInfo { + symbol: symbol.to_owned(), + file: *file, + }); + } + }); + } + }); + } + + results.into_inner().unwrap() +} + +/// A symbol found in the workspace and dependencies, including the +/// file it was found in. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AllSymbolInfo { + /// The symbol information + pub symbol: SymbolInfo<'static>, + /// The file containing the symbol + pub file: File, +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::tests::CursorTest; + use crate::tests::IntoDiagnostic; + use insta::assert_snapshot; + use ruff_db::diagnostic::{ + Annotation, Diagnostic, DiagnosticId, LintName, Severity, Span, SubDiagnostic, + SubDiagnosticSeverity, + }; + + #[test] + fn test_all_symbols_multi_file() { + // We use odd symbol names here so that we can + // write queries that target them specifically + // and (hopefully) nothing else. + let test = CursorTest::builder() + .source( + "utils.py", + " +def abcdefghijklmnop(): + '''A helpful utility function''' + pass +", + ) + .source( + "models.py", + " +class Abcdefghijklmnop: + '''A data model class''' + def __init__(self): + pass +", + ) + .source( + "constants.py", + " +ABCDEFGHIJKLMNOP = 'https://api.example.com' +", + ) + .build(); + + assert_snapshot!(test.all_symbols("ufunc"), @r" + info[all-symbols]: AllSymbolInfo + --> utils.py:2:5 + | + 2 | ABCDEFGHIJKLMNOP = 'https://api.example.com' + | ^^^^^^^^^^^^^^^^ + | + info: Function utility_function + "); + + assert_snapshot!(test.all_symbols("data"), @r" + info[all-symbols]: AllSymbolInfo + --> models.py:2:7 + | + 2 | class Abcdefghijklmnop: + | ^^^^^^^^^^^^^^^^ + 3 | '''A data model class''' + 4 | def __init__(self): + | + info: Class DataModel + "); + + info[all-symbols]: AllSymbolInfo + --> constants.py:2:1 + | + 2 | def abcdefghijklmnop(): + | ^^^^^^^^^^^^^^^^ + 3 | '''A helpful utility function''' + 4 | pass + | + info: Function abcdefghijklmnop + "); + } + + impl CursorTest { + fn all_symbols(&self, query: &str) -> String { + let symbols = all_symbols(&self.db, query); + + if symbols.is_empty() { + return "No symbols found".to_string(); + } + + self.render_diagnostics(symbols.into_iter().map(AllSymbolDiagnostic::new)) + } + } + + struct AllSymbolDiagnostic { + symbol_info: AllSymbolInfo, + } + + impl AllSymbolDiagnostic { + fn new(symbol_info: AllSymbolInfo) -> Self { + Self { symbol_info } + } + } + + impl IntoDiagnostic for AllSymbolDiagnostic { + fn into_diagnostic(self) -> Diagnostic { + let symbol_kind_str = self.symbol_info.symbol.kind.to_string(); + + let info_text = format!("{} {}", symbol_kind_str, self.symbol_info.symbol.name); + + let sub = SubDiagnostic::new(SubDiagnosticSeverity::Info, info_text); + + let mut main = Diagnostic::new( + DiagnosticId::Lint(LintName::of("all-symbols")), + Severity::Info, + "AllSymbolInfo".to_string(), + ); + main.annotate(Annotation::primary( + Span::from(self.symbol_info.file).with_range(self.symbol_info.symbol.name_range), + )); + main.sub(sub); + + main + } + } +} diff --git a/crates/ty_ide/src/lib.rs b/crates/ty_ide/src/lib.rs index b695ee5804..b23c54d462 100644 --- a/crates/ty_ide/src/lib.rs +++ b/crates/ty_ide/src/lib.rs @@ -2,6 +2,7 @@ clippy::disallowed_methods, reason = "Prefer System trait methods over std methods in ty crates" )] +mod all_symbols; mod completion; mod doc_highlights; mod docstring; @@ -24,6 +25,7 @@ mod stub_mapping; mod symbols; mod workspace_symbols; +pub use all_symbols::{AllSymbolInfo, all_symbols}; pub use completion::completion; pub use doc_highlights::document_highlights; pub use document_symbols::document_symbols; From 8e52027a884f9383dd5d13cdc21baf5c800337fc Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Thu, 28 Aug 2025 14:52:50 -0400 Subject: [PATCH 12/20] [ty] Add naive implementation of completions for unimported symbols This re-works the `all_symbols` based added previously to work across all modules available, and not just what is directly in the workspace. Note that we always pass an empty string as a query, which makes the results always empty. We'll fix this in a subsequent commit. --- crates/ty_ide/src/all_symbols.rs | 39 +++++++++++-------- crates/ty_ide/src/completion.rs | 28 +++++++++++-- crates/ty_ide/src/symbols.rs | 22 +++++++++++ crates/ty_python_semantic/src/lib.rs | 4 +- .../src/module_resolver/list.rs | 14 +++++++ .../src/module_resolver/mod.rs | 2 +- .../ty_python_semantic/src/semantic_model.rs | 39 +++++++++++++++---- .../src/server/api/requests/completion.rs | 2 +- crates/ty_wasm/src/lib.rs | 5 ++- 9 files changed, 121 insertions(+), 34 deletions(-) diff --git a/crates/ty_ide/src/all_symbols.rs b/crates/ty_ide/src/all_symbols.rs index 6b78a8c367..02ed56d6db 100644 --- a/crates/ty_ide/src/all_symbols.rs +++ b/crates/ty_ide/src/all_symbols.rs @@ -1,6 +1,8 @@ -use crate::symbols::{QueryPattern, SymbolInfo, symbols_for_file_global_only}; use ruff_db::files::File; use ty_project::Db; +use ty_python_semantic::all_modules; + +use crate::symbols::{QueryPattern, SymbolInfo, symbols_for_file_global_only}; /// Get all symbols matching the query string. /// @@ -12,29 +14,29 @@ pub fn all_symbols(db: &dyn Db, query: &str) -> Vec { return Vec::new(); } - let project = db.project(); - let query = QueryPattern::new(query); - let files = project.files(db); let results = std::sync::Mutex::new(Vec::new()); { + let modules = all_modules(db); let db = db.dyn_clone(); - let files = &files; let results = &results; let query = &query; rayon::scope(move |s| { // For each file, extract symbols and add them to results - for file in files.iter() { + for module in modules { let db = db.dyn_clone(); + let Some(file) = module.file(&*db) else { + continue; + }; s.spawn(move |_| { - for (_, symbol) in symbols_for_file_global_only(&*db, *file).search(query) { + for (_, symbol) in symbols_for_file_global_only(&*db, file).search(query) { // It seems like we could do better here than // locking `results` for every single symbol, // but this works pretty well as it is. results.lock().unwrap().push(AllSymbolInfo { symbol: symbol.to_owned(), - file: *file, + file, }); } }); @@ -42,7 +44,13 @@ pub fn all_symbols(db: &dyn Db, query: &str) -> Vec { }); } - results.into_inner().unwrap() + let mut results = results.into_inner().unwrap(); + results.sort_by(|s1, s2| { + let key1 = (&s1.symbol.name, s1.file.path(db).as_str()); + let key2 = (&s2.symbol.name, s2.file.path(db).as_str()); + key1.cmp(&key2) + }); + results } /// A symbol found in the workspace and dependencies, including the @@ -97,17 +105,15 @@ ABCDEFGHIJKLMNOP = 'https://api.example.com' ) .build(); - assert_snapshot!(test.all_symbols("ufunc"), @r" + assert_snapshot!(test.all_symbols("acegikmo"), @r" info[all-symbols]: AllSymbolInfo - --> utils.py:2:5 + --> constants.py:2:1 | 2 | ABCDEFGHIJKLMNOP = 'https://api.example.com' | ^^^^^^^^^^^^^^^^ | - info: Function utility_function - "); + info: Constant ABCDEFGHIJKLMNOP - assert_snapshot!(test.all_symbols("data"), @r" info[all-symbols]: AllSymbolInfo --> models.py:2:7 | @@ -116,11 +122,10 @@ ABCDEFGHIJKLMNOP = 'https://api.example.com' 3 | '''A data model class''' 4 | def __init__(self): | - info: Class DataModel - "); + info: Class Abcdefghijklmnop info[all-symbols]: AllSymbolInfo - --> constants.py:2:1 + --> utils.py:2:5 | 2 | def abcdefghijklmnop(): | ^^^^^^^^^^^^^^^^ diff --git a/crates/ty_ide/src/completion.rs b/crates/ty_ide/src/completion.rs index 11f648ebd7..4d9959e9e2 100644 --- a/crates/ty_ide/src/completion.rs +++ b/crates/ty_ide/src/completion.rs @@ -7,10 +7,10 @@ use ruff_python_parser::{Token, TokenAt, TokenKind}; use ruff_text_size::{Ranged, TextRange, TextSize}; use ty_python_semantic::{Completion, NameKind, SemanticModel}; -use crate::Db; use crate::docstring::Docstring; use crate::find_node::covering_node; use crate::goto::DefinitionsOrTargets; +use crate::{Db, all_symbols}; pub fn completion(db: &dyn Db, file: File, offset: TextSize) -> Vec> { let parsed = parsed_module(db, file).load(db); @@ -37,14 +37,27 @@ pub fn completion(db: &dyn Db, file: File, offset: TextSize) -> Vec { model.import_completions() } - CompletionTargetAst::Scoped { node } => model.scoped_completions(node), + CompletionTargetAst::Scoped { node } => { + let mut completions = model.scoped_completions(node); + for symbol in all_symbols(db, "") { + completions.push(Completion { + name: ast::name::Name::new(&symbol.symbol.name), + ty: None, + kind: symbol.symbol.kind.to_completion_kind(), + builtin: false, + }); + } + completions + } }; completions.sort_by(compare_suggestions); completions.dedup_by(|c1, c2| c1.name == c2.name); completions .into_iter() .map(|completion| { - let definition = DefinitionsOrTargets::from_ty(db, completion.ty); + let definition = completion + .ty + .and_then(|ty| DefinitionsOrTargets::from_ty(db, ty)); let documentation = definition.and_then(|def| def.docstring(db)); DetailedCompletion { inner: completion, @@ -3040,7 +3053,14 @@ from os. fn completions_without_builtins_with_types(&self) -> String { self.completions_if_snapshot( |c| !c.builtin, - |c| format!("{} :: {}", c.name, c.ty.display(&self.db)), + |c| { + format!( + "{} :: {}", + c.name, + c.ty.map(|ty| ty.display(&self.db).to_string()) + .unwrap_or_else(|| "Unavailable".to_string()) + ) + }, ) } diff --git a/crates/ty_ide/src/symbols.rs b/crates/ty_ide/src/symbols.rs index 646c89537b..6ba3d7101d 100644 --- a/crates/ty_ide/src/symbols.rs +++ b/crates/ty_ide/src/symbols.rs @@ -13,6 +13,7 @@ use ruff_python_ast::visitor::source_order::{self, SourceOrderVisitor}; use ruff_python_ast::{Expr, Stmt}; use ruff_text_size::{Ranged, TextRange}; use ty_project::Db; +use ty_python_semantic::CompletionKind; /// A compiled query pattern used for searching symbols. /// @@ -282,6 +283,27 @@ impl SymbolKind { SymbolKind::Import => "Import", } } + + /// Maps this to a "completion" kind if a sensible mapping exists. + pub fn to_completion_kind(self) -> Option { + Some(match self { + SymbolKind::Module => CompletionKind::Module, + SymbolKind::Class => CompletionKind::Class, + SymbolKind::Method => CompletionKind::Method, + SymbolKind::Function => CompletionKind::Function, + SymbolKind::Variable => CompletionKind::Variable, + SymbolKind::Constant => CompletionKind::Constant, + SymbolKind::Property => CompletionKind::Property, + SymbolKind::Field => CompletionKind::Field, + SymbolKind::Constructor => CompletionKind::Constructor, + SymbolKind::Parameter => CompletionKind::Variable, + SymbolKind::TypeParameter => CompletionKind::TypeParameter, + // Not quite sure what to do with this one. I guess + // in theory the import should be "resolved" to its + // underlying kind, but that seems expensive. + SymbolKind::Import => return None, + }) + } } /// Returns a flat list of symbols in the file given. diff --git a/crates/ty_python_semantic/src/lib.rs b/crates/ty_python_semantic/src/lib.rs index cf58ab93a6..48a2cc9eb1 100644 --- a/crates/ty_python_semantic/src/lib.rs +++ b/crates/ty_python_semantic/src/lib.rs @@ -11,8 +11,8 @@ use crate::suppression::{INVALID_IGNORE_COMMENT, UNKNOWN_RULE, UNUSED_IGNORE_COM pub use db::Db; pub use module_name::ModuleName; pub use module_resolver::{ - Module, SearchPath, SearchPathValidationError, SearchPaths, list_modules, resolve_module, - resolve_real_module, system_module_search_paths, + Module, SearchPath, SearchPathValidationError, SearchPaths, all_modules, list_modules, + resolve_module, resolve_real_module, system_module_search_paths, }; pub use program::{ Program, ProgramSettings, PythonVersionFileSource, PythonVersionSource, diff --git a/crates/ty_python_semantic/src/module_resolver/list.rs b/crates/ty_python_semantic/src/module_resolver/list.rs index 0d0f95140c..bcbad13534 100644 --- a/crates/ty_python_semantic/src/module_resolver/list.rs +++ b/crates/ty_python_semantic/src/module_resolver/list.rs @@ -12,6 +12,20 @@ use super::resolver::{ ModuleResolveMode, ResolverContext, is_non_shadowable, resolve_file_module, search_paths, }; +/// List all available modules, including all sub-modules, sorted in lexicographic order. +pub fn all_modules(db: &dyn Db) -> Vec> { + let mut modules = list_modules(db); + let mut stack = modules.clone(); + while let Some(module) = stack.pop() { + for &submodule in module.all_submodules(db) { + modules.push(submodule); + stack.push(submodule); + } + } + modules.sort_by_key(|module| module.name(db)); + modules +} + /// List all available top-level modules. #[salsa::tracked] pub fn list_modules(db: &dyn Db) -> Vec> { diff --git a/crates/ty_python_semantic/src/module_resolver/mod.rs b/crates/ty_python_semantic/src/module_resolver/mod.rs index 549ffa2c96..9beec6b385 100644 --- a/crates/ty_python_semantic/src/module_resolver/mod.rs +++ b/crates/ty_python_semantic/src/module_resolver/mod.rs @@ -1,6 +1,6 @@ use std::iter::FusedIterator; -pub use list::list_modules; +pub use list::{all_modules, list_modules}; pub(crate) use module::KnownModule; pub use module::Module; pub use path::{SearchPath, SearchPathValidationError}; diff --git a/crates/ty_python_semantic/src/semantic_model.rs b/crates/ty_python_semantic/src/semantic_model.rs index ba633b5d22..fc6bf60c4b 100644 --- a/crates/ty_python_semantic/src/semantic_model.rs +++ b/crates/ty_python_semantic/src/semantic_model.rs @@ -50,7 +50,8 @@ impl<'db> SemanticModel<'db> { let ty = Type::module_literal(self.db, self.file, module); Completion { name: Name::new(module.name(self.db).as_str()), - ty, + ty: Some(ty), + kind: None, builtin, } }) @@ -162,7 +163,12 @@ impl<'db> SemanticModel<'db> { let mut completions = vec![]; for crate::types::Member { name, ty } in crate::types::all_members(self.db, ty) { - completions.push(Completion { name, ty, builtin }); + completions.push(Completion { + name, + ty: Some(ty), + kind: None, + builtin, + }); } completions.extend(self.submodule_completions(&module)); completions @@ -177,7 +183,8 @@ impl<'db> SemanticModel<'db> { let ty = Type::module_literal(self.db, self.file, *submodule); completions.push(Completion { name: Name::new(submodule.name(self.db).as_str()), - ty, + ty: Some(ty), + kind: None, builtin, }); } @@ -191,7 +198,8 @@ impl<'db> SemanticModel<'db> { .into_iter() .map(|member| Completion { name: member.name, - ty: member.ty, + ty: Some(member.ty), + kind: None, builtin: false, }) .collect() @@ -227,7 +235,8 @@ impl<'db> SemanticModel<'db> { all_declarations_and_bindings(self.db, file_scope.to_scope_id(self.db, self.file)) .map(|member| Completion { name: member.name, - ty: member.ty, + ty: Some(member.ty), + kind: None, builtin: false, }), ); @@ -277,8 +286,22 @@ impl NameKind { pub struct Completion<'db> { /// The label shown to the user for this suggestion. pub name: Name, - /// The type of this completion. - pub ty: Type<'db>, + /// The type of this completion, if available. + /// + /// Generally speaking, this is always available + /// *unless* this was a completion corresponding to + /// an unimported symbol. In that case, computing the + /// type of all such symbols could be quite expensive. + pub ty: Option>, + /// The "kind" of this completion. + /// + /// When this is set, it takes priority over any kind + /// inferred from `ty`. + /// + /// Usually this is set when `ty` is `None`, since it + /// may be cheaper to compute at scale. (e.g., For + /// unimported symbol completions.) + pub kind: Option, /// Whether this suggestion came from builtins or not. /// /// At time of writing (2025-06-26), this information @@ -336,7 +359,7 @@ impl<'db> Completion<'db> { Type::TypeAlias(alias) => imp(db, alias.value_type(db))?, }) } - imp(db, self.ty) + self.kind.or_else(|| self.ty.and_then(|ty| imp(db, ty))) } } diff --git a/crates/ty_server/src/server/api/requests/completion.rs b/crates/ty_server/src/server/api/requests/completion.rs index e175ed49fe..c2c5d49e4e 100644 --- a/crates/ty_server/src/server/api/requests/completion.rs +++ b/crates/ty_server/src/server/api/requests/completion.rs @@ -71,7 +71,7 @@ impl BackgroundDocumentRequestHandler for CompletionRequestHandler { label: comp.inner.name.into(), kind, sort_text: Some(format!("{i:-max_index_len$}")), - detail: comp.inner.ty.display(db).to_string().into(), + detail: comp.inner.ty.map(|ty| ty.display(db).to_string()), documentation: comp .documentation .map(|docstring| Documentation::String(docstring.render_plaintext())), diff --git a/crates/ty_wasm/src/lib.rs b/crates/ty_wasm/src/lib.rs index 8722afcbe7..cf88ce6392 100644 --- a/crates/ty_wasm/src/lib.rs +++ b/crates/ty_wasm/src/lib.rs @@ -425,7 +425,10 @@ impl Workspace { documentation: completion .documentation .map(|documentation| documentation.render_plaintext()), - detail: completion.inner.ty.display(&self.db).to_string().into(), + detail: completion + .inner + .ty + .map(|ty| ty.display(&self.db).to_string()), }) .collect()) } From 0a0eaf5a9bc3f3ee69e2fa6614a09b8abba0b7f4 Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Fri, 29 Aug 2025 14:44:56 -0400 Subject: [PATCH 13/20] [ty] Make auto-import completions opt-in via an experimental option Instead of waiting to land auto-import until it is "ready for users," it'd be nicer to get incremental progress merged to `main`. By making it an experimental opt-in, we avoid making the default completion experience worse but permit developers and motivated users to try it. --- crates/ty_ide/src/completion.rs | 39 +++++++++++++------ crates/ty_ide/src/lib.rs | 2 +- .../src/server/api/requests/completion.rs | 7 +++- crates/ty_server/src/session.rs | 7 ++++ crates/ty_server/src/session/options.rs | 13 +++++++ crates/ty_server/src/session/settings.rs | 5 +++ crates/ty_wasm/src/lib.rs | 6 ++- 7 files changed, 63 insertions(+), 16 deletions(-) diff --git a/crates/ty_ide/src/completion.rs b/crates/ty_ide/src/completion.rs index 4d9959e9e2..80239b18e1 100644 --- a/crates/ty_ide/src/completion.rs +++ b/crates/ty_ide/src/completion.rs @@ -12,7 +12,17 @@ use crate::find_node::covering_node; use crate::goto::DefinitionsOrTargets; use crate::{Db, all_symbols}; -pub fn completion(db: &dyn Db, file: File, offset: TextSize) -> Vec> { +#[derive(Clone, Debug, Default)] +pub struct CompletionSettings { + pub auto_import: bool, +} + +pub fn completion<'db>( + db: &'db dyn Db, + settings: &CompletionSettings, + file: File, + offset: TextSize, +) -> Vec> { let parsed = parsed_module(db, file).load(db); let Some(target_token) = CompletionTargetTokens::find(&parsed, offset) else { @@ -39,13 +49,15 @@ pub fn completion(db: &dyn Db, file: File, offset: TextSize) -> Vec { let mut completions = model.scoped_completions(node); - for symbol in all_symbols(db, "") { - completions.push(Completion { - name: ast::name::Name::new(&symbol.symbol.name), - ty: None, - kind: symbol.symbol.kind.to_completion_kind(), - builtin: false, - }); + if settings.auto_import { + for symbol in all_symbols(db, "") { + completions.push(Completion { + name: ast::name::Name::new(&symbol.symbol.name), + ty: None, + kind: symbol.symbol.kind.to_completion_kind(), + builtin: false, + }); + } } completions } @@ -518,7 +530,7 @@ mod tests { use crate::completion::{DetailedCompletion, completion}; use crate::tests::{CursorTest, cursor_test}; - use super::token_suffix_by_kinds; + use super::{CompletionSettings, token_suffix_by_kinds}; #[test] fn token_suffixes_match() { @@ -3073,7 +3085,8 @@ from os. predicate: impl Fn(&DetailedCompletion) -> bool, snapshot: impl Fn(&DetailedCompletion) -> String, ) -> String { - let completions = completion(&self.db, self.cursor.file, self.cursor.offset); + let settings = CompletionSettings::default(); + let completions = completion(&self.db, &settings, self.cursor.file, self.cursor.offset); if completions.is_empty() { return "".to_string(); } @@ -3096,7 +3109,8 @@ from os. #[track_caller] fn assert_completions_include(&self, expected: &str) { - let completions = completion(&self.db, self.cursor.file, self.cursor.offset); + let settings = CompletionSettings::default(); + let completions = completion(&self.db, &settings, self.cursor.file, self.cursor.offset); assert!( completions @@ -3108,7 +3122,8 @@ from os. #[track_caller] fn assert_completions_do_not_include(&self, unexpected: &str) { - let completions = completion(&self.db, self.cursor.file, self.cursor.offset); + let settings = CompletionSettings::default(); + let completions = completion(&self.db, &settings, self.cursor.file, self.cursor.offset); assert!( completions diff --git a/crates/ty_ide/src/lib.rs b/crates/ty_ide/src/lib.rs index b23c54d462..c481c5ad14 100644 --- a/crates/ty_ide/src/lib.rs +++ b/crates/ty_ide/src/lib.rs @@ -26,7 +26,7 @@ mod symbols; mod workspace_symbols; pub use all_symbols::{AllSymbolInfo, all_symbols}; -pub use completion::completion; +pub use completion::{CompletionSettings, completion}; pub use doc_highlights::document_highlights; pub use document_symbols::document_symbols; pub use goto::{goto_declaration, goto_definition, goto_type_definition}; diff --git a/crates/ty_server/src/server/api/requests/completion.rs b/crates/ty_server/src/server/api/requests/completion.rs index c2c5d49e4e..4307ad6dbc 100644 --- a/crates/ty_server/src/server/api/requests/completion.rs +++ b/crates/ty_server/src/server/api/requests/completion.rs @@ -7,7 +7,7 @@ use lsp_types::{ }; use ruff_db::source::{line_index, source_text}; use ruff_source_file::OneIndexed; -use ty_ide::completion; +use ty_ide::{CompletionSettings, completion}; use ty_project::ProjectDatabase; use ty_python_semantic::CompletionKind; @@ -55,7 +55,10 @@ impl BackgroundDocumentRequestHandler for CompletionRequestHandler { &line_index, snapshot.encoding(), ); - let completions = completion(db, file, offset); + let settings = CompletionSettings { + auto_import: snapshot.global_settings().is_auto_import_enabled(), + }; + let completions = completion(db, &settings, file, offset); if completions.is_empty() { return Ok(None); } diff --git a/crates/ty_server/src/session.rs b/crates/ty_server/src/session.rs index 4c53371c24..5e8b84ecf6 100644 --- a/crates/ty_server/src/session.rs +++ b/crates/ty_server/src/session.rs @@ -826,6 +826,7 @@ impl Session { .map_err(DocumentQueryError::InvalidUrl); DocumentSnapshot { resolved_client_capabilities: self.resolved_client_capabilities, + global_settings: self.global_settings.clone(), workspace_settings: key .as_ref() .ok() @@ -1000,6 +1001,7 @@ impl Drop for MutIndexGuard<'_> { #[derive(Debug)] pub(crate) struct DocumentSnapshot { resolved_client_capabilities: ResolvedClientCapabilities, + global_settings: Arc, workspace_settings: Arc, position_encoding: PositionEncoding, document_query_result: Result, @@ -1016,6 +1018,11 @@ impl DocumentSnapshot { self.position_encoding } + /// Returns the client settings for all workspaces. + pub(crate) fn global_settings(&self) -> &GlobalSettings { + &self.global_settings + } + /// Returns the client settings for the workspace that this document belongs to. pub(crate) fn workspace_settings(&self) -> &WorkspaceSettings { &self.workspace_settings diff --git a/crates/ty_server/src/session/options.rs b/crates/ty_server/src/session/options.rs index bf5f048f9f..e890f05fef 100644 --- a/crates/ty_server/src/session/options.rs +++ b/crates/ty_server/src/session/options.rs @@ -122,6 +122,12 @@ impl ClientOptions { self } + #[must_use] + pub fn with_experimental_auto_import(mut self, enabled: bool) -> Self { + self.global.experimental.get_or_insert_default().auto_import = Some(enabled); + self + } + #[must_use] pub fn with_unknown(mut self, unknown: HashMap) -> Self { self.unknown = unknown; @@ -149,6 +155,7 @@ impl GlobalOptions { .experimental .map(|experimental| ExperimentalSettings { rename: experimental.rename.unwrap_or(true), + auto_import: experimental.auto_import.unwrap_or(false), }) .unwrap_or_default(); @@ -293,6 +300,12 @@ impl Combine for DiagnosticMode { pub(crate) struct Experimental { /// Whether to enable the experimental symbol rename feature. pub(crate) rename: Option, + /// Whether to enable the experimental "auto-import" feature. + /// + /// At time of writing (2025-08-29), this feature is still + /// under active development. It may not work right or may be + /// incomplete. + pub(crate) auto_import: Option, } #[derive(Clone, Debug, Serialize, Deserialize, Default)] diff --git a/crates/ty_server/src/session/settings.rs b/crates/ty_server/src/session/settings.rs index 9f54c7069d..7b3e95f3ed 100644 --- a/crates/ty_server/src/session/settings.rs +++ b/crates/ty_server/src/session/settings.rs @@ -14,6 +14,10 @@ impl GlobalSettings { pub(crate) fn is_rename_enabled(&self) -> bool { self.experimental.rename } + + pub(crate) fn is_auto_import_enabled(&self) -> bool { + self.experimental.auto_import + } } impl GlobalSettings { @@ -25,6 +29,7 @@ impl GlobalSettings { #[derive(Clone, Default, Debug, PartialEq)] pub(crate) struct ExperimentalSettings { pub(super) rename: bool, + pub(super) auto_import: bool, } /// Resolved client settings for a specific workspace. diff --git a/crates/ty_wasm/src/lib.rs b/crates/ty_wasm/src/lib.rs index cf88ce6392..4bf3d19249 100644 --- a/crates/ty_wasm/src/lib.rs +++ b/crates/ty_wasm/src/lib.rs @@ -415,7 +415,11 @@ impl Workspace { let offset = position.to_text_size(&source, &index, self.position_encoding)?; - let completions = ty_ide::completion(&self.db, file_id.file, offset); + // NOTE: At time of writing, 2025-08-29, auto-import isn't + // ready to be enabled by default yet. Once it is, we should + // either just enable it or provide a way to configure it. + let settings = ty_ide::CompletionSettings { auto_import: false }; + let completions = ty_ide::completion(&self.db, &settings, file_id.file, offset); Ok(completions .into_iter() From 6bc33a041f71963c1bcb83150d0432e54ae1631e Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Tue, 2 Sep 2025 09:40:56 -0400 Subject: [PATCH 14/20] [ty] Pick up typed text as a query for unimported completions It's almost certainly bad juju to show literally every single possible symbol when completions are requested but there is nothing typed yet. Moreover, since there are so many symbols, it is likely beneficial to try and winnow them down before sending them to the client. This change tries to extract text that has been typed and then uses that as a query to listing all available symbols. --- crates/ty_ide/src/completion.rs | 67 ++++++++++++++++++++++++++------- 1 file changed, 54 insertions(+), 13 deletions(-) diff --git a/crates/ty_ide/src/completion.rs b/crates/ty_ide/src/completion.rs index 80239b18e1..8ef7e02e32 100644 --- a/crates/ty_ide/src/completion.rs +++ b/crates/ty_ide/src/completion.rs @@ -47,16 +47,18 @@ pub fn completion<'db>( CompletionTargetAst::Import { .. } | CompletionTargetAst::ImportViaFrom { .. } => { model.import_completions() } - CompletionTargetAst::Scoped { node } => { + CompletionTargetAst::Scoped { node, typed } => { let mut completions = model.scoped_completions(node); if settings.auto_import { - for symbol in all_symbols(db, "") { - completions.push(Completion { - name: ast::name::Name::new(&symbol.symbol.name), - ty: None, - kind: symbol.symbol.kind.to_completion_kind(), - builtin: false, - }); + if let Some(typed) = typed { + for symbol in all_symbols(db, typed) { + completions.push(Completion { + name: ast::name::Name::new(&symbol.symbol.name), + ty: None, + kind: symbol.symbol.kind.to_completion_kind(), + builtin: false, + }); + } } } completions @@ -79,10 +81,12 @@ pub fn completion<'db>( .collect() } +#[derive(Clone, Debug)] pub struct DetailedCompletion<'db> { pub inner: Completion<'db>, pub documentation: Option, } + impl<'db> std::ops::Deref for DetailedCompletion<'db> { type Target = Completion<'db>; fn deref(&self) -> &Self::Target { @@ -283,16 +287,22 @@ impl<'t> CompletionTargetTokens<'t> { } } CompletionTargetTokens::Generic { token } => { - let covering_node = covering_node(parsed.syntax().into(), token.range()); - Some(CompletionTargetAst::Scoped { - node: covering_node.node(), - }) + let node = covering_node(parsed.syntax().into(), token.range()).node(); + let typed = match node { + ast::AnyNodeRef::ExprName(ast::ExprName { id, .. }) => { + let name = id.as_str(); + if name.is_empty() { None } else { Some(name) } + } + _ => None, + }; + Some(CompletionTargetAst::Scoped { node, typed }) } CompletionTargetTokens::Unknown => { let range = TextRange::empty(offset); let covering_node = covering_node(parsed.syntax().into(), range); Some(CompletionTargetAst::Scoped { node: covering_node.node(), + typed: None, }) } } @@ -346,7 +356,16 @@ enum CompletionTargetAst<'t> { }, /// A scoped scenario, where we want to list all items available in /// the most narrow scope containing the giving AST node. - Scoped { node: ast::AnyNodeRef<'t> }, + Scoped { + /// The node with the smallest range that fully covers + /// the token under the cursor. + node: ast::AnyNodeRef<'t>, + /// The text that has been typed so far, if available. + /// + /// When not `None`, the typed text is guaranteed to be + /// non-empty. + typed: Option<&'t str>, + }, } /// Returns a suffix of `tokens` corresponding to the `kinds` given. @@ -3038,6 +3057,24 @@ from os. test.assert_completions_do_not_include("abspath"); } + #[test] + fn auto_import_with_submodule() { + let test = CursorTest::builder() + .source("main.py", "Abra") + .source("package/__init__.py", "AbraKadabra = 1") + .build(); + + let settings = CompletionSettings { auto_import: true }; + let expected = "AbraKadabra"; + let completions = completion(&test.db, &settings, test.cursor.file, test.cursor.offset); + assert!( + completions + .iter() + .any(|completion| completion.name == expected), + "Expected completions to include `{expected}`" + ); + } + #[test] fn regression_test_issue_642() { // Regression test for https://github.com/astral-sh/ty/issues/642 @@ -3056,6 +3093,10 @@ from os. ); } + // NOTE: The methods below are getting somewhat ridiculous. + // We should refactor this by converting to using a builder + // to set different modes. ---AG + impl CursorTest { /// Returns all completions except for builtins. fn completions_without_builtins(&self) -> String { From c402bf8ae295d86c579338df15bb8163b02f5370 Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Wed, 3 Sep 2025 09:30:26 -0400 Subject: [PATCH 15/20] [ty] Correct default value for experimental rename setting Ref https://github.com/astral-sh/ruff/pull/20207#discussion_r2318336964 --- crates/ty_server/src/session/options.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ty_server/src/session/options.rs b/crates/ty_server/src/session/options.rs index e890f05fef..c646c767de 100644 --- a/crates/ty_server/src/session/options.rs +++ b/crates/ty_server/src/session/options.rs @@ -154,7 +154,7 @@ impl GlobalOptions { let experimental = self .experimental .map(|experimental| ExperimentalSettings { - rename: experimental.rename.unwrap_or(true), + rename: experimental.rename.unwrap_or(false), auto_import: experimental.auto_import.unwrap_or(false), }) .unwrap_or_default(); From 5d7c17c20adbbea270234fe65b4849fc76a786d6 Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Wed, 3 Sep 2025 22:12:11 +0800 Subject: [PATCH 16/20] [`airflow`] Convert `DatasetOrTimeSchedule(datasets=...)` to `AssetOrTimeSchedule(assets=...)` (`AIR311`) (#20202) ## Summary update the argument `datasets` as `assets` ## Test Plan update fixture accordingly --- .../test/fixtures/airflow/AIR311_names.py | 2 +- .../airflow/rules/suggested_to_update_3_0.rs | 3 +++ ...irflow__tests__AIR311_AIR311_names.py.snap | 27 ++++++++++++++++--- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/airflow/AIR311_names.py b/crates/ruff_linter/resources/test/fixtures/airflow/AIR311_names.py index 9b4ce1e38e..2b415aa407 100644 --- a/crates/ruff_linter/resources/test/fixtures/airflow/AIR311_names.py +++ b/crates/ruff_linter/resources/test/fixtures/airflow/AIR311_names.py @@ -70,7 +70,7 @@ from airflow.timetables.datasets import DatasetOrTimeSchedule from airflow.utils.dag_parsing_context import get_parsing_context # airflow.timetables.datasets -DatasetOrTimeSchedule() +DatasetOrTimeSchedule(datasets=[]) # airflow.utils.dag_parsing_context get_parsing_context() diff --git a/crates/ruff_linter/src/rules/airflow/rules/suggested_to_update_3_0.rs b/crates/ruff_linter/src/rules/airflow/rules/suggested_to_update_3_0.rs index 41bc35d352..a7c83abbca 100644 --- a/crates/ruff_linter/src/rules/airflow/rules/suggested_to_update_3_0.rs +++ b/crates/ruff_linter/src/rules/airflow/rules/suggested_to_update_3_0.rs @@ -157,6 +157,9 @@ fn check_call_arguments(checker: &Checker, qualified_name: &QualifiedName, argum ["airflow", .., "DAG" | "dag"] => { diagnostic_for_argument(checker, arguments, "sla_miss_callback", None); } + ["airflow", "timetables", "datasets", "DatasetOrTimeSchedule"] => { + diagnostic_for_argument(checker, arguments, "datasets", Some("assets")); + } segments => { if is_airflow_builtin_or_provider(segments, "operators", "Operator") { diagnostic_for_argument(checker, arguments, "sla", None); diff --git a/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR311_AIR311_names.py.snap b/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR311_AIR311_names.py.snap index 3fcbf77530..a3bced5174 100644 --- a/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR311_AIR311_names.py.snap +++ b/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR311_AIR311_names.py.snap @@ -558,7 +558,7 @@ AIR311 [*] `airflow.timetables.datasets.DatasetOrTimeSchedule` is removed in Air --> AIR311_names.py:73:1 | 72 | # airflow.timetables.datasets -73 | DatasetOrTimeSchedule() +73 | DatasetOrTimeSchedule(datasets=[]) | ^^^^^^^^^^^^^^^^^^^^^ 74 | 75 | # airflow.utils.dag_parsing_context @@ -570,12 +570,31 @@ help: Use `AssetOrTimeSchedule` from `airflow.timetables.assets` instead. 71 + from airflow.timetables.assets import AssetOrTimeSchedule 72 | 73 | # airflow.timetables.datasets - - DatasetOrTimeSchedule() -74 + AssetOrTimeSchedule() + - DatasetOrTimeSchedule(datasets=[]) +74 + AssetOrTimeSchedule(datasets=[]) 75 | 76 | # airflow.utils.dag_parsing_context 77 | get_parsing_context() +AIR311 [*] `datasets` is removed in Airflow 3.0; It still works in Airflow 3.0 but is expected to be removed in a future version. + --> AIR311_names.py:73:23 + | +72 | # airflow.timetables.datasets +73 | DatasetOrTimeSchedule(datasets=[]) + | ^^^^^^^^ +74 | +75 | # airflow.utils.dag_parsing_context + | +help: Use `assets` instead +70 | from airflow.utils.dag_parsing_context import get_parsing_context +71 | +72 | # airflow.timetables.datasets + - DatasetOrTimeSchedule(datasets=[]) +73 + DatasetOrTimeSchedule(assets=[]) +74 | +75 | # airflow.utils.dag_parsing_context +76 | get_parsing_context() + AIR311 [*] `airflow.utils.dag_parsing_context.get_parsing_context` is removed in Airflow 3.0; It still works in Airflow 3.0 but is expected to be removed in a future version. --> AIR311_names.py:76:1 | @@ -593,7 +612,7 @@ help: Use `get_parsing_context` from `airflow.sdk` instead. 70 + from airflow.sdk import get_parsing_context 71 | 72 | # airflow.timetables.datasets -73 | DatasetOrTimeSchedule() +73 | DatasetOrTimeSchedule(datasets=[]) note: This is an unsafe fix and may change runtime behavior AIR311 [*] `airflow.decorators.base.DecoratedMappedOperator` is removed in Airflow 3.0; It still works in Airflow 3.0 but is expected to be removed in a future version. From 4c3e1930f6543ad4dea3083dfe4a17349eb2fe76 Mon Sep 17 00:00:00 2001 From: Bhuminjay Soni Date: Wed, 3 Sep 2025 19:43:05 +0530 Subject: [PATCH 17/20] [syntax-errors] Detect `yield from` inside async function (#20051) ## Summary This PR implements https://docs.astral.sh/ruff/rules/yield-from-in-async-function/ as a syntax semantic error ## Test Plan I have written a simple inline test as directed in [https://github.com/astral-sh/ruff/issues/17412](https://github.com/astral-sh/ruff/issues/17412) --------- Signed-off-by: 11happy Co-authored-by: Alex Waygood Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com> --- .../yield_from_in_async_function.py | 1 + .../src/checkers/ast/analyze/expression.rs | 5 +- crates/ruff_linter/src/checkers/ast/mod.rs | 10 ++- crates/ruff_linter/src/linter.rs | 4 ++ .../rules/yield_from_in_async_function.rs | 14 ---- ...ests__yield_from_in_async_function.py.snap | 9 +++ .../err/yield_from_in_async_function.py | 1 + .../ruff_python_parser/src/semantic_errors.rs | 16 +++++ crates/ruff_python_parser/tests/fixtures.rs | 20 ++++-- ...yntax@yield_from_in_async_function.py.snap | 68 +++++++++++++++++++ .../resources/mdtest/annotations/invalid.md | 25 ++++--- 11 files changed, 140 insertions(+), 33 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/syntax_errors/yield_from_in_async_function.py create mode 100644 crates/ruff_linter/src/snapshots/ruff_linter__linter__tests__yield_from_in_async_function.py.snap create mode 100644 crates/ruff_python_parser/resources/inline/err/yield_from_in_async_function.py create mode 100644 crates/ruff_python_parser/tests/snapshots/invalid_syntax@yield_from_in_async_function.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/syntax_errors/yield_from_in_async_function.py b/crates/ruff_linter/resources/test/fixtures/syntax_errors/yield_from_in_async_function.py new file mode 100644 index 0000000000..59e11eabe1 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/syntax_errors/yield_from_in_async_function.py @@ -0,0 +1 @@ +async def f(): yield from x # error \ No newline at end of file diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 39c1b460ee..1851959029 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -1319,13 +1319,10 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) { pylint::rules::yield_in_init(checker, expr); } } - Expr::YieldFrom(yield_from) => { + Expr::YieldFrom(_) => { if checker.is_rule_enabled(Rule::YieldInInit) { pylint::rules::yield_in_init(checker, expr); } - if checker.is_rule_enabled(Rule::YieldFromInAsyncFunction) { - pylint::rules::yield_from_in_async_function(checker, yield_from); - } } Expr::FString(f_string_expr @ ast::ExprFString { value, .. }) => { if checker.is_rule_enabled(Rule::FStringMissingPlaceholders) { diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 906fe2491b..a38086137e 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -71,7 +71,9 @@ use crate::registry::Rule; use crate::rules::pyflakes::rules::{ LateFutureImport, ReturnOutsideFunction, YieldOutsideFunction, }; -use crate::rules::pylint::rules::{AwaitOutsideAsync, LoadBeforeGlobalDeclaration}; +use crate::rules::pylint::rules::{ + AwaitOutsideAsync, LoadBeforeGlobalDeclaration, YieldFromInAsyncFunction, +}; use crate::rules::{flake8_pyi, flake8_type_checking, pyflakes, pyupgrade}; use crate::settings::rule_table::RuleTable; use crate::settings::{LinterSettings, TargetVersion, flags}; @@ -668,6 +670,12 @@ impl SemanticSyntaxContext for Checker<'_> { self.report_diagnostic(AwaitOutsideAsync, error.range); } } + SemanticSyntaxErrorKind::YieldFromInAsyncFunction => { + // PLE1700 + if self.is_rule_enabled(Rule::YieldFromInAsyncFunction) { + self.report_diagnostic(YieldFromInAsyncFunction, error.range); + } + } SemanticSyntaxErrorKind::ReboundComprehensionVariable | SemanticSyntaxErrorKind::DuplicateTypeParameter | SemanticSyntaxErrorKind::MultipleCaseAssignment(_) diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index e6dc3f3658..eeb321cab9 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -1231,6 +1231,10 @@ mod tests { )] #[test_case(Rule::AwaitOutsideAsync, Path::new("await_outside_async_function.py"))] #[test_case(Rule::AwaitOutsideAsync, Path::new("async_comprehension.py"))] + #[test_case( + Rule::YieldFromInAsyncFunction, + Path::new("yield_from_in_async_function.py") + )] fn test_syntax_errors(rule: Rule, path: &Path) -> Result<()> { let snapshot = path.to_string_lossy().to_string(); let path = Path::new("resources/test/fixtures/syntax_errors").join(path); diff --git a/crates/ruff_linter/src/rules/pylint/rules/yield_from_in_async_function.rs b/crates/ruff_linter/src/rules/pylint/rules/yield_from_in_async_function.rs index 60fc10bc52..fd9e2ee618 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/yield_from_in_async_function.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/yield_from_in_async_function.rs @@ -1,10 +1,6 @@ use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::{self as ast}; -use ruff_python_semantic::ScopeKind; -use ruff_text_size::Ranged; use crate::Violation; -use crate::checkers::ast::Checker; /// ## What it does /// Checks for uses of `yield from` in async functions. @@ -36,13 +32,3 @@ impl Violation for YieldFromInAsyncFunction { "`yield from` statement in async function; use `async for` instead".to_string() } } - -/// PLE1700 -pub(crate) fn yield_from_in_async_function(checker: &Checker, expr: &ast::ExprYieldFrom) { - if matches!( - checker.semantic().current_scope().kind, - ScopeKind::Function(ast::StmtFunctionDef { is_async: true, .. }) - ) { - checker.report_diagnostic(YieldFromInAsyncFunction, expr.range()); - } -} diff --git a/crates/ruff_linter/src/snapshots/ruff_linter__linter__tests__yield_from_in_async_function.py.snap b/crates/ruff_linter/src/snapshots/ruff_linter__linter__tests__yield_from_in_async_function.py.snap new file mode 100644 index 0000000000..ab60900419 --- /dev/null +++ b/crates/ruff_linter/src/snapshots/ruff_linter__linter__tests__yield_from_in_async_function.py.snap @@ -0,0 +1,9 @@ +--- +source: crates/ruff_linter/src/linter.rs +--- +PLE1700 `yield from` statement in async function; use `async for` instead + --> resources/test/fixtures/syntax_errors/yield_from_in_async_function.py:1:16 + | +1 | async def f(): yield from x # error + | ^^^^^^^^^^^^ + | diff --git a/crates/ruff_python_parser/resources/inline/err/yield_from_in_async_function.py b/crates/ruff_python_parser/resources/inline/err/yield_from_in_async_function.py new file mode 100644 index 0000000000..79354bc05c --- /dev/null +++ b/crates/ruff_python_parser/resources/inline/err/yield_from_in_async_function.py @@ -0,0 +1 @@ +async def f(): yield from x diff --git a/crates/ruff_python_parser/src/semantic_errors.rs b/crates/ruff_python_parser/src/semantic_errors.rs index 6a3ce2a850..9b4d42ffd5 100644 --- a/crates/ruff_python_parser/src/semantic_errors.rs +++ b/crates/ruff_python_parser/src/semantic_errors.rs @@ -709,6 +709,16 @@ impl SemanticSyntaxChecker { } Expr::YieldFrom(_) => { Self::yield_outside_function(ctx, expr, YieldOutsideFunctionKind::YieldFrom); + if ctx.in_function_scope() && ctx.in_async_context() { + // test_err yield_from_in_async_function + // async def f(): yield from x + + Self::add_error( + ctx, + SemanticSyntaxErrorKind::YieldFromInAsyncFunction, + expr.range(), + ); + } } Expr::Await(_) => { Self::yield_outside_function(ctx, expr, YieldOutsideFunctionKind::Await); @@ -989,6 +999,9 @@ impl Display for SemanticSyntaxError { SemanticSyntaxErrorKind::AnnotatedNonlocal(name) => { write!(f, "annotated name `{name}` can't be nonlocal") } + SemanticSyntaxErrorKind::YieldFromInAsyncFunction => { + f.write_str("`yield from` statement in async function; use `async for` instead") + } } } } @@ -1346,6 +1359,9 @@ pub enum SemanticSyntaxErrorKind { /// Represents a type annotation on a variable that's been declared nonlocal AnnotatedNonlocal(String), + + /// Represents the use of `yield from` inside an asynchronous function. + YieldFromInAsyncFunction, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, get_size2::GetSize)] diff --git a/crates/ruff_python_parser/tests/fixtures.rs b/crates/ruff_python_parser/tests/fixtures.rs index 7aa7a6f2e2..e3679ea50a 100644 --- a/crates/ruff_python_parser/tests/fixtures.rs +++ b/crates/ruff_python_parser/tests/fixtures.rs @@ -465,7 +465,7 @@ impl<'ast> SourceOrderVisitor<'ast> for ValidateAstVisitor<'ast> { enum Scope { Module, - Function, + Function { is_async: bool }, Comprehension { is_async: bool }, Class, } @@ -528,7 +528,15 @@ impl SemanticSyntaxContext for SemanticSyntaxCheckerVisitor<'_> { } fn in_async_context(&self) -> bool { - true + if let Some(scope) = self.scopes.iter().next_back() { + match scope { + Scope::Class | Scope::Module => false, + Scope::Comprehension { is_async } => *is_async, + Scope::Function { is_async } => *is_async, + } + } else { + false + } } fn in_sync_comprehension(&self) -> bool { @@ -589,8 +597,10 @@ impl Visitor<'_> for SemanticSyntaxCheckerVisitor<'_> { self.visit_body(body); self.scopes.pop().unwrap(); } - ast::Stmt::FunctionDef(ast::StmtFunctionDef { .. }) => { - self.scopes.push(Scope::Function); + ast::Stmt::FunctionDef(ast::StmtFunctionDef { is_async, .. }) => { + self.scopes.push(Scope::Function { + is_async: *is_async, + }); ast::visitor::walk_stmt(self, stmt); self.scopes.pop().unwrap(); } @@ -604,7 +614,7 @@ impl Visitor<'_> for SemanticSyntaxCheckerVisitor<'_> { self.with_semantic_checker(|semantic, context| semantic.visit_expr(expr, context)); match expr { ast::Expr::Lambda(_) => { - self.scopes.push(Scope::Function); + self.scopes.push(Scope::Function { is_async: false }); ast::visitor::walk_expr(self, expr); self.scopes.pop().unwrap(); } diff --git a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@yield_from_in_async_function.py.snap b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@yield_from_in_async_function.py.snap new file mode 100644 index 0000000000..86372e56d9 --- /dev/null +++ b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@yield_from_in_async_function.py.snap @@ -0,0 +1,68 @@ +--- +source: crates/ruff_python_parser/tests/fixtures.rs +input_file: crates/ruff_python_parser/resources/inline/err/yield_from_in_async_function.py +--- +## AST + +``` +Module( + ModModule { + node_index: NodeIndex(None), + range: 0..28, + body: [ + FunctionDef( + StmtFunctionDef { + node_index: NodeIndex(None), + range: 0..27, + is_async: true, + decorator_list: [], + name: Identifier { + id: Name("f"), + range: 10..11, + node_index: NodeIndex(None), + }, + type_params: None, + parameters: Parameters { + range: 11..13, + node_index: NodeIndex(None), + posonlyargs: [], + args: [], + vararg: None, + kwonlyargs: [], + kwarg: None, + }, + returns: None, + body: [ + Expr( + StmtExpr { + node_index: NodeIndex(None), + range: 15..27, + value: YieldFrom( + ExprYieldFrom { + node_index: NodeIndex(None), + range: 15..27, + value: Name( + ExprName { + node_index: NodeIndex(None), + range: 26..27, + id: Name("x"), + ctx: Load, + }, + ), + }, + ), + }, + ), + ], + }, + ), + ], + }, +) +``` +## Semantic Syntax Errors + + | +1 | async def f(): yield from x + | ^^^^^^^^^^^^ Syntax Error: `yield from` statement in async function; use `async for` instead + | diff --git a/crates/ty_python_semantic/resources/mdtest/annotations/invalid.md b/crates/ty_python_semantic/resources/mdtest/annotations/invalid.md index 91d55f7352..5202b0921d 100644 --- a/crates/ty_python_semantic/resources/mdtest/annotations/invalid.md +++ b/crates/ty_python_semantic/resources/mdtest/annotations/invalid.md @@ -74,8 +74,13 @@ def _( def bar() -> None: return None +def outer_sync(): # `yield` from is only valid syntax inside a synchronous function + def _( + a: (yield from [1]), # error: [invalid-type-form] "`yield from` expressions are not allowed in type expressions" + ): ... + async def baz(): ... -async def outer(): # avoid unrelated syntax errors on yield, yield from, and await +async def outer_async(): # avoid unrelated syntax errors on `yield` and `await` def _( a: 1, # error: [invalid-type-form] "Int literals are not allowed in this context in a type expression" b: 2.3, # error: [invalid-type-form] "Float literals are not allowed in type expressions" @@ -90,11 +95,10 @@ async def outer(): # avoid unrelated syntax errors on yield, yield from, and aw k: 1 if True else 2, # error: [invalid-type-form] "`if` expressions are not allowed in type expressions" l: await baz(), # error: [invalid-type-form] "`await` expressions are not allowed in type expressions" m: (yield 1), # error: [invalid-type-form] "`yield` expressions are not allowed in type expressions" - n: (yield from [1]), # error: [invalid-type-form] "`yield from` expressions are not allowed in type expressions" - o: 1 < 2, # error: [invalid-type-form] "Comparison expressions are not allowed in type expressions" - p: bar(), # error: [invalid-type-form] "Function calls are not allowed in type expressions" - q: int | f"foo", # error: [invalid-type-form] "F-strings are not allowed in type expressions" - r: [1, 2, 3][1:2], # error: [invalid-type-form] "Slices are not allowed in type expressions" + n: 1 < 2, # error: [invalid-type-form] "Comparison expressions are not allowed in type expressions" + o: bar(), # error: [invalid-type-form] "Function calls are not allowed in type expressions" + p: int | f"foo", # error: [invalid-type-form] "F-strings are not allowed in type expressions" + q: [1, 2, 3][1:2], # error: [invalid-type-form] "Slices are not allowed in type expressions" ): reveal_type(a) # revealed: Unknown reveal_type(b) # revealed: Unknown @@ -107,9 +111,12 @@ async def outer(): # avoid unrelated syntax errors on yield, yield from, and aw reveal_type(i) # revealed: Unknown reveal_type(j) # revealed: Unknown reveal_type(k) # revealed: Unknown - reveal_type(p) # revealed: Unknown - reveal_type(q) # revealed: int | Unknown - reveal_type(r) # revealed: @Todo(unknown type subscript) + reveal_type(l) # revealed: Unknown + reveal_type(m) # revealed: Unknown + reveal_type(n) # revealed: Unknown + reveal_type(o) # revealed: Unknown + reveal_type(p) # revealed: int | Unknown + reveal_type(q) # revealed: @Todo(unknown type subscript) class Mat: def __init__(self, value: int): From c452a2cb79078b8fb12b6df40e866f604ba20195 Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Wed, 3 Sep 2025 22:18:17 +0800 Subject: [PATCH 18/20] [`airflow`] Move `airflow.operators.postgres_operator.Mapping` from `AIR302` to `AIR301` (#20172) ## Summary ### Why Removal should be grouped into the same category. It doesn't matter whether it's from a provider or not (and the only case we used to have was not anyway). `ProviderReplacement` is used to indicate that we have a replacement and we might need to install an extra Python package to cater to it. ### What Move `airflow.operators.postgres_operator.Mapping` from AIR302 to AIR301 and get rid of `ProviderReplace::None` ## Test Plan Update the test fixtures accordingly in the first commit and reorganize them in the second commit --- .../test/fixtures/airflow/AIR301_names.py | 3 + .../ruff_linter/src/rules/airflow/helpers.rs | 1 - .../airflow/rules/moved_to_provider_in_3.rs | 15 - .../src/rules/airflow/rules/removal_in_3.rs | 1 + .../suggested_to_move_to_provider_in_3.rs | 14 - ...irflow__tests__AIR301_AIR301_names.py.snap | 368 +++++++++--------- ...low__tests__AIR302_AIR302_postgres.py.snap | 8 - 7 files changed, 194 insertions(+), 216 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/airflow/AIR301_names.py b/crates/ruff_linter/resources/test/fixtures/airflow/AIR301_names.py index 640f855f05..68e5435884 100644 --- a/crates/ruff_linter/resources/test/fixtures/airflow/AIR301_names.py +++ b/crates/ruff_linter/resources/test/fixtures/airflow/AIR301_names.py @@ -12,6 +12,7 @@ from airflow import ( from airflow.api_connexion.security import requires_access from airflow.contrib.aws_athena_hook import AWSAthenaHook from airflow.datasets import DatasetAliasEvent +from airflow.operators.postgres_operator import Mapping from airflow.operators.subdag import SubDagOperator from airflow.secrets.cache import SecretCache from airflow.secrets.local_filesystem import LocalFilesystemBackend @@ -52,6 +53,8 @@ DatasetAliasEvent() # airflow.operators.subdag.* SubDagOperator() +# airflow.operators.postgres_operator +Mapping() # airflow.secrets # get_connection diff --git a/crates/ruff_linter/src/rules/airflow/helpers.rs b/crates/ruff_linter/src/rules/airflow/helpers.rs index b5997e916f..d33e7c32e2 100644 --- a/crates/ruff_linter/src/rules/airflow/helpers.rs +++ b/crates/ruff_linter/src/rules/airflow/helpers.rs @@ -37,7 +37,6 @@ pub(crate) enum Replacement { #[derive(Clone, Debug, Eq, PartialEq)] pub(crate) enum ProviderReplacement { - None, AutoImport { module: &'static str, name: &'static str, diff --git a/crates/ruff_linter/src/rules/airflow/rules/moved_to_provider_in_3.rs b/crates/ruff_linter/src/rules/airflow/rules/moved_to_provider_in_3.rs index e22d6fbd62..88035562ca 100644 --- a/crates/ruff_linter/src/rules/airflow/rules/moved_to_provider_in_3.rs +++ b/crates/ruff_linter/src/rules/airflow/rules/moved_to_provider_in_3.rs @@ -50,9 +50,6 @@ impl Violation for Airflow3MovedToProvider<'_> { replacement, } = self; match replacement { - ProviderReplacement::None => { - format!("`{deprecated}` is removed in Airflow 3.0") - } ProviderReplacement::AutoImport { name: _, module: _, @@ -85,7 +82,6 @@ impl Violation for Airflow3MovedToProvider<'_> { provider, version, } => Some((module, name.as_str(), provider, version)), - ProviderReplacement::None => None, } { Some(format!( "Install `apache-airflow-providers-{provider}>={version}` and use `{name}` from `{module}` instead." @@ -1020,7 +1016,6 @@ fn check_names_moved_to_provider(checker: &Checker, expr: &Expr, ranged: TextRan provider: "postgres", version: "1.0.0", }, - ["airflow", "operators", "postgres_operator", "Mapping"] => ProviderReplacement::None, // apache-airflow-providers-presto ["airflow", "hooks", "presto_hook", "PrestoHook"] => ProviderReplacement::AutoImport { @@ -1209,16 +1204,6 @@ fn check_names_moved_to_provider(checker: &Checker, expr: &Expr, ranged: TextRan ProviderReplacement::SourceModuleMovedToProvider { module, name, .. } => { (module, name.as_str()) } - ProviderReplacement::None => { - checker.report_diagnostic( - Airflow3MovedToProvider { - deprecated: qualified_name, - replacement, - }, - ranged, - ); - return; - } }; if is_guarded_by_try_except(expr, module, name, checker.semantic()) { diff --git a/crates/ruff_linter/src/rules/airflow/rules/removal_in_3.rs b/crates/ruff_linter/src/rules/airflow/rules/removal_in_3.rs index 4d822812c0..a3d3e3bc95 100644 --- a/crates/ruff_linter/src/rules/airflow/rules/removal_in_3.rs +++ b/crates/ruff_linter/src/rules/airflow/rules/removal_in_3.rs @@ -704,6 +704,7 @@ fn check_name(checker: &Checker, expr: &Expr, range: TextRange) { ["airflow", "operators", "subdag", ..] => { Replacement::Message("The whole `airflow.subdag` module has been removed.") } + ["airflow", "operators", "postgres_operator", "Mapping"] => Replacement::None, ["airflow", "operators", "python", "get_current_context"] => Replacement::AutoImport { module: "airflow.sdk", name: "get_current_context", diff --git a/crates/ruff_linter/src/rules/airflow/rules/suggested_to_move_to_provider_in_3.rs b/crates/ruff_linter/src/rules/airflow/rules/suggested_to_move_to_provider_in_3.rs index 5f88370c37..42c647e61a 100644 --- a/crates/ruff_linter/src/rules/airflow/rules/suggested_to_move_to_provider_in_3.rs +++ b/crates/ruff_linter/src/rules/airflow/rules/suggested_to_move_to_provider_in_3.rs @@ -65,9 +65,6 @@ impl Violation for Airflow3SuggestedToMoveToProvider<'_> { replacement, } = self; match replacement { - ProviderReplacement::None => { - format!("`{deprecated}` is removed in Airflow 3.0") - } ProviderReplacement::AutoImport { name: _, module: _, @@ -91,7 +88,6 @@ impl Violation for Airflow3SuggestedToMoveToProvider<'_> { fn fix_title(&self) -> Option { let Airflow3SuggestedToMoveToProvider { replacement, .. } = self; match replacement { - ProviderReplacement::None => None, ProviderReplacement::AutoImport { module, name, @@ -319,16 +315,6 @@ fn check_names_moved_to_provider(checker: &Checker, expr: &Expr, ranged: TextRan ProviderReplacement::SourceModuleMovedToProvider { module, name, .. } => { (module, name.as_str()) } - ProviderReplacement::None => { - checker.report_diagnostic( - Airflow3SuggestedToMoveToProvider { - deprecated: qualified_name, - replacement: replacement.clone(), - }, - ranged.range(), - ); - return; - } }; if is_guarded_by_try_except(expr, module, name, checker.semantic()) { diff --git a/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR301_AIR301_names.py.snap b/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR301_AIR301_names.py.snap index db6c2744af..95b78f3d97 100644 --- a/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR301_AIR301_names.py.snap +++ b/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR301_AIR301_names.py.snap @@ -2,350 +2,362 @@ source: crates/ruff_linter/src/rules/airflow/mod.rs --- AIR301 `airflow.PY36` is removed in Airflow 3.0 - --> AIR301_names.py:39:1 + --> AIR301_names.py:40:1 | -38 | # airflow root -39 | PY36, PY37, PY38, PY39, PY310, PY311, PY312 +39 | # airflow root +40 | PY36, PY37, PY38, PY39, PY310, PY311, PY312 | ^^^^ -40 | -41 | # airflow.api_connexion.security +41 | +42 | # airflow.api_connexion.security | help: Use `sys.version_info` instead AIR301 `airflow.PY37` is removed in Airflow 3.0 - --> AIR301_names.py:39:7 + --> AIR301_names.py:40:7 | -38 | # airflow root -39 | PY36, PY37, PY38, PY39, PY310, PY311, PY312 +39 | # airflow root +40 | PY36, PY37, PY38, PY39, PY310, PY311, PY312 | ^^^^ -40 | -41 | # airflow.api_connexion.security +41 | +42 | # airflow.api_connexion.security | help: Use `sys.version_info` instead AIR301 `airflow.PY38` is removed in Airflow 3.0 - --> AIR301_names.py:39:13 + --> AIR301_names.py:40:13 | -38 | # airflow root -39 | PY36, PY37, PY38, PY39, PY310, PY311, PY312 +39 | # airflow root +40 | PY36, PY37, PY38, PY39, PY310, PY311, PY312 | ^^^^ -40 | -41 | # airflow.api_connexion.security +41 | +42 | # airflow.api_connexion.security | help: Use `sys.version_info` instead AIR301 `airflow.PY39` is removed in Airflow 3.0 - --> AIR301_names.py:39:19 + --> AIR301_names.py:40:19 | -38 | # airflow root -39 | PY36, PY37, PY38, PY39, PY310, PY311, PY312 +39 | # airflow root +40 | PY36, PY37, PY38, PY39, PY310, PY311, PY312 | ^^^^ -40 | -41 | # airflow.api_connexion.security +41 | +42 | # airflow.api_connexion.security | help: Use `sys.version_info` instead AIR301 `airflow.PY310` is removed in Airflow 3.0 - --> AIR301_names.py:39:25 + --> AIR301_names.py:40:25 | -38 | # airflow root -39 | PY36, PY37, PY38, PY39, PY310, PY311, PY312 +39 | # airflow root +40 | PY36, PY37, PY38, PY39, PY310, PY311, PY312 | ^^^^^ -40 | -41 | # airflow.api_connexion.security +41 | +42 | # airflow.api_connexion.security | help: Use `sys.version_info` instead AIR301 `airflow.PY311` is removed in Airflow 3.0 - --> AIR301_names.py:39:32 + --> AIR301_names.py:40:32 | -38 | # airflow root -39 | PY36, PY37, PY38, PY39, PY310, PY311, PY312 +39 | # airflow root +40 | PY36, PY37, PY38, PY39, PY310, PY311, PY312 | ^^^^^ -40 | -41 | # airflow.api_connexion.security +41 | +42 | # airflow.api_connexion.security | help: Use `sys.version_info` instead AIR301 `airflow.PY312` is removed in Airflow 3.0 - --> AIR301_names.py:39:39 + --> AIR301_names.py:40:39 | -38 | # airflow root -39 | PY36, PY37, PY38, PY39, PY310, PY311, PY312 +39 | # airflow root +40 | PY36, PY37, PY38, PY39, PY310, PY311, PY312 | ^^^^^ -40 | -41 | # airflow.api_connexion.security +41 | +42 | # airflow.api_connexion.security | help: Use `sys.version_info` instead AIR301 `airflow.api_connexion.security.requires_access` is removed in Airflow 3.0 - --> AIR301_names.py:42:1 + --> AIR301_names.py:43:1 | -41 | # airflow.api_connexion.security -42 | requires_access +42 | # airflow.api_connexion.security +43 | requires_access | ^^^^^^^^^^^^^^^ -43 | -44 | # airflow.contrib.* +44 | +45 | # airflow.contrib.* | help: Use `airflow.api_fastapi.core_api.security.requires_access_*` instead AIR301 `airflow.contrib.aws_athena_hook.AWSAthenaHook` is removed in Airflow 3.0 - --> AIR301_names.py:45:1 + --> AIR301_names.py:46:1 | -44 | # airflow.contrib.* -45 | AWSAthenaHook() +45 | # airflow.contrib.* +46 | AWSAthenaHook() | ^^^^^^^^^^^^^ | help: The whole `airflow.contrib` module has been removed. AIR301 `airflow.datasets.DatasetAliasEvent` is removed in Airflow 3.0 - --> AIR301_names.py:49:1 + --> AIR301_names.py:50:1 | -48 | # airflow.datasets -49 | DatasetAliasEvent() +49 | # airflow.datasets +50 | DatasetAliasEvent() | ^^^^^^^^^^^^^^^^^ | AIR301 `airflow.operators.subdag.SubDagOperator` is removed in Airflow 3.0 - --> AIR301_names.py:53:1 + --> AIR301_names.py:54:1 | -52 | # airflow.operators.subdag.* -53 | SubDagOperator() +53 | # airflow.operators.subdag.* +54 | SubDagOperator() | ^^^^^^^^^^^^^^ +55 | +56 | # airflow.operators.postgres_operator | help: The whole `airflow.subdag` module has been removed. -AIR301 [*] `airflow.secrets.cache.SecretCache` is removed in Airflow 3.0 - --> AIR301_names.py:61:1 +AIR301 `airflow.operators.postgres_operator.Mapping` is removed in Airflow 3.0 + --> AIR301_names.py:57:1 | -60 | # airflow.secrets.cache -61 | SecretCache() +56 | # airflow.operators.postgres_operator +57 | Mapping() + | ^^^^^^^ +58 | +59 | # airflow.secrets + | + +AIR301 [*] `airflow.secrets.cache.SecretCache` is removed in Airflow 3.0 + --> AIR301_names.py:64:1 + | +63 | # airflow.secrets.cache +64 | SecretCache() | ^^^^^^^^^^^ | help: Use `SecretCache` from `airflow.sdk` instead. -13 | from airflow.contrib.aws_athena_hook import AWSAthenaHook 14 | from airflow.datasets import DatasetAliasEvent -15 | from airflow.operators.subdag import SubDagOperator +15 | from airflow.operators.postgres_operator import Mapping +16 | from airflow.operators.subdag import SubDagOperator - from airflow.secrets.cache import SecretCache -16 | from airflow.secrets.local_filesystem import LocalFilesystemBackend -17 | from airflow.triggers.external_task import TaskStateTrigger -18 | from airflow.utils import dates +17 | from airflow.secrets.local_filesystem import LocalFilesystemBackend +18 | from airflow.triggers.external_task import TaskStateTrigger +19 | from airflow.utils import dates -------------------------------------------------------------------------------- -33 | from airflow.utils.trigger_rule import TriggerRule -34 | from airflow.www.auth import has_access, has_access_dataset -35 | from airflow.www.utils import get_sensitive_variables_fields, should_hide_value_for_key -36 + from airflow.sdk import SecretCache -37 | -38 | # airflow root -39 | PY36, PY37, PY38, PY39, PY310, PY311, PY312 +34 | from airflow.utils.trigger_rule import TriggerRule +35 | from airflow.www.auth import has_access, has_access_dataset +36 | from airflow.www.utils import get_sensitive_variables_fields, should_hide_value_for_key +37 + from airflow.sdk import SecretCache +38 | +39 | # airflow root +40 | PY36, PY37, PY38, PY39, PY310, PY311, PY312 note: This is an unsafe fix and may change runtime behavior AIR301 `airflow.triggers.external_task.TaskStateTrigger` is removed in Airflow 3.0 - --> AIR301_names.py:65:1 - | -64 | # airflow.triggers.external_task -65 | TaskStateTrigger() - | ^^^^^^^^^^^^^^^^ -66 | -67 | # airflow.utils.date - | - -AIR301 `airflow.utils.dates.date_range` is removed in Airflow 3.0 --> AIR301_names.py:68:1 | -67 | # airflow.utils.date -68 | dates.date_range +67 | # airflow.triggers.external_task +68 | TaskStateTrigger() | ^^^^^^^^^^^^^^^^ -69 | dates.days_ago +69 | +70 | # airflow.utils.date | -AIR301 `airflow.utils.dates.days_ago` is removed in Airflow 3.0 - --> AIR301_names.py:69:1 - | -67 | # airflow.utils.date -68 | dates.date_range -69 | dates.days_ago - | ^^^^^^^^^^^^^^ -70 | -71 | date_range - | -help: Use `pendulum.today('UTC').add(days=-N, ...)` instead - AIR301 `airflow.utils.dates.date_range` is removed in Airflow 3.0 --> AIR301_names.py:71:1 | -69 | dates.days_ago -70 | -71 | date_range - | ^^^^^^^^^^ -72 | days_ago -73 | infer_time_unit +70 | # airflow.utils.date +71 | dates.date_range + | ^^^^^^^^^^^^^^^^ +72 | dates.days_ago | AIR301 `airflow.utils.dates.days_ago` is removed in Airflow 3.0 --> AIR301_names.py:72:1 | -71 | date_range -72 | days_ago +70 | # airflow.utils.date +71 | dates.date_range +72 | dates.days_ago + | ^^^^^^^^^^^^^^ +73 | +74 | date_range + | +help: Use `pendulum.today('UTC').add(days=-N, ...)` instead + +AIR301 `airflow.utils.dates.date_range` is removed in Airflow 3.0 + --> AIR301_names.py:74:1 + | +72 | dates.days_ago +73 | +74 | date_range + | ^^^^^^^^^^ +75 | days_ago +76 | infer_time_unit + | + +AIR301 `airflow.utils.dates.days_ago` is removed in Airflow 3.0 + --> AIR301_names.py:75:1 + | +74 | date_range +75 | days_ago | ^^^^^^^^ -73 | infer_time_unit -74 | parse_execution_date +76 | infer_time_unit +77 | parse_execution_date | help: Use `pendulum.today('UTC').add(days=-N, ...)` instead AIR301 `airflow.utils.dates.infer_time_unit` is removed in Airflow 3.0 - --> AIR301_names.py:73:1 + --> AIR301_names.py:76:1 | -71 | date_range -72 | days_ago -73 | infer_time_unit +74 | date_range +75 | days_ago +76 | infer_time_unit | ^^^^^^^^^^^^^^^ -74 | parse_execution_date -75 | round_time +77 | parse_execution_date +78 | round_time | AIR301 `airflow.utils.dates.parse_execution_date` is removed in Airflow 3.0 - --> AIR301_names.py:74:1 + --> AIR301_names.py:77:1 | -72 | days_ago -73 | infer_time_unit -74 | parse_execution_date +75 | days_ago +76 | infer_time_unit +77 | parse_execution_date | ^^^^^^^^^^^^^^^^^^^^ -75 | round_time -76 | scale_time_units +78 | round_time +79 | scale_time_units | AIR301 `airflow.utils.dates.round_time` is removed in Airflow 3.0 - --> AIR301_names.py:75:1 + --> AIR301_names.py:78:1 | -73 | infer_time_unit -74 | parse_execution_date -75 | round_time +76 | infer_time_unit +77 | parse_execution_date +78 | round_time | ^^^^^^^^^^ -76 | scale_time_units +79 | scale_time_units | AIR301 `airflow.utils.dates.scale_time_units` is removed in Airflow 3.0 - --> AIR301_names.py:76:1 + --> AIR301_names.py:79:1 | -74 | parse_execution_date -75 | round_time -76 | scale_time_units +77 | parse_execution_date +78 | round_time +79 | scale_time_units | ^^^^^^^^^^^^^^^^ -77 | -78 | # This one was not deprecated. +80 | +81 | # This one was not deprecated. | AIR301 `airflow.utils.dag_cycle_tester.test_cycle` is removed in Airflow 3.0 - --> AIR301_names.py:83:1 + --> AIR301_names.py:86:1 | -82 | # airflow.utils.dag_cycle_tester -83 | test_cycle +85 | # airflow.utils.dag_cycle_tester +86 | test_cycle | ^^^^^^^^^^ | AIR301 `airflow.utils.db.create_session` is removed in Airflow 3.0 - --> AIR301_names.py:87:1 + --> AIR301_names.py:90:1 | -86 | # airflow.utils.db -87 | create_session +89 | # airflow.utils.db +90 | create_session | ^^^^^^^^^^^^^^ -88 | -89 | # airflow.utils.decorators +91 | +92 | # airflow.utils.decorators | AIR301 `airflow.utils.decorators.apply_defaults` is removed in Airflow 3.0 - --> AIR301_names.py:90:1 + --> AIR301_names.py:93:1 | -89 | # airflow.utils.decorators -90 | apply_defaults +92 | # airflow.utils.decorators +93 | apply_defaults | ^^^^^^^^^^^^^^ -91 | -92 | # airflow.utils.file +94 | +95 | # airflow.utils.file | help: `apply_defaults` is now unconditionally done and can be safely removed. AIR301 `airflow.utils.file.mkdirs` is removed in Airflow 3.0 - --> AIR301_names.py:93:1 + --> AIR301_names.py:96:1 | -92 | # airflow.utils.file -93 | mkdirs +95 | # airflow.utils.file +96 | mkdirs | ^^^^^^ | help: Use `pathlib.Path({path}).mkdir` instead AIR301 `airflow.utils.state.SHUTDOWN` is removed in Airflow 3.0 - --> AIR301_names.py:97:1 - | -96 | # airflow.utils.state -97 | SHUTDOWN - | ^^^^^^^^ -98 | terminating_states - | + --> AIR301_names.py:100:1 + | + 99 | # airflow.utils.state +100 | SHUTDOWN + | ^^^^^^^^ +101 | terminating_states + | AIR301 `airflow.utils.state.terminating_states` is removed in Airflow 3.0 - --> AIR301_names.py:98:1 + --> AIR301_names.py:101:1 | - 96 | # airflow.utils.state - 97 | SHUTDOWN - 98 | terminating_states + 99 | # airflow.utils.state +100 | SHUTDOWN +101 | terminating_states | ^^^^^^^^^^^^^^^^^^ - 99 | -100 | # airflow.utils.trigger_rule +102 | +103 | # airflow.utils.trigger_rule | AIR301 `airflow.utils.trigger_rule.TriggerRule.DUMMY` is removed in Airflow 3.0 - --> AIR301_names.py:101:1 + --> AIR301_names.py:104:1 | -100 | # airflow.utils.trigger_rule -101 | TriggerRule.DUMMY +103 | # airflow.utils.trigger_rule +104 | TriggerRule.DUMMY | ^^^^^^^^^^^^^^^^^ -102 | TriggerRule.NONE_FAILED_OR_SKIPPED +105 | TriggerRule.NONE_FAILED_OR_SKIPPED | AIR301 `airflow.utils.trigger_rule.TriggerRule.NONE_FAILED_OR_SKIPPED` is removed in Airflow 3.0 - --> AIR301_names.py:102:1 + --> AIR301_names.py:105:1 | -100 | # airflow.utils.trigger_rule -101 | TriggerRule.DUMMY -102 | TriggerRule.NONE_FAILED_OR_SKIPPED +103 | # airflow.utils.trigger_rule +104 | TriggerRule.DUMMY +105 | TriggerRule.NONE_FAILED_OR_SKIPPED | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | AIR301 `airflow.www.auth.has_access` is removed in Airflow 3.0 - --> AIR301_names.py:106:1 + --> AIR301_names.py:109:1 | -105 | # airflow.www.auth -106 | has_access +108 | # airflow.www.auth +109 | has_access | ^^^^^^^^^^ -107 | has_access_dataset +110 | has_access_dataset | AIR301 `airflow.www.auth.has_access_dataset` is removed in Airflow 3.0 - --> AIR301_names.py:107:1 + --> AIR301_names.py:110:1 | -105 | # airflow.www.auth -106 | has_access -107 | has_access_dataset +108 | # airflow.www.auth +109 | has_access +110 | has_access_dataset | ^^^^^^^^^^^^^^^^^^ -108 | -109 | # airflow.www.utils +111 | +112 | # airflow.www.utils | AIR301 `airflow.www.utils.get_sensitive_variables_fields` is removed in Airflow 3.0 - --> AIR301_names.py:110:1 + --> AIR301_names.py:113:1 | -109 | # airflow.www.utils -110 | get_sensitive_variables_fields +112 | # airflow.www.utils +113 | get_sensitive_variables_fields | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -111 | should_hide_value_for_key +114 | should_hide_value_for_key | AIR301 `airflow.www.utils.should_hide_value_for_key` is removed in Airflow 3.0 - --> AIR301_names.py:111:1 + --> AIR301_names.py:114:1 | -109 | # airflow.www.utils -110 | get_sensitive_variables_fields -111 | should_hide_value_for_key +112 | # airflow.www.utils +113 | get_sensitive_variables_fields +114 | should_hide_value_for_key | ^^^^^^^^^^^^^^^^^^^^^^^^^ | diff --git a/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR302_AIR302_postgres.py.snap b/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR302_AIR302_postgres.py.snap index e2397c6bbe..b3d3f529e7 100644 --- a/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR302_AIR302_postgres.py.snap +++ b/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR302_AIR302_postgres.py.snap @@ -20,11 +20,3 @@ help: Install `apache-airflow-providers-postgres>=1.0.0` and use `PostgresHook` 6 | PostgresHook() 7 | Mapping() note: This is an unsafe fix and may change runtime behavior - -AIR302 `airflow.operators.postgres_operator.Mapping` is removed in Airflow 3.0 - --> AIR302_postgres.py:7:1 - | -6 | PostgresHook() -7 | Mapping() - | ^^^^^^^ - | From b14fc961413c5ca032fdfdff29fcc6be4f9c8a75 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 3 Sep 2025 10:48:38 -0400 Subject: [PATCH 19/20] Update Rust crate tracing-subscriber to v0.3.20 (#20162) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [tracing-subscriber](https://tokio.rs) ([source](https://redirect.github.com/tokio-rs/tracing)) | workspace.dependencies | patch | `0.3.19` -> `0.3.20` | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. ### GitHub Vulnerability Alerts #### [CVE-2025-58160](https://redirect.github.com/tokio-rs/tracing/security/advisories/GHSA-xwfj-jgwm-7wp5) ### Impact Previous versions of tracing-subscriber were vulnerable to ANSI escape sequence injection attacks. Untrusted user input containing ANSI escape sequences could be injected into terminal output when logged, potentially allowing attackers to: - Manipulate terminal title bars - Clear screens or modify terminal display - Potentially mislead users through terminal manipulation In isolation, impact is minimal, however security issues have been found in terminal emulators that enabled an attacker to use ANSI escape sequences via logs to exploit vulnerabilities in the terminal emulator. ### Patches `tracing-subscriber` version 0.3.20 fixes this vulnerability by escaping ANSI control characters in when writing events to destinations that may be printed to the terminal. ### Workarounds Avoid printing logs to terminal emulators without escaping ANSI control sequences. ### References https://www.packetlabs.net/posts/weaponizing-ansi-escape-sequences/ ### Acknowledgments We would like to thank [zefr0x](http://github.com/zefr0x) who responsibly reported the issue at `security@tokio.rs`. If you believe you have found a security vulnerability in any tokio-rs project, please email us at `security@tokio.rs`. --- ### Release Notes
tokio-rs/tracing (tracing-subscriber) ### [`v0.3.20`](https://redirect.github.com/tokio-rs/tracing/releases/tag/tracing-subscriber-0.3.20): tracing-subscriber 0.3.20 [Compare Source](https://redirect.github.com/tokio-rs/tracing/compare/tracing-subscriber-0.3.19...tracing-subscriber-0.3.20) **Security Fix**: ANSI Escape Sequence Injection (CVE-TBD) #### Impact Previous versions of tracing-subscriber were vulnerable to ANSI escape sequence injection attacks. Untrusted user input containing ANSI escape sequences could be injected into terminal output when logged, potentially allowing attackers to: - Manipulate terminal title bars - Clear screens or modify terminal display - Potentially mislead users through terminal manipulation In isolation, impact is minimal, however security issues have been found in terminal emulators that enabled an attacker to use ANSI escape sequences via logs to exploit vulnerabilities in the terminal emulator. #### Solution Version 0.3.20 fixes this vulnerability by escaping ANSI control characters in when writing events to destinations that may be printed to the terminal. #### Affected Versions All versions of tracing-subscriber prior to 0.3.20 are affected by this vulnerability. #### Recommendations Immediate Action Required: We recommend upgrading to tracing-subscriber 0.3.20 immediately, especially if your application: - Logs user-provided input (form data, HTTP headers, query parameters, etc.) - Runs in environments where terminal output is displayed to users #### Migration This is a patch release with no breaking API changes. Simply update your Cargo.toml: ```toml [dependencies] tracing-subscriber = "0.3.20" ``` #### Acknowledgments We would like to thank [zefr0x](http://github.com/zefr0x) who responsibly reported the issue at `security@tokio.rs`. If you believe you have found a security vulnerability in any tokio-rs project, please email us at `security@tokio.rs`.
--- ### Configuration 📅 **Schedule**: Branch creation - "" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/astral-sh/ruff). Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- Cargo.lock | 87 ++++++++++++++++-------------------------------------- 1 file changed, 26 insertions(+), 61 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 27e1808bb4..21536010ec 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -295,7 +295,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "234113d19d0d7d613b40e86fb654acf958910802bcceab913a4f9e7cda03b1a4" dependencies = [ "memchr", - "regex-automata 0.4.10", + "regex-automata", "serde", ] @@ -1231,8 +1231,8 @@ dependencies = [ "aho-corasick", "bstr", "log", - "regex-automata 0.4.10", - "regex-syntax 0.8.5", + "regex-automata", + "regex-syntax", ] [[package]] @@ -1459,7 +1459,7 @@ dependencies = [ "globset", "log", "memchr", - "regex-automata 0.4.10", + "regex-automata", "same-file", "walkdir", "winapi-util", @@ -1913,11 +1913,11 @@ dependencies = [ [[package]] name = "matchers" -version = "0.1.0" +version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8263075bb86c5a1b1427b5ae862e8889656f126e9f77c484496e8b47cf5c5558" +checksum = "d1525a2a28c7f4fa0fc98bb91ae755d1e2d1505079e05539e35bc876b5d65ae9" dependencies = [ - "regex-automata 0.1.10", + "regex-automata", ] [[package]] @@ -2074,12 +2074,11 @@ checksum = "5e0826a989adedc2a244799e823aece04662b66609d96af8dff7ac6df9a8925d" [[package]] name = "nu-ansi-term" -version = "0.46.0" +version = "0.50.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "77a8165726e8236064dbb45459242600304b42a5ea24ee2948e18e023bf7ba84" +checksum = "d4a28e057d01f97e61255210fcff094d74ed0466038633e95017f5beb68e4399" dependencies = [ - "overload", - "winapi", + "windows-sys 0.52.0", ] [[package]] @@ -2154,12 +2153,6 @@ dependencies = [ "memchr", ] -[[package]] -name = "overload" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b15813163c1d831bf4a13c3610c05c0d03b39feb07f7e09fa234dac9b15aaf39" - [[package]] name = "parking_lot" version = "0.12.4" @@ -2688,17 +2681,8 @@ checksum = "23d7fd106d8c02486a8d64e778353d1cffe08ce79ac2e82f540c86d0facf6912" dependencies = [ "aho-corasick", "memchr", - "regex-automata 0.4.10", - "regex-syntax 0.8.5", -] - -[[package]] -name = "regex-automata" -version = "0.1.10" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6c230d73fb8d8c1b9c0b3135c5142a8acee3a0558fb8db5cf1cb65f8d7862132" -dependencies = [ - "regex-syntax 0.6.29", + "regex-automata", + "regex-syntax", ] [[package]] @@ -2709,7 +2693,7 @@ checksum = "6b9458fa0bfeeac22b5ca447c63aaf45f28439a709ccd244698632f9aa6394d6" dependencies = [ "aho-corasick", "memchr", - "regex-syntax 0.8.5", + "regex-syntax", ] [[package]] @@ -2718,12 +2702,6 @@ version = "0.1.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "53a49587ad06b26609c52e423de037e7f57f20d53535d66e08c695f347df952a" -[[package]] -name = "regex-syntax" -version = "0.6.29" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f162c6dd7b008981e4d40210aca20b4bd0f9b60ca9271061b07f78537722f2e1" - [[package]] name = "regex-syntax" version = "0.8.5" @@ -4161,15 +4139,15 @@ dependencies = [ [[package]] name = "tracing-subscriber" -version = "0.3.19" +version = "0.3.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e8189decb5ac0fa7bc8b96b7cb9b2701d60d48805aca84a238004d665fcc4008" +checksum = "2054a14f5307d601f88daf0553e1cbf472acc4f2c51afab632431cdcd72124d5" dependencies = [ "chrono", "matchers", "nu-ansi-term", "once_cell", - "regex", + "regex-automata", "sharded-slab", "smallvec", "thread_local", @@ -4279,7 +4257,7 @@ dependencies = [ "pep440_rs", "rayon", "regex", - "regex-automata 0.4.10", + "regex-automata", "ruff_cache", "ruff_db", "ruff_macros", @@ -4893,22 +4871,6 @@ dependencies = [ "glob", ] -[[package]] -name = "winapi" -version = "0.3.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5c839a674fcd7a98952e593242ea400abe93992746761e38641405d28b00f419" -dependencies = [ - "winapi-i686-pc-windows-gnu", - "winapi-x86_64-pc-windows-gnu", -] - -[[package]] -name = "winapi-i686-pc-windows-gnu" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" - [[package]] name = "winapi-util" version = "0.1.9" @@ -4918,12 +4880,6 @@ dependencies = [ "windows-sys 0.59.0", ] -[[package]] -name = "winapi-x86_64-pc-windows-gnu" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" - [[package]] name = "windows-core" version = "0.61.2" @@ -4983,6 +4939,15 @@ dependencies = [ "windows-link", ] +[[package]] +name = "windows-sys" +version = "0.52.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "282be5f36a8ce781fad8c8ae18fa3f9beff57ec1b52cb3de0789201425d9a33d" +dependencies = [ + "windows-targets 0.52.6", +] + [[package]] name = "windows-sys" version = "0.59.0" From cda376afe079b54b6779704bdd740c9e81423e39 Mon Sep 17 00:00:00 2001 From: Renkai Ge Date: Wed, 3 Sep 2025 23:34:22 +0800 Subject: [PATCH 20/20] [ty]eliminate definitely-impossible types from union in equality narrowing (#20164) solves https://github.com/astral-sh/ty/issues/939 --------- Co-authored-by: Carl Meyer --- .../mdtest/narrow/conditionals/in.md | 103 ++++++++++++++++++ crates/ty_python_semantic/src/types.rs | 18 +-- crates/ty_python_semantic/src/types/narrow.rs | 98 ++++++++++++++--- 3 files changed, 193 insertions(+), 26 deletions(-) diff --git a/crates/ty_python_semantic/resources/mdtest/narrow/conditionals/in.md b/crates/ty_python_semantic/resources/mdtest/narrow/conditionals/in.md index 865eb48788..6090c9c4ac 100644 --- a/crates/ty_python_semantic/resources/mdtest/narrow/conditionals/in.md +++ b/crates/ty_python_semantic/resources/mdtest/narrow/conditionals/in.md @@ -92,3 +92,106 @@ if (x := f()) in (1,): else: reveal_type(x) # revealed: Literal[2, 3] ``` + +## Union with `Literal`, `None` and `int` + +```py +from typing import Literal + +def test(x: Literal["a", "b", "c"] | None | int = None): + if x in ("a", "b"): + # int is included because custom __eq__ methods could make + # an int equal to "a" or "b", so we can't eliminate it + reveal_type(x) # revealed: Literal["a", "b"] | int + else: + reveal_type(x) # revealed: Literal["c"] | None | int +``` + +## Direct `not in` conditional + +```py +from typing import Literal + +def test(x: Literal["a", "b", "c"] | None | int = None): + if x not in ("a", "c"): + # int is included because custom __eq__ methods could make + # an int equal to "a" or "b", so we can't eliminate it + reveal_type(x) # revealed: Literal["b"] | None | int + else: + reveal_type(x) # revealed: Literal["a", "c"] | int +``` + +## bool + +```py +def _(x: bool): + if x in (True,): + reveal_type(x) # revealed: Literal[True] + else: + reveal_type(x) # revealed: Literal[False] + +def _(x: bool | str): + if x in (False,): + # `str` remains due to possible custom __eq__ methods on a subclass + reveal_type(x) # revealed: Literal[False] | str + else: + reveal_type(x) # revealed: Literal[True] | str +``` + +## LiteralString + +```py +from typing_extensions import LiteralString + +def _(x: LiteralString): + if x in ("a", "b", "c"): + reveal_type(x) # revealed: Literal["a", "b", "c"] + else: + reveal_type(x) # revealed: LiteralString & ~Literal["a"] & ~Literal["b"] & ~Literal["c"] + +def _(x: LiteralString | int): + if x in ("a", "b", "c"): + reveal_type(x) # revealed: Literal["a", "b", "c"] | int + else: + reveal_type(x) # revealed: (LiteralString & ~Literal["a"] & ~Literal["b"] & ~Literal["c"]) | int +``` + +## enums + +```py +from enum import Enum + +class Color(Enum): + RED = "red" + GREEN = "green" + BLUE = "blue" + +def _(x: Color): + if x in (Color.RED, Color.GREEN): + # TODO should be `Literal[Color.RED, Color.GREEN]` + reveal_type(x) # revealed: Color + else: + # TODO should be `Literal[Color.BLUE]` + reveal_type(x) # revealed: Color +``` + +## Union with enum and `int` + +```py +from enum import Enum + +class Status(Enum): + PENDING = 1 + APPROVED = 2 + REJECTED = 3 + +def test(x: Status | int): + if x in (Status.PENDING, Status.APPROVED): + # TODO should be `Literal[Status.PENDING, Status.APPROVED] | int` + # int is included because custom __eq__ methods could make + # an int equal to Status.PENDING or Status.APPROVED, so we can't eliminate it + reveal_type(x) # revealed: Status | int + else: + # TODO should be `Literal[Status.REJECTED] | int` + reveal_type(x) # revealed: Status | int +``` diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index 23f4422867..6f62b815db 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -1054,6 +1054,16 @@ impl<'db> Type<'db> { || self.is_literal_string() } + pub(crate) fn is_union_with_single_valued(&self, db: &'db dyn Db) -> bool { + self.into_union().is_some_and(|union| { + union + .elements(db) + .iter() + .any(|ty| ty.is_single_valued(db) || ty.is_bool(db) || ty.is_literal_string()) + }) || self.is_bool(db) + || self.is_literal_string() + } + pub(crate) fn into_string_literal(self) -> Option> { match self { Type::StringLiteral(string_literal) => Some(string_literal), @@ -9953,14 +9963,6 @@ impl<'db> StringLiteralType<'db> { pub(crate) fn python_len(self, db: &'db dyn Db) -> usize { self.value(db).chars().count() } - - /// Return an iterator over each character in the string literal. - /// as would be returned by Python's `iter()`. - pub(crate) fn iter_each_char(self, db: &'db dyn Db) -> impl Iterator { - self.value(db) - .chars() - .map(|c| StringLiteralType::new(db, c.to_string().into_boxed_str())) - } } /// # Ordering diff --git a/crates/ty_python_semantic/src/types/narrow.rs b/crates/ty_python_semantic/src/types/narrow.rs index 46a0b5a8f5..fcfae4851a 100644 --- a/crates/ty_python_semantic/src/types/narrow.rs +++ b/crates/ty_python_semantic/src/types/narrow.rs @@ -615,24 +615,88 @@ impl<'db, 'ast> NarrowingConstraintsBuilder<'db, 'ast> { } } + // TODO `expr_in` and `expr_not_in` should perhaps be unified with `expr_eq` and `expr_ne`, + // since `eq` and `ne` are equivalent to `in` and `not in` with only one element in the RHS. fn evaluate_expr_in(&mut self, lhs_ty: Type<'db>, rhs_ty: Type<'db>) -> Option> { if lhs_ty.is_single_valued(self.db) || lhs_ty.is_union_of_single_valued(self.db) { - if let Type::StringLiteral(string_literal) = rhs_ty { - Some(UnionType::from_elements( - self.db, - string_literal - .iter_each_char(self.db) - .map(Type::StringLiteral), - )) - } else if let Some(tuple_spec) = rhs_ty.tuple_instance_spec(self.db) { - // N.B. Strictly speaking this is unsound, since a tuple subclass might override `__contains__` - // but we'd still apply the narrowing here. This seems unlikely, however, and narrowing is - // generally unsound in numerous ways anyway (attribute narrowing, subscript, narrowing, - // narrowing of globals, etc.). So this doesn't seem worth worrying about too much. - Some(UnionType::from_elements(self.db, tuple_spec.all_elements())) - } else { - None + rhs_ty + .try_iterate(self.db) + .ok() + .map(|iterable| iterable.homogeneous_element_type(self.db)) + } else if lhs_ty.is_union_with_single_valued(self.db) { + let rhs_values = rhs_ty + .try_iterate(self.db) + .ok()? + .homogeneous_element_type(self.db); + + let mut builder = UnionBuilder::new(self.db); + + // Add the narrowed values from the RHS first, to keep literals before broader types. + builder = builder.add(rhs_values); + + if let Some(lhs_union) = lhs_ty.into_union() { + for element in lhs_union.elements(self.db) { + // Keep only the non-single-valued portion of the original type. + if !element.is_single_valued(self.db) + && !element.is_literal_string() + && !element.is_bool(self.db) + { + builder = builder.add(*element); + } + } } + Some(builder.build()) + } else { + None + } + } + + fn evaluate_expr_not_in(&mut self, lhs_ty: Type<'db>, rhs_ty: Type<'db>) -> Option> { + let rhs_values = rhs_ty + .try_iterate(self.db) + .ok()? + .homogeneous_element_type(self.db); + + if lhs_ty.is_single_valued(self.db) || lhs_ty.is_union_of_single_valued(self.db) { + // Exclude the RHS values from the entire (single-valued) LHS domain. + let complement = IntersectionBuilder::new(self.db) + .add_positive(lhs_ty) + .add_negative(rhs_values) + .build(); + Some(complement) + } else if lhs_ty.is_union_with_single_valued(self.db) { + // Split LHS into single-valued portion and the rest. Exclude RHS values from the + // single-valued portion, keep the rest intact. + let mut single_builder = UnionBuilder::new(self.db); + let mut rest_builder = UnionBuilder::new(self.db); + + if let Some(lhs_union) = lhs_ty.into_union() { + for element in lhs_union.elements(self.db) { + if element.is_single_valued(self.db) + || element.is_literal_string() + || element.is_bool(self.db) + { + single_builder = single_builder.add(*element); + } else { + rest_builder = rest_builder.add(*element); + } + } + } + + let single_union = single_builder.build(); + let rest_union = rest_builder.build(); + + let narrowed_single = IntersectionBuilder::new(self.db) + .add_positive(single_union) + .add_negative(rhs_values) + .build(); + + // Keep order: first literal complement, then broader arms. + let result = UnionBuilder::new(self.db) + .add(narrowed_single) + .add(rest_union) + .build(); + Some(result) } else { None } @@ -660,9 +724,7 @@ impl<'db, 'ast> NarrowingConstraintsBuilder<'db, 'ast> { ast::CmpOp::Eq => self.evaluate_expr_eq(lhs_ty, rhs_ty), ast::CmpOp::NotEq => self.evaluate_expr_ne(lhs_ty, rhs_ty), ast::CmpOp::In => self.evaluate_expr_in(lhs_ty, rhs_ty), - ast::CmpOp::NotIn => self - .evaluate_expr_in(lhs_ty, rhs_ty) - .map(|ty| ty.negate(self.db)), + ast::CmpOp::NotIn => self.evaluate_expr_not_in(lhs_ty, rhs_ty), _ => None, } }