Swallow type errors if possible

fixtures/invalid/invalidfakes/fake_has_valid_imports.go.txt

@@ -0,0 +1,116 @@
+// Code generated by counterfeiter. DO NOT EDIT.
+package invalidfakes
+
+import (
+	"io"
+	"net/http"
+	"os"
+	"sync"
+
+	"github.com/maxbrunsfeld/counterfeiter/v6/fixtures/invalid"
+)
+
+type FakeHasValidImports struct {
+	DoThingsStub        func(io.Writer, *os.File) *http.Client
+	doThingsMutex       sync.RWMutex
+	doThingsArgsForCall []struct {
+		arg1 io.Writer
+		arg2 *os.File
+	}
+	doThingsReturns struct {
+		result1 *http.Client
+	}
+	doThingsReturnsOnCall map[int]struct {
+		result1 *http.Client
+	}
+	invocations      map[string][][]interface{}
+	invocationsMutex sync.RWMutex
+}
+
+func (fake *FakeHasValidImports) DoThings(arg1 io.Writer, arg2 *os.File) *http.Client {
+	fake.doThingsMutex.Lock()
+	ret, specificReturn := fake.doThingsReturnsOnCall[len(fake.doThingsArgsForCall)]
+	fake.doThingsArgsForCall = append(fake.doThingsArgsForCall, struct {
+		arg1 io.Writer
+		arg2 *os.File
+	}{arg1, arg2})
+	stub := fake.DoThingsStub
+	fakeReturns := fake.doThingsReturns
+	fake.recordInvocation("DoThings", []interface{}{arg1, arg2})
+	fake.doThingsMutex.Unlock()
+	if stub != nil {
+		return stub(arg1, arg2)
+	}
+	if specificReturn {
+		return ret.result1
+	}
+	return fakeReturns.result1
+}
+
+func (fake *FakeHasValidImports) DoThingsCallCount() int {
+	fake.doThingsMutex.RLock()
+	defer fake.doThingsMutex.RUnlock()
+	return len(fake.doThingsArgsForCall)
+}
+
+func (fake *FakeHasValidImports) DoThingsCalls(stub func(io.Writer, *os.File) *http.Client) {
+	fake.doThingsMutex.Lock()
+	defer fake.doThingsMutex.Unlock()
+	fake.DoThingsStub = stub
+}
+
+func (fake *FakeHasValidImports) DoThingsArgsForCall(i int) (io.Writer, *os.File) {
+	fake.doThingsMutex.RLock()
+	defer fake.doThingsMutex.RUnlock()
+	argsForCall := fake.doThingsArgsForCall[i]
+	return argsForCall.arg1, argsForCall.arg2
+}
+
+func (fake *FakeHasValidImports) DoThingsReturns(result1 *http.Client) {
+	fake.doThingsMutex.Lock()
+	defer fake.doThingsMutex.Unlock()
+	fake.DoThingsStub = nil
+	fake.doThingsReturns = struct {
+		result1 *http.Client
+	}{result1}
+}
+
+func (fake *FakeHasValidImports) DoThingsReturnsOnCall(i int, result1 *http.Client) {
+	fake.doThingsMutex.Lock()
+	defer fake.doThingsMutex.Unlock()
+	fake.DoThingsStub = nil
+	if fake.doThingsReturnsOnCall == nil {
+		fake.doThingsReturnsOnCall = make(map[int]struct {
+			result1 *http.Client
+		})
+	}
+	fake.doThingsReturnsOnCall[i] = struct {
+		result1 *http.Client
+	}{result1}
+}
+
+func (fake *FakeHasValidImports) Invocations() map[string][][]interface{} {
+	fake.invocationsMutex.RLock()
+	defer fake.invocationsMutex.RUnlock()
+	fake.doThingsMutex.RLock()
+	defer fake.doThingsMutex.RUnlock()
+	copiedInvocations := map[string][][]interface{}{}
+	for key, value := range fake.invocations {
+		copiedInvocations[key] = value
+	}
+	return copiedInvocations
+}
+
+func (fake *FakeHasValidImports) recordInvocation(key string, args []interface{}) {
+	fake.invocationsMutex.Lock()
+	defer fake.invocationsMutex.Unlock()
+	if fake.invocations == nil {
+		fake.invocations = map[string][][]interface{}{}
+	}
+	if fake.invocations[key] == nil {
+		fake.invocations[key] = [][]interface{}{}
+	}
+	fake.invocations[key] = append(fake.invocations[key], args)
+}
+
+var _ invalid.HasValidImports = new(FakeHasValidImports)

