C#: Autoformat QL queries

csharp/ql/src/API Abuse/CallToGCCollect.ql

@@ -8,12 +8,13 @@
  * @tags efficiency
  *       maintainability
  */
+
 import csharp
 
 from MethodCall c, Method gcCollect
 where
-  c.getTarget() = gcCollect
+  c.getTarget() = gcCollect and
-  and gcCollect.hasName("Collect")
+  gcCollect.hasName("Collect") and
-  and gcCollect.hasNoParameters()
+  gcCollect.hasNoParameters() and
-  and gcCollect.getDeclaringType().hasQualifiedName("System.GC")
+  gcCollect.getDeclaringType().hasQualifiedName("System.GC")
 select c, "Call to 'GC.Collect()'."

csharp/ql/src/API Abuse/CallToObsoleteMethod.ql

@@ -14,13 +14,12 @@
 import csharp
 
 class ObsoleteAttribute extends Attribute {
-  ObsoleteAttribute() {
+  ObsoleteAttribute() { this.getType().hasQualifiedName("System", "ObsoleteAttribute") }
-    this.getType().hasQualifiedName("System", "ObsoleteAttribute")
-  }
 }
 
 from MethodCall c, Method m
-where m = c.getTarget()
+where
-and m.getAnAttribute() instanceof ObsoleteAttribute
+  m = c.getTarget() and
-and not c.getEnclosingCallable().(Attributable).getAnAttribute() instanceof ObsoleteAttribute
+  m.getAnAttribute() instanceof ObsoleteAttribute and
+  not c.getEnclosingCallable().(Attributable).getAnAttribute() instanceof ObsoleteAttribute
 select c, "Call to obsolete method $@.", m, m.getName()

csharp/ql/src/API Abuse/ClassDoesNotImplementEquals.ql

@@ -16,28 +16,30 @@ import semmle.code.csharp.frameworks.System
 
 from Class c, Element item, string message, string itemText
 where
-  c.isSourceDeclaration()
+  c.isSourceDeclaration() and
-  and not implementsEquals(c)
+  not implementsEquals(c) and
-  and not c.isAbstract()
+  not c.isAbstract() and
-  and
   (
     exists(MethodCall callToEquals |
-      callToEquals.getTarget() instanceof EqualsMethod
+      callToEquals.getTarget() instanceof EqualsMethod and
-      and callToEquals.getQualifier().getType() = c
+      callToEquals.getQualifier().getType() = c and
-      and message = "but it is called $@"
+      message = "but it is called $@" and
-      and item = callToEquals
+      item = callToEquals and
-      and itemText = "here" )
+      itemText = "here"
+    )
     or
-    ( item = c.getAnOperator().(EQOperator)
+    (
-      and message = "but it implements $@"
+      item = c.getAnOperator().(EQOperator) and
-      and itemText = "operator ==" )
+      message = "but it implements $@" and
+      itemText = "operator =="
+    )
     or
-    exists(IEquatableEqualsMethod eq | item = eq
+    exists(IEquatableEqualsMethod eq |
-      and eq = c.getAMethod()
+      item = eq and
-      and message = "but it implements $@"
+      eq = c.getAMethod() and
-      and itemText = "IEquatable<" + eq.getParameter(0).getType() + ">.Equals"
+      message = "but it implements $@" and
+      itemText = "IEquatable<" + eq.getParameter(0).getType() + ">.Equals"
     )
   )
-
+select c, "Class '" + c.getName() + "' does not implement Equals(object), " + message + ".", item,
-select c, "Class '" + c.getName() + "' does not implement Equals(object), " + message + ".",
+  itemText
-  item, itemText

csharp/ql/src/API Abuse/ClassImplementsICloneable.ql

@@ -14,8 +14,8 @@ import csharp
 
 from ValueOrRefType c
 where
-  c.fromSource()
+  c.fromSource() and
-  and c.getABaseInterface+().hasQualifiedName("System", "ICloneable")
+  c.getABaseInterface+().hasQualifiedName("System", "ICloneable") and
-  and not c.isSealed()
+  not c.isSealed() and
-  and exists(Method m | m.getDeclaringType() = c and m.hasName("Clone"))
+  exists(Method m | m.getDeclaringType() = c and m.hasName("Clone"))
 select c, "Class '" + c.getName() + "' implements 'ICloneable'."

csharp/ql/src/API Abuse/DisposeNotCalledOnException.ql

