Skip to content

Fix invalid URI for untitled references/definitions #1344

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/ast/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -10072,9 +10072,9 @@
}

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) {

Check failure on line 10075 in internal/ast/ast.go

View workflow job for this annotation

GitHub Actions / lint (ubuntu-latest)

syntax error: unexpected {, expected )

Check failure on line 10075 in internal/ast/ast.go

View workflow job for this annotation

GitHub Actions / lint (ubuntu-latest)

syntax error: unexpected {, expected )

Check failure on line 10075 in internal/ast/ast.go

View workflow job for this annotation

GitHub Actions / lint (ubuntu-latest)

syntax error: unexpected {, expected )

Check failure on line 10075 in internal/ast/ast.go

View workflow job for this annotation

GitHub Actions / baselines

syntax error: unexpected {, expected )

Check failure on line 10075 in internal/ast/ast.go

View workflow job for this annotation

GitHub Actions / copilot

syntax error: unexpected {, expected )

Check failure on line 10075 in internal/ast/ast.go

View workflow job for this annotation

GitHub Actions / copilot

syntax error: unexpected {, expected )

Check failure on line 10075 in internal/ast/ast.go

View workflow job for this annotation

GitHub Actions / copilot

syntax error: unexpected {, expected )

Check failure on line 10075 in internal/ast/ast.go

View workflow job for this annotation

GitHub Actions / copilot

syntax error: unexpected {, expected )
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

panic(fmt.Sprintf("fileName should be normalized and absolute: %q", opts.FileName))

Check failure on line 10076 in internal/ast/ast.go

View workflow job for this annotation

GitHub Actions / lint (ubuntu-latest)

syntax error: unexpected name panic, expected {

Check failure on line 10076 in internal/ast/ast.go

View workflow job for this annotation

GitHub Actions / lint (ubuntu-latest)

syntax error: unexpected name panic, expected {

Check failure on line 10076 in internal/ast/ast.go

View workflow job for this annotation

GitHub Actions / baselines

syntax error: unexpected name panic, expected {

Check failure on line 10076 in internal/ast/ast.go

View workflow job for this annotation

GitHub Actions / copilot

syntax error: unexpected name panic, expected {

Check failure on line 10076 in internal/ast/ast.go

View workflow job for this annotation

GitHub Actions / copilot

syntax error: unexpected name panic, expected {

Check failure on line 10076 in internal/ast/ast.go

View workflow job for this annotation

GitHub Actions / copilot

syntax error: unexpected name panic, expected {
}

Check failure on line 10077 in internal/ast/ast.go

View workflow job for this annotation

GitHub Actions / lint (ubuntu-latest)

syntax error: unexpected }, expected expression) (typecheck)

Check failure on line 10077 in internal/ast/ast.go

View workflow job for this annotation

GitHub Actions / lint (ubuntu-latest)

syntax error: unexpected }, expected expression) (typecheck)

Check failure on line 10077 in internal/ast/ast.go

View workflow job for this annotation

GitHub Actions / baselines

syntax error: unexpected }, expected expression

Check failure on line 10077 in internal/ast/ast.go

View workflow job for this annotation

GitHub Actions / copilot

syntax error: unexpected }, expected expression) (typecheck)

Check failure on line 10077 in internal/ast/ast.go

View workflow job for this annotation

GitHub Actions / copilot

syntax error: unexpected }, expected expression) (typecheck)

Check failure on line 10077 in internal/ast/ast.go

View workflow job for this annotation

GitHub Actions / copilot

syntax error: unexpected }, expected expression
data := &SourceFile{}
data.fileName = opts.FileName
data.parseOptions = opts
Expand Down
156 changes: 156 additions & 0 deletions internal/ls/untitled_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
package ls_test

import (
"strings"
"testing"

"github.com/microsoft/typescript-go/internal/bundled"
"github.com/microsoft/typescript-go/internal/ls"
"github.com/microsoft/typescript-go/internal/lsp/lsproto"
"github.com/microsoft/typescript-go/internal/testutil/projecttestutil"
"github.com/microsoft/typescript-go/internal/tspath"
"gotest.tools/v3/assert"
)

