Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable some more golangci-lint checks, fix findings #1164

Merged
merged 3 commits into from
Nov 4, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Unexport NewErrBadName and remove IsErrBadName
  • Loading branch information
imjasonh committed Nov 4, 2021
commit 063e659a7506fefe6bc9d23ff671e7e72b4eab09
4 changes: 2 additions & 2 deletions pkg/name/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ func stripRunesFn(runes string) func(rune) rune {
func checkElement(name, element, allowedRunes string, minRunes, maxRunes int) error {
numRunes := utf8.RuneCountInString(element)
if (numRunes < minRunes) || (maxRunes < numRunes) {
return NewErrBadName("%s must be between %d and %d runes in length: %s", name, minRunes, maxRunes, element)
return newErrBadName("%s must be between %d and %d runes in length: %s", name, minRunes, maxRunes, element)
} else if len(strings.Map(stripRunesFn(allowedRunes), element)) != 0 {
return NewErrBadName("%s can only contain the runes `%s`: %s", name, allowedRunes, element)
return newErrBadName("%s can only contain the runes `%s`: %s", name, allowedRunes, element)
}
return nil
}
2 changes: 1 addition & 1 deletion pkg/name/digest.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func NewDigest(name string, opts ...Option) (Digest, error) {
// Split on "@"
parts := strings.Split(name, digestDelim)
if len(parts) != 2 {
return Digest{}, NewErrBadName("a digest must contain exactly one '@' separator (e.g. registry/repository@digest) saw: %s", name)
return Digest{}, newErrBadName("a digest must contain exactly one '@' separator (e.g. registry/repository@digest) saw: %s", name)
}
base := parts[0]
digest := parts[1]
Expand Down
11 changes: 2 additions & 9 deletions pkg/name/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package name

import (
"errors"
"fmt"
)

Expand All @@ -28,13 +27,7 @@ func (e *ErrBadName) Error() string {
return e.info
}

// NewErrBadName returns a ErrBadName which returns the given formatted string from Error().
func NewErrBadName(fmtStr string, args ...interface{}) *ErrBadName {
// newErrBadName returns a ErrBadName which returns the given formatted string from Error().
func newErrBadName(fmtStr string, args ...interface{}) *ErrBadName {
imjasonh marked this conversation as resolved.
Show resolved Hide resolved
return &ErrBadName{fmt.Sprintf(fmtStr, args...)}
}

// IsErrBadName returns true if the given error is an ErrBadName.
func IsErrBadName(err error) bool {
var berr *ErrBadName
return errors.As(err, &berr)
}
6 changes: 4 additions & 2 deletions pkg/name/errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@
package name

import (
"errors"
"testing"
)

func TestBadName(t *testing.T) {
_, err := ParseReference("@@")
if !IsErrBadName(err) {
t.Errorf("IsBadErrName == false: %v", err)
var berr *ErrBadName
if !errors.As(err, &berr) {
t.Errorf("Not an ErrBadName: %v", err)
}
if err.Error() != "could not parse reference: @@" {
t.Errorf("Unexpected string: %v", err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/name/ref.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func ParseReference(s string, opts ...Option) (Reference, error) {
if d, err := NewDigest(s, opts...); err == nil {
return d, nil
}
return nil, NewErrBadName("could not parse reference: " + s)
return nil, newErrBadName("could not parse reference: " + s)
}

type stringConst string
Expand Down
4 changes: 2 additions & 2 deletions pkg/name/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func checkRegistry(name string) error {
// Per RFC 3986, registries (authorities) are required to be prefixed with "//"
// url.Host == hostname[:port] == authority
if url, err := url.Parse("//" + name); err != nil || url.Host != name {
return NewErrBadName("registries must be valid RFC 3986 URI authorities: %s", name)
return newErrBadName("registries must be valid RFC 3986 URI authorities: %s", name)
}
return nil
}
Expand All @@ -108,7 +108,7 @@ func checkRegistry(name string) error {
func NewRegistry(name string, opts ...Option) (Registry, error) {
opt := makeOptions(opts...)
if opt.strict && len(name) == 0 {
return Registry{}, NewErrBadName("strict validation requires the registry to be explicitly defined")
return Registry{}, newErrBadName("strict validation requires the registry to be explicitly defined")
}

if err := checkRegistry(name); err != nil {
Expand Down
4 changes: 2 additions & 2 deletions pkg/name/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func checkRepository(repository string) error {
func NewRepository(name string, opts ...Option) (Repository, error) {
opt := makeOptions(opts...)
if len(name) == 0 {
return Repository{}, NewErrBadName("a repository name must be specified")
return Repository{}, newErrBadName("a repository name must be specified")
}

var registry string
Expand All @@ -95,7 +95,7 @@ func NewRepository(name string, opts ...Option) (Repository, error) {
return Repository{}, err
}
if hasImplicitNamespace(repo, reg) && opt.strict {
return Repository{}, NewErrBadName("strict validation requires the full repository path (missing 'library')")
return Repository{}, newErrBadName("strict validation requires the full repository path (missing 'library')")
}
return Repository{reg, repo}, nil
}
Expand Down
6 changes: 4 additions & 2 deletions pkg/name/repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package name

import (
"errors"
"strings"
"testing"
)
Expand Down Expand Up @@ -116,8 +117,9 @@ func TestRepositoryScopes(t *testing.T) {
}

func TestRepositoryBadDefaulting(t *testing.T) {
if _, err := NewRepository("index.docker.io/foo", StrictValidation); !IsErrBadName(err) {
t.Errorf("IsBadErrName == false: %v", err)
var berr *ErrBadName
if _, err := NewRepository("index.docker.io/foo", StrictValidation); !errors.As(err, &berr) {
t.Errorf("Not an ErrBadName: %v", err)
}
}

Expand Down
3 changes: 1 addition & 2 deletions pkg/v1/remote/descriptor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@ func TestGetSchema1(t *testing.T) {
want := `unsupported MediaType: "application/vnd.docker.distribution.manifest.v1+prettyjws", see https://github.com/google/go-containerregistry/issues/377`
// Should fail based on media type.
if _, err := desc.Image(); err != nil {
var s1err ErrSchema1
if errors.Is(err, &s1err) {
if errors.Is(err, &ErrSchema1{}) {
t.Errorf("Image() = %v, expected remote.ErrSchema1", err)
}
if diff := cmp.Diff(want, err.Error()); diff != "" {
Expand Down