New: `no-unused-modules` rule by rfermann · Pull Request #1142 · import-js/eslint-plugin-import

rfermann · 2018-07-23T15:51:52Z

As discussed in #186, this rule adds the possibility to find modules without any exports and/or exports within a module, which aren't used in other modules

ToDos:

implement feature/fix bug
update docs
write tests
make a note in change log

This is the first rule I created. Writing tests for it may therefor take some additional time.

Signed-off-by: René Fermann <rene.fermann@gmx.de>

coveralls · 2018-07-23T16:14:17Z

Coverage increased (+3.6%) to 97.775% when pulling d35b7ff on rfermann:master into af976b9 on benmosher:master.

Signed-off-by: René Fermann <rene.fermann@gmx.de>

…ilename Signed-off-by: René Fermann <rene.fermann@gmx.de>

Signed-off-by: René Fermann <rene.fermann@gmx.de>

coreyfarrell · 2018-08-02T20:19:54Z

While looking into how ignore is implemented I noticed a couple language features which are not tested.

import *

I'm unsure what should happen when import * as localVar is used, any limitations should be documented. Take the following source:

import * as localVar from './file-1';
localVar.something();

Can we detect that we're actually using the named export something? If not I think the import * syntax should cause all exports of ./file-1 to be marked as used and we should document how it works. Probably suggest also using a rule that forbids use of import *.

export from

Given file-2.js containing export {something} from './file-1', tests should verify this is recognized as:

An import, so even if nothing else imports ./file-1 it should not be reported as unused.
An export, if nothing imports something from file-2.js it is reported.

dynamic import

I suspect dynamic import cannot be detected, this should be mentioned in the docs.

ljharb · 2018-08-03T03:52:38Z

Yes, an import * should mark all exports as used, i think.

… sanity checks Signed-off-by: René Fermann <rene.fermann@gmx.de>

Signed-off-by: René Fermann <rene.fermann@gmx.de>

…rom ', revised docs

rfermann · 2018-08-19T17:23:37Z

Support for import * from 'somewhere', export * from 'somewhere' and export { stuff } from 'somewhere' has been added with the last 2 commits.
The documentation has been adjusted accordingly.

How can I analyze, why the AppVeyor build is failing all the time? It btw also fails for tests I haven't touched at all 😕

rfermann · 2018-09-01T18:40:24Z

@ljharb: do you have any ideas how to tackle the failing AppVeyor build?

ljharb · 2018-09-01T20:37:07Z

@rfermann i believe it's on master, and unrelated to this PR.

rfermann · 2018-09-02T18:01:40Z

Okay, good to know.

What else needs to be done from my site to let this PR be merged? Should I adjust the change log? Or is that one of these tasks you will perform @ljharb?

ljharb · 2018-09-09T21:42:20Z

@rfermann you don't have to worry about the changelog; the appveyor issue should be fixed on master if you rebase.

…orts', revised docs Signed-off-by: René Fermann <rene.fermann@gmx.de>

Signed-off-by: René Fermann <rene.fermann@gmx.de>

rfermann · 2018-10-14T09:55:06Z

@ljharb , @benmosher: what else am I expected to do to get this PR merged? Is it the outstanding rebase which is blocking the merge?

ljharb

Doing a fresh review - looks good overall. Just a few comments.

…riable, fixed documentation Signed-off-by: Fermann <rene.fermann@capgemini.com>

ljharb

Please ensure all files have trailing newlines.

rfermann · 2018-12-18T21:21:00Z

Thanks for your latest feedback. I implemented your suggestions.

Please ensure all files have trailing newlines.

I don't get that. Did I forgot the trailing newline anywhere? I checked the files I pushed today and all already had a trailing newline 😕

ljharb · 2018-12-23T17:11:01Z

