This makes it so, e.g., `os<CURSOR>` will suggest the top-level stdlib
`os` module even if there is an `os` symbol elsewhere in your project.
The way this is done is somewhat overwrought, but it's done to avoid
suggesting top-level modules over other symbols already in scope.
Fixesastral-sh/issues#1852
Part of this was already done, but it was half-assed. We now look at the
search path that a symbol came from and centralize a symbol's origin
classification.
The preference ordering here is maybe not the right one, but we can
iterate as users give us feedback. Note also that the preference
ordering based on the origin is pretty low in the relevance sorting.
This means that other more specific criteria will and can override this.
This results in some nice improvements to our evaluation tasks.
This commit adds two new tests. One checks that a symbol in the current
project gets priority over a symbol in the standard library. Another
checks that a symbol in a third party dependency gets priority over a
symbol in the standard library. We don't get either of these right
today.
Note that these comparisons are done ceteris paribus. A symbol from the
standard library could still be ranked above a symbol elsewhere.
(Although I believe currently this is somewhat rare.)
I apparently don't know how to use my own API. Previously,
we would skip, e.g., `.venv`, but still descend into it.
This was annoying in practice because I sometimes have an
environment in one of the truth task directories. The eval
command should ignore that entirely, but it ended up
choking on it without properly ignoring hidden files
and directories.
I went through https://github.com/astral-sh/ty/issues/1274 and tried to
extract what I could into eval tasks. Some of the suggestions from that
issue have already been done, but most haven't.
This captures the status quo.
There are cases where the python grammar enforces expressions
after certain statements. In such cases we want to suppress
irrelevant keywords from the auto-complete suggestions.
E.g. `with a<CURSOR>`, suggesting `raise` here never makes sense
because it is not valid by the grammar.
These are all improvements here with one slight regression on
`reveal_type` ranking. The previous completions offered were:
```
$ cargo r -q -p ty_completion_eval show-one ty-extensions-lower-stdlib
ENOTRECOVERABLE (module: errno)
REG_WHOLE_HIVE_VOLATILE (module: winreg)
SQLITE_NOTICE_RECOVER_WAL (module: _sqlite3)
SupportsGetItemViewable (module: _typeshed)
removeHandler (module: unittest.signals)
reveal_mro (module: ty_extensions)
reveal_protocol_interface (module: ty_extensions)
reveal_type (module: typing) (*, 8/10)
_remove_original_values (module: _osx_support)
_remove_universal_flags (module: _osx_support)
-----
found 10 completions
```
And now they are:
```
$ cargo r -q -p ty_completion_eval show-one ty-extensions-lower-stdlib
ENOTRECOVERABLE (module: errno)
REG_WHOLE_HIVE_VOLATILE (module: winreg)
SQLITE_NOTICE_RECOVER_WAL (module: sqlite3)
SQLITE_NOTICE_RECOVER_WAL (module: sqlite3.dbapi2)
removeHandler (module: unittest)
removeHandler (module: unittest.signals)
reveal_mro (module: ty_extensions)
reveal_protocol_interface (module: ty_extensions)
reveal_type (module: typing) (*, 9/9)
-----
found 9 completions
```
Some completions were removed (because they are now considered
unexported) and some were added (likely do to better re-export support).
This particular case probably warrants more special attention anyway.
So I think this is fine. (It's only a one-ranking regression.)
This works by adding a third module resolution mode that lets the caller
opt into _some_ shadowing of modules that is otherwise not allowed (for
`typing` and `typing_extensions`).
Fixesastral-sh/ty#1658
This only applies to items that have a type associated with them. That
is, things that are already in scope. For items that don't have a type
associated with them (i.e., suggestions from auto-import), we still
suggest them since we can't know if they're appropriate or not. It's not
quite clear on how best to improve here for the auto-import case. (Short
of, say, asking for the type of each such symbol. But the performance
implications of that aren't known yet.)
Note that because of auto-import, we were still suggesting
`NotImplemented` even though astral-sh/ty#1262 specifically cites it as
the motivating example that we *shouldn't* suggest. This was occuring
because auto-import was including symbols from the `builtins` module,
even though those are actually already in scope. So this PR also gets
rid of those suggestions from auto-import.
Overall, this means that, at least, `raise NotImpl` won't suggest
`NotImplemented`.
Fixesastral-sh/ty#1262
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.
<!--
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.
This is still early days, but I hope the framework introduced here makes
it very easy to add new truth data. Truth data should be seen as a form
of regression test for non-ideal ranking of completion suggestions.
I think it would help to read `crates/ty_completion_eval/README.md`
first to get an idea of what you're reviewing.
