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

x/tools/gopls: bump go2 support #41020

Closed
OneOfOne opened this issue Aug 25, 2020 · 7 comments
Closed

x/tools/gopls: bump go2 support #41020

OneOfOne opened this issue Aug 25, 2020 · 7 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@OneOfOne
Copy link
Contributor

Please update the go2go branch or at least have an option to use brackets.

Getting spammed with inconsistent use of () or [] for type parameters, had to hack src/go/types/check.go manually.

@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Aug 25, 2020
@gopherbot gopherbot added this to the Unreleased milestone Aug 25, 2020
@stamblerre
Copy link
Contributor

Do you mind sharing the patch you applied? If the issue is in go/types, I imagine that gopls itself does not have to be updated, but rather just rebuilt with the latest version of go from the dev.go2go branch.

@stamblerre stamblerre modified the milestones: Unreleased, gopls/v.0.5.0 Aug 25, 2020
@OneOfOne
Copy link
Contributor Author

It has to be updated sadly because brackets isn't the default in the go2go branch either (which I also hacked), the relevant patch for x/tools is:

diff --git a/go/loader/loader.go b/go/loader/loader.go
index bc12ca33..f3b45a01 100644
--- a/go/loader/loader.go
+++ b/go/loader/loader.go
@@ -1027,6 +1027,9 @@ func (imp *importer) addFiles(info *PackageInfo, files []*ast.File, cycleCheck b
 	} else {
 		// Ignore the returned (first) error since we
 		// already collect them all in the PackageInfo.
+		for _, f := range files {
+			f.UseBrackets = true
+		}
 		info.checker.Files(files)
 		info.Files = append(info.Files, files...)
 	}

@stamblerre
Copy link
Contributor

Hm, I'm not sure that anything in gopls uses go/loader, but I can look into this tomorrow.

@OneOfOne
Copy link
Contributor Author

I also have a similar patch in go/src/go/types/check.go Checker.checkFiles, I pretty much ack'ed for checker.Files and manually changed it in all the places I saw, so ymmv.

@stamblerre
Copy link
Contributor

Yeah, it will be the go/types version that made gopls work, but if that's an exported field we can set it in gopls itself.
Do you mind sharing a code snippet that reproduces the problem?

@stamblerre
Copy link
Contributor

Based on https://groups.google.com/forum/?oldui=1#!topic/golang-nuts/iAD0NBz3DYw, it seems to me that there should be no reason to hardcode anything to true here.

@stamblerre stamblerre added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Aug 26, 2020
@OneOfOne
Copy link
Contributor Author

Seems like after the new set of updates from 2-3 days ago I no longer have to patch the stdlib.

@golang golang locked and limited conversation to collaborators Aug 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

3 participants