From f603c0626d99aea256a3b0b7eb8c644f0759a54f Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Sun, 31 Jan 2016 02:12:01 +0900 Subject: [PATCH] Fix: `no-missing-*` should work for relative paths to a directory (fixes #21) --- lib/util/exists.js | 4 +- lib/util/import-target.js | 88 ++++++++++++++++++---- tests/fixtures/no-missing/bar/main.js | 2 + tests/fixtures/no-missing/foo/main.js | 2 + tests/fixtures/no-missing/foo/package.json | 3 + tests/fixtures/no-missing/index.js | 2 + tests/lib/rules/no-missing-import.js | 42 +++++++++++ tests/lib/rules/no-missing-require.js | 36 +++++++++ 8 files changed, 163 insertions(+), 16 deletions(-) create mode 100644 tests/fixtures/no-missing/bar/main.js create mode 100644 tests/fixtures/no-missing/foo/main.js create mode 100644 tests/fixtures/no-missing/foo/package.json create mode 100644 tests/fixtures/no-missing/index.js diff --git a/lib/util/exists.js b/lib/util/exists.js index b0f86316..eb947d8f 100644 --- a/lib/util/exists.js +++ b/lib/util/exists.js @@ -17,10 +17,10 @@ var fs = require("fs"); //------------------------------------------------------------------------------ /** - * Checks whether or not the file which is at given path exists. + * Checks whether or not the file of a given path exists. * * @param {string} filePath - A file path to check. - * @returns {boolean} `true` if the file which is at given path exists. + * @returns {boolean} `true` if the file of a given path exists. */ module.exports = function exists(filePath) { try { diff --git a/lib/util/import-target.js b/lib/util/import-target.js index 4879e88f..8a0e9679 100644 --- a/lib/util/import-target.js +++ b/lib/util/import-target.js @@ -10,36 +10,96 @@ // Requirements //------------------------------------------------------------------------------ +var fs = require("fs"); var path = require("path"); var exists = require("./exists"); +var getPackageJson = require("./get-package-json"); //------------------------------------------------------------------------------ // Helpers //------------------------------------------------------------------------------ +/** + * Checks whether or not a given path is a directory. + * + * @param {string} filePath - A file path to check. + * @returns {boolean} `true` if the path is a directory. + */ +function isDirectory(filePath) { + try { + return fs.statSync(filePath).isDirectory(); + } + catch (err) { + return false; + } +} + +/** + * Resolve a given path as a file with given extensions. + * + * @param {string} filePath - A path to resolve. + * @param {string[]} exts - Extensions that it checks whether or not the file exists. + * @returns {string|null} The resolved path. Or `null` if failed to resolve. + */ +function tryExtentions(filePath, exts) { + for (var i = 0; i < exts.length; ++i) { + var ext = exts[i]; + if (exists(filePath + ext)) { + return filePath + ext; + } + } + + return null; +} + +/** + * Resolve a given path as a file. + * + * @param {string} filePath - A path to resolve. + * @param {string[]} exts - Extensions that it checks whether or not the file exists. + * @returns {string|null} The resolved path. Or `null` if failed to resolve. + */ +function resolveAsFile(filePath, exts) { + if (exists(filePath)) { + return filePath; + } + return tryExtentions(filePath, exts); +} + +/** + * Resolve a given path as a directory. + * + * @param {string} filePath - A path to resolve. + * @param {string[]} exts - Extensions that it checks whether or not the file exists. + * @returns {string|null} The resolved path. Or `null` if failed to resolve. + */ +function resolveAsDirectory(filePath, exts) { + if (!isDirectory(filePath)) { + return null; + } + + var p = getPackageJson(path.join(filePath, "package.json")); + if (p && path.dirname(p.filePath) === filePath && p.main) { + return resolveAsFile(path.join(filePath, p.main), exts); + } + return tryExtentions(path.join(filePath, "index"), exts); +} + /** * Resolves the file. * * @param {string} basedir - The path of base directory to resolve relative path. * @param {string} name - The name of an import target. * @param {string[]} exts - Extensions that it checks whether or not the file exists. - * @returns {string} Resolved path. + * @returns {string} The resolved path. */ function resolve(basedir, name, exts) { var resolvedPath = path.resolve(basedir, name); - - // Checks with extensions. - if (!exists(resolvedPath)) { - for (var i = 0; i < exts.length; ++i) { - var ext = exts[i]; - - if (exists(resolvedPath + ext)) { - return resolvedPath + ext; - } - } - } - - return resolvedPath; + return ( + resolveAsFile(resolvedPath, exts) || + resolveAsDirectory(resolvedPath, exts) || + resolvedPath + ); } /** diff --git a/tests/fixtures/no-missing/bar/main.js b/tests/fixtures/no-missing/bar/main.js new file mode 100644 index 00000000..ed469db8 --- /dev/null +++ b/tests/fixtures/no-missing/bar/main.js @@ -0,0 +1,2 @@ +"use strict"; +console.log("hello!"); diff --git a/tests/fixtures/no-missing/foo/main.js b/tests/fixtures/no-missing/foo/main.js new file mode 100644 index 00000000..ed469db8 --- /dev/null +++ b/tests/fixtures/no-missing/foo/main.js @@ -0,0 +1,2 @@ +"use strict"; +console.log("hello!"); diff --git a/tests/fixtures/no-missing/foo/package.json b/tests/fixtures/no-missing/foo/package.json new file mode 100644 index 00000000..bd18befe --- /dev/null +++ b/tests/fixtures/no-missing/foo/package.json @@ -0,0 +1,3 @@ +{ + "main": "main.js" +} diff --git a/tests/fixtures/no-missing/index.js b/tests/fixtures/no-missing/index.js new file mode 100644 index 00000000..ed469db8 --- /dev/null +++ b/tests/fixtures/no-missing/index.js @@ -0,0 +1,2 @@ +"use strict"; +console.log("hello!"); diff --git a/tests/lib/rules/no-missing-import.js b/tests/lib/rules/no-missing-import.js index 46736603..c94aff40 100644 --- a/tests/lib/rules/no-missing-import.js +++ b/tests/lib/rules/no-missing-import.js @@ -150,6 +150,32 @@ ruleTester.run("no-missing-import", rule, { code: "import a from './a';", ecmaFeatures: {modules: true}, parserOptions: {sourceType: "module"} + }, + + // Relative paths to a directory should work. + { + filename: fixture("test.js"), + code: "import a from '.';", + ecmaFeatures: {modules: true}, + parserOptions: {sourceType: "module"} + }, + { + filename: fixture("test.js"), + code: "import a from './';", + ecmaFeatures: {modules: true}, + parserOptions: {sourceType: "module"} + }, + { + filename: fixture("test.js"), + code: "import a from './foo';", + ecmaFeatures: {modules: true}, + parserOptions: {sourceType: "module"} + }, + { + filename: fixture("test.js"), + code: "import a from './foo/';", + ecmaFeatures: {modules: true}, + parserOptions: {sourceType: "module"} } ], invalid: [ @@ -203,6 +229,22 @@ ruleTester.run("no-missing-import", rule, { ecmaFeatures: {modules: true}, parserOptions: {sourceType: "module"}, errors: ["\"./c\" is not found."] + }, + + // Relative paths to a directory should work. + { + filename: fixture("test.js"), + code: "import a from './bar';", + ecmaFeatures: {modules: true}, + parserOptions: {sourceType: "module"}, + errors: ["\"./bar\" is not found."] + }, + { + filename: fixture("test.js"), + code: "import a from './bar/';", + ecmaFeatures: {modules: true}, + parserOptions: {sourceType: "module"}, + errors: ["\"./bar/\" is not found."] } ] }); diff --git a/tests/lib/rules/no-missing-require.js b/tests/lib/rules/no-missing-require.js index c281821a..0116aace 100644 --- a/tests/lib/rules/no-missing-require.js +++ b/tests/lib/rules/no-missing-require.js @@ -164,6 +164,28 @@ ruleTester.run("no-missing-require", rule, { filename: "tests/fixtures/no-missing/test.js", code: "require('./a');", env: {node: true} + }, + + // Relative paths to a directory should work. + { + filename: fixture("test.js"), + code: "require('.');", + env: {node: true} + }, + { + filename: fixture("test.js"), + code: "require('./');", + env: {node: true} + }, + { + filename: fixture("test.js"), + code: "require('./foo');", + env: {node: true} + }, + { + filename: fixture("test.js"), + code: "require('./foo/');", + env: {node: true} } ], invalid: [ @@ -210,6 +232,20 @@ ruleTester.run("no-missing-require", rule, { code: "require('./c');", env: {node: true}, errors: ["\"./c\" is not found."] + }, + + // Relative paths to a directory should work. + { + filename: fixture("test.js"), + code: "require('./bar');", + env: {node: true}, + errors: ["\"./bar\" is not found."] + }, + { + filename: fixture("test.js"), + code: "require('./bar/');", + env: {node: true}, + errors: ["\"./bar/\" is not found."] } ] });