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

customFormatter setting in go.alternateTools does not resolve variables in path #2582

Closed
dcarney-stripe opened this issue Dec 21, 2022 · 10 comments
Labels
HelpWanted Issues that are not prioritized by the maintainers. Help is requested from community contributors. NeedsFix The path to resolution is known, but the work has not been done.

Comments

@dcarney-stripe
Copy link

dcarney-stripe commented Dec 21, 2022

What version of Go, VS Code & VS Code Go extension are you using?

Version Information
  • Run go version to get version of Go from the VS Code integrated terminal.
go version go1.19.4 darwin/arm64
  • Run gopls -v version to get version of Gopls from the VS Code integrated terminal.
gopls -v version
Build info
----------
golang.org/x/tools/gopls v0.11.0
    golang.org/x/tools/gopls@v0.11.0 h1:/nvKHdTtePQmrv9XN3gIUN9MOdUrKzO/dcqgbG6x8EY=
    github.com/BurntSushi/toml@v1.2.1 h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak=
    github.com/google/go-cmp@v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
    github.com/sergi/go-diff@v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
    golang.org/x/exp@v0.0.0-20221031165847-c99f073a8326 h1:QfTh0HpN6hlw6D3vu8DAwC8pBIwikq0AI1evdm+FksE=
    golang.org/x/exp/typeparams@v0.0.0-20221031165847-c99f073a8326 h1:fl8k2zg28yA23264d82M4dp+YlJ3ngDcpuB1bewkQi4=
    golang.org/x/mod@v0.7.0 h1:LapD9S96VoQRhi/GrNTqeBJFrUjs5UHCAtTlgwA5oZA=
    golang.org/x/sync@v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o=
    golang.org/x/sys@v0.2.0 h1:ljd4t30dBnAvMZaQCevtY0xLLD0A+bRZXbgLMLU1F/A=
    golang.org/x/text@v0.4.0 h1:BrVqGRd7+k1DiOgtnFvAkoQEWQvBc25ouMJM6429SFg=
    golang.org/x/tools@v0.3.1-0.20221213193459-ca17b2c27ca8 h1:7/HkGkN/2ktghBCSRRgp31wAww4syfsW52tj7yirjWk=
    golang.org/x/vuln@v0.0.0-20221109205719-3af8368ee4fe h1:qptQiQwEpETwDiz85LKtChqif9xhVkAm8Nhxs0xnTww=
    honnef.co/go/tools@v0.3.3 h1:oDx7VAwstgpYpb3wv0oxiZlxY+foCpRAwY7Vk6XpAgA=
    mvdan.cc/gofumpt@v0.4.0 h1:JVf4NN1mIpHogBj7ABpgOyZc65/UUOkKQFkoURsz4MM=
    mvdan.cc/xurls/v2@v2.4.0 h1:tzxjVAj+wSBmDcF6zBB7/myTy3gX9xvi8Tyr28AuQgc=
go: go1.19.4
  • Run code -v or code-insiders -v to get version of VS Code or VS Code Insiders.
code -v
1.74.0
5235c6bb189b60b01b1f49062f4ffa42384f8c91
arm64
  • Check your installed extensions to get the version of the VS Code Go extension
0.37.0
  • Run Ctrl+Shift+P (Cmd+Shift+P on Mac OS) > Go: Locate Configured Go Tools command.
Checking configured tools....
GOBIN: undefined
toolsGopath: 
gopath: /Users/dcarney/go
GOROOT: /opt/homebrew/Cellar/go/1.19.4/libexec
PATH:  (redacted)

	go:	/opt/homebrew/Cellar/go/1.19.4/libexec/bin/go: go version go1.19.4 darwin/arm64

	gotests:	not installed
	gomodifytags:	not installed
	impl:	not installed
	goplay:	not installed
	dlv:	/Users/dcarney/go/bin/dlv	(version: v1.9.1 built with go: go1.19.2)
	staticcheck:	/Users/dcarney/go/bin/staticcheck	(version: v0.3.3 built with go: go1.19.2)
	gopls:	/Users/dcarney/go/bin/gopls	(version: v0.11.0 built with go: go1.19.4)

