Merge pull request #658 from calumgrant/cs/extractor/for-is

hvitved · web-flow · commit e14259126e16 · 2018-12-17T16:16:00.000+01:00
C#: Fix extraction bug for variable declarations in for condition
diff --git a/change-notes/1.20/analysis-csharp.md b/change-notes/1.20/analysis-csharp.md
@@ -17,6 +17,7 @@
 
 ## Changes to code extraction
 
+* Fix extraction of `for` statements where the condition declares new variables using `is`.
 * Initializers of `stackalloc` arrays are now extracted.
 
 ## Changes to QL libraries
diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Statements/For.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Statements/For.cs
@@ -28,18 +28,18 @@ protected override void Populate()
                 Expression.Create(cx, init, this, child--);
             }
 
-            Statement.Create(cx, Stmt.Statement, this, 1 + Stmt.Incrementors.Count);
+            if (Stmt.Condition != null)
+            {
+                Expression.Create(cx, Stmt.Condition, this, 0);
+            }
 
             child = 1;
             foreach (var inc in Stmt.Incrementors)
             {
                 Expression.Create(cx, inc, this, child++);
             }
 
-            if (Stmt.Condition != null)
-            {
-                Expression.Create(cx, Stmt.Condition, this, 0);
-            }
+            Statement.Create(cx, Stmt.Statement, this, 1 + Stmt.Incrementors.Count);
         }
     }
 }
diff --git a/csharp/ql/test/library-tests/csharp7/CSharp7.cs b/csharp/ql/test/library-tests/csharp7/CSharp7.cs
@@ -290,3 +290,14 @@ void Test()
         foreach (var (a, b) in list) { }
     }
 }
+
+class ForLoops
+{
+    void Test()
+    {
+        for(int x=0; x<10 && x is int y; ++x)
+        {
+            Console.WriteLine(y);
+        }
+    }
+}
diff --git a/csharp/ql/test/library-tests/csharp7/DefUse.expected b/csharp/ql/test/library-tests/csharp7/DefUse.expected
@@ -66,3 +66,10 @@
 | CSharp7.cs:284:13:284:62 | IEnumerable<(Int32,String)> list = ... | CSharp7.cs:290:32:290:35 | access to local variable list |
 | CSharp7.cs:284:32:284:35 | item | CSharp7.cs:284:41:284:44 | access to parameter item |
 | CSharp7.cs:284:32:284:35 | item | CSharp7.cs:284:51:284:54 | access to parameter item |
+| CSharp7.cs:298:17:298:19 | Int32 x = ... | CSharp7.cs:298:22:298:22 | access to local variable x |
+| CSharp7.cs:298:17:298:19 | Int32 x = ... | CSharp7.cs:298:30:298:30 | access to local variable x |
+| CSharp7.cs:298:17:298:19 | Int32 x = ... | CSharp7.cs:298:44:298:44 | access to local variable x |
+| CSharp7.cs:298:35:298:39 | Int32 y | CSharp7.cs:300:31:300:31 | access to local variable y |
+| CSharp7.cs:298:42:298:44 | ++... | CSharp7.cs:298:22:298:22 | access to local variable x |
+| CSharp7.cs:298:42:298:44 | ++... | CSharp7.cs:298:30:298:30 | access to local variable x |
+| CSharp7.cs:298:42:298:44 | ++... | CSharp7.cs:298:44:298:44 | access to local variable x |
diff --git a/csharp/ql/test/library-tests/csharp7/IsPatterns.expected b/csharp/ql/test/library-tests/csharp7/IsPatterns.expected
@@ -2,3 +2,4 @@
 | CSharp7.cs:238:18:238:31 | ... is ... | CSharp7.cs:238:23:238:28 | access to type String | String | CSharp7.cs:238:23:238:31 | String s1 | false |
 | CSharp7.cs:245:18:245:28 | ... is ... | CSharp7.cs:245:23:245:25 | access to type Object | Object | CSharp7.cs:245:23:245:28 | Object v1 | true |
 | CSharp7.cs:255:27:255:40 | ... is ... | CSharp7.cs:255:32:255:37 | access to type String | String | CSharp7.cs:255:32:255:40 | String s4 | false |
