Merge pull request #3953 from mwichmann/util-lint

Reduce pylint complaints about Util

9 files changed, 811 insertions, 581 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index 864095b6f..7715f263d 100755
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -98,6 +98,7 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
       SetOption equivalents to --hash-chunksize, --implicit-deps-unchanged
       and --implicit-deps-changed are enabled.
     - Add tests for SetOption failing on disallowed options and value types.
+    - Maintenance: eliminate lots of checker complaints about Util.py.
     - Maintenance: fix checker-spotted issues in Environment (apply_tools)
       and EnvironmentTests (asserts comparing with self).
       For consistency, env.Tool() now returns a tool object the same way
diff --git a/SCons/Tool/MSCommon/common.py b/SCons/Tool/MSCommon/common.py
index 81004df81..9d0183543 100644
--- a/SCons/Tool/MSCommon/common.py
+++ b/SCons/Tool/MSCommon/common.py
@@ -163,7 +163,7 @@ def has_reg(value):
     try:
         SCons.Util.RegOpenKeyEx(SCons.Util.HKEY_LOCAL_MACHINE, value)
         ret = True
-    except SCons.Util.WinError:
+    except OSError:
         ret = False
     return ret
 
diff --git a/SCons/Tool/MSCommon/netframework.py b/SCons/Tool/MSCommon/netframework.py
index b40576a87..5e2c33a69 100644
--- a/SCons/Tool/MSCommon/netframework.py
+++ b/SCons/Tool/MSCommon/netframework.py
@@ -41,7 +41,7 @@ def find_framework_root():
     try:
         froot = read_reg(_FRAMEWORKDIR_HKEY_ROOT)
         debug("Found framework install root in registry: {}".format(froot))
-    except SCons.Util.WinError as e:
+    except OSError:
         debug("Could not read reg key {}".format(_FRAMEWORKDIR_HKEY_ROOT))
         return None
 
diff --git a/SCons/Tool/MSCommon/sdk.py b/SCons/Tool/MSCommon/sdk.py
index b76fbddf4..439a7ade8 100644
--- a/SCons/Tool/MSCommon/sdk.py
+++ b/SCons/Tool/MSCommon/sdk.py
@@ -78,7 +78,7 @@ class SDKDefinition:
 
         try:
             sdk_dir = read_reg(hkey)
-        except SCons.Util.WinError as e:
+        except OSError:
             debug('find_sdk_dir(): no SDK registry key {}'.format(repr(hkey)))
             return None
 
diff --git a/SCons/Tool/MSCommon/vc.py b/SCons/Tool/MSCommon/vc.py
index 87a1064c4..66a081f1b 100644
--- a/SCons/Tool/MSCommon/vc.py
+++ b/SCons/Tool/MSCommon/vc.py
@@ -447,7 +447,7 @@ def find_vc_pdir(env, msvc_version):
                 comps = find_vc_pdir_vswhere(msvc_version, env)
                 if not comps:
                     debug('no VC found for version {}'.format(repr(msvc_version)))
-                    raise SCons.Util.WinError
+                    raise OSError
                 debug('VC found: {}'.format(repr(msvc_version)))
                 return comps
             else:
@@ -455,13 +455,13 @@ def find_vc_pdir(env, msvc_version):
                     try:
                         # ordinarily at win64, try Wow6432Node first.
                         comps = common.read_reg(root + 'Wow6432Node\\' + key, hkroot)
-                    except SCons.Util.WinError as e:
+                    except OSError:
                         # at Microsoft Visual Studio for Python 2.7, value is not in Wow6432Node
                         pass
                 if not comps:
                     # not Win64, or Microsoft Visual Studio for Python 2.7
                     comps = common.read_reg(root + key, hkroot)
-        except SCons.Util.WinError as e:
+        except OSError:
             debug('no VC registry key {}'.format(repr(key)))
         else:
             debug('found VC in registry: {}'.format(comps))
diff --git a/SCons/Tool/MSCommon/vs.py b/SCons/Tool/MSCommon/vs.py
index cc8946f52..7be7049ec 100644
--- a/SCons/Tool/MSCommon/vs.py
+++ b/SCons/Tool/MSCommon/vs.py
@@ -82,7 +82,7 @@ class VisualStudio:
             key = root + key
             try:
                 comps = read_reg(key)
-            except SCons.Util.WinError as e:
+            except OSError:
                 debug('no VS registry key {}'.format(repr(key)))
             else:
                 debug('found VS in registry: {}'.format(comps))
diff --git a/SCons/Tool/intelc.py b/SCons/Tool/intelc.py
index 0d07c728f..ac6fa6077 100644
--- a/SCons/Tool/intelc.py
+++ b/SCons/Tool/intelc.py
@@ -175,9 +175,7 @@ def get_intel_registry_value(valuename, version=None, abi=None):
             except SCons.Util.RegError:
                 raise MissingRegistryError("%s was not found in the registry, for Intel compiler version %s, abi='%s'"%(K, version,abi))
 
-        except SCons.Util.RegError:
-            raise MissingRegistryError("%s was not found in the registry, for Intel compiler version %s, abi='%s'"%(K, version,abi))
-        except SCons.Util.WinError:
+        except (SCons.Util.RegError, OSError):
             raise MissingRegistryError("%s was not found in the registry, for Intel compiler version %s, abi='%s'"%(K, version,abi))
 
     # Get the value:
@@ -201,7 +199,7 @@ def get_all_compiler_versions():
         try:
             k = SCons.Util.RegOpenKeyEx(SCons.Util.HKEY_LOCAL_MACHINE,
                                         keyname)
-        except SCons.Util.WinError:
+        except OSError:
             # For version 13 or later, check for default instance UUID
             if is_win64:
                 keyname = 'Software\\WoW6432Node\\Intel\\Suites'
@@ -210,7 +208,7 @@ def get_all_compiler_versions():
             try:
                 k = SCons.Util.RegOpenKeyEx(SCons.Util.HKEY_LOCAL_MACHINE,
                                             keyname)
-            except SCons.Util.WinError:
+            except OSError:
                 return []
         i = 0
         versions = []
diff --git a/SCons/Util.py b/SCons/Util.py
index 4e575d578..5f521b316 100644
--- a/SCons/Util.py
+++ b/SCons/Util.py
@@ -31,7 +31,16 @@ import re
 import sys
 from collections import UserDict, UserList, UserString, OrderedDict
 from collections.abc import MappingView
+from contextlib import suppress
 from types import MethodType, FunctionType
+from typing import Optional, Union
+
+# Note: Util module cannot import other bits of SCons globally without getting
+# into import loops. Both the below modules import SCons.Util early on.
+# --> SCons.Warnings
+# --> SCons.Errors
+# Thus the local imports, which are annotated for pylint to show we mean it.
+
 
 PYPY = hasattr(sys, 'pypy_translation_info')
 
@@ -40,56 +49,64 @@ PYPY = hasattr(sys, 'pypy_translation_info')
 NOFILE = "SCONS_MAGIC_MISSING_FILE_STRING"
 
 # unused?
-def dictify(keys, values, result=None):
+def dictify(keys, values, result=None) -> dict:
     if result is None:
         result = {}
     result.update(dict(zip(keys, values)))
     return result
 
-_altsep = os.altsep
-if _altsep is None and sys.platform == 'win32':
+_ALTSEP = os.altsep
+if _ALTSEP is None and sys.platform == 'win32':
     # My ActivePython 2.0.1 doesn't set os.altsep!  What gives?
-    _altsep = '/'
-if _altsep:
+    _ALTSEP = '/'
+if _ALTSEP:
     def rightmost_separator(path, sep):
-        return max(path.rfind(sep), path.rfind(_altsep))
+        return max(path.rfind(sep), path.rfind(_ALTSEP))
 else:
     def rightmost_separator(path, sep):
         return path.rfind(sep)
 
 # First two from the Python Cookbook, just for completeness.
 # (Yeah, yeah, YAGNI...)
-def containsAny(str, set):
-    """Check whether sequence str contains ANY of the items in set."""
-    for c in set:
-        if c in str: return 1
-    return 0
-
-def containsAll(str, set):
-    """Check whether sequence str contains ALL of the items in set."""
-    for c in set:
-        if c not in str: return 0
-    return 1
-
-def containsOnly(str, set):
-    """Check whether sequence str contains ONLY items in set."""
-    for c in str:
-        if c not in set: return 0
-    return 1
-
-def splitext(path):
-    """Same as os.path.splitext() but faster."""
+def containsAny(s, pat) -> bool:
+    """Check whether string `s` contains ANY of the items in `pat`."""
+    for c in pat:
+        if c in s:
+            return True
+    return False
+
+def containsAll(s, pat) -> bool:
+    """Check whether string `s` contains ALL of the items in `pat`."""
+    for c in pat:
+        if c not in s:
+            return False
+    return True
+
+def containsOnly(s, pat) -> bool:
+    """Check whether string `s` contains ONLY items in `pat`."""
+    for c in s:
+        if c not in pat:
+            return False
+    return True
+
+
+# TODO: Verify this method is STILL faster than os.path.splitext
+def splitext(path) -> tuple:
+    """Split `path` into a (root, ext) pair.
+
+    Same as :mod:`os.path.splitext` but faster.
+    """
     sep = rightmost_separator(path, os.sep)
     dot = path.rfind('.')
     # An ext is only real if it has at least one non-digit char
     if dot > sep and not containsOnly(path[dot:], "0123456789."):
-        return path[:dot],path[dot:]
-    else:
-        return path,""
+        return path[:dot], path[dot:]
+
+    return path, ""
+
+def updrive(path) -> str:
+    """Make the drive letter (if any) upper case.
 
-def updrive(path):
-    """
-    Make the drive letter (if any) upper case.
     This is useful because Windows is inconsistent on the case
     of the drive letter, which can cause inconsistencies when
     calculating command signatures.
@@ -100,33 +117,20 @@ def updrive(path):
     return path
 
 class NodeList(UserList):
-    """This class is almost exactly like a regular list of Nodes
+    """A list of Nodes with special attribute retrieval.
+
+    This class is almost exactly like a regular list of Nodes
     (actually it can hold any object), with one important difference.
     If you try to get an attribute from this list, it will return that
     attribute from every item in the list.  For example:
 
-    >>> someList = NodeList([ '  foo  ', '  bar  ' ])
+    >>> someList = NodeList(['  foo  ', '  bar  '])
     >>> someList.strip()
