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.

Sign up for GitHub

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

Jump to bottom

Fix unused variables warnings at java #26433

Merged
merged 3 commits into from
Nov 7, 2024
Merged

Fix unused variables warnings at java #26433

merged 3 commits into from
Nov 7, 2024

Conversation

AlexeyBarabash
Copy link
Contributor

@AlexeyBarabash AlexeyBarabash commented Nov 7, 2024 •
edited
Loading

This PR fixes all UnusedVariable errors for Brave Java files.

We need it to be prepared for upcoming cr132, where this error type is enabled for all non-test code: https://source.chromium.org/chromium/chromium/src/+/ce29c6390da3befff7b16ae789b49410e15f1754 .

To create this PR I commented out 'UnusedVariable' at src/build/android/gyp/compile_java.py:102 and fixed all errors for Brave java files.

Resolves brave/brave-browser#42153

Submitter Checklist:

  • I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally:
    • npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
    • npm run presubmit wiki, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

N/A, CI should succeed.

@AlexeyBarabash AlexeyBarabash added CI/skip-macos-x64 Do not run CI builds for macOS x64 CI/skip-ios Do not run CI builds for iOS CI/skip-windows-x64 Do not run CI builds for Windows x64 CI/skip-macos-arm64 Do not run CI builds for macOS arm64 labels Nov 7, 2024
@AlexeyBarabash AlexeyBarabash self-assigned this Nov 7, 2024
@AlexeyBarabash AlexeyBarabash marked this pull request as ready for review November 7, 2024 15:24
samartnik
samartnik approved these changes Nov 7, 2024
View reviewed changes
Copy link
Contributor

@samartnik samartnik left a comment

Choose a reason for hiding this comment

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

lgtm

SergeyZhukovsky
SergeyZhukovsky approved these changes Nov 7, 2024
View reviewed changes
Copy link
Member

@SergeyZhukovsky SergeyZhukovsky left a comment

Choose a reason for hiding this comment

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

++

@AlexeyBarabash
Copy link
Contributor Author

CI failed on

[2024-11-07T15:55:28.773Z] ../../brave/chromium_src/components/omnibox/browser/location_bar_model_util.cc:6:10: fatal error: 'brave/components/vector_icons/vector_icons.h' file not found
[2024-11-07T15:55:28.773Z]     6 | #include "brave/components/vector_icons/vector_icons.h"
[2024-11-07T15:55:28.773Z]       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[2024-11-07T15:55:28.774Z] 1 error generated.

Rebasing.

deeppandya
deeppandya approved these changes Nov 7, 2024
View reviewed changes
Copy link
Contributor

@deeppandya deeppandya left a comment

Choose a reason for hiding this comment

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

LGTM

@deeppandya
Copy link
Contributor

Thank you @AlexeyBarabash for the changes.

@AlexeyBarabash AlexeyBarabash force-pushed the java_fix_unused_variables_warnings branch from 2a7eae5 to 696947d Compare November 7, 2024 20:23
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 7, 2024

[puLL-Merge] - brave/brave-core@26433

Here's my review of the changes:

Description

This pull request makes a series of minor code cleanup changes across multiple files in the Brave Android codebase. The changes primarily focus on removing unused variables, imports, and methods, as well as addressing some minor code style issues.

Changes

