Skip to content

[ty] Fix file root matching for /#20684

Merged
BurntSushi merged 2 commits intomainfrom
ag/fix-1277
Oct 3, 2025
Merged

[ty] Fix file root matching for /#20684
BurntSushi merged 2 commits intomainfrom
ag/fix-1277

Conversation

@BurntSushi
Copy link
Member

@BurntSushi BurntSushi commented Oct 2, 2025

Previously, we would always add /{*filepath} as our wildcard to match
descendant paths. But when the root is just / (as it can be in tests,
weird environments or in the ty playground), this causes a double /
and inhibits most descendant matches.

The regression test added in this commit fails without this fix.
Specifically, it panics because it can't find a file root for
/project.

This also enables the log feature on tracing, which means we get
better debug messages in the JavaScript console when running the
playground.

Fixes astral-sh/ty#1277

@BurntSushi BurntSushi added bug Something isn't working server Related to the LSP server labels Oct 2, 2025
@BurntSushi BurntSushi changed the title ag/fix 1277 [ty] Fix file root matching for / Oct 2, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Oct 2, 2025

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

@github-actions
Copy link
Contributor

github-actions bot commented Oct 2, 2025

mypy_primer results

No ecosystem changes detected ✅
No memory usage changes detected ✅

@AlexWaygood AlexWaygood added the ty Multi-file analysis & type inference label Oct 2, 2025
@github-actions

This comment was marked as off-topic.

Cargo.toml Outdated
tikv-jemallocator = { version = "0.6.0" }
toml = { version = "0.9.0" }
tracing = { version = "0.1.40" }
tracing = { version = "0.1.40", features = ["log"] }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we only enable this in ty_wasm?

It makes it a bit easier to reason about the change

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense to enable this "globally," but I get wanting to do it narrowly. I've switched the PR to enabling log only for ty_wasm.

This has the effect of emitting tracing events via `log`
whenever there isn't an active tracing subscriber present.

This makes it so `ty_wasm` logs tracing messages to the
JavaScript console automatically (via our use of `console_log`).
Previously, we would always add `/{*filepath}` as our wildcard to match
descendant paths. But when the root is just `/` (as it can be in tests,
weird environments or in the ty playground), this causes a double `/`
and inhibits most descendant matches.

The regression test added in this commit fails without this fix.
Specifically, it panics because it can't find a file root for
`/project`.

Fixes #1277
@BurntSushi BurntSushi merged commit 92eee81 into main Oct 3, 2025
39 checks passed
@BurntSushi BurntSushi deleted the ag/fix-1277 branch October 3, 2025 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working server Related to the LSP server ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Server panic on a src/package/__init__.py file containing just fr

4 participants