Go: Add Import command allows import for packages that are already imported

[ramya-rao-a]

In the below sample go file, fmt is already imported

package main
import "fmt"

func main() {
     fmt.Println("Hello!")
}

Now run Go: Add Import command from the command palette.
The quick open shows fmt as an option as well.
Choose fmt, and an import to fmt gets added

package main
import "fmt"
import "fmt"

func main() {
     fmt.Println("Hello!")
}

Either, we shouldn't show an already imported package in the quick open or when chosen, show an info/error box saying that the package is already imported

[lukehoban]

The current design of "Add Import" kind of relies on gofmt/goimports to clean up the imports instead of baking in too much fanciness to the Add Import command itself. I'm not sure that's a bad thing. But looks like the changes in #500 may add more of this formatting logic into Add Import.

Broader note - this extension tries as hard as it can to not have to interpret Go code at all, leaving that responsibility to utilities written in Go which can use the official parsers and type checking logic built into the Go runtime. This Add Import feature is one of the places where the extension goes the furthest in trying to parse and interpret Go code. It's already a bit brittle (regexps for parsing!), and I'm worried that making it more intelligent may lead to more additional cases where it breaks in unexpected ways.

[ramya-rao-a]

@lukehoban

Agreed that parsing is best left to utilities written in Go. Lucky for us you already have written one :)
go-outline returns imported packages along with rest of the declarations in a go file.

The need for removing duplicates from the packages list doesn't arise much for the Add Import command. Like you said, formatting would clean it up. Worst case, the duplicate would get marked with red squiggly lines getting the user's attention.

Personally, this ask is more for my work in #437 where I add importable packages in autocomplete suggestions. I make use of the listPackages() method from "goImports.go" file. If a package is already imported then my autocomplete gives 2 suggestions instead of 1. Which is why I needed listPackages() to not return packages that are already imported.

One catch of using go-outline as-is, is that for large go files we unnecessarily will be parsing the whole file. For this, we could update go-outline itself to take in more arguments like 'imports-only', 'package-only' and then make use of ImportsOnly mode of the parser.ParseFile method

[lukehoban]

Got it.

Makes sense to update go-outline with this new feature. I think we would not want to rely on that feature as we can't force existing users to upgrade to a new version of go-outline, so we'll want to make sure the experience is acceptable in largish files even when needing to run the full go-outline. I think it probably will be for the performance that addImport needs.