Repository: mgechev/revive (go)

Issue:
Extra empty line at the end of a block because that empty line is a comment
(i am currently in the process of adding "revive" to our linter stack and i am going through the false-positives for us)

**Describe the bug**

```go
{
   some code

   // comment
}
```

leads to "extra empty line at the end of a block" even though it shouldn't.

**Expected behavior**

Such comments are meant for exactly being at that location. Marking the comment as a probem, is a false-positive for our source code.


Pull request:
fix #670
Closes #670


Patch:
diff --git a/rule/empty-lines.go b/rule/empty-lines.go
index b443476..1286607 100644
--- a/rule/empty-lines.go
+++ b/rule/empty-lines.go
@@ -11,14 +11,14 @@ import (
 type EmptyLinesRule struct{}
 
 // Apply applies the rule to given file.
-func (*EmptyLinesRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
+func (r *EmptyLinesRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
 	var failures []lint.Failure
 
 	onFailure := func(failure lint.Failure) {
 		failures = append(failures, failure)
 	}
 
-	w := lintEmptyLines{file, file.CommentMap(), onFailure}
+	w := lintEmptyLines{file, r.commentLines(file.CommentMap(), file), onFailure}
 	ast.Walk(w, file.AST)
 	return failures
 }
@@ -30,13 +30,13 @@ func (*EmptyLinesRule) Name() string {
 
 type lintEmptyLines struct {
 	file      *lint.File
-	cmap      ast.CommentMap
+	cmap      map[int]struct{}
 	onFailure func(lint.Failure)
 }
 
 func (w lintEmptyLines) Visit(node ast.Node) ast.Visitor {
 	block, ok := node.(*ast.BlockStmt)
-	if !ok {
+	if !ok || len(block.List) == 0 {
 		return w
 	}
 
@@ -47,67 +47,59 @@ func (w lintEmptyLines) Visit(node ast.Node) ast.Visitor {
 }
 
 func (w lintEmptyLines) checkStart(block *ast.BlockStmt) {
-	if len(block.List) == 0 {
-		return
-	}
-
-	start := w.position(block.Lbrace)
+	blockStart := w.position(block.Lbrace)
 	firstNode := block.List[0]
+	firstStmt := w.position(firstNode.Pos())
 
-	if w.commentBetween(start, firstNode) {
+	firstBlockLineIsStmt := firstStmt.Line-(blockStart.Line+1) == 0
+	_, firstBlockLineIsComment := w.cmap[blockStart.Line+1]
+	if firstBlockLineIsStmt || firstBlockLineIsComment {
 		return
 	}
 
-	first := w.position(firstNode.Pos())
-	if first.Line-start.Line > 1 {
-		w.onFailure(lint.Failure{
-			Confidence: 1,
-			Node:       block,
-			Category:   "style",
-			Failure:    "extra empty line at the start of a block",
-		})
-	}
+	w.onFailure(lint.Failure{
+		Confidence: 1,
+		Node:       block,
+		Category:   "style",
+		Failure:    "extra empty line at the start of a block",
+	})
 }
 
 func (w lintEmptyLines) checkEnd(block *ast.BlockStmt) {
-	if len(block.List) < 1 {
-		return
-	}
-
-	end := w.position(block.Rbrace)
+	blockEnd := w.position(block.Rbrace)
 	lastNode := block.List[len(block.List)-1]
+	lastStmt := w.position(lastNode.End())
 
-	if w.commentBetween(end, lastNode) {
+	lastBlockLineIsStmt := (blockEnd.Line-1)-lastStmt.Line == 0
+	_, lastBlockLineIsComment := w.cmap[blockEnd.Line-1]
+	if lastBlockLineIsStmt || lastBlockLineIsComment {
 		return
 	}
 
-	last := w.position(lastNode.End())
-	if end.Line-last.Line > 1 {
-		w.onFailure(lint.Failure{
-			Confidence: 1,
-			Node:       lastNode,
-			Category:   "style",
-			Failure:    "extra empty line at the end of a block",
-		})
-	}
+	w.onFailure(lint.Failure{
+		Confidence: 1,
+		Node:       block,
+		Category:   "style",
+		Failure:    "extra empty line at the end of a block",
+	})
 }
 
-func (w lintEmptyLines) commentBetween(position token.Position, node ast.Node) bool {
-	comments := w.cmap.Filter(node).Comments()
-	if len(comments) == 0 {
-		return false
-	}
+func (w lintEmptyLines) position(pos token.Pos) token.Position {
+	return w.file.ToPosition(pos)
+}
+
+func (*EmptyLinesRule) commentLines(cmap ast.CommentMap, file *lint.File) map[int]struct{} {
+	result := map[int]struct{}{}
 
-	for _, comment := range comments {
-		start, end := w.position(comment.Pos()), w.position(comment.End())
-		if start.Line-position.Line == 1 || position.Line-end.Line == 1 {
-			return true
+	for _, comments := range cmap {
+		for _, comment := range comments {
+			start := file.ToPosition(comment.Pos())
+			end := file.ToPosition(comment.End())
+			for i := start.Line; i <= end.Line; i++ {
+				result[i] = struct{}{}
+			}
 		}
 	}
 
-	return false
-}
-
-func (w lintEmptyLines) position(pos token.Pos) token.Position {
-	return w.file.ToPosition(pos)
+	return result
 }


Test patch:
diff --git a/testdata/empty-lines.go b/testdata/empty-lines.go
index 0e9d57c..a37a778 100644
--- a/testdata/empty-lines.go
+++ b/testdata/empty-lines.go
@@ -9,14 +9,14 @@ func f1(x *int) bool { // MATCH /extra empty line at the start of a block/
 	return x > 2
 }
 
-func f2(x *int) bool {
-	return x > 2 // MATCH /extra empty line at the end of a block/
+func f2(x *int) bool { // MATCH /extra empty line at the end of a block/
+	return x > 2
 
 }
 
 func f3(x *int) bool { // MATCH /extra empty line at the start of a block/
 
-	return x > 2 // MATCH /extra empty line at the end of a block/
+	return x > 2 // MATCH:17 /extra empty line at the end of a block/
 
 }
 
@@ -36,8 +36,8 @@ func f6(x *int) bool {
 	// This is fine.
 }
 
-func f7(x *int) bool {
-	return x > 2 // MATCH /extra empty line at the end of a block/
+func f7(x *int) bool { // MATCH /extra empty line at the end of a block/
+	return x > 2
 	// This is _not_ fine.
 
 }
@@ -52,8 +52,8 @@ func f8(*int) bool {
 }
 
 func f9(*int) bool {
-	if x > 2 {
-		return true // MATCH /extra empty line at the end of a block/
+	if x > 2 { // MATCH /extra empty line at the end of a block/
+		return true
 
 	}
 
@@ -75,8 +75,8 @@ func f11(x *int) bool {
 	}
 }
 
-func f12(x *int) bool {
-	if x > 2 { // MATCH /extra empty line at the end of a block/
+func f12(x *int) bool { // MATCH /extra empty line at the end of a block/
+	if x > 2 {
 		return true
 	}
 
@@ -89,8 +89,8 @@ func f13(x *int) bool {
 	}
 }
 
-func f14(x *int) bool {
-	switch { // MATCH /extra empty line at the end of a block/
+func f14(x *int) bool { // MATCH /extra empty line at the end of a block/
+	switch {
 	case x == 2:
 		return false
 	}
@@ -98,8 +98,8 @@ func f14(x *int) bool {
 }
 
 func f15(x *int) bool {
-	switch {
-	case x == 2: // MATCH /extra empty line at the end of a block/
+	switch { // MATCH /extra empty line at the end of a block/
+	case x == 2:
 		return false
 
 	}
@@ -111,8 +111,8 @@ func f16(x *int) bool {
 	).Execute()
 }
 
-func f17(x *int) bool {
-	return Query( // MATCH /extra empty line at the end of a block/
+func f17(x *int) bool { // MATCH /extra empty line at the end of a block/
+	return Query(
 		qm("x = ?", x),
 	).Execute()
 
@@ -120,7 +120,7 @@ func f17(x *int) bool {
 
 func f18(x *int) bool {
 	if true {
-		if true { // MATCH /extra empty line at the end of a block/
+		if true {
 			return true
 		}
 
@@ -160,3 +160,11 @@ func x() {
 		}
 	}
 }
+
+func ShouldNotWarn() {
+	// comment
+
+	println()
+
+	// comment
+}