build(ci): bump ty to 0.0.70 and skip the PR title check for Dependabot - #244
Open
r0ny123 wants to merge 2 commits into
Open
build(ci): bump ty to 0.0.70 and skip the PR title check for Dependabot#244r0ny123 wants to merge 2 commits into
r0ny123 wants to merge 2 commits into
Conversation
ty 0.0.70 turns an unchanged tree red: 36 error-level diagnostics where 0.0.67 through 0.0.69 reported none. Two distinct causes. unsound-return-statement (29) and unsound-yield (5) are new in 0.0.70 and ship with severity "ignore"; checking the tree against stock rule defaults produces zero of them. They are errors here only because [tool.ty.rules] all = "error" enables every rule the checker has, including ones that did not exist when that line was written. Both fire wherever a value inferred as Unknown reaches an annotated return or yield, which is most of a largely unannotated codebase, and the suggested remedy is an assert at each site for no runtime benefit. Downgraded to "warn", matching how unresolved-attribute already handles third-party dynamism: visible in output, non-blocking under error-on-warning = false. The remaining two are real. Both SmdaFunction and SmdaReport declare binweight = 0, so its type infers as int, while SmdaFunction accumulates float(sum(...)) into it deliberately - the value is serialized and must stay a float. Annotated as float in both classes. The default literal stays 0 rather than 0.0 on purpose: it is emitted in toDict output for a function with no blocks, and 0.0 would change that serialized form. make typecheck now exits 0 with 263 diagnostics, all warnings.
Dependabot titles its PRs `build(deps-dev): ...` (or `build(deps): ...`), and neither scope is on this workflow's allowlist, so every dependency update opens with a failing required check. The title is generated by the bot and a maintainer cannot edit it without closing the PR, leaving no way to clear the failure other than merging past it. Skip the job for PRs opened by dependabot[bot] rather than widening the scope list, which would also let a human PR use a deps scope.
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
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.
Summary
Two CI fixes. Bumps the pinned
tyfrom 0.0.68 to 0.0.70, downgrading the two rules that release introduced and fixing the one genuine type error it surfaced, somake typecheckgoes back to exiting 0. Also exempts Dependabot from the Semantic PR Title check, which currently fails on every dependency update.Why the bump is not mechanical
ty 0.0.70 turns an unchanged tree red: 36
error[...]diagnostics, where 0.0.67, 0.0.68 and 0.0.69 all report the same 229 diagnostics with none at error level and exit 0. The Code Quality job fails on any PR that picks up the newer version. The 36 split into two unrelated causes.34 are new rules that the config opts into before they exist.
unsound-return-statement(29 hits) andunsound-yield(5) are new in 0.0.70 and ship with severityignore— checking this tree against stock rule defaults produces zero of them. They are errors here only because[tool.ty.rules] all = "error"enables every rule the checker has, including ones added after that line was written. Both fire wherever a value inferred asUnknownreaches an annotatedreturnoryield, which is most of a largely unannotated codebase; the sites cluster inDisassembler.py,SmdaReport.py,SmdaFunction.py, the Delphi label providers, the three synthesizers andStringExtractor.py. ty's suggested remedy is anassertat each site, which would add runtime work and noise for no behavioral gain, so both rules are set towarn— the same treatmentunresolved-attributealready gets for third-party dynamism. They stay visible in output and are non-blocking undererror-on-warning = false.2 are real, and come from an inference improvement.
SmdaFunctionandSmdaReportboth declarebinweight = 0, so its type infers asint, whileSmdaFunction._parseBlocksdeliberately accumulatesfloat(sum(...))into it — the value is serialized, so it has to stay a float. 0.0.70 started catching the widening; earlier versions did not.Changes
pyproject.toml: pin moved toty==0.0.70, with the pin comment restated around the actual mechanism (all = "error"opting into not-yet-existing rules) rather than "adds lint rules in patch releases".pyproject.toml:unsound-return-statementandunsound-yieldset towarnin[tool.ty.rules].SmdaFunctionandSmdaReport:binweightannotated asfloat.The
binweightdefault literal stays0rather than becoming0.0on purpose. It reachestoDict()output for a function with no blocks, so0.0would change the serialized form of existing reports. This is an annotation-only change with no runtime effect.Dependabot and the PR title check
Dependabot titles its PRs
build(deps-dev): ...orbuild(deps): ..., and neither scope is on the workflow's allowlist, so every dependency update opens with a failing required check — #242 is the current example. The title is generated by the bot, and a maintainer cannot edit it without closing the PR, so there is no way to clear the failure other than merging past it.The job is now skipped for PRs opened by
dependabot[bot], rather than addingdeps/deps-devto the scope list, which would also let a human PR use them. Because the workflow runs onpull_request_target, it is evaluated from the base branch's copy — the skip takes effect for Dependabot PRs once this is merged, and does not retroactively clear the check on #242.Validation
Diff coverage of the two changed source lines is 100% against
master.The "no errors under stock defaults" claim above was checked directly, by running 0.0.70 with a configuration that silences only the pre-existing import/attribute noise and leaves every other rule at its shipped severity; neither
unsound-*rule produced a diagnostic. The version boundary was bisected by running 0.0.68, 0.0.69 and 0.0.70 against this same tree.