☂️ Parser bugs and problems

[xunilrj]

Description

Not everything on this list is a bug. They are bullet points that we need to address. We can choose to keep it like it is today.

Tasks

• [ ] Top level return
• [ ] export name from 'a'
• [x] export { \n default as a } from 'a'
• [ ] multiple "export default"
• [ ] const arguments/package and others invalid bindings
• [ ] export anonymous function on .d.ts
• [x] Not all string are directives

Top Level Return

Some node.js scripts are using "top level return".

https://github.com/ant-design/ant-design/blob/master/scripts/check-version-md.js https://stackoverflow.com/questions/28955047/why-does-a-module-level-return-statement-work-in-node-js

https://play.rome.tools/?lineWidth=80&indentStyle=tab&indentWidth=2&typescript=true&jsx=true#cmV0dXJuOw==

Examples: https://github.com/ant-design/ant-design/blob/master/scripts/check-version-md.js#L60
https://github.com/marmelab/react-admin/blob/master/cypress/start.js#L4

Export from

We don't support "export default from" and have a problem with "export { \n default ..." (mind the new line):

https://play.rome.tools/?lineWidth=80&indentStyle=tab&indentWidth=2&typescript=true&jsx=true#ZXhwb3J0IGRlZmF1bHQgZnJvbSAnYSc7CmV4cG9ydCB7IGRlZmF1bHQgYXMgQSB9IGZyb20gJ2EnOwpleHBvcnQgeyAKICAgIGRlZmF1bHQgYXMgQSB9IGZyb20gJ2EnOw==

babel seems to like "export default from 'a';"; ts on the other hand does not.

https://babeljs.io/repl#?browsers=defaults%2C%20not%20ie%2011%2C%20not%20ie_mob%2011&build=&builtIns=false&corejs=3.21&spec=false&loose=false&code_lz=KYDwDg9gTgLgBAE2AMwIYFcA29lQgWzgHJUiBuIA&debug=false&forceAllTransforms=false&shippedProposals=false&circleciRepo=&evaluate=false&fileSize=false&timeTravel=false&sourceType=module&lineWrap=true&presets=env%2Creact%2Cstage-2&prettier=false&targets=&version=7.17.8&externalPlugins=%40babel%2Fplugin-proposal-export-default-from%407.16.7&assumptions=%7B%7D

https://ts-ast-viewer.com/#code/KYDwDg9gTgLgBAE2AMwIYFcA29lQgWzgHJUiBuIA

Another issue is

export getComponentGroupInfo from './getComponentGroupInfo'

https://play.rome.tools/?lineWidth=80&indentStyle=tab&indentWidth=2&typescript=true&jsx=true#ZXhwb3J0IGdldENvbXBvbmVudEdyb3VwSW5mbyBmcm9tICcuL2dldENvbXBvbmVudEdyb3VwSW5mbyc=

JSX

For some reason we can parse "" but not "="

https://play.rome.tools/?lineWidth=80&indentStyle=tab&indentWidth=2&typescript=true&jsx=true#PHNwYW4+PC9zcGFuPgo8c3Bhbj49PC9zcGFuPg==

The problem is that ">=" is lexed as a token and breaks the JSX parsing.

 0: L_ANGLE@0..1 "<" [] []
            1: JSX_NAME@1..5
              0: JSX_IDENT@1..5 "span" [] []
            2: JS_UNKNOWN@5..7
              0: JS_UNKNOWN_MEMBER@5..7
                0: GTEQ@5..7 ">=" [] []

Multiple Export default

For example, react-hook-form uses multiple "export default function". https://github.com/react-hook-form/react-hook-form/blob/master/examples/V7/nestedFields.tsx

This is unanimously frowned upon across all parsers with errors; but running on Stackblitz, for example, actually works. Although the result is not intuitive at all: https://stackblitz.com/edit/js-mnyyat

Const arguments/package and others invalid bindings

This is a little bit silly, but ts and node actually support variables named "arguments" and we do not. https://play.rome.tools/?lineWidth=80&indentStyle=tab&indentWidth=2&typescript=true&jsx=true#Y29uc3QgYXJndW1lbnRzID0gMTs=

https://ts-ast-viewer.com/#code/MYewdgzgLgBAhgJwOYFcC2BTMUIwLwwCMA3EA

Example of use: https://github.com/reduxjs/redux/blob/master/scripts/mangleErrors.js#L74
https://github.com/marmelab/react-admin/blob/master/jest.config.js#L4

Export anonymous function on .d.ts

On .d.ts, the code below is not working.

export default function() {}

