fix: point and word the playground's common syntax and alias errors clearly

crates/sqlquack-convert/src/snowflake/conversion/_helpers/queries/query.rs

@@ -4,7 +4,8 @@ use serde_json::{Map, Value, json};
 use sqlquack_blocks::blocks::main::quoted_identifier::quoted_identifier;
 use sqlquack_diagnostics::classification::models::{Construct, ConstructFamily};
 use sqlquack_diagnostics::diagnostics::constants::{
-    REPEATED_RESULT_NAME, UNALIASED_OUTPUT_NAME_CONSTRUCT,
+    REPEATED_RESULT_NAME, UNALIASED_OUTPUT_NAME_CONSTRUCT, UNALIASED_QUOTED_NAME,
+    UNALIASED_TEXT_NAME,
 };
 use sqlquack_diagnostics::provenance::constants::QUERY_CONSTRUCT;
 use sqlquack_diagnostics::provenance::models::ShapeArgument;
@@ -34,6 +35,7 @@ use crate::snowflake::conversion::_helpers::support::tree::{
     function_construct, identifier_name, list, node_construct, node_kind, payload_object,
     require_understood,
 };
+use crate::snowflake::conversion::constants::UNALIASED_QUOTED_KINDS;
 use crate::snowflake::conversion::constants::WINDOW_MARKER;
 use crate::snowflake::conversion::constants::{
     ALIAS, COMMENT, COMMON_TABLE, DISTINCT, GENERATOR_SEQUENCE_COLUMN, GROUP_BY, HAVING,
@@ -694,7 +696,10 @@ fn convert_projection(
                     location: site.context.locator.locate_written(item),
                     ..missing(
                         Construct::new(ConstructFamily::Syntax, UNALIASED_OUTPUT_NAME_CONSTRUCT),
-                        format!("Snowflake names this `{kind}` column after its text"),
+                        match UNALIASED_QUOTED_KINDS.contains(&kind) {
+                            true => UNALIASED_QUOTED_NAME.to_owned(),
+                            false => UNALIASED_TEXT_NAME.to_owned(),
+                        },
                     )
                 }),
             }

crates/sqlquack-convert/src/snowflake/conversion/constants.rs

@@ -1384,3 +1384,6 @@ pub(crate) const CORRELATED_DERIVED: &str =
 pub(crate) const AMBIGUOUS_SUFFIX: &str = "is ambiguous";
 /// A JSON-held VARIANT as a sort key, in ARRAY_SORT's recorded order of kinds.
 pub const VARIANT_ORDER_KEY: &str = "rule.variant_order_key";
+
+/// The select item kinds whose Snowflake name keeps quotes or a path: literals and paths into semi-structured values.
+pub(crate) const UNALIASED_QUOTED_KINDS: &[&str] = &["j_s_o_n_extract", "literal", "subscript"];

crates/sqlquack-diagnostics/src/diagnostics/constants.rs

@@ -12,6 +12,13 @@ pub const CASE_ONLY_MATCH_HELP: &str = "Snowflake folds unquoted names to upper
     quote the name in the SQL to keep its case";
 /// The words of an unresolved name's reason when two result columns share a name.
 pub const REPEATED_RESULT_NAME: &str = "repeats an earlier result column's name";
+/// The label of an unaliased item Snowflake names after its text.
+pub const UNALIASED_TEXT_NAME: &str = "Snowflake names this column after its text";
+/// The label of an unaliased literal or path, whose Snowflake name keeps its quotes or path, so no alias is suggested.
+pub const UNALIASED_QUOTED_NAME: &str =
+    "Snowflake names this column after its full text, quotes and path included";
+/// The help for an unaliased literal or path.
+pub const UNALIASED_QUOTED_NAME_HELP: &str = "add an alias, which names the column in both engines";
 /// The help for such a name, replacing SQ0004's own.
 pub const REPEATED_RESULT_NAME_HELP: &str =
     "give each result column its own alias, such as `SELECT 1 AS first, 1 AS second`";

