From 1c42e32befd2a80468190a5e5e985ba34204403c Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Mon, 22 Jun 2026 08:12:45 -0600 Subject: [PATCH] Subst cleanups Changes to doctrings and typing in function signatures. scons_subst and scons_subst_list now check if the ovverides arg is None. Small tweak to ActionTests is to make sure it actually calls scons_subst and scons_subst_list with overrides as a dict (previously it defaulted to a bool). Signed-off-by: Mats Wichmann --- RELEASE.txt | 2 +- SCons/ActionTests.py | 8 ++- SCons/Subst.py | 120 ++++++++++++++++++++++++++----------------- 3 files changed, 81 insertions(+), 49 deletions(-) diff --git a/RELEASE.txt b/RELEASE.txt index 50438076c..eb11830ca 100644 --- a/RELEASE.txt +++ b/RELEASE.txt @@ -74,7 +74,7 @@ IMPROVEMENTS * Fix for_signature logic bug in ListSubber.expand() for correct signature generation * Modernize string formatting to f-strings - Also, separately, some code-style cleanups in Subst.py. + Also, separately, some code-style cleanups and docstring work in Subst.py. - Test runner reworked to use Python logging; the portion of the test suite which tests the runner was adjusted to match. diff --git a/SCons/ActionTests.py b/SCons/ActionTests.py index ec3c36d20..7f5d1fc6b 100644 --- a/SCons/ActionTests.py +++ b/SCons/ActionTests.py @@ -154,13 +154,17 @@ def __init__(self, **kw) -> None: self.d[k] = v # Just use the underlying scons_subst*() utility methods. - def subst(self, strSubst, raw: int=0, target=[], source=[], conv=None, overrides: bool=False): + def subst(self, strSubst, raw: int=0, target=[], source=[], conv=None, overrides: dict | None = None): + if overrides is None: + overrides = {} return SCons.Subst.scons_subst(strSubst, self, raw, target, source, self.d, conv=conv, overrides=overrides) subst_target_source = subst - def subst_list(self, strSubst, raw: int=0, target=[], source=[], conv=None, overrides: bool=False): + def subst_list(self, strSubst, raw: int=0, target=[], source=[], conv=None, overrides: dict | None = None): + if overrides is None: + overrides = {} return SCons.Subst.scons_subst_list(strSubst, self, raw, target, source, self.d, conv=conv, overrides=overrides) diff --git a/SCons/Subst.py b/SCons/Subst.py index feffe4ca9..64f851c85 100644 --- a/SCons/Subst.py +++ b/SCons/Subst.py @@ -28,11 +28,12 @@ import re from collections import UserList, UserString from functools import lru_cache -from inspect import signature, Parameter +from inspect import Parameter, signature +from typing import Callable import SCons.Errors import SCons.Util -from SCons.Util import is_String, is_Sequence +from SCons.Util import is_Sequence, is_String # Indexed by the SUBST_* constants below. _strconv = [ @@ -58,11 +59,11 @@ def raise_exception(exception, target, s): class Literal: - """A string wrapper for a string to prevent expansion. + """A wrapper for non-subsitutable strings. - If you use this object wrapped around a string, then it will - be interpreted as literal. When passed to the command interpreter, - all special characters will be escaped. + The substitution logic will not change the wrapped string. + When passed to the command interpreter, all special + characters will be escaped and/or the string quoted. """ def __init__(self, lstr) -> None: @@ -71,7 +72,7 @@ def __init__(self, lstr) -> None: def __str__(self) -> str: return self.lstr - def escape(self, escape_func): + def escape(self, escape_func: Callable) -> str: return escape_func(self.lstr) def for_signature(self): @@ -97,18 +98,20 @@ def __contains__(self, key) -> bool: return key in self.lstr class SpecialAttrWrapper: - """This is a wrapper for what we call a 'Node special attribute.' - This is any of the attributes of a Node that we can reference from - Environment variable substitution, such as $TARGET.abspath or - $SOURCES[1].filebase. We implement the same methods as Literal - so we can handle special characters, plus a for_signature method, - such that we can return some canonical string during signature - calculation to avoid unnecessary rebuilds.""" - - def __init__(self, lstr, for_signature=None) -> None: - """The for_signature parameter, if supplied, will be the - canonical string we return from for_signature(). Else - we will simply return lstr.""" + """A wrapper for Node special attributes. + + "Special" are any attributes of a Node that we can reference from + Environment variable substitution, such as ``$TARGET.abspath`` or + ``$SOURCES[1].filebase``. Implements the same methods as :class:`Literal + so we can handle special characters, plus a :meth:`for_signature` method, + so we can return some canonical string during signature + calculation to avoid unnecessary rebuilds. + + If the *for_signature* parameter is supplied at creation time, + it is the "signature" :meth:`for_signature` will return. + """ + + def __init__(self, lstr: str, for_signature: str | None = None) -> None: self.lstr = lstr if for_signature: self.forsig = for_signature @@ -118,22 +121,21 @@ def __init__(self, lstr, for_signature=None) -> None: def __str__(self) -> str: return self.lstr - def escape(self, escape_func): + def escape(self, escape_func: Callable) -> str: return escape_func(self.lstr) - def for_signature(self): + def for_signature(self) -> str: return self.forsig def is_literal(self) -> bool: return True -def quote_spaces(arg): +def quote_spaces(arg: str) -> str: """Generic function for putting double quotes around any string that has white space in it.""" if ' ' in arg or '\t' in arg: return f'"{arg}"' - else: - return str(arg) + return str(arg) class CmdStringHolder(UserString): """Holder for substituted strings intended for the command line. @@ -152,14 +154,19 @@ def __init__(self, cmd, literal: bool = False) -> None: def is_literal(self) -> bool: return self.literal - def escape(self, escape_func, quote_func=quote_spaces) -> str: - """Escape the string with the supplied function. The - function is expected to take an arbitrary string, then - return it with all special characters escaped and ready - for passing to the command interpreter. + def escape(self, escape_func: Callable, quote_func: Callable = quote_spaces) -> str: + """Escape characters in a saved string. - After calling this function, the next call to str() will - return the escaped string. + Like ``escape`` methods in other subst-related classes, this is + indirect - you call ``object.escape``, but have to pass a function + which actually performs the escaping - this is because the approach + needed is platform-specific. + + Args: + escape_func: function to escape special characters. + quote_func: function for quoting the string. Used only if + the string is not marked as "literal". Defaults to + :func:`quote_spaces`. """ data = self.data if self.literal: @@ -168,11 +175,11 @@ def escape(self, escape_func, quote_func=quote_spaces) -> str: return quote_func(data) return data -def escape_list(mylist, escape_func) -> list[str]: +def escape_list(mylist: list, escape_func: Callable) -> list[str]: """Escape a list of arguments by running the specified escape_func on every object in the list that has an escape() method.""" - def escape(obj, escape_func=escape_func): + def escape(obj, escape_func: Callable = escape_func): try: e = obj.escape except AttributeError: @@ -403,7 +410,7 @@ class StringSubber: the expansion. """ - def __init__(self, env, mode: int, conv, gvars: dict) -> None: + def __init__(self, env, mode: int, conv: Callable, gvars: dict) -> None: self.env = env self.mode = mode self.conv = conv @@ -559,7 +566,7 @@ class ListSubber(UserList): internally. """ - def __init__(self, env, mode: int, conv, gvars: dict) -> None: + def __init__(self, env, mode: int, conv: Callable, gvars: dict) -> None: super().__init__([]) self.env = env self.mode = mode @@ -595,8 +602,8 @@ def expanded(self, s) -> bool: return False return _unexpandable.search(s) is None - def expand(self, s, lvars, within_list): - """Expand a single "token" as necessary, appending the + def expand(self, s, lvars, within_list) -> None: + """Expand a single token *s* as necessary, appending the expansion to the current result. This handles expanding different types of things (strings, @@ -696,7 +703,7 @@ def expand(self, s, lvars, within_list): else: self.append(s) - def substitute(self, args, lvars, within_list) -> None: + def substitute(self, args, lvars, within_list: bool) -> None: """Substitute expansions in an argument or list of arguments. This serves as a wrapper for splitting up a string into @@ -884,9 +891,18 @@ def _remove_list(list): _space_sep = re.compile(r'[\t ]+(?![^{]*})') -def scons_subst(strSubst, env, mode=SUBST_RAW, target=None, source=None, gvars=None, lvars=None, conv=None, overrides: dict | None = None): - """Expand a string or list containing construction variable - substitutions. +def scons_subst( + strSubst, + env, + mode=SUBST_RAW, + target=None, + source=None, + gvars: dict | None = None, + lvars: dict | None = None, + conv: Callable | None = None, + overrides: dict | None = None, +): + """Expand a string or list containing construction variable substitutions. This is the work-horse function for substitutions in file names and the like. The companion scons_subst_list() function (below) @@ -902,9 +918,10 @@ def scons_subst(strSubst, env, mode=SUBST_RAW, target=None, source=None, gvars=N gvars = {} if lvars is None: lvars = {} - if conv is None: conv = _strconv[mode] + if overrides is None: + overrides = {} # Doing this every time is a bit of a waste, since the Executor # has typically already populated the OverrideEnvironment with @@ -973,7 +990,17 @@ def scons_subst(strSubst, env, mode=SUBST_RAW, target=None, source=None, gvars=N return result -def scons_subst_list(strSubst, env, mode=SUBST_RAW, target=None, source=None, gvars=None, lvars=None, conv=None, overrides: dict | None = None): +def scons_subst_list( + strSubst, + env, + mode=SUBST_RAW, + target=None, + source=None, + gvars: dict | None = None, + lvars: dict | None = None, + conv: Callable | None = None, + overrides: dict | None = None, +): """Substitute construction variables in a string (or list or other object) and separate the arguments into a command list. @@ -985,9 +1012,10 @@ def scons_subst_list(strSubst, env, mode=SUBST_RAW, target=None, source=None, gv gvars = {} if lvars is None: lvars = {} - if conv is None: conv = _strconv[mode] + if overrides is None: + overrides = {} # Doing this every time is a bit of a waste, since the Executor # has typically already populated the OverrideEnvironment with @@ -1070,5 +1098,5 @@ def sub_match(match, val=val, matchlist=matchlist): if is_String(strSubst): return _dollar_exps.sub(sub_match, strSubst) - else: - return strSubst + + return strSubst