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

fix: address gocritic issues #127

Merged
merged 3 commits into from
Oct 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
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
4 changes: 0 additions & 4 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@ linters:

# Logical next step
- forcetypeassert # Priority: that can lead to serious crashes.
- gocritic # Priority:
# - security issues (slice copy)
# - unexpected behavior
# - Go practices ('p += k' and not 'p = p + k')
- gosec # Priority: Security (as the name implies)
- revive # Too many issues.
# It's important to at least address:
Expand Down
6 changes: 2 additions & 4 deletions cmds/ocm/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,10 +243,8 @@ func (o *CLIOptions) Complete() error {
}
if len(attrs) != 0 {
o.Context.CredentialsContext().SetCredentialsForConsumer(id, credentials.NewCredentials(attrs))
} else {
if len(id) != 0 {
return errors.Newf("empty credential attribute set for %s", id.String())
}
} else if len(id) != 0 {
return errors.Newf("empty credential attribute set for %s", id.String())
}

set, err := common2.ParseLabels(o.Settings, "attribute setting")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func clean(iterable data.Iterable) data.Iterable {
d := depth[dig]
if l == 0 && l < d && (e.Spec.Tag == nil || *e.Spec.Tag == tags[dig]) {
j := i + 1
prefix := append(e.History, common.NewNameVersion("", dig.String()))
prefix := e.History.Append(common.NewNameVersion("", dig.String()))
for ; j < len(data) && data[j].(*Object).History.HasPrefix(prefix); j++ {
}
data = append(data[:i], data[j:]...)
Expand Down
6 changes: 2 additions & 4 deletions cmds/ocm/commands/ocmcmds/common/inputs/types/spiff/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,8 @@ func (s *Spec) Validate(fldPath *field.Path, ctx clictx.Context, inputFilePath s
fileInfo, filePath, err := inputs.FileInfo(ctx, v, inputFilePath)
if err != nil {
allErrs = append(allErrs, field.Invalid(pathField, filePath, err.Error()))
} else {
if !fileInfo.Mode().IsRegular() {
allErrs = append(allErrs, field.Invalid(pathField, filePath, "no regular file"))
}
} else if !fileInfo.Mode().IsRegular() {
allErrs = append(allErrs, field.Invalid(pathField, filePath, "no regular file"))
}
}
return allErrs
Expand Down
8 changes: 3 additions & 5 deletions cmds/ocm/commands/ocmcmds/common/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,11 +326,9 @@ func Validate(r *ResourceInput, ctx clictx.Context, inputFilePath string) error
}
raw, _ := r.Access.GetRaw()
allErrs = append(allErrs, field.Invalid(fldPath.Child("access"), string(raw), err.Error()))
} else {
if acc.(ocm.AccessSpec).IsLocal(ctx.OCMContext()) {
kind := runtime.ObjectVersionedType(r.Access.ObjectType).GetKind()
allErrs = append(allErrs, field.Invalid(fldPath.Child("access", "type"), kind, "local access no possible"))
}
} else if acc.(ocm.AccessSpec).IsLocal(ctx.OCMContext()) {
kind := runtime.ObjectVersionedType(r.Access.ObjectType).GetKind()
allErrs = append(allErrs, field.Invalid(fldPath.Child("access", "type"), kind, "local access no possible"))
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions cmds/ocm/commands/ocmcmds/references/get/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
"github.com/open-component-model/ocm/cmds/ocm/pkg/output"
"github.com/open-component-model/ocm/cmds/ocm/pkg/processing"
"github.com/open-component-model/ocm/cmds/ocm/pkg/utils"
gcommom "github.com/open-component-model/ocm/pkg/common"
gcommon "github.com/open-component-model/ocm/pkg/common"
"github.com/open-component-model/ocm/pkg/contexts/clictx"
"github.com/open-component-model/ocm/pkg/contexts/ocm"
metav1 "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc/meta/v1"
Expand Down Expand Up @@ -99,9 +99,9 @@ outer:
for i := 0; i < len(slice); i++ {
o := slice[i]
e := common.Elem(o)
key := gcommom.NewNameVersion(e.ComponentName, e.Version)
key := gcommon.NewNameVersion(e.ComponentName, e.Version)
hist := o.GetHistory()
nested := append(hist, key)
nested := hist.Append(key)
var j int
for j = i + 1; j < len(slice); j++ {
n := slice[j]
Expand Down
6 changes: 2 additions & 4 deletions cmds/ocm/commands/ocmcmds/resources/add/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,8 @@ func (ResourceSpecHandler) Set(v ocm.ComponentVersionAccess, r common.Resource,
if spec.Relation == metav1.LocalRelation {
if vers == "" || vers == ComponentVersionTag {
vers = v.GetVersion()
} else {
if vers != v.GetVersion() {
return errors.Newf("local resource %q (%s) has non-matching version %q", spec.Name, r.Source(), vers)
}
} else if vers != v.GetVersion() {
return errors.Newf("local resource %q (%s) has non-matching version %q", spec.Name, r.Source(), vers)
}
}
if vers == ComponentVersionTag {
Expand Down
6 changes: 2 additions & 4 deletions cmds/ocm/commands/ocmcmds/resources/download/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,10 +229,8 @@ func (d *action) Save(o *elemhdlr.Object, f string) error {
if err != nil {
return err
}
} else {
if eff != f && pathIn {
out.Outf(d.opts.Context, "output path %q changed to %q by downloader", f, eff)
}
} else if eff != f && pathIn {
out.Outf(d.opts.Context, "output path %q changed to %q by downloader", f, eff)
}
return nil
}
8 changes: 3 additions & 5 deletions cmds/ocm/commands/ocmcmds/sources/add/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,9 @@ func (r *ResourceSpec) Validate(ctx clictx.Context, input *common.ResourceInput)
if err != nil {
raw, _ := r.Access.GetRaw()
allErrs = append(allErrs, field.Invalid(fldPath.Child("access"), string(raw), err.Error()))
} else {
if acc.(ocm.AccessSpec).IsLocal(ctx.OCMContext()) {
kind := runtime.ObjectVersionedType(r.Access.ObjectType).GetKind()
allErrs = append(allErrs, field.Invalid(fldPath.Child("access", "type"), kind, "local access no possible"))
}
} else if acc.(ocm.AccessSpec).IsLocal(ctx.OCMContext()) {
kind := runtime.ObjectVersionedType(r.Access.ObjectType).GetKind()
allErrs = append(allErrs, field.Invalid(fldPath.Child("access", "type"), kind, "local access no possible"))
}
}
}
Expand Down
6 changes: 2 additions & 4 deletions cmds/ocm/pkg/output/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,8 @@ func FormatTable(ctx Context, gap string, data [][]string) {
for i, col := range row {
if i >= len(columns) {
columns = append(columns, len(col))
} else {
if columns[i] < len(col) {
columns[i] = len(col)
}
} else if columns[i] < len(col) {
columns[i] = len(col)
}
if len(col) > max {
max = len(col)
Expand Down
12 changes: 4 additions & 8 deletions cmds/ocm/pkg/processing/chain.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,8 @@ func (this *_ProcessChain) new(p *_ProcessChain, creator stepCreator) *_ProcessC
if p != nil {
if p.creator != nil {
this.parent = p
} else {
if p.parent != nil {
this.parent = p.parent
}
} else if p.parent != nil {
this.parent = p.parent
}
}
if this.parent != nil && creator == nil {
Expand Down Expand Up @@ -129,10 +127,8 @@ func (this *_ProcessChain) Process(data data.Iterable) ProcessingResult {
p, ok := data.(ProcessingResult)
if this.parent != nil {
p = this.parent.Process(data)
} else {
if !ok {
p = Process(data)
}
} else if !ok {
p = Process(data)
}
return this.step(p)
}
Expand Down
2 changes: 1 addition & 1 deletion cmds/ocm/pkg/utils/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func SetupCommand(ocmcmd OCMCommand, names ...string) *cobra.Command {
return err
}
if u, ok := ocmcmd.(options.Usage); ok {
c.Long = c.Long + u.Usage()
c.Long += u.Usage()
}
ocmcmd.AddFlags(c.Flags())
return c
Expand Down
2 changes: 1 addition & 1 deletion pkg/cobrautils/links.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func LinkForCmd(cmd *cobra.Command) string {

func LinkForPath(path string) string {
link := path + ".md"
link = strings.Replace(link, " ", "_", -1)
link = strings.ReplaceAll(link, " ", "_")
return link
}

Expand Down
6 changes: 2 additions & 4 deletions pkg/common/accessobj/filesystemaccess.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,8 @@ func (a *FileSystemBlobAccess) AddBlob(blob accessio.BlobAccess) error {

if ok, err := vfs.FileExists(a.base.GetFileSystem(), path); ok {
return nil
} else {
if err != nil {
return fmt.Errorf("failed to check if '%s' file exists: %w", path, err)
}
} else if err != nil {
return fmt.Errorf("failed to check if '%s' file exists: %w", path, err)
}

r, err := blob.Reader()
Expand Down
7 changes: 4 additions & 3 deletions pkg/common/accessobj/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,17 +133,18 @@ func DefaultOpenOptsFileHandling(kind string, info *AccessObjectInfo, acc Access
var file vfs.File
var err error
var closer Closer
if opts.Reader != nil {
switch {
case opts.Reader != nil:
reader = opts.Reader
defer opts.Reader.Close()
} else if opts.File == nil {
case opts.File == nil:
// we expect that the path point to a tar
file, err = opts.PathFileSystem.Open(path)
if err != nil {
return nil, fmt.Errorf("unable to open %s from %s: %w", kind, path, err)
}
defer file.Close()
} else {
default:
file = opts.File
}
if file != nil {
Expand Down
7 changes: 7 additions & 0 deletions pkg/common/history.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,13 @@ func (h *History) Add(kind string, nv NameVersion) error {
return nil
}

func (h History) Append(nv ...NameVersion) History {
result := make(History, len(h)+len(nv))
copy(result, h)
copy(result[len(h):], nv)
return result
}

func (h History) Copy() History {
return append(h[:0:0], h...)
}
Expand Down
6 changes: 2 additions & 4 deletions pkg/contexts/oci/attrs/cacheattr/attr.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,8 @@ func (a AttributeType) Decode(data []byte, unmarshaller runtime.Unmarshaler) (in
if err == nil {
return accessio.NewStaticBlobCache(value)
}
} else {
if err == nil {
err = errors.Newf("file path missing")
}
} else if err == nil {
err = errors.Newf("file path missing")
}
return value, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/contexts/oci/core/uniform.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func (u *UniformRepositorySpec) ComposeRef(art string) string {
func (u *UniformRepositorySpec) RepositoryRef() string {
t := u.Type
if t != "" {
t = t + "::"
t += "::"
}
if u.Info != "" {
return fmt.Sprintf("%s%s", t, u.Info)
Expand Down
6 changes: 2 additions & 4 deletions pkg/contexts/oci/cpi/flavor_artefact.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,10 +257,8 @@ func AdjustSize(d *artdesc.Descriptor, size int64) error {
if size != accessio.BLOB_UNKNOWN_SIZE {
if d.Size == accessio.BLOB_UNKNOWN_SIZE {
d.Size = size
} else {
if d.Size != size {
return errors.Newf("blob size mismatch %d != %d", size, d.Size)
}
} else if d.Size != size {
return errors.Newf("blob size mismatch %d != %d", size, d.Size)
}
}
return nil
Expand Down
6 changes: 2 additions & 4 deletions pkg/contexts/oci/cpi/support/flavor_artefact.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,10 +232,8 @@ func AdjustSize(d *artdesc.Descriptor, size int64) error {
if size != accessio.BLOB_UNKNOWN_SIZE {
if d.Size == accessio.BLOB_UNKNOWN_SIZE {
d.Size = size
} else {
if d.Size != size {
return errors.Newf("blob size mismatch %d != %d", size, d.Size)
}
} else if d.Size != size {
return errors.Newf("blob size mismatch %d != %d", size, d.Size)
}
}
return nil
Expand Down
13 changes: 7 additions & 6 deletions pkg/contexts/oci/ociutils/helm/ignore/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,10 @@ func (r *Rules) parseRule(rule string) error {
rule = strings.TrimSuffix(rule, "/")
}

if strings.HasPrefix(rule, "/") {
switch {
case strings.HasPrefix(rule, "/"):
// Require path matches the root path.
p.match = func(n string, fi os.FileInfo) bool {
p.match = func(n string, _ os.FileInfo) bool {
rule = strings.TrimPrefix(rule, "/")
ok, err := filepath.Match(rule, n)
if err != nil {
Expand All @@ -184,18 +185,18 @@ func (r *Rules) parseRule(rule string) error {
}
return ok
}
} else if strings.Contains(rule, "/") {
case strings.Contains(rule, "/"):
// require structural match.
p.match = func(n string, fi os.FileInfo) bool {
p.match = func(n string, _ os.FileInfo) bool {
ok, err := filepath.Match(rule, n)
if err != nil {
log.Printf("Failed to compile %q: %s", rule, err)
return false
}
return ok
}
} else {
p.match = func(n string, fi os.FileInfo) bool {
default:
p.match = func(n string, _ os.FileInfo) bool {
// When there is no slash in the pattern, we evaluate ONLY the
// filename.
n = filepath.Base(n)
Expand Down
6 changes: 2 additions & 4 deletions pkg/contexts/oci/repositories/ctf/index/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,8 @@ func (r *RepositoryIndex) GetTags(repo string) []string {
if !strings.HasPrefix(t, "@") {
result = append(result, t)
digests[a.Digest] = true
} else {
if !digests[a.Digest] {
digests[a.Digest] = false
}
} else if !digests[a.Digest] {
digests[a.Digest] = false
}
}
/* TODO: how to query untagged entries at api level
Expand Down
6 changes: 2 additions & 4 deletions pkg/contexts/ocm/accessmethods/ociblob/method.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,8 @@ func (m *accessMethod) getBlob() (cpi.BlobAccess, error) {
}
if m.spec.Size == accessio.BLOB_UNKNOWN_SIZE {
m.spec.Size = size
} else {
if size != accessio.BLOB_UNKNOWN_SIZE {
return nil, errors.Newf("blob size mismatch %d != %d", size, m.spec.Size)
}
} else if size != accessio.BLOB_UNKNOWN_SIZE {
return nil, errors.Newf("blob size mismatch %d != %d", size, m.spec.Size)
}
m.blob = accessio.BlobAccessForDataAccess(m.spec.Digest, m.spec.Size, m.spec.MediaType, acc)
return m.blob, nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/contexts/ocm/compdesc/codecs.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,6 @@ var DefaultYAMLCodec = CodecWrapper{
// DefaultJSONCodec implements Codec interface with the json decoder encoder.
var DefaultJSONLCodec = CodecWrapper{
Decoder: DecoderFunc(json.Unmarshal),
StrictDecoder: StrictDecoderFunc(func(data []byte, obj interface{}) error { return json.Unmarshal(data, obj) }),
StrictDecoder: StrictDecoderFunc(json.Unmarshal),
Encoder: EncoderFunc(json.Marshal),
}
2 changes: 1 addition & 1 deletion pkg/contexts/ocm/core/uniform.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (r *UniformRepositorySpec) CredHost() string {
func (u *UniformRepositorySpec) String() string {
t := u.Type
if t != "" && !ocireg.IsKind(t) {
t = t + "::"
t += "::"
}
if u.Info != "" {
return fmt.Sprintf("%s%s", t, u.Info)
Expand Down
5 changes: 2 additions & 3 deletions pkg/contexts/ocm/cpi/support/compversaccess.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ func (c *componentVersionAccessImpl) SetResource(meta *cpi.ResourceMeta, acc com

cd := c.GetDescriptor()
if idx := cd.GetResourceIndex(meta); idx == -1 {
cd.Resources = append(c.GetDescriptor().Resources, *res)
cd.Resources = append(cd.Resources, *res)
mandelsoft marked this conversation as resolved.
Show resolved Hide resolved
cd.Signatures = nil
} else {
if !cd.Resources[idx].ResourceMeta.HashEqual(&res.ResourceMeta) {
Expand Down Expand Up @@ -332,8 +332,7 @@ func (c *componentVersionAccessImpl) SetSource(meta *cpi.SourceMeta, acc compdes
Access: acc,
}

switch res.Version {
case "":
if res.Version == "" {
mandelsoft marked this conversation as resolved.
Show resolved Hide resolved
res.Version = c.GetVersion()
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/contexts/ocm/download/handlers/helm/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func (h Handler) Download(ctx out.Context, racc cpi.ResourceAccess, path string,
return true, "", errors.Newf("no layers found")
}
if !strings.HasSuffix(path, ".tgz") {
path = path + ".tgz"
path += ".tgz"
}
blob, err := m.GetBlob(m.GetDescriptor().Layers[0].Digest)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/contexts/ocm/ref.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func (r *RefSpec) HostPort() (string, string) {
func (r *RefSpec) Reference() string {
t := r.Type
if t != "" {
t = t + "::"
t += "::"
}
s := r.SubPath
if s != "" {
Expand Down
Loading