[ty] Add completion support for import and from ... import statements by BurntSushi · Pull Request #19883 · astral-sh/ruff (original) (raw)
Related to the LSP server
Multi-file analysis & type inference
labels
BurntSushi added a commit that referenced this pull request
This ensures there is some level of consistency between the APIs.
This did require exposing a couple more things on Module for good
error messages. This also motivated a switch to an interned struct
instead of a tracked struct. This ensures that list_modules and
resolve_modules reuse the same Module values when the inputs are the
same.
Ref #19883 (comment)
BurntSushi added a commit that referenced this pull request
This ensures there is some level of consistency between the APIs.
This did require exposing a couple more things on Module for good
error messages. This also motivated a switch to an interned struct
instead of a tracked struct. This ensures that list_modules and
resolve_modules reuse the same Module values when the inputs are the
same.
Ref #19883 (comment)
BurntSushi added a commit that referenced this pull request
This ensures there is some level of consistency between the APIs.
This did require exposing a couple more things on Module for good
error messages. This also motivated a switch to an interned struct
instead of a tracked struct. This ensures that list_modules and
resolve_modules reuse the same Module values when the inputs are the
same.
Ref #19883 (comment)
BurntSushi added a commit that referenced this pull request
This ensures there is some level of consistency between the APIs.
This did require exposing a couple more things on Module for good
error messages. This also motivated a switch to an interned struct
instead of a tracked struct. This ensures that list_modules and
resolve_modules reuse the same Module values when the inputs are the
same.
Ref #19883 (comment)
BurntSushi added a commit that referenced this pull request
This ensures there is some level of consistency between the APIs.
This did require exposing a couple more things on Module for good
error messages. This also motivated a switch to an interned struct
instead of a tracked struct. This ensures that list_modules and
resolve_modules reuse the same Module values when the inputs are the
same.
Ref #19883 (comment)
BurntSushi added a commit that referenced this pull request
This change rejiggers how we register globs for file watching with the
LSP client. Previously, we registered a few globs like **/*.py,
**/pyproject.toml and more. There were two problems with this
approach.
Firstly, it only watches files within the project root. Search paths may be outside the project root. Such as virtualenv directory.
Secondly, there is variation on how tools interact with virtual
environments. In the case of uv, depending on its link mode, we might
not get any file change notifications after running uv add foo or
uv remove foo.
To remedy this, we instead just list for file change notifications on
all files for all search paths. This simplifies the globs we use, but
does potentially increase the number of notifications we'll get.
However, given the somewhat simplistic interface supported by the LSP
protocol, I think this is unavoidable (unless we used our own file
watcher, which has its own considerably downsides). Moreover, this is
seemingly consistent with how ty check --watch works.
This also required moving file watcher registration to after workspaces are initialized, or else we don't know what the right search paths are.
This change is in service of #19883, which in order for cache invalidation to work right, the LSP client needs to send notifications whenever a dependency is added or removed. This change should make that possible.
I tried this patch with #19883 in addition to my work to activate Salsa caching, and everything seems to work as I'd expect. That is, completions no longer show stale results after a dependency is added or removed.
BurntSushi added a commit that referenced this pull request
This change rejiggers how we register globs for file watching with the
LSP client. Previously, we registered a few globs like **/*.py,
**/pyproject.toml and more. There were two problems with this
approach.
Firstly, it only watches files within the project root. Search paths may be outside the project root. Such as virtualenv directory.
Secondly, there is variation on how tools interact with virtual
environments. In the case of uv, depending on its link mode, we might
not get any file change notifications after running uv add foo or
uv remove foo.
To remedy this, we instead just list for file change notifications on
all files for all search paths. This simplifies the globs we use, but
does potentially increase the number of notifications we'll get.
However, given the somewhat simplistic interface supported by the LSP
protocol, I think this is unavoidable (unless we used our own file
watcher, which has its own considerably downsides). Moreover, this is
seemingly consistent with how ty check --watch works.
This also required moving file watcher registration to after workspaces are initialized, or else we don't know what the right search paths are.
This change is in service of #19883, which in order for cache invalidation to work right, the LSP client needs to send notifications whenever a dependency is added or removed. This change should make that possible.
I tried this patch with #19883 in addition to my work to activate Salsa caching, and everything seems to work as I'd expect. That is, completions no longer show stale results after a dependency is added or removed.
BurntSushi added a commit that referenced this pull request
This ensures there is some level of consistency between the APIs.
This did require exposing a couple more things on Module for good
error messages. This also motivated a switch to an interned struct
instead of a tracked struct. This ensures that list_modules and
resolve_modules reuse the same Module values when the inputs are the
same.
Ref #19883 (comment)
BurntSushi added a commit that referenced this pull request
This change rejiggers how we register globs for file watching with the
LSP client. Previously, we registered a few globs like **/*.py,
**/pyproject.toml and more. There were two problems with this
approach.
Firstly, it only watches files within the project root. Search paths may be outside the project root. Such as virtualenv directory.
Secondly, there is variation on how tools interact with virtual
environments. In the case of uv, depending on its link mode, we might
not get any file change notifications after running uv add foo or
uv remove foo.
To remedy this, we instead just list for file change notifications on
all files for all search paths. This simplifies the globs we use, but
does potentially increase the number of notifications we'll get.
However, given the somewhat simplistic interface supported by the LSP
protocol, I think this is unavoidable (unless we used our own file
watcher, which has its own considerably downsides). Moreover, this is
seemingly consistent with how ty check --watch works.
This also required moving file watcher registration to after workspaces are initialized, or else we don't know what the right search paths are.
This change is in service of #19883, which in order for cache invalidation to work right, the LSP client needs to send notifications whenever a dependency is added or removed. This change should make that possible.
I tried this patch with #19883 in addition to my work to activate Salsa caching, and everything seems to work as I'd expect. That is, completions no longer show stale results after a dependency is added or removed.
BurntSushi added a commit that referenced this pull request
This change rejiggers how we register globs for file watching with the
LSP client. Previously, we registered a few globs like **/*.py,
**/pyproject.toml and more. There were two problems with this
approach.
Firstly, it only watches files within the project root. Search paths may be outside the project root. Such as virtualenv directory.
Secondly, there is variation on how tools interact with virtual
environments. In the case of uv, depending on its link mode, we might
not get any file change notifications after running uv add foo or
uv remove foo.
To remedy this, we instead just list for file change notifications on
all files for all search paths. This simplifies the globs we use, but
does potentially increase the number of notifications we'll get.
However, given the somewhat simplistic interface supported by the LSP
protocol, I think this is unavoidable (unless we used our own file
watcher, which has its own considerably downsides). Moreover, this is
seemingly consistent with how ty check --watch works.
This also required moving file watcher registration to after workspaces are initialized, or else we don't know what the right search paths are.
This change is in service of #19883, which in order for cache invalidation to work right, the LSP client needs to send notifications whenever a dependency is added or removed. This change should make that possible.
I tried this patch with #19883 in addition to my work to activate Salsa caching, and everything seems to work as I'd expect. That is, completions no longer show stale results after a dependency is added or removed.
This makes it easier to do exhaustive case analysis
on a SearchPath depending on whether it is a vendored
or system path.
We'll want to use these when implementing the "list modules" API.
These tests capture existing behavior.
I added these when I stumbled upon what I thought was an
oddity: we prioritize foo.pyi over foo.py, but
prioritize foo/__init__.py over foo.pyi.
(I plan to investigate this more closely in follow-up work. Particularly, to look at other type checkers. It seems like we may want to change this to always prioritize stubs.)
This makes it a little more flexible to call. For example,
we might have a StmtImport and not a StmtImportFrom.
Previously, if the module was just foo-stubs, we'd skip over
stripping the -stubs suffix which would lead to us returning
None.
This function is now a little convoluted and could be simpler
if we did an intermediate allocation. But I kept the iterative
approach and added a special case to handle foo-stubs.
This will make it easier to emit this info into snapshots for testing.
The actual implementation wasn't too bad. It's not long but pretty fiddly. I copied over the tests from the existing module resolver and adapted them to work with this API. Then I added a number of my own tests as well.
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.
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.
This ensures there is some level of consistency between the APIs.
This did require exposing a couple more things on Module for good
error messages. This also motivated a switch to an interned struct
instead of a tracked struct. This ensures that list_modules and
resolve_modules reuse the same Module values when the inputs are the
same.
Ref #19883 (comment)
KotlinIsland pushed a commit to KotlinIsland/basedpython that referenced this pull request
This ensures there is some level of consistency between the APIs.
This did require exposing a couple more things on Module for good
error messages. This also motivated a switch to an interned struct
instead of a tracked struct. This ensures that list_modules and
resolve_modules reuse the same Module values when the inputs are the
same.
Ref astral-sh/ruff#19883 (comment)
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
[ Show hidden characters]({{ revealButtonHref }})