Merge pull request #48 from github/better-json

Generate better JSON output

Author: mhagger

README.md

@@ -173,7 +173,7 @@ at the command line to view the contents of the object. (Use `--names=none` if y
 
 By default, only statistics above a minimal level of concern are reported. Use `--verbose` (as above) to request that all statistics be output. Use `--threshold=<value>` to suppress the reporting of statistics below a specified level of concern. (`<value>` is interpreted as a numerical value corresponding to the number of asterisks.) Use `--critical` to report only statistics with a critical level of concern (equivalent to `--threshold=30`).
 
-If you'd like the output in machine-readable format, including exact numbers, use the `--json` option.
+If you'd like the output in machine-readable format, including exact numbers, use the `--json` option. You can use `--json-version=1` or `--json-version=2` to choose between old and new style JSON output.
 
 To get a list of other options, run
 

counts/counts.go

@@ -14,8 +14,8 @@ func NewCount32(n uint64) Count32 {
 	return Count32(n)
 }
 
-func (n Count32) ToUint64() uint64 {
-	return uint64(n)
+func (n Count32) ToUint64() (uint64, bool) {
+	return uint64(n), n == math.MaxUint32
 }
 
 // Return the sum of two Count32s, capped at math.MaxUint32.
@@ -62,8 +62,8 @@ func NewCount64(n uint64) Count64 {
 	return Count64(n)
 }
 
-func (n Count64) ToUint64() uint64 {
-	return uint64(n)
+func (n Count64) ToUint64() (uint64, bool) {
+	return uint64(n), n == math.MaxUint64
 }
 
 // Return the sum of two Count64s, capped at math.MaxUint64.

counts/counts_test.go

@@ -11,25 +11,43 @@ import (
 func TestCount32(t *testing.T) {
 	assert := assert.New(t)
 
+	var value uint64
+	var overflow bool
+
 	c := counts.NewCount32(0)
-	assert.Equalf(uint64(0), c.ToUint64(), "NewCount32(0).ToUint64() should be 0")
+	value, overflow = c.ToUint64()
+	assert.Equalf(uint64(0), value, "NewCount32(0).ToUint64() should be 0")
+	assert.False(overflow, "NewCount32(0).ToUint64() does not overflow")
 
 	c.Increment(counts.Count32(0xf0000000))
-	assert.Equalf(uint64(0xf0000000), c.ToUint64(), "Count32(0xf0000000).ToUint64() value")
+	value, overflow = c.ToUint64()
+	assert.Equalf(uint64(0xf0000000), value, "Count32(0xf0000000).ToUint64() value")
+	assert.False(overflow, "NewCount32(0xf0000000).ToUint64() does not overflow")
 
 	c.Increment(counts.Count32(0xf0000000))
-	assert.Equalf(uint64(0xffffffff), c.ToUint64(), "Count32(0xffffffff).ToUint64() value")
+	value, overflow = c.ToUint64()
+	assert.Equalf(uint64(0xffffffff), value, "Count32(0xffffffff).ToUint64() value")
+	assert.True(overflow, "NewCount32(0xffffffff).ToUint64() overflows")
 }
 
 func TestCount64(t *testing.T) {
 	assert := assert.New(t)
 
+	var value uint64
+	var overflow bool
+
 	c := counts.NewCount64(0)
-	assert.Equalf(uint64(0), c.ToUint64(), "NewCount64(0).ToUint64() should be 0")
+	value, overflow = c.ToUint64()
+	assert.Equalf(uint64(0), value, "NewCount64(0).ToUint64() should be 0")
+	assert.False(overflow, "NewCount64(0).ToUint64() does not overflow")
 
 	c.Increment(counts.Count64(0xf000000000000000))
-	assert.Equalf(uint64(0xf000000000000000), c.ToUint64(), "Count64(0xf000000000000000).ToUint64() value")
+	value, overflow = c.ToUint64()
+	assert.Equalf(uint64(0xf000000000000000), value, "Count64(0xf000000000000000).ToUint64() value")
+	assert.False(overflow, "NewCount64(0xf000000000000000).ToUint64() does not overflow")
 
 	c.Increment(counts.Count64(0xf000000000000000))
-	assert.Equalf(uint64(0xffffffffffffffff), c.ToUint64(), "Count64(0xffffffffffffffff).ToUint64() value")
+	value, overflow = c.ToUint64()
+	assert.Equalf(uint64(0xffffffffffffffff), value, "Count64(0xffffffffffffffff).ToUint64() value")
+	assert.True(overflow, "NewCount64(0xffffffffffffffff).ToUint64() overflows")
 }

counts/human.go

@@ -2,51 +2,61 @@ package counts
 
 import (
 	"fmt"
-	"math"
 )
 
-type Prefix struct {
-	Name       string
-	Multiplier uint64
+// A quantity that can be made human-readable using Human().
+type Humanable interface {
+	// Return the value as a uint64, and a boolean telling whether it
+	// overflowed.
+	ToUint64() (uint64, bool)
 }
 
-type Humaner interface {
-	Human([]Prefix, string) (string, string)
-	ToUint64() uint64
+// An object that can format a Humanable in human-readable format.
+type Humaner struct {
+	name     string
+	prefixes []Prefix
 }
 
-var MetricPrefixes []Prefix
+type Prefix struct {
+	Name       string
+	Multiplier uint64
+}
 
-func init() {
-	MetricPrefixes = []Prefix{
+var Metric = Humaner{
+	name: "metric",
+	prefixes: []Prefix{
 		{"", 1},
 		{"k", 1e3},
 		{"M", 1e6},
 		{"G", 1e9},
 		{"T", 1e12},
 		{"P", 1e15},
-	}
+	},
 }
 
-var BinaryPrefixes []Prefix
-
-func init() {
-	BinaryPrefixes = []Prefix{
+var Binary = Humaner{
+	name: "binary",
+	prefixes: []Prefix{
 		{"", 1 << (10 * 0)},
 		{"Ki", 1 << (10 * 1)},
 		{"Mi", 1 << (10 * 2)},
 		{"Gi", 1 << (10 * 3)},
 		{"Ti", 1 << (10 * 4)},
 		{"Pi", 1 << (10 * 5)},
-	}
+	},
 }
 
-// Format values, aligned, in `len(unit) + 10` or fewer characters
-// (except for extremely large numbers).
-func Human(n uint64, prefixes []Prefix, unit string) (string, string) {
-	prefix := prefixes[0]
+func (h *Humaner) Name() string {
+	return h.name
+}
+
+// Format n, aligned, in `len(unit) + 10` or fewer characters (except
+// for extremely large numbers).
+func (h *Humaner) FormatNumber(n uint64, unit string) (string, string) {
+	prefix := h.prefixes[0]
+
 	wholePart := n
-	for _, p := range prefixes {
+	for _, p := range h.prefixes {
 		w := n / p.Multiplier
 		if w >= 1 {
 			wholePart = w
@@ -72,18 +82,13 @@ func Human(n uint64, prefixes []Prefix, unit string) (string, string) {
 	}
 }
 
-func (n Count32) Human(prefixes []Prefix, unit string) (string, string) {
-	if n == math.MaxUint32 {
+// Format values, aligned, in `len(unit) + 10` or fewer characters
+// (except for extremely large numbers).
+func (h *Humaner) Format(value Humanable, unit string) (string, string) {
+	n, overflow := value.ToUint64()
+	if overflow {
 		return "∞", unit
-	} else {
-		return Human(uint64(n), prefixes, unit)
 	}
-}
 
-func (n Count64) Human(prefixes []Prefix, unit string) (string, string) {
-	if n == math.MaxUint64 {
-		return "∞", unit
-	} else {
-		return Human(uint64(n), prefixes, unit)
-	}
+	return h.FormatNumber(n, unit)
 }

counts/human_test.go

@@ -52,18 +52,18 @@ func TestMetric(t *testing.T) {
 		{12345678900000000000, "12346", "Pcd"}, // Not ideal, but ok
 		{0xffffffffffffffff, "18447", "Pcd"},   // Not ideal, but ok
 	} {
-		number, unit := counts.Human(ht.n, counts.MetricPrefixes, "cd")
+		number, unit := counts.Metric.FormatNumber(ht.n, "cd")
 		assert.Equalf(ht.number, number, "Number for %d in metric", ht.n)
 		assert.Equalf(ht.unit, unit, "Unit for %d in metric", ht.n)
 		if ht.n < 0xffffffff {
 			c := counts.NewCount32(ht.n)
-			number, unit := c.Human(counts.MetricPrefixes, "cd")
+			number, unit := counts.Metric.Format(c, "cd")
 			assert.Equalf(ht.number, number, "Number for Count32(%d) in metric", ht.n)
 			assert.Equalf(ht.unit, unit, "Unit for Count32(%d) in metric", ht.n)
 		}
 		if ht.n < 0xffffffffffffffff {
 			c := counts.NewCount64(ht.n)
-			number, unit := c.Human(counts.MetricPrefixes, "cd")
+			number, unit := counts.Metric.Format(c, "cd")
 			assert.Equalf(ht.number, number, "Number for Count64(%d) in metric", ht.n)
 			assert.Equalf(ht.unit, unit, "Unit for Count64(%d) in metric", ht.n)
 		}
@@ -91,18 +91,18 @@ func TestBinary(t *testing.T) {
 		{1152921504606846976, "1024", "PiB"},
 		{0xffffffffffffffff, "16384", "PiB"},
 	} {
-		number, unit := counts.Human(ht.n, counts.BinaryPrefixes, "B")
+		number, unit := counts.Binary.FormatNumber(ht.n, "B")
 		assert.Equalf(ht.number, number, "Number for %d in binary", ht.n)
 		assert.Equalf(ht.unit, unit, "Unit for %d in binary", ht.n)
 		if ht.n < 0xffffffff {
 			c := counts.NewCount32(ht.n)
-			number, unit := c.Human(counts.BinaryPrefixes, "B")
+			number, unit := counts.Binary.Format(c, "B")
 			assert.Equalf(ht.number, number, "Number for Count32(%d) in binary", ht.n)
 			assert.Equalf(ht.unit, unit, "Unit for Count32(%d) in binary", ht.n)
 		}
 		if ht.n < 0xffffffffffffffff {
 			c := counts.NewCount64(ht.n)
-			number, unit := c.Human(counts.BinaryPrefixes, "B")
+			number, unit := counts.Binary.Format(c, "B")
 			assert.Equalf(ht.number, number, "Number for Count64(%d) in binary", ht.n)
 			assert.Equalf(ht.unit, unit, "Unit for Count64(%d) in binary", ht.n)
 		}
@@ -113,16 +113,16 @@ func TestLimits32(t *testing.T) {
 	assert := assert.New(t)
 
 	c := counts.NewCount32(0xffffffff)
-	number, unit := c.Human(counts.MetricPrefixes, "cd")
-	assert.Equalf("∞", number, "Number for Count32(%d) in metric", c.ToUint64())
-	assert.Equalf("cd", unit, "Unit for Count32(%d) in metric", c.ToUint64())
+	number, unit := counts.Metric.Format(c, "cd")
+	assert.Equalf("∞", number, "Number for Count32(0xffffffff) in metric")
+	assert.Equalf("cd", unit, "Unit for Count32(0xffffffff) in metric")
 }
 
 func TestLimits64(t *testing.T) {
 	assert := assert.New(t)
 
 	c := counts.NewCount64(0xffffffffffffffff)
-	number, unit := c.Human(counts.MetricPrefixes, "B")
-	assert.Equalf("∞", number, "Number for Count64(%d) in metric", c.ToUint64())
-	assert.Equalf("B", unit, "Unit for Count64(%d) in metric", c.ToUint64())
+	number, unit := counts.Metric.Format(c, "B")
+	assert.Equalf("∞", number, "Number for Count64(0xffffffffffffffff) in metric")
+	assert.Equalf("B", unit, "Unit for Count64(0xffffffffffffffff) in metric")
 }

git-sizer.go

@@ -60,57 +60,68 @@ func mainImplementation() error {
 	var nameStyle sizes.NameStyle = sizes.NameStyleFull
 	var cpuprofile string
 	var jsonOutput bool
+	var jsonVersion uint
 	var threshold sizes.Threshold = 1
 	var progress bool
 	var version bool
 
-	pflag.BoolVar(&processBranches, "branches", false, "process all branches")
-	pflag.BoolVar(&processTags, "tags", false, "process all tags")
-	pflag.BoolVar(&processRemotes, "remotes", false, "process all remote-tracking branches")
+	flags := pflag.NewFlagSet("", pflag.ContinueOnError)
 
-	pflag.VarP(
+	flags.BoolVar(&processBranches, "branches", false, "process all branches")
+	flags.BoolVar(&processTags, "tags", false, "process all tags")
+	flags.BoolVar(&processRemotes, "remotes", false, "process all remote-tracking branches")
+
+	flags.VarP(
 		sizes.NewThresholdFlagValue(&threshold, 0),
 		"verbose", "v", "report all statistics, whether concerning or not",
 	)
-	pflag.Lookup("verbose").NoOptDefVal = "true"
+	flags.Lookup("verbose").NoOptDefVal = "true"
 
-	pflag.Var(
+	flags.Var(
 		&threshold, "threshold",
 		"minimum level of concern (i.e., number of stars) that should be\n"+
 			"                              reported",
 	)
 
-	pflag.Var(
+	flags.Var(
 		sizes.NewThresholdFlagValue(&threshold, 30),
 		"critical", "only report critical statistics",
 	)
-	pflag.Lookup("critical").NoOptDefVal = "true"
+	flags.Lookup("critical").NoOptDefVal = "true"
 
-	pflag.Var(
+	flags.Var(
 		&nameStyle, "names",
 		"display names of large objects in the specified `style`:\n"+
 			"        --names=none            omit footnotes entirely\n"+
 			"        --names=hash            show only the SHA-1s of objects\n"+
 			"        --names=full            show full names",
 	)
 
-	pflag.BoolVarP(&jsonOutput, "json", "j", false, "output results in JSON format")
+	flags.BoolVarP(&jsonOutput, "json", "j", false, "output results in JSON format")
+	flags.UintVar(&jsonVersion, "json-version", 1, "JSON format version to output (1 or 2)")
 
 	atty, err := isatty.Isatty(os.Stderr.Fd())
 	if err != nil {
 		atty = false
 	}
-	pflag.BoolVar(&progress, "progress", atty, "report progress to stderr")
-	pflag.BoolVar(&version, "version", false, "report the git-sizer version number")
-	pflag.Var(&NegatedBoolValue{&progress}, "no-progress", "suppress progress output")
-	pflag.Lookup("no-progress").NoOptDefVal = "true"
+	flags.BoolVar(&progress, "progress", atty, "report progress to stderr")
+	flags.BoolVar(&version, "version", false, "report the git-sizer version number")
+	flags.Var(&NegatedBoolValue{&progress}, "no-progress", "suppress progress output")
+	flags.Lookup("no-progress").NoOptDefVal = "true"
+
+	flags.StringVar(&cpuprofile, "cpuprofile", "", "write cpu profile to file")
+	flags.MarkHidden("cpuprofile")
 
-	pflag.StringVar(&cpuprofile, "cpuprofile", "", "write cpu profile to file")
-	pflag.CommandLine.MarkHidden("cpuprofile")
+	flags.SortFlags = false
 
-	pflag.CommandLine.SortFlags = false
+	err = flags.Parse(os.Args[1:])
+	if err != nil {
+		return err
+	}
 
-	pflag.Parse()
+	if jsonOutput && !(jsonVersion == 1 || jsonVersion == 2) {
+		return fmt.Errorf("JSON version must be 1 or 2")
+	}
 
 	if cpuprofile != "" {
 		f, err := os.Create(cpuprofile)
@@ -130,7 +141,7 @@ func mainImplementation() error {
 		return nil
 	}
 
-	args := pflag.Args()
+	args := flags.Args()
 
 	if len(args) != 0 {
 		return errors.New("excess arguments")
@@ -167,11 +178,20 @@ func mainImplementation() error {
 	}
 
 	if jsonOutput {
-		s, err := json.MarshalIndent(historySize, "", "    ")
+		var j []byte
+		var err error
+		switch jsonVersion {
+		case 1:
+			j, err = json.MarshalIndent(historySize, "", "    ")
+		case 2:
+			j, err = historySize.JSON(threshold, nameStyle)
+		default:
+			return fmt.Errorf("JSON version must be 1 or 2")
+		}
 		if err != nil {
 			return fmt.Errorf("could not convert %v to json: %s", historySize, err)
 		}
-		fmt.Printf("%s\n", s)
+		fmt.Printf("%s\n", j)
 	} else {
 		io.WriteString(os.Stdout, historySize.TableString(threshold, nameStyle))
 	}

git/git.go

@@ -310,17 +310,11 @@ func PrefixFilter(prefix string) ReferenceFilter {
 }
 
 var (
-	BranchesFilter ReferenceFilter
-	TagsFilter     ReferenceFilter
-	RemotesFilter  ReferenceFilter
+	BranchesFilter ReferenceFilter = PrefixFilter("refs/heads/")
+	TagsFilter     ReferenceFilter = PrefixFilter("refs/tags/")
+	RemotesFilter  ReferenceFilter = PrefixFilter("refs/remotes/")
 )
 
-func init() {
-	BranchesFilter = PrefixFilter("refs/heads/")
-	TagsFilter = PrefixFilter("refs/tags/")
-	RemotesFilter = PrefixFilter("refs/remotes/")
-}
-
 func notNilFilters(filters ...ReferenceFilter) []ReferenceFilter {
 	var ret []ReferenceFilter
 	for _, filter := range filters {