(interpreter) Fix ICE for ClrType unexpectedly null (#279)

Closes #239

src/Perlang.Interpreter/Typing/TypeResolver.cs

@@ -335,18 +335,20 @@ public override VoidObject VisitGetExpr(Expr.Get expr)
             // The "== null" part is kind of sneaky. We run into that scenario whenever method calls are chained.
             // It still feels somewhat better than allowing any kind of wild binding to pass through at this
             // point.
-            if (binding is ClassBinding ||
-                binding is VariableBinding ||
-                binding is NativeClassBinding ||
-                binding is NativeObjectBinding ||
-                binding == null)
+            if (binding is ClassBinding or
+                VariableBinding or
+                NativeClassBinding or
+                NativeObjectBinding or
+                null)
             {
                 Type? type = expr.Object.TypeReference.ClrType;
 
                 if (type == null)
                 {
-                    // TODO: Use a better exception type here
-                    throw new NameResolutionTypeValidationError(expr.Name, $"Internal compiler error: ClrType for '{expr.Object}' was unexpectedly null.");
+                    // This is a legitimate code path in cases where a method call is attempted on an unknown type, like
+                    // `var f: Foo; f.some_method()`. In this case, the ClrType will be null for the given `expr.Object`
+                    // type reference.
+                    return VoidObject.Void;
                 }
 
                 // Perlang uses snake_case by convention, but we still want to be able to call regular PascalCased

src/Perlang.Interpreter/Typing/TypesResolvedValidator.cs

@@ -1,3 +1,4 @@
+#nullable enable
 #pragma warning disable S907
 #pragma warning disable S3218
 #pragma warning disable SA1118
@@ -55,7 +56,29 @@ private void VisitCallExprForGetCallee(Expr.Call call, Expr.Get get)
         {
             string methodName = get.Name.Lexeme;
 
-            if (get.Methods.Length == 1)
+            if (get.Methods.Length == 0)
+            {
+                if (!get.Object.TypeReference.IsResolved)
+                {
+                    // This is a bit of an oddball, but... We get here when an attempt is made to call a method on some
+                    // undefined type. (`Foo.do_stuff`)
+                    //
+                    // Now, this is a compile-time error, but the problem is that it's handled by this class itself;
+                    // encountering this error will not abort the tree traversal so we must avoid breaking it.
+                }
+                else
+                {
+                    // This is even more odd, but we must ensure that we have well-defined semantics in the weird case
+                    // where this would happen.
+                    TypeValidationErrorCallback(new TypeValidationError(
+                        call.Paren,
+                        $"Internal compiler error: no methods with name '{methodName}' could be found. This is a critical " +
+                        $"error that should have aborted the compilation before the {nameof(TypesResolvedValidator)} " +
+                        "validation is started. "
+                    ));
+                }
+            }
+            else if (get.Methods.Length == 1)
             {
                 MethodInfo method = get.Methods.Single();
                 var parameters = method.GetParameters();
@@ -90,7 +113,7 @@ private void VisitCallExprForGetCallee(Expr.Call call, Expr.Get get)
                     {
                         // Very likely refers to a native method, where parameter names are not available at this point.
                         TypeValidationErrorCallback(new TypeValidationError(
-                            argument.TypeReference.TypeSpecifier,
+                            argument.TypeReference.TypeSpecifier!,
                             $"Cannot pass {argument.TypeReference.ClrType.ToTypeKeyword()} argument as {parameter.ParameterType.ToTypeKeyword()} parameter to {methodName}()"));
                     }
                 }
@@ -210,14 +233,14 @@ private void VisitCallExprForOtherCallee(Expr.Call expr)
                     if (parameter.Name != null)
                     {
                         TypeValidationErrorCallback(new TypeValidationError(
-                            argument.TypeReference.TypeSpecifier,
+                            argument.TypeReference.TypeSpecifier!,
                             $"Cannot pass {argument.TypeReference.ClrType.ToTypeKeyword()} argument as parameter '{parameter.Name.Lexeme}: {parameter.TypeReference.ClrType.ToTypeKeyword()}' to {functionName}()"));
                     }
                     else
                     {
                         // Very likely refers to a native method, where parameter names are not available at this point.
                         TypeValidationErrorCallback(new TypeValidationError(
-                            argument.TypeReference.TypeSpecifier,
+                            argument.TypeReference.TypeSpecifier!,
                             $"Cannot pass {argument.TypeReference.ClrType.ToTypeKeyword()} argument as {parameter.TypeReference.ClrType.ToTypeKeyword()} parameter to {functionName}()"));
                     }
                 }
@@ -246,8 +269,8 @@ public override VoidObject VisitFunctionStmt(Stmt.Function stmt)
                 else
                 {
                     TypeValidationErrorCallback(new TypeValidationError(
-                        stmt.ReturnTypeReference.TypeSpecifier,
-                        $"Type not found: {stmt.ReturnTypeReference.TypeSpecifier.Lexeme}"
+                        stmt.ReturnTypeReference.TypeSpecifier!,
+                        $"Type not found: {stmt.ReturnTypeReference.TypeSpecifier!.Lexeme}"
                     ));
                 }
             }
@@ -328,7 +351,7 @@ public override VoidObject VisitVarStmt(Stmt.Var stmt)
                 return VoidObject.Void;
             }
 
-            if (stmt.TypeReference.IsResolved)
+            if (stmt.TypeReference!.IsResolved)
             {
                 if (stmt.Initializer != null)
                 {
@@ -355,28 +378,19 @@ public override VoidObject VisitVarStmt(Stmt.Var stmt)
                     }
                 }
             }
