Merge pull request #4136 from tamasvajk/feature/qldocs

C#: Add missing QlDocs

Author: hvitved

csharp/ql/src/Linq/Helpers.qll

@@ -6,18 +6,32 @@
 import csharp
 
 //#################### PREDICATES ####################
-Stmt firstStmt(ForeachStmt fes) {
+private Stmt firstStmt(ForeachStmt fes) {
   if fes.getBody() instanceof BlockStmt
   then result = fes.getBody().(BlockStmt).getStmt(0)
   else result = fes.getBody()
 }
 
+private int numStmts(ForeachStmt fes) {
+  if fes.getBody() instanceof BlockStmt
+  then result = count(fes.getBody().(BlockStmt).getAStmt())
+  else result = 1
+}
+
+/** Holds if the type's qualified name is "System.Linq.Enumerable" */
 predicate isEnumerableType(ValueOrRefType t) { t.hasQualifiedName("System.Linq.Enumerable") }
 
+/** Holds if the type's qualified name starts with "System.Collections.Generic.IEnumerable" */
 predicate isIEnumerableType(ValueOrRefType t) {
   t.getQualifiedName().matches("System.Collections.Generic.IEnumerable%")
 }
 
+/**
+ * Holds if `foreach` statement `fes` could be converted to a `.All()` call.
+ * That is, the `ForeachStmt` contains a single `if` with a condition that
+ * accesses the loop variable and with a body that assigns `false` to a variable
+ * and `break`s out of the `foreach`.
+ */
 predicate missedAllOpportunity(ForeachStmt fes) {
   exists(IfStmt is |
     // The loop contains an if statement with no else case, and nothing else.
@@ -36,6 +50,12 @@ predicate missedAllOpportunity(ForeachStmt fes) {
   )
 }
 
+/**
+ * Holds if `foreach` statement `fes` could be converted to a `.Cast()` call.
+ * That is, the loop variable is accessed only in the first statement of the
+ * block, and the access is a cast. The first statement needs to be a
+ * `LocalVariableDeclStmt`.
+ */
 predicate missedCastOpportunity(ForeachStmt fes, LocalVariableDeclStmt s) {
   s = firstStmt(fes) and
   forex(VariableAccess va | va = fes.getVariable().getAnAccess() |
@@ -47,6 +67,12 @@ predicate missedCastOpportunity(ForeachStmt fes, LocalVariableDeclStmt s) {
   )
 }
 
+/**
+ * Holds if `foreach` statement `fes` could be converted to an `.OfType()` call.
+ * That is, the loop variable is accessed only in the first statement of the
+ * block, and the access is a cast with the `as` operator. The first statement
+ * needs to be a `LocalVariableDeclStmt`.
+ */
 predicate missedOfTypeOpportunity(ForeachStmt fes, LocalVariableDeclStmt s) {
   s = firstStmt(fes) and
   forex(VariableAccess va | va = fes.getVariable().getAnAccess() |
@@ -58,6 +84,12 @@ predicate missedOfTypeOpportunity(ForeachStmt fes, LocalVariableDeclStmt s) {
   )
 }
 
+/**
+ * Holds if `foreach` statement `fes` could be converted to a `.Select()` call.
+ * That is, the loop variable is accessed only in the first statement of the
+ * block, and the access is not a cast. The first statement needs to be a
+ * `LocalVariableDeclStmt`.
+ */
 predicate missedSelectOpportunity(ForeachStmt fes, LocalVariableDeclStmt s) {
   s = firstStmt(fes) and
   forex(VariableAccess va | va = fes.getVariable().getAnAccess() |
@@ -66,6 +98,12 @@ predicate missedSelectOpportunity(ForeachStmt fes, LocalVariableDeclStmt s) {
   not s.getAVariableDeclExpr().getInitializer() instanceof Cast
 }
 
+/**
+ * Holds if `foreach` statement `fes` could be converted to a `.Where()` call.
+ * That is, first statement of the loop is an `if`, which accesses the loop
+ * variable, and the body of the `if` is either a `continue` or there's nothing
+ * else in the loop than the `if`.
+ */
 predicate missedWhereOpportunity(ForeachStmt fes, IfStmt is) {
   // The very first thing the foreach loop does is test its iteration variable.
   is = firstStmt(fes) and
@@ -82,12 +120,6 @@ predicate missedWhereOpportunity(ForeachStmt fes, IfStmt is) {
   )
 }
 
-int numStmts(ForeachStmt fes) {
-  if fes.getBody() instanceof BlockStmt
-  then result = count(fes.getBody().(BlockStmt).getAStmt())
-  else result = 1
-}
-
 //#################### CLASSES ####################
 /** A LINQ Any(...) call. */
 class AnyCall extends MethodCall {

csharp/ql/src/external/CodeDuplication.qll

@@ -66,9 +66,9 @@ class SimilarBlock extends Copy, @similarity {
   }
 }
 
-Method sourceMethod() { method_location(result, _) and numlines(result, _, _, _) }
+private Method sourceMethod() { method_location(result, _) and numlines(result, _, _, _) }
 
-int numberOfSourceMethods(Class c) {
+private int numberOfSourceMethods(Class c) {
   result = count(Method m | m = sourceMethod() and m.getDeclaringType() = c)
 }
 
@@ -97,6 +97,7 @@ private predicate duplicateStatement(Method m1, Method m2, Stmt s1, Stmt s2) {
   )
 }
 
+/** Holds if `duplicate` number of statements are duplicated in the methods. */
 predicate duplicateStatements(Method m1, Method m2, int duplicate, int total) {
   duplicate = strictcount(Stmt s | duplicateStatement(m1, m2, s, _)) and
   total = strictcount(statementInMethod(m1))
@@ -109,7 +110,7 @@ predicate duplicateMethod(Method m, Method other) {
   exists(int total | duplicateStatements(m, other, total, total))
 }
 
-predicate similarLines(File f, int line) {
+private predicate similarLines(File f, int line) {
   exists(SimilarBlock b | b.sourceFile() = f and line in [b.sourceStartLine() .. b.sourceEndLine()])
 }
 
@@ -148,7 +149,7 @@ private predicate similarLinesCovered(File f, int coveredLines, File otherFile)
   )
 }
 
-predicate duplicateLines(File f, int line) {
+private predicate duplicateLines(File f, int line) {
   exists(DuplicateBlock b |
     b.sourceFile() = f and line in [b.sourceStartLine() .. b.sourceEndLine()]
   )
@@ -189,6 +190,7 @@ private predicate duplicateLinesCovered(File f, int coveredLines, File otherFile
   )
 }
 
+/** Holds if the two files are not duplicated but have more than 80% similar lines. */
 predicate similarFiles(File f, File other, int percent) {
   exists(int covered, int total |
     similarLinesCovered(f, covered, other) and
@@ -199,6 +201,7 @@ predicate similarFiles(File f, File other, int percent) {
   not duplicateFiles(f, other, _)
 }
 
+/** Holds if the two files have more than 70% duplicated lines. */
 predicate duplicateFiles(File f, File other, int percent) {
   exists(int covered, int total |
     duplicateLinesCovered(f, covered, other) and
@@ -209,7 +212,7 @@ predicate duplicateFiles(File f, File other, int percent) {
 }
 
 pragma[noopt]
-predicate duplicateAnonymousClass(AnonymousClass c, AnonymousClass other) {
+private predicate duplicateAnonymousClass(AnonymousClass c, AnonymousClass other) {
   exists(int numDup |
     numDup =
       strictcount(Method m1 |
@@ -248,6 +251,7 @@ private predicate mostlyDuplicateClassBase(Class c, Class other, int numDup, int
   not other instanceof AnonymousClass
 }
 
+/** Holds if the methods in the two classes are more than 80% duplicated. */
 predicate mostlyDuplicateClass(Class c, Class other, string message) {
   exists(int numDup, int total |
     mostlyDuplicateClassBase(c, other, numDup, total) and
@@ -272,19 +276,28 @@ predicate mostlyDuplicateClass(Class c, Class other, string message) {
   )
 }
 
+/** Holds if the two files are similar or duplicated. */
 predicate fileLevelDuplication(File f, File other) {
   similarFiles(f, other, _) or duplicateFiles(f, other, _)
 }
 
+/**
+ * Holds if the two classes are duplicated anonymous classes or more than 80% of
+ * their methods are duplicated.
+ */
 predicate classLevelDuplication(Class c, Class other) {
   duplicateAnonymousClass(c, other) or mostlyDuplicateClass(c, other, _)
 }
 
-Element whitelistedDuplicateElement() {
+private Element whitelistedDuplicateElement() {
   result instanceof UsingNamespaceDirective or
   result instanceof UsingStaticDirective
 }
 
+/**
+ * Holds if the `line` in the `file` contains an element, such as a `using`
+ * directive, that is not considered for code duplication.
+ */
 predicate whitelistedLineForDuplication(File file, int line) {
   exists(Location loc | loc = whitelistedDuplicateElement().getLocation() |
     line = loc.getStartLine() and file = loc.getFile()

csharp/ql/src/semmle/code/cil/Access.qll

@@ -31,6 +31,7 @@ class ReadRefAccess extends ReadAccess, ReadRef { }
 
 /** An instruction that writes a variable. */
 class WriteAccess extends VariableAccess, @cil_write_access {
+  /** Gets the expression whose value is used in this variable write. */
   Expr getExpr() { none() }
 }
 

csharp/ql/src/semmle/code/csharp/Callable.qll

@@ -437,6 +437,9 @@ class Operator extends Callable, Member, Attributable, @operator {
 
   override string getName() { operators(this, _, result, _, _, _) }
 
+  /**
+   * Gets the metadata name of the operator, such as `op_implicit` or `op_RightShift`.
+   */
   string getFunctionName() { none() }
 
   override ValueOrRefType getDeclaringType() { operators(this, _, _, result, _, _) }

csharp/ql/src/semmle/code/csharp/Conversion.qll

@@ -514,6 +514,11 @@ predicate convNullableType(ValueOrRefType fromType, NullableType toType) {
   )
 }
 
+/**
+ * Holds if `fromType` is `NullType`, and `toType` is a type that can represent
+ * the `null` value, such as a reference type, `Nullable<T>` or a type parameter
+ * with contraints that restrict it to a reference type.
+ */
 // This is a deliberate, small Cartesian product, so we have manually lifted it to force the
 // evaluator to evaluate it in its entirety, rather than trying to optimize it in context.
 pragma[noinline]

csharp/ql/src/semmle/code/csharp/dispatch/RuntimeCallable.qll

@@ -26,6 +26,7 @@ class RuntimeMethod extends RuntimeCallable {
     this instanceof CIL::Method
   }
 
+  /** Holds if the method is `static`. */
   predicate isStatic() { this.(Method).isStatic() or this.(CIL::Method).isStatic() }
 }
 

csharp/ql/src/semmle/code/csharp/exprs/Expr.qll

@@ -1011,6 +1011,7 @@ class RangeExpr extends Expr, @range_expr {
 
 /** An index expression, for example `^1` meaning "1 from the end". */
 class IndexExpr extends Expr, @index_expr {
+  /** Gets the sub expression of this index expression. */
   Expr getExpr() { result.getParent() = this }
 
   override string toString() { result = "^..." }

csharp/ql/src/semmle/code/csharp/frameworks/EntityFramework.qll

@@ -8,7 +8,12 @@ private import semmle.code.csharp.frameworks.system.collections.Generic
 private import semmle.code.csharp.frameworks.Sql
 private import semmle.code.csharp.dataflow.LibraryTypeDataFlow
 
+/**
+ * Definitions relating to the `System.ComponentModel.DataAnnotations`
+ * namespace.
+ */
 module DataAnnotations {
+  /** Class for `NotMappedAttribute`. */
   class NotMappedAttribute extends Attribute {
     NotMappedAttribute() {
       this
@@ -18,6 +23,10 @@ module DataAnnotations {
   }
 }
 
+/**
+ * Definitions relating to the `Microsoft.EntityFrameworkCore` or
+ * `System.Data.Entity` namespaces.
+ */
 module EntityFramework {
   /** An EF6 or EFCore namespace. */
   class EFNamespace extends Namespace {
@@ -43,12 +52,14 @@ module EntityFramework {
   class DbContext extends EFClass {
     DbContext() { this.getName() = "DbContext" }
 
+    /** Gets a `Find` or `FindAsync` method in the `DbContext`. */
     Method getAFindMethod() {
       result = this.getAMethod("Find")
       or
       result = this.getAMethod("FindAsync")
     }
 
+    /** Gets an `Update` method in the `DbContext`. */
     Method getAnUpdateMethod() { result = this.getAMethod("Update") }
   }
 
@@ -119,6 +130,7 @@ module EntityFramework {
       preservesValue = false
     }
 
+    /** Gets a conversion operator from `string` to `RawSqlString`. */
     ConversionOperator getAConversionTo() {
       result = this.getAMember() and
       result.getTargetType() instanceof RawSqlStringStruct and

csharp/ql/src/semmle/code/csharp/frameworks/JsonNET.qll

@@ -135,8 +135,10 @@ module JsonNET {
   class JsonSerializerClass extends JsonClass, LibraryTypeDataFlow {
     JsonSerializerClass() { this.hasName("JsonSerializer") }
 
+    /** Gets the method for `JsonSerializer.Serialize`. */
     Method getSerializeMethod() { result = this.getAMethod("Serialize") }
 
+    /** Gets the method for `JsonSerializer.Deserialize`. */
     Method getDeserializeMethod() { result = this.getAMethod("Deserialize") }
 
     override predicate callableFlow(

csharp/ql/src/semmle/code/csharp/frameworks/NHibernate.qll

@@ -1,8 +1,13 @@
+/**
+ * Classes for modelling NHibernate.
+ */
+
 import csharp
 private import semmle.code.csharp.frameworks.System
 private import semmle.code.csharp.frameworks.system.Collections
 private import semmle.code.csharp.frameworks.Sql
 
+/** Definitions relating to the `NHibernate` package. */
 module NHibernate {
   /** A class that is mapped to the database. */
   abstract class MappedClass extends Class { }