C#: Address review comments

calumgrant · calumgrant · commit 20752c80c947 · 2019-06-04T18:10:49.000+01:00
diff --git a/csharp/ql/src/semmle/code/csharp/Stmt.qll b/csharp/ql/src/semmle/code/csharp/Stmt.qll
@@ -1295,11 +1295,11 @@ class LocalConstantDeclStmt extends LocalVariableDeclStmt, @const_decl_stmt {
 class UsingDeclStmt extends LocalVariableDeclStmt, UsingStmt, @using_decl_stmt {
   override string toString() { result = "using ... ...;" }
 
-  override LocalConstantDeclExpr getAVariableDeclExpr() {
+  override LocalVariableDeclExpr getAVariableDeclExpr() {
     result = LocalVariableDeclStmt.super.getAVariableDeclExpr()
   }
 
-  override LocalConstantDeclExpr getVariableDeclExpr(int n) {
+  override LocalVariableDeclExpr getVariableDeclExpr(int n) {
     result = LocalVariableDeclStmt.super.getVariableDeclExpr(n)
   }
 
diff --git a/csharp/ql/test/library-tests/assignables/AssignableAccess.expected b/csharp/ql/test/library-tests/assignables/AssignableAccess.expected
@@ -104,6 +104,7 @@
 | Assignables.cs:133:15:133:15 | access to local variable x | Assignables.cs:132:13:132:13 | x | read |
 | Assignables.cs:133:15:133:15 | access to local variable x | Assignables.cs:132:13:132:13 | x | write |
 | Assignables.cs:133:29:133:29 | String s | Assignables.cs:133:29:133:29 | s | write |
+| Assignables.cs:138:19:138:19 | access to local variable x | Assignables.cs:138:19:138:19 | x | write |
 | Discards.cs:7:9:7:9 | access to parameter x | Discards.cs:5:30:5:30 | x | write |
 | Discards.cs:20:32:20:32 | Boolean z | Discards.cs:20:32:20:32 | z | write |
 | Discards.cs:25:27:25:30 | access to parameter args | Discards.cs:23:27:23:30 | args | read |
diff --git a/csharp/ql/test/library-tests/assignables/AssignableDefinition.expected b/csharp/ql/test/library-tests/assignables/AssignableDefinition.expected
@@ -87,6 +87,7 @@
 | Assignables.cs:132:13:132:13 | x | Assignables.cs:132:13:132:17 | Int32 x = ... | Assignables.cs:132:13:132:13 | access to local variable x | Assignables.cs:132:17:132:17 | 0 | certain |
 | Assignables.cs:132:13:132:13 | x | Assignables.cs:133:15:133:15 | access to local variable x | Assignables.cs:133:15:133:15 | access to local variable x | Assignables.cs:133:15:133:15 | <none> | certain |
 | Assignables.cs:133:29:133:29 | s | Assignables.cs:133:29:133:29 | String s | Assignables.cs:133:29:133:29 | String s | Assignables.cs:133:29:133:29 | <none> | certain |
+| Assignables.cs:138:19:138:19 | x | Assignables.cs:138:19:138:50 | MemoryStream x = ... | Assignables.cs:138:19:138:19 | access to local variable x | Assignables.cs:138:23:138:50 | object creation of type MemoryStream | certain |
 | Discards.cs:5:30:5:30 | x | Discards.cs:5:30:5:30 | x | Discards.cs:5:30:5:30 | <none> | Discards.cs:5:30:5:30 | <none> | certain |
 | Discards.cs:5:30:5:30 | x | Discards.cs:7:9:7:17 | ... = ... | Discards.cs:7:9:7:9 | access to parameter x | Discards.cs:7:13:7:17 | false | certain |
 | Discards.cs:19:14:19:14 | x | Discards.cs:19:9:19:29 | ... = ... | Discards.cs:19:9:19:29 | <none> | Discards.cs:19:9:19:29 | <none> | certain |
diff --git a/csharp/ql/test/library-tests/assignables/Assignables.cs b/csharp/ql/test/library-tests/assignables/Assignables.cs
@@ -132,4 +132,11 @@ void DelegateRef(Delegate d)
         var x = 0;
         d(ref x, out string s);
     }
+
+    void UsingDeclarations()
+    {
+        using var x = new System.IO.MemoryStream();
+    }
 }
+
+// semmle-extractor-options: /langversion:8.0
diff --git a/csharp/ql/test/library-tests/assignables/Assignables.expected b/csharp/ql/test/library-tests/assignables/Assignables.expected
@@ -95,6 +95,7 @@
 | Assignables.cs:130:31:130:31 | d |
 | Assignables.cs:132:13:132:13 | x |
 | Assignables.cs:133:29:133:29 | s |
+| Assignables.cs:138:19:138:19 | x |
 | Discards.cs:5:6:5:8 | Item1 |
 | Discards.cs:5:11:5:16 | Item2 |
 | Discards.cs:5:30:5:30 | x |
diff --git a/csharp/ql/test/library-tests/assignables/GetAnAssignedValue.expected b/csharp/ql/test/library-tests/assignables/GetAnAssignedValue.expected
@@ -37,6 +37,7 @@
 | Assignables.cs:125:13:125:13 | s | Assignables.cs:125:17:125:25 | nameof(...) |
 | Assignables.cs:125:13:125:13 | s | Assignables.cs:126:13:126:30 | nameof(...) |
 | Assignables.cs:132:13:132:13 | x | Assignables.cs:132:17:132:17 | 0 |
+| Assignables.cs:138:19:138:19 | x | Assignables.cs:138:23:138:50 | object creation of type MemoryStream |
 | Discards.cs:5:30:5:30 | x | Discards.cs:7:13:7:17 | false |
 | Finally.cs:7:13:7:13 | i | Finally.cs:7:17:7:17 | 0 |
 | Finally.cs:7:13:7:13 | i | Finally.cs:15:17:15:17 | 1 |
diff --git a/csharp/ql/test/library-tests/csharp8/UsingDeclarations.expected b/csharp/ql/test/library-tests/csharp8/UsingDeclarations.expected
@@ -4,13 +4,23 @@ localVars
 | UsingDeclarations.cs:10:26:10:30 | file3 |
 | UsingDeclarations.cs:10:72:10:76 | file4 |
 localVariableDeclarations
+| UsingDeclarations.cs:8:9:8:116 | using ... ...; | 0 | UsingDeclarations.cs:8:26:8:69 | FileStream file1 = ... |
+| UsingDeclarations.cs:8:9:8:116 | using ... ...; | 1 | UsingDeclarations.cs:8:72:8:115 | FileStream file2 = ... |
 usingStmts1
 | UsingDeclarations.cs:8:9:8:116 | using ... ...; |
 | UsingDeclarations.cs:10:9:12:9 | using (...) {...} |
 | UsingDeclarations.cs:14:9:15:13 | using (...) {...} |
 usingStmts
+| UsingDeclarations.cs:8:9:8:116 | using ... ...; | 0 | UsingDeclarations.cs:8:26:8:69 | FileStream file1 = ... |
+| UsingDeclarations.cs:8:9:8:116 | using ... ...; | 1 | UsingDeclarations.cs:8:72:8:115 | FileStream file2 = ... |
 | UsingDeclarations.cs:10:9:12:9 | using (...) {...} | 0 | UsingDeclarations.cs:10:26:10:69 | FileStream file3 = ... |
 | UsingDeclarations.cs:10:9:12:9 | using (...) {...} | 1 | UsingDeclarations.cs:10:72:10:115 | FileStream file4 = ... |
 usingDecls
 | UsingDeclarations.cs:8:9:8:116 | using ... ...; | 0 | UsingDeclarations.cs:8:26:8:69 | FileStream file1 = ... |
 | UsingDeclarations.cs:8:9:8:116 | using ... ...; | 1 | UsingDeclarations.cs:8:72:8:115 | FileStream file2 = ... |
+usingExprs
+| UsingDeclarations.cs:8:9:8:116 | using ... ...; | UsingDeclarations.cs:8:34:8:69 | object creation of type FileStream |
+| UsingDeclarations.cs:8:9:8:116 | using ... ...; | UsingDeclarations.cs:8:80:8:115 | object creation of type FileStream |
+| UsingDeclarations.cs:10:9:12:9 | using (...) {...} | UsingDeclarations.cs:10:34:10:69 | object creation of type FileStream |
+| UsingDeclarations.cs:10:9:12:9 | using (...) {...} | UsingDeclarations.cs:10:80:10:115 | object creation of type FileStream |
+| UsingDeclarations.cs:14:9:15:13 | using (...) {...} | UsingDeclarations.cs:14:15:14:50 | object creation of type FileStream |
diff --git a/csharp/ql/test/library-tests/csharp8/UsingDeclarations.ql b/csharp/ql/test/library-tests/csharp8/UsingDeclarations.ql
@@ -18,3 +18,5 @@ query predicate usingStmts(UsingStmt stmt, int i, LocalVariableDeclExpr decl) {
 }
 
 query predicate usingDecls(UsingDeclStmt stmt, int i, Expr e) { e = stmt.getChild(i) }
+
+query predicate usingExprs(UsingStmt stmt, Expr e) { e = stmt.getAnExpr() }
diff --git a/csharp/ql/test/query-tests/API Abuse/DisposeNotCalledOnException/DisposeNotCalledOnException.cs b/csharp/ql/test/query-tests/API Abuse/DisposeNotCalledOnException/DisposeNotCalledOnException.cs
@@ -58,6 +58,10 @@ public void Method()
         SqlConnection c1f = new SqlConnection();
         Throw2(c1f);
         c1f.Dispose();
+
+        // GOOD: using declaration
+        using SqlConnection c2 = new SqlConnection("");
+        c2.Open();
     }
 
     void Throw1(SqlConnection sc)
@@ -71,3 +75,5 @@ SqlConnection Throw2(SqlConnection sc)
         return sc == null ? throw new Exception() : sc;
     }
 }
