Browse Source

[js] fix type inference for Promise callback arguments (closes #6790) (#7644)

* [js] fix type inference for Promise callback arguments (closes #6790)

* [tests] adapt test for #4540

now the error message is also nicer \o/
Dan Korostelev 6 years ago
parent
commit
dd443c3c68

+ 30 - 25
std/js/Promise.hx

@@ -25,21 +25,21 @@ package js;
 import haxe.extern.EitherType;
 import haxe.extern.EitherType;
 
 
 /**
 /**
-	The Promise object represents the eventual completion (or failure) of an 
+	The Promise object represents the eventual completion (or failure) of an
 	asynchronous operation and its resulting value.
 	asynchronous operation and its resulting value.
-	
+
 	Documentation [Promise](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise) by [Mozilla Contributors](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise$history), licensed under [CC-BY-SA 2.5](https://creativecommons.org/licenses/by-sa/2.5/).
 	Documentation [Promise](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise) by [Mozilla Contributors](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise$history), licensed under [CC-BY-SA 2.5](https://creativecommons.org/licenses/by-sa/2.5/).
 **/
 **/
 @:native("Promise")
 @:native("Promise")
 extern class Promise<T>
 extern class Promise<T>
 {
 {
 	/**
 	/**
-		Returns a Promise object that is resolved with the given value. If the 
-		value is Thenable, the returned promise will "follow" that 
-		thenable, adopting its eventual state; 
-		otherwise the returned promise will be fulfilled with the value. 
-		Generally, when it's unknown when value is a promise or not, 
-		use `Promise.resolve(value)` instead and work with the return value as 
+		Returns a Promise object that is resolved with the given value. If the
+		value is Thenable, the returned promise will "follow" that
+		thenable, adopting its eventual state;
+		otherwise the returned promise will be fulfilled with the value.
+		Generally, when it's unknown when value is a promise or not,
+		use `Promise.resolve(value)` instead and work with the return value as
 		a promise.
 		a promise.
 	**/
 	**/
 	@:overload(function<T>(promise : Promise<T>) : Promise<T> {})
 	@:overload(function<T>(promise : Promise<T>) : Promise<T> {})
@@ -52,20 +52,20 @@ extern class Promise<T>
 	static function reject<T>( ?reason : Dynamic ) : Promise<T>;
 	static function reject<T>( ?reason : Dynamic ) : Promise<T>;
 
 
 	/**
 	/**
-		Returns a promise that either fulfills when all of the promises in the 
+		Returns a promise that either fulfills when all of the promises in the
 		iterable argument have fulfilled or rejects as soon as one of the
 		iterable argument have fulfilled or rejects as soon as one of the
-		promises in the iterable argument rejects. If the returned promise 
-		fulfills, it is fulfilled with an array of the values from the 
-		fulfilled promises in the same order as defined in the iterable. 
-		If the returned promise rejects, it is rejected with the reason from 
-		the first promise in the iterable that rejected. This method can be 
+		promises in the iterable argument rejects. If the returned promise
+		fulfills, it is fulfilled with an array of the values from the
+		fulfilled promises in the same order as defined in the iterable.
+		If the returned promise rejects, it is rejected with the reason from
+		the first promise in the iterable that rejected. This method can be
 		useful for aggregating results of multiple promises.
 		useful for aggregating results of multiple promises.
 	**/
 	**/
 	static function all( iterable : Array<Dynamic> ) : Promise<Array<Dynamic>>;
 	static function all( iterable : Array<Dynamic> ) : Promise<Array<Dynamic>>;
 
 
 	/**
 	/**
-		Returns a promise that fulfills or rejects as soon as one of the 
-		promises in the iterable fulfills or rejects, with the value or reason 
+		Returns a promise that fulfills or rejects as soon as one of the
+		promises in the iterable fulfills or rejects, with the value or reason
 		from that promise.
 		from that promise.
 	**/
 	**/
 	static function race( iterable : Array<Dynamic> ) : Promise<Dynamic>;
 	static function race( iterable : Array<Dynamic> ) : Promise<Dynamic>;
@@ -74,26 +74,31 @@ extern class Promise<T>
 	function new( init : (resolve : (value : T) -> Void, reject: (reason : Dynamic) -> Void) -> Void ) : Void;
 	function new( init : (resolve : (value : T) -> Void, reject: (reason : Dynamic) -> Void) -> Void ) : Void;
 
 
 	/**
 	/**
-		Appends fulfillment and rejection handlers to the promise and returns a 
-		new promise resolving to the return value of the called handler, or to 
-		its original settled value if the promise was not handled 
+		Appends fulfillment and rejection handlers to the promise and returns a
+		new promise resolving to the return value of the called handler, or to
+		its original settled value if the promise was not handled
 		(i.e. if the relevant handler onFulfilled or onRejected is not a function).
 		(i.e. if the relevant handler onFulfilled or onRejected is not a function).
 	**/
 	**/
-	function then<TOut>( fulfillCallback : Null<PromiseCallback<T, TOut>>, ?rejectCallback : EitherType<Dynamic -> Void, PromiseCallback<Dynamic, TOut>> ) : Promise<TOut>;
+	function then<TOut>( fulfillCallback : Null<PromiseHandler<T, TOut>>, ?rejectCallback : PromiseHandler<Dynamic, TOut> ) : Promise<TOut>;
 
 
 	/**
 	/**
-		Appends a rejection handler callback to the promise, and returns a new 
-		promise resolving to the return value of the callback if it is called, 
+		Appends a rejection handler callback to the promise, and returns a new
+		promise resolving to the return value of the callback if it is called,
 		or to its original fulfillment value if the promise is instead fulfilled.
 		or to its original fulfillment value if the promise is instead fulfilled.
 	**/
 	**/
 	@:native("catch")
 	@:native("catch")
-	function catchError<TOut>( rejectCallback : EitherType<Dynamic -> Void, PromiseCallback<Dynamic, TOut>> ) : Promise<TOut>;
+	function catchError<TOut>( rejectCallback : PromiseHandler<Dynamic, TOut> ) : Promise<TOut>;
 }
 }
 
 
 /**
 /**
-	Callback for the Promise object.
+	Handler type for the Promise object.
 **/
 **/
-typedef PromiseCallback<T, TOut> = EitherType<T -> TOut, T -> Promise<TOut>>;
+abstract PromiseHandler<T,TOut>(T->Dynamic) // T->Dynamic, so the compiler always knows the type of the argument and can infer it for then/catch callbacks
+	from T->Promise<TOut> // direct casts have priority, so we use that to prioritize the Promise<TOut> return type
+{
+	// function casts are checked after direct ones, so we place T->TOut here to make it have lower priority than T->Promise<TOut>
+	@:from static inline function fromNonPromise<T,TOut>(f:T->TOut):PromiseHandler<T,TOut> return cast f;
+}
 
 
 /**
 /**
 	A value with a `then` method.
 	A value with a `then` method.

+ 2 - 2
tests/misc/projects/Issue4540/compile-fail.hxml.stderr

@@ -1,2 +1,2 @@
-Main.hx:3: characters 36-55 : (c : { test : String }) -> Void should be Null<js.PromiseCallback<Int, Void>>
-Main.hx:3: characters 36-55 : For function argument 'fulfillCallback'
+Main.hx:3: characters 53-54 : Int should be { test : String }
+Main.hx:3: characters 53-54 : For function argument 'a'

+ 24 - 0
tests/misc/projects/Issue6790/Main.hx

@@ -0,0 +1,24 @@
+import js.Promise;
+
+class Main {
+	static function main() {
+		var p = new Promise<String>(null);
+
+		$type(p.then(function(x) $type(x)));
+		$type(p.then(function(x) $type(x), function(e) $type(e)));
+
+		$type(p.then(function(x) {$type(x); return 1;}));
+		$type(p.then(function(x) {$type(x); return Promise.resolve(1);}));
+
+		$type(p.then(null,function(x) {$type(x); return 1;}));
+		$type(p.then(null,function(x) {$type(x); return Promise.resolve(1);}));
+
+		$type(p.then(function(x) {$type(x); return 1;}, function(e) {$type(e); return 1;}));
+		$type(p.then(function(x) {$type(x); return Promise.resolve(1);}, function(e) {$type(e); return 1;}));
+		$type(p.then(function(x) {$type(x); return 1;}, function(e) {$type(e); return Promise.resolve(1);}));
+		$type(p.then(function(x) {$type(x); return Promise.resolve(1);}, function(e) {$type(e); return Promise.resolve(1);}));
+
+		$type(p.catchError(function(x) {$type(x);}));
+		$type(p.catchError(function(x) {$type(x); return Promise.resolve(1);}));
+	}
+}

+ 8 - 0
tests/misc/projects/Issue6790/Mismatch.hx

@@ -0,0 +1,8 @@
+import js.Promise;
+
+class Mismatch {
+	static function main() {
+		var p = new Promise<String>(null);
+		p.then(x -> 10, e -> "");
+	}
+}

+ 3 - 0
tests/misc/projects/Issue6790/compile-fail.hxml

@@ -0,0 +1,3 @@
+-main Mismatch
+-js whatever.js
+--no-output

+ 2 - 0
tests/misc/projects/Issue6790/compile-fail.hxml.stderr

@@ -0,0 +1,2 @@
+Mismatch.hx:6: characters 19-26 : (e : Unknown<0>) -> String should be Null<js.PromiseHandler<Dynamic, Int>>
+Mismatch.hx:6: characters 19-26 : For optional function argument 'rejectCallback'

+ 3 - 0
tests/misc/projects/Issue6790/compile.hxml

@@ -0,0 +1,3 @@
+-main Main
+-js whatever.js
+--no-output

+ 29 - 0
tests/misc/projects/Issue6790/compile.hxml.stderr

@@ -0,0 +1,29 @@
+Main.hx:7: characters 34-35 : Warning : String
+Main.hx:7: characters 9-37 : Warning : js.Promise<Void>
+Main.hx:8: characters 34-35 : Warning : String
+Main.hx:8: characters 56-57 : Warning : Unknown<0>
+Main.hx:8: characters 9-59 : Warning : js.Promise<Void>
+Main.hx:10: characters 35-36 : Warning : String
+Main.hx:10: characters 9-50 : Warning : js.Promise<Int>
+Main.hx:11: characters 35-36 : Warning : String
+Main.hx:11: characters 9-67 : Warning : js.Promise<Int>
+Main.hx:13: characters 40-41 : Warning : Unknown<0>
+Main.hx:13: characters 9-55 : Warning : js.Promise<Int>
+Main.hx:14: characters 40-41 : Warning : Unknown<0>
+Main.hx:14: characters 9-72 : Warning : js.Promise<Int>
+Main.hx:16: characters 35-36 : Warning : String
+Main.hx:16: characters 70-71 : Warning : Unknown<0>
+Main.hx:16: characters 9-85 : Warning : js.Promise<Int>
+Main.hx:17: characters 35-36 : Warning : String
+Main.hx:17: characters 87-88 : Warning : Unknown<0>
+Main.hx:17: characters 9-102 : Warning : js.Promise<Int>
+Main.hx:18: characters 35-36 : Warning : String
+Main.hx:18: characters 70-71 : Warning : Unknown<0>
+Main.hx:18: characters 9-102 : Warning : js.Promise<Int>
+Main.hx:19: characters 35-36 : Warning : String
+Main.hx:19: characters 87-88 : Warning : Unknown<0>
+Main.hx:19: characters 9-119 : Warning : js.Promise<Int>
+Main.hx:21: characters 41-42 : Warning : Unknown<0>
+Main.hx:21: characters 9-46 : Warning : js.Promise<Void>
+Main.hx:22: characters 41-42 : Warning : Unknown<0>
+Main.hx:22: characters 9-73 : Warning : js.Promise<Int>