diff --git a/.github/workflows/ty-ecosystem-analyzer.yaml b/.github/workflows/ty-ecosystem-analyzer.yaml index 615c5e0997..9f3d8b7db8 100644 --- a/.github/workflows/ty-ecosystem-analyzer.yaml +++ b/.github/workflows/ty-ecosystem-analyzer.yaml @@ -64,11 +64,12 @@ 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 \ diff \ + --profile=release \ --projects-old ruff/projects_old.txt \ --projects-new ruff/projects_new.txt \ --old old_commit \ diff --git a/Cargo.lock b/Cargo.lock index 1a1af3dcb1..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", @@ -4241,6 +4219,7 @@ name = "ty_ide" version = "0.0.0" dependencies = [ "bitflags 2.9.3", + "camino", "get-size2", "insta", "itertools 0.14.0", @@ -4278,7 +4257,7 @@ dependencies = [ "pep440_rs", "rayon", "regex", - "regex-automata 0.4.10", + "regex-automata", "ruff_cache", "ruff_db", "ruff_macros", @@ -4892,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" @@ -4917,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" @@ -4982,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" 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/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/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/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/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/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/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/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__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() 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() - | ^^^^^^^ - | 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. 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 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/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/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_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/all_symbols.rs b/crates/ty_ide/src/all_symbols.rs new file mode 100644 index 0000000000..02ed56d6db --- /dev/null +++ b/crates/ty_ide/src/all_symbols.rs @@ -0,0 +1,182 @@ +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. +/// +/// 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 query = QueryPattern::new(query); + let results = std::sync::Mutex::new(Vec::new()); + { + let modules = all_modules(db); + let db = db.dyn_clone(); + let results = &results; + let query = &query; + + rayon::scope(move |s| { + // For each file, extract symbols and add them to results + 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) { + // 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, + }); + } + }); + } + }); + } + + 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 +/// 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("acegikmo"), @r" + info[all-symbols]: AllSymbolInfo + --> constants.py:2:1 + | + 2 | ABCDEFGHIJKLMNOP = 'https://api.example.com' + | ^^^^^^^^^^^^^^^^ + | + info: Constant ABCDEFGHIJKLMNOP + + info[all-symbols]: AllSymbolInfo + --> models.py:2:7 + | + 2 | class Abcdefghijklmnop: + | ^^^^^^^^^^^^^^^^ + 3 | '''A data model class''' + 4 | def __init__(self): + | + info: Class Abcdefghijklmnop + + info[all-symbols]: AllSymbolInfo + --> utils.py:2:5 + | + 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/completion.rs b/crates/ty_ide/src/completion.rs index 11f648ebd7..8ef7e02e32 100644 --- a/crates/ty_ide/src/completion.rs +++ b/crates/ty_ide/src/completion.rs @@ -7,12 +7,22 @@ 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> { +#[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 { @@ -37,14 +47,31 @@ pub fn completion(db: &dyn Db, file: File, offset: TextSize) -> Vec { model.import_completions() } - CompletionTargetAst::Scoped { node } => model.scoped_completions(node), + CompletionTargetAst::Scoped { node, typed } => { + let mut completions = model.scoped_completions(node); + if settings.auto_import { + 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 + } }; 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, @@ -54,10 +81,12 @@ pub fn completion(db: &dyn Db, file: File, offset: TextSize) -> Vec { pub inner: Completion<'db>, pub documentation: Option, } + impl<'db> std::ops::Deref for DetailedCompletion<'db> { type Target = Completion<'db>; fn deref(&self) -> &Self::Target { @@ -258,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, }) } } @@ -321,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. @@ -505,7 +549,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() { @@ -3013,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 @@ -3031,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 { @@ -3040,7 +3106,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()) + ) + }, ) } @@ -3053,7 +3126,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(); } @@ -3076,7 +3150,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 @@ -3088,7 +3163,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/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 cdb2cd31ed..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() | ^^^^^^^ | @@ -1355,6 +1372,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..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) | ^^^^^^^ | @@ -802,6 +818,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..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 @@ -791,7 +908,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 +1421,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/lib.rs b/crates/ty_ide/src/lib.rs index 6baea75a8f..c481c5ad14 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,7 +25,8 @@ mod stub_mapping; mod symbols; mod workspace_symbols; -pub use completion::completion; +pub use all_symbols::{AllSymbolInfo, all_symbols}; +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}; @@ -286,10 +288,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 +382,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_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 + } } } 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_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/crates/ty_python_semantic/resources/mdtest/annotations/invalid.md b/crates/ty_python_semantic/resources/mdtest/annotations/invalid.md index c4b475da1b..53e81032e8 100644 --- a/crates/ty_python_semantic/resources/mdtest/annotations/invalid.md +++ b/crates/ty_python_semantic/resources/mdtest/annotations/invalid.md @@ -74,10 +74,15 @@ def _( def bar() -> None: return None -async def baz() -> int: - return 42 +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 outer(): # avoid unrelated syntax errors on yield, yield from, and await +async def baz(): + yield + +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" @@ -92,11 +97,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 @@ -109,9 +113,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): 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/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/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/module_resolver/module.rs b/crates/ty_python_semantic/src/module_resolver/module.rs index 3586f52933..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`", @@ -161,9 +175,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) @@ -187,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() } @@ -207,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..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 @@ -173,20 +179,12 @@ 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(), - ty, + name: Name::new(submodule.name(self.db).as_str()), + ty: Some(ty), + kind: None, builtin, }); } @@ -200,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() @@ -236,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, }), ); @@ -286,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 @@ -345,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_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index 460fb65f6e..0ea04b5137 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -1082,6 +1082,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), @@ -10178,14 +10188,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/function.rs b/crates/ty_python_semantic/src/types/function.rs index 9c1bc7a144..6562a68159 100644 --- a/crates/ty_python_semantic/src/types/function.rs +++ b/crates/ty_python_semantic/src/types/function.rs @@ -739,7 +739,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 a1d4268239..d13f34a2da 100644 --- a/crates/ty_python_semantic/src/types/infer.rs +++ b/crates/ty_python_semantic/src/types/infer.rs @@ -9321,7 +9321,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) = 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, } } diff --git a/crates/ty_server/src/server/api/requests/completion.rs b/crates/ty_server/src/server/api/requests/completion.rs index e175ed49fe..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); } @@ -71,7 +74,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_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..c646c767de 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; @@ -148,7 +154,8 @@ 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(); @@ -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 8722afcbe7..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() @@ -425,7 +429,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()) } 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" + ] } ] },