+
+// semmle-extractor-options: /langversion:8.0
diff --git a/csharp/ql/test/query-tests/API Abuse/NoDisposeCallOnLocalIDisposable/NoDisposeCallOnLocalIDisposable.cs b/csharp/ql/test/query-tests/API Abuse/NoDisposeCallOnLocalIDisposable/NoDisposeCallOnLocalIDisposable.cs
@@ -78,6 +78,9 @@ public IDisposable Method()
         // GOOD: Passed to a library. This is only detected in CIL.
         Console.SetOut(new StreamWriter("output.txt"));
 
+        // GOOD: Disposed automatically.
+        using var c2 = new Timer(TimerProc);
+
         return null;
     }
 
@@ -88,3 +91,5 @@ void TimerProc(object obj)
     {
     }
 }
+
+// semmle-extractor-options: /langversion:8.0
diff --git a/csharp/upgrades/22d4df34563b234c6fc108ccebdffe8ae9b8e49c/upgrade.properties b/csharp/upgrades/22d4df34563b234c6fc108ccebdffe8ae9b8e49c/upgrade.properties
@@ -1,2 +1,2 @@
-description: Support for null-coalescing assignment, static local functions and using declarations.
+description: Support for null-coalescing assignment, static local functions, and using declarations.
 compatibility: backwards

