Use typesinfo to compare packages

[MichaelUrman]

This avoids a false positive in shadow.go, and enables detecting dot-imported.

Fixes #5

[MichaelUrman]

This ended up being a larger change to the original implementation than I'm comfortable requesting. So please feel free to take either the whole, or any pieces of it that you like.

Main changes:

• use typesinfo, as called out, for pkg comparisons (replaces import tracking)
• detect dotimport EOF = nil
• detect &global, whether selexpr or dotimport
• use inspector Preorder for presumably faster peformance in metalinters
• test ./... in testdata/$subdir ("a", "dotimport", "shadow" didn't work outside of subdir=defaults)

[chokoswitch]

Thanks @MichaelUrman! I think this means the linter will move from the syntax phase to types phase of golanglint which is fine. I'm not sure if &global is always, or even almost always an error - I suppose the idea is that if a global was meant to be used as a pointer, it already would be. But it seems to have high risk of false positive, can we make that opt-in?

[MichaelUrman]

I'm not familiar enough with the phases to comment, but I do agree that this approach requires types. I don't know if the inspector dependency changes anything related to that; I saw it a performance tradeoff that's easy to revert.

As for &global, it's more an idea. It follows the same filtering as the rest of the globals, so it defaults to Err...|EOF, and I can't think of many legitimate reasons to take their addresses. But I agree with your point, especially without further analysis. So I see two options: reject &global (at least this implementation); or accept it with an opt-in flag.

An alternate implementation might leverage SSA. Stores to addresses that come only from globals should be safe to report. Ones that potentially come from elsewhere (including function params) would be easy to skip. So at the cost of certain kinds of false negatives, false positives should be preventable, and the reports would be on the assignment. But introducing SSA processing is a much larger change. Perhaps it would be better to reject &global for now so that SSA could be proven out on a separate PR.

Feel free to choose a direction and ask me to update the PR accordingly. (No guarantees on my turnaround.) Or to update this code yourself or take ideas from it and merge your own version separately from this PR. I won't be offended.

(I can't believe I committed with // want "foo" in there. That was just to prove that shadow.go was included in tests... 🤦)

[chokoswitch]

Thanks @MichaelUrman for the context and discussion. Agree that it sounds like it's good to keep the change to fixing the issue with shadowing for now and explore address-taking in the future. I have gone ahead and pushed the change to this PR

[chokoswitch]

Thanks again @MichaelUrman - will try to get this released and a PR to update golangci within the next few days