Resolve name shadowing between imports and params

novas0x2a commented 6 years ago

If the interface to be mocked requires an import that is shadowed by a param with the same name, it's unlikely to be a problem for the upstream interface (and thus go undetected), but it causes moq to generate invalid code.

Example in the wild: https://github.com/kubernetes/helm/blob/534193640179fe3c9ffc3012a7bc4e8b23fbe832/pkg/helm/interface.go#L28 (the conflict is between the chart param and the chart import)

matryer commented 6 years ago

What kind of code does moq generate here?

novas0x2a commented 6 years ago

It only changes anything if it detects a conflict with an import name, and even then it only changes the generated local name of the argument, not the interface on the exposed mock (since those are exported, the names don't conflict)

package main

import "context"

type Thing interface {
        WithConflict(context context.Context)
        NoConflict(ctx context.Context)
}

$ ./moq.fbe042a -out before . Thing
$ ./moq.2e9ae34 -out after . Thing
$ diff -u before after


--- before      2018-07-24 12:42:55.253016061 -0700
+++ after       2018-07-24 12:43:06.617187737 -0700
@@ -22,7 +22,7 @@
 //             NoConflictFunc: func(ctx context.Context)  {
 //                    panic("TODO: mock out the NoConflict method")
 //             },
-//             WithConflictFunc: func(context context.Context)  {
+//             WithConflictFunc: func(contextMoqParam context.Context)  {
 //                    panic("TODO: mock out the WithConflict method")
 //             },
 //         }
@@ -36,7 +36,7 @@
        NoConflictFunc func(ctx context.Context)

        // WithConflictFunc mocks the WithConflict method.
-       WithConflictFunc func(context context.Context)
+       WithConflictFunc func(contextMoqParam context.Context)

        // calls tracks calls to the methods.
        calls struct {
@@ -85,19 +85,19 @@
 }

 // WithConflict calls WithConflictFunc.
-func (mock *ThingMock) WithConflict(context context.Context) {
+func (mock *ThingMock) WithConflict(contextMoqParam context.Context) {
        if mock.WithConflictFunc == nil {
                panic("ThingMock.WithConflictFunc: method is nil but Thing.WithConflict was just called")
        }
        callInfo := struct {
                Context context.Context
        }{
-               Context: context,
+               Context: contextMoqParam,
        }
        lockThingMockWithConflict.Lock()
        mock.calls.WithConflict = append(mock.calls.WithConflict, callInfo)
        lockThingMockWithConflict.Unlock()
-       mock.WithConflictFunc(context)
+       mock.WithConflictFunc(contextMoqParam)
 }

 // WithConflictCalls gets all the calls that were made to WithConflict.

novas0x2a commented 6 years ago

The before copy in this example fails to compile because the context arg shadows the context import, so the context.Context in the callInfo struct decl blows up; this code makes sure that in that case, the context arg is deconflicted so there's no shadowing

novas0x2a commented 5 years ago

Finally got back to this again, with a test this time.