Merge branch 'files' into 'master'

files: refactoring and improvements

See merge request greut/eclint!7

.golangci.yml

@@ -7,21 +7,31 @@ linters-settings:
 linters:
   enable:
     - deadcode
+    - depguard
+    - dogsled
     - dupl
     - errcheck
+    - funlen
+    - gochecknoglobals
+    - gochecknoinits
+    - goconst
     - gocritic
     - gocyclo
     - goimports
     - golint
-    - gosimple
-    - govet
+    - gosec
     - ineffassign
+    - interfacer
+    - lll
+    - maligned
     - megacheck
+    - misspell
     - nakedret
+    - prealloc
     - scopelint
-    - staticcheck
     - structcheck
+    - unconvert
     - unused
     - varcheck
-    - gosec
+    - whitespace
   disable-all: true

.goreleaser.yml

@@ -21,7 +21,6 @@ dockers:
 - image_templates:
   - greut/eclint:latest
   - greut/eclint:{{ .Tag }}
-  - greut/eclint:{{ .Tag }}
   - greut/eclint:v{{ .Major }}
   goos: linux
   goarch: amd64

files.go

@@ -0,0 +1,75 @@
+package main
+
+import (
+	"bytes"
+	"os"
+	"os/exec"
+	"path/filepath"
+
+	"github.com/go-logr/logr"
+)
+
+// listFiles returns the list of files based on the input.
+//
+// When its empty, it relies on `git ls-files` first, which
+// whould fail if `git` is not present or the current working
+// directory is not managed by it. In that case, it work the
+// current working directory.
+//
+// When args are given, it recursively walks into them.
+func listFiles(log logr.Logger, args ...string) ([]string, error) {
+	if len(args) == 0 {
+		fs, err := gitLsFiles(log, ".")
+		if err == nil {
+			return fs, nil
+		}
+
+		log.Error(err, "git ls-files failure")
+		args = append(args, ".")
+	}
+
+	return walk(log, args...)
+}
+
+// walk iterates on each path item recursively.
+func walk(log logr.Logger, paths ...string) ([]string, error) {
+	files := make([]string, 0)
+	for _, path := range paths {
+		err := filepath.Walk(path, func(p string, i os.FileInfo, e error) error {
+			if e != nil {
+				return e
+			}
+			mode := i.Mode()
+			if mode.IsRegular() && !mode.IsDir() {
+				log.V(4).Info("index %s", p)
+				files = append(files, p)
+			}
+			return nil
+		})
+		if err != nil {
+			return files, err
+		}
+	}
+	return files, nil
+}
+
+// gitLsFiles returns the list of file based on what is in the git index.
+//
+// -z is mandatory as some repositories non-ASCII file names which creates
+// quoted and escaped file names.
+func gitLsFiles(log logr.Logger, path string) ([]string, error) {
+	output, err := exec.Command("git", "ls-files", "-z", path).Output()
+	if err != nil {
+		return nil, err
+	}
+
+	fs := bytes.Split(output, []byte{0})
+	// last line is empty
+	files := make([]string, len(fs)-1)
+	for i := 0; i < len(files); i++ {
+		p := string(fs[i])
+		log.V(4).Info("index %s", p)
+		files[i] = p
+	}
+	return files, nil
+}

files_test.go

@@ -0,0 +1,128 @@
+package main
+
+import (
+	"fmt"
+	"os"
+	"testing"
+
+	tlogr "github.com/go-logr/logr/testing"
+)
+
+const (
+	// testdataSimple contains a sample editorconfig directory with
+	// some errors.
+	testdataSimple = "testdata/simple"
+)
+
+func TestListFiles(t *testing.T) {
+	l := tlogr.TestLogger{}
+	d := testdataSimple
+	fs, err := listFiles(l, d)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(fs) != 2 {
+		t.Errorf("%s should have two files, got %d", d, len(fs))
+	}
+}
+
+func TestListFilesNoArgs(t *testing.T) {
+	l := tlogr.TestLogger{}
+	d := testdataSimple
+
+	cwd, err := os.Getwd()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() {
+		if err := os.Chdir(cwd); err != nil {
+			t.Fatal(err)
+		}
+	}()
+
+	err = os.Chdir(d)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	fs, err := listFiles(l)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(fs) != 2 {
+		t.Errorf("%s should have two files, got %d", d, len(fs))
+	}
+}
+
+func TestListFilesNoGit(t *testing.T) {
+	// FIXME... should be the null logger, right?
+	l := tlogr.NullLogger{}
+	d := fmt.Sprintf("/tmp/eclint/%d", os.Getpid())
+
+	err := os.MkdirAll(d, 0700)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	cwd, err := os.Getwd()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() {
+		if err := os.Chdir(cwd); err != nil {
+			t.Fatal(err)
+		}
+	}()
+	err = os.Chdir(d)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	fs, err := listFiles(l)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(fs) != 0 {
+		t.Errorf("%s should have two files, got %d", d, len(fs))
+	}
+}
+
+func TestWalk(t *testing.T) {
+	l := tlogr.TestLogger{}
+	d := testdataSimple
+	fs, err := walk(l, d)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(fs) != 2 {
+		t.Errorf("%s should have two files, got %d", d, len(fs))
+	}
+}
+
+func TestGitLsFiles(t *testing.T) {
+	l := tlogr.TestLogger{}
+	d := testdataSimple
+
+	fs, err := gitLsFiles(l, d)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(fs) != 2 {
+		t.Errorf("%s should have two files, got %d", d, len(fs))
+	}
+}
+
+func TestGitLsFilesFailure(t *testing.T) {
+	l := tlogr.TestLogger{}
+	d := fmt.Sprintf("/tmp/eclint/%d", os.Getpid())
+
+	err := os.MkdirAll(d, 0700)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = gitLsFiles(l, d)
+	if err == nil {
+		t.Error("an error was expected")
+	}
+}

