Repository: tomarrell/lbadd
Pull request #122 (2020-05-07 17:54:50)

Fix ctxfunc
`ctxfunc` analyzes function arguments, if they are a `context.Context`, they had to fulfil the following conditions.
* be the first parameter
* variable must not be anonymous
* only one context per function
* context argument must have the name `ctx`
* variable must be named
However, in an interface like this
```
type Foo interface {
    Bar(context.Context)
}
```
the context is not named, and the ctxFunc analyzer reported a diagnostic.
This has been fixed now, and unnamed context arguments are allowed.
This decision was made on the following assumptions.
* a func like `func x(context.Context)` ignores the context, and this must be catched by a reviewer, as the analysis proved quite complicated and inefficient. if we still need it, we will invest the time for that
* a func like `func x(_ context.Context, s string)` is not allowed by the analyzer
* something like `func x(ctx context.Context) { _ = ctx }` must be catched by the reviewer, as it is hard to detect with an analyzer



Patch:
diff --git a/internal/network/server.go b/internal/network/server.go
index cd907e2..0643412 100644
--- a/internal/network/server.go
+++ b/internal/network/server.go
@@ -19,9 +19,12 @@ type ConnHandler func(Conn)
 type Server interface {
 	io.Closer
 
-	// Open opens the server on the given address. To choose the server a random
-	// free port for you, specify a port ":0".
+	// Open opens the server on the given address. To make the server choose a
+	// random free port for you, specify a port ":0".
 	Open(string) error
+	// Listening can be used to get a signal when the server has allocated a
+	// port and is now actively listening for incoming connections.
+	Listening() <-chan struct{}
 	// Addr returns the address that this server is listening to.
 	Addr() net.Addr
 
diff --git a/internal/network/tcp_server.go b/internal/network/tcp_server.go
index 2d8c359..7faec1f 100644
--- a/internal/network/tcp_server.go
+++ b/internal/network/tcp_server.go
@@ -14,8 +14,9 @@ var _ Server = (*tcpServer)(nil)
 type tcpServer struct {
 	log zerolog.Logger
 
-	open bool
-	lis  net.Listener
+	open      bool
+	listening chan struct{}
+	lis       net.Listener
 
 	onConnect ConnHandler
 }
@@ -24,7 +25,8 @@ type tcpServer struct {
 // logger.
 func NewTCPServer(log zerolog.Logger) Server {
 	return &tcpServer{
-		log: log,
+		log:       log,
+		listening: make(chan struct{}),
 	}
 }
 
@@ -49,6 +51,10 @@ func (s *tcpServer) Open(addr string) error {
 	return nil
 }
 
+func (s *tcpServer) Listening() <-chan struct{} {
+	return s.listening
+}
+
 func (s *tcpServer) Addr() net.Addr {
 	if s.lis == nil {
 		return nil
@@ -71,6 +77,7 @@ func (s *tcpServer) Close() error {
 }
 
 func (s *tcpServer) handleIncomingConnections() {
+	close(s.listening)
 	for {
 		conn, err := s.lis.Accept()
 		if err != nil {
diff --git a/internal/tool/analysis/ctxfunc/ctxfunc.go b/internal/tool/analysis/ctxfunc/ctxfunc.go
index 2167f46..e382753 100644
--- a/internal/tool/analysis/ctxfunc/ctxfunc.go
+++ b/internal/tool/analysis/ctxfunc/ctxfunc.go
@@ -44,8 +44,7 @@ func checkFunction(fn *ast.FuncType, pass *analysis.Pass) {
 			if namedType.String() == "context.Context" { // we found a context.Context argument
 				n := len(arg.Names)
 				if n < 1 {
-					pass.Reportf(arg.Pos(), "unused context.Context argument")
-					return
+					continue
 				}
 				if n > 1 || foundContext {
 					pass.Reportf(arg.Pos(), "more than one context.Context argument")
@@ -61,7 +60,10 @@ func checkFunction(fn *ast.FuncType, pass *analysis.Pass) {
 
 				// there is a single context.Context argument in the first
 				// position, now check if it's named 'ctx'
-				if arg.Names[0].String() != "ctx" {
+				if arg.Names[0].String() == "_" {
+					pass.Reportf(arg.Pos(), "unused context.Context argument")
+					return
+				} else if arg.Names[0].String() != "ctx" {
 					pass.Reportf(arg.Names[0].Pos(), "context.Context argument should be named 'ctx'")
 					return
 				}


Test patch:
diff --git a/internal/network/example_test.go b/internal/network/example_test.go
index cc00069..c62bbae 100644
--- a/internal/network/example_test.go
+++ b/internal/network/example_test.go
@@ -3,7 +3,6 @@ package network_test
 import (
 	"fmt"
 	"log"
-	"time"
 
 	"github.com/rs/zerolog"
 	"github.com/tomarrell/lbadd/internal/network"
@@ -22,7 +21,8 @@ func ExampleServer() {
 		}
 	}()
 
-	time.Sleep(10 * time.Millisecond)
+	<-srv.Listening() // wait for the server to come up
+
 	client, _ := network.DialTCP(":59513")
 	defer func() {
 		_ = client.Close()
diff --git a/internal/tool/analysis/ctxfunc/testdata/test1.go b/internal/tool/analysis/ctxfunc/testdata/test1.go
index 354a7b3..00cdd9d 100644
--- a/internal/tool/analysis/ctxfunc/testdata/test1.go
+++ b/internal/tool/analysis/ctxfunc/testdata/test1.go
@@ -20,16 +20,16 @@ func main() {
 // functions without body
 
 func testFn0(ctx context.Context, s string, i int)                // valid
-func testFn1(context.Context)                                     // want `unused context.Context argument`
-func testFn2(ctx, ctx2 context.Context)                           // want `more than one context.Context argument`
-func testFn3(ctx context.Context, ctx2 context.Context)           // want `more than one context.Context argument`
-func testFn4(ctx1 context.Context, ctx2 context.Context)          // want `context.Context argument should be named 'ctx'`
-func testFn5(ctx1 context.Context)                                // want `context.Context argument should be named 'ctx'`
-func testFn6(s string, ctx context.Context)                       // want `context.Context should be the first parameter of a function`
-func testFn7(s string, ctx1 context.Context)                      // want `context.Context should be the first parameter of a function`
-func testFn8(ctx context.Context, s string, ctx2 context.Context) // want `more than one context.Context argument`
+func testFn1(ctx, ctx2 context.Context)                           // want `more than one context.Context argument`
+func testFn2(ctx context.Context, ctx2 context.Context)           // want `more than one context.Context argument`
+func testFn3(ctx1 context.Context, ctx2 context.Context)          // want `context.Context argument should be named 'ctx'`
+func testFn4(ctx1 context.Context)                                // want `context.Context argument should be named 'ctx'`
+func testFn5(s string, ctx context.Context)                       // want `context.Context should be the first parameter of a function`
+func testFn6(s string, ctx1 context.Context)                      // want `context.Context should be the first parameter of a function`
+func testFn7(ctx context.Context, s string, ctx2 context.Context) // want `more than one context.Context argument`
+func testFn8(_ context.Context, s string)                         // want `unused context.Context argument`
 
 // functions with body
 
 func testFnB0(ctx context.Context, s string, i int) {} // valid
-func testFnB1(context.Context)                      {} // want `unused context.Context argument`
+func testFnB1(_ context.Context)                    {} // want `unused context.Context argument`
diff --git a/internal/tool/analysis/ctxfunc/testdata/test2.go b/internal/tool/analysis/ctxfunc/testdata/test2.go
new file mode 100644
index 0000000..70d6d39
--- /dev/null
+++ b/internal/tool/analysis/ctxfunc/testdata/test2.go
@@ -0,0 +1,8 @@
+package main
+
+import "context"
+
+type Foo interface {
+	MyFunc1(context.Context)   // valid
+	MyFunc2(_ context.Context) // want `unused context.Context argument`
+}