diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 1c97a8af4f..83332187bc 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -452,7 +452,7 @@ jobs: - name: "Install Rust toolchain" run: rustup show - name: "Install cargo-binstall" - uses: cargo-bins/cargo-binstall@20aa316bab4942180bbbabe93237858e8d77f1ed # v1.15.5 + uses: cargo-bins/cargo-binstall@38e8f5e4c386b611d51e8aa997b9a06a3c8eb67a # v1.15.6 - name: "Install cargo-fuzz" # Download the latest version from quick install and not the github releases because github releases only has MUSL targets. run: cargo binstall cargo-fuzz --force --disable-strategies crate-meta-data --no-confirm @@ -703,7 +703,7 @@ jobs: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: persist-credentials: false - - uses: cargo-bins/cargo-binstall@20aa316bab4942180bbbabe93237858e8d77f1ed # v1.15.5 + - uses: cargo-bins/cargo-binstall@38e8f5e4c386b611d51e8aa997b9a06a3c8eb67a # v1.15.6 - run: cargo binstall --no-confirm cargo-shear - run: cargo shear diff --git a/.github/workflows/ty-ecosystem-analyzer.yaml b/.github/workflows/ty-ecosystem-analyzer.yaml index 1051e88846..f32956cac0 100644 --- a/.github/workflows/ty-ecosystem-analyzer.yaml +++ b/.github/workflows/ty-ecosystem-analyzer.yaml @@ -64,7 +64,7 @@ jobs: cd .. - uv tool install "git+https://github.com/astral-sh/ecosystem-analyzer@fc0f612798710b0dd69bb7528bc9b361dc60bd43" + uv tool install "git+https://github.com/astral-sh/ecosystem-analyzer@6ce3a609575bc84eaf5d247739529c60b6c2ae5b" ecosystem-analyzer \ --repository ruff \ diff --git a/.github/workflows/ty-ecosystem-report.yaml b/.github/workflows/ty-ecosystem-report.yaml index 2df9640229..571adc7b21 100644 --- a/.github/workflows/ty-ecosystem-report.yaml +++ b/.github/workflows/ty-ecosystem-report.yaml @@ -49,7 +49,7 @@ jobs: cd .. - uv tool install "git+https://github.com/astral-sh/ecosystem-analyzer@fc0f612798710b0dd69bb7528bc9b361dc60bd43" + uv tool install "git+https://github.com/astral-sh/ecosystem-analyzer@6ce3a609575bc84eaf5d247739529c60b6c2ae5b" ecosystem-analyzer \ --verbose \ diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 713c750779..22d1dc0072 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -37,6 +37,8 @@ exploration of new features, we will often close these pull requests immediately new feature to ruff creates a long-term maintenance burden and requires strong consensus from the ruff team before it is appropriate to begin work on an implementation. +## The Basics + ### Prerequisites Ruff is written in Rust. You'll need to install the diff --git a/crates/ruff/src/args.rs b/crates/ruff/src/args.rs index aef5280955..180b251816 100644 --- a/crates/ruff/src/args.rs +++ b/crates/ruff/src/args.rs @@ -416,6 +416,7 @@ pub struct CheckCommand { conflicts_with = "stdin_filename", conflicts_with = "watch", conflicts_with = "fix", + conflicts_with = "diff", )] pub add_noqa: bool, /// See the files Ruff will be run against with the current settings. diff --git a/crates/ruff/src/commands/check.rs b/crates/ruff/src/commands/check.rs index f6e7c57ddb..e8b5817db4 100644 --- a/crates/ruff/src/commands/check.rs +++ b/crates/ruff/src/commands/check.rs @@ -227,7 +227,8 @@ mod test { use rustc_hash::FxHashMap; use tempfile::TempDir; - use ruff_linter::message::{Emitter, EmitterContext, TextEmitter}; + use ruff_db::diagnostic::{DiagnosticFormat, DisplayDiagnosticConfig, DisplayDiagnostics}; + use ruff_linter::message::EmitterContext; use ruff_linter::registry::Rule; use ruff_linter::settings::types::UnsafeFixes; use ruff_linter::settings::{LinterSettings, flags}; @@ -280,19 +281,16 @@ mod test { UnsafeFixes::Enabled, ) .unwrap(); - let mut output = Vec::new(); - TextEmitter::default() - .with_show_fix_status(true) - .with_color(false) - .emit( - &mut output, - &diagnostics.inner, - &EmitterContext::new(&FxHashMap::default()), - ) - .unwrap(); - - let messages = String::from_utf8(output).unwrap(); + let config = DisplayDiagnosticConfig::default() + .format(DiagnosticFormat::Concise) + .hide_severity(true); + let messages = DisplayDiagnostics::new( + &EmitterContext::new(&FxHashMap::default()), + &config, + &diagnostics.inner, + ) + .to_string(); insta::with_settings!({ omit_expression => true, diff --git a/crates/ruff/src/printer.rs b/crates/ruff/src/printer.rs index 2cfc0e7492..5c1b1d0e6a 100644 --- a/crates/ruff/src/printer.rs +++ b/crates/ruff/src/printer.rs @@ -10,12 +10,11 @@ use ruff_linter::linter::FixTable; use serde::Serialize; use ruff_db::diagnostic::{ - Diagnostic, DiagnosticFormat, DisplayDiagnosticConfig, DisplayDiagnostics, - DisplayGithubDiagnostics, GithubRenderer, SecondaryCode, + Diagnostic, DiagnosticFormat, DisplayDiagnosticConfig, DisplayDiagnostics, SecondaryCode, }; use ruff_linter::fs::relativize_path; use ruff_linter::logging::LogLevel; -use ruff_linter::message::{Emitter, EmitterContext, GroupedEmitter, SarifEmitter, TextEmitter}; +use ruff_linter::message::{EmitterContext, render_diagnostics}; use ruff_linter::notify_user; use ruff_linter::settings::flags::{self}; use ruff_linter::settings::types::{OutputFormat, UnsafeFixes}; @@ -225,86 +224,28 @@ impl Printer { let context = EmitterContext::new(&diagnostics.notebook_indexes); let fixables = FixableStatistics::try_from(diagnostics, self.unsafe_fixes); - let config = DisplayDiagnosticConfig::default().preview(preview); + let config = DisplayDiagnosticConfig::default() + .preview(preview) + .hide_severity(true) + .color(!cfg!(test) && colored::control::SHOULD_COLORIZE.should_colorize()) + .with_show_fix_status(show_fix_status(self.fix_mode, fixables.as_ref())) + .with_fix_applicability(self.unsafe_fixes.required_applicability()) + .show_fix_diff(preview); - match self.format { - OutputFormat::Json => { - let config = config.format(DiagnosticFormat::Json); - let value = DisplayDiagnostics::new(&context, &config, &diagnostics.inner); - write!(writer, "{value}")?; - } - OutputFormat::Rdjson => { - let config = config.format(DiagnosticFormat::Rdjson); - let value = DisplayDiagnostics::new(&context, &config, &diagnostics.inner); - write!(writer, "{value}")?; - } - OutputFormat::JsonLines => { - let config = config.format(DiagnosticFormat::JsonLines); - let value = DisplayDiagnostics::new(&context, &config, &diagnostics.inner); - write!(writer, "{value}")?; - } - OutputFormat::Junit => { - let config = config.format(DiagnosticFormat::Junit); - let value = DisplayDiagnostics::new(&context, &config, &diagnostics.inner); - write!(writer, "{value}")?; - } - OutputFormat::Concise | OutputFormat::Full => { - TextEmitter::default() - .with_show_fix_status(show_fix_status(self.fix_mode, fixables.as_ref())) - .with_show_fix_diff(self.format == OutputFormat::Full && preview) - .with_show_source(self.format == OutputFormat::Full) - .with_fix_applicability(self.unsafe_fixes.required_applicability()) - .with_preview(preview) - .emit(writer, &diagnostics.inner, &context)?; + render_diagnostics(writer, self.format, config, &context, &diagnostics.inner)?; - if self.flags.intersects(Flags::SHOW_FIX_SUMMARY) { - if !diagnostics.fixed.is_empty() { - writeln!(writer)?; - print_fix_summary(writer, &diagnostics.fixed)?; - writeln!(writer)?; - } + if matches!( + self.format, + OutputFormat::Full | OutputFormat::Concise | OutputFormat::Grouped + ) { + if self.flags.intersects(Flags::SHOW_FIX_SUMMARY) { + if !diagnostics.fixed.is_empty() { + writeln!(writer)?; + print_fix_summary(writer, &diagnostics.fixed)?; + writeln!(writer)?; } - - self.write_summary_text(writer, diagnostics)?; - } - OutputFormat::Grouped => { - GroupedEmitter::default() - .with_show_fix_status(show_fix_status(self.fix_mode, fixables.as_ref())) - .with_unsafe_fixes(self.unsafe_fixes) - .emit(writer, &diagnostics.inner, &context)?; - - if self.flags.intersects(Flags::SHOW_FIX_SUMMARY) { - if !diagnostics.fixed.is_empty() { - writeln!(writer)?; - print_fix_summary(writer, &diagnostics.fixed)?; - writeln!(writer)?; - } - } - self.write_summary_text(writer, diagnostics)?; - } - OutputFormat::Github => { - let renderer = GithubRenderer::new(&context, "Ruff"); - let value = DisplayGithubDiagnostics::new(&renderer, &diagnostics.inner); - write!(writer, "{value}")?; - } - OutputFormat::Gitlab => { - let config = config.format(DiagnosticFormat::Gitlab); - let value = DisplayDiagnostics::new(&context, &config, &diagnostics.inner); - write!(writer, "{value}")?; - } - OutputFormat::Pylint => { - let config = config.format(DiagnosticFormat::Pylint); - let value = DisplayDiagnostics::new(&context, &config, &diagnostics.inner); - write!(writer, "{value}")?; - } - OutputFormat::Azure => { - let config = config.format(DiagnosticFormat::Azure); - let value = DisplayDiagnostics::new(&context, &config, &diagnostics.inner); - write!(writer, "{value}")?; - } - OutputFormat::Sarif => { - SarifEmitter.emit(writer, &diagnostics.inner, &context)?; } + self.write_summary_text(writer, diagnostics)?; } writer.flush()?; @@ -448,11 +389,22 @@ impl Printer { } let context = EmitterContext::new(&diagnostics.notebook_indexes); - TextEmitter::default() + let format = if preview { + DiagnosticFormat::Full + } else { + DiagnosticFormat::Concise + }; + let config = DisplayDiagnosticConfig::default() + .hide_severity(true) + .color(!cfg!(test) && colored::control::SHOULD_COLORIZE.should_colorize()) .with_show_fix_status(show_fix_status(self.fix_mode, fixables.as_ref())) - .with_show_source(preview) - .with_fix_applicability(self.unsafe_fixes.required_applicability()) - .emit(writer, &diagnostics.inner, &context)?; + .format(format) + .with_fix_applicability(self.unsafe_fixes.required_applicability()); + write!( + writer, + "{}", + DisplayDiagnostics::new(&context, &config, &diagnostics.inner) + )?; } writer.flush()?; diff --git a/crates/ruff/tests/lint.rs b/crates/ruff/tests/lint.rs index 75e267e1b4..c44f7b9ecd 100644 --- a/crates/ruff/tests/lint.rs +++ b/crates/ruff/tests/lint.rs @@ -6199,6 +6199,36 @@ match 42: # invalid-syntax Ok(()) } +#[test_case::test_case("concise"; "concise_show_fixes")] +#[test_case::test_case("full"; "full_show_fixes")] +#[test_case::test_case("grouped"; "grouped_show_fixes")] +fn output_format_show_fixes(output_format: &str) -> Result<()> { + let tempdir = TempDir::new()?; + let input = tempdir.path().join("input.py"); + fs::write(&input, "import os # F401")?; + + let snapshot = format!("output_format_show_fixes_{output_format}"); + + assert_cmd_snapshot!( + snapshot, + Command::new(get_cargo_bin(BIN_NAME)) + .args([ + "check", + "--no-cache", + "--output-format", + output_format, + "--select", + "F401", + "--fix", + "--show-fixes", + "input.py", + ]) + .current_dir(&tempdir), + ); + + Ok(()) +} + #[test] fn up045_nested_optional_flatten_all() { let contents = "\ diff --git a/crates/ruff/tests/snapshots/integration_test__rule_f401.snap b/crates/ruff/tests/snapshots/integration_test__rule_f401.snap index 55c35568cd..9213ccb0d7 100644 --- a/crates/ruff/tests/snapshots/integration_test__rule_f401.snap +++ b/crates/ruff/tests/snapshots/integration_test__rule_f401.snap @@ -44,6 +44,43 @@ import some_module __all__ = ["some_module"] ``` +## Preview +When [preview] is enabled (and certain simplifying assumptions +are met), we analyze all import statements for a given module +when determining whether an import is used, rather than simply +the last of these statements. This can result in both different and +more import statements being marked as unused. + +For example, if a module consists of + +```python +import a +import a.b +``` + +then both statements are marked as unused under [preview], whereas +only the second is marked as unused under stable behavior. + +As another example, if a module consists of + +```python +import a.b +import a + +a.b.foo() +``` + +then a diagnostic will only be emitted for the first line under [preview], +whereas a diagnostic would only be emitted for the second line under +stable behavior. + +Note that this behavior is somewhat subjective and is designed +to conform to the developer's intuition rather than Python's actual +execution. To wit, the statement `import a.b` automatically executes +`import a`, so in some sense `import a` is _always_ redundant +in the presence of `import a.b`. + + ## Fix safety Fixes to remove unused imports are safe, except in `__init__.py` files. @@ -96,4 +133,6 @@ else: - [Python documentation: `importlib.util.find_spec`](https://docs.python.org/3/library/importlib.html#importlib.util.find_spec) - [Typing documentation: interface conventions](https://typing.python.org/en/latest/spec/distributing.html#library-interface-public-and-private-symbols) +[preview]: https://docs.astral.sh/ruff/preview/ + ----- stderr ----- diff --git a/crates/ruff/tests/snapshots/lint__output_format_sarif.snap b/crates/ruff/tests/snapshots/lint__output_format_sarif.snap index e5dba49431..1744357e80 100644 --- a/crates/ruff/tests/snapshots/lint__output_format_sarif.snap +++ b/crates/ruff/tests/snapshots/lint__output_format_sarif.snap @@ -119,7 +119,7 @@ exit_code: 1 "rules": [ { "fullDescription": { - "text": "## What it does\nChecks for unused imports.\n\n## Why is this bad?\nUnused imports add a performance overhead at runtime, and risk creating\nimport cycles. They also increase the cognitive load of reading the code.\n\nIf an import statement is used to check for the availability or existence\nof a module, consider using `importlib.util.find_spec` instead.\n\nIf an import statement is used to re-export a symbol as part of a module's\npublic interface, consider using a \"redundant\" import alias, which\ninstructs Ruff (and other tools) to respect the re-export, and avoid\nmarking it as unused, as in:\n\n```python\nfrom module import member as member\n```\n\nAlternatively, you can use `__all__` to declare a symbol as part of the module's\ninterface, as in:\n\n```python\n# __init__.py\nimport some_module\n\n__all__ = [\"some_module\"]\n```\n\n## Fix safety\n\nFixes to remove unused imports are safe, except in `__init__.py` files.\n\nApplying fixes to `__init__.py` files is currently in preview. The fix offered depends on the\ntype of the unused import. Ruff will suggest a safe fix to export first-party imports with\neither a redundant alias or, if already present in the file, an `__all__` entry. If multiple\n`__all__` declarations are present, Ruff will not offer a fix. Ruff will suggest an unsafe fix\nto remove third-party and standard library imports -- the fix is unsafe because the module's\ninterface changes.\n\nSee [this FAQ section](https://docs.astral.sh/ruff/faq/#how-does-ruff-determine-which-of-my-imports-are-first-party-third-party-etc)\nfor more details on how Ruff\ndetermines whether an import is first or third-party.\n\n## Example\n\n```python\nimport numpy as np # unused import\n\n\ndef area(radius):\n return 3.14 * radius**2\n```\n\nUse instead:\n\n```python\ndef area(radius):\n return 3.14 * radius**2\n```\n\nTo check the availability of a module, use `importlib.util.find_spec`:\n\n```python\nfrom importlib.util import find_spec\n\nif find_spec(\"numpy\") is not None:\n print(\"numpy is installed\")\nelse:\n print(\"numpy is not installed\")\n```\n\n## Options\n- `lint.ignore-init-module-imports`\n- `lint.pyflakes.allowed-unused-imports`\n\n## References\n- [Python documentation: `import`](https://docs.python.org/3/reference/simple_stmts.html#the-import-statement)\n- [Python documentation: `importlib.util.find_spec`](https://docs.python.org/3/library/importlib.html#importlib.util.find_spec)\n- [Typing documentation: interface conventions](https://typing.python.org/en/latest/spec/distributing.html#library-interface-public-and-private-symbols)\n" + "text": "## What it does\nChecks for unused imports.\n\n## Why is this bad?\nUnused imports add a performance overhead at runtime, and risk creating\nimport cycles. They also increase the cognitive load of reading the code.\n\nIf an import statement is used to check for the availability or existence\nof a module, consider using `importlib.util.find_spec` instead.\n\nIf an import statement is used to re-export a symbol as part of a module's\npublic interface, consider using a \"redundant\" import alias, which\ninstructs Ruff (and other tools) to respect the re-export, and avoid\nmarking it as unused, as in:\n\n```python\nfrom module import member as member\n```\n\nAlternatively, you can use `__all__` to declare a symbol as part of the module's\ninterface, as in:\n\n```python\n# __init__.py\nimport some_module\n\n__all__ = [\"some_module\"]\n```\n\n## Preview\nWhen [preview] is enabled (and certain simplifying assumptions\nare met), we analyze all import statements for a given module\nwhen determining whether an import is used, rather than simply\nthe last of these statements. This can result in both different and\nmore import statements being marked as unused.\n\nFor example, if a module consists of\n\n```python\nimport a\nimport a.b\n```\n\nthen both statements are marked as unused under [preview], whereas\nonly the second is marked as unused under stable behavior.\n\nAs another example, if a module consists of\n\n```python\nimport a.b\nimport a\n\na.b.foo()\n```\n\nthen a diagnostic will only be emitted for the first line under [preview],\nwhereas a diagnostic would only be emitted for the second line under\nstable behavior.\n\nNote that this behavior is somewhat subjective and is designed\nto conform to the developer's intuition rather than Python's actual\nexecution. To wit, the statement `import a.b` automatically executes\n`import a`, so in some sense `import a` is _always_ redundant\nin the presence of `import a.b`.\n\n\n## Fix safety\n\nFixes to remove unused imports are safe, except in `__init__.py` files.\n\nApplying fixes to `__init__.py` files is currently in preview. The fix offered depends on the\ntype of the unused import. Ruff will suggest a safe fix to export first-party imports with\neither a redundant alias or, if already present in the file, an `__all__` entry. If multiple\n`__all__` declarations are present, Ruff will not offer a fix. Ruff will suggest an unsafe fix\nto remove third-party and standard library imports -- the fix is unsafe because the module's\ninterface changes.\n\nSee [this FAQ section](https://docs.astral.sh/ruff/faq/#how-does-ruff-determine-which-of-my-imports-are-first-party-third-party-etc)\nfor more details on how Ruff\ndetermines whether an import is first or third-party.\n\n## Example\n\n```python\nimport numpy as np # unused import\n\n\ndef area(radius):\n return 3.14 * radius**2\n```\n\nUse instead:\n\n```python\ndef area(radius):\n return 3.14 * radius**2\n```\n\nTo check the availability of a module, use `importlib.util.find_spec`:\n\n```python\nfrom importlib.util import find_spec\n\nif find_spec(\"numpy\") is not None:\n print(\"numpy is installed\")\nelse:\n print(\"numpy is not installed\")\n```\n\n## Options\n- `lint.ignore-init-module-imports`\n- `lint.pyflakes.allowed-unused-imports`\n\n## References\n- [Python documentation: `import`](https://docs.python.org/3/reference/simple_stmts.html#the-import-statement)\n- [Python documentation: `importlib.util.find_spec`](https://docs.python.org/3/library/importlib.html#importlib.util.find_spec)\n- [Typing documentation: interface conventions](https://typing.python.org/en/latest/spec/distributing.html#library-interface-public-and-private-symbols)\n\n[preview]: https://docs.astral.sh/ruff/preview/\n" }, "help": { "text": "`{name}` imported but unused; consider using `importlib.util.find_spec` to test for availability" diff --git a/crates/ruff/tests/snapshots/lint__output_format_show_fixes_concise.snap b/crates/ruff/tests/snapshots/lint__output_format_show_fixes_concise.snap new file mode 100644 index 0000000000..e72b277305 --- /dev/null +++ b/crates/ruff/tests/snapshots/lint__output_format_show_fixes_concise.snap @@ -0,0 +1,26 @@ +--- +source: crates/ruff/tests/lint.rs +info: + program: ruff + args: + - check + - "--no-cache" + - "--output-format" + - concise + - "--select" + - F401 + - "--fix" + - "--show-fixes" + - input.py +--- +success: true +exit_code: 0 +----- stdout ----- + +Fixed 1 error: +- input.py: + 1 × F401 (unused-import) + +Found 1 error (1 fixed, 0 remaining). + +----- stderr ----- diff --git a/crates/ruff/tests/snapshots/lint__output_format_show_fixes_full.snap b/crates/ruff/tests/snapshots/lint__output_format_show_fixes_full.snap new file mode 100644 index 0000000000..96bbccb76e --- /dev/null +++ b/crates/ruff/tests/snapshots/lint__output_format_show_fixes_full.snap @@ -0,0 +1,26 @@ +--- +source: crates/ruff/tests/lint.rs +info: + program: ruff + args: + - check + - "--no-cache" + - "--output-format" + - full + - "--select" + - F401 + - "--fix" + - "--show-fixes" + - input.py +--- +success: true +exit_code: 0 +----- stdout ----- + +Fixed 1 error: +- input.py: + 1 × F401 (unused-import) + +Found 1 error (1 fixed, 0 remaining). + +----- stderr ----- diff --git a/crates/ruff/tests/snapshots/lint__output_format_show_fixes_grouped.snap b/crates/ruff/tests/snapshots/lint__output_format_show_fixes_grouped.snap new file mode 100644 index 0000000000..ce0e16ff12 --- /dev/null +++ b/crates/ruff/tests/snapshots/lint__output_format_show_fixes_grouped.snap @@ -0,0 +1,26 @@ +--- +source: crates/ruff/tests/lint.rs +info: + program: ruff + args: + - check + - "--no-cache" + - "--output-format" + - grouped + - "--select" + - F401 + - "--fix" + - "--show-fixes" + - input.py +--- +success: true +exit_code: 0 +----- stdout ----- + +Fixed 1 error: +- input.py: + 1 × F401 (unused-import) + +Found 1 error (1 fixed, 0 remaining). + +----- stderr ----- diff --git a/crates/ruff_benchmark/benches/ty.rs b/crates/ruff_benchmark/benches/ty.rs index f1000fb050..9fb13aca01 100644 --- a/crates/ruff_benchmark/benches/ty.rs +++ b/crates/ruff_benchmark/benches/ty.rs @@ -444,7 +444,7 @@ fn benchmark_complex_constrained_attributes_2(criterion: &mut Criterion) { criterion.bench_function("ty_micro[complex_constrained_attributes_2]", |b| { b.iter_batched_ref( || { - // This is is similar to the case above, but now the attributes are actually defined. + // This is similar to the case above, but now the attributes are actually defined. // https://github.com/astral-sh/ty/issues/711 setup_micro_case( r#" diff --git a/crates/ruff_benchmark/benches/ty_walltime.rs b/crates/ruff_benchmark/benches/ty_walltime.rs index ef7cd9333b..c74611d0b2 100644 --- a/crates/ruff_benchmark/benches/ty_walltime.rs +++ b/crates/ruff_benchmark/benches/ty_walltime.rs @@ -117,7 +117,7 @@ static COLOUR_SCIENCE: std::sync::LazyLock> = std::sync::Lazy max_dep_date: "2025-06-17", python_version: PythonVersion::PY310, }, - 477, + 500, ) }); diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index 99b8b9d83b..413e08cbe2 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -1353,7 +1353,7 @@ impl DisplayDiagnosticConfig { } /// Whether to show a fix's availability or not. - pub fn show_fix_status(self, yes: bool) -> DisplayDiagnosticConfig { + pub fn with_show_fix_status(self, yes: bool) -> DisplayDiagnosticConfig { DisplayDiagnosticConfig { show_fix_status: yes, ..self @@ -1374,12 +1374,20 @@ impl DisplayDiagnosticConfig { /// availability for unsafe or display-only fixes. /// /// Note that this option is currently ignored when `hide_severity` is false. - pub fn fix_applicability(self, applicability: Applicability) -> DisplayDiagnosticConfig { + pub fn with_fix_applicability(self, applicability: Applicability) -> DisplayDiagnosticConfig { DisplayDiagnosticConfig { fix_applicability: applicability, ..self } } + + pub fn show_fix_status(&self) -> bool { + self.show_fix_status + } + + pub fn fix_applicability(&self) -> Applicability { + self.fix_applicability + } } impl Default for DisplayDiagnosticConfig { diff --git a/crates/ruff_db/src/diagnostic/render.rs b/crates/ruff_db/src/diagnostic/render.rs index 86b04fef00..8f1b9184a8 100644 --- a/crates/ruff_db/src/diagnostic/render.rs +++ b/crates/ruff_db/src/diagnostic/render.rs @@ -2618,7 +2618,7 @@ watermelon /// Show fix availability when rendering. pub(super) fn show_fix_status(&mut self, yes: bool) { let mut config = std::mem::take(&mut self.config); - config = config.show_fix_status(yes); + config = config.with_show_fix_status(yes); self.config = config; } @@ -2632,7 +2632,7 @@ watermelon /// The lowest fix applicability to show when rendering. pub(super) fn fix_applicability(&mut self, applicability: Applicability) { let mut config = std::mem::take(&mut self.config); - config = config.fix_applicability(applicability); + config = config.with_fix_applicability(applicability); self.config = config; } diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 961052eebb..1a1f462e8e 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -2360,7 +2360,7 @@ impl<'a> Checker<'a> { } } - /// Visit an body of [`Stmt`] nodes within a type-checking block. + /// Visit a body of [`Stmt`] nodes within a type-checking block. fn visit_type_checking_block(&mut self, body: &'a [Stmt]) { let snapshot = self.semantic.flags; self.semantic.flags |= SemanticModelFlags::TYPE_CHECKING_BLOCK; diff --git a/crates/ruff_linter/src/message/grouped.rs b/crates/ruff_linter/src/message/grouped.rs index 4626ab43cf..0426697728 100644 --- a/crates/ruff_linter/src/message/grouped.rs +++ b/crates/ruff_linter/src/message/grouped.rs @@ -6,17 +6,25 @@ use std::num::NonZeroUsize; use colored::Colorize; use ruff_db::diagnostic::Diagnostic; +use ruff_diagnostics::Applicability; use ruff_notebook::NotebookIndex; use ruff_source_file::{LineColumn, OneIndexed}; use crate::fs::relativize_path; use crate::message::{Emitter, EmitterContext}; -use crate::settings::types::UnsafeFixes; -#[derive(Default)] pub struct GroupedEmitter { show_fix_status: bool, - unsafe_fixes: UnsafeFixes, + applicability: Applicability, +} + +impl Default for GroupedEmitter { + fn default() -> Self { + Self { + show_fix_status: false, + applicability: Applicability::Safe, + } + } } impl GroupedEmitter { @@ -27,8 +35,8 @@ impl GroupedEmitter { } #[must_use] - pub fn with_unsafe_fixes(mut self, unsafe_fixes: UnsafeFixes) -> Self { - self.unsafe_fixes = unsafe_fixes; + pub fn with_applicability(mut self, applicability: Applicability) -> Self { + self.applicability = applicability; self } } @@ -67,7 +75,7 @@ impl Emitter for GroupedEmitter { notebook_index: context.notebook_index(&message.expect_ruff_filename()), message, show_fix_status: self.show_fix_status, - unsafe_fixes: self.unsafe_fixes, + applicability: self.applicability, row_length, column_length, } @@ -114,7 +122,7 @@ fn group_diagnostics_by_filename( struct DisplayGroupedMessage<'a> { message: MessageWithLocation<'a>, show_fix_status: bool, - unsafe_fixes: UnsafeFixes, + applicability: Applicability, row_length: NonZeroUsize, column_length: NonZeroUsize, notebook_index: Option<&'a NotebookIndex>, @@ -162,7 +170,7 @@ impl Display for DisplayGroupedMessage<'_> { code_and_body = RuleCodeAndBody { message, show_fix_status: self.show_fix_status, - unsafe_fixes: self.unsafe_fixes + applicability: self.applicability }, )?; @@ -173,7 +181,7 @@ impl Display for DisplayGroupedMessage<'_> { pub(super) struct RuleCodeAndBody<'a> { pub(crate) message: &'a Diagnostic, pub(crate) show_fix_status: bool, - pub(crate) unsafe_fixes: UnsafeFixes, + pub(crate) applicability: Applicability, } impl Display for RuleCodeAndBody<'_> { @@ -181,7 +189,7 @@ impl Display for RuleCodeAndBody<'_> { if self.show_fix_status { if let Some(fix) = self.message.fix() { // Do not display an indicator for inapplicable fixes - if fix.applies(self.unsafe_fixes.required_applicability()) { + if fix.applies(self.applicability) { if let Some(code) = self.message.secondary_code() { write!(f, "{} ", code.red().bold())?; } @@ -217,11 +225,12 @@ impl Display for RuleCodeAndBody<'_> { mod tests { use insta::assert_snapshot; + use ruff_diagnostics::Applicability; + use crate::message::GroupedEmitter; use crate::message::tests::{ capture_emitter_output, create_diagnostics, create_syntax_error_diagnostics, }; - use crate::settings::types::UnsafeFixes; #[test] fn default() { @@ -251,7 +260,7 @@ mod tests { fn fix_status_unsafe() { let mut emitter = GroupedEmitter::default() .with_show_fix_status(true) - .with_unsafe_fixes(UnsafeFixes::Enabled); + .with_applicability(Applicability::Unsafe); let content = capture_emitter_output(&mut emitter, &create_diagnostics()); assert_snapshot!(content); diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index 994dc81296..5ac4712a61 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -4,8 +4,9 @@ use std::io::Write; use rustc_hash::FxHashMap; use ruff_db::diagnostic::{ - Annotation, Diagnostic, DiagnosticId, FileResolver, Input, LintName, SecondaryCode, Severity, - Span, UnifiedFile, + Annotation, Diagnostic, DiagnosticFormat, DiagnosticId, DisplayDiagnosticConfig, + DisplayDiagnostics, DisplayGithubDiagnostics, FileResolver, GithubRenderer, Input, LintName, + SecondaryCode, Severity, Span, UnifiedFile, }; use ruff_db::files::File; @@ -14,14 +15,13 @@ use ruff_notebook::NotebookIndex; use ruff_source_file::SourceFile; use ruff_text_size::{Ranged, TextRange, TextSize}; pub use sarif::SarifEmitter; -pub use text::TextEmitter; use crate::Fix; use crate::registry::Rule; +use crate::settings::types::{OutputFormat, RuffOutputFormat}; mod grouped; mod sarif; -mod text; /// Creates a `Diagnostic` from a syntax error, with the format expected by Ruff. /// @@ -160,14 +160,48 @@ impl<'a> EmitterContext<'a> { } } +pub fn render_diagnostics( + writer: &mut dyn Write, + format: OutputFormat, + config: DisplayDiagnosticConfig, + context: &EmitterContext<'_>, + diagnostics: &[Diagnostic], +) -> std::io::Result<()> { + match DiagnosticFormat::try_from(format) { + Ok(format) => { + let config = config.format(format); + let value = DisplayDiagnostics::new(context, &config, diagnostics); + write!(writer, "{value}")?; + } + Err(RuffOutputFormat::Github) => { + let renderer = GithubRenderer::new(context, "Ruff"); + let value = DisplayGithubDiagnostics::new(&renderer, diagnostics); + write!(writer, "{value}")?; + } + Err(RuffOutputFormat::Grouped) => { + GroupedEmitter::default() + .with_show_fix_status(config.show_fix_status()) + .with_applicability(config.fix_applicability()) + .emit(writer, diagnostics, context) + .map_err(std::io::Error::other)?; + } + Err(RuffOutputFormat::Sarif) => { + SarifEmitter + .emit(writer, diagnostics, context) + .map_err(std::io::Error::other)?; + } + } + + Ok(()) +} + #[cfg(test)] mod tests { use rustc_hash::FxHashMap; use ruff_db::diagnostic::Diagnostic; - use ruff_notebook::NotebookIndex; use ruff_python_parser::{Mode, ParseOptions, parse_unchecked}; - use ruff_source_file::{OneIndexed, SourceFileBuilder}; + use ruff_source_file::SourceFileBuilder; use ruff_text_size::{TextRange, TextSize}; use crate::codes::Rule; @@ -257,104 +291,6 @@ def fibonacci(n): vec![unused_import, unused_variable, undefined_name] } - pub(super) fn create_notebook_diagnostics() - -> (Vec, FxHashMap) { - let notebook = r"# cell 1 -import os -# cell 2 -import math - -print('hello world') -# cell 3 -def foo(): - print() - x = 1 -"; - - let notebook_source = SourceFileBuilder::new("notebook.ipynb", notebook).finish(); - - let unused_import_os_start = TextSize::from(16); - let unused_import_os = create_lint_diagnostic( - "`os` imported but unused", - Some("Remove unused import: `os`"), - TextRange::new(unused_import_os_start, TextSize::from(18)), - Some(Fix::safe_edit(Edit::range_deletion(TextRange::new( - TextSize::from(9), - TextSize::from(19), - )))), - None, - notebook_source.clone(), - Some(unused_import_os_start), - Rule::UnusedImport, - ); - - let unused_import_math_start = TextSize::from(35); - let unused_import_math = create_lint_diagnostic( - "`math` imported but unused", - Some("Remove unused import: `math`"), - TextRange::new(unused_import_math_start, TextSize::from(39)), - Some(Fix::safe_edit(Edit::range_deletion(TextRange::new( - TextSize::from(28), - TextSize::from(40), - )))), - None, - notebook_source.clone(), - Some(unused_import_math_start), - Rule::UnusedImport, - ); - - let unused_variable_start = TextSize::from(98); - let unused_variable = create_lint_diagnostic( - "Local variable `x` is assigned to but never used", - Some("Remove assignment to unused variable `x`"), - TextRange::new(unused_variable_start, TextSize::from(99)), - Some(Fix::unsafe_edit(Edit::deletion( - TextSize::from(94), - TextSize::from(104), - ))), - None, - notebook_source, - Some(unused_variable_start), - Rule::UnusedVariable, - ); - - let mut notebook_indexes = FxHashMap::default(); - notebook_indexes.insert( - "notebook.ipynb".to_string(), - NotebookIndex::new( - vec![ - OneIndexed::from_zero_indexed(0), - OneIndexed::from_zero_indexed(0), - OneIndexed::from_zero_indexed(1), - OneIndexed::from_zero_indexed(1), - OneIndexed::from_zero_indexed(1), - OneIndexed::from_zero_indexed(1), - OneIndexed::from_zero_indexed(2), - OneIndexed::from_zero_indexed(2), - OneIndexed::from_zero_indexed(2), - OneIndexed::from_zero_indexed(2), - ], - vec![ - OneIndexed::from_zero_indexed(0), - OneIndexed::from_zero_indexed(1), - OneIndexed::from_zero_indexed(0), - OneIndexed::from_zero_indexed(1), - OneIndexed::from_zero_indexed(2), - OneIndexed::from_zero_indexed(3), - OneIndexed::from_zero_indexed(0), - OneIndexed::from_zero_indexed(1), - OneIndexed::from_zero_indexed(2), - OneIndexed::from_zero_indexed(3), - ], - ), - ); - - ( - vec![unused_import_os, unused_import_math, unused_variable], - notebook_indexes, - ) - } - pub(super) fn capture_emitter_output( emitter: &mut dyn Emitter, diagnostics: &[Diagnostic], @@ -366,16 +302,4 @@ def foo(): String::from_utf8(output).expect("Output to be valid UTF-8") } - - pub(super) fn capture_emitter_notebook_output( - emitter: &mut dyn Emitter, - diagnostics: &[Diagnostic], - notebook_indexes: &FxHashMap, - ) -> String { - let context = EmitterContext::new(notebook_indexes); - let mut output: Vec = Vec::new(); - emitter.emit(&mut output, diagnostics, &context).unwrap(); - - String::from_utf8(output).expect("Output to be valid UTF-8") - } } diff --git a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__sarif__tests__results.snap b/crates/ruff_linter/src/message/snapshots/ruff_linter__message__sarif__tests__results.snap index bafe793739..c8c125c6a5 100644 --- a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__sarif__tests__results.snap +++ b/crates/ruff_linter/src/message/snapshots/ruff_linter__message__sarif__tests__results.snap @@ -129,7 +129,7 @@ expression: value "rules": [ { "fullDescription": { - "text": "## What it does\nChecks for unused imports.\n\n## Why is this bad?\nUnused imports add a performance overhead at runtime, and risk creating\nimport cycles. They also increase the cognitive load of reading the code.\n\nIf an import statement is used to check for the availability or existence\nof a module, consider using `importlib.util.find_spec` instead.\n\nIf an import statement is used to re-export a symbol as part of a module's\npublic interface, consider using a \"redundant\" import alias, which\ninstructs Ruff (and other tools) to respect the re-export, and avoid\nmarking it as unused, as in:\n\n```python\nfrom module import member as member\n```\n\nAlternatively, you can use `__all__` to declare a symbol as part of the module's\ninterface, as in:\n\n```python\n# __init__.py\nimport some_module\n\n__all__ = [\"some_module\"]\n```\n\n## Fix safety\n\nFixes to remove unused imports are safe, except in `__init__.py` files.\n\nApplying fixes to `__init__.py` files is currently in preview. The fix offered depends on the\ntype of the unused import. Ruff will suggest a safe fix to export first-party imports with\neither a redundant alias or, if already present in the file, an `__all__` entry. If multiple\n`__all__` declarations are present, Ruff will not offer a fix. Ruff will suggest an unsafe fix\nto remove third-party and standard library imports -- the fix is unsafe because the module's\ninterface changes.\n\nSee [this FAQ section](https://docs.astral.sh/ruff/faq/#how-does-ruff-determine-which-of-my-imports-are-first-party-third-party-etc)\nfor more details on how Ruff\ndetermines whether an import is first or third-party.\n\n## Example\n\n```python\nimport numpy as np # unused import\n\n\ndef area(radius):\n return 3.14 * radius**2\n```\n\nUse instead:\n\n```python\ndef area(radius):\n return 3.14 * radius**2\n```\n\nTo check the availability of a module, use `importlib.util.find_spec`:\n\n```python\nfrom importlib.util import find_spec\n\nif find_spec(\"numpy\") is not None:\n print(\"numpy is installed\")\nelse:\n print(\"numpy is not installed\")\n```\n\n## Options\n- `lint.ignore-init-module-imports`\n- `lint.pyflakes.allowed-unused-imports`\n\n## References\n- [Python documentation: `import`](https://docs.python.org/3/reference/simple_stmts.html#the-import-statement)\n- [Python documentation: `importlib.util.find_spec`](https://docs.python.org/3/library/importlib.html#importlib.util.find_spec)\n- [Typing documentation: interface conventions](https://typing.python.org/en/latest/spec/distributing.html#library-interface-public-and-private-symbols)\n" + "text": "## What it does\nChecks for unused imports.\n\n## Why is this bad?\nUnused imports add a performance overhead at runtime, and risk creating\nimport cycles. They also increase the cognitive load of reading the code.\n\nIf an import statement is used to check for the availability or existence\nof a module, consider using `importlib.util.find_spec` instead.\n\nIf an import statement is used to re-export a symbol as part of a module's\npublic interface, consider using a \"redundant\" import alias, which\ninstructs Ruff (and other tools) to respect the re-export, and avoid\nmarking it as unused, as in:\n\n```python\nfrom module import member as member\n```\n\nAlternatively, you can use `__all__` to declare a symbol as part of the module's\ninterface, as in:\n\n```python\n# __init__.py\nimport some_module\n\n__all__ = [\"some_module\"]\n```\n\n## Preview\nWhen [preview] is enabled (and certain simplifying assumptions\nare met), we analyze all import statements for a given module\nwhen determining whether an import is used, rather than simply\nthe last of these statements. This can result in both different and\nmore import statements being marked as unused.\n\nFor example, if a module consists of\n\n```python\nimport a\nimport a.b\n```\n\nthen both statements are marked as unused under [preview], whereas\nonly the second is marked as unused under stable behavior.\n\nAs another example, if a module consists of\n\n```python\nimport a.b\nimport a\n\na.b.foo()\n```\n\nthen a diagnostic will only be emitted for the first line under [preview],\nwhereas a diagnostic would only be emitted for the second line under\nstable behavior.\n\nNote that this behavior is somewhat subjective and is designed\nto conform to the developer's intuition rather than Python's actual\nexecution. To wit, the statement `import a.b` automatically executes\n`import a`, so in some sense `import a` is _always_ redundant\nin the presence of `import a.b`.\n\n\n## Fix safety\n\nFixes to remove unused imports are safe, except in `__init__.py` files.\n\nApplying fixes to `__init__.py` files is currently in preview. The fix offered depends on the\ntype of the unused import. Ruff will suggest a safe fix to export first-party imports with\neither a redundant alias or, if already present in the file, an `__all__` entry. If multiple\n`__all__` declarations are present, Ruff will not offer a fix. Ruff will suggest an unsafe fix\nto remove third-party and standard library imports -- the fix is unsafe because the module's\ninterface changes.\n\nSee [this FAQ section](https://docs.astral.sh/ruff/faq/#how-does-ruff-determine-which-of-my-imports-are-first-party-third-party-etc)\nfor more details on how Ruff\ndetermines whether an import is first or third-party.\n\n## Example\n\n```python\nimport numpy as np # unused import\n\n\ndef area(radius):\n return 3.14 * radius**2\n```\n\nUse instead:\n\n```python\ndef area(radius):\n return 3.14 * radius**2\n```\n\nTo check the availability of a module, use `importlib.util.find_spec`:\n\n```python\nfrom importlib.util import find_spec\n\nif find_spec(\"numpy\") is not None:\n print(\"numpy is installed\")\nelse:\n print(\"numpy is not installed\")\n```\n\n## Options\n- `lint.ignore-init-module-imports`\n- `lint.pyflakes.allowed-unused-imports`\n\n## References\n- [Python documentation: `import`](https://docs.python.org/3/reference/simple_stmts.html#the-import-statement)\n- [Python documentation: `importlib.util.find_spec`](https://docs.python.org/3/library/importlib.html#importlib.util.find_spec)\n- [Typing documentation: interface conventions](https://typing.python.org/en/latest/spec/distributing.html#library-interface-public-and-private-symbols)\n\n[preview]: https://docs.astral.sh/ruff/preview/\n" }, "help": { "text": "`{name}` imported but unused; consider using `importlib.util.find_spec` to test for availability" diff --git a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__text__tests__default.snap b/crates/ruff_linter/src/message/snapshots/ruff_linter__message__text__tests__default.snap deleted file mode 100644 index 480dd582a7..0000000000 --- a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__text__tests__default.snap +++ /dev/null @@ -1,30 +0,0 @@ ---- -source: crates/ruff_linter/src/message/text.rs -expression: content ---- -F401 `os` imported but unused - --> fib.py:1:8 - | -1 | import os - | ^^ - | -help: Remove unused import: `os` - -F841 Local variable `x` is assigned to but never used - --> fib.py:6:5 - | -4 | def fibonacci(n): -5 | """Compute the nth number in the Fibonacci sequence.""" -6 | x = 1 - | ^ -7 | if n == 0: -8 | return 0 - | -help: Remove assignment to unused variable `x` - -F821 Undefined name `a` - --> undef.py:1:4 - | -1 | if a == 1: pass - | ^ - | diff --git a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__text__tests__fix_status.snap b/crates/ruff_linter/src/message/snapshots/ruff_linter__message__text__tests__fix_status.snap deleted file mode 100644 index 480dd582a7..0000000000 --- a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__text__tests__fix_status.snap +++ /dev/null @@ -1,30 +0,0 @@ ---- -source: crates/ruff_linter/src/message/text.rs -expression: content ---- -F401 `os` imported but unused - --> fib.py:1:8 - | -1 | import os - | ^^ - | -help: Remove unused import: `os` - -F841 Local variable `x` is assigned to but never used - --> fib.py:6:5 - | -4 | def fibonacci(n): -5 | """Compute the nth number in the Fibonacci sequence.""" -6 | x = 1 - | ^ -7 | if n == 0: -8 | return 0 - | -help: Remove assignment to unused variable `x` - -F821 Undefined name `a` - --> undef.py:1:4 - | -1 | if a == 1: pass - | ^ - | diff --git a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__text__tests__fix_status_unsafe.snap b/crates/ruff_linter/src/message/snapshots/ruff_linter__message__text__tests__fix_status_unsafe.snap deleted file mode 100644 index adee375b6c..0000000000 --- a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__text__tests__fix_status_unsafe.snap +++ /dev/null @@ -1,30 +0,0 @@ ---- -source: crates/ruff_linter/src/message/text.rs -expression: content ---- -F401 [*] `os` imported but unused - --> fib.py:1:8 - | -1 | import os - | ^^ - | -help: Remove unused import: `os` - -F841 [*] Local variable `x` is assigned to but never used - --> fib.py:6:5 - | -4 | def fibonacci(n): -5 | """Compute the nth number in the Fibonacci sequence.""" -6 | x = 1 - | ^ -7 | if n == 0: -8 | return 0 - | -help: Remove assignment to unused variable `x` - -F821 Undefined name `a` - --> undef.py:1:4 - | -1 | if a == 1: pass - | ^ - | diff --git a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__text__tests__notebook_output.snap b/crates/ruff_linter/src/message/snapshots/ruff_linter__message__text__tests__notebook_output.snap deleted file mode 100644 index 969d44dc7e..0000000000 --- a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__text__tests__notebook_output.snap +++ /dev/null @@ -1,33 +0,0 @@ ---- -source: crates/ruff_linter/src/message/text.rs -expression: content ---- -F401 [*] `os` imported but unused - --> notebook.ipynb:cell 1:2:8 - | -1 | # cell 1 -2 | import os - | ^^ - | -help: Remove unused import: `os` - -F401 [*] `math` imported but unused - --> notebook.ipynb:cell 2:2:8 - | -1 | # cell 2 -2 | import math - | ^^^^ -3 | -4 | print('hello world') - | -help: Remove unused import: `math` - -F841 [*] Local variable `x` is assigned to but never used - --> notebook.ipynb:cell 3:4:5 - | -2 | def foo(): -3 | print() -4 | x = 1 - | ^ - | -help: Remove assignment to unused variable `x` diff --git a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__text__tests__syntax_errors.snap b/crates/ruff_linter/src/message/snapshots/ruff_linter__message__text__tests__syntax_errors.snap deleted file mode 100644 index af19a135dd..0000000000 --- a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__text__tests__syntax_errors.snap +++ /dev/null @@ -1,23 +0,0 @@ ---- -source: crates/ruff_linter/src/message/text.rs -expression: content ---- -invalid-syntax: Expected one or more symbol names after import - --> syntax_errors.py:1:15 - | -1 | from os import - | ^ -2 | -3 | if call(foo - | - -invalid-syntax: Expected ')', found newline - --> syntax_errors.py:3:12 - | -1 | from os import -2 | -3 | if call(foo - | ^ -4 | def bar(): -5 | pass - | diff --git a/crates/ruff_linter/src/message/text.rs b/crates/ruff_linter/src/message/text.rs deleted file mode 100644 index 4f6478266c..0000000000 --- a/crates/ruff_linter/src/message/text.rs +++ /dev/null @@ -1,143 +0,0 @@ -use std::io::Write; - -use ruff_db::diagnostic::{ - Diagnostic, DiagnosticFormat, DisplayDiagnosticConfig, DisplayDiagnostics, -}; -use ruff_diagnostics::Applicability; - -use crate::message::{Emitter, EmitterContext}; - -pub struct TextEmitter { - config: DisplayDiagnosticConfig, -} - -impl Default for TextEmitter { - fn default() -> Self { - Self { - config: DisplayDiagnosticConfig::default() - .format(DiagnosticFormat::Concise) - .hide_severity(true) - .color(!cfg!(test) && colored::control::SHOULD_COLORIZE.should_colorize()), - } - } -} - -impl TextEmitter { - #[must_use] - pub fn with_show_fix_status(mut self, show_fix_status: bool) -> Self { - self.config = self.config.show_fix_status(show_fix_status); - self - } - - #[must_use] - pub fn with_show_fix_diff(mut self, show_fix_diff: bool) -> Self { - self.config = self.config.show_fix_diff(show_fix_diff); - self - } - - #[must_use] - pub fn with_show_source(mut self, show_source: bool) -> Self { - self.config = self.config.format(if show_source { - DiagnosticFormat::Full - } else { - DiagnosticFormat::Concise - }); - self - } - - #[must_use] - pub fn with_fix_applicability(mut self, applicability: Applicability) -> Self { - self.config = self.config.fix_applicability(applicability); - self - } - - #[must_use] - pub fn with_preview(mut self, preview: bool) -> Self { - self.config = self.config.preview(preview); - self - } - - #[must_use] - pub fn with_color(mut self, color: bool) -> Self { - self.config = self.config.color(color); - self - } -} - -impl Emitter for TextEmitter { - fn emit( - &mut self, - writer: &mut dyn Write, - diagnostics: &[Diagnostic], - context: &EmitterContext, - ) -> anyhow::Result<()> { - write!( - writer, - "{}", - DisplayDiagnostics::new(context, &self.config, diagnostics) - )?; - - Ok(()) - } -} - -#[cfg(test)] -mod tests { - use insta::assert_snapshot; - use ruff_diagnostics::Applicability; - - use crate::message::TextEmitter; - use crate::message::tests::{ - capture_emitter_notebook_output, capture_emitter_output, create_diagnostics, - create_notebook_diagnostics, create_syntax_error_diagnostics, - }; - - #[test] - fn default() { - let mut emitter = TextEmitter::default().with_show_source(true); - let content = capture_emitter_output(&mut emitter, &create_diagnostics()); - - assert_snapshot!(content); - } - - #[test] - fn fix_status() { - let mut emitter = TextEmitter::default() - .with_show_fix_status(true) - .with_show_source(true); - let content = capture_emitter_output(&mut emitter, &create_diagnostics()); - - assert_snapshot!(content); - } - - #[test] - fn fix_status_unsafe() { - let mut emitter = TextEmitter::default() - .with_show_fix_status(true) - .with_show_source(true) - .with_fix_applicability(Applicability::Unsafe); - let content = capture_emitter_output(&mut emitter, &create_diagnostics()); - - assert_snapshot!(content); - } - - #[test] - fn notebook_output() { - let mut emitter = TextEmitter::default() - .with_show_fix_status(true) - .with_show_source(true) - .with_fix_applicability(Applicability::Unsafe); - let (messages, notebook_indexes) = create_notebook_diagnostics(); - let content = capture_emitter_notebook_output(&mut emitter, &messages, ¬ebook_indexes); - - assert_snapshot!(content); - } - - #[test] - fn syntax_errors() { - let mut emitter = TextEmitter::default().with_show_source(true); - let content = capture_emitter_output(&mut emitter, &create_syntax_error_diagnostics()); - - assert_snapshot!(content); - } -} diff --git a/crates/ruff_linter/src/preview.rs b/crates/ruff_linter/src/preview.rs index be0a925fc7..3cfca8379f 100644 --- a/crates/ruff_linter/src/preview.rs +++ b/crates/ruff_linter/src/preview.rs @@ -235,3 +235,8 @@ pub(crate) const fn is_a003_class_scope_shadowing_expansion_enabled( ) -> bool { settings.preview.is_enabled() } + +// https://github.com/astral-sh/ruff/pull/20200 +pub(crate) const fn is_refined_submodule_import_match_enabled(settings: &LinterSettings) -> bool { + settings.preview.is_enabled() +} diff --git a/crates/ruff_linter/src/rules/pyflakes/mod.rs b/crates/ruff_linter/src/rules/pyflakes/mod.rs index b645198b49..6f3e7bbafe 100644 --- a/crates/ruff_linter/src/rules/pyflakes/mod.rs +++ b/crates/ruff_linter/src/rules/pyflakes/mod.rs @@ -29,7 +29,7 @@ mod tests { use crate::settings::{LinterSettings, flags}; use crate::source_kind::SourceKind; use crate::test::{test_contents, test_path, test_snippet}; - use crate::{Locator, assert_diagnostics, directives}; + use crate::{Locator, assert_diagnostics, assert_diagnostics_diff, directives}; #[test_case(Rule::UnusedImport, Path::new("F401_0.py"))] #[test_case(Rule::UnusedImport, Path::new("F401_1.py"))] @@ -392,6 +392,154 @@ mod tests { Ok(()) } + #[test_case(Rule::UnusedImport, Path::new("F401_0.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_1.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_2.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_3.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_4.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_5.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_6.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_7.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_8.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_9.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_10.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_11.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_12.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_13.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_14.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_15.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_16.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_17.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_18.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_19.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_20.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_21.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_22.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_23.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_32.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_34.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_35.py"))] + fn f401_preview_refined_submodule_handling_diffs(rule_code: Rule, path: &Path) -> Result<()> { + let snapshot = format!("preview_diff__{}", path.to_string_lossy()); + assert_diagnostics_diff!( + snapshot, + Path::new("pyflakes").join(path).as_path(), + &LinterSettings::for_rule(rule_code), + &LinterSettings { + preview: PreviewMode::Enabled, + ..LinterSettings::for_rule(rule_code) + } + ); + Ok(()) + } + + #[test_case( + r" + import a + import a.b + import a.c", + "f401_multiple_unused_submodules" + )] + #[test_case( + r" + import a + import a.b + a.foo()", + "f401_use_top_member" + )] + #[test_case( + r" + import a + import a.b + a.foo() + a.bar()", + "f401_use_top_member_twice" + )] + #[test_case( + r" + # reverts to stable behavior - used between imports + import a + a.foo() + import a.b", + "f401_use_top_member_before_second_import" + )] + #[test_case( + r" + # reverts to stable behavior - used between imports + import a + a.foo() + a = 1 + import a.b", + "f401_use_top_member_and_redefine_before_second_import" + )] + #[test_case( + r" + # reverts to stable behavior - used between imports + import a + a.foo() + import a.b + a = 1", + "f401_use_top_member_then_import_then_redefine" + )] + #[test_case( + r#" + import a + import a.b + __all__ = ["a"]"#, + "f401_use_in_dunder_all" + )] + #[test_case( + r" + import a.c + import a.b + a.foo()", + "f401_import_submodules_but_use_top_level" + )] + #[test_case( + r" + import a.c + import a.b.d + a.foo()", + "f401_import_submodules_different_lengths_but_use_top_level" + )] + #[test_case( + r" + # refined logic only applied _within_ scope + import a + def foo(): + import a.b + a.foo()", + "f401_import_submodules_in_function_scope" + )] + #[test_case( + r" + # reverts to stable behavior - used between bindings + import a + a.b + import a.b", + "f401_use_in_between_imports" + )] + #[test_case( + r" + # reverts to stable behavior - used between bindings + import a.b + a + import a", + "f401_use_in_between_imports" + )] + fn f401_preview_refined_submodule_handling(contents: &str, snapshot: &str) { + let diagnostics = test_contents( + &SourceKind::Python(dedent(contents).to_string()), + Path::new("f401_preview_submodule.py"), + &LinterSettings { + preview: PreviewMode::Enabled, + ..LinterSettings::for_rule(Rule::UnusedImport) + }, + ) + .0; + assert_diagnostics!(snapshot, diagnostics); + } + #[test] fn f841_dummy_variable_rgx() -> Result<()> { let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs b/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs index 49adff0133..22bcd0cf0e 100644 --- a/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs +++ b/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs @@ -5,19 +5,22 @@ use anyhow::{Result, anyhow, bail}; use std::collections::BTreeMap; use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::name::QualifiedName; +use ruff_python_ast::name::{QualifiedName, QualifiedNameBuilder}; use ruff_python_ast::{self as ast, Stmt}; use ruff_python_semantic::{ - AnyImport, BindingKind, Exceptions, Imported, NodeId, Scope, ScopeId, SemanticModel, - SubmoduleImport, + AnyImport, Binding, BindingFlags, BindingId, BindingKind, Exceptions, Imported, NodeId, Scope, + ScopeId, SemanticModel, SubmoduleImport, }; use ruff_text_size::{Ranged, TextRange}; use crate::checkers::ast::Checker; use crate::fix; -use crate::preview::is_dunder_init_fix_unused_import_enabled; +use crate::preview::{ + is_dunder_init_fix_unused_import_enabled, is_refined_submodule_import_match_enabled, +}; use crate::registry::Rule; use crate::rules::{isort, isort::ImportSection, isort::ImportType}; +use crate::settings::LinterSettings; use crate::{Applicability, Fix, FixAvailability, Violation}; /// ## What it does @@ -49,6 +52,43 @@ use crate::{Applicability, Fix, FixAvailability, Violation}; /// __all__ = ["some_module"] /// ``` /// +/// ## Preview +/// When [preview] is enabled (and certain simplifying assumptions +/// are met), we analyze all import statements for a given module +/// when determining whether an import is used, rather than simply +/// the last of these statements. This can result in both different and +/// more import statements being marked as unused. +/// +/// For example, if a module consists of +/// +/// ```python +/// import a +/// import a.b +/// ``` +/// +/// then both statements are marked as unused under [preview], whereas +/// only the second is marked as unused under stable behavior. +/// +/// As another example, if a module consists of +/// +/// ```python +/// import a.b +/// import a +/// +/// a.b.foo() +/// ``` +/// +/// then a diagnostic will only be emitted for the first line under [preview], +/// whereas a diagnostic would only be emitted for the second line under +/// stable behavior. +/// +/// Note that this behavior is somewhat subjective and is designed +/// to conform to the developer's intuition rather than Python's actual +/// execution. To wit, the statement `import a.b` automatically executes +/// `import a`, so in some sense `import a` is _always_ redundant +/// in the presence of `import a.b`. +/// +/// /// ## Fix safety /// /// Fixes to remove unused imports are safe, except in `__init__.py` files. @@ -100,6 +140,8 @@ use crate::{Applicability, Fix, FixAvailability, Violation}; /// - [Python documentation: `import`](https://docs.python.org/3/reference/simple_stmts.html#the-import-statement) /// - [Python documentation: `importlib.util.find_spec`](https://docs.python.org/3/library/importlib.html#importlib.util.find_spec) /// - [Typing documentation: interface conventions](https://typing.python.org/en/latest/spec/distributing.html#library-interface-public-and-private-symbols) +/// +/// [preview]: https://docs.astral.sh/ruff/preview/ #[derive(ViolationMetadata)] pub(crate) struct UnusedImport { /// Qualified name of the import @@ -284,17 +326,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope) { let mut unused: BTreeMap<(NodeId, Exceptions), Vec> = BTreeMap::default(); let mut ignored: BTreeMap<(NodeId, Exceptions), Vec> = BTreeMap::default(); - for binding_id in scope.binding_ids() { - let binding = checker.semantic().binding(binding_id); - - if binding.is_used() - || binding.is_explicit_export() - || binding.is_nonlocal() - || binding.is_global() - { - continue; - } - + for binding in unused_imports_in_scope(checker.semantic(), scope, checker.settings()) { let Some(import) = binding.as_any_import() else { continue; }; @@ -586,3 +618,302 @@ fn fix_by_reexporting<'a>( let isolation = Checker::isolation(checker.semantic().parent_statement_id(node_id)); Ok(Fix::safe_edits(head, tail).isolate(isolation)) } + +/// Returns an iterator over bindings to import statements that appear unused. +/// +/// The stable behavior is to return those bindings to imports +/// satisfying the following properties: +/// +/// - they are not shadowed +/// - they are not `global`, not `nonlocal`, and not explicit exports (i.e. `import foo as foo`) +/// - they have no references, according to the semantic model +/// +/// Under preview, there is a more refined analysis performed +/// in the case where all bindings shadowed by a given import +/// binding (including the binding itself) are of a simple form: +/// they are required to be un-aliased imports or submodule imports. +/// +/// This alternative analysis is described in the documentation for +/// [`unused_imports_from_binding`]. +fn unused_imports_in_scope<'a, 'b>( + semantic: &'a SemanticModel<'b>, + scope: &'a Scope, + settings: &'a LinterSettings, +) -> impl Iterator> { + scope + .binding_ids() + .map(|id| (id, semantic.binding(id))) + .filter(|(_, bdg)| { + matches!( + bdg.kind, + BindingKind::Import(_) + | BindingKind::FromImport(_) + | BindingKind::SubmoduleImport(_) + ) + }) + .filter(|(_, bdg)| !bdg.is_global() && !bdg.is_nonlocal() && !bdg.is_explicit_export()) + .flat_map(|(id, bdg)| { + if is_refined_submodule_import_match_enabled(settings) + // No need to apply refined logic if there is only a single binding + && scope.shadowed_bindings(id).nth(1).is_some() + // Only apply the new logic in certain situations to avoid + // complexity, false positives, and intersection with + // `redefined-while-unused` (`F811`). + && has_simple_shadowed_bindings(scope, id, semantic) + { + unused_imports_from_binding(semantic, id, scope) + } else if bdg.is_used() { + vec![] + } else { + vec![bdg] + } + }) +} + +/// Returns a `Vec` of bindings to unused import statements that +/// are shadowed by a given binding. +/// +/// This is best explained by example. So suppose we have: +/// +/// ```python +/// import a +/// import a.b +/// import a.b.c +/// +/// __all__ = ["a"] +/// +/// a.b.foo() +/// ``` +/// +/// As of 2025-09-25, Ruff's semantic model, upon visiting +/// the whole module, will have a single live binding for +/// the symbol `a` that points to the line `import a.b.c`, +/// and the remaining two import bindings are considered shadowed +/// by the last. +/// +/// This function expects to receive the `id` +/// for the live binding and will begin by collecting +/// all bindings shadowed by the given one - i.e. all +/// the different import statements binding the symbol `a`. +/// We iterate over references to this +/// module and decide (somewhat subjectively) which +/// import statement the user "intends" to reference. To that end, +/// to each reference we attempt to build a [`QualifiedName`] +/// corresponding to an iterated attribute access (e.g. `a.b.foo`). +/// We then determine the closest matching import statement to that +/// qualified name, and mark it as used. +/// +/// In the present example, the qualified name associated to the +/// reference from the dunder all export is `"a"` and the qualified +/// name associated to the reference in the last line is `"a.b.foo"`. +/// The closest matches are `import a` and `import a.b`, respectively, +/// leaving `import a.b.c` unused. +/// +/// For a precise definition of "closest match" see [`best_match`] +/// and [`rank_matches`]. +/// +/// Note: if any reference comes from something other than +/// a `Name` or a dunder all expression, then we return just +/// the original binding, thus reverting the stable behavior. +fn unused_imports_from_binding<'a, 'b>( + semantic: &'a SemanticModel<'b>, + id: BindingId, + scope: &'a Scope, +) -> Vec<&'a Binding<'b>> { + let mut marked = MarkedBindings::from_binding_id(semantic, id, scope); + + let binding = semantic.binding(id); + + // ensure we only do this once + let mut marked_dunder_all = false; + + for ref_id in binding.references() { + let resolved_reference = semantic.reference(ref_id); + if !marked_dunder_all && resolved_reference.in_dunder_all_definition() { + let first = *binding + .as_any_import() + .expect("binding to be import binding since current function called after restricting to these in `unused_imports_in_scope`") + .qualified_name() + .segments().first().expect("import binding to have nonempty qualified name"); + mark_uses_of_qualified_name(&mut marked, &QualifiedName::user_defined(first)); + marked_dunder_all = true; + continue; + } + let Some(expr_id) = resolved_reference.expression_id() else { + // If there is some other kind of reference, abandon + // the refined approach for the usual one + return vec![binding]; + }; + let Some(prototype) = expand_to_qualified_name_attribute(semantic, expr_id) else { + return vec![binding]; + }; + + mark_uses_of_qualified_name(&mut marked, &prototype); + } + + marked.into_unused() +} + +#[derive(Debug)] +struct MarkedBindings<'a, 'b> { + bindings: Vec<&'a Binding<'b>>, + used: Vec, +} + +impl<'a, 'b> MarkedBindings<'a, 'b> { + fn from_binding_id(semantic: &'a SemanticModel<'b>, id: BindingId, scope: &'a Scope) -> Self { + let bindings: Vec<_> = scope + .shadowed_bindings(id) + .map(|id| semantic.binding(id)) + .collect(); + + Self { + used: vec![false; bindings.len()], + bindings, + } + } + + fn into_unused(self) -> Vec<&'a Binding<'b>> { + self.bindings + .into_iter() + .zip(self.used) + .filter_map(|(bdg, is_used)| (!is_used).then_some(bdg)) + .collect() + } + + fn iter_mut(&mut self) -> impl Iterator, &mut bool)> { + self.bindings.iter().copied().zip(self.used.iter_mut()) + } +} + +/// Returns `Some` [`QualifiedName`] delineating the path for the +/// maximal [`ExprName`] or [`ExprAttribute`] containing the expression +/// associated to the given [`NodeId`], or `None` otherwise. +/// +/// For example, if the `expr_id` points to `a` in `a.b.c.foo()` +/// then the qualified name would have segments [`a`, `b`, `c`, `foo`]. +fn expand_to_qualified_name_attribute<'b>( + semantic: &SemanticModel<'b>, + expr_id: NodeId, +) -> Option> { + let mut builder = QualifiedNameBuilder::with_capacity(16); + + let mut expr_id = expr_id; + + let expr = semantic.expression(expr_id)?; + + let name = expr.as_name_expr()?; + + builder.push(&name.id); + + while let Some(node_id) = semantic.parent_expression_id(expr_id) { + let Some(expr) = semantic.expression(node_id) else { + break; + }; + let Some(expr_attr) = expr.as_attribute_expr() else { + break; + }; + builder.push(expr_attr.attr.as_str()); + expr_id = node_id; + } + Some(builder.build()) +} + +fn mark_uses_of_qualified_name(marked: &mut MarkedBindings, prototype: &QualifiedName) { + let Some(best) = best_match(&marked.bindings, prototype) else { + return; + }; + + let Some(best_import) = best.as_any_import() else { + return; + }; + + let best_name = best_import.qualified_name(); + + // We loop through all bindings in case there are repeated instances + // of the `best_name`. For example, if we have + // + // ```python + // import a + // import a + // + // a.foo() + // ``` + // + // then we want to mark both import statements as used. It + // is the job of `redefined-while-unused` (`F811`) to catch + // the repeated binding in this case. + for (binding, is_used) in marked.iter_mut() { + if *is_used { + continue; + } + + if binding + .as_any_import() + .is_some_and(|imp| imp.qualified_name() == best_name) + { + *is_used = true; + } + } +} + +/// Returns a pair with first component the length of the largest +/// shared prefix between the qualified name of the import binding +/// and the `prototype` and second component the length of the +/// qualified name of the import binding (i.e. the number of path +/// segments). Moreover, we regard the second component as ordered +/// in reverse. +/// +/// For example, if the binding corresponds to `import a.b.c` +/// and the prototype to `a.b.foo()`, then the function returns +/// `(2,std::cmp::Reverse(3))`. +fn rank_matches(binding: &Binding, prototype: &QualifiedName) -> (usize, std::cmp::Reverse) { + let Some(import) = binding.as_any_import() else { + unreachable!() + }; + let qname = import.qualified_name(); + let left = qname + .segments() + .iter() + .zip(prototype.segments()) + .take_while(|(x, y)| x == y) + .count(); + (left, std::cmp::Reverse(qname.segments().len())) +} + +/// Returns the import binding that shares the longest prefix +/// with the `prototype` and is of minimal length amongst these. +/// +/// See also [`rank_matches`]. +fn best_match<'a, 'b>( + bindings: &Vec<&'a Binding<'b>>, + prototype: &QualifiedName, +) -> Option<&'a Binding<'b>> { + bindings + .iter() + .copied() + .max_by_key(|binding| rank_matches(binding, prototype)) +} + +#[inline] +fn has_simple_shadowed_bindings(scope: &Scope, id: BindingId, semantic: &SemanticModel) -> bool { + scope.shadowed_bindings(id).enumerate().all(|(i, shadow)| { + let shadowed_binding = semantic.binding(shadow); + // Bail if one of the shadowed bindings is + // used before the last live binding. This is + // to avoid situations like this: + // + // ``` + // import a + // a.b + // import a.b + // ``` + if i > 0 && shadowed_binding.is_used() { + return false; + } + matches!( + shadowed_binding.kind, + BindingKind::Import(_) | BindingKind::SubmoduleImport(_) + ) && !shadowed_binding.flags.contains(BindingFlags::ALIAS) + }) +} diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_import_submodules_but_use_top_level.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_import_submodules_but_use_top_level.snap new file mode 100644 index 0000000000..c4ad92dca6 --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_import_submodules_but_use_top_level.snap @@ -0,0 +1,16 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +F401 [*] `a.b` imported but unused + --> f401_preview_submodule.py:3:8 + | +2 | import a.c +3 | import a.b + | ^^^ +4 | a.foo() + | +help: Remove unused import: `a.b` +1 | +2 | import a.c + - import a.b +3 | a.foo() diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_import_submodules_different_lengths_but_use_top_level.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_import_submodules_different_lengths_but_use_top_level.snap new file mode 100644 index 0000000000..12df1553b8 --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_import_submodules_different_lengths_but_use_top_level.snap @@ -0,0 +1,16 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +F401 [*] `a.b.d` imported but unused + --> f401_preview_submodule.py:3:8 + | +2 | import a.c +3 | import a.b.d + | ^^^^^ +4 | a.foo() + | +help: Remove unused import: `a.b.d` +1 | +2 | import a.c + - import a.b.d +3 | a.foo() diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_import_submodules_in_function_scope.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_import_submodules_in_function_scope.snap new file mode 100644 index 0000000000..f99502873a --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_import_submodules_in_function_scope.snap @@ -0,0 +1,19 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +F401 [*] `a` imported but unused + --> f401_preview_submodule.py:3:8 + | +2 | # refined logic only applied _within_ scope +3 | import a + | ^ +4 | def foo(): +5 | import a.b + | +help: Remove unused import: `a` +1 | +2 | # refined logic only applied _within_ scope + - import a +3 | def foo(): +4 | import a.b +5 | a.foo() diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_multiple_unused_submodules.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_multiple_unused_submodules.snap new file mode 100644 index 0000000000..4eed530d8f --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_multiple_unused_submodules.snap @@ -0,0 +1,44 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +F401 [*] `a` imported but unused + --> f401_preview_submodule.py:2:8 + | +2 | import a + | ^ +3 | import a.b +4 | import a.c + | +help: Remove unused import: `a` +1 | + - import a +2 | import a.b +3 | import a.c + +F401 [*] `a.b` imported but unused + --> f401_preview_submodule.py:3:8 + | +2 | import a +3 | import a.b + | ^^^ +4 | import a.c + | +help: Remove unused import: `a.b` +1 | +2 | import a + - import a.b +3 | import a.c + +F401 [*] `a.c` imported but unused + --> f401_preview_submodule.py:4:8 + | +2 | import a +3 | import a.b +4 | import a.c + | ^^^ + | +help: Remove unused import: `a.c` +1 | +2 | import a +3 | import a.b + - import a.c diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_in_between_imports.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_in_between_imports.snap new file mode 100644 index 0000000000..d0b409f39e --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_in_between_imports.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- + diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_in_dunder_all.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_in_dunder_all.snap new file mode 100644 index 0000000000..58ec910de0 --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_in_dunder_all.snap @@ -0,0 +1,16 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +F401 [*] `a.b` imported but unused + --> f401_preview_submodule.py:3:8 + | +2 | import a +3 | import a.b + | ^^^ +4 | __all__ = ["a"] + | +help: Remove unused import: `a.b` +1 | +2 | import a + - import a.b +3 | __all__ = ["a"] diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_top_member.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_top_member.snap new file mode 100644 index 0000000000..de8d07c090 --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_top_member.snap @@ -0,0 +1,16 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +F401 [*] `a.b` imported but unused + --> f401_preview_submodule.py:3:8 + | +2 | import a +3 | import a.b + | ^^^ +4 | a.foo() + | +help: Remove unused import: `a.b` +1 | +2 | import a + - import a.b +3 | a.foo() diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_top_member_and_redefine_before_second_import.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_top_member_and_redefine_before_second_import.snap new file mode 100644 index 0000000000..d0b409f39e --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_top_member_and_redefine_before_second_import.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- + diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_top_member_before_second_import.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_top_member_before_second_import.snap new file mode 100644 index 0000000000..d0b409f39e --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_top_member_before_second_import.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- + diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_top_member_then_import_then_redefine.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_top_member_then_import_then_redefine.snap new file mode 100644 index 0000000000..d0b409f39e --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_top_member_then_import_then_redefine.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- + diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_top_member_twice.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_top_member_twice.snap new file mode 100644 index 0000000000..a46036a45f --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_top_member_twice.snap @@ -0,0 +1,18 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +F401 [*] `a.b` imported but unused + --> f401_preview_submodule.py:3:8 + | +2 | import a +3 | import a.b + | ^^^ +4 | a.foo() +5 | a.bar() + | +help: Remove unused import: `a.b` +1 | +2 | import a + - import a.b +3 | a.foo() +4 | a.bar() diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_0.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_0.py.snap new file mode 100644 index 0000000000..64cb811dd4 --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_0.py.snap @@ -0,0 +1,50 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 2 + +--- Added --- +F401 [*] `multiprocessing.process` imported but unused + --> F401_0.py:10:8 + | + 8 | ) + 9 | import multiprocessing.pool +10 | import multiprocessing.process + | ^^^^^^^^^^^^^^^^^^^^^^^ +11 | import logging.config +12 | import logging.handlers + | +help: Remove unused import: `multiprocessing.process` +7 | namedtuple, +8 | ) +9 | import multiprocessing.pool + - import multiprocessing.process +10 | import logging.config +11 | import logging.handlers +12 | from typing import ( + + +F401 [*] `logging.config` imported but unused + --> F401_0.py:11:8 + | + 9 | import multiprocessing.pool +10 | import multiprocessing.process +11 | import logging.config + | ^^^^^^^^^^^^^^ +12 | import logging.handlers +13 | from typing import ( + | +help: Remove unused import: `logging.config` +8 | ) +9 | import multiprocessing.pool +10 | import multiprocessing.process + - import logging.config +11 | import logging.handlers +12 | from typing import ( +13 | TYPE_CHECKING, diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_1.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_1.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_1.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_10.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_10.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_10.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_11.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_11.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_11.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_12.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_12.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_12.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_13.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_13.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_13.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_14.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_14.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_14.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_15.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_15.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_15.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_16.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_16.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_16.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_17.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_17.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_17.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_18.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_18.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_18.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_19.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_19.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_19.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_2.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_2.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_2.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_20.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_20.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_20.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_21.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_21.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_21.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_22.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_22.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_22.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_23.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_23.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_23.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_3.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_3.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_3.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_32.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_32.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_32.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_34.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_34.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_34.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_35.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_35.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_35.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_4.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_4.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_4.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_5.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_5.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_5.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_6.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_6.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_6.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_7.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_7.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_7.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_8.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_8.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_8.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_9.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_9.py.snap new file mode 100644 index 0000000000..4da451424d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_9.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +--- Linter settings --- +-linter.preview = disabled ++linter.preview = enabled + +--- Summary --- +Removed: 0 +Added: 0 diff --git a/crates/ruff_linter/src/rules/tryceratops/rules/try_consider_else.rs b/crates/ruff_linter/src/rules/tryceratops/rules/try_consider_else.rs index feb5b7eb6c..cbd24bf61e 100644 --- a/crates/ruff_linter/src/rules/tryceratops/rules/try_consider_else.rs +++ b/crates/ruff_linter/src/rules/tryceratops/rules/try_consider_else.rs @@ -29,6 +29,7 @@ use crate::checkers::ast::Checker; /// return rec /// except ZeroDivisionError: /// logging.exception("Exception occurred") +/// raise /// ``` /// /// Use instead: @@ -41,6 +42,7 @@ use crate::checkers::ast::Checker; /// rec = 1 / n /// except ZeroDivisionError: /// logging.exception("Exception occurred") +/// raise /// else: /// print(f"reciprocal of {n} is {rec}") /// return rec diff --git a/crates/ruff_linter/src/settings/types.rs b/crates/ruff_linter/src/settings/types.rs index a72e80284a..760422a0ce 100644 --- a/crates/ruff_linter/src/settings/types.rs +++ b/crates/ruff_linter/src/settings/types.rs @@ -9,6 +9,7 @@ use anyhow::{Context, Result, bail}; use globset::{Glob, GlobMatcher, GlobSet, GlobSetBuilder}; use log::debug; use pep440_rs::{VersionSpecifier, VersionSpecifiers}; +use ruff_db::diagnostic::DiagnosticFormat; use rustc_hash::FxHashMap; use serde::{Deserialize, Deserializer, Serialize, de}; use strum_macros::EnumIter; @@ -553,6 +554,34 @@ impl Display for OutputFormat { } } +/// The subset of output formats only implemented in Ruff, not in `ruff_db` via `DisplayDiagnostics`. +pub enum RuffOutputFormat { + Github, + Grouped, + Sarif, +} + +impl TryFrom for DiagnosticFormat { + type Error = RuffOutputFormat; + + fn try_from(format: OutputFormat) -> std::result::Result { + match format { + OutputFormat::Concise => Ok(DiagnosticFormat::Concise), + OutputFormat::Full => Ok(DiagnosticFormat::Full), + OutputFormat::Json => Ok(DiagnosticFormat::Json), + OutputFormat::JsonLines => Ok(DiagnosticFormat::JsonLines), + OutputFormat::Junit => Ok(DiagnosticFormat::Junit), + OutputFormat::Gitlab => Ok(DiagnosticFormat::Gitlab), + OutputFormat::Pylint => Ok(DiagnosticFormat::Pylint), + OutputFormat::Rdjson => Ok(DiagnosticFormat::Rdjson), + OutputFormat::Azure => Ok(DiagnosticFormat::Azure), + OutputFormat::Github => Err(RuffOutputFormat::Github), + OutputFormat::Grouped => Err(RuffOutputFormat::Grouped), + OutputFormat::Sarif => Err(RuffOutputFormat::Sarif), + } + } +} + #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, Hash)] #[serde(try_from = "String")] pub struct RequiredVersion(VersionSpecifiers); diff --git a/crates/ruff_linter/src/test.rs b/crates/ruff_linter/src/test.rs index b63e23fc87..2ebb244d62 100644 --- a/crates/ruff_linter/src/test.rs +++ b/crates/ruff_linter/src/test.rs @@ -10,7 +10,9 @@ use anyhow::Result; use itertools::Itertools; use rustc_hash::FxHashMap; -use ruff_db::diagnostic::{Diagnostic, Span}; +use ruff_db::diagnostic::{ + Diagnostic, DiagnosticFormat, DisplayDiagnosticConfig, DisplayDiagnostics, Span, +}; use ruff_notebook::Notebook; #[cfg(not(fuzzing))] use ruff_notebook::NotebookError; @@ -24,7 +26,7 @@ use ruff_source_file::SourceFileBuilder; use crate::codes::Rule; use crate::fix::{FixResult, fix_file}; use crate::linter::check_path; -use crate::message::{Emitter, EmitterContext, TextEmitter, create_syntax_error_diagnostic}; +use crate::message::{EmitterContext, create_syntax_error_diagnostic}; use crate::package::PackageRoot; use crate::packaging::detect_package_root; use crate::settings::types::UnsafeFixes; @@ -444,42 +446,38 @@ pub(crate) fn print_jupyter_messages( path: &Path, notebook: &Notebook, ) -> String { - let mut output = Vec::new(); - - TextEmitter::default() + let config = DisplayDiagnosticConfig::default() + .format(DiagnosticFormat::Full) + .hide_severity(true) .with_show_fix_status(true) - .with_show_fix_diff(true) - .with_show_source(true) - .with_fix_applicability(Applicability::DisplayOnly) - .emit( - &mut output, - diagnostics, - &EmitterContext::new(&FxHashMap::from_iter([( - path.file_name().unwrap().to_string_lossy().to_string(), - notebook.index().clone(), - )])), - ) - .unwrap(); + .show_fix_diff(true) + .with_fix_applicability(Applicability::DisplayOnly); - String::from_utf8(output).unwrap() + DisplayDiagnostics::new( + &EmitterContext::new(&FxHashMap::from_iter([( + path.file_name().unwrap().to_string_lossy().to_string(), + notebook.index().clone(), + )])), + &config, + diagnostics, + ) + .to_string() } pub(crate) fn print_messages(diagnostics: &[Diagnostic]) -> String { - let mut output = Vec::new(); - - TextEmitter::default() + let config = DisplayDiagnosticConfig::default() + .format(DiagnosticFormat::Full) + .hide_severity(true) .with_show_fix_status(true) - .with_show_fix_diff(true) - .with_show_source(true) - .with_fix_applicability(Applicability::DisplayOnly) - .emit( - &mut output, - diagnostics, - &EmitterContext::new(&FxHashMap::default()), - ) - .unwrap(); + .show_fix_diff(true) + .with_fix_applicability(Applicability::DisplayOnly); - String::from_utf8(output).unwrap() + DisplayDiagnostics::new( + &EmitterContext::new(&FxHashMap::default()), + &config, + diagnostics, + ) + .to_string() } #[macro_export] diff --git a/crates/ruff_python_ast/src/nodes.rs b/crates/ruff_python_ast/src/nodes.rs index 27cfc06da7..b53582218f 100644 --- a/crates/ruff_python_ast/src/nodes.rs +++ b/crates/ruff_python_ast/src/nodes.rs @@ -3030,6 +3030,12 @@ impl Parameters { .find(|arg| arg.parameter.name.as_str() == name) } + /// Returns the index of the parameter with the given name + pub fn index(&self, name: &str) -> Option { + self.iter_non_variadic_params() + .position(|arg| arg.parameter.name.as_str() == name) + } + /// Returns an iterator over all parameters included in this [`Parameters`] node. pub fn iter(&self) -> ParametersIterator<'_> { ParametersIterator::new(self) diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index b8de310a36..4f2dc47162 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -2101,7 +2101,7 @@ impl<'a> SemanticModel<'a> { /// Finds and returns the [`Scope`] corresponding to a given [`ast::StmtFunctionDef`]. /// /// This method searches all scopes created by a function definition, comparing the - /// [`TextRange`] of the provided `function_def` with the the range of the function + /// [`TextRange`] of the provided `function_def` with the range of the function /// associated with the scope. pub fn function_scope(&self, function_def: &ast::StmtFunctionDef) -> Option<&Scope<'_>> { self.scopes.iter().find(|scope| { diff --git a/crates/ty/tests/cli/python_environment.rs b/crates/ty/tests/cli/python_environment.rs index 062dbbef9b..483379368a 100644 --- a/crates/ty/tests/cli/python_environment.rs +++ b/crates/ty/tests/cli/python_environment.rs @@ -188,6 +188,40 @@ fn config_file_annotation_showing_where_python_version_set_typing_error() -> any Ok(()) } +/// If `.` and `./src` are both registered as first-party search paths, +/// the `./src` directory should take precedence for module resolution, +/// because it is relative to `.`. +#[test] +fn src_subdirectory_takes_precedence_over_repo_root() -> anyhow::Result<()> { + let case = CliTest::with_files([( + "src/package/__init__.py", + "from . import nonexistent_submodule", + )])?; + + // If `./src` didn't take priority over `.` here, we would report + // "Module `src.package` has no member `nonexistent_submodule`" + // instead of "Module `package` has no member `nonexistent_submodule`". + assert_cmd_snapshot!(case.command(), @r" + success: false + exit_code: 1 + ----- stdout ----- + error[unresolved-import]: Module `package` has no member `nonexistent_submodule` + --> src/package/__init__.py:1:15 + | + 1 | from . import nonexistent_submodule + | ^^^^^^^^^^^^^^^^^^^^^ + | + info: rule `unresolved-import` is enabled by default + + Found 1 diagnostic + + ----- stderr ----- + WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors. + "); + + Ok(()) +} + /// This tests that, even if no Python *version* has been specified on the CLI or in a config file, /// ty is still able to infer the Python version from a `--python` argument on the CLI, /// *even if* the `--python` argument points to a system installation. @@ -1738,8 +1772,8 @@ fn default_root_tests_package() -> anyhow::Result<()> { 5 | print(f"{foo} {bar}") | info: Searched in the following paths during module resolution: - info: 1. / (first-party code) - info: 2. /src (first-party code) + info: 1. /src (first-party code) + info: 2. / (first-party code) info: 3. vendored://stdlib (stdlib typeshed stubs vendored by ty) info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment info: rule `unresolved-import` is enabled by default @@ -1814,8 +1848,8 @@ fn default_root_python_package() -> anyhow::Result<()> { 5 | print(f"{foo} {bar}") | info: Searched in the following paths during module resolution: - info: 1. / (first-party code) - info: 2. /src (first-party code) + info: 1. /src (first-party code) + info: 2. / (first-party code) info: 3. vendored://stdlib (stdlib typeshed stubs vendored by ty) info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment info: rule `unresolved-import` is enabled by default @@ -1861,8 +1895,8 @@ fn default_root_python_package_pyi() -> anyhow::Result<()> { 5 | print(f"{foo} {bar}") | info: Searched in the following paths during module resolution: - info: 1. / (first-party code) - info: 2. /src (first-party code) + info: 1. /src (first-party code) + info: 2. / (first-party code) info: 3. vendored://stdlib (stdlib typeshed stubs vendored by ty) info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment info: rule `unresolved-import` is enabled by default @@ -1902,8 +1936,8 @@ fn pythonpath_is_respected() -> anyhow::Result<()> { 3 | print(f"{baz.it}") | info: Searched in the following paths during module resolution: - info: 1. / (first-party code) - info: 2. /src (first-party code) + info: 1. /src (first-party code) + info: 2. / (first-party code) info: 3. vendored://stdlib (stdlib typeshed stubs vendored by ty) info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment info: rule `unresolved-import` is enabled by default @@ -1959,8 +1993,8 @@ fn pythonpath_multiple_dirs_is_respected() -> anyhow::Result<()> { 3 | import foo | info: Searched in the following paths during module resolution: - info: 1. / (first-party code) - info: 2. /src (first-party code) + info: 1. /src (first-party code) + info: 2. / (first-party code) info: 3. vendored://stdlib (stdlib typeshed stubs vendored by ty) info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment info: rule `unresolved-import` is enabled by default @@ -1975,8 +2009,8 @@ fn pythonpath_multiple_dirs_is_respected() -> anyhow::Result<()> { 5 | print(f"{baz.it}") | info: Searched in the following paths during module resolution: - info: 1. / (first-party code) - info: 2. /src (first-party code) + info: 1. /src (first-party code) + info: 2. / (first-party code) info: 3. vendored://stdlib (stdlib typeshed stubs vendored by ty) info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment info: rule `unresolved-import` is enabled by default diff --git a/crates/ty_ide/src/completion.rs b/crates/ty_ide/src/completion.rs index 24c7f946af..32c3ef02e1 100644 --- a/crates/ty_ide/src/completion.rs +++ b/crates/ty_ide/src/completion.rs @@ -688,7 +688,7 @@ fn import_from_tokens(tokens: &[Token]) -> Option<&Token> { /// This also handles cases like `import foo, c, bar`. /// /// If found, a token corresponding to the `import` or `from` keyword -/// and the the closest point of the `` is returned. +/// and the closest point of the `` is returned. /// /// It is assumed that callers will call `from_import_tokens` first to /// try and recognize a `from ... import ...` statement before using diff --git a/crates/ty_ide/src/importer.rs b/crates/ty_ide/src/importer.rs index bbea834f30..bbc5281ddf 100644 --- a/crates/ty_ide/src/importer.rs +++ b/crates/ty_ide/src/importer.rs @@ -123,7 +123,7 @@ impl<'a> Importer<'a> { /// then the existing style is always respected instead. /// /// `members` should be a map of symbols in scope at the position - /// where the the imported symbol should be available. This is used + /// where the imported symbol should be available. This is used /// to craft import statements in a way that doesn't conflict with /// symbols in scope. If it's not feasible to provide this map, then /// providing an empty map is generally fine. But it does mean that diff --git a/crates/ty_project/src/glob.rs b/crates/ty_project/src/glob.rs index 7127f55f41..81842f4948 100644 --- a/crates/ty_project/src/glob.rs +++ b/crates/ty_project/src/glob.rs @@ -10,7 +10,7 @@ mod exclude; mod include; mod portable; -/// Path filtering based on an an exclude and include glob pattern set. +/// Path filtering based on an exclude and include glob pattern set. /// /// Exclude patterns take precedence over includes. #[derive(Clone, Debug, Eq, PartialEq, get_size2::GetSize)] diff --git a/crates/ty_project/src/metadata/options.rs b/crates/ty_project/src/metadata/options.rs index 9bcc0067c4..c0e0cfeb91 100644 --- a/crates/ty_project/src/metadata/options.rs +++ b/crates/ty_project/src/metadata/options.rs @@ -237,15 +237,16 @@ impl Options { .map(|root| root.absolute(project_root, system)) .collect() } else { + let mut roots = vec![]; let src = project_root.join("src"); - let mut roots = if system.is_directory(&src) { + if system.is_directory(&src) { // Default to `src` and the project root if `src` exists and the root hasn't been specified. // This corresponds to the `src-layout` tracing::debug!( "Including `.` and `./src` in `environment.root` because a `./src` directory exists" ); - vec![project_root.to_path_buf(), src] + roots.push(src); } else if system.is_directory(&project_root.join(project_name).join(project_name)) { // `src-layout` but when the folder isn't called `src` but has the same name as the project. // For example, the "src" folder for `psycopg` is called `psycopg` and the python files are in `psycopg/psycopg/_adapters_map.py` @@ -253,12 +254,11 @@ impl Options { "Including `.` and `/{project_name}` in `environment.root` because a `./{project_name}/{project_name}` directory exists" ); - vec![project_root.to_path_buf(), project_root.join(project_name)] + roots.push(project_root.join(project_name)); } else { // Default to a [flat project structure](https://packaging.python.org/en/latest/discussions/src-layout-vs-flat-layout/). tracing::debug!("Including `.` in `environment.root`"); - vec![project_root.to_path_buf()] - }; + } let python = project_root.join("python"); if system.is_directory(&python) @@ -293,6 +293,10 @@ impl Options { roots.push(tests_dir); } + // The project root should always be included, and should always come + // after any subdirectories such as `./src`, `./tests` and/or `./python`. + roots.push(project_root.to_path_buf()); + roots }; diff --git a/crates/ty_python_semantic/resources/mdtest/annotations/self.md b/crates/ty_python_semantic/resources/mdtest/annotations/self.md index 1b42dde0b5..be8b660eb9 100644 --- a/crates/ty_python_semantic/resources/mdtest/annotations/self.md +++ b/crates/ty_python_semantic/resources/mdtest/annotations/self.md @@ -33,11 +33,6 @@ class Shape: reveal_type(x) # revealed: Self@nested_func_without_enclosing_binding inner(self) - def implicit_self(self) -> Self: - # TODO: first argument in a method should be considered as "typing.Self" - reveal_type(self) # revealed: Unknown - return self - reveal_type(Shape().nested_type()) # revealed: list[Shape] reveal_type(Shape().nested_func()) # revealed: Shape @@ -53,6 +48,150 @@ class Outer: return self ``` +## Type of (unannotated) `self` parameters + +In instance methods, the first parameter (regardless of its name) is assumed to have the type +`typing.Self`, unless it has an explicit annotation. This does not apply to `@classmethod` and +`@staticmethod`s. + +```toml +[environment] +python-version = "3.11" +``` + +```py +from typing import Self + +class A: + def implicit_self(self) -> Self: + # TODO: This should be Self@implicit_self + reveal_type(self) # revealed: Unknown + + return self + + def a_method(self) -> int: + def first_arg_is_not_self(a: int) -> int: + reveal_type(a) # revealed: int + return a + return first_arg_is_not_self(1) + + @classmethod + def a_classmethod(cls) -> Self: + # TODO: This should be type[Self@bar] + reveal_type(cls) # revealed: Unknown + return cls() + + @staticmethod + def a_staticmethod(x: int): ... + +a = A() + +reveal_type(a.implicit_self()) # revealed: A +reveal_type(a.implicit_self) # revealed: bound method A.implicit_self() -> A +``` + +Calling an instance method explicitly verifies the first argument: + +```py +A.implicit_self(a) + +# error: [invalid-argument-type] "Argument to function `implicit_self` is incorrect: Argument type `Literal[1]` does not satisfy upper bound `A` of type variable `Self`" +A.implicit_self(1) +``` + +Passing `self` implicitly also verifies the type: + +```py +from typing import Never + +class Strange: + def can_not_be_called(self: Never) -> None: ... + +# error: [invalid-argument-type] "Argument to bound method `can_not_be_called` is incorrect: Expected `Never`, found `Strange`" +Strange().can_not_be_called() +``` + +If the method is a class or static method then first argument is not inferred as `Self`: + +```py +A.a_classmethod() +A.a_classmethod(a) # error: [too-many-positional-arguments] +A.a_staticmethod(1) +a.a_staticmethod(1) +A.a_staticmethod(a) # error: [invalid-argument-type] +``` + +The first parameter of instance methods always has type `Self`, if it is not explicitly annotated. +The name `self` is not special in any way. + +```py +class B: + def name_does_not_matter(this) -> Self: + # TODO: Should reveal Self@name_does_not_matter + reveal_type(this) # revealed: Unknown + + return this + + def positional_only(self, /, x: int) -> Self: + # TODO: Should reveal Self@positional_only + reveal_type(self) # revealed: Unknown + return self + + def keyword_only(self, *, x: int) -> Self: + # TODO: Should reveal Self@keyword_only + reveal_type(self) # revealed: Unknown + return self + + @property + def a_property(self) -> Self: + # TODO: Should reveal Self@a_property + reveal_type(self) # revealed: Unknown + return self + +reveal_type(B().name_does_not_matter()) # revealed: B +reveal_type(B().positional_only(1)) # revealed: B +reveal_type(B().keyword_only(x=1)) # revealed: B + +# TODO: this should be B +reveal_type(B().a_property) # revealed: Unknown +``` + +This also works for generic classes: + +```py +from typing import Self, Generic, TypeVar + +T = TypeVar("T") + +class G(Generic[T]): + def id(self) -> Self: + # TODO: Should reveal Self@id + reveal_type(self) # revealed: Unknown + + return self + +reveal_type(G[int]().id()) # revealed: G[int] +reveal_type(G[str]().id()) # revealed: G[str] +``` + +Free functions and nested functions do not use implicit `Self`: + +```py +def not_a_method(self): + reveal_type(self) # revealed: Unknown + +# error: [invalid-type-form] +def does_not_return_self(self) -> Self: + return self + +class C: + def outer(self) -> None: + def inner(self): + reveal_type(self) # revealed: Unknown + +reveal_type(not_a_method) # revealed: def not_a_method(self) -> Unknown +``` + ## typing_extensions ```toml @@ -208,6 +347,47 @@ class MyMetaclass(type): return super().__new__(cls) ``` +## Explicit annotations override implicit `Self` + +If the first parameter is explicitly annotated, that annotation takes precedence over the implicit +`Self` type. + +```toml +[environment] +python-version = "3.12" +``` + +```py +from __future__ import annotations + +from typing import final + +@final +class Disjoint: ... + +class Explicit: + # TODO: We could emit a warning if the annotated type of `self` is disjoint from `Explicit` + def bad(self: Disjoint) -> None: + reveal_type(self) # revealed: Disjoint + + def forward(self: Explicit) -> None: + reveal_type(self) # revealed: Explicit + +# error: [invalid-argument-type] "Argument to bound method `bad` is incorrect: Expected `Disjoint`, found `Explicit`" +Explicit().bad() + +Explicit().forward() + +class ExplicitGeneric[T]: + def special(self: ExplicitGeneric[int]) -> None: + reveal_type(self) # revealed: ExplicitGeneric[int] + +ExplicitGeneric[int]().special() + +# TODO: this should be an `invalid-argument-type` error +ExplicitGeneric[str]().special() +``` + ## Binding a method fixes `Self` When a method is bound, any instances of `Self` in its signature are "fixed", since we now know the diff --git a/crates/ty_python_semantic/resources/mdtest/call/methods.md b/crates/ty_python_semantic/resources/mdtest/call/methods.md index 68c2175e6f..f7bc04fba9 100644 --- a/crates/ty_python_semantic/resources/mdtest/call/methods.md +++ b/crates/ty_python_semantic/resources/mdtest/call/methods.md @@ -69,7 +69,9 @@ reveal_type(bound_method(1)) # revealed: str When we call the function object itself, we need to pass the `instance` explicitly: ```py -C.f(1) # error: [missing-argument] +# error: [invalid-argument-type] "Argument to function `f` is incorrect: Expected `C`, found `Literal[1]`" +# error: [missing-argument] +C.f(1) reveal_type(C.f(C(), 1)) # revealed: str ``` diff --git a/crates/ty_python_semantic/resources/mdtest/descriptor_protocol.md b/crates/ty_python_semantic/resources/mdtest/descriptor_protocol.md index ed80839a76..9210c84483 100644 --- a/crates/ty_python_semantic/resources/mdtest/descriptor_protocol.md +++ b/crates/ty_python_semantic/resources/mdtest/descriptor_protocol.md @@ -431,6 +431,8 @@ def _(flag: bool): reveal_type(C7.union_of_class_data_descriptor_and_attribute) # revealed: Literal["data", 2] C7.union_of_metaclass_attributes = 2 if flag else 1 + # TODO: https://github.com/astral-sh/ty/issues/1163 + # error: [invalid-assignment] C7.union_of_metaclass_data_descriptor_and_attribute = 2 if flag else 100 C7.union_of_class_attributes = 2 if flag else 1 C7.union_of_class_data_descriptor_and_attribute = 2 if flag else DataDescriptor() diff --git a/crates/ty_python_semantic/resources/mdtest/diagnostics/same_names.md b/crates/ty_python_semantic/resources/mdtest/diagnostics/same_names.md index 1c4593fc6e..6e6dfac60d 100644 --- a/crates/ty_python_semantic/resources/mdtest/diagnostics/same_names.md +++ b/crates/ty_python_semantic/resources/mdtest/diagnostics/same_names.md @@ -43,8 +43,7 @@ import b df: a.DataFrame = b.DataFrame() # error: [invalid-assignment] "Object of type `b.DataFrame` is not assignable to `a.DataFrame`" def _(dfs: list[b.DataFrame]): - # TODO should be"Object of type `list[b.DataFrame]` is not assignable to `list[a.DataFrame]` - # error: [invalid-assignment] "Object of type `list[DataFrame]` is not assignable to `list[DataFrame]`" + # error: [invalid-assignment] "Object of type `list[b.DataFrame]` is not assignable to `list[a.DataFrame]`" dataframes: list[a.DataFrame] = dfs ``` @@ -171,6 +170,36 @@ class Container(Generic[T]): ## Protocols +### Differing members + +`bad.py`: + +```py +from typing import Protocol, TypeVar + +T_co = TypeVar("T_co", covariant=True) + +class Iterator(Protocol[T_co]): + def __nexxt__(self) -> T_co: ... + +def bad() -> Iterator[str]: + raise NotImplementedError +``` + +`main.py`: + +```py +from typing import Iterator + +def f() -> Iterator[str]: + import bad + + # error: [invalid-return-type] "Return type does not match returned value: expected `typing.Iterator[str]`, found `bad.Iterator[str]" + return bad.bad() +``` + +### Same members but with different types + ```py from typing import Protocol import proto_a @@ -228,3 +257,21 @@ from typing import TypedDict class Person(TypedDict): name: bytes ``` + +## Tuple specializations + +`module.py`: + +```py +class Model: ... +``` + +```py +class Model: ... + +def get_models_tuple() -> tuple[Model]: + from module import Model + + # error: [invalid-return-type] "Return type does not match returned value: expected `tuple[mdtest_snippet.Model]`, found `tuple[module.Model]`" + return (Model(),) +``` diff --git a/crates/ty_python_semantic/resources/mdtest/generics/legacy/classes.md b/crates/ty_python_semantic/resources/mdtest/generics/legacy/classes.md index 234de56a8c..17e12d9150 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/legacy/classes.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/legacy/classes.md @@ -562,17 +562,17 @@ class C(Generic[T]): return u reveal_type(generic_context(C)) # revealed: tuple[T@C] -reveal_type(generic_context(C.method)) # revealed: None -reveal_type(generic_context(C.generic_method)) # revealed: tuple[U@generic_method] +reveal_type(generic_context(C.method)) # revealed: tuple[Self@method] +reveal_type(generic_context(C.generic_method)) # revealed: tuple[Self@generic_method, U@generic_method] reveal_type(generic_context(C[int])) # revealed: None -reveal_type(generic_context(C[int].method)) # revealed: None -reveal_type(generic_context(C[int].generic_method)) # revealed: tuple[U@generic_method] +reveal_type(generic_context(C[int].method)) # revealed: tuple[Self@method] +reveal_type(generic_context(C[int].generic_method)) # revealed: tuple[Self@generic_method, U@generic_method] c: C[int] = C[int]() reveal_type(c.generic_method(1, "string")) # revealed: Literal["string"] reveal_type(generic_context(c)) # revealed: None -reveal_type(generic_context(c.method)) # revealed: None -reveal_type(generic_context(c.generic_method)) # revealed: tuple[U@generic_method] +reveal_type(generic_context(c.method)) # revealed: tuple[Self@method] +reveal_type(generic_context(c.generic_method)) # revealed: tuple[Self@generic_method, U@generic_method] ``` ## Specializations propagate diff --git a/crates/ty_python_semantic/resources/mdtest/generics/legacy/functions.md b/crates/ty_python_semantic/resources/mdtest/generics/legacy/functions.md index 42941acac3..346346e686 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/legacy/functions.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/legacy/functions.md @@ -464,6 +464,7 @@ def f(x: str): from typing import TypeVar, overload T = TypeVar("T") +S = TypeVar("S") def outer(t: T) -> None: def inner(t: T) -> None: ... @@ -479,6 +480,13 @@ def overloaded_outer(t: T | None = None) -> None: if t is not None: inner(t) + +def outer(t: T) -> None: + def inner(inner_t: T, s: S) -> tuple[T, S]: + return inner_t, s + reveal_type(inner(t, 1)) # revealed: tuple[T@outer, Literal[1]] + + inner("wrong", 1) # error: [invalid-argument-type] ``` ## Unpacking a TypeVar diff --git a/crates/ty_python_semantic/resources/mdtest/generics/pep695/classes.md b/crates/ty_python_semantic/resources/mdtest/generics/pep695/classes.md index 73607224c9..c71621f0ea 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/pep695/classes.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/pep695/classes.md @@ -504,17 +504,17 @@ class C[T]: def cannot_shadow_class_typevar[T](self, t: T): ... reveal_type(generic_context(C)) # revealed: tuple[T@C] -reveal_type(generic_context(C.method)) # revealed: None -reveal_type(generic_context(C.generic_method)) # revealed: tuple[U@generic_method] +reveal_type(generic_context(C.method)) # revealed: tuple[Self@method] +reveal_type(generic_context(C.generic_method)) # revealed: tuple[Self@generic_method, U@generic_method] reveal_type(generic_context(C[int])) # revealed: None -reveal_type(generic_context(C[int].method)) # revealed: None -reveal_type(generic_context(C[int].generic_method)) # revealed: tuple[U@generic_method] +reveal_type(generic_context(C[int].method)) # revealed: tuple[Self@method] +reveal_type(generic_context(C[int].generic_method)) # revealed: tuple[Self@generic_method, U@generic_method] c: C[int] = C[int]() reveal_type(c.generic_method(1, "string")) # revealed: Literal["string"] reveal_type(generic_context(c)) # revealed: None -reveal_type(generic_context(c.method)) # revealed: None -reveal_type(generic_context(c.generic_method)) # revealed: tuple[U@generic_method] +reveal_type(generic_context(c.method)) # revealed: tuple[Self@method] +reveal_type(generic_context(c.generic_method)) # revealed: tuple[Self@generic_method, U@generic_method] ``` ## Specializations propagate diff --git a/crates/ty_python_semantic/resources/mdtest/generics/pep695/functions.md b/crates/ty_python_semantic/resources/mdtest/generics/pep695/functions.md index 0a70d9ae3f..b99e311305 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/pep695/functions.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/pep695/functions.md @@ -474,6 +474,13 @@ def overloaded_outer[T](t: T | None = None) -> None: if t is not None: inner(t) + +def outer[T](t: T) -> None: + def inner[S](inner_t: T, s: S) -> tuple[T, S]: + return inner_t, s + reveal_type(inner(t, 1)) # revealed: tuple[T@outer, Literal[1]] + + inner("wrong", 1) # error: [invalid-argument-type] ``` ## Unpacking a TypeVar @@ -534,6 +541,5 @@ class C: def _(x: int): reveal_type(C().explicit_self(x)) # revealed: tuple[C, int] - # TODO: this should be `tuple[C, int]` as well, once we support implicit `self` - reveal_type(C().implicit_self(x)) # revealed: tuple[Unknown, int] + reveal_type(C().implicit_self(x)) # revealed: tuple[C, int] ``` diff --git a/crates/ty_python_semantic/resources/mdtest/generics/scoping.md b/crates/ty_python_semantic/resources/mdtest/generics/scoping.md index 02142d006b..fb8b74428c 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/scoping.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/scoping.md @@ -117,6 +117,7 @@ reveal_type(bound_method.__func__) # revealed: def f(self, x: int) -> str reveal_type(C[int]().f(1)) # revealed: str reveal_type(bound_method(1)) # revealed: str +# error: [invalid-argument-type] "Argument to function `f` is incorrect: Argument type `Literal[1]` does not satisfy upper bound `C[T@C]` of type variable `Self`" C[int].f(1) # error: [missing-argument] reveal_type(C[int].f(C[int](), 1)) # revealed: str @@ -154,7 +155,7 @@ from ty_extensions import generic_context legacy.m("string", None) # error: [invalid-argument-type] reveal_type(legacy.m) # revealed: bound method Legacy[int].m[S](x: int, y: S@m) -> S@m reveal_type(generic_context(Legacy)) # revealed: tuple[T@Legacy] -reveal_type(generic_context(legacy.m)) # revealed: tuple[S@m] +reveal_type(generic_context(legacy.m)) # revealed: tuple[Self@m, S@m] ``` With PEP 695 syntax, it is clearer that the method uses a separate typevar: diff --git a/crates/ty_python_semantic/resources/mdtest/named_tuple.md b/crates/ty_python_semantic/resources/mdtest/named_tuple.md index 0159b03a6f..3a2b7ce14b 100644 --- a/crates/ty_python_semantic/resources/mdtest/named_tuple.md +++ b/crates/ty_python_semantic/resources/mdtest/named_tuple.md @@ -278,8 +278,7 @@ reveal_type(Person._make(("Alice", 42))) # revealed: Unknown person = Person("Alice", 42) reveal_type(person._asdict()) # revealed: dict[str, Any] -# TODO: should be `Person` once we support implicit type of `self` -reveal_type(person._replace(name="Bob")) # revealed: Unknown +reveal_type(person._replace(name="Bob")) # revealed: Person ``` When accessing them on child classes of generic `NamedTuple`s, the return type is specialized @@ -296,8 +295,7 @@ class Box(NamedTuple, Generic[T]): class IntBox(Box[int]): pass -# TODO: should be `IntBox` once we support the implicit type of `self` -reveal_type(IntBox(1)._replace(content=42)) # revealed: Unknown +reveal_type(IntBox(1)._replace(content=42)) # revealed: IntBox ``` ## `collections.namedtuple` diff --git a/crates/ty_python_semantic/resources/mdtest/narrow/isinstance.md b/crates/ty_python_semantic/resources/mdtest/narrow/isinstance.md index 486c354c24..60ec2fa844 100644 --- a/crates/ty_python_semantic/resources/mdtest/narrow/isinstance.md +++ b/crates/ty_python_semantic/resources/mdtest/narrow/isinstance.md @@ -324,8 +324,7 @@ a covariant generic, this is equivalent to using the upper bound of the type par from typing import Self class Covariant[T]: - # TODO: remove the explicit `Self` annotation, once we support the implicit type of `self` - def get(self: Self) -> T: + def get(self) -> T: raise NotImplementedError def _(x: object): @@ -338,8 +337,7 @@ Similarly, contravariant type parameters use their lower bound of `Never`: ```py class Contravariant[T]: - # TODO: remove the explicit `Self` annotation, once we support the implicit type of `self` - def push(self: Self, x: T) -> None: ... + def push(self, x: T) -> None: ... def _(x: object): if isinstance(x, Contravariant): @@ -354,10 +352,8 @@ the type system, so we represent it with the internal `Top[]` special form. ```py class Invariant[T]: - # TODO: remove the explicit `Self` annotation, once we support the implicit type of `self` - def push(self: Self, x: T) -> None: ... - # TODO: remove the explicit `Self` annotation, once we support the implicit type of `self` - def get(self: Self) -> T: + def push(self, x: T) -> None: ... + def get(self) -> T: raise NotImplementedError def _(x: object): diff --git a/crates/ty_python_semantic/resources/mdtest/narrow/type_guards.md b/crates/ty_python_semantic/resources/mdtest/narrow/type_guards.md index 5d9bcda7c0..bd5fc0705d 100644 --- a/crates/ty_python_semantic/resources/mdtest/narrow/type_guards.md +++ b/crates/ty_python_semantic/resources/mdtest/narrow/type_guards.md @@ -173,6 +173,11 @@ def _(d: Any): ## Narrowing +```toml +[environment] +python-version = "3.12" +``` + ```py from typing import Any from typing_extensions import TypeGuard, TypeIs @@ -295,6 +300,38 @@ def _(a: Foo): reveal_type(a) # revealed: Foo & Bar ``` +For generics, we transform the argument passed into `TypeIs[]` from `X` to `Top[X]`. This helps +especially when using various functions from typeshed that are annotated as returning +`TypeIs[SomeCovariantGeneric[Any]]` to avoid false positives in other type checkers. For ty's +purposes, it would usually lead to more intuitive results if `object` was used as the specialization +for a covariant generic inside the `TypeIs` special form, but this is mitigated by our implicit +transformation from `TypeIs[SomeCovariantGeneric[Any]]` to `TypeIs[Top[SomeCovariantGeneric[Any]]]` +(which just simplifies to `TypeIs[SomeCovariantGeneric[object]]`). + +```py +class Unrelated: ... + +class Covariant[T]: + def get(self) -> T: + raise NotImplementedError + +def is_instance_of_covariant(arg: object) -> TypeIs[Covariant[Any]]: + return isinstance(arg, Covariant) + +def needs_instance_of_unrelated(arg: Unrelated): + pass + +def _(x: Unrelated | Covariant[int]): + if is_instance_of_covariant(x): + raise RuntimeError("oh no") + + reveal_type(x) # revealed: Unrelated & ~Covariant[object] + + # We would emit a false-positive diagnostic here if we didn't implicitly transform + # `TypeIs[Covariant[Any]]` to `TypeIs[Covariant[object]]` + needs_instance_of_unrelated(x) +``` + ## `TypeGuard` special cases ```py 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 b3b1d15ef8..9821987281 100644 --- a/crates/ty_python_semantic/resources/mdtest/pep695_type_aliases.md +++ b/crates/ty_python_semantic/resources/mdtest/pep695_type_aliases.md @@ -325,7 +325,7 @@ type A = list[Union["A", str]] def f(x: A): reveal_type(x) # revealed: list[A | str] for item in x: - reveal_type(item) # revealed: list[A | str] | str + reveal_type(item) # revealed: list[Any | str] | str ``` #### With new-style union @@ -336,7 +336,7 @@ type A = list["A" | str] def f(x: A): reveal_type(x) # revealed: list[A | str] for item in x: - reveal_type(item) # revealed: list[A | str] | str + reveal_type(item) # revealed: list[Any | str] | str ``` #### With Optional @@ -349,7 +349,7 @@ type A = list[Optional[Union["A", str]]] def f(x: A): reveal_type(x) # revealed: list[A | str | None] for item in x: - reveal_type(item) # revealed: list[A | str | None] | str | None + reveal_type(item) # revealed: list[Any | str | None] | str | None ``` ### Tuple comparison diff --git a/crates/ty_python_semantic/resources/mdtest/protocols.md b/crates/ty_python_semantic/resources/mdtest/protocols.md index 91cef8e818..663b093e17 100644 --- a/crates/ty_python_semantic/resources/mdtest/protocols.md +++ b/crates/ty_python_semantic/resources/mdtest/protocols.md @@ -893,8 +893,10 @@ class LotsOfBindings(Protocol): match object(): case l: # error: [ambiguous-protocol-member] ... + # error: [ambiguous-protocol-member] "Consider adding an annotation, e.g. `m: int | str = ...`" + m = 1 if 1.2 > 3.4 else "a" -# revealed: frozenset[Literal["Nested", "NestedProtocol", "a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l"]] +# revealed: frozenset[Literal["Nested", "NestedProtocol", "a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m"]] reveal_type(get_protocol_members(LotsOfBindings)) class Foo(Protocol): @@ -1977,12 +1979,12 @@ from typing_extensions import TypeVar, Self, Protocol from ty_extensions import is_equivalent_to, static_assert, is_assignable_to, is_subtype_of class NewStyleClassScoped[T](Protocol): - def method(self: Self, input: T) -> None: ... + def method(self, input: T) -> None: ... S = TypeVar("S") class LegacyClassScoped(Protocol[S]): - def method(self: Self, input: S) -> None: ... + def method(self, input: S) -> None: ... # TODO: these should pass static_assert(is_equivalent_to(NewStyleClassScoped, LegacyClassScoped)) # error: [static-assert-error] diff --git a/crates/ty_python_semantic/resources/mdtest/public_types.md b/crates/ty_python_semantic/resources/mdtest/public_types.md index b0bd3a8a74..4a6ef1c6fb 100644 --- a/crates/ty_python_semantic/resources/mdtest/public_types.md +++ b/crates/ty_python_semantic/resources/mdtest/public_types.md @@ -339,7 +339,7 @@ class A: ... def f(x: A): # TODO: no error - # error: [invalid-assignment] "Object of type `A | A` is not assignable to `A`" + # error: [invalid-assignment] "Object of type `mdtest_snippet.A | mdtest_snippet.A` is not assignable to `mdtest_snippet.A`" x = A() ``` diff --git a/crates/ty_python_semantic/resources/mdtest/type_properties/is_equivalent_to.md b/crates/ty_python_semantic/resources/mdtest/type_properties/is_equivalent_to.md index 83579a0e06..ebb350ee28 100644 --- a/crates/ty_python_semantic/resources/mdtest/type_properties/is_equivalent_to.md +++ b/crates/ty_python_semantic/resources/mdtest/type_properties/is_equivalent_to.md @@ -133,6 +133,11 @@ class Single(Enum): VALUE = 1 static_assert(is_equivalent_to(P | Q | Single, Literal[Single.VALUE] | Q | P)) + +static_assert(is_equivalent_to(Any, Any | Intersection[Any, str])) +static_assert(is_equivalent_to(Any, Intersection[str, Any] | Any)) +static_assert(is_equivalent_to(Any, Any | Intersection[Any, Not[None]])) +static_assert(is_equivalent_to(Any, Intersection[Not[None], Any] | Any)) ``` ## Tuples diff --git a/crates/ty_python_semantic/resources/mdtest/type_properties/is_subtype_of.md b/crates/ty_python_semantic/resources/mdtest/type_properties/is_subtype_of.md index 71ed2b81f2..c3057d6c82 100644 --- a/crates/ty_python_semantic/resources/mdtest/type_properties/is_subtype_of.md +++ b/crates/ty_python_semantic/resources/mdtest/type_properties/is_subtype_of.md @@ -1948,8 +1948,6 @@ static_assert(is_subtype_of(TypeOf[A.g], Callable[[int], int])) static_assert(not is_subtype_of(TypeOf[a.f], Callable[[float], int])) static_assert(not is_subtype_of(TypeOf[A.g], Callable[[], int])) -# TODO: This assertion should be true -# error: [static-assert-error] "Static assertion error: argument of type `ty_extensions.ConstraintSet[never]` is statically known to be falsy" static_assert(is_subtype_of(TypeOf[A.f], Callable[[A, int], int])) ``` diff --git a/crates/ty_python_semantic/resources/mdtest/typed_dict.md b/crates/ty_python_semantic/resources/mdtest/typed_dict.md index 3ad8df2eba..b96b953e20 100644 --- a/crates/ty_python_semantic/resources/mdtest/typed_dict.md +++ b/crates/ty_python_semantic/resources/mdtest/typed_dict.md @@ -657,16 +657,14 @@ alice: Employee = {"name": "Alice", "employee_id": 1} eve: Employee = {"name": "Eve"} def combine(p: Person, e: Employee): - # TODO: Should be `Person` once we support the implicit type of self - reveal_type(p.copy()) # revealed: Unknown - # TODO: Should be `Employee` once we support the implicit type of self - reveal_type(e.copy()) # revealed: Unknown + reveal_type(p.copy()) # revealed: Person + reveal_type(e.copy()) # revealed: Employee reveal_type(p | p) # revealed: Person reveal_type(e | e) # revealed: Employee - # TODO: Should be `Person` once we support the implicit type of self and subtyping for TypedDicts - reveal_type(p | e) # revealed: Employee + # TODO: Should be `Person` once we support subtyping for TypedDicts + reveal_type(p | e) # revealed: Person | Employee ``` When inheriting from a `TypedDict` with a different `total` setting, inherited fields maintain their diff --git a/crates/ty_python_semantic/resources/mdtest/with/async.md b/crates/ty_python_semantic/resources/mdtest/with/async.md index 0c55e5087d..6d556d438b 100644 --- a/crates/ty_python_semantic/resources/mdtest/with/async.md +++ b/crates/ty_python_semantic/resources/mdtest/with/async.md @@ -254,8 +254,7 @@ async def long_running_task(): async def main(): async with asyncio.TaskGroup() as tg: - # TODO: should be `TaskGroup` - reveal_type(tg) # revealed: Unknown + reveal_type(tg) # revealed: TaskGroup tg.create_task(long_running_task()) ``` diff --git a/crates/ty_python_semantic/src/semantic_index/builder.rs b/crates/ty_python_semantic/src/semantic_index/builder.rs index 3b1792a87f..2cbdea0261 100644 --- a/crates/ty_python_semantic/src/semantic_index/builder.rs +++ b/crates/ty_python_semantic/src/semantic_index/builder.rs @@ -2272,7 +2272,7 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { // like `sys.exit()`, and not within sub-expression like `3 + sys.exit()` etc. // // We also only add these inside function scopes, since considering module-level - // constraints can affect the the type of imported symbols, leading to a lot more + // constraints can affect the type of imported symbols, leading to a lot more // work in third-party code. if let ast::Expr::Call(ast::ExprCall { func, .. }) = value.as_ref() { if !self.source_type.is_stub() && self.in_function_scope() { diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index 4994ee8aeb..429c065490 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -52,8 +52,8 @@ use crate::types::function::{ DataclassTransformerParams, FunctionSpans, FunctionType, KnownFunction, }; use crate::types::generics::{ - GenericContext, PartialSpecialization, Specialization, bind_typevar, walk_generic_context, - walk_partial_specialization, walk_specialization, + GenericContext, PartialSpecialization, Specialization, bind_typevar, typing_self, + walk_generic_context, }; pub use crate::types::ide_support::{ CallSignatureDetails, Member, MemberWithDefinition, all_members, call_signature_details, @@ -1050,6 +1050,13 @@ impl<'db> Type<'db> { } } + pub(crate) const fn into_intersection(self) -> Option> { + match self { + Type::Intersection(intersection_type) => Some(intersection_type), + _ => None, + } + } + #[cfg(test)] #[track_caller] pub(crate) fn expect_union(self) -> UnionType<'db> { @@ -1159,21 +1166,26 @@ impl<'db> Type<'db> { } } - /// If this type is a literal, promote it to a type that this literal is an instance of. + /// Promote (possibly nested) literals to types that these literals are instances of. /// /// Note that this function tries to promote literals to a more user-friendly form than their /// fallback instance type. For example, `def _() -> int` is promoted to `Callable[[], int]`, /// as opposed to `FunctionType`. - pub(crate) fn literal_promotion_type(self, db: &'db dyn Db) -> Option> { + pub(crate) fn promote_literals(self, db: &'db dyn Db) -> Type<'db> { + self.apply_type_mapping(db, &TypeMapping::PromoteLiterals) + } + + /// Like [`Type::promote_literals`], but does not recurse into nested types. + fn promote_literals_impl(self, db: &'db dyn Db) -> Type<'db> { match self { - Type::StringLiteral(_) | Type::LiteralString => Some(KnownClass::Str.to_instance(db)), - Type::BooleanLiteral(_) => Some(KnownClass::Bool.to_instance(db)), - Type::IntLiteral(_) => Some(KnownClass::Int.to_instance(db)), - Type::BytesLiteral(_) => Some(KnownClass::Bytes.to_instance(db)), - Type::ModuleLiteral(_) => Some(KnownClass::ModuleType.to_instance(db)), - Type::EnumLiteral(literal) => Some(literal.enum_class_instance(db)), - Type::FunctionLiteral(literal) => Some(Type::Callable(literal.into_callable_type(db))), - _ => None, + Type::StringLiteral(_) | Type::LiteralString => KnownClass::Str.to_instance(db), + Type::BooleanLiteral(_) => KnownClass::Bool.to_instance(db), + Type::IntLiteral(_) => KnownClass::Int.to_instance(db), + Type::BytesLiteral(_) => KnownClass::Bytes.to_instance(db), + Type::ModuleLiteral(_) => KnownClass::ModuleType.to_instance(db), + Type::EnumLiteral(literal) => literal.enum_class_instance(db), + Type::FunctionLiteral(literal) => Type::Callable(literal.into_callable_type(db)), + _ => self, } } @@ -3153,7 +3165,7 @@ impl<'db> Type<'db> { ); match self { Type::Callable(callable) if callable.is_function_like(db) => { - // For "function-like" callables, model the the behavior of `FunctionType.__get__`. + // For "function-like" callables, model the behavior of `FunctionType.__get__`. // // It is a shortcut to model this in `try_call_dunder_get`. If we want to be really precise, // we should instead return a new method-wrapper type variant for the synthesized `__get__` @@ -5356,12 +5368,10 @@ impl<'db> Type<'db> { Some(generic_context) => ( Some(class), Some(generic_context), - Type::from(class.apply_specialization(db, |_| { - // It is important that identity_specialization specializes the class with - // _inferable_ typevars, so that our specialization inference logic will - // try to find a specialization for them. - generic_context.identity_specialization(db) - })), + // It is important that identity_specialization specializes the class with + // _inferable_ typevars, so that our specialization inference logic will + // try to find a specialization for them. + Type::from(class.identity_specialization(db, &Type::TypeVar)), ), _ => (None, None, self), }, @@ -5633,11 +5643,9 @@ impl<'db> Type<'db> { Type::KnownInstance(known_instance) => match known_instance { KnownInstanceType::TypeAliasType(alias) => Ok(Type::TypeAlias(*alias)), KnownInstanceType::TypeVar(typevar) => { - let module = parsed_module(db, scope_id.file(db)).load(db); let index = semantic_index(db, scope_id.file(db)); Ok(bind_typevar( db, - &module, index, scope_id.file_scope_id(db), typevar_binding_context, @@ -5710,7 +5718,6 @@ impl<'db> Type<'db> { .build()), SpecialFormType::TypingSelf => { - let module = parsed_module(db, scope_id.file(db)).load(db); let index = semantic_index(db, scope_id.file(db)); let Some(class) = nearest_enclosing_class(db, index, scope_id) else { return Err(InvalidTypeExpressionError { @@ -5721,40 +5728,13 @@ impl<'db> Type<'db> { }); }; - let upper_bound = Type::instance( + Ok(typing_self( db, - class.apply_specialization(db, |generic_context| { - let types = generic_context - .variables(db) - .iter() - .map(|typevar| Type::NonInferableTypeVar(*typevar)); - - generic_context.specialize(db, types.collect()) - }), - ); - - let class_definition = class.definition(db); - let typevar = TypeVarInstance::new( - db, - ast::name::Name::new_static("Self"), - Some(class_definition), - Some(TypeVarBoundOrConstraints::UpperBound(upper_bound).into()), - // According to the [spec], we can consider `Self` - // equivalent to an invariant type variable - // [spec]: https://typing.python.org/en/latest/spec/generics.html#self - Some(TypeVarVariance::Invariant), - None, - TypeVarKind::TypingSelf, - ); - Ok(bind_typevar( - db, - &module, - index, - scope_id.file_scope_id(db), + scope_id, typevar_binding_context, - typevar, + class, + &Type::NonInferableTypeVar, ) - .map(Type::NonInferableTypeVar) .unwrap_or(*self)) } SpecialFormType::TypeAlias => Ok(Type::Dynamic(DynamicType::TodoTypeAlias)), @@ -6109,19 +6089,18 @@ impl<'db> Type<'db> { } Type::FunctionLiteral(function) => { - let function = Type::FunctionLiteral(function.with_type_mapping(db, type_mapping)); + let function = Type::FunctionLiteral(function.apply_type_mapping_impl(db, type_mapping, visitor)); match type_mapping { - TypeMapping::PromoteLiterals => function.literal_promotion_type(db) - .expect("function literal should have a promotion type"), + TypeMapping::PromoteLiterals => function.promote_literals_impl(db), _ => function } } Type::BoundMethod(method) => Type::BoundMethod(BoundMethodType::new( db, - method.function(db).with_type_mapping(db, type_mapping), - method.self_instance(db).apply_type_mapping(db, type_mapping), + method.function(db).apply_type_mapping_impl(db, type_mapping, visitor), + method.self_instance(db).apply_type_mapping_impl(db, type_mapping, visitor), )), Type::NominalInstance(instance) => @@ -6140,13 +6119,13 @@ impl<'db> Type<'db> { Type::KnownBoundMethod(KnownBoundMethodType::FunctionTypeDunderGet(function)) => { Type::KnownBoundMethod(KnownBoundMethodType::FunctionTypeDunderGet( - function.with_type_mapping(db, type_mapping), + function.apply_type_mapping_impl(db, type_mapping, visitor), )) } Type::KnownBoundMethod(KnownBoundMethodType::FunctionTypeDunderCall(function)) => { Type::KnownBoundMethod(KnownBoundMethodType::FunctionTypeDunderCall( - function.with_type_mapping(db, type_mapping), + function.apply_type_mapping_impl(db, type_mapping, visitor), )) } @@ -6221,8 +6200,7 @@ impl<'db> Type<'db> { TypeMapping::ReplaceSelf { .. } | TypeMapping::MarkTypeVarsInferable(_) | TypeMapping::Materialize(_) => self, - TypeMapping::PromoteLiterals => self.literal_promotion_type(db) - .expect("literal type should have a promotion type"), + TypeMapping::PromoteLiterals => self.promote_literals_impl(db) } Type::Dynamic(_) => match type_mapping { @@ -6782,84 +6760,7 @@ pub enum TypeMapping<'a, 'db> { Materialize(MaterializationKind), } -fn walk_type_mapping<'db, V: visitor::TypeVisitor<'db> + ?Sized>( - db: &'db dyn Db, - mapping: &TypeMapping<'_, 'db>, - visitor: &V, -) { - match mapping { - TypeMapping::Specialization(specialization) => { - walk_specialization(db, *specialization, visitor); - } - TypeMapping::PartialSpecialization(specialization) => { - walk_partial_specialization(db, specialization, visitor); - } - TypeMapping::BindSelf(self_type) => { - visitor.visit_type(db, *self_type); - } - TypeMapping::ReplaceSelf { new_upper_bound } => { - visitor.visit_type(db, *new_upper_bound); - } - TypeMapping::PromoteLiterals - | TypeMapping::BindLegacyTypevars(_) - | TypeMapping::MarkTypeVarsInferable(_) - | TypeMapping::Materialize(_) => {} - } -} - impl<'db> TypeMapping<'_, 'db> { - fn to_owned(&self) -> TypeMapping<'db, 'db> { - match self { - TypeMapping::Specialization(specialization) => { - TypeMapping::Specialization(*specialization) - } - TypeMapping::PartialSpecialization(partial) => { - TypeMapping::PartialSpecialization(partial.to_owned()) - } - TypeMapping::PromoteLiterals => TypeMapping::PromoteLiterals, - TypeMapping::BindLegacyTypevars(binding_context) => { - TypeMapping::BindLegacyTypevars(*binding_context) - } - TypeMapping::BindSelf(self_type) => TypeMapping::BindSelf(*self_type), - TypeMapping::ReplaceSelf { new_upper_bound } => TypeMapping::ReplaceSelf { - new_upper_bound: *new_upper_bound, - }, - TypeMapping::MarkTypeVarsInferable(binding_context) => { - TypeMapping::MarkTypeVarsInferable(*binding_context) - } - TypeMapping::Materialize(materialization_kind) => { - TypeMapping::Materialize(*materialization_kind) - } - } - } - - fn normalized_impl(&self, db: &'db dyn Db, visitor: &NormalizedVisitor<'db>) -> Self { - match self { - TypeMapping::Specialization(specialization) => { - TypeMapping::Specialization(specialization.normalized_impl(db, visitor)) - } - TypeMapping::PartialSpecialization(partial) => { - TypeMapping::PartialSpecialization(partial.normalized_impl(db, visitor)) - } - TypeMapping::PromoteLiterals => TypeMapping::PromoteLiterals, - TypeMapping::BindLegacyTypevars(binding_context) => { - TypeMapping::BindLegacyTypevars(*binding_context) - } - TypeMapping::BindSelf(self_type) => { - TypeMapping::BindSelf(self_type.normalized_impl(db, visitor)) - } - TypeMapping::ReplaceSelf { new_upper_bound } => TypeMapping::ReplaceSelf { - new_upper_bound: new_upper_bound.normalized_impl(db, visitor), - }, - TypeMapping::MarkTypeVarsInferable(binding_context) => { - TypeMapping::MarkTypeVarsInferable(*binding_context) - } - TypeMapping::Materialize(materialization_kind) => { - TypeMapping::Materialize(*materialization_kind) - } - } - } - /// Update the generic context of a [`Signature`] according to the current type mapping pub(crate) fn update_signature_generic_context( &self, @@ -7085,7 +6986,11 @@ impl<'db> KnownInstanceType<'db> { if let Some(specialization) = alias.specialization(self.db) { f.write_str(alias.name(self.db))?; specialization - .display_short(self.db, TupleSpecialization::No) + .display_short( + self.db, + TupleSpecialization::No, + DisplaySettings::default(), + ) .fmt(f) } else { f.write_str("typing.TypeAliasType") diff --git a/crates/ty_python_semantic/src/types/builder.rs b/crates/ty_python_semantic/src/types/builder.rs index cc1c35790a..556edcb368 100644 --- a/crates/ty_python_semantic/src/types/builder.rs +++ b/crates/ty_python_semantic/src/types/builder.rs @@ -504,9 +504,16 @@ impl<'db> UnionBuilder<'db> { if should_simplify_full && !matches!(element_type, Type::TypeAlias(_)) { if ty.is_equivalent_to(self.db, element_type) || ty.is_subtype_of(self.db, element_type) + || ty.into_intersection().is_some_and(|intersection| { + intersection.positive(self.db).contains(&element_type) + }) { return; - } else if element_type.is_subtype_of(self.db, ty) { + } else if element_type.is_subtype_of(self.db, ty) + || element_type + .into_intersection() + .is_some_and(|intersection| intersection.positive(self.db).contains(&ty)) + { to_remove.push(index); } else if ty_negated.is_subtype_of(self.db, element_type) { // We add `ty` to the union. We just checked that `~ty` is a subtype of an diff --git a/crates/ty_python_semantic/src/types/class.rs b/crates/ty_python_semantic/src/types/class.rs index 0afb14480a..65d4d3d9f5 100644 --- a/crates/ty_python_semantic/src/types/class.rs +++ b/crates/ty_python_semantic/src/types/class.rs @@ -376,8 +376,8 @@ pub enum ClassType<'db> { #[salsa::tracked] impl<'db> ClassType<'db> { - pub(super) const fn is_not_generic(self) -> bool { - matches!(self, Self::NonGeneric(_)) + pub(super) const fn is_generic(self) -> bool { + matches!(self, Self::Generic(_)) } pub(super) const fn into_generic_alias(self) -> Option> { @@ -1527,6 +1527,18 @@ impl<'db> ClassLiteral<'db> { }) } + /// Returns a specialization of this class where each typevar is mapped to itself. The second + /// parameter can be `Type::TypeVar` or `Type::NonInferableTypeVar`, depending on the use case. + pub(crate) fn identity_specialization( + self, + db: &'db dyn Db, + typevar_to_type: &impl Fn(BoundTypeVarInstance<'db>) -> Type<'db>, + ) -> ClassType<'db> { + self.apply_specialization(db, |generic_context| { + generic_context.identity_specialization(db, typevar_to_type) + }) + } + /// Return an iterator over the inferred types of this class's *explicit* bases. /// /// Note that any class (except for `object`) that has no explicit @@ -2625,7 +2637,14 @@ impl<'db> ClassLiteral<'db> { .map_type(|ty| ty.apply_type_mapping( db, - &TypeMapping::ReplaceSelf {new_upper_bound: determine_upper_bound(db, self, specialization, ClassBase::is_typed_dict) } + &TypeMapping::ReplaceSelf { + new_upper_bound: determine_upper_bound( + db, + self, + specialization, + ClassBase::is_typed_dict + ) + } ) ) } @@ -4174,6 +4193,91 @@ impl KnownClass { } } + /// Return `true` if this class is a typeshed fallback class which is used to provide attributes and + /// methods for another type (e.g. `NamedTupleFallback` for actual `NamedTuple`s). These fallback + /// classes need special treatment in some places. For example, implicit usages of `Self` should not + /// be eagerly replaced with the fallback class itself. Instead, `Self` should eventually be treated + /// as referring to the destination type (e.g. the actual `NamedTuple`). + pub(crate) const fn is_fallback_class(self) -> bool { + match self { + KnownClass::Bool + | KnownClass::Object + | KnownClass::Bytes + | KnownClass::Bytearray + | KnownClass::Type + | KnownClass::Int + | KnownClass::Float + | KnownClass::Complex + | KnownClass::Str + | KnownClass::List + | KnownClass::Tuple + | KnownClass::Set + | KnownClass::FrozenSet + | KnownClass::Dict + | KnownClass::Slice + | KnownClass::Property + | KnownClass::BaseException + | KnownClass::Exception + | KnownClass::BaseExceptionGroup + | KnownClass::ExceptionGroup + | KnownClass::Staticmethod + | KnownClass::Classmethod + | KnownClass::Super + | KnownClass::Enum + | KnownClass::EnumType + | KnownClass::Auto + | KnownClass::Member + | KnownClass::Nonmember + | KnownClass::StrEnum + | KnownClass::ABCMeta + | KnownClass::GenericAlias + | KnownClass::ModuleType + | KnownClass::FunctionType + | KnownClass::MethodType + | KnownClass::MethodWrapperType + | KnownClass::WrapperDescriptorType + | KnownClass::UnionType + | KnownClass::GeneratorType + | KnownClass::AsyncGeneratorType + | KnownClass::CoroutineType + | KnownClass::NotImplementedType + | KnownClass::BuiltinFunctionType + | KnownClass::EllipsisType + | KnownClass::NoneType + | KnownClass::Awaitable + | KnownClass::Generator + | KnownClass::Deprecated + | KnownClass::StdlibAlias + | KnownClass::SpecialForm + | KnownClass::TypeVar + | KnownClass::ParamSpec + | KnownClass::ParamSpecArgs + | KnownClass::ParamSpecKwargs + | KnownClass::ProtocolMeta + | KnownClass::TypeVarTuple + | KnownClass::TypeAliasType + | KnownClass::NoDefaultType + | KnownClass::NewType + | KnownClass::SupportsIndex + | KnownClass::Iterable + | KnownClass::Iterator + | KnownClass::ChainMap + | KnownClass::Counter + | KnownClass::DefaultDict + | KnownClass::Deque + | KnownClass::OrderedDict + | KnownClass::VersionInfo + | KnownClass::Field + | KnownClass::KwOnly + | KnownClass::NamedTupleLike + | KnownClass::Template + | KnownClass::Path + | KnownClass::ConstraintSet + | KnownClass::InitVar => false, + KnownClass::NamedTupleFallback | KnownClass::TypedDictFallback => true, + } + } + pub(crate) fn name(self, db: &dyn Db) -> &'static str { match self { Self::Bool => "bool", diff --git a/crates/ty_python_semantic/src/types/diagnostic.rs b/crates/ty_python_semantic/src/types/diagnostic.rs index 214eb30749..ea7d54841c 100644 --- a/crates/ty_python_semantic/src/types/diagnostic.rs +++ b/crates/ty_python_semantic/src/types/diagnostic.rs @@ -1986,7 +1986,7 @@ pub(super) fn report_invalid_assignment<'db>( target_ty, format_args!( "Object of type `{}` is not assignable to `{}`", - source_ty.display_with(context.db(), settings), + source_ty.display_with(context.db(), settings.clone()), target_ty.display_with(context.db(), settings) ), ); @@ -2068,8 +2068,8 @@ pub(super) fn report_invalid_return_type( let mut diag = builder.into_diagnostic("Return type does not match returned value"); diag.set_primary_message(format_args!( "expected `{expected_ty}`, found `{actual_ty}`", - expected_ty = expected_ty.display_with(context.db(), settings), - actual_ty = actual_ty.display_with(context.db(), settings), + expected_ty = expected_ty.display_with(context.db(), settings.clone()), + actual_ty = actual_ty.display_with(context.db(), settings.clone()), )); diag.annotate( Annotation::secondary(return_type_span).message(format_args!( @@ -2625,6 +2625,12 @@ pub(crate) fn report_undeclared_protocol_member( SubclassOfInner::Dynamic(_) => return false, }, Type::NominalInstance(instance) => instance.class(db), + Type::Union(union) => { + return union + .elements(db) + .iter() + .all(|elem| should_give_hint(db, *elem)); + } _ => return false, }; @@ -2656,9 +2662,7 @@ pub(crate) fn report_undeclared_protocol_member( if definition.kind(db).is_unannotated_assignment() { let binding_type = binding_type(db, definition); - let suggestion = binding_type - .literal_promotion_type(db) - .unwrap_or(binding_type); + let suggestion = binding_type.promote_literals(db); if should_give_hint(db, suggestion) { diagnostic.set_primary_message(format_args!( diff --git a/crates/ty_python_semantic/src/types/display.rs b/crates/ty_python_semantic/src/types/display.rs index a49d563807..4bf9c235d2 100644 --- a/crates/ty_python_semantic/src/types/display.rs +++ b/crates/ty_python_semantic/src/types/display.rs @@ -1,11 +1,15 @@ //! Display implementations for types. +use std::cell::RefCell; +use std::collections::hash_map::Entry; use std::fmt::{self, Display, Formatter, Write}; +use std::rc::Rc; use ruff_db::display::FormatterJoinExtension; use ruff_python_ast::str::{Quote, TripleQuotes}; use ruff_python_literal::escape::AsciiEscape; use ruff_text_size::{TextRange, TextSize}; +use rustc_hash::{FxHashMap, FxHashSet}; use crate::Db; use crate::module_resolver::file_to_module; @@ -15,117 +19,156 @@ use crate::types::function::{FunctionType, OverloadLiteral}; use crate::types::generics::{GenericContext, Specialization}; use crate::types::signatures::{CallableSignature, Parameter, Parameters, Signature}; use crate::types::tuple::TupleSpec; +use crate::types::visitor::TypeVisitor; use crate::types::{ BoundTypeVarInstance, CallableType, IntersectionType, KnownBoundMethodType, KnownClass, MaterializationKind, Protocol, ProtocolInstanceType, StringLiteralType, SubclassOfInner, Type, - UnionType, WrapperDescriptorKind, + UnionType, WrapperDescriptorKind, visitor, }; use ruff_db::parsed::parsed_module; /// Settings for displaying types and signatures -#[derive(Debug, Copy, Clone, Default)] -pub struct DisplaySettings { +#[derive(Debug, Clone, Default)] +pub struct DisplaySettings<'db> { /// Whether rendering can be multiline pub multiline: bool, - /// Whether rendering will show qualified display (e.g., module.class) - pub qualified: bool, + /// Class names that should be displayed fully qualified + /// (e.g., `module.ClassName` instead of just `ClassName`) + pub qualified: Rc>, } -impl DisplaySettings { +impl<'db> DisplaySettings<'db> { #[must_use] - pub fn multiline(self) -> Self { + pub fn multiline(&self) -> Self { Self { multiline: true, - ..self + ..self.clone() } } #[must_use] - pub fn singleline(self) -> Self { + pub fn singleline(&self) -> Self { Self { multiline: false, - ..self + ..self.clone() } } #[must_use] - pub fn qualified(self) -> Self { - Self { - qualified: true, - ..self - } - } - - #[must_use] - pub fn from_possibly_ambiguous_type_pair<'db>( + pub fn from_possibly_ambiguous_type_pair( db: &'db dyn Db, type_1: Type<'db>, type_2: Type<'db>, ) -> Self { - let result = Self::default(); + let collector = AmbiguousClassCollector::default(); + collector.visit_type(db, type_1); + collector.visit_type(db, type_2); - let Some(class_1) = type_to_class_literal(db, type_1) else { - return result; - }; - - let Some(class_2) = type_to_class_literal(db, type_2) else { - return result; - }; - - if class_1 == class_2 { - return result; - } - - if class_1.name(db) == class_2.name(db) { - result.qualified() - } else { - result + Self { + qualified: Rc::new( + collector + .class_names + .borrow() + .iter() + .filter_map(|(name, ambiguity)| ambiguity.is_ambiguous().then_some(*name)) + .collect(), + ), + ..Self::default() } } } -// TODO: generalize this to a method that takes any two types, walks them recursively, and returns -// a set of types with ambiguous names whose display should be qualified. Then we can use this in -// any diagnostic that displays two types. -fn type_to_class_literal<'db>(db: &'db dyn Db, ty: Type<'db>) -> Option> { - match ty { - Type::ClassLiteral(class) => Some(class), - Type::NominalInstance(instance) => Some(instance.class_literal(db)), - Type::EnumLiteral(enum_literal) => Some(enum_literal.enum_class(db)), - Type::GenericAlias(alias) => Some(alias.origin(db)), - Type::ProtocolInstance(ProtocolInstanceType { - inner: Protocol::FromClass(class), - .. - }) => type_to_class_literal(db, Type::from(class)), - Type::TypedDict(typed_dict) => { - type_to_class_literal(db, Type::from(typed_dict.defining_class())) +#[derive(Debug, Default)] +struct AmbiguousClassCollector<'db> { + visited_types: RefCell>>, + class_names: RefCell>>, +} + +impl<'db> AmbiguousClassCollector<'db> { + fn record_class(&self, db: &'db dyn Db, class: ClassLiteral<'db>) { + match self.class_names.borrow_mut().entry(class.name(db)) { + Entry::Vacant(entry) => { + entry.insert(AmbiguityState::Unambiguous(class)); + } + Entry::Occupied(mut entry) => { + let value = entry.get_mut(); + if let AmbiguityState::Unambiguous(existing) = value + && *existing != class + { + *value = AmbiguityState::Ambiguous; + } + } } - Type::SubclassOf(subclass_of) => { - type_to_class_literal(db, Type::from(subclass_of.subclass_of().into_class()?)) + } +} + +/// Whether or not a class can be unambiguously identified by its *unqualified* name +/// given the other types that are present in the same context. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum AmbiguityState<'db> { + /// The class can be displayed unambiguously using its unqualified name + Unambiguous(ClassLiteral<'db>), + /// The class must be displayed using its fully qualified name to avoid ambiguity. + Ambiguous, +} + +impl AmbiguityState<'_> { + const fn is_ambiguous(self) -> bool { + matches!(self, AmbiguityState::Ambiguous) + } +} + +impl<'db> super::visitor::TypeVisitor<'db> for AmbiguousClassCollector<'db> { + fn should_visit_lazy_type_attributes(&self) -> bool { + false + } + + fn visit_type(&self, db: &'db dyn Db, ty: Type<'db>) { + match ty { + Type::ClassLiteral(class) => self.record_class(db, class), + Type::EnumLiteral(literal) => self.record_class(db, literal.enum_class(db)), + Type::GenericAlias(alias) => self.record_class(db, alias.origin(db)), + // Visit the class (as if it were a nominal-instance type) + // rather than the protocol members, if it is a class-based protocol. + // (For the purposes of displaying the type, we'll use the class name.) + Type::ProtocolInstance(ProtocolInstanceType { + inner: Protocol::FromClass(class), + .. + }) => return self.visit_type(db, Type::from(class)), + _ => {} + } + + if let visitor::TypeKind::NonAtomic(t) = visitor::TypeKind::from(ty) { + if !self.visited_types.borrow_mut().insert(ty) { + // If we have already seen this type, we can skip it. + return; + } + visitor::walk_non_atomic_type(db, t, self); } - _ => None, } } impl<'db> Type<'db> { - pub fn display(&self, db: &'db dyn Db) -> DisplayType<'_> { + pub fn display(self, db: &'db dyn Db) -> DisplayType<'db> { DisplayType { ty: self, settings: DisplaySettings::default(), db, } } - pub fn display_with(&self, db: &'db dyn Db, settings: DisplaySettings) -> DisplayType<'_> { + + pub fn display_with(self, db: &'db dyn Db, settings: DisplaySettings<'db>) -> DisplayType<'db> { DisplayType { ty: self, db, settings, } } + fn representation( self, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, ) -> DisplayRepresentation<'db> { DisplayRepresentation { db, @@ -135,16 +178,15 @@ impl<'db> Type<'db> { } } -#[derive(Copy, Clone)] pub struct DisplayType<'db> { - ty: &'db Type<'db>, + ty: Type<'db>, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, } impl Display for DisplayType<'_> { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - let representation = self.ty.representation(self.db, self.settings); + let representation = self.ty.representation(self.db, self.settings.clone()); match self.ty { Type::IntLiteral(_) | Type::BooleanLiteral(_) @@ -165,7 +207,7 @@ impl fmt::Debug for DisplayType<'_> { } impl<'db> ClassLiteral<'db> { - fn display_with(self, db: &'db dyn Db, settings: DisplaySettings) -> ClassDisplay<'db> { + fn display_with(self, db: &'db dyn Db, settings: DisplaySettings<'db>) -> ClassDisplay<'db> { ClassDisplay { db, class: self, @@ -177,7 +219,7 @@ impl<'db> ClassLiteral<'db> { struct ClassDisplay<'db> { db: &'db dyn Db, class: ClassLiteral<'db>, - settings: DisplaySettings, + settings: DisplaySettings<'db>, } impl ClassDisplay<'_> { @@ -224,7 +266,11 @@ impl ClassDisplay<'_> { impl Display for ClassDisplay<'_> { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - if self.settings.qualified { + if self + .settings + .qualified + .contains(&**self.class.name(self.db)) + { for parent in self.class_parents() { f.write_str(&parent)?; f.write_char('.')?; @@ -240,7 +286,7 @@ impl Display for ClassDisplay<'_> { struct DisplayRepresentation<'db> { ty: Type<'db>, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, } impl Display for DisplayRepresentation<'_> { @@ -258,20 +304,20 @@ impl Display for DisplayRepresentation<'_> { .specialization(self.db) .tuple(self.db) .expect("Specialization::tuple() should always return `Some()` for `KnownClass::Tuple`") - .display_with(self.db, self.settings) + .display_with(self.db, self.settings.clone()) .fmt(f), (ClassType::NonGeneric(class), _) => { - class.display_with(self.db, self.settings).fmt(f) + class.display_with(self.db, self.settings.clone()).fmt(f) }, - (ClassType::Generic(alias), _) => alias.display_with(self.db, self.settings).fmt(f), + (ClassType::Generic(alias), _) => alias.display_with(self.db, self.settings.clone()).fmt(f), } } Type::ProtocolInstance(protocol) => match protocol.inner { Protocol::FromClass(ClassType::NonGeneric(class)) => { - class.display_with(self.db, self.settings).fmt(f) + class.display_with(self.db, self.settings.clone()).fmt(f) } Protocol::FromClass(ClassType::Generic(alias)) => { - alias.display_with(self.db, self.settings).fmt(f) + alias.display_with(self.db, self.settings.clone()).fmt(f) } Protocol::Synthesized(synthetic) => { f.write_str(" { Type::ClassLiteral(class) => write!( f, "", - class.display_with(self.db, self.settings) + class.display_with(self.db, self.settings.clone()) ), Type::GenericAlias(generic) => write!( f, @@ -304,7 +350,11 @@ impl Display for DisplayRepresentation<'_> { ), Type::SubclassOf(subclass_of_ty) => match subclass_of_ty.subclass_of() { SubclassOfInner::Class(ClassType::NonGeneric(class)) => { - write!(f, "type[{}]", class.display_with(self.db, self.settings)) + write!( + f, + "type[{}]", + class.display_with(self.db, self.settings.clone()) + ) } SubclassOfInner::Class(ClassType::Generic(alias)) => { write!( @@ -317,8 +367,12 @@ impl Display for DisplayRepresentation<'_> { }, Type::SpecialForm(special_form) => special_form.fmt(f), Type::KnownInstance(known_instance) => known_instance.repr(self.db).fmt(f), - Type::FunctionLiteral(function) => function.display_with(self.db, self.settings).fmt(f), - Type::Callable(callable) => callable.display_with(self.db, self.settings).fmt(f), + Type::FunctionLiteral(function) => { + function.display_with(self.db, self.settings.clone()).fmt(f) + } + Type::Callable(callable) => { + callable.display_with(self.db, self.settings.clone()).fmt(f) + } Type::BoundMethod(bound_method) => { let function = bound_method.function(self.db); let self_ty = bound_method.self_instance(self.db); @@ -329,7 +383,7 @@ impl Display for DisplayRepresentation<'_> { let type_parameters = DisplayOptionalGenericContext { generic_context: signature.generic_context.as_ref(), db: self.db, - settings: self.settings, + settings: self.settings.clone(), }; write!( @@ -340,7 +394,7 @@ impl Display for DisplayRepresentation<'_> { type_parameters = type_parameters, signature = signature .bind_self(self.db, Some(typing_self_ty)) - .display_with(self.db, self.settings) + .display_with(self.db, self.settings.clone()) ) } signatures => { @@ -354,7 +408,7 @@ impl Display for DisplayRepresentation<'_> { join.entry( &signature .bind_self(self.db, Some(typing_self_ty)) - .display_with(self.db, self.settings), + .display_with(self.db, self.settings.clone()), ); } if !self.settings.multiline { @@ -404,13 +458,15 @@ impl Display for DisplayRepresentation<'_> { Type::DataclassTransformer(_) => { f.write_str("") } - Type::Union(union) => union.display_with(self.db, self.settings).fmt(f), - Type::Intersection(intersection) => { - intersection.display_with(self.db, self.settings).fmt(f) - } + Type::Union(union) => union.display_with(self.db, self.settings.clone()).fmt(f), + Type::Intersection(intersection) => intersection + .display_with(self.db, self.settings.clone()) + .fmt(f), Type::IntLiteral(n) => n.fmt(f), Type::BooleanLiteral(boolean) => f.write_str(if boolean { "True" } else { "False" }), - Type::StringLiteral(string) => string.display_with(self.db, self.settings).fmt(f), + Type::StringLiteral(string) => { + string.display_with(self.db, self.settings.clone()).fmt(f) + } Type::LiteralString => f.write_str("LiteralString"), Type::BytesLiteral(bytes) => { let escape = AsciiEscape::with_preferred_quote(bytes.value(self.db), Quote::Double); @@ -422,7 +478,7 @@ impl Display for DisplayRepresentation<'_> { "{enum_class}.{literal_name}", enum_class = enum_literal .enum_class(self.db) - .display_with(self.db, self.settings), + .display_with(self.db, self.settings.clone()), literal_name = enum_literal.name(self.db) ), Type::NonInferableTypeVar(bound_typevar) | Type::TypeVar(bound_typevar) => { @@ -458,7 +514,7 @@ impl Display for DisplayRepresentation<'_> { .defining_class() .class_literal(self.db) .0 - .display_with(self.db, self.settings) + .display_with(self.db, self.settings.clone()) .fmt(f), Type::TypeAlias(alias) => f.write_str(alias.name(self.db)), } @@ -493,7 +549,7 @@ impl<'db> TupleSpec<'db> { pub(crate) fn display_with( &'db self, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, ) -> DisplayTuple<'db> { DisplayTuple { tuple: self, @@ -506,7 +562,7 @@ impl<'db> TupleSpec<'db> { pub(crate) struct DisplayTuple<'db> { tuple: &'db TupleSpec<'db>, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, } impl Display for DisplayTuple<'_> { @@ -580,7 +636,7 @@ impl<'db> OverloadLiteral<'db> { pub(crate) fn display_with( self, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, ) -> DisplayOverloadLiteral<'db> { DisplayOverloadLiteral { literal: self, @@ -593,7 +649,7 @@ impl<'db> OverloadLiteral<'db> { pub(crate) struct DisplayOverloadLiteral<'db> { literal: OverloadLiteral<'db>, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, } impl Display for DisplayOverloadLiteral<'_> { @@ -602,7 +658,7 @@ impl Display for DisplayOverloadLiteral<'_> { let type_parameters = DisplayOptionalGenericContext { generic_context: signature.generic_context.as_ref(), db: self.db, - settings: self.settings, + settings: self.settings.clone(), }; write!( @@ -610,7 +666,7 @@ impl Display for DisplayOverloadLiteral<'_> { "def {name}{type_parameters}{signature}", name = self.literal.name(self.db), type_parameters = type_parameters, - signature = signature.display_with(self.db, self.settings) + signature = signature.display_with(self.db, self.settings.clone()) ) } } @@ -619,7 +675,7 @@ impl<'db> FunctionType<'db> { pub(crate) fn display_with( self, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, ) -> DisplayFunctionType<'db> { DisplayFunctionType { ty: self, @@ -632,7 +688,7 @@ impl<'db> FunctionType<'db> { pub(crate) struct DisplayFunctionType<'db> { ty: FunctionType<'db>, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, } impl Display for DisplayFunctionType<'_> { @@ -644,7 +700,7 @@ impl Display for DisplayFunctionType<'_> { let type_parameters = DisplayOptionalGenericContext { generic_context: signature.generic_context.as_ref(), db: self.db, - settings: self.settings, + settings: self.settings.clone(), }; write!( @@ -652,7 +708,7 @@ impl Display for DisplayFunctionType<'_> { "def {name}{type_parameters}{signature}", name = self.ty.name(self.db), type_parameters = type_parameters, - signature = signature.display_with(self.db, self.settings) + signature = signature.display_with(self.db, self.settings.clone()) ) } signatures => { @@ -663,7 +719,7 @@ impl Display for DisplayFunctionType<'_> { let separator = if self.settings.multiline { "\n" } else { ", " }; let mut join = f.join(separator); for signature in signatures { - join.entry(&signature.display_with(self.db, self.settings)); + join.entry(&signature.display_with(self.db, self.settings.clone())); } if !self.settings.multiline { f.write_str("]")?; @@ -678,7 +734,7 @@ impl<'db> GenericAlias<'db> { pub(crate) fn display_with( &'db self, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, ) -> DisplayGenericAlias<'db> { DisplayGenericAlias { origin: self.origin(db), @@ -693,13 +749,13 @@ pub(crate) struct DisplayGenericAlias<'db> { origin: ClassLiteral<'db>, specialization: Specialization<'db>, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, } impl Display for DisplayGenericAlias<'_> { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { if let Some(tuple) = self.specialization.tuple(self.db) { - tuple.display_with(self.db, self.settings).fmt(f) + tuple.display_with(self.db, self.settings.clone()).fmt(f) } else { let prefix = match self.specialization.materialization_kind(self.db) { None => "", @@ -714,10 +770,11 @@ impl Display for DisplayGenericAlias<'_> { f, "{prefix}{origin}{specialization}{suffix}", prefix = prefix, - origin = self.origin.display_with(self.db, self.settings), + origin = self.origin.display_with(self.db, self.settings.clone()), specialization = self.specialization.display_short( self.db, - TupleSpecialization::from_class(self.db, self.origin) + TupleSpecialization::from_class(self.db, self.origin), + self.settings.clone() ), suffix = suffix, ) @@ -732,7 +789,7 @@ impl<'db> GenericContext<'db> { pub fn display_with( &'db self, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, ) -> DisplayGenericContext<'db> { DisplayGenericContext { generic_context: self, @@ -745,7 +802,7 @@ impl<'db> GenericContext<'db> { struct DisplayOptionalGenericContext<'db> { generic_context: Option<&'db GenericContext<'db>>, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, } impl Display for DisplayOptionalGenericContext<'_> { @@ -754,7 +811,7 @@ impl Display for DisplayOptionalGenericContext<'_> { DisplayGenericContext { generic_context, db: self.db, - settings: self.settings, + settings: self.settings.clone(), } .fmt(f) } else { @@ -767,7 +824,7 @@ pub struct DisplayGenericContext<'db> { generic_context: &'db GenericContext<'db>, db: &'db dyn Db, #[expect(dead_code)] - settings: DisplaySettings, + settings: DisplaySettings<'db>, } impl Display for DisplayGenericContext<'_> { @@ -800,12 +857,13 @@ impl<'db> Specialization<'db> { &'db self, db: &'db dyn Db, tuple_specialization: TupleSpecialization, + settings: DisplaySettings<'db>, ) -> DisplaySpecialization<'db> { DisplaySpecialization { types: self.types(db), db, tuple_specialization, - settings: DisplaySettings::default(), + settings, } } } @@ -814,7 +872,7 @@ pub struct DisplaySpecialization<'db> { types: &'db [Type<'db>], db: &'db dyn Db, tuple_specialization: TupleSpecialization, - settings: DisplaySettings, + settings: DisplaySettings<'db>, } impl Display for DisplaySpecialization<'_> { @@ -824,7 +882,7 @@ impl Display for DisplaySpecialization<'_> { if idx > 0 { f.write_str(", ")?; } - ty.display_with(self.db, self.settings).fmt(f)?; + ty.display_with(self.db, self.settings.clone()).fmt(f)?; } if self.tuple_specialization.is_yes() { f.write_str(", ...")?; @@ -861,7 +919,7 @@ impl<'db> CallableType<'db> { pub(crate) fn display_with( &'db self, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, ) -> DisplayCallableType<'db> { DisplayCallableType { signatures: self.signatures(db), @@ -874,13 +932,15 @@ impl<'db> CallableType<'db> { pub(crate) struct DisplayCallableType<'db> { signatures: &'db CallableSignature<'db>, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, } impl Display for DisplayCallableType<'_> { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { match self.signatures.overloads.as_slice() { - [signature] => signature.display_with(self.db, self.settings).fmt(f), + [signature] => signature + .display_with(self.db, self.settings.clone()) + .fmt(f), signatures => { // TODO: How to display overloads? if !self.settings.multiline { @@ -889,7 +949,7 @@ impl Display for DisplayCallableType<'_> { let separator = if self.settings.multiline { "\n" } else { ", " }; let mut join = f.join(separator); for signature in signatures { - join.entry(&signature.display_with(self.db, self.settings)); + join.entry(&signature.display_with(self.db, self.settings.clone())); } join.finish()?; if !self.settings.multiline { @@ -909,7 +969,7 @@ impl<'db> Signature<'db> { pub(crate) fn display_with( &'db self, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, ) -> DisplaySignature<'db> { DisplaySignature { parameters: self.parameters(), @@ -924,7 +984,7 @@ pub(crate) struct DisplaySignature<'db> { parameters: &'db Parameters<'db>, return_ty: Option>, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, } impl DisplaySignature<'_> { @@ -1128,7 +1188,7 @@ impl<'db> Parameter<'db> { fn display_with( &'db self, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, ) -> DisplayParameter<'db> { DisplayParameter { param: self, @@ -1141,7 +1201,7 @@ impl<'db> Parameter<'db> { struct DisplayParameter<'db> { param: &'db Parameter<'db>, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, } impl Display for DisplayParameter<'_> { @@ -1149,24 +1209,34 @@ impl Display for DisplayParameter<'_> { if let Some(name) = self.param.display_name() { f.write_str(&name)?; if let Some(annotated_type) = self.param.annotated_type() { - write!( - f, - ": {}", - annotated_type.display_with(self.db, self.settings) - )?; + if self.param.should_annotation_be_displayed() { + write!( + f, + ": {}", + annotated_type.display_with(self.db, self.settings.clone()) + )?; + } } // Default value can only be specified if `name` is given. if let Some(default_ty) = self.param.default_type() { if self.param.annotated_type().is_some() { - write!(f, " = {}", default_ty.display_with(self.db, self.settings))?; + write!( + f, + " = {}", + default_ty.display_with(self.db, self.settings.clone()) + )?; } else { - write!(f, "={}", default_ty.display_with(self.db, self.settings))?; + write!( + f, + "={}", + default_ty.display_with(self.db, self.settings.clone()) + )?; } } } else if let Some(ty) = self.param.annotated_type() { // This case is specifically for the `Callable` signature where name and default value // cannot be provided. - ty.display_with(self.db, self.settings).fmt(f)?; + ty.display_with(self.db, self.settings.clone()).fmt(f)?; } Ok(()) } @@ -1176,7 +1246,7 @@ impl<'db> UnionType<'db> { fn display_with( &'db self, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, ) -> DisplayUnionType<'db> { DisplayUnionType { db, @@ -1189,7 +1259,7 @@ impl<'db> UnionType<'db> { struct DisplayUnionType<'db> { ty: &'db UnionType<'db>, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, } impl Display for DisplayUnionType<'_> { @@ -1249,7 +1319,7 @@ impl fmt::Debug for DisplayUnionType<'_> { struct DisplayLiteralGroup<'db> { literals: Vec>, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, } impl Display for DisplayLiteralGroup<'_> { @@ -1270,7 +1340,7 @@ impl<'db> IntersectionType<'db> { fn display_with( &'db self, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, ) -> DisplayIntersectionType<'db> { DisplayIntersectionType { db, @@ -1283,7 +1353,7 @@ impl<'db> IntersectionType<'db> { struct DisplayIntersectionType<'db> { ty: &'db IntersectionType<'db>, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, } impl Display for DisplayIntersectionType<'_> { @@ -1323,7 +1393,7 @@ struct DisplayMaybeNegatedType<'db> { ty: Type<'db>, db: &'db dyn Db, negated: bool, - settings: DisplaySettings, + settings: DisplaySettings<'db>, } impl Display for DisplayMaybeNegatedType<'_> { @@ -1334,7 +1404,7 @@ impl Display for DisplayMaybeNegatedType<'_> { DisplayMaybeParenthesizedType { ty: self.ty, db: self.db, - settings: self.settings, + settings: self.settings.clone(), } .fmt(f) } @@ -1343,13 +1413,18 @@ impl Display for DisplayMaybeNegatedType<'_> { struct DisplayMaybeParenthesizedType<'db> { ty: Type<'db>, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, } impl Display for DisplayMaybeParenthesizedType<'_> { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - let write_parentheses = - |f: &mut Formatter<'_>| write!(f, "({})", self.ty.display_with(self.db, self.settings)); + let write_parentheses = |f: &mut Formatter<'_>| { + write!( + f, + "({})", + self.ty.display_with(self.db, self.settings.clone()) + ) + }; match self.ty { Type::Callable(_) | Type::KnownBoundMethod(_) @@ -1359,21 +1434,24 @@ impl Display for DisplayMaybeParenthesizedType<'_> { Type::Intersection(intersection) if !intersection.has_one_element(self.db) => { write_parentheses(f) } - _ => self.ty.display_with(self.db, self.settings).fmt(f), + _ => self.ty.display_with(self.db, self.settings.clone()).fmt(f), } } } pub(crate) trait TypeArrayDisplay<'db> { - fn display_with(&self, db: &'db dyn Db, settings: DisplaySettings) - -> DisplayTypeArray<'_, 'db>; + fn display_with( + &self, + db: &'db dyn Db, + settings: DisplaySettings<'db>, + ) -> DisplayTypeArray<'_, 'db>; } impl<'db> TypeArrayDisplay<'db> for Box<[Type<'db>]> { fn display_with( &self, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, ) -> DisplayTypeArray<'_, 'db> { DisplayTypeArray { types: self, @@ -1387,7 +1465,7 @@ impl<'db> TypeArrayDisplay<'db> for Vec> { fn display_with( &self, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, ) -> DisplayTypeArray<'_, 'db> { DisplayTypeArray { types: self, @@ -1401,7 +1479,7 @@ impl<'db> TypeArrayDisplay<'db> for [Type<'db>] { fn display_with( &self, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, ) -> DisplayTypeArray<'_, 'db> { DisplayTypeArray { types: self, @@ -1414,7 +1492,7 @@ impl<'db> TypeArrayDisplay<'db> for [Type<'db>] { pub(crate) struct DisplayTypeArray<'b, 'db> { types: &'b [Type<'db>], db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, } impl Display for DisplayTypeArray<'_, '_> { @@ -1433,20 +1511,19 @@ impl<'db> StringLiteralType<'db> { fn display_with( &'db self, db: &'db dyn Db, - settings: DisplaySettings, + settings: DisplaySettings<'db>, ) -> DisplayStringLiteralType<'db> { - display_quoted_string(self.value(db), settings) + DisplayStringLiteralType { + string: self.value(db), + settings, + } } } -fn display_quoted_string(string: &str, settings: DisplaySettings) -> DisplayStringLiteralType<'_> { - DisplayStringLiteralType { string, settings } -} - struct DisplayStringLiteralType<'db> { string: &'db str, #[expect(dead_code)] - settings: DisplaySettings, + settings: DisplaySettings<'db>, } impl Display for DisplayStringLiteralType<'_> { diff --git a/crates/ty_python_semantic/src/types/function.rs b/crates/ty_python_semantic/src/types/function.rs index d87927fdac..0cd7efd286 100644 --- a/crates/ty_python_semantic/src/types/function.rs +++ b/crates/ty_python_semantic/src/types/function.rs @@ -77,11 +77,11 @@ use crate::types::narrow::ClassInfoConstraintFunction; use crate::types::signatures::{CallableSignature, Signature}; use crate::types::visitor::any_over_type; use crate::types::{ - BoundMethodType, BoundTypeVarInstance, CallableType, ClassBase, ClassLiteral, ClassType, - DeprecatedInstance, DynamicType, FindLegacyTypeVarsVisitor, HasRelationToVisitor, - IsEquivalentVisitor, KnownClass, KnownInstanceType, NormalizedVisitor, SpecialFormType, - Truthiness, Type, TypeMapping, TypeRelation, UnionBuilder, ValidSpecializationsConstraintSet, - all_members, binding_type, todo_type, walk_type_mapping, + ApplyTypeMappingVisitor, BoundMethodType, BoundTypeVarInstance, CallableType, ClassBase, + ClassLiteral, ClassType, DeprecatedInstance, DynamicType, FindLegacyTypeVarsVisitor, + HasRelationToVisitor, IsEquivalentVisitor, KnownClass, KnownInstanceType, NormalizedVisitor, + SpecialFormType, Truthiness, Type, TypeMapping, TypeRelation, UnionBuilder, + ValidSpecializationsConstraintSet, all_members, binding_type, todo_type, walk_signature, }; use crate::{Db, FxOrderSet, ModuleName, resolve_module}; @@ -623,33 +623,24 @@ impl<'db> FunctionLiteral<'db> { /// calling query is not in the same file as this function is defined in, then this will create /// a cross-module dependency directly on the full AST which will lead to cache /// over-invalidation. - fn signature<'a>( - self, - db: &'db dyn Db, - type_mappings: &'a [TypeMapping<'a, 'db>], - ) -> CallableSignature<'db> - where - 'db: 'a, - { + fn signature(self, db: &'db dyn Db) -> CallableSignature<'db> { // We only include an implementation (i.e. a definition not decorated with `@overload`) if // it's the only definition. let inherited_generic_context = self.inherited_generic_context(db); let (overloads, implementation) = self.overloads_and_implementation(db); if let Some(implementation) = implementation { if overloads.is_empty() { - return CallableSignature::single(type_mappings.iter().fold( + return CallableSignature::single( implementation.signature(db, inherited_generic_context), - |sig, mapping| sig.apply_type_mapping(db, mapping), - )); + ); } } - CallableSignature::from_overloads(overloads.iter().map(|overload| { - type_mappings.iter().fold( - overload.signature(db, inherited_generic_context), - |sig, mapping| sig.apply_type_mapping(db, mapping), - ) - })) + CallableSignature::from_overloads( + overloads + .iter() + .map(|overload| overload.signature(db, inherited_generic_context)), + ) } /// Typed externally-visible signature of the last overload or implementation of this function. @@ -660,20 +651,10 @@ impl<'db> FunctionLiteral<'db> { /// calling query is not in the same file as this function is defined in, then this will create /// a cross-module dependency directly on the full AST which will lead to cache /// over-invalidation. - fn last_definition_signature<'a>( - self, - db: &'db dyn Db, - type_mappings: &'a [TypeMapping<'a, 'db>], - ) -> Signature<'db> - where - 'db: 'a, - { + fn last_definition_signature(self, db: &'db dyn Db) -> Signature<'db> { let inherited_generic_context = self.inherited_generic_context(db); - type_mappings.iter().fold( - self.last_definition(db) - .signature(db, inherited_generic_context), - |sig, mapping| sig.apply_type_mapping(db, mapping), - ) + self.last_definition(db) + .signature(db, inherited_generic_context) } fn normalized_impl(self, db: &'db dyn Db, visitor: &NormalizedVisitor<'db>) -> Self { @@ -691,11 +672,19 @@ impl<'db> FunctionLiteral<'db> { pub struct FunctionType<'db> { pub(crate) literal: FunctionLiteral<'db>, - /// Type mappings that should be applied to the function's parameter and return types. This - /// might include specializations of enclosing generic contexts (e.g. for non-generic methods - /// of a specialized generic class). - #[returns(deref)] - type_mappings: Box<[TypeMapping<'db, 'db>]>, + /// Contains a potentially modified signature for this function literal, in case certain operations + /// (like type mappings) have been applied to it. + /// + /// See also: [`FunctionLiteral::updated_signature`]. + #[returns(as_ref)] + updated_signature: Option>, + + /// Contains a potentially modified signature for the last overload or the implementation of this + /// function literal, in case certain operations (like type mappings) have been applied to it. + /// + /// See also: [`FunctionLiteral::last_definition_signature`]. + #[returns(as_ref)] + updated_last_definition_signature: Option>, } // The Salsa heap is tracked separately. @@ -707,8 +696,13 @@ pub(super) fn walk_function_type<'db, V: super::visitor::TypeVisitor<'db> + ?Siz visitor: &V, ) { walk_function_literal(db, function.literal(db), visitor); - for mapping in function.type_mappings(db) { - walk_type_mapping(db, mapping, visitor); + if let Some(callable_signature) = function.updated_signature(db) { + for signature in &callable_signature.overloads { + walk_signature(db, signature, visitor); + } + } + if let Some(signature) = function.updated_last_definition_signature(db) { + walk_signature(db, signature, visitor); } } @@ -722,21 +716,41 @@ impl<'db> FunctionType<'db> { let literal = self .literal(db) .with_inherited_generic_context(db, inherited_generic_context); - Self::new(db, literal, self.type_mappings(db)) + let updated_signature = self.updated_signature(db).map(|signature| { + signature.with_inherited_generic_context(Some(inherited_generic_context)) + }); + let updated_last_definition_signature = + self.updated_last_definition_signature(db).map(|signature| { + signature + .clone() + .with_inherited_generic_context(Some(inherited_generic_context)) + }); + Self::new( + db, + literal, + updated_signature, + updated_last_definition_signature, + ) } - pub(crate) fn with_type_mapping<'a>( + pub(crate) fn apply_type_mapping_impl<'a>( self, db: &'db dyn Db, type_mapping: &TypeMapping<'a, 'db>, + visitor: &ApplyTypeMappingVisitor<'db>, ) -> Self { - let type_mappings: Box<[_]> = self - .type_mappings(db) - .iter() - .cloned() - .chain(std::iter::once(type_mapping.to_owned())) - .collect(); - Self::new(db, self.literal(db), type_mappings) + let updated_signature = + self.signature(db) + .apply_type_mapping_impl(db, type_mapping, visitor); + let updated_last_definition_signature = self + .last_definition_signature(db) + .apply_type_mapping_impl(db, type_mapping, visitor); + Self::new( + db, + self.literal(db), + Some(updated_signature), + Some(updated_last_definition_signature), + ) } pub(crate) fn with_dataclass_transformer_params( @@ -752,7 +766,7 @@ impl<'db> FunctionType<'db> { .with_dataclass_transformer_params(db, params); let literal = FunctionLiteral::new(db, last_definition, literal.inherited_generic_context(db)); - Self::new(db, literal, self.type_mappings(db)) + Self::new(db, literal, None, None) } /// Returns the [`File`] in which this function is defined. @@ -907,7 +921,9 @@ impl<'db> FunctionType<'db> { /// would depend on the function's AST and rerun for every change in that file. #[salsa::tracked(returns(ref), cycle_fn=signature_cycle_recover, cycle_initial=signature_cycle_initial, heap_size=ruff_memory_usage::heap_size)] pub(crate) fn signature(self, db: &'db dyn Db) -> CallableSignature<'db> { - self.literal(db).signature(db, self.type_mappings(db)) + self.updated_signature(db) + .cloned() + .unwrap_or_else(|| self.literal(db).signature(db)) } /// Typed externally-visible signature of the last overload or implementation of this function. @@ -926,8 +942,9 @@ impl<'db> FunctionType<'db> { heap_size=ruff_memory_usage::heap_size, )] pub(crate) fn last_definition_signature(self, db: &'db dyn Db) -> Signature<'db> { - self.literal(db) - .last_definition_signature(db, self.type_mappings(db)) + self.updated_last_definition_signature(db) + .cloned() + .unwrap_or_else(|| self.literal(db).last_definition_signature(db)) } /// Convert the `FunctionType` into a [`CallableType`]. @@ -1017,12 +1034,19 @@ impl<'db> FunctionType<'db> { } pub(crate) fn normalized_impl(self, db: &'db dyn Db, visitor: &NormalizedVisitor<'db>) -> Self { - let mappings: Box<_> = self - .type_mappings(db) - .iter() - .map(|mapping| mapping.normalized_impl(db, visitor)) - .collect(); - Self::new(db, self.literal(db).normalized_impl(db, visitor), mappings) + let literal = self.literal(db).normalized_impl(db, visitor); + let updated_signature = self + .updated_signature(db) + .map(|signature| signature.normalized_impl(db, visitor)); + let updated_last_definition_signature = self + .updated_last_definition_signature(db) + .map(|signature| signature.normalized_impl(db, visitor)); + Self::new( + db, + literal, + updated_signature, + updated_last_definition_signature, + ) } } diff --git a/crates/ty_python_semantic/src/types/generics.rs b/crates/ty_python_semantic/src/types/generics.rs index f7ada11007..45ff3ae689 100644 --- a/crates/ty_python_semantic/src/types/generics.rs +++ b/crates/ty_python_semantic/src/types/generics.rs @@ -1,13 +1,10 @@ -use std::borrow::Cow; - use itertools::Itertools; -use ruff_db::parsed::ParsedModuleRef; use ruff_python_ast as ast; use rustc_hash::FxHashMap; -use crate::semantic_index::SemanticIndex; use crate::semantic_index::definition::Definition; -use crate::semantic_index::scope::{FileScopeId, NodeWithScopeKind}; +use crate::semantic_index::scope::{FileScopeId, NodeWithScopeKind, ScopeId}; +use crate::semantic_index::{SemanticIndex, semantic_index}; use crate::types::class::ClassType; use crate::types::class_base::ClassBase; use crate::types::constraints::ConstraintSet; @@ -16,10 +13,10 @@ use crate::types::instance::{Protocol, ProtocolInstanceType}; use crate::types::signatures::{Parameter, Parameters, Signature}; use crate::types::tuple::{TupleSpec, TupleType, walk_tuple_type}; use crate::types::{ - ApplyTypeMappingVisitor, BoundTypeVarInstance, FindLegacyTypeVarsVisitor, HasRelationToVisitor, - IsEquivalentVisitor, KnownClass, KnownInstanceType, MaterializationKind, NormalizedVisitor, - Type, TypeMapping, TypeRelation, TypeVarBoundOrConstraints, TypeVarInstance, TypeVarKind, - TypeVarVariance, UnionType, binding_type, declaration_type, + ApplyTypeMappingVisitor, BoundTypeVarInstance, ClassLiteral, FindLegacyTypeVarsVisitor, + HasRelationToVisitor, IsEquivalentVisitor, KnownClass, KnownInstanceType, MaterializationKind, + NormalizedVisitor, Type, TypeMapping, TypeRelation, TypeVarBoundOrConstraints, TypeVarInstance, + TypeVarKind, TypeVarVariance, UnionType, binding_type, declaration_type, }; use crate::{Db, FxOrderSet}; @@ -27,7 +24,6 @@ use crate::{Db, FxOrderSet}; /// scope. fn enclosing_generic_contexts<'db>( db: &'db dyn Db, - module: &ParsedModuleRef, index: &SemanticIndex<'db>, scope: FileScopeId, ) -> impl Iterator> { @@ -35,13 +31,13 @@ fn enclosing_generic_contexts<'db>( .ancestor_scopes(scope) .filter_map(|(_, ancestor_scope)| match ancestor_scope.node() { NodeWithScopeKind::Class(class) => { - let definition = index.expect_single_definition(class.node(module)); + let definition = index.expect_single_definition(class); binding_type(db, definition) .into_class_literal()? .generic_context(db) } NodeWithScopeKind::Function(function) => { - let definition = index.expect_single_definition(function.node(module)); + let definition = index.expect_single_definition(function); infer_definition_types(db, definition) .undecorated_type() .expect("function should have undecorated type") @@ -50,7 +46,7 @@ fn enclosing_generic_contexts<'db>( .generic_context } NodeWithScopeKind::TypeAlias(type_alias) => { - let definition = index.expect_single_definition(type_alias.node(module)); + let definition = index.expect_single_definition(type_alias); binding_type(db, definition) .into_type_alias()? .into_pep_695_type_alias()? @@ -76,7 +72,6 @@ fn enclosing_generic_contexts<'db>( /// bind the typevar with that new binding context. pub(crate) fn bind_typevar<'db>( db: &'db dyn Db, - module: &ParsedModuleRef, index: &SemanticIndex<'db>, containing_scope: FileScopeId, typevar_binding_context: Option>, @@ -87,13 +82,13 @@ pub(crate) fn bind_typevar<'db>( for ((_, inner), (_, outer)) in index.ancestor_scopes(containing_scope).tuple_windows() { if outer.kind().is_class() { if let NodeWithScopeKind::Function(function) = inner.node() { - let definition = index.expect_single_definition(function.node(module)); + let definition = index.expect_single_definition(function); return Some(typevar.with_binding_context(db, definition)); } } } } - enclosing_generic_contexts(db, module, index, containing_scope) + enclosing_generic_contexts(db, index, containing_scope) .find_map(|enclosing_context| enclosing_context.binds_typevar(db, typevar)) .or_else(|| { typevar_binding_context.map(|typevar_binding_context| { @@ -102,6 +97,45 @@ pub(crate) fn bind_typevar<'db>( }) } +/// Create a `typing.Self` type variable for a given class. +pub(crate) fn typing_self<'db>( + db: &'db dyn Db, + scope_id: ScopeId, + typevar_binding_context: Option>, + class: ClassLiteral<'db>, + typevar_to_type: &impl Fn(BoundTypeVarInstance<'db>) -> Type<'db>, +) -> Option> { + let index = semantic_index(db, scope_id.file(db)); + + let typevar = TypeVarInstance::new( + db, + ast::name::Name::new_static("Self"), + Some(class.definition(db)), + Some( + TypeVarBoundOrConstraints::UpperBound(Type::instance( + db, + class.identity_specialization(db, typevar_to_type), + )) + .into(), + ), + // According to the [spec], we can consider `Self` + // equivalent to an invariant type variable + // [spec]: https://typing.python.org/en/latest/spec/generics.html#self + Some(TypeVarVariance::Invariant), + None, + TypeVarKind::TypingSelf, + ); + + bind_typevar( + db, + index, + scope_id.file_scope_id(db), + typevar_binding_context, + typevar, + ) + .map(typevar_to_type) +} + /// A list of formal type variables for a generic function, class, or type alias. /// /// TODO: Handle nested generic contexts better, with actual parent links to the lexically @@ -290,14 +324,17 @@ impl<'db> GenericContext<'db> { } /// Returns a specialization of this generic context where each typevar is mapped to itself. - /// (And in particular, to an _inferable_ version of itself, since this will be used in our - /// class constructor invocation machinery to infer a specialization for the class from the - /// arguments passed to its constructor.) - pub(crate) fn identity_specialization(self, db: &'db dyn Db) -> Specialization<'db> { + /// The second parameter can be `Type::TypeVar` or `Type::NonInferableTypeVar`, depending on + /// the use case. + pub(crate) fn identity_specialization( + self, + db: &'db dyn Db, + typevar_to_type: &impl Fn(BoundTypeVarInstance<'db>) -> Type<'db>, + ) -> Specialization<'db> { let types = self .variables(db) .iter() - .map(|typevar| Type::TypeVar(*typevar)) + .map(|typevar| typevar_to_type(*typevar)) .collect(); self.specialize(db, types) } @@ -391,7 +428,7 @@ impl<'db> GenericContext<'db> { // requirement for legacy contexts.) let partial = PartialSpecialization { generic_context: self, - types: Cow::Borrowed(&expanded[0..idx]), + types: &expanded[0..idx], }; let default = default.apply_type_mapping(db, &TypeMapping::PartialSpecialization(partial)); @@ -946,18 +983,7 @@ impl<'db> Specialization<'db> { #[derive(Clone, Debug, Eq, Hash, PartialEq, get_size2::GetSize)] pub struct PartialSpecialization<'a, 'db> { generic_context: GenericContext<'db>, - types: Cow<'a, [Type<'db>]>, -} - -pub(super) fn walk_partial_specialization<'db, V: super::visitor::TypeVisitor<'db> + ?Sized>( - db: &'db dyn Db, - specialization: &PartialSpecialization<'_, 'db>, - visitor: &V, -) { - walk_generic_context(db, specialization.generic_context, visitor); - for ty in &*specialization.types { - visitor.visit_type(db, *ty); - } + types: &'a [Type<'db>], } impl<'db> PartialSpecialization<'_, 'db> { @@ -974,31 +1000,6 @@ impl<'db> PartialSpecialization<'_, 'db> { .get_index_of(&bound_typevar)?; self.types.get(index).copied() } - - pub(crate) fn to_owned(&self) -> PartialSpecialization<'db, 'db> { - PartialSpecialization { - generic_context: self.generic_context, - types: Cow::from(self.types.clone().into_owned()), - } - } - - pub(crate) fn normalized_impl( - &self, - db: &'db dyn Db, - visitor: &NormalizedVisitor<'db>, - ) -> PartialSpecialization<'db, 'db> { - let generic_context = self.generic_context.normalized_impl(db, visitor); - let types: Cow<_> = self - .types - .iter() - .map(|ty| ty.normalized_impl(db, visitor)) - .collect(); - - PartialSpecialization { - generic_context, - types, - } - } } /// Performs type inference between parameter annotations and argument types, producing a diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index 3b58e51d59..9177d9a03e 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -92,9 +92,9 @@ use crate::types::{ DynamicType, IntersectionBuilder, IntersectionType, KnownClass, KnownInstanceType, MemberLookupPolicy, MetaclassCandidate, PEP695TypeAliasType, Parameter, ParameterForm, Parameters, SpecialFormType, SubclassOfType, Truthiness, Type, TypeAliasType, - TypeAndQualifiers, TypeContext, TypeMapping, TypeQualifiers, - TypeVarBoundOrConstraintsEvaluation, TypeVarDefaultEvaluation, TypeVarInstance, TypeVarKind, - UnionBuilder, UnionType, ValidSpecializationsConstraintSet, binding_type, todo_type, + TypeAndQualifiers, TypeContext, TypeQualifiers, TypeVarBoundOrConstraintsEvaluation, + TypeVarDefaultEvaluation, TypeVarInstance, TypeVarKind, UnionBuilder, UnionType, + ValidSpecializationsConstraintSet, binding_type, todo_type, }; use crate::types::{ClassBase, add_inferred_python_version_hint_to_diagnostic}; use crate::unpack::{EvaluationMode, UnpackPosition}; @@ -2144,12 +2144,8 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { let function_literal = FunctionLiteral::new(self.db(), overload_literal, inherited_generic_context); - let type_mappings = Box::default(); - let mut inferred_ty = Type::FunctionLiteral(FunctionType::new( - self.db(), - function_literal, - type_mappings, - )); + let mut inferred_ty = + Type::FunctionLiteral(FunctionType::new(self.db(), function_literal, None, None)); self.undecorated_type = Some(inferred_ty); for (decorator_ty, decorator_node) in decorator_types_and_nodes.iter().rev() { @@ -5436,8 +5432,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { // Convert any element literals to their promoted type form to avoid excessively large // unions for large nested list literals, which the constraint solver struggles with. - let inferred_elt_ty = - inferred_elt_ty.apply_type_mapping(self.db(), &TypeMapping::PromoteLiterals); + let inferred_elt_ty = inferred_elt_ty.promote_literals(self.db()); builder .infer(Type::TypeVar(*elt_ty), inferred_elt_ty) @@ -5954,7 +5949,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { // but constructor calls to `tuple[int]`, `tuple[int, ...]`, `tuple[int, *tuple[str, ...]]` (etc.) // are handled by the default constructor-call logic (we synthesize a `__new__` method for them // in `ClassType::own_class_member()`). - class.is_known(self.db(), KnownClass::Tuple) && class.is_not_generic() + class.is_known(self.db(), KnownClass::Tuple) && !class.is_generic() ); // temporary special-casing for all subclasses of `enum.Enum` @@ -8989,7 +8984,6 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { if let Type::KnownInstance(KnownInstanceType::TypeVar(typevar)) = typevar { bind_typevar( self.db(), - self.module(), self.index, self.scope().file_scope_id(self.db()), self.typevar_binding_context, diff --git a/crates/ty_python_semantic/src/types/infer/builder/type_expression.rs b/crates/ty_python_semantic/src/types/infer/builder/type_expression.rs index c0c0483c7e..3bca7f1eca 100644 --- a/crates/ty_python_semantic/src/types/infer/builder/type_expression.rs +++ b/crates/ty_python_semantic/src/types/infer/builder/type_expression.rs @@ -1286,7 +1286,17 @@ impl<'db> TypeInferenceBuilder<'db, '_> { Type::unknown() } - _ => TypeIsType::unbound(self.db(), self.infer_type_expression(arguments_slice)), + _ => TypeIsType::unbound( + self.db(), + // N.B. Using the top materialization here is a pragmatic decision + // that makes us produce more intuitive results given how + // `TypeIs` is used in the real world (in particular, in typeshed). + // However, there's some debate about whether this is really + // fully correct. See + // for more discussion. + self.infer_type_expression(arguments_slice) + .top_materialization(self.db()), + ), }, SpecialFormType::TypeGuard => { self.infer_type_expression(arguments_slice); diff --git a/crates/ty_python_semantic/src/types/signatures.rs b/crates/ty_python_semantic/src/types/signatures.rs index d5ae64fdaf..46d0438c04 100644 --- a/crates/ty_python_semantic/src/types/signatures.rs +++ b/crates/ty_python_semantic/src/types/signatures.rs @@ -13,20 +13,58 @@ use std::{collections::HashMap, slice::Iter}; use itertools::{EitherOrBoth, Itertools}; +use ruff_python_ast::ParameterWithDefault; use smallvec::{SmallVec, smallvec_inline}; -use super::{DynamicType, Type, TypeVarVariance, definition_expression_type}; +use super::{ + DynamicType, Type, TypeVarVariance, definition_expression_type, infer_definition_types, + semantic_index, +}; use crate::semantic_index::definition::Definition; use crate::types::constraints::{ConstraintSet, IteratorConstraintsExtension}; -use crate::types::generics::{GenericContext, walk_generic_context}; +use crate::types::function::FunctionType; +use crate::types::generics::{GenericContext, typing_self, walk_generic_context}; +use crate::types::infer::nearest_enclosing_class; use crate::types::{ - ApplyTypeMappingVisitor, BindingContext, BoundTypeVarInstance, FindLegacyTypeVarsVisitor, - HasRelationToVisitor, IsEquivalentVisitor, KnownClass, MaterializationKind, NormalizedVisitor, - TypeMapping, TypeRelation, VarianceInferable, todo_type, + ApplyTypeMappingVisitor, BindingContext, BoundTypeVarInstance, ClassType, + FindLegacyTypeVarsVisitor, HasRelationToVisitor, IsEquivalentVisitor, KnownClass, + MaterializationKind, NormalizedVisitor, TypeMapping, TypeRelation, VarianceInferable, + todo_type, }; use crate::{Db, FxOrderSet}; use ruff_python_ast::{self as ast, name::Name}; +#[derive(Clone, Copy, Debug)] +struct MethodInformation<'db> { + method: FunctionType<'db>, + class: ClassType<'db>, +} + +fn infer_method_information<'db>( + db: &'db dyn Db, + definition: Definition<'db>, +) -> Option> { + let class_scope_id = definition.scope(db); + let file = class_scope_id.file(db); + let index = semantic_index(db, file); + + let class_scope = index.scope(class_scope_id.file_scope_id(db)); + let class_node = class_scope.node().as_class()?; + + let method = infer_definition_types(db, definition) + .declaration_type(definition) + .inner_type() + .into_function_literal()?; + + let class_def = index.expect_single_definition(class_node); + let class_literal = infer_definition_types(db, class_def) + .declaration_type(class_def) + .inner_type(); + let class = class_literal.to_class_type(db)?; + + Some(MethodInformation { method, class }) +} + /// The signature of a single callable. If the callable is overloaded, there is a separate /// [`Signature`] for each overload. #[derive(Clone, Debug, PartialEq, Eq, Hash, salsa::Update, get_size2::GetSize)] @@ -62,6 +100,17 @@ impl<'db> CallableSignature<'db> { self.overloads.iter() } + pub(crate) fn with_inherited_generic_context( + &self, + inherited_generic_context: Option>, + ) -> Self { + Self::from_overloads(self.overloads.iter().map(|signature| { + signature + .clone() + .with_inherited_generic_context(inherited_generic_context) + })) + } + pub(crate) fn normalized_impl( &self, db: &'db dyn Db, @@ -451,14 +500,6 @@ impl<'db> Signature<'db> { } } - pub(crate) fn apply_type_mapping<'a>( - &self, - db: &'db dyn Db, - type_mapping: &TypeMapping<'a, 'db>, - ) -> Self { - self.apply_type_mapping_impl(db, type_mapping, &ApplyTypeMappingVisitor::default()) - } - pub(crate) fn apply_type_mapping_impl<'a>( &self, db: &'db dyn Db, @@ -1182,16 +1223,72 @@ impl<'db> Parameters<'db> { .map(|default| definition_expression_type(db, definition, default)) }; + let method_info = infer_method_information(db, definition); + let is_static_or_classmethod = method_info + .is_some_and(|f| f.method.is_staticmethod(db) || f.method.is_classmethod(db)); + + let inferred_annotation = |arg: &ParameterWithDefault| { + if let Some(MethodInformation { method, class }) = method_info + && !is_static_or_classmethod + && arg.parameter.annotation().is_none() + && parameters.index(arg.name().id()) == Some(0) + { + let method_has_self_in_generic_context = + method.signature(db).overloads.iter().any(|s| { + s.generic_context.is_some_and(|context| { + context + .variables(db) + .iter() + .any(|v| v.typevar(db).is_self(db)) + }) + }); + + if method_has_self_in_generic_context + || class.is_generic() + || class.known(db).is_some_and(KnownClass::is_fallback_class) + { + let scope_id = definition.scope(db); + let typevar_binding_context = Some(definition); + let index = semantic_index(db, scope_id.file(db)); + let class = nearest_enclosing_class(db, index, scope_id).unwrap(); + + Some( + typing_self(db, scope_id, typevar_binding_context, class, &Type::TypeVar) + .expect("We should always find the surrounding class for an implicit self: Self annotation"), + ) + } else { + // For methods of non-generic classes that are not otherwise generic (e.g. return `Self` or + // have additional type parameters), the implicit `Self` type of the `self` parameter would + // be the only type variable, so we can just use the class directly. + Some(Type::instance(db, class)) + } + } else { + None + } + }; + let pos_only_param = |param: &ast::ParameterWithDefault| { - Parameter::from_node_and_kind( - db, - definition, - ¶m.parameter, - ParameterKind::PositionalOnly { - name: Some(param.parameter.name.id.clone()), - default_type: default_type(param), - }, - ) + if let Some(inferred_annotation_type) = inferred_annotation(param) { + Parameter { + annotated_type: Some(inferred_annotation_type), + inferred_annotation: true, + kind: ParameterKind::PositionalOnly { + name: Some(param.parameter.name.id.clone()), + default_type: default_type(param), + }, + form: ParameterForm::Value, + } + } else { + Parameter::from_node_and_kind( + db, + definition, + ¶m.parameter, + ParameterKind::PositionalOnly { + name: Some(param.parameter.name.id.clone()), + default_type: default_type(param), + }, + ) + } }; let mut positional_only: Vec = posonlyargs.iter().map(pos_only_param).collect(); @@ -1215,15 +1312,27 @@ impl<'db> Parameters<'db> { } let positional_or_keyword = pos_or_keyword_iter.map(|arg| { - Parameter::from_node_and_kind( - db, - definition, - &arg.parameter, - ParameterKind::PositionalOrKeyword { - name: arg.parameter.name.id.clone(), - default_type: default_type(arg), - }, - ) + if let Some(inferred_annotation_type) = inferred_annotation(arg) { + Parameter { + annotated_type: Some(inferred_annotation_type), + inferred_annotation: true, + kind: ParameterKind::PositionalOrKeyword { + name: arg.parameter.name.id.clone(), + default_type: default_type(arg), + }, + form: ParameterForm::Value, + } + } else { + Parameter::from_node_and_kind( + db, + definition, + &arg.parameter, + ParameterKind::PositionalOrKeyword { + name: arg.parameter.name.id.clone(), + default_type: default_type(arg), + }, + ) + } }); let variadic = vararg.as_ref().map(|arg| { @@ -1396,6 +1505,10 @@ pub(crate) struct Parameter<'db> { /// Annotated type of the parameter. annotated_type: Option>, + /// Does the type of this parameter come from an explicit annotation, or was it inferred from + /// the context, like `Self` for the `self` parameter of instance methods. + inferred_annotation: bool, + kind: ParameterKind<'db>, pub(crate) form: ParameterForm, } @@ -1404,6 +1517,7 @@ impl<'db> Parameter<'db> { pub(crate) fn positional_only(name: Option) -> Self { Self { annotated_type: None, + inferred_annotation: false, kind: ParameterKind::PositionalOnly { name, default_type: None, @@ -1415,6 +1529,7 @@ impl<'db> Parameter<'db> { pub(crate) fn positional_or_keyword(name: Name) -> Self { Self { annotated_type: None, + inferred_annotation: false, kind: ParameterKind::PositionalOrKeyword { name, default_type: None, @@ -1426,6 +1541,7 @@ impl<'db> Parameter<'db> { pub(crate) fn variadic(name: Name) -> Self { Self { annotated_type: None, + inferred_annotation: false, kind: ParameterKind::Variadic { name }, form: ParameterForm::Value, } @@ -1434,6 +1550,7 @@ impl<'db> Parameter<'db> { pub(crate) fn keyword_only(name: Name) -> Self { Self { annotated_type: None, + inferred_annotation: false, kind: ParameterKind::KeywordOnly { name, default_type: None, @@ -1445,6 +1562,7 @@ impl<'db> Parameter<'db> { pub(crate) fn keyword_variadic(name: Name) -> Self { Self { annotated_type: None, + inferred_annotation: false, kind: ParameterKind::KeywordVariadic { name }, form: ParameterForm::Value, } @@ -1483,6 +1601,7 @@ impl<'db> Parameter<'db> { .annotated_type .map(|ty| ty.apply_type_mapping_impl(db, type_mapping, visitor)), kind: self.kind.apply_type_mapping_impl(db, type_mapping, visitor), + inferred_annotation: self.inferred_annotation, form: self.form, } } @@ -1498,6 +1617,7 @@ impl<'db> Parameter<'db> { ) -> Self { let Parameter { annotated_type, + inferred_annotation, kind, form, } = self; @@ -1541,6 +1661,7 @@ impl<'db> Parameter<'db> { Self { annotated_type: Some(annotated_type), + inferred_annotation: *inferred_annotation, kind, form: *form, } @@ -1563,6 +1684,7 @@ impl<'db> Parameter<'db> { }), kind, form: ParameterForm::Value, + inferred_annotation: false, } } @@ -1617,6 +1739,11 @@ impl<'db> Parameter<'db> { &self.kind } + /// Whether or not the type of this parameter should be displayed. + pub(crate) fn should_annotation_be_displayed(&self) -> bool { + !self.inferred_annotation + } + /// Name of the parameter (if it has one). pub(crate) fn name(&self) -> Option<&ast::name::Name> { match &self.kind { diff --git a/crates/ty_python_semantic/src/types/visitor.rs b/crates/ty_python_semantic/src/types/visitor.rs index aae08c5d9a..9ca0f98e4c 100644 --- a/crates/ty_python_semantic/src/types/visitor.rs +++ b/crates/ty_python_semantic/src/types/visitor.rs @@ -107,7 +107,7 @@ pub(crate) trait TypeVisitor<'db> { /// Enumeration of types that may contain other types, such as unions, intersections, and generics. #[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)] -pub(crate) enum NonAtomicType<'db> { +pub(super) enum NonAtomicType<'db> { Union(UnionType<'db>), Intersection(IntersectionType<'db>), FunctionLiteral(FunctionType<'db>), @@ -128,7 +128,7 @@ pub(crate) enum NonAtomicType<'db> { TypeAlias(TypeAliasType<'db>), } -pub(crate) enum TypeKind<'db> { +pub(super) enum TypeKind<'db> { Atomic, NonAtomic(NonAtomicType<'db>), } @@ -200,7 +200,7 @@ impl<'db> From> for TypeKind<'db> { } } -pub(crate) fn walk_non_atomic_type<'db, V: TypeVisitor<'db> + ?Sized>( +pub(super) fn walk_non_atomic_type<'db, V: TypeVisitor<'db> + ?Sized>( db: &'db dyn Db, non_atomic_type: NonAtomicType<'db>, visitor: &V, diff --git a/crates/ty_vendored/ty_extensions/ty_extensions.pyi b/crates/ty_vendored/ty_extensions/ty_extensions.pyi index ccaf90a06e..def2cd0963 100644 --- a/crates/ty_vendored/ty_extensions/ty_extensions.pyi +++ b/crates/ty_vendored/ty_extensions/ty_extensions.pyi @@ -103,6 +103,6 @@ class NamedTupleLike(Protocol): @classmethod def _make(cls: type[Self], iterable: Iterable[Any]) -> Self: ... def _asdict(self, /) -> dict[str, Any]: ... - def _replace(self: Self, /, **kwargs) -> Self: ... + def _replace(self, /, **kwargs) -> Self: ... if sys.version_info >= (3, 13): - def __replace__(self: Self, **kwargs) -> Self: ... + def __replace__(self, **kwargs) -> Self: ... diff --git a/docs/requirements-insiders.txt b/docs/requirements-insiders.txt index f68acbaa77..7a81ca1bd0 100644 --- a/docs/requirements-insiders.txt +++ b/docs/requirements-insiders.txt @@ -1,5 +1,5 @@ -PyYAML==6.0.2 -ruff==0.13.1 +PyYAML==6.0.3 +ruff==0.13.2 mkdocs==1.6.1 mkdocs-material @ git+ssh://git@github.com/astral-sh/mkdocs-material-insiders.git@39da7a5e761410349e9a1b8abf593b0cdd5453ff mkdocs-redirects==1.2.2 diff --git a/docs/requirements.txt b/docs/requirements.txt index 10748e0b6c..f406db67b3 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -1,5 +1,5 @@ -PyYAML==6.0.2 -ruff==0.13.1 +PyYAML==6.0.3 +ruff==0.13.2 mkdocs==1.6.1 mkdocs-material==9.5.38 mkdocs-redirects==1.2.2 diff --git a/playground/ruff/src/Editor/SourceEditor.tsx b/playground/ruff/src/Editor/SourceEditor.tsx index eb39a8c063..1dd8b20a64 100644 --- a/playground/ruff/src/Editor/SourceEditor.tsx +++ b/playground/ruff/src/Editor/SourceEditor.tsx @@ -125,17 +125,16 @@ class RuffCodeActionProvider implements CodeActionProvider { ): languages.ProviderResult { const actions = this.diagnostics // Show fixes for any diagnostic whose range intersects the requested range - .filter((check) => - Range.areIntersecting( - new Range( - check.start_location.row, - check.start_location.column, - check.end_location.row, - check.end_location.column, - ), - range, - ), - ) + .filter((check) => { + const diagnosticRange = new Range( + check.start_location.row, + check.start_location.column, + check.end_location.row, + check.end_location.column, + ); + + return Range.areIntersectingOrTouching(diagnosticRange, range); + }) .filter(({ fix }) => fix) .map((check) => ({ title: check.fix