Unresolved extensions

[benmosher]

Fixes #295 and #271.

Actually made the rule work report on violations in spite of file resolution; it will use the source path if the file doesn't exist. I think this is probably ideal, in contrast to other rules that ignore unresolved files (in spite of my previous comments).

I also added an early exit for builtins (i.e. path) regardless of the options, built on @jfmengels' importType infrastructure. I figure even with always mode enabled, it should be possible to import from core modules.

Also, note that extension is "" when the provided path is null or has no extension, thus the added !extension check for the extension-enforcing branch.

Feedback from @vvo, @lo1tuma, @sindresorhus, @jfmengels appreciated.

[benmosher]

Seeing no feedback (what, not an exciting PR? 😉) I'll probably merge this tomorrow or Thursday.

[jfmengels]

It's not a part of the repo I'm familiar with (like most of it), but I'll try and take a look tonight ;)

[lo1tuma]

Thanks for fixing this @benmosher. Code LGTM.

[jfmengels]

I'm discovering the rule here, but isn't disallowing import express from 'express/index.js'; an anti-pattern? You're not supposed to know the internal structure of a package.

I would suggest removing it the extension check for packages. And if you're doing that, then you can have an early exit for external type imports, just like for builtins. I know it's not necessarily in the scope, so this can be discussed later, but wanted to mention it here as it might simplify things a bit.

[jfmengels]

What is the expected behavior if you do import thing from "non-package.js" with option "always"?

[benmosher]

Re: express/index.js: that is the spec for always mode, at the moment. I also think it would make sense to allow externals, at least in some mode, but I figured we could let someone ask for that.

Re: non-package.js: if it resolved to a file (mysteriously), and the file was non-package.js, it would not report. If it did not resolve, it also wouldn't report. Oh, I see now... Yeah, I think there is a bug there. If it resolved to some index.js, it might not report, but I think it should according to the spec.