Repository: mgechev/revive
Pull request #854 (2023-07-29 20:41:32)

feat: add rule for redundant import alias
<!-- ### IMPORTANT ### -->
PR for issue https://github.com/mgechev/revive/issues/853

A new rule `redundant-import-alias` that would warn upon redundant import aliases, i.e., when the imported package name matches the alias names.

damif94: > Hi, thanks for the rule idea and its PR! I've left some comments, please let me know what do you think about them.

I agree with your suggestions, "redundant" seems like more appropriate. I already did the changes.

What's your view on making this rule as default? It seems like a pretty unquestionable linting rule, but on the other hand I got fear of retrocompatibility on upgrading versions and not sure on what's the project policies on upgrades.
chavacava: > > Hi, thanks for the rule idea and its PR! I've left some comments, please let me know what do you think about them.
> 
> I agree with your suggestions, "redundant" seems like more appropriate. I already did the changes.
> 
> What's your view on making this rule as default? It seems like a pretty unquestionable linting rule, but on the other hand I got fear of retrocompatibility on upgrading versions and not sure on what's the project policies on upgrades.

Thanks for the modifications.
About making this rule default: historically, `revive` default rules were those of `golint`. I propose to let the new rule out of the defaults and actvite it through the config. 

Patch:
diff --git a/README.md b/README.md
index e164826..43800ae 100644
--- a/README.md
+++ b/README.md
@@ -501,6 +501,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | [`use-any`](./RULES_DESCRIPTIONS.md#use-any)          |  n/a   |  Proposes to replace `interface{}` with its alias `any` |    no    |  no   |
 | [`datarace`](./RULES_DESCRIPTIONS.md#datarace)          |  n/a   |  Spots potential dataraces |    no    |  no   |
 | [`comment-spacings`](./RULES_DESCRIPTIONS.md#comment-spacings)          |  []string   |  Warns on malformed comments |    no    |  no   |
+| [`redundant-import-alias`](./RULES_DESCRIPTIONS.md#redundant-import-alias)          |  n/a   |  Warns on import aliases matching the imported package name |    no    |  no   |
 
 
 ## Configurable rules
diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md
index e6fa3d0..4b48b52 100644
--- a/RULES_DESCRIPTIONS.md
+++ b/RULES_DESCRIPTIONS.md
@@ -75,6 +75,7 @@ List of all available rules.
   - [use-any](#use-any)
   - [useless-break](#useless-break)
   - [waitgroup-by-value](#waitgroup-by-value)
+  - [redundant-import-alias](#redundant-import-alias)
 
 ## add-constant
 
@@ -760,3 +761,9 @@ _Description_: Function parameters that are passed by value, are in fact a copy
 This rule warns when a `sync.WaitGroup` expected as a by-value parameter in a function or method.
 
 _Configuration_: N/A
+
+## redundant-import-alias
+
+_Description_: This rule warns on redundant import aliases. This happens when the alias used on the import statement matches the imported package name.
+
+_Configuration_: N/A
\ No newline at end of file
diff --git a/config/config.go b/config/config.go
index f6ee2a4..225a570 100644
--- a/config/config.go
+++ b/config/config.go
@@ -87,6 +87,7 @@ var allRules = append([]lint.Rule{
 	&rule.DataRaceRule{},
 	&rule.CommentSpacingsRule{},
 	&rule.IfReturnRule{},
+	&rule.RedundantImportAlias{},
 }, defaultRules...)
 
 var allFormatters = []lint.Formatter{
diff --git a/rule/redundant-import-alias.go b/rule/redundant-import-alias.go
new file mode 100644
index 0000000..fa5281f
--- /dev/null
+++ b/rule/redundant-import-alias.go
@@ -0,0 +1,52 @@
+package rule
+
+import (
+	"fmt"
+	"go/ast"
+	"strings"
+
+	"github.com/mgechev/revive/lint"
+)
+
+// RedundantImportAlias lints given else constructs.
+type RedundantImportAlias struct{}
+
+// Apply applies the rule to given file.
+func (*RedundantImportAlias) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
+	var failures []lint.Failure
+
+	for _, imp := range file.AST.Imports {
+		if imp.Name == nil {
+			continue
+		}
+
+		if getImportPackageName(imp) == imp.Name.Name {
+			failures = append(failures, lint.Failure{
+				Confidence: 1,
+				Failure:    fmt.Sprintf("Import alias \"%s\" is redundant", imp.Name.Name),
+				Node:       imp,
+				Category:   "imports",
+			})
+		}
+	}
+
+	return failures
+}
+
+// Name returns the rule name.
+func (*RedundantImportAlias) Name() string {
+	return "redundant-import-alias"
+}
+
+func getImportPackageName(imp *ast.ImportSpec) string {
+	const pathSep = "/"
+	const strDelim = `"`
+
+	path := imp.Path.Value
+	i := strings.LastIndex(path, pathSep)
+	if i == -1 {
+		return strings.Trim(path, strDelim)
+	}
+
+	return strings.Trim(path[i+1:], strDelim)
+}


Test patch:
diff --git a/test/redundant-import-alias_test.go b/test/redundant-import-alias_test.go
new file mode 100644
index 0000000..e7bff5f
--- /dev/null
+++ b/test/redundant-import-alias_test.go
@@ -0,0 +1,11 @@
+package test
+
+import (
+	"github.com/mgechev/revive/rule"
+	"testing"
+)
+
+// TestRedundantImportAlias rule.
+func TestRedundantImportAlias(t *testing.T) {
+	testRule(t, "redundant-import-alias", &rule.RedundantImportAlias{})
+}
diff --git a/testdata/redundant-import-alias.go b/testdata/redundant-import-alias.go
new file mode 100644
index 0000000..555bca2
--- /dev/null
+++ b/testdata/redundant-import-alias.go
@@ -0,0 +1,12 @@
+package fixtures
+
+import(
+	"crypto/md5"
+		"strings"
+		_ "crypto/md5"
+		str "strings"
+		strings "strings"  // MATCH /Import alias "strings" is redundant/
+		crypto "crypto/md5"
+		md5 "crypto/md5"  // MATCH /Import alias "md5" is redundant/
+)
+