fixtures/invalid/unused_invalid_import.go.txt

@@ -0,0 +1,17 @@
+package invalid
+
+import (
+	"io"
+	"net/http"
+	some_alias "os"
+	nonexistent "this.com/does/not/exist"
+)
+
+//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . HasValidImports
+type HasValidImports interface {
+	DoThings(io.Writer, *some_alias.File) *http.Client
+}
+
+func ProcessHasValidImports(instance HasValidImports) nonexistent.Report {
+	return nonexistent.NewReport().WithHasValidImports(instance)
+}

generator/fake.go

@@ -73,7 +73,10 @@ func NewFake(fakeMode FakeMode, targetName string, packagePath string, fakeName
 	}
 
 	if f.IsInterface() || f.Mode == Package {
-		f.loadMethods()
+		err = f.loadMethods()
+		if err != nil {
+			return nil, err
+		}
 	}
 	if f.IsFunction() {
 		err = f.loadMethodForFunction()

generator/function_loader.go

@@ -15,6 +15,10 @@ func (f *Fake) loadMethodForFunction() error {
 		return errors.New("target does not have an underlying function signature")
 	}
 	f.addTypesForMethod(sig)
-	f.Function = methodForSignature(sig, f.TargetName, f.Imports)
+	method, err := methodForSignature(sig, f.TargetName, f.Imports)
+	if err != nil {
+		return err
+	}
+	f.Function = method
 	return nil
 }

generator/generator_internals_test.go

