Skip to content

Commit

Permalink
gopls/internal/hooks: clean language version before passing to gofumpt
Browse files Browse the repository at this point in the history
Fixes golang/go#61692

Change-Id: I97bd85c063ac1f525fd01c2b1a8b5ffe574e1c66
Reviewed-on: https://go-review.googlesource.com/c/tools/+/514815
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
  • Loading branch information
findleyr committed Aug 1, 2023
1 parent 2160c5f commit 5b4d426
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 1 deletion.
56 changes: 55 additions & 1 deletion gopls/internal/hooks/gofumpt_118.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,70 @@ package hooks

import (
"context"
"fmt"

"golang.org/x/tools/gopls/internal/lsp/source"
"mvdan.cc/gofumpt/format"
)

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
}
53 changes: 53 additions & 0 deletions gopls/internal/hooks/gofumpt_118_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
27 changes: 27 additions & 0 deletions gopls/internal/regtest/misc/formatting_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
}

0 comments on commit 5b4d426

Please sign in to comment.