Fix invalid URI for untitled references/definitions #1344
+220
−1
Conversation
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
Co-authored-by: DanielRosenwasser <[email protected]>
The issue was that untitled filenames (^/untitled/ts-nul-authority/...) were being treated as relative paths and resolved against the current working directory, corrupting their format. - Modified GetEncodedRootLength to recognize ^/ as a rooted path prefix - Added comprehensive tests for untitled path handling - Verified fix resolves the URI conversion round-trip issue Co-authored-by: DanielRosenwasser <[email protected]>
This test simulates the exact issue scenario: - VS Code sends untitled:Untitled-2 URI - Verifies the path is not corrupted by current directory resolution - Confirms round-trip URI conversion works correctly - Proves references/definitions will return correct untitled URIs Co-authored-by: DanielRosenwasser <[email protected]>
Copilot
AI
changed the title
uri for untitled references/definitions
Copilot finished work on behalf of
DanielRosenwasser
July 2, 2025 00:18
internal/tspath/path.go
Outdated
Comment on lines
167
to
171
| // Untitled paths (e.g., "^/untitled/ts-nul-authority/Untitled-1") | ||
| if ch0 == '^' && ln > 1 && path[1] == '/' { | ||
| return 2 // Untitled: "^/" | ||
| } | ||
|
|
There was a problem hiding this comment.
This is uncommon, so move it right before the URL checks.
There was a problem hiding this comment.
Moved the untitled path check to before the URL checks as requested. This improves performance by checking common cases (POSIX/DOS paths) first before the uncommon untitled paths. Fixed in 0fd6461.
Co-authored-by: DanielRosenwasser <[email protected]>
Copilot finished work on behalf of
DanielRosenwasser
July 2, 2025 00:50
|
There's a check for |
Co-authored-by: DanielRosenwasser <[email protected]>
Removed the redundant check as requested. Since |
Copilot finished work on behalf of
DanielRosenwasser
July 2, 2025 02:01
internal/ast/ast.go
Outdated
| @@ -10072,7 +10072,7 @@ type SourceFile struct { | |||
| } | |||
|
|
|||
| func (f *NodeFactory) NewSourceFile(opts SourceFileParseOptions, text string, statements *NodeList, endOfFileToken *TokenNode) *Node { | |||
| if (tspath.GetEncodedRootLength(opts.FileName) == 0 && !strings.HasPrefix(opts.FileName, "^/")) || opts.FileName != tspath.NormalizePath(opts.FileName) { | |||
| if (tspath.GetEncodedRootLength(opts.FileName) == 0 || opts.FileName != tspath.NormalizePath(opts.FileName) { | |||
There was a problem hiding this comment.
Suggested change
| if (tspath.GetEncodedRootLength(opts.FileName) == 0 || opts.FileName != tspath.NormalizePath(opts.FileName) { | |
| if tspath.GetEncodedRootLength(opts.FileName) == 0 || opts.FileName != tspath.NormalizePath(opts.FileName) { |
and then run tests
There was a problem hiding this comment.
Fixed the syntax error in the condition as suggested. Removed the extra parenthesis and fixed the missing closing parenthesis. Also fixed a related test type issue. Fixed in 1201772.
Co-authored-by: DanielRosenwasser <[email protected]>
|
This is not work to do, just noting my thoughts (do not take these as instructions). I applied this to Strada, and it failed: Yet, kinda in a good way: diff --git a/./tests/baselines/reference/tsserver/documentRegistry/works-when-reusing-orphan-script-info-with-different-scriptKind.js b/./tests/baselines/local/tsserver/documentRegistry/works-when-reusing-orphan-script-info-with-different-scriptKind.js
index 215648946d..1e7a396fd5 100644
--- a/./tests/baselines/reference/tsserver/documentRegistry/works-when-reusing-orphan-script-info-with-different-scriptKind.js
+++ b/./tests/baselines/local/tsserver/documentRegistry/works-when-reusing-orphan-script-info-with-different-scriptKind.js
@@ -35,12 +35,6 @@ Info seq [hh:mm:ss:mss] getConfigFileNameForFile:: File: ^/inmemory/model/6 Pro
Info seq [hh:mm:ss:mss] Creating InferredProject: /dev/null/inferredProject1*, currentDirectory: /users/user/projects/san
Info seq [hh:mm:ss:mss] Starting updateGraphWorker: Project: /dev/null/inferredProject1*
Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /home/src/tslibs/TS/Lib/lib.d.ts 500 undefined WatchType: Closed Script info
-Info seq [hh:mm:ss:mss] DirectoryWatcher:: Added:: WatchInfo: /users/user/projects/san/^ 1 undefined Project: /dev/null/inferredProject1* WatchType: Failed Lookup Locations
-Info seq [hh:mm:ss:mss] Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /users/user/projects/san/^ 1 undefined Project: /dev/null/inferredProject1* WatchType: Failed Lookup Locations
-Info seq [hh:mm:ss:mss] DirectoryWatcher:: Added:: WatchInfo: /users/user/projects/san/node_modules 1 undefined Project: /dev/null/inferredProject1* WatchType: Failed Lookup Locations
-Info seq [hh:mm:ss:mss] Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /users/user/projects/san/node_modules 1 undefined Project: /dev/null/inferredProject1* WatchType: Failed Lookup Locations
-Info seq [hh:mm:ss:mss] DirectoryWatcher:: Added:: WatchInfo: /users/user/projects/node_modules 1 undefined Project: /dev/null/inferredProject1* WatchType: Failed Lookup Locations
-Info seq [hh:mm:ss:mss] Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /users/user/projects/node_modules 1 undefined Project: /dev/null/inferredProject1* WatchType: Failed Lookup Locations
Info seq [hh:mm:ss:mss] DirectoryWatcher:: Added:: WatchInfo: /users/user/projects/san/node_modules/@types 1 undefined Project: /dev/null/inferredProject1* WatchType: Type roots
Info seq [hh:mm:ss:mss] Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /users/user/projects/san/node_modules/@types 1 undefined Project: /dev/null/inferredProject1* WatchType: Type roots
Info seq [hh:mm:ss:mss] DirectoryWatcher:: Added:: WatchInfo: /users/user/projects/node_modules/@types 1 undefined Project: /dev/null/inferredProject1* WatchType: Type roots
@@ -79,14 +73,8 @@ Info seq [hh:mm:ss:mss] response:
After request
PolledWatches::
-/users/user/projects/node_modules: *new*
- {"pollingInterval":500}
/users/user/projects/node_modules/@types: *new*
{"pollingInterval":500}
-/users/user/projects/san/^: *new*
- {"pollingInterval":500}
-/users/user/projects/san/node_modules: *new*
- {"pollingInterval":500}
/users/user/projects/san/node_modules/@types: *new*
{"pollingInterval":500}I think this fix is reasonably correct. |
jakebailey
approved these changes
Jul 7, 2025
andrewbranch
approved these changes
Jul 8, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses the issue where find references and go-to-definition operations on untitled files return incorrect URIs that cannot be linked back to the original location.
Root Cause Analysis
The issue was in the path handling logic in
internal/tspath/path.go. When VS Code sends requests with untitled URIs likeuntitled:Untitled-2, the following sequence occurs:DocumentURIToFileNamecorrectly convertsuntitled:Untitled-2to^/untitled/ts-nul-authority/Untitled-2GetEncodedRootLengthdid not recognize^/as a rooted path, returning 0ToPathtreated the untitled filename as relative and resolved it against current directory^/untitled/ts-nul-authority/Untitled-2into/home/user/project/^/untitled/ts-nul-authority/Untitled-2FileNameToDocumentURIwas called, it couldn't recognize the corrupted path as untitled and returned a regular file URISolution
Modified
GetEncodedRootLengthto recognize paths starting with^/as rooted paths (similar to how/andc:are handled). This prevents untitled filenames from being resolved against the current directory.Implementation
^/prefix inGetEncodedRootLengthfunction^/)Before Fix
After Fix
The integration test confirms the complete flow works correctly, simulating the exact scenario from the original issue where references and definitions on untitled files now return proper
untitled:URIs instead of brokenfile://URIs.Fixes #1343.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.