Skip to content
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

Fix failing tests and path parsing on Windows #73

Merged
merged 2 commits into from
Apr 29, 2020
Merged
Show file tree
Hide file tree
Changes from all 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 commands/completion_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func (c *CompletionCommand) Run(args []string) int {
return 1
}

lspUri := lsp.DocumentURI("file://" + path)
lspUri := ilsp.FileHandlerFromPath(path).DocumentURI()
parts := strings.Split(c.atPos, ":")
if len(parts) != 2 {
c.Ui.Error(fmt.Sprintf("Error parsing at-pos argument: %q (expected line:col format)\n", c.atPos))
Expand Down
2 changes: 1 addition & 1 deletion internal/filesystem/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ type FileNotOpenErr struct {
}

func (e *FileNotOpenErr) Error() string {
return fmt.Sprintf("file is not open: %s", e.FileHandler.DocumentURI())
return fmt.Sprintf("file is not open: %s", e.FileHandler.URI())
}
6 changes: 2 additions & 4 deletions internal/filesystem/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ func NewFile(fullPath string, content []byte) *file {
return &file{fullPath: fullPath, content: content}
}

const uriPrefix = "file://"

func (f *file) FullPath() string {
return f.fullPath
}
Expand All @@ -38,8 +36,8 @@ func (f *file) Filename() string {
return filepath.Base(f.FullPath())
}

func (f *file) DocumentURI() string {
return uriPrefix + f.fullPath
func (f *file) URI() string {
return URIFromPath(f.fullPath)
}

func (f *file) Lines() source.Lines {
Expand Down
2 changes: 1 addition & 1 deletion internal/filesystem/filesystem_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ type testHandler struct {
uri string
}

func (fh *testHandler) DocumentURI() string {
func (fh *testHandler) URI() string {
return fh.uri
}

Expand Down
2 changes: 1 addition & 1 deletion internal/filesystem/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type VersionedFileHandler interface {
}

type FileHandler interface {
DocumentURI() string
URI() string
FullPath() string
Dir() string
Filename() string
Expand Down
17 changes: 17 additions & 0 deletions internal/filesystem/uri.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package filesystem

import (
"net/url"
"path/filepath"
)

func URIFromPath(path string) string {
p := filepath.ToSlash(path)
p = wrapPath(p)

u := &url.URL{
Scheme: "file",
Path: p,
}
return u.String()
}
8 changes: 8 additions & 0 deletions internal/filesystem/uri_unix.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// +build !windows

package filesystem

// wrapPath is no-op for unix-style paths
func wrapPath(path string) string {
return path
}
18 changes: 18 additions & 0 deletions internal/filesystem/uri_unix_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// +build !windows

package filesystem

import (
"testing"
)

func TestURIFromPath(t *testing.T) {
path := "/random/path"
uri := URIFromPath(path)

expectedURI := "file:///random/path"
if uri != expectedURI {
t.Fatalf("URI doesn't match.\nExpected: %q\nGiven: %q",
expectedURI, uri)
}
}
8 changes: 8 additions & 0 deletions internal/filesystem/uri_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package filesystem

// wrapPath prepends Windows-style paths (C:\path)
// with an additional slash to account for an empty hostname
// in a valid file-scheme URI per RFC 8089
func wrapPath(path string) string {
return "/" + path
}
16 changes: 16 additions & 0 deletions internal/filesystem/uri_windows_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package filesystem

import (
"testing"
)

func TestURIFromPath(t *testing.T) {
path := `C:\Users\With Space\file.tf`
uri := URIFromPath(path)

expectedURI := "file:///C:/Users/With%20Space/file.tf"
if uri != expectedURI {
t.Fatalf("URI doesn't match.\nExpected: %q\nGiven: %q",
expectedURI, uri)
}
}
6 changes: 3 additions & 3 deletions internal/lsp/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
)

type File interface {
DocumentURI() string
URI() string
FullPath() string
Dir() string
Filename() string
Expand All @@ -19,8 +19,8 @@ type file struct {
text []byte
}

func (f *file) DocumentURI() string {
return f.fh.DocumentURI()
func (f *file) URI() string {
return f.fh.URI()
}

func (f *file) FullPath() string {
Expand Down
33 changes: 18 additions & 15 deletions internal/lsp/file_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,34 +3,29 @@ package lsp
import (
"net/url"
"path/filepath"
"strings"

"github.com/hashicorp/terraform-ls/internal/filesystem"
"github.com/sourcegraph/go-lsp"
)

const uriPrefix = "file://"

type FileHandler string

func (fh FileHandler) Valid() bool {
if !strings.HasPrefix(string(fh), uriPrefix) {
return false
}
p := string(fh[len(uriPrefix):])
_, err := url.PathUnescape(p)
_, err := fh.parsePath()
if err != nil {
return false
}

return true
}

func (fh FileHandler) FullPath() string {
if !fh.Valid() {
panic("invalid uri")
func (fh FileHandler) parsePath() (string, error) {
u, err := url.ParseRequestURI(string(fh))
if err != nil {
return "", err
}
p := string(fh[len(uriPrefix):])
p, _ = url.PathUnescape(p)
return filepath.FromSlash(p)

return url.PathUnescape(u.Path)
}

func (fh FileHandler) Dir() string {
Expand All @@ -41,7 +36,11 @@ func (fh FileHandler) Filename() string {
return filepath.Base(fh.FullPath())
}

func (fh FileHandler) DocumentURI() string {
func (fh FileHandler) DocumentURI() lsp.DocumentURI {
return lsp.DocumentURI(fh)
}

func (fh FileHandler) URI() string {
return string(fh)
}

Expand All @@ -60,3 +59,7 @@ func VersionedFileHandler(doc lsp.VersionedTextDocumentIdentifier) *versionedFil
func (fh *versionedFileHandler) Version() int {
return fh.v
}

func FileHandlerFromPath(path string) FileHandler {
return FileHandler(filesystem.URIFromPath(path))
}
37 changes: 5 additions & 32 deletions internal/lsp/file_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,42 +4,15 @@ import (
"testing"
)

var (
validUnixPath = "file:///valid/path/to/file.tf"
validWindowsPath = "file:///C:/Users/With%20Space/tf-test/file.tf"
)

func TestFileHandler_invalid(t *testing.T) {
path := "invalidpath"
fh := FileHandler(path)
if fh.Valid() {
t.Fatalf("Expected %q to be invalid", path)
}
}

func TestFileHandler_valid(t *testing.T) {
path := "file://valid/path/to/file.tf"
fh := FileHandler(path)
if !fh.Valid() {
t.Fatalf("Expected %q to be valid", path)
}

expectedDir := "valid/path/to"
if fh.Dir() != expectedDir {
t.Fatalf("Expected dir: %q, given: %q",
expectedDir, fh.Dir())
}

expectedFilename := "file.tf"
if fh.Filename() != expectedFilename {
t.Fatalf("Expected filename: %q, given: %q",
expectedFilename, fh.Filename())
}

expectedFullPath := "valid/path/to/file.tf"
if fh.FullPath() != expectedFullPath {
t.Fatalf("Expected full path: %q, given: %q",
expectedFullPath, fh.FullPath())
}

expectedDocumentURI := "file://valid/path/to/file.tf"
if fh.DocumentURI() != expectedDocumentURI {
t.Fatalf("Expected document URI: %q, given: %q",
expectedDocumentURI, fh.DocumentURI())
}
}
16 changes: 16 additions & 0 deletions internal/lsp/file_handler_unix.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// +build !windows

package lsp

import (
"path/filepath"
)

func (fh FileHandler) FullPath() string {
p, err := fh.parsePath()
if err != nil {
panic("invalid uri")
}

return filepath.FromSlash(p)
}
37 changes: 37 additions & 0 deletions internal/lsp/file_handler_unix_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// +build !windows

package lsp

import (
"testing"
)

func TestFileHandler_valid_unix(t *testing.T) {
fh := FileHandler(validUnixPath)
if !fh.Valid() {
t.Fatalf("Expected %q to be valid", validUnixPath)
}

expectedDir := "/valid/path/to"
if fh.Dir() != expectedDir {
t.Fatalf("Expected dir: %q, given: %q",
expectedDir, fh.Dir())
}

expectedFilename := "file.tf"
if fh.Filename() != expectedFilename {
t.Fatalf("Expected filename: %q, given: %q",
expectedFilename, fh.Filename())
}

expectedFullPath := "/valid/path/to/file.tf"
if fh.FullPath() != expectedFullPath {
t.Fatalf("Expected full path: %q, given: %q",
expectedFullPath, fh.FullPath())
}

if fh.URI() != validUnixPath {
t.Fatalf("Expected document URI: %q, given: %q",
validUnixPath, fh.URI())
}
}
21 changes: 21 additions & 0 deletions internal/lsp/file_handler_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package lsp

import (
"path/filepath"
"strings"
)

// FullPath on Windows strips the leading '/'
// which occurs in Windows-style paths (e.g. file:///C:/)
// as url.URL methods don't account for that
// (see golang/go#6027).
func (fh FileHandler) FullPath() string {
p, err := fh.parsePath()
if err != nil {
panic("invalid uri")
}

p = strings.TrimPrefix(p, "/")

return filepath.FromSlash(p)
}
36 changes: 36 additions & 0 deletions internal/lsp/file_handler_windows_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package lsp

import (
"testing"
)

func TestFileHandler_valid_windows(t *testing.T) {
path := "file:///C:/Users/With%20Space/tf-test/file.tf"
fh := FileHandler(path)
if !fh.Valid() {
t.Fatalf("Expected %q to be valid", path)
}

expectedDir := `C:\Users\With Space\tf-test`
if fh.Dir() != expectedDir {
t.Fatalf("Expected dir: %q, given: %q",
expectedDir, fh.Dir())
}

expectedFilename := "file.tf"
if fh.Filename() != expectedFilename {
t.Fatalf("Expected filename: %q, given: %q",
expectedFilename, fh.Filename())
}

expectedFullPath := `C:\Users\With Space\tf-test\file.tf`
if fh.FullPath() != expectedFullPath {
t.Fatalf("Expected full path: %q, given: %q",
expectedFullPath, fh.FullPath())
}

if fh.URI() != validWindowsPath {
t.Fatalf("Expected document URI: %q, given: %q",
validWindowsPath, fh.URI())
}
}
4 changes: 2 additions & 2 deletions internal/lsp/position.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ func (p *filePosition) Position() hcl.Pos {
return p.pos
}

func (p *filePosition) DocumentURI() string {
return p.fh.DocumentURI()
func (p *filePosition) URI() string {
return p.fh.URI()
}

func (p *filePosition) FullPath() string {
Expand Down
Loading