SONARJAVA-6721 Extend rule S2442 to all "java.util.concurrent" objects

java-checks-test-sources/default/src/main/java/checks/SynchronizedOnConcurrentObjectCheckSample.java

@@ -0,0 +1,134 @@
+package checks;
+
+import java.util.concurrent.ArrayBlockingQueue;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.Future;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.Semaphore;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.locks.Condition;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+class SynchronizedOnConcurrentObjectCheckSample {
+
+  private final ReentrantLock reentrantLock = new ReentrantLock();
+  private final ReentrantReadWriteLock rwLock = new ReentrantReadWriteLock();
+  private final Lock lock = new ReentrantLock();
+  private final Semaphore semaphore = new Semaphore(1);
+  private final CountDownLatch latch = new CountDownLatch(1);
+  private final CyclicBarrier barrier = new CyclicBarrier(2);
+  private final BlockingQueue<String> blockingQueue = new ArrayBlockingQueue<>(10);
+  private final ArrayBlockingQueue<String> arrayBlockingQueue = new ArrayBlockingQueue<>(10);
+  private final LinkedBlockingQueue<String> linkedBlockingQueue = new LinkedBlockingQueue<>();
+  private final AtomicBoolean atomicBoolean = new AtomicBoolean();
+  private final AtomicInteger atomicInteger = new AtomicInteger();
+  private final CustomLock customLock = new CustomLock();
+  private final CustomLockImpl customLockImpl = new CustomLockImpl();
+
+  private final Object objectLock = new Object();
+  private final ConcurrentHashMap<String, String> concurrentMap = new ConcurrentHashMap<>();
+  private Future<?> future;
+
+  void noncompliant() {
+    synchronized (reentrantLock) { // Noncompliant {{Use the "ReentrantLock" API for synchronization instead of a "synchronized" block.}}
+    //            ^^^^^^^^^^^^^
+    }
+
+    synchronized (lock) { // Noncompliant {{Use the "Lock" API for synchronization instead of a "synchronized" block.}}
+    //            ^^^^
+    }
+
+    synchronized (rwLock) { // Noncompliant {{Use the "ReentrantReadWriteLock" API for synchronization instead of a "synchronized" block.}}
+    //            ^^^^^^
+    }
+
+    synchronized (semaphore) { // Noncompliant {{Use the "Semaphore" API for synchronization instead of a "synchronized" block.}}
+    //            ^^^^^^^^^
+    }
+
+    synchronized (latch) { // Noncompliant {{Use the "CountDownLatch" API for synchronization instead of a "synchronized" block.}}
+    //            ^^^^^
+    }
+
+    synchronized (barrier) { // Noncompliant {{Use the "CyclicBarrier" API for synchronization instead of a "synchronized" block.}}
+    //            ^^^^^^^
+    }
+
+    synchronized (blockingQueue) { // Noncompliant {{Use the "BlockingQueue" API for synchronization instead of a "synchronized" block.}}
+    //            ^^^^^^^^^^^^^
+    }
+
+    synchronized (arrayBlockingQueue) { // Noncompliant {{Use the "ArrayBlockingQueue" API for synchronization instead of a "synchronized" block.}}
+    //            ^^^^^^^^^^^^^^^^^^
+    }
+
+    synchronized (linkedBlockingQueue) { // Noncompliant {{Use the "LinkedBlockingQueue" API for synchronization instead of a "synchronized" block.}}
+    //            ^^^^^^^^^^^^^^^^^^^
+    }
+
+    synchronized (atomicBoolean) { // Noncompliant {{Use the "AtomicBoolean" API for synchronization instead of a "synchronized" block.}}
+    //            ^^^^^^^^^^^^^
+    }
+
+    synchronized (atomicInteger) { // Noncompliant {{Use the "AtomicInteger" API for synchronization instead of a "synchronized" block.}}
+    //            ^^^^^^^^^^^^^
+    }
+
+    synchronized (customLock) { // Noncompliant {{Use the "CustomLock" API for synchronization instead of a "synchronized" block.}}
+    //            ^^^^^^^^^^
+    }
+
+    synchronized (customLockImpl) { // Noncompliant {{Use the "CustomLockImpl" API for synchronization instead of a "synchronized" block.}}
+    //            ^^^^^^^^^^^^^^
+    }
+  }
+
+  void compliant() {
+    synchronized (objectLock) {
+    }
+
+    synchronized (concurrentMap) {
+    }
+
+    synchronized (future) {
+    }
+
+    reentrantLock.lock();
+    try {
+    } finally {
+      reentrantLock.unlock();
+    }
+
+    rwLock.writeLock().lock();
+    try {
+    } finally {
+      rwLock.writeLock().unlock();
+    }
+  }
+
+  void example() {
+    var lock2 = new ReentrantLock();
+    synchronized (lock2) { // Noncompliant
+    }
+  }
+
+  static class CustomLock extends ReentrantLock {
+  }
+
+  // Custom Lock implementation outside java.util.concurrent.locks — caught via isSubtypeOf(Lock)
+  static class CustomLockImpl implements Lock {
+    @Override public void lock() {}
+    @Override public void lockInterruptibly() throws InterruptedException {}
+    @Override public boolean tryLock() { return false; }
+    @Override public boolean tryLock(long time, TimeUnit unit) throws InterruptedException { return false; }
+    @Override public void unlock() {}
+    @Override public Condition newCondition() { return null; }
+  }
+}