crates/sqlquack-diagnostics/src/diagnostics/main/diagnose.rs

@@ -10,8 +10,8 @@ use crate::diagnostics::constants::{
     CASE_ONLY_MATCH, CASE_ONLY_MATCH_HELP, CONVERSION_BUDGET, DIVERGENCE_REFUSED,
     FUNCTION_OPERAND_HELP, GUARD_PREFIX, JINJA_HELP, MISSING_RULE, NOT_EXPRESSIBLE, PARSE_FIDELITY,
     REPEATED_RESULT_NAME, REPEATED_RESULT_NAME_HELP, SUGGESTED_NAME_MAX_CHARS, TYPE_CONDITION,
-    UNALIASED_OUTPUT_NAME, UNPARSED_PREFIX, UNRESOLVED_NAME, ZONED_LOCAL_TIME,
-    ZONED_LOCAL_TIME_HELP, ZONED_LOCAL_TIME_NOTE,
+    UNALIASED_OUTPUT_NAME, UNALIASED_QUOTED_NAME, UNALIASED_QUOTED_NAME_HELP, UNPARSED_PREFIX,
+    UNRESOLVED_NAME, ZONED_LOCAL_TIME, ZONED_LOCAL_TIME_HELP, ZONED_LOCAL_TIME_NOTE,
 };
 use crate::diagnostics::models::Diagnostic;
 use crate::refusals::models::{Refusal, RefusalCategory};
@@ -52,7 +52,9 @@ pub fn diagnose(refusals: &[Refusal], source: &str) -> Vec<Diagnostic> {
         {
             diagnostic.help = Some(FUNCTION_OPERAND_HELP.to_owned());
         }
-        if code == UNALIASED_OUTPUT_NAME
+        if code == UNALIASED_OUTPUT_NAME && refusal.reason.contains(UNALIASED_QUOTED_NAME) {
+            diagnostic.help = Some(UNALIASED_QUOTED_NAME_HELP.to_owned());
+        } else if code == UNALIASED_OUTPUT_NAME
             && let Some(name) = diagnostic
                 .span
                 .and_then(|span| suggested_name(source, span.start, span.end))

crates/sqlquack-syntax-snowflake/src/parser/_helpers/expressions/primary.rs

@@ -1,6 +1,7 @@
 //! Parser expressions: primary.
 
 use crate::parser::constants::ARRAY;
+use crate::parser::constants::CONDITION_CLAUSES;
 use crate::parser::constants::MAP;
 use crate::parser::constants::ODBC_DATE_ESCAPE;
 use crate::parser::constants::ODBC_TIMESTAMP_ESCAPE;
@@ -441,6 +442,14 @@ impl Parser {
             return self.maybe_parse_subscript(col);
         }
 
+        if self.is_at_end() || self.check(TokenType::Semicolon) {
+            let previous = self.previous().text.to_ascii_uppercase();
+            let expected = match CONDITION_CLAUSES.contains(&previous.as_str()) {
+                true => "a condition",
+                false => "an expression",
+            };
+            return Err(self.parse_error(format!("Expected {expected} after {previous}")));
+        }
         Err(self.parse_error(format!("Unexpected token: {:?}", self.peek().token_type)))
     }
 

crates/sqlquack-syntax-snowflake/src/parser/_helpers/statements/analyze.rs

@@ -122,7 +122,7 @@ impl Parser {
         false
     }
 
