[MAINT] Create custom golangci-lint to integrate tfproviderlint

[deiga]

Before the change?

• Not able to use tfproviderlint as part of golangci-lint

After the change?

• Capable to use tfproviderlint as part of golangci-lint
• Capable of adding new checks for the terraform provider schema
  • Example: tools/tfproviderlint/checks/L001/L001.go
• Add 3 custom linter rules
  • L001: Instructs to use ValidateDiagFunc instead of ValidateFunc
  • L002: Instructs to use Context-aware CRUD functions
  • L003: Instructs to not call any CRUD functions directly
• Enable all tfproviderlint checks, which seem sensible and don't require fixes right now
• Add make lint-new to run stricter linters on only code since main
• Refactor ci.yaml workflow, to run linters in parallel using golangci-lint-action
  • This enables inline linter complaints in PRs

Pull request checklist

• Schema migrations have been created if needed (example)
• Tests for the changes have been added (for bug fixes / features)
• Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

• Yes
• No

---

[github-actions]

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

[stevehipwell]

I'm sorry but I'm not currently onboard with this change as it stands. It's a long way away from the principle of least surprise and changes standard patterns I'd expect to see in a TF provider codebase. Is there a reason why we can't run tfproviderlint as a separate job so at least the complexity is kept away from the other more standard linting?

[deiga]

@stevehipwell I don't think I understand your objection. The PR forces no changes on existing code and provides a stable way, with oversight, to have all new changes linted with a stricter ruleset.

It basically already does exactly what you're asking for: running the new linter rules in a separate job. While also providing seamless integration into the linting landscape I use.

These changes would reduce a big part of review comments we currently have to type out manually.

Could you elaborate what the problem with these changes is, so that I could understand better?

[stevehipwell]

@deiga you've changed how CI and the local default linting works, neither of which is desirable. I'm keen to have advanced checks but not at the cost of slowdown for local development or changing the fast feedback loop and understandability of the CI job.

Why not run tfproviderlint as a standalone?

[deiga]

Why not run tfproviderlint as a standalone?

• We'd need to use a custom golangci-lint or revive (not investigated) to enable adding our own linter rules.
  • And to get the GitHub Actions tie-in
  • And IMO the custom rules here are the most important part of this change

you've changed how CI and the local default linting works

In some ways, yes. The linter is now fully dependent on make, or at least for the necessary targets to have been run once. After that, one could call the ./bin/custom-gcl directly. BUT it doesn't really change how default linting works IMO. It still uses the same logic for linting and the same ruleset. It just runs a few more rules now.

but not at the cost of slowdown for local development or changing the fast feedback loop and understandability of the CI job.

I haven't measured it, but there should not be any major slowdown locally. Only the first run after each of these forces a recompile: updated golangci-lint version or updated go (non-test) files in ./tools/. Neither of which happen too often, unless you're developing new linters.

If there is an impact on the understandability of the CI job, that has not been intentional and needs to be addressed. Please point to this degradation