From 6f2e1b60a97ddbe492af67808c7794d2ec34265b Mon Sep 17 00:00:00 2001 From: Dave Cheney Date: Mon, 4 Mar 2019 12:37:23 +1100 Subject: [PATCH] all: address some issues identified by gocritic Signed-off-by: Dave Cheney --- cmd/contour/cli.go | 3 +- cmd/contour/contour.go | 2 +- internal/contour/route.go | 10 +++--- internal/dag/builder.go | 4 +-- internal/dag/builder_test.go | 58 +++++++++++++------------------- internal/debug/dot.go | 6 +--- internal/httpsvc/http.go | 2 +- internal/metrics/metrics_test.go | 33 +++++++++--------- 8 files changed, 52 insertions(+), 66 deletions(-) diff --git a/cmd/contour/cli.go b/cmd/contour/cli.go index 4727cb8e3fb..76bd42e1546 100644 --- a/cmd/contour/cli.go +++ b/cmd/contour/cli.go @@ -84,6 +84,7 @@ func watchstream(st stream, typeURL string, resources []string) { check(err) resp, err := st.Recv() check(err) - m.Marshal(os.Stdout, resp) + err = m.Marshal(os.Stdout, resp) + check(err) } } diff --git a/cmd/contour/contour.go b/cmd/contour/contour.go index 1e3be4da284..cf0ed59b537 100644 --- a/cmd/contour/contour.go +++ b/cmd/contour/contour.go @@ -216,7 +216,7 @@ func main() { defer log.Println("stopped") return s.Serve(l) }) - g.Run() + _ = g.Run() default: app.Usage(args) os.Exit(2) diff --git a/internal/contour/route.go b/internal/contour/route.go index 67ddc239d2c..e5a54c5665a 100644 --- a/internal/contour/route.go +++ b/internal/contour/route.go @@ -114,9 +114,8 @@ func (v *routeVisitor) visit(vertex dag.Vertex) { switch vh := vertex.(type) { case *dag.VirtualHost: vhost := envoy.VirtualHost(vh.Name, l.Port) - vh.Visit(func(r dag.Vertex) { - switch r := r.(type) { - case *dag.Route: + vh.Visit(func(v dag.Vertex) { + if r, ok := v.(*dag.Route); ok { var svcs []*dag.HTTPService r.Visit(func(s dag.Vertex) { if s, ok := s.(*dag.HTTPService); ok { @@ -145,9 +144,8 @@ func (v *routeVisitor) visit(vertex dag.Vertex) { v.routes["ingress_http"].VirtualHosts = append(v.routes["ingress_http"].VirtualHosts, vhost) case *dag.SecureVirtualHost: vhost := envoy.VirtualHost(vh.VirtualHost.Name, l.Port) - vh.Visit(func(r dag.Vertex) { - switch r := r.(type) { - case *dag.Route: + vh.Visit(func(v dag.Vertex) { + if r, ok := v.(*dag.Route); ok { var svcs []*dag.HTTPService r.Visit(func(s dag.Vertex) { if s, ok := s.(*dag.HTTPService); ok { diff --git a/internal/dag/builder.go b/internal/dag/builder.go index aaace1b0822..f27c3d00913 100644 --- a/internal/dag/builder.go +++ b/internal/dag/builder.go @@ -808,10 +808,10 @@ func matchesPathPrefix(path, prefix string) bool { return false } if prefix[len(prefix)-1] != '/' { - prefix = prefix + "/" + prefix += "/" } if path[len(path)-1] != '/' { - path = path + "/" + path += "/" } return strings.HasPrefix(path, prefix) } diff --git a/internal/dag/builder_test.go b/internal/dag/builder_test.go index 1c833402546..be629403b83 100644 --- a/internal/dag/builder_test.go +++ b/internal/dag/builder_test.go @@ -2763,17 +2763,15 @@ func TestDAGInsert(t *testing.T) { got := make(map[int]*Listener) dag.Visit(func(v Vertex) { - switch v := v.(type) { - case *Listener: - got[v.Port] = v + if l, ok := v.(*Listener); ok { + got[l.Port] = l } }) want := make(map[int]*Listener) for _, v := range tc.want { - switch v := v.(type) { - case *Listener: - want[v.Port] = v + if l, ok := v.(*Listener); ok { + want[l.Port] = l } } @@ -2925,9 +2923,8 @@ func TestDAGIngressRouteCycle(t *testing.T) { got := make(map[int]*Listener) dag.Visit(func(v Vertex) { - switch v := v.(type) { - case *Listener: - got[v.Port] = v + if l, ok := v.(*Listener); ok { + got[l.Port] = l } }) @@ -2976,9 +2973,8 @@ func TestDAGIngressRouteCycleSelfEdge(t *testing.T) { got := make(map[int]*Listener) dag.Visit(func(v Vertex) { - switch v := v.(type) { - case *Listener: - got[v.Port] = v + if l, ok := v.(*Listener); ok { + got[l.Port] = l } }) @@ -3017,9 +3013,8 @@ func TestDAGIngressRouteDelegatesToNonExistent(t *testing.T) { got := make(map[int]*Listener) dag.Visit(func(v Vertex) { - switch v := v.(type) { - case *Listener: - got[v.Port] = v + if l, ok := v.(*Listener); ok { + got[l.Port] = l } }) @@ -3073,9 +3068,8 @@ func TestDAGIngressRouteDelegatePrefixDoesntMatch(t *testing.T) { got := make(map[int]*Listener) dag.Visit(func(v Vertex) { - switch v := v.(type) { - case *Listener: - got[v.Port] = v + if l, ok := v.(*Listener); ok { + got[l.Port] = l } }) @@ -3186,14 +3180,11 @@ func TestDAGRootNamespaces(t *testing.T) { var count int dag.Visit(func(v Vertex) { - switch v := v.(type) { - case *Listener: - v.Visit(func(v Vertex) { - if _, ok := v.(*VirtualHost); ok { - count++ - } - }) - } + v.Visit(func(v Vertex) { + if _, ok := v.(*VirtualHost); ok { + count++ + } + }) }) if tc.want != count { @@ -3249,9 +3240,8 @@ func TestDAGIngressRouteDelegatePrefixMatchesStringPrefixButNotPathPrefix(t *tes got := make(map[int]*Listener) dag.Visit(func(v Vertex) { - switch v := v.(type) { - case *Listener: - got[v.Port] = v + if l, ok := v.(*Listener); ok { + got[l.Port] = l } }) @@ -3835,17 +3825,15 @@ func TestDAGIngressRouteUniqueFQDNs(t *testing.T) { dag := b.Build() got := make(map[int]*Listener) dag.Visit(func(v Vertex) { - switch v := v.(type) { - case *Listener: - got[v.Port] = v + if l, ok := v.(*Listener); ok { + got[l.Port] = l } }) want := make(map[int]*Listener) for _, v := range tc.want { - switch v := v.(type) { - case *Listener: - want[v.Port] = v + if l, ok := v.(*Listener); ok { + want[l.Port] = l } } diff --git a/internal/debug/dot.go b/internal/debug/dot.go index 95468b931b4..21c20228250 100644 --- a/internal/debug/dot.go +++ b/internal/debug/dot.go @@ -66,11 +66,7 @@ func (c *ctx) writeEdge(parent, child dag.Vertex) { return } c.edges[pair{parent, child}] = true - switch child := child.(type) { - default: - fmt.Fprintf(c.w, `"%p" -> "%p"`+"\n", parent, child) - } - + fmt.Fprintf(c.w, `"%p" -> "%p"`+"\n", parent, child) } func (dw *dotWriter) writeDot(w io.Writer) { diff --git a/internal/httpsvc/http.go b/internal/httpsvc/http.go index 8f31210e16c..ea6db9da855 100644 --- a/internal/httpsvc/http.go +++ b/internal/httpsvc/http.go @@ -60,7 +60,7 @@ func (svc *Service) Start(stop <-chan struct{}) (err error) { ctx := context.Background() ctx, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() - s.Shutdown(ctx) + _ = s.Shutdown(ctx) // ignored, will always be a cancelation error }() svc.WithField("address", s.Addr).Info("started") diff --git a/internal/metrics/metrics_test.go b/internal/metrics/metrics_test.go index ab3a987e101..b0c43ee70d3 100644 --- a/internal/metrics/metrics_test.go +++ b/internal/metrics/metrics_test.go @@ -216,15 +216,16 @@ func TestWriteIngressRouteMetric(t *testing.T) { gotOrphaned := []*io_prometheus_client.Metric{} gotRoot := []*io_prometheus_client.Metric{} for _, mf := range gathering { - if mf.GetName() == tc.total.metric { + switch mf.GetName() { + case tc.total.metric: gotTotal = mf.Metric - } else if mf.GetName() == tc.valid.metric { + case tc.valid.metric: gotValid = mf.Metric - } else if mf.GetName() == tc.invalid.metric { + case tc.invalid.metric: gotInvalid = mf.Metric - } else if mf.GetName() == tc.orphaned.metric { + case tc.orphaned.metric: gotOrphaned = mf.Metric - } else if mf.GetName() == tc.root.metric { + case tc.root.metric: gotRoot = mf.Metric } } @@ -640,15 +641,16 @@ func TestRemoveMetric(t *testing.T) { gotOrphaned := []*io_prometheus_client.Metric{} gotRoot := []*io_prometheus_client.Metric{} for _, mf := range gathering { - if mf.GetName() == total.metric { + switch mf.GetName() { + case total.metric: gotTotal = mf.Metric - } else if mf.GetName() == valid.metric { + case valid.metric: gotValid = mf.Metric - } else if mf.GetName() == invalid.metric { + case invalid.metric: gotInvalid = mf.Metric - } else if mf.GetName() == orphaned.metric { + case orphaned.metric: gotOrphaned = mf.Metric - } else if mf.GetName() == root.metric { + case root.metric: gotRoot = mf.Metric } } @@ -688,15 +690,16 @@ func TestRemoveMetric(t *testing.T) { gotOrphaned = []*io_prometheus_client.Metric{} gotRoot = []*io_prometheus_client.Metric{} for _, mf := range gathering { - if mf.GetName() == total.metric { + switch mf.GetName() { + case total.metric: gotTotal = mf.Metric - } else if mf.GetName() == valid.metric { + case valid.metric: gotValid = mf.Metric - } else if mf.GetName() == invalid.metric { + case invalid.metric: gotInvalid = mf.Metric - } else if mf.GetName() == orphaned.metric { + case orphaned.metric: gotOrphaned = mf.Metric - } else if mf.GetName() == root.metric { + case root.metric: gotRoot = mf.Metric } }