bug(#3480): Attr replacement with Phi#4223
Conversation
🚀 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
|
|
@maxonfjvipon please check |
|
@maxonfjvipon reminder |
WalkthroughThis change eliminates the legacy Changes
Sequence Diagram(s)sequenceDiagram
participant EO_Object as EO Object (extends PhDefault)
participant Attribute as PhVoid/PhComposite/PhCached/PhRho
participant Client as Client Code
Client->>EO_Object: new EO_Object()
EO_Object->>Attribute: add("attr", new PhVoid("attr"))
Client->>EO_Object: take("attr")
EO_Object->>Attribute: take("attr")
Attribute-->>Client: returns value or throws if unset
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (12)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🔭 Outside diff range comments (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
84-95:⚠️ Potential issueSame race condition in positional put method.
This method has the identical TOCTOU race condition as the named put method.
Apply the same fix using
compareAndSet:- public void put(final int pos, final Phi phi) { - if (this.object.get() == null) { - this.object.set(phi); - } else { - throw new ExReadOnly( - String.format( - "This void attribute \"%s\" is already set, can't reset", - this.name - ) - ); - } - } + public void put(final int pos, final Phi phi) { + if (!this.object.compareAndSet(null, phi)) { + throw new ExReadOnly( + String.format( + "This void attribute \"%s\" is already set, can't reset", + this.name + ) + ); + } + }
♻️ Duplicate comments (4)
eo-runtime/src/main/java/org/eolang/PhaLogged.java (1)
16-16: Consider merging with PhLogged class.This appears to be similar functionality to
PhLogged. Consider if these classes can be consolidated to avoid duplication.eo-runtime/src/main/java/org/eolang/PhComposite.java (3)
44-46: Verified: Consistent implementation aligns with past review discussion.Both
take(int pos)andtake(String name)returnthis.expr.apply(this.argument), which is consistent with the previous discussion where it was clarified that the result of the expression needs to be taken out, not its nested object.Also applies to: 59-61
44-46: Implementation appears correct based on maintainer feedback.The
take(int pos)method returns the result of the expression directly. Based on past review comments, this design was intentionally chosen to return the evaluated expression result rather than taking a nested object from it.
59-61: Implementation appears correct based on maintainer feedback.Similar to
take(int pos), this method returns the expression result directly, which aligns with the maintainer's explanation that "we need the result ofexprto be taken out, not its nested object."
🧹 Nitpick comments (8)
eo-integration-tests/src/test/java/integration/ReadmeSnippetsIT.java (1)
23-34: Appropriate temporary measure for plugin compatibility.Disabling this test class is the correct approach while the
eo-maven-pluginis updated to version 0.56.3 to support the newPhiinterface. This prevents CI failures due to the removedorg.eolang.Attrclass.Consider adding a TODO comment or tracking issue to ensure these tests are re-enabled once the plugin is updated.
eo-runtime/src/test/java/org/eolang/PhCompositeTest.java (1)
14-29: LGTM: Class rename and TODO comment noted.The class renaming from
AtCompositeTesttoPhCompositeTestis correct. The TODO comment at line 24-26 indicates awareness of technical debt regarding theTO_ADD_MESSAGEfield usage across multiple test files.eo-runtime/src/test/java/org/eolang/PhMethodTest.java (2)
21-24: PreferassertNotSame(or Hamcrest’snot(sameInstance())) for identity checksThe intent here is to confirm that two invocations of
num.take("plus")yield different objects (identity inequality), not merely objects that are not equal viaequals().Using
Matchers.not(Matchers.equalTo(...))will also pass when the two instances are identical but theirequals()implementation returnsfalse. A safer expectation is to assert they are not the same instance.-Matchers.not(Matchers.equalTo(num.take("plus"))) +Matchers.not(Matchers.sameInstance(num.take("plus")))(or use
assertNotSamefrom JUnit).
This keeps the test aligned with the semantic goal.
100-113: Race-condition possibility oncountin concurrent tests
countis incremented from a lambda executed insidePhComposite. If the test suite is ever executed in parallel (JUnit 5 allows this), multiple threads could callnew Dataized(phi).take()simultaneously, leading to a data race oncount.Consider either:
- Marking
countasAtomicInteger, or- Restricting access through synchronization.
This is not an immediate bug for the current single-threaded test, but hardening the test class will avoid flaky behaviour under parallel execution settings.
eo-runtime/src/main/java/org/eolang/PhWithRho.java (1)
100-106: Package-privateorigin()tight-couples callers to implementation
origin()is exposed solely forPhDefault.put(int, Phi)and comes with a TODO. Keeping it encourages down-casting ((PhWithRho)) and violates encapsulation.Consider:
- Moving
origin()to a dedicated interface used byPhDefault, or- Replacing the cast with a polymorphic query (
isVoid()?) onPhi.Tidying this up will make it easier to drop the PMD suppression and improves maintainability.
eo-runtime/src/main/java/org/eolang/PhDefault.java (3)
123-134: Unsafe cast toPhWithRhomay break when foreignPhiis injected
put(int, Phi)assumes every stored attribute is aPhWithRho, butdefaults()inserts a plainPhRhoand external code could mutateattrs(reflection, tests).if (!(((PhWithRho) this.attrs.get(name)).origin() instanceof PhVoid)) {Consider replacing the cast with a safe check:
final Phi stored = this.attrs.get(name); if (!(stored instanceof PhWithRho w && w.origin() instanceof PhVoid)) { ... }This avoids
ClassCastExceptionand clarifies intent.
251-256:add()bypasses null-checks; defensive copy recommended
add(String, Phi)inserts the passedattrdirectly. Passingnullsilently causes NPEs later.
Add a precondition:Objects.requireNonNull(attr, "Attribute must not be null");to fail fast and aid debugging.
313-317:defaults()returns mutable map exposed to instance stateAlthough a new
HashMapis created per object, returning a concrete mutable implementation invites accidental sharing if reused elsewhere.
ReturnCollections.unmodifiableMap(...)or wrap the entry on insertion to avoid external mutation.private static Map<String, Phi> defaults() { return Map.of(Phi.RHO, new PhRho()); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (101)
eo-integration-tests/src/test/java/integration/ReadmeSnippetsIT.java(2 hunks)eo-integration-tests/src/test/java/integration/SnippetIT.java(1 hunks)eo-maven-plugin/src/main/resources/org/eolang/maven/transpile/to-java.xsl(4 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOand.java(2 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOconcat.java(2 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOeq.java(3 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOnot.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOor.java(2 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOright.java(2 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOsize.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOslice.java(3 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOxor.java(2 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOerror.java(2 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOdir$EOmade$EOmkdir.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOdir$EOtmpfile$EOtouch.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOdir$EOwalk.java(2 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOfile$EOdeleted$EOdelete.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOfile$EOexists.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOfile$EOis_directory.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOfile$EOmoved$EOmove.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOfile$EOopen$EOfile_stream$EOread$EOread_bytes.java(2 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOfile$EOopen$EOfile_stream$EOwrite$EOwritten_bytes.java(2 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOfile$EOopen$EOprocess_file.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOfile$EOsize.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOfile$EOtouched$EOtouch.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOi16$EOas_i32.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOi32$EOas_i64.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOi64$EOas_number.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOi64$EOdiv.java(3 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOi64$EOgt.java(2 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOi64$EOplus.java(3 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOi64$EOtimes.java(3 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOmalloc$EOof$EOallocated$EOread.java(2 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOmalloc$EOof$EOallocated$EOresized.java(2 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOmalloc$EOof$EOallocated$EOsize.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOmalloc$EOof$EOallocated$EOwrite.java(2 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOmalloc$EOof$EOφ.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOmath/EOangle$EOcos.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOmath/EOangle$EOsin.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOmath/EOreal$EOacos.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOmath/EOreal$EOasin.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOmath/EOreal$EOln.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOmath/EOreal$EOpow.java(2 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOmath/EOreal$EOsqrt.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOas_i64.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOdiv.java(2 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOfloor.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOgt.java(2 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOplus.java(2 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOtimes.java(2 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOsys/EOposix$EOφ.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOsys/EOwin32$EOφ.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOtry.java(2 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOtxt/EOregex$EOpattern$EOmatch$EOmatched_from_index.java(2 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOtxt/EOregex$EOφ.java(1 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOtxt/EOsprintf.java(2 hunks)eo-runtime/src/main/java/EOorg/EOeolang/EOtxt/EOsscanf.java(2 hunks)eo-runtime/src/main/java/org/eolang/AtComposite.java(0 hunks)eo-runtime/src/main/java/org/eolang/AtLogged.java(0 hunks)eo-runtime/src/main/java/org/eolang/AtOnce.java(0 hunks)eo-runtime/src/main/java/org/eolang/AtRho.java(0 hunks)eo-runtime/src/main/java/org/eolang/AtWithRho.java(0 hunks)eo-runtime/src/main/java/org/eolang/Attr.java(0 hunks)eo-runtime/src/main/java/org/eolang/Data.java(1 hunks)eo-runtime/src/main/java/org/eolang/PhCached.java(1 hunks)eo-runtime/src/main/java/org/eolang/PhComposite.java(1 hunks)eo-runtime/src/main/java/org/eolang/PhDefault.java(11 hunks)eo-runtime/src/main/java/org/eolang/PhLogged.java(1 hunks)eo-runtime/src/main/java/org/eolang/PhOnce.java(1 hunks)eo-runtime/src/main/java/org/eolang/PhPackage.java(3 hunks)eo-runtime/src/main/java/org/eolang/PhRho.java(1 hunks)eo-runtime/src/main/java/org/eolang/PhSafe.java(1 hunks)eo-runtime/src/main/java/org/eolang/PhVoid.java(4 hunks)eo-runtime/src/main/java/org/eolang/PhWithRho.java(1 hunks)eo-runtime/src/main/java/org/eolang/PhaLogged.java(1 hunks)eo-runtime/src/main/java/org/eolang/Phi.java(2 hunks)eo-runtime/src/test/java/EOorg/EOeolang/EObytesEOconcatTest.java(2 hunks)eo-runtime/src/test/java/EOorg/EOeolang/EOerrorTest.java(3 hunks)eo-runtime/src/test/java/EOorg/EOeolang/EOmallocTest.java(5 hunks)eo-runtime/src/test/java/EOorg/EOeolang/EOnet/EOsocketTest.java(5 hunks)eo-runtime/src/test/java/EOorg/EOeolang/EOnumberTest.java(6 hunks)eo-runtime/src/test/java/EOorg/EOeolang/EOtryTest.java(7 hunks)eo-runtime/src/test/java/EOorg/EOeolang/HeapsTest.java(15 hunks)eo-runtime/src/test/java/org/eolang/AtEnvelope.java(0 hunks)eo-runtime/src/test/java/org/eolang/BytesOfTest.java(12 hunks)eo-runtime/src/test/java/org/eolang/DataTest.java(2 hunks)eo-runtime/src/test/java/org/eolang/ExInterruptedTest.java(1 hunks)eo-runtime/src/test/java/org/eolang/ExpectTest.java(6 hunks)eo-runtime/src/test/java/org/eolang/JavaPathTest.java(1 hunks)eo-runtime/src/test/java/org/eolang/MainTest.java(3 hunks)eo-runtime/src/test/java/org/eolang/PhCompositeTest.java(3 hunks)eo-runtime/src/test/java/org/eolang/PhCopyTest.java(2 hunks)eo-runtime/src/test/java/org/eolang/PhDefaultTest.java(31 hunks)eo-runtime/src/test/java/org/eolang/PhLoggedTest.java(4 hunks)eo-runtime/src/test/java/org/eolang/PhMethodTest.java(7 hunks)eo-runtime/src/test/java/org/eolang/PhPackageTest.java(7 hunks)eo-runtime/src/test/java/org/eolang/PhWithTest.java(6 hunks)eo-runtime/src/test/java/org/eolang/PhaEnvelope.java(1 hunks)eo-runtime/src/test/java/org/eolang/PhaLoggedTest.java(3 hunks)eo-runtime/src/test/java/org/eolang/PhaSimple.java(1 hunks)eo-runtime/src/test/java/org/eolang/PhiTest.java(5 hunks)
💤 Files with no reviewable changes (7)
- eo-runtime/src/test/java/org/eolang/AtEnvelope.java
- eo-runtime/src/main/java/org/eolang/AtWithRho.java
- eo-runtime/src/main/java/org/eolang/AtRho.java
- eo-runtime/src/main/java/org/eolang/Attr.java
- eo-runtime/src/main/java/org/eolang/AtLogged.java
- eo-runtime/src/main/java/org/eolang/AtComposite.java
- eo-runtime/src/main/java/org/eolang/AtOnce.java
🧰 Additional context used
🧬 Code Graph Analysis (30)
eo-runtime/src/main/java/EOorg/EOeolang/EOerror.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/main/java/EOorg/EOeolang/EOmath/EOreal$EOpow.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/main/java/EOorg/EOeolang/EOtry.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-integration-tests/src/test/java/integration/ReadmeSnippetsIT.java (1)
eo-integration-tests/src/test/java/integration/SnippetIT.java (1)
Disabled(37-97)
eo-runtime/src/main/java/EOorg/EOeolang/EOtxt/EOsprintf.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/main/java/EOorg/EOeolang/EOtxt/EOsscanf.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOtimes.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOplus.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/main/java/EOorg/EOeolang/EOtxt/EOregex$EOpattern$EOmatch$EOmatched_from_index.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOeq.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOslice.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/test/java/EOorg/EOeolang/EOnet/EOsocketTest.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/main/java/EOorg/EOeolang/EOmalloc$EOof$EOallocated$EOwrite.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-integration-tests/src/test/java/integration/SnippetIT.java (1)
eo-integration-tests/src/test/java/integration/ReadmeSnippetsIT.java (1)
Disabled(34-98)
eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOfile$EOopen$EOfile_stream$EOread$EOread_bytes.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOxor.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/main/java/EOorg/EOeolang/EOi64$EOdiv.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/main/java/EOorg/EOeolang/EOi64$EOtimes.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOfile$EOopen$EOfile_stream$EOwrite$EOwritten_bytes.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOright.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOdiv.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/main/java/EOorg/EOeolang/EOi64$EOgt.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/main/java/EOorg/EOeolang/EOmalloc$EOof$EOallocated$EOread.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/main/java/EOorg/EOeolang/EOmalloc$EOof$EOallocated$EOresized.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOor.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/test/java/EOorg/EOeolang/EOtryTest.java (2)
eo-runtime/src/main/java/org/eolang/PhComposite.java (1)
PhComposite(17-91)eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/main/java/EOorg/EOeolang/EOi64$EOplus.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOconcat.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOdir$EOwalk.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
eo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOgt.java (1)
eo-runtime/src/main/java/org/eolang/PhVoid.java (1)
PhVoid(18-137)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: ebnf
- GitHub Check: integration
- GitHub Check: mvn (windows-2022, 23)
- GitHub Check: mvn (ubuntu-24.04, 11)
- GitHub Check: mvn (ubuntu-24.04, 23)
🔇 Additional comments (182)
eo-runtime/src/test/java/org/eolang/JavaPathTest.java (1)
30-30: Update test to use PhComposite constant
Changed the assertion fromAtCompositeTest.TO_ADD_MESSAGEtoPhCompositeTest.TO_ADD_MESSAGE, reflecting the interface consolidation and class renaming.eo-runtime/src/main/java/EOorg/EOeolang/EOtxt/EOregex$EOφ.java (1)
35-35: Switch attribute access to Phi.RHO
Replacedthis.take(Attr.RHO)withthis.take(Phi.RHO)to align with the removal of theAttrinterface and use the unifiedPhiconstants.eo-runtime/src/main/java/EOorg/EOeolang/EOsys/EOwin32$EOφ.java (1)
74-74: Use Phi.RHO instead of Attr.RHO
Updated thelambda()method to callthis.take(Phi.RHO), consistent with the newPhi-based attribute model.eo-runtime/src/main/java/EOorg/EOeolang/EOsys/EOposix$EOφ.java (1)
72-72: Adopt Phi.RHO for attribute retrieval
Switched fromthis.take(Attr.RHO)tothis.take(Phi.RHO)inlambda(), completing the migration to thePhiinterface.eo-runtime/src/main/java/EOorg/EOeolang/EOmalloc$EOof$EOφ.java (1)
29-29: Replace Attr.RHO with Phi.RHO
Adjusted thelambda()implementation to usethis.take(Phi.RHO)in place of the deprecatedAttr.RHO.eo-runtime/src/main/java/EOorg/EOeolang/EOerror.java (2)
20-20: ImportPhVoidin place of legacyAtVoid.The new import aligns with the migration from the old
Attr/AtVoidAPI to thePhi/PhVoidmodel.
43-43: UsePhVoidfor the"message"attribute.Replacing the old
AtVoidwithPhVoidensures theEOerrorobject initializes its"message"field via the newPhi-based void attribute.eo-runtime/src/main/java/EOorg/EOeolang/EOi64$EOas_number.java (1)
30-30: Switch toPhi.RHOinstead of deprecatedAttr.RHO.This update correctly references the new attribute constant on the
Phiinterface forrho.eo-runtime/src/main/java/EOorg/EOeolang/EOtry.java (2)
16-16: Add import forPhVoid.Ensures the new void attribute class is available for initializing
"main","catch", and"finally".
33-35: Initialize"main","catch", and"finally"withPhVoid.Replacing
AtVoidusage withPhVoidis consistent with the refactoredPhi-based attribute model.eo-runtime/src/main/java/EOorg/EOeolang/EOi32$EOas_i64.java (1)
36-36: Update attribute access to usePhi.RHO.This replaces the old
Attr.RHOconstant with the newPhi.RHOconstant, aligning with the broader refactoring.eo-runtime/src/test/java/EOorg/EOeolang/EObytesEOconcatTest.java (2)
14-14: Replace import ofAtCompositeTestwithPhCompositeTest.Tests should reference the new
PhCompositeTestclass following the migration of composite attribute implementations.
32-32: UsePhCompositeTest.TO_ADD_MESSAGEinstead ofAtCompositeTest.TO_ADD_MESSAGE.Aligns the test’s failure message constant with the updated
PhCompositeTest.eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOfile$EOexists.java (2)
36-36: LGTM! Consistent refactoring change.The update from
Attr.RHOtoPhi.RHOis correct and aligns with the PR objective to replace theAttrinterface withPhi. The functionality remains unchanged.
36-36: LGTM! Clean refactoring from Attr to Phi interface.The constant replacement from
Attr.RHOtoPhi.RHOis correct and consistent with the broader refactoring effort to migrate from theAttrinterface to thePhiinterface.eo-runtime/src/test/java/org/eolang/PhCopyTest.java (2)
21-21: LGTM! Necessary test constant update.The change from
AtCompositeTest.TO_ADD_MESSAGEtoPhCompositeTest.TO_ADD_MESSAGEis correct and necessary as part of the refactoring fromAt*toPh*classes.
33-33: LGTM! Consistent with the test message update.Same appropriate change as line 21, updating the test message constant reference to the new
PhCompositeTestclass.eo-runtime/src/main/java/org/eolang/PhLogged.java (1)
87-90: LGTM! Proper interface method implementation.The new
copy(Phi self)method correctly implements the newPhiinterface requirement by delegating to the existingcopy()method. This is the appropriate implementation for a logging decorator class.eo-runtime/src/main/java/EOorg/EOeolang/EOmalloc$EOof$EOallocated$EOsize.java (2)
31-31: LGTM! Consistent refactoring change.The update from
Attr.RHOtoPhi.RHOis correct and maintains the same functionality while aligning with the PR objective to replace theAttrinterface withPhi.
31-31: LGTM! Consistent with the Attr-to-Phi migration.The replacement of
Attr.RHOwithPhi.RHOis correct and maintains the existing behavior while adopting the newPhiinterface.eo-runtime/src/main/java/EOorg/EOeolang/EOi16$EOas_i32.java (1)
34-34: LGTM! Correct migration from Attr.RHO to Phi.RHO.This change correctly updates the constant reference as part of the migration from the legacy
Attrinterface to the unifiedPhiinterface. The functionality remains unchanged.eo-runtime/src/test/java/org/eolang/PhLoggedTest.java (1)
22-22: LGTM! Correct test reference migration.The test assertion failure messages have been correctly updated to reference
PhCompositeTest.TO_ADD_MESSAGEinstead ofAtCompositeTest.TO_ADD_MESSAGE, aligning with the migration from legacyAt*classes to newPh*classes.Also applies to: 31-31, 40-40, 50-50
eo-runtime/src/main/java/EOorg/EOeolang/EOmath/EOreal$EOacos.java (1)
30-30: LGTM! Consistent migration from Attr.RHO to Phi.RHO.This change correctly updates the constant reference as part of the systematic migration from the legacy
Attrinterface to the unifiedPhiinterface. The core mathematical logic remains unchanged.eo-runtime/src/test/java/EOorg/EOeolang/EOnet/EOsocketTest.java (2)
34-34: LGTM! Correct import migration from AtVoid to PhVoid.The import has been correctly updated to use the new
PhVoidclass as part of the migration from legacyAt*classes to newPh*classes implementing thePhiinterface.
859-859: LGTM! Correct AtVoid to PhVoid migration in inner classes.The migration from
new AtVoid("s")tonew PhVoid("s")in all four inner classes (Simple, Server, Receiver, Client) is correct. Based on thePhVoidclass implementation, the constructor signature and functionality are compatible with the previousAtVoidusage.Also applies to: 888-888, 917-917, 946-946
eo-runtime/src/test/java/org/eolang/ExInterruptedTest.java (1)
22-23: LGTM! Correct refactoring to new Phi interface.The changes correctly replace the deprecated
Attr.PHIconstant withPhi.PHIand update the test message constant reference fromAtCompositeTesttoPhCompositeTest. The test logic remains unchanged while aligning with the new interface consolidation.eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOdir$EOmade$EOmkdir.java (1)
34-34: LGTM! Correct migration to Phi interface.The change correctly replaces
Attr.RHOwithPhi.RHOin the attribute access chain, maintaining the same functionality while aligning with the interface consolidation effort.eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOfile$EOopen$EOprocess_file.java (1)
33-35: LGTM! Consistent migration to Phi interface.Both instances of
Attr.RHOhave been correctly replaced withPhi.RHO, maintaining the same attribute access patterns while conforming to the new interface consolidation. The file processing logic remains unchanged.eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOnot.java (1)
31-31:Details
✅ Verification successful
LGTM! Correct refactoring to Phi interface.
The change correctly replaces
Attr.RHOwithPhi.RHO, maintaining the same functionality while aligning with the interface consolidation effort.To ensure the refactoring is complete, let's verify that no remaining
Attrreferences exist in the codebase:
🏁 Script executed:
#!/bin/bash # Description: Verify that all Attr references have been successfully migrated to Phi # Expected: No remaining references to Attr.RHO, Attr.PHI, Attr.LAMBDA, or AtVoid/AtComposite classes echo "Checking for remaining Attr constant references..." rg "Attr\.(RHO|PHI|LAMBDA)" --type java echo "Checking for remaining deprecated attribute classes..." rg "(AtVoid|AtComposite|AtOnce|AtRho|AtWithRho|AtLogged)" --type java echo "Checking for import statements of deprecated classes..." rg "import.*\.(Attr|AtVoid|AtComposite)" --type javaLength of output: 519
🏁 Script executed:
#!/bin/bash # Broader search for any remaining Attr references echo "Scanning for any 'Attr.' usage in Java sources..." rg "Attr\." --type java || echo "No direct Attr.* references found." echo "Scanning for any import statements mentioning Attr" rg "import\s+.*Attr" --type java || echo "No Attr imports detected."Length of output: 426
Refactoring Complete: No Remaining Attr References
The replacement of
Attr.RHOwithPhi.RHOineo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOnot.javacorrectly preserves functionality and aligns with the consolidatedPhiinterface. A repository-wide scan confirms:
- No occurrences of
Attr.*in Java sources- No
importstatements referencingAttror deprecated attribute classesAll
Attrreferences have been successfully migrated.eo-runtime/src/test/java/org/eolang/DataTest.java (1)
20-20:Details
✅ Verification successful
Verify the new message constant exists and maintains the same meaning.
The consistent replacement of
AtCompositeTest.TO_ADD_MESSAGEwithPhCompositeTest.TO_ADD_MESSAGEaligns with the broader refactoring. However, please ensure thatPhCompositeTest.TO_ADD_MESSAGEprovides the same assertion failure context as the original constant.Also applies to: 33-33, 38-38, 43-43, 48-48
🏁 Script executed:
#!/bin/bash # Verify that PhCompositeTest.TO_ADD_MESSAGE exists and check its value rg -A 2 -B 2 "TO_ADD_MESSAGE.*=" --type javaLength of output: 897
🏁 Script executed:
#!/bin/bash # Locate AtCompositeTest and display its TO_ADD_MESSAGE constant file=$(rg -l "class AtCompositeTest" --type java) echo "Found AtCompositeTest file: $file" rg -n "public static final String TO_ADD_MESSAGE" "$file"Length of output: 215
PhCompositeTest.TO_ADD_MESSAGE confirmed
- Verified that
PhCompositeTest.TO_ADD_MESSAGEis defined ineo-runtime/src/test/java/org/eolang/PhCompositeTest.javaas
public static final String TO_ADD_MESSAGE = "TO ADD ASSERTION MESSAGE";- The value matches the original
AtCompositeTestconstant, ensuring identical assertion context.No further changes required.
eo-runtime/src/test/java/org/eolang/BytesOfTest.java (1)
27-27: LGTM! Consistent message constant updates.All assertion message constants have been systematically updated from
AtCompositeTest.TO_ADD_MESSAGEtoPhCompositeTest.TO_ADD_MESSAGE, maintaining consistency with the broader refactoring fromAt*toPh*classes.Also applies to: 37-37, 47-47, 57-57, 66-66, 75-75, 85-85, 95-95, 107-107, 124-124, 146-146, 158-158
eo-runtime/src/main/java/org/eolang/PhOnce.java (1)
99-102: LGTM! Proper interface implementation.The new
copy(Phi self)override correctly implements the extendedPhiinterface by delegating to the existingcopy()method. ForPhOnce, ignoring theselfparameter is appropriate since the lazy evaluation semantics remain consistent regardless of context.eo-runtime/src/main/java/org/eolang/Data.java (1)
104-107: LGTM! Consistent interface implementation.The new
copy(Phi self)override properly implements the extendedPhiinterface by delegating to the existingcopy()method. This delegation pattern is appropriate for theToPhiwrapper class and maintains consistency with otherPhiimplementations in this refactoring.eo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOas_i64.java (1)
35-35: LGTM! Correct constant replacement as part of Attr → Phi refactoring.The change from
Attr.RHOtoPhi.RHOis consistent with the broader refactoring effort to replace the deprecatedAttrinterface with the unifiedPhiinterface.eo-runtime/src/main/java/org/eolang/PhSafe.java (1)
160-163: LGTM! Correct implementation of new Phi interface method.The new
copy(Phi self)method correctly implements the updatedPhiinterface requirement by delegating to the existing parameterlesscopy()method. This maintains backward compatibility while supporting the new interface signature.eo-runtime/src/main/java/EOorg/EOeolang/EOmath/EOreal$EOasin.java (1)
30-30: LGTM! Consistent constant replacement in math operation.The change from
Attr.RHOtoPhi.RHOis correct and maintains the existing arcsine calculation logic while aligning with the Attr → Phi interface refactoring.eo-runtime/src/main/java/EOorg/EOeolang/EOmath/EOangle$EOsin.java (1)
31-31: LGTM! Final consistent constant replacement completing the refactoring pattern.The change from
Attr.RHOtoPhi.RHOis correct and maintains the existing sine calculation logic. This completes the consistent pattern of Attr → Phi interface refactoring across all reviewed math operation files.eo-runtime/src/main/java/EOorg/EOeolang/EOmath/EOreal$EOsqrt.java (1)
30-30: LGTM: Correct Attr to Phi migration.The replacement of
Attr.RHOwithPhi.RHOis correct and aligns with the systematic refactoring to remove the legacyAttrinterface in favor of the unifiedPhiinterface.eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOfile$EOdeleted$EOdelete.java (1)
36-36: LGTM: Correct Attr to Phi migration in chained attribute access.The replacement of
Attr.RHOwithPhi.RHOin the chained attribute access pattern is correct and maintains the same semantics for navigating the nested object hierarchy to reach the "path" attribute.eo-runtime/src/main/java/EOorg/EOeolang/EOmath/EOreal$EOpow.java (2)
15-15: LGTM: Correct migration from AtVoid to PhVoid.The replacement of
AtVoidwithPhVoidin the import and constructor is correct. The newPhVoidclass provides equivalent functionality with improved thread-safety usingAtomicReferenceand consistent error handling for uninitialized access.Also applies to: 33-33
40-40: LGTM: Correct Attr to Phi constant migration.The replacement of
Attr.RHOwithPhi.RHOmaintains the same attribute access semantics while aligning with the unifiedPhiinterface approach.eo-runtime/src/test/java/org/eolang/PhiTest.java (1)
21-21: LGTM: Correct test constant migration.The systematic replacement of
AtCompositeTest.TO_ADD_MESSAGEwithPhCompositeTest.TO_ADD_MESSAGEacross all test methods correctly aligns the test assertions with the newPhCompositeclass structure while maintaining the same test logic and validation.Also applies to: 52-52, 71-71, 82-82, 98-98
eo-runtime/src/main/java/EOorg/EOeolang/EOmath/EOreal$EOln.java (1)
30-30: Consistent replacement of attribute constant
Replacing the deprecatedAttr.RHOwithPhi.RHOaligns this method with the newPhi-based attribute API. The natural logarithm logic remains untouched.eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOfile$EOis_directory.java (1)
34-34: Unified attribute constant usage
Switching fromAttr.RHOtoPhi.RHOproperly follows the new convention for root attribute access; functionality is preserved.eo-runtime/src/main/java/EOorg/EOeolang/EOmath/EOangle$EOcos.java (1)
31-31: Correct migration to Phi interface
The update to usePhi.RHOfor attribute retrieval is in line with the broader refactoring. The cosine calculation itself remains correct.eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOsize.java (1)
30-30: Appropriate constant replacement
ReplacingAttr.RHOwithPhi.RHOkeeps the byte‐array size logic intact while ensuring consistency with the new attribute handling model.eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOfile$EOtouched$EOtouch.java (1)
34-34: Attribute constant references updated
The nested.take()calls now consistently usePhi.RHOinstead ofAttr.RHO, preserving the original behavior of resolving the file path.eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOdir$EOtmpfile$EOtouch.java (1)
36-36: LGTM: Correct migration from Attr to Phi interface.The replacement of
Attr.RHOwithPhi.RHOcorrectly aligns with the refactoring to remove the legacyAttrinterface in favor of the unifiedPhiinterface.eo-runtime/src/test/java/org/eolang/ExpectTest.java (1)
158-272: LGTM: Consistent test updates for Phi interface migration.All test methods have been correctly updated to use
Phi.RHOinstead ofAttr.RHO. The test logic remains unchanged - these are purely mechanical replacements that maintain the same functionality while using the new interface.eo-runtime/src/test/java/org/eolang/MainTest.java (1)
139-169: LGTM: Test constants updated for new PhComposite class.The replacement of
AtCompositeTest.TO_ADD_MESSAGEwithPhCompositeTest.TO_ADD_MESSAGEcorrectly reflects the migration fromAtCompositetoPhCompositeclasses. The test assertions remain functionally identical.eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOfile$EOmoved$EOmove.java (1)
34-39: LGTM: Correct Phi interface migration in file operations.The replacement of
Attr.RHOwithPhi.RHOcorrectly updates the attribute access pattern while maintaining the same file move operation logic. This change aligns with the broader refactoring to use the unifiedPhiinterface.eo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOfloor.java (1)
31-31: LGTM! Consistent with the Attr → Phi migration.The replacement of
Attr.RHOwithPhi.RHOis correct and aligns with the broader refactoring to consolidate attribute handling under thePhiinterface.eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOfile$EOsize.java (1)
33-33: LGTM! Proper constant migration from Attr to Phi.The change from
Attr.RHOtoPhi.RHOmaintains the same functionality while adopting the new unifiedPhiinterface constants.eo-runtime/src/main/java/EOorg/EOeolang/EOtxt/EOsscanf.java (2)
24-24: LGTM! Proper import for the new PhVoid class.The import of
PhVoidis correctly added to support the migration fromAtVoidto the newPhi-based attribute system.
59-61: LGTM! Successful migration from AtVoid to PhVoid.The replacement of
AtVoidwithPhVoidfor both "format" and "read" attributes is correct. Based on thePhVoidimplementation in the codebase (lines 18-137 inPhVoid.java), this new class provides equivalent functionality with proper error handling for unset attributes and read-only protection, while implementing the unifiedPhiinterface.eo-runtime/src/main/java/EOorg/EOeolang/EOtxt/EOsprintf.java (2)
25-25: LGTM! Correct import for PhVoid class.The import statement properly includes
PhVoidto support the attribute system migration fromAtVoidto the newPhi-based architecture.
60-61: LGTM! Proper AtVoid to PhVoid migration.The constructor correctly replaces
AtVoidinstances withPhVoidfor both "format" and "args" attributes. This change aligns with the broader refactoring to unify attribute handling under thePhiinterface, andPhVoidprovides equivalent functionality with improved consistency across the codebase.eo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOtimes.java (3)
16-16: LGTM: Import updated for new attribute system.The import correctly adds
PhVoidas part of the migration from the legacyAttrinterface to the newPhiinterface system.
34-34: LGTM: Attribute initialization updated to use PhVoid.The constructor correctly replaces
AtVoidwithPhVoidwhile maintaining the same attribute name parameter. This change aligns with the broader refactoring to use the newPhi-based attribute system.
39-39: LGTM: Constant reference updated for new interface.The change from
Attr.RHOtoPhi.RHOcorrectly updates the constant reference to match the newPhiinterface system while preserving the same functionality for accessing the root object.eo-runtime/src/main/java/EOorg/EOeolang/EOtxt/EOregex$EOpattern$EOmatch$EOmatched_from_index.java (3)
22-22: LGTM: Import updated for new attribute system.The import correctly adds
PhVoidas part of the migration from the legacyAttrinterface to the newPhiinterface system.
53-53: LGTM: Attribute initialization updated to use PhVoid.Both attribute initializations correctly replace
AtVoidwithPhVoidwhile maintaining the same attribute name parameters (POSITIONandSTART). This change aligns with the broader refactoring to use the newPhi-based attribute system.Also applies to: 57-57
63-63: LGTM: Constant references updated for new interface.Both changes from
Attr.RHOtoPhi.RHOcorrectly update the constant references to match the newPhiinterface system while preserving the same functionality for accessing root and nested objects.Also applies to: 65-65
eo-runtime/src/main/java/EOorg/EOeolang/EOmalloc$EOof$EOallocated$EOwrite.java (3)
16-16: LGTM: Import updated for new attribute system.The import correctly adds
PhVoidas part of the migration from the legacyAttrinterface to the newPhiinterface system.
33-34: LGTM: Attribute initialization updated to use PhVoid.Both attribute initializations correctly replace
AtVoidwithPhVoidwhile maintaining the same attribute name parameters ("offset"and"data"). This change aligns with the broader refactoring to use the newPhi-based attribute system.
40-40: LGTM: Constant reference updated for new interface.The change from
Attr.RHOtoPhi.RHOcorrectly updates the constant reference to match the newPhiinterface system while preserving the same functionality for accessing the parent object.eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOeq.java (3)
17-17: LGTM: Import updated for new attribute system.The import correctly adds
PhVoidas part of the migration from the legacyAttrinterface to the newPhiinterface system.
35-35: LGTM: Attribute initialization updated to use PhVoid.The constructor correctly replaces
AtVoidwithPhVoidwhile maintaining the same attribute name parameter ("b"). This change aligns with the broader refactoring to use the newPhi-based attribute system.
45-45: LGTM: Constant reference updated for new interface.The change from
Attr.RHOtoPhi.RHOcorrectly updates the constant reference to match the newPhiinterface system while preserving the same functionality for accessing the root object.eo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOplus.java (3)
16-16: LGTM! Correct import addition for new PhVoid class.The import statement correctly adds
PhVoidto support the migration fromAtVoid.
34-34: LGTM! Proper migration from AtVoid to PhVoid.The constructor correctly initializes the "x" attribute using
PhVoidinstead of the deprecatedAtVoid. This change is consistent with the broader refactoring effort.
39-39: LGTM! Correct migration from Attr.RHO to Phi.RHO.The lambda method correctly uses
Phi.RHOinstead of the deprecatedAttr.RHOfor accessing the parent object attribute. This maintains the same functionality while using the new interface.eo-runtime/src/test/java/org/eolang/PhPackageTest.java (2)
35-35: LGTM! Correct migration of test message constants.All references to
AtCompositeTest.TO_ADD_MESSAGEhave been properly updated toPhCompositeTest.TO_ADD_MESSAGE, reflecting the migration from the oldAtCompositeclass to the newPhCompositeclass.Also applies to: 73-73, 82-82, 94-94, 105-105, 131-131
49-49: LGTM! Correct migration from Attr.RHO to Phi.RHO.All attribute references have been properly updated from
Attr.RHOtoPhi.RHO, maintaining the same test semantics while using the new interface. This ensures the tests remain valid for the migrated codebase.Also applies to: 52-52, 62-63, 74-74
eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOslice.java (3)
18-18: LGTM! Correct import addition for new PhVoid class.The import statement correctly adds
PhVoidto support the migration fromAtVoid.
36-37: LGTM! Proper migration from AtVoid to PhVoid for multiple attributes.The constructor correctly initializes both "start" and "len" attributes using
PhVoidinstead of the deprecatedAtVoid. This change is consistent with the broader refactoring effort and maintains the same attribute initialization semantics.
62-62: LGTM! Correct migration from Attr.RHO to Phi.RHO.The lambda method correctly uses
Phi.RHOinstead of the deprecatedAttr.RHOfor accessing the parent bytes object. This maintains the same functionality while using the new interface.eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOand.java (3)
16-16: LGTM! Correct import addition for new PhVoid class.The import statement correctly adds
PhVoidto support the migration fromAtVoid.
34-34: LGTM! Proper migration from AtVoid to PhVoid.The constructor correctly initializes the "b" attribute using
PhVoidinstead of the deprecatedAtVoid. This change is consistent with the broader refactoring effort.
40-40: LGTM! Correct migration from Attr.RHO to Phi.RHO.The lambda method correctly uses
Phi.RHOinstead of the deprecatedAttr.RHOfor accessing the parent bytes object. This maintains the same bitwise AND operation functionality while using the new interface.eo-runtime/src/main/java/org/eolang/PhPackage.java (2)
56-59: LGTM: Consistent implementation of copy method.The addition of
copy(Phi self)method correctly delegates to the existingcopy()method, following the pattern established in otherPhiimplementations as mentioned in the AI summary.
75-82: LGTM: Correct migration from Attr.RHO to Phi.RHO.All references to
Attr.RHOhave been consistently updated toPhi.RHOincluding in the conditional check, map operations, and error message. This maintains the same logic while using the new constant location.Also applies to: 97-97
eo-runtime/src/test/java/org/eolang/PhWithTest.java (3)
28-28: LGTM: Consistent test message constant updates.All test assertion messages have been correctly updated from
AtCompositeTest.TO_ADD_MESSAGEtoPhCompositeTest.TO_ADD_MESSAGE, reflecting the test class rename in the refactoring.Also applies to: 36-36, 48-48, 69-69, 84-84
104-104: LGTM: Correct migration from AtVoid to PhVoid.The change from
AtVoidtoPhVoidmaintains the same void attribute functionality while using the newPhi-based implementation.
119-119: LGTM: Correct migration from AtComposite to PhComposite.The change from
AtCompositetoPhCompositemaintains the same composite attribute functionality with the same lambda parameter, now using the newPhi-based implementation.eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOor.java (2)
16-16: LGTM: Correct migration from AtVoid to PhVoid.The import and constructor have been correctly updated to use
PhVoidinstead ofAtVoid. Based on the relevant code snippets,PhVoidimplements the same void attribute functionality asAtVoidbut using the newPhiinterface.Also applies to: 34-34
40-40: LGTM: Correct constant migration.The change from
Attr.RHOtoPhi.RHOis consistent with the refactoring and maintains the same functionality for accessing the rho attribute.eo-runtime/src/test/java/EOorg/EOeolang/EOnumberTest.java (2)
15-15: LGTM: Consistent test updates for renamed test class.The import and all test assertion messages have been correctly updated from
AtCompositeTesttoPhCompositeTest, reflecting the test class rename in the refactoring.Also applies to: 37-37, 46-46, 55-55
77-77: LGTM: Correct constant migration in parameterized tests.The changes from
Attr.RHOtoPhi.RHOin both parameterized tests are consistent with the refactoring and maintain the same test logic for verifying error messages with the rho attribute.Also applies to: 107-107
eo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOdiv.java (3)
16-16: LGTM: Import updated for new Phi-based architecture.The import correctly changes from the deprecated
AtVoidto the newPhVoidclass as part of the Attr→Phi interface migration.
34-34: LGTM: Constructor updated to use PhVoid.The attribute initialization correctly migrated from
AtVoidtoPhVoid. Based on the relevant code snippet fromPhVoid.java, this maintains the same functionality while using the new Phi-based architecture.
39-39: LGTM: Constant reference updated to Phi interface.The change from
Attr.RHOtoPhi.RHOis consistent with the migration to the unified Phi interface and maintains the same functionality.eo-runtime/src/test/java/EOorg/EOeolang/HeapsTest.java (3)
14-17: LGTM: Test imports updated for Phi-based architecture.The import statements correctly migrate from the deprecated
AtComposite,AtCompositeTest, andAtVoidto their newPhComposite,PhCompositeTest, andPhVoidcounterparts.
41-41: LGTM: Test message constants updated consistently.All test assertions correctly migrated from
AtCompositeTest.TO_ADD_MESSAGEtoPhCompositeTest.TO_ADD_MESSAGE, maintaining consistent test messaging across the refactored codebase.Also applies to: 53-53, 62-62, 75-75, 87-87, 96-96, 106-106, 115-115, 128-128, 140-140, 151-151, 165-165, 174-174
279-280: LGTM: Test helper class updated to use new Phi classes.The
PhFakeconstructor correctly migrated fromAtVoidandAtCompositetoPhVoidandPhComposite, maintaining the same test functionality while using the new architecture.eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOfile$EOopen$EOfile_stream$EOread$EOread_bytes.java (3)
18-18: LGTM: Import updated for new Phi-based architecture.The import correctly changes from
AtVoidtoPhVoidas part of the systematic Attr→Phi interface migration.
39-39: LGTM: Attribute initialization updated to use PhVoid.The "size" attribute initialization correctly migrated from
AtVoidtoPhVoid, maintaining the same void attribute semantics with the new Phi-based architecture.
46-49: LGTM: RHO constant references updated consistently.The chained
take(Phi.RHO)calls correctly migrated fromAttr.RHOtoPhi.RHO, maintaining the same attribute traversal functionality while using the unified Phi interface.eo-maven-plugin/src/main/resources/org/eolang/maven/transpile/to-java.xsl (4)
369-371: LGTM: Void template updated to generate PhVoid.The XSL template correctly updates void element generation from
AtVoidtoPhVoid, ensuring transpiled EO code uses the new Phi-based architecture.
399-425: LGTM: Atom template updated to use PhCached and PhComposite.The template correctly migrates from
AtOnce(AtComposite(...))toPhCached(PhComposite(...)), maintaining the same caching and composition semantics while using the new Phi-based classes.
435-468: LGTM: Abstract template updated to use PhCached and PhComposite.The template correctly updates abstract object generation from the deprecated
AtOnce(AtComposite(...))pattern toPhCached(PhComposite(...)), maintaining lazy evaluation and caching functionality.
476-497: LGTM: Bound template updated to use PhCached and PhComposite.The template correctly migrates bound object generation to use
PhCached(PhComposite(...))instead ofAtOnce(AtComposite(...)), completing the transpiler migration to the new Phi-based architecture.eo-runtime/src/main/java/EOorg/EOeolang/EOi64$EOtimes.java (3)
17-17: LGTM: Correct import for the new PhVoid class.The import change from
AtVoidtoPhVoidis consistent with the systematic migration from theAttrinterface to thePhiinterface.
34-34: LGTM: Correct migration from AtVoid to PhVoid.The constructor change properly replaces
AtVoidwithPhVoidwhile maintaining the same parameter pattern. ThePhVoidclass provides equivalent functionality within the newPhi-based architecture.
44-44: LGTM: Correct migration from Attr.RHO to Phi.RHO.The change properly updates the root object access to use the new
Phi.RHOconstant instead of the removedAttr.RHO. This maintains the same functionality while using the new interface.eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOxor.java (2)
16-16: LGTM: Consistent migration pattern from AtVoid to PhVoid.The import addition and constructor change follow the same systematic migration pattern seen across the codebase, replacing
AtVoidwithPhVoidconsistently.Also applies to: 34-34
40-40: LGTM: Consistent migration from Attr.RHO to Phi.RHO.The attribute access change maintains consistency with the systematic migration across the codebase.
eo-integration-tests/src/test/java/integration/SnippetIT.java (2)
25-25: LGTM: Appropriate temporary disabling of integration tests.The
@Disabledannotation is correctly applied to prevent test failures during the migration period.Also applies to: 37-37
32-35: Excellent documentation for temporary test disabling.The TODO comment provides clear and comprehensive explanation for why the tests are disabled, what specific dependency issue causes the failure, and when they should be re-enabled. This is exactly how temporary test disabling should be documented.
eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOfile$EOopen$EOfile_stream$EOwrite$EOwritten_bytes.java (2)
19-19: LGTM: Consistent migration pattern for PhVoid.The import and constructor changes follow the same systematic migration pattern, replacing
AtVoidwithPhVoidfor the "buffer" attribute.Also applies to: 40-40
47-50: LGTM: Correct migration of chained attribute access.The chained
.take(Phi.RHO)calls properly navigate through the nested object hierarchy, consistently replacingAttr.RHOwithPhi.RHOthroughout the chain to access the "path" attribute.eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOconcat.java (3)
16-16: Import change looks correct.The import of
PhVoidis appropriate for the refactoring fromAtVoidtoPhVoid.
34-34: Constructor update is correct.The replacement of
AtVoidwithPhVoidfor the "b" attribute is consistent with the architectural migration and maintains the same semantics.
39-39: Constant usage update is correct.The change from
Attr.RHOtoPhi.RHOaligns with the interface migration and maintains the same functionality for accessing the root object.eo-runtime/src/main/java/EOorg/EOeolang/EOi64$EOdiv.java (3)
17-17: Import addition is correct.The
PhVoidimport is properly added to support the refactoring fromAtVoid.
34-34: Attribute initialization updated correctly.The change from
AtVoidtoPhVoidfor the "x" attribute maintains the same void attribute semantics while using the new interface.
44-44: Root object access updated correctly.The migration from
Attr.RHOtoPhi.RHOis consistent with the interface refactoring and preserves the same functionality for accessing the root object in the division operation.eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOright.java (3)
16-16: Import update is appropriate.The addition of
PhVoidimport supports the migration from the legacyAtVoidclass.
34-34: Attribute creation updated correctly.The replacement of
AtVoidwithPhVoidfor the "x" attribute is consistent with the architectural changes and maintains the same void attribute behavior.
40-40: Root object constant updated correctly.The change from
Attr.RHOtoPhi.RHOcorrectly follows the interface migration pattern and preserves the functionality for accessing the root object in the byte shift operation.eo-runtime/src/main/java/EOorg/EOeolang/EOi64$EOgt.java (3)
15-15: Import addition is correct.The
PhVoidimport is properly added to support the migration fromAtVoid.
33-33: Constructor change is correct.The replacement of
AtVoidwithPhVoidfor the "x" attribute follows the established refactoring pattern and maintains the same semantics.
39-39: Root object access updated correctly.The change from
Attr.RHOtoPhi.RHOis consistent with the interface migration and maintains the same functionality for accessing the root object in the comparison operation.eo-runtime/src/test/java/EOorg/EOeolang/EOerrorTest.java (3)
17-19: LGTM! Import updates align with the Attr to Phi migration.The import statements have been correctly updated to use the new
PhCached,PhComposite, andPhCompositeTestclasses instead of theirAt*counterparts.
50-50: Correct update of test assertion message constant.The test assertion message has been properly updated to reference
PhCompositeTest.TO_ADD_MESSAGEinstead of the deprecatedAtCompositeTest.TO_ADD_MESSAGE.
103-104: Proper migration from AtOnce/AtComposite to PhCached/PhComposite.The constructor calls have been correctly updated to use the new
PhCachedandPhCompositeclasses while maintaining the same nesting structure and functionality.eo-runtime/src/main/java/EOorg/EOeolang/EOi64$EOplus.java (3)
17-17: Import correctly updated for the new PhVoid class.The import has been properly updated to use
org.eolang.PhVoidinstead of the deprecatedAtVoidclass.
35-35: Attribute initialization migrated to PhVoid.The attribute initialization has been correctly updated to use
new PhVoid("x")instead ofnew AtVoid("x"). Based on the provided code snippets,PhVoidprovides equivalent functionality for void attribute handling.
46-46: Constant reference correctly migrated from Attr to Phi.The RHO constant reference has been properly updated from
Attr.RHOtoPhi.RHO, aligning with the migration of attribute constants to the Phi interface.eo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOgt.java (3)
16-16: Import statement correctly updated for PhVoid.The import has been properly updated to use the new
PhVoidclass instead of the deprecatedAtVoid.
34-34: Attribute initialization correctly migrated.The void attribute initialization has been properly updated to use
new PhVoid("x"), maintaining the same semantics while using the new class hierarchy.
39-39: RHO constant reference properly migrated.The constant reference has been correctly updated from
Attr.RHOtoPhi.RHO, consistent with the overall migration to the Phi interface.eo-runtime/src/test/java/EOorg/EOeolang/EOmallocTest.java (4)
15-19: Import statements correctly updated for the new class hierarchy.All import statements have been properly updated to use the new
PhComposite,PhCompositeTest, andPhVoidclasses instead of their deprecatedAt*counterparts.
44-44: Test assertion messages consistently updated.All test assertion messages have been properly updated to reference
PhCompositeTest.TO_ADD_MESSAGEinstead of the deprecatedAtCompositeTest.TO_ADD_MESSAGE, ensuring consistency across the test file.Also applies to: 59-59, 64-64
96-96: Void attribute initialization correctly migrated.Both
DummyandErrorDummyclasses have been properly updated to usenew PhVoid("m")instead ofnew AtVoid("m")for attribute initialization.Also applies to: 127-127
98-99: Phi constant and composite construction properly migrated.The constant references have been correctly updated from
Attr.PHItoPhi.PHI, and the composite object construction has been properly updated to usenew PhComposite(...)instead ofnew AtComposite(...).Also applies to: 129-130
eo-runtime/src/test/java/org/eolang/PhaSimple.java (3)
21-21: LGTM! Correct class inheritance update.The class properly extends
PhaEnvelopeinstead of the removedAtEnvelope, maintaining the same wrapper pattern for the new Phi-based architecture.
25-27: LGTM! Constructor properly updated.The no-argument constructor correctly delegates to the parameterized constructor while maintaining the same behavior with
Phi.Φ.
33-35: LGTM! Correct migration to PhComposite.The constructor parameter and superclass call have been properly updated to use
PhCompositeinstead ofAtComposite, maintaining the same functional behavior while adopting the new Phi-based attribute model.eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOdir$EOwalk.java (3)
23-23: LGTM! Correct import update.The import has been properly updated from
AtVoidtoPhVoidas part of the Attr → Phi migration.
41-41: LGTM! Proper PhVoid attribute initialization.The attribute is correctly initialized with
PhVoidinstead ofAtVoid. Based on thePhVoidimplementation, this maintains the same lazy initialization semantics while implementing the newPhiinterface.
48-48: LGTM! Correct constant migration.The root object access has been properly updated from
Attr.RHOtoPhi.RHO, aligning with the migration of constants to thePhiinterface.eo-runtime/src/main/java/EOorg/EOeolang/EOmalloc$EOof$EOallocated$EOread.java (3)
16-16: LGTM! Correct import update.The import has been properly updated from
AtVoidtoPhVoidas part of the comprehensive Attr → Phi interface migration.
33-34: LGTM! Proper attribute initialization with PhVoid.Both "offset" and "length" attributes are correctly initialized using
PhVoidinstead ofAtVoid. This maintains the same lazy initialization behavior while implementing the newPhiinterface, ensuring consistency with the broader refactoring.
41-41: LGTM! Correct root object access.The code properly uses
Phi.RHOinstead ofAttr.RHOto access the root object's "id" attribute, consistent with the migration of constants to thePhiinterface.eo-runtime/src/main/java/EOorg/EOeolang/EOmalloc$EOof$EOallocated$EOresized.java (3)
16-16: LGTM! Correct import update.The import has been properly updated from
AtVoidtoPhVoid, maintaining consistency with the Attr → Phi interface migration across the codebase.
33-33: LGTM! Proper PhVoid attribute initialization.The "new-size" attribute is correctly initialized using
PhVoidinstead ofAtVoid. This preserves the lazy initialization semantics while adopting the newPhiinterface implementation.
38-38: LGTM! Correct root object access pattern.The code properly uses
Phi.RHOinstead ofAttr.RHOto access the root object, completing the migration from the legacyAttrconstants to the newPhiinterface constants.eo-runtime/src/test/java/org/eolang/PhaLoggedTest.java (4)
19-28: LGTM: Class rename and field declarations are consistent.The refactoring from
AtLoggedTesttoPhaLoggedTestis correctly implemented with proper field type updates to use the newPha*classes.
59-65: LGTM: Object initialization updated correctly.The test setup properly uses the new
PhaSimple,PhaLogged, andPhVoidclasses instead of theirAt*predecessors.
104-104: LGTM: Put method signature updated correctly.The
putmethod call is correctly updated to use the new signatureput("start", Phi.Φ)which accepts a key-value pair instead of a single argument.
87-89:Details
✅ Verification successful
Verify method signature change from get() to take(0).
The test correctly updates from
get()totake(0)method calls, which aligns with the newPhiinterface. However, ensure this change is semantically equivalent across all implementations.
🏁 Script executed:
#!/bin/bash # Description: Verify that take(0) is the correct replacement for get() in Phi implementations # Search for get() method implementations in old Attr classes to understand the semantics rg -A 5 "public.*get\(\)" --type java # Search for take(0) method implementations in new Phi classes rg -A 5 "public.*take\(.*int.*\)" --type javaLength of output: 11148
Confirm take(0) correctly replaces legacy get()
AllPhi.take(int)overrides—includingPhaLogged.take(0)—delegate to the same 0th attribute as the oldget(), yielding identical behavior. No further changes needed.eo-runtime/src/test/java/org/eolang/PhCompositeTest.java (2)
35-42: LGTM: Test logic updated correctly for new PhComposite class.The test instantiation and method call are properly updated to use
PhCompositeandtake(0)instead of the previousAtCompositeandget().
48-70: LGTM: Method signature and constants updated consistently.The test correctly uses:
PhMethodwithPhi.LAMBDAconstant (instead ofAttr.LAMBDA)- Proper reference to
PhCompositeTest.TO_ADD_MESSAGEeo-runtime/src/test/java/EOorg/EOeolang/EOtryTest.java (6)
15-21: LGTM: Import statements updated consistently.All imports have been correctly updated to use the new
Ph*classes:
PhCompositeinstead ofAtCompositePhCompositeTestinstead ofAtCompositeTestPhVoidinstead ofAtVoid
78-78: LGTM: Test message constant reference updated.The reference to
PhCompositeTest.TO_ADD_MESSAGEis correctly updated from the previousAtCompositeTest.TO_ADD_MESSAGE.
128-136: LGTM: Inner class MainWithCounter updated correctly.The class properly uses:
Phi.PHIconstant instead ofAttr.PHIPhCompositeinstead ofAtCompositeThe logic and structure remain unchanged, which preserves test behavior.
153-159: LGTM: Inner class Main updated correctly.The
Mainclass correctly usesPhCompositeinstead ofAtCompositewhile preserving the same test logic.
175-181: LGTM: Inner class Broken updated correctly.The
Brokenclass correctly usesPhCompositeinstead ofAtCompositefor exception testing.
195-202: LGTM: Inner class Catcher updated correctly.The
Catcherclass properly uses:
PhVoidinstead ofAtVoidPhCompositeinstead ofAtCompositeeo-runtime/src/main/java/org/eolang/Phi.java (2)
22-36: LGTM: Attribute constants correctly defined.The three constants are properly defined with their corresponding Unicode values:
LAMBDA = "λ"PHI = "φ"(with appropriate PMD suppression)RHO = "ρ"The PMD suppression for
PHIis justified since this field intentionally matches the interface name by design.
99-108:Details
✅ Verification successful
Address the TODO comment for temporary copy(Phi self) method.
The new
copy(Phi self)method includes a TODO comment indicating it should be removed after the refactoring is complete. This suggests it's a temporary bridge method during the transition fromAttrtoPhi.Consider creating an issue to track the removal of this temporary method once the refactoring is fully complete.
🏁 Script executed:
#!/bin/bash # Description: Check if copy(Phi self) method is still needed across the codebase # Search for usages of copy(Phi) method rg "\.copy\(" --type java -A 2 -B 2 # Search for PhDefault.copy() implementation to understand the dependency ast-grep --pattern 'class PhDefault { $$$ copy() { $$$ } $$$ }'Length of output: 64725
Keep
copy(Phi self)as a temporary bridge—still widely used
We’ve confirmed that thePhi.copy(Phi self)method is invoked across the codebase—in core implementations (e.g. PhEnvelope, PhCached, PhWithRho) and in scores of tests—and cannot yet be removed. The existing@todo #3480:45min Remove Phi.copy(Phi) method from the interface.remains valid. Please keep the TODO and open an issue (tracking #3480) to schedule its removal once all callers have migrated to
Phi.copy().eo-runtime/src/main/java/org/eolang/PhDefault.java (1)
150-161: Potentially incomplete value retrieval viatake(name)
take(name)returnsattrs.get(name).take(0), ignoringname.
After the Attr→Phi migration, position 0 is not guaranteed to yield the attribute’s value; it might be a lambda/placeholder. This mirrors the issue noted inPhWithRho.Please review whether the original behaviour should instead be:
object = this.attrs.get(name).take(name);or simply:
object = this.attrs.get(name);If the current design is intentional, add explanatory Javadoc to prevent future regressions.
eo-runtime/src/test/java/org/eolang/PhDefaultTest.java (3)
64-65: LGTM! Correct migration from Attr to Phi constants.The systematic replacement of
Attr.RHOandAttr.PHIwithPhi.RHOandPhi.PHIcorrectly reflects the migration from the deprecatedAttrinterface to the unifiedPhiinterface.Also applies to: 73-75, 83-85, 119-123, 130-131, 155-159, 163-167, 178-184, 192-195
516-517: LGTM! Proper replacement of legacy attribute classes.The systematic replacement of
At*classes (AtVoid,AtSimple,AtOnce,AtComposite) with their newPh*counterparts (PhVoid,PhaSimple,PhCached,PhComposite) correctly implements the architectural shift to thePhi-based attribute model.Also applies to: 520-525, 529-541, 555-558, 571-571, 602-602, 616-617, 637-637, 671-672, 705-706
228-228: LGTM! Test message constants correctly updated.The replacement of
AtCompositeTest.TO_ADD_MESSAGEwithPhCompositeTest.TO_ADD_MESSAGEproperly aligns with the class renaming in the refactoring.Also applies to: 238-238, 250-250, 255-255, 275-275, 301-301, 312-312, 326-326, 335-335, 346-346, 357-357, 371-371, 380-380, 408-408, 425-425, 467-467
eo-runtime/src/main/java/org/eolang/PhRho.java (1)
68-80: LGTM! Proper single-assignment semantics.The
put()methods correctly implement single-assignment semantics by only setting the rho if it's currently null, ignoring subsequent assignments.eo-runtime/src/main/java/org/eolang/PhCached.java (2)
74-81: LGTM! Proper read-only enforcement.The
put()methods correctly throwExReadOnlyexceptions to prevent modification of cached attributes, which is the expected behavior for a caching wrapper.Also applies to: 84-91
38-41: LGTM! Proper caching behavior in copy method.The
copy(Phi self)method correctly creates a newPhCachedinstance with the copied origin, ensuring that cached values are not shared between copies.eo-runtime/src/main/java/org/eolang/PhComposite.java (5)
17-17: LGTM: Well-designed replacement for AtComposite.The class design is clean and properly implements the
Phiinterface as part of the migration from theAttr-based system.
33-36: LGTM: Constructor implementation is correct.The constructor properly initializes both the argument and expression fields, maintaining the context-dependent nature of the composite attribute.
64-68: LGTM: Proper read-only enforcement.The
putmethods correctly throwExReadOnlyexceptions to enforce immutability of lambda expressions, which is the expected behavior for composite attributes.Also applies to: 81-85
64-68: Proper read-only enforcement.Correctly throws
ExReadOnlyto prevent mutation of the lambda expression, maintaining immutability.
81-85: Consistent read-only enforcement.Properly prevents mutation attempts through positional assignment, maintaining the read-only nature of the composite attribute.
eo-runtime/src/main/java/org/eolang/PhVoid.java (8)
18-18: LGTM: Successful migration from AtVoid to PhVoid.The class has been properly renamed and refactored to implement the
Phiinterface as part of the broader migration from theAttr-based system.
48-57: LGTM: Proper null handling in copy method.The
copy(Phi self)method correctly handles the case where the internal object might be null, ensuring safe copying behavior.
70-81: LGTM: Proper single-assignment semantics.The
putmethods correctly implement single-assignment semantics with thread-safe checks and appropriate error messages when attempting to reassign.Also applies to: 84-95
98-108: LGTM: Proper null checking in take methods.Both
takemethods correctly check for null and throwExUnsetwith descriptive error messages when the attribute is not initialized.Also applies to: 121-131
116-118:Details
✅ Verification successful
Verify hasRho() implementation.
The method always returns
false. Please verify that this is correct for void attributes, or if it should delegate to the internal object when present.
🏁 Script executed:
#!/bin/bash # Description: Search for other hasRho() implementations to understand the expected behavior # Expected: Find patterns for how hasRho() should be implemented for void attributes echo "Searching for hasRho implementations in Phi classes:" rg -A 3 "hasRho\(\)" --type java echo -e "\nSearching for documentation or comments about hasRho behavior:" rg -A 5 -B 5 "hasRho|has.*rho" --type javaLength of output: 19027
No changes needed: PhVoid.hasRho() is correct
The void‐wrapper is meant to suppress any bound “rho” attribute, so always returning false here is intentional. On copy, PhVoid unwraps itself (viaobject.get().copy()), allowing the underlying object’s ownhasRho()to be used thereafter.
98-108: Proper null checking and error handling.The
take(int pos)method correctly checks for null and provides a clear error message when the attribute is uninitialized.
121-131: Consistent null checking for named access.The
take(String nme)method properly mirrors the positional take method with appropriate null checking and error messages.
48-57: Good copy semantics with null handling.The
copy(Phi self)method properly handles the case where the internal object might be null, ensuring safe copying behavior.
|
@maxonfjvipon updated. Take a look, please |
|
@h1alexbel what do you think about suggestions made by the Rabbit above? Some of them look reasonable to me. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
eo-runtime/src/main/java/org/eolang/PhWithRho.java (2)
37-40: LGTM! Previous issue with copy() has been resolved.The
copy()method now correctly returns a newPhWithRhoinstance that preserves the wrapper behavior, addressing the previous concern about losing the RHO-propagating wrapper.
48-65: LGTM! Previous issue with take() methods has been resolved.Both
take(String name)andtake(int pos)methods now correctly use their respective parameters (nameandpos) instead of the hardcoded0, addressing the previous functional bug. The logic for auto-injecting rho when attributes don't have it is also implemented correctly.
🧹 Nitpick comments (3)
eo-runtime/src/main/java/org/eolang/PhWithRho.java (3)
32-35: Consider adding null safety checks.The constructor doesn't validate that
attrandrhoparameters are non-null. While this might be intentional if null values are valid in the EO runtime, consider adding documentation or validation if null values could cause issues downstream.PhWithRho(final Phi attr, final Phi rho) { + if (attr == null) { + throw new IllegalArgumentException("attr cannot be null"); + } + if (rho == null) { + throw new IllegalArgumentException("rho cannot be null"); + } this.original = attr; this.rho = rho; }
11-13: Address the TODO comment about moving origin() method.The TODO comment indicates that the
origin()method should be moved elsewhere to reduce the method count and eliminate the PMD violation. This refactoring could improve the class design.Would you like me to suggest an alternative design pattern or help identify where this method could be moved to address the
TooManyMethodsPMD violation?Also applies to: 104-106
50-53: Consider performance implications of frequent copying.The
take()methods create a copy of the attribute when injecting rho. While this preserves immutability and avoids side effects, it could impact performance if called frequently in hot paths.Consider documenting this behavior or exploring alternatives like lazy rho injection if performance becomes a concern:
// Current approach: always copy when injecting rho if (!ret.hasRho()) { ret = ret.copy(); // This creates a new instance ret.put(Phi.RHO, this.rho); }Also applies to: 60-63
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
eo-runtime/src/main/java/EOorg/EOeolang/EOi64$EOgt.java(2 hunks)eo-runtime/src/main/java/org/eolang/PhCached.java(1 hunks)eo-runtime/src/main/java/org/eolang/PhVoid.java(4 hunks)eo-runtime/src/main/java/org/eolang/PhWithRho.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- eo-runtime/src/main/java/EOorg/EOeolang/EOi64$EOgt.java
- eo-runtime/src/main/java/org/eolang/PhCached.java
- eo-runtime/src/main/java/org/eolang/PhVoid.java
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: ort
- GitHub Check: benchmark
- GitHub Check: qulice
- GitHub Check: integration
- GitHub Check: mvn (macos-15, 23)
- GitHub Check: mvn (ubuntu-24.04, 11)
- GitHub Check: mvn (ubuntu-24.04, 23)
- GitHub Check: trufflehog
- GitHub Check: mvn (windows-2022, 23)
- GitHub Check: snippets
- GitHub Check: ebnf
|
@yegor256 @maxonfjvipon addressed Rabbit comments |
|
@maxonfjvipon reminder |
|
@h1alexbel what about Rabbit's suggestions? Some of them look reasonable. |
Head branch was pushed to by a user without write access
|
@yegor256 applied, take a look again, please |
|
@h1alexbel will be merged once all conversations are resolved above |
|
@yegor256 Hey there, code review superstar! 🌟 You've just racked up a sweet +24 points for your awesome review. Here's the breakdown: +12 for stepping up, +16 for tackling those 2102 hits-of-code (impressive!), and +6 for your 27 insightful comments. We had to trim 11 points to stay within the cap, but that's still a fantastic score! Your dedication to improving code quality is truly paying off. Keep up the great work! By the way, your current balance stands at -342. Let's keep that momentum going! |
|
@h1alexbel 🎉 Great contribution! You've earned +4 points for your hard work. While the code was a bit lengthy (2102 hits-of-code) and there were quite a few review comments (39), we appreciate your effort. Your current balance is +462. Keep up the good work and remember to focus on quality in your future submissions! |
4 participants
In this PR I've removed redundant
org.eolang.Attrinterface in lieu oforg.eolang.Phi. Left several puzzles.see #3480
History:
Attrinterface is redundant, can be removed #3480): PhRho, PhWithRhoAttrinterface is redundant, can be removed #3480): normal putAttrinterface is redundant, can be removed #3480): put phi offAttrinterface is redundant, can be removed #3480): get offAttrinterface is redundant, can be removed #3480): rename to PhVoidAttrinterface is redundant, can be removed #3480): to PhiAttrinterface is redundant, can be removed #3480): no methodsAttrinterface is redundant, can be removed #3480): Attr offAttrinterface is redundant, can be removed #3480): rho implementsAttrinterface is redundant, can be removed #3480): implementAttrinterface is redundant, can be removed #3480): fails ITsAttrinterface is redundant, can be removed #3480): todoAttrinterface is redundant, can be removed #3480): readme snippets tooSummary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests
Chores