Skip to content

Conversation

@lemonadern
Copy link
Collaborator

@lemonadern lemonadern commented Nov 14, 2025

Summary

新しいルール no-function-on-column-in-join-or-where を追加しました。
WHERE 句や JOIN の結合条件において、カラムに対して関数を適用している箇所を検出します。

Implementation Notes

関数を表すノードをトリガーに発動し、以下の2ステップを踏みます。

  1. 関数が検出範囲内に存在するか検証する
    • 関数ノードから親を辿り、 where_clause か over_clause を見つける
    • 見つからない場合や先に select のルート(サブクエリを含む)に来たら検出範囲内でないことがわかる
  2. 関数引数内のカラム参照を検出する
    • 関数の filter 句や over 句を除いた部分の子孫うち、カラム参照を洗い出す
    • 関数直下のカラム参照であれば関数本体を検出対象として確定
    • 直下かどうかの判定
      • それぞれのカラム参照から親を辿り、最初に見つかった func_expr がルールのトリガーになった func_expr と同一であればネストしていないと判断する
      • 最初に見つかった func_expr がルールのトリガーになった func_expr と同一でなければさらに内側の関数があるためそちらで検出される

@lemonadern lemonadern force-pushed the feat/linter-functions-on-join-or-where branch from c56b300 to 6866b87 Compare December 2, 2025 06:38
@lemonadern lemonadern marked this pull request as ready for review December 2, 2025 06:39
@lemonadern lemonadern changed the title feat(linter rule): functions-on-join-or-where feat(linter rule): add no-function-on-column-in-join-or-where Dec 2, 2025
Comment on lines +185 to +195
assert!(
spans.iter().any(|span| span == &"trim(users.name)"),
"trim should be reported"
);

assert!(
spans
.iter()
.any(|span| span == &"coalesce(users.deleted_at, trim(users.name))"),
"outer coalesce should still be reported"
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

細かいところですが、以下の方がシンプルに書けそうです。

            assert!(
                spans.contains(&"trim(users.name)"),
                "trim should be reported"
            );

            assert!(
                spans.contains(&"coalesce(users.deleted_at, trim(users.name))"),
                "outer coalesce should still be reported"
            );

Comment on lines +24 to +26
fn target_kinds(&self) -> &'static [SyntaxKind] {
&[SyntaxKind::func_expr]
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

columnref から上に辿って行った方が検出ロジックはシンプルになりそうですが、以下のような理由で現状の実装を選択したのでしょうか?

  1. coalesce(users.deleted_at, users.updated_at) のようなケースで同一の diagnostic を複数作成しないようにする
  2. func_expr ノードは columnref ノードよりも数が少ないので、func_expr ノードを起点にチェックしたほうが効率的

個人的には、diagnostic のメッセージにカラム名も含めてしまい、同一範囲のdiagnostic を複数作成してしまってもよいのかなと思いました(vscode で診断結果を hover すると、範囲が被っている全ての診断結果のメッセージが表示されるので、同一範囲のdiagnostic を複数あっても問題ないかなと)
また、もしパフォーマンスについて懸念されているのであれば、問題となるケースが見つかってからでよいのかなと思っています。

いかがでしょうか。 @ppputtyo @lemonadern

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants