Skip to content

Commit

Permalink
Allow options for != ternary check
Browse files Browse the repository at this point in the history
Provide more configuration options for `SpringTernaryCheck` and change
the default to only ban `==` when comparing to `null`.

Closes spring-iogh-46
  • Loading branch information
philwebb committed Jun 25, 2018
1 parent 494a18a commit 96b7726
Show file tree
Hide file tree
Showing 8 changed files with 143 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package io.spring.javaformat.checkstyle.check;

import java.util.Locale;

import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
Expand All @@ -29,6 +31,8 @@
*/
public class SpringTernaryCheck extends AbstractCheck {

private EqualsTest equalsTest = EqualsTest.NEVER_FOR_NULLS;

@Override
public int[] getDefaultTokens() {
return getAcceptableTokens();
Expand Down Expand Up @@ -61,7 +65,7 @@ private void visitQuestion(DetailAST ast) {
log(ast.getLineNo(), ast.getColumnNo(), "ternary.missingParen");
}
}
if (hasType(ast.getFirstChild(), TokenTypes.EQUAL)) {
if (hasType(ast.getFirstChild(), TokenTypes.EQUAL) && !isEqualsTestAllowed(ast)) {
log(ast.getLineNo(), ast.getColumnNo(), "ternary.equalOperator");
}
}
Expand All @@ -73,8 +77,53 @@ private boolean isAllowedGrandParent(DetailAST grandParent) {
|| hasType(grandParent, TokenTypes.LITERAL_WHILE);
}

private boolean isEqualsTestAllowed(DetailAST ast) {
switch (this.equalsTest) {
case ANY:
return true;
case NEVER:
return false;
case NEVER_FOR_NULLS:
DetailAST equal = ast.findFirstToken(TokenTypes.EQUAL);
return equal.findFirstToken(TokenTypes.LITERAL_NULL) == null;
}
throw new IllegalStateException("Unsupported equals test " + this.equalsTest);
}

private boolean hasType(DetailAST ast, int type) {
return (ast != null && ast.getType() == type);
}

public void setEqualsTest(String equalsTest) {
try {
this.equalsTest = Enum.valueOf(EqualsTest.class,
equalsTest.trim().toUpperCase(Locale.ENGLISH));
}
catch (final IllegalArgumentException ex) {
throw new IllegalArgumentException("unable to parse " + equalsTest, ex);
}
}

/**
* Type of equals operators allowed in the test condition.
*/
public enum EqualsTest {

/**
* Equals checks can be used for any test.
*/
ANY,

/**
* Equals tests can never be used.
*/
NEVER,

/**
* Equals tests can never be used for {@code null} checks.
*/
NEVER_FOR_NULLS

}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
+0 errors
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
+TernaryEqualsEqualsNever.java:27:33: Ternary operation should use != when testing. [SpringTernary]
+1 error
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC "-//Puppy Crawl//DTD Check Configuration 1.2//EN" "http://www.puppycrawl.com/dtds/configuration_1_2.dtd">
<module name="com.puppycrawl.tools.checkstyle.Checker">
<module name="com.puppycrawl.tools.checkstyle.TreeWalker">
<module name="io.spring.javaformat.checkstyle.check.SpringTernaryCheck">
<property name="equalsTest" value="any"/>
</module>
</module>
</module>
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC "-//Puppy Crawl//DTD Check Configuration 1.2//EN" "http://www.puppycrawl.com/dtds/configuration_1_2.dtd">
<module name="com.puppycrawl.tools.checkstyle.Checker">
<module name="com.puppycrawl.tools.checkstyle.TreeWalker">
<module name="io.spring.javaformat.checkstyle.check.SpringTernaryCheck">
<property name="equalsTest" value="never"/>
</module>
</module>
</module>
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

/**
* This is a valid example of a ternary expression.
* This is an invalid example of a ternary expression.
*
* @author Phillip Webb
*/
Expand All @@ -24,7 +24,12 @@ public class TernaryEqualsEquals {
public void test() {
boolean a = true;
boolean b = false;
int c = (a == b ? 1 : 2);
int c = (a != b ? 1 : 2);
}

public void test2() {
Boolean a = true;
int c = (a == null ? 1 : 2);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Copyright 2017-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* This is an invalid example of a ternary expression.
*
* @author Phillip Webb
*/
public class TernaryEqualsEqualsAlways {

public void test() {
boolean a = true;
boolean b = false;
int c = (a != b ? 1 : 2);
}

public void test2() {
Boolean a = true;
int c = (a == null ? 1 : 2);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* Copyright 2017-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* This is a valid example of a ternary expression.
*
* @author Phillip Webb
*/
public class TernaryEqualsEqualsNever {

public void test() {
boolean a = true;
boolean b = false;
int c = (a == b ? 1 : 2);
}

}

0 comments on commit 96b7726

Please sign in to comment.