hhvm / hacktest

A unit testing framework for Hack
MIT License
29 stars 12 forks source link

[easy] make VerboseCLIOutput forward compatible with future HSL IO ch… #98

Closed fredemmott closed 4 years ago

fredemmott commented 4 years ago

…ange

HSL IO is changing writeAsync() to return Awaitable<int>, but the function here is Awaitable<void> - return await is no longer compatible.

Tangentially, I'm a little surprised that explicit return values are OK in Awaitable functions :/

Wilfred commented 4 years ago

@fredemmott what were you expecting? The following works:

async function foo(): Awaitable<int> {
  return 1;
}

This code seems equivalent AFAICS.

fredemmott commented 4 years ago

https://travis-ci.org/github/hhvm/hsl-experimental/jobs/680389022

The function here returns Awaitable<void>, not int. Posted in feedback group about explicit returns being OK for Awaitable<void> which feels like a bug

fredemmott commented 4 years ago

This is closer to:

async function foo(): Awaitable<void> {
  return 1; // int, not void
}
Wilfred commented 4 years ago

but return; is appropriate in an Awaitable<void>, and writeFailureDetailsAsync is awaitable void. Your foo is rejected by the type checker, as I'd expect. I think I've misunderstood something.

jjergus commented 4 years ago

@Wilfred This is an example that passes typechecker but perhaps shouldn't:

async function void1(): Awaitable<void> {
}

async function void2(): Awaitable<void> {
  return await void1();
}
Wilfred commented 4 years ago

Oh, I was being dumb! I was looking at the new version of the code, and totally missed the old version.

Yeah, that's fine as far as inference is concerned. We have some additional typechecker checks to make sure that you don't write return; in a non-void function or return ...; in a void function, but looks like it misses Awaitable<void>.

void handling could use some refactoring to better handle cases like this.