eslint-plugin-import icon indicating copy to clipboard operation
eslint-plugin-import copied to clipboard

Finding never imported files in no-unused-modules.ignoreExports

Open DamienCassou opened this issue 3 years ago • 9 comments

Hi,

I'm using no-unused-modules with unusedExports and missingExports both true. My test files are in the ignoreExports array of no-unused-modules because they don't export anything explicitly: they add test suites by calling describe(). All tests files must be imported in tests/tests.js to be part of the test suite.

Expected: Adding a test file that is not imported in tests/tests.js should trigger an eslint error because the file is useless.

Actual: No eslint error is reported because all test files are ignored.

This might be related to #1326 and #1452.

DamienCassou avatar Aug 24 '21 06:08 DamienCassou

“test files are ignored” shouldn’t be a thing in the rule. Are you sure your config doesn’t use overrides to disable the rule for tests?

(Also, in virtually every codebase, test files are ran by virtue of matching a file pattern, and are never imported)

ljharb avatar Aug 24 '21 14:08 ljharb

“test files are ignored” shouldn’t be a thing in the rule. Are you sure your config doesn’t use overrides to disable the rule for tests?

I'm not sure what you mean. I explicitly said in the first paragraph that my test files are in the ignoreExports array:

{
  "import/no-unused-modules": [
    "error",
    {
      unusedExports: true,
      missingExports: true,
      ignoreExports:
        // List of files not exporting anything:
        [
          "js/tests/**/*-tests.js",
          ...
        ],
    },
  ],
}

The test files are in this array to avoid error messages saying that these files don't export anything. Still, I want to make sure they are imported.

DamienCassou avatar Aug 24 '21 14:08 DamienCassou

ahh, gotcha. In that case, it sounds like maybe you want to omit ignoreExports, but use overrides to target only test files and ignoreExports in them - that way non-test files still check the test files' exports? (i'm not 100% sure this will work, but it's worth a shot)

ljharb avatar Aug 24 '21 16:08 ljharb

I can't make that work. Here is a very small project to show the problem: https://github.com/DamienCassou/eslint-plugin-import-issue-2205.

$ ./node_modules/.bin/eslint .

expected: eslint detects that js/tests/bar-tests.js is not imported actual: eslint detects no problem

DamienCassou avatar Sep 18 '21 09:09 DamienCassou

did I misunderstand you?

DamienCassou avatar Mar 01 '22 16:03 DamienCassou

Sorry for the delay.

@DamienCassou when i apply this patch to your project:

diff --git a/.eslintrc.json b/.eslintrc.json
index 668275a..b9f9eac 100644
--- a/.eslintrc.json
+++ b/.eslintrc.json
@@ -33,13 +33,7 @@
 		      "error",
 		      {
 			      "unusedExports": true,
-			      "missingExports": true,
-			      "ignoreExports":
-				    // List of files not exporting anything:
-				    [
-        	    "js/tests/**/*-tests.js",
-					    "js/tests/tests.js"
-            ]
+			      "missingExports": true
 		      }
 	      ]
       }

I get these results:

$ npx eslint .

$PWD/js/tests/bar-tests.js
  1:1  error  No exports found  import/no-unused-modules

$PWD/js/tests/foo-tests.js
  1:1  error  No exports found  import/no-unused-modules

$PWD/js/tests/tests.js
  1:1  error  No exports found  import/no-unused-modules

✖ 3 problems (3 errors, 0 warnings)

Is that what you're looking for?

ljharb avatar Jun 30 '22 20:06 ljharb

Is that what you're looking for?

no. There should be no errors with foo-tests.js and tests.js: it is actually quite fine that these files don't export anything.

The only problem is in bar-tests.js which is never imported.

By the way I'm very sorry for the broken indentation in the eslint configuration file in this project. I've just fixed it.

DamienCassou avatar Jul 01 '22 07:07 DamienCassou

@DamienCassou but you have js/tests/**/*-tests.js as "files that are OK not exporting anything", and that glob includes js/tests/bar-tests.js, so it seems like it's working correctly?

ljharb avatar Sep 02 '22 20:09 ljharb

you have js/tests/**/*-tests.js as "files that are OK not exporting anything", and that glob includes js/tests/bar-tests.js, so it seems like it's working correctly?

The problem isn't that bar-tests.js is not exporting anything, that is quite fine indeed. The problem is that bar-tests.js is never imported. I want to see an error indicating that bar-tests.js is unused.

Said differently: src/tests.js imports foo-tests.js (which is good) but doesn't import bar-tests.js (which is bad). Because no code imports bar-tests.js, I want an "unused file" linting error.

DamienCassou avatar Sep 06 '22 14:09 DamienCassou