-    [ 'foo', 'bar' ]
+    ['foo', 'bar']
     """
 
-#     def __init__(self, initlist=None):
-#         self.data = []
-# #        print("TYPE:%s"%type(initlist))
-#         if initlist is not None:
-#             # XXX should this accept an arbitrary sequence?
-#             if type(initlist) == type(self.data):
-#                 self.data[:] = initlist
-#             elif isinstance(initlist, (UserList, NodeList)):
-#                 self.data[:] = initlist.data[:]
-#             elif isinstance(initlist, Iterable):
-#                 self.data = list(initlist)
-#             else:
-#                 self.data = [ initlist,]
-
-
     def __bool__(self):
-        return len(self.data) != 0
+        return bool(self.data)
 
     def __str__(self):
         return ' '.join(map(str, self.data))
@@ -155,38 +159,45 @@ class NodeList(UserList):
             # Expand the slice object using range()
             # limited by number of items in self.data
             indices = index.indices(len(self.data))
-            return self.__class__([self[x] for x in
-                    range(*indices)])
-        else:
-            # Return one item of the tart
-            return self.data[index]
+            return self.__class__([self[x] for x in range(*indices)])
+
+        # Return one item of the tart
+        return self.data[index]
 
 
 _get_env_var = re.compile(r'^\$([_a-zA-Z]\w*|{[_a-zA-Z]\w*})$')
 
-def get_environment_var(varstr):
-    """Given a string, first determine if it looks like a reference
-    to a single environment variable, like "$FOO" or "${FOO}".
-    If so, return that variable with no decorations ("FOO").
-    If not, return None."""
-    mo=_get_env_var.match(to_String(varstr))
+def get_environment_var(varstr) -> Optional[str]:
+    """Return undecorated construction variable string.
+
+    Determine if `varstr` looks like a reference
+    to a single environment variable, like `"$FOO"` or `"${FOO}"`.
+    If so, return that variable with no decorations, like `"FOO"`.
+    If not, return `None`.
+    """
+
+    mo = _get_env_var.match(to_String(varstr))
     if mo:
         var = mo.group(1)
         if var[0] == '{':
             return var[1:-1]
-        else:
-            return var
-    else:
-        return None
+        return var
+
+    return None
 
 
 class DisplayEngine:
+    """A callable class used to display SCons messages."""
+
     print_it = True
 
     def __call__(self, text, append_newline=1):
         if not self.print_it:
             return
-        if append_newline: text = text + '\n'
+
+        if append_newline:
+            text = text + '\n'
+
         try:
             sys.stdout.write(str(text))
         except IOError:
@@ -203,16 +214,18 @@ class DisplayEngine:
         self.print_it = mode
 
 
+# TODO: W0102: Dangerous default value [] as argument (dangerous-default-value)
 def render_tree(root, child_func, prune=0, margin=[0], visited=None):
-    """
-    Render a tree of nodes into an ASCII tree view.
-
-    :Parameters:
-        - `root`:       the root node of the tree
-        - `child_func`: the function called to get the children of a node
-        - `prune`:      don't visit the same node twice
-        - `margin`:     the format of the left margin to use for children of root. 1 results in a pipe, and 0 results in no pipe.
-        - `visited`:    a dictionary of visited nodes in the current branch if not prune, or in the whole tree if prune.
+    """Render a tree of nodes into an ASCII tree view.
+
+    Args:
+        root: the root node of the tree
+        child_func: the function called to get the children of a node
+        prune: don't visit the same node twice
+        margin: the format of the left margin to use for children of `root`.
+          1 results in a pipe, and 0 results in no pipe.
+        visited: a dictionary of visited nodes in the current branch if
+          `prune` is 0, or in the whole tree if `prune` is 1.
     """
 
     rname = str(root)
@@ -235,16 +248,21 @@ def render_tree(root, child_func, prune=0, margin=[0], visited=None):
     retval = retval + "+-" + rname + "\n"
     if not prune:
         visited = copy.copy(visited)
-    visited[rname] = 1
+    visited[rname] = True
 
-    for i in range(len(children)):
+    for i, child in enumerate(children):
         margin.append(i < len(children)-1)
-        retval = retval + render_tree(children[i], child_func, prune, margin, visited)
+        retval = retval + render_tree(child, child_func, prune, margin, visited)
         margin.pop()
 
     return retval
 
-IDX = lambda N: N and 1 or 0
+def IDX(n) -> bool:
+    """Generate in index into strings from the tree legends.
+
+    These are always a choice between two, so bool works fine.
+    """
+    return bool(n)
 
 # unicode line drawing chars:
 BOX_HORIZ = chr(0x2500)  # '─'
@@ -257,25 +275,37 @@ BOX_VERT_RIGHT = chr(0x251c)  # '├'
 BOX_HORIZ_DOWN = chr(0x252c)  # '┬'
 
 
