Skip to content

🐛 fix(mq-check): skip Refs with Selector children in type narrowing#1434

Merged
harehare merged 1 commit intomainfrom
fix/narrowing-skip-selector-refs
Mar 12, 2026
Merged

🐛 fix(mq-check): skip Refs with Selector children in type narrowing#1434
harehare merged 1 commit intomainfrom
fix/narrowing-skip-selector-refs

Conversation

@harehare
Copy link
Copy Markdown
Owner

No description provided.

Copilot AI review requested due to automatic review settings March 12, 2026 13:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes an interaction between flow-sensitive type narrowing and selector-based attribute access in mq-check, preventing narrowed variable types from overwriting selector result types.

Changes:

  • Build a set of Ref symbol IDs that have Selector children.
  • Skip applying type narrowings to those Refs so selector output types (e.g., md.depth: Number) aren’t corrupted.

Comment on lines +442 to +446
let mut refs_with_selector_child: rustc_hash::FxHashSet<SymbolId> = rustc_hash::FxHashSet::default();
for (_, sym) in hir.symbols() {
if let Some(parent_id) = sym.parent
&& matches!(sym.kind, mq_hir::SymbolKind::Selector(_))
{
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

FxHashSet is already imported (along with SymbolKind), so using the fully-qualified rustc_hash::FxHashSet / mq_hir::SymbolKind here is redundant and inconsistent with the rest of the module. Consider using the existing imports (and letting the type be inferred) to keep this block idiomatic and reduce noise.

Copilot uses AI. Check for mistakes.
Comment on lines +458 to +463
// Skip Refs that have Selector children (e.g., `node.row`): their stored
// type is the selector output, not the variable type, so narrowing them
// would corrupt the selector output.
if refs_with_selector_child.contains(&ref_id) {
continue;
}
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

This change fixes a subtle narrowing/selector interaction, but there’s no regression test covering it. Please add a test case where a variable is known to be Markdown (e.g., let x = .h;) and inside if (is_markdown(x)) an attribute selector like x.depth is used in a numeric context (e.g., x.depth + 1), asserting it type-checks. Without the skip, narrowing would incorrectly override the Ref’s selector-output type and the test would fail.

Copilot generated this review using guidance from repository custom instructions.
@harehare harehare merged commit 3968660 into main Mar 12, 2026
8 checks passed
@harehare harehare deleted the fix/narrowing-skip-selector-refs branch March 12, 2026 14:04
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.

2 participants