Repository: reviewdog/reviewdog
Pull request #412 (2020-01-05 09:30:46)

Refactor FilteredResultMap so that it can hold report level
Report level will be used later (#411).


Patch:
diff --git a/cmd/reviewdog/doghouse.go b/cmd/reviewdog/doghouse.go
index ad13481b..835baef5 100644
--- a/cmd/reviewdog/doghouse.go
+++ b/cmd/reviewdog/doghouse.go
@@ -110,10 +110,10 @@ func checkResultSet(ctx context.Context, r io.Reader, opt *option, isProject boo
 	return resultSet, nil
 }
 
-func postResultSet(ctx context.Context, resultSet *reviewdog.ResultMap, ghInfo *cienv.BuildInfo, cli client.DogHouseClientInterface) (*reviewdog.FilteredCheckMap, error) {
+func postResultSet(ctx context.Context, resultSet *reviewdog.ResultMap, ghInfo *cienv.BuildInfo, cli client.DogHouseClientInterface) (*reviewdog.FilteredResultMap, error) {
 	var g errgroup.Group
 	wd, _ := os.Getwd()
-	filteredResultSet := new(reviewdog.FilteredCheckMap)
+	filteredResultSet := new(reviewdog.FilteredResultMap)
 	resultSet.Range(func(name string, result *reviewdog.Result) {
 		checkResults := result.CheckResults
 		as := make([]*doghouse.Annotation, 0, len(checkResults))
@@ -139,7 +139,10 @@ func postResultSet(ctx context.Context, resultSet *reviewdog.ResultMap, ghInfo *
 				log.Printf("[%s] reported: %s", name, res.ReportURL)
 			}
 			if res.CheckedResults != nil {
-				filteredResultSet.Store(name, res.CheckedResults)
+				filteredResultSet.Store(name, &reviewdog.FilteredResult{
+					Level:         result.Level,
+					FilteredCheck: res.CheckedResults,
+				})
 			}
 			if res.ReportURL == "" && res.CheckedResults == nil {
 				return fmt.Errorf("no result found for %q", name)
@@ -161,10 +164,10 @@ func checkResultToAnnotation(c *reviewdog.CheckResult, wd string) *doghouse.Anno
 
 // reportResults reports results to given io.Writer and return true if at least
 // one annotation result is in diff.
-func reportResults(w io.Writer, filteredResultSet *reviewdog.FilteredCheckMap) bool {
+func reportResults(w io.Writer, filteredResultSet *reviewdog.FilteredResultMap) bool {
 	// Sort names to get deterministic result.
 	var names []string
-	filteredResultSet.Range(func(name string, results []*reviewdog.FilteredCheck) {
+	filteredResultSet.Range(func(name string, results *reviewdog.FilteredResult) {
 		names = append(names, name)
 	})
 	sort.Strings(names)
@@ -180,7 +183,7 @@ func reportResults(w io.Writer, filteredResultSet *reviewdog.FilteredCheckMap) b
 		fmt.Fprintf(w, "reviewdog: Reporting results for %q\n", name)
 		foundResultPerName := false
 		filteredNum := 0
-		for _, result := range results {
+		for _, result := range results.FilteredCheck {
 			if !result.InDiff {
 				filteredNum++
 				continue
diff --git a/go.sum b/go.sum
index 867ddc79..113e9a06 100644
--- a/go.sum
+++ b/go.sum
@@ -114,6 +114,7 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk
 golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20190605123033-f99c8df09eb5 h1:58fnuSXlxZmFdJyvtTFVmVhcMLU6v5fEb/ok4wyqtNU=
 golang.org/x/crypto v0.0.0-20190605123033-f99c8df09eb5/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
+golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550 h1:ObdrDkeb4kJdCP557AjRjq69pTHfNouLtWZG7j9rPN8=
 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
 golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
diff --git a/resultmap.go b/resultmap.go
index acce6796..f07690ae 100644
--- a/resultmap.go
+++ b/resultmap.go
@@ -54,41 +54,46 @@ func (rm *ResultMap) Len() int {
 	return l
 }
 
-// FilteredCheckMap represents a concurrent-safe map to store CheckResults generated by concurrent jobs.
-type FilteredCheckMap struct {
+type FilteredResult struct {
+	Level         string
+	FilteredCheck []*FilteredCheck
+}
+
+// FilteredResultMap represents a concurrent-safe map to store CheckResults generated by concurrent jobs.
+type FilteredResultMap struct {
 	sm sync.Map
 }
 
-// Store saves a new []*FilteredCheck into FilteredCheckMap.
-func (rm *FilteredCheckMap) Store(key string, r []*FilteredCheck) {
+// Store saves a new []*FilteredCheckFilteredResult into FilteredResultMap.
+func (rm *FilteredResultMap) Store(key string, r *FilteredResult) {
 	rm.sm.Store(key, r)
 }
 
-// Load fetches []*FilteredCheck from FilteredCheckMap
-func (rm *FilteredCheckMap) Load(key string) ([]*FilteredCheck, error) {
+// Load fetches FilteredResult from FilteredResultMap
+func (rm *FilteredResultMap) Load(key string) (*FilteredResult, error) {
 	v, ok := rm.sm.Load(key)
 	if !ok {
 		return nil, fmt.Errorf("fail to get the value of key %q from results", key)
 	}
 
-	t, ok := v.([]*FilteredCheck)
+	t, ok := v.(*FilteredResult)
 	if !ok {
-		return nil, errors.New("stored type in FilteredCheckMap is invalid")
+		return nil, errors.New("stored type in FilteredResultMap is invalid")
 	}
 
 	return t, nil
 }
 
-// Range retrieves `key` and `values` from FilteredCheckMap iteratively.
-func (rm *FilteredCheckMap) Range(f func(key string, val []*FilteredCheck)) {
+// Range retrieves `key` and `values` from FilteredResultMap iteratively.
+func (rm *FilteredResultMap) Range(f func(key string, val *FilteredResult)) {
 	rm.sm.Range(func(k, v interface{}) bool {
-		f(k.(string), v.([]*FilteredCheck))
+		f(k.(string), v.(*FilteredResult))
 		return true
 	})
 }
 
-// Len returns the length of FilteredCheckMap count. Len() is not yet officially not supported by Go. (ref: https://github.com/golang/go/issues/20680)
-func (rm *FilteredCheckMap) Len() int {
+// Len returns the length of FilteredResultMap count. Len() is not yet officially not supported by Go. (ref: https://github.com/golang/go/issues/20680)
+func (rm *FilteredResultMap) Len() int {
 	l := 0
 	rm.sm.Range(func(_, _ interface{}) bool {
 		l++


Test patch:
diff --git a/cmd/reviewdog/doghouse_test.go b/cmd/reviewdog/doghouse_test.go
index 85a8ebac..32bdaeb7 100644
--- a/cmd/reviewdog/doghouse_test.go
+++ b/cmd/reviewdog/doghouse_test.go
@@ -308,7 +308,7 @@ func TestPostResultSet_withoutReportURL(t *testing.T) {
 	if err != nil {
 		t.Fatalf("should have result for name1: %v", err)
 	}
-	if diff := cmp.Diff(results, wantResults); diff != "" {
+	if diff := cmp.Diff(results.FilteredCheck, wantResults); diff != "" {
 		t.Errorf("results has diff:\n%s", diff)
 	}
 }
@@ -337,27 +337,31 @@ func TestPostResultSet_withEmptyResponse(t *testing.T) {
 }
 
 func TestReportResults(t *testing.T) {
-	filteredResultSet := new(reviewdog.FilteredCheckMap)
-	filteredResultSet.Store("name1", []*reviewdog.FilteredCheck{
-		{
-			CheckResult: &reviewdog.CheckResult{
-				Lines: []string{"name1-L1", "name1-L2"},
+	filteredResultSet := new(reviewdog.FilteredResultMap)
+	filteredResultSet.Store("name1", &reviewdog.FilteredResult{
+		FilteredCheck: []*reviewdog.FilteredCheck{
+			{
+				CheckResult: &reviewdog.CheckResult{
+					Lines: []string{"name1-L1", "name1-L2"},
+				},
+				InDiff: true,
 			},
-			InDiff: true,
-		},
-		{
-			CheckResult: &reviewdog.CheckResult{
-				Lines: []string{"name1.2-L1", "name1.2-L2"},
+			{
+				CheckResult: &reviewdog.CheckResult{
+					Lines: []string{"name1.2-L1", "name1.2-L2"},
+				},
+				InDiff: false,
 			},
-			InDiff: false,
 		},
 	})
-	filteredResultSet.Store("name2", []*reviewdog.FilteredCheck{
-		{
-			CheckResult: &reviewdog.CheckResult{
-				Lines: []string{"name1-L1", "name1-L2"},
+	filteredResultSet.Store("name2", &reviewdog.FilteredResult{
+		FilteredCheck: []*reviewdog.FilteredCheck{
+			{
+				CheckResult: &reviewdog.CheckResult{
+					Lines: []string{"name1-L1", "name1-L2"},
+				},
+				InDiff: false,
 			},
-			InDiff: false,
 		},
 	})
 	stdout := new(bytes.Buffer)
@@ -377,27 +381,31 @@ reviewdog: No results found for "name2". 1 results found outside diff.
 }
 
 func TestReportResults_noResultsInDiff(t *testing.T) {
-	filteredResultSet := new(reviewdog.FilteredCheckMap)
-	filteredResultSet.Store("name1", []*reviewdog.FilteredCheck{
-		{
-			CheckResult: &reviewdog.CheckResult{
-				Lines: []string{"name1-L1", "name1-L2"},
+	filteredResultSet := new(reviewdog.FilteredResultMap)
+	filteredResultSet.Store("name1", &reviewdog.FilteredResult{
+		FilteredCheck: []*reviewdog.FilteredCheck{
+			{
+				CheckResult: &reviewdog.CheckResult{
+					Lines: []string{"name1-L1", "name1-L2"},
+				},
+				InDiff: false,
 			},
-			InDiff: false,
-		},
-		{
-			CheckResult: &reviewdog.CheckResult{
-				Lines: []string{"name1.2-L1", "name1.2-L2"},
+			{
+				CheckResult: &reviewdog.CheckResult{
+					Lines: []string{"name1.2-L1", "name1.2-L2"},
+				},
+				InDiff: false,
 			},
-			InDiff: false,
 		},
 	})
-	filteredResultSet.Store("name2", []*reviewdog.FilteredCheck{
-		{
-			CheckResult: &reviewdog.CheckResult{
-				Lines: []string{"name1-L1", "name1-L2"},
+	filteredResultSet.Store("name2", &reviewdog.FilteredResult{
+		FilteredCheck: []*reviewdog.FilteredCheck{
+			{
+				CheckResult: &reviewdog.CheckResult{
+					Lines: []string{"name1-L1", "name1-L2"},
+				},
+				InDiff: false,
 			},
-			InDiff: false,
 		},
 	})
 	stdout := new(bytes.Buffer)