🚀

Author: gbbr

README.md

@@ -1,6 +1,6 @@
 # breakcheck
 
-breackcheck checks exported values, types and function declarations in your working tree for potential breaking changes against a given git reference. 
+breakcheck checks exported values, types and function declarations in your working tree for potential breaking changes against a given git reference. 
 
 ## Usage
 
@@ -23,19 +23,32 @@ Flags:
 Below is an example output from running breakcheck against the datadog-agent repository:
 
 ```
-$ breakcheck --base=head~40
+$ breakcheck --base=HEAD~40
 pkg/util/clusteragent:
-	• removed: struct DCAClient, field ClusterAgentAPIEndpoint
-	• changed: struct DCAClient, field ClusterAgentVersion type from string to version.Version
-	•  before: func GetClusterAgentClient() (*DCAClient, error)
-	      now: func GetClusterAgentClient() (DCAClientInterface, error)
-	           (return value 0 changed from *DCAClient to DCAClientInterface)
-
-pkg/logs/config:
-	• removed: const ContainerdType 
+  
+• Removed struct field "ClusterAgentAPIEndpoint":
+    - clusteragent.go:42@HEAD~40:
+        struct DCAClient
+    - clusteragent.go:57:
+        struct DCAClient
+  
+• Struct field "ClusterAgentVersion" type changed from string to version.Version:
+    - clusteragent.go:42@HEAD~40:
+        DCAClient
+  
+• Return value (0) changed from *DCAClient to DCAClientInterface:
+    - clusteragent.go:60@HEAD~40:
+        func GetClusterAgentClient() (*DCAClient, error)
+    - clusteragent.go:75:
+        func GetClusterAgentClient() (DCAClientInterface, error)
 ```
 
 ## Caveats
 
 * If a function's argument is changed to an alias of the same type, breakcheck will fail to detect this and will report it as a change. Technically this is not a breaking change.
 * Detecting changes in exported package level value declarations is limited to their name and type (when known). 
+
+## Similar work
+
+* https://golang.org/x/tools/internal/apidiff
+* https://github.com/bradleyfalzon/apicompat

comparer.go

@@ -3,18 +3,23 @@ package main
 import (
 	"fmt"
 	"go/ast"
+	"go/token"
 	"strings"
 )
 
