very good, very good

Author: juleswritescode

Cargo.lock

@@ -14,13 +14,15 @@ version = "0.0.0"
 [dependencies]
 globset = "0.4.16"
 itertools = { version = "0.14.0" }
+once_cell = "1.20.2"
 pgt_console.workspace = true
 pgt_diagnostics.workspace = true
 pgt_query.workspace = true
 pgt_schema_cache.workspace = true
 pgt_text_size.workspace = true
 pgt_treesitter.workspace = true
 pgt_treesitter_grammar.workspace = true
+regex = "1.11.1"
 sqlx.workspace = true
 tokio.workspace = true
 tree-sitter.workspace = true

crates/pgt_typecheck/Cargo.toml

@@ -1,11 +1,13 @@
 use std::io;
 
+use once_cell::sync::Lazy;
 use pgt_console::markup;
 use pgt_diagnostics::{Advices, Diagnostic, LogCategory, MessageAndDescription, Severity, Visit};
 use pgt_text_size::{TextRange, TextSize};
+use regex::Regex;
 use sqlx::postgres::{PgDatabaseError, PgSeverity};
 
-use crate::{IdentifierType, TypedReplacement};
+use crate::typed_identifier::{IdentifierReplacement, TypedReplacement};
 
 /// A specialized diagnostic for the typechecker.
 ///
@@ -96,36 +98,65 @@ impl Advices for TypecheckAdvices {
  }
 }
 
