Consider reporting avoidable type conversion for untyped constants (typed constants are already handled).

[dmitshur]

Consider the following Go program:

package main

import "fmt"

func main() {
	const foo = "foo"

	f(string(foo) + "bar")
}

func f(s string) {
	fmt.Println(s)
}

I expected unconvert to report that string(foo) is an unnecessary conversion, but it didn't.

$ unconvert
$ echo $?
0

Is that intentional, or a missing feature?

Tested using Go 1.8beta2 and latest version of unconvert and its depenencies:

$ go version
go version go1.8beta2 darwin/amd64
$ gostatus -v .../unconvert
     github.com/mdempsky/unconvert/...
$ go list -f '{{join .Deps "\n"}}' github.com/mdempsky/unconvert | gostatus -stdin -v
     github.com/kisielk/gotool/...
     golang.org/x/text/...
   $ golang.org/x/tools/...
	$ Stash exists
$ binstale unconvert
unconvert
	up to date: github.com/mdempsky/unconvert

/cc @dominikh in case this is a better fit for one of your tools.

[dmitshur]

Not sure if this is the same issue, or another but related one...

unconvert is also missing opportunities to point out unnecessary conversion of nil slices:

package main

import (
	"fmt"
)

func main() {
	var s []string
	fmt.Println(s)
	s = []string(nil)
	fmt.Println(s)
}

The conversion of nil to []string is unnecessary, meaning s = []string(nil) could be simplified to just s = nil.

Using go1.9 and latest unconvert and all of its dependencies. /cc @mdempsky

[mdempsky]

Interesting. So strictly speaking, string("foo") and []string(nil) do not meet the definition of "unnecessary" that unconvert uses: "foo" and nil are an untyped string and untyped nil, respectively, whereas the conversion converts them to string and []string, respectively.

That said, I think there's merit to supporting these cases. Maybe behind a flag.

The second one can probably already be handled by recognizing that s = []string(...) is a "safe" context.

The first case is theoretically just an issue of recognizing another safe context, but it's a bit trickier because of the extra string concatenation.

[dmitshur]

Very good point about the distinction between typed vs untyped constants. I agree that supporting it would still be helpful, but being able to control this behavior via a flag would also make sense.

[dmitshur]

I've confirmed, if I change the const in the original program from untyped string to typed string, by doing this:

-const foo = "foo"
+const foo string = "foo"

Or this:

-const foo = "foo"
+const foo = string("foo")

Then unconvert reports the unnecessary conversion on line 8:

$ unconvert
main.go:8:10: unnecessary conversion

I'll rename this issue to more accurately represent this issue, which is now more of a feature request. It's not that unconvert doesn't handle constants, because it does. It's only about untyped constants.

but it's a bit trickier because of the extra string concatenation.

It seems to handle that part fine. I'm not too surprised, given you're using go/types, which figures this stuff out.