Merge pull request #29 from golangci/support/fix-nolint-with-spaces

#26: fix spaces in nolint directive

Author: golangci

pkg/result/processors/nolint.go

@@ -115,7 +115,6 @@ func (p *Nolint) shouldPassIssue(i *result.Issue) (bool, error) {
 			if i.FromLinter == linter {
 				return false, nil
 			}
-			// TODO: check linter name
 		}
 	}
 
@@ -127,20 +126,30 @@ func extractFileComments(fset *token.FileSet, comments ...*ast.CommentGroup) fil
 	for _, g := range comments {
 		for _, c := range g.List {
 			text := strings.TrimLeft(c.Text, "/ ")
-			if strings.HasPrefix(text, "nolint") {
-				var linters []string
-				if strings.HasPrefix(text, "nolint:") {
-					text = strings.Split(text, " ")[0] // allow arbitrary text after this comment
-					for _, linter := range strings.Split(strings.TrimPrefix(text, "nolint:"), ",") {
-						linters = append(linters, strings.TrimSpace(linter))
-					}
-				}
-				pos := fset.Position(g.Pos())
+			if !strings.HasPrefix(text, "nolint") {
+				continue
+			}
+
+			pos := fset.Position(g.Pos())
+			if !strings.HasPrefix(text, "nolint:") { // ignore all linters
 				ret = append(ret, comment{
-					linters: linters,
-					line:    pos.Line,
+					line: pos.Line,
 				})
+				continue
+			}
+
+			// ignore specific linters
+			var linters []string
+			text = strings.Split(text, "//")[0] // allow another comment after this comment
+			linterItems := strings.Split(strings.TrimPrefix(text, "nolint:"), ",")
+			for _, linter := range linterItems {
+				linterName := strings.TrimSpace(linter) // TODO: validate it here
+				linters = append(linters, linterName)
 			}
+			ret = append(ret, comment{
+				linters: linters,
+				line:    pos.Line,
+			})
 		}
 	}
 

pkg/result/processors/nolint_test.go

@@ -24,10 +24,17 @@ func TestNolint(t *testing.T) {
 	processAssertEmpty(t, p, newNolintFileIssue(3, "gofmt")) // check cached is ok
 	processAssertSame(t, p, newNolintFileIssue(3, "gofmtA")) // check different name
 
-	processAssertEmpty(t, p, newNolintFileIssue(4, "any"))
-	processAssertEmpty(t, p, newNolintFileIssue(5, "any"))
+	processAssertEmpty(t, p, newNolintFileIssue(4, "gofmt"))
+	processAssertSame(t, p, newNolintFileIssue(4, "gofmtA")) // check different name
 
-	processAssertSame(t, p, newNolintFileIssue(1, "golint"))
+	processAssertEmpty(t, p, newNolintFileIssue(5, "gofmt"))
+	processAssertEmpty(t, p, newNolintFileIssue(5, "govet"))
+	processAssertSame(t, p, newNolintFileIssue(5, "gofmtA")) // check different name
+
+	processAssertEmpty(t, p, newNolintFileIssue(6, "any"))
+	processAssertEmpty(t, p, newNolintFileIssue(7, "any"))
+
+	processAssertSame(t, p, newNolintFileIssue(1, "golint")) // no directive
 }
 
 func TestNoIssuesInAutogeneratedFile(t *testing.T) {

pkg/result/processors/testdata/nolint.go

@@ -1,5 +1,7 @@
 package testdata
 
 var nolintSpecific int    // nolint:gofmt
+var nolintSpace int       // nolint: gofmt
+var nolintSpaces int      //nolint: gofmt, govet
 var nolintAll int         // nolint
-var nolintAndAppendix int // nolint Some My Text
+var nolintAndAppendix int // nolint // another comment