Typescript: Disabling unreachable code detection partially for the condition completeness check

[falsandtru]

I want to disable the unreachable code detection for the condition completeness check.

function f(n) {
  switch(n) {
    case 0: return 0;
    default: return 1;
  }
  // comment base disabling
  // @unreachable
  assert(false); // remove by unassert tools before the release
  throw new Error();
}

[RyanCavanaugh]

Is there any precedent for this? It seems odd.

If you want a workaround, you can put the default section in an if (true) { ... }.

[mhegazy]

also you can just use --allowUnreachableCode

[falsandtru]

I cannot find a precedent now. But developers sometimes forget the default and else clause. This is useful for an annotation and a prevention. Prevention is better than cure.

other case:

function f(n) {
  if (n >= 0) {
    return n;
  }
  else if (n < 0) {
    return n;
  }
  else { // don't forget
    return 0;
  }
  // call if you forget the else clause
  throw new Error();
}

@mhegazy I want to disable partially.

[DanielRosenwasser]

I think this would be better served by a lint tool, but others can weigh in on this.

[mhegazy]

I agree with @DanielRosenwasser

[falsandtru]

Additionally, a reachability check cannot detect that fault case because an incomplete condition reach the tail statement.

@DanielRosenwasser But an lint tool cannot allow that code in compilation.

Reachability check disallow that prevention code completely?

[RyanCavanaugh]

I find the entire construct here very confusing. You want to make sure that you don't have any unreachable code... so you add a line of unreachable code?

Many coding styles (e.g. ours) say that if you unconditionally return in the true branch of an if, you should _not_ have an else block (the remainder of the function body is essentially the else).

It seems like what you're trying to enforce is "return on all code paths", which is a reasonable thing for a linter to detect.

[falsandtru]

I add oneself a reachability check code instead of reachability check by compiler. This is my use case using a DbC(Design by Contract) like coding style. Is this not good?

src\lib\message.ts(126,5): error TS7027: Unreachable code detected.
src\lib\observable.ts(33,5): error TS7027: Unreachable code detected.
src\lib\observable.ts(79,5): error TS7027: Unreachable code detected.
src\stream\transform.ts(68,7): error TS7027: Unreachable code detected.

https://github.com/falsandtru/arch-stream/blob/v0.0.36/src/lib/message.ts#L126
https://github.com/falsandtru/arch-stream/blob/v0.0.36/src/lib/observable.ts#L33
https://github.com/falsandtru/arch-stream/blob/v0.0.36/src/lib/observable.ts#L79
https://github.com/falsandtru/arch-stream/blob/v0.0.36/src/stream/transform.ts#L68

Assertions will remove by unassert: https://github.com/twada/unassert

[vladima]

I guess my question is: in what situation do you expect this assert to be raised? If compiler can statically prove that this code is unreachable (using a very conservative set of checks) then what is the point of putting the bit of code that will never be triggered. The only situation I can imagine when it might be useful is if you want to make your code more resilient to occasional modifications, i.e. if somebody removes a default case in switch. In this case compiler can still help you: by using--noImplicitReturns you can enforce that all code paths in function should ultimately either return or throw.

[falsandtru]

The only situation I can imagine when it might be useful is if you want to make your code more resilient to occasional modifications, i.e. if somebody removes a default case in switch.

It is right. I feel calm writing that clearly contracted code. I am already using a --noImplicitReturns. It is a great evolution but I cannot trust --noImplicitReturns and unreachable code detection completely because TypeScript has not a few bugs. I don't want to lose an infallible means. Embedded codes are a trustworthy method, and prevention is better than cure.

[vladima]

can you please elaborate: did you step on any issue related to reachability checks or you just suspect that they might exists?

[falsandtru]

I did not step on any issue because my tail assertion work effectively. I explain the background:

That my switch statement simulate the overloaded function for pattern matching like a Haskell. Haskell check the typings and the condition completeness. My tail assertion is embedding for the checking of the condition completeness. --noImplicitReturns and unreachable code detection covered my needs(see following case), but it is unstable. That new features cannot use with manual checking.

In short, must I remove the prevention code immediately to use that new features?

[zpdDG4gta8XKpMCd]

why won't we ask for exhaustive switch and if/then/else that eliminates a possibility of forgotten cases and elses instead?

[falsandtru]

--noImplicitReturns and unreachable code detection does not work in this case:

expected:

function f(n) {
    switch (n) {
        case 0:
            return 0;
        default:
            if (n > 0) {
                return n;
            }
            else {
                return -n;
            }
    }
    // assert(false) or throw error

    function g() {
    }
    // many functions...
    function z() {
    }
}

actual:

function f(n) {
    switch (n) {
        case 0:
            return 0;
        default:
            if (n > 0) {
                return n;
            }
            // forgotten else...
    }
    // assert(false) or throw error

    function g() {
    }
    // many functions...
    function z() {
    }

    return n; // bad hidden code
}

This is a weak point of --noImplicitReturns and unreachable code detection.

--noImplicitReturns and unreachable code detection makes the bad code by rejection of the tail assertion as the prevention code.

[falsandtru]

Please review this point, @vladima @RyanCavanaugh @mhegazy @DanielRosenwasser .

I think, unreachable code detection should be an optional feature because unreachable code detection cannot to be the perfect in JavaScript. JavaScript cannot segregate the main statement block(for detect the tail statement of main flow in function) and the declaration block like a where statement of Haskell. We cannot find the tail statement quickly without an assertion such as return, throw or assert(false). That tail assertion works as a segregation of the declaration block. So if possible, unreachable code detection should be able to disable partially for that tail assertion.

[falsandtru]

Improve to #6009