Raise on a field of a null object, interpreted and compiled

docs/advanced.md

@@ -556,6 +556,7 @@ blacklists the interpreter's own machinery and every script using `Std` dies wit
 | `interpClass` | `Interp` | the interpreter subclass to instantiate |
 | `typedMode` | on (`-D hxscript_dynamic` flips it) | enforce declared types at runtime |
 | `strictAccess` | `false` | enforce `private` on script-declared members. Not the only gate: the check runs when this **or** `typedMode` is set, and `typedMode` defaults on, so `private` is enforced out of the box. Only explicit `private` counts; unmarked members stay public, unlike Haxe, because existing scripts rely on it. `@:privateAccess` waives it at the call site |
+| `strictNullAccess` | `false` | raise `Null access to field f` on a field of a null object, as Haxe does, instead of reading null. Off because scripts rely on that null, often as a check one level too deep (`a.b.c != null` with `a.b` null). On, compiled code also raises on a write the interpreter already refused, where it wrote nowhere. Not covered by HashLink's compiled code yet |
 | `preprocessorValues` | host defines + `hxscript` | what `#if` in a script sees |
 | `typeProxy` | `Std`, `Type`, `Reflect`, and `Math` on hl and python | swap a type name for a stand-in class |
 | `globalVariables` | `null`/`true`/`false`, `Int`/`Float`/`Bool` tokens | values in every interpreter |

docs/embedding.md

@@ -239,6 +239,7 @@ project, and nothing to do for a 3D one, which is why it is off by default rathe
 | `globalStatics` | bare names answered by a host static, as `owner.path::field` |
 | `blacklist` | types scripts may not touch, by `ByType`, `ByModule` or `ByPackage` |
 | `strictAccess` | enforce `private` on script-declared members |
+| `strictNullAccess` | raise on a field of a null object instead of reading null |
 | `typedMode` | runtime type enforcement, on unless `-D hxscript_dynamic` |
 | `callShims` | a closure standing in for a member with no runtime form, keyed `Owner.method` |
 | `preprocessorValues` | values a script's `#if` can test |

src/hxscript/Config.hx

