From 10d96ee02f6fa0e84693135ffb830acc35d2cb8e Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Tue, 9 Apr 2024 10:31:48 +0200 Subject: [PATCH] C#: Address review comments. --- .../diag_recursive_generics/Types.ql | 7 +++---- csharp/ql/lib/semmle/code/csharp/Element.qll | 2 ++ .../ql/lib/semmle/code/csharp/dataflow/SSA.qll | 8 +------- .../attributes/AttributeElements.ql | 8 +++----- .../library-tests/constructors/Destructors1.ql | 3 +-- .../test/library-tests/csharp11/fileScoped.ql | 6 +----- .../ql/test/library-tests/csharp11/nativeInt.ql | 6 ++---- .../library-tests/csharp9/covariantReturn.ql | 13 ++++--------- csharp/ql/test/library-tests/csharp9/foreach.ql | 17 ++++++----------- csharp/ql/test/library-tests/csharp9/record.ql | 10 +--------- .../ql/test/library-tests/csharp9/withExpr.ql | 14 +++----------- csharp/ql/test/library-tests/enums/Enums3.ql | 8 +++----- .../ql/test/library-tests/generics/Generics.ql | 12 ++++-------- 13 files changed, 34 insertions(+), 80 deletions(-) diff --git a/csharp/ql/integration-tests/all-platforms/diag_recursive_generics/Types.ql b/csharp/ql/integration-tests/all-platforms/diag_recursive_generics/Types.ql index fff011dcbd53..de95e0fcbe7c 100644 --- a/csharp/ql/integration-tests/all-platforms/diag_recursive_generics/Types.ql +++ b/csharp/ql/integration-tests/all-platforms/diag_recursive_generics/Types.ql @@ -1,6 +1,5 @@ import csharp -import semmle.code.csharp.commons.QualifiedName -from Class c, string qualifier, string name -where c.fromSource() and c.getBaseClass().hasFullyQualifiedName(qualifier, name) -select c, getQualifiedName(qualifier, name) +from Class c +where c.fromSource() +select c, c.getBaseClass().getFullyQualifiedNameDebug() diff --git a/csharp/ql/lib/semmle/code/csharp/Element.qll b/csharp/ql/lib/semmle/code/csharp/Element.qll index f8cb018be681..a48241a14084 100644 --- a/csharp/ql/lib/semmle/code/csharp/Element.qll +++ b/csharp/ql/lib/semmle/code/csharp/Element.qll @@ -145,6 +145,8 @@ class NamedElement extends Element, @named_element { * Unbound generic types, such as `IList`, are represented as * ``System.Collections.Generic.IList`1``. */ + bindingset[this] + pragma[inline_late] final string getFullyQualifiedNameDebug() { exists(string qualifier, string name | this.hasFullyQualifiedName(qualifier, name) | if qualifier = "" then result = name else result = qualifier + "." + name diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll index 9cd33ea260f1..0d79eafdf5c6 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll @@ -3,7 +3,6 @@ */ import csharp -private import semmle.code.csharp.commons.QualifiedName /** * Provides classes for working with static single assignment (SSA) form. @@ -121,12 +120,7 @@ module Ssa { result = prefix + "." + this.getAssignable() | if f.(Modifiable).isStatic() - then - exists(string qualifier, string name | - f.getDeclaringType().hasFullyQualifiedName(qualifier, name) - | - prefix = getQualifiedName(qualifier, name) - ) + then prefix = f.getDeclaringType().getName() else prefix = "this" ) } diff --git a/csharp/ql/test/library-tests/attributes/AttributeElements.ql b/csharp/ql/test/library-tests/attributes/AttributeElements.ql index 17ce2ea3d93e..679d7567ea52 100644 --- a/csharp/ql/test/library-tests/attributes/AttributeElements.ql +++ b/csharp/ql/test/library-tests/attributes/AttributeElements.ql @@ -1,9 +1,7 @@ import csharp -import semmle.code.csharp.commons.QualifiedName -from Attributable element, Attribute attribute, string qualifier, string name +from Attributable element, Attribute attribute where attribute = element.getAnAttribute() and - (attribute.fromSource() or element.(Assembly).getName() in ["attributes", "Assembly1"]) and - attribute.getType().hasFullyQualifiedName(qualifier, name) -select element, attribute, getQualifiedName(qualifier, name) + (attribute.fromSource() or element.(Assembly).getName() in ["attributes", "Assembly1"]) +select element, attribute, attribute.getType().getFullyQualifiedNameDebug() diff --git a/csharp/ql/test/library-tests/constructors/Destructors1.ql b/csharp/ql/test/library-tests/constructors/Destructors1.ql index 673a0e941f08..792d50da7bbc 100644 --- a/csharp/ql/test/library-tests/constructors/Destructors1.ql +++ b/csharp/ql/test/library-tests/constructors/Destructors1.ql @@ -3,11 +3,10 @@ */ import csharp -import semmle.code.csharp.commons.QualifiedName from Destructor c, string qualifier, string name where c.getDeclaringType().hasFullyQualifiedName(qualifier, name) and qualifier = "Constructors" and name = "Class" -select c, getQualifiedName(qualifier, name) +select c, c.getDeclaringType().getFullyQualifiedNameDebug() diff --git a/csharp/ql/test/library-tests/csharp11/fileScoped.ql b/csharp/ql/test/library-tests/csharp11/fileScoped.ql index 3003fc801bf7..33697255896a 100644 --- a/csharp/ql/test/library-tests/csharp11/fileScoped.ql +++ b/csharp/ql/test/library-tests/csharp11/fileScoped.ql @@ -1,5 +1,4 @@ import csharp -private import semmle.code.csharp.commons.QualifiedName private predicate isInteresting(Type t) { ( @@ -20,10 +19,7 @@ query predicate typemodifiers(Type t, string modifier) { query predicate qualifiedtypes(Type t, string qualifiedName) { isInteresting(t) and - exists(string qualifier, string name | - t.hasFullyQualifiedName(qualifier, name) and - qualifiedName = getQualifiedName(qualifier, name) - ) + qualifiedName = t.getFullyQualifiedNameDebug() } query predicate filetypes(Type t) { diff --git a/csharp/ql/test/library-tests/csharp11/nativeInt.ql b/csharp/ql/test/library-tests/csharp11/nativeInt.ql index 80d7974de56c..adbc062baf6d 100644 --- a/csharp/ql/test/library-tests/csharp11/nativeInt.ql +++ b/csharp/ql/test/library-tests/csharp11/nativeInt.ql @@ -1,12 +1,10 @@ import csharp -import semmle.code.csharp.commons.QualifiedName -from LocalVariable v1, LocalVariable v2, Type t, string qualifier, string name +from LocalVariable v1, LocalVariable v2, Type t where v1.getFile().getStem() = "NativeInt" and v2.getFile().getStem() = "NativeInt" and t = v1.getType() and t = v2.getType() and - t.hasFullyQualifiedName(qualifier, name) and v1 != v2 -select v1, v2, getQualifiedName(qualifier, name) +select v1, v2, t.getFullyQualifiedNameDebug() diff --git a/csharp/ql/test/library-tests/csharp9/covariantReturn.ql b/csharp/ql/test/library-tests/csharp9/covariantReturn.ql index 6227ed18d6de..b4bab047322b 100644 --- a/csharp/ql/test/library-tests/csharp9/covariantReturn.ql +++ b/csharp/ql/test/library-tests/csharp9/covariantReturn.ql @@ -1,13 +1,8 @@ import csharp -import semmle.code.csharp.commons.QualifiedName -from - Method m, Method overrider, string mnamespace, string mtype, string mname, string onamespace, - string otype, string oname +from Method m, Method overrider where m.getAnOverrider() = overrider and - m.getFile().getStem() = "CovariantReturn" and - m.hasFullyQualifiedName(mnamespace, mtype, mname) and - overrider.hasFullyQualifiedName(onamespace, otype, oname) -select getQualifiedName(mnamespace, mtype, mname), m.getReturnType().toString(), - getQualifiedName(onamespace, otype, oname), overrider.getReturnType().toString() + m.getFile().getStem() = "CovariantReturn" +select m.getFullyQualifiedNameDebug(), m.getReturnType().toString(), + overrider.getFullyQualifiedNameDebug(), overrider.getReturnType().toString() diff --git a/csharp/ql/test/library-tests/csharp9/foreach.ql b/csharp/ql/test/library-tests/csharp9/foreach.ql index cf6fcf2514a1..343ecc556ab8 100644 --- a/csharp/ql/test/library-tests/csharp9/foreach.ql +++ b/csharp/ql/test/library-tests/csharp9/foreach.ql @@ -1,5 +1,4 @@ import csharp -import semmle.code.csharp.commons.QualifiedName private string getLocation(Member m) { if m.fromSource() then result = m.getALocation().(SourceLocation).toString() else result = "-" @@ -9,13 +8,9 @@ private string getIsAsync(ForeachStmt f) { if f.isAsync() then result = "async" else result = "sync" } -from - ForeachStmt f, string qualifier1, string type1, string qualifier2, string type2, - string qualifier3, string type3 -where - f.getGetEnumerator().getDeclaringType().hasFullyQualifiedName(qualifier1, type1) and - f.getCurrent().getDeclaringType().hasFullyQualifiedName(qualifier2, type2) and - f.getMoveNext().getDeclaringType().hasFullyQualifiedName(qualifier3, type3) -select f, f.getElementType().toString(), getIsAsync(f), getQualifiedName(qualifier1, type1), - getLocation(f.getGetEnumerator()), getQualifiedName(qualifier2, type2), - getLocation(f.getCurrent()), getQualifiedName(qualifier3, type3), getLocation(f.getMoveNext()) +from ForeachStmt f +select f, f.getElementType().toString(), getIsAsync(f), + f.getGetEnumerator().getDeclaringType().getFullyQualifiedNameDebug(), + getLocation(f.getGetEnumerator()), f.getCurrent().getDeclaringType().getFullyQualifiedNameDebug(), + getLocation(f.getCurrent()), f.getMoveNext().getDeclaringType().getFullyQualifiedNameDebug(), + getLocation(f.getMoveNext()) diff --git a/csharp/ql/test/library-tests/csharp9/record.ql b/csharp/ql/test/library-tests/csharp9/record.ql index a2a9a9c0786b..58cf579bac6b 100644 --- a/csharp/ql/test/library-tests/csharp9/record.ql +++ b/csharp/ql/test/library-tests/csharp9/record.ql @@ -7,18 +7,10 @@ query predicate records(RecordClass t, string i, RecordCloneMethod clone) { t.fromSource() } -private string getMemberName(Member m) { - exists(string qualifier, string name | - m.getDeclaringType().hasFullyQualifiedName(qualifier, name) - | - result = getQualifiedName(qualifier, name) + "." + m.toStringWithTypes() - ) -} - query predicate members(RecordClass t, string ms, string l) { t.fromSource() and exists(Member m | t.hasMember(m) | - ms = getMemberName(m) and + ms = getFullyQualifiedNameWithTypes(m) and if m.fromSource() then l = m.getLocation().toString() else l = "no location" ) } diff --git a/csharp/ql/test/library-tests/csharp9/withExpr.ql b/csharp/ql/test/library-tests/csharp9/withExpr.ql index 6683d7c54f67..564cbe529aa1 100644 --- a/csharp/ql/test/library-tests/csharp9/withExpr.ql +++ b/csharp/ql/test/library-tests/csharp9/withExpr.ql @@ -1,19 +1,11 @@ import csharp import semmle.code.csharp.commons.QualifiedName -private string getSignature(Method m) { - exists(string qualifier, string name | - m.getDeclaringType().hasFullyQualifiedName(qualifier, name) - | - result = getQualifiedName(qualifier, name) + "." + m.toStringWithTypes() - ) -} - query predicate withExpr(WithExpr with, string type, Expr expr, ObjectInitializer init, string clone) { type = with.getType().toStringWithTypes() and expr = with.getExpr() and init = with.getInitializer() and - clone = getSignature(with.getCloneMethod()) + clone = getFullyQualifiedNameWithTypes(with.getCloneMethod()) } query predicate withTarget(WithExpr with, RecordCloneMethod clone, Constructor ctor) { @@ -25,7 +17,7 @@ query predicate cloneOverrides(string b, string o) { exists(RecordCloneMethod base, RecordCloneMethod overrider | base.getDeclaringType().fromSource() and base.getAnOverrider() = overrider and - b = getSignature(base) and - o = getSignature(overrider) + b = getFullyQualifiedNameWithTypes(base) and + o = getFullyQualifiedNameWithTypes(overrider) ) } diff --git a/csharp/ql/test/library-tests/enums/Enums3.ql b/csharp/ql/test/library-tests/enums/Enums3.ql index 5cfbdc561932..b01ffb97e184 100644 --- a/csharp/ql/test/library-tests/enums/Enums3.ql +++ b/csharp/ql/test/library-tests/enums/Enums3.ql @@ -3,13 +3,11 @@ */ import csharp -import semmle.code.csharp.commons.QualifiedName -from EnumConstant c, string namespace, string name +from EnumConstant c where c.getName() = "Green" and c.getDeclaringType().hasFullyQualifiedName("Enums", "LongColor") and c.getType() = c.getDeclaringType() and - c.getValue() = "1" and - c.getDeclaringType().getBaseClass().hasFullyQualifiedName(namespace, name) -select c, getQualifiedName(namespace, name) + c.getValue() = "1" +select c, c.getDeclaringType().getBaseClass().getFullyQualifiedNameDebug() diff --git a/csharp/ql/test/library-tests/generics/Generics.ql b/csharp/ql/test/library-tests/generics/Generics.ql index 71e21a5815e0..e75f1fdc9088 100644 --- a/csharp/ql/test/library-tests/generics/Generics.ql +++ b/csharp/ql/test/library-tests/generics/Generics.ql @@ -268,16 +268,12 @@ query predicate test33(ConstructedMethod cm, string s1, string s2) { query predicate test34(UnboundGeneric ug, string s1, string s2) { ug.fromSource() and - exists(string qualifier, string name | - ug.hasFullyQualifiedName(qualifier, name) and s1 = getQualifiedName(qualifier, name) - ) and - getFullyQualifiedNameWithTypes(ug) = s2 + s1 = ug.getFullyQualifiedNameDebug() and + s2 = getFullyQualifiedNameWithTypes(ug) } query predicate test35(UnboundGenericMethod gm, string s1, string s2) { gm.fromSource() and - exists(string namespace, string type, string name | - gm.hasFullyQualifiedName(namespace, type, name) and s1 = getQualifiedName(namespace, type, name) - ) and - getFullyQualifiedNameWithTypes(gm) = s2 + s1 = gm.getFullyQualifiedNameDebug() and + s2 = getFullyQualifiedNameWithTypes(gm) }