Hi,
I would like to propose a new rule for SonarJava. While it is certainly not a candidate for inclusion in the default Sonar Way quality profile, I believe it will still be useful to many teams and organizations, including my own company.
If the rule gets approved, I will finish up my proof of concept implementation and create a PR.
Here are the details:
Rule description & motivation
Java’s bitwise operators (and/or/xor, left/right shift, unsigned right shift and corresponding compound assignment operators) have several valid uses, e.g. binary file formats, cryptography algorithms or graphics programming.
That said, many developers working outside these areas are not accustomed to these operators because their application code usually has no need for them. In such applications, performing bitwise manipulation of numbers may be an indicator of premature optimization attempts and/or developers who are more familiar with low-level coding than with object-oriented concepts or the Java API (e.g. enum
and EnumSet<MyEnum>
as a replacement for the classic bit fields approach).
For teams who are working in such contexts, SonarJava should therefore offer a rule that alerts them whenever bitwise operators are used. This way, the code can be verified to be a justified and correctly implemented use of bit manipulation, or replaced with more obvious and/or type-safe high-level code.
Noncompliant Code
public int double(int base) {
return original << 1;
}
class Style {
public static final int BOLD = 1;
public static final int ITALICS = 2;
public static final int UNDERLINE = 4;
}
public void demo() {
int heading = Style.BOLD | Style.UNDERLINE;
apply(heading);
}
public void apply(int styles) {
if ((styles & Style.BOLD) == Style.BOLD) applyBold();
if ((styles & Style.ITALICS) == Style.ITALICS) applyItalics();
if ((styles & Style.UNDERLINE) == Style.UNDERLINE) applyUnderline();
}
Compliant Code
public int double(int base) {
return original * 2;
}
enum Style {
BOLD, ITALICS, UNDERLINE;
}
public void demo() {
Set<Style> heading = EnumSet.of(Style.BOLD, Style.UNDERLINE);
apply(heading);
}
public void apply(Set<Style> styles) {
if (styles.contains(Style.BOLD)) applyBold();
if (styles.contains(Style.ITALICS)) applyItalics();
if (styles.contains(Style.UNDERLINE)) applyUnderline();
}
Impact to keep this code as it is
Code which needlessly uses bit manipulation tends to make the author’s intention less obvious than the object-oriented equivalent. Also, it increases the risk that future changes introduce bugs, e.g. adding another flag to an int
bit field but accidentally using the same value as an existing one.
Notes
No issue should be raised when the and/or/xor operators or their compound assignment counterparts are used with boolean
values.
References
- Vojtech Ruzicka’s Programming Blog: Bit Manipulation in Java – Bitwise and Bit Shift operations
-
Oracle: Guide to Typesafe Enums - lists problems of the bit flag approach, explains
enum
in detail
Type
Code Smell
Tags
clumsy, pitfall