@@ -22,6 +22,14 @@ class Config {
 	 */
 	public static var strictAccess:Bool = false;
 
+	/**
+	 * Raises on reading a field of a null object, `a.b` with `a` null, as Haxe does, instead of
+	 * answering null. Off by default because scripts rely on the null, typically as a check one level
+	 * too deep: `if (level.player.sprite != null)` while `level.player` is still null. Compiled code
+	 * also stops ignoring a write to a field of a null object, which the interpreter always refused.
+	 */
+	public static var strictNullAccess:Bool = false;
+
 	/**
 	 * Whether cppia declares a `Bool` field with its real type, which needs the hxcpp fix in
 	 * `HXCPP-ISSUES.md`. `-D hxscript_cppia_bool_compat` turns it off for a stock hxcpp, where a

src/hxscript/cppia/Backend.hx

@@ -43,6 +43,9 @@ class Backend {
 	/** Names the static-extension helper for the same reason. */
 	@:keep static var extensions:Class<Dynamic> = hxscript.runtime.Using;
 
+	/** Names the host field reader and writer for the same reason. */
+	@:keep static var fields:Class<Dynamic> = hxscript.runtime.Fields;
+
 	/** Names the runtime raiser for the same reason. */
 	@:keep static var raiser:Class<Dynamic> = hxscript.runtime.Raise;
 

src/hxscript/cppia/Emitter.hx

@@ -1719,8 +1719,8 @@ class Emitter {
 				w.int(3);
 				w.pos(line);
 				w.token('FSTATIC');
-				w.type('Reflect');
-				w.str('setProperty');
+				w.type('hxscript.runtime.Fields');
+				w.str('set');
 				expr(target);
 				w.pos(line);
 				w.token('s');
@@ -2372,7 +2372,7 @@ class Emitter {
 		/**
 		 * An accessor the host's own abstract declares, which is a static taking the value first.
 		 *
-		 * Reading one had no spelling at all and fell to `Reflect.getProperty` below, which is handed
+		 * Reading one had no spelling at all and fell to `Fields.get` below (`Reflect.getProperty`), which is handed
 		 * the UNDERLYING value: `colour.red` asked an `Int` for a field of that name and answered null,
 		 * silently and with nothing refused. Reading only, since an abstract's setter assigns to `this`
 		 * and only the host's own inlining makes that reach the variable holding the value.
@@ -2393,8 +2393,8 @@ class Emitter {
 		w.int(2);
 		w.pos(line);
 		w.token('FSTATIC');
-		w.type('Reflect');
-		w.str('getProperty');
+		w.type('hxscript.runtime.Fields');
+		w.str('get');
 		expr(obj);
 		w.pos(line);
 		w.token('s');

src/hxscript/error/ErrorKind.hx

@@ -49,6 +49,9 @@ enum ErrorKind {
 	/** Field `f` cannot be accessed (e.g. a `private` violation). */
 	EInvalidAccess(f:String);
 
+	/** Field `f` was read, written or called on a null object. */
+	ENullAccess(f:String);
+
 	/** An arbitrary message. */
 	ECustom(msg:String);
 }

src/hxscript/error/Hint.hx

@@ -38,6 +38,10 @@ class Hint {
 				'inside the type that declares it; expose a public accessor, or set Config.strictAccess\n' +
 				'to false if this host wants scripts to reach private members.';
 
+			case ENullAccess(f):
+				'The object `$f` was asked of is null. Check it before using its fields, or use `?.`, which\n' +
+				'answers null instead.';
+
 			case ECustom(message):
 				custom(message);
 

src/hxscript/error/Printer.hx

@@ -159,6 +159,7 @@ class Printer {
 			case EInvalidIterator(v): "Invalid iterator: " + v;
 			case EInvalidOp(op): "Invalid operator: " + op;
 			case EInvalidAccess(f): "Invalid access to field " + f;
+			case ENullAccess(f): "Null access to field " + f;
 			case ECustom(msg): msg;
 		};
 	}

src/hxscript/runtime/Fields.hx

@@ -0,0 +1,45 @@
+package hxscript.runtime;
+
+import hxscript.error.ErrorKind;
+import hxscript.error.InterpException;
+
+/**
+ * A host object's field, read or written by name from a compiled body.
+ *
+ * `Reflect.getProperty` answers null for a null object and `Reflect.setProperty` does nothing. With
+ * `Config.strictNullAccess` both raise here instead, with the interpreter's text, so a compiled body
+ * stops at the line an interpreted one stops at. Without it they keep doing what they always did.
+ */
+@:keep
+class Fields {
+	/**
+	 * @param o The object.
+	 * @param f The field.
+	 * @return Its value, through its getter if it has one.
+	 */
+	public static function get(o:Dynamic, f:String):Dynamic {
+		if (o == null && Config.strictNullAccess)
+			nullAccess(f);
+
+		return Reflect.getProperty(o, f);
+	}
+
+	/**
+	 * @param o The object.
+	 * @param f The field.
+	 * @param v What to store, through its setter if it has one.
+	 * @return The value stored, because an assignment is an expression.
+	 */
+	public static function set(o:Dynamic, f:String, v:Dynamic):Dynamic {
+		if (o == null && Config.strictNullAccess)
+			nullAccess(f);
+
+		Reflect.setProperty(o, f, v);
+		return v;
+	}
+
+	/** @param f The field a null object was asked for. */
+	static function nullAccess(f:String):Void {
+		throw new InterpException(null, 'Null access to field ' + f, null, ENullAccess(f));
+	}
+}

src/hxscript/runtime/Interp.hx

@@ -2939,7 +2939,7 @@ class Interp {
 						if (obj == null) {
 							if (m)
 								return null;
-							error(EInvalidAccess(f));
+							error(ENullAccess(f));
 						}
 						return fcall(obj, f, args);
 					default:
@@ -3089,6 +3089,14 @@ class Interp {
 			namedHolder:Dynamic = null,
 			unknown:Null<String> = null;
 
+		/**
+		 * Whether `got` holds a value rather than a path still being resolved. A null value is then an
+		 * object with no fields, not a prefix of a longer type name. Without `Config.strictNullAccess`,
+		 * `h.kept` with `h` null is still looked up as a type called `h.kept`, finds nothing, and reads
+		 * as null, which scripts rely on.
+		 */
+		var valued:Bool = false;
+
 		var fullProp:String = '';
 		for (i => prop in __tempResolveFields) {
 			var field:String, maybe:Bool;
@@ -3102,24 +3110,34 @@ class Interp {
 					maybe = true;
 				case RExpr(e):
 					got = expr(e);
+					valued = true;
 					continue;
 			}
 
+			if (got == null && valued && Config.strictNullAccess) {
+				if (maybe)
+					return null;
+				error(ENullAccess(field));
+			}
+
 			if (got == null) {
 				fullProp = (fullProp.length == 0 ? field : '$fullProp.$field');
 
 				if (i == 0) {
 					if (captures.exists(field)) {
 						got = captures.get(field);
+						valued = true;
 					} else if (locals.exists(field)) {
 						got = getLocal(field);
+						valued = true;
 					} else if (isResolvable(field)) {
 						got = resolve(field);
+						valued = true;
 					} else {
 						unknown = field;
 					}
 
-					if (got != null)
+					if (valued)
 						continue;
 				}
 
@@ -3134,8 +3152,10 @@ class Interp {
 				} else if (gotType != null) {
 					var t = gotType.resolve(environment);
 					got = fromType(t, field, maybe);
+					valued = true;
 				} else if (namedHolder != null) {
 					got = fromType(namedHolder, field, maybe);
+					valued = true;
 				} else {
 					/**
 					 * A fully-qualified path the index does not carry, resolved by name.
@@ -3939,7 +3959,7 @@ class Interp {
 
 		if (o == null) {
 			if (!maybe) {
-				error(EInvalidAccess(f));
+				error(ENullAccess(f));
 			} else {
 				return null;
 			}
@@ -4121,7 +4141,7 @@ class Interp {
 			hxscript.debug.Metrics.writes++;
 
 		if (o == null)
-			error(EInvalidAccess(f));
+			error(ENullAccess(f));
 
 		if (canDefer && o is IScriptedType && !o.initialized)
 			throw DDefer;

test/common/fixtures/HostNullAccess.hx

@@ -0,0 +1,17 @@
+/**
+ * Switches `Config.strictNullAccess` from inside a script, which cannot name `hxscript.Config`.
+ *
+ * The flag is read at the access in every mode, so a case can turn it on around the access it checks
+ * and the same case runs interpreted and compiled.
+ */
+class HostNullAccess {
+	/**
+	 * @param on The value to set.
+	 * @return The value it had.
+	 */
+	public static function strict(on:Bool):Bool {
+		var was:Bool = hxscript.Config.strictNullAccess;
+		hxscript.Config.strictNullAccess = on;
+		return was;
+	}
+}

test/cpp/build.hxml

@@ -24,7 +24,7 @@
 # and not here, and nothing ran the whole matrix afterwards, so five cases sat failing.
 --macro hxscript.setup.Autowire.run()
 -D hxscript_bridge_types=HostBase,HostQuirk,HostShaped,HostOpenReturn,HostFinalField,app.HostShadow,HostPrivateInit,HostEnumSwitch,HostNamedType,HostNamedReflect,HostPrivateType,HostIndexOf,HostMarked,HostKinded,HostKeptNull,HostOptionalBool,HostUIntString,hostalias.Input,HostBagUser,HostTwinNew,HostPrivateStatic,HostStaticRead,HostImportStatic,HostSwitchField,HostSwitchMethod,HostSwitchStatic,HostSwitchRoot,HostCatchName,HostSwitchPattern
--D hxscript_keep=haxe.Json,haxe.ds.Option,HostVec,HostVecImpl,HostFlag,HostDial,HostTint,HostAxes,OpColor,OpVec,OpBlend
+-D hxscript_keep=haxe.Json,haxe.ds.Option,HostVec,HostVecImpl,HostFlag,HostDial,HostTint,HostAxes,HostNullAccess,OpColor,OpVec,OpBlend
 
 -main AllCommon
 -cpp bin_test/cpp/common
@@ -48,7 +48,7 @@
 # and not here, and nothing ran the whole matrix afterwards, so five cases sat failing.
 --macro hxscript.setup.Autowire.run()
 -D hxscript_bridge_types=HostBase,HostQuirk,HostShaped,HostOpenReturn,HostFinalField,app.HostShadow,HostPrivateInit,HostEnumSwitch,HostNamedType,HostNamedReflect,HostPrivateType,HostIndexOf,HostMarked,HostKinded,HostKeptNull,HostOptionalBool,HostUIntString,hostalias.Input,HostBagUser,HostTwinNew,HostPrivateStatic,HostStaticRead,HostImportStatic,HostSwitchField,HostSwitchMethod,HostSwitchStatic,HostSwitchRoot,HostCatchName,HostSwitchPattern
--D hxscript_keep=haxe.Json,haxe.ds.Option,HostVec,HostVecImpl,HostFlag,HostDial,HostTint,HostAxes,OpColor,OpVec,OpBlend
+-D hxscript_keep=haxe.Json,haxe.ds.Option,HostVec,HostVecImpl,HostFlag,HostDial,HostTint,HostAxes,HostNullAccess,OpColor,OpVec,OpBlend
 
 -main AllCpp
 -cpp bin_test/cpp/cppia
@@ -71,7 +71,7 @@
 # and not here, and nothing ran the whole matrix afterwards, so five cases sat failing.
 --macro hxscript.setup.Autowire.run()
 -D hxscript_bridge_types=HostBase,HostQuirk,HostShaped,HostOpenReturn,HostFinalField,app.HostShadow,HostPrivateInit,HostEnumSwitch,HostNamedType,HostNamedReflect,HostPrivateType,HostIndexOf,HostMarked,HostKinded,HostKeptNull,HostOptionalBool,HostUIntString,hostalias.Input,HostBagUser,HostTwinNew,HostPrivateStatic,HostStaticRead,HostImportStatic,HostSwitchField,HostSwitchMethod,HostSwitchStatic,HostSwitchRoot,HostCatchName,HostSwitchPattern
--D hxscript_keep=haxe.Json,haxe.ds.Option,HostVec,HostVecImpl,HostFlag,HostDial,HostTint,HostAxes,OpColor,OpVec,OpBlend
+-D hxscript_keep=haxe.Json,haxe.ds.Option,HostVec,HostVecImpl,HostFlag,HostDial,HostTint,HostAxes,HostNullAccess,OpColor,OpVec,OpBlend
 
 -main FrontierTest
 -cpp bin_test/cppia

test/lib/Corpus.hx

@@ -289,6 +289,28 @@ class Corpus {
 		check('host plain field beside a property',
 			'var h = new HostBase(); h.kept = 2; h.scaled = 3; return h.kept + h.scaled;', '8', null, host);
 
+		// A field of a null host object. By default a read answers null in every mode, which scripts
+		// rely on, so it stays that way; `Config.strictNullAccess` makes reads and writes raise, and
+		// compiled code then stops where interpreted code stops instead of reading null or writing
+		// nowhere. `HostNullAccess` switches the flag from inside the case, so every mode runs it alike.
+		check('host field of a null object reads null',
+			'var h:HostBase = null; return Std.string(h.kept);', 'null', null, host);
+		check('host property of a null object reads null',
+			'var h:HostBase = null; return Std.string(h.scaled);', 'null', null, host);
+
+		var strict:String->String = function(body:String):String {
+			return 'var h:HostBase = null; var r = "none"; var was = HostNullAccess.strict(true); '
+				+ 'try { $body } catch (e:Dynamic) { r = Std.string(e); } '
+				+ 'HostNullAccess.strict(was); return r;';
+		}
+		check('strict: host field of a null object raises', strict('r = Std.string(h.kept);'), 'Null access to field kept', null,
+			host);
+		check('strict: host property of a null object raises', strict('r = Std.string(h.scaled);'), 'Null access to field scaled',
+			null, host);
+		check('strict: host field write on a null object raises', strict('h.kept = 1; r = "wrote";'), 'Null access to field kept',
+			null, host);
+		check('strict: ?. on a null object still reads null', strict('r = Std.string(h?.kept);'), 'null', null, host);
+
 		// An array literal has nothing in it to say what it holds, so it used to be built loose while an
 		// annotation promised a specific kind. Reading it back through that annotation reinterprets the
 		// memory, which crashes rather than misbehaves, so these index one after the round trip.

test/lib/conformance.sh

@@ -38,7 +38,7 @@ CP="-cp $ROOT/src -cp $ROOT/test/common -cp $ROOT/test/common/fixtures -cp $ROOT
 # ships. The base is a fixture rather than a standard-library class because a bridge has to be
 # generatable at all and most of the library is not: `StringBuf` and `haxe.io.BytesBuffer` inline an
 # abstract's constructor, and `haxe.Timer` starts a real timer and ended the run.
-KEEP="--macro hxscript.macro.Keep.run() --macro hxscript.setup.Autowire.run() -D hxscript_bridge_types=HostBase,HostQuirk,HostShaped,HostOpenReturn,HostFinalField,app.HostShadow,HostPrivateInit,HostEnumSwitch,HostNamedType,HostNamedReflect,HostPrivateType,HostIndexOf,HostMarked,HostKinded,HostKeptNull,HostOptionalBool,HostUIntString,hostalias.Input,HostBagUser,HostTwinNew,HostPrivateStatic,HostStaticRead,HostImportStatic,HostSwitchField,HostSwitchMethod,HostSwitchStatic,HostSwitchRoot,HostCatchName,HostSwitchPattern -D hxscript_keep=haxe.Json,haxe.ds.Option,HostVec,HostVecImpl,HostFlag,HostDial,HostTint,HostAxes,OpColor,OpVec,OpBlend"
+KEEP="--macro hxscript.macro.Keep.run() --macro hxscript.setup.Autowire.run() -D hxscript_bridge_types=HostBase,HostQuirk,HostShaped,HostOpenReturn,HostFinalField,app.HostShadow,HostPrivateInit,HostEnumSwitch,HostNamedType,HostNamedReflect,HostPrivateType,HostIndexOf,HostMarked,HostKinded,HostKeptNull,HostOptionalBool,HostUIntString,hostalias.Input,HostBagUser,HostTwinNew,HostPrivateStatic,HostStaticRead,HostImportStatic,HostSwitchField,HostSwitchMethod,HostSwitchStatic,HostSwitchRoot,HostCatchName,HostSwitchPattern -D hxscript_keep=haxe.Json,haxe.ds.Option,HostVec,HostVecImpl,HostFlag,HostDial,HostTint,HostAxes,HostNullAccess,OpColor,OpVec,OpBlend"
 
 cd "$ROOT"
 mkdir -p "$OUT"