Identical expression comparison analyzer (#7066)

* static analyzer with tests
* Merge branch 'origin-master' into identical-expression-comparison-analyzer
* resolve analyzer errors
* Merge branch 'origin-master' into identical-expression-comparison-analyzer
* change excluded file in nogo_config
* remove tests from bazel file
* exclude test file explicitly
* extracted common code to helper file
* Revert "extracted common code to helper file"

This reverts commit e9ccea73604c920451975214d0305e6d16943b3c.
* Merge refs/heads/master into identical-expression-comparison-analyzer
* Merge refs/heads/master into identical-expression-comparison-analyzer

BUILD.bazel

@@ -108,6 +108,7 @@ nogo(
         "//tools/analyzers/cryptorand:go_tool_library",
         "//tools/analyzers/errcheck:go_tool_library",
         "//tools/analyzers/featureconfig:go_tool_library",
+        "//tools/analyzers/comparesame:go_tool_library",
     ] + select({
         # nogo checks that fail with coverage enabled.
         ":coverage_enabled": [],

nogo_config.json

@@ -109,5 +109,12 @@
       "shared/rand/rand\\.go": "Abstracts CSPRNGs for common use",
       "shared/aggregation/testing/bitlistutils.go": "Test-only package"
     }
+  },
+  "comparesame": {
+    "exclude_files": {
+      "external/.*": "Third party code",
+      "rules_go_work-.*": "Third party code",
+      "tools/analyzers/comparesame/testdata/compare_len.go": "Analyzer testdata has to break rules"
+    }
   }
 }

tools/analyzers/comparesame/BUILD.bazel

@@ -0,0 +1,28 @@
+load("@prysm//tools/go:def.bzl", "go_library")
+load("@io_bazel_rules_go//go:def.bzl", "go_tool_library")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["analyzer.go"],
+    importpath = "github.com/prysmaticlabs/prysm/tools/analyzers/comparesame",
+    visibility = ["//visibility:public"],
+    deps = [
+        "@org_golang_x_tools//go/analysis:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/inspect:go_default_library",
+        "@org_golang_x_tools//go/ast/inspector:go_default_library",
+    ],
+)
+
+go_tool_library(
+    name = "go_tool_library",
+    srcs = ["analyzer.go"],
+    importpath = "github.com/prysmaticlabs/prysm/tools/analyzers/comparesame",
+    visibility = ["//visibility:public"],
+    deps = [
+        "@org_golang_x_tools//go/analysis:go_tool_library",
+        "@org_golang_x_tools//go/analysis/passes/inspect:go_tool_library",
+        "@org_golang_x_tools//go/ast/inspector:go_tool_library",
+    ],
+)
+
+# gazelle:exclude analyzer_test.go

tools/analyzers/comparesame/analyzer.go

@@ -0,0 +1,65 @@
+package comparesame
+
+import (
+	"bytes"
+	"errors"
+	"go/ast"
+	"go/printer"
+	"go/token"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+	"golang.org/x/tools/go/ast/inspector"
+)
+
+// Doc explaining the tool.
+const Doc = "Tool to detect comparison (==, !=, >=, <=, >, <) of identical boolean expressions."
+
+const messageTemplate = "Boolean expression has identical expressions on both sides. The result is always %v."
+
+// Analyzer runs static analysis.
+var Analyzer = &analysis.Analyzer{
+	Name:     "comparesame",
+	Doc:      Doc,
+	Requires: []*analysis.Analyzer{inspect.Analyzer},
+	Run:      run,
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+	inspect, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+	if !ok {
+		return nil, errors.New("analyzer is not type *inspector.Inspector")
+	}
+
+	nodeFilter := []ast.Node{
+		(*ast.BinaryExpr)(nil),
+	}
+
+	inspect.Preorder(nodeFilter, func(node ast.Node) {
+		expr, ok := node.(*ast.BinaryExpr)
+		if !ok {
+			return
+		}
+
+		switch expr.Op {
+		case token.EQL, token.NEQ, token.GEQ, token.LEQ, token.GTR, token.LSS:
+			var xBuf, yBuf bytes.Buffer
+			if err := printer.Fprint(&xBuf, pass.Fset, expr.X); err != nil {
+				pass.Reportf(expr.X.Pos(), err.Error())
+			}
+			if err := printer.Fprint(&yBuf, pass.Fset, expr.Y); err != nil {
+				pass.Reportf(expr.Y.Pos(), err.Error())
+			}
+			if xBuf.String() == yBuf.String() {
+				switch expr.Op {
+				case token.EQL, token.NEQ, token.GEQ, token.LEQ:
+					pass.Reportf(expr.OpPos, messageTemplate, true)
+				case token.GTR, token.LSS:
+					pass.Reportf(expr.OpPos, messageTemplate, false)
+				}
+			}
+		}
+	})
+
+	return nil, nil
+}

tools/analyzers/comparesame/analyzer_test.go

@@ -0,0 +1,11 @@
+package comparesame
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+func TestAnalyzer(t *testing.T) {
+	analysistest.Run(t, analysistest.TestData(), Analyzer)
+}

tools/analyzers/comparesame/testdata/BUILD.bazel

@@ -0,0 +1,8 @@
+load("@prysm//tools/go:def.bzl", "go_library")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["compare_len.go"],
+    importpath = "github.com/prysmaticlabs/prysm/tools/analyzers/comparesame/testdata",
+    visibility = ["//visibility:public"],
+)

tools/analyzers/comparesame/testdata/compare_len.go

@@ -0,0 +1,37 @@
+package testdata
+
+func Equal() {
+	x := []string{"a"}
+	if len(x) == len(x) { // want "Boolean expression has identical expressions on both sides. The result is always true."
+	}
+}
+
+func NotEqual() {
+	x := []string{"a"}
+	if len(x) != len(x) { // want "Boolean expression has identical expressions on both sides. The result is always true."
+	}
+}
+
+func GreaterThanOrEqual() {
+	x := []string{"a"}
+	if len(x) >= len(x) { // want "Boolean expression has identical expressions on both sides. The result is always true."
+	}
+}
+
+func LessThanOrEqual() {
+	x := []string{"a"}
+	if len(x) <= len(x) { // want "Boolean expression has identical expressions on both sides. The result is always true."
+	}
+}
+
+func GreaterThan() {
+	x := []string{"a"}
+	if len(x) > len(x) { // want "Boolean expression has identical expressions on both sides. The result is always false."
+	}
+}
+
+func LessThan() {
+	x := []string{"a"}
+	if len(x) < len(x) { // want "Boolean expression has identical expressions on both sides. The result is always false."
+	}
+}