remove unused code spotted by -coverprofile

Remove some asthelper APIs that haven't been used for some time.
They can be recovered from the git history if needed again.

One type assertion in the literals package is always true.

Embedded field objects are handled near the top of transformGo, so the
extra !obj.Embedded() check was always true. Remove it.

We always obfuscate standalone funcs now, so the obfuscatedTypesPackage
check is no longer necessary. This was necessary when we used to not
obfuscate func names when they were used in linkname directives.

The workaround for test package imports in obfuscatedTypesPackage I had
to add a few commits ago no longer seems to be necessary. This might be
thanks to the simplification with functions in the paragraph just above.

It's impossible to run garble without -trimpath nowadays, as we error
before the build even starts:

	$ go build -toolexec=garble
	go tool compile: exit status 1
	cannot open shared file, this is most likely due to not running "garble [command]"

When run as "garble build", the trimpath flag is always set. So the
check in alterTrimpath never triggers anymore, and couldn't be tested.

Finally, simplify the handling of comment syntax in printFile, and add a
few TODOs for other code paths not covered by our existing tests.

Total code coverage is up from 90.3% to 91.0%.

internal/asthelper/asthelper.go

@@ -26,22 +26,6 @@ func IntLit(value int) *ast.BasicLit {
 	}
 }
 