@@ -29,7 +29,8 @@ ExceptionClass getAThrownException(Method m) {
   or
   exists(ControlFlowElement cfe |
     cfe = any(ThrowElement te | result = te.getExpr().getType()) or
-    cfe = any(MethodCall mc | result = getAThrownException(mc.getARuntimeTarget())) |
+    cfe = any(MethodCall mc | result = getAThrownException(mc.getARuntimeTarget()))
+  |
     cfe.getEnclosingCallable() = m and
     not isTriedAgainstException(cfe, result)
   )
@@ -39,7 +40,7 @@ ExceptionClass getAThrownException(Method m) {
  * Holds if control flow element is tried against throwing an exception of type
  * `ec`.
  */
-pragma [noinline]
+pragma[noinline]
 predicate isTriedAgainstException(ControlFlowElement cfe, ExceptionClass ec) {
   (cfe instanceof ThrowElement or cfe instanceof MethodCall) and
   exists(TryStmt ts |
@@ -53,22 +54,27 @@ predicate isTriedAgainstException(ControlFlowElement cfe, ExceptionClass ec) {
  */
 predicate disposeReachableFromDisposableCreation(MethodCall disposeCall, Expr disposableCreation) {
   // The qualifier of the Dispose call flows from something that introduced a disposable into scope
-  (disposableCreation instanceof LocalScopeDisposableCreation or disposableCreation instanceof MethodCall)
+  (
-  and DataFlow::localFlowStep+(DataFlow::exprNode(disposableCreation), DataFlow::exprNode(disposeCall.getQualifier()))
+    disposableCreation instanceof LocalScopeDisposableCreation or
-  and disposeCall.getTarget() instanceof DisposeMethod
+    disposableCreation instanceof MethodCall
+  ) and
+  DataFlow::localFlowStep+(DataFlow::exprNode(disposableCreation),
+    DataFlow::exprNode(disposeCall.getQualifier())) and
+  disposeCall.getTarget() instanceof DisposeMethod
 }
 
-
 from MethodCall disposeCall, Expr disposableCreation, MethodCall callThatThrows
 where
-  disposeReachableFromDisposableCreation(disposeCall, disposableCreation)
+  disposeReachableFromDisposableCreation(disposeCall, disposableCreation) and
   // The dispose call is not, itself, within a dispose method.
-  and not disposeCall.getEnclosingCallable() instanceof DisposeMethod
+  not disposeCall.getEnclosingCallable() instanceof DisposeMethod and
   // Dispose call not within a finally or catch block
-  and not exists(TryStmt ts |
+  not exists(TryStmt ts |
-    ts.getACatchClause().getAChild*() = disposeCall or ts.getFinally().getAChild*() = disposeCall)
+    ts.getACatchClause().getAChild*() = disposeCall or ts.getFinally().getAChild*() = disposeCall
+  ) and
   // At least one method call exists between the allocation and disposal that could throw
-  and disposableCreation.getAReachableElement() = callThatThrows
+  disposableCreation.getAReachableElement() = callThatThrows and
-  and callThatThrows.getAReachableElement() = disposeCall
+  callThatThrows.getAReachableElement() = disposeCall and
-  and exists(getAThrownException(callThatThrows.getARuntimeTarget()))
+  exists(getAThrownException(callThatThrows.getARuntimeTarget()))
-select disposeCall, "Dispose missed if exception is thrown by $@.", callThatThrows, callThatThrows.toString()
+select disposeCall, "Dispose missed if exception is thrown by $@.", callThatThrows,
+  callThatThrows.toString()

csharp/ql/src/API Abuse/FormatMissingArgument.ql

@@ -13,8 +13,9 @@ import csharp
 import semmle.code.csharp.frameworks.Format
 
 from FormatCall format, ValidFormatString src, int used, int supplied
-where src = format.getAFormatSource()
+where
-and used = src.getAnInsert()
+  src = format.getAFormatSource() and
-and supplied = format.getSuppliedArguments()
+  used = src.getAnInsert() and
-and used >= supplied
+  supplied = format.getSuppliedArguments() and
+  used >= supplied
 select format, "Argument '{" + used + "}' has not been supplied to $@ format string.", src, "this"

csharp/ql/src/API Abuse/FormatUnusedArgument.ql

@@ -13,6 +13,8 @@ import csharp
 import semmle.code.csharp.frameworks.Format
 
 from FormatCall format, int unused, ValidFormatString src
-where src = format.getAFormatSource()
+where
-and unused = format.getAnUnusedArgument(src)
+  src = format.getAFormatSource() and
-select format, "The $@ ignores $@.", src, "format string", format.getSuppliedExpr(unused), "this supplied value"
+  unused = format.getAnUnusedArgument(src)
+select format, "The $@ ignores $@.", src, "format string", format.getSuppliedExpr(unused),
+  "this supplied value"

csharp/ql/src/API Abuse/InconsistentEqualsGetHashCode.ql

@@ -10,22 +10,25 @@
  *       maintainability
  *       external/cwe/cwe-581
  */
+
 import csharp
 import semmle.code.csharp.frameworks.System
 
 from Class c, Method present, string missing
-where c.isSourceDeclaration() and
+where
+  c.isSourceDeclaration() and
   (
     (
-      present = (EqualsMethod)c.getAMethod() and
+      present = c.getAMethod().(EqualsMethod) and
       not c.getAMethod() instanceof GetHashCodeMethod and
       missing = "GetHashCode()"
-    ) or
+    )
+    or
     (
-      present = (GetHashCodeMethod)c.getAMethod() and
+      present = c.getAMethod().(GetHashCodeMethod) and
       not implementsEquals(c) and
       missing = "Equals(object)"
     )
   )
-select c, "Class '" + c.getName() + "' overrides $@, but not " + missing + ".",
+select c, "Class '" + c.getName() + "' overrides $@, but not " + missing + ".", present,
-  present, present.getName()
+  present.getName()

csharp/ql/src/API Abuse/IncorrectCompareToSignature.ql

@@ -13,8 +13,7 @@ import csharp
 import semmle.code.csharp.frameworks.System
 
 predicate implementsIComparable(ValueOrRefType t, Type paramType) {
-  exists(ConstructedType ct |
+  exists(ConstructedType ct | t.getABaseType+() = ct |
-    t.getABaseType+() = ct |
     ct = any(SystemIComparableTInterface i).getAConstructedGeneric() and
     paramType = ct.getATypeArgument()
   )
@@ -28,20 +27,21 @@ predicate implementsIComparable(ValueOrRefType t, Type paramType) {
 }
 
 predicate compareToMethod(Method m, Type paramType) {
-  m.hasName("CompareTo")
+  m.hasName("CompareTo") and
-  and m.fromSource()
+  m.fromSource() and
-  and m.isPublic()
+  m.isPublic() and
-  and m.getReturnType() instanceof IntType
+  m.getReturnType() instanceof IntType and
-  and m.getNumberOfParameters() = 1
+  m.getNumberOfParameters() = 1 and
-  and paramType = m.getAParameter().getType()
+  paramType = m.getAParameter().getType()
 }
 
 from Method m, RefType declaringType, Type actualParamType
-where m.isSourceDeclaration()
+where
-  and declaringType = m.getDeclaringType()
+  m.isSourceDeclaration() and
-  and compareToMethod(m, actualParamType)
+  declaringType = m.getDeclaringType() and
-  and not implementsIComparable(declaringType, actualParamType)
+  compareToMethod(m, actualParamType) and
-select m, "The parameter of this 'CompareTo' method is of type $@, but $@ does not implement 'IComparable<$@>'.",
+  not implementsIComparable(declaringType, actualParamType)
-  actualParamType, actualParamType.getName(),
+select m,
-  declaringType, declaringType.getName(),
+  "The parameter of this 'CompareTo' method is of type $@, but $@ does not implement 'IComparable<$@>'.",
+  actualParamType, actualParamType.getName(), declaringType, declaringType.getName(),
   actualParamType, actualParamType.getName()

csharp/ql/src/API Abuse/IncorrectEqualsSignature.ql

@@ -14,21 +14,22 @@ import semmle.code.csharp.frameworks.System
 
 class EqualsOtherMethod extends Method {
   EqualsOtherMethod() {
-    this.hasName("Equals")
+    this.hasName("Equals") and
-    and this.getNumberOfParameters() = 1
+    this.getNumberOfParameters() = 1 and
-    and this.getReturnType() instanceof BoolType
+    this.getReturnType() instanceof BoolType and
-    and this.getDeclaringType() instanceof Class
+    this.getDeclaringType() instanceof Class and
-    and not this instanceof EqualsMethod
+    not this instanceof EqualsMethod and
-    and not this instanceof IEquatableEqualsMethod
+    not this instanceof IEquatableEqualsMethod
   }
 
-  Type getType() {
+  Type getType() { result = this.getParameter(0).getType() }
-    result = this.getParameter(0).getType()
-  }
 }
 
 from EqualsOtherMethod equalsOther
-where equalsOther.isSourceDeclaration()
+where
-  and not implementsEquals(equalsOther.getDeclaringType())
+  equalsOther.isSourceDeclaration() and
-select equalsOther, "The $@ of this 'Equals(" + equalsOther.getType().getName() + ")' method does not override 'Equals(object)'.",
+  not implementsEquals(equalsOther.getDeclaringType())
-  equalsOther.getDeclaringType(), "declaring type"
+select equalsOther,
+  "The $@ of this 'Equals(" + equalsOther.getType().getName() +
+    ")' method does not override 'Equals(object)'.", equalsOther.getDeclaringType(),
+  "declaring type"

csharp/ql/src/API Abuse/MissingDisposeCall.ql

@@ -17,15 +17,16 @@ import Dispose
 import semmle.code.csharp.frameworks.System
 
 from DisposableType t, DisposableField f, Method dispose
-where f.getDeclaringType() = t
+where
-  and not f.isStatic()
+  f.getDeclaringType() = t and
-  and t.isSourceDeclaration()
+  not f.isStatic() and
-  and dispose = getInvokedDisposeMethod(t)
+  t.isSourceDeclaration() and
-  and dispose.getDeclaringType() = t
+  dispose = getInvokedDisposeMethod(t) and
-  and not exists(MethodCall mc |
+  dispose.getDeclaringType() = t and
-    mc.getTarget() instanceof DisposeMethod
+  not exists(MethodCall mc |
-    and mc.getQualifier() = f.getAnAccess()
+    mc.getTarget() instanceof DisposeMethod and
-    and mc.getEnclosingCallable() = dispose
+    mc.getQualifier() = f.getAnAccess() and
+    mc.getEnclosingCallable() = dispose
   )
-select dispose, "This 'Dispose()' method does not call 'Dispose()' on `IDisposable` field $@.",
+select dispose, "This 'Dispose()' method does not call 'Dispose()' on `IDisposable` field $@.", f,
-  f, f.getName()
+  f.getName()

csharp/ql/src/API Abuse/MissingDisposeMethod.ql

@@ -17,10 +17,10 @@ import Dispose
 import semmle.code.csharp.frameworks.System
 
 from DisposableType t, DisposableField f
-where f.getDeclaringType() = t
+where
-  and t.isSourceDeclaration()
+  f.getDeclaringType() = t and
-  and not f.isStatic()
+  t.isSourceDeclaration() and
-  and not implementsDispose(t)
+  not f.isStatic() and
-  and not isAutoDisposedWebControl(f)
+  not implementsDispose(t) and
-select t, "This type does not override 'Dispose()' but has disposable field $@.",
+  not isAutoDisposedWebControl(f)
-  f, f.getName()
+select t, "This type does not override 'Dispose()' but has disposable field $@.", f, f.getName()

csharp/ql/src/API Abuse/NoDisposeCallOnLocalIDisposable.ql

@@ -22,55 +22,57 @@ import semmle.code.csharp.commons.Disposal
 /** Holds if expression `e` escapes the local method scope. */
 predicate escapes(Expr e) {
   // Things that return escape
-  exists(Callable c | c.canReturn(e) or c.canYieldReturn(e)) or
+  exists(Callable c | c.canReturn(e) or c.canYieldReturn(e))
+  or
   // Things that are assigned to fields, properties, or indexers escape the scope
   exists(AssignableDefinition def, Assignable a |
     def.getSource() = e and
-    a = def.getTarget() |
+    a = def.getTarget()
+  |
     a instanceof Field or
     a instanceof Property or
     a instanceof Indexer
-  ) or
-  // Things that are added to a collection of some kind are likely to escape the scope
-  exists(MethodCall mc |
-    mc.getTarget().hasName("Add") |
-    mc.getAnArgument() = e
   )
+  or
+  // Things that are added to a collection of some kind are likely to escape the scope
+  exists(MethodCall mc | mc.getTarget().hasName("Add") | mc.getAnArgument() = e)
 }
 
 /** Holds if the `disposable` is a whitelisted result. */
 predicate isWhitelisted(LocalScopeDisposableCreation disposable) {
   exists(MethodCall mc |
     // Close can often be used instead of Dispose
-    mc.getTarget().hasName("Close") or
+    mc.getTarget().hasName("Close")
+    or
     // Used as an alias for Dispose
     mc.getTarget().hasName("Clear")
-    |
+  |
     mc.getQualifier() = disposable.getADisposeTarget()
-  ) or
+  )
+  or
   // WebControls are usually disposed automatically
   disposable.getType() instanceof WebControl
 }
 
 from LocalScopeDisposableCreation disposable
-  // The disposable is local scope - the lifetime is the execution of this method
+// The disposable is local scope - the lifetime is the execution of this method
-where not escapes(disposable.getADisposeTarget())
+where
+  not escapes(disposable.getADisposeTarget()) and
   // Only care about library types - user types often have spurious IDisposable declarations
-  and disposable.getType().fromLibrary()
+  disposable.getType().fromLibrary() and
   // Disposables constructed in the initializer of a `using` are safe
-  and not exists(UsingStmt us | us.getAnExpr() = disposable.getADisposeTarget())
+  not exists(UsingStmt us | us.getAnExpr() = disposable.getADisposeTarget()) and
   // Foreach calls Dispose
-  and not exists(ForeachStmt fs | fs.getIterableExpr() = disposable.getADisposeTarget())
+  not exists(ForeachStmt fs | fs.getIterableExpr() = disposable.getADisposeTarget()) and
   // As are disposables on which the Dispose method is called explicitly
-  and not exists(MethodCall mc |
+  not exists(MethodCall mc |
-    mc.getTarget() instanceof DisposeMethod
+    mc.getTarget() instanceof DisposeMethod and
-    and mc.getQualifier() = disposable.getADisposeTarget()
+    mc.getQualifier() = disposable.getADisposeTarget()
-  )
+  ) and
   // Ignore whitelisted results
-  and not isWhitelisted(disposable)
+  not isWhitelisted(disposable) and
   // Not passed to a disposing method
-  and not exists(Call c, Parameter p |
+  not exists(Call c, Parameter p | disposable.getADisposeTarget() = c.getArgumentForParameter(p) |
-    disposable.getADisposeTarget() = c.getArgumentForParameter(p) |
     mayBeDisposed(p)
   )
 select disposable, "Disposable '" + disposable.getType() + "' is created here but is not disposed."

csharp/ql/src/API Abuse/NonOverridingMethod.ql

@@ -21,23 +21,24 @@ private predicate potentialOverride(Method vm, Method m) {
  * but does not do so.
  */
 predicate nonOverridingMethod(Method m, Method vm) {
-  vm.isVirtual()
+  vm.isVirtual() and
-  and not vm.isOverride()
+  not vm.isOverride() and
-  and not vm.overrides()
+  not vm.overrides() and
-  and potentialOverride(vm, m)
+  potentialOverride(vm, m) and
-  and not m.overrides()
+  not m.overrides() and
-  and not m.isOverride()
+  not m.isOverride() and
-  and not m.isNew()
+  not m.isNew() and
-  and m=m.getSourceDeclaration()
+  m = m.getSourceDeclaration() and
-  and m.getNumberOfParameters() = vm.getNumberOfParameters()
+  m.getNumberOfParameters() = vm.getNumberOfParameters() and
-  and forall(int i, Parameter p1, Parameter p2 |
+  forall(int i, Parameter p1, Parameter p2 | p1 = m.getParameter(i) and p2 = vm.getParameter(i) |
-    p1=m.getParameter(i) and p2=vm.getParameter(i) |
+    p1.getType() = p2.getType()
-    p1.getType() = p2.getType())
+  ) and
-  and m.getName().toLowerCase() = vm.getName().toLowerCase()
+  m.getName().toLowerCase() = vm.getName().toLowerCase()
 }
 
 from Method m, Method vm
-where m.fromSource()
+where
-  and nonOverridingMethod(m, vm)
+  m.fromSource() and
+  nonOverridingMethod(m, vm)
 select m, "Method '" + m.getName() + "' looks like it should override $@ but does not do so.",
   vm.getSourceDeclaration(), vm.getQualifiedName()

csharp/ql/src/API Abuse/NullArgumentToEquals.ql

@@ -14,7 +14,8 @@ import csharp
 import semmle.code.csharp.frameworks.System
 
 from MethodCall c, EqualsMethod equals
-where c.getTarget().getSourceDeclaration() = equals
+where
-  and c.getArgument(0) instanceof NullLiteral
+  c.getTarget().getSourceDeclaration() = equals and
-  and not c.getQualifier().getType() instanceof NullableType
+  c.getArgument(0) instanceof NullLiteral and
+  not c.getQualifier().getType() instanceof NullableType
 select c, "Equality test with 'null' will never be true, but may throw a 'NullReferenceException'."

csharp/ql/src/API Abuse/UncheckedReturnValue.ql

@@ -12,19 +12,18 @@
  *       statistical
  *       non-attributable
  */
+
 import csharp
 import semmle.code.csharp.frameworks.system.IO
 import semmle.code.csharp.Chaining
 
 /** Holds if `m` is a method whose return value should always be checked. */
 predicate important(Method m) {
-  exists(Method read |
+  exists(Method read | read = any(SystemIOStreamClass c).getReadMethod() |
-    read = any(SystemIOStreamClass c).getReadMethod() |
     m = read.getAnOverrider*()
   )
   or
-  exists(Method readByte |
+  exists(Method readByte | readByte = any(SystemIOStreamClass c).getReadByteMethod() |
-    readByte = any(SystemIOStreamClass c).getReadByteMethod() |
     m = readByte.getAnOverrider*()
   )
   // add more ...
@@ -49,14 +48,14 @@ predicate dubious(Method m, int percentage) {
   exists(int used, int total, Method target |
     target = m.getSourceDeclaration() and
     used = count(MethodCall mc |
-      mc.getTarget().getSourceDeclaration() = target and
+        mc.getTarget().getSourceDeclaration() = target and
-      not mc instanceof DiscardedMethodCall and
+        not mc instanceof DiscardedMethodCall and
-      (methodHasGenericReturnType(m) implies m.getReturnType() = mc.getTarget().getReturnType())
+        (methodHasGenericReturnType(m) implies m.getReturnType() = mc.getTarget().getReturnType())
-    ) and
+      ) and
     total = count(MethodCall mc |
-      mc.getTarget().getSourceDeclaration() = target and
+        mc.getTarget().getSourceDeclaration() = target and
-      (methodHasGenericReturnType(m) implies m.getReturnType() = mc.getTarget().getReturnType())
+        (methodHasGenericReturnType(m) implies m.getReturnType() = mc.getTarget().getReturnType())
-    ) and
+      ) and
     used != total and
     percentage = used * 100 / total and
     percentage >= 90 and
@@ -66,9 +65,9 @@ predicate dubious(Method m, int percentage) {
 
 int chainedUses(Method m) {
   result = count(MethodCall mc, MethodCall qual |
-    m = mc.getTarget() and
+      m = mc.getTarget() and
-    hasQualifierAndTarget(mc, qual, qual.getTarget())
+      hasQualifierAndTarget(mc, qual, qual.getTarget())
-  )
+    )
 }
 
 predicate hasQualifierAndTarget(MethodCall mc, Expr qualifier, Method m) {
@@ -78,10 +77,11 @@ predicate hasQualifierAndTarget(MethodCall mc, Expr qualifier, Method m) {
 
 /** Holds if `m` is a white-listed method where checking the return value is not required. */
 predicate whitelist(Method m) {
-  m.hasName("TryGetValue") or
+  m.hasName("TryGetValue")
-  m.hasName("TryParse") or
+  or
-  exists(Namespace n |
+  m.hasName("TryParse")
-    n = m.getDeclaringType().getNamespace().getParentNamespace*() |
+  or
+  exists(Namespace n | n = m.getDeclaringType().getNamespace().getParentNamespace*() |
     n.getName().regexpMatch("(Fluent)?NHibernate") or
     n.getName() = "Moq"
   )
@@ -89,9 +89,7 @@ predicate whitelist(Method m) {
 }
 
 class DiscardedMethodCall extends MethodCall {
-  DiscardedMethodCall() {
+  DiscardedMethodCall() { this.getParent() instanceof ExprStmt }
-    this.getParent() instanceof ExprStmt
-  }
 
   string query() {
     exists(Method m |
@@ -99,11 +97,11 @@ class DiscardedMethodCall extends MethodCall {
       not whitelist(m) and
       // Do not alert on "void wrapper methods", i.e., methods that are inserted
       // to deliberately ignore the returned value
-      not getEnclosingCallable().getStatementBody().getNumberOfStmts() = 1 |
+      not getEnclosingCallable().getStatementBody().getNumberOfStmts() = 1
+    |
       (important(m) and result = "should always be checked")
       or
-      exists(int percentage |
+      exists(int percentage | dubious(m, percentage) |
-        dubious(m, percentage) |
         result = percentage.toString() + "% of calls to this method have their result used"
       )
     )

csharp/ql/src/ASP/NonInternationalizedText.ql

@@ -12,6 +12,5 @@
 import semmle.code.asp.AspNet
 
 from AspText text
-where
+where exists(text.getBody().regexpFind("\\w{3,}", _, _))
-  exists(text.getBody().regexpFind("\\w{3,}", _, _))
 select text, "This text has not been internationalized."

csharp/ql/src/ASP/SplitControlStructure.ql

@@ -12,6 +12,5 @@
 import semmle.code.asp.AspNet
 
 from AspCode code
-where
+where exists(code.getBody().regexpFind("(Then|\\{)\\s*$", _, _))
-  exists(code.getBody().regexpFind("(Then|\\{)\\s*$",_,_))
 select code, "Split control structure."

csharp/ql/src/AlertSuppression.ql

@@ -14,8 +14,7 @@ class SuppressionComment extends SinglelineComment {
   string annotation;
 
   SuppressionComment() {
-    exists(string text |
+    exists(string text | text = this.getText() |
-      text = this.getText() |
       // match `lgtm[...]` anywhere in the comment
       annotation = text.regexpFind("(?i)\\blgtm\\s*\\[[^\\]]*\\]", _, _)
       or
@@ -25,57 +24,49 @@ class SuppressionComment extends SinglelineComment {
   }
 
   /** Gets the suppression annotation in this comment. */
-  string getAnnotation() {
+  string getAnnotation() { result = annotation }
-    result = annotation
-  }
 
   /**
-  * Holds if this comment applies to the range from column `startcolumn` of line `startline`
+   * Holds if this comment applies to the range from column `startcolumn` of line `startline`
-  * to column `endcolumn` of line `endline` in file `filepath`.
+   * to column `endcolumn` of line `endline` in file `filepath`.
-  */
+   */
   predicate covers(string filepath, int startline, int startcolumn, int endline, int endcolumn) {
     this.getLocation().hasLocationInfo(filepath, startline, _, endline, endcolumn) and
     startcolumn = 1
   }
 
   /** Gets the scope of this suppression. */
-  SuppressionScope getScope() {
+  SuppressionScope getScope() { this = result.getSuppressionComment() }
-    this = result.getSuppressionComment()
-  }
 }
 
 /**
  * The scope of an alert suppression comment.
  */
 class SuppressionScope extends @commentline {
-  SuppressionScope() {
+  SuppressionScope() { this instanceof SuppressionComment }
-    this instanceof SuppressionComment
-  }
 
   /** Gets a suppression comment with this scope. */
-  SuppressionComment getSuppressionComment() {
+  SuppressionComment getSuppressionComment() { result = this }
-    result = this
-  }
 
   /**
-  * Holds if this element is at the specified location.
+   * Holds if this element is at the specified location.
-  * The location spans column `startcolumn` of line `startline` to
+   * The location spans column `startcolumn` of line `startline` to
-  * column `endcolumn` of line `endline` in file `filepath`.
+   * column `endcolumn` of line `endline` in file `filepath`.
-  * For more information, see
+   * For more information, see
-  * [LGTM locations](https://lgtm.com/help/ql/locations).
+   * [LGTM locations](https://lgtm.com/help/ql/locations).
-  */
+   */
-  predicate hasLocationInfo(string filepath, int startline, int startcolumn, int endline, int endcolumn) {
+  predicate hasLocationInfo(
+    string filepath, int startline, int startcolumn, int endline, int endcolumn
+  ) {
     this.(SuppressionComment).covers(filepath, startline, startcolumn, endline, endcolumn)
   }
 
   /** Gets a textual representation of this element. */
-  string toString() {
+  string toString() { result = "suppression range" }
-    result = "suppression range"
-  }
 }
 
 from SuppressionComment c
-select c,                 // suppression comment
+select c, // suppression comment
-       c.getText(),       // text of suppression comment (excluding delimiters)
+  c.getText(), // text of suppression comment (excluding delimiters)
-       c.getAnnotation(), // text of suppression annotation
+  c.getAnnotation(), // text of suppression annotation
-       c.getScope()       // scope of suppression
+  c.getScope() // scope of suppression

csharp/ql/src/Architecture/Dependencies/MutualDependency.ql

@@ -15,32 +15,27 @@ import semmle.code.csharp.metrics.Coupling
 
 /** inner is nested (possibly more than one level deep) within outer */
 predicate nestedWithin(ValueOrRefType outer, NestedType inner) {
-  inner.getDeclaringType() = outer
+  inner.getDeclaringType() = outer or
-  or nestedWithin(outer, inner.getDeclaringType())
+  nestedWithin(outer, inner.getDeclaringType())
 }
 
 from ValueOrRefType t1, ValueOrRefType t2
 where
-  t1 != t2
+  t1 != t2 and
-  and depends(t1, t2)
+  depends(t1, t2) and
-  and depends(t2, t1)
+  depends(t2, t1) and
-
   // PREVENT SYMMETRICAL RESULTS
-  and t1.getName() < t2.getName()
+  t1.getName() < t2.getName() and
-
   // ADDITIONAL CONSTRAINTS
-  and t1.fromSource()
+  t1.fromSource() and
-  and t2.fromSource()
+  t2.fromSource() and
-
   // EXCLUSIONS
-  and not
+  not (
-  (
+    nestedWithin(t1, t2) or
-    nestedWithin(t1, t2)
+    nestedWithin(t2, t1) or
-    or nestedWithin(t2, t1)
+    t1.getName().toLowerCase().matches("%visitor%") or
-    or t1.getName().toLowerCase().matches("%visitor%")
+    t2.getName().toLowerCase().matches("%visitor%") or
-    or t2.getName().toLowerCase().matches("%visitor%")
+    t1.getAMember().getName().toLowerCase().matches("%visit%") or
-    or t1.getAMember().getName().toLowerCase().matches("%visit%")
+    t2.getAMember().getName().toLowerCase().matches("%visit%")
-    or t2.getAMember().getName().toLowerCase().matches("%visit%")
   )
-select t1, "This type and type $@ are mutually dependent.",
+select t1, "This type and type $@ are mutually dependent.", t2, t2.getName()
-  t2, t2.getName()

csharp/ql/src/Architecture/Refactoring Opportunities/FeatureEnvy.ql

@@ -11,31 +11,24 @@
  *       maintainability
  *       modularity
  */
+
 import csharp
 
 Member getAUsedMember(Method m) {
-  exists(MemberAccess ma |
+  exists(MemberAccess ma | ma.getEnclosingCallable() = m |
-    ma.getEnclosingCallable() = m |
     result = ma.getTarget().getSourceDeclaration()
   )
   or
-  exists(Call c |
+  exists(Call c | c.getEnclosingCallable() = m | result = c.getTarget().getSourceDeclaration())
-    c.getEnclosingCallable() = m |
-    result = c.getTarget().getSourceDeclaration()
-  )
 }
 
 int dependencyCount(Method source, RefType target) {
   result = strictcount(Member m | m = getAUsedMember(source) and m = target.getAMember())
 }
 
-predicate methodDependsOn(Method m, RefType target) {
+predicate methodDependsOn(Method m, RefType target) { exists(dependencyCount(m, target)) }
-  exists(dependencyCount(m, target))
-}
 
-predicate dependsOn(RefType source, RefType target) {
+predicate dependsOn(RefType source, RefType target) { methodDependsOn(source.getAMethod(), target) }
-  methodDependsOn(source.getAMethod(), target)
-}
 
 int selfDependencyCount(Method source) {
   result = sum(dependencyCount(source, source.getDeclaringType+()))
@@ -44,12 +37,12 @@ int selfDependencyCount(Method source) {
 predicate dependsHighlyOn(Method source, RefType target, int selfCount, int depCount) {
   depCount = dependencyCount(source, target) and
   selfCount = selfDependencyCount(source) and
-  depCount > 2*selfCount and
+  depCount > 2 * selfCount and
   depCount > 4
 }
 
 predicate query(Method m, RefType targetType, int selfCount, int depCount) {
-  exists (RefType sourceType | sourceType = m.getDeclaringType() |
+  exists(RefType sourceType | sourceType = m.getDeclaringType() |
     dependsHighlyOn(m, targetType, selfCount, depCount) and
     // Interfaces are depended upon by their very nature
     not targetType instanceof Interface and
@@ -63,8 +56,7 @@ predicate query(Method m, RefType targetType, int selfCount, int depCount) {
     // Do not move between nested types
     not (sourceType.getDeclaringType*() = targetType or targetType.getDeclaringType*() = sourceType) and
     // Check that the target type already depends on every type used by the method
-    forall(RefType dependency |
+    forall(RefType dependency | methodDependsOn(m, dependency) |
-      methodDependsOn(m, dependency) |
       dependsOn(targetType, dependency) or
       targetType = dependency or
       dependency.getNamespace().hasName("System")
@@ -73,10 +65,12 @@ predicate query(Method m, RefType targetType, int selfCount, int depCount) {
 }
 
 from Method m, RefType other, int selfCount, int depCount
-where query(m, other, selfCount, depCount) and
+where
-      // Don't include types that are used from many different places - we only highlight
+  query(m, other, selfCount, depCount) and
-      // relatively local fixes that could reasonably be implemented.
+  // Don't include types that are used from many different places - we only highlight
-      count(Method yetAnotherMethod | query(yetAnotherMethod, other, _, _)) < 10
+  // relatively local fixes that could reasonably be implemented.
-select m, "Method " + m.getName() + " is too closely tied to $@: " + depCount +
+  count(Method yetAnotherMethod | query(yetAnotherMethod, other, _, _)) < 10
-          " dependencies to it, but only " + selfCount + " dependencies to its own type.",
+select m,
-          other, other.getName()
+  "Method " + m.getName() + " is too closely tied to $@: " + depCount +
+    " dependencies to it, but only " + selfCount + " dependencies to its own type.", other,
+  other.getName()

csharp/ql/src/Architecture/Refactoring Opportunities/InappropriateIntimacy.ql

@@ -9,6 +9,7 @@
  * @tags maintainability
  *       modularity
  */
+
 import csharp
 
 predicate enclosingRefType(Variable v, RefType type) {
@@ -45,16 +46,19 @@ predicate dependencyCount(RefType source, RefType target, int res) {
   exists(int varCount, int funCount |
     variableDependencyCount(source, target, varCount) and
     functionDependencyCount(source, target, funCount) and
-  res = varCount + funCount
+    res = varCount + funCount and
-  and res > 15)
+    res > 15
+  )
 }
 
 from RefType a, RefType b, int ca, int cb
-where dependencyCount(a, b, ca) and
+where
-      dependencyCount(b, a, cb) and
+  dependencyCount(a, b, ca) and
-      ca > 15 and
+  dependencyCount(b, a, cb) and
-      cb > 15 and
+  ca > 15 and
-      ca >= cb and
+  cb > 15 and
-      a != b
+  ca >= cb and
-select a, "Type " + a.getName() + " is too closely tied to $@ (" + ca.toString() +
+  a != b
-          " dependencies one way and " + cb.toString() + " the other).", b, b.getName()
+select a,
+  "Type " + a.getName() + " is too closely tied to $@ (" + ca.toString() +
+    " dependencies one way and " + cb.toString() + " the other).", b, b.getName()

csharp/ql/src/Bad Practices/CallsUnmanagedCode.ql

@@ -12,7 +12,8 @@
 import csharp
 
 from Class c, Method m, MethodCall call
-where m.isExtern()
+where
-  and m.getDeclaringType() = c
+  m.isExtern() and
-  and call.getTarget() = m
+  m.getDeclaringType() = c and
+  call.getTarget() = m
 select call, "Replace this call with a call to managed code if possible."

csharp/ql/src/Bad Practices/CatchOfNullReferenceException.ql

@@ -9,6 +9,7 @@
  *       correctness
  *       external/cwe/cwe-395
  */
+
 import csharp
 
 from SpecificCatchClause scc

csharp/ql/src/Bad Practices/Comments/CommentedOutCode.ql

@@ -12,12 +12,10 @@
 
 import csharp
 
-class CommentedOutCode extends CommentBlock
+class CommentedOutCode extends CommentBlock {
-{
+  CommentedOutCode() {
-  CommentedOutCode()
-  {
     not isXmlCommentBlock() and
-    2*count(getAProbableCodeLine()) > count(getANonEmptyLine())
+    2 * count(getAProbableCodeLine()) > count(getANonEmptyLine())
   }
 }
 

csharp/ql/src/Bad Practices/Comments/TodoComments.ql

@@ -9,6 +9,7 @@
  * @tags maintainability
  *       external/cwe/cwe-546
  */
+
 import csharp
 
 from CommentLine c

csharp/ql/src/Bad Practices/Control-Flow/ConstantCondition.ql

@@ -11,6 +11,7 @@
  *       readability
  *       external/cwe/cwe-835
  */
+
 import csharp
 import semmle.code.csharp.commons.Assertions
 import semmle.code.csharp.commons.Constants
@@ -28,13 +29,9 @@ abstract class ConstantCondition extends Expr {
 class ConstantBooleanCondition extends ConstantCondition {
   boolean b;
 
-  ConstantBooleanCondition() {
+  ConstantBooleanCondition() { isConstantCondition(this, b) }
-    isConstantCondition(this, b)
-  }
 
-  override string getMessage() {
+  override string getMessage() { result = "Condition always evaluates to '" + b + "'." }
-    result = "Condition always evaluates to '" + b + "'."
-  }
 
   override predicate isWhiteListed() {
     // E.g. `x ?? false`
@@ -60,9 +57,7 @@ class ConstantIfCondition extends ConstantBooleanCondition {
 
 /** A constant loop condition. */
 class ConstantLoopCondition extends ConstantBooleanCondition {
-  ConstantLoopCondition() {
+  ConstantLoopCondition() { this = any(LoopStmt ls).getCondition() }
-    this = any(LoopStmt ls).getCondition()
-  }
 
   override predicate isWhiteListed() {
     // Clearly intentional infinite loops are allowed
@@ -75,10 +70,8 @@ class ConstantNullnessCondition extends ConstantCondition {
   boolean b;
 
   ConstantNullnessCondition() {
-    forex(ControlFlow::Node cfn |
+    forex(ControlFlow::Node cfn | cfn = this.getAControlFlowNode() |
-      cfn = this.getAControlFlowNode() |
+      exists(ControlFlow::SuccessorTypes::NullnessSuccessor t | exists(cfn.getASuccessorByType(t)) |
-      exists(ControlFlow::SuccessorTypes::NullnessSuccessor t |
-        exists(cfn.getASuccessorByType(t)) |
         b = t.getValue()
       ) and
       strictcount(ControlFlow::SuccessorType t | exists(cfn.getASuccessorByType(t))) = 1
@@ -86,10 +79,9 @@ class ConstantNullnessCondition extends ConstantCondition {
   }
 
   override string getMessage() {
-    if b = true then
+    if b = true
-      result = "Expression is always 'null'."
+    then result = "Expression is always 'null'."
-    else
+    else result = "Expression is never 'null'."
-      result = "Expression is never 'null'."
   }
 }
 
@@ -98,10 +90,8 @@ class ConstantMatchingCondition extends ConstantCondition {
   boolean b;
 
   ConstantMatchingCondition() {
-    forex(ControlFlow::Node cfn |
+    forex(ControlFlow::Node cfn | cfn = this.getAControlFlowNode() |
-      cfn = this.getAControlFlowNode() |
+      exists(ControlFlow::SuccessorTypes::MatchingSuccessor t | exists(cfn.getASuccessorByType(t)) |
-      exists(ControlFlow::SuccessorTypes::MatchingSuccessor t |
-        exists(cfn.getASuccessorByType(t)) |
         b = t.getValue()
       ) and
       strictcount(ControlFlow::SuccessorType t | exists(cfn.getASuccessorByType(t))) = 1
@@ -109,15 +99,13 @@ class ConstantMatchingCondition extends ConstantCondition {
   }
 
   override string getMessage() {
-    if b = true then
+    if b = true then result = "Pattern always matches." else result = "Pattern never matches."
-      result = "Pattern always matches."
-    else
-      result = "Pattern never matches."
   }
 }
 
 from ConstantCondition c, string msg
-where msg = c.getMessage()
+where
-  and not c.isWhiteListed()
+  msg = c.getMessage() and
-  and not isExprInAssertion(c)
+  not c.isWhiteListed() and
+  not isExprInAssertion(c)
 select c, msg

csharp/ql/src/Bad Practices/Declarations/EmptyInterface.ql

@@ -13,7 +13,8 @@
 import csharp
 
 from Interface i
-where not exists(i.getAMember())
+where
-  and i.isSourceDeclaration()
+  not exists(i.getAMember()) and
-  and count(Interface base | i.getABaseInterface() = base) <= 1
+  i.isSourceDeclaration() and
+  count(Interface base | i.getABaseInterface() = base) <= 1
 select i, "Interface '" + i.getName() + "' does not declare any members."

csharp/ql/src/Bad Practices/Declarations/LocalScopeVariableShadowsMember.ql

@@ -9,9 +9,10 @@
  * @tags maintainability
  *       readability
  */
+
 import csharp
 
-pragma [noinline]
+pragma[noinline]
 private string localVarInType(ValueOrRefType t, LocalScopeVariable v) {
   v.getCallable().getDeclaringType() = t and
   result = v.getName() and
@@ -28,12 +29,10 @@ private string memberInType(ValueOrRefType t, Member m) {
 }
 
 private predicate acceptableShadowing(LocalScopeVariable v, Member m) {
-  exists(ValueOrRefType t |
+  exists(ValueOrRefType t | localVarInType(t, v) = memberInType(t, m) |
-    localVarInType(t, v) = memberInType(t, m) |
     // If the callable declaring the local also accesses the shadowed member
     // using an explicit `this` qualifier, the shadowing is likely deliberate.
-    exists(MemberAccess ma |
+    exists(MemberAccess ma | ma.getTarget() = m |
-      ma.getTarget() = m |
       ma.getEnclosingCallable() = v.getCallable() and
       ma.targetIsLocalInstance() and
       not ma.getQualifier().isImplicit()
@@ -49,7 +48,9 @@ private predicate shadowing(ValueOrRefType t, LocalScopeVariable v, Member m) {
 }
 
 from LocalScopeVariable v, Callable c, ValueOrRefType t, Member m
-where c = v.getCallable()
+where
-  and shadowing(t, v, m)
+  c = v.getCallable() and
-  and (c.(Modifiable).isStatic() implies m.isStatic())
+  shadowing(t, v, m) and
-select v, "Local scope variable '" + v.getName() + "' shadows $@.", m, t.getName() + "." + m.getName()
+  (c.(Modifiable).isStatic() implies m.isStatic())
+select v, "Local scope variable '" + v.getName() + "' shadows $@.", m,
+  t.getName() + "." + m.getName()

csharp/ql/src/Bad Practices/Declarations/NoConstantsOnly.ql

@@ -9,6 +9,7 @@
  * @tags maintainability
  *       modularity
  */
+
 import csharp
 
 class ConstantField extends Field {
@@ -20,11 +21,11 @@ class ConstantField extends Field {
 }
 
 from Class c
-where c.isSourceDeclaration()
+where
-  and c.isAbstract()
+  c.isSourceDeclaration() and
-  and c.getAMember() instanceof ConstantField
+  c.isAbstract() and
-  and forex(Member m |
+  c.getAMember() instanceof ConstantField and
-    m = c.getAMember() |
+  forex(Member m | m = c.getAMember() |
     m instanceof ConstantField or
     m instanceof Constructor
   )

csharp/ql/src/Bad Practices/Declarations/TooManyRefParameters.ql

@@ -13,7 +13,9 @@
 import csharp
 
 from Method m, int n
-where m.isSourceDeclaration()
+where
-  and n = count(Parameter p | p = m.getAParameter() and p.isRef())
+  m.isSourceDeclaration() and
-  and n > 2
+  n = count(Parameter p | p = m.getAParameter() and p.isRef()) and
-select m, "Method '" + m.getName() + "' has " + n + " 'ref' parameters and might be hard to understand."
+  n > 2
+select m,
+  "Method '" + m.getName() + "' has " + n + " 'ref' parameters and might be hard to understand."

csharp/ql/src/Bad Practices/EmptyCatchBlock.ql

@@ -11,9 +11,11 @@
  *       external/cwe/cwe-390
  *       external/cwe/cwe-391
  */
+
 import csharp
 
 from CatchClause cc
-where cc.getBlock().isEmpty()
+where
-and not exists(CommentBlock cb | cb.getParent()=cc.getBlock())
+  cc.getBlock().isEmpty() and
+  not exists(CommentBlock cb | cb.getParent() = cc.getBlock())
 select cc, "Poor error handling: empty catch block."

csharp/ql/src/Bad Practices/ErroneousClassCompare.ql

@@ -9,31 +9,28 @@
  *       correctness
  *       external/cwe/cwe-486
  */
+
 import csharp
 import semmle.code.csharp.commons.ComparisonTest
 import semmle.code.csharp.frameworks.System
 
-ComparisonTest getComparisonTest(Expr e) {
+ComparisonTest getComparisonTest(Expr e) { result.getExpr() = e }
-  result.getExpr() = e
-}
 
 class StringComparison extends Expr {
   StringComparison() {
-    exists(ComparisonTest ct |
+    exists(ComparisonTest ct | ct = getComparisonTest(this) |
-      ct = getComparisonTest(this) |
       ct.getComparisonKind().isEquality() and
       ct.getFirstArgument().stripCasts().getType() instanceof StringType and
       ct.getSecondArgument().stripCasts().getType() instanceof StringType
     )
   }
 
-  Expr getAnOperand() {
+  Expr getAnOperand() { result = getComparisonTest(this).getAnArgument() }
-    result = getComparisonTest(this).getAnArgument()
-  }
 }
 
 from StringComparison sc, PropertyAccess pa
-where sc.getAnOperand() instanceof StringLiteral
+where
- and sc.getAnOperand() = pa
+  sc.getAnOperand() instanceof StringLiteral and
- and pa.getTarget() = any(SystemTypeClass c).getFullNameProperty()
+  sc.getAnOperand() = pa and
+  pa.getTarget() = any(SystemTypeClass c).getFullNameProperty()
 select sc, "Erroneous class compare."

csharp/ql/src/Bad Practices/Implementation Hiding/AbstractToConcreteCollection.ql

@@ -12,6 +12,7 @@
  *       modularity
  *       external/cwe/cwe-485
  */
+
 import csharp
 import semmle.code.csharp.frameworks.system.Collections
 import semmle.code.csharp.frameworks.system.collections.Generic
@@ -19,8 +20,7 @@ import semmle.code.csharp.frameworks.system.collections.Generic
 /** A collection interface. */
 class CollectionInterface extends Interface {
   CollectionInterface() {
-    exists(Interface i |
+    exists(Interface i | i = this.getABaseInterface*() |
-      i = this.getABaseInterface*() |
       i instanceof SystemCollectionsICollectionInterface or
       i.getSourceDeclaration() instanceof SystemCollectionsGenericICollectionInterface or
       i instanceof SystemCollectionsIEnumerableInterface or
@@ -30,8 +30,10 @@ class CollectionInterface extends Interface {
 }
 
 from CastExpr e, Class c, CollectionInterface i
-where e.getType() = c
+where
-  and e.getExpr().getType() = i
+  e.getType() = c and
-  and c.isImplicitlyConvertibleTo(i)
+  e.getExpr().getType() = i and
-select e, "Questionable cast from abstract '" + i.getName()
+  c.isImplicitlyConvertibleTo(i)
-            + "' to concrete implementation '" + c.getName() + "'."
+select e,
+  "Questionable cast from abstract '" + i.getName() + "' to concrete implementation '" + c.getName()
+    + "'."

csharp/ql/src/Bad Practices/Implementation Hiding/ExposeRepresentation.ql

@@ -9,6 +9,7 @@
  * @tags reliability
  *       external/cwe/cwe-485
  */
+
 import csharp
 import semmle.code.csharp.commons.Collections
 import DataFlow
@@ -31,13 +32,9 @@ predicate returnsCollection(Callable c, Field f) {
 predicate mayWriteToCollection(Expr modified) {
   modified instanceof CollectionModificationAccess
   or
-  exists(Expr mid |
+  exists(Expr mid | mayWriteToCollection(mid) | localFlow(exprNode(modified), exprNode(mid)))
-    mayWriteToCollection(mid) |
-    localFlow(exprNode(modified), exprNode(mid))
-  )
   or
-  exists(MethodCall mid, Callable c |
+  exists(MethodCall mid, Callable c | mayWriteToCollection(mid) |
-    mayWriteToCollection(mid) |
     mid.getTarget() = c and
     c.canReturn(modified)
   )
@@ -49,16 +46,12 @@ predicate modificationAfter(Expr before, Expr after) {
 }
 
 VariableAccess varPassedInto(Callable c, Parameter p) {
-  exists(Call call |
+  exists(Call call | call.getTarget() = c | call.getArgumentForParameter(p) = result)
-    call.getTarget() = c |
-    call.getArgumentForParameter(p) = result
-  )
 }
 
 predicate exposesByReturn(Callable c, Field f, Expr why, string whyText) {
   returnsCollection(c, f) and
-  exists(MethodCall ma |
+  exists(MethodCall ma | ma.getTarget() = c |
-    ma.getTarget() = c |
     mayWriteToCollection(ma) and
     why = ma and
     whyText = "after this call to " + c.getName()
@@ -75,8 +68,9 @@ predicate exposesByStore(Callable c, Field f, Expr why, string whyText) {
 }
 
 from Callable c, Field f, Expr why, string whyText
-where exposesByReturn(c, f, why, whyText) or
+where
-      exposesByStore(c, f, why, whyText)
+  exposesByReturn(c, f, why, whyText) or
-select c, "'" + c.getName() + "' exposes the internal representation stored in field '" + f.getName() +
+  exposesByStore(c, f, why, whyText)
-          "'. The value may be modified $@.",
+select c,
-          why.getLocation(), whyText
+  "'" + c.getName() + "' exposes the internal representation stored in field '" + f.getName() +
+    "'. The value may be modified $@.", why.getLocation(), whyText

csharp/ql/src/Bad Practices/Implementation Hiding/StaticArray.ql

@@ -10,31 +10,32 @@
  *       modularity
  *       external/cwe/cwe-582
  */
+
 import csharp
 
 predicate nonEmptyArrayLiteralOrNull(Expr e) {
   e = any(ArrayCreation arr |
-    exists(arr.getInitializer().getAnElement())
+      exists(arr.getInitializer().getAnElement())
-    or
+      or
-    not arr.getALengthArgument().getValue() = "0"
+      not arr.getALengthArgument().getValue() = "0"
-  )
+    )
   or
   e instanceof NullLiteral
   or
   e = any(ConditionalExpr cond |
-    nonEmptyArrayLiteralOrNull(cond.getThen()) and
+      nonEmptyArrayLiteralOrNull(cond.getThen()) and
-    nonEmptyArrayLiteralOrNull(cond.getElse())
+      nonEmptyArrayLiteralOrNull(cond.getElse())
-  )
+    )
 }
 
 from Field f
-where f.isPublic()
+where
-  and f.isStatic()
+  f.isPublic() and
-  and f.isReadOnly()
+  f.isStatic() and
-  and f.getType() instanceof ArrayType
+  f.isReadOnly() and
-  and f.fromSource()
+  f.getType() instanceof ArrayType and
-  and forall(AssignableDefinition def |
+  f.fromSource() and
-    def.getTarget() = f |
+  forall(AssignableDefinition def | def.getTarget() = f |
     nonEmptyArrayLiteralOrNull(def.getSource())
   )
 select f, "The array constant '" + f.getName() + "' is vulnerable to mutation."

csharp/ql/src/Bad Practices/LeftoverDebugCode.ql

@@ -10,11 +10,15 @@
  *       frameworks/asp.net
  *       external/cwe/cwe-489
  */
+
 import csharp
 import semmle.code.csharp.commons.Util
 
 from MainMethod m
-where m.fromSource()
+where
-  and exists(UsingNamespaceDirective u | u.getFile() = m.getFile()
+  m.fromSource() and
-                    and u.getImportedNamespace().hasQualifiedName("System.Web"))
+  exists(UsingNamespaceDirective u |
+    u.getFile() = m.getFile() and
+    u.getImportedNamespace().hasQualifiedName("System.Web")
+  )
 select m, "Remove debug code if your ASP.NET application is in production."

csharp/ql/src/Bad Practices/Magic Constants/MagicConstantsNumbers.ql

@@ -8,15 +8,15 @@
  * @tags changeability
  *       maintainability
  */
+
 import csharp
 import MagicConstants
 
-predicate selection(Element e, string msg) {
+predicate selection(Element e, string msg) { magicConstant(e, msg) }
-  magicConstant(e, msg)
-}
 
 from Literal e, string msg
-where selection(e, msg)
+where
-  and isNumber(e)
+  selection(e, msg) and
-  and not exists(Field f | f.getInitializer() = e)
+  isNumber(e) and
+  not exists(Field f | f.getInitializer() = e)
 select e, msg

csharp/ql/src/Bad Practices/Magic Constants/MagicConstantsString.ql

@@ -11,9 +11,7 @@
 
 import MagicConstants
 
-predicate selection(Element e, string msg) {
+predicate selection(Element e, string msg) { magicConstant(e, msg) }
-  magicConstant(e, msg)
-}
 
 from StringLiteral e, string msg
 where selection(e, msg)

csharp/ql/src/Bad Practices/Magic Constants/MagicNumbersUseConstant.ql

@@ -12,6 +12,7 @@
 import MagicConstants
 
 from Literal magicLiteral, string message, Field constant
-where isNumber(magicLiteral)
+where
-  and literalInsteadOfConstant(magicLiteral, _, message, constant)
+  isNumber(magicLiteral) and
+  literalInsteadOfConstant(magicLiteral, _, message, constant)
 select magicLiteral, message, constant, constant.getName()

csharp/ql/src/Bad Practices/Naming Conventions/ConfusingMethodNames.ql

@@ -9,6 +9,7 @@
  *       readability
  *       naming
  */
+
 import csharp
 
 predicate typeWithConfusingName(ValueOrRefType type) {
@@ -16,11 +17,12 @@ predicate typeWithConfusingName(ValueOrRefType type) {
 }
 
 from Method m, Method n, ValueOrRefType type
-where typeWithConfusingName(type)
+where
-  and type.fromSource()
+  typeWithConfusingName(type) and
-  and m = type.getAMethod()
+  type.fromSource() and
-  and n = type.getAMethod()
+  m = type.getAMethod() and
-  and m != n
+  n = type.getAMethod() and
-  and m.getName().toLowerCase() = n.getName().toLowerCase()
+  m != n and
-  and m.getName() < n.getName()
+  m.getName().toLowerCase() = n.getName().toLowerCase() and
+  m.getName() < n.getName()
 select m, "Confusing method name, compare with $@.", n, n.getName()

csharp/ql/src/Bad Practices/Naming Conventions/ConfusingOverridesNames.ql

@@ -10,15 +10,18 @@
  *       readability
  *       naming
  */
+
 import csharp
 
 predicate hasSubtypeStar(RefType t, RefType u) {
-  t = u
+  t = u or
-  or u.getABaseType+() = t
+  u.getABaseType+() = t
 }
 
-/** for each class, get all methods from this class or its
+/**
-    superclasses, with their names in lowercase */
+ * for each class, get all methods from this class or its
+ *    superclasses, with their names in lowercase
+ */
 predicate methodNames(RefType t, Method m, string lowercase) {
   exists(RefType t2 |
     m.getDeclaringType() = t2 and
@@ -28,8 +31,10 @@ predicate methodNames(RefType t, Method m, string lowercase) {
   lowercase.length() > 1
 }
 
-/** For each class, find the pairs of methods that
+/**
-    are candidates for being confusing in this class */
+ * For each class, find the pairs of methods that
+ *    are candidates for being confusing in this class
+ */
 predicate confusing(Method m1, Method m2) {
   exists(RefType t, string lower |
     methodNames(t, m1, lower) and
@@ -38,33 +43,37 @@ predicate confusing(Method m1, Method m2) {
   )
 }
 
-/* Two method names are confusing if all of the following conditions hold:
+/*
-*   They are both static methods or both instance methods.
+ * Two method names are confusing if all of the following conditions hold:
-*   They are not declared in the same class, and the superclass method is
+ *   They are both static methods or both instance methods.
-*     not overridden in an intermediate class
+ *   They are not declared in the same class, and the superclass method is
-*   They have different names.
+ *     not overridden in an intermediate class
-*   They have the same names if case is ignored.
+ *   They have different names.
-*   There is no method in the subclass that has the same name as
+ *   They have the same names if case is ignored.
-*     the superclass method
+ *   There is no method in the subclass that has the same name as
-There is an additional check that only methods with names longer than one character
+ *     the superclass method
-can be considered confusing. */
+ * There is an additional check that only methods with names longer than one character
+ * can be considered confusing.
+ */
 
 from Method m1, Method m2
-where confusing(m1,m2) and
+where
-      m1.getDeclaringType() != m2.getDeclaringType() and
+  confusing(m1, m2) and
-      (
+  m1.getDeclaringType() != m2.getDeclaringType() and
-       m1.isStatic() and m2.isStatic()
+  (
-       or
+    m1.isStatic() and m2.isStatic()
-       not m1.isStatic() and not m2.isStatic()
+    or
-      ) and
+    not m1.isStatic() and not m2.isStatic()
-      not exists(Method mid |
+  ) and
-        confusing(m1, mid) and
+  not exists(Method mid |
-        mid.getDeclaringType().getABaseType+() = m2.getDeclaringType()
+    confusing(m1, mid) and
-      ) and
+    mid.getDeclaringType().getABaseType+() = m2.getDeclaringType()
-      not exists(Method notConfusing |
+  ) and
-        notConfusing.getDeclaringType() = m1.getDeclaringType() and
+  not exists(Method notConfusing |
-        notConfusing.getName() = m2.getName()
+    notConfusing.getDeclaringType() = m1.getDeclaringType() and
-      ) and
+    notConfusing.getName() = m2.getName()
-      m1.fromSource()
+  ) and
-select m1, "confusing to have methods " + m1.getName() + " in " + m1.getDeclaringType().getName() + " and "
+  m1.fromSource()
-                                        + m2.getName() + " in " + m2.getDeclaringType().getName() + "."
+select m1,
+  "confusing to have methods " + m1.getName() + " in " + m1.getDeclaringType().getName() + " and " +
+    m2.getName() + " in " + m2.getDeclaringType().getName() + "."

csharp/ql/src/Bad Practices/Naming Conventions/ConstantNaming.ql

@@ -23,13 +23,12 @@ class PublicConstantField extends Field {
 from PublicConstantField f
 where
   // The first character of the field's name is not uppercase.
-  not(f.getName().charAt(0).isUppercase())
+  not (f.getName().charAt(0).isUppercase())
   or
   (
     // The field's name is uppercase.
-    f.getName().isUppercase()
+    f.getName().isUppercase() and
-
     // The field's name is at least 4 characters long.
-    and f.getName().length() >= 4
+    f.getName().length() >= 4
   )
 select f, "Public static read-only fields should be named in PascalCase."

csharp/ql/src/Bad Practices/Naming Conventions/ControlNamePrefixes.ql

@@ -12,51 +12,89 @@
 import csharp
 
 string prefix(string typename) {
-  (typename = "System.Web.UI.WebControls.Label" and result = "lbl") or
+  (typename = "System.Web.UI.WebControls.Label" and result = "lbl")
-  (typename = "System.Web.UI.WebControls.TextBox" and result = "txt") or
+  or
-  (typename = "System.Web.UI.WebControls.Button" and result = "btn") or
+  (typename = "System.Web.UI.WebControls.TextBox" and result = "txt")
-  (typename = "System.Web.UI.WebControls.LinkButton" and result = "btn") or
+  or
-  (typename = "System.Web.UI.WebControls.ImageButton" and result = "ibtn") or
+  (typename = "System.Web.UI.WebControls.Button" and result = "btn")
-  (typename = "System.Web.UI.WebControls.Hyperlink" and result = "hpl") or
+  or
-  (typename = "System.Web.UI.WebControls.DropDownList" and result = "cmb") or
+  (typename = "System.Web.UI.WebControls.LinkButton" and result = "btn")
-  (typename = "System.Web.UI.WebControls.ListBox" and result = "lst") or
+  or
-  (typename = "System.Web.UI.WebControls.Datagrid" and result = "dgr") or
+  (typename = "System.Web.UI.WebControls.ImageButton" and result = "ibtn")
-  (typename = "System.Web.UI.WebControls.Datalist" and result = "dtl") or
+  or
-  (typename = "System.Web.UI.WebControls.Repeater" and result = "rpt") or
+  (typename = "System.Web.UI.WebControls.Hyperlink" and result = "hpl")
-  (typename = "System.Web.UI.WebControls.CheckBox" and result = "chk") or
+  or
-  (typename = "System.Web.UI.WebControls.CheckBoxList" and result = "chklst") or
+  (typename = "System.Web.UI.WebControls.DropDownList" and result = "cmb")
-  (typename = "System.Web.UI.WebControls.RadioButtonList" and result = "radlst") or
+  or
-  (typename = "System.Web.UI.WebControls.RadioButton" and result = "rad") or
+  (typename = "System.Web.UI.WebControls.ListBox" and result = "lst")
-  (typename = "System.Web.UI.WebControls.Image" and result = "img") or
+  or
-  (typename = "System.Web.UI.WebControls.Panel" and result = "pnl") or
+  (typename = "System.Web.UI.WebControls.Datagrid" and result = "dgr")
-  (typename = "System.Web.UI.WebControls.PlaceHolder" and result = "plh") or
+  or
-  (typename = "System.Web.UI.WebControls.Calendar" and result = "cal") or
+  (typename = "System.Web.UI.WebControls.Datalist" and result = "dtl")
-  (typename = "System.Web.UI.WebControls.AdRotator" and result = "adr") or
+  or
-  (typename = "System.Web.UI.WebControls.Table" and result = "tbl") or
+  (typename = "System.Web.UI.WebControls.Repeater" and result = "rpt")
-  (typename = "System.Web.UI.WebControls.RequiredFieldValidator" and result = "rfv") or
+  or
-  (typename = "System.Web.UI.WebControls.CompareValidator" and result = "cmv") or
+  (typename = "System.Web.UI.WebControls.CheckBox" and result = "chk")
-  (typename = "System.Web.UI.WebControls.RegularExpressionValidator" and result = "rev") or
+  or
-  (typename = "System.Web.UI.WebControls.CustomValidator" and result = "csv") or
+  (typename = "System.Web.UI.WebControls.CheckBoxList" and result = "chklst")
-  (typename = "System.Web.UI.WebControls.ValidationSummary" and result = "vsm") or
+  or
-  (typename = "System.Web.UI.WebControls.XML" and result = "xml") or
+  (typename = "System.Web.UI.WebControls.RadioButtonList" and result = "radlst")
-  (typename = "System.Web.UI.WebControls.Literal" and result = "lit") or
+  or
-  (typename = "System.Web.UI.WebControls.Form" and result = "frm") or
+  (typename = "System.Web.UI.WebControls.RadioButton" and result = "rad")
-  (typename = "System.Web.UI.WebControls.Frame" and result = "fra") or
+  or
-  (typename = "System.Web.UI.WebControls.CrystalReportViewer" and result = "crvr") or
+  (typename = "System.Web.UI.WebControls.Image" and result = "img")
-
+  or
-  (typename = "System.Web.UI.HtmlControls.TextArea" and result = "txa") or
+  (typename = "System.Web.UI.WebControls.Panel" and result = "pnl")
-  (typename = "System.Web.UI.HtmlControls.FileField" and result = "fle") or
+  or
-  (typename = "System.Web.UI.HtmlControls.PasswordField" and result = "pwd") or
+  (typename = "System.Web.UI.WebControls.PlaceHolder" and result = "plh")
-  (typename = "System.Web.UI.HtmlControls.Hidden" and result = "hdn") or
+  or
-  (typename = "System.Web.UI.HtmlControls.Table" and result = "tbl") or
+  (typename = "System.Web.UI.WebControls.Calendar" and result = "cal")
-  (typename = "System.Web.UI.HtmlControls.FlowLayoutPanel" and result = "flp") or
+  or
-  (typename = "System.Web.UI.HtmlControls.GridLayoutPanel" and result = "glp") or
+  (typename = "System.Web.UI.WebControls.AdRotator" and result = "adr")
+  or
+  (typename = "System.Web.UI.WebControls.Table" and result = "tbl")
+  or
+  (typename = "System.Web.UI.WebControls.RequiredFieldValidator" and result = "rfv")
+  or
+  (typename = "System.Web.UI.WebControls.CompareValidator" and result = "cmv")
+  or
+  (typename = "System.Web.UI.WebControls.RegularExpressionValidator" and result = "rev")
+  or
+  (typename = "System.Web.UI.WebControls.CustomValidator" and result = "csv")
+  or
+  (typename = "System.Web.UI.WebControls.ValidationSummary" and result = "vsm")
+  or
+  (typename = "System.Web.UI.WebControls.XML" and result = "xml")
+  or
+  (typename = "System.Web.UI.WebControls.Literal" and result = "lit")
+  or
+  (typename = "System.Web.UI.WebControls.Form" and result = "frm")
+  or
+  (typename = "System.Web.UI.WebControls.Frame" and result = "fra")
+  or
+  (typename = "System.Web.UI.WebControls.CrystalReportViewer" and result = "crvr")
+  or
+  (typename = "System.Web.UI.HtmlControls.TextArea" and result = "txa")
+  or
+  (typename = "System.Web.UI.HtmlControls.FileField" and result = "fle")
+  or
+  (typename = "System.Web.UI.HtmlControls.PasswordField" and result = "pwd")
+  or
+  (typename = "System.Web.UI.HtmlControls.Hidden" and result = "hdn")
+  or
+  (typename = "System.Web.UI.HtmlControls.Table" and result = "tbl")
+  or
+  (typename = "System.Web.UI.HtmlControls.FlowLayoutPanel" and result = "flp")
+  or
+  (typename = "System.Web.UI.HtmlControls.GridLayoutPanel" and result = "glp")
+  or
   (typename = "System.Web.UI.HtmlControls.HorizontalRule" and result = "hr")
 }
 
 from Field f, RefType t, string name, string prefix
-where f.getType() = t
+where
-  and f.getName() = name
+  f.getType() = t and
-  and prefix = prefix(t.getQualifiedName())
+  f.getName() = name and
-  and not name.matches(prefix + "%")
+  prefix = prefix(t.getQualifiedName()) and
+  not name.matches(prefix + "%")
 select f, "This field should have the prefix '" + prefix + "' to match its types."

csharp/ql/src/Bad Practices/Naming Conventions/DefaultControlNames.ql

@@ -1,7 +1,7 @@
 /**
  * @name Windows controls with generated names
  * @description Replacing the generated names in windows forms with meaningful names
-                makes it easier for other developers to understand the code.
+ *                makes it easier for other developers to understand the code.
  * @kind problem
  * @problem.severity recommendation
  * @precision medium
@@ -9,10 +9,10 @@
  * @tags readability
  *       naming
  */
+
 import csharp
 
-predicate controlName(string prefix)
+predicate controlName(string prefix) {
-{
   prefix = "[Ll]abel" or
   prefix = "[Bb]utton" or
   prefix = "[Pp]anel" or
@@ -28,13 +28,15 @@ predicate controlName(string prefix)
 predicate usedInHumanWrittenCode(Field f) {
   exists(File file |
     f.getAnAccess().getFile() = file and
-    not file.getBaseName().toLowerCase().matches("%.designer.cs"))
+    not file.getBaseName().toLowerCase().matches("%.designer.cs")
+  )
 }
 
 from Field field, ValueOrRefType widget, string prefix
-where widget.getABaseType*().hasQualifiedName("System.Windows.Forms.Control")
+where
-  and field.getType() = widget
+  widget.getABaseType*().hasQualifiedName("System.Windows.Forms.Control") and
-  and field.getName().regexpMatch(prefix + "[0-9]+")
+  field.getType() = widget and
-  and controlName(prefix)
+  field.getName().regexpMatch(prefix + "[0-9]+") and
-  and usedInHumanWrittenCode(field)
+  controlName(prefix) and
+  usedInHumanWrittenCode(field)
 select field, "Control '" + field.getName() + "' should have a meaningful name."

csharp/ql/src/Bad Practices/Naming Conventions/FieldMasksSuperField.ql

@@ -11,6 +11,7 @@
  *       readability
  *       naming
  */
+
 import csharp
 
 class VisibleInstanceField extends Field {
@@ -20,14 +21,16 @@ class VisibleInstanceField extends Field {
   }
 }
 
-from RefType type, RefType supertype,
+from RefType type, RefType supertype, VisibleInstanceField masked, VisibleInstanceField masking
-     VisibleInstanceField masked, VisibleInstanceField masking
+where
-where type.getABaseType+() =supertype and
+  type.getABaseType+() = supertype and
-      masking.getDeclaringType() = type and
+  masking.getDeclaringType() = type and
-      masked.getDeclaringType() = supertype and
+  masked.getDeclaringType() = supertype and
-      masked.getName() = masking.getName() and
+  masked.getName() = masking.getName() and
-      // exclude intentional masking
+  // exclude intentional masking
-      not exists(FieldAccess va | va.getTarget() = masked and
+  not exists(FieldAccess va |
-                                va.getQualifier() instanceof BaseAccess) and
+    va.getTarget() = masked and
-      type.fromSource()
+    va.getQualifier() instanceof BaseAccess
+  ) and
+  type.fromSource()
 select masking, "This field shadows another field in a superclass."

csharp/ql/src/Bad Practices/Naming Conventions/SameNameAsSuper.ql

@@ -9,10 +9,12 @@
  *       readability
  *       naming
  */
+
 import csharp
 
 from RefType sub, RefType sup
-where sub.getABaseType() = sup and
+where
-      sub.getName() = sup.getName() and
+  sub.getABaseType() = sup and
-      sub.fromSource()
+  sub.getName() = sup.getName() and
+  sub.fromSource()
 select sub, "Class has the same name as its base class."

csharp/ql/src/Bad Practices/Naming Conventions/VariableNameTooShort.ql

@@ -10,50 +10,56 @@
 
 import csharp
 
-predicate sourceFile(File f)
+predicate sourceFile(File f) {
-{
   f.fromSource() and
   not f.getAbsolutePath() = "null" and
   not f.getAbsolutePath().matches("%ActivXTabControl%.cs")
 }
 
 from Variable variable, string name
-where name = variable.getName()
+where
-  and variable.fromSource()
+  name = variable.getName() and
-  and sourceFile(variable.getFile())
+  variable.fromSource() and
-  and not allowedName(name)
+  sourceFile(variable.getFile()) and
-  and not allowedVariable(variable)
+  not allowedName(name) and
-//
+  not allowedVariable(variable) and
-// Adjustable parameter:
+  //
-//
+  // Adjustable parameter:
-  and name.length() < 3
+  //
+  name.length() < 3
 //
 select variable, "Variable name '" + name + "' is too short."
 
 //
 // Adjustable: acceptable short names
 //
-predicate allowedName(string name)
+predicate allowedName(string name) {
-{
+  name = "url" or
-  name = "url" or name = "cmd" or name = "UK" or name = "uri" or
+  name = "cmd" or
-  name = "top" or name = "row" or name = "pin" or name = "log" or
+  name = "UK" or
-  name = "key" or name = "_"
+  name = "uri" or
+  name = "top" or
+  name = "row" or
+  name = "pin" or
+  name = "log" or
+  name = "key" or
+  name = "_"
 }
 
 //
 // Adjustable: variables that are allowed to have short names
 //
-predicate allowedVariable(Variable variable)
+predicate allowedVariable(Variable variable) {
-{
+  exists(Parameter param |
-  exists(Parameter param | variable = param and
+    variable = param and
     not exists(param.getAnAccess()) and
-    param.getType().getName().matches("%EventArgs"))
+    param.getType().getName().matches("%EventArgs")
-  or
-  exists(LocalVariable local | variable = local and
-    local.getVariableDeclExpr().getParent() instanceof CatchClause)
-  or
-  exists(Call c, LambdaExpr le |
-    le.getAParameter() = variable |
-    c.getAnArgument() = le
   )
+  or
+  exists(LocalVariable local |
+    variable = local and
+    local.getVariableDeclExpr().getParent() instanceof CatchClause
+  )
+  or
+  exists(Call c, LambdaExpr le | le.getAParameter() = variable | c.getAnArgument() = le)
 }

csharp/ql/src/Bad Practices/UnmanagedCodeCheck.ql

@@ -12,6 +12,7 @@
 import csharp
 
 from Class c, Method m
-where m.isExtern()
+where
-  and m.getDeclaringType() = c
+  m.isExtern() and
+  m.getDeclaringType() = c
 select m, "Minimise the use of unmanaged code."

csharp/ql/src/Bad Practices/UseOfHtmlInputHidden.ql

@@ -10,6 +10,7 @@
  *       frameworks/asp.net
  *       external/cwe/cwe-472
  */
+
 import csharp
 
 from ObjectCreation oc

csharp/ql/src/Bad Practices/UseOfSystemOutputStream.ql

@@ -7,17 +7,22 @@
  * @id cs/console-output
  * @tags maintainability
  */
+
 import csharp
 import semmle.code.csharp.commons.Util
 
 predicate isConsoleOutRedefinedSomewhere() {
-  exists(MethodCall mc | mc.getTarget().hasName("SetOut") and
+  exists(MethodCall mc |
-                         mc.getTarget().getDeclaringType().hasQualifiedName("System.Console"))
+    mc.getTarget().hasName("SetOut") and
+    mc.getTarget().getDeclaringType().hasQualifiedName("System.Console")
+  )
 }
 
 predicate isConsoleErrorRedefinedSomewhere() {
-  exists(MethodCall mc | mc.getTarget().hasName("SetError") and
+  exists(MethodCall mc |
-                         mc.getTarget().getDeclaringType().hasQualifiedName("System.Console"))
+    mc.getTarget().hasName("SetError") and
+    mc.getTarget().getDeclaringType().hasQualifiedName("System.Console")
+  )
 }
 
 predicate isCallToConsoleWrite(MethodCall mc) {
@@ -36,8 +41,13 @@ predicate isAccessToConsoleError(PropertyAccess pa) {
 }
 
 from Expr e
-where (isCallToConsoleWrite(e) and not isConsoleOutRedefinedSomewhere()
+where
-   or isAccessToConsoleOut(e) and not isConsoleOutRedefinedSomewhere()
+  (
-   or isAccessToConsoleError(e) and not isConsoleErrorRedefinedSomewhere())
+    isCallToConsoleWrite(e) and not isConsoleOutRedefinedSomewhere()
-   and not e.getEnclosingCallable() instanceof MainMethod
+    or
+    isAccessToConsoleOut(e) and not isConsoleOutRedefinedSomewhere()
+    or
+    isAccessToConsoleError(e) and not isConsoleErrorRedefinedSomewhere()
+  ) and
+  not e.getEnclosingCallable() instanceof MainMethod
 select e, "Poor logging: use of system output stream."

csharp/ql/src/Bad Practices/VirtualCallInConstructorOrDestructor.ql

@@ -10,41 +10,38 @@
  *       maintainability
  *       modularity
  */
+
 import csharp
 
 predicate virtualCallToSelfInConstructor(Expr e) {
   exists(RefType t, Virtualizable d, Callable c |
-    c = e.getEnclosingCallable()
+    c = e.getEnclosingCallable() and
-    and
+    (c instanceof Constructor or c instanceof Destructor) and
-    (c instanceof Constructor or c instanceof Destructor)
+    t = c.getDeclaringType() and
-    and
+    virtualAccessWithThisQualifier(e, d) and
-    t = c.getDeclaringType()
+    t.getABaseType*() = d.getDeclaringType() and
-    and
+    not t.isSealed() and
-    virtualAccessWithThisQualifier(e, d)
-    and
-    t.getABaseType*() = d.getDeclaringType()
-    and
-    not t.isSealed()
-    and
     not overriddenSealed(t.getABaseType*(), d)
   )
 }
 
 predicate overriddenSealed(RefType t, Virtualizable d) {
   exists(Virtualizable od |
-    od.getDeclaringType() = t
+    od.getDeclaringType() = t and
-    and
+    (od.getOverridee() = d or od.getImplementee() = d) and
-    (od.getOverridee() = d or od.getImplementee() = d)
-    and
     not od.isOverridableOrImplementable()
   )
 }
 
 predicate virtualAccessWithThisQualifier(Expr e, Member d) {
-  exists(VirtualMethodCall c | c = e and c.getTarget() = d and c.hasThisQualifier()) or
+  exists(VirtualMethodCall c | c = e and c.getTarget() = d and c.hasThisQualifier())
-  exists(VirtualMethodAccess c | c = e and c.getTarget() = d and c.hasThisQualifier()) or
+  or
-  exists(VirtualPropertyAccess c | c = e and c.getTarget() = d and c.hasThisQualifier()) or
+  exists(VirtualMethodAccess c | c = e and c.getTarget() = d and c.hasThisQualifier())
-  exists(VirtualIndexerAccess c | c = e and c.getTarget() = d and c.hasThisQualifier()) or
+  or
+  exists(VirtualPropertyAccess c | c = e and c.getTarget() = d and c.hasThisQualifier())
+  or
+  exists(VirtualIndexerAccess c | c = e and c.getTarget() = d and c.hasThisQualifier())
+  or
   exists(VirtualEventAccess c | c = e and c.getTarget() = d and c.hasThisQualifier())
 }
 

csharp/ql/src/CSI/CompareIdenticalValues.ql

@@ -16,8 +16,7 @@ import semmle.code.csharp.commons.ComparisonTest
 import semmle.code.csharp.commons.Constants
 
 predicate isBoxingTestCase(StaticEqualsCallComparisonTest ct) {
-  ct.isReferenceEquals()
+  ct.isReferenceEquals() and
-  and
   exists(TypeParameter tp |
     tp = ct.getFirstArgument().stripCasts().getType() and
     not tp.isValueType() and
@@ -25,8 +24,7 @@ predicate isBoxingTestCase(StaticEqualsCallComparisonTest ct) {
   )
 }
 
-predicate isMutatingOperation(Expr e)
+predicate isMutatingOperation(Expr e) {
-{
   e.(MethodCall).getTarget().hasName("Pop")
   or
   e.(MethodCall).getTarget().hasName("Push")
@@ -35,16 +33,19 @@ predicate isMutatingOperation(Expr e)
 }
 
 from ComparisonTest ct, Expr e, string msg
-where comparesIdenticalValues(ct)
+where
-  and e = ct.getExpr()
+  comparesIdenticalValues(ct) and
-  and not isBoxingTestCase(ct)
+  e = ct.getExpr() and
-  and (
+  not isBoxingTestCase(ct) and
-    exists(string m | comparesIdenticalValuesNan(ct, m) | msg = "Comparison is equivalent to using " + m)
+  (
+    exists(string m | comparesIdenticalValuesNan(ct, m) |
+      msg = "Comparison is equivalent to using " + m
+    )
     or
     not comparesIdenticalValuesNan(ct, _) and msg = "Comparison of identical values."
-  )
+  ) and
-  and not isMutatingOperation(ct.getAnArgument().getAChild*())
+  not isMutatingOperation(ct.getAnArgument().getAChild*()) and
-  and not isConstantCondition(e, _) // Avoid overlap with cs/constant-condition
+  not isConstantCondition(e, _) and // Avoid overlap with cs/constant-condition
-  and not isConstantComparison(e, _) // Avoid overlap with cs/constant-comparison
+  not isConstantComparison(e, _) and // Avoid overlap with cs/constant-comparison
-  and not isExprInAssertion(e)
+  not isExprInAssertion(e)
 select ct, msg

csharp/ql/src/CSI/NullMaybe.ql

@@ -16,6 +16,7 @@ import csharp
 import semmle.code.csharp.dataflow.Nullness
 import PathGraph
 
-from Dereference d, PathNode source, PathNode sink, Ssa::SourceVariable v, string msg, Element reason
+from
+  Dereference d, PathNode source, PathNode sink, Ssa::SourceVariable v, string msg, Element reason
 where d.isFirstMaybeNull(v.getAnSsaDefinition(), source, sink, msg, reason)
 select d, source, sink, "Variable $@ may be null here " + msg + ".", v, v.toString(), reason, "this"

csharp/ql/src/Complexity/BlockWithTooManyStatements.ql

@@ -11,14 +11,17 @@
  *       testability
  *       complexity
  */
+
 import csharp
 
 class ComplexStmt extends Stmt {
   ComplexStmt() {
-    (this instanceof ForStmt or
+    (
-     this instanceof WhileStmt or
+      this instanceof ForStmt or
-     this instanceof DoStmt or
+      this instanceof WhileStmt or
-     this instanceof SwitchStmt)
+      this instanceof DoStmt or
+      this instanceof SwitchStmt
+    )
   }
 }
 

csharp/ql/src/Complexity/ComplexCondition.ql

@@ -8,6 +8,7 @@
  * @tags testability
  *       readability
  */
+
 import csharp
 
 predicate nontrivialLogicalOperator(BinaryLogicalOperation e) {
@@ -17,13 +18,13 @@ predicate nontrivialLogicalOperator(BinaryLogicalOperation e) {
   )
 }
 
-predicate logicalParent(LogicalOperation op, LogicalOperation parent)
+predicate logicalParent(LogicalOperation op, LogicalOperation parent) { parent = op.getParent() }
-{
-  parent = op.getParent()
-}
 
 from Expr e, int operators
-where not (e.getParent() instanceof LogicalOperation)
+where
-      and operators = count(BinaryLogicalOperation op | logicalParent*(op, e) and nontrivialLogicalOperator(op))
+  not (e.getParent() instanceof LogicalOperation) and
-      and operators > 3
+  operators = count(BinaryLogicalOperation op |
+      logicalParent*(op, e) and nontrivialLogicalOperator(op)
+    ) and
+  operators > 3
 select e.getLocation(), "Complex condition: too many logical operations in this expression."

csharp/ql/src/Concurrency/FutileSyncOnField.ql

@@ -21,8 +21,10 @@ predicate lockedFieldUpdate(LockStmt lock, Field f, AssignableDefinition def) {
 }
 
 from LockStmt lock, Expr e, Field f, AssignableDefinition def
-where e = lock.getExpr()
+where
-  and f.getAnAccess() = e
+  e = lock.getExpr() and
-  and lockedFieldUpdate(lock, f, def)
+  f.getAnAccess() = e and
-select e, "Locking field $@ guards the initial value, not the value which may be seen from another thread after $@.",
+  lockedFieldUpdate(lock, f, def)
+select e,
+  "Locking field $@ guards the initial value, not the value which may be seen from another thread after $@.",
   f, f.getName(), def, "reassignment"

csharp/ql/src/Concurrency/LockOrder.ql

@@ -38,7 +38,8 @@ LockStmt getAReachableLockStmt(Callable callable) {
 predicate nestedLocks(Variable outerVariable, Variable innerVariable, LockStmt outer, LockStmt inner) {
   outerVariable = outer.getLockVariable() and
   innerVariable = inner.getLockVariable() and
-  outerVariable != innerVariable and (
+  outerVariable != innerVariable and
+  (
     inner = outer.getALockedStmt()
     or
     exists(Call call | call.getEnclosingStmt() = outer.getALockedStmt() |
@@ -54,9 +55,6 @@ where
   nestedLocks(v1, v2, outer1, inner1) and
   nestedLocks(v2, v1, outer2, inner2) and
   v1.getName() <= v2.getName()
-select v1, "Inconsistent lock sequence with $@. Lock sequences $@, $@ and $@, $@ found.",
+select v1, "Inconsistent lock sequence with $@. Lock sequences $@, $@ and $@, $@ found.", v2,
-  v2, v2.getName(),
+  v2.getName(), outer1, v1.getName(), inner1, v2.getName(), outer2, v2.getName(), inner2,
-  outer1, v1.getName(),
+  v1.getName()
-  inner1, v2.getName(),
-  outer2, v2.getName(),
-  inner2, v1.getName()

csharp/ql/src/Concurrency/LockThis.ql

@@ -15,5 +15,5 @@
 import csharp
 
 from LockStmt s, ThisAccess a
-where a=s.getExpr()
+where a = s.getExpr()
 select a, "'this' used in lock statement."

csharp/ql/src/Concurrency/LockedWait.ql

@@ -16,10 +16,19 @@ import csharp
 import Concurrency
 
 from LockedBlock l, WaitStmt w, string lockedItem
-where l.getALockedStmt()=w and
+where
+  l.getALockedStmt() = w and
   (
-    exists( Variable v | v=l.getLockVariable() and not v=w.getWaitVariable() and lockedItem=v.getName() ) or
+    exists(Variable v |
-    exists( Type t | t=l.getLockTypeObject() and not t=w.getWaitTypeObject() and lockedItem = "typeof("+t.getName()+")" ) or
+      v = l.getLockVariable() and not v = w.getWaitVariable() and lockedItem = v.getName()
-    ( l.isLockThis() and not w.isWaitThis() and lockedItem="this" )
+    )
+    or
+    exists(Type t |
+      t = l.getLockTypeObject() and
+      not t = w.getWaitTypeObject() and
+      lockedItem = "typeof(" + t.getName() + ")"
+    )
+    or
+    (l.isLockThis() and not w.isWaitThis() and lockedItem = "this")
   )
 select w, "'" + lockedItem + "' is locked during this wait."

csharp/ql/src/Concurrency/SynchSetUnsynchGet.ql

@@ -15,26 +15,17 @@ import csharp
 
 from Property p, Field f
 where
-  f.getDeclaringType() = p.getDeclaringType()
+  f.getDeclaringType() = p.getDeclaringType() and
-  and
   exists(Setter setter, LockStmt writelock, FieldWrite writeaccess |
-    p.getSetter() = setter
+    p.getSetter() = setter and
-    and
+    writeaccess = f.getAnAccess() and
-    writeaccess = f.getAnAccess()
+    writelock.getEnclosingCallable() = setter and
-    and
-    writelock.getEnclosingCallable() = setter
-    and
     writelock.getAChildStmt+().getAChildExpr+() = writeaccess
-  )
+  ) and
-  and
   exists(Getter getter, FieldRead readaccess |
-    getter = p.getGetter()
+    getter = p.getGetter() and
-    and
+    readaccess = f.getAnAccess() and
-    readaccess = f.getAnAccess()
+    readaccess.getEnclosingCallable() = getter and
-    and
+    not exists(LockStmt readlock | readlock.getAChildStmt+().getAChildExpr+() = readaccess)
-    readaccess.getEnclosingCallable() = getter
+  )
-    and
-    not exists(LockStmt readlock |
-      readlock.getAChildStmt+().getAChildExpr+() = readaccess)
-    )
 select p, "Field '$@' is guarded by a lock in the setter but not in the getter.", f, f.getName()

csharp/ql/src/Concurrency/UnsafeLazyInitialization.ql

@@ -15,7 +15,7 @@ import csharp
 import semmle.code.csharp.commons.StructuralComparison
 
 class DoubleCheckedLock extends StructuralComparisonConfiguration {
-  DoubleCheckedLock() { this="double checked lock" }
+  DoubleCheckedLock() { this = "double checked lock" }
 
   override predicate candidate(Element x, Element y) {
     exists(IfStmt unlockedIf, IfStmt lockedIf, LockStmt lock |
@@ -28,8 +28,7 @@ class DoubleCheckedLock extends StructuralComparisonConfiguration {
 }
 
 predicate doubleCheckedLock(Field field, IfStmt ifs) {
-  exists(DoubleCheckedLock config, LockStmt lock, Expr eq1, Expr eq2 |
+  exists(DoubleCheckedLock config, LockStmt lock, Expr eq1, Expr eq2 | ifs.getCondition() = eq1 |
-    ifs.getCondition() = eq1 |
     lock = ifs.getThen().stripSingletonBlocks() and
     config.same(eq1, eq2) and
     field.getAnAccess() = eq1.getAChildExpr*()
@@ -37,9 +36,7 @@ predicate doubleCheckedLock(Field field, IfStmt ifs) {
 }
 
 from Field field, IfStmt ifs
-where doubleCheckedLock(field, ifs)
+where
-and not field.isVolatile()
+  doubleCheckedLock(field, ifs) and
-select ifs, "Field $@ should be 'volatile' for this double-checked lock.",
+  not field.isVolatile()
-  field, field.getName()
+select ifs, "Field $@ should be 'volatile' for this double-checked lock.", field, field.getName()
-
-

csharp/ql/src/Concurrency/UnsynchronizedStaticAccess.ql

@@ -10,6 +10,7 @@
  *       external/cwe/cwe-362
  *       external/cwe/cwe-567
  */
+
 import csharp
 import DataMembers
 import ThreadCreation
@@ -24,7 +25,8 @@ predicate correctlySynchronized(CollectionMember c, Expr access) {
 }
 
 ControlFlow::Node unlockedReachable(Callable a) {
-  result = a.getEntryPoint() or
+  result = a.getEntryPoint()
+  or
   exists(ControlFlow::Node mid | mid = unlockedReachable(a) |
     not mid.getElement() instanceof LockingCall and
     result = mid.getASuccessor()
@@ -40,22 +42,22 @@ predicate unlockedCalls(Callable a, Callable b) {
 }
 
 predicate writtenStaticDictionary(CollectionMember c) {
-  c.getType().(ValueOrRefType).getABaseType*().hasName("IDictionary")
+  c.getType().(ValueOrRefType).getABaseType*().hasName("IDictionary") and
-  and c.isStatic()
+  c.isStatic() and
-  and exists(Expr write | write = c.getAWrite() |
+  exists(Expr write | write = c.getAWrite() |
     not write.getEnclosingCallable() instanceof StaticConstructor
   )
 }
 
-predicate nonStaticCallable(Callable c) {
+predicate nonStaticCallable(Callable c) { not c.(Modifiable).isStatic() }
-  not c.(Modifiable).isStatic()
-}
 
-from CollectionMember c, Expr a, ConcurrentEntryPoint e,Callable enclosing
+from CollectionMember c, Expr a, ConcurrentEntryPoint e, Callable enclosing
-where a = c.getAReadOrWrite()
+where
-  and enclosing = a.getEnclosingCallable()
+  a = c.getAReadOrWrite() and
-  and nonStaticCallable(enclosing)
+  enclosing = a.getEnclosingCallable() and
-  and not correctlySynchronized(c, a)
+  nonStaticCallable(enclosing) and
-  and unlockedCalls*(e, enclosing)
+  not correctlySynchronized(c, a) and
-  and writtenStaticDictionary(c)
+  unlockedCalls*(e, enclosing) and
-select a, "Unsynchronized access to $@ in non-static context from $@.", c, c.getName(), e, e.getName()
+  writtenStaticDictionary(c)
+select a, "Unsynchronized access to $@ in non-static context from $@.", c, c.getName(), e,
+  e.getName()

csharp/ql/src/Configuration/EmptyPasswordInConfigurationFile.ql

@@ -9,10 +9,14 @@
  *       external/cwe/cwe-258
  *       external/cwe/cwe-862
  */
+
 import csharp
 
 from XMLAttribute a
-where a.getName().toLowerCase() = "password" and a.getValue() = ""
+where
-   or a.getName().toLowerCase() = "pwd" and a.getValue() = ""
+  a.getName().toLowerCase() = "password" and a.getValue() = ""
-   or a.getValue().regexpMatch("(?is).*(pwd|password)\\s*=\\s*;.*")
+  or
+  a.getName().toLowerCase() = "pwd" and a.getValue() = ""
+  or
+  a.getValue().regexpMatch("(?is).*(pwd|password)\\s*=\\s*;.*")
 select a, "Do not use empty passwords."

csharp/ql/src/Configuration/PasswordInConfigurationFile.ql

@@ -10,10 +10,14 @@
  *       external/cwe/cwe-256
  *       external/cwe/cwe-313
  */
+
 import csharp
 
 from XMLAttribute a
-where a.getName().toLowerCase() = "password" and not a.getValue() = ""
+where
-   or a.getName().toLowerCase() = "pwd" and not a.getValue() = ""
+  a.getName().toLowerCase() = "password" and not a.getValue() = ""
-   or a.getValue().regexpMatch("(?is).*(pwd|password)\\s*=(?!\\s*;).*")
+  or
+  a.getName().toLowerCase() = "pwd" and not a.getValue() = ""
+  or
+  a.getValue().regexpMatch("(?is).*(pwd|password)\\s*=(?!\\s*;).*")
 select a, "Avoid plaintext passwords in configuration files."

csharp/ql/src/Dead Code/DeadRefTypes.ql

@@ -9,45 +9,43 @@
  *       useless-code
  *       external/cwe/cwe-561
  */
+
 import csharp
 import semmle.code.csharp.commons.Util
 import semmle.code.csharp.frameworks.Test
 import semmle.code.csharp.metrics.Coupling
 
-predicate potentiallyUsedFromXaml(RefType t)
+predicate potentiallyUsedFromXaml(RefType t) {
-{
   exists(string name | name = t.getABaseType*().getQualifiedName() |
-    name = "System.Windows.Data.IValueConverter"
+    name = "System.Windows.Data.IValueConverter" or
-    or name = "System.Windows.Data.IMultiValueConverter"
+    name = "System.Windows.Data.IMultiValueConverter"
   )
 }
 
-class ExportAttribute extends Attribute
+class ExportAttribute extends Attribute {
-{
+  ExportAttribute() {
-  ExportAttribute()
-  {
     getType().hasQualifiedName("System.ComponentModel.Composition.ExportAttribute")
   }
 }
 
 from RefType t
 where
-  not extractionIsStandalone()
+  not extractionIsStandalone() and
-  and t.fromSource()
+  t.fromSource() and
-  and t = t.getSourceDeclaration()
+  t = t.getSourceDeclaration() and
-  and not t instanceof AnonymousClass
+  not t instanceof AnonymousClass and
-  and not (t.isPublic() or t.isProtected())
+  not (t.isPublic() or t.isProtected()) and
-  and not exists (ValueOrRefType dependent | depends(dependent, t) and dependent != t)
+  not exists(ValueOrRefType dependent | depends(dependent, t) and dependent != t) and
-  and not exists (ConstructedType ct | usesType(ct, t))
+  not exists(ConstructedType ct | usesType(ct, t)) and
-  and not exists (MethodCall call | usesType(call.getTarget().(ConstructedMethod).getATypeArgument(), t))
+  not exists(MethodCall call | usesType(call.getTarget().(ConstructedMethod).getATypeArgument(), t)) and
-  and not t.getAMethod() instanceof MainMethod
+  not t.getAMethod() instanceof MainMethod and
-  and not potentiallyUsedFromXaml(t)
+  not potentiallyUsedFromXaml(t) and
-  and not exists(TypeofExpr typeof | typeof.getTypeAccess().getTarget() = t)
+  not exists(TypeofExpr typeof | typeof.getTypeAccess().getTarget() = t) and
-  and not t instanceof TestClass
+  not t instanceof TestClass and
   // MemberConstant nodes are compile-time constant and can appear in various contexts
   // where they don't have enclosing callables or types (e.g. in attribute values).
   // Classes that are declared purely to hold member constants which are used are,
   // therefore, not dead.
-  and not exists(t.getAMember().(MemberConstant).getAnAccess())
+  not exists(t.getAMember().(MemberConstant).getAnAccess()) and
-  and not t.getAnAttribute() instanceof ExportAttribute
+  not t.getAnAttribute() instanceof ExportAttribute
 select t, "Unused reference type " + t + "."

csharp/ql/src/Dead Code/DeadStoreOfLocal.ql

@@ -20,8 +20,7 @@ import csharp
 Callable getACapturingCallableAncestor(LocalVariable v) {
   result = v.getACapturingCallable()
   or
-  exists(Callable mid |
+  exists(Callable mid | mid = getACapturingCallableAncestor(v) |
-    mid = getACapturingCallableAncestor(v) |
     result = mid.getEnclosingCallable() and
     not v.getEnclosingCallable() = result
   )
@@ -37,8 +36,7 @@ Expr getADelegateExpr(Callable c) {
  * Holds if `c` is a call where any delegate argument is evaluated immediately.
  */
 predicate nonEscapingCall(Call c) {
-  exists(string name |
+  exists(string name | c.getTarget().hasName(name) |
-    c.getTarget().hasName(name) |
     name = "ForEach" or
     name = "Count" or
     name = "Any" or
@@ -62,8 +60,7 @@ predicate nonEscapingCall(Call c) {
  * `v` may escape the local scope.
  */
 predicate mayEscape(LocalVariable v) {
-  exists(Callable c, Expr e, Expr succ |
+  exists(Callable c, Expr e, Expr succ | c = getACapturingCallableAncestor(v) |
-    c = getACapturingCallableAncestor(v) |
     e = getADelegateExpr(c) and
     DataFlow::localFlow(DataFlow::exprNode(e), DataFlow::exprNode(succ)) and
     not succ = any(DelegateCall dc).getDelegateExpr() and
@@ -80,15 +77,15 @@ class RelevantDefinition extends AssignableDefinition {
     this instanceof AssignableDefinitions::MutationDefinition
     or
     this instanceof AssignableDefinitions::TupleAssignmentDefinition
+    or
     // Discards in out assignments are only possible from C# 7 (2017), so we disable this case
     // for now
     //or
     //this.(AssignableDefinitions::OutRefDefinition).getTargetAccess().isOutArgument()
-    or
     this.(AssignableDefinitions::LocalVariableDefinition).getDeclaration() = any(LocalVariableDeclExpr lvde |
-      lvde = any(SpecificCatchClause scc).getVariableDeclExpr() or
+        lvde = any(SpecificCatchClause scc).getVariableDeclExpr() or
-      lvde = any(ForeachStmt fs).getVariableDeclExpr()
+        lvde = any(ForeachStmt fs).getVariableDeclExpr()
-    )
+      )
     or
     this instanceof AssignableDefinitions::IsPatternDefinition
     or
@@ -97,8 +94,7 @@ class RelevantDefinition extends AssignableDefinition {
 
   /** Holds if this assignment may be live. */
   private predicate isMaybeLive() {
-    exists(LocalVariable v |
+    exists(LocalVariable v | v = this.getTarget() |
-      v = this.getTarget() |
       // SSA definitions are only created for live variables
       this = any(Ssa::ExplicitDefinition ssaDef).getADefinition()
       or
@@ -117,10 +113,8 @@ class RelevantDefinition extends AssignableDefinition {
    */
   private predicate isDefaultLikeInitializer() {
     this.isInitializer() and
-    exists(Expr e |
+    exists(Expr e | e = this.getSource() |
-      e = this.getSource() |
+      exists(string val | val = e.getValue() |
-      exists(string val |
-        val = e.getValue() |
         val = "0" or
         val = "-1" or
         val = "" or
@@ -129,7 +123,10 @@ class RelevantDefinition extends AssignableDefinition {
       or
       e instanceof NullLiteral
       or
-      e = any(Field f | f.isStatic() and (f.isReadOnly() or f.isConst())).getAnAccess()
+      e = any(Field f |
+          f.isStatic() and
+          (f.isReadOnly() or f.isConst())
+        ).getAnAccess()
       or
       e instanceof DefaultValueExpr
       or
@@ -151,8 +148,7 @@ class RelevantDefinition extends AssignableDefinition {
       (
         not this.isDefaultLikeInitializer()
         or
-        not exists(AssignableDefinition other |
+        not exists(AssignableDefinition other | other.getTarget() = this.getTarget() |
-          other.getTarget() = this.getTarget() |
           other != this
         )
       )
@@ -161,6 +157,7 @@ class RelevantDefinition extends AssignableDefinition {
 }
 
 from RelevantDefinition def, LocalVariable v
-where v = def.getTarget()
+where
-  and def.isDead()
+  v = def.getTarget() and
+  def.isDead()
 select def, "This assignment to $@ is useless, since its value is never read.", v, v.getName()

csharp/ql/src/Dead Code/NonAssignedFields.ql

@@ -16,108 +16,102 @@ import semmle.code.csharp.frameworks.System
 import semmle.code.csharp.frameworks.system.runtime.InteropServices
 
 // Any field transitively contained in t.
-Field getANestedField(ValueOrRefType t)
+Field getANestedField(ValueOrRefType t) {
-{
   result.getDeclaringType() = t
   or
   exists(Field mid |
-    mid=getANestedField(t)
+    mid = getANestedField(t) and
-    and
+    mid.getType() = result.getDeclaringType()
-    mid.getType() = result.getDeclaringType())
+  )
 }
 
 // Any ValueOrRefType referenced by a Type, including TypeParameters.
-ValueOrRefType getAReferencedType(Type t)
+ValueOrRefType getAReferencedType(Type t) {
-{
   result = t
   or
   result = t.(TypeParameter).getASuppliedType()
 }
 
-predicate isTypeExternallyInitialized(ValueOrRefType t)
+predicate isTypeExternallyInitialized(ValueOrRefType t) {
-{
   // The type got created via a call to PtrToStructure().
   exists(MethodCall mc |
-    mc.getTarget() = any(SystemRuntimeInteropServicesMarshalClass c).getPtrToStructureTypeMethod()
+    mc.getTarget() = any(SystemRuntimeInteropServicesMarshalClass c).getPtrToStructureTypeMethod() and
-    and
     t = getAReferencedType(mc.getArgument(1).(TypeofExpr).getTypeAccess().getTarget())
-    )
+  )
-
+  or
   // The type got created via a call to PtrToStructure().
-  or exists(MethodCall mc |
+  exists(MethodCall mc |
-    mc.getTarget() = any(SystemRuntimeInteropServicesMarshalClass c).getPtrToStructureObjectMethod()
+    mc.getTarget() = any(SystemRuntimeInteropServicesMarshalClass c).getPtrToStructureObjectMethod() and
-    and
     t = getAReferencedType(mc.getArgument(1).getType())
-    )
+  )
-
+  or
   // An extern method exists which could initialize the type.
-  or exists(Method m, Parameter p |
+  exists(Method m, Parameter p |
     isExternMethod(m) and
-    p=m.getAParameter() and
+    p = m.getAParameter() and
     t = p.getType()
-    |
+  |
-    p.isOut() or p.isRef())
+    p.isOut() or p.isRef()
-
+  )
+  or
   // The data structure has been cast to a pointer - all bets are off.
-  or exists(CastExpr c |
+  exists(CastExpr c | t = getAReferencedType(c.getTargetType().(PointerType).getReferentType()))
-    t = getAReferencedType(c.getTargetType().(PointerType).getReferentType()))
 }
 
 // The type is potentially marshaled using an extern or interop.
-predicate isFieldExternallyInitialized(Field f)
+predicate isFieldExternallyInitialized(Field f) {
-{
   exists(ValueOrRefType t |
-    f = getANestedField(t)
+    f = getANestedField(t) and
-    and
+    isTypeExternallyInitialized(t)
-    isTypeExternallyInitialized(t))
+  )
 }
 
-predicate isExternMethod(Method externMethod)
+predicate isExternMethod(Method externMethod) {
-{
   externMethod.isExtern()
   or
-  externMethod.getAnAttribute().getType() instanceof SystemRuntimeInteropServicesDllImportAttributeClass
+  externMethod.getAnAttribute().getType() instanceof
+    SystemRuntimeInteropServicesDllImportAttributeClass
   or
-  externMethod.getDeclaringType().getAnAttribute().getType() instanceof SystemRuntimeInteropServicesComImportAttributeClass
+  externMethod.getDeclaringType().getAnAttribute().getType() instanceof
+    SystemRuntimeInteropServicesComImportAttributeClass
 }
 
 from Field f, FieldRead fa
 where
-  f.fromSource()
+  f.fromSource() and
-  and not extractionIsStandalone()
+  not extractionIsStandalone() and
-  and not f.isReadOnly()
+  not f.isReadOnly() and
-  and not f.isConst()
+  not f.isConst() and
-  and not f.getDeclaringType() instanceof Enum
+  not f.getDeclaringType() instanceof Enum and
-  and not f.getType() instanceof Struct
+  not f.getType() instanceof Struct and
-  and not exists(Assignment ae, Field g |
+  not exists(Assignment ae, Field g |
-    ae.getLValue().(FieldAccess).getTarget() = g
+    ae.getLValue().(FieldAccess).getTarget() = g and
-    and g.getSourceDeclaration() = f
+    g.getSourceDeclaration() = f and
-    and not (ae.getRValue() instanceof NullLiteral)
+    not (ae.getRValue() instanceof NullLiteral)
-  )
+  ) and
-  and not exists(MethodCall mc, int i, Field g |
+  not exists(MethodCall mc, int i, Field g |
-    exists(Parameter p | mc.getTarget().getParameter(i) = p |
+    exists(Parameter p | mc.getTarget().getParameter(i) = p | p.isOut() or p.isRef()) and
-      p.isOut() or p.isRef()
+    mc.getArgument(i) = g.getAnAccess() and
-    )
-    and mc.getArgument(i) = g.getAnAccess()
-    and g.getSourceDeclaration() = f
-  )
-  and not isFieldExternallyInitialized(f)
-  and not exists(f.getAnAttribute())
-  and not exists(Expr init, Field g |
     g.getSourceDeclaration() = f
-    and g.getInitializer() = init
+  ) and
-    and not init instanceof NullLiteral
+  not isFieldExternallyInitialized(f) and
-  )
+  not exists(f.getAnAttribute()) and
-  and not exists(AssignOperation ua, Field g |
+  not exists(Expr init, Field g |
-    ua.getLValue().(FieldAccess).getTarget() = g
+    g.getSourceDeclaration() = f and
-    and g.getSourceDeclaration() = f
+    g.getInitializer() = init and
-  )
+    not init instanceof NullLiteral
-  and not exists(MutatorOperation op |
+  ) and
+  not exists(AssignOperation ua, Field g |
+    ua.getLValue().(FieldAccess).getTarget() = g and
+    g.getSourceDeclaration() = f
+  ) and
+  not exists(MutatorOperation op |
     op.getAnOperand().(FieldAccess).getTarget().getSourceDeclaration() = f
+  ) and
+  exists(Field g |
+    fa.getTarget() = g and
+    g.getSourceDeclaration() = f
   )
-  and exists(Field g |
+select f,
-    fa.getTarget() = g
+  "The field '" + f.getName() + "' is never explicitly assigned a value, yet it is read $@.", fa,
-    and g.getSourceDeclaration() = f
+  "here"
-  )
-select f, "The field '" + f.getName() + "' is never explicitly assigned a value, yet it is read $@.",
-  fa, "here"

csharp/ql/src/Dead Code/UnusedField.ql

@@ -9,13 +9,14 @@
  *       useless-code
  *       external/cwe/cwe-561
  */
+
 import csharp
 import DeadCode
 
 from Field f
 where
-  not extractionIsStandalone()
+  not extractionIsStandalone() and
-  and f.fromSource()
+  f.fromSource() and
-  and isDeadField(f)
+  isDeadField(f) and
-  and not f.getDeclaringType().isPartial()
+  not f.getDeclaringType().isPartial()
 select f, "Unused field (or field used from dead method only)"

csharp/ql/src/Dead Code/UnusedMethod.ql

@@ -10,13 +10,14 @@
  *       useless-code
  *       external/cwe/cwe-561
  */
+
 import csharp
 import DeadCode
 
 from Method m
 where
-  not extractionIsStandalone()
+  not extractionIsStandalone() and
-  and m.fromSource()
+  m.fromSource() and
-  and isDeadMethod(m)
+  isDeadMethod(m) and
-  and not m.getDeclaringType().isPartial()
+  not m.getDeclaringType().isPartial()
 select m, "Unused method (or method called from dead method only)"

csharp/ql/src/Documentation/XmldocExtraParam.ql

@@ -12,7 +12,8 @@
 import Documentation
 
 from SourceMethodOrConstructor m, ParamXmlComment comment, string paramName
-where comment = getADeclarationXmlComment(m)
+where
-and comment.getName(_)=paramName
+  comment = getADeclarationXmlComment(m) and
-and not m.getAParameter().getName() = paramName
+  comment.getName(_) = paramName and
+  not m.getAParameter().getName() = paramName
 select m, "Documentation specifies an invalid parameter name $@.", comment, paramName

csharp/ql/src/Documentation/XmldocExtraTypeParam.ql

@@ -12,7 +12,8 @@
 import Documentation
 
 from UnboundGeneric d, TypeparamXmlComment comment, string paramName
-where comment = getADeclarationXmlComment(d)
+where
-and comment.getName(_) = paramName
+  comment = getADeclarationXmlComment(d) and
-and not d.getATypeParameter().getName() = paramName
+  comment.getName(_) = paramName and
+  not d.getATypeParameter().getName() = paramName
 select d, "Documentation specifies an invalid type parameter name $@.", comment, paramName

csharp/ql/src/Documentation/XmldocMissing.ql

@@ -12,16 +12,20 @@
 import Documentation
 
 /** Generate a user-friendly string for the declaration */
-string declarationDescription(Declaration d)
+string declarationDescription(Declaration d) {
-{
+  d instanceof Class and result = "class"
-  d instanceof Class and result="class" or
+  or
-  d instanceof Interface and result="interface" or
+  d instanceof Interface and result = "interface"
-  d instanceof Method and result="method" or
+  or
-  d instanceof Constructor and result="constructor" or
+  d instanceof Method and result = "method"
-  d instanceof Struct and result="struct"
+  or
+  d instanceof Constructor and result = "constructor"
+  or
+  d instanceof Struct and result = "struct"
 }
 
 from Declaration decl
-where isDocumentationNeeded(decl)
+where
-and not declarationHasXmlComment(decl)
+  isDocumentationNeeded(decl) and
+  not declarationHasXmlComment(decl)
 select decl, "Public " + declarationDescription(decl) + " should be documented."

csharp/ql/src/Documentation/XmldocMissingException.ql

@@ -12,15 +12,16 @@
 import Documentation
 
 from SourceMethodOrConstructor m, ThrowElement throw, RefType throwType
-where declarationHasXmlComment(m)
+where
-and m = throw.getEnclosingCallable()
+  declarationHasXmlComment(m) and
-and throwType = throw.getExpr().getType()
+  m = throw.getEnclosingCallable() and
-and not exists(ExceptionXmlComment comment, int offset, string exceptionName, RefType throwBaseType |
+  throwType = throw.getExpr().getType() and
-    comment = getADeclarationXmlComment(m)
+  not exists(ExceptionXmlComment comment, int offset, string exceptionName, RefType throwBaseType |
-    and exceptionName = comment.getCref(offset)
+    comment = getADeclarationXmlComment(m) and
-    and throwType.getABaseType*() = throwBaseType
+    exceptionName = comment.getCref(offset) and
-    and (throwBaseType.hasName(exceptionName) or throwBaseType.hasQualifiedName(exceptionName))
+    throwType.getABaseType*() = throwBaseType and
+    (throwBaseType.hasName(exceptionName) or throwBaseType.hasQualifiedName(exceptionName))
     // and comment.hasBody(offset) // Too slow
-    )
+  ) and
-and not getADeclarationXmlComment(m) instanceof InheritDocXmlComment
+  not getADeclarationXmlComment(m) instanceof InheritDocXmlComment
 select m, "Exception $@ should be documented.", throw, throw.getExpr().getType().getName()

csharp/ql/src/Documentation/XmldocMissingParam.ql

@@ -12,11 +12,13 @@
 import Documentation
 
 from SourceMethodOrConstructor m, SourceParameter p
-where p = m.getAParameter()
+where
-and declarationHasXmlComment(m)
+  p = m.getAParameter() and
-and not exists( ParamXmlComment c, int offset |
+  declarationHasXmlComment(m) and
-  c = getADeclarationXmlComment(m)
+  not exists(ParamXmlComment c, int offset |
-  and c.getName(offset) = p.getName()
+    c = getADeclarationXmlComment(m) and
-  and c.hasBody(offset) )
+    c.getName(offset) = p.getName() and
-and not getADeclarationXmlComment(m) instanceof InheritDocXmlComment
+    c.hasBody(offset)
+  ) and
+  not getADeclarationXmlComment(m) instanceof InheritDocXmlComment
 select p, "Parameter should be documented."

csharp/ql/src/Documentation/XmldocMissingReturn.ql

@@ -12,9 +12,11 @@
 import Documentation
 
 from SourceMethod m
-where declarationHasXmlComment(m)
+where
-and forall(ReturnsXmlComment c | c=getADeclarationXmlComment(m) |
+  declarationHasXmlComment(m) and
-        forall(int offset | c.isOpenTag(offset) | c.isEmptyTag(offset) ) )
+  forall(ReturnsXmlComment c | c = getADeclarationXmlComment(m) |
-and not m.getReturnType() instanceof VoidType
+    forall(int offset | c.isOpenTag(offset) | c.isEmptyTag(offset))
-and not getADeclarationXmlComment(m) instanceof InheritDocXmlComment
+  ) and
+  not m.getReturnType() instanceof VoidType and
+  not getADeclarationXmlComment(m) instanceof InheritDocXmlComment
 select m, "Return value should be documented."

csharp/ql/src/Documentation/XmldocMissingSummary.ql

@@ -11,9 +11,11 @@
 import Documentation
 
 from Declaration decl
-where declarationHasXmlComment(decl)
+where
-and isDocumentationNeeded(decl)
+  declarationHasXmlComment(decl) and
-and forall(SummaryXmlComment c | c=getADeclarationXmlComment(decl) |
+  isDocumentationNeeded(decl) and
-        forall(int offset | c.isOpenTag(offset) | c.isEmptyTag(offset) ) )
+  forall(SummaryXmlComment c | c = getADeclarationXmlComment(decl) |
-and not getADeclarationXmlComment(decl) instanceof InheritDocXmlComment
+    forall(int offset | c.isOpenTag(offset) | c.isEmptyTag(offset))
+  ) and
+  not getADeclarationXmlComment(decl) instanceof InheritDocXmlComment
 select decl, "Documentation should have a summary."

csharp/ql/src/Documentation/XmldocMissingTypeParam.ql

@@ -12,11 +12,13 @@
 import Documentation
 
 from UnboundGeneric d, TypeParameter p
-where p=d.getATypeParameter()
+where
-and declarationHasXmlComment(d)
+  p = d.getATypeParameter() and
-and not exists(TypeparamXmlComment comment, int offset |
+  declarationHasXmlComment(d) and
-  comment = getADeclarationXmlComment(d)
+  not exists(TypeparamXmlComment comment, int offset |
-  and comment.getName(offset) = p.getName()
+    comment = getADeclarationXmlComment(d) and
-  and comment.hasBody(offset) )
+    comment.getName(offset) = p.getName() and
-and not getADeclarationXmlComment(d) instanceof InheritDocXmlComment
+    comment.hasBody(offset)
+  ) and
+  not getADeclarationXmlComment(d) instanceof InheritDocXmlComment
 select p, "Type parameter should be documented."

csharp/ql/src/Input Validation/UseOfFileUpload.ql

@@ -10,9 +10,11 @@
  *       frameworks/asp.net
  *       external/cwe/cwe-434
  */
+
 import csharp
 
 from PropertyAccess pa
-where pa.getTarget().hasName("PostedFile")
+where
-  and pa.getTarget().getDeclaringType().hasQualifiedName("System.Web.UI.HtmlControls", "HtmlInputFile")
+  pa.getTarget().hasName("PostedFile") and
+  pa.getTarget().getDeclaringType().hasQualifiedName("System.Web.UI.HtmlControls", "HtmlInputFile")
 select pa, "Avoid using file upload."

csharp/ql/src/Input Validation/ValueShadowing.ql

@@ -9,10 +9,12 @@
  *       maintainability
  *       frameworks/asp.net
  */
+
 import csharp
 import semmle.code.csharp.frameworks.system.web.Http
 
 from IndexerAccess ia
-where ia.getTarget().getDeclaringType().hasQualifiedName("System.Web", "HttpRequest")
+where
-  and not isServerVariable(ia.getIndex(0))
+  ia.getTarget().getDeclaringType().hasQualifiedName("System.Web", "HttpRequest") and
+  not isServerVariable(ia.getIndex(0))
 select ia, "Ambiguous access to variable."

csharp/ql/src/Input Validation/ValueShadowingServerVariable.ql

@@ -9,10 +9,12 @@
  *       maintainability
  *       frameworks/asp.net
  */
+
 import csharp
 import semmle.code.csharp.frameworks.system.web.Http
 
 from IndexerAccess ia
-where ia.getTarget().getDeclaringType().hasQualifiedName("System.Web", "HttpRequest")
+where
-  and isServerVariable(ia.getIndex(0))
+  ia.getTarget().getDeclaringType().hasQualifiedName("System.Web", "HttpRequest") and
+  isServerVariable(ia.getIndex(0))
 select ia, "Ambiguous access to server variable."

csharp/ql/src/Language Abuse/CastThisToTypeParameter.ql

@@ -13,10 +13,11 @@
 import csharp
 
 from ExplicitCast c, ConstructedType src, TypeParameter dest
-where c.getExpr() instanceof ThisAccess
+where
-  and src = c.getExpr().getType()
+  c.getExpr() instanceof ThisAccess and
-  and dest = c.getTargetType()
+  src = c.getExpr().getType() and
-  and dest = src.getUnboundGeneric().getATypeParameter()
+  dest = c.getTargetType() and
-select c, "Casting 'this' to $@, a type parameter of $@, masks an implicit type constraint that should be explicitly stated.",
+  dest = src.getUnboundGeneric().getATypeParameter()
-  dest, dest.getName(),
+select c,
-  src, src.getName()
+  "Casting 'this' to $@, a type parameter of $@, masks an implicit type constraint that should be explicitly stated.",
+  dest, dest.getName(), src, src.getName()

csharp/ql/src/Language Abuse/CatchOfGenericException.ql

@@ -19,16 +19,18 @@ class GenericCatchClause extends CatchClause {
     this instanceof GeneralCatchClause
     or
     this = any(SpecificCatchClause scc |
-      scc.getCaughtExceptionType() instanceof SystemExceptionClass and
+        scc.getCaughtExceptionType() instanceof SystemExceptionClass and
-      not scc.hasFilterClause()
+        not scc.hasFilterClause()
-    )
+      )
   }
 }
 
 from GenericCatchClause gcc
-where forall(ThrowStmt throw |
+where
-  // ok to catch all exceptions if they may be rethrown
+  forall(ThrowStmt throw |
-  gcc.getBlock().getAChildStmt+() = throw |
+    // ok to catch all exceptions if they may be rethrown
-  exists(throw.getExpr())
+    gcc.getBlock().getAChildStmt+() = throw
-)
+  |
+    exists(throw.getExpr())
+  )
 select gcc, "Generic catch clause."

csharp/ql/src/Language Abuse/ChainedIs.ql

@@ -15,25 +15,20 @@ import csharp
 
 int isCountForIfChain(IfStmt is) {
   exists(int rest |
+    (if is.getElse() instanceof IfStmt then rest = isCountForIfChain(is.getElse()) else rest = 0) and
     (
-      if is.getElse() instanceof IfStmt then
+      if is.getCondition().(IsTypeExpr).getCheckedType().getSourceDeclaration().fromSource()
-        rest = isCountForIfChain(is.getElse())
+      then result = 1 + rest
-      else
+      else result = rest
-        rest = 0
-    )
-    and
-    (
-      if is.getCondition().(IsTypeExpr).getCheckedType().getSourceDeclaration().fromSource() then
-        result = 1 + rest
-      else
-        result = rest
     )
   )
 }
 
 from IfStmt is, int n
 where
-  n = isCountForIfChain(is)
+  n = isCountForIfChain(is) and
-  and n > 5
+  n > 5 and
-  and not exists(IfStmt other | is = other.getElse())
+  not exists(IfStmt other | is = other.getElse())
-select is, "This if block performs a chain of " + n + " type tests - consider alternatives, e.g. polymorphism or the visitor pattern."
+select is,
+  "This if block performs a chain of " + n +
+    " type tests - consider alternatives, e.g. polymorphism or the visitor pattern."

csharp/ql/src/Language Abuse/DubiousDowncastOfThis.ql

@@ -13,10 +13,10 @@
 import csharp
 
 from ExplicitCast c, ValueOrRefType src, ValueOrRefType dest
-where c.getExpr() instanceof ThisAccess
+where
-  and src = c.getExpr().getType()
+  c.getExpr() instanceof ThisAccess and
-  and dest = c.getTargetType()
+  src = c.getExpr().getType() and
-  and src = dest.getABaseType+()
+  dest = c.getTargetType() and
+  src = dest.getABaseType+()
 select c, "Downcasting 'this' from $@ to $@ introduces a dependency cycle between the two types.",
-  src, src.getName(),
+  src, src.getName(), dest, dest.getName()
-  dest, dest.getName()

csharp/ql/src/Language Abuse/DubiousTypeTestOfThis.ql

@@ -15,11 +15,11 @@ import semmle.code.csharp.commons.Assertions
 
 from IsTypeExpr ise, ValueOrRefType t, ValueOrRefType ct
 where
-  ise.getExpr() instanceof ThisAccess
+  ise.getExpr() instanceof ThisAccess and
-  and t = ise.getExpr().getType()
+  t = ise.getExpr().getType() and
-  and ct = ise.getCheckedType()
+  ct = ise.getCheckedType() and
-  and ct.getABaseType*() = t
+  ct.getABaseType*() = t and
-  and not isExprInAssertion(ise)
+  not isExprInAssertion(ise)
-select ise, "Testing whether 'this' is an instance of $@ in $@ introduces a dependency cycle between the two types.",
+select ise,
-  ct, ct.getName(),
+  "Testing whether 'this' is an instance of $@ in $@ introduces a dependency cycle between the two types.",
-  t, t.getName()
+  ct, ct.getName(), t, t.getName()

csharp/ql/src/Language Abuse/ForeachCapture.ql

@@ -17,10 +17,7 @@ import semmle.code.csharp.frameworks.system.Collections
 import semmle.code.csharp.frameworks.system.collections.Generic
 
 predicate lambdaCaptures(AnonymousFunctionExpr lambda, Variable v) {
-  exists(VariableAccess va |
+  exists(VariableAccess va | va.getEnclosingCallable() = lambda | va.getTarget() = v)
-    va.getEnclosingCallable() = lambda |
-    va.getTarget() = v
-  )
 }
 
 predicate lambdaCapturesLoopVariable(AnonymousFunctionExpr lambda, ForeachStmt loop, Variable v) {
@@ -33,41 +30,34 @@ predicate inForeachStmtBody(ForeachStmt loop, Element e) {
   e = loop.getBody()
   or
   exists(Element mid |
-    inForeachStmtBody(loop, mid)
+    inForeachStmtBody(loop, mid) and
-    and
     e = mid.getAChild()
   )
 }
 
 class LambdaDataFlowConfiguration extends DataFlow::Configuration {
-  LambdaDataFlowConfiguration() {
+  LambdaDataFlowConfiguration() { this = "LambdaDataFlowConfiguration" }
-    this = "LambdaDataFlowConfiguration"
-  }
 
   override predicate isSource(DataFlow::Node source) {
     lambdaCapturesLoopVariable(source.asExpr(), _, _)
   }
 
-  override predicate isSink(DataFlow::Node sink) {
+  override predicate isSink(DataFlow::Node sink) { exists(getAssignmentTarget(sink.asExpr())) }
-    exists(getAssignmentTarget(sink.asExpr()))
-  }
 
-  predicate capturesLoopVarAndIsStoredIn(AnonymousFunctionExpr lambda, Variable loopVar, Element storage) {
+  predicate capturesLoopVarAndIsStoredIn(
-    exists(DataFlow::Node sink |
+    AnonymousFunctionExpr lambda, Variable loopVar, Element storage
-      this.hasFlow(DataFlow::exprNode(lambda), sink) |
+  ) {
+    exists(DataFlow::Node sink | this.hasFlow(DataFlow::exprNode(lambda), sink) |
       storage = getAssignmentTarget(sink.asExpr())
-    )
+    ) and
-    and
+    exists(ForeachStmt loop | lambdaCapturesLoopVariable(lambda, loop, loopVar) |
-    exists(ForeachStmt loop |
-      lambdaCapturesLoopVariable(lambda, loop, loopVar) |
       not declaredInsideLoop(loop, storage)
     )
   }
 }
 
 Element getAssignmentTarget(Expr e) {
-  exists(Assignment a |
+  exists(Assignment a | a.getRValue() = e |
-    a.getRValue() = e |
     result = a.getLValue().(PropertyAccess).getTarget() or
     result = a.getLValue().(FieldAccess).getTarget() or
     result = a.getLValue().(LocalVariableAccess).getTarget() or
@@ -84,19 +74,16 @@ Element getAssignmentTarget(Expr e) {
 
 Element getCollectionAssignmentTarget(Expr e) {
   // Store into collection via method
-  exists(MethodCall mc, Method m, IEnumerableFlow ief, CallableFlowSourceArg source, CallableFlowSinkQualifier sink, int i |
+  exists(
-    mc.getQualifier() = result.(Variable).getAnAccess()
+    MethodCall mc, Method m, IEnumerableFlow ief, CallableFlowSourceArg source,
-    and
+    CallableFlowSinkQualifier sink, int i
-    ief = mc.getQualifier().getType().getSourceDeclaration()
+  |
-    and
+    mc.getQualifier() = result.(Variable).getAnAccess() and
-    m = mc.getTarget().getSourceDeclaration()
+    ief = mc.getQualifier().getType().getSourceDeclaration() and
-    and
+    m = mc.getTarget().getSourceDeclaration() and
-    ief.callableFlow(source, sink, m, _)
+    ief.callableFlow(source, sink, m, _) and
-    and
+    source.getCallable() = m and
-    source.getCallable() = m
+    source.getArgumentIndex() = i and
-    and
-    source.getArgumentIndex() = i
-    and
     e = mc.getArgument(i)
   )
   or
@@ -104,26 +91,29 @@ Element getCollectionAssignmentTarget(Expr e) {
   e = result.(ArrayCreation).getInitializer().getAnElement()
   or
   // Collection initializer
-  e = result.(ObjectCreation).getInitializer().(CollectionInitializer).getElementInitializer(_).getAnArgument()
+  e = result
+        .(ObjectCreation)
+        .getInitializer()
+        .(CollectionInitializer)
+        .getElementInitializer(_)
+        .getAnArgument()
   or
   // Store values using indexer
   exists(IndexerAccess ia, AssignExpr ae |
-    ia.getQualifier() = result.(Variable).getAnAccess()
+    ia.getQualifier() = result.(Variable).getAnAccess() and
-    and
+    ia = ae.getLValue() and
-    ia = ae.getLValue()
-    and
     e = ae.getRValue()
   )
 }
 
 // Variable v is declared inside the loop body
 predicate declaredInsideLoop(ForeachStmt loop, LocalVariable v) {
-  exists(LocalVariableDeclStmt decl |
+  exists(LocalVariableDeclStmt decl | decl.getVariableDeclExpr(_).getVariable() = v |
-    decl.getVariableDeclExpr(_).getVariable() = v |
     inForeachStmtBody(loop, decl)
   )
 }
 
 from LambdaDataFlowConfiguration c, AnonymousFunctionExpr lambda, Variable loopVar, Element storage
 where c.capturesLoopVarAndIsStoredIn(lambda, loopVar, storage)
-select lambda, "Function which may be stored in $@ captures variable $@", storage, storage.toString(), loopVar, loopVar.getName()
+select lambda, "Function which may be stored in $@ captures variable $@", storage,
+  storage.toString(), loopVar, loopVar.getName()

csharp/ql/src/Language Abuse/MissedReadonlyOpportunity.ql

@@ -13,8 +13,7 @@
 import csharp
 
 predicate isReadonlyCompatibleDefinition(AssignableDefinition def, Field f) {
-  def.getTarget().getSourceDeclaration() = f
+  def.getTarget().getSourceDeclaration() = f and
-  and
   (
     def.getEnclosingCallable().(Constructor).getDeclaringType() = f.getDeclaringType()
     or
@@ -23,15 +22,15 @@ predicate isReadonlyCompatibleDefinition(AssignableDefinition def, Field f) {
 }
 
 predicate canBeReadonly(Field f) {
-  forex(AssignableDefinition def |
+  forex(AssignableDefinition def | def.getTarget().getSourceDeclaration() = f |
-    def.getTarget().getSourceDeclaration() = f |
     isReadonlyCompatibleDefinition(def, f)
   )
 }
 
 from Field f
-where canBeReadonly(f)
+where
-  and not f.isConst()
+  canBeReadonly(f) and
-  and not f.isReadOnly()
+  not f.isConst() and
-  and (f.isEffectivelyPrivate() or f.isEffectivelyInternal())
+  not f.isReadOnly() and
+  (f.isEffectivelyPrivate() or f.isEffectivelyInternal())
 select f, "Field '" + f.getName() + "' can be 'readonly'."

csharp/ql/src/Language Abuse/MissedTernaryOpportunity.ql

@@ -13,25 +13,21 @@ import csharp
 import semmle.code.csharp.commons.StructuralComparison
 
 class StructuralComparisonConfig extends StructuralComparisonConfiguration {
-  StructuralComparisonConfig() {
+  StructuralComparisonConfig() { this = "MissedTernaryOpportunity" }
-    this = "MissedTernaryOpportunity"
-  }
 
   override predicate candidate(Element x, Element y) {
     exists(IfStmt is, AssignExpr ae1 |
-      ae1 = is.getThen().stripSingletonBlocks().(ExprStmt).getExpr() |
+      ae1 = is.getThen().stripSingletonBlocks().(ExprStmt).getExpr()
-      x = ae1.getLValue()
+    |
-      and
+      x = ae1.getLValue() and
-      exists(AssignExpr ae2 |
+      exists(AssignExpr ae2 | ae2 = is.getElse().stripSingletonBlocks().(ExprStmt).getExpr() |
-        ae2 = is.getElse().stripSingletonBlocks().(ExprStmt).getExpr() |
         y = ae2.getLValue()
       )
     )
   }
 
   IfStmt getIfStmt() {
-    exists(AssignExpr ae |
+    exists(AssignExpr ae | ae = result.getThen().stripSingletonBlocks().(ExprStmt).getExpr() |
-      ae = result.getThen().stripSingletonBlocks().(ExprStmt).getExpr() |
       same(ae.getLValue(), _)
     )
   }
@@ -41,14 +37,16 @@ from IfStmt is, string what
 where
   (
     (
-      is.getThen().stripSingletonBlocks() instanceof ReturnStmt
+      is.getThen().stripSingletonBlocks() instanceof ReturnStmt and
-      and is.getElse().stripSingletonBlocks() instanceof ReturnStmt
+      is.getElse().stripSingletonBlocks() instanceof ReturnStmt and
-      and what = "return"
+      what = "return"
     )
-    or exists(StructuralComparisonConfig c |
+    or
-      is = c.getIfStmt()
+    exists(StructuralComparisonConfig c |
-      and what = "write to the same variable"
+      is = c.getIfStmt() and
+      what = "write to the same variable"
     )
-  )
+  ) and
-  and not exists(IfStmt other | is = other.getElse())
+  not exists(IfStmt other | is = other.getElse())
-select is, "Both branches of this 'if' statement " + what + " - consider using '?' to express intent better."
+select is,
+  "Both branches of this 'if' statement " + what + " - consider using '?' to express intent better."

csharp/ql/src/Language Abuse/MissedUsingOpportunity.ql

@@ -13,27 +13,22 @@ import csharp
 import semmle.code.csharp.frameworks.System
 
 /** A call to IDisposable.Dispose or a method that overrides it. */
-class DisposeCall extends MethodCall
+class DisposeCall extends MethodCall {
-{
+  DisposeCall() { getTarget() instanceof DisposeMethod }
-  DisposeCall()
-  {
-    getTarget() instanceof DisposeMethod
-  }
 
   /** The object being disposed by the call (provided it can be easily determined). */
-  Variable getDisposee()
+  Variable getDisposee() {
-  {
     exists(VariableAccess va |
-      va = getQualifier().stripCasts()
+      va = getQualifier().stripCasts() and
-      and result = va.getTarget()
+      result = va.getTarget()
     )
   }
 }
 
 from Variable v, DisposeCall c, TryStmt ts
 where
-  v = c.getDisposee()
+  v = c.getDisposee() and
-  and c = ts.getFinally().getAChild*()
+  c = ts.getFinally().getAChild*()
-select v, "This variable is manually $@ in a $@ - consider a C# using statement as a preferable resource management technique.",
+select v,
-  c, "disposed",
+  "This variable is manually $@ in a $@ - consider a C# using statement as a preferable resource management technique.",
-  ts.getFinally(), "finally block"
+  c, "disposed", ts.getFinally(), "finally block"

csharp/ql/src/Language Abuse/NestedIf.ql

@@ -13,9 +13,7 @@ import csharp
 
 from IfStmt outer, IfStmt inner
 where
-  inner = outer.getThen().stripSingletonBlocks()
+  inner = outer.getThen().stripSingletonBlocks() and
-and
+  not exists(outer.getElse()) and
-  not exists(outer.getElse())
-and
   not exists(inner.getElse())
 select outer, "These 'if' statements can be combined."

csharp/ql/src/Language Abuse/RethrowException.ql

@@ -13,19 +13,18 @@
 
 import csharp
 
-CatchClause containingCatchClause(Stmt s)
+CatchClause containingCatchClause(Stmt s) {
-{
   result.getBlock() = s
   or
   exists(Stmt mid |
-    result = containingCatchClause(mid)
+    result = containingCatchClause(mid) and
-    and
+    mid.getAChildStmt() = s and
-    mid.getAChildStmt() = s
+    not mid instanceof CatchClause
-    and
+  )
-    not mid instanceof CatchClause)
 }
 
 from SpecificCatchClause cc, ThrowStmt throw
-where throw.getExpr() = cc.getVariable().getAnAccess()
+where
-and containingCatchClause(throw) = cc
+  throw.getExpr() = cc.getVariable().getAnAccess() and
+  containingCatchClause(throw) = cc
 select throw, "Rethrowing exception variable."

csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql

@@ -16,8 +16,7 @@ import csharp
  * No other child nodes are boolean literals.
  */
 predicate literalChild(Expr expr, int child, boolean value) {
-  value = expr.getChild(child).(BoolLiteral).getBoolValue()
+  value = expr.getChild(child).(BoolLiteral).getBoolValue() and
-  and
   forall(int c | c != child | not expr.getChild(c) instanceof BoolLiteral)
 }
 
@@ -26,10 +25,8 @@ predicate literalChild(Expr expr, int child, boolean value) {
  * No other child nodes are boolean literals.
  */
 predicate literalChildren(Expr expr, int child1, boolean value1, int child2, boolean value2) {
-  value1 = expr.getChild(child1).(BoolLiteral).getBoolValue()
+  value1 = expr.getChild(child1).(BoolLiteral).getBoolValue() and
-  and
+  value2 = expr.getChild(child2).(BoolLiteral).getBoolValue() and
-  value2 = expr.getChild(child2).(BoolLiteral).getBoolValue()
-  and
   forall(int c | c != child1 and c != child2 | not expr.getChild(c) instanceof BoolLiteral)
 }
 
@@ -40,7 +37,10 @@ predicate rewriteBinaryExpr(BinaryOperation op, boolean value, string oldPattern
 }
 
 bindingset[withFalseOperand, withTrueOperand]
-predicate rewriteBinaryExpr(BinaryOperation op, string oldPattern, string withFalseOperand, string withTrueOperand, string newPattern) {
+predicate rewriteBinaryExpr(
+  BinaryOperation op, string oldPattern, string withFalseOperand, string withTrueOperand,
+  string newPattern
+) {
   rewriteBinaryExpr(op, false, oldPattern) and newPattern = withFalseOperand
   or
   rewriteBinaryExpr(op, true, oldPattern) and newPattern = withTrueOperand
@@ -55,7 +55,9 @@ predicate rewriteConditionalExpr(ConditionalExpr cond, string oldPattern, string
   or
   literalChild(cond, 2, true) and oldPattern = "A ? B : true" and newPattern = "!A || B"
   or
-  exists(boolean b | literalChildren(cond, 1, b, 2, b) | oldPattern = "A ? " + b + " : " + b and newPattern = b.toString())
+  exists(boolean b | literalChildren(cond, 1, b, 2, b) |
+    oldPattern = "A ? " + b + " : " + b and newPattern = b.toString()
+  )
   or
   literalChildren(cond, 1, true, 2, false) and oldPattern = "A ? true : false" and newPattern = "A"
   or
@@ -86,19 +88,18 @@ predicate pushNegation(LogicalNotExpr expr, string oldPattern, string newPattern
   expr.getOperand() instanceof LogicalNotExpr and oldPattern = "!!A" and newPattern = "A"
   or
   exists(string oldOperator, string newOperator |
-    oldOperator = expr.getOperand().(BinaryOperation).getOperator()
+    oldOperator = expr.getOperand().(BinaryOperation).getOperator() and
-    and
     negatedOperators(oldOperator, newOperator)
-    |
+  |
-    oldPattern = "!(A " + oldOperator + " B)"
+    oldPattern = "!(A " + oldOperator + " B)" and
-    and
     newPattern = "A " + newOperator + " B"
   )
 }
 
 predicate rewrite(Expr expr, string oldPattern, string newPattern) {
   exists(string withFalseOperand, string withTrueOperand |
-    simplifyBinaryExpr(expr.(BinaryOperation).getOperator(), withFalseOperand, withTrueOperand) |
+    simplifyBinaryExpr(expr.(BinaryOperation).getOperator(), withFalseOperand, withTrueOperand)
+  |
     rewriteBinaryExpr(expr, oldPattern, withFalseOperand, withTrueOperand, newPattern)
   )
   or
@@ -108,8 +109,9 @@ predicate rewrite(Expr expr, string oldPattern, string newPattern) {
 }
 
 from Expr expr, string oldPattern, string newPattern, string action
-where rewrite(expr, oldPattern, newPattern)
+where
-  and if newPattern = "true" or newPattern = "false"
+  rewrite(expr, oldPattern, newPattern) and
+  if newPattern = "true" or newPattern = "false"
   then action = "is always"
   else action = "can be simplified to"
 select expr, "The expression '" + oldPattern + "' " + action + " '" + newPattern + "'."

csharp/ql/src/Language Abuse/UnusedPropertyValue.ql

@@ -14,10 +14,11 @@
 import csharp
 
 from Setter setter
-where not exists(setter.getAParameter().getAnAccess())
+where
-and not exists(ThrowStmt t | t.getEnclosingCallable()=setter)
+  not exists(setter.getAParameter().getAnAccess()) and
-and setter.hasBody() // Trivial setter is OK
+  not exists(ThrowStmt t | t.getEnclosingCallable() = setter) and
-and not setter.getDeclaration().overrides()
+  setter.hasBody() and // Trivial setter is OK
-and not setter.getDeclaration().implements()
+  not setter.getDeclaration().overrides() and
-and not setter.getDeclaration().isVirtual()
+  not setter.getDeclaration().implements() and
+  not setter.getDeclaration().isVirtual()
 select setter, "Value ignored when setting property."

csharp/ql/src/Language Abuse/UselessCastToSelf.ql

@@ -13,9 +13,10 @@
 import csharp
 
 from ExplicitCast cast, Expr e, Type type
-where e = cast.getExpr()
+where
-  and type = cast.getTargetType()
+  e = cast.getExpr() and
-  and type = e.getType()
+  type = cast.getTargetType() and
-  and not type instanceof NullType
+  type = e.getType() and
-  and not e.(ImplicitDelegateCreation).getArgument() instanceof AnonymousFunctionExpr
+  not type instanceof NullType and
+  not e.(ImplicitDelegateCreation).getArgument() instanceof AnonymousFunctionExpr
 select cast, "This cast is redundant because the expression already has type " + type + "."

csharp/ql/src/Language Abuse/UselessIsBeforeAs.ql

@@ -14,20 +14,15 @@ import csharp
 import semmle.code.csharp.commons.StructuralComparison
 
 class StructuralComparisonConfig extends StructuralComparisonConfiguration {
-  StructuralComparisonConfig() {
+  StructuralComparisonConfig() { this = "UselessIsBeforeAs" }
-    this = "UselessIsBeforeAs"
-  }
 
   override predicate candidate(Element x, Element y) {
     exists(IfStmt is, AsExpr ae, IsTypeExpr ie |
-      ie = is.getCondition().getAChild*()
+      ie = is.getCondition().getAChild*() and
-      and
+      ae.getTargetType() = ie.getCheckedType() and
-      ae.getTargetType() = ie.getCheckedType()
+      x = ie.getExpr() and
-      and
-      x = ie.getExpr()
-      and
       y = ae.getExpr()
-      |
+    |
       ae = is.getThen().getAChild*()
       or
       ae = is.getElse().getAChild*()
@@ -36,10 +31,8 @@ class StructuralComparisonConfig extends StructuralComparisonConfiguration {
 
   predicate uselessIsBeforeAs(AsExpr ae, IsExpr ie) {
     exists(Expr x, Expr y |
-      same(x, y)
+      same(x, y) and
-      and
+      ie.getExpr() = x and
-      ie.getExpr() = x
-      and
       ae.getExpr() = y
     )
   }
@@ -47,8 +40,8 @@ class StructuralComparisonConfig extends StructuralComparisonConfiguration {
 
 from AsExpr ae, IsExpr ie
 where
-  exists(StructuralComparisonConfig c | c.uselessIsBeforeAs(ae, ie))
+  exists(StructuralComparisonConfig c | c.uselessIsBeforeAs(ae, ie)) and
-  and
   not exists(MethodCall mc | ae = mc.getAnArgument().getAChildExpr*())
-select ae, "This 'as' expression performs a type test - it should be directly compared against null, rendering the 'is' $@ potentially redundant.",
+select ae,
+  "This 'as' expression performs a type test - it should be directly compared against null, rendering the 'is' $@ potentially redundant.",
   ie, "here"

csharp/ql/src/Language Abuse/UselessNullCoalescingExpression.ql

@@ -15,23 +15,19 @@ import csharp
 import semmle.code.csharp.commons.StructuralComparison
 
 class StructuralComparisonConfig extends StructuralComparisonConfiguration {
-  StructuralComparisonConfig() {
+  StructuralComparisonConfig() { this = "UselessNullCoalescingExpression" }
-    this = "UselessNullCoalescingExpression"
-  }
 
   override predicate candidate(Element x, Element y) {
     exists(NullCoalescingExpr nce |
-      x.(Access) = nce.getLeftOperand()
+      x.(Access) = nce.getLeftOperand() and
-      and
       y.(Access) = nce.getRightOperand().getAChildExpr*()
     )
   }
 
   NullCoalescingExpr getUselessNullCoalescingExpr() {
     exists(AssignableAccess x |
-      result.getLeftOperand() = x
+      result.getLeftOperand() = x and
-      and
+      forex(AssignableAccess y | same(x, y) | y instanceof AssignableRead and not y.isRefArgument())
-      forex(AssignableAccess y | same(x,y) | y instanceof AssignableRead and not y.isRefArgument())
     )
   }
 }

csharp/ql/src/Language Abuse/UselessTypeTest.ql

@@ -14,9 +14,9 @@ import csharp
 
 from IsTypeExpr ise, ValueOrRefType t, ValueOrRefType ct
 where
-  t = ise.getExpr().getType()
+  t = ise.getExpr().getType() and
-  and ct = ise.getCheckedType()
+  ct = ise.getCheckedType() and
-  and ct = t.getABaseType+()
+  ct = t.getABaseType+()
-select ise, "There is no need to test whether an instance of $@ is also an instance of $@ - it always is.",
+select ise,
-  t, t.getName(),
+  "There is no need to test whether an instance of $@ is also an instance of $@ - it always is.", t,
-  ct, ct.getName()
+  t.getName(), ct, ct.getName()

csharp/ql/src/Language Abuse/UselessUpcast.ql

@@ -13,17 +13,11 @@
 import csharp
 
 /** A static callable. */
-class StaticCallable extends Callable {
+class StaticCallable extends Callable { StaticCallable() { this.(Modifiable).isStatic() } }
-  StaticCallable() {
-    this.(Modifiable).isStatic()
-  }
-}
 
 /** An instance callable, that is, a non-static callable. */
 class InstanceCallable extends Callable {
-  InstanceCallable() {
+  InstanceCallable() { not this instanceof StaticCallable }
-    not this instanceof StaticCallable
-  }
 }
 
 /** A call to a static callable. */
@@ -35,21 +29,21 @@ class StaticCall extends Call {
 }
 
 /** Holds `t` has instance callable `c` as a member, with name `name`. */
-pragma [noinline]
+pragma[noinline]
 predicate hasInstanceCallable(ValueOrRefType t, InstanceCallable c, string name) {
   t.hasMember(c) and
   name = c.getName()
 }
 
 /** Holds if extension method `m` is a method on `t` with name `name`. */
-pragma [noinline]
+pragma[noinline]
 predicate hasExtensionMethod(ValueOrRefType t, ExtensionMethod m, string name) {
   t.isImplicitlyConvertibleTo(m.getExtendedType()) and
   name = m.getName()
 }
 
 /** Holds `t` has static callable `c` as a member, with name `name`. */
-pragma [noinline]
+pragma[noinline]
 predicate hasStaticCallable(ValueOrRefType t, StaticCallable c, string name) {
   t.hasMember(c) and
   name = c.getName()
@@ -58,9 +52,9 @@ predicate hasStaticCallable(ValueOrRefType t, StaticCallable c, string name) {
 /** Gets the minimum number of arguments required to call `c`. */
 int getMinimumArguments(Callable c) {
   result = count(Parameter p |
-    p =  c.getAParameter() and
+      p = c.getAParameter() and
-    not p.hasDefaultValue()
+      not p.hasDefaultValue()
-  )
+    )
 }
 
 /** Gets the maximum number of arguments allowed to call `c`, if any. */
@@ -72,6 +66,7 @@ int getMaximumArguments(Callable c) {
 /** An explicit upcast. */
 class ExplicitUpcast extends ExplicitCast {
   ValueOrRefType src;
+
   ValueOrRefType dest;
 
   ExplicitUpcast() {
@@ -93,10 +88,9 @@ class ExplicitUpcast extends ExplicitCast {
   }
 
   /** Holds if this upcast may be used to disambiguate the target of an instance call. */
-  pragma [nomagic]
+  pragma[nomagic]
   private predicate isDisambiguatingInstanceCall(InstanceCallable other, int args) {
-    exists(Call c, InstanceCallable target, ValueOrRefType t |
+    exists(Call c, InstanceCallable target, ValueOrRefType t | this.isArgument(c, target) |
-      this.isArgument(c, target) |
       t = c.(QualifiableExpr).getQualifier().getType() and
       hasInstanceCallable(t, other, target.getName()) and
       args = c.getNumberOfArguments() and
@@ -105,10 +99,11 @@ class ExplicitUpcast extends ExplicitCast {
   }
 
   /** Holds if this upcast may be used to disambiguate the target of an extension method call. */
-  pragma [nomagic]
+  pragma[nomagic]
   private predicate isDisambiguatingExtensionCall(ExtensionMethod other, int args) {
     exists(ExtensionMethodCall c, ExtensionMethod target, ValueOrRefType t |
-      this.isArgument(c, target) |
+      this.isArgument(c, target)
+    |
       not c.isOrdinaryStaticCall() and
       t = target.getParameter(0).getType() and
       hasExtensionMethod(t, other, target.getName()) and
@@ -117,8 +112,10 @@ class ExplicitUpcast extends ExplicitCast {
     )
   }
 
-  pragma [nomagic]
+  pragma[nomagic]
-  private predicate isDisambiguatingStaticCall0(StaticCall c, StaticCallable target, string name, ValueOrRefType t) {
+  private predicate isDisambiguatingStaticCall0(
+    StaticCall c, StaticCallable target, string name, ValueOrRefType t
+  ) {
     this.isArgument(c, target) and
     name = target.getName() and
     (
@@ -130,10 +127,11 @@ class ExplicitUpcast extends ExplicitCast {
   }
 
   /** Holds if this upcast may be used to disambiguate the target of a static call. */
-  pragma [nomagic]
+  pragma[nomagic]
   private predicate isDisambiguatingStaticCall(StaticCallable other, int args) {
     exists(StaticCall c, StaticCallable target, ValueOrRefType t, string name |
-      this.isDisambiguatingStaticCall0(c, target, name, t) |
+      this.isDisambiguatingStaticCall0(c, target, name, t)
+    |
       hasStaticCallable(t, other, name) and
       args = c.getNumberOfArguments() and
       other != target
@@ -148,7 +146,7 @@ class ExplicitUpcast extends ExplicitCast {
       this.isDisambiguatingExtensionCall(other, args)
       or
       this.isDisambiguatingStaticCall(other, args)
-      |
+    |
       args >= getMinimumArguments(other) and
       not args > getMaximumArguments(other)
     )
@@ -164,29 +162,27 @@ class ExplicitUpcast extends ExplicitCast {
     this = any(OperatorCall oc).getAnArgument()
     or
     this = any(Operation o |
-      not o instanceof Assignment and
+        not o instanceof Assignment and
-      not o instanceof UnaryBitwiseOperation and
+        not o instanceof UnaryBitwiseOperation and
-      not o instanceof SizeofExpr and
+        not o instanceof SizeofExpr and
-      not o instanceof PointerIndirectionExpr and
+        not o instanceof PointerIndirectionExpr and
-      not o instanceof AddressOfExpr and
+        not o instanceof AddressOfExpr and
-      not o instanceof UnaryLogicalOperation and
+        not o instanceof UnaryLogicalOperation and
-      not o instanceof BinaryBitwiseOperation and
+        not o instanceof BinaryBitwiseOperation and
-      not o instanceof LogicalAndExpr and
+        not o instanceof LogicalAndExpr and
-      not o instanceof LogicalOrExpr
+        not o instanceof LogicalOrExpr
-    ).getAnOperand()
+      ).getAnOperand()
     or
-    this = any(LocalVariableDeclAndInitExpr decl |
+    this = any(LocalVariableDeclAndInitExpr decl | decl.isImplicitlyTyped()).getInitializer()
-      decl.isImplicitlyTyped()
-    ).getInitializer()
     or
     exists(LambdaExpr c | c.canReturn(this))
   }
 }
 
 from ExplicitUpcast u, ValueOrRefType src, ValueOrRefType dest
-where src = u.getSourceType()
+where
-  and dest = u.getTargetType()
+  src = u.getSourceType() and
-  and not u.isUseful()
+  dest = u.getTargetType() and
-select u, "There is no need to upcast from $@ to $@ - the conversion can be done implicitly.",
+  not u.isUseful()
-  src, src.getName(),
+select u, "There is no need to upcast from $@ to $@ - the conversion can be done implicitly.", src,
-  dest, dest.getName()
+  src.getName(), dest, dest.getName()

csharp/ql/src/Likely Bugs/BadCheckOdd.ql

@@ -9,6 +9,7 @@
  * @tags reliability
  *       correctness
  */
+
 import csharp
 
 predicate isDefinitelyPositive(Expr e) {
@@ -18,11 +19,16 @@ predicate isDefinitelyPositive(Expr e) {
 }
 
 from BinaryOperation t, RemExpr lhs, IntegerLiteral rhs, string parity
-where t.getLeftOperand() = lhs and
+where
-      t.getRightOperand() = rhs and
+  t.getLeftOperand() = lhs and
-      not isDefinitelyPositive(lhs.getLeftOperand().stripCasts()) and
+  t.getRightOperand() = rhs and
-      lhs.getRightOperand().(IntegerLiteral).getValue() = "2" and
+  not isDefinitelyPositive(lhs.getLeftOperand().stripCasts()) and
-      ((t instanceof EQExpr and rhs.getValue() = "1" and parity = "oddness") or
+  lhs.getRightOperand().(IntegerLiteral).getValue() = "2" and
-       (t instanceof NEExpr and rhs.getValue() = "1" and parity = "evenness") or
+  (
-       (t instanceof GTExpr and rhs.getValue() = "0" and parity = "oddness"))
+    (t instanceof EQExpr and rhs.getValue() = "1" and parity = "oddness")
+    or
+    (t instanceof NEExpr and rhs.getValue() = "1" and parity = "evenness")
+    or
+    (t instanceof GTExpr and rhs.getValue() = "0" and parity = "oddness")
+  )
 select t, "Possibly invalid test for " + parity + ". This will fail for negative numbers."