java-checks/src/main/java/org/sonar/java/checks/SynchronizedOnConcurrentObjectCheck.java

@@ -0,0 +1,73 @@
+/*
+ * SonarQube Java
+ * Copyright (C) SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * You can redistribute and/or modify this program under the terms of
+ * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the Sonar Source-Available License for more details.
+ *
+ * You should have received a copy of the Sonar Source-Available License
+ * along with this program; if not, see https://sonarsource.com/license/ssal/
+ */
+package org.sonar.java.checks;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+import org.sonar.check.Rule;
+import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
+import org.sonar.plugins.java.api.semantic.Type;
+import org.sonar.plugins.java.api.tree.ExpressionTree;
+import org.sonar.plugins.java.api.tree.SynchronizedStatementTree;
+import org.sonar.plugins.java.api.tree.Tree;
+import org.sonar.plugins.java.api.tree.Tree.Kind;
+
+@Rule(key = "S2442")
+public class SynchronizedOnConcurrentObjectCheck extends IssuableSubscriptionVisitor {
+
+  private static final String CONCURRENT_LOCKS_PREFIX = "java.util.concurrent.locks.";
+  private static final String CONCURRENT_ATOMIC_PREFIX = "java.util.concurrent.atomic.";
+  private static final Set<String> CONCURRENT_SYNC_TYPES = Set.of(
+    "java.util.concurrent.Semaphore",
+    "java.util.concurrent.CountDownLatch",
+    "java.util.concurrent.CyclicBarrier",
+    "java.util.concurrent.Exchanger",
+    "java.util.concurrent.Phaser",
+    "java.util.concurrent.BlockingQueue",
+    "java.util.concurrent.BlockingDeque",
+    "java.util.concurrent.TransferQueue");
+
+  @Override
+  public List<Kind> nodesToVisit() {
+    return Collections.singletonList(Kind.SYNCHRONIZED_STATEMENT);
+  }
+
+  @Override
+  public void visitNode(Tree tree) {
+    ExpressionTree expression = ((SynchronizedStatementTree) tree).expression();
+    Type type = expression.symbolType();
+    if (isSynchronizationPrimitive(type)) {
+      reportIssue(expression, String.format(
+        "Use the \"%s\" API for synchronization instead of a \"synchronized\" block.", type.name()));
+    }
+  }
+
+  private static boolean isSynchronizationPrimitive(Type type) {
+    return type.isSubtypeOf("java.util.concurrent.locks.Lock")
+      || isKnownSyncPrimitive(type)
+      || type.symbol().superTypes().stream().anyMatch(SynchronizedOnConcurrentObjectCheck::isKnownSyncPrimitive);
+  }
+
+  private static boolean isKnownSyncPrimitive(Type type) {
+    String fqn = type.fullyQualifiedName();
+    return fqn.startsWith(CONCURRENT_LOCKS_PREFIX)
+      || fqn.startsWith(CONCURRENT_ATOMIC_PREFIX)
+      || CONCURRENT_SYNC_TYPES.contains(fqn);
+  }
+
+}

java-checks/src/test/java/org/sonar/java/checks/SynchronizedLockCheckTest.java → java-checks/src/test/java/org/sonar/java/checks/SynchronizedOnConcurrentObjectCheckTest.java

@@ -21,13 +21,23 @@
 
 import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;
 
-class SynchronizedLockCheckTest {
+class SynchronizedOnConcurrentObjectCheckTest {
 
   @Test
   void test() {
     CheckVerifier.newVerifier()
-      .onFile(mainCodeSourcesPath("checks/SynchronizedLockCheckSample.java"))
-      .withCheck(new SynchronizedLockCheck())
+      .onFile(mainCodeSourcesPath("checks/SynchronizedOnConcurrentObjectCheckSample.java"))
+      .withCheck(new SynchronizedOnConcurrentObjectCheck())
       .verifyIssues();
   }
+
+  @Test
+  void test_without_semantic() {
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath("checks/SynchronizedOnConcurrentObjectCheckSample.java"))
+      .withCheck(new SynchronizedOnConcurrentObjectCheck())
+      .withoutSemantic()
+      .verifyIssues();
+  }
+
 }