From a9c8163ab3c2e7b685a12cf632c6be1939fffc6e Mon Sep 17 00:00:00 2001 From: Taus Date: Wed, 13 Oct 2021 13:24:46 +0000 Subject: [PATCH 1/2] Python: Fix uses of implicit `this` Quoting the style guide: "14. _Always_ qualify _calls_ to predicates of the same class with `this`." --- python/ql/lib/semmle/python/ApiGraphs.qll | 30 ++++++++++--------- python/ql/lib/semmle/python/Exprs.qll | 2 +- python/ql/lib/semmle/python/Files.qll | 10 +++++-- python/ql/lib/semmle/python/Flow.qll | 6 ++-- python/ql/lib/semmle/python/Import.qll | 18 +++++------ python/ql/lib/semmle/python/Module.qll | 4 +-- python/ql/lib/semmle/python/RegexTreeView.qll | 26 ++++++++-------- .../dataflow/new/internal/DataFlowPrivate.qll | 4 +-- .../dataflow/new/internal/LocalSources.qll | 8 ++--- 9 files changed, 57 insertions(+), 51 deletions(-) diff --git a/python/ql/lib/semmle/python/ApiGraphs.qll b/python/ql/lib/semmle/python/ApiGraphs.qll index 62afe4ef8652..9ab510d0ee57 100644 --- a/python/ql/lib/semmle/python/ApiGraphs.qll +++ b/python/ql/lib/semmle/python/ApiGraphs.qll @@ -55,7 +55,7 @@ module API { /** * Gets a call to the function represented by this API component. */ - DataFlow::CallCfgNode getACall() { result = getReturn().getAnImmediateUse() } + DataFlow::CallCfgNode getACall() { result = this.getReturn().getAnImmediateUse() } /** * Gets a node representing member `m` of this API component. @@ -67,21 +67,21 @@ module API { */ bindingset[m] bindingset[result] - Node getMember(string m) { result = getASuccessor(Label::member(m)) } + Node getMember(string m) { result = this.getASuccessor(Label::member(m)) } /** * Gets a node representing a member of this API component where the name of the member is * not known statically. */ - Node getUnknownMember() { result = getASuccessor(Label::unknownMember()) } + Node getUnknownMember() { result = this.getASuccessor(Label::unknownMember()) } /** * Gets a node representing a member of this API component where the name of the member may * or may not be known statically. */ Node getAMember() { - result = getASuccessor(Label::member(_)) or - result = getUnknownMember() + result = this.getASuccessor(Label::member(_)) or + result = this.getUnknownMember() } /** @@ -90,23 +90,25 @@ module API { * This predicate may have multiple results when there are multiple invocations of this API component. * Consider using `getACall()` if there is a need to distinguish between individual calls. */ - Node getReturn() { result = getASuccessor(Label::return()) } + Node getReturn() { result = this.getASuccessor(Label::return()) } /** * Gets a node representing a subclass of the class represented by this node. */ - Node getASubclass() { result = getASuccessor(Label::subclass()) } + Node getASubclass() { result = this.getASuccessor(Label::subclass()) } /** * Gets a node representing the result from awaiting this node. */ - Node getAwaited() { result = getASuccessor(Label::await()) } + Node getAwaited() { result = this.getASuccessor(Label::await()) } /** * Gets a string representation of the lexicographically least among all shortest access paths * from the root to this node. */ - string getPath() { result = min(string p | p = getAPath(Impl::distanceFromRoot(this)) | p) } + string getPath() { + result = min(string p | p = this.getAPath(Impl::distanceFromRoot(this)) | p) + } /** * Gets a node such that there is an edge in the API graph between this node and the other @@ -124,13 +126,13 @@ module API { * Gets a node such that there is an edge in the API graph between this node and the other * one. */ - Node getAPredecessor() { result = getAPredecessor(_) } + Node getAPredecessor() { result = this.getAPredecessor(_) } /** * Gets a node such that there is an edge in the API graph between that other node and * this one. */ - Node getASuccessor() { result = getASuccessor(_) } + Node getASuccessor() { result = this.getASuccessor(_) } /** * Gets the data-flow node that gives rise to this node, if any. @@ -147,11 +149,11 @@ module API { predicate hasLocationInfo( string filepath, int startline, int startcolumn, int endline, int endcolumn ) { - getInducingNode().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) + this.getInducingNode().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) or // For nodes that do not have a meaningful location, `path` is the empty string and all other // parameters are zero. - not exists(getInducingNode()) and + not exists(this.getInducingNode()) and filepath = "" and startline = 0 and startcolumn = 0 and @@ -202,7 +204,7 @@ module API { or this = Impl::MkModuleImport(_) and type = "ModuleImport " | - result = type + getPath() + result = type + this.getPath() or not exists(this.getPath()) and result = type + "with no path" ) diff --git a/python/ql/lib/semmle/python/Exprs.qll b/python/ql/lib/semmle/python/Exprs.qll index 98c24b126a42..5a56a704b6f0 100644 --- a/python/ql/lib/semmle/python/Exprs.qll +++ b/python/ql/lib/semmle/python/Exprs.qll @@ -240,7 +240,7 @@ class Call extends Call_ { /** Gets the tuple (*) argument of this call, provided there is exactly one. */ Expr getStarArg() { count(this.getStarargs()) < 2 and - result = getStarargs() + result = this.getStarargs() } } diff --git a/python/ql/lib/semmle/python/Files.qll b/python/ql/lib/semmle/python/Files.qll index 66dfca681cc9..99570fc4d7a7 100644 --- a/python/ql/lib/semmle/python/Files.qll +++ b/python/ql/lib/semmle/python/Files.qll @@ -256,7 +256,7 @@ abstract class Container extends @container { * */ string getBaseName() { - result = getAbsolutePath().regexpCapture(".*/(([^/]*?)(?:\\.([^.]*))?)", 1) + result = this.getAbsolutePath().regexpCapture(".*/(([^/]*?)(?:\\.([^.]*))?)", 1) } /** @@ -282,7 +282,9 @@ abstract class Container extends @container { * "/tmp/x.tar.gz""gz" * */ - string getExtension() { result = getAbsolutePath().regexpCapture(".*/([^/]*?)(\\.([^.]*))?", 3) } + string getExtension() { + result = this.getAbsolutePath().regexpCapture(".*/([^/]*?)(\\.([^.]*))?", 3) + } /** * Gets the stem of this container, that is, the prefix of its base name up to @@ -301,7 +303,9 @@ abstract class Container extends @container { * "/tmp/x.tar.gz""x.tar" * */ - string getStem() { result = getAbsolutePath().regexpCapture(".*/([^/]*?)(?:\\.([^.]*))?", 1) } + string getStem() { + result = this.getAbsolutePath().regexpCapture(".*/([^/]*?)(?:\\.([^.]*))?", 1) + } File getFile(string baseName) { result = this.getAFile() and diff --git a/python/ql/lib/semmle/python/Flow.qll b/python/ql/lib/semmle/python/Flow.qll index 652469278726..ab3d0a5f393d 100755 --- a/python/ql/lib/semmle/python/Flow.qll +++ b/python/ql/lib/semmle/python/Flow.qll @@ -851,9 +851,9 @@ class ForNode extends ControlFlowNode { /** Holds if this `for` statement causes iteration over `sequence` storing each step of the iteration in `target` */ predicate iterates(ControlFlowNode target, ControlFlowNode sequence) { - sequence = getSequence() and - target = possibleTarget() and - not target = unrolledSuffix().possibleTarget() + sequence = this.getSequence() and + target = this.possibleTarget() and + not target = this.unrolledSuffix().possibleTarget() } /** Gets the sequence node for this `for` statement. */ diff --git a/python/ql/lib/semmle/python/Import.qll b/python/ql/lib/semmle/python/Import.qll index 40c1c27a8512..9620b01e4c60 100644 --- a/python/ql/lib/semmle/python/Import.qll +++ b/python/ql/lib/semmle/python/Import.qll @@ -31,7 +31,7 @@ class ImportExpr extends ImportExpr_ { // relative imports are no longer allowed in Python 3 major_version() < 3 and // and can be explicitly turned off in later versions of Python 2 - not getEnclosingModule().hasFromFuture("absolute_import") + not this.getEnclosingModule().hasFromFuture("absolute_import") } /** @@ -53,8 +53,8 @@ class ImportExpr extends ImportExpr_ { * the name of the topmost module that will be imported. */ private string relativeTopName() { - getLevel() = -1 and - result = basePackageName(1) + "." + this.getTopName() and + this.getLevel() = -1 and + result = this.basePackageName(1) + "." + this.getTopName() and valid_module_name(result) } @@ -62,7 +62,7 @@ class ImportExpr extends ImportExpr_ { if this.getLevel() <= 0 then result = this.getTopName() else ( - result = basePackageName(this.getLevel()) and + result = this.basePackageName(this.getLevel()) and valid_module_name(result) ) } @@ -73,17 +73,17 @@ class ImportExpr extends ImportExpr_ { * which may not be the name of the module. */ string bottomModuleName() { - result = relativeTopName() + this.remainderOfName() + result = this.relativeTopName() + this.remainderOfName() or - not exists(relativeTopName()) and + not exists(this.relativeTopName()) and result = this.qualifiedTopName() + this.remainderOfName() } /** Gets the name of topmost module or package being imported */ string topModuleName() { - result = relativeTopName() + result = this.relativeTopName() or - not exists(relativeTopName()) and + not exists(this.relativeTopName()) and result = this.qualifiedTopName() } @@ -94,7 +94,7 @@ class ImportExpr extends ImportExpr_ { */ string getImportedModuleName() { exists(string bottomName | bottomName = this.bottomModuleName() | - if this.isTop() then result = topModuleName() else result = bottomName + if this.isTop() then result = this.topModuleName() else result = bottomName ) } diff --git a/python/ql/lib/semmle/python/Module.qll b/python/ql/lib/semmle/python/Module.qll index 8f9344f60c07..6baf41b4a03e 100644 --- a/python/ql/lib/semmle/python/Module.qll +++ b/python/ql/lib/semmle/python/Module.qll @@ -86,13 +86,13 @@ class Module extends Module_, Scope, AstNode { /** Gets the package containing this module (or parent package if this is a package) */ Module getPackage() { this.getName().matches("%.%") and - result.getName() = getName().regexpReplaceAll("\\.[^.]*$", "") + result.getName() = this.getName().regexpReplaceAll("\\.[^.]*$", "") } /** Gets the name of the package containing this module */ string getPackageName() { this.getName().matches("%.%") and - result = getName().regexpReplaceAll("\\.[^.]*$", "") + result = this.getName().regexpReplaceAll("\\.[^.]*$", "") } /** Gets the metrics for this module */ diff --git a/python/ql/lib/semmle/python/RegexTreeView.qll b/python/ql/lib/semmle/python/RegexTreeView.qll index ad1949e4bc42..833abf41e09c 100644 --- a/python/ql/lib/semmle/python/RegexTreeView.qll +++ b/python/ql/lib/semmle/python/RegexTreeView.qll @@ -55,10 +55,10 @@ class RegExpParent extends TRegExpParent { abstract RegExpTerm getChild(int i); /** Gets a child term . */ - RegExpTerm getAChild() { result = getChild(_) } + RegExpTerm getAChild() { result = this.getChild(_) } /** Gets the number of child terms. */ - int getNumChild() { result = count(getAChild()) } + int getNumChild() { result = count(this.getAChild()) } /** Gets the associated regex. */ abstract Regex getRegex(); @@ -117,7 +117,7 @@ class RegExpTerm extends RegExpParent { RegExpTerm getRootTerm() { this.isRootTerm() and result = this or - result = getParent().(RegExpTerm).getRootTerm() + result = this.getParent().(RegExpTerm).getRootTerm() } /** @@ -196,7 +196,7 @@ class RegExpTerm extends RegExpParent { /** Gets the regular expression term that is matched (textually) before this one, if any. */ RegExpTerm getPredecessor() { - exists(RegExpTerm parent | parent = getParent() | + exists(RegExpTerm parent | parent = this.getParent() | result = parent.(RegExpSequence).previousElement(this) or not exists(parent.(RegExpSequence).previousElement(this)) and @@ -207,7 +207,7 @@ class RegExpTerm extends RegExpParent { /** Gets the regular expression term that is matched (textually) after this one, if any. */ RegExpTerm getSuccessor() { - exists(RegExpTerm parent | parent = getParent() | + exists(RegExpTerm parent | parent = this.getParent() | result = parent.(RegExpSequence).nextElement(this) or not exists(parent.(RegExpSequence).nextElement(this)) and @@ -358,7 +358,7 @@ class RegExpSequence extends RegExpTerm, TRegExpSequence { override RegExpTerm getChild(int i) { result = seqChild(re, start, end, i) } /** Gets the element preceding `element` in this sequence. */ - RegExpTerm previousElement(RegExpTerm element) { element = nextElement(result) } + RegExpTerm previousElement(RegExpTerm element) { element = this.nextElement(result) } /** Gets the element following `element` in this sequence. */ RegExpTerm nextElement(RegExpTerm element) { @@ -461,8 +461,8 @@ class RegExpEscape extends RegExpNormalChar { // TODO: Find a way to include a formfeed character // this.getUnescaped() = "f" and result = " " // or - isUnicode() and - result = getUnicode() + this.isUnicode() and + result = this.getUnicode() } predicate isIdentityEscape() { not this.getUnescaped() in ["n", "r", "t", "f"] } @@ -479,7 +479,7 @@ class RegExpEscape extends RegExpNormalChar { /** * Holds if this is a unicode escape. */ - private predicate isUnicode() { getText().prefix(2) = ["\\u", "\\U"] } + private predicate isUnicode() { this.getText().prefix(2) = ["\\u", "\\U"] } /** * Gets the unicode char for this escape. @@ -569,13 +569,13 @@ class RegExpCharacterClass extends RegExpTerm, TRegExpCharacterClass { predicate isUniversalClass() { // [^] - isInverted() and not exists(getAChild()) + this.isInverted() and not exists(this.getAChild()) or // [\w\W] and similar - not isInverted() and + not this.isInverted() and exists(string cce1, string cce2 | - cce1 = getAChild().(RegExpCharacterClassEscape).getValue() and - cce2 = getAChild().(RegExpCharacterClassEscape).getValue() + cce1 = this.getAChild().(RegExpCharacterClassEscape).getValue() and + cce2 = this.getAChild().(RegExpCharacterClassEscape).getValue() | cce1 != cce2 and cce1.toLowerCase() = cce2.toLowerCase() ) diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll index 169ebd191ba7..b41edd28898b 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll @@ -610,11 +610,11 @@ class DataFlowLambda extends DataFlowCallable, TLambda { override string toString() { result = lambda.toString() } - override CallNode getACall() { result = getCallableValue().getACall() } + override CallNode getACall() { result = this.getCallableValue().getACall() } override Scope getScope() { result = lambda.getEvaluatingScope() } - override NameNode getParameter(int n) { result = getParameter(getCallableValue(), n) } + override NameNode getParameter(int n) { result = getParameter(this.getCallableValue(), n) } override string getName() { result = "Lambda callable" } diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/LocalSources.qll b/python/ql/lib/semmle/python/dataflow/new/internal/LocalSources.qll index df1ee7bba165..76cc1573b248 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/LocalSources.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/LocalSources.qll @@ -62,12 +62,12 @@ class LocalSourceNode extends Node { /** * Gets a read of attribute `attrName` on this node. */ - AttrRead getAnAttributeRead(string attrName) { result = getAnAttributeReference(attrName) } + AttrRead getAnAttributeRead(string attrName) { result = this.getAnAttributeReference(attrName) } /** * Gets a write of attribute `attrName` on this node. */ - AttrWrite getAnAttributeWrite(string attrName) { result = getAnAttributeReference(attrName) } + AttrWrite getAnAttributeWrite(string attrName) { result = this.getAnAttributeReference(attrName) } /** * Gets a reference (read or write) of any attribute on this node. @@ -81,12 +81,12 @@ class LocalSourceNode extends Node { /** * Gets a read of any attribute on this node. */ - AttrRead getAnAttributeRead() { result = getAnAttributeReference() } + AttrRead getAnAttributeRead() { result = this.getAnAttributeReference() } /** * Gets a write of any attribute on this node. */ - AttrWrite getAnAttributeWrite() { result = getAnAttributeReference() } + AttrWrite getAnAttributeWrite() { result = this.getAnAttributeReference() } /** * Gets a call to this node. From a6115687aa712f0f3d12f1af7baf27445d38d33a Mon Sep 17 00:00:00 2001 From: Taus Date: Wed, 13 Oct 2021 13:31:27 +0000 Subject: [PATCH 2/2] Python: More implicit `this` --- python/ql/lib/semmle/python/essa/Definitions.qll | 4 ++-- python/ql/lib/semmle/python/essa/Essa.qll | 4 ++-- python/ql/lib/semmle/python/frameworks/Django.qll | 10 ++++++---- python/ql/lib/semmle/python/frameworks/Flask.qll | 4 ++-- python/ql/lib/semmle/python/frameworks/Stdlib.qll | 4 ++-- 5 files changed, 14 insertions(+), 12 deletions(-) diff --git a/python/ql/lib/semmle/python/essa/Definitions.qll b/python/ql/lib/semmle/python/essa/Definitions.qll index 752ff9da3295..e4e4bba747fd 100644 --- a/python/ql/lib/semmle/python/essa/Definitions.qll +++ b/python/ql/lib/semmle/python/essa/Definitions.qll @@ -225,9 +225,9 @@ class ModuleVariable extends SsaSourceVariable { } override ControlFlowNode getAnImplicitUse() { - result = global_variable_callnode() + result = this.global_variable_callnode() or - result = global_variable_import() + result = this.global_variable_import() or exists(ImportTimeScope scope | scope.entryEdge(result, _) | this = scope.getOuterVariable(_) or diff --git a/python/ql/lib/semmle/python/essa/Essa.qll b/python/ql/lib/semmle/python/essa/Essa.qll index d703b72242ab..2d403070c4c1 100644 --- a/python/ql/lib/semmle/python/essa/Essa.qll +++ b/python/ql/lib/semmle/python/essa/Essa.qll @@ -41,7 +41,7 @@ class EssaVariable extends TEssaDefinition { */ ControlFlowNode getASourceUse() { exists(SsaSourceVariable var | - result = use_for_var(var) and + result = this.use_for_var(var) and result = var.getASourceUse() ) } @@ -258,7 +258,7 @@ class PhiFunction extends EssaDefinition, TPhiFunction { /** Gets another definition of the same source variable that reaches this definition. */ private EssaDefinition reachingDefinition(BasicBlock pred) { result.getScope() = this.getScope() and - result.getSourceVariable() = pred_var(pred) and + result.getSourceVariable() = this.pred_var(pred) and result.reachesEndOfBlock(pred) } diff --git a/python/ql/lib/semmle/python/frameworks/Django.qll b/python/ql/lib/semmle/python/frameworks/Django.qll index 08afa55635ae..89ff0537c975 100644 --- a/python/ql/lib/semmle/python/frameworks/Django.qll +++ b/python/ql/lib/semmle/python/frameworks/Django.qll @@ -1844,11 +1844,13 @@ private module PrivateDjango { t.start() and result.asCfgNode().(CallNode).getFunction() = this.asViewRef().asCfgNode() or - exists(DataFlow::TypeTracker t2 | result = asViewResult(t2).track(t2, t)) + exists(DataFlow::TypeTracker t2 | result = this.asViewResult(t2).track(t2, t)) } /** Gets a reference to the result of calling the `as_view` classmethod of this class. */ - DataFlow::Node asViewResult() { asViewResult(DataFlow::TypeTracker::end()).flowsTo(result) } + DataFlow::Node asViewResult() { + this.asViewResult(DataFlow::TypeTracker::end()).flowsTo(result) + } } /** A class that we consider a django View class. */ @@ -1944,10 +1946,10 @@ private module PrivateDjango { abstract DataFlow::Node getViewArg(); final override DjangoRouteHandler getARequestHandler() { - poorMansFunctionTracker(result) = getViewArg() + poorMansFunctionTracker(result) = this.getViewArg() or exists(DjangoViewClass vc | - getViewArg() = vc.asViewResult() and + this.getViewArg() = vc.asViewResult() and result = vc.getARequestHandler() ) } diff --git a/python/ql/lib/semmle/python/frameworks/Flask.qll b/python/ql/lib/semmle/python/frameworks/Flask.qll index e854e07658b9..3c2cc5af5ecf 100644 --- a/python/ql/lib/semmle/python/frameworks/Flask.qll +++ b/python/ql/lib/semmle/python/frameworks/Flask.qll @@ -292,12 +292,12 @@ module Flask { override Function getARequestHandler() { exists(DataFlow::LocalSourceNode func_src | - func_src.flowsTo(getViewArg()) and + func_src.flowsTo(this.getViewArg()) and func_src.asExpr().(CallableExpr) = result.getDefinition() ) or exists(FlaskViewClass vc | - getViewArg() = vc.asViewResult().getAUse() and + this.getViewArg() = vc.asViewResult().getAUse() and result = vc.getARequestHandler() ) } diff --git a/python/ql/lib/semmle/python/frameworks/Stdlib.qll b/python/ql/lib/semmle/python/frameworks/Stdlib.qll index f89f74f3c34f..0267d2a80b9a 100644 --- a/python/ql/lib/semmle/python/frameworks/Stdlib.qll +++ b/python/ql/lib/semmle/python/frameworks/Stdlib.qll @@ -397,8 +397,8 @@ private module StdlibPrivate { result = this.get_executable_arg() or exists(DataFlow::Node arg_args, boolean shell | - arg_args = get_args_arg() and - shell = get_shell_arg_value() + arg_args = this.get_args_arg() and + shell = this.get_shell_arg_value() | // When "executable" argument is set, and "shell" argument is `False`, the // "args" argument will only be used to set the program name and arguments to