Repository: checkstyle/checkstyle

Problem statement:
NoWhitespaceAfter false positive on synchronized method
Hello,

I have the following checkstyle configuration in `pom.xml`:
```xml
<module name="NoWhitespaceAfter">
      <property name="tokens" value="ARRAY_INIT,AT,INC,DEC,UNARY_MINUS,UNARY_PLUS,BNOT,LNOT,DOT,TYPECAST,ARRAY_DECLARATOR,INDEX_OP,LITERAL_SYNCHRONIZED,METHOD_REF" />
</module>
```

With a `mvn verify`, the following piece of code reports an error `'synchronized' is followed by whitespace`:

```java
public class TestClass {
    private synchronized Row waitForNextRow() {
    }
}
```

It seems to be a false positive because the check should only apply on structures like:
```java
synchronized(lock) { ... }
```

Here is the output:
```
$ cat TestClass.java
public class TestClass {
    private synchronized Row waitForNextRow() {
    }
}

$ cat TestConfig.xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "https://checkstyle.org/dtds/configuration_1_3.dtd">

<module name="Checker">
    <property name="charset" value="UTF-8"/>

    <module name="TreeWalker">
<module name="NoWhitespaceAfter">
      <property name="tokens" value="ARRAY_INIT,AT,INC,DEC,UNARY_MINUS,UNARY_PLUS,BNOT,LNOT,DOT,TYPECAST,ARRAY_DECLARATOR,INDEX_OP,LITERAL_SYNCHRONIZED,METHOD_REF" />
</module>
    </module>
</module>

$ java -jar checkstyle-10.5.0-all.jar -c TestConfig.xml TestClass.java
Starting audit...
[ERROR] TestClass.java:2:13: 'synchronized' is followed by whitespace. [NoWhitespaceAfter]
Audit done.
Checkstyle ends with 1 errors.
```

Thanks & best regards.


Hints:
@rnveach Oops it seems you edited my comments and I edited them back behind, sorry. Don't hesitate if you think I'm missing some important information.
We ask for issues to reported in our format https://checkstyle.org/report_issue.html#How_to_report_a_bug.3F
@rnveach Thanks. I put back the output you've added previously. Hope it is good.
I am on it

Patch:
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/NoWhitespaceAfterCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/NoWhitespaceAfterCheck.java
--- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/NoWhitespaceAfterCheck.java
+++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/NoWhitespaceAfterCheck.java
@@ -50,11 +50,16 @@
  * INDEX_OP</a> will be ignored.
  * </p>
  * <p>
- * If the annotation is between the type and the array, the check will skip validation for spaces
+ * If the annotation is between the type and the array, like {@code char @NotNull [] param},
+ * the check will skip validation for spaces.
+ * </p>
+ * <p>
+ * Note: This check processes the
+ * <a href="https://checkstyle.org/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LITERAL_SYNCHRONIZED">
+ * LITERAL_SYNCHRONIZED</a> token only when it appears as a part of a
+ * <a href="https://docs.oracle.com/javase/specs/jls/se19/html/jls-14.html#jls-14.19">
+ * synchronized statement</a>, i.e. {@code synchronized(this) {}}.
  * </p>
- * <pre>
- * public void foo(final char @NotNull [] param) {} // No violation
- * </pre>
  * <ul>
  * <li>
  * Property {@code allowLineBreaks} - Control whether whitespace is allowed
@@ -283,12 +288,11 @@ private static DetailAST getWhitespaceFollowedNode(DetailAST ast) {
      * @return true if whitespace after ast should be checked
      */
     private static boolean shouldCheckWhitespaceAfter(DetailAST ast) {
-        boolean checkWhitespace = true;
         final DetailAST previousSibling = ast.getPreviousSibling();
-        if (previousSibling != null && previousSibling.getType() == TokenTypes.ANNOTATIONS) {
-            checkWhitespace = false;
-        }
-        return checkWhitespace;
+        final boolean isSynchronizedMethod = ast.getType() == TokenTypes.LITERAL_SYNCHRONIZED
+                        && ast.getFirstChild() == null;
+        return !isSynchronizedMethod
+                && (previousSibling == null || previousSibling.getType() != TokenTypes.ANNOTATIONS);
     }
 
     /**


Test patch:
diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/whitespace/NoWhitespaceAfterCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/whitespace/NoWhitespaceAfterCheckTest.java
--- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/whitespace/NoWhitespaceAfterCheckTest.java
+++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/whitespace/NoWhitespaceAfterCheckTest.java
@@ -354,6 +354,17 @@ public void testNoWhitespaceAfterWithEmoji() throws Exception {
                 getPath("InputNoWhitespaceAfterWithEmoji.java"), expected);
     }
 
+    @Test
+    public void testNoWhitespaceAfterSynchronized() throws Exception {
+        final String[] expected = {
+            "18:9: " + getCheckMessage(MSG_KEY, "synchronized"),
+        };
+
+        verifyWithInlineConfigParser(
+                getPath("InputNoWhitespaceAfterSynchronized.java"),
+                expected);
+    }
+
     /**
      * Creates MOCK lexical token and returns AST node for this token.
      *
diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/nowhitespaceafter/InputNoWhitespaceAfterSynchronized.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/nowhitespaceafter/InputNoWhitespaceAfterSynchronized.java
new file mode 100644
--- /dev/null
+++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/nowhitespaceafter/InputNoWhitespaceAfterSynchronized.java
@@ -0,0 +1,59 @@
+/*
+NoWhitespaceAfter
+allowLineBreaks = (default)true
+tokens = LITERAL_SYNCHRONIZED
+
+
+*/
+package com.puppycrawl.tools.checkstyle.checks.whitespace.nowhitespaceafter;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Target;
+
+public class InputNoWhitespaceAfterSynchronized<T> {
+
+    private final Object lock = new Object();
+
+    void m1() {
+        synchronized (lock) {} // violation
+        synchronized(lock) {} // ok
+    }
+
+    synchronized int m2() { // ok
+        return 2;
+    }
+
+    synchronized private int[] m3() { // ok
+        return new int[]{2};
+    }
+
+    synchronized int[] m4() { // ok
+        return new int[]{2};
+    }
+
+    private final synchronized int m5() { // ok
+        return 2;
+    }
+
+    synchronized T m6() { // ok
+        return null;
+    }
+
+    @SyncAnno synchronized T m7() { // ok
+        return null;
+    }
+
+    @SyncAnno private synchronized T m8() { // ok
+        return null;
+    }
+
+    // Note that we do not check 'synchronized' as a modifier,
+    // convention is to have type parameters of a generic method
+    // be surrounded by whitespace.
+    synchronized <T2> void m9() {} // ok
+    synchronized<T2> void m10() {} // ok
+}
+
+@Target(ElementType.METHOD)
+@interface SyncAnno {
+}