Add ability to replace type constraints #750

DustinJSilk · 2024-01-24T12:40:41Z

Description

Adds a config option that can replace generic types with a concrete type.

Fixes Option to replace a generic type parameter with a specific type #747

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

Version of Golang used when building/testing:

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Checklist

My code follows the style guidelines of this project
I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

LandonTClipp · 2024-01-24T21:24:18Z

Excellent! I'll give it a proper review in a few days. Thanks for the contribution.

codecov · 2024-01-24T21:25:25Z

Codecov Report

Attention: 82 lines in your changes are missing coverage. Please review.

Comparison is base (3b25f39) 42.45067% compared to head (601d297) 42.65889%.
Report is 4 commits behind head on master.

Files	Patch %	Lines
...m/vektra/mockery/v2/pkg/fixtures/ReplaceGeneric.go	53.01205%	30 Missing and 9 partials ⚠️
pkg/generator.go	43.47826%	20 Missing and 6 partials ⚠️
...ktra/mockery/v2/pkg/fixtures/ReplaceGenericSelf.go	60.60606%	10 Missing and 3 partials ⚠️
...hub.com/vektra/mockery/v2/pkg/fixtures/Variadic.go	0.00000%	2 Missing ⚠️
...ktra/mockery/v2/pkg/fixtures/VariadicReturnFunc.go	0.00000%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@                 Coverage Diff                 @@
##              master        #750         +/-   ##
===================================================
+ Coverage   42.45067%   42.65889%   +0.20821%     
===================================================
  Files             61          63          +2     
  Lines           4815        4972        +157     
===================================================
+ Hits            2044        2121         +77     
- Misses          2591        2653         +62     
- Partials         180         198         +18

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pkg/generator.go

DustinJSilk · 2024-01-31T10:53:28Z

Hey @LandonTClipp
These are the 2 new formats I'd propose for replacing generic types using the replace-type option, please let me know what you think of the 2 examples below:

Replace a type parameter:
module.com/foo.Foo[T]=module.com/baz.Baz

type Foo[T any] struct {}
func (*Foo) Get() T {}

type Foo[T baz.Baz] struct {}
func (*Foo) Get() T {}

Remove a type parameter and set each instance of it:
module.com/foo.Foo[-T]=module.com/baz.Baz

type Foo[T any] struct {}
func (*Foo) Get() T {}

type Foo struct {}
func (*Foo) Get() baz.Baz {}

DustinJSilk · 2024-02-08T11:24:23Z

Hi @LandonTClipp

I've updated the API to the suggestion above which I'm using successfully for my usecase and hopefully it provides more flexibility for others.

I'm happy to change the API if you have any ideas.

LandonTClipp · 2024-02-08T19:13:37Z

Hi @DustinJSilk, apologies for the lack of responses the last few weeks. I have been on the hunt for a new job and I haven't been able to devote much time or energy to this project, but luckily my search is coming to a close (hopefully, knock on wood).

As far as your proposals, I think I'm on board with it. The only thing I'm unhappy about (which has nothing to do with your proposal or implementation) is that I don't enjoy these complex, custom parameter strings. I think in hindsight, I would have preferred replace-types to simply be a list of maps with individual parameters, instead of having to parse this string here. That ship has already sailed... it's something I'll probably deprecate once a better implementation is available.

LandonTClipp

Looks great, I think I'm okay with the new format here. I have a few comments, if we can get those addressed then I will merge it!

docs/features.md

LandonTClipp · 2024-02-08T19:31:06Z

pkg/generator.go

