From 57e1ff8294d630b0f265d875caad80e32c8ee6b6 Mon Sep 17 00:00:00 2001 From: Dylan Date: Fri, 26 Sep 2025 08:22:26 -0500 Subject: [PATCH 01/21] [`pyflakes`] Handle some common submodule import situations for `unused-import` (`F401`) (#20200) # Summary The PR under review attempts to make progress towards the age-old problem of submodule imports, specifically with regards to their treatment by the rule [`unused-import` (`F401`)](https://docs.astral.sh/ruff/rules/unused-import/). Some related issues: - https://github.com/astral-sh/ruff/issues/60 - https://github.com/astral-sh/ruff/issues/4656 Prior art: - https://github.com/astral-sh/ruff/pull/13965 - https://github.com/astral-sh/ruff/pull/5010 - https://github.com/astral-sh/ruff/pull/5011 - https://github.com/astral-sh/ruff/pull/666 See the PR summary for a detailed description. --- .../integration_test__rule_f401.snap | 39 ++ .../snapshots/lint__output_format_sarif.snap | 2 +- ...inter__message__sarif__tests__results.snap | 2 +- crates/ruff_linter/src/preview.rs | 5 + crates/ruff_linter/src/rules/pyflakes/mod.rs | 150 +++++++- .../src/rules/pyflakes/rules/unused_import.rs | 361 +++++++++++++++++- ...1_import_submodules_but_use_top_level.snap | 16 + ...s_different_lengths_but_use_top_level.snap | 16 + ...1_import_submodules_in_function_scope.snap | 19 + ...ests__f401_multiple_unused_submodules.snap | 44 +++ ...s__tests__f401_use_in_between_imports.snap | 4 + ...flakes__tests__f401_use_in_dunder_all.snap | 16 + ..._pyflakes__tests__f401_use_top_member.snap | 16 + ...ber_and_redefine_before_second_import.snap | 4 + ...1_use_top_member_before_second_import.snap | 4 + ..._top_member_then_import_then_redefine.snap | 4 + ...kes__tests__f401_use_top_member_twice.snap | 18 + ...lakes__tests__preview_diff__F401_0.py.snap | 50 +++ ...lakes__tests__preview_diff__F401_1.py.snap | 10 + ...akes__tests__preview_diff__F401_10.py.snap | 10 + ...akes__tests__preview_diff__F401_11.py.snap | 10 + ...akes__tests__preview_diff__F401_12.py.snap | 10 + ...akes__tests__preview_diff__F401_13.py.snap | 10 + ...akes__tests__preview_diff__F401_14.py.snap | 10 + ...akes__tests__preview_diff__F401_15.py.snap | 10 + ...akes__tests__preview_diff__F401_16.py.snap | 10 + ...akes__tests__preview_diff__F401_17.py.snap | 10 + ...akes__tests__preview_diff__F401_18.py.snap | 10 + ...akes__tests__preview_diff__F401_19.py.snap | 10 + ...lakes__tests__preview_diff__F401_2.py.snap | 10 + ...akes__tests__preview_diff__F401_20.py.snap | 10 + ...akes__tests__preview_diff__F401_21.py.snap | 10 + ...akes__tests__preview_diff__F401_22.py.snap | 10 + ...akes__tests__preview_diff__F401_23.py.snap | 10 + ...lakes__tests__preview_diff__F401_3.py.snap | 10 + ...akes__tests__preview_diff__F401_32.py.snap | 10 + ...akes__tests__preview_diff__F401_34.py.snap | 10 + ...akes__tests__preview_diff__F401_35.py.snap | 10 + ...lakes__tests__preview_diff__F401_4.py.snap | 10 + ...lakes__tests__preview_diff__F401_5.py.snap | 10 + ...lakes__tests__preview_diff__F401_6.py.snap | 10 + ...lakes__tests__preview_diff__F401_7.py.snap | 10 + ...lakes__tests__preview_diff__F401_8.py.snap | 10 + ...lakes__tests__preview_diff__F401_9.py.snap | 10 + 44 files changed, 1012 insertions(+), 18 deletions(-) create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_import_submodules_but_use_top_level.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_import_submodules_different_lengths_but_use_top_level.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_import_submodules_in_function_scope.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_multiple_unused_submodules.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_in_between_imports.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_in_dunder_all.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_top_member.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_top_member_and_redefine_before_second_import.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_top_member_before_second_import.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_top_member_then_import_then_redefine.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f401_use_top_member_twice.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_0.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_1.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_10.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_11.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_12.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_13.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_14.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_15.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_16.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_17.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_18.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_19.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_2.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_20.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_21.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_22.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_23.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_3.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_32.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_34.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_35.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_4.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_5.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_6.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_7.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_8.py.snap create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview_diff__F401_9.py.snap 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_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/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 From e4de179cddd2683eb958db5c619289e017d482eb Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 26 Sep 2025 17:00:10 +0100 Subject: [PATCH 02/21] [ty] Simplify `Any | (Any & T)` to `Any` (#20593) --- .../resources/mdtest/type_properties/is_equivalent_to.md | 5 +++++ crates/ty_python_semantic/src/types.rs | 7 +++++++ crates/ty_python_semantic/src/types/builder.rs | 9 ++++++++- 3 files changed, 20 insertions(+), 1 deletion(-) 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/src/types.rs b/crates/ty_python_semantic/src/types.rs index b2cb2d61f1..7b643a168a 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -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> { 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 From 6b3c493cff01ad1ab51102048d0d4f489b1e1aa4 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 26 Sep 2025 18:24:43 +0100 Subject: [PATCH 03/21] [ty] Use `Top` materializations for `TypeIs` special form (#20591) --- .../resources/mdtest/narrow/type_guards.md | 37 +++++++++++++++++++ .../types/infer/builder/type_expression.rs | 12 +++++- 2 files changed, 48 insertions(+), 1 deletion(-) 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/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); From 4e335011153ba570adf14f49e231e11427cc214e Mon Sep 17 00:00:00 2001 From: Rahul Sahoo Date: Sat, 27 Sep 2025 19:20:51 +0530 Subject: [PATCH 04/21] Fixed documentation for try_consider_else (#20587) ## Summary This PR addresses #20570 . In the example, the correct usage had a bug/issue where in the except block after logging exception, None was getting returned, which made the linters flag out the code. So adding an empty raise solves the issue. ## Test Plan Tested it by building the doc locally. --- .../src/rules/tryceratops/rules/try_consider_else.rs | 2 ++ 1 file changed, 2 insertions(+) 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 From 256b520a528d47d1b6ac694c137ef50c2f9155de Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 29 Sep 2025 08:55:13 +0200 Subject: [PATCH 05/21] Update cargo-bins/cargo-binstall action to v1.15.6 (#20620) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- .github/workflows/ci.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 From 65e805de62c1347c26e58d82d55dff9deacea984 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 29 Sep 2025 08:55:39 +0200 Subject: [PATCH 06/21] Update dependency PyYAML to v6.0.3 (#20621) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- docs/requirements-insiders.txt | 2 +- docs/requirements.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/requirements-insiders.txt b/docs/requirements-insiders.txt index f68acbaa77..e9202082de 100644 --- a/docs/requirements-insiders.txt +++ b/docs/requirements-insiders.txt @@ -1,4 +1,4 @@ -PyYAML==6.0.2 +PyYAML==6.0.3 ruff==0.13.1 mkdocs==1.6.1 mkdocs-material @ git+ssh://git@github.com/astral-sh/mkdocs-material-insiders.git@39da7a5e761410349e9a1b8abf593b0cdd5453ff diff --git a/docs/requirements.txt b/docs/requirements.txt index 10748e0b6c..9622f089ba 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -1,4 +1,4 @@ -PyYAML==6.0.2 +PyYAML==6.0.3 ruff==0.13.1 mkdocs==1.6.1 mkdocs-material==9.5.38 From 666f53331ffe752ecab5bc8226fbca91cf3b8552 Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Mon, 29 Sep 2025 15:56:23 +0900 Subject: [PATCH 07/21] [`ruff`] Fix minor typos in doc comments (#20623) --- crates/ruff_benchmark/benches/ty.rs | 2 +- crates/ruff_linter/src/checkers/ast/mod.rs | 2 +- crates/ruff_python_semantic/src/model.rs | 2 +- crates/ty_ide/src/completion.rs | 2 +- crates/ty_ide/src/importer.rs | 2 +- crates/ty_project/src/glob.rs | 2 +- crates/ty_python_semantic/src/semantic_index/builder.rs | 2 +- crates/ty_python_semantic/src/types.rs | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) 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_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_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_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_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 7b643a168a..7d468058bd 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -3152,7 +3152,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__` From e5faf6c2688b576445c6f13038ddd67ec6cd33fe Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 29 Sep 2025 08:59:50 +0200 Subject: [PATCH 08/21] Update dependency ruff to v0.13.2 (#20622) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Coming soon: The Renovate bot (GitHub App) will be renamed to Mend. PRs from Renovate will soon appear from 'Mend'. Learn more [here](https://redirect.github.com/renovatebot/renovate/discussions/37842). This PR contains the following updates: | Package | Change | Age | Confidence | |---|---|---|---| | [ruff](https://docs.astral.sh/ruff) ([source](https://redirect.github.com/astral-sh/ruff), [changelog](https://redirect.github.com/astral-sh/ruff/blob/main/CHANGELOG.md)) | `==0.13.1` -> `==0.13.2` | [![age](https://developer.mend.io/api/mc/badges/age/pypi/ruff/0.13.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/ruff/0.13.1/0.13.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes
astral-sh/ruff (ruff) ### [`v0.13.2`](https://redirect.github.com/astral-sh/ruff/blob/HEAD/CHANGELOG.md#0132) [Compare Source](https://redirect.github.com/astral-sh/ruff/compare/0.13.1...0.13.2) Released on 2025-09-25. ##### Preview features - \[`flake8-async`] Implement `blocking-path-method` (`ASYNC240`) ([#​20264](https://redirect.github.com/astral-sh/ruff/pull/20264)) - \[`flake8-bugbear`] Implement `map-without-explicit-strict` (`B912`) ([#​20429](https://redirect.github.com/astral-sh/ruff/pull/20429)) - \[`flake8-bultins`] Detect class-scope builtin shadowing in decorators, default args, and attribute initializers (`A003`) ([#​20178](https://redirect.github.com/astral-sh/ruff/pull/20178)) - \[`ruff`] Implement `logging-eager-conversion` (`RUF065`) ([#​19942](https://redirect.github.com/astral-sh/ruff/pull/19942)) - Include `.pyw` files by default when linting and formatting ([#​20458](https://redirect.github.com/astral-sh/ruff/pull/20458)) ##### Bug fixes - Deduplicate input paths ([#​20105](https://redirect.github.com/astral-sh/ruff/pull/20105)) - \[`flake8-comprehensions`] Preserve trailing commas for single-element lists (`C409`) ([#​19571](https://redirect.github.com/astral-sh/ruff/pull/19571)) - \[`flake8-pyi`] Avoid syntax error from conflict with `PIE790` (`PYI021`) ([#​20010](https://redirect.github.com/astral-sh/ruff/pull/20010)) - \[`flake8-simplify`] Correct fix for positive `maxsplit` without separator (`SIM905`) ([#​20056](https://redirect.github.com/astral-sh/ruff/pull/20056)) - \[`pyupgrade`] Fix `UP008` not to apply when `__class__` is a local variable ([#​20497](https://redirect.github.com/astral-sh/ruff/pull/20497)) - \[`ruff`] Fix `B004` to skip invalid `hasattr`/`getattr` calls ([#​20486](https://redirect.github.com/astral-sh/ruff/pull/20486)) - \[`ruff`] Replace `-nan` with `nan` when using the value to construct a `Decimal` (`FURB164` ) ([#​20391](https://redirect.github.com/astral-sh/ruff/pull/20391)) ##### Documentation - Add 'Finding ways to help' to CONTRIBUTING.md ([#​20567](https://redirect.github.com/astral-sh/ruff/pull/20567)) - Update import path to `ruff-wasm-web` ([#​20539](https://redirect.github.com/astral-sh/ruff/pull/20539)) - \[`flake8-bandit`] Clarify the supported hashing functions (`S324`) ([#​20534](https://redirect.github.com/astral-sh/ruff/pull/20534)) ##### Other changes - \[`playground`] Allow hover quick fixes to appear for overlapping diagnostics ([#​20527](https://redirect.github.com/astral-sh/ruff/pull/20527)) - \[`playground`] Fix non‑BMP code point handling in quick fixes and markers ([#​20526](https://redirect.github.com/astral-sh/ruff/pull/20526)) ##### Contributors - [@​BurntSushi](https://redirect.github.com/BurntSushi) - [@​mtshiba](https://redirect.github.com/mtshiba) - [@​second-ed](https://redirect.github.com/second-ed) - [@​danparizher](https://redirect.github.com/danparizher) - [@​ShikChen](https://redirect.github.com/ShikChen) - [@​PieterCK](https://redirect.github.com/PieterCK) - [@​GDYendell](https://redirect.github.com/GDYendell) - [@​RazerM](https://redirect.github.com/RazerM) - [@​TaKO8Ki](https://redirect.github.com/TaKO8Ki) - [@​amyreese](https://redirect.github.com/amyreese) - [@​ntbre](https://redirect.github.com/ntBre) - [@​MichaReiser](https://redirect.github.com/MichaReiser)
--- ### Configuration 📅 **Schedule**: Branch creation - "before 4am on Monday" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. â™» **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/astral-sh/ruff). Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- docs/requirements-insiders.txt | 2 +- docs/requirements.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/requirements-insiders.txt b/docs/requirements-insiders.txt index e9202082de..7a81ca1bd0 100644 --- a/docs/requirements-insiders.txt +++ b/docs/requirements-insiders.txt @@ -1,5 +1,5 @@ PyYAML==6.0.3 -ruff==0.13.1 +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 9622f089ba..f406db67b3 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -1,5 +1,5 @@ PyYAML==6.0.3 -ruff==0.13.1 +ruff==0.13.2 mkdocs==1.6.1 mkdocs-material==9.5.38 mkdocs-redirects==1.2.2 From 053c750c9383ee166cab41317faeed767ac378bd Mon Sep 17 00:00:00 2001 From: Dan Parizher <105245560+danparizher@users.noreply.github.com> Date: Mon, 29 Sep 2025 03:38:32 -0400 Subject: [PATCH 09/21] [`playground`] Fix quick fixes for empty ranges in playground (#20599) Co-authored-by: Micha Reiser --- playground/ruff/src/Editor/SourceEditor.tsx | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) 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 From 81f43a1fc8bea4839997d95774b1fcbf6888e282 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Mon, 29 Sep 2025 08:46:01 +0100 Subject: [PATCH 10/21] Add the *The Basics* title back to CONTRIBUTING.md (#20624) --- CONTRIBUTING.md | 2 ++ 1 file changed, 2 insertions(+) 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 From 3f640dacd42666f83d1a36d29efc507d31116060 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 29 Sep 2025 11:43:11 +0100 Subject: [PATCH 11/21] [ty] Improve disambiguation of class names in diagnostics (#20603) --- .../mdtest/diagnostics/same_names.md | 21 +- .../resources/mdtest/public_types.md | 2 +- crates/ty_python_semantic/src/types.rs | 6 +- .../src/types/diagnostic.rs | 6 +- .../ty_python_semantic/src/types/display.rs | 356 +++++++++++------- .../ty_python_semantic/src/types/visitor.rs | 6 +- 6 files changed, 243 insertions(+), 154 deletions(-) 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..f55d7d106c 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 ``` @@ -228,3 +227,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/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/src/types.rs b/crates/ty_python_semantic/src/types.rs index 7d468058bd..c6d85c037d 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -7065,7 +7065,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/diagnostic.rs b/crates/ty_python_semantic/src/types/diagnostic.rs index 214eb30749..29d84041f3 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!( diff --git a/crates/ty_python_semantic/src/types/display.rs b/crates/ty_python_semantic/src/types/display.rs index a49d563807..a2043c9b28 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,149 @@ 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, + MaterializationKind, Protocol, StringLiteralType, SubclassOfInner, Type, 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)), + _ => {} + } + + 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 +171,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 +200,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 +212,7 @@ impl<'db> ClassLiteral<'db> { struct ClassDisplay<'db> { db: &'db dyn Db, class: ClassLiteral<'db>, - settings: DisplaySettings, + settings: DisplaySettings<'db>, } impl ClassDisplay<'_> { @@ -224,7 +259,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 +279,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 +297,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 +343,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 +360,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 +376,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 +387,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 +401,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 +451,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 +471,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 +507,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 +542,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 +555,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 +629,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 +642,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 +651,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 +659,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 +668,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 +681,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 +693,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 +701,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 +712,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 +727,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 +742,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 +763,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 +782,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 +795,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 +804,7 @@ impl Display for DisplayOptionalGenericContext<'_> { DisplayGenericContext { generic_context, db: self.db, - settings: self.settings, + settings: self.settings.clone(), } .fmt(f) } else { @@ -767,7 +817,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 +850,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 +865,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 +875,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 +912,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 +925,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 +942,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 +962,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 +977,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 +1181,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 +1194,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<'_> { @@ -1152,21 +1205,29 @@ impl Display for DisplayParameter<'_> { write!( f, ": {}", - annotated_type.display_with(self.db, self.settings) + 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 +1237,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 +1250,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 +1310,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 +1331,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 +1344,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 +1384,7 @@ struct DisplayMaybeNegatedType<'db> { ty: Type<'db>, db: &'db dyn Db, negated: bool, - settings: DisplaySettings, + settings: DisplaySettings<'db>, } impl Display for DisplayMaybeNegatedType<'_> { @@ -1334,7 +1395,7 @@ impl Display for DisplayMaybeNegatedType<'_> { DisplayMaybeParenthesizedType { ty: self.ty, db: self.db, - settings: self.settings, + settings: self.settings.clone(), } .fmt(f) } @@ -1343,13 +1404,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 +1425,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 +1456,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 +1470,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 +1483,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 +1502,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/visitor.rs b/crates/ty_python_semantic/src/types/visitor.rs index 35d9d5f1a1..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)] -enum NonAtomicType<'db> { +pub(super) enum NonAtomicType<'db> { Union(UnionType<'db>), Intersection(IntersectionType<'db>), FunctionLiteral(FunctionType<'db>), @@ -128,7 +128,7 @@ enum NonAtomicType<'db> { TypeAlias(TypeAliasType<'db>), } -enum TypeKind<'db> { +pub(super) enum TypeKind<'db> { Atomic, NonAtomic(NonAtomicType<'db>), } @@ -200,7 +200,7 @@ impl<'db> From> for TypeKind<'db> { } } -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, From cf2b083668f3ce69c430996227e038a5ad8b36ae Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Mon, 29 Sep 2025 07:24:40 -0400 Subject: [PATCH 12/21] [ty] Apply type mappings to functions eagerly (#20596) `TypeMapping` is no longer cow-shaped. Before, `TypeMapping` defined a `to_owned` method, which would make an owned copy of the type mapping. This let us apply type mappings to function literals lazily. The primary part of a function that you have to apply the type mapping to is its signature. The hypothesis was that doing this lazily would prevent us from constructing the signature of a function just to apply a type mapping; if you never ended up needed the updated function signature, that would be extraneous work. But looking at the CI for this PR, it looks like that hypothesis is wrong! And this definitely cleans up the code quite a bit. It also means that over time we can consider replacing all of these `TypeMapping` enum variants with separate `TypeTransformer` impls. --------- Co-authored-by: David Peter --- crates/ty_python_semantic/src/types.rs | 88 +---------- .../ty_python_semantic/src/types/function.rs | 146 ++++++++++-------- .../ty_python_semantic/src/types/generics.rs | 42 +---- .../src/types/infer/builder.rs | 8 +- .../src/types/signatures.rs | 19 ++- 5 files changed, 105 insertions(+), 198 deletions(-) diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index c6d85c037d..d52b909115 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -53,7 +53,6 @@ use crate::types::function::{ }; use crate::types::generics::{ GenericContext, PartialSpecialization, Specialization, bind_typevar, walk_generic_context, - walk_partial_specialization, walk_specialization, }; pub use crate::types::ide_support::{ CallSignatureDetails, Member, MemberWithDefinition, all_members, call_signature_details, @@ -6109,7 +6108,7 @@ 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) @@ -6120,8 +6119,8 @@ impl<'db> Type<'db> { 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 +6139,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), )) } @@ -6782,84 +6781,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, diff --git a/crates/ty_python_semantic/src/types/function.rs b/crates/ty_python_semantic/src/types/function.rs index ed10c7b819..c062ee3b1d 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, - TrackedConstraintSet, Truthiness, Type, TypeMapping, TypeRelation, UnionBuilder, 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, TrackedConstraintSet, Truthiness, Type, TypeMapping, TypeRelation, + UnionBuilder, 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 3eb227b647..c781d4a69e 100644 --- a/crates/ty_python_semantic/src/types/generics.rs +++ b/crates/ty_python_semantic/src/types/generics.rs @@ -1,5 +1,3 @@ -use std::borrow::Cow; - use crate::types::constraints::ConstraintSet; use itertools::Itertools; @@ -392,7 +390,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)); @@ -947,18 +945,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> { @@ -975,31 +962,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 79cd816b0a..f7eb128766 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -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() { diff --git a/crates/ty_python_semantic/src/types/signatures.rs b/crates/ty_python_semantic/src/types/signatures.rs index d5ae64fdaf..3d3868b06c 100644 --- a/crates/ty_python_semantic/src/types/signatures.rs +++ b/crates/ty_python_semantic/src/types/signatures.rs @@ -62,6 +62,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 +462,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, From 803d61e21f38521dbd21215f49f4f289cb86eda4 Mon Sep 17 00:00:00 2001 From: David Peter Date: Mon, 29 Sep 2025 13:36:14 +0200 Subject: [PATCH 13/21] [ty] Ecosystem analyzer: relax timeout thresholds (#20626) ## Summary Pull in a small upstream change (https://github.com/astral-sh/ecosystem-analyzer/commit/6ce3a609575bc84eaf5d247739529c60b6c2ae5b), because some type check times were close to the previous limits, which prevents us from seeing diagnostics diffs (in case they run into a timeout). --- .github/workflows/ty-ecosystem-analyzer.yaml | 2 +- .github/workflows/ty-ecosystem-report.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 \ From 1cf19732b94b1a11ec17f0375fd4d1a31ca8ea66 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 29 Sep 2025 13:02:07 +0100 Subject: [PATCH 14/21] [ty] Use fully qualified names to distinguish ambiguous protocols in diagnostics (#20627) --- .../mdtest/diagnostics/same_names.md | 30 +++++++++++++++++++ .../ty_python_semantic/src/types/display.rs | 11 +++++-- 2 files changed, 39 insertions(+), 2 deletions(-) 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 f55d7d106c..6e6dfac60d 100644 --- a/crates/ty_python_semantic/resources/mdtest/diagnostics/same_names.md +++ b/crates/ty_python_semantic/resources/mdtest/diagnostics/same_names.md @@ -170,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 diff --git a/crates/ty_python_semantic/src/types/display.rs b/crates/ty_python_semantic/src/types/display.rs index a2043c9b28..d3b18131b3 100644 --- a/crates/ty_python_semantic/src/types/display.rs +++ b/crates/ty_python_semantic/src/types/display.rs @@ -22,8 +22,8 @@ use crate::types::tuple::TupleSpec; use crate::types::visitor::TypeVisitor; use crate::types::{ BoundTypeVarInstance, CallableType, IntersectionType, KnownBoundMethodType, KnownClass, - MaterializationKind, Protocol, StringLiteralType, SubclassOfInner, Type, UnionType, - WrapperDescriptorKind, visitor, + MaterializationKind, Protocol, ProtocolInstanceType, StringLiteralType, SubclassOfInner, Type, + UnionType, WrapperDescriptorKind, visitor, }; use ruff_db::parsed::parsed_module; @@ -128,6 +128,13 @@ impl<'db> super::visitor::TypeVisitor<'db> for AmbiguousClassCollector<'db> { 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)), _ => {} } From 00c8851ef8d643434340577f1b1ee65b5e728ac1 Mon Sep 17 00:00:00 2001 From: Brent Westbrook <36778786+ntBre@users.noreply.github.com> Date: Mon, 29 Sep 2025 08:46:25 -0400 Subject: [PATCH 15/21] Remove `TextEmitter` (#20595) ## Summary Addresses https://github.com/astral-sh/ruff/pull/20443#discussion_r2381237640 by factoring out the `match` on the ruff output format in a way that should be reusable by the formatter. I didn't think this was going to work at first, but the fact that the config holds options that apply only to certain output formats works in our favor here. We can set up a single config for all of the output formats and then use `try_from` to convert the `OutputFormat` to a `DiagnosticFormat` later. ## Test Plan Existing tests, plus a few new ones to make sure relocating the `SHOW_FIX_SUMMARY` rendering worked, that was untested before. I deleted a bunch of test code along with the `text` module, but I believe all of it is now well-covered by the `full` and `concise` tests in `ruff_db`. I also merged this branch into https://github.com/astral-sh/ruff/pull/20443 locally and made sure that the API actually helps. `render_diagnostics` dropped in perfectly and passed the tests there too. --- crates/ruff/src/commands/check.rs | 24 ++- crates/ruff/src/printer.rs | 118 ++++--------- crates/ruff/tests/lint.rs | 30 ++++ ...int__output_format_show_fixes_concise.snap | 26 +++ .../lint__output_format_show_fixes_full.snap | 26 +++ ...int__output_format_show_fixes_grouped.snap | 26 +++ crates/ruff_db/src/diagnostic/mod.rs | 12 +- crates/ruff_db/src/diagnostic/render.rs | 4 +- crates/ruff_linter/src/message/grouped.rs | 33 ++-- crates/ruff_linter/src/message/mod.rs | 156 +++++------------- ...linter__message__text__tests__default.snap | 30 ---- ...ter__message__text__tests__fix_status.snap | 30 ---- ...ssage__text__tests__fix_status_unsafe.snap | 30 ---- ...message__text__tests__notebook_output.snap | 33 ---- ...__message__text__tests__syntax_errors.snap | 23 --- crates/ruff_linter/src/message/text.rs | 143 ---------------- crates/ruff_linter/src/settings/types.rs | 29 ++++ crates/ruff_linter/src/test.rs | 60 ++++--- 18 files changed, 285 insertions(+), 548 deletions(-) create mode 100644 crates/ruff/tests/snapshots/lint__output_format_show_fixes_concise.snap create mode 100644 crates/ruff/tests/snapshots/lint__output_format_show_fixes_full.snap create mode 100644 crates/ruff/tests/snapshots/lint__output_format_show_fixes_grouped.snap delete mode 100644 crates/ruff_linter/src/message/snapshots/ruff_linter__message__text__tests__default.snap delete mode 100644 crates/ruff_linter/src/message/snapshots/ruff_linter__message__text__tests__fix_status.snap delete mode 100644 crates/ruff_linter/src/message/snapshots/ruff_linter__message__text__tests__fix_status_unsafe.snap delete mode 100644 crates/ruff_linter/src/message/snapshots/ruff_linter__message__text__tests__notebook_output.snap delete mode 100644 crates/ruff_linter/src/message/snapshots/ruff_linter__message__text__tests__syntax_errors.snap delete mode 100644 crates/ruff_linter/src/message/text.rs 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/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_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/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__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/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] From 1d3e4a91534448121c6db99c2baf83c1ccd2d909 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 29 Sep 2025 16:05:12 +0100 Subject: [PATCH 16/21] [ty] Remove unnecessary `parsed_module()` calls (#20630) --- crates/ty_python_semantic/src/types.rs | 4 ---- crates/ty_python_semantic/src/types/generics.rs | 13 +++++-------- .../ty_python_semantic/src/types/infer/builder.rs | 1 - 3 files changed, 5 insertions(+), 13 deletions(-) diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index d52b909115..3eac918bdd 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -5632,11 +5632,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, @@ -5709,7 +5707,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 { @@ -5747,7 +5744,6 @@ impl<'db> Type<'db> { ); Ok(bind_typevar( db, - &module, index, scope_id.file_scope_id(db), typevar_binding_context, diff --git a/crates/ty_python_semantic/src/types/generics.rs b/crates/ty_python_semantic/src/types/generics.rs index c781d4a69e..b73ad66728 100644 --- a/crates/ty_python_semantic/src/types/generics.rs +++ b/crates/ty_python_semantic/src/types/generics.rs @@ -1,7 +1,6 @@ use crate::types::constraints::ConstraintSet; use itertools::Itertools; -use ruff_db::parsed::ParsedModuleRef; use ruff_python_ast as ast; use rustc_hash::FxHashMap; @@ -26,7 +25,6 @@ use crate::{Db, FxOrderSet}; /// scope. fn enclosing_generic_contexts<'db>( db: &'db dyn Db, - module: &ParsedModuleRef, index: &SemanticIndex<'db>, scope: FileScopeId, ) -> impl Iterator> { @@ -34,13 +32,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") @@ -49,7 +47,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()? @@ -75,7 +73,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>, @@ -86,13 +83,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| { diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index f7eb128766..c7b1b5d26e 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -8978,7 +8978,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, From 00927943026af2a2a5071acc5d35879579a4d410 Mon Sep 17 00:00:00 2001 From: David Peter Date: Mon, 29 Sep 2025 21:08:08 +0200 Subject: [PATCH 17/21] [ty] Use `typing.Self` for the first parameter of instance methods (#20517) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Modify the (external) signature of instance methods such that the first parameter uses `Self` unless it is explicitly annotated. This allows us to correctly type-check more code, and allows us to infer correct return types for many functions that return `Self`. For example: ```py from pathlib import Path from datetime import datetime, timedelta reveal_type(Path(".config") / ".ty") # now Path, previously Unknown def _(dt: datetime, delta: timedelta): reveal_type(dt - delta) # now datetime, previously Unknown ``` part of https://github.com/astral-sh/ty/issues/159 ## Performance I ran benchmarks locally on `attrs`, `freqtrade` and `colour`, the projects with the largest regressions on CodSpeed. I see much smaller effects locally, but can definitely reproduce the regression on `attrs`. From looking at the profiling results (on Codspeed), it seems that we simply do more type inference work, which seems plausible, given that we now understand much more return types (of many stdlib functions). In particular, whenever a function uses an implicit `self` and returns `Self` (without mentioning `Self` anywhere else in its signature), we will now infer the correct type, whereas we would previously return `Unknown`. This also means that we need to invoke the generics solver in more cases. Comparing half a million lines of log output on attrs, I can see that we do 5% more "work" (number of lines in the log), and have a lot more `apply_specialization` events (7108 vs 4304). On freqtrade, I see similar numbers for `apply_specialization` (11360 vs 5138 calls). Given these results, I'm not sure if it's generally worth doing more performance work, especially since none of the code modifications themselves seem to be likely candidates for regressions. | Command | Mean [ms] | Min [ms] | Max [ms] | Relative | |:---|---:|---:|---:|---:| | `./ty_main check /home/shark/ecosystem/attrs` | 92.6 ± 3.6 | 85.9 | 102.6 | 1.00 | | `./ty_self check /home/shark/ecosystem/attrs` | 101.7 ± 3.5 | 96.9 | 113.8 | 1.10 ± 0.06 | | Command | Mean [ms] | Min [ms] | Max [ms] | Relative | |:---|---:|---:|---:|---:| | `./ty_main check /home/shark/ecosystem/freqtrade` | 599.0 ± 20.2 | 568.2 | 627.5 | 1.00 | | `./ty_self check /home/shark/ecosystem/freqtrade` | 607.9 ± 11.5 | 594.9 | 626.4 | 1.01 ± 0.04 | | Command | Mean [ms] | Min [ms] | Max [ms] | Relative | |:---|---:|---:|---:|---:| | `./ty_main check /home/shark/ecosystem/colour` | 423.9 ± 17.9 | 394.6 | 447.4 | 1.00 | | `./ty_self check /home/shark/ecosystem/colour` | 426.9 ± 24.9 | 373.8 | 456.6 | 1.01 ± 0.07 | ## Test Plan New Markdown tests ## Ecosystem report * apprise: ~300 new diagnostics related to problematic stubs in apprise :weary: * attrs: a new true positive, since [this function](https://github.com/python-attrs/attrs/blob/4e2c89c82398914dbbe24fe00b03f00a8b8efe05/tests/test_make.py#L2135) is missing a `@staticmethod`? * Some legitimate true positives * sympy: lots of new `invalid-operator` false positives in [matrix multiplication](https://github.com/sympy/sympy/blob/cf9f4b680520821b31e2baceb4245252939306be/sympy/matrices/matrixbase.py#L3267-L3269) due to our limited understanding of [generic `Callable[[Callable[[T1, T2], T3]], Callable[[T1, T2], T3]]` "identity" types](https://github.com/sympy/sympy/blob/cf9f4b680520821b31e2baceb4245252939306be/sympy/core/decorators.py#L83-L84) of decorators. This is not related to type-of-self. ## Typing conformance results The changes are all correct, except for ```diff +generics_self_usage.py:50:5: error[invalid-assignment] Object of type `def foo(self) -> int` is not assignable to `(typing.Self, /) -> int` ``` which is related to an assignability problem involving type variables on both sides: ```py class CallableAttribute: def foo(self) -> int: return 0 bar: Callable[[Self], int] = foo # <- we currently error on this assignment ``` --------- Co-authored-by: Shaygan Hooshyari --- crates/ruff_benchmark/benches/ty_walltime.rs | 2 +- crates/ruff_python_ast/src/nodes.rs | 6 + .../resources/mdtest/annotations/self.md | 190 +++++++++++++++++- .../resources/mdtest/call/methods.md | 4 +- .../resources/mdtest/descriptor_protocol.md | 2 + .../mdtest/generics/legacy/classes.md | 12 +- .../mdtest/generics/pep695/classes.md | 12 +- .../mdtest/generics/pep695/functions.md | 3 +- .../resources/mdtest/generics/scoping.md | 3 +- .../resources/mdtest/named_tuple.md | 6 +- .../resources/mdtest/narrow/isinstance.md | 12 +- .../resources/mdtest/pep695_type_aliases.md | 6 +- .../resources/mdtest/protocols.md | 4 +- .../mdtest/type_properties/is_subtype_of.md | 2 - .../resources/mdtest/typed_dict.md | 10 +- .../resources/mdtest/with/async.md | 3 +- crates/ty_python_semantic/src/types.rs | 47 +---- crates/ty_python_semantic/src/types/class.rs | 110 +++++++++- .../ty_python_semantic/src/types/display.rs | 12 +- .../ty_python_semantic/src/types/generics.rs | 64 +++++- .../src/types/infer/builder.rs | 2 +- .../src/types/signatures.rs | 170 +++++++++++++--- .../ty_extensions/ty_extensions.pyi | 4 +- 23 files changed, 555 insertions(+), 131 deletions(-) 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_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/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/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/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 a9224d46c8..4aef52a26e 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/pep695/functions.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/pep695/functions.md @@ -534,6 +534,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/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..6df8c1a2d3 100644 --- a/crates/ty_python_semantic/resources/mdtest/protocols.md +++ b/crates/ty_python_semantic/resources/mdtest/protocols.md @@ -1977,12 +1977,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/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/types.rs b/crates/ty_python_semantic/src/types.rs index 3eac918bdd..3764ebee53 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -52,7 +52,8 @@ use crate::types::function::{ DataclassTransformerParams, FunctionSpans, FunctionType, KnownFunction, }; use crate::types::generics::{ - GenericContext, PartialSpecialization, Specialization, bind_typevar, walk_generic_context, + GenericContext, PartialSpecialization, Specialization, bind_typevar, typing_self, + walk_generic_context, }; pub use crate::types::ide_support::{ CallSignatureDetails, Member, MemberWithDefinition, all_members, call_signature_details, @@ -5355,12 +5356,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), }, @@ -5717,39 +5716,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, - 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)), 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/display.rs b/crates/ty_python_semantic/src/types/display.rs index d3b18131b3..4bf9c235d2 100644 --- a/crates/ty_python_semantic/src/types/display.rs +++ b/crates/ty_python_semantic/src/types/display.rs @@ -1209,11 +1209,13 @@ 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.clone()) - )?; + 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() { diff --git a/crates/ty_python_semantic/src/types/generics.rs b/crates/ty_python_semantic/src/types/generics.rs index b73ad66728..fe82ac5c14 100644 --- a/crates/ty_python_semantic/src/types/generics.rs +++ b/crates/ty_python_semantic/src/types/generics.rs @@ -4,9 +4,9 @@ use itertools::Itertools; 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::infer::infer_definition_types; @@ -14,10 +14,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}; @@ -98,6 +98,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 @@ -286,14 +325,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) } diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index c7b1b5d26e..922aeb7d56 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -5950,7 +5950,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` diff --git a/crates/ty_python_semantic/src/types/signatures.rs b/crates/ty_python_semantic/src/types/signatures.rs index 3d3868b06c..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)] @@ -1185,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(); @@ -1218,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| { @@ -1399,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, } @@ -1407,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, @@ -1418,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, @@ -1429,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, } @@ -1437,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, @@ -1448,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, } @@ -1486,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, } } @@ -1501,6 +1617,7 @@ impl<'db> Parameter<'db> { ) -> Self { let Parameter { annotated_type, + inferred_annotation, kind, form, } = self; @@ -1544,6 +1661,7 @@ impl<'db> Parameter<'db> { Self { annotated_type: Some(annotated_type), + inferred_annotation: *inferred_annotation, kind, form: *form, } @@ -1566,6 +1684,7 @@ impl<'db> Parameter<'db> { }), kind, form: ParameterForm::Value, + inferred_annotation: false, } } @@ -1620,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_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: ... From 8664842d00d846ac7fa1ff8b7ecf05ef67cd75af Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 29 Sep 2025 21:19:13 +0100 Subject: [PATCH 18/21] [ty] Ensure first-party search paths always appear in a sensible order (#20629) This PR ensures that we always put `./src` before `.` in our list of first-party search paths. This better emulates the fact that at runtime, the module name of a file `src/foo.py` would almost certainly be `foo` rather than `src.foo`. I wondered if fixing this might fix https://github.com/astral-sh/ruff/pull/20603#issuecomment-3345317444. It seems like that's not the case, but it also seems like it leads to better diagnostics because we report much more intuitive module names to the user in our error messages -- so, it's probably a good change anyway. --- crates/ty/tests/cli/python_environment.rs | 58 ++++++++++++++++++----- crates/ty_project/src/metadata/options.rs | 14 ++++-- 2 files changed, 55 insertions(+), 17 deletions(-) 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_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 }; From 1c08f71a0087c710641227e3f9b929fd8d038d5a Mon Sep 17 00:00:00 2001 From: Dan Parizher <105245560+danparizher@users.noreply.github.com> Date: Tue, 30 Sep 2025 02:34:18 -0400 Subject: [PATCH 19/21] [`cli`] Add conflict between `--add-noqa` and `--diff` options (#20642) --- crates/ruff/src/args.rs | 1 + 1 file changed, 1 insertion(+) 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. From 130a794c2b894d8ffc96e49c2d48bdddd11c99a6 Mon Sep 17 00:00:00 2001 From: David Peter Date: Tue, 30 Sep 2025 08:44:18 +0200 Subject: [PATCH 20/21] [ty] Add tests for nested generic functions (#20631) ## Summary Add two simple tests that we recently discussed with @dcreager. They demonstrate that the `TypeMapping::MarkTypeVarsInferable` operation really does need to keep track of the binding context. ## Test Plan Made sure that those tests fail if we create `TypeMapping::MarkTypeVarsInferable(None)`s everywhere. --- .../resources/mdtest/generics/legacy/functions.md | 8 ++++++++ .../resources/mdtest/generics/pep695/functions.md | 7 +++++++ 2 files changed, 15 insertions(+) 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 3175ed7216..50a5be7058 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/functions.md b/crates/ty_python_semantic/resources/mdtest/generics/pep695/functions.md index 4aef52a26e..1193e91ba5 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 From b483d3b0b90aefed29a078630f7791a166a64159 Mon Sep 17 00:00:00 2001 From: David Peter Date: Tue, 30 Sep 2025 14:22:36 +0200 Subject: [PATCH 21/21] [ty] Literal promotion refactor (#20646) ## Summary Not sure if this was the original intention, but it looks to me like the previous `Type::literal_promotion_type` was more of an implementation detail for the actual operation of promoting all literals in a possibly-nested position of a type. This is not a pure refactor, as I'm technically changing the behavior for that protocols diagnostic message suggestion. ## Test Plan New Markdown test --- .../resources/mdtest/protocols.md | 4 ++- crates/ty_python_semantic/src/types.rs | 31 ++++++++++--------- .../src/types/diagnostic.rs | 10 ++++-- .../src/types/infer/builder.rs | 5 ++- 4 files changed, 29 insertions(+), 21 deletions(-) diff --git a/crates/ty_python_semantic/resources/mdtest/protocols.md b/crates/ty_python_semantic/resources/mdtest/protocols.md index 6df8c1a2d3..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): diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index 3764ebee53..a9111f3581 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -1166,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, } } @@ -6080,8 +6085,7 @@ impl<'db> Type<'db> { 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 } } @@ -6189,8 +6193,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 { diff --git a/crates/ty_python_semantic/src/types/diagnostic.rs b/crates/ty_python_semantic/src/types/diagnostic.rs index 29d84041f3..ea7d54841c 100644 --- a/crates/ty_python_semantic/src/types/diagnostic.rs +++ b/crates/ty_python_semantic/src/types/diagnostic.rs @@ -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/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index 922aeb7d56..204d14d570 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -92,7 +92,7 @@ use crate::types::{ DynamicType, IntersectionBuilder, IntersectionType, KnownClass, KnownInstanceType, MemberLookupPolicy, MetaclassCandidate, PEP695TypeAliasType, Parameter, ParameterForm, Parameters, SpecialFormType, SubclassOfType, TrackedConstraintSet, Truthiness, Type, - TypeAliasType, TypeAndQualifiers, TypeContext, TypeMapping, TypeQualifiers, + TypeAliasType, TypeAndQualifiers, TypeContext, TypeQualifiers, TypeVarBoundOrConstraintsEvaluation, TypeVarDefaultEvaluation, TypeVarInstance, TypeVarKind, UnionBuilder, UnionType, binding_type, todo_type, }; @@ -5432,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)