action #44159
closedcoordination #44075: [functional][y][epic] Use more static code style checks to make life for test developers such a pain that nobody dares to add new tests which we would not be able to maintain anyway ;)
[functional][y][timeboxed:4h] Use more static code style checks - evaluate what perl critic policies make sense
Added by okurz over 5 years ago. Updated over 5 years ago.
0%
Description
Suggestions¶
- Evaluate which https://metacpan.org/pod/Perl::Critic policies would make sense for os-autoinst-distri-opensuse
Updated by okurz over 5 years ago
- Copied to action #44165: [functional][y] Use more static code style checks - try to use https://metacpan.org/pod/Perl::Critic::Policy::ControlStructures::ProhibitDeepNests added
Updated by okurz over 5 years ago
- Copied to deleted (action #44165: [functional][y] Use more static code style checks - try to use https://metacpan.org/pod/Perl::Critic::Policy::ControlStructures::ProhibitDeepNests)
Updated by JERiveraMoya over 5 years ago
- Status changed from Workable to In Progress
- Assignee set to JERiveraMoya
Updated by JERiveraMoya over 5 years ago
When we go down one level of severity (severity=4) we found this running statistics with perlcritic:
1,243 files.
4,021 subroutines/methods.
81,356 statements.
111,964 lines, consisting of:
16,181 blank lines.
21,926 comment lines.
4 data lines.
69,175 lines of Perl code.
4,678 lines of POD.
Average McCabe score of subroutines was 3.36.
4,518 violations.
Violations per file was 3.635.
Violations per statement was 0.056.
Violations per line of code was 0.040.
4,518 severity 4 violations.
22 violations of BuiltinFunctions::RequireBlockGrep.
1 violations of BuiltinFunctions::RequireBlockMap.
6 violations of ControlStructures::ProhibitUnreachableCode.
2 violations of ControlStructures::ProhibitYadaOperator.
8 violations of InputOutput::ProhibitOneArgSelect.
27 violations of InputOutput::RequireBriefOpen.
47 violations of Modules::ProhibitAutomaticExportation.
8 violations of Modules::ProhibitMultiplePackages.
9 violations of Modules::RequireEndWithOne.
1,003 violations of Modules::RequireExplicitPackage.
35 violations of Objects::ProhibitIndirectSyntax.
11 violations of Subroutines::ProhibitBuiltinHomonyms.
2,259 violations of Subroutines::RequireFinalReturn.
945 violations of TestingAndDebugging::RequireUseWarnings.
2 violations of ValuesAndExpressions::ProhibitCommaSeparatedStatements.
27 violations of ValuesAndExpressions::ProhibitConstantPragma.
25 violations of ValuesAndExpressions::ProhibitMixedBooleanOperators.
2 violations of Variables::ProhibitAugmentedAssignmentInDeclaration.
79 violations of Variables::RequireLocalizedPunctuationVars.
According to this info above we can think about what policies would be nice to have but at the same time which one are applicables, because fix 8 could be achivable but perhaps we have to have a lot interest in one if has for example 79.
I will study this numbers in the remaining ~2h.
Updated by JERiveraMoya over 5 years ago
These is the whole thing, the statistic for severity 1, the most strict, and my feelings about it: (crossed out text => not useful, bold text => good candidate to add/enforce IMHO, normal text => nice to have or useful):
23 violations of BuiltinFunctions::ProhibitBooleanGrep.
3 violations of BuiltinFunctions::ProhibitReverseSortBlock.
31 violations of BuiltinFunctions::ProhibitStringySplit.
4 violations of BuiltinFunctions::ProhibitUselessTopic.
19 violations of BuiltinFunctions::RequireBlockGrep.
1 violations of BuiltinFunctions::RequireBlockMap.
1 violations of ClassHierarchies::ProhibitAutoloading.
2 violations of ClassHierarchies::ProhibitExplicitISA.
3 violations of CodeLayout::ProhibitHardTabs.
779 violations of CodeLayout::ProhibitParensWithBuiltins.
7 violations of CodeLayout::ProhibitQuotedWordLists.
3 violations of CodeLayout::RequireTidyCode.
30 violations of CodeLayout::RequireTrailingCommas.
27 violations of ControlStructures::ProhibitCStyleForLoops.
50 violations of ControlStructures::ProhibitCascadingIfElse.
25 violations of ControlStructures::ProhibitNegativeExpressionsInUnlessAndUntilConditions.
1,325 violations of ControlStructures::ProhibitPostfixControls.
132 violations of ControlStructures::ProhibitUnlessBlocks.
5 violations of ControlStructures::ProhibitUnreachableCode.
5 violations of ControlStructures::ProhibitUntilBlocks.
1 violations of ControlStructures::ProhibitYadaOperator.
1 violations of Documentation::RequirePackageMatchesPodName.
48 violations of Documentation::RequirePodAtEnd.
580 violations of Documentation::RequirePodSections.
631 violations of ErrorHandling::RequireCarping.
46 violations of ErrorHandling::RequireCheckingReturnValueOfEval.
21 violations of InputOutput::ProhibitBacktickOperators.
1 violations of InputOutput::ProhibitJoinedReadline.
6 violations of InputOutput::ProhibitOneArgSelect.
66 violations of InputOutput::RequireBracedFileHandleWithPrint.
13 violations of InputOutput::RequireBriefOpen.
17 violations of InputOutput::RequireCheckedClose.
8 violations of InputOutput::RequireCheckedOpen.
156 violations of InputOutput::RequireCheckedSyscalls.
16 violations of Miscellanea::ProhibitUnrestrictedNoCritic.
2 violations of Miscellanea::ProhibitUselessNoCritic.
42 violations of Modules::ProhibitAutomaticExportation.
3 violations of Modules::ProhibitConditionalUseStatements.
4 violations of Modules::ProhibitExcessMainComplexity.
4 violations of Modules::ProhibitMultiplePackages.
6 violations of Modules::RequireEndWithOne.
987 violations of Modules::RequireExplicitPackage.
5 violations of Modules::RequireNoMatchVarsWithUseEnglish.
1,126 violations of Modules::RequireVersionVar.
164 violations of NamingConventions::Capitalization.
1 violations of NamingConventions::ProhibitAmbiguousNames.
18 violations of Objects::ProhibitIndirectSyntax.
136 violations of References::ProhibitDoubleSigils.
29 violations of RegularExpressions::ProhibitCaptureWithoutTest.
32 violations of RegularExpressions::ProhibitComplexRegexes.
34 violations of RegularExpressions::ProhibitEnumeratedClasses.
137 violations of RegularExpressions::ProhibitEscapedMetacharacters.
40 violations of RegularExpressions::ProhibitFixedStringMatches.
2 violations of RegularExpressions::ProhibitSingleCharAlternation.
5 violations of RegularExpressions::ProhibitUnusedCapture.
51 violations of RegularExpressions::ProhibitUnusualDelimiters.
16 violations of RegularExpressions::ProhibitUselessTopic.
9 violations of RegularExpressions::RequireBracesForMultiline.
1,111 violations of RegularExpressions::RequireDotMatchAnything.
939 violations of RegularExpressions::RequireExtendedFormatting.
1,088 violations of RegularExpressions::RequireLineBoundaryMatching.
11 violations of Subroutines::ProhibitAmpersandSigils.
6 violations of Subroutines::ProhibitBuiltinHomonyms.
49 violations of Subroutines::ProhibitExcessComplexity.
4 violations of Subroutines::ProhibitManyArgs.
3 violations of Subroutines::ProhibitUnusedPrivateSubroutines.
45 violations of Subroutines::RequireArgUnpacking.
2,047 violations of Subroutines::RequireFinalReturn.
27 violations of TestingAndDebugging::RequireTestLabels.
915 violations of TestingAndDebugging::RequireUseWarnings.
1 violations of ValuesAndExpressions::ProhibitCommaSeparatedStatements.
15 violations of ValuesAndExpressions::ProhibitConstantPragma.
474 violations of ValuesAndExpressions::ProhibitEmptyQuotes.
1 violations of ValuesAndExpressions::ProhibitEscapedCharacters.
103 violations of ValuesAndExpressions::ProhibitImplicitNewlines.
8,022 violations of ValuesAndExpressions::ProhibitInterpolationOfLiterals.
1 violations of ValuesAndExpressions::ProhibitLongChainsOfMethodCalls.
1,737 violations of ValuesAndExpressions::ProhibitMagicNumbers.
18 violations of ValuesAndExpressions::ProhibitMismatchedOperators.
24 violations of ValuesAndExpressions::ProhibitMixedBooleanOperators.
205 violations of ValuesAndExpressions::ProhibitNoisyQuotes.
1 violations of ValuesAndExpressions::ProhibitQuotesAsQuotelikeOperatorDelimiters.
198 violations of ValuesAndExpressions::RequireInterpolationOfMetachars.
27 violations of ValuesAndExpressions::RequireNumberSeparators.
9 violations of ValuesAndExpressions::RequireQuotedHeredocTerminator.
5 violations of ValuesAndExpressions::RequireUpperCaseHeredocTerminator.
1 violations of Variables::ProhibitAugmentedAssignmentInDeclaration.
1 violations of Variables::ProhibitLocalVars.
343 violations of Variables::ProhibitPackageVars.
188 violations of Variables::ProhibitPunctuationVars.
21 violations of Variables::ProhibitReusedNames.
15 violations of Variables::ProhibitUnusedVariables.
10 violations of Variables::RequireInitializationForLocalVars.
41 violations of Variables::RequireLocalizedPunctuationVars.
Feel free to check if some of the crossed out policies make sense for someone else.
Note1: Some I marked as bold have a lot of hits but I had to add them as I encounter doubts which them in PR reviews and it might worth the time (i.e: how many kind of export we have?:) )
Note2: I was running the tool and counting without using --statistics and matches perfectly. To simulate each one just needs to add at the end of .perlcriticrc:
[PolicyName]
severity = 5
What this does, it is include this particular policy in gentle severity regardless of its own default severity, so we can visualize result when we run perlcritic with --gentle
Updated by JERiveraMoya over 5 years ago
- Status changed from In Progress to Feedback
Updated by okurz over 5 years ago
Hi @JERiveraMoya, really good evaluation. I guess from here on we can discuss this ticket together, e.g. in daily or review or retro and then follow on with specific tickets about the policies that we want to work on.
JERiveraMoya wrote:
These is the whole thing, the statistic for severity 1, the most strict, and my feelings about it: (crossed out text => not useful, bold text => good candidate to add/enforce IMHO, normal text => nice to have or useful):
ok, cool. I don't see any bold though.
and now, just my personal taste of where my opinion differs, for all not mentioned I agree:
23 violations of BuiltinFunctions::ProhibitBooleanGrep.
50 violations of ControlStructures::ProhibitCascadingIfElse.
that one sounds attractive, e.g. for main.pm if-else madness?
4 violations of Modules::ProhibitExcessMainComplexity.
16 violations of RegularExpressions::ProhibitUselessTopic.
49 violations of Subroutines::ProhibitExcessComplexity.
915 violations of TestingAndDebugging::RequireUseWarnings.
we should check this why we have so many violations
474 violations of ValuesAndExpressions::ProhibitEmptyQuotes.
1 violations of ValuesAndExpressions::ProhibitLongChainsOfMethodCalls.
Updated by JERiveraMoya over 5 years ago
- Status changed from Feedback to Resolved
Thanks, I agree, [ControlStructures::ProhibitCascadingIfElse] sounds attractive, the risk is to change logic and make fail multiple testsuites (but perhaps it is easier to fix that I can think of right now).
For those that hit too many, it might be too late, but sure, we can discuss with the team, I guess after the demo (in the retro?) would be more opportunity for the rest of the team to know what was done in this task and anyone can rise hand about some policy that they'd rather to add/remove according to their individual experiences.