[java-inspections] IDEA-318681, IDEA-318678, IDEA-318677, IDEA-318676 Improve tainted analysis

GitOrigin-RevId: 93ef8d87e4dadd2b5cbfcc16b91610503df95664

jvm/jvm-analysis-java-tests/testData/codeInspection/sourceToSinkFlow/JsrSimple.java

@@ -15,9 +15,14 @@ class Simple {
   
     void unknown() {
       String s = foo();
+      sink(s);
+    }
+
+    void unknown2(String f) {
+      String s = foo2(f);
       sink(<warning descr="Unknown string is used as safe parameter">s</warning>);
     }
-  
+
     void literalOnly() {
       String s = null;
       s = "safe";
@@ -53,7 +58,7 @@ class Simple {
       String s1 = (source());
       s1 = (foo());
       String s = (s1);
-      sink((<warning descr="Unsafe string is used as safe parameter">s</warning>));
+      sink(<warning descr="Unsafe string is used as safe parameter">(s)</warning>);
     }
   
     @Untainted String unsafeReturn() {
@@ -69,19 +74,19 @@ class Simple {
       @Tainted String s = source();
       String s1 = "safe";
       String s2 = "safe2";
-      sink(s1 + <warning descr="Unsafe string is used as safe parameter">s</warning> + s2);
+      sink(<warning descr="Unsafe string is used as safe parameter">s1 + s + s2</warning>);
     }
 
     void unsafeTernary(boolean b) {
       @Tainted String s = source();
-      sink(b ? <warning descr="Unsafe string is used as safe parameter">s</warning> : null);
+      sink(<warning descr="Unsafe string is used as safe parameter">b ? s : null</warning>);
     }
   
     void fieldFromGetter() {
       String s = getField();
       sink(<warning descr="Unknown string is used as safe parameter">s</warning>);
     }
-    
+
     void assignToSafeLocalVar() {
       String s1 = getField();
       @Untainted String safe = <warning descr="Unknown string is assigned to safe variable">s1</warning>;
@@ -99,10 +104,14 @@ class Simple {
       return source();
     }
 
-    String foo() {
+    private String foo() {
       return "some";
     }
 
+    String foo2(String a) {
+      return a;
+    }
+
     @Untainted
     String safe() {
       return "safe";

jvm/jvm-analysis-java-tests/testData/codeInspection/sourceToSinkFlow/Simple.java

@@ -53,7 +53,7 @@ public class Simple {
       String s1 = (source());
       s1 = (foo());
       String s = (s1);
-      sink((<warning descr="Unsafe string is used as safe parameter">s</warning>));
+      sink(<warning descr="Unsafe string is used as safe parameter">(s)</warning>);
     }
   
     @Untainted String unsafeReturn() {
@@ -69,12 +69,12 @@ public class Simple {
       @Tainted String s = source();
       String s1 = "safe";
       String s2 = "safe2";
-      sink(s1 + <warning descr="Unsafe string is used as safe parameter">s</warning> + s2);
+      sink(<warning descr="Unsafe string is used as safe parameter">s1 + s + s2</warning>);
     }
 
     void unsafeTernary(boolean b) {
       @Tainted String s = source();
-      sink(b ? <warning descr="Unsafe string is used as safe parameter">s</warning> : null);
+      sink(<warning descr="Unsafe string is used as safe parameter">b ? s : null</warning>);
     }
   
     void fieldFromGetter() {

jvm/jvm-analysis-java-tests/testData/codeInspection/sourceToSinkFlow/markAsSafeFix/CommonCasesCheckFramework.after.java

@@ -1,7 +1,7 @@
 import org.checkerframework.checker.tainting.qual.Untainted;
 
 class CommonCases {
-  private @Untainted String sField;
+  public @Untainted String sField;
 
   @Untainted
   private String test(@Untainted String s) {

jvm/jvm-analysis-java-tests/testData/codeInspection/sourceToSinkFlow/markAsSafeFix/CommonCasesCheckFramework.java

@@ -1,7 +1,7 @@
 import org.checkerframework.checker.tainting.qual.Untainted;
 
 class CommonCases {
-  private String sField;
+  public String sField;
 
   @Untainted
   private String test(String s) {

jvm/jvm-analysis-java-tests/testData/codeInspection/sourceToSinkFlow/markAsSafeFix/CommonCasesJsr.after.java

@@ -1,8 +1,7 @@
 import javax.annotation.Untainted;
 
 class CommonCases {
-    @Untainted
-    private String sField;
+  private String sField;
 
   @Untainted
   private String test(@Untainted String s) {

jvm/jvm-analysis-java-tests/testData/codeInspection/sourceToSinkFlow/markAsSafeFix/Field.after.java

@@ -9,7 +9,7 @@ class Simple {
       sink(s2);
     }
 
-    String foo() {
+    @Untainted String foo() {
       return "foo";
     }
 

jvm/jvm-analysis-java-tests/testData/codeInspection/sourceToSinkFlow/markAsSafeFix/Recursive.java

@@ -6,7 +6,7 @@ class Simple {
         String s = "";
         String s1 = id(id(s));
         s += s1;
-        sink(id(<caret>s));
+        sink(id<caret>(s));
     }
 
     String id(String s) {

jvm/jvm-analysis-java-tests/testData/codeInspection/sourceToSinkFlow/markAsSafeFix/SafeVar.java

@@ -1,4 +1,3 @@
-// "Mark 's' as requiring validation" "false"
 package org.checkerframework.checker.tainting.qual;
 
 class Simple {

jvm/jvm-analysis-java-tests/testData/codeInspection/sourceToSinkFlow/propagateSafe/AnotherClassMethodCall.after.java

@@ -12,15 +12,17 @@ class Simple {
         return ((another.foo()) + (another.foo()));
     }
 
-    static @Untainted String bar() {
-        return "safe";
+    static String bar(x) {
+        return x;
     }
 
     void sink(@Untainted String s) {}
 }
 
 class Another {
+  String x;
+
     @Untainted String foo() {
-        return Simple.bar();
+        return Simple.bar(x);
     }
 }

jvm/jvm-analysis-java-tests/testData/codeInspection/sourceToSinkFlow/propagateSafe/AnotherClassMethodCall.java

@@ -12,15 +12,16 @@ class Simple {
         return ((another.foo()) + (another.foo()));
     }
 
-    static String bar() {
-        return "safe";
+    static String bar(x) {
+        return x;
     }
 
     void sink(@Untainted String s) {}
 }
 
 class Another {
+  String x;
     String foo() {
-        return Simple.bar();
+        return Simple.bar(x);
     }
 }

jvm/jvm-analysis-java-tests/testData/codeInspection/sourceToSinkFlow/propagateSafe/MethodFieldMethodTainted.after.java

@@ -2,14 +2,14 @@ import org.checkerframework.checker.tainting.qual.*;
 
 class Simple {
 
-  String field = "safe";
+    @Untainted String field = "safe";
 
   void test() {
     String s = foo();
     sink(<caret>s);
   }
 
-  String foo() {
+    @Untainted String foo() {
     return field;
   }
 

jvm/jvm-analysis-java-tests/testData/codeInspection/sourceToSinkFlow/propagateSafe/MethodFieldMethodUnknown.after.java

@@ -17,7 +17,7 @@ class Simple {
     this.field = bar();
   }
 
-    @Untainted String bar() {
+  String bar() {
     return "safe";
   }
 

jvm/jvm-analysis-java-tests/testData/codeInspection/sourceToSinkFlow/propagateSafe/MethodFieldTainted.after.java

@@ -9,7 +9,7 @@ class Simple {
     sink(<caret>s);
   }
 
-  String foo() {
+    @Untainted String foo() {
     return field;
   }
 

jvm/jvm-analysis-java-tests/testData/codeInspection/sourceToSinkFlow/propagateSafe/MethodMethodTainted.after.java

@@ -7,7 +7,7 @@ class Simple {
     sink(<caret>s);
   }
 
-  String foo() {
+    @Untainted String foo() {
     return source();
   }
 

jvm/jvm-analysis-java-tests/testData/codeInspection/sourceToSinkFlow/propagateSafe/ParameterFieldTainted.after.java

@@ -9,7 +9,7 @@ class Simple {
     test(s);
   }
 
-  void test(String s) {
+  void test(@Untainted String s) {
     sink(s);
   }
 

jvm/jvm-analysis-java-tests/testData/codeInspection/sourceToSinkFlow/propagateSafe/ParameterFieldUnknown.after.java

@@ -2,7 +2,7 @@ import org.checkerframework.checker.tainting.qual.*;
 
 class Simple {
 
-    @Untainted String field = "safe";
+  String field = "safe";
 
   void callTest() {
     String s = field;

jvm/jvm-analysis-java-tests/testData/codeInspection/sourceToSinkFlow/propagateSafe/ParameterMethodTainted.after.java

@@ -11,7 +11,7 @@ class Simple {
     return "unsafe";
   }
 
-  void test(String s) {
+  void test(@Untainted String s) {
     sink(<caret>s);
   }
 

jvm/jvm-analysis-java-tests/testData/codeInspection/sourceToSinkFlow/propagateSafe/ParameterMethodUnknown.after.java

@@ -7,7 +7,7 @@ class Simple {
     test(s);
   }
 
-    @Untainted String foo() {
+   String foo() {
     return "safe";
   }
 

jvm/jvm-analysis-java-tests/testData/codeInspection/sourceToSinkFlow/propagateSafe/ParameterMethodUnknown.java

@@ -6,8 +6,8 @@ class Simple {
     String s = foo();
     test(s);
   }
-  
-  String foo() {
+
+   String foo() {
     return "safe";
   }
 

jvm/jvm-analysis-java-tests/testSrc/com/intellij/codeInspection/tests/java/sourceToSink/JavaMarkAsSafeFixSourceToSinkFlowInspectionTest.kt

@@ -1,13 +1,14 @@
 package com.intellij.codeInspection.tests.java.sourceToSink
 
+import com.intellij.analysis.JvmAnalysisBundle
 import com.intellij.codeInspection.tests.sourceToSink.SourceToSinkFlowInspectionTestBase
 import com.intellij.jvm.analysis.JavaJvmAnalysisTestUtil
 import com.intellij.testFramework.LightProjectDescriptor
 import com.intellij.testFramework.TestDataPath
 
-private const val inspectionPath = "/codeInspection/sourceToSinkFlow/markAsSafeFix"
+private const val INSPECTION_PATH = "/codeInspection/sourceToSinkFlow/markAsSafeFix"
 
-@TestDataPath("\$CONTENT_ROOT/testData$inspectionPath")
+@TestDataPath("\$CONTENT_ROOT/testData$INSPECTION_PATH")
 class JavaMarkAsSafeFixSourceToSinkFlowInspectionTest : SourceToSinkFlowInspectionTestBase() {
 
   override fun getProjectDescriptor(): LightProjectDescriptor {
@@ -15,87 +16,90 @@ class JavaMarkAsSafeFixSourceToSinkFlowInspectionTest : SourceToSinkFlowInspecti
   }
 
   override fun getBasePath(): String {
-    return JavaJvmAnalysisTestUtil.TEST_DATA_PROJECT_RELATIVE_BASE_PATH + inspectionPath
+    return JavaJvmAnalysisTestUtil.TEST_DATA_PROJECT_RELATIVE_BASE_PATH + INSPECTION_PATH
   }
+
+  private fun getMessage() = JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.mark.as.safe.text")
+
   fun `test unsafe var`() {
     prepareCheckFramework()
-    myFixture.testQuickFixUnavailable("UnsafeVar.java", "Mark 's2' as requiring validation")
+    myFixture.testQuickFixUnavailable("UnsafeVar.java", getMessage())
   }
   fun `test unsafe method call`() {
     prepareCheckFramework()
-    myFixture.testQuickFixUnavailable("UnsafeMethodCall.java", "Mark 'source' as requiring validation")
+    myFixture.testQuickFixUnavailable("UnsafeMethodCall.java", getMessage())
   }
   fun `test simple`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("Simple.java", "Mark 's' as requiring validation", true)
+    myFixture.testQuickFix("Simple.java", getMessage(), true)
   }
 
   fun `test safe var`() {
     prepareCheckFramework()
-    myFixture.testQuickFixUnavailable("SafeVar.java", "Mark 's' as requiring validation")
+    myFixture.testQuickFixUnavailable("SafeVar.java", getMessage())
   }
 
   fun `test multiple methods`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("MultipleMethods.java", "Mark 's2' as requiring validation", true)
+    myFixture.testQuickFix("MultipleMethods.java", getMessage(), true)
   }
 
   fun `test method calls`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("MethodCall.java", "Mark 'foo' as requiring validation", true)
+    myFixture.testQuickFix("MethodCall.java", getMessage(), true)
   }
 
   fun `test fields`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("Field.java", "Mark 's2' as requiring validation", true)
+    myFixture.testQuickFix("Field.java", getMessage(), true)
   }
 
   fun `test chained vars`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("ChainedVars.java", "Mark 's2' as requiring validation", true)
+    myFixture.testQuickFix("ChainedVars.java", getMessage(), true)
   }
 
   fun `test alias var`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("AliasVar.java", "Mark 'alias' as requiring validation", true)
+    myFixture.testQuickFix("AliasVar.java", getMessage(), true)
   }
 
   fun `test records`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("Records.java", "Mark 's' as requiring validation", true)
+    myFixture.testQuickFix("Records.java", getMessage(), true)
   }
 
   fun `test common cases with checkFramework`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("CommonCasesCheckFramework.java", "Mark 's1' as requiring validation", true)
+    myFixture.testQuickFix("CommonCasesCheckFramework.java", getMessage(), true)
   }
 
   fun `test common cases with jsr`() {
     prepareJsr()
-    myFixture.testQuickFix("CommonCasesJsr.java", "Mark 's1' as requiring validation", true)
+    myFixture.testQuickFix("CommonCasesJsr.java", getMessage(), true)
   }
 
   fun `test unknown field`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("UnknownField.java", "Mark 's' as requiring validation", true)
+    myFixture.testQuickFix("UnknownField.java", getMessage(), true)
   }
 
   fun `test unknown method`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("UnknownMethod.java", "Mark 's' as requiring validation", true)
+    myFixture.testQuickFix("UnknownMethod.java", getMessage(), true)
   }
 
   fun `test tainted method`() {
     prepareCheckFramework()
-    myFixture.testQuickFixUnavailable("TaintedMethod.java", "Mark 's' as requiring validation")
+    myFixture.testQuickFixUnavailable("TaintedMethod.java", getMessage())
   }
 
   fun `test recursive 2 path`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("RecursiveTwoPaths.java", "Mark 's' as requiring validation", true)
+    myFixture.testQuickFix("RecursiveTwoPaths.java", getMessage(), true)
   }
   fun `test recursive`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("Recursive.java", "Mark 'id' as requiring validation", true)
+    myFixture.testQuickFix("Recursive.java", getMessage(), true)
   }
 }

jvm/jvm-analysis-java-tests/testSrc/com/intellij/codeInspection/tests/java/sourceToSink/JavaPropagateFixSourceToSinkFlowInspectionTest.kt

@@ -1,13 +1,14 @@
 package com.intellij.codeInspection.tests.java.sourceToSink
 
+import com.intellij.analysis.JvmAnalysisBundle
 import com.intellij.codeInspection.tests.sourceToSink.SourceToSinkFlowInspectionTestBase
 import com.intellij.jvm.analysis.JavaJvmAnalysisTestUtil
 import com.intellij.testFramework.LightProjectDescriptor
 import com.intellij.testFramework.TestDataPath
 
-private const val inspectionPath = "/codeInspection/sourceToSinkFlow/propagateSafe"
+private const val INSPECTION_PATH = "/codeInspection/sourceToSinkFlow/propagateSafe"
 
-@TestDataPath("\$CONTENT_ROOT/testData$inspectionPath")
+@TestDataPath("\$CONTENT_ROOT/testData$INSPECTION_PATH")
 class JavaPropagateFixSourceToSinkFlowInspectionTest : SourceToSinkFlowInspectionTestBase() {
 
   override fun getProjectDescriptor(): LightProjectDescriptor {
@@ -15,81 +16,83 @@ class JavaPropagateFixSourceToSinkFlowInspectionTest : SourceToSinkFlowInspectio
   }
 
   override fun getBasePath(): String {
-    return JavaJvmAnalysisTestUtil.TEST_DATA_PROJECT_RELATIVE_BASE_PATH + inspectionPath
+    return JavaJvmAnalysisTestUtil.TEST_DATA_PROJECT_RELATIVE_BASE_PATH + INSPECTION_PATH
   }
 
+  private fun getMessage() = JvmAnalysisBundle.message("jvm.inspections.source.unsafe.to.sink.flow.propagate.safe.text")
+
   fun `test ParameterMethodUntainted`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("ParameterMethodUntainted.java", "Show propagation tree from 'a'")
+    myFixture.testQuickFix("ParameterMethodUntainted.java", getMessage())
   }
 
   fun `test tainted field`() {
     prepareCheckFramework()
-    myFixture.testQuickFixUnavailable("TaintedField.java", "Show propagation tree from 'a'")
+    myFixture.testQuickFixUnavailable("TaintedField.java", getMessage())
   }
 
   fun `test ParameterParameterUntainted`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("ParameterParameterUntainted.java", "Show propagation tree from 'a'")
+    myFixture.testQuickFix("ParameterParameterUntainted.java", getMessage())
   }
 
   fun `test ParameterMethodUnknown`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("ParameterMethodUnknown.java", "Show propagation tree from 's'")
+    myFixture.testQuickFix("ParameterMethodUnknown.java", getMessage())
   }
 
   fun `test ParameterMethodTainted`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("ParameterMethodTainted.java", "Show propagation tree from 's'")
+    myFixture.testQuickFix("ParameterMethodTainted.java", getMessage())
   }
 
   fun `test ParameterFieldUnknown`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("ParameterFieldUnknown.java", "Show propagation tree from 's'")
+    myFixture.testQuickFix("ParameterFieldUnknown.java", getMessage())
   }
 
   fun `test ParameterFieldTainted`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("ParameterFieldTainted.java", "Show propagation tree from 's'")
+    myFixture.testQuickFix("ParameterFieldTainted.java", getMessage())
   }
 
   fun `test Parameter`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("Parameter.java", "Show propagation tree from 's'")
+    myFixture.testQuickFix("Parameter.java", getMessage())
   }
 
   fun `test MethodMethodUnknown`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("MethodMethodUnknown.java", "Show propagation tree from 's'")
+    myFixture.testQuickFix("MethodMethodUnknown.java", getMessage())
   }
 
   fun `test MethodMethodTainted`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("MethodMethodTainted.java", "Show propagation tree from 's'")
+    myFixture.testQuickFix("MethodMethodTainted.java", getMessage())
   }
 
   fun `test MethodFieldUnknown`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("MethodFieldUnknown.java", "Show propagation tree from 's'")
+    myFixture.testQuickFix("MethodFieldUnknown.java", getMessage())
   }
 
   fun `test MethodFieldTainted`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("MethodFieldTainted.java", "Show propagation tree from 's'")
+    myFixture.testQuickFix("MethodFieldTainted.java", getMessage())
   }
 
   fun `test MethodFieldMethodUnknown`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("MethodFieldMethodUnknown.java", "Show propagation tree from 's'")
+    myFixture.testQuickFix("MethodFieldMethodUnknown.java", getMessage())
   }
 
   fun `test MethodFieldMethodTainted`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("MethodFieldMethodTainted.java", "Show propagation tree from 's'")
+    myFixture.testQuickFix("MethodFieldMethodTainted.java", getMessage())
   }
 
   fun `test AnotherClassMethodCall`() {
     prepareCheckFramework()
-    myFixture.testQuickFix("AnotherClassMethodCall.java", "Show propagation tree from 's'")
+    myFixture.testQuickFix("AnotherClassMethodCall.java", getMessage())
   }
 }

jvm/jvm-analysis-java-tests/testSrc/com/intellij/java/propertyBased/SourceToSinkPropertyInspectionTest.java

@@ -34,10 +34,11 @@ public class SourceToSinkPropertyInspectionTest extends LightJavaCodeInsightFixt
     import com.jetbrains.OtherClass;
     import org.checkerframework.checker.tainting.qual.*;
     class A {
+        String tainted;
         void callSink() {}
         void sink(@Untainted String s) {}
         static @Tainted String source() { return "unsafe"; }
-        static String foo() { return "bar"; }
+        static String foo(String... a) { return a.length==0? "bar" : a[0]; }
         static @Untainted String safe() { return "safe"; }
     }""";
 
@@ -75,7 +76,7 @@ public class SourceToSinkPropertyInspectionTest extends LightJavaCodeInsightFixt
                          import org.checkerframework.checker.tainting.qual.*;
                          class OtherClass {
                              static @Tainted String source() { return "unsafe"; }
-                             static String foo() { return "bar"; }
+                             static String foo(String... a) { return a.length==0? "bar" : a[0]; }
                              static @Untainted String safe() { return "safe"; }
                          }""");
     PropertyChecker.customized()
@@ -191,11 +192,11 @@ public class SourceToSinkPropertyInspectionTest extends LightJavaCodeInsightFixt
     }
 
     public void declareField(@NotNull String varName, @NotNull String declarationText) {
-      declarationText = String.format("private String %s = %s", varName, declarationText);
+      declarationText = String.format("public String %s = %s", varName, declarationText);
       PsiField field = myFactory.createFieldFromText(declarationText, null);
       PsiClass containingClass = myMethod.getContainingClass();
-      PsiElement lBrace = containingClass.getLBrace();
-      containingClass.addAfter(field, lBrace);
+      PsiElement previousField = PsiTreeUtil.findChildOfType(containingClass, PsiField.class);
+      containingClass.addAfter(field, previousField);
     }
 
     public void declareLocalVariable(@NotNull String varName, @NotNull String declarationText) {
@@ -231,6 +232,9 @@ public class SourceToSinkPropertyInspectionTest extends LightJavaCodeInsightFixt
 
     @Override
     public @NotNull String getText() {
+      if (myIsExternal && myCallType.methodName().equals("foo")) {
+        return "OtherClass.foo(tainted)";
+      }
       return (myIsExternal ? "OtherClass." : "") + myCallType.methodName() + "()";
     }