Skip to content

Commit

Permalink
cmd/errtrace: Fix blank import handling (#92)
Browse files Browse the repository at this point in the history
With toolexec mode, a blank import of errtrace is more likely to opt-in
to rewriting. However, this blank import can't be used to call errtrace
functions, so we need to track both:

 * Whether errtrace is imported (used to opt-in toolexec rewriting)
 * Whether errtrace needs to be imported for any Wrap calls.

Since this change modifies the rewriting logic, we bump the errtrace
toolexec version manually.
  • Loading branch information
prashantv committed Feb 18, 2024
1 parent 2fb2821 commit 02aad23
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 10 deletions.
16 changes: 11 additions & 5 deletions cmd/errtrace/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ type parsedFile struct {
file *ast.File

errtracePkg string
importsErrtrace bool
importsErrtrace bool // includes blank imports
inserts []insert
unusedOptouts []int // list of line numbers
}
Expand All @@ -410,21 +410,27 @@ func (cmd *mainCmd) parseFile(filename string, src []byte) (parsedFile, error) {
return parsedFile{}, errtrace.Wrap(err)
}

errtracePkg := "errtrace" // name to use for errtrace package
var importsErrtrace bool // whether the file imports errtrace already
errtracePkg := "errtrace" // name to use for errtrace package
var importsErrtrace bool // whether there's any errtrace import, including blank imports
needErrtraceImport := true // whether to add a new import.
for _, imp := range f.Imports {
if imp.Path.Value == `"braces.dev/errtrace"` {
importsErrtrace = true
if imp.Name != nil {
if imp.Name.Name == "_" {
// Can't use a blank import, keep processing imports.
continue
}
// If the file already imports errtrace,
// we'll want to use the name it's imported under.
errtracePkg = imp.Name.Name
}
needErrtraceImport = false
break
}
}

if !importsErrtrace {
if needErrtraceImport {
// If the file doesn't import errtrace already,
// do a quick check to find an unused identifier name.
idents := make(map[string]struct{})
Expand Down Expand Up @@ -475,7 +481,7 @@ func (cmd *mainCmd) parseFile(filename string, src []byte) (parsedFile, error) {
// If errtrace isn't imported, but at least one insert was made,
// we'll need to import errtrace.
// Add an import declaration to the file.
if !importsErrtrace && len(inserts) > 0 {
if needErrtraceImport && len(inserts) > 0 {
// We want to insert the import after the last existing import.
// If the last import is part of a group, we'll make it part of the group.
//
Expand Down
17 changes: 17 additions & 0 deletions cmd/errtrace/testdata/golden/imported_blank.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
//go:build ignore

package foo

import (
"strconv"

_ "braces.dev/errtrace"
)

func Unwrapped(s string) (int, error) {
i, err := strconv.Atoi(s)
if err != nil {
return 0, err
}
return i + 42, nil
}
17 changes: 17 additions & 0 deletions cmd/errtrace/testdata/golden/imported_blank.go.golden
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
//go:build ignore

package foo

import (
"strconv"

_ "braces.dev/errtrace"; "braces.dev/errtrace"
)

func Unwrapped(s string) (int, error) {
i, err := strconv.Atoi(s)
if err != nil {
return 0, errtrace.Wrap(err)
}
return i + 42, nil
}
4 changes: 0 additions & 4 deletions cmd/errtrace/testdata/toolexec-test/errtrace.go

This file was deleted.

1 change: 1 addition & 0 deletions cmd/errtrace/testdata/toolexec-test/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
"fmt"

_ "braces.dev/errtrace" // Opt-in to errtrace wrapping with toolexec.
"braces.dev/errtrace/cmd/errtrace/testdata/toolexec-test/p1"
)

Expand Down
2 changes: 1 addition & 1 deletion cmd/errtrace/toolexec.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func (cmd *mainCmd) toolExecVersion(args []string) int {
}

// TODO: This version number should change whenever the rewriting changes.
fmt.Fprintf(cmd.Stdout, "%s-errtrace0\n", strings.TrimSpace(stdout.String()))
fmt.Fprintf(cmd.Stdout, "%s-errtrace1\n", strings.TrimSpace(stdout.String()))
return 0
}

Expand Down

0 comments on commit 02aad23

Please sign in to comment.