haxe-instrument icon indicating copy to clipboard operation
haxe-instrument copied to clipboard

Safe navigation operator and/or null coalescing operator not supported

Open cedx opened this issue 2 years ago • 3 comments

Not sure if I should report this issue on the Haxe repo or here... I'm using some new features from Haxe 4.3.0 in a project: safe navigation and null coalescing operators.

public function new(?options: CacheOptions) {
	defaultDuration = options?.defaultDuration ?? 0;
	keyPrefix = options?.keyPrefix ?? "";
	serializer = options?.serializer ?? {
		serialize: #if php Lib.serialize #else Serializer.run #end,
		unserialize: #if php Lib.unserialize #else Unserializer.run #end
	};
}

When instrumentation is disabled, the project tests work fine. Here is the PHP code that is generated by the Haxe compiler:

public function __construct ($options = null) {
	$tmp = ($options !== null ? $options->defaultDuration : null);
	$this->defaultDuration = ($tmp !== null ? $tmp : 0);
	$tmp = ($options !== null ? $options->keyPrefix : null);
	$this->keyPrefix = ($tmp !== null ? $tmp : "");
	$tmp = ($options !== null ? $options->serializer : null);
	$this->serializer = ($tmp !== null ? $tmp : new HxAnon([
		"serialize" => Boot::getStaticClosure(Lib::class, 'serialize'),
		"unserialize" => Boot::getStaticClosure(Lib::class, 'unserialize'),
	]));
}

But when instrumentation is enabled, I get errors on Java and PHP targets. The generated code has a different behavior:

public function __construct ($options = null) {
	CoverageContext::logExpression(1538);
	CoverageContext::logExpression(1539);
	$tmp = $options->defaultDuration;
	$this->defaultDuration = ($tmp !== null ? $tmp : 0);
	CoverageContext::logExpression(1540);
	$tmp = $options->keyPrefix;
	$this->keyPrefix = ($tmp !== null ? $tmp : "");
	CoverageContext::logExpression(1541);
	$tmp = $options->serializer;
	$this->serializer = ($tmp !== null ? $tmp : new HxAnon([
		"serialize" => Boot::getStaticClosure(Lib::class, 'serialize'),
		"unserialize" => Boot::getStaticClosure(Lib::class, 'unserialize'),
	]));
}

Notice how the $options argument is not checked anymore for a possible null value. It triggers this kind of error:

# Java
Exception in thread "main" java.lang.NullPointerException: defaultDuration
        at haxe.jvm.Jvm.readField(C:\[...]/haxe/versions/4.3.0/std/jvm/Jvm.hx:347

# PHP
PHP Fatal error:  Uncaught ErrorException: Attempt to read property "defaultDuration" on null in C:\[...]\Cache.php:49

cedx avatar Apr 06 '23 22:04 cedx

thanks for reporting!

my latest commit changed quite a few things (hopefully for the better!). those changes most likely come with a change in coverage results. please take a minute to go through your coverage report / source code to check for places where you disagree with new coverage results. I plan to release version 1.2.0 soonish, so any feedback before that would be great!! - if time permits, if not there is always 1.2.1:).

AlexHaxe avatar Apr 12 '23 23:04 AlexHaxe

Thanks for your efforts but it still doesn't work. Unfortunately I can't give you access to the initial code (it's a private project), but I have 3 other projects where you can see the problem appear.

https://github.com/cedx/cookies.hx (target: js) https://github.com/cedx/webstorage.hx (target: js) https://github.com/cedx/which.hx (targets: hl, java, js and php)

To reproduce the bug:

  • clone the repo
  • lix Install (or lix download if you get an error about the Install class not found)
  • lix install gh:AlexHaxe/haxe-instrument
  • remove the @:ignoreInstrument metadata from the code
  • lix Test (generated files go into the var directory)

On my private project, the initial issue was solved, but new problems appeared. This worked very well:

abstract Errors(Map<String, String>) from Map<String, String> to Map<String, String> {
	public function new(?errors: Map<String, String>) this = errors ?? [];

	@:from static function ofError(error: Error) {
		final data: {details: Array<Array<String>>} = try Json.parse(error.data) catch (e) {details: []};
		return new Errors([for (entry in data.details) if (entry.length == 2) entry[0] => entry[1]]);
	}
}

But now the code does not compile anymore (unless I add the @:ignoreInstrument meta):

return new Errors([for (entry in data.details) if (entry.length == 2) entry[0] => entry[1]]);
// characters 67-71 : haxe.ds.Map<String, String> has no field push

cedx avatar Apr 14 '23 09:04 cedx

sorry, for my inactivity.

I've been looking at the issues every now an then, trying to fix them. and I think at least for one of them I have a working solution. for the other issue I've encountered there is currently no solution other then not add coverage instrumentation to safe navigation operations - at least until there is a fix / workaround for https://github.com/HaxeFoundation/haxe/issues/11246.

AlexHaxe avatar Jun 06 '23 10:06 AlexHaxe