mysticatea / eslint-plugin-node

Additional ESLint's rules for Node.js
MIT License
960 stars 169 forks source link

Cannot read property 'indexOf' of undefined writing import statement #207

Open crystalfp opened 4 years ago

crystalfp commented 4 years ago

Moved here from microsoft/vscode-eslint #897

  1. Have a types.d.ts file defining and exporting the interface MyInterface.
  2. In a ts file start writing the import statement import {MyInterface} from
  3. At this point a notification appears with the following text:
    ESLint: Cannot read property 'indexOf' of undefined
    Occurred while linting D:\Projects\IdeaIgniterEnvironment\client\east-viewmodel.ts:3.
    Please see the 'ESLint' output channel for details.

    The output channel shows the following stack trace:

    [Error - 6:05:10 AM] TypeError: Cannot read property 'indexOf' of undefined
    Occurred while linting D:\Projects\IdeaIgniterEnvironment\client\east-viewmodel.ts:3
    at stripImportPathParams (D:\Projects\IdeaIgniterEnvironment\node_modules\eslint-plugin-node\lib\util\strip-import-path-params.js:8:20)
    at ExportAllDeclaration,ExportNamedDeclaration,ImportDeclaration,ImportExpression (D:\Projects\IdeaIgniterEnvironment\node_modules\eslint-plugin-node\lib\util\visit-import.js:55:40)
    at D:\Projects\IdeaIgniterEnvironment\node_modules\eslint\lib\linter\safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (D:\Projects\IdeaIgniterEnvironment\node_modules\eslint\lib\linter\safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (D:\Projects\IdeaIgniterEnvironment\node_modules\eslint\lib\linter\node-event-generator.js:254:26)
    at NodeEventGenerator.applySelectors (D:\Projects\IdeaIgniterEnvironment\node_modules\eslint\lib\linter\node-event-generator.js:283:22)
    at NodeEventGenerator.enterNode (D:\Projects\IdeaIgniterEnvironment\node_modules\eslint\lib\linter\node-event-generator.js:297:14)
    at CodePathAnalyzer.enterNode (D:\Projects\IdeaIgniterEnvironment\node_modules\eslint\lib\linter\code-path-analysis\code-path-analyzer.js:634:23)
    at D:\Projects\IdeaIgniterEnvironment\node_modules\eslint\lib\linter\linter.js:936:32

    Not critical, but annoying. Thanks for looking! mario

Extension version: 2.0.15 VS Code version: Code 1.42.0 (ae08d5460b5a45169385ff3fd44208f431992451, 2020-02-06T10:51:34.058Z) OS version: Windows_NT x64 10.0.18363

mysticatea commented 4 years ago

Thank you for your report.

Sounds like a bug in @typescript-eslint/parser rather than this plugin. The code import {MyInterface} from must be a parsing error and don't pass invalid AST to plugin rules.

crystalfp commented 4 years ago

Well, as suggested I moved it to the parser issues (typescript-eslint/typescript-eslint #1616). But I'm not entirely convinced the error is not inside this plugin, because the error is generated here:

stripImportPathParams (D:\Projects\IdeaIgniterEnvironment\node_modules\eslint-plugin-node\lib\util\strip-import-path-params.js:8:20)
crystalfp commented 4 years ago

As suspected this seems to be an issue with this plugin at:

eslint-plugin-node\lib\util\visit-import.js:55:40
skjnldsv commented 4 years ago

Same here @mysticatea

aldeed commented 4 years ago

EDIT: Nevermind, I see that my issue was fixed here and released in v11. I was still using v10.

I'm getting this error, and in my case it's because of something like this:

const importPath = path.join(a, b);
await import(importPath);

The problematic code is here in visit-import.js:

[[
            "ExportAllDeclaration",
            "ExportNamedDeclaration",
            "ImportDeclaration",
            "ImportExpression",
        ]](node) {
            const sourceNode = node.source
            const name = sourceNode && stripImportPathParams(sourceNode.value)
            if (name && (includeCore || !resolve.isCore(name))) {
                targets.push(new ImportTarget(sourceNode, name, options))
            }
        },

sourceNode.value is passed to stripImportPathParams with the assumption that sourceNode is always a literal and sourceNode.value will always be the string. However, for a dynamic import (ImportExpression), that isn't necessarily true. sourceNode may be an identifier because it's dynamic.

pagarwal-ignitetech commented 3 years ago

I see the issue still exist in latest version. how to fix it. Below is the code

import { entrypoint } from '@trilogy-group/alphacoachbot-import-utils';
import { loadCredentials } from './commonLitAPI';
import { getLogger } from 'log4js';

const logger = getLogger();
logger.level = 'all';

exports.handler = async event => {
  if (event.operation) {
    event.learningAppName = 'CommonLit';
    return entrypoint(event, require(`./${event.operation}`), loadCredentials);
  } else {
    throw new Error('An operation should be provided');
  }
};

on running eslint , i get

" --> stderr: Oops! Something went wrong! :("
" --> stderr: ESLint: 7.32.0"
" --> stderr: "
" --> stderr: TypeError: Cannot read property 'indexOf' of undefined"
" --> stderr: Occurred while linting /tmp/github.com/trilogy-group/alpha-coach-bot/data/amplify/backend/function/importCommonLitData/src/index.ts:2"
" --> stderr: at stripImportPathParams (/node_modules/eslint-plugin-node/lib/util/strip-import-path-params.js:8:20)"
" --> stderr: at ExportAllDeclaration,ExportNamedDeclaration,ImportDeclaration,ImportExpression (/node_modules/eslint-plugin-node/lib/util/visit-import.js:55:40)"
" --> stderr: at /node_modules/eslint/lib/linter/safe-emitter.js:45:58"
" --> stderr: at Array.forEach (<anonymous>)"
" --> stderr: at Object.emit (/node_modules/eslint/lib/linter/safe-emitter.js:45:38)"
jaredcm commented 3 years ago

I was able to silence the exception by adding the following in visit-import.js:

if (sourceNode && sourceNode.parent.type === 'ImportDeclaration' && sourceNode.type !== 'Literal') {
 return;
}

It is similar to the check above it, but in that case it looks for 'ImportExpression'.

pagarwal-ignitetech commented 3 years ago

I cannot change the codebase, I need it to be fixed in plugin new release

jaredcm commented 3 years ago

@pagarwal-ignitetech My comment was intended to point the maintainers to the problem and supply a possible fix.