Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/tools/go/analysis/passes/fieldalignment: suggested fix producing garbled output #43233

Open
shreyaskarnik opened this issue Dec 16, 2020 · 4 comments
Open

x/tools/go/analysis/passes/fieldalignment: suggested fix producing garbled output #43233

shreyaskarnik opened this issue Dec 16, 2020 · 4 comments
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Refactoring Issues related to refactoring tools Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@shreyaskarnik
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.15.5 darwin/amd64

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/user/Library/Caches/go-build"
GOENV="/Users/user/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/user/work/go/pkg/mod"
GOOS="darwin"
GOPATH="/Users/user/work/go"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.15.5/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.15.5/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/tmp/test-gopls/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/vh/853pkypj3tnd78rgvfhgc9kr0000gn/T/go-build258818279=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

enabled the following analysis after upgrading to gopls 0.6.0
"analyses": {
"fieldalignment": true
}

What did you expect to see?

Following sample code to demonstrate the issue:

package main

import "fmt"

type Settings struct {
	AllowedEvents []*string // allowed events
	BlockedEvents []*string // blocked events
	APIVersion    string    `mapstructure:"api_version"`
	BaseURL       string    `mapstructure:"base_url"`
	AccessToken   string    `mapstructure:"access_token"`
}
func main() {
	s := &Settings{
		APIVersion:    "",
		BaseURL:       "",
		AccessToken:   "",
		AllowedEvents: []*string{},
		BlockedEvents: []*string{},
	}
	fmt.Println(s)
}

applied fieldalignment codelens in vscode

What did you see instead?

type Settings struct {
	APIVersion    string    `mapstructure:"api_version"`
	BaseURL       string    `mapstructure:"base_url"`
	AccessToken   string    `mapstructure:"access_token"`
	AllowedEvents []*string
	BlockedEvents [ // allowed events
	]*string      // blocked events
}

initially I thought that I made a mistake with the comments so I tried the following struct

type Settings struct {
	// test1
	AllowedEvents []*string
	// test2
	BlockedEvents []*string
	APIVersion    string `mapstructure:"api_version"`
	BaseURL       string `mapstructure:"base_url"`
	AccessToken   string `mapstructure:"access_token"`
}

after applying fieldanalysis codelens to above struct it became garbled too

type Settings struct {
	APIVersion    string `mapstructure:"api_version"`
	BaseURL       string `mapstructure:"base_url"`
	AccessToken   string `mapstructure:"access_token"`
	AllowedEvents [ // test1
	]*string
	BlockedEvents [ // test2
	]*string
}
@gopherbot gopherbot added the gopls Issues related to the Go language server, gopls. label Dec 16, 2020
@stamblerre stamblerre changed the title field alignment analysis and codelens producing garbled output with gopls x/tools/go/analysis/passes/fieldalignment: suggested fix producing garbled output Dec 16, 2020
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Dec 16, 2020
@gopherbot gopherbot added this to the Unreleased milestone Dec 16, 2020
@stamblerre
Copy link
Contributor

/cc @leitzler

@gopherbot
Copy link

Change https://golang.org/cl/278872 mentions this issue: go/analysis/passes/fieldalignment: workaround for garbled output

gopherbot pushed a commit to golang/tools that referenced this issue Dec 17, 2020
@stamblerre
go/analysis/passes/fieldalignment: filter comments from suggested fix
5b43ef9 
go/ast has problem with comments from its design, for now, best way is
to get rid of the comments.

See golang/go#20744 for general problem.

Updates golang/go#43233

Change-Id: I7c29dc9d0573f5f894859cd8b24e010a777ebba2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/278872
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
@stamblerre stamblerre removed the gopls Issues related to the Go language server, gopls. label Dec 18, 2020
@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 21, 2020
@leitzler
Copy link
Contributor

@shreyaskarnik could you try to reproduce it with the latest version of gopls? It shouldn't produce garbled output, and CL 278872 should solve that issue. However, due to how AST parsing works right now it is solved by removing comments.

@shreyaskarnik
Copy link
Author

@leitzler Thanks for fixing this, it is working as described.

@adonovan adonovan added Analysis Issues related to static analysis (vet, x/tools/go/analysis) Refactoring Issues related to refactoring tools labels Apr 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Refactoring Issues related to refactoring tools Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
6 participants
@cagedmantis @shreyaskarnik @leitzler @adonovan @stamblerre @gopherbot