+| CSharp7.cs:298:30:298:39 | ... is ... | CSharp7.cs:298:35:298:37 | access to type Int32 | Int32 | CSharp7.cs:298:35:298:39 | Int32 y | false |
diff --git a/csharp/ql/test/library-tests/csharp7/LocalTaintFlow.expected b/csharp/ql/test/library-tests/csharp7/LocalTaintFlow.expected
@@ -193,3 +193,14 @@
 | CSharp7.cs:288:36:288:39 | access to local variable list | CSharp7.cs:290:32:290:35 | access to local variable list |
 | CSharp7.cs:290:23:290:23 | Int32 a | CSharp7.cs:290:18:290:27 | (..., ...) |
 | CSharp7.cs:290:26:290:26 | String b | CSharp7.cs:290:18:290:27 | (..., ...) |
+| CSharp7.cs:298:17:298:19 | SSA def(x) | CSharp7.cs:298:22:298:39 | SSA phi(x) |
+| CSharp7.cs:298:19:298:19 | 0 | CSharp7.cs:298:17:298:19 | SSA def(x) |
+| CSharp7.cs:298:22:298:22 | access to local variable x | CSharp7.cs:298:22:298:25 | ... < ... |
+| CSharp7.cs:298:22:298:22 | access to local variable x | CSharp7.cs:298:30:298:30 | access to local variable x |
+| CSharp7.cs:298:22:298:25 | ... < ... | CSharp7.cs:298:22:298:39 | ... && ... |
+| CSharp7.cs:298:22:298:39 | SSA phi(x) | CSharp7.cs:298:22:298:22 | access to local variable x |
+| CSharp7.cs:298:30:298:30 | access to local variable x | CSharp7.cs:298:35:298:39 | SSA def(y) |
+| CSharp7.cs:298:30:298:30 | access to local variable x | CSharp7.cs:298:44:298:44 | access to local variable x |
+| CSharp7.cs:298:30:298:39 | ... is ... | CSharp7.cs:298:22:298:39 | ... && ... |
+| CSharp7.cs:298:35:298:39 | SSA def(y) | CSharp7.cs:300:31:300:31 | access to local variable y |
+| CSharp7.cs:298:42:298:44 | SSA def(x) | CSharp7.cs:298:22:298:39 | SSA phi(x) |
diff --git a/csharp/ql/test/library-tests/csharp7/LocalVariables.expected b/csharp/ql/test/library-tests/csharp7/LocalVariables.expected
@@ -65,3 +65,5 @@
 | CSharp7.cs:288:30:288:30 | b | string |
 | CSharp7.cs:290:23:290:23 | a | int |
 | CSharp7.cs:290:26:290:26 | b | string |
+| CSharp7.cs:298:17:298:17 | x | int |
+| CSharp7.cs:298:39:298:39 | y | int |

Original file line number	Diff line number	Diff line change
`@@ -28,18 +28,18 @@ protected override void Populate()`
`28`	`28`	`Expression.Create(cx, init, this, child--);`
`29`	`29`	`}`
`30`	`30`
`31`		`- Statement.Create(cx, Stmt.Statement, this, 1 + Stmt.Incrementors.Count);`
	`31`	`+ if (Stmt.Condition != null)`
	`32`	`+ {`
	`33`	`+ Expression.Create(cx, Stmt.Condition, this, 0);`
	`34`	`+ }`
`32`	`35`
`33`	`36`	`child = 1;`
`34`	`37`	`foreach (var inc in Stmt.Incrementors)`
`35`	`38`	`{`
`36`	`39`	`Expression.Create(cx, inc, this, child++);`
`37`	`40`	`}`
`38`	`41`
`39`		`- if (Stmt.Condition != null)`
`40`		`- {`
`41`		`- Expression.Create(cx, Stmt.Condition, this, 0);`
`42`		`- }`
	`42`	`+ Statement.Create(cx, Stmt.Statement, this, 1 + Stmt.Incrementors.Count);`
`43`	`43`	`}`
`44`	`44`	`}`
`45`	`45`	`}`