-def print_tree(root, child_func, prune=0, showtags=0, margin=[0], visited=None, lastChild=False, singleLineDraw=False):
-    """
-    Print a tree of nodes.  This is like render_tree, except it prints
-    lines directly instead of creating a string representation in memory,
-    so that huge trees can be printed.
-
-    :Parameters:
-        - `root`       - the root node of the tree
-        - `child_func` - the function called to get the children of a node
-        - `prune`      - don't visit the same node twice
-        - `showtags`   - print status information to the left of each node line
-        - `margin`     - the format of the left margin to use for children of root. 1 results in a pipe, and 0 results in no pipe.
-        - `visited`    - a dictionary of visited nodes in the current branch if not prune, or in the whole tree if prune.
-        - `singleLineDraw` - use line-drawing characters rather than ASCII.
+# TODO: W0102: Dangerous default value [] as argument (dangerous-default-value)
+def print_tree(
+    root,
+    child_func,
+    prune=0,
+    showtags=False,
+    margin=[0],
+    visited=None,
+    lastChild=False,
+    singleLineDraw=False,
+):
+    """Print a tree of nodes.
+
+    This is like func:`render_tree`, except it prints lines directly instead
+    of creating a string representation in memory, so that huge trees can
+    be handled.
+
+    Args:
+        root: the root node of the tree
+        child_func: the function called to get the children of a node
+        prune: don't visit the same node twice
+        showtags: print status information to the left of each node line
+        margin: the format of the left margin to use for children of `root`.
+          1 results in a pipe, and 0 results in no pipe.
+        visited: a dictionary of visited nodes in the current branch if
+          prune` is 0, or in the whole tree if `prune` is 1.
+        singleLineDraw: use line-drawing characters rather than ASCII.
     """
 
     rname = str(root)
 
-
     # Initialize 'visited' dict, if required
     if visited is None:
         visited = {}
@@ -319,14 +349,11 @@ def print_tree(root, child_func, prune=0, showtags=0, margin=[0], visited=None,
     def MMM(m):
         if singleLineDraw:
             return ["  ", BOX_VERT + " "][m]
-        else:
-            return ["  ", "| "][m]
 
-    margins = list(map(MMM, margin[:-1]))
+        return ["  ", "| "][m]
 
+    margins = list(map(MMM, margin[:-1]))
     children = child_func(root)
-
-
     cross = "+-"
     if singleLineDraw:
         cross = BOX_VERT_RIGHT + BOX_HORIZ   # sign used to point to the leaf.
@@ -353,15 +380,26 @@ def print_tree(root, child_func, prune=0, showtags=0, margin=[0], visited=None,
 
     # if this item has children:
     if children:
-        margin.append(1) # Initialize margin with 1 for vertical bar.
+        margin.append(1)  # Initialize margin with 1 for vertical bar.
         idx = IDX(showtags)
-        _child = 0 # Initialize this for the first child.
+        _child = 0  # Initialize this for the first child.
         for C in children[:-1]:
-            _child = _child + 1 # number the children
-            print_tree(C, child_func, prune, idx, margin, visited, (len(children) - _child) <= 0 ,singleLineDraw)
-        margin[-1] = 0  # margins are with space (index 0) because we arrived to the last child.
-        print_tree(children[-1], child_func, prune, idx, margin, visited, True ,singleLineDraw) # for this call child and nr of children needs to be set 0, to signal the second phase.
-        margin.pop() # destroy the last margin added
+            _child = _child + 1  # number the children
+            print_tree(
+                C,
+                child_func,
+                prune,
+                idx,
+                margin,
+                visited,
+                (len(children) - _child) <= 0,
+                singleLineDraw,
+            )
+        # margins are with space (index 0) because we arrived to the last child.
+        margin[-1] = 0
+        # for this call child and nr of children needs to be set 0, to signal the second phase.
+        print_tree(children[-1], child_func, prune, idx, margin, visited, True, singleLineDraw)
+        margin.pop()  # destroy the last margin added
 
 
 # Functions for deciding if things are like various types, mainly to
@@ -377,6 +415,9 @@ def print_tree(root, child_func, prune=0, showtags=0, margin=[0], visited=None,
 # the global functions and constants used by these functions. This
 # transforms accesses to global variable into local variables
 # accesses (i.e. LOAD_FAST instead of LOAD_GLOBAL).
+# Since checkers dislike this, it's now annotated for pylint to flag
+# (mostly for other readers of this code) we're doing this intentionally.
+# TODO: PY3 check these are still valid choices for all of these funcs.
 
 DictTypes = (dict, UserDict)
 ListTypes = (list, UserList)
@@ -384,31 +425,49 @@ ListTypes = (list, UserList)
 # Handle getting dictionary views.
 SequenceTypes = (list, tuple, UserList, MappingView)
 
-# TODO: PY3 check this benchmarking is still correct.
 # Note that profiling data shows a speed-up when comparing
 # explicitly with str instead of simply comparing
 # with basestring. (at least on Python 2.5.1)
+# TODO: PY3 check this benchmarking is still correct.
 StringTypes = (str, UserString)
 
 # Empirically, it is faster to check explicitly for str than for basestring.
 BaseStringTypes = str
 
-def is_Dict(obj, isinstance=isinstance, DictTypes=DictTypes):
+def is_Dict(  # pylint: disable=redefined-outer-name,redefined-builtin
+    obj, isinstance=isinstance, DictTypes=DictTypes
+) -> bool:
     return isinstance(obj, DictTypes)
 
-def is_List(obj, isinstance=isinstance, ListTypes=ListTypes):
+
+def is_List(  # pylint: disable=redefined-outer-name,redefined-builtin
+    obj, isinstance=isinstance, ListTypes=ListTypes
+) -> bool:
     return isinstance(obj, ListTypes)
 
-def is_Sequence(obj, isinstance=isinstance, SequenceTypes=SequenceTypes):
+
+def is_Sequence(  # pylint: disable=redefined-outer-name,redefined-builtin
+    obj, isinstance=isinstance, SequenceTypes=SequenceTypes
+) -> bool:
     return isinstance(obj, SequenceTypes)
 
-def is_Tuple(obj, isinstance=isinstance, tuple=tuple):
+
+def is_Tuple(  # pylint: disable=redefined-builtin
+    obj, isinstance=isinstance, tuple=tuple
+) -> bool:
     return isinstance(obj, tuple)
 
-def is_String(obj, isinstance=isinstance, StringTypes=StringTypes):
+
+def is_String(  # pylint: disable=redefined-outer-name,redefined-builtin
+    obj, isinstance=isinstance, StringTypes=StringTypes
+) -> bool:
     return isinstance(obj, StringTypes)
 
-def is_Scalar(obj, isinstance=isinstance, StringTypes=StringTypes, SequenceTypes=SequenceTypes):
+
+def is_Scalar(  # pylint: disable=redefined-outer-name,redefined-builtin
+    obj, isinstance=isinstance, StringTypes=StringTypes, SequenceTypes=SequenceTypes
+) -> bool:
+
     # Profiling shows that there is an impressive speed-up of 2x
     # when explicitly checking for strings instead of just not
     # sequence when the argument (i.e. obj) is already a string.
@@ -417,21 +476,33 @@ def is_Scalar(obj, isinstance=isinstance, StringTypes=StringTypes, SequenceTypes
     # assumes that the obj argument is a string most of the time.
     return isinstance(obj, StringTypes) or not isinstance(obj, SequenceTypes)
 
-def do_flatten(sequence, result, isinstance=isinstance,
-               StringTypes=StringTypes, SequenceTypes=SequenceTypes):
+
+def do_flatten(
+    sequence,
+    result,
+    isinstance=isinstance,
+    StringTypes=StringTypes,
+    SequenceTypes=SequenceTypes,
+):  # pylint: disable=redefined-outer-name,redefined-builtin
     for item in sequence:
         if isinstance(item, StringTypes) or not isinstance(item, SequenceTypes):
             result.append(item)
         else:
             do_flatten(item, result)
 
-def flatten(obj, isinstance=isinstance, StringTypes=StringTypes,
-            SequenceTypes=SequenceTypes, do_flatten=do_flatten):
+
+def flatten(  # pylint: disable=redefined-outer-name,redefined-builtin
+    obj,
+    isinstance=isinstance,
+    StringTypes=StringTypes,
+    SequenceTypes=SequenceTypes,
+    do_flatten=do_flatten,
+) -> list:
     """Flatten a sequence to a non-nested list.
 
-    Flatten() converts either a single scalar or a nested sequence
-    to a non-nested list. Note that flatten() considers strings
-    to be scalars instead of sequences like Python would.
+    Converts either a single scalar or a nested sequence to a non-nested list.
+    Note that :func:`flatten` considers strings
+    to be scalars instead of sequences like pure Python would.
     """
     if isinstance(obj, StringTypes) or not isinstance(obj, SequenceTypes):
         return [obj]
@@ -443,13 +514,19 @@ def flatten(obj, isinstance=isinstance, StringTypes=StringTypes,
             do_flatten(item, result)
     return result
 
-def flatten_sequence(sequence, isinstance=isinstance, StringTypes=StringTypes,
-                     SequenceTypes=SequenceTypes, do_flatten=do_flatten):
+
+def flatten_sequence(  # pylint: disable=redefined-outer-name,redefined-builtin
+    sequence,
+    isinstance=isinstance,
+    StringTypes=StringTypes,
+    SequenceTypes=SequenceTypes,
+    do_flatten=do_flatten,
+) -> list:
     """Flatten a sequence to a non-nested list.
 
-    Same as flatten(), but it does not handle the single scalar
-    case. This is slightly more efficient when one knows that
-    the sequence to flatten can not be a scalar.
+    Same as :func:`flatten`, but it does not handle the single scalar case.
+    This is slightly more efficient when one knows that the sequence
+    to flatten can not be a scalar.
     """
     result = []
     for item in sequence:
@@ -462,41 +539,58 @@ def flatten_sequence(sequence, isinstance=isinstance, StringTypes=StringTypes,
 # Generic convert-to-string functions.  The wrapper
 # to_String_for_signature() will use a for_signature() method if the
 # specified object has one.
-#
 
+def to_String(  # pylint: disable=redefined-outer-name,redefined-builtin
+    obj,
+    isinstance=isinstance,
+    str=str,
+    UserString=UserString,
+    BaseStringTypes=BaseStringTypes,
+) -> str:
+    """Return a string version of obj."""
 
-def to_String(s,
-              isinstance=isinstance, str=str,
-              UserString=UserString, BaseStringTypes=BaseStringTypes):
-    if isinstance(s, BaseStringTypes):
+    if isinstance(obj, BaseStringTypes):
         # Early out when already a string!
-        return s
-    elif isinstance(s, UserString):
-        # s.data can only be a regular string. Please see the UserString initializer.
-        return s.data
-    else:
-        return str(s)
+        return obj
+
+    if isinstance(obj, UserString):
+        # obj.data can only be a regular string. Please see the UserString initializer.
+        return obj.data
 
+    return str(obj)
 
-def to_String_for_subst(s,
-                        isinstance=isinstance, str=str, to_String=to_String,
-                        BaseStringTypes=BaseStringTypes, SequenceTypes=SequenceTypes,
-                        UserString=UserString):
+def to_String_for_subst(  # pylint: disable=redefined-outer-name,redefined-builtin
+    obj,
+    isinstance=isinstance,
+    str=str,
+    BaseStringTypes=BaseStringTypes,
+    SequenceTypes=SequenceTypes,
+    UserString=UserString,
+) -> str:
+    """Return a string version of obj for subst usage."""
 
     # Note that the test cases are sorted by order of probability.
-    if isinstance(s, BaseStringTypes):
-        return s
-    elif isinstance(s, SequenceTypes):
-        return ' '.join([to_String_for_subst(e) for e in s])
-    elif isinstance(s, UserString):
-        # s.data can only a regular string. Please see the UserString initializer.
-        return s.data
-    else:
-        return str(s)
+    if isinstance(obj, BaseStringTypes):
+        return obj
+
+    if isinstance(obj, SequenceTypes):
+        return ' '.join([to_String_for_subst(e) for e in obj])
+
+    if isinstance(obj, UserString):
+        # obj.data can only a regular string. Please see the UserString initializer.
+        return obj.data
 
+    return str(obj)
+
+def to_String_for_signature(  # pylint: disable=redefined-outer-name,redefined-builtin
+    obj, to_String_for_subst=to_String_for_subst, AttributeError=AttributeError
+) -> str:
+    """Return a string version of obj for signature usage.
+
+    Like :func:`to_String_for_subst` but has special handling for
+    scons objects that have a :meth:`for_signature` method, and for dicts.
+    """
 
-def to_String_for_signature(obj, to_String_for_subst=to_String_for_subst,
-                            AttributeError=AttributeError):
     try:
         f = obj.for_signature
     except AttributeError:
@@ -506,8 +600,7 @@ def to_String_for_signature(obj, to_String_for_subst=to_String_for_subst,
             # which was undefined until py3.6 (where it's by insertion order) was not wise.
             # TODO: Change code when floor is raised to PY36
             return pprint.pformat(obj, width=1000000)
-        else:
-            return to_String_for_subst(obj)
+        return to_String_for_subst(obj)
     else:
         return f()
 
@@ -525,71 +618,65 @@ def to_String_for_signature(obj, to_String_for_subst=to_String_for_subst,
 # The dispatch table approach used here is a direct rip-off from the
 # normal Python copy module.
 
-_semi_deepcopy_dispatch = d = {}
-
-def semi_deepcopy_dict(x, exclude = [] ):
-    copy = {}
-    for key, val in x.items():
-        # The regular Python copy.deepcopy() also deepcopies the key,
-        # as follows:
-        #
-        #    copy[semi_deepcopy(key)] = semi_deepcopy(val)
-        #
-        # Doesn't seem like we need to, but we'll comment it just in case.
-        if key not in exclude:
-            copy[key] = semi_deepcopy(val)
-    return copy
-d[dict] = semi_deepcopy_dict
-
-def _semi_deepcopy_list(x):
-    return list(map(semi_deepcopy, x))
-d[list] = _semi_deepcopy_list
-
-def _semi_deepcopy_tuple(x):
-    return tuple(map(semi_deepcopy, x))
-d[tuple] = _semi_deepcopy_tuple
-
-def semi_deepcopy(x):
-    copier = _semi_deepcopy_dispatch.get(type(x))
+def semi_deepcopy_dict(obj, exclude=None) -> dict:
+    if exclude is None:
+        exclude = []
+    return {k: semi_deepcopy(v) for k, v in obj.items() if k not in exclude}
+
+def _semi_deepcopy_list(obj) -> list:
+    return [semi_deepcopy(item) for item in obj]
+
+def _semi_deepcopy_tuple(obj) -> tuple:
+    return tuple(map(semi_deepcopy, obj))
+
+_semi_deepcopy_dispatch = {
+    dict: semi_deepcopy_dict,
+    list: _semi_deepcopy_list,
+    tuple: _semi_deepcopy_tuple,
+}
+
+def semi_deepcopy(obj):
+    copier = _semi_deepcopy_dispatch.get(type(obj))
     if copier:
-        return copier(x)
-    else:
-        if hasattr(x, '__semi_deepcopy__') and callable(x.__semi_deepcopy__):
-            return x.__semi_deepcopy__()
-        elif isinstance(x, UserDict):
-            return x.__class__(semi_deepcopy_dict(x))
-        elif isinstance(x, UserList):
-            return x.__class__(_semi_deepcopy_list(x))
+        return copier(obj)
 
-        return x
+    if hasattr(obj, '__semi_deepcopy__') and callable(obj.__semi_deepcopy__):
+        return obj.__semi_deepcopy__()
+
+    if isinstance(obj, UserDict):
+        return obj.__class__(semi_deepcopy_dict(obj))
+
+    if isinstance(obj, UserList):
+        return obj.__class__(_semi_deepcopy_list(obj))
+
+    return obj
 
 
 class Proxy:
-    """A simple generic Proxy class, forwarding all calls to
-    subject.  So, for the benefit of the python newbie, what does
-    this really mean?  Well, it means that you can take an object, let's
-    call it 'objA', and wrap it in this Proxy class, with a statement
-    like this
+    """A simple generic Proxy class, forwarding all calls to subject.
 
-                 proxyObj = Proxy(objA),
+    This means you can take an object, let's call it `'obj_a`,
+    and wrap it in this Proxy class, with a statement like this::
 
-    Then, if in the future, you do something like this
+        proxy_obj = Proxy(obj_a)
 
-                 x = proxyObj.var1,
+    Then, if in the future, you do something like this::
 
-    since Proxy does not have a 'var1' attribute (but presumably objA does),
-    the request actually is equivalent to saying
+        x = proxy_obj.var1
 
-                 x = objA.var1
+    since the :class:`Proxy` class does not have a :attr:`var1` attribute
+    (but presumably `objA` does), the request actually is equivalent to saying::
+
+        x = obj_a.var1
 
     Inherit from this class to create a Proxy.
 
