Fix: When a RequiredProvider is having syntax errors, return an empty one to allow further validations to generate their diags (#2408)

Signed-off-by: yottta <andrei.ciobanu@opentofu.org>
Co-authored-by: yottta <andpectech@gmail.com>

CHANGELOG.md

@@ -23,6 +23,7 @@ BUG FIXES:
 - The `format` and `formatlist` functions can now accept `null` as one of the arguments without causing problems during the apply phase. Previously these functions would incorrectly return an unknown value when given `null` and so could cause a failure during the apply phase where no unknown values are allowed. ([#2371](https://github.com/opentofu/opentofu/pull/2371))
 - Provider used in import is correctly identified. ([#2336](https://github.com/opentofu/opentofu/pull/2336))
 - `plantimestamp()` now returns unknown value during validation ([#2397](https://github.com/opentofu/opentofu/issues/2397))
+- Syntax error in the `required_providers` block does not panic anymore, but yields "syntax error" ([2344](https://github.com/opentofu/opentofu/issues/2344))
 
 ## Previous Releases
 

internal/configs/module_test.go

@@ -210,6 +210,31 @@ func TestModule_required_provider_overrides(t *testing.T) {
 	}
 }
 
+// When having multiple required providers defined, and one with syntax error,
+// ensure that the diagnostics are returned correctly for each and every validation.
+// In case a required_provider is containing syntax errors, we are returning an empty one just to allow the
+// later validations to add their results.
+func TestModule_required_providers_multiple_one_with_syntax_error(t *testing.T) {
+	_, diags := testModuleFromDir("testdata/invalid-modules/multiple-required-providers-with-syntax-error")
+	if !diags.HasErrors() {
+		t.Fatal("module should have error diags, but does not")
+	}
+
+	want := []string{
+		`Missing attribute value; Expected an attribute value`,
+		`Unexpected "resource" block; Blocks are not allowed here`,
+		`Duplicate required providers configuration`,
+	}
+	if wantLen, gotLen := len(want), len(diags.Errs()); wantLen != gotLen {
+		t.Fatalf("expected %d errors but got %d", wantLen, gotLen)
+	}
+	for i, e := range diags.Errs() {
+		if got := e.Error(); !strings.Contains(got, want[i]) {
+			t.Errorf("expected error to contain %q\nerror was: \n\t%q\n", want[i], got)
+		}
+	}
+}
+
 // Resources without explicit provider configuration are assigned a provider
 // implied based on the resource type. For example, this resource:
 //

internal/configs/provider_requirements.go

@@ -33,16 +33,18 @@ type RequiredProviders struct {
 }
 
 func decodeRequiredProvidersBlock(block *hcl.Block) (*RequiredProviders, hcl.Diagnostics) {
-	attrs, diags := block.Body.JustAttributes()
-	if diags.HasErrors() {
-		return nil, diags
-	}
-
 	ret := &RequiredProviders{
 		RequiredProviders: make(map[string]*RequiredProvider),
 		DeclRange:         block.DefRange,
 	}
 
+	attrs, diags := block.Body.JustAttributes()
+	if diags.HasErrors() {
+		// Returns an empty RequiredProvider to allow further validations to work properly,
+		// allowing to return all the diagnostics correctly.
+		return ret, diags
+	}
+
 	for name, attr := range attrs {
 		rp := &RequiredProvider{
 			Name:      name,

internal/configs/testdata/invalid-modules/multiple-required-providers-with-syntax-error/main.tf

@@ -0,0 +1,10 @@
+terraform {
+  required_providers {
+    tfcoremock = {
+      source  = "tfcoremock"
+      version = "0.3.0"
+    }
+  }
+}
+
+resource "tfcoremock_simple_resource" "foo" {}

internal/configs/testdata/invalid-modules/multiple-required-providers-with-syntax-error/other.tf

@@ -0,0 +1,10 @@
+terraform {
+  required_providers {
+    tfcoremock = {
+      source  = "tfcoremock"
+      version = "0.3.0"
+    {
+  }
+}
+
+resource "tfcoremock_simple_resource" "bar" {}