From da63d0a9655db4b84c5599db41e0d967efdf5bf3 Mon Sep 17 00:00:00 2001
From: SalvadorC <salvadorcavadini+github@gmail.com>
Date: Wed, 3 Oct 2018 20:56:57 +0200
Subject: [PATCH] Refactors `atomic` rule code to move `gofmt` function to
 utils.go (issue #79)  (#80)

* Refactoring on atomic rule:
-Main modification: move func gofmt to utils.go

* Refactoring on constant-logical-expr rule
Simplifies equality check of subexpressions by using gofmt function
---
 rule/atomic.go                | 31 ++++++++++---------------------
 rule/constant-logical-expr.go | 20 +-------------------
 rule/utils.go                 | 10 ++++++++++
 3 files changed, 21 insertions(+), 40 deletions(-)

diff --git a/rule/atomic.go b/rule/atomic.go
index 89daa79..572e141 100644
--- a/rule/atomic.go
+++ b/rule/atomic.go
@@ -1,10 +1,7 @@
 package rule
 
 import (
-	"bytes"
-	"fmt"
 	"go/ast"
-	"go/printer"
 	"go/token"
 	"go/types"
 
@@ -18,7 +15,7 @@ type AtomicRule struct{}
 func (r *AtomicRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
 	var failures []lint.Failure
 	walker := atomic{
-		file: file,
+		pkgTypesInfo: file.Pkg.TypesInfo,
 		onFailure: func(failure lint.Failure) {
 			failures = append(failures, failure)
 		},
@@ -35,8 +32,8 @@ func (r *AtomicRule) Name() string {
 }
 
 type atomic struct {
-	file      *lint.File
-	onFailure func(lint.Failure)
+	pkgTypesInfo *types.Info
+	onFailure    func(lint.Failure)
 }
 
 func (w atomic) Visit(node ast.Node) ast.Visitor {
@@ -46,10 +43,10 @@ func (w atomic) Visit(node ast.Node) ast.Visitor {
 	}
 
 	if len(n.Lhs) != len(n.Rhs) {
-		return w
+		return nil // skip assignment sub-tree
 	}
 	if len(n.Lhs) == 1 && n.Tok == token.DEFINE {
-		return w
+		return nil // skip assignment sub-tree
 	}
 
 	for i, right := range n.Rhs {
@@ -62,8 +59,8 @@ func (w atomic) Visit(node ast.Node) ast.Visitor {
 			continue
 		}
 		pkgIdent, _ := sel.X.(*ast.Ident)
-		if w.file.Pkg.TypesInfo != nil {
-			pkgName, ok := w.file.Pkg.TypesInfo.Uses[pkgIdent].(*types.PkgName)
+		if w.pkgTypesInfo != nil {
+			pkgName, ok := w.pkgTypesInfo.Uses[pkgIdent].(*types.PkgName)
 			if !ok || pkgName.Imported().Path() != "sync/atomic" {
 				continue
 			}
@@ -79,15 +76,15 @@ func (w atomic) Visit(node ast.Node) ast.Visitor {
 			broken := false
 
 			if uarg, ok := arg.(*ast.UnaryExpr); ok && uarg.Op == token.AND {
-				broken = w.gofmt(left) == w.gofmt(uarg.X)
+				broken = gofmt(left) == gofmt(uarg.X)
 			} else if star, ok := left.(*ast.StarExpr); ok {
-				broken = w.gofmt(star.X) == w.gofmt(arg)
+				broken = gofmt(star.X) == gofmt(arg)
 			}
 
 			if broken {
 				w.onFailure(lint.Failure{
 					Confidence: 1,
-					Failure:    fmt.Sprintf("direct assignment to atomic value"),
+					Failure:    "direct assignment to atomic value",
 					Node:       n,
 				})
 			}
@@ -95,11 +92,3 @@ func (w atomic) Visit(node ast.Node) ast.Visitor {
 	}
 	return w
 }
-
-// gofmt returns a string representation of the expression.
-func (w atomic) gofmt(x ast.Expr) string {
-	buf := bytes.Buffer{}
-	fs := token.NewFileSet()
-	printer.Fprint(&buf, fs, x)
-	return buf.String()
-}
diff --git a/rule/constant-logical-expr.go b/rule/constant-logical-expr.go
index 8764154..6a91561 100644
--- a/rule/constant-logical-expr.go
+++ b/rule/constant-logical-expr.go
@@ -1,11 +1,8 @@
 package rule
 
 import (
-	"bytes"
-	"fmt"
 	"github.com/mgechev/revive/lint"
 	"go/ast"
-	"go/format"
 	"go/token"
 )
 
@@ -43,7 +40,7 @@ func (w *lintConstantLogicalExpr) Visit(node ast.Node) ast.Visitor {
 			return w
 		}
 
-		if !w.areEqual(n.X, n.Y) {
+		if gofmt(n.X) != gofmt(n.Y) { // check if subexpressions are the same
 			return w
 		}
 
@@ -81,21 +78,6 @@ func (w *lintConstantLogicalExpr) isInequalityOperator(t token.Token) bool {
 	return false
 }
 
-func (w lintConstantLogicalExpr) areEqual(x, y ast.Expr) bool {
-	fset := token.NewFileSet()
-	var buf1 bytes.Buffer
-	if err := format.Node(&buf1, fset, x); err != nil {
-		return false // keep going in case of error
-	}
-
-	var buf2 bytes.Buffer
-	if err := format.Node(&buf2, fset, y); err != nil {
-		return false // keep going in case of error
-	}
-
-	return fmt.Sprintf("%s", buf1.Bytes()) == fmt.Sprintf("%s", buf2.Bytes())
-}
-
 func (w lintConstantLogicalExpr) newFailure(node ast.Node, msg string) {
 	w.onFailure(lint.Failure{
 		Confidence: 1,
diff --git a/rule/utils.go b/rule/utils.go
index ee6b3da..6ba542b 100644
--- a/rule/utils.go
+++ b/rule/utils.go
@@ -1,8 +1,10 @@
 package rule
 
 import (
+	"bytes"
 	"fmt"
 	"go/ast"
+	"go/printer"
 	"go/token"
 	"go/types"
 	"regexp"
@@ -179,3 +181,11 @@ func isExprABooleanLit(n ast.Node) (lexeme string, ok bool) {
 
 	return oper.Name, (oper.Name == trueName || oper.Name == falseName)
 }
+
+// gofmt returns a string representation of the expression.
+func gofmt(x ast.Expr) string {
+	buf := bytes.Buffer{}
+	fs := token.NewFileSet()
+	printer.Fprint(&buf, fs, x)
+	return buf.String()
+}