Merge pull request #9 from bzz/fix-path

Fix path in comment text

.travis.yml

@@ -8,6 +8,8 @@ stages:
   - name: release
     if: tag IS present
 
+go_import_path: github.com/src-d/lookout-gometalint-analyzer
+
 jobs:
   include:
     - name: 'Unit tests'
@@ -37,7 +39,7 @@ jobs:
         - cat analyzer.log
         - cat ../lookout-install.log
 
-    - name: 'Generated code'
+    - name: 'Check deps'
       install:
         - curl https://raw.githubusercontent.com/golang/dep/master/install.sh | sh
       script:

README.md

@@ -44,8 +44,8 @@ $ lookout-sdk review -v ipv4://localhost:2001 \
 | -- | -- | -- |
 | `GOMETALINT_HOST` | `0.0.0.0` | IP address to bind the gRCP serve |
 | `GOMETALINT_PORT` | `2001` | Port to bind the gRPC server |
-| `GOMETALINT_SERVER_URL` | `ipv4://localhost:10302` | gRPC URL of the [Data service](https://github.com/src-d/lookout/tree/master/docs#components)
-| `GOMETALINT_LOG_LEVEL` | `info` | Logging level (info, debug, warning or error) |
+| `GOMETALINT_DATA_SERVICE_URL` | `ipv4://localhost:10301` | gRPC URL of the [Data service](https://github.com/src-d/lookout/tree/master/docs#components)
+| `GOMETALINT_LOG_LEVEL` | `info` | Logging level ("info", "debug", "warning" or "error") |
 
 
 # Licens

_tools/install-lookout-latest.sh

@@ -4,23 +4,34 @@
 # Depends on GNU grep
 set -x
 
-#TODO(bzz): check local cached version
+#TODO(bzz): check for local cached version first
 
-curl -s --connect-timeout 5 \
+OS="$(uname | tr '[:upper:]' '[:lower:]')"
+
+oIFS=$IFS IFS=' '
+curl -v ${GITHUB_TOKEN:+'-H' "Authorization: token $GITHUB_TOKEN"} \
+    --connect-timeout 5 \
     --max-time 10 \
     --retry 5 \
     --retry-delay 0 \
     --retry-max-time 40\
     "https://api.github.com/repos/src-d/lookout/releases/latest" \
-  |& tee -a ../lookout-install.log \
+  | tee -a ../lookout-install.log \
   | grep -oP '"browser_download_url": "\K(.*)(?=")' \
-  | grep linux \
+  | grep "${OS}" \
   | wget -qi -
 
-if [[ "${PIPESTATUS[0]}" -ne 0 || "${PIPESTATUS[1]}" -ne 0 || "${PIPESTATUS[2]}" -ne 0 || "${PIPESTATUS[4]}" -ne 0 ]]; then
+if [[ "${PIPESTATUS[0]}" -ne 0 || \
+      "${PIPESTATUS[1]}" -ne 0 || \
+      "${PIPESTATUS[2]}" -ne 0 || \
+      "${PIPESTATUS[3]}" -ne 0 || \
+      "${PIPESTATUS[4]}" -ne 0 ]];
+then
   echo "Unable download latest lookout SDK release" >&2
   exit 2
 fi
+IFS=$oIFS; unset -v oIFS
+# http://mywiki.wooledge.org/BashFAQ/050#I_only_want_to_pass_options_if_the_runtime_data_needs_them
 
 if ! tar -xvzf lookout-sdk_*.tar.gz ; then
   echo "Unable to extract lookout release archive" >&2

analyzer.go

@@ -102,19 +102,18 @@ func (a *Analyzer) NotifyReviewEvent(ctx context.Context, e *lookout.ReviewEvent
 		log.Debugf("no Golang files found. skip running gometalinter")
 		return &lookout.EventResponse{AnalyzerVersion: a.Version}, nil
 	}
+	log.Debugf("%d Golang files found. running gometalinter", saved)
 
 	withArgs := append(append(a.Args, tmp), a.linterArguments(e.Configuration)...)
 	comments := RunGometalinter(withArgs)
 	var allComments []*lookout.Comment
 	for _, comment := range comments {
-		//TrimLeft(, tmp) but \w rel path
-		file := comment.file[strings.LastIndex(comment.file, tmp)+len(tmp):]
+		origPathFile := revertOriginalPath(comment.file, tmp)
+		origPathText := revertOriginalPathIn(comment.text, tmp)
 		newComment := lookout.Comment{
-			File: strings.TrimLeft(
-				path.Join(strings.Split(file, artificialSep)...),
-				string(os.PathSeparator)),
+			File: origPathFile,
 			Line: comment.lino,
-			Text: comment.text,
+			Text: origPathText,
 		}
 		allComments = append(allComments, &newComment)
 		log.Debugf("Get comment %v", newComment)
@@ -127,16 +126,47 @@ func (a *Analyzer) NotifyReviewEvent(ctx context.Context, e *lookout.ReviewEvent
 	}, nil
 }
 
+// flattenPath flattens relative path and puts it inside tmp.
+func flattenPath(file string, tmp string) string {
+	nFile := strings.Join(strings.Split(file, string(os.PathSeparator)), artificialSep)
+	nPath := path.Join(tmp, nFile)
+	return nPath
+}
+
+// revertOriginalPath reverses origina path from a flat one.
+func revertOriginalPath(file string, tmp string) string {
+	//TrimLeft(, tmp) but works for rel paths
+	noTmpfile := file[strings.Index(file, tmp)+len(tmp):]
+	origPathFile := strings.TrimLeft(
+		path.Join(strings.Split(noTmpfile, artificialSep)...),
+		string(os.PathSeparator))
+	return origPathFile
+}
+
+// revertOriginalPathIn a given text, recovers original path in words
+// that have 'artificialSep'.
+func revertOriginalPathIn(text string, tmp string) string {
+	if strings.LastIndex(text, artificialSep) < 0 {
+		return text
+	}
+	var words []string
+	for _, word := range strings.Fields(text) {
+		if strings.Index(word, artificialSep) >= 0 {
+			word = revertOriginalPath(word, tmp)
+		}
+		words = append(words, word)
+	}
+	return strings.Join(words, " ")
+}
+
 // tryToSaveTo saves a file to given dir, preserving it's original path.
 // It only loggs any errors and does not fail. All files saved this way will
 // be in the root of the same dir.
 func tryToSaveTo(file *lookout.File, tmp string) {
-	nFile := strings.Join(strings.Split(file.Path, string(os.PathSeparator)), artificialSep)
-	nPath := path.Join(tmp, nFile)
-	log.Debugf("Saving file '%s', as '%s'", file.Path, nPath)
-	err := ioutil.WriteFile(nPath, file.Content, 0644)
+	flatPath := flattenPath(file.Path, tmp)
+	err := ioutil.WriteFile(flatPath, file.Content, 0644)
 	if err != nil {
-		log.Errorf(err, "failed to write a file %s", nPath)
+		log.Errorf(err, "failed to write a file %q", flatPath)
 	}
 }
 func (a *Analyzer) NotifyPushEvent(ctx context.Context, e *lookout.PushEvent) (*lookout.EventResponse, error) {

analyzer_test.go

@@ -5,6 +5,7 @@ import (
 
 	types "github.com/gogo/protobuf/types"
 	"github.com/src-d/lookout/util/grpchelper"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -75,3 +76,32 @@ func TestArgsCorrect(t *testing.T) {
 		},
 	})))
 }
