Skip to content

Commit c56b300

Browse files
lemonadernlemonadern
committed
wip
1 parent 4681c3f commit c56b300

File tree

3 files changed

+87
-97
lines changed

3 files changed

+87
-97
lines changed

‎crates/uroborosql-lint/src/linter.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use crate::{
33
diagnostic::{Diagnostic, Severity},
44
rule::Rule,
55
rules::{
6-
MissingTwoWaySample, NoDistinct, NoFunctionInJoinOrWhere, NoNotIn, NoUnionDistinct,
6+
MissingTwoWaySample, NoDistinct, NoFunctionOnColumnInJoinOrWhere, NoNotIn, NoUnionDistinct,
77
NoWildcardProjection, TooLargeInList,
88
},
99
tree::collect_preorder,
@@ -110,7 +110,7 @@ fn default_rules() -> Vec<Box<dyn Rule>> {
110110
Box::new(NoDistinct),
111111
Box::new(NoNotIn),
112112
Box::new(NoUnionDistinct),
113-
Box::new(NoFunctionInJoinOrWhere),
113+
Box::new(NoFunctionOnColumnInJoinOrWhere),
114114
Box::new(NoWildcardProjection),
115115
Box::new(MissingTwoWaySample),
116116
Box::new(TooLargeInList),

‎crates/uroborosql-lint/src/rules.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
mod missing_two_way_sample;
22
mod no_distinct;
3-
mod no_function_in_join_or_where;
3+
mod no_function_on_column_in_join_or_where;
44
mod no_not_in;
55
mod no_union_distinct;
66
mod no_wildcard_projection;
77
mod too_large_in_list;
88

99
pub use missing_two_way_sample::MissingTwoWaySample;
1010
pub use no_distinct::NoDistinct;
11-
pub use no_function_in_join_or_where::NoFunctionInJoinOrWhere;
11+
pub use no_function_on_column_in_join_or_where::NoFunctionOnColumnInJoinOrWhere;
1212
pub use no_not_in::NoNotIn;
1313
pub use no_union_distinct::NoUnionDistinct;
1414
pub use no_wildcard_projection::NoWildcardProjection;

‎crates/uroborosql-lint/src/rules/no_function_in_join_or_where.rs‎ renamed to ‎crates/uroborosql-lint/src/rules/no_function_on_column_in_join_or_where.rs‎

Lines changed: 83 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -8,64 +8,51 @@ use postgresql_cst_parser::{
88
tree_sitter::{Node, Range},
99
};
1010

11-
/// Detects functions use in JOIN or WHERE conditions.
11+
/// Detects functions use on columns in JOIN or WHERE conditions.
1212
/// source: https://future-architect.github.io/coding-standards/documents/forSQL/SQL%E3%82%B3%E3%83%BC%E3%83%87%E3%82%A3%E3%83%B3%E3%82%B0%E8%A6%8F%E7%B4%84%EF%BC%88PostgreSQL%EF%BC%89.html#:~:text=1-,%E3%82%A4%E3%83%B3%E3%83%87%E3%83%83%E3%82%AF%E3%82%B9%E3%82%AB%E3%83%A9%E3%83%A0%E3%81%AB%E9%96%A2%E6%95%B0,-%E3%82%92%E9%80%9A%E3%81%97%E3%81%9F
13-
pub struct NoFunctionInJoinOrWhere;
13+
pub struct NoFunctionOnColumnInJoinOrWhere;
1414

15-
impl Rule for NoFunctionInJoinOrWhere {
15+
impl Rule for NoFunctionOnColumnInJoinOrWhere {
1616
fn name(&self) -> &'static str {
17-
"no-function-in-join-or-where"
17+
"no-function-on-column-in-join-or-where"
1818
}
1919

2020
fn default_severity(&self) -> Severity {
2121
Severity::Warning
2222
}
2323

2424
fn target_kinds(&self) -> &'static [SyntaxKind] {
25-
&[SyntaxKind::join_qual, SyntaxKind::where_clause]
25+
&[SyntaxKind::func_expr]
2626
}
2727

2828
fn run_on_node<'tree>(&self, node: &Node<'tree>, ctx: &mut LintContext, severity: Severity) {
29-
assert!(matches!(
30-
node.kind(),
31-
SyntaxKind::join_qual | SyntaxKind::where_clause
32-
));
33-
34-
let Some(top_expr) = find_top_expr(node) else {
29+
assert_eq!(node.kind(), SyntaxKind::func_expr);
30+
let func_expr = node;
31+
32+
// 親を参照し、 join_qual か where_clause があるかをチェックする
33+
// その途中で select_no_parens があれば探索を停止する
34+
if !is_in_detection_range(func_expr) {
3535
return;
36-
};
37-
38-
let ranges = detect_column_function_calls(&top_expr);
39-
40-
for range in ranges {
41-
let diagnostic = Diagnostic::new(
42-
self.name(),
43-
severity,
44-
"Functions in JOIN or WHERE conditions can prevent index usage; rewrite without wrapping the column.",
45-
&range,
46-
);
47-
ctx.report(diagnostic);
4836
}
49-
}
50-
}
51-
52-
/// Finds the top `a_expr` in a JOIN or WHERE clause.
53-
fn find_top_expr<'a>(join_qual_or_where_clause: &'a Node<'a>) -> Option<Node<'a>> {
54-
// join_qual:
55-
// - ON a_expr
56-
// - USING '(' name_list ')' opt_alias_clause_for_joiln_using
57-
//
58-
// where_clause:
59-
// - a_expr
60-
61-
let last_child = join_qual_or_where_clause
62-
.last_child()
63-
.expect("join_qual or where_clause must have a last child.");
64-
65-
if last_child.kind() == SyntaxKind::a_expr {
66-
Some(last_child)
67-
} else {
68-
None
37+
38+
// func_expr の最初の子供は func_application, json_aggregate_func, または func_expr_common_subexpr のいずれかである
39+
let function_body = func_expr.first_child().expect("func_expr should have one of func_application, json_aggregate_func, or func_expr_common_subexpr as its first child.");
40+
41+
// 引数にカラムがあるか判定
42+
43+
// 警告範囲には filter や over を含めない
44+
// func_expr の最初の子供の範囲とする
45+
46+
47+
unimplemented!()
48+
49+
let diagnostic = Diagnostic::new(
50+
self.name(),
51+
severity,
52+
"Functions in JOIN or WHERE conditions can prevent index usage; rewrite without wrapping the column.",
53+
&range,
54+
);
55+
ctx.report(diagnostic);
6956
}
7057
}
7158

@@ -79,6 +66,13 @@ fn find_top_expr<'a>(join_qual_or_where_clause: &'a Node<'a>) -> Option<Node<'a>
7966
//
8067
// - func_application
8168
// - カラムが出現しうる箇所は子供のうち func_arg_list か func_arg_expr を見れば良さそう
69+
// - func_name '(' ')'
70+
// - func_name '(' func_arg_list opt_sort_clause ')'
71+
// - func_name '(' VARIADIC func_arg_expr opt_sort_clause ')'
72+
// - func_name '(' func_arg_list ',' VARIADIC func_arg_expr opt_sort_clause ')'
73+
// - func_name '(' ALL func_arg_list opt_sort_clause ')'
74+
// - func_name '(' DISTINCT func_arg_list opt_sort_clause ')'
75+
// - func_name '(' '*' ')'
8276
//
8377
// - json_aggregate_func
8478
// - JSON_ARRAY_AGG '(' json_value_expr_list json_array_constructor_null_clause_opt json_returning_clause_opt
@@ -91,6 +85,34 @@ fn find_top_expr<'a>(join_qual_or_where_clause: &'a Node<'a>) -> Option<Node<'a>
9185
// - a_expr ':' json_value_expr
9286
//
9387
// - func_expr_common_subexpr
88+
// - nothing
89+
// - a_expr
90+
// - extract_list (EXTRACT '(' extract_list ')')
91+
// - overlay_list (OVERLAY '(' overlay_list ')')
92+
// - func_arg_list_opt (func_name '(' func_arg_list_opt ')')
93+
// - func_arg_list
94+
// - position_list (POSITION '(' position_list ')')
95+
// - substr_list (SUBSTRING '(' substr_list ')')
96+
// - trim_list (TRIM '(' trim_list ')')
97+
// - expr_list
98+
// - xmlexists_argument (XMLEXISTS '(' c_expr xmlexists_argument ')')
99+
// - c_expr
100+
// - xml_attribute_list (XMLFOREST '(' xml_attribute_list ')')
101+
102+
103+
104+
// - JSON_OBJECT '(' json_name_and_value_list json_object_constructor_null_clause_opt json_key_uniqueness_constraint_opt json_returning_clause_opt ')'
105+
// - JSON_OBJECT '(' json_returning_clause_opt ')'
106+
// - JSON_ARRAY '(' json_value_expr_list json_array_constructor_null_clause_opt json_returning_clause_opt ')'
107+
// - JSON_ARRAY '(' select_no_parens json_format_clause_opt json_returning_clause_opt ')'
108+
// - JSON_ARRAY '(' json_returning_clause_opt ')'
109+
// - JSON '(' json_value_expr json_key_uniqueness_constraint_opt ')'
110+
// - JSON_SERIALIZE '(' json_value_expr json_returning_clause_opt ')'
111+
// - JSON_QUERY '(' json_value_expr ',' a_expr json_passing_clause_opt json_returning_clause_opt json_wrapper_behavior json_quotes_clause_opt json_behavior_clause_opt ')'
112+
// - JSON_EXISTS '(' json_value_expr ',' a_expr json_passing_clause_opt json_on_error_clause_opt ')'
113+
// - JSON_VALUE '(' json_value_expr ',' a_expr json_passing_clause_opt json_returning_clause_opt json_behavior_clause_opt ')'
114+
115+
94116
//
95117
// a_expr の子孫で func_expr が現れるまでの最短ルート
96118
// a_expr
@@ -103,57 +125,25 @@ const FUNCTION_KINDS: &[SyntaxKind] = &[
103125
SyntaxKind::json_aggregate_func,
104126
];
105127

106-
// SELECT サブクエリ以下は JOIN / WHERE の外側なので探索対象外。
107-
const SUBQUERY_KINDS: &[SyntaxKind] =
108-
&[SyntaxKind::select_with_parens, SyntaxKind::select_no_parens];
109-
110-
fn detect_column_function_calls(a_expr: &Node<'_>) -> Vec<Range> {
111-
let mut ranges = Vec::new();
112-
let mut stack = vec![a_expr.clone()];
113-
114-
while let Some(current) = stack.pop() {
115-
// 対象となる関数ノードを見つけたら、直下に列参照があるかを判定する。
116-
// ネストしている場合は「列に最も近い（内側の）関数だけ」を診断対象にする。
117-
if FUNCTION_KINDS.contains(&current.kind())
118-
&& contains_columnref_excluding_nested_functions(&current)
119-
{
120-
ranges.push(current.range());
121-
}
122-
push_child_nodes(&mut stack, &current);
123-
}
124-
125-
ranges
126-
}
128+
fn is_in_detection_range(func_expr: &Node) -> bool {
129+
// 親を辿り、 join_qual か where_clause があるかを検証する
130+
// その途中で select_no_parens があれば探索を停止する
127131

128-
fn contains_columnref_excluding_nested_functions(node: &Node<'_>) -> bool {
129-
let mut stack = Vec::new();
130-
push_child_nodes(&mut stack, node);
131-
132-
while let Some(current) = stack.pop() {
133-
if current.kind() == SyntaxKind::columnref {
134-
return true;
135-
}
136-
// 内側に別の関数が現れたら、そちらで改めて判定するためここでは追わない。
137-
if FUNCTION_KINDS.contains(&current.kind()) {
138-
continue;
132+
let mut node = func_expr.parent();
133+
while let Some(current) = node {
134+
match current.kind() {
135+
SyntaxKind::join_qual | SyntaxKind::where_clause => return true,
136+
SyntaxKind::select_no_parens => return false,
137+
_ => (),
139138
}
140-
push_child_nodes(&mut stack, &current);
139+
node = current.parent();
141140
}
142-
143141
false
144142
}
145143

146-
fn push_child_nodes<'tree>(stack: &mut Vec<Node<'tree>>, node: &Node<'tree>) {
147-
if SUBQUERY_KINDS.contains(&node.kind()) {
148-
return;
149-
}
150-
151-
for child in node.children() {
152-
if child.child_count() == 0 {
153-
continue;
154-
}
155-
stack.push(child);
156-
}
144+
// 引数にカラムを含むかどうか
145+
fn has_column_argument(func_expr: &Node) -> bool {
146+
unimplemented!()
157147
}
158148

159149
#[cfg(test)]
@@ -162,7 +152,7 @@ mod tests {
162152
use crate::{linter::tests::run_with_rules, Diagnostic, SqlSpan};
163153

164154
fn run(sql: &str) -> Vec<Diagnostic> {
165-
run_with_rules(sql, vec![Box::new(NoFunctionInJoinOrWhere)])
155+
run_with_rules(sql, vec![Box::new(NoFunctionOnColumnInJoinOrWhere)])
166156
}
167157

168158
mod where_clause {
@@ -192,7 +182,7 @@ mod tests {
192182

193183
assert!(diagnostics
194184
.iter()
195-
.any(|diag| diag.rule_id == "no-function-in-join-or-where"),);
185+
.any(|diag| diag.rule_id == "no-function-on-column-in-join-or-where"),);
196186

197187
assert_eq!(diagnostics.len(), 1);
198188

@@ -248,7 +238,7 @@ mod tests {
248238

249239
assert!(diagnostics
250240
.iter()
251-
.all(|diag| diag.rule_id == "no-function-in-join-or-where"),);
241+
.all(|diag| diag.rule_id == "no-function-on-column-in-join-or-where"),);
252242

253243
assert_eq!(
254244
diagnostics.len(),
@@ -291,7 +281,7 @@ mod tests {
291281

292282
assert!(diagnostics
293283
.iter()
294-
.any(|diag| diag.rule_id == "no-function-in-join-or-where"),);
284+
.any(|diag| diag.rule_id == "no-function-on-column-in-join-or-where"),);
295285

296286
assert_eq!(diagnostics.len(), 1);
297287

@@ -306,7 +296,7 @@ mod tests {
306296

307297
assert!(diagnostics
308298
.iter()
309-
.all(|diag| diag.rule_id == "no-function-in-join-or-where"),);
299+
.all(|diag| diag.rule_id == "no-function-on-column-in-join-or-where"),);
310300

311301
assert_eq!(diagnostics.len(), 2,);
312302

0 commit comments

Comments
 (0)