Skip to content

Discuss and Implement Go Static Analysis Tooling #96

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.

Sign up for GitHub

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

Jump to bottom
Closed
bflad opened this issue Aug 12, 2021 · 2 comments · Fixed by #255
Closed

Discuss and Implement Go Static Analysis Tooling #96

bflad opened this issue Aug 12, 2021 · 2 comments · Fixed by #255
Assignees
@bflad
Labels
tech-debt Issues tracking technical debt that we're carrying.
Milestone
v0.6.0

Comments

@bflad
Copy link
Contributor

bflad commented Aug 12, 2021

Module version

v0.2.0

Use-cases

The Go ecosystem has a plethora of static analysis tooling, which can help catch code bugs, enhance quality, and automate style decisions. For example, revive (the spiritual successor to golint) can require Go documentation comments on exported types and functions, which seems desirable given the purpose of this codebase.

Proposal

I have worked successfully with golangci-lint via GitHub Actions (and before that gometalinter), which brings many static analysis tools under one tool and configuration. This is open for discussion though!
@bflad bflad added the enhancement New feature or request label Aug 12, 2021
@bflad bflad mentioned this issue Aug 12, 2021
Closed
@paddycarver
Copy link
Contributor

I could've sworn we were running golangci-lint on PRs and commits, but I appear to be mistaken. I'd be in favor of turning it on.

@paddycarver paddycarver added tech-debt Issues tracking technical debt that we're carrying. and removed enhancement New feature or request labels Sep 21, 2021
@bflad bflad mentioned this issue Sep 27, 2021
Closed
bflad added a commit that referenced this issue Jan 28, 2022
@bflad
Migrate from CircleCI to GitHub Actions for tests and releases, enabl…
b3c03f7 
…e linting

Reference: #96
Reference: #97

Fixes the previous linting issues:

```console
❯ golangci-lint run ./...
internal/reflect/helpers.go:47:20: `getStructTags` - `ctx` is unused (unparam)
func getStructTags(ctx context.Context, in reflect.Value, path *tftypes.AttributePath) (map[string]int, error) {
                   ^
internal/reflect/pointer.go:54:27: `pointerSafeZeroValue` - `ctx` is unused (unparam)
func pointerSafeZeroValue(ctx context.Context, target reflect.Value) reflect.Value {
                          ^
types/float64.go:13:22: `float64Validate` - `ctx` is unused (unparam)
func float64Validate(ctx context.Context, in tftypes.Value, path *tftypes.AttributePath) diag.Diagnostics {
                     ^
types/int64.go:13:20: `int64Validate` - `ctx` is unused (unparam)
func int64Validate(ctx context.Context, in tftypes.Value, path *tftypes.AttributePath) diag.Diagnostics {
                   ^
tfsdk/plan.go:210:26: `(Plan).pathExists` - `ctx` is unused (unparam)
func (p Plan) pathExists(ctx context.Context, path *tftypes.AttributePath) (bool, diag.Diagnostics) {
                         ^
tfsdk/serve.go:70:43: `(*server).cancelRegisteredContexts` - `ctx` is unused (unparam)
func (s *server) cancelRegisteredContexts(ctx context.Context) {
                                          ^
tfsdk/state.go:219:27: `(State).pathExists` - `ctx` is unused (unparam)
func (s State) pathExists(ctx context.Context, path *tftypes.AttributePath) (bool, diag.Diagnostics) {
                          ^
tfsdk/tftypes_value.go:16:24: `createParentValue` - `ctx` is unused (unparam)
func createParentValue(ctx context.Context, parentPath *tftypes.AttributePath, parentType tftypes.Type, childValue interface{}) (tftypes.Value, diag.Diagnostics) {
                       ^
tfsdk/tftypes_value.go:61:23: `upsertChildValue` - `ctx` is unused (unparam)
func upsertChildValue(ctx context.Context, parentPath *tftypes.AttributePath, parentValue tftypes.Value, childStep tftypes.AttributePathStep, childValue tftypes.Value) (tftypes.Value, diag.Diagnostics) {
                      ^
tfsdk/value_as_test.go:109:16: S1040: type assertion to the same type: target already has type attr.Value (gosimple)
        if !val.Equal(target.(attr.Value)) {
                      ^
tfsdk/attribute_plan_modification.go:310:9: SA9003: empty branch (staticcheck)
        } else if resp.RequiresReplace {
```