-// Float32Lit returns an ast.BasicLit of kind FLOAT, 32 bit
-func Float32Lit(value float32) *ast.BasicLit {
-	return &ast.BasicLit{
-		Kind:  token.FLOAT,
-		Value: strconv.FormatFloat(float64(value), 'f', -1, 32),
-	}
-}
-
-// Float64Lit returns an ast.BasicLit of kind FLOAT, 64 bit
-func Float64Lit(value float64) *ast.BasicLit {
-	return &ast.BasicLit{
-		Kind:  token.FLOAT,
-		Value: strconv.FormatFloat(value, 'f', -1, 64),
-	}
-}
-
 // IndexExpr "name[index]"
 func IndexExpr(name string, index ast.Expr) *ast.IndexExpr {
 	return &ast.IndexExpr{
@@ -81,15 +65,6 @@ func ReturnStmt(results ...ast.Expr) *ast.ReturnStmt {
 	}
 }
 
-// BoundsCheck "_ = name[pos]"
-func BoundsCheck(name string, pos int) *ast.AssignStmt {
-	return &ast.AssignStmt{
-		Lhs: []ast.Expr{ast.NewIdent("_")},
-		Tok: token.ASSIGN,
-		Rhs: []ast.Expr{IndexExpr("data", IntLit(pos))},
-	}
-}
-
 // BlockStmt a block of multiple statments e.g. a function body
 func BlockStmt(stmts ...ast.Stmt) *ast.BlockStmt {
 	return &ast.BlockStmt{List: stmts}

internal/literals/literals.go

@@ -42,11 +42,7 @@ func Obfuscate(file *ast.File, info *types.Info, fset *token.FileSet, ignoreObj
 				return true
 			}
 			for _, spec := range x.Specs {
-				spec, ok := spec.(*ast.ValueSpec)
+				spec := spec.(*ast.ValueSpec) // guaranteed for Tok==CONST
-				if !ok {
-					return false
-				}
-
 				if len(spec.Values) == 0 {
 					// skip constants with inferred values
 					return false

main.go

@@ -137,19 +137,6 @@ func obfuscatedTypesPackage(path string) *types.Package {
 		panic("called obfuscatedTypesPackage on the current package?")
 	}
 	entry, ok := importCfgEntries[path]
-
-	// A "test/bar_test [test/bar.test]" package can try to look at
-	// "test/bar [test/bar.test]", for some reason.
-	// That really shouldn't happen, because external test packages are
-	// meant to just import the original non-test package.
-	// For now, correct for this weirdness by stripping the suffix.
-	// Without this change, test.txt fails.
-	// TODO(mvdan): figure out the cause of this.
-	if !ok && strings.HasSuffix(path, ".test]") {
-		path = path[:strings.IndexByte(path, ' ')]
-		entry, ok = importCfgEntries[path]
-	}
-
 	if !ok {
 		// Handle the case where the name is defined in an indirectly
 		// imported package. Since only direct imports show up in our
@@ -281,6 +268,7 @@ func goVersionOK() bool {
 		startDateIdx := strings.IndexByte(commitAndDate, ' ') + 1
 		if startDateIdx < 0 {
 			// Custom version; assume the user knows what they're doing.
+			// TODO: cover this in a test
 			return true
 		}
 
@@ -510,11 +498,7 @@ func transformAsm(args []string) ([]string, error) {
 		flags = flagSetValue(flags, "-p", curPkg.obfuscatedImportPath())
 	}
 
-	var err error
+	flags = alterTrimpath(flags)
-	flags, err = alterTrimpath(flags)
-	if err != nil {
-		return nil, err
-	}
 
 	// We need to replace all function references with their obfuscated name
 	// counterparts.
@@ -643,10 +627,7 @@ func transformCompile(args []string) ([]string, error) {
 		}
 	}
 
-	flags, err = alterTrimpath(flags)
+	flags = alterTrimpath(flags)
-	if err != nil {
-		return nil, err
-	}
 
 	newImportCfg, err := processImportCfg(flags)
 	if err != nil {
@@ -1253,27 +1234,25 @@ func (tf *transformer) transformGo(file *ast.File) *ast.File {
 
 			// If the struct of this field was not obfuscated, do not obfuscate
 			// any of that struct's fields.
-			if !obj.Embedded() {
+			parent, ok := cursor.Parent().(*ast.SelectorExpr)
-				parent, ok := cursor.Parent().(*ast.SelectorExpr)
+			if !ok {
-				if !ok {
+				break
-					break
+			}
-				}
+			named := namedType(tf.info.TypeOf(parent.X))
-				named := namedType(tf.info.TypeOf(parent.X))
+			if named == nil {
-				if named == nil {
+				break // TODO(mvdan): add a test
-					break // TODO(mvdan): add a test
+			}
-				}
+			if name := named.Obj().Name(); strings.HasPrefix(name, "_Ctype") {
-				if name := named.Obj().Name(); strings.HasPrefix(name, "_Ctype") {
+				// A field accessor on a cgo type, such as a C struct.
-					// A field accessor on a cgo type, such as a C struct.
+				// We're not obfuscating cgo names.
-					// We're not obfuscating cgo names.
+				return true
+			}
+			if path != curPkg.ImportPath {
+				obfPkg := obfuscatedTypesPackage(path)
+				if obfPkg.Scope().Lookup(named.Obj().Name()) != nil {
+					tf.recordIgnore(named, path)
 					return true
 				}
-				if path != curPkg.ImportPath {
-					obfPkg := obfuscatedTypesPackage(path)
-					if obfPkg.Scope().Lookup(named.Obj().Name()) != nil {
-						tf.recordIgnore(named, path)
-						return true
-					}
-				}
 			}
 		case *types.TypeName:
 			if parentScope != pkg.Scope() {
@@ -1306,17 +1285,6 @@ func (tf *transformer) transformGo(file *ast.File) *ast.File {
 			if strings.HasPrefix(node.Name, "Test") && isTestSignature(sign) {
 				return true // don't break tests
 			}
-
-			// If this is an imported func that was linknamed to a
-			// different symbol name, the imported package did not
-			// obfuscate the original func name.
-			// Don't do it here either.
-			if path != curPkg.ImportPath {
-				obfPkg := obfuscatedTypesPackage(path)
-				if obfPkg.Scope().Lookup(obj.Name()) != nil {
-					return true
-				}
-			}
 		default:
 			return true // we only want to rename the above
 		}
@@ -1511,19 +1479,15 @@ func splitFlagsFromArgs(all []string) (flags, args []string) {
 	return all, nil
 }
 
-func alterTrimpath(flags []string) ([]string, error) {
+func alterTrimpath(flags []string) []string {
 	// If the value of -trimpath doesn't contain the separator ';', the 'go
 	// build' command is most likely not using '-trimpath'.
 	trimpath := flagValue(flags, "-trimpath")
-	if !strings.Contains(trimpath, ";") {
-		return nil, fmt.Errorf("-toolexec=garble should be used alongside -trimpath")
-	}
 
 	// Add our temporary dir to the beginning of -trimpath, so that we don't
 	// leak temporary dirs. Needs to be at the beginning, since there may be
 	// shorter prefixes later in the list, such as $PWD if TMPDIR=$PWD/tmp.
-	flags = flagSetValue(flags, "-trimpath", sharedTempDir+"=>;"+trimpath)
+	return flagSetValue(flags, "-trimpath", sharedTempDir+"=>;"+trimpath)
-	return flags, nil
 }
 
 // buildFlags is obtained from 'go help build' as of Go 1.15.

position.go

@@ -135,17 +135,14 @@ func printFile(file1 *ast.File) ([]byte, error) {
 		buf2.Write(src[copied:comment.offset])
 		copied = comment.offset
 
+		// We assume that all comments are of the form "/*text*/".
 		// Make sure there is whitespace at either side of a comment.
 		// Otherwise, we could change the syntax of the program.
 		// Inserting "/*text*/" in "a/b" // must be "a/ /*text*/ b",
 		// as "a//*text*/b" is tokenized as a "//" comment.
 		buf2.WriteByte(' ')
 		buf2.WriteString(comment.text)
-		if strings.HasPrefix(comment.text, "//") {
+		buf2.WriteByte(' ')
-			buf2.WriteByte('\n')
-		} else {
-			buf2.WriteByte(' ')
-		}
 	}
 	buf2.Write(src[copied:])
 	return buf2.Bytes(), nil

reverse.go

@@ -149,6 +149,7 @@ func commandReverse(args []string) error {
 		}
 		return nil
 	}
+	// TODO: cover this code in the tests too
 	anyModified := false
 	for _, path := range args {
 		f, err := os.Open(path)