Eslint-plugin-import: `newline-after-import` breaks again

[ljharb]

Now that #386 is resolved, I'm now getting the following:

Cannot read property 'range' of undefined
TypeError: Cannot read property 'range' of undefined
    at containsNodeOrEqual ($PWD/node_modules/eslint-plugin-import/lib/rules/newline-after-import.js:23:19)
    at $PWD/node_modules/eslint-plugin-import/lib/rules/newline-after-import.js:111:34
    at Array.forEach (native)
    at $PWD/node_modules/eslint-plugin-import/lib/rules/newline-after-import.js:104:22
    at Array.forEach (native)
    at EventEmitter.ProgramExit ($PWD/node_modules/eslint-plugin-import/lib/rules/newline-after-import.js:100:14)
    at emitOne (events.js:82:20)
    at EventEmitter.emit (events.js:169:7)
    at NodeEventGenerator.leaveNode ($PWD/node_modules/eslint/lib/util/node-event-generator.js:49:22)
    at CodePathAnalyzer.leaveNode ($PWD/node_modules/eslint/lib/code-path-analysis/code-path-analyzer.js:627:23)
    at CommentEventGenerator.leaveNode ($PWD/node_modules/eslint/lib/util/comment-event-generator.js:110:23)

[benmosher]

Seems to be () => require('x') && require('y') that gets into this state.

I'm disqualifying ArrowFunctionExpressions (more generally, single-statement scopes) from newline-after-import. Strictly an improvement, since they are currently broken.

[ljharb]

I don't have any && require or || require in my codebase, fwiw, so I'm not sure if this will fix the problem or not.

[benmosher]

I'm not either. If it doesn't, it'd be ideal to have a failing test case.

This was the first thing I could come up with just syntax-ing around that threw the same exception, and while I could imagine other execution states that could throw the same error, I was unable to imagine syntax (and consequently, test cases) to go with them.

[benmosher]

Published v1.11.0 to npm. Fingers crossed.

[ljharb]

Sadly this still occurs. How can I help figure out which file is causing the issue?

[benmosher]

Bummer. 😞

Best idea I have had: some sort of find . -exec eslint that stops when ESLint returns a nonzero exit code?

[ljharb]

What about modifying the rule in eslint-plugin-import to console.log the filename being worked on, and i can just look for the last one? Do you know how I can get the filename from a scope?

[ljharb]

@benmosher OK, I think I found it:

    switch (foo) {
      case '1':
        return require('../path/to/file1.jst.hbs')(renderData, options);
      case '2':
        return require('../path/to/file2.jst.hbs')(renderData, options);
      case '3':
        return require('../path/to/file3.jst.hbs')(renderData, options);
      case '4':
        return require('../path/to/file4.jst.hbs')(renderData, options);
      case '5':
        return require('../path/to/file5.jst.hbs')(renderData, options);
      case '6':
        return require('../path/to/file6.jst.hbs')(renderData, options);
      case '7':
        return require('../path/to/file7.jst.hbs')(renderData, options);
      case '8':
        return require('../path/to/file8.jst.hbs')(renderData, options);
      case '9':
        return require('../path/to/file9.jst.hbs')(renderData, options);
      case '10':
        return require('../path/to/file10.jst.hbs')(renderData, options);
      case '11':
        return require('../path/to/file11.jst.hbs')(renderData, options);
      case '12':
        return something();
      default:
        return somethingElse();
    }

[ljharb]

It appears that statementWithRequireCall is undefined here: https://github.com/benmosher/eslint-plugin-import/blob/master/src/rules/newline-after-import.js#L100 - if i do an early return when that's falsy, it seems to fix the problem (but no idea if that's the right fix).

[benmosher]

Makes sense. Same thing was happening with the arrows; the scope position comes back as -1 and thus the statement is undefined.

Thanks for the repro, I'll debug and try to trace it back from there. May add a debug logger to get the file name as you've suggested, as well.

[benmosher]

Turns out the sourceType matters for this. Added a test with the code you provided, and it passed. After playing with parserOptions and parser for a little bit, I discovered that the module parsing goal matters for this, for reasons I don't understand.

Will publish as 1.11.1, and then we can reopen if it's still broken (for this or new reasons).

Did add logging as well, so DEBUG=eslint-plugin-import:* eslint [...] should print out some helpful messages from now on, for this rule.