RealyUniqueName / Safety

Null safety for Haxe
MIT License
54 stars 5 forks source link

Maybe not a bug. Safety: Cannot cast nullable value to not nullable type with Std.instance #17

Open restorer opened 5 years ago

restorer commented 5 years ago
class SomeClass {
    public function new() {}
}

class MaybeBug12 {
    public function bug() : Void {
        var a = new SomeClass();

        // Safety: Cannot cast nullable value to not nullable type.
        // Probably this is not a bug in Safety, and this issue related to Std.instance() signature
        var b : Null<SomeClass> = Std.instance(a, SomeClass);
    }
}

See https://github.com/restorer/haxe-safety-bugs/blob/master/safetybugs/Main.hx#L163 for working example.

restorer commented 5 years ago

This is for Haxe 4.0.0-preview.5 and Safety from master.

RealyUniqueName commented 5 years ago

This is an intended behavior for a typed cast: https://github.com/RealyUniqueName/Safety/blob/9dc8579041dd9121eba1825ac1b6a7124a85129a/test/cases/TestCompileTime.hx#L505-L509

However, Std.instance() uses an untyped cast, which should ignore null safety checks: https://github.com/RealyUniqueName/Safety/blob/9dc8579041dd9121eba1825ac1b6a7124a85129a/test/cases/TestCompileTime.hx#L511-L514

Maybe Haxe typer makes them indistinguishable for the Safety plugin in case of Std.instance()

restorer commented 5 years ago

I think problem is in Std.instance signature, which is

public static function instance<T:{},S:T>( value : T, c : Class<S> ) : S;

However, documentation says:

This method checks if a downcast is possible. That is, if the runtime type of value is assignable to the class specified by c, value is returned. Otherwise null is returned.

Proper signature should be

public static function instance<T:{},S:T>( value : T, c : Class<S> ) : Null<S>;

I "fix" this by

@:safety(unsafe)
public static function instanceExt<T : {}, S : T>(value : T, c : Class<S>) : Null<S> {
    return Std.instance(value, c);
}

I understand that this is something not on Safety side, but maybe you can propose this change for Haxe.