Merge pull request #14849 from github/kaeluka/automodel-extraction-skip-primitive-types-candidates

Java Automodel extraction: remove primitives in framework mode

Author: Stephan Brandauer

java/ql/automodel/src/AutomodelFrameworkModeCharacteristics.qll

@@ -24,9 +24,21 @@ newtype JavaRelatedLocationType =
   ClassDoc()
 
 newtype TFrameworkModeEndpoint =
-  TExplicitParameter(Parameter p) or
+  TExplicitParameter(Parameter p) {
+    not p.getType() instanceof PrimitiveType and
+    not p.getType() instanceof BoxedType and
+    not p.getType() instanceof NumberType
+  } or
   TQualifier(Callable c) { not c instanceof Constructor } or
-  TReturnValue(Callable c) or
+  TReturnValue(Callable c) {
+    c instanceof Constructor
+    or
+    c instanceof Method and
+    (
+      not c.getReturnType() instanceof VoidType and
+      not c.getReturnType() instanceof PrimitiveType
+    )
+  } or
   TOverridableParameter(Method m, Parameter p) {
     p.getCallable() = m and
     m instanceof ModelExclusions::ModelApi and

java/ql/automodel/test/AutomodelFrameworkModeExtraction/AutomodelFrameworkModeExtractCandidates.expected

@@ -1,5 +1,4 @@
 | java/io/File.java:4:16:4:24 | compareTo | known non-sink\nrelated locations: $@, $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@, $@, $@. | java/io/File.java:4:16:4:24 | compareTo | MethodDoc | java/io/File.java:4:16:4:24 | compareTo | ClassDoc | file://java.io:1:1:1:1 | java.io | package | file://File:1:1:1:1 | File | type | file://true:1:1:1:1 | true | subtypes | file://compareTo:1:1:1:1 | compareTo | name | file://(File):1:1:1:1 | (File) | signature | file://:1:1:1:1 |  | input | file://Parameter[this]:1:1:1:1 | Parameter[this] | output | file://this:1:1:1:1 | this | parameterName | file://sourceModel:1:1:1:1 | sourceModel | extensibleType |
-| java/io/File.java:4:16:4:24 | compareTo | known non-sink\nrelated locations: $@, $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@, $@, $@. | java/io/File.java:4:16:4:24 | compareTo | MethodDoc | java/io/File.java:4:16:4:24 | compareTo | ClassDoc | file://java.io:1:1:1:1 | java.io | package | file://File:1:1:1:1 | File | type | file://true:1:1:1:1 | true | subtypes | file://compareTo:1:1:1:1 | compareTo | name | file://(File):1:1:1:1 | (File) | signature | file://:1:1:1:1 |  | input | file://ReturnValue:1:1:1:1 | ReturnValue | output | file://:1:1:1:1 |  | parameterName | file://sourceModel:1:1:1:1 | sourceModel | extensibleType |
 | java/io/File.java:4:16:4:24 | compareTo | known non-sink\nrelated locations: $@, $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@, $@, $@. | java/io/File.java:4:16:4:24 | compareTo | MethodDoc | java/io/File.java:4:16:4:24 | compareTo | ClassDoc | file://java.io:1:1:1:1 | java.io | package | file://File:1:1:1:1 | File | type | file://true:1:1:1:1 | true | subtypes | file://compareTo:1:1:1:1 | compareTo | name | file://(File):1:1:1:1 | (File) | signature | file://Argument[this]:1:1:1:1 | Argument[this] | input | file://:1:1:1:1 |  | output | file://this:1:1:1:1 | this | parameterName | file://sinkModel:1:1:1:1 | sinkModel | extensibleType |
 | java/io/File.java:5:9:5:21 | pathname | known non-sink\nrelated locations: $@, $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@, $@, $@. | java/io/File.java:5:9:5:21 | pathname | MethodDoc | java/io/File.java:5:9:5:21 | pathname | ClassDoc | file://java.io:1:1:1:1 | java.io | package | file://File:1:1:1:1 | File | type | file://true:1:1:1:1 | true | subtypes | file://compareTo:1:1:1:1 | compareTo | name | file://(File):1:1:1:1 | (File) | signature | file://:1:1:1:1 |  | input | file://Parameter[0]:1:1:1:1 | Parameter[0] | output | file://pathname:1:1:1:1 | pathname | parameterName | file://sourceModel:1:1:1:1 | sourceModel | extensibleType |
 | java/io/File.java:5:9:5:21 | pathname | known non-sink\nrelated locations: $@, $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@, $@, $@. | java/io/File.java:5:9:5:21 | pathname | MethodDoc | java/io/File.java:5:9:5:21 | pathname | ClassDoc | file://java.io:1:1:1:1 | java.io | package | file://File:1:1:1:1 | File | type | file://true:1:1:1:1 | true | subtypes | file://compareTo:1:1:1:1 | compareTo | name | file://(File):1:1:1:1 | (File) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | file://:1:1:1:1 |  | output | file://pathname:1:1:1:1 | pathname | parameterName | file://sinkModel:1:1:1:1 | sinkModel | extensibleType |

java/ql/automodel/test/AutomodelFrameworkModeExtraction/AutomodelFrameworkModeExtractNegativeExamples.expected

@@ -3,11 +3,11 @@
 public class PublicClass {
   public void stuff(String arg) { // `arg` is a sink candidate, `this` is a candidate, `arg` is a source candidate (overrideable method)
     System.out.println(arg);
-  } // method stuff is a candidate source
+  }
 
   public static void staticStuff(String arg) { // `arg` is a candidate, `this` is not a candidate (static method), `arg` is not a source candidate (static methods can not be overloaded)
     System.out.println(arg);
-  } // method staticStuff is a candidate source
+  }
 
   // `arg` and `this` are candidates because the method is protected (may be called from downstream repositories). The return value is a candidate source for the same reason.
   protected void nonPublicStuff(String arg) {

java/ql/automodel/test/AutomodelFrameworkModeExtraction/com/github/codeql/test/PublicClass.java

@@ -1,9 +1,9 @@
 package com.github.codeql.test;
 
 public interface PublicInterface {
-    public void stuff(String arg); // `arg` is a candidate, `this` is a candidate, method stuff is a candidate source, `arg` is a source candidate (overrideable method)
+    public int stuff(String arg); // `arg` is a candidate, `this` is a candidate, method stuff is _not_ a candidate source (primitive return type), `arg` is a source candidate (overridable method)
 
     public static void staticStuff(String arg) { // `arg` is a candidate, `this` is not a candidate (static method)
       System.out.println(arg);
-    } // method staticStuff is a candidate source
+    }
 }

java/ql/automodel/test/AutomodelFrameworkModeExtraction/com/github/codeql/test/PublicInterface.java

@@ -5,5 +5,9 @@ public int compareTo( // `this` is a negative example - this is modeled as a neu
         File pathname // negative example - this is modeled as a neutral model
     ) {
         return 0;
-    } // also a negative example for ReturnValue source
+    }
+
+    public boolean setLastModified(long time) {
+        return false;
+    } // return value is not a source candidate because it's a primitive
 }