Repository: googleapis/api-linter
Pull request #58 (2019-05-13 14:07:47)

Removed string alias Category.
`Category` is set only by users in their configs and reflected in the returned problems, and thus it will always be `string` and does not need any validation functions. So I removed the alias to make it clearer.


Patch:
diff --git a/cmd/api-linter/configs.go b/cmd/api-linter/configs.go
index 5a486a1..152e239 100644
--- a/cmd/api-linter/configs.go
+++ b/cmd/api-linter/configs.go
@@ -22,10 +22,7 @@ func configs() lint.Configs {
 		lint.Config{
 			IncludedPaths: []string{"**/*.proto"},
 			RuleConfigs: map[string]lint.RuleConfig{
-				"core": {
-					Disabled: false,
-					Category: lint.Warning,
-				},
+				"core": {},
 			},
 		},
 	}
diff --git a/cmd/api-linter/examples/example_config.json b/cmd/api-linter/examples/example_config.json
deleted file mode 100644
index 0717d11..0000000
--- a/cmd/api-linter/examples/example_config.json
+++ /dev/null
@@ -1,11 +0,0 @@
-[
-  {
-    "included_paths": ["**/*.proto"],
-    "rule_configs": {
-      "": {
-        "status": "enabled",
-        "category": "warning"
-      }
-    }
-  }
-]
\ No newline at end of file
diff --git a/lint/config.go b/lint/config.go
index a5579f1..9b5bdb7 100644
--- a/lint/config.go
+++ b/lint/config.go
@@ -56,12 +56,8 @@ type Config struct {
 
 // RuleConfig stores configurable status and category of a rule.
 type RuleConfig struct {
-	Disabled bool     `json:"disabled" yaml:"disabled"`
-	Category Category `json:"category" yaml:"category"`
-}
-
-func getDefaultRuleConfig() RuleConfig {
-	return RuleConfig{Category: Warning}
+	Disabled bool   `json:"disabled" yaml:"disabled"`
+	Category string `json:"category" yaml:"category"`
 }
 
 // ReadConfigsFromFile reads Configs from a file.
diff --git a/lint/linter.go b/lint/linter.go
index d74a215..d698cf7 100644
--- a/lint/linter.go
+++ b/lint/linter.go
@@ -71,7 +71,7 @@ func (l *Linter) run(req Request) (Response, error) {
 	var errMessages []string
 
 	for name, rl := range l.rules {
-		config := getDefaultRuleConfig()
+		var config RuleConfig
 
 		if c, err := l.configs.getRuleConfig(req.ProtoFile().Path(), name); err == nil {
 			config = config.withOverride(c)
diff --git a/lint/response.go b/lint/response.go
index a101591..9fe559f 100644
--- a/lint/response.go
+++ b/lint/response.go
@@ -27,7 +27,7 @@ type Problem struct {
 	// Message provides a short description of the problem.
 	Message string `json:"message" yaml:"message"`
 	// Suggestion provides a suggested fix, if applicable.
-	Suggestion string `json:"suggestion" yaml:"suggestion"`
+	Suggestion string `json:"suggestion,omitempty" yaml:"suggestion,omitempty"`
 	// Location provides the location of the problem. If both
 	// `Location` and `Descriptor` are specified, the location
 	// is then used from `Location` instead of `Descriptor`.
@@ -38,7 +38,11 @@ type Problem struct {
 	// is used as the location of the problem.
 	Descriptor protoreflect.Descriptor `json:"-" yaml:"-"`
 
-	// The following fields will be set by Runtime.
-	Category Category `json:"category" yaml:"category"`
-	RuleID   RuleName `json:"rule_id" yaml:"rule_id"`
+	// RuleID provides the ID of the rule that this problem belongs to.
+	// DO NOT SET: this field will be set by the linter based on rule info
+	// and user configs.
+	RuleID RuleName `json:"rule_id" yaml:"rule_id"`
+
+	// DO NOT SET:  this field will be set by the linter based on user configs.
+	Category string `json:"category,omitempty" yaml:"category,omitempty"`
 }
diff --git a/lint/rule_info.go b/lint/rule_info.go
index e9417c4..bd7ffc2 100644
--- a/lint/rule_info.go
+++ b/lint/rule_info.go
@@ -37,16 +37,6 @@ const (
 	ProtoRequest RequestType = "proto-request"
 )
 
-// Category defines the category of the findings produced by a rule.
-type Category string
-
-const (
-	// Error indicates that in the API, something will cause errors.
-	Error Category = "error"
-	// Warning indicates that in the API, something can be do better.
-	Warning Category = "warning"
-)
-
 // Status defines whether a rule is enabled, disabled or deprecated.
 type Status string
 


Test patch:
diff --git a/lint/config_test.go b/lint/config_test.go
index 753ec2d..deb7443 100644
--- a/lint/config_test.go
+++ b/lint/config_test.go
@@ -21,7 +21,7 @@ import (
 )
 
 func TestRuleConfigs_getRuleConfig(t *testing.T) {
-	matchConfig := RuleConfig{Disabled: false, Category: Warning}
+	matchConfig := RuleConfig{Disabled: false, Category: "warning"}
 
 	tests := []struct {
 		configs Configs
@@ -153,39 +153,39 @@ func TestRuleConfig_WithOverride(t *testing.T) {
 		result   RuleConfig
 	}{
 		{
-			RuleConfig{Disabled: false, Category: Warning},
-			RuleConfig{Disabled: false, Category: Warning},
-			RuleConfig{Disabled: false, Category: Warning},
+			RuleConfig{Disabled: false, Category: "warning"},
+			RuleConfig{Disabled: false, Category: "warning"},
+			RuleConfig{Disabled: false, Category: "warning"},
 		},
 		{
 			RuleConfig{},
-			RuleConfig{Disabled: false, Category: Warning},
-			RuleConfig{Disabled: false, Category: Warning},
+			RuleConfig{Disabled: false, Category: "warning"},
+			RuleConfig{Disabled: false, Category: "warning"},
 		},
 		{
 			RuleConfig{Category: ""},
-			RuleConfig{Disabled: true, Category: Warning},
-			RuleConfig{Disabled: true, Category: Warning},
+			RuleConfig{Disabled: true, Category: "warning"},
+			RuleConfig{Disabled: true, Category: "warning"},
 		},
 		{
-			RuleConfig{Disabled: false, Category: Warning},
-			RuleConfig{Disabled: true, Category: Error},
-			RuleConfig{Disabled: true, Category: Error},
+			RuleConfig{Disabled: false, Category: "warning"},
+			RuleConfig{Disabled: true, Category: "error"},
+			RuleConfig{Disabled: true, Category: "error"},
 		},
 		{
-			RuleConfig{Disabled: false, Category: Warning},
+			RuleConfig{Disabled: false, Category: "warning"},
 			RuleConfig{Category: ""},
-			RuleConfig{Disabled: false, Category: Warning},
+			RuleConfig{Disabled: false, Category: "warning"},
 		},
 		{
-			RuleConfig{Disabled: false, Category: Warning},
+			RuleConfig{Disabled: false, Category: "warning"},
 			RuleConfig{Disabled: true, Category: ""},
-			RuleConfig{Disabled: true, Category: Warning},
+			RuleConfig{Disabled: true, Category: "warning"},
 		},
 		{
-			RuleConfig{Disabled: false, Category: Warning},
-			RuleConfig{Category: Error},
-			RuleConfig{Disabled: false, Category: Error},
+			RuleConfig{Disabled: false, Category: "warning"},
+			RuleConfig{Category: "error"},
+			RuleConfig{Disabled: false, Category: "error"},
 		},
 	}
 
diff --git a/lint/linter_test.go b/lint/linter_test.go
index 3d830c1..5c300ec 100644
--- a/lint/linter_test.go
+++ b/lint/linter_test.go
@@ -32,7 +32,7 @@ func TestLinter_run(t *testing.T) {
 		{[]string{"**"}, []string{}, map[string]RuleConfig{}},
 	}
 
-	ruleProblems := []Problem{{Message: "rule1_problem", Category: Warning, RuleID: "test::rule1"}}
+	ruleProblems := []Problem{{Message: "rule1_problem", Category: "", RuleID: "test::rule1"}}
 
 	tests := []struct {
 		desc    string
@@ -82,12 +82,12 @@ func TestLinter_run(t *testing.T) {
 				Config{
 					IncludedPaths: []string{"*"},
 					RuleConfigs: map[string]RuleConfig{
-						"test::rule1": {Category: Error},
+						"test::rule1": {Category: "error"},
 					},
 				},
 			),
 			Response{
-				Problems: []Problem{{Message: "rule1_problem", Category: Error, RuleID: "test::rule1"}},
+				Problems: []Problem{{Message: "rule1_problem", Category: "error", RuleID: "test::rule1"}},
 				FilePath: req.ProtoFile().Path(),
 			},
 		},