WIP: Improvement/12/ci code rules superglobals#11729
Conversation
…espace Move the legacy-UI usage rules from scripts/PHPStan/Rules into a dedicated Rules/LegacyUI subdirectory and namespace (ILIAS\Scripts\PHPStan\Rules\LegacyUI), and update legacy_ui.neon to the new fully-qualified class names. Pure refactor, no behavioural change.
Introduce a second, gating PHPStan ruleset (code_rules.neon) next to the existing legacy-UI report, run by the code-rules.yml GitHub Action via scripts/PHPStan/run_code_rules.sh (ERROR_FORMAT selects the output format). Ships the in-code exemption mechanism — the AllowRuleViolation attribute (class/method/ function scoped, keyed by rule identifier) and the RuleViolationAllowance checker that rules call to skip exempted positions — and StepSummaryFormatter, a custom PHPStan error formatter that renders the violations as a Markdown summary for the GitHub step summary, reading each rule's human-readable name from the error metadata. No policy rules are registered yet; exemptions live in the code, there is no baseline.
Add the SuperGlobals rule set: writing to $_GET / $_POST / $_REQUEST / $_COOKIE / $_FILES (plain, compound and by-ref assignment, incl. nested keys, appends and whole-array overwrites) is forbidden — the request is immutable. Ships the AllowSuperglobalWrite convenience attribute and exempts ilInitialisation, which has to sanitize the request before the HTTP service exists.
|
Results will be shown here: https://github.com/ILIAS-eLearning/ILIAS/actions/runs/28867914340/job/85622704513?pr=11729 / Table here: https://github.com/ILIAS-eLearning/ILIAS/actions/runs/28867914340?pr=11729 The first run takes quite a long time but caching should make further runs faster |
|
The Summary outputs a Table with violations (per rule) and top components, e.g. ILIAS Code Rules — 66 violations
Top components
|
|
Some more exception of the rule here: 928efd5 Results in ILIAS Code Rules — 60 violations
Top components
This (cached) run only took 52s, the initial run took 8m 15s. |
|
Hi @chfsx , This is a really valuable contribution, thank you for working on this. In my opinion, this is a major step forward for identifying architectural violations across the code base. Since we removed the dependency on I can see this becoming an important part of preserving the overall structure and quality of the code base going forward. I will remove the problematic case in the Best regards, |
2 participants
as mentioned in #11471 I here'd like to introduce the basic infrastructure for ILIAS PHStan-Rules and it's first usage to prevent more SuperGlobal changes.
I must mark this as WIP for the moment since we still have 66 violations. as you can see, there is a possibility to exclude a class/method from a violation-report as I implemented in ilInitialisation: 1b7048d#diff-1a374c973bc1ede22ce53539af43a3911c28bb6dcefb858ac77d2fc3000a8344R55
but there are several other location where the authorities must have a look first