Skip to content

Conversation

@ThomasLamprecht
Copy link

This adds an option flag that, when enabled, causes perlimports to stop outputting empty import lists for use statements where there is no symbol to be imported.

While some projects and people want empty import lists to increase clarity and prevent the used module from having default exports in the future, others see them as unnecessary visual noise and would like to avoid them.

The CLI option is named --skip-empty-imports, and the configuration option is named skip_empty_imports. Both default to false, maintaining the current behavior that provides more deterministic clarity and future-proofs against export changes in the used modules.

Thanks to the existing code infrastructure, the implementation is relatively straightforward. We just add new options and wire them up so that they are available in the Include module when outputting. Since there is already a dedicated code branch for modules that never export or from which nothing is imported, we can modify the output format string template there to differentiate between the respective values of the new skip_empty_imports flag.

Add two tests to ensure that enabling and disabling explicitly work. Reuse the existing original-imports.pl test data module, as it is a fitting test asset.

Closes: #118

Open questions/tasks:

  • In Add an option to allow one to opt-out of adding empty import lists #118 (comment) it was mentioned to "flag it as beta/experimental". I checked the git history and found the commits adding the --range-begin and --range-end flags, and there the experimental status was only mentioned in the respective entry for the Changes file (ref commit e412e9c), so I opted for doing the same thing. Is that OK, or should I add other experimental markings somewhere?
  • Is the CLI option name --skip-empty-imports and the config property name skip_empty_imports OK as is?
  • Text in docs and changes might do well with another typo/grammar check as I'm not a native English speaker.
  • Anything else I should evaluate or test to add confidence to the implementation?

And FWICT from implementing this my available time should be more than enough to stick around for bug fixes, albeit–for full transparency–I might need a few days, and seldomly even one or two weeks to respond, depending on work crunch/time-off/etc.

This adds an option flag that, when enabled, causes perlimports to
stop outputting empty import lists for use statements where there is
no symbol to be imported.

While some projects and people want empty import lists to increase
clarity and prevent the used module from having default exports in the
future, others see them as unnecessary visual noise and would like to
avoid them.

The CLI option is named `--skip-empty-imports`, and the configuration
option is named `skip_empty_imports`. Both default to false,
maintaining the current behavior that provides more deterministic
clarity and future-proofs against export changes in the used modules.

Thanks to the existing code infrastructure, the implementation is
relatively straightforward. We just add new options and wire them up
so that they are available in the Include module when outputting.
Since there is already a dedicated code branch for modules that never
export or from which nothing is imported, we can modify the output
format string template there to differentiate between the respective
values of the new skip_empty_imports flag.

Add two tests to ensure that enabling and disabling explicitly work.
Reuse the existing original-imports.pl test data module, as it is a
fitting test asset.

Closes: perl-ide#118
Signed-off-by: Thomas Lamprecht <[email protected]>
@ThomasLamprecht
Copy link
Author

ThomasLamprecht commented May 15, 2025

Hmm, after talking a bit with a colleague in the office about this I got some doubts about semantic details.

Namely, are modules that export by default, e.g., populate EXPORT and not just EXPORT_OK, really to be counted as producing no import?

And if not so, should one prefer to unconditionally add empty import list for modules that export something by default, or should we allow the user to choose here by transforming the option into a tri-state enum?

Both would not be much more code in total, so I think this is mostly a higher level question.

I got a POC for a tri-state enum that makes the skip-empty-imports option accept the following values:

  • none (default), always output an empty list if nothing can be explicitly imported, equivalent to the boolean false value in the current PR.
    Example:
use App::perlimports::Sandbox  ();
use POSIX ();
  • all, always skip outputting an empty list if nothing can be explicitly imported, equivalent to the boolean true value in the current PR.
    Example:
use App::perlimports::Sandbox;
use POSIX;
  • no-explicit-exports, only output an empty import list if the module has default export that might pollute some (future) symbols.
    Example:
use App::perlimports::Sandbox;
use POSIX ();

At work, we would be content with the option staying boolean and the no-explicit-exports behavior used when it's enabled, as that gives us a good balance between low noise and import safety/clarity. But others might see this different.

@mrsdizzie: maybe you can also chime in here given that you also commented on #118

@oalders
Copy link
Member

oalders commented Oct 22, 2025

@ThomasLamprecht I'm so sorry for how long this took. I won't bore you with the details. 😅

Thank you so much for this PR. Do you still have the branch with the tri-state enum handy? That sounds like a good solution. Barring that, I think we could still move forward with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add an option to allow one to opt-out of adding empty import lists

2 participants