go env
Workspace Folder (redacted): (redacted)
	GO111MODULE="on"
	GOARCH="arm64"
	GOBIN=""
	GOCACHE="/Users/dcarney/Library/Caches/go-build"
	GOENV="/Users/dcarney/Library/Application Support/go/env"
	GOEXE=""
	GOEXPERIMENT=""
	GOFLAGS=""
	GOHOSTARCH="arm64"
	GOHOSTOS="darwin"
	GOINSECURE=""
	GOMODCACHE="/Users/dcarney/go/pkg/mod"
	GOOS="darwin"
	GOPATH="/Users/dcarney/go"
	GOPROXY="https://proxy.golang.org,direct"
	GOROOT="/opt/homebrew/Cellar/go/1.19.4/libexec"
	GOSUMDB="sum.golang.org"
	GOTMPDIR=""
	GOTOOLDIR="/opt/homebrew/Cellar/go/1.19.4/libexec/pkg/tool/darwin_arm64"
	GOVCS=""
	GOVERSION="go1.19.4"
	GCCGO="gccgo"
	AR="ar"
	CC="clang"
	CXX="clang++"
	CGO_ENABLED="0"
	GOMOD="(redacted)"
	GOWORK=""
	CGO_CFLAGS="-g -O2"
	CGO_CPPFLAGS=""
	CGO_CXXFLAGS="-g -O2"
	CGO_FFLAGS="-g -O2"
	CGO_LDFLAGS="-g -O2"
	PKG_CONFIG="pkg-config"
	GOGCCFLAGS="-fPIC -arch arm64 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/lt/r0k33__90291gp354_ydjs5w0000gn/T/go-build2975243760=/tmp/go-build -gno-record-gcc-switches -fno-common"
	

Share the Go related settings you have added/edited

Run Preferences: Open Settings (JSON) command to open your settings.json file.
Share all the settings with the go. or ["go"] or gopls prefixes.

  "go.enableCodeLens": {
    "references": false,
    "runtest": false
  },
  "gopls": {
    "build.directoryFilters": [
      // Bazel symlinks
      "-bazel-bin",
      "-bazel-gocode",
      "-bazel-out",
      "-bazel-testlogs"
    ],
    "ui.semanticTokens": true,
    "ui.codelenses": {
      "gc_details": false,
      "regenerate_cgo": false,
      "generate": false,
      "test": false,
      "tidy": false,
      "upgrade_dependency": false,
      "vendor": false
    },
    "build.memoryMode": "DegradeClosed"
  },
  "go.useLanguageServer": true,
  "go.buildOnSave": "off",
  "go.lintOnSave": "off",
  "go.vetOnSave": "off",
  "go.toolsEnvVars": {
    "CGO_ENABLED": "0"
  },
  "go.languageServerFlags": [
        "-rpc.trace"
  ],
  "go.formatTool": "custom",
  "go.alternateTools": {
      "customFormatter": "${workspaceFolder}/bin/goimports.sh"
      "gopls": "${workspaceFolder}/bin/gopls.sh"
  }
}

Describe the bug

A clear and concise description of what the bug.

I recently noticed this commit, which enables the use of a custom formatter while also using gopls! 🎉

However, it appears that the value supplied for the path to the custom formatter doesn't get variables resolved,

For example, given the following in .vscode/settings.json:

 "go.formatTool": "custom",
 "go.alternateTools": {
      "gopls": "${workspaceFolder}/bin/gopls.sh",
      "customFormatter": "${workspaceFolder}/bin/goimports.sh"
  }

the alternate tool for gopls gets resolved properly, but the tool for customFormatter does not, leading to a popup like:

vscode-go

(actual PATH redacted)

A clear and concise description of what you expected to happen.

Steps to reproduce the behavior:

  • set "go.formatTool" to "custom"
  • set "customFormatter" to a valid path that contains the ${workspaceFolder} variable in "go.alternateTools"
  • save a change to a .go file to attempt to run the custom formatter

Screenshots or recordings

See above screencap of the popup that is shown when such a customFormatter value is used.

