Item1518: Perlcritic should give only a small set of known and acknowledged errors
Priority: Low
Current State: Closed
Released In: 1.1.0
Target Release: minor
Applies To: Engine
Component:
Branches:
Perlcritic (on trunk) should only give errors that are known and valid (for speed or some other reasons).
The current logfile can be found here:
http://fosiki.com/Foswiki_trunk/Foswiki-PerlCritic.log
--
OlivierRaginel - 24 Apr 2009
For example:
- lib/Foswiki/Configure/Checker.pm: Code before strictures are enabled at line 5, column 1. See page 429 of PBP. (Severity: 5)
- This means there is some code before
use strict;
, which is easily avoidable -- OlivierRaginel
- lib/Foswiki/Configure/Checker.pm: Private Member Data shouldn't be accessed directly. at line 189, column 9. Accessing an objects data directly breaks encapsulation and should be avoided. Example: $object->{ some_key }. (Severity: 5)
- This is because one should use getter and setter. This requires a change of the entire API, and will probably be slower, so this should be left as-is -- OlivierRaginel
- lib/Foswiki/Configure/Checker.pm: Stricture disabled at line 200, column 13. See page 429 of PBP. (Severity: 5)
- This means there was a
no strict
, probably to bypass some checking temporarily. This should be reviewed, but normally is OK -- OlivierRaginel
- lib/Foswiki/Configure/Checker.pm: Expression form of "eval" for something other than require at line 201, column 13. It's okay to use stringy eval to require a module by name, but otherwise it's probably a mistake.. (Severity: 5)
- This means there was some
eval "some code";
. This is discouraged because it will only be compiled upon execution. So, unless it's some use Module
, which shouldn't be compiled, otherwise it will fail, it should most likely be written: eval { some code }
-- OlivierRaginel
--
OlivierRaginel - 25 Apr 2009
I've "fixed" these:
- Code before strictures are enabled. Page 429 of PBP
- "return" statement with explicit "undef". Page 199 of PBP
- Two-argument "open". Page 207 of PBP.
It revealed some interesting issues, like some typos, or some variables wrongly named, which I hope I fixed properly. For example:
diff --git a/core/lib/Foswiki/Search/Parser.pm b/core/lib/Foswiki/Search/Parser.
index e238f12..ddf18f8 100644
--- a/core/lib/Foswiki/Search/Parser.pm
+++ b/core/lib/Foswiki/Search/Parser.pm
@@ -107,7 +110,7 @@ sub parse {
split( /[\s]+/, $searchString ); # split on spaces
}
- $result = new Foswiki::Search::Node($search, \@tokens, $options );
+ my $result = new Foswiki::Search::Node($searchString, \@tokens, $options );
return $result;
}
Note the $search instead of $searchString.
Will try to continue working on this when I have some time to kill and no brain power available
--
OlivierRaginel - 20 May 2009
distro:7970b4b52b42 breaks TWiki::Attrs in
TWikiCompatibilityPlugin with a
Can't use string ("Foswiki::Attrs::") as a symbol ref while "strict refs" in use at /home/www-data/foswiki/trunk/core/lib/TWiki/Attrs.pm line 9
--
MichaelDaum - 04 Jun 2009
Fixed by
distro:a720e4adb750, and I've checked other modules from
TCP, they all look OK.
Thanks for reporting that Micha.
--
OlivierRaginel - 05 Jun 2009
current status:
lib/
410 files.
2,692 subroutines/methods.
60,671 statements.
98,456 lines.
Average McCabe score of subroutines was 4.89.
6,646 violations.
Violations per file was 16.210.
Violations per statement was 0.110.
Violations per line of code was 0.068.
6,646 severity 5 violations.
bin/
24 files.
34 subroutines/methods.
978 statements.
1,735 lines.
Average McCabe score of subroutines was 3.41.
33 violations.
Violations per file was 1.375.
Violations per statement was 0.034.
Violations per line of code was 0.019.
33 severity 5 violations.
--
WillNorris - 29 Dec 2009
where does the
Private Member Data shouldn't be accessed directly. at line 189, column 9. Accessing an objects data directly breaks encapsulation and should be avoided. Example: $object->{ some_key }. (Severity: 5)
error come from? i don't get these error messages in my log file when building locally (using
autoBuildFoswiki.pl
)
checking in
--exclude=Variables::ProtectPrivateVars
as my best guess...
(NOTE:
https://rt.cpan.org/Ticket/Display.html?id=50727 says that the computed
McCabe scores are too high)
--
WillNorris - 29 Dec 2009
HELP! log broken:
http://fosiki.com/Foswiki_trunk/Foswiki-PerlCritic.log shows
sh: /usr/local/bin/perlcritic: No such file or directory
--
WillNorris - 24 Mar 2010
ah, making a new build box meant it had moved to
/usr/bin/perlcritic
. should be fixed now
--
SvenDowideit - 30 Apr 2010
Closing this - checkins have been picked up in a release.
--
GeorgeClark - 09 Mar 2011