Merge pull request #8336 from tausbn/python-fix-a-bunch-of-ql-warnings

Python: Fix a bunch of QL warnings

python/ql/src/Classes/ClassAttributes.qll

@@ -1,7 +1,7 @@
 import python
 private import semmle.python.pointsto.PointsTo
 
-/** Helper class for UndefinedClassAttribute.ql and MaybeUndefinedClassAttribute.ql */
+/** A helper class for UndefinedClassAttribute.ql and MaybeUndefinedClassAttribute.ql */
 class CheckClass extends ClassObject {
   private predicate ofInterest() {
     not this.unknowableAttributes() and

python/ql/src/Exceptions/UnguardedNextInGenerator.ql

@@ -32,7 +32,11 @@ predicate guarded_not_empty_sequence(EssaVariable sequence) {
   sequence.getDefinition() instanceof EssaEdgeRefinement
 }
 
-/** The pattern `next(iter(x))` is often used where `x` is known not be empty. Check for that. */
+/**
+ * Holds if `iterator` is not exhausted.
+ *
+ * The pattern `next(iter(x))` is often used where `x` is known not be empty. Check for that.
+ */
 predicate iter_not_exhausted(EssaVariable iterator) {
   exists(EssaVariable sequence |
     call_to_iter(iterator.getDefinition().(AssignmentDefinition).getValue(), sequence) and

python/ql/src/Expressions/Formatting/AdvancedFormatting.qll

@@ -1,7 +1,7 @@
 import python
 
 /** A string constant that looks like it may be used in string formatting operations. */
-library class PossibleAdvancedFormatString extends StrConst {
+class PossibleAdvancedFormatString extends StrConst {
   PossibleAdvancedFormatString() { this.getText().matches("%{%}%") }
 
   private predicate field(int start, int end) {

python/ql/src/Imports/DeprecatedModule.ql

@@ -13,7 +13,7 @@
 import python
 
 /**
- * The module `name` was deprecated in Python version `major`.`minor`,
+ * Holds if the module `name` was deprecated in Python version `major`.`minor`,
  * and module `instead` should be used instead (or `instead = "no replacement"`)
  */
 predicate deprecated_module(string name, string instead, int major, int minor) {

python/ql/src/Lexical/CommentedOutCode.qll

@@ -178,7 +178,7 @@ class CommentedOutCodeBlock extends @py_comment {
   /** Gets a textual representation of this element. */
   string toString() { result = "Commented out code" }
 
-  /** Whether this commented-out code block contains the comment c */
+  /** Holds if this commented-out code block contains the comment c */
   predicate contains(Comment c) {
     this = c
     or
@@ -189,7 +189,7 @@ class CommentedOutCodeBlock extends @py_comment {
     )
   }
 
-  /** The length of this comment block (in comments) */
+  /** Gets the length of this comment block (in comments) */
   int length() { result = count(Comment c | this.contains(c)) }
 
   /**

python/ql/src/Security/CWE-022/TarSlip.ql

@@ -122,7 +122,7 @@ class ExtractMembersSink extends TaintSink {
 class TarFileInfoSanitizer extends Sanitizer {
   TarFileInfoSanitizer() { this = "TarInfo sanitizer" }
 
-  /** The test `if <path_sanitizing_test>:` clears taint on its `false` edge. */
+  /* The test `if <path_sanitizing_test>:` clears taint on its `false` edge. */
   override predicate sanitizingEdge(TaintKind taint, PyEdgeRefinement test) {
     taint instanceof TarFileInfo and
     clears_taint_on_false_edge(test.getTest(), test.getSense())

python/ql/src/Security/CWE-327/TlsLibraryModel.qll

@@ -85,7 +85,7 @@ abstract class TlsLibrary extends string {
   bindingset[this]
   TlsLibrary() { any() }
 
-  /** The name of a specific protocol version. */
+  /** Gets the name of a specific protocol version. */
   abstract string specific_version_name(ProtocolVersion version);
 
   /** Gets a name, which is a member of `version_constants`, that can be used to specify the protocol family `family`. */

python/ql/src/Variables/Definition.qll

@@ -5,12 +5,12 @@ import python
  */
 class Definition extends NameNode, DefinitionNode {
   /**
-   * The variable defined by this control-flow node.
+   * Gets the variable defined by this control-flow node.
    */
   Variable getVariable() { this.defines(result) }
 
   /**
-   * The SSA variable corresponding to the current definition. Since SSA variables
+   * Gets the SSA variable corresponding to the current definition. Since SSA variables
    * are only generated for definitions with at least one use, not all definitions
    * will have an SSA variable.
    */
@@ -67,7 +67,7 @@ class Definition extends NameNode, DefinitionNode {
   }
 
   /**
-   * An immediate re-definition of this definition's variable.
+   * Gets an immediate re-definition of this definition's variable.
    */
   Definition getARedef() {
     result != this and

python/ql/src/Variables/MultiplyDefined.ql

@@ -47,6 +47,8 @@ predicate simple_literal(Expr e) {
 }
 
 /**
+ * Holds if the redefinition is uninteresting.
+ *
  * A multiple definition is 'uninteresting' if it sets a variable to a
  * simple literal before reassigning it.
  * x = None

python/ql/src/Variables/ShadowBuiltin.ql

@@ -17,30 +17,13 @@ import Shadowing
 import semmle.python.types.Builtins
 
 predicate allow_list(string name) {
-  /* These are rarely used and thus unlikely to be confusing */
-  name = "iter" or
-  name = "next" or
-  name = "input" or
-  name = "file" or
-  name = "apply" or
-  name = "slice" or
-  name = "buffer" or
-  name = "coerce" or
-  name = "intern" or
-  name = "exit" or
-  name = "quit" or
-  name = "license" or
-  /* These are short and/or hard to avoid */
-  name = "dir" or
-  name = "id" or
-  name = "max" or
-  name = "min" or
-  name = "sum" or
-  name = "cmp" or
-  name = "chr" or
-  name = "ord" or
-  name = "bytes" or
-  name = "_"
+  name in [
+      /* These are rarely used and thus unlikely to be confusing */
+      "iter", "next", "input", "file", "apply", "slice", "buffer", "coerce", "intern", "exit",
+      "quit", "license",
+      /* These are short and/or hard to avoid */
+      "dir", "id", "max", "min", "sum", "cmp", "chr", "ord", "bytes", "_",
+    ]
 }
 
 predicate shadows(Name d, string name, Function scope, int line) {

python/ql/src/Variables/Undefined.qll

@@ -2,7 +2,7 @@ import python
 import Loop
 import semmle.python.dataflow.TaintTracking
 
-/** Marker for "uninitialized". */
+/** A marker for "uninitialized". */
 class Uninitialized extends TaintKind {
   Uninitialized() { this = "undefined" }
 }

python/ql/src/analysis/DefinitionTracking.qll

@@ -467,10 +467,10 @@ Definition getUniqueDefinition(Expr use) {
   not result = TLocalDefinition(use)
 }
 
-/** Helper class to get suitable locations for attributes */
-class NiceLocationExpr extends @py_expr {
+/** A helper class to get suitable locations for attributes */
+class NiceLocationExpr extends Expr {
   /** Gets a textual representation of this element. */
-  string toString() { result = this.(Expr).toString() }
+  override string toString() { result = this.(Expr).toString() }
 
   /**
    * Holds if this element is at the specified location.

python/ql/src/analysis/ImportFailure.ql

@@ -58,7 +58,7 @@ predicate ok_to_fail(ImportExpr ie) {
   os_specific_import(ie) != get_os()
 }
 
-class VersionTest extends @py_flow_node {
+class VersionTest extends ControlFlowNode {
   VersionTest() {
     exists(string name |
       name.matches("%version%") and
@@ -66,7 +66,7 @@ class VersionTest extends @py_flow_node {
     )
   }
 
-  string toString() { result = "VersionTest" }
+  override string toString() { result = "VersionTest" }
 }
 
 /** A guard on the version of the Python interpreter */

python/ql/src/experimental/Security/CWE-348/ClientSuppliedIpUsedInSecurityCheck.ql

@@ -18,7 +18,7 @@ import ClientSuppliedIpUsedInSecurityCheckLib
 import DataFlow::PathGraph
 
 /**
- * Taint-tracking configuration tracing flow from obtaining a client ip from an HTTP header to a sensitive use.
+ * A taint-tracking configuration tracing flow from obtaining a client ip from an HTTP header to a sensitive use.
  */
 class ClientSuppliedIpUsedInSecurityCheckConfig extends TaintTracking::Configuration {
   ClientSuppliedIpUsedInSecurityCheckConfig() { this = "ClientSuppliedIpUsedInSecurityCheckConfig" }

python/ql/src/experimental/semmle/python/templates/Airspeed.qll

@@ -8,7 +8,7 @@ import experimental.semmle.python.templates.SSTISink
 deprecated ClassValue theAirspeedTemplateClass() { result = Value::named("airspeed.Template") }
 
 /**
- * Sink representing the `airspeed.Template` class instantiation argument.
+ * A sink representing the `airspeed.Template` class instantiation argument.
  *
  *  import airspeed
  *  temp = airspeed.Template(`"sink"`)

python/ql/src/experimental/semmle/python/templates/Bottle.qll

@@ -10,7 +10,7 @@ deprecated ClassValue theBottleSimpleTemplateClass() {
 }
 
 /**
- * Sink representing the `bottle.SimpleTemplate` class instantiation argument.
+ * A sink representing the `bottle.SimpleTemplate` class instantiation argument.
  *
  *  from bottle import SimpleTemplate
  *  template = SimpleTemplate(`sink`)
@@ -29,7 +29,7 @@ deprecated class BottleSimpleTemplateSink extends SSTISink {
 }
 
 /**
- * Sink representing the `bottle.template` function call argument.
+ * A sink representing the `bottle.template` function call argument.
  *
  *  from bottle import template
  *  tmp = template(`sink`)

python/ql/src/experimental/semmle/python/templates/Chameleon.qll

@@ -10,7 +10,7 @@ deprecated ClassValue theChameleonPageTemplateClass() {
 }
 
 /**
- * Sink representing the `chameleon.PageTemplate` class instantiation argument.
+ * A sink representing the `chameleon.PageTemplate` class instantiation argument.
  *
  *  from chameleon import PageTemplate
  *  template = PageTemplate(`sink`)

python/ql/src/experimental/semmle/python/templates/Cheetah.qll

@@ -10,7 +10,7 @@ deprecated ClassValue theCheetahTemplateClass() {
 }
 
 /**
- * Sink representing the instantiation argument of any class which derives from
+ * A sink representing the instantiation argument of any class which derives from
  * the `Cheetah.Template.Template` class .
  *
  *  from Cheetah.Template import Template

python/ql/src/experimental/semmle/python/templates/Chevron.qll

@@ -8,7 +8,7 @@ import experimental.semmle.python.templates.SSTISink
 deprecated Value theChevronRenderFunc() { result = Value::named("chevron.render") }
 
 /**
- * Sink representing the `chevron.render` function call argument.
+ * A sink representing the `chevron.render` function call argument.
  *
  *  import chevron
  *  tmp = chevron.render(`sink`,{ 'key' : 'value' })

python/ql/src/experimental/semmle/python/templates/DjangoTemplate.qll

@@ -7,7 +7,7 @@ import experimental.semmle.python.templates.SSTISink
 deprecated ClassValue theDjangoTemplateClass() { result = Value::named("django.template.Template") }
 
 /**
- * Sink representng `django.template.Template` class instantiation argument.
+ * A sink representng `django.template.Template` class instantiation argument.
  *
  *  from django.template import Template
  *  template = Template(`sink`)

python/ql/src/experimental/semmle/python/templates/FlaskTemplate.qll

@@ -9,7 +9,7 @@ deprecated Value theFlaskRenderTemplateClass() {
 }
 
 /**
- * Sink representng `flask.render_template_string` function call argument.
+ * A sink representng `flask.render_template_string` function call argument.
  *
  *  from flask import render_template_string
  *  render_template_string(`sink`)

python/ql/src/experimental/semmle/python/templates/Genshi.qll

@@ -15,7 +15,7 @@ deprecated ClassValue theGenshiMarkupTemplateClass() {
 }
 
 /**
- * Sink representing the `genshi.template.TextTemplate` class instantiation argument.
+ * A sink representing the `genshi.template.TextTemplate` class instantiation argument.
  *
  *  from genshi.template import TextTemplate
  *  tmpl = TextTemplate('sink')
@@ -34,7 +34,7 @@ deprecated class GenshiTextTemplateSink extends SSTISink {
 }
 
 /**
- * Sink representing the `genshi.template.MarkupTemplate` class instantiation argument.
+ * A sink representing the `genshi.template.MarkupTemplate` class instantiation argument.
  *
  *  from genshi.template import MarkupTemplate
  *  tmpl = MarkupTemplate('sink')

python/ql/src/experimental/semmle/python/templates/Jinja.qll

@@ -11,7 +11,7 @@ deprecated ClassValue theJinja2TemplateClass() { result = Value::named("jinja2.T
 deprecated Value theJinja2FromStringValue() { result = Value::named("jinja2.from_string") }
 
 /**
- * Sink representing the `jinja2.Template` class instantiation argument.
+ * A sink representing the `jinja2.Template` class instantiation argument.
  *
  *  from jinja2 import Template
  *  template = Template(`sink`)
@@ -30,7 +30,7 @@ deprecated class Jinja2TemplateSink extends SSTISink {
 }
 
 /**
- * Sink representing the `jinja2.from_string` function call argument.
+ * A sink representing the `jinja2.from_string` function call argument.
  *
  *  from jinja2 import from_string
  *  template = from_string(`sink`)

python/ql/src/experimental/semmle/python/templates/Mako.qll

@@ -8,7 +8,7 @@ import experimental.semmle.python.templates.SSTISink
 deprecated ClassValue theMakoTemplateClass() { result = Value::named("mako.template.Template") }
 
 /**
- * Sink representing the `mako.template.Template` class instantiation argument.
+ * A sink representing the `mako.template.Template` class instantiation argument.
  *
  *  from mako.template import Template
  *  mytemplate = Template("hello world!")

python/ql/src/experimental/semmle/python/templates/TRender.qll

@@ -8,7 +8,7 @@ import experimental.semmle.python.templates.SSTISink
 deprecated ClassValue theTRenderTemplateClass() { result = Value::named("trender.TRender") }
 
 /**
- * Sink representing the `trender.TRender` class instantiation argument.
+ * A sink representing the `trender.TRender` class instantiation argument.
  *
  *  from trender import TRender
  *  template = TRender(`sink`)

python/ql/src/printAst.ql

@@ -12,7 +12,7 @@ import semmle.python.PrintAst
 import analysis.DefinitionTracking
 
 /**
- * The source file to generate an AST from.
+ * Gets the source file that will be used to generate the AST.
  */
 external string selectedSourceFile();
 

python/ql/src/semmle/python/functions/ModificationOfParameterWithDefaultCustomizations.qll

@@ -81,7 +81,7 @@ module ModificationOfParameterWithDefault {
   }
 
   /**
-   * A name of a list function that modifies the list.
+   * Gets the name of a list function that modifies the list.
    * See https://docs.python.org/3/tutorial/datastructures.html#more-on-lists
    */
   string list_modifying_method() {
@@ -89,7 +89,7 @@ module ModificationOfParameterWithDefault {
   }
 
   /**
-   * A name of a dict function that modifies the dict.
+   * Gets the name of a dict function that modifies the dict.
    * See https://docs.python.org/3/library/stdtypes.html#dict
    */
   string dict_modifying_method() { result in ["clear", "pop", "popitem", "setdefault", "update"] }