Repository: checkstyle/checkstyle

Problem statement:
UnnecessaryParenthesesCheck does not flag unnecessary parentheses in conditional expression
detected at https://github.com/checkstyle/checkstyle/pull/15250#discussion_r1676837498
I have read check documentation: https://checkstyle.org/checks/coding/unnecessaryparentheses.html#UnnecessaryParentheses
I have downloaded the latest checkstyle from: https://checkstyle.org/cmdline.html#Download_and_Run
I have executed the cli and showed it below, as cli describes the problem better than 1,000 words

---

```
PS D:\CS\test> javac src/Test.java
PS D:\CS\test> cat config.xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
          "https://checkstyle.org/dtds/configuration_1_3.dtd">

<module name="Checker">
  <property name="haltOnException" value="true"/>
  <module name="TreeWalker">
    <module name="UnnecessaryParentheses">
    </module>
  </module>
</module>
PS D:\CS\test> cat src/Test.java

public class Test {
    void test(int x) {
        boolean a = (x == 3); // violation
        boolean b = (!(x == 3)); // violation

        boolean c = (x == 3) ? (x == 3) : (!(x == 3));  // expected 3 violations
    }

}
PS D:\CS\test> java  -jar checkstyle-10.17.0-all.jar -c config.xml  src/Test.java
Starting audit...
[ERROR] D:\CS\test\src\Test.java:4:21: Unnecessary parentheses around assignment right-hand side. [UnnecessaryParentheses]
[ERROR] D:\CS\test\src\Test.java:5:21: Unnecessary parentheses around assignment right-hand side. [UnnecessaryParentheses]
Audit done.
Checkstyle ends with 2 errors.


```
---

**Describe what you expect in detail.**

`UnnecessaryParentheses` should flag unnecessary parentheses in conditional expressions. We should add `Question` to the acceptable tokens or something.


---



Hints:
Issue looks good to me.


please add false negative label
we usually place "bug" label after fix (as this label is for release notes)
Is this still open and not assigned ? 
I think I have a solution for it but not sure if precedence order would affect this solution
@MohanadKh03 , thanks a lot for help, you are always welcome to fix any issues.

We highly recommend (but not a mandatory  requirement) for new contributors to finish one issue in. Each group "good xxxx issue" in this case contributor learning gradually how our project works, and there less likely to be failure to finish fix.
@romani  
I have actually reviewed some of the previous fixes in this check especially so I have a brief idea about it.
But I have some questions regarding how should the multiple violations in a single line be reported ? Like as a `MSG_EXPR` ? 
I already have an idea about how it could be fixed so if it's possible I can raise a PR and we can discuss more details about it there ?  
Better to discuss in PR, see you in PR

Patch:
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/UnnecessaryParenthesesCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/UnnecessaryParenthesesCheck.java
--- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/UnnecessaryParenthesesCheck.java
+++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/UnnecessaryParenthesesCheck.java
@@ -19,6 +19,9 @@
 
 package com.puppycrawl.tools.checkstyle.checks.coding;
 
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
 import java.util.regex.Pattern;
 
 import com.puppycrawl.tools.checkstyle.FileStatefulCheck;
@@ -441,6 +444,7 @@ public int[] getAcceptableTokens() {
             TokenTypes.BXOR,
             TokenTypes.BOR,
             TokenTypes.BAND,
+            TokenTypes.QUESTION,
         };
     }
 
