Merge pull request #8466 from michaelnebel/csharp/refactor-aspartial

C#: Refactor asPartial to allow re-use.

Author: michaelnebel

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll

@@ -2031,3 +2031,47 @@ abstract class SyntheticField extends string {
  * Holds if the the content `c` is a container.
  */
 predicate containerContent(DataFlow::Content c) { c instanceof DataFlow::ElementContent }
+
+/**
+ * A module containing predicates related to generating models as data.
+ */
+module Csv {
+  private string parameterQualifiedTypeNamesToString(DataFlowCallable c) {
+    result =
+      concat(Parameter p, int i |
+        p = c.getParameter(i)
+      |
+        p.getType().getQualifiedName(), "," order by i
+      )
+  }
+
+  /** Holds if the summary should apply for all overrides of `c`. */
+  predicate isBaseCallableOrPrototype(DataFlowCallable c) {
+    c.getDeclaringType() instanceof Interface
+    or
+    exists(Modifiable m | m = [c.(Modifiable), c.(Accessor).getDeclaration()] |
+      m.isAbstract()
+      or
+      c.getDeclaringType().(Modifiable).isAbstract() and m.(Virtualizable).isVirtual()
+    )
+  }
+
+  /** Gets a string representing whether the summary should apply for all overrides of `c`. */
+  private string getCallableOverride(DataFlowCallable c) {
+    if isBaseCallableOrPrototype(c) then result = "true" else result = "false"
+  }
+
+  /** Computes the first 6 columns for CSV rows of `c`. */
+  string asPartialModel(DataFlowCallable c) {
+    exists(string namespace, string type |
+      c.getDeclaringType().hasQualifiedName(namespace, type) and
+      result =
+        namespace + ";" //
+          + type + ";" //
+          + getCallableOverride(c) + ";" //
+          + c.getName() + ";" //
+          + "(" + parameterQualifiedTypeNamesToString(c) + ")" //
+          + /* ext + */ ";" //
+    )
+  }
+}

csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll

@@ -1056,7 +1056,7 @@ module Private {
       |
         c.relevantSummary(input, output, preservesValue) and
         csv =
-          c.getCallableCsv() + ";;" + getComponentStackCsv(input) + ";" +
+          c.getCallableCsv() + ";" + getComponentStackCsv(input) + ";" +
             getComponentStackCsv(output) + ";" + renderKind(preservesValue)
       )
     }

csharp/ql/src/utils/model-generator/ModelGeneratorUtilsSpecific.qll

@@ -7,57 +7,20 @@ private import semmle.code.csharp.dataflow.internal.DataFlowDispatch
 private predicate isRelevantForModels(Callable api) { not api instanceof MainMethod }
 
 /**
- * A class of Callables that are relevant for generating summary, source and sinks models for.
+ * A class of callables that are relevant generating summary, source and sinks models for.
  *
- * In the Standard library and 3rd party libraries it the Callables that can be called
+ * In the Standard library and 3rd party libraries it the callables that can be called
  * from outside the library itself.
  */
