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

UnnecessaryParentheses should be ignored for bit operations #1929

Closed
Mygod opened this issue Sep 17, 2019 · 5 comments
Closed

UnnecessaryParentheses should be ignored for bit operations #1929

Mygod opened this issue Sep 17, 2019 · 5 comments
Labels
Milestone

Comments

@Mygod
Copy link
Contributor

Mygod commented Sep 17, 2019

Expected Behavior of the rule

Example:

        val mask = 256 - (1 shl (i * 8 + 8 - prefixSize))
        return (a[i].toInt() and mask) == (b[i].toInt() and mask)

Context

Bitwise operations in C-like languages have confusing priorities so people usually add parentheses for everything.
This sample code also does not trigger warnings in latest Android Studio (3.6 alpha).

detekt 1.0.1

@3flex
Copy link
Member

3flex commented Sep 17, 2019

1.1.0 has a change in behaviour for this rule - it should only flag issues that are also flagged by Kotlin plugin for IntelliJ (see #1890).

When 1.1.0 is released please see if the new behaviour helps.

If not it's still unlikely that we'll make changes and instead suggest suppressing the rule in these cases, since there were a few issues with this rule in the past and too many edge cases, so we'll now just use same logic as Kotlin itself does.

@schalkms
Copy link
Member

This has been fixed by 61f1734.
We won’t invest more time in this rule, since detekt complies with IntelliJ’s rule.

@arturbosch arturbosch added this to the 1.1.0 milestone Oct 4, 2019
@Mygod
Copy link
Contributor Author

Mygod commented Oct 17, 2019

In 1.1.0 this issue is still there. Also IntelliJ/Android Studio does not report this.

@3flex
Copy link
Member

3flex commented Oct 17, 2019

Sorry I wasn't clearer earlier.

IntelliJ has intentions and inspections. Inspections show warnings or errors, intentions are used to change code but don't show a warning or error. Your example code is picked up by the intention detection in IntelliJ as expected, and detekt does the same:

image

@Mygod
Copy link
Contributor Author

Mygod commented Dec 9, 2019

Oh I see. Thank you!

@lock lock bot locked as resolved and limited conversation to collaborators Mar 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants