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/cmd/goimports: support gofmt option to simplify code #21476

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

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

aronatkins opened this issue Aug 16, 2017 · 5 comments
Labels
Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@aronatkins
Copy link 

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
Copy link
Contributor

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).

@bhcleek bhcleek mentioned this issue Oct 24, 2017
Closed
@mbyio
Copy link

mbyio 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
Copy link
Contributor

@mbyio Yes.

@rileykarson rileykarson mentioned this issue Dec 14, 2018
Merged
@jimeh
Copy link

jimeh commented Dec 31, 2018

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

@griesemer
Copy link
Contributor

@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.

@negz negz mentioned this issue Mar 6, 2019
Merged
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Sep 12, 2019
@mvdan mvdan mentioned this issue Oct 31, 2019
Closed
@corneliusweig corneliusweig mentioned this issue Dec 4, 2019
Merged
This was referenced Oct 5, 2020
Merged
Closed
@lesam lesam mentioned this issue Jan 29, 2021
Merged
3 tasks
@gqgs gqgs mentioned this issue Jul 7, 2021
Closed
@ialidzhikov ialidzhikov mentioned this issue Sep 2, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
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
5 participants
@jimeh @aronatkins @mbyio @griesemer @gopherbot