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

x/tools/cmd/goimports: support gofmt option to simplify code #21476

Open
aronatkins opened this issue Aug 16, 2017 · 5 comments

Comments

@aronatkins
Copy link

commented Aug 16, 2017

go version go1.8.3 darwin/amd64

Using goimports as of golang/tools@84a35ef

The goimports command handles import fixing and gofmt code formatting. It does not, however, implement the code simplification option -s.

This means that goimports cannot be used as a drop-in replacement to save-hooks that use gofmt -s (for example, when using -s as the gofmt-args value for the save-hook with go-mode.el).

package main

import "fmt"

type Thing struct {
	value int
}

func main() {
	things := []Thing{
		Thing{value: 42},
	}
	fmt.Printf("things: %+v\n", things)
}

There are no problems identified by goimports:

$ goimports -d complex.go

With gofmt -s, we get:

$ gofmt -s -d complex.go
diff complex.go gofmt/complex.go
--- /var/folders/ts/s940qvdj5vj1czr9qh07fvtw0000gn/T/gofmt822245016	2017-08-16 12:34:31.000000000 -0400
+++ /var/folders/ts/s940qvdj5vj1czr9qh07fvtw0000gn/T/gofmt235716375	2017-08-16 12:34:31.000000000 -0400
@@ -8,7 +8,7 @@

 func main() {
 	things := []Thing{
-		Thing{value: 42},
+		{value: 42},
 	}
 	fmt.Printf("things: %+v\n", things)
 }

@gopherbot gopherbot added this to the Unreleased milestone Aug 16, 2017

@griesemer

This comment has been minimized.

Copy link
Contributor

commented Aug 17, 2017

FWIW, it's a historical artifact that the simplify (-s) mechanism is implemented as part of the gofmt command. I think AST simplification (and rewriting) should be factored out and provided via separate libraries. Then those libraries could be trivially used from other places (incl. gofmt or goimports if so desired).

@mbyio

This comment has been minimized.

Copy link

commented Jul 25, 2018

@griesemer So basically, to implement this, someone should first factor out the simplification code and then use that to implement simplification in goimports?

@griesemer

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2018

@mbyio Yes.

@jimeh

This comment has been minimized.

Copy link

commented Dec 31, 2018

@griesemer isn't the point of goimports that it's a drop-in replacement for gofmt?

@griesemer

This comment has been minimized.

Copy link
Contributor

commented Jan 7, 2019

@jimeh goimports already does more than gofmt. I'm just suggesting that simplifications such as this should be factored out into a separate library. Maybe that library is used by gofmt and goimports, or just goimports. But the main point is that the code should be isolated into a separate package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.