Recipe quality triage: bugs, wiring and design questions (A1–F3) #1116

We triaged all recipes in this repo against our recipe review checklist: 216 Java classes and 5 YAML files, the same way as openrewrite/rewrite-spring#1201, openrewrite/rewrite-testing-frameworks#1211 and openrewrite/rewrite-migrate-java#1330. This issue tracks behavioural bugs, wiring and design questions. Low-risk mechanical polish is tracked separately in #1115.

We reproduced 36 of the bugs with a throwaway RewriteTest, and marked those PROBED. Two suspected bugs did not reproduce and were left out. CommonStaticAnalysis is the flagship composite here, so sections B and C cover only recipes it runs. Sections D and E in the first comment cover the rest.

Each item is self-contained, so you can pick one up independently or point an agent at it ("work on item C3 of this issue"). Please reference the item ID in your PR title or description. As soon as you open the PR, tick the item's box and add a link to the PR after it; there's no need to wait for the merge.

A. Wiring & placement (decide first)

• A1 Take IndexOfChecksShouldUseAStartPosition out of CommonStaticAnalysis (PROBED)

  Details

  • Where: IndexOfChecksShouldUseAStartPosition.java:54-69; the @DocumentExample (changeLhsWithLiteral) asserts the broken output.
  • Problem: s.indexOf("x") > 2 ("the first match is after index 2") becomes s.indexOf("x", 2) > -1 ("some match is at index 2 or later"). That is off by one, and also true when an early match is followed by a later one. There is no equivalent single call.
  • Options: remove it from CSA and make it search-only, or delete it.

• A2 HideUtilityClassConstructor: active in CodeCleanup, commented out of CommonStaticAnalysis, and breaks callers (PROBED)

  Details

  • Where: HideUtilityClassConstructorVisitor.java:84-98,246-262; static-analysis.yml:38; common-static-analysis.yml:46 (no reason given).
  • Problem: a class counts as a utility class from its own members only. With class MoreConstants extends Constants {} and new Constants() elsewhere, private Constants() {} is still added, and both stop compiling.
  • Groovy (Suspected): the context-sensitive Java constructor template runs inside Groovy classes; add a Groovy test.
  • Fix: skip classes that are instantiated or extended in the source set (scan per JavaProject). Then decide whether it belongs in CSA, CC, both or neither, and comment the decision.

• A3 ReplaceThreadRunWithThreadStart rewrites super.run() (PROBED)

  Details

  • Where: static-analysis.yml:51-62: a bare ChangeMethodName on java.lang.Thread run(), wired into CC.
  • Problem: super.run() inside a Thread subclass's run() (the standard delegation to the Runnable target) becomes super.start(), which throws IllegalThreadStateException at runtime. thread::run passed as a Runnable becomes thread::start and spawns an extra thread. S1217 flags neither.
  • Fix: a Java recipe that skips super. receivers, calls inside a Thread subclass, and member references. Decide whether it belongs in CC (a "cleanup") or next to its RSPEC siblings in CSA.
  • Related: Thread.run() should not be called directly.RSPEC-1217 #43 (the rule this implements).

• A4 NoFinalizer deletes finalizer logic and makes ObjectFinalizeCallsSuper dead in CommonStaticAnalysis

  Details

  • Where: NoFinalizer.java:52-64; common-static-analysis.yml:64,69.
  • Problem: the cleanup code in a finalize() body is lost. Callers of a public finalize() from other packages stop compiling, because the call now resolves to the protected Object#finalize. CSA runs NoFinalizer first, so ObjectFinalizeCallsSuper (D9) never acts.
  • Decision: which of the two should CSA run? NoFinalizer could become search-only.
  • Related: ObjectFinalizeCallsSuper: declare the overridden finalize() exceptions when adding super.finalize() #969 and Widen finalize() throws clauses across class hierarchies in ObjectFinalizeCallsSuper #993 (open ObjectFinalizeCallsSuper fixes).

