Skip to content

Improvements | String Literals on the left on comparisons (SonarQube)#1077

Merged
ulvii merged 1 commit intomicrosoft:devfrom
dvmarcilio:string_literal_left_on_comparisons
Jul 4, 2019
Merged

Improvements | String Literals on the left on comparisons (SonarQube)#1077
ulvii merged 1 commit intomicrosoft:devfrom
dvmarcilio:string_literal_left_on_comparisons

Conversation

@dvmarcilio
Copy link
Copy Markdown
Contributor

@dvmarcilio dvmarcilio commented Jun 7, 2019

Transforming from someString.equals("string") to "string".equals(someString). This avoids checking for null and also prevents NullPointerException being thrown.

These fixes multiple SonarQube violations of the rule: Strings literals should be placed on the left side when checking for equality

@msftclas
Copy link
Copy Markdown

msftclas commented Jun 7, 2019 •
edited
Loading

CLA assistant check
All CLA requirements met.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Jun 7, 2019 •
edited
Loading

Codecov Report

Merging #1077 into dev will decrease coverage by 0.01%.
The diff coverage is 55.35%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev    #1077      +/-   ##
============================================
- Coverage     53.24%   53.22%   -0.02%     
+ Complexity     3190     3187       -3     
============================================
  Files           119      119              
  Lines         28118    28118              
  Branches       4694     4694              
============================================
- Hits          14971    14967       -4     
+ Misses        10933    10928       -5     
- Partials       2214     2223       +9
Impacted Files Coverage Δ Complexity Δ
.../com/microsoft/sqlserver/jdbc/SQLServerDriver.java 83.4% <0%> (ø) 37 <0> (ø) ⬇️
...m/microsoft/sqlserver/jdbc/SQLServerDataTable.java 81.75% <100%> (ø) 42 <0> (ø) ⬇️
...oft/sqlserver/jdbc/SQLServerPreparedStatement.java 53.38% <15.38%> (+0.06%) 219 <0> (+3) ⬆️
...sqlserver/jdbc/SQLServerBulkBatchInsertRecord.java 30.06% <25%> (ø) 17 <0> (ø) ⬇️
...oft/sqlserver/jdbc/SQLServerParameterMetaData.java 73.86% <61.53%> (ø) 103 <0> (ø) ⬇️
...osoft/sqlserver/jdbc/SQLServerSpatialDatatype.java 85.49% <77.77%> (ø) 289 <0> (ø) ⬇️
...m/microsoft/sqlserver/jdbc/SQLServerStatement.java 68.36% <80%> (-0.2%) 170 <0> (-2)
...om/microsoft/sqlserver/jdbc/ReaderInputStream.java 43.95% <0%> (-3.3%) 15% <0%> (ø)
...ava/com/microsoft/sqlserver/jdbc/ScrollWindow.java 75.3% <0%> (-2.47%) 18% <0%> (-2%)
...a/com/microsoft/sqlserver/jdbc/PLPInputStream.java 59.76% <0%> (-2.37%) 34% <0%> (-2%)
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 156267c...2266e15. Read the comment docs.

@cheenamalhotra
Copy link
Copy Markdown
Member

cheenamalhotra commented Jun 7, 2019

Hi @dvmarcilio

There are more occurrences of such comparisons that you may wanna fix altogether for this rule.

@dvmarcilio
Copy link
Copy Markdown
Contributor Author

dvmarcilio commented Jun 8, 2019 •
edited
Loading

Hi @cheenamalhotra

I'm covering all occurrences that showed up in Sonar. Unfortunately, the building is failing, and the error message does not provide much information:

[ERROR] testDatabaseMetaData Time elapsed: 1.512 s <<< ERROR!
com.microsoft.sqlserver.jdbc.SQLServerException: Transaction (Process ID 56) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction. at com.microsoft.sqlserver.jdbc.connection.DBMetadataTest.testDatabaseMetaData(DBMetadataTest.java:52)

Is that on my end? Any directions? Thanks!

@dvmarcilio
Copy link
Copy Markdown
Contributor Author

dvmarcilio commented Jun 10, 2019

Nevermind the previous comment. The building is passing now.

@lilgreenbird
Copy link
Copy Markdown
Contributor

lilgreenbird commented Jul 3, 2019

hi @dvmarcilio,
There are some conflicts in this branch if you could please resolve it we can review and look into merging it. Thanks!

@dvmarcilio
Copy link
Copy Markdown
Contributor Author

dvmarcilio commented Jul 3, 2019

Hi @lilgreenbird. I resolved the conflicts. I think the build error is not on my end.
Please let me know if there is anything else I can do.

@ulvii ulvii requested review from lilgreenbird and rene-ye July 4, 2019 20:41
@ulvii
Copy link
Copy Markdown
Contributor

ulvii commented Jul 4, 2019

Thanks for the contribution, @dvmarcilio 👍.

@ulvii ulvii merged commit 9da67c0 into microsoft:dev Jul 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@rene-ye rene-ye rene-ye approved these changes

@lilgreenbird lilgreenbird lilgreenbird approved these changes

@ulvii ulvii ulvii approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@dvmarcilio @msftclas @codecov-io @cheenamalhotra @lilgreenbird @ulvii @rene-ye