-    Note that, with new-style classes, this does *not* work transparently
-    for Proxy subclasses that use special .__*__() method names, because
-    those names are now bound to the class, not the individual instances.
-    You now need to know in advance which .__*__() method names you want
-    to pass on to the underlying Proxy object, and specifically delegate
-    their calls like this:
+    With Python 3.5+ this does *not* work transparently
+    for :class:`Proxy` subclasses that use special .__*__() method names,
+    because those names are now bound to the class, not the individual
+    instances.  You now need to know in advance which special method names you
+    want to pass on to the underlying Proxy object, and specifically delegate
+    their calls like this::
 
         class Foo(Proxy):
             __str__ = Delegate('__str__')
@@ -600,8 +687,11 @@ class Proxy:
         self._subject = subject
 
     def __getattr__(self, name):
-        """Retrieve an attribute from the wrapped object.  If the named
-           attribute doesn't exist, AttributeError is raised"""
+        """Retrieve an attribute from the wrapped object.
+
+        Raises:
+           AttributeError: if attribute `name` doesn't exist.
+        """
         return getattr(self._subject, name)
 
     def get(self):
@@ -616,7 +706,7 @@ class Proxy:
 
 class Delegate:
     """A Python Descriptor class that delegates attribute fetches
-    to an underlying wrapped subject of a Proxy.  Typical use:
+    to an underlying wrapped subject of a Proxy.  Typical use::
 
         class Foo(Proxy):
             __str__ = Delegate('__str__')
@@ -627,8 +717,8 @@ class Delegate:
     def __get__(self, obj, cls):
         if isinstance(obj, cls):
             return getattr(obj._subject, self.attribute)
-        else:
-            return self
+
+        return self
 
 
 class MethodWrapper:
@@ -637,7 +727,7 @@ class MethodWrapper:
     As part of creating this MethodWrapper object an attribute with the
     specified name (by default, the name of the supplied method) is added
     to the underlying object.  When that new "method" is called, our
-    __call__() method adds the object as the first argument, simulating
+    :meth:`__call__` method adds the object as the first argument, simulating
     the Python behavior of supplying "self" on method calls.
 
     We hang on to the name by which the method was added to the underlying
@@ -645,10 +735,10 @@ class MethodWrapper:
     a new underlying object being copied (without which we wouldn't need
     to save that info).
     """
-    def __init__(self, object, method, name=None):
+    def __init__(self, obj, method, name=None):
         if name is None:
             name = method.__name__
-        self.object = object
+        self.object = obj
         self.method = method
         self.name = name
         setattr(self.object, name, self)
@@ -666,71 +756,58 @@ class MethodWrapper:
 
 
 # attempt to load the windows registry module:
-can_read_reg = 0
+can_read_reg = False
 try:
     import winreg
 
-    can_read_reg = 1
+    can_read_reg = True
     hkey_mod = winreg
 
-    RegOpenKeyEx    = winreg.OpenKeyEx
-    RegEnumKey      = winreg.EnumKey
-    RegEnumValue    = winreg.EnumValue
-    RegQueryValueEx = winreg.QueryValueEx
-    RegError        = winreg.error
-
 except ImportError:
     class _NoError(Exception):
         pass
     RegError = _NoError
 
+if can_read_reg:
+    HKEY_CLASSES_ROOT = hkey_mod.HKEY_CLASSES_ROOT
+    HKEY_LOCAL_MACHINE = hkey_mod.HKEY_LOCAL_MACHINE
+    HKEY_CURRENT_USER = hkey_mod.HKEY_CURRENT_USER
+    HKEY_USERS = hkey_mod.HKEY_USERS
 
-# Make sure we have a definition of WindowsError so we can
-# run platform-independent tests of Windows functionality on
-# platforms other than Windows.  (WindowsError is, in fact, an
-# OSError subclass on Windows.)
-
-class PlainWindowsError(OSError):
-    pass
+    RegOpenKeyEx = winreg.OpenKeyEx
+    RegEnumKey = winreg.EnumKey
+    RegEnumValue = winreg.EnumValue
+    RegQueryValueEx = winreg.QueryValueEx
+    RegError = winreg.error
 
-try:
-    WinError = WindowsError
-except NameError:
-    WinError = PlainWindowsError
+    def RegGetValue(root, key):
+        r"""Returns a registry value without having to open the key first.
 
+        Only available on Windows platforms with a version of Python that
+        can read the registry.
 
-if can_read_reg:
-    HKEY_CLASSES_ROOT  = hkey_mod.HKEY_CLASSES_ROOT
-    HKEY_LOCAL_MACHINE = hkey_mod.HKEY_LOCAL_MACHINE
-    HKEY_CURRENT_USER  = hkey_mod.HKEY_CURRENT_USER
-    HKEY_USERS         = hkey_mod.HKEY_USERS
+        Returns the same thing as :func:`RegQueryValueEx`, except you just
+        specify the entire path to the value, and don't have to bother
+        opening the key first.  So, instead of::
 
-    def RegGetValue(root, key):
-        r"""This utility function returns a value in the registry
-        without having to open the key first.  Only available on
-        Windows platforms with a version of Python that can read the
-        registry.  Returns the same thing as
-        SCons.Util.RegQueryValueEx, except you just specify the entire
-        path to the value, and don't have to bother opening the key
-        first.  So:
-
-        Instead of:
           k = SCons.Util.RegOpenKeyEx(SCons.Util.HKEY_LOCAL_MACHINE,
                 r'SOFTWARE\Microsoft\Windows\CurrentVersion')
-          out = SCons.Util.RegQueryValueEx(k,
-                'ProgramFilesDir')
+          out = SCons.Util.RegQueryValueEx(k, 'ProgramFilesDir')
+
+        You can write::
 
-        You can write:
           out = SCons.Util.RegGetValue(SCons.Util.HKEY_LOCAL_MACHINE,
                 r'SOFTWARE\Microsoft\Windows\CurrentVersion\ProgramFilesDir')
         """
         # I would use os.path.split here, but it's not a filesystem
         # path...
         p = key.rfind('\\') + 1
-        keyp = key[:p-1]          # -1 to omit trailing slash
+        keyp = key[: p - 1]  # -1 to omit trailing slash
         val = key[p:]
         k = RegOpenKeyEx(root, keyp)
-        return RegQueryValueEx(k,val)
+        return RegQueryValueEx(k, val)
+
+
 else:
     HKEY_CLASSES_ROOT = None
     HKEY_LOCAL_MACHINE = None
@@ -738,14 +815,15 @@ else:
     HKEY_USERS = None
 
     def RegGetValue(root, key):
-        raise WinError
+        raise OSError
 
     def RegOpenKeyEx(root, key):
-        raise WinError
+        raise OSError
+
 
 if sys.platform == 'win32':
 
-    def WhereIs(file, path=None, pathext=None, reject=None):
+    def WhereIs(file, path=None, pathext=None, reject=None) -> Optional[str]:
         if path is None:
             try:
                 path = os.environ['PATH']
@@ -768,8 +846,8 @@ if sys.platform == 'win32':
             reject = []
         if not is_List(reject) and not is_Tuple(reject):
             reject = [reject]
-        for dir in path:
-            f = os.path.join(dir, file)
+        for p in path:
+            f = os.path.join(p, file)
             for ext in pathext:
                 fext = f + ext
                 if os.path.isfile(fext):
@@ -782,7 +860,7 @@ if sys.platform == 'win32':
 
 elif os.name == 'os2':
 
-    def WhereIs(file, path=None, pathext=None, reject=None):
+    def WhereIs(file, path=None, pathext=None, reject=None) -> Optional[str]:
         if path is None:
             try:
                 path = os.environ['PATH']
@@ -800,8 +878,8 @@ elif os.name == 'os2':
             reject = []
         if not is_List(reject) and not is_Tuple(reject):
             reject = [reject]
-        for dir in path:
-            f = os.path.join(dir, file)
+        for p in path:
+            f = os.path.join(p, file)
             for ext in pathext:
                 fext = f + ext
                 if os.path.isfile(fext):
@@ -814,8 +892,9 @@ elif os.name == 'os2':
 
 else:
 
-    def WhereIs(file, path=None, pathext=None, reject=None):
-        import stat
+    def WhereIs(file, path=None, pathext=None, reject=None) -> Optional[str]:
+        import stat  # pylint: disable=import-outside-toplevel
+
         if path is None:
             try:
                 path = os.environ['PATH']
@@ -827,8 +906,8 @@ else:
             reject = []
         if not is_List(reject) and not is_Tuple(reject):
             reject = [reject]
-        for d in path:
-            f = os.path.join(d, file)
+        for p in path:
+            f = os.path.join(p, file)
             if os.path.isfile(f):
                 try:
                     st = os.stat(f)
@@ -846,35 +925,53 @@ else:
                     continue
         return None
 
-def PrependPath(oldpath, newpath, sep = os.pathsep,
-                delete_existing=1, canonicalize=None):
-    """This prepends newpath elements to the given oldpath.  Will only
-    add any particular path once (leaving the first one it encounters
-    and ignoring the rest, to preserve path order), and will
-    os.path.normpath and os.path.normcase all paths to help assure
-    this.  This can also handle the case where the given old path
-    variable is a list instead of a string, in which case a list will
-    be returned instead of a string.
-
-    Example:
-      Old Path: "/foo/bar:/foo"
-      New Path: "/biz/boom:/foo"
-      Result:   "/biz/boom:/foo:/foo/bar"
-
-    If delete_existing is 0, then adding a path that exists will
-    not move it to the beginning; it will stay where it is in the
-    list.
-
-    If canonicalize is not None, it is applied to each element of
-    newpath before use.
+WhereIs.__doc__ = """\
+Return the path to an executable that matches `file`.
+
+Searches the given `path` for `file`, respecting any filename
+extensions `pathext` (on the Windows platform only), and
+returns the full path to the matching command.  If no
+command is found, return ``None``.
+
+If `path` is not specified, :attr:`os.environ[PATH]` is used.
+If `pathext` is not specified, :attr:`os.environ[PATHEXT]`
+is used. Will not select any path name or names in the optional
+`reject` list.
+"""
+
+def PrependPath(
+    oldpath, newpath, sep=os.pathsep, delete_existing=True, canonicalize=None
+) -> Union[list, str]:
+    """Prepends `newpath` path elements to `oldpath`.
+
+    Will only add any particular path once (leaving the first one it
+    encounters and ignoring the rest, to preserve path order), and will
+    :mod:`os.path.normpath` and :mod:`os.path.normcase` all paths to help
+    assure this.  This can also handle the case where `oldpath`
+    is a list instead of a string, in which case a list will be returned
+    instead of a string. For example:
+
+    >>> p = PrependPath("/foo/bar:/foo", "/biz/boom:/foo")
+    >>> print(p)
+    /biz/boom:/foo:/foo/bar
+
+    If `delete_existing` is ``False``, then adding a path that exists will
+    not move it to the beginning; it will stay where it is in the list.
+
+    >>> p = PrependPath("/foo/bar:/foo", "/biz/boom:/foo", delete_existing=False)
+    >>> print(p)
+    /biz/boom:/foo/bar:/foo
+
+    If `canonicalize` is not ``None``, it is applied to each element of
+    `newpath` before use.
     """
 
     orig = oldpath