• A5 CommonDeclarationSiteTypeVariances: the excludeBounds key is silently ignored (PROBED); key fixed in Use excludedBounds in CommonDeclarationSiteTypeVariances #1131, overrides part still open

  Details

  • Where: static-analysis.yml:90 says excludeBounds:, but the option is excludedBounds (DeclarationSiteTypeVariance.java:51).
  • Problem: java.lang.* is never excluded, so Function<String, Integer> becomes Function<? super String, ? extends Integer>. validateAll() reports the recipe as valid: unknown YAML option keys aren't flagged anywhere. That is worth raising in openrewrite/rewrite.
  • Also (Confirmed): DeclarationSiteTypeVariance changes an interface or base method signature while its @Override implementations are skipped, so they stop compiling. The overriddenMethods test (DeclarationSiteTypeVarianceTest:249) hides this with SourceSpec::skip. Not covered by Skip declaration-site variance for invariant storage #1007.
  • Fix: rename the key, skip overridable methods that have overrides in the source set, and remove the skip from the test.

• A6 Decide on CommonStaticAnalysis recipes that change runtime values by design

  Details

  • PreferSystemGetPropertyOverGetenv (:37-45): getenv("HOME") and user.home differ in containers, CI and with sudo -H. getenv("TMPDIR") is usually null on Linux while java.io.tmpdir never is, so if (System.getenv("TMPDIR") != null) flips. There is no RSPEC rule behind it.
  • BigDecimalDoubleConstructor (:24-47): BigDecimal.valueOf(1.0) has scale 1 where new BigDecimal(1.0) has scale 0, so equals, hashCode and toString change. The description says the value is preserved. Related: BigDecimalDoubleConstructorRecipe should also convert BigDecimal.valueOf(doubleLiteral) -> new BigDecimal(stringLiteral) #255.
  • CaseInsensitiveComparisonsDoNotChangeCase (:56-67): "straße".toUpperCase().equalsIgnoreCase("STRASSE") is true; the rewrite is false.
  • UpperCaseLiteralSuffixes (:79-81): upper-casing every hex literal (0xff → 0xFF) is beyond S818 and causes high-volume churn.
  • WriteOctalValuesAsDecimal (:53-65,71-105): Go os.FileMode is exempt, but Java permission masks (TarArchiveEntry#setMode(0644)) become 420.
  • Decision per recipe: keep and document, restrict, or drop from CSA.

• A7 Scope follow-up visitors to what the recipe changed (PROBED for InlineVariable) A7 + B6: scope follow-up visitors to what each recipe changed #1122 A7 + B6 follow-up: side effects, comments and formatting scope #1123

  Details

  • InlineVariable.java:82 runs RemoveUnusedLocalVariables over the whole file whenever it inlines one variable. So a recipe that is deliberately commented out of CSA (common-static-analysis.yml:79) runs anyway. Probe: void g() { int unused = 42; } int h(int a) { int r; r = a * 2; return r; } empties g(). Combined with B6, int unused = counter++; is removed with its increment.
  • The same pattern elsewhere (Confirmed): one match makes an unrelated recipe rewrite the whole file or class.
    • EmptyBlock: RemoveCallsToSystemGc:62, SimplifyConstantIfBranchExecution:59-69 (also RemoveUnneededBlock)
    • RemoveUnusedImports: RemoveMethodsOnlyCallSuper:122, UseLambdaForFunctionalInterface:147, RemoveUnusedPrivateMethods:105
    • RemoveUnneededBlock: TernaryOperatorsShouldNotBeNested:84,97, AllBranchesIdentical:93
    • SimplifyBooleanExpression: MoveConditionsToWhile:82; also SimplifyConstantIfBranchExecution in AnnotateRequiredParameters:96-99
    • EqualsAvoidsNull: StringLiteralEquality:63
    • Auto-format of the enclosing block or method: UnwrapElseAfterReturn:88, UnnecessaryPrimitiveAnnotations:63-67, TypecastParenPad:61-66 (CC)
  • Fix: apply the follow-up to the changed subtree only (maybeRemoveImport for specific types, a cursor-bounded visitor, autoFormat of the new element). For InlineVariable, remove only the declaration it emptied.
  • Related: EmptyBlock and RemoveMethodsOnlyCallSuper delete blocks and methods whose only content is a comment #1050 (EmptyBlock deleting comment-only blocks, which these file-wide runs spread).

• A8 CovariantEquals is @Incubating, runs in CommonStaticAnalysis, and crashes on abstract methods (PROBED) Add a delegating equals(Object) in CovariantEquals (A8 of #1116) #1134

  Details

  • Where: CovariantEquals.java:88-128.
  • Problem:
    • abstract class Shape { public abstract boolean equals(Shape other); } throws an AssertionError at :115 (an NPE without -ea).
    • The generated getClass() check makes subclass instances unequal.
    • A subclass's @Override equals(Point) in another file stops compiling.
    • A value-based equals without hashCode violates the hash contract.
  • Fix: bail out on a null body, use instanceof, skip overridden hierarchies (or keep a delegating overload), and decide whether it stays in CSA.

• A9 Reachability: 115 of 205 recipes are reached by no composite in this repo

  Details

  • Wire after a scale run (small, typed, SAST-style, no consumer anywhere): RemoveUnneededBlock, UnnecessaryFinalInTryWithResources, SillyEqualsCheck, RemoveUnnecessaryBitwiseOperation, SimplifyRedundantLogicalExpression (fix E4 first), RemoveDuplicateConditions, RemoveSelfAssignment, RemoveUnreachableMultiCatchAlternative, UseStringCaseInsensitiveOrderRecipe, SortedSetStreamToLinkedHashSet, StaticAccessViaInstance, CollapsibleIfStatements, UseMapEntrySetIteration, InterruptedExceptionHandling (E7), and UseMapContainsKey (fold the single-entry JavaApiBestPractices into CSA).
  • Bug-prevention rules consumed only by JBP that arguably belong in CSA: ReplaceWeekYearWithYear, RemoveHashCodeCallsFromArrayInstances, RemoveToStringCallsFromArrayInstances, UseObjectNotifyAll, URLEqualsHashCodeRecipes, CompareEnumsWithEqualityOperator, MissingOverrideAnnotation.
  • Opt-in by design (style policies, opposing pairs, downgrades): say so in their descriptions, and don't wire them. Examples: ExplicitThis, FinalizeLocalVariables vs NoFinalizedLocalVariables, ReplaceTextBlockWithString and RemoveInstanceOfPatternMatch (downgrades), ReorderAnnotationAttributes, SingleLineCommentSpacing, HiddenField, LowercasePackage, ReferentialEqualityToObjectEquals.
  • Search recipes with no composite: FindMainWithThrowsClause, FindMissingJavadocOnPublicMethods, FindNewExceptionWithoutCause, FindNonCaseLabelsInSwitch, FindSystemAndRuntimeExitCalls, FindThreadGroupUsages, FindWaitWithMultipleLocksHeld. Group them in a FindStaticAnalysisIssues composite.
  • ModernizeCollections is unreferenced; of its members only ReplaceStackWithDeque is wired anywhere (JBP), and that one has the worst bugs (D1).
  • Related: AllBranchesIdentical to hoist side-effect statements #1032 and Preserve observable array accesses and casts in identical branches #995 (AllBranchesIdentical), Java: Fix AddSerialVersionUidToSerializable for generic classes #1060 and AddSerialVersionUidToSerializable skips generic classes implementing Serializable #1055 (AddSerialVersionUidToSerializable); wiring those waits on these.

• A10 CommonStaticAnalysis, CodeCleanup and JavaBestPractices overlap

  Details

  • 11 recipes are listed in both CSA and CC, and the lists have drifted (HideUtilityClassConstructor, A2).
  • JBP re-lists 7 CSA entries instead of including a shared composite, so runs that chain CSA and JBP visit UnnecessaryParentheses, EqualsAvoidsNull and others twice.
  • Decision: have CC include a shared composite or document that CSA and CC are alternatives, and coordinate with rewrite-migrate-java.

• A11 ReplaceValidateNotNullHavingSingleArgWithObjectsRequireNonNull matches every overload; move the Commons Lang recipes to rewrite-apache

  Details

  • Where: java-best-practices.yml:50-62.
  • Problem (Confirmed): the pattern is notNull(..), so run on its own the recipe turns Validate.notNull(o, "%s is null", n) into Objects.requireNonNull(o, "%s is null", n), which doesn't exist. The parent composite only works because it lists the varargs recipe first.
  • Also (Confirmed): the varargs recipe moves message arguments into a lambda, so reassigned locals no longer compile (ReplaceValidateNotNullHavingVarargsWithObjectsRequireNonNull:57-69).
  • Fix: use notNull(java.lang.Object), eager String.format, and move the trio to rewrite-apache's PreferJavaStandardLibrary.

B. CommonStaticAnalysis: statements and control flow

• B1 Five recipes return null for a statement in a brace-less body (PROBED) Don’t leave a null body when removing the only statement of a brace-less if or loop #1149

  Details

  • Where: RemoveCallsToSystemGc:61-65, RemoveSystemOutPrintln:70-77, RemoveCallsToObjectFinalize:62-66, RemoveSelfAssignment:49-55, RemoveUnneededAssertion:103-106. The first and third are in JBP; the others are unwired.
  • Problem: returning null only works inside a J.Block. As the body of a brace-less if, else or loop, the tree is left with a null body.
    • if (low) System.gc(); c.clear(); makes the follow-up EmptyBlockVisitor.visitIf (:108) throw an NPE. The test harness hides the cause, because its error handler NPEs on J.If.toString() too.
    • if (d) System.out.println(x); else log(x); becomes if (d) else log(x);.
  • Same family: SimplifyCompoundVisitor:33-36,52-55 (return b &= true; → return ;), and LambdaBlockToExpression:56-61 (() -> { return; }, CSA).
  • Fix: return an empty J.Block when the parent isn't a block, as RemoveUnnecessaryBitwiseOperation:128-130 does. Bail out in expression position.

• B2 CatchClauseOnlyRethrows: inlining an emptied try (PROBED)

  Details

  • Where: CatchClauseOnlyRethrows.java:52-64,67-85.
  • Problem:
    • Two sibling try { String s = r.readLine(); … } catch (IOException e) { throw e; } blocks give duplicate String s declarations.
    • Under case 1: the result is a bare try { r.close(); }.
    • Kotlin (PROBED): val text = try { load() } catch (e: IOException) { throw e } becomes val text = try { load() }, which doesn't compile, and the removed import leaves a blank line.
    • Comments in the try's prefix are dropped.
  • Fix: inline only when the body declares nothing and the parent is a block; otherwise keep a plain { … } block. For a Kotlin try expression, replace it with its body when that is a single expression, and otherwise leave it. UnnecessaryCatch:66-78 has the same inlining code (D6).

• B3 EmptyBlock deletes try {} finally {…}, synchronized and switch selectors (PROBED) B3: EmptyBlock keeps finally statements and switch selector side effects, and leaves synchronized alone #1117

  Details

  • Where: EmptyBlockVisitor.java:86-89,183-186,192-195; also in CC.
  • Problem:
    • try { } finally { r.run(); } is removed whole, so r.run() is lost.
    • synchronized (lock) {} and switch (sel()) {} are removed with their lock and selector side effects.
  • Also (Confirmed, :112-152,218-230): the if-inversion has the NaN flaw from C3, and side-effect extraction turns if (m.find() && m.group(1) != null) {} into m.find(); m.group(1);, which throws when there's no match.
  • Fix: keep the finally statements, extract side effects as the if path does, skip floating-point inversion, and keep &&/|| intact.
  • Related: EmptyBlock and RemoveMethodsOnlyCallSuper delete blocks and methods whose only content is a comment #1050.

• B4 FallThrough adds unreachable breaks, and break inside switch expressions (PROBED)

  Details

  • Where: FallThroughVisitor.java:183-225 (no J.Synchronized, J.DoWhileLoop or J.Yield), :279-293, :72-77 (firstEnclosing(J.Switch.class) finds the outer switch); also in CC.
  • Problem:
    • case 1: synchronized (lock) { return 1; } gets an unreachable break;.
    • A colon-style switch expression nested in a switch statement gets break; after each yield (illegal) and after return r;.
    • The recipe needs a second cycle; cycle 2 adds a second break; at each place, because FindLastLineBreaks compares case_ == scope against the already-modified case.
    • do { … } while (true) is also missed (Confirmed).
  • Also (Confirmed, skip-only): an if/else counts as breaking when only the then branch breaks (:188-191).
  • Related: FallThrough should not add break when switch is last statement in a method requiring a return #229.

• B5 RedundantFileCreation: new FileInputStream(new File(uri)) → new FileInputStream(uri) (PROBED) B5: RedundantFileCreation only unwraps new File(String) #1124

  Details

  • Where: RedundantFileCreation.java:42-53.
  • Problem: any one-argument new File(..) is unwrapped, including File(URI), but FileInputStream has no URI constructor. Type validation doesn't catch it.
  • Fix: match java.io.File <constructor>(java.lang.String) with a MethodMatcher.

• B6 RemoveUnusedLocalVariables drops ++/--/compound-assignment side effects (PROBED) A7 + B6: scope follow-up visitors to what each recipe changed #1122 A7 + B6 follow-up: side effects, comments and formatting scope #1123

  Details

  • Where: RemoveUnusedLocalVariables.java:216-241 (mightSideEffect only flags invocations, new and assignments), :176-184. Reached from CSA through InlineVariable (A7), though it is commented out there.
  • Probe: int next() { int unused = counter++; return counter; }, in a file where InlineVariable fires: the increment is gone.
  • Fix: use SideEffects.mayHaveSideEffects for initializers and assignment references.
  • Also (Confirmed): VariableReferences:88-142 matches by simple name, so count = 5; int count = 0; with a field count removes the field write. Fix: compare getFieldType(), and check isModifying() in visitUnary.
  • Also (Suspected, :192-212): removed comments are parked on the enclosing class and land on the next member. Without a class (a Groovy script) dropParentUntil throws.

• B7 RemoveMethodsOnlyCallSuper removes visibility-widening overrides (PROBED)

  Details

  • Where: RemoveMethodsOnlyCallSuper.java:196-220; in CC.
  • Problem: widensVisibility only looks at the direct supertype's declared methods. class Sub extends Base { @Override public Object clone() … { return super.clone(); } } loses its public clone(), and callers of s.clone() stop compiling.
  • Fix: compare against TypeUtils.findOverriddenMethod(..).
  • Related: EmptyBlock and RemoveMethodsOnlyCallSuper delete blocks and methods whose only content is a comment #1050.

• B8 MinimumSwitchCases evaluates the selector twice and retargets nested breaks (PROBED)

  Details

  • Where: MinimumSwitchCases.java:124,135,156 and :95-102,286-301.
  • Problem:
    • switch (it.next()) { case 1: … case 2: … } becomes if (it.next() == 1) … else if (it.next() == 2) ….
    • default: if (skip) break; b(); inside a for loop keeps the break, which now exits the loop and skips the code after the switch.
  • Also (Confirmed):
    • The enum branch builds comparisons without parentheses.
    • The default-only unwrap (:59-73) drops the selector's side effects and can clash with later declarations.
    • redeclareAssignments (:316-344) redeclares on every top-level assignment, so x = 2; x = 3; becomes two int x declarations.
  • Suspected: arrow cases with default first (:80,116-157) pass default to the template. Groovy case String: (isCase semantics) becomes ==; only C# is excluded.
  • Fix: bail on selectors with side effects and on any non-trailing break. For Groovy, only convert cases whose labels are literals, where isCase and == agree.

• B9 NoRedundantJumpStatements removes continue outer; (PROBED)

  Details

  • Where: NoRedundantJumpStatements.java:56-80.
  • Problem: labels are ignored. if (stop) { continue outer; } else { work(); } becomes if (!stop) { work(); }, so the inner loop keeps going.
  • Also (Confirmed, :93-94): removing a trailing return; drops its comments, and UnnecessaryReturnAsLastStatement:59-62,75-76 does the same.
  • Fix: require getLabel() == null, and move comments to the block end.

C. CommonStaticAnalysis: expressions and declarations

• C1 NoValueOfOnStringType: String.valueOf(a) + String.valueOf(b) → a + b (PROBED) C1: NoValueOfOnStringType keeps one String.valueOf when both concatenation operands are valueOf calls #1127

  Details

  • Where: NoValueOfOnStringType.java:110-120.
  • Problem: each operand sees the original parent binary, in which the other operand is still a String-typed valueOf, so both are removed. The string concatenation becomes int addition.
  • Fix: don't remove when the other operand is itself a removable String.valueOf.

• C2 NoEqualityInForCondition doesn't flip the operator when the variable is on the right (PROBED) C2: NoEqualityInForCondition flips the operator when the loop variable is on the right #1128

  Details

  • Where: NoEqualityInForCondition.java:61-70,90-101.
  • Problem: for (int i = 0; 10 != i; i++) becomes 10 < i, so the loop never runs. Matching is by simple name.
  • Fix: flip the operator or skip the case, and use SemanticallyEqual.

• C3 BooleanChecksNotInverted breaks the NaN check (PROBED) Preserve NaN semantics when simplifying negated comparisons #1142

  Details

  • Where: BooleanChecksNotInverted.java:61-69; the same inversion is in EmptyBlockVisitor:131-152.
  • Problem: if (!(f > 0)) throw … with a double f becomes if (f <= 0), which no longer rejects NaN. That idiom is often deliberate.
  • Fix: skip relational operators with a floating-point or unknown operand.

• C4 EqualsAvoidsNull removes the null check before contentEquals (PROBED) Keep the null check before contentEquals in EqualsAvoidsNull #1133

  Details

  • Where: EqualsAvoidsNull.java:88-101 with :48,140; also in CC and JBP.
  • Problem: s != null && s.contentEquals("abc") becomes "abc".contentEquals(s), which throws an NPE for null, unlike equals.
  • Fix: keep the null check for CONTENT_EQUALS.

• C5 NoDoubleBraceInitialization adds the collection as receiver to calls on the enclosing class (PROBED)

  Details

  • Where: NoDoubleBraceInitialization.java:142-170.
  • Problem: new HashMap<>() {{ put(key(), "b"); }} becomes m.put(m.key(), "b"). The recipe also reformats the unrelated key() method.
  • Also (Confirmed, :130-134,211-219): for method locals, the statements are spliced into the enclosing block, so a local declared inside the braces clashes with a later one.
  • Fix: add the select only when the declaring type is the collection supertype, and wrap or bail when the braces declare variables.
  • Kotlin (Suspected): object : HashMap<String, String>() { init { put(..) } } may match too; add a Kotlin test.

• C6 NoEmptyCollectionWithRawType doesn't compile under a cast (PROBED)

  Details

  • Where: NoEmptyCollectionWithRawType.java:80-107.
  • Problem: (List<String>) Collections.EMPTY_LIST becomes (List<String>) Collections.emptyList(). javac reports List<Object> cannot be converted to List<String>, because a cast isn't a poly context. Type validation doesn't catch it.
  • Fix: use a type witness and drop the cast, or skip cast contexts.

• C7 NoPrimitiveWrappersForToStringOrCompareTo: new Float(d).toString() → Float.toString(d) (PROBED) Preserve Float double constructor conversions (1116 C7) #1143 (Float constructor fix; lambda concerns remain open)

  Details

  • Where: NoPrimitiveWrappersForToStringOrCompareTo.java:77-107.
  • Problem: a double passed where float is expected is a lossy-conversion error. The same applies to compareTo → Float.compare.
  • Also (Confirmed, LambdaBlockToExpression:51-82): trailing comments in a lambda block are dropped, and new Task(() -> { compute(); }) can switch to a Callable constructor overload, because only method invocations are guarded.
  • Fix: bail on a narrowing argument, or insert a cast.

• C8 ExplicitInitialization treats 0x100000000L as zero (PROBED)

  Details

  • Where: ExplicitInitializationVisitor.java:106-111 (intValue() == 0); also in CC.
  • Problem: private long mask = 0x100000000L; becomes private long mask;.
  • Also (Confirmed): Lombok @FieldDefaults(makeFinal = true) classes lose initializers that the generated final fields need (:79-82,90).
  • Also (Suspected): C# const int Zero = 0 loses its required initializer.
  • Fix: longValue() == 0, skip @FieldDefaults(makeFinal = true), and skip const.

• C9 FinalizePrivateFields finalizes fields read by initializers, and breaks Kotlin (PROBED) Fix FinalizePrivateFields for initializer reads, Kotlin var and Groovy (C9) #1139

  Details

  • Where: FinalizePrivateFields.java:91-101,170-219 (reads at depth 0 aren't tracked), :50 (only JavaScript is excluded), :117-122,146; also in CC.
  • Java: private int x; private int y = x + 1; A() { x = 5; } makes both fields final, so y's initializer reads an unassigned final. In CSA ExplicitInitialization runs first and strips = 0, which makes this more likely.
  • Kotlin: private var count = 0 becomes private var val count = 0, because var is a LanguageExtension modifier and val prints for any Final.
  • Groovy (Suspected): private fields aren't enforced, so outside writes hit a ReadOnlyPropertyException.
  • Fix: exclude fields read directly in instance initializers. For Kotlin, replace the var modifier with val instead of appending a Final one, and add a Kotlin test. For Groovy, skip fields that may be written dynamically, or skip Groovy if that can't be told apart.

• C10 UseDiamondOperator adds a diamond to an anonymous class assigned to a wildcard type (PROBED) Keep type arguments of an anonymous class assigned to a wildcard type in UseDiamondOperator (C10 of #1116) #1161

  Details

  • Where: UseDiamondOperator.java:94-104,227-248.
  • Problem: on Java 9+, c = new Consumer<String>() { public void accept(String s) {} } with Consumer<?> c becomes new Consumer<>() {…}, which infers Object and no longer overrides. The argument path guards against wildcards (:130-143); the assignment path doesn't.
  • Fix: bail when a target type argument is a wildcard or type variable.

• C11 UseJavaStyleArrayDeclarations gives every variable the array type (PROBED) C11: UseJavaStyleArrayDeclarations only rewrites declarations with matching dimensions #1132

  Details

  • Where: UseJavaStyleArrayDeclarations.java:49-81.
  • Problem: int a, b[]; becomes int[] a, b;, and for (int i = 0, a[] = {1, 2, 3}; …) becomes for (int[] i = 0, a = …). MultipleVariableDeclarations only splits block statements, so for initializers reach this recipe unsplit in CSA.
  • Fix: only rewrite when every variable has the same number of dimensions.

• C12 ReplaceClassIsInstanceWithInstanceof produces instanceof that doesn't compile (PROBED) Skip ReplaceClassIsInstanceWithInstanceof on incompatible types or primitives (C12 of #1116) #1164

  Details

  • Where: ReplaceClassIsInstanceWithInstanceof.java:64-73,90-98; tests at :34-60 (@DocumentExample) and :120-135.
  • Problem: Integer.class.isInstance(s) with String s becomes s instanceof Integer (incompatible types), and String.class.isInstance(o.hashCode()) becomes o.hashCode() instanceof String. The tests assert both.
  • Fix: skip primitives and provably unrelated types, and fix the tests.

• C13 ReplaceLambdaWithMethodReference qualifies this with a superclass inside nested lambdas (PROBED) Fix qualified this resolution in ReplaceLambdaWithMethodReference #1153

  Details

  • Where: ReplaceLambdaWithMethodReference.java:235-248; also in JBP.
  • Problem: the nearest J.Lambda is treated as the owner of this. ls.forEach(l -> l.forEach(s -> log(s))), with log inherited from Base, becomes l.forEach(Base.this::log), which doesn't compile. Own methods get a needless A.this::m.
  • Fix: don't stop at lambdas; use this when the innermost class is assignable to the declaring type.

• C14 ReplaceStringBuilderWithString drops a CharSequence seed and copies literal source text (PROBED) Fix ReplaceStringBuilderWithString literal conversion, CharSequence seeds and Kotlin output #1150

  Details

  • Where: ReplaceStringBuilderWithString.java:173-183,95-118,127,168; also in CC and JBP.
  • Problem:
    • new StringBuilder(prefix).append("x") with a CharSequence prefix becomes "x".
    • .append('"').append(n) becomes """ + n, which doesn't compile; append(10L) becomes "10L".
    • Kotlin (PROBED): StringBuilder().append(count).append(" items").toString() becomes String.valueOf(count) + " items"; kotlin.String has no valueOf.
  • Fix: bail on non-String, non-int constructor arguments, escape converted literals or skip them, and emit count.toString() (or a string template) in Kotlin. Preserve argument comments and ranged append behavior in string-builder conversion #994 covers argument comments and ranged append, not these.

• C15 RenameMethodsNamedHashcodeEqualOrToString gives weaker access, hides with static, and skips other files (PROBED)

  Details

  • Where: RenameMethodsNamedHashcodeEqualOrToString.java:62-72.
  • Problem: package-private int hashcode() becomes int hashCode() (weaker access than Object's), and static String tostring() becomes static String toString(). Both fail to compile. The rename runs through doAfterVisit, so callers in other files keep the old name (Confirmed).
  • Fix: only rename public instance methods, and rename callers across files or bail. Avoid existing methods when renaming near-miss Object methods #974 and Avoid non-compiling collisions when renaming near-miss Object methods #987 cover name collisions, not these.

• C16 SimplifyArraysAsList unwraps primitive arrays (PROBED) SimplifyArraysAsList: do not unwrap primitive arrays (C16 from #1116) #1125

  Details

  • Where: SimplifyArraysAsList.java:60-68.
  • Problem: Arrays.asList(new int[]{1, 2}) is a one-element List<int[]>. After the rewrite it is Arrays.asList(1, 2), a two-element List<Integer>.
  • Fix: require a reference component type.

• C17 SimplifyBooleanExpression on Kotlin Boolean? (PROBED) C17: SimplifyBooleanExpression keeps == true on Kotlin calls returning Boolean? #1126

  Details

  • Where: SimplifyBooleanExpression.java:59-65.
  • Problem: if (flag() == true) with fun flag(): Boolean? becomes if (flag()), a type mismatch. The method-invocation override returns before the nullable-type check from SimplifyBooleanExpression: skip nullable boolean comparisons in Kotlin #839.
  • Fix: return !isNullSafe && super.shouldSimplifyEqualsOn(j);.
  • Related: SimplifyBooleanExpression ignore excessively deep boolean expressions #387.

• C18 SimplifyBooleanReturn returns a non-boolean in Python (PROBED)

  Details

  • Where: SimplifyBooleanReturn.java:64-97; SimplifyBooleanReturnTest:341-378 expects return re.match(...).
  • Problem: if items: return True / return False becomes return items, which returns the list. Groovy def methods and JS/TS have the same risk (Suspected).
  • Also (Confirmed, :81,208-209): an empty else {} throws IndexOutOfBoundsException.
  • Fix: require a boolean condition type, and guard the empty block.

• C19 MethodNameCasing produces duplicate methods and renames native methods (PROBED)

  Details

  • Where: MethodNameCasing.java:99-127.
  • Problem: in main sources, void Get_Value(){} void GetValue(){} gives two getValue() methods, and private native void native_init(); becomes nativeInit, which fails with UnsatisfiedLinkError at runtime.
  • Also (Confirmed): there is no override check, so a protected base method is renamed while its public override isn't.
  • Not reproduced: a Kotlin fun Get_Name() was not renamed with default options.
  • Fix: track claimed names per type, skip native methods and overrides, and treat each override family all-or-nothing.

• C20 InlineVariable changes semantics (Confirmed)

  Details

  • Where: InlineVariable.java:64-84; also in rewrite-rewrite's best practices.
  • Problem:
    • long v = 1; return v; in an Object method becomes return 1;, which returns an Integer.
    • Runnable r = () -> {}; return r; in an Object method becomes return () -> {};, which doesn't compile.
    • res = compute(); return res; with finally { log(res); } now logs 0.
  • Fix: bail when the declared type differs from the initializer's, on lambdas and method references, and when the variable is read elsewhere.
  • Related: InlineVariable to handle more complex expressions too #803.

• C21 Smaller CommonStaticAnalysis items (Confirmed unless noted)

  Details

  • ForLoopIncrementInUpdate:84-88: inserts the moved increment in alphabetical order, so for (…; count = i) { …; i++; } becomes count = i, i++; the @DocumentExample asserts the sorting. It also drops the comment above the increment.
  • FixStringFormatExpressions:97-103: dropping "unused" arguments drops side effects (n[0]++) and can change varargs binding for a trailing array.
  • ExternalizableHasNoArgsConstructor:65,118-141: adds public X() {} when the parent's no-arg constructor is private, to enums, and to records. In a Kotlin class with only secondary constructors it inserts a Java constructor (Suspected).
  • FinalClassVisitor:73,136-137: final non-sealed class.
  • CollectionToArrayShouldHaveProperType:139-141: NPE on an implicit this (toArray() in an ArrayList subclass).
  • ChainStringBuilderAppendCalls:110-115,182: comments on operands are dropped, and a char[] operand is now appended by content.
  • MultipleVariableDeclarationsVisitor:52,62: comments after the commas are dropped.
  • RemoveExtraSemicolons:73-83,113-123: comments on a trailing empty statement are dropped.
  • UsePortableNewlines:75-89: nested format calls in the arguments are never converted.
  • UnnecessaryCloseInTryWithResources:67-88: NPE on a field-access resource (try (this.in)); check against UnnecessaryCloseInTryWithResources: only remove a trailing close of a Closeable #978.
  • ReplaceStringConcatenationWithStringValueOf:62-71: unwraps every parenthesized String.valueOf, not only its own.
  • StaticMethodNotFinal:51-53 (Suspected): a double space when final was the first modifier.
  • Related: Keep char[] and Groovy array operands out of String.valueOf conversion #975 (ReplaceStringConcatenationWithStringValueOf), Preserve escaped backslash-newline text in UsePortableNewlines #1006 (UsePortableNewlines).