From b6522cb5342d80784f3585729f18d1b3470bd264 Mon Sep 17 00:00:00 2001 From: Kot Date: Thu, 28 Aug 2025 13:35:48 -0700 Subject: [PATCH 01/10] [`pylint`] Add U+061C to `PLE2502` (#20106) Resolves #20058 --- .../fixtures/pylint/bidirectional_unicode.py | 3 ++ crates/ruff_linter/src/preview.rs | 5 ++ crates/ruff_linter/src/rules/pylint/mod.rs | 24 +++++++++ .../pylint/rules/bidirectional_unicode.rs | 11 +++- ...sts__PLE2502_bidirectional_unicode.py.snap | 26 +++++----- ...iew__PLE2502_bidirectional_unicode.py.snap | 52 +++++++++++++++++++ 6 files changed, 106 insertions(+), 15 deletions(-) create mode 100644 crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__preview__PLE2502_bidirectional_unicode.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/bidirectional_unicode.py b/crates/ruff_linter/resources/test/fixtures/pylint/bidirectional_unicode.py index 25accb3aad..f72695d0c4 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/bidirectional_unicode.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/bidirectional_unicode.py @@ -4,6 +4,9 @@ print("שלום‬") # E2502 example = "x‏" * 100 # "‏x" is assigned +# E2502 +another = "x؜" * 50 # "؜x" is assigned + # E2502 if access_level != "none‮⁦": # Check if admin ⁩⁦' and access_level != 'user print("You are an admin.") diff --git a/crates/ruff_linter/src/preview.rs b/crates/ruff_linter/src/preview.rs index 6b6a4d15e5..f932b2b059 100644 --- a/crates/ruff_linter/src/preview.rs +++ b/crates/ruff_linter/src/preview.rs @@ -255,3 +255,8 @@ pub(crate) const fn is_trailing_comma_type_params_enabled(settings: &LinterSetti pub(crate) const fn is_maxsplit_without_separator_fix_enabled(settings: &LinterSettings) -> bool { settings.preview.is_enabled() } + +// https://github.com/astral-sh/ruff/pull/20106 +pub(crate) const fn is_bidi_forbid_arabic_letter_mark_enabled(settings: &LinterSettings) -> bool { + settings.preview.is_enabled() +} diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs index 101c92a16b..e181c0a146 100644 --- a/crates/ruff_linter/src/rules/pylint/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/mod.rs @@ -252,6 +252,30 @@ mod tests { Ok(()) } + #[test_case(Rule::BidirectionalUnicode, Path::new("bidirectional_unicode.py"))] + fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> { + let snapshot = format!( + "preview__{}_{}", + rule_code.noqa_code(), + path.to_string_lossy() + ); + let diagnostics = test_path( + Path::new("pylint").join(path).as_path(), + &LinterSettings { + pylint: pylint::settings::Settings { + allow_dunder_method_names: FxHashSet::from_iter([ + "__special_custom_magic__".to_string() + ]), + ..pylint::settings::Settings::default() + }, + preview: PreviewMode::Enabled, + ..LinterSettings::for_rule(rule_code) + }, + )?; + assert_diagnostics!(snapshot, diagnostics); + Ok(()) + } + #[test] fn continue_in_finally() -> Result<()> { let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/pylint/rules/bidirectional_unicode.rs b/crates/ruff_linter/src/rules/pylint/rules/bidirectional_unicode.rs index fce3e6b36d..0a80f28b45 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/bidirectional_unicode.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/bidirectional_unicode.rs @@ -1,7 +1,9 @@ use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_source_file::Line; -use crate::{Violation, checkers::ast::LintContext}; +use crate::{ + Violation, checkers::ast::LintContext, preview::is_bidi_forbid_arabic_letter_mark_enabled, +}; const BIDI_UNICODE: [char; 10] = [ '\u{202A}', //{LEFT-TO-RIGHT EMBEDDING} @@ -60,7 +62,12 @@ impl Violation for BidirectionalUnicode { /// PLE2502 pub(crate) fn bidirectional_unicode(line: &Line, context: &LintContext) { - if line.contains(BIDI_UNICODE) { + if line.contains(BIDI_UNICODE) + || (is_bidi_forbid_arabic_letter_mark_enabled(context.settings()) + && line.contains( + '\u{061C}', //{ARABIC LETTER MARK} + )) + { context.report_diagnostic(BidirectionalUnicode, line.full_range()); } } diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE2502_bidirectional_unicode.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE2502_bidirectional_unicode.py.snap index 18356831d7..0d584e8108 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE2502_bidirectional_unicode.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE2502_bidirectional_unicode.py.snap @@ -22,21 +22,21 @@ PLE2502 Contains control characters that can permit obfuscated code | PLE2502 Contains control characters that can permit obfuscated code - --> bidirectional_unicode.py:8:1 - | -7 | # E2502 -8 | if access_level != "none": # Check if admin ' and access_level != 'user - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -9 | print("You are an admin.") - | + --> bidirectional_unicode.py:11:1 + | +10 | # E2502 +11 | if access_level != "none": # Check if admin ' and access_level != 'user + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +12 | print("You are an admin.") + | PLE2502 Contains control characters that can permit obfuscated code - --> bidirectional_unicode.py:14:1 + --> bidirectional_unicode.py:17:1 | -12 | # E2502 -13 | def subtract_funds(account: str, amount: int): -14 | """Subtract funds from bank account then """ +15 | # E2502 +16 | def subtract_funds(account: str, amount: int): +17 | """Subtract funds from bank account then """ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -15 | return -16 | bank[account] -= amount +18 | return +19 | bank[account] -= amount | diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__preview__PLE2502_bidirectional_unicode.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__preview__PLE2502_bidirectional_unicode.py.snap new file mode 100644 index 0000000000..4a1555ebe8 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__preview__PLE2502_bidirectional_unicode.py.snap @@ -0,0 +1,52 @@ +--- +source: crates/ruff_linter/src/rules/pylint/mod.rs +--- +PLE2502 Contains control characters that can permit obfuscated code + --> bidirectional_unicode.py:2:1 + | +1 | # E2502 +2 | print("שלום") + | ^^^^^^^^^^^^^ +3 | +4 | # E2502 + | + +PLE2502 Contains control characters that can permit obfuscated code + --> bidirectional_unicode.py:5:1 + | +4 | # E2502 +5 | example = "x‏" * 100 # "‏x" is assigned + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +6 | +7 | # E2502 + | + +PLE2502 Contains control characters that can permit obfuscated code + --> bidirectional_unicode.py:8:1 + | + 7 | # E2502 + 8 | another = "x؜" * 50 # "؜x" is assigned + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 9 | +10 | # E2502 + | + +PLE2502 Contains control characters that can permit obfuscated code + --> bidirectional_unicode.py:11:1 + | +10 | # E2502 +11 | if access_level != "none": # Check if admin ' and access_level != 'user + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +12 | print("You are an admin.") + | + +PLE2502 Contains control characters that can permit obfuscated code + --> bidirectional_unicode.py:17:1 + | +15 | # E2502 +16 | def subtract_funds(account: str, amount: int): +17 | """Subtract funds from bank account then """ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +18 | return +19 | bank[account] -= amount + | From 26082e8ec1c7e89d0206b690b1430cdee5bcece7 Mon Sep 17 00:00:00 2001 From: Dan Parizher <105245560+danparizher@users.noreply.github.com> Date: Thu, 28 Aug 2025 16:58:39 -0400 Subject: [PATCH 02/10] [`ruff`] Fix false negative for empty f-strings in `deque` calls (`RUF037`) (#20109) ## Summary Fixes #20050 --- .../resources/test/fixtures/ruff/RUF037.py | 3 +++ .../unnecessary_literal_within_deque_call.rs | 4 +++- ...r__rules__ruff__tests__RUF037_RUF037.py.snap | 17 +++++++++++++++++ crates/ruff_python_ast/src/helpers.rs | 2 +- 4 files changed, 24 insertions(+), 2 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF037.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF037.py index 75818e741b..e0460df98c 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF037.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF037.py @@ -107,3 +107,6 @@ deque(f"{x}" "") # OK deque(t"") deque(t"" t"") deque(t"{""}") # OK + +# https://github.com/astral-sh/ruff/issues/20050 +deque(f"{""}") # RUF037 diff --git a/crates/ruff_linter/src/rules/ruff/rules/unnecessary_literal_within_deque_call.rs b/crates/ruff_linter/src/rules/ruff/rules/unnecessary_literal_within_deque_call.rs index 88374119c8..0eb3640ad8 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/unnecessary_literal_within_deque_call.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/unnecessary_literal_within_deque_call.rs @@ -1,5 +1,7 @@ use ruff_diagnostics::{Applicability, Edit}; use ruff_macros::{ViolationMetadata, derive_message_formats}; + +use ruff_python_ast::helpers::is_empty_f_string; use ruff_python_ast::parenthesize::parenthesized_range; use ruff_python_ast::{self as ast, Expr}; use ruff_text_size::Ranged; @@ -102,7 +104,7 @@ pub(crate) fn unnecessary_literal_within_deque_call(checker: &Checker, deque: &a } Expr::StringLiteral(string) => string.value.is_empty(), Expr::BytesLiteral(bytes) => bytes.value.is_empty(), - Expr::FString(fstring) => fstring.value.is_empty_literal(), + Expr::FString(fstring) => is_empty_f_string(fstring), Expr::TString(tstring) => tstring.value.is_empty_iterable(), _ => false, }; diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF037_RUF037.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF037_RUF037.py.snap index 8950219905..7334701e64 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF037_RUF037.py.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF037_RUF037.py.snap @@ -369,6 +369,7 @@ help: Replace with `deque()` 107 + deque() 108 | deque(t"" t"") 109 | deque(t"{""}") # OK +110 | RUF037 [*] Unnecessary empty iterable within a deque call --> RUF037.py:108:1 @@ -386,3 +387,19 @@ help: Replace with `deque()` - deque(t"" t"") 108 + deque() 109 | deque(t"{""}") # OK +110 | +111 | # https://github.com/astral-sh/ruff/issues/20050 + +RUF037 [*] Unnecessary empty iterable within a deque call + --> RUF037.py:112:1 + | +111 | # https://github.com/astral-sh/ruff/issues/20050 +112 | deque(f"{""}") # RUF037 + | ^^^^^^^^^^^^^^ + | +help: Replace with `deque()` +109 | deque(t"{""}") # OK +110 | +111 | # https://github.com/astral-sh/ruff/issues/20050 + - deque(f"{""}") # RUF037 +112 + deque() # RUF037 diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index bc1195d347..4390659858 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -1406,7 +1406,7 @@ fn is_non_empty_f_string(expr: &ast::ExprFString) -> bool { /// Returns `true` if the expression definitely resolves to the empty string, when used as an f-string /// expression. -fn is_empty_f_string(expr: &ast::ExprFString) -> bool { +pub fn is_empty_f_string(expr: &ast::ExprFString) -> bool { fn inner(expr: &Expr) -> bool { match expr { Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => value.is_empty(), From 5c2d4d8d8fcec3a57f731c5972119c7fcb470b1e Mon Sep 17 00:00:00 2001 From: Lior Weissman <38353773+liortct@users.noreply.github.com> Date: Fri, 29 Aug 2025 00:37:40 +0300 Subject: [PATCH 03/10] [`perflint`] Handle tuples in dictionary comprehensions (`PERF403`) (#19934) This pull request fixes the bug described in issue [#19153](https://github.com/astral-sh/ruff/issues/19153). The issue occurred when `PERF403` incorrectly flagged cases involving tuple unpacking in a for loop. For example: ```python def f(): v = {} for (o, p), x in [("op", "x")]: v[x] = o, p ``` This code was wrongly suggested to be rewritten into a dictionary comprehension, which changes the semantics. Changes in this PR: Updated the `PERF403` rule to correctly handle tuple unpacking in loop targets. Added regression tests to ensure this case (and similar ones) are no longer flagged incorrectly. Why: This ensures that `PERF403` only triggers when a dictionary comprehension is semantically equivalent to the original loop, preventing false positives. --------- Co-authored-by: Brent Westbrook --- .../test/fixtures/perflint/PERF403.py | 21 ++++++ .../rules/manual_dict_comprehension.rs | 38 +++++++---- ...__perflint__tests__PERF403_PERF403.py.snap | 33 ++++++++++ ...t__tests__preview__PERF403_PERF403.py.snap | 66 ++++++++++++++++++- 4 files changed, 146 insertions(+), 12 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/perflint/PERF403.py b/crates/ruff_linter/resources/test/fixtures/perflint/PERF403.py index 868b268ed1..a393823e61 100644 --- a/crates/ruff_linter/resources/test/fixtures/perflint/PERF403.py +++ b/crates/ruff_linter/resources/test/fixtures/perflint/PERF403.py @@ -192,3 +192,24 @@ def issue_19005_3(): c = {} for a[0], a[1] in (): c[a[0]] = a[1] + + +def issue_19153_1(): + v = {} + for o, (x,) in ["ox"]: + v[x,] = o + return v + + +def issue_19153_2(): + v = {} + for (o, p), x in [("op", "x")]: + v[x] = o, p + return v + + +def issue_19153_3(): + v = {} + for o, (x,) in ["ox"]: + v[(x,)] = o + return v \ No newline at end of file diff --git a/crates/ruff_linter/src/rules/perflint/rules/manual_dict_comprehension.rs b/crates/ruff_linter/src/rules/perflint/rules/manual_dict_comprehension.rs index d67b3803d8..99f34f620a 100644 --- a/crates/ruff_linter/src/rules/perflint/rules/manual_dict_comprehension.rs +++ b/crates/ruff_linter/src/rules/perflint/rules/manual_dict_comprehension.rs @@ -354,21 +354,37 @@ fn convert_to_dict_comprehension( "for" }; // Handles the case where `key` has a trailing comma, e.g, `dict[x,] = y` - let key_range = if let Expr::Tuple(ast::ExprTuple { elts, .. }) = key { - let [expr] = elts.as_slice() else { + let key_str = if let Expr::Tuple(ast::ExprTuple { + elts, + parenthesized, + .. + }) = key + { + if elts.len() != 1 { return None; - }; - expr.range() + } + if *parenthesized { + locator.slice(key).to_string() + } else { + format!("({})", locator.slice(key)) + } } else { - key.range() + locator.slice(key).to_string() }; - let elt_str = format!( - "{}: {}", - locator.slice(key_range), - locator.slice(value.range()) - ); - let comprehension_str = format!("{{{elt_str} {for_type} {target_str} in {iter_str}{if_str}}}"); + // If the value is a tuple without parentheses, add them + let value_str = if let Expr::Tuple(ast::ExprTuple { + parenthesized: false, + .. + }) = value + { + format!("({})", locator.slice(value)) + } else { + locator.slice(value).to_string() + }; + + let comprehension_str = + format!("{{{key_str}: {value_str} {for_type} {target_str} in {iter_str}{if_str}}}"); let for_loop_inline_comments = comment_strings_in_range( checker, diff --git a/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF403_PERF403.py.snap b/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF403_PERF403.py.snap index 30a30469eb..fa06705416 100644 --- a/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF403_PERF403.py.snap +++ b/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF403_PERF403.py.snap @@ -176,3 +176,36 @@ PERF403 Use a dictionary comprehension instead of a for-loop | ^^^^^^^ | help: Replace for loop with dict comprehension + +PERF403 Use a dictionary comprehension instead of a for-loop + --> PERF403.py:200:9 + | +198 | v = {} +199 | for o, (x,) in ["ox"]: +200 | v[x,] = o + | ^^^^^^^^^ +201 | return v + | +help: Replace for loop with dict comprehension + +PERF403 Use a dictionary comprehension instead of a for-loop + --> PERF403.py:207:9 + | +205 | v = {} +206 | for (o, p), x in [("op", "x")]: +207 | v[x] = o, p + | ^^^^^^^^^^^ +208 | return v + | +help: Replace for loop with dict comprehension + +PERF403 Use a dictionary comprehension instead of a for-loop + --> PERF403.py:214:9 + | +212 | v = {} +213 | for o, (x,) in ["ox"]: +214 | v[(x,)] = o + | ^^^^^^^^^^^ +215 | return v + | +help: Replace for loop with dict comprehension diff --git a/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__preview__PERF403_PERF403.py.snap b/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__preview__PERF403_PERF403.py.snap index dbd210aae3..810e014b2e 100644 --- a/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__preview__PERF403_PERF403.py.snap +++ b/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__preview__PERF403_PERF403.py.snap @@ -372,8 +372,72 @@ help: Replace for loop with dict comprehension - v = {} - for o,(x,)in(): - v[x,]=o -170 + v = {x: o for o,(x,) in ()} +170 + v = {(x,): o for o,(x,) in ()} 171 | 172 | 173 | # https://github.com/astral-sh/ruff/issues/19005 note: This is an unsafe fix and may change runtime behavior + +PERF403 [*] Use a dictionary comprehension instead of a for-loop + --> PERF403.py:200:9 + | +198 | v = {} +199 | for o, (x,) in ["ox"]: +200 | v[x,] = o + | ^^^^^^^^^ +201 | return v + | +help: Replace for loop with dict comprehension +195 | +196 | +197 | def issue_19153_1(): + - v = {} + - for o, (x,) in ["ox"]: + - v[x,] = o +198 + v = {(x,): o for o, (x,) in ["ox"]} +199 | return v +200 | +201 | +note: This is an unsafe fix and may change runtime behavior + +PERF403 [*] Use a dictionary comprehension instead of a for-loop + --> PERF403.py:207:9 + | +205 | v = {} +206 | for (o, p), x in [("op", "x")]: +207 | v[x] = o, p + | ^^^^^^^^^^^ +208 | return v + | +help: Replace for loop with dict comprehension +202 | +203 | +204 | def issue_19153_2(): + - v = {} + - for (o, p), x in [("op", "x")]: + - v[x] = o, p +205 + v = {x: (o, p) for (o, p), x in [("op", "x")]} +206 | return v +207 | +208 | +note: This is an unsafe fix and may change runtime behavior + +PERF403 [*] Use a dictionary comprehension instead of a for-loop + --> PERF403.py:214:9 + | +212 | v = {} +213 | for o, (x,) in ["ox"]: +214 | v[(x,)] = o + | ^^^^^^^^^^^ +215 | return v + | +help: Replace for loop with dict comprehension +209 | +210 | +211 | def issue_19153_3(): + - v = {} + - for o, (x,) in ["ox"]: + - v[(x,)] = o +212 + v = {(x,): o for o, (x,) in ["ox"]} +213 | return v +note: This is an unsafe fix and may change runtime behavior From a8039f80f06a1437bead4f1eb5a42888c73ee2e7 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Thu, 28 Aug 2025 20:04:29 -0400 Subject: [PATCH 04/10] [ty] Add constraint set implementation (#19997) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR adds an implementation of constraint sets. An individual constraint restricts the specialization of a single typevar to be within a particular lower and upper bound: the typevar can only specialize to types that are a supertype of the lower bound, and a subtype of the upper bound. (Note that lower and upper bounds are fully static; we take the bottom and top materializations of the bounds to remove any gradual forms if needed.) Either bound can be “closed” (where the bound is a valid specialization), or “open” (where it is not). You can then build up more complex constraint sets using union, intersection, and negation operations. We use a disjunctive normal form (DNF) representation, just like we do for types: a _constraint set_ is the union of zero or more _clauses_, each of which is the intersection of zero or more individual constraints. Note that the constraint set that contains no clauses is never satisfiable (`⋃ {} = 0`); and the constraint set that contains a single clause, which contains no constraints, is always satisfiable (`⋃ {⋂ {}} = 1`). One thing to note is that this PR does not change the logic of the actual assignability checks, and in particular, we still aren't ever trying to create an "individual constraint" that constrains a typevar. Technically we're still operating only on `bool`s, since we only ever instantiate `C::always_satisfiable` (i.e., `true`) and `C::unsatisfiable` (i.e., `false`) in the `has_relation_to` methods. So if you thought that #19838 introduced an unnecessarily complex stand-in for `bool`, well here you go, this one is worse! (But still seemingly not yielding a performance regression!) The next PR in this series, #20093, is where we will actually create some non-trivial constraint sets and use them in anger. That said, the PR does go ahead and update the assignability checks to use the new `ConstraintSet` type instead of `bool`. That part is fairly straightforward since we had already updated the assignability checks to use the `Constraints` trait; we just have to actively choose a different impl type. (For the `is_whatever` variants, which still return a `bool`, we have to convert the constraint set, but the explicit `is_always_satisfiable` calls serve as nice documentation of our intent.) --------- Co-authored-by: Alex Waygood Co-authored-by: Carl Meyer --- crates/ty_python_semantic/src/types.rs | 24 +- .../ty_python_semantic/src/types/call/bind.rs | 12 +- crates/ty_python_semantic/src/types/class.rs | 5 +- .../src/types/constraints.rs | 1065 ++++++++++++++++- .../ty_python_semantic/src/types/display.rs | 36 +- .../ty_python_semantic/src/types/instance.rs | 17 +- .../src/types/signatures.rs | 8 +- 7 files changed, 1098 insertions(+), 69 deletions(-) diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index 69dd6b1dc4..5a910fc1c3 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -39,7 +39,7 @@ use crate::suppression::check_suppressions; use crate::types::call::{Binding, Bindings, CallArguments, CallableBinding}; pub(crate) use crate::types::class_base::ClassBase; use crate::types::constraints::{ - Constraints, IteratorConstraintsExtension, OptionConstraintsExtension, + ConstraintSet, Constraints, IteratorConstraintsExtension, OptionConstraintsExtension, }; use crate::types::context::{LintDiagnosticGuard, LintDiagnosticGuardBuilder}; use crate::types::diagnostic::{INVALID_AWAIT, INVALID_TYPE_FORM, UNSUPPORTED_BOOL_CONVERSION}; @@ -1360,7 +1360,8 @@ impl<'db> Type<'db> { /// intersection simplification dependent on the order in which elements are added), so we do /// not use this more general definition of subtyping. pub(crate) fn is_subtype_of(self, db: &'db dyn Db, target: Type<'db>) -> bool { - self.when_subtype_of(db, target) + self.when_subtype_of::(db, target) + .is_always_satisfied(db) } fn when_subtype_of>(self, db: &'db dyn Db, target: Type<'db>) -> C { @@ -1371,7 +1372,8 @@ impl<'db> Type<'db> { /// /// [assignable to]: https://typing.python.org/en/latest/spec/concepts.html#the-assignable-to-or-consistent-subtyping-relation pub(crate) fn is_assignable_to(self, db: &'db dyn Db, target: Type<'db>) -> bool { - self.when_assignable_to(db, target) + self.when_assignable_to::(db, target) + .is_always_satisfied(db) } fn when_assignable_to>(self, db: &'db dyn Db, target: Type<'db>) -> C { @@ -1849,7 +1851,8 @@ impl<'db> Type<'db> { /// /// [equivalent to]: https://typing.python.org/en/latest/spec/glossary.html#term-equivalent pub(crate) fn is_equivalent_to(self, db: &'db dyn Db, other: Type<'db>) -> bool { - self.when_equivalent_to(db, other) + self.when_equivalent_to::(db, other) + .is_always_satisfied(db) } fn when_equivalent_to>(self, db: &'db dyn Db, other: Type<'db>) -> C { @@ -1949,7 +1952,8 @@ impl<'db> Type<'db> { /// Note: This function aims to have no false positives, but might return /// wrong `false` answers in some cases. pub(crate) fn is_disjoint_from(self, db: &'db dyn Db, other: Type<'db>) -> bool { - self.when_disjoint_from(db, other) + self.when_disjoint_from::(db, other) + .is_always_satisfied(db) } fn when_disjoint_from>(self, db: &'db dyn Db, other: Type<'db>) -> C { @@ -9701,6 +9705,16 @@ pub(super) fn walk_intersection_type<'db, V: visitor::TypeVisitor<'db> + ?Sized> } impl<'db> IntersectionType<'db> { + pub(crate) fn from_elements(db: &'db dyn Db, elements: I) -> Type<'db> + where + I: IntoIterator, + T: Into>, + { + IntersectionBuilder::new(db) + .positive_elements(elements) + .build() + } + /// Return a new `IntersectionType` instance with the positive and negative types sorted /// according to a canonical ordering, and other normalizations applied to each element as applicable. /// diff --git a/crates/ty_python_semantic/src/types/call/bind.rs b/crates/ty_python_semantic/src/types/call/bind.rs index 4933e515e6..7627602fa6 100644 --- a/crates/ty_python_semantic/src/types/call/bind.rs +++ b/crates/ty_python_semantic/src/types/call/bind.rs @@ -17,6 +17,7 @@ use crate::db::Db; use crate::dunder_all::dunder_all_names; use crate::place::{Boundness, Place}; use crate::types::call::arguments::{Expansion, is_expandable_type}; +use crate::types::constraints::{ConstraintSet, Constraints}; use crate::types::diagnostic::{ CALL_NON_CALLABLE, CONFLICTING_ARGUMENT_FORMS, INVALID_ARGUMENT_TYPE, MISSING_ARGUMENT, NO_MATCHING_OVERLOAD, PARAMETER_ALREADY_ASSIGNED, TOO_MANY_POSITIONAL_ARGUMENTS, @@ -2198,7 +2199,16 @@ impl<'a, 'db> ArgumentTypeChecker<'a, 'db> { argument_type.apply_specialization(self.db, inherited_specialization); expected_ty = expected_ty.apply_specialization(self.db, inherited_specialization); } - if !argument_type.is_assignable_to(self.db, expected_ty) { + // This is one of the few places where we want to check if there's _any_ specialization + // where assignability holds; normally we want to check that assignability holds for + // _all_ specializations. + // TODO: Soon we will go further, and build the actual specializations from the + // constraint set that we get from this assignability check, instead of inferring and + // building them in an earlier separate step. + if argument_type + .when_assignable_to::(self.db, expected_ty) + .is_never_satisfied(self.db) + { let positional = matches!(argument, Argument::Positional | Argument::Synthetic) && !parameter.is_variadic(); self.errors.push(BindingError::InvalidArgumentType { diff --git a/crates/ty_python_semantic/src/types/class.rs b/crates/ty_python_semantic/src/types/class.rs index f33ab47f18..7f4175eb37 100644 --- a/crates/ty_python_semantic/src/types/class.rs +++ b/crates/ty_python_semantic/src/types/class.rs @@ -18,7 +18,7 @@ use crate::semantic_index::{ BindingWithConstraints, DeclarationWithConstraint, SemanticIndex, attribute_declarations, attribute_scopes, }; -use crate::types::constraints::{Constraints, IteratorConstraintsExtension}; +use crate::types::constraints::{ConstraintSet, Constraints, IteratorConstraintsExtension}; use crate::types::context::InferContext; use crate::types::diagnostic::{INVALID_LEGACY_TYPE_VARIABLE, INVALID_TYPE_ALIAS_TYPE}; use crate::types::enums::enum_metadata; @@ -552,7 +552,8 @@ impl<'db> ClassType<'db> { /// Return `true` if `other` is present in this class's MRO. pub(super) fn is_subclass_of(self, db: &'db dyn Db, other: ClassType<'db>) -> bool { - self.when_subclass_of(db, other) + self.when_subclass_of::(db, other) + .is_always_satisfied(db) } pub(super) fn when_subclass_of>( diff --git a/crates/ty_python_semantic/src/types/constraints.rs b/crates/ty_python_semantic/src/types/constraints.rs index ddf003adf3..77c968b535 100644 --- a/crates/ty_python_semantic/src/types/constraints.rs +++ b/crates/ty_python_semantic/src/types/constraints.rs @@ -12,21 +12,73 @@ //! question is: "Under what constraints is this type assignable to another?". //! //! This module provides the machinery for representing the "under what constraints" part of that -//! question. An individual constraint restricts the specialization of a single typevar to be within a -//! particular lower and upper bound. You can then build up more complex constraint sets using -//! union, intersection, and negation operations (just like types themselves). +//! question. //! -//! NOTE: This module is currently in a transitional state: we've added a trait that our constraint -//! set implementations will conform to, and updated all of our type property implementations to -//! work on any impl of that trait. But the only impl we have right now is `bool`, which means that -//! we are still not tracking the full detail as promised in the description above. (`bool` is a -//! perfectly fine impl, but it can generate false positives when you have to break down a -//! particular assignability check into subchecks: each subcheck might say "yes", but technically -//! under conflicting constraints, which a single `bool` can't track.) Soon we will add a proper -//! constraint set implementation, and the `bool` impl of the trait (and possibly the trait itself) -//! will go away. +//! An individual constraint restricts the specialization of a single typevar to be within a +//! particular lower and upper bound: the typevar can only specialize to a type that is a supertype +//! of the lower bound, and a subtype of the upper bound. (Note that lower and upper bounds are +//! fully static; we take the bottom and top materializations of the bounds to remove any gradual +//! forms if needed.) Either bound can be "closed" (where the bound is a valid specialization), or +//! "open" (where it is not). +//! +//! You can then build up more complex constraint sets using union, intersection, and negation +//! operations. We use a disjunctive normal form (DNF) representation, just like we do for types: a +//! [constraint set][ConstraintSet] is the union of zero or more [clauses][ConstraintClause], each +//! of which is the intersection of zero or more [individual constraints][AtomicConstraint]. Note +//! that the constraint set that contains no clauses is never satisfiable (`⋃ {} = 0`); and the +//! constraint set that contains a single clause, where that clause contains no constraints, +//! is always satisfiable (`⋃ {⋂ {}} = 1`). +//! +//! NOTE: This module is currently in a transitional state: we've added a [`Constraints`] trait, +//! and updated all of our type property implementations to work on any impl of that trait. We have +//! added the DNF [`ConstraintSet`] representation, and updated all of our property checks to build +//! up a constraint set and then check whether it is ever or always satisfiable, as appropriate. We +//! are not yet inferring specializations from those constraints, and we will likely remove the +//! [`Constraints`] trait once everything has stabilized. +//! +//! ### Examples +//! +//! For instance, in the following Python code: +//! +//! ```py +//! class A: ... +//! class B(A): ... +//! +//! def _[T: B](t: T) -> None: ... +//! def _[U: (int, str)](u: U) -> None: ... +//! ``` +//! +//! The typevar `T` has an upper bound of `B`, which would translate into the constraint +//! `Never ≤ T ≤ B`. (Every type is a supertype of `Never`, so having `Never` as a closed lower +//! bound means that there is effectively no lower bound. Similarly, a closed upper bound of +//! `object` means that there is effectively no upper bound.) The `T ≤ B` part expresses that the +//! type can specialize to any type that is a subtype of B. The bound is "closed", which means that +//! this includes `B` itself. +//! +//! The typevar `U` is constrained to be either `int` or `str`, which would translate into the +//! constraint `(int ≤ T ≤ int) ∪ (str ≤ T ≤ str)`. When the lower and upper bounds are the same +//! (and both closed), the constraint says that the typevar must specialize to that _exact_ type, +//! not to a subtype or supertype of it. +//! +//! Python does not give us an easy way to construct this, but we can also consider a typevar that +//! can specialize to any type that `T` _cannot_ specialize to — that is, the negation of `T`'s +//! constraint. Another way to write `Never ≤ V ≤ B` is `Never ≤ V ∩ V ≤ B`; if we negate that, we +//! get `¬(Never ≤ V) ∪ ¬(V ≤ B)`, or `V < Never ∪ B < V`. Note that the bounds in this constraint +//! are now open! `B < V` indicates that `V` can specialize to any type that is a supertype of `B` +//! — but not to `B` itself. (For instance, it _can_ specialize to `A`.) `V < Never` is also open, +//! and says that `V` can specialize to any type that is a subtype of `Never`, but not to `Never` +//! itself. There aren't any types that satisfy that constraint (the type would have to somehow +//! contain a negative number of values). You can think of a constraint that cannot be satisfied as +//! an empty set (of types), which means we can simplify it out of the union. That gives us a final +//! constraint of `B < V` for the negation of `T`'s constraint. + +use std::fmt::Display; + +use itertools::{EitherOrBoth, Itertools}; +use smallvec::{SmallVec, smallvec}; use crate::Db; +use crate::types::{BoundTypeVarInstance, IntersectionType, Type, UnionType}; /// Encodes the constraints under which a type property (e.g. assignability) holds. pub(crate) trait Constraints<'db>: Clone + Sized { @@ -79,38 +131,12 @@ pub(crate) trait Constraints<'db>: Clone + Sized { } self } -} -impl<'db> Constraints<'db> for bool { - fn unsatisfiable(_db: &'db dyn Db) -> Self { - false - } - - fn always_satisfiable(_db: &'db dyn Db) -> Self { - true - } - - fn is_never_satisfied(&self, _db: &'db dyn Db) -> bool { - !*self - } - - fn is_always_satisfied(&self, _db: &'db dyn Db) -> bool { - *self - } - - fn union(&mut self, _db: &'db dyn Db, other: Self) -> &Self { - *self = *self || other; - self - } - - fn intersect(&mut self, _db: &'db dyn Db, other: Self) -> &Self { - *self = *self && other; - self - } - - fn negate(self, _db: &'db dyn Db) -> Self { - !self - } + // This is here so that we can easily print constraint sets when debugging. + // TODO: Add a ty_extensions function to reveal constraint sets so that this is no longer dead + // code, and so that we verify the contents of our rendering. + #[expect(dead_code)] + fn display(&self, db: &'db dyn Db) -> impl Display; } /// An extension trait for building constraint sets from [`Option`] values. @@ -182,3 +208,956 @@ where result } } + +/// A set of constraints under which a type property holds. +/// +/// We use a DNF representation, so a set contains a list of zero or more +/// [clauses][ConstraintClause], each of which is an intersection of zero or more +/// [constraints][AtomicConstraint]. +/// +/// This is called a "set of constraint sets", and denoted _𝒮_, in [[POPL2015][]]. +/// +/// ### Invariants +/// +/// - The clauses are simplified as much as possible — there are no two clauses in the set that can +/// be simplified into a single clause. +/// +/// [POPL2015]: https://doi.org/10.1145/2676726.2676991 +#[derive(Clone, Debug)] +pub(crate) struct ConstraintSet<'db> { + // NOTE: We use 2 here because there are a couple of places where we create unions of 2 clauses + // as temporary values — in particular when negating a constraint — and this lets us avoid + // spilling the temporary value to the heap. + clauses: SmallVec<[ConstraintClause<'db>; 2]>, +} + +impl<'db> ConstraintSet<'db> { + /// Returns the constraint set that is never satisfiable. + fn never() -> Self { + Self { + clauses: smallvec![], + } + } + + /// Returns a constraint set that contains a single clause. + fn singleton(clause: ConstraintClause<'db>) -> Self { + Self { + clauses: smallvec![clause], + } + } + + /// Updates this set to be the union of itself and a constraint. + fn union_constraint( + &mut self, + db: &'db dyn Db, + constraint: Satisfiable>, + ) { + match constraint { + // ... ∪ 0 = ... + Satisfiable::Never => {} + // ... ∪ 1 = 1 + Satisfiable::Always => { + self.clauses.clear(); + self.clauses.push(ConstraintClause::always()); + } + // Otherwise wrap the constraint into a singleton clause and use the logic below to add + // it. + Satisfiable::Constrained(constraint) => { + self.union_clause(db, ConstraintClause::singleton(constraint)); + } + } + } + + /// Updates this set to be the union of itself and a clause. To maintain the invariants of this + /// type, we must simplify this clause against all existing clauses, if possible. + fn union_clause(&mut self, db: &'db dyn Db, mut clause: ConstraintClause<'db>) { + // Naively, we would just append the new clause to the set's list of clauses. But that + // doesn't ensure that the clauses are simplified with respect to each other. So instead, + // we iterate through the list of existing clauses, and try to simplify the new clause + // against each one in turn. (We can assume that the existing clauses are already + // simplified with respect to each other, since we can assume that the invariant holds upon + // entry to this method.) + let mut existing_clauses = std::mem::take(&mut self.clauses).into_iter(); + for existing in existing_clauses.by_ref() { + // Try to simplify the new clause against an existing clause. + match existing.simplify_clauses(db, clause) { + Simplifiable::NeverSatisfiable => { + // If two clauses cancel out to 0, that does NOT cause the entire set to become + // 0. We need to keep whatever clauses have already been added to the result, + // and also need to copy over any later clauses that we hadn't processed yet. + self.clauses.extend(existing_clauses); + return; + } + + Simplifiable::AlwaysSatisfiable => { + // If two clauses cancel out to 1, that makes the entire set 1, and all + // existing clauses are simplified away. + self.clauses.clear(); + self.clauses.push(ConstraintClause::always()); + return; + } + + Simplifiable::NotSimplified(existing, c) => { + // We couldn't simplify the new clause relative to this existing clause, so add + // the existing clause to the result. Continue trying to simplify the new + // clause against the later existing clauses. + self.clauses.push(existing); + clause = c; + } + + Simplifiable::Simplified(c) => { + // We were able to simplify the new clause relative to this existing clause. + // Don't add it to the result yet; instead, try to simplify the result further + // against later existing clauses. + clause = c; + } + } + } + + // If we fall through then we need to add the new clause to the clause list (either because + // we couldn't simplify it with anything, or because we did without it canceling out). + self.clauses.push(clause); + } + + /// Updates this set to be the union of itself and another set. + fn union_set(&mut self, db: &'db dyn Db, other: Self) { + for clause in other.clauses { + self.union_clause(db, clause); + } + } + + /// Updates this set to be the intersection of itself and another set. + fn intersect_set(&mut self, db: &'db dyn Db, other: &Self) { + // This is the distributive law: + // (A ∪ B) ∩ (C ∪ D ∪ E) = (A ∩ C) ∪ (A ∩ D) ∪ (A ∩ E) ∪ (B ∩ C) ∪ (B ∩ D) ∪ (B ∩ E) + let self_clauses = std::mem::take(&mut self.clauses); + for self_clause in &self_clauses { + for other_clause in &other.clauses { + match self_clause.intersect_clause(db, other_clause) { + Satisfiable::Never => continue, + Satisfiable::Always => { + self.clauses.clear(); + self.clauses.push(ConstraintClause::always()); + return; + } + Satisfiable::Constrained(clause) => self.union_clause(db, clause), + } + } + } + } + + // This is here so that we can easily print constraint sets when debugging. + // TODO: Add a ty_extensions function to reveal constraint sets so that this is no longer dead + // code, and so that we verify the contents of our rendering. + #[expect(dead_code)] + pub(crate) fn display(&self, db: &'db dyn Db) -> impl Display { + struct DisplayConstraintSet<'a, 'db> { + set: &'a ConstraintSet<'db>, + db: &'db dyn Db, + } + + impl Display for DisplayConstraintSet<'_, '_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if self.set.clauses.is_empty() { + return f.write_str("0"); + } + for (i, clause) in self.set.clauses.iter().enumerate() { + if i > 0 { + f.write_str(" ∨ ")?; + } + clause.display(self.db).fmt(f)?; + } + Ok(()) + } + } + + DisplayConstraintSet { set: self, db } + } +} + +impl<'db> Constraints<'db> for ConstraintSet<'db> { + fn unsatisfiable(_db: &'db dyn Db) -> Self { + Self::never() + } + + fn always_satisfiable(_db: &'db dyn Db) -> Self { + Self::singleton(ConstraintClause::always()) + } + + fn is_never_satisfied(&self, _db: &'db dyn Db) -> bool { + self.clauses.is_empty() + } + + fn is_always_satisfied(&self, _db: &'db dyn Db) -> bool { + self.clauses.len() == 1 && self.clauses[0].is_always() + } + + fn union(&mut self, db: &'db dyn Db, other: Self) -> &Self { + self.union_set(db, other); + self + } + + fn intersect(&mut self, db: &'db dyn Db, other: Self) -> &Self { + self.intersect_set(db, &other); + self + } + + fn negate(self, db: &'db dyn Db) -> Self { + let mut result = Self::always_satisfiable(db); + for clause in self.clauses { + result.intersect_set(db, &clause.negate(db)); + } + result + } + + fn display(&self, db: &'db dyn Db) -> impl Display { + self.display(db) + } +} + +/// The intersection of zero or more atomic constraints. +/// +/// This is called a "constraint set", and denoted _C_, in [[POPL2015][]]. +/// +/// ### Invariants +/// +/// - No two constraints in the clause will constrain the same typevar. +/// - The constraints are sorted by typevar. +/// +/// [POPL2015]: https://doi.org/10.1145/2676726.2676991 +#[derive(Clone, Debug)] +pub(crate) struct ConstraintClause<'db> { + // NOTE: We use 1 here because most clauses only mention a single typevar. + constraints: SmallVec<[AtomicConstraint<'db>; 1]>, +} + +impl<'db> ConstraintClause<'db> { + /// Returns the clause that is always satisfiable. + fn always() -> Self { + Self { + constraints: smallvec![], + } + } + + /// Returns a clause containing a single constraint. + fn singleton(constraint: AtomicConstraint<'db>) -> Self { + Self { + constraints: smallvec![constraint], + } + } + + /// Returns whether this constraint is always satisfiable. + fn is_always(&self) -> bool { + self.constraints.is_empty() + } + + /// Updates this clause to be the intersection of itself and an atomic constraint. Returns a + /// flag indicating whether the updated clause is never, always, or sometimes satisfied. + fn intersect_constraint( + &mut self, + db: &'db dyn Db, + constraint: &AtomicConstraint<'db>, + ) -> Satisfiable<()> { + // If the clause does not already contain a constraint for this typevar, we just insert the + // new constraint into the clause and return. + let index = match (self.constraints) + .binary_search_by_key(&constraint.typevar, |existing| existing.typevar) + { + Ok(index) => index, + Err(index) => { + self.constraints.insert(index, constraint.clone()); + return Satisfiable::Constrained(()); + } + }; + + // If the clause already contains a constraint for this typevar, we need to intersect the + // existing and new constraints, and simplify the clause accordingly. + match self.constraints[index].intersect(db, constraint) { + // ... ∩ 0 ∩ ... == 0 + // If the intersected constraint cannot be satisfied, that causes this whole clause to + // be unsatisfiable too. + Satisfiable::Never => Satisfiable::Never, + + // ... ∩ 1 ∩ ... == ... + // If the intersected result is always satisfied, then the constraint no longer + // contributes anything to the clause, and can be removed. + Satisfiable::Always => { + self.constraints.remove(index); + if self.is_always() { + // If there are no further constraints in the clause, the clause is now always + // satisfied. + Satisfiable::Always + } else { + Satisfiable::Constrained(()) + } + } + + // ... ∩ X ∩ ... == ... ∩ X ∩ ... + // If the intersection is a single constraint, we can reuse the existing constraint's + // place in the clause's constraint list. + Satisfiable::Constrained(constraint) => { + self.constraints[index] = constraint; + Satisfiable::Constrained(()) + } + } + } + + /// Returns the intersection of this clause with another. + fn intersect_clause(&self, db: &'db dyn Db, other: &Self) -> Satisfiable { + // Add each `other` constraint in turn. Short-circuit if the result ever becomes 0. + let mut result = self.clone(); + for constraint in &other.constraints { + match result.intersect_constraint(db, constraint) { + Satisfiable::Never => return Satisfiable::Never, + Satisfiable::Always | Satisfiable::Constrained(()) => {} + } + } + if result.is_always() { + Satisfiable::Always + } else { + Satisfiable::Constrained(result) + } + } + + /// Tries to simplify the union of two clauses into a single clause, if possible. + fn simplify_clauses(self, db: &'db dyn Db, other: Self) -> Simplifiable { + // Saturation + // + // If either clause is always satisfiable, the union is too. (`1 ∪ C₂ = 1`, `C₁ ∪ 1 = 1`) + // + // ```py + // class A[T]: ... + // + // class C1[U]: + // # T can specialize to any type, so this is "always satisfiable", or `1` + // x: A[U] + // + // class C2[V: int]: + // # `T ≤ int` + // x: A[V] + // + // class Saturation[U, V: int]: + // # `1 ∪ (T ≤ int)` + // # simplifies via saturation to + // # `T ≤ int` + // x: A[U] | A[V] + // ``` + if self.is_always() || other.is_always() { + return Simplifiable::Simplified(Self::always()); + } + + // Subsumption + // + // If either clause subsumes (is "smaller than") the other, then the union simplifies to + // the "bigger" clause (the one being subsumed): + // + // - `A ∩ B` must be at least as large as `A ∩ B ∩ C` + // - Therefore, `(A ∩ B) ∪ (A ∩ B ∩ C) = (A ∩ B)` + // + // (Note that possibly counterintuitively, "bigger" here means _fewer_ constraints in the + // intersection, since intersecting more things can only make the result smaller.) + // + // ```py + // class A[T, U, V]: ... + // + // class C1[X: int, Y: str, Z]: + // # `(T ≤ int ∩ U ≤ str)` + // x: A[X, Y, Z] + // + // class C2[X: int, Y: str, Z: bytes]: + // # `(T ≤ int ∩ U ≤ str ∩ V ≤ bytes)` + // x: A[X, Y, Z] + // + // class Subsumption[X1: int, Y1: str, Z2, X2: int, Y2: str, Z2: bytes]: + // # `(T ≤ int ∩ U ≤ str) ∪ (T ≤ int ∩ U ≤ str ∩ V ≤ bytes)` + // # simplifies via subsumption to + // # `(T ≤ int ∩ U ≤ str)` + // x: A[X1, Y1, Z2] | A[X2, Y2, Z2] + // ``` + // + // TODO: Consider checking both directions in one pass, possibly via a tri-valued return + // value. + if self.subsumes_via_intersection(db, &other) { + return Simplifiable::Simplified(other); + } + if other.subsumes_via_intersection(db, &self) { + return Simplifiable::Simplified(self); + } + + // Distribution + // + // If the two clauses constrain the same typevar in an "overlapping" way, we can factor + // that out: + // + // (A₁ ∩ B ∩ C) ∪ (A₂ ∩ B ∩ C) = (A₁ ∪ A₂) ∩ B ∩ C + // + // ```py + // class A[T, U, V]: ... + // + // class C1[X: int, Y: str, Z: bytes]: + // # `(T ≤ int ∩ U ≤ str ∩ V ≤ bytes)` + // x: A[X, Y, Z] + // + // class C2[X: bool, Y: str, Z: bytes]: + // # `(T ≤ bool ∩ U ≤ str ∩ V ≤ bytes)` + // x: A[X, Y, Z] + // + // class Distribution[X1: int, Y1: str, Z2: bytes, X2: bool, Y2: str, Z2: bytes]: + // # `(T ≤ int ∩ U ≤ str ∩ V ≤ bytes) ∪ (T ≤ bool ∩ U ≤ str ∩ V ≤ bytes)` + // # simplifies via distribution to + // # `(T ≤ int ∪ T ≤ bool) ∩ U ≤ str ∩ V ≤ bytes)` + // # which (because `bool ≤ int`) is equivalent to + // # `(T ≤ int ∩ U ≤ str ∩ V ≤ bytes)` + // x: A[X1, Y1, Z2] | A[X2, Y2, Z2] + // ``` + if let Some(simplified) = self.simplifies_via_distribution(db, &other) { + if simplified.is_always() { + return Simplifiable::AlwaysSatisfiable; + } + return Simplifiable::Simplified(simplified); + } + + // Can't be simplified + Simplifiable::NotSimplified(self, other) + } + + /// Returns whether this clause subsumes `other` via intersection — that is, if the + /// intersection of `self` and `other` is `self`. + fn subsumes_via_intersection(&self, db: &'db dyn Db, other: &Self) -> bool { + // See the notes in `simplify_clauses` for more details on subsumption, including Python + // examples that cause it to fire. + + if self.constraints.len() != other.constraints.len() { + return false; + } + + let pairwise = (self.constraints.iter()) + .merge_join_by(&other.constraints, |a, b| a.typevar.cmp(&b.typevar)); + for pair in pairwise { + match pair { + // `other` contains a constraint whose typevar doesn't appear in `self`, so `self` + // cannot be smaller. + EitherOrBoth::Right(_) => return false, + + // `self` contains a constraint whose typevar doesn't appear in `other`. `self` + // might be smaller, but we still have to check the remaining constraints. + EitherOrBoth::Left(_) => continue, + + // Both clauses contain a constraint with this typevar; verify that the constraint + // in `self` is smaller. + EitherOrBoth::Both(self_constraint, other_constraint) => { + if !self_constraint.subsumes(db, other_constraint) { + return false; + } + } + } + } + true + } + + /// If the union of two clauses is simpler than either of the individual clauses, returns the + /// union. This happens when (a) they mention the same set of typevars, (b) the union of the + /// constraints for exactly one typevar simplifies to a single constraint, and (c) the + /// constraints for all other typevars are identical. Otherwise returns `None`. + fn simplifies_via_distribution(&self, db: &'db dyn Db, other: &Self) -> Option { + // See the notes in `simplify_clauses` for more details on distribution, including Python + // examples that cause it to fire. + + if self.constraints.len() != other.constraints.len() { + return None; + } + + // Verify that the constraints for precisely one typevar simplify, and the constraints for + // all other typevars are identical. Remember the index of the typevar whose constraints + // simplify. + let mut simplified_index = None; + let pairwise = (self.constraints.iter()) + .merge_join_by(&other.constraints, |a, b| a.typevar.cmp(&b.typevar)); + for (index, pair) in pairwise.enumerate() { + match pair { + // If either clause contains a constraint whose typevar doesn't appear in the + // other, the clauses don't simplify. + EitherOrBoth::Left(_) | EitherOrBoth::Right(_) => return None, + + EitherOrBoth::Both(self_constraint, other_constraint) => { + if self_constraint == other_constraint { + continue; + } + let union_constraint = match self_constraint.union(db, other_constraint) { + Simplifiable::NotSimplified(_, _) => { + // The constraints for this typevar are not identical, nor do they + // simplify. + return None; + } + Simplifiable::Simplified(union_constraint) => Some(union_constraint), + Simplifiable::AlwaysSatisfiable => None, + Simplifiable::NeverSatisfiable => { + panic!("unioning two non-never constraints should not be never") + } + }; + if simplified_index + .replace((index, union_constraint)) + .is_some() + { + // More than one constraint simplify, which doesn't allow the clause as a + // whole to simplify. + return None; + } + } + } + } + + let Some((index, union_constraint)) = simplified_index else { + // We never found a typevar whose constraints simplify. + return None; + }; + let mut constraints = self.constraints.clone(); + if let Some(union_constraint) = union_constraint { + constraints[index] = union_constraint; + } else { + // If the simplified union of constraints is Always, then we can remove this typevar + // from the constraint completely. + constraints.remove(index); + } + Some(Self { constraints }) + } + + /// Returns the negation of this clause. The result is a set since negating an intersection + /// produces a union. + fn negate(&self, db: &'db dyn Db) -> ConstraintSet<'db> { + let mut result = ConstraintSet::never(); + for constraint in &self.constraints { + result.union_set(db, constraint.negate(db)); + } + result + } + + fn display(&self, db: &'db dyn Db) -> impl Display { + struct DisplayConstraintClause<'a, 'db> { + clause: &'a ConstraintClause<'db>, + db: &'db dyn Db, + } + + impl Display for DisplayConstraintClause<'_, '_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if self.clause.constraints.is_empty() { + return f.write_str("1"); + } + + if self.clause.constraints.len() > 1 { + f.write_str("(")?; + } + for (i, constraint) in self.clause.constraints.iter().enumerate() { + if i > 0 { + f.write_str(" ∧ ")?; + } + constraint.display(self.db).fmt(f)?; + } + if self.clause.constraints.len() > 1 { + f.write_str(")")?; + } + Ok(()) + } + } + + DisplayConstraintClause { clause: self, db } + } +} + +/// A constraint on a single typevar, providing lower and upper bounds for the types that it can +/// specialize to. The lower and upper bounds can each be either _closed_ (the bound itself is +/// included) or _open_ (the bound itself is not included). +/// +/// In our documentation, we will show constraints using a few different notations: +/// +/// - "Interval" notation: `[s, t]`, `(s, t)`, `[s, t)`, or `(s, t]`, where a square bracket +/// indicates that bound is closed, and a parenthesis indicates that it is open. +/// +/// - ASCII art: `s┠──┨t`, `s╟──╢t`, `s┠──╢t`, or `s╟──┨t`, where a solid bar indicates a closed +/// bound, and a double bar indicates an open bound. +/// +/// - "Comparison" notation: `s ≤ T ≤ t`, `s < T < t`, `s ≤ T < t`, or `s < T ≤ T`, where `≤` +/// indicates a closed bound, and `<` indicates an open bound. Note that this is the only +/// notation that includes the typevar being constrained. +/// +/// ### Invariants +/// +/// - The bounds must be fully static. +/// - The bounds must actually constrain the typevar. If the typevar can be specialized to any +/// type, or if there is no valid type that it can be specialized to, then we don't create an +/// `AtomicConstraint` for the typevar. +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) struct AtomicConstraint<'db> { + typevar: BoundTypeVarInstance<'db>, + lower: ConstraintBound<'db>, + upper: ConstraintBound<'db>, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum ConstraintBound<'db> { + Open(Type<'db>), + Closed(Type<'db>), +} + +impl<'db> ConstraintBound<'db> { + const fn flip(self) -> Self { + match self { + ConstraintBound::Open(bound) => ConstraintBound::Closed(bound), + ConstraintBound::Closed(bound) => ConstraintBound::Open(bound), + } + } + + const fn bound_type(self) -> Type<'db> { + match self { + ConstraintBound::Open(bound) => bound, + ConstraintBound::Closed(bound) => bound, + } + } + + const fn is_open(self) -> bool { + matches!(self, ConstraintBound::Open(_)) + } + + /// Returns the minimum of two upper bounds. (This produces the upper bound of the + /// [intersection][AtomicConstraint::intersect] of two constraints.) + /// + /// We use intersection to combine the types of the bounds (mnemonic: minimum and intersection + /// both make the result smaller). + /// + /// If either of the input upper bounds is open — `[s, t)` or `(s, t)` — then `t` must not be + /// included in the result. If the intersection is equivalent to `t`, then the result must + /// therefore be an open bound. If the intersection is not equivalent to `t`, then it must be + /// smaller than `t`, since intersection cannot make the result larger; therefore `t` is + /// already not included in the result, and the bound will be closed. + fn min_upper(self, db: &'db dyn Db, other: Self) -> Self { + let result_bound = + IntersectionType::from_elements(db, [self.bound_type(), other.bound_type()]); + match (self, other) { + (ConstraintBound::Open(bound), _) | (_, ConstraintBound::Open(bound)) + if bound.is_equivalent_to(db, result_bound) => + { + ConstraintBound::Open(result_bound) + } + _ => ConstraintBound::Closed(result_bound), + } + } + + /// Returns the maximum of two upper bounds. (This produces the upper bound of the + /// [union][AtomicConstraint::union] of two constraints.) + /// + /// We use union to combine the types of the bounds (mnemonic: maximum and union both make the + /// result larger). + /// + /// For the result to be open, the union must be equivalent to one of the input bounds. (Union + /// can only make types "bigger", so if the union is not equivalent to either input, it is + /// strictly larger than both, and the result bound should therefore be closed.) + /// + /// There are only three situations where the result is open: + /// + /// ```text + /// ────╢t₁ ────╢t₁ ────╢t₁ ────┨t₁ ────┨t₁ ────┨t₁ ────╢t₁ + /// ──╢t₂ ──┨t₂ ────╢t₂ ──╢t₂ ──┨t₂ ────┨t₂ ────┨t₂ + /// ⇓ ⇓ ⇓ ⇓ ⇓ ⇓ ⇓ + /// ────╢t₁ ────╢t₁ ────╢t₁ ────┨t₁ ────┨t₁ ────┨t₁ ────┨t₁ + /// ``` + /// + /// In all of these cases, the union is equivalent to `t₁`. (There are symmetric cases + /// where the intersection is equivalent to `t₂`, but we'll handle them by deciding to call the + /// "smaller" input `t₁`.) Note that the result is open if `t₂ < t₁` (_strictly_ less than), or + /// if _both_ inputs are open and `t₁ = t₂`. In all other cases, the result is closed. + fn max_upper(self, db: &'db dyn Db, other: Self) -> Self { + let result_bound = UnionType::from_elements(db, [self.bound_type(), other.bound_type()]); + match (self, other) { + (ConstraintBound::Open(self_bound), ConstraintBound::Open(other_bound)) + if self_bound.is_equivalent_to(db, result_bound) + || other_bound.is_equivalent_to(db, result_bound) => + { + ConstraintBound::Open(result_bound) + } + + (ConstraintBound::Closed(other_bound), ConstraintBound::Open(open_bound)) + | (ConstraintBound::Open(open_bound), ConstraintBound::Closed(other_bound)) + if open_bound.is_equivalent_to(db, result_bound) + && other_bound.is_subtype_of(db, open_bound) + && !other_bound.is_equivalent_to(db, open_bound) => + { + ConstraintBound::Open(result_bound) + } + + _ => ConstraintBound::Closed(result_bound), + } + } + + /// Returns the minimum of two lower bounds. (This produces the lower bound of the + /// [union][AtomicConstraint::union] of two constraints.) + /// + /// We use intersection to combine the types of the bounds (mnemonic: minimum and intersection + /// both make the result smaller). + /// + /// For the result to be open, the intersection must be equivalent to one of the input bounds. + /// (Intersection can only make types "smaller", so if the intersection is not equivalent to + /// either input, it is strictly smaller than both, and the result bound should therefore be + /// closed.) + /// + /// There are only three situations where the result is open: + /// + /// ```text + /// s₁╟──── s₁╟──── s₁╟──── s₁┠──── s₁┠──── s₁┠──── s₁╟──── + /// s₂╟── s₂┠── s₂╟──── s₂╟── s₂┠── s₂┠──── s₂┠──── + /// ⇓ ⇓ ⇓ ⇓ ⇓ ⇓ ⇓ + /// s₁╟──── s₁╟──── s₁╟──── s₁┠──── s₁┠──── s₁┠──── s₁┠──── + /// ``` + /// + /// In all of these cases, the intersection is equivalent to `s₁`. (There are symmetric cases + /// where the intersection is equivalent to `s₂`, but we'll handle them by deciding to call the + /// "smaller" input `s₁`.) Note that the result is open if `s₁ < s₂` (_strictly_ less than), or + /// if _both_ inputs are open and `s₁ = s₂`. In all other cases, the result is closed. + fn min_lower(self, db: &'db dyn Db, other: Self) -> Self { + let result_bound = + IntersectionType::from_elements(db, [self.bound_type(), other.bound_type()]); + match (self, other) { + (ConstraintBound::Open(self_bound), ConstraintBound::Open(other_bound)) + if self_bound.is_equivalent_to(db, result_bound) + || other_bound.is_equivalent_to(db, result_bound) => + { + ConstraintBound::Open(result_bound) + } + + (ConstraintBound::Closed(other_bound), ConstraintBound::Open(open_bound)) + | (ConstraintBound::Open(open_bound), ConstraintBound::Closed(other_bound)) + if open_bound.is_equivalent_to(db, result_bound) + && open_bound.is_subtype_of(db, other_bound) + && !open_bound.is_equivalent_to(db, other_bound) => + { + ConstraintBound::Open(result_bound) + } + + _ => ConstraintBound::Closed(result_bound), + } + } + + /// Returns the maximum of two lower bounds. (This produces the lower bound of the + /// [intersection][AtomicConstraint::intersect] of two constraints.) + /// + /// We use union to combine the types of the bounds (mnemonic: maximum and union both make the + /// result larger). + /// + /// If either of the input lower bounds is open — `(s, t]` or `(s, t)` — then `s` must not be + /// included in the result. If the union is equivalent to `s`, then the result must therefore + /// be an open bound. If the union is not equivalent to `s`, then it must be larger than `s`, + /// since union cannot make the result smaller; therefore `s` is already not included in the + /// result, and the bound will be closed. + fn max_lower(self, db: &'db dyn Db, other: Self) -> Self { + let result_bound = UnionType::from_elements(db, [self.bound_type(), other.bound_type()]); + match (self, other) { + (ConstraintBound::Open(bound), _) | (_, ConstraintBound::Open(bound)) + if bound.is_equivalent_to(db, result_bound) => + { + ConstraintBound::Open(result_bound) + } + _ => ConstraintBound::Closed(result_bound), + } + } +} + +impl<'db> AtomicConstraint<'db> { + /// Returns a new atomic constraint. + /// + /// Panics if `lower` and `upper` are not both fully static. + fn new( + db: &'db dyn Db, + typevar: BoundTypeVarInstance<'db>, + lower: ConstraintBound<'db>, + upper: ConstraintBound<'db>, + ) -> Satisfiable { + let lower_type = lower.bound_type(); + let upper_type = upper.bound_type(); + debug_assert_eq!(lower_type, lower_type.bottom_materialization(db)); + debug_assert_eq!(upper_type, upper_type.top_materialization(db)); + + // If `lower ≰ upper`, then the constraint cannot be satisfied, since there is no type that + // is both greater than `lower`, and less than `upper`. (This is true regardless of whether + // the upper and lower bounds are open are closed.) + if !lower_type.is_subtype_of(db, upper_type) { + return Satisfiable::Never; + } + + // If both bounds are open, then `lower` must be _strictly_ less than `upper`. (If they + // are equivalent, then there is no type that is both strictly greater than that type, and + // strictly less than it.) + if (lower.is_open() || upper.is_open()) && lower_type.is_equivalent_to(db, upper_type) { + return Satisfiable::Never; + } + + // If the requested constraint is `Never ≤ T ≤ object`, then the typevar can be specialized + // to _any_ type, and the constraint does nothing. (Note that both bounds have to be closed + // for this to hold.) + if let (ConstraintBound::Closed(lower), ConstraintBound::Closed(upper)) = (lower, upper) { + if lower.is_never() && upper.is_object(db) { + return Satisfiable::Always; + } + } + + Satisfiable::Constrained(Self { + typevar, + lower, + upper, + }) + } + + /// Returns the negation of this atomic constraint. + /// + /// Because a constraint has both a lower bound and an upper bound, it is technically the + /// intersection of two subtyping checks; the result is therefore a union: + /// + /// ```text + /// ¬(s ≤ T ≤ t) ⇒ ¬(s ≤ T ∧ T ≤ t) ⇒ (s > T) ∨ (T > t) + /// ``` + fn negate(&self, db: &'db dyn Db) -> ConstraintSet<'db> { + let mut result = ConstraintSet::never(); + result.union_constraint( + db, + Self::new( + db, + self.typevar, + ConstraintBound::Closed(Type::Never), + self.lower.flip(), + ), + ); + result.union_constraint( + db, + Self::new( + db, + self.typevar, + self.upper.flip(), + ConstraintBound::Closed(Type::object(db)), + ), + ); + result + } + + /// Returns whether `self` has tighter bounds than `other` — that is, if the intersection of + /// `self` and `other` is `self`. + fn subsumes(&self, db: &'db dyn Db, other: &Self) -> bool { + debug_assert_eq!(self.typevar, other.typevar); + match self.intersect(db, other) { + Satisfiable::Constrained(intersection) => intersection == *self, + _ => false, + } + } + + /// Returns the intersection of this atomic constraint and another. + /// + /// Panics if the two constraints have different typevars. + fn intersect(&self, db: &'db dyn Db, other: &Self) -> Satisfiable { + debug_assert_eq!(self.typevar, other.typevar); + + // The result is always `max_lower(s₁,s₂) : min_upper(t₁,t₂)`. (See the documentation of + // `max_lower` and `min_upper` for details on how we determine whether the corresponding + // bound is open or closed.) + Self::new( + db, + self.typevar, + self.lower.max_lower(db, other.lower), + self.upper.min_upper(db, other.upper), + ) + } + + /// Returns the union of this atomic constraint and another. + /// + /// Panics if the two constraints have different typevars. + fn union(&self, db: &'db dyn Db, other: &Self) -> Simplifiable { + debug_assert_eq!(self.typevar, other.typevar); + + // When the two constraints are disjoint, then they cannot be simplified. + // + // self: s₁┠─┨t₁ + // other: s₂┠─┨t₂ + // + // result: s₁┠─┨t₁ s₂┠─┨t₂ + let is_subtype_of = |left: ConstraintBound<'db>, right: ConstraintBound<'db>| { + let left_type = left.bound_type(); + let right_type = right.bound_type(); + if !left_type.is_subtype_of(db, right_type) { + return false; + } + if left.is_open() && right.is_open() { + return !left_type.is_equivalent_to(db, right_type); + } + true + }; + if !is_subtype_of(self.lower, other.upper) || !is_subtype_of(other.lower, self.upper) { + return Simplifiable::NotSimplified(self.clone(), other.clone()); + } + + // Otherwise the result is `min_lower(s₁,s₂) : max_upper(t₁,t₂)`. (See the documentation of + // `min_lower` and `max_upper` for details on how we determine whether the corresponding + // bound is open or closed.) + Simplifiable::from_one(Self::new( + db, + self.typevar, + self.lower.min_lower(db, other.lower), + self.upper.max_upper(db, other.upper), + )) + } + + fn display(&self, db: &'db dyn Db) -> impl Display { + struct DisplayAtomicConstraint<'a, 'db> { + constraint: &'a AtomicConstraint<'db>, + db: &'db dyn Db, + } + + impl Display for DisplayAtomicConstraint<'_, '_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("(")?; + match self.constraint.lower { + ConstraintBound::Closed(bound) if bound.is_never() => {} + ConstraintBound::Closed(bound) => write!(f, "{} ≤ ", bound.display(self.db))?, + ConstraintBound::Open(bound) => write!(f, "{} < ", bound.display(self.db))?, + } + self.constraint.typevar.display(self.db).fmt(f)?; + match self.constraint.upper { + ConstraintBound::Closed(bound) if bound.is_object(self.db) => {} + ConstraintBound::Closed(bound) => write!(f, " ≤ {}", bound.display(self.db))?, + ConstraintBound::Open(bound) => write!(f, " < {}", bound.display(self.db))?, + } + f.write_str(")") + } + } + + DisplayAtomicConstraint { + constraint: self, + db, + } + } +} + +/// Wraps a constraint (or clause, or set), while using distinct variants to represent when the +/// constraint is never satisfiable or always satisfiable. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum Satisfiable { + Never, + Always, + Constrained(T), +} + +/// The result of trying to simplify two constraints (or clauses, or sets). Like [`Satisfiable`], +/// we use distinct variants to represent when the simplification is never satisfiable or always +/// satisfiable. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum Simplifiable { + NeverSatisfiable, + AlwaysSatisfiable, + Simplified(T), + NotSimplified(T, T), +} + +impl Simplifiable { + fn from_one(constraint: Satisfiable) -> Self { + match constraint { + Satisfiable::Never => Simplifiable::NeverSatisfiable, + Satisfiable::Always => Simplifiable::AlwaysSatisfiable, + Satisfiable::Constrained(constraint) => Simplifiable::Simplified(constraint), + } + } +} diff --git a/crates/ty_python_semantic/src/types/display.rs b/crates/ty_python_semantic/src/types/display.rs index 63c3447889..2c28957510 100644 --- a/crates/ty_python_semantic/src/types/display.rs +++ b/crates/ty_python_semantic/src/types/display.rs @@ -16,8 +16,9 @@ use crate::types::generics::{GenericContext, Specialization}; use crate::types::signatures::{CallableSignature, Parameter, Parameters, Signature}; use crate::types::tuple::TupleSpec; use crate::types::{ - CallableType, IntersectionType, KnownClass, MaterializationKind, MethodWrapperKind, Protocol, - StringLiteralType, SubclassOfInner, Type, UnionType, WrapperDescriptorKind, + BoundTypeVarInstance, CallableType, IntersectionType, KnownClass, MaterializationKind, + MethodWrapperKind, Protocol, StringLiteralType, SubclassOfInner, Type, UnionType, + WrapperDescriptorKind, }; use ruff_db::parsed::parsed_module; @@ -360,12 +361,7 @@ impl Display for DisplayRepresentation<'_> { f.write_str(enum_literal.name(self.db)) } Type::NonInferableTypeVar(bound_typevar) | Type::TypeVar(bound_typevar) => { - f.write_str(bound_typevar.typevar(self.db).name(self.db))?; - if let Some(binding_context) = bound_typevar.binding_context(self.db).name(self.db) - { - write!(f, "@{binding_context}")?; - } - Ok(()) + bound_typevar.display(self.db).fmt(f) } Type::AlwaysTruthy => f.write_str("AlwaysTruthy"), Type::AlwaysFalsy => f.write_str("AlwaysFalsy"), @@ -402,6 +398,30 @@ impl Display for DisplayRepresentation<'_> { } } +impl<'db> BoundTypeVarInstance<'db> { + pub(crate) fn display(self, db: &'db dyn Db) -> impl Display { + DisplayBoundTypeVarInstance { + bound_typevar: self, + db, + } + } +} + +struct DisplayBoundTypeVarInstance<'db> { + bound_typevar: BoundTypeVarInstance<'db>, + db: &'db dyn Db, +} + +impl Display for DisplayBoundTypeVarInstance<'_> { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + f.write_str(self.bound_typevar.typevar(self.db).name(self.db))?; + if let Some(binding_context) = self.bound_typevar.binding_context(self.db).name(self.db) { + write!(f, "@{binding_context}")?; + } + Ok(()) + } +} + impl<'db> TupleSpec<'db> { pub(crate) fn display_with( &'db self, diff --git a/crates/ty_python_semantic/src/types/instance.rs b/crates/ty_python_semantic/src/types/instance.rs index f4e68dc45a..51c406ccc2 100644 --- a/crates/ty_python_semantic/src/types/instance.rs +++ b/crates/ty_python_semantic/src/types/instance.rs @@ -7,7 +7,7 @@ use super::protocol_class::ProtocolInterface; use super::{BoundTypeVarInstance, ClassType, KnownClass, SubclassOfType, Type, TypeVarVariance}; use crate::place::PlaceAndQualifiers; use crate::semantic_index::definition::Definition; -use crate::types::constraints::{Constraints, IteratorConstraintsExtension}; +use crate::types::constraints::{ConstraintSet, Constraints, IteratorConstraintsExtension}; use crate::types::enums::is_single_member_enum; use crate::types::protocol_class::walk_protocol_interface; use crate::types::tuple::{TupleSpec, TupleType}; @@ -513,12 +513,15 @@ impl<'db> ProtocolInstanceType<'db> { visitor: &NormalizedVisitor<'db>, ) -> Type<'db> { let object = Type::object(db); - if object.satisfies_protocol( - db, - self, - TypeRelation::Subtyping, - &HasRelationToVisitor::new(true), - ) { + if object + .satisfies_protocol( + db, + self, + TypeRelation::Subtyping, + &HasRelationToVisitor::new(ConstraintSet::always_satisfiable(db)), + ) + .is_always_satisfied(db) + { return object; } match self.inner { diff --git a/crates/ty_python_semantic/src/types/signatures.rs b/crates/ty_python_semantic/src/types/signatures.rs index 89abbd4199..509c1b4ffb 100644 --- a/crates/ty_python_semantic/src/types/signatures.rs +++ b/crates/ty_python_semantic/src/types/signatures.rs @@ -17,7 +17,7 @@ use smallvec::{SmallVec, smallvec_inline}; use super::{DynamicType, Type, TypeVarVariance, definition_expression_type}; use crate::semantic_index::definition::Definition; -use crate::types::constraints::{Constraints, IteratorConstraintsExtension}; +use crate::types::constraints::{ConstraintSet, Constraints, IteratorConstraintsExtension}; use crate::types::generics::{GenericContext, walk_generic_context}; use crate::types::{ BindingContext, BoundTypeVarInstance, FindLegacyTypeVarsVisitor, HasRelationToVisitor, @@ -123,7 +123,8 @@ impl<'db> CallableSignature<'db> { /// /// See [`Type::is_subtype_of`] for more details. pub(crate) fn is_subtype_of(&self, db: &'db dyn Db, other: &Self) -> bool { - self.is_subtype_of_impl(db, other) + self.is_subtype_of_impl::(db, other) + .is_always_satisfied(db) } fn is_subtype_of_impl>(&self, db: &'db dyn Db, other: &Self) -> C { @@ -143,8 +144,9 @@ impl<'db> CallableSignature<'db> { db, other, TypeRelation::Assignability, - &HasRelationToVisitor::new(true), + &HasRelationToVisitor::new(ConstraintSet::always_satisfiable(db)), ) + .is_always_satisfied(db) } pub(crate) fn has_relation_to_impl>( From 4ca38b2974dd305125c7ed64edbbb2cec008bb4d Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Fri, 29 Aug 2025 09:57:28 +0530 Subject: [PATCH 05/10] [ty] Unpack variadic argument type in specialization (#20130) ## Summary This PR fixes various TODOs around overload call when a variadic argument is used. The reason this bug existed is because the specialization wouldn't account for unpacking the type of the variadic argument. This is fixed by expanding `MatchedArgument` to contain the type of that argument _only_ when it is a variadic argument. The reason is that there's a split for when the argument type is inferred -- the non-variadic arguments are inferred using `infer_argument_types` _after_ parameter matching while the variadic argument type is inferred _during_ the parameter matching. And, the `MatchedArgument` is populated _during_ parameter matching which means the unpacking would need to happen during parameter matching. This split seems a bit inconsistent but I don't want to spend a lot of time on trying to merge them such that all argument type inference happens in a single place. I might look into it while adding support for `**kwargs`. ## Test Plan Update existing tests by resolving the todos. The ecosystem changes looks correct to me except for the `slice` call but it seems that it's unrelated to this PR as we infer `slice[Any, Any, Any]` for a `slice(1, 2, 3)` call on `main` as well ([playground](https://play.ty.dev/9eacce00-c7d5-4dd5-a932-4265cb2bb4f6)). --- .../resources/mdtest/call/overloads.md | 33 ++---- crates/ty_python_semantic/src/types.rs | 4 +- .../ty_python_semantic/src/types/call/bind.rs | 100 ++++++++++++++---- .../src/types/ide_support.rs | 6 +- crates/ty_python_semantic/src/types/infer.rs | 4 +- 5 files changed, 94 insertions(+), 53 deletions(-) diff --git a/crates/ty_python_semantic/resources/mdtest/call/overloads.md b/crates/ty_python_semantic/resources/mdtest/call/overloads.md index ca34c94b5e..a59de5d27b 100644 --- a/crates/ty_python_semantic/resources/mdtest/call/overloads.md +++ b/crates/ty_python_semantic/resources/mdtest/call/overloads.md @@ -290,16 +290,10 @@ from overloaded import A, f def _(x: int, y: A | int): reveal_type(f(x)) # revealed: int - # TODO: revealed: int - # TODO: no error - # error: [no-matching-overload] - reveal_type(f(*(x,))) # revealed: Unknown + reveal_type(f(*(x,))) # revealed: int reveal_type(f(y)) # revealed: A | int - # TODO: revealed: A | int - # TODO: no error - # error: [no-matching-overload] - reveal_type(f(*(y,))) # revealed: Unknown + reveal_type(f(*(y,))) # revealed: A | int ``` ### Generics (PEP 695) @@ -328,16 +322,10 @@ from overloaded import B, f def _(x: int, y: B | int): reveal_type(f(x)) # revealed: int - # TODO: revealed: int - # TODO: no error - # error: [no-matching-overload] - reveal_type(f(*(x,))) # revealed: Unknown + reveal_type(f(*(x,))) # revealed: int reveal_type(f(y)) # revealed: B | int - # TODO: revealed: B | int - # TODO: no error - # error: [no-matching-overload] - reveal_type(f(*(y,))) # revealed: Unknown + reveal_type(f(*(y,))) # revealed: B | int ``` ### Expanding `bool` @@ -1236,21 +1224,14 @@ def _(integer: int, string: str, any: Any, list_any: list[Any]): reveal_type(f(*(integer, string))) # revealed: int reveal_type(f(string, integer)) # revealed: int - # TODO: revealed: int - # TODO: no error - # error: [no-matching-overload] - reveal_type(f(*(string, integer))) # revealed: Unknown + reveal_type(f(*(string, integer))) # revealed: int # This matches the second overload and is _not_ the case of ambiguous overload matching. reveal_type(f(string, any)) # revealed: Any - # TODO: Any - reveal_type(f(*(string, any))) # revealed: tuple[str, Any] + reveal_type(f(*(string, any))) # revealed: Any reveal_type(f(string, list_any)) # revealed: list[Any] - # TODO: revealed: list[Any] - # TODO: no error - # error: [no-matching-overload] - reveal_type(f(*(string, list_any))) # revealed: Unknown + reveal_type(f(*(string, list_any))) # revealed: list[Any] ``` ### Generic `self` diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index 5a910fc1c3..0b90b9a31c 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -4843,7 +4843,7 @@ impl<'db> Type<'db> { argument_types: &CallArguments<'_, 'db>, ) -> Result, CallError<'db>> { self.bindings(db) - .match_parameters(argument_types) + .match_parameters(db, argument_types) .check_types(db, argument_types) } @@ -4892,7 +4892,7 @@ impl<'db> Type<'db> { Place::Type(dunder_callable, boundness) => { let bindings = dunder_callable .bindings(db) - .match_parameters(argument_types) + .match_parameters(db, argument_types) .check_types(db, argument_types)?; if boundness == Boundness::PossiblyUnbound { return Err(CallDunderError::PossiblyUnbound(Box::new(bindings))); diff --git a/crates/ty_python_semantic/src/types/call/bind.rs b/crates/ty_python_semantic/src/types/call/bind.rs index 7627602fa6..43c1aa90f2 100644 --- a/crates/ty_python_semantic/src/types/call/bind.rs +++ b/crates/ty_python_semantic/src/types/call/bind.rs @@ -7,7 +7,7 @@ use std::borrow::Cow; use std::collections::HashSet; use std::fmt; -use itertools::Itertools; +use itertools::{Either, Itertools}; use ruff_db::parsed::parsed_module; use smallvec::{SmallVec, smallvec, smallvec_inline}; @@ -101,11 +101,15 @@ impl<'db> Bindings<'db> { /// /// Once you have argument types available, you can call [`check_types`][Self::check_types] to /// verify that each argument type is assignable to the corresponding parameter type. - pub(crate) fn match_parameters(mut self, arguments: &CallArguments<'_, 'db>) -> Self { + pub(crate) fn match_parameters( + mut self, + db: &'db dyn Db, + arguments: &CallArguments<'_, 'db>, + ) -> Self { let mut argument_forms = vec![None; arguments.len()]; let mut conflicting_forms = vec![false; arguments.len()]; for binding in &mut self.elements { - binding.match_parameters(arguments, &mut argument_forms, &mut conflicting_forms); + binding.match_parameters(db, arguments, &mut argument_forms, &mut conflicting_forms); } self.argument_forms = argument_forms.into(); self.conflicting_forms = conflicting_forms.into(); @@ -1243,6 +1247,7 @@ impl<'db> CallableBinding<'db> { fn match_parameters( &mut self, + db: &'db dyn Db, arguments: &CallArguments<'_, 'db>, argument_forms: &mut [Option], conflicting_forms: &mut [bool], @@ -1252,7 +1257,7 @@ impl<'db> CallableBinding<'db> { let arguments = arguments.with_self(self.bound_type); for overload in &mut self.overloads { - overload.match_parameters(arguments.as_ref(), argument_forms, conflicting_forms); + overload.match_parameters(db, arguments.as_ref(), argument_forms, conflicting_forms); } } @@ -1903,7 +1908,7 @@ struct ArgumentMatcher<'a, 'db> { conflicting_forms: &'a mut [bool], errors: &'a mut Vec>, - argument_matches: Vec, + argument_matches: Vec>, parameter_matched: Vec, next_positional: usize, first_excess_positional: Option, @@ -1947,6 +1952,7 @@ impl<'a, 'db> ArgumentMatcher<'a, 'db> { &mut self, argument_index: usize, argument: Argument<'a>, + argument_type: Option>, parameter_index: usize, parameter: &Parameter<'db>, positional: bool, @@ -1970,6 +1976,7 @@ impl<'a, 'db> ArgumentMatcher<'a, 'db> { } let matched_argument = &mut self.argument_matches[argument_index]; matched_argument.parameters.push(parameter_index); + matched_argument.types.push(argument_type); matched_argument.matched = true; self.parameter_matched[parameter_index] = true; } @@ -1978,6 +1985,7 @@ impl<'a, 'db> ArgumentMatcher<'a, 'db> { &mut self, argument_index: usize, argument: Argument<'a>, + argument_type: Option>, ) -> Result<(), ()> { if matches!(argument, Argument::Synthetic) { self.num_synthetic_args += 1; @@ -1996,6 +2004,7 @@ impl<'a, 'db> ArgumentMatcher<'a, 'db> { self.assign_argument( argument_index, argument, + argument_type, parameter_index, parameter, !parameter.is_variadic(), @@ -2020,20 +2029,35 @@ impl<'a, 'db> ArgumentMatcher<'a, 'db> { }); return Err(()); }; - self.assign_argument(argument_index, argument, parameter_index, parameter, false); + self.assign_argument( + argument_index, + argument, + None, + parameter_index, + parameter, + false, + ); Ok(()) } fn match_variadic( &mut self, + db: &'db dyn Db, argument_index: usize, argument: Argument<'a>, + argument_type: Option>, length: TupleLength, ) -> Result<(), ()> { + let tuple = argument_type.map(|ty| ty.iterate(db)); + let mut argument_types = match tuple.as_ref() { + Some(tuple) => Either::Left(tuple.all_elements().copied()), + None => Either::Right(std::iter::empty()), + }; + // We must be able to match up the fixed-length portion of the argument with positional // parameters, so we pass on any errors that occur. for _ in 0..length.minimum() { - self.match_positional(argument_index, argument)?; + self.match_positional(argument_index, argument, argument_types.next())?; } // If the tuple is variable-length, we assume that it will soak up all remaining positional @@ -2044,14 +2068,14 @@ impl<'a, 'db> ArgumentMatcher<'a, 'db> { .get_positional(self.next_positional) .is_some() { - self.match_positional(argument_index, argument)?; + self.match_positional(argument_index, argument, argument_types.next())?; } } Ok(()) } - fn finish(self) -> Box<[MatchedArgument]> { + fn finish(self) -> Box<[MatchedArgument<'db>]> { if let Some(first_excess_argument_index) = self.first_excess_positional { self.errors.push(BindingError::TooManyPositionalArguments { first_excess_argument_index: self.get_argument_index(first_excess_argument_index), @@ -2088,7 +2112,7 @@ struct ArgumentTypeChecker<'a, 'db> { db: &'db dyn Db, signature: &'a Signature<'db>, arguments: &'a CallArguments<'a, 'db>, - argument_matches: &'a [MatchedArgument], + argument_matches: &'a [MatchedArgument<'db>], parameter_tys: &'a mut [Option>], errors: &'a mut Vec>, @@ -2101,7 +2125,7 @@ impl<'a, 'db> ArgumentTypeChecker<'a, 'db> { db: &'db dyn Db, signature: &'a Signature<'db>, arguments: &'a CallArguments<'a, 'db>, - argument_matches: &'a [MatchedArgument], + argument_matches: &'a [MatchedArgument<'db>], parameter_tys: &'a mut [Option>], errors: &'a mut Vec>, ) -> Self { @@ -2156,12 +2180,17 @@ impl<'a, 'db> ArgumentTypeChecker<'a, 'db> { for (argument_index, adjusted_argument_index, _, argument_type) in self.enumerate_argument_types() { - for parameter_index in &self.argument_matches[argument_index].parameters { - let parameter = ¶meters[*parameter_index]; + for (parameter_index, variadic_argument_type) in + self.argument_matches[argument_index].iter() + { + let parameter = ¶meters[parameter_index]; let Some(expected_type) = parameter.annotated_type() else { continue; }; - if let Err(error) = builder.infer(expected_type, argument_type) { + if let Err(error) = builder.infer( + expected_type, + variadic_argument_type.unwrap_or(argument_type), + ) { self.errors.push(BindingError::SpecializationError { error, argument_index: adjusted_argument_index, @@ -2305,7 +2334,7 @@ impl<'a, 'db> ArgumentTypeChecker<'a, 'db> { /// Information about which parameter(s) an argument was matched against. This is tracked /// separately for each overload. #[derive(Clone, Debug, Default)] -pub struct MatchedArgument { +pub struct MatchedArgument<'db> { /// The index of the parameter(s) that an argument was matched against. A splatted argument /// might be matched against multiple parameters. pub parameters: SmallVec<[usize; 1]>, @@ -2314,6 +2343,33 @@ pub struct MatchedArgument { /// elements must have been successfully matched. (That means that this can be `false` while /// the `parameters` field is non-empty.) pub matched: bool, + + /// The types of a variadic argument when it's unpacked. + /// + /// The length of this vector is always the same as the `parameters` vector i.e., these are the + /// types assigned to each matched parameter. This isn't necessarily the same as the number of + /// types in the argument type which might not be a fixed-length iterable. + /// + /// Another thing to note is that the way this is populated means that for any other argument + /// kind (synthetic, positional, keyword, keyword-variadic), this will be a single-element + /// vector containing `None`, since we don't know the type of the argument when this is + /// constructed. So, this field is populated only for variadic arguments. + /// + /// For example, given a `*args` whose type is `tuple[A, B, C]` and the following parameters: + /// - `(x, *args)`: the `types` field will only have two elements (`B`, `C`) since `A` has been + /// matched with `x`. + /// - `(*args)`: the `types` field will have all the three elements (`A`, `B`, `C`) + types: SmallVec<[Option>; 1]>, +} + +impl<'db> MatchedArgument<'db> { + /// Returns an iterator over the parameter indices and the corresponding argument type. + pub fn iter(&self) -> impl Iterator>)> + '_ { + self.parameters + .iter() + .copied() + .zip(self.types.iter().copied()) + } } /// Binding information for one of the overloads of a callable. @@ -2341,7 +2397,7 @@ pub(crate) struct Binding<'db> { /// Information about which parameter(s) each argument was matched with, in argument source /// order. - argument_matches: Box<[MatchedArgument]>, + argument_matches: Box<[MatchedArgument<'db>]>, /// Bound types for parameters, in parameter source order, or `None` if no argument was matched /// to that parameter. @@ -2374,6 +2430,7 @@ impl<'db> Binding<'db> { pub(crate) fn match_parameters( &mut self, + db: &'db dyn Db, arguments: &CallArguments<'_, 'db>, argument_forms: &mut [Option], conflicting_forms: &mut [bool], @@ -2386,16 +2443,17 @@ impl<'db> Binding<'db> { conflicting_forms, &mut self.errors, ); - for (argument_index, (argument, _)) in arguments.iter().enumerate() { + for (argument_index, (argument, argument_type)) in arguments.iter().enumerate() { match argument { Argument::Positional | Argument::Synthetic => { - let _ = matcher.match_positional(argument_index, argument); + let _ = matcher.match_positional(argument_index, argument, None); } Argument::Keyword(name) => { let _ = matcher.match_keyword(argument_index, argument, name); } Argument::Variadic(length) => { - let _ = matcher.match_variadic(argument_index, argument, length); + let _ = + matcher.match_variadic(db, argument_index, argument, argument_type, length); } Argument::Keywords => { // TODO @@ -2532,7 +2590,7 @@ impl<'db> Binding<'db> { /// Returns a vector where each index corresponds to an argument position, /// and the value is the parameter index that argument maps to (if any). - pub(crate) fn argument_matches(&self) -> &[MatchedArgument] { + pub(crate) fn argument_matches(&self) -> &[MatchedArgument<'db>] { &self.argument_matches } } @@ -2542,7 +2600,7 @@ struct BindingSnapshot<'db> { return_ty: Type<'db>, specialization: Option>, inherited_specialization: Option>, - argument_matches: Box<[MatchedArgument]>, + argument_matches: Box<[MatchedArgument<'db>]>, parameter_tys: Box<[Option>]>, errors: Vec>, } diff --git a/crates/ty_python_semantic/src/types/ide_support.rs b/crates/ty_python_semantic/src/types/ide_support.rs index f96401bd2f..328f5d20d3 100644 --- a/crates/ty_python_semantic/src/types/ide_support.rs +++ b/crates/ty_python_semantic/src/types/ide_support.rs @@ -801,7 +801,7 @@ pub struct CallSignatureDetails<'db> { /// Mapping from argument indices to parameter indices. This helps /// determine which parameter corresponds to which argument position. - pub argument_to_parameter_mapping: Vec, + pub argument_to_parameter_mapping: Vec>, } /// Extract signature details from a function call expression. @@ -821,7 +821,9 @@ pub fn call_signature_details<'db>( CallArguments::from_arguments(db, &call_expr.arguments, |_, splatted_value| { splatted_value.inferred_type(model) }); - let bindings = callable_type.bindings(db).match_parameters(&call_arguments); + let bindings = callable_type + .bindings(db) + .match_parameters(db, &call_arguments); // Extract signature details from all callable bindings bindings diff --git a/crates/ty_python_semantic/src/types/infer.rs b/crates/ty_python_semantic/src/types/infer.rs index f190205d6f..2fa57298bd 100644 --- a/crates/ty_python_semantic/src/types/infer.rs +++ b/crates/ty_python_semantic/src/types/infer.rs @@ -6360,7 +6360,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { let bindings = callable_type .bindings(self.db()) - .match_parameters(&call_arguments); + .match_parameters(self.db(), &call_arguments); self.infer_argument_types(arguments, &mut call_arguments, &bindings.argument_forms); // Validate `TypedDict` constructor calls after argument type inference @@ -8812,7 +8812,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { }; let binding = Binding::single(value_ty, generic_context.signature(self.db())); let bindings = match Bindings::from(binding) - .match_parameters(&call_argument_types) + .match_parameters(self.db(), &call_argument_types) .check_types(self.db(), &call_argument_types) { Ok(bindings) => bindings, From 0d7ed32494df58f62ed6b48a3491f258ca7fd086 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 29 Aug 2025 08:31:26 +0100 Subject: [PATCH 06/10] [ty] Enforce that an attribute on a class `X` must be callable in order to satisfy a member on a protocol `P` (#20142) ## Summary Small, incremental progress towards checking the types of method members. ## Test Plan Added an mdtest --- crates/ty_python_semantic/resources/mdtest/protocols.md | 4 ++++ crates/ty_python_semantic/src/types/protocol_class.rs | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/ty_python_semantic/resources/mdtest/protocols.md b/crates/ty_python_semantic/resources/mdtest/protocols.md index 427868e855..b94f2277a9 100644 --- a/crates/ty_python_semantic/resources/mdtest/protocols.md +++ b/crates/ty_python_semantic/resources/mdtest/protocols.md @@ -1704,7 +1704,11 @@ class NotSubtype: def m(self, x: int) -> int: return 42 +class DefinitelyNotSubtype: + m = None + static_assert(is_subtype_of(NominalSubtype, P)) +static_assert(not is_subtype_of(DefinitelyNotSubtype, P)) # TODO: should pass static_assert(not is_subtype_of(NotSubtype, P)) # error: [static-assert-error] diff --git a/crates/ty_python_semantic/src/types/protocol_class.rs b/crates/ty_python_semantic/src/types/protocol_class.rs index 3135c12011..20811ca497 100644 --- a/crates/ty_python_semantic/src/types/protocol_class.rs +++ b/crates/ty_python_semantic/src/types/protocol_class.rs @@ -560,7 +560,8 @@ impl<'a, 'db> ProtocolMember<'a, 'db> { db, matches!( other.to_meta_type(db).member(db, self.name).place, - Place::Type(_, Boundness::Bound) + Place::Type(ty, Boundness::Bound) + if ty.is_assignable_to(db, CallableType::single(db, Signature::dynamic(Type::any()))) ), ), // TODO: consider the types of the attribute on `other` for property members From 04dc223710e9dfeb265603b9dcc04c08fd7a3601 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 29 Aug 2025 09:44:18 +0100 Subject: [PATCH 07/10] [ty] Improve disambiguation of types via fully qualified names (#20141) --- .../resources/mdtest/function/return_type.md | 17 ++ ...nvalid_return_type_(a91e0c67519cd77f).snap | 48 ++++++ .../src/types/diagnostic.rs | 54 +------ .../ty_python_semantic/src/types/display.rs | 152 +++++++++++++----- 4 files changed, 183 insertions(+), 88 deletions(-) diff --git a/crates/ty_python_semantic/resources/mdtest/function/return_type.md b/crates/ty_python_semantic/resources/mdtest/function/return_type.md index fe44fe3b9d..980afbdf46 100644 --- a/crates/ty_python_semantic/resources/mdtest/function/return_type.md +++ b/crates/ty_python_semantic/resources/mdtest/function/return_type.md @@ -260,6 +260,11 @@ def f(cond: bool) -> int: +```toml +[environment] +python-version = "3.12" +``` + ```py # error: [invalid-return-type] def f() -> int: @@ -279,6 +284,18 @@ T = TypeVar("T") # error: [invalid-return-type] def m(x: T) -> T: ... + +class A[T]: ... + +def f() -> A[int]: + class A[T]: ... + return A[int]() # error: [invalid-return-type] + +class B: ... + +def g() -> B: + class B: ... + return B() # error: [invalid-return-type] ``` ## Invalid return type in stub file diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/return_type.md_-_Function_return_type_-_Invalid_return_type_(a91e0c67519cd77f).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/return_type.md_-_Function_return_type_-_Invalid_return_type_(a91e0c67519cd77f).snap index 9c3379586a..e0f0371720 100644 --- a/crates/ty_python_semantic/resources/mdtest/snapshots/return_type.md_-_Function_return_type_-_Invalid_return_type_(a91e0c67519cd77f).snap +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/return_type.md_-_Function_return_type_-_Invalid_return_type_(a91e0c67519cd77f).snap @@ -30,6 +30,18 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/function/return_type.md 16 | 17 | # error: [invalid-return-type] 18 | def m(x: T) -> T: ... +19 | +20 | class A[T]: ... +21 | +22 | def f() -> A[int]: +23 | class A[T]: ... +24 | return A[int]() # error: [invalid-return-type] +25 | +26 | class B: ... +27 | +28 | def g() -> B: +29 | class B: ... +30 | return B() # error: [invalid-return-type] ``` # Diagnostics @@ -91,9 +103,45 @@ error[invalid-return-type]: Function always implicitly returns `None`, which is 17 | # error: [invalid-return-type] 18 | def m(x: T) -> T: ... | ^ +19 | +20 | class A[T]: ... | info: Consider changing the return annotation to `-> None` or adding a `return` statement info: Only functions in stub files, methods on protocol classes, or methods with `@abstractmethod` are permitted to have empty bodies info: rule `invalid-return-type` is enabled by default ``` + +``` +error[invalid-return-type]: Return type does not match returned value + --> src/mdtest_snippet.py:22:12 + | +20 | class A[T]: ... +21 | +22 | def f() -> A[int]: + | ------ Expected `mdtest_snippet.A[int]` because of return type +23 | class A[T]: ... +24 | return A[int]() # error: [invalid-return-type] + | ^^^^^^^^ expected `mdtest_snippet.A[int]`, found `mdtest_snippet..A[int]` +25 | +26 | class B: ... + | +info: rule `invalid-return-type` is enabled by default + +``` + +``` +error[invalid-return-type]: Return type does not match returned value + --> src/mdtest_snippet.py:28:12 + | +26 | class B: ... +27 | +28 | def g() -> B: + | - Expected `mdtest_snippet.B` because of return type +29 | class B: ... +30 | return B() # error: [invalid-return-type] + | ^^^ expected `mdtest_snippet.B`, found `mdtest_snippet..B` + | +info: rule `invalid-return-type` is enabled by default + +``` diff --git a/crates/ty_python_semantic/src/types/diagnostic.rs b/crates/ty_python_semantic/src/types/diagnostic.rs index ca953928ed..a75ce8796f 100644 --- a/crates/ty_python_semantic/src/types/diagnostic.rs +++ b/crates/ty_python_semantic/src/types/diagnostic.rs @@ -10,7 +10,7 @@ use crate::semantic_index::SemanticIndex; use crate::semantic_index::definition::Definition; use crate::semantic_index::place::{PlaceTable, ScopedPlaceId}; use crate::suppression::FileSuppressionId; -use crate::types::class::{ClassType, DisjointBase, DisjointBaseKind, Field}; +use crate::types::class::{DisjointBase, DisjointBaseKind, Field}; use crate::types::function::KnownFunction; use crate::types::string_annotation::{ BYTE_STRING_TYPE_ANNOTATION, ESCAPE_CHARACTER_IN_FORWARD_ANNOTATION, FSTRING_TYPE_ANNOTATION, @@ -1945,18 +1945,8 @@ pub(super) fn report_invalid_assignment( target_ty: Type, source_ty: Type, ) { - let mut settings = DisplaySettings::default(); - // Handles the situation where the report naming is confusing, such as class with the same Name, - // but from different scopes. - if let Some(target_class) = type_to_class_literal(target_ty, context.db()) { - if let Some(source_class) = type_to_class_literal(source_ty, context.db()) { - if target_class != source_class - && target_class.name(context.db()) == source_class.name(context.db()) - { - settings = settings.qualified(); - } - } - } + let settings = + DisplaySettings::from_possibly_ambiguous_type_pair(context.db(), target_ty, source_ty); report_invalid_assignment_with_message( context, @@ -1970,36 +1960,6 @@ pub(super) fn report_invalid_assignment( ); } -// TODO: generalize this to a method that takes any two types, walks them recursively, and returns -// a set of types with ambiguous names whose display should be qualified. Then we can use this in -// any diagnostic that displays two types. -fn type_to_class_literal<'db>(ty: Type<'db>, db: &'db dyn crate::Db) -> Option> { - match ty { - Type::ClassLiteral(class) => Some(class), - Type::NominalInstance(instance) => match instance.class(db) { - crate::types::class::ClassType::NonGeneric(class) => Some(class), - crate::types::class::ClassType::Generic(alias) => Some(alias.origin(db)), - }, - Type::EnumLiteral(enum_literal) => Some(enum_literal.enum_class(db)), - Type::GenericAlias(alias) => Some(alias.origin(db)), - Type::ProtocolInstance(ProtocolInstanceType { - inner: Protocol::FromClass(class), - .. - }) => match class { - ClassType::NonGeneric(class) => Some(class), - ClassType::Generic(alias) => Some(alias.origin(db)), - }, - Type::TypedDict(typed_dict) => match typed_dict.defining_class() { - ClassType::NonGeneric(class) => Some(class), - ClassType::Generic(alias) => Some(alias.origin(db)), - }, - Type::SubclassOf(subclass_of) => { - type_to_class_literal(Type::from(subclass_of.subclass_of().into_class()?), db) - } - _ => None, - } -} - pub(super) fn report_invalid_attribute_assignment( context: &InferContext, node: AnyNodeRef, @@ -2030,18 +1990,20 @@ pub(super) fn report_invalid_return_type( return; }; + let settings = + DisplaySettings::from_possibly_ambiguous_type_pair(context.db(), expected_ty, actual_ty); let return_type_span = context.span(return_type_range); let mut diag = builder.into_diagnostic("Return type does not match returned value"); diag.set_primary_message(format_args!( "expected `{expected_ty}`, found `{actual_ty}`", - expected_ty = expected_ty.display(context.db()), - actual_ty = actual_ty.display(context.db()), + expected_ty = expected_ty.display_with(context.db(), settings), + actual_ty = actual_ty.display_with(context.db(), settings), )); diag.annotate( Annotation::secondary(return_type_span).message(format_args!( "Expected `{expected_ty}` because of return type", - expected_ty = expected_ty.display(context.db()), + expected_ty = expected_ty.display_with(context.db(), settings), )), ); } diff --git a/crates/ty_python_semantic/src/types/display.rs b/crates/ty_python_semantic/src/types/display.rs index 2c28957510..a54d0e92f7 100644 --- a/crates/ty_python_semantic/src/types/display.rs +++ b/crates/ty_python_semantic/src/types/display.rs @@ -17,8 +17,8 @@ use crate::types::signatures::{CallableSignature, Parameter, Parameters, Signatu use crate::types::tuple::TupleSpec; use crate::types::{ BoundTypeVarInstance, CallableType, IntersectionType, KnownClass, MaterializationKind, - MethodWrapperKind, Protocol, StringLiteralType, SubclassOfInner, Type, UnionType, - WrapperDescriptorKind, + MethodWrapperKind, Protocol, ProtocolInstanceType, StringLiteralType, SubclassOfInner, Type, + UnionType, WrapperDescriptorKind, }; use ruff_db::parsed::parsed_module; @@ -55,6 +55,58 @@ impl DisplaySettings { ..self } } + + #[must_use] + pub fn from_possibly_ambiguous_type_pair<'db>( + db: &'db dyn Db, + type_1: Type<'db>, + type_2: Type<'db>, + ) -> Self { + let result = Self::default(); + + let Some(class_1) = type_to_class_literal(db, type_1) else { + return result; + }; + + let Some(class_2) = type_to_class_literal(db, type_2) else { + return result; + }; + + if class_1 == class_2 { + return result; + } + + if class_1.name(db) == class_2.name(db) { + result.qualified() + } else { + result + } + } +} + +// TODO: generalize this to a method that takes any two types, walks them recursively, and returns +// a set of types with ambiguous names whose display should be qualified. Then we can use this in +// any diagnostic that displays two types. +fn type_to_class_literal<'db>(db: &'db dyn Db, ty: Type<'db>) -> Option> { + match ty { + Type::ClassLiteral(class) => Some(class), + Type::NominalInstance(instance) => { + type_to_class_literal(db, Type::from(instance.class(db))) + } + Type::EnumLiteral(enum_literal) => Some(enum_literal.enum_class(db)), + Type::GenericAlias(alias) => Some(alias.origin(db)), + Type::ProtocolInstance(ProtocolInstanceType { + inner: Protocol::FromClass(class), + .. + }) => type_to_class_literal(db, Type::from(class)), + Type::TypedDict(typed_dict) => { + type_to_class_literal(db, Type::from(typed_dict.defining_class())) + } + Type::SubclassOf(subclass_of) => { + type_to_class_literal(db, Type::from(subclass_of.subclass_of().into_class()?)) + } + _ => None, + } } impl<'db> Type<'db> { @@ -114,18 +166,25 @@ impl fmt::Debug for DisplayType<'_> { } } -/// Writes the string representation of a type, which is the value displayed either as -/// `Literal[]` or `Literal[, ]` for literal types or as `` for -/// non literals -struct DisplayRepresentation<'db> { - ty: Type<'db>, +impl<'db> ClassLiteral<'db> { + fn display_with(self, db: &'db dyn Db, settings: DisplaySettings) -> ClassDisplay<'db> { + ClassDisplay { + db, + class: self, + settings, + } + } +} + +struct ClassDisplay<'db> { db: &'db dyn Db, + class: ClassLiteral<'db>, settings: DisplaySettings, } -impl DisplayRepresentation<'_> { - fn class_parents(&self, class: ClassLiteral) -> Vec { - let body_scope = class.body_scope(self.db); +impl ClassDisplay<'_> { + fn class_parents(&self) -> Vec { + let body_scope = self.class.body_scope(self.db); let file = body_scope.file(self.db); let module_ast = parsed_module(self.db, file).load(self.db); let index = semantic_index(self.db, file); @@ -165,23 +224,29 @@ impl DisplayRepresentation<'_> { name_parts.reverse(); name_parts } +} - fn write_maybe_qualified_class( - &self, - f: &mut Formatter<'_>, - class: ClassLiteral, - ) -> fmt::Result { +impl Display for ClassDisplay<'_> { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { if self.settings.qualified { - let parents = self.class_parents(class); - if !parents.is_empty() { - f.write_str(&parents.join("."))?; + for parent in self.class_parents() { + f.write_str(&parent)?; f.write_char('.')?; } } - f.write_str(class.name(self.db)) + f.write_str(self.class.name(self.db)) } } +/// Writes the string representation of a type, which is the value displayed either as +/// `Literal[]` or `Literal[, ]` for literal types or as `` for +/// non literals +struct DisplayRepresentation<'db> { + ty: Type<'db>, + db: &'db dyn Db, + settings: DisplaySettings, +} + impl Display for DisplayRepresentation<'_> { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { match self.ty { @@ -200,14 +265,14 @@ impl Display for DisplayRepresentation<'_> { .display_with(self.db, self.settings) .fmt(f), (ClassType::NonGeneric(class), _) => { - self.write_maybe_qualified_class(f, class) + class.display_with(self.db, self.settings).fmt(f) }, (ClassType::Generic(alias), _) => alias.display_with(self.db, self.settings).fmt(f), } } Type::ProtocolInstance(protocol) => match protocol.inner { Protocol::FromClass(ClassType::NonGeneric(class)) => { - self.write_maybe_qualified_class(f, class) + class.display_with(self.db, self.settings).fmt(f) } Protocol::FromClass(ClassType::Generic(alias)) => { alias.display_with(self.db, self.settings).fmt(f) @@ -231,11 +296,11 @@ impl Display for DisplayRepresentation<'_> { Type::ModuleLiteral(module) => { write!(f, "", module.module(self.db).name(self.db)) } - Type::ClassLiteral(class) => { - write!(f, "") - } + Type::ClassLiteral(class) => write!( + f, + "", + class.display_with(self.db, self.settings) + ), Type::GenericAlias(generic) => write!( f, "", @@ -243,9 +308,7 @@ impl Display for DisplayRepresentation<'_> { ), Type::SubclassOf(subclass_of_ty) => match subclass_of_ty.subclass_of() { SubclassOfInner::Class(ClassType::NonGeneric(class)) => { - write!(f, "type[")?; - self.write_maybe_qualified_class(f, class)?; - write!(f, "]") + write!(f, "type[{}]", class.display_with(self.db, self.settings)) } SubclassOfInner::Class(ClassType::Generic(alias)) => { write!( @@ -320,13 +383,13 @@ impl Display for DisplayRepresentation<'_> { ) } Type::MethodWrapper(MethodWrapperKind::PropertyDunderGet(_)) => { - write!(f, "",) + f.write_str("") } Type::MethodWrapper(MethodWrapperKind::PropertyDunderSet(_)) => { - write!(f, "",) + f.write_str("") } Type::MethodWrapper(MethodWrapperKind::StrStartswith(_)) => { - write!(f, "",) + f.write_str("") } Type::WrapperDescriptor(kind) => { let (method, object) = match kind { @@ -355,11 +418,14 @@ impl Display for DisplayRepresentation<'_> { escape.bytes_repr(TripleQuotes::No).write(f) } - Type::EnumLiteral(enum_literal) => { - self.write_maybe_qualified_class(f, enum_literal.enum_class(self.db))?; - f.write_char('.')?; - f.write_str(enum_literal.name(self.db)) - } + Type::EnumLiteral(enum_literal) => write!( + f, + "{enum_class}.{literal_name}", + enum_class = enum_literal + .enum_class(self.db) + .display_with(self.db, self.settings), + literal_name = enum_literal.name(self.db) + ), Type::NonInferableTypeVar(bound_typevar) | Type::TypeVar(bound_typevar) => { bound_typevar.display(self.db).fmt(f) } @@ -389,10 +455,12 @@ impl Display for DisplayRepresentation<'_> { } f.write_str("]") } - Type::TypedDict(typed_dict) => self.write_maybe_qualified_class( - f, - typed_dict.defining_class().class_literal(self.db).0, - ), + Type::TypedDict(typed_dict) => typed_dict + .defining_class() + .class_literal(self.db) + .0 + .display_with(self.db, self.settings) + .fmt(f), Type::TypeAlias(alias) => f.write_str(alias.name(self.db)), } } @@ -647,7 +715,7 @@ impl Display for DisplayGenericAlias<'_> { f, "{prefix}{origin}{specialization}{suffix}", prefix = prefix, - origin = self.origin.name(self.db), + origin = self.origin.display_with(self.db, self.settings), specialization = self.specialization.display_short( self.db, TupleSpecialization::from_class(self.db, self.origin) From f77315776cc2067f123f80402a1e6758e77bde24 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 29 Aug 2025 14:22:23 +0100 Subject: [PATCH 08/10] [ty] Better error message for attempting to assign to a read-only property (#20150) --- crates/ty/docs/rules.md | 128 +++++++++--------- .../resources/mdtest/descriptor_protocol.md | 16 +++ .../resources/mdtest/named_tuple.md | 10 +- ...roperties_with_no_s…_(176795bc1727dda7).snap | 40 ++++++ crates/ty_python_semantic/src/types/call.rs | 22 +++ .../ty_python_semantic/src/types/call/bind.rs | 38 ++++-- .../src/types/diagnostic.rs | 42 +++++- crates/ty_python_semantic/src/types/infer.rs | 73 +++++----- 8 files changed, 247 insertions(+), 122 deletions(-) create mode 100644 crates/ty_python_semantic/resources/mdtest/snapshots/descriptor_protocol.…_-_Descriptor_protocol_-_Special_descriptors_-_Properties_with_no_s…_(176795bc1727dda7).snap diff --git a/crates/ty/docs/rules.md b/crates/ty/docs/rules.md index d1c4e2e9a4..7904c1cb7f 100644 --- a/crates/ty/docs/rules.md +++ b/crates/ty/docs/rules.md @@ -36,7 +36,7 @@ def test(): -> "int": Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20call-non-callable) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L112) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L113) **What it does** @@ -58,7 +58,7 @@ Calling a non-callable object will raise a `TypeError` at runtime. Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20conflicting-argument-forms) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L156) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L157) **What it does** @@ -88,7 +88,7 @@ f(int) # error Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20conflicting-declarations) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L182) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L183) **What it does** @@ -117,7 +117,7 @@ a = 1 Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20conflicting-metaclass) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L207) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L208) **What it does** @@ -147,7 +147,7 @@ class C(A, B): ... Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20cyclic-class-definition) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L233) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L234) **What it does** @@ -177,7 +177,7 @@ class B(A): ... Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20duplicate-base) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L298) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L299) **What it does** @@ -202,7 +202,7 @@ class B(A, A): ... Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20duplicate-kw-only) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L319) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L320) **What it does** @@ -306,7 +306,7 @@ def test(): -> "Literal[5]": Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20inconsistent-mro) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L522) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L523) **What it does** @@ -334,7 +334,7 @@ class C(A, B): ... Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20index-out-of-bounds) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L546) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L547) **What it does** @@ -358,7 +358,7 @@ t[3] # IndexError: tuple index out of range Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20instance-layout-conflict) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L351) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L352) **What it does** @@ -445,7 +445,7 @@ an atypical memory layout. Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-argument-type) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L591) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L592) **What it does** @@ -470,7 +470,7 @@ func("foo") # error: [invalid-argument-type] Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-assignment) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L631) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L632) **What it does** @@ -496,7 +496,7 @@ a: int = '' Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-attribute-access) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1665) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1666) **What it does** @@ -528,7 +528,7 @@ C.instance_var = 3 # error: Cannot assign to instance variable Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-await) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L653) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L654) **What it does** @@ -562,7 +562,7 @@ asyncio.run(main()) Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-base) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L683) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L684) **What it does** @@ -584,7 +584,7 @@ class A(42): ... # error: [invalid-base] Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-context-manager) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L734) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L735) **What it does** @@ -609,7 +609,7 @@ with 1: Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-declaration) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L755) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L756) **What it does** @@ -636,7 +636,7 @@ a: str Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-exception-caught) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L778) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L779) **What it does** @@ -678,7 +678,7 @@ except ZeroDivisionError: Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-generic-class) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L814) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L815) **What it does** @@ -709,7 +709,7 @@ class C[U](Generic[T]): ... Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-key) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L566) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L567) **What it does** @@ -738,7 +738,7 @@ alice["height"] # KeyError: 'height' Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-legacy-type-variable) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L840) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L841) **What it does** @@ -771,7 +771,7 @@ def f(t: TypeVar("U")): ... Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-metaclass) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L889) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L890) **What it does** @@ -803,7 +803,7 @@ class B(metaclass=f): ... Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-named-tuple) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L496) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L497) **What it does** @@ -833,7 +833,7 @@ TypeError: can only inherit from a NamedTuple type and Generic Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-overload) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L916) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L917) **What it does** @@ -881,7 +881,7 @@ def foo(x: int) -> int: ... Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-parameter-default) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L959) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L960) **What it does** @@ -905,7 +905,7 @@ def f(a: int = ''): ... Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-protocol) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L433) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L434) **What it does** @@ -937,7 +937,7 @@ TypeError: Protocols can only inherit from other protocols, got Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-raise) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L979) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L980) Checks for `raise` statements that raise non-exceptions or use invalid @@ -984,7 +984,7 @@ def g(): Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-return-type) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L612) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L613) **What it does** @@ -1007,7 +1007,7 @@ def func() -> int: Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-super-argument) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1022) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1023) **What it does** @@ -1061,7 +1061,7 @@ TODO #14889 Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-type-alias-type) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L868) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L869) **What it does** @@ -1086,7 +1086,7 @@ NewAlias = TypeAliasType(get_name(), int) # error: TypeAliasType name mus Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-type-checking-constant) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1061) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1062) **What it does** @@ -1114,7 +1114,7 @@ TYPE_CHECKING = '' Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-type-form) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1085) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1086) **What it does** @@ -1142,7 +1142,7 @@ b: Annotated[int] # `Annotated` expects at least two arguments Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-type-guard-call) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1137) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1138) **What it does** @@ -1174,7 +1174,7 @@ f(10) # Error Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-type-guard-definition) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1109) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1110) **What it does** @@ -1206,7 +1206,7 @@ class C: Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20invalid-type-variable-constraints) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1165) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1166) **What it does** @@ -1239,7 +1239,7 @@ T = TypeVar('T', bound=str) # valid bound TypeVar Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20missing-argument) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1194) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1195) **What it does** @@ -1262,7 +1262,7 @@ func() # TypeError: func() missing 1 required positional argument: 'x' Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20missing-typed-dict-key) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1764) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1765) **What it does** @@ -1293,7 +1293,7 @@ alice["age"] # KeyError Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20no-matching-overload) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1213) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1214) **What it does** @@ -1320,7 +1320,7 @@ func("string") # error: [no-matching-overload] Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20non-subscriptable) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1236) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1237) **What it does** @@ -1342,7 +1342,7 @@ Subscripting an object that does not support it will raise a `TypeError` at runt Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20not-iterable) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1254) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1255) **What it does** @@ -1366,7 +1366,7 @@ for i in 34: # TypeError: 'int' object is not iterable Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20parameter-already-assigned) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1305) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1306) **What it does** @@ -1420,7 +1420,7 @@ def test(): -> "int": Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20static-assert-error) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1641) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1642) **What it does** @@ -1448,7 +1448,7 @@ static_assert(int(2.0 * 3.0) == 6) # error: does not have a statically known tr Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20subclass-of-final-class) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1396) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1397) **What it does** @@ -1475,7 +1475,7 @@ class B(A): ... # Error raised here Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20too-many-positional-arguments) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1441) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1442) **What it does** @@ -1500,7 +1500,7 @@ f("foo") # Error raised here Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20type-assertion-failure) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1419) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1420) **What it does** @@ -1526,7 +1526,7 @@ def _(x: int): Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20unavailable-implicit-super-arguments) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1462) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1463) **What it does** @@ -1570,7 +1570,7 @@ class A: Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20unknown-argument) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1519) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1520) **What it does** @@ -1595,7 +1595,7 @@ f(x=1, y=2) # Error raised here Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20unresolved-attribute) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1540) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1541) **What it does** @@ -1621,7 +1621,7 @@ A().foo # AttributeError: 'A' object has no attribute 'foo' Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20unresolved-import) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1562) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1563) **What it does** @@ -1644,7 +1644,7 @@ import foo # ModuleNotFoundError: No module named 'foo' Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20unresolved-reference) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1581) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1582) **What it does** @@ -1667,7 +1667,7 @@ print(x) # NameError: name 'x' is not defined Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20unsupported-bool-conversion) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1274) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1275) **What it does** @@ -1702,7 +1702,7 @@ b1 < b2 < b1 # exception raised here Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20unsupported-operator) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1600) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1601) **What it does** @@ -1728,7 +1728,7 @@ A() + A() # TypeError: unsupported operand type(s) for +: 'A' and 'A' Default level: [`error`](../rules.md#rule-levels "This lint has a default level of 'error'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20zero-stepsize-in-slice) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1622) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1623) **What it does** @@ -1751,7 +1751,7 @@ l[1:10:0] # ValueError: slice step cannot be zero Default level: [`warn`](../rules.md#rule-levels "This lint has a default level of 'warn'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20ambiguous-protocol-member) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L461) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L462) **What it does** @@ -1790,7 +1790,7 @@ class SubProto(BaseProto, Protocol): Default level: [`warn`](../rules.md#rule-levels "This lint has a default level of 'warn'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20deprecated) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L277) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L278) **What it does** @@ -1843,7 +1843,7 @@ a = 20 / 0 # type: ignore Default level: [`warn`](../rules.md#rule-levels "This lint has a default level of 'warn'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20possibly-unbound-attribute) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1326) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1327) **What it does** @@ -1869,7 +1869,7 @@ A.c # AttributeError: type object 'A' has no attribute 'c' Default level: [`warn`](../rules.md#rule-levels "This lint has a default level of 'warn'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20possibly-unbound-implicit-call) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L130) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L131) **What it does** @@ -1899,7 +1899,7 @@ A()[0] # TypeError: 'A' object is not subscriptable Default level: [`warn`](../rules.md#rule-levels "This lint has a default level of 'warn'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20possibly-unbound-import) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1348) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1349) **What it does** @@ -1929,7 +1929,7 @@ from module import a # ImportError: cannot import name 'a' from 'module' Default level: [`warn`](../rules.md#rule-levels "This lint has a default level of 'warn'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20redundant-cast) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1693) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1694) **What it does** @@ -1954,7 +1954,7 @@ cast(int, f()) # Redundant Default level: [`warn`](../rules.md#rule-levels "This lint has a default level of 'warn'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20undefined-reveal) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1501) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1502) **What it does** @@ -2005,7 +2005,7 @@ a = 20 / 0 # ty: ignore[division-by-zero] Default level: [`warn`](../rules.md#rule-levels "This lint has a default level of 'warn'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20unresolved-global) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1714) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1715) **What it does** @@ -2059,7 +2059,7 @@ def g(): Default level: [`warn`](../rules.md#rule-levels "This lint has a default level of 'warn'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20unsupported-base) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L701) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L702) **What it does** @@ -2096,7 +2096,7 @@ class D(C): ... # error: [unsupported-base] Default level: [`ignore`](../rules.md#rule-levels "This lint has a default level of 'ignore'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20division-by-zero) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L259) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L260) **What it does** @@ -2118,7 +2118,7 @@ Dividing by zero raises a `ZeroDivisionError` at runtime. Default level: [`ignore`](../rules.md#rule-levels "This lint has a default level of 'ignore'.") · [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20possibly-unresolved-reference) · -[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1374) +[View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L1375) **What it does** diff --git a/crates/ty_python_semantic/resources/mdtest/descriptor_protocol.md b/crates/ty_python_semantic/resources/mdtest/descriptor_protocol.md index 6d413f7100..8c7363909b 100644 --- a/crates/ty_python_semantic/resources/mdtest/descriptor_protocol.md +++ b/crates/ty_python_semantic/resources/mdtest/descriptor_protocol.md @@ -522,6 +522,22 @@ c.name = None c.name = 42 ``` +### Properties with no setters + + + +If a property has no setter, we emit a bespoke error message when a user attempts to set that +attribute, since this is a common error. + +```py +class DontAssignToMe: + @property + def immutable(self): ... + +# error: [invalid-assignment] +DontAssignToMe().immutable = "the properties, they are a-changing" +``` + ### Built-in `classmethod` descriptor Similarly to `property`, `classmethod` decorator creates an implicit descriptor that binds the first diff --git a/crates/ty_python_semantic/resources/mdtest/named_tuple.md b/crates/ty_python_semantic/resources/mdtest/named_tuple.md index 29b17542a8..2703f8554a 100644 --- a/crates/ty_python_semantic/resources/mdtest/named_tuple.md +++ b/crates/ty_python_semantic/resources/mdtest/named_tuple.md @@ -78,10 +78,7 @@ reveal_type(Person.id) # revealed: property reveal_type(Person.name) # revealed: property reveal_type(Person.age) # revealed: property -# TODO... the error is correct, but this is not the friendliest error message -# for assigning to a read-only property :-) -# -# error: [invalid-assignment] "Invalid assignment to data descriptor attribute `id` on type `Person` with custom `__set__` method" +# error: [invalid-assignment] "Cannot assign to read-only property `id` on object of type `Person`" alice.id = 42 # error: [invalid-assignment] bob.age = None @@ -221,10 +218,7 @@ james = SuperUser(0, "James", 42, "Jimmy") # on the subclass james.name = "Robert" -# TODO: the error is correct (can't assign to the read-only property inherited from the superclass) -# but the error message could be friendlier :-) -# -# error: [invalid-assignment] "Invalid assignment to data descriptor attribute `nickname` on type `SuperUser` with custom `__set__` method" +# error: [invalid-assignment] "Cannot assign to read-only property `nickname` on object of type `SuperUser`" james.nickname = "Bob" ``` diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/descriptor_protocol.…_-_Descriptor_protocol_-_Special_descriptors_-_Properties_with_no_s…_(176795bc1727dda7).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/descriptor_protocol.…_-_Descriptor_protocol_-_Special_descriptors_-_Properties_with_no_s…_(176795bc1727dda7).snap new file mode 100644 index 0000000000..3590d7e0ec --- /dev/null +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/descriptor_protocol.…_-_Descriptor_protocol_-_Special_descriptors_-_Properties_with_no_s…_(176795bc1727dda7).snap @@ -0,0 +1,40 @@ +--- +source: crates/ty_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: descriptor_protocol.md - Descriptor protocol - Special descriptors - Properties with no setters +mdtest path: crates/ty_python_semantic/resources/mdtest/descriptor_protocol.md +--- + +# Python source files + +## mdtest_snippet.py + +``` +1 | class DontAssignToMe: +2 | @property +3 | def immutable(self): ... +4 | +5 | # error: [invalid-assignment] +6 | DontAssignToMe().immutable = "the properties, they are a-changing" +``` + +# Diagnostics + +``` +error[invalid-assignment]: Cannot assign to read-only property `immutable` on object of type `DontAssignToMe` + --> src/mdtest_snippet.py:3:9 + | +1 | class DontAssignToMe: +2 | @property +3 | def immutable(self): ... + | --------- Property `DontAssignToMe.immutable` defined here with no setter +4 | +5 | # error: [invalid-assignment] +6 | DontAssignToMe().immutable = "the properties, they are a-changing" + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ Attempted assignment to `DontAssignToMe.immutable` here + | +info: rule `invalid-assignment` is enabled by default + +``` diff --git a/crates/ty_python_semantic/src/types/call.rs b/crates/ty_python_semantic/src/types/call.rs index be85b3922a..8c00ab3479 100644 --- a/crates/ty_python_semantic/src/types/call.rs +++ b/crates/ty_python_semantic/src/types/call.rs @@ -1,6 +1,8 @@ use super::context::InferContext; use super::{Signature, Type}; use crate::Db; +use crate::types::PropertyInstanceType; +use crate::types::call::bind::BindingError; mod arguments; pub(crate) mod bind; @@ -14,6 +16,26 @@ pub(super) use bind::{Binding, Bindings, CallableBinding, MatchedArgument}; #[derive(Debug)] pub(crate) struct CallError<'db>(pub(crate) CallErrorKind, pub(crate) Box>); +impl<'db> CallError<'db> { + /// Returns `Some(property)` if the call error was caused by an attempt to set a property + /// that has no setter, and `None` otherwise. + pub(crate) fn as_attempt_to_set_property_with_no_setter( + &self, + ) -> Option> { + if self.0 != CallErrorKind::BindingError { + return None; + } + self.1 + .into_iter() + .flatten() + .flat_map(bind::Binding::errors) + .find_map(|error| match error { + BindingError::PropertyHasNoSetter(property) => Some(*property), + _ => None, + }) + } +} + /// The reason why calling a type failed. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum CallErrorKind { diff --git a/crates/ty_python_semantic/src/types/call/bind.rs b/crates/ty_python_semantic/src/types/call/bind.rs index 43c1aa90f2..ce218ab90e 100644 --- a/crates/ty_python_semantic/src/types/call/bind.rs +++ b/crates/ty_python_semantic/src/types/call/bind.rs @@ -426,9 +426,9 @@ impl<'db> Bindings<'db> { overload.set_return_type(Type::unknown()); } } else { - overload.errors.push(BindingError::InternalCallError( - "property has no getter", - )); + overload + .errors + .push(BindingError::PropertyHasNoSetter(*property)); overload.set_return_type(Type::Never); } } @@ -482,9 +482,9 @@ impl<'db> Bindings<'db> { )); } } else { - overload.errors.push(BindingError::InternalCallError( - "property has no setter", - )); + overload + .errors + .push(BindingError::PropertyHasNoSetter(*property)); } } } @@ -500,9 +500,9 @@ impl<'db> Bindings<'db> { )); } } else { - overload.errors.push(BindingError::InternalCallError( - "property has no setter", - )); + overload + .errors + .push(BindingError::PropertyHasNoSetter(property)); } } } @@ -2593,6 +2593,10 @@ impl<'db> Binding<'db> { pub(crate) fn argument_matches(&self) -> &[MatchedArgument<'db>] { &self.argument_matches } + + pub(crate) fn errors(&self) -> &[BindingError<'db>] { + &self.errors + } } #[derive(Clone, Debug)] @@ -2811,7 +2815,9 @@ pub(crate) enum BindingError<'db> { provided_ty: Type<'db>, }, /// One or more required parameters (that is, with no default) is not supplied by any argument. - MissingArguments { parameters: ParameterContexts }, + MissingArguments { + parameters: ParameterContexts, + }, /// A call argument can't be matched to any parameter. UnknownArgument { argument_name: ast::name::Name, @@ -2833,6 +2839,7 @@ pub(crate) enum BindingError<'db> { error: SpecializationError<'db>, argument_index: Option, }, + PropertyHasNoSetter(PropertyInstanceType<'db>), /// The call itself might be well constructed, but an error occurred while evaluating the call. /// We use this variant to report errors in `property.__get__` and `property.__set__`, which /// can occur when the call to the underlying getter/setter fails. @@ -3099,6 +3106,17 @@ impl<'db> BindingError<'db> { } } + Self::PropertyHasNoSetter(_) => { + BindingError::InternalCallError("property has no setter").report_diagnostic( + context, + node, + callable_ty, + callable_description, + union_diag, + matching_overload, + ); + } + Self::InternalCallError(reason) => { let node = Self::get_node(node, None); if let Some(builder) = context.report_lint(&CALL_NON_CALLABLE, node) { diff --git a/crates/ty_python_semantic/src/types/diagnostic.rs b/crates/ty_python_semantic/src/types/diagnostic.rs index a75ce8796f..7449c73828 100644 --- a/crates/ty_python_semantic/src/types/diagnostic.rs +++ b/crates/ty_python_semantic/src/types/diagnostic.rs @@ -10,6 +10,7 @@ use crate::semantic_index::SemanticIndex; use crate::semantic_index::definition::Definition; use crate::semantic_index::place::{PlaceTable, ScopedPlaceId}; use crate::suppression::FileSuppressionId; +use crate::types::call::CallError; use crate::types::class::{DisjointBase, DisjointBaseKind, Field}; use crate::types::function::KnownFunction; use crate::types::string_annotation::{ @@ -26,7 +27,7 @@ use crate::{ Db, DisplaySettings, FxIndexMap, FxOrderMap, Module, ModuleName, Program, declare_lint, }; use itertools::Itertools; -use ruff_db::diagnostic::{Annotation, Diagnostic, SubDiagnostic, SubDiagnosticSeverity}; +use ruff_db::diagnostic::{Annotation, Diagnostic, Span, SubDiagnostic, SubDiagnosticSeverity}; use ruff_python_ast::name::Name; use ruff_python_ast::{self as ast, AnyNodeRef}; use ruff_text_size::{Ranged, TextRange}; @@ -1979,6 +1980,45 @@ pub(super) fn report_invalid_attribute_assignment( ); } +pub(super) fn report_bad_dunder_set_call<'db>( + context: &InferContext<'db, '_>, + dunder_set_failure: &CallError<'db>, + attribute: &str, + object_type: Type<'db>, + target: &ast::ExprAttribute, +) { + let Some(builder) = context.report_lint(&INVALID_ASSIGNMENT, target) else { + return; + }; + let db = context.db(); + if let Some(property) = dunder_set_failure.as_attempt_to_set_property_with_no_setter() { + let object_type = object_type.display(db); + let mut diagnostic = builder.into_diagnostic(format_args!( + "Cannot assign to read-only property `{attribute}` on object of type `{object_type}`", + )); + if let Some(file_range) = property + .getter(db) + .and_then(|getter| getter.definition(db)) + .and_then(|definition| definition.focus_range(db)) + { + diagnostic.annotate(Annotation::secondary(Span::from(file_range)).message( + format_args!("Property `{object_type}.{attribute}` defined here with no setter"), + )); + diagnostic.set_primary_message(format_args!( + "Attempted assignment to `{object_type}.{attribute}` here" + )); + } + } else { + // TODO: Here, it would be nice to emit an additional diagnostic + // that explains why the call failed + builder.into_diagnostic(format_args!( + "Invalid assignment to data descriptor attribute \ + `{attribute}` on type `{}` with custom `__set__` method", + object_type.display(db) + )); + } +} + pub(super) fn report_invalid_return_type( context: &InferContext, object_range: impl Ranged, diff --git a/crates/ty_python_semantic/src/types/infer.rs b/crates/ty_python_semantic/src/types/infer.rs index 2fa57298bd..a120610785 100644 --- a/crates/ty_python_semantic/src/types/infer.rs +++ b/crates/ty_python_semantic/src/types/infer.rs @@ -102,7 +102,7 @@ use crate::types::diagnostic::{ INVALID_TYPE_VARIABLE_CONSTRAINTS, IncompatibleBases, POSSIBLY_UNBOUND_IMPLICIT_CALL, POSSIBLY_UNBOUND_IMPORT, TypeCheckDiagnostics, UNDEFINED_REVEAL, UNRESOLVED_ATTRIBUTE, UNRESOLVED_GLOBAL, UNRESOLVED_IMPORT, UNRESOLVED_REFERENCE, UNSUPPORTED_OPERATOR, - report_implicit_return_type, report_instance_layout_conflict, + report_bad_dunder_set_call, report_implicit_return_type, report_instance_layout_conflict, report_invalid_argument_number_to_special_form, report_invalid_arguments_to_annotated, report_invalid_arguments_to_callable, report_invalid_assignment, report_invalid_attribute_assignment, report_invalid_generator_function_return_type, @@ -4217,32 +4217,30 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { if let Place::Type(meta_dunder_set, _) = meta_attr_ty.class_member(db, "__set__".into()).place { - let successful_call = meta_dunder_set - .try_call( - db, - &CallArguments::positional([ - meta_attr_ty, - object_ty, - value_ty, - ]), - ) - .is_ok(); + let dunder_set_result = meta_dunder_set.try_call( + db, + &CallArguments::positional([ + meta_attr_ty, + object_ty, + value_ty, + ]), + ); - if !successful_call && emit_diagnostics { - if let Some(builder) = self - .context - .report_lint(&INVALID_ASSIGNMENT, target) + if emit_diagnostics { + if let Err(dunder_set_failure) = + dunder_set_result.as_ref() { - // TODO: Here, it would be nice to emit an additional diagnostic that explains why the call failed - builder.into_diagnostic(format_args!( - "Invalid assignment to data descriptor attribute \ - `{attribute}` on type `{}` with custom `__set__` method", - object_ty.display(db) - )); + report_bad_dunder_set_call( + &self.context, + dunder_set_failure, + attribute, + object_ty, + target, + ); } } - successful_call + dunder_set_result.is_ok() } else { ensure_assignable_to(meta_attr_ty) }; @@ -4343,27 +4341,24 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { let assignable_to_meta_attr = if let Place::Type(meta_dunder_set, _) = meta_attr_ty.class_member(db, "__set__".into()).place { - let successful_call = meta_dunder_set - .try_call( - db, - &CallArguments::positional([meta_attr_ty, object_ty, value_ty]), - ) - .is_ok(); + let dunder_set_result = meta_dunder_set.try_call( + db, + &CallArguments::positional([meta_attr_ty, object_ty, value_ty]), + ); - if !successful_call && emit_diagnostics { - if let Some(builder) = - self.context.report_lint(&INVALID_ASSIGNMENT, target) - { - // TODO: Here, it would be nice to emit an additional diagnostic that explains why the call failed - builder.into_diagnostic(format_args!( - "Invalid assignment to data descriptor attribute \ - `{attribute}` on type `{}` with custom `__set__` method", - object_ty.display(db) - )); + if emit_diagnostics { + if let Err(dunder_set_failure) = dunder_set_result.as_ref() { + report_bad_dunder_set_call( + &self.context, + dunder_set_failure, + attribute, + object_ty, + target, + ); } } - successful_call + dunder_set_result.is_ok() } else { ensure_assignable_to(meta_attr_ty) }; From 0ff0c7030239a2608a1f6314097e2298840b6c7a Mon Sep 17 00:00:00 2001 From: Dan Parizher <105245560+danparizher@users.noreply.github.com> Date: Fri, 29 Aug 2025 09:40:25 -0400 Subject: [PATCH 09/10] [`fastapi`] Fix false positive for paths with spaces around parameters (`FAST003`) (#20077) ## Summary Fixes #20060 --- .../test/fixtures/fastapi/FAST003.py | 14 ++++++++++++++ .../rules/fastapi_unused_path_parameter.rs | 5 ++++- ...-api-unused-path-parameter_FAST003.py.snap | 19 ------------------- 3 files changed, 18 insertions(+), 20 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/fastapi/FAST003.py b/crates/ruff_linter/resources/test/fixtures/fastapi/FAST003.py index cd8653026c..8641334f2e 100644 --- a/crates/ruff_linter/resources/test/fixtures/fastapi/FAST003.py +++ b/crates/ruff_linter/resources/test/fixtures/fastapi/FAST003.py @@ -213,3 +213,17 @@ async def get_id_pydantic_full( async def get_id_pydantic_short(params: Annotated[PydanticParams, Depends()]): ... @app.get("/{my_id}") async def get_id_init_not_annotated(params = Depends(InitParams)): ... + +@app.get("/things/{ thing_id }") +async def read_thing(query: str): + return {"query": query} + + +@app.get("/things/{ thing_id : path }") +async def read_thing(query: str): + return {"query": query} + + +@app.get("/things/{ thing_id : str }") +async def read_thing(query: str): + return {"query": query} diff --git a/crates/ruff_linter/src/rules/fastapi/rules/fastapi_unused_path_parameter.rs b/crates/ruff_linter/src/rules/fastapi/rules/fastapi_unused_path_parameter.rs index e0d215b45c..d4d3674930 100644 --- a/crates/ruff_linter/src/rules/fastapi/rules/fastapi_unused_path_parameter.rs +++ b/crates/ruff_linter/src/rules/fastapi/rules/fastapi_unused_path_parameter.rs @@ -457,6 +457,9 @@ fn parameter_alias<'a>(parameter: &'a Parameter, semantic: &SemanticModel) -> Op /// /// The iterator yields tuples of the parameter name and the range of the parameter in the input, /// inclusive of curly braces. +/// +/// FastAPI only recognizes path parameters when there are no leading or trailing spaces around +/// the parameter name. For example, `/{x}` is a valid parameter, but `/{ x }` is treated literally. #[derive(Debug)] struct PathParamIterator<'a> { input: &'a str, @@ -483,7 +486,7 @@ impl<'a> Iterator for PathParamIterator<'a> { // We ignore text after a colon, since those are path converters // See also: https://fastapi.tiangolo.com/tutorial/path-params/?h=path#path-convertor let param_name_end = param_content.find(':').unwrap_or(param_content.len()); - let param_name = ¶m_content[..param_name_end].trim(); + let param_name = ¶m_content[..param_name_end]; #[expect(clippy::range_plus_one)] return Some((param_name, start..end + 1)); diff --git a/crates/ruff_linter/src/rules/fastapi/snapshots/ruff_linter__rules__fastapi__tests__fast-api-unused-path-parameter_FAST003.py.snap b/crates/ruff_linter/src/rules/fastapi/snapshots/ruff_linter__rules__fastapi__tests__fast-api-unused-path-parameter_FAST003.py.snap index 62ca78ce45..a3ea2bf7ce 100644 --- a/crates/ruff_linter/src/rules/fastapi/snapshots/ruff_linter__rules__fastapi__tests__fast-api-unused-path-parameter_FAST003.py.snap +++ b/crates/ruff_linter/src/rules/fastapi/snapshots/ruff_linter__rules__fastapi__tests__fast-api-unused-path-parameter_FAST003.py.snap @@ -59,25 +59,6 @@ help: Add `thing_id` to function signature 23 | note: This is an unsafe fix and may change runtime behavior -FAST003 [*] Parameter `thing_id` appears in route path, but not in `read_thing` signature - --> FAST003.py:24:19 - | -24 | @app.get("/things/{thing_id : path}") - | ^^^^^^^^^^^^^^^^^ -25 | async def read_thing(query: str): -26 | return {"query": query} - | -help: Add `thing_id` to function signature -22 | -23 | -24 | @app.get("/things/{thing_id : path}") - - async def read_thing(query: str): -25 + async def read_thing(query: str, thing_id): -26 | return {"query": query} -27 | -28 | -note: This is an unsafe fix and may change runtime behavior - FAST003 [*] Parameter `title` appears in route path, but not in `read_thing` signature --> FAST003.py:29:27 | From ffcdd4ea42114f0af4a1cbab4aff931be4eb0b0d Mon Sep 17 00:00:00 2001 From: Hans Date: Fri, 29 Aug 2025 21:41:06 +0800 Subject: [PATCH 10/10] [`refurb`] Add fix safety section (`FURB105`) (#17499) ## Summary This PR add the `fix safety` section for rule `FURB105` in `print_empty_string.rs` for #15584 Before: ``` def get_sep(): print("side effect") return "" print("", sep=get_sep()) ``` After: ``` def get_sep(): print("side effect") return "" print() ``` --------- Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com> --- .../ruff_linter/src/rules/refurb/rules/print_empty_string.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/ruff_linter/src/rules/refurb/rules/print_empty_string.rs b/crates/ruff_linter/src/rules/refurb/rules/print_empty_string.rs index b0b17456ab..29a26f44ca 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/print_empty_string.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/print_empty_string.rs @@ -30,6 +30,11 @@ use crate::{Applicability, Edit, Fix, FixAvailability, Violation}; /// print() /// ``` /// +/// ## Fix safety +/// This fix is marked as unsafe if it removes an unused `sep` keyword argument +/// that may have side effects. Removing such arguments may change the program's +/// behavior by skipping the execution of those side effects. +/// /// ## References /// - [Python documentation: `print`](https://docs.python.org/3/library/functions.html#print) #[derive(ViolationMetadata)]