Changes

  1. android/java/org/chromium/chrome/browser/BraveIntentHandler.java:

    • Renamed unused parameters with unused_ prefix.

  2. android/java/org/chromium/chrome/browser/BraveRewardsNativeWorker.java:

    • Removed unused constants.

  3. android/java/org/chromium/chrome/browser/BraveSyncWorker.java:

    • Removed unused imports and variables.

  4. android/java/org/chromium/chrome/browser/QRCodeShareDialogFragment.java:

    • Removed unused constants.

  5. android/java/org/chromium/chrome/browser/app/BraveActivity.java:

    • Removed unused imports, constants, and variables.
    • Simplified method signatures by removing unused parameters.

  6. android/java/org/chromium/chrome/browser/app/domain/CryptoModel.java:

    • Added @SuppressWarnings("UnusedVariable") annotation for unused members.

  7. android/java/org/chromium/chrome/browser/app/helpers/ImageLoader.java:

    • Removed unused imports and constants.

  8. android/java/org/chromium/chrome/browser/appmenu/BraveTabbedAppMenuPropertiesDelegate.java:

    • Removed unused variable.

  9. android/java/org/chromium/chrome/browser/autofill/BraveAutofillBackgroundServiceImpl.java:

    • Simplified method signature by removing unused parameter.

  10. android/java/org/chromium/chrome/browser/billing/InAppPurchaseWrapper.java:

    • Renamed unused variable with unused_ prefix.

  11. android/java/org/chromium/chrome/browser/brave_news/BraveNewsUtils.java:

    • Added @SuppressWarnings("UnusedVariable") annotation for unused method.

  12. android/java/org/chromium/chrome/browser/brave_news/CardBuilderFeedCard.java:

    • Removed unused imports, variables, and simplified some code.

  13. android/java/org/chromium/chrome/browser/compositor/layouts/BraveToolbarSwipeLayout.java:

    • Added @SuppressWarnings("UnusedVariable") annotation for unused field.

  14. android/java/org/chromium/chrome/browser/crypto_wallet/activities/AddAccountActivity.java:

    • Fixed variable initialization.

  15. android/java/org/chromium/chrome/browser/crypto_wallet/activities/BraveWalletActivity.java:

    • Removed unused import and variable.

  16. android/java/org/chromium/chrome/browser/crypto_wallet/adapters/CreateAccountAdapter.java:

    • Removed unused imports and variables.

  17. android/java/org/chromium/chrome/browser/crypto_wallet/presenters/SolanaInstructionPresenter.java:

    • Added @SuppressWarnings("UnusedVariable") annotation for unused field.

  18. android/java/org/chromium/chrome/browser/crypto_wallet/util/ParsedTransaction.java:

    • Removed unused code block.

  19. android/java/org/chromium/chrome/browser/crypto_wallet/util/Utils.java:

    • Removed unused variable.

  20. android/java/org/chromium/chrome/browser/custom_layout/popup_window_tooltip/PopupWindowTooltip.java:

    • Added @SuppressWarnings("UnusedVariable") annotation for unused fields.

  21. android/java/org/chromium/chrome/browser/firstrun/WelcomeOnboardingActivity.java:

    • Removed unused variables.

  22. android/java/org/chromium/chrome/browser/local_database/DatabaseHelper.java:

    • Renamed unused variable with unused_ prefix.

  23. android/java/org/chromium/chrome/browser/notifications/BraveNotificationWarningDialog.java:

    • Simplified method signatures by removing unused parameters.

  24. android/java/org/chromium/chrome/browser/notifications/BraveOnboardingNotification.java:

    • Removed unused variable.

  25. android/java/org/chromium/chrome/browser/notifications/permissions/BraveNotificationPermissionRationaleDialogController.java:

    • Added @SuppressWarnings("UnusedVariable") annotation for unused parameter.

  26. android/java/org/chromium/chrome/browser/ntp/BraveNewTabPage.java:

    • Added @SuppressWarnings("UnusedVariable") annotation for unused field.

  27. android/java/org/chromium/chrome/browser/ntp/BraveNewTabPageLayout.java:

    • Removed unused imports and variables.
    • Simplified method signatures by removing unused parameters.

  28. android/java/org/chromium/chrome/browser/ntp_background_images/model/Wallpaper.java:

    • Removed unused variables.

  29. android/java/org/chromium/chrome/browser/ntp_background_images/util/NTPImageUtil.java:

    • Added @SuppressWarnings("UnusedVariable") annotation for unused method.

  30. android/java/org/chromium/chrome/browser/omnibox/suggestions/BraveAutocompleteMediator.java:

    • Removed unused variable.

  31. android/java/org/chromium/chrome/browser/onboarding/BraveRewardsOnboardingFragment.java:

    • Removed unused variables.

  32. android/java/org/chromium/chrome/browser/onboarding/OnboardingPrefManager.java:

    • Removed unused constants.

  33. android/java/org/chromium/chrome/browser/onboarding/OnboardingViewPagerAdapter.java:

    • Renamed unused parameter with unused_ prefix.

  34. android/java/org/chromium/chrome/browser/onboarding/v2/HighlightDialogFragment.java:

    • Removed unused variable.

  35. android/java/org/chromium/chrome/browser/onboarding/v2/HighlightView.java:

    • Removed unused variable.

  36. android/java/org/chromium/chrome/browser/playlist/hls_content/HlsUtils.java:

    • Removed unused variables.

  37. android/java/org/chromium/chrome/browser/playlist/settings/BravePlaylistResetPreference.java:

    • Removed unused variable.

  38. android/java/org/chromium/chrome/browser/preferences/BravePrefServiceBridge.java:

    • Removed unused variable.

  39. android/java/org/chromium/chrome/browser/privacy/settings/BravePrivacySettings.java:

    • Removed unused imports and variables.

  40. android/java/org/chromium/chrome/browser/qrreader/CameraSource.java:

    • Added @SuppressWarnings("UnusedVariable") annotation for unused field.

  41. android/java/org/chromium/chrome/browser/quick_search_engines/settings/QuickSearchEnginesAdapter.java:

    • Removed unused variable.

  42. android/java/org/chromium/chrome/browser/quick_search_engines/views/QuickSearchEnginesViewAdapter.java:

    • Removed unused variable.

  43. android/java/org/chromium/chrome/browser/rate/RateUtils.java:

    • Removed unused constant.

  44. android/java/org/chromium/chrome/browser/rewards/BraveRewardsPanel.java:

    • Removed unused imports, constants, and variables.

  45. android/java/org/chromium/chrome/browser/rewards/onboarding/RewardsOnboarding.java:

    • Removed unused variable.

  46. android/java/org/chromium/chrome/browser/rewards/tipping/RewardsTippingBannerActivity.java:

    • Removed unused variables.

  47. android/java/org/chromium/chrome/browser/rewards/tipping/RewardsTippingBannerFragment.java

