walker: precise per-literal spans for expression WARNINGs
Expression WARNING diagnostics (type mismatch, = NULL) carried a coarse span — the whole WHERE clause, from the `where` keyword to end of input. They now span exactly the offending literal operand, read from the Operand source span added in the previous commit. predicate_warnings derives the span per warning; pair_type_mismatch returns (message, literal-span); the dead where_clause_span helper is removed. 5 walker tests assert the spans cover exactly the literal / identifier (type mismatch, = NULL, BETWEEN bounds, IN item, unknown-column ERROR). 1105 passing, clippy clean.
This commit is contained in:
+125
-53
@@ -441,45 +441,35 @@ const fn command_where_expr(command: &Command) -> Option<&Expr> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// A coarse span covering the WHERE clause — from the `where`
|
|
||||||
/// keyword to the end of input. The validity indicator reads
|
|
||||||
/// only the severity; the span is for the (eventual)
|
|
||||||
/// highlight overlay.
|
|
||||||
fn where_clause_span(path: &MatchedPath, source_len: usize) -> (usize, usize) {
|
|
||||||
path.items
|
|
||||||
.iter()
|
|
||||||
.find(|i| matches!(&i.kind, outcome::MatchedKind::Word("where")))
|
|
||||||
.map_or((0, source_len), |w| (w.span.0, source_len))
|
|
||||||
}
|
|
||||||
|
|
||||||
/// WARNING diagnostics for a WHERE expression (ADR-0026 §7):
|
/// WARNING diagnostics for a WHERE expression (ADR-0026 §7):
|
||||||
/// a type-mismatched comparison, or `= NULL` / `!= NULL`.
|
/// a type-mismatched comparison, or `= NULL` / `!= NULL`.
|
||||||
/// Both are valid and runnable — the warning is advisory.
|
/// Both are valid and runnable — the warning is advisory.
|
||||||
|
///
|
||||||
|
/// Each diagnostic's span is the offending **literal operand**
|
||||||
|
/// — precise enough for a per-literal highlight (ADR-0027).
|
||||||
fn expr_warnings(
|
fn expr_warnings(
|
||||||
expr: &Expr,
|
expr: &Expr,
|
||||||
columns: &[crate::completion::TableColumn],
|
columns: &[crate::completion::TableColumn],
|
||||||
span: (usize, usize),
|
|
||||||
) -> Vec<outcome::Diagnostic> {
|
) -> Vec<outcome::Diagnostic> {
|
||||||
let mut out = Vec::new();
|
let mut out = Vec::new();
|
||||||
collect_expr_warnings(expr, columns, span, &mut out);
|
collect_expr_warnings(expr, columns, &mut out);
|
||||||
out
|
out
|
||||||
}
|
}
|
||||||
|
|
||||||
fn collect_expr_warnings(
|
fn collect_expr_warnings(
|
||||||
expr: &Expr,
|
expr: &Expr,
|
||||||
columns: &[crate::completion::TableColumn],
|
columns: &[crate::completion::TableColumn],
|
||||||
span: (usize, usize),
|
|
||||||
out: &mut Vec<outcome::Diagnostic>,
|
out: &mut Vec<outcome::Diagnostic>,
|
||||||
) {
|
) {
|
||||||
match expr {
|
match expr {
|
||||||
Expr::Or(terms) | Expr::And(terms) => {
|
Expr::Or(terms) | Expr::And(terms) => {
|
||||||
for term in terms {
|
for term in terms {
|
||||||
collect_expr_warnings(term, columns, span, out);
|
collect_expr_warnings(term, columns, out);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
Expr::Not(inner) => collect_expr_warnings(inner, columns, span, out),
|
Expr::Not(inner) => collect_expr_warnings(inner, columns, out),
|
||||||
Expr::Predicate(predicate) => {
|
Expr::Predicate(predicate) => {
|
||||||
predicate_warnings(predicate, columns, span, out);
|
predicate_warnings(predicate, columns, out);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -487,11 +477,10 @@ fn collect_expr_warnings(
|
|||||||
fn predicate_warnings(
|
fn predicate_warnings(
|
||||||
predicate: &Predicate,
|
predicate: &Predicate,
|
||||||
columns: &[crate::completion::TableColumn],
|
columns: &[crate::completion::TableColumn],
|
||||||
span: (usize, usize),
|
|
||||||
out: &mut Vec<outcome::Diagnostic>,
|
out: &mut Vec<outcome::Diagnostic>,
|
||||||
) {
|
) {
|
||||||
use outcome::{Diagnostic, Severity};
|
use outcome::{Diagnostic, Severity};
|
||||||
let warn = |message: String| Diagnostic {
|
let warn = |message: String, span: (usize, usize)| Diagnostic {
|
||||||
severity: Severity::Warning,
|
severity: Severity::Warning,
|
||||||
span,
|
span,
|
||||||
message,
|
message,
|
||||||
@@ -500,36 +489,40 @@ fn predicate_warnings(
|
|||||||
Predicate::Compare { left, op, right } => {
|
Predicate::Compare { left, op, right } => {
|
||||||
// `= NULL` / `!= NULL`: valid syntax that is never
|
// `= NULL` / `!= NULL`: valid syntax that is never
|
||||||
// true — the user almost certainly means IS NULL.
|
// true — the user almost certainly means IS NULL.
|
||||||
if matches!(op, CompareOp::Eq | CompareOp::NotEq)
|
// The highlight points at the `null` literal itself.
|
||||||
&& (is_null_literal(left) || is_null_literal(right))
|
let null_operand = if matches!(op, CompareOp::Eq | CompareOp::NotEq) {
|
||||||
{
|
[left, right].into_iter().find(|&o| is_null_literal(o))
|
||||||
out.push(warn(crate::friendly::translate(
|
} else {
|
||||||
"diagnostic.eq_null",
|
None
|
||||||
&[],
|
};
|
||||||
)));
|
if let Some(operand) = null_operand {
|
||||||
} else if let Some(message) =
|
out.push(warn(
|
||||||
|
crate::friendly::translate("diagnostic.eq_null", &[]),
|
||||||
|
operand.span(),
|
||||||
|
));
|
||||||
|
} else if let Some((message, span)) =
|
||||||
pair_type_mismatch(left, right, columns)
|
pair_type_mismatch(left, right, columns)
|
||||||
{
|
{
|
||||||
out.push(warn(message));
|
out.push(warn(message, span));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
Predicate::Between {
|
Predicate::Between {
|
||||||
target, low, high, ..
|
target, low, high, ..
|
||||||
} => {
|
} => {
|
||||||
for bound in [low, high] {
|
for bound in [low, high] {
|
||||||
if let Some(message) =
|
if let Some((message, span)) =
|
||||||
pair_type_mismatch(target, bound, columns)
|
pair_type_mismatch(target, bound, columns)
|
||||||
{
|
{
|
||||||
out.push(warn(message));
|
out.push(warn(message, span));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
Predicate::In { target, items, .. } => {
|
Predicate::In { target, items, .. } => {
|
||||||
for item in items {
|
for item in items {
|
||||||
if let Some(message) =
|
if let Some((message, span)) =
|
||||||
pair_type_mismatch(target, item, columns)
|
pair_type_mismatch(target, item, columns)
|
||||||
{
|
{
|
||||||
out.push(warn(message));
|
out.push(warn(message, span));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -552,20 +545,24 @@ const fn is_null_literal(operand: &Operand) -> bool {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// If one operand is a known column and the other a non-null
|
/// If one operand is a known column and the other a non-null
|
||||||
/// literal whose type the column cannot hold, the message for
|
/// literal whose type the column cannot hold, the type-mismatch
|
||||||
/// a type-mismatch WARNING; otherwise `None` (column-to-column,
|
/// WARNING message paired with the **literal operand's span**;
|
||||||
/// literal-to-literal, an unknown column — already an ERROR —
|
/// otherwise `None` (column-to-column, literal-to-literal, an
|
||||||
/// or a compatible pair).
|
/// unknown column — already an ERROR — or a compatible pair).
|
||||||
fn pair_type_mismatch(
|
fn pair_type_mismatch(
|
||||||
a: &Operand,
|
a: &Operand,
|
||||||
b: &Operand,
|
b: &Operand,
|
||||||
columns: &[crate::completion::TableColumn],
|
columns: &[crate::completion::TableColumn],
|
||||||
) -> Option<String> {
|
) -> Option<(String, (usize, usize))> {
|
||||||
let (column, literal) = match (a, b) {
|
let (column, literal, span) = match (a, b) {
|
||||||
(Operand::Column { name: c, .. }, Operand::Literal { value: v, .. })
|
(
|
||||||
| (Operand::Literal { value: v, .. }, Operand::Column { name: c, .. }) => {
|
Operand::Column { name, .. },
|
||||||
(c, v)
|
Operand::Literal { value, span },
|
||||||
}
|
)
|
||||||
|
| (
|
||||||
|
Operand::Literal { value, span },
|
||||||
|
Operand::Column { name, .. },
|
||||||
|
) => (name, value, *span),
|
||||||
_ => return None,
|
_ => return None,
|
||||||
};
|
};
|
||||||
// `null` fits any column; `= NULL` is flagged separately.
|
// `null` fits any column; `= NULL` is flagged separately.
|
||||||
@@ -579,12 +576,15 @@ fn pair_type_mismatch(
|
|||||||
if literal.bind_for_column(column, ty).is_ok() {
|
if literal.bind_for_column(column, ty).is_ok() {
|
||||||
return None;
|
return None;
|
||||||
}
|
}
|
||||||
Some(crate::friendly::translate(
|
Some((
|
||||||
"diagnostic.type_mismatch",
|
crate::friendly::translate(
|
||||||
&[
|
"diagnostic.type_mismatch",
|
||||||
("column", column as &dyn std::fmt::Display),
|
&[
|
||||||
("type", &ty.keyword() as &dyn std::fmt::Display),
|
("column", column as &dyn std::fmt::Display),
|
||||||
],
|
("type", &ty.keyword() as &dyn std::fmt::Display),
|
||||||
|
],
|
||||||
|
),
|
||||||
|
span,
|
||||||
))
|
))
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -878,11 +878,7 @@ pub fn walk<'a>(
|
|||||||
&& let Some(expr) = command_where_expr(command)
|
&& let Some(expr) = command_where_expr(command)
|
||||||
{
|
{
|
||||||
let columns = ctx.current_table_columns.as_deref().unwrap_or(&[]);
|
let columns = ctx.current_table_columns.as_deref().unwrap_or(&[]);
|
||||||
diagnostics.extend(expr_warnings(
|
diagnostics.extend(expr_warnings(expr, columns));
|
||||||
expr,
|
|
||||||
columns,
|
|
||||||
where_clause_span(&path, effective_source.len()),
|
|
||||||
));
|
|
||||||
}
|
}
|
||||||
let result = WalkResult {
|
let result = WalkResult {
|
||||||
outcome: final_outcome,
|
outcome: final_outcome,
|
||||||
@@ -1744,6 +1740,82 @@ mod tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ---- precise diagnostic spans (ADR-0027 highlight wiring) -
|
||||||
|
|
||||||
|
/// Walk `input` with `schema` and return the diagnostics the
|
||||||
|
/// walk produced.
|
||||||
|
fn diagnostics(
|
||||||
|
input: &str,
|
||||||
|
schema: &SchemaCache,
|
||||||
|
) -> Vec<super::outcome::Diagnostic> {
|
||||||
|
let mut ctx = super::context::WalkContext::with_schema(schema);
|
||||||
|
let (result, _cmd) =
|
||||||
|
super::walk(input, super::outcome::WalkBound::EndOfInput, &mut ctx);
|
||||||
|
result.map_or_else(Vec::new, |r| r.diagnostics)
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn type_mismatch_warning_span_covers_only_the_literal() {
|
||||||
|
// The coarse pre-ADR-0027 span was the whole WHERE
|
||||||
|
// clause; it is now exactly the offending literal.
|
||||||
|
let schema = schema_with("Customers", &[("Age", Type::Int)]);
|
||||||
|
let input = "delete from Customers where Age = 'hello'";
|
||||||
|
let diags = diagnostics(input, &schema);
|
||||||
|
assert_eq!(diags.len(), 1);
|
||||||
|
assert_eq!(diags[0].severity, super::Severity::Warning);
|
||||||
|
let (s, e) = diags[0].span;
|
||||||
|
assert_eq!(
|
||||||
|
&input[s..e],
|
||||||
|
"'hello'",
|
||||||
|
"the WARNING span should cover exactly the literal",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn eq_null_warning_span_covers_the_null_literal() {
|
||||||
|
let schema = schema_with("Customers", &[("Name", Type::Text)]);
|
||||||
|
let input = "delete from Customers where Name = null";
|
||||||
|
let diags = diagnostics(input, &schema);
|
||||||
|
assert_eq!(diags.len(), 1);
|
||||||
|
let (s, e) = diags[0].span;
|
||||||
|
assert_eq!(&input[s..e], "null");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn between_warning_spans_each_offending_bound() {
|
||||||
|
// `Age` is int; both text bounds mismatch — two
|
||||||
|
// distinct WARNINGs, each spanning its own bound.
|
||||||
|
let schema = schema_with("Customers", &[("Age", Type::Int)]);
|
||||||
|
let input = "delete from Customers where Age between 'a' and 'z'";
|
||||||
|
let diags = diagnostics(input, &schema);
|
||||||
|
assert_eq!(diags.len(), 2);
|
||||||
|
let spans: Vec<&str> =
|
||||||
|
diags.iter().map(|d| &input[d.span.0..d.span.1]).collect();
|
||||||
|
assert_eq!(spans, vec!["'a'", "'z'"]);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn in_warning_spans_only_the_mismatched_item() {
|
||||||
|
// `Age` is int; of `(1, 'two', 3)` only `'two'` is wrong.
|
||||||
|
let schema = schema_with("Customers", &[("Age", Type::Int)]);
|
||||||
|
let input = "delete from Customers where Age in (1, 'two', 3)";
|
||||||
|
let diags = diagnostics(input, &schema);
|
||||||
|
assert_eq!(diags.len(), 1);
|
||||||
|
let (s, e) = diags[0].span;
|
||||||
|
assert_eq!(&input[s..e], "'two'");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn unknown_column_error_span_covers_the_identifier() {
|
||||||
|
let schema = schema_with("Customers", &[("id", Type::Int)]);
|
||||||
|
let input = "delete from Customers where NoSuchCol = 1";
|
||||||
|
let diags = diagnostics(input, &schema);
|
||||||
|
assert_eq!(diags.len(), 1);
|
||||||
|
assert_eq!(diags[0].severity, super::Severity::Error);
|
||||||
|
let (s, e) = diags[0].span;
|
||||||
|
assert_eq!(&input[s..e], "NoSuchCol");
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn walker_parses_insert_with_explicit_column_list() {
|
fn walker_parses_insert_with_explicit_column_list() {
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
|
|||||||
Reference in New Issue
Block a user