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

Comments

@OneOfOne
Copy link
Contributor

@OneOfOne OneOfOne commented Aug 25, 2020

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 this to the Unreleased milestone Aug 25, 2020
@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Aug 25, 2020

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

@OneOfOne OneOfOne commented Aug 25, 2020

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

@stamblerre stamblerre commented Aug 25, 2020

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

@OneOfOne
Copy link
Contributor Author

@OneOfOne OneOfOne commented Aug 25, 2020

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

@stamblerre stamblerre commented Aug 25, 2020

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

@stamblerre stamblerre commented Aug 25, 2020

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.

@OneOfOne
Copy link
Contributor Author

@OneOfOne OneOfOne commented Aug 29, 2020

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

@OneOfOne OneOfOne closed this Aug 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.