From 8038b8fd8605a4a42c7a9dcd12aca8d5000e3474 Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Thu, 11 Nov 2010 15:57:44 +0100 Subject: [PATCH] new "non-atomic operation on volatile field" inspection drop "arithmetic operation on volatile field" inspection --- .../siyeh/InspectionGadgetsBundle.properties | 4 +- .../com/siyeh/ig/InspectionGadgetsPlugin.java | 2 +- .../ArithmeticOnVolatileFieldInspection.java | 105 --------------- ...micOperationOnVolatileFieldInspection.java | 127 ++++++++++++++++++ .../ArithmeticOnVolatileField.html | 10 -- .../NonAtomicOperationOnVolatileField.html | 13 ++ 6 files changed, 143 insertions(+), 118 deletions(-) delete mode 100644 plugins/InspectionGadgets/src/com/siyeh/ig/threading/ArithmeticOnVolatileFieldInspection.java create mode 100644 plugins/InspectionGadgets/src/com/siyeh/ig/threading/NonAtomicOperationOnVolatileFieldInspection.java delete mode 100644 plugins/InspectionGadgets/src/inspectionDescriptions/ArithmeticOnVolatileField.html create mode 100644 plugins/InspectionGadgets/src/inspectionDescriptions/NonAtomicOperationOnVolatileField.html diff --git a/plugins/InspectionGadgets/src/com/siyeh/InspectionGadgetsBundle.properties b/plugins/InspectionGadgets/src/com/siyeh/InspectionGadgetsBundle.properties index e2c4033377fd..859e402e3b82 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/InspectionGadgetsBundle.properties +++ b/plugins/InspectionGadgets/src/com/siyeh/InspectionGadgetsBundle.properties @@ -816,7 +816,7 @@ set.replaceable.by.enum.set.display.name=Set replaceable with EnumSet non.static.inner.class.in.secure.context.display.name=Non-static inner class in secure context tail.recursion.display.name=Tail recursion finally.block.cannot.complete.normally.display.name='finally' block which can not complete normally -arithmetic.on.volatile.field.display.name=Arithmetic operation on volatile field +non.atomic.operation.on.volatile.field.display.name=Non-atomic operation on volatile field public.static.collection.field.display.name='public static' collection field non.exception.name.ends.with.exception.display.name=Non-exception class name ends with 'Exception' synchronized.method.display.name='synchronized' method @@ -993,7 +993,7 @@ serializable.inner.class.has.serial.version.uid.field.problem.descriptor=Inner c serializable.inner.class.with.non.serializable.outer.class.problem.descriptor=Inner class #ref is serializable while its outer class is not #loc busy.wait.problem.descriptor=Call to Thread.#ref() in a loop, probably busy-waiting #loc sleep.while.holding.lock.problem.descriptor=Call to Thread.#ref() while synchronized #loc -arithmetic.on.volatile.field.problem.descriptor=Arithmetic operation on volatile field #ref #loc +non.atomic.operation.on.volatile.field.problem.descriptor=Non-atomic operation on volatile field #ref #loc call.to.native.method.while.locked.problem.descriptor=Call to native method #ref() in a synchronized context #loc object.notify.problem.descriptor=#ref should probably be replaced with 'notifyAll()' #loc condition.signal.problem.descriptor=#ref should probably be replaced with 'signalAll()' #loc diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/InspectionGadgetsPlugin.java b/plugins/InspectionGadgets/src/com/siyeh/ig/InspectionGadgetsPlugin.java index c3ee767d914f..466d5a5c6c98 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/InspectionGadgetsPlugin.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/InspectionGadgetsPlugin.java @@ -821,7 +821,6 @@ public class InspectionGadgetsPlugin implements ApplicationComponent, private void registerThreadingInspections() { m_inspectionClasses.add(AccessToNonThreadSafeStaticFieldFromInstanceInspection.class); m_inspectionClasses.add(AccessToStaticFieldLockedOnInstanceInspection.class); - m_inspectionClasses.add(ArithmeticOnVolatileFieldInspection.class); m_inspectionClasses.add(AwaitNotInLoopInspection.class); m_inspectionClasses.add(AwaitWithoutCorrespondingSignalInspection.class); m_inspectionClasses.add(BusyWaitInspection.class); @@ -834,6 +833,7 @@ public class InspectionGadgetsPlugin implements ApplicationComponent, m_inspectionClasses.add(MethodMayBeSynchronizedInspection.class); m_inspectionClasses.add(NakedNotifyInspection.class); m_inspectionClasses.add(NestedSynchronizedStatementInspection.class); + m_inspectionClasses.add(NonAtomicOperationOnVolatileFieldInspection.class); m_inspectionClasses.add(NonSynchronizedMethodOverridesSynchronizedMethodInspection.class); m_inspectionClasses.add(NotifyCalledOnConditionInspection.class); m_inspectionClasses.add(NotifyNotInSynchronizedContextInspection.class); diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/threading/ArithmeticOnVolatileFieldInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/threading/ArithmeticOnVolatileFieldInspection.java deleted file mode 100644 index 212a98d849d3..000000000000 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/threading/ArithmeticOnVolatileFieldInspection.java +++ /dev/null @@ -1,105 +0,0 @@ -/* - * Copyright 2003-2007 Dave Griffith, Bas Leijdekkers - * - * 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.psi.*; -import com.intellij.psi.tree.IElementType; -import com.siyeh.InspectionGadgetsBundle; -import com.siyeh.ig.BaseInspection; -import com.siyeh.ig.BaseInspectionVisitor; -import com.siyeh.ig.psiutils.WellFormednessUtils; -import org.jetbrains.annotations.NotNull; - -public class ArithmeticOnVolatileFieldInspection extends BaseInspection { - - @NotNull - public String getDisplayName() { - return InspectionGadgetsBundle.message( - "arithmetic.on.volatile.field.display.name"); - } - - @NotNull - protected String buildErrorString(Object... infos) { - return InspectionGadgetsBundle.message( - "arithmetic.on.volatile.field.problem.descriptor"); - } - - public BaseInspectionVisitor buildVisitor() { - return new AritmeticOnVolatileFieldInspection(); - } - - private static class AritmeticOnVolatileFieldInspection - extends BaseInspectionVisitor { - - @Override public void visitBinaryExpression( - @NotNull PsiBinaryExpression expression) { - super.visitBinaryExpression(expression); - if (expression.getROperand() == null) { - return; - } - final PsiJavaToken sign = expression.getOperationSign(); - final IElementType tokenType = sign.getTokenType(); - if (!JavaTokenType.ASTERISK.equals(tokenType) && - !JavaTokenType.DIV.equals(tokenType) && - !JavaTokenType.PLUS.equals(tokenType) && - !JavaTokenType.MINUS.equals(tokenType) && - !JavaTokenType.PERC.equals(tokenType)) { - return; - } - final PsiExpression lhs = expression.getLOperand(); - checkForVolatile(lhs); - final PsiExpression rhs = expression.getROperand(); - checkForVolatile(rhs); - } - - @Override public void visitAssignmentExpression( - @NotNull PsiAssignmentExpression expression) { - super.visitAssignmentExpression(expression); - if (!WellFormednessUtils.isWellFormed(expression)) { - return; - } - final PsiJavaToken sign = expression.getOperationSign(); - final IElementType tokenType = sign.getTokenType(); - if (!JavaTokenType.ASTERISKEQ.equals(tokenType) && - !JavaTokenType.DIVEQ.equals(tokenType) && - !JavaTokenType.PLUSEQ.equals(tokenType) && - !JavaTokenType.MINUSEQ.equals(tokenType) && - !JavaTokenType.PERCEQ.equals(tokenType)) { - return; - } - final PsiExpression lhs = expression.getLExpression(); - checkForVolatile(lhs); - final PsiExpression rhs = expression.getRExpression(); - checkForVolatile(rhs); - } - - private void checkForVolatile(PsiExpression expression) { - if (!(expression instanceof PsiReferenceExpression)) { - return; - } - final PsiReferenceExpression reference = - (PsiReferenceExpression)expression; - final PsiElement referent = reference.resolve(); - if (!(referent instanceof PsiField)) { - return; - } - final PsiField field = (PsiField)referent; - if (field.hasModifierProperty(PsiModifier.VOLATILE)) { - registerError(expression); - } - } - } -} \ No newline at end of file diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/threading/NonAtomicOperationOnVolatileFieldInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/threading/NonAtomicOperationOnVolatileFieldInspection.java new file mode 100644 index 000000000000..920adbe19f62 --- /dev/null +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/threading/NonAtomicOperationOnVolatileFieldInspection.java @@ -0,0 +1,127 @@ +/* + * Copyright 2003-2010 Dave Griffith, Bas Leijdekkers + * + * 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.psi.*; +import com.intellij.psi.tree.IElementType; +import com.siyeh.InspectionGadgetsBundle; +import com.siyeh.ig.BaseInspection; +import com.siyeh.ig.BaseInspectionVisitor; +import com.siyeh.ig.psiutils.VariableAccessUtils; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +public class NonAtomicOperationOnVolatileFieldInspection + extends BaseInspection { + + @Override + @NotNull + public String getDisplayName() { + return InspectionGadgetsBundle.message( + "non.atomic.operation.on.volatile.field.display.name"); + } + + @Override + @NotNull + protected String buildErrorString(Object... infos) { + return InspectionGadgetsBundle.message( + "non.atomic.operation.on.volatile.field.problem.descriptor"); + } + + @Override + public BaseInspectionVisitor buildVisitor() { + return new NonAtomicOperationOnVolatileFieldVisitor(); + } + + private static class NonAtomicOperationOnVolatileFieldVisitor + extends BaseInspectionVisitor { + + @Override public void visitAssignmentExpression( + @NotNull PsiAssignmentExpression expression) { + super.visitAssignmentExpression(expression); + final PsiExpression rhs = expression.getRExpression(); + if (rhs == null) { + return; + } + final PsiExpression lhs = expression.getLExpression(); + final PsiField volatileField = findVolatileField(lhs); + if (volatileField == null) { + return; + } + final IElementType tokenType = expression.getOperationTokenType(); + if (tokenType.equals(JavaTokenType.PLUSEQ) || + tokenType.equals(JavaTokenType.MINUSEQ) || + tokenType.equals(JavaTokenType.ASTERISKEQ) || + tokenType.equals(JavaTokenType.DIVEQ) || + tokenType.equals(JavaTokenType.ANDEQ) || + tokenType.equals(JavaTokenType.OREQ)|| + tokenType.equals(JavaTokenType.XOREQ)|| + tokenType.equals(JavaTokenType.PERCEQ)|| + tokenType.equals(JavaTokenType.LTLTEQ)|| + tokenType.equals(JavaTokenType.GTGTEQ)|| + tokenType.equals(JavaTokenType.GTGTGTEQ)) { + registerError(lhs); + return; + } + if (VariableAccessUtils.variableIsUsed(volatileField, rhs)) { + registerError(lhs); + } + } + + @Override + public void visitPrefixExpression(PsiPrefixExpression expression) { + super.visitPrefixExpression(expression); + final PsiExpression operand = expression.getOperand(); + if (operand == null) { + return; + } + final PsiField volatileField = findVolatileField(operand); + if (volatileField == null) { + return; + } + registerError(operand); + } + + @Override + public void visitPostfixExpression(PsiPostfixExpression expression) { + super.visitPostfixExpression(expression); + final PsiExpression operand = expression.getOperand(); + final PsiField volatileField = findVolatileField(operand); + if (volatileField == null) { + return; + } + registerError(operand); + } + + @Nullable + private static PsiField findVolatileField(PsiExpression expression) { + if (!(expression instanceof PsiReferenceExpression)) { + return null; + } + final PsiReferenceExpression reference = + (PsiReferenceExpression)expression; + final PsiElement referent = reference.resolve(); + if (!(referent instanceof PsiField)) { + return null; + } + final PsiField field = (PsiField)referent; + if (!field.hasModifierProperty(PsiModifier.VOLATILE)) { + return null; + } + return field; + } + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/src/inspectionDescriptions/ArithmeticOnVolatileField.html b/plugins/InspectionGadgets/src/inspectionDescriptions/ArithmeticOnVolatileField.html deleted file mode 100644 index 9fd582c5ed02..000000000000 --- a/plugins/InspectionGadgets/src/inspectionDescriptions/ArithmeticOnVolatileField.html +++ /dev/null @@ -1,10 +0,0 @@ - -
- -This inspection reports any uses of volatile fields in arithmetic operations. -It's a common misconception that such operations are effectively atomic, but the -Java Memory Model only specifies that loads and stores on volatile variables are -atomic. This can lead to unexpected results, including lost updates, when using -volatile fields in arithmetic operations. -
Powered by InspectionGadgets
- \ No newline at end of file diff --git a/plugins/InspectionGadgets/src/inspectionDescriptions/NonAtomicOperationOnVolatileField.html b/plugins/InspectionGadgets/src/inspectionDescriptions/NonAtomicOperationOnVolatileField.html new file mode 100644 index 000000000000..2e6bcad4ccad --- /dev/null +++ b/plugins/InspectionGadgets/src/inspectionDescriptions/NonAtomicOperationOnVolatileField.html @@ -0,0 +1,13 @@ + +
+ +This inspection reports any non-atomic operations on volatile fields. Non-atomic +operations on volatile fields are operations where the volatile field is read and +the value is used to update the volatile field. It is possible for the value of the +field to change between the read and write, making the operation possibly invalid. +In such cases it is better to use one of the Atomic* or + Atomic*FieldUpdater classes +from the java.util.concurrent.atomic package. +
New in 10, Powered by InspectionGadgets
+ \ No newline at end of file