+  specifiers.some(({ type }) => type === IMPORT_DEFAULT_SPECIFIER)
+
+module.exports = {
+  isNodeModule,


why is this exported?

Basically for testing, just as you supposed.

This shouldn't need direct testing; let's please not export it.

ljharb

@rfermann hey! i know it's been awhile; are you still interested in addressing the outstanding comments, rebasing, and ensuring the tests pass?

ljharb · 2019-03-27T00:59:57Z

+  specifiers.some(({ type }) => type === IMPORT_DEFAULT_SPECIFIER)
+
+module.exports = {
+  isNodeModule,


This shouldn't need direct testing; let's please not export it.

…slint-plugin-import

futpib · 2019-04-05T11:20:13Z

+
+module.exports = {
+  meta: {
+    docs: { url: docsUrl('no-unused-modules') },


Isn't the name no-unused-modules misleading? This rule reports individual export declarations, but the name suggests it reports whole files as unused.

my understanding is that it reports on both - both unused files, and unused exports.

Just as @ljharb says, it reports on both:

modules without any exports are reported here

and modules with unused exports are reported here and here

However this rule currently does not report, if a module only consists of unused exports. Each of these exports is reported separately.
I am not sure whether it makes sense to have one single report being triggered if none of the existing exports is used.

It does make sense to me in that case to only provide one report; just as I’d expect when a module has no exports.

ljharb

Thanks, let's give this a shot :-)

New: no-unused-modules rule

75a9eab

Signed-off-by: René Fermann <rene.fermann@gmx.de>

ljharb requested changes Jul 23, 2018

View reviewed changes

Comment thread docs/rules/no-unused-modules.md Outdated

rfermann added 5 commits July 30, 2018 18:26

New: no-unused-modules rule - minor refactoring

1d8ddf7

Signed-off-by: René Fermann <rene.fermann@gmx.de>

New: no-unused-modules rule - added tests

153599d

Signed-off-by: René Fermann <rene.fermann@gmx.de>

New: no-unused-modules rule - removed destructoring of context.getF…

fd1ccc0

…ilename Signed-off-by: René Fermann <rene.fermann@gmx.de>

New: no-unused-modules rule - minor refactoring

a402023

Signed-off-by: René Fermann <rene.fermann@gmx.de>

New: no-unused-modules rule - added more tests

230122b

Signed-off-by: René Fermann <rene.fermann@gmx.de>

This comment has been minimized.

Sign in to view

coreyfarrell reviewed Aug 1, 2018

View reviewed changes

Comment thread docs/rules/no-unused-modules.md Outdated

Comment thread tests/src/rules/no-unused-modules.js Outdated

Comment thread docs/rules/no-unused-modules.md Outdated

rfermann added 3 commits August 5, 2018 16:28

New: no-unused-modules rule - added default src, more comprehensive…

aa7253b

… sanity checks Signed-off-by: René Fermann <rene.fermann@gmx.de>

New: no-unused-modules rule - add support for 'import *'

d47ccdf

Signed-off-by: René Fermann <rene.fermann@gmx.de>

New: rule - added support for 'export * from' and 'export { stuff } f…

06a0976

…rom ', revised docs

rfermann changed the title ~~WIP: New: no-unused-modules rule~~ New: no-unused-modules rule Sep 1, 2018

rfermann added 2 commits September 12, 2018 13:17

New: no-unused-modules rule - renamed 'ignore' option to 'ignoreExp…

c483514

…orts', revised docs Signed-off-by: René Fermann <rene.fermann@gmx.de>

New: no-unused-modules rule - corrected typo in docs

e04c87c

Signed-off-by: René Fermann <rene.fermann@gmx.de>

ljharb requested changes Oct 14, 2018

View reviewed changes

Comment thread docs/rules/no-unused-modules.md Outdated

Comment thread docs/rules/no-unused-modules.md Outdated

Comment thread src/rules/no-unused-modules.js

Comment thread src/rules/no-unused-modules.js Outdated

Comment thread src/rules/no-unused-modules.js Outdated

New: no-unused-modules rule - reworked schema, removed UNDEFINED va…

2922910

…riable, fixed documentation Signed-off-by: Fermann <rene.fermann@capgemini.com>

ljharb reviewed Dec 8, 2018

View reviewed changes

New: no-unused-modules rule - implemented latest feedback

0f5c2a5

ljharb reviewed Dec 18, 2018

View reviewed changes

Comment thread docs/rules/no-unused-modules.md

Comment thread tests/files/no-unused-modules/file-b.js Outdated

Comment thread tests/src/rules/no-unused-modules.js Outdated

Comment thread src/rules/no-unused-modules.js

Comment thread src/rules/no-unused-modules.js Outdated

rfermann added 2 commits December 23, 2018 10:31

New: no-unused-modules rule - added trailing newlines

191c77b

New: no-unused-modules rule - added stricter check for node_modules

ae9942f

ljharb requested changes Dec 23, 2018

View reviewed changes

Comment thread package.json Outdated

ljharb reviewed Dec 23, 2018

View reviewed changes

ljharb mentioned this pull request Dec 23, 2018

path 'eslint/lib/util/glob-util' should be available until next major version eslint/eslint#11206

Closed

rfermann added 2 commits December 23, 2018 22:20

New: no-unused-modules rule - revert changing eslint version

32f4c23

New: no-unused-modules rule - removed whitespace, replaced if-stmt

90f7217

rfermann mentioned this pull request Dec 29, 2018

Expose listFilesToProcess() on CLIEngine eslint/eslint#11222

Closed

ljharb reviewed Mar 27, 2019

View reviewed changes

rfermann added 3 commits March 31, 2019 17:47

ew: no-unused-modules rule - remove unnecessary exports

0dd398c

ew: no-unused-modules rule - remove unnecessary exports

1ac4bd7

Merge branches 'master' and 'master' of https://github.com/rfermann/e…

d35b7ff

…slint-plugin-import

futpib reviewed Apr 5, 2019

View reviewed changes

ljharb approved these changes Apr 7, 2019

View reviewed changes

ljharb merged commit 3aefa79 into import-js:master Apr 7, 2019

This was referenced Apr 7, 2019

rule: no-unused #186

Closed

no-unused-modules appveyor tests failing #1317

Closed

This was referenced Jan 22, 2021

Rule no-unused-modules does not count re-export all declarations #1945

Open

[no-unused-modules] Dynamic import doesn't mark exports as used #1951

Closed

Uh oh!

Conversation

rfermann commented Jul 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Jul 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

coreyfarrell commented Aug 2, 2018

import *

export from

dynamic import

Uh oh!

ljharb commented Aug 3, 2018

Uh oh!

rfermann commented Aug 19, 2018

Uh oh!

rfermann commented Sep 1, 2018

Uh oh!

ljharb commented Sep 1, 2018

Uh oh!

rfermann commented Sep 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ljharb commented Sep 9, 2018

Uh oh!

rfermann commented Oct 14, 2018

Uh oh!

ljharb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ljharb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rfermann commented Dec 18, 2018

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ljharb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

rfermann commented Jul 23, 2018 •

edited

Loading

coveralls commented Jul 23, 2018 •

edited

Loading

rfermann commented Sep 2, 2018 •

edited

Loading