Fix: Ensure constant format strings in fmt and printf calls

Go 1.24 introduces stricter checks for format string validation.
This commit fixes instances where non-constant format strings were
used in calls to functions like `fmt.Errorf`, `fmt.Printf`, and similar.

Changes include:
- Replacing dynamically constructed strings passed as format strings
with constant format strings.
- Refactoring `fmt.Sprintf` calls to ensure the format string matches
the number of arguments provided.
- Simplifying redundant formatting and ensuring compliance with Go
1.24's stricter `vet` tool checks.

This update ensures compatibility with Go 1.24 and prevents potential
runtime errors caused by misinterpreted dynamic format strings.

Resolves #2389

Signed-off-by: Mikel Olasagasti Uranga <mikel@olasagasti.info>
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
This commit is contained in:
Mikel Olasagasti Uranga 2025-01-19 12:08:27 +01:00 committed by Martin Atkins
parent ecd4dc5c61
commit abad8d400c
23 changed files with 54 additions and 53 deletions

View File

@ -117,7 +117,7 @@ func TestParseAbsOutputValueStr(t *testing.T) {
t.Run(input, func(t *testing.T) {
got, diags := ParseAbsOutputValueStr(input)
for _, problem := range deep.Equal(got, tc.want) {
t.Errorf(problem)
t.Errorf("%s", problem)
}
if len(diags) > 0 {
gotErr := diags.Err().Error()

View File

@ -116,7 +116,7 @@ func TestParseRefInTestingScope(t *testing.T) {
}
for _, problem := range deep.Equal(got, test.Want) {
t.Errorf(problem)
t.Errorf("%s", problem)
}
})
}
@ -931,7 +931,7 @@ func TestParseRef(t *testing.T) {
}
for _, problem := range deep.Equal(got, test.Want) {
t.Errorf(problem)
t.Errorf("%s", problem)
}
})
}

View File

@ -386,7 +386,7 @@ func TestParseTarget(t *testing.T) {
}
for _, problem := range deep.Equal(got, test.Want) {
t.Errorf(problem)
t.Errorf("%s", problem)
}
})
}

View File

