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

Enable noImplicitAny and type everything properly #2066

Open
Timmmm opened this issue Jan 28, 2021 · 1 comment
Open

Enable noImplicitAny and type everything properly #2066

Timmmm opened this issue Jan 28, 2021 · 1 comment

Comments

@Timmmm
Copy link

Timmmm commented Jan 28, 2021

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 Timmmm mentioned this issue Jan 28, 2021
12 tasks
@Timmmm
Copy link
Author

Timmmm commented Jan 29, 2021

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

  • It's really hard to tell what the types of loads of the functions in parser.ts take - a lot of them seem to take RawToken, but then later you find they get passed a Node.Node. Also some functions like delegate are very ambiguous.

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!).

  • More critically, in some places the type of a node is transmuted, e.g. in reinterpretExpressionAsPattern():
        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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant