[ty] Add search paths info to unresolved import diagnostics#20040
[ty] Add search paths info to unresolved import diagnostics#20040BurntSushi merged 15 commits intoastral-sh:mainfrom
Conversation
Renkai
requested review from
AlexWaygood,
carljm,
dcreager and
sharkdp
as code owners
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
AlexWaygood
changed the title
|
Renkai
changed the title
|
This solution called |
|
Thanks for working on this! @AlexWaygood can you take the lead on review here? |
|
Sure |
AlexWaygood
left a comment
AlexWaygood
left a comment
There was a problem hiding this comment.
Thanks, this looks great! I pushed a few minor tweaks to the PR.
It would be great to add a few more integration tests that have extra-paths set and/or an editable install.
| SearchPathInner::Extra(_) => { | ||
| "extra search path specified on the CLI or in your config file" | ||
| } | ||
| SearchPathInner::FirstParty(_) => "first-party code", | ||
| SearchPathInner::StandardLibraryCustom(_) => { | ||
| "custom stdlib stubs specified on the CLI or in your config file" | ||
| } |
There was a problem hiding this comment.
Ideally I feel like we'd be able to tell the user whether it was a CLI option or a config-file setting that caused us to look at an extra search path (or a custom stdlib search paths) during module resolution. But that would involve a fair bit of refactoring probably; I think it's best to leave it out of this PR.
BurntSushi
left a comment
BurntSushi
left a comment
There was a problem hiding this comment.
LGTM with one minor nit and passing CI.
| // Add search paths information to the diagnostic | ||
| // Use the same search paths function that is used in actual module resolution | ||
| let search_paths: Vec<_> = | ||
| search_paths(self.db(), ModuleResolveMode::StubsAllowed).collect(); |
There was a problem hiding this comment.
Why collect all the search paths into memory first? It seems a little gratuitous. You could do let search_paths = search_paths(..).peekable(); and then if search_paths.peek().is_some() { and for (index, path) in search_paths.enumerate() { below.
There was a problem hiding this comment.
updated by peekable. It seems requires more review to trigger next CI test. But the test looks fine on my computer now.
|
Thank you!!! |
* main: [`ruff`] Preserve relative whitespace in multi-line expressions (`RUF033`) (astral-sh#19647) [ty] Optimize TDD atom ordering (astral-sh#20098) [`airflow`] Extend `AIR311` and `AIR312` rules (astral-sh#20082) [ty] Preserve qualifiers when accessing attributes on unions/intersections (astral-sh#20114) [ty] Fix the inferred interface of specialized generic protocols (astral-sh#19866) [ty] Infer slightly more precise types for comprehensions (astral-sh#20111) [ty] Add more tests for protocols (astral-sh#20095) [ty] don't eagerly unpack aliases in user-authored unions (astral-sh#20055) [`flake8-use-pathlib`] Update links to the table showing the correspondence between `os` and `pathlib` (astral-sh#20103) [`flake8-use-pathlib`] Make `PTH100` fix unsafe because it can change behavior (astral-sh#20100) [`flake8-use-pathlib`] Delete unused `Rule::OsSymlink` enabled check (astral-sh#20099) [ty] Add search paths info to unresolved import diagnostics (astral-sh#20040) [`flake8-logging-format`] Add auto-fix for f-string logging calls (`G004`) (astral-sh#19303) Add a `ScopeKind` for the `__class__` cell (astral-sh#20048) Fix incorrect D413 links in docstrings convention FAQ (astral-sh#20089) [ty] Refactor inlay hints structure to use separate parts (astral-sh#20052)
…h#20040) Fixes astral-sh/ty#457 --------- Co-authored-by: Alex Waygood <alex.waygood@gmail.com>
4 participants
Fixes astral-sh/ty#457