Repository: checkstyle/checkstyle

Problem statement:
Resolve SimplifiableIfStatement inspection violations 
Taken from https://github.com/checkstyle/checkstyle/issues/11566

see commit: https://github.com/checkstyle/checkstyle/commit/009cffaae0f1bd79f5c0084f5354f45ed391e346

we need to revert suppression of SimplifiableIfStatement or explain why they should be permanent.


Hints:
On it

Patch:
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheck.java
--- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheck.java
+++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheck.java
@@ -383,16 +383,12 @@ private boolean checkLoop(final DetailAST ast) {
      * @param useBreak should we consider break as terminator
      * @param useContinue should we consider continue as terminator
      * @return true if try/catch/finally block is terminated
-     * @noinspection SimplifiableIfStatement
      */
     private boolean checkTry(final DetailAST ast, boolean useBreak,
                              boolean useContinue) {
         final DetailAST finalStmt = ast.getLastChild();
-        boolean isTerminated = false;
-        if (finalStmt.getType() == TokenTypes.LITERAL_FINALLY) {
-            isTerminated = isTerminated(finalStmt.findFirstToken(TokenTypes.SLIST),
-                                useBreak, useContinue);
-        }
+        boolean isTerminated = finalStmt.getType() == TokenTypes.LITERAL_FINALLY
+                && isTerminated(finalStmt.findFirstToken(TokenTypes.SLIST), useBreak, useContinue);
 
         if (!isTerminated) {
             DetailAST firstChild = ast.getFirstChild();
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/UnusedLocalVariableCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/UnusedLocalVariableCheck.java
--- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/UnusedLocalVariableCheck.java
+++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/UnusedLocalVariableCheck.java
@@ -363,9 +363,15 @@ private void visitVariableDefToken(DetailAST varDefAst) {
      * @param variablesStack stack of all the relevant variables in the scope
      */
     private static void visitIdentToken(DetailAST identAst, Deque<VariableDesc> variablesStack) {
-        final DetailAST parentAst = identAst.getParent();
-        if (!TokenUtil.isOfType(parentAst, UNACCEPTABLE_PARENT_OF_IDENT)
-                && shouldCheckIdentWithMethodRefParent(identAst)) {
+        final DetailAST parent = identAst.getParent();
+        final boolean isMethodReferenceMethodName = parent.getType() == TokenTypes.METHOD_REF
+                && parent.getFirstChild() != identAst;
+        final boolean isConstructorReference = parent.getType() == TokenTypes.METHOD_REF
+                && parent.getLastChild().getType() == TokenTypes.LITERAL_NEW;
+
+        if (!isMethodReferenceMethodName
+                && !isConstructorReference
+                && !TokenUtil.isOfType(parent, UNACCEPTABLE_PARENT_OF_IDENT)) {
             checkIdentifierAst(identAst, variablesStack);
         }
     }
@@ -728,32 +734,13 @@ private void customLeaveToken(DetailAST ast, Deque<VariableDesc> variablesStack)
         logViolations(ast, variablesStack);
     }
 
-    /**
-     * Whether an ident with parent node of type {@link TokenTypes#METHOD_REF}
-     * should be checked or not.
-     *
-     * @param identAst identAst
-     * @return true if an ident with parent node of type {@link TokenTypes#METHOD_REF}
-     *         should be checked or if the parent type is not {@link TokenTypes#METHOD_REF}
-     * @noinspection SimplifiableIfStatement
-     */
-    public static boolean shouldCheckIdentWithMethodRefParent(DetailAST identAst) {
-        final DetailAST parent = identAst.getParent();
-        boolean result = true;
-        if (parent.getType() == TokenTypes.METHOD_REF) {
-            result = parent.getFirstChild() == identAst
-                    && parent.getLastChild().getType() != TokenTypes.LITERAL_NEW;
-        }
-        return result;
-    }
-
     /**
      * Whether to check identifier token nested under dotAst.
      *
      * @param dotAst dotAst
      * @return true if ident nested under dotAst should be checked
      */
-    public static boolean shouldCheckIdentTokenNestedUnderDot(DetailAST dotAst) {
+    private static boolean shouldCheckIdentTokenNestedUnderDot(DetailAST dotAst) {
 
         return TokenUtil.findFirstTokenByPredicate(dotAst,
                         childAst -> {


Test patch:
diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/UnusedLocalVariableCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/UnusedLocalVariableCheckTest.java
--- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/UnusedLocalVariableCheckTest.java
+++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/UnusedLocalVariableCheckTest.java
@@ -36,6 +36,7 @@
 import com.puppycrawl.tools.checkstyle.api.DetailAST;
 import com.puppycrawl.tools.checkstyle.api.TokenTypes;
 import com.puppycrawl.tools.checkstyle.internal.utils.TestUtil;
+import com.puppycrawl.tools.checkstyle.utils.CommonUtil;
 
 public class UnusedLocalVariableCheckTest extends AbstractModuleTestSupport {
 
@@ -265,6 +266,14 @@ public void testUnusedLocalVarWithoutPackageStatement() throws Exception {
                 expected);
     }
 
+    @Test
+    public void testUnusedLocalVariableTernaryAndExpressions() throws Exception {
+        final String[] expected = CommonUtil.EMPTY_STRING_ARRAY;
+        verifyWithInlineConfigParser(
+                getPath("InputUnusedLocalVariableTernaryAndExpressions.java"),
+                expected);
+    }
+
     @Test
     public void testClearStateVariables() throws Exception {
         final UnusedLocalVariableCheck check = new UnusedLocalVariableCheck();
diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/unusedlocalvariable/InputUnusedLocalVariableTernaryAndExpressions.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/unusedlocalvariable/InputUnusedLocalVariableTernaryAndExpressions.java
new file mode 100644
--- /dev/null
+++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/unusedlocalvariable/InputUnusedLocalVariableTernaryAndExpressions.java
@@ -0,0 +1,28 @@
+/*
+UnusedLocalVariable
+
+
+*/
+package com.puppycrawl.tools.checkstyle.checks.coding.unusedlocalvariable;
+
+public class InputUnusedLocalVariableTernaryAndExpressions {
+    void m1() {
+        int time = new Integer(2); // ok
+        assert(time < new Integer(3));
+    }
+
+    Object m2() {
+        Object x = getY(); // ok
+        String z = null;
+        return z == null ? x : new Object();
+    }
+
+    String getY() {
+        return "y";
+    }
+
+    int m3() {
+        boolean b = m2().equals(new Object()); // ok
+        return b ? 1 : new Integer(3);
+    }
+}