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

Issue #12345: fix NoWhitespaceAfterCheck false positive #12372

Merged
merged 1 commit into from
Nov 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4520,13 +4520,6 @@
<lineContent>result = latestDefinition == methodDef.getParent().getParent();</lineContent>
</checkerFrameworkError>

<checkerFrameworkError unstable="false">
<fileName>src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/NoWhitespaceAfterCheck.java</fileName>
<specifier>introduce.eliminate</specifier>
<message>It is bad style to create an Optional just to chain methods to get a value.</message>
<lineContent>return Optional.ofNullable(referencedMemberDot).orElse(referencedClassDot);</lineContent>
</checkerFrameworkError>

<checkerFrameworkError unstable="false">
<fileName>src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/ParenPadCheck.java</fileName>
<specifier>not.interned</specifier>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -506,13 +506,12 @@ private static DetailAST getPreviousNodeWithParentOfTypeAst(DetailAST ast, Detai
final DetailAST previousElement;
final DetailAST ident = getIdentLastToken(parent.getParent());
final DetailAST lastTypeNode = getTypeLastNode(ast);
final boolean isTypeCast = parent.getParent().getType() == TokenTypes.TYPECAST;
// sometimes there are ident-less sentences
// i.e. "(Object[]) null", but in casual case should be
// checked whether ident or lastTypeNode has preceding position
// determining if it is java style or C style

if (ident == null || isTypeCast || ident.getLineNo() > ast.getLineNo()) {
if (ident == null || ident.getLineNo() > ast.getLineNo()) {
previousElement = lastTypeNode;
}
else if (ident.getLineNo() < ast.getLineNo()) {
Expand Down Expand Up @@ -544,9 +543,9 @@ else if (ident.getLineNo() < ast.getLineNo()) {
*/
private static DetailAST getIdentLastToken(DetailAST ast) {
final DetailAST result;
final DetailAST dot = getPrecedingDot(ast);
final Optional<DetailAST> dot = getPrecedingDot(ast);
// method call case
if (dot == null || ast.getFirstChild().getType() == TokenTypes.METHOD_CALL) {
if (dot.isEmpty() || ast.getFirstChild().getType() == TokenTypes.METHOD_CALL) {
final DetailAST methodCall = ast.findFirstToken(TokenTypes.METHOD_CALL);
if (methodCall == null) {
result = ast.findFirstToken(TokenTypes.IDENT);
Expand All @@ -557,7 +556,7 @@ private static DetailAST getIdentLastToken(DetailAST ast) {
}
// qualified name case
else {
result = dot.getFirstChild().getNextSibling();
result = dot.get().getFirstChild().getNextSibling();
}
return result;
}
Expand All @@ -569,11 +568,24 @@ private static DetailAST getIdentLastToken(DetailAST ast) {
* @param leftBracket the ast we are checking
* @return dot preceding the left bracket
*/
private static DetailAST getPrecedingDot(DetailAST leftBracket) {
final DetailAST referencedClassDot =
leftBracket.getParent().findFirstToken(TokenTypes.DOT);
private static Optional<DetailAST> getPrecedingDot(DetailAST leftBracket) {
final DetailAST referencedMemberDot = leftBracket.findFirstToken(TokenTypes.DOT);
return Optional.ofNullable(referencedMemberDot).orElse(referencedClassDot);
final Optional<DetailAST> result = Optional.ofNullable(referencedMemberDot);
return result.or(() -> getReferencedClassDot(leftBracket));
nrmancuso marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Gets the dot preceding a class reference.
*
* @param leftBracket the ast we are checking
* @return dot preceding the left bracket
*/
private static Optional<DetailAST> getReferencedClassDot(DetailAST leftBracket) {
final DetailAST parent = leftBracket.getParent();
Optional<DetailAST> classDot = Optional.empty();
if (parent.getType() != TokenTypes.ASSIGN) {
classDot = Optional.ofNullable(parent.findFirstToken(TokenTypes.DOT));
}
return classDot;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,13 @@ public void testDefault() throws Exception {
getPath("InputNoWhitespaceAfterTestDefault.java"), expected);
}

@Test
public void testAssignment() throws Exception {
final String[] expected = CommonUtil.EMPTY_STRING_ARRAY;
verifyWithInlineConfigParser(
getPath("InputNoWhitespaceAfterTestAssignment.java"), expected);
}

@Test
public void testDotAllowLineBreaks() throws Exception {
final String[] expected = {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
NoWhitespaceAfter
allowLineBreaks = false
tokens = (default)ARRAY_INIT, AT, INC, DEC, UNARY_MINUS, UNARY_PLUS, BNOT, LNOT, \
DOT, ARRAY_DECLARATOR, INDEX_OP


*/

package com.puppycrawl.tools.checkstyle.checks.whitespace.nowhitespaceafter; // ^ 2 violations above

public class InputNoWhitespaceAfterTestAssignment {

Object o;
static boolean b = true;

void some() {
Object oo = new Object[4];
Object[] oo2 = new Object[4];
this.o = ((Object[]) oo)[1]; // ok
this.o = ((java.lang.Object[]) oo)[1]; // ok
this.o = oo2[1];
QualifiedAssignment.o1 = ((Object[]) oo)[1]; // ok
QualifiedAssignment.o1 = ((java.lang.Object[]) oo)[1]; // ok
QualifiedAssignment.o1 = oo2[1];
QualifiedAssignment qa1 = null;
QualifiedAssignment[] qa2 = null;
int idx = 0;
(qa1 = (QualifiedAssignment)qa2[idx]).o1 = (new QualifiedAssignment[idx][idx][idx])[idx];
(b ? (new QualifiedAssignment().q1 = new QualifiedAssignment()) :
(QualifiedAssignment)(new QualifiedAssignment().q1 = new QualifiedAssignment())).q1 =
(new QualifiedAssignment[new QualifiedAssignment().idx = (QualifiedAssignment.idx =
QualifiedAssignment.idx)])[QualifiedAssignment.idx];
}
}

class QualifiedAssignment {
static Object o1;
static QualifiedAssignment q1;
static int idx = 1;
}
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ void rfe521323()
/** bug 806243 (NoWhitespaceBeforeCheck violation for anonymous inner class) */
void bug806243()
{
Object o = new InputNoWhitespaceAfterTestDefault() {
Object o = new InputNoWhitespaceAfterTestAssignment() {
private int j ;
// ^ whitespace
};
Expand Down