Fix empty diags not getting associated with source.

[paddycarver]

Right now, there's a bug that if a diagnostic comes back from the
provider with an AttributePath set, but no steps in the AttributePath,
Terraform thinks it's an attribute-specific diagnostic and not a
whole-resource diagnostic, but then doesn't associate it with any
specific attribute, meaning the diagnostic doesn't get associated with
the config at all.

This PR changes things to check if there are any steps in the
AttributePath before deciding this isn't a whole-resource diagnostic,
and if there aren't, treats it as a whole-resource diagnostic, instead.

See hashicorp/terraform-plugin-sdk#561 for more details on how this
surfaces in the wild.

[codecov]

Codecov Report

Merging #27710 (c840e9c) into master (9c16e57) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted Files	Coverage Δ
plugin/convert/diagnostics.go	50.64% <100.00%> (ø)
terraform/node_resource_plan.go	98.05% <0.00%> (+1.94%)	⬆️
internal/providercache/dir.go	73.46% <0.00%> (+6.12%)	⬆️

[paddycarver]

(I wasn't quite clear on how to test this, as nothing about the surfaced diagnostic makes it clear that this has happened, but if anyone has any suggestions, I'm willing to try them!)

[alisdair]

This looks good to me!

Below is a test which covers this change. Can you check if it makes sense to you? This is my first time looking at this code and I may have missed something.

// Test that a diagnostic with a present but empty attribute results in a
// whole body diagnostic. We verify this by inspecting the resulting Subject
// from the diagnostic when considered in the context of a config body.
func TestProtoDiagnostics_emptyAttributePath(t *testing.T) {
	protoDiags := []*proto.Diagnostic{
		{
			Severity: proto.Diagnostic_ERROR,
			Summary:  "error 1",
			Detail:   "error 1 detail",
			Attribute: &proto.AttributePath{
				Steps: []*proto.AttributePath_Step{
					// this slice is intentionally left empty
				},
			},
		},
	}
	tfDiags := ProtoToDiagnostics(protoDiags)

	testConfig := `provider "test" {
  foo = "bar"
}`
	f, parseDiags := hclsyntax.ParseConfig([]byte(testConfig), "test.tf", hcl.Pos{Line: 1, Column: 1})
	if parseDiags.HasErrors() {
		t.Fatal(parseDiags)
	}
	diags := tfDiags.InConfigBody(f.Body)

	if len(tfDiags) != 1 {
		t.Fatalf("expected 1 diag, got %d", len(tfDiags))
	}
	got := diags[0].Source().Subject
	want := &tfdiags.SourceRange{
		Filename: "test.tf",
		Start:    tfdiags.SourcePos{Line: 1, Column: 1},
		End:      tfdiags.SourcePos{Line: 1, Column: 1},
	}

	if !cmp.Equal(got, want, typeComparer, valueComparer) {
		t.Fatal(cmp.Diff(got, want, typeComparer, valueComparer))
	}
}

[jbardin]

Thanks @paddycarver!

[bflad]

Should this be backported to 0.14 as well?

Reference: #27283

[pselle]

Thank you for raising this question, @bflad (and linking to a closeable issue 😇 )... I'll put this as an item for the Core team to discuss since we're in a beta period for 0.15 now, and we have bit more process around backporting during these times 😄

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.