Green tooling_tests, and two false positives the sibling corpora found - #163
Merged
Conversation
Porting the same four cleanups to samples and samples-stack exercised these rules against dispatchers and statements this corpus does not contain, and both rules were wrong at the edges. redundant-init-display matched on the init arm alone, so it would have flagged samples' apps 012 and 488 (whose navigated branch calls on_navigation( ), not view_display( )) and app 024 (which handles an app return there first). In all three the fork DOES decide something. It now requires both arms to be the bare view_display( ), and tolerates the blank lines samples puts between branches - which is also what made the corpus-wide fix miss two classes there until the pattern was widened. stacked-short-call would have joined a string template split with && - which abaplint then rejects with reduce_string_templates, found on samples-stack's app 007 - and the sections of a RAP statement (COMMIT ENTITIES ... FAILED ... REPORTED), which carry meaning stacked exactly as EXPORTING/EXCEPTIONS do. Both are exempt now. Re-measured against this corpus as it stood before the cleanup: 201 / 145 / 77, unchanged. The rules lost no coverage here, only the false positives. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTmU5ANuAvvSTEUdqUpVag
…ects tooling_tests went red on the PR: test 29 builds a fixture port that is asserted to be CLEAN, and its dispatcher was the redundant `IF check_on_init( ). view_display( ). ELSEIF check_on_navigated( ). view_display( ).` - which redundant-init-display now reports, correctly. My mistake in sequencing, not in the rule: I ran the tooling tests right after changing json-to-abap.mjs and never again after inserting the three new rules. The fixture now carries the house form the corpus was moved to. node --test scripts/test/tooling.test.mjs: 32 pass, 0 fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTmU5ANuAvvSTEUdqUpVag
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.
Follow-up to #160, which merged at
75be0e5— before these two commits existed.mainis currently red:tooling_testsfails on it.1.
tooling_testsis red on main — one fixture lineTest 29 (
pattern-lint: an ABAP Doc header on a port is an error, and the clean fixture passes) builds a fixture port that is asserted to be clean, and its dispatcher was the very shape #160's newredundant-init-displayrule rejects:The rule is right; the fixture was stale. My mistake in sequencing — I ran the tooling tests right after changing
json-to-abap.mjsand never again after inserting the three new rules.node --test scripts/test/tooling.test.mjs: 32 pass, 0 fail.2. Two false positives, found by porting the same change to the sibling corpora
Carrying #160's cleanups into abap2UI5/samples#807 and abap2UI5/samples-stack#63 ran the new rules against dispatchers and statements this corpus does not contain, and both rules were wrong at the edges:
redundant-init-displaymatched on the init arm alone, so it would have flagged three legitimate dispatchers insamples: apps 012 and 488 callon_navigation( )rather thanview_display( )in the navigated branch, and app 024 handles an app return there first. In all three the fork does decide something. It now requires both arms to be the bareview_display( ), and tolerates the blank linessamplesputs between branches — which is also what made the corpus-wide fix miss two classes there until the pattern was widened.stacked-short-callwould have joined a string template split with&&— which abaplint then rejects withreduce_string_templates, found on samples-stack's app 007 — and the sections of a RAP statement (COMMIT ENTITIES … FAILED … REPORTED), which carry meaning stacked exactly asEXPORTING/EXCEPTIONSdo. Both are exempt now.Coverage is unchanged. Re-measured against the corpus as it stood before #160: 201 / 145 / 77 findings, the same three sets that were fixed. The rules lost only the false positives.
Verification
tooling_tests(32/0),pattern-lint(0 errors, 388 warnings — identical to before),check:chains(637 files, 0 failing), prose-names, pins, archive, validate-meta, mcp-contract,structural-diff --strict,data-fidelity, andmeta_valid(all five generators re-run,git diffclean).🤖 Generated with Claude Code
https://claude.ai/code/session_01XTmU5ANuAvvSTEUdqUpVag
Generated by Claude Code