search

pulumi / pulumi-terraform-bridge

A library allowing Terraform providers to be bridged into Pulumi.
Apache License 2.0
192 stars 45 forks source link

Ensure linter catches variable capture in closures #951

Open t0yv0 opened 1 year ago

t0yv0 commented 1 year ago

Thanks to @abhinav for pointers:

For loops, go vet  does this by default. The analysis is implemented here: https://pkg.go.dev/golang.org/x/tools@v0.7.0/go/analysis/passes/loopclosure
In the default list of linters for go vet: https://cs.opensource.google/go/go/+/refs/tags/go1.20.3:src/cmd/vet/main.go;l=58

We should install the tooling that checks this. It's going to be time saver.

abhinav commented 1 year ago

Echoing Slack: I believe you already have the appropriate tooling in the repository. go test runs go vet by default, which includes this linter by default. golangci-lint also includes govet as a linter and it's enabled: https://github.com/pulumi/pulumi-terraform-bridge/blob/master/.golangci.yml#L14

t0yv0 commented 1 year ago

cd pf && golangci-lint run -c ../.golangci.yml is silent about this bug.

make lint also - that one is calling my strange script to call golangci-lint for every go.mod in the repository but it might have been missing redirecting golangci-lint to the right config (-c ../.golangci.yml).

t0yv0 commented 1 year ago

go vet doesn't catch it either. Interesting.

t0yv0 commented 1 year ago

original: https://github.com/pulumi/pulumi-terraform-bridge/pull/950

t0yv0 commented 1 year ago

This finds it (thanks @AaronFriel)

go install github.com/fatanugraha/noloopclosure/cmd/noloopclosure@latest
cd pf && ~/go/bin/noloopclosure ./...

At v0.1.2

But not when installed into golangci-lint as a plugin for some reason.