-    /// Whether a keyword may be an identifier: structural ones (FROM, WHERE, ...) may not.
+    /// Whether a keyword may be an identifier: structural ones (FROM, WHERE, ...) may not, nor GROUP or ORDER before BY, which start a clause.
     pub(crate) fn is_safe_keyword_as_identifier(&self) -> bool {
         if self.is_at_end() {
             return false;
@@ -156,8 +156,10 @@ impl Parser {
                 | TokenType::Lateral
                 | TokenType::Natural
         );
+        let starts_clause = matches!(token_type, TokenType::Group | TokenType::Order)
+            && self.check_next(TokenType::By);
 
-        self.peek().token_type.is_keyword() && !is_structural
+        self.peek().token_type.is_keyword() && !is_structural && !starts_clause
     }
 
     /// Whether the token here ends the expression, so IS or KEEP is an alias, not an operator.

crates/sqlquack-syntax-snowflake/src/parser/constants.rs

@@ -191,6 +191,9 @@ pub(crate) const SET: &str = "SET";
 /// The text "SET MASKING POLICY", as comparisons test it.
 pub(crate) const SET_MASKING_POLICY: &str = "SET MASKING POLICY";
 
+/// The clauses whose operand is a condition, as an error names what is missing after one.
+pub(crate) const CONDITION_CLAUSES: &[&str] = &["HAVING", "ON", "QUALIFY", "WHERE"];
+
 /// The text "SET TAG", as comparisons test it.
 pub(crate) const SET_TAG: &str = "SET TAG";
 

crates/sqlquack-syntax-snowflake/tests/parser.rs

@@ -8,6 +8,8 @@ mod test_fixes;
 mod test_located;
 #[path = "parser/test_lossless.rs"]
 mod test_lossless;
+#[path = "parser/test_parse_errors.rs"]
+mod test_parse_errors;
 #[path = "parser/test_scripting.rs"]
 mod test_scripting;
 #[path = "parser/test_snowflake_alter.rs"]

crates/sqlquack-syntax-snowflake/tests/parser/test_parse_errors.rs

@@ -0,0 +1,106 @@
+use sqlquack_syntax::{CallMode, Dialect, ParseOptions};
+use sqlquack_syntax_snowflake::parse_statements_with;
+
+use crate::test_types::{ParseErrorTestCase, ParseMessageTestCase};
+
+#[test]
+fn given_group_or_order_by_after_an_operator_when_parsing_snowflake_then_the_error_is_at_the_clause(
+) {
+    let test_cases = [
+        ParseErrorTestCase {
+            description: "AND before GROUP BY on the next line fails at GROUP, not at BY",
+            sql: "SELECT customer_id, SUM(total) AS revenue\nFROM orders\nWHERE status = 'shipped' AND\nGROUP BY customer_id",
+            expected_error_at: Some("GROUP"),
+        },
+        ParseErrorTestCase {
+            description: "AND before ORDER BY on the same line fails at ORDER",
+            sql: "SELECT order_id FROM orders WHERE total > 1 AND ORDER BY order_id",
+            expected_error_at: Some("ORDER"),
+        },
+        ParseErrorTestCase {
+            description: "NOT before GROUP BY fails at GROUP",
+            sql: "SELECT status FROM orders WHERE NOT\ngroup by status",
+            expected_error_at: Some("group"),
+        },
+        ParseErrorTestCase {
+            description: "GROUP BY and ORDER BY after WHERE still parse",
+            sql: "SELECT status FROM orders WHERE total > 1\nGROUP BY status\nORDER BY status",
+            expected_error_at: None,
+        },
+        ParseErrorTestCase {
+            description: "a quoted GROUP column still parses",
+            sql: "SELECT \"GROUP\" FROM orders ORDER BY 1",
+            expected_error_at: None,
+        },
+    ];
+
+    for test_case in test_cases {
+        let error_at = parse_statements_with(
+            test_case.sql,
+            Dialect::Snowflake,
+            &ParseOptions::default(),
+            CallMode::Faithful,
+        )
+        .err()
+        .map(|error| {
+            let start = error.start().unwrap_or_default();
+            let end = error.end().unwrap_or_default();
+            test_case.sql.get(start..end).unwrap_or_default().to_owned()
+        });
+        assert_eq!(
+            error_at.as_deref(),
+            test_case.expected_error_at,
+            "{}",
+            test_case.description
+        );
+    }
+}
+
+#[test]
+fn given_a_statement_ending_where_an_expression_belongs_when_parsing_snowflake_then_the_error_names_what_is_missing(
+) {
+    let test_cases = [
+        ParseMessageTestCase {
+            description: "an empty WHERE asks for a condition",
+            sql: "SELECT * FROM orders WHERE",
+            expected_message: "Expected a condition after WHERE",
+        },
+        ParseMessageTestCase {
+            description: "an empty WHERE before a semicolon asks for a condition",
+            sql: "SELECT * FROM orders WHERE ;",
+            expected_message: "Expected a condition after WHERE",
+        },
+        ParseMessageTestCase {
+            description: "an empty HAVING asks for a condition",
+            sql: "SELECT status FROM orders GROUP BY status HAVING",
+            expected_message: "Expected a condition after HAVING",
+        },
+        ParseMessageTestCase {
+            description: "a join's empty ON asks for a condition",
+            sql: "SELECT 1 FROM orders o JOIN customers c ON",
+            expected_message: "Expected a condition after ON",
+        },
+        ParseMessageTestCase {
+            description: "a comparison with no right side asks for an expression",
+            sql: "SELECT 1 FROM orders WHERE total >",
+            expected_message: "Expected an expression after >",
+        },
+    ];
+
+    for test_case in test_cases {
+        let message = parse_statements_with(
+            test_case.sql,
+            Dialect::Snowflake,
+            &ParseOptions::default(),
+            CallMode::Faithful,
+        )
+        .err()
+        .map(|error| error.to_string())
+        .unwrap_or_default();
+        assert!(
+            message.contains(test_case.expected_message),
+            "{}: {message}",
+            test_case.description
+        );
+    }
+}

crates/sqlquack-syntax-snowflake/tests/parser/test_types.rs

@@ -67,6 +67,18 @@ pub(crate) struct ScriptingVerdictTestCase {
     pub(crate) expected_error: Option<&'static str>,
 }
 
+pub(crate) struct ParseErrorTestCase {
+    pub(crate) description: &'static str,
+    pub(crate) sql: &'static str,
+    pub(crate) expected_error_at: Option<&'static str>,
+}
+
+pub(crate) struct ParseMessageTestCase {
+    pub(crate) description: &'static str,
+    pub(crate) sql: &'static str,
+    pub(crate) expected_message: &'static str,
+}
+
 pub(crate) struct ScriptingDialectTestCase {
     pub(crate) description: &'static str,
     pub(crate) sql: &'static str,

crates/sqlquack/tests/cli/test_convert.rs

@@ -1566,7 +1566,7 @@ fn given_refused_sql_when_converting_then_text_and_json_carry_the_same_coded_dia
             expected_text: concat!(
                 "error[SQ0009]: This column needs an alias, because SQLQuack can't be sure of the name ",
                 "Snowflake gives it\n --> <stdin>:1:18\n  |\n1 | SELECT 'a' AS a, UPPER('é')\n",
-                "  |                  ^^^^^^^^^^ Snowflake names this `function` column after its text\n",
+                "  |                  ^^^^^^^^^^ Snowflake names this column after its text\n",
                 "  |\n  = help: add an alias, such as `AS total`, which names the column in both engines\n",
                 "sqlquack convert: refused, 1 refusal\n",
             ),

crates/sqlquack/tests/cli/test_output_names.rs

@@ -489,8 +489,8 @@ fn given_a_refused_result_name_when_converting_then_the_diagnostic_underlines_th
             expected_text: concat!(
                 "error[SQ0009]: This column needs an alias, because SQLQuack can't be sure of the name ",
                 "Snowflake gives it\n --> <stdin>:1:18\n  |\n1 | SELECT order_id, 'it''s' FROM orders\n",
-                "  |                  ^^^^^^^ Snowflake names this `literal` column after its text\n",
-                "  |\n  = help: add an alias, such as `AS \"'IT''S'\"`, which names the column in both engines\n",
+                "  |                  ^^^^^^^ Snowflake names this column after its full text, quotes and ",
+                "path included\n  |\n  = help: add an alias, which names the column in both engines\n",
                 "sqlquack convert: refused, 1 refusal\n",
             ),
             expected_span: ("SQ0009", 17, 24),