Separate Envelope validation from construction

This change moves Envelope validation out of constructors, and into a
separate `Validate` method. This is similar to the recent changes for
regular geometries.

It includes two further small breaking changes, so that all breaking
changes are bundled together:

- The `ExtendToIncludeXY` method is renamed to `ExpandToIncludeXY` (to
  be consistent with `ExpandToIncludeEnvelope`).

- The `NewEnvelope` now accepts a variadic list of `XY` rather than a
  slice of `XY`. This is just a small ergonomic improvement.

CHANGELOG.md

@@ -8,6 +8,17 @@ YYYY-MM-DD
 
 - Simplifies the internal representation of the `Envelope` type.
 
+- **Breaking change**: Renames `Envelope`'s `ExtendToIncludeXY` method to
+  `ExpandToIncludeXY`. This makes the names of the `ExpandToIncludeXY` and
+  `ExpandToIncludeEnvelope` methods have consistent naming.
+
+- **Breaking change**: Alters the signature (and behaviour) of the
+  `NewEnvelope` function. It previously returned an `Envelope` and an `error`,
+  and new just returns an `Envelope`. It no longer validates that its inputs
+  don't contain NaN or +/- infinity (this can be checked via the `Validate`
+  method if desired). It also now accepts a variadic list of `XY` values,
+  rather than a slice of `XY` values.
+
 ## v0.45.1
 
 2023-09-29

geom/alg_intersects.go

