From 5b4d4266651c799fcd0de7475fcaf3afd7512a58 Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Tue, 1 Aug 2023 11:12:46 -0400 Subject: [PATCH] gopls/internal/hooks: clean language version before passing to gofumpt Fixes golang/go#61692 Change-Id: I97bd85c063ac1f525fd01c2b1a8b5ffe574e1c66 Reviewed-on: https://go-review.googlesource.com/c/tools/+/514815 TryBot-Result: Gopher Robot gopls-CI: kokoro Reviewed-by: Peter Weinberger Run-TryBot: Robert Findley --- gopls/internal/hooks/gofumpt_118.go | 56 ++++++++++++++++++- gopls/internal/hooks/gofumpt_118_test.go | 53 ++++++++++++++++++ .../internal/regtest/misc/formatting_test.go | 27 +++++++++ 3 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 gopls/internal/hooks/gofumpt_118_test.go diff --git a/gopls/internal/hooks/gofumpt_118.go b/gopls/internal/hooks/gofumpt_118.go index 4eb523261dc..bf0ba41e744 100644 --- a/gopls/internal/hooks/gofumpt_118.go +++ b/gopls/internal/hooks/gofumpt_118.go @@ -9,6 +9,7 @@ package hooks import ( "context" + "fmt" "golang.org/x/tools/gopls/internal/lsp/source" "mvdan.cc/gofumpt/format" @@ -16,9 +17,62 @@ import ( func updateGofumpt(options *source.Options) { options.GofumptFormat = func(ctx context.Context, langVersion, modulePath string, src []byte) ([]byte, error) { + fixedVersion, err := fixLangVersion(langVersion) + if err != nil { + return nil, err + } return format.Source(src, format.Options{ - LangVersion: langVersion, + LangVersion: fixedVersion, ModulePath: modulePath, }) } } + +// fixLangVersion function cleans the input so that gofumpt doesn't panic. It is +// rather permissive, and accepts version strings that aren't technically valid +// in a go.mod file. +// +// More specifically, it looks for an optional 'v' followed by 1-3 +// '.'-separated numbers. The resulting string is stripped of any suffix beyond +// this expected version number pattern. +// +// See also golang/go#61692: gofumpt does not accept the new language versions +// appearing in go.mod files (e.g. go1.21rc3). +func fixLangVersion(input string) (string, error) { + bad := func() (string, error) { + return "", fmt.Errorf("invalid language version syntax %q", input) + } + if input == "" { + return input, nil + } + i := 0 + if input[0] == 'v' { // be flexible about 'v' + i++ + } + // takeDigits consumes ascii numerals 0-9 and reports if at least one was + // consumed. + takeDigits := func() bool { + found := false + for ; i < len(input) && '0' <= input[i] && input[i] <= '9'; i++ { + found = true + } + return found + } + if !takeDigits() { // versions must start with at least one number + return bad() + } + + // Accept optional minor and patch versions. + for n := 0; n < 2; n++ { + if i < len(input) && input[i] == '.' { + // Look for minor/patch version. + i++ + if !takeDigits() { + i-- + break + } + } + } + // Accept any suffix. + return input[:i], nil +} diff --git a/gopls/internal/hooks/gofumpt_118_test.go b/gopls/internal/hooks/gofumpt_118_test.go new file mode 100644 index 00000000000..838ce73176c --- /dev/null +++ b/gopls/internal/hooks/gofumpt_118_test.go @@ -0,0 +1,53 @@ +// 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. + +//go:build go1.18 +// +build go1.18 + +package hooks + +import "testing" + +func TestFixLangVersion(t *testing.T) { + tests := []struct { + input, want string + wantErr bool + }{ + {"", "", false}, + {"1.18", "1.18", false}, + {"v1.18", "v1.18", false}, + {"1.21", "1.21", false}, + {"1.21rc3", "1.21", false}, + {"1.21.0", "1.21.0", false}, + {"1.21.1", "1.21.1", false}, + {"v1.21.1", "v1.21.1", false}, + {"v1.21.0rc1", "v1.21.0", false}, // not technically valid, but we're flexible + {"v1.21.0.0", "v1.21.0", false}, // also technically invalid + {"1.1", "1.1", false}, + {"v1", "v1", false}, + {"1", "1", false}, + {"v1.21.", "v1.21", false}, // also invalid + {"1.21.", "1.21", false}, + + // Error cases. + {"rc1", "", true}, + {"x1.2.3", "", true}, + } + + for _, test := range tests { + got, err := fixLangVersion(test.input) + if test.wantErr { + if err == nil { + t.Errorf("fixLangVersion(%q) succeeded unexpectedly", test.input) + } + continue + } + if err != nil { + t.Fatalf("fixLangVersion(%q) failed: %v", test.input, err) + } + if got != test.want { + t.Errorf("fixLangVersion(%q) = %s, want %s", test.input, got, test.want) + } + } +} diff --git a/gopls/internal/regtest/misc/formatting_test.go b/gopls/internal/regtest/misc/formatting_test.go index ee8098cc93b..1556bb7f918 100644 --- a/gopls/internal/regtest/misc/formatting_test.go +++ b/gopls/internal/regtest/misc/formatting_test.go @@ -366,3 +366,30 @@ const Bar = 42 } }) } + +func TestGofumpt_Issue61692(t *testing.T) { + testenv.NeedsGo1Point(t, 21) + + const input = ` +-- go.mod -- +module foo + +go 1.21rc3 +-- foo.go -- +package foo + +func _() { + foo := + "bar" +} +` + + WithOptions( + Settings{ + "gofumpt": true, + }, + ).Run(t, input, func(t *testing.T, env *Env) { + env.OpenFile("foo.go") + env.FormatBuffer("foo.go") // golang/go#61692: must not panic + }) +}