Repository: googleapis/api-linter
Pull request #332 (2019-11-10 03:31:32)

Segment matching in file comments.
Fix issue #326 


Patch:
diff --git a/lint/rule.go b/lint/rule.go
index bc0f88e..70038c5 100644
--- a/lint/rule.go
+++ b/lint/rule.go
@@ -15,7 +15,7 @@
 package lint
 
 import (
-	"fmt"
+	"regexp"
 	"strings"
 
 	dpb "github.com/golang/protobuf/protoc-gen-go/descriptor"
@@ -291,39 +291,49 @@ func (r *DescriptorRule) Lint(fd *desc.FileDescriptor) []Problem {
 	return problems
 }
 
+var disableRuleNameRegex = regexp.MustCompile(`api-linter:\s*(.+)\s*=\s*disabled`)
+
+func extractDisabledRuleName(commentLine string) string {
+	match := disableRuleNameRegex.FindStringSubmatch(commentLine)
+	if len(match) > 0 {
+		return match[1]
+	}
+	return ""
+}
+
 // ruleIsEnabled returns true if the rule is enabled (not disabled by the comments
 // for the given descriptor or its file), false otherwise.
 func ruleIsEnabled(rule ProtoRule, d desc.Descriptor, aliasMap map[string]string) bool {
 	// Some rules have a legacy name. We add it to the check list.
 	ruleName := string(rule.GetName())
-	names := []string{ruleName}
-	if alias := aliasMap[ruleName]; alias != "" {
-		names = append(names, alias)
+	names := []string{ruleName, aliasMap[ruleName]}
+
+	commentLines := strings.Split(fileHeader(d.GetFile()), "\n")
+	commentLines = append(commentLines, strings.Split(getLeadingComments(d), "\n")...)
+	disabledRules := []string{}
+	for _, commentLine := range commentLines {
+		r := extractDisabledRuleName(commentLine)
+		if r != "" {
+			disabledRules = append(disabledRules, r)
+		}
 	}
 
-	directives := []string{}
 	for _, name := range names {
-		directives = append(directives, fmt.Sprintf("api-linter: %s=disabled", name))
-	}
-
-	for _, directive := range directives {
-		// The rule may be disabled in the comments above the descriptor.
-		if sourceInfo := d.GetSourceInfo(); sourceInfo != nil {
-			if strings.Contains(sourceInfo.GetLeadingComments(), directive) {
-				return false
-			}
-		}
-
-		// The rule may also be disabled at the file level.
-		if strings.Contains(fileHeader(d.GetFile()), directive) {
+		if matchRule(name, disabledRules...) {
 			return false
 		}
 	}
 
-	// The rule is enabled.
 	return true
 }
 
+func getLeadingComments(d desc.Descriptor) string {
+	if sourceInfo := d.GetSourceInfo(); sourceInfo != nil {
+		return sourceInfo.GetLeadingComments()
+	}
+	return ""
+}
+
 // getAllMessages returns a slice with every message (not just top-level
 // messages) in the file.
 func getAllMessages(f *desc.FileDescriptor) (messages []*desc.MessageDescriptor) {


Test patch:
diff --git a/lint/rule_test.go b/lint/rule_test.go
index 1f00be9..e60c959 100644
--- a/lint/rule_test.go
+++ b/lint/rule_test.go
@@ -324,14 +324,14 @@ func TestDescriptorRule(t *testing.T) {
 func TestRuleIsEnabled(t *testing.T) {
 	// Create a no-op rule, which we can check enabled status on.
 	rule := &FileRule{
-		Name: RuleName("test"),
+		Name: RuleName("a::b::c"),
 		LintFile: func(fd *desc.FileDescriptor) []Problem {
 			return []Problem{}
 		},
 	}
 
 	aliases := map[string]string{
-		"test": "alias",
+		"a::b::c": "d::e::f",
 	}
 
 	// Create appropriate test permutations.
@@ -342,10 +342,19 @@ func TestRuleIsEnabled(t *testing.T) {
 		enabled        bool
 	}{
 		{"Enabled", "", "", true},
-		{"FileDisabled", "api-linter: test=disabled", "", false},
-		{"MessageDisabled", "", "api-linter: test=disabled", false},
+		{"FileDisabled", "api-linter: a::b::c=disabled", "", false},
+		{"MessageDisabled", "", "api-linter: a::b::c=disabled", false},
 		{"NameNotMatch", "", "api-linter: other=disabled", true},
-		{"AliasDisabled", "", "api-linter: alias=disabled", false},
+		{"RegexpNotMatch", "", "api-lint: a::b::c=disabled", true},
+		{"AliasDisabled", "", "api-linter: d::e::f=disabled", false},
+		{"FileComments_PrefixMatched_Disabled", "api-linter: a=disabled", "", false},
+		{"FileComments_MiddleMatched_Disabled", "api-linter: b=disabled", "", false},
+		{"FileComments_SuffixMatched_Disabled", "api-linter: c=disabled", "", false},
+		{"FileComments_MultipleLinesMatched_Disabled", "api-linter: x=disabled\napi-linter: a=disabled", "", false},
+		{"MessageComments_PrefixMatched_Disabled", "", "api-linter: a=disabled", false},
+		{"MessageComments_MiddleMatched_Disabled", "", "api-linter: b=disabled", false},
+		{"MessageComments_SuffixMatched_Disabled", "", "api-linter: c=disabled", false},
+		{"MessageComments_MultipleLinesMatched_Disabled", "", "api-linter: x=disabled\napi-linter: a=disabled", false},
 	}
 
 	// Run the specific tests individually.