@@ -226,19 +226,19 @@ func hasIntersectionBetweenLines(
 
 			if !populateExtension {
 				env = inter.ptA.uncheckedEnvelope()
-				env = env.uncheckedExtend(inter.ptB)
+				env = env.ExpandToIncludeXY(inter.ptB)
 				return rtree.Stop
 			}
 
 			if inter.ptA != inter.ptB {
 				env = inter.ptA.uncheckedEnvelope()
-				env = env.uncheckedExtend(inter.ptB)
+				env = env.ExpandToIncludeXY(inter.ptB)
 				return rtree.Stop
 			}
 
 			// Single point intersection case from here onwards:
 
-			env = env.uncheckedExtend(inter.ptA)
+			env = env.ExpandToIncludeXY(inter.ptA)
 			if !env.IsPoint() {
 				return rtree.Stop
 			}

geom/perf_test.go

@@ -167,8 +167,7 @@ func BenchmarkPolygonMultipleRingsValidation(b *testing.B) {
 func BenchmarkPolygonZigZagRingsValidation(b *testing.B) {
 	for _, sz := range []int{10, 100, 1000, 10000} {
 		b.Run(fmt.Sprintf("n=%d", sz), func(b *testing.B) {
-			outerRingEnv, err := NewEnvelope([]XY{{}, {7, float64(sz + 1)}})
-			expectNoErr(b, err)
+			outerRingEnv := NewEnvelope(XY{}, XY{7, float64(sz + 1)})
 			outerRing := outerRingEnv.AsGeometry().MustAsPolygon().ExteriorRing()
 			var leftFloats, rightFloats []float64
 			for i := 0; i < sz; i++ {

geom/twkb_parser.go

@@ -95,16 +95,16 @@ func UnmarshalTWKBEnvelope(twkb []byte) (Envelope, error) {
 	if !p.hasBBox {
 		return Envelope{}, nil
 	}
-	return NewEnvelope([]XY{
-		{
+	return NewEnvelope(
+		XY{
 			p.scalings[0] * float64(p.bbox[0]),
 			p.scalings[1] * float64(p.bbox[2]),
 		},
-		{
+		XY{
 			p.scalings[0] * float64(p.bbox[0]+p.bbox[1]),
 			p.scalings[1] * float64(p.bbox[2]+p.bbox[3]),
 		},
-	})
+	), nil
 }
 
 // twkbParser holds all state information for interpreting TWKB buffers

geom/type_envelope.go

@@ -28,22 +28,34 @@ type Envelope struct {
 // NewEnvelope returns the smallest envelope that contains all provided XYs.
 // It returns an error if any of the XYs contain NaN or +/- Infinity
 // coordinates.
-func NewEnvelope(xys []XY) (Envelope, error) {
+func NewEnvelope(xys ...XY) Envelope {
 	var env Envelope
 	for _, xy := range xys {
-		var err error
-		env, err = env.ExtendToIncludeXY(xy)
-		if err != nil {
-			return Envelope{}, err
-		}
+		env = env.ExpandToIncludeXY(xy)
 	}
-	return env, nil
+	return env
 }
 
 func newUncheckedEnvelope(min, max XY) Envelope {
 	return Envelope{min, max, true}
 }
 
+// Validate checks if the Envelope is valid. The only validation rule is that
+// the coordinates the Envelope was constructed from must not be NaN or +/-
+// infinity. An empty Envelope is always valid.
+func (e Envelope) Validate() error {
+	if e.IsEmpty() {
+		return nil
+	}
+	if err := e.min.validate(); err != nil {
+		return wrap(err, "min coords invalid")
+	}
+	if err := e.max.validate(); err != nil {
+		return wrap(err, "max coords invalid")
+	}
+	return nil
+}
+
 // IsEmpty returns true if and only if this envelope is empty.
 func (e Envelope) IsEmpty() bool {
 	return !e.nonEmpty
@@ -123,20 +135,11 @@ func (e Envelope) MinMaxXYs() (XY, XY, bool) {
 	return e.min, e.max, true
 }
 
-// ExtendToIncludeXY returns the smallest envelope that contains all of the
-// points in this envelope along with the provided point. It gives an error if
-// the XY contains NaN or +/- Infinite coordinates.
-func (e Envelope) ExtendToIncludeXY(xy XY) (Envelope, error) {
-	if err := xy.validate(); err != nil {
-		return Envelope{}, err
-	}
-	return e.uncheckedExtend(xy), nil
-}
-
-// uncheckedExtend extends the envelope in the same manner as
-// ExtendToIncludeXY but doesn't validate the XY. It should only be used
-// when the XY doesn't come directly from user input.
-func (e Envelope) uncheckedExtend(xy XY) Envelope {
+// ExpandToIncludeXY returns the smallest envelope that contains all of the
+// points in this envelope along with the provided point. If will produce an
+// invalid envelope if any of the coordinates in the existing envelope or new
+// XY contain NaN or +/- infinity.
+func (e Envelope) ExpandToIncludeXY(xy XY) Envelope {
 	if e.IsEmpty() {
 		return newUncheckedEnvelope(xy, xy)
 	}
@@ -147,7 +150,9 @@ func (e Envelope) uncheckedExtend(xy XY) Envelope {
 }
 
 // ExpandToIncludeEnvelope returns the smallest envelope that contains all of
-// the points in this envelope and another envelope.
+// the points in this envelope and another envelope. It will produce an invalid
+// envelope if min or max coordinates of either envelope contain NaN or +/-
+// infinity.
 func (e Envelope) ExpandToIncludeEnvelope(o Envelope) Envelope {
 	if e.IsEmpty() {
 		return o

geom/type_envelope_test.go

@@ -11,11 +11,7 @@ import (
 )
 
 func onePtEnv(x, y float64) Envelope {
-	env, err := Envelope{}.ExtendToIncludeXY(XY{X: x, Y: y})
-	if err != nil {
-		panic("could not construct env")
-	}
-	return env
+	return Envelope{}.ExpandToIncludeXY(XY{X: x, Y: y})
 }
 
 func twoPtEnv(minX, minY, maxX, maxY float64) Envelope {
@@ -25,11 +21,7 @@ func twoPtEnv(minX, minY, maxX, maxY float64) Envelope {
 	if minY > maxY {
 		panic(fmt.Sprintf("Y values out of order: %v %v", minY, maxY))
 	}
-	env, err := onePtEnv(minX, minY).ExtendToIncludeXY(XY{X: maxX, Y: maxY})
-	if err != nil {
-		panic("could not construct env")
-	}
-	return env
+	return onePtEnv(minX, minY).ExpandToIncludeXY(XY{X: maxX, Y: maxY})
 }
 
 func TestEnvelopeNew(t *testing.T) {
@@ -65,8 +57,7 @@ func TestEnvelopeNew(t *testing.T) {
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
-			got, err := NewEnvelope(tc.xys)
-			expectNoErr(t, err)
+			got := NewEnvelope(tc.xys...)
 			expectEnvEq(t, got, tc.want)
 		})
 	}
@@ -207,34 +198,29 @@ func TestEnvelopeAttributes(t *testing.T) {
 	}
 }
 
-func TestEnvelopeExtendToIncludeXY(t *testing.T) {
+func TestEnvelopeExpandToIncludeXY(t *testing.T) {
 	t.Run("empty", func(t *testing.T) {
-		env, err := Envelope{}.ExtendToIncludeXY(XY{1, 2})
-		expectNoErr(t, err)
+		env := Envelope{}.ExpandToIncludeXY(XY{1, 2})
 		expectGeomEqWKT(t, env.Min().AsGeometry(), "POINT(1 2)")
 		expectGeomEqWKT(t, env.Max().AsGeometry(), "POINT(1 2)")
 	})
 	t.Run("single point extend to same", func(t *testing.T) {
-		env, err := onePtEnv(1, 2).ExtendToIncludeXY(XY{1, 2})
-		expectNoErr(t, err)
+		env := onePtEnv(1, 2).ExpandToIncludeXY(XY{1, 2})
 		expectGeomEqWKT(t, env.Min().AsGeometry(), "POINT(1 2)")
 		expectGeomEqWKT(t, env.Max().AsGeometry(), "POINT(1 2)")
 	})
 	t.Run("single point extend to different", func(t *testing.T) {
-		env, err := onePtEnv(1, 2).ExtendToIncludeXY(XY{-1, 3})
-		expectNoErr(t, err)
+		env := onePtEnv(1, 2).ExpandToIncludeXY(XY{-1, 3})
 		expectGeomEqWKT(t, env.Min().AsGeometry(), "POINT(-1 2)")
 		expectGeomEqWKT(t, env.Max().AsGeometry(), "POINT(1 3)")
 	})
 	t.Run("area extend within", func(t *testing.T) {
-		env, err := twoPtEnv(1, 2, 3, 4).ExtendToIncludeXY(XY{2, 3})
-		expectNoErr(t, err)
+		env := twoPtEnv(1, 2, 3, 4).ExpandToIncludeXY(XY{2, 3})
 		expectGeomEqWKT(t, env.Min().AsGeometry(), "POINT(1 2)")
 		expectGeomEqWKT(t, env.Max().AsGeometry(), "POINT(3 4)")
 	})
 	t.Run("area extend outside", func(t *testing.T) {
-		env, err := twoPtEnv(1, 2, 3, 4).ExtendToIncludeXY(XY{100, 200})
-		expectNoErr(t, err)
+		env := twoPtEnv(1, 2, 3, 4).ExpandToIncludeXY(XY{100, 200})
 		expectGeomEqWKT(t, env.Min().AsGeometry(), "POINT(1 2)")
 		expectGeomEqWKT(t, env.Max().AsGeometry(), "POINT(100 200)")
 	})
@@ -371,22 +357,37 @@ func TestEnvelopeInvalidXYInteractions(t *testing.T) {
 		{-inf, -inf},
 	} {
 		t.Run(fmt.Sprintf("new_envelope_with_first_arg_invalid_%d", i), func(t *testing.T) {
-			_, err := NewEnvelope([]XY{tc})
-			expectErr(t, err)
+			env := NewEnvelope(tc)
+			expectErr(t, env.Validate())
 		})
 		t.Run(fmt.Sprintf("new_envelope_with_second_arg_invalid_%d", i), func(t *testing.T) {
-			_, err := NewEnvelope([]XY{{}, tc})
-			expectErr(t, err)
+			env := NewEnvelope(XY{}, tc)
+			expectErr(t, env.Validate())
+		})
+		t.Run(fmt.Sprintf("expand_to_include_invalid_xy_%d", i), func(t *testing.T) {
+			env := NewEnvelope(XY{-1, -1}, XY{1, 1})
+			env = env.ExpandToIncludeXY(tc)
+			expectErr(t, env.Validate())
+		})
+		t.Run(fmt.Sprintf("expand_from_invalid_to_include_env_%d", i), func(t *testing.T) {
+			env := NewEnvelope(tc)
+			env = env.ExpandToIncludeXY(XY{1, 1})
+			expectErr(t, env.Validate())
+		})
+		t.Run(fmt.Sprintf("expand_to_include_invalid_env_%d", i), func(t *testing.T) {
+			base := NewEnvelope(XY{-1, -1}, XY{1, 1})
+			other := NewEnvelope(tc)
+			env := base.ExpandToIncludeEnvelope(other)
+			expectErr(t, env.Validate())
 		})
-		t.Run(fmt.Sprintf("extend_to_include_invalid_xy_%d", i), func(t *testing.T) {
-			env, err := NewEnvelope([]XY{{-1, -1}, {1, 1}})
-			expectNoErr(t, err)
-			_, err = env.ExtendToIncludeXY(tc)
-			expectErr(t, err)
+		t.Run(fmt.Sprintf("expand_from_invalid_to_include_env_%d", i), func(t *testing.T) {
+			base := NewEnvelope(tc)
+			other := NewEnvelope(XY{-1, -1}, XY{1, 1})
+			env := base.ExpandToIncludeEnvelope(other)
+			expectErr(t, env.Validate())
 		})
 		t.Run(fmt.Sprintf("contains_invalid_xy_%d", i), func(t *testing.T) {
-			env, err := NewEnvelope([]XY{{-1, -1}, {1, 1}})
-			expectNoErr(t, err)
+			env := NewEnvelope(XY{-1, -1}, XY{1, 1})
 			expectFalse(t, env.Contains(tc))
 		})
 	}
@@ -611,8 +612,7 @@ func TestBoundingDiagonal(t *testing.T) {
 		},
 	} {
 		t.Run(strconv.Itoa(i), func(t *testing.T) {
-			env, err := NewEnvelope(tc.env)
-			expectNoErr(t, err)
+			env := NewEnvelope(tc.env...)
 			got := env.BoundingDiagonal()
 			expectGeomEqWKT(t, got, tc.want)
 		})

geom/type_point.go

@@ -97,7 +97,7 @@ func (p Point) IsSimple() bool {
 // envelope, or an envelope covering a single point).
 func (p Point) Envelope() Envelope {
 	if xy, ok := p.XY(); ok {
-		return Envelope{}.uncheckedExtend(xy)
+		return Envelope{}.ExpandToIncludeXY(xy)
 	}
 	return Envelope{}
 }

internal/cmprefimpl/cmpgeos/handle.go

@@ -509,7 +509,11 @@ func (h *Handle) envelope(g geom.Geometry) (geom.Envelope, error) {
 		if C.GEOSGeomGetY_r(h.context, env, &y) == 0 {
 			return geom.Envelope{}, h.err()
 		}
-		return geom.NewEnvelope([]geom.XY{{X: float64(x), Y: float64(y)}})
+		env := geom.NewEnvelope(geom.XY{X: float64(x), Y: float64(y)})
+		if err := env.Validate(); err != nil {
+			return geom.Envelope{}, err
+		}
+		return env, nil
 	case "Polygon":
 		// Continues below
 	default:
@@ -549,12 +553,12 @@ func (h *Handle) envelope(g geom.Geometry) (geom.Envelope, error) {
 			return geom.Envelope{}, h.err()
 		}
 		xy := geom.XY{X: float64(x), Y: float64(y)}
-		sfEnv, err = sfEnv.ExtendToIncludeXY(xy)
-		if err != nil {
-			return geom.Envelope{}, err
-		}
+		sfEnv = sfEnv.ExpandToIncludeXY(xy)
 	}
 
+	if err := sfEnv.Validate(); err != nil {
+		return geom.Envelope{}, err
+	}
 	return sfEnv, nil
 }