From 0b795bec0fad008bf07f6239d21fe63a51cc0ead Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Sat, 7 Sep 2013 21:24:57 +0200 Subject: [PATCH] IDEA-85224 (Inspection "non thread-safe static field access" gives false +ve for static initializers) --- .../src/com/siyeh/ig/psiutils/TypeUtils.java | 29 ++++++---- ...StaticFieldFromInstanceInspectionBase.java | 35 ++++++++++-- ...StaticFieldFromInstanceInspectionTest.java | 54 +++++++++++++++++++ 3 files changed, 104 insertions(+), 14 deletions(-) create mode 100644 plugins/InspectionGadgets/testsrc/com/siyeh/ig/threading/AccessToNonThreadSafeStaticFieldFromInstanceInspectionTest.java diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/TypeUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/TypeUtils.java index 30acc98df96a..5bb992dbbc90 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/TypeUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/TypeUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2012 Dave Griffith, Bas Leijdekkers + * Copyright 2003-2013 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. @@ -23,12 +23,9 @@ import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import java.util.Collection; - public class TypeUtils { - private TypeUtils() { - } + private TypeUtils() {} public static boolean typeEquals(@NonNls @NotNull String typeName, @Nullable PsiType targetType) { return targetType != null && targetType.equalsToText(typeName); @@ -57,6 +54,21 @@ public class TypeUtils { return typeEquals(CommonClassNames.JAVA_LANG_STRING, targetType); } + public static boolean isExpressionTypeAssignableWith(@NotNull PsiExpression expression, @NotNull Iterable rhsTypeTexts) { + final PsiType type = expression.getType(); + if (type == null) { + return false; + } + final PsiElementFactory factory = JavaPsiFacade.getInstance(expression.getProject()).getElementFactory(); + for (String rhsTypeText : rhsTypeTexts) { + final PsiClassType rhsType = factory.createTypeByFQClassName(rhsTypeText, expression.getResolveScope()); + if (type.isAssignableFrom(rhsType)) { + return true; + } + } + return false; + } + public static boolean expressionHasTypeOrSubtype(@Nullable PsiExpression expression, @NonNls @NotNull String typeName) { if (expression == null) { return false; @@ -98,7 +110,7 @@ public class TypeUtils { return null; } - public static boolean expressionHasTypeOrSubtype(@Nullable PsiExpression expression, @NonNls @NotNull Collection typeNames) { + public static boolean expressionHasTypeOrSubtype(@Nullable PsiExpression expression, @NonNls @NotNull Iterable typeNames) { if (expression == null) { return false; } @@ -148,9 +160,6 @@ public class TypeUtils { return false; } final PsiType type = expression.getType(); - if (type == null) { - return false; - } - return PsiType.FLOAT.equals(type) || PsiType.DOUBLE.equals(type); + return type != null && (PsiType.FLOAT.equals(type) || PsiType.DOUBLE.equals(type)); } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/threading/AccessToNonThreadSafeStaticFieldFromInstanceInspectionBase.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/threading/AccessToNonThreadSafeStaticFieldFromInstanceInspectionBase.java index efe3776b29c9..b051e2e653a2 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/threading/AccessToNonThreadSafeStaticFieldFromInstanceInspectionBase.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/threading/AccessToNonThreadSafeStaticFieldFromInstanceInspectionBase.java @@ -21,6 +21,7 @@ import com.intellij.psi.util.PsiTreeUtil; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; +import com.siyeh.ig.psiutils.TypeUtils; import com.siyeh.ig.ui.ExternalizableStringSet; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NonNls; @@ -92,8 +93,12 @@ public class AccessToNonThreadSafeStaticFieldFromInstanceInspectionBase extends return; } } - final PsiExpression qualifier = expression.getQualifierExpression(); - if (qualifier != null) { + if (parent instanceof PsiField || parent instanceof PsiClassInitializer) { + if (parent.hasModifierProperty(PsiModifier.STATIC)) { + return; + } + } + if (expression.getQualifierExpression() != null) { return; } final PsiType type = expression.getType(); @@ -102,8 +107,12 @@ public class AccessToNonThreadSafeStaticFieldFromInstanceInspectionBase extends } final PsiClassType classType = (PsiClassType)type; final String className = classType.rawType().getCanonicalText(); + boolean deepCheck = false; if (!nonThreadSafeClasses.contains(className)) { - return; + if (!TypeUtils.isExpressionTypeAssignableWith(expression, nonThreadSafeClasses)) { + return; + } + deepCheck = true; } final PsiElement target = expression.resolve(); if (!(target instanceof PsiField)) { @@ -113,7 +122,25 @@ public class AccessToNonThreadSafeStaticFieldFromInstanceInspectionBase extends if (!field.hasModifierProperty(PsiModifier.STATIC)) { return; } - registerError(expression, className); + if (deepCheck) { + final PsiExpression initializer = field.getInitializer(); + if (initializer == null) { + return; + } + final PsiType initializerType = initializer.getType(); + if (!(initializerType instanceof PsiClassType)) { + return; + } + final PsiClassType classType2 = (PsiClassType)initializerType; + final String className2 = classType2.rawType().getCanonicalText(); + if (!nonThreadSafeClasses.contains(className2)) { + return; + } + registerError(expression, className2); + } + else { + registerError(expression, className); + } } } } diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/threading/AccessToNonThreadSafeStaticFieldFromInstanceInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/threading/AccessToNonThreadSafeStaticFieldFromInstanceInspectionTest.java new file mode 100644 index 000000000000..ae551ccb3ea0 --- /dev/null +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/threading/AccessToNonThreadSafeStaticFieldFromInstanceInspectionTest.java @@ -0,0 +1,54 @@ +package com.siyeh.ig.threading; + +import com.intellij.codeInspection.LocalInspectionTool; +import com.siyeh.ig.LightInspectionTestCase; + +/** + * @author Bas Leijdekkers + */ +public class AccessToNonThreadSafeStaticFieldFromInstanceInspectionTest extends LightInspectionTestCase { + + @Override + protected String[] getEnvironmentClasses() { + return new String[]{ + "package java.text;" + + "import java.util.Date;" + + "public abstract class DateFormat {" + + " public String format(Date d) {}" + + "}", + "package java.text;" + + "public class SimpleDateFormat extends DateFormat {" + + " public SimpleDateFormat(String s) {}" + + "}" + }; + } + + public void testSimple() { + doTest("import java.util.Date;" + + "import java.text.DateFormat;" + + "import java.text.SimpleDateFormat;" + + "class C {" + + " private static final SimpleDateFormat df = new SimpleDateFormat(\"yyyy-MM-dd\");" + + " private String s = /*Access to non thread-safe static field 'df' of type 'java.text.SimpleDateFormat'*/df/**/.format(new Date());" + + "}"); + } + + public void testDeepCheck() { + doTest("import java.util.Date;" + + "import java.text.DateFormat;" + + "import java.text.SimpleDateFormat;" + + "class C {" + + " private static final DateFormat df = new SimpleDateFormat(\"yyyy-MM-dd\");" + + " private static final Date d = new Date();" + + " private static final String s1 = df.format(d);" + + " String m() {" + + " return /*Access to non thread-safe static field 'df' of type 'java.text.SimpleDateFormat'*/df/**/.format(d);" + + " }" + + "}"); + } + + @Override + protected LocalInspectionTool getInspection() { + return new AccessToNonThreadSafeStaticFieldFromInstanceInspection(); + } +} \ No newline at end of file