There are some other perlcritic warnings about modifying $_ in list
functions
(in sa-update, spamassassin, spamd). Opening a PR would be warranted.
What is a PR? A bugzilla ticket?
Yes, a problem report. Sorry for using terminology from another
context.
If there are other perl critic warnings, they are likely not enabled
in that test.
Opened a Bug 7119.
Probably depends on a version of Perl::Critic.
# Perl::Critic found these violations in
"../blib/lib/Mail/SpamAssassin/Util.pm":
# "return" statement with explicit "undef" at line 287, column 5.
Opened a Bug 7120. It can be used as a reference in a comment
when adding "## no critic" annotations in code.
Looking back, this return undef was added after 3.4.0's release and
appears to have been added during the profiling work on
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6854
I probably fixed that potential or actual problem in passing.
The MS::Util::untaint_var() is still broken...
I'm reverting some recent changes and reverting back to the one undef
Thanks!
Do you feel confident enough about your change that I should modify or
notate the build process notes to ignore that issue for that specific
return undef?
For the 'return undef' in untaint_var() most definitely so.
That function can be used when assembling arguments in a
call to some other function, just as described in Bug 7120.
That statement deserves the '## no critic' annotation,
something like (untested) :
# see Bug 7120
return undef if !defined $_[0]; ## no critic
(ProhibitExplicitReturnUndef)
For other cases I didn't check.
Btw, common use of 'return 0' in TxRep to indicate a boolean response
falls into the same category as 'return undef', although
Test::Perl::Critic
does not check for that.
I use no warnings 'uninitialized', 'once'; for a lot of my day to day
work
so I can see that but my key concern is passing the tests required of a
release without trying to change those tests right now.
Testing in a boolean context does not issue a warning when
an expression is undefined. There is no reason to return
a zero (instead of undef or an empty list) when a subroutine
result is considered a boolean value.
Mark
