From e09c12077847389b04745a8dd11c73b6162e2767 Mon Sep 17 00:00:00 2001 From: Keepers Date: Wed, 29 Mar 2023 15:16:05 -0600 Subject: [PATCH] add linter for using clues to create and wrap errs (#2959) #### Does this PR need a docs update or release note? - [x] :no_entry: No #### Type of change - [x] :broom: Tech Debt/Cleanup #### Issue(s) * #1970 #### Test Plan - [x] :zap: Unit test --- src/.golangci.yml | 6 +++++- src/internal/connector/discovery/discovery.go | 3 +-- src/internal/connector/exchange/api/contacts.go | 2 +- src/internal/connector/exchange/api/events.go | 2 +- src/internal/connector/exchange/api/mail.go | 2 +- src/internal/connector/exchange/api/options.go | 3 ++- src/internal/connector/exchange/api/shared.go | 4 ++-- src/internal/connector/exchange/service_restore.go | 2 +- src/internal/connector/support/m365Transform.go | 9 +++++---- src/pkg/fault/fault_test.go | 3 +-- src/pkg/selectors/selectors.go | 4 ++-- 11 files changed, 22 insertions(+), 18 deletions(-) diff --git a/src/.golangci.yml b/src/.golangci.yml index c35a4fbb7..15f93b7b4 100644 --- a/src/.golangci.yml +++ b/src/.golangci.yml @@ -34,11 +34,14 @@ linters-settings: # Use filepath instead. - '\bpath\.(Ext|Base|Dir|Join)' # Don't allow the typo m356 to be used in place of m365. - - '[Mm]356' + - '[Mm]356(# typo: should be 365)?' # Don't allow use of testify suite directly. Use one of the wrappers from # tester/suite.go instead. Use an ignore lower down to exclude packages # that result in import cycles if they try to use the wrapper. - 'suite\.Suite(# tests should use one of the Suite wrappers in tester package )?' + # All errors should be constructed and wrapped with the clues package. + # String formatting should be avoided in favor of structured errors (ie: err.With(k, v)). + - '(errors|fmt)\.(New|Stack|Wrap|Error)f?\((# error handling should use clues pkg)?' lll: line-length: 120 revive: @@ -120,6 +123,7 @@ issues: - gofumpt - misspell - errcheck + - forbidigo - path: internal/tester/suite.go linters: - forbidigo diff --git a/src/internal/connector/discovery/discovery.go b/src/internal/connector/discovery/discovery.go index ab1efba60..0090c51ea 100644 --- a/src/internal/connector/discovery/discovery.go +++ b/src/internal/connector/discovery/discovery.go @@ -2,7 +2,6 @@ package discovery import ( "context" - "fmt" "github.com/alcionai/clues" "github.com/microsoftgraph/msgraph-sdk-go/models" @@ -57,7 +56,7 @@ func User(ctx context.Context, gwi getWithInfoer, userID string) (models.Userabl u, err := gwi.GetByID(ctx, userID) if err != nil { if graph.IsErrUserNotFound(err) { - return nil, nil, fmt.Errorf("resource owner [%s] not found within tenant", userID) + return nil, nil, clues.New("resource owner not found within tenant").With("user_id", userID) } return nil, nil, clues.Wrap(err, "getting user") diff --git a/src/internal/connector/exchange/api/contacts.go b/src/internal/connector/exchange/api/contacts.go index 2c8362d7b..c9cf824fe 100644 --- a/src/internal/connector/exchange/api/contacts.go +++ b/src/internal/connector/exchange/api/contacts.go @@ -279,7 +279,7 @@ func (c Contacts) Serialize( ) ([]byte, error) { contact, ok := item.(models.Contactable) if !ok { - return nil, clues.Wrap(fmt.Errorf("parseable type: %T", item), "parsable is not a Contactable") + return nil, clues.New(fmt.Sprintf("item is not a Contactable: %T", item)) } ctx = clues.Add(ctx, "item_id", ptr.Val(contact.GetId())) diff --git a/src/internal/connector/exchange/api/events.go b/src/internal/connector/exchange/api/events.go index 5b99b3324..cfb79e80b 100644 --- a/src/internal/connector/exchange/api/events.go +++ b/src/internal/connector/exchange/api/events.go @@ -314,7 +314,7 @@ func (c Events) Serialize( ) ([]byte, error) { event, ok := item.(models.Eventable) if !ok { - return nil, clues.Wrap(fmt.Errorf("parseable type: %T", item), "parsable is not an Eventable") + return nil, clues.New(fmt.Sprintf("item is not an Eventable: %T", item)) } ctx = clues.Add(ctx, "item_id", ptr.Val(event.GetId())) diff --git a/src/internal/connector/exchange/api/mail.go b/src/internal/connector/exchange/api/mail.go index ff8dbb476..890276c54 100644 --- a/src/internal/connector/exchange/api/mail.go +++ b/src/internal/connector/exchange/api/mail.go @@ -324,7 +324,7 @@ func (c Mail) Serialize( ) ([]byte, error) { msg, ok := item.(models.Messageable) if !ok { - return nil, clues.Wrap(fmt.Errorf("parseable type: %T", item), "parsable is not a Messageable") + return nil, clues.New(fmt.Sprintf("item is not a Messageable: %T", item)) } ctx = clues.Add(ctx, "item_id", ptr.Val(msg.GetId())) diff --git a/src/internal/connector/exchange/api/options.go b/src/internal/connector/exchange/api/options.go index 3c2c39c0e..b822070d9 100644 --- a/src/internal/connector/exchange/api/options.go +++ b/src/internal/connector/exchange/api/options.go @@ -3,6 +3,7 @@ package api import ( "fmt" + "github.com/alcionai/clues" abstractions "github.com/microsoft/kiota-abstractions-go" "github.com/microsoftgraph/msgraph-sdk-go/users" ) @@ -262,7 +263,7 @@ func buildOptions(fields []string, allowed map[string]struct{}) ([]string, error for _, entry := range fields { _, ok := allowed[entry] if !ok { - return nil, fmt.Errorf("unsupported field: %v", entry) + return nil, clues.New("unsupported field: " + entry) } } diff --git a/src/internal/connector/exchange/api/shared.go b/src/internal/connector/exchange/api/shared.go index 8431f58c5..b5bb4fb90 100644 --- a/src/internal/connector/exchange/api/shared.go +++ b/src/internal/connector/exchange/api/shared.go @@ -36,7 +36,7 @@ type getIDAndAddtler interface { func toValues[T any](a any) ([]getIDAndAddtler, error) { gv, ok := a.(interface{ GetValue() []T }) if !ok { - return nil, clues.Wrap(fmt.Errorf("%T", a), "does not comply with the GetValue() interface") + return nil, clues.New(fmt.Sprintf("type does not comply with the GetValue() interface: %T", a)) } items := gv.GetValue() @@ -47,7 +47,7 @@ func toValues[T any](a any) ([]getIDAndAddtler, error) { ri, ok := a.(getIDAndAddtler) if !ok { - return nil, clues.Wrap(fmt.Errorf("%T", item), "does not comply with the getIDAndAddtler interface") + return nil, clues.New(fmt.Sprintf("type does not comply with the getIDAndAddtler interface: %T", item)) } r = append(r, ri) diff --git a/src/internal/connector/exchange/service_restore.go b/src/internal/connector/exchange/service_restore.go index d323bbbfd..3a0cdd68e 100644 --- a/src/internal/connector/exchange/service_restore.go +++ b/src/internal/connector/exchange/service_restore.go @@ -569,7 +569,7 @@ func CreateContainerDestination( errs) default: - return "", clues.Wrap(fmt.Errorf("%T", category), "not support for exchange cache").WithClues(ctx) + return "", clues.New(fmt.Sprintf("type not supported: %T", category)).WithClues(ctx) } } diff --git a/src/internal/connector/support/m365Transform.go b/src/internal/connector/support/m365Transform.go index 67f88fbcf..c5f73fe8f 100644 --- a/src/internal/connector/support/m365Transform.go +++ b/src/internal/connector/support/m365Transform.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "github.com/alcionai/clues" "github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/alcionai/corso/src/internal/common/ptr" @@ -325,13 +326,13 @@ const ( func ToItemAttachment(orig models.Attachmentable) (models.Attachmentable, error) { transform, ok := orig.(models.ItemAttachmentable) if !ok { // Shouldn't ever happen - return nil, fmt.Errorf("transforming attachment to item attachment") + return nil, clues.New("transforming attachment to item attachment") } item := transform.GetItem() - itemType := item.GetOdataType() + itemType := ptr.Val(item.GetOdataType()) - switch *itemType { + switch itemType { case contactItemType: contact := item.(models.Contactable) revised := sanitizeContact(contact) @@ -362,7 +363,7 @@ func ToItemAttachment(orig models.Attachmentable) (models.Attachmentable, error) return transform, nil default: - return nil, fmt.Errorf("exiting ToItemAttachment: %s not supported", *itemType) + return nil, clues.New(fmt.Sprintf("unsupported attachment type: %T", itemType)) } } diff --git a/src/pkg/fault/fault_test.go b/src/pkg/fault/fault_test.go index a56ddcc03..3370e3d3e 100644 --- a/src/pkg/fault/fault_test.go +++ b/src/pkg/fault/fault_test.go @@ -2,7 +2,6 @@ package fault_test import ( "encoding/json" - "fmt" "testing" "github.com/alcionai/clues" @@ -388,7 +387,7 @@ func (suite *FaultErrorsUnitSuite) TestUnmarshalLegacy() { t := suite.T() oldData := &legacyErrorsData{ - Errs: []error{fmt.Errorf("foo error"), fmt.Errorf("foo error"), fmt.Errorf("foo error")}, + Errs: []error{clues.New("foo error"), clues.New("foo error"), clues.New("foo error")}, } jsonStr, err := json.Marshal(oldData) diff --git a/src/pkg/selectors/selectors.go b/src/pkg/selectors/selectors.go index 10b56f4e7..caf1f5cb1 100644 --- a/src/pkg/selectors/selectors.go +++ b/src/pkg/selectors/selectors.go @@ -3,10 +3,10 @@ package selectors import ( "context" "encoding/json" + "fmt" "strings" "github.com/alcionai/clues" - "github.com/pkg/errors" "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/fault" @@ -391,7 +391,7 @@ func pathComparator() option { } func badCastErr(cast, is service) error { - return clues.Stack(ErrorBadSelectorCast, errors.Errorf("%s is not %s", cast, is)) + return clues.Stack(ErrorBadSelectorCast, clues.New(fmt.Sprintf("%s is not %s", cast, is))) } func join(s ...string) string {