-/// Finds the original type at the given position in the adjusted text
-fn find_type_at_position(
- adjusted_position: TextSize,
- type_info: &[(TextRange, IdentifierType)],
-) -> Option<&IdentifierType> {
- type_info
- .iter()
- .find(|(range, _)| range.contains(adjusted_position))
- .map(|(_, type_)| type_)
+/// Pattern and rewrite rule for error messages
+struct ErrorRewriteRule {
+ pattern: Regex,
+ rewrite: fn(&regex::Captures, &IdentifierReplacement) -> String,
 }
 
-/// Rewrites error messages to show the original type name instead of the replaced literal value
-fn rewrite_error_message(original_message: &str, identifier_type: &IdentifierType) -> String {
- // pattern: invalid input syntax for type X: "literal_value"
- // we want to replace "literal_value" with the type name
-
- if let Some(colon_pos) = original_message.rfind(": ") {
- let before_value = &original_message[..colon_pos];
-
- // build the type name, including schema if present
- let type_name = if let Some(schema) = &identifier_type.schema {
- format!("{}.{}", schema, identifier_type.name)
- } else {
- identifier_type.name.clone()
- };
+static ERROR_RULES: Lazy<Vec<ErrorRewriteRule>> = Lazy::new(|| {
+ vec![
+ ErrorRewriteRule {
+ pattern: Regex::new(r#"invalid input syntax for type (\w+): "([^"]*)""#).unwrap(),
+ rewrite: |caps, replacement| {
+ let expected_type = &caps[1];
+ format!(
+ "`{}` is of type {}, not {}",
+ replacement.original_name, replacement.type_name, expected_type
+ )
+ },
+ },
+ ErrorRewriteRule {
+ pattern: Regex::new(
+ r#"column "([^"]*)" is of type (\w+) but expression is of type (\w+)"#,
+ )
+ .unwrap(),
+ rewrite: |caps, replacement| {
+ let column = &caps[1];
+ let expected_type = &caps[2];
+ format!(
+ "column `{}` expects {}, but `{}` is of type {}",
+ column, expected_type, replacement.original_name, replacement.type_name
+ )
+ },
+ },
+ ErrorRewriteRule {
+ pattern: Regex::new(r#"operator does not exist: (.+)"#).unwrap(),
+ rewrite: |caps, replacement| {
+ let operator_expr = &caps[1];
+ format!(
+ "operator does not exist: {} (parameter `{}` is of type {})",
+ operator_expr, replacement.original_name, replacement.type_name
+ )
+ },
+ },
+ ]
+});
 
- format!("{}: {}", before_value, type_name)
- } else {
- original_message.to_string()
+/// Rewrites Postgres error messages to be more user-friendly
+fn rewrite_error_message(pg_error_message: &str, replacement: &IdentifierReplacement) -> String {
+ // try each rule
+ for rule in ERROR_RULES.iter() {
+ if let Some(caps) = rule.pattern.captures(pg_error_message) {
+ return (rule.rewrite)(&caps, replacement);
+ }
  }
+
+ // fallback: generic value replacement
+ let unquoted_default = replacement.default_value.trim_matches('\'');
+ pg_error_message
+ .replace(&format!("\"{}\"", unquoted_default), &replacement.type_name)
+ .replace(&format!("'{}'", unquoted_default), &replacement.type_name)
 }
 
 pub(crate) fn create_type_error(
@@ -138,11 +169,17 @@ pub(crate) fn create_type_error(
  _ => None,
  });
 
- let range = position.and_then(|pos| {
- let adjusted = typed_replacement.replacement.to_original_position(TextSize::new(pos.try_into().unwrap()));
+ let original_position = position.map(|p| {
+ let pos = TextSize::new(p.try_into().unwrap());
+
+ typed_replacement
+ .text_replacement()
+ .to_original_position(pos)
+ });
 
+ let range = original_position.and_then(|pos| {
  ts.root_node()
- .named_descendant_for_byte_range(adjusted.into(), adjusted.into())
+ .named_descendant_for_byte_range(pos.into(), pos.into())
  .map(|node| {
  TextRange::new(
  node.start_byte().try_into().unwrap(),
@@ -162,11 +199,9 @@ pub(crate) fn create_type_error(
  PgSeverity::Log => Severity::Information,
  };
 
- // check if the error position corresponds to a replaced parameter
- let message = if let Some(pos) = position {
- let adjusted_pos = TextSize::new(pos.try_into().unwrap());
- if let Some(original_type) = find_type_at_position(adjusted_pos, &typed_replacement.type_info) {
- rewrite_error_message(pg_err.message(), original_type)
+ let message = if let Some(pos) = original_position {
+ if let Some(replacement) = typed_replacement.find_type_at_position(pos) {
+ rewrite_error_message(pg_err.message(), replacement)
  } else {
  pg_err.to_string()
  }

crates/pgt_typecheck/src/diagnostics.rs

@@ -10,7 +10,7 @@ use sqlx::postgres::PgDatabaseError;
 pub use sqlx::postgres::PgSeverity;
 use sqlx::{Executor, PgPool};
 use typed_identifier::apply_identifiers;
-pub use typed_identifier::{IdentifierType, TypedIdentifier, TypedReplacement};
+pub use typed_identifier::{IdentifierType, TypedIdentifier};
 
 #[derive(Debug)]
 pub struct TypecheckParams<'a> {
@@ -68,13 +68,19 @@ pub async fn check_sql(
  conn.execute(&*search_path_query).await?;
  }
 
- let res = conn.prepare(typed_replacement.replacement.text()).await;
+ let res = conn
+ .prepare(typed_replacement.text_replacement().text())
+ .await;
 
  match res {
  Ok(_) => Ok(None),
  Err(sqlx::Error::Database(err)) => {
  let pg_err = err.downcast_ref::<PgDatabaseError>();
- Ok(Some(create_type_error(pg_err, params.tree, typed_replacement)))
+ Ok(Some(create_type_error(
+ pg_err,
+ params.tree,
+ typed_replacement,
+ )))
  }
  Err(err) => Err(err),
  }

crates/pgt_typecheck/src/lib.rs

@@ -1,5 +1,5 @@
 use pgt_schema_cache::PostgresType;
-use pgt_text_size::{TextRange, TextRangeReplacement, TextRangeReplacementBuilder};
+use pgt_text_size::{TextRange, TextRangeReplacement, TextRangeReplacementBuilder, TextSize};
 use pgt_treesitter::queries::{ParameterMatch, TreeSitterQueriesExecutor};
 
 /// It is used to replace parameters within the SQL string.
@@ -21,11 +21,52 @@ pub struct IdentifierType {
  pub is_array: bool,
 }
 
+#[derive(Debug)]
+pub(crate) struct IdentifierReplacement {
+ pub original_name: String,
+ pub original_range: std::ops::Range<usize>,
+ /// The default value with which the identifier was replaced, e.g. `''` for a TEXT param.
+ pub default_value: String,
+ pub type_name: String,
+}
+
 /// Contains the text replacement along with metadata about which ranges correspond to which types.
 #[derive(Debug)]
-pub struct TypedReplacement {
- pub replacement: TextRangeReplacement,
- pub type_info: Vec<(TextRange, IdentifierType)>,
+pub(crate) struct TypedReplacement {
+ text_replacement: TextRangeReplacement,
+ identifier_replacements: Vec<IdentifierReplacement>,
+}
+
+impl TypedReplacement {
+ pub fn new(sql: &str, replacements: Vec<IdentifierReplacement>) -> Self {
+ let mut text_range_replacement_builder = TextRangeReplacementBuilder::new(sql);
+
+ for replacement in &replacements {
+ let text_range: TextRange = replacement.original_range.clone().try_into().unwrap();
+ text_range_replacement_builder.replace_range(text_range, &replacement.default_value);
+ }
+
+ Self {
+ identifier_replacements: replacements,
+ text_replacement: text_range_replacement_builder.build(),
+ }
+ }
+
+ /// Finds the original type at the given position in the adjusted text
+ pub(crate) fn find_type_at_position(
+ &self,
+ original_position: TextSize,
+ ) -> Option<&IdentifierReplacement> {
+ self.identifier_replacements.iter().find(|replacement| {
+ replacement
+ .original_range
+ .contains(&original_position.try_into().unwrap())
+ })
+ }
+
+ pub(crate) fn text_replacement(&self) -> &TextRangeReplacement {
+ &self.text_replacement
+ }
 }
 
 /// Applies the identifiers to the SQL string by replacing them with their default values.
@@ -40,7 +81,7 @@ pub fn apply_identifiers<'a>(
  executor.add_query_results::<ParameterMatch>();
 
  // Collect all replacements first to avoid modifying the string while iterating
- let replacements: Vec<_> = executor
+ let replacements: Vec<IdentifierReplacement> = executor
  .get_iter(None)
  .filter_map(|q| {
  let m: &ParameterMatch = q.try_into().ok()?;
@@ -51,28 +92,23 @@ pub fn apply_identifiers<'a>(
  let (identifier, position) = find_matching_identifier(&parts, &identifiers)?;
 
  // Resolve the type based on whether we're accessing a field of a composite type
- let type_ = resolve_type(identifier, position, &parts, schema_cache)?;
-
- Some((m.get_byte_range(), type_, identifier.type_.clone()))
- })
- .collect();
+ let postgres_type = resolve_type(identifier, position, &parts, schema_cache)?;
 
- let mut text_range_replacement_builder = TextRangeReplacementBuilder::new(sql);
- let mut type_info = vec![];
+  let default_value =
+  get_formatted_default_value(postgres_type, identifier.type_.is_array);
 
- for (range, type_, original_type) in replacements {
- let default_value = get_formatted_default_value(type_, original_type.is_array);
+ let replacement = IdentifierReplacement {
+ default_value,
+ original_name: identifier.name.clone().unwrap_or("".into()),
+ original_range: m.get_byte_range(),
+ type_name: identifier.type_.name.clone(),
+ };
 
- text_range_replacement_builder
- .replace_range(range.clone().try_into().unwrap(), &default_value);
-
- type_info.push((range.try_into().unwrap(), original_type));
- }
+ Some(replacement)
+ })
+ .collect();
 
- TypedReplacement {
- replacement: text_range_replacement_builder.build(),
- type_info,
- }
+ TypedReplacement::new(sql, replacements)
 }
 
 /// Format the default value based on the type and whether it's an array
@@ -320,7 +356,7 @@ mod tests {
  let replacement = super::apply_identifiers(identifiers, &schema_cache, &tree, input);
 
  assert_eq!(
- replacement.replacement.text(),
+ replacement.text_replacement.text(),
  // the numeric parameters are filled with 0;
  "select 0 + 0 + 0 + 0 + 0 + 'critical'"
  );
@@ -370,7 +406,7 @@ mod tests {
  let replacement = super::apply_identifiers(identifiers, &schema_cache, &tree, input);
 
  assert_eq!(
- replacement.replacement.text(),
+ replacement.text_replacement.text(),
  r#"select id from auth.users where email_change_confirm_status = '00000000-0000-0000-0000-000000000000' and email = '';"#
  );
  }

crates/pgt_typecheck/src/typed_identifier.rs

@@ -4,4 +4,4 @@ expression: content
 ---
 delete from public.contacts where id = ~~~uid~~~;
 
-invalid input syntax for type integer: uuid
+`uid` is of type uuid, not integer

crates/pgt_typecheck/tests/snapshots/invalid_type_in_function_longer_default.snap

@@ -4,4 +4,4 @@ expression: content
 ---
 delete from public.contacts where id = ~~~contact_name~~~;
 
-invalid input syntax for type integer: text
+`contact_name` is of type text, not integer