1

Perlcritic complains about the following line of code:

require "bin/menu.pl";

"require" statement with library name as string Use a bareword instead.

However if I change it to a bare word:

require bin/menu.pl

I get an illegal division by zero error.

To reproduce just run perlcritic on any perl script that 'requires' another perl script.

The docs for require suggest using a string: https://perldoc.perl.org/functions/require Is this a bug in perlcritic?

turtle
  • 417
  • 3
  • 11

3 Answers3

2

Perl critic tells you the canonical way is to use the bareword form of require:

require Some::Module;

Requiring a script is not a good idea (because it doesn't run it every time it's required, but might run it more than once if the path is not the same; also, it's unclear what namespace the required script pollutes). You should require modules (or, even better, use them).

A bareword can't contain a slash. It's therefore interpreted as the division operator, and as you aren't using strict, it interprets the barewords bin, menu, and pl as strings. As they don't start with digits, their value in numeric context (which / enforces) is 0.

$ perl -MO=Deparse,-p -e 'lib/menu.pl'
(('lib' / 'menu') . 'pl');
-e syntax OK
choroba
  • 231,213
  • 25
  • 204
  • 289
  • Thanks for your answer, its a pretty large old project, not of my design, it has a big mix of script files and modules, and I can't rename files. The require for a .pl file has to stay for the time being. I am still unclear what the hazard or bug is with using require "bin/script.pl',. I have otherwise fixed all other perlcritic complaints. – turtle Aug 25 '23 at 20:41
  • @turtle: The hazard is the script won't run where you expect it to, because it has already run when required somewhere else. Another problem might be running it several times, importing the subs into different namespaces. Requiring scripts was a thing in the 90s when `use` didn't exist yet. – choroba Aug 25 '23 at 21:15
2

If bin/menu.pl has a package statement

Say the package statement is package My::Menu;.

  • Make sure it ends with a true value (e.g. has 1; at the end).
  • Rename the file to My/Menu.pm.
  • Use use My::Menu;.

If bin/menu.pl doesn't have package statement

It's not a module, so don't load it like one.

  • Make sure it ends with a true value (e.g. has 1; at the end).
  • Use do( "bin/menu.pl" ) or die( $@ // $! );.

Consider converting it to a module. We haven't been constrained to this poor style of programming for about 25 years.

See What is the difference between library files and modules?

ikegami
  • 367,544
  • 15
  • 269
  • 518
2

In this case you should ignore Perl::Critic. It's perfectly fine to load a library with require like you are doing. It's not popular anymore, but that is the original idea of require. Tell Perl::Critic to ignore that line:

require 'some/path/some_library.pl'; ## nocritic

Don't use Perl::Critic to tell you what to do. Use it as a tool to look at things that might need attention, then make a decision based on the context of your task.

As people have noted, require will only execute the code the first time around. If that's what you want (and it probably is because that's been working up to now), leave the code as is.

Some people have suggested turning this into a module, in which case you probably just end up exporting all the subroutines anyway. You end up in the same place with extra steps. That might be fine when you're looking for something to do and can't find anything more pressing.

brian d foy
  • 129,424
  • 31
  • 207
  • 592
  • 1
    Re "*Don't use Perl::Critic to tell you what to do.*", True, except if found an actual bug here. /// Re "*It's perfectly fine to load a library with require like you are doing.*", No, it's not. The `require` could do absolutely nothing instead of importing the contents of the file. `require` is the wrong tool for old-style "libraries". `require` should only use for modules, which is to say files with a `package`. `do` should be used in this situation. – ikegami Aug 26 '23 at 22:30