+
+var pathTests = []struct {
+	in  string
+	out string
+}{
+	{"a/b.go", "/tmp/a___.___b.go"},
+	{"tmp/a/b.go", "/tmp/tmp___.___a___.___b.go"},
+	{"a/b/c/d/e.go", "/tmp/a___.___b___.___c___.___d___.___e.go"},
+}
+
+func TestPathTransformations(t *testing.T) {
+	for _, tt := range pathTests {
+		t.Run(tt.in, func(t *testing.T) {
+			assert.Equal(t, tt.out, flattenPath(tt.in, "/tmp"))
+			assert.Equal(t, tt.in, revertOriginalPath(tt.out, "/tmp"))
+		})
+	}
+}
+
+func TestPathInTextTransformations(t *testing.T) {
+	tmp := "/var/folders/rx/z9zyr71d70x92zwbn3rrjx4c0000gn/T/gometalint584398570"
+	text := "duplicate of /var/folders/rx/z9zyr71d70x92zwbn3rrjx4c0000gn/T/gometalint584398570/provider___.___github___.___poster_test.go:549-554 (dupl)"
+	expectedText := "duplicate of provider/github/poster_test.go:549-554 (dupl)"
+
+	newText := revertOriginalPathIn(text, tmp)
+	if newText != expectedText {
+		t.Fatalf("got %q, want %q", newText, expectedText)
+	}
+}

cmd/gometalint-analyzer/main.go

@@ -52,6 +52,8 @@ func main() {
 	var conf config
 	envconfig.MustProcess("GOMETALINT", &conf)
 	log.Infof("Starting %s, %s", name, litter.Sdump(conf))
+	log.DefaultFactory = &log.LoggerFactory{Level: conf.LogLevel}
+	log.DefaultLogger = log.New(nil)
 
 	grpcAddr, err := grpchelper.ToGoGrpcAddress(conf.DataServiceURL)
 	if err != nil {

gometalint.go

@@ -30,7 +30,7 @@ type Comment struct {
 func RunGometalinter(args []string) []Comment {
 	dArgs := append([]string(nil), defaultArgs...)
 	args = append(dArgs, args...)
-	log.Infof("Running '%s %v'\n", bin, args)
+	log.Debugf("Running '%s %v'\n", bin, args)
 	out, _ := exec.Command(bin, args...).Output() // nolint: gas
 	// ignoring err, as it's always not nil if anything found