Files with _GOOS suffix cause build & documentation issue

[SteelPhase]

Hover Information, Signature Help, Go to/Peek Definition, Find All References, Rename Symbol, and
Change All Occurrences all seem to be affected by this.

Current workaround:

This is a clunky solution, but also breaks the same features listed above for files using other os suffixes

• Start vscode with a modified goos environment value. env GOOS=linux code .

Alternative workaround:

Less clunky version of the above workaround, but still breaks the above features for files using other os suffixes

• update user settings "go.toolsEnvVars": {"GOOS": "linux"}

Steps to Reproduce:

1. clone https://github.com/kardianos/service
2. open any file with a suffix that doesn't match your default GOOS
3. Attempt to view hover information of a function

Is there some way to get vscode to handle these functions without overwriting the GOOS environment variable every time?

[ramya-rao-a]

Isnt this a tooling problem? We use tools like godef, gogetdoc, guru, gorename etc for the features that you have mentioned. If they do not work with such files, they would have a good reason to. I wouldnt want to be making assumptions and overriding the tool behavior.

[SteelPhase]

All the tools do appear to work, but is depended on the Specific GOOS at the time of their execution it appears.

The go extention for vscode should be able to resolve this issue in one way or another.
A quick and dirty solution i would suggest would be modifying the GOOS environment variable for executions of these tools when the are attempted on files with GOOS suffixes.

Even if it isn't default functionality, an option to turn it on would be nice.

[ramya-rao-a]

How are we to identify the files for which GOOS should be changed? Is it based on the file names? Is that a standard or something only https://github.com/kardianos/service uses?

[SteelPhase]

Excellent point. Definitely should have included this before. This functionality is standard in the Golang build system. Along with file suffixes, build comments can be included at the top of the file that introduce the same limitation.

Build Constraints

A build constraint, also known as a build tag, is a line comment that begins

// +build

that lists the conditions under which a file should be included in the package. Constraints may appear in any kind of source file (not just Go), but they must appear near the top of the file, preceded only by blank lines and other line comments. These rules mean that in Go files a build constraint must appear before the package clause.

To distinguish build constraints from package documentation, a series of build constraints must be followed by a blank line.

A build constraint is evaluated as the OR of space-separated options; each option evaluates as the AND of its comma-separated terms; and each term is an alphanumeric word or, preceded by !, its negation. That is, the build constraint:

// +build linux,386 darwin,!cgo

corresponds to the boolean formula:

(linux AND 386) OR (darwin AND (NOT cgo))

A file may have multiple build constraints. The overall constraint is the AND of the individual constraints. That is, the build constraints:

// +build linux darwin
// +build 386

corresponds to the boolean formula:

(linux OR darwin) AND 386

During a particular build, the following words are satisfied:

- the target operating system, as spelled by runtime.GOOS
- the target architecture, as spelled by runtime.GOARCH
- the compiler being used, either "gc" or "gccgo"
- "cgo", if ctxt.CgoEnabled is true
- "go1.1", from Go version 1.1 onward
- "go1.2", from Go version 1.2 onward
- "go1.3", from Go version 1.3 onward
- "go1.4", from Go version 1.4 onward
- "go1.5", from Go version 1.5 onward
- "go1.6", from Go version 1.6 onward
- "go1.7", from Go version 1.7 onward
- "go1.8", from Go version 1.8 onward
- "go1.9", from Go version 1.9 onward
- "go1.10", from Go version 1.10 onward
- any additional words listed in ctxt.BuildTags

If a file's name, after stripping the extension and a possible _test suffix, matches any of the following patterns:

*_GOOS
*_GOARCH
*_GOOS_GOARCH

(example: source_windows_amd64.go) where GOOS and GOARCH represent any known operating system and architecture values respectively, then the file is considered to have an implicit build constraint requiring those terms (in addition to any explicit constraints in the file).

To keep a file from being considered for the build:

// +build ignore
(any other unsatisfied word will work as well, but “ignore” is conventional.)

To build a file only when using cgo, and only on Linux and OS X:

// +build linux,cgo darwin,cgo

Such a file is usually paired with another file implementing the default functionality for other systems, which in this case would carry the constraint:

// +build !linux,!darwin !cgo

Naming a file dns_windows.go will cause it to be included only when building the package for Windows; similarly, math_386.s will be included only when building the package for 32-bit x86.

Using GOOS=android matches build tags and files as for GOOS=linux in addition to android tags and files.

[ramya-rao-a]

Cool! I knew about build constraints when used as build tags, but wasnt aware of file names having a part to play as well.

Ok, so in this case, do we need to make the change for only the tools that take the -tags parameter or just all Go tools in general?

[SteelPhase]

So in my testing altering the GOOS environment variable passed to the tools enabled the functionality listed originally. GOARCH should work the same way. Parsing those out of the file name should be relatively straight forward. As an example "go.toolsEnvVars": {"GOOS": "linux"} was enough to handle that. The drawback there is that delve fails to work now, as the build would be for a linux OS. I guess i would limit it to only modify those environment variables in an instance where a user input on that file would cause a go command to execute.

Parsing build constraint comments seems like a can of worms to me, and I'm not sure what the best approach would be to handle constraints outside of GOOS and GOARCH

[ramya-rao-a]

Ok, then we can do the below:

• Update the getToolsEnvVars function to take an optional param say currentFileName
• When this parameter is passed, we parse the file name and add the additional env vars
• Then, we do a case by case pass on all usages of the go.toolEnvVars and pass the right value for the new param

PRs are welcome.

• Build and Debug extension has steps on getting started.
• getToolEnvVars is defined at https://github.com/Microsoft/vscode-go/blob/0.6.87/src/util.ts#L364

[ramya-rao-a]

For features that depend on individual files, the above approach would work. For example;

• code completion
• code navigation
• formatting

But what about features that work at a package level? Like build and vet?

[mreynolds]

Behaving like the golang platform itself is the lowest friction option ; eval for the current platform you're running on only. If parameters are passed in (ENV or config) that override GOOS for compilation reasons, the plugin should respect this, but I wouldn't bother trying to manage it proactively. I wouldn't try to build a fancy "multi-GOOS"-aware set of functionality without better golang support from the tools themselves ( https://golang.org/cmd/go/#hdr-GOPATH_and_Modules ; gofmt ).

[ramya-rao-a]

Since the build/vet/lint on save feature is triggered from a file save, I guess I can use that file's name to determine if the GOOS value needs to be set

[stamblerre]

I think that it will be very difficult to handle this correctly, particularly because it will be hard to debug cases where build tags are implicitly applied. I suggest that we delegate build tag handling to the underlying tools. After gopls/v1.0.0, for instance, we intend to work on improving support for build constraints more generally, and that will resolve some of the issues here.

[ramya-rao-a]

On giving this further thought, I agree with @stamblerre
Rather than duct tape these checks in the extension, I'd rather rely on the language server to do the right thing.

If we have features/commands not supported by the language server that need similar handling, they can be taken on a case to case basis