refactor(#4884): upgrade qulice to version 0.25.1 in eo-runtime#4903
Merged
yegor256 merged 1 commit intoMar 1, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR pins qulice-maven-plugin to 0.25.1, adjusts PMD suppression annotations across many runtime classes and tests, adds several Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR upgrades the qulice static analysis tool from an earlier version to 0.25.1 in the eo-runtime module. The upgrade enforces stricter code quality rules, resulting in numerous automated code style improvements throughout the test and production codebase.
Changes:
- Upgraded qulice version to 0.25.1 in pom.xml
- Inlined unnecessary local variables to comply with PMD.UnnecessaryLocalRule
- Added
finalmodifier to inner test classes - Split tests with multiple assertions into separate test methods
- Removed obsolete PMD suppressions (ConstructorOnlyInitializesOrCallOtherConstructors, AvoidDollarSigns, CognitiveComplexity)
- Added
@FunctionalInterfaceannotations to single-method interfaces - Reformatted method signatures and added appropriate PMD suppressions for synchronized blocks
- Initialized StringBuilder with explicit capacity values
- Removed redundant parentheses and type casts
Reviewed changes
Copilot reviewed 64 out of 64 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| eo-runtime/pom.xml | Updated qulice plugin version to 0.25.1 |
| PhWithTest.java | Inlined local variables, made inner classes final, removed constructor suppression |
| PhSafeTest.java | Added suppressions for tests with multiple assertions |
| PhPackageTest.java | Inlined local variables, added suppression |
| PhMethodTest.java | Inlined variables, made inner class final, removed constructor suppression |
| PhDefaultTest.java | Split tests, made inner classes final, removed PMD.GodClass suppression, removed constructor suppressions |
| MainTest.java | Inlined local variables into assertions |
| ExpectTest.java | Added suppressions for tests with multiple assertions |
| DataizedTest.java | Made inner class final, added suppressions |
| DataTest.java | Converted multiple similar tests into parameterized test |
| BytesOfTest.java | Inlined local variables |
| AtWithRhoTest.java | Split tests into separate test methods |
| AtOnceTest.java | Added suppression for unnecessary local rule |
| AtCompositeTest.java | Inlined local variables, added suppression |
| HeapsTest.java | Inlined local variables, added suppressions |
| EOtryTest.java | Made inner classes final, removed constructor suppressions |
| EOnumberTest.java | Added suppressions for tests with multiple assertions |
| EOsocketTest.java | Removed PMD.CloseResource suppression, added suppressions, inlined variables, removed redundant casts |
| EOmallocTest.java | Made inner classes final, added suppressions |
| InputOutputTest.java | Inlined local variables, removed PMD.TooManyMethods suppression, reformatted method signatures |
| FilesTest.java | Reformatted try-with-resources statements |
| EOerrorTest.java | Removed constructor suppression, added suppression |
| EObytesEOsliceTest.java | Added suppression |
| VerboseBytesAsString.java | Inlined local variable |
| PhSafe.java | Reformatted method signature, added suppression, changed PMD.AvoidCatchingThrowable to AvoidCatchingGenericException |
| PhPackage.java | Removed suppressions, reformatted exception handling |
| PhOnce.java | Added TODO about replacing synchronized with ReentrantLock, added suppression |
| PhLogged.java | Removed PMD.TooManyMethods suppression, added method-level suppressions |
| PhDefault.java | Removed PMD.GodClass suppression |
| PhCached.java | Added TODO about synchronized, added suppression |
| Main.java | Added suppressions |
| JavaPath.java | Fixed static field access pattern |
| Dataized.java | Added suppression |
| Data.java | Added @FunctionalInterface annotation |
| BytesRaw.java | Changed StringBuilder initial capacity, removed redundant parentheses |
| BytesOf.java | Removed PMD.CognitiveComplexity suppression |
| AtomSafe.java | Removed PMD.AvoidRethrowingException suppression |
| Atom.java | Added @FunctionalInterface annotation |
| AtOnce.java | Added TODO about synchronized, added class-level suppression |
| Heaps.java | Added TODO about synchronized, added suppressions |
| EOsscanf.java | Removed PMD.AvoidDollarSigns suppression, added StringBuilder capacity |
| EOsprintf.java | Removed PMD.AvoidDollarSigns suppression |
| EOregex...matched_from_index.java | Removed unused import, inlined local variables |
| WinNT.java | Added PMD.ConstantsInInterface suppression |
| WinDef.java | Added suppressions for comparable classes |
| WinBase.java | Added PMD.DataClass suppression |
| RecvFuncCall.java | Removed redundant cast |
| ReadFileFuncCall.java | Removed redundant cast |
| InetAddrFuncCall.java | Inlined local variable |
| Syscall.java | Added @FunctionalInterface annotation |
| SockaddrIn.java | Removed PMD.DataClass suppression |
| RecvSyscall.java (Posix) | Removed redundant cast |
| ReadSyscall.java (Posix) | Removed redundant cast |
| EOnumber$EOtimes.java | Inlined local variables |
| EOnumber$EOplus.java | Inlined local variables |
| EOnumber$EOgt.java | Added suppression |
| EOnumber$EOdiv.java | Added suppression |
| EOpow.java | Removed PMD.AvoidDollarSigns suppression |
| EOmalloc$...EOresized.java | Added suppression |
| Files.java | Added TODO about synchronized, added suppressions |
| EOdir$EOwalk.java | Inlined local variable, added suppression |
| EOerror.java | Inlined local variable, reformatted method signature |
| EObytes$EOslice.java | Added suppression |
Comments suppressed due to low confidence (9)
eo-runtime/src/test/java/org/eolang/PhDefaultTest.java:165
- This test violates the coding guideline that states "Each test ends with an assertion—there is no code after it." Line 165 executes
phi.take(this.plus()).take(Phi.RHO);which is a side effect that should be part of the test setup, not placed after the variable initialization and before the assertion.
eo-runtime/src/test/java/org/eolang/AtWithRhoTest.java:30 - This test violates the coding guideline that states "Each test ends with an assertion—there is no code after it." Line 30 executes
phi.take(Phi.RHO);which is a side effect that should be part of the test setup, not placed after the variable initialization and before the assertion.
eo-runtime/src/test/java/org/eolang/PhDefaultTest.java:122 - This test violates the coding guideline that states "Each test ends with an assertion—there is no code after it." Line 122 executes
plus.take(Phi.RHO);which appears to be required setup for the assertion that follows, but the assertion is not the final line of code execution in the test. The side effect of callingtake(Phi.RHO)should be part of the test setup, not placed between the variable initialization and the assertion.
eo-runtime/src/test/java/org/eolang/AtWithRhoTest.java:68 - This test violates the coding guideline that states "Each test ends with an assertion—there is no code after it." Line 68 executes
res.take(Phi.RHO);which is a side effect that should be part of the test setup, not placed after the variable initialization and before the assertion.
eo-runtime/src/test/java/org/eolang/AtWithRhoTest.java:93 - This test violates the coding guideline that states "Each test ends with an assertion—there is no code after it." Line 93 executes
res.take(Phi.RHO);which is a side effect that should be part of the test setup, not placed after the variable initialization and before the assertion.
eo-runtime/src/main/java/org/eolang/PhOnce.java:17 - The TODO comment contains a typo: "syznchronized" should be "synchronized".
eo-runtime/src/main/java/org/eolang/BytesRaw.java:195 - The removal of parentheses in the bitwise operation changes operator precedence. The original code was
(carry & 0xFF)which is then ANDed with the shifted result. The new codecarry & 0xFFhas the same precedence as the previous AND operation, which may change the evaluation order. Since&has the same precedence and is left-associative, the expressionbytes[source + 1] >>> (Byte.SIZE - mod) & carry & 0xFFwill be evaluated as((bytes[source + 1] >>> (Byte.SIZE - mod)) & carry) & 0xFF, which is equivalent to the original. However, removing the parentheses reduces code clarity and the change appears to be purely cosmetic without functional benefit.
eo-runtime/src/main/java/org/eolang/BytesRaw.java:219 - The removal of parentheses in the bitwise operation changes operator precedence. The original code was
(carry & 0xFF)which is then ANDed with the shifted result. The new codecarry & 0xFFhas the same precedence as the previous AND operation, which may change the evaluation order. Since&has the same precedence and is left-associative, the expressionbytes[source - 1] << (Byte.SIZE - mod) & carry & 0xFFwill be evaluated as((bytes[source - 1] << (Byte.SIZE - mod)) & carry) & 0xFF, which is equivalent to the original. However, removing the parentheses reduces code clarity and the change appears to be purely cosmetic without functional benefit.
eo-runtime/src/test/java/org/eolang/PhDefaultTest.java:108 - The test method name contains a typo: "Afttribute" should be "Attribute". This makes the test name unclear and unprofessional.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
🚀 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
eo-runtime/src/test/java/org/eolang/PhDefaultTest.java (1)
835-845:⚠️ Potential issue | 🟠 MajorFix recursion guard to use the correct counter.
On Line 844, the stop condition checks
PhDefaultTest.RecursivePhi.countwhile this class decrementsPhDefaultTest.RecursivePhiViaNew.counton Line 842. That makes this test helper depend on another class state and can mask recursion/caching regressions.Proposed fix
RecursivePhiViaNew() { this.add( "φ", new AtComposite( this, rho -> { --PhDefaultTest.RecursivePhiViaNew.count; final Phi result; - if (PhDefaultTest.RecursivePhi.count <= 0) { + if (PhDefaultTest.RecursivePhiViaNew.count <= 0) { result = new Data.ToPhi(0L); } else { result = new Data.ToPhi( new Dataized( new RecursivePhiViaNew()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-runtime/src/test/java/org/eolang/PhDefaultTest.java` around lines 835 - 845, The recursion stop condition in the RecursivePhiViaNew test helper is referencing the wrong static counter (PhDefaultTest.RecursivePhi.count) while the code decrements PhDefaultTest.RecursivePhiViaNew.count; change the conditional to check PhDefaultTest.RecursivePhiViaNew.count so the guard uses the same counter that is decremented in the RecursivePhiViaNew constructor (look for the lambda in AtComposite inside RecursivePhiViaNew).eo-runtime/src/main/java/EOorg/EOeolang/EOfs/Files.java (1)
57-65:⚠️ Potential issue | 🟠 Major
open()can leak file descriptors on duplicate opens.At line 59,
putIfAbsentreceives streams created at lines 62-63. Those streams are created eagerly even when the key already exists, and then discarded without closing. Under repeated or concurrentopen(name)calls with the same name, this can exhaust file descriptors.Note: The
@SuppressWarnings("java:S2095")annotation on line 56 indicates this was a known resource warning that was suppressed rather than fixed.💡 Proposed fix
void open(final String name) throws IOException { final Path path = Paths.get(name); - this.streams.putIfAbsent( - name, - new Object[]{ - java.nio.file.Files.newInputStream(path), - java.nio.file.Files.newOutputStream(path, StandardOpenOption.APPEND), - } - ); + final InputStream input = java.nio.file.Files.newInputStream(path); + try { + final OutputStream output = + java.nio.file.Files.newOutputStream(path, StandardOpenOption.APPEND); + final Object[] pair = new Object[]{input, output}; + final Object[] prev = this.streams.putIfAbsent(name, pair); + if (prev != null) { + input.close(); + output.close(); + } + } catch (final IOException err) { + input.close(); + throw err; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-runtime/src/main/java/EOorg/EOeolang/EOfs/Files.java` around lines 57 - 65, The open(String name) method currently creates input/output streams before putIfAbsent, leaking descriptors when the key already exists; change it to avoid eager creation by first checking presence and creating streams only when needed, e.g. perform a containsKey check or synchronized block around create-and-put: if absent, create the two streams and call putIfAbsent(name, createdArray); if putIfAbsent returns a non-null existing value (race), immediately close the newly created streams to avoid leaks; reference the open method and the this.streams.putIfAbsent usage and ensure any created java.nio.file.Files.newInputStream(...) and newOutputStream(...) are closed when not stored.eo-runtime/src/test/java/org/eolang/AtWithRhoTest.java (1)
31-35:⚠️ Potential issue | 🟡 MinorUse
sameInstance()matcher for identity comparisons in tests.Lines 34, 72, and 97 currently use
Matchers.is(obj), which checks equality. These assertions are testing whether the returned object is the same instance as the original (for copy detection). AlthoughPhDefaultoverridesequals()with a hashCode-based comparison, the test semantics are clearly about identity—the test descriptions explicitly state "must copy original object" and "must not copy original object." UsingMatchers.sameInstance(obj)is more semantically correct and protects against future equals/hashCode implementation changes that could break these assertions unexpectedly.Suggested patch
- Matchers.not(Matchers.is(obj)) + Matchers.not(Matchers.sameInstance(obj)) ... - Matchers.is(obj) + Matchers.sameInstance(obj) ... - Matchers.not(Matchers.is(obj)) + Matchers.not(Matchers.sameInstance(obj))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-runtime/src/test/java/org/eolang/AtWithRhoTest.java` around lines 31 - 35, In AtWithRhoTest update the identity assertions that currently use Matchers.is(obj) to use Matchers.sameInstance(obj) (and their negations) so the tests assert instance identity rather than equality; locate the MatcherAssert.assertThat calls that pass Matchers.is(obj) or Matchers.not(Matchers.is(obj)) (variables phi and obj) and replace them with Matchers.sameInstance(obj) and Matchers.not(Matchers.sameInstance(obj)) respectively.eo-runtime/src/main/java/EOorg/EOeolang/EOsm/Posix/RecvSyscall.java (1)
40-47:⚠️ Potential issue | 🔴 CriticalHandle negative
recv()result before copying bytes.Line 47 can throw
NegativeArraySizeExceptionwhenrecv()returns-1, which hides the real syscall failure path.💡 Proposed fix
final int received = CStdLib.INSTANCE.recv( new Dataized(params[0]).asNumber().intValue(), buf, size, new Dataized(params[2]).asNumber().intValue() ); result.put(0, new Data.ToPhi(received)); - result.put(1, new Data.ToPhi(Arrays.copyOf(buf, received))); + result.put( + 1, + new Data.ToPhi(Arrays.copyOf(buf, Math.max(received, 0))) + ); return result;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-runtime/src/main/java/EOorg/EOeolang/EOsm/Posix/RecvSyscall.java` around lines 40 - 47, The code calls CStdLib.INSTANCE.recv and immediately uses Arrays.copyOf(buf, received), which throws NegativeArraySizeException when recv() returns -1; update the RecvSyscall implementation to check the int received from recv() before copying: if received < 0, skip Arrays.copyOf and set result.put(1, new Data.ToPhi(new byte[0])) (or an appropriate empty/NULL Phi value) while leaving result.put(0, new Data.ToPhi(received)) to preserve the syscall error code; only call Arrays.copyOf(buf, received) and set result.put(1, ...) when received >= 0 to avoid hiding the real syscall failure.eo-runtime/pom.xml (1)
386-394:⚠️ Potential issue | 🔴 CriticalVersion 0.25.1 not found in Maven Central—this change will break the build.
The
qulice-maven-pluginversion0.25.1does not appear to exist in Maven Central Repository. The latest available version is0.24.2. The build will fail at dependency resolution when attempting to download this non-existent version. Either use the latest available version (0.24.2) or confirm that0.25.1is available in an alternate repository and add that repository to the POM.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-runtime/pom.xml` around lines 386 - 394, The POM references qulice-maven-plugin with version 0.25.1 which is not available in Maven Central; update the <version> for the qulice-maven-plugin entry to an available release (e.g., 0.24.2) or, if you must use 0.25.1, add the alternate Maven repository that hosts it to the POM so dependency resolution succeeds; locate the qulice-maven-plugin block (artifactId qulice-maven-plugin) and either change its <version> value to 0.24.2 or add the repository configuration that contains 0.25.1.
🧹 Nitpick comments (7)
eo-runtime/src/main/java/org/eolang/Main.java (1)
201-202: Add a brief rationale for both suppressions.At Line 201 and Line 235, consider adding a short inline reason (or Javadoc note) explaining why
PMD.UnnecessaryLocalRuleis intentionally suppressed, so future cleanup is easier.Also applies to: 235-236
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-runtime/src/main/java/org/eolang/Main.java` around lines 201 - 202, Add a short rationale for each PMD suppression near the annotations so future readers know why the local variable suppression is intentional: update the `@SuppressWarnings`("PMD.UnnecessaryLocalRule") on the Main.run(List<String>) method to include a brief inline comment or Javadoc explaining which local variables are kept for clarity/compatibility and why they must remain, and do the same for the second suppression around the other block (the suppression at ~line 235) so it documents the specific local-variable usage being preserved and the reason (readability, debugging, or future refactor constraints). Ensure the comment references the method/block names (e.g., run) so it’s easy to locate.eo-runtime/src/main/java/EOorg/EOeolang/EOtt/EOregex$EOpattern$EOmatch$EOmatched_from_index.java (1)
75-80: Redundant attribute lookup—reusestartvariable.The
startvariable is already retrieved at line 75 but lines 77-79 callthis.take(START)again. Reuse the existing variable:♻️ Suggested refactor
final Phi start = this.take(EOregex$EOpattern$EOmatch$EOmatched_from_index.START); final boolean found = matcher.find( - new Dataized( - this.take(EOregex$EOpattern$EOmatch$EOmatched_from_index.START) - ).asNumber().intValue() + new Dataized(start).asNumber().intValue() );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-runtime/src/main/java/EOorg/EOeolang/EOtt/EOregex`$EOpattern$EOmatch$EOmatched_from_index.java around lines 75 - 80, The code redundantly calls this.take(EOregex$EOpattern$EOmatch$EOmatched_from_index.START) twice; reuse the already obtained local variable start instead: replace the second this.take(...) passed into new Dataized(...) with the existing start variable so matcher.find(new Dataized(start).asNumber().intValue()) is used; ensure the variable name start (from Phi start = this.take(...)) is referenced and no extra this.take call remains.eo-runtime/src/test/java/org/eolang/PhDefaultTest.java (1)
108-108: Rename test method to fix typo (Afttribute→Attribute).Small naming typo in Line 108 makes searchability and suite readability worse.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-runtime/src/test/java/org/eolang/PhDefaultTest.java` at line 108, Rename the test method hasKidWithSetRhoAfterCopyingShouldGetAfttribute in class PhDefaultTest to hasKidWithSetRhoAfterCopyingShouldGetAttribute (fix the typo Afttribute → Attribute) so the test name is correct and searchable; update any references to this method (e.g., in test runners or annotations) to use the new method name.eo-runtime/src/test/java/org/eolang/AtWithRhoTest.java (2)
64-64: Method name contradicts test expectation.Line 64 says
MustCopyOriginalIfRhoIsSet, but the assertion/message verifies the opposite (must not copy). Rename to match behavior.Suggested rename
- void doesNotCopyAndSetRhoIfAlreadySetMustCopyOriginalIfRhoIsSet() { + void doesNotCopyAndSetRhoIfAlreadySetMustNotCopyOriginalIfRhoIsSet() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-runtime/src/test/java/org/eolang/AtWithRhoTest.java` at line 64, Rename the test method whose current name is doesNotCopyAndSetRhoIfAlreadySetMustCopyOriginalIfRhoIsSet in class AtWithRhoTest to match the asserted behavior (it should indicate "must not copy" rather than "must copy"); update the method name to a clear, behavior-accurate identifier (for example doesNotCopyAndSetRhoIfAlreadySet or mustNotCopyOriginalIfRhoIsSet) so the test name aligns with its assertions and messages; ensure any references (e.g., in IDE/run configurations) are updated accordingly.
77-77: Typo in test name (Pho→Rho).Line 77 likely has a typo that makes the test intent harder to scan.
Suggested rename
- void copiesWithNewRhoMustSetNewPhoOnCopy() { + void copiesWithNewRhoMustSetNewRhoOnCopy() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-runtime/src/test/java/org/eolang/AtWithRhoTest.java` at line 77, Rename the test method copiesWithNewRhoMustSetNewPhoOnCopy to copiesWithNewRhoMustSetNewRhoOnCopy (i.e., replace "Pho" with "Rho") so the method name matches its intent; update the method declaration copiesWithNewRhoMustSetNewPhoOnCopy() to copiesWithNewRhoMustSetNewRhoOnCopy() and adjust any references to that method (e.g., calls or annotations) accordingly.eo-runtime/src/main/java/org/eolang/AtOnce.java (1)
48-55: Consider moving the return inside the synchronized block.The
return this.cached.get()on line 54 is outside the synchronized block. WhileAtomicReferenceprovides visibility guarantees making this safe, the pattern is slightly awkward—you're using both synchronization and atomic operations. For the futureReentrantLockrefactor, consider either:
- Using only
AtomicReferencewithcompareAndSet/updateAndGet, or- Keeping the return inside the lock scope
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-runtime/src/main/java/org/eolang/AtOnce.java` around lines 48 - 55, The get() method mixes synchronized(this.cached) with AtomicReference access and returns this.cached.get() outside the synchronized block; move the final return inside the synchronized block to keep the read and potential write consistent within the same lock scope (modify AtOnce.get so the synchronized(this.cached) block contains both the lazy-initialize check/assignment and the return of this.cached.get()); alternatively, if you prefer lock-free code, replace the synchronized block with an AtomicReference-based update using compareAndSet or updateAndGet on cached while reading origin when necessary — reference the AtOnce.get method and the fields cached and origin to locate the change.eo-runtime/src/main/java/EOorg/EOeolang/Heaps.java (1)
37-38: Consider usingMapinterface type instead of suppression.The field only uses standard
Mapmethods (containsKey,get,put,remove). Consider declaring it asMap<Integer, byte[]>to satisfy PMD.LooseCoupling, while keeping theConcurrentHashMapinstantiation for thread-safety documentation.That said, keeping the concrete type makes the thread-safety intention explicit, so the suppression is acceptable if preferred for clarity.
♻️ Optional refactor
- `@SuppressWarnings`("PMD.LooseCoupling") - private final ConcurrentHashMap<Integer, byte[]> blocks; + private final Map<Integer, byte[]> blocks;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-runtime/src/main/java/EOorg/EOeolang/Heaps.java` around lines 37 - 38, The field declaration in class Heaps uses a concrete ConcurrentHashMap type with a PMD suppression; change the declaration type to the Map interface to satisfy PMD.LooseCoupling while keeping the ConcurrentHashMap on instantiation: replace the field type for blocks from ConcurrentHashMap<Integer, byte[]> to Map<Integer, byte[]> (leave the constructor/new ConcurrentHashMap<>() as-is) and remove the `@SuppressWarnings`("PMD.LooseCoupling") annotation; update any direct references to blocks (containsKey, get, put, remove) will continue to work unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@eo-runtime/src/main/java/org/eolang/PhOnce.java`:
- Around line 15-20: The TODO comment in class PhOnce (constructor area)
contains a typo: change "syznchronized" to "synchronized" in the TODO block (the
comment starting with "@todo `#4884`...") — update the text in the PhOnce
constructor comment to fix the spelling while leaving the rest of the note
unchanged.
In `@eo-runtime/src/test/java/EOorg/EOeolang/EOio/InputOutputTest.java`:
- Around line 930-957: In the writesToStdoutViaWindowsWriteFileFunction test the
Dataized instance is constructed but not executed, so the Win32 WriteFile path
never runs; update the lambda passed to InputOutputTest.windowsStdout to dataize
the created object (call .take() on the new Dataized(...) result) so the PhWith
chain (Phi, PhWith, Dataized) actually executes and performs the write; locate
this in the writesToStdoutViaWindowsWriteFileFunction method and ensure the
Dataized created from the PhWith(...).take("code") is immediately dataized
(Dataized(...).take()) before returning from the lambda.
In `@eo-runtime/src/test/java/EOorg/EOeolang/EOnk/EOsocketTest.java`:
- Line 81: Replace the invalid PMD rule name in the `@SuppressWarnings` annotation
used in the EOsocketTest class: find occurrences of
`@SuppressWarnings`({"PMD.UnnecessaryLocalRule",
"PMD.UnitTestContainsTooManyAsserts"}) and change "PMD.UnnecessaryLocalRule" to
"PMD.UnnecessaryLocalBeforeReturn" (or to "VariableCanBeInlined" if targeting
newer PMD); ensure each occurrence in EOsocketTest is updated consistently.
---
Outside diff comments:
In `@eo-runtime/pom.xml`:
- Around line 386-394: The POM references qulice-maven-plugin with version
0.25.1 which is not available in Maven Central; update the <version> for the
qulice-maven-plugin entry to an available release (e.g., 0.24.2) or, if you must
use 0.25.1, add the alternate Maven repository that hosts it to the POM so
dependency resolution succeeds; locate the qulice-maven-plugin block (artifactId
qulice-maven-plugin) and either change its <version> value to 0.24.2 or add the
repository configuration that contains 0.25.1.
In `@eo-runtime/src/main/java/EOorg/EOeolang/EOfs/Files.java`:
- Around line 57-65: The open(String name) method currently creates input/output
streams before putIfAbsent, leaking descriptors when the key already exists;
change it to avoid eager creation by first checking presence and creating
streams only when needed, e.g. perform a containsKey check or synchronized block
around create-and-put: if absent, create the two streams and call
putIfAbsent(name, createdArray); if putIfAbsent returns a non-null existing
value (race), immediately close the newly created streams to avoid leaks;
reference the open method and the this.streams.putIfAbsent usage and ensure any
created java.nio.file.Files.newInputStream(...) and newOutputStream(...) are
closed when not stored.
In `@eo-runtime/src/main/java/EOorg/EOeolang/EOsm/Posix/RecvSyscall.java`:
- Around line 40-47: The code calls CStdLib.INSTANCE.recv and immediately uses
Arrays.copyOf(buf, received), which throws NegativeArraySizeException when
recv() returns -1; update the RecvSyscall implementation to check the int
received from recv() before copying: if received < 0, skip Arrays.copyOf and set
result.put(1, new Data.ToPhi(new byte[0])) (or an appropriate empty/NULL Phi
value) while leaving result.put(0, new Data.ToPhi(received)) to preserve the
syscall error code; only call Arrays.copyOf(buf, received) and set result.put(1,
...) when received >= 0 to avoid hiding the real syscall failure.
In `@eo-runtime/src/test/java/org/eolang/AtWithRhoTest.java`:
- Around line 31-35: In AtWithRhoTest update the identity assertions that
currently use Matchers.is(obj) to use Matchers.sameInstance(obj) (and their
negations) so the tests assert instance identity rather than equality; locate
the MatcherAssert.assertThat calls that pass Matchers.is(obj) or
Matchers.not(Matchers.is(obj)) (variables phi and obj) and replace them with
Matchers.sameInstance(obj) and Matchers.not(Matchers.sameInstance(obj))
respectively.
In `@eo-runtime/src/test/java/org/eolang/PhDefaultTest.java`:
- Around line 835-845: The recursion stop condition in the RecursivePhiViaNew
test helper is referencing the wrong static counter
(PhDefaultTest.RecursivePhi.count) while the code decrements
PhDefaultTest.RecursivePhiViaNew.count; change the conditional to check
PhDefaultTest.RecursivePhiViaNew.count so the guard uses the same counter that
is decremented in the RecursivePhiViaNew constructor (look for the lambda in
AtComposite inside RecursivePhiViaNew).
---
Nitpick comments:
In
`@eo-runtime/src/main/java/EOorg/EOeolang/EOtt/EOregex`$EOpattern$EOmatch$EOmatched_from_index.java:
- Around line 75-80: The code redundantly calls
this.take(EOregex$EOpattern$EOmatch$EOmatched_from_index.START) twice; reuse the
already obtained local variable start instead: replace the second this.take(...)
passed into new Dataized(...) with the existing start variable so
matcher.find(new Dataized(start).asNumber().intValue()) is used; ensure the
variable name start (from Phi start = this.take(...)) is referenced and no extra
this.take call remains.
In `@eo-runtime/src/main/java/EOorg/EOeolang/Heaps.java`:
- Around line 37-38: The field declaration in class Heaps uses a concrete
ConcurrentHashMap type with a PMD suppression; change the declaration type to
the Map interface to satisfy PMD.LooseCoupling while keeping the
ConcurrentHashMap on instantiation: replace the field type for blocks from
ConcurrentHashMap<Integer, byte[]> to Map<Integer, byte[]> (leave the
constructor/new ConcurrentHashMap<>() as-is) and remove the
`@SuppressWarnings`("PMD.LooseCoupling") annotation; update any direct references
to blocks (containsKey, get, put, remove) will continue to work unchanged.
In `@eo-runtime/src/main/java/org/eolang/AtOnce.java`:
- Around line 48-55: The get() method mixes synchronized(this.cached) with
AtomicReference access and returns this.cached.get() outside the synchronized
block; move the final return inside the synchronized block to keep the read and
potential write consistent within the same lock scope (modify AtOnce.get so the
synchronized(this.cached) block contains both the lazy-initialize
check/assignment and the return of this.cached.get()); alternatively, if you
prefer lock-free code, replace the synchronized block with an
AtomicReference-based update using compareAndSet or updateAndGet on cached while
reading origin when necessary — reference the AtOnce.get method and the fields
cached and origin to locate the change.
In `@eo-runtime/src/main/java/org/eolang/Main.java`:
- Around line 201-202: Add a short rationale for each PMD suppression near the
annotations so future readers know why the local variable suppression is
intentional: update the `@SuppressWarnings`("PMD.UnnecessaryLocalRule") on the
Main.run(List<String>) method to include a brief inline comment or Javadoc
explaining which local variables are kept for clarity/compatibility and why they
must remain, and do the same for the second suppression around the other block
(the suppression at ~line 235) so it documents the specific local-variable usage
being preserved and the reason (readability, debugging, or future refactor
constraints). Ensure the comment references the method/block names (e.g., run)
so it’s easy to locate.
In `@eo-runtime/src/test/java/org/eolang/AtWithRhoTest.java`:
- Line 64: Rename the test method whose current name is
doesNotCopyAndSetRhoIfAlreadySetMustCopyOriginalIfRhoIsSet in class
AtWithRhoTest to match the asserted behavior (it should indicate "must not copy"
rather than "must copy"); update the method name to a clear, behavior-accurate
identifier (for example doesNotCopyAndSetRhoIfAlreadySet or
mustNotCopyOriginalIfRhoIsSet) so the test name aligns with its assertions and
messages; ensure any references (e.g., in IDE/run configurations) are updated
accordingly.
- Line 77: Rename the test method copiesWithNewRhoMustSetNewPhoOnCopy to
copiesWithNewRhoMustSetNewRhoOnCopy (i.e., replace "Pho" with "Rho") so the
method name matches its intent; update the method declaration
copiesWithNewRhoMustSetNewPhoOnCopy() to copiesWithNewRhoMustSetNewRhoOnCopy()
and adjust any references to that method (e.g., calls or annotations)
accordingly.
In `@eo-runtime/src/test/java/org/eolang/PhDefaultTest.java`:
- Line 108: Rename the test method
hasKidWithSetRhoAfterCopyingShouldGetAfttribute in class PhDefaultTest to
hasKidWithSetRhoAfterCopyingShouldGetAttribute (fix the typo Afttribute →
Attribute) so the test name is correct and searchable; update any references to
this method (e.g., in test runners or annotations) to use the new method name.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (64)
eo-runtime/pom.xmleo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOslice.javaeo-runtime/src/main/java/EOorg/EOeolang/EOerror.javaeo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOdir$EOwalk.javaeo-runtime/src/main/java/EOorg/EOeolang/EOfs/Files.javaeo-runtime/src/main/java/EOorg/EOeolang/EOmalloc$EOof$EOallocated$EOresized.javaeo-runtime/src/main/java/EOorg/EOeolang/EOms/EOpow.javaeo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOdiv.javaeo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOgt.javaeo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOplus.javaeo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOtimes.javaeo-runtime/src/main/java/EOorg/EOeolang/EOsm/Posix/ReadSyscall.javaeo-runtime/src/main/java/EOorg/EOeolang/EOsm/Posix/RecvSyscall.javaeo-runtime/src/main/java/EOorg/EOeolang/EOsm/SockaddrIn.javaeo-runtime/src/main/java/EOorg/EOeolang/EOsm/Syscall.javaeo-runtime/src/main/java/EOorg/EOeolang/EOsm/Win32/InetAddrFuncCall.javaeo-runtime/src/main/java/EOorg/EOeolang/EOsm/Win32/ReadFileFuncCall.javaeo-runtime/src/main/java/EOorg/EOeolang/EOsm/Win32/RecvFuncCall.javaeo-runtime/src/main/java/EOorg/EOeolang/EOsm/Win32/WinBase.javaeo-runtime/src/main/java/EOorg/EOeolang/EOsm/Win32/WinDef.javaeo-runtime/src/main/java/EOorg/EOeolang/EOsm/Win32/WinNT.javaeo-runtime/src/main/java/EOorg/EOeolang/EOtt/EOregex$EOpattern$EOmatch$EOmatched_from_index.javaeo-runtime/src/main/java/EOorg/EOeolang/EOtt/EOregex$EOφ.javaeo-runtime/src/main/java/EOorg/EOeolang/EOtt/EOsprintf.javaeo-runtime/src/main/java/EOorg/EOeolang/EOtt/EOsscanf.javaeo-runtime/src/main/java/EOorg/EOeolang/Heaps.javaeo-runtime/src/main/java/org/eolang/AtOnce.javaeo-runtime/src/main/java/org/eolang/Atom.javaeo-runtime/src/main/java/org/eolang/AtomSafe.javaeo-runtime/src/main/java/org/eolang/BytesOf.javaeo-runtime/src/main/java/org/eolang/BytesRaw.javaeo-runtime/src/main/java/org/eolang/Data.javaeo-runtime/src/main/java/org/eolang/Dataized.javaeo-runtime/src/main/java/org/eolang/JavaPath.javaeo-runtime/src/main/java/org/eolang/Main.javaeo-runtime/src/main/java/org/eolang/PhCached.javaeo-runtime/src/main/java/org/eolang/PhDefault.javaeo-runtime/src/main/java/org/eolang/PhLogged.javaeo-runtime/src/main/java/org/eolang/PhOnce.javaeo-runtime/src/main/java/org/eolang/PhPackage.javaeo-runtime/src/main/java/org/eolang/PhSafe.javaeo-runtime/src/main/java/org/eolang/VerboseBytesAsString.javaeo-runtime/src/test/java/EOorg/EOeolang/EObytesEOsliceTest.javaeo-runtime/src/test/java/EOorg/EOeolang/EOerrorTest.javaeo-runtime/src/test/java/EOorg/EOeolang/EOfs/FilesTest.javaeo-runtime/src/test/java/EOorg/EOeolang/EOio/InputOutputTest.javaeo-runtime/src/test/java/EOorg/EOeolang/EOmallocTest.javaeo-runtime/src/test/java/EOorg/EOeolang/EOnk/EOsocketTest.javaeo-runtime/src/test/java/EOorg/EOeolang/EOnumberTest.javaeo-runtime/src/test/java/EOorg/EOeolang/EOtryTest.javaeo-runtime/src/test/java/EOorg/EOeolang/HeapsTest.javaeo-runtime/src/test/java/org/eolang/AtCompositeTest.javaeo-runtime/src/test/java/org/eolang/AtOnceTest.javaeo-runtime/src/test/java/org/eolang/AtWithRhoTest.javaeo-runtime/src/test/java/org/eolang/BytesOfTest.javaeo-runtime/src/test/java/org/eolang/DataTest.javaeo-runtime/src/test/java/org/eolang/DataizedTest.javaeo-runtime/src/test/java/org/eolang/ExpectTest.javaeo-runtime/src/test/java/org/eolang/MainTest.javaeo-runtime/src/test/java/org/eolang/PhDefaultTest.javaeo-runtime/src/test/java/org/eolang/PhMethodTest.javaeo-runtime/src/test/java/org/eolang/PhPackageTest.javaeo-runtime/src/test/java/org/eolang/PhSafeTest.javaeo-runtime/src/test/java/org/eolang/PhWithTest.java
💤 Files with no reviewable changes (3)
- eo-runtime/src/main/java/org/eolang/BytesOf.java
- eo-runtime/src/main/java/EOorg/EOeolang/EOms/EOpow.java
- eo-runtime/src/main/java/EOorg/EOeolang/EOtt/EOsprintf.java
Comment on lines
+15
to
20
| * @todo #4884:30min Use ReentrantLock instead of synchronized block in the constructor. | ||
| * This will allow to avoid blocking the whole object while fetching the wrapped one. | ||
| * Moreover, using 'syznchronized' is forbidden by qulice. | ||
| * Don't forget to remove the suppression of PMD.AvoidSynchronizedStatement in | ||
| * the constructor after that. | ||
| * @checkstyle DesignForExtensionCheck (100 lines) |
Contributor
There was a problem hiding this comment.
Typo in TODO comment.
Line 17 has a typo: "syznchronized" should be "synchronized".
✏️ Proposed fix
- * Moreover, using 'syznchronized' is forbidden by qulice.
+ * Moreover, using 'synchronized' is forbidden by qulice.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| * @todo #4884:30min Use ReentrantLock instead of synchronized block in the constructor. | |
| * This will allow to avoid blocking the whole object while fetching the wrapped one. | |
| * Moreover, using 'syznchronized' is forbidden by qulice. | |
| * Don't forget to remove the suppression of PMD.AvoidSynchronizedStatement in | |
| * the constructor after that. | |
| * @checkstyle DesignForExtensionCheck (100 lines) | |
| * `@todo` `#4884`:30min Use ReentrantLock instead of synchronized block in the constructor. | |
| * This will allow to avoid blocking the whole object while fetching the wrapped one. | |
| * Moreover, using 'synchronized' is forbidden by qulice. | |
| * Don't forget to remove the suppression of PMD.AvoidSynchronizedStatement in | |
| * the constructor after that. | |
| * `@checkstyle` DesignForExtensionCheck (100 lines) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@eo-runtime/src/main/java/org/eolang/PhOnce.java` around lines 15 - 20, The
TODO comment in class PhOnce (constructor area) contains a typo: change
"syznchronized" to "synchronized" in the TODO block (the comment starting with
"@todo `#4884`...") — update the text in the PhOnce constructor comment to fix the
spelling while leaving the rest of the note unchanged.
| } | ||
|
|
||
| @Test | ||
| @SuppressWarnings({"PMD.UnnecessaryLocalRule", "PMD.UnitTestContainsTooManyAsserts"}) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
PMD rule "UnnecessaryLocalBeforeReturn" official documentation
💡 Result:
PMD’s official rule documentation for UnnecessaryLocalBeforeReturn (Java, Code Style category) is on the PMD site here: (pmd.github.io)
Key points from the official doc:
- Purpose: “Avoid the creation of unnecessary local variables.” (pmd.github.io)
- Status: Deprecated since PMD 7.17.0; use
VariableCanBeInlinedinstead (covers more cases, incl.throw). (pmd.github.io) - Rule reference (XML):
category/java/codestyle.xml/UnnecessaryLocalBeforeReturn(pmd.github.io) - Property:
statementOrderMatters(defaulttrue). (pmd.github.io)
Citations:
- 1: https://pmd.github.io/pmd/pmd_rules_java_codestyle.html?utm_source=openai
- 2: https://pmd.github.io/pmd/pmd_rules_java_codestyle.html?utm_source=openai
- 3: https://pmd.github.io/pmd/pmd_rules_java_codestyle.html?utm_source=openai
- 4: https://pmd.github.io/pmd/pmd_rules_java_codestyle.html?utm_source=openai
- 5: https://pmd.github.io/pmd/pmd_rules_java_codestyle.html?utm_source=openai
🌐 Web query:
PMD rule "UnnecessaryLocalRule" invalid
💡 Result:
UnnecessaryLocalRule is not a PMD rule name, so if your ruleset references it, PMD will report it as “invalid” because it can’t resolve that rule. In current PMD Java rules, the closest matching rule is UnnecessaryLocalBeforeReturn (and in newer PMD versions it’s deprecated in favor of VariableCanBeInlined). [1]
Fix (recommended for newer PMD 7.x)
Use:
<rule ref="category/java/codestyle.xml/VariableCanBeInlined" />If you must keep the older behavior
Use:
<rule ref="category/java/codestyle.xml/UnnecessaryLocalBeforeReturn" />PMD’s rule names/locations can change across major versions (rules get deprecated/removed), so if you upgraded PMD, update your ruleset references accordingly. [1], [2]
Sources:
[1] PMD Java codestyle rules (shows UnnecessaryLocalBeforeReturn deprecated and replacement VariableCanBeInlined)
[2] PMD rule deprecation policy (explains how rules are deprecated/removed across releases)
Replace invalid PMD rule name PMD.UnnecessaryLocalRule with PMD.UnnecessaryLocalBeforeReturn.
The rule name UnnecessaryLocalRule does not exist in PMD and will not suppress any warnings. The correct rule name is UnnecessaryLocalBeforeReturn. Note that this rule is deprecated since PMD 7.17.0; consider using VariableCanBeInlined instead for newer PMD versions.
This invalid rule name appears at lines 81, 181, 208, 233, 259, 327, 591, and 651 in this file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@eo-runtime/src/test/java/EOorg/EOeolang/EOnk/EOsocketTest.java` at line 81,
Replace the invalid PMD rule name in the `@SuppressWarnings` annotation used in
the EOsocketTest class: find occurrences of
`@SuppressWarnings`({"PMD.UnnecessaryLocalRule",
"PMD.UnitTestContainsTooManyAsserts"}) and change "PMD.UnnecessaryLocalRule" to
"PMD.UnnecessaryLocalBeforeReturn" (or to "VariableCanBeInlined" if targeting
newer PMD); ensure each occurrence in EOsocketTest is updated consistently.
volodya-lombrozo
force-pushed
the
4884-qulice-upgrade
branch
from
1999083 to
890f7bb
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
eo-runtime/src/test/java/org/eolang/AtWithRhoTest.java (1)
64-73:⚠️ Potential issue | 🟡 MinorTest name contradicts assertion/message.
The method name says
MustCopyOriginalIfRhoIsSet, but the assertion/message validate the opposite (must not copy). Please rename to avoid misleading failures and reports.Proposed rename
- void doesNotCopyAndSetRhoIfAlreadySetMustCopyOriginalIfRhoIsSet() { + void doesNotCopyAndSetRhoIfAlreadySetMustNotCopyOriginalIfRhoIsSet() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-runtime/src/test/java/org/eolang/AtWithRhoTest.java` around lines 64 - 73, Rename the confusing test method doesNotCopyAndSetRhoIfAlreadySetMustCopyOriginalIfRhoIsSet in AtWithRhoTest to a name that matches the assertion and message (e.g., doesNotCopyWhenRhoAlreadySet or doesNotCopyAndDoesNotSetRhoIfAlreadySet); update the method declaration and any references so the test name clearly states that AtWithRho must not copy the original when Phi.RHO is already set (the test exercising AtWithRho and AtComposite with PhDefault should keep behavior and assertions unchanged).eo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOslice.java (1)
61-65:⚠️ Potential issue | 🟡 MinorGuard slice end computation from integer overflow.
Line 64 computes
start + lengthdirectly; for large inputs this can overflow and fail unexpectedly inArrays.copyOfRange(...).💡 Proposed fix
return new Data.ToPhi( Arrays.copyOfRange( new Dataized(this.take(Phi.RHO)).take(), - start, start + length + start, Math.addExact(start, length) ) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-runtime/src/main/java/EOorg/EOeolang/EObytes`$EOslice.java around lines 61 - 65, The slice implementation in EObytes$EOslice computes end as start + length which can overflow; change it to compute a safe end by first performing a checked addition (e.g., if (length > Integer.MAX_VALUE - start) set end to Integer.MAX_VALUE else end = start + length), then clamp end to the actual byte array length (end = Math.min(end, bytes.length)), and validate bounds (0 <= start <= end) before calling Arrays.copyOfRange on the byte array obtained from Dataized(this.take(Phi.RHO)).take(); update the method that contains this logic (EObytes$EOslice) to use these guarded computations and throw or handle invalid bounds appropriately.eo-runtime/src/test/java/org/eolang/PhDefaultTest.java (1)
825-858:⚠️ Potential issue | 🟠 MajorFix incorrect counter check in
RecursivePhiViaNew.Line 844 checks
PhDefaultTest.RecursivePhi.countinstead ofPhDefaultTest.RecursivePhiViaNew.count. The class decrements its own counter but checks a different class's counter, breaking the recursion termination logic. This is a copy-paste error from theRecursivePhiclass.Proposed fix
rho -> { --PhDefaultTest.RecursivePhiViaNew.count; final Phi result; - if (PhDefaultTest.RecursivePhi.count <= 0) { + if (PhDefaultTest.RecursivePhiViaNew.count <= 0) { result = new Data.ToPhi(0L); } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-runtime/src/test/java/org/eolang/PhDefaultTest.java` around lines 825 - 858, The recursion termination check in the RecursivePhiViaNew constructor is referencing the wrong counter (PhDefaultTest.RecursivePhi.count) after decrementing PhDefaultTest.RecursivePhiViaNew.count; update the conditional in the AtComposite lambda inside RecursivePhiViaNew to test PhDefaultTest.RecursivePhiViaNew.count <= 0 (matching the decremented static field) so the recursion stops correctly.eo-runtime/pom.xml (1)
385-394:⚠️ Potential issue | 🔴 CriticalVersion inconsistency detected: qulice-maven-plugin upgrade is incomplete across modules.
The pinning of
qulice-maven-pluginto0.25.1in eo-runtime/pom.xml contradicts the root parent pom.xml which still declares version0.25.0. Additionally, eo-integration-tests/pom.xml has no explicit version declaration, so it will inherit the parent's0.25.0. This creates a build environment where:
- eo-parser, eo-maven-plugin, and eo-runtime use version
0.25.1- Root parent and eo-integration-tests use version
0.25.0This version inconsistency undermines build reproducibility rather than ensuring it. Align all modules to the same qulice version (
0.25.1) by updating the root pom.xml parent profile and ensuring eo-integration-tests inherits the correct version.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-runtime/pom.xml` around lines 385 - 394, The project has mixed qulice-maven-plugin versions (0.25.0 in the root parent and 0.25.1 in eo-runtime), so update the root parent POM's qulice-maven-plugin declaration to version 0.25.1 to match eo-runtime and other modules, and ensure eo-integration-tests inherits that version (remove any hard-coded 0.25.0 or add the unified version to the parent/pluginManagement if needed); target the qulice-maven-plugin declaration in the root pom (plugin/pluginManagement or plugin sections) and verify no module (e.g., eo-integration-tests) explicitly pins 0.25.0 so all modules consistently use 0.25.1.
♻️ Duplicate comments (5)
eo-runtime/src/test/java/EOorg/EOeolang/EOnk/EOsocketTest.java (1)
81-81:⚠️ Potential issue | 🟡 MinorInvalid PMD rule name
PMD.UnnecessaryLocalRule.As noted in a previous review,
UnnecessaryLocalRuleis not a valid PMD rule name. The correct name isUnnecessaryLocalBeforeReturn(deprecated since PMD 7.17.0) orVariableCanBeInlinedfor newer PMD versions.This invalid rule name appears at multiple locations in this file (lines 81, 181, 208, 233, 259, 327, 591, 651).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-runtime/src/test/java/EOorg/EOeolang/EOnk/EOsocketTest.java` at line 81, The `@SuppressWarnings` annotations in EOsocketTest.java use an invalid PMD rule name "PMD.UnnecessaryLocalRule"; update each occurrence (e.g., the annotation on the EOsocketTest class/tests) to use a valid PMD rule name such as "PMD.UnnecessaryLocalBeforeReturn" (for older PMD) or "PMD.VariableCanBeInlined" (for newer PMD versions) and keep the existing "PMD.UnitTestContainsTooManyAsserts" entry; apply this change for all occurrences listed in the review (lines referenced in the comment) so the suppressions are valid.eo-runtime/src/test/java/EOorg/EOeolang/EOio/InputOutputTest.java (2)
940-957:⚠️ Potential issue | 🟠 MajorDataized object is created but not executed.
The
Dataizedobject is instantiated but.take()is never called, so the Windows WriteFile syscall may not actually execute. A previous review flagged this same issue, but it appears to remain unfixed.🐛 Proposed fix
new Dataized( new PhWith( new PhWith( Phi.Φ.take("org.eolang.sm.win32").copy(), 0, new Data.ToPhi("WriteFile") ), 1, new Data.ToPhi( new Phi[]{ new Data.ToPhi(Kernel32.STD_OUTPUT_HANDLE), new Data.ToPhi(msg), new Data.ToPhi(msg.length()), } ) ).take("code") - ); + ).take();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-runtime/src/test/java/EOorg/EOeolang/EOio/InputOutputTest.java` around lines 940 - 957, The Dataized instance created around the PhWith/WriteFile invocation is never executed — you need to call its .take(...) to actually run the operation; locate the Dataized(...) expression that wraps PhWith(...).take("code") (the block that builds the WriteFile call using Phi.Φ.take("org.eolang.sm.win32").copy() and PhWith) and invoke .take() on the Dataized result (using the appropriate return type expected by surrounding test code) so the WriteFile syscall is executed during the test.
151-151:⚠️ Potential issue | 🟡 MinorInvalid PMD rule name
PMD.UnnecessaryLocalRule.The rule name
UnnecessaryLocalRuledoes not exist in PMD. UseUnnecessaryLocalBeforeReturnorVariableCanBeInlinedinstead.This same issue appears at lines 203, 239, 293, 857, and 929 in this file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-runtime/src/test/java/EOorg/EOeolang/EOio/InputOutputTest.java` at line 151, Replace the invalid PMD rule name used in the `@SuppressWarnings` annotation: change "PMD.UnnecessaryLocalRule" to a valid rule such as "PMD.UnnecessaryLocalBeforeReturn" or "PMD.VariableCanBeInlined" in the InputOutputTest class where the annotation appears (the several `@SuppressWarnings`("PMD.UnnecessaryLocalRule") occurrences); update each annotation so the PMD suppression references a valid rule name.eo-runtime/src/test/java/EOorg/EOeolang/HeapsTest.java (1)
32-32:⚠️ Potential issue | 🟡 MinorInvalid PMD rule name
PMD.UnnecessaryLocalRule.The rule name
UnnecessaryLocalRuledoes not exist in PMD. This annotation will have no effect. The correct rule name isUnnecessaryLocalBeforeReturn, though note that this rule is deprecated since PMD 7.17.0 in favor ofVariableCanBeInlined.This same issue appears at lines 43, 119, 132, 158, and 199 in this file.
🔧 Proposed fix
- `@SuppressWarnings`("PMD.UnnecessaryLocalRule") + `@SuppressWarnings`("PMD.UnnecessaryLocalBeforeReturn")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-runtime/src/test/java/EOorg/EOeolang/HeapsTest.java` at line 32, Replace the invalid PMD suppression "PMD.UnnecessaryLocalRule" with the correct rule name; update each occurrence in HeapsTest (the `@SuppressWarnings` annotations) to use either "PMD.UnnecessaryLocalBeforeReturn" or the newer "PMD.VariableCanBeInlined" (prefer the latter if using PMD >=7.17.0), ensuring all instances referenced in the review (the annotations around the class/methods in HeapsTest.java) are changed consistently.eo-runtime/src/test/java/org/eolang/PhDefaultTest.java (1)
81-81:⚠️ Potential issue | 🟡 MinorInvalid PMD rule name
PMD.UnnecessaryLocalRule.The rule name
UnnecessaryLocalRuledoes not exist in PMD. UseUnnecessaryLocalBeforeReturnorVariableCanBeInlinedinstead.This same issue appears at lines 134, 177, 330, and 540 in this file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-runtime/src/test/java/org/eolang/PhDefaultTest.java` at line 81, The `@SuppressWarnings` annotation in PhDefaultTest.java uses an invalid PMD rule name "PMD.UnnecessaryLocalRule"; replace that string with a valid rule such as "PMD.UnnecessaryLocalBeforeReturn" or "PMD.VariableCanBeInlined" (choose the rule that matches the intention) in the `@SuppressWarnings` declarations found in PhDefaultTest (the occurrences matching the current annotation at the top and the other similar suppressions in the file).
🧹 Nitpick comments (3)
eo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOplus.java (1)
39-44: Consider using the+operator for consistency.
EOnumber$EOtimesuses the*operator directly, while this usesDouble.sum(). Using+would be more consistent and slightly more readable for direct arithmetic.♻️ Suggested change for consistency
public Phi lambda() { return new Data.ToPhi( - Double.sum( - new Expect.Number(Expect.at(this, Phi.RHO)).it(), - new Expect.Number(Expect.at(this, "x")).it() - ) + new Expect.Number(Expect.at(this, Phi.RHO)).it() + + new Expect.Number(Expect.at(this, "x")).it() ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-runtime/src/main/java/EOorg/EOeolang/EOnumber`$EOplus.java around lines 39 - 44, The implementation of EOnumber$EOplus currently uses Double.sum(...) which is inconsistent with EOnumber$EOtimes using the * operator; replace the Double.sum call with a direct "+" expression between the two primitive double values returned by Expect.Number(Expect.at(this, Phi.RHO)).it() and Expect.Number(Expect.at(this, "x")).it() so the method returns new Data.ToPhi(...first + second...) for readability and consistency.eo-runtime/src/main/java/EOorg/EOeolang/EOtt/EOregex$EOpattern$EOmatch$EOmatched_from_index.java (1)
75-80: Redundantthis.take()call.The
startvariable captured on line 75 is the same value retrieved again on lines 77-79. Consider reusingstartdirectly.♻️ Proposed simplification
final Phi start = this.take(EOregex$EOpattern$EOmatch$EOmatched_from_index.START); final boolean found = matcher.find( - new Dataized( - this.take(EOregex$EOpattern$EOmatch$EOmatched_from_index.START) - ).asNumber().intValue() + new Dataized(start).asNumber().intValue() );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-runtime/src/main/java/EOorg/EOeolang/EOtt/EOregex`$EOpattern$EOmatch$EOmatched_from_index.java around lines 75 - 80, The code redundantly calls this.take(...) twice to get the START value; reuse the already-captured Phi start variable when calling matcher.find by passing new Dataized(start).asNumber().intValue() instead of calling this.take(...) again. Update the matcher.find invocation in EOregex$EOpattern$EOmatch$EOmatched_from_index.java to use start so the START retrieval is not duplicated.eo-runtime/src/main/java/EOorg/EOeolang/EOtt/EOregex$EOφ.java (1)
49-52: Consider using try-with-resources for stream management.The
ObjectOutputStreamis manually closed at line 52, but if an exception occurs at line 50 or 51 (beforeous.close()), the stream won't be properly closed. This is pre-existing behavior, not introduced by this PR, but worth addressing.♻️ Optional refactor using try-with-resources
final ByteArrayOutputStream baos = new ByteArrayOutputStream(); final Phi pattern = regex.take("pattern"); - try { - final ObjectOutputStream ous = new ObjectOutputStream(baos); + try (ObjectOutputStream ous = new ObjectOutputStream(baos)) { ous.writeObject(Pattern.compile(builder.toString())); pattern.put(0, new Data.ToPhi(baos.toByteArray())); - ous.close(); return pattern; } catch (final PatternSyntaxException exception) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-runtime/src/main/java/EOorg/EOeolang/EOtt/EOregex`$EOφ.java around lines 49 - 52, Replace the manual creation and explicit close of ObjectOutputStream in EOorg.EOeolang.EOtt.EOregex$EOφ (the block that creates baos, new ObjectOutputStream(baos), ous.writeObject(...), pattern.put(...), ous.close()) with a try-with-resources that constructs the ObjectOutputStream in the resource header so it is always closed even if writeObject or pattern.put throws; you can keep ByteArrayOutputStream as-is or include it in the try-with-resources as well, but ensure the same serialized bytes are produced and pattern.put(0, new Data.ToPhi(baos.toByteArray())) remains executed after the write.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@eo-runtime/src/test/java/org/eolang/AtWithRhoTest.java`:
- Line 77: Rename the test method copiesWithNewRhoMustSetNewPhoOnCopy in class
AtWithRhoTest to correct the typo (e.g., copiesWithNewRhoMustSetNewRhoOnCopy);
update its declaration and any references or test annotations so the test still
runs and grep/history reflect "Rho" consistently.
---
Outside diff comments:
In `@eo-runtime/pom.xml`:
- Around line 385-394: The project has mixed qulice-maven-plugin versions
(0.25.0 in the root parent and 0.25.1 in eo-runtime), so update the root parent
POM's qulice-maven-plugin declaration to version 0.25.1 to match eo-runtime and
other modules, and ensure eo-integration-tests inherits that version (remove any
hard-coded 0.25.0 or add the unified version to the parent/pluginManagement if
needed); target the qulice-maven-plugin declaration in the root pom
(plugin/pluginManagement or plugin sections) and verify no module (e.g.,
eo-integration-tests) explicitly pins 0.25.0 so all modules consistently use
0.25.1.
In `@eo-runtime/src/main/java/EOorg/EOeolang/EObytes`$EOslice.java:
- Around line 61-65: The slice implementation in EObytes$EOslice computes end as
start + length which can overflow; change it to compute a safe end by first
performing a checked addition (e.g., if (length > Integer.MAX_VALUE - start) set
end to Integer.MAX_VALUE else end = start + length), then clamp end to the
actual byte array length (end = Math.min(end, bytes.length)), and validate
bounds (0 <= start <= end) before calling Arrays.copyOfRange on the byte array
obtained from Dataized(this.take(Phi.RHO)).take(); update the method that
contains this logic (EObytes$EOslice) to use these guarded computations and
throw or handle invalid bounds appropriately.
In `@eo-runtime/src/test/java/org/eolang/AtWithRhoTest.java`:
- Around line 64-73: Rename the confusing test method
doesNotCopyAndSetRhoIfAlreadySetMustCopyOriginalIfRhoIsSet in AtWithRhoTest to a
name that matches the assertion and message (e.g., doesNotCopyWhenRhoAlreadySet
or doesNotCopyAndDoesNotSetRhoIfAlreadySet); update the method declaration and
any references so the test name clearly states that AtWithRho must not copy the
original when Phi.RHO is already set (the test exercising AtWithRho and
AtComposite with PhDefault should keep behavior and assertions unchanged).
In `@eo-runtime/src/test/java/org/eolang/PhDefaultTest.java`:
- Around line 825-858: The recursion termination check in the RecursivePhiViaNew
constructor is referencing the wrong counter (PhDefaultTest.RecursivePhi.count)
after decrementing PhDefaultTest.RecursivePhiViaNew.count; update the
conditional in the AtComposite lambda inside RecursivePhiViaNew to test
PhDefaultTest.RecursivePhiViaNew.count <= 0 (matching the decremented static
field) so the recursion stops correctly.
---
Duplicate comments:
In `@eo-runtime/src/test/java/EOorg/EOeolang/EOio/InputOutputTest.java`:
- Around line 940-957: The Dataized instance created around the PhWith/WriteFile
invocation is never executed — you need to call its .take(...) to actually run
the operation; locate the Dataized(...) expression that wraps
PhWith(...).take("code") (the block that builds the WriteFile call using
Phi.Φ.take("org.eolang.sm.win32").copy() and PhWith) and invoke .take() on the
Dataized result (using the appropriate return type expected by surrounding test
code) so the WriteFile syscall is executed during the test.
- Line 151: Replace the invalid PMD rule name used in the `@SuppressWarnings`
annotation: change "PMD.UnnecessaryLocalRule" to a valid rule such as
"PMD.UnnecessaryLocalBeforeReturn" or "PMD.VariableCanBeInlined" in the
InputOutputTest class where the annotation appears (the several
`@SuppressWarnings`("PMD.UnnecessaryLocalRule") occurrences); update each
annotation so the PMD suppression references a valid rule name.
In `@eo-runtime/src/test/java/EOorg/EOeolang/EOnk/EOsocketTest.java`:
- Line 81: The `@SuppressWarnings` annotations in EOsocketTest.java use an invalid
PMD rule name "PMD.UnnecessaryLocalRule"; update each occurrence (e.g., the
annotation on the EOsocketTest class/tests) to use a valid PMD rule name such as
"PMD.UnnecessaryLocalBeforeReturn" (for older PMD) or "PMD.VariableCanBeInlined"
(for newer PMD versions) and keep the existing
"PMD.UnitTestContainsTooManyAsserts" entry; apply this change for all
occurrences listed in the review (lines referenced in the comment) so the
suppressions are valid.
In `@eo-runtime/src/test/java/EOorg/EOeolang/HeapsTest.java`:
- Line 32: Replace the invalid PMD suppression "PMD.UnnecessaryLocalRule" with
the correct rule name; update each occurrence in HeapsTest (the
`@SuppressWarnings` annotations) to use either "PMD.UnnecessaryLocalBeforeReturn"
or the newer "PMD.VariableCanBeInlined" (prefer the latter if using PMD
>=7.17.0), ensuring all instances referenced in the review (the annotations
around the class/methods in HeapsTest.java) are changed consistently.
In `@eo-runtime/src/test/java/org/eolang/PhDefaultTest.java`:
- Line 81: The `@SuppressWarnings` annotation in PhDefaultTest.java uses an
invalid PMD rule name "PMD.UnnecessaryLocalRule"; replace that string with a
valid rule such as "PMD.UnnecessaryLocalBeforeReturn" or
"PMD.VariableCanBeInlined" (choose the rule that matches the intention) in the
`@SuppressWarnings` declarations found in PhDefaultTest (the occurrences matching
the current annotation at the top and the other similar suppressions in the
file).
---
Nitpick comments:
In `@eo-runtime/src/main/java/EOorg/EOeolang/EOnumber`$EOplus.java:
- Around line 39-44: The implementation of EOnumber$EOplus currently uses
Double.sum(...) which is inconsistent with EOnumber$EOtimes using the *
operator; replace the Double.sum call with a direct "+" expression between the
two primitive double values returned by Expect.Number(Expect.at(this,
Phi.RHO)).it() and Expect.Number(Expect.at(this, "x")).it() so the method
returns new Data.ToPhi(...first + second...) for readability and consistency.
In
`@eo-runtime/src/main/java/EOorg/EOeolang/EOtt/EOregex`$EOpattern$EOmatch$EOmatched_from_index.java:
- Around line 75-80: The code redundantly calls this.take(...) twice to get the
START value; reuse the already-captured Phi start variable when calling
matcher.find by passing new Dataized(start).asNumber().intValue() instead of
calling this.take(...) again. Update the matcher.find invocation in
EOregex$EOpattern$EOmatch$EOmatched_from_index.java to use start so the START
retrieval is not duplicated.
In `@eo-runtime/src/main/java/EOorg/EOeolang/EOtt/EOregex`$EOφ.java:
- Around line 49-52: Replace the manual creation and explicit close of
ObjectOutputStream in EOorg.EOeolang.EOtt.EOregex$EOφ (the block that creates
baos, new ObjectOutputStream(baos), ous.writeObject(...), pattern.put(...),
ous.close()) with a try-with-resources that constructs the ObjectOutputStream in
the resource header so it is always closed even if writeObject or pattern.put
throws; you can keep ByteArrayOutputStream as-is or include it in the
try-with-resources as well, but ensure the same serialized bytes are produced
and pattern.put(0, new Data.ToPhi(baos.toByteArray())) remains executed after
the write.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (64)
eo-runtime/pom.xmleo-runtime/src/main/java/EOorg/EOeolang/EObytes$EOslice.javaeo-runtime/src/main/java/EOorg/EOeolang/EOerror.javaeo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOdir$EOwalk.javaeo-runtime/src/main/java/EOorg/EOeolang/EOfs/Files.javaeo-runtime/src/main/java/EOorg/EOeolang/EOmalloc$EOof$EOallocated$EOresized.javaeo-runtime/src/main/java/EOorg/EOeolang/EOms/EOpow.javaeo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOdiv.javaeo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOgt.javaeo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOplus.javaeo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOtimes.javaeo-runtime/src/main/java/EOorg/EOeolang/EOsm/Posix/ReadSyscall.javaeo-runtime/src/main/java/EOorg/EOeolang/EOsm/Posix/RecvSyscall.javaeo-runtime/src/main/java/EOorg/EOeolang/EOsm/SockaddrIn.javaeo-runtime/src/main/java/EOorg/EOeolang/EOsm/Syscall.javaeo-runtime/src/main/java/EOorg/EOeolang/EOsm/Win32/InetAddrFuncCall.javaeo-runtime/src/main/java/EOorg/EOeolang/EOsm/Win32/ReadFileFuncCall.javaeo-runtime/src/main/java/EOorg/EOeolang/EOsm/Win32/RecvFuncCall.javaeo-runtime/src/main/java/EOorg/EOeolang/EOsm/Win32/WinBase.javaeo-runtime/src/main/java/EOorg/EOeolang/EOsm/Win32/WinDef.javaeo-runtime/src/main/java/EOorg/EOeolang/EOsm/Win32/WinNT.javaeo-runtime/src/main/java/EOorg/EOeolang/EOtt/EOregex$EOpattern$EOmatch$EOmatched_from_index.javaeo-runtime/src/main/java/EOorg/EOeolang/EOtt/EOregex$EOφ.javaeo-runtime/src/main/java/EOorg/EOeolang/EOtt/EOsprintf.javaeo-runtime/src/main/java/EOorg/EOeolang/EOtt/EOsscanf.javaeo-runtime/src/main/java/EOorg/EOeolang/Heaps.javaeo-runtime/src/main/java/org/eolang/AtOnce.javaeo-runtime/src/main/java/org/eolang/Atom.javaeo-runtime/src/main/java/org/eolang/AtomSafe.javaeo-runtime/src/main/java/org/eolang/BytesOf.javaeo-runtime/src/main/java/org/eolang/BytesRaw.javaeo-runtime/src/main/java/org/eolang/Data.javaeo-runtime/src/main/java/org/eolang/Dataized.javaeo-runtime/src/main/java/org/eolang/JavaPath.javaeo-runtime/src/main/java/org/eolang/Main.javaeo-runtime/src/main/java/org/eolang/PhCached.javaeo-runtime/src/main/java/org/eolang/PhDefault.javaeo-runtime/src/main/java/org/eolang/PhLogged.javaeo-runtime/src/main/java/org/eolang/PhOnce.javaeo-runtime/src/main/java/org/eolang/PhPackage.javaeo-runtime/src/main/java/org/eolang/PhSafe.javaeo-runtime/src/main/java/org/eolang/VerboseBytesAsString.javaeo-runtime/src/test/java/EOorg/EOeolang/EObytesEOsliceTest.javaeo-runtime/src/test/java/EOorg/EOeolang/EOerrorTest.javaeo-runtime/src/test/java/EOorg/EOeolang/EOfs/FilesTest.javaeo-runtime/src/test/java/EOorg/EOeolang/EOio/InputOutputTest.javaeo-runtime/src/test/java/EOorg/EOeolang/EOmallocTest.javaeo-runtime/src/test/java/EOorg/EOeolang/EOnk/EOsocketTest.javaeo-runtime/src/test/java/EOorg/EOeolang/EOnumberTest.javaeo-runtime/src/test/java/EOorg/EOeolang/EOtryTest.javaeo-runtime/src/test/java/EOorg/EOeolang/HeapsTest.javaeo-runtime/src/test/java/org/eolang/AtCompositeTest.javaeo-runtime/src/test/java/org/eolang/AtOnceTest.javaeo-runtime/src/test/java/org/eolang/AtWithRhoTest.javaeo-runtime/src/test/java/org/eolang/BytesOfTest.javaeo-runtime/src/test/java/org/eolang/DataTest.javaeo-runtime/src/test/java/org/eolang/DataizedTest.javaeo-runtime/src/test/java/org/eolang/ExpectTest.javaeo-runtime/src/test/java/org/eolang/MainTest.javaeo-runtime/src/test/java/org/eolang/PhDefaultTest.javaeo-runtime/src/test/java/org/eolang/PhMethodTest.javaeo-runtime/src/test/java/org/eolang/PhPackageTest.javaeo-runtime/src/test/java/org/eolang/PhSafeTest.javaeo-runtime/src/test/java/org/eolang/PhWithTest.java
💤 Files with no reviewable changes (3)
- eo-runtime/src/main/java/EOorg/EOeolang/EOtt/EOsprintf.java
- eo-runtime/src/main/java/org/eolang/BytesOf.java
- eo-runtime/src/main/java/EOorg/EOeolang/EOms/EOpow.java
✅ Files skipped from review due to trivial changes (2)
- eo-runtime/src/main/java/EOorg/EOeolang/EOerror.java
- eo-runtime/src/test/java/org/eolang/ExpectTest.java
🚧 Files skipped from review as they are similar to previous changes (26)
- eo-runtime/src/test/java/EOorg/EOeolang/EOnumberTest.java
- eo-runtime/src/test/java/org/eolang/PhPackageTest.java
- eo-runtime/src/main/java/org/eolang/AtomSafe.java
- eo-runtime/src/test/java/org/eolang/MainTest.java
- eo-runtime/src/main/java/EOorg/EOeolang/EOsm/Win32/WinDef.java
- eo-runtime/src/main/java/org/eolang/Main.java
- eo-runtime/src/main/java/EOorg/EOeolang/EOtt/EOsscanf.java
- eo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOdiv.java
- eo-runtime/src/main/java/org/eolang/PhLogged.java
- eo-runtime/src/test/java/org/eolang/AtCompositeTest.java
- eo-runtime/src/main/java/org/eolang/Dataized.java
- eo-runtime/src/test/java/org/eolang/PhWithTest.java
- eo-runtime/src/main/java/EOorg/EOeolang/EOfs/Files.java
- eo-runtime/src/main/java/EOorg/EOeolang/EOsm/Win32/InetAddrFuncCall.java
- eo-runtime/src/main/java/EOorg/EOeolang/EOsm/Win32/WinBase.java
- eo-runtime/src/test/java/org/eolang/PhSafeTest.java
- eo-runtime/src/main/java/EOorg/EOeolang/EOfs/EOdir$EOwalk.java
- eo-runtime/src/main/java/EOorg/EOeolang/EOsm/Win32/RecvFuncCall.java
- eo-runtime/src/main/java/EOorg/EOeolang/EOsm/Posix/ReadSyscall.java
- eo-runtime/src/main/java/org/eolang/PhOnce.java
- eo-runtime/src/main/java/org/eolang/PhCached.java
- eo-runtime/src/test/java/org/eolang/DataizedTest.java
- eo-runtime/src/test/java/EOorg/EOeolang/EOerrorTest.java
- eo-runtime/src/test/java/org/eolang/AtOnceTest.java
- eo-runtime/src/test/java/EOorg/EOeolang/EOfs/FilesTest.java
- eo-runtime/src/main/java/EOorg/EOeolang/EOnumber$EOgt.java
| void copiesWithNewRho() { | ||
| final Phi obj = new PhDefault(); | ||
| final Phi rho = new PhDefault(); | ||
| void copiesWithNewRhoMustSetNewPhoOnCopy() { |
Contributor
There was a problem hiding this comment.
Typo in test name (Pho vs Rho).
Small naming typo makes grep/history harder to follow.
Proposed rename
- void copiesWithNewRhoMustSetNewPhoOnCopy() {
+ void copiesWithNewRhoMustSetNewRhoOnCopy() {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| void copiesWithNewRhoMustSetNewPhoOnCopy() { | |
| void copiesWithNewRhoMustSetNewRhoOnCopy() { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@eo-runtime/src/test/java/org/eolang/AtWithRhoTest.java` at line 77, Rename
the test method copiesWithNewRhoMustSetNewPhoOnCopy in class AtWithRhoTest to
correct the typo (e.g., copiesWithNewRhoMustSetNewRhoOnCopy); update its
declaration and any references or test annotations so the test still runs and
grep/history reflect "Rho" consistently.
Member
Author
|
@yegor256 could you have a look, please? |
|
@volodya-lombrozo Hey there! Nice work on your contribution – you've earned +4 points this time! Here's the breakdown: +16 as the base reward, but we had to deduct -4 for going over 100 hits-of-code (you hit 1375) and another -8 for crossing the 400 threshold. While bigger contributions show dedication, try to keep future ones a bit more focused – smaller, well-crafted changes tend to score better and are easier to review! Your running total is looking great at +385, and don't forget to peek at your Zerocracy account when you get a chance. Keep the contributions flowing! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR upgrades the
quliceversion to0.25.1ineo-runtime.Related to #4884
Summary by CodeRabbit
Chores
Refactor
Tests