IDEA-169967 Enhance "'while' loop spins on field" to suggest to insert Thread.onSpinWait() in Java 9

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/BlockUtils.java

@@ -50,9 +50,13 @@ public class BlockUtils {
       for (PsiStatement newStatement : newStatements) {
         codeBlock.add(newStatement);
       }
-      codeBlock.add(oldStatement);
+      int oldCount = 0;
+      if(!(oldStatement instanceof PsiEmptyStatement)) {
+        oldCount = 1;
+        codeBlock.add(oldStatement);
+      }
       final PsiStatement[] statements = ((PsiBlockStatement)oldStatement.replace(newBlockStatement)).getCodeBlock().getStatements();
-      result = statements[statements.length - 2];
+      result = statements[statements.length - 1 - oldCount];
     }
     return (PsiStatement)result;
   }

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ControlFlowUtils.java

@@ -832,6 +832,33 @@ public class ControlFlowUtils {
     return false;
   }
 
+  /**
+   * Returns true if statement essentially contains no executable code
+   *
+   * @param statement statement to test
+   * @return true if statement essentially contains no executable code
+   */
+  public static boolean statementIsEmpty(PsiStatement statement) {
+    if (statement == null) {
+      return false;
+    }
+    if (statement instanceof PsiEmptyStatement) {
+      return true;
+    }
+    if (statement instanceof PsiBlockStatement) {
+      final PsiBlockStatement blockStatement = (PsiBlockStatement)statement;
+      final PsiCodeBlock codeBlock = blockStatement.getCodeBlock();
+      final PsiStatement[] codeBlockStatements = codeBlock.getStatements();
+      for (PsiStatement codeBlockStatement : codeBlockStatements) {
+        if (!statementIsEmpty(codeBlockStatement)) {
+          return false;
+        }
+      }
+      return true;
+    }
+    return false;
+  }
+
   public enum InitializerUsageStatus {
     // Variable is declared just before the wanted place
     DECLARED_JUST_BEFORE,

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/threading/WhileLoopSpinsOnFieldInspection.java

@@ -15,18 +15,27 @@
  */
 package com.siyeh.ig.threading;
 
+import com.intellij.codeInspection.ProblemDescriptor;
 import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel;
+import com.intellij.openapi.project.Project;
 import com.intellij.psi.*;
+import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.psi.util.PsiUtil;
 import com.siyeh.InspectionGadgetsBundle;
 import com.siyeh.ig.BaseInspection;
 import com.siyeh.ig.BaseInspectionVisitor;
+import com.siyeh.ig.InspectionGadgetsFix;
+import com.siyeh.ig.callMatcher.CallMatcher;
+import com.siyeh.ig.psiutils.BlockUtils;
+import com.siyeh.ig.psiutils.ControlFlowUtils;
 import com.siyeh.ig.psiutils.ExpressionUtils;
 import com.siyeh.ig.psiutils.VariableAccessUtils;
+import org.jetbrains.annotations.Nls;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
 import javax.swing.*;
+import java.util.function.Predicate;
 
 public class WhileLoopSpinsOnFieldInspection extends BaseInspection {
 
@@ -47,6 +56,12 @@ public class WhileLoopSpinsOnFieldInspection extends BaseInspection {
       "while.loop.spins.on.field.problem.descriptor");
   }
 
+  @Nullable
+  @Override
+  protected InspectionGadgetsFix buildFix(Object... infos) {
+    return new SpinLoopFix((PsiField)infos[0], (boolean)infos[1]);
+  }
+
   @Override
   @Nullable
   public JComponent createOptionsPanel() {
@@ -66,7 +81,8 @@ public class WhileLoopSpinsOnFieldInspection extends BaseInspection {
     public void visitWhileStatement(@NotNull PsiWhileStatement statement) {
       super.visitWhileStatement(statement);
       final PsiStatement body = statement.getBody();
-      if (ignoreNonEmtpyLoops && !statementIsEmpty(body)) {
+      boolean empty = ControlFlowUtils.statementIsEmpty(body);
+      if (ignoreNonEmtpyLoops && !empty) {
         return;
       }
       final PsiExpression condition = statement.getCondition();
@@ -74,20 +90,24 @@ public class WhileLoopSpinsOnFieldInspection extends BaseInspection {
       if (field == null) {
         return;
       }
-      if (body != null && (VariableAccessUtils.variableIsAssigned(field, body) ||
-                           containsWaitCall(body))) {
+      boolean hasOnSpinWait = containsCall(body, CallMatcher.staticCall("java.lang.Thread", "onSpinWait"));
+      boolean java9 = PsiUtil.isLanguageLevel9OrHigher(field);
+      if (java9 && hasOnSpinWait && field.hasModifierProperty(PsiModifier.VOLATILE)) {
         return;
       }
-      registerStatementError(statement);
+      if (body != null && (VariableAccessUtils.variableIsAssigned(field, body) || containsCall(body, ThreadingUtils::isWaitCall))) {
+        return;
+      }
+      registerStatementError(statement, field, java9 && !hasOnSpinWait);
     }
 
-    private boolean containsWaitCall(PsiElement element) {
+    private boolean containsCall(PsiElement element, Predicate<PsiMethodCallExpression> predicate) {
       final boolean[] result = new boolean[1];
       element.accept(new JavaRecursiveElementWalkingVisitor() {
         @Override
         public void visitMethodCallExpression(PsiMethodCallExpression expression) {
           super.visitMethodCallExpression(expression);
-          if (ThreadingUtils.isWaitCall(expression)) {
+          if (predicate.test(expression)) {
             result[0] = true;
             stopWalking();
           }
@@ -152,33 +172,78 @@ public class WhileLoopSpinsOnFieldInspection extends BaseInspection {
         return null;
       }
       final PsiField field = (PsiField)referent;
-      if (field.hasModifierProperty(PsiModifier.VOLATILE)) {
+      if (field.hasModifierProperty(PsiModifier.VOLATILE) && !PsiUtil.isLanguageLevel9OrHigher(field)) {
         return null;
       }
       else {
         return field;
       }
     }
+  }
 
-    private boolean statementIsEmpty(PsiStatement statement) {
-      if (statement == null) {
-        return false;
+  private static class SpinLoopFix extends InspectionGadgetsFix {
+    private final SmartPsiElementPointer<PsiField> myFieldPointer;
+    private final String myFieldName;
+    private final boolean myAddOnSpinWait;
+    private final boolean myAddVolatile;
+
+    public SpinLoopFix(PsiField field, boolean addOnSpinWait) {
+      myFieldPointer = SmartPointerManager.getInstance(field.getProject()).createSmartPsiElementPointer(field);
+      myFieldName = field.getName();
+      myAddOnSpinWait = addOnSpinWait;
+      myAddVolatile = !field.hasModifierProperty(PsiModifier.VOLATILE);
+    }
+
+    @Nls
+    @NotNull
+    @Override
+    public String getName() {
+      if(myAddOnSpinWait && myAddVolatile) {
+        return "Declare field '" + myFieldName + "' as 'volatile' and add Thread.onSpinWait()";
       }
-      if (statement instanceof PsiEmptyStatement) {
-        return true;
+      if(myAddOnSpinWait) {
+        return "Add Thread.onSpinWait()";
       }
-      if (statement instanceof PsiBlockStatement) {
-        final PsiBlockStatement blockStatement = (PsiBlockStatement)statement;
-        final PsiCodeBlock codeBlock = blockStatement.getCodeBlock();
-        final PsiStatement[] codeBlockStatements = codeBlock.getStatements();
-        for (PsiStatement codeBlockStatement : codeBlockStatements) {
-          if (!statementIsEmpty(codeBlockStatement)) {
-            return false;
-          }
-        }
-        return true;
+      return "Declare field '" + myFieldName + "' as 'volatile'";
+    }
+
+    @Nls
+    @NotNull
+    @Override
+    public String getFamilyName() {
+      return "Declare field as 'volatile'";
+    }
+
+    @Override
+    protected void doFix(Project project, ProblemDescriptor descriptor) {
+      if(myAddVolatile) {
+        addVolatile(myFieldPointer.getElement());
       }
-      return false;
+      if(myAddOnSpinWait) {
+        addOnSpinWait(descriptor.getStartElement());
+      }
+    }
+
+    private static void addOnSpinWait(PsiElement element) {
+      PsiLoopStatement loop = PsiTreeUtil.getParentOfType(element, PsiLoopStatement.class);
+      if(loop == null) return;
+      PsiStatement body = loop.getBody();
+      if(body == null) return;
+      PsiStatement spinCall =
+        JavaPsiFacade.getElementFactory(element.getProject()).createStatementFromText("java.lang.Thread.onSpinWait();", element);
+      if(body instanceof PsiBlockStatement) {
+        PsiCodeBlock block = ((PsiBlockStatement)body).getCodeBlock();
+        block.addAfter(spinCall, null);
+      } else {
+        BlockUtils.addBefore(body, spinCall);
+      }
+    }
+
+    private static void addVolatile(PsiField field) {
+      if (field == null) return;
+      PsiModifierList list = field.getModifierList();
+      if (list == null) return;
+      list.setModifierProperty(PsiModifier.VOLATILE, true);
     }
   }
 }

plugins/InspectionGadgets/src/inspectionDescriptions/WhileLoopSpinsOnField.html

@@ -1,11 +1,17 @@
 <html>
 <body>
 Reports on <b>while</b> loops which spin on the
-value of a non-volatile field, waiting for it to be changed by another thread. In addition to being potentially
-extremely CPU intensive when little work is done inside the loop, such
+value of a non-volatile field, waiting for it to be changed by another thread.
+<p>
+In addition to being potentially extremely CPU intensive when little work is done inside the loop, such
 loops are likely have different semantics than intended, as the Java Memory Model allows such field accesses
 to be hoisted out of the loop, causing the loop to never complete even if another thread does change the
 field's value.
+</p>
+<p>
+  Additionally since Java 9 it's recommended to call <code>Thread.onSpinWait()</code> inside spin loop
+  on a volatile field which may significantly improve performance on some hardware.
+</p>
 <!-- tooltip end -->
 <p>
 

plugins/InspectionGadgets/test/com/siyeh/igtest/threading/while_loop_spins_on_field/afterSimple.java

@@ -0,0 +1,26 @@
+/*
+ * Copyright 2000-2017 JetBrains s.r.o.
+ *
+ * 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.
+ */
+
+// "Declare field 'b' as 'volatile'" "true"
+import java.util.*;
+
+public class Test {
+    private volatile boolean b;
+
+    public void test() {
+        while (b);
+    }
+}

plugins/InspectionGadgets/test/com/siyeh/igtest/threading/while_loop_spins_on_field/afterSimpleCodeBlockJava9.java

@@ -0,0 +1,28 @@
+/*
+ * Copyright 2000-2017 JetBrains s.r.o.
+ *
+ * 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.
+ */
+
+// "Declare field 'b' as 'volatile' and add Thread.onSpinWait()" "true"
+import java.util.*;
+
+public class Test {
+    private volatile boolean b;
+
+    public void test() {
+        while (b) {
+            Thread.onSpinWait();
+        }
+    }
+}

plugins/InspectionGadgets/test/com/siyeh/igtest/threading/while_loop_spins_on_field/afterSimpleHasSpinWaitJava9.java

@@ -0,0 +1,31 @@
+/*
+ * Copyright 2000-2017 JetBrains s.r.o.
+ *
+ * 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.
+ */
+
+// "Declare field 'b' as 'volatile'" "true"
+import java.util.*;
+
+public class Test {
+    private volatile boolean b;
+
+    public void test() {
+        int counter = 0;
+        while (b) {
+            if(counter++ > 100) {
+                Thread.onSpinWait();
+            }
+        }
+    }
+}

plugins/InspectionGadgets/test/com/siyeh/igtest/threading/while_loop_spins_on_field/afterSimpleJava9.java

@@ -0,0 +1,28 @@
+/*
+ * Copyright 2000-2017 JetBrains s.r.o.
+ *
+ * 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.
+ */
+
+// "Declare field 'b' as 'volatile' and add Thread.onSpinWait()" "true"
+import java.util.*;
+
+public class Test {
+    private volatile boolean b;
+
+    public void test() {
+        while (b) {
+            Thread.onSpinWait();
+        }
+    }
+}

plugins/InspectionGadgets/test/com/siyeh/igtest/threading/while_loop_spins_on_field/afterVolatileJava9.java

@@ -0,0 +1,28 @@
+/*
+ * Copyright 2000-2017 JetBrains s.r.o.
+ *
+ * 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.
+ */
+
+// "Add Thread.onSpinWait()" "true"
+import java.util.*;
+
+public class Test {
+    private volatile boolean b;
+
+    public void test() {
+        while (b) {
+            Thread.onSpinWait();
+        }
+    }
+}

plugins/InspectionGadgets/test/com/siyeh/igtest/threading/while_loop_spins_on_field/beforeSimple.java

@@ -0,0 +1,26 @@
+/*
+ * Copyright 2000-2017 JetBrains s.r.o.
+ *
+ * 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.
+ */
+
+// "Declare field 'b' as 'volatile'" "true"
+import java.util.*;
+
+public class Test {
+    private boolean b;
+
+    public void test() {
+        whi<caret>le (b);
+    }
+}

plugins/InspectionGadgets/test/com/siyeh/igtest/threading/while_loop_spins_on_field/beforeSimpleCodeBlockJava9.java

@@ -0,0 +1,26 @@
+/*
+ * Copyright 2000-2017 JetBrains s.r.o.
+ *
+ * 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.
+ */
+
+// "Declare field 'b' as 'volatile' and add Thread.onSpinWait()" "true"
+import java.util.*;
+
+public class Test {
+    private boolean b;
+
+    public void test() {
+        whi<caret>le (b) {}
+    }
+}

plugins/InspectionGadgets/test/com/siyeh/igtest/threading/while_loop_spins_on_field/beforeSimpleHasSpinWaitJava9.java

@@ -0,0 +1,31 @@
+/*
+ * Copyright 2000-2017 JetBrains s.r.o.
+ *
+ * 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.
+ */
+
+// "Declare field 'b' as 'volatile'" "true"
+import java.util.*;
+
+public class Test {
+    private boolean b;
+
+    public void test() {
+        int counter = 0;
+        whi<caret>le (b) {
+            if(counter++ > 100) {
+                Thread.onSpinWait();
+            }
+        }
+    }
+}

plugins/InspectionGadgets/test/com/siyeh/igtest/threading/while_loop_spins_on_field/beforeSimpleJava9.java

@@ -0,0 +1,26 @@
+/*
+ * Copyright 2000-2017 JetBrains s.r.o.
+ *
+ * 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.
+ */
+
+// "Declare field 'b' as 'volatile' and add Thread.onSpinWait()" "true"
+import java.util.*;
+
+public class Test {
+    private boolean b;
+
+    public void test() {
+        whi<caret>le (b);
+    }
+}

plugins/InspectionGadgets/test/com/siyeh/igtest/threading/while_loop_spins_on_field/beforeVolatile.java

@@ -0,0 +1,26 @@
+/*
+ * Copyright 2000-2017 JetBrains s.r.o.
+ *
+ * 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.
+ */
+
+// "Fix all ''while' loop spins on field' problems in file" "false"
+import java.util.*;
+
+public class Test {
+    private volatile boolean b;
+
+    public void test() {
+        whi<caret>le (b);
+    }
+}

plugins/InspectionGadgets/test/com/siyeh/igtest/threading/while_loop_spins_on_field/beforeVolatileJava9.java

@@ -0,0 +1,26 @@
+/*
+ * Copyright 2000-2017 JetBrains s.r.o.
+ *
+ * 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.
+ */
+
+// "Add Thread.onSpinWait()" "true"
+import java.util.*;
+
+public class Test {
+    private volatile boolean b;
+
+    public void test() {
+        whi<caret>le (b);
+    }
+}

plugins/InspectionGadgets/testsrc/com/siyeh/ig/threading/WhileLoopSpinsOnFieldInspectionFixTest.java

@@ -0,0 +1,67 @@
+/*
+ * Copyright 2000-2017 JetBrains s.r.o.
+ *
+ * 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.
+ */
+package com.siyeh.ig.threading;
+
+import com.intellij.codeInsight.daemon.quickFix.LightQuickFixParameterizedTestCase;
+import com.intellij.codeInspection.LocalInspectionTool;
+import com.intellij.openapi.application.PluginPathManager;
+import com.intellij.openapi.projectRoots.Sdk;
+import com.intellij.pom.java.LanguageLevel;
+import com.intellij.testFramework.IdeaTestUtil;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * @author Tagir Valeev
+ */
+public class WhileLoopSpinsOnFieldInspectionFixTest extends LightQuickFixParameterizedTestCase {
+
+  @NotNull
+  @Override
+  protected LocalInspectionTool[] configureLocalInspectionTools() {
+    return new LocalInspectionTool[]{new WhileLoopSpinsOnFieldInspection()};
+  }
+
+  @Override
+  protected LanguageLevel getLanguageLevel() {
+    if(getTestName(false).endsWith("Java9.java")) {
+      return LanguageLevel.JDK_1_9;
+    }
+    return LanguageLevel.JDK_1_8;
+  }
+
+  @Override
+  protected Sdk getProjectJDK() {
+    if(getTestName(false).endsWith("Java9.java")) {
+      return IdeaTestUtil.getMockJdk9();
+    }
+    return IdeaTestUtil.getMockJdk18();
+  }
+
+  public void test() throws Exception {
+    doAllTests();
+  }
+
+  @Override
+  protected String getBasePath() {
+    return "/com/siyeh/igtest/threading/while_loop_spins_on_field";
+  }
+
+  @NotNull
+  @Override
+  protected String getTestDataPath() {
+    return PluginPathManager.getPluginHomePath("InspectionGadgets") + "/test";
+  }
+}