Report unannotated strings passed to @Nls

Part of IDEA-246014 Enhance hardcoded strings inspection GitOrigin-RevId: b6922daf5e67b5bd30cd938b3fbf5cd296f01347
2026-09-27 10:03:11 +07:00 · 2020-07-20 02:08:10 +00:00
parent d3b6351999
commit 7a655affe6
6 changed files with 169 additions and 22 deletions
@@ -45,6 +45,7 @@ inspection.error.dialog.title=Error
 inspection.i18n.display.name=Hard coded strings
 inspection.i18n.expression.is.invalid.error.message=The I18nized Expression template is not a valid expression
 inspection.i18n.message.general.with.value=Hardcoded string literal: {0}
+inspection.i18n.message.non.localized.passed.to.localized=Reference to non-localized string is used where localized string is expected
 inspection.i18n.option.ignore.assert=Ignore for assert statement arguments
 inspection.i18n.option.ignore.assigned.to.constants=Ignore literals assigned to constants
 inspection.i18n.option.ignore.comment.pattern=Ignore lines containing this comment (pattern in java.util.Pattern format):
@@ -54,6 +55,7 @@ inspection.i18n.option.ignore.for.exception.constructor.arguments=Ignore for exc
 inspection.i18n.option.ignore.for.junit.assert.arguments=Ignore for JUnit assert arguments
 inspection.i18n.option.ignore.for.specified.exception.constructor.arguments=Ignore for Specified Exception Constructor Arguments
 inspection.i18n.option.ignore.nls=Ignore if target is not annotated with @Nls
+inspection.i18n.option.report.unannotated.refs=Report unannotated references
 inspection.i18n.option.ignore.nonalphanumerics=Ignore literals which do not contain alphabetic characters
 inspection.i18n.option.ignore.property.keys=Ignore literals which have value equal to existing property key
 inspection.i18n.option.ignore.qualified.class.names=Ignore literals which have value equal to existing qualified class name
@@ -4,6 +4,7 @@ package com.intellij.codeInspection.i18n;

 import com.intellij.codeInsight.AnnotationUtil;
 import com.intellij.codeInsight.externalAnnotation.NonNlsAnnotationProvider;
+import com.intellij.codeInsight.intention.AddAnnotationFix;
 import com.intellij.codeInspection.*;
 import com.intellij.ide.util.TreeClassChooser;
 import com.intellij.ide.util.TreeClassChooserFactory;
@@ -35,8 +36,10 @@ import com.intellij.util.ObjectUtils;
 import com.intellij.util.ThreeState;
 import com.intellij.util.containers.ContainerUtil;
 import com.siyeh.HardcodedMethodConstants;
+import com.siyeh.ig.callMatcher.CallMatcher;
 import com.siyeh.ig.psiutils.ExpressionUtils;
 import com.siyeh.ig.psiutils.MethodUtils;
+import com.siyeh.ig.psiutils.TypeUtils;
 import gnu.trove.THashSet;
 import org.jdom.Element;
 import org.jetbrains.annotations.NonNls;
@@ -63,6 +66,9 @@ import java.util.regex.Pattern;
 import static com.intellij.codeInsight.AnnotationUtil.CHECK_EXTERNAL;

 public class I18nInspection extends AbstractBaseUastLocalInspectionTool implements CustomSuppressableInspectionTool {
+  private static final Set<String> IGNORED = ContainerUtil.immutableSet("<html>", "</html>", "<b>", "</b>");
+  private static final CallMatcher IGNORED_METHODS = CallMatcher.exactInstanceCall(CommonClassNames.JAVA_LANG_STRING, "substring", "trim");
+  
  public boolean ignoreForAssertStatements = true;
  public boolean ignoreForExceptionConstructors = true;
  @NonNls
@@ -72,6 +78,7 @@ public class I18nInspection extends AbstractBaseUastLocalInspectionTool implemen
  public boolean ignoreForPropertyKeyReferences = true;
  public boolean ignoreForNonAlpha = true;
  private boolean ignoreForAllButNls = false;
+  public boolean reportUnannotatedReferences = false;
  public boolean ignoreAssignedToConstants;
  public boolean ignoreToString;
  @NonNls public String nonNlsCommentPattern = "NON-NLS";
@@ -250,6 +257,13 @@ public class I18nInspection extends AbstractBaseUastLocalInspectionTool implemen
        ignoreForAllButNls = ignoreAllButNls.isSelected();
      }
    });
+    final JCheckBox reportRefs = new JCheckBox(JavaI18nBundle.message("inspection.i18n.option.report.unannotated.refs"), reportUnannotatedReferences);
+    reportRefs.addChangeListener(new ChangeListener() {
+      @Override
+      public void stateChanged(@NotNull ChangeEvent e) {
+        reportUnannotatedReferences = reportRefs.isSelected();
+      }
+    });

    final GridBagConstraints gc = new GridBagConstraints();
    gc.fill = GridBagConstraints.HORIZONTAL;