-    is_list = 1
+    is_list = True
     paths = orig
     if not is_List(orig) and not is_Tuple(orig):
         paths = paths.split(sep)
-        is_list = 0
+        is_list = False
 
     if is_String(newpath):
         newpaths = newpath.split(sep)
@@ -925,42 +1022,47 @@ def PrependPath(oldpath, newpath, sep = os.pathsep,
 
     if is_list:
         return paths
-    else:
-        return sep.join(paths)
-
-def AppendPath(oldpath, newpath, sep = os.pathsep,
-               delete_existing=1, canonicalize=None):
-    """This appends new path elements to the given old path.  Will
-    only add any particular path once (leaving the last one it
-    encounters and ignoring the rest, to preserve path order), and
-    will os.path.normpath and os.path.normcase all paths to help
-    assure this.  This can also handle the case where the given old
-    path variable is a list instead of a string, in which case a list
-    will be returned instead of a string.
 
-    Example:
-      Old Path: "/foo/bar:/foo"
-      New Path: "/biz/boom:/foo"
-      Result:   "/foo/bar:/biz/boom:/foo"
+    return sep.join(paths)
 
-    If delete_existing is 0, then adding a path that exists
+def AppendPath(
+    oldpath, newpath, sep=os.pathsep, delete_existing=True, canonicalize=None
+) -> Union[list, str]:
+    """Appends `newpath` path elements to `oldpath`.
+
+    Will only add any particular path once (leaving the last one it
+    encounters and ignoring the rest, to preserve path order), and will
+    :mod:`os.path.normpath` and :mod:`os.path.normcase` all paths to help
+    assure this.  This can also handle the case where `oldpath`
+    is a list instead of a string, in which case a list will be returned
+    instead of a string. For example:
+
+    >>> p = AppendPath("/foo/bar:/foo", "/biz/boom:/foo")
+    >>> print(p)
+    /foo/bar:/biz/boom:/foo
+
+    If `delete_existing` is ``False``, then adding a path that exists
     will not move it to the end; it will stay where it is in the list.
 
-    If canonicalize is not None, it is applied to each element of
-    newpath before use.
+    >>> p = AppendPath("/foo/bar:/foo", "/biz/boom:/foo", delete_existing=False)
+    >>> print(p)
+    /foo/bar:/foo:/biz/boom
+
+    If `canonicalize` is not ``None``, it is applied to each element of
+    `newpath` before use.
     """
 
     orig = oldpath
-    is_list = 1
+    is_list = True
     paths = orig
     if not is_List(orig) and not is_Tuple(orig):
         paths = paths.split(sep)
-        is_list = 0
+        is_list = False
 
     if is_String(newpath):
         newpaths = newpath.split(sep)
     elif not is_List(newpath) and not is_Tuple(newpath):
-        newpaths = [ newpath ]  # might be a Dir
+        newpaths = [newpath]  # might be a Dir
     else:
         newpaths = newpath
 
@@ -1006,22 +1108,29 @@ def AppendPath(oldpath, newpath, sep = os.pathsep,
 
     if is_list:
         return paths
-    else:
-        return sep.join(paths)
+
+    return sep.join(paths)
 
 def AddPathIfNotExists(env_dict, key, path, sep=os.pathsep):
-    """This function will take 'key' out of the dictionary
-    'env_dict', then add the path 'path' to that key if it is not
-    already there.  This treats the value of env_dict[key] as if it
-    has a similar format to the PATH variable...a list of paths
-    separated by tokens.  The 'path' will get added to the list if it
-    is not already there."""
+    """Add a path element to a construction variable.
+
+    `key` is looked up in `env_dict`, and `path` is added to it if it
+    is not already present. `env_dict[key]` is assumed to be in the
+    format of a PATH variable: a list of paths separated by `sep` tokens.
+    Example:
+
+    >>> env = {'PATH': '/bin:/usr/bin:/usr/local/bin'}
+    >>> AddPathIfNotExists(env, 'PATH', '/opt/bin')
+    >>> print(env['PATH'])
+    /opt/bin:/bin:/usr/bin:/usr/local/bin
+    """
+
     try:
-        is_list = 1
+        is_list = True
         paths = env_dict[key]
         if not is_List(env_dict[key]):
             paths = paths.split(sep)
-            is_list = 0
+            is_list = False
         if os.path.normcase(path) not in list(map(os.path.normcase, paths)):
             paths = [ path ] + paths
         if is_list:
@@ -1032,50 +1141,79 @@ def AddPathIfNotExists(env_dict, key, path, sep=os.pathsep):
         env_dict[key] = path
 
 if sys.platform == 'cygwin':
-    def get_native_path(path):
-        """Transforms an absolute path into a native path for the system.  In
-        Cygwin, this converts from a Cygwin path to a Windows one."""
-        with os.popen('cygpath -w ' + path) as p:
-            npath = p.read().replace('\n', '')
-        return npath
+    import subprocess  # pylint: disable=import-outside-toplevel
+
+    def get_native_path(path) -> str:
+        cp = subprocess.run(('cygpath', '-w', path), check=False, stdout=subprocess.PIPE)
+        return cp.stdout.decode().replace('\n', '')
 else:
-    def get_native_path(path):
-        """Transforms an absolute path into a native path for the system.
-        Non-Cygwin version, just leave the path alone."""
+    def get_native_path(path) -> str:
         return path
 
+get_native_path.__doc__ = """\
+Transform an absolute path into a native path for the system.
+
+In Cygwin, this converts from a Cygwin path to a Windows path,
+without regard to whether `path` refers to an existing file
+system object.  For other platforms, `path` is unchanged.
+"""
+
+
 display = DisplayEngine()
 
-def Split(arg):
+def Split(arg) -> list:
+    """Returns a list of file names or other objects.
+
+    If `arg` is a string, it will be split on strings of white-space
+    characters within the string.  If `arg` is already a list, the list
+    will be returned untouched. If `arg` is any other type of object,
+    it will be returned as a list containing just the object.
+
+    >>> print(Split(" this  is  a  string  "))
+    ['this', 'is', 'a', 'string']
+    >>> print(Split(["stringlist", " preserving ", " spaces "]))
+    ['stringlist', ' preserving ', ' spaces ']
+    """
     if is_List(arg) or is_Tuple(arg):
         return arg
-    elif is_String(arg):
+
+    if is_String(arg):
         return arg.split()
-    else:
-        return [arg]
+
+    return [arg]
 
 
 class CLVar(UserList):
-    """A class for command-line construction variables.
-
-    Forces the use of a list of strings, matching individual arguments
-    that will be issued on the command line.  Like UserList,
-    but the argument passed to __init__ will be processed by the
-    Split function, which includes special handling for string types -
-    they will be split into a list of words, not coereced directly
-    to a list.  The same happens if adding a string,
-    which allows us to Do the Right Thing with Append() and
-    Prepend() (as well as straight Python foo = env['VAR'] + 'arg1
-    arg2') regardless of whether a user adds a list or a string to a
-    command-line construction variable.
+    """A container for command-line construction variables.
+
+    Forces the use of a list of strings intended as command-line
+    arguments.  Like :class:`collections.UserList`, but the argument
+    passed to the initializter will be processed by the :func:`Split`
+    function, which includes special handling for string types: they
+    will be split into a list of words, not coereced directly to a list.
+    The same happens if a string is added to a :class:`CLVar`,
+    which allows doing the right thing with both
+    :func:`Append`/:func:`Prepend` methods,
+    as well as with pure Python addition, regardless of whether adding
+    a list or a string to a construction variable.
 
     Side effect: spaces will be stripped from individual string
     arguments. If you need spaces preserved, pass strings containing
     spaces inside a list argument.
+
+    >>> u = UserList("--some --opts and args")
+    >>> print(len(u), repr(u))
+    22 ['-', '-', 's', 'o', 'm', 'e', ' ', '-', '-', 'o', 'p', 't', 's', ' ', 'a', 'n', 'd', ' ', 'a', 'r', 'g', 's']
+    >>> c = CLVar("--some --opts and args")
+    >>> print(len(c), repr(c))
+    4 ['--some', '--opts', 'and', 'args']
+    >>> c += "    strips spaces    "
+    >>> print(len(c), repr(c))
+    6 ['--some', '--opts', 'and', 'args', 'strips', 'spaces']
     """
 
-    def __init__(self, seq=[]):
-        super().__init__(Split(seq))
+    def __init__(self, initlist=None):
+        super().__init__(Split(initlist))
 
     def __add__(self, other):
         return super().__add__(CLVar(other))
@@ -1093,7 +1231,8 @@ class CLVar(UserList):
 class Selector(OrderedDict):
     """A callable ordered dictionary that maps file suffixes to
     dictionary values.  We preserve the order in which items are added
-    so that get_suffix() calls always return the first suffix added."""
+    so that :func:`get_suffix` calls always return the first suffix added.
+    """
     def __call__(self, env, source, ext=None):
         if ext is None:
             try:
@@ -1128,32 +1267,34 @@ class Selector(OrderedDict):
 if sys.platform == 'cygwin':
     # On Cygwin, os.path.normcase() lies, so just report back the
     # fact that the underlying Windows OS is case-insensitive.
-    def case_sensitive_suffixes(s1, s2):
-        return 0
+    def case_sensitive_suffixes(s1, s2) -> bool:  # pylint: disable=unused-argument
+        return False
+
 else:
-    def case_sensitive_suffixes(s1, s2):
-        return (os.path.normcase(s1) != os.path.normcase(s2))
+    def case_sensitive_suffixes(s1, s2) -> bool:
+        return os.path.normcase(s1) != os.path.normcase(s2)
 
 
-def adjustixes(fname, pre, suf, ensure_suffix=False):
-    """
-    Add prefix to file if specified.
-    Add suffix to file if specified and if ensure_suffix = True
+def adjustixes(fname, pre, suf, ensure_suffix=False) -> str:
+    """Adjust filename prefixes and suffixes as needed.
 
+    Add `prefix` to `fname` if specified.
+    Add `suffix` to `fname` if specified and if `ensure_suffix` is ``True``
     """
+
     if pre:
         path, fn = os.path.split(os.path.normpath(fname))
 
         # Handle the odd case where the filename = the prefix.
         # In that case, we still want to add the prefix to the file
-        if fn[:len(pre)] != pre or fn == pre:
+        if not fn.startswith(pre) or fn == pre:
             fname = os.path.join(path, pre + fn)
     # Only append a suffix if the suffix we're going to add isn't already
     # there, and if either we've been asked to ensure the specific suffix
     # is present or there's no suffix on it at all.
     # Also handle the odd case where the filename = the suffix.
     # in that case we still want to append the suffix
-    if suf and fname[-len(suf):] != suf and \
+    if suf and not fname.endswith(suf) and \
             (ensure_suffix or not splitext(fname)[1]):
         fname = fname + suf
     return fname
@@ -1164,14 +1305,20 @@ def adjustixes(fname, pre, suf, ensure_suffix=False):
 # https://code.activestate.com/recipes/52560
 # ASPN: Python Cookbook: Remove duplicates from a sequence
 # (Also in the printed Python Cookbook.)
+# Updated. This algorithm is used by some scanners and tools.
 
-def unique(s):
-    """Return a list of the elements in s, but without duplicates.
+def unique(seq):
+    """Return a list of the elements in seq without duplicates, ignoring order.
 
-    For example, unique([1,2,3,1,2,3]) is some permutation of [1,2,3],
-    unique("abcabc") some permutation of ["a", "b", "c"], and
-    unique(([1, 2], [2, 3], [1, 2])) some permutation of
-    [[2, 3], [1, 2]].
+    >>> mylist = unique([1, 2, 3, 1, 2, 3])
+    >>> print(sorted(mylist))
+    [1, 2, 3]
+    >>> mylist = unique("abcabc")
+    >>> print(sorted(mylist))
+    ['a', 'b', 'c']
+    >>> mylist = unique(([1, 2], [2, 3], [1, 2]))
+    >>> print(sorted(mylist))
+    [[1, 2], [2, 3]]
 
     For best speed, all sequence elements should be hashable.  Then
     unique() will usually work in linear time.
@@ -1182,41 +1329,33 @@ def unique(s):
     usually work in O(N*log2(N)) time.
 
     If that's not possible either, the sequence elements must support
-    equality-testing.  Then unique() will usually work in quadratic
-    time.
+    equality-testing.  Then unique() will usually work in quadratic time.
     """
 
-    n = len(s)
-    if n == 0:
+    if not seq:
         return []
 
     # Try using a dict first, as that's the fastest and will usually
     # work.  If it doesn't work, it will usually fail quickly, so it
     # usually doesn't cost much to *try* it.  It requires that all the
     # sequence elements be hashable, and support equality comparison.
-    u = {}
-    try:
-        for x in s:
-            u[x] = 1
-    except TypeError:
-        pass    # move on to the next method
-    else:
-        return list(u.keys())
-    del u
+    # TODO: should be even faster: return(list(set(seq)))
+    with suppress(TypeError):
+        return list(dict.fromkeys(seq))
 
-    # We can't hash all the elements.  Second fastest is to sort,
-    # which brings the equal elements together; then duplicates are
-    # easy to weed out in a single pass.
+    # We couldn't hash all the elements (got a TypeError).
+    # Next fastest is to sort, which brings the equal elements together;
+    # then duplicates are easy to weed out in a single pass.
     # NOTE:  Python's list.sort() was designed to be efficient in the
     # presence of many duplicate elements.  This isn't true of all
     # sort functions in all languages or libraries, so this approach
     # is more effective in Python than it may be elsewhere.
+    n = len(seq)
     try:
-        t = sorted(s)
+        t = sorted(seq)
     except TypeError:
         pass    # move on to the next method
     else:
-        assert n > 0
         last = t[0]
         lasti = i = 1
         while i < n:
@@ -1225,11 +1364,10 @@ def unique(s):
                 lasti = lasti + 1
             i = i + 1
         return t[:lasti]
-    del t
 
     # Brute force is all that's left.
     u = []
-    for x in s:
+    for x in seq:
         if x not in u:
             u.append(x)
     return u
@@ -1260,7 +1398,7 @@ def uniquer(seq, idfun=None):
 
 # A more efficient implementation of Alex's uniquer(), this avoids the
 # idfun() argument and function-call overhead by assuming that all
-# items in the sequence are hashable.
+# items in the sequence are hashable.  Order-preserving.
 
 def uniquer_hashables(seq):
     seen = {}
@@ -1295,140 +1433,176 @@ def logical_lines(physical_lines, joiner=''.join):
 class LogicalLines:
     """ Wrapper class for the logical_lines method.
 
-        Allows us to read all "logical" lines at once from a
-        given file object.
+    Allows us to read all "logical" lines at once from a given file object.
     """
 
     def __init__(self, fileobj):
         self.fileobj = fileobj
 
     def readlines(self):
-        result = [l for l in logical_lines(self.fileobj)]
-        return result
+        return list(logical_lines(self.fileobj))
 
 
 class UniqueList(UserList):
-    def __init__(self, seq = []):
-        UserList.__init__(self, seq)
+    """A list which maintains uniqueness.
+
+    Uniquing is lazy: rather than being assured on list changes, it is fixed
+    up on access by those methods which need to act on a uniqe list to be
+    correct. That means things like "in" don't have to eat the uniquing time.
+    """
+    def __init__(self, initlist=None):
+        super().__init__(initlist)
         self.unique = True
+
     def __make_unique(self):
         if not self.unique:
             self.data = uniquer_hashables(self.data)
             self.unique = True
+
+    def __repr__(self):
+        self.__make_unique()
+        return super().__repr__()
+
     def __lt__(self, other):
         self.__make_unique()
-        return UserList.__lt__(self, other)
+        return super().__lt__(other)
+
     def __le__(self, other):
         self.__make_unique()
-        return UserList.__le__(self, other)
+        return super().__le__(other)
+
     def __eq__(self, other):
         self.__make_unique()
-        return UserList.__eq__(self, other)
+        return super().__eq__(other)
+
     def __ne__(self, other):
         self.__make_unique()
-        return UserList.__ne__(self, other)
+        return super().__ne__(other)
+
     def __gt__(self, other):
         self.__make_unique()
-        return UserList.__gt__(self, other)
+        return super().__gt__(other)
+
     def __ge__(self, other):
         self.__make_unique()
-        return UserList.__ge__(self, other)
-    def __cmp__(self, other):
-        self.__make_unique()
-        return UserList.__cmp__(self, other)
+        return super().__ge__(other)
+
+    # __contains__ doesn't need to worry about uniquing, inherit
+
     def __len__(self):
         self.__make_unique()
-        return UserList.__len__(self)
+        return super().__len__()
+
     def __getitem__(self, i):
         self.__make_unique()
-        return UserList.__getitem__(self, i)
+        return super().__getitem__(i)
+
     def __setitem__(self, i, item):
-        UserList.__setitem__(self, i, item)
+        super().__setitem__(i, item)
         self.unique = False
+
+    # __delitem__ doesn't need to worry about uniquing, inherit
+
     def __add__(self, other):
-        result = UserList.__add__(self, other)
+        result = super().__add__(other)
         result.unique = False
         return result
+
     def __radd__(self, other):
-        result = UserList.__radd__(self, other)
+        result = super().__radd__(other)
         result.unique = False
         return result
+
     def __iadd__(self, other):
-        result = UserList.__iadd__(self, other)
+        result = super().__iadd__(other)
         result.unique = False
         return result
+
     def __mul__(self, other):
-        result = UserList.__mul__(self, other)
+        result = super().__mul__(other)
         result.unique = False
         return result
+
     def __rmul__(self, other):
-        result = UserList.__rmul__(self, other)
+        result = super().__rmul__(other)
         result.unique = False
         return result
+
     def __imul__(self, other):
-        result = UserList.__imul__(self, other)
+        result = super().__imul__(other)
         result.unique = False
         return result
+
     def append(self, item):
-        UserList.append(self, item)
+        super().append(item)
         self.unique = False
-    def insert(self, i):
-        UserList.insert(self, i)
+
+    def insert(self, i, item):
+        super().insert(i, item)
         self.unique = False
+
     def count(self, item):
         self.__make_unique()
-        return UserList.count(self, item)
-    def index(self, item):
+        return super().count(item)
+
+    def index(self, item, *args):
         self.__make_unique()
-        return UserList.index(self, item)
+        return super().index(item, *args)
+
     def reverse(self):
         self.__make_unique()
-        UserList.reverse(self)
+        super().reverse()
+
+    # TODO: Py3.8: def sort(self, /, *args, **kwds):
     def sort(self, *args, **kwds):
         self.__make_unique()
-        return UserList.sort(self, *args, **kwds)
+        return super().sort(*args, **kwds)
+
     def extend(self, other):
-        UserList.extend(self, other)
+        super().extend(other)
         self.unique = False
 
 
 class Unbuffered:
-    """
-    A proxy class that wraps a file object, flushing after every write,
-    and delegating everything else to the wrapped object.
+    """A proxy  that wraps a file object, flushing after every write.
+
+    Delegates everything else to the wrapped object.
     """
     def __init__(self, file):
         self.file = file
-        self.softspace = 0  ## backward compatibility; not supported in Py3k
+
     def write(self, arg):
-        try:
+        # Stdout might be connected to a pipe that has been closed
+        # by now. The most likely reason for the pipe being closed
+        # is that the user has press ctrl-c. It this is the case,
+        # then SCons is currently shutdown. We therefore ignore
+        # IOError's here so that SCons can continue and shutdown
+        # properly so that the .sconsign is correctly written
+        # before SCons exits.
+        with suppress(IOError):
             self.file.write(arg)
             self.file.flush()
-        except IOError:
-            # Stdout might be connected to a pipe that has been closed
-            # by now. The most likely reason for the pipe being closed
-            # is that the user has press ctrl-c. It this is the case,
-            # then SCons is currently shutdown. We therefore ignore
-            # IOError's here so that SCons can continue and shutdown
-            # properly so that the .sconsign is correctly written
-            # before SCons exits.
-            pass
+
+    def writelines(self, arg):
+        with suppress(IOError):
+            self.file.writelines(arg)
+            self.file.flush()
+
     def __getattr__(self, attr):
         return getattr(self.file, attr)
 
-def make_path_relative(path):
-    """ makes an absolute path name to a relative pathname.
-    """
+def make_path_relative(path) -> str:
+    """Converts an absolute path name to a relative pathname."""
+
     if os.path.isabs(path):
-        drive_s,path = os.path.splitdrive(path)
+        drive_s, path = os.path.splitdrive(path)
 
-        import re
         if not drive_s:
-            path=re.compile("/*(.*)").findall(path)[0]
+            path=re.compile(r"/*(.*)").findall(path)[0]
         else:
             path=path[1:]
 
-    assert( not os.path.isabs( path ) ), path
+    assert not os.path.isabs(path), path
     return path
 
 
@@ -1460,18 +1634,23 @@ def AddMethod(obj, function, name=None):
     construction environments; it is preferred to use env.AddMethod
     to add to an individual environment.
 
-    Example::
-
-        class A:
-            ...
-        a = A()
-        def f(self, x, y):
-            self.z = x + y
-        AddMethod(f, A, "add")
-        a.add(2, 4)
-        print(a.z)
-        AddMethod(lambda self, i: self.l[i], a, "listIndex")
-        print(a.listIndex(5))
+    >>> class A:
+    ...    ...
+
+    >>> a = A()
+
+    >>> def f(self, x, y):
+    ...    self.z = x + y
+
+    >>> AddMethod(A, f, "add")
+    >>> a.add(2, 4)
+    >>> print(a.z)
+    6
+    >>> a.data = ['a', 'b', 'c', 'd', 'e', 'f']
+    >>> AddMethod(a, lambda self, i: self.data[i], "listIndex")
+    >>> print(a.listIndex(3))
+    d
+
     """
     if name is None:
         name = function.__name__
@@ -1496,58 +1675,63 @@ def AddMethod(obj, function, name=None):
 
 
 # Default hash function and format. SCons-internal.
-_hash_function = None
-_hash_format = None
+ALLOWED_HASH_FORMATS = ['md5', 'sha1', 'sha256']
+_HASH_FUNCTION = None
+_HASH_FORMAT = None
 
 
 def get_hash_format():
-    """
-    Retrieves the hash format or None if not overridden. A return value of None
+    """Retrieves the hash format or ``None`` if not overridden.
+
+    A return value of ``None``
     does not guarantee that MD5 is being used; instead, it means that the
-    default precedence order documented in SCons.Util.set_hash_format is
-    respected.
+    default precedence order documented in :func:`SCons.Util.set_hash_format`
+    is respected.
     """
-    return _hash_format
+    return _HASH_FORMAT
 
 
 def set_hash_format(hash_format):
-    """
-    Sets the default hash format used by SCons. If hash_format is None or
+    """Sets the default hash format used by SCons.
+
+    If `hash_format` is ``None`` or
     an empty string, the default is determined by this function.
 
     Currently the default behavior is to use the first available format of
     the following options: MD5, SHA1, SHA256.
     """
-    global _hash_format, _hash_function
+    global _HASH_FORMAT, _HASH_FUNCTION
 
-    _hash_format = hash_format
+    _HASH_FORMAT = hash_format
     if hash_format:
         hash_format_lower = hash_format.lower()
-        allowed_hash_formats = ['md5', 'sha1', 'sha256']
-        if hash_format_lower not in allowed_hash_formats:
-            from SCons.Errors import UserError
+        if hash_format_lower not in ALLOWED_HASH_FORMATS:
+            from SCons.Errors import UserError  # pylint: disable=import-outside-toplevel
+
             raise UserError('Hash format "%s" is not supported by SCons. Only '
                             'the following hash formats are supported: %s' %
                             (hash_format_lower,
-                             ', '.join(allowed_hash_formats)))
+                             ', '.join(ALLOWED_HASH_FORMATS)))
+
+        _HASH_FUNCTION = getattr(hashlib, hash_format_lower, None)
+        if _HASH_FUNCTION is None:
+            from SCons.Errors import UserError  # pylint: disable=import-outside-toplevel
 
-        _hash_function = getattr(hashlib, hash_format_lower, None)
-        if _hash_function is None:
-            from SCons.Errors import UserError
             raise UserError(
-                'Hash format "%s" is not available in your Python '
-                'interpreter.' % hash_format_lower)
+                'Hash format "%s" is not available in your Python interpreter.'
+                % hash_format_lower
+            )
     else:
         # Set the default hash format based on what is available, defaulting
         # to md5 for backwards compatibility.
-        choices = ['md5', 'sha1', 'sha256']
-        for choice in choices:
-            _hash_function = getattr(hashlib, choice, None)
-            if _hash_function is not None:
+        for choice in ALLOWED_HASH_FORMATS:
+            _HASH_FUNCTION = getattr(hashlib, choice, None)
+            if _HASH_FUNCTION is not None:
                 break
         else:
             # This is not expected to happen in practice.
-            from SCons.Errors import UserError
+            from SCons.Errors import UserError  # pylint: disable=import-outside-toplevel
+
             raise UserError(
                 'Your Python interpreter does not have MD5, SHA1, or SHA256. '
                 'SCons requires at least one.')
@@ -1560,34 +1744,42 @@ set_hash_format(None)
 
 
 def _get_hash_object(hash_format):
-    """
-    Allocates a hash object using the requested hash format.
+    """Allocates a hash object using the requested hash format.
 
-    :param hash_format: Hash format to use.
-    :return: hashlib object.
+    Args:
+        hash_format: Hash format to use.
+
+    Returns:
+        hashlib object.
     """
     if hash_format is None:
-        if _hash_function is None:
-            from SCons.Errors import UserError
+        if _HASH_FUNCTION is None:
+            from SCons.Errors import UserError  # pylint: disable=import-outside-toplevel
+
             raise UserError('There is no default hash function. Did you call '
                             'a hashing function before SCons was initialized?')
-        return _hash_function()
-    elif not hasattr(hashlib, hash_format):
-        from SCons.Errors import UserError
+        return _HASH_FUNCTION()
+
+    if not hasattr(hashlib, hash_format):
+        from SCons.Errors import UserError  # pylint: disable=import-outside-toplevel
+
         raise UserError(
             'Hash format "%s" is not available in your Python interpreter.' %
             hash_format)
-    else:
-        return getattr(hashlib, hash_format)()
+
+    return getattr(hashlib, hash_format)()
 
 
 def hash_signature(s, hash_format=None):
     """
     Generate hash signature of a string
 
-    :param s: either string or bytes. Normally should be bytes
-    :param hash_format: Specify to override default hash format
-    :return: String of hex digits representing the signature
+    Args:
+        s: either string or bytes. Normally should be bytes
+        hash_format: Specify to override default hash format
+
+    Returns:
+        String of hex digits representing the signature
     """
     m = _get_hash_object(hash_format)
     try:
@@ -1602,11 +1794,15 @@ def hash_file_signature(fname, chunksize=65536, hash_format=None):
     """
     Generate the md5 signature of a file
 
-    :param fname: file to hash
-    :param chunksize: chunk size to read
-    :param hash_format: Specify to override default hash format
-    :return: String of Hex digits representing the signature
+    Args:
+        fname: file to hash
+        chunksize: chunk size to read
+        hash_format: Specify to override default hash format
+
+    Returns:
+        String of Hex digits representing the signature
     """
+
     m = _get_hash_object(hash_format)
     with open(fname, "rb") as f:
         while True:
@@ -1621,60 +1817,60 @@ def hash_collect(signatures, hash_format=None):
     """
     Collects a list of signatures into an aggregate signature.
 
-    :param signatures: a list of signatures
-    :param hash_format: Specify to override default hash format
-    :return: - the aggregate signature
+    Args:
+        signatures: a list of signatures
+        hash_format: Specify to override default hash format
+
+    Returns:
+        the aggregate signature
     """
+
     if len(signatures) == 1:
         return signatures[0]
-    else:
-        return hash_signature(', '.join(signatures), hash_format)
+
+    return hash_signature(', '.join(signatures), hash_format)
 
 
-_md5_warning_shown = False
+_MD5_WARNING_SHOWN = False
 
 def _show_md5_warning(function_name):
-    """
-    Shows a deprecation warning for various MD5 functions.
-    """
-    global _md5_warning_shown
+    """Shows a deprecation warning for various MD5 functions."""
 
-    if not _md5_warning_shown:
-        import SCons.Warnings
+    global _MD5_WARNING_SHOWN
+
+    if not _MD5_WARNING_SHOWN:
+        import SCons.Warnings  # pylint: disable=import-outside-toplevel
 
         SCons.Warnings.warn(SCons.Warnings.DeprecatedWarning,
                             "Function %s is deprecated" % function_name)
-        _md5_warning_shown = True
+        _MD5_WARNING_SHOWN = True
 
 
 def MD5signature(s):
-    """
-    Deprecated. Use hash_signature instead.
-    """
+    """Deprecated. Use :func:`hash_signature` instead."""
+
     _show_md5_warning("MD5signature")
     return hash_signature(s)
 
 
 def MD5filesignature(fname, chunksize=65536):
-    """
-    Deprecated. Use hash_file_signature instead.
-    """
+    """Deprecated. Use :func:`hash_file_signature` instead."""
+
     _show_md5_warning("MD5filesignature")
     return hash_file_signature(fname, chunksize)
 
 
 def MD5collect(signatures):
-    """
-    Deprecated. Use hash_collect instead.
-    """
+    """Deprecated. Use :func:`hash_collect` instead."""
+
     _show_md5_warning("MD5collect")
     return hash_collect(signatures)
 
 
 def silent_intern(x):
     """
-    Perform sys.intern() on the passed argument and return the result.
-    If the input is ineligible the original argument is
+    Perform :mod:`sys.intern` on the passed argument and return the result.
+    If the input is ineligible for interning the original argument is
     returned and no exception is thrown.
     """
     try:
@@ -1724,7 +1920,7 @@ class NullSeq(Null):
         return self
 
 
-def to_bytes(s):
+def to_bytes(s) -> bytes:
     if s is None:
         return b'None'
     if isinstance(s, (bytes, bytearray)):
@@ -1733,7 +1929,7 @@ def to_bytes(s):
     return bytes(s, 'utf-8')
 
 
-def to_str(s):
+def to_str(s) -> str:
     if s is None:
         return 'None'
     if is_String(s):
@@ -1741,30 +1937,28 @@ def to_str(s):
     return str(s, 'utf-8')
 
 
-def cmp(a, b):
-    """
-    Define cmp because it's no longer available in python3
-    Works under python 2 as well
-    """
+def cmp(a, b) -> bool:
+    """A cmp function because one is no longer available in python3."""
     return (a > b) - (a < b)
 
 
-def get_env_bool(env, name, default=False):
+def get_env_bool(env, name, default=False) -> bool:
     """Convert a construction variable to bool.
 
-    If the value of *name* in *env* is 'true', 'yes', 'y', 'on' (case
+    If the value of `name` in `env` is 'true', 'yes', 'y', 'on' (case
     insensitive) or anything convertible to int that yields non-zero then
-    return True; if 'false', 'no', 'n', 'off' (case insensitive)
-    or a number that converts to integer zero return False.
-    Otherwise, return *default*.
+    return ``True``; if 'false', 'no', 'n', 'off' (case insensitive)
+    or a number that converts to integer zero return ``False``.
+    Otherwise, return `default`.
 
     Args:
         env: construction environment, or any dict-like object
         name: name of the variable
-        default: value to return if *name* not in *env* or cannot
+        default: value to return if `name` not in `env` or cannot
           be converted (default: False)
+
     Returns:
-        bool: the "truthiness" of *name*
+        the "truthiness" of `name`
     """
     try:
         var = env[name]
@@ -1775,21 +1969,24 @@ def get_env_bool(env, name, default=False):
     except ValueError:
         if str(var).lower() in ('true', 'yes', 'y', 'on'):
             return True
-        elif str(var).lower() in ('false', 'no', 'n', 'off'):
+
+        if str(var).lower() in ('false', 'no', 'n', 'off'):
             return False
-        else:
-            return default
+
+        return default
 
 
-def get_os_env_bool(name, default=False):
+def get_os_env_bool(name, default=False) -> bool:
     """Convert an environment variable to bool.
 
     Conversion is the same as for :func:`get_env_bool`.
     """
     return get_env_bool(os.environ, name, default)
 
+
 def print_time():
     """Hack to return a value from Main if can't import Main."""
+    # pylint: disable=redefined-outer-name,import-outside-toplevel
     from SCons.Script.Main import print_time
     return print_time
 
diff --git a/SCons/UtilTests.py b/SCons/UtilTests.py
index 8e39a16a7..6cdc2ee7f 100644
--- a/SCons/UtilTests.py
+++ b/SCons/UtilTests.py
@@ -32,10 +32,44 @@ import TestCmd
 
 import SCons.Errors
 import SCons.compat
-from SCons.Util import hash_signature, get_os_env_bool, get_env_bool, flatten, to_bytes, to_String, render_tree, to_str, \
-    is_Dict, is_String, is_List, splitext, print_tree, is_Tuple, silent_intern, get_native_path, get_environment_var, \
-    display, containsAny, containsAll, containsOnly, WhereIs, Selector, adjustixes, Proxy, AddPathIfNotExists, \
-    PrependPath, NodeList, AppendPath, LogicalLines, hash_collect
+from SCons.Util import (
+    AddPathIfNotExists,
+    AppendPath,
+    CLVar,
+    LogicalLines,
+    NodeList,
+    PrependPath,
+    Proxy,
+    Selector,
+    WhereIs,
+    adjustixes,
+    containsAll,
+    containsAny,
+    containsOnly,
+    dictify,
+    display,
+    flatten,
+    get_env_bool,
+    get_environment_var,
+    get_native_path,
+    get_os_env_bool,
+    hash_collect,
+    hash_signature,
+    is_Dict,
+    is_List,
+    is_String,
+    is_Tuple,
+    print_tree,
+    render_tree,
+    silent_intern,
+    splitext,
+    to_String,
+    to_bytes,
+    to_str,
+)
+
+# These Util classes have no unit tests. Some don't make sense to test?
+# DisplayEngine, Delegate, MethodWrapper, UniqueList, Unbuffered, Null, NullSeq
 
 
 class OutBuffer:
@@ -49,13 +83,13 @@ class OutBuffer:
 class dictifyTestCase(unittest.TestCase):
     def test_dictify(self):
         """Test the dictify() function"""
-        r = SCons.Util.dictify(['a', 'b', 'c'], [1, 2, 3])
+        r = dictify(['a', 'b', 'c'], [1, 2, 3])
         assert r == {'a': 1, 'b': 2, 'c': 3}, r
 
         r = {}
-        SCons.Util.dictify(['a'], [1], r)
-        SCons.Util.dictify(['b'], [2], r)
-        SCons.Util.dictify(['c'], [3], r)
+        dictify(['a'], [1], r)
+        dictify(['b'], [2], r)
+        dictify(['c'], [3], r)
         assert r == {'a': 1, 'b': 2, 'c': 3}, r
 
 
@@ -552,115 +586,115 @@ class UtilTestCase(unittest.TestCase):
         """Test the command-line construction variable class"""
 
         # input to CLVar is a string - should be split
-        f = SCons.Util.CLVar('aa bb')
+        f = CLVar('aa bb')
 
         r = f + 'cc dd'
-        assert isinstance(r, SCons.Util.CLVar), type(r)
+        assert isinstance(r, CLVar), type(r)
         assert r.data == ['aa', 'bb', 'cc', 'dd'], r.data
         assert str(r) == 'aa bb cc dd', str(r)
 
         r = f + ' cc dd'
-        assert isinstance(r, SCons.Util.CLVar), type(r)
+        assert isinstance(r, CLVar), type(r)
         assert r.data == ['aa', 'bb', 'cc', 'dd'], r.data
         assert str(r) == 'aa bb cc dd', str(r)
 
         r = f + ['cc dd']
-        assert isinstance(r, SCons.Util.CLVar), type(r)
+        assert isinstance(r, CLVar), type(r)
         assert r.data == ['aa', 'bb', 'cc dd'], r.data
         assert str(r) == 'aa bb cc dd', str(r)
 
         r = f + [' cc dd']
-        assert isinstance(r, SCons.Util.CLVar), type(r)
+        assert isinstance(r, CLVar), type(r)
         assert r.data == ['aa', 'bb', ' cc dd'], r.data
         assert str(r) == 'aa bb  cc dd', str(r)
 
         r = f + ['cc', 'dd']
-        assert isinstance(r, SCons.Util.CLVar), type(r)
+        assert isinstance(r, CLVar), type(r)
         assert r.data == ['aa', 'bb', 'cc', 'dd'], r.data
         assert str(r) == 'aa bb cc dd', str(r)
 
         r = f + [' cc', 'dd']
-        assert isinstance(r, SCons.Util.CLVar), type(r)
+        assert isinstance(r, CLVar), type(r)
         assert r.data == ['aa', 'bb', ' cc', 'dd'], r.data
         assert str(r) == 'aa bb  cc dd', str(r)
 
         # input to CLVar is a list of one string, should not be split
-        f = SCons.Util.CLVar(['aa bb'])
+        f = CLVar(['aa bb'])
 
         r = f + 'cc dd'
-        assert isinstance(r, SCons.Util.CLVar), type(r)
+        assert isinstance(r, CLVar), type(r)
         assert r.data == ['aa bb', 'cc', 'dd'], r.data
         assert str(r) == 'aa bb cc dd', str(r)
 
         r = f + ' cc dd'
-        assert isinstance(r, SCons.Util.CLVar), type(r)
+        assert isinstance(r, CLVar), type(r)
         assert r.data == ['aa bb', 'cc', 'dd'], r.data
         assert str(r) == 'aa bb cc dd', str(r)
 
         r = f + ['cc dd']
-        assert isinstance(r, SCons.Util.CLVar), type(r)
+        assert isinstance(r, CLVar), type(r)
         assert r.data == ['aa bb', 'cc dd'], r.data
         assert str(r) == 'aa bb cc dd', str(r)
 
         r = f + [' cc dd']
-        assert isinstance(r, SCons.Util.CLVar), type(r)
+        assert isinstance(r, CLVar), type(r)
         assert r.data == ['aa bb', ' cc dd'], r.data
         assert str(r) == 'aa bb  cc dd', str(r)
 
         r = f + ['cc', 'dd']
-        assert isinstance(r, SCons.Util.CLVar), type(r)
+        assert isinstance(r, CLVar), type(r)
         assert r.data == ['aa bb', 'cc', 'dd'], r.data
         assert str(r) == 'aa bb cc dd', str(r)
 
         r = f + [' cc', 'dd']
-        assert isinstance(r, SCons.Util.CLVar), type(r)
+        assert isinstance(r, CLVar), type(r)
         assert r.data == ['aa bb', ' cc', 'dd'], r.data
         assert str(r) == 'aa bb  cc dd', str(r)
 
         # input to CLVar is a list of strings
-        f = SCons.Util.CLVar(['aa', 'bb'])
+        f = CLVar(['aa', 'bb'])
 
         r = f + 'cc dd'
-        assert isinstance(r, SCons.Util.CLVar), type(r)
+        assert isinstance(r, CLVar), type(r)
         assert r.data == ['aa', 'bb', 'cc', 'dd'], r.data
         assert str(r) == 'aa bb cc dd', str(r)
 
         r = f + ' cc dd'
-        assert isinstance(r, SCons.Util.CLVar), type(r)
+        assert isinstance(r, CLVar), type(r)
         assert r.data == ['aa', 'bb', 'cc', 'dd'], r.data
         assert str(r) == 'aa bb cc dd', str(r)
 
         r = f + ['cc dd']
-        assert isinstance(r, SCons.Util.CLVar), type(r)
+        assert isinstance(r, CLVar), type(r)
         assert r.data == ['aa', 'bb', 'cc dd'], r.data
         assert str(r) == 'aa bb cc dd', str(r)
 
         r = f + [' cc dd']
-        assert isinstance(r, SCons.Util.CLVar), type(r)
+        assert isinstance(r, CLVar), type(r)
         assert r.data == ['aa', 'bb', ' cc dd'], r.data
         assert str(r) == 'aa bb  cc dd', str(r)
 
         r = f + ['cc', 'dd']
-        assert isinstance(r, SCons.Util.CLVar), type(r)
+        assert isinstance(r, CLVar), type(r)
         assert r.data == ['aa', 'bb', 'cc', 'dd'], r.data
         assert str(r) == 'aa bb cc dd', str(r)
 
         r = f + [' cc', 'dd']
-        assert isinstance(r, SCons.Util.CLVar), type(r)
+        assert isinstance(r, CLVar), type(r)
         assert r.data == ['aa', 'bb', ' cc', 'dd'], r.data
         assert str(r) == 'aa bb  cc dd', str(r)
 
         # make sure inplace adding a string works as well (issue 2399)
         # UserList would convert the string to a list of chars
-        f = SCons.Util.CLVar(['aa', 'bb'])
+        f = CLVar(['aa', 'bb'])
         f += 'cc dd'
-        assert isinstance(f, SCons.Util.CLVar), type(f)
+        assert isinstance(f, CLVar), type(f)
         assert f.data == ['aa', 'bb', 'cc', 'dd'], f.data
         assert str(f) == 'aa bb cc dd', str(f)
 
-        f = SCons.Util.CLVar(['aa', 'bb'])
+        f = CLVar(['aa', 'bb'])
         f += ' cc dd'
-        assert isinstance(f, SCons.Util.CLVar), type(f)
+        assert isinstance(f, CLVar), type(f)
         assert f.data == ['aa', 'bb', 'cc', 'dd'], f.data
         assert str(f) == 'aa bb cc dd', str(f)