@gopherbot gopherbot added this to the Untriaged milestone Dec 21, 2022
@dcarney-stripe dcarney-stripe changed the title customFormatter setting does not resolve variables in path customFormatter setting in go.alternateTools does not resolve variables in path Dec 21, 2022
@hyangah hyangah modified the milestones: Untriaged, v0.38.0 Dec 22, 2022
@hyangah
Copy link
Contributor

hyangah commented Dec 22, 2022

Thanks for the report.

At some point, we need to call resolvePath due to vscode's current limitation microsoft/vscode#2809.

Out of curiosity - @dcarney-stripe what is the reason the default formatter gopls provides isn't sufficient?

@hyangah hyangah added the NeedsFix The path to resolution is known, but the work has not been done. label Dec 22, 2022
@hyangah hyangah modified the milestones: v0.38.0, vscode-go/later Dec 22, 2022
@dcarney-stripe
Copy link
Author

@hyangah Great, thanks for taking a look!

We have a custom formatter the wraps goimports and does several things that goimports doesn't do natively:

  • ignores certain generated files (for which we have no control over the formatting)
  • builds/installs a goimports binary for the user if they don't already have one present on their machine
  • handles the consolidation of various "import groups", similar to the discussion in this open issue

On that last point: running goimports on a file with imports like the following:

import (
        "errors"
        "net/http"

        "github.com/foo/bar"
        
        "github.com/hello/world"

        "flag"
        "fmt"
)

won't produce any substantive changes, whereas our custom formatter would produce:

import (
        "errors"
        "flag"
        "fmt"
        "net/http"

        "github.com/foo/bar"
        "github.com/hello/world"
)

@dcarney-stripe
Copy link
Author

At some point, we need to call resolvePath due to vscode's current limitation microsoft/vscode#2809.

@hyangah Yeah, the weird behavior (as mentioned in the issue report), is that the variable resolution appears to work for other tools in the "go.alternateTools" block, just not the "customFormatter" value.

@hyangah hyangah added the HelpWanted Issues that are not prioritized by the maintainers. Help is requested from community contributors. label Dec 22, 2022
@Aramatis
Copy link

Hi, I have a similar problem and I'm not sure if it warrants opening a new issue:
Variables are not being resolved (for settings.json) in any argument added to go.lintFlags either, I noticed trying to add a custom config file with the following config:

{
    "go.lintFlags": [
        "-c",
        "${workspaceFolder}/.code_quality/.golangci.yml",
        "--fast",
    ],
    "go.lintTool": "golangci-lint",
}

And getting an error with the variable without expanding, and then tried other variables that seem to be supported for expansion in the extension according to resolvePath and noticed that neither of them were being expanded either.

@uniquefine
Copy link
Contributor

@hyangah Would it be enough to call resolvePath here?

return goConfig['alternateTools']['customFormatter'] || 'goimports';

@hyangah
Copy link
Contributor

hyangah commented Aug 15, 2023

@uniquefine Yes, that will work.

@uniquefine
Copy link
Contributor

@Aramatis not sure if it's still relevant but variables in arguments are expanded if the argument starts with --config= or -config=

vscode-go/src/goLint.ts

Lines 104 to 112 in b2303c5

if (flag.startsWith('--config=') || flag.startsWith('-config=')) {
let configFilePath = flag.substr(flag.indexOf('=') + 1).trim();
if (!configFilePath) {
return;
}
configFilePath = resolvePath(configFilePath);
args.push(`${flag.substr(0, flag.indexOf('=') + 1)}${configFilePath}`);
return;
}

@ian-h-chamberlain
Copy link

ian-h-chamberlain commented Aug 23, 2023

@uniquefine Thank you for pointing that out! Is that documented anywhere, or is there any reason it's not resolved in all the lintFlags ? I had the same problem as @Aramatis and find this special-casing for --config= to be rather surprising behavior. Should I open a new issue for documentation and/or more consistent support for variable resolution across all flags?

Edit: maybe actually a question for @hyangah

@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/576296 mentions this issue: extension: resolve variables in customFormatter field.

@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/580418 mentions this issue: extension: resolve variables in customFormatter field.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HelpWanted Issues that are not prioritized by the maintainers. Help is requested from community contributors. NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants