diff --git a/python/ql/lib/change-notes/2022-11-14-grouped-exceptions-breaking.md b/python/ql/lib/change-notes/2022-11-14-grouped-exceptions-breaking.md index 06bca7fe433..3e42a65a046 100644 --- a/python/ql/lib/change-notes/2022-11-14-grouped-exceptions-breaking.md +++ b/python/ql/lib/change-notes/2022-11-14-grouped-exceptions-breaking.md @@ -1,4 +1,4 @@ --- category: breaking --- -* `Try.getAHandler()` and `Try.getHandler()` 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()` 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`. diff --git a/python/ql/lib/semmle/python/AstGenerated.qll b/python/ql/lib/semmle/python/AstGenerated.qll index b6fab779b15..d7e1818bd80 100644 --- a/python/ql/lib/semmle/python/AstGenerated.qll +++ b/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) } diff --git a/python/ql/lib/semmle/python/Stmts.qll b/python/ql/lib/semmle/python/Stmts.qll index 07fa7f02a76..829aba2ddb9 100644 --- a/python/ql/lib/semmle/python/Stmts.qll +++ b/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() } } diff --git a/python/ql/src/Exceptions/UnguardedNextInGenerator.ql b/python/ql/src/Exceptions/UnguardedNextInGenerator.ql index 3af67abdcfd..a6969218fdd 100644 --- a/python/ql/src/Exceptions/UnguardedNextInGenerator.ql +++ b/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() ) } diff --git a/python/ql/src/Expressions/HashedButNoHash.ql b/python/ql/src/Expressions/HashedButNoHash.ql index ec6c85183b2..336c344fa37 100644 --- a/python/ql/src/Expressions/HashedButNoHash.ql +++ b/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()) ) } diff --git a/python/ql/src/Imports/DeprecatedModule.ql b/python/ql/src/Imports/DeprecatedModule.ql index 32ffde42613..5f5a8af3ae9 100644 --- a/python/ql/src/Imports/DeprecatedModule.ql +++ b/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) ) diff --git a/python/ql/src/Variables/UndefinedGlobal.ql b/python/ql/src/Variables/UndefinedGlobal.ql index 3197ed5f119..f88f5504df8 100644 --- a/python/ql/src/Variables/UndefinedGlobal.ql +++ b/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 | diff --git a/python/ql/src/Variables/UninitializedLocal.ql b/python/ql/src/Variables/UninitializedLocal.ql index cc1853ca465..23a063be5ab 100644 --- a/python/ql/src/Variables/UninitializedLocal.ql +++ b/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()) ) }