AlexeyBarabash added a commit that referenced this pull request Nov 7, 2024
@AlexeyBarabash
[cr132][Android][wip] Temporary disable UnusedVariable warning for Ja…
748f087 
…va compiler.

Must be reverted when the proper fix will reach cr132 from master.
Related PR to master: #26433

Chromium change:
https://source.chromium.org/chromium/chromium/src/+/ce29c6390da3befff7b16ae789b49410e15f1754

	Android: Enable Error Prone's UnusedVariable warning for non-test code

	All violations have been fixed, so turning it on! Note that this catches
	unused variables only in *private* code, so it should not hinder our
	ability to do cross-repo patches.

	If a new violation lands at the same time as this CL, please consider
	fixing / suppressing it rather than reverting this CL.

	To suppress, either:
	1) add @SuppressWarnings("UnusedVariable") on the class or method, or
	2) prefix variables with "unused_"

	Bug: 372458640, 40661145
	Change-Id: Ie9fbd1b34632dcc99bfbba4496bb1ec2b72e238f
	Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5935057
@AlexeyBarabash AlexeyBarabash merged commit d6d6a31 into master Nov 7, 2024
17 checks passed
@AlexeyBarabash AlexeyBarabash deleted the java_fix_unused_variables_warnings branch November 7, 2024 22:11
@github-actions github-actions bot added this to the 1.75.x - Nightly milestone Nov 7, 2024
@brave-builds
Copy link
Collaborator

Released in v1.75.4

cdesouza-chromium pushed a commit that referenced this pull request Nov 9, 2024
@AlexeyBarabash @cdesouza-chromium
[cr132][Android][wip] Temporary disable UnusedVariable warning for Ja…
0de0ba0 
…va compiler.

Must be reverted when the proper fix will reach cr132 from master.
Related PR to master: #26433

Chromium change:
https://source.chromium.org/chromium/chromium/src/+/ce29c6390da3befff7b16ae789b49410e15f1754

	Android: Enable Error Prone's UnusedVariable warning for non-test code

	All violations have been fixed, so turning it on! Note that this catches
	unused variables only in *private* code, so it should not hinder our
	ability to do cross-repo patches.

	If a new violation lands at the same time as this CL, please consider
	fixing / suppressing it rather than reverting this CL.

	To suppress, either:
	1) add @SuppressWarnings("UnusedVariable") on the class or method, or
	2) prefix variables with "unused_"

	Bug: 372458640, 40661145
	Change-Id: Ie9fbd1b34632dcc99bfbba4496bb1ec2b72e238f
	Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5935057
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samartnik samartnik samartnik approved these changes

@SergeyZhukovsky SergeyZhukovsky SergeyZhukovsky approved these changes

@deeppandya deeppandya deeppandya approved these changes

@zenparsing zenparsing Awaiting requested review from zenparsing

@simoarpe simoarpe Awaiting requested review from simoarpe

Assignees

@AlexeyBarabash AlexeyBarabash

Labels
CI/skip-ios Do not run CI builds for iOS CI/skip-macos-arm64 Do not run CI builds for macOS arm64 CI/skip-macos-x64 Do not run CI builds for macOS x64 CI/skip-windows-x64 Do not run CI builds for Windows x64 feature/web3/wallet puLL-Merge
Projects
None yet
Milestone
1.75.x - Nightly
Development

Successfully merging this pull request may close these issues.

Fix unused variables warnings at java
5 participants
@AlexeyBarabash @deeppandya @brave-builds @SergeyZhukovsky @samartnik