ci: add a PR gate that lints the generated code - #69
Merged
Conversation
- 新增 scripts/ci-syntax.sh，把语法闸做成「全量扫描 + 分级判定」： G1 `php -l` 抓解析错误与编译期符号冲突（模板漏去重导致的重复 `use`）； G2 scripts/dup-check.sh 抓同一文件内重复的顶层 class/interface/trait —— 这类重复 `php -l` 一律 exit 0，属运行期错误，必须单独查。 两段都全量扫 src/，但退出码只由本轮变更的文件决定，其余问题只告警。 为什么分级：仓里存量有 23 个 *Response.php 带重复 `use`（12 个产品、81 行）， 整棵树一律阻断会让**任意一个**产品的 codegen PR 被这些无关的存量问题拦红， check 变红 → mergeable_state 转 unstable → 自动合并永久失效；而这些文件是 生成产物，只有对应产品各自重新发布才会被修好的模板覆盖，等多久不可控。 同一后果在 python 侧已被实证并否决（pipeline 2507836 发的是 UHost+UAI-Modelverse、 根本没重新生成 uphost，uphost 的存量问题照样被扫出来），2026-07-31 定的处置是 「判定边界取本轮变更文件，存量只告警」，见 ucloud-sdk-release/scripts/ python-fatal-check.sh 文件头。本次是同一决策在 php 仓的落地。 存量仍全量扫、全量打印：不阻断不等于不看见，债务要保持可度量。 变更集来自环境变量 CI_BASE_SHA，严格集取 `git diff --name-only $CI_BASE_SHA...HEAD` 在 src/ 下的 .php（三点 diff，基线分支前进过也只算 PR 自己引入的改动）。 fail-closed：未设置、解析不出 commit、diff 失败，一律退回整棵树严格判定并打印 退回原因。本地不设该变量跑 `make ci-syntax` 看到的就是整棵树严格，与改造前一致。 实测 `php -l` 失败时退出码为 255，GNU xargs 见到 255 会立刻中止、只报第一个坏文件 （BSD xargs 无此行为，本地跑不出来）。故 wrapper 无论 php 退什么码都 exit 0， 「哪个文件坏了」改用结果文件传出，判定不依赖 xargs 退出码；再用「结果行数必须等于 输入文件数」兜底，一旦提前中止就 FATAL，不会当成没问题。 - scripts/dup-check.sh 增加 `--strict-list <清单文件>`：目录照样全量扫， 只有清单内文件的命中影响退出码并标 [严格]，清单外标 [告警]。 不给该参数时行为与改造前完全一致，既有用法不受影响。 - php.yml 改名 ci.yml：setup-php 8.3 + composer install， ci-syntax / lint / test 三个 job 汇聚到 ci-gate。 分支保护只需把 ci-gate 配成 required check，其余 job 增删不影响它。 触发从裸 [push, pull_request] 收窄到 master；checkout v1 升到 v7。 ci-syntax job 加 fetch-depth: 0（默认浅克隆拿不到 base sha，会静默退回整棵树严格） 并注入 CI_BASE_SHA：pull_request 取 base.sha，push 取 before。 codecov 上传是 test job 内的 step 且 continue-on-error，不进 ci-gate 的 needs。 - 新增 compat.yml：定时跑 make compatible（PHPCompatibility，testVersion 5.6）， 这是 composer.json 里 "php": ">=5.6" 这条下界唯一的真实验证手段，此前无 CI 在跑。 只挂 schedule + workflow_dispatch，不在 PR 上产生 check。 - test-cov 额外产出 coverage.xml 供 codecov 消费。原先这个产物是 workflow 里 裸跑 phpunit 得到的；现在语言命令一律只经 Makefile 走，就得由 target 自己出。 - 不动 .gitlab-ci.yml。
兼容矩阵是纯巡检，不参与门禁判定，也没人会盯一个非阻断的定时任务。 声明下界还成不成立是另一个问题，要验的时候手工跑一次即可， 不必为它常驻一条每天运行的 workflow。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
php.yml→ci.yml—ci-syntax/lint/test, aggregated byci-gate.github/workflows/compat.yml— runs the existingmake compatible(PHPCompatibility,testVersion 5.6), which nothing in CI was runningscripts/ci-syntax.sh+scripts/dup-check.shMakefile:ci-syntaxtarget;test-covalso emitscoverage.xml.gitlab-ci.ymlis untouched.The two gates
php -lover every file insrc/scripts/dup-check.sh php src— duplicate top-level class/interface/trait in one fileG2 is not redundant:
php -lcatches duplicateuse(compile-time symbol clash) but exits 0 on twoclass Fooin the same file — that's a runtime error, invisible to lint. Verified both ways on this repo.A trap worth flagging:
php -lexits 255, and xargs aborts on 255The obvious spelling is wrong, and it fails in the direction that looks fine:
Measured in a
php:8.3-clicontainer (GNU findutils xargs 4.10.0, i.e. whatubuntu-latesthas) against this repo, which currently has 23 bad files:xargs … php -lxargs: php: exited with status 255; abortingscripts/ci-syntax.shThe exit code is non-zero either way, so it looks like the gate works — it just silently hides 22 of the 23. macOS's BSD xargs doesn't abort on 255, so testing on a Mac gives a false sense of safety.
The script sidesteps xargs' exit code entirely: the wrapper always exits 0 and which file failed is carried out through a results file. There is also a guard that the number of result lines equals the number of input files (2293/2293) — that guard is the regression detector for this trap.
Only this PR's files block
ci-syntaxscans all ofsrc/but only fails on files changed in this PR; hits in untouched files print as warnings and don't affect the exit code.Without this, the 23 pre-existing bad files would red-flag every product's codegen PR until all 12 affected products happen to be released — blocking releases for reasons unrelated to their content.
Changed set:
git diff --name-only --diff-filter=d "$CI_BASE_SHA...HEAD" -- src(three-dot).CI_BASE_SHAis supplied by the workflow, withfetch-depth: 0on checkout. Fail-closed: unset, unresolvable, not a git repo, or a failed diff all fall back to strict whole-tree and print the reason. Localmake ci-syntaxwith no variable is strict whole-tree, so local and CI never disagree silently.Verified locally
CI_BASE_SHA(strict whole tree)useinjected into a changed filephp -lalone exits 0 — G2 is what catches it)CI_BASE_SHAmake lint/make test-cov/make compatibleExpected:
ci-gatewill be red on merge23
*Response.phpfiles across 12 products emit a duplicateuse, which is a PHP fatal — the class cannot be instantiated. The cause was a generator template bug, already fixed upstream; these files are build output and get corrected when their product is next regenerated. Please don't hand-edit them — they would grow back on the next codegen. Until then they are warnings and block nothing, and the warning count is a usable progress metric.