More fixes and tests for unreachable rules, v0.6.2

CHANGELOG.md

@@ -1,3 +1,7 @@
+## v0.6.2 [2016-04-26]
+
+- More fixes for detection of unreachable rules with `or`
+
 ## v0.6.1 [2016-04-25]
 
 - Fixed false positive for unreachable rules checks with `or`

checks/unreachable.go

@@ -42,8 +42,10 @@ func CheckUnreachableRules(s *lib.Section) []lib.Problem {
 			if prevLine, found := usedACLs[*acl]; found {
 				// if current keyword is same as in line when this ACL was used
 				if _, ok := checkableKWs[curKW]; ok && curKW == kwMap[prevLine] {
+					// we need to strip acls with 'or' before them to correctly find shadowing
+					prevACLs := A.StripOrs(aclsMap[prevLine])
 					// if previous acl line is more generic than current
-					if A.In(aclsMap[prevLine], curACLs) && !A.HasOrs(curACLs) {
+					if A.In(prevACLs, curACLs) && !A.HasOrs(curACLs) {
 						problems = append(
 							problems,
 							lib.Problem{

checks/unreachable_test.go

@@ -1,20 +1,117 @@
 package checks
 
 import (
+	"reflect"
 	"testing"
 
 	"github.com/abulimov/haproxy-lint/lib"
 )
 
 func TestCheckUnreachableRules(t *testing.T) {
-	lines, err := lib.GetConfig("../testdata/haproxy.cfg", "")
-	if err != nil {
-		t.Fatalf("Failed to read test data: %v", err)
+	tests := []struct {
+		// Test description.
+		name string
+		// Parameters.
+		s *lib.Section
+		// Expected results.
+		want []lib.Problem
+	}{
+		{
+			name: "ACL with 'or' should't match",
+			s: &lib.Section{
+				Type: "frontend",
+				Name: "name",
+				Line: 0,
+				Content: map[int]string{
+					1: "use_backend undefined-servers if h_test",
+					2: "use_backend servers if h_test or { ssl_fc }",
+					3: "use_backend servers if h_test h_some",
+				},
+			},
+			want: []lib.Problem{
+				{
+					Col:      0,
+					Line:     1,
+					Severity: "warning",
+					Message:  "This line shadows rule 'use_backend servers if h_test h_some' on line 3",
+				},
+				{
+					Col:      0,
+					Line:     3,
+					Severity: "warning",
+					Message:  "Unreachable rule - 'use_backend undefined-servers if h_test' on line 1 will match first",
+				},
+			},
+		},
+		{
+			name: "multiple ACLs should match",
+			s: &lib.Section{
+				Type: "frontend",
+				Name: "name",
+				Line: 0,
+				Content: map[int]string{
+					1: "use_backend undefined-servers if h_test",
+					2: "use_backend servers if h_test { ssl_fc }",
+					3: "use_backend servers if h_test h_some",
+				},
+			},
+			want: []lib.Problem{
+				{
+					Col:      0,
+					Line:     1,
+					Severity: "warning",
+					Message:  "This line shadows rule 'use_backend servers if h_test { ssl_fc }' on line 2",
+				},
+				{
+					Col:      0,
+					Line:     2,
+					Severity: "warning",
+					Message:  "Unreachable rule - 'use_backend undefined-servers if h_test' on line 1 will match first",
+				},
+				{
+					Col:      0,
+					Line:     1,
+					Severity: "warning",
+					Message:  "This line shadows rule 'use_backend servers if h_test h_some' on line 3",
+				},
+				{
+					Col:      0,
+					Line:     3,
+					Severity: "warning",
+					Message:  "Unreachable rule - 'use_backend undefined-servers if h_test' on line 1 will match first",
+				},
+			},
+		},
+		{
+			name: "General acl with 'or' should be found",
+			s: &lib.Section{
+				Type: "frontend",
+				Name: "name",
+				Line: 0,
+				Content: map[int]string{
+					1: "use_backend undefined-servers if h_test or { ssl_fc }",
+					2: "use_backend servers if h_test h_some",
+				},
+			},
+			want: []lib.Problem{
+				{
+					Col:      0,
+					Line:     1,
+					Severity: "warning",
+					Message:  "This line shadows rule 'use_backend servers if h_test h_some' on line 2",
+				},
+				{
+					Col:      0,
+					Line:     2,
+					Severity: "warning",
+					Message:  "Unreachable rule - 'use_backend undefined-servers if h_test or { ssl_fc }' on line 1 will match first",
+				},
+			},
+		},
 	}
-	sections := lib.GetSection("frontend", lines)
-	problems := CheckUnreachableRules(sections[1])
-
-	if len(problems) != 2 {
-		t.Errorf("Expected %d problems, got %d", 2, len(problems))
+	for _, tt := range tests {
+		if got := CheckUnreachableRules(tt.s); !reflect.DeepEqual(got, tt.want) {
+			t.Errorf("%q. CheckUnreachableRules() = %v, want %v", tt.name, got, tt.want)
+		}
 	}
 }

haproxy-lint.go

@@ -28,7 +28,7 @@ import (
 	"github.com/abulimov/haproxy-lint/lib"
 )
 
-var version = "0.6.1"
+var version = "0.6.2"
 
 func myUsage() {
 	fmt.Printf("Usage: %s [OPTIONS] haproxy.cfg\n", os.Args[0])

lib/acl/acl.go

@@ -128,6 +128,17 @@ func HasOrs(acls []*ACL) bool {
 	return false
 }
 
+// StripOrs removes acl with 'or' before them from list of ACLs
+func StripOrs(acls []*ACL) []*ACL {
+	var result []*ACL
+	for _, a := range acls {
+		if !a.Or {
+			result = append(result, a)
+		}
+	}
+	return result
+}
+
 // In checks if second list of acls contains first
 func In(first, second []*ACL) bool {
 	m := make(map[ACL]bool)