Skip to content

Exclude generated code from coverage #23

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.

Sign up for GitHub

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

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

Exclude generated code from coverage #23

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
490a775
Exclude generated code from coverage
breml Mar 28, 2022
3795772
Use isGenerated from golang/lint
breml Mar 29, 2022
0e4312f
Add flag to exclude generated code from coverage
breml Mar 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ error rather than a panic.
### Notest comments
Blocks or files with a `// notest` comment are excluded.

### Generated code
Generated code files, that contain the respective comment line that is
specified by the [Go Team](https://github.com/golang/go/issues/41196) in
[`go generate`](https://cs.opensource.google/go/go/+/refs/tags/go1.18:src/cmd/go/internal/generate/generate.go;l=66-73).

### Blocks returning a error tested to be non-nil
We only exclude blocks where the error being returned has been tested to be
non-nil, so:
Expand Down
19 changes: 19 additions & 0 deletions scanner/scanner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package scanner

import (
"bytes"
"fmt"
"go/ast"
"go/constant"
"go/token"
"go/types"
"os"
"regexp"

"github.com/dave/astrid"
"github.com/dave/brenda"
Expand Down Expand Up @@ -116,6 +119,8 @@ func (c *CodeMap) ScanPackages() error {
return nil
}

var doNotEditRe = regexp.MustCompile(`(?m)^// Code generated .* DO NOT EDIT\.$`)

// ScanPackage scans a single package
func (p *PackageMap) ScanPackage() error {
for _, f := range p.pkg.Syntax {
Expand All @@ -129,6 +134,20 @@ func (p *PackageMap) ScanPackage() error {
return errors.WithStack(err)
}
}

// Exclude complete files, if the code is generated.
for _, f := range p.pkg.GoFiles {
body, err := os.ReadFile(f)
if err != nil {
return errors.WithStack(err)
}
if !doNotEditRe.Match(body) {

@rubensayshi rubensayshi Mar 29, 2022 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

hmm, maybe it would be good to make the check a bit more thorough, similar to the isGenerated from the lint tool?
just to be a bit more strict / thorough and not exclude some frankenstein file with a comment floating around somewhere mid-file?

https://pkg.go.dev/cmd/go#hdr-Generate_Go_files_by_processing_source specifies:

This line must appear before the first non-comment, non-blank text in the file.

you can see the test case as well from here which things should be valid: https://github.com/golang/lint/blob/85993ffd0a6cd043291f3f63d45d656d97b165bd/lint_test.go#L292

@breml breml Mar 29, 2022

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@rubensayshi You are right and I am aware of this specification (I linked it myself in hexdigest/gowrap#20). My issue is, that one of my main use-cases is exactly with gowrap and this tool generates a valid comment, but does not (yet) comply with strict requirement for the position, because the comment is inserted after the package statement.

If you agree, I can change the detection, such that it complies with the test set you linked, which would be sufficient for me.

@breml breml Mar 29, 2022

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@rubensayshi I added the isGenerated function you linked to in a separate commit.

@rubensayshi rubensayshi Mar 30, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ehm, I suppose it's fine to adjust isGenerated to also accept package before the comment as an exception?

@breml breml Mar 30, 2022

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This is already the case in the "original" version of isGenerated, see:

courtney/scanner/generated_test.go

Line 25 in 0e4312f

{"package foo\n// Code generated by some tool. DO NOT EDIT.\ntype foo int\n", true},

I assume, this because this code predates the decision in golang/go#41196

continue
}
for i := range bytes.Split(body, []byte("\n")) {
p.addExclude(f, i+1)
}
}
return nil
}

Expand Down
14 changes: 13 additions & 1 deletion scanner/scanner_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,17 @@ func TestComments(t *testing.T) {
test(t, tests)
}

func TestGenerated(t *testing.T) {
tests := map[string]string{
"generated": `// Code generated by courtney. DO NOT EDIT.
package foo // *
func Baz() int { // *
return 0 // *
} // *`,
}
test(t, tests)
}

func test(t *testing.T, tests map[string]string) {
for name, source := range tests {
env := vos.Mock()
Expand Down Expand Up @@ -726,7 +737,8 @@ func test(t *testing.T, tests map[string]string) {
for i, line := range strings.Split(source, "\n") {
expected := strings.HasSuffix(line, "// *") ||
strings.HasSuffix(line, "//notest") ||
strings.HasSuffix(line, "// notest")
strings.HasSuffix(line, "// notest") ||
strings.HasSuffix(line, "// Code generated by courtney. DO NOT EDIT.")
if result[i+1] != expected {
t.Fatalf("Unexpected state in %s, line %d: %s\n", name, i, strconv.Quote(strings.Trim(line, "\t")))
}
Expand Down