Skip to content
Open
47 changes: 46 additions & 1 deletion _codegen/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ func parseTemplates() (*template.Template, *template.Template, error) {
funcTemplate = string(f)
}
tmpl, err := template.New("function").Funcs(template.FuncMap{
"replace": strings.ReplaceAll,
"replace": strings.ReplaceAll,
"requireCommentParseIf": requireCommentParseIf,
}).Parse(funcTemplate)
if err != nil {
return nil, nil, err
Expand Down Expand Up @@ -298,6 +299,50 @@ func (f *testFunc) CommentWithoutT(receiver string) string {
return strings.Replace(f.Comment(), search, replace, -1)
}

func requireCommentParseIf(s string) string {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This method should be tested

Copy link
Collaborator

@dolmen dolmen Aug 25, 2025

Choose a reason for hiding this comment

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

This is codegen. Tests are not strictly necessary: checking the generated output is enough.

However, tests would still be helpful to document what the function expects and does. Tests will also be useful for future refactorings.

Even more helpful would be some godoc for the function because the require prefix is misleading: on a first read I got it as verb while it is about the require package.

Copy link
Collaborator

Choose a reason for hiding this comment

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

An issue is that it's unclear from the function name or the code; what is this function is supposed to do. A test or a good comment before the function would help.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That was my point, indeed

Copy link
Author

Choose a reason for hiding this comment

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

#1780 (comment)
I wasn’t familiar with the reply feature, so I didn’t realize multiple conversations were happening. 😅
Considering tests, would it be better to separate this method and handle it individually?

Copy link
Collaborator

@dolmen dolmen Sep 2, 2025

Choose a reason for hiding this comment

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

@PCloud63514 Please start by adding a comment block to document the function.

Copy link
Author

Choose a reason for hiding this comment

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

Added a doc comment for requireCommentParseIf to clarify its purpose.requireCommentParseIf

lines := strings.Split(s, "\n")
out := make([]string, 0, len(lines))
rePrefix := regexp.MustCompile(`//[ \t]*`)
ifDepth := 0
existsIfRequire := false
SomeFunctionLineIdx := 0

for i, line := range lines {
commentPrefix := rePrefix.FindString(line)
comment := strings.TrimSpace(line[2:])

if strings.HasSuffix(comment, "SomeFunction()") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't like the explicit reference to SomeFunction(). This should instead look for the first line of an example block by looking at indent. But maybe there is a reason why this wasn't enough?

Copy link
Author

@PCloud63514 PCloud63514 Aug 26, 2025

Choose a reason for hiding this comment

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

Hi @dolmen

I’ve removed the explicit SomeFunction() reference.
That logic was originally added considering the type S struct { comment,
but now the handling is simplified to focus only on if require. cases.

SomeFunctionLineIdx = i
}

if ifDepth > 0 && strings.HasPrefix(comment, "}") {
ifDepth = ifDepth - 1
continue
}

if strings.HasPrefix(comment, "if require.") && strings.HasSuffix(comment, "{") {
ifDepth = ifDepth + 1
existsIfRequire = true
comment = strings.TrimPrefix(comment, "if ")
comment = strings.TrimSpace(comment)
comment = strings.TrimSuffix(comment, "{")
}

if ifDepth > 0 {
commentPrefix = "//\t"
}

out = append(out, commentPrefix+comment)
}

if existsIfRequire && SomeFunctionLineIdx != 0 {
comment := out[SomeFunctionLineIdx][2:]
out[SomeFunctionLineIdx] = "//\t" + strings.TrimSpace(comment)
}

return strings.Join(out, "\n")
}

// Standard header https://go.dev/s/generatedcode.
var headerTemplate = `// Code generated with github.com/stretchr/testify/_codegen; DO NOT EDIT.

Expand Down
24 changes: 10 additions & 14 deletions require/require.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion require/require.go.tmpl
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{{ replace .Comment "assert." "require."}}
{{ replace .Comment "assert." "require." | requireCommentParseIf }}
func {{.DocInfo.Name}}(t TestingT, {{.Params}}) {
if h, ok := t.(tHelper); ok { h.Helper() }
if assert.{{.DocInfo.Name}}(t, {{.ForwardedParams}}) { return }
Expand Down