ronantakizawa/codereview-bench huggingface dataset:
Repository: traefik/traefik
File: pkg/provider/kubernetes/gateway/kubernetes_test.go (Go)

Code before:
		{
			desc: "Group is nil",
			gateway: &v1alpha2.Gateway{
				ObjectMeta: metav1.ObjectMeta{
					Name:      "gateway",
					Namespace: "default",
				},
			},
			listener: v1alpha2.Listener{
				Name: "foo",
			},
			routeNamespace: "default",
			routeSpec: v1alpha2.CommonRouteSpec{
				ParentRefs: []v1alpha2.ParentRef{
					{
						SectionName: sectionNamePtr("bar"),
						Name:        "gateway",
						Namespace:   namespacePtr("default"),
						Kind:        kindPtr("Gateway"),
					},
				},
			},
			expectedAttach: false,
		},
		{
			desc: "SectionName does not match a listener desc",
			gateway: &v1alpha2.Gateway{
				ObjectMeta: metav1.ObjectMeta{
					Name:      "gateway",
					Namespace: "default",
				},
			},
			listener: v1alpha2.Listener{
				Name: "foo",
			},
			routeNamespace: "default",
			routeSpec: v1alpha2.CommonRouteSpec{
				ParentRefs: []v1alpha2.ParentRef{
					{
						SectionName: sectionNamePtr("bar"),
						Name:        "gateway",
						Namespace:   namespacePtr("default"),
						Group:       groupPtr(v1alpha2.GroupName),
						Kind:        kindPtr("Gateway"),
					},
				},
			},
			expectedAttach: false,
		},
		{
			desc: "Namespace does not match the Gateway namespace",

Review comment (suggestion):
If #9531 is merged, it should be the following:

```suggestion
			expectedRule: "(HostSNI(`bar.foo`) || HostSNI(`foo.foo`)) || HostSNIRegexp(`^[a-zA-Z0-9-]+\.foo$`)",
```

Code after:
			expectedRule: "HostSNIRegexp(`^[a-zA-Z0-9-]+\\.foo$`)",
		},
		{
			desc:        "Multiple malformed wildcard",
			hostnames:   []v1alpha2.Hostname{"*.foo.*"},
			expectError: true,
		},
		{
			desc:         "Some empty hostnames",
			hostnames:    []v1alpha2.Hostname{"foo", "", "bar"},
			expectedRule: "HostSNI(`foo`) || HostSNI(`bar`)",
		},
		{
			desc:         "Valid hostname",
			hostnames:    []v1alpha2.Hostname{"foo"},
			expectedRule: "HostSNI(`foo`)",
		},
		{
			desc:         "Multiple valid hostnames",
			hostnames:    []v1alpha2.Hostname{"foo", "bar"},
			expectedRule: "HostSNI(`foo`) || HostSNI(`bar`)",
		},
		{
			desc:         "Multiple valid hostnames with wildcard",
			hostnames:    []v1alpha2.Hostname{"bar.foo", "foo.foo", "*.foo"},
			expectedRule: "HostSNI(`bar.foo`) || HostSNI(`foo.foo`) || HostSNIRegexp(`^[a-zA-Z0-9-]+\\.foo$`)",
		},
		{
			desc:         "Multiple overlapping hostnames",
			hostnames:    []v1alpha2.Hostname{"foo", "bar", "foo", "baz"},
			expectedRule: "HostSNI(`foo`) || HostSNI(`bar`) || HostSNI(`baz`)",
		},
	}

	for _, test := range testCases {
		test := test
		t.Run(test.desc, func(t *testing.T) {
			t.Parallel()

			rule, err := hostSNIRule(test.hostnames)
			if test.expectError {
				assert.Error(t, err)
				return
			}

			require.NoError(t, err)
			assert.Equal(t, test.expectedRule, rule)
		})
	}
}
Repository: traefik/traefik
File: pkg/server/configurationwatcher_test.go (Go)

