Python: Minor documantation updates to several quality queries

[joefarebrother]

Updates documentation of several quality queries, to use python 3 rather than python 2 documentation as references, and a few minor grammatical fixes.

[Copilot]

Pull Request Overview

This PR updates documentation for several Python quality queries to reference Python 3 instead of Python 2 documentation links, along with minor grammatical improvements and formatting fixes.

• Updates documentation links from Python 2.7 to Python 3 across multiple query help files
• Fixes grammatical issues by adding missing commas and improving sentence structure
• Updates code formatting to use backticks for consistency and modernizes Python 2 syntax to Python 3

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
ConsistentReturns.expected	Updates error message punctuation (comma addition)
ModificationOfParameterWithDefault.qhelp	Updates Python documentation link from version 2 to 3
InitIsGenerator.qhelp	Updates Python documentation link from version 2.7 to 3
ConsistentReturns.ql	Improves code formatting and error message punctuation
ConsistentReturns.qhelp	Adds missing comma and updates documentation link
UnsupportedFormatCharacter.qhelp	Clarifies description and updates documentation link
IncorrectComparisonUsingIs.ql	Updates code formatting for consistency
IncorrectComparisonUsingIs.qhelp	Updates Python documentation link
ExplicitCallToDel.qhelp	Adds missing comma for grammatical correctness
DuplicateKeyInDictionaryLiteral.qhelp	Improves punctuation and updates documentation link
DuplicateKeyInDictionaryLiteral.py	Modernizes Python 2 syntax to Python 3 and adds explanatory comment
CallToSuperWrongClass.qhelp	Adds missing commas and updates documentation link
EmptyExcept.qhelp	Adds missing comma for grammatical correctness
CatchingBaseException.qhelp	Updates multiple Python documentation links from version 2 to 3

[github-actions]

QHelp previews:

python/ql/src/Exceptions/CatchingBaseException.qhelp

Except block handles 'BaseException'

All exception classes in Python derive from BaseException. BaseException has three important subclasses, Exception from which all errors and normal exceptions derive, KeyboardInterrupt which is raised when the user interrupts the program from the keyboard and SystemExit which is raised by the sys.exit() function to terminate the program.

Since KeyboardInterrupt and SystemExit are special they should not be grouped together with other Exception classes.

Catching BaseException, rather than its subclasses may prevent proper handling of KeyboardInterrupt or SystemExit. It is easy to catch BaseException accidentally as it is caught implicitly by an empty except: statement.

Recommendation

Handle Exception, KeyboardInterrupt and SystemExit separately. Do not use the plain except: form.

Example

In these examples, a function application.main() is called that might raise SystemExit. In the first two functions, BaseException is caught, but this will discard KeyboardInterrupt. In the third function, call_main_program_fixed only SystemExit is caught, leaving KeyboardInterrupt to propagate.

In these examples KeyboardInterrupt is accidentally ignored.

def call_main_program_implicit_handle_base_exception():
    try:
        #application.main calls sys.exit() when done.
        application.main()
    except Exception as ex:
        log(ex)
    except:
        pass

def call_main_program_explicit_handle_base_exception():
    try:
        #application.main calls sys.exit() when done.
        application.main()
    except Exception as ex:
        log(ex)
    except BaseException:
        pass

def call_main_program_fixed():
    try:
        #application.main calls sys.exit() when done.
        application.main()
    except Exception as ex:
        log(ex)
    except SystemExit:
        pass

References

• Python Language Reference: The try statement, Exceptions.
• M. Lutz, Learning Python, Section 35.3: Exception Design Tips and Gotchas, O'Reilly Media, 2013.
• Python Tutorial: Errors and Exceptions.
• Common Weakness Enumeration: CWE-396.

python/ql/src/Exceptions/EmptyExcept.qhelp

Empty except

Ignoring exceptions that should be dealt with in some way is almost always a bad idea. The loss of information can lead to hard to debug errors and incomplete log files. It is even possible that ignoring an exception can cause a security vulnerability. An empty except block may be an indication that the programmer intended to handle the exception, but never wrote the code to do so.

Recommendation

Ensure all exceptions are handled correctly.

Example

In this example, the program keeps running with the same privileges if it fails to drop to lower privileges.

# ...
try:
    security_manager.drop_privileges()
except SecurityError:
    pass
# ...

References

