parser: upgrade grammar with various fixes (#716)

Author: sbdchd

PLAN.md

@@ -47,11 +47,15 @@ support SQL embedded in other languages
 
 parse and warn, helps with copy pasting examples
 
+https://www.enterprisedb.com/blog/psqls-scripting-language-turing-complete-or-fibonacci-psql
+
 ### Other Dialects
 
 support Trino, BigQuery, Aurora DSLQ, etc.
 
-### Check `format()` calls
+### Validations
+
+#### Check `format()` calls
 
 https://www.postgresql.org/docs/18/functions-string.html#FUNCTIONS-STRING-FORMAT
 
@@ -63,6 +67,47 @@ SELECT format('Hello %s %s', 'World');
 -- error                ^^
 ```
 
+#### Warn about invalid column notation
+
+```sql
+with t as (select 1 as data)
+select (data).id from t;
+```
+
+postgres says:
+
+```
+Query 1 ERROR at Line 40: : ERROR:  column notation .id applied to type integer, which is not a composite type
+LINE 2: select (data).id from t;
+                ^
+```
+
+#### Warn about invalid missing column notation
+
+```sql
+create type f as (
+  id integer,
+  name text
+);
+with t as (select (1, 'a')::f as data)
+select data.id from t;
+```
+
+postgres says:
+
+```
+Query 1 ERROR at Line 41: : ERROR:  missing FROM-clause entry for table "data"
+LINE 2: select data.id from t;
+               ^
+```
+
+we should suggest an autofix to:
+
+```sql
+with t as (select (1, 'a')::f as data)
+select (data).id from t;
+```
+
 ### Formatter
 
 ```shell
@@ -137,6 +182,10 @@ sql for benchmarks maybe?
 
   https://github.com/cedardb/DOOMQL
 
+- rrule_plpgsql
+
+  https://github.com/sirrodgepodge/rrule_plpgsql
+
 ### CLI
 
 from `deno`
@@ -589,7 +638,7 @@ via: https://www.postgresql.org/docs/17/sql-createview.html
 
 https://www.postgresql.org/docs/17/sql-dropgroup.html
 
-### Rule: `create table as` to `select into`
+### Rule: `select into` to `create table as`
 
 ```sql
 SELECT * INTO films_recent FROM films WHERE date_prod >= '2002-01-01';

crates/squawk_ide/src/expand_selection.rs

@@ -40,12 +40,19 @@ const DELIMITED_LIST_KINDS: &[SyntaxKind] = &[
     SyntaxKind::COLUMN_LIST,
     SyntaxKind::CONFLICT_INDEX_ITEM_LIST,
     SyntaxKind::CONSTRAINT_EXCLUSION_LIST,
+    SyntaxKind::DROP_OP_CLASS_OPTION_LIST,
+    SyntaxKind::FDW_OPTION_LIST,
     SyntaxKind::FUNCTION_SIG_LIST,
     SyntaxKind::GROUP_BY_LIST,
     SyntaxKind::JSON_TABLE_COLUMN_LIST,
+    SyntaxKind::OPERATOR_CLASS_OPTION_LIST,
     SyntaxKind::OPTION_ITEM_LIST,
+    SyntaxKind::OP_SIG_LIST,
     SyntaxKind::PARAM_LIST,
     SyntaxKind::PARTITION_ITEM_LIST,
+    SyntaxKind::RETURNING_OPTION_LIST,
+    SyntaxKind::REVOKE_COMMAND_LIST,
+    SyntaxKind::ROLE_LIST,
     SyntaxKind::ROW_LIST,
     SyntaxKind::SET_COLUMN_LIST,
     SyntaxKind::SET_EXPR_LIST,
@@ -540,9 +547,11 @@ $0
     #[test]
     fn list_variants() {
         let delimited_ws_list_kinds = &[
+            SyntaxKind::CREATE_DATABASE_OPTION_LIST,
             SyntaxKind::FUNC_OPTION_LIST,
             SyntaxKind::ROLE_OPTION_LIST,
             SyntaxKind::SEQUENCE_OPTION_LIST,
+            SyntaxKind::TRIGGER_EVENT_LIST,
             SyntaxKind::XML_COLUMN_OPTION_LIST,
             SyntaxKind::WHEN_CLAUSE_LIST,
         ];

crates/squawk_linter/src/analyze.rs

@@ -205,6 +205,7 @@ pub fn possibly_slow_stmt(stmt: &ast::Stmt) -> bool {
         | ast::Stmt::SetConstraints(_)
         | ast::Stmt::SetRole(_)
         | ast::Stmt::SetSessionAuth(_)
+        | ast::Stmt::ResetSessionAuth(_)
         | ast::Stmt::SetTransaction(_)
         | ast::Stmt::Show(_)
         | ast::Stmt::Table(_)

crates/squawk_linter/src/ignore.rs

@@ -22,20 +22,19 @@ fn comment_body(token: &SyntaxToken) -> Option<(&str, TextRange)> {
     let range = token.text_range();
     if token.kind() == SyntaxKind::COMMENT {
         let text = token.text();
-        if let Some(trimmed) = text.strip_prefix("--") {
-            if let Some(start) = range.start().checked_add(2.into()) {
-                let end = range.end();
-                let updated_range = TextRange::new(start, end);
-                return Some((trimmed, updated_range));
-            }
+        if let Some(trimmed) = text.strip_prefix("--")
+            && let Some(start) = range.start().checked_add(2.into())
+        {
+            let end = range.end();
+            let updated_range = TextRange::new(start, end);
+            return Some((trimmed, updated_range));
         }
-        if let Some(trimmed) = text.strip_prefix("/*").and_then(|x| x.strip_suffix("*/")) {
-            if let Some(start) = range.start().checked_add(2.into()) {
-                if let Some(end) = range.end().checked_sub(2.into()) {
-                    let updated_range = TextRange::new(start, end);
-                    return Some((trimmed, updated_range));
-                }
-            }
+        if let Some(trimmed) = text.strip_prefix("/*").and_then(|x| x.strip_suffix("*/"))
+            && let Some(start) = range.start().checked_add(2.into())
+            && let Some(end) = range.end().checked_sub(2.into())
+        {
+            let updated_range = TextRange::new(start, end);
+            return Some((trimmed, updated_range));
         }
     }
     None

crates/squawk_linter/src/rules/adding_field_with_default.rs

@@ -23,10 +23,10 @@ fn is_non_volatile_or_const(expr: &ast::Expr) -> bool {
         ast::Expr::Literal(_) => true,
         ast::Expr::ArrayExpr(_) => true,
         ast::Expr::BinExpr(bin_expr) => {
-            if let Some(lhs) = bin_expr.lhs() {
-                if let Some(rhs) = bin_expr.rhs() {
-                    return is_non_volatile_or_const(&lhs) && is_non_volatile_or_const(&rhs);
-                }
+            if let Some(lhs) = bin_expr.lhs()
+                && let Some(rhs) = bin_expr.rhs()
+            {
+                return is_non_volatile_or_const(&lhs) && is_non_volatile_or_const(&rhs);
             }
             false
         }

crates/squawk_linter/src/rules/ban_char_field.rs

@@ -40,15 +40,14 @@ fn check_path_type(ctx: &mut Linter, path_type: ast::PathType) {
         .path()
         .and_then(|x| x.segment())
         .and_then(|x| x.name_ref())
+        && is_char_type(name_ref.text())
     {
-        if is_char_type(name_ref.text()) {
-            let fix = create_fix(name_ref.syntax().text_range(), path_type.arg_list());
-            ctx.report(Violation::for_node(
-                Rule::BanCharField,
-                "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.".into(),
-                path_type.syntax(),
-            ).fix(Some(fix)));
-        }
+        let fix = create_fix(name_ref.syntax().text_range(), path_type.arg_list());
+        ctx.report(Violation::for_node(
+            Rule::BanCharField,
+            "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.".into(),
+            path_type.syntax(),
+        ).fix(Some(fix)));
     }
 }
 

crates/squawk_linter/src/rules/constraint_missing_not_valid.rs

@@ -67,13 +67,11 @@ fn not_valid_validate_in_transaction(
                             }
                         }
                         ast::AlterTableAction::AddConstraint(add_constraint) => {
-                            if add_constraint.not_valid().is_some() {
-                                if let Some(constraint) = add_constraint.constraint() {
-                                    if let Some(constraint_name) = constraint.name() {
-                                        not_valid_names
-                                            .insert(Identifier::new(&constraint_name.text()));
-                                    }
-                                }
+                            if add_constraint.not_valid().is_some()
+                                && let Some(constraint) = add_constraint.constraint()
+                                && let Some(constraint_name) = constraint.name()
+                            {
+                                not_valid_names.insert(Identifier::new(&constraint_name.text()));
                             }
                         }
                         _ => (),

crates/squawk_linter/src/rules/prefer_robust_stmts.rs

@@ -91,15 +91,13 @@ pub(crate) fn prefer_robust_stmts(ctx: &mut Linter, parse: &Parse<SourceFile>) {
                         }
                         ast::AlterTableAction::AddConstraint(add_constraint) => {
                             let constraint = add_constraint.constraint();
-                            if let Some(constraint_name) = constraint.and_then(|x| x.name()) {
-                                let name_text = constraint_name.text();
-                                let name = Identifier::new(name_text.as_str());
-                                if let Some(constraint) = constraint_names.get_mut(&name) {
-                                    if *constraint == Constraint::Dropped {
-                                        *constraint = Constraint::Added;
-                                        continue;
-                                    }
-                                }
+                            if let Some(constraint_name) = constraint.and_then(|x| x.name())
+                                && let Some(constraint) = constraint_names
+                                    .get_mut(&Identifier::new(constraint_name.text().as_str()))
+                                && *constraint == Constraint::Dropped
+                            {
+                                *constraint = Constraint::Added;
+                                continue;
                             }
                             (ActionErrorMessage::None, None)
                         }

crates/squawk_linter/src/rules/renaming_table.rs

@@ -10,7 +10,7 @@ pub(crate) fn renaming_table(ctx: &mut Linter, parse: &Parse<SourceFile>) {
     for stmt in file.stmts() {
         if let ast::Stmt::AlterTable(alter_table) = stmt {
             for action in alter_table.actions() {
-                if let ast::AlterTableAction::RenameTable(rename_table) = action {
+                if let ast::AlterTableAction::RenameTo(rename_table) = action {
                     ctx.report(Violation::for_node(
                         Rule::RenamingTable,
                         "Renaming a table may break existing clients.".into(),

crates/squawk_linter/src/rules/require_timeout_settings.rs

@@ -59,14 +59,14 @@ pub(crate) fn require_timeout_settings(ctx: &mut Linter, parse: &Parse<SourceFil
                     if path.qualifier().is_some() {
                         continue;
                     }
-                    if let Some(segment) = path.segment() {
-                        if let Some(name_ref) = segment.name_ref() {
-                            let name_ident = Identifier::new(name_ref.text().as_str());
-                            if name_ident == Identifier::new("lock_timeout") {
-                                lock_timeout = ReportOnce::Present;
-                            } else if name_ident == Identifier::new("statement_timeout") {
-                                stmt_timeout = ReportOnce::Present;
-                            }
+                    if let Some(segment) = path.segment()
+                        && let Some(name_ref) = segment.name_ref()
+                    {
+                        let name_ident = Identifier::new(name_ref.text().as_str());
+                        if name_ident == Identifier::new("lock_timeout") {
+                            lock_timeout = ReportOnce::Present;
+                        } else if name_ident == Identifier::new("statement_timeout") {
+                            stmt_timeout = ReportOnce::Present;
                         }
                     }
                 }