Define "legacy exclusions" as discouraged rules
jjanvier opened this issue · 3 comments
in strict mode, discouraged rules are counted and displayed as violations
in "cool" mode, they are not
I'm trying to figure out if this approach is useable in our context.
We want to use this tool on our CI to forbid new violations, in this context, it's only useable as "cool not strict mode" because we want 0 issues and raise only new ones. Problem is if a Rule is "discouraged", until we fix it and pass it as "forbidden", new issues can occur with this not enforced rule.
Today, with our 7 rules, 4 rules cannot be used as "forbidden":
- Akeneo\Component : 3 issue
- Pim\Component : 323 issues
- Pim\Bundle\CatalogBundle: 40 issues
- Pim\Bundle\ConnectorBundle : 1 issue
Let's take this last issue as example, the Rule "Pim\Bundle\ConnectorBundle" violated in file "src/Pim/Bundle/ConnectorBundle/Writer/File/ContextableCsvWriter.php" because use Pim\Bundle\BaseConnectorBundle\Writer\File\CsvWriter
ContextableCsvWriter has been introduced in a 1.4 patch in a no BC mode, in 1.5 we re-worked/moved it, we depreciate this class with an expected removing in 1.6. Means the rule "Pim\Bundle\ConnectorBundle" cannot be used as "forbidden" until 1.6 and we can introduce new violations on new developments during this period because we cannot use this rule as strict.
We could split these rules in smaller rules or deal with exclude inside the rules with regex but it will make the definition of rules far harder.
We could also define 'only' rules but issue with this approach is we have to list all "uses", quite doable in a already defined component for instance, but far more harder in a brand new dev or in a bundle which move a lot (EnrichBundle for instance).
Digging and trying stuff that i will share here :)
Trying to compare with other tools, it's like if when we use phpmd we say the CyclomaticComplexity rule is not enforced at all because i already have 10 violations.
In this case, the use of 10 "suppressWarning" option seems a good compromise to me because it's less permissive than totally muting the rule and forbid new violations to occur.
In phpmd this "suppressWarning" is grabbed from the Node because they use annotations, here i would prefer a configuration not enforced in the code itself but that you can easily define in the conf of the project.
cf http://phpmd.org/rules/codesize.html + http://phpmd.org/documentation/suppress-warnings.html
@jjanvier i'm continuing to dig into our real use cases related to violations to see if most of these can be easily fixed and what are those we should keep for longer time due to deprecated strategy (for instance by moving catalog bundle models to catalog component we solve 144 / 367 violations in CE).