diff --git a/.golangci-complexity.yml b/.golangci-complexity.yml new file mode 100644 index 0000000000..566456a2e2 --- /dev/null +++ b/.golangci-complexity.yml @@ -0,0 +1,64 @@ +# Copyright (c) The OpenTofu Authors +# SPDX-License-Identifier: MPL-2.0 +# Copyright (c) 2023 HashiCorp, Inc. +# SPDX-License-Identifier: MPL-2.0 + +# This is a temporary variant of .golangci.yml to support our work on improving +# existing functions to pass the code complexity lint rules: +# https://github.com/opentofu/opentofu/issues/2325 +# +# If you are working on the code complexity improvement project then you can +# check our progress on the complexity-related lints by running the linters +# as follows: +# golangci-lint run -c .golangci-complexity.yml +# +# Many existing functions were failing multiple linters at once at the start +# of this project, and for as long as that remains true it might be helpful +# to add the --uniq-by-line=false option to review all of the failures at +# once. +# +# This file should be deleted at the same time as we re-enable the five +# complexity-related linters in .golangci.yml. + +run: + timeout: 30m + +linters-settings: + funlen: + lines: 100 + statements: 50 + ignore-comments: true + + nolintlint: + require-explanation: true + require-specific: true + + cyclop: + max-complexity: 20 + + gocognit: + min-complexity: 50 + + nestif: + min-complexity: 6 + +issues: + exclude-rules: + - path: (.+)_test.go + linters: + - funlen + - dupl + - revive + - path: (.+)_test.go + text: "ST1003" + - path: (.+)_test.go + text: "var-naming: don't use underscores in Go names" + +linters: + disable-all: true + enable: + - cyclop + - funlen + - gocognit + - gocyclo + - nestif diff --git a/.golangci.yml b/.golangci.yml index 7d1f1f9347..4917ffb1f8 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -67,7 +67,7 @@ linters: - asciicheck - bidichk - bodyclose - - cyclop + #- cyclop # (refer to .golangci-complexity.yml) - dupl - durationcheck - errcheck @@ -76,14 +76,14 @@ linters: - exhaustive - exportloopref - forbidigo - - funlen + #- funlen # (refer to .golangci-complexity.yml) - gocheckcompilerdirectives - gochecknoglobals - gochecknoinits - - gocognit + #- gocognit # (refer to .golangci-complexity.yml) - goconst - gocritic - - gocyclo + #- gocyclo # (refer to .golangci-complexity.yml) - goimports #- gomoddirectives Disabled while we deal with the HCL fork - gomodguard @@ -98,7 +98,7 @@ linters: - mnd - musttag - nakedret - - nestif + #- nestif # (refer to .golangci-complexity.yml) - nilerr - nilnil - noctx diff --git a/internal/backend/local/backend_local.go b/internal/backend/local/backend_local.go index 2b913df415..07b12d508f 100644 --- a/internal/backend/local/backend_local.go +++ b/internal/backend/local/backend_local.go @@ -41,7 +41,6 @@ func (b *Local) LocalRun(ctx context.Context, op *backend.Operation) (*backend.L return lr, stateMgr, diags } -//nolint:funlen // Historical function predates our complexity rules func (b *Local) localRun(ctx context.Context, op *backend.Operation) (*backend.LocalRun, *configload.Snapshot, statemgr.Full, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics diff --git a/internal/backend/remote/backend_context.go b/internal/backend/remote/backend_context.go index d90ce9abb8..2e2cfd6703 100644 --- a/internal/backend/remote/backend_context.go +++ b/internal/backend/remote/backend_context.go @@ -23,8 +23,6 @@ import ( ) // Context implements backend.Local. -// -//nolint:funlen,nestif // Historical function predates our complexity rules func (b *Remote) LocalRun(_ context.Context, op *backend.Operation) (*backend.LocalRun, statemgr.Full, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics ret := &backend.LocalRun{ diff --git a/internal/cloud/backend_context.go b/internal/cloud/backend_context.go index a0901a26c9..3ce4e2c3a1 100644 --- a/internal/cloud/backend_context.go +++ b/internal/cloud/backend_context.go @@ -23,8 +23,6 @@ import ( ) // LocalRun implements backend.Local -// -//nolint:funlen,nestif // Historical function predates our complexity rules func (b *Cloud) LocalRun(_ context.Context, op *backend.Operation) (*backend.LocalRun, statemgr.Full, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics ret := &backend.LocalRun{ diff --git a/internal/command/graph.go b/internal/command/graph.go index de8637c7eb..23a4a83892 100644 --- a/internal/command/graph.go +++ b/internal/command/graph.go @@ -81,7 +81,6 @@ func (c *GraphCommand) Run(args []string) int { // Load the backend var b backend.Enhanced - //nolint: nestif // This is inspired by apply:PrepareBackend if lp, ok := planFile.Local(); ok { plan, planErr := lp.ReadPlan() if planErr != nil { diff --git a/internal/command/test.go b/internal/command/test.go index 5e208ddeff..462c036517 100644 --- a/internal/command/test.go +++ b/internal/command/test.go @@ -507,7 +507,6 @@ func (runner *TestFileRunner) ExecuteTestFile(ctx context.Context, file *modulet } } -//nolint:funlen // Historical function predates our complexity rules func (runner *TestFileRunner) ExecuteTestRun(ctx context.Context, run *moduletest.Run, file *moduletest.File, state *states.State, config *configs.Config) (*states.State, bool) { log.Printf("[TRACE] TestFileRunner: executing run block %s/%s", file.Name, run.Name) diff --git a/internal/command/views/json/diagnostic.go b/internal/command/views/json/diagnostic.go index 55c96904a9..e06e97aa3e 100644 --- a/internal/command/views/json/diagnostic.go +++ b/internal/command/views/json/diagnostic.go @@ -133,7 +133,7 @@ type DiagnosticFunctionCall struct { // NewDiagnostic takes a tfdiags.Diagnostic and a map of configuration sources, // and returns a Diagnostic struct. -func NewDiagnostic(diag tfdiags.Diagnostic, sources map[string]*hcl.File) *Diagnostic { //nolint:funlen,gocognit,gocyclo,cyclop // TODO(1818): Refactor this function. +func NewDiagnostic(diag tfdiags.Diagnostic, sources map[string]*hcl.File) *Diagnostic { var sev string switch diag.Severity() { case tfdiags.Error: diff --git a/internal/configs/module_call.go b/internal/configs/module_call.go index 789c501ae3..c9b1b7289e 100644 --- a/internal/configs/module_call.go +++ b/internal/configs/module_call.go @@ -165,7 +165,6 @@ func (mc *ModuleCall) decodeStaticSource(eval *StaticEvaluator) hcl.Diagnostics // Decode source field diags := eval.DecodeExpression(mc.Source, StaticIdentifier{Module: eval.call.addr, Subject: fmt.Sprintf("module.%s.source", mc.Name), DeclRange: mc.Source.Range()}, &mc.SourceAddrRaw) - //nolint:nestif // Keeping this similar to the original decode logic for easy review if !diags.HasErrors() { // NOTE: This code was originally executed as part of decodeModuleBlock and is now deferred until we have the config merged and static context built var err error diff --git a/internal/configs/provider_validation.go b/internal/configs/provider_validation.go index db61776988..f39315c9db 100644 --- a/internal/configs/provider_validation.go +++ b/internal/configs/provider_validation.go @@ -872,8 +872,6 @@ func providerIterationIdenticalWarning(blockType, target string, sourceExpr, ins } // Compares two for_each statements to see if they are "identical". This is on a best-effort basis to help prevent foot-guns. -// -//nolint:funlen,gocognit,gocyclo,cyclop // just a lot of branches func providerIterationIdentical(a, b hcl.Expression) bool { if a == nil && b == nil { return true diff --git a/internal/encryption/base.go b/internal/encryption/base.go index bfb071a570..4d8ff7e525 100644 --- a/internal/encryption/base.go +++ b/internal/encryption/base.go @@ -136,7 +136,6 @@ func (base *baseEncryption) decrypt(data []byte, validator func([]byte) error) ( inputData := basedata{} err := json.Unmarshal(data, &inputData) - //nolint:nestif // bugger off if len(inputData.Version) == 0 || err != nil { // Not a valid payload, might be already decrypted verr := validator(data) diff --git a/internal/moduletest/run.go b/internal/moduletest/run.go index c8ffb08055..3ab8691492 100644 --- a/internal/moduletest/run.go +++ b/internal/moduletest/run.go @@ -130,7 +130,7 @@ func (run *Run) GetReferences() ([]*addrs.Reference, tfdiags.Diagnostics) { // diagnostics were generated by custom conditions. OpenTofu adds the // addrs.CheckRule that generated each diagnostic to the diagnostic itself so we // can tell which diagnostics can be expected. -func (run *Run) ValidateExpectedFailures(expectedFailures addrs.Map[addrs.Referenceable, bool], sourceRanges addrs.Map[addrs.Referenceable, tfdiags.SourceRange], originals tfdiags.Diagnostics) tfdiags.Diagnostics { //nolint: funlen,gocognit,cyclop // legacy code causes this to trigger, we actually reduced the statements +func (run *Run) ValidateExpectedFailures(expectedFailures addrs.Map[addrs.Referenceable, bool], sourceRanges addrs.Map[addrs.Referenceable, tfdiags.SourceRange], originals tfdiags.Diagnostics) tfdiags.Diagnostics { var diags tfdiags.Diagnostics for _, diag := range originals { if rule, ok := addrs.DiagnosticOriginatesFromCheckRule(diag); ok { diff --git a/internal/tofu/context_input.go b/internal/tofu/context_input.go index 89cfc348f7..fde8bd009c 100644 --- a/internal/tofu/context_input.go +++ b/internal/tofu/context_input.go @@ -36,8 +36,6 @@ import ( // any other Context method with a different config, because the aforementioned // modified internal state won't match. Again, this is an architectural wart // that we'll hopefully resolve in future. -// -//nolint:cyclop,funlen,gocognit,nestif // Historical function predates our complexity rules func (c *Context) Input(ctx context.Context, config *configs.Config, mode InputMode) tfdiags.Diagnostics { // This function used to be responsible for more than it is now, so its // interface is more general than its current functionality requires. diff --git a/internal/tofu/context_plan.go b/internal/tofu/context_plan.go index 586c0c26d5..ed77cbfd51 100644 --- a/internal/tofu/context_plan.go +++ b/internal/tofu/context_plan.go @@ -124,8 +124,6 @@ type PlanOpts struct { // planned so far, which is not safe to apply but could potentially be used // by the UI layer to give extra context to support understanding of the // returned error messages. -// -//nolint:cyclop,funlen // Historical function predates our complexity rules func (c *Context) Plan(ctx context.Context, config *configs.Config, prevRunState *states.State, opts *PlanOpts) (*plans.Plan, tfdiags.Diagnostics) { defer c.acquireRun("plan")() var diags tfdiags.Diagnostics diff --git a/internal/tofu/node_resource_abstract_instance.go b/internal/tofu/node_resource_abstract_instance.go index 44d758c415..ea6cab300a 100644 --- a/internal/tofu/node_resource_abstract_instance.go +++ b/internal/tofu/node_resource_abstract_instance.go @@ -118,7 +118,6 @@ func (n *NodeAbstractResourceInstance) resolveProvider(ctx EvalContext, hasExpan useStateFallback := false - //nolint:nestif // complexity if n.ResolvedProvider.KeyExact != nil { // Pass through from state n.ResolvedProviderKey = n.ResolvedProvider.KeyExact