+	// Match type parameter substitution
+	match := regexp.MustCompile(`\[(.*?)\]$`).FindStringSubmatch(t)
+	if len(match) >= 2 {
+		if match[1][:1] == "-" {


I was initially confused why you did that instead of:

match[1][0]

but realized the slicing operator will return a string instead of a single byte. To make this cleaner, I'd instead do this:

if strings.HasPrefix(match[1], "-")

It's a bit more obvious what's happening.

Let me know if it still makes sense for you, i was able to put it in one line with strings.CutPrefix

LandonTClipp · 2024-02-08T19:47:06Z

pkg/generator_test.go

+func (s *GeneratorSuite) TestReplaceTypeGeneric() {
+	cfg := GeneratorConfig{InPackage: false, ReplaceType: []string{
+		"github.com/vektra/mockery/v2/pkg/fixtures.ReplaceGeneric[-TImport]=github.com/vektra/mockery/v2/pkg/fixtures/redefined_type_b.B",
+		"github.com/vektra/mockery/v2/pkg/fixtures.ReplaceGeneric[TConstraint]=github.com/vektra/mockery/v2/pkg/fixtures/constraints.Integer",
+		"github.com/vektra/mockery/v2/pkg/fixtures.ReplaceGenericSelf[-T]=github.com/vektra/mockery/v2/pkg/fixtures.*ReplaceGenericSelf",
+	}}
+
+	s.checkGenerationRegexWithConfig("generic.go", "ReplaceGeneric", cfg, []regexpExpected{
+		// type ReplaceGeneric[TConstraint constraints.Integer, TKeep interface{}] struct
+		{true, regexp.MustCompile(`type ReplaceGeneric\[TConstraint constraints.Integer\, TKeep interface\{\}] struct`)},
+		// func (_m *ReplaceGeneric[TConstraint, TKeep]) A(t1 test.B) TKeep
+		{true, regexp.MustCompile(`func \(_m \*ReplaceGeneric\[TConstraint, TKeep\]\) A\(t1 test\.B\) TKeep`)},
+		// func (_m *ReplaceGeneric[TConstraint, TKeep]) B() test.B
+		{true, regexp.MustCompile(`func \(_m \*ReplaceGeneric\[TConstraint, TKeep\]\) B\(\) test\.B`)},
+		// func (_m *ReplaceGeneric[TConstraint, TKeep]) C() TConstraint
+		{true, regexp.MustCompile(`func \(_m \*ReplaceGeneric\[TConstraint, TKeep\]\) C\(\) TConstraint`)},
+	})
+
+	s.checkGenerationRegexWithConfig("generic.go", "ReplaceGenericSelf", cfg, []regexpExpected{
+		// type ReplaceGenericSelf struct
+		{true, regexp.MustCompile(`type ReplaceGenericSelf struct`)},
+		// func (_m *ReplaceGenericSelf) A() *ReplaceGenericSelf
+		{true, regexp.MustCompile(`func \(_m \*ReplaceGenericSelf\) A\(\) \*ReplaceGenericSelf`)},
+	})
+}
+


I do not like these kinds of tests and am asking people not to do them. Any changes to the formatting can/will cause these kinds of tests to fail. Instead I prefer if we just create a new test file, probably in pkg/fixtures/test that checks these various scenarios through a functional test, rather than string comparison. You can just rely on the compiler to fail if the param types are mismatched, or maybe use type assertion through an empty interface in the test (actually, type assertion might be better in this case so that we don't cause all the tests to fail if the mock implementation is wrong somehow).

Good idea! I've added some better test cases and removed these. I didn't quite figure out exactly how we could test just with type assertions, at the moment the compiler would complain if the types no longer match

LandonTClipp

Excellent, this looks good to me. Thanks again!

LandonTClipp · 2024-02-15T19:27:13Z

The tests are failing due to an unrelated bug that I will fix in a separate PR.

LandonTClipp · 2024-02-16T21:05:05Z

Good work @DustinJSilk, feature has been released.

DustinJSilk · 2024-02-17T09:49:02Z

Amazing thanks @LandonTClipp

LandonTClipp reviewed Jan 24, 2024

View reviewed changes

pkg/generator.go Outdated Show resolved Hide resolved

use replace-type config

4b08243

DustinJSilk force-pushed the replace-generic branch from 93da8ee to 4b08243 Compare February 8, 2024 11:13

Show support for pointer type replacements

e724b8a

LandonTClipp requested changes Feb 8, 2024

View reviewed changes

fixes

78755a1

LandonTClipp approved these changes Feb 14, 2024

View reviewed changes

LandonTClipp changed the title ~~Add replace-generic config~~ Add ability to replace type constraints Feb 15, 2024

Merge branch 'master' into replace-generic

601d297

LandonTClipp merged commit 8b86cf2 into vektra:master Feb 16, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ability to replace type constraints #750

Add ability to replace type constraints #750

DustinJSilk commented Jan 24, 2024

LandonTClipp commented Jan 24, 2024

codecov bot commented Jan 24, 2024 •

edited

DustinJSilk commented Jan 31, 2024

DustinJSilk commented Feb 8, 2024

LandonTClipp commented Feb 8, 2024

LandonTClipp left a comment

LandonTClipp Feb 8, 2024 •

edited

DustinJSilk Feb 14, 2024

LandonTClipp Feb 8, 2024

DustinJSilk Feb 14, 2024

LandonTClipp left a comment

LandonTClipp commented Feb 15, 2024

LandonTClipp commented Feb 16, 2024

DustinJSilk commented Feb 17, 2024

Add ability to replace type constraints #750

Add ability to replace type constraints #750

Conversation

DustinJSilk commented Jan 24, 2024

Description

Type of change

Version of Golang used when building/testing:

How Has This Been Tested?

Checklist

LandonTClipp commented Jan 24, 2024

codecov bot commented Jan 24, 2024 • edited

Codecov Report

DustinJSilk commented Jan 31, 2024

DustinJSilk commented Feb 8, 2024

LandonTClipp commented Feb 8, 2024

LandonTClipp left a comment

Choose a reason for hiding this comment

LandonTClipp Feb 8, 2024 • edited

Choose a reason for hiding this comment

DustinJSilk Feb 14, 2024

Choose a reason for hiding this comment

LandonTClipp Feb 8, 2024

Choose a reason for hiding this comment

DustinJSilk Feb 14, 2024

Choose a reason for hiding this comment

LandonTClipp left a comment

Choose a reason for hiding this comment

LandonTClipp commented Feb 15, 2024

LandonTClipp commented Feb 16, 2024

DustinJSilk commented Feb 17, 2024

codecov bot commented Jan 24, 2024 •

edited

LandonTClipp Feb 8, 2024 •

edited