bug(#4411): idempotency attribute inside formation#4463
bug(#4411): idempotency attribute inside formation#4463yegor256 merged 11 commits intoobjectionary:masterfrom
Conversation
WalkthroughIntroduces and propagates a sentinel object base ξ.xi🌵 through parsing and XSL transforms, adjusts Java transpilation to guard/handle bound attributes and anonymous abstract generation, changes listener routing for Tformation, and updates numerous tests and snapshots to reflect the new sentinel positioning and idempotency behavior. Disables three Maven transpile tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Src as EO Source
participant Parser as XeEoListener
participant XMIR as XMIR (AST)
participant XSL1 as move-voids-up.xsl
participant XSL2 as print/to-eo.xsl
participant XSL3 as transpile/to-java.xsl
participant J as Java Output
Src->>Parser: Parse (includes Tformation)
Note over Parser: Tformation -> startTest()<br/>Abstract -> startAbstract()<br/>sets inner base=ξ.xi🌵
Parser->>XMIR: Build objects (include ξ.xi🌵 sentinels)
XMIR->>XSL1: Move/normalize voids
Note over XSL1: Emit o[@base='ξ.xi🌵'] early<br/>Exclude them from non-void branch
XSL1->>XMIR: Rewritten XMIR
XMIR->>XSL2: Print to EO (for snapshots)
Note over XSL2: Exclude base=ξ.xi🌵 in non-free attr template
XMIR->>XSL3: Transpile to Java
Note over XSL3: Guard unnamed attrs when bound=ξ.xi🌵<br/>Refine .add(...) emission and PhDefault<br/>Prefer PhDefault for idempotent single-inner
XSL3-->>J: Generated Java
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
🚀 Performance AnalysisAll benchmarks are within the acceptable range. No critical degradation detected (threshold is 100%). Please refer to the detailed report for more information. Click to see the detailed report
✅ Performance gain: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/auto-named-abstract.yaml (1)
16-22: Update remaining print-pack fixtures for apostrophe-suffixed targets and referencesI ran the suggested checks and discovered several stale, unsuffixed
>>occurrences in our YAML fixtures. These must be updated to the new>>'convention (and any un-apostrophed target names, likey, should likewise be suffixed):• eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/const-auto-named.yaml
– line 9:5 > [] >>!
– line 15:[] >>!'
• eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/auto-named-abstract.yaml
– line 9:[] >>
– line 12:[f]:1 >>Please update those
>>instances to>>'(and verify anyytargets are renamed toy'), so the fixtures consistently reflect the PR-wide printing rules.Optional: for future verification, you can use corrected ripgrep patterns such as:
rg -nP '\[\] > y(?!\')' eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml rg -nP '>>(?!\')' eo-parser/src/test/resources/org/eolang/parser/print-packs/yamlto catch any remaining unsuffixed references.
eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/dataless.yaml (1)
22-31: Inconsistent print-pack fixtures: missingmain'suffix and lingeringQQ.prefixesA number of YAML fixtures still use the old
maintarget name and/or theQQ.prefix, which breaks the new printed-form conventions. Please update these to ensure:
- All root targets are named
main'(with the prime suffix)- All
QQ.prefixes are removed from operation names (e.g.QQ.io.stdout→io.stdout)- Every token includes its
:<index>suffixFiles requiring fixes:
• Fixtures still using plain
maininstead ofmain'
- eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/just-float.yaml (line 7)
- eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/stars-tuples.yaml (line 7)
- eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/inheritance.yaml (line 12)
- eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/dataless.yaml (line 9)
- eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/emoji.yaml (line 7)
- eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/bytes.yaml (line 7)
- eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/backslash.yaml (line 7)
• Fixtures still containing
QQ.prefixes
- eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/times.yaml (lines 8–9)
- eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/multiline-string.yaml (lines 8–9)
- eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/dataless.yaml (line 10)
- eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/empty-string.yaml (line 8)
Please perform a full sweep of
eo-parser/src/test/resources/**/print-packs/**/*.yamlto align all fixtures with the new naming and formatting conventions.
♻️ Duplicate comments (2)
eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/empty-string.yaml (1)
13-13: Apostrophe suffix: covered by the consistency checkThe change to
[] > app'aligns with the new convention. Consistency across all print-pack fixtures is verified by the script provided in compares-bool-to-bytes.yaml.eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/multiline-string.yaml (1)
16-16: Apostrophe suffix: covered by the consistency checkThe change to
[] > app'matches the new printed naming convention. See the verification script in compares-bool-to-bytes.yaml to ensure repository-wide consistency.
🧹 Nitpick comments (22)
eo-parser/src/test/resources/org/eolang/parser/eo-syntax/full-syntax.yaml (1)
18-18: Make the assertion explicit about the sentinel child, not just the new count.Raising
count(o)to 4 matches the new AST shape, but the intent (idempotency/sentinel injection) would be clearer and less brittle if we also asserted the presence of the sentinel itself underatom, without relying on child order.Apply this diff to strengthen the check:
- - //o[o[@name='λ'] and @name='atom' and count(o)=4 and o[@name='a']] + - //o[@name='atom' + and o[@name='λ'] + and o[@name='a'] + and o[@base='ξ.xi🌵'] + and count(o)=4]eo-parser/src/test/resources/org/eolang/parser/eo-syntax/application-in-tests.yaml (1)
8-9: Avoid order-sensitive sentinel checks.
o[1][@base='ξ.xi🌵']bakes in child order; if printer/transform order shifts, this will fail despite correct structure. Prefer a position-agnostic predicate + keep the count.Apply this diff:
- - /object/o[@name='foo'][count(o)=4] - - /object/o[@name='foo' and o[1][@base='ξ.xi🌵']] + - /object/o[@name='foo'][count(o)=4] + - /object/o[@name='foo' and o[@base='ξ.xi🌵']]eo-parser/src/test/resources/org/eolang/parser/eo-syntax/anonymous-more.yaml (1)
7-8: Scope the sentinel assertion to the intended node to prevent false positives.A global
//o[@base='ξ.xi🌵']could match unrelated subtrees as more tests adopt the sentinel. Anchor it undermain(or another precise context) while retaining the updated child count.Suggested tweak:
- - //o[@base='ξ.xi🌵'] + - /object/o[@name='main']/o[@base='ξ.xi🌵']If
mainisn't the only target here, consider a position-agnostic but scoped form like:/object/o[@name='main'][o[@base='ξ.xi🌵'] and count(o)=4]Please re-run the suite to confirm no unintended breakage. If needed, I can search the repo for other broad
//o[@base='ξ.xi🌵']usages and propose tightened scopes.eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/emoji.yaml (1)
13-13: LGTM on target renaming; consider broader Unicode cases.The surrogate-pair escapes for 🌵 and 😀 look correct, and the main → main' rename matches the convention. Add one more fixture with ZWJ sequences or skin-tone modifiers to ensure the printer preserves grapheme clusters and doesn’t split or over-escape.
eo-parser/src/test/resources/org/eolang/parser/eo-packs/parse/stars-to-tuples.yaml (1)
9-11: Make the sentinel assertion less order-dependent to reduce test fragilityBumping count(o) to 9 is expected. However, asserting that the sentinel is specifically o[1] may be brittle if future transforms reorder children. Consider asserting presence rather than position.
Apply this change to the assertion:
- - /object/o[count(o)=9] - - /object/o[@name='main' and o[1][@base='ξ.xi🌵']] + - /object/o[count(o)=9] + - /object/o[@name='main' and o[@base='ξ.xi🌵']]If the first-child placement is a deliberate invariant you want to lock in, keep the current assertion; otherwise, the relaxed predicate above will make the test more robust across XSL tweaks.
eo-parser/src/test/resources/org/eolang/parser/eo-syntax/empty-vs-free.yaml (1)
8-8: Make the XPath assertion resilient to future child insertionsHard-requiring
count(o)=3is brittle and will break as soon as we add another implicit/sentinel child. Keep the “sentinel is first” guarantee, but relax the exact count to a lower bound.Apply this diff:
- - //o[@name='foo' and count(o)=3 and o[1][@base='ξ.xi🌵']] + - //o[@name='foo'][o[1][@base='ξ.xi🌵'] and count(o) >= 3]eo-maven-plugin/src/test/java/org/eolang/maven/MjTranspileTest.java (1)
79-79: Prefer disabling with an explicit reason and a tracking referenceBlindly disabling tests reduces protection against regressions. If these are temporarily broken by idempotency/sentinel propagation, annotate with a reason and an issue link, and plan re-enablement. Optionally, tag them for quarantine to allow selective execution.
Apply this diff to make the intent clear:
- @Disabled + @Disabled("Temporarily disabled due to formation idempotency sentinel ('ξ.xi🌵') changes; see #4411. TODO: re-enable after expectations are updated.") @Test void doesNotTouchAtom(@Mktmp final Path temp) throws IOException {- @Disabled + @Disabled("Temporarily disabled due to formation idempotency sentinel ('ξ.xi🌵') changes; see #4411. TODO: re-enable after expectations are updated.") @Test void createsPackageInfoFilesForAllPackages(@Mktmp final Path temp) throws IOException {- @Disabled + @Disabled("Temporarily disabled due to formation idempotency sentinel ('ξ.xi🌵') changes; see #4411. TODO: re-enable after expectations are updated.") @Test void savesValidContentToPackageInfoFile(@Mktmp final Path temp) throws Exception {If helpful, I can draft updated expectations or replacement assertions that align with the new transpiler output so these tests can be re-enabled promptly.
Also applies to: 104-104, 125-125
eo-parser/src/test/resources/org/eolang/parser/eo-syntax/auto-phi-formation.yaml (1)
8-9: Good: explicit sentinel placement under formation; add cardinality guard to enforce idempotency.The XPath now asserts
o[1][@base='ξ.xi🌵']for both formations, which matches the new listener/XSL pipeline. To make idempotency guarantees explicit, add an assertion that each such formation has exactly one sentinel, e.g.,count(o[@base='ξ.xi🌵'])=1per object under test.Apply this diff to strengthen the test:
asserts: - /object[not(errors)] - //o[not(@base) and @name='a🌵78' and o[1][@base='ξ.xi🌵'] and o[2][@name='i' and @base='∅'] and o[3][@name='φ']] - //o[not(@base) and @name='a🌵88' and o[1][@base='ξ.xi🌵'] and o[2][@name='i' and @base='∅'] and o[3][@name='φ']] + - //o[@name='a🌵78' and count(o[@base='ξ.xi🌵'])=1] + - //o[@name='a🌵88' and count(o[@base='ξ.xi🌵'])=1]Note: This complements the new formation-idempotency.yaml; keeping a local guard here reduces future brittleness if formation structure around these nodes evolves again.
eo-parser/src/test/resources/org/eolang/parser/eo-packs/parse/roll-bases.yaml (1)
31-31: Reduce XPath brittleness by avoiding positional child indices affected by sentinel reordering.Switching from
o[1]too[2]fixes the test for today but encodes ordering assumptions that may shift again with sentinel handling. Prefer matching by base and structure without positional indices.Apply this diff to make the assertion order-independent:
- - //o[@name='bts']/o[2][@base='.eq']/o[1][@base='.as-bytes']/o[1][@base='Φ.org.eolang.bytes']/o[1][@base='Φ.org.eolang.bytes']/o[text()!=''] + - //o[@name='bts']//o[@base='.eq']//o[@base='.as-bytes']//o[@base='Φ.org.eolang.bytes']//o[@base='Φ.org.eolang.bytes']/o[text()!='']This keeps the same structural intent while tolerating reordering introduced by
ξ.xi🌵handling.eo-parser/src/main/resources/org/eolang/parser/print/to-eo.xsl (1)
74-110: Don’t render sentinel in head; consider centralizing the sentinel FQN and defensively excluding at OBJECT-match.
- The head guard not(@base='ξ.xi🌵') is correct and aligns with the non-recursive child filtering.
- Optional hardening: also exclude sentinels at the OBJECT, NOT FREE ATTRIBUTE match to ensure they can’t be selected via any future template expansion path.
Apply locally-scoped constants and reuse them (improves maintainability, avoids typos):
@@ <xsl:variable name="auto" select="concat('a', $eo:cactoos)"/> + <xsl:variable name="idemp" select="'ξ.xi🌵'"/> @@ - <xsl:apply-templates select="o[not(eo:void(.)) and not(@base='ξ.xi🌵')]"> + <xsl:apply-templates select="o[not(eo:void(.)) and not(@base=$idemp)]"> @@ - <xsl:template match="o[@base and not(@base='ξ.xi🌵') and not(eo:has-data(.))]" mode="head"> + <xsl:template match="o[@base and not(@base=$idemp) and not(eo:has-data(.))]" mode="head">Outside the changed hunk (defensive, optional):
- <xsl:template match="o[not(eo:void(.)) and not(@name=$eo:lambda)]"> + <xsl:template match="o[not(eo:void(.)) and not(@name=$eo:lambda) and not(@base=$idemp)]">eo-parser/src/main/java/org/eolang/parser/XeEoListener.java (2)
253-255: Route Tformation through a shallow start to avoid auto-inserting the sentinel.Switching enterTformation to startTest(ctx) prevents an extra ξ.xi🌵 child in tests, which matches the design intent. Minor naming nit: startTest reads like a unit-test hook; consider a more intention-revealing name (e.g., startShallow or startTformation) plus a short Javadoc.
Apply this local rename for clarity:
- public void enterTformation(final EoParser.TformationContext ctx) { - this.startTest(ctx); + public void enterTformation(final EoParser.TformationContext ctx) { + this.startShallow(ctx); @@ - private Objects startTest(final ParserRuleContext ctx) { + /** + * Start an object without inserting the idempotency sentinel (used by Tformation). + */ + private Objects startShallow(final ParserRuleContext ctx) { return this.objects.start(ctx).leave(); }
1145-1149: Centralize the ξ.xi🌵 sentinel stringTo avoid scattering the hard-coded sentinel and to reduce the risk of accidental duplicate injections, extract the
"ξ.xi🌵"literal into a shared constant and update both injection sites:Locations to update:
XeEoListener.startAbstract(...)(around line 1145)XeEoListener.enterSuffix(...)(around line 1000)Test impact:
- The existing XMIR snapshot test (
XmirTest.java:62) uses the literal only once and won’t break, since the constant value remains unchanged.- A heuristic scan of
eo-parser/src/test/resourcesshows no current double-sentinel occurrences in test fixtures.Proposed diff:
--- a/eo-parser/src/main/java/org/eolang/parser/XeEoListener.java +++ b/eo-parser/src/main/java/org/eolang/parser/XeEoListener.java @@ class XeEoListener implements EoListener, Iterable<Directive> { /** Idempotency sentinel FQN for abstract and suffix injections. */ private static final String IDEMPOTENCY_SENTINEL = "ξ.xi🌵"; @@ - private Objects startAbstract(final ParserRuleContext ctx) { - return this.objects - .start(ctx) - .start(ctx) - .prop("base", "ξ.xi🌵") - .leave() - .leave(); - } + private Objects startAbstract(final ParserRuleContext ctx) { + return this.objects + .start(ctx) + .start(ctx) + .prop("base", IDEMPOTENCY_SENTINEL) + .leave() + .leave(); + } @@ - @Override - public void enterSuffix(final SuffixContext ctx) { - if (ctx.APOSTROPHE() != null) { - this.objects.start(ctx).prop("base", "ξ.xi🌵").leave(); - } - } + @Override + public void enterSuffix(final SuffixContext ctx) { + if (ctx.APOSTROPHE() != null) { + // Optional: guard against duplicate injection if needed + this.objects.start(ctx).prop("base", IDEMPOTENCY_SENTINEL).leave(); + } + }With this change, all places consistently reference a single source of truth, reducing maintenance overhead and the chance of mismatched literals.
eo-parser/src/main/resources/org/eolang/parser/parse/move-voids-up.xsl (1)
27-29: Exclude ξ.xi🌵 from the non-void pass; consider factoring the sentinel into a variable.Functionally correct; minor maintainability improvement: define a local variable for the sentinel FQN and reuse it in the two predicates.
Example:
@@ - <xsl:template match="o[o[not(eo:void(.))]/following-sibling::o[eo:void(.)]]" mode="with-diff-attrs"> + <xsl:template match="o[o[not(eo:void(.))]/following-sibling::o[eo:void(.)]]" mode="with-diff-attrs"> + <xsl:variable name="idemp" select="'ξ.xi🌵'"/> @@ - <xsl:for-each select="o[@base='ξ.xi🌵']"> + <xsl:for-each select="o[@base=$idemp]"> <xsl:copy-of select="."/> </xsl:for-each> @@ - <xsl:for-each select="o[not(eo:void(.)) and not(@base='ξ.xi🌵')]"> + <xsl:for-each select="o[not(eo:void(.)) and not(@base=$idemp)]"> <xsl:apply-templates select="."/> </xsl:for-each>eo-parser/src/test/resources/org/eolang/parser/eo-syntax/anonymous.yaml (1)
7-9: Tighten the sentinel assertion to verify ordering and reduce false positivesThe new check
//o[@base='ξ.xi🌵']only asserts existence anywhere in the document. Given the PR intent, we should ensure the sentinel is the first inner object of each formation; otherwise a valid-but-misplaced sentinel would pass.Consider strengthening with position- and scope-aware assertions alongside your current ones, e.g.:
asserts: - //object/o[count(o)=4] - - //o[@base='ξ.xi🌵'] + - //object/o[@name='first']/o[1][@base='ξ.xi🌵'] + - //object/o[@name='second']/o[1][@base='ξ.xi🌵'] + - //object/o[@name='bar']/o[1][@base='ξ.xi🌵']If you prefer to keep it generic, at least assert that the first child is the sentinel for every anonymous formation under
main:- - //o[@base='ξ.xi🌵'] + - //object/o[@name='first' or @name='second' or @name='bar']/o[1][@base='ξ.xi🌵']Additionally, to guard against duplication, you may assert uniqueness per formation:
+ - //object/o[@name='first'][count(o[@base='ξ.xi🌵'])=1] + - //object/o[@name='second'][count(o[@base='ξ.xi🌵'])=1] + - //object/o[@name='bar'][count(o[@base='ξ.xi🌵'])=1]eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/just-float.yaml (1)
15-19: Printed names now suffixed with': add a safety to avoid double-suffixing on re-printThe shift to
main'andfoo'in the printed block looks intentional. To prevent regressions, let's ensure the printer is idempotent (i.e., it won’t producemain''on subsequent print cycles).If your print pipeline supports round-tripping, consider adding (in a nearby print-pack test) an assertion that a second print pass doesn’t add another
'. Alternatively, add a guard in the printing XSL to only append'if the name doesn’t already end with it.eo-parser/src/test/resources/org/eolang/parser/eo-syntax/formation-idempotency.yaml (1)
7-15: Great targeted coverage; consider asserting exact shape and uniqueness per levelThese assertions are on point and exercise the sentinel-first invariant. To harden against future accidental additions, assert the exact number of inner
onodes and uniqueness of the sentinel at each level.Suggested additions:
asserts: - /object[not(errors)] - //o[not(@base) and @name='foo' and o[1][@base='ξ.xi🌵'] and o[2][@base='∅' and @name='f']] - //o[not(@base) and @name='bar' and o[1][@base='ξ.xi🌵'] and o[2][@base='∅' and @name='a']] - //o[not(@base) and @name='xyz' and o[1][@base='ξ.xi🌵'] and o[2][@base='∅' and @name='w']] + - //o[@name='foo' or @name='bar' or @name='xyz'][count(o)=2] + - //o[@name='foo' or @name='bar' or @name='xyz'][count(o[@base='ξ.xi🌵'])=1]eo-parser/src/test/resources/org/eolang/parser/eo-syntax/atoms-with-inner-tests.yaml (1)
8-13: Assert sentinel position and non-leakage into test objects
- Using
o[@base='ξ.xi🌵']underfoochecks existence but not placement. Tests and downstream transforms typically rely on the sentinel being the first inner object.- For
+test-1/2/3the currento[not(@base='ξ.xi🌵')]asserts the existence of a non-sentinel, not the absence of a sentinel.Consider:
- - /object/o[@name='foo']/o[@base='ξ.xi🌵'] + - /object/o[@name='foo']/o[1][@base='ξ.xi🌵'] - - /object/o[@name='foo']/o[@name='+test-1']/o[not(@base='ξ.xi🌵')] - - /object/o[@name='foo']/o[@name='+test-2']/o[not(@base='ξ.xi🌵')] - - /object/o[@name='foo']/o[@name='+test-3']/o[not(@base='ξ.xi🌵')] + - /object/o[@name='foo']/o[@name='+test-1'][not(o[@base='ξ.xi🌵'])] + - /object/o[@name='foo']/o[@name='+test-2'][not(o[@base='ξ.xi🌵'])] + - /object/o[@name='foo']/o[@name='+test-3'][not(o[@base='ξ.xi🌵'])]Optionally, if the count is sensitive to sentinel insertion, keep
count(o)=5but be aware it’s brittle as more sentinels or structuralos appear in the future.eo-maven-plugin/src/main/resources/org/eolang/maven/transpile/to-java.xsl (1)
510-527: Ensure PhDefault path still receives attributes when skipping inner-class generationSwitching to
new PhDefaultwhencount(o)=1 and eo:idempotent(.)avoids generating a nested class — good for bytecode size — but it changes how inner attributes are wired. With no nested class ctor, make sure the attributes that would have been added inside<nested>...<attr/>are still attached somewhere (e.g., via the caller path that invokesattrtemplates withcontextset to this variable).To make the intent explicit and safer, you can eagerly attach attributes for the PhDefault case right here:
@@ <xsl:template match="o[not(@base) and not(@name)]" mode="object"> <xsl:param name="indent"/> <xsl:param name="name"/> @@ - <xsl:text>();</xsl:text> + <xsl:text>();</xsl:text> + <!-- If we didn't generate a nested class, attach attributes directly --> + <xsl:if test="not(o) or count(o)=1 and eo:idempotent(.)"> + <xsl:apply-templates select="attr"> + <xsl:with-param name="indent" select="$indent + 1"/> + <xsl:with-param name="parent" select="'Inline'"/> + <xsl:with-param name="context" select="$name"/> + </xsl:apply-templates> + </xsl:if>If attributes are guaranteed absent in this branch (only sentinel + one ∅/formal), documenting that guarantee with an explanatory comment would also resolve the concern.
eo-parser/src/test/resources/org/eolang/parser/eo-syntax/numbers.yaml (1)
7-8: Tighten the XPath to target main explicitly and stabilize sentinel orderingAnchor the count to the main object and assert the sentinel as the first child to match the ordering guarantee from the transforms and avoid accidental matches.
- - /object/o[@name='main']/o[@base='ξ.xi🌵'] - - /object/o[count(o)=8] + - /object/o[@name='main']/o[1][@base='ξ.xi🌵'] + - /object/o[@name='main' and count(o)=8]Nitpick: for cross-platform consistency with Java tests, consider using the escaped form
ξ.xi\uD83C\uDF35instead of the literal emoji here.eo-parser/src/test/resources/org/eolang/parser/eo-syntax/scopes.yaml (1)
7-7: Anchor to top-level object; avoid broad // to reduce false positivesNarrow the XPath to the top-level object to prevent matching similarly shaped nested nodes in future test additions.
- - //o[not(@base) and count(o)=2 and @name='aliases' and o[1][@base='ξ.xi🌵']] + - /object/o[not(@base) and @name='aliases' and count(o)=2 and o[1][@base='ξ.xi🌵']]eo-parser/src/test/java/org/eolang/parser/EoSyntaxTest.java (2)
192-195: Reduce brittleness: avoid relying on child index for 'f' positionIndexing
o[3]ties the test to ordering details that could change with unrelated transformations. Prefer asserting structure and sentinel presence without the hard position.- "/object/o[@name='base' and o[1][@base='ξ.xi\uD83C\uDF35']]", - "/object/o[@name='base' and o[3][@name='f' and o[1][@base='ξ.xi\uD83C\uDF35']]]" + "/object/o[@name='base' and o[1][@base='ξ.xi\uD83C\uDF35']]", + "/object/o[@name='base' and o[@name='f' and o[1][@base='ξ.xi\uD83C\uDF35']]]"Optionally, add uniqueness checks to assert idempotency (exactly one sentinel under each formation):
+ "/object/o[@name='base' and count(o[@base='ξ.xi\uD83C\uDF35'])=1]", + "/object/o[@name='base' and o[@name='f' and count(o[@base='ξ.xi\uD83C\uDF35'])=1]]"
193-195: Deduplicate sentinel literal with a constant for maintainabilityThe sentinel base string appears multiple times and across tests; extracting it reduces typo risk.
Add near the top of the class (outside this hunk):
private static final String XI_CACTUS = "ξ.xi\uD83C\uDF35";Then replace occurrences in this test:
- "/object/o[@name='base' and o[1][@base='ξ.xi\uD83C\uDF35']]", + String.format("/object/o[@name='base' and o[1][@base='%s']]", XI_CACTUS), - "/object/o[@name='base' and o[@name='f' and o[1][@base='ξ.xi\uD83C\uDF35']]]" + String.format("/object/o[@name='base' and o[@name='f' and o[1][@base='%s']]]", XI_CACTUS)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (34)
eo-maven-plugin/src/main/resources/org/eolang/maven/transpile/to-java.xsl(2 hunks)eo-maven-plugin/src/test/java/org/eolang/maven/MjTranspileTest.java(3 hunks)eo-parser/src/main/java/org/eolang/parser/XeEoListener.java(2 hunks)eo-parser/src/main/resources/org/eolang/parser/parse/move-voids-up.xsl(1 hunks)eo-parser/src/main/resources/org/eolang/parser/print/to-eo.xsl(1 hunks)eo-parser/src/test/java/org/eolang/parser/EoSyntaxTest.java(1 hunks)eo-parser/src/test/resources/org/eolang/parser/eo-packs/parse/roll-bases.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/eo-packs/parse/stars-to-tuples.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/eo-syntax/anonymous-more.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/eo-syntax/anonymous.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/eo-syntax/application-in-tests.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/eo-syntax/atoms-with-inner-tests.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/eo-syntax/auto-phi-formation.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/eo-syntax/empty-vs-free.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/eo-syntax/formation-idempotency.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/eo-syntax/full-syntax.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/eo-syntax/numbers.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/eo-syntax/scopes.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/auto-named-abstract.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/backslash.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/bytes.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/compares-bool-to-bytes.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/const-auto-named.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/dataized.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/dataless.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/emoji.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/empty-string.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/idiomatic.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/inheritance.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/just-float.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/methods-by-index.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/multiline-string.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/stars-tuples.yaml(1 hunks)eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/times.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-12T15:24:05.011Z
Learnt from: h1alexbel
PR: objectionary/eo#4431
File: eo-parser/src/main/resources/XMIR.xsd:80-81
Timestamp: 2025-08-12T15:24:05.011Z
Learning: In the EO parser architecture, Φ̇ is an alias for Φ.org.eolang that gets expanded during XMIR decoration by expand-qqs.xsl. The XSD schema for FQN patterns should only validate canonical forms (Φ and ξ as roots) since alias expansion happens before XSD validation.
Applied to files:
eo-parser/src/main/resources/org/eolang/parser/print/to-eo.xsleo-parser/src/test/resources/org/eolang/parser/eo-syntax/auto-phi-formation.yaml
🧬 Code graph analysis (1)
eo-maven-plugin/src/test/java/org/eolang/maven/MjTranspileTest.java (1)
eo-maven-plugin/src/test/java/org/eolang/maven/MjLintIT.java (1)
Disabled(37-101)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: mvn (windows-2022, 23)
- GitHub Check: mvn (macos-15, 23)
- GitHub Check: qulice
- GitHub Check: mvn (ubuntu-24.04, 23)
- GitHub Check: integration
- GitHub Check: ort
🔇 Additional comments (12)
eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/const-auto-named.yaml (1)
13-16: Action Required: Verify and Enforce Idempotent Apostrophe-SuffixingIt wasn’t possible to conclusively detect double-suffixing via the scanned fixtures—please manually verify that no name or operator in
eo-parser/src/test/resources/org/eolang/parser/print-packs/yamlhas received two apostrophes, and add an automated idempotency check to prevent regressions.• Locations to inspect:
- All YAML fixtures under
eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml(e.g.const-auto-named.yaml)- Any newly printed targets or operators using
'or>>!in other print-packs.
• Recommended addition:- A round-trip unit test (e.g. in
eo-parser/src/test/java/...) asserting
print(parse(print(parse(input)))) == print(parse(input))
to guard against accumulating apostrophes on repeated print cycles.
• CI Enforcement:- Integrate a quick script or assertion in your build to scan for
''in printed output, failing the build if any double-apostrophes are found.eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/bytes.yaml (1)
16-16: main → main' rename matches the new printed naming conventionOnly the top-level binding changes; inner bindings remain untouched. Looks good.
eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/times.yaml (1)
15-15: Change approved: sentinel absent in printed YAMLRan the sanity check to ensure that no YAML fixtures include the sentinel ξ.xi🌵:
rg -n '\xCE\xBE\.xi' eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml # no matches foundThis confirms that printed YAML blocks do not embed the sentinel. Approving the code changes.
eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/compares-bool-to-bytes.yaml (1)
13-13: Printed-root apostrophes: all fixtures compliantAll YAML fixtures under print-packs have been scanned for
[] > …lines withinprinted: |blocks, and every printed root ends with the required apostrophe. No missing apostrophes were detected.eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/methods-by-index.yaml (1)
16-16: Apostrophe-suffixed printed target looks consistent; consider collision policy test.Renaming
ytoy'in the printed block aligns with the new printing semantics seen across the suite. To avoid future regressions, please add (in this or a nearby fixture) a case where an input already containsy'(and eveny'') to verify deterministic de-duplication and collision handling for targets that are auto-prime’d.eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/backslash.yaml (1)
12-12: Printed target apostrophe is fine; please double-check escape stability for the regex literal.The
main → main'rename is consistent. Given historical fragility around backslash escapes, consider adding (in a syntax/XMIR-level test) an assertion that the printed XMIR node for the regex holds the exact byte sequence as before (no loss of\xNNescapes), independent of target renaming.eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/idiomatic.yaml (1)
37-41: Confirm grammar acceptance of apostrophe after a “questioned” target; consider a dedicated edge-case fixture.
idiomatic → idiomatic'andrr ? → rr ?'match the new rule. Please confirm EO grammar and printer round-trip accept an apostrophe appended to a target whose name ends with?(and includes a space). A tiny edge-case fixture with references to such targets would help ensure symbol resolution is unaffected by the trailing prime.eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/dataized.yaml (1)
15-18: Printed targets now correctly reflect idempotency (trailing apostrophe).Looks consistent with eo:idempotent(.) printing in to-eo.xsl and the PR goal. No further action from my side on this fixture.
eo-parser/src/main/resources/org/eolang/parser/print/to-eo.xsl (1)
69-71: Exclude ξ.xi🌵 children from recursion to suppress sentinel rendering.Good call to filter the synthetic sentinel nodes at the child-objects level; this prevents ghost/blank lines for internals while keeping normal objects intact.
eo-parser/src/test/resources/org/eolang/parser/print-packs/yaml/stars-tuples.yaml (1)
22-36: Fixture aligns with new printing: main is idempotent, array/xs layout expanded.The updated snapshot (main' + expanded tuple formatting and quoted string) matches the new print rules. Looks good.
eo-parser/src/main/resources/org/eolang/parser/parse/move-voids-up.xsl (1)
21-23: Emit ξ.xi🌵 sentinels first when reordering attributes.Pre-copying sentinel nodes keeps their special semantics intact and ensures stable order ahead of voids. This complements printing/exclusion logic.
eo-maven-plugin/src/main/resources/org/eolang/maven/transpile/to-java.xsl (1)
356-363: Casting to PhDefault for non-thiscontexts: confirm type safetyThe explicit cast
((PhDefault) <context>).add(...)assumes any non-thiscontext is aPhDefault. That matches theabstract/atomtemplates which initializectxasPhDefault, but if any future path passes aPhOnceor anotherPhihere, you get aClassCastException.Add a brief comment near this cast stating the invariant (e.g., “non-
thiscontext is alwaysPhDefault”), and consider defensive casting only when needed:- <xsl:text>((PhDefault) </xsl:text> + <xsl:text>((</xsl:text> + <xsl:text>PhDefault</xsl:text> + <xsl:text>) </xsl:text>Plus a unit/integration test transpiling a case where attributes are added inside a nested
abstractand verifying no cast crashes at runtime.
|
@maxonfjvipon please check |
|
@maxonfjvipon reminder |
|
@yegor256 take a look, please |
|
@maxonfjvipon Great job on the review! 🌟 You've earned +4 points: +12 base, -10 for limited comments (4), +2 bonus. Your Objectionary balance is now +129. Remember to check your Zerocracy balance too. Keep up the good work! |
|
@yegor256 Hey there! 👋 Nice job on the review, you've snagged +4 points for your effort! Here's the breakdown: +12 for doing the review, -10 because there were only 4 comments (we're aiming for at least 6), and +2 to keep you in the game. Remember, more comments can boost your points up to 8 max! Your current balance sits at -624. Keep an eye on your Zerocracy balance too - every hit-of-code counts! 💻 Let's aim for more detailed reviews next time to max out those bonus points! |
|
@h1alexbel Hey there, coding superstar! 🌟 You've just scored a whopping +24 points for your awesome contribution! That's the max reward possible, and here's why: You got the base 16 points, plus a sweet 8 points for your 177 hits-of-code (staying under that 200 threshold - smart move!). We had to trim 1 point to keep it under the cap, but that's still incredible! Keep the code flowing, and don't forget to check your Zerocracy balance - your running total is now at an impressive +162! Let's see how high we can push that number, shall we? |
4 participants
In this PR I've introduced idempotency attributes for each formation.
see #4411
Summary by CodeRabbit
Bug Fixes
Refactor
Tests