@@ -458,6 +462,10 @@ public void visitToken(DetailAST ast) {
         if (isLambdaSingleParameterSurrounded(ast)) {
             log(ast, MSG_LAMBDA);
         }
+        else if (ast.getType() == TokenTypes.QUESTION) {
+            getParenthesesChildrenAroundQuestion(ast)
+                .forEach(unnecessaryChild -> log(unnecessaryChild, MSG_EXPR));
+        }
         else if (parent.getType() != TokenTypes.ANNOTATION_MEMBER_VALUE_PAIR) {
             final int type = ast.getType();
             final boolean surrounded = isSurrounded(ast);
@@ -684,6 +692,27 @@ private static boolean isLambdaSingleParameterSurrounded(DetailAST ast) {
         return result;
     }
 
+    /**
+     *  Returns the direct LPAREN tokens children to a given QUESTION token which
+     *  contain an expression not a literal variable.
+     *
+     *  @param questionToken {@code DetailAST} question token to be checked
+     *  @return the direct children to the given question token which their types are LPAREN
+     *          tokens and not contain a literal inside the parentheses
+     */
+    private static List<DetailAST> getParenthesesChildrenAroundQuestion(DetailAST questionToken) {
+        final List<DetailAST> surroundedChildren = new ArrayList<>();
+        DetailAST directChild = questionToken.getFirstChild();
+        while (directChild != null) {
+            if (directChild.getType() == TokenTypes.LPAREN
+                    && !TokenUtil.isOfType(directChild.getNextSibling(), LITERALS)) {
+                surroundedChildren.add(directChild);
+            }
+            directChild = directChild.getNextSibling();
+        }
+        return Collections.unmodifiableList(surroundedChildren);
+    }
+
     /**
      * Returns the specified string chopped to {@code MAX_QUOTED_LENGTH}
      * plus an ellipsis (...) if the length of the string exceeds {@code
diff --git a/src/xdocs-examples/resources/com/puppycrawl/tools/checkstyle/checks/coding/unnecessaryparentheses/Example3.java b/src/xdocs-examples/resources/com/puppycrawl/tools/checkstyle/checks/coding/unnecessaryparentheses/Example3.java
new file mode 100644
--- /dev/null
+++ b/src/xdocs-examples/resources/com/puppycrawl/tools/checkstyle/checks/coding/unnecessaryparentheses/Example3.java
@@ -0,0 +1,24 @@
+/*xml
+<module name="Checker">
+  <module name="TreeWalker">
+    <module name="UnnecessaryParentheses">
+      <property name="tokens" value="QUESTION" />
+    </module>
+  </module>
+</module>
+*/
+package com.puppycrawl.tools.checkstyle.checks.coding.unnecessaryparentheses;
+
+// xdoc section -- start
+class Example3 {
+
+  void method() {
+    int a = 9, b = 8;
+    int c = (a > b) ? 1 : 0; // violation 'Unnecessary parentheses around expression'
+
+    int d = c == 1 ? (b % 2 == 0) ? 1 : 0 : 5;
+    // violation above 'Unnecessary parentheses around expression'
+  }
+
+}
+// xdoc section -- end


Test patch:
diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/UnnecessaryParenthesesCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/UnnecessaryParenthesesCheckTest.java
--- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/UnnecessaryParenthesesCheckTest.java
+++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/UnnecessaryParenthesesCheckTest.java
@@ -357,4 +357,22 @@ public void testWhenExpressions() throws Exception {
         verifyWithInlineConfigParser(
                 getNonCompilablePath("InputUnnecessaryParenthesesWhenExpressions.java"), expected);
     }
+
+    @Test
+    public void testUnnecessaryParenthesesConditionalExpression() throws Exception {
+        final String[] expected = {
+            "19:17: " + getCheckMessage(MSG_EXPR),
+            "19:29: " + getCheckMessage(MSG_LITERAL, "3"),
+            "19:35: " + getCheckMessage(MSG_LITERAL, "4"),
+            "25:18: " + getCheckMessage(MSG_EXPR),
+            "28:18: " + getCheckMessage(MSG_EXPR),
+            "28:33: " + getCheckMessage(MSG_EXPR),
+            "35:26: " + getCheckMessage(MSG_EXPR),
+            "36:17: " + getCheckMessage(MSG_EXPR),
+            "36:41: " + getCheckMessage(MSG_EXPR),
+        };
+        verifyWithInlineConfigParser(
+                getPath("InputUnnecessaryParenthesesConditionalExpression.java"), expected);
+
+    }
 }
diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/unnecessaryparentheses/InputUnnecessaryParenthesesConditionalExpression.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/unnecessaryparentheses/InputUnnecessaryParenthesesConditionalExpression.java
new file mode 100644
--- /dev/null
+++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/unnecessaryparentheses/InputUnnecessaryParenthesesConditionalExpression.java
@@ -0,0 +1,42 @@
+/*
+UnnecessaryParentheses
+tokens = EXPR, IDENT, NUM_DOUBLE, NUM_FLOAT, NUM_INT, NUM_LONG, \
+         STRING_LITERAL, LITERAL_NULL, LITERAL_FALSE, LITERAL_TRUE, ASSIGN, \
+         BAND_ASSIGN, BOR_ASSIGN, BSR_ASSIGN, BXOR_ASSIGN, DIV_ASSIGN, \
+         MINUS_ASSIGN, MOD_ASSIGN, PLUS_ASSIGN, SL_ASSIGN, SR_ASSIGN, STAR_ASSIGN, \
+         LAMBDA, TEXT_BLOCK_LITERAL_BEGIN, LAND, LITERAL_INSTANCEOF, GT, LT, GE, \
+         LE, EQUAL, NOT_EQUAL, UNARY_MINUS, UNARY_PLUS, INC, DEC, LNOT, BNOT, \
+         POST_INC, POST_DEC, QUESTION
+
+
+*/
+
+package com.puppycrawl.tools.checkstyle.checks.coding.unnecessaryparentheses;
+
+public class InputUnnecessaryParenthesesConditionalExpression {
+    void method() {
+        int w = 5;
+        int x = (w == 3) ? (3) : (4);
+        // 3 violations above
+        //  'Unnecessary parentheses around expression'
+        //  'Unnecessary parentheses around literal '3''.
+        //  'Unnecessary parentheses around literal '4''.
+        int y = !(w>x) ? 3 : 4;
+        int z1 = (!(y >= w)) ? 5 : 6; // violation 'Unnecessary parentheses around expression'
+
+        int z2 = 5 > 3 ? 2 : 1;
+        int z3 = (z2 > 0) ? 5 : (z2 < -10) ? 7 : 3;
+        // 2 violations above
+        //  'Unnecessary parentheses around expression'
+        //  'Unnecessary parentheses around expression'
+
+        Object o = "";
+        // violation below 'Unnecessary parentheses around expression'
+        boolean result = (o instanceof String) ?
+                (o instanceof String) : (!(o instanceof String));
+        // 2 violations above
+        //  'Unnecessary parentheses around expression'
+        //  'Unnecessary parentheses around expression'
+
+    }
+}