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.

Sign up for GitHub

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

Jump to bottom

x/tools/internal/span: URI.IsFile() should support file:/absolute/path URIs #49501

Open
briandealwis opened this issue Nov 10, 2021 · 4 comments · May be fixed by golang/tools#358
Open

x/tools/internal/span: URI.IsFile() should support file:/absolute/path URIs #49501

briandealwis opened this issue Nov 10, 2021 · 4 comments · May be fixed by golang/tools#358
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@briandealwis
Copy link

briandealwis commented Nov 10, 2021 •
edited

gopls v0.7.3 with Eclipse LSP4e 0.13.8 is breaking as LSP4e sends local file URIs with a single / (protocol trace below). This causes internal/span.URI.IsFile() to reject the URI as a file: URI:

func (uri URI) IsFile() bool {
	return strings.HasPrefix(string(uri), "file://")
}

and leads to gopls rejecting the workspace folder:

unsupported URI schemes: [{file:/Users/bdealwis/Projects/Skaffold/repo-skaffold skaffold}] (gopls only supports file URIs)

The file: scheme an absolute path is a permitted value under RFC8089 (The "file" URI Scheme) as the file-hier-part can be a local-path, which in turn is an path-absolute

file-URI       = file-scheme ":" file-hier-part
file-scheme    = "file"
file-hier-part = ( "//" auth-path )
                     / local-path
local-path     = path-absolute

path-absolute is referenced from RFC3986:

path-absolute = "/" [ segment-nz *( "/" segment ) ]
segment       = *pchar
segment-nz    = 1*pchar

Although the protocol trace shows Eclipse LSP4e is inconsistently using both file:/// and file:/ URIs, gopls should not reject permitted values.

And indeed Go's net/url.Parse() has tests to ensure support of file:/ URIs.

pretty-printed protocol trace 
[t=1636554142972] LSP4E to ca.mt.gore.server:
{
  "jsonrpc": "2.0",
  "id": "1",
  "method": "initialize",
  "params": {
    "processId": 58708,
    "rootPath": "/Users/bdealwis/Projects/Skaffold/repo-skaffold/",
    "rootUri": "file:///Users/bdealwis/Projects/Skaffold/repo-skaffold/",
    "initializationOptions": {
      "experimentalWorkspaceModule": false,
      "experimentalDiagnosticsDelay": "100ms",
      "codelenses": {
        "test": true
      }
    },
    "capabilities": {
      "workspace": {
        "applyEdit": true,
        "workspaceEdit": {
          "documentChanges": true,
          "resourceOperations": [
            "create",
            "delete",
            "rename"
          ],
          "failureHandling": "undo"
        },
        "symbol": {
          "dynamicRegistration": true
        },
        "executeCommand": {
          "dynamicRegistration": true
        },
        "workspaceFolders": true
      },
      "textDocument": {
        "synchronization": {
          "willSave": true,
          "willSaveWaitUntil": true,
          "didSave": true
        },
        "completion": {
          "completionItem": {
            "snippetSupport": true,
            "documentationFormat": [
              "markdown",
              "plaintext"
            ]
          }
        },
        "hover": {
          "contentFormat": [
            "markdown",
            "plaintext"
          ]
        },
        "signatureHelp": {},
        "references": {},
        "documentHighlight": {},
        "documentSymbol": {
          "symbolKind": {
            "valueSet": [18,17,5,14,9,10,22,24,8,1,12,11,20,6,2,3,21,16,19,25,4,7,15,23,26,13]
          },
          "hierarchicalDocumentSymbolSupport": true
        },
        "formatting": {
          "dynamicRegistration": true
        },
        "rangeFormatting": {},
        "definition": {
          "linkSupport": true
        },
        "typeDefinition": {
          "linkSupport": true
        },
        "codeAction": {
          "codeActionLiteralSupport": {
            "codeActionKind": {
              "valueSet": [
                "quickfix",
                "refactor",
                "refactor.extract",
                "refactor.inline",
                "refactor.rewrite",
                "source",
                "source.organizeImports"
              ]
            }
          },
          "dynamicRegistration": true
        },
        "codeLens": {},
        "documentLink": {},
        "colorProvider": {},
        "rename": {},
        "foldingRange": {}
      }
    },
    "clientName": "Eclipse IDE",
    "trace": "messages",
    "workspaceFolders": [
      {
        "uri": "file:/Users/bdealwis/Projects/Skaffold/repo-skaffold",
        "name": "skaffold"
      }
    ]
  }
}

[t=1636554143277] ca.mt.gore.server to LSP4E:
{
  "jsonrpc": "2.0",
  "error": {
    "code": 0,
    "message": "unsupported URI schemes: [{file:/Users/bdealwis/Projects/Skaffold/repo-skaffold skaffold}] (gopls only supports file URIs)"
  },
  "id": "1"
}

(ca.mt.gore.server is a simple Eclipse plugin to install and configure gopls for Eclipse.)
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Nov 10, 2021
@gopherbot gopherbot added this to the Unreleased milestone Nov 10, 2021
@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 11, 2021
@cagedmantis
Copy link
Contributor

/cc @ianthehat @heschi

@heschi
Copy link
Contributor

heschi commented Nov 11, 2021

This is really part of gopls, cc @findleyr.

@briandealwis briandealwis linked a pull request Jan 7, 2022 that will close this issue
Open
@briandealwis
Copy link
Author

I've pushed a trivial patch in golang/tools#358

@gopherbot
Copy link

Change https://golang.org/cl/376234 mentions this issue: internal/span: fix URI.IsFile() to support local-path URIs

This was referenced Jan 7, 2022
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

Successfully merging a pull request may close this issue.

internal/span: fix URI.IsFile() to support local-path URIs briandealwis/tools
4 participants
@cagedmantis @briandealwis @gopherbot @heschi