Merge pull request #140 from hvitved/csharp/cfg/renaming

C#: Rename and restructure control flow graph entities

Author: calumgrant

change-notes/1.18/analysis-csharp.md

@@ -40,4 +40,22 @@
 
 ## Changes to QL libraries
 
-* A new non-member predicate `mayBeDisposed()` can be used to determine if a variable is potentially disposed inside a library. It will analyse the CIL code in the library to determine this.
+* A new non-member predicate `mayBeDisposed()` can be used to determine if a variable is potentially disposed inside a library. It will analyse the CIL code in the library to determine this.
+* Several control flow graph entities have been renamed (the old names still exist for backwards compatibility):
+  - `ControlFlowNode` has been renamed to `ControlFlow::Node`.
+  - `CallableEntryNode` has been renamed to `ControlFlow::Nodes::EntryNode`.
+  - `CallableExitNode` has been renamed to `ControlFlow::Nodes::ExitNode`.
+  - `ControlFlowEdgeType` has been renamed to `ControlFlow::SuccessorType`.
+  - `ControlFlowEdgeSuccessor` has been renamed to `ControlFlow::SuccessorTypes::NormalSuccessor`.
+  - `ControlFlowEdgeConditional` has been renamed to `ControlFlow::SuccessorTypes::ConditionalSuccessor`.
+  - `ControlFlowEdgeBoolean` has been renamed to `ControlFlow::SuccessorTypes::BooleanSuccessor`.
+  - `ControlFlowEdgeNullness` has been renamed to `ControlFlow::SuccessorTypes::NullnessSuccessor`.
+  - `ControlFlowEdgeMatching` has been renamed to `ControlFlow::SuccessorTypes::MatchingSuccessor`.
+  - `ControlFlowEdgeEmptiness` has been renamed to `ControlFlow::SuccessorTypes::EmptinessSuccessor`.
+  - `ControlFlowEdgeReturn` has been renamed to `ControlFlow::SuccessorTypes::ReturnSuccessor`.
+  - `ControlFlowEdgeBreak` has been renamed to `ControlFlow::SuccessorTypes::BreakSuccessor`.
+  - `ControlFlowEdgeContinue` has been renamed to `ControlFlow::SuccessorTypes::ContinueSuccessor`.
+  - `ControlFlowEdgeGotoLabel` has been renamed to `ControlFlow::SuccessorTypes::GotoLabelSuccessor`.
+  - `ControlFlowEdgeGotoCase` has been renamed to `ControlFlow::SuccessorTypes::GotoCaseSuccessor`.
+  - `ControlFlowEdgeGotoDefault` has been renamed to `ControlFlow::SuccessorTypes::GotoDefaultSuccessor`.
+  - `ControlFlowEdgeException` has been renamed to `ControlFlow::SuccessorTypes::ExceptionSuccessor`.

csharp/ql/src/API Abuse/Dispose.qll

@@ -5,7 +5,7 @@ private import semmle.code.csharp.frameworks.system.web.UI
 
 class DisposableType extends RefType {
   DisposableType() {
-    this.getABaseType+() = getSystemIDisposableInterface()
+    this.getABaseType+() instanceof SystemIDisposableInterface
   }
 }
 
