Declared class methods fail rules that depend on function body

[soda0289]

Eslint: 3.15 (with eslint-scope#master)
TypeScript: next
typescipt-eslint-parser: master

What code were you trying to parse?

declare namespace FF {
    class Foo extends Bar.Baz {
        far(): any;
    }
}

or

declare module "FF" {
    class Foo extends Bar.Baz {
        far(): any;
    }
}

or

declare class Foo extends Bar.Baz {
    far(): any;
}

What did you expect to happen?
It should parse

What happened?
Exception:

Cannot read property 'type' of null
TypeError: Cannot read property 'type' of null
    at EventEmitter.onCodePathStart (/home/reyad/Workspace/eslint/lib/rules/array-callback-return.js:187:34)
    at emitTwo (events.js:92:20)
    at EventEmitter.emit (events.js:172:7)
    at processCodePathToEnter (/home/reyad/Workspace/eslint/lib/code-path-analysis/code-path-analyzer.js:357:30)
    at CodePathAnalyzer.enterNode (/home/reyad/Workspace/eslint/lib/code-path-analysis/code-path-analyzer.js:604:9)
    at CommentEventGenerator.enterNode (/home/reyad/Workspace/eslint/lib/util/comment-event-generator.js:98:23)
    at Controller.enter (/home/reyad/Workspace/eslint/lib/eslint.js:928:36)
    at Controller.__execute (/home/reyad/Workspace/eslint/node_modules/estraverse/estraverse.js:397:31)
    at Controller.traverse (/home/reyad/Workspace/eslint/node_modules/estraverse/estraverse.js:501:28)
    at Controller.Traverser.controller.traverse (/home/reyad/Workspace/eslint/lib/util/traverser.js:36:33)

We should detect if the parent is being declared and not use FunctionExpression to define functions that have no body. Similar to what is done with functions in a namespace.

[corbinu]

@soda0289 So just ran a smoke test with the latest eslint#master which I switched to eslint-scope and the latest version of this repo with the your #166, #165, and #163 PRs added in

I set ALL eslint rules to warn plus all the rules for these plugins:

• eslint-plugin-import
• eslint-plugin-lodash
• eslint-plugin-mysticatea
• eslint-plugin-node
• eslint-plugin-promise
• eslint-plugin-unicorn

Plus obviously eslint-plugin-typescript

The projects I smoke tested:

• angular@2.4.7
• glimmer@0.21.1
• vscode@
• nativescript@2.5.0
• vscode@1.9.1
• nativescript grocery sample
• some random definitions from DefiantlyTyped

Obviously there are quite a few rules showing warnings when they should not. However the super good news is only 3 of the plugin rules fail and no eslint rules now fail!!!!!!

I am super excited about this

Not that I expect them fixed here but just an FYI the failing rules are:

• node/no-unsupported-features // Blows up all
• import/newline-after-import //Blows up nativescript, vscode, vscode type files
• unicorn/custom-error-definition // Blows up nativescript

Just wanted to update you. Amazing work!

[soda0289]

@corbinu I have been using your smoke test and have found several issues. I have also linted the typescript code base without error which is pretty cool. Thanks for taking the time to create it!

I will take a look at those plugins and see if they I can fix them up.

[corbinu]

@soda0289 Oh I didn't know anybody was using it will push the latest version. It has gotten a lot more complex so need to just make it totally automated again.

Thanks I am going to work on if I can replicate each of those projects tslint configs and open issues for bad warnings.

[soda0289]

@JamesHenry Should we update the README and start accepting issues for rules producing incorrect errors/warnings?

[corbinu]

@soda0289 Sorry yes I am happy to hold off. Or maybe we should just open one issue either here or on the typescript plugin to track all known incorrect rules.

[soda0289]

@corbinu I'd like to write a test suite for integration tests between typescript parser and eslint rules, which I think should be part of this parser. We already have an issue #77 for tracking some rule failures. Hold off until we hear from @JamesHenry, see if he's ok with opening issues here, adding them to #77, or posting them in the typescript plugin.

[corbinu]

@soda0289 Ok either way I will finish up my update to the smoke tests and start figuring out what rules are broke and how to make. At this point I just want to figure out how much and what work needs to be done to make this a valid replacement to TSLint for those projects

[JamesHenry]

Thanks, guys!

It's my view that we just look to get everything to a place where it is not throwing errors. Some of these known issues have been around for a long time, delayed by the idea of a perfect solution. I think we should do whatever we can now to resolve those, and I will give some proper thought to your PRs @soda0289. Thanks, as always.

I also want to refactor the codebase, and massively improve its documentation.

If we do accept PRs which we deem to be "workaround" in nature, I would like to document why we are doing it within the code.

Naturally, user-facing documentation is also due an update.

Feel free to record any of the issues you mentioned against this repo for now. I'll think about what we want to do in the future.

Cheers!