• Common Weakness Enumeration: CWE-390.

python/ql/src/Expressions/CallToSuperWrongClass.qhelp

First argument to super() is not enclosing class

The super class should be called with the enclosing class as its first argument and self as its second argument.

Passing a different class may work correctly, provided the class passed is a super class of the enclosing class and the enclosing class does not define an __init__ method. However, this may result in incorrect object initialization if the enclosing class is later subclassed using multiple inheritance.

Recommendation

Ensure that the first argument to super() is the enclosing class.

Example

In this example, the call to super(Vehicle, self) in Car.__init__ is incorrect, as it passes Vehicle rather than Car as the first argument to super. As a result, super(SportsCar, self).__init__() in the SportsCar.__init__ method will not call all __init__() methods because the call to super(Vehicle, self).__init__() skips StatusSymbol.__init__().

class Vehicle(object):
    pass
        
class Car(Vehicle):
    
    def __init__(self):
        #This is OK provided that Car is not subclassed.
        super(Vehicle, self).__init__()
        self.car_init()
        
class StatusSymbol(object):
    
    def __init__(self):
        super(StatusSymbol, self).__init__()
        self.show_off()
        
class SportsCar(Car, StatusSymbol):
    
    def __init__(self):
        #This will not call StatusSymbol.__init__()
        super(SportsCar, self).__init__()
        self.sports_car_init()

        
#Fix Car by passing Car to super().
#SportsCar does not need to be changed.
class Car(Car, Vehicle):
    
    def __init__(self):
        super(Car, self).__init__()
        self.car_init()
        

References

• Python Standard Library: super.
• Artima Developer: Things to Know About Python Super.
• Common Weakness Enumeration: CWE-687.

python/ql/src/Expressions/DuplicateKeyInDictionaryLiteral.qhelp

Duplicate key in dict literal

Dictionary literals are constructed in the order given in the source. This means that if a key is duplicated, the second key-value pair will overwrite the first; as a dictionary can only have one value per key.

Recommendation

Check for typos to ensure that the keys are supposed to be the same. If they are then decide which value is wanted and delete the other one.

Example

The following example will output "c", because the mapping between 2 and "b" is overwritten by the mapping from 2 to "c". The programmer may have meant to map 3 to "c" instead.

dictionary = {1:"a", 2:"b", 2:"c"} # BAD: The `2` key is duplicated.
print(dictionary[2])

References

• Python: Dictionary literals.
• Common Weakness Enumeration: CWE-561.

python/ql/src/Expressions/ExplicitCallToDel.qhelp

__del__ is called explicitly

The __del__ special method is designed to be called by the Python virtual machine when an object is no longer reachable, but before it is destroyed. Calling a __del__ method explicitly may cause an object to enter an unsafe state.

Recommendation

If explicit clean up of an object is required, a close() method should be called or, better still, wrap the use of the object in a with statement.

Example

In the first example, rather than close the zip file in a conventional manner, the programmer has called __del__. A safer alternative is shown in the second example.

def extract_bad(zippath, dest):
    zipped = ZipFile(zippath)
    try:
        zipped.extractall(dest)
    finally:
        zipped.__del__()

def extract_good(zippath, dest):
    zipped = ZipFile(zippath)
    try:
        zipped.extractall(dest)
    finally:
        zipped.close()

References

• Python Standard Library: object.del

python/ql/src/Expressions/IncorrectComparisonUsingIs.qhelp

Comparison using is when operands support __eq__

When you compare two values using the is or is not operator, it is the object identities of the two values that is tested rather than their equality. If the class of either of the values in the comparison redefines equality then the is operator may return False even though the objects compare as equal. Equality is defined by the __eq__ or, in Python2, __cmp__ method. To compare two objects for equality, use the == or != operator instead.

Recommendation

When you want to compare the value of two literals, use the comparison operator == or != in place of is or is not.

If the uniqueness property or performance are important then use an object that does not redefine equality.

Example

In the first line of the following example the programmer tests the value of value against DEFAULT using the is operator. Unfortunately, this may fail when the function is called with the string "default".

To function correctly, change the expression value is DEFAULT to value == DEFAULT. Alternatively, if the uniqueness property is desirable, then change the definition of DEFAULT to either of the alternatives below.

DEFAULT = "default"

def get_color(name, fallback):
    if name in COLORS:
        return COLORS[name]
    elif fallback is DEFAULT:
        return DEFAULT_COLOR
    else:
        return fallback