@@ -261,6 +275,9 @@ public class I18nInspection extends AbstractBaseUastLocalInspectionTool implemen
    gc.weighty = 0;
    panel.add(ignoreAllButNls, gc);

+    gc.gridy ++;
+    panel.add(reportRefs, gc);
+
    gc.gridy ++;
    panel.add(assertStatementsCheckbox, gc);

@@ -425,7 +442,7 @@ public class I18nInspection extends AbstractBaseUastLocalInspectionTool implemen
    List<ProblemDescriptor> result = new ArrayList<>();

    for (UMethod method : aClass.getMethods()) {
-      if (method.getSourcePsi() == aClass.getSourcePsi()) { // primary constructor that will not be proccsed other way
+      if (method.getSourcePsi() == aClass.getSourcePsi()) { // primary constructor that will not be processed other way
        checkMethodBody(method, manager, isOnTheFly, result);
      }
    }
@@ -532,13 +549,24 @@ public class I18nInspection extends AbstractBaseUastLocalInspectionTool implemen
        return;
      }

+      Class[] wantedClasses = ignoreForAllButNls && reportUnannotatedReferences ?
+                              new Class[]{UInjectionHost.class, UAnnotation.class, UCallExpression.class, UReferenceExpression.class} :
+                              new Class[]{UInjectionHost.class, UAnnotation.class};
      UElement uElement =
-        UastContextKt.toUElementOfExpectedTypes(element, UInjectionHost.class, UAnnotation.class);
+        UastContextKt.toUElementOfExpectedTypes(element, wantedClasses);

      if (uElement instanceof UInjectionHost) {
        visitLiteralExpression(element, (UInjectionHost)uElement);
        return;
      }
+      
+      if (uElement instanceof UCallExpression) {
+        visitCallExpression(element, (UCallExpression)uElement);
+      }
+      
+      if (uElement instanceof UReferenceExpression) {
+        visitReferenceExpression(element, (UReferenceExpression)uElement);
+      }

      if (uElement instanceof UAnnotation) {
        //prevent from @SuppressWarnings
@@ -550,8 +578,42 @@ public class I18nInspection extends AbstractBaseUastLocalInspectionTool implemen
      element.acceptChildren(this);
    }

