C#: Address comments in the QL implementation.

Author: michaelnebel

csharp/ql/lib/csharp.qll

@@ -36,7 +36,7 @@ import semmle.code.csharp.controlflow.ControlFlowGraph
 import semmle.code.csharp.dataflow.DataFlow
 import semmle.code.csharp.dataflow.TaintTracking
 import semmle.code.csharp.dataflow.SSA
-private import semmle.code.csharp.Overlay
+private import semmle.code.csharp.internal.Overlay
 
 /** Whether the source was extracted without a build command. */
 predicate extractionIsStandalone() { exists(SourceFile f | f.extractedStandalone()) }

csharp/ql/lib/semmle/code/csharp/Overlay.qll renamed to csharp/ql/lib/semmle/code/csharp/internal/Overlay.qll

@@ -10,13 +10,46 @@
 overlay[local]
 predicate isOverlay() { databaseMetadata("isOverlay", "true") }
 
+overlay[local]
+private string getLocationFilePath(@location_default loc) {
+  exists(@file file | locations_default(loc, file, _, _, _, _) | files(file, result))
+}
+
 /**
  * An abstract base class for all elements that can be discarded from the base.
  */
 overlay[local]
-abstract private class DiscardableEntity extends @locatable {
+private class DiscardableEntity extends @locatable {
   /** Gets the path to the file in which this element occurs. */
-  abstract string getPath();
+  string getFilePath() {
+    exists(@location_default loc | result = getLocationFilePath(loc) |
+      expr_location(this, loc) or
+      stmt_location(this, loc) or
+      using_directive_location(this, loc) or
+      namespace_declaration_location(this, loc) or
+      preprocessor_directive_location(this, loc) or
+      type_location(this, loc) or
+      attribute_location(this, loc) or
+      type_parameter_constraints_location(this, loc) or
+      property_location(this, loc) or
+      indexer_location(this, loc) or
+      accessor_location(this, loc) or
+      event_location(this, loc) or
+      event_accessor_location(this, loc) or
+      operator_location(this, loc) or
+      method_location(this, loc) or
+      constructor_location(this, loc) or
+      destructor_location(this, loc) or
+      field_location(this, loc) or
+      localvar_location(this, loc) or
+      param_location(this, loc) or
+      type_mention_location(this, loc) or
+      commentline_location(this, loc) or
+      commentblock_location(this, loc) or
+      diagnostics(this, _, _, _, _, loc) or
+      extractor_messages(this, _, _, _, _, loc, _)
+    )
+  }
 
   /** Holds if this element exists in the base variant. */
   predicate existsInBase() { not isOverlay() and exists(this) }
@@ -34,74 +67,39 @@ abstract private class DiscardableEntity extends @locatable {
  */
 overlay[local]
 private class StarEntity =
-  @expr or @stmt or @diagnostic or @extractor_message or @using_directive or @type_mention;
+  @expr or @stmt or @diagnostic or @extractor_message or @using_directive or @type_mention or
+      @local_variable;
 
 overlay[discard_entity]
 private predicate discardStarEntity(@locatable e) {
   e instanceof StarEntity and
   // Entities with *-ids can exist either in base or overlay, but not both.
-  exists(DiscardableEntity de | de = e |
-    overlayChangedFiles(de.getPath()) and
-    de.existsInBase()
-  )
+  e =
+    any(DiscardableEntity de |
+      overlayChangedFiles(de.getFilePath()) and
+      de.existsInBase()
+    )
 }
 
 overlay[discard_entity]
 private predicate discardNamedEntity(@locatable e) {
   not e instanceof StarEntity and
   // Entities with named IDs can exist both in base, overlay, or both.
-  exists(DiscardableEntity de | de = e |
-    overlayChangedFiles(de.getPath()) and
-    not de.existsInOverlay()
-  )
-}
-
-overlay[local]
-private string getLocationPath(@location_default loc) {
-  exists(@file file | locations_default(loc, file, _, _, _, _) | files(file, result))
+  e =
+    any(DiscardableEntity de |
+      overlayChangedFiles(de.getFilePath()) and
+      not de.existsInOverlay()
+    )
 }
 
 overlay[local]
 private predicate discardableLocation(@location_default loc, string path) {
   not isOverlay() and
-  path = getLocationPath(loc)
+  path = getLocationFilePath(loc)
 }
 
 // Discard locations that are in changed files from the base variant.
 overlay[discard_entity]
 private predicate discardLocation(@location_default loc) {
   exists(string path | discardableLocation(loc, path) | overlayChangedFiles(path))
 }
-
-overlay[local]
-private class PossibleDiscardableEntity extends DiscardableEntity instanceof @locatable {
-  override string getPath() {
-    exists(@location_default loc | result = getLocationPath(loc) |
-      expr_location(this, loc) or
-      stmt_location(this, loc) or
-      using_directive_location(this, loc) or
-      namespace_declaration_location(this, loc) or
-      preprocessor_directive_location(this, loc) or
-      type_location(this, loc) or
-      attribute_location(this, loc) or
-      type_parameter_constraints_location(this, loc) or
-      property_location(this, loc) or
-      indexer_location(this, loc) or
-      accessor_location(this, loc) or
-      event_location(this, loc) or
-      event_accessor_location(this, loc) or
-      operator_location(this, loc) or
-      method_location(this, loc) or
-      constructor_location(this, loc) or
-      destructor_location(this, loc) or
-      field_location(this, loc) or
-      localvar_location(this, loc) or
-      param_location(this, loc) or
-      type_mention_location(this, loc) or
-      commentline_location(this, loc) or
-      commentblock_location(this, loc) or
-      diagnostics(this, _, _, _, _, loc) or
-      extractor_messages(this, _, _, _, _, loc, _)
-    )
-  }
-}

csharp/ql/test/library-tests/overlay/testlocations.ql

@@ -5,25 +5,22 @@ import csharp
 query predicate elementsWithMultipleSourceLocations(Element e, SourceLocation loc) {
   e.fromSource() and
   not e instanceof Namespace and
-  count(SourceLocation l0 | l0 = e.getALocation()) > 1 and
+  strictcount(SourceLocation l0 | l0 = e.getALocation()) > 1 and
   loc = e.getALocation()
 }
 
 query predicate typeMentionsWithMultipleSourceLocations(TypeMention tm, SourceLocation loc) {
-  tm.getLocation().getFile().fromSource() and
-  count(SourceLocation l0 | l0 = tm.getLocation()) > 1 and
+  strictcount(SourceLocation l0 | l0 = tm.getLocation()) > 1 and
   loc = tm.getLocation()
 }
 
 query predicate commentLinesWithMultipleSourceLocations(CommentLine cl, SourceLocation loc) {
-  cl.getLocation().getFile().fromSource() and
-  count(SourceLocation l0 | l0 = cl.getLocation()) > 1 and
+  strictcount(SourceLocation l0 | l0 = cl.getLocation()) > 1 and
   loc = cl.getLocation()
 }
 
 query predicate commentBlocksWithMultipleSourceLocations(CommentBlock cb, SourceLocation loc) {
-  cb.getLocation().getFile().fromSource() and
-  count(SourceLocation l0 | l0 = cb.getLocation()) > 1 and
+  strictcount(SourceLocation l0 | l0 = cb.getLocation()) > 1 and
   loc = cb.getLocation()
 }