dudykr/stc

Different comment values in TSC CLI and test file, Which one should We trust?

sunrabbit123 opened this issue · 14 comments

class C {
  private p: string;
}

var str: string;
var bool: boolean;
var num: number;
var strOrNum: string | number;
var strOrBool: string | boolean;
var numOrBool: number | boolean;
var strOrNumOrBool: string | number | boolean;
var strOrC: string | C;
var numOrC: number | C;
var boolOrC: boolean | C;
var emptyObj: {};
var c: C;

// A type guard of the form typeof x === s,
// where s is a string literal with any value but 'string', 'number' or 'boolean',
//  - when true, removes the primitive types string, number, and boolean from the type of x, or
//  - when false, has no effect on the type of x.

if (typeof strOrC === "Object") {
  c = strOrC; // C
} else {
  var r2: string = strOrC; // string
}
if (typeof numOrC === "Object") {
  c = numOrC; // C
} else {
  var r3: number = numOrC; // number
}
if (typeof boolOrC === "Object") {
  c = boolOrC; // C
} else {
  var r4: boolean = boolOrC; // boolean
}
if (typeof strOrC === ("Object" as string)) {
  // comparison is OK with cast
  c = strOrC; // error: but no narrowing to C
} else {
  var r5: string = strOrC; // error: no narrowing to string
}

if (typeof strOrNumOrBool === "Object") {
  let q1: {} = strOrNumOrBool; // {}
} else {
  let q2: string | number | boolean = strOrNumOrBool; // string | number | boolean
}

what is the value of strOrC at the var r2: string = strOrC

kdy1 commented

I think strOrC should be never or string | C intuitively, but tsc may behave differently

In the absence of an actual_ty, TSC will not be a type_guard in an else statement.
However, comment wants to be a type_guard.
Feeding it a NotObject will fix it, but I'm not sure what's a good direction.

https://github.com/dudykr/stc/blob/main/crates/stc_ts_type_checker/tests/conformance/expressions/typeGuards/typeGuardOfFormTypeOfOther.error-diff.json
If you want it to be string | C, then your error-diff.json needs to be modified.

@kdy1

If you're right, and if TSC's findings are correct, the error is not an extra error, but it is being labeled as an extra error(Line 9).


if (typeof strOrC === "Object") {
c = strOrC; // C
}
else {
var r2: string = strOrC; // string
}

kdy1 commented

I'm considering using the result of tsc cli instead of official test reference, because that's how tsc actually behave. @sunrabbit123 What do you think about this?

kdy1 commented

It will be similar to #1040
If we decide to go with the CLI, tsc test suite will invoke tsc on the first run. (first run only because tsc is super slow)

@kdy1

I think so, too
Becasue I trust tsc cli feature, but I don't think apply every update

Unfortunately, the only official specification is tsc cli

Also, if the specification of tsc cli is changed, it will be casue that a lot of code will get type error
That's why I think we can trust tsc cli


ps. We have some additional extra errors that occurred for similar reasons
I would like to ask you how you intend to approach this work

A bit confusing type check here...
typeof xyz === "Object" should be lowercase 'object' (if stc implementation is case-insensitive, it might and probably would cause bugs in the consumer code):

> typeof {}
< 'object'
> typeof {} === 'Object'
< false

I suppose, it's from theese test cases?
https://github.com/dudykr/stc/blob/693cf5a891c5580542811b906616f0c15d0dd0fc/crates/stc_ts_type_checker/tests/conformance/expressions/typeGuards/typeGuardOfFormTypeOfOther.ts
https://github.com/dudykr/stc/blob/693cf5a891c5580542811b906616f0c15d0dd0fc/crates/stc_ts_type_checker/tests/conformance/expressions/typeGuards/typeGuardOfFormTypeOfNotEqualHasNoEffect.ts

@ankhzet
this issue mean next case

if (typeof strOrC === "Object") {
c = strOrC; // C
}
else {
var r2: string = strOrC; // string
}

this code should not work type guard, but comment mean work type guard
of course, tsc not work type guard

The comment is also attached to the recent typescript test code.

Ok, got it.
Found the relevant PR microsoft/TypeScript#5442
Seems, narrowing to 'string' on the line 25 is intentional, invalidity of the typeguard is solved by the union intersection check itself failing with TS2367 error, as far as i understand.

The essence of this issue is what to do when the results of the TSC differ from the test annotations.

In fact, line 25 is inferred to be string | C.
But the code comment is written as string.

 if (typeof strOrC === "Object") { 
     c = strOrC; // C 
 } 
 else { 
     strOrC; // expected string, but this type is string | C
     var r2: string = strOrC;
 } 

playground

As far as i understand, the test was written when strictNullChecks was not yet introduced (TS v2.0). If you test the code declaring strOrC as declare var strOrC: string | C; or initialize it, f.e. var strOrC: string | C = undefined as any;, then strOrC would be narrowed to string (this is how i tested the code in the playground).
Alternatively you can disable the flag in the playground, so it behaves the same way it was at the time when the test case was introduced: playground - strOrC is narrowed to string, no error.

So the issue might be a discrepancy between the tsconfig used for reference tests execution ty the TS team and the default tsc/playground config you usually use for testing, i guess.

omg, thanks your report
I missing important information about options

Issue has been created in relation to it.

The reason for the discussion of the issue is deemed to have disappeared, so we close it.