Verify registered listeners when starting the app. (ServiceWeaver#561)

This ensures that the application listeners match the information
stored in the read-only section of the application binary, and used
by various deployers. If there is a mismatch, the user will be prompted
to run `weaver generate`.

This fixes one of the common issues that came up in workshops, where
people forgot to re-run `weaver generate` after declaring the listener.

Author: spetrovic77

dev/build_and_test

@@ -54,7 +54,7 @@ function cmd_vet() {
 
 function cmd_lint() {
   if ! exists staticcheck; then
-    printf "staticcheck not found; install via\ngo install honnef.co/go/tools/cmd/staticcheck@0.4.3\n" >&2
+    printf "staticcheck not found; install via\ngo install honnef.co/go/tools/cmd/staticcheck@v0.4.3\n" >&2
     exit 1
   fi
 

fill.go

@@ -101,7 +101,7 @@ func fillListeners(impl any, get func(name string) (net.Listener, string, error)
 		// The listener's name is the field name, unless a tag is present.
 		name := t.Name
 		if tag, ok := t.Tag.Lookup("weaver"); ok {
-			if !isValidListenerName(tag) {
+			if !isValidListenerName(name) {
 				return fmt.Errorf("FillListeners: listener tag %s is not a valid Go identifier", tag)
 			}
 			name = tag

godeps.txt

@@ -10,6 +10,7 @@ github.com/ServiceWeaver/weaver
     github.com/ServiceWeaver/weaver/runtime/codegen
     go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp
     go.opentelemetry.io/otel/trace
+    golang.org/x/exp/slices
     log/slog
     math/rand
     net

internal/tool/generate/generator.go

@@ -510,6 +510,16 @@ func extractComponent(opt Options, pkg *packages.Package, file *ast.File, tset *
 		return nil, err
 	}
 
+	// Check that listener names are unique.
+	seenLis := map[string]struct{}{}
+	for _, lis := range listeners {
+		if _, ok := seenLis[lis]; ok {
+			return nil, errorf(pkg.Fset, spec.Pos(),
+				"component implementation %s declares multiple listeners with name %s. Please disambiguate.", formatType(pkg, impl), lis)
+		}
+		seenLis[lis] = struct{}{}
+	}
+
 	// Warn the user if the component has a mistyped Init method. Init methods
 	// are supposed to have type "func(context.Context) error", but it's easy
 	// to forget to add a context.Context argument or error return. Without

internal/tool/generate/testdata/errors/listeners_name_clash.go

@@ -0,0 +1,30 @@
+// Copyright 2022 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// ERROR: multiple listeners with name bar
+
+// Multiple listeners with the same name.
+package foo
+
+import (
+	"github.com/ServiceWeaver/weaver"
+)
+
+type foo interface{}
+
+type impl struct {
+	weaver.Implements[foo]
+	bar weaver.Listener
+	_   weaver.Listener `weaver:"bar"`
+}

validate.go

@@ -22,6 +22,7 @@ import (
 
 	"github.com/ServiceWeaver/weaver/internal/reflection"
 	"github.com/ServiceWeaver/weaver/runtime/codegen"
+	"golang.org/x/exp/slices"
 )
 
 // validateRegistrations validates the provided registrations, returning an
@@ -48,16 +49,25 @@ func validateRegistrations(regs []*codegen.Registration) error {
 				if _, ok := intfs[v.Type]; !ok {
 					// T is not a registered component interface.
 					err := fmt.Errorf(
-						"component implementation struct %v has field %v, but component %v was not registered; maybe you forgot to run 'weaver generate'",
+						"component implementation struct %v has component reference field %v, but component %v was not registered; maybe you forgot to run 'weaver generate'",
 						reg.Impl, f.Type, v.Type,
 					)
 					errs = append(errs, err)
 				}
 
 			case f.Type == reflection.Type[Listener]():
 				// f is a weaver.Listener.
-				if tag, ok := f.Tag.Lookup("weaver"); ok && !isValidListenerName(tag) {
-					err := fmt.Errorf("component implementation struct %v has invalid listener tag %q", reg.Impl, tag)
+				name := f.Name
+				if tag, ok := f.Tag.Lookup("weaver"); ok {
+					if !isValidListenerName(tag) {
+						err := fmt.Errorf("component implementation struct %v has invalid listener tag %q", reg.Impl, tag)
+						errs = append(errs, err)
+						continue
+					}
+					name = tag
+				}
+				if !slices.Contains(reg.Listeners, name) {
+					err := fmt.Errorf("component implementation struct %v has a listener field %v, but listener %v hasn't been registered; maybe you forgot to run 'weaver generate'", reg.Impl, name, name)
 					errs = append(errs, err)
 				}
 			}

validate_test.go

@@ -34,18 +34,21 @@ func TestValidateNoRegistrations(t *testing.T) {
 // TestValidateValidRegistrations tests that validateRegistrations succeeds on
 // a set of valid registrations.
 func TestValidateValidRegistrations(t *testing.T) {
-	type foo struct{}
-	type bar struct{}
+	type foo interface{}
+	type bar interface{}
 	type fooImpl struct {
 		Ref[bar]
-		Listener `weaver:"foo"`
+		Listener `weaver:"lis1"`
+		_        Listener `weaver:"lis2"`
+		lis3     Listener //lint:ignore U1000 Present for code generation.
 	}
 	type barImpl struct{ Ref[foo] }
 	regs := []*codegen.Registration{
 		{
-			Name:  "foo",
-			Iface: reflection.Type[foo](),
-			Impl:  reflection.Type[fooImpl](),
+			Name:      "foo",
+			Iface:     reflection.Type[foo](),
+			Impl:      reflection.Type[fooImpl](),
+			Listeners: []string{"lis1", "lis2", "lis3"},
 		},
 		{
 			Name:  "bar",
@@ -61,7 +64,7 @@ func TestValidateValidRegistrations(t *testing.T) {
 // TestValidateUnregisteredRef tests that validateRegistrations fails when a
 // component has a weaver.Ref on an unregistered component.
 func TestValidateUnregisteredRef(t *testing.T) {
-	type foo struct{}
+	type foo interface{}
 	type fooImpl struct{ Ref[io.Reader] }
 	regs := []*codegen.Registration{
 		{
@@ -83,7 +86,7 @@ func TestValidateUnregisteredRef(t *testing.T) {
 // TestValidateInvalidListenerNames tests that validateRegistrations fails on
 // invalid listener names.
 func TestValidateInvalidListenerNames(t *testing.T) {
-	type foo struct{}
+	type foo interface{}
 	type fooImpl struct {
 		_ Listener `weaver:""`             // empty name
 		_ Listener `weaver:" "`            // whitespace name
@@ -114,3 +117,35 @@ func TestValidateInvalidListenerNames(t *testing.T) {
 		}
 	}
 }
+
+// TestValidateUnregisteredListeners tests that validateRegistrations fails on
+// listener names that haven't been registered.
+func TestValidateUnregisteredListener(t *testing.T) {
+	type foo interface{}
+	type fooImpl struct {
+		foo Listener //lint:ignore U1000 Present for code generation.
+		bar Listener //lint:ignore U1000 Present for code generation.
+		baz Listener //lint:ignore U1000 Present for code generation.
+	}
+
+	regs := []*codegen.Registration{
+		{
+			Name:      "foo",
+			Iface:     reflection.Type[foo](),
+			Impl:      reflection.Type[fooImpl](),
+			Listeners: []string{"foo"},
+		},
+	}
+	err := validateRegistrations(regs)
+	if err == nil {
+		t.Fatal("unexpected validateRegistrations success")
+	}
+	for _, want := range []string{
+		`listener bar hasn't been registered`,
+		`listener baz hasn't been registered`,
+	} {
+		if !strings.Contains(err.Error(), want) {
+			t.Errorf("validateRegistrations: got %q, want %q", err, want)
+		}
+	}
+}