feat: variable declaration statements are not allowed as the body of loop

crates/sema/src/ast_passes.rs

@@ -5,6 +5,8 @@ use solar_data_structures::Never;
 use solar_interface::{diagnostics::DiagCtxt, sym, Session, Span};
 use std::ops::ControlFlow;
 
+mod utils;
+
 #[instrument(name = "ast_passes", level = "debug", skip_all)]
 pub(crate) fn run(sess: &Session, ast: &ast::SourceUnit<'_>) {
     validate(sess, ast);
@@ -121,11 +123,34 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> {
 
     fn visit_stmt(&mut self, stmt: &'ast ast::Stmt<'ast>) -> ControlFlow<Self::BreakValue> {
         match &stmt.kind {
-            ast::StmtKind::While(_, body, ..)
-            | ast::StmtKind::DoWhile(body, ..)
-            | ast::StmtKind::For { body, .. } => {
+            ast::StmtKind::While(cond, body) => {
+                self.visit_expr(cond)?;
+                self.in_loop_depth += 1;
+                let r = self.visit_stmt(body);
+                utils::check_if_loop_body_is_a_variable_declaration(body, self.dcx());
+                self.in_loop_depth -= 1;
+                return r;
+            }
+            ast::StmtKind::DoWhile(body, ..) => {
+                self.in_loop_depth += 1;
+                let r = self.visit_stmt(body);
+                utils::check_if_loop_body_is_a_variable_declaration(body, self.dcx());
+                self.in_loop_depth -= 1;
+                return r;
+            }
+            ast::StmtKind::For { init, cond, next, body } => {
+                if let Some(init) = init {
+                    self.visit_stmt(init)?;
+                }
+                if let Some(cond) = cond {
+                    self.visit_expr(cond)?;
+                }
+                if let Some(next) = next {
+                    self.visit_expr(next)?;
+                }
                 self.in_loop_depth += 1;
-                let r = self.walk_stmt(body);
+                let r = self.visit_stmt(body);
+                utils::check_if_loop_body_is_a_variable_declaration(body, self.dcx());
                 self.in_loop_depth -= 1;
                 return r;
             }
@@ -147,7 +172,7 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> {
 
                 let prev = self.in_unchecked_block;
                 self.in_unchecked_block = true;
-                let r = self.walk_block(block);
+                let r = self.visit_block(block);
                 self.in_unchecked_block = prev;
                 return r;
             }
@@ -245,13 +270,4 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> {
         }
         self.walk_using_directive(using)
     }
-
-    // Intentionally override unused default implementations to reduce bloat.
-    fn visit_expr(&mut self, _expr: &'ast ast::Expr<'ast>) -> ControlFlow<Self::BreakValue> {
-        ControlFlow::Continue(())
-    }
-
-    fn visit_ty(&mut self, _ty: &'ast ast::Type<'ast>) -> ControlFlow<Self::BreakValue> {
-        ControlFlow::Continue(())
-    }
 }

crates/sema/src/ast_passes/utils.rs

@@ -0,0 +1,70 @@
+use solar_ast::{visit::Visit, Stmt};
+use solar_data_structures::Never;
+use solar_interface::diagnostics::DiagCtxt;
+use std::ops::ControlFlow;
+
+struct VariableDeclarationAsLoopBodyChecker<'ast, 'sess> {
+    visited_block: bool,
+    #[allow(dead_code)]
+    body: &'ast &'ast mut Stmt<'ast>,
+    dcx: &'sess DiagCtxt,
+}
+
+impl<'ast, 'sess> VariableDeclarationAsLoopBodyChecker<'ast, 'sess> {
+    pub(crate) fn new(body: &'ast &'ast mut Stmt<'ast>, dcx: &'sess DiagCtxt) -> Self {
+        Self { visited_block: false, body, dcx }
+    }
+
+    /// Returns the diagnostics context.
+    #[inline]
+    fn dcx(&self) -> &'sess DiagCtxt {
+        self.dcx
+    }
+}
+
+impl<'ast> Visit<'ast> for VariableDeclarationAsLoopBodyChecker<'ast, '_> {
+    type BreakValue = Never;
+
+    fn visit_block(
+        &mut self,
+        block: &'ast solar_ast::Block<'ast>,
+    ) -> ControlFlow<Self::BreakValue> {
+        self.visited_block = true;
+        self.walk_block(block)
+    }
+
+    fn visit_variable_definition(
+        &mut self,
+        var: &'ast solar_ast::VariableDefinition<'ast>,
+    ) -> ControlFlow<Self::BreakValue> {
+        if !self.visited_block {
+            self.dcx()
+                .err("variable declarations are not allowed as the body of a loop")
+                .span(var.span)
+                .help("wrap the statement in a block (`{ ... }`)")
+                .emit();
+        }
+        self.walk_variable_definition(var)
+    }
+
+    fn visit_stmt(&mut self, stmt: &'ast solar_ast::Stmt<'ast>) -> ControlFlow<Self::BreakValue> {
+        match &stmt.kind {
+            solar_ast::StmtKind::While(..)
+            | solar_ast::StmtKind::DoWhile(..)
+            | solar_ast::StmtKind::For { .. } => {
+                return ControlFlow::Continue(());
+            }
+            _ => {}
+        };
+        self.walk_stmt(stmt)
+    }
+}
+
+pub(crate) fn check_if_loop_body_is_a_variable_declaration<'ast>(
+    body: &'ast &'ast mut Stmt<'ast>,
+    dcx: &DiagCtxt,
+) {
+    // Check and emit
+    let mut checker = VariableDeclarationAsLoopBodyChecker::new(body, dcx);
+    checker.visit_stmt(body);
+}

tests/ui/resolve/loops.sol

@@ -8,25 +8,30 @@ function funky() {
 
     for (;;) break;
     for (; i < 40; i++) continue;
-    for (; i++ < 50;) continue;
+    for (; i++ < 50; ) continue;
 
-    // ---
-    // TODO: `Error: Variable declarations can only be used inside blocks.`
-
-    while (a == 0) uint a = 0; //~ ERROR: unresolved symbol
+    while (a == 0) { //~ ERROR: unresolved symbol 
+        uint a = 0;
+    }
     a; //~ ERROR: unresolved symbol
-    while (b == 0) { uint b = 0; } //~ ERROR: unresolved symbol
+    while (b == 0) { //~ ERROR: unresolved symbol
+        uint b = 0;
+    } 
     b; //~ ERROR: unresolved symbol
 
-    do uint c; while (c == 0); //~ ERROR: unresolved symbol
+    do { uint c; } while (c == 0); //~ ERROR: unresolved symbol
     c; //~ ERROR: unresolved symbol
-    do { uint d; } while (d == 0); //~ ERROR: unresolved symbol
+    do {
+        uint d;
+    } while (d == 0); //~ ERROR: unresolved symbol
     d; //~ ERROR: unresolved symbol
 
-    for (; false; e++) uint e; //~ ERROR: unresolved symbol
+    for (; false; e++) { uint e; } //~ ERROR: unresolved symbol
     e; //~ ERROR: unresolved symbol
-    for (; false; f++) { uint f; } //~ ERROR: unresolved symbol
-    f; //~ ERROR: unresolved symbol
+    for (; false; f++) {//~ ERROR: unresolved symbol
+
+        uint f;
+    }     f; //~ ERROR: unresolved symbol
     for (uint g; false; g++) {
         g;
     }

tests/ui/resolve/loops.stderr

@@ -1,7 +1,7 @@
 error: unresolved symbol `a`
   --> ROOT/tests/ui/resolve/loops.sol:LL:CC
    |
-LL |     while (a == 0) uint a = 0;
+LL |     while (a == 0) {
    |            ^
    |
 
@@ -15,7 +15,7 @@ LL |     a;
 error: unresolved symbol `b`
   --> ROOT/tests/ui/resolve/loops.sol:LL:CC
    |
-LL |     while (b == 0) { uint b = 0; }
+LL |     while (b == 0) {
    |            ^
    |
 
@@ -29,8 +29,8 @@ LL |     b;
 error: unresolved symbol `c`
   --> ROOT/tests/ui/resolve/loops.sol:LL:CC
    |
-LL |     do uint c; while (c == 0);
-   |                       ^
+LL |     do { uint c; } while (c == 0);
+   |                           ^
    |
 
 error: unresolved symbol `c`
@@ -43,8 +43,8 @@ LL |     c;
 error: unresolved symbol `d`
   --> ROOT/tests/ui/resolve/loops.sol:LL:CC
    |
-LL |     do { uint d; } while (d == 0);
-   |                           ^
+LL |     } while (d == 0);
+   |              ^
    |
 
 error: unresolved symbol `d`
@@ -57,7 +57,7 @@ LL |     d;
 error: unresolved symbol `e`
   --> ROOT/tests/ui/resolve/loops.sol:LL:CC
    |
-LL |     for (; false; e++) uint e;
+LL |     for (; false; e++) { uint e; }
    |                   ^
    |
 
@@ -71,15 +71,15 @@ LL |     e;
 error: unresolved symbol `f`
   --> ROOT/tests/ui/resolve/loops.sol:LL:CC
    |
-LL |     for (; false; f++) { uint f; }
+LL |     for (; false; f++) {
    |                   ^
    |
 
 error: unresolved symbol `f`
   --> ROOT/tests/ui/resolve/loops.sol:LL:CC
    |
-LL |     f;
-   |     ^
+LL |     }     f;
+   |           ^
    |
 
 error: unresolved symbol `g`

tests/ui/typeck/var_decl_as_loop_body.sol

@@ -0,0 +1,29 @@
+contract C {
+    function var_decl_inside_loops() external {
+        for (uint256 i = 0; i < 100; ++i) {
+            uint256 m_count = i + 1 * 2;
+        }
+
+        for (uint256 i = 0; i < 100; ++i) uint256 m_count = i + 1 * 2; //~ ERROR: variable declarations are not allowed as the body of a loop
+
+        for (uint256 i = 0; i < 100; ++i)
+            for (uint256 j = 0; i < 100; ++j) {
+                uint256 k = i + j;
+            }
+
+        for (uint256 i = 0; i < 100; ++i)
+            for (uint256 j = 0; i < 100; ++j) uint256 k = i + j; //~ ERROR: variable declarations are not allowed as the body of a loop
+
+        while (true) uint256 x = 4; //~ ERROR: variable declarations are not allowed as the body of a loop
+
+        do uint256 x = 4; while (true); //~ ERROR: variable declarations are not allowed as the body of a loop
+
+        unchecked {
+            {
+                {
+                    for (uint256 i = 0; i < 10; ++i) uint256 y = 0; //~ ERROR: variable declarations are not allowed as the body of a loop
+                }
+            }
+        }
+    }
+}

tests/ui/typeck/var_decl_as_loop_body.stderr

@@ -0,0 +1,42 @@
+error: variable declarations are not allowed as the body of a loop
+  --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC
+   |
+LL | ...   for (uint256 i = 0; i < 100; ++i) uint256 m_count = i + 1 * 2;
+   |                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: wrap the statement in a block (`{ ... }`)
+
+error: variable declarations are not allowed as the body of a loop
+  --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC
+   |
+LL |             for (uint256 j = 0; i < 100; ++j) uint256 k = i + j;
+   |                                               ^^^^^^^^^^^^^^^^^
+   |
+   = help: wrap the statement in a block (`{ ... }`)
+
+error: variable declarations are not allowed as the body of a loop
+  --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC
+   |
+LL |         while (true) uint256 x = 4;
+   |                      ^^^^^^^^^^^^^
+   |
+   = help: wrap the statement in a block (`{ ... }`)
+
+error: variable declarations are not allowed as the body of a loop
+  --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC
+   |
+LL |         do uint256 x = 4; while (true);
+   |            ^^^^^^^^^^^^^
+   |
+   = help: wrap the statement in a block (`{ ... }`)
+
+error: variable declarations are not allowed as the body of a loop
+  --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC
+   |
+LL | ...   for (uint256 i = 0; i < 10; ++i) uint256 y = 0;
+   |                                        ^^^^^^^^^^^^^
+   |
+   = help: wrap the statement in a block (`{ ... }`)
+
+error: aborting due to 5 previous errors
+