jquery / esprima

ECMAScript parsing infrastructure for multipurpose analysis
http://esprima.org
BSD 2-Clause "Simplified" License
7.04k stars 786 forks source link

Enable noImplicitAny and type everything properly #2066

Open Timmmm opened 3 years ago

Timmmm commented 3 years ago

Using Typescript is great (when type declarations are shipped anyway), but it's kind of rubbish that so many of the types are just any. It would be a good idea to enable the noImplicitAny option and then fix all of the errors. For example here are the types for the main functions:

export declare function parse(code: string, options: any, delegate: any): any;
export declare function parseModule(code: string, options: any, delegate: any): any;
export declare function parseScript(code: string, options: any, delegate: any): any;
export declare function tokenize(code: string, options: any, delegate: any): any;

Not very useful!

Timmmm commented 3 years ago

I had a good go at this, but ran into two issues:

For example there's this code (types added):


    visitComment(node: NodeComment, metadata: Metadata): void {
        const type = (node.type[0] === 'L') ? 'Line' : 'Block';

But that's literally the only place where 'Line' and 'Block' are used. Everywhere else is 'LineComment' or 'BlockComment'. Maybe it's just a bug (see! If you use Typescript properly you get fewer bugs!).

        switch (expr.type) {
...
            case Syntax.AssignmentExpression:
                expr.type = Syntax.AssignmentPattern;
                delete expr.operator;

That is pretty horrible. Not just because Typescript can't deal with it. Now you have an object whose .type field does not match instanceof. You could probably force this to work, but still... ew.

In any case, I think really only the original authors have enough knowledge to do this in a reasonable amount of time because it requires inferring intent.

I also found that the type declarations for the currently released version are not as bad as I though. Weirdly they assume that the AST conforms to the Espree type definitions, but you don't actually use those in Esprima for some reason.

I can put my attempt up somewhere if you like.