On Tue Nov 30 11:59:34 2010, EDAVIS wrote:
Show quoted text> I was surprised that perlcritic didn't catch the following mistake:
>
> print 'a',
> print 'b';
>
> That first comma was intended to be a semicolon. Perl itself also gives
> no warning for this.
Hmmm. Tricky. I'll have to think about it.
My first impression was that this was a bug in
ValuesAndExpressions::ProhibitCommaSeparatedStatements. But there's
actually nothing wrong with the general case of
foo 1, 2, 3, bar 4, 5, 6;
which is why Perl didn't complain. Your example looks strange to someone
with a background in statement-oriented languages like Fortran, but in
an expression-oriented language like Perl is another matter.
Show quoted text>
> I can think of a few possible policies that would catch it:
>
> - If a line ends with a comma, the next line should be indented.
There are, in fact, Perl::Critic policies about code layout;
specifically the seven CodeLayout:: policies. But these explicitly
enforce layout policies. I suppose there could be something like
CodeLayout::RequireArgumentListsToBeIndented, but the violation in this
case would be something like "Indent argument list at line 2 column 4",
which is an _extremely_ oblique way to say "you used a comma where you
should have used a semicolon". And although I indent argument lists (and
I presume you do too), the say-so of two people hardly makes this
sufficiently a "best practice" to go in core Perl::Critic.
Show quoted text>
> - Although the return code from the print() and say() builtins can be
> checked, it should not itself be printed directly.
Devil's advocate: why not? But see below.
Show quoted text>
> - print() and say() and perhaps other builtins should appear only as the
> left-hand part of an expression or statement.
> This would forbid stuff like $j = ($i++ && print 'hello')
> but (somehow) when print appears leftmost, as print('hello') || die
> then it would be allowed.
I'm also unsure about this approach. I have certainly written things like
$i++ or print "Header\n";
which seems to me perfectly legit.
Show quoted text>
> What do you think? The indentation check is the most straightforward
> and I think would quickly catch bugs, but it may be a new departure for
> perlcritic to issue warnings based on code layout.
See above for my take on the layout check.
Another way to get at it might be a policy that prohibits passing the
results of certain subroutines (be they built-ins or not) as arguments
to other subroutines. The thing is, surely _someone_ has a legitimate
reason to call
foo 1, 2, print "Hello world!\n"
I can't imagine what it is, but that doesn't count for much. If I
personally were to write such a policy for core Perl::Critic, I would
probably have it be disabled by default, by leaving the list of
forbidden subroutine names empty.
This whole thing would be a moot point if there were a thriving
ecosystem of one-policy Perl-Critic-Something-Or-Other distributions
from which people could pick and choose. If such a policy didn't already
exist it could be written, people who agree with it could install it,
and people who don't could ignore it. But that hasn't happened.
Maybe one of the other maintainers has a better idea.
Tom Wyant
