C#: Address review comments.

Author: aschackmull

csharp/extractor/Semmle.Extraction.CSharp/Entities/ObjectInitMethod.cs

@@ -13,7 +13,7 @@ private ObjectInitMethod(Context cx, Type containingType)
             this.ContainingType = containingType;
         }
 
-        public static readonly string Name = "<object initializer>";
+        private static readonly string Name = "<object initializer>";
 
         public static ObjectInitMethod Create(Context cx, Type containingType)
         {

csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraphImpl.qll

@@ -14,16 +14,16 @@ private module Initializers {
    * A non-static member with an initializer, for example a field `int Field = 0`.
    */
   class InitializedInstanceMember extends Member {
+    private AssignExpr ae;
+
     InitializedInstanceMember() {
-      exists(AssignExpr ae |
-        not this.isStatic() and
-        expr_parent_top_level(ae, _, this) and
-        not ae = any(Callable c).getExpressionBody()
-      )
+      not this.isStatic() and
+      expr_parent_top_level(ae, _, this) and
+      not ae = any(Callable c).getExpressionBody()
     }
 
     /** Gets the initializer expression. */
-    AssignExpr getInitializer() { expr_parent_top_level(result, _, this) }
+    AssignExpr getInitializer() { result = ae }
   }
 
   /**
@@ -54,7 +54,7 @@ private module Initializers {
   }
 
   /**
-   * Gets the last member initializer expression for non-static constructor `c`
+   * Gets the last member initializer expression for object initializer method `obinit`
    * in compilation `comp`.
    */
   AssignExpr lastInitializer(ObjectInitMethod obinit, CompilationExt comp) {
@@ -224,6 +224,13 @@ predicate scopeLast(CfgScope scope, AstNode last, Completion c) {
       or
       last(callable.(Constructor).getInitializer(), last, c) and
       not callable.hasBody()
+      or
+      // This is only relevant in the context of compilation errors, since
+      // normally the existence of an object initializer call implies the
+      // existence of an initializer.
+      last(callable.(Constructor).getObjectInitializerCall(), last, c) and
+      not callable.(Constructor).hasInitializer() and
+      not callable.hasBody()
     )
   or
   last(Initializers::lastInitializer(scope, _), last, c)
@@ -278,8 +285,15 @@ private class ConstructorTree extends ControlFlowTree instanceof Constructor {
   final override predicate succ(AstNode pred, AstNode succ, Completion c) {
     exists(CompilationExt comp |
       last(this.getObjectInitializerCall(comp), pred, c) and
-      c instanceof NormalCompletion and
+      c instanceof NormalCompletion
+    |
       first(this.getInitializer(comp), succ)
+      or
+      // This is only relevant in the context of compilation errors, since
+      // normally the existence of an object initializer call implies the
+      // existence of an initializer.
+      not exists(this.getInitializer(comp)) and
+      first(this.getBody(comp), succ)
     )
   }
 }

csharp/ql/test/library-tests/obinit/Flow.expected

@@ -6,6 +6,8 @@ edges
 | obinit.cs:20:15:20:15 | access to local variable a : A [field s] : String | obinit.cs:21:18:21:18 | access to local variable a : A [field s] : String | provenance |  |
 | obinit.cs:20:19:20:25 | object creation of type A : A [field s] : String | obinit.cs:20:15:20:15 | access to local variable a : A [field s] : String | provenance |  |
 | obinit.cs:21:18:21:18 | access to local variable a : A [field s] : String | obinit.cs:21:18:21:20 | access to field s | provenance |  |
+flow
+| obinit.cs:21:18:21:20 | access to field s |
 nodes
 | obinit.cs:5:23:5:23 | [post] this access : A [field s] : String | semmle.label | [post] this access : A [field s] : String |
 | obinit.cs:5:27:5:34 | "source" : String | semmle.label | "source" : String |
@@ -16,5 +18,3 @@ nodes
 | obinit.cs:21:18:21:18 | access to local variable a : A [field s] : String | semmle.label | access to local variable a : A [field s] : String |
 | obinit.cs:21:18:21:20 | access to field s | semmle.label | access to field s |
 subpaths
-#select
-| obinit.cs:21:18:21:20 | access to field s |

csharp/ql/test/library-tests/obinit/Flow.ql

@@ -17,6 +17,4 @@ module Flow = DataFlow::Global<FlowConfig>;
 
 import Flow::PathGraph
 
-from DataFlow::Node source, DataFlow::Node sink
-where Flow::flow(source, sink)
-select sink
+query predicate flow(DataFlow::Node sink) { Flow::flowTo(sink) }

csharp/ql/test/library-tests/obinit/Flow.qlref

@@ -0,0 +1,2 @@
+query: Flow.ql
+postprocess: utils/test/InlineExpectationsTestQuery.ql

csharp/ql/test/library-tests/obinit/obinit.cs

@@ -18,13 +18,13 @@ static void Sink(string s) { }
 
         static void Foo() {
             A a = new A();
-            Sink(a.s);
+            Sink(a.s); // $ flow
 
             A a2 = new A(0, 0);
-            Sink(a2.s);
+            Sink(a2.s); // $ MISSING: flow
 
             B b = new B();
-            Sink(b.s);
+            Sink(b.s); // $ MISSING: flow
         }
     }
 }