From 8470477000d11e777fc162ada8aa57287d5acbc9 Mon Sep 17 00:00:00 2001 From: harehare Date: Tue, 27 Oct 2026 20:32:32 +0900 Subject: [PATCH 1/6] =?UTF-8?q?=F0=9F=90=9B=20fix(lint):=20skip=20pipeline?= =?UTF-8?q?-tail=20if=20and=20depth-reading=20queries?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit missing_else_in_expr no longer reports an if at the end of a pipeline, which is the documented way to drop a node. prefer_specific_heading no longer reports .h in queries that read .depth or .level. --- .../rules/correctness/missing_else_in_expr.rs | 37 +++++++++++++++++-- .../rules/style/prefer_specific_heading.rs | 16 ++++++++ 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/crates/mq-lint/src/rules/correctness/missing_else_in_expr.rs b/crates/mq-lint/src/rules/correctness/missing_else_in_expr.rs index cd4177ff6..029ece13c 100644 --- a/crates/mq-lint/src/rules/correctness/missing_else_in_expr.rs +++ b/crates/mq-lint/src/rules/correctness/missing_else_in_expr.rs @@ -2,6 +2,9 @@ use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; use mq_hir::SymbolKind; /// Reports `if` expressions that have no `else` branch. +/// +/// An `if` at the end of a pipeline is exempt: without `else` it yields `None`, +/// which is the idiom for dropping a node. pub struct MissingElseInExpr; impl LintRule for MissingElseInExpr { @@ -23,9 +26,32 @@ impl LintRule for MissingElseInExpr { .filter_map(|(_, s)| s.parent) .collect(); + let is_pipeline_tail = |sym: &mq_hir::Symbol| { + let Some(range) = sym.source.text_range else { + return false; + }; + let in_pipeline = match sym.parent { + None => true, + Some(parent) => ctx + .hir + .symbol(parent) + .is_some_and(|p| matches!(p.kind, SymbolKind::Function(_) | SymbolKind::Block)), + }; + in_pipeline + && !ctx.hir.symbols_for_source(ctx.source_id).any(|(_, other)| { + other.parent == sym.parent + && other + .source + .text_range + .is_some_and(|r| (r.start.line, r.start.column) > (range.start.line, range.start.column)) + }) + }; + ctx.hir .symbols_for_source(ctx.source_id) - .filter(|(id, s)| matches!(s.kind, SymbolKind::If) && !if_ids_with_else.contains(id)) + .filter(|(id, s)| { + matches!(s.kind, SymbolKind::If) && !if_ids_with_else.contains(id) && !is_pipeline_tail(s) + }) .map(|(_, sym)| { let mut d = Diagnostic::new(LintMessage::MissingElseInExpr, self.severity()); if let Some(range) = sym.source.text_range { @@ -54,9 +80,9 @@ mod tests { } #[rstest] - #[case("if (true): 1;")] - #[case("if (.h1): 2;")] - #[case("if (true): 1 elif (false): 2;")] + #[case("if (true): 1 | add(1)")] + #[case("if (.h1): 2 | add(1)")] + #[case("if (true): 1 elif (false): 2 | add(1)")] fn detects_missing_else(#[case] code: &str) { let diags = check(code); assert_eq!(diags.len(), 1); @@ -67,6 +93,9 @@ mod tests { #[case("if (true): 1 else: 2;")] #[case("if (true): 1 elif (false): 2 else: 3;")] #[case(".h1")] + #[case("if (true): 1;")] + #[case(".h | if (!is_none(1)): to_md_list(1, 2)")] + #[case("def f(x): add(x, 1) | if (x): 1;")] fn no_diagnostic(#[case] code: &str) { let diags = check(code); assert_eq!(diags.len(), 0); diff --git a/crates/mq-lint/src/rules/style/prefer_specific_heading.rs b/crates/mq-lint/src/rules/style/prefer_specific_heading.rs index 073d9f05f..a176e5c7d 100644 --- a/crates/mq-lint/src/rules/style/prefer_specific_heading.rs +++ b/crates/mq-lint/src/rules/style/prefer_specific_heading.rs @@ -15,6 +15,19 @@ impl LintRule for PreferSpecificHeading { fn check(&self, ctx: &LintContext<'_>) -> Vec { // Selector symbols use insert_symbol → use all_symbols + // `.h.depth` has no `.hN` equivalent when the depth is read at runtime. + let reads_depth = ctx.all_symbols().any(|(_, s)| { + matches!( + s.kind, + SymbolKind::Selector(mq_lang::Selector::Attr( + mq_lang::AttrKind::Depth | mq_lang::AttrKind::Level + )) + ) + }); + if reads_depth { + return Vec::new(); + } + ctx.all_symbols() .filter(|(_, s)| matches!(s.kind, SymbolKind::Selector(mq_lang::Selector::Heading(None)))) .map(|(_, sym)| { @@ -56,6 +69,9 @@ mod tests { #[case(".h2")] #[case(".h6")] #[case(".h1 | .value")] + #[case(".h.depth")] + #[case(".h.level")] + #[case(".h | .depth")] fn no_diagnostic(#[case] code: &str) { let diags = check(code); assert_eq!(diags.len(), 0); From 9c3e6b138a14e044cee82af749ab8d18f2c87392 Mon Sep 17 00:00:00 2001 From: harehare Date: Tue, 27 Oct 2026 20:32:39 +0900 Subject: [PATCH 2/6] =?UTF-8?q?=F0=9F=90=9B=20fix(check):=20type=20argumen?= =?UTF-8?q?t=20selectors=20on=20unknown=20input=20and=20self[...]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A selector argument without a known input type is now an unresolved type variable, since it may select from a node or an array of nodes (nodes | compact(.code)). self[0] and self[:n] are no longer reported as unresolved symbols. --- crates/mq-check/src/constraint.rs | 5 +++++ crates/mq-check/tests/node_types_test.rs | 8 ++++++++ crates/mq-hir/src/error.rs | 19 +++++++++++++++++++ 3 files changed, 32 insertions(+) diff --git a/crates/mq-check/src/constraint.rs b/crates/mq-check/src/constraint.rs index eb7736f9b..e16019b07 100644 --- a/crates/mq-check/src/constraint.rs +++ b/crates/mq-check/src/constraint.rs @@ -1757,6 +1757,11 @@ pub(super) fn generate_symbol_constraints( // an array of all descendant Markdown nodes. if let mq_lang::Selector::Attr(attr_kind) = selector { attr_kind_to_type(attr_kind) + } else if matches!(ctx.input_type(), Type::Dynamic) && !matches!(selector, mq_lang::Selector::Recursive) + { + // An argument selector runs on the unknown input, so it may select from + // a single node or from an array of nodes (`nodes | compact(.code)`). + Type::Var(ctx.fresh_var()) } else if matches!(selector, mq_lang::Selector::Recursive) { Type::array(Type::markdown()) } else { diff --git a/crates/mq-check/tests/node_types_test.rs b/crates/mq-check/tests/node_types_test.rs index 4d3572f52..f92a8e576 100644 --- a/crates/mq-check/tests/node_types_test.rs +++ b/crates/mq-check/tests/node_types_test.rs @@ -308,3 +308,11 @@ fn test_user_functions_that_are_not_exact_predicates_do_not_narrow(#[case] defin let types = ref_types(&code, "h"); assert_eq!(types[types.len() - 2], "markdown", "{types:?}"); } + +#[rstest] +#[case::compact(r#"nodes | compact(.code)"#)] +#[case::first_of_compact(r#"nodes | let n = first(compact(.code)) | n"#)] +#[case::map(r#"nodes | map(.code, to_text)"#)] +fn test_argument_selector_on_unknown_input_may_select_an_array(#[case] code: &str) { + assert!(errors(code).is_empty(), "{code}"); +} diff --git a/crates/mq-hir/src/error.rs b/crates/mq-hir/src/error.rs index ac326726e..933bb1646 100644 --- a/crates/mq-hir/src/error.rs +++ b/crates/mq-hir/src/error.rs @@ -64,7 +64,10 @@ impl Hir { SymbolKind::Call | SymbolKind::Ref => { // builtin.mq may call functions gated behind mq-lang features this build lacks. // A nameless `Call` is the index/slice in `f(x)[0]`, so there is nothing to resolve. + // `self[0]` indexes the keyword value, which is never defined. if symbol.value.is_none() + || (matches!(symbol.value.as_deref(), Some("self")) + && self.bracket_group_counts.contains_key(&symbol_id)) || self.references.contains_key(&symbol_id) || self.is_builtin_symbol(symbol) { @@ -240,6 +243,22 @@ mod tests { use super::*; + #[rstest] + #[case::slice("self[:10]", 0)] + #[case::index("self[0]", 0)] + #[case::unknown("selfx[0]", 1)] + fn test_indexed_self_is_not_unresolved(#[case] code: &str, #[case] expected: usize) { + let mut hir = Hir::default(); + let _ = hir.add_code(None, code); + + let unresolved = hir + .errors() + .iter() + .filter(|e| matches!(e, HirError::UnresolvedSymbol { .. })) + .count(); + assert_eq!(unresolved, expected, "{code}"); + } + #[test] fn test_find_similar_names() { let mut hir = Hir::default(); From f7d80fd4dc741e782a1f5b214adba15e8d48e63b Mon Sep 17 00:00:00 2001 From: harehare Date: Tue, 27 Oct 2026 22:05:46 +0900 Subject: [PATCH 3/6] =?UTF-8?q?=F0=9F=90=9B=20fix(lint):=20count=20selecto?= =?UTF-8?q?r=20and=20variable=20stages=20as=20pipeline=20tail?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit missing_else_in_expr looked for later stages only among symbols added with add_symbol, so an if followed by .h1 or let x = 2 was treated as the end of the pipeline and its warning was suppressed. --- crates/mq-lint/src/rules/correctness/missing_else_in_expr.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/mq-lint/src/rules/correctness/missing_else_in_expr.rs b/crates/mq-lint/src/rules/correctness/missing_else_in_expr.rs index 029ece13c..8fd9b47bd 100644 --- a/crates/mq-lint/src/rules/correctness/missing_else_in_expr.rs +++ b/crates/mq-lint/src/rules/correctness/missing_else_in_expr.rs @@ -38,7 +38,7 @@ impl LintRule for MissingElseInExpr { .is_some_and(|p| matches!(p.kind, SymbolKind::Function(_) | SymbolKind::Block)), }; in_pipeline - && !ctx.hir.symbols_for_source(ctx.source_id).any(|(_, other)| { + && !ctx.all_symbols().any(|(_, other)| { other.parent == sym.parent && other .source @@ -83,6 +83,8 @@ mod tests { #[case("if (true): 1 | add(1)")] #[case("if (.h1): 2 | add(1)")] #[case("if (true): 1 elif (false): 2 | add(1)")] + #[case("if (true): 1 | .h1")] + #[case("if (true): 1 | let x = 2")] fn detects_missing_else(#[case] code: &str) { let diags = check(code); assert_eq!(diags.len(), 1); From e33f7d5332472891208f39d2b06aafd45854f21a Mon Sep 17 00:00:00 2001 From: harehare Date: Tue, 27 Oct 2026 22:05:46 +0900 Subject: [PATCH 4/6] =?UTF-8?q?=F0=9F=90=9B=20fix(check):=20keep=20self[..?= =?UTF-8?q?.]=20element=20type=20and=20argument=20selector=20chains?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit self[i] and self[a:b] now index or slice the piped input like a variable does, instead of leaving the result unconstrained. Children of an argument selector on unknown input are checked against the selected node kind, so compact(.code.depth) reports the missing attribute while the selector's own result still varies with its input. --- crates/mq-check/src/constraint.rs | 180 ++++++++++++++--------- crates/mq-check/src/deferred.rs | 5 +- crates/mq-check/src/infer.rs | 4 +- crates/mq-check/tests/node_types_test.rs | 19 +++ 4 files changed, 132 insertions(+), 76 deletions(-) diff --git a/crates/mq-check/src/constraint.rs b/crates/mq-check/src/constraint.rs index e16019b07..f02dbd435 100644 --- a/crates/mq-check/src/constraint.rs +++ b/crates/mq-check/src/constraint.rs @@ -1688,6 +1688,12 @@ pub(super) fn generate_symbol_constraints( // Attr selectors (`Selector::Attr`) return the concrete attribute type via `attr_kind_to_type`. // Non-Attr selectors (`.h1`, `.code`, etc.) always return `Type::markdown()`. SymbolKind::Selector(ref selector) => { + // An argument selector runs on the unknown input, so it may select from a single + // node or from an array of nodes (`nodes | compact(.code)`). + let varies_with_input = ctx.get_piped_input(symbol_id).is_none() + && matches!(ctx.input_type(), Type::Dynamic) + && !matches!(selector, mq_lang::Selector::Attr(_) | mq_lang::Selector::Recursive); + // Compute this selector's own output type based on the incoming piped input. let own_type = if let Some(piped_ty) = ctx.get_piped_input(symbol_id).cloned() { let resolved = ctx.resolve_type(&piped_ty); @@ -1757,10 +1763,7 @@ pub(super) fn generate_symbol_constraints( // an array of all descendant Markdown nodes. if let mq_lang::Selector::Attr(attr_kind) = selector { attr_kind_to_type(attr_kind) - } else if matches!(ctx.input_type(), Type::Dynamic) && !matches!(selector, mq_lang::Selector::Recursive) - { - // An argument selector runs on the unknown input, so it may select from - // a single node or from an array of nodes (`nodes | compact(.code)`). + } else if varies_with_input { Type::Var(ctx.fresh_var()) } else if matches!(selector, mq_lang::Selector::Recursive) { Type::array(Type::markdown()) @@ -1784,8 +1787,14 @@ pub(super) fn generate_symbol_constraints( if child_selectors.is_empty() { ctx.set_symbol_type(symbol_id, own_type); } else { - // Thread the output type through the chain of child selectors. - ctx.set_piped_input(child_selectors[0], own_type); + // Thread the output type through the chain of child selectors. The children of + // an argument selector are checked against the selected node kind. + let chain_input = if varies_with_input { + selector_kinds(selector).map_or_else(Type::markdown, Type::Node) + } else { + own_type + }; + ctx.set_piped_input(child_selectors[0], chain_input); for i in 0..child_selectors.len() { if let Some(child_sym) = hir.symbol(child_selectors[i]) { generate_symbol_constraints( @@ -1802,7 +1811,11 @@ pub(super) fn generate_symbol_constraints( } } // The parent's type is the last child's type. - let last_ty = ctx.get_or_create_symbol_type(*child_selectors.last().unwrap()); + let last_ty = if varies_with_input { + Type::Var(ctx.fresh_var()) + } else { + ctx.get_or_create_symbol_type(*child_selectors.last().unwrap()) + }; ctx.set_symbol_type(symbol_id, last_ty); } } @@ -1958,6 +1971,75 @@ fn is_index_assign_target(symbol_id: SymbolId, hir: &Hir, children_index: &Child .is_some_and(|parent| get_children(children_index, parent).first() == Some(&symbol_id)) } +/// Constrains the bracket access `v[i]` or `v[a:b]` on a value of type `container_ty`. +/// +/// Two args are a range slice `v[start:end]` lowered as `v(start, end)`. One arg is an element +/// access `v[i]` lowered as `v(i)`. Chained accesses are lowered the same way: `v[0][:key]` is +/// `v(0, :key)`. A String/Symbol literal after the first argument can never be a slice bound, so +/// it marks such a chain. +fn constrain_bracket_access( + hir: &Hir, + ctx: &mut InferenceContext, + symbol_id: SymbolId, + children: &[SymbolId], + explicit_arg_tys: &[Type], + container_ty: Type, + range: Option, +) { + let is_chained_access = children.iter().skip(1).any(|&child_id| { + hir.symbol(child_id) + .is_some_and(|s| matches!(s.kind, SymbolKind::String | SymbolKind::Symbol)) + }); + if is_chained_access { + // The container's element type is not tracked through the chain, so leave the + // result unconstrained. + let ty_var = ctx.fresh_var(); + ctx.set_symbol_type(symbol_id, Type::Var(ty_var)); + } else if children.len() == 2 { + // Range slice: delegate to the "slice" builtin. + let mut slice_arg_tys = vec![container_ty]; + slice_arg_tys.extend(explicit_arg_tys.iter().cloned()); + let defer = might_receive_piped_input(hir, symbol_id); + resolve_builtin_call(ctx, symbol_id, "slice", &slice_arg_tys, range, defer); + } else { + // Element access: defer as a tuple access so that Array/Tuple/Dict container types + // are handled correctly after unification. + let literal_index = children.first().and_then(|&arg_id| { + let arg_sym = hir.symbol(arg_id)?; + if matches!(arg_sym.kind, SymbolKind::Number) { + arg_sym.value.as_ref()?.parse::().ok() + } else { + None + } + }); + + // Only constrain the index to Number when it is a literal numeric index (e.g. v[0]). + // For variable indices (e.g. a String key used for Dict access), skip this constraint: + // the correct element type is resolved via resolve_deferred_tuple_accesses once the + // container's type is known. + if literal_index.is_some() + && let Some(index_ty) = explicit_arg_tys.first() + { + ctx.add_constraint(Constraint::Equal( + index_ty.clone(), + Type::Number, + range, + ConstraintOrigin::General, + )); + } + + let ty_var = ctx.fresh_var(); + ctx.set_symbol_type(symbol_id, Type::Var(ty_var)); + + ctx.add_deferred_tuple_access(infer::DeferredTupleAccess { + call_symbol_id: symbol_id, + container_ty, + index: literal_index, + range: get_symbol_range(hir, symbol_id), + }); + } +} + /// Constrain a call: a user function (with arity, defaults and piped input), or a builtin. fn generate_call_constraints( hir: &Hir, @@ -1982,6 +2064,16 @@ fn generate_call_constraints( record_attr_call(hir, symbol_id, &children, &explicit_arg_tys, func_name == "get", ctx); } + // `self[i]` and `self[a:b]` index or slice the piped input. + if func_name == "self" && hir.bracket_group_count(symbol_id) > 0 { + let self_ty = ctx + .get_piped_input(symbol_id) + .cloned() + .unwrap_or_else(|| Type::Var(ctx.fresh_var())); + constrain_bracket_access(hir, ctx, symbol_id, &children, &explicit_arg_tys, self_ty, range); + return; + } + // Try user-defined function first (via HIR reference resolution) if let Some(def_id) = hir.resolve_reference_symbol(symbol_id) { let def_symbol = hir.symbol(def_id); @@ -2153,69 +2245,17 @@ fn generate_call_constraints( } } else { // Non-function variable with bracket access. - // Two args → range slice v[start:end] lowered as v(start, end). - // One arg → element access v[i] lowered as v(i). - // - // Chained accesses are lowered the same way: `v[0][:key]` is - // `v(0, :key)`. A String/Symbol literal after the first argument - // can never be a slice bound, so it marks such a chain. - let is_chained_access = children.iter().skip(1).any(|&child_id| { - hir.symbol(child_id) - .is_some_and(|s| matches!(s.kind, SymbolKind::String | SymbolKind::Symbol)) - }); - if is_chained_access { - // The container's element type is not tracked through the - // chain, so leave the result unconstrained. - let ty_var = ctx.fresh_var(); - ctx.set_symbol_type(symbol_id, Type::Var(ty_var)); - } else if children.len() == 2 { - // Range slice: delegate to the "slice" builtin. - // Use original_func_ty (not func_ty) so the container's - // type variable stays unified with its definition site. - let mut slice_arg_tys = vec![original_func_ty.clone()]; - slice_arg_tys.extend(explicit_arg_tys.iter().cloned()); - let defer = might_receive_piped_input(hir, symbol_id); - resolve_builtin_call(ctx, symbol_id, "slice", &slice_arg_tys, range, defer); - } else { - // Element access: defer as a tuple access so that - // Array/Tuple/Dict container types are handled correctly - // after unification. - let literal_index = children.first().and_then(|&arg_id| { - let arg_sym = hir.symbol(arg_id)?; - if matches!(arg_sym.kind, SymbolKind::Number) { - arg_sym.value.as_ref()?.parse::().ok() - } else { - None - } - }); - - // Only constrain the index to Number when it is a literal - // numeric index (e.g. v[0]). For variable indices - // (e.g. a String key used for Dict access), skip this - // constraint: the correct element type is resolved via - // resolve_deferred_tuple_accesses once the container's - // type is known. - if literal_index.is_some() - && let Some(index_ty) = explicit_arg_tys.first() - { - ctx.add_constraint(Constraint::Equal( - index_ty.clone(), - Type::Number, - range, - ConstraintOrigin::General, - )); - } - - let ty_var = ctx.fresh_var(); - ctx.set_symbol_type(symbol_id, Type::Var(ty_var)); - - ctx.add_deferred_tuple_access(infer::DeferredTupleAccess { - call_symbol_id: symbol_id, - def_id, - index: literal_index, - range: get_symbol_range(hir, symbol_id), - }); - } + // Use original_func_ty (not func_ty) so the container's type + // variable stays unified with its definition site. + constrain_bracket_access( + hir, + ctx, + symbol_id, + &children, + &explicit_arg_tys, + original_func_ty.clone(), + range, + ); } } } diff --git a/crates/mq-check/src/deferred.rs b/crates/mq-check/src/deferred.rs index 9fe70da64..0d8a2df7f 100644 --- a/crates/mq-check/src/deferred.rs +++ b/crates/mq-check/src/deferred.rs @@ -323,10 +323,7 @@ pub(crate) fn resolve_deferred_tuple_accesses(ctx: &mut InferenceContext) -> boo let mut resolved_any = false; for access in &accesses { - let var_ty = match ctx.get_symbol_type(access.def_id).cloned() { - Some(ty) => ctx.resolve_type(&ty), - None => continue, - }; + let var_ty = ctx.resolve_type(&access.container_ty); match &var_ty { types::Type::Tuple(elems) => { diff --git a/crates/mq-check/src/infer.rs b/crates/mq-check/src/infer.rs index 15f612772..e8b37de17 100644 --- a/crates/mq-check/src/infer.rs +++ b/crates/mq-check/src/infer.rs @@ -167,8 +167,8 @@ pub struct DeferredCallReturnAccess { pub struct DeferredTupleAccess { /// The call symbol ID (the bracket access expression) pub call_symbol_id: SymbolId, - /// The variable definition symbol ID - pub def_id: SymbolId, + /// The type of the indexed value + pub container_ty: Type, /// The literal index value (None if dynamic index) pub index: Option, /// Source range for error reporting diff --git a/crates/mq-check/tests/node_types_test.rs b/crates/mq-check/tests/node_types_test.rs index f92a8e576..03fcbba2e 100644 --- a/crates/mq-check/tests/node_types_test.rs +++ b/crates/mq-check/tests/node_types_test.rs @@ -316,3 +316,22 @@ fn test_user_functions_that_are_not_exact_predicates_do_not_narrow(#[case] defin fn test_argument_selector_on_unknown_input_may_select_an_array(#[case] code: &str) { assert!(errors(code).is_empty(), "{code}"); } + +#[rstest] +#[case::missing_attribute(r#"compact(.code.depth)"#, false)] +#[case::missing_attribute_on_unknown_input(r#"nodes | compact(.code.depth)"#, false)] +#[case::valid_attribute(r#"nodes | compact(.h.depth)"#, true)] +#[case::valid_value(r#"nodes | compact(.code.value)"#, true)] +fn test_argument_selector_chain_checks_attributes(#[case] code: &str, #[case] ok: bool) { + assert_eq!(errors(code).is_empty(), ok, "{code}"); +} + +#[rstest] +#[case::number_element(r#"[1, 2] | self[0] | upcase()"#, false)] +#[case::string_element(r#"["a", "b"] | self[0] | upcase()"#, true)] +#[case::number_slice(r#"[1, 2, 3] | self[1:2] | upcase()"#, false)] +#[case::string_index(r#""abc" | self[0] | upcase()"#, true)] +#[case::unknown_input(r#"self[0] | upcase()"#, true)] +fn test_indexed_self_has_the_element_type_of_the_piped_input(#[case] code: &str, #[case] ok: bool) { + assert_eq!(errors(code).is_empty(), ok, "{code}"); +} From dd2fdd9934b1871ea20fe18e9afc81d88d5bf607 Mon Sep 17 00:00:00 2001 From: harehare Date: Tue, 27 Oct 2026 22:49:30 +0900 Subject: [PATCH 5/6] =?UTF-8?q?=F0=9F=90=9B=20fix(check):=20infer=20chaine?= =?UTF-8?q?d=20self=20indices=20as=20element=20access?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit self[0][1] has two bracket groups with one index each, but two children were always routed to the slice overload, so the result was an array. bracket_group_count now tells a chain from a slice, and each index is applied to the previous result. A chain mixed with slices stays unconstrained. DeferredTupleAccess carries the result type so the intermediate steps need no symbol. --- crates/mq-check/src/constraint.rs | 100 ++++++++++++++--------- crates/mq-check/src/deferred.rs | 12 +-- crates/mq-check/src/infer.rs | 4 +- crates/mq-check/tests/node_types_test.rs | 3 + 4 files changed, 73 insertions(+), 46 deletions(-) diff --git a/crates/mq-check/src/constraint.rs b/crates/mq-check/src/constraint.rs index f02dbd435..78cfd0546 100644 --- a/crates/mq-check/src/constraint.rs +++ b/crates/mq-check/src/constraint.rs @@ -1973,10 +1973,12 @@ fn is_index_assign_target(symbol_id: SymbolId, hir: &Hir, children_index: &Child /// Constrains the bracket access `v[i]` or `v[a:b]` on a value of type `container_ty`. /// -/// Two args are a range slice `v[start:end]` lowered as `v(start, end)`. One arg is an element -/// access `v[i]` lowered as `v(i)`. Chained accesses are lowered the same way: `v[0][:key]` is -/// `v(0, :key)`. A String/Symbol literal after the first argument can never be a slice bound, so -/// it marks such a chain. +/// Two args in one bracket group are a range slice `v[start:end]` lowered as `v(start, end)`. +/// One arg is an element access `v[i]` lowered as `v(i)`. Chained accesses are lowered the same +/// way: `v[0][1]` is `v(0, 1)` and `v[0][:key]` is `v(0, :key)`. The bracket group count tells +/// them apart from a slice. A String/Symbol literal after the first argument can never be a +/// slice bound, so it marks such a chain. A chain mixed with slices is ambiguous and left +/// unconstrained. fn constrain_bracket_access( hir: &Hir, ctx: &mut InferenceContext, @@ -1986,15 +1988,28 @@ fn constrain_bracket_access( container_ty: Type, range: Option, ) { + let groups = hir.bracket_group_count(symbol_id); let is_chained_access = children.iter().skip(1).any(|&child_id| { hir.symbol(child_id) .is_some_and(|s| matches!(s.kind, SymbolKind::String | SymbolKind::Symbol)) }); - if is_chained_access { + if is_chained_access || (groups > 1 && children.len() != groups) { // The container's element type is not tracked through the chain, so leave the // result unconstrained. let ty_var = ctx.fresh_var(); ctx.set_symbol_type(symbol_id, Type::Var(ty_var)); + } else if groups > 1 { + // Element access chain: each index applies to the previous result. + let mut container_ty = container_ty; + for (i, (&arg_id, arg_ty)) in children.iter().zip(explicit_arg_tys).enumerate() { + let result_ty = Type::Var(ctx.fresh_var()); + constrain_element_access(hir, ctx, container_ty, arg_id, arg_ty, result_ty.clone(), range); + if i + 1 == children.len() { + ctx.set_symbol_type(symbol_id, result_ty); + break; + } + container_ty = result_ty; + } } else if children.len() == 2 { // Range slice: delegate to the "slice" builtin. let mut slice_arg_tys = vec![container_ty]; @@ -2002,42 +2017,51 @@ fn constrain_bracket_access( let defer = might_receive_piped_input(hir, symbol_id); resolve_builtin_call(ctx, symbol_id, "slice", &slice_arg_tys, range, defer); } else { - // Element access: defer as a tuple access so that Array/Tuple/Dict container types - // are handled correctly after unification. - let literal_index = children.first().and_then(|&arg_id| { - let arg_sym = hir.symbol(arg_id)?; - if matches!(arg_sym.kind, SymbolKind::Number) { - arg_sym.value.as_ref()?.parse::().ok() - } else { - None - } - }); - - // Only constrain the index to Number when it is a literal numeric index (e.g. v[0]). - // For variable indices (e.g. a String key used for Dict access), skip this constraint: - // the correct element type is resolved via resolve_deferred_tuple_accesses once the - // container's type is known. - if literal_index.is_some() - && let Some(index_ty) = explicit_arg_tys.first() - { - ctx.add_constraint(Constraint::Equal( - index_ty.clone(), - Type::Number, - range, - ConstraintOrigin::General, - )); + let result_ty = Type::Var(ctx.fresh_var()); + ctx.set_symbol_type(symbol_id, result_ty.clone()); + if let (Some(&arg_id), Some(arg_ty)) = (children.first(), explicit_arg_tys.first()) { + constrain_element_access(hir, ctx, container_ty, arg_id, arg_ty, result_ty, range); } + } +} - let ty_var = ctx.fresh_var(); - ctx.set_symbol_type(symbol_id, Type::Var(ty_var)); - - ctx.add_deferred_tuple_access(infer::DeferredTupleAccess { - call_symbol_id: symbol_id, - container_ty, - index: literal_index, - range: get_symbol_range(hir, symbol_id), - }); +/// Constrains the element access `container[arg]`, whose result is `result_ty`. +/// +/// It is deferred as a tuple access so that Array/Tuple/Dict container types are handled +/// correctly after unification. +fn constrain_element_access( + hir: &Hir, + ctx: &mut InferenceContext, + container_ty: Type, + arg_id: SymbolId, + arg_ty: &Type, + result_ty: Type, + range: Option, +) { + let literal_index = hir + .symbol(arg_id) + .filter(|arg_sym| matches!(arg_sym.kind, SymbolKind::Number)) + .and_then(|arg_sym| arg_sym.value.as_ref()?.parse::().ok()); + + // Only constrain the index to Number when it is a literal numeric index (e.g. v[0]). + // For variable indices (e.g. a String key used for Dict access), skip this constraint: + // the correct element type is resolved via resolve_deferred_tuple_accesses once the + // container's type is known. + if literal_index.is_some() { + ctx.add_constraint(Constraint::Equal( + arg_ty.clone(), + Type::Number, + range, + ConstraintOrigin::General, + )); } + + ctx.add_deferred_tuple_access(infer::DeferredTupleAccess { + result_ty, + container_ty, + index: literal_index, + range, + }); } /// Constrain a call: a user function (with arity, defaults and piped input), or a builtin. diff --git a/crates/mq-check/src/deferred.rs b/crates/mq-check/src/deferred.rs index 0d8a2df7f..010541b3a 100644 --- a/crates/mq-check/src/deferred.rs +++ b/crates/mq-check/src/deferred.rs @@ -338,13 +338,13 @@ pub(crate) fn resolve_deferred_tuple_accesses(ctx: &mut InferenceContext) -> boo // Dynamic index, return Union of all element types types::Type::union(elems.clone()) }; - let call_ty = ctx.get_or_create_symbol_type(access.call_symbol_id); + let call_ty = access.result_ty.clone(); ctx.add_constraint(Constraint::Equal(call_ty, result_ty, None, ConstraintOrigin::General)); resolved_any = true; } types::Type::Array(elem) => { // Normal array, bind to element type - let call_ty = ctx.get_or_create_symbol_type(access.call_symbol_id); + let call_ty = access.result_ty.clone(); ctx.add_constraint(Constraint::Equal( call_ty, *elem.clone(), @@ -386,7 +386,7 @@ pub(crate) fn resolve_deferred_tuple_accesses(ctx: &mut InferenceContext) -> boo } else { types::Type::union(elem_types) }; - let call_ty = ctx.get_or_create_symbol_type(access.call_symbol_id); + let call_ty = access.result_ty.clone(); ctx.add_constraint(Constraint::Equal(call_ty, result_ty, None, ConstraintOrigin::General)); resolved_any = true; } @@ -394,7 +394,7 @@ pub(crate) fn resolve_deferred_tuple_accesses(ctx: &mut InferenceContext) -> boo types::Type::Dict(_, value) => { // Dict bracket access: result type is the dict's value type. // e.g. `d[key]` where d: Dict(String, V) → V - let call_ty = ctx.get_or_create_symbol_type(access.call_symbol_id); + let call_ty = access.result_ty.clone(); ctx.add_constraint(Constraint::Equal( call_ty, *value.clone(), @@ -405,7 +405,7 @@ pub(crate) fn resolve_deferred_tuple_accesses(ctx: &mut InferenceContext) -> boo } types::Type::String => { // String bracket access: `s[n]` or `s[n:m]` returns a String (character/slice). - let call_ty = ctx.get_or_create_symbol_type(access.call_symbol_id); + let call_ty = access.result_ty.clone(); ctx.add_constraint(Constraint::Equal( call_ty, types::Type::String, @@ -436,7 +436,7 @@ pub(crate) fn resolve_deferred_tuple_accesses(ctx: &mut InferenceContext) -> boo access.range, ConstraintOrigin::General, )); - let call_ty = ctx.get_or_create_symbol_type(access.call_symbol_id); + let call_ty = access.result_ty.clone(); ctx.add_constraint(Constraint::Equal(call_ty, elem_ty, None, ConstraintOrigin::General)); resolved_any = true; } diff --git a/crates/mq-check/src/infer.rs b/crates/mq-check/src/infer.rs index e8b37de17..71a4a9f33 100644 --- a/crates/mq-check/src/infer.rs +++ b/crates/mq-check/src/infer.rs @@ -165,8 +165,8 @@ pub struct DeferredCallReturnAccess { /// the variable's Tuple type is fully resolved. #[derive(Debug, Clone)] pub struct DeferredTupleAccess { - /// The call symbol ID (the bracket access expression) - pub call_symbol_id: SymbolId, + /// The type of the accessed element + pub result_ty: Type, /// The type of the indexed value pub container_ty: Type, /// The literal index value (None if dynamic index) diff --git a/crates/mq-check/tests/node_types_test.rs b/crates/mq-check/tests/node_types_test.rs index 03fcbba2e..324f8f1a6 100644 --- a/crates/mq-check/tests/node_types_test.rs +++ b/crates/mq-check/tests/node_types_test.rs @@ -332,6 +332,9 @@ fn test_argument_selector_chain_checks_attributes(#[case] code: &str, #[case] ok #[case::number_slice(r#"[1, 2, 3] | self[1:2] | upcase()"#, false)] #[case::string_index(r#""abc" | self[0] | upcase()"#, true)] #[case::unknown_input(r#"self[0] | upcase()"#, true)] +#[case::chained_string_element(r#"[["a", "b"]] | self[0][1] | upcase()"#, true)] +#[case::chained_number_element(r#"[[1, 2]] | self[0][1] | upcase()"#, false)] +#[case::chained_inner_array(r#"[["a", "b"]] | self[0] | upcase()"#, false)] fn test_indexed_self_has_the_element_type_of_the_piped_input(#[case] code: &str, #[case] ok: bool) { assert_eq!(errors(code).is_empty(), ok, "{code}"); } From 6b85bac9a3866813af92e6103b2eb329c79fd2d9 Mon Sep 17 00:00:00 2001 From: harehare Date: Wed, 28 Oct 2026 09:41:00 +0900 Subject: [PATCH 6/6] =?UTF-8?q?=F0=9F=90=9B=20fix(check):=20resolve=20deep?= =?UTF-8?q?=20bracket=20chains=20until=20no=20accesses=20remain?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Chained lookups like self[0][0][0][0] resolve one level per pass, but the checker ran the deferred tuple access resolver a fixed three times, leaving the fourth element untyped. Drain the resolver until it makes no progress. --- crates/mq-check/src/deferred.rs | 10 ++++++++++ crates/mq-check/src/lib.rs | 12 +++--------- crates/mq-check/tests/type_errors_test.rs | 11 +++++++++++ 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/crates/mq-check/src/deferred.rs b/crates/mq-check/src/deferred.rs index 010541b3a..24167e124 100644 --- a/crates/mq-check/src/deferred.rs +++ b/crates/mq-check/src/deferred.rs @@ -445,6 +445,16 @@ pub(crate) fn resolve_deferred_tuple_accesses(ctx: &mut InferenceContext) -> boo resolved_any } +/// Resolves deferred tuple accesses until no more resolve, solving constraints between rounds. +/// +/// Chained lookups like `v[0][0][0]` use the previous result as the next container, so each +/// one resolves in a later round. Every productive round removes at least one queued access. +pub(crate) fn drain_deferred_tuple_accesses(ctx: &mut InferenceContext) { + while resolve_deferred_tuple_accesses(ctx) { + unify::solve_constraints(ctx); + } +} + /// Result of checking union member overload compatibility. enum UnionCheckResult { /// All non-None members return a consistent type, with optional None propagation. diff --git a/crates/mq-check/src/lib.rs b/crates/mq-check/src/lib.rs index c242e6273..f1adb6a37 100644 --- a/crates/mq-check/src/lib.rs +++ b/crates/mq-check/src/lib.rs @@ -454,9 +454,7 @@ impl TypeChecker { laps.lap("narrowing"); // Resolve deferred tuple index accesses now that variable types are known. - if deferred::resolve_deferred_tuple_accesses(&mut ctx) { - unify::solve_constraints(&mut ctx); - } + deferred::drain_deferred_tuple_accesses(&mut ctx); // Check the value assigned through `a[i][j] = v` against the element it replaces. if deferred::resolve_deferred_index_assigns(&mut ctx) { @@ -500,18 +498,14 @@ impl TypeChecker { // `resolve_deferred_overloads` runs. This ensures that index accesses on variables // with union types containing unresolved vars (e.g., `Union(None, Var)`) are // retried with the now-concrete member types. - if deferred::resolve_deferred_tuple_accesses(&mut ctx) { - unify::solve_constraints(&mut ctx); - } + deferred::drain_deferred_tuple_accesses(&mut ctx); laps.lap("deferred_accesses"); // Resolve deferred try/catch branch-type merges, then retry accesses/operators // that depend on a `let v = try: ... catch: ...` variable's now-final type. deferred::resolve_deferred_try_catches(&mut ctx); - if deferred::resolve_deferred_tuple_accesses(&mut ctx) { - unify::solve_constraints(&mut ctx); - } + deferred::drain_deferred_tuple_accesses(&mut ctx); laps.lap("deferred_try_catches"); deferred::resolve_deferred_overloads(&mut ctx); laps.lap("resolve_deferred_overloads"); diff --git a/crates/mq-check/tests/type_errors_test.rs b/crates/mq-check/tests/type_errors_test.rs index 5a32e7014..eda61621f 100644 --- a/crates/mq-check/tests/type_errors_test.rs +++ b/crates/mq-check/tests/type_errors_test.rs @@ -531,6 +531,17 @@ fn test_tuple_literal_index_number_element() { ); } +#[rstest] +#[case::four(r#"[[[[1]]]] | self[0][0][0][0] | upcase()"#)] +#[case::five(r#"[[[[[1]]]]] | self[0][0][0][0][0] | upcase()"#)] +fn test_deep_bracket_chain_resolves_element(#[case] code: &str) { + let result = check_types_with_builtins(code); + assert!( + !result.is_empty(), + "deep index chain should resolve to a number: {code}" + ); +} + #[test] fn test_tuple_literal_index_string_element() { // v[1] should be String, so v[1] - 1 should be a type error