[ty] Support goto-definition on vendored typeshed stubs#21020
Conversation
Gankra
requested review from
AlexWaygood,
MichaReiser,
dcreager and
sharkdp
as code owners
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
| /// Get the cache-relative path where vendored paths should be written to. | ||
| pub fn relative_cached_vendored_root() -> SystemPathBuf { | ||
| // The vendored files are uniquely identified by the source commit. | ||
| SystemPathBuf::from(format!("vendored/typeshed/{}", ty_vendored::SOURCE_COMMIT)) | ||
| } |
There was a problem hiding this comment.
This is morally a const fn but I didn't want to deal with making that work and it's not a huge deal.
There was a problem hiding this comment.
haha, I wanted something similar recently too and did draw the very same conclusion: I don't want to deal with this right now :)
|
MichaReiser
left a comment
MichaReiser
left a comment
There was a problem hiding this comment.
I like that. I agree that it's hard to predict if there are cases where it will break, but we'll see. Thanks for giving this a try!
| /// Get the cache-relative path where vendored paths should be written to. | ||
| pub fn relative_cached_vendored_root() -> SystemPathBuf { | ||
| // The vendored files are uniquely identified by the source commit. | ||
| SystemPathBuf::from(format!("vendored/typeshed/{}", ty_vendored::SOURCE_COMMIT)) | ||
| } |
There was a problem hiding this comment.
haha, I wanted something similar recently too and did draw the very same conclusion: I don't want to deal with this right now :)
| /// Get the cache-relative path where vendored paths should be written to. | ||
| pub fn relative_cached_vendored_root() -> SystemPathBuf { | ||
| // The vendored files are uniquely identified by the source commit. | ||
| SystemPathBuf::from(format!("vendored/typeshed/{}", ty_vendored::SOURCE_COMMIT)) |
There was a problem hiding this comment.
| SystemPathBuf::from(format!("vendored/typeshed/{}", ty_vendored::SOURCE_COMMIT)) | |
| SystemPath::from(concat!("vendored/typeshed", ty_vendored::SOURCE_COMMIT)) |
There was a problem hiding this comment.
Unfortunately concat concatenates only literals so this is a compiler error :(
* main: (65 commits) [ty] Some more simplifications when rendering constraint sets (#21009) [ty] Make `attributes.md` mdtests faster (#21030) [ty] Set `INSTA_FORCE_PASS` and `INSTA_OUTPUT` environment variables from mdtest.py (#21029) [ty] Fall back to `Divergent` for deeply nested specializations (#20988) [`ruff`] Autogenerate TypeParam nodes (#21028) [ty] Add assertions to ensure that we never call `KnownClass::Tuple.to_instance()` or similar (#21027) [`ruff`] Auto generate ast Pattern nodes (#21024) [`flake8-simplify`] Skip `SIM911` when unknown arguments are present (#20697) Render a diagnostic for syntax errors introduced in formatter tests (#21021) [ty] Support goto-definition on vendored typeshed stubs (#21020) [ty] Implement go-to for binary and unary operators (#21001) [ty] Avoid ever-growing default types (#20991) [syntax-errors] Name is parameter and global (#20426) [ty] Disable panicking mdtest (#21016) [ty] Fix completions at end of file (#20993) [ty] Fix out-of-order semantic token for function with regular argument after kwargs (#21013) [ty] Fix auto import for files with `from __future__` import (#20987) [`fastapi`] Handle ellipsis defaults in FAST002 autofix (`FAST002`) (#20810) [`ruff`] Skip autofix for keyword and `__debug__` path params (#20960) [`flake8-bugbear`] Skip `B905` and `B912` if <2 iterables and no starred arguments (#20998) ...
2 participants
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 astral-sh/ty#1054