checkstyle / checkstyle/checkstyle
NeedBraces extention to support non simple singleline statements
- Dominant language
- Java
- Stars
- 9.6k
- Forks
- 4.2k
- Avg merge
- 21h 32m
- Merged PRs (30d)
- 233
Description
base on discussion at #1006 and #1007
It is required to update NeedBraces - http://checkstyle.sourceforge.net/config_blocks.html#NeedBraces
Reason is that according to Java grammar, Statement could contain blocks, blocks contains statements.So term "statement" is not very certain.
Details at: https://docs.oracle.com/javase/specs/jls/se8/html/jls-14.html
Proposal is to add
maxNestingSingleLineStatement (see proposal at [comment](https://github.com/checkstyle/checkstyle/issues/1007#issuecomment-120601784)) to work with `allowSingleLineStatement` to let user define depth.
After conversations in issues ,now looks like problem is not only in nesting depth but in location of "trailing sub statements".
Looks at [spec](https://docs.oracle.com/javase/specs/jls/se8/html/jls-14.html#jls-14.5) , "StatementWithoutTrailingSubstatement". So we might need new option "allowSameLineTrailingSubstatement"
To to avoid conflict of interest and ..... User should define himself allowed depth of single line statement.
User could define nesting level (`maxNestingSingleLineStatement`) to satisfy code that they like or use `allowSameLineTrailingSubstatement`.
All cases from that issues should be covered by UTs.
**Critical point** - By default allowSameLineTrailingSubstatement should be `FALSE`, and work the same as it is now.
By default should be ok:
`if (x == 0) return foo();`
Not ok by default (violations expected when `allowSameLineTrailingSubstatement = FALSE` ):
```
if (x == 0) x = 1;
else if (x == 1) x = 2;
else if (x == 2) x = 3;
else x = 4;
```
No violation, With `allowSameLineTrailingSubstatement = TRUE` :
```
int x = 0;
if (x == 0) x = 1;
else if (x == 1) x = 2;
else if (x == 2) x = 3;
else x = 4;
```
```
public static EDataType makeDataType(String name) {
EcorePackage ePkg = getFactory().getEcorePackage();
if (name.equals("EBoolean")) return ePkg.getEBoolean();
else if (name.equals("EBooleanObject")) return ePkg.getEBooleanObject();
else if (name.equals("EByte")) return ePkg.getEByte();
else if (name.equals("EByteObject")) return ePkg.getEByteObject();
else if (name.equals("EByteArray")) return ePkg.getEByteArray();
else if (name.equals("EChar")) return ePkg.getEChar();
else if (name.equals("ECharacterObject")) return ePkg.getECharacterObject();
else if (name.equals("EDouble")) return ePkg.getEDouble();
else if (name.equals("EDoubleObject")) return ePkg.getEDoubleObject();
else if (name.equals("EFloat")) return ePkg.getEFloat();
else if (name.equals("EFloatObject")) return ePkg.getEFloatObject();
else if (name.equals("EInt")) return ePkg.getEInt();
else if (name.equals("EIntegerObject")) return ePkg.getEIntegerObject();
else if (name.equals("EJavaObject")) return ePkg.getEJavaObject();
else if (name.equals("ELong")) return ePkg.getELong();
else if (name.equals("ELongObject")) return ePkg.getELongObject();
else if (name.equals("EShort")) return ePkg.getEShort();
else if (name.equals("EShortObject")) return ePkg.getEShortObject();
else if (name.equals("EString")) return ePkg.getEString();
else {
Assert.fail(fmt("Unknown standard data type \"%s\".", name));
return null; // Not reached.
}
}
```
```
if (l == loc1) r.add("1");
else if (l == loc2) r.add("2");
else if (l == loc3) r.add("3");
else if (l == loc4) r.add("4");
else r.add("?");
```
```
for (int i = 0; i < 10; i++) if (i > 0) for (int j = 0; j < 10; j++) if (j > 0) break;
for (int i = 0; i < 10; i++) if (i == 5) break;
for (idx = 0; idx <= lastIndex; idx++) if (incomingBlknums.get(idx) == badBlocknum) break;
for (BDD bdd: aut.initialsVars) if (bdd != null) bdd.free();
for (BDD bdd: aut.initialsComps) bdd.free();
```
```
public boolean isInitial() {
for (int loc: srcLocs) if (loc != 0) return false;
return true;
}
```
```
public static boolean isEmptyIntersection(Set set1, Set set2) {
if (set1.isEmpty() || set2.isEmpty()) return true;
if (set1.size() < set2.size()) {
for (T s: set1) if (set2.contains(s)) return false;
} else {
for (T s: set2) if (set1.contains(s)) return false;
}
return true;
}
```
------------------------------------
ATTENTION:
With `allowSameLineTrailingSubstatement = TRUE`
the following should NOT be ok, as it is missing braces for multiline 'else if' part:
```
int x = 0;
if (x == 0) x = 1;
else if (x == 1)
x = 2;
else if (x == 2) x = 3;
else x = 4;
```
```
for (int i = 0; i < 10; i++)
if (i == 5)
break;
```
---
Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/39798782-needbraces-extention-to-support-non-simple-singleline-statements?utm_campaign=plugin&utm_content=tracker%2F500528&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F500528&utm_medium=issues&utm_source=github).
Contributor guide
Research direction
Start with the NeedBraces documentation and the linked discussions in issues #1006 and #1007, then compare the Java grammar section cited in the issue with the listed examples. Define the option behavior and defaults before locating the NeedBraces tests; done means all described single-line, nested, trailing-substatement, and multiline cases are covered by unit tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- tooling
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 20/100