Merge pull request #21564 from Veykril/push-kvlytkymtvks

fix: Fix upvar analysis of nested closures

src/tools/rust-analyzer/crates/hir-def/src/expr_store.rs

@@ -474,8 +474,8 @@ impl ExpressionStore {
         match expr_only.binding_owners.get(&binding) {
             Some(it) => {
                 // We assign expression ids in a way that outer closures will receive
-                // a lower id
-                it.into_raw() < relative_to.into_raw()
+                // a higher id (allocated after their body is collected)
+                it.into_raw() > relative_to.into_raw()
             }
             None => true,
         }

src/tools/rust-analyzer/crates/hir-ty/src/tests/closure_captures.rs

@@ -135,7 +135,7 @@ fn check_closure_captures(#[rust_analyzer::rust_fixture] ra_fixture: &str, expec
 fn deref_in_let() {
     check_closure_captures(
         r#"
-//- minicore:copy
+//- minicore:copy, fn
 fn main() {
     let a = &mut true;
     let closure = || { let b = *a; };
@@ -149,7 +149,7 @@ fn main() {
 fn deref_then_ref_pattern() {
     check_closure_captures(
         r#"
-//- minicore:copy
+//- minicore:copy, fn
 fn main() {
     let a = &mut true;
     let closure = || { let &mut ref b = a; };
@@ -159,7 +159,7 @@ fn main() {
     );
     check_closure_captures(
         r#"
-//- minicore:copy
+//- minicore:copy, fn
 fn main() {
     let a = &mut true;
     let closure = || { let &mut ref mut b = a; };
@@ -173,7 +173,7 @@ fn main() {
 fn unique_borrow() {
     check_closure_captures(
         r#"
-//- minicore:copy
+//- minicore:copy, fn
 fn main() {
     let a = &mut true;
     let closure = || { *a = false; };
@@ -187,7 +187,7 @@ fn main() {
 fn deref_ref_mut() {
     check_closure_captures(
         r#"
-//- minicore:copy
+//- minicore:copy, fn
 fn main() {
     let a = &mut true;
     let closure = || { let ref mut b = *a; };
@@ -201,7 +201,7 @@ fn main() {
 fn let_else_not_consuming() {
     check_closure_captures(
         r#"
-//- minicore:copy
+//- minicore:copy, fn
 fn main() {
     let a = &mut true;
     let closure = || { let _ = *a else { return; }; };
@@ -215,7 +215,7 @@ fn main() {
 fn consume() {
     check_closure_captures(
         r#"
-//- minicore:copy
+//- minicore:copy, fn
 struct NonCopy;
 fn main() {
     let a = NonCopy;
@@ -230,7 +230,7 @@ fn main() {
 fn ref_to_upvar() {
     check_closure_captures(
         r#"
-//- minicore:copy
+//- minicore:copy, fn
 struct NonCopy;
 fn main() {
     let mut a = NonCopy;
@@ -248,7 +248,7 @@ fn main() {
 fn field() {
     check_closure_captures(
         r#"
-//- minicore:copy
+//- minicore:copy, fn
 struct Foo { a: i32, b: i32 }
 fn main() {
     let a = Foo { a: 0, b: 0 };
@@ -263,7 +263,7 @@ fn main() {
 fn fields_different_mode() {
     check_closure_captures(
         r#"
-//- minicore:copy
+//- minicore:copy, fn
 struct NonCopy;
 struct Foo { a: i32, b: i32, c: NonCopy, d: bool }
 fn main() {
@@ -286,7 +286,7 @@ fn main() {
 fn autoref() {
     check_closure_captures(
         r#"
-//- minicore:copy
+//- minicore:copy, fn
 struct Foo;
 impl Foo {
     fn imm(&self) {}
@@ -308,7 +308,7 @@ fn main() {
 fn captures_priority() {
     check_closure_captures(
         r#"
-//- minicore:copy
+//- minicore:copy, fn
 struct NonCopy;
 fn main() {
     let mut a = &mut true;
@@ -336,7 +336,7 @@ fn main() {
 fn let_underscore() {
     check_closure_captures(
         r#"
-//- minicore:copy
+//- minicore:copy, fn
 fn main() {
     let mut a = true;
     let closure = || { let _ = a; };
@@ -350,7 +350,7 @@ fn main() {
 fn match_wildcard() {
     check_closure_captures(
         r#"
-//- minicore:copy
+//- minicore:copy, fn
 struct NonCopy;
 fn main() {
     let mut a = NonCopy;
@@ -375,7 +375,7 @@ fn main() {
 fn multiple_bindings() {
     check_closure_captures(
         r#"
-//- minicore:copy
+//- minicore:copy, fn
 fn main() {
     let mut a = false;
     let mut closure = || { let (b | b) = a; };
@@ -389,7 +389,7 @@ fn main() {
 fn multiple_usages() {
     check_closure_captures(
         r#"
-//- minicore:copy
+//- minicore:copy, fn
 fn main() {
     let mut a = false;
     let mut closure = || {
@@ -410,7 +410,7 @@ fn main() {
 fn ref_then_deref() {
     check_closure_captures(
         r#"
-//- minicore:copy
+//- minicore:copy, fn
 fn main() {
     let mut a = false;
     let mut closure = || { let b = *&mut a; };
@@ -424,7 +424,7 @@ fn main() {
 fn ref_of_ref() {
     check_closure_captures(
         r#"
-//- minicore:copy
+//- minicore:copy, fn
 fn main() {
     let mut a = &false;
     let closure = || { let b = &a; };
@@ -446,7 +446,7 @@ fn main() {
 fn multiple_capture_usages() {
     check_closure_captures(
         r#"
-//- minicore:copy
+//- minicore:copy, fn
 struct A { a: i32, b: bool }
 fn main() {
     let mut a = A { a: 123, b: false };
@@ -465,7 +465,7 @@ fn main() {
 fn let_binding_is_a_ref_capture_in_ref_binding() {
     check_closure_captures(
         r#"
-//- minicore:copy
+//- minicore:copy, fn
 struct S;
 fn main() {
     let mut s = S;
@@ -489,7 +489,7 @@ fn main() {
 fn let_binding_is_a_value_capture_in_binding() {
     check_closure_captures(
         r#"
-//- minicore:copy, option
+//- minicore:copy, fn, option
 struct Box(i32);
 fn main() {
     let b = Some(Box(0));
@@ -508,7 +508,7 @@ fn main() {
 fn alias_needs_to_be_normalized() {
     check_closure_captures(
         r#"
-//- minicore:copy
+//- minicore:copy, fn
 trait Trait {
     type Associated;
 }
@@ -528,3 +528,41 @@ fn main() {
         expect!["220..257;174..175;245..250 ByRef(Shared) c.b.x &'? i32"],
     );
 }
+
+#[test]
+fn nested_ref_captures_from_outer() {
+    check_closure_captures(
+        r#"
+//- minicore:copy, fn
+fn f() {
+    let a = 1;
+    let a_closure = || {
+        let b_closure = || {
+            { a };
+        };
+    };
+}
+"#,
+        expect![[r#"
+            44..113;17..18;92..93 ByRef(Shared) a &'? i32
+            73..106;17..18;92..93 ByRef(Shared) a &'? i32"#]],
+    );
+}
+
+#[test]
+fn nested_ref_captures() {
+    check_closure_captures(
+        r#"
+//- minicore:copy, fn
+fn f() {
+    let a_closure = || {
+        let b = 2;
+        let b_closure = || {
+            { b };
+        };
+    };
+}
+"#,
+        expect!["77..110;46..47;96..97 ByRef(Shared) b &'? i32"],
+    );
+}

src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/mutability_errors.rs

@@ -1,5 +1,3 @@
-#![expect(unused, reason = "diagnostics is temporarily disabled due to too many false positives")]
-
 use hir::db::ExpandDatabase;
 use ide_db::source_change::SourceChange;
 use ide_db::text_edit::TextEdit;
@@ -90,17 +88,16 @@ pub(crate) fn unused_mut(ctx: &DiagnosticsContext<'_>, d: &hir::UnusedMut) -> Op
         )])
     })();
     let ast = d.local.primary_source(ctx.sema.db).syntax_ptr();
-    // Some(
-    //     Diagnostic::new_with_syntax_node_ptr(
-    //         ctx,
-    //         DiagnosticCode::RustcLint("unused_mut"),
-    //         "variable does not need to be mutable",
-    //         ast,
-    //     )
-    //     // Not supporting `#[allow(unused_mut)]` in proc macros leads to false positive, hence not stable.
-    //     .with_fixes(fixes),
-    // )
-    None
+    Some(
+        Diagnostic::new_with_syntax_node_ptr(
+            ctx,
+            DiagnosticCode::RustcLint("unused_mut"),
+            "variable does not need to be mutable",
+            ast,
+        )
+        // Not supporting `#[allow(unused_mut)]` in proc macros leads to false positive, hence not stable.
+        .with_fixes(fixes),
+    )
 }
 
 pub(super) fn token(parent: &SyntaxNode, kind: SyntaxKind) -> Option<SyntaxToken> {
@@ -108,7 +105,6 @@ pub(super) fn token(parent: &SyntaxNode, kind: SyntaxKind) -> Option<SyntaxToken
 }
 
 #[cfg(test)]
-#[cfg(false)] // Diagnostic temporarily disabled
 mod tests {
     use crate::tests::{check_diagnostics, check_diagnostics_with_disabled, check_fix};
 
@@ -999,10 +995,6 @@ fn fn_once(mut x: impl FnOnce(u8) -> u8) -> u8 {
         }
                     "#,
         );
-        // FIXME: There should be no "unused variable" here, and there should be a mutability error,
-        // but our MIR infra is horribly broken and due to the order in which expressions are lowered
-        // there is no `StorageLive` for `x` in the closure (in fact, `x` should not even be a variable
-        // of the closure, the environment should be, but as I said, our MIR infra is horribly broken).
         check_diagnostics(
             r#"
 //- minicore: copy, fn
@@ -1011,8 +1003,8 @@ fn f() {
         || {
             || {
                 let x = 2;
-                 // ^ 💡 warn: unused variable
                 || { || { x = 5; } }
+                        //^^^^^ 💡 error: cannot mutate immutable variable `x`
             }
         }
     };

src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/unused_variables.rs

@@ -1,5 +1,3 @@
-#![expect(unused, reason = "diagnostics is temporarily disabled due to too many false positives")]
-
 use hir::Name;
 use ide_db::text_edit::TextEdit;
 use ide_db::{
@@ -42,26 +40,25 @@ pub(crate) fn unused_variables(
         .and_then(syntax::ast::RecordPatField::cast)
         .is_some_and(|field| field.colon_token().is_none());
     let var_name = d.local.name(ctx.sema.db);
-    // Some(
-    //     Diagnostic::new_with_syntax_node_ptr(
-    //         ctx,
-    //         DiagnosticCode::RustcLint("unused_variables"),
-    //         "unused variable",
-    //         ast,
-    //     )
-    //     .with_fixes(name_range.and_then(|it| {
-    //         fixes(
-    //             ctx.sema.db,
-    //             var_name,
-    //             it.range,
-    //             diagnostic_range,
-    //             ast.file_id.is_macro(),
-    //             is_shorthand_field,
-    //             ctx.edition,
-    //         )
-    //     })),
-    // )
-    None
+    Some(
+        Diagnostic::new_with_syntax_node_ptr(
+            ctx,
+            DiagnosticCode::RustcLint("unused_variables"),
+            "unused variable",
+            ast,
+        )
+        .with_fixes(name_range.and_then(|it| {
+            fixes(
+                ctx.sema.db,
+                var_name,
+                it.range,
+                diagnostic_range,
+                ast.file_id.is_macro(),
+                is_shorthand_field,
+                ctx.edition,
+            )
+        })),
+    )
 }
 
 fn fixes(
@@ -94,7 +91,6 @@ fn fixes(
 }
 
 #[cfg(test)]
-#[cfg(false)] // Diagnostic temporarily disabled
 mod tests {
     use crate::tests::{check_diagnostics, check_fix};