-            else if (stmt.Initializer == null)
+            else if (!stmt.TypeReference.ExplicitTypeSpecified && stmt.Initializer == null)
             {
                 TypeValidationErrorCallback(new TypeValidationError(
-                    null,
+                    stmt.Name,
                     $"Type inference for variable '{stmt.Name.Lexeme}' cannot be performed when initializer is not specified. " +
                     "Either provide an initializer, or specify the type explicitly."
                 ));
             }
-            else if (!stmt.TypeReference.ExplicitTypeSpecified)
-            {
-                // FIXME: Let's see if we'll ever go into this branch. If we don't have a test that reproduces
-                // this once all tests are green, we should consider wiping it from the face off the earth.
-                TypeValidationErrorCallback(new TypeValidationError(
-                    null,
-                    $"Failed to infer type for variable '{stmt.Name.Lexeme}' from usage. Try specifying the type explicitly."
-                ));
-            }
             else
             {
                 TypeValidationErrorCallback(new TypeValidationError(
-                    stmt.TypeReference.TypeSpecifier,
-                    $"Type not found: {stmt.TypeReference.TypeSpecifier.Lexeme}"
+                    stmt.TypeReference.TypeSpecifier!,
+                    $"Type not found: {stmt.TypeReference.TypeSpecifier!.Lexeme}"
                 ));
             }
 

src/Perlang.Tests.Integration/Typing/TypingTests.cs

@@ -208,7 +208,7 @@ public void var_declaration_detects_reassignment_of_incoercible_type()
             var exception = result.Errors.FirstOrDefault();
 
             Assert.Single(result.Errors);
-            Assert.Matches("Cannot assign string to int variable", exception.Message);
+            Assert.Matches("Cannot assign string to int variable", exception!.Message);
         }
 
         [Fact]
@@ -259,7 +259,7 @@ public void var_declaration_detects_reassignment_to_null_for_value_type()
             var exception = result.Errors.FirstOrDefault();
 
             Assert.Single(result.Errors);
-            Assert.Matches("Cannot assign null to int variable", exception.Message);
+            Assert.Matches("Cannot assign null to int variable", exception!.Message);
         }
 
         [Fact]
@@ -293,7 +293,7 @@ fun foo(s: String): void {
             var exception = result.Errors.FirstOrDefault();
 
             Assert.Single(result.Errors);
-            Assert.Matches("Cannot pass int argument as parameter 's: string'", exception.Message);
+            Assert.Matches("Cannot pass int argument as parameter 's: string'", exception!.Message);
         }
 
         [Fact]
@@ -347,7 +347,7 @@ fun foo(i: int): void {
             var exception = result.Errors.FirstOrDefault();
 
             Assert.Single(result.Errors);
-            Assert.Matches("Cannot pass null argument as parameter 'i: int'", exception.Message);
+            Assert.Matches("Cannot pass null argument as parameter 'i: int'", exception!.Message);
         }
 
         [Fact]
@@ -396,7 +396,7 @@ fun foo(): SomeUnknownType {
             var exception = result.Errors.FirstOrDefault();
 
             Assert.Single(result.Errors);
-            Assert.Matches("Type not found: SomeUnknownType", exception.Message);
+            Assert.Matches("Type not found: SomeUnknownType", exception!.Message);
         }
 
         [Fact]
@@ -416,7 +416,38 @@ fun foo(): string {
             var exception = result.Errors.FirstOrDefault();
 
             Assert.Single(result.Errors);
-            Assert.Equal("Cannot assign string to int variable", exception.Message);
+            Assert.Equal("Cannot assign string to int variable", exception!.Message);
+        }
+
+        [Fact]
+        public void var_of_non_existent_type_with_initializer_emits_expected_error()
+        {
+            // TODO: I have a feeling that this somehow incorrectly infers the type from the 1 integer to some parts of
+            // the parsed syntax tree (since NonexistentType does not exist). Investigate this someday, since it could
+            // lead to subtle, very hard-to-track bugs.
+            string source = @"
+                var i: NonexistentType = 1; i++; print i.get_type();
+            ";
+
+            var result = EvalWithValidationErrorCatch(source);
+            var exception = result.Errors.FirstOrDefault();
+
+            Assert.Single(result.Errors);
+            Assert.Equal("Type not found: NonexistentType", exception!.Message);
+        }
+
+        [Fact]
+        public void var_of_non_existent_type_without_initializer_emits_expected_error()
+        {
+            string source = @"
+                var f: Foo; f.do_stuff();
+            ";
+
+            var result = EvalWithValidationErrorCatch(source);
+            var exception = result.Errors.FirstOrDefault();
+
+            Assert.Single(result.Errors);
+            Assert.Equal("Type not found: Foo", exception!.Message);
         }
     }
 }

src/Perlang.Tests/Interpreter/Typing/TypeResolverTest.cs

@@ -86,7 +86,8 @@ public void Resolve_implicitly_typed_var_initialized_from_long_var_has_expected_
             Assert.True(resolver.Globals.ContainsKey("l"));
             Assert.True(resolver.Globals.ContainsKey("m"));
 
-            // Both of these are expected to have been resolved this way; the first because of the explicit type specifier and the second because of type inference.
+            // Both of these are expected to have been resolved this way; the first because of the explicit type
+            // specifier and the second because of type inference.
             Assert.Equal(typeof(Int64), ((Stmt.Var)firstStmt).TypeReference.ClrType);
             Assert.Equal(typeof(Int64), ((Stmt.Var)secondStmt).TypeReference.ClrType);
         }