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

Recognize OpenDaylight's Immutable interface contract #1703

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rovarga
Copy link
Contributor

@rovarga rovarga commented Sep 14, 2021

Aside from recognizing annotations, also treat any class that implements
an interface named Immutable as an immutable class. Fixes #1697.

Signed-off-by: Robert Varga robert.varga@pantheon.tech


Make sure these boxes are checked before submitting your PR -- thank you!

  • Added an entry into CHANGELOG.md if you have changed SpotBugs code

@rovarga
Copy link
Contributor Author

rovarga commented Sep 14, 2021

This will be reworked on top of #1704

@rovarga rovarga force-pushed the immutable-iface branch 4 times, most recently from 1a32099 to fbf488b Compare September 14, 2021 21:42
@rovarga rovarga changed the title Recognize Immutable interface contract Recognize OpenDaylight's Immutable interface contract Sep 14, 2021
@rovarga rovarga marked this pull request as ready for review September 14, 2021 21:56
@rovarga
Copy link
Contributor Author

rovarga commented Sep 14, 2021

This builds on top of #1705

ThrawnCA
ThrawnCA previously approved these changes Sep 15, 2021
OpenDaylight has a type-safe marker interface which acts as API contract
specification of effective immutability. Recognize this contract as
equivalent to an inherited @immutable annotation. Fixes spotbugs#1697.

Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
@ThrawnCA
Copy link
Contributor

Please avoid force-pushing; it wrecks the history.

Copy link
Member

@KengoTODA KengoTODA left a comment

Choose a reason for hiding this comment

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

The marker interface has no meaning in the bytecode level; annotated classes still be able to be mutable. Then I suggest not to merge this change but enhance the mutability check algorithm to reduce false positive and false negative.

@rovarga
Copy link
Contributor Author

rovarga commented Sep 16, 2021

Please avoid force-pushing; it wrecks the history.

Ack

@rovarga
Copy link
Contributor Author

rovarga commented Sep 16, 2021

The marker interface has no meaning in the bytecode level; annotated classes still be able to be mutable. Then I suggest not to merge this change but enhance the mutability check algorithm to reduce false positive and false negative.

That depends on what the expectations on MutableClasses are from three perspectives:

  • perfomance, i.e. the depth of reasoning we make about the JavaClass
  • accuracy of reported boolean
  • what assumptions are made about the JavaClass: is it any reference we see? is it something we can load? is it a class that is part of the project being analyzed or is it on classpath? have we already performed bytecode/dataflow analysis on it to conclusively prove it is mutable (or immutable)?

At the end of the day, we are trusting @Immutable annotations here and this just uses a different aspect of what shape the class has.

@github-actions github-actions bot added the Stale label Mar 5, 2024
@github-actions github-actions bot closed this Apr 9, 2024
@hazendaz hazendaz reopened this Apr 12, 2024
@github-actions github-actions bot removed the Stale label Apr 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Recognize OpenDaylight's Immutable interface contract
4 participants