All testing and linting workflows verified as successful via:

```shell
act -s GITHUB_TOKEN=$GITHUB_TOKEN pull_request
```
@bflad bflad mentioned this issue Jan 28, 2022
Merged
@bflad bflad added this to the v0.6.0 milestone Jan 28, 2022
bflad added a commit that referenced this issue Jan 28, 2022
@bflad
Migrate from CircleCI to GitHub Actions for tests and releases, enabl…
5dba37d 
…e linting (#255)

Reference: #96
Reference: #97

Fixes the previous linting issues:

```console
❯ golangci-lint run ./...
internal/reflect/helpers.go:47:20: `getStructTags` - `ctx` is unused (unparam)
func getStructTags(ctx context.Context, in reflect.Value, path *tftypes.AttributePath) (map[string]int, error) {
                   ^
internal/reflect/pointer.go:54:27: `pointerSafeZeroValue` - `ctx` is unused (unparam)
func pointerSafeZeroValue(ctx context.Context, target reflect.Value) reflect.Value {
                          ^
types/float64.go:13:22: `float64Validate` - `ctx` is unused (unparam)
func float64Validate(ctx context.Context, in tftypes.Value, path *tftypes.AttributePath) diag.Diagnostics {
                     ^
types/int64.go:13:20: `int64Validate` - `ctx` is unused (unparam)
func int64Validate(ctx context.Context, in tftypes.Value, path *tftypes.AttributePath) diag.Diagnostics {
                   ^
tfsdk/plan.go:210:26: `(Plan).pathExists` - `ctx` is unused (unparam)
func (p Plan) pathExists(ctx context.Context, path *tftypes.AttributePath) (bool, diag.Diagnostics) {
                         ^
tfsdk/serve.go:70:43: `(*server).cancelRegisteredContexts` - `ctx` is unused (unparam)
func (s *server) cancelRegisteredContexts(ctx context.Context) {
                                          ^
tfsdk/state.go:219:27: `(State).pathExists` - `ctx` is unused (unparam)
func (s State) pathExists(ctx context.Context, path *tftypes.AttributePath) (bool, diag.Diagnostics) {
                          ^
tfsdk/tftypes_value.go:16:24: `createParentValue` - `ctx` is unused (unparam)
func createParentValue(ctx context.Context, parentPath *tftypes.AttributePath, parentType tftypes.Type, childValue interface{}) (tftypes.Value, diag.Diagnostics) {
                       ^
tfsdk/tftypes_value.go:61:23: `upsertChildValue` - `ctx` is unused (unparam)
func upsertChildValue(ctx context.Context, parentPath *tftypes.AttributePath, parentValue tftypes.Value, childStep tftypes.AttributePathStep, childValue tftypes.Value) (tftypes.Value, diag.Diagnostics) {
                      ^
tfsdk/value_as_test.go:109:16: S1040: type assertion to the same type: target already has type attr.Value (gosimple)
        if !val.Equal(target.(attr.Value)) {
                      ^
tfsdk/attribute_plan_modification.go:310:9: SA9003: empty branch (staticcheck)
        } else if resp.RequiresReplace {
```

All testing and linting workflows verified as successful via:

```shell
act -s GITHUB_TOKEN=$GITHUB_TOKEN pull_request
```
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@bflad bflad

Labels
tech-debt Issues tracking technical debt that we're carrying.
Projects
None yet
Milestone
v0.6.0
2 participants
@paddycarver @bflad