Refactor data extensions editor queries to reduce duplication

This refactors the data extensions editor queries to use a new
`AutomodelVsCode` module. This module is based on the `ExternalApi`
module, but is more general and can be used for retrieving public
methods from the source as well. The actual conditions are now in the
queries themselves.

This reduces the duplicated module in the framework mode query and will
mean that when we update the `ExternalApi` module, we will just have to
port it to the `AutomodelVsCode` module, and not to the `ExternalApi`
and a separate framework mode query.

extensions/ql-vscode/src/data-extensions-editor/queries/csharp.ts

@@ -9,18 +9,19 @@ export const fetchExternalApisQuery: Query = {
  * @id cs/telemetry/fetch-external-apis
  */
 
-import csharp
+private import csharp
-import ExternalApi
+private import AutomodelVsCode
+
+class ExternalApi extends CallableMethod {
+  ExternalApi() {
+    this.isUnboundDeclaration() and
+    this.fromLibrary() and
+    this.(Modifiable).isEffectivelyPublic()
+  }
+}
 
 private Call aUsage(ExternalApi api) { result.getTarget().getUnboundDeclaration() = api }
 
-private boolean isSupported(ExternalApi api) {
-  api.isSupported() and result = true
-  or
-  not api.isSupported() and
-  result = false
-}
-
 from ExternalApi api, string apiName, boolean supported, Call usage
 where
   apiName = api.getApiName() and
@@ -29,142 +30,30 @@ where
 select usage, apiName, supported.toString(), "supported", api.getFile().getBaseName(), "library"
 `,
   frameworkModeQuery: `/**
- * @name Usage of APIs coming from external libraries
+ * @name Public methods
- * @description A list of 3rd party APIs used in the codebase.
+ * @description A list of APIs callable by consumers. Excludes test and generated code.
  * @tags telemetry
  * @kind problem
- * @id cs/telemetry/fetch-external-apis
+ * @id cs/telemetry/fetch-public-methods
  */
 
 private import csharp
-private import cil
 private import dotnet
-private import semmle.code.csharp.dispatch.Dispatch
+private import AutomodelVsCode
-private import semmle.code.csharp.dataflow.ExternalFlow
-private import semmle.code.csharp.dataflow.FlowSummary
-private import semmle.code.csharp.dataflow.internal.DataFlowImplCommon as DataFlowImplCommon
-private import semmle.code.csharp.dataflow.internal.DataFlowPrivate
-private import semmle.code.csharp.dataflow.internal.DataFlowDispatch as DataFlowDispatch
-private import semmle.code.csharp.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl
-private import semmle.code.csharp.dataflow.internal.TaintTrackingPrivate
-private import semmle.code.csharp.security.dataflow.flowsources.Remote
 
-pragma[nomagic]
+class PublicMethod extends CallableMethod {
-private predicate isTestNamespace(Namespace ns) {
+  PublicMethod() { this.fromSource() }
-  ns.getFullName()
-      .matches([
-          "NUnit.Framework%", "Xunit%", "Microsoft.VisualStudio.TestTools.UnitTesting%", "Moq%"
-        ])
-}
-
-/**
- * A test library.
- */
-class TestLibrary extends RefType {
-  TestLibrary() { isTestNamespace(this.getNamespace()) }
-}
-
-/** Holds if the given callable is not worth supporting. */
-private predicate isUninteresting(DotNet::Callable c) {
-  c.getDeclaringType() instanceof TestLibrary or
-  c.(Constructor).isParameterless() or
-  c.getDeclaringType() instanceof AnonymousClass
-}
-
-class PublicMethod extends DotNet::Member {
-  PublicMethod() { this.isPublic() and not isUninteresting(this) }
-
-  /**
-   * Gets the unbound type, name and parameter types of this API.
-   */
-  bindingset[this]
-  private string getSignature() {
-    result =
-      this.getDeclaringType().getUnboundDeclaration() + "." + this.getName() + "(" +
-        parameterQualifiedTypeNamesToString(this) + ")"
-  }
-
-  /**
-   * Gets the namespace of this API.
-   */
-  bindingset[this]
-  string getNamespace() { this.getDeclaringType().hasQualifiedName(result, _) }
-
-  /**
-   * Gets the namespace and signature of this API.
-   */
-  bindingset[this]
-  string getApiName() { result = this.getNamespace() + "#" + this.getSignature() }
-
-  /** Gets a node that is an input to a call to this API. */
-  private ArgumentNode getAnInput() {
-    result
-        .getCall()
-        .(DataFlowDispatch::NonDelegateDataFlowCall)
-        .getATarget(_)
-        .getUnboundDeclaration() = this
-  }
-
-  /** Gets a node that is an output from a call to this API. */
-  private DataFlow::Node getAnOutput() {
-    exists(
-      Call c, DataFlowDispatch::NonDelegateDataFlowCall dc, DataFlowImplCommon::ReturnKindExt ret
-    |
-      dc.getDispatchCall().getCall() = c and
-      c.getTarget().getUnboundDeclaration() = this
-    |
-      result = ret.getAnOutNode(dc)
-    )
-  }
-
-  /** Holds if this API has a supported summary. */
-  pragma[nomagic]
-  predicate hasSummary() {
-    this instanceof SummarizedCallable
-    or
-    defaultAdditionalTaintStep(this.getAnInput(), _)
-  }
-
-  /** Holds if this API is a known source. */
-  pragma[nomagic]
-  predicate isSource() {
-    this.getAnOutput() instanceof RemoteFlowSource or sourceNode(this.getAnOutput(), _)
-  }
-
-  /** Holds if this API is a known sink. */
-  pragma[nomagic]
-  predicate isSink() { sinkNode(this.getAnInput(), _) }
-
-  /** Holds if this API is a known neutral. */
-  pragma[nomagic]
-  predicate isNeutral() { this instanceof FlowSummaryImpl::Public::NeutralCallable }
-
-  /**
-   * Holds if this API is supported by existing CodeQL libraries, that is, it is either a
-   * recognized source, sink or neutral or it has a flow summary.
-   */
-  predicate isSupported() {
-    this.hasSummary() or this.isSource() or this.isSink() or this.isNeutral()
-  }
-}
-
-private boolean isSupported(PublicMethod publicMethod) {
-  publicMethod.isSupported() and result = true
-  or
-  not publicMethod.isSupported() and
-  result = false
 }
 
 from PublicMethod publicMethod, string apiName, boolean supported
 where
   apiName = publicMethod.getApiName() and
-  publicMethod.getDeclaringType().fromSource() and
   supported = isSupported(publicMethod)
 select publicMethod, apiName, supported.toString(), "supported",
   publicMethod.getFile().getBaseName(), "library"
 `,
   dependencies: {
-    "ExternalApi.qll": `/** Provides classes and predicates related to handling APIs from external libraries. */
+    "AutomodelVsCode.qll": `/** Provides classes and predicates related to handling APIs for the VS Code extension. */
 
 private import csharp
 private import dotnet
@@ -194,18 +83,17 @@ class TestLibrary extends RefType {
 }
 
 /** Holds if the given callable is not worth supporting. */
-private predicate isUninteresting(DotNet::Callable c) {
+private predicate isUninteresting(DotNet::Declaration c) {
   c.getDeclaringType() instanceof TestLibrary or
-  c.(Constructor).isParameterless()
+  c.(Constructor).isParameterless() or
+  c.getDeclaringType() instanceof AnonymousClass
 }
 
 /**
- * An external API from either the C# Standard Library or a 3rd party library.
+ * An callable method from either the C# Standard Library, a 3rd party library, or from the source.
  */
-class ExternalApi extends DotNet::Callable {
+class CallableMethod extends DotNet::Declaration {
-  ExternalApi() {
+  CallableMethod() {
-    this.isUnboundDeclaration() and
-    this.fromLibrary() and
     this.(Modifiable).isEffectivelyPublic() and
     not isUninteresting(this)
   }
@@ -284,47 +172,11 @@ class ExternalApi extends DotNet::Callable {
   }
 }
 
-/**
+boolean isSupported(CallableMethod callableMethod) {
- * Gets the limit for the number of results produced by a telemetry query.
+  callableMethod.isSupported() and result = true
- */
+  or
-int resultLimit() { result = 1000 }
+  not callableMethod.isSupported() and
-
+  result = false
-/**
- * Holds if it is relevant to count usages of \`api\`.
- */
-signature predicate relevantApi(ExternalApi api);
-
-/**
- * Given a predicate to count relevant API usages, this module provides a predicate
- * for restricting the number or returned results based on a certain limit.
- */
-module Results<relevantApi/1 getRelevantUsages> {
-  private int getUsages(string apiName) {
-    result =
-      strictcount(Call c, ExternalApi api |
-        c.getTarget().getUnboundDeclaration() = api and
-        apiName = api.getApiName() and
-        getRelevantUsages(api)
-      )
-  }
-
-  private int getOrder(string apiName) {
-    apiName =
-      rank[result](string name, int usages |
-        usages = getUsages(name)
-      |
-        name order by usages desc, name
-      )
-  }
-
-  /**
-   * Holds if there exists an API with \`apiName\` that is being used \`usages\` times
-   * and if it is in the top results (guarded by resultLimit).
-   */
-  predicate restrict(string apiName, int usages) {
-    usages = getUsages(apiName) and
-    getOrder(apiName) <= resultLimit()
-  }
 }
 `,
   },

extensions/ql-vscode/src/data-extensions-editor/queries/java.ts

@@ -10,25 +10,24 @@ export const fetchExternalApisQuery: Query = {
  */
 
 import java
-import ExternalApi
+import AutomodelVsCode
+
+class ExternalApi extends CallableMethod {
+  ExternalApi() { not this.fromSource() }
+}
 
 private Call aUsage(ExternalApi api) {
   result.getCallee().getSourceDeclaration() = api and
   not result.getFile() instanceof GeneratedFile
 }
 
-private boolean isSupported(ExternalApi api) {
+from ExternalApi externalApi, string apiName, boolean supported, Call usage
-  api.isSupported() and result = true
-  or
-  not api.isSupported() and result = false
-}
-
-from ExternalApi api, string apiName, boolean supported, Call usage
 where
-  apiName = api.getApiName() and
+  apiName = externalApi.getApiName() and
-  supported = isSupported(api) and
+  supported = isSupported(externalApi) and
-  usage = aUsage(api)
+  usage = aUsage(externalApi)
-select usage, apiName, supported.toString(), "supported", api.jarContainer(), "library"
+select usage, apiName, supported.toString(), "supported",
+  externalApi.getCompilationUnit().getParentContainer().getBaseName(), "library"
 `,
   frameworkModeQuery: `/**
  * @name Public methods
@@ -39,94 +38,21 @@ select usage, apiName, supported.toString(), "supported", api.jarContainer(), "l
  */
 
 import java
-private import semmle.code.java.dataflow.DataFlow
+import AutomodelVsCode
-private import semmle.code.java.dataflow.ExternalFlow
-private import semmle.code.java.dataflow.FlowSources
-private import semmle.code.java.dataflow.FlowSummary
-private import semmle.code.java.dataflow.internal.DataFlowPrivate
-private import semmle.code.java.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl
-private import semmle.code.java.dataflow.internal.ModelExclusions
 
-/** Holds if the given callable is not worth supporting. */
+class PublicMethodFromSource extends CallableMethod {
-private predicate isUninteresting(Callable c) {
+  PublicMethodFromSource() { this.isPublic() and this.fromSource() }
-  c.getDeclaringType() instanceof TestLibrary or
-  c.(Constructor).isParameterless() or
-  c.getDeclaringType() instanceof AnonymousClass
 }
 
-class PublicMethod extends Method {
+from PublicMethodFromSource publicMethod, string apiName, boolean supported
-  PublicMethod() { this.isPublic() and not isUninteresting(this) }
-
-  /**
-   * Gets information about the method in the form expected by the MaD modeling framework.
-   */
-  string getApiName() {
-    result =
-      this.getDeclaringType().getPackage() + "." + this.getDeclaringType().getSourceDeclaration() +
-        "#" + this.getName() + paramsString(this)
-  }
-
-  /** Gets a node that is an input to a call to this API. */
-  private DataFlow::Node getAnInput() {
-    exists(Call call | call.getCallee().getSourceDeclaration() = this |
-      result.asExpr().(Argument).getCall() = call or
-      result.(ArgumentNode).getCall().asCall() = call
-    )
-  }
-
-  /** Gets a node that is an output from a call to this API. */
-  private DataFlow::Node getAnOutput() {
-    exists(Call call | call.getCallee().getSourceDeclaration() = this |
-      result.asExpr() = call or
-      result.(DataFlow::PostUpdateNode).getPreUpdateNode().(ArgumentNode).getCall().asCall() = call
-    )
-  }
-
-  /** Holds if this API has a supported summary. */
-  pragma[nomagic]
-  predicate hasSummary() {
-    this = any(SummarizedCallable sc).asCallable() or
-    TaintTracking::localAdditionalTaintStep(this.getAnInput(), _)
-  }
-
-  pragma[nomagic]
-  predicate isSource() {
-    this.getAnOutput() instanceof RemoteFlowSource or sourceNode(this.getAnOutput(), _)
-  }
-
-  /** Holds if this API is a known sink. */
-  pragma[nomagic]
-  predicate isSink() { sinkNode(this.getAnInput(), _) }
-
-  /** Holds if this API is a known neutral. */
-  pragma[nomagic]
-  predicate isNeutral() { this = any(FlowSummaryImpl::Public::NeutralCallable nsc).asCallable() }
-
-  /**
-   * Holds if this API is supported by existing CodeQL libraries, that is, it is either a
-   * recognized source, sink or neutral or it has a flow summary.
-   */
-  predicate isSupported() {
-    this.hasSummary() or this.isSource() or this.isSink() or this.isNeutral()
-  }
-}
-
-private boolean isSupported(PublicMethod publicMethod) {
-  publicMethod.isSupported() and result = true
-  or
-  not publicMethod.isSupported() and result = false
-}
-
-from PublicMethod publicMethod, string apiName, boolean supported
 where
   apiName = publicMethod.getApiName() and
-  publicMethod.getCompilationUnit().isSourceFile() and
   supported = isSupported(publicMethod)
 select publicMethod, apiName, supported.toString(), "supported",
   publicMethod.getCompilationUnit().getParentContainer().getBaseName(), "library"
 `,
   dependencies: {
-    "ExternalApi.qll": `/** Provides classes and predicates related to handling APIs from external libraries. */
+    "AutomodelVsCode.qll": `/** Provides classes and predicates related to handling APIs for the VS Code extension. */
 
 private import java
 private import semmle.code.java.dataflow.DataFlow
@@ -138,17 +64,18 @@ private import semmle.code.java.dataflow.internal.FlowSummaryImpl as FlowSummary
 private import semmle.code.java.dataflow.TaintTracking
 private import semmle.code.java.dataflow.internal.ModelExclusions
 
-/** Holds if the given callable is not worth supporting. */
+/** Holds if the given callable/method is not worth supporting. */
 private predicate isUninteresting(Callable c) {
   c.getDeclaringType() instanceof TestLibrary or
-  c.(Constructor).isParameterless()
+  c.(Constructor).isParameterless() or
+  c.getDeclaringType() instanceof AnonymousClass
 }
 
 /**
- * An external API from either the Standard Library or a 3rd party library.
+ * A callable method from either the Standard Library, a 3rd party library or from the source.
  */
-class ExternalApi extends Callable {
+class CallableMethod extends Method {
-  ExternalApi() { not this.fromSource() and not isUninteresting(this) }
+  CallableMethod() { not isUninteresting(this) }
 
   /**
    * Gets information about the external API in the form expected by the MaD modeling framework.
@@ -217,51 +144,10 @@ class ExternalApi extends Callable {
   }
 }
 
-/** DEPRECATED: Alias for ExternalApi */
+boolean isSupported(CallableMethod method) {
-deprecated class ExternalAPI = ExternalApi;
+  method.isSupported() and result = true
-
+  or
-/**
+  not method.isSupported() and result = false
- * Gets the limit for the number of results produced by a telemetry query.
- */
-int resultLimit() { result = 1000 }
-
-/**
- * Holds if it is relevant to count usages of \`api\`.
- */
-signature predicate relevantApi(ExternalApi api);
-
-/**
- * Given a predicate to count relevant API usages, this module provides a predicate
- * for restricting the number or returned results based on a certain limit.
- */
-module Results<relevantApi/1 getRelevantUsages> {
-  private int getUsages(string apiName) {
-    result =
-      strictcount(Call c, ExternalApi api |
-        c.getCallee().getSourceDeclaration() = api and
-        not c.getFile() instanceof GeneratedFile and
-        apiName = api.getApiName() and
-        getRelevantUsages(api)
-      )
-  }
-
-  private int getOrder(string apiInfo) {
-    apiInfo =
-      rank[result](string info, int usages |
-        usages = getUsages(info)
-      |
-        info order by usages desc, info
-      )
-  }
-
-  /**
-   * Holds if there exists an API with \`apiName\` that is being used \`usages\` times
-   * and if it is in the top results (guarded by resultLimit).
-   */
-  predicate restrict(string apiName, int usages) {
-    usages = getUsages(apiName) and
-    getOrder(apiName) <= resultLimit()
-  }
 }
 `,
   },

extensions/ql-vscode/test/vscode-tests/no-workspace/data-extensions-editor/external-api-usage-query.test.ts

@@ -117,7 +117,11 @@ describe("runQuery", () => {
 
       const queryFiles = await readdir(queryDirectory);
       expect(queryFiles.sort()).toEqual(
-        ["codeql-pack.yml", "FetchExternalApis.ql", "ExternalApi.qll"].sort(),
+        [
+          "codeql-pack.yml",
+          "FetchExternalApis.ql",
+          "AutomodelVsCode.qll",
+        ].sort(),
       );
 
       const suiteFileContents = await readFile(