Code before:
							"foo": {},
						},
					},
				},
			},
			{
				ProviderName: "mock",
				Configuration: &dynamic.Configuration{
					TCP: &dynamic.TCPConfiguration{
						Routers: map[string]*dynamic.TCPRouter{
							"foo": {},
						},
					},
				},
			},
		},
	}

	watcher := NewConfigurationWatcher(routinesPool, pvd, []string{}, "")

	publishedConfigCount := 0
	watcher.AddListener(func(configuration dynamic.Configuration) {
		publishedConfigCount++

		// Modify the provided configuration.
		// This should not modify the configuration stored in the configuration watcher and cause a new publishing.
		configuration.TCP.Routers["foo@mock"].Rule = "bar"
	})

	watcher.Start()

	t.Cleanup(watcher.Stop)
	t.Cleanup(routinesPool.Stop)

	// give some time so that the configuration can be processed.
	time.Sleep(100 * time.Millisecond)

	assert.Equal(t, 1, publishedConfigCount)
}


Review comment (suggestion):
```suggestion
		// This should not modify the configuration stored in the configuration watcher and therefore there will be no new publishing.
```

Code after:
							"foo": {},
						},
					},
				},
			},
			{
				ProviderName: "mock",
				Configuration: &dynamic.Configuration{
					TCP: &dynamic.TCPConfiguration{
						Routers: map[string]*dynamic.TCPRouter{
							"foo": {},
						},
					},
				},
			},
		},
	}

	watcher := NewConfigurationWatcher(routinesPool, pvd, []string{}, "")

	publishedConfigCount := 0
	watcher.AddListener(func(configuration dynamic.Configuration) {
		publishedConfigCount++

		// Modify the provided configuration.
		// This should not modify the configuration stored in the configuration watcher and therefore there will be no new publishing.
		configuration.TCP.Routers["foo@mock"].Rule = "bar"
	})

	watcher.Start()

	t.Cleanup(watcher.Stop)
	t.Cleanup(routinesPool.Stop)

	// give some time so that the configuration can be processed.
	time.Sleep(100 * time.Millisecond)

	assert.Equal(t, 1, publishedConfigCount)
}
Repository: traefik/traefik
File: pkg/middlewares/redirect/redirect_scheme.go (Go)

Code before:
package redirect

import (
	"context"
	"errors"
	"net"
	"net/http"
	"strings"

	"github.com/traefik/traefik/v2/pkg/config/dynamic"
	"github.com/traefik/traefik/v2/pkg/log"
	"github.com/traefik/traefik/v2/pkg/middlewares"
)

const (
	typeSchemeName  = "RedirectScheme"
	uriPattern      = `^(https?:\/\/)(\[[\w:.]+\]|[\w\._-]+)?(:\d+)?(.*)$`
	xForwardedProto = "X-Forwarded-Proto"
)

type redirectScheme struct {
	http.Handler

	name string
}