lint.go

@@ -15,8 +15,11 @@ import (
 // DefaultTabWidth sets the width of a tab used when counting the line length
 const DefaultTabWidth = 8
 
+// UnsetValue is the value equivalent to an empty / unset one.
+const UnsetValue = "unset"
+
 // validate is where the validations rules are applied
-func validate(r io.Reader, log logr.Logger, def *editorconfig.Definition) []error { //nolint:gocyclo
+func validate(r io.Reader, log logr.Logger, def *editorconfig.Definition) []error { //nolint:funlen,gocyclo
 	var buf *bytes.Buffer
 	// chardet uses a 8192 bytebuf for detection
 	bufSize := 8192
@@ -30,17 +33,17 @@ func validate(r io.Reader, log logr.Logger, def *editorconfig.Definition) []erro
 	var blockCommentStart []byte
 	var blockComment []byte
 	var blockCommentEnd []byte
-	if def.IndentStyle != "" && def.IndentStyle != "unset" {
+	if def.IndentStyle != "" && def.IndentStyle != UnsetValue {
 		bs, ok := def.Raw["block_comment_start"]
-		if ok && bs != "" && bs != "unset" {
+		if ok && bs != "" && bs != UnsetValue {
 			blockCommentStart = []byte(bs)
 			bc, ok := def.Raw["block_comment"]
-			if ok && bc != "" && bs != "unset" {
+			if ok && bc != "" && bs != UnsetValue {
 				blockComment = []byte(bc)
 			}
 
 			be, ok := def.Raw["block_comment_end"]
-			if !ok || be == "" || be == "unset" {
+			if !ok || be == "" || be == UnsetValue {
 				return []error{fmt.Errorf("block_comment_end was expected, none were found")}
 			}
 			blockCommentEnd = []byte(be)
@@ -49,7 +52,7 @@ func validate(r io.Reader, log logr.Logger, def *editorconfig.Definition) []erro
 
 	maxLength := 0
 	tabWidth := def.TabWidth
-	if mll, ok := def.Raw["max_line_length"]; ok && mll != "off" && mll != "unset" {
+	if mll, ok := def.Raw["max_line_length"]; ok && mll != "off" && mll != UnsetValue {
 		ml, err := strconv.Atoi(mll)
 		if err != nil || ml < 0 {
 			return []error{fmt.Errorf("max_line_length expected a non-negative number, got %s", mll)}
@@ -99,7 +102,7 @@ func validate(r io.Reader, log logr.Logger, def *editorconfig.Definition) []erro
 			}
 		}
 
-		if def.IndentStyle != "" && def.IndentStyle != "unset" {
+		if def.IndentStyle != "" && def.IndentStyle != UnsetValue {
 			if insideBlockComment && blockCommentEnd != nil {
 				insideBlockComment = !isBlockCommentEnd(blockCommentEnd, data)
 			}
@@ -202,9 +205,9 @@ func overrideUsingPrefix(def *editorconfig.Definition, prefix string) error {
 				}
 				def.TabWidth = i
 			case "trim_trailing_whitespace":
-				return fmt.Errorf("%v cannot be overriden yet, pr welcome", nk)
+				return fmt.Errorf("%v cannot be overridden yet, pr welcome", nk)
 			case "insert_final_newline":
-				return fmt.Errorf("%v cannot be overriden yet, pr welcome", nk)
+				return fmt.Errorf("%v cannot be overridden yet, pr welcome", nk)
 			}
 		}
 	}