more advanced type detection for null checks (#64 wip)

src/kiss/FieldForms.hx

@@ -101,10 +101,13 @@ class FieldForms {
             var name = Helpers.varName(formName, args[0]);
             var access = fieldAccess(formName, name, args[0]);
 
+            var type = Helpers.explicitType(args[0]);
+            k.typeHints.push({name: name, type: type});
+
             ({
                 name: name,
                 access: access,
-                kind: FVar(Helpers.explicitType(args[0]), if (args.length > 1) k.convert(args[1]) else null),
+                kind: FVar(type, if (args.length > 1) k.convert(args[1]) else null),
                 pos: wholeExp.macroPos()
             } : Field);
         }

src/kiss/Helpers.hx

@@ -173,6 +173,26 @@ class Helpers {
             };
         }
 
+        var args = switch (argList.def) {
+            case ListExp(funcArgs):
+                funcArgs.map(makeFuncArg);
+            case CallExp(_, _):
+                throw KissError.fromExp(argList, 'expected an argument list. Change the parens () to brackets []');
+            default:
+                throw KissError.fromExp(argList, 'expected an argument list');
+        };
+
+        var vars = [for (arg in args) {
+            {
+                name: arg.name,
+                type: arg.type
+            }
+        }];
+
+        for (v in vars) {
+            k.typeHints.push(v);
+        }
+
         var expr = if (body.length == 0) {
             EReturn(null).withMacroPosOf(if (name != null) name else argList);
         } else {
@@ -185,20 +205,17 @@ class Helpers {
             };
         }
 
+        for (v in vars) {
+            k.typeHints.remove(v);
+        }
+
         // To make function args immutable by default, we would use (let...) instead of (begin...)
         // to make the body expression.
         // But setting null arguments to default values is so common, and arguments are not settable references,
         // so function args are not immutable.
         return {
             ret: if (name != null) Helpers.explicitType(name) else null,
-            args: switch (argList.def) {
-                case ListExp(funcArgs):
-                    funcArgs.map(makeFuncArg);
-                case CallExp(_, _):
-                    throw KissError.fromExp(argList, 'expected an argument list. Change the parens () to brackets []');
-                default:
-                    throw KissError.fromExp(argList, 'expected an argument list');
-            },
+            args: args,
             expr: expr
         }
     }

src/kiss/Kiss.hx

@@ -57,7 +57,8 @@ typedef KissState = {
     hscript:Bool,
     macroVars:Map<String, Dynamic>,
     collectedBlocks:Map<String, Array<ReaderExp>>,
-    inStaticFunction:Bool
+    inStaticFunction:Bool,
+    typeHints:Array<Var>
 };
 #end
 
@@ -144,7 +145,8 @@ class Kiss {
             hscript: false,
             macroVars: new Map(),
             collectedBlocks: new Map(),
-            inStaticFunction: false
+            inStaticFunction: false,
+            typeHints: [],
         };
 
         k.doc = (form:String, minArgs:Null<Int>, maxArgs:Null<Int>, expectedForm = "", doc = "") -> {

src/kiss/SpecialForms.hx

@@ -203,10 +203,20 @@ class SpecialForms {
                 throw KissError.fromArgs(args, '(let....) expression needs a body');
             }
 
-            EBlock([
+            for (v in varDefs) {
+                k.typeHints.push(v);
+            }
+
+            var block = EBlock([
                 EVars(varDefs).withMacroPosOf(wholeExp),
                 EBlock(body.map(k.convert)).withMacroPosOf(wholeExp)
             ]).withMacroPosOf(wholeExp);
+
+            for (v in varDefs) {
+                k.typeHints.remove(v);
+            }
+
+            block;
         };
 
         k.doc("lambda", 2, null, "(lambda [<argsNames...>] <body...>)");
@@ -313,11 +323,11 @@ class SpecialForms {
             // On C#, C++, and HashLink, value types (specifically Float and Int) cannot be null, so they cannot be compared with null.
             // Therefore a null case doesn't need to be added--and will cause a compile failure if it is.
             var canCompareNull = if (Context.defined('cs') || Context.defined('cpp') || Context.defined('hl')) {
-                // TODO can locals from let bindings and localVar be gathered and passed to this? Would be difficult and maybe require a separate stack in KissState for each (begin) conversion
-                switch (exp.typeof()) {
-                    case Success(TAbstract(ref, [])) if (["Int", "Float"].indexOf(ref.get().name) != -1):
+                switch (exp.typeof(k.typeHints)) {
+                    case Success(TAbstract(ref, [])) if (["Int", "Float", "Bool"].indexOf(ref.get().name) != -1):
                         false;
                     case Failure(_):
+                        KissError.warnFromExp(args[0], "Can't detect whether expression can be null-checked");
                         false;
                     default: true;
                 }

src/test/cases/BasicTestCase.kiss

@@ -494,6 +494,16 @@ From:[(assert false (+ \"false \" \"should \" \"have \" \"been \" \"true\"))]" m
     (-= num 5 6)
     (Assert.equals -10 num))
 
+// these declarations have to come before the ifLets to get the type detection correct for issue 64
+(var :String realStringVar "something")
+(var :String nullStringVar null)
+
+(function issue64withArgs [:String realStringArg :String nullStringArg]
+    (unlessLet [something realStringArg]
+            (Assert.fail))
+        (whenLet [something nullStringArg]
+            (Assert.fail)))
+
 (function _testPatternLets []
     (let [some5 (Some 5)
             some6 (Some 6)
@@ -519,7 +529,22 @@ From:[(assert false (+ \"false \" \"should \" \"have \" \"been \" \"true\"))]" m
             (Assert.equals 2 v))
             
         (assertThrows (assertLet [(Some thing) none] thing))
-        (Assert.equals 5 (assertLet [(Some thing) some5] thing))))
+        (Assert.equals 5 (assertLet [(Some thing) some5] thing)))
+        
+        // Issue #64 regression tests:
+        (whenLet [something null]
+            (Assert.fail))
+        (let [:String realString "something"
+                :String nullString null]
+            (unlessLet [something realString]
+                (Assert.fail))
+            (whenLet [something nullString]
+                (Assert.fail)))
+        (unlessLet [something realStringVar]
+            (Assert.fail))
+        (whenLet [something nullStringVar]
+            (Assert.fail))
+        (issue64withArgs "something" null))
 
 (function _testRawString []
     (Assert.equals "\\"  #"\"#)