Perl::Critic::Policy::ValuesAndExpressions::ProhibitFiletest_f - don't use the -f file test

This policy is part of the Perl::Critic::Pulp|Perl::Critic::Pulp addon. It asks you not to use the -f file test because it's so often wrong or unnecessarily restrictive (and on that basis this policy is under the "bugs" theme, see POLICY THEMES in Perl::Critic).

-f is not the opposite of -d

If you're traversing a tree and want to distinguish files to process from directories to descend into then -d should be used so device files or pipes can be processed.

    if (-f $filename) {      # bad
      process ($filename);
    } else {
      descend ($filename);
    }

    if (-d $filename) {      # better
      descend ($filename);
    } else {
      process ($filename);
    }

-f doesn't mean readable/writable/seekable

Char specials and named pipes are perfectly good for reading and writing, and char specials can support seeking. Demanding -f is an unnecessary restriction in your code. You might only ever use ordinary files normally, but there's no need to prevent someone else running it on a tape drive, /dev/null, etc. You must test each seek etc for success anyway, and that will tell you if it's seekable.

    seek HANDLE, 123, 0
      or die "Error seeking: $!";

-e better than -f

A few inflexible functions or operations may not have a good "file not found" indication and may force you to test for a file before invoking. Using -e is better than -f as again it doesn't unnecessarily disallow device files.

    if (-f $filename) {      # bad
      require $filename;
    }

    if (-e $filename) {      # better
      require $filename;
    }

-f before opening is a race condition

Testing a filename before opening is bad. Any test before opening is basically useless because the file can change or be removed in between the test and the open.

    if (-f $filename) {               # bad
      open HANDLE, '<', $filename
    }

If you want to know if the file can be opened then open the file! The error return from open must be checked, so a test beforehand only duplicates that, and is an opportunity to wrongly anticipate what the system can or can't do. On opening ENOENT will say if there was no such file, or EISDIR if it's in fact a directory.

    if (! open HANDLE, '<', $filename) {  # better
      if ($! == POSIX::ENOENT()) {
        ...
      }
    }

If you really do want to enquire into the nature of the file, to only allow ordinary files, then -f can be used on the opened handle. That's unusual outside an archiving or backup program.

Incidentally, for error messages $! is normally the best thing to print. It can be slightly technical, but its values are familiar from other programs and are translated into the user's locale.

Most uses of -f tend to be shell script style code written in Perl. In the shell it's usually not possible to do better (though -d or -e is generally wanted instead of -f), but in Perl it is.

A blanket prohibition like this policy is harsh, but is meant as a building block or at least to make you think carefully whether -f is really right. As always though you can disable ProhibitFiletest_f from your .perlcriticrc in the usual way (see CONFIGURATION in Perl::Critic),

    [-ValuesAndExpressions::ProhibitFiletest_f]

Perl::Critic::Pulp, Perl::Critic, open in perlfunc, ERRNO in POSIX, Errno, errno(3)

http://user42.tuxfamily.org/perl-critic-pulp/index.html

Copyright 2009, 2010, 2011 Kevin Ryde

Perl-Critic-Pulp is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation; either version 3, or (at your option) any later version.

Perl-Critic-Pulp is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.

You should have received a copy of the GNU General Public License along with Perl-Critic-Pulp. If not, see <http://www.gnu.org/licenses>.