From 900159e355b5d28065da154789669f4595aeb08a Mon Sep 17 00:00:00 2001 From: Georgii Kliukovkin Date: Tue, 27 Aug 2024 20:25:45 -0700 Subject: [PATCH 01/11] add interfaces flag with unit test --- mockgen/mockgen.go | 34 ++++++++++++++ mockgen/mockgen_test.go | 101 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+) diff --git a/mockgen/mockgen.go b/mockgen/mockgen.go index 5d57868e..9903a2a1 100644 --- a/mockgen/mockgen.go +++ b/mockgen/mockgen.go @@ -71,6 +71,7 @@ var ( debugParser = flag.Bool("debug_parser", false, "Print out parser results only.") showVersion = flag.Bool("version", false, "Print version.") + interfaces = flag.String("interfaces", "", "List of interfaces to generate mocks for; if empty, mockgen will generate mocks for all interfaces found in the input file(s).") ) func main() { @@ -115,6 +116,14 @@ func main() { return } + if len(*interfaces) > 0 { + ifaces := strings.Split(*interfaces, ",") + if pkg.Interfaces, err = filterInterfaces(pkg.Interfaces, ifaces); err != nil { + log.Fatalf("Filtering interfaces failed: %v", err) + } + + } + outputPackageName := *packageOut if outputPackageName == "" { // pkg.Name in reflect mode is the base name of the import path, @@ -894,3 +903,28 @@ func parsePackageImport(srcDir string) (string, error) { } return "", errOutsideGoPath } + +func filterInterfaces(all []*model.Interface, requested []string) ([]*model.Interface, error) { + if len(requested) == 0 { + return nil, fmt.Errorf("no interfaces requested, other provide them or remove flag -interfaces") + } + requestedIfaces := make(map[string]struct{}) + for _, iface := range requested { + requestedIfaces[iface] = struct{}{} + } + result := make([]*model.Interface, 0, len(all)) + for _, iface := range all { + if _, ok := requestedIfaces[iface.Name]; ok { + result = append(result, iface) + delete(requestedIfaces, iface.Name) + } + } + if len(requestedIfaces) > 0 { + var missing []string + for iface := range requestedIfaces { + missing = append(missing, iface) + } + return nil, fmt.Errorf("missing interfaces: %s", strings.Join(missing, ", ")) + } + return result, nil +} diff --git a/mockgen/mockgen_test.go b/mockgen/mockgen_test.go index 6b171272..50e838e5 100644 --- a/mockgen/mockgen_test.go +++ b/mockgen/mockgen_test.go @@ -467,3 +467,104 @@ func TestParseExcludeInterfaces(t *testing.T) { }) } } + +func Test_filterInterfaces(t *testing.T) { + type args struct { + all []*model.Interface + requested []string + } + tests := []struct { + name string + args args + want []*model.Interface + wantErr bool + }{ + { + name: "no filter", + args: args{ + all: []*model.Interface{ + { + Name: "Foo", + }, + { + Name: "Bar", + }, + }, + requested: []string{}, + }, + want: nil, + wantErr: true, + }, + { + name: "filter by Foo", + args: args{ + all: []*model.Interface{ + { + Name: "Foo", + }, + { + Name: "Bar", + }, + }, + requested: []string{"Foo"}, + }, + want: []*model.Interface{ + { + Name: "Foo", + }, + }, + wantErr: false, + }, + { + name: "filter by Foo and Bar", + args: args{ + all: []*model.Interface{ + { + Name: "Foo", + }, + { + Name: "Bar", + }, + }, + requested: []string{"Foo", "Bar"}, + }, + want: []*model.Interface{ + { + Name: "Foo", + }, + { + Name: "Bar", + }, + }, + wantErr: false, + }, + { + name: "incorrect filter by Foo and Baz", + args: args{ + all: []*model.Interface{ + { + Name: "Foo", + }, + { + Name: "Bar", + }, + }, + requested: []string{"Foo", "Baz"}, + }, + want: nil, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := filterInterfaces(tt.args.all, tt.args.requested) + if (err != nil) != tt.wantErr { + t.Errorf("filterInterfaces() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("filterInterfaces() got = %v, want %v", got, tt.want) + } + }) + } +} From 5a7af371c4541fdce16cef961522669c18b30d46 Mon Sep 17 00:00:00 2001 From: Georgii Kliukovkin Date: Wed, 28 Aug 2024 21:00:15 -0700 Subject: [PATCH 02/11] move reading to a better place --- mockgen/mockgen.go | 7 ------- mockgen/parse.go | 9 +++++++++ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/mockgen/mockgen.go b/mockgen/mockgen.go index 9903a2a1..ec85c479 100644 --- a/mockgen/mockgen.go +++ b/mockgen/mockgen.go @@ -116,13 +116,6 @@ func main() { return } - if len(*interfaces) > 0 { - ifaces := strings.Split(*interfaces, ",") - if pkg.Interfaces, err = filterInterfaces(pkg.Interfaces, ifaces); err != nil { - log.Fatalf("Filtering interfaces failed: %v", err) - } - - } outputPackageName := *packageOut if outputPackageName == "" { diff --git a/mockgen/parse.go b/mockgen/parse.go index f43321c3..1d7f7b8f 100644 --- a/mockgen/parse.go +++ b/mockgen/parse.go @@ -92,6 +92,15 @@ func sourceMode(source string) (*model.Package, error) { for pkgPath := range dotImports { pkg.DotImports = append(pkg.DotImports, pkgPath) } + + if len(*interfaces) > 0 { + ifaces := strings.Split(*interfaces, ",") + if pkg.Interfaces, err = filterInterfaces(pkg.Interfaces, ifaces); err != nil { + log.Fatalf("Filtering interfaces failed: %v", err) + } + + } + return pkg, nil } From 6cc01b16e1017aaea03481a652b64150ad4981a1 Mon Sep 17 00:00:00 2001 From: Georgii Kliukovkin Date: Wed, 28 Aug 2024 21:04:45 -0700 Subject: [PATCH 03/11] Moving to a parse.go --- mockgen/mockgen.go | 26 ---------- mockgen/mockgen_test.go | 101 +------------------------------------- mockgen/parse.go | 27 ++++++++++- mockgen/parse_test.go | 104 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 131 insertions(+), 127 deletions(-) diff --git a/mockgen/mockgen.go b/mockgen/mockgen.go index ec85c479..47216c1a 100644 --- a/mockgen/mockgen.go +++ b/mockgen/mockgen.go @@ -116,7 +116,6 @@ func main() { return } - outputPackageName := *packageOut if outputPackageName == "" { // pkg.Name in reflect mode is the base name of the import path, @@ -896,28 +895,3 @@ func parsePackageImport(srcDir string) (string, error) { } return "", errOutsideGoPath } - -func filterInterfaces(all []*model.Interface, requested []string) ([]*model.Interface, error) { - if len(requested) == 0 { - return nil, fmt.Errorf("no interfaces requested, other provide them or remove flag -interfaces") - } - requestedIfaces := make(map[string]struct{}) - for _, iface := range requested { - requestedIfaces[iface] = struct{}{} - } - result := make([]*model.Interface, 0, len(all)) - for _, iface := range all { - if _, ok := requestedIfaces[iface.Name]; ok { - result = append(result, iface) - delete(requestedIfaces, iface.Name) - } - } - if len(requestedIfaces) > 0 { - var missing []string - for iface := range requestedIfaces { - missing = append(missing, iface) - } - return nil, fmt.Errorf("missing interfaces: %s", strings.Join(missing, ", ")) - } - return result, nil -} diff --git a/mockgen/mockgen_test.go b/mockgen/mockgen_test.go index 50e838e5..00313455 100644 --- a/mockgen/mockgen_test.go +++ b/mockgen/mockgen_test.go @@ -468,103 +468,4 @@ func TestParseExcludeInterfaces(t *testing.T) { } } -func Test_filterInterfaces(t *testing.T) { - type args struct { - all []*model.Interface - requested []string - } - tests := []struct { - name string - args args - want []*model.Interface - wantErr bool - }{ - { - name: "no filter", - args: args{ - all: []*model.Interface{ - { - Name: "Foo", - }, - { - Name: "Bar", - }, - }, - requested: []string{}, - }, - want: nil, - wantErr: true, - }, - { - name: "filter by Foo", - args: args{ - all: []*model.Interface{ - { - Name: "Foo", - }, - { - Name: "Bar", - }, - }, - requested: []string{"Foo"}, - }, - want: []*model.Interface{ - { - Name: "Foo", - }, - }, - wantErr: false, - }, - { - name: "filter by Foo and Bar", - args: args{ - all: []*model.Interface{ - { - Name: "Foo", - }, - { - Name: "Bar", - }, - }, - requested: []string{"Foo", "Bar"}, - }, - want: []*model.Interface{ - { - Name: "Foo", - }, - { - Name: "Bar", - }, - }, - wantErr: false, - }, - { - name: "incorrect filter by Foo and Baz", - args: args{ - all: []*model.Interface{ - { - Name: "Foo", - }, - { - Name: "Bar", - }, - }, - requested: []string{"Foo", "Baz"}, - }, - want: nil, - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := filterInterfaces(tt.args.all, tt.args.requested) - if (err != nil) != tt.wantErr { - t.Errorf("filterInterfaces() error = %v, wantErr %v", err, tt.wantErr) - return - } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("filterInterfaces() got = %v, want %v", got, tt.want) - } - }) - } -} + diff --git a/mockgen/parse.go b/mockgen/parse.go index 1d7f7b8f..3b3143ce 100644 --- a/mockgen/parse.go +++ b/mockgen/parse.go @@ -100,7 +100,7 @@ func sourceMode(source string) (*model.Package, error) { } } - + return pkg, nil } @@ -811,4 +811,29 @@ func packageNameOfDir(srcDir string) (string, error) { return packageImport, nil } +func filterInterfaces(all []*model.Interface, requested []string) ([]*model.Interface, error) { + if len(requested) == 0 { + return nil, fmt.Errorf("no interfaces requested, other provide them or remove flag -interfaces") + } + requestedIfaces := make(map[string]struct{}) + for _, iface := range requested { + requestedIfaces[iface] = struct{}{} + } + result := make([]*model.Interface, 0, len(all)) + for _, iface := range all { + if _, ok := requestedIfaces[iface.Name]; ok { + result = append(result, iface) + delete(requestedIfaces, iface.Name) + } + } + if len(requestedIfaces) > 0 { + var missing []string + for iface := range requestedIfaces { + missing = append(missing, iface) + } + return nil, fmt.Errorf("missing interfaces: %s", strings.Join(missing, ", ")) + } + return result, nil +} + var errOutsideGoPath = errors.New("source directory is outside GOPATH") diff --git a/mockgen/parse_test.go b/mockgen/parse_test.go index 3c4ba4cf..6683a72b 100644 --- a/mockgen/parse_test.go +++ b/mockgen/parse_test.go @@ -4,6 +4,9 @@ import ( "go/parser" "go/token" "testing" + "reflect" + + "go.uber.org/mock/mockgen/model" ) func TestFileParser_ParseFile(t *testing.T) { @@ -143,3 +146,104 @@ func TestParseArrayWithConstLength(t *testing.T) { } } } + +func Test_filterInterfaces(t *testing.T) { + type args struct { + all []*model.Interface + requested []string + } + tests := []struct { + name string + args args + want []*model.Interface + wantErr bool + }{ + { + name: "no filter", + args: args{ + all: []*model.Interface{ + { + Name: "Foo", + }, + { + Name: "Bar", + }, + }, + requested: []string{}, + }, + want: nil, + wantErr: true, + }, + { + name: "filter by Foo", + args: args{ + all: []*model.Interface{ + { + Name: "Foo", + }, + { + Name: "Bar", + }, + }, + requested: []string{"Foo"}, + }, + want: []*model.Interface{ + { + Name: "Foo", + }, + }, + wantErr: false, + }, + { + name: "filter by Foo and Bar", + args: args{ + all: []*model.Interface{ + { + Name: "Foo", + }, + { + Name: "Bar", + }, + }, + requested: []string{"Foo", "Bar"}, + }, + want: []*model.Interface{ + { + Name: "Foo", + }, + { + Name: "Bar", + }, + }, + wantErr: false, + }, + { + name: "incorrect filter by Foo and Baz", + args: args{ + all: []*model.Interface{ + { + Name: "Foo", + }, + { + Name: "Bar", + }, + }, + requested: []string{"Foo", "Baz"}, + }, + want: nil, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := filterInterfaces(tt.args.all, tt.args.requested) + if (err != nil) != tt.wantErr { + t.Errorf("filterInterfaces() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("filterInterfaces() got = %v, want %v", got, tt.want) + } + }) + } +} \ No newline at end of file From 255383515f9d82c7b241093bdebbfaba18fcf8f8 Mon Sep 17 00:00:00 2001 From: Georgii Kliukovkin Date: Wed, 28 Aug 2024 21:05:33 -0700 Subject: [PATCH 04/11] remove spaces --- mockgen/mockgen_test.go | 2 -- mockgen/parse_test.go | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/mockgen/mockgen_test.go b/mockgen/mockgen_test.go index 00313455..6b171272 100644 --- a/mockgen/mockgen_test.go +++ b/mockgen/mockgen_test.go @@ -467,5 +467,3 @@ func TestParseExcludeInterfaces(t *testing.T) { }) } } - - diff --git a/mockgen/parse_test.go b/mockgen/parse_test.go index 6683a72b..ac6494f2 100644 --- a/mockgen/parse_test.go +++ b/mockgen/parse_test.go @@ -5,7 +5,7 @@ import ( "go/token" "testing" "reflect" - + "go.uber.org/mock/mockgen/model" ) @@ -246,4 +246,4 @@ func Test_filterInterfaces(t *testing.T) { } }) } -} \ No newline at end of file +} From ec91f0c2187cf11404bf6f1bed1cce6485cfbb7f Mon Sep 17 00:00:00 2001 From: Georgii Kliukovkin Date: Sun, 13 Oct 2024 18:23:24 -0700 Subject: [PATCH 05/11] update the PR according to the comments: - Instead of using an -interfaces flag, what do you think about having source mode read positional arguments (i.e., mockgen -source InterfaceOne,InterfaceTwo) to align it with how reflect mode works? (If there are no positional arguments specified, we would parse all interfaces to keep backwards compatibility) - Instead of parsing and then dropping interfaces that aren't specified, can we simply not parse ones that aren't requested? This is similar to how the exclusion flag already works and would avoid some wasted computation. --- mockgen/mockgen.go | 1 - mockgen/parse.go | 63 ++++++++++++++++++++++++++----------------- mockgen/parse_test.go | 31 ++++++++++++++++++--- 3 files changed, 65 insertions(+), 30 deletions(-) diff --git a/mockgen/mockgen.go b/mockgen/mockgen.go index 47216c1a..5d57868e 100644 --- a/mockgen/mockgen.go +++ b/mockgen/mockgen.go @@ -71,7 +71,6 @@ var ( debugParser = flag.Bool("debug_parser", false, "Print out parser results only.") showVersion = flag.Bool("version", false, "Print version.") - interfaces = flag.String("interfaces", "", "List of interfaces to generate mocks for; if empty, mockgen will generate mocks for all interfaces found in the input file(s).") ) func main() { diff --git a/mockgen/parse.go b/mockgen/parse.go index 3b3143ce..3429ab05 100644 --- a/mockgen/parse.go +++ b/mockgen/parse.go @@ -18,6 +18,7 @@ package main import ( "errors" + "flag" "fmt" "go/ast" "go/build" @@ -93,12 +94,17 @@ func sourceMode(source string) (*model.Package, error) { pkg.DotImports = append(pkg.DotImports, pkgPath) } - if len(*interfaces) > 0 { - ifaces := strings.Split(*interfaces, ",") + // Get positional arguments after the flags + ifaces := flag.Args() + + // If there are interfaces provided as positional arguments, filter them + if len(ifaces) > 0 { if pkg.Interfaces, err = filterInterfaces(pkg.Interfaces, ifaces); err != nil { log.Fatalf("Filtering interfaces failed: %v", err) } - + } else { + // No interfaces provided, process all interfaces for backward compatibility + log.Printf("No interfaces specified, processing all interfaces") } return pkg, nil @@ -812,28 +818,35 @@ func packageNameOfDir(srcDir string) (string, error) { } func filterInterfaces(all []*model.Interface, requested []string) ([]*model.Interface, error) { - if len(requested) == 0 { - return nil, fmt.Errorf("no interfaces requested, other provide them or remove flag -interfaces") - } - requestedIfaces := make(map[string]struct{}) - for _, iface := range requested { - requestedIfaces[iface] = struct{}{} - } - result := make([]*model.Interface, 0, len(all)) - for _, iface := range all { - if _, ok := requestedIfaces[iface.Name]; ok { - result = append(result, iface) - delete(requestedIfaces, iface.Name) - } - } - if len(requestedIfaces) > 0 { - var missing []string - for iface := range requestedIfaces { - missing = append(missing, iface) - } - return nil, fmt.Errorf("missing interfaces: %s", strings.Join(missing, ", ")) - } - return result, nil + // If no interfaces are requested, return all interfaces + if len(requested) == 0 { + return all, nil + } + + requestedIfaces := make(map[string]struct{}) + for _, iface := range requested { + requestedIfaces[iface] = struct{}{} + } + + result := make([]*model.Interface, 0, len(requestedIfaces)) + for _, iface := range all { + // Only add interfaces that are requested + if _, ok := requestedIfaces[iface.Name]; ok { + result = append(result, iface) + delete(requestedIfaces, iface.Name) // Remove matched iface from requested + } + } + + // If any requested interfaces were not found, return an error + if len(requestedIfaces) > 0 { + var missing []string + for iface := range requestedIfaces { + missing = append(missing, iface) + } + return nil, fmt.Errorf("missing interfaces: %s", strings.Join(missing, ", ")) + } + + return result, nil } var errOutsideGoPath = errors.New("source directory is outside GOPATH") diff --git a/mockgen/parse_test.go b/mockgen/parse_test.go index ac6494f2..2c8da674 100644 --- a/mockgen/parse_test.go +++ b/mockgen/parse_test.go @@ -3,8 +3,8 @@ package main import ( "go/parser" "go/token" - "testing" "reflect" + "testing" "go.uber.org/mock/mockgen/model" ) @@ -159,7 +159,7 @@ func Test_filterInterfaces(t *testing.T) { wantErr bool }{ { - name: "no filter", + name: "no filter (returns all interfaces)", args: args{ all: []*model.Interface{ { @@ -171,8 +171,15 @@ func Test_filterInterfaces(t *testing.T) { }, requested: []string{}, }, - want: nil, - wantErr: true, + want: []*model.Interface{ + { + Name: "Foo", + }, + { + Name: "Bar", + }, + }, + wantErr: false, }, { name: "filter by Foo", @@ -233,6 +240,22 @@ func Test_filterInterfaces(t *testing.T) { want: nil, wantErr: true, }, + { + name: "missing interface (Baz not found)", + args: args{ + all: []*model.Interface{ + { + Name: "Foo", + }, + { + Name: "Bar", + }, + }, + requested: []string{"Baz"}, + }, + want: nil, + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From df7752f6f175ebec7e90165b314e680b019eedbf Mon Sep 17 00:00:00 2001 From: Georgii Kliukovkin Date: Thu, 2 Oct 2025 22:40:20 -0700 Subject: [PATCH 06/11] Refactor interface filtering: inline logic into parseFile, remove filterInterfaces, update tests --- mockgen/parse.go | 84 ++++++++---------- mockgen/parse_test.go | 197 ++++++++++++++++-------------------------- 2 files changed, 113 insertions(+), 168 deletions(-) diff --git a/mockgen/parse.go b/mockgen/parse.go index 3429ab05..393d2584 100644 --- a/mockgen/parse.go +++ b/mockgen/parse.go @@ -30,6 +30,7 @@ import ( "os" "path" "path/filepath" + "sort" "strconv" "strings" @@ -62,6 +63,20 @@ func sourceMode(source string) (*model.Package, error) { srcDir: srcDir, } + // positional interface names -> include set + ifaces := flag.Args() + if len(ifaces) > 0 { + p.includeNamesSet = make(map[string]struct{}, len(ifaces)) + for _, arg := range ifaces { + for _, name := range strings.Split(arg, ",") { + name = strings.TrimSpace(name) + if name != "" { + p.includeNamesSet[name] = struct{}{} + } + } + } + } + // Handle -imports. dotImports := make(map[string]bool) if *imports != "" { @@ -94,19 +109,6 @@ func sourceMode(source string) (*model.Package, error) { pkg.DotImports = append(pkg.DotImports, pkgPath) } - // Get positional arguments after the flags - ifaces := flag.Args() - - // If there are interfaces provided as positional arguments, filter them - if len(ifaces) > 0 { - if pkg.Interfaces, err = filterInterfaces(pkg.Interfaces, ifaces); err != nil { - log.Fatalf("Filtering interfaces failed: %v", err) - } - } else { - // No interfaces provided, process all interfaces for backward compatibility - log.Printf("No interfaces specified, processing all interfaces") - } - return pkg, nil } @@ -183,6 +185,7 @@ type fileParser struct { auxInterfaces *interfaceCache srcDir string excludeNamesSet map[string]struct{} + includeNamesSet map[string]struct{} } func (p *fileParser) errorf(pos token.Pos, format string, args ...any) error { @@ -243,10 +246,19 @@ func (p *fileParser) parseFile(importPath string, file *ast.File) (*model.Packag var is []*model.Interface for ni := range iterInterfaces(file) { - if _, ok := p.excludeNamesSet[ni.name.String()]; ok { + name := ni.name.String() + + if _, ok := p.excludeNamesSet[name]; ok { continue } - i, err := p.parseInterface(ni.name.String(), importPath, ni) + + if len(p.includeNamesSet) > 0 { + if _, ok := p.includeNamesSet[name]; !ok { + continue + } + } + + i, err := p.parseInterface(name, importPath, ni) if errors.Is(err, errConstraintInterface) { continue } @@ -254,6 +266,16 @@ func (p *fileParser) parseFile(importPath string, file *ast.File) (*model.Packag return nil, err } is = append(is, i) + + delete(p.includeNamesSet, name) + } + if len(p.includeNamesSet) > 0 { + missing := make([]string, 0, len(p.includeNamesSet)) + for n := range p.includeNamesSet { + missing = append(missing, n) + } + sort.Strings(missing) + return nil, fmt.Errorf("requested interfaces not found: %s", strings.Join(missing, ", ")) } return &model.Package{ Name: file.Name.String(), @@ -817,36 +839,4 @@ func packageNameOfDir(srcDir string) (string, error) { return packageImport, nil } -func filterInterfaces(all []*model.Interface, requested []string) ([]*model.Interface, error) { - // If no interfaces are requested, return all interfaces - if len(requested) == 0 { - return all, nil - } - - requestedIfaces := make(map[string]struct{}) - for _, iface := range requested { - requestedIfaces[iface] = struct{}{} - } - - result := make([]*model.Interface, 0, len(requestedIfaces)) - for _, iface := range all { - // Only add interfaces that are requested - if _, ok := requestedIfaces[iface.Name]; ok { - result = append(result, iface) - delete(requestedIfaces, iface.Name) // Remove matched iface from requested - } - } - - // If any requested interfaces were not found, return an error - if len(requestedIfaces) > 0 { - var missing []string - for iface := range requestedIfaces { - missing = append(missing, iface) - } - return nil, fmt.Errorf("missing interfaces: %s", strings.Join(missing, ", ")) - } - - return result, nil -} - var errOutsideGoPath = errors.New("source directory is outside GOPATH") diff --git a/mockgen/parse_test.go b/mockgen/parse_test.go index 2c8da674..a39f8c2c 100644 --- a/mockgen/parse_test.go +++ b/mockgen/parse_test.go @@ -3,10 +3,8 @@ package main import ( "go/parser" "go/token" - "reflect" + "strings" "testing" - - "go.uber.org/mock/mockgen/model" ) func TestFileParser_ParseFile(t *testing.T) { @@ -147,126 +145,83 @@ func TestParseArrayWithConstLength(t *testing.T) { } } -func Test_filterInterfaces(t *testing.T) { - type args struct { - all []*model.Interface - requested []string +func TestParseFile_IncludeOnlyRequested(t *testing.T) { + fs := token.NewFileSet() + file, err := parser.ParseFile(fs, "internal/tests/custom_package_name/greeter/greeter.go", nil, 0) + if err != nil { + t.Fatalf("Unexpected error: %v", err) } - tests := []struct { - name string - args args - want []*model.Interface - wantErr bool - }{ - { - name: "no filter (returns all interfaces)", - args: args{ - all: []*model.Interface{ - { - Name: "Foo", - }, - { - Name: "Bar", - }, - }, - requested: []string{}, - }, - want: []*model.Interface{ - { - Name: "Foo", - }, - { - Name: "Bar", - }, - }, - wantErr: false, - }, - { - name: "filter by Foo", - args: args{ - all: []*model.Interface{ - { - Name: "Foo", - }, - { - Name: "Bar", - }, - }, - requested: []string{"Foo"}, - }, - want: []*model.Interface{ - { - Name: "Foo", - }, - }, - wantErr: false, - }, - { - name: "filter by Foo and Bar", - args: args{ - all: []*model.Interface{ - { - Name: "Foo", - }, - { - Name: "Bar", - }, - }, - requested: []string{"Foo", "Bar"}, - }, - want: []*model.Interface{ - { - Name: "Foo", - }, - { - Name: "Bar", - }, - }, - wantErr: false, - }, - { - name: "incorrect filter by Foo and Baz", - args: args{ - all: []*model.Interface{ - { - Name: "Foo", - }, - { - Name: "Bar", - }, - }, - requested: []string{"Foo", "Baz"}, - }, - want: nil, - wantErr: true, - }, - { - name: "missing interface (Baz not found)", - args: args{ - all: []*model.Interface{ - { - Name: "Foo", - }, - { - Name: "Bar", - }, - }, - requested: []string{"Baz"}, - }, - want: nil, - wantErr: true, - }, + + p := fileParser{ + fileSet: fs, + imports: make(map[string]importedPackage), + importedInterfaces: newInterfaceCache(), + // include только один интерфейс + includeNamesSet: map[string]struct{}{"InputMaker": {}}, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := filterInterfaces(tt.args.all, tt.args.requested) - if (err != nil) != tt.wantErr { - t.Errorf("filterInterfaces() error = %v, wantErr %v", err, tt.wantErr) - return - } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("filterInterfaces() got = %v, want %v", got, tt.want) + + pkg, err := p.parseFile("", file) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + if len(pkg.Interfaces) != 1 || pkg.Interfaces[0].Name != "InputMaker" { + t.Fatalf("Expected only InputMaker, got %v", pkg.Interfaces) + } +} + +func TestParseFile_IncludeMissing_ReturnsError(t *testing.T) { + fs := token.NewFileSet() + file, err := parser.ParseFile(fs, "internal/tests/custom_package_name/greeter/greeter.go", nil, 0) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + p := fileParser{ + fileSet: fs, + imports: make(map[string]importedPackage), + importedInterfaces: newInterfaceCache(), + includeNamesSet: map[string]struct{}{"DoesNotExist": {}}, + } + + _, err = p.parseFile("", file) + if err == nil || !strings.Contains(err.Error(), "requested interfaces not found") { + t.Fatalf("Expected missing interface error, got %v", err) + } +} + +func TestParseFile_IncludeWithDuplicates_Dedupes(t *testing.T) { + fs := token.NewFileSet() + file, err := parser.ParseFile(fs, "internal/tests/custom_package_name/greeter/greeter.go", nil, 0) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + // Эмулируем «случайно указали дубликаты» как это делает sourceMode (через позиционные аргументы) + args := []string{"InputMaker", "InputMaker"} // дубликаты + include := make(map[string]struct{}) + for _, a := range args { + for _, name := range strings.Split(a, ",") { + name = strings.TrimSpace(name) + if name != "" { + include[name] = struct{}{} } - }) + } + } + + p := fileParser{ + fileSet: fs, + imports: make(map[string]importedPackage), + importedInterfaces: newInterfaceCache(), + includeNamesSet: include, + } + + pkg, err := p.parseFile("", file) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + if len(pkg.Interfaces) != 1 || pkg.Interfaces[0].Name != "InputMaker" { + t.Fatalf("Expected only InputMaker once, got %v", pkg.Interfaces) } } From 89fa9e99f399940658876a7b5fad581ecef233f8 Mon Sep 17 00:00:00 2001 From: Georgii Kliukovkin Date: Fri, 3 Oct 2025 08:55:12 -0700 Subject: [PATCH 07/11] Apply suggestions from code review Co-authored-by: Abhinav Gupta --- mockgen/parse.go | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/mockgen/parse.go b/mockgen/parse.go index 393d2584..41fdd8d7 100644 --- a/mockgen/parse.go +++ b/mockgen/parse.go @@ -64,16 +64,14 @@ func sourceMode(source string) (*model.Package, error) { } // positional interface names -> include set - ifaces := flag.Args() - if len(ifaces) > 0 { + if flag.NArg() > 1 { + return nil, errors.New("-source mode accepts at most one argument") + } + if flag.NArg() == 1 { + ifaces := strings.Split(flag.Arg(0), ",") p.includeNamesSet = make(map[string]struct{}, len(ifaces)) - for _, arg := range ifaces { - for _, name := range strings.Split(arg, ",") { - name = strings.TrimSpace(name) - if name != "" { - p.includeNamesSet[name] = struct{}{} - } - } + for _, name := range ifaces { + p.includeNamesSet[name] = struct{}{} } } @@ -185,7 +183,7 @@ type fileParser struct { auxInterfaces *interfaceCache srcDir string excludeNamesSet map[string]struct{} - includeNamesSet map[string]struct{} + includeNamesSet map[string]struct{} // empty to include all } func (p *fileParser) errorf(pos token.Pos, format string, args ...any) error { @@ -252,6 +250,7 @@ func (p *fileParser) parseFile(importPath string, file *ast.File) (*model.Packag continue } + // All interfaces are included if no filter was specified. if len(p.includeNamesSet) > 0 { if _, ok := p.includeNamesSet[name]; !ok { continue @@ -270,11 +269,7 @@ func (p *fileParser) parseFile(importPath string, file *ast.File) (*model.Packag delete(p.includeNamesSet, name) } if len(p.includeNamesSet) > 0 { - missing := make([]string, 0, len(p.includeNamesSet)) - for n := range p.includeNamesSet { - missing = append(missing, n) - } - sort.Strings(missing) + missing := slices.Sorted(maps.Keys)) return nil, fmt.Errorf("requested interfaces not found: %s", strings.Join(missing, ", ")) } return &model.Package{ From db487ea9acd6f02385a69c7a4a0674faef2ab81c Mon Sep 17 00:00:00 2001 From: Georgii Kliukovkin Date: Fri, 3 Oct 2025 09:16:28 -0700 Subject: [PATCH 08/11] Update parseFile: ignore missing requested interfaces, adjust tests accordingly --- mockgen/parse.go | 6 +----- mockgen/parse_test.go | 34 +++++++++++++++++++--------------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/mockgen/parse.go b/mockgen/parse.go index 41fdd8d7..da0cc5ed 100644 --- a/mockgen/parse.go +++ b/mockgen/parse.go @@ -30,7 +30,6 @@ import ( "os" "path" "path/filepath" - "sort" "strconv" "strings" @@ -268,10 +267,7 @@ func (p *fileParser) parseFile(importPath string, file *ast.File) (*model.Packag delete(p.includeNamesSet, name) } - if len(p.includeNamesSet) > 0 { - missing := slices.Sorted(maps.Keys)) - return nil, fmt.Errorf("requested interfaces not found: %s", strings.Join(missing, ", ")) - } + return &model.Package{ Name: file.Name.String(), PkgPath: importPath, diff --git a/mockgen/parse_test.go b/mockgen/parse_test.go index a39f8c2c..d672927a 100644 --- a/mockgen/parse_test.go +++ b/mockgen/parse_test.go @@ -170,23 +170,27 @@ func TestParseFile_IncludeOnlyRequested(t *testing.T) { } } -func TestParseFile_IncludeMissing_ReturnsError(t *testing.T) { - fs := token.NewFileSet() - file, err := parser.ParseFile(fs, "internal/tests/custom_package_name/greeter/greeter.go", nil, 0) - if err != nil { +// When requested interface is missing, parser should ignore it (no error, no interfaces). +func TestParseFile_IncludeMissing_Ignored(t *testing.T) { + fs := token.NewFileSet() + file, err := parser.ParseFile(fs, "internal/tests/custom_package_name/greeter/greeter.go", nil, 0) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + p := fileParser{ + fileSet: fs, + imports: make(map[string]importedPackage), + importedInterfaces: newInterfaceCache(), + includeNamesSet: map[string]struct{}{"DoesNotExist": {}}, + } + + pkg, err := p.parseFile("", file) + if err != nil { t.Fatalf("Unexpected error: %v", err) } - - p := fileParser{ - fileSet: fs, - imports: make(map[string]importedPackage), - importedInterfaces: newInterfaceCache(), - includeNamesSet: map[string]struct{}{"DoesNotExist": {}}, - } - - _, err = p.parseFile("", file) - if err == nil || !strings.Contains(err.Error(), "requested interfaces not found") { - t.Fatalf("Expected missing interface error, got %v", err) + if len(pkg.Interfaces) != 0 { + t.Fatalf("Expected no interfaces, got %v", pkg.Interfaces) } } From ca7477a0447804633ccfcc6a4a459490a37b19d4 Mon Sep 17 00:00:00 2001 From: Georgii Kliukovkin Date: Fri, 3 Oct 2025 16:58:55 -0700 Subject: [PATCH 09/11] Update generated mocks (bugreport_mock.go) --- .../internal/tests/typed/bugreport_mock.go | 111 ------------------ 1 file changed, 111 deletions(-) diff --git a/mockgen/internal/tests/typed/bugreport_mock.go b/mockgen/internal/tests/typed/bugreport_mock.go index 00589b14..41b0b28d 100644 --- a/mockgen/internal/tests/typed/bugreport_mock.go +++ b/mockgen/internal/tests/typed/bugreport_mock.go @@ -8,114 +8,3 @@ // Package typed is a generated GoMock package. package typed - -import ( - reflect "reflect" - - gomock "go.uber.org/mock/gomock" - faux "go.uber.org/mock/mockgen/internal/tests/typed/faux" -) - -// MockSource is a mock of Source interface. -type MockSource struct { - ctrl *gomock.Controller - recorder *MockSourceMockRecorder -} - -// MockSourceMockRecorder is the mock recorder for MockSource. -type MockSourceMockRecorder struct { - mock *MockSource -} - -// NewMockSource creates a new mock instance. -func NewMockSource(ctrl *gomock.Controller) *MockSource { - mock := &MockSource{ctrl: ctrl} - mock.recorder = &MockSourceMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockSource) EXPECT() *MockSourceMockRecorder { - return m.recorder -} - -// ISGOMOCK indicates that this struct is a gomock mock. -func (m *MockSource) ISGOMOCK() struct{} { - return struct{}{} -} - -// Error mocks base method. -func (m *MockSource) Error() string { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Error") - ret0, _ := ret[0].(string) - return ret0 -} - -// Error indicates an expected call of Error. -func (mr *MockSourceMockRecorder) Error() *MockSourceErrorCall { - mr.mock.ctrl.T.Helper() - call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Error", reflect.TypeOf((*MockSource)(nil).Error)) - return &MockSourceErrorCall{Call: call} -} - -// MockSourceErrorCall wrap *gomock.Call -type MockSourceErrorCall struct { - *gomock.Call -} - -// Return rewrite *gomock.Call.Return -func (c *MockSourceErrorCall) Return(arg0 string) *MockSourceErrorCall { - c.Call = c.Call.Return(arg0) - return c -} - -// Do rewrite *gomock.Call.Do -func (c *MockSourceErrorCall) Do(f func() string) *MockSourceErrorCall { - c.Call = c.Call.Do(f) - return c -} - -// DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockSourceErrorCall) DoAndReturn(f func() string) *MockSourceErrorCall { - c.Call = c.Call.DoAndReturn(f) - return c -} - -// Method mocks base method. -func (m *MockSource) Method() faux.Return { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Method") - ret0, _ := ret[0].(faux.Return) - return ret0 -} - -// Method indicates an expected call of Method. -func (mr *MockSourceMockRecorder) Method() *MockSourceMethodCall { - mr.mock.ctrl.T.Helper() - call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Method", reflect.TypeOf((*MockSource)(nil).Method)) - return &MockSourceMethodCall{Call: call} -} - -// MockSourceMethodCall wrap *gomock.Call -type MockSourceMethodCall struct { - *gomock.Call -} - -// Return rewrite *gomock.Call.Return -func (c *MockSourceMethodCall) Return(arg0 faux.Return) *MockSourceMethodCall { - c.Call = c.Call.Return(arg0) - return c -} - -// Do rewrite *gomock.Call.Do -func (c *MockSourceMethodCall) Do(f func() faux.Return) *MockSourceMethodCall { - c.Call = c.Call.Do(f) - return c -} - -// DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockSourceMethodCall) DoAndReturn(f func() faux.Return) *MockSourceMethodCall { - c.Call = c.Call.DoAndReturn(f) - return c -} From e1e3ec960d4e23d87fb9340b21bfecd51320be2d Mon Sep 17 00:00:00 2001 From: Georgii Kliukovkin Date: Fri, 3 Oct 2025 17:07:54 -0700 Subject: [PATCH 10/11] test(typed): regenerate bugreport_mock.go after upstream merge --- .../internal/tests/typed/bugreport_mock.go | 107 ------------------ 1 file changed, 107 deletions(-) diff --git a/mockgen/internal/tests/typed/bugreport_mock.go b/mockgen/internal/tests/typed/bugreport_mock.go index 5c928121..41b0b28d 100644 --- a/mockgen/internal/tests/typed/bugreport_mock.go +++ b/mockgen/internal/tests/typed/bugreport_mock.go @@ -8,110 +8,3 @@ // Package typed is a generated GoMock package. package typed - -import ( - reflect "reflect" - - gomock "go.uber.org/mock/gomock" - faux "go.uber.org/mock/mockgen/internal/tests/typed/faux" -) - -// MockSource is a mock of Source interface. -type MockSource struct { - ctrl *gomock.Controller - recorder *MockSourceMockRecorder - isgomock struct{} -} - -// MockSourceMockRecorder is the mock recorder for MockSource. -type MockSourceMockRecorder struct { - mock *MockSource -} - -// NewMockSource creates a new mock instance. -func NewMockSource(ctrl *gomock.Controller) *MockSource { - mock := &MockSource{ctrl: ctrl} - mock.recorder = &MockSourceMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockSource) EXPECT() *MockSourceMockRecorder { - return m.recorder -} - -// Error mocks base method. -func (m *MockSource) Error() string { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Error") - ret0, _ := ret[0].(string) - return ret0 -} - -// Error indicates an expected call of Error. -func (mr *MockSourceMockRecorder) Error() *MockSourceErrorCall { - mr.mock.ctrl.T.Helper() - call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Error", reflect.TypeOf((*MockSource)(nil).Error)) - return &MockSourceErrorCall{Call: call} -} - -// MockSourceErrorCall wrap *gomock.Call -type MockSourceErrorCall struct { - *gomock.Call -} - -// Return rewrite *gomock.Call.Return -func (c *MockSourceErrorCall) Return(arg0 string) *MockSourceErrorCall { - c.Call = c.Call.Return(arg0) - return c -} - -// Do rewrite *gomock.Call.Do -func (c *MockSourceErrorCall) Do(f func() string) *MockSourceErrorCall { - c.Call = c.Call.Do(f) - return c -} - -// DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockSourceErrorCall) DoAndReturn(f func() string) *MockSourceErrorCall { - c.Call = c.Call.DoAndReturn(f) - return c -} - -// Method mocks base method. -func (m *MockSource) Method() faux.Return { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Method") - ret0, _ := ret[0].(faux.Return) - return ret0 -} - -// Method indicates an expected call of Method. -func (mr *MockSourceMockRecorder) Method() *MockSourceMethodCall { - mr.mock.ctrl.T.Helper() - call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Method", reflect.TypeOf((*MockSource)(nil).Method)) - return &MockSourceMethodCall{Call: call} -} - -// MockSourceMethodCall wrap *gomock.Call -type MockSourceMethodCall struct { - *gomock.Call -} - -// Return rewrite *gomock.Call.Return -func (c *MockSourceMethodCall) Return(arg0 faux.Return) *MockSourceMethodCall { - c.Call = c.Call.Return(arg0) - return c -} - -// Do rewrite *gomock.Call.Do -func (c *MockSourceMethodCall) Do(f func() faux.Return) *MockSourceMethodCall { - c.Call = c.Call.Do(f) - return c -} - -// DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockSourceMethodCall) DoAndReturn(f func() faux.Return) *MockSourceMethodCall { - c.Call = c.Call.DoAndReturn(f) - return c -} From a6a5b062b621d478e1bf6c455b94ba2cec8570e0 Mon Sep 17 00:00:00 2001 From: Georgii Kliukovkin Date: Fri, 3 Oct 2025 19:04:35 -0700 Subject: [PATCH 11/11] regenerate goldens: update generated mocks after upstream merge --- mockgen/internal/tests/typed/bugreport.go | 2 +- .../internal/tests/typed/bugreport_mock.go | 109 +++++++++++++++++- 2 files changed, 109 insertions(+), 2 deletions(-) diff --git a/mockgen/internal/tests/typed/bugreport.go b/mockgen/internal/tests/typed/bugreport.go index 49fc286b..60a916af 100644 --- a/mockgen/internal/tests/typed/bugreport.go +++ b/mockgen/internal/tests/typed/bugreport.go @@ -1,6 +1,6 @@ package typed -//go:generate mockgen -typed -aux_files faux=faux/faux.go -destination bugreport_mock.go -package typed -source=bugreport.go Example +//go:generate mockgen -typed -aux_files faux=faux/faux.go -destination bugreport_mock.go -package typed -source=bugreport.go Source import ( "log" diff --git a/mockgen/internal/tests/typed/bugreport_mock.go b/mockgen/internal/tests/typed/bugreport_mock.go index 41b0b28d..038811e6 100644 --- a/mockgen/internal/tests/typed/bugreport_mock.go +++ b/mockgen/internal/tests/typed/bugreport_mock.go @@ -3,8 +3,115 @@ // // Generated by this command: // -// mockgen -typed -aux_files faux=faux/faux.go -destination bugreport_mock.go -package typed -source=bugreport.go Example +// mockgen -typed -aux_files faux=faux/faux.go -destination bugreport_mock.go -package typed -source=bugreport.go Source // // Package typed is a generated GoMock package. package typed + +import ( + reflect "reflect" + + gomock "go.uber.org/mock/gomock" + faux "go.uber.org/mock/mockgen/internal/tests/typed/faux" +) + +// MockSource is a mock of Source interface. +type MockSource struct { + ctrl *gomock.Controller + recorder *MockSourceMockRecorder + isgomock struct{} +} + +// MockSourceMockRecorder is the mock recorder for MockSource. +type MockSourceMockRecorder struct { + mock *MockSource +} + +// NewMockSource creates a new mock instance. +func NewMockSource(ctrl *gomock.Controller) *MockSource { + mock := &MockSource{ctrl: ctrl} + mock.recorder = &MockSourceMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockSource) EXPECT() *MockSourceMockRecorder { + return m.recorder +} + +// Error mocks base method. +func (m *MockSource) Error() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Error") + ret0, _ := ret[0].(string) + return ret0 +} + +// Error indicates an expected call of Error. +func (mr *MockSourceMockRecorder) Error() *MockSourceErrorCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Error", reflect.TypeOf((*MockSource)(nil).Error)) + return &MockSourceErrorCall{Call: call} +} + +// MockSourceErrorCall wrap *gomock.Call +type MockSourceErrorCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockSourceErrorCall) Return(arg0 string) *MockSourceErrorCall { + c.Call = c.Call.Return(arg0) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockSourceErrorCall) Do(f func() string) *MockSourceErrorCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockSourceErrorCall) DoAndReturn(f func() string) *MockSourceErrorCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + +// Method mocks base method. +func (m *MockSource) Method() faux.Return { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Method") + ret0, _ := ret[0].(faux.Return) + return ret0 +} + +// Method indicates an expected call of Method. +func (mr *MockSourceMockRecorder) Method() *MockSourceMethodCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Method", reflect.TypeOf((*MockSource)(nil).Method)) + return &MockSourceMethodCall{Call: call} +} + +// MockSourceMethodCall wrap *gomock.Call +type MockSourceMethodCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockSourceMethodCall) Return(arg0 faux.Return) *MockSourceMethodCall { + c.Call = c.Call.Return(arg0) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockSourceMethodCall) Do(f func() faux.Return) *MockSourceMethodCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockSourceMethodCall) DoAndReturn(f func() faux.Return) *MockSourceMethodCall { + c.Call = c.Call.DoAndReturn(f) + return c +}