tfdiags: Treat unknown-related or sensitive-related messages differently

By observing the sorts of questions people ask in the community, and the
ways they ask them, we've inferred that various different people have been
confused by Terraform reporting that a value won't be known until apply
or that a value is sensitive as part of an error message when that message
doesn't actually relate to the known-ness and sensitivity of any value.

Quite reasonably, someone who sees Terraform discussing an unfamiliar
concept like unknown values can assume that it must be somehow relevant to
the problem being discussed, and so in that sense Terraform's current
error messages are giving "too much information": information that isn't
actually helpful in understanding the problem being described, and in the
worst case is a distraction from understanding the problem being described.

With that in mind then, here we introduce an explicit annotation on
diagnostic objects that are directly talking about unknown values or
sensitive values, and then the diagnostic renderer will react to that to
avoid using the terminology "known only after apply" or "sensitive" in the
generated diagnostic annotations unless we're rendering a message that is
explicitly related to one of those topics.

This ends up being a bit of a cross-cutting concern because the code that
generates these diagnostics and the code that renders them are in separate
packages and are not directly aware of each other. With that in mind, the
logic for actually deciding for a particular diagnostic whether it's
flagged in one of these special ways lives inside the tfdiags package as
an intermediation point, which both the diagnostic generator (in the core
package) and the diagnostic renderer can both depend on.

internal/command/format/diagnostic_test.go

@@ -130,6 +130,7 @@ func TestDiagnostic(t *testing.T) {
 						}),
 					},
 				},
+				Extra: diagnosticCausedBySensitive(true),
 			},
 			`[red]╷[reset]
 [red]│[reset] [bold][red]Error: [reset][bold]Bad bad bad[reset]
@@ -164,6 +165,7 @@ func TestDiagnostic(t *testing.T) {
 						}),
 					},
 				},
+				Extra: diagnosticCausedByUnknown(true),
 			},
 			`[red]╷[reset]
 [red]│[reset] [bold][red]Error: [reset][bold]Bad bad bad[reset]
@@ -198,6 +200,7 @@ func TestDiagnostic(t *testing.T) {
 						}),
 					},
 				},
+				Extra: diagnosticCausedByUnknown(true),
 			},
 			`[red]╷[reset]
 [red]│[reset] [bold][red]Error: [reset][bold]Bad bad bad[reset]
@@ -391,6 +394,7 @@ Whatever shall we do?
 						}),
 					},
 				},
+				Extra: diagnosticCausedBySensitive(true),
 			},
 			`
 Error: Bad bad bad
@@ -400,6 +404,37 @@ Error: Bad bad bad
     ├────────────────
     │ boop.beep has a sensitive value
 
+Whatever shall we do?
+`,
+		},
+		"error with source code subject and expression referring to sensitive value when not related to sensitivity": {
+			&hcl.Diagnostic{
+				Severity: hcl.DiagError,
+				Summary:  "Bad bad bad",
+				Detail:   "Whatever shall we do?",
+				Subject: &hcl.Range{
+					Filename: "test.tf",
+					Start:    hcl.Pos{Line: 1, Column: 6, Byte: 5},
+					End:      hcl.Pos{Line: 1, Column: 12, Byte: 11},
+				},
+				Expression: hcltest.MockExprTraversal(hcl.Traversal{
+					hcl.TraverseRoot{Name: "boop"},
+					hcl.TraverseAttr{Name: "beep"},
+				}),
+				EvalContext: &hcl.EvalContext{
+					Variables: map[string]cty.Value{
+						"boop": cty.ObjectVal(map[string]cty.Value{
+							"beep": cty.StringVal("blah").Mark(marks.Sensitive),
+						}),
+					},
+				},
+			},
+			`
+Error: Bad bad bad
+
+  on test.tf line 1:
+   1: test source code
+
 Whatever shall we do?
 `,
 		},
@@ -424,6 +459,7 @@ Whatever shall we do?
 						}),
 					},
 				},
+				Extra: diagnosticCausedByUnknown(true),
 			},
 			`
 Error: Bad bad bad
@@ -433,6 +469,39 @@ Error: Bad bad bad
     ├────────────────
     │ boop.beep is a string, known only after apply
 
+Whatever shall we do?
+`,
+		},
+		"error with source code subject and unknown string expression when problem isn't unknown-related": {
+			&hcl.Diagnostic{
+				Severity: hcl.DiagError,
+				Summary:  "Bad bad bad",
+				Detail:   "Whatever shall we do?",
+				Subject: &hcl.Range{
+					Filename: "test.tf",
+					Start:    hcl.Pos{Line: 1, Column: 6, Byte: 5},
+					End:      hcl.Pos{Line: 1, Column: 12, Byte: 11},
+				},
+				Expression: hcltest.MockExprTraversal(hcl.Traversal{
+					hcl.TraverseRoot{Name: "boop"},
+					hcl.TraverseAttr{Name: "beep"},
+				}),
+				EvalContext: &hcl.EvalContext{
+					Variables: map[string]cty.Value{
+						"boop": cty.ObjectVal(map[string]cty.Value{
+							"beep": cty.UnknownVal(cty.String),
+						}),
+					},
+				},
+			},
+			`
+Error: Bad bad bad
+
+  on test.tf line 1:
+   1: test source code
+    ├────────────────
+    │ boop.beep is a string
+
 Whatever shall we do?
 `,
 		},
@@ -457,6 +526,7 @@ Whatever shall we do?
 						}),
 					},
 				},
+				Extra: diagnosticCausedByUnknown(true),
 			},
 			`
 Error: Bad bad bad
