golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.8k stars 17.51k forks source link

x/tools/imports: fill in missing type information in composite literals #9827

Open Sajmani opened 9 years ago

Sajmani commented 9 years ago

Idea from campoy + discussions with crawshaw.

The spec now permits some types to be elided from composite literals: "Within a composite literal of array, slice, or map type T, elements that are themselves composite literals may elide the respective literal type if it is identical to the element type of T. Similarly, elements that are addresses of composite literals may elide the &T when the element type is *T."

Struct literals must still specify the types for each element, since without them the literals are difficult to understand. But these types are fully determined by the outermost type of the literal expression and the field names. So it should be possible to fill them in automatically. Since goimports is already processing the AST and adding missing imports, let's see whether it can also fill in these missing types.

For example:

goimports <<EOF
func main() {
  fmt.Println(image.Rectangle{Min: {X: 2}})
}
EOF
should output:
package main

import (
  "fmt"
  "image"
)

func main() {
  fmt.Println(image.Rectangle{Min: image.Point{X: 2}})
}
neild commented 9 years ago

I know this has been discussed before, but I disagree with this premise: "literals must still specify the types for each element, since without them the literals are difficult to understand."

Literals can be difficult to understand without the types, but I don't believe there is any difference in comprehensibility between

rect := image.Rectangle{Min: image.Point{X: 2}} and rect.Min.X = 2

(Actually, there is a difference--the latter is clearly easier to read.)

We allow the latter; why not the former?

crawshaw commented 9 years ago

@neild language design discussions happen on the golang-nuts mailing list, not the issue tracker. This is a goimports feature request.

bcmills commented 9 years ago

I think neild has the right idea, though. If this problem is worth solving, it seems much better addressed by making type-elision more uniform rather than having goimports add redundant information for non-imports.

crawshaw commented 9 years ago

Again, this does not belong on the issue tracker. Please take this conversation to golang-nuts.

bcmills commented 9 years ago

The question of whether the goimports feature is worth adding certainly does belong on the issue tracker.

crawshaw commented 9 years ago

The language change has been discussed before and rejected because it makes code easier to write in return for making code harder to read. The goimports feature makes code easier to write, and maintains the current easier to read Go.

If you want to claim that the language should change, that should happen on golang-nuts. If you want to claim that goimports should not be modified because the language should be changed, that is a language change and should also be on golang-nuts. If you want to argue that the language should stay the same and this should not be part of goimports at all, that should be made here. It is however, not the argument you made.

bcmills commented 9 years ago

The argument I am making is: If the underlying problem is worth addressing, then goimports is not the right place to address it. I am specifically avoiding the question of whether the underlying problem is worth addressing.

Goimports transforms ambiguous programs into unambiguous programs by guessing at the intended imports. That task must be performed under the supervision of the package author, because only they are in a position to resolve the ambiguity. But there is nothing ambiguous about a composite literal - either it is well-formed or it is not.

crawshaw commented 9 years ago

I want this on my save hook, where goimports is right now. Unless Brad disagrees, I don't see why another tool is necessary.

bradfitz commented 9 years ago

goimports is a tool to turn almost-valid programs into more-valid programs, so I think this is in scope. See also: https://github.com/sqs/goreturns

I actually had a conversation with @sqs about whether we should make goimports pluggable so people could add their own passes. But then do we link them all in? And then does it have a config file so people can enable/disable certain passes? Or is the bar only that non-annoying/usually-mostly-correct ones go in?

This is what I wanted to discuss today in our weekly meeting, but we didn't get to it in time.

I don't know where the line is, but I think this composite literal-fleshing-out thing isn't crossing the line.

I agree with @crawshaw that this is the best of both worlds: it lets the author write a program where they know what they meant, but it transforms it into one that's more readable (with the types included), which is why in the Go 1 release meeting when @rsc had prototyped more type elision, we looked at before & after code samples for a bunch of composite literals and pretty much everybody agreed that the version including more types was more readable, which is why the language requires you to write them in all but some of the most obvious cases. But if the programmer knows what they're doing, a program can make it more explicit.

Sajmani commented 9 years ago

Paraphrasing from a discussion with @rsc --

@robpike has similar concerns.

adg commented 9 years ago

extending goimports beyond imports slippery slope towards fixing all kinds of issues

FWIW, we have a great track record of maintaining traction on slippery slopes.