From e2ec2bc3062ecbc14f3d3c0c01640e4aa40b62ae Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Sat, 13 Dec 2025 09:02:22 +0100 Subject: [PATCH 01/30] Use datatest for formatter tests (#21933) --- Cargo.lock | 56 ++- Cargo.toml | 1 + crates/ruff_python_formatter/Cargo.toml | 7 +- .../ruff_python_formatter/tests/fixtures.rs | 427 ++++++++---------- crates/ruff_python_parser/Cargo.toml | 7 +- crates/ruff_python_parser/tests/fixtures.rs | 114 ++--- 6 files changed, 323 insertions(+), 289 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e75eedb6c2..9c5a084a58 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -254,6 +254,21 @@ dependencies = [ "syn", ] +[[package]] +name = "bit-set" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08807e080ed7f9d5433fa9b275196cfc35414f66a0c79d864dc51a0d825231a3" +dependencies = [ + "bit-vec", +] + +[[package]] +name = "bit-vec" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e764a1d40d510daf35e07be9eb06e75770908c27d411ee6c92109c9840eaaf7" + [[package]] name = "bitflags" version = "1.3.2" @@ -944,6 +959,18 @@ dependencies = [ "parking_lot_core", ] +[[package]] +name = "datatest-stable" +version = "0.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a867d7322eb69cf3a68a5426387a25b45cb3b9c5ee41023ee6cea92e2afadd82" +dependencies = [ + "camino", + "fancy-regex", + "libtest-mimic 0.8.1", + "walkdir", +] + [[package]] name = "derive-where" version = "1.6.0" @@ -1138,6 +1165,17 @@ dependencies = [ "windows-sys 0.61.0", ] +[[package]] +name = "fancy-regex" +version = "0.14.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6e24cb5a94bcae1e5408b0effca5cd7172ea3c5755049c5f3af4cd283a165298" +dependencies = [ + "bit-set", + "regex-automata", + "regex-syntax", +] + [[package]] name = "fastrand" version = "2.3.0" @@ -1625,7 +1663,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "46fdb647ebde000f43b5b53f773c30cf9b0cb4300453208713fa38b2c70935a0" dependencies = [ "console 0.15.11", - "globset", "once_cell", "pest", "pest_derive", @@ -1633,7 +1670,6 @@ dependencies = [ "ron", "serde", "similar", - "walkdir", ] [[package]] @@ -1919,6 +1955,18 @@ dependencies = [ "threadpool", ] +[[package]] +name = "libtest-mimic" +version = "0.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5297962ef19edda4ce33aaa484386e0a5b3d7f2f4e037cbeee00503ef6b29d33" +dependencies = [ + "anstream", + "anstyle", + "clap", + "escape8259", +] + [[package]] name = "linux-raw-sys" version = "0.11.0" @@ -3278,6 +3326,7 @@ dependencies = [ "anyhow", "clap", "countme", + "datatest-stable", "insta", "itertools 0.14.0", "memchr", @@ -3347,6 +3396,7 @@ dependencies = [ "bitflags 2.10.0", "bstr", "compact_str", + "datatest-stable", "get-size2", "insta", "itertools 0.14.0", @@ -4311,7 +4361,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5fe242ee9e646acec9ab73a5c540e8543ed1b107f0ce42be831e0775d423c396" dependencies = [ "ignore", - "libtest-mimic", + "libtest-mimic 0.7.3", "snapbox", ] diff --git a/Cargo.toml b/Cargo.toml index 0badc79e3c..dbd9808fdd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -81,6 +81,7 @@ compact_str = "0.9.0" criterion = { version = "0.7.0", default-features = false } crossbeam = { version = "0.8.4" } dashmap = { version = "6.0.1" } +datatest-stable = { version = "0.3.3" } dir-test = { version = "0.4.0" } dunce = { version = "1.0.5" } drop_bomb = { version = "0.1.5" } diff --git a/crates/ruff_python_formatter/Cargo.toml b/crates/ruff_python_formatter/Cargo.toml index 95ca60ee10..d285f88d22 100644 --- a/crates/ruff_python_formatter/Cargo.toml +++ b/crates/ruff_python_formatter/Cargo.toml @@ -43,7 +43,8 @@ tracing = { workspace = true } [dev-dependencies] ruff_formatter = { workspace = true } -insta = { workspace = true, features = ["glob"] } +datatest-stable = { workspace = true } +insta = { workspace = true } regex = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } @@ -54,8 +55,8 @@ similar = { workspace = true } ignored = ["ruff_cache"] [[test]] -name = "ruff_python_formatter_fixtures" -path = "tests/fixtures.rs" +name = "fixtures" +harness = false test = true required-features = ["serde"] diff --git a/crates/ruff_python_formatter/tests/fixtures.rs b/crates/ruff_python_formatter/tests/fixtures.rs index 33741bd744..d75a42c569 100644 --- a/crates/ruff_python_formatter/tests/fixtures.rs +++ b/crates/ruff_python_formatter/tests/fixtures.rs @@ -1,4 +1,7 @@ use crate::normalizer::Normalizer; +use anyhow::anyhow; +use datatest_stable::Utf8Path; +use insta::assert_snapshot; use ruff_db::diagnostic::{ Annotation, Diagnostic, DiagnosticFormat, DiagnosticId, DisplayDiagnosticConfig, DisplayDiagnostics, DummyFileResolver, Severity, Span, SubDiagnostic, SubDiagnosticSeverity, @@ -24,26 +27,27 @@ use std::{fmt, fs}; mod normalizer; -#[test] -fn black_compatibility() { - let test_file = |input_path: &Path| { - let content = fs::read_to_string(input_path).unwrap(); +#[expect(clippy::needless_pass_by_value)] +fn black_compatibility(input_path: &Utf8Path, content: String) -> datatest_stable::Result<()> { + let test_name = input_path + .strip_prefix("./resources/test/fixtures/black") + .unwrap_or(input_path) + .as_str(); - let options_path = input_path.with_extension("options.json"); + let options_path = input_path.with_extension("options.json"); - let options: PyFormatOptions = if let Ok(options_file) = fs::File::open(&options_path) { - let reader = BufReader::new(options_file); - serde_json::from_reader(reader).unwrap_or_else(|_| { - panic!("Expected option file {options_path:?} to be a valid Json file") - }) - } else { - PyFormatOptions::from_extension(input_path) - }; + let options: PyFormatOptions = if let Ok(options_file) = fs::File::open(&options_path) { + let reader = BufReader::new(options_file); + serde_json::from_reader(reader).map_err(|err| { + anyhow!("Expected option file {options_path:?} to be a valid Json file: {err}") + })? + } else { + PyFormatOptions::from_extension(input_path.as_std_path()) + }; - let first_line = content.lines().next().unwrap_or_default(); - let formatted_code = if first_line.starts_with("# flags:") - && first_line.contains("--line-ranges=") - { + let first_line = content.lines().next().unwrap_or_default(); + let formatted_code = + if first_line.starts_with("# flags:") && first_line.contains("--line-ranges=") { let line_index = LineIndex::from_source_text(&content); let ranges = first_line @@ -69,13 +73,9 @@ fn black_compatibility() { let mut formatted_code = content.clone(); for range in ranges { - let formatted = - format_range(&content, range, options.clone()).unwrap_or_else(|err| { - panic!( - "Range-formatting of {} to succeed but encountered error {err}", - input_path.display() - ) - }); + let formatted = format_range(&content, range, options.clone()).map_err(|err| { + anyhow!("Range-formatting to succeed but encountered error {err}") + })?; let range = formatted.source_range(); @@ -86,12 +86,8 @@ fn black_compatibility() { formatted_code } else { - let printed = format_module_source(&content, options.clone()).unwrap_or_else(|err| { - panic!( - "Formatting of {} to succeed but encountered error {err}", - input_path.display() - ) - }); + let printed = format_module_source(&content, options.clone()) + .map_err(|err| anyhow!("Formatting to succeed but encountered error {err}"))?; let formatted_code = printed.into_code(); @@ -100,191 +96,133 @@ fn black_compatibility() { formatted_code }; - let extension = input_path - .extension() - .expect("Test file to have py or pyi extension") - .to_string_lossy(); - let expected_path = input_path.with_extension(format!("{extension}.expect")); - let expected_output = fs::read_to_string(&expected_path) - .unwrap_or_else(|_| panic!("Expected Black output file '{expected_path:?}' to exist")); + let extension = input_path + .extension() + .expect("Test file to have py or pyi extension"); + let expected_path = input_path.with_extension(format!("{extension}.expect")); + let expected_output = fs::read_to_string(&expected_path) + .unwrap_or_else(|_| panic!("Expected Black output file '{expected_path:?}' to exist")); - let unsupported_syntax_errors = - ensure_unchanged_ast(&content, &formatted_code, &options, input_path); + let unsupported_syntax_errors = + ensure_unchanged_ast(&content, &formatted_code, &options, input_path); - if formatted_code == expected_output { - // Black and Ruff formatting matches. Delete any existing snapshot files because the Black output - // already perfectly captures the expected output. - // The following code mimics insta's logic generating the snapshot name for a test. - let workspace_path = std::env::var("CARGO_MANIFEST_DIR").unwrap(); + // Black and Ruff formatting matches. Delete any existing snapshot files because the Black output + // already perfectly captures the expected output. + // The following code mimics insta's logic generating the snapshot name for a test. + let workspace_path = std::env::var("CARGO_MANIFEST_DIR").unwrap(); - let mut components = input_path.components().rev(); - let file_name = components.next().unwrap(); - let test_suite = components.next().unwrap(); + let full_snapshot_name = format!("black_compatibility@{test_name}.snap",); - let snapshot_name = format!( - "black_compatibility@{}__{}.snap", - test_suite.as_os_str().to_string_lossy(), - file_name.as_os_str().to_string_lossy() - ); + let snapshot_path = Path::new(&workspace_path) + .join("tests/snapshots") + .join(full_snapshot_name); - let snapshot_path = Path::new(&workspace_path) - .join("tests/snapshots") - .join(snapshot_name); - if snapshot_path.exists() && snapshot_path.is_file() { - // SAFETY: This is a convenience feature. That's why we don't want to abort - // when deleting a no longer needed snapshot fails. - fs::remove_file(&snapshot_path).ok(); - } - - let new_snapshot_path = snapshot_path.with_extension("snap.new"); - if new_snapshot_path.exists() && new_snapshot_path.is_file() { - // SAFETY: This is a convenience feature. That's why we don't want to abort - // when deleting a no longer needed snapshot fails. - fs::remove_file(&new_snapshot_path).ok(); - } - } else { - // Black and Ruff have different formatting. Write out a snapshot that covers the differences - // today. - let mut snapshot = String::new(); - write!(snapshot, "{}", Header::new("Input")).unwrap(); - write!(snapshot, "{}", CodeFrame::new("python", &content)).unwrap(); - - write!(snapshot, "{}", Header::new("Black Differences")).unwrap(); - - let diff = TextDiff::from_lines(expected_output.as_str(), &formatted_code) - .unified_diff() - .header("Black", "Ruff") - .to_string(); - - write!(snapshot, "{}", CodeFrame::new("diff", &diff)).unwrap(); - - write!(snapshot, "{}", Header::new("Ruff Output")).unwrap(); - write!(snapshot, "{}", CodeFrame::new("python", &formatted_code)).unwrap(); - - write!(snapshot, "{}", Header::new("Black Output")).unwrap(); - write!(snapshot, "{}", CodeFrame::new("python", &expected_output)).unwrap(); - - if !unsupported_syntax_errors.is_empty() { - write!(snapshot, "{}", Header::new("New Unsupported Syntax Errors")).unwrap(); - writeln!( - snapshot, - "{}", - DisplayDiagnostics::new( - &DummyFileResolver, - &DisplayDiagnosticConfig::default().format(DiagnosticFormat::Full), - &unsupported_syntax_errors - ) - ) - .unwrap(); - } - - insta::with_settings!({ - omit_expression => true, - input_file => input_path, - prepend_module_to_snapshot => false, - }, { - insta::assert_snapshot!(snapshot); - }); + if formatted_code == expected_output { + if snapshot_path.exists() && snapshot_path.is_file() { + // SAFETY: This is a convenience feature. That's why we don't want to abort + // when deleting a no longer needed snapshot fails. + fs::remove_file(&snapshot_path).ok(); } - }; - insta::glob!( - "../resources", - "test/fixtures/black/**/*.{py,pyi}", - test_file - ); + let new_snapshot_path = snapshot_path.with_extension("snap.new"); + if new_snapshot_path.exists() && new_snapshot_path.is_file() { + // SAFETY: This is a convenience feature. That's why we don't want to abort + // when deleting a no longer needed snapshot fails. + fs::remove_file(&new_snapshot_path).ok(); + } + } else { + // Black and Ruff have different formatting. Write out a snapshot that covers the differences + // today. + let mut snapshot = String::new(); + write!(snapshot, "{}", Header::new("Input")).unwrap(); + write!(snapshot, "{}", CodeFrame::new("python", &content)).unwrap(); + + write!(snapshot, "{}", Header::new("Black Differences")).unwrap(); + + let diff = TextDiff::from_lines(expected_output.as_str(), &formatted_code) + .unified_diff() + .header("Black", "Ruff") + .to_string(); + + write!(snapshot, "{}", CodeFrame::new("diff", &diff)).unwrap(); + + write!(snapshot, "{}", Header::new("Ruff Output")).unwrap(); + write!(snapshot, "{}", CodeFrame::new("python", &formatted_code)).unwrap(); + + write!(snapshot, "{}", Header::new("Black Output")).unwrap(); + write!(snapshot, "{}", CodeFrame::new("python", &expected_output)).unwrap(); + + if !unsupported_syntax_errors.is_empty() { + write!(snapshot, "{}", Header::new("New Unsupported Syntax Errors")).unwrap(); + writeln!( + snapshot, + "{}", + DisplayDiagnostics::new( + &DummyFileResolver, + &DisplayDiagnosticConfig::default().format(DiagnosticFormat::Full), + &unsupported_syntax_errors + ) + ) + .unwrap(); + } + + let mut settings = insta::Settings::clone_current(); + settings.set_omit_expression(true); + settings.set_input_file(input_path); + settings.set_prepend_module_to_snapshot(false); + settings.set_snapshot_suffix(test_name); + let _settings = settings.bind_to_scope(); + + assert_snapshot!(snapshot); + } + Ok(()) } -#[test] -fn format() { - let test_file = |input_path: &Path| { - let content = fs::read_to_string(input_path).unwrap(); +#[expect(clippy::needless_pass_by_value)] +fn format(input_path: &Utf8Path, content: String) -> datatest_stable::Result<()> { + let test_name = input_path + .strip_prefix("./resources/test/fixtures/ruff") + .unwrap_or(input_path) + .as_str(); - let mut snapshot = format!("## Input\n{}", CodeFrame::new("python", &content)); - let options_path = input_path.with_extension("options.json"); + let mut snapshot = format!("## Input\n{}", CodeFrame::new("python", &content)); + let options_path = input_path.with_extension("options.json"); - if let Ok(options_file) = fs::File::open(&options_path) { - let reader = BufReader::new(options_file); - let options: Vec = - serde_json::from_reader(reader).unwrap_or_else(|_| { - panic!("Expected option file {options_path:?} to be a valid Json file") - }); + if let Ok(options_file) = fs::File::open(&options_path) { + let reader = BufReader::new(options_file); + let options: Vec = serde_json::from_reader(reader).map_err(|_| { + anyhow!("Expected option file {options_path:?} to be a valid Json file") + })?; - writeln!(snapshot, "## Outputs").unwrap(); + writeln!(snapshot, "## Outputs").unwrap(); - for (i, options) in options.into_iter().enumerate() { - let (formatted_code, unsupported_syntax_errors) = - format_file(&content, &options, input_path); - - writeln!( - snapshot, - "### Output {}\n{}{}", - i + 1, - CodeFrame::new("", &DisplayPyOptions(&options)), - CodeFrame::new("python", &formatted_code) - ) - .unwrap(); - - if options.preview().is_enabled() { - continue; - } - - // We want to capture the differences in the preview style in our fixtures - let options_preview = options.with_preview(PreviewMode::Enabled); - let (formatted_preview, _) = format_file(&content, &options_preview, input_path); - - if formatted_code != formatted_preview { - // Having both snapshots makes it hard to see the difference, so we're keeping only - // diff. - writeln!( - snapshot, - "#### Preview changes\n{}", - CodeFrame::new( - "diff", - TextDiff::from_lines(&formatted_code, &formatted_preview) - .unified_diff() - .header("Stable", "Preview") - ) - ) - .unwrap(); - } - - if !unsupported_syntax_errors.is_empty() { - writeln!( - snapshot, - "### Unsupported Syntax Errors\n{}", - DisplayDiagnostics::new( - &DummyFileResolver, - &DisplayDiagnosticConfig::default().format(DiagnosticFormat::Full), - &unsupported_syntax_errors - ) - ) - .unwrap(); - } - } - } else { - // We want to capture the differences in the preview style in our fixtures - let options = PyFormatOptions::from_extension(input_path); + for (i, options) in options.into_iter().enumerate() { let (formatted_code, unsupported_syntax_errors) = format_file(&content, &options, input_path); + writeln!( + snapshot, + "### Output {}\n{}{}", + i + 1, + CodeFrame::new("", &DisplayPyOptions(&options)), + CodeFrame::new("python", &formatted_code) + ) + .unwrap(); + + if options.preview().is_enabled() { + continue; + } + + // We want to capture the differences in the preview style in our fixtures let options_preview = options.with_preview(PreviewMode::Enabled); let (formatted_preview, _) = format_file(&content, &options_preview, input_path); - if formatted_code == formatted_preview { - writeln!( - snapshot, - "## Output\n{}", - CodeFrame::new("python", &formatted_code) - ) - .unwrap(); - } else { + if formatted_code != formatted_preview { // Having both snapshots makes it hard to see the difference, so we're keeping only // diff. writeln!( snapshot, - "## Output\n{}\n## Preview changes\n{}", - CodeFrame::new("python", &formatted_code), + "#### Preview changes\n{}", CodeFrame::new( "diff", TextDiff::from_lines(&formatted_code, &formatted_preview) @@ -298,7 +236,7 @@ fn format() { if !unsupported_syntax_errors.is_empty() { writeln!( snapshot, - "## Unsupported Syntax Errors\n{}", + "### Unsupported Syntax Errors\n{}", DisplayDiagnostics::new( &DummyFileResolver, &DisplayDiagnosticConfig::default().format(DiagnosticFormat::Full), @@ -308,27 +246,74 @@ fn format() { .unwrap(); } } + } else { + // We want to capture the differences in the preview style in our fixtures + let options = PyFormatOptions::from_extension(input_path.as_std_path()); + let (formatted_code, unsupported_syntax_errors) = + format_file(&content, &options, input_path); - insta::with_settings!({ - omit_expression => true, - input_file => input_path, - prepend_module_to_snapshot => false, - }, { - insta::assert_snapshot!(snapshot); - }); - }; + let options_preview = options.with_preview(PreviewMode::Enabled); + let (formatted_preview, _) = format_file(&content, &options_preview, input_path); - insta::glob!( - "../resources", - "test/fixtures/ruff/**/*.{py,pyi}", - test_file - ); + if formatted_code == formatted_preview { + writeln!( + snapshot, + "## Output\n{}", + CodeFrame::new("python", &formatted_code) + ) + .unwrap(); + } else { + // Having both snapshots makes it hard to see the difference, so we're keeping only + // diff. + writeln!( + snapshot, + "## Output\n{}\n## Preview changes\n{}", + CodeFrame::new("python", &formatted_code), + CodeFrame::new( + "diff", + TextDiff::from_lines(&formatted_code, &formatted_preview) + .unified_diff() + .header("Stable", "Preview") + ) + ) + .unwrap(); + } + + if !unsupported_syntax_errors.is_empty() { + writeln!( + snapshot, + "## Unsupported Syntax Errors\n{}", + DisplayDiagnostics::new( + &DummyFileResolver, + &DisplayDiagnosticConfig::default().format(DiagnosticFormat::Full), + &unsupported_syntax_errors + ) + ) + .unwrap(); + } + } + + let mut settings = insta::Settings::clone_current(); + settings.set_omit_expression(true); + settings.set_input_file(input_path); + settings.set_prepend_module_to_snapshot(false); + settings.set_snapshot_suffix(test_name); + let _settings = settings.bind_to_scope(); + + assert_snapshot!(snapshot); + + Ok(()) +} + +datatest_stable::harness! { + { test = black_compatibility, root = "./resources/test/fixtures/black", pattern = r".+\.pyi?$" }, + { test = format, root="./resources/test/fixtures/ruff", pattern = r".+\.pyi?$" } } fn format_file( source: &str, options: &PyFormatOptions, - input_path: &Path, + input_path: &Utf8Path, ) -> (String, Vec) { let (unformatted, formatted_code) = if source.contains("") { let mut content = source.to_string(); @@ -363,8 +348,7 @@ fn format_file( let formatted = format_range(&format_input, range, options.clone()).unwrap_or_else(|err| { panic!( - "Range-formatting of {} to succeed but encountered error {err}", - input_path.display() + "Range-formatting of {input_path} to succeed but encountered error {err}", ) }); @@ -377,10 +361,7 @@ fn format_file( (Cow::Owned(without_markers), content) } else { let printed = format_module_source(source, options.clone()).unwrap_or_else(|err| { - panic!( - "Formatting `{input_path} was expected to succeed but it failed: {err}", - input_path = input_path.display() - ) + panic!("Formatting `{input_path} was expected to succeed but it failed: {err}",) }); let formatted_code = printed.into_code(); @@ -399,22 +380,20 @@ fn format_file( fn ensure_stability_when_formatting_twice( formatted_code: &str, options: &PyFormatOptions, - input_path: &Path, + input_path: &Utf8Path, ) { let reformatted = match format_module_source(formatted_code, options.clone()) { Ok(reformatted) => reformatted, Err(err) => { let mut diag = Diagnostic::from(&err); if let Some(range) = err.range() { - let file = - SourceFileBuilder::new(input_path.to_string_lossy(), formatted_code).finish(); + let file = SourceFileBuilder::new(input_path.as_str(), formatted_code).finish(); let span = Span::from(file).with_range(range); diag.annotate(Annotation::primary(span)); } panic!( - "Expected formatted code of {} to be valid syntax: {err}:\ + "Expected formatted code of {input_path} to be valid syntax: {err}:\ \n---\n{formatted_code}---\n{}", - input_path.display(), diag.display(&DummyFileResolver, &DisplayDiagnosticConfig::default()), ); } @@ -440,7 +419,6 @@ Formatted once: Formatted twice: --- {reformatted}---"#, - input_path = input_path.display(), options = &DisplayPyOptions(options), reformatted = reformatted.as_code(), ); @@ -467,7 +445,7 @@ fn ensure_unchanged_ast( unformatted_code: &str, formatted_code: &str, options: &PyFormatOptions, - input_path: &Path, + input_path: &Utf8Path, ) -> Vec { let source_type = options.source_type(); @@ -499,11 +477,7 @@ fn ensure_unchanged_ast( formatted_unsupported_syntax_errors .retain(|fingerprint, _| !unformatted_unsupported_syntax_errors.contains_key(fingerprint)); - let file = SourceFileBuilder::new( - input_path.file_name().unwrap().to_string_lossy(), - formatted_code, - ) - .finish(); + let file = SourceFileBuilder::new(input_path.file_name().unwrap(), formatted_code).finish(); let diagnostics = formatted_unsupported_syntax_errors .values() .map(|error| { @@ -533,11 +507,10 @@ fn ensure_unchanged_ast( .header("Unformatted", "Formatted") .to_string(); panic!( - r#"Reformatting the unformatted code of {} resulted in AST changes. + r#"Reformatting the unformatted code of {input_path} resulted in AST changes. --- {diff} "#, - input_path.display(), ); } diff --git a/crates/ruff_python_parser/Cargo.toml b/crates/ruff_python_parser/Cargo.toml index c527f96e11..579882ee5e 100644 --- a/crates/ruff_python_parser/Cargo.toml +++ b/crates/ruff_python_parser/Cargo.toml @@ -12,6 +12,10 @@ license = { workspace = true } [lib] +[[test]] +name = "fixtures" +harness = false + [dependencies] ruff_python_ast = { workspace = true, features = ["get-size"] } ruff_python_trivia = { workspace = true } @@ -34,7 +38,8 @@ ruff_python_ast = { workspace = true, features = ["serde"] } ruff_source_file = { workspace = true } anyhow = { workspace = true } -insta = { workspace = true, features = ["glob"] } +datatest-stable = { workspace = true } +insta = { workspace = true } itertools = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } diff --git a/crates/ruff_python_parser/tests/fixtures.rs b/crates/ruff_python_parser/tests/fixtures.rs index a9378eddfe..0f29c46970 100644 --- a/crates/ruff_python_parser/tests/fixtures.rs +++ b/crates/ruff_python_parser/tests/fixtures.rs @@ -1,9 +1,8 @@ use std::cell::RefCell; use std::cmp::Ordering; use std::fmt::{Formatter, Write}; -use std::fs; -use std::path::Path; +use datatest_stable::Utf8Path; use itertools::Itertools; use ruff_annotate_snippets::{Level, Renderer, Snippet}; use ruff_python_ast::token::{Token, Tokens}; @@ -17,38 +16,49 @@ use ruff_python_parser::{Mode, ParseErrorType, ParseOptions, Parsed, parse_unche use ruff_source_file::{LineIndex, OneIndexed, SourceCode}; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; -#[test] -fn valid_syntax() { - insta::glob!("../resources", "valid/**/*.py", test_valid_syntax); +#[expect(clippy::needless_pass_by_value, clippy::unnecessary_wraps)] +fn valid_syntax(path: &Utf8Path, content: String) -> datatest_stable::Result<()> { + test_valid_syntax(path, &content, "./resources/valid"); + Ok(()) } -#[test] -fn invalid_syntax() { - insta::glob!("../resources", "invalid/**/*.py", test_invalid_syntax); +#[expect(clippy::needless_pass_by_value, clippy::unnecessary_wraps)] +fn invalid_syntax(path: &Utf8Path, content: String) -> datatest_stable::Result<()> { + test_invalid_syntax(path, &content, "./resources/invalid"); + Ok(()) } -#[test] -fn inline_ok() { - insta::glob!("../resources/inline", "ok/**/*.py", test_valid_syntax); +#[expect(clippy::needless_pass_by_value, clippy::unnecessary_wraps)] +fn inline_ok(path: &Utf8Path, content: String) -> datatest_stable::Result<()> { + test_valid_syntax(path, &content, "./resources/inline/ok"); + Ok(()) } -#[test] -fn inline_err() { - insta::glob!("../resources/inline", "err/**/*.py", test_invalid_syntax); +#[expect(clippy::needless_pass_by_value, clippy::unnecessary_wraps)] +fn inline_err(path: &Utf8Path, content: String) -> datatest_stable::Result<()> { + test_invalid_syntax(path, &content, "./resources/inline/err"); + Ok(()) +} + +datatest_stable::harness! { + { test = valid_syntax, root = "./resources/valid", pattern = r"\.pyi?$" }, + { test = inline_ok, root = "./resources/inline/ok", pattern = r"\.pyi?$" }, + { test = invalid_syntax, root = "./resources/invalid", pattern = r"\.pyi?$" }, + { test = inline_err, root="./resources/inline/err", pattern = r"\.pyi?$" } } /// Asserts that the parser generates no syntax errors for a valid program. /// Snapshots the AST. -fn test_valid_syntax(input_path: &Path) { - let source = fs::read_to_string(input_path).expect("Expected test file to exist"); - let options = extract_options(&source).unwrap_or_else(|| { +fn test_valid_syntax(input_path: &Utf8Path, source: &str, root: &str) { + let test_name = input_path.strip_prefix(root).unwrap_or(input_path).as_str(); + let options = extract_options(source).unwrap_or_else(|| { ParseOptions::from(Mode::Module).with_target_version(PythonVersion::latest_preview()) }); - let parsed = parse_unchecked(&source, options.clone()); + let parsed = parse_unchecked(source, options.clone()); if parsed.has_syntax_errors() { - let line_index = LineIndex::from_source_text(&source); - let source_code = SourceCode::new(&source, &line_index); + let line_index = LineIndex::from_source_text(source); + let source_code = SourceCode::new(source, &line_index); let mut message = "Expected no syntax errors for a valid program but the parser generated the following errors:\n".to_string(); @@ -81,8 +91,8 @@ fn test_valid_syntax(input_path: &Path) { panic!("{input_path:?}: {message}"); } - validate_tokens(parsed.tokens(), source.text_len(), input_path); - validate_ast(&parsed, source.text_len(), input_path); + validate_tokens(parsed.tokens(), source.text_len()); + validate_ast(&parsed, source.text_len()); let mut output = String::new(); writeln!(&mut output, "## AST").unwrap(); @@ -91,7 +101,7 @@ fn test_valid_syntax(input_path: &Path) { let parsed = parsed.try_into_module().expect("Parsed with Mode::Module"); let mut visitor = - SemanticSyntaxCheckerVisitor::new(&source).with_python_version(options.target_version()); + SemanticSyntaxCheckerVisitor::new(source).with_python_version(options.target_version()); for stmt in parsed.suite() { visitor.visit_stmt(stmt); @@ -102,8 +112,8 @@ fn test_valid_syntax(input_path: &Path) { if !semantic_syntax_errors.is_empty() { let mut message = "Expected no semantic syntax errors for a valid program:\n".to_string(); - let line_index = LineIndex::from_source_text(&source); - let source_code = SourceCode::new(&source, &line_index); + let line_index = LineIndex::from_source_text(source); + let source_code = SourceCode::new(source, &line_index); for error in semantic_syntax_errors { writeln!( @@ -125,6 +135,7 @@ fn test_valid_syntax(input_path: &Path) { omit_expression => true, input_file => input_path, prepend_module_to_snapshot => false, + snapshot_suffix => test_name }, { insta::assert_snapshot!(output); }); @@ -132,22 +143,23 @@ fn test_valid_syntax(input_path: &Path) { /// Assert that the parser generates at least one syntax error for the given input file. /// Snapshots the AST and the error messages. -fn test_invalid_syntax(input_path: &Path) { - let source = fs::read_to_string(input_path).expect("Expected test file to exist"); - let options = extract_options(&source).unwrap_or_else(|| { +fn test_invalid_syntax(input_path: &Utf8Path, source: &str, root: &str) { + let test_name = input_path.strip_prefix(root).unwrap_or(input_path).as_str(); + + let options = extract_options(source).unwrap_or_else(|| { ParseOptions::from(Mode::Module).with_target_version(PythonVersion::PY314) }); - let parsed = parse_unchecked(&source, options.clone()); + let parsed = parse_unchecked(source, options.clone()); - validate_tokens(parsed.tokens(), source.text_len(), input_path); - validate_ast(&parsed, source.text_len(), input_path); + validate_tokens(parsed.tokens(), source.text_len()); + validate_ast(&parsed, source.text_len()); let mut output = String::new(); writeln!(&mut output, "## AST").unwrap(); writeln!(&mut output, "\n```\n{:#?}\n```", parsed.syntax()).unwrap(); - let line_index = LineIndex::from_source_text(&source); - let source_code = SourceCode::new(&source, &line_index); + let line_index = LineIndex::from_source_text(source); + let source_code = SourceCode::new(source, &line_index); if !parsed.errors().is_empty() { writeln!(&mut output, "## Errors\n").unwrap(); @@ -186,7 +198,7 @@ fn test_invalid_syntax(input_path: &Path) { let parsed = parsed.try_into_module().expect("Parsed with Mode::Module"); let mut visitor = - SemanticSyntaxCheckerVisitor::new(&source).with_python_version(options.target_version()); + SemanticSyntaxCheckerVisitor::new(source).with_python_version(options.target_version()); for stmt in parsed.suite() { visitor.visit_stmt(stmt); @@ -196,7 +208,7 @@ fn test_invalid_syntax(input_path: &Path) { assert!( parsed.has_syntax_errors() || !semantic_syntax_errors.is_empty(), - "{input_path:?}: Expected parser to generate at least one syntax error for a program containing syntax errors." + "Expected parser to generate at least one syntax error for a program containing syntax errors." ); if !semantic_syntax_errors.is_empty() { @@ -220,6 +232,7 @@ fn test_invalid_syntax(input_path: &Path) { omit_expression => true, input_file => input_path, prepend_module_to_snapshot => false, + snapshot_suffix => test_name }, { insta::assert_snapshot!(output); }); @@ -372,26 +385,24 @@ impl std::fmt::Display for CodeFrame<'_> { /// Verifies that: /// * the ranges are strictly increasing when loop the tokens in insertion order /// * all ranges are within the length of the source code -fn validate_tokens(tokens: &[Token], source_length: TextSize, test_path: &Path) { +fn validate_tokens(tokens: &[Token], source_length: TextSize) { let mut previous: Option<&Token> = None; for token in tokens { assert!( token.end() <= source_length, - "{path}: Token range exceeds the source code length. Token: {token:#?}", - path = test_path.display() + "Token range exceeds the source code length. Token: {token:#?}", ); if let Some(previous) = previous { assert_eq!( previous.range().ordering(token.range()), Ordering::Less, - "{path}: Token ranges are not in increasing order + "Token ranges are not in increasing order Previous token: {previous:#?} Current token: {token:#?} Tokens: {tokens:#?} ", - path = test_path.display(), ); } @@ -403,9 +414,9 @@ Tokens: {tokens:#?} /// * the range of the parent node fully encloses all its child nodes /// * the ranges are strictly increasing when traversing the nodes in pre-order. /// * all ranges are within the length of the source code. -fn validate_ast(parsed: &Parsed, source_len: TextSize, test_path: &Path) { +fn validate_ast(parsed: &Parsed, source_len: TextSize) { walk_module( - &mut ValidateAstVisitor::new(parsed.tokens(), source_len, test_path), + &mut ValidateAstVisitor::new(parsed.tokens(), source_len), parsed.syntax(), ); } @@ -416,17 +427,15 @@ struct ValidateAstVisitor<'a> { parents: Vec>, previous: Option>, source_length: TextSize, - test_path: &'a Path, } impl<'a> ValidateAstVisitor<'a> { - fn new(tokens: &'a Tokens, source_length: TextSize, test_path: &'a Path) -> Self { + fn new(tokens: &'a Tokens, source_length: TextSize) -> Self { Self { tokens: tokens.iter().peekable(), parents: Vec::new(), previous: None, source_length, - test_path, } } } @@ -444,8 +453,7 @@ impl ValidateAstVisitor<'_> { // At this point, next_token.end() > node.start() assert!( next.start() >= node.start(), - "{path}: The start of the node falls within a token.\nNode: {node:#?}\n\nToken: {next:#?}\n\nRoot: {root:#?}", - path = self.test_path.display(), + "The start of the node falls within a token.\nNode: {node:#?}\n\nToken: {next:#?}\n\nRoot: {root:#?}", root = self.parents.first() ); } @@ -464,8 +472,7 @@ impl ValidateAstVisitor<'_> { // At this point, `next_token.end() > node.end()` assert!( next.start() >= node.end(), - "{path}: The end of the node falls within a token.\nNode: {node:#?}\n\nToken: {next:#?}\n\nRoot: {root:#?}", - path = self.test_path.display(), + "The end of the node falls within a token.\nNode: {node:#?}\n\nToken: {next:#?}\n\nRoot: {root:#?}", root = self.parents.first() ); } @@ -476,16 +483,14 @@ impl<'ast> SourceOrderVisitor<'ast> for ValidateAstVisitor<'ast> { fn enter_node(&mut self, node: AnyNodeRef<'ast>) -> TraversalSignal { assert!( node.end() <= self.source_length, - "{path}: The range of the node exceeds the length of the source code. Node: {node:#?}", - path = self.test_path.display() + "The range of the node exceeds the length of the source code. Node: {node:#?}", ); if let Some(previous) = self.previous { assert_ne!( previous.range().ordering(node.range()), Ordering::Greater, - "{path}: The ranges of the nodes are not strictly increasing when traversing the AST in pre-order.\nPrevious node: {previous:#?}\n\nCurrent node: {node:#?}\n\nRoot: {root:#?}", - path = self.test_path.display(), + "The ranges of the nodes are not strictly increasing when traversing the AST in pre-order.\nPrevious node: {previous:#?}\n\nCurrent node: {node:#?}\n\nRoot: {root:#?}", root = self.parents.first() ); } @@ -493,8 +498,7 @@ impl<'ast> SourceOrderVisitor<'ast> for ValidateAstVisitor<'ast> { if let Some(parent) = self.parents.last() { assert!( parent.range().contains_range(node.range()), - "{path}: The range of the parent node does not fully enclose the range of the child node.\nParent node: {parent:#?}\n\nChild node: {node:#?}\n\nRoot: {root:#?}", - path = self.test_path.display(), + "The range of the parent node does not fully enclose the range of the child node.\nParent node: {parent:#?}\n\nChild node: {node:#?}\n\nRoot: {root:#?}", root = self.parents.first() ); } From 82a7598aa8dc5cd2dde18e4da6e0e24b48dc7392 Mon Sep 17 00:00:00 2001 From: David Peter Date: Sat, 13 Dec 2025 16:32:09 +0100 Subject: [PATCH 02/30] [ty] Remove now-unnecessary Divergent check (#21935) ## Summary This check is not necessary thanks to https://github.com/astral-sh/ruff/pull/21906. --- crates/ty_python_semantic/src/types/call/bind.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/crates/ty_python_semantic/src/types/call/bind.rs b/crates/ty_python_semantic/src/types/call/bind.rs index e81d26d8b8..005013e70b 100644 --- a/crates/ty_python_semantic/src/types/call/bind.rs +++ b/crates/ty_python_semantic/src/types/call/bind.rs @@ -4659,15 +4659,6 @@ fn asynccontextmanager_return_type<'db>(db: &'db dyn Db, func_ty: Type<'db>) -> .ok()? .homogeneous_element_type(db); - if yield_ty.is_divergent() - || signature - .parameters() - .iter() - .any(|param| param.annotated_type().is_some_and(|ty| ty.is_divergent())) - { - return Some(yield_ty); - } - let context_manager = known_module_symbol(db, KnownModule::Contextlib, "_AsyncGeneratorContextManager") .place From f57917becd6af50e6e73029d193635a89fdba625 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sat, 13 Dec 2025 18:21:46 +0000 Subject: [PATCH 03/30] fix typo in `fuzz/README.md` (#21963) --- fuzz/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fuzz/README.md b/fuzz/README.md index 8d249780d0..fc912cd3a7 100644 --- a/fuzz/README.md +++ b/fuzz/README.md @@ -7,7 +7,7 @@ Fuzzers and associated utilities for automatic testing of Ruff. To use the fuzzers provided in this directory, start by invoking: ```bash -./fuzz/init-fuzzers.sh +./fuzz/init-fuzzer.sh ``` This will install [`cargo-fuzz`](https://github.com/rust-fuzz/cargo-fuzz) and optionally download a From bb464ed924ffaf66c2489813e961b6c003ffb555 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sat, 13 Dec 2025 20:23:16 +0000 Subject: [PATCH 04/30] [ty] Use unqualified names for displays of `TypeAliasType`s and unbound `ParamSpec`s/`TypeVar`s (#21960) --- crates/ty_ide/src/hover.rs | 8 +- crates/ty_ide/src/inlay_hints.rs | 143 ++++++++++++++---- .../resources/mdtest/call/methods.md | 2 +- .../resources/mdtest/class/super.md | 2 +- .../mdtest/generics/legacy/paramspec.md | 2 +- .../mdtest/generics/legacy/variables.md | 8 +- .../mdtest/generics/pep695/paramspec.md | 10 +- .../mdtest/generics/pep695/variables.md | 8 +- .../resources/mdtest/implicit_type_aliases.md | 2 +- .../resources/mdtest/pep695_type_aliases.md | 4 +- .../regression/paramspec_on_python39.md | 2 +- .../ty_python_semantic/src/types/display.rs | 6 +- .../src/types/infer/tests.rs | 12 +- 13 files changed, 149 insertions(+), 60 deletions(-) diff --git a/crates/ty_ide/src/hover.rs b/crates/ty_ide/src/hover.rs index 7389b4c1cc..2960b9da83 100644 --- a/crates/ty_ide/src/hover.rs +++ b/crates/ty_ide/src/hover.rs @@ -3057,10 +3057,10 @@ def function(): ); assert_snapshot!(test.hover(), @r" - typing.TypeVar + TypeVar --------------------------------------------- ```python - typing.TypeVar + TypeVar ``` --------------------------------------------- info[hover]: Hovered content is @@ -3120,10 +3120,10 @@ def function(): ); assert_snapshot!(test.hover(), @r" - typing.TypeVar + TypeVar --------------------------------------------- ```python - typing.TypeVar + TypeVar ``` --------------------------------------------- info[hover]: Hovered content is diff --git a/crates/ty_ide/src/inlay_hints.rs b/crates/ty_ide/src/inlay_hints.rs index 4f9858e61e..a958b4451d 100644 --- a/crates/ty_ide/src/inlay_hints.rs +++ b/crates/ty_ide/src/inlay_hints.rs @@ -6635,26 +6635,9 @@ mod tests { assert_snapshot!(test.inlay_hints(), @r" from typing import Protocol, TypeVar - T[: typing.TypeVar] = TypeVar([name=]'T') + T = TypeVar([name=]'T') Strange[: ] = Protocol[T] --------------------------------------------- - info[inlay-hint-location]: Inlay Hint Target - --> main.py:3:1 - | - 2 | from typing import Protocol, TypeVar - 3 | T = TypeVar('T') - | ^ - 4 | Strange = Protocol[T] - | - info: Source - --> main2.py:3:5 - | - 2 | from typing import Protocol, TypeVar - 3 | T[: typing.TypeVar] = TypeVar([name=]'T') - | ^^^^^^^^^^^^^^ - 4 | Strange[: ] = Protocol[T] - | - info[inlay-hint-location]: Inlay Hint Target --> stdlib/typing.pyi:276:13 | @@ -6666,11 +6649,11 @@ mod tests { 278 | bound: Any | None = None, # AnnotationForm | info: Source - --> main2.py:3:32 + --> main2.py:3:14 | 2 | from typing import Protocol, TypeVar - 3 | T[: typing.TypeVar] = TypeVar([name=]'T') - | ^^^^ + 3 | T = TypeVar([name=]'T') + | ^^^^ 4 | Strange[: ] = Protocol[T] | @@ -6687,7 +6670,7 @@ mod tests { --> main2.py:4:26 | 2 | from typing import Protocol, TypeVar - 3 | T[: typing.TypeVar] = TypeVar([name=]'T') + 3 | T = TypeVar([name=]'T') 4 | Strange[: ] = Protocol[T] | ^^^^^^^^^^^^^^^ | @@ -6704,18 +6687,124 @@ mod tests { --> main2.py:4:42 | 2 | from typing import Protocol, TypeVar - 3 | T[: typing.TypeVar] = TypeVar([name=]'T') + 3 | T = TypeVar([name=]'T') 4 | Strange[: ] = Protocol[T] | ^ | + "); + } + #[test] + fn test_paramspec_creation_inlay_hint() { + let mut test = inlay_hint_test( + " + from typing import ParamSpec + P = ParamSpec('P')", + ); + + assert_snapshot!(test.inlay_hints(), @r" + from typing import ParamSpec + P = ParamSpec([name=]'P') --------------------------------------------- - info[inlay-hint-edit]: File after edits + info[inlay-hint-location]: Inlay Hint Target + --> stdlib/typing.pyi:552:17 + | + 550 | def __new__( + 551 | cls, + 552 | name: str, + | ^^^^ + 553 | *, + 554 | bound: Any | None = None, # AnnotationForm + | info: Source + --> main2.py:3:16 + | + 2 | from typing import ParamSpec + 3 | P = ParamSpec([name=]'P') + | ^^^^ + | + "); + } - from typing import Protocol, TypeVar - T: typing.TypeVar = TypeVar('T') - Strange = Protocol[T] + #[test] + fn test_typealiastype_creation_inlay_hint() { + let mut test = inlay_hint_test( + " + from typing_extensions import TypeAliasType + A = TypeAliasType('A', str)", + ); + + assert_snapshot!(test.inlay_hints(), @r#" + from typing_extensions import TypeAliasType + A = TypeAliasType([name=]'A', [value=]str) + --------------------------------------------- + info[inlay-hint-location]: Inlay Hint Target + --> stdlib/typing.pyi:2032:26 + | + 2030 | """ + 2031 | + 2032 | def __new__(cls, name: str, value: Any, *, type_params: tuple[_TypeParameter, ...] = ()) -> Self: ... + | ^^^^ + 2033 | @property + 2034 | def __value__(self) -> Any: ... # AnnotationForm + | + info: Source + --> main2.py:3:20 + | + 2 | from typing_extensions import TypeAliasType + 3 | A = TypeAliasType([name=]'A', [value=]str) + | ^^^^ + | + + info[inlay-hint-location]: Inlay Hint Target + --> stdlib/typing.pyi:2032:37 + | + 2030 | """ + 2031 | + 2032 | def __new__(cls, name: str, value: Any, *, type_params: tuple[_TypeParameter, ...] = ()) -> Self: ... + | ^^^^^ + 2033 | @property + 2034 | def __value__(self) -> Any: ... # AnnotationForm + | + info: Source + --> main2.py:3:32 + | + 2 | from typing_extensions import TypeAliasType + 3 | A = TypeAliasType([name=]'A', [value=]str) + | ^^^^^ + | + "#); + } + + #[test] + fn test_typevartuple_creation_inlay_hint() { + let mut test = inlay_hint_test( + " + from typing_extensions import TypeVarTuple + Ts = TypeVarTuple('Ts')", + ); + + assert_snapshot!(test.inlay_hints(), @r" + from typing_extensions import TypeVarTuple + Ts = TypeVarTuple([name=]'Ts') + --------------------------------------------- + info[inlay-hint-location]: Inlay Hint Target + --> stdlib/typing.pyi:412:30 + | + 410 | def has_default(self) -> bool: ... + 411 | if sys.version_info >= (3, 13): + 412 | def __new__(cls, name: str, *, default: Any = ...) -> Self: ... # AnnotationForm + | ^^^^ + 413 | elif sys.version_info >= (3, 12): + 414 | def __new__(cls, name: str) -> Self: ... + | + info: Source + --> main2.py:3:20 + | + 2 | from typing_extensions import TypeVarTuple + 3 | Ts = TypeVarTuple([name=]'Ts') + | ^^^^ + | "); } diff --git a/crates/ty_python_semantic/resources/mdtest/call/methods.md b/crates/ty_python_semantic/resources/mdtest/call/methods.md index a2c7d043b4..eb02c134f3 100644 --- a/crates/ty_python_semantic/resources/mdtest/call/methods.md +++ b/crates/ty_python_semantic/resources/mdtest/call/methods.md @@ -201,7 +201,7 @@ python-version = "3.12" ```py type IntOrStr = int | str -reveal_type(IntOrStr.__or__) # revealed: bound method typing.TypeAliasType.__or__(right: Any, /) -> _SpecialForm +reveal_type(IntOrStr.__or__) # revealed: bound method TypeAliasType.__or__(right: Any, /) -> _SpecialForm ``` ## Method calls on types not disjoint from `None` diff --git a/crates/ty_python_semantic/resources/mdtest/class/super.md b/crates/ty_python_semantic/resources/mdtest/class/super.md index 4c3d09560f..ab96e93ca5 100644 --- a/crates/ty_python_semantic/resources/mdtest/class/super.md +++ b/crates/ty_python_semantic/resources/mdtest/class/super.md @@ -567,7 +567,7 @@ def f(x: int): super(x, x) type IntAlias = int - # error: [invalid-super-argument] "`typing.TypeAliasType` is not a valid class" + # error: [invalid-super-argument] "`TypeAliasType` is not a valid class" super(IntAlias, 0) # error: [invalid-super-argument] "`str` is not an instance or subclass of `` in `super(, str)` call" diff --git a/crates/ty_python_semantic/resources/mdtest/generics/legacy/paramspec.md b/crates/ty_python_semantic/resources/mdtest/generics/legacy/paramspec.md index ec3d608506..7a365b6405 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/legacy/paramspec.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/legacy/paramspec.md @@ -9,7 +9,7 @@ from typing import ParamSpec P = ParamSpec("P") reveal_type(type(P)) # revealed: -reveal_type(P) # revealed: typing.ParamSpec +reveal_type(P) # revealed: ParamSpec reveal_type(P.__name__) # revealed: Literal["P"] ``` diff --git a/crates/ty_python_semantic/resources/mdtest/generics/legacy/variables.md b/crates/ty_python_semantic/resources/mdtest/generics/legacy/variables.md index 13547e81be..b419b61e71 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/legacy/variables.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/legacy/variables.md @@ -22,7 +22,7 @@ from typing import TypeVar T = TypeVar("T") reveal_type(type(T)) # revealed: -reveal_type(T) # revealed: typing.TypeVar +reveal_type(T) # revealed: TypeVar reveal_type(T.__name__) # revealed: Literal["T"] ``` @@ -146,7 +146,7 @@ from typing import TypeVar T = TypeVar("T", default=int) reveal_type(type(T)) # revealed: -reveal_type(T) # revealed: typing.TypeVar +reveal_type(T) # revealed: TypeVar reveal_type(T.__default__) # revealed: int reveal_type(T.__bound__) # revealed: None reveal_type(T.__constraints__) # revealed: tuple[()] @@ -187,7 +187,7 @@ from typing import TypeVar T = TypeVar("T", bound=int) reveal_type(type(T)) # revealed: -reveal_type(T) # revealed: typing.TypeVar +reveal_type(T) # revealed: TypeVar reveal_type(T.__bound__) # revealed: int reveal_type(T.__constraints__) # revealed: tuple[()] @@ -211,7 +211,7 @@ from typing import TypeVar T = TypeVar("T", int, str) reveal_type(type(T)) # revealed: -reveal_type(T) # revealed: typing.TypeVar +reveal_type(T) # revealed: TypeVar reveal_type(T.__constraints__) # revealed: tuple[int, str] S = TypeVar("S") diff --git a/crates/ty_python_semantic/resources/mdtest/generics/pep695/paramspec.md b/crates/ty_python_semantic/resources/mdtest/generics/pep695/paramspec.md index 92e1e64116..354521288e 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/pep695/paramspec.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/pep695/paramspec.md @@ -12,7 +12,7 @@ python-version = "3.13" ```py def foo1[**P]() -> None: - reveal_type(P) # revealed: typing.ParamSpec + reveal_type(P) # revealed: ParamSpec ``` ## Bounds and constraints @@ -45,14 +45,14 @@ The default value for a `ParamSpec` can be either a list of types, `...`, or ano ```py def foo2[**P = ...]() -> None: - reveal_type(P) # revealed: typing.ParamSpec + reveal_type(P) # revealed: ParamSpec def foo3[**P = [int, str]]() -> None: - reveal_type(P) # revealed: typing.ParamSpec + reveal_type(P) # revealed: ParamSpec def foo4[**P, **Q = P](): - reveal_type(P) # revealed: typing.ParamSpec - reveal_type(Q) # revealed: typing.ParamSpec + reveal_type(P) # revealed: ParamSpec + reveal_type(Q) # revealed: ParamSpec ``` Other values are invalid. diff --git a/crates/ty_python_semantic/resources/mdtest/generics/pep695/variables.md b/crates/ty_python_semantic/resources/mdtest/generics/pep695/variables.md index 8338227538..d70c130649 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/pep695/variables.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/pep695/variables.md @@ -17,7 +17,7 @@ instances of `typing.TypeVar`, just like legacy type variables. ```py def f[T](): reveal_type(type(T)) # revealed: - reveal_type(T) # revealed: typing.TypeVar + reveal_type(T) # revealed: TypeVar reveal_type(T.__name__) # revealed: Literal["T"] ``` @@ -33,7 +33,7 @@ python-version = "3.13" ```py def f[T = int](): reveal_type(type(T)) # revealed: - reveal_type(T) # revealed: typing.TypeVar + reveal_type(T) # revealed: TypeVar reveal_type(T.__default__) # revealed: int reveal_type(T.__bound__) # revealed: None reveal_type(T.__constraints__) # revealed: tuple[()] @@ -66,7 +66,7 @@ class Invalid[S = T]: ... ```py def f[T: int](): reveal_type(type(T)) # revealed: - reveal_type(T) # revealed: typing.TypeVar + reveal_type(T) # revealed: TypeVar reveal_type(T.__bound__) # revealed: int reveal_type(T.__constraints__) # revealed: tuple[()] @@ -79,7 +79,7 @@ def g[S](): ```py def f[T: (int, str)](): reveal_type(type(T)) # revealed: - reveal_type(T) # revealed: typing.TypeVar + reveal_type(T) # revealed: TypeVar reveal_type(T.__constraints__) # revealed: tuple[int, str] reveal_type(T.__bound__) # revealed: None diff --git a/crates/ty_python_semantic/resources/mdtest/implicit_type_aliases.md b/crates/ty_python_semantic/resources/mdtest/implicit_type_aliases.md index 977991611c..ef6f07283f 100644 --- a/crates/ty_python_semantic/resources/mdtest/implicit_type_aliases.md +++ b/crates/ty_python_semantic/resources/mdtest/implicit_type_aliases.md @@ -400,7 +400,7 @@ reveal_type(ListOrTuple) # revealed: T@MyCallable'> reveal_type(AnnotatedType) # revealed: ]'> -reveal_type(TransparentAlias) # revealed: typing.TypeVar +reveal_type(TransparentAlias) # revealed: TypeVar reveal_type(MyOptional) # revealed: def _( diff --git a/crates/ty_python_semantic/resources/mdtest/pep695_type_aliases.md b/crates/ty_python_semantic/resources/mdtest/pep695_type_aliases.md index 799d1fc36f..aadae3f907 100644 --- a/crates/ty_python_semantic/resources/mdtest/pep695_type_aliases.md +++ b/crates/ty_python_semantic/resources/mdtest/pep695_type_aliases.md @@ -12,7 +12,7 @@ python-version = "3.12" ```py type IntOrStr = int | str -reveal_type(IntOrStr) # revealed: typing.TypeAliasType +reveal_type(IntOrStr) # revealed: TypeAliasType reveal_type(IntOrStr.__name__) # revealed: Literal["IntOrStr"] x: IntOrStr = 1 @@ -205,7 +205,7 @@ from typing_extensions import TypeAliasType, Union IntOrStr = TypeAliasType("IntOrStr", Union[int, str]) -reveal_type(IntOrStr) # revealed: typing.TypeAliasType +reveal_type(IntOrStr) # revealed: TypeAliasType reveal_type(IntOrStr.__name__) # revealed: Literal["IntOrStr"] diff --git a/crates/ty_python_semantic/resources/mdtest/regression/paramspec_on_python39.md b/crates/ty_python_semantic/resources/mdtest/regression/paramspec_on_python39.md index 1760669cf0..f0dd802adb 100644 --- a/crates/ty_python_semantic/resources/mdtest/regression/paramspec_on_python39.md +++ b/crates/ty_python_semantic/resources/mdtest/regression/paramspec_on_python39.md @@ -13,7 +13,7 @@ diagnostic message for `invalid-exception-caught` expects to construct `typing.P def foo[**P]() -> None: try: pass - # error: [invalid-exception-caught] "Invalid object caught in an exception handler: Object has type `typing.ParamSpec`" + # error: [invalid-exception-caught] "Invalid object caught in an exception handler: Object has type `ParamSpec`" except P: pass ``` diff --git a/crates/ty_python_semantic/src/types/display.rs b/crates/ty_python_semantic/src/types/display.rs index f6bee388f5..790fcc404f 100644 --- a/crates/ty_python_semantic/src/types/display.rs +++ b/crates/ty_python_semantic/src/types/display.rs @@ -2358,7 +2358,7 @@ impl<'db> FmtDetailed<'db> for DisplayKnownInstanceRepr<'db> { .fmt_detailed(f)?; f.write_str("'>") } else { - f.with_type(ty).write_str("typing.TypeAliasType") + f.with_type(ty).write_str("TypeAliasType") } } // This is a legacy `TypeVar` _outside_ of any generic class or function, so we render @@ -2366,9 +2366,9 @@ impl<'db> FmtDetailed<'db> for DisplayKnownInstanceRepr<'db> { // have a `Type::TypeVar(_)`, which is rendered as the typevar's name. KnownInstanceType::TypeVar(typevar_instance) => { if typevar_instance.kind(self.db).is_paramspec() { - f.with_type(ty).write_str("typing.ParamSpec") + f.with_type(ty).write_str("ParamSpec") } else { - f.with_type(ty).write_str("typing.TypeVar") + f.with_type(ty).write_str("TypeVar") } } KnownInstanceType::Deprecated(_) => f.write_str("warnings.deprecated"), diff --git a/crates/ty_python_semantic/src/types/infer/tests.rs b/crates/ty_python_semantic/src/types/infer/tests.rs index 9f6ae996be..90c79fd252 100644 --- a/crates/ty_python_semantic/src/types/infer/tests.rs +++ b/crates/ty_python_semantic/src/types/infer/tests.rs @@ -222,14 +222,14 @@ fn pep695_type_params() { ); }; - check_typevar("T", "typing.TypeVar", None, None, None); - check_typevar("U", "typing.TypeVar", Some("A"), None, None); - check_typevar("V", "typing.TypeVar", None, Some(&["A", "B"]), None); - check_typevar("W", "typing.TypeVar", None, None, Some("A")); - check_typevar("X", "typing.TypeVar", Some("A"), None, Some("A1")); + check_typevar("T", "TypeVar", None, None, None); + check_typevar("U", "TypeVar", Some("A"), None, None); + check_typevar("V", "TypeVar", None, Some(&["A", "B"]), None); + check_typevar("W", "TypeVar", None, None, Some("A")); + check_typevar("X", "TypeVar", Some("A"), None, Some("A1")); // a typevar with less than two constraints is treated as unconstrained - check_typevar("Y", "typing.TypeVar", None, None, None); + check_typevar("Y", "TypeVar", None, None, None); } /// Test that a symbol known to be unbound in a scope does not still trigger cycle-causing From a544c59186731d8cab4c5cce05a228af585fbafe Mon Sep 17 00:00:00 2001 From: Simon Lamon <32477463+silamon@users.noreply.github.com> Date: Sat, 13 Dec 2025 21:59:26 +0100 Subject: [PATCH 05/30] [ty] Emit a diagnostic when frozen dataclass inherits a non-frozen dataclass and the other way around (#21962) Co-authored-by: Alex Waygood --- crates/ty/docs/rules.md | 192 +++++++++++------- .../mdtest/dataclasses/dataclasses.md | 67 ++++++ ...rozen__non-frozen_inโ€ฆ_(9af2ab07b8e829e).snap | 154 ++++++++++++++ crates/ty_python_semantic/src/types.rs | 6 + crates/ty_python_semantic/src/types/class.rs | 22 ++ .../src/types/diagnostic.rs | 129 +++++++++++- .../src/types/infer/builder.rs | 40 +++- .../e2e__commands__debug_command.snap | 1 + ty.schema.json | 10 + 9 files changed, 537 insertions(+), 84 deletions(-) create mode 100644 crates/ty_python_semantic/resources/mdtest/snapshots/dataclasses.md_-_Dataclasses_-_Other_dataclass_paraโ€ฆ_-_frozen__non-frozen_inโ€ฆ_(9af2ab07b8e829e).snap diff --git a/crates/ty/docs/rules.md b/crates/ty/docs/rules.md index e32e8d3e53..c2b450ae10 100644 --- a/crates/ty/docs/rules.md +++ b/crates/ty/docs/rules.md @@ -39,7 +39,7 @@ def test(): -> "int": Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -63,7 +63,7 @@ Calling a non-callable object will raise a `TypeError` at runtime. Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -95,7 +95,7 @@ f(int) # error Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -126,7 +126,7 @@ a = 1 Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -158,7 +158,7 @@ class C(A, B): ... Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -190,7 +190,7 @@ class B(A): ... Default level: error ยท Preview (since 1.0.0) ยท Related issues ยท -View source +View source @@ -218,7 +218,7 @@ type B = A Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -245,7 +245,7 @@ class B(A, A): ... Default level: error ยท Added in 0.0.1-alpha.12 ยท Related issues ยท -View source +View source @@ -357,7 +357,7 @@ def test(): -> "Literal[5]": Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -387,7 +387,7 @@ class C(A, B): ... Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -413,7 +413,7 @@ t[3] # IndexError: tuple index out of range Default level: error ยท Added in 0.0.1-alpha.12 ยท Related issues ยท -View source +View source @@ -502,7 +502,7 @@ an atypical memory layout. Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -529,7 +529,7 @@ func("foo") # error: [invalid-argument-type] Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -557,7 +557,7 @@ a: int = '' Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -591,7 +591,7 @@ C.instance_var = 3 # error: Cannot assign to instance variable Default level: error ยท Added in 0.0.1-alpha.19 ยท Related issues ยท -View source +View source @@ -627,7 +627,7 @@ asyncio.run(main()) Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -651,7 +651,7 @@ class A(42): ... # error: [invalid-base] Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -678,7 +678,7 @@ with 1: Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -707,7 +707,7 @@ a: str Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -751,7 +751,7 @@ except ZeroDivisionError: Default level: error ยท Added in 0.0.1-alpha.28 ยท Related issues ยท -View source +View source @@ -787,13 +787,57 @@ class D(A): def foo(self): ... # fine: overrides `A.foo` ``` +## `invalid-frozen-dataclass-subclass` + + +Default level: error ยท +Added in 0.0.1-alpha.35 ยท +Related issues ยท +View source + + + +**What it does** + +Checks for dataclasses with invalid frozen inheritance: +- A frozen dataclass cannot inherit from a non-frozen dataclass. +- A non-frozen dataclass cannot inherit from a frozen dataclass. + +**Why is this bad?** + +Python raises a `TypeError` at runtime when either of these inheritance +patterns occurs. + +**Example** + + +```python +from dataclasses import dataclass + +@dataclass +class Base: + x: int + +@dataclass(frozen=True) +class Child(Base): # Error raised here + y: int + +@dataclass(frozen=True) +class FrozenBase: + x: int + +@dataclass +class NonFrozenChild(FrozenBase): # Error raised here + y: int +``` + ## `invalid-generic-class` Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -826,7 +870,7 @@ class C[U](Generic[T]): ... Default level: error ยท Added in 0.0.1-alpha.17 ยท Related issues ยท -View source +View source @@ -865,7 +909,7 @@ carol = Person(name="Carol", age=25) # typo! Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -900,7 +944,7 @@ def f(t: TypeVar("U")): ... Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -934,7 +978,7 @@ class B(metaclass=f): ... Default level: error ยท Added in 0.0.1-alpha.20 ยท Related issues ยท -View source +View source @@ -1041,7 +1085,7 @@ Correct use of `@override` is enforced by ty's `invalid-explicit-override` rule. Default level: error ยท Added in 0.0.1-alpha.19 ยท Related issues ยท -View source +View source @@ -1095,7 +1139,7 @@ AttributeError: Cannot overwrite NamedTuple attribute _asdict Default level: error ยท Preview (since 1.0.0) ยท Related issues ยท -View source +View source @@ -1125,7 +1169,7 @@ Baz = NewType("Baz", int | str) # error: invalid base for `typing.NewType` Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -1175,7 +1219,7 @@ def foo(x: int) -> int: ... Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -1201,7 +1245,7 @@ def f(a: int = ''): ... Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -1232,7 +1276,7 @@ P2 = ParamSpec("S2") # error: ParamSpec name must match the variable it's assig Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -1266,7 +1310,7 @@ TypeError: Protocols can only inherit from other protocols, got Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -1315,7 +1359,7 @@ def g(): Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -1340,7 +1384,7 @@ def func() -> int: Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -1398,7 +1442,7 @@ TODO #14889 Default level: error ยท Added in 0.0.1-alpha.6 ยท Related issues ยท -View source +View source @@ -1425,7 +1469,7 @@ NewAlias = TypeAliasType(get_name(), int) # error: TypeAliasType name mus Default level: error ยท Added in 0.0.1-alpha.29 ยท Related issues ยท -View source +View source @@ -1472,7 +1516,7 @@ Bar[int] # error: too few arguments Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -1502,7 +1546,7 @@ TYPE_CHECKING = '' Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -1532,7 +1576,7 @@ b: Annotated[int] # `Annotated` expects at least two arguments Default level: error ยท Added in 0.0.1-alpha.11 ยท Related issues ยท -View source +View source @@ -1566,7 +1610,7 @@ f(10) # Error Default level: error ยท Added in 0.0.1-alpha.11 ยท Related issues ยท -View source +View source @@ -1600,7 +1644,7 @@ class C: Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -1635,7 +1679,7 @@ T = TypeVar('T', bound=str) # valid bound TypeVar Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -1660,7 +1704,7 @@ func() # TypeError: func() missing 1 required positional argument: 'x' Default level: error ยท Added in 0.0.1-alpha.20 ยท Related issues ยท -View source +View source @@ -1693,7 +1737,7 @@ alice["age"] # KeyError Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -1722,7 +1766,7 @@ func("string") # error: [no-matching-overload] Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -1746,7 +1790,7 @@ Subscripting an object that does not support it will raise a `TypeError` at runt Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -1772,7 +1816,7 @@ for i in 34: # TypeError: 'int' object is not iterable Default level: error ยท Added in 0.0.1-alpha.29 ยท Related issues ยท -View source +View source @@ -1805,7 +1849,7 @@ class B(A): Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -1832,7 +1876,7 @@ f(1, x=2) # Error raised here Default level: error ยท Added in 0.0.1-alpha.22 ยท Related issues ยท -View source +View source @@ -1890,7 +1934,7 @@ def test(): -> "int": Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -1920,7 +1964,7 @@ static_assert(int(2.0 * 3.0) == 6) # error: does not have a statically known tr Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -1949,7 +1993,7 @@ class B(A): ... # Error raised here Default level: error ยท Preview (since 0.0.1-alpha.30) ยท Related issues ยท -View source +View source @@ -1983,7 +2027,7 @@ class F(NamedTuple): Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -2010,7 +2054,7 @@ f("foo") # Error raised here Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -2038,7 +2082,7 @@ def _(x: int): Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -2084,7 +2128,7 @@ class A: Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -2111,7 +2155,7 @@ f(x=1, y=2) # Error raised here Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -2139,7 +2183,7 @@ A().foo # AttributeError: 'A' object has no attribute 'foo' Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -2164,7 +2208,7 @@ import foo # ModuleNotFoundError: No module named 'foo' Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -2189,7 +2233,7 @@ print(x) # NameError: name 'x' is not defined Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -2226,7 +2270,7 @@ b1 < b2 < b1 # exception raised here Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -2254,7 +2298,7 @@ A() + A() # TypeError: unsupported operand type(s) for +: 'A' and 'A' Default level: error ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -2279,7 +2323,7 @@ l[1:10:0] # ValueError: slice step cannot be zero Default level: warn ยท Added in 0.0.1-alpha.20 ยท Related issues ยท -View source +View source @@ -2320,7 +2364,7 @@ class SubProto(BaseProto, Protocol): Default level: warn ยท Added in 0.0.1-alpha.16 ยท Related issues ยท -View source +View source @@ -2408,7 +2452,7 @@ a = 20 / 0 # type: ignore Default level: warn ยท Added in 0.0.1-alpha.22 ยท Related issues ยท -View source +View source @@ -2436,7 +2480,7 @@ A.c # AttributeError: type object 'A' has no attribute 'c' Default level: warn ยท Added in 0.0.1-alpha.22 ยท Related issues ยท -View source +View source @@ -2468,7 +2512,7 @@ A()[0] # TypeError: 'A' object is not subscriptable Default level: warn ยท Added in 0.0.1-alpha.22 ยท Related issues ยท -View source +View source @@ -2500,7 +2544,7 @@ from module import a # ImportError: cannot import name 'a' from 'module' Default level: warn ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -2527,7 +2571,7 @@ cast(int, f()) # Redundant Default level: warn ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source @@ -2551,7 +2595,7 @@ reveal_type(1) # NameError: name 'reveal_type' is not defined Default level: warn ยท Added in 0.0.1-alpha.15 ยท Related issues ยท -View source +View source @@ -2609,7 +2653,7 @@ def g(): Default level: warn ยท Added in 0.0.1-alpha.7 ยท Related issues ยท -View source +View source @@ -2648,7 +2692,7 @@ class D(C): ... # error: [unsupported-base] Default level: warn ยท Added in 0.0.1-alpha.22 ยท Related issues ยท -View source +View source @@ -2711,7 +2755,7 @@ def foo(x: int | str) -> int | str: Default level: ignore ยท Preview (since 0.0.1-alpha.1) ยท Related issues ยท -View source +View source @@ -2735,7 +2779,7 @@ Dividing by zero raises a `ZeroDivisionError` at runtime. Default level: ignore ยท Added in 0.0.1-alpha.1 ยท Related issues ยท -View source +View source diff --git a/crates/ty_python_semantic/resources/mdtest/dataclasses/dataclasses.md b/crates/ty_python_semantic/resources/mdtest/dataclasses/dataclasses.md index 7c64175658..60bfd36176 100644 --- a/crates/ty_python_semantic/resources/mdtest/dataclasses/dataclasses.md +++ b/crates/ty_python_semantic/resources/mdtest/dataclasses/dataclasses.md @@ -521,6 +521,73 @@ frozen = MyFrozenChildClass() del frozen.x # TODO this should emit an [invalid-assignment] ``` +### frozen/non-frozen inheritance + +If a non-frozen dataclass inherits from a frozen dataclass, an exception is raised at runtime. We +catch this error: + + + +`a.py`: + +```py +from dataclasses import dataclass + +@dataclass(frozen=True) +class FrozenBase: + x: int + +@dataclass +# error: [invalid-frozen-dataclass-subclass] "Non-frozen dataclass `Child` cannot inherit from frozen dataclass `FrozenBase`" +class Child(FrozenBase): + y: int +``` + +Frozen dataclasses inheriting from non-frozen dataclasses are also illegal: + +`b.py`: + +```py +from dataclasses import dataclass + +@dataclass +class Base: + x: int + +@dataclass(frozen=True) +# error: [invalid-frozen-dataclass-subclass] "Frozen dataclass `FrozenChild` cannot inherit from non-frozen dataclass `Base`" +class FrozenChild(Base): + y: int +``` + +Example of diagnostics when there are multiple files involved: + +`module.py`: + +```py +import dataclasses + +@dataclasses.dataclass(frozen=False) +class NotFrozenBase: + x: int +``` + +`main.py`: + +```py +from functools import total_ordering +from typing import final +from dataclasses import dataclass + +from module import NotFrozenBase + +@final +@dataclass(frozen=True) +@total_ordering +class FrozenChild(NotFrozenBase): # error: [invalid-frozen-dataclass-subclass] + y: str +``` + ### `match_args` If `match_args` is set to `True` (the default), the `__match_args__` attribute is a tuple created diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/dataclasses.md_-_Dataclasses_-_Other_dataclass_paraโ€ฆ_-_frozen__non-frozen_inโ€ฆ_(9af2ab07b8e829e).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/dataclasses.md_-_Dataclasses_-_Other_dataclass_paraโ€ฆ_-_frozen__non-frozen_inโ€ฆ_(9af2ab07b8e829e).snap new file mode 100644 index 0000000000..3b3a006b6e --- /dev/null +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/dataclasses.md_-_Dataclasses_-_Other_dataclass_paraโ€ฆ_-_frozen__non-frozen_inโ€ฆ_(9af2ab07b8e829e).snap @@ -0,0 +1,154 @@ +--- +source: crates/ty_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: dataclasses.md - Dataclasses - Other dataclass parameters - frozen/non-frozen inheritance +mdtest path: crates/ty_python_semantic/resources/mdtest/dataclasses/dataclasses.md +--- + +# Python source files + +## a.py + +``` + 1 | from dataclasses import dataclass + 2 | + 3 | @dataclass(frozen=True) + 4 | class FrozenBase: + 5 | x: int + 6 | + 7 | @dataclass + 8 | # error: [invalid-frozen-dataclass-subclass] "Non-frozen dataclass `Child` cannot inherit from frozen dataclass `FrozenBase`" + 9 | class Child(FrozenBase): +10 | y: int +``` + +## b.py + +``` + 1 | from dataclasses import dataclass + 2 | + 3 | @dataclass + 4 | class Base: + 5 | x: int + 6 | + 7 | @dataclass(frozen=True) + 8 | # error: [invalid-frozen-dataclass-subclass] "Frozen dataclass `FrozenChild` cannot inherit from non-frozen dataclass `Base`" + 9 | class FrozenChild(Base): +10 | y: int +``` + +## module.py + +``` +1 | import dataclasses +2 | +3 | @dataclasses.dataclass(frozen=False) +4 | class NotFrozenBase: +5 | x: int +``` + +## main.py + +``` + 1 | from functools import total_ordering + 2 | from typing import final + 3 | from dataclasses import dataclass + 4 | + 5 | from module import NotFrozenBase + 6 | + 7 | @final + 8 | @dataclass(frozen=True) + 9 | @total_ordering +10 | class FrozenChild(NotFrozenBase): # error: [invalid-frozen-dataclass-subclass] +11 | y: str +``` + +# Diagnostics + +``` +error[invalid-frozen-dataclass-subclass]: Non-frozen dataclass cannot inherit from frozen dataclass + --> src/a.py:7:1 + | + 5 | x: int + 6 | + 7 | @dataclass + | ---------- `Child` dataclass parameters + 8 | # error: [invalid-frozen-dataclass-subclass] "Non-frozen dataclass `Child` cannot inherit from frozen dataclass `FrozenBase`" + 9 | class Child(FrozenBase): + | ^^^^^^----------^ Subclass `Child` is not frozen but base class `FrozenBase` is +10 | y: int + | +info: This causes the class creation to fail +info: Base class definition + --> src/a.py:3:1 + | +1 | from dataclasses import dataclass +2 | +3 | @dataclass(frozen=True) + | ----------------------- `FrozenBase` dataclass parameters +4 | class FrozenBase: + | ^^^^^^^^^^ `FrozenBase` definition +5 | x: int + | +info: rule `invalid-frozen-dataclass-subclass` is enabled by default + +``` + +``` +error[invalid-frozen-dataclass-subclass]: Frozen dataclass cannot inherit from non-frozen dataclass + --> src/b.py:7:1 + | + 5 | x: int + 6 | + 7 | @dataclass(frozen=True) + | ----------------------- `FrozenChild` dataclass parameters + 8 | # error: [invalid-frozen-dataclass-subclass] "Frozen dataclass `FrozenChild` cannot inherit from non-frozen dataclass `Base`" + 9 | class FrozenChild(Base): + | ^^^^^^^^^^^^----^ Subclass `FrozenChild` is frozen but base class `Base` is not +10 | y: int + | +info: This causes the class creation to fail +info: Base class definition + --> src/b.py:3:1 + | +1 | from dataclasses import dataclass +2 | +3 | @dataclass + | ---------- `Base` dataclass parameters +4 | class Base: + | ^^^^ `Base` definition +5 | x: int + | +info: rule `invalid-frozen-dataclass-subclass` is enabled by default + +``` + +``` +error[invalid-frozen-dataclass-subclass]: Frozen dataclass cannot inherit from non-frozen dataclass + --> src/main.py:8:1 + | + 7 | @final + 8 | @dataclass(frozen=True) + | ----------------------- `FrozenChild` dataclass parameters + 9 | @total_ordering +10 | class FrozenChild(NotFrozenBase): # error: [invalid-frozen-dataclass-subclass] + | ^^^^^^^^^^^^-------------^ Subclass `FrozenChild` is frozen but base class `NotFrozenBase` is not +11 | y: str + | +info: This causes the class creation to fail +info: Base class definition + --> src/module.py:3:1 + | +1 | import dataclasses +2 | +3 | @dataclasses.dataclass(frozen=False) + | ------------------------------------ `NotFrozenBase` dataclass parameters +4 | class NotFrozenBase: + | ^^^^^^^^^^^^^ `NotFrozenBase` definition +5 | x: int + | +info: rule `invalid-frozen-dataclass-subclass` is enabled by default + +``` diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index e9311547d7..726decfc07 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -677,6 +677,12 @@ bitflags! { } } +impl DataclassFlags { + pub(crate) const fn is_frozen(self) -> bool { + self.contains(Self::FROZEN) + } +} + pub(crate) const DATACLASS_FLAGS: &[(&str, DataclassFlags)] = &[ ("init", DataclassFlags::INIT), ("repr", DataclassFlags::REPR), diff --git a/crates/ty_python_semantic/src/types/class.rs b/crates/ty_python_semantic/src/types/class.rs index 32962ea128..022e63fe33 100644 --- a/crates/ty_python_semantic/src/types/class.rs +++ b/crates/ty_python_semantic/src/types/class.rs @@ -1856,6 +1856,28 @@ impl<'db> ClassLiteral<'db> { .filter_map(|decorator| decorator.known(db)) } + /// Iterate through the decorators on this class, returning the position of the first one + /// that matches the given predicate. + pub(super) fn find_decorator_position( + self, + db: &'db dyn Db, + predicate: impl Fn(Type<'db>) -> bool, + ) -> Option { + self.decorators(db) + .iter() + .position(|decorator| predicate(*decorator)) + } + + /// Iterate through the decorators on this class, returning the index of the first one + /// that is either `@dataclass` or `@dataclass(...)`. + pub(super) fn find_dataclass_decorator_position(self, db: &'db dyn Db) -> Option { + self.find_decorator_position(db, |ty| match ty { + Type::FunctionLiteral(function) => function.is_known(db, KnownFunction::Dataclass), + Type::DataclassDecorator(_) => true, + _ => false, + }) + } + /// Is this class final? pub(super) fn is_final(self, db: &'db dyn Db) -> bool { self.known_function_decorators(db) diff --git a/crates/ty_python_semantic/src/types/diagnostic.rs b/crates/ty_python_semantic/src/types/diagnostic.rs index dcc2f6b3c5..3acd7b0a64 100644 --- a/crates/ty_python_semantic/src/types/diagnostic.rs +++ b/crates/ty_python_semantic/src/types/diagnostic.rs @@ -30,7 +30,7 @@ use crate::types::{ ProtocolInstanceType, SpecialFormType, SubclassOfInner, Type, TypeContext, binding_type, protocol_class::ProtocolClass, }; -use crate::types::{KnownInstanceType, MemberLookupPolicy}; +use crate::types::{DataclassFlags, KnownInstanceType, MemberLookupPolicy}; use crate::{Db, DisplaySettings, FxIndexMap, Module, ModuleName, Program, declare_lint}; use itertools::Itertools; use ruff_db::{ @@ -121,6 +121,7 @@ pub(crate) fn register_lints(registry: &mut LintRegistryBuilder) { registry.register_lint(&INVALID_METHOD_OVERRIDE); registry.register_lint(&INVALID_EXPLICIT_OVERRIDE); registry.register_lint(&SUPER_CALL_IN_NAMED_TUPLE_METHOD); + registry.register_lint(&INVALID_FROZEN_DATACLASS_SUBCLASS); // String annotations registry.register_lint(&BYTE_STRING_TYPE_ANNOTATION); @@ -2220,6 +2221,44 @@ declare_lint! { } } +declare_lint! { + /// ## What it does + /// Checks for dataclasses with invalid frozen inheritance: + /// - A frozen dataclass cannot inherit from a non-frozen dataclass. + /// - A non-frozen dataclass cannot inherit from a frozen dataclass. + /// + /// ## Why is this bad? + /// Python raises a `TypeError` at runtime when either of these inheritance + /// patterns occurs. + /// + /// ## Example + /// + /// ```python + /// from dataclasses import dataclass + /// + /// @dataclass + /// class Base: + /// x: int + /// + /// @dataclass(frozen=True) + /// class Child(Base): # Error raised here + /// y: int + /// + /// @dataclass(frozen=True) + /// class FrozenBase: + /// x: int + /// + /// @dataclass + /// class NonFrozenChild(FrozenBase): # Error raised here + /// y: int + /// ``` + pub(crate) static INVALID_FROZEN_DATACLASS_SUBCLASS = { + summary: "detects dataclasses with invalid frozen/non-frozen subclassing", + status: LintStatus::stable("0.0.1-alpha.35"), + default_level: Level::Error, + } +} + /// A collection of type check diagnostics. #[derive(Default, Eq, PartialEq, get_size2::GetSize)] pub struct TypeCheckDiagnostics { @@ -4269,6 +4308,94 @@ fn report_unsupported_binary_operation_impl<'a>( Some(diagnostic) } +pub(super) fn report_bad_frozen_dataclass_inheritance<'db>( + context: &InferContext<'db, '_>, + class: ClassLiteral<'db>, + class_node: &ast::StmtClassDef, + base_class: ClassLiteral<'db>, + base_class_node: &ast::Expr, + base_class_params: DataclassFlags, +) { + let db = context.db(); + + let Some(builder) = + context.report_lint(&INVALID_FROZEN_DATACLASS_SUBCLASS, class.header_range(db)) + else { + return; + }; + + let mut diagnostic = if base_class_params.is_frozen() { + let mut diagnostic = + builder.into_diagnostic("Non-frozen dataclass cannot inherit from frozen dataclass"); + diagnostic.set_concise_message(format_args!( + "Non-frozen dataclass `{}` cannot inherit from frozen dataclass `{}`", + class.name(db), + base_class.name(db) + )); + diagnostic.set_primary_message(format_args!( + "Subclass `{}` is not frozen but base class `{}` is", + class.name(db), + base_class.name(db) + )); + diagnostic + } else { + let mut diagnostic = + builder.into_diagnostic("Frozen dataclass cannot inherit from non-frozen dataclass"); + diagnostic.set_concise_message(format_args!( + "Frozen dataclass `{}` cannot inherit from non-frozen dataclass `{}`", + class.name(db), + base_class.name(db) + )); + diagnostic.set_primary_message(format_args!( + "Subclass `{}` is frozen but base class `{}` is not", + class.name(db), + base_class.name(db) + )); + diagnostic + }; + + diagnostic.annotate(context.secondary(base_class_node)); + + if let Some(position) = class.find_dataclass_decorator_position(db) { + diagnostic.annotate( + context + .secondary(&class_node.decorator_list[position]) + .message(format_args!("`{}` dataclass parameters", class.name(db))), + ); + } + diagnostic.info("This causes the class creation to fail"); + + if let Some(decorator_position) = base_class.find_dataclass_decorator_position(db) { + let mut sub = SubDiagnostic::new( + SubDiagnosticSeverity::Info, + format_args!("Base class definition"), + ); + sub.annotate( + Annotation::primary(base_class.header_span(db)) + .message(format_args!("`{}` definition", base_class.name(db))), + ); + + let base_class_file = base_class.file(db); + let module = parsed_module(db, base_class_file).load(db); + + let decorator_range = base_class + .body_scope(db) + .node(db) + .expect_class() + .node(&module) + .decorator_list[decorator_position] + .range(); + + sub.annotate( + Annotation::secondary(Span::from(base_class_file).with_range(decorator_range)).message( + format_args!("`{}` dataclass parameters", base_class.name(db)), + ), + ); + + diagnostic.sub(sub); + } +} + /// This function receives an unresolved `from foo import bar` import, /// where `foo` can be resolved to a module but that module does not /// have a `bar` member or submodule. diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index 1b78a02c32..43940a4d88 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -69,15 +69,16 @@ use crate::types::diagnostic::{ UNRESOLVED_IMPORT, UNRESOLVED_REFERENCE, UNSUPPORTED_OPERATOR, USELESS_OVERLOAD_BODY, hint_if_stdlib_attribute_exists_on_other_versions, hint_if_stdlib_submodule_exists_on_other_versions, report_attempted_protocol_instantiation, - report_bad_dunder_set_call, report_cannot_pop_required_field_on_typed_dict, - report_duplicate_bases, report_implicit_return_type, report_index_out_of_bounds, - report_instance_layout_conflict, report_invalid_arguments_to_annotated, - report_invalid_assignment, report_invalid_attribute_assignment, - report_invalid_exception_caught, report_invalid_exception_cause, - report_invalid_exception_raised, report_invalid_exception_tuple_caught, - report_invalid_generator_function_return_type, report_invalid_key_on_typed_dict, - report_invalid_or_unsupported_base, report_invalid_return_type, - report_invalid_type_checking_constant, report_named_tuple_field_with_leading_underscore, + report_bad_dunder_set_call, report_bad_frozen_dataclass_inheritance, + report_cannot_pop_required_field_on_typed_dict, report_duplicate_bases, + report_implicit_return_type, report_index_out_of_bounds, report_instance_layout_conflict, + report_invalid_arguments_to_annotated, report_invalid_assignment, + report_invalid_attribute_assignment, report_invalid_exception_caught, + report_invalid_exception_cause, report_invalid_exception_raised, + report_invalid_exception_tuple_caught, report_invalid_generator_function_return_type, + report_invalid_key_on_typed_dict, report_invalid_or_unsupported_base, + report_invalid_return_type, report_invalid_type_checking_constant, + report_named_tuple_field_with_leading_underscore, report_namedtuple_field_without_default_after_field_with_default, report_non_subscriptable, report_possibly_missing_attribute, report_possibly_unresolved_reference, report_rebound_typevar, report_slice_step_size_zero, report_unsupported_augmented_assignment, @@ -755,6 +756,27 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { )); } } + + let (base_class_literal, _) = base_class.class_literal(self.db()); + + if let (Some(base_params), Some(class_params)) = ( + base_class_literal.dataclass_params(self.db()), + class.dataclass_params(self.db()), + ) { + let base_params = base_params.flags(self.db()); + let class_is_frozen = class_params.flags(self.db()).is_frozen(); + + if base_params.is_frozen() != class_is_frozen { + report_bad_frozen_dataclass_inheritance( + &self.context, + class, + class_node, + base_class_literal, + &class_node.bases()[i], + base_params, + ); + } + } } // (4) Check that the class's MRO is resolvable diff --git a/crates/ty_server/tests/e2e/snapshots/e2e__commands__debug_command.snap b/crates/ty_server/tests/e2e/snapshots/e2e__commands__debug_command.snap index 21c4b05014..cf1b0afc64 100644 --- a/crates/ty_server/tests/e2e/snapshots/e2e__commands__debug_command.snap +++ b/crates/ty_server/tests/e2e/snapshots/e2e__commands__debug_command.snap @@ -55,6 +55,7 @@ Settings: Settings { "invalid-declaration": Error (Default), "invalid-exception-caught": Error (Default), "invalid-explicit-override": Error (Default), + "invalid-frozen-dataclass-subclass": Error (Default), "invalid-generic-class": Error (Default), "invalid-ignore-comment": Warning (Default), "invalid-key": Error (Default), diff --git a/ty.schema.json b/ty.schema.json index 13d1ac9c47..87feeb2507 100644 --- a/ty.schema.json +++ b/ty.schema.json @@ -583,6 +583,16 @@ } ] }, + "invalid-frozen-dataclass-subclass": { + "title": "detects dataclasses with invalid frozen/non-frozen subclassing", + "description": "## What it does\nChecks for dataclasses with invalid frozen inheritance:\n- A frozen dataclass cannot inherit from a non-frozen dataclass.\n- A non-frozen dataclass cannot inherit from a frozen dataclass.\n\n## Why is this bad?\nPython raises a `TypeError` at runtime when either of these inheritance\npatterns occurs.\n\n## Example\n\n```python\nfrom dataclasses import dataclass\n\n@dataclass\nclass Base:\n x: int\n\n@dataclass(frozen=True)\nclass Child(Base): # Error raised here\n y: int\n\n@dataclass(frozen=True)\nclass FrozenBase:\n x: int\n\n@dataclass\nclass NonFrozenChild(FrozenBase): # Error raised here\n y: int\n```", + "default": "error", + "oneOf": [ + { + "$ref": "#/definitions/Level" + } + ] + }, "invalid-generic-class": { "title": "detects invalid generic classes", "description": "## What it does\nChecks for the creation of invalid generic classes\n\n## Why is this bad?\nThere are several requirements that you must follow when defining a generic class.\n\n## Examples\n```python\nfrom typing import Generic, TypeVar\n\nT = TypeVar(\"T\") # okay\n\n# error: class uses both PEP-695 syntax and legacy syntax\nclass C[U](Generic[T]): ...\n```\n\n## References\n- [Typing spec: Generics](https://typing.python.org/en/latest/spec/generics.html#introduction)", From be8eb92946bb6bcc83674be050f118aa00d92d6a Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 13 Dec 2025 17:10:25 -0500 Subject: [PATCH 06/30] [ty] Add support for `__qualname__` and other implicit class attributes (#21966) ## Summary Closes https://github.com/astral-sh/ty/issues/1873 --- .../mdtest/scopes/class_implicit_attrs.md | 120 ++++++++++++++++++ crates/ty_python_semantic/src/place.rs | 30 +++++ .../src/types/infer/builder.rs | 25 +++- 3 files changed, 172 insertions(+), 3 deletions(-) create mode 100644 crates/ty_python_semantic/resources/mdtest/scopes/class_implicit_attrs.md diff --git a/crates/ty_python_semantic/resources/mdtest/scopes/class_implicit_attrs.md b/crates/ty_python_semantic/resources/mdtest/scopes/class_implicit_attrs.md new file mode 100644 index 0000000000..7130538acf --- /dev/null +++ b/crates/ty_python_semantic/resources/mdtest/scopes/class_implicit_attrs.md @@ -0,0 +1,120 @@ +# Implicit class body attributes + +## Class body implicit attributes + +Python makes certain names available implicitly inside class body scopes. These are `__qualname__`, +`__module__`, and `__doc__`, as documented at +. + +```py +class Foo: + reveal_type(__qualname__) # revealed: str + reveal_type(__module__) # revealed: str + reveal_type(__doc__) # revealed: str | None +``` + +## `__firstlineno__` (Python 3.13+) + +Python 3.13 added `__firstlineno__` to the class body namespace. + +### Available in Python 3.13+ + +```toml +[environment] +python-version = "3.13" +``` + +```py +class Foo: + reveal_type(__firstlineno__) # revealed: int +``` + +### Not available in Python 3.12 and earlier + +```toml +[environment] +python-version = "3.12" +``` + +```py +class Foo: + # error: [unresolved-reference] + __firstlineno__ +``` + +## Nested classes + +These implicit attributes are also available in nested classes, and refer to the nested class: + +```py +class Outer: + class Inner: + reveal_type(__qualname__) # revealed: str + reveal_type(__module__) # revealed: str +``` + +## Class body implicit attributes have priority over globals + +If a global variable with the same name exists, the class body implicit attribute takes priority +within the class body: + +```py +__qualname__ = 42 +__module__ = 42 + +class Foo: + # Inside the class body, these are the implicit class attributes + reveal_type(__qualname__) # revealed: str + reveal_type(__module__) # revealed: str + +# Outside the class, the globals are visible +reveal_type(__qualname__) # revealed: Literal[42] +reveal_type(__module__) # revealed: Literal[42] +``` + +## `__firstlineno__` has priority over globals (Python 3.13+) + +The same applies to `__firstlineno__` on Python 3.13+: + +```toml +[environment] +python-version = "3.13" +``` + +```py +__firstlineno__ = "not an int" + +class Foo: + reveal_type(__firstlineno__) # revealed: int + +reveal_type(__firstlineno__) # revealed: Literal["not an int"] +``` + +## Class body implicit attributes are not visible in methods + +The implicit class body attributes are only available directly in the class body, not in nested +function scopes (methods): + +```py +class Foo: + # Available directly in the class body + x = __qualname__ + reveal_type(x) # revealed: str + + def method(self): + # Not available in methods - falls back to builtins/globals + # error: [unresolved-reference] + __qualname__ +``` + +## Real-world use case: logging + +A common use case is defining a logger with the class name: + +```py +import logging + +class MyClass: + logger = logging.getLogger(__qualname__) + reveal_type(logger) # revealed: Logger +``` diff --git a/crates/ty_python_semantic/src/place.rs b/crates/ty_python_semantic/src/place.rs index a1319a3250..21dfb955a9 100644 --- a/crates/ty_python_semantic/src/place.rs +++ b/crates/ty_python_semantic/src/place.rs @@ -1,4 +1,5 @@ use ruff_db::files::File; +use ruff_python_ast::PythonVersion; use crate::dunder_all::dunder_all_names; use crate::module_resolver::{KnownModule, file_to_module, resolve_module_confident}; @@ -1633,6 +1634,35 @@ mod implicit_globals { } } +/// Looks up the type of an "implicit class body symbol". Returns [`Place::Undefined`] if +/// `name` is not present as an implicit symbol in class bodies. +/// +/// Implicit class body symbols are symbols such as `__qualname__`, `__module__`, `__doc__`, +/// and `__firstlineno__` that Python implicitly makes available inside a class body during +/// class creation. +/// +/// See +pub(crate) fn class_body_implicit_symbol<'db>( + db: &'db dyn Db, + name: &str, +) -> PlaceAndQualifiers<'db> { + match name { + "__qualname__" => Place::bound(KnownClass::Str.to_instance(db)).into(), + "__module__" => Place::bound(KnownClass::Str.to_instance(db)).into(), + // __doc__ is `str` if there's a docstring, `None` if there isn't + "__doc__" => Place::bound(UnionType::from_elements( + db, + [KnownClass::Str.to_instance(db), Type::none(db)], + )) + .into(), + // __firstlineno__ was added in Python 3.13 + "__firstlineno__" if Program::get(db).python_version(db) >= PythonVersion::PY313 => { + Place::bound(KnownClass::Int.to_instance(db)).into() + } + _ => Place::Undefined.into(), + } +} + #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] pub(crate) enum RequiresExplicitReExport { Yes, diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index 43940a4d88..9ae325a9ae 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -28,9 +28,9 @@ use crate::module_resolver::{ use crate::node_key::NodeKey; use crate::place::{ ConsideredDefinitions, Definedness, LookupError, Place, PlaceAndQualifiers, TypeOrigin, - builtins_module_scope, builtins_symbol, explicit_global_symbol, global_symbol, - module_type_implicit_global_declaration, module_type_implicit_global_symbol, place, - place_from_bindings, place_from_declarations, typing_extensions_symbol, + builtins_module_scope, builtins_symbol, class_body_implicit_symbol, explicit_global_symbol, + global_symbol, module_type_implicit_global_declaration, module_type_implicit_global_symbol, + place, place_from_bindings, place_from_declarations, typing_extensions_symbol, }; use crate::semantic_index::ast_ids::node_key::ExpressionNodeKey; use crate::semantic_index::ast_ids::{HasScopedUseId, ScopedUseId}; @@ -9210,6 +9210,25 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } PlaceAndQualifiers::from(Place::Undefined) + // If we're in a class body, check for implicit class body symbols first. + // These take precedence over globals. + .or_fall_back_to(db, || { + if scope.node(db).scope_kind().is_class() + && let Some(symbol) = place_expr.as_symbol() + { + let implicit = class_body_implicit_symbol(db, symbol.name()); + if implicit.place.is_definitely_bound() { + return implicit.map_type(|ty| { + self.narrow_place_with_applicable_constraints( + place_expr, + ty, + &constraint_keys, + ) + }); + } + } + Place::Undefined.into() + }) // No nonlocal binding? Check the module's explicit globals. // Avoid infinite recursion if `self.scope` already is the module's global scope. .or_fall_back_to(db, || { From c7eea1f2e33ba67480d4367a45559bf8f2bd1e49 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Sat, 13 Dec 2025 22:56:59 +0000 Subject: [PATCH 07/30] Update debug_assert which pointed at missing method (#21969) ## Summary I assume that the class has been renamed or split since this assertion was created. ## Test Plan Compiled locally, nothing more. Relying on CI given the triviality of this change. --- crates/ruff_diagnostics/src/edit.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_diagnostics/src/edit.rs b/crates/ruff_diagnostics/src/edit.rs index 194b4e4494..ae52088608 100644 --- a/crates/ruff_diagnostics/src/edit.rs +++ b/crates/ruff_diagnostics/src/edit.rs @@ -39,7 +39,7 @@ impl Edit { /// Creates an edit that replaces the content in `range` with `content`. pub fn range_replacement(content: String, range: TextRange) -> Self { - debug_assert!(!content.is_empty(), "Prefer `Fix::deletion`"); + debug_assert!(!content.is_empty(), "Prefer `Edit::deletion`"); Self { content: Some(Box::from(content)), From 8bc753b842967b53dc2698fdff8e1a410ffafa46 Mon Sep 17 00:00:00 2001 From: Leandro Braga <18340809+leandrobbraga@users.noreply.github.com> Date: Sun, 14 Dec 2025 06:21:54 -0300 Subject: [PATCH 08/30] [ty] Fix callout syntax in configuration mkdocs (#1875) (#21961) --- crates/ruff_dev/src/generate_ty_options.rs | 4 ++-- crates/ty/docs/configuration.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/ruff_dev/src/generate_ty_options.rs b/crates/ruff_dev/src/generate_ty_options.rs index 4e4ab0a949..733af8b00a 100644 --- a/crates/ruff_dev/src/generate_ty_options.rs +++ b/crates/ruff_dev/src/generate_ty_options.rs @@ -144,8 +144,8 @@ fn emit_field(output: &mut String, name: &str, field: &OptionField, parents: &[S output.push('\n'); if let Some(deprecated) = &field.deprecated { - output.push_str("> [!WARN] \"Deprecated\"\n"); - output.push_str("> This option has been deprecated"); + output.push_str("!!! warning \"Deprecated\"\n"); + output.push_str(" This option has been deprecated"); if let Some(since) = deprecated.since { write!(output, " in {since}").unwrap(); diff --git a/crates/ty/docs/configuration.md b/crates/ty/docs/configuration.md index 0768f1b26a..edbea9c803 100644 --- a/crates/ty/docs/configuration.md +++ b/crates/ty/docs/configuration.md @@ -432,8 +432,8 @@ respect-ignore-files = false ### `root` -> [!WARN] "Deprecated" -> This option has been deprecated. Use `environment.root` instead. +!!! warning "Deprecated" + This option has been deprecated. Use `environment.root` instead. The root of the project, used for finding first-party modules. From 86555989018e02a726058e6b19a7d590d99ca174 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Sun, 14 Dec 2025 12:56:21 -0500 Subject: [PATCH 09/30] codex attempt 1 --- .../mdtest/generics/specialize_constrained.md | 10 +- .../src/types/constraints.rs | 430 +++++++++++++----- 2 files changed, 310 insertions(+), 130 deletions(-) diff --git a/crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md b/crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md index 0cf656ccc3..da1882b375 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md @@ -43,8 +43,7 @@ def unbounded[T](): # revealed: None reveal_type(generic_context(unbounded).specialize_constrained(ConstraintSet.range(bool, T, bool) & ConstraintSet.range(Never, T, str))) - # TODO: revealed: ty_extensions.Specialization[T@unbounded = int] - # revealed: ty_extensions.Specialization[T@unbounded = bool] + # revealed: ty_extensions.Specialization[T@unbounded = int] reveal_type(generic_context(unbounded).specialize_constrained(ConstraintSet.range(Never, T, int) | ConstraintSet.range(Never, T, bool))) # revealed: ty_extensions.Specialization[T@unbounded = Never] reveal_type(generic_context(unbounded).specialize_constrained(ConstraintSet.range(Never, T, int) | ConstraintSet.range(Never, T, str))) @@ -311,19 +310,18 @@ def constrained_by_gradual_list[T: (list[Base], list[Any])](): # Same tests as above, but with the typevar constraints in a different order, to make sure the # results do not depend on our BDD variable ordering. def constrained_by_gradual_list_reverse[T: (list[Any], list[Base])](): - # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Base]] + # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = Top[list[Any]]] reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.always())) # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[object]] reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.range(Never, T, list[object]))) - # TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Any]] - # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Base] & list[Any]] + # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Any]] reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.range(Never, T, list[Any]))) # revealed: None reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.never())) # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Base]] reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.range(Never, T, list[Base]))) - # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Base]] + # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = Top[list[Any]]] reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.range(list[Base], T, object))) # TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual = list[Any]] diff --git a/crates/ty_python_semantic/src/types/constraints.rs b/crates/ty_python_semantic/src/types/constraints.rs index d4f3e436f6..8f9c34dfd5 100644 --- a/crates/ty_python_semantic/src/types/constraints.rs +++ b/crates/ty_python_semantic/src/types/constraints.rs @@ -552,6 +552,7 @@ impl<'db> ConstrainedTypeVar<'db> { return Node::new_constraint( db, ConstrainedTypeVar::new(db, typevar, Type::Never, Type::object()), + 1, ) .negate(db); } @@ -603,6 +604,7 @@ impl<'db> ConstrainedTypeVar<'db> { Type::TypeVar(bound), Type::TypeVar(bound), ), + 1, ) } @@ -613,10 +615,12 @@ impl<'db> ConstrainedTypeVar<'db> { let lower = Node::new_constraint( db, ConstrainedTypeVar::new(db, lower, Type::Never, Type::TypeVar(typevar)), + 1, ); let upper = Node::new_constraint( db, ConstrainedTypeVar::new(db, upper, Type::TypeVar(typevar), Type::object()), + 1, ); lower.and(db, upper) } @@ -626,6 +630,7 @@ impl<'db> ConstrainedTypeVar<'db> { let lower = Node::new_constraint( db, ConstrainedTypeVar::new(db, lower, Type::Never, Type::TypeVar(typevar)), + 1, ); let upper = if upper.is_object() { Node::AlwaysTrue @@ -645,11 +650,12 @@ impl<'db> ConstrainedTypeVar<'db> { let upper = Node::new_constraint( db, ConstrainedTypeVar::new(db, upper, Type::TypeVar(typevar), Type::object()), + 1, ); lower.and(db, upper) } - _ => Node::new_constraint(db, ConstrainedTypeVar::new(db, typevar, lower, upper)), + _ => Node::new_constraint(db, ConstrainedTypeVar::new(db, typevar, lower, upper), 1), } } @@ -830,6 +836,7 @@ impl<'db> Node<'db> { constraint: ConstrainedTypeVar<'db>, if_true: Node<'db>, if_false: Node<'db>, + source_order: usize, ) -> Self { debug_assert!((if_true.root_constraint(db)).is_none_or(|root_constraint| { root_constraint.ordering(db) > constraint.ordering(db) @@ -842,36 +849,60 @@ impl<'db> Node<'db> { if if_true == Node::AlwaysFalse && if_false == Node::AlwaysFalse { return Node::AlwaysFalse; } - Self::Interior(InteriorNode::new(db, constraint, if_true, if_false)) + let max_source_order = source_order + .max(if_true.max_source_order(db)) + .max(if_false.max_source_order(db)); + Self::Interior(InteriorNode::new( + db, + constraint, + if_true, + if_false, + source_order, + max_source_order, + )) } /// Creates a new BDD node for an individual constraint. (The BDD will evaluate to `true` when /// the constraint holds, and to `false` when it does not.) - fn new_constraint(db: &'db dyn Db, constraint: ConstrainedTypeVar<'db>) -> Self { + fn new_constraint( + db: &'db dyn Db, + constraint: ConstrainedTypeVar<'db>, + source_order: usize, + ) -> Self { Self::Interior(InteriorNode::new( db, constraint, Node::AlwaysTrue, Node::AlwaysFalse, + source_order, + source_order, )) } /// Creates a new BDD node for a positive or negative individual constraint. (For a positive /// constraint, this returns the same BDD node as [`new_constraint`][Self::new_constraint]. For /// a negative constraint, it returns the negation of that BDD node.) - fn new_satisfied_constraint(db: &'db dyn Db, constraint: ConstraintAssignment<'db>) -> Self { + fn new_satisfied_constraint( + db: &'db dyn Db, + constraint: ConstraintAssignment<'db>, + source_order: usize, + ) -> Self { match constraint { ConstraintAssignment::Positive(constraint) => Self::Interior(InteriorNode::new( db, constraint, Node::AlwaysTrue, Node::AlwaysFalse, + source_order, + source_order, )), ConstraintAssignment::Negative(constraint) => Self::Interior(InteriorNode::new( db, constraint, Node::AlwaysFalse, Node::AlwaysTrue, + source_order, + source_order, )), } } @@ -885,6 +916,56 @@ impl<'db> Node<'db> { } } + fn max_source_order(self, db: &'db dyn Db) -> usize { + match self { + Node::Interior(interior) => interior.max_source_order(db), + Node::AlwaysTrue | Node::AlwaysFalse => 0, + } + } + + fn with_source_order_offset(self, db: &'db dyn Db, offset: usize) -> Self { + if offset == 0 { + return self; + } + match self { + Node::AlwaysTrue => Node::AlwaysTrue, + Node::AlwaysFalse => Node::AlwaysFalse, + Node::Interior(interior) => Node::new( + db, + interior.constraint(db), + interior.if_true(db).with_source_order_offset(db, offset), + interior.if_false(db).with_source_order_offset(db, offset), + interior.source_order(db) + offset, + ), + } + } + + fn source_order_for(self, db: &'db dyn Db, constraint: ConstrainedTypeVar<'db>) -> usize { + fn walk<'db>( + db: &'db dyn Db, + node: Node<'db>, + constraint: ConstrainedTypeVar<'db>, + best: Option, + ) -> Option { + match node { + Node::AlwaysTrue | Node::AlwaysFalse => best, + Node::Interior(interior) => { + let best = if interior.constraint(db) == constraint { + Some(best.map_or(interior.source_order(db), |b| { + b.min(interior.source_order(db)) + })) + } else { + best + }; + let best = walk(db, interior.if_true(db), constraint, best); + walk(db, interior.if_false(db), constraint, best) + } + } + } + + walk(db, self, constraint, None).unwrap_or(1) + } + /// Returns whether this BDD represent the constant function `true`. fn is_always_satisfied(self, db: &'db dyn Db) -> bool { match self { @@ -992,41 +1073,33 @@ impl<'db> Node<'db> { /// Returns the `or` or union of two BDDs. fn or(self, db: &'db dyn Db, other: Self) -> Self { + let rhs_offset = self.max_source_order(db); + self.or_with_rhs_offset(db, other, rhs_offset) + } + + fn or_with_rhs_offset(self, db: &'db dyn Db, other: Self, rhs_offset: usize) -> Self { match (self, other) { - (Node::AlwaysTrue, Node::AlwaysTrue) => Node::AlwaysTrue, - (Node::AlwaysFalse, other) | (other, Node::AlwaysFalse) => other, - (Node::AlwaysTrue, Node::Interior(interior)) - | (Node::Interior(interior), Node::AlwaysTrue) => Node::new( - db, - interior.constraint(db), - Node::AlwaysTrue, - Node::AlwaysTrue, - ), - (Node::Interior(a), Node::Interior(b)) => { - // OR is commutative, which lets us halve the cache requirements - let (a, b) = if b.0 < a.0 { (b, a) } else { (a, b) }; - a.or(db, b) - } + (Node::AlwaysTrue, _) => Node::AlwaysTrue, + (_, Node::AlwaysTrue) => Node::AlwaysTrue, + (Node::AlwaysFalse, rhs) => rhs.with_source_order_offset(db, rhs_offset), + (lhs, Node::AlwaysFalse) => lhs, + (Node::Interior(lhs), Node::Interior(rhs)) => lhs.or(db, rhs, rhs_offset), } } /// Returns the `and` or intersection of two BDDs. fn and(self, db: &'db dyn Db, other: Self) -> Self { + let rhs_offset = self.max_source_order(db); + self.and_with_rhs_offset(db, other, rhs_offset) + } + + fn and_with_rhs_offset(self, db: &'db dyn Db, other: Self, rhs_offset: usize) -> Self { match (self, other) { - (Node::AlwaysFalse, Node::AlwaysFalse) => Node::AlwaysFalse, - (Node::AlwaysTrue, other) | (other, Node::AlwaysTrue) => other, - (Node::AlwaysFalse, Node::Interior(interior)) - | (Node::Interior(interior), Node::AlwaysFalse) => Node::new( - db, - interior.constraint(db), - Node::AlwaysFalse, - Node::AlwaysFalse, - ), - (Node::Interior(a), Node::Interior(b)) => { - // AND is commutative, which lets us halve the cache requirements - let (a, b) = if b.0 < a.0 { (b, a) } else { (a, b) }; - a.and(db, b) - } + (Node::AlwaysFalse, _) => Node::AlwaysFalse, + (_, Node::AlwaysFalse) => Node::AlwaysFalse, + (Node::AlwaysTrue, rhs) => rhs.with_source_order_offset(db, rhs_offset), + (lhs, Node::AlwaysTrue) => lhs, + (Node::Interior(lhs), Node::Interior(rhs)) => lhs.and(db, rhs, rhs_offset), } } @@ -1038,6 +1111,11 @@ impl<'db> Node<'db> { /// Returns a new BDD that evaluates to `true` when both input BDDs evaluate to the same /// result. fn iff(self, db: &'db dyn Db, other: Self) -> Self { + let rhs_offset = self.max_source_order(db); + self.iff_with_rhs_offset(db, other, rhs_offset) + } + + fn iff_with_rhs_offset(self, db: &'db dyn Db, other: Self, rhs_offset: usize) -> Self { match (self, other) { (Node::AlwaysFalse, Node::AlwaysFalse) | (Node::AlwaysTrue, Node::AlwaysTrue) => { Node::AlwaysTrue @@ -1048,20 +1126,22 @@ impl<'db> Node<'db> { (Node::AlwaysTrue | Node::AlwaysFalse, Node::Interior(interior)) => Node::new( db, interior.constraint(db), - self.iff(db, interior.if_true(db)), - self.iff(db, interior.if_false(db)), + self.iff_with_rhs_offset(db, interior.if_true(db), rhs_offset), + self.iff_with_rhs_offset(db, interior.if_false(db), rhs_offset), + interior.source_order(db) + rhs_offset, ), (Node::Interior(interior), Node::AlwaysTrue | Node::AlwaysFalse) => Node::new( db, interior.constraint(db), - interior.if_true(db).iff(db, other), - interior.if_false(db).iff(db, other), + interior + .if_true(db) + .iff_with_rhs_offset(db, other, rhs_offset), + interior + .if_false(db) + .iff_with_rhs_offset(db, other, rhs_offset), + interior.source_order(db), ), - (Node::Interior(a), Node::Interior(b)) => { - // IFF is commutative, which lets us halve the cache requirements - let (a, b) = if b.0 < a.0 { (b, a) } else { (a, b) }; - a.iff(db, b) - } + (Node::Interior(a), Node::Interior(b)) => a.iff(db, b, rhs_offset), } } @@ -1205,11 +1285,14 @@ impl<'db> Node<'db> { should_remove: &mut dyn FnMut(ConstrainedTypeVar<'db>) -> bool, map: &SequentMap<'db>, path: &mut PathAssignments<'db>, + max_source_order: &mut usize, ) -> Self { match self { Node::AlwaysTrue => Node::AlwaysTrue, Node::AlwaysFalse => Node::AlwaysFalse, - Node::Interior(interior) => interior.abstract_one_inner(db, should_remove, map, path), + Node::Interior(interior) => { + interior.abstract_one_inner(db, should_remove, map, path, max_source_order) + } } } @@ -1377,8 +1460,13 @@ impl<'db> Node<'db> { // false // // (Note that the `else` branch shouldn't be reachable, but we have to provide something!) - let left_node = Node::new_satisfied_constraint(db, left); - let right_node = Node::new_satisfied_constraint(db, right); + let left_node = + Node::new_satisfied_constraint(db, left, self.source_order_for(db, left.constraint())); + let right_node = Node::new_satisfied_constraint( + db, + right, + self.source_order_for(db, right.constraint()), + ); let right_result = right_node.ite(db, Node::AlwaysFalse, when_left_but_not_right); let left_result = left_node.ite(db, right_result, when_not_left); let result = replacement.ite(db, when_left_and_right, left_result); @@ -1441,8 +1529,13 @@ impl<'db> Node<'db> { // Lastly, verify that the result is consistent with the input. (It must produce the same // results when `left โˆจ right`.) If it doesn't, the substitution isn't valid, and we should // return the original BDD unmodified. - let left_node = Node::new_satisfied_constraint(db, left); - let right_node = Node::new_satisfied_constraint(db, right); + let left_node = + Node::new_satisfied_constraint(db, left, self.source_order_for(db, left.constraint())); + let right_node = Node::new_satisfied_constraint( + db, + right, + self.source_order_for(db, right.constraint()), + ); let validity = replacement.iff(db, left_node.or(db, right_node)); let constrained_original = self.and(db, validity); let constrained_replacement = result.and(db, validity); @@ -1632,6 +1725,8 @@ struct InteriorNode<'db> { constraint: ConstrainedTypeVar<'db>, if_true: Node<'db>, if_false: Node<'db>, + source_order: usize, + max_source_order: usize, } // The Salsa heap is tracked separately. @@ -1646,83 +1741,105 @@ impl<'db> InteriorNode<'db> { self.constraint(db), self.if_true(db).negate(db), self.if_false(db).negate(db), + self.source_order(db), ) } #[salsa::tracked(heap_size=ruff_memory_usage::heap_size)] - fn or(self, db: &'db dyn Db, other: Self) -> Node<'db> { + fn or(self, db: &'db dyn Db, other: Self, rhs_offset: usize) -> Node<'db> { let self_constraint = self.constraint(db); let other_constraint = other.constraint(db); match (self_constraint.ordering(db)).cmp(&other_constraint.ordering(db)) { Ordering::Equal => Node::new( db, self_constraint, - self.if_true(db).or(db, other.if_true(db)), - self.if_false(db).or(db, other.if_false(db)), + self.if_true(db) + .or_with_rhs_offset(db, other.if_true(db), rhs_offset), + self.if_false(db) + .or_with_rhs_offset(db, other.if_false(db), rhs_offset), + self.source_order(db), ), Ordering::Less => Node::new( db, self_constraint, - self.if_true(db).or(db, Node::Interior(other)), - self.if_false(db).or(db, Node::Interior(other)), + self.if_true(db) + .or_with_rhs_offset(db, Node::Interior(other), rhs_offset), + self.if_false(db) + .or_with_rhs_offset(db, Node::Interior(other), rhs_offset), + self.source_order(db), ), Ordering::Greater => Node::new( db, other_constraint, - Node::Interior(self).or(db, other.if_true(db)), - Node::Interior(self).or(db, other.if_false(db)), + Node::Interior(self).or_with_rhs_offset(db, other.if_true(db), rhs_offset), + Node::Interior(self).or_with_rhs_offset(db, other.if_false(db), rhs_offset), + other.source_order(db) + rhs_offset, ), } } #[salsa::tracked(heap_size=ruff_memory_usage::heap_size)] - fn and(self, db: &'db dyn Db, other: Self) -> Node<'db> { + fn and(self, db: &'db dyn Db, other: Self, rhs_offset: usize) -> Node<'db> { let self_constraint = self.constraint(db); let other_constraint = other.constraint(db); match (self_constraint.ordering(db)).cmp(&other_constraint.ordering(db)) { Ordering::Equal => Node::new( db, self_constraint, - self.if_true(db).and(db, other.if_true(db)), - self.if_false(db).and(db, other.if_false(db)), + self.if_true(db) + .and_with_rhs_offset(db, other.if_true(db), rhs_offset), + self.if_false(db) + .and_with_rhs_offset(db, other.if_false(db), rhs_offset), + self.source_order(db), ), Ordering::Less => Node::new( db, self_constraint, - self.if_true(db).and(db, Node::Interior(other)), - self.if_false(db).and(db, Node::Interior(other)), + self.if_true(db) + .and_with_rhs_offset(db, Node::Interior(other), rhs_offset), + self.if_false(db) + .and_with_rhs_offset(db, Node::Interior(other), rhs_offset), + self.source_order(db), ), Ordering::Greater => Node::new( db, other_constraint, - Node::Interior(self).and(db, other.if_true(db)), - Node::Interior(self).and(db, other.if_false(db)), + Node::Interior(self).and_with_rhs_offset(db, other.if_true(db), rhs_offset), + Node::Interior(self).and_with_rhs_offset(db, other.if_false(db), rhs_offset), + other.source_order(db) + rhs_offset, ), } } #[salsa::tracked(heap_size=ruff_memory_usage::heap_size)] - fn iff(self, db: &'db dyn Db, other: Self) -> Node<'db> { + fn iff(self, db: &'db dyn Db, other: Self, rhs_offset: usize) -> Node<'db> { let self_constraint = self.constraint(db); let other_constraint = other.constraint(db); match (self_constraint.ordering(db)).cmp(&other_constraint.ordering(db)) { Ordering::Equal => Node::new( db, self_constraint, - self.if_true(db).iff(db, other.if_true(db)), - self.if_false(db).iff(db, other.if_false(db)), + self.if_true(db) + .iff_with_rhs_offset(db, other.if_true(db), rhs_offset), + self.if_false(db) + .iff_with_rhs_offset(db, other.if_false(db), rhs_offset), + self.source_order(db), ), Ordering::Less => Node::new( db, self_constraint, - self.if_true(db).iff(db, Node::Interior(other)), - self.if_false(db).iff(db, Node::Interior(other)), + self.if_true(db) + .iff_with_rhs_offset(db, Node::Interior(other), rhs_offset), + self.if_false(db) + .iff_with_rhs_offset(db, Node::Interior(other), rhs_offset), + self.source_order(db), ), Ordering::Greater => Node::new( db, other_constraint, - Node::Interior(self).iff(db, other.if_true(db)), - Node::Interior(self).iff(db, other.if_false(db)), + Node::Interior(self).iff_with_rhs_offset(db, other.if_true(db), rhs_offset), + Node::Interior(self).iff_with_rhs_offset(db, other.if_false(db), rhs_offset), + other.source_order(db) + rhs_offset, ), } } @@ -1731,6 +1848,7 @@ impl<'db> InteriorNode<'db> { fn exists_one(self, db: &'db dyn Db, bound_typevar: BoundTypeVarIdentity<'db>) -> Node<'db> { let map = self.sequent_map(db); let mut path = PathAssignments::default(); + let mut max_source_order = self.max_source_order(db); self.abstract_one_inner( db, // Remove any node that constrains `bound_typevar`, or that has a lower/upper bound of @@ -1753,6 +1871,7 @@ impl<'db> InteriorNode<'db> { }, map, &mut path, + &mut max_source_order, ) } @@ -1760,6 +1879,7 @@ impl<'db> InteriorNode<'db> { fn retain_one(self, db: &'db dyn Db, bound_typevar: BoundTypeVarIdentity<'db>) -> Node<'db> { let map = self.sequent_map(db); let mut path = PathAssignments::default(); + let mut max_source_order = self.max_source_order(db); self.abstract_one_inner( db, // Remove any node that constrains some other typevar than `bound_typevar`, and any @@ -1779,6 +1899,7 @@ impl<'db> InteriorNode<'db> { }, map, &mut path, + &mut max_source_order, ) } @@ -1788,7 +1909,9 @@ impl<'db> InteriorNode<'db> { should_remove: &mut dyn FnMut(ConstrainedTypeVar<'db>) -> bool, map: &SequentMap<'db>, path: &mut PathAssignments<'db>, + max_source_order: &mut usize, ) -> Node<'db> { + *max_source_order = (*max_source_order).max(self.max_source_order(db)); let self_constraint = self.constraint(db); if should_remove(self_constraint) { // If we should remove constraints involving this typevar, then we replace this node @@ -1800,9 +1923,13 @@ impl<'db> InteriorNode<'db> { // corresponding branch. let if_true = path .walk_edge(db, map, self_constraint.when_true(), |path, new_range| { - let branch = self - .if_true(db) - .abstract_one_inner(db, should_remove, map, path); + let branch = self.if_true(db).abstract_one_inner( + db, + should_remove, + map, + path, + max_source_order, + ); path.assignments[new_range] .iter() .filter(|assignment| { @@ -1811,15 +1938,23 @@ impl<'db> InteriorNode<'db> { !should_remove(assignment.constraint()) }) .fold(branch, |branch, assignment| { - branch.and(db, Node::new_satisfied_constraint(db, *assignment)) + *max_source_order += 1; + branch.and( + db, + Node::new_satisfied_constraint(db, *assignment, *max_source_order), + ) }) }) .unwrap_or(Node::AlwaysFalse); let if_false = path .walk_edge(db, map, self_constraint.when_false(), |path, new_range| { - let branch = self - .if_false(db) - .abstract_one_inner(db, should_remove, map, path); + let branch = self.if_false(db).abstract_one_inner( + db, + should_remove, + map, + path, + max_source_order, + ); path.assignments[new_range] .iter() .filter(|assignment| { @@ -1828,7 +1963,11 @@ impl<'db> InteriorNode<'db> { !should_remove(assignment.constraint()) }) .fold(branch, |branch, assignment| { - branch.and(db, Node::new_satisfied_constraint(db, *assignment)) + *max_source_order += 1; + branch.and( + db, + Node::new_satisfied_constraint(db, *assignment, *max_source_order), + ) }) }) .unwrap_or(Node::AlwaysFalse); @@ -1837,20 +1976,31 @@ impl<'db> InteriorNode<'db> { // Otherwise, we abstract the if_false/if_true edges recursively. let if_true = path .walk_edge(db, map, self_constraint.when_true(), |path, _| { - self.if_true(db) - .abstract_one_inner(db, should_remove, map, path) + self.if_true(db).abstract_one_inner( + db, + should_remove, + map, + path, + max_source_order, + ) }) .unwrap_or(Node::AlwaysFalse); let if_false = path .walk_edge(db, map, self_constraint.when_false(), |path, _| { - self.if_false(db) - .abstract_one_inner(db, should_remove, map, path) + self.if_false(db).abstract_one_inner( + db, + should_remove, + map, + path, + max_source_order, + ) }) .unwrap_or(Node::AlwaysFalse); // NB: We cannot use `Node::new` here, because the recursive calls might introduce new // derived constraints into the result, and those constraints might appear before this // one in the BDD ordering. - Node::new_constraint(db, self_constraint).ite(db, if_true, if_false) + Node::new_constraint(db, self_constraint, self.source_order(db)) + .ite(db, if_true, if_false) } } @@ -1878,7 +2028,13 @@ impl<'db> InteriorNode<'db> { let (if_true, found_in_true) = self.if_true(db).restrict_one(db, assignment); let (if_false, found_in_false) = self.if_false(db).restrict_one(db, assignment); ( - Node::new(db, self_constraint, if_true, if_false), + Node::new( + db, + self_constraint, + if_true, + if_false, + self.source_order(db), + ), found_in_true || found_in_false, ) } @@ -2003,11 +2159,18 @@ impl<'db> InteriorNode<'db> { if seen_constraints.contains(&new_constraint) { continue; } - let new_node = Node::new_constraint(db, new_constraint); - let positive_left_node = - Node::new_satisfied_constraint(db, left_constraint.when_true()); - let positive_right_node = - Node::new_satisfied_constraint(db, right_constraint.when_true()); + let derived_source_order = simplified.max_source_order(db) + 1; + let new_node = Node::new_constraint(db, new_constraint, derived_source_order); + let positive_left_node = Node::new_satisfied_constraint( + db, + left_constraint.when_true(), + simplified.source_order_for(db, left_constraint), + ); + let positive_right_node = Node::new_satisfied_constraint( + db, + right_constraint.when_true(), + simplified.source_order_for(db, right_constraint), + ); let lhs = positive_left_node.and(db, positive_right_node); let intersection = new_node.ite(db, lhs, Node::AlwaysFalse); simplified = simplified.and(db, intersection); @@ -2037,10 +2200,16 @@ impl<'db> InteriorNode<'db> { None }; if let Some((larger_constraint, smaller_constraint)) = larger_smaller { - let positive_larger_node = - Node::new_satisfied_constraint(db, larger_constraint.when_true()); - let negative_larger_node = - Node::new_satisfied_constraint(db, larger_constraint.when_false()); + let positive_larger_node = Node::new_satisfied_constraint( + db, + larger_constraint.when_true(), + simplified.source_order_for(db, larger_constraint), + ); + let negative_larger_node = Node::new_satisfied_constraint( + db, + larger_constraint.when_false(), + simplified.source_order_for(db, larger_constraint), + ); // larger โˆจ smaller = larger simplified = simplified.substitute_union( @@ -2094,20 +2263,39 @@ impl<'db> InteriorNode<'db> { .map(|seen| (seen, intersection_constraint)), ); } - let positive_intersection_node = - Node::new_satisfied_constraint(db, intersection_constraint.when_true()); - let negative_intersection_node = - Node::new_satisfied_constraint(db, intersection_constraint.when_false()); + let derived_source_order = simplified.max_source_order(db) + 1; + let positive_intersection_node = Node::new_satisfied_constraint( + db, + intersection_constraint.when_true(), + derived_source_order, + ); + let negative_intersection_node = Node::new_satisfied_constraint( + db, + intersection_constraint.when_false(), + derived_source_order, + ); - let positive_left_node = - Node::new_satisfied_constraint(db, left_constraint.when_true()); - let negative_left_node = - Node::new_satisfied_constraint(db, left_constraint.when_false()); + let positive_left_node = Node::new_satisfied_constraint( + db, + left_constraint.when_true(), + simplified.source_order_for(db, left_constraint), + ); + let negative_left_node = Node::new_satisfied_constraint( + db, + left_constraint.when_false(), + simplified.source_order_for(db, left_constraint), + ); - let positive_right_node = - Node::new_satisfied_constraint(db, right_constraint.when_true()); - let negative_right_node = - Node::new_satisfied_constraint(db, right_constraint.when_false()); + let positive_right_node = Node::new_satisfied_constraint( + db, + right_constraint.when_true(), + simplified.source_order_for(db, right_constraint), + ); + let negative_right_node = Node::new_satisfied_constraint( + db, + right_constraint.when_false(), + simplified.source_order_for(db, right_constraint), + ); // left โˆง right = intersection simplified = simplified.substitute_intersection( @@ -2172,10 +2360,16 @@ impl<'db> InteriorNode<'db> { // All of the below hold because we just proved that the intersection of left // and right is empty. - let positive_left_node = - Node::new_satisfied_constraint(db, left_constraint.when_true()); - let positive_right_node = - Node::new_satisfied_constraint(db, right_constraint.when_true()); + let positive_left_node = Node::new_satisfied_constraint( + db, + left_constraint.when_true(), + simplified.source_order_for(db, left_constraint), + ); + let positive_right_node = Node::new_satisfied_constraint( + db, + right_constraint.when_true(), + simplified.source_order_for(db, right_constraint), + ); // left โˆง right = false simplified = simplified.substitute_intersection( @@ -3482,26 +3676,14 @@ mod tests { fn test_display_graph_output() { let expected = indoc! {r#" (T = str) - โ”กโ”โ‚ (T = bool) - โ”‚ โ”กโ”โ‚ (U = str) - โ”‚ โ”‚ โ”กโ”โ‚ (U = bool) - โ”‚ โ”‚ โ”‚ โ”กโ”โ‚ always - โ”‚ โ”‚ โ”‚ โ””โ”€โ‚€ always - โ”‚ โ”‚ โ””โ”€โ‚€ (U = bool) - โ”‚ โ”‚ โ”กโ”โ‚ always - โ”‚ โ”‚ โ””โ”€โ‚€ never - โ”‚ โ””โ”€โ‚€ (U = str) - โ”‚ โ”กโ”โ‚ (U = bool) - โ”‚ โ”‚ โ”กโ”โ‚ always - โ”‚ โ”‚ โ””โ”€โ‚€ always - โ”‚ โ””โ”€โ‚€ (U = bool) - โ”‚ โ”กโ”โ‚ always - โ”‚ โ””โ”€โ‚€ never + โ”กโ”โ‚ (U = str) + โ”‚ โ”กโ”โ‚ always + โ”‚ โ””โ”€โ‚€ (U = bool) + โ”‚ โ”กโ”โ‚ always + โ”‚ โ””โ”€โ‚€ never โ””โ”€โ‚€ (T = bool) โ”กโ”โ‚ (U = str) - โ”‚ โ”กโ”โ‚ (U = bool) - โ”‚ โ”‚ โ”กโ”โ‚ always - โ”‚ โ”‚ โ””โ”€โ‚€ always + โ”‚ โ”กโ”โ‚ always โ”‚ โ””โ”€โ‚€ (U = bool) โ”‚ โ”กโ”โ‚ always โ”‚ โ””โ”€โ‚€ never From 86271d605dddb7167aaa70f90257266c1bfbcd6c Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Sun, 14 Dec 2025 13:10:51 -0500 Subject: [PATCH 10/30] codex 2 --- .../mdtest/generics/specialize_constrained.md | 8 +-- .../src/types/constraints.rs | 58 +++++++++++++++---- 2 files changed, 52 insertions(+), 14 deletions(-) diff --git a/crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md b/crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md index da1882b375..4dad6d3090 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md @@ -43,7 +43,7 @@ def unbounded[T](): # revealed: None reveal_type(generic_context(unbounded).specialize_constrained(ConstraintSet.range(bool, T, bool) & ConstraintSet.range(Never, T, str))) - # revealed: ty_extensions.Specialization[T@unbounded = int] + # revealed: ty_extensions.Specialization[T@unbounded = bool] reveal_type(generic_context(unbounded).specialize_constrained(ConstraintSet.range(Never, T, int) | ConstraintSet.range(Never, T, bool))) # revealed: ty_extensions.Specialization[T@unbounded = Never] reveal_type(generic_context(unbounded).specialize_constrained(ConstraintSet.range(Never, T, int) | ConstraintSet.range(Never, T, str))) @@ -310,18 +310,18 @@ def constrained_by_gradual_list[T: (list[Base], list[Any])](): # Same tests as above, but with the typevar constraints in a different order, to make sure the # results do not depend on our BDD variable ordering. def constrained_by_gradual_list_reverse[T: (list[Any], list[Base])](): - # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = Top[list[Any]]] + # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Base]] reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.always())) # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[object]] reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.range(Never, T, list[object]))) - # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Any]] + # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Base] & list[Any]] reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.range(Never, T, list[Any]))) # revealed: None reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.never())) # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Base]] reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.range(Never, T, list[Base]))) - # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = Top[list[Any]]] + # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Base]] reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.range(list[Base], T, object))) # TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual = list[Any]] diff --git a/crates/ty_python_semantic/src/types/constraints.rs b/crates/ty_python_semantic/src/types/constraints.rs index 8f9c34dfd5..31a7465522 100644 --- a/crates/ty_python_semantic/src/types/constraints.rs +++ b/crates/ty_python_semantic/src/types/constraints.rs @@ -1079,8 +1079,21 @@ impl<'db> Node<'db> { fn or_with_rhs_offset(self, db: &'db dyn Db, other: Self, rhs_offset: usize) -> Self { match (self, other) { - (Node::AlwaysTrue, _) => Node::AlwaysTrue, - (_, Node::AlwaysTrue) => Node::AlwaysTrue, + (Node::AlwaysTrue, Node::AlwaysTrue) => Node::AlwaysTrue, + (Node::AlwaysTrue, Node::Interior(rhs)) => Node::new( + db, + rhs.constraint(db), + Node::AlwaysTrue, + Node::AlwaysTrue, + rhs.source_order(db) + rhs_offset, + ), + (Node::Interior(lhs), Node::AlwaysTrue) => Node::new( + db, + lhs.constraint(db), + Node::AlwaysTrue, + Node::AlwaysTrue, + lhs.source_order(db), + ), (Node::AlwaysFalse, rhs) => rhs.with_source_order_offset(db, rhs_offset), (lhs, Node::AlwaysFalse) => lhs, (Node::Interior(lhs), Node::Interior(rhs)) => lhs.or(db, rhs, rhs_offset), @@ -1095,8 +1108,21 @@ impl<'db> Node<'db> { fn and_with_rhs_offset(self, db: &'db dyn Db, other: Self, rhs_offset: usize) -> Self { match (self, other) { - (Node::AlwaysFalse, _) => Node::AlwaysFalse, - (_, Node::AlwaysFalse) => Node::AlwaysFalse, + (Node::AlwaysFalse, Node::AlwaysFalse) => Node::AlwaysFalse, + (Node::AlwaysFalse, Node::Interior(rhs)) => Node::new( + db, + rhs.constraint(db), + Node::AlwaysFalse, + Node::AlwaysFalse, + rhs.source_order(db) + rhs_offset, + ), + (Node::Interior(lhs), Node::AlwaysFalse) => Node::new( + db, + lhs.constraint(db), + Node::AlwaysFalse, + Node::AlwaysFalse, + lhs.source_order(db), + ), (Node::AlwaysTrue, rhs) => rhs.with_source_order_offset(db, rhs_offset), (lhs, Node::AlwaysTrue) => lhs, (Node::Interior(lhs), Node::Interior(rhs)) => lhs.and(db, rhs, rhs_offset), @@ -3676,14 +3702,26 @@ mod tests { fn test_display_graph_output() { let expected = indoc! {r#" (T = str) - โ”กโ”โ‚ (U = str) - โ”‚ โ”กโ”โ‚ always - โ”‚ โ””โ”€โ‚€ (U = bool) - โ”‚ โ”กโ”โ‚ always - โ”‚ โ””โ”€โ‚€ never + โ”กโ”โ‚ (T = bool) + โ”‚ โ”กโ”โ‚ (U = str) + โ”‚ โ”‚ โ”กโ”โ‚ (U = bool) + โ”‚ โ”‚ โ”‚ โ”กโ”โ‚ always + โ”‚ โ”‚ โ”‚ โ””โ”€โ‚€ always + โ”‚ โ”‚ โ””โ”€โ‚€ (U = bool) + โ”‚ โ”‚ โ”กโ”โ‚ always + โ”‚ โ”‚ โ””โ”€โ‚€ never + โ”‚ โ””โ”€โ‚€ (U = str) + โ”‚ โ”กโ”โ‚ (U = bool) + โ”‚ โ”‚ โ”กโ”โ‚ always + โ”‚ โ”‚ โ””โ”€โ‚€ always + โ”‚ โ””โ”€โ‚€ (U = bool) + โ”‚ โ”กโ”โ‚ always + โ”‚ โ””โ”€โ‚€ never โ””โ”€โ‚€ (T = bool) โ”กโ”โ‚ (U = str) - โ”‚ โ”กโ”โ‚ always + โ”‚ โ”กโ”โ‚ (U = bool) + โ”‚ โ”‚ โ”กโ”โ‚ always + โ”‚ โ”‚ โ””โ”€โ‚€ always โ”‚ โ””โ”€โ‚€ (U = bool) โ”‚ โ”กโ”โ‚ always โ”‚ โ””โ”€โ‚€ never From e583cb7682fab3dfeb2671aed12bdb39cac00999 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Sun, 14 Dec 2025 13:11:31 -0500 Subject: [PATCH 11/30] restore TODOs --- .../resources/mdtest/generics/specialize_constrained.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md b/crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md index 4dad6d3090..0cf656ccc3 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md @@ -43,6 +43,7 @@ def unbounded[T](): # revealed: None reveal_type(generic_context(unbounded).specialize_constrained(ConstraintSet.range(bool, T, bool) & ConstraintSet.range(Never, T, str))) + # TODO: revealed: ty_extensions.Specialization[T@unbounded = int] # revealed: ty_extensions.Specialization[T@unbounded = bool] reveal_type(generic_context(unbounded).specialize_constrained(ConstraintSet.range(Never, T, int) | ConstraintSet.range(Never, T, bool))) # revealed: ty_extensions.Specialization[T@unbounded = Never] @@ -314,6 +315,7 @@ def constrained_by_gradual_list_reverse[T: (list[Any], list[Base])](): reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.always())) # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[object]] reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.range(Never, T, list[object]))) + # TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Any]] # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Base] & list[Any]] reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.range(Never, T, list[Any]))) # revealed: None From bdaf8e58128521255d030f141427c675be5a7903 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Sun, 14 Dec 2025 13:17:41 -0500 Subject: [PATCH 12/30] doc --- crates/ty_python_semantic/src/types/constraints.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/ty_python_semantic/src/types/constraints.rs b/crates/ty_python_semantic/src/types/constraints.rs index 31a7465522..92a74880bc 100644 --- a/crates/ty_python_semantic/src/types/constraints.rs +++ b/crates/ty_python_semantic/src/types/constraints.rs @@ -940,6 +940,7 @@ impl<'db> Node<'db> { } } + /// Returns the smallest source_order associated with the given constraint. fn source_order_for(self, db: &'db dyn Db, constraint: ConstrainedTypeVar<'db>) -> usize { fn walk<'db>( db: &'db dyn Db, From a4a3aff8d61b6d0022f1b6d5ae5d63aaeb705116 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Sun, 14 Dec 2025 18:47:45 -0500 Subject: [PATCH 13/30] simpler source_order_for --- .../src/types/constraints.rs | 44 +++++++------------ 1 file changed, 16 insertions(+), 28 deletions(-) diff --git a/crates/ty_python_semantic/src/types/constraints.rs b/crates/ty_python_semantic/src/types/constraints.rs index 92a74880bc..f0a8735762 100644 --- a/crates/ty_python_semantic/src/types/constraints.rs +++ b/crates/ty_python_semantic/src/types/constraints.rs @@ -283,7 +283,7 @@ impl<'db> ConstraintSet<'db> { BoundTypeVarIdentity<'db>, FxHashSet>, > = FxHashMap::default(); - self.node.for_each_constraint(db, &mut |constraint| { + self.node.for_each_constraint(db, &mut |constraint, _| { let visitor = CollectReachability::default(); visitor.visit_type(db, constraint.lower(db)); visitor.visit_type(db, constraint.upper(db)); @@ -942,29 +942,13 @@ impl<'db> Node<'db> { /// Returns the smallest source_order associated with the given constraint. fn source_order_for(self, db: &'db dyn Db, constraint: ConstrainedTypeVar<'db>) -> usize { - fn walk<'db>( - db: &'db dyn Db, - node: Node<'db>, - constraint: ConstrainedTypeVar<'db>, - best: Option, - ) -> Option { - match node { - Node::AlwaysTrue | Node::AlwaysFalse => best, - Node::Interior(interior) => { - let best = if interior.constraint(db) == constraint { - Some(best.map_or(interior.source_order(db), |b| { - b.min(interior.source_order(db)) - })) - } else { - best - }; - let best = walk(db, interior.if_true(db), constraint, best); - walk(db, interior.if_false(db), constraint, best) - } + let mut best: Option = None; + self.for_each_constraint(db, &mut |candidate, order| { + if candidate == constraint { + best = Some(best.map_or(order, |b| b.min(order))); } - } - - walk(db, self, constraint, None).unwrap_or(1) + }); + best.unwrap_or(1) } /// Returns whether this BDD represent the constant function `true`. @@ -1219,7 +1203,7 @@ impl<'db> Node<'db> { } let mut typevars = FxHashSet::default(); - self.for_each_constraint(db, &mut |constraint| { + self.for_each_constraint(db, &mut |constraint, _| { typevars.insert(constraint.typevar(db)); }); @@ -1577,11 +1561,15 @@ impl<'db> Node<'db> { /// constraint can appear multiple times in different paths from the root; we do not /// deduplicate those constraints, and will instead invoke the callback each time we encounter /// the constraint.) - fn for_each_constraint(self, db: &'db dyn Db, f: &mut dyn FnMut(ConstrainedTypeVar<'db>)) { + fn for_each_constraint( + self, + db: &'db dyn Db, + f: &mut dyn FnMut(ConstrainedTypeVar<'db>, usize), + ) { let Node::Interior(interior) = self else { return; }; - f(interior.constraint(db)); + f(interior.constraint(db), interior.source_order(db)); interior.if_true(db).for_each_constraint(db, f); interior.if_false(db).for_each_constraint(db, f); } @@ -2081,7 +2069,7 @@ impl<'db> InteriorNode<'db> { "create sequent map", ); let mut map = SequentMap::default(); - Node::Interior(self).for_each_constraint(db, &mut |constraint| { + Node::Interior(self).for_each_constraint(db, &mut |constraint, _| { map.add(db, constraint); }); map @@ -2110,7 +2098,7 @@ impl<'db> InteriorNode<'db> { // visit queue with all pairs of those constraints. (We use "combinations" because we don't // need to compare a constraint against itself, and because ordering doesn't matter.) let mut seen_constraints = FxHashSet::default(); - Node::Interior(self).for_each_constraint(db, &mut |constraint| { + Node::Interior(self).for_each_constraint(db, &mut |constraint, _| { seen_constraints.insert(constraint); }); let mut to_visit: Vec<(_, _)> = (seen_constraints.iter().copied()) From d223f64af1aab5a0ce44ab995b7cd5bcd2885c51 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Sun, 14 Dec 2025 18:56:22 -0500 Subject: [PATCH 14/30] remove source_order_for --- .../src/types/constraints.rs | 122 +++++++++++------- 1 file changed, 78 insertions(+), 44 deletions(-) diff --git a/crates/ty_python_semantic/src/types/constraints.rs b/crates/ty_python_semantic/src/types/constraints.rs index f0a8735762..b412148e6a 100644 --- a/crates/ty_python_semantic/src/types/constraints.rs +++ b/crates/ty_python_semantic/src/types/constraints.rs @@ -940,17 +940,6 @@ impl<'db> Node<'db> { } } - /// Returns the smallest source_order associated with the given constraint. - fn source_order_for(self, db: &'db dyn Db, constraint: ConstrainedTypeVar<'db>) -> usize { - let mut best: Option = None; - self.for_each_constraint(db, &mut |candidate, order| { - if candidate == constraint { - best = Some(best.map_or(order, |b| b.min(order))); - } - }); - best.unwrap_or(1) - } - /// Returns whether this BDD represent the constant function `true`. fn is_always_satisfied(self, db: &'db dyn Db) -> bool { match self { @@ -1437,7 +1426,9 @@ impl<'db> Node<'db> { self, db: &'db dyn Db, left: ConstraintAssignment<'db>, + left_source_order: usize, right: ConstraintAssignment<'db>, + right_source_order: usize, replacement: Node<'db>, ) -> Self { // We perform a Shannon expansion to find out what the input BDD evaluates to when: @@ -1471,13 +1462,8 @@ impl<'db> Node<'db> { // false // // (Note that the `else` branch shouldn't be reachable, but we have to provide something!) - let left_node = - Node::new_satisfied_constraint(db, left, self.source_order_for(db, left.constraint())); - let right_node = Node::new_satisfied_constraint( - db, - right, - self.source_order_for(db, right.constraint()), - ); + let left_node = Node::new_satisfied_constraint(db, left, left_source_order); + let right_node = Node::new_satisfied_constraint(db, right, right_source_order); let right_result = right_node.ite(db, Node::AlwaysFalse, when_left_but_not_right); let left_result = left_node.ite(db, right_result, when_not_left); let result = replacement.ite(db, when_left_and_right, left_result); @@ -1500,7 +1486,9 @@ impl<'db> Node<'db> { self, db: &'db dyn Db, left: ConstraintAssignment<'db>, + left_source_order: usize, right: ConstraintAssignment<'db>, + right_source_order: usize, replacement: Node<'db>, ) -> Self { // We perform a Shannon expansion to find out what the input BDD evaluates to when: @@ -1540,13 +1528,8 @@ impl<'db> Node<'db> { // Lastly, verify that the result is consistent with the input. (It must produce the same // results when `left โˆจ right`.) If it doesn't, the substitution isn't valid, and we should // return the original BDD unmodified. - let left_node = - Node::new_satisfied_constraint(db, left, self.source_order_for(db, left.constraint())); - let right_node = Node::new_satisfied_constraint( - db, - right, - self.source_order_for(db, right.constraint()), - ); + let left_node = Node::new_satisfied_constraint(db, left, left_source_order); + let right_node = Node::new_satisfied_constraint(db, right, right_source_order); let validity = replacement.iff(db, left_node.or(db, right_node)); let constrained_original = self.and(db, validity); let constrained_replacement = result.and(db, validity); @@ -2098,8 +2081,10 @@ impl<'db> InteriorNode<'db> { // visit queue with all pairs of those constraints. (We use "combinations" because we don't // need to compare a constraint against itself, and because ordering doesn't matter.) let mut seen_constraints = FxHashSet::default(); - Node::Interior(self).for_each_constraint(db, &mut |constraint, _| { + let mut source_orders = FxHashMap::default(); + Node::Interior(self).for_each_constraint(db, &mut |constraint, source_order| { seen_constraints.insert(constraint); + source_orders.insert(constraint, source_order); }); let mut to_visit: Vec<(_, _)> = (seen_constraints.iter().copied()) .tuple_combinations() @@ -2108,7 +2093,11 @@ impl<'db> InteriorNode<'db> { // Repeatedly pop constraint pairs off of the visit queue, checking whether each pair can // be simplified. let mut simplified = Node::Interior(self); + let mut next_source_order = self.max_source_order(db) + 1; while let Some((left_constraint, right_constraint)) = to_visit.pop() { + let left_source_order = source_orders[&left_constraint]; + let right_source_order = source_orders[&right_constraint]; + // If the constraints refer to different typevars, the only simplifications we can make // are of the form `S โ‰ค T โˆง T โ‰ค int โ†’ S โ‰ค int`. let left_typevar = left_constraint.typevar(db); @@ -2174,17 +2163,17 @@ impl<'db> InteriorNode<'db> { if seen_constraints.contains(&new_constraint) { continue; } - let derived_source_order = simplified.max_source_order(db) + 1; - let new_node = Node::new_constraint(db, new_constraint, derived_source_order); + let new_node = Node::new_constraint(db, new_constraint, next_source_order); + next_source_order += 1; let positive_left_node = Node::new_satisfied_constraint( db, left_constraint.when_true(), - simplified.source_order_for(db, left_constraint), + left_source_order, ); let positive_right_node = Node::new_satisfied_constraint( db, right_constraint.when_true(), - simplified.source_order_for(db, right_constraint), + right_source_order, ); let lhs = positive_left_node.and(db, positive_right_node); let intersection = new_node.ite(db, lhs, Node::AlwaysFalse); @@ -2208,29 +2197,47 @@ impl<'db> InteriorNode<'db> { // Containment: The range of one constraint might completely contain the range of the // other. If so, there are several potential simplifications. let larger_smaller = if left_constraint.implies(db, right_constraint) { - Some((right_constraint, left_constraint)) + Some(( + right_constraint, + right_source_order, + left_constraint, + left_source_order, + )) } else if right_constraint.implies(db, left_constraint) { - Some((left_constraint, right_constraint)) + Some(( + left_constraint, + left_source_order, + right_constraint, + right_source_order, + )) } else { None }; - if let Some((larger_constraint, smaller_constraint)) = larger_smaller { + if let Some(( + larger_constraint, + larger_source_order, + smaller_constraint, + smaller_source_order, + )) = larger_smaller + { let positive_larger_node = Node::new_satisfied_constraint( db, larger_constraint.when_true(), - simplified.source_order_for(db, larger_constraint), + larger_source_order, ); let negative_larger_node = Node::new_satisfied_constraint( db, larger_constraint.when_false(), - simplified.source_order_for(db, larger_constraint), + larger_source_order, ); // larger โˆจ smaller = larger simplified = simplified.substitute_union( db, larger_constraint.when_true(), + larger_source_order, smaller_constraint.when_true(), + smaller_source_order, positive_larger_node, ); @@ -2238,7 +2245,9 @@ impl<'db> InteriorNode<'db> { simplified = simplified.substitute_intersection( db, larger_constraint.when_false(), + larger_source_order, smaller_constraint.when_false(), + smaller_source_order, negative_larger_node, ); @@ -2247,7 +2256,9 @@ impl<'db> InteriorNode<'db> { simplified = simplified.substitute_intersection( db, larger_constraint.when_false(), + larger_source_order, smaller_constraint.when_true(), + smaller_source_order, Node::AlwaysFalse, ); @@ -2256,7 +2267,9 @@ impl<'db> InteriorNode<'db> { simplified = simplified.substitute_union( db, larger_constraint.when_true(), + larger_source_order, smaller_constraint.when_false(), + smaller_source_order, Node::AlwaysTrue, ); } @@ -2278,45 +2291,48 @@ impl<'db> InteriorNode<'db> { .map(|seen| (seen, intersection_constraint)), ); } - let derived_source_order = simplified.max_source_order(db) + 1; let positive_intersection_node = Node::new_satisfied_constraint( db, intersection_constraint.when_true(), - derived_source_order, + next_source_order, ); + next_source_order += 1; let negative_intersection_node = Node::new_satisfied_constraint( db, intersection_constraint.when_false(), - derived_source_order, + next_source_order, ); + next_source_order += 1; let positive_left_node = Node::new_satisfied_constraint( db, left_constraint.when_true(), - simplified.source_order_for(db, left_constraint), + left_source_order, ); let negative_left_node = Node::new_satisfied_constraint( db, left_constraint.when_false(), - simplified.source_order_for(db, left_constraint), + left_source_order, ); let positive_right_node = Node::new_satisfied_constraint( db, right_constraint.when_true(), - simplified.source_order_for(db, right_constraint), + right_source_order, ); let negative_right_node = Node::new_satisfied_constraint( db, right_constraint.when_false(), - simplified.source_order_for(db, right_constraint), + right_source_order, ); // left โˆง right = intersection simplified = simplified.substitute_intersection( db, left_constraint.when_true(), + left_source_order, right_constraint.when_true(), + right_source_order, positive_intersection_node, ); @@ -2324,7 +2340,9 @@ impl<'db> InteriorNode<'db> { simplified = simplified.substitute_union( db, left_constraint.when_false(), + left_source_order, right_constraint.when_false(), + right_source_order, negative_intersection_node, ); @@ -2334,7 +2352,9 @@ impl<'db> InteriorNode<'db> { simplified = simplified.substitute_intersection( db, left_constraint.when_true(), + left_source_order, right_constraint.when_false(), + right_source_order, positive_left_node.and(db, negative_intersection_node), ); @@ -2343,7 +2363,9 @@ impl<'db> InteriorNode<'db> { simplified = simplified.substitute_intersection( db, left_constraint.when_false(), + left_source_order, right_constraint.when_true(), + right_source_order, positive_right_node.and(db, negative_intersection_node), ); @@ -2353,7 +2375,9 @@ impl<'db> InteriorNode<'db> { simplified = simplified.substitute_union( db, left_constraint.when_true(), + left_source_order, right_constraint.when_false(), + right_source_order, negative_right_node.or(db, positive_intersection_node), ); @@ -2362,7 +2386,9 @@ impl<'db> InteriorNode<'db> { simplified = simplified.substitute_union( db, left_constraint.when_false(), + left_source_order, right_constraint.when_true(), + right_source_order, negative_left_node.or(db, positive_intersection_node), ); } @@ -2378,19 +2404,21 @@ impl<'db> InteriorNode<'db> { let positive_left_node = Node::new_satisfied_constraint( db, left_constraint.when_true(), - simplified.source_order_for(db, left_constraint), + left_source_order, ); let positive_right_node = Node::new_satisfied_constraint( db, right_constraint.when_true(), - simplified.source_order_for(db, right_constraint), + right_source_order, ); // left โˆง right = false simplified = simplified.substitute_intersection( db, left_constraint.when_true(), + left_source_order, right_constraint.when_true(), + right_source_order, Node::AlwaysFalse, ); @@ -2398,7 +2426,9 @@ impl<'db> InteriorNode<'db> { simplified = simplified.substitute_union( db, left_constraint.when_false(), + left_source_order, right_constraint.when_false(), + right_source_order, Node::AlwaysTrue, ); @@ -2407,7 +2437,9 @@ impl<'db> InteriorNode<'db> { simplified = simplified.substitute_intersection( db, left_constraint.when_true(), + left_source_order, right_constraint.when_false(), + right_source_order, positive_left_node, ); @@ -2416,7 +2448,9 @@ impl<'db> InteriorNode<'db> { simplified = simplified.substitute_intersection( db, left_constraint.when_false(), + left_source_order, right_constraint.when_true(), + right_source_order, positive_right_node, ); } From 49ca97a20e7269debe39fe8acf58f74ce22259fe Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Sun, 14 Dec 2025 19:01:44 -0500 Subject: [PATCH 15/30] lots of renaming --- .../src/types/constraints.rs | 76 +++++++++---------- 1 file changed, 36 insertions(+), 40 deletions(-) diff --git a/crates/ty_python_semantic/src/types/constraints.rs b/crates/ty_python_semantic/src/types/constraints.rs index b412148e6a..5aa6c0443c 100644 --- a/crates/ty_python_semantic/src/types/constraints.rs +++ b/crates/ty_python_semantic/src/types/constraints.rs @@ -923,8 +923,9 @@ impl<'db> Node<'db> { } } - fn with_source_order_offset(self, db: &'db dyn Db, offset: usize) -> Self { - if offset == 0 { + /// Returns a copy of this BDD node with all `source_order`s adjusted by the given amount. + fn with_adjusted_source_order(self, db: &'db dyn Db, delta: usize) -> Self { + if delta == 0 { return self; } match self { @@ -933,9 +934,9 @@ impl<'db> Node<'db> { Node::Interior(interior) => Node::new( db, interior.constraint(db), - interior.if_true(db).with_source_order_offset(db, offset), - interior.if_false(db).with_source_order_offset(db, offset), - interior.source_order(db) + offset, + interior.if_true(db).with_adjusted_source_order(db, delta), + interior.if_false(db).with_adjusted_source_order(db, delta), + interior.source_order(db) + delta, ), } } @@ -1048,10 +1049,10 @@ impl<'db> Node<'db> { /// Returns the `or` or union of two BDDs. fn or(self, db: &'db dyn Db, other: Self) -> Self { let rhs_offset = self.max_source_order(db); - self.or_with_rhs_offset(db, other, rhs_offset) + self.or_inner(db, other, rhs_offset) } - fn or_with_rhs_offset(self, db: &'db dyn Db, other: Self, rhs_offset: usize) -> Self { + fn or_inner(self, db: &'db dyn Db, other: Self, rhs_offset: usize) -> Self { match (self, other) { (Node::AlwaysTrue, Node::AlwaysTrue) => Node::AlwaysTrue, (Node::AlwaysTrue, Node::Interior(rhs)) => Node::new( @@ -1068,7 +1069,7 @@ impl<'db> Node<'db> { Node::AlwaysTrue, lhs.source_order(db), ), - (Node::AlwaysFalse, rhs) => rhs.with_source_order_offset(db, rhs_offset), + (Node::AlwaysFalse, rhs) => rhs.with_adjusted_source_order(db, rhs_offset), (lhs, Node::AlwaysFalse) => lhs, (Node::Interior(lhs), Node::Interior(rhs)) => lhs.or(db, rhs, rhs_offset), } @@ -1077,10 +1078,10 @@ impl<'db> Node<'db> { /// Returns the `and` or intersection of two BDDs. fn and(self, db: &'db dyn Db, other: Self) -> Self { let rhs_offset = self.max_source_order(db); - self.and_with_rhs_offset(db, other, rhs_offset) + self.and_inner(db, other, rhs_offset) } - fn and_with_rhs_offset(self, db: &'db dyn Db, other: Self, rhs_offset: usize) -> Self { + fn and_inner(self, db: &'db dyn Db, other: Self, rhs_offset: usize) -> Self { match (self, other) { (Node::AlwaysFalse, Node::AlwaysFalse) => Node::AlwaysFalse, (Node::AlwaysFalse, Node::Interior(rhs)) => Node::new( @@ -1097,7 +1098,7 @@ impl<'db> Node<'db> { Node::AlwaysFalse, lhs.source_order(db), ), - (Node::AlwaysTrue, rhs) => rhs.with_source_order_offset(db, rhs_offset), + (Node::AlwaysTrue, rhs) => rhs.with_adjusted_source_order(db, rhs_offset), (lhs, Node::AlwaysTrue) => lhs, (Node::Interior(lhs), Node::Interior(rhs)) => lhs.and(db, rhs, rhs_offset), } @@ -1112,10 +1113,10 @@ impl<'db> Node<'db> { /// result. fn iff(self, db: &'db dyn Db, other: Self) -> Self { let rhs_offset = self.max_source_order(db); - self.iff_with_rhs_offset(db, other, rhs_offset) + self.iff_inner(db, other, rhs_offset) } - fn iff_with_rhs_offset(self, db: &'db dyn Db, other: Self, rhs_offset: usize) -> Self { + fn iff_inner(self, db: &'db dyn Db, other: Self, rhs_offset: usize) -> Self { match (self, other) { (Node::AlwaysFalse, Node::AlwaysFalse) | (Node::AlwaysTrue, Node::AlwaysTrue) => { Node::AlwaysTrue @@ -1126,19 +1127,15 @@ impl<'db> Node<'db> { (Node::AlwaysTrue | Node::AlwaysFalse, Node::Interior(interior)) => Node::new( db, interior.constraint(db), - self.iff_with_rhs_offset(db, interior.if_true(db), rhs_offset), - self.iff_with_rhs_offset(db, interior.if_false(db), rhs_offset), + self.iff_inner(db, interior.if_true(db), rhs_offset), + self.iff_inner(db, interior.if_false(db), rhs_offset), interior.source_order(db) + rhs_offset, ), (Node::Interior(interior), Node::AlwaysTrue | Node::AlwaysFalse) => Node::new( db, interior.constraint(db), - interior - .if_true(db) - .iff_with_rhs_offset(db, other, rhs_offset), - interior - .if_false(db) - .iff_with_rhs_offset(db, other, rhs_offset), + interior.if_true(db).iff_inner(db, other, rhs_offset), + interior.if_false(db).iff_inner(db, other, rhs_offset), interior.source_order(db), ), (Node::Interior(a), Node::Interior(b)) => a.iff(db, b, rhs_offset), @@ -1751,26 +1748,25 @@ impl<'db> InteriorNode<'db> { Ordering::Equal => Node::new( db, self_constraint, - self.if_true(db) - .or_with_rhs_offset(db, other.if_true(db), rhs_offset), + self.if_true(db).or_inner(db, other.if_true(db), rhs_offset), self.if_false(db) - .or_with_rhs_offset(db, other.if_false(db), rhs_offset), + .or_inner(db, other.if_false(db), rhs_offset), self.source_order(db), ), Ordering::Less => Node::new( db, self_constraint, self.if_true(db) - .or_with_rhs_offset(db, Node::Interior(other), rhs_offset), + .or_inner(db, Node::Interior(other), rhs_offset), self.if_false(db) - .or_with_rhs_offset(db, Node::Interior(other), rhs_offset), + .or_inner(db, Node::Interior(other), rhs_offset), self.source_order(db), ), Ordering::Greater => Node::new( db, other_constraint, - Node::Interior(self).or_with_rhs_offset(db, other.if_true(db), rhs_offset), - Node::Interior(self).or_with_rhs_offset(db, other.if_false(db), rhs_offset), + Node::Interior(self).or_inner(db, other.if_true(db), rhs_offset), + Node::Interior(self).or_inner(db, other.if_false(db), rhs_offset), other.source_order(db) + rhs_offset, ), } @@ -1785,25 +1781,25 @@ impl<'db> InteriorNode<'db> { db, self_constraint, self.if_true(db) - .and_with_rhs_offset(db, other.if_true(db), rhs_offset), + .and_inner(db, other.if_true(db), rhs_offset), self.if_false(db) - .and_with_rhs_offset(db, other.if_false(db), rhs_offset), + .and_inner(db, other.if_false(db), rhs_offset), self.source_order(db), ), Ordering::Less => Node::new( db, self_constraint, self.if_true(db) - .and_with_rhs_offset(db, Node::Interior(other), rhs_offset), + .and_inner(db, Node::Interior(other), rhs_offset), self.if_false(db) - .and_with_rhs_offset(db, Node::Interior(other), rhs_offset), + .and_inner(db, Node::Interior(other), rhs_offset), self.source_order(db), ), Ordering::Greater => Node::new( db, other_constraint, - Node::Interior(self).and_with_rhs_offset(db, other.if_true(db), rhs_offset), - Node::Interior(self).and_with_rhs_offset(db, other.if_false(db), rhs_offset), + Node::Interior(self).and_inner(db, other.if_true(db), rhs_offset), + Node::Interior(self).and_inner(db, other.if_false(db), rhs_offset), other.source_order(db) + rhs_offset, ), } @@ -1818,25 +1814,25 @@ impl<'db> InteriorNode<'db> { db, self_constraint, self.if_true(db) - .iff_with_rhs_offset(db, other.if_true(db), rhs_offset), + .iff_inner(db, other.if_true(db), rhs_offset), self.if_false(db) - .iff_with_rhs_offset(db, other.if_false(db), rhs_offset), + .iff_inner(db, other.if_false(db), rhs_offset), self.source_order(db), ), Ordering::Less => Node::new( db, self_constraint, self.if_true(db) - .iff_with_rhs_offset(db, Node::Interior(other), rhs_offset), + .iff_inner(db, Node::Interior(other), rhs_offset), self.if_false(db) - .iff_with_rhs_offset(db, Node::Interior(other), rhs_offset), + .iff_inner(db, Node::Interior(other), rhs_offset), self.source_order(db), ), Ordering::Greater => Node::new( db, other_constraint, - Node::Interior(self).iff_with_rhs_offset(db, other.if_true(db), rhs_offset), - Node::Interior(self).iff_with_rhs_offset(db, other.if_false(db), rhs_offset), + Node::Interior(self).iff_inner(db, other.if_true(db), rhs_offset), + Node::Interior(self).iff_inner(db, other.if_false(db), rhs_offset), other.source_order(db) + rhs_offset, ), } From 5a8a9500b94c9fc16d9e4386527efcd32a1aa683 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Sun, 14 Dec 2025 19:38:07 -0500 Subject: [PATCH 16/30] sort specialize_constrained by source_order --- .../src/types/constraints.rs | 70 ++++++++++++------- 1 file changed, 45 insertions(+), 25 deletions(-) diff --git a/crates/ty_python_semantic/src/types/constraints.rs b/crates/ty_python_semantic/src/types/constraints.rs index 5aa6c0443c..1875bbeb97 100644 --- a/crates/ty_python_semantic/src/types/constraints.rs +++ b/crates/ty_python_semantic/src/types/constraints.rs @@ -1312,33 +1312,41 @@ impl<'db> Node<'db> { mut f: impl FnMut(Option<(Type<'db>, Type<'db>)>), ) { self.retain_one(db, bound_typevar) - .find_representative_types_inner(db, None, &mut f); + .find_representative_types_inner(db, &mut Vec::default(), &mut f); } fn find_representative_types_inner( self, db: &'db dyn Db, - current_bounds: Option<(Type<'db>, Type<'db>)>, + current_bounds: &mut Vec>, f: &mut dyn FnMut(Option<(Type<'db>, Type<'db>)>), ) { match self { Node::AlwaysTrue => { + if current_bounds.is_empty() { + f(None); + return; + } + // If we reach the `true` terminal, the path we've been following represents one // representative type. + current_bounds.sort_unstable_by_key(|bounds| bounds.source_order); + let greatest_lower_bound = + UnionType::from_elements(db, current_bounds.iter().map(|bounds| bounds.lower)); + let least_upper_bound = IntersectionType::from_elements( + db, + current_bounds.iter().map(|bounds| bounds.upper), + ); // If `lower โ‰ฐ upper`, then this path somehow represents in invalid specialization. // That should have been removed from the BDD domain as part of the simplification // process. - debug_assert!(current_bounds.is_none_or( - |(greatest_lower_bound, least_upper_bound)| { - greatest_lower_bound.is_assignable_to(db, least_upper_bound) - } - )); + debug_assert!(greatest_lower_bound.is_assignable_to(db, least_upper_bound)); // We've been tracking the lower and upper bound that the types for this path must // satisfy. Pass those bounds along and let the caller choose a representative type // from within that range. - f(current_bounds); + f(Some((greatest_lower_bound, least_upper_bound))); } Node::AlwaysFalse => { @@ -1347,8 +1355,7 @@ impl<'db> Node<'db> { } Node::Interior(interior) => { - let (greatest_lower_bound, least_upper_bound) = - current_bounds.unwrap_or((Type::Never, Type::object())); + let reset_point = current_bounds.len(); // For an interior node, there are two outgoing paths: one for the `if_true` // branch, and one for the `if_false` branch. @@ -1357,16 +1364,11 @@ impl<'db> Node<'db> { // on the types that satisfy the current path through the BDD. So we intersect the // current glb/lub with the constraint's bounds to get the new glb/lub for the // recursive call. - let constraint = interior.constraint(db); - let new_greatest_lower_bound = - UnionType::from_elements(db, [greatest_lower_bound, constraint.lower(db)]); - let new_least_upper_bound = - IntersectionType::from_elements(db, [least_upper_bound, constraint.upper(db)]); - interior.if_true(db).find_representative_types_inner( - db, - Some((new_greatest_lower_bound, new_least_upper_bound)), - f, - ); + current_bounds.push(RepresentativeBounds::from_interior_node(db, interior)); + interior + .if_true(db) + .find_representative_types_inner(db, current_bounds, f); + current_bounds.truncate(reset_point); // For the `if_false` branch, then the types that satisfy the current path through // the BDD do _not_ satisfy the node's constraint. Because we used `retain_one` to @@ -1378,11 +1380,9 @@ impl<'db> Node<'db> { // without updating the lower/upper bounds, relying on the other constraints along // the path to incorporate that negative "hole" in the set of valid types for this // path. - interior.if_false(db).find_representative_types_inner( - db, - Some((greatest_lower_bound, least_upper_bound)), - f, - ); + interior + .if_false(db) + .find_representative_types_inner(db, current_bounds, f); } } } @@ -1714,6 +1714,26 @@ impl<'db> Node<'db> { } } +struct RepresentativeBounds<'db> { + lower: Type<'db>, + upper: Type<'db>, + source_order: usize, +} + +impl<'db> RepresentativeBounds<'db> { + fn from_interior_node(db: &'db dyn Db, interior: InteriorNode<'db>) -> Self { + let constraint = interior.constraint(db); + let lower = constraint.lower(db); + let upper = constraint.upper(db); + let source_order = interior.source_order(db); + Self { + lower, + upper, + source_order, + } + } +} + /// An interior node of a BDD #[salsa::interned(debug, heap_size=ruff_memory_usage::heap_size)] struct InteriorNode<'db> { From 92894d3712b35bdd631be7711a21ef898bc923b0 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Sun, 14 Dec 2025 21:49:50 -0500 Subject: [PATCH 17/30] reuse self source_order --- .../src/types/constraints.rs | 63 ++++++------------- 1 file changed, 20 insertions(+), 43 deletions(-) diff --git a/crates/ty_python_semantic/src/types/constraints.rs b/crates/ty_python_semantic/src/types/constraints.rs index 1875bbeb97..4a18f83410 100644 --- a/crates/ty_python_semantic/src/types/constraints.rs +++ b/crates/ty_python_semantic/src/types/constraints.rs @@ -1282,14 +1282,11 @@ impl<'db> Node<'db> { should_remove: &mut dyn FnMut(ConstrainedTypeVar<'db>) -> bool, map: &SequentMap<'db>, path: &mut PathAssignments<'db>, - max_source_order: &mut usize, ) -> Self { match self { Node::AlwaysTrue => Node::AlwaysTrue, Node::AlwaysFalse => Node::AlwaysFalse, - Node::Interior(interior) => { - interior.abstract_one_inner(db, should_remove, map, path, max_source_order) - } + Node::Interior(interior) => interior.abstract_one_inner(db, should_remove, map, path), } } @@ -1862,7 +1859,6 @@ impl<'db> InteriorNode<'db> { fn exists_one(self, db: &'db dyn Db, bound_typevar: BoundTypeVarIdentity<'db>) -> Node<'db> { let map = self.sequent_map(db); let mut path = PathAssignments::default(); - let mut max_source_order = self.max_source_order(db); self.abstract_one_inner( db, // Remove any node that constrains `bound_typevar`, or that has a lower/upper bound of @@ -1885,7 +1881,6 @@ impl<'db> InteriorNode<'db> { }, map, &mut path, - &mut max_source_order, ) } @@ -1893,7 +1888,6 @@ impl<'db> InteriorNode<'db> { fn retain_one(self, db: &'db dyn Db, bound_typevar: BoundTypeVarIdentity<'db>) -> Node<'db> { let map = self.sequent_map(db); let mut path = PathAssignments::default(); - let mut max_source_order = self.max_source_order(db); self.abstract_one_inner( db, // Remove any node that constrains some other typevar than `bound_typevar`, and any @@ -1913,7 +1907,6 @@ impl<'db> InteriorNode<'db> { }, map, &mut path, - &mut max_source_order, ) } @@ -1923,9 +1916,7 @@ impl<'db> InteriorNode<'db> { should_remove: &mut dyn FnMut(ConstrainedTypeVar<'db>) -> bool, map: &SequentMap<'db>, path: &mut PathAssignments<'db>, - max_source_order: &mut usize, ) -> Node<'db> { - *max_source_order = (*max_source_order).max(self.max_source_order(db)); let self_constraint = self.constraint(db); if should_remove(self_constraint) { // If we should remove constraints involving this typevar, then we replace this node @@ -1934,16 +1925,18 @@ impl<'db> InteriorNode<'db> { // // We also have to check if there are any derived facts that depend on the constraint // we're about to remove. If so, we need to "remember" them by AND-ing them in with the - // corresponding branch. + // corresponding branch. We currently reuse the `source_order` of the constraint being + // removed when we add these derived facts. + // + // TODO: This might not be stable enough, if we add more than one derived fact for this + // constraint. If we still see inconsistent test output, we might need a more complex + // way of tracking source order for derived facts. + let self_source_order = self.source_order(db); let if_true = path .walk_edge(db, map, self_constraint.when_true(), |path, new_range| { - let branch = self.if_true(db).abstract_one_inner( - db, - should_remove, - map, - path, - max_source_order, - ); + let branch = self + .if_true(db) + .abstract_one_inner(db, should_remove, map, path); path.assignments[new_range] .iter() .filter(|assignment| { @@ -1952,23 +1945,18 @@ impl<'db> InteriorNode<'db> { !should_remove(assignment.constraint()) }) .fold(branch, |branch, assignment| { - *max_source_order += 1; branch.and( db, - Node::new_satisfied_constraint(db, *assignment, *max_source_order), + Node::new_satisfied_constraint(db, *assignment, self_source_order), ) }) }) .unwrap_or(Node::AlwaysFalse); let if_false = path .walk_edge(db, map, self_constraint.when_false(), |path, new_range| { - let branch = self.if_false(db).abstract_one_inner( - db, - should_remove, - map, - path, - max_source_order, - ); + let branch = self + .if_false(db) + .abstract_one_inner(db, should_remove, map, path); path.assignments[new_range] .iter() .filter(|assignment| { @@ -1977,10 +1965,9 @@ impl<'db> InteriorNode<'db> { !should_remove(assignment.constraint()) }) .fold(branch, |branch, assignment| { - *max_source_order += 1; branch.and( db, - Node::new_satisfied_constraint(db, *assignment, *max_source_order), + Node::new_satisfied_constraint(db, *assignment, self_source_order), ) }) }) @@ -1990,24 +1977,14 @@ impl<'db> InteriorNode<'db> { // Otherwise, we abstract the if_false/if_true edges recursively. let if_true = path .walk_edge(db, map, self_constraint.when_true(), |path, _| { - self.if_true(db).abstract_one_inner( - db, - should_remove, - map, - path, - max_source_order, - ) + self.if_true(db) + .abstract_one_inner(db, should_remove, map, path) }) .unwrap_or(Node::AlwaysFalse); let if_false = path .walk_edge(db, map, self_constraint.when_false(), |path, _| { - self.if_false(db).abstract_one_inner( - db, - should_remove, - map, - path, - max_source_order, - ) + self.if_false(db) + .abstract_one_inner(db, should_remove, map, path) }) .unwrap_or(Node::AlwaysFalse); // NB: We cannot use `Node::new` here, because the recursive calls might introduce new From 649c7bce582b9ffb47d5948307de8937cdcfe47d Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Sun, 14 Dec 2025 21:51:56 -0500 Subject: [PATCH 18/30] more comment --- crates/ty_python_semantic/src/types/constraints.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/ty_python_semantic/src/types/constraints.rs b/crates/ty_python_semantic/src/types/constraints.rs index 4a18f83410..5fe7584280 100644 --- a/crates/ty_python_semantic/src/types/constraints.rs +++ b/crates/ty_python_semantic/src/types/constraints.rs @@ -1326,7 +1326,9 @@ impl<'db> Node<'db> { } // If we reach the `true` terminal, the path we've been following represents one - // representative type. + // representative type. Before constructing the final lower and upper bound, sort + // the constraints by their source order. This should give us a consistently + // ordered specialization, regardless of the variable ordering of the original BDD. current_bounds.sort_unstable_by_key(|bounds| bounds.source_order); let greatest_lower_bound = UnionType::from_elements(db, current_bounds.iter().map(|bounds| bounds.lower)); From 1f34f4374531d34d47865c6499360113ee1c150e Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Sun, 14 Dec 2025 21:54:14 -0500 Subject: [PATCH 19/30] document overall approach --- crates/ty_python_semantic/src/types/constraints.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/ty_python_semantic/src/types/constraints.rs b/crates/ty_python_semantic/src/types/constraints.rs index 5fe7584280..ac70d1592e 100644 --- a/crates/ty_python_semantic/src/types/constraints.rs +++ b/crates/ty_python_semantic/src/types/constraints.rs @@ -172,6 +172,10 @@ where /// /// This is called a "set of constraint sets", and denoted _๐’ฎ_, in [[POPL2015][]]. /// +/// The underlying representation tracks the order that individual constraints appear in the +/// underlying Python source. For this to work, you should ensure that you call "combining" +/// operators like [`and`][Self::and] and [`or`][Self::or] in a consistent order. +/// /// [POPL2015]: https://doi.org/10.1145/2676726.2676991 #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, get_size2::GetSize, salsa::Update)] pub struct ConstraintSet<'db> { @@ -822,6 +826,14 @@ impl<'db> ConstrainedTypeVar<'db> { /// BDD nodes are also _ordered_, meaning that every path from the root of a BDD to a terminal node /// visits variables in the same order. [`ConstrainedTypeVar::ordering`] defines the variable /// ordering that we use for constraint set BDDs. +/// +/// In addition to this BDD variable ordering, we also track a `source_order` for each individual +/// constraint. This tracks the order in which constraints appear in the underlying Python source +/// code. This provides an ordering that is stable across multiple runs, for consistent test and +/// diagnostic output. (We cannot use this ordering as our BDD variable ordering, since we might +/// have different `source_order`s for the same constraints if they appear in multiple constraint +/// sets, but we need the BDD variable ordering to be consistent across all BDDs that we create in +/// the process.) #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, get_size2::GetSize, salsa::Update)] enum Node<'db> { AlwaysFalse, From 7e2ea8bd69f6f84f7142cf5f34fabda0049dd80c Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Mon, 15 Dec 2025 08:35:50 -0500 Subject: [PATCH 20/30] use source order in specialize_constrained too --- .../src/types/constraints.rs | 84 ++++++++++--------- 1 file changed, 44 insertions(+), 40 deletions(-) diff --git a/crates/ty_python_semantic/src/types/constraints.rs b/crates/ty_python_semantic/src/types/constraints.rs index ac70d1592e..3f63735f68 100644 --- a/crates/ty_python_semantic/src/types/constraints.rs +++ b/crates/ty_python_semantic/src/types/constraints.rs @@ -78,8 +78,8 @@ use salsa::plumbing::AsId; use crate::types::generics::{GenericContext, InferableTypeVars, Specialization}; use crate::types::visitor::{TypeCollector, TypeVisitor, walk_type_with_recursion_guard}; use crate::types::{ - BoundTypeVarIdentity, BoundTypeVarInstance, IntersectionBuilder, IntersectionType, Type, - TypeVarBoundOrConstraints, UnionBuilder, UnionType, walk_bound_type_var_type, + BoundTypeVarIdentity, BoundTypeVarInstance, IntersectionType, Type, TypeVarBoundOrConstraints, + UnionType, walk_bound_type_var_type, }; use crate::{Db, FxOrderSet}; @@ -1318,7 +1318,7 @@ impl<'db> Node<'db> { self, db: &'db dyn Db, bound_typevar: BoundTypeVarIdentity<'db>, - mut f: impl FnMut(Option<(Type<'db>, Type<'db>)>), + mut f: impl FnMut(Option<(Type<'db>, Type<'db>, usize)>), ) { self.retain_one(db, bound_typevar) .find_representative_types_inner(db, &mut Vec::default(), &mut f); @@ -1328,7 +1328,7 @@ impl<'db> Node<'db> { self, db: &'db dyn Db, current_bounds: &mut Vec>, - f: &mut dyn FnMut(Option<(Type<'db>, Type<'db>)>), + f: &mut dyn FnMut(Option<(Type<'db>, Type<'db>, usize)>), ) { match self { Node::AlwaysTrue => { @@ -1354,10 +1354,17 @@ impl<'db> Node<'db> { // process. debug_assert!(greatest_lower_bound.is_assignable_to(db, least_upper_bound)); + // SAFETY: Checked that current_bounds is non-empty above. + let minimum_source_order = current_bounds[0].source_order; + // We've been tracking the lower and upper bound that the types for this path must // satisfy. Pass those bounds along and let the caller choose a representative type // from within that range. - f(Some((greatest_lower_bound, least_upper_bound))); + f(Some(( + greatest_lower_bound, + least_upper_bound, + minimum_source_order, + ))); } Node::AlwaysFalse => { @@ -3617,6 +3624,7 @@ impl<'db> GenericContext<'db> { // Then we find all of the "representative types" for each typevar in the constraint set. let mut error_occurred = false; + let mut constraints = Vec::new(); let types = self.variables(db).map(|bound_typevar| { // Each representative type represents one of the ways that the typevar can satisfy the // constraint, expressed as a lower/upper bound on the types that the typevar can @@ -3628,10 +3636,7 @@ impl<'db> GenericContext<'db> { // (This happens most often with constrained typevars.) We could in the future turn // _each_ of the paths into separate specializations, but it's not clear what we would // do with that, so instead we just report the ambiguity as a specialization failure. - let mut satisfied = false; let mut unconstrained = false; - let mut greatest_lower_bound = UnionBuilder::new(db).order_elements(true); - let mut least_upper_bound = IntersectionBuilder::new(db).order_elements(true); let identity = bound_typevar.identity(db); tracing::trace!( target: "ty_python_semantic::types::constraints::specialize_constrained", @@ -3639,39 +3644,23 @@ impl<'db> GenericContext<'db> { abstracted = %abstracted.retain_one(db, identity).display(db), "find specialization for typevar", ); - abstracted.find_representative_types(db, identity, |bounds| { - satisfied = true; - match bounds { - Some((lower_bound, upper_bound)) => { - tracing::trace!( - target: "ty_python_semantic::types::constraints::specialize_constrained", - bound_typevar = %identity.display(db), - lower_bound = %lower_bound.display(db), - upper_bound = %upper_bound.display(db), - "found representative type", - ); - greatest_lower_bound.add_in_place(lower_bound); - least_upper_bound.add_positive_in_place(upper_bound); - } - None => { - unconstrained = true; - } + constraints.clear(); + abstracted.find_representative_types(db, identity, |bounds| match bounds { + Some(bounds @ (lower_bound, upper_bound, _)) => { + tracing::trace!( + target: "ty_python_semantic::types::constraints::specialize_constrained", + bound_typevar = %identity.display(db), + lower_bound = %lower_bound.display(db), + upper_bound = %upper_bound.display(db), + "found representative type", + ); + constraints.push(bounds); + } + None => { + unconstrained = true; } }); - // If there are no satisfiable paths in the BDD, then there is no valid specialization - // for this constraint set. - if !satisfied { - // TODO: Construct a useful error here - tracing::debug!( - target: "ty_python_semantic::types::constraints::specialize_constrained", - bound_typevar = %identity.display(db), - "typevar cannot be satisfied", - ); - error_occurred = true; - return None; - } - // The BDD is satisfiable, but the typevar is unconstrained, then we use `None` to tell // specialize_recursive to fall back on the typevar's default. if unconstrained { @@ -3683,10 +3672,25 @@ impl<'db> GenericContext<'db> { return None; } + // If there are no satisfiable paths in the BDD, then there is no valid specialization + // for this constraint set. + if constraints.is_empty() { + // TODO: Construct a useful error here + tracing::debug!( + target: "ty_python_semantic::types::constraints::specialize_constrained", + bound_typevar = %identity.display(db), + "typevar cannot be satisfied", + ); + error_occurred = true; + return None; + } + // If `lower โ‰ฐ upper`, then there is no type that satisfies all of the paths in the // BDD. That's an ambiguous specialization, as described above. - let greatest_lower_bound = greatest_lower_bound.build(); - let least_upper_bound = least_upper_bound.build(); + let greatest_lower_bound = + UnionType::from_elements(db, constraints.iter().map(|(lower, _, _)| *lower)); + let least_upper_bound = + IntersectionType::from_elements(db, constraints.iter().map(|(_, upper, _)| *upper)); if !greatest_lower_bound.is_assignable_to(db, least_upper_bound) { tracing::debug!( target: "ty_python_semantic::types::constraints::specialize_constrained", From da31e138b4b1b7862a4db19399ac07a095c24f25 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Mon, 15 Dec 2025 08:39:21 -0500 Subject: [PATCH 21/30] fix test expectation --- .../resources/mdtest/generics/specialize_constrained.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md b/crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md index 0cf656ccc3..20e04ab24b 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md @@ -316,7 +316,7 @@ def constrained_by_gradual_list_reverse[T: (list[Any], list[Base])](): # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[object]] reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.range(Never, T, list[object]))) # TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Any]] - # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Base] & list[Any]] + # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Any] & list[Base]] reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.range(Never, T, list[Any]))) # revealed: None reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.never())) From ccb03d3b23e8dd089fcbadb78e234b324dded387 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Mon, 15 Dec 2025 08:40:45 -0500 Subject: [PATCH 22/30] remove now-unused items --- crates/ty_python_semantic/src/types/builder.rs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/crates/ty_python_semantic/src/types/builder.rs b/crates/ty_python_semantic/src/types/builder.rs index ae2c21759b..c279ddbd27 100644 --- a/crates/ty_python_semantic/src/types/builder.rs +++ b/crates/ty_python_semantic/src/types/builder.rs @@ -689,20 +689,10 @@ impl<'db> IntersectionBuilder<'db> { } } - pub(crate) fn order_elements(mut self, val: bool) -> Self { - self.order_elements = val; - self - } - pub(crate) fn add_positive(self, ty: Type<'db>) -> Self { self.add_positive_impl(ty, &mut vec![]) } - pub(crate) fn add_positive_in_place(&mut self, ty: Type<'db>) { - let updated = std::mem::replace(self, Self::empty(self.db)).add_positive(ty); - *self = updated; - } - pub(crate) fn add_positive_impl( mut self, ty: Type<'db>, From 019db2a22ed4d1293be1e28e0ee80e0d046c128f Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Mon, 15 Dec 2025 08:52:44 -0500 Subject: [PATCH 23/30] more comments --- .../src/types/constraints.rs | 180 +++++++++++------- 1 file changed, 114 insertions(+), 66 deletions(-) diff --git a/crates/ty_python_semantic/src/types/constraints.rs b/crates/ty_python_semantic/src/types/constraints.rs index 3f63735f68..edc40c3a22 100644 --- a/crates/ty_python_semantic/src/types/constraints.rs +++ b/crates/ty_python_semantic/src/types/constraints.rs @@ -172,9 +172,10 @@ where /// /// This is called a "set of constraint sets", and denoted _๐’ฎ_, in [[POPL2015][]]. /// -/// The underlying representation tracks the order that individual constraints appear in the -/// underlying Python source. For this to work, you should ensure that you call "combining" -/// operators like [`and`][Self::and] and [`or`][Self::or] in a consistent order. +/// The underlying representation tracks the order that individual constraints are added to the +/// constraint set, which typically tracks when they appear in the underlying Python source. For +/// this to work, you should ensure that you call "combining" operators like [`and`][Self::and] and +/// [`or`][Self::or] in a consistent order. /// /// [POPL2015]: https://doi.org/10.1145/2676726.2676991 #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, get_size2::GetSize, salsa::Update)] @@ -349,12 +350,18 @@ impl<'db> ConstraintSet<'db> { } /// Updates this constraint set to hold the union of itself and another constraint set. + /// + /// In the result, `self` will appear before `other` according to the `source_order` of the BDD + /// nodes. pub(crate) fn union(&mut self, db: &'db dyn Db, other: Self) -> Self { self.node = self.node.or(db, other.node); *self } /// Updates this constraint set to hold the intersection of itself and another constraint set. + /// + /// In the result, `self` will appear before `other` according to the `source_order` of the BDD + /// nodes. pub(crate) fn intersect(&mut self, db: &'db dyn Db, other: Self) -> Self { self.node = self.node.and(db, other.node); *self @@ -370,6 +377,9 @@ impl<'db> ConstraintSet<'db> { /// Returns the intersection of this constraint set and another. The other constraint set is /// provided as a thunk, to implement short-circuiting: the thunk is not forced if the /// constraint set is already saturated. + /// + /// In the result, `self` will appear before `other` according to the `source_order` of the BDD + /// nodes. pub(crate) fn and(mut self, db: &'db dyn Db, other: impl FnOnce() -> Self) -> Self { if !self.is_never_satisfied(db) { self.intersect(db, other()); @@ -380,6 +390,9 @@ impl<'db> ConstraintSet<'db> { /// Returns the union of this constraint set and another. The other constraint set is provided /// as a thunk, to implement short-circuiting: the thunk is not forced if the constraint set is /// already saturated. + /// + /// In the result, `self` will appear before `other` according to the `source_order` of the BDD + /// nodes. pub(crate) fn or(mut self, db: &'db dyn Db, other: impl FnOnce() -> Self) -> Self { if !self.is_always_satisfied(db) { self.union(db, other()); @@ -388,10 +401,17 @@ impl<'db> ConstraintSet<'db> { } /// Returns a constraint set encoding that this constraint set implies another. + /// + /// In the result, `self` will appear before `other` according to the `source_order` of the BDD + /// nodes. pub(crate) fn implies(self, db: &'db dyn Db, other: impl FnOnce() -> Self) -> Self { self.negate(db).or(db, other) } + /// Returns a constraint set encoding that this constraint set is equivalent to another. + /// + /// In the result, `self` will appear before `other` according to the `source_order` of the BDD + /// nodes. pub(crate) fn iff(self, db: &'db dyn Db, other: Self) -> Self { ConstraintSet { node: self.node.iff(db, other.node), @@ -828,12 +848,12 @@ impl<'db> ConstrainedTypeVar<'db> { /// ordering that we use for constraint set BDDs. /// /// In addition to this BDD variable ordering, we also track a `source_order` for each individual -/// constraint. This tracks the order in which constraints appear in the underlying Python source -/// code. This provides an ordering that is stable across multiple runs, for consistent test and -/// diagnostic output. (We cannot use this ordering as our BDD variable ordering, since we might -/// have different `source_order`s for the same constraints if they appear in multiple constraint -/// sets, but we need the BDD variable ordering to be consistent across all BDDs that we create in -/// the process.) +/// constraint. This records the order in which constraints are added to the constraint set, which +/// typically tracks when they appear in the underlying Python source code. This provides an +/// ordering that is stable across multiple runs, for consistent test and diagnostic output. (We +/// cannot use this ordering as our BDD variable ordering, since we calculate it from already +/// constructed BDDs, and we need the BDD variable ordering to be fixed and available before +/// construction starts.) #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, get_size2::GetSize, salsa::Update)] enum Node<'db> { AlwaysFalse, @@ -1059,60 +1079,74 @@ impl<'db> Node<'db> { } /// Returns the `or` or union of two BDDs. + /// + /// In the result, `self` will appear before `other` according to the `source_order` of the BDD + /// nodes. fn or(self, db: &'db dyn Db, other: Self) -> Self { - let rhs_offset = self.max_source_order(db); - self.or_inner(db, other, rhs_offset) + // To ensure that `self` appears before `other` in `source_order`, we add the maximum + // `source_order` of the lhs to all of the `source_order`s in the rhs. + let other_offset = self.max_source_order(db); + self.or_inner(db, other, other_offset) } - fn or_inner(self, db: &'db dyn Db, other: Self, rhs_offset: usize) -> Self { + fn or_inner(self, db: &'db dyn Db, other: Self, other_offset: usize) -> Self { match (self, other) { (Node::AlwaysTrue, Node::AlwaysTrue) => Node::AlwaysTrue, - (Node::AlwaysTrue, Node::Interior(rhs)) => Node::new( + (Node::AlwaysTrue, Node::Interior(other_interior)) => Node::new( db, - rhs.constraint(db), + other_interior.constraint(db), Node::AlwaysTrue, Node::AlwaysTrue, - rhs.source_order(db) + rhs_offset, + other_interior.source_order(db) + other_offset, ), - (Node::Interior(lhs), Node::AlwaysTrue) => Node::new( + (Node::Interior(self_interior), Node::AlwaysTrue) => Node::new( db, - lhs.constraint(db), + self_interior.constraint(db), Node::AlwaysTrue, Node::AlwaysTrue, - lhs.source_order(db), + self_interior.source_order(db), ), - (Node::AlwaysFalse, rhs) => rhs.with_adjusted_source_order(db, rhs_offset), - (lhs, Node::AlwaysFalse) => lhs, - (Node::Interior(lhs), Node::Interior(rhs)) => lhs.or(db, rhs, rhs_offset), + (Node::AlwaysFalse, _) => other.with_adjusted_source_order(db, other_offset), + (_, Node::AlwaysFalse) => self, + (Node::Interior(self_interior), Node::Interior(other_interior)) => { + self_interior.or(db, other_interior, other_offset) + } } } /// Returns the `and` or intersection of two BDDs. + /// + /// In the result, `self` will appear before `other` according to the `source_order` of the BDD + /// nodes. fn and(self, db: &'db dyn Db, other: Self) -> Self { - let rhs_offset = self.max_source_order(db); - self.and_inner(db, other, rhs_offset) + // To ensure that `self` appears before `other` in `source_order`, we add the maximum + // `source_order` of the lhs to all of the `source_order`s in the rhs. + let other_offset = self.max_source_order(db); + self.and_inner(db, other, other_offset) } - fn and_inner(self, db: &'db dyn Db, other: Self, rhs_offset: usize) -> Self { + fn and_inner(self, db: &'db dyn Db, other: Self, other_offset: usize) -> Self { match (self, other) { (Node::AlwaysFalse, Node::AlwaysFalse) => Node::AlwaysFalse, - (Node::AlwaysFalse, Node::Interior(rhs)) => Node::new( + (Node::AlwaysFalse, Node::Interior(other_interior)) => Node::new( db, - rhs.constraint(db), + other_interior.constraint(db), Node::AlwaysFalse, Node::AlwaysFalse, - rhs.source_order(db) + rhs_offset, + other_interior.source_order(db) + other_offset, ), - (Node::Interior(lhs), Node::AlwaysFalse) => Node::new( + (Node::Interior(self_interior), Node::AlwaysFalse) => Node::new( db, - lhs.constraint(db), + self_interior.constraint(db), Node::AlwaysFalse, Node::AlwaysFalse, - lhs.source_order(db), + self_interior.source_order(db), ), - (Node::AlwaysTrue, rhs) => rhs.with_adjusted_source_order(db, rhs_offset), - (lhs, Node::AlwaysTrue) => lhs, - (Node::Interior(lhs), Node::Interior(rhs)) => lhs.and(db, rhs, rhs_offset), + (Node::AlwaysTrue, _) => other.with_adjusted_source_order(db, other_offset), + (_, Node::AlwaysTrue) => self, + (Node::Interior(self_interior), Node::Interior(other_interior)) => { + self_interior.and(db, other_interior, other_offset) + } } } @@ -1123,12 +1157,17 @@ impl<'db> Node<'db> { /// Returns a new BDD that evaluates to `true` when both input BDDs evaluate to the same /// result. + /// + /// In the result, `self` will appear before `other` according to the `source_order` of the BDD + /// nodes. fn iff(self, db: &'db dyn Db, other: Self) -> Self { - let rhs_offset = self.max_source_order(db); - self.iff_inner(db, other, rhs_offset) + // To ensure that `self` appears before `other` in `source_order`, we add the maximum + // `source_order` of the lhs to all of the `source_order`s in the rhs. + let other_offset = self.max_source_order(db); + self.iff_inner(db, other, other_offset) } - fn iff_inner(self, db: &'db dyn Db, other: Self, rhs_offset: usize) -> Self { + fn iff_inner(self, db: &'db dyn Db, other: Self, other_offset: usize) -> Self { match (self, other) { (Node::AlwaysFalse, Node::AlwaysFalse) | (Node::AlwaysTrue, Node::AlwaysTrue) => { Node::AlwaysTrue @@ -1139,18 +1178,18 @@ impl<'db> Node<'db> { (Node::AlwaysTrue | Node::AlwaysFalse, Node::Interior(interior)) => Node::new( db, interior.constraint(db), - self.iff_inner(db, interior.if_true(db), rhs_offset), - self.iff_inner(db, interior.if_false(db), rhs_offset), - interior.source_order(db) + rhs_offset, + self.iff_inner(db, interior.if_true(db), other_offset), + self.iff_inner(db, interior.if_false(db), other_offset), + interior.source_order(db) + other_offset, ), (Node::Interior(interior), Node::AlwaysTrue | Node::AlwaysFalse) => Node::new( db, interior.constraint(db), - interior.if_true(db).iff_inner(db, other, rhs_offset), - interior.if_false(db).iff_inner(db, other, rhs_offset), + interior.if_true(db).iff_inner(db, other, other_offset), + interior.if_false(db).iff_inner(db, other, other_offset), interior.source_order(db), ), - (Node::Interior(a), Node::Interior(b)) => a.iff(db, b, rhs_offset), + (Node::Interior(a), Node::Interior(b)) => a.iff(db, b, other_offset), } } @@ -1758,7 +1797,15 @@ struct InteriorNode<'db> { constraint: ConstrainedTypeVar<'db>, if_true: Node<'db>, if_false: Node<'db>, + + /// Represents the order in which this node's constraint was added to the containing constraint + /// set, relative to all of the other constraints in the set. This starts off at 1 for a simple + /// single-constraint set (e.g. created with [`Node::new_constraint`] or + /// [`Node::new_satisfied_constraint`]). It will get incremented, if needed, as that simple BDD + /// is combined into larger BDDs. source_order: usize, + + /// The maximum `source_order` across this node and all of its descendants. max_source_order: usize, } @@ -1779,39 +1826,40 @@ impl<'db> InteriorNode<'db> { } #[salsa::tracked(heap_size=ruff_memory_usage::heap_size)] - fn or(self, db: &'db dyn Db, other: Self, rhs_offset: usize) -> Node<'db> { + fn or(self, db: &'db dyn Db, other: Self, other_offset: usize) -> Node<'db> { let self_constraint = self.constraint(db); let other_constraint = other.constraint(db); match (self_constraint.ordering(db)).cmp(&other_constraint.ordering(db)) { Ordering::Equal => Node::new( db, self_constraint, - self.if_true(db).or_inner(db, other.if_true(db), rhs_offset), + self.if_true(db) + .or_inner(db, other.if_true(db), other_offset), self.if_false(db) - .or_inner(db, other.if_false(db), rhs_offset), + .or_inner(db, other.if_false(db), other_offset), self.source_order(db), ), Ordering::Less => Node::new( db, self_constraint, self.if_true(db) - .or_inner(db, Node::Interior(other), rhs_offset), + .or_inner(db, Node::Interior(other), other_offset), self.if_false(db) - .or_inner(db, Node::Interior(other), rhs_offset), + .or_inner(db, Node::Interior(other), other_offset), self.source_order(db), ), Ordering::Greater => Node::new( db, other_constraint, - Node::Interior(self).or_inner(db, other.if_true(db), rhs_offset), - Node::Interior(self).or_inner(db, other.if_false(db), rhs_offset), - other.source_order(db) + rhs_offset, + Node::Interior(self).or_inner(db, other.if_true(db), other_offset), + Node::Interior(self).or_inner(db, other.if_false(db), other_offset), + other.source_order(db) + other_offset, ), } } #[salsa::tracked(heap_size=ruff_memory_usage::heap_size)] - fn and(self, db: &'db dyn Db, other: Self, rhs_offset: usize) -> Node<'db> { + fn and(self, db: &'db dyn Db, other: Self, other_offset: usize) -> Node<'db> { let self_constraint = self.constraint(db); let other_constraint = other.constraint(db); match (self_constraint.ordering(db)).cmp(&other_constraint.ordering(db)) { @@ -1819,32 +1867,32 @@ impl<'db> InteriorNode<'db> { db, self_constraint, self.if_true(db) - .and_inner(db, other.if_true(db), rhs_offset), + .and_inner(db, other.if_true(db), other_offset), self.if_false(db) - .and_inner(db, other.if_false(db), rhs_offset), + .and_inner(db, other.if_false(db), other_offset), self.source_order(db), ), Ordering::Less => Node::new( db, self_constraint, self.if_true(db) - .and_inner(db, Node::Interior(other), rhs_offset), + .and_inner(db, Node::Interior(other), other_offset), self.if_false(db) - .and_inner(db, Node::Interior(other), rhs_offset), + .and_inner(db, Node::Interior(other), other_offset), self.source_order(db), ), Ordering::Greater => Node::new( db, other_constraint, - Node::Interior(self).and_inner(db, other.if_true(db), rhs_offset), - Node::Interior(self).and_inner(db, other.if_false(db), rhs_offset), - other.source_order(db) + rhs_offset, + Node::Interior(self).and_inner(db, other.if_true(db), other_offset), + Node::Interior(self).and_inner(db, other.if_false(db), other_offset), + other.source_order(db) + other_offset, ), } } #[salsa::tracked(heap_size=ruff_memory_usage::heap_size)] - fn iff(self, db: &'db dyn Db, other: Self, rhs_offset: usize) -> Node<'db> { + fn iff(self, db: &'db dyn Db, other: Self, other_offset: usize) -> Node<'db> { let self_constraint = self.constraint(db); let other_constraint = other.constraint(db); match (self_constraint.ordering(db)).cmp(&other_constraint.ordering(db)) { @@ -1852,26 +1900,26 @@ impl<'db> InteriorNode<'db> { db, self_constraint, self.if_true(db) - .iff_inner(db, other.if_true(db), rhs_offset), + .iff_inner(db, other.if_true(db), other_offset), self.if_false(db) - .iff_inner(db, other.if_false(db), rhs_offset), + .iff_inner(db, other.if_false(db), other_offset), self.source_order(db), ), Ordering::Less => Node::new( db, self_constraint, self.if_true(db) - .iff_inner(db, Node::Interior(other), rhs_offset), + .iff_inner(db, Node::Interior(other), other_offset), self.if_false(db) - .iff_inner(db, Node::Interior(other), rhs_offset), + .iff_inner(db, Node::Interior(other), other_offset), self.source_order(db), ), Ordering::Greater => Node::new( db, other_constraint, - Node::Interior(self).iff_inner(db, other.if_true(db), rhs_offset), - Node::Interior(self).iff_inner(db, other.if_false(db), rhs_offset), - other.source_order(db) + rhs_offset, + Node::Interior(self).iff_inner(db, other.if_true(db), other_offset), + Node::Interior(self).iff_inner(db, other.if_false(db), other_offset), + other.source_order(db) + other_offset, ), } } From 358185b5e2b62fa1f6715564621fe48b74aa7069 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Mon, 15 Dec 2025 09:09:41 -0500 Subject: [PATCH 24/30] document display source_order --- crates/ty_python_semantic/src/types/constraints.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/ty_python_semantic/src/types/constraints.rs b/crates/ty_python_semantic/src/types/constraints.rs index edc40c3a22..f083f142a1 100644 --- a/crates/ty_python_semantic/src/types/constraints.rs +++ b/crates/ty_python_semantic/src/types/constraints.rs @@ -2153,7 +2153,10 @@ impl<'db> InteriorNode<'db> { .collect(); // Repeatedly pop constraint pairs off of the visit queue, checking whether each pair can - // be simplified. + // be simplified. If we add any derived constraints, we will place them at the end in + // source order. (We do not have any test cases that depend on constraint sets being + // displayed in a consistent ordering, so we don't need to be clever in assigning these + // `source_order`s.) let mut simplified = Node::Interior(self); let mut next_source_order = self.max_source_order(db) + 1; while let Some((left_constraint, right_constraint)) = to_visit.pop() { @@ -2358,7 +2361,6 @@ impl<'db> InteriorNode<'db> { intersection_constraint.when_true(), next_source_order, ); - next_source_order += 1; let negative_intersection_node = Node::new_satisfied_constraint( db, intersection_constraint.when_false(), From 63c75d85d0777876d1eff2c7b5498c771a46ef61 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Mon, 15 Dec 2025 09:55:17 -0500 Subject: [PATCH 25/30] only fold once --- .../src/types/constraints.rs | 212 +++++++++--------- 1 file changed, 105 insertions(+), 107 deletions(-) diff --git a/crates/ty_python_semantic/src/types/constraints.rs b/crates/ty_python_semantic/src/types/constraints.rs index f083f142a1..b199ded6bf 100644 --- a/crates/ty_python_semantic/src/types/constraints.rs +++ b/crates/ty_python_semantic/src/types/constraints.rs @@ -1357,7 +1357,7 @@ impl<'db> Node<'db> { self, db: &'db dyn Db, bound_typevar: BoundTypeVarIdentity<'db>, - mut f: impl FnMut(Option<(Type<'db>, Type<'db>, usize)>), + mut f: impl FnMut(Option<&[RepresentativeBounds<'db>]>), ) { self.retain_one(db, bound_typevar) .find_representative_types_inner(db, &mut Vec::default(), &mut f); @@ -1367,43 +1367,37 @@ impl<'db> Node<'db> { self, db: &'db dyn Db, current_bounds: &mut Vec>, - f: &mut dyn FnMut(Option<(Type<'db>, Type<'db>, usize)>), + f: &mut dyn FnMut(Option<&[RepresentativeBounds<'db>]>), ) { match self { Node::AlwaysTrue => { + // If we reach the `true` terminal, the path we've been following represents one + // representative type. if current_bounds.is_empty() { f(None); return; } - // If we reach the `true` terminal, the path we've been following represents one - // representative type. Before constructing the final lower and upper bound, sort - // the constraints by their source order. This should give us a consistently - // ordered specialization, regardless of the variable ordering of the original BDD. - current_bounds.sort_unstable_by_key(|bounds| bounds.source_order); - let greatest_lower_bound = - UnionType::from_elements(db, current_bounds.iter().map(|bounds| bounds.lower)); - let least_upper_bound = IntersectionType::from_elements( - db, - current_bounds.iter().map(|bounds| bounds.upper), - ); - // If `lower โ‰ฐ upper`, then this path somehow represents in invalid specialization. // That should have been removed from the BDD domain as part of the simplification - // process. - debug_assert!(greatest_lower_bound.is_assignable_to(db, least_upper_bound)); - - // SAFETY: Checked that current_bounds is non-empty above. - let minimum_source_order = current_bounds[0].source_order; + // process. (Here we are just checking assignability, so we don't need to construct + // the lower and upper bounds in a consistent order.) + debug_assert!({ + let greatest_lower_bound = UnionType::from_elements( + db, + current_bounds.iter().map(|bounds| bounds.lower), + ); + let least_upper_bound = IntersectionType::from_elements( + db, + current_bounds.iter().map(|bounds| bounds.upper), + ); + greatest_lower_bound.is_assignable_to(db, least_upper_bound) + }); // We've been tracking the lower and upper bound that the types for this path must // satisfy. Pass those bounds along and let the caller choose a representative type // from within that range. - f(Some(( - greatest_lower_bound, - least_upper_bound, - minimum_source_order, - ))); + f(Some(¤t_bounds)); } Node::AlwaysFalse => { @@ -1771,6 +1765,7 @@ impl<'db> Node<'db> { } } +#[derive(Clone, Copy, Debug)] struct RepresentativeBounds<'db> { lower: Type<'db>, upper: Type<'db>, @@ -3674,96 +3669,99 @@ impl<'db> GenericContext<'db> { // Then we find all of the "representative types" for each typevar in the constraint set. let mut error_occurred = false; - let mut constraints = Vec::new(); - let types = self.variables(db).map(|bound_typevar| { - // Each representative type represents one of the ways that the typevar can satisfy the - // constraint, expressed as a lower/upper bound on the types that the typevar can - // specialize to. - // - // If there are multiple paths in the BDD, they technically represent independent - // possible specializations. If there's a type that satisfies all of them, we will - // return that as the specialization. If not, then the constraint set is ambiguous. - // (This happens most often with constrained typevars.) We could in the future turn - // _each_ of the paths into separate specializations, but it's not clear what we would - // do with that, so instead we just report the ambiguity as a specialization failure. - let mut unconstrained = false; - let identity = bound_typevar.identity(db); - tracing::trace!( - target: "ty_python_semantic::types::constraints::specialize_constrained", - bound_typevar = %identity.display(db), - abstracted = %abstracted.retain_one(db, identity).display(db), - "find specialization for typevar", - ); - constraints.clear(); - abstracted.find_representative_types(db, identity, |bounds| match bounds { - Some(bounds @ (lower_bound, upper_bound, _)) => { - tracing::trace!( + let mut representatives = Vec::new(); + let types = + self.variables(db).map(|bound_typevar| { + // Each representative type represents one of the ways that the typevar can satisfy the + // constraint, expressed as a lower/upper bound on the types that the typevar can + // specialize to. + // + // If there are multiple paths in the BDD, they technically represent independent + // possible specializations. If there's a type that satisfies all of them, we will + // return that as the specialization. If not, then the constraint set is ambiguous. + // (This happens most often with constrained typevars.) We could in the future turn + // _each_ of the paths into separate specializations, but it's not clear what we would + // do with that, so instead we just report the ambiguity as a specialization failure. + let mut unconstrained = false; + let identity = bound_typevar.identity(db); + tracing::trace!( + target: "ty_python_semantic::types::constraints::specialize_constrained", + bound_typevar = %identity.display(db), + abstracted = %abstracted.retain_one(db, identity).display(db), + "find specialization for typevar", + ); + representatives.clear(); + abstracted.find_representative_types(db, identity, |representative| { + match representative { + Some(representative) => { + representatives.extend_from_slice(representative); + } + None => { + unconstrained = true; + } + } + }); + + // The BDD is satisfiable, but the typevar is unconstrained, then we use `None` to tell + // specialize_recursive to fall back on the typevar's default. + if unconstrained { + tracing::debug!( target: "ty_python_semantic::types::constraints::specialize_constrained", bound_typevar = %identity.display(db), - lower_bound = %lower_bound.display(db), - upper_bound = %upper_bound.display(db), - "found representative type", + "typevar is unconstrained", ); - constraints.push(bounds); + return None; } - None => { - unconstrained = true; + + // If there are no satisfiable paths in the BDD, then there is no valid specialization + // for this constraint set. + if representatives.is_empty() { + // TODO: Construct a useful error here + tracing::debug!( + target: "ty_python_semantic::types::constraints::specialize_constrained", + bound_typevar = %identity.display(db), + "typevar cannot be satisfied", + ); + error_occurred = true; + return None; } + + // Before constructing the final lower and upper bound, sort the constraints by + // their source order. This should give us a consistently ordered specialization, + // regardless of the variable ordering of the original BDD. + representatives.sort_unstable_by_key(|bounds| bounds.source_order); + let greatest_lower_bound = + UnionType::from_elements(db, representatives.iter().map(|bounds| bounds.lower)); + let least_upper_bound = IntersectionType::from_elements( + db, + representatives.iter().map(|bounds| bounds.upper), + ); + + // If `lower โ‰ฐ upper`, then there is no type that satisfies all of the paths in the + // BDD. That's an ambiguous specialization, as described above. + if !greatest_lower_bound.is_assignable_to(db, least_upper_bound) { + tracing::debug!( + target: "ty_python_semantic::types::constraints::specialize_constrained", + bound_typevar = %identity.display(db), + greatest_lower_bound = %greatest_lower_bound.display(db), + least_upper_bound = %least_upper_bound.display(db), + "typevar bounds are incompatible", + ); + error_occurred = true; + return None; + } + + // Of all of the types that satisfy all of the paths in the BDD, we choose the + // "largest" one (i.e., "closest to `object`") as the specialization. + tracing::debug!( + target: "ty_python_semantic::types::constraints::specialize_constrained", + bound_typevar = %identity.display(db), + specialization = %least_upper_bound.display(db), + "found specialization for typevar", + ); + Some(least_upper_bound) }); - // The BDD is satisfiable, but the typevar is unconstrained, then we use `None` to tell - // specialize_recursive to fall back on the typevar's default. - if unconstrained { - tracing::debug!( - target: "ty_python_semantic::types::constraints::specialize_constrained", - bound_typevar = %identity.display(db), - "typevar is unconstrained", - ); - return None; - } - - // If there are no satisfiable paths in the BDD, then there is no valid specialization - // for this constraint set. - if constraints.is_empty() { - // TODO: Construct a useful error here - tracing::debug!( - target: "ty_python_semantic::types::constraints::specialize_constrained", - bound_typevar = %identity.display(db), - "typevar cannot be satisfied", - ); - error_occurred = true; - return None; - } - - // If `lower โ‰ฐ upper`, then there is no type that satisfies all of the paths in the - // BDD. That's an ambiguous specialization, as described above. - let greatest_lower_bound = - UnionType::from_elements(db, constraints.iter().map(|(lower, _, _)| *lower)); - let least_upper_bound = - IntersectionType::from_elements(db, constraints.iter().map(|(_, upper, _)| *upper)); - if !greatest_lower_bound.is_assignable_to(db, least_upper_bound) { - tracing::debug!( - target: "ty_python_semantic::types::constraints::specialize_constrained", - bound_typevar = %identity.display(db), - greatest_lower_bound = %greatest_lower_bound.display(db), - least_upper_bound = %least_upper_bound.display(db), - "typevar bounds are incompatible", - ); - error_occurred = true; - return None; - } - - // Of all of the types that satisfy all of the paths in the BDD, we choose the - // "largest" one (i.e., "closest to `object`") as the specialization. - tracing::debug!( - target: "ty_python_semantic::types::constraints::specialize_constrained", - bound_typevar = %identity.display(db), - specialization = %least_upper_bound.display(db), - "found specialization for typevar", - ); - Some(least_upper_bound) - }); - let specialization = self.specialize_recursive(db, types); if error_occurred { return Err(()); From 88eb5eba22bb641ccf94628231be786f419d0390 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Mon, 15 Dec 2025 10:05:49 -0500 Subject: [PATCH 26/30] don't always bump --- .../src/types/constraints.rs | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/crates/ty_python_semantic/src/types/constraints.rs b/crates/ty_python_semantic/src/types/constraints.rs index b199ded6bf..541b351c17 100644 --- a/crates/ty_python_semantic/src/types/constraints.rs +++ b/crates/ty_python_semantic/src/types/constraints.rs @@ -354,7 +354,7 @@ impl<'db> ConstraintSet<'db> { /// In the result, `self` will appear before `other` according to the `source_order` of the BDD /// nodes. pub(crate) fn union(&mut self, db: &'db dyn Db, other: Self) -> Self { - self.node = self.node.or(db, other.node); + self.node = self.node.or_with_offset(db, other.node); *self } @@ -363,7 +363,7 @@ impl<'db> ConstraintSet<'db> { /// In the result, `self` will appear before `other` according to the `source_order` of the BDD /// nodes. pub(crate) fn intersect(&mut self, db: &'db dyn Db, other: Self) -> Self { - self.node = self.node.and(db, other.node); + self.node = self.node.and_with_offset(db, other.node); *self } @@ -414,7 +414,7 @@ impl<'db> ConstraintSet<'db> { /// nodes. pub(crate) fn iff(self, db: &'db dyn Db, other: Self) -> Self { ConstraintSet { - node: self.node.iff(db, other.node), + node: self.node.iff_with_offset(db, other.node), } } @@ -1082,13 +1082,17 @@ impl<'db> Node<'db> { /// /// In the result, `self` will appear before `other` according to the `source_order` of the BDD /// nodes. - fn or(self, db: &'db dyn Db, other: Self) -> Self { + fn or_with_offset(self, db: &'db dyn Db, other: Self) -> Self { // To ensure that `self` appears before `other` in `source_order`, we add the maximum // `source_order` of the lhs to all of the `source_order`s in the rhs. let other_offset = self.max_source_order(db); self.or_inner(db, other, other_offset) } + fn or(self, db: &'db dyn Db, other: Self) -> Self { + self.or_inner(db, other, 0) + } + fn or_inner(self, db: &'db dyn Db, other: Self, other_offset: usize) -> Self { match (self, other) { (Node::AlwaysTrue, Node::AlwaysTrue) => Node::AlwaysTrue, @@ -1118,13 +1122,17 @@ impl<'db> Node<'db> { /// /// In the result, `self` will appear before `other` according to the `source_order` of the BDD /// nodes. - fn and(self, db: &'db dyn Db, other: Self) -> Self { + fn and_with_offset(self, db: &'db dyn Db, other: Self) -> Self { // To ensure that `self` appears before `other` in `source_order`, we add the maximum // `source_order` of the lhs to all of the `source_order`s in the rhs. let other_offset = self.max_source_order(db); self.and_inner(db, other, other_offset) } + fn and(self, db: &'db dyn Db, other: Self) -> Self { + self.and_inner(db, other, 0) + } + fn and_inner(self, db: &'db dyn Db, other: Self, other_offset: usize) -> Self { match (self, other) { (Node::AlwaysFalse, Node::AlwaysFalse) => Node::AlwaysFalse, @@ -1160,13 +1168,17 @@ impl<'db> Node<'db> { /// /// In the result, `self` will appear before `other` according to the `source_order` of the BDD /// nodes. - fn iff(self, db: &'db dyn Db, other: Self) -> Self { + fn iff_with_offset(self, db: &'db dyn Db, other: Self) -> Self { // To ensure that `self` appears before `other` in `source_order`, we add the maximum // `source_order` of the lhs to all of the `source_order`s in the rhs. let other_offset = self.max_source_order(db); self.iff_inner(db, other, other_offset) } + fn iff(self, db: &'db dyn Db, other: Self) -> Self { + self.iff_inner(db, other, 0) + } + fn iff_inner(self, db: &'db dyn Db, other: Self, other_offset: usize) -> Self { match (self, other) { (Node::AlwaysFalse, Node::AlwaysFalse) | (Node::AlwaysTrue, Node::AlwaysTrue) => { @@ -3568,7 +3580,7 @@ impl<'db> BoundTypeVarInstance<'db> { for constraint in constraints.elements(db) { let constraint_lower = constraint.bottom_materialization(db); let constraint_upper = constraint.top_materialization(db); - specializations = specializations.or( + specializations = specializations.or_with_offset( db, ConstrainedTypeVar::new_node(db, self, constraint_lower, constraint_upper), ); @@ -3618,7 +3630,8 @@ impl<'db> BoundTypeVarInstance<'db> { let constraint = ConstrainedTypeVar::new_node(db, self, constraint_lower, constraint_upper); if constraint_lower == constraint_upper { - non_gradual_constraints = non_gradual_constraints.or(db, constraint); + non_gradual_constraints = + non_gradual_constraints.or_with_offset(db, constraint); } else { gradual_constraints.push(constraint); } @@ -3659,7 +3672,7 @@ impl<'db> GenericContext<'db> { let abstracted = self .variables(db) .fold(constraints.node, |constraints, bound_typevar| { - constraints.and(db, bound_typevar.valid_specializations(db)) + constraints.and_with_offset(db, bound_typevar.valid_specializations(db)) }); tracing::debug!( target: "ty_python_semantic::types::constraints::specialize_constrained", From 7f4893d2006288288a73a54996389fd0a16e22cc Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Mon, 15 Dec 2025 09:15:22 -0500 Subject: [PATCH 27/30] place bounds/constraints first --- crates/ty_python_semantic/src/types/constraints.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/ty_python_semantic/src/types/constraints.rs b/crates/ty_python_semantic/src/types/constraints.rs index 541b351c17..85a18994c1 100644 --- a/crates/ty_python_semantic/src/types/constraints.rs +++ b/crates/ty_python_semantic/src/types/constraints.rs @@ -3671,9 +3671,10 @@ impl<'db> GenericContext<'db> { // each typevar. let abstracted = self .variables(db) - .fold(constraints.node, |constraints, bound_typevar| { + .fold(Node::AlwaysTrue, |constraints, bound_typevar| { constraints.and_with_offset(db, bound_typevar.valid_specializations(db)) - }); + }) + .and_with_offset(db, constraints.node); tracing::debug!( target: "ty_python_semantic::types::constraints::specialize_constrained", valid = %abstracted.display(db), From d9429754b9f26743a3767067a40eb128b6098f50 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Mon, 15 Dec 2025 10:16:40 -0500 Subject: [PATCH 28/30] include source_order in display_graph output --- .../src/types/constraints.rs | 36 +++++++++++-------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/crates/ty_python_semantic/src/types/constraints.rs b/crates/ty_python_semantic/src/types/constraints.rs index 85a18994c1..0fbd9fc7db 100644 --- a/crates/ty_python_semantic/src/types/constraints.rs +++ b/crates/ty_python_semantic/src/types/constraints.rs @@ -1744,7 +1744,13 @@ impl<'db> Node<'db> { Node::AlwaysTrue => write!(f, "always"), Node::AlwaysFalse => write!(f, "never"), Node::Interior(interior) => { - interior.constraint(self.db).display(self.db).fmt(f)?; + write!( + f, + "{} {}/{}", + interior.constraint(self.db).display(self.db), + interior.source_order(self.db), + interior.max_source_order(self.db), + )?; // Calling display_graph recursively here causes rustc to claim that the // expect(unused) up above is unfulfilled! write!( @@ -3797,28 +3803,28 @@ mod tests { #[test] fn test_display_graph_output() { let expected = indoc! {r#" - (T = str) - โ”กโ”โ‚ (T = bool) - โ”‚ โ”กโ”โ‚ (U = str) - โ”‚ โ”‚ โ”กโ”โ‚ (U = bool) + (T = str) 3/4 + โ”กโ”โ‚ (T = bool) 4/4 + โ”‚ โ”กโ”โ‚ (U = str) 1/2 + โ”‚ โ”‚ โ”กโ”โ‚ (U = bool) 2/2 โ”‚ โ”‚ โ”‚ โ”กโ”โ‚ always โ”‚ โ”‚ โ”‚ โ””โ”€โ‚€ always - โ”‚ โ”‚ โ””โ”€โ‚€ (U = bool) + โ”‚ โ”‚ โ””โ”€โ‚€ (U = bool) 2/2 โ”‚ โ”‚ โ”กโ”โ‚ always โ”‚ โ”‚ โ””โ”€โ‚€ never - โ”‚ โ””โ”€โ‚€ (U = str) - โ”‚ โ”กโ”โ‚ (U = bool) + โ”‚ โ””โ”€โ‚€ (U = str) 1/2 + โ”‚ โ”กโ”โ‚ (U = bool) 2/2 โ”‚ โ”‚ โ”กโ”โ‚ always โ”‚ โ”‚ โ””โ”€โ‚€ always - โ”‚ โ””โ”€โ‚€ (U = bool) + โ”‚ โ””โ”€โ‚€ (U = bool) 2/2 โ”‚ โ”กโ”โ‚ always โ”‚ โ””โ”€โ‚€ never - โ””โ”€โ‚€ (T = bool) - โ”กโ”โ‚ (U = str) - โ”‚ โ”กโ”โ‚ (U = bool) + โ””โ”€โ‚€ (T = bool) 4/4 + โ”กโ”โ‚ (U = str) 1/2 + โ”‚ โ”กโ”โ‚ (U = bool) 2/2 โ”‚ โ”‚ โ”กโ”โ‚ always โ”‚ โ”‚ โ””โ”€โ‚€ always - โ”‚ โ””โ”€โ‚€ (U = bool) + โ”‚ โ””โ”€โ‚€ (U = bool) 2/2 โ”‚ โ”กโ”โ‚ always โ”‚ โ””โ”€โ‚€ never โ””โ”€โ‚€ never @@ -3834,7 +3840,9 @@ mod tests { let t_bool = ConstraintSet::range(&db, bool_type, t, bool_type); let u_str = ConstraintSet::range(&db, str_type, u, str_type); let u_bool = ConstraintSet::range(&db, bool_type, u, bool_type); - let constraints = (t_str.or(&db, || t_bool)).and(&db, || u_str.or(&db, || u_bool)); + // Construct this in a different order than above to make the source_orders more + // interesting. + let constraints = (u_str.or(&db, || u_bool)).and(&db, || t_str.or(&db, || t_bool)); let actual = constraints.node.display_graph(&db, &"").to_string(); assert_eq!(actual, expected); } From 1dd3cf0e58aa46ff6535fb72911cff1519bc9fdb Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Mon, 15 Dec 2025 10:22:27 -0500 Subject: [PATCH 29/30] fix test expectations (again) --- .../resources/mdtest/generics/specialize_constrained.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md b/crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md index 20e04ab24b..0cf656ccc3 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md @@ -316,7 +316,7 @@ def constrained_by_gradual_list_reverse[T: (list[Any], list[Base])](): # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[object]] reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.range(Never, T, list[object]))) # TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Any]] - # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Any] & list[Base]] + # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Base] & list[Any]] reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.range(Never, T, list[Any]))) # revealed: None reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.never())) From cba45acd86209bb654ff46888facd57e8ad5b1fa Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Mon, 15 Dec 2025 10:23:37 -0500 Subject: [PATCH 30/30] clippy --- crates/ty_python_semantic/src/types/constraints.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ty_python_semantic/src/types/constraints.rs b/crates/ty_python_semantic/src/types/constraints.rs index 0fbd9fc7db..2bf58f483b 100644 --- a/crates/ty_python_semantic/src/types/constraints.rs +++ b/crates/ty_python_semantic/src/types/constraints.rs @@ -1409,7 +1409,7 @@ impl<'db> Node<'db> { // We've been tracking the lower and upper bound that the types for this path must // satisfy. Pass those bounds along and let the caller choose a representative type // from within that range. - f(Some(¤t_bounds)); + f(Some(current_bounds)); } Node::AlwaysFalse => {