Repository: mgechev/revive (go)

Issue:
add new rule to suggest using time.Time.Equal instead of == and !=
according to this [link](https://pkg.go.dev/time#Time) is better used `time.Time.Equal` method instead of == and != in most codes

Pull request:
add new rule time equal 
Closes #583 

Patch:
diff --git a/README.md b/README.md
index 9fb6e36..0056ad3 100644
--- a/README.md
+++ b/README.md
@@ -398,6 +398,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | Name                  | Config | Description                                                      | `golint` | Typed |
 | --------------------- | :----: | :--------------------------------------------------------------- | :------: | :---: |
 | [`context-keys-type`](./RULES_DESCRIPTIONS.md#context-key-types)   |  n/a   | Disallows the usage of basic types in `context.WithValue`.       |   yes    |  yes  |
+| [`time-equal`](./RULES_DESCRIPTIONS.md#time-equal)         |  n/a   | Suggests to use `time.Time.Equal` instead of `==` and `!=` for equality check time.                 |   no    |  yes  |
 | [`time-naming`](./RULES_DESCRIPTIONS.md#time-naming)         |  n/a   | Conventions around the naming of time variables.                 |   yes    |  yes  |
 | [`var-declaration`](./RULES_DESCRIPTIONS.md#var-declaration)     |  n/a   | Reduces redundancies around variable declaration.                |   yes    |  yes  |
 | [`unexported-return`](./RULES_DESCRIPTIONS.md#unexported-return)   |  n/a   | Warns when a public return is from unexported type.              |   yes    |  yes  |
diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md
index 2b9f589..5a3848b 100644
--- a/RULES_DESCRIPTIONS.md
+++ b/RULES_DESCRIPTIONS.md
@@ -56,6 +56,7 @@ List of all available rules.
   - [struct-tag](#struct-tag)
   - [string-format](#string-format)
   - [superfluous-else](#superfluous-else)
+  - [time-equal](#time-equal)
   - [time-naming](#time-naming)
   - [var-naming](#var-naming)
   - [var-declaration](#var-declaration)
@@ -547,6 +548,12 @@ This rule highlights redundant _else-blocks_ that can be eliminated from the cod
 
 _Configuration_: N/A
 
+## time-equal
+
+_Description_: This rule warns when using `==` and `!=` for equality check `time.Time` and suggest to `time.time.Equal` method, for about information follow this [link](https://pkg.go.dev/time#Time)
+
+_Configuration_: N/A
+
 ## time-naming
 
 _Description_: Using unit-specific suffix like "Secs", "Mins", ... when naming variables of type `time.Duration` can be misleading, this rule highlights those cases.
diff --git a/config/config.go b/config/config.go
index 298c0bd..8666cf4 100644
--- a/config/config.go
+++ b/config/config.go
@@ -81,6 +81,7 @@ var allRules = append([]lint.Rule{
 	&rule.NestedStructs{},
 	&rule.IfReturnRule{},
 	&rule.UselessBreak{},
+	&rule.TimeEqualRule{},
 }, defaultRules...)
 
 var allFormatters = []lint.Formatter{
@@ -105,30 +106,6 @@ func getFormatters() map[string]lint.Formatter {
 
 // GetLintingRules yields the linting rules that must be applied by the linter
 func GetLintingRules(config *lint.Config) ([]lint.Rule, error) {
-	if config.EnableAllRules {
-		return getAllRules(config)
-	}
-
-	return getEnabledRules(config)
-}
-
-// getAllRules yields the list of all available rules except those disabled by configuration
-func getAllRules(config *lint.Config) ([]lint.Rule, error) {
-	lintingRules := []lint.Rule{}
-	for _, r := range allRules {
-		ruleConf := config.Rules[r.Name()]
-		if ruleConf.Disabled {
-			continue // skip disabled rules
-		}
-
-		lintingRules = append(lintingRules, r)
-	}
-
-	return lintingRules, nil
-}
-
-// getEnabledRules yields the list of rules that are enabled by configuration
-func getEnabledRules(config *lint.Config) ([]lint.Rule, error) {
 	rulesMap := map[string]lint.Rule{}
 	for _, r := range allRules {
 		rulesMap[r.Name()] = r
@@ -165,9 +142,27 @@ func parseConfig(path string) (*lint.Config, error) {
 }
 
 func normalizeConfig(config *lint.Config) {
+	const defaultConfidence = 0.8
 	if config.Confidence == 0 {
-		config.Confidence = 0.8
+		config.Confidence = defaultConfidence
+	}
+
+	if len(config.Rules) == 0 {
+		config.Rules = map[string]lint.RuleConfig{}
 	}
+	if config.EnableAllRules {
+		// Add to the configuration all rules not yet present in it
+		for _, rule := range allRules {
+			ruleName := rule.Name()
+			_, alreadyInConf := config.Rules[ruleName]
+			if alreadyInConf {
+				continue
+			}
+			// Add the rule with an empty conf for
+			config.Rules[ruleName] = lint.RuleConfig{}
+		}
+	}
+
 	severity := config.Severity
 	if severity != "" {
 		for k, v := range config.Rules {
diff --git a/go.mod b/go.mod
index ac49ede..62633f8 100644
--- a/go.mod
+++ b/go.mod
@@ -11,5 +11,5 @@ require (
 	github.com/mitchellh/go-homedir v1.1.0
 	github.com/olekukonko/tablewriter v0.0.5
 	github.com/pkg/errors v0.9.1
-	golang.org/x/tools v0.1.6
+	golang.org/x/tools v0.1.7
 )
diff --git a/go.sum b/go.sum
index 6f29791..3cc14c4 100644
--- a/go.sum
+++ b/go.sum
@@ -96,6 +96,8 @@ golang.org/x/tools v0.1.5 h1:ouewzE6p+/VEB31YYnTbEJdi8pFqKp4P4n85vwo3DHA=
 golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
 golang.org/x/tools v0.1.6 h1:SIasE1FVIQOWz2GEAHFOmoW7xchJcqlucjSULTL0Ag4=
 golang.org/x/tools v0.1.6/go.mod h1:LGqMHiF4EqQNHR1JncWGqT5BVaXmza+X+BDGol+dOxo=
+golang.org/x/tools v0.1.7 h1:6j8CgantCy3yc8JGBqkDLMKWqZ0RDU2g1HVgacojGWQ=
+golang.org/x/tools v0.1.7/go.mod h1:LGqMHiF4EqQNHR1JncWGqT5BVaXmza+X+BDGol+dOxo=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
diff --git a/rule/time-equal.go b/rule/time-equal.go
new file mode 100644
index 0000000..72ecf26
--- /dev/null
+++ b/rule/time-equal.go
@@ -0,0 +1,76 @@
+package rule
+
+import (
+	"fmt"
+	"go/ast"
+	"go/token"
+
+	"github.com/mgechev/revive/lint"
+)
+
+// TimeEqualRule shows where "==" and "!=" used for equality check time.Time
+type TimeEqualRule struct{}
+
+// Apply applies the rule to given file.
+func (*TimeEqualRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
+	var failures []lint.Failure
+
+	onFailure := func(failure lint.Failure) {
+		failures = append(failures, failure)
+	}
+
+	w := &lintTimeEqual{file, onFailure}
+	if w.file.Pkg.TypeCheck() != nil {
+		return nil
+	}
+
+	ast.Walk(w, file.AST)
+	return failures
+}
+
+// Name returns the rule name.
+func (*TimeEqualRule) Name() string {
+	return "time-equal"
+}
+
+type lintTimeEqual struct {
+	file      *lint.File
+	onFailure func(lint.Failure)
+}
+
+func (l *lintTimeEqual) Visit(node ast.Node) ast.Visitor {
+	expr, ok := node.(*ast.BinaryExpr)
+	if !ok {
+		return l
+	}
+
+	switch expr.Op {
+	case token.EQL, token.NEQ:
+	default:
+		return l
+	}
+
+	xtyp := l.file.Pkg.TypeOf(expr.X)
+	ytyp := l.file.Pkg.TypeOf(expr.Y)
+
+	if !isNamedType(xtyp, "time", "Time") || !isNamedType(ytyp, "time", "Time") {
+		return l
+	}
+
+	var failure string
+	switch expr.Op {
+	case token.EQL:
+		failure = fmt.Sprintf("use %s.Equal(%s) instead of %q operator", expr.X, expr.Y, expr.Op)
+	case token.NEQ:
+		failure = fmt.Sprintf("use !%s.Equal(%s) instead of %q operator", expr.X, expr.Y, expr.Op)
+	}
+
+	l.onFailure(lint.Failure{
+		Category:   "time",
+		Confidence: 1,
+		Node:       node,
+		Failure:    failure,
+	})
+
+	return l
+}


Test patch:
diff --git a/config/config_test.go b/config/config_test.go
index d10b0df..3bf8540 100644
--- a/config/config_test.go
+++ b/config/config_test.go
@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/mgechev/revive/lint"
+	"github.com/mgechev/revive/rule"
 )
 
 func TestGetConfig(t *testing.T) {
@@ -46,3 +47,94 @@ func TestGetConfig(t *testing.T) {
 		})
 	}
 }
+
+func TestGetLintingRules(t *testing.T) {
+	tt := map[string]struct {
+		confPath       string
+		wantRulesCount int
+	}{
+		"no rules": {
+			confPath:       "testdata/noRules.toml",
+			wantRulesCount: 0,
+		},
+		"enableAllRules without disabled rules": {
+			confPath:       "testdata/enableAll.toml",
+			wantRulesCount: len(allRules),
+		},
+		"enableAllRules with 2 disabled rules": {
+			confPath:       "testdata/enableAllBut2.toml",
+			wantRulesCount: len(allRules) - 2,
+		},
+		"enable 2 rules": {
+			confPath:       "testdata/enable2.toml",
+			wantRulesCount: 2,
+		},
+	}
+
+	for name, tc := range tt {
+		t.Run(name, func(t *testing.T) {
+			cfg, err := GetConfig(tc.confPath)
+			if err != nil {
+				t.Fatalf("Unexpected error while loading conf: %v", err)
+			}
+			rules, err := GetLintingRules(cfg)
+			switch {
+			case err != nil:
+				t.Fatalf("Unexpected error\n\t%v", err)
+			case len(rules) != tc.wantRulesCount:
+				t.Fatalf("Expected %v enabled linting rules got: %v", tc.wantRulesCount, len(rules))
+			}
+
+		})
+	}
+}
+
+func TestGetGlobalSeverity(t *testing.T) {
+	tt := map[string]struct {
+		confPath               string
+		wantGlobalSeverity     string
+		particularRule         lint.Rule
+		wantParticularSeverity string
+	}{
+		"enable 2 rules with one specific severity": {
+			confPath:               "testdata/enable2OneSpecificSeverity.toml",
+			wantGlobalSeverity:     "warning",
+			particularRule:         &rule.CyclomaticRule{},
+			wantParticularSeverity: "error",
+		},
+		"enableAllRules with one specific severity": {
+			confPath:               "testdata/enableAllOneSpecificSeverity.toml",
+			wantGlobalSeverity:     "error",
+			particularRule:         &rule.DeepExitRule{},
+			wantParticularSeverity: "warning",
+		},
+	}
+
+	for name, tc := range tt {
+		t.Run(name, func(t *testing.T) {
+			cfg, err := GetConfig(tc.confPath)
+			if err != nil {
+				t.Fatalf("Unexpected error while loading conf: %v", err)
+			}
+			rules, err := GetLintingRules(cfg)
+			if err != nil {
+				t.Fatalf("Unexpected error while loading conf: %v", err)
+			}
+			for _, r := range rules {
+				ruleName := r.Name()
+				ruleCfg := cfg.Rules[ruleName]
+				ruleSeverity := string(ruleCfg.Severity)
+				switch ruleName {
+				case tc.particularRule.Name():
+					if tc.wantParticularSeverity != ruleSeverity {
+						t.Fatalf("Expected Severity %v for rule %v, got %v", tc.wantParticularSeverity, ruleName, ruleSeverity)
+					}
+				default:
+					if tc.wantGlobalSeverity != ruleSeverity {
+						t.Fatalf("Expected Severity %v for rule %v, got %v", tc.wantGlobalSeverity, ruleName, ruleSeverity)
+					}
+				}
+			}
+		})
+	}
+}
diff --git a/config/testdata/enable2.toml b/config/testdata/enable2.toml
new file mode 100644
index 0000000..843c6bc
--- /dev/null
+++ b/config/testdata/enable2.toml
@@ -0,0 +1,12 @@
+ignoreGeneratedHeader = false
+severity = "warning"
+confidence = 0.8
+errorCode = 0
+warningCode = 0
+
+enableAllRules = false
+
+[rule.exported]
+    severity="error"
+[rule.cyclomatic]
+
diff --git a/config/testdata/enable2OneSpecificSeverity.toml b/config/testdata/enable2OneSpecificSeverity.toml
new file mode 100644
index 0000000..004dc83
--- /dev/null
+++ b/config/testdata/enable2OneSpecificSeverity.toml
@@ -0,0 +1,10 @@
+ignoreGeneratedHeader = false
+severity = "warning"
+confidence = 0.8
+errorCode = 0
+warningCode = 0
+
+[rule.deep-exit]
+
+[rule.cyclomatic]
+    severity="error"
diff --git a/config/testdata/enableAll.toml b/config/testdata/enableAll.toml
new file mode 100644
index 0000000..8538e3e
--- /dev/null
+++ b/config/testdata/enableAll.toml
@@ -0,0 +1,7 @@
+ignoreGeneratedHeader = false
+severity = "warning"
+confidence = 0.8
+errorCode = 0
+warningCode = 0
+
+enableAllRules = true
diff --git a/config/testdata/enableAllBut2.toml b/config/testdata/enableAllBut2.toml
new file mode 100644
index 0000000..adc8f76
--- /dev/null
+++ b/config/testdata/enableAllBut2.toml
@@ -0,0 +1,12 @@
+ignoreGeneratedHeader = false
+severity = "warning"
+confidence = 0.8
+errorCode = 0
+warningCode = 0
+
+enableAllRules = true
+
+[rule.exported]
+    disabled=true
+[rule.cyclomatic]
+    disabled=true    
diff --git a/config/testdata/enableAllOneSpecificSeverity.toml b/config/testdata/enableAllOneSpecificSeverity.toml
new file mode 100644
index 0000000..87e09f4
--- /dev/null
+++ b/config/testdata/enableAllOneSpecificSeverity.toml
@@ -0,0 +1,10 @@
+ignoreGeneratedHeader = false
+severity = "error"
+confidence = 0.8
+errorCode = 0
+warningCode = 0
+
+enableAllRules = true
+
+[rule.deep-exit]
+    severity="warning"
diff --git a/config/testdata/noRules.toml b/config/testdata/noRules.toml
new file mode 100644
index 0000000..fad1dc7
--- /dev/null
+++ b/config/testdata/noRules.toml
@@ -0,0 +1,5 @@
+ignoreGeneratedHeader = false
+severity = "warning"
+confidence = 0.8
+errorCode = 0
+warningCode = 0
diff --git a/test/time-equal_test.go b/test/time-equal_test.go
new file mode 100644
index 0000000..7e54269
--- /dev/null
+++ b/test/time-equal_test.go
@@ -0,0 +1,12 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/mgechev/revive/rule"
+)
+
+// TestTimeEqual rule.
+func TestTimeEqual(t *testing.T) {
+	testRule(t, "time-equal", &rule.TimeEqualRule{})
+}
diff --git a/testdata/time-equal.go b/testdata/time-equal.go
new file mode 100644
index 0000000..be72c4e
--- /dev/null
+++ b/testdata/time-equal.go
@@ -0,0 +1,14 @@
+package pkg
+
+import "time"
+
+func t() bool {
+	t := time.Now()
+	u := t
+
+	if !t.After(u) {
+		return t == u // MATCH /use t.Equal(u) instead of "==" operator/
+	}
+
+	return t != u // MATCH /use !t.Equal(u) instead of "!=" operator/
+}