Repository: uber/prototool
Pull request #282 (2019-01-10 00:52:13)

Add --diff-lint-groups flag to prototool lint
This is something that you commonly want to know:

```
$ prototool lint --diff-lint-groups uber,uber2
< ENUM_FIELD_PREFIXES
< ENUM_ZERO_VALUES_INVALID
< FILE_OPTIONS_EQUAL_GO_PACKAGE_PB_SUFFIX
> ENUM_FIELD_PREFIXES_EXCEPT_MESSAGE
> ENUM_ZERO_VALUES_INVALID_EXCEPT_MESSAGE
> FIELDS_NOT_RESERVED
> FILE_OPTIONS_EQUAL_GO_PACKAGE_LAST_TWO_SUFFIX
> GOGO_NOT_IMPORTED
> IMPORTS_NOT_PUBLIC
> IMPORTS_NOT_WEAK
> MESSAGE_FIELDS_NOT_FLOATS
> MESSAGE_FIELDS_NO_JSON_NAME
> PACKAGE_MAJOR_BETA_VERSIONED
> REQUEST_RESPONSE_NAMES_MATCH_RPC
```

codecov-io: # [Codecov](https://codecov.io/gh/uber/prototool/pull/282?src=pr&el=h1) Report
> Merging [#282](https://codecov.io/gh/uber/prototool/pull/282?src=pr&el=desc) into [dev](https://codecov.io/gh/uber/prototool/commit/99cbc8734c47725683b35385a76148330a0a8e95?src=pr&el=desc) will **decrease** coverage by `0.43%`.
> The diff coverage is `12.19%`.

[![Impacted file tree graph](https://codecov.io/gh/uber/prototool/pull/282/graphs/tree.svg?width=650&token=A9V0DApqZg&height=150&src=pr)](https://codecov.io/gh/uber/prototool/pull/282?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##              dev     #282      +/-   ##
==========================================
- Coverage   72.15%   71.72%   -0.44%     
==========================================
  Files         120      120              
  Lines        5413     5450      +37     
==========================================
+ Hits         3906     3909       +3     
- Misses       1135     1168      +33     
- Partials      372      373       +1
```


| [Impacted Files](https://codecov.io/gh/uber/prototool/pull/282?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [internal/exec/exec.go](https://codecov.io/gh/uber/prototool/pull/282/diff?src=pr&el=tree#diff-aW50ZXJuYWwvZXhlYy9leGVjLmdv) | `57.14% <ø> (ø)` | :arrow_up: |
| [internal/cmd/templates.go](https://codecov.io/gh/uber/prototool/pull/282/diff?src=pr&el=tree#diff-aW50ZXJuYWwvY21kL3RlbXBsYXRlcy5nbw==) | `82.04% <100%> (+0.07%)` | :arrow_up: |
| [internal/cmd/flags.go](https://codecov.io/gh/uber/prototool/pull/282/diff?src=pr&el=tree#diff-aW50ZXJuYWwvY21kL2ZsYWdzLmdv) | `100% <100%> (ø)` | :arrow_up: |
| [internal/exec/runner.go](https://codecov.io/gh/uber/prototool/pull/282/diff?src=pr&el=tree#diff-aW50ZXJuYWwvZXhlYy9ydW5uZXIuZ28=) | `43.03% <2.7%> (-2.45%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/uber/prototool/pull/282?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/uber/prototool/pull/282?src=pr&el=footer). Last update [99cbc87...753d030](https://codecov.io/gh/uber/prototool/pull/282?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/README.md b/README.md
index 923a2e6..83efb8a 100644
--- a/README.md
+++ b/README.md
@@ -177,6 +177,12 @@ Lint rules can be set using the configuration file. See the configuration at [et
 - `uber2`: This lint group is the v2 of the `uber` lint group, and makes some modifcations to more closely follow the Google Cloud APIs file
   structure, as well as adding even more rules to enforce more consistent development patterns. This lint group is under development.
 
+To see the differences between lint groups, use the `--diff-lint-groups` flag:
+
+```
+prototool lint --diff-lint-groups google,uber
+```
+
 Configuration of your group can be done by setting the `lint.group` option in your `prototool.yaml` file:
 
 ```yaml
diff --git a/internal/cmd/flags.go b/internal/cmd/flags.go
index 38916d5..c16e85a 100644
--- a/internal/cmd/flags.go
+++ b/internal/cmd/flags.go
@@ -32,6 +32,7 @@ type flags struct {
 	connectTimeout    string
 	data              string
 	debug             bool
+	diffLintGroups    string
 	diffMode          bool
 	disableFormat     bool
 	disableLint       bool
@@ -88,6 +89,10 @@ func (f *flags) bindDebug(flagSet *pflag.FlagSet) {
 	flagSet.BoolVar(&f.debug, "debug", false, "Run in debug mode, which will print out debug logging.")
 }
 
+func (f *flags) bindDiffLintGroups(flagSet *pflag.FlagSet) {
+	flagSet.StringVar(&f.diffLintGroups, "diff-lint-groups", "", "Diff the two lint groups separated by '.', for example google,uber.")
+}
+
 func (f *flags) bindDiffMode(flagSet *pflag.FlagSet) {
 	flagSet.BoolVarP(&f.diffMode, "diff", "d", false, "Write a diff instead of writing the formatted file to stdout.")
 }
diff --git a/internal/cmd/templates.go b/internal/cmd/templates.go
index d183d60..ce47975 100644
--- a/internal/cmd/templates.go
+++ b/internal/cmd/templates.go
@@ -458,7 +458,7 @@ sys	0m0.924s`,
 
 		Args: cobra.MaximumNArgs(1),
 		Run: func(runner exec.Runner, args []string, flags *flags) error {
-			return runner.Lint(args, flags.listAllLinters, flags.listLinters, flags.listAllLintGroups, flags.listLintGroup)
+			return runner.Lint(args, flags.listAllLinters, flags.listLinters, flags.listAllLintGroups, flags.listLintGroup, flags.diffLintGroups)
 		},
 		BindFlags: func(flagSet *pflag.FlagSet, flags *flags) {
 			flags.bindCachePath(flagSet)
@@ -468,6 +468,7 @@ sys	0m0.924s`,
 			flags.bindListLinters(flagSet)
 			flags.bindListAllLintGroups(flagSet)
 			flags.bindListLintGroup(flagSet)
+			flags.bindDiffLintGroups(flagSet)
 			flags.bindProtocURL(flagSet)
 			flags.bindProtocBinPath(flagSet)
 			flags.bindProtocWKTPath(flagSet)
diff --git a/internal/exec/exec.go b/internal/exec/exec.go
index 3a3c4f8..149d094 100644
--- a/internal/exec/exec.go
+++ b/internal/exec/exec.go
@@ -53,7 +53,7 @@ type Runner interface {
 	Files(args []string) error
 	Compile(args []string, dryRun bool) error
 	Gen(args []string, dryRun bool) error
-	Lint(args []string, listAllLinters bool, listLinters bool, listAllLintGroups bool, listLintGroup string) error
+	Lint(args []string, listAllLinters bool, listLinters bool, listAllLintGroups bool, listLintGroup string, diffLintGroups string) error
 	Format(args []string, overwrite, diffMode, lintMode, fix bool) error
 	All(args []string, disableFormat, disableLint, fix bool) error
 	GRPC(args, headers []string, address, method, data, callTimeout, connectTimeout, keepaliveTime string, stdin bool) error
diff --git a/internal/exec/runner.go b/internal/exec/runner.go
index 7286e88..fc5536d 100644
--- a/internal/exec/runner.go
+++ b/internal/exec/runner.go
@@ -272,9 +272,9 @@ func (r *runner) printCommands(doGen bool, protoSet *file.ProtoSet) error {
 	return nil
 }
 
-func (r *runner) Lint(args []string, listAllLinters bool, listLinters bool, listAllLintGroups bool, listLintGroup string) error {
-	if moreThanOneSet(listAllLinters, listLinters, listAllLintGroups, listLintGroup != "") {
-		return newExitErrorf(255, "can only set one of list-all-linters, list-linters, list-all-lint-groups, list-lint-group")
+func (r *runner) Lint(args []string, listAllLinters bool, listLinters bool, listAllLintGroups bool, listLintGroup string, diffLintGroups string) error {
+	if moreThanOneSet(listAllLinters, listLinters, listAllLintGroups, listLintGroup != "", diffLintGroups != "") {
+		return newExitErrorf(255, "can only set one of list-all-linters, list-linters, list-all-lint-groups, list-lint-group, diff-lint-groups")
 	}
 	if listAllLinters {
 		return r.listAllLinters()
@@ -288,6 +288,9 @@ func (r *runner) Lint(args []string, listAllLinters bool, listLinters bool, list
 	if listLintGroup != "" {
 		return r.listLintGroup(listLintGroup)
 	}
+	if diffLintGroups != "" {
+		return r.diffLintGroups(diffLintGroups)
+	}
 	meta, err := r.getMeta(args)
 	if err != nil {
 		return err
@@ -352,6 +355,27 @@ func (r *runner) listAllLintGroups() error {
 	return nil
 }
 
+func (r *runner) diffLintGroups(groups string) error {
+	split := strings.Split(groups, ",")
+	if len(split) != 2 {
+		return fmt.Errorf("argument to --diff-lint-groups must be two lint groups separated by '.', for example google,uber")
+	}
+	firstLinterIDs, err := getLinterIDs(split[0])
+	if err != nil {
+		return err
+	}
+	secondLinterIDs, err := getLinterIDs(split[1])
+	if err != nil {
+		return err
+	}
+	for _, s := range diffMaps(firstLinterIDs, secondLinterIDs) {
+		if err := r.println(s); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 func (r *runner) Format(args []string, overwrite, diffMode, lintMode, fix bool) error {
 	if moreThanOneSet(overwrite, diffMode, lintMode) {
 		return newExitErrorf(255, "can only set one of overwrite, diff, lint")
@@ -988,3 +1012,31 @@ func extractSortPackageNames(m map[string]*extract.Package) []string {
 	sort.Strings(s)
 	return s
 }
+
+func getLinterIDs(group string) (map[string]struct{}, error) {
+	linters, ok := lint.GroupToLinters[strings.ToLower(group)]
+	if !ok {
+		return nil, newExitErrorf(255, "unknown lint group: %s", strings.ToLower(group))
+	}
+	m := make(map[string]struct{})
+	for _, linter := range linters {
+		m[linter.ID()] = struct{}{}
+	}
+	return m, nil
+}
+
+func diffMaps(one map[string]struct{}, two map[string]struct{}) []string {
+	var s []string
+	for key := range one {
+		if _, ok := two[key]; !ok {
+			s = append(s, "< "+key)
+		}
+	}
+	for key := range two {
+		if _, ok := one[key]; !ok {
+			s = append(s, "> "+key)
+		}
+	}
+	sort.Strings(s)
+	return s
+}


Test patch:
diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go
index bad4716..04da277 100644
--- a/internal/cmd/cmd_test.go
+++ b/internal/cmd/cmd_test.go
@@ -768,6 +768,38 @@ func TestListLintGroup(t *testing.T) {
 	assertLinters(t, lint.Uber2Linters, "lint", "--list-lint-group", "uber2")
 }
 
+func TestDiffLintGroups(t *testing.T) {
+	assertExact(
+		t,
+		0,
+		`> COMMENTS_NO_C_STYLE
+> ENUMS_NO_ALLOW_ALIAS
+> ENUM_FIELD_PREFIXES
+> ENUM_ZERO_VALUES_INVALID
+> FILE_OPTIONS_EQUAL_GO_PACKAGE_PB_SUFFIX
+> FILE_OPTIONS_EQUAL_JAVA_MULTIPLE_FILES_TRUE
+> FILE_OPTIONS_EQUAL_JAVA_OUTER_CLASSNAME_PROTO_SUFFIX
+> FILE_OPTIONS_EQUAL_JAVA_PACKAGE_COM_PREFIX
+> FILE_OPTIONS_GO_PACKAGE_NOT_LONG_FORM
+> FILE_OPTIONS_GO_PACKAGE_SAME_IN_DIR
+> FILE_OPTIONS_JAVA_MULTIPLE_FILES_SAME_IN_DIR
+> FILE_OPTIONS_JAVA_PACKAGE_SAME_IN_DIR
+> FILE_OPTIONS_REQUIRE_GO_PACKAGE
+> FILE_OPTIONS_REQUIRE_JAVA_MULTIPLE_FILES
+> FILE_OPTIONS_REQUIRE_JAVA_OUTER_CLASSNAME
+> FILE_OPTIONS_REQUIRE_JAVA_PACKAGE
+> ONEOF_NAMES_LOWER_SNAKE_CASE
+> PACKAGES_SAME_IN_DIR
+> PACKAGE_IS_DECLARED
+> PACKAGE_LOWER_SNAKE_CASE
+> REQUEST_RESPONSE_TYPES_IN_SAME_FILE
+> REQUEST_RESPONSE_TYPES_UNIQUE
+> SYNTAX_PROTO3
+> WKT_DIRECTLY_IMPORTED`,
+		"lint", "--diff-lint-groups", "google,uber",
+	)
+}
+
 func assertLinters(t *testing.T, linters []lint.Linter, args ...string) {
 	linterIDs := make([]string, 0, len(linters))
 	for _, linter := range linters {