fix(receiver-naming): distinguish types with parameters (#692)

* fix(receiver-naming): distinguish types with parameters

* chore: run tests using supported Go versions matrix

Author: tie

.github/workflows/build-test.yaml renamed to .github/workflows/build.yaml

@@ -1,5 +1,6 @@
-name: Build and Test
+name: Build
 on:
+  workflow_dispatch:
   pull_request:
     types: [opened, edited, synchronize, reopened]
 
@@ -11,10 +12,3 @@ jobs:
       - uses: actions/checkout@v2
       - name: build
         run: make build
-  test:
-    name: Test
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v2
-      - name: test
-        run: make test

.github/workflows/lint.yaml

@@ -1,5 +1,6 @@
 name: Lint
 on:
+  workflow_dispatch:
   pull_request:
     types: [opened, edited, synchronize, reopened]
 

.github/workflows/test.yaml

@@ -0,0 +1,28 @@
+name: Test
+on:
+  workflow_dispatch:
+  pull_request:
+    types: [opened, edited, synchronize, reopened]
+
+jobs:
+  test:
+    name: Test
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        go-version:
+          - 1.16.x
+          - 1.17.x
+          - 1.18.x
+    steps:
+      - name: Checkout code
+        uses: actions/[email protected]
+      - name: Set up Go
+        uses: actions/[email protected]
+        with:
+          go-version: ${{ matrix.go-version }}
+          cache: true
+          cache-dependency-path: '**/go.sum'
+      - name: Run tests
+        run: go test -race ./...

internal/typeparams/typeparams.go

@@ -0,0 +1,29 @@
+// Package typeparams provides utilities for working with Go ASTs with support
+// for type parameters when built with Go 1.18 and higher.
+package typeparams
+
+import (
+	"go/ast"
+)
+
+// Enabled reports whether type parameters are enabled in the current build
+// environment.
+func Enabled() bool {
+	return enabled
+}
+
+// ReceiverType returns the named type of the method receiver, sans "*" and type
+// parameters, or "invalid-type" if fn.Recv is ill formed.
+func ReceiverType(fn *ast.FuncDecl) string {
+	e := fn.Recv.List[0].Type
+	if s, ok := e.(*ast.StarExpr); ok {
+		e = s.X
+	}
+	if enabled {
+		e = unpackIndexExpr(e)
+	}
+	if id, ok := e.(*ast.Ident); ok {
+		return id.Name
+	}
+	return "invalid-type"
+}

internal/typeparams/typeparams_go117.go

@@ -0,0 +1,12 @@
+//go:build !go1.18
+// +build !go1.18
+
+package typeparams
+
+import (
+	"go/ast"
+)
+
+const enabled = false
+
+func unpackIndexExpr(e ast.Expr) ast.Expr { return e }

internal/typeparams/typeparams_go118.go

@@ -0,0 +1,20 @@
+//go:build go1.18
+// +build go1.18
+
+package typeparams
+
+import (
+	"go/ast"
+)
+
+const enabled = true
+
+func unpackIndexExpr(e ast.Expr) ast.Expr {
+	switch e := e.(type) {
+	case *ast.IndexExpr:
+		return e.X
+	case *ast.IndexListExpr:
+		return e.X
+	}
+	return e
+}

lint/package.go

@@ -7,6 +7,8 @@ import (
 	"sync"
 
 	"golang.org/x/tools/go/gcexportdata"
+
+	"github.com/mgechev/revive/internal/typeparams"
 )
 
 // Package represents a package in the project.
@@ -146,7 +148,7 @@ func (w *walker) Visit(n ast.Node) ast.Visitor {
 		return w
 	}
 	// TODO(dsymonds): We could check the signature to be more precise.
-	recv := receiverType(fn)
+	recv := typeparams.ReceiverType(fn)
 	if i, ok := w.nmap[fn.Name.Name]; ok {
 		w.has[recv] |= i
 	}
@@ -174,21 +176,6 @@ func (p *Package) scanSortable() {
 	}
 }
 
