golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.68k stars 17.49k forks source link

x/tools/gopls: bump go2 support #41020

Closed OneOfOne closed 4 years ago

OneOfOne commented 4 years ago

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.

stamblerre commented 4 years ago

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.

OneOfOne commented 4 years ago

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 commented 4 years ago

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

OneOfOne commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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