Flow: Flow coverage reports empty vars as uncovered.

[lrowe]

I use switch statements to refine over a discriminated union type. To avoid complaints from #451 I throw an error from my default clause taking an empty value to ensure I have exhaustively handled my type:

//@flow
"use strict";
type ExprA = { type: "ExprA" };
type ExprB = { type: "ExprB" };
type Expr = ExprA | ExprB;

class UnknownExprError extends Error {
  constructor(expr: empty) { // uncovered code `expr`
    super(expr.type);  // uncovered code `expr` and `type`
  }
}
UnknownExprError.prototype.name = "UnknownExprError";

function someExpr(expr: Expr): boolean {
  switch (expr.type) {
    case "ExprA":
      return true;
    case "ExprB":
      return false;
    default:
      throw new UnknownExprError(expr);  // uncovered code `expr`
  }
}

However flow coverage (as reported by flow-for-vscode) reports uncovered code for the variables with type empty.

With the possible exception of expr.type I think coverage should not report this unreachable code as uncovered.

[vkurchatkin]

expr in this position is not different from any, so it's indeed uncovered

[lrowe]

I think there's a useful distinction to be made between the use of empty and any in that while any avoids the type checker empty leverages the type checker to report an error.

If I change my code to the following, Flow will report no uncovered code:

function someExpr(expr: Expr): boolean {
  switch (expr.type) {
    case "ExprA":
      return true;
    case "ExprB":
      return false;
    default:
      throw new Error("Unknown expr");
  }
}

Removing the empty type check didn't make my code any safer since I may have forgotten to check all possible types. Flow approves of the following code too:

function someExpr(expr: Expr): boolean {
  switch (expr.type) {
    case "ExprA":
      return true;
    default:
      throw new Error("Unknown expr");
  }
}

Taking a similar example from the docs: Typing Redux reducers I see uncovered code reported for (action: empty).

Coverage reporting is useful when it encourages safer typing. Explicitly using empty like this increases type safety so we shouldn't discourage it.

[forticulous]

I just ran into this issue in our codebase where we're attempting to keep 100% flow coverage. I ended up doing a workaround like the following, but I'm not happy with it and would much rather use empty

// Passes coverage but lacks type safety
type SomeUnion = 'a' | 'b'

const mySomeUnion: SomeUnion = 'a'

switch (mySomeUnion) {
  case 'a':
    console.log('It was a')
    break
  case 'b':
    console.log('It was b')
    break
  default:
    // what i would like to do here
    // (mySomeUnion: empty)
    throw new Error('unreachable')
}

// Exhaustively type checks and passes coverage
type NotSomeUnion = {}
type SomeUnionExhaustive = SomeUnion | NotSomeUnion

const mySomeUnionExhaustive: SomeUnionExhaustive = 'a'

switch (mySomeUnionExhaustive) {
  case 'a':
    console.log('a')
    break
  case 'b':
    console.log('b')
    break
  default:
    (mySomeUnionExhaustive: NotSomeUnion)
    throw new Error('unreachable')
}

[minedeljkovic]

I believe this is made intentional from the first version of coverage command introduced in #1903 on this line

@nmote can you, please, give us some reason why you considered expression of type empty as uncovered, when, in general, empty is considered safe (bottom) type?

Related #6082

[nmote]

I just rewrote the existing logic in that PR. empty was already considered uncovered so I kept the current behavior.

[joseprio]

Shouldn't empty behave more like mixed than any? IMHO, accessing any property of an empty type should be an error.