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, |
|
Jour Fixe, 13 JUL 2026: Fabian presented a new automatic testing infrastructure to identify architectural violations. It cannot be merged before the 66 violations of the first rule have not been fixed. But once this is done, this infrastructure should be available for every developer of core components. Additional rules will be added per PR in the future. |
3 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