Skip to content

Commit

Permalink
mod/...: remove three somewhat unnecessary APIs
Browse files Browse the repository at this point in the history
Using golang.org/x/tools/cmd/deadcode via the command

    deadcode ./... | grep '^mod/'

I spotted a number of APIs which weren't reachable by cmd/cue,
which is a signal that perhaps they're not very useful or necessary.
Going one by one:

    mod/modregistry/client.go:86:6: unreachable func: NewClient
    mod/modregistry/client.go:471:25: unreachable func: SingleResolver.ResolveToRegistry

NewClient uses SingleResolver to interact with a registry interface:

    modregistry.NewClient(reg)

One could create the resolver if we properly exposed SingleResolver:

    modregistry.NewClientWithResolver(modregistry.SingleResolver(reg))

However, SingleResolver is an opaque struct that can't be used directly,
and it is only useful as part of NewClient anyway, so NewClient is best.
Keep NewClient as-is for the common use case, and hide SingleResolver,
since no third party user should have a direct need for it.

    mod/module/error.go:17:6: unreachable func: VersionError

The returned ModuleError type is used, but this func is never used.

    mod/module/module.go:165:6: unreachable func: MustParseVersion
    mod/module/module.go:177:6: unreachable func: ParseVersion

While ParseVersion is only used in the tests, it's the only way to
construct a module.Version from a string, so it's good for consistency.
The "must" helpers like MustParseVersion and MustNewVersion
seem to exist mainly for the sake of writing tests,
so I would personally remove them, but Roger prefers to leave them.

    mod/module/path.go:483:6: unreachable func: MatchPathMajor

This is a shortcut for `CheckPathMajor(v, pathMajor) == nil`
and is never used, not even in the tests. Remove it.

    mod/modzip/zip.go:201:6: unreachable func: CheckFiles
    mod/modzip/zip.go:369:6: unreachable func: CheckDir
    mod/modzip/zip.go:396:6: unreachable func: CheckZipFile

These aren't used by our implementation, as it only needs CheckZip,
but they make the API a bit friendlier to use.
CheckZip takes a ReaderAt plus a size, which is a bit awkward,
whereas the others take more straightforward values such a a filename.

For instance, trying to rewrite the tests to use CheckZip
made me realise that most users will not want to use CheckZip directly.

    mod/modzip/zip.go:886:22: unreachable func: ZipFileIO.Path
    mod/modzip/zip.go:889:18: unreachable func: ZipFileIO.Lstat
    mod/modzip/zip.go:892:18: unreachable func: ZipFileIO.Open

This implementation of FileIO is never used; while it seems nice to have
given that the package is already geared towards zip archives,
the FileIO abstraction is for the slice taken by CheckFiles,
and CheckZip works entirely differently and does not use FileIO at all.

While the FileIO abstraction seems nice as we use it elsewhere
for CheckFiles on txtar archives, ZipFileIO seems unnecessary for now.

For #2896.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: Ib57e4b224ba3ee7c2d713395b50ac9942c56034c
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1177674
Reviewed-by: Roger Peppe <rogpeppe@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@gmail.com>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
  • Loading branch information
mvdan committed Mar 4, 2024
1 parent aa60650 commit 6cba4c0
Show file tree
Hide file tree
Showing 4 changed files with 4 additions and 44 deletions.
8 changes: 4 additions & 4 deletions mod/modregistry/client.go
Expand Up @@ -85,7 +85,7 @@ const (
// hostname.
func NewClient(registry ociregistry.Interface) *Client {
return &Client{
resolver: SingleResolver{registry},
resolver: singleResolver{registry},
}
}

Expand Down Expand Up @@ -461,14 +461,14 @@ func (c *Client) scratchConfig(ctx context.Context, loc RegistryLocation, mediaT
return desc, nil
}

// SingleResolver implements Resolver by always returning R,
// singleResolver implements Resolver by always returning R,
// and mapping module paths directly to repository paths in
// the registry.
type SingleResolver struct {
type singleResolver struct {
R ociregistry.Interface
}

func (r SingleResolver) ResolveToRegistry(mpath, vers string) (RegistryLocation, error) {
func (r singleResolver) ResolveToRegistry(mpath, vers string) (RegistryLocation, error) {
return RegistryLocation{
Registry: r.R,
Repository: mpath,
Expand Down
15 changes: 0 additions & 15 deletions mod/module/error.go
@@ -1,7 +1,6 @@
package module

import (
"errors"
"fmt"
)

Expand All @@ -12,20 +11,6 @@ type ModuleError struct {
Err error
}

// VersionError returns a ModuleError derived from a Version and error,
// or err itself if it is already such an error.
func VersionError(v Version, err error) error {
var mErr *ModuleError
if errors.As(err, &mErr) && mErr.Path == v.Path() && mErr.Version == v.Version() {
return err
}
return &ModuleError{
Path: v.Path(),
Version: v.Version(),
Err: err,
}
}

func (e *ModuleError) Error() string {
if v, ok := e.Err.(*InvalidVersionError); ok {
return fmt.Sprintf("%s@%s: invalid version: %v", e.Path, v.Version, v.Err)
Expand Down
8 changes: 0 additions & 8 deletions mod/module/path.go
Expand Up @@ -476,14 +476,6 @@ func ParseImportPath(p string) ImportPath {
return parts
}

// MatchPathMajor reports whether the semantic version v
// matches the path major version pathMajor.
//
// MatchPathMajor returns true if and only if CheckPathMajor returns nil.
func MatchPathMajor(v, pathMajor string) bool {
return CheckPathMajor(v, pathMajor) == nil
}

// CheckPathMajor returns a non-nil error if the semantic version v
// does not match the path major version pathMajor.
func CheckPathMajor(v, pathMajor string) error {
Expand Down
17 changes: 0 additions & 17 deletions mod/modzip/zip.go
Expand Up @@ -875,20 +875,3 @@ func splitCUEMod(p string) (string, string) {
s = dir
}
}

// ZipFileIO implements FileIO for *zip.File.
type ZipFileIO struct {
// StripPrefix causes the given prefix to be stripped from
// all file names with that prefix.
StripPrefix string
}

func (fio ZipFileIO) Path(f *zip.File) string {
return strings.TrimPrefix(f.Name, fio.StripPrefix)
}
func (ZipFileIO) Lstat(f *zip.File) (os.FileInfo, error) {
return f.FileInfo(), nil
}
func (ZipFileIO) Open(f *zip.File) (io.ReadCloser, error) {
return f.Open()
}

0 comments on commit 6cba4c0

Please sign in to comment.