C#: Address review comments.

michaelnebel · michaelnebel · commit 29ccac8e93e3 · 2022-12-06T12:05:48.000+01:00
diff --git a/csharp/ql/lib/semmle/code/cil/ConsistencyChecks.qll b/csharp/ql/lib/semmle/code/cil/ConsistencyChecks.qll
@@ -489,7 +489,7 @@ class InvalidOverride extends MethodViolation {
       base.getDeclaringType().hasQualifiedName(namespace, type)
     |
       result =
-        "Overridden method from " + printQualifiedName(namespace, type) + " is not in a base type"
+        "Overridden method from " + getQualifiedName(namespace, type) + " is not in a base type"
     )
   }
 }
diff --git a/csharp/ql/lib/semmle/code/cil/Type.qll b/csharp/ql/lib/semmle/code/cil/Type.qll
@@ -54,7 +54,7 @@ class Type extends DotNet::Type, Declaration, TypeContainer, @cil_type {
   override predicate hasQualifiedName(string namespace, string name) {
     name = this.getName() and
     exists(string pnamespace, string pname | this.getParent().hasQualifiedName(pnamespace, pname) |
-      namespace = printQualifiedName(pnamespace, pname)
+      namespace = getQualifiedName(pnamespace, pname)
     )
   }
 
diff --git a/csharp/ql/lib/semmle/code/csharp/Callable.qll b/csharp/ql/lib/semmle/code/csharp/Callable.qll
@@ -1004,7 +1004,7 @@ class LocalFunction extends Callable, Modifiable, Attributable, @local_function
   override predicate hasQualifiedName(string namespace, string name) {
     exists(string cnamespace, string type |
       this.getEnclosingCallable().hasQualifiedName(cnamespace, type) and
-      namespace = printQualifiedName(cnamespace, type)
+      namespace = getQualifiedName(cnamespace, type)
     ) and
     name = this.getName()
   }
diff --git a/csharp/ql/lib/semmle/code/csharp/Generics.qll b/csharp/ql/lib/semmle/code/csharp/Generics.qll
@@ -102,7 +102,7 @@ bindingset[t]
 private string getFullName(Type t) {
   exists(string namespace, string name |
     t.hasQualifiedName(namespace, name) and
-    result = printQualifiedName(namespace, name)
+    result = getQualifiedName(namespace, name)
   )
 }
 
diff --git a/csharp/ql/lib/semmle/code/csharp/commons/QualifiedName.qll b/csharp/ql/lib/semmle/code/csharp/commons/QualifiedName.qll
@@ -1,34 +1,34 @@
 /**
- * Provides predicates related to C# qualified name.
+ * Provides predicates related to C# qualified names.
  */
 
 /**
- * Returns the concatenation of `qualifier` and `name`, separated by a dot.
+ * Returns the concatenation of `namespace` and `name`, separated by a dot.
  */
 bindingset[namespace, name]
-string printQualifiedName(string namespace, string name) {
+string getQualifiedName(string namespace, string name) {
   if namespace = "" then result = name else result = namespace + "." + name
 }
 
 /**
- * Returns the concatenation of `qualifier`, `type` and `name`, separated by a dot.
+ * Returns the concatenation of `namespace`, `type` and `name`, separated by a dot.
  */
 bindingset[namespace, type, name]
-string printQualifiedName(string namespace, string type, string name) {
-  result = printQualifiedName(namespace, type) + "." + name
+string getQualifiedName(string namespace, string type, string name) {
+  result = getQualifiedName(namespace, type) + "." + name
 }
 
-private string getNameSplitter() { result = "(.*)\\.([^\\.]+)$" }
-
 /**
+ * Holds if `qualifiedName` is the concatenation of `qualifier` and `name`, separated by a dot.
  */
 bindingset[qualifiedName]
 predicate splitQualifiedName(string qualifiedName, string qualifier, string name) {
-  if qualifiedName.regexpMatch(getNameSplitter())
-  then
-    qualifier = qualifiedName.regexpCapture(getNameSplitter(), 1) and
-    name = qualifiedName.regexpCapture(getNameSplitter(), 2)
-  else (
-    qualifier = "" and name = qualifiedName
+  exists(string nameSplitter | nameSplitter = "(.*)\\.([^\\.]+)$" |
+    qualifier = qualifiedName.regexpCapture(nameSplitter, 1) and
+    name = qualifiedName.regexpCapture(nameSplitter, 2)
+    or
+    not qualifiedName.regexpMatch(nameSplitter) and
+    qualifier = "" and
+    name = qualifiedName
   )
 }
diff --git a/csharp/ql/lib/semmle/code/csharp/security/dataflow/ExternalAPIsQuery.qll b/csharp/ql/lib/semmle/code/csharp/security/dataflow/ExternalAPIsQuery.qll
@@ -83,7 +83,7 @@ class ExternalApiDataNode extends DataFlow::Node {
    */
   deprecated string getCallableDescription() {
     exists(string namespace, string name |
-      this.hasQualifiedName(namespace, name) and result = printQualifiedName(namespace, name)
+      this.hasQualifiedName(namespace, name) and result = getQualifiedName(namespace, name)
     )
   }
 }
diff --git a/csharp/ql/lib/semmle/code/dotnet/Declaration.qll b/csharp/ql/lib/semmle/code/dotnet/Declaration.qll
@@ -11,7 +11,7 @@ class Declaration extends NamedElement, @dotnet_declaration {
   override predicate hasQualifiedName(string namespace, string name) {
     exists(string dnamespace, string dname |
       this.getDeclaringType().hasQualifiedName(dnamespace, dname) and
-      namespace = printQualifiedName(dnamespace, dname)
+      namespace = getQualifiedName(dnamespace, dname)
     ) and
     name = this.getName()
   }
diff --git a/csharp/ql/lib/semmle/code/dotnet/Namespace.qll b/csharp/ql/lib/semmle/code/dotnet/Namespace.qll
@@ -28,7 +28,7 @@ class Namespace extends Declaration, @namespace {
   override predicate hasQualifiedName(string namespace, string name) {
     exists(string pnamespace, string pname |
       this.getParentNamespace().hasQualifiedName(pnamespace, pname) and
-      namespace = printQualifiedName(pnamespace, pname)
+      namespace = getQualifiedName(pnamespace, pname)
     ) and
     name = this.getName()
   }
@@ -52,7 +52,7 @@ class Namespace extends Declaration, @namespace {
   string getFullName() {
     exists(string namespace, string name |
       this.hasQualifiedName(namespace, name) and
-      result = printQualifiedName(namespace, name)
+      result = getQualifiedName(namespace, name)
     )
   }
 }
diff --git a/csharp/ql/src/API Abuse/NonOverridingMethod.ql b/csharp/ql/src/API Abuse/NonOverridingMethod.ql
@@ -43,4 +43,4 @@ where
   nonOverridingMethod(m, vm) and
   vm.hasQualifiedName(namespace, type, name)
 select m, "Method '" + m.getName() + "' looks like it should override $@ but does not do so.",
-  vm.getUnboundDeclaration(), printQualifiedName(namespace, type, name)
+  vm.getUnboundDeclaration(), getQualifiedName(namespace, type, name)
diff --git a/csharp/ql/src/Security Features/CWE-020/UntrustedDataToExternalAPI.ql b/csharp/ql/src/Security Features/CWE-020/UntrustedDataToExternalAPI.ql
@@ -21,5 +21,5 @@ where
   config.hasFlowPath(source, sink) and
   sink.getNode().(ExternalApiDataNode).hasQualifiedName(namespace, name)
 select sink, source, sink,
-  "Call to " + printQualifiedName(namespace, name) + " with untrusted data from $@.", source,
+  "Call to " + getQualifiedName(namespace, name) + " with untrusted data from $@.", source,
   source.toString()
diff --git a/csharp/ql/src/Stubs/Stubs.qll b/csharp/ql/src/Stubs/Stubs.qll
@@ -124,7 +124,7 @@ abstract private class GeneratedType extends Type, GeneratedElement {
     exists(string namespace, string name |
       this.hasQualifiedName(namespace, name) and
       result =
-        "// Generated from `" + printQualifiedName(namespace, name) + "` in `" +
+        "// Generated from `" + getQualifiedName(namespace, name) + "` in `" +
           concat(this.getALocation().toString(), "; ") + "`\n"
     )
   }
diff --git a/csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/delegated-security-validations-always-return-true.ql b/csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/delegated-security-validations-always-return-true.ql
@@ -22,4 +22,4 @@ from
 where e = p.getAnAssignedValue() and p.hasQualifiedName(namespace, name)
 select e,
   "JsonWebTokenHandler security-sensitive property $@ is being delegated to this callable that always returns \"true\".",
-  p, printQualifiedName(namespace, name)
+  p, getQualifiedName(namespace, name)
diff --git a/csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/security-validation-disabled.ql b/csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/security-validation-disabled.ql
@@ -23,4 +23,4 @@ where
   sink.asExpr() = pw.getAnAssignedValue() and
   pw.hasQualifiedName(namespace, name)
 select sink, "The security sensitive property $@ is being disabled by the following value: $@.", pw,
-  printQualifiedName(namespace, name), source, "false"
+  getQualifiedName(namespace, name), source, "false"
diff --git a/csharp/ql/test/library-tests/attributes/AttributeElements.ql b/csharp/ql/test/library-tests/attributes/AttributeElements.ql
@@ -6,4 +6,4 @@ where
   attribute = element.getAnAttribute() and
   (attribute.fromSource() or element.(Assembly).getName() in ["attributes", "Assembly1"]) and
   attribute.getType().hasQualifiedName(namespace, name)
-select element, attribute, printQualifiedName(namespace, name)
+select element, attribute, getQualifiedName(namespace, name)
diff --git a/csharp/ql/test/library-tests/cil/consistency/Handles.ql b/csharp/ql/test/library-tests/cil/consistency/Handles.ql
@@ -15,7 +15,7 @@ query predicate tooManyHandles(string s) {
   exists(MetadataEntity e, Assembly a, string namespace, string name |
     strictcount(int handle | metadata_handle(e, a, handle)) > 1 and
     e.hasQualifiedName(namespace, name) and
-    s = printQualifiedName(namespace, name)
+    s = getQualifiedName(namespace, name)
   )
 }
 
@@ -35,7 +35,7 @@ query predicate tooManyMatchingHandles(string s) {
     metadata_handle(e, a, handle) and
     strictcount(UniqueMetadataEntity e2 | metadata_handle(e2, a, handle)) > 2 and
     e.hasQualifiedName(namespace, name) and
-    s = printQualifiedName(namespace, name)
+    s = getQualifiedName(namespace, name)
   )
 }
 
diff --git a/csharp/ql/test/library-tests/cil/dataflow/TrivialProperties.ql b/csharp/ql/test/library-tests/cil/dataflow/TrivialProperties.ql
@@ -12,4 +12,4 @@ where
     namespace = "Dataflow" and type = "Properties"
   ) and
   prop.hasQualifiedName(namespace, type, name)
-select printQualifiedName(namespace, type, name)
+select getQualifiedName(namespace, type, name)
diff --git a/csharp/ql/test/library-tests/cil/enums/enums.ql b/csharp/ql/test/library-tests/cil/enums/enums.ql
@@ -5,4 +5,4 @@ from Enum e, string namespace, string name
 where
   e.hasQualifiedName(namespace, name) and
   not (namespace = "Interop.Sys" and name = "LockType") // doesn't exist on osx
-select printQualifiedName(namespace, name), e.getUnderlyingType().toStringWithTypes()
+select getQualifiedName(namespace, name), e.getUnderlyingType().toStringWithTypes()
diff --git a/csharp/ql/test/library-tests/cil/functionPointers/functionPointers.ql b/csharp/ql/test/library-tests/cil/functionPointers/functionPointers.ql
@@ -32,7 +32,7 @@ query predicate modifiers(string fnptr, string modifier, string sKind) {
   |
     cil_custom_modifiers(fn, modType, kind) and
     modType.hasQualifiedName(namespace, name) and
-    modifier = printQualifiedName(namespace, name) and
+    modifier = getQualifiedName(namespace, name) and
     sKind = getKind(kind)
   )
 }
diff --git a/csharp/ql/test/library-tests/cil/init-only-prop/customModifiers.ql b/csharp/ql/test/library-tests/cil/init-only-prop/customModifiers.ql
@@ -10,6 +10,6 @@ where
     receiver = cmr.toString() and
     cil_custom_modifiers(cmr, modType, kind) and
     modType.hasQualifiedName(namespace, name) and
-    modifier = printQualifiedName(namespace, name)
+    modifier = getQualifiedName(namespace, name)
   )
 select receiver, modifier, getKind(kind)
diff --git a/csharp/ql/test/library-tests/cil/typeAnnotations/typeAnnotations.ql b/csharp/ql/test/library-tests/cil/typeAnnotations/typeAnnotations.ql
@@ -4,7 +4,7 @@ import semmle.code.cil.Type
 
 private string elementType(Element e, string toString) {
   exists(string namespace, string type, string name |
-    toString = printQualifiedName(namespace, type, name)
+    toString = getQualifiedName(namespace, type, name)
   |
     e.(Method).hasQualifiedName(namespace, type, name) and result = "method"
     or
@@ -16,7 +16,7 @@ private string elementType(Element e, string toString) {
       exists(string namespace, string name |
         p.getDeclaringElement().hasQualifiedName(namespace, name)
       |
-        toString = "Parameter " + p.getIndex() + " of " + printQualifiedName(namespace, name)
+        toString = "Parameter " + p.getIndex() + " of " + getQualifiedName(namespace, name)
       )
     ) and
   result = "parameter"
@@ -27,14 +27,13 @@ private string elementType(Element e, string toString) {
         v.getImplementation().getMethod().hasQualifiedName(namespace, type, name)
       |
         toString =
-          "Local variable " + v.getIndex() + " of method " +
-            printQualifiedName(namespace, type, name)
+          "Local variable " + v.getIndex() + " of method " + getQualifiedName(namespace, type, name)
       )
     ) and
   result = "local"
   or
   exists(string namespace, string name | e.(FunctionPointerType).hasQualifiedName(namespace, name) |
-    toString = printQualifiedName(namespace, name)
+    toString = getQualifiedName(namespace, name)
   ) and
   result = "fnptr"
   or
diff --git a/csharp/ql/test/library-tests/commons/Disposal/DisposedFields.ql b/csharp/ql/test/library-tests/commons/Disposal/DisposedFields.ql
@@ -7,4 +7,4 @@ where
   mayBeDisposed(field) and
   field.getDeclaringType().hasQualifiedName("DisposalTests", "Class1") and
   field.hasQualifiedName(namespace, name)
-select printQualifiedName(namespace, name)
+select getQualifiedName(namespace, name)
diff --git a/csharp/ql/test/library-tests/csharp9/covariantReturn.ql b/csharp/ql/test/library-tests/csharp9/covariantReturn.ql
@@ -9,5 +9,5 @@ where
   m.getFile().getStem() = "CovariantReturn" and
   m.hasQualifiedName(mnamespace, mtype, mname) and
   overrider.hasQualifiedName(onamespace, otype, oname)
-select printQualifiedName(mnamespace, mtype, mname), m.getReturnType().toString(),
-  printQualifiedName(onamespace, otype, oname), overrider.getReturnType().toString()
+select getQualifiedName(mnamespace, mtype, mname), m.getReturnType().toString(),
+  getQualifiedName(onamespace, otype, oname), overrider.getReturnType().toString()
diff --git a/csharp/ql/test/library-tests/csharp9/foreach.ql b/csharp/ql/test/library-tests/csharp9/foreach.ql
@@ -16,6 +16,6 @@ where
   f.getGetEnumerator().getDeclaringType().hasQualifiedName(namespace1, type1) and
   f.getCurrent().getDeclaringType().hasQualifiedName(namespace2, type2) and
   f.getMoveNext().getDeclaringType().hasQualifiedName(namespace3, type3)
-select f, f.getElementType().toString(), getIsAsync(f), printQualifiedName(namespace1, type1),
-  getLocation(f.getGetEnumerator()), printQualifiedName(namespace2, type2),
-  getLocation(f.getCurrent()), printQualifiedName(namespace3, type3), getLocation(f.getMoveNext())
+select f, f.getElementType().toString(), getIsAsync(f), getQualifiedName(namespace1, type1),
+  getLocation(f.getGetEnumerator()), getQualifiedName(namespace2, type2),
+  getLocation(f.getCurrent()), getQualifiedName(namespace3, type3), getLocation(f.getMoveNext())
diff --git a/csharp/ql/test/library-tests/csharp9/record.ql b/csharp/ql/test/library-tests/csharp9/record.ql
@@ -9,7 +9,7 @@ query predicate records(RecordClass t, string i, RecordCloneMethod clone) {
 
 private string getMemberName(Member m) {
   exists(string namespace, string name | m.getDeclaringType().hasQualifiedName(namespace, name) |
-    result = printQualifiedName(namespace, name) + "." + m.toStringWithTypes()
+    result = getQualifiedName(namespace, name) + "." + m.toStringWithTypes()
   )
 }
 
diff --git a/csharp/ql/test/library-tests/csharp9/withExpr.ql b/csharp/ql/test/library-tests/csharp9/withExpr.ql
@@ -3,7 +3,7 @@ import semmle.code.csharp.commons.QualifiedName
 
 private string getSignature(Method m) {
   exists(string namespace, string name | m.getDeclaringType().hasQualifiedName(namespace, name) |
-    result = printQualifiedName(namespace, name) + "." + m.toStringWithTypes()
+    result = getQualifiedName(namespace, name) + "." + m.toStringWithTypes()
   )
 }
 
diff --git a/csharp/ql/test/library-tests/enums/Enums3.ql b/csharp/ql/test/library-tests/enums/Enums3.ql
@@ -12,4 +12,4 @@ where
   c.getType() = c.getDeclaringType() and
   c.getValue() = "1" and
   c.getDeclaringType().getBaseClass().hasQualifiedName(namespace, name)
-select c, printQualifiedName(namespace, name)
+select c, getQualifiedName(namespace, name)
diff --git a/csharp/ql/test/library-tests/generics/Generics.ql b/csharp/ql/test/library-tests/generics/Generics.ql
@@ -261,23 +261,23 @@ query predicate test32(ConstructedGeneric cg, string s1, string s2) {
 query predicate test33(ConstructedMethod cm, string s1, string s2) {
   cm.fromSource() and
   exists(string namespace, string type, string name |
-    cm.hasQualifiedName(namespace, type, name) and s1 = printQualifiedName(namespace, type, name)
+    cm.hasQualifiedName(namespace, type, name) and s1 = getQualifiedName(namespace, type, name)
   ) and
   cm.getQualifiedNameWithTypes() = s2
 }
 
 query predicate test34(UnboundGeneric ug, string s1, string s2) {
   ug.fromSource() and
   exists(string namespace, string name |
-    ug.hasQualifiedName(namespace, name) and s1 = printQualifiedName(namespace, name)
+    ug.hasQualifiedName(namespace, name) and s1 = getQualifiedName(namespace, name)
   ) and
   ug.getQualifiedNameWithTypes() = s2
 }
 
 query predicate test35(UnboundGenericMethod gm, string s1, string s2) {
   gm.fromSource() and
   exists(string namespace, string type, string name |
-    gm.hasQualifiedName(namespace, type, name) and s1 = printQualifiedName(namespace, type, name)
+    gm.hasQualifiedName(namespace, type, name) and s1 = getQualifiedName(namespace, type, name)
   ) and
   gm.getQualifiedNameWithTypes() = s2
 }

Original file line number	Diff line number	Diff line change
`@@ -489,7 +489,7 @@ class InvalidOverride extends MethodViolation {`
`489`	`489`	`base.getDeclaringType().hasQualifiedName(namespace, type)`
`490`	`490`	`\|`
`491`	`491`	`result =`
`492`		`- "Overridden method from " + printQualifiedName(namespace, type) + " is not in a base type"`
	`492`	`+ "Overridden method from " + getQualifiedName(namespace, type) + " is not in a base type"`
`493`	`493`	`)`
`494`	`494`	`}`
`495`	`495`	`}`
Original file line number	Diff line number	Diff line change
`@@ -54,7 +54,7 @@ class Type extends DotNet::Type, Declaration, TypeContainer, @cil_type {`
`54`	`54`	`override predicate hasQualifiedName(string namespace, string name) {`
`55`	`55`	`name = this.getName() and`
`56`	`56`	`exists(string pnamespace, string pname \| this.getParent().hasQualifiedName(pnamespace, pname) \|`
`57`		`- namespace = printQualifiedName(pnamespace, pname)`
	`57`	`+ namespace = getQualifiedName(pnamespace, pname)`
`58`	`58`	`)`
`59`	`59`	`}`
`60`	`60`
Original file line number	Diff line number	Diff line change
`@@ -1004,7 +1004,7 @@ class LocalFunction extends Callable, Modifiable, Attributable, @local_function`
`1004`	`1004`	`override predicate hasQualifiedName(string namespace, string name) {`
`1005`	`1005`	`exists(string cnamespace, string type \|`
`1006`	`1006`	`this.getEnclosingCallable().hasQualifiedName(cnamespace, type) and`
`1007`		`- namespace = printQualifiedName(cnamespace, type)`
	`1007`	`+ namespace = getQualifiedName(cnamespace, type)`
`1008`	`1008`	`) and`
`1009`	`1009`	`name = this.getName()`
`1010`	`1010`	`}`
Original file line number	Diff line number	Diff line change
`@@ -102,7 +102,7 @@ bindingset[t]`
`102`	`102`	`private string getFullName(Type t) {`
`103`	`103`	`exists(string namespace, string name \|`
`104`	`104`	`t.hasQualifiedName(namespace, name) and`
`105`		`- result = printQualifiedName(namespace, name)`
	`105`	`+ result = getQualifiedName(namespace, name)`
`106`	`106`	`)`
`107`	`107`	`}`
`108`	`108`
Original file line number	Diff line number	Diff line change
`@@ -83,7 +83,7 @@ class ExternalApiDataNode extends DataFlow::Node {`
`83`	`83`	`*/`
`84`	`84`	`deprecated string getCallableDescription() {`
`85`	`85`	`exists(string namespace, string name \|`
`86`		`- this.hasQualifiedName(namespace, name) and result = printQualifiedName(namespace, name)`
	`86`	`+ this.hasQualifiedName(namespace, name) and result = getQualifiedName(namespace, name)`
`87`	`87`	`)`
`88`	`88`	`}`
`89`	`89`	`}`
Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@ class Declaration extends NamedElement, @dotnet_declaration {`
`11`	`11`	`override predicate hasQualifiedName(string namespace, string name) {`
`12`	`12`	`exists(string dnamespace, string dname \|`
`13`	`13`	`this.getDeclaringType().hasQualifiedName(dnamespace, dname) and`
`14`		`- namespace = printQualifiedName(dnamespace, dname)`
	`14`	`+ namespace = getQualifiedName(dnamespace, dname)`
`15`	`15`	`) and`
`16`	`16`	`name = this.getName()`
`17`	`17`	`}`
Original file line number	Diff line number	Diff line change
`@@ -28,7 +28,7 @@ class Namespace extends Declaration, @namespace {`
`28`	`28`	`override predicate hasQualifiedName(string namespace, string name) {`
`29`	`29`	`exists(string pnamespace, string pname \|`
`30`	`30`	`this.getParentNamespace().hasQualifiedName(pnamespace, pname) and`
`31`		`- namespace = printQualifiedName(pnamespace, pname)`
	`31`	`+ namespace = getQualifiedName(pnamespace, pname)`
`32`	`32`	`) and`
`33`	`33`	`name = this.getName()`
`34`	`34`	`}`
`@@ -52,7 +52,7 @@ class Namespace extends Declaration, @namespace {`
`52`	`52`	`string getFullName() {`
`53`	`53`	`exists(string namespace, string name \|`
`54`	`54`	`this.hasQualifiedName(namespace, name) and`
`55`		`- result = printQualifiedName(namespace, name)`
	`55`	`+ result = getQualifiedName(namespace, name)`
`56`	`56`	`)`
`57`	`57`	`}`
`58`	`58`	`}`