Partially implemented StringToUpperWithoutLocale2Inspection (w/o registration)

jvm/jvm-analysis-api/src/com/intellij/codeInspection/NonNlsUastUtil.java

@@ -4,6 +4,7 @@ package com.intellij.codeInspection;
 import com.intellij.codeInsight.AnnotationUtil;
 import com.intellij.psi.PsiElement;
 import com.intellij.psi.PsiMethod;
+import com.intellij.psi.PsiModifierListOwner;
 import com.intellij.psi.PsiParameter;
 import com.siyeh.HardcodedMethodConstants;
 import com.siyeh.ig.psiutils.MethodUtils;
@@ -26,6 +27,15 @@ public final class NonNlsUastUtil {
     return element instanceof UAnnotated && ((UAnnotated)element).findAnnotation(AnnotationUtil.NON_NLS) != null;
   }
 
+  /**
+   * @return <code>true</code> if expression qualifier (part before <code>::</code>) should be considered non-localizable;
+   * <code>false</code> otherwise.
+   */
+  public static boolean isCallableReferenceExpressionWithNonNlsQualifier(@Nullable UCallableReferenceExpression expression) {
+    if (expression == null) return false;
+    return false; //TODO implement
+  }
+
   /**
    * @return <code>true</code> if expression receiver should be considered non-localizable; <code>false</code> otherwise.
    */
@@ -40,6 +50,10 @@ public final class NonNlsUastUtil {
     if (receiver instanceof USimpleNameReferenceExpression) { // reference to variable/field/parameter
       return isReferenceToNonNlsElement((USimpleNameReferenceExpression)receiver);
     }
+    if (receiver instanceof UCallExpression || receiver instanceof UQualifiedReferenceExpression) { // call chain
+      PsiElement resolved = ((UResolvable)receiver).resolve();
+      return resolved != null && isNonNlsAnnotatedPsi(resolved);
+    }
 
     UElement expressionParent = expression.getUastParent();
     if (expressionParent instanceof UVariable && isNonNlsAnnotated(expressionParent)) { // initialized field/variable
@@ -57,27 +71,43 @@ public final class NonNlsUastUtil {
     if (!UastLiteralUtils.isStringLiteral(expression)) return false;
     if (isPlacedInNonNlsClass(expression) || isPlacedInNonNlsPackage(expression)) return true;
 
-    UElement parent = expression.getUastParent();
-    if (parent instanceof UPolyadicExpression && !(parent instanceof UBinaryExpression)) { // multiline string literals
-      parent = parent.getUastParent();
-    }
+    UElement parent = UastUtils.getParentOfType(expression,
+                                                true,
+                                                UExpressionList.class,
+                                                UVariable.class,
+                                                UReturnExpression.class,
+                                                UCallExpression.class,
+                                                UBinaryExpression.class);
 
     if (parent instanceof UField || parent instanceof ULocalVariable || parent instanceof UParameter) {
       return isNonNlsAnnotated(parent);
     }
-    else if (parent instanceof UBinaryExpression) { // e.g. assigning value to parameter
-      UExpression leftOperand = ((UBinaryExpression)parent).getLeftOperand();
-      if (leftOperand instanceof USimpleNameReferenceExpression) {
-        return isReferenceToNonNlsElement((USimpleNameReferenceExpression)leftOperand);
-      }
-    }
-    else if (parent instanceof UReturnExpression) {
+    if (parent instanceof UReturnExpression) {
       return isReturnExpressionInNonNlsMethod((UReturnExpression)parent);
     }
-    else if (parent instanceof UCallExpression) {
+    if (parent instanceof UCallExpression) {
       return isNonNlsArgument(expression, (UCallExpression)parent) ||
              isCallExpressionWithNonNlsReceiver((UCallExpression)parent);
     }
+
+    while (parent instanceof UBinaryExpression) {
+      if (!isAssignmentExpression((UBinaryExpression)parent)) {
+        // go upper to find assignment expression (or field/variable declaration)
+        parent = UastUtils.getParentOfType(parent, true, UBinaryExpression.class, UVariable.class);
+        if (parent instanceof UVariable) {
+          return isNonNlsAnnotated(parent);
+        }
+        continue;
+      }
+
+      UExpression leftOperand = ((UBinaryExpression)parent).getLeftOperand();
+      //TODO probably this doesn't cover all cases
+      return leftOperand instanceof USimpleNameReferenceExpression &&
+             isReferenceToNonNlsElement((USimpleNameReferenceExpression)leftOperand);
+    }
+
+    //TODO UExpressionList?
+
     return false;
   }
 
@@ -116,7 +146,7 @@ public final class NonNlsUastUtil {
   private static boolean isNonNlsArgument(@NotNull ULiteralExpression argument, @NotNull UCallExpression callExpression) {
     PsiParameter parameter = UastUtils.getParameterForArgument(callExpression, argument);
     if (parameter == null) return false;
-    if (AnnotationUtil.findAnnotation(parameter, AnnotationUtil.NON_NLS) != null) return true;
+    if (isNonNlsAnnotatedPsi(parameter)) return true;
 
     // special handling for equals()
     if (!HardcodedMethodConstants.EQUALS.equals(callExpression.getMethodName())) return false;
@@ -124,4 +154,17 @@ public final class NonNlsUastUtil {
     if (!MethodUtils.isEquals(method)) return false;
     return isCallExpressionWithNonNlsReceiver(callExpression);
   }
+
+  //TODO looks dirty, is there a better way to check this?  <--  see UElementAsPsiInspection
+  private static boolean isAssignmentExpression(@NotNull UBinaryExpression expression) {
+    UIdentifier operatorIdentifier = expression.getOperatorIdentifier();
+    if (operatorIdentifier == null) return false;
+    String operatorName = operatorIdentifier.getName();
+    return operatorName.equals("=") || operatorName.equals("+=");
+  }
+
+  private static boolean isNonNlsAnnotatedPsi(@NotNull PsiElement element) {
+    return element instanceof PsiModifierListOwner &&
+           AnnotationUtil.findAnnotation((PsiModifierListOwner)element, AnnotationUtil.NON_NLS) != null;
+  }
 }

jvm/jvm-analysis-impl/resources/com/intellij/jvm/analysis/JvmAnalysisBundle.properties

@@ -2,4 +2,7 @@ jvm.inspections.group.name=JVM languages
 
 jvm.inspections.unstable.api.usage.display.name=Unstable API Usage
 jvm.inspections.unstable.api.usage.annotations.list=Unstable API annotations
-jvm.inspections.unstable.api.usage.description=''{0}'' is marked unstable
+jvm.inspections.unstable.api.usage.description=''{0}'' is marked unstable
+
+jvm.inspections.string.touppercase.tolowercase.without.locale.display.name=Call to 'String.toUpperCase()' or 'toLowerCase()' without a Locale
+jvm.inspections.string.touppercase.tolowercase.without.locale.description=<code>String.{0}()</code> called without specifying a Locale using internationalized strings #loc

jvm/jvm-analysis-impl/resources/inspectionDescriptions/StringToUpperWithoutLocale2.html

@@ -0,0 +1,11 @@
+<html>
+<body>
+Reports any call of <b>toUpperCase()</b> or
+<b>toLowerCase()</b> on <b>String</b> objects which
+do not specify a <b>java.util.Locale</b>.
+Such calls are usually incorrect in an internationalized environment.
+<!-- tooltip end -->
+<p>
+
+</body>
+</html>

jvm/jvm-analysis-impl/src/com/intellij/codeInspection/StringToUpperWithoutLocale2Inspection.java

@@ -0,0 +1,84 @@
+// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
+package com.intellij.codeInspection;
+
+import com.intellij.analysis.JvmAnalysisBundle;
+import com.intellij.psi.CommonClassNames;
+import com.intellij.psi.PsiElement;
+import com.intellij.psi.PsiElementVisitor;
+import com.intellij.psi.PsiIdentifier;
+import com.siyeh.HardcodedMethodConstants;
+import org.jetbrains.annotations.Nls;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.uast.UCallExpression;
+import org.jetbrains.uast.UCallableReferenceExpression;
+import org.jetbrains.uast.UElement;
+import org.jetbrains.uast.UastContextKt;
+
+
+public class StringToUpperWithoutLocale2Inspection extends AbstractBaseUastLocalInspectionTool {
+  @Nls
+  @NotNull
+  @Override
+  public String getDisplayName() { //TODO remove once inspection is registered in JvmAnalysisPlugin.xml
+    return "StringToUpperWithoutLocale2Inspection";
+  }
+
+  private static final UastCallMatcher MATCHER = UastCallMatcher.anyOf(
+    UastCallMatcher.builder()
+                   .withMethodName(HardcodedMethodConstants.TO_UPPER_CASE)
+                   .withClassFqn(CommonClassNames.JAVA_LANG_STRING)
+                   .withArgumentsCount(0).build(),
+    UastCallMatcher.builder()
+                   .withMethodName(HardcodedMethodConstants.TO_LOWER_CASE)
+                   .withClassFqn(CommonClassNames.JAVA_LANG_STRING)
+                   .withArgumentsCount(0).build()
+  );
+
+  @NotNull
+  @Override
+  public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) {
+    return new PsiElementVisitor() {
+      @Override
+      public void visitElement(PsiElement element) {
+        UCallExpression callExpression = AnalysisUastUtil.getUCallExpression(element);
+        if (callExpression != null) {
+          handleCallExpression(callExpression, holder);
+          return;
+        }
+
+        if (!(element instanceof PsiIdentifier)) return;
+        PsiElement parent = element.getParent();
+        UElement parentUElement = UastContextKt.toUElement(parent);
+        if (parentUElement instanceof UCallableReferenceExpression) {
+          handleCallableReferenceExpression((UCallableReferenceExpression)parentUElement, element, holder);
+        }
+      }
+    };
+  }
+
+  private static void handleCallExpression(@NotNull UCallExpression callExpression, @NotNull ProblemsHolder holder) {
+    if (!MATCHER.testCallExpression(callExpression)) return;
+    if (NonNlsUastUtil.isCallExpressionWithNonNlsReceiver(callExpression)) return;
+
+    PsiElement methodIdentifierPsi = AnalysisUastUtil.getMethodIdentifierSourcePsi(callExpression);
+    if (methodIdentifierPsi == null) return;
+
+    String methodName = callExpression.getMethodName();
+    if (methodName == null) return; // shouldn't happen
+    holder.registerProblem(methodIdentifierPsi, getErrorDescription(methodName));
+  }
+
+  private static void handleCallableReferenceExpression(@NotNull UCallableReferenceExpression expression,
+                                                        @NotNull PsiElement identifier,
+                                                        @NotNull ProblemsHolder holder) {
+    if (!MATCHER.testCallableReferenceExpression(expression)) return;
+    if (NonNlsUastUtil.isCallableReferenceExpressionWithNonNlsQualifier(expression)) return;
+
+    holder.registerProblem(identifier, getErrorDescription(expression.getCallableName()));
+  }
+
+  @NotNull
+  private static String getErrorDescription(@NotNull String methodName) {
+    return JvmAnalysisBundle.message("jvm.inspections.string.touppercase.tolowercase.without.locale.description", methodName);
+  }
+}

jvm/jvm-analysis-java-tests/testData/codeInspection/toUpperWithoutLocale/NonNlsCases.java

@@ -0,0 +1,79 @@
+// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
+package com.intellij.codeInspection.aaaNonNlsTest;
+
+import org.jetbrains.annotations.NonNls;
+
+@SuppressWarnings({"ResultOfMethodCallIgnored", "MethodMayBeStatic"})
+public class NonNlsCases {
+  String plainField = "3";
+  @NonNls String nonNlsField = "4";
+
+  String initializedPlainField = "1".<warning descr="'String.toUpperCase()' called without specifying a Locale using internationalized strings">toUpperCase</warning>() + "2".<warning descr="'String.toLowerCase()' called without specifying a Locale using internationalized strings">toLowerCase</warning>();
+  @NonNls String initializedNonNlsField = "1".toUpperCase() + "2".toLowerCase();
+  @NonNls String initializedNonNlsField2 = initializedNonNlsField.toUpperCase();
+
+
+  public void checkFields() {
+    plainField.<warning descr="'String.toUpperCase()' called without specifying a Locale using internationalized strings">toUpperCase</warning>();
+    String s = plainField.<warning descr="'String.toUpperCase()' called without specifying a Locale using internationalized strings">toUpperCase</warning>();
+    nonNlsField.toUpperCase();
+    String ss = nonNlsField.toUpperCase();
+
+    plainField.concat("1".<warning descr="'String.toUpperCase()' called without specifying a Locale using internationalized strings">toUpperCase</warning>());
+    nonNlsField.concat("1".toUpperCase());
+  }
+
+
+  public void checkVariables() {
+    String plain = "123";
+    @NonNls String nonNls = "123";
+
+    plain.<warning descr="'String.toUpperCase()' called without specifying a Locale using internationalized strings">toUpperCase</warning>();
+    nonNls.toUpperCase();
+
+    plain.concat("1".<warning descr="'String.toUpperCase()' called without specifying a Locale using internationalized strings">toUpperCase</warning>());
+    nonNls.concat("1".toUpperCase());
+
+    @NonNls String s = "123".toUpperCase();
+    s.toLowerCase();
+    String ss = "123".<warning descr="'String.toUpperCase()' called without specifying a Locale using internationalized strings">toUpperCase</warning>();
+    ss.<warning descr="'String.toLowerCase()' called without specifying a Locale using internationalized strings">toLowerCase</warning>();
+  }
+
+
+  public void checkParameters(@NonNls String nonNlsParam, String plainParam) {
+    nonNlsParam.toUpperCase();
+    plainParam.<warning descr="'String.toUpperCase()' called without specifying a Locale using internationalized strings">toUpperCase</warning>();
+
+    plainParam.concat("1".<warning descr="'String.toUpperCase()' called without specifying a Locale using internationalized strings">toUpperCase</warning>());
+    nonNlsParam.concat("1".toUpperCase());
+  }
+
+
+  public void checkMethods() {
+    nonNlsMethod().toUpperCase();
+    plainMethod().<warning descr="'String.toUpperCase()' called without specifying a Locale using internationalized strings">toUpperCase</warning>();
+
+    Clazz clazzInstance = new Clazz();
+    clazzInstance.nestedNonNlsMethod().toUpperCase();
+    clazzInstance.nestedPlainMethod().<warning descr="'String.toUpperCase()' called without specifying a Locale using internationalized strings">toUpperCase</warning>();
+  }
+
+
+
+  @NonNls
+  String nonNlsMethod() { return ""; }
+  String plainMethod() { return ""; }
+
+  static class Clazz {
+    @NonNls
+    String nestedNonNlsMethod() { return ""; }
+    String nestedPlainMethod() { return ""; }
+  }
+
+
+  @NonNls
+  static class NonNlsClass {
+    String s = "123".toUpperCase();
+  }
+}

jvm/jvm-analysis-java-tests/testData/codeInspection/toUpperWithoutLocale/SimpleCases.java

@@ -0,0 +1,19 @@
+// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
+package com.intellij.codeInspection;
+
+import java.util.List;
+import java.util.Locale;
+
+public class SimpleCases {
+  String s1 = "123".<warning descr="'String.toUpperCase()' called without specifying a Locale using internationalized strings">toUpperCase</warning>();
+  String s2 = "123".toUpperCase(Locale.ENGLISH);
+
+  public void foo() {
+    String foo = "foo".<warning descr="'String.toUpperCase()' called without specifying a Locale using internationalized strings">toUpperCase</warning>();
+    String bar = "bar".toLowerCase(Locale.US);
+  }
+
+  public void methodRef(List<String> list) {
+    list.stream().map(String::<warning descr="'String.toUpperCase()' called without specifying a Locale using internationalized strings">toUpperCase</warning>);
+  }
+}

jvm/jvm-analysis-java-tests/testSrc/com/intellij/codeInspection/StringToUpperWithoutLocaleInspectionTest.java

@@ -0,0 +1,26 @@
+package com.intellij.codeInspection;
+
+import com.intellij.jvm.analysis.JvmAnalysisTestsUtil;
+import com.intellij.testFramework.TestDataPath;
+
+@TestDataPath("$CONTENT_ROOT/testData/codeInspection/toUpperWithoutLocale")
+public class StringToUpperWithoutLocaleInspectionTest extends StringToUpperWithoutLocaleInspectionTestBase {
+  @Override
+  protected String getBasePath() {
+    return JvmAnalysisTestsUtil.TEST_DATA_PROJECT_RELATIVE_BASE_PATH + "/codeInspection/toUpperWithoutLocale";
+  }
+
+  public void testSimpleCases() {
+    myFixture.testHighlighting("SimpleCases.java");
+    //TODO test after quickfix once it's implemented
+  }
+
+  public void testNonNlsCases() {
+    //TODO test #4 (about equals())
+    //TODO test #5 (about @NonNls in the left side of assignment expression)
+    //TODO test #6 with assigning method return value to variable? If this is not super hard to support.
+    myFixture.testHighlighting("NonNlsCases.java");
+  }
+
+  //TODO test pkg/subPkg
+}

jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/KtNonNlsUastUtilTest.java

@@ -6,8 +6,10 @@ import com.intellij.testFramework.TestDataPath;
 import com.intellij.testFramework.builders.JavaModuleFixtureBuilder;
 import com.intellij.testFramework.fixtures.JavaCodeInsightFixtureTestCase;
 import com.intellij.util.PathUtil;
+import kotlin.KotlinVersion;
 import org.jetbrains.annotations.NonNls;
 import org.jetbrains.uast.ULiteralExpression;
+import org.junit.Assume;
 
 import java.util.Set;
 
@@ -16,6 +18,12 @@ import static com.intellij.codeInspection.NonNlsUastUtil.isNonNlsStringLiteral;
 
 @TestDataPath("$CONTENT_ROOT/testData/codeInspection/nonNls")
 public class KtNonNlsUastUtilTest extends JavaCodeInsightFixtureTestCase {
+  @Override
+  protected void setUp() throws Exception {
+    Assume.assumeTrue(KotlinVersion.CURRENT.isAtLeast(1, 2, 60));
+    super.setUp();
+  }
+
   @Override
   protected String getBasePath() {
     return JvmAnalysisKtTestsUtil.TEST_DATA_PROJECT_RELATIVE_BASE_PATH + "/codeInspection/nonNls";

jvm/jvm-analysis-tests-api/src/com/intellij/codeInspection/StringToUpperWithoutLocaleInspectionTestBase.java

@@ -0,0 +1,26 @@
+package com.intellij.codeInspection;
+
+import com.intellij.pom.java.LanguageLevel;
+import com.intellij.testFramework.IdeaTestUtil;
+import com.intellij.testFramework.builders.JavaModuleFixtureBuilder;
+import com.intellij.testFramework.fixtures.JavaCodeInsightFixtureTestCase;
+import com.intellij.util.PathUtil;
+import org.jetbrains.annotations.NonNls;
+
+import java.util.stream.Stream;
+
+public abstract class StringToUpperWithoutLocaleInspectionTestBase extends JavaCodeInsightFixtureTestCase {
+  @Override
+  protected void setUp() throws Exception {
+    super.setUp();
+    myFixture.enableInspections(new StringToUpperWithoutLocale2Inspection());
+  }
+
+  @Override
+  protected void tuneFixture(JavaModuleFixtureBuilder moduleBuilder) {
+    moduleBuilder.setLanguageLevel(LanguageLevel.JDK_1_8);
+    moduleBuilder.addJdk(IdeaTestUtil.getMockJdk18Path().getPath());
+    moduleBuilder.addLibrary("annotations", PathUtil.getJarPathForClass(NonNls.class));
+    moduleBuilder.addLibrary("javaUtil", PathUtil.getJarPathForClass(Stream.class));
+  }
+}