search

hashicorp / terraform-provider-aws

The AWS Provider enables Terraform to manage AWS resources.
https://registry.terraform.io/providers/hashicorp/aws
Mozilla Public License 2.0
9.85k stars 9.19k forks source link

tests/provider: Fix and enable wrapcheck linter #15892

Closed bflad closed 1 year ago

bflad commented 4 years ago

Community Note

Description

golangci-lint version 1.32.0 introduced the wrapcheck linter, which by its own description:

A simple Go linter to check that errors from external packages are wrapped during return to help identify the error source during debugging.

This is great because this is a common review item where we prefer to return error message context about when/where an error is occurring. This linter is a little more focused than the goerr113 linter, which is also in golangci-lint but is problematic from our perspective due to:

Also, any call of errors.New() and fmt.Errorf() methods are reported except the calls used to initialise package-level variables and the fmt.Errorf() calls wrapping the other errors.

We do this regularly as this project is not intended to be used as a library.

Example Reports

aws/data_source_aws_ami.go:207:10: error returned from external package is unwrapped (wrapcheck)
        return err
               ^
aws/data_source_aws_ami.go:292:10: error returned from external package is unwrapped (wrapcheck)
        return err
               ^
aws/data_source_aws_ami.go:295:10: error returned from external package is unwrapped (wrapcheck)
        return err
               ^
aws/data_source_aws_ami.go:298:10: error returned from external package is unwrapped (wrapcheck)
        return err
               ^
aws/data_source_aws_ami_ids.go:73:10: error returned from external package is unwrapped (wrapcheck)
        return err
               ^
aws/data_source_aws_api_gateway_api_key.go:61:10: error returned from external package is unwrapped (wrapcheck)
        return err
               ^
aws/data_source_aws_api_gateway_rest_api.go:159:9: error returned from external package is unwrapped (wrapcheck)
    return err
           ^

Affected Resources

TBD

Definition of Done

mattburgess commented 3 years ago

I'm gonna pick this up if folks don't mind. I initially got lulled into a false sense of security by running golangci-lint run --enable wrapcheck ./aws. It turns out that running golangci-lint --max-issues-per-linter 0 run ./aws is far more illuminating as to the scale of the fixes required :smile:

Here's the full list of data sources and resources affected; their acceptance tests are also affected. Some ancilliary files also show hits which will probably get fixed up in the final PR in the series which will enable the linter:

bflad commented 3 years ago

@mattburgess before you get too far, we should work through your existing pull requests (ensuring they all still pass acceptance testing) and discuss the intended fixes here.

bflad commented 3 years ago

Here's some of the missing Contributing Guide documentation: https://github.com/hashicorp/terraform-provider-aws/pull/16794 👍

github-actions[bot] commented 1 year ago

Marking this issue as stale due to inactivity. This helps our maintainers find and focus on the active issues. If this issue receives no comments in the next 30 days it will automatically be closed. Maintainers can also remove the stale label.

If this issue was automatically closed and you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thank you!

github-actions[bot] commented 1 year ago

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.