From c4feb1abaf33fe8db1c18c1d9d3020905a094e14 Mon Sep 17 00:00:00 2001
From: Dillon Kellar <39827493+d1ll0n@users.noreply.github.com>
Date: Wed, 9 Nov 2022 02:45:10 -0600
Subject: [PATCH] Fix incorrect block scoping rules for legacy code (#165)

What title said. Thanks @d1ll0n
---
 src/ast/definitions.ts                       | 39 +++++++++++++-------
 test/samples/solidity/resolving/block_04.sol | 22 +++++++++++
 test/samples/solidity/resolving/block_05.sol | 19 ++++++++++
 3 files changed, 67 insertions(+), 13 deletions(-)

diff --git a/src/ast/definitions.ts b/src/ast/definitions.ts
index ee57c187..6d57aa26 100644
--- a/src/ast/definitions.ts
+++ b/src/ast/definitions.ts
@@ -289,16 +289,28 @@ function* lookupInFunctionDefinition(
 /**
  * Lookup the definition corresponding to `name` in the `Block|UncheckedBlock` `scope`. Yield all matches.
  */
-function* lookupInBlock(name: string, scope: Block | UncheckedBlock): Iterable<AnyResolvable> {
-    for (const node of scope.children) {
-        if (!(node instanceof VariableDeclarationStatement)) {
-            continue;
-        }
-
-        for (const decl of node.vDeclarations) {
-            if (decl.name === name) {
-                yield decl;
-            }
+function* lookupInBlock(
+    name: string,
+    scope: Block | UncheckedBlock,
+    version: string
+): Iterable<AnyResolvable> {
+    let declarations: VariableDeclaration[];
+    if (version && lt(version, "0.5.0")) {
+        declarations = scope.getChildrenByType(VariableDeclaration);
+    } else {
+        declarations = scope.children
+            .filter((node) => node instanceof VariableDeclarationStatement)
+            .reduce(
+                (declarations: VariableDeclaration[], statement) => [
+                    ...declarations,
+                    ...(statement as VariableDeclarationStatement).vDeclarations
+                ],
+                []
+            );
+    }
+    for (const declaration of declarations) {
+        if (declaration.name === name) {
+            yield declaration;
         }
     }
 }
@@ -313,7 +325,8 @@ function lookupInScope(
     name: string,
     scope: ScopeNode,
     visitedUnits = new Set<SourceUnit>(),
-    ignoreVisiblity: boolean
+    ignoreVisiblity: boolean,
+    version = ""
 ): Set<AnyResolvable> {
     let results: Iterable<AnyResolvable>;
 
@@ -328,7 +341,7 @@ function lookupInScope(
     } else if (scope instanceof VariableDeclarationStatement) {
         results = scope.vDeclarations.filter((decl) => decl.name === name);
     } else if (scope instanceof Block || scope instanceof UncheckedBlock) {
-        results = lookupInBlock(name, scope);
+        results = lookupInBlock(name, scope, version);
     } else if (scope instanceof TryCatchClause) {
         results = scope.vParameters
             ? scope.vParameters.vParameters.filter((param) => param.name === name)
@@ -376,7 +389,7 @@ export function resolveAny(
             // If this is the first element (e.g. `A` in `A.B.C`), walk up the
             // stack of scopes starting from the current context, looking for `A`
             while (scope !== undefined) {
-                res = lookupInScope(element, scope, undefined, ignoreVisiblity);
+                res = lookupInScope(element, scope, undefined, ignoreVisiblity, version);
 
                 if (res.size > 0) {
                     // Sanity check - when multiple results are found, they must either be overloaded events
diff --git a/test/samples/solidity/resolving/block_04.sol b/test/samples/solidity/resolving/block_04.sol
index 3bd449f3..9d730e8c 100644
--- a/test/samples/solidity/resolving/block_04.sol
+++ b/test/samples/solidity/resolving/block_04.sol
@@ -8,5 +8,27 @@ contract Foo {
         foo memory m = foo(bar);
         
         uint bar = m.x;
+
+        uint m1 = k;
+        {uint k;}
+    }
+
+    uint256 x1;
+    uint256 x2;
+    uint256 x3;
+    uint256 x4;
+
+    function second() internal {
+        {
+            uint256 x1 = 1;
+            if (x1 > 0) {
+                uint256 x2 = 2;
+            }
+            {
+                uint256 x3 = 3;
+            }
+        }
+        for (uint256 x4; x4 < 10; x4++) {}
+        uint256 y = x1 + x2 + x3 + x4;
     }
 }
diff --git a/test/samples/solidity/resolving/block_05.sol b/test/samples/solidity/resolving/block_05.sol
index 564d5088..5c3d30e6 100644
--- a/test/samples/solidity/resolving/block_05.sol
+++ b/test/samples/solidity/resolving/block_05.sol
@@ -13,4 +13,23 @@ contract Foo {
         
         foo = 2;
     }
+
+    uint256 x1;
+    uint256 x2;
+    uint256 x3;
+    uint256 x4;
+
+    function second() internal {
+        {
+            uint256 x1 = 1;
+            if (x1 > 0) {
+                uint256 x2 = 2;
+            }
+            {
+                uint256 x3 = 3;
+            }
+        }
+        for (uint256 x4; x4 < 10; x4++) {}
+        uint256 y = x1 + x2 + x3 + x4;
+    }
 }