code cleanup in parse.go drops all import statements and is potentially buggy in other infrequent scenarios

[ahicks92]

Here is a fun issue:

• I import context, errors, and github.com/go-resty/resty in a fairly complicated source file inside a pre-modules monorepo setup that has to do fun things to GOPATH to work.
• The code in parse.go for cleaning up the output here drops all import statements.
• The invocation of golang.org/x/tools/imports apparently tries to work out what you need to import in order to automatically add it code
• But the import statements are missing, so it has nothing to go off of, and it's only able to work out context and github.com/go-resty/resty, dropping errors and causing code to not compile.

My temporary fix is to fork Genny in order to remove the continue statements used to skip import statements. This can't work on multiple generated types in the same invocation, but I'm generating one type per file for a REST client for our internal microservices, so it lets me make progress.

Having the import statements does fix it.

I think that golang.org/x/tools/imports adding imports back in hid the issue. The first thing i tried to do before I dug in far enough to figure out what is actually going on was make a test case. I couldn't cause this to fail in an isolated setting. I think that mostly it's smart enough to magically work out what it needs to do just enough that dropping all import statements can work in most cases.

I can't share the monorepo or the code, and since I can't get a standalone test case working I'm not sure how to let someone else reproduce it. If necessary, I may be able to get permission to share the specific file being used as input, since it contains no code specific to our setup. Dropping all import statements can be seen by modifying the Generics function in parse.go to return just before invocation of golang.org/x/tools/imports.

Since the way Genny works means that we can't have variation in the import statements themselves based off the type parameters (or, at least as far as I know it doesn't), I think the fix here is to refactor the code cleanup to run per typeset rather than on the entire concatenated output. Flags can then be passed to the first one telling it that it's the first one, so that it knows not to drop package and import statements, then unconditionally drop from all the others.

Furthermore, it looks like there's some other oddities that probably should be handled in the same refactor, as I think they can be solved in a similar way:

• code cleanup isn't robust against whitespace. I'm not sure who would indent their package and import statements, but it's possible and it looks like this code isn't going to recognize them because it doesn't strip lines before checking their prefix.
• It might also not be robust against variables named like packageContainer (i.e. packageContainer := NewPackageContainer()). Currently I think that gets through because such code is frequently indented.
• It looks like the import case is relatively robust as long as it's a block. Most import statements are, but it's possible to just have import "foo" and I don't think this code handles that either.

If refactored to run code cleanup once per typeset, it's guaranteed to be package, imports, code in that order possibly with comments, which should handle the latter points and make it safe to strip whitespace. Specifically, I believe ignoring everything until a non-comment non-package non-import line is encountered and then just including everything as-is after that point will do it.

I'll try to find the time to refactor if I'm headed in the right direction, but it might be better if someone who is familiar with the internals does it, plus I haven't got a lot of time on my hands so it might be a while. If anyone can at least let me know if this all seems right before I put the time in to refactor, I'd appreciate it.

[pdrum]

@camlorn You're right. There are multiple bugs resulting from how imports are being handled. Basically the code currently ignores all imports and then tries to figure out imports using go tools. This results in quite a lot of problems. I'd appreciate if you could take the time to make a PR. I was also planning to fix it a while ago but didn't have the time to do so (still planning to do so :D).

Issues marked with this milestone are also relevant to this issue.