Closed Bug 1316023 Opened 3 years ago Closed 3 years ago

[findbugs] [UC] Useless condition

Categories

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

All
Android
defect

Tracking

()

RESOLVED FIXED
Firefox 53
Tracking Status
firefox53 --- fixed

People

(Reporter: sebastian, Assigned: swaroop.rao, Mentored)

References

(Blocks 1 open bug)

Details

(Whiteboard: [lang=java][good first bug])

Attachments

(1 file)

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."
Priority: P2 → P3
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.
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
Hi Sebastian!
I want to solve this bug. But need one more day for the building and setup of android environment.
Okay, no problem. There's no need to hurry.
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')
Hi Sebastian!

I would like to solve this bug. This is my first try.
If no one is working on this I would like to try :)
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.
Attachment #8811862 - Flags: review?(s.kaspari) → review+
Assignee: nobody → swaroop.rao
Status: NEW → ASSIGNED
Sorry about that. I thought I had deleted that line too. Should I fix it and re-upload another patch?
Flags: needinfo?(s.kaspari)
For bugs of this nature is it better to use a text editor or an IDE?

Thanks
(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).
Flags: needinfo?(s.kaspari)
Fixed. I found one more unreachable/useless block and removed it. Submitted for review.
Pushed by s.kaspari@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/dcdee53249e1
Version 2 of fixes. Removed unreachable statements. r=sebastian
https://hg.mozilla.org/mozilla-central/rev/dcdee53249e1
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
You need to log in before you can comment on or make changes to this bug.