This function was too long for our function length linting rule, so we'll
split each of the type kinds with special handling into their own function
and thus the main typeString function is just a straightforward dispatch
table with only one statement per case.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
This value was too long for our function length lint rule, and factoring
out the printing of null values makes this more balanced with how we're
already handling unknown values and sensitive values so that the main
body of FormatValue is focused on the normal value printing case.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
The functions in these files are for handling older state snapshot formats
that current OpenTofu versions never generate, and so it's highly unlikely
that we'll ever make substantial changes to these functions.
Therefore it's unjustified to risk reworking it to pass linting rules, and
so we'll add nolint comments instead. Our priority is to make as few
changes as possible to these functions, to minimize the risk of regressing
a upgrade paths that are exercised very infrequently.
(For context, state version 4 has been current ever since Terraform
v0.12.0, and so the earlier versions are long obsolete.)
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
These packages are frozen copies of old code from much older versions of
the product that are preserved to keep the state storage backends working
until we decide on a way to get them out of this codebase entirely.
Therefore the only potential future change to this code is to delete it
once it's no longer needed. It would not be worth the risk or time
investment to rework these to meet our strict complexity linting rules.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
The code in this package is all snapshot from the Go codebase in older
versions, inlined here to allow OpenTofu's cidr-calculation-related
functions to preserve their original behavior despite upstream changing
the parsing rules in a breaking way.
This code is intentionally modified as little as possible from the upstream
code it was derived from. We are imposing on ourselves considerably
stricter style conventions than the Go project follows and so we need
to disable various linters for this package to allow this code to remain
written in the Go idiomatic style, rather than in OpenTofu's stricter
local style.
In particular, we've chosen to prohibit ourselves from using named return
values or package-global variables, despite those both being typical in the
standard library and in other codebases.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
We're intending to gradually improve all of the existing functions that
fail these checks as a separate project from other work, because fixing
for these particular lint rules tends to be too invasive to be safe or
sensible to combine with other work.
Therefore we'll temporarily disable these lints from the main lint run
and add a separate .golangci-complexity.yml that we can use to track our
progress towards eliminating those lint failures without continuing to
litter the code with nolint comments in the meantime.
This also removes all of the existing nolint comments for these linters so
that we can start fresh and review each one as part of our improvement
project.
We'll re-enable these linters (and remove .golangci-complexity.yml) once
each example has either been rewritten to pass the checks or we've
concluded that further decomposition would hurt readability and so added
"nolint" comments back in so we can review whether our lint rules are too
strict once we've got a bunch of examples to consider together.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
We currently have a set of aspirational linting rules in the project's
golangci-lint configuration, but this codebase was derived from a much
older codebase that was not written under those lint rules and so we made
the pragmatic decision that only code that has changed since the addition
of the lint rules is subjected to those lint rules.
That approach aims to make the compromise of encouraging us to gradually
improve code "while we're in the area" working on other changes, while
avoiding the need for a huge retrofit of existing code.
However, that compromise seems to be less appropriate for the subset of
linting rules related to code complexity in particular. That category of
rules typically imposes some arbitrary limit on a qualitative metric that
the linting tool can measure. These particular rules therefore have a
relatively broad scope and tend to require very disruptive changes to
existing code in order to resolve them.
This proposal aims to find a pragmatic path that will lead to a codebase
that _does_ conform to the complexity lint rules in the long run, but to
treat those improvements as a separate project in their own right rather
than as something we aim to gradually improve as part of other work.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
By building a map from module path to snapshotModule we can avoid repeatedly scanning the set of modules for each call to Open.
Signed-off-by: Jon Johnson <jon.johnson@chainguard.dev>
Using the word "iteration" to describe what count and for_each do tends to
confuse people because it sounds like explicit control flow rather than
just dynamically declaring multiple objects.
Elsewhere in the codebase we refer to this idea as "expansion" so this is
a rename for consistency with that and to remove the confusing terminology.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
Previously we made a very generic suggestion to use -target to split a
change into two parts as a workaround for the fact that count and for_each
must be known during planning. That works, but we didn't have enough
information available to tell the operator exactly what to target and so
anyone who is not an expert on the configuration they're working with tends
to get stuck unable to figure out exactly what they need to do.
The new -exclude option gives us an opportunity to do better here: we tend
to know for which object we're currently evaluating count or for_each, and
so we can mention that object directly in the error message when if we
recommend to use -exclude instead of -target.
Not all objects that support count/for_each will necessarily be directly
targetable, so we can still potentially recommend -target when we're
dealing with one of those objects. For example, as of this commit that
is true for for_each in a provider block, because there is not currently
any syntax for specifying a provider configuration as an addrs.Targetable.
Perhaps we'll introduce such a thing in the future, but that's outside the
scope of this change that's primarily focused on improving the messaging
for resource and module count/for_each.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
The original proposal called for the state snapshot loader to accept a
resource instance with both an instance-level provider instance address
and a resource-level provider instance address.
The final implementation does follow that specification, but it also emits
a warning in that case to draw attention to the inconsistency.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
The original proposal called for the state snapshot writer to generate a
resource-level provider property if all of the instances of the resource
had the same provider instance address, regardless of what that address
actually is.
The actual implementation instead chose to generate the resource-level
property only if none of the instances of the resource refer to a provider
instance that has an instance key.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
Kind of odd code smell, but the only alternative I could think of was a
panic. Would rather ensure this requirement at compile time instead.
Signed-off-by: Christian Mesh <christianmesh1@gmail.com>
In the main language runtime input variables have both a "raw" value as
provided by the caller and a "finalized" value that has been
type-converted, default-attributes-inserted, and validated.
Unfortunately the "early eval" codepath is essentially a reimplementation
of the language runtime in terms of data available in the static
configuration, and it previously wasn't properly emulating the finalization
of input variable values and was thus incorrectly exposing the "raw"
values into a module instead of the "finalized" values.
Since we are already in the v1.9 prerelease period significant refactoring
is too risky, and so this just copies the most important transformations
from the language runtime into the early eval runtime. We hope to find a
more sustainable way to implement this in the future, but that will likely
require refactoring of both the early eval codepath _and_ the traditional
language runtime, and so that work needs to begin early in a minor release
period.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
Signed-off-by: Christian Mesh <christianmesh1@gmail.com>
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
Co-authored-by: Martin Atkins <mart@degeneration.co.uk>
