diff --git a/tfdiags/consolidate_warnings.go b/tfdiags/consolidate_warnings.go new file mode 100644 index 000000000..3e0983ee4 --- /dev/null +++ b/tfdiags/consolidate_warnings.go @@ -0,0 +1,130 @@ +package tfdiags + +import "fmt" + +// ConsolidateWarnings checks if there is an unreasonable amount of warnings +// with the same summary in the receiver and, if so, returns a new diagnostics +// with some of those warnings consolidated into a single warning in order +// to reduce the verbosity of the output. +// +// This mechanism is here primarily for diagnostics printed out at the CLI. In +// other contexts it is likely better to just return the warnings directly, +// particularly if they are going to be interpreted by software rather than +// by a human reader. +// +// The returned slice always has a separate backing array from the reciever, +// but some diagnostic values themselves might be shared. +// +// The definition of "unreasonable" may change in future releases. +func (diags Diagnostics) ConsolidateWarnings() Diagnostics { + // We'll start grouping when there are more than this number of warnings + // with the same summary. + const unreasonableThreshold = 2 + + if len(diags) == 0 { + return nil + } + + newDiags := make(Diagnostics, 0, len(diags)) + + // We'll track how many times we've seen each warning summary so we can + // decide when to start consolidating. Once we _have_ started consolidating, + // we'll also track the object representing the consolidated warning + // so we can continue appending to it. + warningStats := make(map[string]int) + warningGroups := make(map[string]*warningGroup) + + for _, diag := range diags { + severity := diag.Severity() + if severity != Warning || diag.Source().Subject == nil { + // Only warnings can get special treatment, and we only + // consolidate warnings that have source locations because + // our primary goal here is to deal with the situation where + // some configuration language feature is producing a warning + // each time it's used across a potentially-large config. + newDiags = newDiags.Append(diag) + continue + } + + desc := diag.Description() + summary := desc.Summary + if g, ok := warningGroups[summary]; ok { + // We're already grouping this one, so we'll just continue it. + g.Append(diag) + continue + } + + warningStats[summary]++ + if warningStats[summary] == unreasonableThreshold { + // Initially creating the group doesn't really change anything + // visibly in the result, since a group with only one warning + // is just a passthrough anyway, but once we do this any additional + // warnings with the same summary will get appended to this group. + g := &warningGroup{} + newDiags = newDiags.Append(g) + warningGroups[summary] = g + g.Append(diag) + continue + } + + // If this warning is not consolidating yet then we'll just append + // it directly. + newDiags = newDiags.Append(diag) + } + + return newDiags +} + +// A warningGroup is one or more warning diagnostics grouped together for +// UI consolidation purposes. +// +// A warningGroup with only one diagnostic in it is just a passthrough for +// that one diagnostic. If it has more than one then it will behave mostly +// like the first one but its detail message will include an additional +// sentence mentioning the consolidation. A warningGroup with no diagnostics +// at all is invalid and will panic when used. +type warningGroup struct { + Warnings Diagnostics +} + +var _ Diagnostic = (*warningGroup)(nil) + +func (wg *warningGroup) Severity() Severity { + return wg.Warnings[0].Severity() +} + +func (wg *warningGroup) Description() Description { + desc := wg.Warnings[0].Description() + if len(wg.Warnings) < 2 { + return desc + } + extraCount := len(wg.Warnings) - 1 + var msg string + switch extraCount { + case 1: + msg = "(and one more similar warning elsewhere)" + default: + msg = fmt.Sprintf("(and %d more similar warnings elsewhere)", extraCount) + } + if desc.Detail != "" { + desc.Detail = desc.Detail + "\n\n" + msg + } else { + desc.Detail = msg + } + return desc +} + +func (wg *warningGroup) Source() Source { + return wg.Warnings[0].Source() +} + +func (wg *warningGroup) FromExpr() *FromExpr { + return wg.Warnings[0].FromExpr() +} + +func (wg *warningGroup) Append(diag Diagnostic) { + if diag.Severity() != Warning { + panic("can't append a non-warning diagnostic to a warningGroup") + } + wg.Warnings = append(wg.Warnings, diag) +} diff --git a/tfdiags/consolidate_warnings_test.go b/tfdiags/consolidate_warnings_test.go new file mode 100644 index 000000000..6eb7eabc3 --- /dev/null +++ b/tfdiags/consolidate_warnings_test.go @@ -0,0 +1,179 @@ +package tfdiags + +import ( + "fmt" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/hcl/v2" +) + +func TestConsolidateWarnings(t *testing.T) { + var diags Diagnostics + + for i := 0; i < 4; i++ { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Warning 1", + Detail: fmt.Sprintf("This one has a subject %d", i), + Subject: &hcl.Range{ + Filename: "foo.tf", + Start: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + }, + }) + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Error 1", + Detail: fmt.Sprintf("This one has a subject %d", i), + Subject: &hcl.Range{ + Filename: "foo.tf", + Start: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + }, + }) + diags = diags.Append(Sourceless( + Warning, + "Warning 2", + fmt.Sprintf("This one is sourceless %d", i), + )) + diags = diags.Append(SimpleWarning("Warning 3")) + } + + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Warning 4", + Detail: "Only one of this one", + Subject: &hcl.Range{ + Filename: "foo.tf", + Start: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + }, + }) + + // We're using ForRPC here to force the diagnostics to be of a consistent + // type that we can easily assert against below. + got := diags.ConsolidateWarnings().ForRPC() + want := Diagnostics{ + // First set + &rpcFriendlyDiag{ + Severity_: Warning, + Summary_: "Warning 1", + Detail_: "This one has a subject 0", + Subject_: &SourceRange{ + Filename: "foo.tf", + Start: SourcePos{Line: 1, Column: 1, Byte: 0}, + End: SourcePos{Line: 1, Column: 1, Byte: 0}, + }, + }, + &rpcFriendlyDiag{ + Severity_: Error, + Summary_: "Error 1", + Detail_: "This one has a subject 0", + Subject_: &SourceRange{ + Filename: "foo.tf", + Start: SourcePos{Line: 1, Column: 1, Byte: 0}, + End: SourcePos{Line: 1, Column: 1, Byte: 0}, + }, + }, + &rpcFriendlyDiag{ + Severity_: Warning, + Summary_: "Warning 2", + Detail_: "This one is sourceless 0", + }, + &rpcFriendlyDiag{ + Severity_: Warning, + Summary_: "Warning 3", + }, + + // Second set (consolidation begins; note additional paragraph in Warning 1 detail) + &rpcFriendlyDiag{ + Severity_: Warning, + Summary_: "Warning 1", + Detail_: "This one has a subject 1\n\n(and 2 more similar warnings elsewhere)", + Subject_: &SourceRange{ + Filename: "foo.tf", + Start: SourcePos{Line: 1, Column: 1, Byte: 0}, + End: SourcePos{Line: 1, Column: 1, Byte: 0}, + }, + }, + &rpcFriendlyDiag{ + Severity_: Error, + Summary_: "Error 1", + Detail_: "This one has a subject 1", + Subject_: &SourceRange{ + Filename: "foo.tf", + Start: SourcePos{Line: 1, Column: 1, Byte: 0}, + End: SourcePos{Line: 1, Column: 1, Byte: 0}, + }, + }, + &rpcFriendlyDiag{ + Severity_: Warning, + Summary_: "Warning 2", + Detail_: "This one is sourceless 1", + }, + &rpcFriendlyDiag{ + Severity_: Warning, + Summary_: "Warning 3", + }, + + // Third set (no more Warning 1, because it's consolidated) + &rpcFriendlyDiag{ + Severity_: Error, + Summary_: "Error 1", + Detail_: "This one has a subject 2", + Subject_: &SourceRange{ + Filename: "foo.tf", + Start: SourcePos{Line: 1, Column: 1, Byte: 0}, + End: SourcePos{Line: 1, Column: 1, Byte: 0}, + }, + }, + &rpcFriendlyDiag{ + Severity_: Warning, + Summary_: "Warning 2", + Detail_: "This one is sourceless 2", + }, + &rpcFriendlyDiag{ + Severity_: Warning, + Summary_: "Warning 3", + }, + + // Fourth set (still no warning 1) + &rpcFriendlyDiag{ + Severity_: Error, + Summary_: "Error 1", + Detail_: "This one has a subject 3", + Subject_: &SourceRange{ + Filename: "foo.tf", + Start: SourcePos{Line: 1, Column: 1, Byte: 0}, + End: SourcePos{Line: 1, Column: 1, Byte: 0}, + }, + }, + &rpcFriendlyDiag{ + Severity_: Warning, + Summary_: "Warning 2", + Detail_: "This one is sourceless 3", + }, + &rpcFriendlyDiag{ + Severity_: Warning, + Summary_: "Warning 3", + }, + + // Special straggler warning gets to show up unconsolidated, because + // there is only one of it. + &rpcFriendlyDiag{ + Severity_: Warning, + Summary_: "Warning 4", + Detail_: "Only one of this one", + Subject_: &SourceRange{ + Filename: "foo.tf", + Start: SourcePos{Line: 1, Column: 1, Byte: 0}, + End: SourcePos{Line: 1, Column: 1, Byte: 0}, + }, + }, + } + + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("wrong result\n%s", diff) + } +}