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

false+ in IdentityBinaryExpression #945

Closed
grandinj opened this issue Feb 21, 2018 · 1 comment · Fixed by #948
Closed

false+ in IdentityBinaryExpression #945

grandinj opened this issue Feb 21, 2018 · 1 comment · Fixed by #948

Comments

@grandinj
Copy link

What version of Error Prone are you using?

2.2.0

Warning message is:

C:\Users\noel\Documents\GitHub\h2database-noel\h2\src\main\org\h2\table\TableLink.java:124: error: [IdentityBinaryExpression] A binary expression where both operands are the same is usually incorrect; the value of this expression is equivalent to rs.next().
if (rs.next() && rs.next()) {
^
(see http://errorprone.info/bugpattern/IdentityBinaryExpression)
1 error

Code looks like:
ResultSet rs = meta.getTables(null, originalSchema, originalTable, null);
if (rs.next() && rs.next()) {
throw DbException.get(ErrorCode.SCHEMA_NAME_MUST_MATCH, originalTable);
}

Probably ResultSet::next should be treated like Iterator next method

@cushon
Copy link
Collaborator

cushon commented Feb 21, 2018

I agree this is a false positive, thanks for the report.

One work-around is to refactor that to move some of the side-effects out of the binary expression, which may make it clearer to the reader that there are side-effects and checking the value twice is deliberate. E.g.:

boolean hasNext = rs.next();
if (hasNext && rs.next()) {
  // ...
}

@ronshapiro ronshapiro mentioned this issue Feb 22, 2018
ronshapiro pushed a commit that referenced this issue Feb 22, 2018
Fixes #945

RELNOTES: N/A

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=186547555
ronshapiro pushed a commit that referenced this issue Feb 22, 2018
Fixes #945

RELNOTES: N/A

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=186547555
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 a pull request may close this issue.

2 participants