no-useless-path-segments: Add detectUselessIndex option

docs/rules/no-useless-path-segments.md

@@ -11,6 +11,9 @@ my-project
 ├── app.js
 ├── footer.js
 ├── header.js
+└── helpers.js
+└── helpers
+    └── index.js
 └── pages
     ├── about.js
     ├── contact.js
@@ -30,6 +33,7 @@ import "../pages/about.js"; // should be "./pages/about.js"
 import "../pages/about"; // should be "./pages/about"
 import "./pages//about"; // should be "./pages/about"
 import "./pages/"; // should be "./pages"
+import "./pages/index"; // should be "./pages" (except if there is a ./pages.js file)
 ```
 
 The following patterns are NOT considered problems:
@@ -46,3 +50,24 @@ import ".";
 import "..";
 import fs from "fs";
 ```
+
+## Options
+
+### detectUselessIndex
+
+If you want to detect unnecessary `/index` imports in your paths, you can enable the option `detectUselessIndex`. By default it is set to `false`:
+```js
+"import/no-useless-path-segments": ["error", {
+  detectUselessIndex: true,
+}]
+```
+
+Additionally to the patterns described above, the following imports are considered problems if `detectUselessIndex` is enabled:
+
+```js
+// in my-project/app.js
+import "./helpers/index"; // should be "./helpers" (not auto-fixable because of helpers.js)
+import "./pages/index"; // should be "./pages" (auto-fixable)
+```
+
+Note: `detectUselessIndex` detects only ambiguous imports for `.js` files if you haven't specified all resolved file extensions. See [Settings: import/extensions](https://github.com/benmosher/eslint-plugin-import#importextensions) for details.

src/rules/no-useless-path-segments.js

@@ -31,9 +31,24 @@ function normalize(fn) {
   return toRelativePath(path.posix.normalize(fn))
 }
 
