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

Fix single | to || for conditional checks to comply with Java S2638 standard #7272

Closed
wants to merge 2 commits into from

Conversation

ASangelina
Copy link

This PR addresses the Java S2638 standard, which recommends using the short-circuit operator || instead of the bitwise operator | for conditional checks. The following change was made:

Before:
if (localInputFuture == null | localExceptionType == null | localFallback == null // This check, unlike all the others, is a volatile read || isCancelled()) { return; }
After:
if (localInputFuture == null || localExceptionType == null || localFallback == null // This check, unlike all the others, is a volatile read || isCancelled()) { return; }

The use of || ensures that the evaluation short-circuits, improving performance and avoiding potential null pointer exceptions. This change helps to improve the safety and efficiency of the code by following recommended best practices.

Please review the changes and provide your feedback.

Copy link

google-cla bot commented Jun 24, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@cpovirk
Copy link
Member

cpovirk commented Jun 24, 2024

This is intentional, but we should note that in AbstractCatchingFuture, too.

I will add that, in general, I have found most Sonar checks to be unhelpful, so we are unlikely to accept PRs except in cases in which there are actual risks of bugs or significant performance improvements. I'm sure that there are various Sonar checks that meet that standard, and maybe Sonar offers a way to prioritize those. I haven't used it enough to know.

copybara-service bot pushed a commit that referenced this pull request Jun 24, 2024
This just came up again in #7272 and b/348626771 / cl/645390331.

Fixes #7272

RELNOTES=n/a
PiperOrigin-RevId: 646079644
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.

None yet

2 participants