diff --git a/crates/filter-parser/src/error.rs b/crates/filter-parser/src/error.rs index 0afe981d2..aec008977 100644 --- a/crates/filter-parser/src/error.rs +++ b/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) => { - writeln!(f, "The `_geoPolygon` filter expects pairs of coordinates but found a group of {number} coordinates instead.")? + ErrorKind::GeoCoordinatesNotPair(number) => { + 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())? diff --git a/crates/filter-parser/src/lib.rs b/crates/filter-parser/src/lib.rs index 939928edf..bc11f7756 100644 --- a/crates/filter-parser/src/lib.rs +++ b/crates/filter-parser/src/lib.rs @@ -424,26 +424,33 @@ fn parse_geo_radius(input: Span) -> IResult { /// If we parse `_geoBoundingBox` we MUST parse the rest of the expression. fn parse_geo_bounding_box(input: Span) -> IResult { // we want to allow space BEFORE the _geoBoundingBox but not after - let parsed = preceded( - tuple((multispace0, word_exact("_geoBoundingBox"))), - // if we were able to parse `_geoBoundingBox` and can't parse the rest of the input we return a failure - cut(delimited( - char('('), - separated_list1( - tag(","), - ws(delimited(char('['), separated_list1(tag(","), ws(recognize_float)), char(']'))), - ), - char(')'), - )), + + 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 + + let (input, args) = delimited( + char('('), + separated_list1( + tag(","), + 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 || args[0].len() != 2 || args[1].len() != 2 { + if args.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 { )(input) .map_cut(ErrorKind::GeoPolygon)?; - if args.len() <= 2 || args.iter().any(|a| a.len() != 2) { - let (number, context) = args - .iter() - .find(|a| a.len() != 2) - .map(|a| (a.len(), a.first().unwrap_or(&input))) - .unwrap_or((args.len(), &input)); - return Err(Error::failure_from_kind(*context, ErrorKind::GeoPolygonNotPairs(number))); + if args.len() < 2 { + let context = args.last().and_then(|a| a.last()).unwrap_or(&input); + return Err(Error::failure_from_kind(*context, ErrorKind::GeoPolygonTooFewPoints)); + } + + 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::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. - 14:15 _geoPolygon([1,2,3]) + The `_geoPolygon` filter expects at least 2 points + 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]) ");