Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2159,3 +2159,113 @@ module DuckTyping {
f.getADecorator().(Name).getId() = "property"
}
}

/**
* Provides a class hierarchy for exception types, covering both builtin
* exceptions (from typeshed models) and user-defined exception classes.
*/
module ExceptionTypes {
private import semmle.python.ApiGraphs
private import semmle.python.frameworks.data.internal.ApiGraphModels

/** Holds if `name` is a builtin exception class name. */
predicate builtinException(string name) {
typeModel("builtins.BaseException~Subclass", "builtins." + name, "")
}

/** Holds if builtin exception `sub` is a direct subclass of builtin exception `base`. */
private predicate builtinExceptionSubclass(string base, string sub) {
typeModel("builtins." + base + "~Subclass", "builtins." + sub, "")
}

/** An exception type, either a builtin exception or a user-defined exception class. */
newtype TExceptType =
/** A user-defined exception class. */
TUserExceptType(Class c) or
/** A builtin exception class, identified by name. */
TBuiltinExceptType(string name) { builtinException(name) }

/** An exception type, either a builtin exception or a user-defined exception class. */
class ExceptType extends TExceptType {
/** Gets the name of this exception type. */
string getName() { none() }

/** Gets a data-flow node that refers to this exception type. */
DataFlow::Node getAUse() { none() }

/** Gets a direct superclass of this exception type. */
ExceptType getADirectSuperclass() { none() }

/** Gets a string representation of this exception type. */
string toString() { result = this.getName() }

/**
* Holds if this element is at the specified location.
* The location spans column `startColumn` of line `startLine` to
* column `endColumn` of line `endLine` in file `filepath`.
* For more information, see
* [Providing locations in CodeQL queries](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
*/
predicate hasLocationInfo(
string filePath, int startLine, int startColumn, int endLine, int endColumn
) {
none()
}
}

/** A user-defined exception class. */
class UserExceptType extends ExceptType, TUserExceptType {
Class cls;

UserExceptType() { this = TUserExceptType(cls) }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is effectively what was there before, but would it be possible to not have all classes here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As in: would you prefer that these were moved to a separate file? If not, then I'm not sure what you're asking.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I think I understand what you're saying now: could we restrict cls to be a class that inherits from a built-in exception (since only these are actually valid in a raise)?

The answer is "I guess?", but I'm not sure it would make much of a difference. It would also make isLegalExceptionType pointless (and make it more complicated to check when people try to raise something that isn't a valid exception).

Do you have a specific reason to want to have fewer classes in this type?


/** Gets the underlying class. */
Class asClass() { result = cls }

override string getName() { result = cls.getName() }

override DataFlow::Node getAUse() { result = classTracker(cls) }

override ExceptType getADirectSuperclass() {
result.(UserExceptType).asClass() = getADirectSuperclass(cls)
or
result.(BuiltinExceptType).getAUse().asExpr() = cls.getABase()
}

override predicate hasLocationInfo(
string filePath, int startLine, int startColumn, int endLine, int endColumn
) {
cls.getLocation().hasLocationInfo(filePath, startLine, startColumn, endLine, endColumn)
}
}

/** A builtin exception class, identified by name. */
class BuiltinExceptType extends ExceptType, TBuiltinExceptType {
string name;

BuiltinExceptType() { this = TBuiltinExceptType(name) }

/** Gets the builtin name. */
string asBuiltinName() { result = name }

override string getName() { result = name }

override DataFlow::Node getAUse() { API::builtin(name).asSource().flowsTo(result) }

override ExceptType getADirectSuperclass() {
builtinExceptionSubclass(result.(BuiltinExceptType).asBuiltinName(), name) and
result != this
}

override predicate hasLocationInfo(
string filePath, int startLine, int startColumn, int endLine, int endColumn
) {
filePath = "" and
startLine = 0 and
startColumn = 0 and
endLine = 0 and
endColumn = 0
}
}
}

69 changes: 1 addition & 68 deletions python/ql/src/Exceptions/IncorrectExceptOrder.ql
Original file line number Diff line number Diff line change
Expand Up @@ -15,74 +15,7 @@

import python
import semmle.python.dataflow.new.internal.DataFlowDispatch
import semmle.python.ApiGraphs
import semmle.python.frameworks.data.internal.ApiGraphModels

predicate builtinException(string name) {
typeModel("builtins.BaseException~Subclass", "builtins." + name, "")
}

predicate builtinExceptionSubclass(string base, string sub) {
typeModel("builtins." + base + "~Subclass", "builtins." + sub, "")
}

newtype TExceptType =
TClass(Class c) or
TBuiltin(string name) { builtinException(name) }

class ExceptType extends TExceptType {
Class asClass() { this = TClass(result) }

string asBuiltinName() { this = TBuiltin(result) }

predicate isBuiltin() { this = TBuiltin(_) }

string getName() {
result = this.asClass().getName()
or
result = this.asBuiltinName()
}

string toString() { result = this.getName() }

DataFlow::Node getAUse() {
result = classTracker(this.asClass())
or
API::builtin(this.asBuiltinName()).asSource().flowsTo(result)
}

ExceptType getADirectSuperclass() {
result.asClass() = getADirectSuperclass(this.asClass())
or
result.isBuiltin() and
result.getAUse().asExpr() = this.asClass().getABase()
or
builtinExceptionSubclass(result.asBuiltinName(), this.asBuiltinName()) and
this != result
}

/**
* Holds if this element is at the specified location.
* The location spans column `startColumn` of line `startLine` to
* column `endColumn` of line `endLine` in file `filepath`.
* For more information, see
* [Providing locations in CodeQL queries](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
*/
predicate hasLocationInfo(
string filePath, int startLine, int startColumn, int endLine, int endColumn
) {
this.asClass()
.getLocation()
.hasLocationInfo(filePath, startLine, startColumn, endLine, endColumn)
or
this.isBuiltin() and
filePath = "" and
startLine = 0 and
startColumn = 0 and
endLine = 0 and
endColumn = 0
}
}
private import ExceptionTypes

predicate incorrectExceptOrder(ExceptStmt ex1, ExceptType cls1, ExceptStmt ex2, ExceptType cls2) {
exists(int i, int j, Try t |
Expand Down