#This works
print (get_color("spam", "def" + "ault"))

#But this does not
print (get_color("spam", "default-spam"[:7]))

#To fix the above code change to object
DEFAULT = object()

#Or if you want better repr() output:
class Default(object):

    def __repr__(self):
        return "DEFAULT"

DEFAULT = Default()

References

• Python Standard Library: Comparisons.

python/ql/src/Expressions/UnsupportedFormatCharacter.qhelp

Unsupported format character

A printf-style format string (i.e. a string that is used as the left hand side of the % operator, such as fmt % arguments) must consist of valid conversion specifiers, such as %s, %d, etc. Otherwise, a ValueError will be raised.

Recommendation

Ensure a valid conversion specifier is used.

Example

In the following example, format_as_tuple_incorrect, %t is not a valid conversion specifier.

def format_as_tuple_incorrect(args):
    return "%t" % args

def format_as_tuple_correct(args):
    return "%r" % (args,)

References

• Python Library Reference: printf-style String Formatting.

python/ql/src/Functions/ConsistentReturns.qhelp

Explicit returns mixed with implicit (fall through) returns

When a function contains both explicit returns (return value) and implicit returns (where code falls off the end of a function), this often indicates that a return statement has been forgotten. It is best to return an explicit return value even when returning None because this makes it easier for other developers to read your code.

Recommendation

Add an explicit return at the end of the function.

Example

In the check_state1 function, the developer probably did intend to use an implicit return value of None as this equates to False. However, the function in check_state2 is easier to read.

    def check_state1(state, interactive=True):
        if not state['good'] or not state['bad']:
            if (good or bad or skip or reset) and interactive:
                return                                          # implicitly return None
            if not state['good']:
                raise util.Abort(_('cannot bisect (no known good revisions)'))
            else:
                raise util.Abort(_('cannot bisect (no known bad revisions)')) 
        return True  							                              
           
    def check_state2(state, interactive=True):
        if not state['good'] or not state['bad']:
            if (good or bad or skip or reset) and interactive:
                return False                                    # return an explicit value
            if not state['good']:
                raise util.Abort(_('cannot bisect (no known good revisions)'))
            else:
                raise util.Abort(_('cannot bisect (no known bad revisions)'))
        return True

References

• Python Language Reference: Function definitions.

python/ql/src/Functions/InitIsGenerator.qhelp

__init__ method is a generator

The __init__ method of a class is used to initialize new objects, not create them. As such, it should not return any value. By including a yield expression in the method turns it into a generator method. On calling it will return a generator resulting in a runtime error.

Recommendation

The presence of a yield expression in an __init__ method suggests a logical error, so it is not possible to suggest a general fix.

Example

In this example the __init__ method contains a yield expression. This is not logical in the context of an initializer.

class InitIsGenerator(object):
    def __init__(self, i):
        yield i

References

• Python: The init method.

python/ql/src/Functions/ModificationOfParameterWithDefault.qhelp

Modification of parameter with default

The default value of a parameter is computed once when the function is created, not for every invocation. The "pre-computed" value is then used for every subsequent call to the function. Consequently, if you modify the default value for a parameter this "modified" default value is used for the parameter in future calls to the function. This means that the function may not behave as expected in future calls and also makes the function more difficult to understand.

Recommendation

If a parameter has a default value, do not modify the default value. When you use a mutable object as a default value, you should use a placeholder value instead of modifying the default value. This is a particular problem when you work with lists and dictionaries but there are standard methods of avoiding modifying the default parameter (see References).

Example

In the following example, the default parameter is set with a default value of an empty list. Other commands in the function then append values to the list. The next time the function is called, the list will contain values, which may not have been intended.

    def __init__(self, name, choices=[], default=[], shortDesc=None,
                 longDesc=None, hints=None, allowNone=1):   # 'default' parameter assigned a value
        self.choices = choices
        if choices and not default:
            default.append(choices[0][1])                   # value of 'default' parameter modified
        Argument.__init__(self, name, default, shortDesc, longDesc, hints, allowNone=allowNone)

The recommended workaround is use a placeholder value. That is, define the function with a default of default=None, check if the parameter is None and then set the parameter to a list.

References

• Effbot: Default Parameter Values in Python.
• Python Language Reference: Function definitions.