@@ -17,13 +17,13 @@ class DisposableField extends Field {
 
 class WebControl extends RefType {
   WebControl() {
-    this.getBaseClass*() = getSystemWebUIControlClass()
+    this.getBaseClass*() instanceof SystemWebUIControlClass
   }
 }
 
 class WebPage extends RefType {
   WebPage() {
-    this.getBaseClass*() = getSystemWebUIPageClass()
+    this.getBaseClass*() instanceof SystemWebUIPageClass
   }
 }
 

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

@@ -14,7 +14,6 @@
 import csharp
 import semmle.code.csharp.commons.Assertions
 import semmle.code.csharp.commons.Constants
-import ControlFlowGraph
 
 /** A constant condition. */
 abstract class ConstantCondition extends Expr {
@@ -76,13 +75,13 @@ class ConstantNullnessCondition extends ConstantCondition {
   boolean b;
 
   ConstantNullnessCondition() {
-    forex(ControlFlowNode cfn |
+    forex(ControlFlow::Node cfn |
       cfn = this.getAControlFlowNode() |
-      exists(ControlFlowEdgeNullness t |
+      exists(ControlFlow::SuccessorTypes::NullnessSuccessor t |
         exists(cfn.getASuccessorByType(t)) |
         if t.isNull() then b = true else b = false
       ) and
-      strictcount(ControlFlowEdgeType t | exists(cfn.getASuccessorByType(t))) = 1
+      strictcount(ControlFlow::SuccessorType t | exists(cfn.getASuccessorByType(t))) = 1
     )
   }
 
@@ -99,13 +98,13 @@ class ConstantMatchingCondition extends ConstantCondition {
   boolean b;
 
   ConstantMatchingCondition() {
-    forex(ControlFlowNode cfn |
+    forex(ControlFlow::Node cfn |
       cfn = this.getAControlFlowNode() |
-      exists(ControlFlowEdgeMatching t |
+      exists(ControlFlow::SuccessorTypes::MatchingSuccessor t |
         exists(cfn.getASuccessorByType(t)) |
         if t.isMatch() then b = true else b = false
       ) and
-      strictcount(ControlFlowEdgeType t | exists(cfn.getASuccessorByType(t))) = 1
+      strictcount(ControlFlow::SuccessorType t | exists(cfn.getASuccessorByType(t))) = 1
     )
   }
 

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

@@ -35,5 +35,5 @@ class StringComparison extends Expr {
 from StringComparison sc, PropertyAccess pa
 where sc.getAnOperand() instanceof StringLiteral
  and sc.getAnOperand() = pa
- and pa.getTarget() = getSystemTypeClass().getFullNameProperty()
+ and pa.getTarget() = any(SystemTypeClass c).getFullNameProperty()
 select sc, "Erroneous class compare."

csharp/ql/src/Concurrency/Concurrency.qll

@@ -102,17 +102,17 @@ class LockStmtBlock extends LockedBlock
     exists( LockStmt s | this=s.getBlock() )
   }
 
-  predicate isLockThis()
+  override predicate isLockThis()
   {
     exists( LockStmt s | this=s.getBlock() and s.isLockThis() )
   }
 
-  Variable getLockVariable()
+  override Variable getLockVariable()
   {
     exists( LockStmt s | this=s.getBlock() and result=s.getLockVariable() )
   }
 
-  Type getLockTypeObject()
+  override Type getLockTypeObject()
   {
     exists( LockStmt s | this=s.getBlock() and result=s.getLockTypeObject() )
   }
@@ -138,17 +138,17 @@ class SynchronizedMethodBlock extends LockedBlock
   {
     exists( SynchronizedMethod m | this=m.getStatementBody() )
   }
-  predicate isLockThis()
+  override predicate isLockThis()
   {
     exists( SynchronizedMethod m | this=m.getStatementBody() and m.isLockThis() )
   }
 
-  Variable getLockVariable()
+  override Variable getLockVariable()
   {
     none()
   }
 
-  Type getLockTypeObject()
+  override Type getLockTypeObject()
   {
     exists( SynchronizedMethod m | this=m.getStatementBody() and result=m.getLockTypeObject() )
   }

csharp/ql/src/Concurrency/UnsynchronizedStaticAccess.ql

@@ -13,7 +13,6 @@
 import csharp
 import DataMembers
 import ThreadCreation
-import ControlFlowGraph
 
 predicate correctlySynchronized(CollectionMember c, Expr access) {
   access = c.getAReadOrWrite() and
@@ -24,9 +23,9 @@ predicate correctlySynchronized(CollectionMember c, Expr access) {
   )
 }
 
-ControlFlowNode unlockedReachable(Callable a) {
+ControlFlow::Node unlockedReachable(Callable a) {
   result = a.getEntryPoint() or
-  exists(ControlFlowNode mid | mid = unlockedReachable(a) |
+  exists(ControlFlow::Node mid | mid = unlockedReachable(a) |
     not mid.getElement() instanceof LockingCall and
     result = mid.getASuccessor()
   )

csharp/ql/src/Likely Bugs/NestedLoopsSameVariable.ql

@@ -13,7 +13,6 @@
 
 import csharp
 import semmle.code.csharp.commons.ComparisonTest
-import semmle.code.csharp.controlflow.ControlFlowGraph::ControlFlowGraph
 import semmle.code.csharp.commons.StructuralComparison as SC
 
 /** A structural comparison configuration for comparing the conditions of nested `for` loops. */
@@ -87,13 +86,13 @@ class NestedForLoopSameVariable extends ForStmt {
   }
 
   /** Finds elements inside the outer loop that are no longer guarded by the loop invariant. */
-  private ControlFlowNode getAnUnguardedNode()
+  private ControlFlow::Node getAnUnguardedNode()
   {
     result.getElement().getParent+() = getOuterForStmt().getBody() and
     (
       result = this.getCondition().(ControlFlowElement).getAControlFlowExitNode().getAFalseSuccessor()
       or
-      exists(ControlFlowNode mid | mid = getAnUnguardedNode() |
+      exists(ControlFlow::Node mid | mid = getAnUnguardedNode() |
         mid.getASuccessor() = result and
         not exists(getAComparisonTest(result.getElement()))
       )

csharp/ql/src/Likely Bugs/Statements/UseBraces.ql

@@ -14,7 +14,7 @@
 import csharp
 
 // Iterate the control flow until we reach a Stmt
-Stmt findSuccessorStmt(ControlFlowGraph::ControlFlowNode n)
+Stmt findSuccessorStmt(ControlFlow::Node n)
 {
   result=n.getElement() or
   not n.getElement() instanceof Stmt and result=findSuccessorStmt(n.getASuccessor())

csharp/ql/src/Likely Bugs/UncheckedCastInEquals.ql

@@ -12,11 +12,11 @@
 import csharp
 import semmle.code.csharp.frameworks.System
 
-predicate nodeBeforeParameterAccess(ControlFlowGraph::ControlFlowNode node)
+predicate nodeBeforeParameterAccess(ControlFlow::Node node)
 {
   exists(EqualsMethod equals | equals.getBody() = node.getElement())
   or
-  exists(EqualsMethod equals, Parameter param, ControlFlowGraph::ControlFlowNode mid |
+  exists(EqualsMethod equals, Parameter param, ControlFlow::Node mid |
     equals.getParameter(0) = param and
     equals.getAChild*() = mid.getElement() and
     nodeBeforeParameterAccess(mid) and

csharp/ql/src/Performance/StringBuilderInLoop.ql

@@ -11,7 +11,7 @@
 import csharp
 import semmle.code.csharp.frameworks.system.Text
 
-from ObjectCreation creation, LoopStmt loop, ControlFlowGraph::ControlFlowNode loopEntryNode
+from ObjectCreation creation, LoopStmt loop, ControlFlow::Node loopEntryNode
 where creation.getType() instanceof SystemTextStringBuilderClass
   and loopEntryNode = loop.getBody().getAControlFlowEntryNode()
   and loop.getBody().getAChild*() = creation