-    private void visitLiteralExpression(@NotNull PsiElement sourcePsi,
-                                        @NotNull UInjectionHost expression) {
+    private void visitCallExpression(@NotNull PsiElement sourcePsi, @NotNull UCallExpression ref) {
+      PsiMethod target = ref.resolve();
+      if (target == null) return;
+      if (IGNORED_METHODS.methodMatches(target)) return;
+      UExpression expr = ref;
+      if (ref.getUastParent() instanceof UQualifiedReferenceExpression) {
+        expr = (UQualifiedReferenceExpression)ref.getUastParent();
+      }
+      processReferenceToNonLocalized(sourcePsi, expr, target);
+    }
+
+    private void visitReferenceExpression(@NotNull PsiElement sourcePsi, @NotNull UReferenceExpression ref) {
+      PsiVariable target = ObjectUtils.tryCast(ref.resolve(), PsiVariable.class);
+      if (target == null || target instanceof PsiLocalVariable) return;
+      processReferenceToNonLocalized(sourcePsi, ref, target);
+    }
+
+    private void processReferenceToNonLocalized(@NotNull PsiElement sourcePsi, @NotNull UExpression ref, PsiModifierListOwner target) {
+      PsiType type = ref.getExpressionType();
+      if (!TypeUtils.isJavaLangString(type)) return;
+      if (NlsInfo.forModifierListOwner(target) instanceof NlsInfo.Localized) return;
+      if (NlsInfo.forType(type) instanceof NlsInfo.Localized) return;
+      
+      String value = target instanceof PsiVariable ? ObjectUtils.tryCast(((PsiVariable)target).computeConstantValue(), String.class) : null;
+
+      NlsInfo targetInfo = getExpectedNlsInfo(myManager.getProject(), ref, value, new THashSet<>());
+      if (targetInfo instanceof NlsInfo.Localized) {
+        AddAnnotationFix fix = new AddAnnotationFix(((NlsInfo.Localized)targetInfo).suggestAnnotation(target), target, AnnotationUtil.NON_NLS);
+        String description = JavaI18nBundle.message("inspection.i18n.message.non.localized.passed.to.localized");
+        final ProblemDescriptor problem = myManager.createProblemDescriptor(
+          sourcePsi, description, myOnTheFly, new LocalQuickFix[] {fix}, ProblemHighlightType.GENERIC_ERROR_OR_WARNING);
+        myProblems.add(problem);
+      }
+    }
+
+    private void visitLiteralExpression(@NotNull PsiElement sourcePsi, @NotNull UInjectionHost expression) {
      String stringValue = getStringValueOfKnownPart(expression);
      if (StringUtil.isEmptyOrSpaces(stringValue)) {
        return;
@@ -667,10 +729,13 @@ public class I18nInspection extends AbstractBaseUastLocalInspectionTool implemen
  }

  private NlsInfo getExpectedNlsInfo(@NotNull Project project,
-                                     @NotNull UInjectionHost expression,
-                                     @NotNull String value,
+                                     @NotNull UExpression expression,
+                                     @Nullable String value,
                                     @NotNull Set<? super PsiModifierListOwner> nonNlsTargets) {
-    if (ignoreForNonAlpha && !StringUtil.containsAlphaCharacters(value)) {
+    if (ignoreForNonAlpha && value != null && !StringUtil.containsAlphaCharacters(value)) {
+      return NlsInfo.nonLocalized();
+    }
+    if (value != null && IGNORED.contains(value.toLowerCase(Locale.ROOT))) {
      return NlsInfo.nonLocalized();
    }

@@ -708,7 +773,7 @@ public class I18nInspection extends AbstractBaseUastLocalInspectionTool implemen
    return NlsInfo.nonLocalized();
  }

-  private boolean isSuppressedByComment(@NotNull Project project, @NotNull UInjectionHost expression) {
+  private boolean isSuppressedByComment(@NotNull Project project, @NotNull UExpression expression) {
    Pattern pattern = myCachedNonNlsPattern;
    if (pattern != null) {
      PsiElement sourcePsi = expression.getSourcePsi();
@@ -734,7 +799,7 @@ public class I18nInspection extends AbstractBaseUastLocalInspectionTool implemen
  }

  private boolean shouldIgnoreUsage(@NotNull Project project,
-                                    @NotNull String value,
+                                    @Nullable String value,
                                    @NotNull Set<? super PsiModifierListOwner> nonNlsTargets,
                                    @NotNull UExpression usage) {
    if (isInNonNlsCall(usage, nonNlsTargets)) {
@@ -764,10 +829,10 @@ public class I18nInspection extends AbstractBaseUastLocalInspectionTool implemen
    if (ignoreForJUnitAsserts && isArgOfJUnitAssertion(usage)) {
      return true;
    }
-    if (ignoreForClassReferences && isClassRef(usage, value)) {
+    if (ignoreForClassReferences && value != null && isClassRef(usage, value)) {
      return true;
    }
-    if (ignoreForPropertyKeyReferences && !PropertiesImplUtil.findPropertiesByKey(project, value).isEmpty()) {
+    if (ignoreForPropertyKeyReferences && value != null && !PropertiesImplUtil.findPropertiesByKey(project, value).isEmpty()) {
      return true;
    }
    if (ignoreToString && isToString(usage)) {
@@ -21,10 +21,7 @@ import org.jetbrains.annotations.Nullable;
 import org.jetbrains.uast.*;
 import org.jetbrains.uast.util.UastExpressionUtils;

-import java.util.Collection;
-import java.util.List;
-import java.util.OptionalInt;
-import java.util.Set;
+import java.util.*;
 import java.util.stream.IntStream;

 /**
@@ -40,20 +37,23 @@ public abstract class NlsInfo {
   * Describes a string that should be localized
   */
  public static final class Localized extends NlsInfo {
-    private static final Localized NLS = new Localized(Capitalization.NotSpecified, "", "");
-    private static final Localized NLS_TITLE = new Localized(Capitalization.Title, "", "");
-    private static final Localized NLS_SENTENCE = new Localized(Capitalization.Sentence, "", "");
+    private static final Localized NLS = new Localized(Capitalization.NotSpecified, "", "", null);
+    private static final Localized NLS_TITLE = new Localized(Capitalization.Title, "", "", null);
+    private static final Localized NLS_SENTENCE = new Localized(Capitalization.Sentence, "", "", null);
    private final @NotNull Capitalization myCapitalization;
    private final @NotNull @NonNls String myPrefix;
    private final @NotNull @NonNls String mySuffix;
+    private final String myAnnotationName;

    private Localized(@NotNull Capitalization capitalization,
                      @NotNull @NonNls String prefix,
-                      @NotNull @NonNls String suffix) {
+                      @NotNull @NonNls String suffix,
+                      @Nullable @NonNls String annotationName) {
      super(ThreeState.YES);
      myCapitalization = capitalization;
      myPrefix = prefix;
      mySuffix = suffix;
+      myAnnotationName = annotationName;
    }

    /**
@@ -63,6 +63,14 @@ public abstract class NlsInfo {
    public @NotNull Capitalization getCapitalization() {
      return myCapitalization;
    }
+    
+    public @NotNull String suggestAnnotation(PsiElement context) {
+      if (myAnnotationName != null &&
+          JavaPsiFacade.getInstance(context.getProject()).findClass(myAnnotationName, context.getResolveScope()) != null) {
+        return myAnnotationName;
+      }
+      return AnnotationUtil.NLS;
+    }

    /**
     * @return desired prefix for new property keys
@@ -80,11 +88,19 @@ public abstract class NlsInfo {
      return mySuffix;
    }

-    private @NotNull NlsInfo withPrefixAndSuffix(@NotNull String prefix, @NotNull String suffix) {
+    private @NotNull Localized withPrefixAndSuffix(@NotNull String prefix, @NotNull String suffix) {
      if (prefix.equals(myPrefix) && suffix.equals(mySuffix)) {
        return this;
      }
-      return new Localized(myCapitalization, prefix, suffix);
+      return new Localized(myCapitalization, prefix, suffix, myAnnotationName);
+    }
+
+    private @NotNull Localized withAnnotation(@NotNull PsiAnnotation annotation) {
+      String qualifiedName = annotation.getQualifiedName();
+      if (Objects.equals(qualifiedName, myAnnotationName)) {
+        return this;
+      }
+      return new Localized(myCapitalization, myPrefix, mySuffix, qualifiedName);
    }
  }

@@ -161,6 +177,10 @@ public abstract class NlsInfo {
    return fromArgument(expression);
  }

+  public static @NotNull NlsInfo forType(@NotNull PsiType type) {
+    return fromAnnotationOwner(type);
+  }
+
  public static @NotNull NlsInfo forModifierListOwner(@NotNull PsiModifierListOwner owner) {
    if (owner instanceof PsiParameter) {
      PsiElement scope = ((PsiParameter)owner).getDeclarationScope();
@@ -399,7 +419,7 @@ public abstract class NlsInfo {
      }
    }
    if (baseInfo instanceof Localized) {
-      return ((Localized)baseInfo).withPrefixAndSuffix(prefix, suffix);
+      return ((Localized)baseInfo).withPrefixAndSuffix(prefix, suffix).withAnnotation(annotation);
    }
    return baseInfo;
  }
@@ -0,0 +1,9 @@
+class X {
+    static final String CONSTANT = <warning descr="Hardcoded string literal: \"Value\"">"Value"</warning>;
+    
+    void test() {
+        use(CONSTANT);
+    }
+    
+    void use(String c) {}
+}
@@ -0,0 +1,36 @@
+import org.jetbrains.annotations.Nls;
+
+class X {
+    static final String CONSTANT = "Value";
+    static final String EMPTY = " ";
+    
+    void test() {
+        use(<warning descr="Reference to non-localized string is used where localized string is expected">CONSTANT</warning>);
+        use(EMPTY);
+    }
+    
+    void testParameter(String s) {
+        use(<warning descr="Reference to non-localized string is used where localized string is expected">s</warning>);
+    }
+
+    void testCall() {
+        use(<warning descr="Reference to non-localized string is used where localized string is expected">getNonAnnotated()</warning>);
+        use(<warning descr="Reference to non-localized string is used where localized string is expected">this.getNonAnnotated()</warning>);
+    }
+    
+    void testNested(String s) {
+        use(nlsResult(s.trim()));
+    }
+    
+    @Nls String getNested(String s) {
+        return X.nlsResultStatic(s.trim());
+    }
+    
+    native String getNonAnnotated();
+    
+    void use(@Nls String c) {}
+    
+    native static @Nls String nlsResultStatic(String nonNlsParam);
+    
+    native @Nls String nlsResult(String nonNlsParam);
+}
@@ -14,6 +14,7 @@ public class I18NInspectionTest extends LightJavaCodeInsightFixtureTestCase {
  I18nInspection myTool = new I18nInspection();
  
  private void doTest() {
+    myTool.reportUnannotatedReferences = true;
    myFixture.enableInspections(myTool);
    myFixture.testHighlighting("i18n/" + getTestName(false) + ".java");
  }
@@ -145,6 +146,20 @@ public class I18NInspectionTest extends LightJavaCodeInsightFixtureTestCase {
      myTool.setIgnoreForAllButNls(old);
    }
  }
+  
+  public void testUseConstant() {
+    doTest();
+  }
+  
+  public void testUseConstantNls() {
+    boolean old = myTool.setIgnoreForAllButNls(true);
+    try {
+      doTest();
+    }
+    finally {
+      myTool.setIgnoreForAllButNls(old);
+    }
+  }

  @Override
  protected String getTestDataPath() {