strictNullChecks false positives in case clause

[ajafff]

TypeScript Version: 2.9.0-dev.20180512

Search Terms:

Code

// @strictNullChecks: true

function test1(param?: {prop: string}) {
  switch (param && param.prop) {
    case 'foo':
      return param.prop; // error: 'param' is possibly undefined
  }
}

function test2(param: {prop?: string}) {
  switch (param && param.prop) {
    case 'foo':
      return param.prop.charAt(0); // error: 'param.prop' is possibly undefined
  }
}

Expected behavior:

Compiles without error.

Actual behavior:

strictNullChecks errors as described in the comments above.

Playground Link: https://agentcooper.github.io/typescript-play/?noImplicitReturns=false#code/GYVwdgxgLglg9mABFApgZygRgBQAcCGATvgLYD8AXIgN66Fy5UaExgDmAvgJQ0BQiiNAHcYUCAAtEeIqUQAyOYgLESAOjoMe1fgMQR8aFIgDkwOHGMUduxIRRQQhJMtLr6uANyIA9N8QpCekIqYxcSY0QYNCU4NDQYACMAGwBPRHAAExRgVhQMnQ5eQt5QSFgEZHQoACZpFSpad0pBKBZ2bj4BYVEJKTD5RTC3TU7dfUMTMwsrGwE7ByclGTUNXFUJIgBBKGwABi4vX39AuGCTIdWIqJi4xNT0sCycsDyCoqA

Related Issues:
#23818

[DanielRosenwasser]

Wow I've never seen anyone do that with a switch/case. I don't know how easy it is to add this to our narrowing logic, but I'll mark it as a bug.

[weswigham]

A survey of our own codebase says that we have seen this pattern before (we just don't use a discriminated union for node and strict null checks yet, so we haven't noticed). 😄

[weswigham]

This isn't unique to switch-case - this also doesn't work:

function test1(param?: { prop: string }) {
    if ((param && param.prop) === "foo") {
        return param.prop;
    }
}

which is equivalent to the switch-case above.

And, similarly/simplified a bit:

function test1(param?: { prop: string | number }): "no" | { prop: string | number } {
    if ((param && "foo") === "foo") {
        return param; // error
    }
    return "no";
}

The root cause looks like it's because when we start diving into (param && "foo") === "foo" to narrow, our logic to locate references to narrow is a little... narrow (we need to derive that param is at least truthy in the true branch of that block).

[defenestrant]

I think I've just come across this same issue. The strict-null check doesn't work when the check is contained in an other function.

type NullOrString = string | null

const testForNull = (param : NullOrString) => param != null
const doStringStuff = (param : String) => { /* ... */ }

const doSomething = (param: NullOrString) =>
{
    if(param != null)
        doStringStuff(param)
}

const doSamething = (param: NullOrString) =>
{
    if(testForNull(param))
        doStringStuff(param)
}

The doSamething-function will give this error:

[ts]
Argument of type 'NullOrString' is not assignable to parameter of type 'String'.
  Type 'null' is not assignable to type 'String'.
(parameter) param: NullOrString

[RyanCavanaugh]

@defenestrant see #10734 or similar issues

[justingrant]

@weswigham @RyanCavanaugh @DanielRosenwasser - this is a common pattern in JavaScript, not only in TypeScript's own codebase. ;-)

Any time you want to switch on a property value or array member of a nullable object, you need to first make sure that the object isn't null or undefined. This is easily done via switch (x && x.foo). A big advantage of this pattern is that you can combine error handling for null or undefined x in the same case (usually a default:) that you're using to handle bad values of x.foo. Having all "bad parameter values" error handling in the same place avoids redundant code and makes the code easier to read and reason about.

The best workaround I found is to transform the switch into if statements and to refactor the if expressions from the broken form if ( (x && x.foo) === 1) to the working form if (x && x.foo===1). This is a painful and potentially error-prone refactor for long switch statements.

Other workarounds like wrapping the switch in if (x) { switch ... } are easier but make error handling harder because handling null x conditions is now in a different place from handling bad values of x.foo. Using a non-null assertion x!.foo in the case is also easy, but it risks breaking type safety if x does actually turn out to be null due to later changes in surrounding code.

Here's simple examples of the broken cases and the (bad, IMHO) workarounds available in TypeScript 3.1.

