Merge pull request 'Properly check variable assignment' (#36) from feat/variable-is-defined into main

Reviewed-on: #36

midas/checker/python.py

@@ -107,7 +107,7 @@ class PythonTyper(
         tree: ast.Module = ast.parse(source, filename=path or "<unknown>")
         parser = PythonParser()
         stmts: list[p.Stmt] = parser.parse_module(tree)
-        resolver = Resolver()
+        resolver = Resolver(reporter)
         resolver.resolve(*stmts)
 
         self.env = self.global_env
@@ -601,6 +601,10 @@ class PythonTyper(
         pass
 
     def visit_for_stmt(self, stmt: p.ForStmt) -> None:
+        outer_env: Environment = self.env
+        inner_env: Environment = Environment(self.env)
+        self.env = inner_env
+
         item_type: Type = UnknownType()
         iterator_type: Type = self.type_of(stmt.iterator)
         if iterator_type != UnknownType():
@@ -614,8 +618,10 @@ class PythonTyper(
 
         self._assign(stmt.location, stmt.target, item_type)
         self.judge(stmt.target, item_type)
-        env: Environment = Environment(self.env)
+        body_returned: bool = self.process_block(stmt.body, inner_env)
-        body_returned: bool = self.process_block(stmt.body, env)
+
+        self.env = outer_env
+
         if body_returned:
             raise ReturnException()
 

midas/checker/resolver.py

@@ -1,4 +1,6 @@
 import midas.ast.python as p
+from midas.ast.location import Location
+from midas.checker.reporter import FileReporter
 
 
 class ResolverError(Exception): ...
@@ -11,9 +13,10 @@ class Resolver(p.Stmt.Visitor[None], p.Expr.Visitor[None]):
     scope is referred to when a variable is referenced
     """
 
-    def __init__(self):
+    def __init__(self, reporter: FileReporter):
         self.locals: dict[p.Expr, int] = {}
         self.scopes: list[dict[str, bool]] = [{}]
+        self.reporter: FileReporter = reporter
 
     def resolve(self, *objects: p.Stmt | p.Expr) -> None:
         """Resolve the given statements or expressions"""
@@ -25,28 +28,28 @@ class Resolver(p.Stmt.Visitor[None], p.Expr.Visitor[None]):
         """Begin a new scope inside the current one"""
         self.scopes.append({})
 
-    def end_scope(self):
+    def end_scope(self) -> dict[str, bool]:
-        """Close the current scope"""
+        """Close and return the current scope"""
-        self.scopes.pop()
+        return self.scopes.pop()
 
-    def declare(self, name: str) -> None:
+    def declare(self, location: Location, name: str) -> None:
         """Declare a variable in the current scope
 
         This method must be called *before* evaluating the variable initializer
 
         Args:
+            location (Location): the location where the name is declared
             name (str): the name of the variable
-
-        Raises:
-            ResolverError: if the variable has already been declared in the current scope
         """
         if len(self.scopes) == 0:
             return
         scope: dict[str, bool] = self.scopes[-1]
         if name in scope:
-            raise ResolverError(
+            self.reporter.error(
-                f"A variable with the name {name} is already declared in this scope"
+                location,
+                f"A variable with the name '{name}' is already declared in this scope",
             )
+        else:
             scope[name] = False
 
     def define(self, name: str) -> None:
@@ -77,7 +80,7 @@ class Resolver(p.Stmt.Visitor[None], p.Expr.Visitor[None]):
                 self.locals[expr] = i
                 return
 
-    def is_defined(self, name: str) -> bool:
+    def is_declared(self, name: str) -> bool:
         """Check whether the given variable is defined in any scope
 
         Args:
@@ -106,7 +109,7 @@ class Resolver(p.Stmt.Visitor[None], p.Expr.Visitor[None]):
                 self.resolve(param.default)
 
         for param in function.params.all:
-            self.declare(param.name)
+            self.declare(function.location, param.name)
             self.define(param.name)
         self.resolve(*function.body)
         self.end_scope()
@@ -116,14 +119,12 @@ class Resolver(p.Stmt.Visitor[None], p.Expr.Visitor[None]):
 
     def visit_function(self, stmt: p.Function) -> None:
         # Declare before resolving body to allow recursion
-        self.declare(stmt.name)
+        self.declare(stmt.location, stmt.name)
         self.define(stmt.name)
         self.resolve_function(stmt)
 
     def visit_type_assign(self, stmt: p.TypeAssign) -> None:
-        self.declare(stmt.name)
+        self.declare(stmt.location, stmt.name)
-        # NOTE: resolve type here?
-        self.define(stmt.name)
 
     def visit_assign_stmt(self, stmt: p.AssignStmt) -> None:
         self.resolve(stmt.value)
@@ -133,8 +134,8 @@ class Resolver(p.Stmt.Visitor[None], p.Expr.Visitor[None]):
     def _visit_assign(self, target: p.Expr):
         match target:
             case p.VariableExpr(name=name):
-                if not self.is_defined(name):
+                if not self.is_declared(name):
-                    self.declare(name)
+                    self.declare(target.location, name)
                 self.define(name)
                 target.accept(self)
 
@@ -145,7 +146,9 @@ class Resolver(p.Stmt.Visitor[None], p.Expr.Visitor[None]):
                 target.accept(self)
 
             case _:
-                raise Exception(f"Unsupported assignment to {target}")
+                self.reporter.error(
+                    target.location, f"Unsupported assignment to {target}"
+                )
 
     def visit_return_stmt(self, stmt: p.ReturnStmt) -> None:
         if stmt.value is not None:
@@ -162,20 +165,25 @@ class Resolver(p.Stmt.Visitor[None], p.Expr.Visitor[None]):
         # Body
         self.begin_scope()
         self.resolve(*stmt.body)
-        self.end_scope()
+        body: dict[str, bool] = self.end_scope()
 
         # Else
         self.begin_scope()
         self.resolve(*stmt.orelse)
-        self.end_scope()
+        else_: dict[str, bool] = self.end_scope()
+
+        # Define variables in this scope if it was defined in both body and else blocks
+        for name, is_defined in body.items():
+            if is_defined and else_.get(name, False):
+                self.define(name)
 
     def visit_pass(self, stmt: p.Pass) -> None:
         pass
 
     def visit_for_stmt(self, stmt: p.ForStmt) -> None:
+        self.begin_scope()
         self.resolve(stmt.iterator)
         self._visit_assign(stmt.target)
-        self.begin_scope()
         self.resolve(*stmt.body)
         self.end_scope()
 
@@ -188,7 +196,7 @@ class Resolver(p.Stmt.Visitor[None], p.Expr.Visitor[None]):
     def _resolve_imports(self, imports: list[p.ImportAlias]) -> None:
         for import_ in imports:
             name: str = import_.imported_name
-            self.declare(name)
+            self.declare(import_.location, name)
             self.define(name)
 
     def visit_raw_stmt(self, stmt: p.RawStmt) -> None:
@@ -220,8 +228,9 @@ class Resolver(p.Stmt.Visitor[None], p.Expr.Visitor[None]):
 
     def visit_variable_expr(self, expr: p.VariableExpr) -> None:
         if len(self.scopes) != 0 and self.scopes[-1].get(expr.name) is False:
-            raise ResolverError(
+            self.reporter.error(
-                f"Cannot use local variable '{expr.name}' in its own initializer"
+                expr.location,
+                f"Variable '{expr.name}' is declared but may not be defined",
             )  # aka. UnboundLocalError
         self.resolve_local(expr, expr.name)
 

tests/cases/checker/07_variance.py

@@ -11,16 +11,16 @@ from _ import (
     Unused,
 )
 
-unused: Unused
+unused: Unused = object()
-covariant: Covariant
+covariant: Covariant = object()
-contravariant: Contravariant
+contravariant: Contravariant = object()
-invariant: Invariant
+invariant: Invariant = object()
-coco: Coco
+coco: Coco = object()
-cocontra: Cocontra
+cocontra: Cocontra = object()
-contraco: Contraco
+contraco: Contraco = object()
-contracontra: Contracontra
+contracontra: Contracontra = object()
-t1: T1
+t1: T1 = object()
-t2: T2
+t2: T2 = object()
 
 # Dummy print to prudce judgements for the expressions
 print(
@@ -36,17 +36,17 @@ print(
     t2,
 )
 
-cov1: Covariant[float]
+cov1: Covariant[float] = object()
-cov2: Covariant[int]
+cov2: Covariant[int] = object()
 cov1 = cov2  # Ok because int <: float => Covariant[int] <: Covariant[float]
 cov2 = cov1  # Invalid
 
-contra1: Contravariant[float]
+contra1: Contravariant[float] = object()
-contra2: Contravariant[int]
+contra2: Contravariant[int] = object()
 contra1 = contra2  # Invalid
 contra2 = contra1  # Ok because int <: float => Covariant[float] <: Covariant[int]
 
-inv1: Invariant[float]
+inv1: Invariant[float] = object()
-inv2: Invariant[int]
+inv2: Invariant[int] = object()
 inv1 = inv2  # Invalid
 inv2 = inv1  # Invalid

tests/cases/checker/09_frame_ops.py

@@ -1,8 +1,9 @@
 # type: ignore
 # ruff: disable [F821]
+import pandas as pd
 
-df1: Frame[i:int, a:int, b:float]
+df1 = cast(Frame[i:int, a:int, b:float], pd.DataFrame())
-df2: Frame[i:int, a:int, b:float]
+df2 = cast(Frame[i:int, a:int, b:float], pd.DataFrame())
 
 _: Any
 

tests/cases/checker/10_variable_defined.py

@@ -0,0 +1,45 @@
+# type: ignore
+# ruff: disable[F821]
+
+_: Any
+
+_ = undeclared
+
+declared: int
+_ = declared
+
+half_defined1: int
+half_defined2: int
+if False:
+    half_defined1 = 0
+else:
+    half_defined2 = 1
+_ = half_defined1
+_ = half_defined2
+
+fully_defined: int
+if False:
+    fully_defined = 0
+else:
+    fully_defined = 1
+_ = fully_defined
+
+defined: int = 0
+_ = defined
+
+no_annotation = 0
+_ = no_annotation
+
+self_ref1 = self_ref1
+self_ref2: int = self_ref2
+
+
+def fact(n: int) -> int:
+    if n <= 1:
+        return 1
+    return n * fact(n - 1)
+
+
+for i in [1, 2, 3]:
+    _ = i
+_ = i

tests/cases/checker/10_variable_defined.py.ref.json

@@ -0,0 +1,651 @@
+{
+  "diagnostics": [
+    {
+      "type": "Error",
+      "location": {
+        "start": [
+          9,
+          4
+        ],
+        "end": [
+          9,
+          12
+        ]
+      },
+      "message": "Variable 'declared' is declared but may not be defined"
+    },
+    {
+      "type": "Error",
+      "location": {
+        "start": [
+          17,
+          4
+        ],
+        "end": [
+          17,
+          17
+        ]
+      },
+      "message": "Variable 'half_defined1' is declared but may not be defined"
+    },
+    {
+      "type": "Error",
+      "location": {
+        "start": [
+          18,
+          4
+        ],
+        "end": [
+          18,
+          17
+        ]
+      },
+      "message": "Variable 'half_defined2' is declared but may not be defined"
+    },
+    {
+      "type": "Error",
+      "location": {
+        "start": [
+          34,
+          17
+        ],
+        "end": [
+          34,
+          26
+        ]
+      },
+      "message": "Variable 'self_ref2' is declared but may not be defined"
+    },
+    {
+      "type": "Warning",
+      "location": {
+        "start": [
+          6,
+          4
+        ],
+        "end": [
+          6,
+          14
+        ]
+      },
+      "message": "Unknown variable"
+    },
+    {
+      "type": "Warning",
+      "location": {
+        "start": [
+          33,
+          12
+        ],
+        "end": [
+          33,
+          21
+        ]
+      },
+      "message": "Unknown variable"
+    },
+    {
+      "type": "Warning",
+      "location": {
+        "start": [
+          45,
+          4
+        ],
+        "end": [
+          45,
+          5
+        ]
+      },
+      "message": "Unknown variable"
+    }
+  ],
+  "judgments": [
+    {
+      "location": {
+        "from": "L6:4",
+        "to": "L6:14"
+      },
+      "expr": {
+        "_type": "VariableExpr",
+        "name": "undeclared"
+      },
+      "type": {}
+    },
+    {
+      "location": {
+        "from": "L9:4",
+        "to": "L9:12"
+      },
+      "expr": {
+        "_type": "VariableExpr",
+        "name": "declared"
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L13:3",
+        "to": "L13:8"
+      },
+      "expr": {
+        "_type": "LiteralExpr",
+        "value": false
+      },
+      "type": {
+        "name": "bool"
+      }
+    },
+    {
+      "location": {
+        "from": "L14:20",
+        "to": "L14:21"
+      },
+      "expr": {
+        "_type": "LiteralExpr",
+        "value": 0
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L16:20",
+        "to": "L16:21"
+      },
+      "expr": {
+        "_type": "LiteralExpr",
+        "value": 1
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L17:4",
+        "to": "L17:17"
+      },
+      "expr": {
+        "_type": "VariableExpr",
+        "name": "half_defined1"
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L18:4",
+        "to": "L18:17"
+      },
+      "expr": {
+        "_type": "VariableExpr",
+        "name": "half_defined2"
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L21:3",
+        "to": "L21:8"
+      },
+      "expr": {
+        "_type": "LiteralExpr",
+        "value": false
+      },
+      "type": {
+        "name": "bool"
+      }
+    },
+    {
+      "location": {
+        "from": "L22:20",
+        "to": "L22:21"
+      },
+      "expr": {
+        "_type": "LiteralExpr",
+        "value": 0
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L24:20",
+        "to": "L24:21"
+      },
+      "expr": {
+        "_type": "LiteralExpr",
+        "value": 1
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L25:4",
+        "to": "L25:17"
+      },
+      "expr": {
+        "_type": "VariableExpr",
+        "name": "fully_defined"
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L27:15",
+        "to": "L27:16"
+      },
+      "expr": {
+        "_type": "LiteralExpr",
+        "value": 0
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L28:4",
+        "to": "L28:11"
+      },
+      "expr": {
+        "_type": "VariableExpr",
+        "name": "defined"
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L30:16",
+        "to": "L30:17"
+      },
+      "expr": {
+        "_type": "LiteralExpr",
+        "value": 0
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L31:4",
+        "to": "L31:17"
+      },
+      "expr": {
+        "_type": "VariableExpr",
+        "name": "no_annotation"
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L33:12",
+        "to": "L33:21"
+      },
+      "expr": {
+        "_type": "VariableExpr",
+        "name": "self_ref1"
+      },
+      "type": {}
+    },
+    {
+      "location": {
+        "from": "L34:17",
+        "to": "L34:26"
+      },
+      "expr": {
+        "_type": "VariableExpr",
+        "name": "self_ref2"
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L38:7",
+        "to": "L38:8"
+      },
+      "expr": {
+        "_type": "VariableExpr",
+        "name": "n"
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L38:12",
+        "to": "L38:13"
+      },
+      "expr": {
+        "_type": "LiteralExpr",
+        "value": 1
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L38:7",
+        "to": "L38:13"
+      },
+      "expr": {
+        "_type": "CompareExpr",
+        "left": {
+          "_type": "VariableExpr",
+          "name": "n"
+        },
+        "operator": "<=",
+        "right": {
+          "_type": "LiteralExpr",
+          "value": 1
+        }
+      },
+      "type": {
+        "name": "bool"
+      }
+    },
+    {
+      "location": {
+        "from": "L39:15",
+        "to": "L39:16"
+      },
+      "expr": {
+        "_type": "LiteralExpr",
+        "value": 1
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L40:11",
+        "to": "L40:12"
+      },
+      "expr": {
+        "_type": "VariableExpr",
+        "name": "n"
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L40:20",
+        "to": "L40:21"
+      },
+      "expr": {
+        "_type": "VariableExpr",
+        "name": "n"
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L40:24",
+        "to": "L40:25"
+      },
+      "expr": {
+        "_type": "LiteralExpr",
+        "value": 1
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L40:20",
+        "to": "L40:25"
+      },
+      "expr": {
+        "_type": "BinaryExpr",
+        "left": {
+          "_type": "VariableExpr",
+          "name": "n"
+        },
+        "operator": "-",
+        "right": {
+          "_type": "LiteralExpr",
+          "value": 1
+        }
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L40:15",
+        "to": "L40:19"
+      },
+      "expr": {
+        "_type": "VariableExpr",
+        "name": "fact"
+      },
+      "type": {
+        "params": {
+          "pos": [],
+          "mixed": [
+            {
+              "pos": 0,
+              "name": "n",
+              "type": {
+                "name": "int"
+              },
+              "required": true,
+              "unsupported": false
+            }
+          ],
+          "kw": []
+        },
+        "returns": {
+          "name": "int"
+        }
+      }
+    },
+    {
+      "location": {
+        "from": "L40:15",
+        "to": "L40:26"
+      },
+      "expr": {
+        "_type": "CallExpr",
+        "callee": {
+          "_type": "VariableExpr",
+          "name": "fact"
+        },
+        "arguments": [
+          {
+            "_type": "BinaryExpr",
+            "left": {
+              "_type": "VariableExpr",
+              "name": "n"
+            },
+            "operator": "-",
+            "right": {
+              "_type": "LiteralExpr",
+              "value": 1
+            }
+          }
+        ],
+        "keywords": {}
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L40:11",
+        "to": "L40:26"
+      },
+      "expr": {
+        "_type": "BinaryExpr",
+        "left": {
+          "_type": "VariableExpr",
+          "name": "n"
+        },
+        "operator": "*",
+        "right": {
+          "_type": "CallExpr",
+          "callee": {
+            "_type": "VariableExpr",
+            "name": "fact"
+          },
+          "arguments": [
+            {
+              "_type": "BinaryExpr",
+              "left": {
+                "_type": "VariableExpr",
+                "name": "n"
+              },
+              "operator": "-",
+              "right": {
+                "_type": "LiteralExpr",
+                "value": 1
+              }
+            }
+          ],
+          "keywords": {}
+        }
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L43:10",
+        "to": "L43:11"
+      },
+      "expr": {
+        "_type": "LiteralExpr",
+        "value": 1
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L43:13",
+        "to": "L43:14"
+      },
+      "expr": {
+        "_type": "LiteralExpr",
+        "value": 2
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L43:16",
+        "to": "L43:17"
+      },
+      "expr": {
+        "_type": "LiteralExpr",
+        "value": 3
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L43:9",
+        "to": "L43:18"
+      },
+      "expr": {
+        "_type": "ListExpr",
+        "items": [
+          {
+            "_type": "LiteralExpr",
+            "value": 1
+          },
+          {
+            "_type": "LiteralExpr",
+            "value": 2
+          },
+          {
+            "_type": "LiteralExpr",
+            "value": 3
+          }
+        ]
+      },
+      "type": {
+        "name": "list",
+        "args": [
+          {
+            "name": "int"
+          }
+        ],
+        "body": {
+          "name": "list"
+        }
+      }
+    },
+    {
+      "location": {
+        "from": "L43:4",
+        "to": "L43:5"
+      },
+      "expr": {
+        "_type": "VariableExpr",
+        "name": "i"
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L44:8",
+        "to": "L44:9"
+      },
+      "expr": {
+        "_type": "VariableExpr",
+        "name": "i"
+      },
+      "type": {
+        "name": "int"
+      }
+    },
+    {
+      "location": {
+        "from": "L45:4",
+        "to": "L45:5"
+      },
+      "expr": {
+        "_type": "VariableExpr",
+        "name": "i"
+      },
+      "type": {}
+    }
+  ]
+}