microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.1k stars 12.37k forks source link

Function declarations inside function expressions should inherit control flow narrowings of the parent function expression #36436

Open ackvf opened 4 years ago

ackvf commented 4 years ago

I am trying to avoid typing null checks for properties that are formerly optional on the outer interface, but are then assigned with default values for the whole context of the function body.

I used a type guard for it, the problem is that the guarded type is lost in a function, but not lost in an arrow function, even though it can't be hoisted before the type guard.

I don't want to introduce new variables for every function parameter in a similar manner and I don't want to call other functions with casting as printMenu(options as DefinedOptions).

TypeScript Version: 3.7.3, 3.8.0-beta

Search Terms: typescript type guard function arrow function Code

export interface Options {
  header?: string
  border?: boolean
  pageSize?: number
  helpMessage?: string
  showKeypress?: boolean
}

interface DefinedOptions extends Options {
  pageSize: number
  helpMessage: string
}

const isType = <T>(arg: any): arg is T => true

export default async function menu(options: Options) {

  options // Options

  options.pageSize = options.pageSize ?? 0
  options.helpMessage = options.helpMessage ?? 'Default message'
  if (!isType<DefinedOptions>(options)) return null

  options // DefinedOptions

  return new Promise((resolve, reject) => {

    function handleMenuKeypress(key: any) {
      options // Options  -  should be DefinedOptions

      printMenu(options) // error
    }

    const candleMenuKeypress = (key: any) => {
      options // DefinedOptions

      printMenu(options) // no error
    }
  })
}

function printMenu(options: DefinedOptions) {

}

Playground Link

Expected behavior:

Type should be DefinedOptions in both.

Actual behavior:

Type is Options in function but DefinedOptions in arrow function.

Related:

10927

nmain100 commented 4 years ago

This is a simplified form of what you're looking at:

interface A { s: string }
interface B extends A { t: boolean }

declare function isb(a: A): asserts a is B;

function foo(a: A) {
  isb(a);
  a; // B
  const f = () => {
    a; // B, since f is not hoisted and couldn't be called before its definition
  }
  function g() {
    a; // A, since definition of g is hoisted and could be called elsewhere
  }
  const h = () => {
    a; // B
    const hf = () => {
      a; // B
    }
    function hg() {
      a; // A, hg is hoisted but it couldn't be called ouside of h?
    }
  }
}

Playground

I'd say that typescript is being overly pessimistic in the case of a regular function declaration inside of an arrow function, and that hg should have the same narrowing that's present in h, even given what's discussed in https://github.com/microsoft/TypeScript/issues/32300.

RyanCavanaugh commented 4 years ago

Thanks for the simplified version @nmain100

The only bug here is hg - it should be B given its position in the CFA graph. Function expressions and function declarations behave differently because one is hoisted and the other isn't.

AleksandrGilmanov commented 4 years ago

is it the same bug as:

function somethingWentWrong(): boolean {
  let definitellyIsTrue: boolean = false;
  ['true'].forEach(() => { definitellyIsTrue = true });
  return definitellyIsTrue == true;
}

console.log(somethingWentWrong());

Playground Link

If you run it with playgroynd you find error "This condition will always return 'false' since the types 'false' and 'true' have no overlap.(2367)"

Or I need to make new issue for this?

nmain100 commented 4 years ago

@AleksandrGilmanov That's a duplicate of #9998; typescript doesn't know that the function passed to forEach is called at that point, and if all narrowings were pessimistically reset after every function call, narrowing would be mostly useless.

falsandtru commented 4 years ago

Another standard case:

function foo(opts?: object) {
  opts = opts || {};
  opts; // object
  return () => opts; // object | undefined <- wrong
}
function bar(a?: string | object, opts?: object) {
  if (typeof a === 'object') [a, opts] = [undefined, a];
  a; // string | undefined
  return () => a; // string | undefined | object <- wrong
}

http://www.typescriptlang.org/play/?ts=3.8.0-dev.20200124&ssl=1&ssc=1&pln=10&pc=2#code/GYVwdgxgLglg9mABMOcAUcAOUDOB+ALkTgCMArAU2gEpEBvAKEWOx0QF4XdEAfH+gL4BuJlxxDEAeknFyVKKIBOFKCEVI0tdgD4xE6bMrReicABMKwGGApnEAHgC0iAO6KEAcwYCGoSLAREEgBDRTRgwkQcKEVrDxNSIygAGjFIxPlaRmYYYEQ0KABPTAo4POCOdk4AcgzoatoAbWDUrFwAXQ5ERvNLa1tU4PaRZmD9GWjYsHj+XqsbMyUVNQ0tXTGpCZi4kzn+u346qAdnN09vIA

vasilii-kovalev commented 4 years ago

Hello there!

Could anybody take a look at this playgroud, please? I left appropriate comments about an error at the bottom of the file. It seems that my issue is partially related to this one, though it is not about function types (function declaration or function expression).

Should I create a separate issue or they are related?

Thanks in advance.

mheripsos commented 2 years ago

Some other update must have resolved this partially because I get the following on 4.5.4:

function workingExample<T>(
    arg: unknown,
    validator: (arg: unknown) => asserts arg is T,
    callme: (arg: T) => void
): void {
    validator(arg);
    const fn = () => {
        callme(arg); // no error. arg: T
    };
}

This seems like the original issue. However, when asserting a member of an obj, it still fails, but very confusingly. The type comments are coming from VSCode hover on TS 4.5.4:

function nonworkingExample<T>(
    arg: {member: unknown},
    validator: (member: unknown) => asserts member is T,
    callme: (member: T) => void
): void {
    validator(arg.member);
    callme(arg.member); // no error. on hover -> arg: {member: unknown}, but arg.member: T ???
    const fn = () => {
        callme(arg.member); // error. arg.member: unknown
    };
}

Edit: Found a work-around for now which is fine I guess:

function woraround<T>(
    arg: {member: unknown},
    validator: (member: unknown) => asserts member is T,
    callme: (member: T) => void
): void {
    const member = arg.member;
    validator(member);
    callme(member); // no error. member: T
    const fn = () => {
        callme(member); // no error. member: T
    };
}