From 1c564ef091e66eb3e4d2cefc3fb8ccb0bab9a97c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Fri, 25 Mar 2022 16:40:06 +0000 Subject: [PATCH] slightly improve code thanks to Go 1.18 APIs strings.Cut makes some string handling code more intuitive. Note that we can't use it everywhere, as some places need LastIndexByte. Start using x/exp/slices, too, which is our first use of generics. Note that its API is experimental and may still change, but since we are not a library, we can control its version updates. I also noticed that we were using TrimSpace for importcfg files. It's actually unnecessary if we swap strings.SplitAfter for Split, as the only whitespace present was the trailing newline. While here, I noticed an unused copy of printfWithoutPackage. --- go.mod | 1 + go.sum | 2 + main.go | 83 +++++++++++++----------------------- position.go | 7 +-- reverse.go | 4 +- testdata/scripts/imports.txt | 5 --- testdata/scripts/reflect.txt | 4 +- 7 files changed, 41 insertions(+), 65 deletions(-) diff --git a/go.mod b/go.mod index e1fbeb6..916243f 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( github.com/frankban/quicktest v1.14.2 github.com/google/go-cmp v0.5.7 github.com/rogpeppe/go-internal v1.8.1 + golang.org/x/exp v0.0.0-20220325121720-054d8573a5d8 golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3 golang.org/x/tools v0.1.10 ) diff --git a/go.sum b/go.sum index 68f903e..bd71cbd 100644 --- a/go.sum +++ b/go.sum @@ -15,6 +15,8 @@ github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsK github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc= github.com/rogpeppe/go-internal v1.8.1 h1:geMPLpDpQOgVyCg5z5GoRwLHepNdb71NXb67XFkP+Eg= github.com/rogpeppe/go-internal v1.8.1/go.mod h1:JeRgkft04UBgHMgCIwADu4Pn6Mtm5d4nPKWu0nJ5d+o= +golang.org/x/exp v0.0.0-20220325121720-054d8573a5d8 h1:Xt4/LzbTwfocTk9ZLEu4onjeFucl88iW+v4j4PWbQuE= +golang.org/x/exp v0.0.0-20220325121720-054d8573a5d8/go.mod h1:lgLbSvA5ygNOMpwM/9anMpWVlVJ7Z+cHWq/eFuinpGE= golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3 h1:kQgndtyPBW/JIYERgdxfwMYh3AVStj88WQTlNDi2a+o= golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3/go.mod h1:3p9vT2HGsQu2K1YbXdKPJLVgG5VJdoTa1poYQBtP1AY= golang.org/x/sys v0.0.0-20211019181941-9d821ace8654 h1:id054HUawV2/6IGm2IV8KZQjqtwAOo2CYlOToYqa0d0= diff --git a/main.go b/main.go index 15647e1..2ca59b6 100644 --- a/main.go +++ b/main.go @@ -34,6 +34,7 @@ import ( "unicode" "unicode/utf8" + "golang.org/x/exp/slices" "golang.org/x/mod/modfile" "golang.org/x/mod/semver" "golang.org/x/tools/go/ast/astutil" @@ -549,16 +550,8 @@ func transformAsm(args []string) ([]string, error) { // If the assembler is running just for -gensymabis, // don't obfuscate the source, as we are not assembling yet. // The assembler will run again later; obfuscating twice is just wasteful. - symabis := false - for _, arg := range args { - if arg == "-gensymabis" { - symabis = true - break - } - } newPaths := make([]string, 0, len(paths)) - if !symabis { - var newPaths []string + if !slices.Contains(args, "-gensymabis") { for _, path := range paths { name := filepath.Base(path) pkgDir := filepath.Join(sharedTempDir, filepath.FromSlash(curPkg.ImportPath)) @@ -578,7 +571,6 @@ func transformAsm(args []string) ([]string, error) { middleDotLen := utf8.RuneLen(middleDot) for _, path := range paths { - // Read the entire file into memory. // If we find issues with large files, we can use bufio. content, err := os.ReadFile(path) @@ -899,33 +891,26 @@ func processImportCfg(flags []string) (newImportCfg string, _ error) { var packagefiles, importmaps [][2]string - for _, line := range strings.SplitAfter(string(data), "\n") { - line = strings.TrimSpace(line) + for _, line := range strings.Split(string(data), "\n") { if line == "" || strings.HasPrefix(line, "#") { continue } - i := strings.IndexByte(line, ' ') - if i < 0 { + verb, args, found := strings.Cut(line, " ") + if !found { continue } - verb := line[:i] switch verb { case "importmap": - args := strings.TrimSpace(line[i+1:]) - j := strings.IndexByte(args, '=') - if j < 0 { + beforePath, afterPath, found := strings.Cut(args, "=") + if !found { continue } - beforePath, afterPath := args[:j], args[j+1:] importmaps = append(importmaps, [2]string{beforePath, afterPath}) case "packagefile": - args := strings.TrimSpace(line[i+1:]) - j := strings.IndexByte(args, '=') - if j < 0 { + importPath, objectPath, found := strings.Cut(args, "=") + if !found { continue } - importPath, objectPath := args[:j], args[j+1:] - packagefiles = append(packagefiles, [2]string{importPath, objectPath}) } } @@ -1187,16 +1172,15 @@ func (tf *transformer) prefillObjectMaps(files []*ast.File) error { return err } flagValueIter(ldflags, "-X", func(val string) { - // val is in the form of "importpath.name=value". - i := strings.IndexByte(val, '=') - if i < 0 { + // val is in the form of "foo.com/bar.name=value". + fullName, stringValue, found := strings.Cut(val, "=") + if !found { return // invalid } - stringValue := val[i+1:] - val = val[:i] // "importpath.name" - i = strings.LastIndexByte(val, '.') - path, name := val[:i], val[i+1:] + // fullName is "foo.com/bar.name" + i := strings.LastIndexByte(fullName, '.') + path, name := fullName[:i], fullName[i+1:] // -X represents the main package as "main", not its import path. if path != curPkg.ImportPath && !(path == "main" && curPkg.Name == "main") { @@ -1793,26 +1777,22 @@ func transformLink(args []string) ([]string, error) { // To cover both obfuscated and non-obfuscated names, // duplicate each flag with a obfuscated version. flagValueIter(flags, "-X", func(val string) { - // val is in the form of "pkg.name=str" - i := strings.IndexByte(val, '=') - if i <= 0 { - return - } - name := val[:i] - str := val[i+1:] - j := strings.LastIndexByte(name, '.') - if j <= 0 { - return + // val is in the form of "foo.com/bar.name=value". + fullName, stringValue, found := strings.Cut(val, "=") + if !found { + return // invalid } - pkg := name[:j] - name = name[j+1:] + + // fullName is "foo.com/bar.name" + i := strings.LastIndexByte(fullName, '.') + path, name := fullName[:i], fullName[i+1:] // If the package path is "main", it's the current top-level // package we are linking. // Otherwise, find it in the cache. lpkg := curPkg - if pkg != "main" { - lpkg = cache.ListedPackages[pkg] + if path != "main" { + lpkg = cache.ListedPackages[path] } if lpkg == nil { // We couldn't find the package. @@ -1821,12 +1801,12 @@ func transformLink(args []string) ([]string, error) { return } // As before, the main package must remain as "main". - newPkg := pkg - if pkg != "main" { - newPkg = lpkg.obfuscatedImportPath() + newPath := path + if path != "main" { + newPath = lpkg.obfuscatedImportPath() } newName := hashWithPackage(lpkg, name) - flags = append(flags, fmt.Sprintf("-X=%s.%s=%s", newPkg, newName, str)) + flags = append(flags, fmt.Sprintf("-X=%s.%s=%s", newPath, newName, stringValue)) }) // Starting in Go 1.17, Go's version is implicitly injected by the linker. @@ -1938,10 +1918,7 @@ func filterForwardBuildFlags(flags []string) (filtered []string, firstUnknown st arg = arg[1:] // "--name" to "-name"; keep the short form } - name := arg - if i := strings.IndexByte(arg, '='); i > 0 { - name = arg[:i] // "-name=value" to "-name" - } + name, _, _ := strings.Cut(arg, "=") // "-name=value" to "-name" buildFlag := forwardBuildFlags[name] if buildFlag { diff --git a/position.go b/position.go index c48e4c9..60b7019 100644 --- a/position.go +++ b/position.go @@ -10,8 +10,9 @@ import ( "go/parser" "go/printer" "path/filepath" - "sort" "strings" + + "golang.org/x/exp/slices" ) func isDirective(text string) bool { @@ -120,8 +121,8 @@ func printFile(file1 *ast.File) ([]byte, error) { }) // We add comments in order. - sort.Slice(toAdd, func(i, j int) bool { - return toAdd[i].offset < toAdd[j].offset + slices.SortFunc(toAdd, func(a, b commentToAdd) bool { + return a.offset < b.offset }) copied := 0 diff --git a/reverse.go b/reverse.go index 614337d..dff72f9 100644 --- a/reverse.go +++ b/reverse.go @@ -162,11 +162,11 @@ One can reverse a captured panic stack trace as follows: return err } defer f.Close() - any, err := reverseContent(os.Stdout, f, repl) + modified, err := reverseContent(os.Stdout, f, repl) if err != nil { return err } - anyModified = anyModified || any + anyModified = anyModified || modified f.Close() // since we're in a loop } if !anyModified { diff --git a/testdata/scripts/imports.txt b/testdata/scripts/imports.txt index 6f29a6e..008d3cd 100644 --- a/testdata/scripts/imports.txt +++ b/testdata/scripts/imports.txt @@ -73,7 +73,6 @@ package main import ( "fmt" - "strings" "test/main/importedpkg" @@ -92,10 +91,6 @@ func main() { fmt.Println(quote.Go()) garbletest.Test() } - -func printfWithoutPackage(format string, v any) { - fmt.Print(strings.Split(fmt.Sprintf(format, v), ".")[1]) -} -- notag_fail.go -- // +build !buildtag diff --git a/testdata/scripts/reflect.txt b/testdata/scripts/reflect.txt index b49f72e..19fc6e0 100644 --- a/testdata/scripts/reflect.txt +++ b/testdata/scripts/reflect.txt @@ -113,8 +113,8 @@ type EmbeddingIndirect struct { func printfWithoutPackage(format string, v any) { s := fmt.Sprintf(format, v) - if i := strings.IndexByte(s, '.'); i > 0 { - s = s[i+1:] + if _, without, found := strings.Cut(s, "."); found { + s = without } fmt.Print(s) }