Skip to content

Commit 33b71c2

Browse files
sbdchdsbdchd
authored
ide: support references column constraints & select t.* from t; (#817)
1 parent 8e3a2c3 commit 33b71c2

20 files changed

+316
-142
lines changed

‎PLAN.md‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,26 @@ FROM (
590590
WHERE total_amount > 1000;
591591
```
592592

593+
### Rule: implicit column name in references
594+
595+
```sql
596+
create table u(
597+
id int primary key
598+
);
599+
create table t(
600+
u_id int references u
601+
-- ^quickfix
602+
);
603+
604+
-- becomes
605+
606+
create table t(
607+
u_id int references u(id)
608+
);
609+
```
610+
611+
and vice versa
612+
593613
### Rule: aggregate free `having` condition
594614

595615
```sql

‎crates/squawk_ide/src/classify.rs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ pub(crate) fn classify_name_ref(name_ref: &ast::NameRef) -> Option<NameRefClass>
6666
let mut in_constraint_include_clause = false;
6767
let mut in_constraint_where_clause = false;
6868
let mut in_partition_item = false;
69+
let mut in_set_null_columns = false;
6970

7071
// TODO: can we combine this if and the one that follows?
7172
if let Some(parent) = name_ref.syntax().parent()
@@ -115,6 +116,7 @@ pub(crate) fn classify_name_ref(name_ref: &ast::NameRef) -> Option<NameRefClass>
115116
// ^^^
116117
.is_some_and(|field_name_ref| field_name_ref.syntax() == name_ref.syntax())
117118
// we're not inside a call expr
119+
&& field_expr.star_token().is_none()
118120
&& field_expr
119121
.syntax()
120122
.parent()
@@ -220,6 +222,9 @@ pub(crate) fn classify_name_ref(name_ref: &ast::NameRef) -> Option<NameRefClass>
220222
{
221223
return Some(NameRefClass::Tablespace);
222224
}
225+
if ast::SetNullColumns::can_cast(ancestor.kind()) {
226+
in_set_null_columns = true;
227+
}
223228
if let Some(foreign_key) = ast::ForeignKeyConstraint::cast(ancestor.clone()) {
224229
if in_column_list {
225230
// TODO: ast is too "flat" here, we need a unique node for to
@@ -241,10 +246,31 @@ pub(crate) fn classify_name_ref(name_ref: &ast::NameRef) -> Option<NameRefClass>
241246
{
242247
return Some(NameRefClass::ForeignKeyLocalColumn);
243248
}
249+
if in_set_null_columns {
250+
return Some(NameRefClass::ForeignKeyLocalColumn);
251+
}
244252
} else {
245253
return Some(NameRefClass::ForeignKeyTable);
246254
}
247255
}
256+
if let Some(references_constraint) = ast::ReferencesConstraint::cast(ancestor.clone()) {
257+
if let Some(column_ref) = references_constraint.column()
258+
&& column_ref
259+
.syntax()
260+
.text_range()
261+
.contains_range(name_ref.syntax().text_range())
262+
{
263+
return Some(NameRefClass::ForeignKeyColumn);
264+
}
265+
if let Some(path) = references_constraint.table()
266+
&& path
267+
.syntax()
268+
.text_range()
269+
.contains_range(name_ref.syntax().text_range())
270+
{
271+
return Some(NameRefClass::ForeignKeyTable);
272+
}
273+
}
248274
if ast::CheckConstraint::can_cast(ancestor.kind()) {
249275
return Some(NameRefClass::CheckConstraintColumn);
250276
}

‎crates/squawk_ide/src/goto_definition.rs‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,72 @@ create table bar(
465465
");
466466
}
467467

468+
#[test]
469+
fn goto_foreign_key_on_delete_set_null_column() {
470+
assert_snapshot!(goto("
471+
create table users (
472+
user_id integer not null,
473+
primary key (user_id)
474+
);
475+
476+
create table posts (
477+
post_id integer not null,
478+
author_id integer,
479+
primary key (post_id),
480+
foreign key (author_id) references users on delete set null (author_id$0)
481+
);
482+
"), @r"
483+
╭▸
484+
9 │ author_id integer,
485+
│ ───────── 2. destination
486+
10 │ primary key (post_id),
487+
11 │ foreign key (author_id) references users on delete set null (author_id)
488+
╰╴ ─ 1. source
489+
");
490+
}
491+
492+
#[test]
493+
fn goto_references_constraint_table() {
494+
assert_snapshot!(goto("
495+
create table t (
496+
id serial primary key
497+
);
498+
499+
create table u (
500+
id serial primary key,
501+
t_id int references t$0
502+
);
503+
"), @r"
504+
╭▸
505+
2 │ create table t (
506+
│ ─ 2. destination
507+
‡
508+
8 │ t_id int references t
509+
╰╴ ─ 1. source
510+
");
511+
}
512+
513+
#[test]
514+
fn goto_references_constraint_column() {
515+
assert_snapshot!(goto("
516+
create table t (
517+
id serial primary key
518+
);
519+
520+
create table u (
521+
id serial primary key,
522+
t_id int references t(id$0)
523+
);
524+
"), @r"
525+
╭▸
526+
3 │ id serial primary key
527+
│ ── 2. destination
528+
‡
529+
8 │ t_id int references t(id)
530+
╰╴ ─ 1. source
531+
");
532+
}
533+
468534
#[test]
469535
fn goto_foreign_key_references_column() {
470536
assert_snapshot!(goto("
@@ -2918,6 +2984,20 @@ select t$0 from t;
29182984
");
29192985
}
29202986

2987+
#[test]
2988+
fn goto_select_table_star_expansion() {
2989+
assert_snapshot!(goto("
2990+
create table t(id int, a int);
2991+
select t$0.* from t;
2992+
"), @r"
2993+
╭▸
2994+
2 │ create table t(id int, a int);
2995+
│ ─ 2. destination
2996+
3 │ select t.* from t;
2997+
╰╴ ─ 1. source
2998+
");
2999+
}
3000+
29213001
#[test]
29223002
fn goto_select_table_as_column_with_schema() {
29233003
assert_snapshot!(goto("

‎crates/squawk_ide/src/resolve.rs‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -121,22 +121,29 @@ pub(crate) fn resolve_name_ref(
121121
resolve_tablespace(binder, &tablespace_name).map(|ptr| smallvec![ptr])
122122
}
123123
NameRefClass::ForeignKeyTable => {
124-
let foreign_key = name_ref
125-
.syntax()
126-
.ancestors()
127-
.find_map(ast::ForeignKeyConstraint::cast)?;
128-
let path = foreign_key.path()?;
124+
let path = name_ref.syntax().ancestors().find_map(ast::Path::cast)?;
129125
let table_name = extract_table_name(&path)?;
130126
let schema = extract_schema_name(&path);
131127
let position = name_ref.syntax().text_range().start();
132128
resolve_table(binder, &table_name, &schema, position).map(|ptr| smallvec![ptr])
133129
}
134130
NameRefClass::ForeignKeyColumn => {
135-
let foreign_key = name_ref
131+
// TODO: the ast is too flat here
132+
let path = if let Some(foreign_key) = name_ref
133+
.syntax()
134+
.ancestors()
135+
.find_map(ast::ForeignKeyConstraint::cast)
136+
{
137+
foreign_key.path()?
138+
} else if let Some(references_constraint) = name_ref
136139
.syntax()
137140
.ancestors()
138-
.find_map(ast::ForeignKeyConstraint::cast)?;
139-
let path = foreign_key.path()?;
141+
.find_map(ast::ReferencesConstraint::cast)
142+
{
143+
references_constraint.table()?
144+
} else {
145+
return None;
146+
};
140147
let column_name = Name::from_node(name_ref);
141148
resolve_column_for_path(binder, &path, column_name).map(|ptr| smallvec![ptr])
142149
}
@@ -678,6 +685,7 @@ fn resolve_select_qualified_column_table(
678685
let explicit_schema = if field_expr
679686
.field()
680687
.is_some_and(|f| f.syntax() == name_ref.syntax())
688+
&& field_expr.star_token().is_none()
681689
{
682690
// if we're at the field `bar` in `foo.bar`
683691
if let ast::Expr::NameRef(schema_name_ref) = field_expr.base()? {

‎crates/squawk_linter/src/rules/constraint_missing_not_valid.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ fn not_valid_validate_in_transaction(
6969
ast::AlterTableAction::AddConstraint(add_constraint) => {
7070
if add_constraint.not_valid().is_some()
7171
&& let Some(constraint) = add_constraint.constraint()
72-
&& let Some(constraint_name) = constraint.name()
72+
&& let Some(constraint_name) =
73+
constraint.constraint_name().and_then(|c| c.name())
7374
{
7475
not_valid_names.insert(Identifier::new(&constraint_name.text()));
7576
}

‎crates/squawk_linter/src/rules/prefer_robust_stmts.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,9 @@ pub(crate) fn prefer_robust_stmts(ctx: &mut Linter, parse: &Parse<SourceFile>) {
9191
}
9292
ast::AlterTableAction::AddConstraint(add_constraint) => {
9393
let constraint = add_constraint.constraint();
94-
if let Some(constraint_name) = constraint.and_then(|x| x.name())
94+
if let Some(constraint_name) = constraint
95+
.and_then(|x| x.constraint_name())
96+
.and_then(|x| x.name())
9597
&& let Some(constraint) = constraint_names
9698
.get_mut(&Identifier::new(constraint_name.text().as_str()))
9799
&& *constraint == Constraint::Dropped

‎crates/squawk_parser/src/generated/syntax_kind.rs‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/squawk_parser/src/grammar.rs‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3760,9 +3760,7 @@ fn opt_column_constraint(p: &mut Parser<'_>) -> Option<CompletedMarker> {
37603760
return None;
37613761
}
37623762
let m = p.start();
3763-
if p.eat(CONSTRAINT_KW) {
3764-
name(p);
3765-
}
3763+
opt_constraint_name(p);
37663764
match opt_constraint_inner(p) {
37673765
Some(kind) => {
37683766
opt_constraint_option_list(p);
@@ -4087,9 +4085,7 @@ fn table_constraint(p: &mut Parser<'_>) -> CompletedMarker {
40874085
assert!(p.at_ts(TABLE_CONSTRAINT_FIRST));
40884086
let m = p.start();
40894087
// [ CONSTRAINT constraint_name ]
4090-
if p.eat(CONSTRAINT_KW) {
4091-
name(p);
4092-
}
4088+
opt_constraint_name(p);
40934089
// CHECK ( expression ) [ NO INHERIT ]
40944090
let kind = match p.current() {
40954091
CHECK_KW => {
@@ -7167,9 +7163,7 @@ fn alter_domain_action(p: &mut Parser<'_>) -> Option<CompletedMarker> {
71677163
// { NOT NULL | CHECK (expression) }
71687164
fn domain_constraint(p: &mut Parser<'_>) {
71697165
let m = p.start();
7170-
if p.eat(CONSTRAINT_KW) {
7171-
name(p);
7172-
}
7166+
opt_constraint_name(p);
71737167
if p.eat(NOT_KW) {
71747168
p.expect(NULL_KW);
71757169
m.complete(p, NOT_NULL_CONSTRAINT);
@@ -7186,6 +7180,16 @@ fn domain_constraint(p: &mut Parser<'_>) {
71867180
}
71877181
}
71887182

7183+
fn opt_constraint_name(p: &mut Parser<'_>) {
7184+
let m = p.start();
7185+
if p.eat(CONSTRAINT_KW) {
7186+
name(p);
7187+
m.complete(p, CONSTRAINT_NAME);
7188+
} else {
7189+
m.abandon(p);
7190+
}
7191+
}
7192+
71897193
// ALTER DEFAULT PRIVILEGES
71907194
// [ FOR { ROLE | USER } target_role [, ...] ]
71917195
// [ IN SCHEMA schema_name [, ...] ]

‎crates/squawk_parser/tests/snapshots/tests__alter_domain_ok.snap‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -103,10 +103,11 @@ SOURCE_FILE
103103
ADD_KW "add"
104104
WHITESPACE " "
105105
CHECK_CONSTRAINT
106-
CONSTRAINT_KW "constraint"
107-
WHITESPACE " "
108-
NAME
109-
IDENT "c"
106+
CONSTRAINT_NAME
107+
CONSTRAINT_KW "constraint"
108+
WHITESPACE " "
109+
NAME
110+
IDENT "c"
110111
WHITESPACE " "
111112
CHECK_KW "check"
112113
WHITESPACE " "
@@ -169,10 +170,11 @@ SOURCE_FILE
169170
ADD_KW "add"
170171
WHITESPACE " "
171172
CHECK_CONSTRAINT
172-
CONSTRAINT_KW "constraint"
173-
WHITESPACE " "
174-
NAME
175-
IDENT "a"
173+
CONSTRAINT_NAME
174+
CONSTRAINT_KW "constraint"
175+
WHITESPACE " "
176+
NAME
177+
IDENT "a"
176178
WHITESPACE " "
177179
CHECK_KW "check"
178180
WHITESPACE " "

‎crates/squawk_parser/tests/snapshots/tests__alter_foreign_table_ok.snap‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1055,10 +1055,11 @@ SOURCE_FILE
10551055
ADD_KW "add"
10561056
WHITESPACE " "
10571057
CHECK_CONSTRAINT
1058-
CONSTRAINT_KW "constraint"
1059-
WHITESPACE " "
1060-
NAME
1061-
IDENT "c"
1058+
CONSTRAINT_NAME
1059+
CONSTRAINT_KW "constraint"
1060+
WHITESPACE " "
1061+
NAME
1062+
IDENT "c"
10621063
WHITESPACE " "
10631064
CHECK_KW "check"
10641065
WHITESPACE " "
@@ -1096,10 +1097,11 @@ SOURCE_FILE
10961097
ADD_KW "add"
10971098
WHITESPACE " "
10981099
CHECK_CONSTRAINT
1099-
CONSTRAINT_KW "constraint"
1100-
WHITESPACE " "
1101-
NAME
1102-
IDENT "c"
1100+
CONSTRAINT_NAME
1101+
CONSTRAINT_KW "constraint"
1102+
WHITESPACE " "
1103+
NAME
1104+
IDENT "c"
11031105
WHITESPACE " "
11041106
CHECK_KW "check"
11051107
WHITESPACE " "

0 commit comments

Comments
 (0)