Original file line number	Diff line number	Diff line change
`@@ -1295,11 +1295,11 @@ class LocalConstantDeclStmt extends LocalVariableDeclStmt, @const_decl_stmt {`
`1295`	`1295`	`class UsingDeclStmt extends LocalVariableDeclStmt, UsingStmt, @using_decl_stmt {`
`1296`	`1296`	`override string toString() { result = "using ... ...;" }`
`1297`	`1297`
`1298`		`- override LocalConstantDeclExpr getAVariableDeclExpr() {`
	`1298`	`+ override LocalVariableDeclExpr getAVariableDeclExpr() {`
`1299`	`1299`	`result = LocalVariableDeclStmt.super.getAVariableDeclExpr()`
`1300`	`1300`	`}`
`1301`	`1301`
`1302`		`- override LocalConstantDeclExpr getVariableDeclExpr(int n) {`
	`1302`	`+ override LocalVariableDeclExpr getVariableDeclExpr(int n) {`
`1303`	`1303`	`result = LocalVariableDeclStmt.super.getVariableDeclExpr(n)`
`1304`	`1304`	`}`
`1305`	`1305`
Original file line number	Diff line number	Diff line change
`@@ -132,4 +132,11 @@ void DelegateRef(Delegate d)`
`132`	`132`	`var x = 0;`
`133`	`133`	`d(ref x, out string s);`
`134`	`134`	`}`
	`135`	`+`
	`136`	`+ void UsingDeclarations()`
	`137`	`+ {`
	`138`	`+ using var x = new System.IO.MemoryStream();`
	`139`	`+ }`
`135`	`140`	`}`
	`141`	`+`
	`142`	`+// semmle-extractor-options: /langversion:8.0`
Original file line number	Diff line number	Diff line change
`@@ -18,3 +18,5 @@ query predicate usingStmts(UsingStmt stmt, int i, LocalVariableDeclExpr decl) {`
`18`	`18`	`}`
`19`	`19`
`20`	`20`	`query predicate usingDecls(UsingDeclStmt stmt, int i, Expr e) { e = stmt.getChild(i) }`
	`21`	`+`
	`22`	`+query predicate usingExprs(UsingStmt stmt, Expr e) { e = stmt.getAnExpr() }`
Original file line number	Diff line number	Diff line change
`@@ -58,6 +58,10 @@ public void Method()`
`58`	`58`	`SqlConnection c1f = new SqlConnection();`
`59`	`59`	`Throw2(c1f);`
`60`	`60`	`c1f.Dispose();`
	`61`	`+`
	`62`	`+ // GOOD: using declaration`
	`63`	`+ using SqlConnection c2 = new SqlConnection("");`
	`64`	`+ c2.Open();`
`61`	`65`	`}`
`62`	`66`
`63`	`67`	`void Throw1(SqlConnection sc)`
`@@ -71,3 +75,5 @@ SqlConnection Throw2(SqlConnection sc)`
`71`	`75`	`return sc == null ? throw new Exception() : sc;`
`72`	`76`	`}`
`73`	`77`	`}`
	`78`	`+`
	`79`	`+// semmle-extractor-options: /langversion:8.0`
Original file line number	Diff line number	Diff line change
`@@ -78,6 +78,9 @@ public IDisposable Method()`
`78`	`78`	`// GOOD: Passed to a library. This is only detected in CIL.`
`79`	`79`	`Console.SetOut(new StreamWriter("output.txt"));`
`80`	`80`
	`81`	`+ // GOOD: Disposed automatically.`
	`82`	`+ using var c2 = new Timer(TimerProc);`
	`83`	`+`
`81`	`84`	`return null;`
`82`	`85`	`}`
`83`	`86`
`@@ -88,3 +91,5 @@ void TimerProc(object obj)`
`88`	`91`	`{`
`89`	`92`	`}`
`90`	`93`	`}`
	`94`	`+`
	`95`	`+// semmle-extractor-options: /langversion:8.0`