Skip to content

Commit

Permalink
Issue #1509: Extra comma is required in multi-line array value
Browse files Browse the repository at this point in the history
  • Loading branch information
Vladlis authored and rnveach committed Apr 22, 2017
1 parent 3526d78 commit 0c382fd
Show file tree
Hide file tree
Showing 4 changed files with 137 additions and 13 deletions.
Expand Up @@ -29,9 +29,57 @@
* </p>
* <p>
* Rationale: Putting this comma in make is easier to change the
* order of the elements or add new elements on the end.
* order of the elements or add new elements on the end. Main benefit of a trailing
* comma is that when you add new entry to an array, no surrounding lines are changed.
* </p>
* <p>
* The check demands a comma at the end if neither left nor right curly braces
* are on the same line as the last element of the array.
* </p>
* <pre>
* return new int[] { 0 };
* return new int[] { 0
* };
* return new int[] {
* 0 };
* </pre>
* <pre>
* {
* 100000000000000000000,
* 200000000000000000000, // OK
* }
*
* {
* 100000000000000000000,
* 200000000000000000000,
* 300000000000000000000, // Just this line added, no other changes
* }
* </pre>
* <p>
* If closing brace is on the same line as training comma, this benefit is gone
* (as the Check does not demand a certain location of curly braces the following
* two cases will not produce a violation):
* </p>
* <pre>
* {100000000000000000000,
* 200000000000000000000,} // Trailing comma not needed, line needs to be modified anyway
*
* {100000000000000000000,
* 200000000000000000000, // Modified line
* 300000000000000000000,} // Added line
* </pre>
* <p>
* If opening brace is on the same line as training comma there's also (more arguable) problem:
* </p>
* <pre>
* {100000000000000000000, // Line cannot be just duplicated to slightly modify entry
* }
*
* {100000000000000000000,
* 100000000000000000001, // More work needed to duplicate
* }
* </pre>
* <p>
* An example of how to configure the check is:
* </p>
* <pre>
Expand Down Expand Up @@ -65,15 +113,14 @@ public int[] getRequiredTokens() {
@Override
public void visitToken(DetailAST arrayInit) {
final DetailAST rcurly = arrayInit.findFirstToken(TokenTypes.RCURLY);
final DetailAST previousSibling = rcurly.getPreviousSibling();

// if curlies are on the same line
// or array is empty then check nothing
if (arrayInit.getLineNo() != rcurly.getLineNo()
&& arrayInit.getChildCount() != 1) {
final DetailAST prev = rcurly.getPreviousSibling();
if (prev.getType() != TokenTypes.COMMA) {
log(rcurly.getLineNo(), MSG_KEY);
}
&& arrayInit.getChildCount() != 1
&& rcurly.getLineNo() != previousSibling.getLineNo()
&& arrayInit.getLineNo() != previousSibling.getLineNo()
&& previousSibling.getType() != TokenTypes.COMMA) {
log(rcurly.getLineNo(), MSG_KEY);
}
}
}
Expand Up @@ -45,7 +45,6 @@ public void testDefault()
createCheckConfig(ArrayTrailingCommaCheck.class);
final String[] expected = {
"17: " + getCheckMessage(MSG_KEY),
"34: " + getCheckMessage(MSG_KEY),
"37: " + getCheckMessage(MSG_KEY),
};
verify(checkConfig, getPath("InputArrayTrailingComma.java"), expected);
Expand Down
Expand Up @@ -38,4 +38,24 @@ public class InputArrayTrailingComma

int[] e1 = new int[] {
};

int[] f1 = new int[] {0, 1
};

int[][] f2 = new int[][]
{
{1,
2,},
};

int[] f3 = new int[]{
1,
2
,
};

int[] f4 = new int[]{
1,
2
,};
}
66 changes: 62 additions & 4 deletions src/xdocs/config_coding.xml
Expand Up @@ -36,17 +36,58 @@ int[] a = new int[]
</source>

<p>
The check allows leaving out the comma at the end if both the left and right curly brackets
are on the same line.
The check demands a comma at the end if neither left nor right curly braces
are on the same line as the last element of the array.
</p>
<source>
return new int[] { 0 };
return new int[] { 0
};
return new int[] {
0 };
</source>

<p>
Rationale: Putting this comma in makes it easier to change the order
of the elements or add new elements on the end.
of the elements or add new elements on the end. Main benefit of a trailing
comma is that when you add new entry to an array, no surrounding lines are changed.
</p>
<source>
{
100000000000000000000,
200000000000000000000, // OK
}

{
100000000000000000000,
200000000000000000000,
300000000000000000000, // Just this line added, no other changes
}
</source>
<p>
If closing brace is on the same line as training comma, this benefit is gone
(as the Check does not demand a certain location of curly braces the following
two cases will not produce a violation):
</p>
<source>
{100000000000000000000,
200000000000000000000,} // Trailing comma not needed, line needs to be modified anyway

{100000000000000000000,
200000000000000000000, // Modified line
300000000000000000000,} // Added line
</source>
<p>
If opening brace is on the same line as training comma there's also (more arguable) problem:
</p>
<source>
{100000000000000000000, // Line cannot be just duplicated to slightly modify entry
}

{100000000000000000000,
100000000000000000001, // More work needed to duplicate
}
</source>
</subsection>

<subsection name="Examples">
Expand All @@ -73,7 +114,24 @@ return new int[] { 0 };
{0.5, 2.3, 1.1,}, //no violation
{1.7, 1.9, 0.6},
{0.8, 7.4, 6.5}
}; //violation
}; // violation as previous line misses a comma

char[] chars = {'a', 'b', 'c'
}; / /no violation

String[] letters = {
"a", "b", "c"}; // no violation

int[] a1 = new int[]{
1,
2
,
}; // no violation

int[] a2 = new int[]{
1,
2
,}; // no violation
</source>
</subsection>

Expand Down

0 comments on commit 0c382fd

Please sign in to comment.