func TestUntitledReferences(t *testing.T) {

Check failure on line 15 in internal/ls/untitled_test.go

View workflow job for this annotation

GitHub Actions / copilot

Function TestUntitledReferences missing the call to method parallel (paralleltest)
if !bundled.Embedded {
t.Skip("bundled files are not embedded")
}

// First test the URI conversion functions to understand the issue
untitledURI := lsproto.DocumentUri("untitled:Untitled-2")
convertedFileName := ls.DocumentURIToFileName(untitledURI)
t.Logf("URI '%s' converts to filename '%s'", untitledURI, convertedFileName)

backToURI := ls.FileNameToDocumentURI(convertedFileName)
t.Logf("Filename '%s' converts back to URI '%s'", convertedFileName, backToURI)

if string(backToURI) != string(untitledURI) {
t.Errorf("Round-trip conversion failed: '%s' -> '%s' -> '%s'", untitledURI, convertedFileName, backToURI)
}

// Create a test case that simulates how untitled files should work
testContent := `let x = 42;

x

x++;`

// Use the converted filename that DocumentURIToFileName would produce
untitledFileName := convertedFileName // "^/untitled/ts-nul-authority/Untitled-2"
t.Logf("Would use untitled filename: %s", untitledFileName)

// Set up the file system with an untitled file -
// But use a regular file first to see the current behavior
files := map[string]any{
"/Untitled-2.ts": testContent,
}

ctx := projecttestutil.WithRequestID(t.Context())
service, done := createLanguageService(ctx, "/Untitled-2.ts", files)

Check failure on line 50 in internal/ls/untitled_test.go

View workflow job for this annotation

GitHub Actions / copilot

cannot use files (variable of type map[string]any) as map[string]string value in argument to createLanguageService
defer done()

// Test the filename that the source file reports
program := service.GetProgram()
sourceFile := program.GetSourceFile("/Untitled-2.ts")
t.Logf("SourceFile.FileName() returns: '%s'", sourceFile.FileName())

// Calculate position of 'x' on line 3 (zero-indexed line 2, character 0)
position := 13 // After "let x = 42;\n\n"

// Call ProvideReferences using the test method
refs := service.TestProvideReferences("/Untitled-2.ts", position)

// Log the results
t.Logf("Input file name: %s", "/Untitled-2.ts")
t.Logf("Number of references found: %d", len(refs))
for i, ref := range refs {
t.Logf("Reference %d: URI=%s, Range=%+v", i+1, ref.Uri, ref.Range)
}

// We expect to find 3 references
assert.Assert(t, len(refs) == 3, "Expected 3 references, got %d", len(refs))

// Also test definition using ProvideDefinition
uri := ls.FileNameToDocumentURI("/Untitled-2.ts")
lspPosition := lsproto.Position{Line: 2, Character: 0}
definition, err := service.ProvideDefinition(t.Context(), uri, lspPosition)
assert.NilError(t, err)
if definition != nil && definition.Locations != nil {
t.Logf("Definition found: %d locations", len(*definition.Locations))
for i, loc := range *definition.Locations {
t.Logf("Definition %d: URI=%s, Range=%+v", i+1, loc.Uri, loc.Range)
}
}
}

func TestUntitledFileNameDebugging(t *testing.T) {

Check failure on line 87 in internal/ls/untitled_test.go

View workflow job for this annotation

GitHub Actions / copilot

Function TestUntitledFileNameDebugging missing the call to method parallel (paralleltest)
if !bundled.Embedded {
t.Skip("bundled files are not embedded")
}

// Test the URI conversion flow
untitledURI := lsproto.DocumentUri("untitled:Untitled-2")
convertedFileName := ls.DocumentURIToFileName(untitledURI)
t.Logf("1. URI '%s' converts to filename '%s'", untitledURI, convertedFileName)

// Test the path handling
currentDir := "/home/daniel/TypeScript"
path := tspath.ToPath(convertedFileName, currentDir, true)
t.Logf("2. ToPath('%s', '%s') returns: '%s'", convertedFileName, currentDir, string(path))

// Verify the path is NOT resolved against current directory
if strings.HasPrefix(string(path), currentDir) {
t.Errorf("Path was incorrectly resolved against current directory: %s", string(path))
}

// Test converting back to URI
backToURI := ls.FileNameToDocumentURI(string(path))
t.Logf("3. Path '%s' converts back to URI '%s'", string(path), backToURI)

if string(backToURI) != string(untitledURI) {
t.Errorf("Round-trip conversion failed: '%s' -> '%s' -> '%s'", untitledURI, string(path), backToURI)
}

t.Logf("✅ Fix working: untitled paths are not resolved against current directory")
}

