test and expand functionality of "Synchronization on an object initialized with a literal" inspection

This commit is contained in:
Bas Leijdekkers
2012-09-05 12:14:05 +02:00
parent 99a391d379
commit 56a9ebcc20
6 changed files with 122 additions and 44 deletions
@@ -1026,8 +1026,11 @@ notify.not.in.synchronized.context.problem.descriptor=Call to <code>#ref()</code
thread.run.problem.descriptor=Calls to <code>#ref()</code> should probably be replaced with 'start()' #loc
thread.start.in.construction.problem.descriptor=Call to <code>#ref</code> during object construction #loc
synchronize.on.lock.problem.descriptor=Synchronization on a ''{0}'' object is unlikely to be intentional #loc
synchronized.on.literal.object.descriptor=Synchronization on {0} <code>#ref</code> which is initialized by a literal #loc
synchronized.on.literal.object.problem.descriptor=Synchronization on {0} <code>#ref</code> which is initialized by a literal #loc
synchronized.on.direct.literal.object.problem.descriptor=Synchronization on {0} literal <code>#ref</code> #loc
synchronized.on.possibly.literal.object.problem.descriptor=Synchronization on {0} <code>#ref</code> #loc
synchronize.on.non.final.field.problem.descriptor=Synchronization on a non-final field <code>#ref</code> #loc
synchronized.on.literal.object.warn.on.all.option=Warn on all possible literals
synchronize.on.this.problem.descriptor=Lock operations on 'this' may have unforeseen side-effects #loc
nested.synchronized.statement.problem.descriptor=Nested <code>#ref</code> statement #loc
empty.synchronized.statement.problem.descriptor=Empty <code>#ref</code> statement #loc
@@ -1,5 +1,5 @@
/*
* Copyright 2007-2010 Bas Leijdekkers
* Copyright 2007-2012 Bas Leijdekkers
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -15,29 +15,50 @@
*/
package com.siyeh.ig.threading;
import com.intellij.openapi.project.Project;
import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel;
import com.intellij.psi.*;
import com.siyeh.InspectionGadgetsBundle;
import com.siyeh.ig.BaseInspection;
import com.siyeh.ig.BaseInspectionVisitor;
import com.siyeh.ig.psiutils.ExpressionUtils;
import com.siyeh.ig.psiutils.TypeUtils;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import javax.swing.*;
public class SynchronizedOnLiteralObjectInspection extends BaseInspection {
@SuppressWarnings("PublicField") public boolean warnOnAllPossiblyLiterals = false;
@Override
@NotNull
public String getDisplayName() {
return InspectionGadgetsBundle.message(
"synchronized.on.literal.object.name");
return InspectionGadgetsBundle.message("synchronized.on.literal.object.name");
}
@Override
@NotNull
protected String buildErrorString(Object... infos) {
final PsiType type = (PsiType)infos[0];
return InspectionGadgetsBundle.message(
"synchronized.on.literal.object.descriptor",
type.getPresentableText());
final String typeText = ((PsiType)infos[0]).getPresentableText();
final int message = ((Integer)infos[1]).intValue();
switch (message) {
case 1:
return InspectionGadgetsBundle.message("synchronized.on.literal.object.problem.descriptor", typeText);
case 2:
return InspectionGadgetsBundle.message("synchronized.on.direct.literal.object.problem.descriptor", typeText);
case 3:
return InspectionGadgetsBundle.message("synchronized.on.possibly.literal.object.problem.descriptor", typeText);
default:
throw new AssertionError();
}
}
@Nullable
@Override
public JComponent createOptionsPanel() {
return new SingleCheckboxOptionsPanel(InspectionGadgetsBundle.message("synchronized.on.literal.object.warn.on.all.option"),
this, "warnOnAllPossiblyLiterals");
}
@Override
@@ -45,55 +66,53 @@ public class SynchronizedOnLiteralObjectInspection extends BaseInspection {
return new SynchronizeOnLiteralVisitor();
}
private static class SynchronizeOnLiteralVisitor
extends BaseInspectionVisitor {
private class SynchronizeOnLiteralVisitor extends BaseInspectionVisitor {
@Override
public void visitSynchronizedStatement(
@NotNull PsiSynchronizedStatement statement) {
public void visitSynchronizedStatement(@NotNull PsiSynchronizedStatement statement) {
super.visitSynchronizedStatement(statement);
final PsiExpression lockExpression = statement.getLockExpression();
if (lockExpression == null) {
return;
}
final PsiType type = lockExpression.getType();
if (type == null) {
return;
}
if (!type.equalsToText(CommonClassNames.JAVA_LANG_STRING) &&
!type.equalsToText(CommonClassNames.JAVA_LANG_BOOLEAN) &&
!type.equalsToText(CommonClassNames.JAVA_LANG_CHARACTER)) {
final PsiClassType javaLangNumberType = TypeUtils.getType(CommonClassNames.JAVA_LANG_NUMBER, statement);
if (!javaLangNumberType.isAssignableFrom(type)) {
return;
}
}
if (!(lockExpression instanceof PsiReferenceExpression)) {
if (ExpressionUtils.isLiteral(lockExpression)) {
registerError(lockExpression, type, Integer.valueOf(2));
}
else if (warnOnAllPossiblyLiterals) {
registerError(lockExpression, type, Integer.valueOf(3));
}
return;
}
if (!isNumberOrStringType(lockExpression)) {
return;
}
final PsiReferenceExpression referenceExpression =
(PsiReferenceExpression)lockExpression;
final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)lockExpression;
final PsiElement target = referenceExpression.resolve();
if (!(target instanceof PsiVariable)) {
if (warnOnAllPossiblyLiterals) {
registerError(lockExpression, type, Integer.valueOf(3));
}
return;
}
final PsiVariable variable = (PsiVariable)target;
final PsiExpression initializer = variable.getInitializer();
if (!(initializer instanceof PsiLiteralExpression)) {
if (!ExpressionUtils.isLiteral(initializer)) {
if (warnOnAllPossiblyLiterals) {
registerError(lockExpression, type, Integer.valueOf(3));
}
return;
}
registerError(lockExpression, lockExpression.getType());
}
public static boolean isNumberOrStringType(PsiExpression expression) {
final PsiType type = expression.getType();
if (type == null) {
return false;
}
final Project project = expression.getProject();
final JavaPsiFacade psiFacade = JavaPsiFacade.getInstance(project);
final PsiClass javaLangNumberClass =
psiFacade.findClass(CommonClassNames.JAVA_LANG_NUMBER,
expression.getResolveScope());
if (javaLangNumberClass == null) {
return false;
}
final PsiElementFactory elementFactory =
psiFacade.getElementFactory();
final PsiClassType javaLangNumberType =
elementFactory.createType(javaLangNumberClass);
return type.equalsToText(CommonClassNames.JAVA_LANG_STRING) ||
type.equalsToText(CommonClassNames.JAVA_LANG_BOOLEAN) ||
type.equalsToText(CommonClassNames.JAVA_LANG_CHARACTER) ||
javaLangNumberType.isAssignableFrom(type);
registerError(lockExpression, type, Integer.valueOf(1));
}
}
}
@@ -2,13 +2,15 @@
<body>
This inspection reports any <b>synchronized</b>
block which locks on an object which is initialized with a literal.
String literals are interned and Number literals can be allocated from a cache.
String literals are interned and Character, Boolean and Number literals can be allocated from a cache.
Because of this, it is possible that some other part of the system
which uses an object initialized with the same literal, is actually holding a
reference to the exact same object. This can create unexpected dead-lock situations,
if the lock object was thought to be private.
<!-- tooltip end -->
<p>
Use the checkbox below to warn on any synchronization on String, Character, Boolean and Number objects.
<p>
<small>Powered by InspectionGadgets</small>
</body>
</html>
@@ -0,0 +1,14 @@
package com.siyeh.igtest.threading.synchronized_on_literal_object;
class SynchronizedOnLiteralObject {
private Integer myInteger = (Integer)1;
private Character c = 'a';
void foo() {
synchronized (myInteger) {}
synchronized ("asdf") {}
synchronized ((Boolean) true) {}
synchronized (c) {}
}
}
@@ -0,0 +1,30 @@
<?xml version="1.0" encoding="UTF-8"?>
<problems>
<problem>
<file>SynchronizedOnLiteralObject.java</file>
<line>9</line>
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Synchronization on an object initialized with a literal</problem_class>
<description>Synchronization on Integer &lt;code&gt;myInteger&lt;/code&gt; which is initialized by a literal #loc</description>
</problem>
<problem>
<file>SynchronizedOnLiteralObject.java</file>
<line>10</line>
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Synchronization on an object initialized with a literal</problem_class>
<description>Synchronization on String &lt;code&gt;&quot;asdf&quot;&lt;/code&gt; which is initialized by a literal #loc</description>
</problem>
<problem>
<file>SynchronizedOnLiteralObject.java</file>
<line>11</line>
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Synchronization on an object initialized with a literal</problem_class>
<description>Synchronization on Boolean &lt;code&gt;(Boolean) true&lt;/code&gt; which is initialized by a literal #loc</description>
</problem>
<problem>
<file>SynchronizedOnLiteralObject.java</file>
<line>12</line>
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Synchronization on an object initialized with a literal</problem_class>
<description>Synchronization on Character &lt;code&gt;c&lt;/code&gt; which is initialized by a literal #loc</description>
</problem>
</problems>
@@ -0,0 +1,10 @@
package com.siyeh.ig.threading;
import com.siyeh.ig.IGInspectionTestCase;
public class SynchronizedOnLiteralObjectInspectionTest extends IGInspectionTestCase {
public void test() throws Exception {
doTest("com/siyeh/igtest/threading/synchronized_on_literal_object", new SynchronizedOnLiteralObjectInspection());
}
}