-// receiverType returns the named type of the method receiver, sans "*",
-// or "invalid-type" if fn.Recv is ill formed.
-func receiverType(fn *ast.FuncDecl) string {
-	switch e := fn.Recv.List[0].Type.(type) {
-	case *ast.Ident:
-		return e.Name
-	case *ast.StarExpr:
-		if id, ok := e.X.(*ast.Ident); ok {
-			return id.Name
-		}
-	}
-	// The parser accepts much more than just the legal forms.
-	return "invalid-type"
-}
-
 func (p *Package) lint(rules []Rule, config Config, failures chan Failure) {
 	p.scanSortable()
 	var wg sync.WaitGroup

rule/exported.go

@@ -9,6 +9,7 @@ import (
 	"unicode"
 	"unicode/utf8"
 
+	"github.com/mgechev/revive/internal/typeparams"
 	"github.com/mgechev/revive/lint"
 )
 
@@ -116,7 +117,7 @@ func (w *lintExported) lintFuncDoc(fn *ast.FuncDecl) {
 	if fn.Recv != nil && len(fn.Recv.List) > 0 {
 		// method
 		kind = "method"
-		recv := receiverType(fn)
+		recv := typeparams.ReceiverType(fn)
 		if !w.checkPrivateReceivers && !ast.IsExported(recv) {
 			// receiver is unexported
 			return

rule/receiver-naming.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"go/ast"
 
+	"github.com/mgechev/revive/internal/typeparams"
 	"github.com/mgechev/revive/lint"
 )
 
@@ -65,7 +66,7 @@ func (w lintReceiverName) Visit(n ast.Node) ast.Visitor {
 		})
 		return w
 	}
-	recv := receiverType(fn)
+	recv := typeparams.ReceiverType(fn)
 	if prev, ok := w.typeReceiver[recv]; ok && prev != name {
 		w.onFailure(lint.Failure{
 			Node:       n,

rule/unexported-return.go

@@ -5,6 +5,7 @@ import (
 	"go/ast"
 	"go/types"
 
+	"github.com/mgechev/revive/internal/typeparams"
 	"github.com/mgechev/revive/lint"
 )
 
@@ -55,7 +56,7 @@ func (w lintUnexportedReturn) Visit(n ast.Node) ast.Visitor {
 	thing := "func"
 	if fn.Recv != nil && len(fn.Recv.List) > 0 {
 		thing = "method"
-		if !ast.IsExported(receiverType(fn)) {
+		if !ast.IsExported(typeparams.ReceiverType(fn)) {
 			// Don't report exported methods of unexported types,
 			// such as private implementations of sort.Interface.
 			return nil

rule/utils.go

@@ -26,19 +26,6 @@ var commonMethods = map[string]bool{
 	"Unwrap":    true,
 }
 
-func receiverType(fn *ast.FuncDecl) string {
-	switch e := fn.Recv.List[0].Type.(type) {
-	case *ast.Ident:
-		return e.Name
-	case *ast.StarExpr:
-		if id, ok := e.X.(*ast.Ident); ok {
-			return id.Name
-		}
-	}
-	// The parser accepts much more than just the legal forms.
-	return "invalid-type"
-}
-
 var knownNameExceptions = map[string]bool{
 	"LastInsertId": true, // must match database/sql
 	"kWh":          true,

test/receiver-naming_test.go

@@ -0,0 +1,15 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/mgechev/revive/internal/typeparams"
+	"github.com/mgechev/revive/rule"
+)
+
+func TestReceiverNamingTypeParams(t *testing.T) {
+	if !typeparams.Enabled() {
+		t.Skip("type parameters are not enabled in the current build environment")
+	}
+	testRule(t, "receiver-naming-issue-669", &rule.ReceiverNamingRule{})
+}

testdata/receiver-naming-issue-669.go

@@ -0,0 +1,37 @@
+package fixtures
+
+type gen1[T any] struct{}
+
+func (g gen1[T]) f1() {}
+
+func (g gen1[U]) f2() {}
+
+func (n gen1[T]) f3() {} // MATCH /receiver name n should be consistent with previous receiver name g for gen1/
+
+func (n gen1[U]) f4() {} // MATCH /receiver name n should be consistent with previous receiver name g for gen1/
+
+func (n gen1[V]) f5() {} // MATCH /receiver name n should be consistent with previous receiver name g for gen1/
+
+func (n *gen1[T]) f6() {} // MATCH /receiver name n should be consistent with previous receiver name g for gen1/
+
+func (n *gen1[U]) f7() {} // MATCH /receiver name n should be consistent with previous receiver name g for gen1/
+
+func (n *gen1[V]) f8() {} // MATCH /receiver name n should be consistent with previous receiver name g for gen1/
+
+type gen2[T1, T2 any] struct{}
+
+func (g gen2[T1, T2]) f1() {}
+
+func (g gen2[U1, U2]) f2() {}
+
+func (n gen2[T1, T2]) f3() {} // MATCH /receiver name n should be consistent with previous receiver name g for gen2/
+
+func (n gen2[U1, U2]) f4() {} // MATCH /receiver name n should be consistent with previous receiver name g for gen2/
+
+func (n gen2[V1, V2]) f5() {} // MATCH /receiver name n should be consistent with previous receiver name g for gen2/
+
+func (n *gen2[T1, T2]) f6() {} // MATCH /receiver name n should be consistent with previous receiver name g for gen2/
+
+func (n *gen2[U1, U2]) f7() {} // MATCH /receiver name n should be consistent with previous receiver name g for gen2/
+
+func (n *gen2[V1, V2]) f8() {} // MATCH /receiver name n should be consistent with previous receiver name g for gen2/