From e54fc9356bf92c14a37f58a9ca9bc7af8fec6bb3 Mon Sep 17 00:00:00 2001 From: Julian Date: Sun, 20 Apr 2025 20:23:44 +0200 Subject: [PATCH 01/22] ok --- crates/pgt_completions/src/builder.rs | 86 +++++--- crates/pgt_completions/src/complete.rs | 2 +- crates/pgt_completions/src/item.rs | 1 - .../pgt_completions/src/providers/columns.rs | 49 ++++- .../src/providers/functions.rs | 16 +- .../pgt_completions/src/providers/schemas.rs | 15 +- .../pgt_completions/src/providers/tables.rs | 16 +- crates/pgt_completions/src/relevance.rs | 188 +----------------- .../src/relevance/filtering.rs | 89 +++++++++ .../pgt_completions/src/relevance/scoring.rs | 182 +++++++++++++++++ crates/pgt_completions/src/sanitization.rs | 24 ++- .../pgt_workspace/src/features/completions.rs | 4 +- 12 files changed, 427 insertions(+), 245 deletions(-) create mode 100644 crates/pgt_completions/src/relevance/filtering.rs create mode 100644 crates/pgt_completions/src/relevance/scoring.rs diff --git a/crates/pgt_completions/src/builder.rs b/crates/pgt_completions/src/builder.rs index 39439afb..bb7b157c 100644 --- a/crates/pgt_completions/src/builder.rs +++ b/crates/pgt_completions/src/builder.rs @@ -1,47 +1,79 @@ -use crate::item::CompletionItem; +use crate::{ + CompletionItemKind, + context::CompletionContext, + item::CompletionItem, + relevance::{filtering::CompletionFilter, scoring::CompletionScore}, +}; -pub(crate) struct CompletionBuilder { - items: Vec, +pub(crate) struct PossibleCompletionItem<'a> { + pub label: String, + pub description: String, + pub kind: CompletionItemKind, + pub score: CompletionScore<'a>, + pub filter: CompletionFilter<'a>, } -impl CompletionBuilder { - pub fn new() -> Self { - CompletionBuilder { items: vec![] } +pub(crate) struct CompletionBuilder<'a> { + items: Vec>, + ctx: &'a CompletionContext<'a>, +} + +impl<'a> CompletionBuilder<'a> { + pub fn new(ctx: &'a CompletionContext) -> Self { + CompletionBuilder { items: vec![], ctx } } - pub fn add_item(&mut self, item: CompletionItem) { + pub fn add_item(&mut self, item: PossibleCompletionItem<'a>) { self.items.push(item); } - pub fn finish(mut self) -> Vec { - self.items - .sort_by(|a, b| b.score.cmp(&a.score).then_with(|| a.label.cmp(&b.label))); + pub fn finish(self) -> Vec { + let mut items: Vec = self + .items + .into_iter() + .filter(|i| i.filter.is_relevant(self.ctx).is_some()) + .collect(); + + for item in items.iter_mut() { + item.score.calc_score(self.ctx); + } - self.items.dedup_by(|a, b| a.label == b.label); - self.items.truncate(crate::LIMIT); + items.sort_by(|a, b| { + b.score + .get_score() + .cmp(&a.score.get_score()) + .then_with(|| a.label.cmp(&b.label)) + }); - let should_preselect_first_item = self.should_preselect_first_item(); + items.dedup_by(|a, b| a.label == b.label); + items.truncate(crate::LIMIT); - self.items + let should_preselect_first_item = should_preselect_first_item(&items); + + items .into_iter() .enumerate() - .map(|(idx, mut item)| { - if idx == 0 { - item.preselected = should_preselect_first_item; + .map(|(idx, item)| { + let preselected = idx == 0 && should_preselect_first_item; + + CompletionItem { + description: item.description, + kind: item.kind, + label: item.label, + preselected, } - item }) .collect() } +} - fn should_preselect_first_item(&mut self) -> bool { - let mut items_iter = self.items.iter(); - let first = items_iter.next(); - let second = items_iter.next(); +fn should_preselect_first_item(items: &Vec) -> bool { + let mut items_iter = items.iter(); + let first = items_iter.next(); + let second = items_iter.next(); - first.is_some_and(|f| match second { - Some(s) => (f.score - s.score) > 10, - None => true, - }) - } + first.is_some_and(|f| match second { + Some(s) => (f.score.get_score() - s.score.get_score()) > 10, + None => true, + }) } diff --git a/crates/pgt_completions/src/complete.rs b/crates/pgt_completions/src/complete.rs index 89d25738..442ee546 100644 --- a/crates/pgt_completions/src/complete.rs +++ b/crates/pgt_completions/src/complete.rs @@ -27,7 +27,7 @@ pub fn complete(params: CompletionParams) -> Vec { let ctx = CompletionContext::new(&sanitized_params); - let mut builder = CompletionBuilder::new(); + let mut builder = CompletionBuilder::new(&ctx); complete_tables(&ctx, &mut builder); complete_functions(&ctx, &mut builder); diff --git a/crates/pgt_completions/src/item.rs b/crates/pgt_completions/src/item.rs index 1f306d78..6506f3e8 100644 --- a/crates/pgt_completions/src/item.rs +++ b/crates/pgt_completions/src/item.rs @@ -14,7 +14,6 @@ pub enum CompletionItemKind { #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] pub struct CompletionItem { pub label: String, - pub(crate) score: i32, pub description: String, pub preselected: bool, pub kind: CompletionItemKind, diff --git a/crates/pgt_completions/src/providers/columns.rs b/crates/pgt_completions/src/providers/columns.rs index 2898b63f..32d37075 100644 --- a/crates/pgt_completions/src/providers/columns.rs +++ b/crates/pgt_completions/src/providers/columns.rs @@ -1,17 +1,21 @@ use crate::{ - CompletionItem, CompletionItemKind, builder::CompletionBuilder, context::CompletionContext, - relevance::CompletionRelevanceData, + CompletionItemKind, + builder::{CompletionBuilder, PossibleCompletionItem}, + context::CompletionContext, + relevance::{CompletionRelevanceData, filtering::CompletionFilter, scoring::CompletionScore}, }; -pub fn complete_columns(ctx: &CompletionContext, builder: &mut CompletionBuilder) { +pub fn complete_columns<'a>(ctx: &CompletionContext<'a>, builder: &mut CompletionBuilder<'a>) { let available_columns = &ctx.schema_cache.columns; for col in available_columns { - let item = CompletionItem { + let relevance = CompletionRelevanceData::Column(col); + + let item = PossibleCompletionItem { label: col.name.clone(), - score: CompletionRelevanceData::Column(col).get_score(ctx), + score: CompletionScore::from(relevance.clone()), + filter: CompletionFilter::from(relevance), description: format!("Table: {}.{}", col.schema_name, col.table_name), - preselected: false, kind: CompletionItemKind::Column, }; @@ -22,7 +26,7 @@ pub fn complete_columns(ctx: &CompletionContext, builder: &mut CompletionBuilder #[cfg(test)] mod tests { use crate::{ - CompletionItem, complete, + CompletionItem, CompletionItemKind, complete, test_helper::{CURSOR_POS, InputQuery, get_test_deps, get_test_params}, }; @@ -225,4 +229,35 @@ mod tests { "`email` not present in first four completion items." ); } + + #[tokio::test] + async fn ignores_cols_in_from_clause() { + let setup = r#" + create schema private; + + create table private.users ( + id serial primary key, + name text, + address text, + email text + ); + "#; + + let test_case = TestCase { + message: "suggests user created tables first", + query: format!(r#"select * from private.{}"#, CURSOR_POS), + label: "", + description: "", + }; + + let (tree, cache) = get_test_deps(setup, test_case.get_input_query()).await; + let params = get_test_params(&tree, &cache, test_case.get_input_query()); + let results = complete(params); + + assert!( + !results + .into_iter() + .any(|item| item.kind == CompletionItemKind::Column) + ); + } } diff --git a/crates/pgt_completions/src/providers/functions.rs b/crates/pgt_completions/src/providers/functions.rs index b4a9c35a..b44a5ef5 100644 --- a/crates/pgt_completions/src/providers/functions.rs +++ b/crates/pgt_completions/src/providers/functions.rs @@ -1,17 +1,21 @@ use crate::{ - CompletionItem, CompletionItemKind, builder::CompletionBuilder, context::CompletionContext, - relevance::CompletionRelevanceData, + CompletionItemKind, + builder::{CompletionBuilder, PossibleCompletionItem}, + context::CompletionContext, + relevance::{CompletionRelevanceData, filtering::CompletionFilter, scoring::CompletionScore}, }; -pub fn complete_functions(ctx: &CompletionContext, builder: &mut CompletionBuilder) { +pub fn complete_functions<'a>(ctx: &'a CompletionContext, builder: &mut CompletionBuilder<'a>) { let available_functions = &ctx.schema_cache.functions; for func in available_functions { - let item = CompletionItem { + let relevance = CompletionRelevanceData::Function(func); + + let item = PossibleCompletionItem { label: func.name.clone(), - score: CompletionRelevanceData::Function(func).get_score(ctx), + score: CompletionScore::from(relevance.clone()), + filter: CompletionFilter::from(relevance), description: format!("Schema: {}", func.schema), - preselected: false, kind: CompletionItemKind::Function, }; diff --git a/crates/pgt_completions/src/providers/schemas.rs b/crates/pgt_completions/src/providers/schemas.rs index 2f41e8c3..3d8f622e 100644 --- a/crates/pgt_completions/src/providers/schemas.rs +++ b/crates/pgt_completions/src/providers/schemas.rs @@ -1,20 +1,21 @@ use crate::{ - CompletionItem, builder::CompletionBuilder, context::CompletionContext, - relevance::CompletionRelevanceData, + builder::{CompletionBuilder, PossibleCompletionItem}, + context::CompletionContext, + relevance::{CompletionRelevanceData, filtering::CompletionFilter, scoring::CompletionScore}, }; -pub fn complete_schemas(ctx: &CompletionContext, builder: &mut CompletionBuilder) { +pub fn complete_schemas<'a>(ctx: &'a CompletionContext, builder: &mut CompletionBuilder<'a>) { let available_schemas = &ctx.schema_cache.schemas; for schema in available_schemas { - let relevance = CompletionRelevanceData::Schema(&schema); + let relevance = CompletionRelevanceData::Schema(schema); - let item = CompletionItem { + let item = PossibleCompletionItem { label: schema.name.clone(), description: "Schema".into(), - preselected: false, kind: crate::CompletionItemKind::Schema, - score: relevance.get_score(ctx), + score: CompletionScore::from(relevance.clone()), + filter: CompletionFilter::from(relevance), }; builder.add_item(item); diff --git a/crates/pgt_completions/src/providers/tables.rs b/crates/pgt_completions/src/providers/tables.rs index 2074a4f1..fcc8fa00 100644 --- a/crates/pgt_completions/src/providers/tables.rs +++ b/crates/pgt_completions/src/providers/tables.rs @@ -1,19 +1,21 @@ use crate::{ - builder::CompletionBuilder, + builder::{CompletionBuilder, PossibleCompletionItem}, context::CompletionContext, - item::{CompletionItem, CompletionItemKind}, - relevance::CompletionRelevanceData, + item::CompletionItemKind, + relevance::{CompletionRelevanceData, filtering::CompletionFilter, scoring::CompletionScore}, }; -pub fn complete_tables(ctx: &CompletionContext, builder: &mut CompletionBuilder) { +pub fn complete_tables<'a>(ctx: &'a CompletionContext, builder: &mut CompletionBuilder<'a>) { let available_tables = &ctx.schema_cache.tables; for table in available_tables { - let item = CompletionItem { + let relevance = CompletionRelevanceData::Table(table); + + let item = PossibleCompletionItem { label: table.name.clone(), - score: CompletionRelevanceData::Table(table).get_score(ctx), + score: CompletionScore::from(relevance.clone()), + filter: CompletionFilter::from(relevance), description: format!("Schema: {}", table.schema), - preselected: false, kind: CompletionItemKind::Table, }; diff --git a/crates/pgt_completions/src/relevance.rs b/crates/pgt_completions/src/relevance.rs index 2abb9f2c..911a6433 100644 --- a/crates/pgt_completions/src/relevance.rs +++ b/crates/pgt_completions/src/relevance.rs @@ -1,192 +1,10 @@ -use crate::context::{ClauseType, CompletionContext, NodeText}; +pub(crate) mod filtering; +pub(crate) mod scoring; -#[derive(Debug)] +#[derive(Debug, Clone)] pub(crate) enum CompletionRelevanceData<'a> { Table(&'a pgt_schema_cache::Table), Function(&'a pgt_schema_cache::Function), Column(&'a pgt_schema_cache::Column), Schema(&'a pgt_schema_cache::Schema), } - -impl CompletionRelevanceData<'_> { - pub fn get_score(self, ctx: &CompletionContext) -> i32 { - CompletionRelevance::from(self).into_score(ctx) - } -} - -impl<'a> From> for CompletionRelevance<'a> { - fn from(value: CompletionRelevanceData<'a>) -> Self { - Self { - score: 0, - data: value, - } - } -} - -#[derive(Debug)] -pub(crate) struct CompletionRelevance<'a> { - score: i32, - data: CompletionRelevanceData<'a>, -} - -impl CompletionRelevance<'_> { - pub fn into_score(mut self, ctx: &CompletionContext) -> i32 { - self.check_is_user_defined(); - self.check_matches_schema(ctx); - self.check_matches_query_input(ctx); - self.check_is_invocation(ctx); - self.check_matching_clause_type(ctx); - self.check_relations_in_stmt(ctx); - - self.score - } - - fn check_matches_query_input(&mut self, ctx: &CompletionContext) { - let node = match ctx.node_under_cursor { - Some(node) => node, - None => return, - }; - - let content = match ctx.get_ts_node_content(node) { - Some(c) => match c { - NodeText::Original(s) => s, - NodeText::Replaced => return, - }, - None => return, - }; - - let name = match self.data { - CompletionRelevanceData::Function(f) => f.name.as_str(), - CompletionRelevanceData::Table(t) => t.name.as_str(), - CompletionRelevanceData::Column(c) => c.name.as_str(), - CompletionRelevanceData::Schema(s) => s.name.as_str(), - }; - - if name.starts_with(content) { - let len: i32 = content - .len() - .try_into() - .expect("The length of the input exceeds i32 capacity"); - - self.score += len * 10; - }; - } - - fn check_matching_clause_type(&mut self, ctx: &CompletionContext) { - let clause_type = match ctx.wrapping_clause_type.as_ref() { - None => return, - Some(ct) => ct, - }; - - let has_mentioned_tables = !ctx.mentioned_relations.is_empty(); - - self.score += match self.data { - CompletionRelevanceData::Table(_) => match clause_type { - ClauseType::From => 5, - ClauseType::Update => 15, - ClauseType::Delete => 15, - _ => -50, - }, - CompletionRelevanceData::Function(_) => match clause_type { - ClauseType::Select if !has_mentioned_tables => 15, - ClauseType::Select if has_mentioned_tables => 0, - ClauseType::From => 0, - _ => -50, - }, - CompletionRelevanceData::Column(_) => match clause_type { - ClauseType::Select if has_mentioned_tables => 10, - ClauseType::Select if !has_mentioned_tables => 0, - ClauseType::Where => 10, - _ => -15, - }, - CompletionRelevanceData::Schema(_) => match clause_type { - ClauseType::From => 10, - _ => -50, - }, - } - } - - fn check_is_invocation(&mut self, ctx: &CompletionContext) { - self.score += match self.data { - CompletionRelevanceData::Function(_) if ctx.is_invocation => 30, - CompletionRelevanceData::Function(_) if !ctx.is_invocation => -10, - _ if ctx.is_invocation => -10, - _ => 0, - }; - } - - fn check_matches_schema(&mut self, ctx: &CompletionContext) { - let schema_name = match ctx.schema_name.as_ref() { - None => return, - Some(n) => n, - }; - - let data_schema = self.get_schema_name(); - - if schema_name == data_schema { - self.score += 25; - } else { - self.score -= 10; - } - } - - fn get_schema_name(&self) -> &str { - match self.data { - CompletionRelevanceData::Function(f) => f.schema.as_str(), - CompletionRelevanceData::Table(t) => t.schema.as_str(), - CompletionRelevanceData::Column(c) => c.schema_name.as_str(), - CompletionRelevanceData::Schema(s) => s.name.as_str(), - } - } - - fn get_table_name(&self) -> Option<&str> { - match self.data { - CompletionRelevanceData::Column(c) => Some(c.table_name.as_str()), - CompletionRelevanceData::Table(t) => Some(t.name.as_str()), - _ => None, - } - } - - fn check_relations_in_stmt(&mut self, ctx: &CompletionContext) { - match self.data { - CompletionRelevanceData::Table(_) | CompletionRelevanceData::Function(_) => return, - _ => {} - } - - let schema = self.get_schema_name().to_string(); - let table_name = match self.get_table_name() { - Some(t) => t, - None => return, - }; - - if ctx - .mentioned_relations - .get(&Some(schema.to_string())) - .is_some_and(|tables| tables.contains(table_name)) - { - self.score += 45; - } else if ctx - .mentioned_relations - .get(&None) - .is_some_and(|tables| tables.contains(table_name)) - { - self.score += 30; - } - } - - fn check_is_user_defined(&mut self) { - let schema = self.get_schema_name().to_string(); - - let system_schemas = ["pg_catalog", "information_schema", "pg_toast"]; - - if system_schemas.contains(&schema.as_str()) { - self.score -= 10; - } - - // "public" is the default postgres schema where users - // create objects. Prefer it by a slight bit. - if schema.as_str() == "public" { - self.score += 2; - } - } -} diff --git a/crates/pgt_completions/src/relevance/filtering.rs b/crates/pgt_completions/src/relevance/filtering.rs new file mode 100644 index 00000000..21f64344 --- /dev/null +++ b/crates/pgt_completions/src/relevance/filtering.rs @@ -0,0 +1,89 @@ +use crate::context::{ClauseType, CompletionContext}; + +use super::CompletionRelevanceData; + +#[derive(Debug)] +pub(crate) struct CompletionFilter<'a> { + data: CompletionRelevanceData<'a>, +} + +impl<'a> From> for CompletionFilter<'a> { + fn from(value: CompletionRelevanceData<'a>) -> Self { + Self { data: value } + } +} + +impl CompletionFilter<'_> { + pub fn is_relevant(&self, ctx: &CompletionContext) -> Option<()> { + self.completable_context(ctx)?; + self.check_clause(ctx)?; + self.check_invocation(ctx)?; + self.check_mentioned_schema(ctx)?; + + Some(()) + } + + fn completable_context(&self, ctx: &CompletionContext) -> Option<()> { + let current_node_kind = ctx.node_under_cursor.map(|n| n.kind()).unwrap_or(""); + + if current_node_kind.starts_with("keyword_") + || current_node_kind == "=" + || current_node_kind == "," + || current_node_kind == "literal" + { + return None; + } + + Some(()) + } + + fn check_clause(&self, ctx: &CompletionContext) -> Option<()> { + let clause = ctx.wrapping_clause_type.as_ref(); + + match self.data { + CompletionRelevanceData::Table(_) => { + let in_select_clause = clause.is_some_and(|c| c == &ClauseType::Select); + let in_where_clause = clause.is_some_and(|c| c == &ClauseType::Where); + + if in_select_clause || in_where_clause { + return None; + }; + } + CompletionRelevanceData::Column(_) => { + let in_from_clause = clause.is_some_and(|c| c == &ClauseType::From); + + if in_from_clause { + return None; + } + } + _ => {} + } + + Some(()) + } + + fn check_invocation(&self, ctx: &CompletionContext) -> Option<()> { + if !ctx.is_invocation { + return Some(()); + } + + match self.data { + CompletionRelevanceData::Table(_) | CompletionRelevanceData::Column(_) => return None, + _ => {} + } + + Some(()) + } + + fn check_mentioned_schema(&self, ctx: &CompletionContext) -> Option<()> { + if ctx.schema_name.is_none() { + return Some(()); + } + + if let CompletionRelevanceData::Schema(_) = self.data { + return None; + } + + Some(()) + } +} diff --git a/crates/pgt_completions/src/relevance/scoring.rs b/crates/pgt_completions/src/relevance/scoring.rs new file mode 100644 index 00000000..7c3f3a06 --- /dev/null +++ b/crates/pgt_completions/src/relevance/scoring.rs @@ -0,0 +1,182 @@ +use crate::context::{ClauseType, CompletionContext, NodeText}; + +use super::CompletionRelevanceData; + +#[derive(Debug)] +pub(crate) struct CompletionScore<'a> { + score: i32, + data: CompletionRelevanceData<'a>, +} + +impl<'a> From> for CompletionScore<'a> { + fn from(value: CompletionRelevanceData<'a>) -> Self { + Self { + score: 0, + data: value, + } + } +} + +impl CompletionScore<'_> { + pub fn get_score(&self) -> i32 { + self.score + } + + pub fn calc_score(&mut self, ctx: &CompletionContext) { + self.check_is_user_defined(); + self.check_matches_schema(ctx); + self.check_matches_query_input(ctx); + self.check_is_invocation(ctx); + self.check_matching_clause_type(ctx); + self.check_relations_in_stmt(ctx); + } + + fn check_matches_query_input(&mut self, ctx: &CompletionContext) { + let node = match ctx.node_under_cursor { + Some(node) => node, + None => return, + }; + + let content = match ctx.get_ts_node_content(node) { + Some(c) => match c { + NodeText::Original(s) => s, + NodeText::Replaced => return, + }, + None => return, + }; + + let name = match self.data { + CompletionRelevanceData::Function(f) => f.name.as_str(), + CompletionRelevanceData::Table(t) => t.name.as_str(), + CompletionRelevanceData::Column(c) => c.name.as_str(), + CompletionRelevanceData::Schema(s) => s.name.as_str(), + }; + + if name.starts_with(content) { + let len: i32 = content + .len() + .try_into() + .expect("The length of the input exceeds i32 capacity"); + + self.score += len * 10; + }; + } + + fn check_matching_clause_type(&mut self, ctx: &CompletionContext) { + let clause_type = match ctx.wrapping_clause_type.as_ref() { + None => return, + Some(ct) => ct, + }; + + let has_mentioned_tables = !ctx.mentioned_relations.is_empty(); + + self.score += match self.data { + CompletionRelevanceData::Table(_) => match clause_type { + ClauseType::From => 5, + ClauseType::Update => 15, + ClauseType::Delete => 15, + _ => -50, + }, + CompletionRelevanceData::Function(_) => match clause_type { + ClauseType::Select if !has_mentioned_tables => 15, + ClauseType::Select if has_mentioned_tables => 0, + ClauseType::From => 0, + _ => -50, + }, + CompletionRelevanceData::Column(_) => match clause_type { + ClauseType::Select if has_mentioned_tables => 10, + ClauseType::Select if !has_mentioned_tables => 0, + ClauseType::Where => 10, + _ => -15, + }, + CompletionRelevanceData::Schema(_) => match clause_type { + ClauseType::From => 10, + _ => -50, + }, + } + } + + fn check_is_invocation(&mut self, ctx: &CompletionContext) { + self.score += match self.data { + CompletionRelevanceData::Function(_) if ctx.is_invocation => 30, + CompletionRelevanceData::Function(_) if !ctx.is_invocation => -10, + _ if ctx.is_invocation => -10, + _ => 0, + }; + } + + fn check_matches_schema(&mut self, ctx: &CompletionContext) { + let schema_name = match ctx.schema_name.as_ref() { + None => return, + Some(n) => n, + }; + + let data_schema = self.get_schema_name(); + + if schema_name == data_schema { + self.score += 25; + } else { + self.score -= 10; + } + } + + fn get_schema_name(&self) -> &str { + match self.data { + CompletionRelevanceData::Function(f) => f.schema.as_str(), + CompletionRelevanceData::Table(t) => t.schema.as_str(), + CompletionRelevanceData::Column(c) => c.schema_name.as_str(), + CompletionRelevanceData::Schema(s) => s.name.as_str(), + } + } + + fn get_table_name(&self) -> Option<&str> { + match self.data { + CompletionRelevanceData::Column(c) => Some(c.table_name.as_str()), + CompletionRelevanceData::Table(t) => Some(t.name.as_str()), + _ => None, + } + } + + fn check_relations_in_stmt(&mut self, ctx: &CompletionContext) { + match self.data { + CompletionRelevanceData::Table(_) | CompletionRelevanceData::Function(_) => return, + _ => {} + } + + let schema = self.get_schema_name().to_string(); + let table_name = match self.get_table_name() { + Some(t) => t, + None => return, + }; + + if ctx + .mentioned_relations + .get(&Some(schema.to_string())) + .is_some_and(|tables| tables.contains(table_name)) + { + self.score += 45; + } else if ctx + .mentioned_relations + .get(&None) + .is_some_and(|tables| tables.contains(table_name)) + { + self.score += 30; + } + } + + fn check_is_user_defined(&mut self) { + let schema = self.get_schema_name().to_string(); + + let system_schemas = ["pg_catalog", "information_schema", "pg_toast"]; + + if system_schemas.contains(&schema.as_str()) { + self.score -= 10; + } + + // "public" is the default postgres schema where users + // create objects. Prefer it by a slight bit. + if schema.as_str() == "public" { + self.score += 2; + } + } +} diff --git a/crates/pgt_completions/src/sanitization.rs b/crates/pgt_completions/src/sanitization.rs index dc093847..41b073f2 100644 --- a/crates/pgt_completions/src/sanitization.rs +++ b/crates/pgt_completions/src/sanitization.rs @@ -12,7 +12,7 @@ pub(crate) struct SanitizedCompletionParams<'a> { } pub fn benchmark_sanitization(params: CompletionParams) -> String { - let params: SanitizedCompletionParams = params.try_into().unwrap(); + let params: SanitizedCompletionParams = params.into(); params.text } @@ -24,6 +24,7 @@ where if cursor_inbetween_nodes(params.tree, params.position) || cursor_prepared_to_write_token_after_last_node(params.tree, params.position) || cursor_before_semicolon(params.tree, params.position) + || cursor_on_a_dot(¶ms.text, params.position) { SanitizedCompletionParams::with_adjusted_sql(params) } else { @@ -149,6 +150,11 @@ fn cursor_prepared_to_write_token_after_last_node( cursor_pos == tree.root_node().end_byte() + 1 } +fn cursor_on_a_dot(sql: &str, position: TextSize) -> bool { + let position: usize = position.into(); + sql.chars().nth(position - 1).is_some_and(|c| c == '.') +} + fn cursor_before_semicolon(tree: &tree_sitter::Tree, position: TextSize) -> bool { let mut cursor = tree.walk(); let mut leaf_node = tree.root_node(); @@ -198,7 +204,7 @@ mod tests { use pgt_text_size::TextSize; use crate::sanitization::{ - cursor_before_semicolon, cursor_inbetween_nodes, + cursor_before_semicolon, cursor_inbetween_nodes, cursor_on_a_dot, cursor_prepared_to_write_token_after_last_node, }; @@ -263,6 +269,20 @@ mod tests { )); } + #[test] + fn on_a_dot() { + let input = "select * from private."; + + // select * from private.| <-- on a dot + assert!(cursor_on_a_dot(&input, TextSize::new(22))); + + // select * from private|. <-- before the dot + assert!(!cursor_on_a_dot(&input, TextSize::new(21))); + + // select * from private. | <-- too far off the dot + assert!(!cursor_on_a_dot(&input, TextSize::new(23))); + } + #[test] fn test_cursor_before_semicolon() { // Idx "13" is the exlusive end of `select * from` (first space after from) diff --git a/crates/pgt_workspace/src/features/completions.rs b/crates/pgt_workspace/src/features/completions.rs index 4a5c5e29..e379018d 100644 --- a/crates/pgt_workspace/src/features/completions.rs +++ b/crates/pgt_workspace/src/features/completions.rs @@ -29,8 +29,8 @@ impl IntoIterator for CompletionsResult { } } -pub(crate) fn get_statement_for_completions<'a>( - doc: &'a ParsedDocument, +pub(crate) fn get_statement_for_completions( + doc: &ParsedDocument, position: TextSize, ) -> Option<(StatementId, TextRange, String, Arc)> { let count = doc.count(); From ecd00348004cc4f551fccb6ab2c369383446a4ce Mon Sep 17 00:00:00 2001 From: Julian Date: Sun, 20 Apr 2025 20:32:51 +0200 Subject: [PATCH 02/22] ignore all irrelevant schemas --- .../pgt_completions/src/relevance/filtering.rs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/crates/pgt_completions/src/relevance/filtering.rs b/crates/pgt_completions/src/relevance/filtering.rs index 21f64344..81b4ddf4 100644 --- a/crates/pgt_completions/src/relevance/filtering.rs +++ b/crates/pgt_completions/src/relevance/filtering.rs @@ -80,7 +80,22 @@ impl CompletionFilter<'_> { return Some(()); } - if let CompletionRelevanceData::Schema(_) = self.data { + let name = ctx.schema_name.as_ref().unwrap(); + + let does_not_match = match self.data { + CompletionRelevanceData::Table(table) => &table.schema != name, + CompletionRelevanceData::Function(foo) => &foo.schema != name, + CompletionRelevanceData::Column(_) => { + // columns belong to tables, not schemas + true + } + CompletionRelevanceData::Schema(_) => { + // we should never allow schema suggestions if there already was one. + true + } + }; + + if does_not_match { return None; } From be10534823b117851121b6f753a6148a27d2e48d Mon Sep 17 00:00:00 2001 From: Julian Date: Mon, 21 Apr 2025 10:07:47 +0200 Subject: [PATCH 03/22] fix bug --- .../pgt_completions/src/providers/columns.rs | 65 +++++++++++++++++++ crates/pgt_completions/src/sanitization.rs | 7 +- crates/pgt_treesitter_queries/src/lib.rs | 27 ++++++++ 3 files changed, 96 insertions(+), 3 deletions(-) diff --git a/crates/pgt_completions/src/providers/columns.rs b/crates/pgt_completions/src/providers/columns.rs index 32d37075..662ae4c8 100644 --- a/crates/pgt_completions/src/providers/columns.rs +++ b/crates/pgt_completions/src/providers/columns.rs @@ -260,4 +260,69 @@ mod tests { .any(|item| item.kind == CompletionItemKind::Column) ); } + + #[tokio::test] + async fn prefers_columns_of_mentioned_tables() { + let setup = r#" + create schema private; + + create table private.users ( + id1 serial primary key, + name1 text, + address1 text, + email1 text + ); + + create table public.users ( + id2 serial primary key, + name2 text, + address2 text, + email2 text + ); + "#; + + { + let test_case = TestCase { + message: "", + query: format!(r#"select {} from users"#, CURSOR_POS), + label: "suggests from table", + description: "", + }; + + let (tree, cache) = get_test_deps(setup, test_case.get_input_query()).await; + let params = get_test_params(&tree, &cache, test_case.get_input_query()); + let results = complete(params); + + assert_eq!( + results + .into_iter() + .take(4) + .map(|item| item.label) + .collect::>(), + vec!["address2", "email2", "id2", "name2"] + ); + } + + { + let test_case = TestCase { + message: "", + query: format!(r#"select {} from private.users"#, CURSOR_POS), + label: "suggests from table", + description: "", + }; + + let (tree, cache) = get_test_deps(setup, test_case.get_input_query()).await; + let params = get_test_params(&tree, &cache, test_case.get_input_query()); + let results = complete(params); + + assert_eq!( + results + .into_iter() + .take(4) + .map(|item| item.label) + .collect::>(), + vec!["address1", "email1", "id1", "name1"] + ); + } + } } diff --git a/crates/pgt_completions/src/sanitization.rs b/crates/pgt_completions/src/sanitization.rs index 41b073f2..f2f912bc 100644 --- a/crates/pgt_completions/src/sanitization.rs +++ b/crates/pgt_completions/src/sanitization.rs @@ -1,4 +1,4 @@ -use std::borrow::Cow; +use std::{borrow::Cow, cmp::max}; use pgt_text_size::TextSize; @@ -45,12 +45,13 @@ where let mut sql_iter = params.text.chars(); - for idx in 0..cursor_pos + 1 { + let max = max(cursor_pos + 1, params.text.len()); + + for idx in 0..max { match sql_iter.next() { Some(c) => { if idx == cursor_pos { sql.push_str(SANITIZED_TOKEN); - sql.push(' '); } sql.push(c); } diff --git a/crates/pgt_treesitter_queries/src/lib.rs b/crates/pgt_treesitter_queries/src/lib.rs index 7d2ba61b..3c6ef183 100644 --- a/crates/pgt_treesitter_queries/src/lib.rs +++ b/crates/pgt_treesitter_queries/src/lib.rs @@ -185,4 +185,31 @@ on sq1.id = pt.id; assert_eq!(results[0].get_schema(sql), Some("private".into())); assert_eq!(results[0].get_table(sql), "something"); } + + #[test] + fn picks_it_up_without_schemas() { + let sql = r#"select * from users"#; + + let mut parser = tree_sitter::Parser::new(); + parser.set_language(tree_sitter_sql::language()).unwrap(); + + let tree = parser.parse(sql, None).unwrap(); + + // trust me bro + + let mut executor = TreeSitterQueriesExecutor::new(tree.root_node(), sql); + + executor.add_query_results::(); + + let results: Vec<&RelationMatch> = executor + .get_iter(None) + .filter_map(|q| q.try_into().ok()) + .collect(); + + println!("{:?}", results); + + assert_eq!(results.len(), 1); + assert_eq!(results[0].get_schema(sql), None); + assert_eq!(results[0].get_table(sql), "users"); + } } From 1f7ef84e83aa7aded8aede4f45edc208513a1277 Mon Sep 17 00:00:00 2001 From: Julian Date: Mon, 21 Apr 2025 11:01:00 +0200 Subject: [PATCH 04/22] ok --- crates/pgt_completions/src/builder.rs | 1 + crates/pgt_completions/src/item.rs | 14 ++++++++ .../pgt_completions/src/providers/columns.rs | 29 +++++++++++++++- crates/pgt_completions/src/test_helper.rs | 33 +++++++++++++++++++ crates/pgt_lsp/src/capabilities.rs | 2 +- crates/pgt_lsp/src/handlers/completions.rs | 3 +- 6 files changed, 79 insertions(+), 3 deletions(-) diff --git a/crates/pgt_completions/src/builder.rs b/crates/pgt_completions/src/builder.rs index bb7b157c..b4749d9f 100644 --- a/crates/pgt_completions/src/builder.rs +++ b/crates/pgt_completions/src/builder.rs @@ -61,6 +61,7 @@ impl<'a> CompletionBuilder<'a> { kind: item.kind, label: item.label, preselected, + sort_text: item.score.get_score().to_string(), } }) .collect() diff --git a/crates/pgt_completions/src/item.rs b/crates/pgt_completions/src/item.rs index 6506f3e8..c753076c 100644 --- a/crates/pgt_completions/src/item.rs +++ b/crates/pgt_completions/src/item.rs @@ -1,3 +1,5 @@ +use std::fmt::Display; + use serde::{Deserialize, Serialize}; #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] @@ -10,6 +12,17 @@ pub enum CompletionItemKind { Schema, } +impl Display for CompletionItemKind { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + CompletionItemKind::Table => write!(f, "Table"), + CompletionItemKind::Function => write!(f, "Function"), + CompletionItemKind::Column => write!(f, "Column"), + CompletionItemKind::Schema => write!(f, "Schema"), + } + } +} + #[derive(Debug, Serialize, Deserialize)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] pub struct CompletionItem { @@ -17,4 +30,5 @@ pub struct CompletionItem { pub description: String, pub preselected: bool, pub kind: CompletionItemKind, + pub sort_text: String, } diff --git a/crates/pgt_completions/src/providers/columns.rs b/crates/pgt_completions/src/providers/columns.rs index 662ae4c8..557dccee 100644 --- a/crates/pgt_completions/src/providers/columns.rs +++ b/crates/pgt_completions/src/providers/columns.rs @@ -27,7 +27,9 @@ pub fn complete_columns<'a>(ctx: &CompletionContext<'a>, builder: &mut Completio mod tests { use crate::{ CompletionItem, CompletionItemKind, complete, - test_helper::{CURSOR_POS, InputQuery, get_test_deps, get_test_params}, + test_helper::{ + CURSOR_POS, InputQuery, get_test_deps, get_test_params, test_against_connection_string, + }, }; struct TestCase { @@ -325,4 +327,29 @@ mod tests { ); } } + + #[tokio::test] + async fn uses_the_cols_against_supabase_db() { + { + let conn_str = "postgresql://postgres:postgres@127.0.0.1:54322/postgres"; + + let input_query = InputQuery::with_sanitized_whitespace( + format!(r#"select {} from auth.users"#, CURSOR_POS).as_str(), + ); + + let (tree, cache) = test_against_connection_string(conn_str, input_query.clone()).await; + + let params = get_test_params(&tree, &cache, input_query); + let results = complete(params); + + println!("{:?}", results); + + assert!( + results + .into_iter() + .map(|item| item.description) + .all(|desc| desc == "Table: auth.users") + ); + } + } } diff --git a/crates/pgt_completions/src/test_helper.rs b/crates/pgt_completions/src/test_helper.rs index 4edf486f..5c8bea9d 100644 --- a/crates/pgt_completions/src/test_helper.rs +++ b/crates/pgt_completions/src/test_helper.rs @@ -6,11 +6,19 @@ use crate::CompletionParams; pub static CURSOR_POS: char = '€'; +#[derive(Clone)] pub struct InputQuery { sql: String, position: usize, } +impl InputQuery { + pub fn with_sanitized_whitespace(sql: &str) -> Self { + let strs: Vec<&str> = sql.split_whitespace().collect(); + strs.join(" ").as_str().into() + } +} + impl From<&str> for InputQuery { fn from(value: &str) -> Self { let position = value @@ -55,6 +63,31 @@ pub(crate) async fn get_test_deps( (tree, schema_cache) } +/// Careful: This will connect against the passed database. +/// Use this only to debug issues. Do not commit to version control. +#[allow(dead_code)] +pub(crate) async fn test_against_connection_string( + conn_str: &str, + input: InputQuery, +) -> (tree_sitter::Tree, pgt_schema_cache::SchemaCache) { + let pool = sqlx::PgPool::connect(conn_str) + .await + .expect("Unable to connect to database."); + + let schema_cache = SchemaCache::load(&pool) + .await + .expect("Failed to load Schema Cache"); + + let mut parser = tree_sitter::Parser::new(); + parser + .set_language(tree_sitter_sql::language()) + .expect("Error loading sql language"); + + let tree = parser.parse(input.to_string(), None).unwrap(); + + (tree, schema_cache) +} + pub(crate) fn get_text_and_position(q: InputQuery) -> (usize, String) { (q.position, q.sql) } diff --git a/crates/pgt_lsp/src/capabilities.rs b/crates/pgt_lsp/src/capabilities.rs index a801dd5e..b3e35b69 100644 --- a/crates/pgt_lsp/src/capabilities.rs +++ b/crates/pgt_lsp/src/capabilities.rs @@ -37,7 +37,7 @@ pub(crate) fn server_capabilities(capabilities: &ClientCapabilities) -> ServerCa // The request is used to get more information about a simple CompletionItem. resolve_provider: None, - trigger_characters: Some(vec![".".to_owned(), ",".to_owned(), " ".to_owned()]), + trigger_characters: Some(vec![".".to_owned(), " ".to_owned()]), // No character will lead to automatically inserting the selected completion-item all_commit_characters: None, diff --git a/crates/pgt_lsp/src/handlers/completions.rs b/crates/pgt_lsp/src/handlers/completions.rs index f9b68e7d..6d687371 100644 --- a/crates/pgt_lsp/src/handlers/completions.rs +++ b/crates/pgt_lsp/src/handlers/completions.rs @@ -32,10 +32,11 @@ pub fn get_completions( label: i.label, label_details: Some(CompletionItemLabelDetails { description: Some(i.description), - detail: None, + detail: Some(i.kind.to_string()), }), preselect: Some(i.preselected), kind: Some(to_lsp_types_completion_item_kind(i.kind)), + sort_text: Some(i.sort_text), ..CompletionItem::default() }) .collect(); From 7731b68f6859feba7c1d9237ce5432338c936a68 Mon Sep 17 00:00:00 2001 From: Julian Date: Mon, 21 Apr 2025 16:08:13 +0200 Subject: [PATCH 05/22] fix sorting --- crates/pgt_completions/src/builder.rs | 6 +++++- crates/pgt_completions/src/item.rs | 1 + crates/pgt_lsp/src/handlers/completions.rs | 6 +++++- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/crates/pgt_completions/src/builder.rs b/crates/pgt_completions/src/builder.rs index b4749d9f..aad1feb7 100644 --- a/crates/pgt_completions/src/builder.rs +++ b/crates/pgt_completions/src/builder.rs @@ -50,6 +50,9 @@ impl<'a> CompletionBuilder<'a> { let should_preselect_first_item = should_preselect_first_item(&items); + // a length of 481 should have a max_padding of 3 + let max_padding = items.len().to_string().len(); + items .into_iter() .enumerate() @@ -61,7 +64,8 @@ impl<'a> CompletionBuilder<'a> { kind: item.kind, label: item.label, preselected, - sort_text: item.score.get_score().to_string(), + score: item.score.get_score(), + sort_text: format!("{:0>padding$}", idx, padding = max_padding), } }) .collect() diff --git a/crates/pgt_completions/src/item.rs b/crates/pgt_completions/src/item.rs index c753076c..2a1c58fd 100644 --- a/crates/pgt_completions/src/item.rs +++ b/crates/pgt_completions/src/item.rs @@ -30,5 +30,6 @@ pub struct CompletionItem { pub description: String, pub preselected: bool, pub kind: CompletionItemKind, + pub score: i32, pub sort_text: String, } diff --git a/crates/pgt_lsp/src/handlers/completions.rs b/crates/pgt_lsp/src/handlers/completions.rs index 6d687371..9f6a3c24 100644 --- a/crates/pgt_lsp/src/handlers/completions.rs +++ b/crates/pgt_lsp/src/handlers/completions.rs @@ -32,7 +32,7 @@ pub fn get_completions( label: i.label, label_details: Some(CompletionItemLabelDetails { description: Some(i.description), - detail: Some(i.kind.to_string()), + detail: Some(format!(" {}", i.kind.to_string())), }), preselect: Some(i.preselected), kind: Some(to_lsp_types_completion_item_kind(i.kind)), @@ -41,6 +41,10 @@ pub fn get_completions( }) .collect(); + for item in &items { + tracing::info!("{}, {}", item.label, item.sort_text.as_ref().unwrap()); + } + Ok(lsp_types::CompletionResponse::Array(items)) } From 79b0fb9e8f1f082b3e7709ed3df7b99868f84cb0 Mon Sep 17 00:00:00 2001 From: Julian Date: Mon, 21 Apr 2025 16:14:33 +0200 Subject: [PATCH 06/22] ok --- .../pgt_completions/src/providers/columns.rs | 25 ------------------- .../src/relevance/filtering.rs | 1 + 2 files changed, 1 insertion(+), 25 deletions(-) diff --git a/crates/pgt_completions/src/providers/columns.rs b/crates/pgt_completions/src/providers/columns.rs index 557dccee..6f8aa447 100644 --- a/crates/pgt_completions/src/providers/columns.rs +++ b/crates/pgt_completions/src/providers/columns.rs @@ -327,29 +327,4 @@ mod tests { ); } } - - #[tokio::test] - async fn uses_the_cols_against_supabase_db() { - { - let conn_str = "postgresql://postgres:postgres@127.0.0.1:54322/postgres"; - - let input_query = InputQuery::with_sanitized_whitespace( - format!(r#"select {} from auth.users"#, CURSOR_POS).as_str(), - ); - - let (tree, cache) = test_against_connection_string(conn_str, input_query.clone()).await; - - let params = get_test_params(&tree, &cache, input_query); - let results = complete(params); - - println!("{:?}", results); - - assert!( - results - .into_iter() - .map(|item| item.description) - .all(|desc| desc == "Table: auth.users") - ); - } - } } diff --git a/crates/pgt_completions/src/relevance/filtering.rs b/crates/pgt_completions/src/relevance/filtering.rs index 81b4ddf4..f1c7a739 100644 --- a/crates/pgt_completions/src/relevance/filtering.rs +++ b/crates/pgt_completions/src/relevance/filtering.rs @@ -30,6 +30,7 @@ impl CompletionFilter<'_> { || current_node_kind == "=" || current_node_kind == "," || current_node_kind == "literal" + || current_node_kind == "ERROR" { return None; } From 5f97ae02ccd70c9a9d5d8be703434a05ca46dd70 Mon Sep 17 00:00:00 2001 From: Julian Date: Mon, 21 Apr 2025 16:17:53 +0200 Subject: [PATCH 07/22] ok --- crates/pgt_lsp/src/handlers/completions.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/crates/pgt_lsp/src/handlers/completions.rs b/crates/pgt_lsp/src/handlers/completions.rs index 9f6a3c24..dee895af 100644 --- a/crates/pgt_lsp/src/handlers/completions.rs +++ b/crates/pgt_lsp/src/handlers/completions.rs @@ -41,10 +41,6 @@ pub fn get_completions( }) .collect(); - for item in &items { - tracing::info!("{}, {}", item.label, item.sort_text.as_ref().unwrap()); - } - Ok(lsp_types::CompletionResponse::Array(items)) } From 0701724957f4b614ccd35a3fe49af940bbe347bf Mon Sep 17 00:00:00 2001 From: Julian Date: Mon, 21 Apr 2025 16:22:00 +0200 Subject: [PATCH 08/22] fix(completions): cursor on dot, other issues --- crates/pgt_completions/src/sanitization.rs | 35 +++++++++++++++++----- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/crates/pgt_completions/src/sanitization.rs b/crates/pgt_completions/src/sanitization.rs index 710d488d..f2f912bc 100644 --- a/crates/pgt_completions/src/sanitization.rs +++ b/crates/pgt_completions/src/sanitization.rs @@ -1,4 +1,4 @@ -use std::borrow::Cow; +use std::{borrow::Cow, cmp::max}; use pgt_text_size::TextSize; @@ -24,6 +24,7 @@ where if cursor_inbetween_nodes(params.tree, params.position) || cursor_prepared_to_write_token_after_last_node(params.tree, params.position) || cursor_before_semicolon(params.tree, params.position) + || cursor_on_a_dot(¶ms.text, params.position) { SanitizedCompletionParams::with_adjusted_sql(params) } else { @@ -44,12 +45,13 @@ where let mut sql_iter = params.text.chars(); - for idx in 0..cursor_pos + 1 { + let max = max(cursor_pos + 1, params.text.len()); + + for idx in 0..max { match sql_iter.next() { Some(c) => { if idx == cursor_pos { sql.push_str(SANITIZED_TOKEN); - sql.push(' '); } sql.push(c); } @@ -149,6 +151,11 @@ fn cursor_prepared_to_write_token_after_last_node( cursor_pos == tree.root_node().end_byte() + 1 } +fn cursor_on_a_dot(sql: &str, position: TextSize) -> bool { + let position: usize = position.into(); + sql.chars().nth(position - 1).is_some_and(|c| c == '.') +} + fn cursor_before_semicolon(tree: &tree_sitter::Tree, position: TextSize) -> bool { let mut cursor = tree.walk(); let mut leaf_node = tree.root_node(); @@ -198,7 +205,7 @@ mod tests { use pgt_text_size::TextSize; use crate::sanitization::{ - cursor_before_semicolon, cursor_inbetween_nodes, + cursor_before_semicolon, cursor_inbetween_nodes, cursor_on_a_dot, cursor_prepared_to_write_token_after_last_node, }; @@ -212,7 +219,7 @@ mod tests { .set_language(tree_sitter_sql::language()) .expect("Error loading sql language"); - let mut tree = parser.parse(input, None).unwrap(); + let mut tree = parser.parse(input.to_string(), None).unwrap(); // select | from users; <-- just right, one space after select token, one space before from assert!(cursor_inbetween_nodes(&mut tree, TextSize::new(7))); @@ -236,7 +243,7 @@ mod tests { .set_language(tree_sitter_sql::language()) .expect("Error loading sql language"); - let mut tree = parser.parse(input, None).unwrap(); + let mut tree = parser.parse(input.to_string(), None).unwrap(); // select * from| <-- still on previous token assert!(!cursor_prepared_to_write_token_after_last_node( @@ -263,6 +270,20 @@ mod tests { )); } + #[test] + fn on_a_dot() { + let input = "select * from private."; + + // select * from private.| <-- on a dot + assert!(cursor_on_a_dot(&input, TextSize::new(22))); + + // select * from private|. <-- before the dot + assert!(!cursor_on_a_dot(&input, TextSize::new(21))); + + // select * from private. | <-- too far off the dot + assert!(!cursor_on_a_dot(&input, TextSize::new(23))); + } + #[test] fn test_cursor_before_semicolon() { // Idx "13" is the exlusive end of `select * from` (first space after from) @@ -274,7 +295,7 @@ mod tests { .set_language(tree_sitter_sql::language()) .expect("Error loading sql language"); - let mut tree = parser.parse(input, None).unwrap(); + let mut tree = parser.parse(input.to_string(), None).unwrap(); // select * from ;| <-- it's after the statement assert!(!cursor_before_semicolon(&mut tree, TextSize::new(19))); From d2ce18a4bede066d928f9f50b4611e1c8a5f34ce Mon Sep 17 00:00:00 2001 From: Julian Date: Mon, 21 Apr 2025 16:48:10 +0200 Subject: [PATCH 09/22] relevance --- crates/pgt_completions/src/builder.rs | 91 +++------ .../pgt_completions/src/providers/columns.rs | 121 +---------- .../src/providers/functions.rs | 16 +- .../pgt_completions/src/providers/schemas.rs | 13 +- .../pgt_completions/src/providers/tables.rs | 16 +- crates/pgt_completions/src/relevance.rs | 188 +++++++++++++++++- .../src/relevance/filtering.rs | 105 ---------- .../pgt_completions/src/relevance/scoring.rs | 182 ----------------- 8 files changed, 240 insertions(+), 492 deletions(-) delete mode 100644 crates/pgt_completions/src/relevance/filtering.rs delete mode 100644 crates/pgt_completions/src/relevance/scoring.rs diff --git a/crates/pgt_completions/src/builder.rs b/crates/pgt_completions/src/builder.rs index aad1feb7..39439afb 100644 --- a/crates/pgt_completions/src/builder.rs +++ b/crates/pgt_completions/src/builder.rs @@ -1,84 +1,47 @@ -use crate::{ - CompletionItemKind, - context::CompletionContext, - item::CompletionItem, - relevance::{filtering::CompletionFilter, scoring::CompletionScore}, -}; +use crate::item::CompletionItem; -pub(crate) struct PossibleCompletionItem<'a> { - pub label: String, - pub description: String, - pub kind: CompletionItemKind, - pub score: CompletionScore<'a>, - pub filter: CompletionFilter<'a>, +pub(crate) struct CompletionBuilder { + items: Vec, } -pub(crate) struct CompletionBuilder<'a> { - items: Vec>, - ctx: &'a CompletionContext<'a>, -} - -impl<'a> CompletionBuilder<'a> { - pub fn new(ctx: &'a CompletionContext) -> Self { - CompletionBuilder { items: vec![], ctx } +impl CompletionBuilder { + pub fn new() -> Self { + CompletionBuilder { items: vec![] } } - pub fn add_item(&mut self, item: PossibleCompletionItem<'a>) { + pub fn add_item(&mut self, item: CompletionItem) { self.items.push(item); } - pub fn finish(self) -> Vec { - let mut items: Vec = self - .items - .into_iter() - .filter(|i| i.filter.is_relevant(self.ctx).is_some()) - .collect(); - - for item in items.iter_mut() { - item.score.calc_score(self.ctx); - } - - items.sort_by(|a, b| { - b.score - .get_score() - .cmp(&a.score.get_score()) - .then_with(|| a.label.cmp(&b.label)) - }); + pub fn finish(mut self) -> Vec { + self.items + .sort_by(|a, b| b.score.cmp(&a.score).then_with(|| a.label.cmp(&b.label))); - items.dedup_by(|a, b| a.label == b.label); - items.truncate(crate::LIMIT); + self.items.dedup_by(|a, b| a.label == b.label); + self.items.truncate(crate::LIMIT); - let should_preselect_first_item = should_preselect_first_item(&items); + let should_preselect_first_item = self.should_preselect_first_item(); - // a length of 481 should have a max_padding of 3 - let max_padding = items.len().to_string().len(); - - items + self.items .into_iter() .enumerate() - .map(|(idx, item)| { - let preselected = idx == 0 && should_preselect_first_item; - - CompletionItem { - description: item.description, - kind: item.kind, - label: item.label, - preselected, - score: item.score.get_score(), - sort_text: format!("{:0>padding$}", idx, padding = max_padding), + .map(|(idx, mut item)| { + if idx == 0 { + item.preselected = should_preselect_first_item; } + item }) .collect() } -} -fn should_preselect_first_item(items: &Vec) -> bool { - let mut items_iter = items.iter(); - let first = items_iter.next(); - let second = items_iter.next(); + fn should_preselect_first_item(&mut self) -> bool { + let mut items_iter = self.items.iter(); + let first = items_iter.next(); + let second = items_iter.next(); - first.is_some_and(|f| match second { - Some(s) => (f.score.get_score() - s.score.get_score()) > 10, - None => true, - }) + first.is_some_and(|f| match second { + Some(s) => (f.score - s.score) > 10, + None => true, + }) + } } diff --git a/crates/pgt_completions/src/providers/columns.rs b/crates/pgt_completions/src/providers/columns.rs index 6f8aa447..b792ba2c 100644 --- a/crates/pgt_completions/src/providers/columns.rs +++ b/crates/pgt_completions/src/providers/columns.rs @@ -1,21 +1,17 @@ use crate::{ - CompletionItemKind, - builder::{CompletionBuilder, PossibleCompletionItem}, - context::CompletionContext, - relevance::{CompletionRelevanceData, filtering::CompletionFilter, scoring::CompletionScore}, + CompletionItem, CompletionItemKind, builder::CompletionBuilder, context::CompletionContext, + relevance::CompletionRelevanceData, }; -pub fn complete_columns<'a>(ctx: &CompletionContext<'a>, builder: &mut CompletionBuilder<'a>) { +pub fn complete_columns(ctx: &CompletionContext, builder: &mut CompletionBuilder) { let available_columns = &ctx.schema_cache.columns; for col in available_columns { - let relevance = CompletionRelevanceData::Column(col); - - let item = PossibleCompletionItem { + let item = CompletionItem { label: col.name.clone(), - score: CompletionScore::from(relevance.clone()), - filter: CompletionFilter::from(relevance), + score: CompletionRelevanceData::Column(col).get_score(ctx), description: format!("Table: {}.{}", col.schema_name, col.table_name), + preselected: false, kind: CompletionItemKind::Column, }; @@ -26,10 +22,8 @@ pub fn complete_columns<'a>(ctx: &CompletionContext<'a>, builder: &mut Completio #[cfg(test)] mod tests { use crate::{ - CompletionItem, CompletionItemKind, complete, - test_helper::{ - CURSOR_POS, InputQuery, get_test_deps, get_test_params, test_against_connection_string, - }, + CompletionItem, complete, + test_helper::{CURSOR_POS, InputQuery, get_test_deps, get_test_params}, }; struct TestCase { @@ -210,8 +204,7 @@ mod tests { let has_column_in_first_four = |col: &'static str| { first_four .iter() - .find(|compl_item| compl_item.label.as_str() == col) - .is_some() + .any(|compl_item| compl_item.label.as_str() == col) }; assert!( @@ -231,100 +224,4 @@ mod tests { "`email` not present in first four completion items." ); } - - #[tokio::test] - async fn ignores_cols_in_from_clause() { - let setup = r#" - create schema private; - - create table private.users ( - id serial primary key, - name text, - address text, - email text - ); - "#; - - let test_case = TestCase { - message: "suggests user created tables first", - query: format!(r#"select * from private.{}"#, CURSOR_POS), - label: "", - description: "", - }; - - let (tree, cache) = get_test_deps(setup, test_case.get_input_query()).await; - let params = get_test_params(&tree, &cache, test_case.get_input_query()); - let results = complete(params); - - assert!( - !results - .into_iter() - .any(|item| item.kind == CompletionItemKind::Column) - ); - } - - #[tokio::test] - async fn prefers_columns_of_mentioned_tables() { - let setup = r#" - create schema private; - - create table private.users ( - id1 serial primary key, - name1 text, - address1 text, - email1 text - ); - - create table public.users ( - id2 serial primary key, - name2 text, - address2 text, - email2 text - ); - "#; - - { - let test_case = TestCase { - message: "", - query: format!(r#"select {} from users"#, CURSOR_POS), - label: "suggests from table", - description: "", - }; - - let (tree, cache) = get_test_deps(setup, test_case.get_input_query()).await; - let params = get_test_params(&tree, &cache, test_case.get_input_query()); - let results = complete(params); - - assert_eq!( - results - .into_iter() - .take(4) - .map(|item| item.label) - .collect::>(), - vec!["address2", "email2", "id2", "name2"] - ); - } - - { - let test_case = TestCase { - message: "", - query: format!(r#"select {} from private.users"#, CURSOR_POS), - label: "suggests from table", - description: "", - }; - - let (tree, cache) = get_test_deps(setup, test_case.get_input_query()).await; - let params = get_test_params(&tree, &cache, test_case.get_input_query()); - let results = complete(params); - - assert_eq!( - results - .into_iter() - .take(4) - .map(|item| item.label) - .collect::>(), - vec!["address1", "email1", "id1", "name1"] - ); - } - } } diff --git a/crates/pgt_completions/src/providers/functions.rs b/crates/pgt_completions/src/providers/functions.rs index b44a5ef5..b4a9c35a 100644 --- a/crates/pgt_completions/src/providers/functions.rs +++ b/crates/pgt_completions/src/providers/functions.rs @@ -1,21 +1,17 @@ use crate::{ - CompletionItemKind, - builder::{CompletionBuilder, PossibleCompletionItem}, - context::CompletionContext, - relevance::{CompletionRelevanceData, filtering::CompletionFilter, scoring::CompletionScore}, + CompletionItem, CompletionItemKind, builder::CompletionBuilder, context::CompletionContext, + relevance::CompletionRelevanceData, }; -pub fn complete_functions<'a>(ctx: &'a CompletionContext, builder: &mut CompletionBuilder<'a>) { +pub fn complete_functions(ctx: &CompletionContext, builder: &mut CompletionBuilder) { let available_functions = &ctx.schema_cache.functions; for func in available_functions { - let relevance = CompletionRelevanceData::Function(func); - - let item = PossibleCompletionItem { + let item = CompletionItem { label: func.name.clone(), - score: CompletionScore::from(relevance.clone()), - filter: CompletionFilter::from(relevance), + score: CompletionRelevanceData::Function(func).get_score(ctx), description: format!("Schema: {}", func.schema), + preselected: false, kind: CompletionItemKind::Function, }; diff --git a/crates/pgt_completions/src/providers/schemas.rs b/crates/pgt_completions/src/providers/schemas.rs index 3d8f622e..6e86ab56 100644 --- a/crates/pgt_completions/src/providers/schemas.rs +++ b/crates/pgt_completions/src/providers/schemas.rs @@ -1,21 +1,20 @@ use crate::{ - builder::{CompletionBuilder, PossibleCompletionItem}, - context::CompletionContext, - relevance::{CompletionRelevanceData, filtering::CompletionFilter, scoring::CompletionScore}, + CompletionItem, builder::CompletionBuilder, context::CompletionContext, + relevance::CompletionRelevanceData, }; -pub fn complete_schemas<'a>(ctx: &'a CompletionContext, builder: &mut CompletionBuilder<'a>) { +pub fn complete_schemas(ctx: &CompletionContext, builder: &mut CompletionBuilder) { let available_schemas = &ctx.schema_cache.schemas; for schema in available_schemas { let relevance = CompletionRelevanceData::Schema(schema); - let item = PossibleCompletionItem { + let item = CompletionItem { label: schema.name.clone(), description: "Schema".into(), + preselected: false, kind: crate::CompletionItemKind::Schema, - score: CompletionScore::from(relevance.clone()), - filter: CompletionFilter::from(relevance), + score: relevance.get_score(ctx), }; builder.add_item(item); diff --git a/crates/pgt_completions/src/providers/tables.rs b/crates/pgt_completions/src/providers/tables.rs index fcc8fa00..2074a4f1 100644 --- a/crates/pgt_completions/src/providers/tables.rs +++ b/crates/pgt_completions/src/providers/tables.rs @@ -1,21 +1,19 @@ use crate::{ - builder::{CompletionBuilder, PossibleCompletionItem}, + builder::CompletionBuilder, context::CompletionContext, - item::CompletionItemKind, - relevance::{CompletionRelevanceData, filtering::CompletionFilter, scoring::CompletionScore}, + item::{CompletionItem, CompletionItemKind}, + relevance::CompletionRelevanceData, }; -pub fn complete_tables<'a>(ctx: &'a CompletionContext, builder: &mut CompletionBuilder<'a>) { +pub fn complete_tables(ctx: &CompletionContext, builder: &mut CompletionBuilder) { let available_tables = &ctx.schema_cache.tables; for table in available_tables { - let relevance = CompletionRelevanceData::Table(table); - - let item = PossibleCompletionItem { + let item = CompletionItem { label: table.name.clone(), - score: CompletionScore::from(relevance.clone()), - filter: CompletionFilter::from(relevance), + score: CompletionRelevanceData::Table(table).get_score(ctx), description: format!("Schema: {}", table.schema), + preselected: false, kind: CompletionItemKind::Table, }; diff --git a/crates/pgt_completions/src/relevance.rs b/crates/pgt_completions/src/relevance.rs index 911a6433..2abb9f2c 100644 --- a/crates/pgt_completions/src/relevance.rs +++ b/crates/pgt_completions/src/relevance.rs @@ -1,10 +1,192 @@ -pub(crate) mod filtering; -pub(crate) mod scoring; +use crate::context::{ClauseType, CompletionContext, NodeText}; -#[derive(Debug, Clone)] +#[derive(Debug)] pub(crate) enum CompletionRelevanceData<'a> { Table(&'a pgt_schema_cache::Table), Function(&'a pgt_schema_cache::Function), Column(&'a pgt_schema_cache::Column), Schema(&'a pgt_schema_cache::Schema), } + +impl CompletionRelevanceData<'_> { + pub fn get_score(self, ctx: &CompletionContext) -> i32 { + CompletionRelevance::from(self).into_score(ctx) + } +} + +impl<'a> From> for CompletionRelevance<'a> { + fn from(value: CompletionRelevanceData<'a>) -> Self { + Self { + score: 0, + data: value, + } + } +} + +#[derive(Debug)] +pub(crate) struct CompletionRelevance<'a> { + score: i32, + data: CompletionRelevanceData<'a>, +} + +impl CompletionRelevance<'_> { + pub fn into_score(mut self, ctx: &CompletionContext) -> i32 { + self.check_is_user_defined(); + self.check_matches_schema(ctx); + self.check_matches_query_input(ctx); + self.check_is_invocation(ctx); + self.check_matching_clause_type(ctx); + self.check_relations_in_stmt(ctx); + + self.score + } + + fn check_matches_query_input(&mut self, ctx: &CompletionContext) { + let node = match ctx.node_under_cursor { + Some(node) => node, + None => return, + }; + + let content = match ctx.get_ts_node_content(node) { + Some(c) => match c { + NodeText::Original(s) => s, + NodeText::Replaced => return, + }, + None => return, + }; + + let name = match self.data { + CompletionRelevanceData::Function(f) => f.name.as_str(), + CompletionRelevanceData::Table(t) => t.name.as_str(), + CompletionRelevanceData::Column(c) => c.name.as_str(), + CompletionRelevanceData::Schema(s) => s.name.as_str(), + }; + + if name.starts_with(content) { + let len: i32 = content + .len() + .try_into() + .expect("The length of the input exceeds i32 capacity"); + + self.score += len * 10; + }; + } + + fn check_matching_clause_type(&mut self, ctx: &CompletionContext) { + let clause_type = match ctx.wrapping_clause_type.as_ref() { + None => return, + Some(ct) => ct, + }; + + let has_mentioned_tables = !ctx.mentioned_relations.is_empty(); + + self.score += match self.data { + CompletionRelevanceData::Table(_) => match clause_type { + ClauseType::From => 5, + ClauseType::Update => 15, + ClauseType::Delete => 15, + _ => -50, + }, + CompletionRelevanceData::Function(_) => match clause_type { + ClauseType::Select if !has_mentioned_tables => 15, + ClauseType::Select if has_mentioned_tables => 0, + ClauseType::From => 0, + _ => -50, + }, + CompletionRelevanceData::Column(_) => match clause_type { + ClauseType::Select if has_mentioned_tables => 10, + ClauseType::Select if !has_mentioned_tables => 0, + ClauseType::Where => 10, + _ => -15, + }, + CompletionRelevanceData::Schema(_) => match clause_type { + ClauseType::From => 10, + _ => -50, + }, + } + } + + fn check_is_invocation(&mut self, ctx: &CompletionContext) { + self.score += match self.data { + CompletionRelevanceData::Function(_) if ctx.is_invocation => 30, + CompletionRelevanceData::Function(_) if !ctx.is_invocation => -10, + _ if ctx.is_invocation => -10, + _ => 0, + }; + } + + fn check_matches_schema(&mut self, ctx: &CompletionContext) { + let schema_name = match ctx.schema_name.as_ref() { + None => return, + Some(n) => n, + }; + + let data_schema = self.get_schema_name(); + + if schema_name == data_schema { + self.score += 25; + } else { + self.score -= 10; + } + } + + fn get_schema_name(&self) -> &str { + match self.data { + CompletionRelevanceData::Function(f) => f.schema.as_str(), + CompletionRelevanceData::Table(t) => t.schema.as_str(), + CompletionRelevanceData::Column(c) => c.schema_name.as_str(), + CompletionRelevanceData::Schema(s) => s.name.as_str(), + } + } + + fn get_table_name(&self) -> Option<&str> { + match self.data { + CompletionRelevanceData::Column(c) => Some(c.table_name.as_str()), + CompletionRelevanceData::Table(t) => Some(t.name.as_str()), + _ => None, + } + } + + fn check_relations_in_stmt(&mut self, ctx: &CompletionContext) { + match self.data { + CompletionRelevanceData::Table(_) | CompletionRelevanceData::Function(_) => return, + _ => {} + } + + let schema = self.get_schema_name().to_string(); + let table_name = match self.get_table_name() { + Some(t) => t, + None => return, + }; + + if ctx + .mentioned_relations + .get(&Some(schema.to_string())) + .is_some_and(|tables| tables.contains(table_name)) + { + self.score += 45; + } else if ctx + .mentioned_relations + .get(&None) + .is_some_and(|tables| tables.contains(table_name)) + { + self.score += 30; + } + } + + fn check_is_user_defined(&mut self) { + let schema = self.get_schema_name().to_string(); + + let system_schemas = ["pg_catalog", "information_schema", "pg_toast"]; + + if system_schemas.contains(&schema.as_str()) { + self.score -= 10; + } + + // "public" is the default postgres schema where users + // create objects. Prefer it by a slight bit. + if schema.as_str() == "public" { + self.score += 2; + } + } +} diff --git a/crates/pgt_completions/src/relevance/filtering.rs b/crates/pgt_completions/src/relevance/filtering.rs deleted file mode 100644 index f1c7a739..00000000 --- a/crates/pgt_completions/src/relevance/filtering.rs +++ /dev/null @@ -1,105 +0,0 @@ -use crate::context::{ClauseType, CompletionContext}; - -use super::CompletionRelevanceData; - -#[derive(Debug)] -pub(crate) struct CompletionFilter<'a> { - data: CompletionRelevanceData<'a>, -} - -impl<'a> From> for CompletionFilter<'a> { - fn from(value: CompletionRelevanceData<'a>) -> Self { - Self { data: value } - } -} - -impl CompletionFilter<'_> { - pub fn is_relevant(&self, ctx: &CompletionContext) -> Option<()> { - self.completable_context(ctx)?; - self.check_clause(ctx)?; - self.check_invocation(ctx)?; - self.check_mentioned_schema(ctx)?; - - Some(()) - } - - fn completable_context(&self, ctx: &CompletionContext) -> Option<()> { - let current_node_kind = ctx.node_under_cursor.map(|n| n.kind()).unwrap_or(""); - - if current_node_kind.starts_with("keyword_") - || current_node_kind == "=" - || current_node_kind == "," - || current_node_kind == "literal" - || current_node_kind == "ERROR" - { - return None; - } - - Some(()) - } - - fn check_clause(&self, ctx: &CompletionContext) -> Option<()> { - let clause = ctx.wrapping_clause_type.as_ref(); - - match self.data { - CompletionRelevanceData::Table(_) => { - let in_select_clause = clause.is_some_and(|c| c == &ClauseType::Select); - let in_where_clause = clause.is_some_and(|c| c == &ClauseType::Where); - - if in_select_clause || in_where_clause { - return None; - }; - } - CompletionRelevanceData::Column(_) => { - let in_from_clause = clause.is_some_and(|c| c == &ClauseType::From); - - if in_from_clause { - return None; - } - } - _ => {} - } - - Some(()) - } - - fn check_invocation(&self, ctx: &CompletionContext) -> Option<()> { - if !ctx.is_invocation { - return Some(()); - } - - match self.data { - CompletionRelevanceData::Table(_) | CompletionRelevanceData::Column(_) => return None, - _ => {} - } - - Some(()) - } - - fn check_mentioned_schema(&self, ctx: &CompletionContext) -> Option<()> { - if ctx.schema_name.is_none() { - return Some(()); - } - - let name = ctx.schema_name.as_ref().unwrap(); - - let does_not_match = match self.data { - CompletionRelevanceData::Table(table) => &table.schema != name, - CompletionRelevanceData::Function(foo) => &foo.schema != name, - CompletionRelevanceData::Column(_) => { - // columns belong to tables, not schemas - true - } - CompletionRelevanceData::Schema(_) => { - // we should never allow schema suggestions if there already was one. - true - } - }; - - if does_not_match { - return None; - } - - Some(()) - } -} diff --git a/crates/pgt_completions/src/relevance/scoring.rs b/crates/pgt_completions/src/relevance/scoring.rs deleted file mode 100644 index 7c3f3a06..00000000 --- a/crates/pgt_completions/src/relevance/scoring.rs +++ /dev/null @@ -1,182 +0,0 @@ -use crate::context::{ClauseType, CompletionContext, NodeText}; - -use super::CompletionRelevanceData; - -#[derive(Debug)] -pub(crate) struct CompletionScore<'a> { - score: i32, - data: CompletionRelevanceData<'a>, -} - -impl<'a> From> for CompletionScore<'a> { - fn from(value: CompletionRelevanceData<'a>) -> Self { - Self { - score: 0, - data: value, - } - } -} - -impl CompletionScore<'_> { - pub fn get_score(&self) -> i32 { - self.score - } - - pub fn calc_score(&mut self, ctx: &CompletionContext) { - self.check_is_user_defined(); - self.check_matches_schema(ctx); - self.check_matches_query_input(ctx); - self.check_is_invocation(ctx); - self.check_matching_clause_type(ctx); - self.check_relations_in_stmt(ctx); - } - - fn check_matches_query_input(&mut self, ctx: &CompletionContext) { - let node = match ctx.node_under_cursor { - Some(node) => node, - None => return, - }; - - let content = match ctx.get_ts_node_content(node) { - Some(c) => match c { - NodeText::Original(s) => s, - NodeText::Replaced => return, - }, - None => return, - }; - - let name = match self.data { - CompletionRelevanceData::Function(f) => f.name.as_str(), - CompletionRelevanceData::Table(t) => t.name.as_str(), - CompletionRelevanceData::Column(c) => c.name.as_str(), - CompletionRelevanceData::Schema(s) => s.name.as_str(), - }; - - if name.starts_with(content) { - let len: i32 = content - .len() - .try_into() - .expect("The length of the input exceeds i32 capacity"); - - self.score += len * 10; - }; - } - - fn check_matching_clause_type(&mut self, ctx: &CompletionContext) { - let clause_type = match ctx.wrapping_clause_type.as_ref() { - None => return, - Some(ct) => ct, - }; - - let has_mentioned_tables = !ctx.mentioned_relations.is_empty(); - - self.score += match self.data { - CompletionRelevanceData::Table(_) => match clause_type { - ClauseType::From => 5, - ClauseType::Update => 15, - ClauseType::Delete => 15, - _ => -50, - }, - CompletionRelevanceData::Function(_) => match clause_type { - ClauseType::Select if !has_mentioned_tables => 15, - ClauseType::Select if has_mentioned_tables => 0, - ClauseType::From => 0, - _ => -50, - }, - CompletionRelevanceData::Column(_) => match clause_type { - ClauseType::Select if has_mentioned_tables => 10, - ClauseType::Select if !has_mentioned_tables => 0, - ClauseType::Where => 10, - _ => -15, - }, - CompletionRelevanceData::Schema(_) => match clause_type { - ClauseType::From => 10, - _ => -50, - }, - } - } - - fn check_is_invocation(&mut self, ctx: &CompletionContext) { - self.score += match self.data { - CompletionRelevanceData::Function(_) if ctx.is_invocation => 30, - CompletionRelevanceData::Function(_) if !ctx.is_invocation => -10, - _ if ctx.is_invocation => -10, - _ => 0, - }; - } - - fn check_matches_schema(&mut self, ctx: &CompletionContext) { - let schema_name = match ctx.schema_name.as_ref() { - None => return, - Some(n) => n, - }; - - let data_schema = self.get_schema_name(); - - if schema_name == data_schema { - self.score += 25; - } else { - self.score -= 10; - } - } - - fn get_schema_name(&self) -> &str { - match self.data { - CompletionRelevanceData::Function(f) => f.schema.as_str(), - CompletionRelevanceData::Table(t) => t.schema.as_str(), - CompletionRelevanceData::Column(c) => c.schema_name.as_str(), - CompletionRelevanceData::Schema(s) => s.name.as_str(), - } - } - - fn get_table_name(&self) -> Option<&str> { - match self.data { - CompletionRelevanceData::Column(c) => Some(c.table_name.as_str()), - CompletionRelevanceData::Table(t) => Some(t.name.as_str()), - _ => None, - } - } - - fn check_relations_in_stmt(&mut self, ctx: &CompletionContext) { - match self.data { - CompletionRelevanceData::Table(_) | CompletionRelevanceData::Function(_) => return, - _ => {} - } - - let schema = self.get_schema_name().to_string(); - let table_name = match self.get_table_name() { - Some(t) => t, - None => return, - }; - - if ctx - .mentioned_relations - .get(&Some(schema.to_string())) - .is_some_and(|tables| tables.contains(table_name)) - { - self.score += 45; - } else if ctx - .mentioned_relations - .get(&None) - .is_some_and(|tables| tables.contains(table_name)) - { - self.score += 30; - } - } - - fn check_is_user_defined(&mut self) { - let schema = self.get_schema_name().to_string(); - - let system_schemas = ["pg_catalog", "information_schema", "pg_toast"]; - - if system_schemas.contains(&schema.as_str()) { - self.score -= 10; - } - - // "public" is the default postgres schema where users - // create objects. Prefer it by a slight bit. - if schema.as_str() == "public" { - self.score += 2; - } - } -} From d315a5931f6f45bd69dc80fa5ee19addf9f1faf4 Mon Sep 17 00:00:00 2001 From: Julian Date: Mon, 21 Apr 2025 16:49:33 +0200 Subject: [PATCH 10/22] ok --- crates/pgt_completions/src/builder.rs | 91 +++++++++++++++++++-------- 1 file changed, 64 insertions(+), 27 deletions(-) diff --git a/crates/pgt_completions/src/builder.rs b/crates/pgt_completions/src/builder.rs index 39439afb..aad1feb7 100644 --- a/crates/pgt_completions/src/builder.rs +++ b/crates/pgt_completions/src/builder.rs @@ -1,47 +1,84 @@ -use crate::item::CompletionItem; +use crate::{ + CompletionItemKind, + context::CompletionContext, + item::CompletionItem, + relevance::{filtering::CompletionFilter, scoring::CompletionScore}, +}; -pub(crate) struct CompletionBuilder { - items: Vec, +pub(crate) struct PossibleCompletionItem<'a> { + pub label: String, + pub description: String, + pub kind: CompletionItemKind, + pub score: CompletionScore<'a>, + pub filter: CompletionFilter<'a>, } -impl CompletionBuilder { - pub fn new() -> Self { - CompletionBuilder { items: vec![] } +pub(crate) struct CompletionBuilder<'a> { + items: Vec>, + ctx: &'a CompletionContext<'a>, +} + +impl<'a> CompletionBuilder<'a> { + pub fn new(ctx: &'a CompletionContext) -> Self { + CompletionBuilder { items: vec![], ctx } } - pub fn add_item(&mut self, item: CompletionItem) { + pub fn add_item(&mut self, item: PossibleCompletionItem<'a>) { self.items.push(item); } - pub fn finish(mut self) -> Vec { - self.items - .sort_by(|a, b| b.score.cmp(&a.score).then_with(|| a.label.cmp(&b.label))); + pub fn finish(self) -> Vec { + let mut items: Vec = self + .items + .into_iter() + .filter(|i| i.filter.is_relevant(self.ctx).is_some()) + .collect(); + + for item in items.iter_mut() { + item.score.calc_score(self.ctx); + } + + items.sort_by(|a, b| { + b.score + .get_score() + .cmp(&a.score.get_score()) + .then_with(|| a.label.cmp(&b.label)) + }); - self.items.dedup_by(|a, b| a.label == b.label); - self.items.truncate(crate::LIMIT); + items.dedup_by(|a, b| a.label == b.label); + items.truncate(crate::LIMIT); - let should_preselect_first_item = self.should_preselect_first_item(); + let should_preselect_first_item = should_preselect_first_item(&items); - self.items + // a length of 481 should have a max_padding of 3 + let max_padding = items.len().to_string().len(); + + items .into_iter() .enumerate() - .map(|(idx, mut item)| { - if idx == 0 { - item.preselected = should_preselect_first_item; + .map(|(idx, item)| { + let preselected = idx == 0 && should_preselect_first_item; + + CompletionItem { + description: item.description, + kind: item.kind, + label: item.label, + preselected, + score: item.score.get_score(), + sort_text: format!("{:0>padding$}", idx, padding = max_padding), } - item }) .collect() } +} - fn should_preselect_first_item(&mut self) -> bool { - let mut items_iter = self.items.iter(); - let first = items_iter.next(); - let second = items_iter.next(); +fn should_preselect_first_item(items: &Vec) -> bool { + let mut items_iter = items.iter(); + let first = items_iter.next(); + let second = items_iter.next(); - first.is_some_and(|f| match second { - Some(s) => (f.score - s.score) > 10, - None => true, - }) - } + first.is_some_and(|f| match second { + Some(s) => (f.score.get_score() - s.score.get_score()) > 10, + None => true, + }) } From 29056adbd80ee752d6b428ea7cb9f7b6602dc118 Mon Sep 17 00:00:00 2001 From: Julian Date: Mon, 21 Apr 2025 16:50:03 +0200 Subject: [PATCH 11/22] ok --- crates/pgt_completions/src/builder.rs | 1 - crates/pgt_completions/src/item.rs | 1 - crates/pgt_completions/src/relevance.rs | 188 +----------------------- 3 files changed, 3 insertions(+), 187 deletions(-) diff --git a/crates/pgt_completions/src/builder.rs b/crates/pgt_completions/src/builder.rs index aad1feb7..f0571142 100644 --- a/crates/pgt_completions/src/builder.rs +++ b/crates/pgt_completions/src/builder.rs @@ -65,7 +65,6 @@ impl<'a> CompletionBuilder<'a> { label: item.label, preselected, score: item.score.get_score(), - sort_text: format!("{:0>padding$}", idx, padding = max_padding), } }) .collect() diff --git a/crates/pgt_completions/src/item.rs b/crates/pgt_completions/src/item.rs index 2a1c58fd..46d527b9 100644 --- a/crates/pgt_completions/src/item.rs +++ b/crates/pgt_completions/src/item.rs @@ -31,5 +31,4 @@ pub struct CompletionItem { pub preselected: bool, pub kind: CompletionItemKind, pub score: i32, - pub sort_text: String, } diff --git a/crates/pgt_completions/src/relevance.rs b/crates/pgt_completions/src/relevance.rs index 2abb9f2c..911a6433 100644 --- a/crates/pgt_completions/src/relevance.rs +++ b/crates/pgt_completions/src/relevance.rs @@ -1,192 +1,10 @@ -use crate::context::{ClauseType, CompletionContext, NodeText}; +pub(crate) mod filtering; +pub(crate) mod scoring; -#[derive(Debug)] +#[derive(Debug, Clone)] pub(crate) enum CompletionRelevanceData<'a> { Table(&'a pgt_schema_cache::Table), Function(&'a pgt_schema_cache::Function), Column(&'a pgt_schema_cache::Column), Schema(&'a pgt_schema_cache::Schema), } - -impl CompletionRelevanceData<'_> { - pub fn get_score(self, ctx: &CompletionContext) -> i32 { - CompletionRelevance::from(self).into_score(ctx) - } -} - -impl<'a> From> for CompletionRelevance<'a> { - fn from(value: CompletionRelevanceData<'a>) -> Self { - Self { - score: 0, - data: value, - } - } -} - -#[derive(Debug)] -pub(crate) struct CompletionRelevance<'a> { - score: i32, - data: CompletionRelevanceData<'a>, -} - -impl CompletionRelevance<'_> { - pub fn into_score(mut self, ctx: &CompletionContext) -> i32 { - self.check_is_user_defined(); - self.check_matches_schema(ctx); - self.check_matches_query_input(ctx); - self.check_is_invocation(ctx); - self.check_matching_clause_type(ctx); - self.check_relations_in_stmt(ctx); - - self.score - } - - fn check_matches_query_input(&mut self, ctx: &CompletionContext) { - let node = match ctx.node_under_cursor { - Some(node) => node, - None => return, - }; - - let content = match ctx.get_ts_node_content(node) { - Some(c) => match c { - NodeText::Original(s) => s, - NodeText::Replaced => return, - }, - None => return, - }; - - let name = match self.data { - CompletionRelevanceData::Function(f) => f.name.as_str(), - CompletionRelevanceData::Table(t) => t.name.as_str(), - CompletionRelevanceData::Column(c) => c.name.as_str(), - CompletionRelevanceData::Schema(s) => s.name.as_str(), - }; - - if name.starts_with(content) { - let len: i32 = content - .len() - .try_into() - .expect("The length of the input exceeds i32 capacity"); - - self.score += len * 10; - }; - } - - fn check_matching_clause_type(&mut self, ctx: &CompletionContext) { - let clause_type = match ctx.wrapping_clause_type.as_ref() { - None => return, - Some(ct) => ct, - }; - - let has_mentioned_tables = !ctx.mentioned_relations.is_empty(); - - self.score += match self.data { - CompletionRelevanceData::Table(_) => match clause_type { - ClauseType::From => 5, - ClauseType::Update => 15, - ClauseType::Delete => 15, - _ => -50, - }, - CompletionRelevanceData::Function(_) => match clause_type { - ClauseType::Select if !has_mentioned_tables => 15, - ClauseType::Select if has_mentioned_tables => 0, - ClauseType::From => 0, - _ => -50, - }, - CompletionRelevanceData::Column(_) => match clause_type { - ClauseType::Select if has_mentioned_tables => 10, - ClauseType::Select if !has_mentioned_tables => 0, - ClauseType::Where => 10, - _ => -15, - }, - CompletionRelevanceData::Schema(_) => match clause_type { - ClauseType::From => 10, - _ => -50, - }, - } - } - - fn check_is_invocation(&mut self, ctx: &CompletionContext) { - self.score += match self.data { - CompletionRelevanceData::Function(_) if ctx.is_invocation => 30, - CompletionRelevanceData::Function(_) if !ctx.is_invocation => -10, - _ if ctx.is_invocation => -10, - _ => 0, - }; - } - - fn check_matches_schema(&mut self, ctx: &CompletionContext) { - let schema_name = match ctx.schema_name.as_ref() { - None => return, - Some(n) => n, - }; - - let data_schema = self.get_schema_name(); - - if schema_name == data_schema { - self.score += 25; - } else { - self.score -= 10; - } - } - - fn get_schema_name(&self) -> &str { - match self.data { - CompletionRelevanceData::Function(f) => f.schema.as_str(), - CompletionRelevanceData::Table(t) => t.schema.as_str(), - CompletionRelevanceData::Column(c) => c.schema_name.as_str(), - CompletionRelevanceData::Schema(s) => s.name.as_str(), - } - } - - fn get_table_name(&self) -> Option<&str> { - match self.data { - CompletionRelevanceData::Column(c) => Some(c.table_name.as_str()), - CompletionRelevanceData::Table(t) => Some(t.name.as_str()), - _ => None, - } - } - - fn check_relations_in_stmt(&mut self, ctx: &CompletionContext) { - match self.data { - CompletionRelevanceData::Table(_) | CompletionRelevanceData::Function(_) => return, - _ => {} - } - - let schema = self.get_schema_name().to_string(); - let table_name = match self.get_table_name() { - Some(t) => t, - None => return, - }; - - if ctx - .mentioned_relations - .get(&Some(schema.to_string())) - .is_some_and(|tables| tables.contains(table_name)) - { - self.score += 45; - } else if ctx - .mentioned_relations - .get(&None) - .is_some_and(|tables| tables.contains(table_name)) - { - self.score += 30; - } - } - - fn check_is_user_defined(&mut self) { - let schema = self.get_schema_name().to_string(); - - let system_schemas = ["pg_catalog", "information_schema", "pg_toast"]; - - if system_schemas.contains(&schema.as_str()) { - self.score -= 10; - } - - // "public" is the default postgres schema where users - // create objects. Prefer it by a slight bit. - if schema.as_str() == "public" { - self.score += 2; - } - } -} From 6e73a3b4858f2652fcdeef250f741d7e6a5b94ff Mon Sep 17 00:00:00 2001 From: Julian Date: Mon, 21 Apr 2025 16:50:27 +0200 Subject: [PATCH 12/22] add folder --- .../src/relevance/filtering.rs | 105 ++++++++++ .../pgt_completions/src/relevance/scoring.rs | 182 ++++++++++++++++++ 2 files changed, 287 insertions(+) create mode 100644 crates/pgt_completions/src/relevance/filtering.rs create mode 100644 crates/pgt_completions/src/relevance/scoring.rs diff --git a/crates/pgt_completions/src/relevance/filtering.rs b/crates/pgt_completions/src/relevance/filtering.rs new file mode 100644 index 00000000..f1c7a739 --- /dev/null +++ b/crates/pgt_completions/src/relevance/filtering.rs @@ -0,0 +1,105 @@ +use crate::context::{ClauseType, CompletionContext}; + +use super::CompletionRelevanceData; + +#[derive(Debug)] +pub(crate) struct CompletionFilter<'a> { + data: CompletionRelevanceData<'a>, +} + +impl<'a> From> for CompletionFilter<'a> { + fn from(value: CompletionRelevanceData<'a>) -> Self { + Self { data: value } + } +} + +impl CompletionFilter<'_> { + pub fn is_relevant(&self, ctx: &CompletionContext) -> Option<()> { + self.completable_context(ctx)?; + self.check_clause(ctx)?; + self.check_invocation(ctx)?; + self.check_mentioned_schema(ctx)?; + + Some(()) + } + + fn completable_context(&self, ctx: &CompletionContext) -> Option<()> { + let current_node_kind = ctx.node_under_cursor.map(|n| n.kind()).unwrap_or(""); + + if current_node_kind.starts_with("keyword_") + || current_node_kind == "=" + || current_node_kind == "," + || current_node_kind == "literal" + || current_node_kind == "ERROR" + { + return None; + } + + Some(()) + } + + fn check_clause(&self, ctx: &CompletionContext) -> Option<()> { + let clause = ctx.wrapping_clause_type.as_ref(); + + match self.data { + CompletionRelevanceData::Table(_) => { + let in_select_clause = clause.is_some_and(|c| c == &ClauseType::Select); + let in_where_clause = clause.is_some_and(|c| c == &ClauseType::Where); + + if in_select_clause || in_where_clause { + return None; + }; + } + CompletionRelevanceData::Column(_) => { + let in_from_clause = clause.is_some_and(|c| c == &ClauseType::From); + + if in_from_clause { + return None; + } + } + _ => {} + } + + Some(()) + } + + fn check_invocation(&self, ctx: &CompletionContext) -> Option<()> { + if !ctx.is_invocation { + return Some(()); + } + + match self.data { + CompletionRelevanceData::Table(_) | CompletionRelevanceData::Column(_) => return None, + _ => {} + } + + Some(()) + } + + fn check_mentioned_schema(&self, ctx: &CompletionContext) -> Option<()> { + if ctx.schema_name.is_none() { + return Some(()); + } + + let name = ctx.schema_name.as_ref().unwrap(); + + let does_not_match = match self.data { + CompletionRelevanceData::Table(table) => &table.schema != name, + CompletionRelevanceData::Function(foo) => &foo.schema != name, + CompletionRelevanceData::Column(_) => { + // columns belong to tables, not schemas + true + } + CompletionRelevanceData::Schema(_) => { + // we should never allow schema suggestions if there already was one. + true + } + }; + + if does_not_match { + return None; + } + + Some(()) + } +} diff --git a/crates/pgt_completions/src/relevance/scoring.rs b/crates/pgt_completions/src/relevance/scoring.rs new file mode 100644 index 00000000..7c3f3a06 --- /dev/null +++ b/crates/pgt_completions/src/relevance/scoring.rs @@ -0,0 +1,182 @@ +use crate::context::{ClauseType, CompletionContext, NodeText}; + +use super::CompletionRelevanceData; + +#[derive(Debug)] +pub(crate) struct CompletionScore<'a> { + score: i32, + data: CompletionRelevanceData<'a>, +} + +impl<'a> From> for CompletionScore<'a> { + fn from(value: CompletionRelevanceData<'a>) -> Self { + Self { + score: 0, + data: value, + } + } +} + +impl CompletionScore<'_> { + pub fn get_score(&self) -> i32 { + self.score + } + + pub fn calc_score(&mut self, ctx: &CompletionContext) { + self.check_is_user_defined(); + self.check_matches_schema(ctx); + self.check_matches_query_input(ctx); + self.check_is_invocation(ctx); + self.check_matching_clause_type(ctx); + self.check_relations_in_stmt(ctx); + } + + fn check_matches_query_input(&mut self, ctx: &CompletionContext) { + let node = match ctx.node_under_cursor { + Some(node) => node, + None => return, + }; + + let content = match ctx.get_ts_node_content(node) { + Some(c) => match c { + NodeText::Original(s) => s, + NodeText::Replaced => return, + }, + None => return, + }; + + let name = match self.data { + CompletionRelevanceData::Function(f) => f.name.as_str(), + CompletionRelevanceData::Table(t) => t.name.as_str(), + CompletionRelevanceData::Column(c) => c.name.as_str(), + CompletionRelevanceData::Schema(s) => s.name.as_str(), + }; + + if name.starts_with(content) { + let len: i32 = content + .len() + .try_into() + .expect("The length of the input exceeds i32 capacity"); + + self.score += len * 10; + }; + } + + fn check_matching_clause_type(&mut self, ctx: &CompletionContext) { + let clause_type = match ctx.wrapping_clause_type.as_ref() { + None => return, + Some(ct) => ct, + }; + + let has_mentioned_tables = !ctx.mentioned_relations.is_empty(); + + self.score += match self.data { + CompletionRelevanceData::Table(_) => match clause_type { + ClauseType::From => 5, + ClauseType::Update => 15, + ClauseType::Delete => 15, + _ => -50, + }, + CompletionRelevanceData::Function(_) => match clause_type { + ClauseType::Select if !has_mentioned_tables => 15, + ClauseType::Select if has_mentioned_tables => 0, + ClauseType::From => 0, + _ => -50, + }, + CompletionRelevanceData::Column(_) => match clause_type { + ClauseType::Select if has_mentioned_tables => 10, + ClauseType::Select if !has_mentioned_tables => 0, + ClauseType::Where => 10, + _ => -15, + }, + CompletionRelevanceData::Schema(_) => match clause_type { + ClauseType::From => 10, + _ => -50, + }, + } + } + + fn check_is_invocation(&mut self, ctx: &CompletionContext) { + self.score += match self.data { + CompletionRelevanceData::Function(_) if ctx.is_invocation => 30, + CompletionRelevanceData::Function(_) if !ctx.is_invocation => -10, + _ if ctx.is_invocation => -10, + _ => 0, + }; + } + + fn check_matches_schema(&mut self, ctx: &CompletionContext) { + let schema_name = match ctx.schema_name.as_ref() { + None => return, + Some(n) => n, + }; + + let data_schema = self.get_schema_name(); + + if schema_name == data_schema { + self.score += 25; + } else { + self.score -= 10; + } + } + + fn get_schema_name(&self) -> &str { + match self.data { + CompletionRelevanceData::Function(f) => f.schema.as_str(), + CompletionRelevanceData::Table(t) => t.schema.as_str(), + CompletionRelevanceData::Column(c) => c.schema_name.as_str(), + CompletionRelevanceData::Schema(s) => s.name.as_str(), + } + } + + fn get_table_name(&self) -> Option<&str> { + match self.data { + CompletionRelevanceData::Column(c) => Some(c.table_name.as_str()), + CompletionRelevanceData::Table(t) => Some(t.name.as_str()), + _ => None, + } + } + + fn check_relations_in_stmt(&mut self, ctx: &CompletionContext) { + match self.data { + CompletionRelevanceData::Table(_) | CompletionRelevanceData::Function(_) => return, + _ => {} + } + + let schema = self.get_schema_name().to_string(); + let table_name = match self.get_table_name() { + Some(t) => t, + None => return, + }; + + if ctx + .mentioned_relations + .get(&Some(schema.to_string())) + .is_some_and(|tables| tables.contains(table_name)) + { + self.score += 45; + } else if ctx + .mentioned_relations + .get(&None) + .is_some_and(|tables| tables.contains(table_name)) + { + self.score += 30; + } + } + + fn check_is_user_defined(&mut self) { + let schema = self.get_schema_name().to_string(); + + let system_schemas = ["pg_catalog", "information_schema", "pg_toast"]; + + if system_schemas.contains(&schema.as_str()) { + self.score -= 10; + } + + // "public" is the default postgres schema where users + // create objects. Prefer it by a slight bit. + if schema.as_str() == "public" { + self.score += 2; + } + } +} From a53cc46640436cbb16469bb6ffdffaefcf303f40 Mon Sep 17 00:00:00 2001 From: Julian Date: Mon, 21 Apr 2025 16:51:09 +0200 Subject: [PATCH 13/22] fixes --- crates/pgt_completions/src/builder.rs | 3 - .../pgt_completions/src/providers/columns.rs | 121 ++++++++++++++++-- .../src/providers/functions.rs | 16 ++- .../pgt_completions/src/providers/schemas.rs | 13 +- .../pgt_completions/src/providers/tables.rs | 16 ++- 5 files changed, 138 insertions(+), 31 deletions(-) diff --git a/crates/pgt_completions/src/builder.rs b/crates/pgt_completions/src/builder.rs index f0571142..45c36d3b 100644 --- a/crates/pgt_completions/src/builder.rs +++ b/crates/pgt_completions/src/builder.rs @@ -50,9 +50,6 @@ impl<'a> CompletionBuilder<'a> { let should_preselect_first_item = should_preselect_first_item(&items); - // a length of 481 should have a max_padding of 3 - let max_padding = items.len().to_string().len(); - items .into_iter() .enumerate() diff --git a/crates/pgt_completions/src/providers/columns.rs b/crates/pgt_completions/src/providers/columns.rs index b792ba2c..6f8aa447 100644 --- a/crates/pgt_completions/src/providers/columns.rs +++ b/crates/pgt_completions/src/providers/columns.rs @@ -1,17 +1,21 @@ use crate::{ - CompletionItem, CompletionItemKind, builder::CompletionBuilder, context::CompletionContext, - relevance::CompletionRelevanceData, + CompletionItemKind, + builder::{CompletionBuilder, PossibleCompletionItem}, + context::CompletionContext, + relevance::{CompletionRelevanceData, filtering::CompletionFilter, scoring::CompletionScore}, }; -pub fn complete_columns(ctx: &CompletionContext, builder: &mut CompletionBuilder) { +pub fn complete_columns<'a>(ctx: &CompletionContext<'a>, builder: &mut CompletionBuilder<'a>) { let available_columns = &ctx.schema_cache.columns; for col in available_columns { - let item = CompletionItem { + let relevance = CompletionRelevanceData::Column(col); + + let item = PossibleCompletionItem { label: col.name.clone(), - score: CompletionRelevanceData::Column(col).get_score(ctx), + score: CompletionScore::from(relevance.clone()), + filter: CompletionFilter::from(relevance), description: format!("Table: {}.{}", col.schema_name, col.table_name), - preselected: false, kind: CompletionItemKind::Column, }; @@ -22,8 +26,10 @@ pub fn complete_columns(ctx: &CompletionContext, builder: &mut CompletionBuilder #[cfg(test)] mod tests { use crate::{ - CompletionItem, complete, - test_helper::{CURSOR_POS, InputQuery, get_test_deps, get_test_params}, + CompletionItem, CompletionItemKind, complete, + test_helper::{ + CURSOR_POS, InputQuery, get_test_deps, get_test_params, test_against_connection_string, + }, }; struct TestCase { @@ -204,7 +210,8 @@ mod tests { let has_column_in_first_four = |col: &'static str| { first_four .iter() - .any(|compl_item| compl_item.label.as_str() == col) + .find(|compl_item| compl_item.label.as_str() == col) + .is_some() }; assert!( @@ -224,4 +231,100 @@ mod tests { "`email` not present in first four completion items." ); } + + #[tokio::test] + async fn ignores_cols_in_from_clause() { + let setup = r#" + create schema private; + + create table private.users ( + id serial primary key, + name text, + address text, + email text + ); + "#; + + let test_case = TestCase { + message: "suggests user created tables first", + query: format!(r#"select * from private.{}"#, CURSOR_POS), + label: "", + description: "", + }; + + let (tree, cache) = get_test_deps(setup, test_case.get_input_query()).await; + let params = get_test_params(&tree, &cache, test_case.get_input_query()); + let results = complete(params); + + assert!( + !results + .into_iter() + .any(|item| item.kind == CompletionItemKind::Column) + ); + } + + #[tokio::test] + async fn prefers_columns_of_mentioned_tables() { + let setup = r#" + create schema private; + + create table private.users ( + id1 serial primary key, + name1 text, + address1 text, + email1 text + ); + + create table public.users ( + id2 serial primary key, + name2 text, + address2 text, + email2 text + ); + "#; + + { + let test_case = TestCase { + message: "", + query: format!(r#"select {} from users"#, CURSOR_POS), + label: "suggests from table", + description: "", + }; + + let (tree, cache) = get_test_deps(setup, test_case.get_input_query()).await; + let params = get_test_params(&tree, &cache, test_case.get_input_query()); + let results = complete(params); + + assert_eq!( + results + .into_iter() + .take(4) + .map(|item| item.label) + .collect::>(), + vec!["address2", "email2", "id2", "name2"] + ); + } + + { + let test_case = TestCase { + message: "", + query: format!(r#"select {} from private.users"#, CURSOR_POS), + label: "suggests from table", + description: "", + }; + + let (tree, cache) = get_test_deps(setup, test_case.get_input_query()).await; + let params = get_test_params(&tree, &cache, test_case.get_input_query()); + let results = complete(params); + + assert_eq!( + results + .into_iter() + .take(4) + .map(|item| item.label) + .collect::>(), + vec!["address1", "email1", "id1", "name1"] + ); + } + } } diff --git a/crates/pgt_completions/src/providers/functions.rs b/crates/pgt_completions/src/providers/functions.rs index b4a9c35a..b44a5ef5 100644 --- a/crates/pgt_completions/src/providers/functions.rs +++ b/crates/pgt_completions/src/providers/functions.rs @@ -1,17 +1,21 @@ use crate::{ - CompletionItem, CompletionItemKind, builder::CompletionBuilder, context::CompletionContext, - relevance::CompletionRelevanceData, + CompletionItemKind, + builder::{CompletionBuilder, PossibleCompletionItem}, + context::CompletionContext, + relevance::{CompletionRelevanceData, filtering::CompletionFilter, scoring::CompletionScore}, }; -pub fn complete_functions(ctx: &CompletionContext, builder: &mut CompletionBuilder) { +pub fn complete_functions<'a>(ctx: &'a CompletionContext, builder: &mut CompletionBuilder<'a>) { let available_functions = &ctx.schema_cache.functions; for func in available_functions { - let item = CompletionItem { + let relevance = CompletionRelevanceData::Function(func); + + let item = PossibleCompletionItem { label: func.name.clone(), - score: CompletionRelevanceData::Function(func).get_score(ctx), + score: CompletionScore::from(relevance.clone()), + filter: CompletionFilter::from(relevance), description: format!("Schema: {}", func.schema), - preselected: false, kind: CompletionItemKind::Function, }; diff --git a/crates/pgt_completions/src/providers/schemas.rs b/crates/pgt_completions/src/providers/schemas.rs index 6e86ab56..3d8f622e 100644 --- a/crates/pgt_completions/src/providers/schemas.rs +++ b/crates/pgt_completions/src/providers/schemas.rs @@ -1,20 +1,21 @@ use crate::{ - CompletionItem, builder::CompletionBuilder, context::CompletionContext, - relevance::CompletionRelevanceData, + builder::{CompletionBuilder, PossibleCompletionItem}, + context::CompletionContext, + relevance::{CompletionRelevanceData, filtering::CompletionFilter, scoring::CompletionScore}, }; -pub fn complete_schemas(ctx: &CompletionContext, builder: &mut CompletionBuilder) { +pub fn complete_schemas<'a>(ctx: &'a CompletionContext, builder: &mut CompletionBuilder<'a>) { let available_schemas = &ctx.schema_cache.schemas; for schema in available_schemas { let relevance = CompletionRelevanceData::Schema(schema); - let item = CompletionItem { + let item = PossibleCompletionItem { label: schema.name.clone(), description: "Schema".into(), - preselected: false, kind: crate::CompletionItemKind::Schema, - score: relevance.get_score(ctx), + score: CompletionScore::from(relevance.clone()), + filter: CompletionFilter::from(relevance), }; builder.add_item(item); diff --git a/crates/pgt_completions/src/providers/tables.rs b/crates/pgt_completions/src/providers/tables.rs index 2074a4f1..fcc8fa00 100644 --- a/crates/pgt_completions/src/providers/tables.rs +++ b/crates/pgt_completions/src/providers/tables.rs @@ -1,19 +1,21 @@ use crate::{ - builder::CompletionBuilder, + builder::{CompletionBuilder, PossibleCompletionItem}, context::CompletionContext, - item::{CompletionItem, CompletionItemKind}, - relevance::CompletionRelevanceData, + item::CompletionItemKind, + relevance::{CompletionRelevanceData, filtering::CompletionFilter, scoring::CompletionScore}, }; -pub fn complete_tables(ctx: &CompletionContext, builder: &mut CompletionBuilder) { +pub fn complete_tables<'a>(ctx: &'a CompletionContext, builder: &mut CompletionBuilder<'a>) { let available_tables = &ctx.schema_cache.tables; for table in available_tables { - let item = CompletionItem { + let relevance = CompletionRelevanceData::Table(table); + + let item = PossibleCompletionItem { label: table.name.clone(), - score: CompletionRelevanceData::Table(table).get_score(ctx), + score: CompletionScore::from(relevance.clone()), + filter: CompletionFilter::from(relevance), description: format!("Schema: {}", table.schema), - preselected: false, kind: CompletionItemKind::Table, }; From 5065b8eb44f9d335af14cfcc30ea1ede065ea8ea Mon Sep 17 00:00:00 2001 From: Julian Date: Mon, 21 Apr 2025 16:51:32 +0200 Subject: [PATCH 14/22] oops --- crates/pgt_lsp/src/handlers/completions.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/pgt_lsp/src/handlers/completions.rs b/crates/pgt_lsp/src/handlers/completions.rs index dee895af..f9b68e7d 100644 --- a/crates/pgt_lsp/src/handlers/completions.rs +++ b/crates/pgt_lsp/src/handlers/completions.rs @@ -32,11 +32,10 @@ pub fn get_completions( label: i.label, label_details: Some(CompletionItemLabelDetails { description: Some(i.description), - detail: Some(format!(" {}", i.kind.to_string())), + detail: None, }), preselect: Some(i.preselected), kind: Some(to_lsp_types_completion_item_kind(i.kind)), - sort_text: Some(i.sort_text), ..CompletionItem::default() }) .collect(); From 3e9462ff131b95b4e5c4180a9aa0bf948312af99 Mon Sep 17 00:00:00 2001 From: Julian Date: Mon, 21 Apr 2025 16:52:33 +0200 Subject: [PATCH 15/22] ok --- crates/pgt_completions/src/relevance/filtering.rs | 2 +- crates/pgt_completions/src/test_helper.rs | 7 ------- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/crates/pgt_completions/src/relevance/filtering.rs b/crates/pgt_completions/src/relevance/filtering.rs index f1c7a739..214fda56 100644 --- a/crates/pgt_completions/src/relevance/filtering.rs +++ b/crates/pgt_completions/src/relevance/filtering.rs @@ -85,7 +85,7 @@ impl CompletionFilter<'_> { let does_not_match = match self.data { CompletionRelevanceData::Table(table) => &table.schema != name, - CompletionRelevanceData::Function(foo) => &foo.schema != name, + CompletionRelevanceData::Function(f) => &f.schema != name, CompletionRelevanceData::Column(_) => { // columns belong to tables, not schemas true diff --git a/crates/pgt_completions/src/test_helper.rs b/crates/pgt_completions/src/test_helper.rs index 5c8bea9d..fc2cf403 100644 --- a/crates/pgt_completions/src/test_helper.rs +++ b/crates/pgt_completions/src/test_helper.rs @@ -12,13 +12,6 @@ pub struct InputQuery { position: usize, } -impl InputQuery { - pub fn with_sanitized_whitespace(sql: &str) -> Self { - let strs: Vec<&str> = sql.split_whitespace().collect(); - strs.join(" ").as_str().into() - } -} - impl From<&str> for InputQuery { fn from(value: &str) -> Self { let position = value From db490405a9ddd6c04ed2c3ff391c2a6a406c6222 Mon Sep 17 00:00:00 2001 From: Julian Date: Mon, 21 Apr 2025 16:55:30 +0200 Subject: [PATCH 16/22] revert things --- crates/pgt_completions/src/item.rs | 15 +---------- crates/pgt_completions/src/sanitization.rs | 29 +++------------------- crates/pgt_lsp/src/capabilities.rs | 2 +- crates/pgt_treesitter_queries/src/lib.rs | 27 -------------------- 4 files changed, 6 insertions(+), 67 deletions(-) diff --git a/crates/pgt_completions/src/item.rs b/crates/pgt_completions/src/item.rs index 46d527b9..1f306d78 100644 --- a/crates/pgt_completions/src/item.rs +++ b/crates/pgt_completions/src/item.rs @@ -1,5 +1,3 @@ -use std::fmt::Display; - use serde::{Deserialize, Serialize}; #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] @@ -12,23 +10,12 @@ pub enum CompletionItemKind { Schema, } -impl Display for CompletionItemKind { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - CompletionItemKind::Table => write!(f, "Table"), - CompletionItemKind::Function => write!(f, "Function"), - CompletionItemKind::Column => write!(f, "Column"), - CompletionItemKind::Schema => write!(f, "Schema"), - } - } -} - #[derive(Debug, Serialize, Deserialize)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] pub struct CompletionItem { pub label: String, + pub(crate) score: i32, pub description: String, pub preselected: bool, pub kind: CompletionItemKind, - pub score: i32, } diff --git a/crates/pgt_completions/src/sanitization.rs b/crates/pgt_completions/src/sanitization.rs index 921c6e1f..710d488d 100644 --- a/crates/pgt_completions/src/sanitization.rs +++ b/crates/pgt_completions/src/sanitization.rs @@ -1,4 +1,4 @@ -use std::{borrow::Cow, cmp::max}; +use std::borrow::Cow; use pgt_text_size::TextSize; @@ -24,7 +24,6 @@ where if cursor_inbetween_nodes(params.tree, params.position) || cursor_prepared_to_write_token_after_last_node(params.tree, params.position) || cursor_before_semicolon(params.tree, params.position) - || cursor_on_a_dot(¶ms.text, params.position) { SanitizedCompletionParams::with_adjusted_sql(params) } else { @@ -45,13 +44,12 @@ where let mut sql_iter = params.text.chars(); - let max = max(cursor_pos + 1, params.text.len()); - - for idx in 0..max { + for idx in 0..cursor_pos + 1 { match sql_iter.next() { Some(c) => { if idx == cursor_pos { sql.push_str(SANITIZED_TOKEN); + sql.push(' '); } sql.push(c); } @@ -151,11 +149,6 @@ fn cursor_prepared_to_write_token_after_last_node( cursor_pos == tree.root_node().end_byte() + 1 } -fn cursor_on_a_dot(sql: &str, position: TextSize) -> bool { - let position: usize = position.into(); - sql.chars().nth(position - 1).is_some_and(|c| c == '.') -} - fn cursor_before_semicolon(tree: &tree_sitter::Tree, position: TextSize) -> bool { let mut cursor = tree.walk(); let mut leaf_node = tree.root_node(); @@ -205,7 +198,7 @@ mod tests { use pgt_text_size::TextSize; use crate::sanitization::{ - cursor_before_semicolon, cursor_inbetween_nodes, cursor_on_a_dot, + cursor_before_semicolon, cursor_inbetween_nodes, cursor_prepared_to_write_token_after_last_node, }; @@ -270,20 +263,6 @@ mod tests { )); } - #[test] - fn on_a_dot() { - let input = "select * from private."; - - // select * from private.| <-- on a dot - assert!(cursor_on_a_dot(&input, TextSize::new(22))); - - // select * from private|. <-- before the dot - assert!(!cursor_on_a_dot(&input, TextSize::new(21))); - - // select * from private. | <-- too far off the dot - assert!(!cursor_on_a_dot(&input, TextSize::new(23))); - } - #[test] fn test_cursor_before_semicolon() { // Idx "13" is the exlusive end of `select * from` (first space after from) diff --git a/crates/pgt_lsp/src/capabilities.rs b/crates/pgt_lsp/src/capabilities.rs index b3e35b69..a801dd5e 100644 --- a/crates/pgt_lsp/src/capabilities.rs +++ b/crates/pgt_lsp/src/capabilities.rs @@ -37,7 +37,7 @@ pub(crate) fn server_capabilities(capabilities: &ClientCapabilities) -> ServerCa // The request is used to get more information about a simple CompletionItem. resolve_provider: None, - trigger_characters: Some(vec![".".to_owned(), " ".to_owned()]), + trigger_characters: Some(vec![".".to_owned(), ",".to_owned(), " ".to_owned()]), // No character will lead to automatically inserting the selected completion-item all_commit_characters: None, diff --git a/crates/pgt_treesitter_queries/src/lib.rs b/crates/pgt_treesitter_queries/src/lib.rs index 3c6ef183..7d2ba61b 100644 --- a/crates/pgt_treesitter_queries/src/lib.rs +++ b/crates/pgt_treesitter_queries/src/lib.rs @@ -185,31 +185,4 @@ on sq1.id = pt.id; assert_eq!(results[0].get_schema(sql), Some("private".into())); assert_eq!(results[0].get_table(sql), "something"); } - - #[test] - fn picks_it_up_without_schemas() { - let sql = r#"select * from users"#; - - let mut parser = tree_sitter::Parser::new(); - parser.set_language(tree_sitter_sql::language()).unwrap(); - - let tree = parser.parse(sql, None).unwrap(); - - // trust me bro - - let mut executor = TreeSitterQueriesExecutor::new(tree.root_node(), sql); - - executor.add_query_results::(); - - let results: Vec<&RelationMatch> = executor - .get_iter(None) - .filter_map(|q| q.try_into().ok()) - .collect(); - - println!("{:?}", results); - - assert_eq!(results.len(), 1); - assert_eq!(results[0].get_schema(sql), None); - assert_eq!(results[0].get_table(sql), "users"); - } } From b7913b10d6086db189970bf6ee1aced0e5bd5ae9 Mon Sep 17 00:00:00 2001 From: Julian Date: Mon, 21 Apr 2025 17:03:38 +0200 Subject: [PATCH 17/22] oK --- crates/pgt_completions/src/builder.rs | 12 +++++++++++- crates/pgt_completions/src/item.rs | 18 +++++++++++++++++- crates/pgt_lsp/src/handlers/completions.rs | 3 ++- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/crates/pgt_completions/src/builder.rs b/crates/pgt_completions/src/builder.rs index 45c36d3b..40db6e1a 100644 --- a/crates/pgt_completions/src/builder.rs +++ b/crates/pgt_completions/src/builder.rs @@ -50,6 +50,14 @@ impl<'a> CompletionBuilder<'a> { let should_preselect_first_item = should_preselect_first_item(&items); + /* + * LSP Clients themselves sort the completion items. + * They'll use the `sort_text` property if present (or fallback to the `label`). + * Since our items are already sorted, we're 'hijacking' the sort_text. + * We're simply adding the index of the item, padded by zeroes to the max length. + */ + let max_padding = items.len().to_string().len(); + items .into_iter() .enumerate() @@ -61,7 +69,9 @@ impl<'a> CompletionBuilder<'a> { kind: item.kind, label: item.label, preselected, - score: item.score.get_score(), + + // wonderous Rust syntax ftw + sort_text: format!("{:0>padding$}", idx, padding = max_padding), } }) .collect() diff --git a/crates/pgt_completions/src/item.rs b/crates/pgt_completions/src/item.rs index 1f306d78..2af853c0 100644 --- a/crates/pgt_completions/src/item.rs +++ b/crates/pgt_completions/src/item.rs @@ -1,3 +1,5 @@ +use std::fmt::Display; + use serde::{Deserialize, Serialize}; #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] @@ -10,12 +12,26 @@ pub enum CompletionItemKind { Schema, } +impl Display for CompletionItemKind { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let txt = match self { + CompletionItemKind::Table => "Table", + CompletionItemKind::Function => "Function", + CompletionItemKind::Column => "Column", + CompletionItemKind::Schema => "Schema", + }; + + write!(f, "{txt}") + } +} + #[derive(Debug, Serialize, Deserialize)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] pub struct CompletionItem { pub label: String, - pub(crate) score: i32, pub description: String, pub preselected: bool, pub kind: CompletionItemKind, + /// String used for sorting by LSP clients. + pub sort_text: String, } diff --git a/crates/pgt_lsp/src/handlers/completions.rs b/crates/pgt_lsp/src/handlers/completions.rs index f9b68e7d..9dd547f9 100644 --- a/crates/pgt_lsp/src/handlers/completions.rs +++ b/crates/pgt_lsp/src/handlers/completions.rs @@ -32,9 +32,10 @@ pub fn get_completions( label: i.label, label_details: Some(CompletionItemLabelDetails { description: Some(i.description), - detail: None, + detail: Some(format!(" {}", i.kind)), }), preselect: Some(i.preselected), + sort_text: Some(i.sort_text), kind: Some(to_lsp_types_completion_item_kind(i.kind)), ..CompletionItem::default() }) From e3e54e45d54721b3ab997a01a049009786a784db Mon Sep 17 00:00:00 2001 From: Julian Date: Mon, 21 Apr 2025 17:07:57 +0200 Subject: [PATCH 18/22] ? --- crates/pgt_completions/src/sanitization.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/pgt_completions/src/sanitization.rs b/crates/pgt_completions/src/sanitization.rs index f2f912bc..921c6e1f 100644 --- a/crates/pgt_completions/src/sanitization.rs +++ b/crates/pgt_completions/src/sanitization.rs @@ -219,7 +219,7 @@ mod tests { .set_language(tree_sitter_sql::language()) .expect("Error loading sql language"); - let mut tree = parser.parse(input.to_string(), None).unwrap(); + let mut tree = parser.parse(input, None).unwrap(); // select | from users; <-- just right, one space after select token, one space before from assert!(cursor_inbetween_nodes(&mut tree, TextSize::new(7))); @@ -243,7 +243,7 @@ mod tests { .set_language(tree_sitter_sql::language()) .expect("Error loading sql language"); - let mut tree = parser.parse(input.to_string(), None).unwrap(); + let mut tree = parser.parse(input, None).unwrap(); // select * from| <-- still on previous token assert!(!cursor_prepared_to_write_token_after_last_node( @@ -295,7 +295,7 @@ mod tests { .set_language(tree_sitter_sql::language()) .expect("Error loading sql language"); - let mut tree = parser.parse(input.to_string(), None).unwrap(); + let mut tree = parser.parse(input, None).unwrap(); // select * from ;| <-- it's after the statement assert!(!cursor_before_semicolon(&mut tree, TextSize::new(19))); From ac122a592fd21c622d414aed5cfb02ea2f143551 Mon Sep 17 00:00:00 2001 From: Julian Date: Mon, 21 Apr 2025 17:12:05 +0200 Subject: [PATCH 19/22] test belongs here --- .../pgt_completions/src/providers/columns.rs | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/crates/pgt_completions/src/providers/columns.rs b/crates/pgt_completions/src/providers/columns.rs index b792ba2c..d1c3e110 100644 --- a/crates/pgt_completions/src/providers/columns.rs +++ b/crates/pgt_completions/src/providers/columns.rs @@ -224,4 +224,69 @@ mod tests { "`email` not present in first four completion items." ); } + + #[tokio::test] + async fn prefers_columns_of_mentioned_tables() { + let setup = r#" + create schema private; + + create table private.users ( + id1 serial primary key, + name1 text, + address1 text, + email1 text + ); + + create table public.users ( + id2 serial primary key, + name2 text, + address2 text, + email2 text + ); + "#; + + { + let test_case = TestCase { + message: "", + query: format!(r#"select {} from users"#, CURSOR_POS), + label: "suggests from table", + description: "", + }; + + let (tree, cache) = get_test_deps(setup, test_case.get_input_query()).await; + let params = get_test_params(&tree, &cache, test_case.get_input_query()); + let results = complete(params); + + assert_eq!( + results + .into_iter() + .take(4) + .map(|item| item.label) + .collect::>(), + vec!["address2", "email2", "id2", "name2"] + ); + } + + { + let test_case = TestCase { + message: "", + query: format!(r#"select {} from private.users"#, CURSOR_POS), + label: "suggests from table", + description: "", + }; + + let (tree, cache) = get_test_deps(setup, test_case.get_input_query()).await; + let params = get_test_params(&tree, &cache, test_case.get_input_query()); + let results = complete(params); + + assert_eq!( + results + .into_iter() + .take(4) + .map(|item| item.label) + .collect::>(), + vec!["address1", "email1", "id1", "name1"] + ); + } + } } From f9f2502c14b02d1861640b9650f8e69dd35fd408 Mon Sep 17 00:00:00 2001 From: Julian Date: Mon, 21 Apr 2025 17:12:25 +0200 Subject: [PATCH 20/22] test goes in other PR --- .../pgt_completions/src/providers/columns.rs | 65 ------------------- 1 file changed, 65 deletions(-) diff --git a/crates/pgt_completions/src/providers/columns.rs b/crates/pgt_completions/src/providers/columns.rs index 6f8aa447..b2b00d23 100644 --- a/crates/pgt_completions/src/providers/columns.rs +++ b/crates/pgt_completions/src/providers/columns.rs @@ -262,69 +262,4 @@ mod tests { .any(|item| item.kind == CompletionItemKind::Column) ); } - - #[tokio::test] - async fn prefers_columns_of_mentioned_tables() { - let setup = r#" - create schema private; - - create table private.users ( - id1 serial primary key, - name1 text, - address1 text, - email1 text - ); - - create table public.users ( - id2 serial primary key, - name2 text, - address2 text, - email2 text - ); - "#; - - { - let test_case = TestCase { - message: "", - query: format!(r#"select {} from users"#, CURSOR_POS), - label: "suggests from table", - description: "", - }; - - let (tree, cache) = get_test_deps(setup, test_case.get_input_query()).await; - let params = get_test_params(&tree, &cache, test_case.get_input_query()); - let results = complete(params); - - assert_eq!( - results - .into_iter() - .take(4) - .map(|item| item.label) - .collect::>(), - vec!["address2", "email2", "id2", "name2"] - ); - } - - { - let test_case = TestCase { - message: "", - query: format!(r#"select {} from private.users"#, CURSOR_POS), - label: "suggests from table", - description: "", - }; - - let (tree, cache) = get_test_deps(setup, test_case.get_input_query()).await; - let params = get_test_params(&tree, &cache, test_case.get_input_query()); - let results = complete(params); - - assert_eq!( - results - .into_iter() - .take(4) - .map(|item| item.label) - .collect::>(), - vec!["address1", "email1", "id1", "name1"] - ); - } - } } From 46ad86a70ec9680aaeaf7f02f97f779a7f81a3fe Mon Sep 17 00:00:00 2001 From: Julian Date: Mon, 21 Apr 2025 17:19:53 +0200 Subject: [PATCH 21/22] ok --- crates/pgt_completions/src/providers/columns.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/pgt_completions/src/providers/columns.rs b/crates/pgt_completions/src/providers/columns.rs index b2b00d23..32d37075 100644 --- a/crates/pgt_completions/src/providers/columns.rs +++ b/crates/pgt_completions/src/providers/columns.rs @@ -27,9 +27,7 @@ pub fn complete_columns<'a>(ctx: &CompletionContext<'a>, builder: &mut Completio mod tests { use crate::{ CompletionItem, CompletionItemKind, complete, - test_helper::{ - CURSOR_POS, InputQuery, get_test_deps, get_test_params, test_against_connection_string, - }, + test_helper::{CURSOR_POS, InputQuery, get_test_deps, get_test_params}, }; struct TestCase { From cc515246da160f4c42cb2208d132c4aa4a8a4537 Mon Sep 17 00:00:00 2001 From: Julian Date: Mon, 21 Apr 2025 17:37:58 +0200 Subject: [PATCH 22/22] fixes --- crates/pgt_completions/src/providers/columns.rs | 3 +-- crates/pgt_completions/src/sanitization.rs | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/pgt_completions/src/providers/columns.rs b/crates/pgt_completions/src/providers/columns.rs index 662ae4c8..e8a51e48 100644 --- a/crates/pgt_completions/src/providers/columns.rs +++ b/crates/pgt_completions/src/providers/columns.rs @@ -208,8 +208,7 @@ mod tests { let has_column_in_first_four = |col: &'static str| { first_four .iter() - .find(|compl_item| compl_item.label.as_str() == col) - .is_some() + .any(|compl_item| compl_item.label.as_str() == col) }; assert!( diff --git a/crates/pgt_completions/src/sanitization.rs b/crates/pgt_completions/src/sanitization.rs index 921c6e1f..59eb609f 100644 --- a/crates/pgt_completions/src/sanitization.rs +++ b/crates/pgt_completions/src/sanitization.rs @@ -275,13 +275,13 @@ mod tests { let input = "select * from private."; // select * from private.| <-- on a dot - assert!(cursor_on_a_dot(&input, TextSize::new(22))); + assert!(cursor_on_a_dot(input, TextSize::new(22))); // select * from private|. <-- before the dot - assert!(!cursor_on_a_dot(&input, TextSize::new(21))); + assert!(!cursor_on_a_dot(input, TextSize::new(21))); // select * from private. | <-- too far off the dot - assert!(!cursor_on_a_dot(&input, TextSize::new(23))); + assert!(!cursor_on_a_dot(input, TextSize::new(23))); } #[test]