error[SyntaxError]: expected an identifier, an array pattern, or an object pattern but instead found '('
  ┌─ parser_smoke_test:1:24
  │
1 │ export default function() {}
  │                        ^ Expected an identifier, an array pattern, or an object pattern here

error[SyntaxError]: A 'declare' function cannot have a function body
  ┌─ parser_smoke_test:1:27
  │
1 │ export default function() {}
  │                           ^^ remove this body

Not all string are directives

We treat lines purely with strings at the beginning of methods as directives. This is not always the case:

function a() {
  'a'
  .something()
}

https://play.rome.tools/?lineWidth=80&indentStyle=tab&indentWidth=2&typescript=true&jsx=true#ZnVuY3Rpb24gYSgpIHsKICAnYScKICAuc29tZXRoaW5nKCkKfQ==

Real case: https://github.com/OnsenUI/OnsenUI/blob/master/onsenui/esm/ons/internal/swipe-reveal.js#L28

[MichaReiser]

Thanks @xunilrj for assembling this list. I've started comparing the issues with the spec

• top level return: Not spec compliment. Neither ScriptBody nor ModuleBody allow for top level return spec.
• export default from. Not yet standardized in ECMA 2022 spec. Currently a stage 1 proposal
• export name from ...: Not standardized spec. Also part of the export default proposal
• JSX: That's funny. I'll have a look
• Multiple default exports: Incorrect to my understanding. Duplicate export names isn't permitted according to the spec
• arguments: Forbidding arguments is correct in strict mode spec
• Directives: Yes, our parsing is incorrect. It's any sequence of expressions spec

[xunilrj]

More problems found in the wild:

• [ ] Optional binding on .d.ts
• [ ] Export anonymous function in .d.ts
• [ ] Initializers in ambient contexts
• [x] Strange unicodes character inside regex

Optional binding on .d.ts

declare namespace A {
  class ErrorsTextOptions {
    separator;
    dataVar;
  }
  export class Ajv {
    errorsText(errors?: string[] | null | undefined, { separator, dataVar }?: ErrorsTextOptions): string;
  }
}

error[SyntaxError]: A binding pattern parameter cannot be optional in an implementation signature.
  ┌─ main.js:7:52
  │
7 │   errorsText(errors?: string[] | null | undefined, { separator, dataVar }?: ErrorsTextOptions): string;
  │                                                    ^^^^^^^^^^^^^^^^^^^^^^    

https://play.rome.tools/?lineWidth=80&indentStyle=tab&indentWidth=2&typescript=true&jsx=false#ZGVjbGFyZSBuYW1lc3BhY2UgQSB7CmNsYXNzIEVycm9yc1RleHRPcHRpb25zIHsKICBzZXBhcmF0b3I7CiAgZGF0YVZhcjsKfQpleHBvcnQgY2xhc3MgQWp2IHsKICBlcnJvcnNUZXh0KGVycm9ycz86IHN0cmluZ1tdIHwgbnVsbCB8IHVuZGVmaW5lZCwgeyBzZXBhcmF0b3IsIGRhdGFWYXIgfT86IEVycm9yc1RleHRPcHRpb25zKTogc3RyaW5nOwp9Cn0=

ts has no problem with this: https://ts-ast-viewer.com/#code/CYUwxgNghgTiAEA7KBbEBnADlMCCC8A3gFCRTrrwCiMMA9jOgCogAeALgPKbsCWdiSiXjx0IbDCjsGAbmIjgUqADVYcgL7E2mBu3hkK8PACsAbkXnwQtBszbsAFNfqMA-AC5R7GL0QBzAG0AXXgAHyQAVwgIMPgIxFAAM18QYAAaIlFxWCkGDMV2FVh4dQ9qG0YWDm4+AXQASk90b18-DWJ1IA

see:

• https://unpkg.com/ajv@8.11.0/dist/core.d.ts
• https://unpkg.com/apollo-cache-inmemory@1.6.6/lib/readFromStore.d.ts

Export anonymous function in .d.ts

export default function (objA: any, objB: any): boolean;

  │
1 │ export default function (objA: any, objB: any): boolean;
  │                         ^ Expected an identifier, an array pattern, or an object pattern here

see: https://unpkg.com/bizcharts@4.1.15/es/utils/shallowEqual.d.ts

Initializers in ambient contexts

I think this is valid since ts version 2.1.

declare namespace A {
    export class ViewHelper {
        readonly isRootView = false;
    }
}

error[SyntaxError]: Initializers are not allowed in ambient contexts.
  ┌─ main.js:3:29
  │
