Skip to content

Commit 4882501

Browse files
lancelot-botlancelot-bot
committed
Fix 22 bugs found in static audit
2 parents 2e5e0a5 + 35497e9 commit 4882501

7 files changed

Lines changed: 166 additions & 49 deletions

File tree

‎crates/knot-compiler/src/codegen.rs‎

Lines changed: 47 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -12867,16 +12867,27 @@ impl Codegen {
1286712867
if let Some(&v) = env.bindings.get(name.as_str()) {
1286812868
v
1286912869
} else {
12870-
let var_expr = ast::Spanned::new(
12871-
ast::ExprKind::Var(name.clone()),
12872-
ast::Span::new(0, 0),
12873-
);
12874-
self.compile_expr(builder, &var_expr, env, db)
12870+
// Check let_bindings first — a do-local let variable
12871+
// would panic if resolved through compile_expr as a
12872+
// top-level function.
12873+
let let_expr = self.let_bindings.get(name).cloned();
12874+
if let Some(let_expr) = let_expr {
12875+
self.compile_expr(builder, &let_expr, env, db)
12876+
} else {
12877+
let var_expr = ast::Spanned::new(
12878+
ast::ExprKind::Var(name.clone()),
12879+
ast::Span::new(0, 0),
12880+
);
12881+
self.compile_expr(builder, &var_expr, env, db)
12882+
}
1287512883
}
1287612884
}
1287712885
SqlParamSource::FieldAccess(var, field) => {
12886+
let let_expr = self.let_bindings.get(var).cloned();
1287812887
let record = if let Some(&v) = env.bindings.get(var.as_str()) {
1287912888
v
12889+
} else if let Some(let_expr) = let_expr {
12890+
self.compile_expr(builder, &let_expr, env, db)
1288012891
} else {
1288112892
let var_expr = ast::Spanned::new(
1288212893
ast::ExprKind::Var(var.clone()),
@@ -17527,26 +17538,33 @@ fn extract_literal(expr: &ast::Expr) -> Option<CompileLit> {
1752717538
/// satisfied, `Some(false)` if it fails, or `None` if it can't be
1752817539
/// evaluated at compile time.
1752917540
fn eval_refine_predicate(pred: &ast::Expr, lit: &CompileLit) -> Option<bool> {
17530-
// The predicate is a lambda `\x -> body`. Extract the body.
17531-
let body = match &pred.node {
17541+
// The predicate is a lambda `\\x -> body`. Extract the parameter name
17542+
// and body so we only substitute for the actual parameter, not any
17543+
// other variable the predicate may reference (e.g. a top-level constant).
17544+
let (param_name, body) = match &pred.node {
1753217545
ast::ExprKind::Lambda { params, body } => {
1753317546
if params.len() != 1 { return None; }
17534-
body
17547+
let name = match &params[0].node {
17548+
ast::PatKind::Var(n) => n.clone(),
17549+
_ => return None,
17550+
};
17551+
(name, body)
1753517552
}
1753617553
_ => return None,
1753717554
};
1753817555
// Evaluate the body with the parameter bound to the literal.
17539-
eval_expr_bool(body, lit)
17556+
eval_expr_bool(body, lit, &param_name)
1754017557
}
1754117558

1754217559
/// Evaluate an expression to a boolean, with the refinement variable
17543-
/// bound to `lit`. Returns `None` if the expression can't be evaluated.
17544-
fn eval_expr_bool(expr: &ast::Expr, lit: &CompileLit) -> Option<bool> {
17560+
/// `param_name` bound to `lit`. Returns `None` if the expression can't be
17561+
/// evaluated (e.g. references a variable other than `param_name`).
17562+
fn eval_expr_bool(expr: &ast::Expr, lit: &CompileLit, param_name: &str) -> Option<bool> {
1754517563
match &expr.node {
1754617564
ast::ExprKind::Lit(ast::Literal::Bool(b)) => Some(*b),
1754717565
ast::ExprKind::BinOp { op, lhs, rhs, .. } => {
17548-
let lv = eval_expr_num(lhs, lit)?;
17549-
let rv = eval_expr_num(rhs, lit)?;
17566+
let lv = eval_expr_num(lhs, lit, param_name)?;
17567+
let rv = eval_expr_num(rhs, lit, param_name)?;
1755017568
match op {
1755117569
ast::BinOp::Lt => Some(lv < rv),
1755217570
ast::BinOp::Gt => Some(lv > rv),
@@ -17555,40 +17573,46 @@ fn eval_expr_bool(expr: &ast::Expr, lit: &CompileLit) -> Option<bool> {
1755517573
ast::BinOp::Eq => Some(lv == rv),
1755617574
ast::BinOp::Neq => Some(lv != rv),
1755717575
ast::BinOp::And => {
17558-
Some(eval_expr_bool(lhs, lit)? && eval_expr_bool(rhs, lit)?)
17576+
Some(eval_expr_bool(lhs, lit, param_name)? && eval_expr_bool(rhs, lit, param_name)?)
1755917577
}
1756017578
ast::BinOp::Or => {
17561-
Some(eval_expr_bool(lhs, lit)? || eval_expr_bool(rhs, lit)?)
17579+
Some(eval_expr_bool(lhs, lit, param_name)? || eval_expr_bool(rhs, lit, param_name)?)
1756217580
}
1756317581
_ => None,
1756417582
}
1756517583
}
1756617584
ast::ExprKind::UnaryOp { op: ast::UnaryOp::Not, operand, .. } => {
17567-
Some(!eval_expr_bool(operand, lit)?)
17585+
Some(!eval_expr_bool(operand, lit, param_name)?)
1756817586
}
1756917587
_ => None,
1757017588
}
1757117589
}
1757217590

1757317591
/// Evaluate an expression to an f64 (for numeric comparisons), with the
17574-
/// refinement variable bound to `lit`.
17575-
fn eval_expr_num(expr: &ast::Expr, lit: &CompileLit) -> Option<f64> {
17592+
/// refinement variable `param_name` bound to `lit`. Returns `None` for any
17593+
/// variable that isn't the refinement parameter.
17594+
fn eval_expr_num(expr: &ast::Expr, lit: &CompileLit, param_name: &str) -> Option<f64> {
1757617595
match &expr.node {
1757717596
ast::ExprKind::Lit(ast::Literal::Int(s)) => s.parse::<f64>().ok(),
1757817597
ast::ExprKind::Lit(ast::Literal::Float(f)) => Some(*f),
17579-
ast::ExprKind::Var(_) => {
17598+
ast::ExprKind::Var(name) if name == param_name => {
1758017599
// The lambda parameter — return the literal value
1758117600
match lit {
1758217601
CompileLit::Int(n) => Some(*n as f64),
1758317602
CompileLit::Float(f) => Some(*f),
1758417603
_ => None,
1758517604
}
1758617605
}
17587-
ast::ExprKind::Annot { expr, .. } => eval_expr_num(expr, lit),
17588-
ast::ExprKind::UnitLit { value, .. } => eval_expr_num(value, lit),
17606+
ast::ExprKind::Var(_) => {
17607+
// A different variable (e.g. a top-level constant) — can't
17608+
// evaluate at compile time, fall back to runtime check.
17609+
None
17610+
}
17611+
ast::ExprKind::Annot { expr, .. } => eval_expr_num(expr, lit, param_name),
17612+
ast::ExprKind::UnitLit { value, .. } => eval_expr_num(value, lit, param_name),
1758917613
ast::ExprKind::BinOp { op, lhs, rhs, .. } => {
17590-
let lv = eval_expr_num(lhs, lit)?;
17591-
let rv = eval_expr_num(rhs, lit)?;
17614+
let lv = eval_expr_num(lhs, lit, param_name)?;
17615+
let rv = eval_expr_num(rhs, lit, param_name)?;
1759217616
match op {
1759317617
ast::BinOp::Add => Some(lv + rv),
1759417618
ast::BinOp::Sub => Some(lv - rv),

‎crates/knot-compiler/src/infer.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6023,11 +6023,13 @@ impl Infer {
60236023
self.unify(&rhs_ty, &Ty::Bool, rhs.span);
60246024
Ty::Bool
60256025
}
6026-
// Concat: both same type (Semigroup), result same type
6026+
// Concat: both same type (Semigroup), result same type — but
6027+
// degrade refinement, since `Short ++ Short` can exceed the
6028+
// length bound (mirrors Add/Sub/Mod and Mul/Div above).
60276029
ast::BinOp::Concat => {
60286030
self.unify_symmetric(&lhs_ty, &rhs_ty, span);
60296031
self.require_trait("Semigroup", &lhs_ty, span);
6030-
lhs_ty
6032+
self.degrade_refinement(lhs_ty, span)
60316033
}
60326034
// Pipe: a |> f = f a
60336035
ast::BinOp::Pipe => {

‎crates/knot-lsp/src/code_action.rs‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2104,14 +2104,25 @@ fn import_is_used(
21042104
return true;
21052105
}
21062106
}
2107-
// Also check direct names from import_defs (in case origins aren't tracked)
2107+
// Also check import_defs directly — import_origins is last-write-wins
2108+
// (HashMap<String, String>), so when the same name is imported from two
2109+
// different modules, only the last import's path survives. If a name is
2110+
// in import_defs (meaning it was imported from SOME file) and is
2111+
// referenced, consider this import used if its path matches the stored
2112+
// origin OR if the name has no other origin that would match.
21082113
for (name, (path, _)) in &doc.import_defs {
2109-
// Reconstruct the "origin" from path: this is best-effort, prefer origins
2110-
let origin = doc.import_origins.get(name);
2111-
if origin == Some(&imp.path) && referenced.contains(name) {
2114+
if !referenced.contains(name) {
2115+
continue;
2116+
}
2117+
// Direct path match (canonical path vs relative — may not match,
2118+
// but try as a fallback).
2119+
if path == &std::path::PathBuf::from(&imp.path) {
2120+
return true;
2121+
}
2122+
// If import_origins maps this name to imp.path, it's used.
2123+
if doc.import_origins.get(name) == Some(&imp.path) {
21122124
return true;
21132125
}
2114-
let _ = path;
21152126
}
21162127
false
21172128
}
@@ -2825,8 +2836,7 @@ fn find_inline_actions(
28252836
// Check if cursor is on the let binding
28262837
if stmt.span.start <= cursor_offset && cursor_offset <= stmt.span.end
28272838
&& let ast::PatKind::Var(var_name) = &pat.node {
2828-
let value_text = &doc.source
2829-
[value_expr.span.start..value_expr.span.end.min(doc.source.len())];
2839+
let value_text = safe_slice(&doc.source, value_expr.span);
28302840

28312841
// Count usages of this variable in subsequent statements
28322842
let use_count = doc
@@ -3291,7 +3301,7 @@ fn find_if_to_case_at(
32913301
offset: usize,
32923302
best: &mut Option<(Span, String)>,
32933303
) {
3294-
if expr.span.start > offset || offset > expr.span.end {
3304+
if expr.span.start > offset || offset >= expr.span.end {
32953305
return;
32963306
}
32973307
if let ast::ExprKind::If {
@@ -3370,7 +3380,7 @@ fn find_flip_binary_at(
33703380
offset: usize,
33713381
best: &mut Option<(Span, String)>,
33723382
) {
3373-
if expr.span.start > offset || offset > expr.span.end {
3383+
if expr.span.start > offset || offset >= expr.span.end {
33743384
return;
33753385
}
33763386
if let ast::ExprKind::BinOp { op, lhs, rhs } = &expr.node {
@@ -3469,7 +3479,7 @@ fn find_pipe_conversion_at(
34693479
is_app_head: bool,
34703480
needs_parens: bool,
34713481
) {
3472-
if expr.span.start > offset || offset > expr.span.end {
3482+
if expr.span.start > offset || offset >= expr.span.end {
34733483
return;
34743484
}
34753485
if let ast::ExprKind::App { func, arg } = &expr.node {

‎crates/knot-lsp/src/completion.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,7 +1022,7 @@ fn offset_in_route_rate_limit(module: &Module, offset: usize) -> bool {
10221022
if let DeclKind::Route { entries, .. } = &decl.node {
10231023
for entry in entries {
10241024
if let Some(rl) = &entry.rate_limit
1025-
&& rl.span.start <= offset && offset <= rl.span.end {
1025+
&& rl.span.start <= offset && offset < rl.span.end {
10261026
return true;
10271027
}
10281028
}
@@ -1084,7 +1084,7 @@ fn route_completions(doc: &DocumentState) -> Vec<CompletionItem> {
10841084
/// record fields, etc.
10851085
fn find_enclosing_do_span(module: &Module, offset: usize) -> Option<Span> {
10861086
fn walk(expr: &ast::Expr, offset: usize, best: &mut Option<Span>) {
1087-
if expr.span.start > offset || offset > expr.span.end {
1087+
if expr.span.start > offset || offset >= expr.span.end {
10881088
return;
10891089
}
10901090
if let ast::ExprKind::Do(_) = &expr.node {
@@ -1204,7 +1204,7 @@ fn monad_for_do_span(
12041204
.collect();
12051205
if let Some((_, kind)) = contained
12061206
.iter()
1207-
.filter(|(s, _)| s.start <= offset && offset <= s.end)
1207+
.filter(|(s, _)| s.start <= offset && offset < s.end)
12081208
.min_by_key(|(s, _)| (s.end - s.start, s.start, s.end))
12091209
{
12101210
return Some((*kind).clone());

‎crates/knot-lsp/src/inlay_hints.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1166,7 +1166,7 @@ fn add_monad_context_hints(
11661166
hints: &mut Vec<InlayHint>,
11671167
) {
11681168
if let ast::ExprKind::Do(_) = &expr.node
1169-
&& expr.span.start >= range_start && expr.span.start <= range_end
1169+
&& expr.span.start >= range_start && expr.span.start < range_end
11701170
&& let Some(monad) = doc.monad_info.get(&expr.span) {
11711171
let label = match monad {
11721172
MonadKind::Relation => "[Relation]".to_string(),

‎crates/knot-lsp/src/rename.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1698,6 +1698,20 @@ fn scan_disk_files(
16981698
if already_scanned.contains(&path) {
16991699
continue;
17001700
}
1701+
// Skip non-owner files that have a pending (unsaved) source in the
1702+
// document cache — the on-disk content may differ from the editor's
1703+
// live buffer, so edits computed against disk bytes would have wrong
1704+
// positions. The owner is NOT skipped: when it's stale, the in-memory
1705+
// scan skipped it, and the disk phase must rename its declaration
1706+
// from the stable on-disk bytes (B70 regression test covers this).
1707+
let is_owner = path == owner.canonical_path;
1708+
if !is_owner {
1709+
if let Some(file_uri) = path_to_uri(&path) {
1710+
if state.pending_sources.get(&file_uri).is_some() {
1711+
continue;
1712+
}
1713+
}
1714+
}
17011715
let (module, file_source) =
17021716
match get_or_parse_file_shared(&path, &state.import_cache) {
17031717
Some(v) => v,

0 commit comments

Comments
 (0)