diff --git a/crates/squawk_ide/src/code_actions/add_explicit_alias.rs b/crates/squawk_ide/src/code_actions/add_explicit_alias.rs index b32cb26d..c06cacdc 100644 --- a/crates/squawk_ide/src/code_actions/add_explicit_alias.rs +++ b/crates/squawk_ide/src/code_actions/add_explicit_alias.rs @@ -4,7 +4,8 @@ use squawk_linter::Edit; use squawk_syntax::ast::{self, AstNode}; use squawk_syntax::quote::quote_column_alias; -use crate::{column_name::ColumnName, file::InFile, offsets::token_from_offset}; +use crate::{file::InFile, offsets::token_from_offset}; +use squawk_syntax::column_name::ColumnName; use super::{ActionKind, CodeAction}; diff --git a/crates/squawk_ide/src/code_actions/remove_redundant_alias.rs b/crates/squawk_ide/src/code_actions/remove_redundant_alias.rs index 48810ffa..bc848572 100644 --- a/crates/squawk_ide/src/code_actions/remove_redundant_alias.rs +++ b/crates/squawk_ide/src/code_actions/remove_redundant_alias.rs @@ -3,7 +3,8 @@ use salsa::Database as Db; use squawk_linter::Edit; use squawk_syntax::ast::{self, AstNode}; -use crate::{column_name::ColumnName, file::InFile, offsets::token_from_offset, symbols::Name}; +use crate::{file::InFile, offsets::token_from_offset, symbols::Name}; +use squawk_syntax::column_name::ColumnName; use super::{ActionKind, CodeAction}; diff --git a/crates/squawk_ide/src/collect.rs b/crates/squawk_ide/src/collect.rs index 7aadc80a..76b531b6 100644 --- a/crates/squawk_ide/src/collect.rs +++ b/crates/squawk_ide/src/collect.rs @@ -1,5 +1,4 @@ use crate::ast_nav; -use crate::column_name::ColumnName; use crate::db::{File, bind, list_files, parse}; use crate::file::InFile; use crate::goto_definition::goto_definition; @@ -11,6 +10,7 @@ use crate::resolve::{ resolve_table_like, resolve_table_name, table_ptr_from_from_item, }; use salsa::Database as Db; +use squawk_syntax::column_name::ColumnName; use squawk_syntax::{ SyntaxNode, SyntaxNodePtr, ast::{self, AstNode}, diff --git a/crates/squawk_ide/src/hover.rs b/crates/squawk_ide/src/hover.rs index 1b2bfb10..fd06c543 100644 --- a/crates/squawk_ide/src/hover.rs +++ b/crates/squawk_ide/src/hover.rs @@ -1,6 +1,5 @@ use crate::ast_nav; use crate::collect; -use crate::column_name::ColumnName; use crate::comments::preceding_comment; use crate::db::{File, bind, list_files, parse}; use crate::file::InFile; @@ -19,6 +18,7 @@ use squawk_line_index::find_newline; use squawk_syntax::SyntaxNode; use squawk_syntax::SyntaxNodePtr; use squawk_syntax::ast::LitKind; +use squawk_syntax::column_name::ColumnName; use squawk_syntax::{ SyntaxKind, ast::{self, AstNode}, diff --git a/crates/squawk_ide/src/lib.rs b/crates/squawk_ide/src/lib.rs index 94e459b8..eb01b571 100644 --- a/crates/squawk_ide/src/lib.rs +++ b/crates/squawk_ide/src/lib.rs @@ -4,7 +4,6 @@ pub mod builtins; mod classify; pub mod code_actions; mod collect; -pub mod column_name; mod comments; pub mod completion; pub mod db; diff --git a/crates/squawk_ide/src/resolve.rs b/crates/squawk_ide/src/resolve.rs index 253efa98..6583c791 100644 --- a/crates/squawk_ide/src/resolve.rs +++ b/crates/squawk_ide/src/resolve.rs @@ -7,7 +7,6 @@ use squawk_syntax::{ }; use crate::binder::ResolvedSchemas; -use crate::column_name::ColumnName; use crate::db::File; use crate::file::InFile; use crate::location::{Location, LocationKind}; @@ -19,6 +18,7 @@ use crate::{ db::{bind, parse}, }; use salsa::Database as Db; +use squawk_syntax::column_name::ColumnName; fn resolve_named_arg_parameter( db: &dyn Db, diff --git a/crates/squawk_linter/src/rules/ban_duplicate_column_assignments.rs b/crates/squawk_linter/src/rules/ban_duplicate_column_assignments.rs index c8814d1b..98158a4f 100644 --- a/crates/squawk_linter/src/rules/ban_duplicate_column_assignments.rs +++ b/crates/squawk_linter/src/rules/ban_duplicate_column_assignments.rs @@ -1,7 +1,8 @@ use rustc_hash::FxHashMap; use squawk_syntax::{ - Parse, SourceFile, SyntaxKind, + Parse, SourceFile, SyntaxKind, SyntaxNode, ast::{self, AstNode, NameLike}, + column_name::ColumnName, }; use rowan::TextRange; @@ -15,7 +16,10 @@ struct Name(String); impl Name { fn from_node(node: &impl NameLike) -> Self { - let mut text = node.text(); + Self::from_string(node.text()) + } + + fn from_string(mut text: String) -> Self { text.truncate(text.floor_char_boundary(MAX_IDENT_BYTES)); Self(text) } @@ -46,10 +50,81 @@ pub(crate) fn ban_duplicate_column_assignments(ctx: &mut Linter, parse: &Parse column.name(), + _ => None, + }), + ); +} + +fn check_target_aliases(ctx: &mut Linter, target_list: &ast::TargetList) { + check_defined_nodes( + ctx, + target_list.targets().filter_map(|target| { + let (name, node) = ColumnName::from_target(target)?; + Some((Name::from_string(name.to_string()?), node)) + }), + ); +} + +fn check_defined_columns(ctx: &mut Linter, columns: impl Iterator) { + check_defined_nodes( + ctx, + columns.map(|column| (Name::from_node(&column), column.syntax().clone())), + ); +} + +fn check_defined_nodes(ctx: &mut Linter, columns: impl Iterator) { + let mut defined_columns: FxHashMap> = FxHashMap::default(); + + for (name, column) in columns { + defined_columns.entry(name).or_default().push(column); + } + + for (name, columns) in defined_columns { + if columns.len() < 2 { + continue; + } + + for column in columns { + ctx.report(Violation::for_node( + Rule::BanDuplicateColumnAssignments, + format!("Column `{}` is specified more than once.", name.as_str()), + &column, + )); } } } @@ -430,6 +505,115 @@ when not matched then "); } + #[test] + fn create_table_err() { + let sql = r#" +create table t ( + a int, + a text +); +"#; + assert_snapshot!(lint(sql), @" + warning[ban-duplicate-column-assignments]: Column `a` is specified more than once. + ╭▸ + 3 │ a int, + ╰╴ ━ + warning[ban-duplicate-column-assignments]: Column `a` is specified more than once. + ╭▸ + 4 │ a text + ╰╴ ━ + "); + } + + #[test] + fn create_foreign_table_err() { + let sql = r#" +create foreign table t ( + a int, + a text +) server s; +"#; + assert_snapshot!(lint(sql), @" + warning[ban-duplicate-column-assignments]: Column `a` is specified more than once. + ╭▸ + 3 │ a int, + ╰╴ ━ + warning[ban-duplicate-column-assignments]: Column `a` is specified more than once. + ╭▸ + 4 │ a text + ╰╴ ━ + "); + } + + #[test] + fn create_view_err() { + let sql = r#" +create view v (a, a) as +select 1, 2; +"#; + assert_snapshot!(lint(sql), @" + warning[ban-duplicate-column-assignments]: Column `a` is specified more than once. + ╭▸ + 2 │ create view v (a, a) as + ╰╴ ━ + warning[ban-duplicate-column-assignments]: Column `a` is specified more than once. + ╭▸ + 2 │ create view v (a, a) as + ╰╴ ━ + "); + } + + #[test] + fn create_view_with_duplicate_target_aliases_err() { + let sql = r#" +create view v as +select 1 a, 2 a; +"#; + assert_snapshot!(lint(sql), @" + warning[ban-duplicate-column-assignments]: Column `a` is specified more than once. + ╭▸ + 3 │ select 1 a, 2 a; + ╰╴ ━ + warning[ban-duplicate-column-assignments]: Column `a` is specified more than once. + ╭▸ + 3 │ select 1 a, 2 a; + ╰╴ ━ + "); + } + + #[test] + fn create_view_with_duplicate_inferred_names_err() { + let sql = r#" +create view v as +select 1, 2; +"#; + assert_snapshot!(lint(sql), @" + warning[ban-duplicate-column-assignments]: Column `?column?` is specified more than once. + ╭▸ + 3 │ select 1, 2; + ╰╴ ━ + warning[ban-duplicate-column-assignments]: Column `?column?` is specified more than once. + ╭▸ + 3 │ select 1, 2; + ╰╴ ━ + "); + } + + #[test] + fn select_into_err() { + let sql = "select 1 a, 2 a into z;"; + assert_snapshot!(lint(sql), @" + warning[ban-duplicate-column-assignments]: Column `a` is specified more than once. + ╭▸ + 1 │ select 1 a, 2 a into z; + ╰╴ ━ + warning[ban-duplicate-column-assignments]: Column `a` is specified more than once. + ╭▸ + 1 │ select 1 a, 2 a into z; + ╰╴ ━ + "); + } + #[test] fn conflict_update_err() { let sql = r#" @@ -503,6 +687,19 @@ set A = 2; insert into k (a, b) values (1, 2); +create table t ( + a int, + b int +); +create foreign table ft ( + a int, + b int +) server s; +create view v (a, b) as +select 1 a, 2 a; +create view inferred_v as +select 1 a, 2 b; +select 1 a, 2 b into z; insert into k (a.x, a.y) values (1, 2); merge into k diff --git a/crates/squawk_ide/src/column_name.rs b/crates/squawk_syntax/src/column_name.rs similarity index 98% rename from crates/squawk_ide/src/column_name.rs rename to crates/squawk_syntax/src/column_name.rs index eb17bf12..b6a3bc9d 100644 --- a/crates/squawk_ide/src/column_name.rs +++ b/crates/squawk_syntax/src/column_name.rs @@ -1,10 +1,10 @@ -use squawk_syntax::{ +use crate::{ SyntaxKind, SyntaxNode, ast::{self, AstNode}, }; #[derive(Clone, Debug, PartialEq)] -pub(crate) enum ColumnName { +pub enum ColumnName { Column(String), /// There's a fallback mechanism that we need to propagate through the /// expressions/types. @@ -23,7 +23,7 @@ pub(crate) enum ColumnName { impl ColumnName { // Get the alias, otherwise infer the column name. - pub(crate) fn from_target(target: ast::Target) -> Option<(ColumnName, SyntaxNode)> { + pub fn from_target(target: ast::Target) -> Option<(ColumnName, SyntaxNode)> { if let Some(as_name) = target.as_name() && let Some(name_node) = as_name.name() { @@ -36,7 +36,7 @@ impl ColumnName { } // Ignore any aliases, just infer the what the column name. - pub(crate) fn inferred_from_target(target: ast::Target) -> Option<(ColumnName, SyntaxNode)> { + pub fn inferred_from_target(target: ast::Target) -> Option<(ColumnName, SyntaxNode)> { if let Some(expr) = target.expr() && let Some(name) = name_from_expr(expr, false) { @@ -55,7 +55,7 @@ impl ColumnName { } } - pub(crate) fn to_string(&self) -> Option { + pub fn to_string(&self) -> Option { match self { ColumnName::Column(string) => Some(string.to_string()), ColumnName::Star => None, @@ -695,7 +695,7 @@ fn examples() { #[track_caller] fn name(sql: &str) -> String { let sql = "select ".to_string() + sql; - let parse = squawk_syntax::SourceFile::parse(&sql); + let parse = crate::SourceFile::parse(&sql); assert_eq!(parse.errors(), vec![]); let file = parse.tree(); diff --git a/crates/squawk_syntax/src/lib.rs b/crates/squawk_syntax/src/lib.rs index 1b40cc43..cc94590f 100644 --- a/crates/squawk_syntax/src/lib.rs +++ b/crates/squawk_syntax/src/lib.rs @@ -25,6 +25,7 @@ // DEALINGS IN THE SOFTWARE. pub mod ast; +pub mod column_name; mod generated; mod parsing; mod ptr; diff --git a/docs/docs/ban-duplicate-column-assignments.md b/docs/docs/ban-duplicate-column-assignments.md index b18c33be..bb393ce5 100644 --- a/docs/docs/ban-duplicate-column-assignments.md +++ b/docs/docs/ban-duplicate-column-assignments.md @@ -5,23 +5,21 @@ title: ban-duplicate-column-assignments ## problem -Assigning to a column more than once in Postgres results in a runtime error. +Assigning/declaring a column more than once results in a runtime error in +Postgres. ```sql -create table t(a int); +create table t(a int, a text); +create view v (a, a) as select 1, 2; update t set a = 1, a = 2; ``` -gives: - -``` -Query 1 ERROR at Line 1: : ERROR: multiple assignments to same column "a" -``` - ## solution -Remove your dupe assignment: +Remove duplicate assignments/declarations: ```sql +create table t(a int); +create view v (a, b) as select 1, 2; update t set a = 2; ```