Repository: import-js/eslint-plugin-import
Pull request #2305 (2021-11-17 22:00:41)

[Fix] `importType`: avoid crashing on a non-string
While writing imports in VSCode recently, I started to get errors throwm
from eslint that appear to originate from eslint-plugin-import.

Here's the strack trace from the output panel:

```
[Error - 3:52:43 PM] TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined
    at validateString (internal/validators.js:124:11)
    at isAbsolute (path.js:1029:5)
    at isAbsolute (/path/to/repo/node_modules/eslint-plugin-import/src/core/importType.js:17:10)
    at typeTest (/path/to/repo/node_modules/eslint-plugin-import/src/core/importType.js:92:7)
    at resolveImportType (/path/to/repo/node_modules/eslint-plugin-import/src/core/importType.js:105:10)
    at reportIfMissing (/path/to/repo/node_modules/eslint-plugin-import/src/rules/no-extraneous-dependencies.js:170:7)
    at commonjs (/path/to/repo/node_modules/eslint-plugin-import/src/rules/no-extraneous-dependencies.js:267:7)
    at checkSourceValue (/path/to/repo/node_modules/eslint-module-utils/moduleVisitor.js:29:5)
    at checkSource (/path/to/repo/node_modules/eslint-module-utils/moduleVisitor.js:34:5)
    at /path/to/repo/node_modules/eslint/lib/linter/safe-emitter.js:45:58
```

I am able to trigger this consistently when writing an import, before I
start the open quote, like this:

```
import foo from
```

I found https://github.com/import-js/eslint-plugin-import/issues/1931
which pointed at a problem in the resolvers causing this problem. We do
use some custom resolvers with this plugin, so I tried disabling that
but the problem persisted.

Thankfully, I was able to reproduce this error in the test suite.

It would be good if this plugin handled this scenario gracefully instead
of throwing an error.

ljharb: I don't think it's possible to have a test here, since this is invalid syntax, so eslint can't parse it. That said, I can repurpose this into a bug fix given that stack trace, thanks!

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0d4d920a..fbb5c809 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,23 +6,26 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
 
 ## [Unreleased]
 
+### Fixed
+- `importType`: avoid crashing on a non-string' ([#2305], thanks [@ljharb])
+
 ## [2.25.3] - 2021-11-09
 
 ### Fixed
-- [`extensions`]: ignore unresolveable type-only imports ([#2270], [#2271], [@jablko])
-- `importType`: fix `isExternalModule` calculation ([#2282], [@mx-bernhard])
-- [`no-import-module-exports`]: avoid false positives with a shadowed `module` or `exports` ([#2297], [@ljharb])
+- [`extensions`]: ignore unresolveable type-only imports ([#2270], [#2271], thanks [@jablko])
+- `importType`: fix `isExternalModule` calculation ([#2282], thanks [@mx-bernhard])
+- [`no-import-module-exports`]: avoid false positives with a shadowed `module` or `exports` ([#2297], thanks [@ljharb])
 
 ### Changed
-- [Docs] [`order`]: add type to the default groups ([#2272], [@charpeni])
-- [readme] Add note to TypeScript docs to install appropriate resolver ([#2279], [@johnthagen])
-- [Refactor] `importType`: combine redundant `isScoped` and `isScopedModule` ([@ljharb])
-- [Docs] HTTP => HTTPS ([#2287], [@Schweinepriester])
+- [Docs] [`order`]: add type to the default groups ([#2272], thanks [@charpeni])
+- [readme] Add note to TypeScript docs to install appropriate resolver ([#2279], thanks [@johnthagen])
+- [Refactor] `importType`: combine redundant `isScoped` and `isScopedModule` (thanks [@ljharb])
+- [Docs] HTTP => HTTPS ([#2287], thanks [@Schweinepriester])
 
 ## [2.25.2] - 2021-10-12
 
 ### Fixed
-- [Deps] update `eslint-module-utils` for real this time ([#2255])
+- [Deps] update `eslint-module-utils` for real this time ([#2255], thanks [@ljharb])
 
 ## [2.25.1] - 2021-10-11
 
@@ -942,6 +945,7 @@ for info on changes for earlier releases.
 
 [`memo-parser`]: ./memo-parser/README.md
 
+[#2305]: https://github.com/import-js/eslint-plugin-import/pull/2305
 [#2297]: https://github.com/import-js/eslint-plugin-import/pull/2297
 [#2287]: https://github.com/import-js/eslint-plugin-import/pull/2287
 [#2282]: https://github.com/import-js/eslint-plugin-import/pull/2282
diff --git a/src/core/importType.js b/src/core/importType.js
index 085ce658..3e579b94 100644
--- a/src/core/importType.js
+++ b/src/core/importType.js
@@ -14,7 +14,7 @@ function baseModule(name) {
 }
 
 export function isAbsolute(name) {
-  return nodeIsAbsolute(name);
+  return typeof name === 'string' && nodeIsAbsolute(name);
 }
 
 // path is defined only when a resolver resolves to a non-standard path
diff --git a/src/rules/no-absolute-path.js b/src/rules/no-absolute-path.js
index 4111fd7e..fe0a1b6d 100644
--- a/src/rules/no-absolute-path.js
+++ b/src/rules/no-absolute-path.js
@@ -13,7 +13,7 @@ module.exports = {
 
   create(context) {
     function reportIfAbsolute(source) {
-      if (typeof source.value === 'string' && isAbsolute(source.value)) {
+      if (isAbsolute(source.value)) {
         context.report(source, 'Do not import modules using an absolute path');
       }
     }


Test patch:
diff --git a/tests/src/core/importType.js b/tests/src/core/importType.js
index 528377e6..4f4a862a 100644
--- a/tests/src/core/importType.js
+++ b/tests/src/core/importType.js
@@ -1,7 +1,7 @@
 import { expect } from 'chai';
 import * as path from 'path';
 
-import importType, { isExternalModule, isScoped } from 'core/importType';
+import importType, { isExternalModule, isScoped, isAbsolute } from 'core/importType';
 
 import { testContext, testFilePath } from '../utils';
 
@@ -256,3 +256,14 @@ describe('importType(name)', function () {
     expect(isScoped('@a/abc/def')).to.equal(true);
   });
 });
+
+describe('isAbsolute', () => {
+  it('does not throw on a non-string', () => {
+    expect(() => isAbsolute()).not.to.throw();
+    expect(() => isAbsolute(null)).not.to.throw();
+    expect(() => isAbsolute(true)).not.to.throw();
+    expect(() => isAbsolute(false)).not.to.throw();
+    expect(() => isAbsolute(0)).not.to.throw();
+    expect(() => isAbsolute(NaN)).not.to.throw();
+  });
+});