Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spotbugs 4.1.4 reports EQ_UNUSUAL on Java 15 record #1367

Closed
dragonjuggler opened this issue Nov 9, 2020 · 6 comments · Fixed by #1503
Closed

Spotbugs 4.1.4 reports EQ_UNUSUAL on Java 15 record #1367

dragonjuggler opened this issue Nov 9, 2020 · 6 comments · Fixed by #1503
Labels

Comments

@dragonjuggler
Copy link

This will be reported on a very simple record:

public record SomeClass(String firstName, String lastName, String middleName) { }

@welcome
Copy link

welcome bot commented Nov 9, 2020

Thanks for opening your first issue here! 😃
Please check our contributing guideline. Especially when you report a problem, make sure you share a Minimal, Complete, and Verifiable example to reproduce it in this issue.

@McPringle
Copy link

Same with Spotbugs 4.2.0:

public record Metadata(
    int zoom, double minX, double maxX, double minY, double maxY,
    long minTime, long maxTime, double speedup
) { }

Spotbugs reports the following warning:

app.gpx_animator.core.renderer.Metadata.equals(Object) is unusual
--
Bug type EQ_UNUSUAL (click for details)
In class app.gpx_animator.core.renderer.Metadata
In method app.gpx_animator.core.renderer.Metadata.equals(Object)
At Metadata.java:[line 4]

Details:

This class doesn't do any of the patterns we recognize for checking
that the type of the argument is compatible with the type of the this
object. There might not be anything wrong with this code, but it is
worth reviewing.

My expectation: Spotbugs should accept the default implementation of Java records.

@lazystone
Copy link

Same happens on recently released JDK 16+SpotBugs 4.2.2

@KengoTODA KengoTODA added Java-14 and removed Java-15 labels Apr 10, 2021
@KengoTODA
Copy link
Member

note: equals() invokes dynamically generated codes, so static analysis cannot handle it property:
https://dzone.com/articles/a-first-look-at-records-in-java-14

public final boolean equals(java.lang.Object);
    descriptor: (Ljava/lang/Object;)Z
    flags: (0x0011) ACC_PUBLIC, ACC_FINAL
    Code:
      stack=2, locals=2, args_size=2
         0: aload_0
         1: aload_1
         2: invokedynamic #30,  0             // InvokeDynamic #0:equals:(LRecordClass;Ljava/lang/Object;)Z
         7: ireturn

SpotBugs needs to ignore classes that extend java.lang.Record.

KengoTODA added a commit that referenced this issue Apr 10, 2021
@electrum
Copy link

Note that users can write their own equals and hash code implementations for records.

@McPringle
Copy link

SpotBugs should only execute the equals check if the class extending Record has the implementation of an own equal method.

KengoTODA added a commit that referenced this issue Apr 12, 2021
@KengoTODA KengoTODA linked a pull request Apr 12, 2021 that will close this issue
1 task
gamesh411 pushed a commit to gamesh411/spotbugs that referenced this issue Apr 15, 2021
okotsopoulos added a commit to DataBiosphere/jade-data-repo that referenced this issue Jun 14, 2022
Includes bugfix: 'false positive EQ_UNUSUAL with record classes' spotbugs/spotbugs#1367
Fix spotbugs errors that result
OOS to upgrade further, 4.3.0 yields 955 violations
okotsopoulos added a commit to DataBiosphere/jade-data-repo that referenced this issue Jun 16, 2022
* Use passiveExpiringMap for IamService cache

Eliminates the need for AuthorizedCacheValue, we now delegate expiration to the map implementation.
Convert AuthorizedCacheKey to record to eliminate boilerplate.

* Remove references to now-unused datarepo.authCacheSize config

Not used in PassiveExpiringMap.

* Add unit test

* Update spotbugs 4.2.2 -> 4.2.3

Includes bugfix: 'false positive EQ_UNUSUAL with record classes' spotbugs/spotbugs#1367
Fix spotbugs errors that result
OOS to upgrade further, 4.3.0 yields 955 violations

* Fix violations exposed by :spotbugsTest

* spotlessJavaApply

* Suppress Spotbugs DMI_RANDOM_USED_ONLY_ONCE warnings rather than misuse SecureRandom

* Add warnings around cache TTL in Swagger profile policy member change endpoints
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants