Improve filter parser errors

crates/filter-parser/src/error.rs

@@ -78,7 +78,7 @@ pub enum ErrorKind<'a> {
     GeoBoundingBox,
     GeoPolygon,
     GeoPolygonTooFewPoints,
-    GeoPolygonNotPairs(usize),
+    GeoCoordinatesNotPair(usize),
     MisusedGeoRadius,
     MisusedGeoBoundingBox,
     VectorFilterLeftover,
@@ -212,8 +212,8 @@ impl Display for Error<'_> {
             ErrorKind::GeoPolygonTooFewPoints => {
                 writeln!(f, "The `_geoPolygon` filter expects at least 2 points")?;
             }
-            ErrorKind::GeoPolygonNotPairs(number) => {
+            ErrorKind::GeoCoordinatesNotPair(number) => {
-                writeln!(f, "The `_geoPolygon` filter expects pairs of coordinates but found a group of {number} coordinates instead.")?
+                writeln!(f, "Expected coordinates in the form of a pair of numbers but found a group of {number} numbers instead.")?
             }
             ErrorKind::ReservedGeo(name) => {
                 writeln!(f, "`{}` is a reserved keyword and thus can't be used as a filter expression. Use the `_geoRadius(latitude, longitude, distance)` or `_geoBoundingBox([latitude, longitude], [latitude, longitude])` built-in rules to filter on `_geo` coordinates.", name.escape_debug())?

crates/filter-parser/src/lib.rs

@@ -424,26 +424,33 @@ fn parse_geo_radius(input: Span) -> IResult<FilterCondition> {
 /// If we parse `_geoBoundingBox` we MUST parse the rest of the expression.
 fn parse_geo_bounding_box(input: Span) -> IResult<FilterCondition> {
     // we want to allow space BEFORE the _geoBoundingBox but not after
-    let parsed = preceded(
+
-        tuple((multispace0, word_exact("_geoBoundingBox"))),
+    let (input, _) = tuple((multispace0, word_exact("_geoBoundingBox")))(input)?;
-        // if we were able to parse `_geoBoundingBox` and can't parse the rest of the input we return a failure
+
-        cut(delimited(
+    // if we were able to parse `_geoBoundingBox` and can't parse the rest of the input we return a failure
-            char('('),
+
-            separated_list1(
+    let (input, args) = delimited(
-                tag(","),
+        char('('),
-                ws(delimited(char('['), separated_list1(tag(","), ws(recognize_float)), char(']'))),
+        separated_list1(
-            ),
+            tag(","),
-            char(')'),
+            ws(delimited(char('['), separated_list1(tag(","), ws(recognize_float)), char(']'))),
-        )),
+        ),
+        char(')'),
     )(input)
-    .map_err(|e| e.map(|_| Error::new_from_kind(input, ErrorKind::GeoBoundingBox)));
+    .map_cut(ErrorKind::GeoBoundingBox)?;
 
-    let (input, args) = parsed?;
+    if args.len() != 2 {
-
-    if args.len() != 2 || args[0].len() != 2 || args[1].len() != 2 {
         return Err(Error::failure_from_kind(input, ErrorKind::GeoBoundingBox));
     }
 
+    if let Some(offending) = args.iter().find(|a| a.len() != 2) {
+        let context = offending.first().unwrap_or(&input);
+        return Err(Error::failure_from_kind(
+            *context,
+            ErrorKind::GeoCoordinatesNotPair(offending.len()),
+        ));
+    }
+
     let res = FilterCondition::GeoBoundingBox {
         top_right_point: [args[0][0].into(), args[0][1].into()],
         bottom_left_point: [args[1][0].into(), args[1][1].into()],
@@ -470,13 +477,17 @@ fn parse_geo_polygon(input: Span) -> IResult<FilterCondition> {
     )(input)
     .map_cut(ErrorKind::GeoPolygon)?;
 
-    if args.len() <= 2 || args.iter().any(|a| a.len() != 2) {
+    if args.len() < 2 {
-        let (number, context) = args
+        let context = args.last().and_then(|a| a.last()).unwrap_or(&input);
-            .iter()
+        return Err(Error::failure_from_kind(*context, ErrorKind::GeoPolygonTooFewPoints));
-            .find(|a| a.len() != 2)
+    }
-            .map(|a| (a.len(), a.first().unwrap_or(&input)))
+
-            .unwrap_or((args.len(), &input));
+    if let Some(offending) = args.iter().find(|a| a.len() != 2) {
-        return Err(Error::failure_from_kind(*context, ErrorKind::GeoPolygonNotPairs(number)));
+        let context = offending.first().unwrap_or(&input);
+        return Err(Error::failure_from_kind(
+            *context,
+            ErrorKind::GeoCoordinatesNotPair(offending.len()),
+        ));
     }
 
     let res = FilterCondition::GeoPolygon {
@@ -913,24 +924,24 @@ pub mod tests {
         1:16 _geoRadius = 12
         "###);
 
-        insta::assert_snapshot!(p("_geoBoundingBox"), @r###"
+        insta::assert_snapshot!(p("_geoBoundingBox"), @r"
         The `_geoBoundingBox` filter expects two pairs of arguments: `_geoBoundingBox([latitude, longitude], [latitude, longitude])`.
-        1:16 _geoBoundingBox
+        16:16 _geoBoundingBox
-        "###);
+        ");
 
-        insta::assert_snapshot!(p("_geoBoundingBox = 12"), @r###"
+        insta::assert_snapshot!(p("_geoBoundingBox = 12"), @r"
         The `_geoBoundingBox` filter expects two pairs of arguments: `_geoBoundingBox([latitude, longitude], [latitude, longitude])`.
-        1:21 _geoBoundingBox = 12
+        16:21 _geoBoundingBox = 12
-        "###);
+        ");
 
-        insta::assert_snapshot!(p("_geoBoundingBox(1.0, 1.0)"), @r###"
+        insta::assert_snapshot!(p("_geoBoundingBox(1.0, 1.0)"), @r"
         The `_geoBoundingBox` filter expects two pairs of arguments: `_geoBoundingBox([latitude, longitude], [latitude, longitude])`.
-        1:26 _geoBoundingBox(1.0, 1.0)
+        17:26 _geoBoundingBox(1.0, 1.0)
-        "###);
+        ");
 
         insta::assert_snapshot!(p("_geoPolygon([1,2,3])"), @r"
-        The `_geoPolygon` filter expects pairs of coordinates but found a group of 3 coordinates instead.
+        The `_geoPolygon` filter expects at least 2 points
-        14:15 _geoPolygon([1,2,3])
+        18:19 _geoPolygon([1,2,3])
         ");
 
         insta::assert_snapshot!(p("_geoPolygon(1,2,3)"), @r"
@@ -939,7 +950,7 @@ pub mod tests {
         ");
 
         insta::assert_snapshot!(p("_geoPolygon([1,2],[1,2,3])"), @r"
-        The `_geoPolygon` filter expects pairs of coordinates but found a group of 3 coordinates instead.
+        Expected coordinates in the form of a pair of numbers but found a group of 3 numbers instead.
         20:21 _geoPolygon([1,2],[1,2,3])
         ");