--name doesn't accept regex values

[leaanthony-sc]

Description

Running mockery with --name "<regex>" results in the following error: Cannot specify --filename or --structname with regex in --name, even when no config or flag has been set for either of these 2 options.

I've tracked this bug down to this section of code. It appears that the check after a valid regex compilation looks for non-zero values for r.Config.Filename and r.Config.StructName, and if either are set then it raises a fatal error. The problem appears to be that a default value is set for r.Config.Filename so this check will always fail.

Mockery Version

v2.32.3

Golang Version

v1.20.7

Installation Method

• Binary Distribution
• Docker
• brew
• go install
• Other: [specify]

Steps to Reproduce

1. Run mockery with the flags --name ".*Client"

Expected Behavior

Mocks generated for all interfaces found with a suffix of Client

Actual Behavior

Fatal error message: Cannot specify --filename or --structname with regex in --name

[LandonTClipp]

The first question I have is whether or not you are using the packages feature. If you are not, then that default value in https://github.com/leaanthony-sc/mockery/blob/3b725d9c23ccfb4839bb503ca17f06425bb217ef/pkg/config/config.go#L96 isn't being applied to your config. If you are using packages, then you should not be hitting this code path at all (and you also shouldn't be using name as that's not a supported parameter in packages. If this is the case, I need to make mockery fail harder and more explicitly).

[leaanthony-sc]

The first question I have is whether or not you are using the packages feature. If you are not, then that default value in https://github.com/leaanthony-sc/mockery/blob/3b725d9c23ccfb4839bb503ca17f06425bb217ef/pkg/config/config.go#L96 isn't being applied to your config.

Yes, we are using the packages config:

packages:
  github.com/xxxxx/xxxxx:
    config:
      recursive: True
      inpackage: True
      case: underscore

If you are using packages, then you should not be hitting this code path at all (and you also shouldn't be using name as that's not a supported parameter in packages. If this is the case, I need to make mockery fail harder and more explicitly).

We are ok not using name however there doesn't appear to be a regex equivalent in packages config, only "explicit" or "all". Is that something you are looking to add or are you dropping this capability entirely? Thanks.

[LandonTClipp]

You're correct there is no equivalent, so you have to explicitly list the interfaces you want, or just do all: True. I'm open to the idea of adding a regex matching pattern, but I haven't received many any requests for it yet so I'm not sure how desirable it is. I try to keep the config model as explicit as possible, and I just don't like regex for many reasons. I can imagine there's a few clean ways it could be done.

[LandonTClipp]

Maybe something like this:

packages:
  github.com/xxxxx/xxxxx:
    config:
      recursive: True
      inpackage: True
      all: True
      include: ".*Client"
      exclude: "FooClient"

🤷🏻
If you can live without it however, I'd prefer to not implement it at the moment.

[leaanthony-sc]

Would you be open to a PR for this? In our use case, generating all mocks increases package size by nearly 100% and takes ~7x more time than filtering out the interfaces before generation (I used a simple HasSuffix test to filter). The regex feature has always been hugely beneficial for our use case. I'd be amazed if we are the only ones.

[rogchap]

Hey @LandonTClipp how are you doing?

Firstly; congrats on the work you have done with this project, you've done so much to keep it fresh and up to date. Kudos.

We're in a similar situation as @leaanthony-sc; I would like to upgrade to v3, but are struggling with some of the new breaking changes.

We have a very large repository that holds all of our protocol buffer services for our entire system, very similar to Google's APIs: https://github.com/googleapis/googleapis

When we generate the Go clients we have 100s (maybe 1000s) of gRPC Clients that get generated. Example: https://github.com/grpc/grpc-go/blob/master/examples/helloworld/helloworld/helloworld_grpc.pb.go#L42

These clients are all auto-generated (ie. have "DO NOT EDIT") and would require a lot of work (script-able... but still a pain) to have an explicit list.

These files/packages contain all the Service interfaces too which we do not need to mock so all is not a good option either.

I would expect that many people would want to mock gRPC clients (perfect use-case for mockery) so would be good to cater to this. What do you think?

[LandonTClipp]

Hi folks, and @rogchap. I'm okay with someone implementing this. It seems like the need is pretty strong so that's okay. If you want to put in a PR that implements the config I suggested then I will accept it!

[benvernier-sc]

+1 on this being a much needed feature when using packages config.

In terms of how it is implemented / setup, is there a way we can leverage the fact that matchString is already a template function to be able to set a regex matching in a generic manner without having to add a dedicated field for it?

[LandonTClipp]

@benvernier-sc I'm not sure if we can leverage that. The config model has the name of the interface as part of the interfaces: map key. If I'm understanding you correctly, you're wondering if we can use matchString inside of the interfaces map keys?

What would you propose instead of the example I posted? Since I'm not sure I understand your ask.

[benvernier-sc]

@LandonTClipp Yes, my instinctive thought when reading the docs and seeing that matchString was a template function was to try something along those lines:

packages:
  github.com/xxxxx/xxxxx:
    config:
      recursive: True
      inpackage: True
    interfaces:
      "{{ matchString(\".*(Suffix1|Suffix2)\", .InterfaceName) }}":

[LandonTClipp]

Hmm, that would complicate some things. There are various parts of the code that assume they can map a single interface name in interfaces. Allowing templates in that map would mean you have to iterate over every key to see if it matches, which would be inefficient to say the least. Instead I think the easier way forward is what I suggest. There is already a Config.ShouldGenerateInterface method that controls what is included/excluded: https://github.com/vektra/mockery/blob/v2.32.4/pkg/config/config.go

It would be a matter of writing two conditionals for the include/exclude regex. Nothing else would have to be touched.

[benvernier-sc]

That makes sense, and a similar conclusion to what I had found when I looked at the implementation, I was mainly sharing an other perspective as to what I intuitively thought would be the way to achieve this.

Regarding the suggested include field, would it be exclusive with the all option? It might be confusing to specify all: True but not have all interfaces be generated because an include field has also been specified. I however imagine that the exclude one would work with either all or include.

[LandonTClipp]

It's a good point that it might be confusing. I wonder if maybe specifying include would be enough by itself. If that's specified, the internal state of mockery might need to pretend that all was set but the config itself would at least be a bit more consistent. I'll need to stew on this tonight, if you have any other ideas I'd be glad to bounce them around.

[LandonTClipp]

After further thought I think the best way forward would be to use a parameter named include_regex by itself (without needing to specify all). If it's specified with all then we should log a warning and ignore the include_regex.

If someone wants to volunteer to implement this then I'd be happy to let you have it. Otherwise it might take me a couple weeks to do it.