From 67e557e9a71f5fb1c2ecd5e40028f01415202492 Mon Sep 17 00:00:00 2001 From: Majd Alfhaily Date: Sat, 27 May 2023 23:21:13 +0200 Subject: [PATCH] Add golangci-lint linter (#188) --- .github/workflows/lint.yml | 21 ++++++++ .github/workflows/unit-tests.yml | 2 +- .golangci.yml | 22 ++++++++ cmd/auth.go | 22 ++++---- cmd/common.go | 21 +++++--- cmd/download.go | 14 +++-- cmd/output_format.go | 1 + cmd/purchase.go | 6 ++- cmd/root.go | 22 ++++---- cmd/search.go | 1 + main.go | 3 +- pkg/appstore/app_test.go | 1 + pkg/appstore/appstore_account_info.go | 1 + pkg/appstore/appstore_account_info_test.go | 1 + pkg/appstore/appstore_download.go | 10 ++-- pkg/appstore/appstore_download_test.go | 15 +++--- pkg/appstore/appstore_login.go | 4 +- pkg/appstore/appstore_login_test.go | 3 +- pkg/appstore/appstore_lookup.go | 6 ++- pkg/appstore/appstore_lookup_test.go | 1 + pkg/appstore/appstore_purchase.go | 7 ++- pkg/appstore/appstore_purchase_test.go | 4 +- pkg/appstore/appstore_replicate_sinf.go | 12 ++++- pkg/appstore/appstore_replicate_sinf_test.go | 11 ++-- pkg/appstore/appstore_revoke_test.go | 1 + pkg/appstore/appstore_search.go | 6 ++- pkg/appstore/appstore_search_test.go | 3 +- pkg/appstore/appstore_test.go | 3 +- pkg/http/client.go | 36 ++++++++++--- pkg/http/client_test.go | 13 ++--- pkg/http/http_test.go | 3 +- pkg/http/payload.go | 3 +- pkg/keychain/keychain_get_test.go | 1 + pkg/keychain/keychain_remove_test.go | 1 + pkg/keychain/keychain_set.go | 1 + pkg/keychain/keychain_set_test.go | 1 + pkg/keychain/keychain_test.go | 3 +- pkg/log/log_test.go | 3 +- pkg/log/logger.go | 3 +- pkg/log/writer.go | 29 ++++++++--- pkg/util/machine/machine.go | 12 +++-- pkg/util/machine/machine_test.go | 5 +- pkg/util/must_test.go | 1 + pkg/util/operatingsystem/operatingsystem.go | 51 +++++++++++++++---- .../operatingsystem/operatingsystem_test.go | 12 +++-- pkg/util/util_test.go | 3 +- 46 files changed, 295 insertions(+), 110 deletions(-) create mode 100644 .github/workflows/lint.yml create mode 100644 .golangci.yml diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 00000000..ea3d8d14 --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,21 @@ +name: Lint + +on: + pull_request: + branches: + - main + +jobs: + lint: + name: Lint + runs-on: macos-latest + steps: + - uses: actions/checkout@v2 + - uses: actions/setup-go@v3 + with: + go-version: '1.19.3' + cache: true + - run: go generate github.com/majd/ipatool/... + - uses: golangci/golangci-lint-action@v3 + with: + version: v1.52.2 diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 10307149..316701c4 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -8,7 +8,7 @@ on: jobs: run_tests: name: Run tests - runs-on: macos-latest + runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 - uses: actions/setup-go@v3 diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..be084d32 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,22 @@ +linters: + enable: + - errcheck + - govet + - goimports + - gofmt + - unused + - ginkgolinter + - godot + - godox + - importas + - nlreturn + - nonamedreturns + - prealloc + - predeclared + - tenv + - unconvert + - unparam + - usestdlibvars + - wastedassign + - wrapcheck + - wsl diff --git a/cmd/auth.go b/cmd/auth.go index 3a0c0b71..08868d3d 100644 --- a/cmd/auth.go +++ b/cmd/auth.go @@ -4,15 +4,16 @@ import ( "bufio" "errors" "fmt" + "os" + "strings" + "time" + "github.com/99designs/keyring" "github.com/avast/retry-go" "github.com/majd/ipatool/pkg/appstore" "github.com/majd/ipatool/pkg/util" "github.com/spf13/cobra" "golang.org/x/term" - "os" - "strings" - "time" ) func authCmd() *cobra.Command { @@ -34,10 +35,9 @@ func authCmd() *cobra.Command { func loginCmd() *cobra.Command { promptForAuthCode := func() (string, error) { - reader := bufio.NewReader(os.Stdin) - authCode, err := reader.ReadString('\n') + authCode, err := bufio.NewReader(os.Stdin).ReadString('\n') if err != nil { - return "", err + return "", fmt.Errorf("failed to read string: %w", err) } authCode = strings.Trim(authCode, "\n") @@ -46,9 +46,7 @@ func loginCmd() *cobra.Command { return authCode, nil } - var email string - var password string - var authCode string + var email, password, authCode string cmd := &cobra.Command{ Use: "login", @@ -72,6 +70,7 @@ func loginCmd() *cobra.Command { var lastErr error + // nolint:wrapcheck return retry.Do(func() error { if errors.Is(lastErr, appstore.ErrAuthCodeRequired) && interactive { dependencies.Logger.Log().Msg("enter 2FA code:") @@ -97,6 +96,7 @@ func loginCmd() *cobra.Command { if err != nil { if errors.Is(err, appstore.ErrAuthCodeRequired) && !interactive { dependencies.Logger.Log().Msg("2FA code is required; run the command again and supply a code using the `--auth-code` flag") + return nil } @@ -117,6 +117,7 @@ func loginCmd() *cobra.Command { retry.Attempts(2), retry.RetryIf(func(err error) bool { lastErr = err + return errors.Is(err, appstore.ErrAuthCodeRequired) }), ) @@ -132,6 +133,7 @@ func loginCmd() *cobra.Command { return cmd } +// nolint:wrapcheck func infoCmd() *cobra.Command { return &cobra.Command{ Use: "info", @@ -153,6 +155,7 @@ func infoCmd() *cobra.Command { } } +// nolint:wrapcheck func revokeCmd() *cobra.Command { return &cobra.Command{ Use: "revoke", @@ -164,6 +167,7 @@ func revokeCmd() *cobra.Command { } dependencies.Logger.Log().Bool("success", true).Send() + return nil }, } diff --git a/cmd/common.go b/cmd/common.go index a3ee285e..4fe46b3e 100644 --- a/cmd/common.go +++ b/cmd/common.go @@ -3,8 +3,13 @@ package cmd import ( "errors" "fmt" + "io" + "os" + "path/filepath" + "strings" + "github.com/99designs/keyring" - "github.com/juju/persistent-cookiejar" + cookiejar "github.com/juju/persistent-cookiejar" "github.com/majd/ipatool/pkg/appstore" "github.com/majd/ipatool/pkg/http" "github.com/majd/ipatool/pkg/keychain" @@ -16,10 +21,6 @@ import ( "github.com/spf13/cobra" "golang.org/x/exp/slices" "golang.org/x/term" - "io" - "os" - "path/filepath" - "strings" ) var dependencies = Dependencies{} @@ -38,23 +39,25 @@ type Dependencies struct { // newLogger returns a new logger instance. func newLogger(format OutputFormat, verbose bool) log.Logger { var writer io.Writer + switch format { case OutputFormatJSON: writer = zerolog.SyncWriter(os.Stdout) case OutputFormatText: writer = log.NewWriter() } + return log.NewLogger(log.Args{ Verbose: verbose, Writer: writer, - }) + }, + ) } // newCookieJar returns a new cookie jar instance. func newCookieJar(machine machine.Machine) http.CookieJar { - path := filepath.Join(machine.HomeDirectory(), ConfigDirectoryName, CookieJarFileName) return util.Must(cookiejar.New(&cookiejar.Options{ - Filename: path, + Filename: filepath.Join(machine.HomeDirectory(), ConfigDirectoryName, CookieJarFileName), })) } @@ -89,6 +92,7 @@ func newKeychain(machine machine.Machine, logger log.Logger, backendType keyring return password, nil }, })) + return keychain.New(keychain.Args{Keyring: ring}) } @@ -134,6 +138,7 @@ func initWithCommand(cmd *cobra.Command) { func createConfigDirectory() error { configDirectoryPath := filepath.Join(dependencies.Machine.HomeDirectory(), ConfigDirectoryName) _, err := dependencies.OS.Stat(configDirectoryPath) + if err != nil && dependencies.OS.IsNotExist(err) { err = dependencies.OS.MkdirAll(configDirectoryPath, 0700) if err != nil { diff --git a/cmd/download.go b/cmd/download.go index aaccd22e..89757f97 100644 --- a/cmd/download.go +++ b/cmd/download.go @@ -2,19 +2,23 @@ package cmd import ( "errors" + "os" + "time" + "github.com/99designs/keyring" "github.com/avast/retry-go" "github.com/majd/ipatool/pkg/appstore" "github.com/schollz/progressbar/v3" "github.com/spf13/cobra" - "os" - "time" ) +// nolint:wrapcheck func downloadCmd() *cobra.Command { - var acquireLicense bool - var outputPath string - var bundleID string + var ( + acquireLicense bool + outputPath string + bundleID string + ) cmd := &cobra.Command{ Use: "download", diff --git a/cmd/output_format.go b/cmd/output_format.go index 1ed9d5e4..79c05673 100644 --- a/cmd/output_format.go +++ b/cmd/output_format.go @@ -2,6 +2,7 @@ package cmd import ( "fmt" + "github.com/thediveo/enumflag/v2" ) diff --git a/cmd/purchase.go b/cmd/purchase.go index 37c1a682..1c02a1b2 100644 --- a/cmd/purchase.go +++ b/cmd/purchase.go @@ -2,13 +2,15 @@ package cmd import ( "errors" + "time" + "github.com/99designs/keyring" "github.com/avast/retry-go" "github.com/majd/ipatool/pkg/appstore" "github.com/spf13/cobra" - "time" ) +// nolint:wrapcheck func purchaseCmd() *cobra.Command { var bundleID string @@ -47,6 +49,7 @@ func purchaseCmd() *cobra.Command { } dependencies.Logger.Log().Bool("success", true).Send() + return nil }, retry.LastErrorOnly(true), @@ -55,6 +58,7 @@ func purchaseCmd() *cobra.Command { retry.Attempts(2), retry.RetryIf(func(err error) bool { lastErr = err + return errors.Is(err, appstore.ErrPasswordTokenExpired) }), ) diff --git a/cmd/root.go b/cmd/root.go index b9ba7ee9..3755c026 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -2,19 +2,22 @@ package cmd import ( "errors" + "reflect" + "github.com/majd/ipatool/pkg/appstore" "github.com/spf13/cobra" "github.com/thediveo/enumflag/v2" "golang.org/x/net/context" - "reflect" ) var version = "dev" func rootCmd() *cobra.Command { - var verbose bool - var nonInteractive bool - var format OutputFormat + var ( + verbose bool + nonInteractive bool + format OutputFormat + ) cmd := &cobra.Command{ Use: "ipatool", @@ -23,7 +26,7 @@ func rootCmd() *cobra.Command { SilenceUsage: true, Version: version, PersistentPreRun: func(cmd *cobra.Command, args []string) { - ctx := context.WithValue(context.Background(), "interactive", nonInteractive == false) + ctx := context.WithValue(context.Background(), "interactive", !nonInteractive) cmd.SetContext(ctx) initWithCommand(cmd) }, @@ -46,12 +49,11 @@ func rootCmd() *cobra.Command { } // Execute runs the program and returns the appropriate exit status code. -func Execute() (exitCode int) { +func Execute() int { cmd := rootCmd() err := cmd.Execute() - if err != nil { - exitCode = 1 + if err != nil { if reflect.ValueOf(dependencies).IsZero() { initWithCommand(cmd) } @@ -70,7 +72,9 @@ func Execute() (exitCode int) { Err(err). Bool("success", false). Send() + + return 1 } - return + return 0 } diff --git a/cmd/search.go b/cmd/search.go index dde1d01d..4af0d1d1 100644 --- a/cmd/search.go +++ b/cmd/search.go @@ -6,6 +6,7 @@ import ( "github.com/spf13/cobra" ) +// nolint:wrapcheck func searchCmd() *cobra.Command { var limit int64 diff --git a/main.go b/main.go index a7ea91fb..2c5b8490 100644 --- a/main.go +++ b/main.go @@ -1,8 +1,9 @@ package main import ( - "github.com/majd/ipatool/cmd" "os" + + "github.com/majd/ipatool/cmd" ) func main() { diff --git a/pkg/appstore/app_test.go b/pkg/appstore/app_test.go index ede12151..c4a1aa7b 100644 --- a/pkg/appstore/app_test.go +++ b/pkg/appstore/app_test.go @@ -3,6 +3,7 @@ package appstore import ( "bytes" "encoding/json" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/rs/zerolog" diff --git a/pkg/appstore/appstore_account_info.go b/pkg/appstore/appstore_account_info.go index 38d5c649..90ec0791 100644 --- a/pkg/appstore/appstore_account_info.go +++ b/pkg/appstore/appstore_account_info.go @@ -16,6 +16,7 @@ func (t *appstore) AccountInfo() (AccountInfoOutput, error) { } var acc Account + err = json.Unmarshal(data, &acc) if err != nil { return AccountInfoOutput{}, fmt.Errorf("failed to unmarshal json: %w", err) diff --git a/pkg/appstore/appstore_account_info_test.go b/pkg/appstore/appstore_account_info_test.go index 166e840b..2cf2d2d4 100644 --- a/pkg/appstore/appstore_account_info_test.go +++ b/pkg/appstore/appstore_account_info_test.go @@ -3,6 +3,7 @@ package appstore import ( "errors" "fmt" + "github.com/golang/mock/gomock" "github.com/majd/ipatool/pkg/keychain" . "github.com/onsi/ginkgo/v2" diff --git a/pkg/appstore/appstore_download.go b/pkg/appstore/appstore_download.go index 2c581c3c..81ac336f 100644 --- a/pkg/appstore/appstore_download.go +++ b/pkg/appstore/appstore_download.go @@ -4,12 +4,13 @@ import ( "archive/zip" "errors" "fmt" - "github.com/majd/ipatool/pkg/http" - "github.com/schollz/progressbar/v3" - "howett.net/plist" "io" "os" "strings" + + "github.com/majd/ipatool/pkg/http" + "github.com/schollz/progressbar/v3" + "howett.net/plist" ) var ( @@ -104,7 +105,7 @@ type downloadResult struct { Items []downloadItemResult `plist:"songList,omitempty"` } -func (t *appstore) downloadFile(src, dst string, progress *progressbar.ProgressBar) (err error) { +func (t *appstore) downloadFile(src, dst string, progress *progressbar.ProgressBar) error { req, err := t.httpClient.NewRequest("GET", src, nil) if err != nil { return fmt.Errorf("failed to create request: %w", err) @@ -139,6 +140,7 @@ func (t *appstore) downloadFile(src, dst string, progress *progressbar.ProgressB func (*appstore) downloadRequest(acc Account, app App, guid string) http.Request { host := fmt.Sprintf("%s-%s", PrivateAppStoreAPIDomainPrefixWithoutAuthCode, PrivateAppStoreAPIDomain) + return http.Request{ URL: fmt.Sprintf("https://%s%s?guid=%s", host, PrivateAppStoreAPIPathDownload, guid), Method: http.MethodPOST, diff --git a/pkg/appstore/appstore_download_test.go b/pkg/appstore/appstore_download_test.go index c7084134..baa8d01d 100644 --- a/pkg/appstore/appstore_download_test.go +++ b/pkg/appstore/appstore_download_test.go @@ -4,6 +4,11 @@ import ( "archive/zip" "errors" "fmt" + "io" + gohttp "net/http" + "os" + "strings" + "github.com/golang/mock/gomock" "github.com/majd/ipatool/pkg/http" "github.com/majd/ipatool/pkg/keychain" @@ -12,10 +17,6 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "howett.net/plist" - "io" - gohttp "net/http" - "os" - "strings" ) var _ = Describe("AppStore (Download)", func() { @@ -404,9 +405,7 @@ var _ = Describe("AppStore (Download)", func() { mockOS.EXPECT(). OpenFile(gomock.Any(), gomock.Any(), gomock.Any()). - DoAndReturn(func(name string, flag int, perm os.FileMode) (*os.File, error) { - return os.OpenFile(name, flag, perm) - }) + DoAndReturn(os.OpenFile) mockOS.EXPECT(). Stat(gomock.Any()). @@ -442,7 +441,7 @@ var _ = Describe("AppStore (Download)", func() { OutputPath: outputPath, }) Expect(err).ToNot(HaveOccurred()) - Expect(len(out.DestinationPath)).ToNot(Equal(0)) + Expect(out.DestinationPath).ToNot(BeEmpty()) }) }) }) diff --git a/pkg/appstore/appstore_login.go b/pkg/appstore/appstore_login.go index 296934d3..a4386f9b 100644 --- a/pkg/appstore/appstore_login.go +++ b/pkg/appstore/appstore_login.go @@ -4,8 +4,9 @@ import ( "encoding/json" "errors" "fmt" - "github.com/majd/ipatool/pkg/http" "strings" + + "github.com/majd/ipatool/pkg/http" ) var ( @@ -61,6 +62,7 @@ type loginResult struct { func (t *appstore) login(email, password, authCode, guid string, attempt int) (Account, error) { request := t.loginRequest(email, password, authCode, guid) res, err := t.loginClient.Send(request) + if err != nil { return Account{}, fmt.Errorf("request failed: %w", err) } diff --git a/pkg/appstore/appstore_login_test.go b/pkg/appstore/appstore_login_test.go index 0652e263..41c5e51c 100644 --- a/pkg/appstore/appstore_login_test.go +++ b/pkg/appstore/appstore_login_test.go @@ -4,13 +4,14 @@ import ( "encoding/json" "errors" "fmt" + "strings" + "github.com/golang/mock/gomock" "github.com/majd/ipatool/pkg/http" "github.com/majd/ipatool/pkg/keychain" "github.com/majd/ipatool/pkg/util/machine" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "strings" ) var _ = Describe("AppStore (Login)", func() { diff --git a/pkg/appstore/appstore_lookup.go b/pkg/appstore/appstore_lookup.go index 94564771..2cbf001a 100644 --- a/pkg/appstore/appstore_lookup.go +++ b/pkg/appstore/appstore_lookup.go @@ -3,8 +3,10 @@ package appstore import ( "errors" "fmt" - "github.com/majd/ipatool/pkg/http" + gohttp "net/http" "net/url" + + "github.com/majd/ipatool/pkg/http" ) type LookupInput struct { @@ -29,7 +31,7 @@ func (t *appstore) Lookup(input LookupInput) (LookupOutput, error) { return LookupOutput{}, fmt.Errorf("request failed: %w", err) } - if res.StatusCode != 200 { + if res.StatusCode != gohttp.StatusOK { return LookupOutput{}, NewErrorWithMetadata(errors.New("invalid response"), res) } diff --git a/pkg/appstore/appstore_lookup_test.go b/pkg/appstore/appstore_lookup_test.go index 5631fe0e..1eed29d7 100644 --- a/pkg/appstore/appstore_lookup_test.go +++ b/pkg/appstore/appstore_lookup_test.go @@ -2,6 +2,7 @@ package appstore import ( "errors" + "github.com/golang/mock/gomock" "github.com/majd/ipatool/pkg/http" . "github.com/onsi/ginkgo/v2" diff --git a/pkg/appstore/appstore_purchase.go b/pkg/appstore/appstore_purchase.go index 3b9b2a41..5fffb810 100644 --- a/pkg/appstore/appstore_purchase.go +++ b/pkg/appstore/appstore_purchase.go @@ -3,8 +3,10 @@ package appstore import ( "errors" "fmt" - "github.com/majd/ipatool/pkg/http" + gohttp "net/http" "strings" + + "github.com/majd/ipatool/pkg/http" ) var ( @@ -57,6 +59,7 @@ type purchaseResult struct { func (t *appstore) purchaseWithParams(acc Account, app App, guid string, pricingParameters string) error { req := t.purchaseRequest(acc, app, acc.StoreFront, guid, pricingParameters) res, err := t.purchaseClient.Send(req) + if err != nil { return fmt.Errorf("request failed: %w", err) } @@ -81,7 +84,7 @@ func (t *appstore) purchaseWithParams(acc Account, app App, guid string, pricing return NewErrorWithMetadata(errors.New("something went wrong"), res) } - if res.StatusCode == 500 { + if res.StatusCode == gohttp.StatusInternalServerError { return fmt.Errorf("license already exists") } diff --git a/pkg/appstore/appstore_purchase_test.go b/pkg/appstore/appstore_purchase_test.go index 8086a95e..2e6d7084 100644 --- a/pkg/appstore/appstore_purchase_test.go +++ b/pkg/appstore/appstore_purchase_test.go @@ -2,6 +2,7 @@ package appstore import ( "errors" + "github.com/golang/mock/gomock" "github.com/majd/ipatool/pkg/http" "github.com/majd/ipatool/pkg/keychain" @@ -298,8 +299,7 @@ type pricingParametersMatcher struct { } func (p pricingParametersMatcher) Matches(in interface{}) bool { - request := in.(http.Request) - return request.Payload.(*http.XMLPayload).Content["pricingParameters"] == p.pricingParameters + return in.(http.Request).Payload.(*http.XMLPayload).Content["pricingParameters"] == p.pricingParameters } func (p pricingParametersMatcher) String() string { diff --git a/pkg/appstore/appstore_replicate_sinf.go b/pkg/appstore/appstore_replicate_sinf.go index b6bae2be..da21dad8 100644 --- a/pkg/appstore/appstore_replicate_sinf.go +++ b/pkg/appstore/appstore_replicate_sinf.go @@ -5,12 +5,13 @@ import ( "bytes" "errors" "fmt" - "github.com/majd/ipatool/pkg/util" - "howett.net/plist" "io" "os" "path/filepath" "strings" + + "github.com/majd/ipatool/pkg/util" + "howett.net/plist" ) type Sinf struct { @@ -32,6 +33,7 @@ func (t *appstore) ReplicateSinf(input ReplicateSinfInput) error { tmpPath := fmt.Sprintf("%s.tmp", input.PackagePath) tmpFile, err := t.os.OpenFile(tmpPath, os.O_CREATE|os.O_WRONLY, 0644) + if err != nil { return fmt.Errorf("failed to open file: %w", err) } @@ -138,6 +140,7 @@ func (t *appstore) replicateZip(src *zip.ReadCloser, dst *zip.Writer) error { header := file.FileHeader dstFile, err := dst.CreateRaw(&header) + if err != nil { return fmt.Errorf("failed to create raw file: %w", err) } @@ -161,12 +164,14 @@ func (*appstore) readInfoPlist(reader *zip.ReadCloser) (*packageInfo, error) { data := new(bytes.Buffer) _, err = io.Copy(data, src) + if err != nil { return nil, fmt.Errorf("failed to copy data: %w", err) } var info packageInfo _, err = plist.Unmarshal(data.Bytes(), &info) + if err != nil { return nil, fmt.Errorf("failed to unmarshal data: %w", err) } @@ -188,11 +193,13 @@ func (*appstore) readManifestPlist(reader *zip.ReadCloser) (*packageManifest, er data := new(bytes.Buffer) _, err = io.Copy(data, src) + if err != nil { return nil, fmt.Errorf("failed to copy data: %w", err) } var manifest packageManifest + _, err = plist.Unmarshal(data.Bytes(), &manifest) if err != nil { return nil, fmt.Errorf("failed to unmarshal data: %w", err) @@ -211,6 +218,7 @@ func (*appstore) readBundleName(reader *zip.ReadCloser) (string, error) { for _, file := range reader.File { if strings.Contains(file.Name, ".app/Info.plist") && !strings.Contains(file.Name, "/Watch/") { bundleName = filepath.Base(strings.TrimSuffix(file.Name, ".app/Info.plist")) + break } } diff --git a/pkg/appstore/appstore_replicate_sinf_test.go b/pkg/appstore/appstore_replicate_sinf_test.go index 21fc0a4a..7295f52f 100644 --- a/pkg/appstore/appstore_replicate_sinf_test.go +++ b/pkg/appstore/appstore_replicate_sinf_test.go @@ -4,6 +4,8 @@ import ( "archive/zip" "errors" "fmt" + "os" + "github.com/golang/mock/gomock" "github.com/majd/ipatool/pkg/http" "github.com/majd/ipatool/pkg/keychain" @@ -12,7 +14,6 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "howett.net/plist" - "os" ) var _ = Describe("AppStore (ReplicateSinf)", func() { @@ -71,9 +72,7 @@ var _ = Describe("AppStore (ReplicateSinf)", func() { BeforeEach(func() { mockOS.EXPECT(). OpenFile(gomock.Any(), gomock.Any(), gomock.Any()). - DoAndReturn(func(name string, flag int, perm os.FileMode) (*os.File, error) { - return os.OpenFile(name, flag, perm) - }) + DoAndReturn(os.OpenFile) mockOS.EXPECT(). Remove(testFile.Name()). @@ -137,9 +136,7 @@ var _ = Describe("AppStore (ReplicateSinf)", func() { BeforeEach(func() { mockOS.EXPECT(). OpenFile(gomock.Any(), gomock.Any(), gomock.Any()). - DoAndReturn(func(name string, flag int, perm os.FileMode) (*os.File, error) { - return os.OpenFile(name, flag, perm) - }) + DoAndReturn(os.OpenFile) mockOS.EXPECT(). Remove(testFile.Name()). diff --git a/pkg/appstore/appstore_revoke_test.go b/pkg/appstore/appstore_revoke_test.go index 62c18b2b..ffe6a18e 100644 --- a/pkg/appstore/appstore_revoke_test.go +++ b/pkg/appstore/appstore_revoke_test.go @@ -2,6 +2,7 @@ package appstore import ( "errors" + "github.com/golang/mock/gomock" "github.com/majd/ipatool/pkg/keychain" . "github.com/onsi/ginkgo/v2" diff --git a/pkg/appstore/appstore_search.go b/pkg/appstore/appstore_search.go index 9154b85a..7b756c2e 100644 --- a/pkg/appstore/appstore_search.go +++ b/pkg/appstore/appstore_search.go @@ -3,9 +3,11 @@ package appstore import ( "errors" "fmt" - "github.com/majd/ipatool/pkg/http" + gohttp "net/http" "net/url" "strconv" + + "github.com/majd/ipatool/pkg/http" ) type SearchInput struct { @@ -32,7 +34,7 @@ func (t *appstore) Search(input SearchInput) (SearchOutput, error) { return SearchOutput{}, fmt.Errorf("request failed: %w", err) } - if res.StatusCode != 200 { + if res.StatusCode != gohttp.StatusOK { return SearchOutput{}, NewErrorWithMetadata(errors.New("request failed"), res) } diff --git a/pkg/appstore/appstore_search_test.go b/pkg/appstore/appstore_search_test.go index b2aac09f..eeb053d8 100644 --- a/pkg/appstore/appstore_search_test.go +++ b/pkg/appstore/appstore_search_test.go @@ -2,6 +2,7 @@ package appstore import ( "errors" + "github.com/golang/mock/gomock" "github.com/majd/ipatool/pkg/http" . "github.com/onsi/ginkgo/v2" @@ -64,7 +65,7 @@ var _ = Describe("AppStore (Search)", func() { }) Expect(err).ToNot(HaveOccurred()) Expect(out.Count).To(Equal(1)) - Expect(len(out.Results)).To(Equal(1)) + Expect(out.Results).To(HaveLen(1)) Expect(out.Results[0]).To(Equal(App{ ID: testID, BundleID: testBundleID, diff --git a/pkg/appstore/appstore_test.go b/pkg/appstore/appstore_test.go index d0b79b57..4604df32 100644 --- a/pkg/appstore/appstore_test.go +++ b/pkg/appstore/appstore_test.go @@ -1,9 +1,10 @@ package appstore import ( + "testing" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "testing" ) func TestAppStore(t *testing.T) { diff --git a/pkg/http/client.go b/pkg/http/client.go index 4913c53d..791fb48a 100644 --- a/pkg/http/client.go +++ b/pkg/http/client.go @@ -4,10 +4,11 @@ import ( "bytes" "encoding/json" "fmt" - "howett.net/plist" "io" "net/http" "strings" + + "howett.net/plist" ) //go:generate go run github.com/golang/mock/mockgen -source=client.go -destination=client_mock.go -package=http @@ -30,11 +31,17 @@ type AddHeaderTransport struct { T http.RoundTripper } -func (adt *AddHeaderTransport) RoundTrip(req *http.Request) (*http.Response, error) { +func (t *AddHeaderTransport) RoundTrip(req *http.Request) (*http.Response, error) { if req.Header.Get("User-Agent") == "" { req.Header.Set("User-Agent", DefaultUserAgent) } - return adt.T.RoundTrip(req) + + res, err := t.T.RoundTrip(req) + if err != nil { + return nil, fmt.Errorf("failed to make round trip: %w", err) + } + + return res, nil } func NewClient[R interface{}](args Args) Client[R] { @@ -49,8 +56,10 @@ func NewClient[R interface{}](args Args) Client[R] { } func (c *client[R]) Send(req Request) (Result[R], error) { - var data []byte - var err error + var ( + data []byte + err error + ) if req.Payload != nil { data, err = req.Payload.data() @@ -81,6 +90,7 @@ func (c *client[R]) Send(req Request) (Result[R], error) { if req.ResponseFormat == ResponseFormatJSON { return c.handleJSONResponse(res) } + if req.ResponseFormat == ResponseFormatXML { return c.handleXMLResponse(res) } @@ -89,11 +99,21 @@ func (c *client[R]) Send(req Request) (Result[R], error) { } func (c *client[R]) Do(req *http.Request) (*http.Response, error) { - return c.internalClient.Do(req) + res, err := c.internalClient.Do(req) + if err != nil { + return nil, fmt.Errorf("received error: %w", err) + } + + return res, nil } func (*client[R]) NewRequest(method, url string, body io.Reader) (*http.Request, error) { - return http.NewRequest(method, url, body) + req, err := http.NewRequest(method, url, body) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } + + return req, nil } func (c *client[R]) handleJSONResponse(res *http.Response) (Result[R], error) { @@ -103,6 +123,7 @@ func (c *client[R]) handleJSONResponse(res *http.Response) (Result[R], error) { } var data R + err = json.Unmarshal(body, &data) if err != nil { return Result[R]{}, fmt.Errorf("failed to unmarshal json: %w", err) @@ -121,6 +142,7 @@ func (c *client[R]) handleXMLResponse(res *http.Response) (Result[R], error) { } var data R + _, err = plist.Unmarshal(body, &data) if err != nil { return Result[R]{}, fmt.Errorf("failed to unmarshal xml: %w", err) diff --git a/pkg/http/client_test.go b/pkg/http/client_test.go index c168f3ba..c142ec99 100644 --- a/pkg/http/client_test.go +++ b/pkg/http/client_test.go @@ -2,11 +2,12 @@ package http import ( "errors" + "net/http" + "net/http/httptest" + "github.com/golang/mock/gomock" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "net/http" - "net/http/httptest" ) var _ = Describe("Client", Ordered, func() { @@ -50,7 +51,7 @@ var _ = Describe("Client", Ordered, func() { }) It("returns response", func() { - mockHandler = func(w http.ResponseWriter, r *http.Request) { + mockHandler = func(_w http.ResponseWriter, r *http.Request) { defer GinkgoRecover() Expect(r.Header.Get("User-Agent")).To(Equal(DefaultUserAgent)) } @@ -95,7 +96,7 @@ var _ = Describe("Client", Ordered, func() { }) It("decodes JSON response", func() { - mockHandler = func(w http.ResponseWriter, r *http.Request) { + mockHandler = func(w http.ResponseWriter, _r *http.Request) { w.Header().Add("Content-Type", "application/json") _, err := w.Write([]byte("{\"foo\":\"bar\"}")) Expect(err).ToNot(HaveOccurred()) @@ -123,7 +124,7 @@ var _ = Describe("Client", Ordered, func() { }) It("decodes XML response", func() { - mockHandler = func(w http.ResponseWriter, r *http.Request) { + mockHandler = func(w http.ResponseWriter, _r *http.Request) { w.Header().Add("Content-Type", "application/xml") _, err := w.Write([]byte("foobar")) Expect(err).ToNot(HaveOccurred()) @@ -143,7 +144,7 @@ var _ = Describe("Client", Ordered, func() { }) It("returns error when content type is not supported", func() { - mockHandler = func(w http.ResponseWriter, r *http.Request) { + mockHandler = func(w http.ResponseWriter, _r *http.Request) { w.Header().Add("Content-Type", "application/xyz") } diff --git a/pkg/http/http_test.go b/pkg/http/http_test.go index 4b13bb42..b2f7f01c 100644 --- a/pkg/http/http_test.go +++ b/pkg/http/http_test.go @@ -1,9 +1,10 @@ package http import ( + "testing" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "testing" ) func TestHTTP(t *testing.T) { diff --git a/pkg/http/payload.go b/pkg/http/payload.go index efaf9048..691dec93 100644 --- a/pkg/http/payload.go +++ b/pkg/http/payload.go @@ -3,9 +3,10 @@ package http import ( "bytes" "fmt" - "howett.net/plist" "net/url" "strconv" + + "howett.net/plist" ) type Payload interface { diff --git a/pkg/keychain/keychain_get_test.go b/pkg/keychain/keychain_get_test.go index bbbc23d9..3a567994 100644 --- a/pkg/keychain/keychain_get_test.go +++ b/pkg/keychain/keychain_get_test.go @@ -2,6 +2,7 @@ package keychain import ( "errors" + "github.com/99designs/keyring" "github.com/golang/mock/gomock" . "github.com/onsi/ginkgo/v2" diff --git a/pkg/keychain/keychain_remove_test.go b/pkg/keychain/keychain_remove_test.go index 21f7f780..7b0213eb 100644 --- a/pkg/keychain/keychain_remove_test.go +++ b/pkg/keychain/keychain_remove_test.go @@ -2,6 +2,7 @@ package keychain import ( "errors" + "github.com/golang/mock/gomock" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" diff --git a/pkg/keychain/keychain_set.go b/pkg/keychain/keychain_set.go index 5e5b07b9..1da26676 100644 --- a/pkg/keychain/keychain_set.go +++ b/pkg/keychain/keychain_set.go @@ -2,6 +2,7 @@ package keychain import ( "fmt" + "github.com/99designs/keyring" ) diff --git a/pkg/keychain/keychain_set_test.go b/pkg/keychain/keychain_set_test.go index 9c004569..c7ce57e7 100644 --- a/pkg/keychain/keychain_set_test.go +++ b/pkg/keychain/keychain_set_test.go @@ -2,6 +2,7 @@ package keychain import ( "errors" + "github.com/99designs/keyring" "github.com/golang/mock/gomock" . "github.com/onsi/ginkgo/v2" diff --git a/pkg/keychain/keychain_test.go b/pkg/keychain/keychain_test.go index 1f141c84..9e3b8f1c 100644 --- a/pkg/keychain/keychain_test.go +++ b/pkg/keychain/keychain_test.go @@ -1,9 +1,10 @@ package keychain import ( + "testing" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "testing" ) func TestKeychain(t *testing.T) { diff --git a/pkg/log/log_test.go b/pkg/log/log_test.go index 8db4f794..d86557e8 100644 --- a/pkg/log/log_test.go +++ b/pkg/log/log_test.go @@ -1,9 +1,10 @@ package log import ( + "testing" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "testing" ) func TestLog(t *testing.T) { diff --git a/pkg/log/logger.go b/pkg/log/logger.go index dd8e7558..ddf611d9 100644 --- a/pkg/log/logger.go +++ b/pkg/log/logger.go @@ -1,10 +1,11 @@ package log import ( + "io" + "github.com/rs/zerolog" "github.com/rs/zerolog/log" "github.com/rs/zerolog/pkgerrors" - "io" ) //go:generate go run github.com/golang/mock/mockgen -source=logger.go -destination=logger_mock.go -package log diff --git a/pkg/log/writer.go b/pkg/log/writer.go index 936f56d3..0749057d 100644 --- a/pkg/log/writer.go +++ b/pkg/log/writer.go @@ -1,9 +1,11 @@ package log import ( - "github.com/rs/zerolog" + "fmt" "io" "os" + + "github.com/rs/zerolog" ) //go:generate go run github.com/golang/mock/mockgen -source=writer.go -destination=writer_mock.go -package log @@ -24,16 +26,31 @@ func NewWriter() Writer { } } -func (l *writer) Write(p []byte) (n int, err error) { - return l.stdOutWriter.Write(p) +func (l *writer) Write(p []byte) (int, error) { + n, err := l.stdOutWriter.Write(p) + if err != nil { + return 0, fmt.Errorf("failed to write data: %w", err) + } + + return n, nil } -func (l *writer) WriteLevel(level zerolog.Level, p []byte) (n int, err error) { +func (l *writer) WriteLevel(level zerolog.Level, p []byte) (int, error) { switch level { case zerolog.DebugLevel, zerolog.InfoLevel, zerolog.WarnLevel: - return l.stdOutWriter.Write(p) + n, err := l.stdOutWriter.Write(p) + if err != nil { + return 0, fmt.Errorf("failed to write data: %w", err) + } + + return n, nil case zerolog.ErrorLevel: - return l.stdErrWriter.Write(p) + n, err := l.stdErrWriter.Write(p) + if err != nil { + return 0, fmt.Errorf("failed to write data: %w", err) + } + + return n, nil default: return len(p), nil } diff --git a/pkg/util/machine/machine.go b/pkg/util/machine/machine.go index 723b8f67..745f5155 100644 --- a/pkg/util/machine/machine.go +++ b/pkg/util/machine/machine.go @@ -2,11 +2,12 @@ package machine import ( "fmt" - "github.com/majd/ipatool/pkg/util/operatingsystem" - "golang.org/x/term" "net" "path/filepath" "runtime" + + "github.com/majd/ipatool/pkg/util/operatingsystem" + "golang.org/x/term" ) //go:generate go run github.com/golang/mock/mockgen -source=machine.go -destination=machine_mock.go -package machine @@ -59,5 +60,10 @@ func (m *machine) HomeDirectory() string { } func (*machine) ReadPassword(fd int) ([]byte, error) { - return term.ReadPassword(fd) + data, err := term.ReadPassword(fd) + if err != nil { + return nil, fmt.Errorf("failed to read password: %w", err) + } + + return data, nil } diff --git a/pkg/util/machine/machine_test.go b/pkg/util/machine/machine_test.go index 7785be77..c8ba76b2 100644 --- a/pkg/util/machine/machine_test.go +++ b/pkg/util/machine/machine_test.go @@ -1,12 +1,13 @@ package machine import ( + "syscall" + "testing" + "github.com/golang/mock/gomock" "github.com/majd/ipatool/pkg/util/operatingsystem" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "syscall" - "testing" ) func TestMachine(t *testing.T) { diff --git a/pkg/util/must_test.go b/pkg/util/must_test.go index ffc3015e..fc69b2c2 100644 --- a/pkg/util/must_test.go +++ b/pkg/util/must_test.go @@ -2,6 +2,7 @@ package util import ( "errors" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) diff --git a/pkg/util/operatingsystem/operatingsystem.go b/pkg/util/operatingsystem/operatingsystem.go index b3ff5ee3..60a316fe 100644 --- a/pkg/util/operatingsystem/operatingsystem.go +++ b/pkg/util/operatingsystem/operatingsystem.go @@ -1,6 +1,9 @@ package operatingsystem -import "os" +import ( + "fmt" + "os" +) //go:generate go run github.com/golang/mock/mockgen -source=operatingsystem.go -destination=operatingsystem_mock.go -package operatingsystem type OperatingSystem interface { @@ -11,7 +14,7 @@ type OperatingSystem interface { Remove(name string) error IsNotExist(err error) bool MkdirAll(path string, perm os.FileMode) error - Rename(old, new string) error + Rename(oldPath, newPath string) error } type operatingSystem struct{} @@ -25,19 +28,39 @@ func (operatingSystem) Getenv(key string) string { } func (operatingSystem) Stat(name string) (os.FileInfo, error) { - return os.Stat(name) + info, err := os.Stat(name) + if err != nil { + return nil, fmt.Errorf("failed to describe file '%s': %w", name, err) + } + + return info, nil } func (operatingSystem) Getwd() (string, error) { - return os.Getwd() + wd, err := os.Getwd() + if err != nil { + return "", fmt.Errorf("failed to get current directory: %w", err) + } + + return wd, nil } func (operatingSystem) OpenFile(name string, flag int, perm os.FileMode) (*os.File, error) { - return os.OpenFile(name, flag, perm) + file, err := os.OpenFile(name, flag, perm) + if err != nil { + return nil, fmt.Errorf("failed to open file '%s': %w", name, err) + } + + return file, nil } func (operatingSystem) Remove(name string) error { - return os.Remove(name) + err := os.Remove(name) + if err != nil { + return fmt.Errorf("failed to remove file '%s': %w", name, err) + } + + return nil } func (operatingSystem) IsNotExist(err error) bool { @@ -45,9 +68,19 @@ func (operatingSystem) IsNotExist(err error) bool { } func (operatingSystem) MkdirAll(path string, perm os.FileMode) error { - return os.MkdirAll(path, perm) + err := os.MkdirAll(path, perm) + if err != nil { + return fmt.Errorf("failed to create directory '%s': %w", path, err) + } + + return nil } -func (operatingSystem) Rename(old, new string) error { - return os.Rename(old, new) +func (operatingSystem) Rename(oldPath, newPath string) error { + err := os.Rename(oldPath, newPath) + if err != nil { + return fmt.Errorf("failed to rename '%s' to '%s': %w", oldPath, newPath, err) + } + + return nil } diff --git a/pkg/util/operatingsystem/operatingsystem_test.go b/pkg/util/operatingsystem/operatingsystem_test.go index 9acdb27f..aff276a4 100644 --- a/pkg/util/operatingsystem/operatingsystem_test.go +++ b/pkg/util/operatingsystem/operatingsystem_test.go @@ -1,15 +1,17 @@ package operatingsystem import ( + "errors" "fmt" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" "io/fs" "math/rand" "os" "path" "testing" "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" ) func TestOS(t *testing.T) { @@ -68,7 +70,7 @@ var _ = Describe("OperatingSystem", func() { Expect(err).ToNot(HaveOccurred()) _, err = sut.Stat(file.Name()) - Expect(os.IsNotExist(err)).To(BeTrue()) + Expect(os.IsNotExist(errors.Unwrap(err))).To(BeTrue()) }) It("renames file", func() { @@ -76,7 +78,9 @@ var _ = Describe("OperatingSystem", func() { newPath := fmt.Sprintf("%s/%d", os.TempDir(), r.Intn(100)) err := sut.Rename(file.Name(), newPath) - defer sut.Remove(newPath) + defer func() { + _ = sut.Remove(newPath) + }() Expect(err).ToNot(HaveOccurred()) }) diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index 6fc0a26b..4c6eb3f2 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -1,9 +1,10 @@ package util import ( + "testing" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "testing" ) func TestUtil(t *testing.T) {