-const countRelativeParents = (pathSegments) => pathSegments.reduce(
-  (sum, pathSegment) => pathSegment === '..' ? sum + 1 : sum, 0
-)
+function countRelativeParents(pathSegments) {
+  return pathSegments.reduce((sum, pathSegment) => pathSegment === '..' ? sum + 1 : sum, 0)
+}
+
+function getFileExtensions(settings) {
+  if (
+    settings &&
+    settings['import/resolver'] &&
+    settings['import/resolver'].node &&
+    settings['import/resolver'].node.extensions
+  ) {
+    return settings['import/resolver'].node.extensions
+  } else {
+    return [
+      '.js',
+    ]
+  }
+}
 
 module.exports = {
   meta: {
@@ -47,6 +62,7 @@ module.exports = {
         type: 'object',
         properties: {
           commonjs: { type: 'boolean' },
+          detectUselessIndex: { type: 'boolean' },
         },
         additionalProperties: false,
       },
@@ -55,25 +71,27 @@ module.exports = {
     fixable: 'code',
     messages: {
       uselessPath: 'Useless path segments for "{{ path }}", should be "{{ proposedPath }}"',
+      uselessAmbiguousPath: 'Useless path segments for "{{ path }}", import path would be ' +
+        'ambiguous: "{{ ambiguousPath1 }}" and "{{ ambiguousPath2 }}"',
     },
   },
 
   create(context) {
     const currentDir = path.dirname(context.getFilename())
-    const config = context.options[0]
+    const options = context.options[0]
 
     function checkSourceValue(source) {
       const { value: importPath } = source
 
-      function report(proposedPath) {
+      function reportWithProposedPath(proposedPath) {
         context.report({
           node: source,
           messageId: 'uselessPath',
           data: {
             path: importPath,
             proposedPath,
           },
-          fix: fixer => fixer.replaceText(source, JSON.stringify(proposedPath)),
+          fix: fixer => proposedPath && fixer.replaceText(source, JSON.stringify(proposedPath)),
         })
       }
 
@@ -87,7 +105,35 @@ module.exports = {
       const normedPath = normalize(importPath)
       const resolvedNormedPath = resolve(normedPath, context)
       if (normedPath !== importPath && resolvedPath === resolvedNormedPath) {
-        return report(normedPath)
+        return reportWithProposedPath(normedPath)
+      }
+
+      // Path contains unnecessary /index
+      if (options && options.detectUselessIndex && importPath.endsWith('index')) {
+        const parentDirectory = path.dirname(importPath)
+
+        if (parentDirectory !== '.' && parentDirectory !== '..') {
+          // Try to find ambiguous imports (to avoid breaking import paths with auto-fixing)
+          const fileExtensions = getFileExtensions(context.settings)
+
+          const resolvedAmbiguousFileExtension = fileExtensions.find(fileExtension =>
+            resolve(`${parentDirectory}${fileExtension}`, context))
+
+          if (resolvedAmbiguousFileExtension) {
+            // No fix possible as import is ambiguous (e.g. /bar.js and /bar/index.js exist both)
+            return context.report({
+              node: source,
+              messageId: 'uselessAmbiguousPath',
+              data: {
+                path: importPath,
+                ambiguousPath1: `${parentDirectory}${resolvedAmbiguousFileExtension}`,
+                ambiguousPath2: parentDirectory,
+              },
+            })
+          }
+        }
+
+        return reportWithProposedPath(parentDirectory)
       }
 
       // Path is shortest possible + starts from the current directory --> Return directly
@@ -113,7 +159,7 @@ module.exports = {
       }
 
       // Report and propose minimal number of required relative parents
-      return report(
+      return reportWithProposedPath(
         toRelativePath(
           importPathSplit
             .slice(0, countExpectedRelativeParents)
@@ -123,6 +169,6 @@ module.exports = {
       )
     }
 
-    return moduleVisitor(checkSourceValue, config)
+    return moduleVisitor(checkSourceValue, options)
   },
 }

tests/src/rules/no-useless-path-segments.js

@@ -17,6 +17,9 @@ function runResolverTests(resolver) {
       test({ code: 'import "."' }),
       test({ code: 'import ".."' }),
       test({ code: 'import fs from "fs"' }),
+      test({ code: 'import fs from "fs"' }),
+      test({ code: 'import "../index"' }), // detectUselessIndex is false by default
+      test({ code: 'import "../index.js"', options: [{ detectUselessIndex: true }] }), // .js should be detected by extension rule
     ],
 
     invalid: [
@@ -61,6 +64,26 @@ function runResolverTests(resolver) {
         options: [{ commonjs: true }],
         errors: [ 'Useless path segments for "./deep//a", should be "./deep/a"'],
       }),
+      test({
+        code: 'require("./importType/index")',
+        options: [{ commonjs: true, detectUselessIndex: true }],
+        errors: ['Useless path segments for "./importType/index", should be "./importType"'],
+      }),
+      test({
+        code: 'require("./index")',
+        options: [{ commonjs: true, detectUselessIndex: true }],
+        errors: ['Useless path segments for "./index", should be "."'],
+      }),
+      test({
+        code: 'require("../index")',
+        options: [{ commonjs: true, detectUselessIndex: true }],
+        errors: ['Useless path segments for "../index", should be ".."'],
+      }),
+      test({
+        code: 'require("./bar/index")',
+        options: [{ commonjs: true, detectUselessIndex: true }],
+        errors: ['Useless path segments for "./bar/index", import path would be ambiguous: "./bar.js" and "./bar"'],
+      }),
 
       // esmodule
       test({
@@ -95,8 +118,28 @@ function runResolverTests(resolver) {
         code: 'import "./deep//a"',
         errors: [ 'Useless path segments for "./deep//a", should be "./deep/a"'],
       }),
-     ],
-   })
+      test({
+        code: 'import "./importType/index"',
+        options: [{ detectUselessIndex: true }],
+        errors: ['Useless path segments for "./importType/index", should be "./importType"'],
+      }),
+      test({
+        code: 'import "./index"',
+        options: [{ detectUselessIndex: true }],
+        errors: ['Useless path segments for "./index", should be "."'],
+      }),
+      test({
+        code: 'import "../index"',
+        options: [{ detectUselessIndex: true }],
+        errors: ['Useless path segments for "../index", should be ".."'],
+      }),
+      test({
+        code: 'import "./bar/index"',
+        options: [{ detectUselessIndex: true }],
+        errors: ['Useless path segments for "./bar/index", import path would be ambiguous: "./bar.js" and "./bar"'],
+      }),
+    ],
+  })
 }
 
 ['node', 'webpack'].forEach(runResolverTests)