// NewRedirectScheme creates a new RedirectScheme middleware.
func NewRedirectScheme(ctx context.Context, next http.Handler, conf dynamic.RedirectScheme, name string) (http.Handler, error) {
	logger := log.FromContext(middlewares.GetLoggerCtx(ctx, name, typeSchemeName))
	logger.Debug("Creating middleware")
	logger.Debugf("Setting up redirection to %s %s", conf.Scheme, conf.Port)

	if len(conf.Scheme) == 0 {
		return nil, errors.New("you must provide a target scheme")
	}

	port := ""
	if len(conf.Port) > 0 && !(conf.Scheme == schemeHTTP && conf.Port == "80" || conf.Scheme == schemeHTTPS && conf.Port == "443") {
		port = ":" + conf.Port
	}

	rs := &redirectScheme{name: name}

Review comment (suggestion):
```suggestion
	uriPattern      = `^((?:wss?|https?):\/\/)(\[[\w:.]+\]|[\w\._-]+)?(:\d+)?(.*)$`
```

We should probably also support `ws` and `wss` schemes.

Code after:
package redirect

import (
	"context"
	"errors"
	"net"
	"net/http"
	"strings"

	"github.com/traefik/traefik/v2/pkg/config/dynamic"
	"github.com/traefik/traefik/v2/pkg/log"
	"github.com/traefik/traefik/v2/pkg/middlewares"
)

const (
	typeSchemeName  = "RedirectScheme"
	uriPattern      = `^(https?:\/\/)?(\[[\w:.]+\]|[\w\._-]+)?(:\d+)?(.*)$`
	xForwardedProto = "X-Forwarded-Proto"
)

type redirectScheme struct {
	http.Handler

	name string
}

// NewRedirectScheme creates a new RedirectScheme middleware.
func NewRedirectScheme(ctx context.Context, next http.Handler, conf dynamic.RedirectScheme, name string) (http.Handler, error) {
	logger := log.FromContext(middlewares.GetLoggerCtx(ctx, name, typeSchemeName))
	logger.Debug("Creating middleware")
	logger.Debugf("Setting up redirection to %s %s", conf.Scheme, conf.Port)

	if len(conf.Scheme) == 0 {
		return nil, errors.New("you must provide a target scheme")
	}

	port := ""
	if len(conf.Port) > 0 && !(conf.Scheme == schemeHTTP && conf.Port == "80" || conf.Scheme == schemeHTTPS && conf.Port == "443") {
		port = ":" + conf.Port
	}

	rs := &redirectScheme{name: name}
Repository: traefik/traefik
File: pkg/muxer/tcp/mux.go (Go)

Code before:
	if len(protos) == 0 {
		return errors.New("empty value for \"ALPN\" matcher is not allowed")
	}

	for _, proto := range protos {
		if proto == tlsalpn01.ACMETLS1Protocol {
			return fmt.Errorf("invalid protocol value for \"ALPN\" matcher, %q is not allowed", proto)
		}
	}

	tree.matcher = func(meta ConnData) bool {
		for _, proto := range meta.alpnProtos {
			for _, filter := range protos {
				if proto == filter {
					return true
				}
			}
		}

		return false
	}

	return nil
}

var almostFQDN = regexp.MustCompile(`^[[:alnum:]\.\-\:]+$`)

// hostSNI checks if the SNI Host of the connection match the matcher host.
func hostSNI(tree *matchersTree, hosts ...string) error {
	if len(hosts) == 0 {
		return errors.New("empty value for \"HostSNI\" matcher is not allowed")
	}

	for i, host := range hosts {
		// Special case to allow global wildcard
		if host == "*" {
			continue
		}

		if !almostFQDN.MatchString(host) {
			return fmt.Errorf("invalid value for \"HostSNI\" matcher, %q is not a valid hostname", host)
		}

		hosts[i] = strings.ToLower(host)
	}

	tree.matcher = func(meta ConnData) bool {
		// Since a HostSNI(`*`) rule has been provided as catchAll for non-TLS TCP,
		// it allows matching with an empty serverName.
		// Which is why we make sure to take that case into account before
		// checking meta.serverName.

Review comment (suggestion):
```suggestion
var almostFQDN = regexp.MustCompile(`^[[:alnum:]\.\:-]+$`)
```

Code after:
	if len(protos) == 0 {
		return errors.New("empty value for \"ALPN\" matcher is not allowed")
	}

	for _, proto := range protos {
		if proto == tlsalpn01.ACMETLS1Protocol {
			return fmt.Errorf("invalid protocol value for \"ALPN\" matcher, %q is not allowed", proto)
		}
	}

	tree.matcher = func(meta ConnData) bool {
		for _, proto := range meta.alpnProtos {
			for _, filter := range protos {
				if proto == filter {
					return true
				}
			}
		}

		return false
	}

	return nil
}

var hostOrIP = regexp.MustCompile(`^[[:alnum:]\.\-\:]+$`)

// hostSNI checks if the SNI Host of the connection match the matcher host.
func hostSNI(tree *matchersTree, hosts ...string) error {
	if len(hosts) == 0 {
		return errors.New("empty value for \"HostSNI\" matcher is not allowed")
	}

	for i, host := range hosts {
		// Special case to allow global wildcard
		if host == "*" {
			continue
		}

		if !hostOrIP.MatchString(host) {
			return fmt.Errorf("invalid value for \"HostSNI\" matcher, %q is not a valid hostname or IP", host)
		}

		hosts[i] = strings.ToLower(host)
	}

	tree.matcher = func(meta ConnData) bool {
		// Since a HostSNI(`*`) rule has been provided as catchAll for non-TLS TCP,
		// it allows matching with an empty serverName.
		// Which is why we make sure to take that case into account before
		// checking meta.serverName.
Repository: traefik/traefik
File: pkg/udp/conn.go (Go)

Code before:
	mu    sync.RWMutex
	conns map[string]*Conn
	// accepting signifies whether the listener is still accepting new sessions.
	// It also serves as a sentinel for Shutdown to be idempotent.
	accepting bool

	acceptCh chan *Conn // no need for a Once, already indirectly guarded by accepting.

	// timeout defines how long to wait on an idle session,
	// before releasing its related resources.
	timeout time.Duration
}

// Listen creates a new listener.
func Listen(listenConfig net.ListenConfig, network, address string, timeout time.Duration) (*Listener, error) {
	if timeout <= 0 {
		return nil, errors.New("timeout should be greater than zero")
	}

	packetConn, err := listenConfig.ListenPacket(context.Background(), network, address)
	if err != nil {
		return nil, err
	}

	l := &Listener{
		pConn:     packetConn.(*net.UDPConn),
		acceptCh:  make(chan *Conn),
		conns:     make(map[string]*Conn),
		accepting: true,
		timeout:   timeout,
	}

	go l.readLoop()

	return l, nil
}

// Accept waits for and returns the next connection to the listener.
func (l *Listener) Accept() (*Conn, error) {
	c := <-l.acceptCh
	if c == nil {
		// l.acceptCh got closed
		return nil, errClosedListener
	}
	return c, nil
}

// Addr returns the listener's network address.
func (l *Listener) Addr() net.Addr {
	return l.pConn.LocalAddr()
}

Review comment (suggestion):
Here I would prefer to do a safe cast something like and return an error if something is not ok, wdyt?

```go
pConn, ok := packetConn.(*net.UDPConn)
if !ok {
  return errors.New("packet conn is not an UDPConn")
}
```



Code after:

	mu    sync.RWMutex
	conns map[string]*Conn
	// accepting signifies whether the listener is still accepting new sessions.
	// It also serves as a sentinel for Shutdown to be idempotent.
	accepting bool

	acceptCh chan *Conn // no need for a Once, already indirectly guarded by accepting.

	// timeout defines how long to wait on an idle session,
	// before releasing its related resources.
	timeout time.Duration
}

// Listen creates a new listener.
func Listen(listenConfig net.ListenConfig, network, address string, timeout time.Duration) (*Listener, error) {
	if timeout <= 0 {
		return nil, errors.New("timeout should be greater than zero")
	}

	packetConn, err := listenConfig.ListenPacket(context.Background(), network, address)
	if err != nil {
		return nil, fmt.Errorf("listen packet: %w", err)
	}
	pConn, ok := packetConn.(*net.UDPConn)
	if !ok {
		return nil, errors.New("packet conn is not an UDPConn")
	}

	l := &Listener{
		pConn:     pConn,
		acceptCh:  make(chan *Conn),
		conns:     make(map[string]*Conn),
		accepting: true,
		timeout:   timeout,
	}

	go l.readLoop()

	return l, nil
}

// Accept waits for and returns the next connection to the listener.
func (l *Listener) Accept() (*Conn, error) {
	c := <-l.acceptCh
	if c == nil {
		// l.acceptCh got closed
		return nil, errClosedListener
	}
	return c, nil
}
Repository: traefik/traefik
File: pkg/udp/conn.go (Go)

Code before:
// Listener augments a session-oriented Listener over a UDP PacketConn.
type Listener struct {
	pConn *net.UDPConn

	mu    sync.RWMutex
	conns map[string]*Conn
	// accepting signifies whether the listener is still accepting new sessions.
	// It also serves as a sentinel for Shutdown to be idempotent.
	accepting bool

	acceptCh chan *Conn // no need for a Once, already indirectly guarded by accepting.

	// timeout defines how long to wait on an idle session,
	// before releasing its related resources.
	timeout time.Duration
}

// Listen creates a new listener.
func Listen(listenConfig net.ListenConfig, network, address string, timeout time.Duration) (*Listener, error) {
	if timeout <= 0 {
		return nil, errors.New("timeout should be greater than zero")
	}

	packetConn, err := listenConfig.ListenPacket(context.Background(), network, address)
	if err != nil {
		return nil, err
	}

	l := &Listener{
		pConn:     packetConn.(*net.UDPConn),
		acceptCh:  make(chan *Conn),
		conns:     make(map[string]*Conn),
		accepting: true,
		timeout:   timeout,
	}

	go l.readLoop()

	return l, nil
}

// Accept waits for and returns the next connection to the listener.
func (l *Listener) Accept() (*Conn, error) {
	c := <-l.acceptCh
	if c == nil {
		// l.acceptCh got closed
		return nil, errClosedListener
	}
	return c, nil
}


Review comment (suggestion):
```suggestion
		return nil, fmt.Errorf("listen packet: %w", err)
```

Code after:

// Listener augments a session-oriented Listener over a UDP PacketConn.
type Listener struct {
	pConn *net.UDPConn

	mu    sync.RWMutex
	conns map[string]*Conn
	// accepting signifies whether the listener is still accepting new sessions.
	// It also serves as a sentinel for Shutdown to be idempotent.
	accepting bool

	acceptCh chan *Conn // no need for a Once, already indirectly guarded by accepting.

	// timeout defines how long to wait on an idle session,
	// before releasing its related resources.
	timeout time.Duration
}

// Listen creates a new listener.
func Listen(listenConfig net.ListenConfig, network, address string, timeout time.Duration) (*Listener, error) {
	if timeout <= 0 {
		return nil, errors.New("timeout should be greater than zero")
	}

	packetConn, err := listenConfig.ListenPacket(context.Background(), network, address)
	if err != nil {
		return nil, fmt.Errorf("listen packet: %w", err)
	}
	pConn, ok := packetConn.(*net.UDPConn)
	if !ok {
		return nil, errors.New("packet conn is not an UDPConn")
	}

	l := &Listener{
		pConn:     pConn,
		acceptCh:  make(chan *Conn),
		conns:     make(map[string]*Conn),
		accepting: true,
		timeout:   timeout,
	}

	go l.readLoop()

	return l, nil
}

// Accept waits for and returns the next connection to the listener.
func (l *Listener) Accept() (*Conn, error) {
	c := <-l.acceptCh
	if c == nil {
		// l.acceptCh got closed
Repository: traefik/traefik
File: cmd/traefik/plugins.go (Go)

Code before:
	client, plgs, localPlgs, err := initPlugins(staticConfiguration)
	if err != nil {
		return nil, err
	}

	return plugins.NewBuilder(client, plgs, localPlgs)
}

func initPlugins(staticCfg *static.Configuration) (*plugins.Client, map[string]plugins.Descriptor, map[string]plugins.LocalDescriptor, error) {
	err := checkUniquePluginNames(staticCfg.Experimental)
	if err != nil {
		return nil, nil, nil, err
	}

	var client *plugins.Client
	plgs := map[string]plugins.Descriptor{}

	if hasPlugins(staticCfg) {
		opts := plugins.ClientOptions{
			Output: outputDir,
		}

		var err error
		client, err = plugins.NewClient(opts)
		if err != nil {
			return nil, nil, nil, fmt.Errorf("failed to create plugins client: %w", err)
		}

		err = plugins.SetupRemotePlugins(client, staticCfg.Experimental.Plugins)
		if err != nil {
			return nil, nil, nil, fmt.Errorf("failed to set up plugins environment: %w", err)
		}

		plgs = staticCfg.Experimental.Plugins
	}

	localPlgs := map[string]plugins.LocalDescriptor{}

	if hasLocalPlugins(staticCfg) {
		err := plugins.SetupLocalPlugins(staticCfg.Experimental.LocalPlugins)
		if err != nil {
			return nil, nil, nil, err
		}

		localPlgs = staticCfg.Experimental.LocalPlugins
	}

	return client, plgs, localPlgs, nil
}

func checkUniquePluginNames(e *static.Experimental) error {

Review comment (suggestion):
```suggestion
			return nil, nil, nil, fmt.Errorf("unable to create plugins client: %w", err)
```

Code after:
	client, plgs, localPlgs, err := initPlugins(staticConfiguration)
	if err != nil {
		return nil, err
	}

	return plugins.NewBuilder(client, plgs, localPlgs)
}

func initPlugins(staticCfg *static.Configuration) (*plugins.Client, map[string]plugins.Descriptor, map[string]plugins.LocalDescriptor, error) {
	err := checkUniquePluginNames(staticCfg.Experimental)
	if err != nil {
		return nil, nil, nil, err
	}

	var client *plugins.Client
	plgs := map[string]plugins.Descriptor{}

	if hasPlugins(staticCfg) {
		opts := plugins.ClientOptions{
			Output: outputDir,
		}

		var err error
		client, err = plugins.NewClient(opts)
		if err != nil {
			return nil, nil, nil, fmt.Errorf("unable to create plugins client: %w", err)
		}

		err = plugins.SetupRemotePlugins(client, staticCfg.Experimental.Plugins)
		if err != nil {
			return nil, nil, nil, fmt.Errorf("unable to set up plugins environment: %w", err)
		}

		plgs = staticCfg.Experimental.Plugins
	}

	localPlgs := map[string]plugins.LocalDescriptor{}

	if hasLocalPlugins(staticCfg) {
		err := plugins.SetupLocalPlugins(staticCfg.Experimental.LocalPlugins)
		if err != nil {
			return nil, nil, nil, err
		}

		localPlgs = staticCfg.Experimental.LocalPlugins
	}

	return client, plgs, localPlgs, nil
}

func checkUniquePluginNames(e *static.Experimental) error {
Repository: traefik/traefik
File: cmd/traefik/plugins.go (Go)

Code before:
	return plugins.NewBuilder(client, plgs, localPlgs)
}

func initPlugins(staticCfg *static.Configuration) (*plugins.Client, map[string]plugins.Descriptor, map[string]plugins.LocalDescriptor, error) {
	err := checkUniquePluginNames(staticCfg.Experimental)
	if err != nil {
		return nil, nil, nil, err
	}

	var client *plugins.Client
	plgs := map[string]plugins.Descriptor{}

	if hasPlugins(staticCfg) {
		opts := plugins.ClientOptions{
			Output: outputDir,
		}

		var err error
		client, err = plugins.NewClient(opts)
		if err != nil {
			return nil, nil, nil, fmt.Errorf("failed to create plugins client: %w", err)
		}

		err = plugins.SetupRemotePlugins(client, staticCfg.Experimental.Plugins)
		if err != nil {
			return nil, nil, nil, fmt.Errorf("failed to set up plugins environment: %w", err)
		}

		plgs = staticCfg.Experimental.Plugins
	}

	localPlgs := map[string]plugins.LocalDescriptor{}

	if hasLocalPlugins(staticCfg) {
		err := plugins.SetupLocalPlugins(staticCfg.Experimental.LocalPlugins)
		if err != nil {
			return nil, nil, nil, err
		}

		localPlgs = staticCfg.Experimental.LocalPlugins
	}

	return client, plgs, localPlgs, nil
}

func checkUniquePluginNames(e *static.Experimental) error {
	if e == nil {
		return nil
	}

	for s := range e.LocalPlugins {

Review comment (suggestion):
```suggestion
			return nil, nil, nil, fmt.Errorf("unable to set up plugins environment: %w", err)
```

Code after:
	return plugins.NewBuilder(client, plgs, localPlgs)
}

func initPlugins(staticCfg *static.Configuration) (*plugins.Client, map[string]plugins.Descriptor, map[string]plugins.LocalDescriptor, error) {
	err := checkUniquePluginNames(staticCfg.Experimental)
	if err != nil {
		return nil, nil, nil, err
	}

	var client *plugins.Client
	plgs := map[string]plugins.Descriptor{}

	if hasPlugins(staticCfg) {
		opts := plugins.ClientOptions{
			Output: outputDir,
		}

		var err error
		client, err = plugins.NewClient(opts)
		if err != nil {
			return nil, nil, nil, fmt.Errorf("unable to create plugins client: %w", err)
		}

		err = plugins.SetupRemotePlugins(client, staticCfg.Experimental.Plugins)
		if err != nil {
			return nil, nil, nil, fmt.Errorf("unable to set up plugins environment: %w", err)
		}

		plgs = staticCfg.Experimental.Plugins
	}

	localPlgs := map[string]plugins.LocalDescriptor{}

	if hasLocalPlugins(staticCfg) {
		err := plugins.SetupLocalPlugins(staticCfg.Experimental.LocalPlugins)
		if err != nil {
			return nil, nil, nil, err
		}

		localPlgs = staticCfg.Experimental.LocalPlugins
	}

	return client, plgs, localPlgs, nil
}

func checkUniquePluginNames(e *static.Experimental) error {
	if e == nil {
		return nil
	}

	for s := range e.LocalPlugins {
Repository: traefik/traefik
File: pkg/plugins/client.go (Go)

Code before:
	return c.unzipArchive(pName, pVersion)
}

func (c *Client) unzipModule(pName, pVersion string) error {
	src := c.buildArchivePath(pName, pVersion)
	dest := filepath.Join(c.sources, filepath.FromSlash(pName))

	return zip.Unzip(dest, module.Version{Path: pName, Version: pVersion}, src)
}

func (c *Client) unzipArchive(pName, pVersion string) error {
	zipPath := c.buildArchivePath(pName, pVersion)

	archive, err := zipa.OpenReader(zipPath)
	if err != nil {
		return err
	}

	defer func() { _ = archive.Close() }()

	dest := filepath.Join(c.sources, filepath.FromSlash(pName))

	for _, f := range archive.File {
		err = unzipFile(f, dest)
		if err != nil {
			return fmt.Errorf("failed to unzip %s: %w", f.Name, err)
		}
	}

	return nil
}

func unzipFile(f *zipa.File, dest string) error {
	rc, err := f.Open()
	if err != nil {
		return err
	}

	defer func() { _ = rc.Close() }()

	pathParts := strings.SplitN(f.Name, "/", 2)
	p := filepath.Join(dest, pathParts[1])

	if f.FileInfo().IsDir() {
		err = os.MkdirAll(p, f.Mode())
		if err != nil {
			return fmt.Errorf("failed to create archive directory %s: %w", p, err)
		}

		return nil
	}

Review comment (suggestion):
```suggestion
			return fmt.Errorf("unable to unzip %s: %w", f.Name, err)
```

Code after:
	return c.unzipArchive(pName, pVersion)
}

func (c *Client) unzipModule(pName, pVersion string) error {
	src := c.buildArchivePath(pName, pVersion)
	dest := filepath.Join(c.sources, filepath.FromSlash(pName))

	return zip.Unzip(dest, module.Version{Path: pName, Version: pVersion}, src)
}

func (c *Client) unzipArchive(pName, pVersion string) error {
	zipPath := c.buildArchivePath(pName, pVersion)

	archive, err := zipa.OpenReader(zipPath)
	if err != nil {
		return err
	}

	defer func() { _ = archive.Close() }()

	dest := filepath.Join(c.sources, filepath.FromSlash(pName))

	for _, f := range archive.File {
		err = unzipFile(f, dest)
		if err != nil {
			return fmt.Errorf("unable to unzip %s: %w", f.Name, err)
		}
	}

	return nil
}

func unzipFile(f *zipa.File, dest string) error {
	rc, err := f.Open()
	if err != nil {
		return err
	}

	defer func() { _ = rc.Close() }()

	pathParts := strings.SplitN(f.Name, "/", 2)
	p := filepath.Join(dest, pathParts[1])

	if f.FileInfo().IsDir() {
		err = os.MkdirAll(p, f.Mode())
		if err != nil {
			return fmt.Errorf("unable to create archive directory %s: %w", p, err)
		}

		return nil
	}