func TestUntitledFileIntegration(t *testing.T) {

Check failure on line 118 in internal/ls/untitled_test.go

View workflow job for this annotation

GitHub Actions / copilot

Function TestUntitledFileIntegration missing the call to method parallel (paralleltest)
if !bundled.Embedded {
t.Skip("bundled files are not embedded")
}

// This test simulates the exact scenario from the issue:
// 1. VS Code sends untitled:Untitled-2 URI
// 2. References/definitions should return untitled:Untitled-2 URIs, not file:// URIs

// Simulate exactly what happens in the LSP flow
originalURI := lsproto.DocumentUri("untitled:Untitled-2")

// Step 1: URI gets converted to filename when file is opened
fileName := ls.DocumentURIToFileName(originalURI)
t.Logf("1. Opening file: URI '%s' -> fileName '%s'", originalURI, fileName)

// Step 2: fileName gets processed through ToPath in project service
currentDir := "/home/daniel/TypeScript" // Current directory from the original issue
path := tspath.ToPath(fileName, currentDir, true)
t.Logf("2. Project service processes: fileName '%s' -> path '%s'", fileName, string(path))

// Step 3: Verify path is NOT corrupted by current directory resolution
if strings.HasPrefix(string(path), currentDir) {
t.Fatalf("❌ BUG: Path was incorrectly resolved against current directory: %s", string(path))
}

// Step 4: When references are found, the path gets converted back to URI
resultURI := ls.FileNameToDocumentURI(string(path))
t.Logf("3. References return: path '%s' -> URI '%s'", string(path), resultURI)

// Step 5: Verify the round-trip conversion works
if string(resultURI) != string(originalURI) {
t.Fatalf("❌ Round-trip failed: %s != %s", originalURI, resultURI)
}

t.Logf("✅ SUCCESS: Untitled file URIs are preserved correctly")
t.Logf(" Original URI: %s", originalURI)
t.Logf(" Final URI: %s", resultURI)
}
5 changes: 5 additions & 0 deletions internal/tspath/path.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,11 @@ func GetEncodedRootLength(path string) int {
}
}

// Untitled paths (e.g., "^/untitled/ts-nul-authority/Untitled-1")
if ch0 == '^' && ln > 1 && path[1] == '/' {
return 2 // Untitled: "^/"
}

// URL
schemeEnd := strings.Index(path, urlSchemeSeparator)
if schemeEnd != -1 {
Expand Down
58 changes: 58 additions & 0 deletions internal/tspath/untitled_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package tspath_test

import (
"testing"

"github.com/microsoft/typescript-go/internal/tspath"
"gotest.tools/v3/assert"
)

func TestUntitledPathHandling(t *testing.T) {

Check failure on line 10 in internal/tspath/untitled_test.go

View workflow job for this annotation

GitHub Actions / copilot

Function TestUntitledPathHandling missing the call to method parallel (paralleltest)
// Test that untitled paths are treated as rooted
untitledPath := "^/untitled/ts-nul-authority/Untitled-2"

// GetEncodedRootLength should return 2 for "^/"
rootLength := tspath.GetEncodedRootLength(untitledPath)
assert.Equal(t, rootLength, 2, "GetEncodedRootLength should return 2 for untitled paths")

// IsRootedDiskPath should return true
isRooted := tspath.IsRootedDiskPath(untitledPath)
assert.Assert(t, isRooted, "IsRootedDiskPath should return true for untitled paths")

// ToPath should not resolve untitled paths against current directory
currentDir := "/home/user/project"
path := tspath.ToPath(untitledPath, currentDir, true)
// The path should be the original untitled path
assert.Equal(t, string(path), "^/untitled/ts-nul-authority/Untitled-2", "ToPath should not resolve untitled paths against current directory")

// Test GetNormalizedAbsolutePath doesn't resolve untitled paths
normalized := tspath.GetNormalizedAbsolutePath(untitledPath, currentDir)
assert.Equal(t, normalized, "^/untitled/ts-nul-authority/Untitled-2", "GetNormalizedAbsolutePath should not resolve untitled paths")
}

func TestUntitledPathEdgeCases(t *testing.T) {

Check failure on line 33 in internal/tspath/untitled_test.go

View workflow job for this annotation

GitHub Actions / copilot

Function TestUntitledPathEdgeCases missing the call to method parallel (paralleltest)
// Test edge cases
testCases := []struct {
path string
expected int
isRooted bool
}{
{"^/", 2, true}, // Minimal untitled path
{"^/untitled/ts-nul-authority/test", 2, true}, // Normal untitled path
{"^", 0, false}, // Just ^ is not rooted
{"^x", 0, false}, // ^x is not untitled
{"^^/", 0, false}, // ^^/ is not untitled
{"x^/", 0, false}, // x^/ is not untitled (doesn't start with ^)
{"^/untitled/ts-nul-authority/path/with/deeper/structure", 2, true}, // Deeper path
}

for _, tc := range testCases {

Check failure on line 49 in internal/tspath/untitled_test.go

View workflow job for this annotation

GitHub Actions / copilot

Range statement for test TestUntitledPathEdgeCases missing the call to method parallel in test Run (paralleltest)
t.Run(tc.path, func(t *testing.T) {
rootLength := tspath.GetEncodedRootLength(tc.path)
assert.Equal(t, rootLength, tc.expected, "GetEncodedRootLength for path %s", tc.path)

isRooted := tspath.IsRootedDiskPath(tc.path)
assert.Equal(t, isRooted, tc.isRooted, "IsRootedDiskPath for path %s", tc.path)
})
}
}
Loading