Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"phpcs": "./vendor/bin/phpcs --standard=phpcs.xml",
"phpcs:fix": "./vendor/bin/phpcbf --standard=phpcs.xml",
"phpcs:output": "./vendor/bin/phpcs --standard=phpcs.xml --report=json lib/ 2>/dev/null | tail -1 > phpcs-output.json",
"phpmd": "./vendor/bin/phpmd lib text phpmd.xml --baseline-file phpmd.baseline.xml",
"phpmd": "E=0; ./vendor/bin/phpmd lib text phpmd.xml --baseline-file phpmd.baseline.xml || E=$?; ./vendor/bin/phpmd lib text phpmd-unusedparams.xml --baseline-file phpmd.baseline.xml || E=$?; exit $E",
"phpmetrics": "./vendor/bin/phpmetrics --report-html=phpmetrics lib/",
"phpmetrics:violations": "./vendor/bin/phpmetrics --violations-xml=phpmetrics/violations.xml lib/",
"psalm": "if [ -f vendor/bin/psalm ]; then ./vendor/bin/psalm --threads=1 --no-cache; else echo 'Psalm not installed, skipping...'; fi",
Expand Down
40 changes: 40 additions & 0 deletions phpmd-unusedparams.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?xml version="1.0"?>
<ruleset name="Unused formal parameters"
xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 http://pmd.sourceforge.net/ruleset_xml_schema.xsd">

<description>
Second PHPMD leg: UnusedFormalParameter only.

This rule lives in its own ruleset because PHPMD honours &lt;exclude-pattern&gt;
ONLY as a direct child of &lt;ruleset&gt;; nested inside a &lt;rule&gt; it is parsed and
discarded (ConductionNL/.github#155). A direct child, however, is applied by
PDepend at file-collection time and drops the file from EVERY rule in the
ruleset. Isolating UnusedFormalParameter here means the lib/Migration
exclusion applies to this rule and this rule ALONE - every other rule still
analyses lib/Migration in the main leg.

Why lib/Migration is excluded from THIS rule: OCP\Migration\IMigrationStep
mandates changeSchema(IOutput $output, Closure $schemaClosure, array $options)
and preSchemaChange/postSchemaChange with the same three parameters. A step
that needs none of them still cannot drop them. The signature cannot change.

The pattern is */lib/Migration/* and deliberately NOT */Migration/* or
*Migration*. Those broader forms silently swallow ORDINARY CLASSES that merely
have "Migration" in their path - measured on openconnector, */Migration/* hides
a genuine finding in lib/Service/Migration/, and *Migration* additionally hides
every lib/Service/MigrationService.php, lib/Controller/MigrationController.php
and lib/Db/*Migration*.php. Those have no mandated signature and must stay
analysed. Only the app's own lib/Migration/ directory holds IMigrationStep
implementations, so only that directory is exempted.

Both legs must always run - see the "phpmd" script in composer.json, which
keeps the worst exit code rather than letting the first leg short-circuit
the second.
</description>

<exclude-pattern>*/lib/Migration/*</exclude-pattern>

<rule ref="rulesets/unusedcode.xml/UnusedFormalParameter"/>
</ruleset>
26 changes: 23 additions & 3 deletions phpmd.xml
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,27 @@
<rule ref="rulesets/unusedcode.xml/UnusedPrivateField"/>
<rule ref="rulesets/unusedcode.xml/UnusedLocalVariable"/>
<rule ref="rulesets/unusedcode.xml/UnusedPrivateMethod"/>
<rule ref="rulesets/unusedcode.xml/UnusedFormalParameter">
<exclude-pattern>*Migration*</exclude-pattern>
</rule>
<!-- UnusedFormalParameter is NOT declared here. It lives alone in
phpmd-unusedparams.xml, which is run as a second leg by the "phpmd"
composer script.

Why: this rule used to carry a NESTED
<exclude-pattern>*Migration*</exclude-pattern>
meant to spare lib/Migration, whose changeSchema/preSchemaChange/
postSchemaChange signatures are mandated by OCP\Migration\IMigrationStep
and cannot drop their unused parameters. That pattern was INERT: PHPMD
2.15 reads exclude-patterns in RuleSetFactory::getIgnorePattern(), which
walks $xml->children() - i.e. only elements DIRECTLY under <ruleset>. A
nested one parses without error and does nothing, so lib/Migration was
always scanned by this rule (ConductionNL/.github#155).

Moving the pattern up to the top level of THIS file would make it
effective, but it is applied by PDepend's ExcludePathFilter at
file-collection time, so it drops lib/Migration from EVERY rule in the
ruleset - real complexity, StaticAccess and method-length findings in
migrations would silently vanish.

Isolating the rule in its own ruleset gives the exclusion a top-level
home that scopes it to this one rule, while the main leg above keeps
analysing lib/Migration with everything else. -->
</ruleset>
Loading