Merge branch 'main' into feat/improved-type-msgs

Author: juleswritescode

…4ab7e7938e9516ea21513265c49d304b6ea.json …258bce50342c2d5cce036dba4f9062cf811.json.sqlx/query-b0163e58e9c646e3af524174081b74ab7e7938e9516ea21513265c49d304b6ea.json renamed to .sqlx/query-1cc58ddce2b52b5d6f6519cde339f258bce50342c2d5cce036dba4f9062cf811.json .sqlx/query-b0163e58e9c646e3af524174081b74ab7e7938e9516ea21513265c49d304b6ea.json renamed to .sqlx/query-1cc58ddce2b52b5d6f6519cde339f258bce50342c2d5cce036dba4f9062cf811.json

@@ -24,6 +24,10 @@ impl HoveredNode {
     pub(crate) fn get(ctx: &pgt_treesitter::context::TreesitterContext) -> Option<Self> {
         let node_content = ctx.get_node_under_cursor_content()?;
 
+        if looks_like_sql_param(node_content.as_str()) {
+            return None;
+        }
+
         let under_cursor = ctx.node_under_cursor.as_ref()?;
 
         match under_cursor.kind() {
@@ -114,3 +118,10 @@ impl HoveredNode {
         }
     }
 }
+
+fn looks_like_sql_param(content: &str) -> bool {
+    (content.starts_with("$") && !content.starts_with("$$"))
+        || (content.starts_with(":") && !content.starts_with("::"))
+        || (content.starts_with("@"))
+        || content.starts_with("?")
+}

crates/pgt_hover/src/hovered_node.rs

@@ -518,3 +518,44 @@ async fn test_grant_table_hover(test_db: PgPool) {
 
     test_hover_at_cursor("grant_select", query, None, &test_db).await;
 }
+
+#[sqlx::test(migrator = "pgt_test_utils::MIGRATIONS")]
+async fn no_hover_results_over_params(test_db: PgPool) {
+    let setup = r#"
+        create table users (
+            id serial primary key,
+            name text
+        );
+    "#;
+
+    test_db.execute(setup).await.unwrap();
+
+    {
+        let query = format!(
+            "select * from users where name = $n{}ame;",
+            QueryWithCursorPosition::cursor_marker()
+        );
+        test_hover_at_cursor("dollar-param", query, None, &test_db).await;
+    }
+    {
+        let query = format!(
+            "select * from users where name = :n{}ame;",
+            QueryWithCursorPosition::cursor_marker()
+        );
+        test_hover_at_cursor("colon-param", query, None, &test_db).await;
+    }
+    {
+        let query = format!(
+            "select * from users where name = @n{}ame;",
+            QueryWithCursorPosition::cursor_marker()
+        );
+        test_hover_at_cursor("at-param", query, None, &test_db).await;
+    }
+    {
+        let query = format!(
+            "select * from users where name = ?n{}ame;",
+            QueryWithCursorPosition::cursor_marker()
+        );
+        test_hover_at_cursor("questionmark-param", query, None, &test_db).await;
+    }
+}

crates/pgt_hover/tests/hover_integration_tests.rs

@@ -0,0 +1,12 @@
+---
+source: crates/pgt_hover/tests/hover_integration_tests.rs
+expression: snapshot
+---
+# Input
+```sql
+select * from users where name = @name;
+                                   ↑ hovered here
+```
+
+# Hover Results
+No hover information found.

crates/pgt_hover/tests/snapshots/at-param.snap

@@ -0,0 +1,12 @@
+---
+source: crates/pgt_hover/tests/hover_integration_tests.rs
+expression: snapshot
+---
+# Input
+```sql
+select * from users where name = :name;
+                                   ↑ hovered here
+```
+
+# Hover Results
+No hover information found.

crates/pgt_hover/tests/snapshots/colon-param.snap

@@ -0,0 +1,12 @@
+---
+source: crates/pgt_hover/tests/hover_integration_tests.rs
+expression: snapshot
+---
+# Input
+```sql
+select * from users where name = $name;
+                                   ↑ hovered here
+```
+
+# Hover Results
+No hover information found.

crates/pgt_hover/tests/snapshots/dollar-param.snap

@@ -0,0 +1,12 @@
+---
+source: crates/pgt_hover/tests/hover_integration_tests.rs
+expression: snapshot
+---
+# Input
+```sql
+select * from users where name = ?name;
+                                   ↑ hovered here
+```
+
+# Hover Results
+No hover information found.

crates/pgt_hover/tests/snapshots/questionmark-param.snap

@@ -19,14 +19,15 @@ with
   available_indexes as (
     select
       unnest (ix.indkey) as attnum,
-      ix.indisprimary as is_primary,
-      ix.indisunique as is_unique,
+      bool_or(ix.indisprimary) as is_primary,
+      bool_or(ix.indisunique) as is_unique,
       ix.indrelid as table_oid
     from
       pg_catalog.pg_class c
       join pg_catalog.pg_index ix on c.oid = ix.indexrelid
     where
       c.relkind = 'i'
+    group by table_oid, attnum
   )
 select
   atts.attname as name,

crates/pgt_schema_cache/src/queries/columns.sql

@@ -165,7 +165,9 @@ pub trait SchemaCacheItem {
 
 #[cfg(test)]
 mod tests {
-    use sqlx::PgPool;
+    use std::collections::HashSet;
+
+    use sqlx::{Executor, PgPool};
 
     use crate::SchemaCache;
 
@@ -175,4 +177,33 @@ mod tests {
             .await
             .expect("Couldnt' load Schema Cache");
     }
+
+    #[sqlx::test(migrator = "pgt_test_utils::MIGRATIONS")]
+    async fn it_does_not_have_duplicate_entries(test_db: PgPool) {
+        // we had some duplicate columns in the schema_cache because of indices including the same column multiple times.
+        // the columns were unnested as duplicates in the query
+        let setup = r#"
+        CREATE TABLE public.mfa_factors (
+            id uuid PRIMARY KEY,
+            factor_name text NOT NULL
+        );
+
+        -- a second index on id!
+        CREATE INDEX idx_mfa_user_factor ON public.mfa_factors(id, factor_name);
+        "#;
+
+        test_db.execute(setup).await.unwrap();
+
+        let cache = SchemaCache::load(&test_db)
+            .await
+            .expect("Couldn't load Schema Cache");
+
+        let set: HashSet<String> = cache
+            .columns
+            .iter()
+            .map(|c| format!("{}.{}.{}", c.schema_name, c.table_name, c.name))
+            .collect();
+
+        assert_eq!(set.len(), cache.columns.len());
+    }
 }

crates/pgt_schema_cache/src/schema_cache.rs

@@ -470,7 +470,9 @@ impl<'a> TreesitterContext<'a> {
         }
 
         // We have arrived at the leaf node
-        if current_node.child_count() == 0 {
+        if current_node.child_count() == 0
+            || current_node.first_child_for_byte(self.position).is_none()
+        {
             self.node_under_cursor = Some(NodeUnderCursor::from(current_node));
             return;
         }
@@ -1214,4 +1216,43 @@ mod tests {
             _ => unreachable!(),
         }
     }
+
+    #[test]
+    fn does_not_overflow_callstack_on_smaller_treesitter_child() {
+        let query = format!(
+            r#"select * from persons where id = @i{}d;"#,
+            QueryWithCursorPosition::cursor_marker()
+        );
+
+        /*
+            The query (currently) yields the following treesitter tree for the WHERE clause:
+
+            where [29..43] 'where id = @id'
+                keyword_where [29..34] 'where'
+                binary_expression [35..43] 'id = @id'
+                field [35..37] 'id'
+                    identifier [35..37] 'id'
+                = [38..39] '='
+                field [40..43] '@id'
+                    identifier [40..43] '@id'
+                        @ [40..41] '@'
+
+            You can see that the '@' is a child of the "identifier" but has a range smaller than its parent's.
+            This would crash our context parsing because, at position 42, we weren't at the leaf node but also couldn't
+            go to a child on that position.
+        */
+
+        let (position, text) = QueryWithCursorPosition::from(query).get_text_and_position();
+
+        let tree = get_tree(text.as_str());
+
+        let params = TreeSitterContextParams {
+            position: (position as u32).into(),
+            text: &text,
+            tree: &tree,
+        };
+
+        // should simply not panic
+        let _ = TreesitterContext::new(params);
+    }
 }