fix(lint): ARCH002 and ARCH003 compare entity_type to a value the linter never returns - #1165
Open
tgolembiewski wants to merge 3 commits into
Open
tgolembiewski wants to merge 3 commits into
tgolembiewski wants to merge 3 commits into
Conversation
ARCH002 and ARCH003 guard with `entity.entity_type != "PERSISTENT"`. The catalog stores PERSISTENT, but LintContext.Entities normalizes the kind to Persistent (the CASE in its Entities query) before a Starlark rule reads it, so the guard held for every entity and neither rule's body ever ran. Nothing said so: no error, no output, and both still appeared under `--list-rules`, which reads as a project with nothing to report. Measured on a generated app with five persistent entities, none of them carrying a unique attribute: ARCH003 reported 0 findings before the change and 5 after. The test loads the two shipped rule files themselves against a one-entity fixture, so a rule that goes back to the stored spelling fails here rather than going quiet again. Reverting either fix in turn takes that rule's subtest to "got 0 violations, want 1".
The field table and the worked example in write-lint-rules both used "persistent", a third spelling that matches nothing. A rule written from either reads as working and reports on no project at all, which is how the two rules fixed in the previous commit were written.
AI Code ReviewWhat Looks Good
RecommendationApprove the pull request. The fix is correct, well-tested, properly documented, and follows all project guidelines. The PR resolves the silent failure of two bundled lint rules without introducing any side effects. Automated review via OpenRouter (Nemotron Super 120B) — workflow source |
This branch has not been deployed
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.
Closes #1164
What
ARCH002andARCH003skip every entity, so neither has ever reported anything on any project:The catalog stores
PERSISTENT;LintContext.Entitiesnormalizes the kind toPersistentbefore a Starlark rule reads it. The comparison is therefore true for every entity and each rule's body never runs — with no error, no output, and the rule still listed by--list-rules, which is indistinguishable from a clean project. The other ten bundled rules that filter on the kind already use"Persistent".The
write-lint-rulesskill documented a third spelling,"persistent", in both its field table and its worked example; a rule written from that page has the same silent failure.Measurements
--rules ARCH003No issues found.--rules ARCH003Commits
fix(lint)— the two rules, plus a test that loads the shipped rule files themselves against a one-entity fixture.docs(skill)— the field table and the example inwrite-lint-rulesnow say what the API returns.docs— CHANGELOG entry under[Unreleased]and one finding record.Tests
TestShippedEntityRulesSeeAPersistentEntityloads.claude/lint-rules/entity_business_key.starand.claude/lint-rules/data_change_microflows.star— the files that ship — and asserts each reports the one persistent entity in the fixture. Reverting either fix in turn takes that rule's subtest togot 0 violations, want 1, so the test detects the defect it is here for rather than the code around it.Validation
Mendix 11.12.1, MPR v2, macOS. The whole of
push-test.ymlrun locally and green:make build,make test,check-tunnel-deps,make check-mdl,make check-skill-pack-js,make check-findings,make check-wiki-pages,check-skill-mdl.shover.claude/skills/mendixanddocs-site/src,make test-integrationagainstmx11.12.1,make lint-go,govulncheck(0 vulnerabilities in called code).No MDL syntax changes and no behaviour change outside the two rules.