Skip to content
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

add a flag that enforces boolean expressions where booleans are expected #7306

Closed
zpdDG4gta8XKpMCd opened this issue Feb 29, 2016 · 9 comments
Labels
Out of Scope This idea sits outside of the TypeScript language design constraints Suggestion An idea for TypeScript

Comments

@zpdDG4gta8XKpMCd
Copy link

a flag that enforces:

  • any expression with the following operators: ||, &&, ! to be boolean
  • a condition expression to be of type boolean in the if statement and in the conditional expression (ternary operator)

related: #7256

@RyanCavanaugh
Copy link
Member

This would be extremely non-idiomatic for JavaScript (as in, I expect almost no codebases to successfully compile under such a flag). I think TSLint would be the place for a rule like this.

@basarat
Copy link
Contributor

basarat commented Mar 1, 2016

Agree with ryan. Just today :

image

It is highly idiomatic to do such checks in JSX/TSX/React 🌹

@mhegazy mhegazy added Suggestion An idea for TypeScript Out of Scope This idea sits outside of the TypeScript language design constraints labels Mar 1, 2016
@mhegazy mhegazy closed this as completed Mar 1, 2016
@myitcv
Copy link

myitcv commented Mar 4, 2016

@Aleksey-Bykov palantir/tslint#253 (amongst others)

This is blocked on palantir/tslint#680 however

@zpdDG4gta8XKpMCd
Copy link
Author

@myitcv Thanks for letting me know. We at our project stopped using TsLint for this exact reason, the rules can only validate the syntax, not the semantics. We developed a homemade rule runner that gets a type checker and runs against the a type checked code of the whole application rather than an AST of a single file.

It would not be a problem to put such rule together.

@myitcv
Copy link

myitcv commented Mar 5, 2016

@Aleksey-Bykov in the absence of support for this kind of rule in tslint have you shared your custom rule runner?

@zpdDG4gta8XKpMCd
Copy link
Author

I cannot share the real runner, but I can outline how to make one:

import * as ts from 'typescript';
import * as fs from 'fs';

export function run(
    projectFileOrFolder: string
): void {

    const options: ts.CompilerOptions = {
        project: projectFileOrFolder
    };

    const host = tlb.toCompilerHost(options);

    const program = ts.createProgram(
        [],
        options,
        host
    );

    const emitted = program.emit();
    if (emitted.diagnostics.length > 0) { throw new Error('program has problems'); }

    const checker = program.getTypeChecker();
    const rootFileNames = program.getRootFileNames();

    rootFileNames.map(rootFileName => {
        const rootNode = program.getSourceFile(rootFileName);
        analyze(rootNode, checker);
    });
}

function analyze(rootNode: ts.Node, checker: ts.TypeChecker): void {
   // TODO: put your rule here
}

function toCompilerHost(
    options: ts.CompilerOptions
): ts.CompilerHost {
    return {
        readFile: (fileName: string): string => {
            return fs.readFileSync(fileName, 'utf8');
        },
        fileExists: (fileName: string): boolean => {
            return fs.existsSync(fileName);
        },
        getSourceFile: function (filepath: string) {
            try {
                return ts.createSourceFile(
                    filepath,
                    fs.readFileSync(filepath, 'utf8'),
                    options.target,
                    true
                );
            }
            catch (e) {
                return undefined;
            }
        },
        writeFile: () => null,
        getDefaultLibFileName: () => 'lib.d.ts',
        useCaseSensitiveFileNames: () => true,
        getCanonicalFileName: (filename: string) => filename,
        getCurrentDirectory: () => '',
        getNewLine: () => '\n'
    };
}

@zpdDG4gta8XKpMCd
Copy link
Author

then comes the simplified rule

function analyze(rootNode: ts.Node, checker: ts.TypeChecker): void {
    visitEachNode(rootNode, node => {
        if (node.kind === ts.SyntaxKind.IfStatement) {
           checkIfStatement(<ts.IfStatement>node, checker);
        } else if (node.kind === ts.SyntaxKind.ConditionalExpression) {
           checkConditionalExpression(<ts.ConditionalExpression>node, checker);
        }
    });   
}

function checkIfStatement(node: ts.IfStatement, checker: ts.TypeChecker): void {
    var type = checker.getTypeAtLocation(node.expression);
    if ((type.flags & ts.TypeFlags.Boolean) !== ts.TypeFlags.Boolean) {
        console.warn('if expression is not boolean');
    }
}

function checkConditionalExpression(node: ts.ConditionalExpression, checker: ts.TypeChecker): void {
    var type = checker.getTypeAtLocation(node.condition);
    if ((type.flags & ts.TypeFlags.Boolean) !== ts.TypeFlags.Boolean) {
        console.warn('if expression is not boolean');
    }
}

function visitEachNode(rootNode: ts.Node, visit: (node: ts.Node) => void): void {
    ts.forEachChild(rootNode, child => {
        visit(child);
        visitEachNode(child, visit);
    });
}

@zpdDG4gta8XKpMCd
Copy link
Author

also take a look at the official example on how to make a linter: https://github.com/Microsoft/TypeScript/wiki/Using-the-Compiler-API#traversing-the-ast-with-a-little-linter

@myitcv
Copy link

myitcv commented Mar 6, 2016

@Aleksey-Bykov thanks for the pointers

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Out of Scope This idea sits outside of the TypeScript language design constraints Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants