Skip to content

Add SillyEqualsCheck recipe (RSPEC-S2159)#834

Merged
timtebeek merged 6 commits intomainfrom
tim/s2159-silly-equals
Apr 13, 2026
Merged

Add SillyEqualsCheck recipe (RSPEC-S2159)#834
timtebeek merged 6 commits intomainfrom
tim/s2159-silly-equals

Conversation

@timtebeek
Copy link
Copy Markdown
Member

Summary

Implements SonarQube rule S2159 "Silly equality checks should not be made", which detects .equals() calls that always return false.

The recipe transforms equals(null) checks to == null comparisons and replaces array.equals() with Arrays.equals(). It also flags comparisons between unrelated types and array-vs-non-array comparisons as search results.

Changes

  • New recipe: SillyEqualsCheck with comprehensive unit tests
  • Handles negation patterns (!x.equals(null) → x != null)
  • Auto-fixes: null checks and array equality comparisons
  • Search results: unrelated types and mixed array/non-array comparisons

Implements SonarQube rule S2159: detects `.equals()` calls that always return false due to incompatible types. The recipe transforms:
- `x.equals(null)` → `x == null` (with negation handling)
- `arr1.equals(arr2)` → `Arrays.equals(arr1, arr2)` for arrays

Unrelated type and array-vs-non-array comparisons are flagged as search results since they represent bugs that cannot be safely auto-fixed.
- Broaden dropParentUntil predicate to handle equals(null) in
  if-conditions, assignments, variable declarations, and other contexts
- Use Arrays.deepEquals() instead of Arrays.equals() for multidimensional
  arrays
- Add tests for multidimensional arrays, if-condition, and method select
@timtebeek
Copy link
Copy Markdown
Member Author

Validation: Tested at scale against Apache Maven (80 repos)

Ran SillyEqualsCheck against 80 Apache Maven repositories using pre-built LSTs from Moderne.

Results

  • 1 repository matched: apache/maven-2
  • 36 files changed — all in maven-model/src/test/java/org/apache/maven/model/
  • 79 repositories: no changes (clean pass, no errors)

All changes are .equals(null)== null

Every match follows the same pattern in testEqualsNullSafe() methods:

-        assertFalse( new ActivationFile().equals( null ) );
+        assertFalse( new ActivationFile() == null );

Affected test classes: ActivationFileTest, ActivationOSTest, ActivationPropertyTest, ActivationTest, BuildTest, CiManagementTest, ContributorTest, DependencyManagementTest, DependencyTest, DeploymentRepositoryTest, DeveloperTest, DistributionManagementTest, ExclusionTest, ExtensionTest, IssueManagementTest, LicenseTest, MailingListTest, ModelTest, NotifierTest, OrganizationTest, ParentTest, PluginConfigurationTest, PluginContainerTest, PluginExecutionTest, PluginManagementTest, PluginTest, PrerequisitesTest, ProfileTest, RelocationTest, ReportPluginTest, ReportSetTest, ReportingTest, RepositoryPolicyTest, RepositoryTest, ResourceTest, ScmTest, SiteTest

Assessment

  • No false positives
  • No crashes or errors
  • No array or unrelated-type matches in this corpus (expected for Maven model test code)
  • Formatting and whitespace preserved correctly

Resolve conflict in recipes.csv: take main's expanded descriptions,
re-add SillyEqualsCheck entry in alphabetical order.
Replace fragile dropParentUntil predicate listing 12 specific AST types
with a simple parent cursor walk through J.Parentheses nodes. This
cannot throw on unlisted AST contexts.
@timtebeek timtebeek moved this from In Progress to Ready to Review in OpenRewrite Apr 11, 2026
Copy link
Copy Markdown
Contributor

@steve-aom-elliott steve-aom-elliott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me. Might want to update the copyright years to 2026, but that's about it.

@timtebeek timtebeek merged commit df3a60b into main Apr 13, 2026
1 check passed
@timtebeek timtebeek deleted the tim/s2159-silly-equals branch April 13, 2026 15:53
@github-project-automation github-project-automation Bot moved this from Ready to Review to Done in OpenRewrite Apr 13, 2026
mergify Bot added a commit to robfrank/linklift that referenced this pull request May 3, 2026
… 2.30.0 to 2.34.0 [skip ci]

Bumps [org.openrewrite.recipe:rewrite-static-analysis](https://github.com/openrewrite/rewrite-static-analysis) from 2.30.0 to 2.34.0.
Release notes

*Sourced from [org.openrewrite.recipe:rewrite-static-analysis's releases](https://github.com/openrewrite/rewrite-static-analysis/releases).*

> 2.34.0
> ------
>
> What's Changed
> --------------
>
> * bugfix: false positive in AnnotateNullableParameters when parameter … by [`@​stefanodallapalma`](https://github.com/stefanodallapalma) in [openrewrite/rewrite-static-analysis#865](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/865)
>
> **Full Changelog**: <openrewrite/rewrite-static-analysis@v2.33.0...v2.34.0>
>
> 2.33.1
> ------
>
> **Full Changelog**: <openrewrite/rewrite-static-analysis@v2.33.0...v2.33.1>
>
> 2.33.0
> ------
>
> What's Changed
> --------------
>
> * A new test case for `SimplifyBooleanExpression` by [`@​greg-at-moderne`](https://github.com/greg-at-moderne) in [openrewrite/rewrite-static-analysis#848](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/848)
> * `InstanceOfPatternMatch` to avoid providing invalid variable definitions pointing to itself by [`@​greg-at-moderne`](https://github.com/greg-at-moderne) in [openrewrite/rewrite-static-analysis#849](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/849)
> * Enrich recipe descriptions with rationale by [`@​jkschneider`](https://github.com/jkschneider) in [openrewrite/rewrite-static-analysis#850](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/850)
> * Fix EmptyBlock, FinalClass, HideUtilityClassConstructor; add regression tests by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#851](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/851)
> * Fix switch-related recipe bugs ([#6](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/6), [#9](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/9), [#14](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/14), [#687](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/687)) by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#852](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/852)
> * Add space after // in single-line comments by [`@​AVIMTA`](https://github.com/AVIMTA) in [openrewrite/rewrite-static-analysis#854](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/854)
> * Fix InstanceOfPatternMatch duplicate pattern variable names with flow scoping by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#855](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/855)
> * Fix RenameExceptionInEmptyCatch crash on Kotlin/Groovy files by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#856](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/856)
> * Fix FinalizeLocalVariables crash on Kotlin/Groovy top-level variables by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#857](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/857)
> * Fix UnnecessaryExplicitTypeArguments within lambda return statements by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#858](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/858)
> * Add SillyEqualsCheck recipe (RSPEC-S2159) by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#834](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/834)
> * Add RemoveUnusedLabels recipe (RSPEC-S1065) by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#835](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/835)
> * Add S2209/S3252 recipe: Static members via class name by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#836](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/836)
> * Add S1185 recipe: Remove methods that only call super by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#837](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/837)
> * NeedBraces: add test for if-else, fix do-while by [`@​greg-at-moderne`](https://github.com/greg-at-moderne) in [openrewrite/rewrite-static-analysis#859](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/859)
> * Add tests confirming [#20](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/20) is fixed by [`@​greg-at-moderne`](https://github.com/greg-at-moderne) in [openrewrite/rewrite-static-analysis#860](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/860)
> * Handle chained addition in PreferIncrementOperator by [`@​greg-at-moderne`](https://github.com/greg-at-moderne) in [openrewrite/rewrite-static-analysis#816](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/816)
> * Fix UnnecessaryCatch for nested try-with-resources close() by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#863](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/863)
> * Fix FinalizePrivateFields breaking code with lambda reads in field initializers by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#862](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/862)
> * Regression test for `ReplaceStackWithDeque` crash on `this` argument by [`@​knutwannheden`](https://github.com/knutwannheden) in [openrewrite/rewrite-static-analysis#864](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/864)
>
> New Contributors
> ----------------
>
> * [`@​AVIMTA`](https://github.com/AVIMTA) made their first contribution in [openrewrite/rewrite-static-analysis#854](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/854)
>
> **Full Changelog**: <openrewrite/rewrite-static-analysis@v2.32.0...v2.33.0>
>
> 2.32.0
> ------
>
> What's Changed
> --------------
>
> * Remove [`@​Disabled`](https://github.com/Disabled) tests in `ReplaceStackWithDequeTest` by [`@​greg-at-moderne`](https://github.com/greg-at-moderne) in [openrewrite/rewrite-static-analysis#840](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/840)
> * A couple of test cases in `RemoveExtraSemicolonsTest`  are no longer expected to fail by [`@​greg-at-moderne`](https://github.com/greg-at-moderne) in [openrewrite/rewrite-static-analysis#841](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/841)
> * Fix compilation after new args added to Cs.CompilationUnit by [`@​greg-at-moderne`](https://github.com/greg-at-moderne) in [openrewrite/rewrite-static-analysis#842](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/842)
> * Improve AnnotateNullableParameters to avoid duplicate annotations and annotation placement issues by [`@​stefanodallapalma`](https://github.com/stefanodallapalma) in [openrewrite/rewrite-static-analysis#843](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/843)
> * Inline JavaTemplate fields at call sites by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#844](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/844)
> * Use `JavaTemplate.apply()` static method by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#846](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/846)
> * Improve AnnotateNullableMethods to avoid duplicate annotations and annotation placement issues by [`@​stefanodallapalma`](https://github.com/stefanodallapalma) in [openrewrite/rewrite-static-analysis#845](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/845)
> * Skip `InstanceOfPatternMatch` for try-with-resources casts by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#847](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/847)

... (truncated)


Commits

* [`90e4a60`](openrewrite/rewrite-static-analysis@90e4a60) bugfix: false positive in AnnotateNullableParameters when parameter … ([#865](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/865))
* [`b315bbc`](openrewrite/rewrite-static-analysis@b315bbc) Extract documentation examples from tests
* [`9ba38b4`](openrewrite/rewrite-static-analysis@9ba38b4) Add regression test for `ReplaceStackWithDeque` crash on `this` argument ([#864](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/864))
* [`78afc25`](openrewrite/rewrite-static-analysis@78afc25) Fix FinalizePrivateFields breaking compilation when field is read in a lambda...
* [`3b2d847`](openrewrite/rewrite-static-analysis@3b2d847) OpenRewrite recipe best practices
* [`2a0baac`](openrewrite/rewrite-static-analysis@2a0baac) Fix UnnecessaryCatch removing IOException for nested try-with-resources close...
* [`47094c2`](openrewrite/rewrite-static-analysis@47094c2) Handle chained addition in PreferIncrementOperator ([#816](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/816))
* [`e097fab`](openrewrite/rewrite-static-analysis@e097fab) Add tests confirming [#20](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/20) is fixed ([#860](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/860))
* [`3710118`](openrewrite/rewrite-static-analysis@3710118) Test for if-else, fix do-while NeedBraces ([#859](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/859))
* [`c6ebfd4`](openrewrite/rewrite-static-analysis@c6ebfd4) Add S1185 recipe: Remove methods that only call super ([#837](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/837))
* Additional commits viewable in [compare view](openrewrite/rewrite-static-analysis@v2.30.0...v2.34.0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants