## Summary
This introduces a very bad and naive
python-docstring-flavoured-reStructuredText to github-flavor-markdown
translator. The main goal is to try to preserve a lot of the formatting
and plaintext, progressively enhance the content when we find things we
know about, and escape the text when we find things that might get
corrupt.
Previously I'd broken this out into rendering each different format, but
with this approach you don't really need to?
## Test Plan
Lots of snapshot tests, also messed around in some random stdlib
modules.
This commit essentially does away of all our old heuristic and piecemeal
code for detecting different kinds of import statements. Instead, we
offer one single state machine that does everything. This on its own
fixes a few bugs. For example, `import collections.abc, unico<CURSOR>`
would previously offer global scope completions instead of module
completions.
For the most part though, this commit is a refactoring that preserves
parity. In the next commit, we'll add support for completions on
relative imports.
This is a small refactor that helps centralize the
logic for how we gather, convert and possibly filter
completions.
Some of this logic was spread out before, which
motivated this refactor. Moreover, as part of other
refactoring, I found myself chaffing against the
lack of this abstraction.
Refs https://github.com/astral-sh/ty/issues/544
## Summary
Takes a more incremental approach to PEP 613 type alias support (vs
https://github.com/astral-sh/ruff/pull/20107). Instead of eagerly
inferring the RHS of a PEP 613 type alias as a type expression, infer it
as a value expression, just like we do for implicit type aliases, taking
advantage of the same support for e.g. unions and other type special
forms.
The main reason I'm following this path instead of the one in
https://github.com/astral-sh/ruff/pull/20107 is that we've realized that
people do sometimes use PEP 613 type aliases as values, not just as
types (because they are just a normal runtime assignment, unlike PEP 695
type aliases which create an opaque `TypeAliasType`).
This PR doesn't yet provide full support for recursive type aliases
(they don't panic, but they just fall back to `Unknown` at the recursion
point). This is future work.
## Test Plan
Added mdtests.
Many new ecosystem diagnostics, mostly because we
understand new types in lots of places.
Conformance suite changes are correct.
Performance regression is due to understanding lots of new
types; nothing we do in this PR is inherently expensive.
This is a very conservative minimal implementation of applying overloads
to resolve a callable-type-being-called down to a single function
signature on hover. If we ever encounter a situation where the answer
doesn't simplify down to a single function call, we bail out to preserve
prettier printing of non-raw-Signatures.
The resulting Signatures are still a bit bare, I'm going to try to
improve that in a followup to improve our Signature printing in general.
Fixes https://github.com/astral-sh/ty/issues/73
As far as I know this change is largely non-functional, largely because
of https://github.com/astral-sh/ty/issues/1601
It's possible some of these like `Type::KnownInstance` produce something
useful sometimes. `LiteralString` is a new introduction, although its
goto-type jumps to `str` which is a bit sad (considering that part of
the SpecialForm discourse for now).
Also wrt the generics testing followup: turns out the snapshot tests
were full of those already.
This PR generalizes the signature_help system's SignatureWriter which
could get the subspans of function parameters.
We now have TypeDetailsWriter which is threaded between type's display
implementations via a new `fmt_detailed` method that many of the Display
types now have.
With this information we can properly add goto-type targets to our inlay
hints. This also lays groundwork for any future "I want to render a type
but get spans" work.
Also a ton of lifetimes are introduced to avoid things getting conflated
with `'db`.
This PR is broken up into a series of commits:
* Generalizing `SignatureWriter` to `TypeDetailsWriter`, but not using
it anywhere else. This commit was confirmed to be a non-functional
change (no test results changed)
* Introducing `fmt_detailed` everywhere to thread through
`TypeDetailsWriter` and annotate various spans as "being" a given Type
-- this is also where I had to reckon with a ton of erroneous `&'db
self`. This commit was also confirmed to be a non-functional change.
* Finally, actually using the results for goto-type on inlay hints!
* Regenerating snapshots, fixups, etc.
<!--
Thank you for contributing to Ruff/ty! To help us out with reviewing,
please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
Partially addresses https://github.com/astral-sh/ty/issues/1562
Only suggest the keyword "as" in import statements when the user have
written `import foo a<CURSOR>` or `from foo import bar a<CURSOR>` as no
other suggestion makes sense here.
Re-uses the existing pattern for incomplete `import from` statements to
determine incomplete import alias statements and make the suggestions
more sane in those cases.
There was a potential suggestion from @BurntSushi in
https://github.com/astral-sh/ty/issues/1562#issue-3626853513 to move the
handling of import statements into one unified state machine but I acted
on the side of caution and fixed this with already established patterns,
pending a potential bigger re-write down the line.
## Test Plan
Added new tests and checked that it behaved reasonable in the
playground.
<!-- How was it tested? -->
There are a few places in Python where it is known that new names are
being introduced and thus we probably shouldn't offer completions. We
already handle this today for things like `class <CURSOR>` and `def
<CURSOR>`. But we didn't handle `as <CURSOR>`, which can appear in
`import`, `with`, `except` and `match` statements. Indeed, these are
exactly the 4 cases where the `as` keyword can occur. So we look for the
presence of `as` and suppress completions based on that.
While we're here, we also make the implementation a bit more robust with
respect to suppressing completions when the user hasn't typed anything.
Namely, previously, we'd still offer completions in a `class <CURSOR>`
context. But it looks like LSP clients (at least, VS Code) doesn't ask
for completions here, so we were "saved" incidentally. This PR detects
this case and suppresses completions there so we don't rely on LSP
client behavior to handle that case correctly.
Fixesastral-sh/ty#1287
It looks like VS Code does this forcefully. As in, I don't think we can
override it. It also seems like a plausibly good idea. But by us doing
it too, it makes our completion evaluation framework match real world
conditions. (To the extent that "VS Code" and "real world conditions"
are the same. Which... they aren't. But it's close, since VS Code is so
popular.)
This should round out the rest of the set. I think I had hesitated doing
this before because some of these don't make sense in every context. But
I think identifying the correct context for every keyword could be quite
difficult. And at the very least, I think offering these at least as a
choice---even if they aren't always correct---is better than not doing
it at all.
It's everyone's favourite language corner case!
Also having kicked the tires on it, I'm pretty happy to call this (in
conjunction with #21367):
Fixes https://github.com/astral-sh/ty/issues/494
There's cases where you can make noisy Literal hints appear, so we can
always iterate on it, but this handles like, 98% of the cases in the
wild, which is great.
---------
Co-authored-by: David Peter <sharkdp@users.noreply.github.com>
I'm not 100% sold on this implementation, but it's a strict improvement
and it adds a ton of snapshot tests for future iteration.
Part of https://github.com/astral-sh/ty/issues/494
<!--
Thank you for contributing to Ruff/ty! To help us out with reviewing,
please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
Resolves https://github.com/astral-sh/ty/issues/1494
## Test Plan
Add a test showing if we are in `from <name> <name> ` we provide the
keyword completion "import"
This elides the following inlay hints:
```py
foo([x=]x)
foo([x=]y.x)
foo([x=]x[0])
foo([x=]x(...))
# composes to complex situations
foo([x=]y.x(..)[0])
```
Fixes https://github.com/astral-sh/ty/issues/1514
## Summary
This PR adds support for understanding the legacy definition and PEP 695
definition for `ParamSpec`.
This is still very initial and doesn't really implement any of the
semantics.
Part of https://github.com/astral-sh/ty/issues/157
## Test Plan
Add mdtest cases.
## Ecosystem analysis
Most of the diagnostics in `starlette` are due to the fact that ty now
understands `ParamSpec` is not a `Todo` type, so the assignability check
fails. The code looks something like:
```py
class _MiddlewareFactory(Protocol[P]):
def __call__(self, app: ASGIApp, /, *args: P.args, **kwargs: P.kwargs) -> ASGIApp: ... # pragma: no cover
class Middleware:
def __init__(
self,
cls: _MiddlewareFactory[P],
*args: P.args,
**kwargs: P.kwargs,
) -> None:
self.cls = cls
self.args = args
self.kwargs = kwargs
# ty complains that `ServerErrorMiddleware` is not assignable to `_MiddlewareFactory[P]`
Middleware(ServerErrorMiddleware, handler=error_handler, debug=debug)
```
There are multiple diagnostics where there's an attribute access on the
`Wrapped` object of `functools` which Pyright also raises:
```py
from functools import wraps
def my_decorator(f):
@wraps(f)
def wrapper(*args, **kwds):
return f(*args, **kwds)
# Pyright: Cannot access attribute "__signature__" for class "_Wrapped[..., Unknown, ..., Unknown]"
Attribute "__signature__" is unknown [reportAttributeAccessIssue]
# ty: Object of type `_Wrapped[Unknown, Unknown, Unknown, Unknown]` has no attribute `__signature__` [unresolved-attribute]
wrapper.__signature__
return wrapper
```
There are additional diagnostics that is due to the assignability checks
failing because ty now infers the `ParamSpec` instead of using the
`Todo` type which would always succeed. This results in a few
`no-matching-overload` diagnostics because the assignability checks
fail.
There are a few diagnostics related to
https://github.com/astral-sh/ty/issues/491 where there's a variable
which is either a bound method or a variable that's annotated with
`Callable` that doesn't contain the instance as the first parameter.
Another set of (valid) diagnostics are where the code hasn't provided
all the type variables. ty is now raising diagnostics for these because
we include `ParamSpec` type variable in the signature. For example,
`staticmethod[Any]` which contains two type variables.
<!--
Thank you for contributing to Ruff/ty! To help us out with reviewing,
please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
Raised by @AlexWaygood.
We previously did not favour imported symbols, when we probably
should've
## Test Plan
Add test showing that we favour imported symbol even if it is
alphabetically after other symbols that are builtin.
<!--
Thank you for contributing to Ruff/ty! To help us out with reviewing,
please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
Resolves https://github.com/astral-sh/ty/issues/1464
We sort the completions before we add the unimported ones, meaning that
imported completions show up before unimported ones.
This is also spoken about in
https://github.com/astral-sh/ty/issues/1274, and this is probably a
duplicate of that.
@AlexWaygood mentions this
[here](https://github.com/astral-sh/ty/issues/1274#issuecomment-3345942698)
too.
## Test Plan
Add a test showing even if an unimported completion "should"
(alphabetically before) come first, we favor the imported one.
<!--
Thank you for contributing to Ruff/ty! To help us out with reviewing,
please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
@BurntSushi provided some feedback in #21146 so i address it here.
## Summary
We weren't correctly modeling it as a `staticmethod` in all cases,
leading us to incorrectly infer that the `cls` argument would be bound
if it was accessed on an instance (rather than the class object).
## Test Plan
Added mdtests that fail on `main`. The primer output also looks good!
## Summary
Infer a type of unannotated `self` parameters in decorated methods /
properties.
closes https://github.com/astral-sh/ty/issues/1448
## Test Plan
Existing tests, some new tests.
Note that this doesn't change the evaluation results unfortunately.
In particular, prior to this fix, the correct result was ranked above
the redundant result. Our MRR-based evaluation doesn't care about
anything below the rank of the correct answer, and so this change isn't
reflected in our evaluation.
Fixesastral-sh/ty#1445
The status quo grew organically and didn't do well when one wanted to
mix and match different settings to generate a snapshot.
This does a small refactor to use more of a builder to generate
snapshots.
## Summary
Infer a type of `Self` for unannotated `self` parameters in methods of
classes.
part of https://github.com/astral-sh/ty/issues/159
closes https://github.com/astral-sh/ty/issues/1081
## Conformance tests changes
```diff
+enums_member_values.py:85:9: error[invalid-assignment] Object of type `int` is not assignable to attribute `_value_` of type `str`
```
A true positive ✔️
```diff
-generics_self_advanced.py:35:9: error[type-assertion-failure] Argument does not have asserted type `Self@method2`
-generics_self_basic.py:14:9: error[type-assertion-failure] Argument does not have asserted type `Self@set_scale
```
Two false positives going away ✔️
```diff
+generics_syntax_infer_variance.py:82:9: error[invalid-assignment] Cannot assign to final attribute `x` on type `Self@__init__`
```
This looks like a true positive to me, even if it's not marked with `#
E` ✔️
```diff
+protocols_explicit.py:56:9: error[invalid-assignment] Object of type `tuple[int, int, str]` is not assignable to attribute `rgb` of type `tuple[int, int, int]`
```
True positive ✔️
```
+protocols_explicit.py:85:9: error[invalid-attribute-access] Cannot assign to ClassVar `cm1` from an instance of type `Self@__init__`
```
This looks like a true positive to me, even if it's not marked with `#
E`. But this is consistent with our understanding of `ClassVar`, I
think. ✔️
```py
+qualifiers_final_annotation.py:52:9: error[invalid-assignment] Cannot assign to final attribute `ID4` on type `Self@__init__`
+qualifiers_final_annotation.py:65:9: error[invalid-assignment] Cannot assign to final attribute `ID7` on type `Self@method1`
```
New true positives ✔️
```py
+qualifiers_final_annotation.py:52:9: error[invalid-assignment] Cannot assign to final attribute `ID4` on type `Self@__init__`
+qualifiers_final_annotation.py:57:13: error[invalid-assignment] Cannot assign to final attribute `ID6` on type `Self@__init__`
+qualifiers_final_annotation.py:59:13: error[invalid-assignment] Cannot assign to final attribute `ID6` on type `Self@__init__`
```
This is a new false positive, but that's a pre-existing issue on main
(if you annotate with `Self`):
https://play.ty.dev/3ee1c56d-7e13-43bb-811a-7a81e236e6ab❌ => reported
as https://github.com/astral-sh/ty/issues/1409
## Ecosystem
* There are 5931 new `unresolved-attribute` and 3292 new
`possibly-missing-attribute` attribute errors, way too many to look at
all of them. I randomly sampled 15 of these errors and found:
* 13 instances where there was simply no such attribute that we could
plausibly see. Sometimes [I didn't find it
anywhere](8644d886c6/openlibrary/plugins/openlibrary/tests/test_listapi.py (L33)).
Sometimes it was set externally on the object. Sometimes there was some
[`setattr` dynamicness going
on](a49f6b927d/setuptools/wheel.py (L88-L94)).
I would consider all of them to be true positives.
* 1 instance where [attribute was set on `obj` in
`__new__`](9e87b44fd4/sympy/tensor/array/array_comprehension.py (L45C1-L45C36)),
which we don't support yet
* 1 instance [where the attribute was defined via `__slots__`
](e250ec0fc8/lib/spack/spack/vendor/pyrsistent/_pdeque.py (L48C5-L48C14))
* I see 44 instances [of the false positive
above](https://github.com/astral-sh/ty/issues/1409) with `Final`
instance attributes being set in `__init__`. I don't think this should
block this PR.
## Test Plan
New Markdown tests.
---------
Co-authored-by: Shaygan Hooshyari <sh.hooshyari@gmail.com>
This is an alternative to #21012 that more narrowly handles this logic
in the stub-mapping machinery rather than pervasively allowing us to
identify cached files as typeshed stubs. Much of the logic is the same
(pulling the logic out of ty_server so it can be reused).
I don't have a good sense for if one approach is "better" or "worse" in
terms of like, semantics and Weird Bugs that this can cause. This one is
just "less spooky in its broad consequences" and "less muddying of
separation of concerns" and puts the extra logic on a much colder path.
I won't be surprised if one day the previous implementation needs to be
revisited for its more sweeping effects but for now this is good.
Fixes https://github.com/astral-sh/ty/issues/1054
We have to track whether a typevar appears in a position where it's
inferable or not. In a non-inferable position (in the body of the
generic class or function that binds it), assignability must hold for
every possible specialization of the typevar. In an inferable position,
it only needs to hold for _some_ specialization.
https://github.com/astral-sh/ruff/pull/20093 is working on using
constraint sets to model assignability of typevars, and the constraint
sets that we produce will be the same for inferable vs non-inferable
typevars; what changes is what we _compare_ that constraint set to. (For
a non-inferable typevar, the constraint set must equal the set of valid
specializations; for an inferable typevar, it must not be `never`.)
When I first added support for tracking inferable vs non-inferable
typevars, it seemed like it would be easiest to have separate `Type`
variants for each. The alternative (which lines up with the Δ set in
[POPL15](https://doi.org/10.1145/2676726.2676991)) would be to
explicitly plumb through a list of inferable typevars through our type
property methods. That seemed cumbersome.
In retrospect, that was the wrong decision. We've had to jump through
hoops to translate types between the inferable and non-inferable
variants, which has been quite brittle. Combined with the original point
above, that much of the assignability logic will become more identical
between inferable and non-inferable, there is less justification for the
two `Type` variants. And plumbing an extra `inferable` parameter through
all of these methods turns out to not be as bad as I anticipated.
---------
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
## Summary
This allows us to handle self-referential bounds/constraints/defaults
without panicking.
Handles more cases from https://github.com/astral-sh/ty/issues/256
This also changes the way we infer the types of legacy TypeVars. Rather
than understanding a constructor call to `typing[_extension].TypeVar`
inside of any (arbitrarily nested) expression, and having to use a
special `assigned_to` field of the semantic index to try to best-effort
figure out what name the typevar was assigned to, we instead understand
the creation of a legacy `TypeVar` only in the supported syntactic
position (RHS of a simple un-annotated assignment with one target). In
any other position, we just infer it as creating an opaque instance of
`typing.TypeVar`. (This behavior matches all other type checkers.)
So we now special-case TypeVar creation in `TypeInferenceBuilder`, as a
special case of an assignment definition, rather than deeper inside call
binding. This does mean we re-implement slightly more of
argument-parsing, but in practice this is minimal and easy to handle
correctly.
This is easier to implement if we also make the RHS of a simple (no
unpacking) one-target assignment statement no longer a standalone
expression. Which is fine to do, because simple one-target assignments
don't need to infer the RHS more than once. This is a bonus performance
(0-3% across various projects) and significant memory-usage win, since
most assignment statements are simple one-target assignment statements,
meaning we now create many fewer standalone-expression salsa
ingredients.
This change does mean that inference of manually-constructed
`TypeAliasType` instances can no longer find its Definition in
`assigned_to`, which regresses go-to-definition for these aliases. In a
future PR, `TypeAliasType` will receive the same treatment that
`TypeVar` did in this PR (moving its special-case inference into
`TypeInferenceBuilder` and supporting it only in the correct syntactic
position, and lazily inferring its value type to support recursion),
which will also fix the go-to-definition regression. (I decided a
temporary edge-case regression is better in this case than doubling the
size of this PR.)
This PR also tightens up and fixes various aspects of the validation of
`TypeVar` creation, as seen in the tests.
We still (for now) treat all typevars as instances of `typing.TypeVar`,
even if they were created using `typing_extensions.TypeVar`. This means
we'll wrongly error on e.g. `T.__default__` on Python 3.11, even if `T`
is a `typing_extensions.TypeVar` instance at runtime. We share this
wrong behavior with both mypy and pyrefly. It will be easier to fix
after we pull in https://github.com/python/typeshed/pull/14840.
There are some issues that showed up here with typevar identity and
`MarkTypeVarsInferable`; the fix here (using the new `original` field
and `is_identical_to` methods on `BoundTypeVarInstance` and
`TypeVarInstance`) is a bit kludgy, but it can go away when we eliminate
`MarkTypeVarsInferable`.
## Test Plan
Added and updated mdtests.
### Conformance suite impact
The impact here is all positive:
* We now correctly error on a legacy TypeVar with exactly one constraint
type given.
* We now correctly error on a legacy TypeVar with both an upper bound
and constraints specified.
### Ecosystem impact
Basically none; in the setuptools case we just issue slightly different
errors on an invalid TypeVar definition, due to the modified validation
code.
---------
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
## Summary
The `types` module currently re-exports a lot of functions and data
types from `types::ide_support`. One of these is called `Member`, a name
that is overloaded several times already. And I'd like to add one more
`Member` struct soon. Making the whole `ide_support` module public seems
cleaner to me, anyway.
## Test Plan
Pure refactoring.
## Summary
Bump the latest supported Python version of ty to 3.14 and updates some
references from 3.13 to 3.14.
This also fixes a bug with `dataclasses.field` on 3.14 (which adds a new
keyword-only parameter to that function, breaking our previously naive
matching on the parameter structure of that function).
## Test Plan
A `ty check` on a file with template strings (without any further
configuration) doesn't raise errors anymore.
We don't attempt to fix these yet. I think there are bigger fish to fry.
I came up with these based on this discussion:
https://github.com/astral-sh/ruff/pull/20439#discussion_r2357769518
Here's one example:
```
if ...:
from foo import MAGIC
else:
from bar import MAGIC
MAG<CURSOR>
```
Now in this example, completions will include `MAGIC` from the local
scope. That is, auto-import is involved with that completion. But at
present, auto-import will suggest importing `foo` and `bar` because we
haven't de-duplicated completions yet. Which is fine.
Here's another example:
```
if ...:
import foo as fubar
else:
import bar as fubar
MAG<CURSOR>
```
Now here, there is no `MAGIC` symbol in scope. So auto-import is in
play. Let's assume that the user selects `MAGIC` from `foo` in this
example. (`bar` also has `MAGIC`.)
Since we currently ignore the declaration site for symbols with
multiple possible bindings, the importer today doesn't know that
`fubar` _could_ contain `MAGIC`. But even if it did, what would we do
with that information? Should we do this?
```
if ...:
import foo as fubar
from foo import MAGIC
else:
import bar as fubar
MAGIC
```
Or could we reason that `bar` also has `MAGIC`?
```
if ...:
import foo as fubar
else:
import bar as fubar
fubar.MAGIC
```
But if we did that, we're making an assumption of user intent, since
they *selected* `foo.MAGIC` but not `bar.MAGIC`.
Anyway, I don't think we need to settle on an answer today, but I
wanted to capture some of these tricky cases in tests at the very
least.
This is somewhat inspired by a similar abstraction in
`ruff_linter`. The main idea is to create an importer once
for a module that you want to add imports to. And then call
`import` to generate an edit for each symbol you want to
add.
I haven't done any performance profiling here yet. I don't
know if it will be a bottleneck. In particular, I do expect
`Importer::import` (but not `Importer::new`) to get called
many times for a single completion request when auto-import
is enabled. Particularly in projects with a lot of unimported
symbols. Because I don't know the perf impact, I didn't do
any premature optimization here. But there are surely some
low hanging fruit if this does prove to be a problem.
New tests make up a big portion of the diff here. I tried to
think of a bunch of different cases, although I'm sure there
are more.
This rejiggers some stuff in the main completions entrypoint
in `ty_ide`. A more refined `Completion` type is defined
with more information. In particular, to support auto-import,
we now include a module name and an "edit" for inserting an
import.
This also rolls the old "detailed completion" into the new
completion type. Previously, we were relying on the completion
type for `ty_python_semantic`. But `ty_ide` is really the code
that owns completions.
Note that this code doesn't build as-is. The next commit will
add the importer used here in `add_unimported_completions`.
Based on how this API is currently implemented, this doesn't
really cost us anything. But it gives us access to more
information about where the symbol is defined.
I think this is a better home for it. This way, `ty_ide`
more clearly owns how the "kind" of a completion is computed.
In particular, it is computed differently for things where
we know its type versus unimported symbols.
## Summary
Adds support for generic PEP695 type aliases, e.g.,
```python
type A[T] = T
reveal_type(A[int]) # A[int]
```
Resolves https://github.com/astral-sh/ty/issues/677.
It's almost certainly bad juju to show literally every single possible
symbol when completions are requested but there is nothing typed yet.
Moreover, since there are so many symbols, it is likely beneficial to
try and winnow them down before sending them to the client.
This change tries to extract text that has been typed and then uses
that as a query to listing all available symbols.
Instead of waiting to land auto-import until it is "ready
for users," it'd be nicer to get incremental progress merged
to `main`. By making it an experimental opt-in, we avoid making
the default completion experience worse but permit developers
and motivated users to try it.
This re-works the `all_symbols` based added previously to work across
all modules available, and not just what is directly in the workspace.
Note that we always pass an empty string as a query, which makes the
results always empty. We'll fix this in a subsequent commit.
This is similar to a change made in the "list top-level modules"
implementation that had been masked by poor Salsa failure modes.
Basically, if we can't find a root here, it *must* be a bug. And if we
just silently skip over it, we risk voiding Salsa's purity contract,
leading to more difficult to debug panics.
This did cause one test to fail, but only because the test wasn't
properly setting up roots.
This introduces `GotoTarget::Call` that represents the kind of
ambiguous/overloaded click of a callable-being-called:
```py
x = mymodule.MyClass(1, 2)
^^^^^^^
```
This is equivalent to `GotoTarget::Expression` for the same span but
enriched
with information about the actual callable implementation.
That is, if you click on `MyClass` in `MyClass()` it is *both* a
reference to the class and to the initializer of the class. Therefore
it would be ideal for goto-* and docstrings to be some intelligent
merging of both the class and the initializer.
In particular the callable-implementation (initializer) is prioritized
over the callable-itself (class) so when showing docstrings we will
preferentially show the docs of the initializer if it exists, and then
fallback to the docs of the class.
For goto-definition/goto-declaration we will yield both the class and
the initializer, requiring you to pick which you want (this is perhaps
needlessly pedantic but...).
Fixes https://github.com/astral-sh/ty/issues/898
Fixes https://github.com/astral-sh/ty/issues/1010
I decided to split out the addition of these tests from other PRs so
that it's easier to follow changes to the LSP's function call handling.
I'm not particularly concerned with whether the results produced by
these tests are "good" or "bad" in this PR, I'm just establishing a
baseline.
## Summary
Our internal inlay hints structure (`ty_ide::InlayHint`) now more
closely resembles `lsp_types::InlayHint`.
This mainly allows us to convert to `lsp_types::InlayHint` with less
hassle, but it also allows us to manage the different parts of the inlay
hint better, which in the future will allow us to implement features
like goto on the type part of the type inlay hint.
It also really isn't important to store a specific `Type` instance in
the `InlayHintContent`. So we remove this and use `InlayHintLabel`
instead which just shows the representation of the type (along with
other information).
We see a similar structure used in rust-analyzer too.
In effect, we make the Salsa query aspect keyed only on whether we want
global symbols. We move everything else (hierarchical and querying) to
an aggregate step *after* the query.
This was a somewhat involved change since we want to return a flattened
list from visiting the source while also preserving enough information
to reform the symbols into a hierarchical structure that the LSP
expects. But I think overall the API has gotten simpler and we encode
more invariants into the type system. (For example, previously you got a
runtime assertion if you tried to provide a query string while enabling
hierarchical mode. But now that's prevented by construction.)
Basically, this splits the implementation into two pieces:
the first piece does the traversal and finds *all* symbols
across the workspace. The second piece does filtering based
on a user provided query string. Only the first piece is
cached by Salsa.
This brings warm "workspace symbols" requests down from
500-600ms to 100-200ms.
While this doesn't typically matter, when ty returns a very
large list of symbols, this can have an impact. Specifically,
when searching `async` in home-assistant, this gets times
closer to 500ms versus closer to 600ms before this change.
It looks like an overall ~50ms improvement (so around 10%),
but variance is all over the place and I didn't do any
statistical tests.
But this does make intuitive sense. Previously, we were
allocating intermediate strings, doing UTF-8 decoding and
consulting Unicode casing tables. Now we're just doing what
is likely a single DFA scan. In effect, we front load all
of the Unicode junk into regex compilation.
There is a small amount of subtlety to this matching routine,
and it could be implemented in a faster way. So let's right some
tests for what we have to ensure we don't break anything when
we optimize it.
## Summary
We use the `System` abstraction in ty to abstract away the host/system
on which ty runs.
This has a few benefits:
* Tests can run in full isolation using a memory system (that uses an
in-memory file system)
* The LSP has a custom implementation where `read_to_string` returns the
content as seen by the editor (e.g. unsaved changes) instead of always
returning the content as it is stored on disk
* We don't require any file system polyfills for wasm in the browser
However, it does require extra care that we don't accidentally use
`std::fs` or `std::env` (etc.) methods in ty's code base (which is very
easy).
This PR sets up Clippy and disallows the most common methods, instead
pointing users towards the corresponding `System` methods.
The setup is a bit awkward because clippy doesn't support inheriting
configurations. That means, a crate can only override the entire
workspace configuration or not at all.
The approach taken in this PR is:
* Configure the disallowed methods at the workspace level
* Allow `disallowed_methods` at the workspace level
* Enable the lint at the crate level using the warn attribute (in code)
The obvious downside is that it won't work if we ever want to disallow
other methods, but we can figure that out once we reach that point.
What about false positives: Just add an `allow` and move on with your
life :) This isn't something that we have to enforce strictly; the goal
is to catch accidental misuse.
## Test Plan
Clippy found a place where we incorrectly used `std::fs::read_to_string`
While implementing similar logic for initializers I noticed that this
code appeared to be walking the ancestors in the wrong direction, and so
if you have nested function calls it would always grab the outermost one
instead of the closest-ancestor.
The four copies of the test are because there's something really evil in
our caching that can't seem to be demonstrated in our cursor testing
framework, which I'm filing a followup for.
This is a fairly simple but effective way to add docstrings to like 95%
of completions from initial experimentation.
Fixes https://github.com/astral-sh/ty/issues/1036
Although ironically this approach *does not* work specifically for
`print` and I haven't looked into why.
This makes `import <CURSOR>` and `from <CURSOR>` completions work.
This also makes `import os.<CURSOR>` and `from os.<CURSOR>`
completions work. In this case, we are careful to only offer
submodule completions.
These tests were added as a regression check that a panic
didn't occur. So we were asserting a bit more than necessary.
In particular, these will soon return completions for modules,
which creates large snapshots that we don't need.
So modify these to just check there is sensible output that
doesn't panic.
Requires some iteration, but this includes the most tedious part --
threading a new concept of DisplaySettings through every type display
impl. Currently it only holds a boolean for multiline, but in the future
it could also take other things like "render to markdown" or "here's
your base indent if you make a newline".
For types which have exposed display functions I've left the old
signature as a compatibility polyfill to avoid having to audit
everywhere that prints types right off the bat (notably I originally
tried doing multiline functions unconditionally and a ton of things
churned that clearly weren't ready for multi-line (diagnostics).
The only real use of this API in this PR is to multiline render function
types in hovers, which is the highest impact (see snapshot changes).
Fixes https://github.com/astral-sh/ty/issues/1000
Aria Desires
Andrew Gallant
Carl Meyer
Alex Waygood
Luca Chiodini
RasmusNygren
Matthew Mckee
github-actions[bot]
Micha Reiser
Jack O'Connor
Dhruv Manilawala
David Peter
decorator-factory
Shunsuke Shibayama
Douglas Creager
Takayuki Maeda
Ibraheem Ahmed