fix: enforce-repeated-arg-type-style rule finds false positives in case of invalid types

[denisvmedia]

closes #1032

[zak-pawel]

@denisvmedia Thanks! I tested code from this PR and it fixes most of the cases.

However, it still sees a problem for such cases (for op):

func printConfig(name string, p telegraf.PluginDescriber, op string, commented bool, di telegraf.DeprecationInfo, outputBuffer io.Writer) {

But when I change the second argument from telegraf.PluginDescriber to int, the function stops being problematic:

func printConfig(name string, p int, op string, commented bool, di telegraf.DeprecationInfo, outputBuffer io.Writer) {

There is a similar problem with return types.

[denisvmedia]

Yeah, that was my oversight. I will fix it. Thanks!

[denisvmedia]

@zak-pawel does it work for you now?

[zak-pawel]

Now it works very well, thanks!

[chavacava]

Hi @denisvmedia thanks for the PR, it LGTM.
Could you please add some test cases to the rule tests?

[denisvmedia]

@chavacava it's actually non-trivial to test it. We'll have to provide the means to pass fake imports in our test engine. Otherwise, any package we import won't be found and the tests will start failing. I played around with a custom (fake) importer, but I didn't have enough time to simulate the invalid package situation. I can try checking it later. Do you think we can merge this PR now without having the tests at the moment?

[chavacava]

IMO relying on actual type analysis is over-killing for such a simple rule.
I wonder if just comparing the string representation of parameters types is not enough.
A canonical string representation of parameters' types can be obtained by calling gofmt on each *Field.Type of FuncDecl.Type.Params.List.

[denisvmedia]

That's what my TODO comment actually says (maybe I wasn't clear enough in it ;)). But what I wanted is to have a quick fix to make sure that this rule at least doesn't have false positives (it seems to me to be better than having false negatives), and then invest some more time to have a full fix. But if you think it makes no sense to merge it as is, I can revise the PR. I'm just not sure when I will have time to play with it.