@@ -305,7 +305,8 @@ func testGenerator(t *testing.T, when spec.G, it spec.S) {
 				it("can load the methods", func() {
 					err := f.findPackage()
 					Expect(err).NotTo(HaveOccurred())
-					f.loadMethods()
+					err = f.loadMethods()
+					Expect(err).NotTo(HaveOccurred())
 					Expect(len(f.Methods)).To(BeNumerically(">=", 51)) // yes, this is crazy because go 1.11 added a function
 					switch runtime.Version()[0:6] {
 					case "go1.15", "go1.14":

generator/interface_loader.go

@@ -19,12 +19,15 @@ func (f *Fake) addTypesForMethod(sig *types.Signature) {
 	}
 }
 
-func methodForSignature(sig *types.Signature, methodName string, imports Imports) Method {
+func methodForSignature(sig *types.Signature, methodName string, imports Imports) (Method, error) {
 	params := []Param{}
 	for i := 0; i < sig.Params().Len(); i++ {
 		param := sig.Params().At(i)
 		isVariadic := i == sig.Params().Len()-1 && sig.Variadic()
 		typ := types.TypeString(param.Type(), imports.AliasForPackage)
+		if isInvalidType(typ) {
+			return Method{}, fmt.Errorf("Unable to resolve method parameter type: %s%s %s", methodName, sig.Params().String(), sig.Results().String())
+		}
 		if isVariadic {
 			typ = "..." + typ[2:] // Change []string to ...string
 		}
@@ -39,17 +42,21 @@ func methodForSignature(sig *types.Signature, methodName string, imports Imports
 	returns := []Return{}
 	for i := 0; i < sig.Results().Len(); i++ {
 		ret := sig.Results().At(i)
+		typ := types.TypeString(ret.Type(), imports.AliasForPackage)
+		if isInvalidType(typ) {
+			return Method{}, fmt.Errorf("Unable to resolve method return type: %s%s %s", methodName, sig.Params().String(), sig.Results().String())
+		}
 		r := Return{
 			Name: fmt.Sprintf("result%v", i+1),
-			Type: types.TypeString(ret.Type(), imports.AliasForPackage),
+			Type: typ,
 		}
 		returns = append(returns, r)
 	}
 	return Method{
 		Name:    methodName,
 		Returns: returns,
 		Params:  params,
-	}
+	}, nil
 }
 
 // interfaceMethodSet identifies the methods that are exported for a given
@@ -81,13 +88,13 @@ func interfaceMethodSet(t types.Type) []*rawMethod {
 	return result
 }
 
-func (f *Fake) loadMethods() {
+func (f *Fake) loadMethods() error {
 	var methods []*rawMethod
 	if f.Mode == Package {
 		methods = packageMethodSet(f.Package)
 	} else {
 		if !f.IsInterface() || f.Target == nil || f.Target.Type() == nil {
-			return
+			return nil
 		}
 		methods = interfaceMethodSet(f.Target.Type())
 	}
@@ -97,7 +104,15 @@ func (f *Fake) loadMethods() {
 	}
 
 	for i := range methods {
-		method := methodForSignature(methods[i].Signature, methods[i].Func.Name(), f.Imports)
+		method, err := methodForSignature(methods[i].Signature, methods[i].Func.Name(), f.Imports)
+		if err != nil {
+			return err
+		}
 		f.Methods = append(f.Methods, method)
 	}
+	return nil
+}
+
+func isInvalidType(typestring string) bool {
+	return strings.Contains(typestring, "invalid type")
 }

generator/loader.go

@@ -39,12 +39,12 @@ func (f *Fake) loadPackages(c Cacher, workingDir string) error {
 		return err
 	}
 	for i := range p {
-		if len(p[i].Errors) > 0 {
-			if i == 0 {
-				err = p[i].Errors[0]
-			}
-			for j := range p[i].Errors {
-				log.Printf("error loading packages: %v", strings.TrimPrefix(fmt.Sprintf("%v", p[i].Errors[j]), "-: "))
+		isTargetPackage := (i == 0)
+		for j := range p[i].Errors {
+			log.Printf("error loading packages: %v", strings.TrimPrefix(fmt.Sprintf("%v", p[i].Errors[j]), "-: "))
+			if isTargetPackage && p[i].Errors[j].Kind != packages.TypeError {
+				err = p[i].Errors[j]
+				break
 			}
 		}
 	}

scripts/ci.sh

@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 
-set -eu
+set -euo pipefail
 
 cd "$(dirname "$0")/.."
 pwd
@@ -46,6 +46,66 @@ echo "Running tests..."
 echo
 GO111MODULE=on go test -race ./...
 
+# run fake generation on transient files
+#
+# Unfortunately, there seems to be no other good way of doing this. Invalid Go files will
+# break external tooling that consumes this repository, so they cannot be stored as Go
+# files in version control. Instead, they are stored as .go.txt files and temporarily
+# converted into .go files in a "sandbox directory", where `go generate` is run and
+# all generated .go files are synced back to the real fixtures directory as .go.txt
+# files for dirtiness checking.
+echo
+echo "Installing temporary fixtures..."
+echo
+
+# move fixtures to a temp directory
+real_fixtures="$(mktemp -d -t fixtures)"
+cp -a fixtures/. "$real_fixtures"
+function restore_fixtures {
+    EXIT_CODE="$?"
+    if [ -d "$real_fixtures" ]; then
+        rm -rf fixtures
+        mv "$real_fixtures" fixtures
+    fi
+    exit "$EXIT_CODE"
+}
+# make sure fixtures are restored if script doesn't succeed
+trap restore_fixtures EXIT
+
+# move .txt.go files to .go files in the sandbox fixtures directory
+rm -rf fixtures/*
+for f in $(find "$real_fixtures" -name "*.go.txt" -exec realpath --relative-to "$real_fixtures" {} \;); do
+    mkdir -p "$(dirname "fixtures/$f")"
+    cp "$real_fixtures/$f" "fixtures/${f%.txt}"
+done
+
+echo
+echo "Generating fakes from temporary fixtures..."
+echo
+GO111MODULE=on go generate ./fixtures/...
+
+echo
+echo "Syncing fakes from temporary fixtures..."
+echo
+# move generate .go files to .txt.go files in the real fixtures directory
+for f in $(find "fixtures" -name "*.go" -exec realpath --relative-to "fixtures" {} \;); do
+    mv "fixtures/${f}" "$real_fixtures/${f}.txt"
+done
+
+# restore the fixtures directory and validate that the generated fake text files match
+# the committed fake text files
+echo
+echo "Validating that temporary fake text files have not changed..."
+echo
+rm -rf fixtures
+mv "$real_fixtures" fixtures
+git diff --exit-code
+if output=$(git status --porcelain) && [ ! -z "$output" ]; then
+  echo "the working copy is not clean; commit or ignore all new .go.txt files"
+  echo "before re-running this script"
+  exit 1
+fi
+
 echo "
  _______  _     _  _______  _______  _______
 |       || | _ | ||       ||       ||       |