-class TargetApi extends Callable {
+class TargetApi extends DataFlowCallable {
   TargetApi() {
     [this.(Modifiable), this.(Accessor).getDeclaration()].isEffectivelyPublic() and
     this.fromSource() and
     isRelevantForModels(this)
   }
 }
 
-private string parameterQualifiedTypeNamesToString(TargetApi api) {
-  result =
-    concat(Parameter p, int i |
-      p = api.getParameter(i)
-    |
-      p.getType().getQualifiedName(), "," order by i
-    )
-}
-
-/** Holds if the summary should apply for all overrides of this. */
-private predicate isBaseCallableOrPrototype(TargetApi api) {
-  api.getDeclaringType() instanceof Interface
-  or
-  exists(Modifiable m | m = [api.(Modifiable), api.(Accessor).getDeclaration()] |
-    m.isAbstract()
-    or
-    api.getDeclaringType().(Modifiable).isAbstract() and m.(Virtualizable).isVirtual()
-  )
-}
-
-/** Gets a string representing whether the summary should apply for all overrides of this. */
-private string getCallableOverride(TargetApi api) {
-  if isBaseCallableOrPrototype(api) then result = "true" else result = "false"
-}
-
-/** Computes the first 6 columns for CSV rows. */
-string asPartialModel(TargetApi api) {
-  exists(string namespace, string type |
-    api.getDeclaringType().hasQualifiedName(namespace, type) and
-    result =
-      namespace + ";" //
-        + type + ";" //
-        + getCallableOverride(api) + ";" //
-        + api.getName() + ";" //
-        + "(" + parameterQualifiedTypeNamesToString(api) + ")" //
-        + /* ext + */ ";" //
-  )
-}
+predicate asPartialModel = Csv::asPartialModel/1;
 
 /**
  * Holds for type `t` for fields that are relevant as an intermediate

csharp/ql/test/library-tests/dataflow/library/FlowSummariesFiltered.ql

@@ -1,4 +1,5 @@
 import shared.FlowSummaries
+private import semmle.code.csharp.dataflow.internal.DataFlowPrivate::Csv
 private import semmle.code.csharp.dataflow.ExternalFlow
 
 class IncludeFilteredSummarizedCallable extends IncludeSummarizedCallable {
@@ -14,7 +15,7 @@ class IncludeFilteredSummarizedCallable extends IncludeSummarizedCallable {
   ) {
     this.propagatesFlow(input, output, preservesValue) and
     not exists(IncludeSummarizedCallable rsc |
-      rsc.isBaseCallableOrPrototype() and
+      isBaseCallableOrPrototype(rsc) and
       rsc.propagatesFlow(input, output, preservesValue) and
       this.(UnboundCallable).overridesOrImplementsUnbound(rsc)
     )

csharp/ql/test/shared/FlowSummaries.qll

@@ -1,44 +1,12 @@
 import semmle.code.csharp.dataflow.FlowSummary
 import semmle.code.csharp.dataflow.internal.FlowSummaryImpl::Private::TestOutput
+private import semmle.code.csharp.dataflow.internal.DataFlowPrivate
 
 abstract class IncludeSummarizedCallable extends RelevantSummarizedCallable {
   IncludeSummarizedCallable() {
     [this.(Modifiable), this.(Accessor).getDeclaration()].isEffectivelyPublic()
   }
 
-  /** Gets the qualified parameter types of this callable as a comma-separated string. */
-  private string parameterQualifiedTypeNamesToString() {
-    result =
-      concat(Parameter p, int i |
-        p = this.getParameter(i)
-      |
-        p.getType().getQualifiedName(), "," order by i
-      )
-  }
-
-  /** Holds if the summary should apply for all overrides of this. */
-  predicate isBaseCallableOrPrototype() {
-    this.getDeclaringType() instanceof Interface
-    or
-    exists(Modifiable m | m = [this.(Modifiable), this.(Accessor).getDeclaration()] |
-      m.isAbstract()
-      or
-      this.getDeclaringType().(Modifiable).isAbstract() and m.(Virtualizable).isVirtual()
-    )
-  }
-
-  /** Gets a string representing whether the summary should apply for all overrides of this. */
-  private string getCallableOverride() {
-    if this.isBaseCallableOrPrototype() then result = "true" else result = "false"
-  }
-
   /** Gets a string representing the callable in semi-colon separated format for use in flow summaries. */
-  final override string getCallableCsv() {
-    exists(string namespace, string type |
-      this.getDeclaringType().hasQualifiedName(namespace, type) and
-      result =
-        namespace + ";" + type + ";" + this.getCallableOverride() + ";" + this.getName() + ";" + "("
-          + this.parameterQualifiedTypeNamesToString() + ")"
-    )
-  }
+  final override string getCallableCsv() { result = Csv::asPartialModel(this) }
 }

java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll

@@ -1056,7 +1056,7 @@ module Private {
       |
         c.relevantSummary(input, output, preservesValue) and
         csv =
-          c.getCallableCsv() + ";;" + getComponentStackCsv(input) + ";" +
+          c.getCallableCsv() + ";" + getComponentStackCsv(input) + ";" +
             getComponentStackCsv(output) + ";" + renderKind(preservesValue)
       )
     }

java/ql/src/utils/model-generator/ModelGeneratorUtilsSpecific.qll

@@ -5,7 +5,7 @@ private import semmle.code.java.dataflow.ExternalFlow
 private import semmle.code.java.dataflow.internal.ContainerFlow
 private import semmle.code.java.dataflow.internal.DataFlowImplCommon
 
-Method superImpl(Method m) {
+private Method superImpl(Method m) {
   result = m.getAnOverride() and
   not exists(result.getAnOverride()) and
   not m instanceof ToStringMethod

ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll

@@ -1056,7 +1056,7 @@ module Private {
       |
         c.relevantSummary(input, output, preservesValue) and
         csv =
-          c.getCallableCsv() + ";;" + getComponentStackCsv(input) + ";" +
+          c.getCallableCsv() + ";" + getComponentStackCsv(input) + ";" +
             getComponentStackCsv(output) + ";" + renderKind(preservesValue)
       )
     }