eslint / typescript-eslint-parser

An ESLint custom parser which leverages TypeScript ESTree to allow for ESLint to lint TypeScript source code.
Other
915 stars 92 forks source link

declare global causing parse errors #570

Closed milesj closed 5 years ago

milesj commented 5 years ago

What version of TypeScript are you using?

3.0

What version of typescript-eslint-parser are you using?

21.0.1

What code were you trying to parse?

declare global {
  namespace NodeJS {
    interface Process {
      beemo: {
        context: any;
        tool: BeemoTool;
      };
    }
  }
}

What did you expect to happen?

It parse.

What happened?

It fails with this error.

/Users/Miles/Sites/beemo/packages/core/src/index.ts
  17:15  error  Parse errors in imported module './types': Cannot read property '0' of undefined (undefined:undefined)  import/export

This has happened on 2 projects now, both because of declare global.

milesj commented 5 years ago

Probably more related to export * coupled with declare global.

Here's the index file: https://github.com/milesj/beemo/blob/master/packages/core/src/index.ts#L17

And types file: https://github.com/milesj/beemo/blob/master/packages/core/src/types.ts#L62

armano2 commented 5 years ago

@milesj this issue is in https://github.com/benmosher/eslint-plugin-import

this plugin is not supporting typescript https://github.com/benmosher/eslint-plugin-import/issues?q=is%3Aopen+is%3Aissue+label%3Atypescript

can you report this issue there?

milesj commented 5 years ago

This was working before a recent parser upgrade though. Do you know exactly what changed that may have caused this?

armano2 commented 5 years ago

@milesj hmm i did some testing with plain eslint configuration, without your wrappers, on core package from your repository. And i was not able to reproduce this issue.

{
  "parser": "typescript-eslint-parser",
  "parserOptions": {
    "ecmaVersion": 8,
    "sourceType": "module",
    "jsx": true,
    "useJSXTextNode": true
  },
  "env": {
      "browser": true,
      "node": true
  },
  "settings": {
    "import/resolver": {
      "node": {
        "extensions": [
          ".js",
          ".jsx",
          ".ts",
          ".tsx"
        ]
      }
    }
  },
  "extends": [
    "eslint:recommended",
    "plugin:import/errors",
    "plugin:import/warnings"
  ],
  "plugins": [
    "import",
    "typescript"
  ],
  "rules": {
    "no-unused-vars": "off",
    "no-undef": "off",
    "import/export": "error"
  }
}
$ eslint --ext .ts src/
Done in 1.43s.
milesj commented 5 years ago

This is the eslint config I'm using: https://github.com/milesj/build-tool-config/blob/master/packages/config/configs/eslint.js

Nothing atm stands out, except for.

'import/parsers': {
      'typescript-eslint-parser': ['.ts', '.tsx'],
},
armano2 commented 5 years ago

yea i found it, and looks like its issue in AST after all xd

armano2 commented 5 years ago
TypeError: Cannot read property '0' of undefined
    at isGlobalAugmentation (******\typescript-eslint-parser\analyze-scope.js:32:26)
    at Referencer.TSModuleDeclaration (******\typescript-eslint-parser\analyze-scope.js:578:13)
    at Referencer.Visitor.visit (******\typescript-eslint-parser\node_modules\esrecurse\esrecurse.js:104:34)
    at Referencer.Visitor.visitChildren (******\typescript-eslint-parser\node_modules\esrecurse\esrecurse.js:83:38)
    at Referencer.Program (******\typescript-eslint-parser\node_modules\eslint-scope\lib\referencer.js:423:14)
    at Referencer.Visitor.visit (******\typescript-eslint-parser\node_modules\esrecurse\esrecurse.js:104:34)
    at module.exports (******\typescript-eslint-parser\analyze-scope.js:685:16)
    at Object.parseForESLint (******\typescript-eslint-parser\parser.js:74:26)
    at Object.exports.parse (******\typescript-eslint-parser\parser.js:79:17)
    at parse (******\test\node_modules\eslint-module-utils\parse.js:36:17)
milesj commented 5 years ago

Oh nice! Luckily we found it.

armano2 commented 5 years ago

this is related to https://github.com/JamesHenry/typescript-estree/issues/27

mysticatea commented 5 years ago

The cause seems that eslint-plugin-import doesn't give implicit tokens option to the parser.

armano2 commented 5 years ago

yes, and no

issue is most likely somewhere in parser

when i have export and declare in one file tokens are empty

export const foo = "";
declare global {
}

but when only one of them tokens are present

mysticatea commented 5 years ago

ESLint gives some implicit options: https://github.com/eslint/eslint/blob/5d451c510c15abc41b5bb14b4955a7db96aeb100/lib/linter.js#L436

  {
        loc: true,
        range: true,
        raw: true,
        tokens: true,
        comment: true,
        eslintVisitorKeys: true,
        eslintScopeManager: true,
        filePath
    }

eslint-plugin-import has some lacking implicit options: https://github.com/benmosher/eslint-plugin-import/blob/798eed7e559adab2eac07bf1b3e3518d4b7a4296/utils/parse.js#L9

  // always include and attach comments
  parserOptions.comment = true
  parserOptions.attachComment = true

  // attach node locations
  parserOptions.loc = true

  // provide the `filePath` like eslint itself does, in `parserOptions`
  // https://github.com/eslint/eslint/blob/3ec436ee/lib/linter.js#L637
  parserOptions.filePath = path

This difference is the cause.

However, probably we should not assume those options. We can have the required options in this parser itself.

armano2 commented 5 years ago

ok, i tested it, options.tokens = true; in parser.js#L35, is solving issue.

armano2 commented 5 years ago

@mysticatea why do check it there anyway?

we can do

if (node.declare && node.id && node.id.type === 'Identifier' && node.id.name === 'global') {
}
mysticatea commented 5 years ago

See JamesHenry/typescript-estree#27

armano2 commented 5 years ago

@milesj for now you can patch it by adding tokens: true, to parserOptions: https://github.com/milesj/build-tool-config/blob/master/packages/config/configs/eslint.js#L31

milesj commented 5 years ago

Thanks both for the deep debugging! Will give that patch a try.

armano2 commented 5 years ago

@platinumazure this issue got fixed in last release

platinumazure commented 5 years ago

Thanks @armano2, next time you can ensure this happens yourself by including "fixes ####" in the commit summary, commit message, or PR body :smile:

Closing as this is resolved.