Repository: reviewdog/reviewdog
Pull request #539 (2020-05-04 14:45:53)

Change default filter mode and return error for unknown flag value
ref: #536


Patch:
diff --git a/cmd/reviewdog/main.go b/cmd/reviewdog/main.go
index 614b9218..7c02d2d1 100644
--- a/cmd/reviewdog/main.go
+++ b/cmd/reviewdog/main.go
@@ -74,11 +74,12 @@ const (
 	levelDoc            = `report level currently used for github-pr-check reporter ("info","warning","error").`
 	guessPullRequestDoc = `guess Pull Request ID by branch name and commit SHA`
 	teeDoc              = `enable "tee"-like mode which outputs tools's output as is while reporting results to -reporter. Useful for debugging as well.`
-	filterModeDoc       = `how to filter checks results. [diff_context, added, file].
-    "diff_context" (default)
-        Filter by diff context, which can include unchanged lines.
-    "added"
+	filterModeDoc       = `how to filter checks results. [added, diff_context, file].
+    "added" (default)
         Filter by added/modified diff lines.
+    "diff_context"
+        Filter by diff context, which can include unchanged lines.
+				i.e. changed lines +-N lines (e.g. N=3 for default git diff).
     "file"
         Filter by added/modified file.
 `
diff --git a/difffilter/filter.go b/difffilter/filter.go
index 1104b89f..46a30efc 100644
--- a/difffilter/filter.go
+++ b/difffilter/filter.go
@@ -1,6 +1,7 @@
 package difffilter
 
 import (
+	"fmt"
 	"path/filepath"
 	"strings"
 
@@ -12,25 +13,30 @@ import (
 type Mode int
 
 const (
-	// ModeDiffContext represents filtering by diff context.
-	ModeDiffContext Mode = iota
+	// ModeDefault represents default mode, which means users doesn't specify
+	// filter-mode. The behavior can be changed depending on reporters/context
+	// later if we want. Basically, it's same as ModeAdded because it's most safe
+	// and basic mode for reporters implementation.
+	ModeDefault Mode = iota
 	// ModeAdded represents filtering by added/changed diff lines.
 	ModeAdded
+	// ModeDiffContext represents filtering by diff context.
+	// i.e. changed lines +-N lines (e.g. N=3 for default git diff).
+	ModeDiffContext
 	// ModeFile represents filtering by changed files.
-	// Note that this mode doesn't work with github-pr-review reporter due to GitHUb API
-	// limiation.
 	ModeFile
 )
 
 // String implements the flag.Value interface
 func (mode *Mode) String() string {
 	names := [...]string{
-		"diff_context",
+		"default",
 		"added",
+		"diff_context",
 		"file",
 	}
-	if *mode < ModeDiffContext || *mode > ModeFile {
-		return "Unknown"
+	if *mode < ModeDefault || *mode > ModeFile {
+		return "Unknown mode"
 	}
 
 	return names[*mode]
@@ -39,14 +45,16 @@ func (mode *Mode) String() string {
 // Set implements the flag.Value interface
 func (mode *Mode) Set(value string) error {
 	switch value {
-	case "diff_context":
-		*mode = ModeDiffContext
+	case "default", "":
+		*mode = ModeDefault
 	case "added":
 		*mode = ModeAdded
+	case "diff_context":
+		*mode = ModeDiffContext
 	case "file":
 		*mode = ModeFile
 	default:
-		*mode = ModeDiffContext
+		return fmt.Errorf("invalid mode name: %s", value)
 	}
 	return nil
 }
@@ -124,7 +132,7 @@ func (df *DiffFilter) isSignificantLine(line *diff.Line) bool {
 	switch df.mode {
 	case ModeDiffContext, ModeFile:
 		return true // any lines in diff are significant.
-	case ModeAdded:
+	case ModeAdded, ModeDefault:
 		return line.Type == diff.LineAdded
 	}
 	return false


Test patch:
diff --git a/difffilter/filter_test.go b/difffilter/filter_test.go
index aee9ccda..4f78bf72 100644
--- a/difffilter/filter_test.go
+++ b/difffilter/filter_test.go
@@ -9,6 +9,33 @@ import (
 	"github.com/reviewdog/reviewdog/diff"
 )
 
+func TestMode_Set(t *testing.T) {
+	tests := []struct {
+		value   string
+		want    Mode
+		wantErr bool
+	}{
+		{value: "", want: ModeDefault},
+		{value: "default", want: ModeDefault},
+		{value: "added", want: ModeAdded},
+		{value: "diff_context", want: ModeDiffContext},
+		{value: "file", want: ModeFile},
+		{value: "unknown", wantErr: true},
+	}
+	for _, tt := range tests {
+		var mode Mode
+		err := (&mode).Set(tt.value)
+		if err != nil && !tt.wantErr {
+			t.Errorf("got error for %q: %v", tt.value, err)
+		} else if err == nil && tt.wantErr {
+			t.Errorf("want error, but got nil for %q", tt.value)
+		}
+		if mode != tt.want {
+			t.Errorf("[value=%s] got %q, want %q", tt.value, mode.String(), tt.want.String())
+		}
+	}
+}
+
 const sampleDiffRoot = `--- a/sample.old.txt	2016-10-13 05:09:35.820791185 +0900
 +++ b/sample.new.txt	2016-10-13 05:15:26.839245048 +0900
 @@ -1,3 +1,4 @@
@@ -159,6 +186,13 @@ func TestDiffFilter_subdir(t *testing.T) {
 			want:         true,
 			wantLnumDiff: 3,
 		},
+		{
+			path:         "sample.new.txt",
+			lnum:         2,
+			mode:         ModeDefault,
+			want:         true,
+			wantLnumDiff: 3,
+		},
 		{
 			path:         filepath.Join(getCwd(), "sample.new.txt"),
 			lnum:         2,