sourcebundle: remove the option to append anything into a diagnostic …

…that will then panic
hashicorp · Aug 20, 2024 · 27f4351 · 27f4351
1 parent 9b7ef21
commit 27f4351
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 33 deletions.
diff --git a/sourcebundle/builder.go b/sourcebundle/builder.go
@@ -256,7 +256,7 @@ func (b *Builder) resolvePending(ctx context.Context) (diags Diagnostics) {
 
 			realSource, err := b.findRegistryPackageSource(ctx, next.sourceAddr, next.versions)
 			if err != nil {
-				diags = diags.Append(&internalDiagnostic{
+				diags = append(diags, &internalDiagnostic{
 					severity: DiagError,
 					summary:  "Cannot resolve module registry package",
 					detail:   fmt.Sprintf("Error resolving module registry source %s: %s.", next.sourceAddr, err),
@@ -279,7 +279,7 @@ func (b *Builder) resolvePending(ctx context.Context) (diags Diagnostics) {
 			pkgAddr := next.sourceAddr.Package()
 			pkgLocalDir, err := b.ensureRemotePackage(ctx, pkgAddr)
 			if err != nil {
-				diags = diags.Append(&internalDiagnostic{
+				diags = append(diags, &internalDiagnostic{
 					severity: DiagError,
 					summary:  "Cannot install source package",
 					detail:   fmt.Sprintf("Error installing %s: %s.", next.sourceAddr.Package(), err),
@@ -317,7 +317,7 @@ func (b *Builder) resolvePending(ctx context.Context) (diags Diagnostics) {
 						})
 					},
 					localResolveErrCb: func(err error) {
-						diags = diags.Append(&internalDiagnostic{
+						diags = append(diags, &internalDiagnostic{
 							severity: DiagError,
 							summary:  "Invalid relative source address",
 							detail:   fmt.Sprintf("Invalid relative path from %s: %s.", next.sourceAddr, err),
@@ -333,7 +333,7 @@ func (b *Builder) resolvePending(ctx context.Context) (diags Diagnostics) {
 						cb(ctx, moreDiags)
 					}
 				}
-				diags = diags.Append(moreDiags)
+				diags = append(diags, moreDiags...)
 				if diags.HasErrors() {
 					continue
 				}

diff --git a/sourcebundle/builder_test.go b/sourcebundle/builder_test.go
@@ -20,8 +20,9 @@ import (
 	"github.com/apparentlymart/go-versions/versions"
 	"github.com/apparentlymart/go-versions/versions/constraints"
 	"github.com/google/go-cmp/cmp"
-	"github.com/hashicorp/go-slug/sourceaddrs"
 	regaddr "github.com/hashicorp/terraform-registry-address"
+
+	"github.com/hashicorp/go-slug/sourceaddrs"
 )
 
 func TestBuilderSimple(t *testing.T) {
@@ -877,13 +878,13 @@ func (f stubDependencyFinder) FindDependencies(fsys fs.FS, subPath string, deps
 	file, err := fsys.Open(filePath)
 	if err != nil {
 		if errors.Is(err, fs.ErrNotExist) {
-			diags = diags.Append(&internalDiagnostic{
+			diags = append(diags, &internalDiagnostic{
 				severity: DiagError,
 				summary:  "Missing stub dependency file",
 				detail:   fmt.Sprintf("There is no file %q in the package.", filePath),
 			})
 		} else {
-			diags = diags.Append(&internalDiagnostic{
+			diags = append(diags, &internalDiagnostic{
 				severity: DiagError,
 				summary:  "Invalid stub dependency file",
 				detail:   fmt.Sprintf("Cannot open %q in the package: %s.", filePath, err),
@@ -901,15 +902,15 @@ func (f stubDependencyFinder) FindDependencies(fsys fs.FS, subPath string, deps
 		sourceAddrRaw, versionsRaw, hasVersions := strings.Cut(line, " ")
 		sourceAddr, err := sourceaddrs.ParseSource(sourceAddrRaw)
 		if err != nil {
-			diags = diags.Append(&internalDiagnostic{
+			diags = append(diags, &internalDiagnostic{
 				severity: DiagError,
 				summary:  "Invalid source address in stub dependency file",
 				detail:   fmt.Sprintf("Cannot use %q as a source address: %s.", sourceAddrRaw, err),
 			})
 			continue
 		}
 		if hasVersions && !sourceAddr.SupportsVersionConstraints() {
-			diags = diags.Append(&internalDiagnostic{
+			diags = append(diags, &internalDiagnostic{
 				severity: DiagError,
 				summary:  "Invalid source address in stub dependency file",
 				detail:   fmt.Sprintf("Cannot specify a version constraint string for %s.", sourceAddr),
@@ -920,7 +921,7 @@ func (f stubDependencyFinder) FindDependencies(fsys fs.FS, subPath string, deps
 		if hasVersions {
 			cnsts, err := constraints.ParseRubyStyleMulti(versionsRaw)
 			if err != nil {
-				diags = diags.Append(&internalDiagnostic{
+				diags = append(diags, &internalDiagnostic{
 					severity: DiagError,
 					summary:  "Invalid version constraints in stub dependency file",
 					detail:   fmt.Sprintf("Cannot use %q as version constraints for %s: %s.", versionsRaw, sourceAddrRaw, err),
@@ -947,7 +948,7 @@ func (f stubDependencyFinder) FindDependencies(fsys fs.FS, subPath string, deps
 		case sourceaddrs.LocalSource:
 			deps.AddLocalSource(sourceAddr, depFinder)
 		default:
-			diags = diags.Append(&internalDiagnostic{
+			diags = append(diags, &internalDiagnostic{
 				severity: DiagError,
 				summary:  "Unsupported source address type",
 				detail:   fmt.Sprintf("stubDependencyFinder doesn't support %T addresses", sourceAddr),
@@ -956,7 +957,7 @@ func (f stubDependencyFinder) FindDependencies(fsys fs.FS, subPath string, deps
 		}
 	}
 	if err := sc.Err(); err != nil {
-		diags = diags.Append(&internalDiagnostic{
+		diags = append(diags, &internalDiagnostic{
 			severity: DiagError,
 			summary:  "Invalid stub dependency file",
 			detail:   fmt.Sprintf("Failed to read %s in the package: %s.", filePath, err),

diff --git a/sourcebundle/diagnostics.go b/sourcebundle/diagnostics.go
@@ -4,16 +4,14 @@
 package sourcebundle
 
 import (
-	"fmt"
-
 	"github.com/hashicorp/go-slug/sourceaddrs"
 )
 
 // Diagnostics is a collection of problems (errors and warnings) that occurred
 // during an operation.
 type Diagnostics []Diagnostic
 
-// Diagnostics represents a single problem (error or warning) that has occurred
+// Diagnostic represents a single problem (error or warning) that has occurred
 // during an operation.
 //
 // This interface has no concrete implementations in this package.
@@ -44,24 +42,6 @@ func (diags Diagnostics) HasErrors() bool {
 	return false
 }
 
-func (diags Diagnostics) Append(more ...interface{}) Diagnostics {
-	for _, item := range more {
-		if item == nil {
-			continue
-		}
-
-		switch item := item.(type) {
-		case Diagnostic:
-			diags = append(diags, item)
-		case Diagnostics:
-			diags = append(diags, item...)
-		default:
-			panic(fmt.Errorf("can't construct diagnostic(s) from %T", item))
-		}
-	}
-	return diags
-}
-
 type DiagSeverity rune
 
 const (