search

MITLibraries / MITLibraries-child

A Wordpress child theme that descends from MITLibraries-parent
GNU General Public License v3.0
1 stars 1 forks source link

Whitelists known analysis failures. #80

Closed matt-bernhardt closed 8 years ago

matt-bernhardt commented 8 years ago

This branch implements an approach that @JPrevost recommended to me some time ago, but I've resisted until now. Specifically, when analyzing projecs with large amounts of technical debt, the approach is to whitelist the checks that we know will fail, in order to focus on finding new problems.

To walk through this approach, check out the result of one of our earliest analyses of this theme:

PHP CODE SNIFFER VIOLATION SOURCE SUMMARY
----------------------------------------------------------------------
    SOURCE                                                       COUNT
----------------------------------------------------------------------
[x] Generic.WhiteSpace.DisallowSpaceIndent.SpacesUsed            329
[x] PEAR.Functions.FunctionCallSignature.SpaceBeforeCloseBracke  210
[x] PEAR.Functions.FunctionCallSignature.SpaceAfterOpenBracket   207
[x] Squiz.WhiteSpace.SuperfluousWhitespace.EndLine               131
[ ] WordPress.NamingConventions.ValidVariableName.NotSnakeCase   101
[x] WordPress.WhiteSpace.ControlStructureSpacing.NoSpaceBeforeC  97
[x] WordPress.WhiteSpace.ControlStructureSpacing.NoSpaceAfterOp  95
[ ] Squiz.Commenting.InlineComment.InvalidEndChar                65
[ ] WordPress.XSS.EscapeOutput.OutputNotEscaped                  64
[x] Squiz.ControlStructures.ControlSignature.SpaceAfterKeyword   36
[x] Squiz.Strings.DoubleQuoteUsage.NotRequired                   34
[x] WordPress.WhiteSpace.ControlStructureSpacing.NoSpaceAfterSt  31
[x] Squiz.Strings.ConcatenationSpacing.PaddingFound              31
[x] WordPress.WhiteSpace.ControlStructureSpacing.NoSpaceBeforeO  29
[x] WordPress.WhiteSpace.ControlStructureSpacing.NoSpaceBetween  27
[x] Generic.Files.EndFileNewline.NotFound                        26
[x] Squiz.ControlStructures.ControlSignature.SpaceAfterClosePar  23
[x] WordPress.Arrays.ArrayDeclaration.NoComma                    20
[ ] WordPress.PHP.YodaConditions.NotYoda                         13
[x] Squiz.Commenting.InlineComment.NoSpaceBefore                 11
[x] Generic.ControlStructures.InlineControlStructure.NotAllowed  9
[x] WordPress.Arrays.ArrayDeclaration.NoCommaAfterLast           9
[x] WordPress.WhiteSpace.OperatorSpacing.NoSpaceAfter            9
[ ] Squiz.Commenting.FileComment.SpacingAfterComment             9
[ ] Generic.Strings.UnnecessaryStringConcat.Found                9
[ ] Squiz.Commenting.FunctionComment.Missing                     9
[ ] WordPress.XSS.EscapeOutput.UnsafePrintingFunction            8
[x] WordPress.Arrays.ArrayDeclaration.NoSpaceAfterOpenParenthes  7
[x] WordPress.WhiteSpace.OperatorSpacing.NoSpaceBefore           7
[ ] Squiz.Commenting.FileComment.Missing                         6
[x] Squiz.Functions.FunctionDeclarationArgumentSpacing.SpacingB  5
[x] Squiz.Functions.FunctionDeclarationArgumentSpacing.SpacingA  5
[ ] Squiz.Commenting.BlockComment.NoEmptyLineBefore              4
[ ] WordPress.VIP.PostsPerPage.posts_per_page                    4
[ ] Squiz.Commenting.FileComment.WrongStyle                      4
[x] Generic.Formatting.DisallowMultipleStatements.SameLine       3
[x] Squiz.Commenting.DocCommentAlignment.SpaceBeforeStar         3
[ ] Squiz.Commenting.FileComment.MissingPackageTag               3
[ ] Squiz.Commenting.FunctionComment.WrongStyle                  3
[ ] Generic.Files.LowercasedFilename.NotFound                    3
[x] Generic.Commenting.DocComment.SpacingBeforeTags              2
[ ] WordPress.NamingConventions.ValidFunctionName.FunctionNameI  2
[ ] WordPress.VIP.RestrictedFunctions.switch_to_blog             2
[x] WordPress.WhiteSpace.ControlStructureSpacing.NoSpaceAfterCl  1
[x] Generic.Functions.OpeningFunctionBraceKernighanRitchie.Spac  1
[x] WordPress.Arrays.ArrayDeclaration.CloseBraceNewLine          1
[x] Squiz.WhiteSpace.SuperfluousWhitespace.StartFile             1
[x] Squiz.WhiteSpace.SuperfluousWhitespace.EndFile               1
[x] Generic.Commenting.DocComment.SpacingAfter                   1
[ ] Squiz.Commenting.FunctionComment.MissingParamTag             1
[ ] Squiz.Commenting.BlockComment.NoCapital                      1
----------------------------------------------------------------------
A TOTAL OF 1713 SNIFF VIOLATIONS WERE FOUND IN 51 SOURCES
----------------------------------------------------------------------
PHPCBF CAN FIX THE 32 MARKED SOURCES AUTOMATICALLY (1402 VIOLATIONS IN TOTAL)
----------------------------------------------------------------------

Compare that with a more recent run:

PHP CODE SNIFFER VIOLATION SOURCE SUMMARY
----------------------------------------------------------------------
    SOURCE                                                       COUNT
----------------------------------------------------------------------
[x] Generic.WhiteSpace.DisallowSpaceIndent.SpacesUsed            190
[ ] WordPress.NamingConventions.ValidVariableName.NotSnakeCase   62
[ ] WordPress.XSS.EscapeOutput.OutputNotEscaped                  44
[x] Squiz.Strings.ConcatenationSpacing.PaddingFound              24
[ ] Generic.Strings.UnnecessaryStringConcat.Found                14
[ ] WordPress.XSS.EscapeOutput.UnsafePrintingFunction            10
[ ] WordPress.VIP.PostsPerPage.posts_per_page                    4
[ ] WordPress.NamingConventions.ValidFunctionName.FunctionNameI  2
[ ] WordPress.VIP.RestrictedFunctions.switch_to_blog             2
[ ] Generic.Files.LowercasedFilename.NotFound                    1
----------------------------------------------------------------------
A TOTAL OF 353 SNIFF VIOLATIONS WERE FOUND IN 10 SOURCES
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SOURCES AUTOMATICALLY (214 VIOLATIONS IN TOTAL)
----------------------------------------------------------------------

Through our efforts, we've caused a large number of sniffers to pass - but the suite as a whole still fails. This is somewhat risky, because new PRs can introduce new problems that might get lost in the noise of failures we already know about. By explicitly whitelisting the tests above, we can cause a passing test - which then causes new problems to stand out.

matt-bernhardt commented 8 years ago

An intrinsic part of this change, if adopted, is a commitment to not forget about the "ignored" problems - that there would need to be work done to resolve (or pardon) the problems on this list. Some things may never be resolvable - one example might be the switch_to_blog checks, which may be necessary given our goals. But we should make sure we know what those problems are, and not just forget them / turn a blind eye.