filter: refactor NormalizePath

cmd/reviewdog/doghouse.go

@@ -8,7 +8,6 @@ import (
 	"log"
 	"net/http"
 	"os"
-	"path/filepath"
 	"sort"
 
 	"golang.org/x/oauth2"
@@ -180,8 +179,7 @@ func postResultSet(ctx context.Context, resultSet *reviewdog.ResultMap,
 }
 
 func checkResultToAnnotation(d *rdf.Diagnostic, wd, gitRelWd string) *doghouse.Annotation {
-	d.GetLocation().Path = filepath.ToSlash(filepath.Join(
-		gitRelWd, filter.CleanPath(d.GetLocation().GetPath(), wd)))
+	d.GetLocation().Path = filter.NormalizePath(d.GetLocation().GetPath(), wd, gitRelWd)
 	return &doghouse.Annotation{
 		Diagnostic: d,
 	}

filter/diff_filter.go

@@ -86,8 +86,8 @@ type difflines map[normalizedPath]map[int]*diff.Line
 // difffiles is a hash table of normalizedPath to *diff.FileDiff.
 type difffiles map[normalizedPath]*diff.FileDiff
 
-// New creates a new DiffFilter.
-func New(diff []*diff.FileDiff, strip int, cwd string, mode Mode) *DiffFilter {
+// NewDiffFilter creates a new DiffFilter.
+func NewDiffFilter(diff []*diff.FileDiff, strip int, cwd string, mode Mode) *DiffFilter {
 	df := &DiffFilter{
 		strip:     strip,
 		cwd:       cwd,
@@ -153,19 +153,7 @@ func (df *DiffFilter) isSignificantLine(line *diff.Line) bool {
 type normalizedPath struct{ p string }
 
 func (df *DiffFilter) normalizePath(path string) normalizedPath {
-	path = filepath.Clean(path)
-	// Convert absolute path to relative path only if the path is in current
-	// directory.
-	if filepath.IsAbs(path) && df.cwd != "" && contains(path, df.cwd) {
-		relPath, err := filepath.Rel(df.cwd, path)
-		if err == nil {
-			path = relPath
-		}
-	}
-	if !filepath.IsAbs(path) && df.projectRelPath != "" {
-		path = filepath.Join(df.projectRelPath, path)
-	}
-	return normalizedPath{p: filepath.ToSlash(path)}
+	return normalizedPath{p: NormalizePath(path, df.cwd, df.projectRelPath)}
 }
 
 func contains(path, base string) bool {

filter/diff_filter_test.go

@@ -210,7 +210,7 @@ func TestDiffFilter_root(t *testing.T) {
 		},
 	}
 	for _, tt := range tests {
-		df := New(files, 1, getCwd(), tt.mode)
+		df := NewDiffFilter(files, 1, getCwd(), tt.mode)
 		if got, gotFile, gotLine := df.ShouldReport(tt.path, tt.lnum); got != tt.want ||
 			(gotFile != nil) != tt.wantFileDiff ||
 			(gotLine != nil) != tt.wantLineDiff {
@@ -267,7 +267,7 @@ func TestDiffFilter_subdir(t *testing.T) {
 		},
 	}
 	for _, tt := range tests {
-		df := New(files, 1, getCwd(), tt.mode)
+		df := NewDiffFilter(files, 1, getCwd(), tt.mode)
 		if got, gotFile, gotLine := df.ShouldReport(tt.path, tt.lnum); got != tt.want ||
 			(gotFile != nil) != tt.wantFileDiff ||
 			(gotLine != nil) != tt.wantLineDiff {

filter/filter.go

@@ -26,10 +26,11 @@ type FilteredCheck struct {
 func FilterCheck(results []*rdf.Diagnostic, diff []*diff.FileDiff, strip int,
 	cwd string, mode Mode) []*FilteredCheck {
 	checks := make([]*FilteredCheck, 0, len(results))
-	df := New(diff, strip, cwd, mode)
+	df := NewDiffFilter(diff, strip, cwd, mode)
 	for _, result := range results {
 		check := &FilteredCheck{Diagnostic: result}
 		loc := result.GetLocation()
+		loc.Path = NormalizePath(loc.GetPath(), cwd, "")
 		startLine := int(loc.GetRange().GetStart().GetLine())
 		endLine := int(loc.GetRange().GetEnd().GetLine())
 		if endLine == 0 {
@@ -49,30 +50,31 @@ func FilterCheck(results []*rdf.Diagnostic, diff []*diff.FileDiff, strip int,
 				}
 			}
 		}
-		loc.Path = CleanPath(loc.GetPath(), cwd)
+		// loc.Path = NormalizePath(loc.GetPath(), cwd, "")
 		checks = append(checks, check)
 	}
 	return checks
 }
 
-// CleanPath clean up given path. If workdir is not empty, it returns relative
-// path to the given workdir.
-//
-// TODO(haya14busa): DRY. Create shared logic between this and
-// filter.normalizePath.
-func CleanPath(path, workdir string) string {
-	p := path
-	if filepath.IsAbs(path) && workdir != "" {
+// NormalizePath return normalized path with workdir and relative path to
+// project.
+func NormalizePath(path, workdir, projectRelPath string) string {
+	path = filepath.Clean(path)
+	if path == "." {
+		return ""
+	}
+	// Convert absolute path to relative path only if the path is in current
+	// directory.
+	if filepath.IsAbs(path) && workdir != "" && contains(path, workdir) {
 		relPath, err := filepath.Rel(workdir, path)
 		if err == nil {
-			p = relPath
+			path = relPath
 		}
 	}
-	p = filepath.Clean(p)
-	if p == "." {
-		return ""
+	if !filepath.IsAbs(path) && projectRelPath != "" {
+		path = filepath.Join(projectRelPath, path)
 	}
-	return filepath.ToSlash(p)
+	return filepath.ToSlash(path)
 }
 
 func getOldPosition(filediff *diff.FileDiff, strip int, newPath string, newLine int) (oldPath string, oldLine int) {