@ -434,7 +434,7 @@ func TestParseProviderSourceStr(t *testing.T) {
for name, test := range tests {
got, diags := ParseProviderSourceString(name)
for _, problem := range deep.Equal(got, test.Want) {
t.Errorf(problem)
t.Errorf("%s", problem)
}
if len(diags) > 0 {
if test.Err == false {

View File

@ -340,7 +340,7 @@ func (b *Remote) costEstimate(stopCtx, cancelCtx context.Context, op *backend.Op
b.CLI.Output("\n------------------------------------------------------------------------")
return nil
case tfe.CostEstimateCanceled:
return fmt.Errorf(msgPrefix + " canceled.")
return fmt.Errorf("%s canceled.", msgPrefix)
default:
return fmt.Errorf("Unknown or unexpected cost estimate state: %s", ce.Status)
}
@ -417,15 +417,15 @@ func (b *Remote) checkPolicy(stopCtx, cancelCtx context.Context, op *backend.Ope
}
continue
case tfe.PolicyErrored:
return fmt.Errorf(msgPrefix + " errored.")
return fmt.Errorf("%s errored.", msgPrefix)
case tfe.PolicyHardFailed:
return fmt.Errorf(msgPrefix + " hard failed.")
return fmt.Errorf("%s hard failed.", msgPrefix)
case tfe.PolicySoftFailed:
runUrl := fmt.Sprintf(runHeader, b.hostname, b.organization, op.Workspace, r.ID)
if op.Type == backend.OperationTypePlan || op.UIOut == nil || op.UIIn == nil ||
!pc.Actions.IsOverridable || !pc.Permissions.CanOverride {
return fmt.Errorf(msgPrefix + " soft failed.\n" + runUrl)
return fmt.Errorf("%s soft failed.\n%s", msgPrefix, runUrl)
}
if op.AutoApprove {

View File

@ -312,7 +312,7 @@ func (b *Cloud) costEstimate(stopCtx, cancelCtx context.Context, op *backend.Ope
b.CLI.Output("\n------------------------------------------------------------------------")
return nil
case tfe.CostEstimateCanceled:
return fmt.Errorf(msgPrefix + " canceled.")
return fmt.Errorf("%s canceled.", msgPrefix)
default:
return fmt.Errorf("Unknown or unexpected cost estimate state: %s", ce.Status)
}
@ -389,15 +389,15 @@ func (b *Cloud) checkPolicy(stopCtx, cancelCtx context.Context, op *backend.Oper
}
continue
case tfe.PolicyErrored:
return fmt.Errorf(msgPrefix + " errored.")
return fmt.Errorf("%s errored.", msgPrefix)
case tfe.PolicyHardFailed:
return fmt.Errorf(msgPrefix + " hard failed.")
return fmt.Errorf("%s hard failed.", msgPrefix)
case tfe.PolicySoftFailed:
runUrl := fmt.Sprintf(runHeader, b.hostname, b.organization, op.Workspace, r.ID)
if op.Type == backend.OperationTypePlan || op.UIOut == nil || op.UIIn == nil ||
!pc.Actions.IsOverridable || !pc.Permissions.CanOverride {
return fmt.Errorf(msgPrefix + " soft failed.\n" + runUrl)
return fmt.Errorf("%s soft failed.\n%s", msgPrefix, runUrl)
}
if op.AutoApprove {
@ -414,9 +414,7 @@ func (b *Cloud) checkPolicy(stopCtx, cancelCtx context.Context, op *backend.Oper
}
err = b.confirm(stopCtx, op, opts, r, "override")
if err != nil && err != errRunOverridden {
return fmt.Errorf(
fmt.Sprintf("Failed to override: %s\n%s\n", err.Error(), runUrl),
)
return fmt.Errorf("Failed to override: %s\n%s\n", err.Error(), runUrl)
}
if err != errRunOverridden {

View File

@ -261,7 +261,7 @@ func skipWithoutRemoteTerraformVersion(t *testing.T) {
version := tfversion.Version
baseVersion, err := goversion.NewVersion(version)
if err != nil {
t.Fatalf(fmt.Sprintf("Error instantiating go-version for %s", version))
t.Fatalf("Error instantiating go-version for %s", version)
}
opts := &tfe.AdminTerraformVersionsListOptions{
ListOptions: tfe.ListOptions{

View File

@ -122,14 +122,14 @@ func TestEncryptionFlow(t *testing.T) {
err := os.Rename(src, dst)
if err != nil {
t.Fatalf(err.Error())
t.Fatalf("%s", err.Error())
}
fn()
err = os.Rename(dst, src)
if err != nil {
t.Fatalf(err.Error())
t.Fatalf("%s", err.Error())
}
}

View File

@ -7158,12 +7158,12 @@ func runTestCases(t *testing.T, testCases map[string]testCase) {
beforeDynamicValue, err := plans.NewDynamicValue(beforeVal, ty)
if err != nil {
t.Fatalf("failed to create dynamic before value: " + err.Error())
t.Fatalf("failed to create dynamic before value: %s", err.Error())
}
afterDynamicValue, err := plans.NewDynamicValue(afterVal, ty)
if err != nil {
t.Fatalf("failed to create dynamic after value: " + err.Error())
t.Fatalf("failed to create dynamic after value: %s", err.Error())
}
src := &plans.ResourceInstanceChangeSrc{
@ -7204,7 +7204,7 @@ func runTestCases(t *testing.T, testCases map[string]testCase) {
}
jsonchanges, err := jsonplan.MarshalResourceChanges([]*plans.ResourceInstanceChangeSrc{src}, tfschemas)
if err != nil {
t.Errorf("failed to marshal resource changes: " + err.Error())
t.Errorf("failed to marshal resource changes: %s", err.Error())
return
}

View File

@ -244,7 +244,7 @@ func (m *Meta) selectWorkspace(b backend.Backend) error {
log.Printf("[TRACE] Meta.selectWorkspace: selecting the new TFC workspace requested by the user (%s)", name)
return m.SetWorkspace(name)
} else {
return fmt.Errorf(strings.TrimSpace(errBackendNoExistingWorkspaces))
return fmt.Errorf("%s", strings.TrimSpace(errBackendNoExistingWorkspaces))
}
}

View File

@ -570,7 +570,7 @@ func (m *Meta) backendMigrateTFC(opts *backendMigrateOpts) error {
if sourceTFC && !destinationTFC {
// From Terraform Cloud to another backend. This is not yet implemented, and
// we recommend people to use the TFC API.
return fmt.Errorf(strings.TrimSpace(errTFCMigrateNotYetImplemented))
return fmt.Errorf("%s", strings.TrimSpace(errTFCMigrateNotYetImplemented))
}
// Everything below, by the above two conditionals, now assumes that the

View File

@ -147,10 +147,12 @@ func (v *OperationHuman) PlanNextStep(planPath string, genConfigPath string) {
if genConfigPath != "" {
v.view.streams.Printf(
"%s",
format.WordWrap(
"\n"+strings.TrimSpace(fmt.Sprintf(planHeaderGenConfig, genConfigPath)),
v.view.outputColumns(),
) + "\n")
)+"\n",
)
}
if planPath == "" {
@ -162,10 +164,11 @@ func (v *OperationHuman) PlanNextStep(planPath string, genConfigPath string) {
)
} else {
v.view.streams.Printf(
"%s",
format.WordWrap(
"\n"+strings.TrimSpace(fmt.Sprintf(planHeaderYesOutput, planPath, planPath)),
v.view.outputColumns(),
) + "\n",
)+"\n",
)
}
}

View File

@ -52,7 +52,7 @@ func TestLoadConfigWithSnapshot(t *testing.T) {
}
problems := deep.Equal(wantModuleDirs, gotModuleDirs)
for _, problem := range problems {
t.Errorf(problem)
t.Errorf("%s", problem)
}
if len(problems) > 0 {
return

View File

@ -234,7 +234,7 @@ func TestSaveLocksToFile(t *testing.T) {
fileInfo, err := os.Stat(filename)
if err != nil {
t.Fatalf(err.Error())
t.Fatalf("%s", err.Error())
}
if mode := fileInfo.Mode(); mode&0111 != 0 {
t.Fatalf("Expected lock file to be non-executable: %o", mode)
@ -242,7 +242,7 @@ func TestSaveLocksToFile(t *testing.T) {
gotContentBytes, err := os.ReadFile(filename)
if err != nil {
t.Fatalf(err.Error())
t.Fatalf("%s", err.Error())
}
gotContent := string(gotContentBytes)
wantContent := `# This file is maintained automatically by "tofu init".

View File

@ -202,7 +202,7 @@ func (c Config) Build() (keyprovider.KeyProvider, keyprovider.KeyMeta, error) {
out += "\n" + diag.Summary() + " : " + diag.Detail()
}
return nil, nil, fmt.Errorf(out)
return nil, nil, fmt.Errorf("%s", out)
}
return &keyProvider{

View File

@ -513,7 +513,7 @@ var SumFunc = function.New(&function.Spec{
ty := args[0].Type()
if !ty.IsListType() && !ty.IsSetType() && !ty.IsTupleType() {
return cty.NilVal, function.NewArgErrorf(0, fmt.Sprintf("argument must be list, set, or tuple. Received %s", ty.FriendlyName()))
return cty.NilVal, function.NewArgErrorf(0, "argument must be list, set, or tuple. Received %s", ty.FriendlyName())
}
if !args[0].IsWhollyKnown() {

View File

@ -177,7 +177,7 @@ var RsaDecryptFunc = function.New(&function.Spec{
default:
errStr = fmt.Sprintf("invalid private key: %s", e)
}
return cty.UnknownVal(cty.String), function.NewArgErrorf(1, errStr)
return cty.UnknownVal(cty.String), function.NewArgErrorf(1, "%s", errStr)
}
privateKey, ok := rawKey.(*rsa.PrivateKey)
if !ok {

View File

@ -81,7 +81,7 @@ func testApplyDiff(t *testing.T,
}
if !cmp.Equal(expectedState, newState, equateEmpty, typeComparer, valueComparer) {
t.Fatalf(cmp.Diff(expectedState, newState, equateEmpty, typeComparer, valueComparer))
t.Fatalf("state diff (-expected +got):\n%s", cmp.Diff(expectedState, newState, equateEmpty, typeComparer, valueComparer))
}
}

View File

@ -391,7 +391,7 @@ func (p *GRPCProvider) ReadResource(r providers.ReadResourceRequest) (resp provi
resSchema, ok := schema.ResourceTypes[r.TypeName]
if !ok {
resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("unknown resource type " + r.TypeName))
resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("unknown resource type %s", r.TypeName))
return resp
}

View File

@ -380,7 +380,7 @@ func (p *GRPCProvider) ReadResource(r providers.ReadResourceRequest) (resp provi
resSchema, ok := schema.ResourceTypes[r.TypeName]
if !ok {
resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("unknown resource type " + r.TypeName))
resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("unknown resource type %s", r.TypeName))
return resp
}

View File

@ -76,7 +76,7 @@ func TestContext2Apply_createBeforeDestroy_deposedKeyPreApply(t *testing.T) {
if diags.HasErrors() {
t.Fatalf("diags: %s", diags.Err())
} else {
t.Logf(legacyDiffComparisonString(plan.Changes))
t.Logf("%s", legacyDiffComparisonString(plan.Changes))
}
_, diags = ctx.Apply(context.Background(), plan, m)

View File

@ -822,7 +822,7 @@ func TestContext2Apply_providerAliasConfigure(t *testing.T) {
if diags.HasErrors() {
t.Fatalf("diags: %s", diags.Err())
} else {
t.Logf(legacyDiffComparisonString(plan.Changes))
t.Logf("%s", legacyDiffComparisonString(plan.Changes))
}
// Configure to record calls AFTER Plan above
@ -959,7 +959,7 @@ func TestContext2Apply_createBeforeDestroy(t *testing.T) {
if diags.HasErrors() {
t.Fatalf("diags: %s", diags.Err())
} else {
t.Logf(legacyDiffComparisonString(plan.Changes))
t.Logf("%s", legacyDiffComparisonString(plan.Changes))
}
state, diags = ctx.Apply(context.Background(), plan, m)
@ -1041,7 +1041,7 @@ func TestContext2Apply_createBeforeDestroyUpdate(t *testing.T) {
if diags.HasErrors() {
t.Fatalf("diags: %s", diags.Err())
} else {
t.Logf(legacyDiffComparisonString(plan.Changes))
t.Logf("%s", legacyDiffComparisonString(plan.Changes))
}
state, diags = ctx.Apply(context.Background(), plan, m)
@ -1100,7 +1100,7 @@ func TestContext2Apply_createBeforeDestroy_dependsNonCBD(t *testing.T) {
if diags.HasErrors() {
t.Fatalf("diags: %s", diags.Err())
} else {
t.Logf(legacyDiffComparisonString(plan.Changes))
t.Logf("%s", legacyDiffComparisonString(plan.Changes))
}
state, diags = ctx.Apply(context.Background(), plan, m)
@ -1165,7 +1165,7 @@ func TestContext2Apply_createBeforeDestroy_hook(t *testing.T) {
if diags.HasErrors() {
t.Fatalf("diags: %s", diags.Err())
} else {
t.Logf(legacyDiffComparisonString(plan.Changes))
t.Logf("%s", legacyDiffComparisonString(plan.Changes))
}
if _, diags := ctx.Apply(context.Background(), plan, m); diags.HasErrors() {
@ -1245,7 +1245,7 @@ func TestContext2Apply_createBeforeDestroy_deposedCount(t *testing.T) {
if diags.HasErrors() {
t.Fatalf("diags: %s", diags.Err())
} else {
t.Logf(legacyDiffComparisonString(plan.Changes))
t.Logf("%s", legacyDiffComparisonString(plan.Changes))
}
state, diags = ctx.Apply(context.Background(), plan, m)
@ -1307,7 +1307,7 @@ func TestContext2Apply_createBeforeDestroy_deposedOnly(t *testing.T) {
if diags.HasErrors() {
t.Fatalf("diags: %s", diags.Err())
} else {
t.Logf(legacyDiffComparisonString(plan.Changes))
t.Logf("%s", legacyDiffComparisonString(plan.Changes))
}
state, diags = ctx.Apply(context.Background(), plan, m)
@ -1646,7 +1646,7 @@ func TestContext2Apply_dataBasic(t *testing.T) {
if diags.HasErrors() {
t.Fatalf("diags: %s", diags.Err())
} else {
t.Logf(legacyDiffComparisonString(plan.Changes))
t.Logf("%s", legacyDiffComparisonString(plan.Changes))
}
state, diags := ctx.Apply(context.Background(), plan, m)
@ -1702,7 +1702,7 @@ func TestContext2Apply_destroyData(t *testing.T) {
if diags.HasErrors() {
t.Fatalf("diags: %s", diags.Err())
} else {
t.Logf(legacyDiffComparisonString(plan.Changes))
t.Logf("%s", legacyDiffComparisonString(plan.Changes))
}
newState, diags := ctx.Apply(context.Background(), plan, m)
@ -1769,7 +1769,7 @@ func TestContext2Apply_destroySkipsCBD(t *testing.T) {
if diags.HasErrors() {
t.Fatalf("diags: %s", diags.Err())
} else {
t.Logf(legacyDiffComparisonString(plan.Changes))
t.Logf("%s", legacyDiffComparisonString(plan.Changes))
}
if _, diags := ctx.Apply(context.Background(), plan, m); diags.HasErrors() {
@ -3795,7 +3795,7 @@ func TestContext2Apply_multiVarComprehensive(t *testing.T) {
t.Run("config for "+key, func(t *testing.T) {
for _, problem := range deep.Equal(got, want) {
t.Errorf(problem)
t.Errorf("%s", problem)
}
})
}
@ -8391,7 +8391,7 @@ func TestContext2Apply_ignoreChangesCreate(t *testing.T) {
if diags.HasErrors() {
t.Fatalf("diags: %s", diags.Err())
} else {
t.Logf(legacyDiffComparisonString(plan.Changes))
t.Logf("%s", legacyDiffComparisonString(plan.Changes))
}
state, diags := ctx.Apply(context.Background(), plan, m)
@ -8537,7 +8537,7 @@ func TestContext2Apply_ignoreChangesAll(t *testing.T) {
logDiagnostics(t, diags)
t.Fatal("plan failed")
} else {
t.Logf(legacyDiffComparisonString(plan.Changes))
t.Logf("%s", legacyDiffComparisonString(plan.Changes))
}
state, diags := ctx.Apply(context.Background(), plan, m)
@ -12798,7 +12798,7 @@ func TestContext2Apply_dataSensitive(t *testing.T) {
if diags.HasErrors() {
t.Fatalf("diags: %s", diags.Err())
} else {
t.Logf(legacyDiffComparisonString(plan.Changes))
t.Logf("%s", legacyDiffComparisonString(plan.Changes))
}
state, diags := ctx.Apply(context.Background(), plan, m)

View File

@ -1397,7 +1397,7 @@ func TestContext2Plan_preventDestroy_bad(t *testing.T) {
expectedErr := "aws_instance.foo has lifecycle.prevent_destroy"
if !strings.Contains(fmt.Sprintf("%s", err), expectedErr) {
if plan != nil {
t.Logf(legacyDiffComparisonString(plan.Changes))
t.Logf("%s", legacyDiffComparisonString(plan.Changes))
}
t.Fatalf("expected err would contain %q\nerr: %s", expectedErr, err)
}
@ -1478,7 +1478,7 @@ func TestContext2Plan_preventDestroy_countBad(t *testing.T) {
expectedErr := "aws_instance.foo[1] has lifecycle.prevent_destroy"
if !strings.Contains(fmt.Sprintf("%s", err), expectedErr) {
if plan != nil {
t.Logf(legacyDiffComparisonString(plan.Changes))
t.Logf("%s", legacyDiffComparisonString(plan.Changes))
}
t.Fatalf("expected err would contain %q\nerr: %s", expectedErr, err)
}
@ -1614,7 +1614,7 @@ func TestContext2Plan_preventDestroy_destroyPlan(t *testing.T) {
expectedErr := "aws_instance.foo has lifecycle.prevent_destroy"
if !strings.Contains(fmt.Sprintf("%s", diags.Err()), expectedErr) {
if plan != nil {
t.Logf(legacyDiffComparisonString(plan.Changes))
t.Logf("%s", legacyDiffComparisonString(plan.Changes))
}
t.Fatalf("expected diagnostics would contain %q\nactual diags: %s", expectedErr, diags.Err())
}