3 │         readonly isRootView = false;  
  │                             ^^^^^^^

https://play.rome.tools/?lineWidth=80&indentStyle=tab&indentWidth=2&typescript=true&jsx=false#ZGVjbGFyZSBuYW1lc3BhY2UgQSB7CiAgICBleHBvcnQgY2xhc3MgVmlld0hlbHBlciB7CiAgICAgICAgcmVhZG9ubHkgaXNSb290VmlldyA9IGZhbHNlOwogICAgfQp9

https://ts-ast-viewer.com/#code/CYUwxgNghgTiAEA7KBbEBnADlMCCC8A3gFDxnwgAemA9jAC7yRTrrwBqAliAO4ASICJhAwipchLhRgNRBACe8TugBKNGvS694AXngAzKBHQgA3OLIBfYpaA

Strange unicode characters inside regex

Our regex parser is getting lost with some "strange unicode" characters.

var matchMonthPatterns = {
        // eslint-disable-next-line no-misleading-character-class
        narrow: /^[يفمئامئ‍ئاسۆند]/i
    }

error[SyntaxError]: expected `,` but instead found `‍`
  ┌─ parser_smoke_test:3:27
  │
3 │         narrow: /^[يفمئامئ‍ئاسۆند]/i
  │                            unexpected

error: unterminated regex literal
  ┌─ parser_smoke_test:3:27
  │
3 │         narrow: /^[يفمئامئ‍ئاسۆند]/i
  │                 -          ...but the line ends here
  │                 │
  │                 a regex literal starts there...

error: Unexpected token `‍`
  ┌─ parser_smoke_test:3:27
  │
3 │         narrow: /^[يفمئامئ‍ئاسۆند]/i
  │

This also bugs the playground.

https://ts-ast-viewer.com/#code/G4QwTgBAtiAuDGALAsgewHa0QBTrApmOgM4QC8EA3gFAR310D0jE+xANgJaYC0AJp2IgARu3w90+AB6weXSRHSoeUQWJAD0Acx5JwIeATC72IYsVoN66cGFQB3AFwRGAPQDagKTBAgmCBRMEBkYIDkYAGAsARBgMxggGNggGJggPRgALqMnJZ0AL5AA

see: https://unpkg.com/date-fns@2.28.0/locale/ug/_lib/match/index.js

[ematipico]

I found a couple of bugs. We fail to parse two files inside the TypeScript repository:

https://github.com/microsoft/TypeScript/blob/main/src/lib/webworker.generated.d.ts

error[SyntaxError]: Illegal use of `arguments` as an identifier in strict mode
     ┌─ main.js:5189:61
     │
5189 │     setInterval(handler: TimerHandler, timeout?: number, ...arguments: any[]): number;
     │                                                             ^^^^^^^^^

error[SyntaxError]: Illegal use of `arguments` as an identifier in strict mode
     ┌─ main.js:5190:60
     │
5190 │     setTimeout(handler: TimerHandler, timeout?: number, ...arguments: any[]): number;
     │                                                            ^^^^^^^^^

The other file is: https://github.com/microsoft/TypeScript/blob/main/src/lib/dom.generated.d.ts for the same reason

[MichaReiser]

I found a couple of bugs. We fail to parse two files inside the TypeScript repository:

https://github.com/microsoft/TypeScript/blob/main/src/lib/webworker.generated.d.ts

error[SyntaxError]: Illegal use of `arguments` as an identifier in strict mode
     ┌─ main.js:5189:61
     │
5189 │     setInterval(handler: TimerHandler, timeout?: number, ...arguments: any[]): number;
     │                                                             ^^^^^^^^^

error[SyntaxError]: Illegal use of `arguments` as an identifier in strict mode
     ┌─ main.js:5190:60
     │
5190 │     setTimeout(handler: TimerHandler, timeout?: number, ...arguments: any[]): number;
     │                                                            ^^^^^^^^^

The other file is: https://github.com/microsoft/TypeScript/blob/main/src/lib/dom.generated.d.ts for the same reason

I guess the issue here is that we parse js files as modules by default. That means they get parsed in strict mode, in which arguments is disallowed. Fixing this would likely require that our parser correctly infers whatever a file is a module or a script depending if it uses any ES module syntax or not, but that's not as straightforward.

[ematipico]

This is a .d.ts file though.

[MichaReiser]

This is a .d.ts file though.

Here it's the same. We, by default, assume it's a module and not a script.

[ematipico]

@xunilrj this is probably related to https://github.com/rome/tools/issues/2960 ? Can we track everything in one single issue?