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: handle error-prone warnings #1334

Merged
merged 3 commits into from Jan 9, 2024
Merged

Conversation

sinelaw
Copy link
Contributor

@sinelaw sinelaw commented Nov 21, 2023

ErrorProne flags several issues in the codebase, this pull request fixes some of them:

  • Missing @Override annotations where needed
  • Using of deprecated or incompatibel APIs for Base64 encoding

There's also a warning by maven due to conflicting / duplicate dependencies in pom.xml, this is also fixed.

…use warnings

This prevents the following errors:

$ mvn install -DskipTests=true
[INFO] Scanning for projects...
[WARNING]
[WARNING] Some problems were encountered while building the effective model for com.google.auth:google-auth-library-oauth2-http:jar:1.20.1-SNAPSHOT
[WARNING] 'dependencies.dependency.(groupId:artifactId:type:classifier)' must be unique: junit:junit:jar -> duplicate declaration of version (?) @ line 249, column 17
[WARNING] 'build.plugins.plugin.(groupId:artifactId)' must be unique but found duplicate declaration of plugin org.apache.maven.plugins:maven-failsafe-plugin @ line 193, column 15
[WARNING]
[WARNING] It is highly recommended to fix these problems because they threaten the stability of your build.
[WARNING]
[WARNING] For this reason, future Maven versions might no longer support building such malformed projects.
[WARNING]
@sinelaw sinelaw requested review from a team as code owners November 21, 2023 19:16
@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Nov 21, 2023
@sinelaw sinelaw changed the title Fixes to error-prone warnings fix: handle error-prone warnings Nov 21, 2023
@ejona86
Copy link
Contributor

ejona86 commented Nov 22, 2023

The Base64 thing is about Android compatibility.

@sinelaw
Copy link
Contributor Author

sinelaw commented Nov 22, 2023

The Base64 thing is about Android compatibility.

Indeed, but it also triggers error prone on https://errorprone.info/bugpattern/AndroidJdkLibsChecker

@ejona86
Copy link
Contributor

ejona86 commented Nov 22, 2023

The Base64 thing is about Android compatibility.

Indeed, but it also triggers error prone on https://errorprone.info/bugpattern/AndroidJdkLibsChecker

Yet the proper solution in many projects is to turn off the check. API desugaring supports this on Android. For a library, it's mostly a question if you require your users to use API desugaring.

You can avoid the API or use it. The point is that it is a decision that is not made "because Error Prone complained." Error Prone is well beyond the "only tell you about highly likely bugs" that it set out to be. You have to tweak and tune it to your usages and be willing to add ignores (as has been repeated to me multiple times when I complained that a suggestion was encouraging security vulnerabilities or was wrong). And some of this is auto-configured inside Google; sometimes an warning means you have a configuration mistake, not a code mistake.

Copy link

sonarcloud bot commented Nov 22, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@sinelaw
Copy link
Contributor Author

sinelaw commented Nov 22, 2023

Removed the Base64 commit from this PR, so we can discuss it separately.

@TimurSadykov TimurSadykov added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 24, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 24, 2023
Copy link
Member

@TimurSadykov TimurSadykov left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Copy link

sonarcloud bot commented Jan 9, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@TimurSadykov TimurSadykov merged commit 927cad8 into googleapis:main Jan 9, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants