Complain about exported functions/methods returning unexported types.
diff --git a/lint.go b/lint.go
index dc8f807..af7e2e6 100644
--- a/lint.go
+++ b/lint.go
@@ -181,6 +181,7 @@
f.lintIncDec()
f.lintMake()
f.lintErrorReturn()
+ f.lintUnexportedReturn()
}
type link string
@@ -1198,6 +1199,61 @@
})
}
+// lintUnexportedReturn examines exported function declarations.
+// It complains if any return an unexported type.
+func (f *file) lintUnexportedReturn() {
+ f.walk(func(n ast.Node) bool {
+ fn, ok := n.(*ast.FuncDecl)
+ if !ok {
+ return true
+ }
+ if fn.Type.Results == nil {
+ return false
+ }
+ if !fn.Name.IsExported() {
+ return false
+ }
+ thing := "func"
+ if fn.Recv != nil && len(fn.Recv.List) > 0 {
+ thing = "method"
+ if !ast.IsExported(receiverType(fn)) {
+ // Don't report exported methods of unexported types,
+ // such as private implementations of sort.Interface.
+ return false
+ }
+ }
+ for _, ret := range fn.Type.Results.List {
+ typ := f.pkg.typeOf(ret.Type)
+ if exportedType(typ) {
+ continue
+ }
+ f.errorf(ret.Type, 0.8, "exported %s %s returns unexported type %s, which can be annoying to use",
+ thing, fn.Name.Name, typ)
+ break // only flag one
+ }
+ return false
+ })
+}
+
+// exportedType reports whether typ is an exported type.
+// It is imprecise, and will err on the side of returning true,
+// such as for composite types.
+func exportedType(typ types.Type) bool {
+ switch T := typ.(type) {
+ case *types.Named:
+ // Builtin types have no package.
+ return T.Obj().Pkg() == nil || T.Obj().Exported()
+ case *types.Map:
+ return exportedType(T.Key()) && exportedType(T.Elem())
+ case interface {
+ Elem() types.Type
+ }: // array, slice, pointer, chan
+ return exportedType(T.Elem())
+ }
+ // Be conservative about other types, such as struct, interface, etc.
+ return true
+}
+
func receiverType(fn *ast.FuncDecl) string {
switch e := fn.Recv.List[0].Type.(type) {
case *ast.Ident:
diff --git a/lint_test.go b/lint_test.go
index 12e3afb..f0a0acc 100644
--- a/lint_test.go
+++ b/lint_test.go
@@ -9,6 +9,7 @@
import (
"bytes"
"flag"
+ "go/ast"
"go/parser"
"go/printer"
"go/token"
@@ -17,6 +18,8 @@
"regexp"
"strings"
"testing"
+
+ "code.google.com/p/go.tools/go/types"
)
var lintMatch = flag.String("lint.match", "", "restrict testdata matches to this pattern")
@@ -195,3 +198,41 @@
}
}
}
+
+func TestExportedType(t *testing.T) {
+ tests := []struct {
+ typString string
+ exp bool
+ }{
+ {"int", true},
+ {"string", false}, // references the shadowed builtin "string"
+ {"T", true},
+ {"t", false},
+ {"*T", true},
+ {"*t", false},
+ {"map[int]complex128", true},
+ }
+ for _, test := range tests {
+ src := `package foo; type T int; type t int; type string struct{}`
+ fset := token.NewFileSet()
+ file, err := parser.ParseFile(fset, "foo.go", src, 0)
+ if err != nil {
+ t.Fatalf("Parsing %q: %v", src, err)
+ }
+ // use the package name as package path
+ pkg, err := types.Check(file.Name.Name, fset, []*ast.File{file})
+ if err != nil {
+ t.Fatalf("Type checking %q: %v", src, err)
+ }
+ // Use the first child scope of the package, which will be the file scope.
+ scope := pkg.Scope().Child(0)
+ typ, _, err := types.Eval(test.typString, pkg, scope)
+ if err != nil {
+ t.Errorf("types.Eval(%q): %v", test.typString, err)
+ continue
+ }
+ if got := exportedType(typ); got != test.exp {
+ t.Errorf("exportedType(%v) = %t, want %t", typ, got, test.exp)
+ }
+ }
+}
diff --git a/testdata/unexp-return.go b/testdata/unexp-return.go
new file mode 100644
index 0000000..7fc9769
--- /dev/null
+++ b/testdata/unexp-return.go
@@ -0,0 +1,46 @@
+// Test for unexported return types.
+
+// Package foo ...
+package foo
+
+import ()
+
+type hidden struct{}
+
+// Exported returns a hidden type, which is annoying.
+func Exported() hidden { // MATCH /Exported.*unexported.*hidden/
+ return hidden{}
+}
+
+// ExpErr returns a builtin type.
+func ExpErr() error { // ok
+}
+
+func (hidden) ExpOnHidden() hidden { // ok
+}
+
+// T is another test type.
+type T struct{}
+
+// MethodOnT returns a hidden type, which is annoying.
+func (T) MethodOnT() hidden { // MATCH /method MethodOnT.*unexported.*hidden/
+ return hidden{}
+}
+
+// ExpT returns a T.
+func ExpT() T { // ok
+ return T{}
+}
+
+func unexp() hidden { // ok
+ return hidden{}
+}
+
+// This is slightly sneaky: we shadow the builtin "int" type.
+
+type int struct{}
+
+// ExportedIntReturner returns an unexported type from this package.
+func ExportedIntReturner() int { // MATCH /ExportedIntReturner.*unexported.*int/
+ return int{}
+}
