1316023 - [findbugs] [UC] Useless condition

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P3)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

Useless condition: it's known that currentVersion >= 2 at this point

> In class org.mozilla.gecko.DataReportingNotification
> In method org.mozilla.gecko.DataReportingNotification.checkAndNotifyPolicy(Context)
> Value currentVersion >= 2
> Unreachable code at DataReportingNotification.java:[line 69]
> At DataReportingNotification.java:[line 65]

Useless condition: it's known that locallyModified == true at this point

> In class org.mozilla.gecko.sync.repositories.android.FormHistoryRepositorySession$8
> In method org.mozilla.gecko.sync.repositories.android.FormHistoryRepositorySession$8.run()
> Value locallyModified == true
> Unreachable code at FormHistoryRepositorySession.java:[line 676]
> At FormHistoryRepositorySession.java:[line 675]

Useless condition: it's known that locallyModified == true at this point

> In class org.mozilla.gecko.sync.repositories.android.PasswordsRepositorySession$5
> In method org.mozilla.gecko.sync.repositories.android.PasswordsRepositorySession$5.run()
> Value locallyModified == true
> Unreachable code at PasswordsRepositorySession.java:[line 320]
> At PasswordsRepositorySession.java:[line 319]

"This condition always produces the same result as the value of the involved variable was narrowed before. Probably something else was meant or condition can be removed."

Comment 1

[Owais Kazi]

Hi Sebastian!
I would like to work on this bug. Can you explain me in brief what i need to do given that this is my first bug.

Comment 2

[Sebastian Kaspari (:sebastian; :pocmo)]

Hi Owais!

To start, set up a build environment - you can see the instructions here:
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_for_Android_build

Then, you'll need to upload a patch - see:
http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/commits.html

To fix this bug you need to remove the unneeded checks in the classes mentioned in comment 0. For example in DataReportingNotification.java we check that currentVersion >= 2. But we already checked that it's not < 1 and not == 1 before that.
So it's already guaranteed to be >= 2. This check is not needed. In fact the whole block is not needed in this case.
Because we only return - and we are already at the end of the method.

If you need any help, you can reply to this bug, or feel free to message me on IRC - my nick is "sebastian" and you can find me and other helpful folks in #mobile. If you need IRC setup instructions, see https://wiki.mozilla.org/IRC

Comment 3

[Owais Kazi]

Hi Sebastian!
I want to solve this bug. But need one more day for the building and setup of android environment.

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

Okay, no problem. There's no need to hurry.

Comment 5

[Owais Kazi]

Hi Sebastian!
I am getting this error when i am building the project. Although i have created the mozconfig file and did the necessary changes (Building with Gradle is only supported for Fennec, i.e., MOZ_BUILD_APP == 'mobile/android')

Comment 6

[Aditya Lahiri]

Hi Sebastian!

I would like to solve this bug. This is my first try.

Comment 7

[Tomislav Jurin]

If no one is working on this I would like to try :)

Comment 8

[swaroop.rao]

Attachment: Bug 1316023 - Version 2 of fixes. Removed unreachable statements.

Review commit: https://reviewboard.mozilla.org/r/93782/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/93782/

Comment 9

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8811862 [details]
Bug 1316023 - Version 2 of fixes. Removed unreachable statements.

https://reviewboard.mozilla.org/r/93782/#review94080

::: mobile/android/base/java/org/mozilla/gecko/DataReportingNotification.java:66
(Diff revision 1)
>              return;
>          }
>  
> -        if (currentVersion >= DATA_REPORTING_VERSION) {
> -            // Do nothing, we're at a current (or future) version.
> +        // Do nothing, we're at a current (or future) version.
> -            return;
> +        return;

This return at the end of the method is not really needed either.

Comment 10

[Sebastian Kaspari (:sebastian; :pocmo)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=294f0ec6fd3b

Comment 11

[swaroop.rao]

Sorry about that. I thought I had deleted that line too. Should I fix it and re-upload another patch?

Comment 12

[Steven Ellul]

For bugs of this nature is it better to use a text editor or an IDE?

Thanks

Comment 13

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to swaroop.rao from comment #11)
> Sorry about that. I thought I had deleted that line too. Should I fix it and
> re-upload another patch?

Yeah, that would be great - if you have the time.

(In reply to Steven Ellul from comment #12)
> For bugs of this nature is it better to use a text editor or an IDE?

I'd always prefer Android Studio (see guide in comment 2).

Comment 14

[swaroop.rao]

Comment on attachment 8811862 [details]
Bug 1316023 - Version 2 of fixes. Removed unreachable statements.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/93782/diff/1-2/

Comment 15

[swaroop.rao]

Fixed. I found one more unreachable/useless block and removed it. Submitted for review.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dcdee53249e1
Version 2 of fixes. Removed unreachable statements. r=sebastian

Comment 17

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/dcdee53249e1