@@ -466,6 +536,37 @@ Error: Bad bad bad
     ├────────────────
     │ boop.beep will be known only after apply
 
+Whatever shall we do?
+`,
+		},
+		"error with source code subject and unknown expression of unknown type when problem isn't unknown-related": {
+			&hcl.Diagnostic{
+				Severity: hcl.DiagError,
+				Summary:  "Bad bad bad",
+				Detail:   "Whatever shall we do?",
+				Subject: &hcl.Range{
+					Filename: "test.tf",
+					Start:    hcl.Pos{Line: 1, Column: 6, Byte: 5},
+					End:      hcl.Pos{Line: 1, Column: 12, Byte: 11},
+				},
+				Expression: hcltest.MockExprTraversal(hcl.Traversal{
+					hcl.TraverseRoot{Name: "boop"},
+					hcl.TraverseAttr{Name: "beep"},
+				}),
+				EvalContext: &hcl.EvalContext{
+					Variables: map[string]cty.Value{
+						"boop": cty.ObjectVal(map[string]cty.Value{
+							"beep": cty.UnknownVal(cty.DynamicPseudoType),
+						}),
+					},
+				},
+			},
+			`
+Error: Bad bad bad
+
+  on test.tf line 1:
+   1: test source code
+
 Whatever shall we do?
 `,
 		},
@@ -820,3 +921,25 @@ func (e fakeDiagFunctionCallExtra) CalledFunctionName() string {
 func (e fakeDiagFunctionCallExtra) FunctionCallError() error {
 	return nil
 }
+
+// diagnosticCausedByUnknown is a testing helper for exercising our logic
+// for selectively showing unknown values alongside our source snippets for
+// diagnostics that are explicitly marked as being caused by unknown values.
+type diagnosticCausedByUnknown bool
+
+var _ tfdiags.DiagnosticExtraBecauseUnknown = diagnosticCausedByUnknown(true)
+
+func (e diagnosticCausedByUnknown) DiagnosticCausedByUnknown() bool {
+	return bool(e)
+}
+
+// diagnosticCausedBySensitive is a testing helper for exercising our logic
+// for selectively showing sensitive values alongside our source snippets for
+// diagnostics that are explicitly marked as being caused by sensitive values.
+type diagnosticCausedBySensitive bool
+
+var _ tfdiags.DiagnosticExtraBecauseSensitive = diagnosticCausedBySensitive(true)
+
+func (e diagnosticCausedBySensitive) DiagnosticCausedBySensitive() bool {
+	return bool(e)
+}

internal/command/views/json/diagnostic.go

@@ -271,6 +271,8 @@ func NewDiagnostic(diag tfdiags.Diagnostic, sources map[string][]byte) *Diagnost
 				vars := expr.Variables()
 				values := make([]DiagnosticExpressionValue, 0, len(vars))
 				seen := make(map[string]struct{}, len(vars))
+				includeUnknown := tfdiags.DiagnosticCausedByUnknown(diag)
+				includeSensitive := tfdiags.DiagnosticCausedBySensitive(diag)
 			Traversals:
 				for _, traversal := range vars {
 					for len(traversal) > 1 {
@@ -292,14 +294,35 @@ func NewDiagnostic(diag tfdiags.Diagnostic, sources map[string][]byte) *Diagnost
 						}
 						switch {
 						case val.HasMark(marks.Sensitive):
-							// We won't say anything at all about sensitive values,
-							// because we might give away something that was
-							// sensitive about them.
+							// We only mention a sensitive value if the diagnostic
+							// we're rendering is explicitly marked as being
+							// caused by sensitive values, because otherwise
+							// readers tend to be misled into thinking the error
+							// is caused by the sensitive value even when it isn't.
+							if !includeSensitive {
+								continue Traversals
+							}
+							// Even when we do mention one, we keep it vague
+							// in order to minimize the chance of giving away
+							// whatever was sensitive about it.
 							value.Statement = "has a sensitive value"
 						case !val.IsKnown():
+							// We'll avoid saying anything about unknown or
+							// "known after apply" unless the diagnostic is
+							// explicitly marked as being caused by unknown
+							// values, because otherwise readers tend to be
+							// misled into thinking the error is caused by the
+							// unknown value even when it isn't.
 							if ty := val.Type(); ty != cty.DynamicPseudoType {
-								value.Statement = fmt.Sprintf("is a %s, known only after apply", ty.FriendlyName())
+								if includeUnknown {
+									value.Statement = fmt.Sprintf("is a %s, known only after apply", ty.FriendlyName())
+								} else {
+									value.Statement = fmt.Sprintf("is a %s", ty.FriendlyName())
+								}
 							} else {
+								if !includeUnknown {
+									continue Traversals
+								}
 								value.Statement = "will be known only after apply"
 							}
 						default: