Skip to content

Type guard narrowing fails on else #4823

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ghost opened this issue Sep 16, 2015 · 1 comment
Closed

Type guard narrowing fails on else #4823

ghost opened this issue Sep 16, 2015 · 1 comment
Labels
By Design Deprecated - use "Working as Intended" or "Design Limitation" instead Duplicate An existing issue was already created

Comments

@ghost
Copy link

ghost commented Sep 16, 2015

The type of the render method in the below code should be string as all code paths return string. Type guards correctly narrow the type within the if blocks, but fail to account for the unconditional return within them. Each if statement should effectively remove any non-array type from the list of possible types following it.

Wrapping the rest of the method in else {} does not seem to help.

enum Person {
    First = 1,
    Second = 2,
    Third = 3
}

enum Gender {
    Indeterminate,
    Masculine,
    Feminine
}

type NounConjugationDefinitionGender = [ string, string, string ];
type NounConjugationDefinitionPerson = [ string|NounConjugationDefinitionGender, string|NounConjugationDefinitionGender, string|NounConjugationDefinitionGender ];
type NounConjugationDefinitionPlural = [ string|NounConjugationDefinitionPerson, string|NounConjugationDefinitionPerson ];
type NounConjugationDefinition = string|NounConjugationDefinitionPlural;

class NounConjugation {
    constructor( nounConjugationDefinition: NounConjugationDefinition ) {
        this.nounConjugationDefinition = nounConjugationDefinition;
    }
    public nounConjugationDefinition: NounConjugationDefinition;

    public render( count: number, person: Person, gender: Gender ) {
        const pluralIndex = count === 1 ? 0 : 1,
            genderIndex = <number>gender,
            personIndex = (<number>person) - 1;
        let current = this.nounConjugationDefinition;
        if( !Array.isArray( current ) ) {
            // correct: current is string
            return current;
        }
        // current must be Array, cannot be string (previous if statement had unconditional return)
        // this eliminates string from string|NounConjugationDefinitionPlural
        // therefore, it must be NounConjugationDefinitionPlural
        current = current[ pluralIndex ];
        // type of current should now be string|NounConjugationDefinitionPerson
        if( !Array.isArray( current ) ) {
            // correct: current is string
            return current;
        }
        // current must be Array, cannot be string (previous if statement had unconditional return)
        // this eliminates string from string|NounConjugationDefinitionPerson
        // therefore, it must be NounConjugationDefinitionPerson
        current = current[ personIndex ];
        // type of current should now be string|NounConjugationDefinitionGender
        if( !Array.isArray( current ) ) {
            // correct: current is string
            return current;
        }
        // current must be Array, cannot be string (previous if statement had unconditional return)
        // this eliminates string from string|NounConjugationDefinitionGender
        // therefore, it must be NounConjugationDefinitionGender
        return current[ genderIndex ]; // should be string
    }
}

// Example usage:
const conjugation = new NounConjugation( [ [ 'I', 'you', [ 'it', 'he', 'she' ] ], [ 'we', 'you', 'they' ] ] );
console.log( conjugation.render( 1, Person.First, Gender.Indeterminate ) ); // I
console.log( conjugation.render( 1, Person.First, Gender.Masculine ) ); // I
console.log( conjugation.render( 1, Person.First, Gender.Feminine ) ); // I
console.log( conjugation.render( 2, Person.First, Gender.Indeterminate ) ); // we
console.log( conjugation.render( 2, Person.First, Gender.Masculine ) ); // we
console.log( conjugation.render( 2, Person.First, Gender.Feminine ) ); // we
console.log( conjugation.render( 1, Person.Second, Gender.Indeterminate ) ); // you
console.log( conjugation.render( 1, Person.Second, Gender.Masculine ) ); // you
console.log( conjugation.render( 1, Person.Second, Gender.Feminine ) ); // you
console.log( conjugation.render( 2, Person.Second, Gender.Indeterminate ) ); // you
console.log( conjugation.render( 2, Person.Second, Gender.Masculine ) ); // you
console.log( conjugation.render( 2, Person.Second, Gender.Feminine ) ); // you
console.log( conjugation.render( 1, Person.Third, Gender.Indeterminate ) ); // it
console.log( conjugation.render( 1, Person.Third, Gender.Masculine ) ); // he
console.log( conjugation.render( 1, Person.Third, Gender.Feminine ) ); // she
console.log( conjugation.render( 2, Person.Third, Gender.Indeterminate ) ); // they
console.log( conjugation.render( 2, Person.Third, Gender.Masculine ) ); // they
console.log( conjugation.render( 2, Person.Third, Gender.Feminine ) ); // they
@DanielRosenwasser
Copy link
Member

@errorx666 thanks for filing this, but type guards currently don't use flow control as their mechanism; however, there is an issue tracking the idea: #2388.

@DanielRosenwasser DanielRosenwasser added By Design Deprecated - use "Working as Intended" or "Design Limitation" instead Duplicate An existing issue was already created labels Sep 16, 2015
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
By Design Deprecated - use "Working as Intended" or "Design Limitation" instead Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

1 participant