build(#4884): upgrade qulice to version 0.25.1 in eo-maven-plugin#4897
Conversation
📝 WalkthroughWalkthroughThis pull request applies widespread code quality refinements across the eo-maven-plugin module. Changes include adding/adjusting PMD suppression annotations, refactoring code to inline variables and remove intermediate assignments, changing type declarations from concrete to more generic types (e.g., ConcurrentHashMap to Map), adding Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
volodya-lombrozo
force-pushed
the
4884-qulice-upgrade
branch
from
03e22f9 to
3becdf3
Compare
🚀 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
|
There was a problem hiding this comment.
Pull request overview
This PR upgrades the qulice-maven-plugin to 0.25.1 for eo-maven-plugin and applies the resulting formatting/refactoring changes across production and test code to satisfy updated static-analysis rules.
Changes:
- Bumped
qulice-maven-pluginversion to0.25.1. - Refactored multiple classes/tests (mostly local-variable inlining, matcher consolidation, and added/adjusted
@SuppressWarnings) to satisfy updated Qulice/PMD checks. - Minor API/annotation tweaks (e.g.,
@FunctionalInterface) and small logic simplifications.
Reviewed changes
Copilot reviewed 63 out of 63 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| eo-maven-plugin/pom.xml | Upgrades qulice-maven-plugin to 0.25.1. |
| eo-maven-plugin/src/main/java/org/eolang/maven/Walk.java | Minor refactor in glob matching implementation. |
| eo-maven-plugin/src/main/java/org/eolang/maven/TjForeign.java | Simplifies equals and adjusts suppressions. |
| eo-maven-plugin/src/main/java/org/eolang/maven/StMeasured.java | Uses explicit UTF-8 when writing measures; updates suppression. |
| eo-maven-plugin/src/main/java/org/eolang/maven/PackageInfos.java | Inlines package name computation into logging/saving flow. |
| eo-maven-plugin/src/main/java/org/eolang/maven/OyRemote.java | Inlines temporary variables in contains/isDirectory. |
| eo-maven-plugin/src/main/java/org/eolang/maven/Objectionary.java | Removes a PMD suppression from Fake. |
| eo-maven-plugin/src/main/java/org/eolang/maven/Moja.java | Expands suppressions for reflection-based field initialization. |
| eo-maven-plugin/src/main/java/org/eolang/maven/MjUnspile.java | Removes/adjusts PMD suppressions; adds suppression on helper. |
| eo-maven-plugin/src/main/java/org/eolang/maven/MjUnplace.java | Removes PMD suppressions; minor cleanup. |
| eo-maven-plugin/src/main/java/org/eolang/maven/MjTranspile.java | Adds suppressions around locals flagged by PMD. |
| eo-maven-plugin/src/main/java/org/eolang/maven/MjSafe.java | Removes a number of PMD.ImmutableField suppressions; minor timeout API cleanup. |
| eo-maven-plugin/src/main/java/org/eolang/maven/MjResolve.java | Adds suppressions for locals; no functional change intended. |
| eo-maven-plugin/src/main/java/org/eolang/maven/MjRegister.java | Inlines threaded registration total into log statement. |
| eo-maven-plugin/src/main/java/org/eolang/maven/MjPull.java | Inlines OyIndexed/OyCached/OyRemote construction; adjusts suppression and local use. |
| eo-maven-plugin/src/main/java/org/eolang/maven/MjProbe.java | Adds suppression for PMD local rule. |
| eo-maven-plugin/src/main/java/org/eolang/maven/MjPrint.java | Inlines program local into Saved(...). |
| eo-maven-plugin/src/main/java/org/eolang/maven/MjParse.java | Adds suppression for PMD local rule. |
| eo-maven-plugin/src/main/java/org/eolang/maven/MjLint.java | Generalizes ConcurrentHashMap type to Map; adds suppression. |
| eo-maven-plugin/src/main/java/org/eolang/maven/MjCompile.java | Adds suppression for PMD local rule. |
| eo-maven-plugin/src/main/java/org/eolang/maven/MjAssemble.java | Adds suppression for PMD local rule. |
| eo-maven-plugin/src/main/java/org/eolang/maven/Footprint.java | Marks interface as @FunctionalInterface. |
| eo-maven-plugin/src/main/java/org/eolang/maven/DpsWithRuntime.java | Uses List<Dep> type; keeps ListOf implementation. |
| eo-maven-plugin/src/main/java/org/eolang/maven/DpsEachWithoutTransitive.java | Refactors transitive filtering into String.join pipeline. |
| eo-maven-plugin/src/main/java/org/eolang/maven/DpsDefault.java | Inlines head/runtime locals into return expression. |
| eo-maven-plugin/src/main/java/org/eolang/maven/Dependencies.java | Marks interface @FunctionalInterface; minor refactor in fake dep generation. |
| eo-maven-plugin/src/main/java/org/eolang/maven/DepDirs.java | Adds suppression for PMD local rule. |
| eo-maven-plugin/src/main/java/org/eolang/maven/ConcurrentCache.java | Adds class suppression for synchronized block + TODO note. |
| eo-maven-plugin/src/main/java/org/eolang/maven/CommitHash.java | Replaces redundant Javadoc with @Override marker. |
| eo-maven-plugin/src/main/java/org/eolang/maven/ChSource.java | Inlines content local into digest loop; adds suppression. |
| eo-maven-plugin/src/main/java/org/eolang/maven/Catalogs.java | Uses Map<Path, Tojos> type; adds suppression + TODO note. |
| eo-maven-plugin/src/main/java/org/eolang/maven/Cache.java | Uses explicit InputStream type; minor digest return simplification. |
| eo-maven-plugin/src/test/java/org/eolang/maven/TjsForeignTest.java | Formatting changes; refactors list creation and exception-message assertion. |
| eo-maven-plugin/src/test/java/org/eolang/maven/ThreadedTest.java | Adds PMD suppression for exception-as-flow-control rule. |
| eo-maven-plugin/src/test/java/org/eolang/maven/ProbesTest.java | Combines size + content assertions via allOf. |
| eo-maven-plugin/src/test/java/org/eolang/maven/PackageInfosTest.java | Splits assertions into separate tests; improves test granularity. |
| eo-maven-plugin/src/test/java/org/eolang/maven/OyRemoteTest.java | Inlines Objectionary creation in assertions. |
| eo-maven-plugin/src/test/java/org/eolang/maven/OyCachedTest.java | Adds suppressions; refactors concurrent test logic. |
| eo-maven-plugin/src/test/java/org/eolang/maven/ObjectsIndexTest.java | Splits behavior checks into separate tests; adds suppressions. |
| eo-maven-plugin/src/test/java/org/eolang/maven/MjUnspileTest.java | Adds suppressions for PMD local rule. |
| eo-maven-plugin/src/test/java/org/eolang/maven/MjUnplaceTest.java | Removes class-level suppression; adds per-test suppressions. |
| eo-maven-plugin/src/test/java/org/eolang/maven/MjTranspileTest.java | Updates suppressions and inlines locals in disabled tests. |
| eo-maven-plugin/src/test/java/org/eolang/maven/MjResolveTest.java | Inlines path building into assertion; adds suppression in one test. |
| eo-maven-plugin/src/test/java/org/eolang/maven/MjRegisterTest.java | Inlines FakeMaven pipeline into assertions. |
| eo-maven-plugin/src/test/java/org/eolang/maven/MjPullTest.java | Consolidates “offline mode” assertions using allOf; adds suppression to one test. |
| eo-maven-plugin/src/test/java/org/eolang/maven/MjProbeTest.java | Removes unnecessary locals by inlining constants. |
| eo-maven-plugin/src/test/java/org/eolang/maven/MjPrintTest.java | Inlines sources path local into configuration. |
| eo-maven-plugin/src/test/java/org/eolang/maven/MjPlaceTest.java | Removes some intermediate assertions and locals; adds suppression. |
| eo-maven-plugin/src/test/java/org/eolang/maven/MjParseTest.java | Adds suppressions and refactors locals; updates concurrency test loop. |
| eo-maven-plugin/src/test/java/org/eolang/maven/MjLintTest.java | Reworks suppressions and per-test PMD settings. |
| eo-maven-plugin/src/test/java/org/eolang/maven/MjCleanTest.java | Refactors directory creation into a single expression. |
| eo-maven-plugin/src/test/java/org/eolang/maven/FpUpdateFromCacheTest.java | Splits behavior into two tests (side-effect vs return value). |
| eo-maven-plugin/src/test/java/org/eolang/maven/FpIfReleasedTest.java | Inlines footprint application into assertion. |
| eo-maven-plugin/src/test/java/org/eolang/maven/FpAppliedWithCacheTest.java | Adds suppressions for PMD local rule. |
| eo-maven-plugin/src/test/java/org/eolang/maven/FileHashTest.java | Inlines path local into assertion. |
| eo-maven-plugin/src/test/java/org/eolang/maven/FakeMaven.java | Removes coupling suppression; inlines locals in defaults and helpers. |
| eo-maven-plugin/src/test/java/org/eolang/maven/DepDirsTest.java | Consolidates multiple assertions into one allOf. |
| eo-maven-plugin/src/test/java/org/eolang/maven/ConcurrentCacheTest.java | Replaces var with explicit types; adds suppression. |
| eo-maven-plugin/src/test/java/org/eolang/maven/CommitHashesMapTest.java | Removes redundant “has key” assertion; directly asserts fetched hash value. |
| eo-maven-plugin/src/test/java/org/eolang/maven/ChTextTest.java | Adds suppressions for PMD local rule. |
| eo-maven-plugin/src/test/java/org/eolang/maven/ChRemoteTest.java | Inlines hash local into assertions. |
| eo-maven-plugin/src/test/java/org/eolang/maven/ChCachedTest.java | Splits cache invariants into separate tests; adds suppressions. |
| eo-maven-plugin/src/test/java/org/eolang/maven/CacheTest.java | Reworks tests to reduce multiple assertions and uses explicit types. |
Comments suppressed due to low confidence (4)
eo-maven-plugin/src/test/java/org/eolang/maven/OyCachedTest.java:88
cachesInConcurrentEnvironmentonly asserts that the origin was called once, but no longer asserts that all threads received the expected cached value (content). This can miss regressions where de-duplication works but the returned value is wrong. Consider asserting both invariants (value equality + single origin call), e.g., via one combined assertion.
eo-maven-plugin/src/test/java/org/eolang/maven/ChCachedTest.java:66- Typo in test method name:
cachesHashAndRemainsConsistenAcrossMultipleCallsshould be...Consistent...for readability/searchability.
eo-maven-plugin/src/main/java/org/eolang/maven/Catalogs.java:28 - Spelling in the TODO:
ReentranLockshould beReentrantLock.
* @todo #4884:30min Use ReentranLock instead of synchronized block in the code.
* It will be more efficient and will not cause deadlocks.
eo-maven-plugin/src/main/java/org/eolang/maven/ConcurrentCache.java:18
- Spelling in the TODO:
Dont' forgetshould beDon't forget.
* This is the suggestion from qulice.
* Dont' forget to remove the PMD suppression for AvoidSynchronizedStatement
* after that.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for (int program = 0; program < total; ++program) { | ||
| MatcherAssert.assertThat( | ||
| "We have to parse concurrently, but we didn't", | ||
| res, | ||
| maven.execute(new FakeMaven.Parse()).result(), | ||
| Matchers.hasKey( |
There was a problem hiding this comment.
The loop in parsesConcurrentlyWithLotsOfPrograms calls maven.execute(new FakeMaven.Parse()).result() on every iteration. That re-runs parsing up to total times (50 here), making the test much slower and potentially changing semantics (each run can mutate state/caches). Consider executing Parse once, storing the result map, and asserting on that single result inside the loop.
| index.contains(object); | ||
| index.contains(object); | ||
| MatcherAssert.assertThat( | ||
| message, | ||
| index.contains(object), | ||
| Matchers.is(true) | ||
| "The number of calls should be 1", | ||
| calls.get(), | ||
| Matchers.is(1) |
There was a problem hiding this comment.
runsContainsWithOnlyOneCallToDecoratedObject ignores the return values of both index.contains(object) calls. The test can pass even if contains returns false; consider asserting the returned value as well (can be combined with the call-count check into a single assertion).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
eo-maven-plugin/src/main/java/org/eolang/maven/DpsEachWithoutTransitive.java (1)
97-102:⚠️ Potential issue | 🟡 MinorPre-existing typo:
"compiled"should be"compile".Line 101 checks for scope
"compiled", which is not a valid Maven dependency scope. The valid scopes arecompile,provided,runtime,test,system, andimport. As written, any dependency with the standard"compile"scope fails this check, soisRuntimeRequiredreturnsfalsefor it — meaning compile-scoped transitive dependencies silently bypass the check initerator()instead of being reported.🐛 Proposed fix
- || "compiled".equals(dep.getScope()); + || "compile".equals(dep.getScope());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-maven-plugin/src/main/java/org/eolang/maven/DpsEachWithoutTransitive.java` around lines 97 - 102, The isRuntimeRequired method currently checks for "compiled" which is a typo; update isRuntimeRequired (and any similar checks) to use the correct Maven scope "compile" instead of "compiled" so compile-scoped dependencies are treated as runtime-required; verify the logic in isRuntimeRequired and the caller iterator() now correctly reports/handles dependencies with scope == null, empty, "runtime" or "compile".eo-maven-plugin/src/main/java/org/eolang/maven/DpsDefault.java (1)
168-178:⚠️ Potential issue | 🟡 MinorBare
.get()on line 175 yields an opaqueNoSuchElementExceptionwhen<part[2]>has no text.The lambda in the outer
.map()calls.text().get()on the second<part>element with no guard. A malformed XMIR where a+rt jvmmeta exists but the coordinate part carries no text content will throwNoSuchElementExceptionwith no diagnostic context (no file path, no element trace).🛡️ Proposed fix — replace bare `.get()` with `orElseThrow`
.map( meta -> meta .elements(Filter.withName("part")) .limit(2) .reduce((first, second) -> second) .get() .text() - .get() + .orElseThrow( + () -> new IllegalStateException( + Logger.format("Missing text in second <part> of +rt meta in %[file]s", file) + ) + ) )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-maven-plugin/src/main/java/org/eolang/maven/DpsDefault.java` around lines 168 - 178, Replace the bare Optional.get() on the second <part> element's text with a guarded orElseThrow that produces a clear diagnostic: locate the expression that calls meta.elements(Filter.withName("part")).limit(2).reduce((first, second) -> second).get().text().get() and change it to explicitly handle missing element/text by using Optional.orElseThrow(...) on both the reduce/get and the text() result, throwing an exception with a message that includes identifying context (e.g., the meta element name/attributes or XMIR coordinate) so failures report which meta entry is malformed instead of raising a NoSuchElementException; use the same symbols Filter.withName("part"), elements(...), reduce(...), text(), and orElseThrow to implement this change.eo-maven-plugin/src/test/java/org/eolang/maven/MjParseTest.java (1)
186-208:⚠️ Potential issue | 🟡 Minor
parsesConcurrentlyWithLotsOfProgramsnow callsexecute()50 times instead of once.Previously the result map was computed once before the assertion loop, making
totalkey-lookups against a single execution. After the refactor,maven.execute(new FakeMaven.Parse())is invoked on every iteration, running a full Maven parse round 50 times sequentially. This significantly increases test execution time and changes what is actually being exercised (re-parsing 50 times vs. one concurrent parse verified 50 times). Reverting to a single precomputed result preserves both performance and the original intent of testing concurrency.🐛 Proposed fix
+ final Map<String, Path> result = maven.execute(new FakeMaven.Parse()).result(); for (int program = 0; program < total; ++program) { MatcherAssert.assertThat( "We have to parse concurrently, but we didn't", - maven.execute(new FakeMaven.Parse()).result(), + result, Matchers.hasKey( String.format( "target/%s/foo/x/main%s.%s", MjParse.DIR, FakeMaven.suffix(program), MjAssemble.XMIR ) ) ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-maven-plugin/src/test/java/org/eolang/maven/MjParseTest.java` around lines 186 - 208, The test parsesConcurrentlyWithLotsOfPrograms currently calls maven.execute(new FakeMaven.Parse()) inside the loop, causing 50 sequential parse runs; change it to run maven.execute(new FakeMaven.Parse()) once before the assertion loop, store its result (e.g., final Map<String, ?> result = maven.execute(new FakeMaven.Parse()).result()), and then use that stored result in the MatcherAssert.assertThat checks (still referencing FakeMaven.suffix(program), MjParse.DIR and MjAssemble.XMIR) so you verify the same single parse output across all iterations without re-executing the parse.
🧹 Nitpick comments (13)
eo-maven-plugin/src/main/java/org/eolang/maven/Catalogs.java (1)
62-62: Widening toMaploses the thread-safety contract at the declaration site.
this.allis initialized as aConcurrentHashMap, so thecomputeIfAbsentcalls at lines 77 and 89 are safe today — JVM dispatch resolves toConcurrentHashMap.computeIfAbsent, not the non-thread-safe default onMap. However, the weakened declared typeMap<Path, Tojos>no longer communicates this requirement. A future refactor replacing the initializer with a plainHashMapwould silently introduce a race condition on those two call sites.Since the TODO on line 27 already signals that this block is under active churn, keeping the stronger type avoids an easy footgun:
♻️ Suggested fix
- private final Map<Path, Tojos> all; + private final ConcurrentHashMap<Path, Tojos> all;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-maven-plugin/src/main/java/org/eolang/maven/Catalogs.java` at line 62, The field declaration weakens the thread-safety contract by typing "all" as Map<Path, Tojos> while it is initialized and used as a concurrent map (computeIfAbsent in class Catalogs); change the declaration to a concurrent type (e.g., ConcurrentMap<Path, Tojos> or ConcurrentHashMap<Path, Tojos>) so the declared API matches the actual thread-safe behavior used by the computeIfAbsent calls and prevents future regressions.eo-maven-plugin/src/main/java/org/eolang/maven/Walk.java (1)
56-57: Consider fixing the coupling root cause instead of suppressing.
PMD.LooseCouplingis almost certainly triggered by the private constructorWalk(Path, List<Path>)accepting a concreteList<Path>rather thanCollection<Path>(orIterable<Path>). Theincludes/excludesmethods are only the call sites of that constructor (viacollect(Collectors.toList())). Suppressing at the call site masks the real issue.Widening the private constructor's parameter to
Collection<Path>eliminates the violation at its source and removes the need for both suppressions:♻️ Proposed refactor
- private Walk(final Path dir, final List<Path> list) { - super(list); + private Walk(final Path dir, final Collection<Path> paths) { + super(new java.util.ArrayList<>(paths)); this.home = dir; }Then drop both
@SuppressWarnings("PMD.LooseCoupling")annotations fromincludesandexcludes.Also applies to: 75-76
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-maven-plugin/src/main/java/org/eolang/maven/Walk.java` around lines 56 - 57, The PMD.LooseCoupling suppression should be removed by widening the private constructor parameter type: change the private constructor Walk(Path, List<Path>) to accept a Collection<Path> (or Iterable<Path>) instead, adjust its internal handling (e.g., copy into an internal List if ordering is required), and update any internal references accordingly; then remove the `@SuppressWarnings`("PMD.LooseCoupling") annotations on the includes and excludes methods (Walk includes(...) and Walk excludes(...)) since the root cause is fixed.eo-maven-plugin/src/main/java/org/eolang/maven/Dependencies.java (1)
114-114: Magic literal3is silently coupled to the array size.
rand.nextInt(3)must stay in sync with the three-element array literal above it. Adding or removing a scope string without updating the bound produces anArrayIndexOutOfBoundsExceptionor silently excludes the new entry.🛠️ Proposed fix — derive the bound from the array itself
+ final String[] scopes = {"test", "compiled", "runtime"}; return Dependencies.Fake.dep( UUID.randomUUID().toString(), UUID.randomUUID().toString(), String.valueOf(rand.nextInt(Integer.MAX_VALUE)), - new String[]{"test", "compiled", "runtime"}[rand.nextInt(3)] + scopes[rand.nextInt(scopes.length)] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-maven-plugin/src/main/java/org/eolang/maven/Dependencies.java` at line 114, Replace the hard-coded bound by deriving it from the scopes array: in Dependencies (the code that builds new String[]{"test","compiled","runtime"}[rand.nextInt(3)]), extract the array into a local variable (e.g., String[] scopes) and call rand.nextInt(scopes.length) and index scopes with that result so the bound always matches the array size and adding/removing scope strings won't break indexing.eo-maven-plugin/src/main/java/org/eolang/maven/MjRegister.java (1)
93-127: ExtractThreaded.total()into a local variable before passing toLogger.info().Embedding substantial side-effectful logic — file registration, hash computation, and exception throwing — directly inside a
Logger.info(...)argument list is atypical and makes the code harder to reason about, trace through a debugger, and test in isolation. A local variable keeps the registration step clearly separated from the logging step.♻️ Proposed refactor
- Logger.info( - this, - "Registered %d EO sources from %[file]s to %[file]s, included %s, excluded %s", - new Threaded<>( - new Walk(this.sourcesDir.toPath()) - .includes(this.includeSources) - .excludes(this.excludeSources), - file -> { - if ( - this.strictFileNames - && !MjRegister.PATTERN.matcher(file.getFileName().toString()).matches() - ) { - throw new IllegalArgumentException( - String.format( - "Incorrect name found: '%s'. EO name must match '%s'", - file.getFileName().toString(), - MjRegister.PATTERN - ) - ); - } - final String name = unplace.make(file); - if (this.scopedTojos().contains(name)) { - Logger.debug(this, "EO source %s already registered", name); - } else { - this.scopedTojos() - .add(name) - .withSource(file.toAbsolutePath()) - .withHash(new ChSource(file)); - Logger.debug(this, "EO source %s registered", name); - } - return 1; - } - ).total(), - this.sourcesDir, - this.foreign, - this.includeSources, - this.excludeSources - ); + final int registered = new Threaded<>( + new Walk(this.sourcesDir.toPath()) + .includes(this.includeSources) + .excludes(this.excludeSources), + file -> { + if ( + this.strictFileNames + && !MjRegister.PATTERN.matcher(file.getFileName().toString()).matches() + ) { + throw new IllegalArgumentException( + String.format( + "Incorrect name found: '%s'. EO name must match '%s'", + file.getFileName().toString(), + MjRegister.PATTERN + ) + ); + } + final String name = unplace.make(file); + if (this.scopedTojos().contains(name)) { + Logger.debug(this, "EO source %s already registered", name); + } else { + this.scopedTojos() + .add(name) + .withSource(file.toAbsolutePath()) + .withHash(new ChSource(file)); + Logger.debug(this, "EO source %s registered", name); + } + return 1; + } + ).total(); + Logger.info( + this, + "Registered %d EO sources from %[file]s to %[file]s, included %s, excluded %s", + registered, + this.sourcesDir, + this.foreign, + this.includeSources, + this.excludeSources + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-maven-plugin/src/main/java/org/eolang/maven/MjRegister.java` around lines 93 - 127, Extract the result of new Threaded<>(new Walk(...).includes(...).excludes(...), file -> { ... }).total() into a local variable (e.g., int total = ... .total()) before calling Logger.info so the side-effectful registration logic inside the Threaded/Walk lambda (which uses MjRegister.PATTERN, unplace.make(file), scopedTojos(), ChSource, etc.) is executed and completed separately from the logging call; then pass that local total variable to Logger.info(), keeping the registration/hash/exception logic out of the Logger.info(...) argument list.eo-maven-plugin/src/main/java/org/eolang/maven/MjLint.java (1)
84-84:Maptype hides the thread-safety contract ofcounts.The
countsmap is accessed concurrently viaThreaded<>→lintOne→linted→counts.compute(...). While the runtime instance is still aConcurrentHashMap(so correctness is unbroken today), widening the declared type toMap<Severity, Integer>silently discards the requirement that this map must be thread-safe. A future refactor substituting any non-concurrentMap(a perfectly validMapimplementation) would cause a data race with no compile-time warning.Consider keeping the declared type as
ConcurrentHashMap<Severity, Integer>(and the private method parameters accordingly), or at minimum add a comment that a concurrent implementation is required.♻️ Proposed change
- final Map<Severity, Integer> counts = new ConcurrentHashMap<>(); + final ConcurrentHashMap<Severity, Integer> counts = new ConcurrentHashMap<>();And restore the parameter types in
lintOneandlintedto match:- private int lintOne( - final TjForeign tojo, - final Map<Severity, Integer> counts, + private int lintOne( + final TjForeign tojo, + final ConcurrentHashMap<Severity, Integer> counts,- private XML linted( - final String program, - final XML xmir, - final Map<Severity, Integer> counts, + private XML linted( + final String program, + final XML xmir, + final ConcurrentHashMap<Severity, Integer> counts,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-maven-plugin/src/main/java/org/eolang/maven/MjLint.java` at line 84, Declaration of counts widens a concurrent requirement by typing it as Map<Severity,Integer>; change the declaration to ConcurrentHashMap<Severity,Integer> (and update any parameter types in methods lintOne and linted that accept this map) so the thread-safety contract is preserved, or at minimum add a clear comment that a concurrent implementation is required; specifically update the field `counts` in class MjLint and the signatures/usages in `lintOne` and `linted` (and any Threaded-related call sites) to reflect `ConcurrentHashMap<Severity,Integer>`.eo-maven-plugin/src/test/java/org/eolang/maven/FakeMaven.java (1)
349-349: Consider extracting the hardcoded EO version to a constant.Line [349] embeds
"0.25.0"directly; centralizing this in aprivate static finalfield would reduce churn across future test updates.♻️ Proposed refactor
final class FakeMaven { + /** + * Default EO object version for test tojos. + */ + private static final String DEFAULT_EO_VERSION = "0.25.0"; @@ this.foreignTojos() .add(object) .withScope(scope) - .withVersion("0.25.0") + .withVersion(FakeMaven.DEFAULT_EO_VERSION) .withSource(src);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-maven-plugin/src/test/java/org/eolang/maven/FakeMaven.java` at line 349, The test hardcodes the EO version string "0.25.0" inline; add a private static final String constant (e.g., EO_VERSION) to the FakeMaven class and replace the inline literal in the .withVersion(...) call with that constant (and update any other occurrences in the same class to use EO_VERSION) so future version bumps require changing a single field.eo-maven-plugin/src/test/java/org/eolang/maven/MjResolveTest.java (1)
232-235: Avoid brittle chained cause dereference in assertion.At Line 235,
getCause().getCause()can make this test fail for the wrong reason. Prefer extracting root cause safely, then assert the message.🛠️ Safer pattern
- MatcherAssert.assertThat( - "Expected that conflicting dependencies were found, but they were not", - Assertions.assertThrows( - IllegalStateException.class, - () -> maven.execute(new FakeMaven.Resolve()) - ).getCause().getCause().getMessage(), + final IllegalStateException thrown = Assertions.assertThrows( + IllegalStateException.class, + () -> maven.execute(new FakeMaven.Resolve()) + ); + Throwable root = thrown; + while (root.getCause() != null) { + root = root.getCause(); + } + MatcherAssert.assertThat( + "Expected that conflicting dependencies were found, but they were not", + root.getMessage(), Matchers.containsString( "1 conflicting dependencies are found: {org.eolang:eo-runtime:jar:=[0.22.0, 0.22.1]}" ) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-maven-plugin/src/test/java/org/eolang/maven/MjResolveTest.java` around lines 232 - 235, The assertion in MjResolveTest uses brittle chained getCause().getCause() on the result of Assertions.assertThrows around maven.execute(new FakeMaven.Resolve()), so change the test to capture the thrown Throwable from Assertions.assertThrows into a variable (e.g., thrown), then safely walk the cause chain (using a loop or utility) to find the root cause or the first non-null cause, and assert that its getMessage() matches the expected message; update the assertion references to use the captured variable (from maven.execute / FakeMaven.Resolve) and ensure you compare the message from the safely-resolved root cause rather than calling getCause().getCause() directly.eo-maven-plugin/src/test/java/org/eolang/maven/OyCachedTest.java (1)
82-82: Assert concurrent call results, not just side effects.At Line 82, the test drops
Together.asList()output. Keeping a value assertion here prevents false positives where call count is1but returned values are inconsistent.✅ Suggested strengthening
- new Together<>(30, thread -> objectionary.get("parallel")).asList(); + MatcherAssert.assertThat( + "All concurrent reads should return the same cached content", + new Together<>(30, thread -> objectionary.get("parallel")).asList(), + Matchers.everyItem(Matchers.equalTo(content)) + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-maven-plugin/src/test/java/org/eolang/maven/OyCachedTest.java` at line 82, The test currently discards the result of new Together<>(30, thread -> objectionary.get("parallel")).asList(), so update OyCachedTest to capture that returned List from Together.asList() and add assertions against its contents (e.g., size equals expected concurrency and all elements match the expected value produced by objectionary.get("parallel")) instead of only checking side effects; locate the Together instantiation and replace the dropped call with a variable (List<?> results = new Together<>(30, thread -> objectionary.get("parallel")).asList()) and add assertions that verify the list length and that each element equals the expected returned value.eo-maven-plugin/src/test/java/org/eolang/maven/ChCachedTest.java (1)
64-79: Simplify single-expression block lambda.Lines 68–70 use a block body
() -> { return "dummy"; }for a trivial constant return, consistent with the same nit inObjectsIndexTest.java.♻️ Proposed simplification
- final ChCached cached = new ChCached( - () -> { - return "dummy"; - } - ); + final ChCached cached = new ChCached(() -> "dummy");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-maven-plugin/src/test/java/org/eolang/maven/ChCachedTest.java` around lines 64 - 79, The test uses a block lambda with an explicit return in cachesHashAndRemainsConsistentAcrossMultipleCalls; simplify the lambda passed to the ChCached constructor to a single-expression lambda by replacing () -> { return "dummy"; } with () -> "dummy" to make the code concise and consistent with other tests like ObjectsIndexTest.eo-maven-plugin/src/test/java/org/eolang/maven/ObjectsIndexTest.java (1)
46-60: Simplify single-expression block lambdas.The lambdas on lines 53–55 and 68–70 use a block body for a trivial return expression. They can be simplified to expression lambdas.
♻️ Proposed simplification
-new ScalarOf<>( - () -> { - return Collections.singleton(object); - } -) +new ScalarOf<>(() -> Collections.singleton(object))-new ScalarOf<>( - () -> { - return Collections.singleton("org.eolang.io.stderr"); - } -) +new ScalarOf<>(() -> Collections.singleton("org.eolang.io.stderr"))Also applies to: 62-75
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-maven-plugin/src/test/java/org/eolang/maven/ObjectsIndexTest.java` around lines 46 - 60, In ObjectsIndexTest (method runsContainsSuccessfully) replace the block lambda that returns a single expression (currently "() -> { return Collections.singleton(object); }") with an expression lambda "() -> Collections.singleton(object)"; do the same for the other similar block lambda referenced around lines 62–75 so both ScalarOf<> suppliers use the concise expression lambda form.eo-maven-plugin/src/main/java/org/eolang/maven/MjSafe.java (1)
634-667: Auto-unboxingInteger timeoutis functionally equivalent but suppresses an NPE signal.
this.timeoutis a boxedInteger. The previous.longValue()call was explicit; the new code silently auto-unboxes it. Both paths NPE iftimeoutisnull(e.g., Maven injects an explicit<timeout/>with no value). No regression is introduced since the field has a= Integer.MAX_VALUEdefault, but consider adding a null-guard or usingintas the field type to make the NPE impossible at the declaration site.♻️ Proposed change
- `@Parameter`(property = "eo.timeout") - protected Integer timeout = Integer.MAX_VALUE; + `@Parameter`(property = "eo.timeout") + protected int timeout = Integer.MAX_VALUE;Switching to primitive
inteliminates the boxing entirely and makes the auto-widening tolonginFuture.get()andTimeUnit.toMillis()both explicit and NPE-safe.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-maven-plugin/src/main/java/org/eolang/maven/MjSafe.java` around lines 634 - 667, The execWithTimeout method currently uses the boxed Integer field timeout which can be null and silently auto-unboxes (e.g., in get(this.timeout, TimeUnit.SECONDS) and TimeUnit.SECONDS.toMillis(this.timeout)), so change the timeout field to a primitive int (or add an explicit null-guard that throws a clear exception) to avoid NPEs: update the declaration of timeout to int (or check this.timeout == null and throw IllegalStateException), and adjust any places that relied on Integer methods (e.g., in execWithTimeout and any other uses) so calls use the primitive value (auto-widening to long where needed) to make the behavior NPE-safe; target symbols: field timeout and method execWithTimeout.eo-maven-plugin/src/test/java/org/eolang/maven/CacheTest.java (2)
57-62: Combined&&in assertion hides which existence check fails.When this assertion fails, the output reports
expected: true but was: falsewith no indication of whether the cache file or the.sha256file is the culprit. Splitting into two independentassertThatcalls removes the short-circuit ambiguity and makes failures self-describing.♻️ Proposed refactor
- MatcherAssert.assertThat( - "Cache file must be created and hash file must be created", - Files.exists(base.resolve(tail)) - && Files.exists(base.resolve(String.format("%s.sha256", tail))), - Matchers.is(true) - ); + MatcherAssert.assertThat( + "Cache file must be created", + Files.exists(base.resolve(tail)), + Matchers.is(true) + ); + MatcherAssert.assertThat( + "Hash file must be created", + Files.exists(base.resolve(String.format("%s.sha256", tail))), + Matchers.is(true) + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-maven-plugin/src/test/java/org/eolang/maven/CacheTest.java` around lines 57 - 62, The combined boolean in the assertion hides which file is missing; change the single MatcherAssert.assertThat call into two separate assertions: one asserting Files.exists(base.resolve(tail)) with a message like "Cache file must be created", and another asserting Files.exists(base.resolve(String.format("%s.sha256", tail))) with a message like "Hash file must be created" so failures pinpoint whether the cache file (variable tail) or its .sha256 sidecar is absent (in CacheTest.java where base and tail are used).
157-212:generatesCorrectHashForLargeFileandgeneratesCorrectHashForTinyFileare near-duplicates.The two tests differ only in the source file name and content, but share all setup, assertion, and teardown logic. They are good candidates for a
@ParameterizedTestto avoid the duplication.♻️ Proposed refactor (optional)
- `@Test` - void generatesCorrectHashForLargeFile( - `@Mktmp` final Path temp - ) throws IOException, NoSuchAlgorithmException { - ... - } - - `@Test` - void generatesCorrectHashForTinyFile( - `@Mktmp` final Path temp - ) throws IOException, NoSuchAlgorithmException { - ... - } + `@ParameterizedTest` + `@MethodSource`("hashTestCases") + void generatesCorrectHashForContent( + final String content, final String filename, `@Mktmp` final Path temp + ) throws IOException, NoSuchAlgorithmException { + final Path cache = temp.resolve("cache"); + Files.createDirectories(cache); + final Path source = temp.resolve(filename); + Files.writeString(source, content, StandardCharsets.UTF_8); + final Path tail = source.getFileName(); + new Cache(cache, p -> content).apply(source, temp.resolve("out.txt"), tail); + MatcherAssert.assertThat( + "SHA-256 hash file has incorrect content", + Files.readString(cache.resolve(String.format("%s.sha256", tail)), StandardCharsets.UTF_8), + Matchers.equalTo( + Base64.getEncoder().encodeToString( + MessageDigest.getInstance("SHA-256").digest(content.getBytes(StandardCharsets.UTF_8)) + ) + ) + ); + } + + static Stream<Arguments> hashTestCases() { + final int lines = 100_000; + final StringBuilder builder = new StringBuilder(lines * 10); + IntStream.range(0, lines).forEach(i -> builder.append("Line ").append(i).append('\n')); + return Stream.of( + Arguments.of(builder.toString(), "large.txt"), + Arguments.of("x", "tiny.txt") + ); + }Note:
@Mktmpinjection with@ParameterizedTestneeds to be verified against howMktmpResolversupports parameter injection in that context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-maven-plugin/src/test/java/org/eolang/maven/CacheTest.java` around lines 157 - 212, Replace the two near-duplicate tests generatesCorrectHashForLargeFile and generatesCorrectHashForTinyFile with a single `@ParameterizedTest` in CacheTest that takes parameters (fileName, content) via `@CsvSource` or `@MethodSource` and runs the shared setup/assertion logic once: create cache dir, write content to temp file, compute tail, call new Cache(cache, p -> content).apply(...), then assert the .sha256 file equals Base64(SHA-256(content)). Ensure the parameterized test can still receive the temp Path: if `@Mktmp` injection is incompatible with `@ParameterizedTest`, switch to using JUnit's `@TempDir` or manually create a temp directory inside the test; keep method name and references to Cache.apply, tail, and the SHA-256 assertion so reviewers can locate the logic.
🤖 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-maven-plugin/src/main/java/org/eolang/maven/Catalogs.java`:
- Line 27: Fix the typo in the TODO comment inside Catalogs (change
"ReentranLock" to "ReentrantLock") so the referenced class name is correct;
update the TODO text in the Catalogs.java comment to "ReentrantLock" and, if
present elsewhere in comments, ensure consistent spelling to avoid confusion
when implementing the actual replacement of the synchronized block.
In `@eo-maven-plugin/src/main/java/org/eolang/maven/ConcurrentCache.java`:
- Around line 15-18: In the Javadoc TODO comment inside the ConcurrentCache
class update the typo "Dont'" to "Don't" so the maintenance note reads
correctly; locate the TODO block that starts with "@todo `#4884`:30min Use
ReentrantLock instead of synchronized block." and replace the malformed
apostrophe in the word Dont' with the proper contraction Don't (no other logic
changes required).
In `@eo-maven-plugin/src/main/java/org/eolang/maven/Dependencies.java`:
- Around line 20-21: Remove the misleading `@FunctionalInterface` annotation from
the Dependencies interface declaration (interface Dependencies extends
Iterable<Dep>) since the project only uses concrete implementations (e.g.,
DpsDefault, DpsWithRuntime, DpsDepgraph, Fake) and does not rely on lambda
construction; then replace the magic number usage in the random scope selection
(currently new String[]{"test","compiled","runtime"}[rand.nextInt(3)]) by
referencing the scopes array length (e.g., use a local scopes String[] and call
rand.nextInt(scopes.length)) or a named constant for the size so the index bound
stays in sync with the array.
In `@eo-maven-plugin/src/main/java/org/eolang/maven/MjLint.java`:
- Line 80: Remove the ineffective suppression annotation
`@SuppressWarnings`("PMD.UnnecessaryLocalRule") from the MjLint class (and
similarly from other files where it was added); locate the annotation near the
lint() method and delete it entirely since the rule name is invalid and the
applicable PMD rule UnnecessaryLocalBeforeReturn cannot apply to a void method
like lint(), so no suppression is needed.
In `@eo-maven-plugin/src/main/java/org/eolang/maven/MjProbe.java`:
- Line 55: The `@SuppressWarnings`("PMD.UnnecessaryLocalRule") annotation in
MjProbe.java is using a non-existent PMD rule; open the MjProbe class and locate
the annotation on the probe() method and either remove the `@SuppressWarnings`
entry entirely (preferred since probe() is void and locals are used) or change
the rule name to the correct PMD identifier
"@SuppressWarnings(\"PMD.UnnecessaryLocalBeforeReturn\")" if you truly intend to
suppress that specific check; update the annotation text accordingly so it
references a valid PMD rule or delete the annotation.
In `@eo-maven-plugin/src/main/java/org/eolang/maven/MjRegister.java`:
- Around line 93-127: The walker lambda currently returns 1 for both skipped and
newly-registered files so Threaded.total() overcounts and any
IllegalArgumentException thrown inside gets wrapped by Threaded.total(); fix by
returning 1 only when a file was newly registered and 0 when already present
(change the lambda return at the "already registered" branch to 0), and wrap the
Threaded.total() invocation call in a try/catch that catches
IllegalStateException (thrown by Threaded.total()) and, if its cause is an
instance of IllegalArgumentException, rethrow that cause to preserve the
original exception type; reference symbols: the lambda inside new
Threaded<>(...), MjRegister.PATTERN check, scopedTojos(), unplace.make(...), and
Threaded.total().
- Around line 93-127: The Threaded class currently wraps all thrown exceptions
into IllegalStateException in Threaded.total()/total() (and its try-catch around
the lambda), which hides intended exceptions like IllegalArgumentException
thrown from the MjRegister lambda; change the catch path in Threaded.total() so
it rethrows runtime exceptions (or at least IllegalArgumentException) unchanged
instead of wrapping them—e.g., in the catch block, if the caught Throwable is an
instance of RuntimeException (or specifically IllegalArgumentException) rethrow
it, otherwise wrap it in IllegalStateException—so exceptions from the lambda
(such as the IllegalArgumentException from MjRegister's file-name check)
propagate with their original type.
In `@eo-maven-plugin/src/main/java/org/eolang/maven/MjTranspile.java`:
- Line 125: The three `@SuppressWarnings` annotations using the invalid PMD rule
name "PMD.UnnecessaryLocalRule" (found in MjTranspile at the three annotated
locations) must be corrected so the suppressions actually work; replace each
occurrence of `@SuppressWarnings`("PMD.UnnecessaryLocalRule") with
`@SuppressWarnings`("PMD.UnnecessaryLocalBeforeReturn") (the valid PMD rule name)
for the annotations at the three spots (around lines with the MjTranspile
methods/blocks) to ensure PMD/Qulice will recognize and honor the suppression.
In `@eo-maven-plugin/src/main/java/org/eolang/maven/StMeasured.java`:
- Line 48: The `@SuppressWarnings`("PMD.UnnecessaryLocalRule") annotation is
invalid; update it to a correct PMD rule name (either
"PMD.UnnecessaryLocalBeforeReturn" for legacy PMD or "PMD.VariableCanBeInlined"
for newer PMD) on the `@SuppressWarnings` annotation in StMeasured (and replace
all other occurrences across the codebase), e.g., change the string literal in
the `@SuppressWarnings`(...) on the class/method where StMeasured is defined to
the chosen valid rule so PMD suppression actually takes effect.
In `@eo-maven-plugin/src/test/java/org/eolang/maven/CacheTest.java`:
- Line 32: Remove all four `@SuppressWarnings`("PMD.UnnecessaryLocalRule")
annotations from the CacheTest class (they are invalid and unnecessary) and
leave the tests unchanged otherwise; the local variables base, source, target,
counter, cache and tail are legitimately used multiple times so no suppression
is needed. Also replace the combined boolean assertion that checks two
conditions at once (the assertion covering two predicates on cache/state) with
two separate assertions/matchers so each condition is asserted independently
(e.g., assertThat(...) for the first predicate and assertThat(...) for the
second) to improve failure diagnostics.
In `@eo-maven-plugin/src/test/java/org/eolang/maven/MjCleanTest.java`:
- Around line 30-32: In MjCleanTest replace the incorrect use of
Files.createDirectories that creates a directory named "child.eo" (the variable
small) with code that ensures the "child" directory exists and then creates an
actual file "child.eo" (e.g. call Files.createDirectories(dir.resolve("child"))
then Files.createFile(dir.resolve("child").resolve("child.eo")) or equivalent);
update the variable small to refer to the created file path so the test truly
exercises cleaning a .eo file rather than a directory.
In `@eo-maven-plugin/src/test/java/org/eolang/maven/MjTranspileTest.java`:
- Line 220: The suppression annotation
`@SuppressWarnings`("PMD.UnnecessaryLocalRule") is invalid; replace it with a
recognized PMD rule such as "PMD.UnnecessaryLocalBeforeReturn" or the newer
"PMD.VariableCanBeInlined" in MjTranspileTest (and the other ~50 occurrences) so
the suppression takes effect; update the string in the `@SuppressWarnings`
annotations (e.g., in the class/method where MjTranspileTest contains that
annotation) across the codebase consistently to the chosen valid rule name.
---
Outside diff comments:
In `@eo-maven-plugin/src/main/java/org/eolang/maven/DpsDefault.java`:
- Around line 168-178: Replace the bare Optional.get() on the second <part>
element's text with a guarded orElseThrow that produces a clear diagnostic:
locate the expression that calls
meta.elements(Filter.withName("part")).limit(2).reduce((first, second) ->
second).get().text().get() and change it to explicitly handle missing
element/text by using Optional.orElseThrow(...) on both the reduce/get and the
text() result, throwing an exception with a message that includes identifying
context (e.g., the meta element name/attributes or XMIR coordinate) so failures
report which meta entry is malformed instead of raising a
NoSuchElementException; use the same symbols Filter.withName("part"),
elements(...), reduce(...), text(), and orElseThrow to implement this change.
In
`@eo-maven-plugin/src/main/java/org/eolang/maven/DpsEachWithoutTransitive.java`:
- Around line 97-102: The isRuntimeRequired method currently checks for
"compiled" which is a typo; update isRuntimeRequired (and any similar checks) to
use the correct Maven scope "compile" instead of "compiled" so compile-scoped
dependencies are treated as runtime-required; verify the logic in
isRuntimeRequired and the caller iterator() now correctly reports/handles
dependencies with scope == null, empty, "runtime" or "compile".
In `@eo-maven-plugin/src/test/java/org/eolang/maven/MjParseTest.java`:
- Around line 186-208: The test parsesConcurrentlyWithLotsOfPrograms currently
calls maven.execute(new FakeMaven.Parse()) inside the loop, causing 50
sequential parse runs; change it to run maven.execute(new FakeMaven.Parse())
once before the assertion loop, store its result (e.g., final Map<String, ?>
result = maven.execute(new FakeMaven.Parse()).result()), and then use that
stored result in the MatcherAssert.assertThat checks (still referencing
FakeMaven.suffix(program), MjParse.DIR and MjAssemble.XMIR) so you verify the
same single parse output across all iterations without re-executing the parse.
---
Nitpick comments:
In `@eo-maven-plugin/src/main/java/org/eolang/maven/Catalogs.java`:
- Line 62: The field declaration weakens the thread-safety contract by typing
"all" as Map<Path, Tojos> while it is initialized and used as a concurrent map
(computeIfAbsent in class Catalogs); change the declaration to a concurrent type
(e.g., ConcurrentMap<Path, Tojos> or ConcurrentHashMap<Path, Tojos>) so the
declared API matches the actual thread-safe behavior used by the computeIfAbsent
calls and prevents future regressions.
In `@eo-maven-plugin/src/main/java/org/eolang/maven/Dependencies.java`:
- Line 114: Replace the hard-coded bound by deriving it from the scopes array:
in Dependencies (the code that builds new
String[]{"test","compiled","runtime"}[rand.nextInt(3)]), extract the array into
a local variable (e.g., String[] scopes) and call rand.nextInt(scopes.length)
and index scopes with that result so the bound always matches the array size and
adding/removing scope strings won't break indexing.
In `@eo-maven-plugin/src/main/java/org/eolang/maven/MjLint.java`:
- Line 84: Declaration of counts widens a concurrent requirement by typing it as
Map<Severity,Integer>; change the declaration to
ConcurrentHashMap<Severity,Integer> (and update any parameter types in methods
lintOne and linted that accept this map) so the thread-safety contract is
preserved, or at minimum add a clear comment that a concurrent implementation is
required; specifically update the field `counts` in class MjLint and the
signatures/usages in `lintOne` and `linted` (and any Threaded-related call
sites) to reflect `ConcurrentHashMap<Severity,Integer>`.
In `@eo-maven-plugin/src/main/java/org/eolang/maven/MjRegister.java`:
- Around line 93-127: Extract the result of new Threaded<>(new
Walk(...).includes(...).excludes(...), file -> { ... }).total() into a local
variable (e.g., int total = ... .total()) before calling Logger.info so the
side-effectful registration logic inside the Threaded/Walk lambda (which uses
MjRegister.PATTERN, unplace.make(file), scopedTojos(), ChSource, etc.) is
executed and completed separately from the logging call; then pass that local
total variable to Logger.info(), keeping the registration/hash/exception logic
out of the Logger.info(...) argument list.
In `@eo-maven-plugin/src/main/java/org/eolang/maven/MjSafe.java`:
- Around line 634-667: The execWithTimeout method currently uses the boxed
Integer field timeout which can be null and silently auto-unboxes (e.g., in
get(this.timeout, TimeUnit.SECONDS) and
TimeUnit.SECONDS.toMillis(this.timeout)), so change the timeout field to a
primitive int (or add an explicit null-guard that throws a clear exception) to
avoid NPEs: update the declaration of timeout to int (or check this.timeout ==
null and throw IllegalStateException), and adjust any places that relied on
Integer methods (e.g., in execWithTimeout and any other uses) so calls use the
primitive value (auto-widening to long where needed) to make the behavior
NPE-safe; target symbols: field timeout and method execWithTimeout.
In `@eo-maven-plugin/src/main/java/org/eolang/maven/Walk.java`:
- Around line 56-57: The PMD.LooseCoupling suppression should be removed by
widening the private constructor parameter type: change the private constructor
Walk(Path, List<Path>) to accept a Collection<Path> (or Iterable<Path>) instead,
adjust its internal handling (e.g., copy into an internal List if ordering is
required), and update any internal references accordingly; then remove the
`@SuppressWarnings`("PMD.LooseCoupling") annotations on the includes and excludes
methods (Walk includes(...) and Walk excludes(...)) since the root cause is
fixed.
In `@eo-maven-plugin/src/test/java/org/eolang/maven/CacheTest.java`:
- Around line 57-62: The combined boolean in the assertion hides which file is
missing; change the single MatcherAssert.assertThat call into two separate
assertions: one asserting Files.exists(base.resolve(tail)) with a message like
"Cache file must be created", and another asserting
Files.exists(base.resolve(String.format("%s.sha256", tail))) with a message like
"Hash file must be created" so failures pinpoint whether the cache file
(variable tail) or its .sha256 sidecar is absent (in CacheTest.java where base
and tail are used).
- Around line 157-212: Replace the two near-duplicate tests
generatesCorrectHashForLargeFile and generatesCorrectHashForTinyFile with a
single `@ParameterizedTest` in CacheTest that takes parameters (fileName, content)
via `@CsvSource` or `@MethodSource` and runs the shared setup/assertion logic once:
create cache dir, write content to temp file, compute tail, call new
Cache(cache, p -> content).apply(...), then assert the .sha256 file equals
Base64(SHA-256(content)). Ensure the parameterized test can still receive the
temp Path: if `@Mktmp` injection is incompatible with `@ParameterizedTest`, switch
to using JUnit's `@TempDir` or manually create a temp directory inside the test;
keep method name and references to Cache.apply, tail, and the SHA-256 assertion
so reviewers can locate the logic.
In `@eo-maven-plugin/src/test/java/org/eolang/maven/ChCachedTest.java`:
- Around line 64-79: The test uses a block lambda with an explicit return in
cachesHashAndRemainsConsistentAcrossMultipleCalls; simplify the lambda passed to
the ChCached constructor to a single-expression lambda by replacing () -> {
return "dummy"; } with () -> "dummy" to make the code concise and consistent
with other tests like ObjectsIndexTest.
In `@eo-maven-plugin/src/test/java/org/eolang/maven/FakeMaven.java`:
- Line 349: The test hardcodes the EO version string "0.25.0" inline; add a
private static final String constant (e.g., EO_VERSION) to the FakeMaven class
and replace the inline literal in the .withVersion(...) call with that constant
(and update any other occurrences in the same class to use EO_VERSION) so future
version bumps require changing a single field.
In `@eo-maven-plugin/src/test/java/org/eolang/maven/MjResolveTest.java`:
- Around line 232-235: The assertion in MjResolveTest uses brittle chained
getCause().getCause() on the result of Assertions.assertThrows around
maven.execute(new FakeMaven.Resolve()), so change the test to capture the thrown
Throwable from Assertions.assertThrows into a variable (e.g., thrown), then
safely walk the cause chain (using a loop or utility) to find the root cause or
the first non-null cause, and assert that its getMessage() matches the expected
message; update the assertion references to use the captured variable (from
maven.execute / FakeMaven.Resolve) and ensure you compare the message from the
safely-resolved root cause rather than calling getCause().getCause() directly.
In `@eo-maven-plugin/src/test/java/org/eolang/maven/ObjectsIndexTest.java`:
- Around line 46-60: In ObjectsIndexTest (method runsContainsSuccessfully)
replace the block lambda that returns a single expression (currently "() -> {
return Collections.singleton(object); }") with an expression lambda "() ->
Collections.singleton(object)"; do the same for the other similar block lambda
referenced around lines 62–75 so both ScalarOf<> suppliers use the concise
expression lambda form.
In `@eo-maven-plugin/src/test/java/org/eolang/maven/OyCachedTest.java`:
- Line 82: The test currently discards the result of new Together<>(30, thread
-> objectionary.get("parallel")).asList(), so update OyCachedTest to capture
that returned List from Together.asList() and add assertions against its
contents (e.g., size equals expected concurrency and all elements match the
expected value produced by objectionary.get("parallel")) instead of only
checking side effects; locate the Together instantiation and replace the dropped
call with a variable (List<?> results = new Together<>(30, thread ->
objectionary.get("parallel")).asList()) and add assertions that verify the list
length and that each element equals the expected returned value.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (63)
eo-maven-plugin/pom.xmleo-maven-plugin/src/main/java/org/eolang/maven/Cache.javaeo-maven-plugin/src/main/java/org/eolang/maven/Catalogs.javaeo-maven-plugin/src/main/java/org/eolang/maven/ChSource.javaeo-maven-plugin/src/main/java/org/eolang/maven/CommitHash.javaeo-maven-plugin/src/main/java/org/eolang/maven/ConcurrentCache.javaeo-maven-plugin/src/main/java/org/eolang/maven/DepDirs.javaeo-maven-plugin/src/main/java/org/eolang/maven/Dependencies.javaeo-maven-plugin/src/main/java/org/eolang/maven/DpsDefault.javaeo-maven-plugin/src/main/java/org/eolang/maven/DpsEachWithoutTransitive.javaeo-maven-plugin/src/main/java/org/eolang/maven/DpsWithRuntime.javaeo-maven-plugin/src/main/java/org/eolang/maven/Footprint.javaeo-maven-plugin/src/main/java/org/eolang/maven/MjAssemble.javaeo-maven-plugin/src/main/java/org/eolang/maven/MjCompile.javaeo-maven-plugin/src/main/java/org/eolang/maven/MjLint.javaeo-maven-plugin/src/main/java/org/eolang/maven/MjParse.javaeo-maven-plugin/src/main/java/org/eolang/maven/MjPrint.javaeo-maven-plugin/src/main/java/org/eolang/maven/MjProbe.javaeo-maven-plugin/src/main/java/org/eolang/maven/MjPull.javaeo-maven-plugin/src/main/java/org/eolang/maven/MjRegister.javaeo-maven-plugin/src/main/java/org/eolang/maven/MjResolve.javaeo-maven-plugin/src/main/java/org/eolang/maven/MjSafe.javaeo-maven-plugin/src/main/java/org/eolang/maven/MjTranspile.javaeo-maven-plugin/src/main/java/org/eolang/maven/MjUnplace.javaeo-maven-plugin/src/main/java/org/eolang/maven/MjUnspile.javaeo-maven-plugin/src/main/java/org/eolang/maven/Moja.javaeo-maven-plugin/src/main/java/org/eolang/maven/Objectionary.javaeo-maven-plugin/src/main/java/org/eolang/maven/OyRemote.javaeo-maven-plugin/src/main/java/org/eolang/maven/PackageInfos.javaeo-maven-plugin/src/main/java/org/eolang/maven/StMeasured.javaeo-maven-plugin/src/main/java/org/eolang/maven/TjForeign.javaeo-maven-plugin/src/main/java/org/eolang/maven/Walk.javaeo-maven-plugin/src/test/java/org/eolang/maven/CacheTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/ChCachedTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/ChRemoteTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/ChTextTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/CommitHashesMapTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/ConcurrentCacheTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/DepDirsTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/FakeMaven.javaeo-maven-plugin/src/test/java/org/eolang/maven/FileHashTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/FpAppliedWithCacheTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/FpIfReleasedTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/FpUpdateFromCacheTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/MjCleanTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/MjLintTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/MjParseTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/MjPlaceTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/MjPrintTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/MjProbeTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/MjPullTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/MjRegisterTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/MjResolveTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/MjTranspileTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/MjUnplaceTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/MjUnspileTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/ObjectsIndexTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/OyCachedTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/OyRemoteTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/PackageInfosTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/ProbesTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/ThreadedTest.javaeo-maven-plugin/src/test/java/org/eolang/maven/TjsForeignTest.java
💤 Files with no reviewable changes (2)
- eo-maven-plugin/src/main/java/org/eolang/maven/MjUnplace.java
- eo-maven-plugin/src/main/java/org/eolang/maven/Objectionary.java
| * All catalogs in one place, to avoid making multiple objects. | ||
| * | ||
| * @since 0.29 | ||
| * @todo #4884:30min Use ReentranLock instead of synchronized block in the code. |
There was a problem hiding this comment.
Typo: ReentranLock → ReentrantLock
The TODO references the wrong class name; it's missing the second t.
📝 Proposed fix
- * `@todo` `#4884`:30min Use ReentranLock instead of synchronized block in the code.
+ * `@todo` `#4884`:30min Use ReentrantLock instead of synchronized block in the code.📝 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.
| * @todo #4884:30min Use ReentranLock instead of synchronized block in the code. | |
| * `@todo` `#4884`:30min Use ReentrantLock instead of synchronized block in the code. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@eo-maven-plugin/src/main/java/org/eolang/maven/Catalogs.java` at line 27, Fix
the typo in the TODO comment inside Catalogs (change "ReentranLock" to
"ReentrantLock") so the referenced class name is correct; update the TODO text
in the Catalogs.java comment to "ReentrantLock" and, if present elsewhere in
comments, ensure consistent spelling to avoid confusion when implementing the
actual replacement of the synchronized block.
| * @todo #4884:30min Use ReentrantLock instead of synchronized block. | ||
| * This is the suggestion from qulice. | ||
| * Dont' forget to remove the PMD suppression for AvoidSynchronizedStatement | ||
| * after that. |
There was a problem hiding this comment.
Fix typo in TODO Javadoc text.
Line 17 has a typo (Dont'). Please correct it to keep maintenance notes clear and professional.
✍️ Suggested edit
- * Dont' forget to remove the PMD suppression for AvoidSynchronizedStatement
+ * Don't forget to remove the PMD suppression for `AvoidSynchronizedStatement`📝 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.
| * @todo #4884:30min Use ReentrantLock instead of synchronized block. | |
| * This is the suggestion from qulice. | |
| * Dont' forget to remove the PMD suppression for AvoidSynchronizedStatement | |
| * after that. | |
| * `@todo` `#4884`:30min Use ReentrantLock instead of synchronized block. | |
| * This is the suggestion from qulice. | |
| * Don't forget to remove the PMD suppression for `AvoidSynchronizedStatement` | |
| * after that. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@eo-maven-plugin/src/main/java/org/eolang/maven/ConcurrentCache.java` around
lines 15 - 18, In the Javadoc TODO comment inside the ConcurrentCache class
update the typo "Dont'" to "Don't" so the maintenance note reads correctly;
locate the TODO block that starts with "@todo `#4884`:30min Use ReentrantLock
instead of synchronized block." and replace the malformed apostrophe in the word
Dont' with the proper contraction Don't (no other logic changes required).
| @FunctionalInterface | ||
| interface Dependencies extends Iterable<Dep> { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let me find the exact file and read it to understand the context
cat -n eo-maven-plugin/src/main/java/org/eolang/maven/Dependencies.javaRepository: objectionary/eo
Length of output: 4656
🏁 Script executed:
# Check if Dependencies is ever instantiated via lambda or method reference
rg -n --type java 'Dependencies\s+[a-zA-Z_$][a-zA-Z0-9_$]*\s*=' eo-maven-plugin/src/main/java/org/eolang/maven/ -A 2 -B 2Repository: objectionary/eo
Length of output: 578
🏁 Script executed:
# Also check for direct lambda instantiation patterns
rg -n --type java '\(\)\s*->\s*' eo-maven-plugin/src/main/java/org/eolang/maven/Dependencies.java -A 2 -B 2Repository: objectionary/eo
Length of output: 41
🏁 Script executed:
# Find all classes implementing Dependencies
rg -n --type java 'implements\s+Dependencies' eo-maven-plugin/src/main/java/org/eolang/maven/Repository: objectionary/eo
Length of output: 1189
🏁 Script executed:
# Check if there are any lambda assignments to Dependencies anywhere
rg -n --type java 'Dependencies\s+\w+\s*=\s*\(' eo-maven-plugin/src/main/java/org/eolang/maven/ -A 1Repository: objectionary/eo
Length of output: 41
Remove @FunctionalInterface—it is misleading and unused.
@FunctionalInterface suggests clients may construct Dependencies via lambda expressions (e.g., Dependencies d = () -> list.iterator();). However, the entire codebase instantiates Dependencies exclusively through concrete class implementations (DpsDefault, DpsWithRuntime, DpsDepgraph, Fake, etc.) and never via lambda. The annotation is technically valid (one abstract method: iterator()) but actively misleading; it should be removed to avoid suggesting an unsupported pattern.
Additionally, extract the magic number at line 114:
new String[]{"test", "compiled", "runtime"}[rand.nextInt(3)]
The hardcoded 3 is decoupled from the array length. Use rand.nextInt(scopes.length) or a named constant to prevent them from falling out of sync if the scopes array is later extended.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@eo-maven-plugin/src/main/java/org/eolang/maven/Dependencies.java` around
lines 20 - 21, Remove the misleading `@FunctionalInterface` annotation from the
Dependencies interface declaration (interface Dependencies extends
Iterable<Dep>) since the project only uses concrete implementations (e.g.,
DpsDefault, DpsWithRuntime, DpsDepgraph, Fake) and does not rely on lambda
construction; then replace the magic number usage in the random scope selection
(currently new String[]{"test","compiled","runtime"}[rand.nextInt(3)]) by
referencing the scopes array length (e.g., use a local scopes String[] and call
rand.nextInt(scopes.length)) or a named constant for the size so the index bound
stays in sync with the array.
| * Lint. | ||
| * @throws IOException If fails | ||
| */ | ||
| @SuppressWarnings("PMD.UnnecessaryLocalRule") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the MjLint.java file around line 80 for full context
sed -n '70,120p' eo-maven-plugin/src/main/java/org/eolang/maven/MjLint.javaRepository: objectionary/eo
Length of output: 2065
🏁 Script executed:
# Search for "UnnecessaryLocalRule" in the entire codebase to check if it's a custom rule
rg "UnnecessaryLocalRule" --type java --type xmlRepository: objectionary/eo
Length of output: 6419
🏁 Script executed:
# Look for PMD configuration files
fd -e xml -e pom | xargs grep -l "PMD\|pmd\|ruleset" 2>/dev/null | head -10Repository: objectionary/eo
Length of output: 82
🏁 Script executed:
# Check the main pom.xml for PMD configuration
head -200 eo-maven-plugin/pom.xml | grep -A 20 -B 5 "pmd\|qulice"Repository: objectionary/eo
Length of output: 41
🏁 Script executed:
# Look for ruleset or PMD configuration files
find . -name "*.xml" | xargs grep -l "UnnecessaryLocalRule\|qulice" | head -5Repository: objectionary/eo
Length of output: 166
🏁 Script executed:
# Check pom.xml more broadly for PMD plugin configuration
cat eo-maven-plugin/pom.xml | grep -A 30 "qulice\|pmd" | head -80Repository: objectionary/eo
Length of output: 936
🏁 Script executed:
# Look for PMD ruleset XML files in the project
find . -name "*.xml" -type f | xargs grep -l "ruleset\|rule" 2>/dev/null | grep -E "(pmd|qulice|config)" | head -10Repository: objectionary/eo
Length of output: 41
🏁 Script executed:
# Check if there's a custom PMD ruleset or configuration file
find . -name "*ruleset*" -o -name "*pmd*config*" 2>/dev/nullRepository: objectionary/eo
Length of output: 41
🏁 Script executed:
# Let's check the root pom.xml for any PMD or qulice rule configuration
cat pom.xml | grep -A 50 "qulice\|pmd" | head -100Repository: objectionary/eo
Length of output: 2048
🏁 Script executed:
# Check qulice documentation or look for rule definitions in dependencies
# Let's search for any mention of how UnnecessaryLocalRule is used in context
grep -r "UnnecessaryLocalRule" --include="*.md" --include="*.txt" --include="*.html" 2>/dev/nullRepository: objectionary/eo
Length of output: 41
🏁 Script executed:
# Let's examine one of the other files that use this annotation to see if it makes sense
sed -n '1,30p' eo-parser/src/main/java/org/eolang/parser/OnDefault.javaRepository: objectionary/eo
Length of output: 806
🌐 Web query:
qulice maven plugin UnnecessaryLocalRule PMD custom rule
💡 Result:
Qulice’s Maven plugin doesn’t expose “use my PMD ruleset / add my PMD rules” configuration. With Qulice you basically have only these PMD-related options:
- Suppress a PMD rule in code (Qulice explicitly supports this approach):
`@SuppressWarnings`("PMD.UnnecessaryLocalBeforeReturn") // or the rule you need- Exclude files/packages from PMD within Qulice (note the
pmd:prefix):
<plugin>
<groupId>com.qulice</groupId>
<artifactId>qulice-maven-plugin</artifactId>
<version>0.24.3</version>
<configuration>
<excludes>
<exclude>pmd:.*/generated/.*</exclude>
</excludes>
</configuration>
</plugin>About UnnecessaryLocalRule
In modern PMD, the rule you likely mean is UnnecessaryLocalBeforeReturn (and it’s deprecated in favor of VariableCanBeInlined), referenced like:
<rule ref="category/java/codestyle.xml/UnnecessaryLocalBeforeReturn" />If you truly need a custom PMD rule/ruleset
Use maven-pmd-plugin alongside (or instead of) Qulice, and point it at your own ruleset XML:
<configuration>
<rulesets>
<ruleset>${project.basedir}/config/pmd/ruleset.xml</ruleset>
</rulesets>
</configuration>Citations:
- 1: https://www.qulice.com/quality.html?utm_source=openai
- 2: https://www.qulice.com/qulice-maven-plugin/example-exclude.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_userdocs_tools_maven.html?utm_source=openai
@SuppressWarnings("PMD.UnnecessaryLocalRule") is not a valid PMD rule — the annotation has no effect.
The correct PMD rule for unnecessary local variables is UnnecessaryLocalBeforeReturn. Additionally, since lint() is a void method with no return statement, that rule cannot apply here regardless. This annotation appears in 40+ files across the codebase, suggesting it was added systematically but based on an incorrect rule name.
The annotation should be removed entirely, as it provides no actual suppression and contributes no value.
🐛 Proposed fix
- `@SuppressWarnings`("PMD.UnnecessaryLocalRule")
private void lint() throws IOException {📝 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.
| @SuppressWarnings("PMD.UnnecessaryLocalRule") | |
| private void lint() throws IOException { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@eo-maven-plugin/src/main/java/org/eolang/maven/MjLint.java` at line 80,
Remove the ineffective suppression annotation
`@SuppressWarnings`("PMD.UnnecessaryLocalRule") from the MjLint class (and
similarly from other files where it was added); locate the annotation near the
lint() method and delete it entirely since the rule name is invalid and the
applicable PMD rule UnnecessaryLocalBeforeReturn cannot apply to a void method
like lint(), so no suppression is needed.
| /** | ||
| * Probe objects. | ||
| */ | ||
| @SuppressWarnings("PMD.UnnecessaryLocalRule") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Test 1: Search for any definition or declaration of UnnecessaryLocalRule in the repo
echo "=== Test 1: Searching for UnnecessaryLocalRule definition ==="
rg -rn "UnnecessaryLocalRule" --type java --type xml -A 2 2>/dev/null || echo "(no results)"
# Test 2: Check for other `@SuppressWarnings` usages with PMD rules
echo -e "\n=== Test 2: Other `@SuppressWarnings` with PMD rules ==="
rg -rn 'SuppressWarnings.*PMD\.Unnecessary' --type java -C 1 2>/dev/null || echo "(no results)"
# Test 3: List all `@SuppressWarnings` in the file to see usage patterns
echo -e "\n=== Test 3: All `@SuppressWarnings` in MjProbe.java ==="
rg -n '@SuppressWarnings' eo-maven-plugin/src/main/java/org/eolang/maven/MjProbe.java 2>/dev/null || echo "(not found)"Repository: objectionary/eo
Length of output: 33366
🏁 Script executed:
#!/bin/bash
# Test 4: Get the full context of the probe() method to verify locals usage
echo "=== Test 4: probe() method context (lines 55-82) ==="
sed -n '55,82p' eo-maven-plugin/src/main/java/org/eolang/maven/MjProbe.javaRepository: objectionary/eo
Length of output: 1260
🏁 Script executed:
#!/bin/bash
# Test 5: Check Qulice/Maven configuration files for custom rule definitions
echo "=== Test 5: Qulice configuration and custom rule definitions ==="
find . -name "pom.xml" -o -name "qulice.xml" -o -name "*rule*.xml" 2>/dev/null | head -20
echo -e "\n=== Checking pom.xml for Qulice dependency version ==="
rg -n "qulice-maven-plugin|qulice" pom.xml -A 2 2>/dev/null | head -30Repository: objectionary/eo
Length of output: 585
🏁 Script executed:
#!/bin/bash
# Additional verification: Check if there's any Qulice/PMD configuration that might define this rule
echo "=== Searching for qulice or PMD rule configuration files ==="
find . -type f \( -name "*.xml" -o -name "*.properties" -o -name "qulice*" \) | xargs grep -l "UnnecessaryLocal" 2>/dev/null || echo "(no match found)"
# Check if there's a checkstyle or PMD config that mentions this rule
echo -e "\n=== Checking for Qulice/PMD config files ==="
find . -type f \( -name "checkstyle*.xml" -o -name "pmd*.xml" -o -name "*qulice*.xml" \) 2>/dev/null | head -10Repository: objectionary/eo
Length of output: 182
🏁 Script executed:
#!/bin/bash
# Verify the exact annotation by examining the actual file
echo "=== Exact context of line 55 in MjProbe.java ==="
sed -n '54,60p' eo-maven-plugin/src/main/java/org/eolang/maven/MjProbe.javaRepository: objectionary/eo
Length of output: 432
Replace or remove the unrecognized PMD rule name PMD.UnnecessaryLocalRule.
The rule UnnecessaryLocalRule is not a recognized PMD rule. The standard PMD rule for unnecessary local variables is UnnecessaryLocalBeforeReturn (under category/java/codestyle.xml). No custom rule by the name UnnecessaryLocalRule exists in the project's Qulice configuration.
Furthermore, the probe() method does not match the pattern that would trigger UnnecessaryLocalBeforeReturn in any case: it is a void method (no return statement), and all three local variables—tojos, start, and probed—are each referenced multiple times within the method body.
Either correct the rule name to PMD.UnnecessaryLocalBeforeReturn if a specific rule check is intended, or remove the annotation entirely if no suppression is needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@eo-maven-plugin/src/main/java/org/eolang/maven/MjProbe.java` at line 55, The
`@SuppressWarnings`("PMD.UnnecessaryLocalRule") annotation in MjProbe.java is
using a non-existent PMD rule; open the MjProbe class and locate the annotation
on the probe() method and either remove the `@SuppressWarnings` entry entirely
(preferred since probe() is void and locals are used) or change the rule name to
the correct PMD identifier
"@SuppressWarnings(\"PMD.UnnecessaryLocalBeforeReturn\")" if you truly intend to
suppress that specific check; update the annotation text accordingly so it
references a valid PMD rule or delete the annotation.
| private boolean transpileTests = true; | ||
|
|
||
| @Override | ||
| @SuppressWarnings("PMD.UnnecessaryLocalRule") |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
PMD UnnecessaryLocalRule
💡 Result:
UnnecessaryLocalRule isn’t a current PMD rule name. In PMD’s Java ruleset, the relevant rule is UnnecessaryLocalBeforeReturn (reports locals that are assigned and then immediately returned), and it has been deprecated in PMD 7.17.0 in favor of VariableCanBeInlined (which also covers throw). [1]
If you’re trying to configure a ruleset, use one of these refs:
category/java/codestyle.xml/UnnecessaryLocalBeforeReturn(deprecated) [1]category/java/codestyle.xml/VariableCanBeInlined(replacement) [2]
Rule behavior example:
int x = doSomething();
return x; // PMD suggests: return doSomething();[1]
Sources:
[1] PMD Java Code Style rules (includes UnnecessaryLocalBeforeReturn + deprecation note) (pmd.github.io)
[2] PMD Java rules index (lists VariableCanBeInlined) (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.html?utm_source=openai
🌐 Web query:
qulice 0.25.1 PMD ruleset rules
💡 Result:
In Qulice 0.25.1, the PMD ruleset is defined in the qulice-pmd module at:
qulice-pmd/src/main/resources/com/qulice/pmd/ruleset.xml(tag0.25.1) [1]
Qulice states that for PMD it uses “inherited rules” (i.e., PMD’s standard rules) plus its own preconfiguration/extra rules. [2]
From the ruleset.xml content, examples of Qulice-specific rules/messages include: [1]
- Prefer Hamcrest matchers over plain JUnit
Assert.assert* - Prefer
String.isEmpty()overString.length() == 0 - Avoid field initialization outside constructors (except when there is no constructor)
- Avoid doing field initialization in multiple constructors; use one “main” constructor and delegate
- Avoid doing work in constructors beyond field assignments (except
this(...)/super(...)) - Avoid direct access to static fields
- Avoid accessing static fields/methods via
this - Public static methods are prohibited
Files.createFileshouldn’t be used in tests
If you want, paste the full ruleset.xml here and I’ll extract the complete rule list (names/refs/priorities) in a clean table.
🌐 Web query:
site:github.com qulice pmd ruleset.xml 0.25.1 UnnecessaryLocalRule
💡 Result:
For Qulice 0.25.1, the PMD configuration lives in the yegor256/qulice repo under:
qulice-pmd/src/main/resources/com/qulice/pmd/ruleset.xml(tag 0.25.1, dated Jan 21, 2026) [1]
Raw file (easy to fetch/download):
https://raw.githubusercontent.com/yegor256/qulice/0.25.1/qulice-pmd/src/main/resources/com/qulice/pmd/ruleset.xml
[2]
Also: UnnecessaryLocalRule is not the PMD Java rule name. The PMD rule you likely mean is UnnecessaryLocalBeforeReturn (implementation class ...UnnecessaryLocalBeforeReturnRule), referenced in rulesets as category/java/codestyle.xml/UnnecessaryLocalBeforeReturn [3]. (PMD notes it’s deprecated in PMD 7.17.0 in favor of VariableCanBeInlined [3].)
Sources: [1] [2] [3]
🌐 Web query:
qulice PMD ruleset custom rules complete list
💡 Result:
Qulice’s custom PMD rules (i.e., rules not inherited from PMD itself) are defined in Qulice’s bundled PMD ruleset (qulice-pmd/src/main/resources/com/qulice/pmd/ruleset.xml). (github.com)
Complete list of Qulice custom PMD rules (as of the current master branch):
- UseHamcrestInsteadOfJUnitAssertions (prefer Hamcrest matchers over plain JUnit
Assert.assert*) - UseStringIsEmpty (prefer
String.isEmpty()overlength() == 0) - AvoidFieldInitializationOutsideConstructor
- AvoidFieldInitializationInMultipleConstructors (one “main” constructor should do real initialization; others delegate)
- AvoidNonAssignmentInConstructor (constructors should only assign fields / call
this(...)/super(...)) - AvoidDirectStaticAccess (don’t access static fields directly)
- AvoidStaticMemberAccessViaThis (don’t access static fields/methods through
this) - ProhibitPublicStaticMethods
- AvoidFilesCreateFileInTests (don’t use
Files.createFilein tests) (raw.githubusercontent.com)
Citations:
- 1: https://github.com/yegor256/qulice
- 2: https://raw.githubusercontent.com/yegor256/qulice/master/qulice-pmd/src/main/resources/com/qulice/pmd/ruleset.xml
Correct the invalid PMD rule name in all three @SuppressWarnings annotations.
UnnecessaryLocalRule is not a valid PMD rule name—neither in the standard PMD ruleset nor in Qulice 0.25.1's custom rules. The correct PMD rule is UnnecessaryLocalBeforeReturn (or its modern replacement VariableCanBeInlined as of PMD 7.17.0). Using the incorrect name means the @SuppressWarnings annotation silently suppresses nothing and will not prevent violations from surfacing in CI.
Update all three annotations:
- Line 125
- Line 165
- Line 265
Change @SuppressWarnings("PMD.UnnecessaryLocalRule") to @SuppressWarnings("PMD.UnnecessaryLocalBeforeReturn").
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@eo-maven-plugin/src/main/java/org/eolang/maven/MjTranspile.java` at line 125,
The three `@SuppressWarnings` annotations using the invalid PMD rule name
"PMD.UnnecessaryLocalRule" (found in MjTranspile at the three annotated
locations) must be corrected so the suppressions actually work; replace each
occurrence of `@SuppressWarnings`("PMD.UnnecessaryLocalRule") with
`@SuppressWarnings`("PMD.UnnecessaryLocalBeforeReturn") (the valid PMD rule name)
for the annotations at the three spots (around lines with the MjTranspile
methods/blocks) to ensure PMD/Qulice will recognize and honor the suppression.
|
|
||
| @Override | ||
| @SuppressWarnings("PMD.PrematureDeclaration") | ||
| @SuppressWarnings("PMD.UnnecessaryLocalRule") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 1) Current PMD suppressions in the repo ==="
rg -n --type=java '@SuppressWarnings\("PMD\.[^"]+"\)' | head -20
echo ""
echo "=== 2) Check for specific UnnecessaryLocal-related rules ==="
rg -n --type=java 'PMD\.UnnecessaryLocalRule|PMD\.UnnecessaryLocalBeforeReturn|PMD\.PrematureDeclaration|PMD\.UnusedLocalVariable'
echo ""
echo "=== 3) Check the specific file in question ==="
if [ -f "eo-maven-plugin/src/main/java/org/eolang/maven/StMeasured.java" ]; then
echo "File found, showing context around line 48:"
sed -n '40,60p' "eo-maven-plugin/src/main/java/org/eolang/maven/StMeasured.java" | cat -n
fi
echo ""
echo "=== 4) Search for PMD configuration files ==="
fd -e xml -e properties | xargs grep -l "UnnecessaryLocal\|pmd\.xml\|ruleset" 2>/dev/null | head -10Repository: objectionary/eo
Length of output: 10227
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 1) Search for PMD/Qulice configuration files ==="
find . -type f \( -name "pmd.xml" -o -name "ruleset.xml" -o -name ".qulice" -o -name "*.qulice" \) 2>/dev/null | head -20
echo ""
echo "=== 2) Check Maven POM files for PMD plugin configuration ==="
rg -l 'maven-pmd-plugin|qulice-maven-plugin' pom.xml 2>/dev/null || echo "No matches in top-level pom.xml"
echo ""
echo "=== 3) Find all pom.xml files and check for PMD/Qulice config ==="
fd 'pom.xml' | xargs rg -l 'maven-pmd-plugin|qulice-maven-plugin' 2>/dev/null | head -10
echo ""
echo "=== 4) Check Qulice documentation or setup files ==="
rg -i 'qulice|UnnecessaryLocal' --type=xml --type=properties | head -20
echo ""
echo "=== 5) Search in any config or properties files ==="
fd -e properties -e xml -e yml -e yaml | xargs rg 'UnnecessaryLocal' 2>/dev/null | head -10Repository: objectionary/eo
Length of output: 453
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 1) Check main pom.xml for Qulice config ==="
rg -A 10 -B 2 'qulice\|maven-pmd-plugin' pom.xml | head -40
echo ""
echo "=== 2) Check eo-maven-plugin/pom.xml ==="
rg -A 10 -B 2 'qulice\|maven-pmd-plugin' eo-maven-plugin/pom.xml | head -40
echo ""
echo "=== 3) Search for any Java files that mention rule definitions ==="
rg 'UnnecessaryLocal' --type=java | head -20
echo ""
echo "=== 4) Check if there's a .qulice-exclude file or similar ==="
find . -type f -name '.qulice*' -o -name '*qulice*' 2>/dev/null | grep -v '.git' | head -20Repository: objectionary/eo
Length of output: 106
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 1) Read main pom.xml (first 100 lines) ==="
head -100 pom.xml
echo ""
echo "=== 2) Search for 'pmd' in pom.xml ==="
grep -i 'pmd\|qulice' pom.xml | head -20
echo ""
echo "=== 3) Search for 'pmd' in eo-maven-plugin pom.xml ==="
grep -i 'pmd\|qulice' eo-maven-plugin/pom.xml | head -20Repository: objectionary/eo
Length of output: 4090
🌐 Web query:
Qulice PMD UnnecessaryLocalRule rule naming convention
💡 Result:
In Qulice, PMD rules use PMD’s own rule names (not a special Qulice naming scheme). To suppress a PMD rule you typically write: @SuppressWarnings("PMD.<RuleName>"). [1]
For the “unnecessary local variable (before return)” check, the PMD rule name is:
UnnecessaryLocalBeforeReturn- PMD ruleset reference:
category/java/codestyle.xml/UnnecessaryLocalBeforeReturn[2]
- PMD ruleset reference:
So the suppression looks like:
`@SuppressWarnings`("PMD.UnnecessaryLocalBeforeReturn")Note: In newer PMD versions, UnnecessaryLocalBeforeReturn is deprecated in favor of VariableCanBeInlined. [2]
Replace PMD.UnnecessaryLocalRule with the correct PMD rule name.
PMD.UnnecessaryLocalRule is not a valid PMD rule. The correct rule is either:
PMD.UnnecessaryLocalBeforeReturn(current/legacy)PMD.VariableCanBeInlined(newer PMD versions)
This invalid suppression is silently ineffective and does not suppress the intended warnings. Update line 48 and all other occurrences throughout the codebase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@eo-maven-plugin/src/main/java/org/eolang/maven/StMeasured.java` at line 48,
The `@SuppressWarnings`("PMD.UnnecessaryLocalRule") annotation is invalid; update
it to a correct PMD rule name (either "PMD.UnnecessaryLocalBeforeReturn" for
legacy PMD or "PMD.VariableCanBeInlined" for newer PMD) on the `@SuppressWarnings`
annotation in StMeasured (and replace all other occurrences across the
codebase), e.g., change the string literal in the `@SuppressWarnings`(...) on the
class/method where StMeasured is defined to the chosen valid rule so PMD
suppression actually takes effect.
| @Test | ||
| void compilesSourceAndAddsToCache(@Mktmp final Path temp) throws Exception { | ||
| final var base = temp.resolve("cache"); | ||
| @SuppressWarnings("PMD.UnnecessaryLocalRule") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the actual test file to see the code and annotations
cat -n eo-maven-plugin/src/test/java/org/eolang/maven/CacheTest.java | head -220Repository: objectionary/eo
Length of output: 10043
🏁 Script executed:
# Search for PMD/qulice configuration files that might define custom rules
fd -e xml -e properties | grep -i -E "(pmd|qulice|checkstyle|ruleset)" | head -20Repository: objectionary/eo
Length of output: 41
🏁 Script executed:
# Search for any references to UnnecessaryLocalRule in the project
rg "UnnecessaryLocalRule" --type xml --type java --type propertiesRepository: objectionary/eo
Length of output: 93
🏁 Script executed:
# Also search for any custom rule definitions or imports
rg "PMD\." eo-maven-plugin/src/test/java/org/eolang/maven/CacheTest.java -A 2 -B 2Repository: objectionary/eo
Length of output: 850
🏁 Script executed:
# Search for qulice or PMD configuration in pom.xml or related build files
rg "qulice|PMD" eo-maven-plugin/pom.xml -A 3 -B 1Repository: objectionary/eo
Length of output: 335
🏁 Script executed:
# Search for any ruleset files
fd -e xml | xargs grep -l "ruleset\|UnnecessaryLocal" 2>/dev/null | head -10Repository: objectionary/eo
Length of output: 41
🏁 Script executed:
# Check if there's a parent pom or qulice config
find . -name "pom.xml" -type f | head -10 | xargs grep -l "qulice" 2>/dev/nullRepository: objectionary/eo
Length of output: 166
🌐 Web query:
PMD UnnecessaryLocalRule rule name valid
💡 Result:
UnnecessaryLocalRule is not a valid PMD rule name in current PMD rule sets.
For “unnecessary local variable before returning”, the valid Java rule names are:
UnnecessaryLocalBeforeReturn(still exists but deprecated since PMD 7.17.0) [1]- Replacement:
VariableCanBeInlined(added in PMD 7.17.0) — use it instead:
<rule ref="category/java/codestyle.xml/VariableCanBeInlined" />[1][2]
If you’re referencing UnnecessaryLocalRule in a custom ruleset, PMD won’t find it—update it to one of the rule refs above. [1]
Sources: [1] [2]
🌐 Web query:
PMD UnnecessaryLocalBeforeReturn rule documentation
💡 Result:
PMD’s UnnecessaryLocalBeforeReturn rule flags cases where a local variable is assigned only to be immediately returned, e.g.:
int x = doSomething();
return x; // prefer: return doSomething();Status / replacement
- Deprecated (since PMD 7.17.0); use
VariableCanBeInlinedinstead (it also coversthrow). [1][2]
Rule details
- Category: Java → Code Style
- Since: PMD 3.3
- Priority: 3 (Medium)
- Implementation class:
net.sourceforge.pmd.lang.java.rule.codestyle.UnnecessaryLocalBeforeReturnRule[1]
Property
statementOrderMatters(defaulttrue): iffalse, the variable declaration and return don’t need to be consecutive; any variable used solely in a return statement may be reported. [1]
How to enable
<rule ref="category/java/codestyle.xml/UnnecessaryLocalBeforeReturn" />[1]
Sources
[1] PMD Java Code Style rules (UnnecessaryLocalBeforeReturn section) (pmd.github.io)
[2] PMD Java rules index (mentions deprecation and replacement) (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.html?utm_source=openai
Remove the invalid PMD.UnnecessaryLocalRule suppressions — the rule does not exist.
PMD.UnnecessaryLocalRule is not a valid PMD rule name. The correct rule for unnecessary local variables is UnnecessaryLocalBeforeReturn (deprecated since PMD 7.17.0; replaced by VariableCanBeInlined). These four annotations silently have no effect.
Moreover, the local variables in these methods (base, source, target, counter, cache, tail) are all referenced multiple times, so even a correct suppression would be superfluous.
Remove the four @SuppressWarnings annotations at lines 32, 66, 89, and 112.
Also consolidate the boolean assertions at lines 59–60 into separate matchers for better failure diagnostics:
- MatcherAssert.assertThat(
- "Cache file must be created and hash file must be created",
- Files.exists(base.resolve(tail))
- && Files.exists(base.resolve(String.format("%s.sha256", tail))),
- Matchers.is(true)
- );
+ MatcherAssert.assertThat(
+ "Cache file must be created",
+ Files.exists(base.resolve(tail)),
+ Matchers.is(true)
+ );
+ MatcherAssert.assertThat(
+ "Hash file must be created",
+ Files.exists(base.resolve(String.format("%s.sha256", tail))),
+ Matchers.is(true)
+ );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@eo-maven-plugin/src/test/java/org/eolang/maven/CacheTest.java` at line 32,
Remove all four `@SuppressWarnings`("PMD.UnnecessaryLocalRule") annotations from
the CacheTest class (they are invalid and unnecessary) and leave the tests
unchanged otherwise; the local variables base, source, target, counter, cache
and tail are legitimately used multiple times so no suppression is needed. Also
replace the combined boolean assertion that checks two conditions at once (the
assertion covering two predicates on cache/state) with two separate
assertions/matchers so each condition is asserted independently (e.g.,
assertThat(...) for the first predicate and assertThat(...) for the second) to
improve failure diagnostics.
| final Path small = Files.createDirectories( | ||
| Files.createDirectories(dir.resolve("child")).resolve("child.eo") | ||
| ); |
There was a problem hiding this comment.
Files.createDirectories creates child.eo as a directory, not a file.
The outer Files.createDirectories(…) call receives the resolved path dir/child/child.eo and creates it as a directory, not a file. The variable name small and the .eo extension clearly imply the intent is to represent an EO source file. The test still passes because MjClean removes the entire target tree regardless, but the scenario exercised silently changed from "a .eo file is cleaned" to "a directory named child.eo is cleaned."
🐛 Proposed fix
- final Path small = Files.createDirectories(
- Files.createDirectories(dir.resolve("child")).resolve("child.eo")
- );
+ final Path small = Files.createFile(
+ Files.createDirectories(dir.resolve("child")).resolve("child.eo")
+ );📝 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.
| final Path small = Files.createDirectories( | |
| Files.createDirectories(dir.resolve("child")).resolve("child.eo") | |
| ); | |
| final Path small = Files.createFile( | |
| Files.createDirectories(dir.resolve("child")).resolve("child.eo") | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@eo-maven-plugin/src/test/java/org/eolang/maven/MjCleanTest.java` around lines
30 - 32, In MjCleanTest replace the incorrect use of Files.createDirectories
that creates a directory named "child.eo" (the variable small) with code that
ensures the "child" directory exists and then creates an actual file "child.eo"
(e.g. call Files.createDirectories(dir.resolve("child")) then
Files.createFile(dir.resolve("child").resolve("child.eo")) or equivalent);
update the variable small to refer to the created file path so the test truly
exercises cleaning a .eo file rather than a directory.
|
|
||
| @Disabled | ||
| @Test | ||
| @SuppressWarnings("PMD.UnnecessaryLocalRule") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, examine the file at line 220 with context
sed -n '210,230p' eo-maven-plugin/src/test/java/org/eolang/maven/MjTranspileTest.javaRepository: objectionary/eo
Length of output: 828
🏁 Script executed:
# Search for qulice configuration files
fd -e 'xml' -e 'properties' -e 'yml' -e 'yaml' | xargs grep -l 'qulice\|pmd' 2>/dev/null | head -20Repository: objectionary/eo
Length of output: 156
🏁 Script executed:
# Search for other uses of UnnecessaryLocalRule or UnnecessaryLocalBeforeReturn in the codebase
rg 'UnnecessaryLocal' --type javaRepository: objectionary/eo
Length of output: 6419
🏁 Script executed:
# Check for .pmd or qulice configuration files
find . -name '.pmd*' -o -name 'qulice*' -o -name 'pmd-rules*' 2>/dev/nullRepository: objectionary/eo
Length of output: 89
🏁 Script executed:
# Examine the qulice workflow configuration
cat .github/workflows/qulice.ymlRepository: objectionary/eo
Length of output: 795
🏁 Script executed:
# Check the main pom.xml for qulice plugin configuration
head -100 pom.xml | grep -A 20 -i quliceRepository: objectionary/eo
Length of output: 41
🏁 Script executed:
# Search for pmd ruleset or qulice configuration
find . -name 'qulice-*.xml' -o -name '.pmdrc*' -o -name 'ruleset*.xml' 2>/dev/nullRepository: objectionary/eo
Length of output: 41
🌐 Web query:
PMD UnnecessaryLocalRule official rule name documentation
💡 Result:
In PMD’s Java rules, there is no official rule named UnnecessaryLocalRule. The rule people often mean is:
- Official rule name (ruleset ID):
UnnecessaryLocalBeforeReturn(Java / Code Style category) [1]- Rule implementation class:
net.sourceforge.pmd.lang.java.rule.codestyle.UnnecessaryLocalBeforeReturnRule[1] - Documentation: see the “UnnecessaryLocalBeforeReturn” section on the PMD Java Code Style rules page [1]
- Rule implementation class:
Note: UnnecessaryLocalBeforeReturn is deprecated since PMD 7.17.0 and PMD recommends VariableCanBeInlined instead [1].
Sources:
[1] PMD Java Code Style rules documentation (current) (pmd.github.io)
Citations:
🏁 Script executed:
# Search for qulice-maven-plugin in pom.xml files
rg 'qulice-maven-plugin' -A 10Repository: objectionary/eo
Length of output: 3405
🏁 Script executed:
# Check if there's any custom ruleset definition
rg 'UnnecessaryLocalRule' --type xml -B 3 -A 3Repository: objectionary/eo
Length of output: 41
🏁 Script executed:
# Look in pom.xml for any profile or plugin configuration related to PMD or qulice
rg 'qulice|pmd' pom.xml -i -B 2 -A 5 | head -60Repository: objectionary/eo
Length of output: 499
🌐 Web query:
qulice UnnecessaryLocalRule custom PMD rule definition
💡 Result:
In Qulice, UnnecessaryLocalRule isn’t a special “Qulice-only” PMD rule name—this check corresponds to PMD’s built-in rule UnnecessaryLocalBeforeReturn (deprecated) or, in newer PMD versions, its replacement VariableCanBeInlined. PMD documents UnnecessaryLocalBeforeReturn as deprecated and says to use VariableCanBeInlined instead. [1]
Use the built-in PMD rule in your ruleset (recommended)
Create a PMD ruleset.xml (template is in PMD docs) [2] and reference the rule:
<?xml version="1.0"?>
<ruleset name="Custom Rules"
xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.sourceforge.io/ruleset_2_0_0.xsd">
<description>My custom rules</description>
<!-- PMD 7+ replacement -->
<rule ref="category/java/codestyle.xml/VariableCanBeInlined" />
<!-- Legacy (deprecated) -->
<!-- <rule ref="category/java/codestyle.xml/UnnecessaryLocalBeforeReturn" /> -->
</ruleset>If you truly meant a custom XPath rule named UnnecessaryLocalRule
PMD supports custom XPath rules and (in PMD 7) uses XPath 3.1; the old version property was removed, and XPath rules should use net.sourceforge.pmd.lang.rule.xpath.XPathRule. [3]
Skeleton:
<rule name="UnnecessaryLocalRule"
language="java"
message="Unnecessary local variable; inline it"
class="net.sourceforge.pmd.lang.rule.xpath.XPathRule">
<priority>3</priority>
<properties>
<property name="xpath">
<value><![CDATA[
(: put your XPath here :)
]]></value>
</property>
</properties>
</rule>If you paste an example snippet that Qulice/PMD flags (the exact pattern), I can provide a correct XPath that matches it.
Sources:
[1] PMD Java codestyle rules (UnnecessaryLocalBeforeReturn deprecated; replacement guidance) https://pmd.github.io/pmd/pmd_rules_java_codestyle.html
[2] PMD “Making rulesets” (ruleset.xml template + rule refs) https://pmd.github.io/pmd/pmd_userdocs_making_rulesets.html
[3] PMD “Writing XPath rules” / PMD 7 XPath changes + XPathRule usage https://pmd.github.io/pmd/pmd_userdocs_extending_writing_xpath_rules.html
PMD.UnnecessaryLocalRule is not a recognized PMD rule — the suppression annotation has no effect.
UnnecessaryLocalBeforeReturn is the standard PMD rule for unnecessary local variables (deprecated in PMD 7.17.0, with VariableCanBeInlined as the recommended replacement). The rule name UnnecessaryLocalRule does not exist in official PMD or Qulice.
Proposed fix
- `@SuppressWarnings`("PMD.UnnecessaryLocalRule")
+ `@SuppressWarnings`("PMD.UnnecessaryLocalBeforeReturn")
void recompilesIfExpired(`@Mktmp` final Path temp) throws IOException {This pattern occurs in 50+ locations across the codebase. Consider a systematic update to use the correct rule name.
📝 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.
| @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-maven-plugin/src/test/java/org/eolang/maven/MjTranspileTest.java` at line
220, The suppression annotation `@SuppressWarnings`("PMD.UnnecessaryLocalRule") is
invalid; replace it with a recognized PMD rule such as
"PMD.UnnecessaryLocalBeforeReturn" or the newer "PMD.VariableCanBeInlined" in
MjTranspileTest (and the other ~50 occurrences) so the suppression takes effect;
update the string in the `@SuppressWarnings` annotations (e.g., in the
class/method where MjTranspileTest contains that annotation) across the codebase
consistently to the chosen valid rule name.
|
@yegor256 could you have a look, please? |
|
@volodya-lombrozo Thanks for your contribution! You've earned +4 points for this submission: +16 as a basis, +16 for hits-of-code (849 * 0.05 capped at 16), -4 for exceeding 100 hits-of-code (849 >= 100), and -8 for exceeding 400 hits-of-code (849 >= 400), with the final score capped at the maximum of 24 points. Please keep them coming! Your running score is +455 - don't forget to check your Zerocracy account too. |
This PR upgrades the
qulice-maven-pluginto version0.25.1ineo-maven-plugin.Related to #4884
Summary by CodeRabbit
Release Notes
Chores
Refactor
Tests