Open
Conversation
gpilikin
force-pushed
the
fix/unused_imports
branch
5 times, most recently
from
1888116 to
4235ce3
Compare
gpilikin
force-pushed
the
fix/unused_imports
branch
6 times, most recently
from
5d3aaba to
1dea0e6
Compare
gpilikin
force-pushed
the
fix/unused_imports
branch
from
939492a to
60281d3
Compare
gpilikin
force-pushed
the
fix/unused_imports
branch
3 times, most recently
from
51fea5e to
1c443cb
Compare
Co-authored-by: Carl Meyer <carl@astral.sh>
## Summary This PR adds support for the `__all__` module variable. Reference spec: https://typing.python.org/en/latest/spec/distributing.html#library-interface-public-and-private-symbols This PR adds a new `dunder_all_names` query that returns a set of `Name`s defined in the `__all__` variable of the given `File`. The query works by implementing the `StatementVisitor` and collects all the names by recognizing the supported idioms as mentioned in the spec. Any idiom that's not recognized are ignored. The current implementation is minimum to what's required for us to remove all the false positives that this is causing. Refer to the "Follow-ups" section below to see what we can do next. I'll a open separate issue to keep track of them. Closes: astral-sh/ty#106 Closes: astral-sh/ty#199 ### Follow-ups * Diagnostics: * Add warning diagnostics for unrecognized `__all__` idioms, `__all__` containing non-string element * Add an error diagnostic for elements that are present in `__all__` but not defined in the module. This could lead to runtime error * Maybe we should return `<type>` instead of `Unknown | <type>` for `module.__all__`. For example: https://playknot.ruff.rs/2a6fe5d7-4e16-45b1-8ec3-d79f2d4ca894 * Mark a symbol that's mentioned in `__all__` as used otherwise it could raise (possibly in the future) "unused-name" diagnostic Supporting diagnostics will require that we update the return type of the query to be something other than `Option<FxHashSet<Name>>`, something that behaves like a result and provides a way to check whether a name exists in `__all__`, loop over elements in `__all__`, loop over the invalid elements, etc. ## Ecosystem analysis The following are the maximum amount of diagnostics **removed** in the ecosystem: * "Type <module '...'> has no attribute ..." * `collections.abc` - 14 * `numpy` - 35534 * `numpy.ma` - 296 * `numpy.char` - 37 * `numpy.testing` - 175 * `hashlib` - 311 * `scipy.fft` - 2 * `scipy.stats` - 38 * "Module '...' has no member ..." * `collections.abc` - 85 * `numpy` - 508 * `numpy.testing` - 741 * `hashlib` - 36 * `scipy.stats` - 68 * `scipy.interpolate` - 7 * `scipy.signal` - 5 The following modules have dynamic `__all__` definition, so `ty` assumes that `__all__` doesn't exists in that module: * `scipy.stats` (https://github.com/scipy/scipy/blob/95a5d6ea8b9f2e482f9a79aa40b719433f8e3c4d/scipy/stats/__init__.py#L665) * `scipy.interpolate` (https://github.com/scipy/scipy/blob/95a5d6ea8b9f2e482f9a79aa40b719433f8e3c4d/scipy/interpolate/__init__.py#L221) * `scipy.signal` (indirectly via https://github.com/scipy/scipy/blob/95a5d6ea8b9f2e482f9a79aa40b719433f8e3c4d/scipy/signal/_signal_api.py#L30) * `numpy.testing` (https://github.com/numpy/numpy/blob/de784cd6ee53ffddf07e0b3f89bf22bda9fd92fb/numpy/testing/__init__.py#L16-L18) ~There's this one category of **false positives** that have been added:~ Fixed the false positives by also ignoring `__all__` from a module that uses unrecognized idioms. <details><summary>Details about the false postivie:</summary> <p> The `scipy.stats` module has dynamic `__all__` and it imports a bunch of symbols via star imports. Some of those modules have a mix of valid and invalid `__all__` idioms. For example, in https://github.com/scipy/scipy/blob/95a5d6ea8b9f2e482f9a79aa40b719433f8e3c4d/scipy/stats/distributions.py#L18-L24, 2 out of 4 `__all__` idioms are invalid but currently `ty` recognizes two of them and says that the module has a `__all__` with 5 values. This leads to around **2055** newly added false positives of the form: ``` Type <module 'scipy.stats'> has no attribute ... ``` I think the fix here is to completely ignore `__all__`, not only if there are invalid elements in it, but also if there are unrecognized idioms used in the module. </p> </details> ## Test Plan Add a bunch of test cases using the new `ty_extensions.dunder_all_names` function to extract a module's `__all__` names. Update various test cases to remove false positives around `*` imports and re-export convention. Add new test cases for named import behavior as `*` imports covers all of it already (thanks Alex!).
Co-authored-by: Micha Reiser <micha@reiser.io>
## Summary We now expect the client to send initialization options to opt-in to experimental (but LSP-standardized) features, like completion support. Specifically, the client should set `"experimental.completions.enable": true`. Closes astral-sh/ty#74.
…astral-sh#17922) Same as astral-sh#17910 but for disjointness
…ma in arguments. (astral-sh#17893) Fixes astral-sh#17867 ## Summary The CPython parser does not allow generator expressions which are the sole arguments in an argument list to have a trailing comma. With this change, we start flagging such instances. ## Test Plan Added new inline tests.
parent: astral-sh#15584 fix was introduced at: astral-sh#9587 reasoning: astral-sh#9572
…iases (astral-sh#17927) astral-sh#17897 added variance handling for legacy typevars — but they were only being considered when checking generic aliases of the same class: ```py class A: ... class B(A): ... class C[T]: ... static_assert(is_subtype_of(C[B], C[A])) ``` and not for generic subclasses: ```py class D[U](C[U]): ... static_assert(is_subtype_of(D[B], C[A])) ``` Now we check those too! Closes astral-sh/ty#101
… generator functions (astral-sh#17930)
## Summary Fixes: astral-sh/ty#159 This PR adds support for using `Self` in methods. When the type of an annotation is `TypingSelf` it is converted to a type var based on: https://typing.python.org/en/latest/spec/generics.html#self I just skipped Protocols because it had more problems and the tests was not useful. Also I need to create a follow up PR that implicitly assumes `self` argument has type `Self`. In order to infer the type in the `in_type_expression` method I needed to have scope id and semantic index available. I used the idea from [this PR](https://github.com/astral-sh/ruff/pull/17589/files) to pass additional context to this method. Also I think in all places that `in_type_expression` is called we need to have this context because `Self` can be there so I didn't split the method into one version with context and one without. ## Test Plan Added new tests from spec. --------- Co-authored-by: Micha Reiser <micha@reiser.io> Co-authored-by: Carl Meyer <carl@astral.sh>
## Summary * Update salsa to pull in salsa-rs/salsa#850. * Some refactoring of salsa event callbacks in various `Db`'s due to salsa-rs/salsa#849 closes astral-sh/ty#108 ## Test Plan Ran `cargo run --bin ty -- -vvv` on a test file to make sure that salsa Events are still logged.
Summary
--
I thought that emitting multiple diagnostics at once would be difficult
to port to a diagnostic construction model closer to ty's
`InferContext::report_lint`, so as a first step toward that, this PR
removes `Checker::report_diagnostics`.
In many cases I was able to do some related refactoring to avoid
allocating a `Vec<Diagnostic>` at all, often by adding a `Checker` field
to a `Visitor` or by passing a `Checker` instead of a `&mut
Vec<Diagnostic>`.
In other cases, I had to fall back on something like
```rust
for diagnostic in diagnostics {
checker.report_diagnostic(diagnostic);
}
```
which I guess is a bit worse than the `extend` call in
`report_diagnostics`, but hopefully it won't make too much of a
difference.
I'm still not quite sure what to do with the remaining loop cases. The
two main use cases for collecting a sequence of diagnostics before
emitting any of them are:
1. Applying a single `Fix` to a group of diagnostics
2. Avoiding an earlier diagnostic if something goes wrong later
I was hoping we could get away with just a `DiagnosticGuard` that
reported a `Diagnostic` on drop, but I guess we will still need a
`DiagnosticGuardBuilder` that can be collected in these cases and
produce a `DiagnosticGuard` once we know we actually want the
diagnostics.
Test Plan
--
Existing tests
## Summary Resolves [astral-sh#461](astral-sh/ty#461). ty was hardcoded to infer `BytesLiteral` types for integer indexing into `BytesLiteral`. It will now infer `IntLiteral` types instead. ## Test Plan Markdown tests.
) ## Summary This PR updates the language server to avoid panicking when there are multiple workspace folders passed during initialization. The server currently picks up the first workspace folder and provides a warning and a log message. ## Test Plan <img width="1724" alt="Screenshot 2025-05-17 at 11 43 09" src="https://github.com/user-attachments/assets/1a7ddbc3-198d-4191-a28f-9b69321e8f99" />
Co-authored-by: Alex Waygood <alex.waygood@gmail.com>
## Summary I think `division-by-zero` is a low-value diagnostic in general; most real division-by-zero errors (especially those that are less obvious to the human eye) will occur on values typed as `int`, in which case we don't issue the diagnostic anyway. Mypy and pyright do not emit this diagnostic. Currently the diagnostic is prone to false positives because a) we do not silence it in unreachable code, and b) we do not implement narrowing of literals from inequality checks. We will probably fix (a) regardless, but (b) is low priority apart from division-by-zero. I think we have many more important things to do and should not allow false positives on a low-value diagnostic to be a distraction. Not opposed to re-enabling this diagnostic in future when we can prioritize reducing its false positives. References astral-sh/ty#443 ## Test Plan Existing tests.
Co-authored-by: Micha Reiser <micha@reiser.io>
…stral-sh#18241) ## Summary Make sure that the following definitions all lead to the same outcome (bug originally noticed by @AlexWaygood) ```py from typing import ClassVar class Descriptor: def __get__(self, instance, owner) -> int: return 42 class C: a: ClassVar[Descriptor] b: Descriptor = Descriptor() c: ClassVar[Descriptor] = Descriptor() reveal_type(C().a) # revealed: int (previously: int | Descriptor) reveal_type(C().b) # revealed: int reveal_type(C().c) # revealed: int ``` ## Test Plan New Markdown tests
…hod with an empty body on a non-protocol subclass of a protocol class (astral-sh#18243)
…pe is used in a type expression and the module has a member which would be valid in a type expression (astral-sh#18244)
## Summary astral-sh/ty#111 This PR adds support for `frozen` dataclasses. It will emit a diagnostic with a similar message to mypy Note: This does not include emitting a diagnostic if `__setattr__` or `__delattr__` are defined on the object as per the [spec](https://docs.python.org/3/library/dataclasses.html#module-contents) ## Test Plan mdtest --------- Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com> Co-authored-by: Carl Meyer <carl@astral.sh>
Fix error when importing modules without submodules. Add attribute check for similar imports with submodule.
gpilikin
force-pushed
the
fix/unused_imports
branch
from
2f238e1 to
8e09a96
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixed incorrect detection of unused imports with submodules. They were always marked as used even though they are not.
For example:
The solution:
Test Plan
cargo test