From 8d06e7f1cf757dd0702aa647590a6c1ff26bdf12 Mon Sep 17 00:00:00 2001 From: Pete Gadomski Date: Wed, 2 Oct 2024 05:42:10 -0600 Subject: [PATCH 1/5] refactor: s/to_cql2_text/to_text/ It's not `to_cql2_json`, so we should align the names. --- src/bin/cql2text.rs | 2 +- src/lib.rs | 29 +++++++++++------------------ tests/ogc_tests.rs | 4 ++-- 3 files changed, 14 insertions(+), 21 deletions(-) diff --git a/src/bin/cql2text.rs b/src/bin/cql2text.rs index 1183890..41ae10f 100644 --- a/src/bin/cql2text.rs +++ b/src/bin/cql2text.rs @@ -2,5 +2,5 @@ use cql2::parse_stdin; fn main() { let parsed = parse_stdin().unwrap(); - println!("{}", parsed.to_cql2_text().unwrap()); + println!("{}", parsed.to_text().unwrap()); } diff --git a/src/lib.rs b/src/lib.rs index c5794ad..9c2e17c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -147,9 +147,9 @@ impl Expr { /// use cql2::Expr; /// /// let expr = Expr::Bool(true); - /// assert_eq!(expr.to_cql2_text().unwrap(), "true"); + /// assert_eq!(expr.to_text().unwrap(), "true"); /// ``` - pub fn to_cql2_text(&self) -> Result { + pub fn to_text(&self) -> Result { Ok(match self { Expr::Bool(v) => v.to_string(), Expr::Float(v) => v.to_string(), @@ -157,27 +157,22 @@ impl Expr { Expr::Property { property } => format!("\"{property}\""), Expr::Interval { interval } => format!( "INTERVAL({},{})", - interval[0].to_cql2_text()?, - interval[1].to_cql2_text()? + interval[0].to_text()?, + interval[1].to_text()? ), - Expr::Date { date } => format!("DATE({})", date.to_cql2_text()?), - Expr::Timestamp { timestamp } => format!("TIMESTAMP({})", timestamp.to_cql2_text()?), + Expr::Date { date } => format!("DATE({})", date.to_text()?), + Expr::Timestamp { timestamp } => format!("TIMESTAMP({})", timestamp.to_text()?), Expr::Geometry(v) => { let gj = GeoJsonString(v.to_string()); gj.to_wkt()? } Expr::Array(v) => { - let array_els: Vec = v - .iter() - .map(|a| a.to_cql2_text()) - .collect::>()?; + let array_els: Vec = + v.iter().map(|a| a.to_text()).collect::>()?; format!("({})", array_els.join(", ")) } Expr::Operation { op, args } => { - let a: Vec = args - .iter() - .map(|x| x.to_cql2_text()) - .collect::>()?; + let a: Vec = args.iter().map(|x| x.to_text()).collect::>()?; match op.as_str() { "and" => format!("({})", a.join(" AND ")), "or" => format!("({})", a.join(" OR ")), @@ -191,10 +186,8 @@ impl Expr { } } Expr::BBox { bbox } => { - let array_els: Vec = bbox - .iter() - .map(|a| a.to_cql2_text()) - .collect::>()?; + let array_els: Vec = + bbox.iter().map(|a| a.to_text()).collect::>()?; format!("BBOX({})", array_els.join(", ")) } }) diff --git a/tests/ogc_tests.rs b/tests/ogc_tests.rs index f2e7891..d4c7c6b 100644 --- a/tests/ogc_tests.rs +++ b/tests/ogc_tests.rs @@ -7,7 +7,7 @@ use std::path::{Path, PathBuf}; fn validate_str(s: &str) -> Expr { let expr = cql2::parse(s).unwrap(); assert!(expr.is_valid()); - let expr_from_txt = cql2::parse(&expr.to_cql2_text().unwrap()).unwrap(); + let expr_from_txt = cql2::parse(&expr.to_text().unwrap()).unwrap(); assert!(expr_from_txt.is_valid()); let json = expr.to_json().unwrap(); let expr_from_json = cql2::parse(&json).unwrap(); @@ -27,7 +27,7 @@ fn validate_path(path: impl AsRef) { assert_eq!(json.trim(), expr.to_json().unwrap()); let text = std::fs::read_to_string(expected.join(file_name).with_extension("txt")).unwrap(); - assert_eq!(text.trim(), expr.to_cql2_text().unwrap()); + assert_eq!(text.trim(), expr.to_text().unwrap()); } #[rstest] From 011fe03014943ce93bedfed0f58af946072f0335 Mon Sep 17 00:00:00 2001 From: Pete Gadomski Date: Wed, 2 Oct 2024 05:43:39 -0600 Subject: [PATCH 2/5] nit: remove unneeded `as_str` --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 9c2e17c..f08e6ed 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -153,7 +153,7 @@ impl Expr { Ok(match self { Expr::Bool(v) => v.to_string(), Expr::Float(v) => v.to_string(), - Expr::Literal(v) => format!("'{}'", v.as_str()), + Expr::Literal(v) => format!("'{}'", v), Expr::Property { property } => format!("\"{property}\""), Expr::Interval { interval } => format!( "INTERVAL({},{})", From 4f395a3e3b628b02751450e9511dfdfb0ffd22fc Mon Sep 17 00:00:00 2001 From: Pete Gadomski Date: Wed, 2 Oct 2024 06:04:51 -0600 Subject: [PATCH 3/5] feat: check arg lengths when going to text --- src/lib.rs | 92 ++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 68 insertions(+), 24 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index f08e6ed..606cfbb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -25,10 +25,22 @@ pub enum Error { #[error(transparent)] BoonCompile(#[from] boon::CompileError), + /// [geozero::error::GeozeroError] + #[error(transparent)] + Geozero(#[from] geozero::error::GeozeroError), + /// Invalid CQL2 text #[error("invalid cql2-text: {0}")] InvalidCql2Text(String), + /// Invalid number of arguments for the expression + #[error("invalid number of arguments for {name}: {actual} (expected {expected})")] + InvalidNumberOfArguments { + name: String, + actual: usize, + expected: usize, + }, + /// [std::io::Error] #[error(transparent)] Io(#[from] std::io::Error), @@ -149,48 +161,80 @@ impl Expr { /// let expr = Expr::Bool(true); /// assert_eq!(expr.to_text().unwrap(), "true"); /// ``` - pub fn to_text(&self) -> Result { - Ok(match self { - Expr::Bool(v) => v.to_string(), - Expr::Float(v) => v.to_string(), - Expr::Literal(v) => format!("'{}'", v), - Expr::Property { property } => format!("\"{property}\""), - Expr::Interval { interval } => format!( - "INTERVAL({},{})", - interval[0].to_text()?, - interval[1].to_text()? - ), - Expr::Date { date } => format!("DATE({})", date.to_text()?), - Expr::Timestamp { timestamp } => format!("TIMESTAMP({})", timestamp.to_text()?), + pub fn to_text(&self) -> Result { + macro_rules! check_len { + ($name:expr, $args:expr, $len:expr, $text:expr) => { + if $args.len() == $len { + Ok($text) + } else { + Err(Error::InvalidNumberOfArguments { + name: $name.to_string(), + actual: $args.len(), + expected: $len, + }) + } + }; + } + + match self { + Expr::Bool(v) => Ok(v.to_string()), + Expr::Float(v) => Ok(v.to_string()), + Expr::Literal(v) => Ok(format!("'{}'", v)), + Expr::Property { property } => Ok(format!("\"{property}\"")), + Expr::Interval { interval } => { + check_len!( + "interval", + interval, + 2, + format!( + "INTERVAL({},{})", + interval[0].to_text()?, + interval[1].to_text()? + ) + ) + } + Expr::Date { date } => Ok(format!("DATE({})", date.to_text()?)), + Expr::Timestamp { timestamp } => Ok(format!("TIMESTAMP({})", timestamp.to_text()?)), Expr::Geometry(v) => { let gj = GeoJsonString(v.to_string()); - gj.to_wkt()? + gj.to_wkt().map_err(Error::from) } Expr::Array(v) => { let array_els: Vec = v.iter().map(|a| a.to_text()).collect::>()?; - format!("({})", array_els.join(", ")) + Ok(format!("({})", array_els.join(", "))) } Expr::Operation { op, args } => { let a: Vec = args.iter().map(|x| x.to_text()).collect::>()?; match op.as_str() { - "and" => format!("({})", a.join(" AND ")), - "or" => format!("({})", a.join(" OR ")), - "between" => format!("({} BETWEEN {} AND {})", a[0], a[1], a[2]), - "not" => format!("(NOT {})", a[0]), - "is null" => format!("({} IS NULL)", a[0]), + "and" => Ok(format!("({})", a.join(" AND "))), + "or" => Ok(format!("({})", a.join(" OR "))), + "between" => { + check_len!( + "between", + a, + 3, + format!("({} BETWEEN {} AND {})", a[0], a[1], a[2]) + ) + } + "not" => { + check_len!("not", a, 1, format!("(NOT {})", a[0])) + } + "is null" => { + check_len!("is null", a, 1, format!("({} IS NULL)", a[0])) + } "+" | "-" | "*" | "/" | "%" | "^" | "=" | "<=" | "<" | "<>" | ">" | ">=" => { - format!("({} {} {})", a[0], op, a[1]) + check_len!(op, a, 2, format!("({} {} {})", a[0], op, a[1])) } - _ => format!("{}({})", op, a.join(", ")), + _ => Ok(format!("{}({})", op, a.join(", "))), } } Expr::BBox { bbox } => { let array_els: Vec = bbox.iter().map(|a| a.to_text()).collect::>()?; - format!("BBOX({})", array_els.join(", ")) + Ok(format!("BBOX({})", array_els.join(", "))) } - }) + } } /// Converts this expression to a [SqlQuery] struct with parameters From 0d0c1929b17cf15c99a93e6ccada651c79881cb8 Mon Sep 17 00:00:00 2001 From: Pete Gadomski Date: Wed, 2 Oct 2024 06:06:57 -0600 Subject: [PATCH 4/5] refactor: simplify normalize_op --- src/lib.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 606cfbb..5ba2987 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -430,12 +430,12 @@ lazy_static::lazy_static! { } fn normalize_op(op: &str) -> String { - let oper = op.to_lowercase(); - let operator: &str = match oper.as_str() { - "eq" => "=", - _ => &oper, - }; - operator.to_string() + let op = op.to_lowercase(); + if op == "eq" { + "=".to_string() + } else { + op + } } fn strip_quotes(quoted_string: &str) -> String { From 1750c3929a0a7c86d61b1e2730686c042b54afee Mon Sep 17 00:00:00 2001 From: Pete Gadomski Date: Wed, 2 Oct 2024 06:10:43 -0600 Subject: [PATCH 5/5] refactor: work with chars in strip_quotes --- src/lib.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 5ba2987..b749d5d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -438,15 +438,11 @@ fn normalize_op(op: &str) -> String { } } -fn strip_quotes(quoted_string: &str) -> String { - let len = quoted_string.len(); - let bytes = quoted_string.as_bytes(); - if (bytes[0] == b'"' && bytes[len - 1] == b'"') - || (bytes[0] == b'\'' && bytes[len - 1] == b'\'') - { - quoted_string[1..len - 1].to_string() +fn strip_quotes(s: &str) -> &str { + if (s.starts_with('"') && s.ends_with('"')) || (s.starts_with('\'') && s.ends_with('\'')) { + &s[1..s.len() - 1] } else { - quoted_string.to_string() + s } } @@ -470,13 +466,13 @@ fn parse_expr(expression_pairs: Pairs<'_, Rule>) -> Expr { .parse::() .expect("Could not cast value to float"), ), - Rule::SingleQuotedString => Expr::Literal(strip_quotes(primary.as_str())), + Rule::SingleQuotedString => Expr::Literal(strip_quotes(primary.as_str()).to_string()), Rule::True | Rule::False => { let bool_value = primary.as_str().to_lowercase().parse::().unwrap(); Expr::Bool(bool_value) } Rule::Identifier => Expr::Property { - property: strip_quotes(primary.as_str()), + property: strip_quotes(primary.as_str()).to_string(), }, Rule::GEOMETRY => { let geom_wkt = Wkt(primary.as_str());