Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions crates/squawk_ide/src/code_actions/add_explicit_alias.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
use rowan::TextSize;
use salsa::Database as Db;
use squawk_syntax::ast::{self, AstNode};
use squawk_syntax::quote::quote_column_alias;

use crate::{
column_name::ColumnName, db::File, offsets::token_from_offset, quote::quote_column_alias,
};
use crate::{column_name::ColumnName, db::File, offsets::token_from_offset};

use super::{ActionKind, CodeAction};

Expand Down
4 changes: 3 additions & 1 deletion crates/squawk_ide/src/code_actions/unquote_identifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ use rowan::TextSize;
use salsa::Database as Db;
use squawk_syntax::ast::{self, AstNode};

use crate::{db::File, offsets::token_from_offset, quote::unquote_ident};
use squawk_syntax::quote::unquote_ident;

use crate::{db::File, offsets::token_from_offset};

use super::{ActionKind, CodeAction};

Expand Down
2 changes: 1 addition & 1 deletion crates/squawk_ide/src/column_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use squawk_syntax::{
ast::{self, AstNode},
};

use crate::quote::normalize_identifier;
use squawk_syntax::quote::normalize_identifier;

#[derive(Clone, Debug, PartialEq)]
pub(crate) enum ColumnName {
Expand Down
1 change: 0 additions & 1 deletion crates/squawk_ide/src/generated/mod.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions crates/squawk_ide/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,13 @@ pub mod expand_selection;
mod file;
pub mod find_references;
pub mod folding_ranges;
mod generated;
pub mod goto_definition;
pub mod hover;
mod infer;
pub mod inlay_hints;
pub mod location;
mod name;
mod offsets;
mod quote;
mod resolve;
mod scope;
pub mod semantic_tokens;
Expand Down
2 changes: 1 addition & 1 deletion crates/squawk_ide/src/name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use smol_str::SmolStr;
use squawk_syntax::ast::{self, AstNode};
use std::fmt;

use crate::quote::normalize_identifier;
use squawk_syntax::quote::normalize_identifier;

#[derive(Clone, Debug, PartialEq, Eq, Hash)]
pub(crate) struct Name(pub(crate) SmolStr);
Expand Down
7 changes: 7 additions & 0 deletions crates/squawk_linter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ use rules::ban_uncommitted_transaction;
use rules::changing_column_type;
use rules::constraint_missing_not_valid;
use rules::disallow_unique_constraint;
use rules::identifier_too_long;
use rules::prefer_bigint_over_int;
use rules::prefer_bigint_over_smallint;
use rules::prefer_identity;
Expand Down Expand Up @@ -94,6 +95,7 @@ pub enum Rule {
BanUncommittedTransaction,
RequireEnumValueOrdering,
RequireTableSchema,
IdentifierTooLong,
// xtask:new-rule:error-name
}

Expand Down Expand Up @@ -146,6 +148,7 @@ impl TryFrom<&str> for Rule {
"ban-uncommitted-transaction" => Ok(Rule::BanUncommittedTransaction),
"require-enum-value-ordering" => Ok(Rule::RequireEnumValueOrdering),
"require-table-schema" => Ok(Rule::RequireTableSchema),
"identifier-too-long" => Ok(Rule::IdentifierTooLong),
// xtask:new-rule:str-name
_ => Err(format!("Unknown violation name: {s}")),
}
Expand Down Expand Up @@ -210,6 +213,7 @@ impl fmt::Display for Rule {
Rule::BanUncommittedTransaction => "ban-uncommitted-transaction",
Rule::RequireEnumValueOrdering => "require-enum-value-ordering",
Rule::RequireTableSchema => "require-table-schema",
Rule::IdentifierTooLong => "identifier-too-long",
// xtask:new-rule:variant-to-name
};
write!(f, "{val}")
Expand Down Expand Up @@ -443,6 +447,9 @@ impl Linter {
if self.rules.contains(&Rule::RequireTableSchema) {
require_table_schema(self, file);
}
if self.rules.contains(&Rule::IdentifierTooLong) {
identifier_too_long(self, file);
}
// xtask:new-rule:rule-call

// locate any ignores in the file
Expand Down
216 changes: 216 additions & 0 deletions crates/squawk_linter/src/rules/identifier_too_long.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
use squawk_syntax::{
Parse, SourceFile,
ast::{self, AstNode},
};

use squawk_syntax::quote::normalize_identifier;

use crate::{Edit, Fix, Linter, Rule, Violation};

// via: https://github.com/postgres/postgres/blob/228a1f9542792c6533ef74c2e7aefad0da1d9a7a/src/include/pg_config_manual.h#L39C6-L39C6
const NAMEDATALEN: usize = 64;
const MAX_IDENT_BYTES: usize = NAMEDATALEN - 1;

pub(crate) fn identifier_too_long(ctx: &mut Linter, parse: &Parse<SourceFile>) {
for node in parse.tree().syntax().descendants() {
if let Some(name) = ast::Name::cast(node.clone()) {
check_name(ctx, &name);
} else if let Some(name_ref) = ast::NameRef::cast(node) {
check_name(ctx, &name_ref);
}
}
}

fn check_name(ctx: &mut Linter, name_like: &impl ast::NameLike) {
let text = name_like.syntax().text().to_string();
let ident = normalize_identifier(&text);
if ident.len() <= MAX_IDENT_BYTES {
return;
}

let fix = truncate(&text).map(|truncated| {
Fix::new(
format!("Rename to `{truncated}`"),
vec![Edit::replace(name_like.syntax().text_range(), truncated)],
)
});

ctx.report(
Violation::for_node(
Rule::IdentifierTooLong,
format!("`{ident}` is too long and will be truncated to {MAX_IDENT_BYTES} bytes."),
name_like.syntax(),
)
.fix(fix),
);
}

fn truncate(text: &str) -> Option<String> {
if has_escaped_quotes(text) {
return None;
}

let unquoted = normalize_identifier(text);
let truncated = &unquoted[..unquoted.floor_char_boundary(MAX_IDENT_BYTES)];

Some(if text.starts_with('"') {
format!("\"{truncated}\"")
} else {
truncated.to_owned()
})
}

fn has_escaped_quotes(text: &str) -> bool {
text.strip_prefix('"')
.and_then(|t| t.strip_suffix('"'))
.is_some_and(|text| text.contains(r#""""#))
}

#[cfg(test)]
mod test {
use insta::assert_snapshot;

use crate::Rule;
use crate::test_utils::{lint_errors, lint_ok};

#[test]
fn create_table_long_name_err() {
let sql = r#"
create table table_very_long_very_long_very_long_very_long_very_long_very_long (column_very_long_very_long_very_long_very_long_very_long_very_long bigint);
"#;
assert_snapshot!(lint_errors(sql, Rule::IdentifierTooLong), @"
warning[identifier-too-long]: `table_very_long_very_long_very_long_very_long_very_long_very_long` is too long and will be truncated to 63 bytes.
╭▸
2 │ create table table_very_long_very_long_very_long_very_long_very_long_very_long (column_very_long_very_long_very_long_very_long_very_lon…
│ ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
╭╴
2 - create table table_very_long_very_long_very_long_very_long_very_long_very_long (column_very_long_very_long_very_long_very_long_very_long_very_long bigint);
2 + create table table_very_long_very_long_very_long_very_long_very_long_very_lo (column_very_long_very_long_very_long_very_long_very_long_very_long bigint);
╰╴
warning[identifier-too-long]: `column_very_long_very_long_very_long_very_long_very_long_very_long` is too long and will be truncated to 63 bytes.
╭▸
2 │ …ng_very_long_very_long_very_long (column_very_long_very_long_very_long_very_long_very_long_very_long bigint);
│ ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
╭╴
2 - create table table_very_long_very_long_very_long_very_long_very_long_very_long (column_very_long_very_long_very_long_very_long_very_long_very_long bigint);
2 + create table table_very_long_very_long_very_long_very_long_very_long_very_long (column_very_long_very_long_very_long_very_long_very_long_very_l bigint);
╰╴
");
}

#[test]
fn create_table_ok() {
let sql = r#"
create table short_name (col bigint);
"#;
lint_ok(sql, Rule::IdentifierTooLong);
}

#[test]
fn create_index_long_name_err() {
let sql = r#"
create index index_very_long_very_long_very_long_very_long_very_long_very_long on t (col);
"#;
assert_snapshot!(lint_errors(sql, Rule::IdentifierTooLong), @"
warning[identifier-too-long]: `index_very_long_very_long_very_long_very_long_very_long_very_long` is too long and will be truncated to 63 bytes.
╭▸
2 │ create index index_very_long_very_long_very_long_very_long_very_long_very_long on t (col);
│ ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
╭╴
2 - create index index_very_long_very_long_very_long_very_long_very_long_very_long on t (col);
2 + create index index_very_long_very_long_very_long_very_long_very_long_very_lo on t (col);
╰╴
");
}

#[test]
fn drop_table_long_name_err() {
let sql = r#"
drop table table_very_long_very_long_very_long_very_long_very_long_very_long;
"#;
assert_snapshot!(lint_errors(sql, Rule::IdentifierTooLong), @"
warning[identifier-too-long]: `table_very_long_very_long_very_long_very_long_very_long_very_long` is too long and will be truncated to 63 bytes.
╭▸
2 │ drop table table_very_long_very_long_very_long_very_long_very_long_very_long;
│ ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
╭╴
2 - drop table table_very_long_very_long_very_long_very_long_very_long_very_long;
2 + drop table table_very_long_very_long_very_long_very_long_very_long_very_lo;
╰╴
");
}

#[test]
fn alter_table_add_column_long_name_err() {
let sql = r#"
alter table t add column column_very_long_very_long_very_long_very_long_very_long_very_long bigint;
"#;
assert_snapshot!(lint_errors(sql, Rule::IdentifierTooLong), @"
warning[identifier-too-long]: `column_very_long_very_long_very_long_very_long_very_long_very_long` is too long and will be truncated to 63 bytes.
╭▸
2 │ alter table t add column column_very_long_very_long_very_long_very_long_very_long_very_long bigint;
│ ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
╭╴
2 - alter table t add column column_very_long_very_long_very_long_very_long_very_long_very_long bigint;
2 + alter table t add column column_very_long_very_long_very_long_very_long_very_long_very_l bigint;
╰╴
");
}

#[test]
fn quoted_identifier_long_err() {
let sql = r#"
create table "table_very_long_very_long_very_long_very_long_very_long_very_long" (id bigint);
"#;
assert_snapshot!(lint_errors(sql, Rule::IdentifierTooLong), @r#"
warning[identifier-too-long]: `table_very_long_very_long_very_long_very_long_very_long_very_long` is too long and will be truncated to 63 bytes.
╭▸
2 │ create table "table_very_long_very_long_very_long_very_long_very_long_very_long" (id bigint);
│ ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
╭╴
2 - create table "table_very_long_very_long_very_long_very_long_very_long_very_long" (id bigint);
2 + create table "table_very_long_very_long_very_long_very_long_very_long_very_lo" (id bigint);
╰╴
"#);
}

#[test]
fn multibyte_emoji_err() {
// postgres ends up slicing the emoji in two
let sql = r#"create table "👨‍👩‍👧‍👦👨‍👩‍👧‍👦👨‍👩‍👧‍👦" (id bigint);"#;
assert_snapshot!(lint_errors(sql, Rule::IdentifierTooLong), @r#"
warning[identifier-too-long]: `👨👩👧👦👨👩👧👦👨👩👧👦` is too long and will be truncated to 63 bytes.
╭▸
1 │ create table "👨👩👧👦👨👩👧👦👨👩👧👦" (id bigint);
│ ━━━━━━━━━━━━━━━━━━━━━━━━━━
╭╴
1 - create table "👨👩👧👦👨👩👧👦👨👩👧👦" (id bigint);
1 + create table "👨👩👧👦👨👩👧👦👨👩" (id bigint);
╰╴
"#);
}

#[test]
fn quoted_identifier_with_escaped_quotes_message_unescapes_quotes() {
let identifier = format!(r#""{}""b""#, "a".repeat(63));
// ^^ escaped quote
let sql = format!("create table {identifier} (id bigint);");
// quoting is complicated so we don't add a fix
assert_snapshot!(lint_errors(&sql, Rule::IdentifierTooLong), @r#"
warning[identifier-too-long]: `aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"b` is too long and will be truncated to 63 bytes.
╭▸
1 │ create table "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa""b" (id bigint);
╰╴ ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
"#);
}

#[test]
fn exactly_63_bytes_ok() {
// exactly 63 bytes -- should not trigger
let sql = r#"
create table table_very_long_very_long_very_long_very_long_very_long_very_lo (id bigint);
-- ^ exactly 63 bytes
"#;
lint_ok(sql, Rule::IdentifierTooLong);
}
}
2 changes: 2 additions & 0 deletions crates/squawk_linter/src/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ pub(crate) mod ban_uncommitted_transaction;
pub(crate) mod changing_column_type;
pub(crate) mod constraint_missing_not_valid;
pub(crate) mod disallow_unique_constraint;
pub(crate) mod identifier_too_long;
pub(crate) mod prefer_bigint_over_int;
pub(crate) mod prefer_bigint_over_smallint;
pub(crate) mod prefer_identity;
Expand Down Expand Up @@ -50,6 +51,7 @@ pub(crate) use ban_uncommitted_transaction::ban_uncommitted_transaction;
pub(crate) use changing_column_type::changing_column_type;
pub(crate) use constraint_missing_not_valid::constraint_missing_not_valid;
pub(crate) use disallow_unique_constraint::disallow_unique_constraint;
pub(crate) use identifier_too_long::identifier_too_long;
pub(crate) use prefer_bigint_over_int::prefer_bigint_over_int;
pub(crate) use prefer_bigint_over_smallint::prefer_bigint_over_smallint;
pub(crate) use prefer_identity::prefer_identity;
Expand Down
1 change: 1 addition & 0 deletions crates/squawk_syntax/src/generated/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub(crate) mod keywords;
2 changes: 2 additions & 0 deletions crates/squawk_syntax/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@
// DEALINGS IN THE SOFTWARE.

pub mod ast;
mod generated;
pub mod identifier;
mod parsing;
mod ptr;
pub mod quote;
pub mod syntax_error;
mod syntax_node;
mod token_text;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
use squawk_syntax::SyntaxNode;

use crate::SyntaxNode;
use crate::generated::keywords::RESERVED_KEYWORDS;

pub(crate) fn quote_column_alias(text: &str) -> String {
pub fn quote_column_alias(text: &str) -> String {
if needs_quoting(text) {
format!(r#""{}""#, text.replace('"', r#""""#))
} else {
text.to_string()
}
}

pub(crate) fn unquote_ident(node: &SyntaxNode) -> Option<String> {
pub fn unquote_ident(node: &SyntaxNode) -> Option<String> {
let text = node.text().to_string();

if !text.starts_with('"') || !text.ends_with('"') {
Expand Down Expand Up @@ -71,17 +70,17 @@ fn needs_quoting(text: &str) -> bool {
false
}

pub(crate) fn is_reserved_word(text: &str) -> bool {
pub fn is_reserved_word(text: &str) -> bool {
RESERVED_KEYWORDS
.binary_search(&text.to_lowercase().as_str())
.is_ok()
}

pub(crate) fn normalize_identifier(text: &str) -> String {
pub fn normalize_identifier(text: &str) -> String {
// TODO: Cow/SmolStr/Salsa Interned?
text.strip_prefix('"')
.and_then(|t| t.strip_suffix('"'))
.map(|x| x.to_string())
.map(|x| x.replace(r#""""#, "\""))
.unwrap_or_else(|| text.to_ascii_lowercase())
}

Expand Down
3 changes: 2 additions & 1 deletion crates/xtask/src/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ pub(crate) fn codegen() -> Result<()> {
project_root().join("crates/squawk_parser/src/generated/syntax_kind.rs");
std::fs::write(syntax_kinds_file, syntax_kinds).context("problem writing syntax kinds")?;

let ide_reserved_keywords = project_root().join("crates/squawk_ide/src/generated/keywords.rs");
let ide_reserved_keywords =
project_root().join("crates/squawk_syntax/src/generated/keywords.rs");
let reserved_keywords = generate_reserved_keywords_array(&keyword_kinds.reserved_keywords)?;
std::fs::write(ide_reserved_keywords, reserved_keywords)
.context("problem writing reserved keywords")?;
Expand Down
Loading
Loading