// compiled with strictNullChecks
function repro (x: string[] | null) { 

  // broken
  switch (x && x.length) {
    case 1: 
      console.log (x[0]); // compiler error: Object is possibly null
      break;
    default: 
      console.log ('invalid x'); 
      break;
  }

  // broken also-- illustrates that root cause is not switch vs. if
  if ((x && x.length)===1) {
    console.log (x[0]); // compiler error: Object is possibly null
  } else {
    console.log ('invalid x'); 
  }
  
  // workaround 1: convert to if
  if (x && x.length===1) {
    console.log (x[0]); // no compiler error
  } else {
    console.log ('invalid x'); 
  }

  // workaround 2: wrap switch with null check. duplicates error handling code!
  if (x) {
    switch (x && x.length) {
      case 1: 
        console.log (x[0]); // no compiler error
        break;
      default: 
        console.log ('invalid x'); 
        break;
    }
  } else {
    console.log ('invalid x'); 
  }

  // workaround 3: nested switch statements. awful!
  switch (x) {
    case null:
    case undefined: 
      console.log ('invalid x'); 
    break;
    default: 
      switch (x.length) {  // no compiler error
        case 1: 
          console.log (x[0]); // no compiler error
          break;
        default: 
          console.log ('invalid x'); 
      }
      break;
  }

  // workaround 4: extract into local variables outside the switch. cumbersome!
  const len = x && x.length; // no compiler error
  const first = x && x.length && x[0]; // no compiler error
  switch (len) {
    case 1: 
      console.log (first); 
      break;
    default: 
      console.log ('invalid x'); 
    break;
  }  

  // workaround 5: use non-null type assertion. not type safe!
  switch (x && x.length) {
    case 1: 
      console.log (x![0]); // no compiler error (but could hide real type-safety errors later)
      break;
    default: 
      console.log ('invalid x'); 
      break;
  }

} 

[mysticatea]

I have a similar issue, but it doesn't regard strictNullCheck option.

type TestType = { type: "a", a: number } | { type: "b", b: number }

function test(obj: TestType | null): number {
    switch (obj && obj.type) {
        case null:
            return 0
        case "a":
            return obj.a // Property 'a' does not exist on type 'TestType'.  Property 'a' does not exist on type '{ type: "b"; b: number; }'.
        case "b":
            return obj.b // Property 'b' does not exist on type 'TestType'.  Property 'b' does not exist on type '{ type: "a"; a: number; }'.
    }
    throw new Error("unreachable")
}

http://www.typescriptlang.org/play/#src=type%20TestType%20%3D%20%7B%20type%3A%20%22a%22%2C%20a%3A%20number%20%7D%20%7C%20%7B%20type%3A%20%22b%22%2C%20b%3A%20number%20%7D%0D%0A%0D%0Afunction%20test(obj%3A%20TestType%20%7C%20null)%3A%20number%20%7B%0D%0A%20%20%20%20switch%20(obj%20%26%26%20obj.type)%20%7B%0D%0A%20%20%20%20%20%20%20%20case%20null%3A%0D%0A%20%20%20%20%20%20%20%20%20%20%20%20return%200%0D%0A%20%20%20%20%20%20%20%20case%20%22a%22%3A%0D%0A%20%20%20%20%20%20%20%20%20%20%20%20return%20obj.a%0D%0A%20%20%20%20%20%20%20%20case%20%22b%22%3A%0D%0A%20%20%20%20%20%20%20%20%20%20%20%20return%20obj.b%0D%0A%20%20%20%20%7D%0D%0A%20%20%20%20throw%20new%20Error(%22unreachable%22)%0D%0A%7D%0D%0A

[brunolemos]

Not sure if fits on this issue or not, but here's an example of false positive I'm facing:

Link: Playground (ps: enable strictNullChecks)

interface Obj { a: number }

function test(obj?: Obj) {
  const hasA = !!(obj && obj.a)

  // Correct: Shows warning
  console.log(obj.a)

  // Correct: Doesn't show warning
  if (obj) {
    console.log(obj.a)
  }

  // Wrong: Should not show warning
  if (hasA) {
    console.log(obj.a)
  }
}

test()
test({ a: 1 })

[test137E29B]

Apologies for bumping this after so long, but I've also stumbled across this issue in React TSX files.

Here:

render() {
  return (
    { state && state.name === "SomeStateString" && (
      <div> render some element </div>
    )}
  )
}

state.name shows TypeScript error: Object is possibly 'undefined'.

In this use case I can get around it in other ways, but it does make it less pretty.