Usage

git clone ... (this repo)
cd ...

# Build docker container with perlcritic
make build

# Run perlcritic against default branch (master)
make run

# Run perlcritic against a specific branch
make run VERSION=master
make run VERSION=2.0.11 PC_ARGS='-4'

Results - Oct 2020 - version 2.0.11

Summary:

Reviewed 2.0.11 (released Oct 1, 2020)
perlcritic found 270 files that were OK out of 322 files
"gentle" (sev5 only) was used.
- Sev4 was a lot noisier, and focussed more on visual code-style vs security.

Warnings:

67x "return" statement with explicit "undef" at line NN, column NN. See page 199 of PBP. (Severity: 5)
- Unlikely a security thing, but it could result in odd bugs when the result is used in an if( ) statement to see if the result was true or not.
- The recommendation is a bare return; statement, which "does the right thing" in both an array and a scalar context.
- https://perlmaven.com/how-to-return-undef-from-a-function
6x Bareword file handle opened at line 135, column 5. See pages 202,204 of PBP.
- Only likely to cause a bug if multi-threading is used, but the fix is pretty straight forward and it's pretty easy to create scoped file handles.
- https://wiki.sei.cmu.edu/confluence/display/perl/FIO00-PL.+Do+not+use+bareword+file+handles
11x Code before strictures are enabled at line 16, column 1. See page 429 of PBP. (Severity: 5)
- use strict was not included in these modules. In some it was commented out.
- It might be worth mentioning why up near the top of the file where use strict is normally placed.
6x Expression form of "eval" at line 14, column 1. See page 161 of PBP. (Severity: 5)
- OK - Evals were all in testing
16x Found use of Switch. This module is deprecated by the Perl 5 Porters at line 13, column 1. Find an alternative module.
- No big deal - Switch is deprecated though. given() { when () {} } is built-in now and is more powerful
- https://metacpan.org/pod/perl5100delta#Switch-and-Smart-Match-operator
- If you have ripgrep installed, you can get an idea of where these are via: rg -g '*.pm' -g '*.pl' '\b[Ss]witch\b' src
1x Glob written as <...> at line 21, column 19. See page 167 of PBP.
- Not a big deal, only exists in tests. I'm not sure what the best practices are for this.
4x Integer with leading zeros: "00700" at line 395, column 50. See page 58 of PBP. (Severity: 5)
- These are probably OK, but the preferred method is oct(700).
- Note that errorId raw values are specified in the following files and could be specified as named constants:
  - src/perl-lib/OESS/lib/OESS/NSI/Provisioning.pm
  - src/perl-lib/OESS/lib/OESS/NSI/Reservation.pm
1x Nested named subroutine at line 303, column 5. Declaring a named sub inside another named sub does not prevent the inner sub from being global. (Severity: 5)
- Lexical scoping isn't shared between between _create_flows and _process_links, so this could be moved if it cleans up code.
1x Stricture disabled at line 71, column 13. See page 429 of PBP. (Severity: 5)
- I'm not sure if no strict qw(refs) is needed in this case.
- This file (src/frontend/webservice/idc/nsi.cgi) is a very complex nest of eval's, do's, and die's. I couldn't make sense of this in a short period of time, but it's probably worth considering refactoring this to be more understandable. It's entirely possible that this is actually the best way to approach it, but it's probably worth a look.
5x Two-argument "open" used at line 151, column 5. See page 207 of PBP. (Severity: 5)
- These are in setup/webservice files, so they're not a big deal.
1x Use IO::Interactive::is_interactive() instead of -t at line 50, column 9. See page 218 of PBP. (Severity: 5)
- This is in a vendored dependency, no bug, but worth looking at.

# Find the code age
$ make VERSION=2.0.11 code-age | sort -n > output/code-age.v2.0.11.txt

# Summarize code age
$ cat output/code-age.v2.0.11.txt | cut -d- -f1 | uniq -c
 943 2012
   3 2013
 828 2014
  42 2015
  15 2016
  72 2017
  54 2018
  91 2019
 450 2020

# Run perlcritic
$ make build
$ make VERSION=2.0.11 perlcritic PC_ARGS='--no-color' | tee output/perlcritic-v2.0.11.txt

...

  322 files.
 1,707 subroutines/methods.
69,980 statements.

85,391 lines, consisting of:
    14,678 blank lines.
     3,999 comment lines.
         0 data lines.
    59,886 lines of Perl code.
     6,828 lines of POD.

Average McCabe score of subroutines was 4.82.

119 violations.
Violations per file was 0.370.
Violations per statement was 0.002.
Violations per line of code was 0.001.

119 severity 5 violations.

 6 violations of BuiltinFunctions::ProhibitStringyEval.
 1 violations of BuiltinFunctions::RequireGlobFunction.
 6 violations of InputOutput::ProhibitBarewordFileHandles.
 1 violations of InputOutput::ProhibitInteractiveTest.
 5 violations of InputOutput::ProhibitTwoArgOpen.
16 violations of Modules::ProhibitEvilModules.
67 violations of Subroutines::ProhibitExplicitReturnUndef.
 1 violations of Subroutines::ProhibitNestedSubs.
 1 violations of TestingAndDebugging::ProhibitNoStrict.
11 violations of TestingAndDebugging::RequireUseStrict.
 4 violations of ValuesAndExpressions::ProhibitLeadingZeros.

jharr/oess-review

About

Resources

Stars

Watchers

Forks

Releases

Packages 0

Languages

Packages