Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(generator): add Expectation structs and functions #3

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

gitworkflows
Copy link

@gitworkflows gitworkflows commented Feb 17, 2025

User description

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

  • Fixes # (issue)

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 Go used when building/testing:

  • 1.22
  • 1.23

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

PR Type

Enhancement


Description

  • Added generateExpectation method for mock generation enhancements.

  • Introduced new struct types for function arguments and returns.

  • Enhanced mock functionality with ApplyExpectation method.

  • Imported slices package for slice cloning operations.


Changes walkthrough 📝

Relevant files
Enhancement
generator.go
Enhance mock generation with new methods and structs         

pkg/generator.go

  • Added generateExpectation method for generating mock expectations.
  • Introduced new struct types for function arguments and returns.
  • Added ApplyExpectation method to apply mock expectations.
  • Imported slices package for improved slice handling.
  • +82/-0   

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • Signed-off-by: gitworkflows <[email protected]>
    Copy link

    gitstream-cm bot commented Feb 17, 2025

    🚨 Warning: Approaching Monthly Automation Limit

    Monthly PRs automated: 247/250

    Your organization has used over 90% of its monthly quota for gitStream automations. Once the quota is reached, new pull requests for this month will not trigger gitStream automations and will be marked as “Skipped”.

    To avoid interruptions, consider optimizing your automation usage or upgrading your plan by contacting LinearB.

    Copy link

    sourcery-ai bot commented Feb 17, 2025

    Reviewer's Guide by Sourcery

    This pull request introduces a new generateExpectation function to generate expectation structs and methods for each mocked method. This allows users to define expectations for mocked methods, including argument matching and return values. The implementation involves creating argument and return structs, an Expectation struct, and an Apply...Expectation method to apply the defined expectations.

    Sequence diagram for expectation application

    sequenceDiagram
        participant User
        participant MockObject
        participant gomock
    
        User->>MockObject: ApplyMethodExpectation(expectation)
        activate MockObject
        MockObject->>gomock: On("MethodName", args...)
        activate gomock
        alt Returns are defined
            gomock->>gomock: Return(returnValues)
        end
        deactivate gomock
        deactivate MockObject
    
    Loading

    File-Level Changes

    Change Details Files
    Introduces a new function generateExpectation to generate expectation structs and methods for each mocked method.
    • Adds a new generateExpectation function to the Generator struct.
    • Creates argument structs for each method, including Anything fields for flexible argument matching.
    • Creates return structs for each method.
    • Creates an Expectation struct that combines arguments and return structs.
    • Adds an Apply...Expectation method to the mock to apply the defined expectations using mock.Anything when specified.
    pkg/generator.go

    Tips and commands

    Interacting with Sourcery

    • Trigger a new review: Comment @sourcery-ai review on the pull request.
    • Continue discussions: Reply directly to Sourcery's review comments.
    • Generate a GitHub issue from a review comment: Ask Sourcery to create an
      issue from a review comment by replying to it. You can also reply to a
      review comment with @sourcery-ai issue to create an issue from it.
    • Generate a pull request title: Write @sourcery-ai anywhere in the pull
      request title to generate a title at any time. You can also comment
      @sourcery-ai title on the pull request to (re-)generate the title at any time.
    • Generate a pull request summary: Write @sourcery-ai summary anywhere in
      the pull request body to generate a PR summary at any time exactly where you
      want it. You can also comment @sourcery-ai summary on the pull request to
      (re-)generate the summary at any time.
    • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
      request to (re-)generate the reviewer's guide at any time.
    • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
      pull request to resolve all Sourcery comments. Useful if you've already
      addressed all the comments and don't want to see them anymore.
    • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
      request to dismiss all existing Sourcery reviews. Especially useful if you
      want to start fresh with a new review - don't forget to comment
      @sourcery-ai review to trigger a new review!
    • Generate a plan of action for an issue: Comment @sourcery-ai plan on
      an issue to generate a plan of action for it.

    Customizing Your Experience

    Access your dashboard to:

    • Enable or disable review features such as the Sourcery-generated pull request
      summary, the reviewer's guide, and others.
    • Change the review language.
    • Add, remove or edit custom review instructions.
    • Adjust other review settings.

    Getting Help

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Error Handling

    The generateExpectation method lacks error handling for edge cases like empty function names or nil parameter/return lists

    func (g *Generator) generateExpectation(fname string, params, returns *paramList) {
    	// clone slices to avoid shadow overwriting
    	params = &paramList{
    		Names: slices.Clone(params.Names),
    		Types: slices.Clone(params.Types),
    	}
    	returns = &paramList{
    		Names: slices.Clone(returns.Names),
    		Types: slices.Clone(returns.Types),
    	}
    	for i, name := range params.Names {
    		params.Names[i] = strings.ToUpper(name[:1]) + name[1:]
    	}
    	for i, name := range returns.Names {
    		returns.Names[i] = strings.ToUpper(name[:1]) + name[1:]
    	}
    	data := struct {
    		FunctionName  string
    		InterfaceName string
    		MockName      string
    		Params        *paramList
    		Returns       *paramList
    	}{
    		FunctionName:  fname,
    		InterfaceName: g.iface.Name,
    		MockName:      g.mockName(),
    		Params:        params,
    		Returns:       returns,
    	}
    
    	g.printTemplate(data, `
    {{- if .Params.Names }}
    type {{ .InterfaceName }}{{ .FunctionName }}Args struct {
        {{- range $i, $name := .Params.Names }}
        {{ $name | firstUpper }} {{ trimPrefix (index $.Params.Types $i) "..."}}
        {{ $name }}Anything bool
        {{- end }}
    }
    {{ end }}
    
    {{- if .Returns.Names }}
    type {{ .InterfaceName }}{{ .FunctionName }}Returns struct {
        {{- range $i, $name := .Returns.Names }}
        {{ $name }} {{ index $.Returns.Types $i }}
        {{- end }}
    }
    {{ end }}
    
    type {{ .InterfaceName }}{{ .FunctionName }}Expectation struct {
        {{- if .Params.Names }}
        Args {{ .InterfaceName }}{{ .FunctionName }}Args
        {{- end }}
        {{- if .Returns.Names }}
        Returns {{ .InterfaceName }}{{ .FunctionName }}Returns
        {{- end }}
    }
    
    func (_m *{{ .MockName }}) Apply{{ .FunctionName }}Expectation(e {{ .InterfaceName }}{{ .FunctionName }}Expectation) {
        var args []interface{}
        {{- range $name := .Params.Names }}
        if e.Args.{{ $name }}Anything {
            args = append(args, mock.Anything)
        } else {
            args = append(args, e.Args.{{ $name }})
        }
        {{- end }}
    
        _m.On("{{ .FunctionName }}", args...)
    	{{- if .Returns.Names -}}
    	.Return(
            {{- range $i, $name := .Returns.Names -}}
            e.Returns.{{ $name }},
            {{- end }}
        )
        {{- end }}
    }
    `)
    }
    Memory Usage

    Deep cloning of parameter lists using slices.Clone may be inefficient for large parameter lists. Consider using pointers or optimizing the cloning logic

    params = &paramList{
    	Names: slices.Clone(params.Names),
    	Types: slices.Clone(params.Types),
    }
    returns = &paramList{
    	Names: slices.Clone(returns.Names),
    	Types: slices.Clone(returns.Types),
    }

    Copy link

    qodo-merge-pro-for-open-source bot commented Feb 17, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Add nil checks for slices

    Add error handling for empty/nil parameter and return slices in
    generateExpectation to prevent potential panics when accessing slice elements

    pkg/generator.go [911-918]

     params = &paramList{
    -    Names: slices.Clone(params.Names),
    -    Types: slices.Clone(params.Types),
    +    Names: make([]string, 0),
    +    Types: make([]string, 0),
    +}
    +if params != nil {
    +    params.Names = slices.Clone(params.Names)
    +    params.Types = slices.Clone(params.Types)
     }
     returns = &paramList{
    -    Names: slices.Clone(returns.Names),
    -    Types: slices.Clone(returns.Types),
    +    Names: make([]string, 0),
    +    Types: make([]string, 0),
    +}
    +if returns != nil {
    +    returns.Names = slices.Clone(returns.Names)
    +    returns.Types = slices.Clone(returns.Types)
     }

    [Suggestion has been applied]

    Suggestion importance[1-10]: 8

    __

    Why: The suggestion addresses a potential runtime panic by adding nil checks and proper initialization of slices. This is an important defensive programming practice for a code generator.

    Medium
    Add string length validation

    Add bounds checking before accessing first character of parameter names to
    prevent panic on empty strings

    pkg/generator.go [919-921]

     for i, name := range params.Names {
    -    params.Names[i] = strings.ToUpper(name[:1]) + name[1:]
    +    if len(name) > 0 {
    +        params.Names[i] = strings.ToUpper(name[:1]) + name[1:]
    +    }
     }
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    __

    Why: The suggestion prevents potential runtime panics by adding length validation before string slicing operations. This is a critical safety check since the code processes user-provided parameter names.

    Medium
    • Update

    Copy link

    @sourcery-ai sourcery-ai bot left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Hey @gitworkflows - I've reviewed your changes - here's some feedback:

    Overall Comments:

    • Consider adding a comment to explain the purpose of the generateExpectation function.
    • The code duplication in cloning params and returns could be reduced by creating a helper function.
    Here's what I looked at during the review
    • 🟢 General issues: all looks good
    • 🟢 Security: all looks good
    • 🟢 Testing: all looks good
    • 🟡 Complexity: 1 issue found
    • 🟢 Documentation: all looks good

    Sourcery is free for open source - if you like our reviews please consider sharing them ✨
    Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

    @@ -903,6 +906,85 @@ func (_m *{{.MockName}}{{.InstantiatedTypeString}}) {{.FunctionName}}({{join .Pa
    }
    }

    func (g *Generator) generateExpectation(fname string, params, returns *paramList) {
    Copy link

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    issue (complexity): Consider refactoring repetitive logic in generateExpectation into helper functions to improve readability and testability.

    The new generateExpectation function does add significant nested logic which may hurt maintainability. Consider refactoring repetitive tasks (such as cloning lists and capitalizing names) into dedicated helper functions. This reduces the function’s nesting and improves readability and testability.

    For example, extract a helper for capitalization:

    func capitalize(name string) string {
    	if name == "" {
    		return name
    	}
    	return strings.ToUpper(name[:1]) + name[1:]
    }

    Then update the loops:

    for i, name := range params.Names {
    	params.Names[i] = capitalize(name)
    }
    for i, name := range returns.Names {
    	returns.Names[i] = capitalize(name)
    }

    Additionally, consider a helper to clone and adjust a parameter list if the same logic applies in multiple places:

    func cloneAndCapitalize(pl *paramList) *paramList {
    	return &paramList{
    		Names: slices.Map(slices.Clone(pl.Names), capitalize),
    		Types: slices.Clone(pl.Types),
    	}
    }

    Now your function can be refactored to:

    func (g *Generator) generateExpectation(fname string, params, returns *paramList) {
    	params = cloneAndCapitalize(params)
    	returns = cloneAndCapitalize(returns)
    
    	data := struct {
    		FunctionName  string
    		InterfaceName string
    		MockName      string
    		Params        *paramList
    		Returns       *paramList
    	}{
    		FunctionName:  fname,
    		InterfaceName: g.iface.Name,
    		MockName:      g.mockName(),
    		Params:        params,
    		Returns:       returns,
    	}
    
    	g.printTemplate(data, `
            ...large multiline template...
        `)
    }

    These changes reduce the complexity without altering functionality.

    Signed-off-by: gitworkflows <[email protected]>
    Signed-off-by: gitworkflows <[email protected]>
    Signed-off-by: gitworkflows <[email protected]>
    @gitworkflows gitworkflows changed the title Update generator.go feat(generator): add Expectation structs and functions Feb 17, 2025
    gitworkflows and others added 4 commits February 17, 2025 18:29
    Co-authored-by: qodo-merge-pro-for-open-source[bot] <189517486+qodo-merge-pro-for-open-source[bot]@users.noreply.github.com>
    Signed-off-by: gitworkflows <[email protected]>
    Signed-off-by: gitworkflows <[email protected]>
    Signed-off-by: gitworkflows <[email protected]>
    Signed-off-by: gitworkflows <[email protected]>
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant