From e6a7e133888e97001109ab82f930ca7ea3449309 Mon Sep 17 00:00:00 2001 From: Zvonimir Pavlinovic Date: Mon, 7 Feb 2022 17:39:16 -0800 Subject: [PATCH 01/15] go/analysis/tools/internal/checker: add support for RunDespiteError When there is a syntax or type error, we should run the analyzers that have the RunDespiteError flag set to true, if any. The current code just skips any analysis whatsoever. For golang/go#51014 Change-Id: I5c3e5f3d9e2e8d791e9996b9114bb719e89daf30 Reviewed-on: https://go-review.googlesource.com/c/tools/+/383974 Reviewed-by: Michael Matloob Reviewed-by: Tim King Trust: Michael Matloob Run-TryBot: Zvonimir Pavlinovic gopls-CI: kokoro TryBot-Result: Gopher Robot --- go/analysis/internal/checker/checker.go | 57 +++++++++++++++---- go/analysis/internal/checker/checker_test.go | 59 ++++++++++++++++++++ 2 files changed, 106 insertions(+), 10 deletions(-) diff --git a/go/analysis/internal/checker/checker.go b/go/analysis/internal/checker/checker.go index 88613930d94..e405a2ae11a 100644 --- a/go/analysis/internal/checker/checker.go +++ b/go/analysis/internal/checker/checker.go @@ -11,6 +11,7 @@ package checker import ( "bytes" "encoding/gob" + "errors" "flag" "fmt" "go/format" @@ -129,8 +130,13 @@ func Run(args []string, analyzers []*analysis.Analyzer) (exitcode int) { allSyntax := needFacts(analyzers) initial, err := load(args, allSyntax) if err != nil { - log.Print(err) - return 1 // load errors + if _, ok := err.(typeParseError); !ok { + // Fail when some of the errors are not + // related to parsing nor typing. + log.Print(err) + return 1 + } + // TODO: filter analyzers based on RunDespiteError? } // Print the results. @@ -139,11 +145,17 @@ func Run(args []string, analyzers []*analysis.Analyzer) (exitcode int) { if Fix { applyFixes(roots) } - return printDiagnostics(roots) } -// load loads the initial packages. +// typeParseError represents a package load error +// that is related to typing and parsing. +type typeParseError struct { + error +} + +// load loads the initial packages. If all loading issues are related to +// typing and parsing, the returned error is of type typeParseError. func load(patterns []string, allSyntax bool) ([]*packages.Package, error) { mode := packages.LoadSyntax if allSyntax { @@ -155,18 +167,43 @@ func load(patterns []string, allSyntax bool) ([]*packages.Package, error) { } initial, err := packages.Load(&conf, patterns...) if err == nil { - if n := packages.PrintErrors(initial); n > 1 { - err = fmt.Errorf("%d errors during loading", n) - } else if n == 1 { - err = fmt.Errorf("error during loading") - } else if len(initial) == 0 { + if len(initial) == 0 { err = fmt.Errorf("%s matched no packages", strings.Join(patterns, " ")) + } else { + err = loadingError(initial) } } - return initial, err } +// loadingError checks for issues during the loading of initial +// packages. Returns nil if there are no issues. Returns error +// of type typeParseError if all errors, including those in +// dependencies, are related to typing or parsing. Otherwise, +// a plain error is returned with an appropriate message. +func loadingError(initial []*packages.Package) error { + var err error + if n := packages.PrintErrors(initial); n > 1 { + err = fmt.Errorf("%d errors during loading", n) + } else if n == 1 { + err = errors.New("error during loading") + } else { + // no errors + return nil + } + all := true + packages.Visit(initial, nil, func(pkg *packages.Package) { + for _, err := range pkg.Errors { + typeOrParse := err.Kind == packages.TypeError || err.Kind == packages.ParseError + all = all && typeOrParse + } + }) + if all { + return typeParseError{err} + } + return err +} + // TestAnalyzer applies an analysis to a set of packages (and their // dependencies if necessary) and returns the results. // diff --git a/go/analysis/internal/checker/checker_test.go b/go/analysis/internal/checker/checker_test.go index 50c51a106ca..eee211c21a4 100644 --- a/go/analysis/internal/checker/checker_test.go +++ b/go/analysis/internal/checker/checker_test.go @@ -99,3 +99,62 @@ func run(pass *analysis.Pass) (interface{}, error) { return nil, nil } + +func TestRunDespiteErrors(t *testing.T) { + testenv.NeedsGoPackages(t) + + files := map[string]string{ + "rderr/test.go": `package rderr + +// Foo deliberately has a type error +func Foo(s string) int { + return s + 1 +} +`} + + testdata, cleanup, err := analysistest.WriteFiles(files) + if err != nil { + t.Fatal(err) + } + path := filepath.Join(testdata, "src/rderr/test.go") + + // A no-op analyzer that should finish regardless of + // parse or type errors in the code. + noop := &analysis.Analyzer{ + Name: "noop", + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: func(pass *analysis.Pass) (interface{}, error) { + return nil, nil + }, + RunDespiteErrors: true, + } + + for _, test := range []struct { + name string + pattern []string + analyzers []*analysis.Analyzer + code int + }{ + // parse/type errors + {name: "skip-error", pattern: []string{"file=" + path}, analyzers: []*analysis.Analyzer{analyzer}, code: 1}, + {name: "despite-error", pattern: []string{"file=" + path}, analyzers: []*analysis.Analyzer{noop}, code: 0}, + // combination of parse/type errors and no errors + {name: "despite-error-and-no-error", pattern: []string{"file=" + path, "sort"}, analyzers: []*analysis.Analyzer{analyzer, noop}, code: 1}, + // non-existing package error + {name: "no-package", pattern: []string{"xyz"}, analyzers: []*analysis.Analyzer{analyzer}, code: 1}, + {name: "no-package-despite-error", pattern: []string{"abc"}, analyzers: []*analysis.Analyzer{noop}, code: 1}, + {name: "no-multi-package-despite-error", pattern: []string{"xyz", "abc"}, analyzers: []*analysis.Analyzer{noop}, code: 1}, + // combination of type/parsing and different errors + {name: "different-errors", pattern: []string{"file=" + path, "xyz"}, analyzers: []*analysis.Analyzer{analyzer, noop}, code: 1}, + // non existing dir error + {name: "no-match-dir", pattern: []string{"file=non/existing/dir"}, analyzers: []*analysis.Analyzer{analyzer, noop}, code: 1}, + // no errors + {name: "no-errors", pattern: []string{"sort"}, analyzers: []*analysis.Analyzer{analyzer, noop}, code: 0}, + } { + if got := checker.Run(test.pattern, test.analyzers); got != test.code { + t.Errorf("got incorrect exit code %d for test %s; want %d", got, test.name, test.code) + } + } + + defer cleanup() +} From 897bd77cd7177f6d679eafa9767ff6ef9211458a Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Fri, 18 Feb 2022 09:35:21 -0500 Subject: [PATCH 02/15] internal/gocommand: remove support for -workfile Remove support for passing the -workfile flag now that the go command no longer supports it. Updates #51215 Change-Id: I95d73fb1a3a6d9bcfaaae5e22e44722118d12c03 Reviewed-on: https://go-review.googlesource.com/c/tools/+/386536 Trust: Robert Findley Run-TryBot: Robert Findley gopls-CI: kokoro Reviewed-by: Hyang-Ah Hana Kim TryBot-Result: Gopher Robot --- internal/gocommand/invoke.go | 14 -------------- internal/lsp/cache/snapshot.go | 13 +------------ 2 files changed, 1 insertion(+), 26 deletions(-) diff --git a/internal/gocommand/invoke.go b/internal/gocommand/invoke.go index 64fbad37651..f7533683465 100644 --- a/internal/gocommand/invoke.go +++ b/internal/gocommand/invoke.go @@ -139,9 +139,6 @@ type Invocation struct { // If ModFile is set, the go command is invoked with -modfile=ModFile. ModFile string - // If WorkFile is set, the go command is invoked with -workfile=WorkFile. - WorkFile string - // If Overlay is set, the go command is invoked with -overlay=Overlay. Overlay string @@ -170,9 +167,6 @@ func (i *Invocation) runWithFriendlyError(ctx context.Context, stdout, stderr io } func (i *Invocation) run(ctx context.Context, stdout, stderr io.Writer) error { - if i.ModFile != "" && i.WorkFile != "" { - return fmt.Errorf("bug: go command invoked with both -modfile and -workfile") - } log := i.Logf if log == nil { log = func(string, ...interface{}) {} @@ -185,11 +179,6 @@ func (i *Invocation) run(ctx context.Context, stdout, stderr io.Writer) error { goArgs = append(goArgs, "-modfile="+i.ModFile) } } - appendWorkFile := func() { - if i.WorkFile != "" { - goArgs = append(goArgs, "-workfile="+i.WorkFile) - } - } appendModFlag := func() { if i.ModFlag != "" { goArgs = append(goArgs, "-mod="+i.ModFlag) @@ -208,19 +197,16 @@ func (i *Invocation) run(ctx context.Context, stdout, stderr io.Writer) error { // mod needs the sub-verb before flags. goArgs = append(goArgs, i.Args[0]) appendModFile() - appendWorkFile() goArgs = append(goArgs, i.Args[1:]...) case "get": goArgs = append(goArgs, i.BuildFlags...) appendModFile() - appendWorkFile() goArgs = append(goArgs, i.Args...) default: // notably list and build. goArgs = append(goArgs, i.BuildFlags...) appendModFile() appendModFlag() - appendWorkFile() appendOverlayFlag() goArgs = append(goArgs, i.Args...) } diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index f3ca77212eb..b9cd36c7458 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -425,18 +425,7 @@ func (s *snapshot) goCommandInvocation(ctx context.Context, flags source.Invocat // 3. We're using at least Go 1.18. useWorkFile := !needTempMod && s.workspace.moduleSource == goWorkWorkspace && s.view.goversion >= 18 if useWorkFile { - workURI := uriForSource(s.workspace.root, goWorkWorkspace) - workFH, err := s.GetFile(ctx, workURI) - if err != nil { - return "", nil, cleanup, err - } - // TODO(rfindley): we should use the last workfile that actually parsed, as - // tracked by the workspace. - tmpURI, cleanup, err = tempWorkFile(workFH) - if err != nil { - return "", nil, cleanup, err - } - inv.WorkFile = tmpURI.Filename() + // TODO(#51215): build a temp workfile and set GOWORK in the environment. } else if useTempMod { if modURI == "" { return "", nil, cleanup, fmt.Errorf("no go.mod file found in %s", inv.WorkingDir) From 43f084e5936dab0669d82f29b31f5c4518e458ab Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Fri, 18 Feb 2022 16:03:07 -0700 Subject: [PATCH 03/15] internal/typesinternal: update typesinternal for 1.18 This CL adds the error codes introduced in 1.17 and 1.18. Change-Id: I5646a2cdfe2b1a86756f3e9beb8b9c781d7cbccf Reviewed-on: https://go-review.googlesource.com/c/tools/+/386874 Trust: Suzy Mueller Run-TryBot: Suzy Mueller gopls-CI: kokoro TryBot-Result: Gopher Robot Reviewed-by: Robert Findley --- internal/typesinternal/errorcode.go | 158 +++++++++++++++++++++ internal/typesinternal/errorcode_string.go | 18 ++- 2 files changed, 174 insertions(+), 2 deletions(-) diff --git a/internal/typesinternal/errorcode.go b/internal/typesinternal/errorcode.go index fa2834e2ab8..d38ee3c27cd 100644 --- a/internal/typesinternal/errorcode.go +++ b/internal/typesinternal/errorcode.go @@ -1365,4 +1365,162 @@ const ( // return i // } InvalidGo + + // All codes below were added in Go 1.17. + + /* decl */ + + // BadDecl occurs when a declaration has invalid syntax. + BadDecl + + // RepeatedDecl occurs when an identifier occurs more than once on the left + // hand side of a short variable declaration. + // + // Example: + // func _() { + // x, y, y := 1, 2, 3 + // } + RepeatedDecl + + /* unsafe */ + + // InvalidUnsafeAdd occurs when unsafe.Add is called with a + // length argument that is not of integer type. + // + // Example: + // import "unsafe" + // + // var p unsafe.Pointer + // var _ = unsafe.Add(p, float64(1)) + InvalidUnsafeAdd + + // InvalidUnsafeSlice occurs when unsafe.Slice is called with a + // pointer argument that is not of pointer type or a length argument + // that is not of integer type, negative, or out of bounds. + // + // Example: + // import "unsafe" + // + // var x int + // var _ = unsafe.Slice(x, 1) + // + // Example: + // import "unsafe" + // + // var x int + // var _ = unsafe.Slice(&x, float64(1)) + // + // Example: + // import "unsafe" + // + // var x int + // var _ = unsafe.Slice(&x, -1) + // + // Example: + // import "unsafe" + // + // var x int + // var _ = unsafe.Slice(&x, uint64(1) << 63) + InvalidUnsafeSlice + + // All codes below were added in Go 1.18. + + /* features */ + + // UnsupportedFeature occurs when a language feature is used that is not + // supported at this Go version. + UnsupportedFeature + + /* type params */ + + // NotAGenericType occurs when a non-generic type is used where a generic + // type is expected: in type or function instantiation. + // + // Example: + // type T int + // + // var _ T[int] + NotAGenericType + + // WrongTypeArgCount occurs when a type or function is instantiated with an + // incorrent number of type arguments, including when a generic type or + // function is used without instantiation. + // + // Errors inolving failed type inference are assigned other error codes. + // + // Example: + // type T[p any] int + // + // var _ T[int, string] + // + // Example: + // func f[T any]() {} + // + // var x = f + WrongTypeArgCount + + // CannotInferTypeArgs occurs when type or function type argument inference + // fails to infer all type arguments. + // + // Example: + // func f[T any]() {} + // + // func _() { + // f() + // } + // + // Example: + // type N[P, Q any] struct{} + // + // var _ N[int] + CannotInferTypeArgs + + // InvalidTypeArg occurs when a type argument does not satisfy its + // corresponding type parameter constraints. + // + // Example: + // type T[P ~int] struct{} + // + // var _ T[string] + InvalidTypeArg // arguments? InferenceFailed + + // InvalidInstanceCycle occurs when an invalid cycle is detected + // within the instantiation graph. + // + // Example: + // func f[T any]() { f[*T]() } + InvalidInstanceCycle + + // InvalidUnion occurs when an embedded union or approximation element is + // not valid. + // + // Example: + // type _ interface { + // ~int | interface{ m() } + // } + InvalidUnion + + // MisplacedConstraintIface occurs when a constraint-type interface is used + // outside of constraint position. + // + // Example: + // type I interface { ~int } + // + // var _ I + MisplacedConstraintIface + + // InvalidMethodTypeParams occurs when methods have type parameters. + // + // It cannot be encountered with an AST parsed using go/parser. + InvalidMethodTypeParams + + // MisplacedTypeParam occurs when a type parameter is used in a place where + // it is not permitted. + // + // Example: + // type T[P any] P + // + // Example: + // type T[P any] struct{ *P } + MisplacedTypeParam ) diff --git a/internal/typesinternal/errorcode_string.go b/internal/typesinternal/errorcode_string.go index 3e5842a5f0f..de90e9515ae 100644 --- a/internal/typesinternal/errorcode_string.go +++ b/internal/typesinternal/errorcode_string.go @@ -138,11 +138,25 @@ func _() { _ = x[UnusedResults-128] _ = x[InvalidDefer-129] _ = x[InvalidGo-130] + _ = x[BadDecl-131] + _ = x[RepeatedDecl-132] + _ = x[InvalidUnsafeAdd-133] + _ = x[InvalidUnsafeSlice-134] + _ = x[UnsupportedFeature-135] + _ = x[NotAGenericType-136] + _ = x[WrongTypeArgCount-137] + _ = x[CannotInferTypeArgs-138] + _ = x[InvalidTypeArg-139] + _ = x[InvalidInstanceCycle-140] + _ = x[InvalidUnion-141] + _ = x[MisplacedConstraintIface-142] + _ = x[InvalidMethodTypeParams-143] + _ = x[MisplacedTypeParam-144] } -const _ErrorCode_name = "TestBlankPkgNameMismatchedPkgNameInvalidPkgUseBadImportPathBrokenImportImportCRenamedUnusedImportInvalidInitCycleDuplicateDeclInvalidDeclCycleInvalidTypeCycleInvalidConstInitInvalidConstValInvalidConstTypeUntypedNilWrongAssignCountUnassignableOperandNoNewVarMultiValAssignOpInvalidIfaceAssignInvalidChanAssignIncompatibleAssignUnaddressableFieldAssignNotATypeInvalidArrayLenBlankIfaceMethodIncomparableMapKeyInvalidIfaceEmbedInvalidPtrEmbedBadRecvInvalidRecvDuplicateFieldAndMethodDuplicateMethodInvalidBlankInvalidIotaMissingInitBodyInvalidInitSigInvalidInitDeclInvalidMainDeclTooManyValuesNotAnExprTruncatedFloatNumericOverflowUndefinedOpMismatchedTypesDivByZeroNonNumericIncDecUnaddressableOperandInvalidIndirectionNonIndexableOperandInvalidIndexSwappedSliceIndicesNonSliceableOperandInvalidSliceExprInvalidShiftCountInvalidShiftOperandInvalidReceiveInvalidSendDuplicateLitKeyMissingLitKeyInvalidLitIndexOversizeArrayLitMixedStructLitInvalidStructLitMissingLitFieldDuplicateLitFieldUnexportedLitFieldInvalidLitFieldUntypedLitInvalidLitAmbiguousSelectorUndeclaredImportedNameUnexportedNameUndeclaredNameMissingFieldOrMethodBadDotDotDotSyntaxNonVariadicDotDotDotMisplacedDotDotDotInvalidDotDotDotOperandInvalidDotDotDotUncalledBuiltinInvalidAppendInvalidCapInvalidCloseInvalidCopyInvalidComplexInvalidDeleteInvalidImagInvalidLenSwappedMakeArgsInvalidMakeInvalidRealInvalidAssertImpossibleAssertInvalidConversionInvalidUntypedConversionBadOffsetofSyntaxInvalidOffsetofUnusedExprUnusedVarMissingReturnWrongResultCountOutOfScopeResultInvalidCondInvalidPostDeclInvalidChanRangeInvalidIterVarInvalidRangeExprMisplacedBreakMisplacedContinueMisplacedFallthroughDuplicateCaseDuplicateDefaultBadTypeKeywordInvalidTypeSwitchInvalidExprSwitchInvalidSelectCaseUndeclaredLabelDuplicateLabelMisplacedLabelUnusedLabelJumpOverDeclJumpIntoBlockInvalidMethodExprWrongArgCountInvalidCallUnusedResultsInvalidDeferInvalidGo" +const _ErrorCode_name = "TestBlankPkgNameMismatchedPkgNameInvalidPkgUseBadImportPathBrokenImportImportCRenamedUnusedImportInvalidInitCycleDuplicateDeclInvalidDeclCycleInvalidTypeCycleInvalidConstInitInvalidConstValInvalidConstTypeUntypedNilWrongAssignCountUnassignableOperandNoNewVarMultiValAssignOpInvalidIfaceAssignInvalidChanAssignIncompatibleAssignUnaddressableFieldAssignNotATypeInvalidArrayLenBlankIfaceMethodIncomparableMapKeyInvalidIfaceEmbedInvalidPtrEmbedBadRecvInvalidRecvDuplicateFieldAndMethodDuplicateMethodInvalidBlankInvalidIotaMissingInitBodyInvalidInitSigInvalidInitDeclInvalidMainDeclTooManyValuesNotAnExprTruncatedFloatNumericOverflowUndefinedOpMismatchedTypesDivByZeroNonNumericIncDecUnaddressableOperandInvalidIndirectionNonIndexableOperandInvalidIndexSwappedSliceIndicesNonSliceableOperandInvalidSliceExprInvalidShiftCountInvalidShiftOperandInvalidReceiveInvalidSendDuplicateLitKeyMissingLitKeyInvalidLitIndexOversizeArrayLitMixedStructLitInvalidStructLitMissingLitFieldDuplicateLitFieldUnexportedLitFieldInvalidLitFieldUntypedLitInvalidLitAmbiguousSelectorUndeclaredImportedNameUnexportedNameUndeclaredNameMissingFieldOrMethodBadDotDotDotSyntaxNonVariadicDotDotDotMisplacedDotDotDotInvalidDotDotDotOperandInvalidDotDotDotUncalledBuiltinInvalidAppendInvalidCapInvalidCloseInvalidCopyInvalidComplexInvalidDeleteInvalidImagInvalidLenSwappedMakeArgsInvalidMakeInvalidRealInvalidAssertImpossibleAssertInvalidConversionInvalidUntypedConversionBadOffsetofSyntaxInvalidOffsetofUnusedExprUnusedVarMissingReturnWrongResultCountOutOfScopeResultInvalidCondInvalidPostDeclInvalidChanRangeInvalidIterVarInvalidRangeExprMisplacedBreakMisplacedContinueMisplacedFallthroughDuplicateCaseDuplicateDefaultBadTypeKeywordInvalidTypeSwitchInvalidExprSwitchInvalidSelectCaseUndeclaredLabelDuplicateLabelMisplacedLabelUnusedLabelJumpOverDeclJumpIntoBlockInvalidMethodExprWrongArgCountInvalidCallUnusedResultsInvalidDeferInvalidGoBadDeclRepeatedDeclInvalidUnsafeAddInvalidUnsafeSliceUnsupportedFeatureNotAGenericTypeWrongTypeArgCountCannotInferTypeArgsInvalidTypeArgInvalidInstanceCycleInvalidUnionMisplacedConstraintIfaceInvalidMethodTypeParamsMisplacedTypeParam" -var _ErrorCode_index = [...]uint16{0, 4, 16, 33, 46, 59, 71, 85, 97, 113, 126, 142, 158, 174, 189, 205, 215, 231, 250, 258, 274, 292, 309, 327, 351, 359, 374, 390, 408, 425, 440, 447, 458, 481, 496, 508, 519, 534, 548, 563, 578, 591, 600, 614, 629, 640, 655, 664, 680, 700, 718, 737, 749, 768, 787, 803, 820, 839, 853, 864, 879, 892, 907, 923, 937, 953, 968, 985, 1003, 1018, 1028, 1038, 1055, 1077, 1091, 1105, 1125, 1143, 1163, 1181, 1204, 1220, 1235, 1248, 1258, 1270, 1281, 1295, 1308, 1319, 1329, 1344, 1355, 1366, 1379, 1395, 1412, 1436, 1453, 1468, 1478, 1487, 1500, 1516, 1532, 1543, 1558, 1574, 1588, 1604, 1618, 1635, 1655, 1668, 1684, 1698, 1715, 1732, 1749, 1764, 1778, 1792, 1803, 1815, 1828, 1845, 1858, 1869, 1882, 1894, 1903} +var _ErrorCode_index = [...]uint16{0, 4, 16, 33, 46, 59, 71, 85, 97, 113, 126, 142, 158, 174, 189, 205, 215, 231, 250, 258, 274, 292, 309, 327, 351, 359, 374, 390, 408, 425, 440, 447, 458, 481, 496, 508, 519, 534, 548, 563, 578, 591, 600, 614, 629, 640, 655, 664, 680, 700, 718, 737, 749, 768, 787, 803, 820, 839, 853, 864, 879, 892, 907, 923, 937, 953, 968, 985, 1003, 1018, 1028, 1038, 1055, 1077, 1091, 1105, 1125, 1143, 1163, 1181, 1204, 1220, 1235, 1248, 1258, 1270, 1281, 1295, 1308, 1319, 1329, 1344, 1355, 1366, 1379, 1395, 1412, 1436, 1453, 1468, 1478, 1487, 1500, 1516, 1532, 1543, 1558, 1574, 1588, 1604, 1618, 1635, 1655, 1668, 1684, 1698, 1715, 1732, 1749, 1764, 1778, 1792, 1803, 1815, 1828, 1845, 1858, 1869, 1882, 1894, 1903, 1910, 1922, 1938, 1956, 1974, 1989, 2006, 2025, 2039, 2059, 2071, 2095, 2118, 2136} func (i ErrorCode) String() string { i -= 1 From 3e31058cd76dc16fda42f21eb56e224b6350332c Mon Sep 17 00:00:00 2001 From: Michael Matloob Date: Wed, 16 Feb 2022 14:09:00 -0500 Subject: [PATCH 04/15] internal/imports: update to permit multiple main modules Now that there there can be multiple main modules due to workspaces in cmd/go, update internal/imports to no longer assume that there will be exactly one main module. Change-Id: I9d044df612fbeaba73b8fc3dbbec0395b54ecba2 Reviewed-on: https://go-review.googlesource.com/c/tools/+/386254 Trust: Michael Matloob Run-TryBot: Michael Matloob Trust: Bryan Mills Reviewed-by: Heschi Kreinick gopls-CI: kokoro TryBot-Result: Gopher Robot --- cmd/godoc/main.go | 6 +- internal/gocommand/vendor.go | 22 +-- internal/imports/mod.go | 25 +-- internal/imports/mod_test.go | 330 +++++++++++++++++++++++++++++++++++ 4 files changed, 359 insertions(+), 24 deletions(-) diff --git a/cmd/godoc/main.go b/cmd/godoc/main.go index 9d6ac869cb6..352bb4b7a75 100644 --- a/cmd/godoc/main.go +++ b/cmd/godoc/main.go @@ -207,21 +207,21 @@ func main() { fmt.Printf("using module mode; GOMOD=%s\n", goModFile) // Detect whether to use vendor mode or not. - mainMod, vendorEnabled, err := gocommand.VendorEnabled(context.Background(), gocommand.Invocation{}, &gocommand.Runner{}) + vendorEnabled, mainModVendor, err := gocommand.VendorEnabled(context.Background(), gocommand.Invocation{}, &gocommand.Runner{}) if err != nil { fmt.Fprintf(os.Stderr, "failed to determine if vendoring is enabled: %v", err) os.Exit(1) } if vendorEnabled { // Bind the root directory of the main module. - fs.Bind(path.Join("/src", mainMod.Path), gatefs.New(vfs.OS(mainMod.Dir), fsGate), "/", vfs.BindAfter) + fs.Bind(path.Join("/src", mainModVendor.Path), gatefs.New(vfs.OS(mainModVendor.Dir), fsGate), "/", vfs.BindAfter) // Bind the vendor directory. // // Note that in module mode, vendor directories in locations // other than the main module's root directory are ignored. // See https://golang.org/ref/mod#vendoring. - vendorDir := filepath.Join(mainMod.Dir, "vendor") + vendorDir := filepath.Join(mainModVendor.Dir, "vendor") fs.Bind("/src", gatefs.New(vfs.OS(vendorDir), fsGate), "/", vfs.BindAfter) } else { diff --git a/internal/gocommand/vendor.go b/internal/gocommand/vendor.go index 5e75bd6d8fa..2d3d408c0be 100644 --- a/internal/gocommand/vendor.go +++ b/internal/gocommand/vendor.go @@ -38,10 +38,10 @@ var modFlagRegexp = regexp.MustCompile(`-mod[ =](\w+)`) // with the supplied context.Context and Invocation. The Invocation can contain pre-defined fields, // of which only Verb and Args are modified to run the appropriate Go command. // Inspired by setDefaultBuildMod in modload/init.go -func VendorEnabled(ctx context.Context, inv Invocation, r *Runner) (*ModuleJSON, bool, error) { +func VendorEnabled(ctx context.Context, inv Invocation, r *Runner) (bool, *ModuleJSON, error) { mainMod, go114, err := getMainModuleAnd114(ctx, inv, r) if err != nil { - return nil, false, err + return false, nil, err } // We check the GOFLAGS to see if there is anything overridden or not. @@ -49,7 +49,7 @@ func VendorEnabled(ctx context.Context, inv Invocation, r *Runner) (*ModuleJSON, inv.Args = []string{"GOFLAGS"} stdout, err := r.Run(ctx, inv) if err != nil { - return nil, false, err + return false, nil, err } goflags := string(bytes.TrimSpace(stdout.Bytes())) matches := modFlagRegexp.FindStringSubmatch(goflags) @@ -57,25 +57,27 @@ func VendorEnabled(ctx context.Context, inv Invocation, r *Runner) (*ModuleJSON, if len(matches) != 0 { modFlag = matches[1] } - if modFlag != "" { - // Don't override an explicit '-mod=' argument. - return mainMod, modFlag == "vendor", nil + // Don't override an explicit '-mod=' argument. + if modFlag == "vendor" { + return true, mainMod, nil + } else if modFlag != "" { + return false, nil, nil } if mainMod == nil || !go114 { - return mainMod, false, nil + return false, nil, nil } // Check 1.14's automatic vendor mode. if fi, err := os.Stat(filepath.Join(mainMod.Dir, "vendor")); err == nil && fi.IsDir() { if mainMod.GoVersion != "" && semver.Compare("v"+mainMod.GoVersion, "v1.14") >= 0 { // The Go version is at least 1.14, and a vendor directory exists. // Set -mod=vendor by default. - return mainMod, true, nil + return true, mainMod, nil } } - return mainMod, false, nil + return false, nil, nil } -// getMainModuleAnd114 gets the main module's information and whether the +// getMainModuleAnd114 gets one of the main modules' information and whether the // go command in use is 1.14+. This is the information needed to figure out // if vendoring should be enabled. func getMainModuleAnd114(ctx context.Context, inv Invocation, r *Runner) (*ModuleJSON, bool, error) { diff --git a/internal/imports/mod.go b/internal/imports/mod.go index dff6d55362c..2bcf41f5fa7 100644 --- a/internal/imports/mod.go +++ b/internal/imports/mod.go @@ -34,7 +34,8 @@ type ModuleResolver struct { scannedRoots map[gopathwalk.Root]bool initialized bool - main *gocommand.ModuleJSON + mains []*gocommand.ModuleJSON + mainByDir map[string]*gocommand.ModuleJSON modsByModPath []*gocommand.ModuleJSON // All modules, ordered by # of path components in module Path... modsByDir []*gocommand.ModuleJSON // ...or Dir. @@ -69,21 +70,21 @@ func (r *ModuleResolver) init() error { Logf: r.env.Logf, WorkingDir: r.env.WorkingDir, } - mainMod, vendorEnabled, err := gocommand.VendorEnabled(context.TODO(), inv, r.env.GocmdRunner) + vendorEnabled, mainModVendor, err := gocommand.VendorEnabled(context.TODO(), inv, r.env.GocmdRunner) if err != nil { return err } - if mainMod != nil && vendorEnabled { + if mainModVendor != nil && vendorEnabled { // Vendor mode is on, so all the non-Main modules are irrelevant, // and we need to search /vendor for everything. - r.main = mainMod + r.mains = []*gocommand.ModuleJSON{mainModVendor} r.dummyVendorMod = &gocommand.ModuleJSON{ Path: "", - Dir: filepath.Join(mainMod.Dir, "vendor"), + Dir: filepath.Join(mainModVendor.Dir, "vendor"), } - r.modsByModPath = []*gocommand.ModuleJSON{mainMod, r.dummyVendorMod} - r.modsByDir = []*gocommand.ModuleJSON{mainMod, r.dummyVendorMod} + r.modsByModPath = []*gocommand.ModuleJSON{mainModVendor, r.dummyVendorMod} + r.modsByDir = []*gocommand.ModuleJSON{mainModVendor, r.dummyVendorMod} } else { // Vendor mode is off, so run go list -m ... to find everything. err := r.initAllMods() @@ -122,8 +123,10 @@ func (r *ModuleResolver) init() error { r.roots = []gopathwalk.Root{ {filepath.Join(goenv["GOROOT"], "/src"), gopathwalk.RootGOROOT}, } - if r.main != nil { - r.roots = append(r.roots, gopathwalk.Root{r.main.Dir, gopathwalk.RootCurrentModule}) + r.mainByDir = make(map[string]*gocommand.ModuleJSON) + for _, main := range r.mains { + r.roots = append(r.roots, gopathwalk.Root{main.Dir, gopathwalk.RootCurrentModule}) + r.mainByDir[main.Dir] = main } if vendorEnabled { r.roots = append(r.roots, gopathwalk.Root{r.dummyVendorMod.Dir, gopathwalk.RootOther}) @@ -189,7 +192,7 @@ func (r *ModuleResolver) initAllMods() error { r.modsByModPath = append(r.modsByModPath, mod) r.modsByDir = append(r.modsByDir, mod) if mod.Main { - r.main = mod + r.mains = append(r.mains, mod) } } return nil @@ -609,7 +612,7 @@ func (r *ModuleResolver) scanDirForPackage(root gopathwalk.Root, dir string) dir } switch root.Type { case gopathwalk.RootCurrentModule: - importPath = path.Join(r.main.Path, filepath.ToSlash(subdir)) + importPath = path.Join(r.mainByDir[root.Path].Path, filepath.ToSlash(subdir)) case gopathwalk.RootModuleCache: matches := modCacheRegexp.FindStringSubmatch(subdir) if len(matches) == 0 { diff --git a/internal/imports/mod_test.go b/internal/imports/mod_test.go index 51bc9679184..5f71805fa77 100644 --- a/internal/imports/mod_test.go +++ b/internal/imports/mod_test.go @@ -552,6 +552,336 @@ package v mt.assertModuleFoundInDir("example.com/vv", "v", `main/v12$`) } +// Tests that go.work files are respected. +func TestModWorkspace(t *testing.T) { + testenv.NeedsGo1Point(t, 18) + + mt := setup(t, ` +-- go.work -- +go 1.18 + +use ( + ./a + ./b +) +-- a/go.mod -- +module example.com/a + +go 1.18 +-- a/a.go -- +package a +-- b/go.mod -- +module example.com/b + +go 1.18 +-- b/b.go -- +package b +`, "") + defer mt.cleanup() + + mt.assertModuleFoundInDir("example.com/a", "a", `main/a$`) + mt.assertModuleFoundInDir("example.com/b", "b", `main/b$`) + mt.assertScanFinds("example.com/a", "a") + mt.assertScanFinds("example.com/b", "b") +} + +// Tests replaces in workspaces. Uses the directory layout in the cmd/go +// work_replace test. It tests both that replaces in go.work files are +// respected and that a wildcard replace in go.work overrides a versioned replace +// in go.mod. +func TestModWorkspaceReplace(t *testing.T) { + testenv.NeedsGo1Point(t, 18) + + mt := setup(t, ` +-- go.work -- +use m + +replace example.com/dep => ./dep +replace example.com/other => ./other2 + +-- m/go.mod -- +module example.com/m + +require example.com/dep v1.0.0 +require example.com/other v1.0.0 + +replace example.com/other v1.0.0 => ./other +-- m/m.go -- +package m + +import "example.com/dep" +import "example.com/other" + +func F() { + dep.G() + other.H() +} +-- dep/go.mod -- +module example.com/dep +-- dep/dep.go -- +package dep + +func G() { +} +-- other/go.mod -- +module example.com/other +-- other/dep.go -- +package other + +func G() { +} +-- other2/go.mod -- +module example.com/other +-- other2/dep.go -- +package other2 + +func G() { +} +`, "") + defer mt.cleanup() + + mt.assertScanFinds("example.com/m", "m") + mt.assertScanFinds("example.com/dep", "dep") + mt.assertModuleFoundInDir("example.com/other", "other2", "main/other2$") + mt.assertScanFinds("example.com/other", "other2") +} + +// Tests a case where conflicting replaces are overridden by a replace +// in the go.work file. +func TestModWorkspaceReplaceOverride(t *testing.T) { + testenv.NeedsGo1Point(t, 18) + + mt := setup(t, `-- go.work -- +use m +use n +replace example.com/dep => ./dep3 +-- m/go.mod -- +module example.com/m + +require example.com/dep v1.0.0 +replace example.com/dep => ./dep1 +-- m/m.go -- +package m + +import "example.com/dep" + +func F() { + dep.G() +} +-- n/go.mod -- +module example.com/n + +require example.com/dep v1.0.0 +replace example.com/dep => ./dep2 +-- n/n.go -- +package n + +import "example.com/dep" + +func F() { + dep.G() +} +-- dep1/go.mod -- +module example.com/dep +-- dep1/dep.go -- +package dep + +func G() { +} +-- dep2/go.mod -- +module example.com/dep +-- dep2/dep.go -- +package dep + +func G() { +} +-- dep3/go.mod -- +module example.com/dep +-- dep3/dep.go -- +package dep + +func G() { +} +`, "") + + mt.assertScanFinds("example.com/m", "m") + mt.assertScanFinds("example.com/n", "n") + mt.assertScanFinds("example.com/dep", "dep") + mt.assertModuleFoundInDir("example.com/dep", "dep", "main/dep3$") +} + +// Tests that the correct versions of modules are found in +// workspaces with module pruning. This is based on the +// cmd/go mod_prune_all script test. +func TestModWorkspacePrune(t *testing.T) { + testenv.NeedsGo1Point(t, 18) + + mt := setup(t, ` +-- go.work -- +go 1.18 + +use ( + ./a + ./p +) + +replace example.com/b v1.0.0 => ./b +replace example.com/q v1.0.0 => ./q1_0_0 +replace example.com/q v1.0.5 => ./q1_0_5 +replace example.com/q v1.1.0 => ./q1_1_0 +replace example.com/r v1.0.0 => ./r +replace example.com/w v1.0.0 => ./w +replace example.com/x v1.0.0 => ./x +replace example.com/y v1.0.0 => ./y +replace example.com/z v1.0.0 => ./z1_0_0 +replace example.com/z v1.1.0 => ./z1_1_0 + +-- a/go.mod -- +module example.com/a + +go 1.18 + +require example.com/b v1.0.0 +require example.com/z v1.0.0 +-- a/foo.go -- +package main + +import "example.com/b" + +func main() { + b.B() +} +-- b/go.mod -- +module example.com/b + +go 1.18 + +require example.com/q v1.1.0 +-- b/b.go -- +package b + +func B() { +} +-- p/go.mod -- +module example.com/p + +go 1.18 + +require example.com/q v1.0.0 + +replace example.com/q v1.0.0 => ../q1_0_0 +replace example.com/q v1.1.0 => ../q1_1_0 +-- p/main.go -- +package main + +import "example.com/q" + +func main() { + q.PrintVersion() +} +-- q1_0_0/go.mod -- +module example.com/q + +go 1.18 +-- q1_0_0/q.go -- +package q + +import "fmt" + +func PrintVersion() { + fmt.Println("version 1.0.0") +} +-- q1_0_5/go.mod -- +module example.com/q + +go 1.18 + +require example.com/r v1.0.0 +-- q1_0_5/q.go -- +package q + +import _ "example.com/r" +-- q1_1_0/go.mod -- +module example.com/q + +require example.com/w v1.0.0 +require example.com/z v1.1.0 + +go 1.18 +-- q1_1_0/q.go -- +package q + +import _ "example.com/w" +import _ "example.com/z" + +import "fmt" + +func PrintVersion() { + fmt.Println("version 1.1.0") +} +-- r/go.mod -- +module example.com/r + +go 1.18 + +require example.com/r v1.0.0 +-- r/r.go -- +package r +-- w/go.mod -- +module example.com/w + +go 1.18 + +require example.com/x v1.0.0 +-- w/w.go -- +package w +-- w/w_test.go -- +package w + +import _ "example.com/x" +-- x/go.mod -- +module example.com/x + +go 1.18 +-- x/x.go -- +package x +-- x/x_test.go -- +package x +import _ "example.com/y" +-- y/go.mod -- +module example.com/y + +go 1.18 +-- y/y.go -- +package y +-- z1_0_0/go.mod -- +module example.com/z + +go 1.18 + +require example.com/q v1.0.5 +-- z1_0_0/z.go -- +package z + +import _ "example.com/q" +-- z1_1_0/go.mod -- +module example.com/z + +go 1.18 +-- z1_1_0/z.go -- +package z +`, "") + + mt.assertScanFinds("example.com/w", "w") + mt.assertScanFinds("example.com/q", "q") + mt.assertScanFinds("example.com/x", "x") + mt.assertScanFinds("example.com/z", "z") + mt.assertModuleFoundInDir("example.com/w", "w", "main/w$") + mt.assertModuleFoundInDir("example.com/q", "q", "main/q1_1_0$") + mt.assertModuleFoundInDir("example.com/x", "x", "main/x$") + mt.assertModuleFoundInDir("example.com/z", "z", "main/z1_1_0$") +} + // Tests that we handle GO111MODULE=on with no go.mod file. See #30855. func TestNoMainModule(t *testing.T) { testenv.NeedsGo1Point(t, 12) From 4f21f7a508454d25e8d2e6b60122151a511d3899 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Tue, 22 Feb 2022 18:14:18 +0000 Subject: [PATCH 05/15] gopls: update gofumpt to v0.3.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also add a TODO inside the format.Options expression, as we are not currently wiring up some information from the main go.mod. The TODO comment contains a summary of what should be done. Change-Id: Ibace6c36de4b729b5ba8b347f6f48f0a8fda695e Reviewed-on: https://go-review.googlesource.com/c/tools/+/387374 Trust: Daniel Martí Reviewed-by: Robert Findley Trust: Robert Findley Run-TryBot: Robert Findley gopls-CI: kokoro TryBot-Result: Gopher Robot --- gopls/go.mod | 6 +++--- gopls/go.sum | 14 ++++++-------- gopls/internal/hooks/hooks.go | 9 ++++++++- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/gopls/go.mod b/gopls/go.mod index 7d482def710..17d077de76e 100644 --- a/gopls/go.mod +++ b/gopls/go.mod @@ -8,10 +8,10 @@ require ( github.com/jba/templatecheck v0.6.0 github.com/sergi/go-diff v1.1.0 golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3 - golang.org/x/sys v0.0.0-20220114195835-da31bd327af9 - golang.org/x/tools v0.1.8 + golang.org/x/sys v0.0.0-20220209214540-3681064d5158 + golang.org/x/tools v0.1.9 honnef.co/go/tools v0.2.2 - mvdan.cc/gofumpt v0.2.1 + mvdan.cc/gofumpt v0.3.0 mvdan.cc/xurls/v2 v2.3.0 ) diff --git a/gopls/go.sum b/gopls/go.sum index 1240d0734ce..e6c10dd1614 100644 --- a/gopls/go.sum +++ b/gopls/go.sum @@ -5,9 +5,8 @@ github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ3 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/frankban/quicktest v1.14.0 h1:+cqqvzZV87b4adx/5ayVOaYZ2CrvM4ejQvUdBzPPUss= -github.com/frankban/quicktest v1.14.0/go.mod h1:NeW+ay9A/U67EYXNFA1nPE8e/tnQv/09mUdL/ijj8og= -github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/frankban/quicktest v1.14.2 h1:SPb1KFFmM+ybpEjPUhCCkZOM5xlovT5UbrMvWnXyBns= +github.com/frankban/quicktest v1.14.2/go.mod h1:mgiwOwqx65TmIk1wJ6Q7wvnVMocbUorkibMOrVTHZps= github.com/google/go-cmp v0.5.7 h1:81/ik6ipDQS2aGcBfIN5dHDB36BwrStyeAQquSYCV4o= github.com/google/go-cmp v0.5.7/go.mod h1:n+brtR0CgQNWTVd5ZUFpTBC8YFBDLK/h/bpaJ8/DtOE= github.com/google/safehtml v0.0.2 h1:ZOt2VXg4x24bW0m2jtzAOkhoXV0iM8vNKc0paByCZqM= @@ -53,9 +52,8 @@ golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20211019181941-9d821ace8654/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20211213223007-03aa0b5f6827/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220114195835-da31bd327af9 h1:XfKQ4OlFl8okEOr5UvAqFRVj8pY/4yfcXrddB8qAbU0= -golang.org/x/sys v0.0.0-20220114195835-da31bd327af9/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220209214540-3681064d5158 h1:rm+CHSpPEEW2IsXUib1ThaHIjuBVZjxNgSKmBLFfD4c= +golang.org/x/sys v0.0.0-20220209214540-3681064d5158/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= @@ -75,7 +73,7 @@ gopkg.in/yaml.v2 v2.2.4 h1:/eiJrUcujPVeJ3xlSWaiNi3uSVmDGBK1pDHUHAnao1I= gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= honnef.co/go/tools v0.2.2 h1:MNh1AVMyVX23VUHE2O27jm6lNj3vjO5DexS4A1xvnzk= honnef.co/go/tools v0.2.2/go.mod h1:lPVVZ2BS5TfnjLyizF7o7hv7j9/L+8cZY2hLyjP9cGY= -mvdan.cc/gofumpt v0.2.1 h1:7jakRGkQcLAJdT+C8Bwc9d0BANkVPSkHZkzNv07pJAs= -mvdan.cc/gofumpt v0.2.1/go.mod h1:a/rvZPhsNaedOJBzqRD9omnwVwHZsBdJirXHa9Gh9Ig= +mvdan.cc/gofumpt v0.3.0 h1:kTojdZo9AcEYbQYhGuLf/zszYthRdhDNDUi2JKTxas4= +mvdan.cc/gofumpt v0.3.0/go.mod h1:0+VyGZWleeIj5oostkOex+nDBA0eyavuDnDusAJ8ylo= mvdan.cc/xurls/v2 v2.3.0 h1:59Olnbt67UKpxF1EwVBopJvkSUBmgtb468E4GVWIZ1I= mvdan.cc/xurls/v2 v2.3.0/go.mod h1:AjuTy7gEiUArFMjgBBDU4SMxlfUYsRokpJQgNWOt3e4= diff --git a/gopls/internal/hooks/hooks.go b/gopls/internal/hooks/hooks.go index 390967d5f6d..3ca21e50399 100644 --- a/gopls/internal/hooks/hooks.go +++ b/gopls/internal/hooks/hooks.go @@ -23,7 +23,14 @@ func Options(options *source.Options) { } options.URLRegexp = relaxedFullWord options.GofumptFormat = func(ctx context.Context, src []byte) ([]byte, error) { - return format.Source(src, format.Options{}) + return format.Source(src, format.Options{ + // TODO: fill LangVersion and ModulePath from the current go.mod. + // The information is availabe as loaded by go/packages via the + // Module type, but it needs to be wired up all the way here. + // We likely want to change the GofumptFormat field type to take + // extra parameters, to then be used in format.Options. + // See https://pkg.go.dev/mvdan.cc/gofumpt@v0.3.0/format#Options. + }) } updateAnalyzers(options) } From b7d29496b73463700839e8f064c1be5202017284 Mon Sep 17 00:00:00 2001 From: Matheus Alcantara Date: Fri, 18 Feb 2022 20:34:48 +0000 Subject: [PATCH 06/15] internal/lsp: don't store diagnostics for builtin.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes golang/go#44866 Change-Id: I9606df932c1b35d91816306fbcb48edc5b023b30 GitHub-Last-Rev: e6f525ed46a08ac77eac3af295c2be684405e88b GitHub-Pull-Request: golang/tools#364 Reviewed-on: https://go-review.googlesource.com/c/tools/+/386219 Trust: Robert Findley Run-TryBot: Robert Findley Trust: Hyang-Ah Hana Kim gopls-CI: kokoro Trust: Nooras Saba‎ TryBot-Result: Gopher Robot Reviewed-by: Hyang-Ah Hana Kim Reviewed-by: Robert Findley --- internal/lsp/diagnostics.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index f4b00640245..e352e4b46bd 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -288,7 +288,11 @@ func (s *Server) diagnosePkg(ctx context.Context, snapshot source.Snapshot, pkg return } for _, cgf := range pkg.CompiledGoFiles() { - s.storeDiagnostics(snapshot, cgf.URI, typeCheckSource, pkgDiagnostics[cgf.URI]) + // builtin.go exists only for documentation purposes, and is not valid Go code. + // Don't report distracting errors + if !snapshot.IsBuiltin(ctx, cgf.URI) { + s.storeDiagnostics(snapshot, cgf.URI, typeCheckSource, pkgDiagnostics[cgf.URI]) + } } if includeAnalysis && !pkg.HasListOrParseErrors() { reports, err := source.Analyze(ctx, snapshot, pkg, false) From 258e47306680682e73d2b873b69fe7e616ae5490 Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Tue, 22 Feb 2022 10:07:30 -0500 Subject: [PATCH 07/15] internal/lsp/source: disable the useany analyzer by default This analyzer was written at a time when any was only allowed in constraints, and usage of any vs interface{} was more likely to be confusing. Any is now allowed anywhere, and so it is inconsistent to suggest it only for constraints. However, we can't suggest any over interface{} everywhere, as that would be incredibly noisy. Perhaps we should remove this analyzer, but for now simply change it to off by default. Change-Id: Ib9726bdb835808d69827c6cd8e4a58dc5d83ad0e Reviewed-on: https://go-review.googlesource.com/c/tools/+/387614 Trust: Robert Findley Run-TryBot: Robert Findley gopls-CI: kokoro TryBot-Result: Gopher Robot Reviewed-by: Hyang-Ah Hana Kim --- gopls/doc/analyzers.md | 2 +- internal/lsp/source/api_json.go | 7 +++---- internal/lsp/source/options.go | 6 +++--- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md index 121fa4964d9..07f846db840 100644 --- a/gopls/doc/analyzers.md +++ b/gopls/doc/analyzers.md @@ -574,7 +574,7 @@ Another example is about non-pointer receiver: check for constraints that could be simplified to "any" -**Enabled by default.** +**Disabled by default. Enable it by setting `"analyses": {"useany": true}`.** ## **fillreturns** diff --git a/internal/lsp/source/api_json.go b/internal/lsp/source/api_json.go index 4742fb11f4d..9af5bd64dd6 100755 --- a/internal/lsp/source/api_json.go +++ b/internal/lsp/source/api_json.go @@ -406,7 +406,7 @@ var GeneratedAPIJSON = &APIJSON{ { Name: "\"useany\"", Doc: "check for constraints that could be simplified to \"any\"", - Default: "true", + Default: "false", }, { Name: "\"fillreturns\"", @@ -922,9 +922,8 @@ var GeneratedAPIJSON = &APIJSON{ Doc: "checks for unused writes\n\nThe analyzer reports instances of writes to struct fields and\narrays that are never read. Specifically, when a struct object\nor an array is copied, its elements are copied implicitly by\nthe compiler, and any element write to this copy does nothing\nwith the original object.\n\nFor example:\n\n\ttype T struct { x int }\n\tfunc f(input []T) {\n\t\tfor i, v := range input { // v is a copy\n\t\t\tv.x = i // unused write to field x\n\t\t}\n\t}\n\nAnother example is about non-pointer receiver:\n\n\ttype T struct { x int }\n\tfunc (t T) f() { // t is a copy\n\t\tt.x = i // unused write to field x\n\t}\n", }, { - Name: "useany", - Doc: "check for constraints that could be simplified to \"any\"", - Default: true, + Name: "useany", + Doc: "check for constraints that could be simplified to \"any\"", }, { Name: "fillreturns", diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go index 139ae7004d2..c7c6228ad2c 100644 --- a/internal/lsp/source/options.go +++ b/internal/lsp/source/options.go @@ -694,8 +694,8 @@ func (o *Options) Clone() *Options { ClientOptions: o.ClientOptions, InternalOptions: o.InternalOptions, Hooks: Hooks{ - GoDiff: o.Hooks.GoDiff, - ComputeEdits: o.Hooks.ComputeEdits, + GoDiff: o.GoDiff, + ComputeEdits: o.ComputeEdits, GofumptFormat: o.GofumptFormat, URLRegexp: o.URLRegexp, }, @@ -1265,7 +1265,7 @@ func defaultAnalyzers() map[string]*Analyzer { testinggoroutine.Analyzer.Name: {Analyzer: testinggoroutine.Analyzer, Enabled: true}, unusedparams.Analyzer.Name: {Analyzer: unusedparams.Analyzer, Enabled: false}, unusedwrite.Analyzer.Name: {Analyzer: unusedwrite.Analyzer, Enabled: false}, - useany.Analyzer.Name: {Analyzer: useany.Analyzer, Enabled: true}, + useany.Analyzer.Name: {Analyzer: useany.Analyzer, Enabled: false}, infertypeargs.Analyzer.Name: {Analyzer: infertypeargs.Analyzer, Enabled: true}, // gofmt -s suite: From e6ef7709393d2d16834adea8451b9c164d8ee4ed Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Wed, 23 Feb 2022 15:37:02 -0500 Subject: [PATCH 08/15] go/types/typeutil: don't recurse into constraints when hashing tparams When hashing signature type parameters, we recursed into their constraints in an attempt to improve the accuracy of the hash (signatures are considered identical modulo type parameter renaming). This introduces a mechanism for infinite recursion via recursive constraints, as reported in golang/go#51314. To fix this, we can hash just the type parameter index and rely on types.Identical for de-duplicating map entries. Fixes golang/go#51314 Change-Id: Ifd7596d978b98293391bbe64f72f1da2b2a5d51c Reviewed-on: https://go-review.googlesource.com/c/tools/+/387754 Trust: Robert Findley Run-TryBot: Robert Findley Reviewed-by: Tim King gopls-CI: kokoro TryBot-Result: Gopher Robot --- go/types/typeutil/map.go | 10 ++++++---- go/types/typeutil/map_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/go/types/typeutil/map.go b/go/types/typeutil/map.go index 490ee904a62..c9f8f25a0d8 100644 --- a/go/types/typeutil/map.go +++ b/go/types/typeutil/map.go @@ -379,7 +379,7 @@ func (h Hasher) hashFor(t types.Type) uint32 { func (h Hasher) hashTuple(tuple *types.Tuple) uint32 { // See go/types.identicalTypes for rationale. n := tuple.Len() - var hash uint32 = 9137 + 2*uint32(n) + hash := 9137 + 2*uint32(n) for i := 0; i < n; i++ { hash += 3 * h.Hash(tuple.At(i).Type()) } @@ -398,7 +398,7 @@ func (h Hasher) hashUnion(t *typeparams.Union) uint32 { } func (h Hasher) hashTermSet(terms []*typeparams.Term) uint32 { - var hash uint32 = 9157 + 2*uint32(len(terms)) + hash := 9157 + 2*uint32(len(terms)) for _, term := range terms { // term order is not significant. termHash := h.Hash(term.Type()) @@ -416,14 +416,16 @@ func (h Hasher) hashTermSet(terms []*typeparams.Term) uint32 { // If h.sigTParams is set and contains t, then we are in the process of hashing // a signature, and the hash value of t must depend only on t's index and // constraint: signatures are considered identical modulo type parameter -// renaming. +// renaming. To avoid infinite recursion, we only hash the type parameter +// index, and rely on types.Identical to handle signatures where constraints +// are not identical. // // Otherwise the hash of t depends only on t's pointer identity. func (h Hasher) hashTypeParam(t *typeparams.TypeParam) uint32 { if h.sigTParams != nil { i := t.Index() if i >= 0 && i < h.sigTParams.Len() && t == h.sigTParams.At(i) { - return 9173 + 2*h.Hash(t.Constraint()) + 3*uint32(i) + return 9173 + 3*uint32(i) } } return h.hashPtr(t.Obj()) diff --git a/go/types/typeutil/map_test.go b/go/types/typeutil/map_test.go index 17f87edfd56..8cd643e5b48 100644 --- a/go/types/typeutil/map_test.go +++ b/go/types/typeutil/map_test.go @@ -233,6 +233,17 @@ var ME2 = G2[int].M // ME1Type should have identical type as ME1. var ME1Type func(G1[int], G1[int], G2[int]) + +// Examples from issue #51314 +type Constraint[T any] interface{} +func Foo[T Constraint[T]]() {} +func Fn[T1 ~*T2, T2 ~*T1](t1 T1, t2 T2) {} + +// Bar and Baz are identical to Foo. +func Bar[P Constraint[P]]() {} +func Baz[Q any]() {} // The underlying type of Constraint[P] is any. +// But Quux is not. +func Quux[Q interface{ quux() }]() {} ` fset := token.NewFileSet() @@ -284,6 +295,13 @@ var ME1Type func(G1[int], G1[int], G2[int]) ME1 = scope.Lookup("ME1").Type() ME1Type = scope.Lookup("ME1Type").Type() ME2 = scope.Lookup("ME2").Type() + + Constraint = scope.Lookup("Constraint").Type() + Foo = scope.Lookup("Foo").Type() + Fn = scope.Lookup("Fn").Type() + Bar = scope.Lookup("Foo").Type() + Baz = scope.Lookup("Foo").Type() + Quux = scope.Lookup("Quux").Type() ) tmap := new(typeutil.Map) @@ -345,6 +363,14 @@ var ME1Type func(G1[int], G1[int], G2[int]) {ME1, "ME1", true}, {ME1Type, "ME1Type", false}, {ME2, "ME2", true}, + + // See golang/go#51314: avoid infinite recursion on cyclic type constraints. + {Constraint, "Constraint", true}, + {Foo, "Foo", true}, + {Fn, "Fn", true}, + {Bar, "Bar", false}, + {Baz, "Baz", false}, + {Quux, "Quux", true}, } for _, step := range steps { From 5210e0ca159383b30b65329a9e96e03da956a2c7 Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Wed, 23 Feb 2022 11:34:11 -0500 Subject: [PATCH 09/15] gopls: wire in LangVersion and ModulePath for gofumpt formatting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If available, pass the LangVersion and ModulePath provided by go/packages to gofumpt. This allows gofumpt to apply additional formatting rules that require this information. Also add a regression test for gofumpt formatting. Fixes golang/go#51327 Change-Id: I47c8c96d595d62e1c444285ce69ce6a4e61fa74c Reviewed-on: https://go-review.googlesource.com/c/tools/+/387634 Trust: Robert Findley Run-TryBot: Robert Findley Trust: Daniel Martí gopls-CI: kokoro Reviewed-by: Hyang-Ah Hana Kim TryBot-Result: Gopher Robot --- gopls/internal/hooks/hooks.go | 10 +-- .../internal/regtest/misc/formatting_test.go | 66 +++++++++++++++++++ internal/lsp/cache/metadata.go | 5 ++ internal/lsp/cache/snapshot.go | 6 +- internal/lsp/source/format.go | 20 +++++- internal/lsp/source/options.go | 15 +++-- internal/lsp/source/view.go | 4 ++ 7 files changed, 111 insertions(+), 15 deletions(-) diff --git a/gopls/internal/hooks/hooks.go b/gopls/internal/hooks/hooks.go index 3ca21e50399..487fb608215 100644 --- a/gopls/internal/hooks/hooks.go +++ b/gopls/internal/hooks/hooks.go @@ -22,14 +22,10 @@ func Options(options *source.Options) { options.ComputeEdits = ComputeEdits } options.URLRegexp = relaxedFullWord - options.GofumptFormat = func(ctx context.Context, src []byte) ([]byte, error) { + options.GofumptFormat = func(ctx context.Context, langVersion, modulePath string, src []byte) ([]byte, error) { return format.Source(src, format.Options{ - // TODO: fill LangVersion and ModulePath from the current go.mod. - // The information is availabe as loaded by go/packages via the - // Module type, but it needs to be wired up all the way here. - // We likely want to change the GofumptFormat field type to take - // extra parameters, to then be used in format.Options. - // See https://pkg.go.dev/mvdan.cc/gofumpt@v0.3.0/format#Options. + LangVersion: langVersion, + ModulePath: modulePath, }) } updateAnalyzers(options) diff --git a/gopls/internal/regtest/misc/formatting_test.go b/gopls/internal/regtest/misc/formatting_test.go index 67e79391341..75d8f622458 100644 --- a/gopls/internal/regtest/misc/formatting_test.go +++ b/gopls/internal/regtest/misc/formatting_test.go @@ -301,3 +301,69 @@ func main() { } }) } + +func TestGofumptFormatting(t *testing.T) { + + // Exercise some gofumpt formatting rules: + // - No empty lines following an assignment operator + // - Octal integer literals should use the 0o prefix on modules using Go + // 1.13 and later. Requires LangVersion to be correctly resolved. + // - std imports must be in a separate group at the top. Requires ModulePath + // to be correctly resolved. + const input = ` +-- go.mod -- +module foo + +go 1.17 +-- foo.go -- +package foo + +import ( + "foo/bar" + "fmt" +) + +const perm = 0755 + +func foo() { + foo := + "bar" + fmt.Println(foo, bar.Bar) +} +-- foo.go.formatted -- +package foo + +import ( + "fmt" + + "foo/bar" +) + +const perm = 0o755 + +func foo() { + foo := "bar" + fmt.Println(foo, bar.Bar) +} +-- bar/bar.go -- +package bar + +const Bar = 42 +` + + WithOptions( + EditorConfig{ + Settings: map[string]interface{}{ + "gofumpt": true, + }, + }, + ).Run(t, input, func(t *testing.T, env *Env) { + env.OpenFile("foo.go") + env.FormatBuffer("foo.go") + got := env.Editor.BufferText("foo.go") + want := env.ReadWorkspaceFile("foo.go.formatted") + if got != want { + t.Errorf("unexpected formatting result:\n%s", tests.Diff(t, want, got)) + } + }) +} diff --git a/internal/lsp/cache/metadata.go b/internal/lsp/cache/metadata.go index bef7bf8e708..618578dd896 100644 --- a/internal/lsp/cache/metadata.go +++ b/internal/lsp/cache/metadata.go @@ -56,6 +56,11 @@ func (m *Metadata) PackagePath() string { return string(m.PkgPath) } +// ModuleInfo implements the source.Metadata interface. +func (m *Metadata) ModuleInfo() *packages.Module { + return m.Module +} + // KnownMetadata is a wrapper around metadata that tracks its validity. type KnownMetadata struct { *Metadata diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index b9cd36c7458..e786c02afb2 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -1025,7 +1025,11 @@ func (s *snapshot) MetadataForFile(ctx context.Context, uri span.URI) ([]source. var mds []source.Metadata for _, id := range knownIDs { md := s.getMetadata(id) - mds = append(mds, md) + // TODO(rfindley): knownIDs and metadata should be in sync, but existing + // code is defensive of nil metadata. + if md != nil { + mds = append(mds, md) + } } return mds, nil } diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go index 4f02e4f34eb..79da0b3adbf 100644 --- a/internal/lsp/source/format.go +++ b/internal/lsp/source/format.go @@ -63,8 +63,24 @@ func Format(ctx context.Context, snapshot Snapshot, fh FileHandle) ([]protocol.T // Apply additional formatting, if any is supported. Currently, the only // supported additional formatter is gofumpt. - if format := snapshot.View().Options().Hooks.GofumptFormat; snapshot.View().Options().Gofumpt && format != nil { - b, err := format(ctx, buf.Bytes()) + if format := snapshot.View().Options().GofumptFormat; snapshot.View().Options().Gofumpt && format != nil { + // gofumpt can customize formatting based on language version and module + // path, if available. + // + // Try to derive this information, but fall-back on the default behavior. + // + // TODO: under which circumstances can we fail to find module information? + // Can this, for example, result in inconsistent formatting across saves, + // due to pending calls to packages.Load? + var langVersion, modulePath string + mds, err := snapshot.MetadataForFile(ctx, fh.URI()) + if err == nil && len(mds) > 0 { + if mi := mds[0].ModuleInfo(); mi != nil { + langVersion = mi.GoVersion + modulePath = mi.Path + } + } + b, err := format(ctx, langVersion, modulePath, buf.Bytes()) if err != nil { return nil, err } diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go index c7c6228ad2c..b1df2905950 100644 --- a/internal/lsp/source/options.go +++ b/internal/lsp/source/options.go @@ -462,11 +462,16 @@ func (u *UserOptions) SetEnvSlice(env []string) { // Hooks contains configuration that is provided to the Gopls command by the // main package. type Hooks struct { - LicensesText string - GoDiff bool - ComputeEdits diff.ComputeEdits - URLRegexp *regexp.Regexp - GofumptFormat func(ctx context.Context, src []byte) ([]byte, error) + LicensesText string + GoDiff bool + ComputeEdits diff.ComputeEdits + URLRegexp *regexp.Regexp + + // GofumptFormat allows the gopls module to wire-in a call to + // gofumpt/format.Source. langVersion and modulePath are used for some + // Gofumpt formatting rules -- see the Gofumpt documentation for details. + GofumptFormat func(ctx context.Context, langVersion, modulePath string, src []byte) ([]byte, error) + DefaultAnalyzers map[string]*Analyzer TypeErrorAnalyzers map[string]*Analyzer ConvenienceAnalyzers map[string]*Analyzer diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index dba45f76a4a..9e9b0357530 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -18,6 +18,7 @@ import ( "golang.org/x/mod/modfile" "golang.org/x/mod/module" "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/packages" "golang.org/x/tools/internal/gocommand" "golang.org/x/tools/internal/imports" "golang.org/x/tools/internal/lsp/progress" @@ -319,6 +320,9 @@ type Metadata interface { // PackagePath is the package path. PackagePath() string + + // ModuleInfo returns the go/packages module information for the given package. + ModuleInfo() *packages.Module } // Session represents a single connection from a client. From b7525f439611adb23d8864047ccdbad0cfef3e5e Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Thu, 24 Feb 2022 10:09:43 -0700 Subject: [PATCH 10/15] internal/lsp: hash go version into package key The go directive affects type checking errors so it should be included in the package key. I manually tested that this fixes the issue in golang/go#51325 Fixes golang/go#51325 Change-Id: I109859ae65e7e4d5fdefc63a0a7a838117ee02cf Reviewed-on: https://go-review.googlesource.com/c/tools/+/387914 Trust: Suzy Mueller Run-TryBot: Suzy Mueller TryBot-Result: Gopher Robot Reviewed-by: Robert Findley --- internal/lsp/cache/check.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go index 616381653a9..f1668635406 100644 --- a/internal/lsp/cache/check.go +++ b/internal/lsp/cache/check.go @@ -194,7 +194,7 @@ func (s *snapshot) buildKey(ctx context.Context, id PackageID, mode source.Parse depKeys = append(depKeys, depHandle.key) } experimentalKey := s.View().Options().ExperimentalPackageCacheKey - ph.key = checkPackageKey(ph.m.ID, compiledGoFiles, m.Config, depKeys, mode, experimentalKey) + ph.key = checkPackageKey(ph.m.ID, compiledGoFiles, m, depKeys, mode, experimentalKey) return ph, deps, nil } @@ -214,15 +214,18 @@ func (s *snapshot) workspaceParseMode(id PackageID) source.ParseMode { return source.ParseExported } -func checkPackageKey(id PackageID, pghs []*parseGoHandle, cfg *packages.Config, deps []packageHandleKey, mode source.ParseMode, experimentalKey bool) packageHandleKey { +func checkPackageKey(id PackageID, pghs []*parseGoHandle, m *KnownMetadata, deps []packageHandleKey, mode source.ParseMode, experimentalKey bool) packageHandleKey { b := bytes.NewBuffer(nil) b.WriteString(string(id)) + if m.Module != nil { + b.WriteString(m.Module.GoVersion) // go version affects type check errors. + } if !experimentalKey { // cfg was used to produce the other hashed inputs (package ID, parsed Go // files, and deps). It should not otherwise affect the inputs to the type // checker, so this experiment omits it. This should increase cache hits on // the daemon as cfg contains the environment and working directory. - b.WriteString(hashConfig(cfg)) + b.WriteString(hashConfig(m.Config)) } b.WriteByte(byte(mode)) for _, dep := range deps { From 9ffa3ad372b3b5bef2ef1824b61ae7836c856bb0 Mon Sep 17 00:00:00 2001 From: pjw Date: Tue, 15 Feb 2022 12:52:23 -0500 Subject: [PATCH 11/15] internal/lsp: Provide completions for test function definitions In test files, function definitions starting with Test, Bench, or Fuzz can be completed almost automatically. For the snippets the user hits tab, completes the name, hits tab again, and the function is defined, except (of course) for its body. Otherwise a completion that fills in the signature is proposed. Where appropriate, 'TestMain(m *testing.M)' is also offered as a completion. Fixes golang/go#46896 and golang/go#51089 Change-Id: I46c05af0ead79c1d82ca40b2c605045e06e1a35d Reviewed-on: https://go-review.googlesource.com/c/tools/+/385974 Run-TryBot: Peter Weinberger Trust: Peter Weinberger TryBot-Result: Gopher Robot gopls-CI: kokoro Reviewed-by: Hyang-Ah Hana Kim --- .../regtest/completion/completion_test.go | 55 +++++++- internal/lsp/source/completion/completion.go | 7 + internal/lsp/source/completion/definition.go | 127 ++++++++++++++++++ 3 files changed, 188 insertions(+), 1 deletion(-) create mode 100644 internal/lsp/source/completion/definition.go diff --git a/gopls/internal/regtest/completion/completion_test.go b/gopls/internal/regtest/completion/completion_test.go index 988d5c354d0..6abcd60b29d 100644 --- a/gopls/internal/regtest/completion/completion_test.go +++ b/gopls/internal/regtest/completion/completion_test.go @@ -256,7 +256,7 @@ func compareCompletionResults(want []string, gotItems []protocol.CompletionItem) for i, v := range got { if v != want[i] { - return fmt.Sprintf("completion results are not the same: got %v, want %v", got, want) + return fmt.Sprintf("%d completion result not the same: got %q, want %q", i, v, want[i]) } } @@ -546,3 +546,56 @@ func main() { } }) } + +func TestDefinition(t *testing.T) { + stuff := ` +-- go.mod -- +module mod.com + +go 1.18 +-- a_test.go -- +package foo +func T() +func TestG() +func TestM() +func TestMi() +func Ben() +func Fuz() +func Testx() +func TestMe(t *testing.T) +func BenchmarkFoo() +` + // All those parentheses are needed for the completion code to see + // later lines as being definitions + tests := []struct { + pat string + want []string + }{ + {"T", []string{"TestXxx(t *testing.T)", "TestMain(m *testing.M)"}}, + {"TestM", []string{"TestMain(m *testing.M)", "TestM(t *testing.T)"}}, + {"TestMi", []string{"TestMi(t *testing.T)"}}, + {"TestG", []string{"TestG(t *testing.T)"}}, + {"B", []string{"BenchmarkXxx(b *testing.B)"}}, + {"BenchmarkFoo", []string{"BenchmarkFoo(b *testing.B)"}}, + {"F", []string{"FuzzXxx(f *testing.F)"}}, + {"Testx", nil}, + {"TestMe", []string{"TestMe"}}, + } + fname := "a_test.go" + Run(t, stuff, func(t *testing.T, env *Env) { + env.OpenFile(fname) + env.Await(env.DoneWithOpen()) + for _, tst := range tests { + pos := env.RegexpSearch(fname, tst.pat) + pos.Column += len(tst.pat) + completions := env.Completion(fname, pos) + result := compareCompletionResults(tst.want, completions.Items) + if result != "" { + t.Errorf("%s failed: %s:%q", tst.pat, result, tst.want) + for i, it := range completions.Items { + t.Errorf("%d got %q %q", i, it.Label, it.Detail) + } + } + } + }) +} diff --git a/internal/lsp/source/completion/completion.go b/internal/lsp/source/completion/completion.go index 94389c74cb4..30d277f41db 100644 --- a/internal/lsp/source/completion/completion.go +++ b/internal/lsp/source/completion/completion.go @@ -485,6 +485,13 @@ func Completion(ctx context.Context, snapshot source.Snapshot, fh source.FileHan qual := types.RelativeTo(pkg.GetTypes()) objStr = types.ObjectString(obj, qual) } + ans, sel := definition(path, obj, snapshot.FileSet(), pgf.Mapper, fh) + if ans != nil { + sort.Slice(ans, func(i, j int) bool { + return ans[i].Score > ans[j].Score + }) + return ans, sel, nil + } return nil, nil, ErrIsDefinition{objStr: objStr} } } diff --git a/internal/lsp/source/completion/definition.go b/internal/lsp/source/completion/definition.go new file mode 100644 index 00000000000..17b251cb06a --- /dev/null +++ b/internal/lsp/source/completion/definition.go @@ -0,0 +1,127 @@ +// Copyright 2022 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package completion + +import ( + "go/ast" + "go/token" + "go/types" + "strings" + "unicode" + "unicode/utf8" + + "golang.org/x/tools/internal/lsp/protocol" + "golang.org/x/tools/internal/lsp/snippet" + "golang.org/x/tools/internal/lsp/source" +) + +// some definitions can be completed +// So far, TestFoo(t *testing.T), TestMain(m *testing.M) +// BenchmarkFoo(b *testing.B), FuzzFoo(f *testing.F) + +// path[0] is known to be *ast.Ident +func definition(path []ast.Node, obj types.Object, fset *token.FileSet, mapper *protocol.ColumnMapper, fh source.FileHandle) ([]CompletionItem, *Selection) { + if _, ok := obj.(*types.Func); !ok { + return nil, nil // not a function at all + } + if !strings.HasSuffix(fh.URI().Filename(), "_test.go") { + return nil, nil + } + + name := path[0].(*ast.Ident).Name + if len(name) == 0 { + // can't happen + return nil, nil + } + pos := path[0].Pos() + sel := &Selection{ + content: "", + cursor: pos, + MappedRange: source.NewMappedRange(fset, mapper, pos, pos), + } + var ans []CompletionItem + + // Always suggest TestMain, if possible + if strings.HasPrefix("TestMain", name) { + ans = []CompletionItem{defItem("TestMain(m *testing.M)", obj)} + } + + // If a snippet is possible, suggest it + if strings.HasPrefix("Test", name) { + ans = append(ans, defSnippet("Test", "Xxx", "(t *testing.T)", obj)) + return ans, sel + } else if strings.HasPrefix("Benchmark", name) { + ans = append(ans, defSnippet("Benchmark", "Xxx", "(b *testing.B)", obj)) + return ans, sel + } else if strings.HasPrefix("Fuzz", name) { + ans = append(ans, defSnippet("Fuzz", "Xxx", "(f *testing.F)", obj)) + return ans, sel + } + + // Fill in the argument for what the user has already typed + if got := defMatches(name, "Test", path, "(t *testing.T)"); got != "" { + ans = append(ans, defItem(got, obj)) + } else if got := defMatches(name, "Benchmark", path, "(b *testing.B)"); got != "" { + ans = append(ans, defItem(got, obj)) + } else if got := defMatches(name, "Fuzz", path, "(f *testing.F)"); got != "" { + ans = append(ans, defItem(got, obj)) + } + return ans, sel +} + +func defMatches(name, pat string, path []ast.Node, arg string) string { + idx := strings.Index(name, pat) + if idx < 0 { + return "" + } + c, _ := utf8.DecodeRuneInString(name[len(pat):]) + if unicode.IsLower(c) { + return "" + } + fd, ok := path[1].(*ast.FuncDecl) + if !ok { + // we don't know what's going on + return "" + } + fp := fd.Type.Params + if fp != nil && len(fp.List) > 0 { + // signature already there, minimal suggestion + return name + } + // suggesting signature too + return name + arg +} + +func defSnippet(prefix, placeholder, suffix string, obj types.Object) CompletionItem { + var sn snippet.Builder + sn.WriteText(prefix) + if placeholder != "" { + sn.WritePlaceholder(func(b *snippet.Builder) { b.WriteText(placeholder) }) + } + sn.WriteText(suffix + " {\n") + sn.WriteFinalTabstop() + sn.WriteText("\n}") + return CompletionItem{ + Label: prefix + placeholder + suffix, + Detail: "tab, type the rest of the name, then tab", + Kind: protocol.FunctionCompletion, + Depth: 0, + Score: 10, + snippet: &sn, + Documentation: prefix + " test function", + obj: obj, + } +} +func defItem(val string, obj types.Object) CompletionItem { + return CompletionItem{ + Label: val, + InsertText: val, + Kind: protocol.FunctionCompletion, + Depth: 0, + Score: 9, // prefer the snippets when available + Documentation: "complete the parameter", + obj: obj, + } +} From acdddf6756bbc8a7a0d43fb7d20507438425d7d0 Mon Sep 17 00:00:00 2001 From: Tim King Date: Fri, 25 Feb 2022 12:23:22 -0800 Subject: [PATCH 12/15] go/ssa: allows right operand of a shift to be signed. Removes the forced conversion of the right operand of a shift to an unsigned type. This allows for clients to correctly model the runtime panic when a signed shift count is negative. Fixes golang/go#51363 Change-Id: If59000eeb503fd45cdc6d4143dcc249242e7a957 Reviewed-on: https://go-review.googlesource.com/c/tools/+/387995 Trust: Tim King Run-TryBot: Tim King gopls-CI: kokoro Reviewed-by: Zvonimir Pavlinovic Reviewed-by: Dominik Honnef Trust: Dominik Honnef TryBot-Result: Gopher Robot --- go/ssa/emit.go | 13 ++++++++--- go/ssa/interp/interp_test.go | 1 + go/ssa/interp/ops.go | 32 ++++++++++++++++++++++++-- go/ssa/interp/testdata/convert.go | 38 +++++++++++++++++++++++++++++++ 4 files changed, 79 insertions(+), 5 deletions(-) create mode 100644 go/ssa/interp/testdata/convert.go diff --git a/go/ssa/emit.go b/go/ssa/emit.go index 7c8cfdc6614..ff53705fa5b 100644 --- a/go/ssa/emit.go +++ b/go/ssa/emit.go @@ -74,9 +74,16 @@ func emitArith(f *Function, op token.Token, x, y Value, t types.Type, pos token. case token.SHL, token.SHR: x = emitConv(f, x, t) // y may be signed or an 'untyped' constant. - // TODO(adonovan): whence signed values? - if b, ok := y.Type().Underlying().(*types.Basic); ok && b.Info()&types.IsUnsigned == 0 { - y = emitConv(f, y, types.Typ[types.Uint64]) + + // There is a runtime panic if y is signed and <0. Instead of inserting a check for y<0 + // and converting to an unsigned value (like the compiler) leave y as is. + + if b, ok := y.Type().Underlying().(*types.Basic); ok && b.Info()&types.IsUntyped != 0 { + // Untyped conversion: + // Spec https://go.dev/ref/spec#Operators: + // The right operand in a shift expression must have integer type or be an untyped constant + // representable by a value of type uint. + y = emitConv(f, y, types.Typ[types.Uint]) } case token.ADD, token.SUB, token.MUL, token.QUO, token.REM, token.AND, token.OR, token.XOR, token.AND_NOT: diff --git a/go/ssa/interp/interp_test.go b/go/ssa/interp/interp_test.go index 28ebf5f8054..1b43742c8a0 100644 --- a/go/ssa/interp/interp_test.go +++ b/go/ssa/interp/interp_test.go @@ -109,6 +109,7 @@ var gorootTestTests = []string{ var testdataTests = []string{ "boundmeth.go", "complit.go", + "convert.go", "coverage.go", "defer.go", "fieldprom.go", diff --git a/go/ssa/interp/ops.go b/go/ssa/interp/ops.go index 6af7847c039..3bc6a4e32b6 100644 --- a/go/ssa/interp/ops.go +++ b/go/ssa/interp/ops.go @@ -137,6 +137,26 @@ func asUint64(x value) uint64 { panic(fmt.Sprintf("cannot convert %T to uint64", x)) } +// asUnsigned returns the value of x, which must be an integer type, as its equivalent unsigned type, +// and returns true if x is non-negative. +func asUnsigned(x value) (value, bool) { + switch x := x.(type) { + case int: + return uint(x), x >= 0 + case int8: + return uint8(x), x >= 0 + case int16: + return uint16(x), x >= 0 + case int32: + return uint32(x), x >= 0 + case int64: + return uint64(x), x >= 0 + case uint, uint8, uint32, uint64, uintptr: + return x, true + } + panic(fmt.Sprintf("cannot convert %T to unsigned", x)) +} + // zero returns a new "zero" value of the specified type. func zero(t types.Type) value { switch t := t.(type) { @@ -576,7 +596,11 @@ func binop(op token.Token, t types.Type, x, y value) value { } case token.SHL: - y := asUint64(y) + u, ok := asUnsigned(y) + if !ok { + panic("negative shift amount") + } + y := asUint64(u) switch x.(type) { case int: return x.(int) << y @@ -603,7 +627,11 @@ func binop(op token.Token, t types.Type, x, y value) value { } case token.SHR: - y := asUint64(y) + u, ok := asUnsigned(y) + if !ok { + panic("negative shift amount") + } + y := asUint64(u) switch x.(type) { case int: return x.(int) >> y diff --git a/go/ssa/interp/testdata/convert.go b/go/ssa/interp/testdata/convert.go new file mode 100644 index 00000000000..0dcf13bdda6 --- /dev/null +++ b/go/ssa/interp/testdata/convert.go @@ -0,0 +1,38 @@ +// Copyright 2022 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Test conversion operations. + +package main + +func left(x int) { _ = 1 << x } +func right(x int) { _ = 1 >> x } + +func main() { + wantPanic( + func() { + left(-1) + }, + "runtime error: negative shift amount", + ) + wantPanic( + func() { + right(-1) + }, + "runtime error: negative shift amount", + ) +} + +func wantPanic(fn func(), s string) { + defer func() { + err := recover() + if err == nil { + panic("expected panic") + } + if got := err.(error).Error(); got != s { + panic("expected panic " + s + " got " + got) + } + }() + fn() +} From 0e44f7a8a70ad9ca9fda1a2a65cf454379a787f6 Mon Sep 17 00:00:00 2001 From: Hana Date: Fri, 25 Feb 2022 14:44:58 -0500 Subject: [PATCH 13/15] gopls/doc/advanced.md: correct commands for unstable version build `go get golang.org/x/tools/gopls@master ...` resulted in warning messages like: go get: installing executables with 'go get' in module mode is deprecated. To adjust and download dependencies of the current module, use 'go get -d'. To install using requirements of the current module, use 'go install'. To install ignoring the current module, use 'go install' with a version, like 'go install example.com/cmd@latest'. For more information, see https://golang.org/doc/go-get-install-deprecation or run 'go help get' or 'go help install'. Change-Id: I1299983604c27e333f02af875826b5dd4d170068 Reviewed-on: https://go-review.googlesource.com/c/tools/+/388074 Trust: Hyang-Ah Hana Kim Reviewed-by: Robert Findley --- gopls/doc/advanced.md | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/gopls/doc/advanced.md b/gopls/doc/advanced.md index dfea673b9f8..c4e9eabef13 100644 --- a/gopls/doc/advanced.md +++ b/gopls/doc/advanced.md @@ -16,7 +16,8 @@ Where `vX.Y.Z` is the desired version. ### Unstable versions -To update `gopls` to the latest **unstable** version, use: +To update `gopls` to the latest **unstable** version, use the following +commands. ```sh # Create an empty go.mod file, only for tracking requirements. @@ -24,11 +25,8 @@ cd $(mktemp -d) go mod init gopls-unstable # Use 'go get' to add requirements and to ensure they work together. -go get golang.org/x/tools/gopls@master golang.org/x/tools@master +go get -d golang.org/x/tools/gopls@master golang.org/x/tools@master -# For go1.17 or older, the above `go get` command will build and -# install `gopls`. For go1.18+ or tip, run the following command to install -# using selected versions in go.mod. go install golang.org/x/tools/gopls ``` From c2ddf3dda23a11a1895fb258dbec5609319f6520 Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Fri, 18 Feb 2022 16:05:08 -0700 Subject: [PATCH 14/15] internal/lsp: add quick fix for unsupported feature Adds a command to run go mod edit -go to allow users to easily upgrade their go directive. Doing this change also revealed that changing the go directive does not invalidate the type check data and there may be stale diagnostics for a package. Updates golang/go#51086 Change-Id: I659a216059c489a88e29cd51b944c3a0274f3700 Reviewed-on: https://go-review.googlesource.com/c/tools/+/386875 Trust: Suzy Mueller Run-TryBot: Suzy Mueller Reviewed-by: Robert Findley gopls-CI: kokoro TryBot-Result: Gopher Robot --- gopls/doc/commands.md | 16 ++++++++++++++++ internal/lsp/cache/errors.go | 25 +++++++++++++++++++++++++ internal/lsp/command.go | 14 ++++++++++++++ internal/lsp/command/command_gen.go | 20 ++++++++++++++++++++ internal/lsp/command/interface.go | 12 ++++++++++++ internal/lsp/source/api_json.go | 6 ++++++ 6 files changed, 93 insertions(+) diff --git a/gopls/doc/commands.md b/gopls/doc/commands.md index b70b0afec86..37a4ca4108f 100644 --- a/gopls/doc/commands.md +++ b/gopls/doc/commands.md @@ -84,6 +84,22 @@ Args: } ``` +### **Run go mod edit -go=version** +Identifier: `gopls.edit_go_directive` + +Runs `go mod edit -go=version` for a module. + +Args: + +``` +{ + // Any document URI within the relevant module. + "URI": string, + // The version to pass to `go mod edit -go`. + "Version": string, +} +``` + ### **Toggle gc_details** Identifier: `gopls.gc_details` diff --git a/internal/lsp/cache/errors.go b/internal/lsp/cache/errors.go index 67909389f90..155b7a40bcc 100644 --- a/internal/lsp/cache/errors.go +++ b/internal/lsp/cache/errors.go @@ -101,6 +101,7 @@ func parseErrorDiagnostics(snapshot *snapshot, pkg *pkg, errList scanner.ErrorLi } var importErrorRe = regexp.MustCompile(`could not import ([^\s]+)`) +var unsupportedFeatureRe = regexp.MustCompile(`.*require go(\d+\.\d+) or later`) func typeErrorDiagnostics(snapshot *snapshot, pkg *pkg, e extendedError) ([]*source.Diagnostic, error) { code, spn, err := typeErrorData(snapshot.FileSet(), pkg, e.primary) @@ -145,6 +146,14 @@ func typeErrorDiagnostics(snapshot *snapshot, pkg *pkg, e extendedError) ([]*sou return nil, err } } + if code == typesinternal.UnsupportedFeature { + if match := unsupportedFeatureRe.FindStringSubmatch(e.primary.Msg); match != nil { + diag.SuggestedFixes, err = editGoDirectiveQuickFix(snapshot, spn.URI(), match[1]) + if err != nil { + return nil, err + } + } + } return []*source.Diagnostic{diag}, nil } @@ -165,6 +174,22 @@ func goGetQuickFixes(snapshot *snapshot, uri span.URI, pkg string) ([]source.Sug return []source.SuggestedFix{source.SuggestedFixFromCommand(cmd, protocol.QuickFix)}, nil } +func editGoDirectiveQuickFix(snapshot *snapshot, uri span.URI, version string) ([]source.SuggestedFix, error) { + // Go mod edit only supports module mode. + if snapshot.workspaceMode()&moduleMode == 0 { + return nil, nil + } + title := fmt.Sprintf("go mod edit -go=%s", version) + cmd, err := command.NewEditGoDirectiveCommand(title, command.EditGoDirectiveArgs{ + URI: protocol.URIFromSpanURI(uri), + Version: version, + }) + if err != nil { + return nil, err + } + return []source.SuggestedFix{source.SuggestedFixFromCommand(cmd, protocol.QuickFix)}, nil +} + func analysisDiagnosticDiagnostics(snapshot *snapshot, pkg *pkg, a *analysis.Analyzer, e *analysis.Diagnostic) ([]*source.Diagnostic, error) { var srcAnalyzer *source.Analyzer // Find the analyzer that generated this diagnostic. diff --git a/internal/lsp/command.go b/internal/lsp/command.go index d8f9d2ce4fb..e9d61d5ce4a 100644 --- a/internal/lsp/command.go +++ b/internal/lsp/command.go @@ -261,6 +261,20 @@ func (c *commandHandler) Vendor(ctx context.Context, args command.URIArg) error }) } +func (c *commandHandler) EditGoDirective(ctx context.Context, args command.EditGoDirectiveArgs) error { + return c.run(ctx, commandConfig{ + requireSave: true, // if go.mod isn't saved it could cause a problem + forURI: args.URI, + }, func(ctx context.Context, deps commandDeps) error { + _, err := deps.snapshot.RunGoCommandDirect(ctx, source.Normal, &gocommand.Invocation{ + Verb: "mod", + Args: []string{"edit", "-go", args.Version}, + WorkingDir: filepath.Dir(args.URI.SpanURI().Filename()), + }) + return err + }) +} + func (c *commandHandler) RemoveDependency(ctx context.Context, args command.RemoveDependencyArgs) error { return c.run(ctx, commandConfig{ progress: "Removing dependency", diff --git a/internal/lsp/command/command_gen.go b/internal/lsp/command/command_gen.go index c814bfe58c2..55696936bac 100644 --- a/internal/lsp/command/command_gen.go +++ b/internal/lsp/command/command_gen.go @@ -23,6 +23,7 @@ const ( AddImport Command = "add_import" ApplyFix Command = "apply_fix" CheckUpgrades Command = "check_upgrades" + EditGoDirective Command = "edit_go_directive" GCDetails Command = "gc_details" Generate Command = "generate" GenerateGoplsMod Command = "generate_gopls_mod" @@ -46,6 +47,7 @@ var Commands = []Command{ AddImport, ApplyFix, CheckUpgrades, + EditGoDirective, GCDetails, Generate, GenerateGoplsMod, @@ -90,6 +92,12 @@ func Dispatch(ctx context.Context, params *protocol.ExecuteCommandParams, s Inte return nil, err } return nil, s.CheckUpgrades(ctx, a0) + case "gopls.edit_go_directive": + var a0 EditGoDirectiveArgs + if err := UnmarshalArgs(params.Arguments, &a0); err != nil { + return nil, err + } + return nil, s.EditGoDirective(ctx, a0) case "gopls.gc_details": var a0 protocol.DocumentURI if err := UnmarshalArgs(params.Arguments, &a0); err != nil { @@ -240,6 +248,18 @@ func NewCheckUpgradesCommand(title string, a0 CheckUpgradesArgs) (protocol.Comma }, nil } +func NewEditGoDirectiveCommand(title string, a0 EditGoDirectiveArgs) (protocol.Command, error) { + args, err := MarshalArgs(a0) + if err != nil { + return protocol.Command{}, err + } + return protocol.Command{ + Title: title, + Command: "gopls.edit_go_directive", + Arguments: args, + }, nil +} + func NewGCDetailsCommand(title string, a0 protocol.DocumentURI) (protocol.Command, error) { args, err := MarshalArgs(a0) if err != nil { diff --git a/internal/lsp/command/interface.go b/internal/lsp/command/interface.go index d5f520dd768..6058c72f11f 100644 --- a/internal/lsp/command/interface.go +++ b/internal/lsp/command/interface.go @@ -68,6 +68,11 @@ type Interface interface { // Runs `go mod vendor` for a module. Vendor(context.Context, URIArg) error + // EditGoDirective: Run go mod edit -go=version + // + // Runs `go mod edit -go=version` for a module. + EditGoDirective(context.Context, EditGoDirectiveArgs) error + // UpdateGoSum: Update go.sum // // Updates the go.sum file for a module. @@ -204,6 +209,13 @@ type RemoveDependencyArgs struct { OnlyDiagnostic bool } +type EditGoDirectiveArgs struct { + // Any document URI within the relevant module. + URI protocol.DocumentURI + // The version to pass to `go mod edit -go`. + Version string +} + type GoGetPackageArgs struct { // Any document URI within the relevant module. URI protocol.DocumentURI diff --git a/internal/lsp/source/api_json.go b/internal/lsp/source/api_json.go index 9af5bd64dd6..f37cc80d293 100755 --- a/internal/lsp/source/api_json.go +++ b/internal/lsp/source/api_json.go @@ -603,6 +603,12 @@ var GeneratedAPIJSON = &APIJSON{ Doc: "Checks for module upgrades.", ArgDoc: "{\n\t// The go.mod file URI.\n\t\"URI\": string,\n\t// The modules to check.\n\t\"Modules\": []string,\n}", }, + { + Command: "gopls.edit_go_directive", + Title: "Run go mod edit -go=version", + Doc: "Runs `go mod edit -go=version` for a module.", + ArgDoc: "{\n\t// Any document URI within the relevant module.\n\t\"URI\": string,\n\t// The version to pass to `go mod edit -go`.\n\t\"Version\": string,\n}", + }, { Command: "gopls.gc_details", Title: "Toggle gc_details", From abc106cd59308b39f08ae71c479890ccc3ad3748 Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Mon, 28 Feb 2022 15:45:43 -0500 Subject: [PATCH 15/15] gopls/integration/govim: build gopls using go1.18rc1 Govim tests expect error messages to use 'any', so build gopls using the RC. For golang/go#51056 Change-Id: I0f8cab5d7eea3efbbd5b3f3dd9918e85831c2d50 Reviewed-on: https://go-review.googlesource.com/c/tools/+/388436 Trust: Robert Findley Run-TryBot: Robert Findley gopls-CI: kokoro TryBot-Result: Gopher Robot Reviewed-by: Jamal Carvalho --- gopls/integration/govim/run_local.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gopls/integration/govim/run_local.sh b/gopls/integration/govim/run_local.sh index b7aba5eb9d1..b5c284fa1e1 100755 --- a/gopls/integration/govim/run_local.sh +++ b/gopls/integration/govim/run_local.sh @@ -13,7 +13,7 @@ Usage: $0 [--sudo] [--short] [--version (semver|latest)] Args: --sudo run docker with sudo --short run `go test` with `-short` - --version run on the specific tagged Go version (or latest) rather + --version run on the specific tagged govim version (or latest) rather than the default branch Run govim tests against HEAD using local docker. @@ -71,7 +71,7 @@ trap "rm -f \"${temp_gopls}\"" EXIT ${SUDO_IF_NEEDED}docker run --rm -t \ -v "${tools_dir}:/src/tools" \ -w "/src/tools/gopls" \ - golang:latest \ + golang:rc \ go build -o $(basename ${temp_gopls}) # Build the test harness. Here we are careful to pass in a very limited build