diff --git a/python/ql/lib/change-notes/2026-10-23-inferred-expression-contexts.md b/python/ql/lib/change-notes/2026-10-23-inferred-expression-contexts.md new file mode 100644 index 000000000000..b379f64efe70 --- /dev/null +++ b/python/ql/lib/change-notes/2026-10-23-inferred-expression-contexts.md @@ -0,0 +1,5 @@ +--- +category: feature +--- +* Added `Expr.isLoadContext()`, `Expr.isStoreContext()`, `Expr.isParameterContext()`, and + `Expr.isDeleteContext()` to classify expression contexts from the AST structure. \ No newline at end of file diff --git a/python/ql/lib/semmle/python/Exprs.qll b/python/ql/lib/semmle/python/Exprs.qll index 937c9e5ffd79..7ee1ced24cae 100644 --- a/python/ql/lib/semmle/python/Exprs.qll +++ b/python/ql/lib/semmle/python/Exprs.qll @@ -4,6 +4,78 @@ module; private import python private import semmle.python.internal.CachedStages +private newtype TContext = + TDel() or + TLoad() or + TParam() or + TStore() + +private predicate hasExpressionContext(Expr e) { + e instanceof Attribute or + e instanceof List or + e instanceof Name or + e instanceof PlaceHolder or + e instanceof Starred or + e instanceof Subscript or + e instanceof TemplateDottedNotation_ or + e instanceof Tuple +} + +private predicate propagatesExpressionContext(Expr parent, Expr child) { + child in [parent.(List).getAnElt(), parent.(Tuple).getAnElt(), parent.(Starred).getValue()] +} + +private predicate hasNonLoadExpressionContextSeed(Expr e, TContext context) { + context = TStore() and + e in [ + any(Assign a).getATarget(), + any(AnnAssign a).getTarget(), + any(AssignExpr a).getTarget(), + any(For f).getTarget(), + any(Comprehension c).getTarget(), + any(With w).getOptionalVars(), + any(ExceptStmt ex).getName(), + any(ExceptGroupStmt ex).getName(), + any(Alias a).getAsname(), + any(MatchAsPattern p).getAlias(), + any(MatchCapturePattern p).getVariable(), + any(TypeAlias t).getName(), + any(TypeVar t).getName(), + any(TypeVarTuple t).getName(), + any(ParamSpec p).getName() + ] + or + context = TParam() and + ( + e = any(Function f).getAnArg() + or + e in [ + any(Function f).getVararg(), + any(Function f).getAKwonlyarg(), + any(Function f).getKwarg() + ] + ) + or + context = TDel() and + e = any(Delete d).getATarget() +} + +private predicate inferredNonLoadExpressionContext(Expr e, TContext context) { + hasExpressionContext(e) and + exists(Expr root | + hasNonLoadExpressionContextSeed(root, context) and + propagatesExpressionContext*(root, e) + ) +} + +private predicate inferredExpressionContext(Expr e, TContext context) { + inferredNonLoadExpressionContext(e, context) + or + context = TLoad() and + hasExpressionContext(e) and + not inferredNonLoadExpressionContext(e, _) +} + /** An expression */ class Expr extends Expr_, AstNode { /** Gets the scope of this expression */ @@ -54,6 +126,18 @@ class Expr extends Expr_, AstNode { /** Gets an immediate (non-nested) sub-expression of this expression */ Expr getASubExpression() { none() } + /** Whether this expression occurs in a load context. */ + predicate isLoadContext() { inferredExpressionContext(this, TLoad()) } + + /** Whether this expression occurs in a store context. */ + predicate isStoreContext() { inferredExpressionContext(this, TStore()) } + + /** Whether this expression occurs in a parameter context. */ + predicate isParameterContext() { inferredExpressionContext(this, TParam()) } + + /** Whether this expression occurs in a deletion context. */ + predicate isDeleteContext() { inferredExpressionContext(this, TDel()) } + override AstNode getAChildNode() { result = this.getASubExpression() } } @@ -432,10 +516,10 @@ class Name extends Name_ { /** Whether this expression is a definition */ predicate isDefinition() { - py_expr_contexts(_, 5, this) + this.isStoreContext() or /* Treat Param as a definition (which it is) */ - py_expr_contexts(_, 4, this) + this.isParameterContext() or /* The target in an augmented assignment is also a definition (and a use) */ exists(AugAssign aa | aa.getTarget() = this) @@ -451,7 +535,7 @@ class Name extends Name_ { } /** Whether this expression is a deletion */ - predicate isDeletion() { py_expr_contexts(_, 2, this) } + predicate isDeletion() { this.isDeleteContext() } /** * Whether this expression deletes variable `v`. @@ -463,7 +547,7 @@ class Name extends Name_ { } /** Whether this expression is a use */ - predicate isUse() { py_expr_contexts(_, 3, this) } + predicate isUse() { this.isLoadContext() } /** * Whether this expression is a use of variable `v` diff --git a/python/ql/lib/semmle/python/Flow.qll b/python/ql/lib/semmle/python/Flow.qll index 76a1f21af157..71f20722db1a 100644 --- a/python/ql/lib/semmle/python/Flow.qll +++ b/python/ql/lib/semmle/python/Flow.qll @@ -35,19 +35,19 @@ private Py::AstNode toAst(ControlFlowNode n) { py_flow_bb_node(n, result, _, _) class ControlFlowNode extends @py_flow_node { /** Whether this control flow node is a load (including those in augmented assignments) */ predicate isLoad() { - exists(Py::Expr e | e = toAst(this) | py_expr_contexts(_, 3, e) and not augstore(_, this)) + exists(Py::Expr e | e = toAst(this) | e.isLoadContext() and not augstore(_, this)) } /** Whether this control flow node is a store (including those in augmented assignments) */ predicate isStore() { - exists(Py::Expr e | e = toAst(this) | py_expr_contexts(_, 5, e) or augstore(_, this)) + exists(Py::Expr e | e = toAst(this) | e.isStoreContext() or augstore(_, this)) } /** Whether this control flow node is a delete */ - predicate isDelete() { exists(Py::Expr e | e = toAst(this) | py_expr_contexts(_, 2, e)) } + predicate isDelete() { exists(Py::Expr e | e = toAst(this) | e.isDeleteContext()) } /** Whether this control flow node is a parameter */ - predicate isParameter() { exists(Py::Expr e | e = toAst(this) | py_expr_contexts(_, 4, e)) } + predicate isParameter() { exists(Py::Expr e | e = toAst(this) | e.isParameterContext()) } /** Whether this control flow node is a store in an augmented assignment */ predicate isAugStore() { augstore(_, this) } @@ -848,9 +848,7 @@ class NameNode extends ControlFlowNode { this.isLoad() and exists(Py::Name u | this.getNode() = u and u.uses(v)) or - exists(Py::PlaceHolder u | - this.getNode() = u and u.getVariable() = v and u.getCtx() instanceof Py::Load - ) + exists(Py::PlaceHolder u | this.getNode() = u and u.getVariable() = v and u.isLoadContext()) or Scopes::use_of_global_variable(this, v.getScope(), v.getId()) } diff --git a/python/ql/lib/semmle/python/SelfAttribute.qll b/python/ql/lib/semmle/python/SelfAttribute.qll index 364e080dcdd7..36efcb8533db 100644 --- a/python/ql/lib/semmle/python/SelfAttribute.qll +++ b/python/ql/lib/semmle/python/SelfAttribute.qll @@ -35,7 +35,7 @@ private predicate self_attribute(Attribute attr, Class cls) { /** A helper class for UndefinedClassAttribute.ql & MaybeUndefinedClassAttribute.ql */ class SelfAttributeRead extends SelfAttribute { SelfAttributeRead() { - this.getCtx() instanceof Load and + this.isLoadContext() and // Be stricter for loads. // We want to generous as to what is defined (i.e. stores), // but strict as to what needs to be defined (i.e. loads). @@ -68,7 +68,7 @@ class SelfAttributeRead extends SelfAttribute { } class SelfAttributeStore extends SelfAttribute { - SelfAttributeStore() { this.getCtx() instanceof Store } + SelfAttributeStore() { this.isStoreContext() } Expr getAssignedValue() { exists(Assign a | a.getATarget() = this | result = a.getValue()) } } diff --git a/python/ql/lib/semmle/python/controlflow/internal/Cfg.qll b/python/ql/lib/semmle/python/controlflow/internal/Cfg.qll index 3441ad0203c1..1aa76db912b3 100644 --- a/python/ql/lib/semmle/python/controlflow/internal/Cfg.qll +++ b/python/ql/lib/semmle/python/controlflow/internal/Cfg.qll @@ -122,16 +122,16 @@ class ControlFlowNode extends CfgImpl::ControlFlowNode { * which holds on the destination of compound and unary assignments * even though the destination is also a write. */ - predicate isLoad() { py_expr_contexts(_, 3, this.asPyExpr()) } + predicate isLoad() { this.asPyExpr().isLoadContext() } /** Holds if this flow node is a store (including those in augmented assignments). */ - predicate isStore() { py_expr_contexts(_, 5, this.asPyExpr()) or augstore(_, this) } + predicate isStore() { this.asPyExpr().isStoreContext() or augstore(_, this) } /** Holds if this flow node is a delete. */ - predicate isDelete() { py_expr_contexts(_, 2, this.asPyExpr()) } + predicate isDelete() { this.asPyExpr().isDeleteContext() } /** Holds if this flow node is a parameter. */ - predicate isParameter() { py_expr_contexts(_, 4, this.asPyExpr()) } + predicate isParameter() { this.asPyExpr().isParameterContext() } /** Holds if this flow node is a store in an augmented assignment. */ predicate isAugStore() { augstore(_, this) } @@ -379,9 +379,7 @@ class NameNode extends ControlFlowNode { this.isLoad() and exists(Py::Name u | u = this.getNode() and u.uses(v)) or - exists(Py::PlaceHolder u | - u = this.getNode() and u.getVariable() = v and u.getCtx() instanceof Py::Load - ) + exists(Py::PlaceHolder u | u = this.getNode() and u.getVariable() = v and u.isLoadContext()) } /** Gets the identifier of this name node. */ diff --git a/python/ql/src/Classes/ClassAttributes.qll b/python/ql/src/Classes/ClassAttributes.qll index 58f8ae5dffc9..f6079400bf6b 100644 --- a/python/ql/src/Classes/ClassAttributes.qll +++ b/python/ql/src/Classes/ClassAttributes.qll @@ -66,7 +66,7 @@ class CheckClass extends ClassObject { private predicate monkeyPatched(string name) { exists(Attribute a, ControlFlowNode objCfg | objCfg.getNode() = a.getObject() and - a.getCtx() instanceof Store and + a.isStoreContext() and PointsTo::points_to(objCfg, _, this, _, _) and a.getName() = name ) diff --git a/python/ql/src/Classes/OverwritingAttributeInSuperClass.ql b/python/ql/src/Classes/OverwritingAttributeInSuperClass.ql index 89d33d548146..8dd25823fba4 100644 --- a/python/ql/src/Classes/OverwritingAttributeInSuperClass.ql +++ b/python/ql/src/Classes/OverwritingAttributeInSuperClass.ql @@ -43,7 +43,7 @@ predicate self_write_stmt(Stmt s, string attr) { self = a.getObject() and s.contains(a) and self.getId() = "self" and - a.getCtx() instanceof Store and + a.isStoreContext() and a.getName() = attr ) } diff --git a/python/ql/src/Classes/UselessClass.ql b/python/ql/src/Classes/UselessClass.ql index 229e42fd292f..b38eac8788c4 100644 --- a/python/ql/src/Classes/UselessClass.ql +++ b/python/ql/src/Classes/UselessClass.ql @@ -34,14 +34,20 @@ predicate no_inheritance(Class c) { predicate is_decorated(Class c) { exists(c.getADecorator()) } +private predicate is_stateful_store(Expr e) { + e.isStoreContext() + or + e = any(AugAssign a).getTarget() +} + predicate is_stateful(Class c) { - exists(Function method, ExprContext ctx | + exists(Function method | method.getScope() = c and - (ctx instanceof Store or ctx instanceof AugStore) - | - exists(Subscript s | s.getScope() = method and s.getCtx() = ctx) - or - exists(Attribute a | a.getScope() = method and a.getCtx() = ctx) + ( + exists(Subscript s | s.getScope() = method and is_stateful_store(s)) + or + exists(Attribute a | a.getScope() = method and is_stateful_store(a)) + ) ) or exists(Function method, Call call, Attribute a, string name | diff --git a/python/ql/src/analysis/RatioOfDefinitions.ql b/python/ql/src/analysis/RatioOfDefinitions.ql index 84b8d7602ec7..65ea48f1d929 100644 --- a/python/ql/src/analysis/RatioOfDefinitions.ql +++ b/python/ql/src/analysis/RatioOfDefinitions.ql @@ -10,9 +10,9 @@ predicate want_to_have_definition(ExprWithPointsTo e) { /* not builtin object like len, tuple, etc. */ not exists(Value builtin | e.pointsTo(builtin) and builtin.isBuiltin()) and ( - e instanceof Name and e.(Name).getCtx() instanceof Load + e instanceof Name and e.isLoadContext() or - e instanceof Attribute and e.(Attribute).getCtx() instanceof Load + e instanceof Attribute and e.isLoadContext() or e instanceof ImportMember or diff --git a/python/ql/test/library-tests/expr-context-parity/ExprContextParity.expected b/python/ql/test/library-tests/expr-context-parity/ExprContextParity.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/python/ql/test/library-tests/expr-context-parity/ExprContextParity.ql b/python/ql/test/library-tests/expr-context-parity/ExprContextParity.ql new file mode 100644 index 000000000000..1f2fce25a15f --- /dev/null +++ b/python/ql/test/library-tests/expr-context-parity/ExprContextParity.ql @@ -0,0 +1,22 @@ +import python + +private predicate inferredContext(Expr e, int kind) { + kind = 2 and e.isDeleteContext() + or + kind = 3 and e.isLoadContext() + or + kind = 4 and e.isParameterContext() + or + kind = 5 and e.isStoreContext() +} + +from Expr e, int kind, string problem +where + py_expr_contexts(_, kind, e) and + not inferredContext(e, kind) and + problem = "Extracted-only context" + or + inferredContext(e, kind) and + not py_expr_contexts(_, kind, e) and + problem = "Inferred-only context" +select e, problem, kind diff --git a/python/ql/test/library-tests/expr-context-parity/legacy.py b/python/ql/test/library-tests/expr-context-parity/legacy.py new file mode 100644 index 000000000000..c12abb2aeeda --- /dev/null +++ b/python/ql/test/library-tests/expr-context-parity/legacy.py @@ -0,0 +1,35 @@ +import package +import package.module as imported_module +from package import member +from package import other as imported_member + + +plain = value +first = second = plain +(tuple_target, [list_target, *rest]) = values +(walrus_target := plain) + +for (loop_item, *loop_rest) in iterable: + pass + +items = [item for item in iterable] + +with manager as (resource, *resources): + pass + +try: + pass +except Exception as error: + pass + +del plain +del object_value.attribute +del sequence[index] +del tuple_target, list_target + + +def function(positional, default=value, *args, keyword_only, **kwargs): + positional += 1 + object_value.attribute = positional + sequence[index] = positional + return positional, default, args, keyword_only, kwargs diff --git a/python/ql/test/library-tests/expr-context-parity/new_parser.py b/python/ql/test/library-tests/expr-context-parity/new_parser.py new file mode 100644 index 000000000000..476012fd4cab --- /dev/null +++ b/python/ql/test/library-tests/expr-context-parity/new_parser.py @@ -0,0 +1,56 @@ +import package +import package.module as imported_module +from package import member +from package import other as imported_member + + +type Alias[T, *Ts, **P] = tuple[T, *Ts] + + +annotated: object = value +plain = annotated +first = second = plain +(tuple_target, [list_target, *rest]) = values +(walrus_target := plain) + +for (loop_item, *loop_rest) in iterable: + pass + +items = [item for item in iterable] + +with manager as (resource, *resources): + pass + +try: + pass +except Exception as error: + pass + +match subject: + case [head, *tail] as whole: + pass + case {"key": captured, **rest_mapping}: + pass + +try: + pass +except* RuntimeError as group_error: + pass + +del plain +del object_value.attribute +del sequence[index] +del tuple_target, list_target + + +def function(positional, default=value, *args, keyword_only, **kwargs): + positional += 1 + object_value.attribute = positional + sequence[index] = positional + return positional, default, args, keyword_only, kwargs + + +lambda_function = ( + lambda positional, default=value, *args, keyword_only, **kwargs: + (positional, default, args, keyword_only, kwargs) +) diff --git a/python/ql/test/query-tests/Classes/useless/test.py b/python/ql/test/query-tests/Classes/useless/test.py index 063da81c1727..6e2c224a4e44 100644 --- a/python/ql/test/query-tests/Classes/useless/test.py +++ b/python/ql/test/query-tests/Classes/useless/test.py @@ -55,6 +55,16 @@ def __init__(self): def do_something(self, x): self.data = x + +class StatefulAugmentedAttribute(object): + + def update(self): + self.value += 1 + +class StatefulAugmentedSubscript(object): + + def update(self): + self.values[0] += 1 class Inherited(object): pass