-func newDeclComparer(summary *packageSummary) *declComparer {
+func newDeclComparer(fset *token.FileSet, path string, summary *packageSummary) *declComparer {
 	return &declComparer{
+		fset: fset,
+		path: path,
 		head: summary,
 	}
 }
 
 // declComparer is an ast.Visitor which ensures that all encountered
 // declarations have been unmodified in the provided summary.
 type declComparer struct {
+	fset   *token.FileSet
+	path   string // relative folder
 	head   *packageSummary
 	report strings.Builder
 }
@@ -47,20 +52,26 @@ func (c *declComparer) Visit(node ast.Node) ast.Visitor {
 func (c *declComparer) compareType(base *ast.TypeSpec) {
 	head, ok := c.head.types[base.Name.Name]
 	if !ok {
-		c.logf("• removed: type %s", base.Name.Name)
+		c.logf("\n• Removed in working tree:")
+		c.logPosition(base, true)
+		c.logf("      type %s", base.Name.Name)
 		return
 	}
 	if x, ok := base.Type.(*ast.StructType); ok {
 		// compare structs, allow adding new fields
 		y, ok := head.Type.(*ast.StructType)
 		if !ok {
-			c.logf("• changed: struct %s type changed to %s", base.Name.Name, describeType(head.Type))
+			c.logf("\n• Type changed to %s:", describeType(head.Type))
+			c.logPosition(base, true)
+			c.logf("      type %s", base.Name.Name)
 		}
 		c.compareStructs(base.Name.Name, x, y)
 		return
 	}
 	if a, b := describeType(base.Type), describeType(head.Type); a != b {
-		c.logf("• changed: type %s from %s to %s", base.Name.Name, a, b)
+		c.logf("\n• Type changed from %s to %s:", a, b)
+		c.logPosition(base, true)
+		c.logf("      type %s", base.Name.Name)
 	}
 }
 
@@ -70,13 +81,19 @@ func (c *declComparer) compareStructs(structName string, base, head *ast.StructT
 			for _, n := range f.Names {
 				if n.Name == name {
 					if a, b := describeType(f.Type), describeType(typ); a != b {
-						c.logf("• changed: struct %s, field %s type from %s to %s", structName, name, b, a)
+						c.logf("\n• Struct field %q type changed from %s to %s:", name, b, a)
+						c.logPosition(base, true)
+						c.logf("      %s", structName)
 					}
 					return
 				}
 			}
 		}
-		c.logf("• removed: struct %s, field %s", structName, name)
+		c.logf("\n• Removed struct field %q:", name)
+		c.logPosition(base, true)
+		c.logf("      struct %s", structName)
+		c.logPosition(head, false)
+		c.logf("      struct %s", structName)
 	}
 	for _, field := range base.Fields.List {
 		for _, name := range field.Names {
@@ -89,11 +106,15 @@ func (c *declComparer) compareValue(base *ast.ValueSpec) {
 	for _, name := range base.Names {
 		head, ok := c.head.value[name.Name]
 		if !ok {
-			c.logf("• removed: %s", printValue(base))
+			c.logf("\n• Value removed in working tree:")
+			c.logPosition(base, true)
+			c.logf("      %s", printValue(base))
 			return
 		}
 		if a, b := describeType(head.Type), describeType(base.Type); a != b {
-			c.logf("• changed: value type %s from %s to %s", name.Name, b, a)
+			c.logf("\n• Value type changed from %s to %s in working tree:", b, a)
+			c.logPosition(base, true)
+			c.logf("      %s", name.Name)
 		}
 		if base.Type == nil {
 			// If the type is nil, this is likely an assignment where the type is inferred
@@ -111,7 +132,9 @@ func (c *declComparer) compareFunc(base *ast.FuncDecl) {
 	head, ok := c.head.funcs[name]
 	if !ok {
 		// func not found
-		c.logf("• removed: %s", printFunc(base.Recv, base.Name, base.Type))
+		c.logf("\n• Func removed:")
+		c.logPosition(base, true)
+		c.logf("      %s", printFunc(base.Recv, base.Name, base.Type))
 		return
 	}
 	headArgs := head.Type.Params.List
@@ -120,46 +143,61 @@ func (c *declComparer) compareFunc(base *ast.FuncDecl) {
 		// if there's only one new argument in base, and that argument
 		// is variadic, then this isn't a breaking change
 		if diff != 1 || !strings.HasPrefix(describeType(headArgs[len(headArgs)-1].Type), "...") {
-			c.logFuncChange(base, head, "change in argument count")
+			c.logFuncChange(base, head, "Change in argument count")
 			return
 		}
 	}
 	for i, arg := range baseArgs {
 		if a, b := describeType(arg.Type), describeType(headArgs[i].Type); a != b {
-			c.logFuncChange(base, head, fmt.Sprintf("argument %d changed from %s to %s", i, a, b))
+			c.logFuncChange(base, head, fmt.Sprintf("Argument (%d) changed from %s to %s", i, a, b))
 			return
 		}
 	}
 	baseResults := base.Type.Results
 	headResults := head.Type.Results
 	if baseResults == nil && headResults != nil {
-		c.logFuncChange(base, head, "return values were added")
+		c.logFuncChange(base, head, "Return values were added")
 		return
 	}
 	if baseResults != nil && headResults == nil {
-		c.logFuncChange(base, head, "return values were removed")
+		c.logFuncChange(base, head, "Return values were removed")
 		return
 	}
 	if baseResults == nil && headResults == nil {
 		// OK
 		return
 	}
 	if len(baseResults.List) != len(headResults.List) {
-		c.logFuncChange(base, head, "change in return value count")
+		c.logFuncChange(base, head, "Change in return value count")
 		return
 	}
 	for i, arg := range baseResults.List {
 		if a, b := describeType(arg.Type), describeType(headResults.List[i].Type); a != b {
-			c.logFuncChange(base, head, fmt.Sprintf("return value %d changed from %s to %s", i, a, b))
+			c.logFuncChange(base, head, fmt.Sprintf("Return value (%d) changed from %s to %s", i, a, b))
 			return
 		}
 	}
 }
 
+func (c *declComparer) logPosition(node ast.Node, base bool) {
+	fset := c.fset
+	path := c.path
+	gitr := "@" + *baseRef
+	if !base {
+		fset = c.head.fset
+		path = c.head.path
+		gitr = ""
+	}
+	pos := fset.Position(node.Pos())
+	c.logf("  - %s:%d%s:", strings.TrimPrefix(pos.Filename, path+"/"), pos.Line, gitr)
+}
+
 func (c *declComparer) logFuncChange(base, head *ast.FuncDecl, reason string) {
-	c.logf("•  before: %s", printFunc(base.Recv, base.Name, base.Type))
-	c.logf("      now: %s", printFunc(head.Recv, head.Name, head.Type))
-	c.logf("           (%s)", reason)
+	c.logf("\n• %s:", reason)
+	c.logPosition(base, true)
+	c.logf("      %s", printFunc(base.Recv, base.Name, base.Type))
+	c.logPosition(head, false)
+	c.logf("      %s", printFunc(head.Recv, head.Name, head.Type))
 }
 
 func (c *declComparer) logf(format string, args ...interface{}) {
@@ -168,7 +206,7 @@ func (c *declComparer) logf(format string, args ...interface{}) {
 		c.report.WriteByte(':')
 		c.report.WriteByte('\n')
 	}
-	c.report.WriteByte('\t')
+	c.report.WriteString("  ")
 	c.report.WriteString(fmt.Sprintf(format, args...))
 	c.report.WriteByte('\n')
 }

go.mod

@@ -1,5 +1,3 @@
 module github.com/gbbr/breakcheck
 
 go 1.12
-
-require github.com/kr/pretty v0.1.0

go.sum

@@ -1,5 +0,0 @@
-github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI=
-github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
-github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
-github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
-github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=

main.go

@@ -13,7 +13,7 @@ import (
 )
 
 var (
-	baseRef  = flag.String("base", "head", "git reference to compare against")
+	baseRef  = flag.String("base", "HEAD", "git reference to compare against")
 	verbose  = flag.Bool("v", false, "enable verbose mode")
 	privRecv = flag.Bool("private", false, "include exported methods with private receivers")
 )
@@ -82,7 +82,7 @@ func main() {
 		}
 
 		// create working path summary
-		summary := newPackageSummary(dir)
+		summary := newPackageSummary(fsetHead, dir)
 		for _, d := range list {
 			if d.IsDir() || !strings.HasSuffix(d.Name(), ".go") || strings.HasSuffix(d.Name(), "_test.go") {
 				continue
@@ -99,7 +99,7 @@ func main() {
 		}
 
 		// scan base, everything found there should exist in working path
-		comparer := newDeclComparer(summary)
+		comparer := newDeclComparer(fsetBase, dir, summary)
 		blobs, err := gitLsTreeGoBlobs(*baseRef, dir)
 		if err != nil {
 			log.Fatal(err)

summary.go

@@ -2,19 +2,22 @@ package main
 
 import (
 	"go/ast"
+	"go/token"
 	"strings"
 )
 
 type packageSummary struct {
+	fset  *token.FileSet
 	path  string                    // relative folder
 	funcs map[string]*ast.FuncDecl  // maps (recv.)name to func
 	types map[string]*ast.TypeSpec  // maps type name to spec
 	value map[string]*ast.ValueSpec // maps value name to spec
 	// TODO: add package name, ensure it wasn't changed
 }
 
-func newPackageSummary(path string) *packageSummary {
+func newPackageSummary(fset *token.FileSet, path string) *packageSummary {
 	return &packageSummary{
+		fset:  fset,
 		path:  path,
 		funcs: make(map[string]*ast.FuncDecl),
 		value: make(map[string]*ast.ValueSpec),