python: rework hierarchy to avoid breaking API

Author: yoff

python/ql/lib/change-notes/2022-11-14-grouped-exceptions-breaking.md

@@ -1,4 +1,4 @@
 ---
 category: breaking
 ---
-* `Try.getAHandler()` and `Try.getHandler(<index>)` have results of type `Stmt` instead of `ExceptStmt`, as handlers may also be `ExceptGroupStmt`s (After Python 3.11 introduced PEP 654). This means that code of the form `try.getAHandler().getType()` will no longer work. Instead, use `try.getANormalHandler().getType()` or `try.getAGroupHandler().getType()`, depending on your use case.
+* The result of `Try::getAHandler` and `Try.getHandler(<index>)` is no longer of type `ExceptStmt`, as handlers may also be `ExceptGroupStmt`s (After Python 3.11 introduced PEP 654). Instead, it is of the new type `ExceptionHandler` of which `ExceptStmt` and `ExceptGroupStmt` are subtypes. To support selecting only one type of handler, `try.getANormalHandler` and `try.getAGroupHandler` have been added. Existing uses of `Try::getAHandler` for which it is important to select only normal handlers, will need to be updated to `try.getANormalHandler`.

python/ql/lib/semmle/python/AstGenerated.qll

@@ -1379,7 +1379,7 @@ class Try_ extends @py_Try, Stmt {
   Stmt getHandler(int index) { result = this.getHandlers().getItem(index) }
 
   /** Gets an exception handler of this try statement. */
-  Stmt getAHandler() { result = this.getHandlers().getAnItem() }
+  ExceptionHandler getAHandler() { result = this.getHandlers().getAnItem() }
 
   /** Gets the finally block of this try statement. */
   StmtList getFinalbody() { py_stmt_lists(result, this, 4) }

python/ql/lib/semmle/python/Stmts.qll

@@ -143,11 +143,46 @@ class Exec extends Exec_ {
   override Stmt getASubStatement() { none() }
 }
 
-/** An except group statement (part of a `try` statement), such as `except* IOError as err:` */
-class ExceptGroupStmt extends ExceptGroupStmt_ {
+/**
+ * An exception handler such as an `except` or an `except*` statement
+ * in a `try` statement.
+ *
+ * We do not wish to expose an abstract class that users are not
+ * meant to extend, so this class is kept private. The user facing
+ * class is `ExceptionHandler`.
+ */
+abstract private class ExceptionHandlerImpl extends Stmt {
+  /** Gets the name of this except group block. */
+  abstract Expr getName();
+
+  /** Gets the type of this except group block. */
+  abstract Expr getType();
+}
+
+/**
+ * An exception handler such as an `except` or an `except*` statement
+ * in a `try` statement.
+ */
+class ExceptionHandler extends Stmt instanceof ExceptionHandlerImpl {
+  ExceptionHandler() {
+    this instanceof ExceptStmt_
+    or
+    this instanceof ExceptGroupStmt_
+  }
+
   /** Gets the immediately enclosing try statement */
   Try getTry() { result.getAHandler() = this }
 
+  /** Gets the name of this except group block. */
+  Expr getName() { result = super.getName() }
+
+  /** Gets the type of this except group block. */
+  Expr getType() { result = super.getType() }
+}
+
+/** An except group statement (part of a `try` statement), such as `except* IOError as err:` */
+class ExceptGroupStmt extends ExceptGroupStmt_, ExceptionHandlerImpl {
+  /* syntax: except Expr [ as Expr ]: */
   override Expr getASubExpression() {
     result = this.getName()
     or
@@ -158,19 +193,18 @@ class ExceptGroupStmt extends ExceptGroupStmt_ {
 
   override Stmt getLastStatement() { result = this.getBody().getLastItem().getLastStatement() }
 
+  override Expr getName() { result = ExceptGroupStmt_.super.getName() }
+
   override Expr getType() {
-    result = super.getType() and not result instanceof Tuple
+    result = ExceptGroupStmt_.super.getType() and not result instanceof Tuple
     or
-    result = super.getType().(Tuple).getAnElt()
+    result = ExceptGroupStmt_.super.getType().(Tuple).getAnElt()
   }
 }
 
 /** An except statement (part of a `try` statement), such as `except IOError as err:` */
-class ExceptStmt extends ExceptStmt_ {
+class ExceptStmt extends ExceptStmt_, ExceptionHandlerImpl {
   /* syntax: except Expr [ as Expr ]: */
-  /** Gets the immediately enclosing try statement */
-  Try getTry() { result.getAHandler() = this }
-
   override Expr getASubExpression() {
     result = this.getName()
     or
@@ -181,10 +215,12 @@ class ExceptStmt extends ExceptStmt_ {
 
   override Stmt getLastStatement() { result = this.getBody().getLastItem().getLastStatement() }
 
+  override Expr getName() { result = ExceptStmt_.super.getName() }
+
   override Expr getType() {
-    result = super.getType() and not result instanceof Tuple
+    result = ExceptStmt_.super.getType() and not result instanceof Tuple
     or
-    result = super.getType().(Tuple).getAnElt()
+    result = ExceptStmt_.super.getType().(Tuple).getAnElt()
   }
 }
 

python/ql/src/Exceptions/UnguardedNextInGenerator.ql

@@ -52,7 +52,7 @@ predicate iter_not_exhausted(EssaVariable iterator) {
 predicate stop_iteration_handled(CallNode call) {
   exists(Try t |
     t.containsInScope(call.getNode()) and
-    t.getANormalHandler().getType() = stopIteration().getAValueReachableFromSource().asExpr()
+    t.getAHandler().getType() = stopIteration().getAValueReachableFromSource().asExpr()
   )
 }
 

python/ql/src/Expressions/HashedButNoHash.ql

@@ -70,7 +70,7 @@ predicate is_unhashable(ControlFlowNode f, ClassValue cls, ControlFlowNode origi
 predicate typeerror_is_caught(ControlFlowNode f) {
   exists(Try try |
     try.getBody().contains(f.getNode()) and
-    try.getANormalHandler().getType().pointsTo(ClassValue::typeError())
+    try.getAHandler().getType().pointsTo(ClassValue::typeError())
   )
 }
 

python/ql/src/Imports/DeprecatedModule.ql

@@ -78,7 +78,7 @@ from ImportExpr imp, string name, string instead
 where
   name = imp.getName() and
   deprecated_module(name, instead, _, _) and
-  not exists(Try try, ExceptStmt except | except = try.getANormalHandler() |
+  not exists(Try try, ExceptStmt except | except = try.getAHandler() |
     except.getType().pointsTo(ClassValue::importError()) and
     except.containsInScope(imp)
   )

python/ql/src/Variables/UndefinedGlobal.ql

@@ -17,7 +17,7 @@ import semmle.python.pointsto.PointsTo
 
 predicate guarded_against_name_error(Name u) {
   exists(Try t | t.getBody().getAnItem().contains(u) |
-    t.getANormalHandler().getType().(Name).getId() = "NameError"
+    t.getAHandler().getType().(Name).getId() = "NameError"
   )
   or
   exists(ConditionBlock guard, BasicBlock controlled, Call globals |

python/ql/src/Variables/UninitializedLocal.ql

@@ -27,7 +27,7 @@ predicate uninitialized_local(NameNode use) {
 predicate explicitly_guarded(NameNode u) {
   exists(Try t |
     t.getBody().contains(u.getNode()) and
-    t.getANormalHandler().getType().pointsTo(ClassValue::nameError())
+    t.getAHandler().getType().pointsTo(ClassValue::nameError())
   )
 }