Skip to content

Commit 3cb0268

Browse files
committed
Assorted typing cleanups
Fix various niggly typing and checker complaints. Environment.py: * To avoid modifying a dict-like thing while iterating over it, we did .copy(), making an awkward chained construct that checkers don't like. Simpler (and the more common idion) is to make a list out of the .keys() view, that way you're iterating over that new list and not the dict (and save creating a dict copy). * Another use of copy() was in detecting a CPPDEFINES deque, where we want to keep a copy of it and not modify the original - use the deque constructor for this instead of calling copy() method. * Align the names of vars in overridden dunder methods in BuilderDict with parent UserDict for more consistency. * arg2nodes node_factory argument *could* be passed a None, indicate that in the signature. * Multiple uses of CacheDir (it was both an import from CacheDir for type checking, and the name of a function in Environment.py). This was confusing to at least one type checker, so just use the qualified name. * The environment Base class _update and _update_onlynew methods were annotated to take an EnvironmentBase argument, but in fact they're designed to update the internal dict from a passed dict. Adjusted the annotation. * Used the global Environment hook directly by name rather than fully qualified, as SCons.Environment doesn't have an import of itself. That's namespace niggling, because it worked as written. * The Repository method (public API level) was written to take kwargs and pass those on to the FS layer's Repository - which doesn't take kwargs. Repository() was never documented as accepting any keyword arguments (and no tests ever poked at that usage), so just removed to align the two. * In VariantDir, make a small readability change and also avoid reassigning the function parameters: the index-zero access occurs on the call to self.fs.VariantDir, not on the end of the call to arg2nodes. Node/FS.py: Glob and glob annotation adjusted to show the exclude argument could also be a single string, as that could be passed down from the public API level, as documented: "The optional exclude argument may be set to a pattern or a list of patterns describing files or directories to filter out of the match list." Tool/FortranCommon.py: fix an error where returns were Tuple, which is not imported; changed to tuple. Signed-off-by: Mats Wichmann <mats@linux.com>
1 parent 22b6173 commit 3cb0268

5 files changed

Lines changed: 39 additions & 36 deletions

File tree

CHANGES.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,8 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER
6666
via the SConsignFile(name=None) call.
6767
- Test framework: tweak module docstrings
6868
- Test suite: end to end tests don't use assert in result checks
69+
- A few more typing cleanups in Environment (and in one case which
70+
affected it in the Node package).
6971

7072

7173
RELEASE 4.10.1 - Sun, 16 Nov 2025 10:51:57 -0700

SCons/Environment.py

Lines changed: 28 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@
8585

8686
if TYPE_CHECKING:
8787
from SCons.Action import ActionBase
88-
from SCons.CacheDir import CacheDir
8988
from SCons.Executor import Executor
9089
from SCons.Node import Node, NodeInfoBase
9190
from SCons.Node.FS import DirNode, EntryNode, FileNode
@@ -159,7 +158,7 @@ def apply_tools(
159158
def copy_non_reserved_keywords(dict_: dict[str, Any]) -> dict[str, Any]:
160159
"""Copy a dict excluding reserved construction variable names."""
161160
result = semi_deepcopy(dict_)
162-
for k in result.copy().keys():
161+
for k in list(result.keys()):
163162
if k in reserved_construction_var_names:
164163
msg = "Ignoring attempt to set reserved variable `$%s'"
165164
SCons.Warnings.warn(SCons.Warnings.ReservedVariableWarning, msg % k)
@@ -181,7 +180,7 @@ def _set_BUILDERS(env: EnvironmentBase, key: str, value):
181180
"""Set the BUILDERS construction variable."""
182181
try:
183182
bd = env._dict[key]
184-
for k in bd.copy().keys():
183+
for k in list(bd.keys()):
185184
del bd[k]
186185
except KeyError:
187186
bd = BuilderDict(bd, env) # XXX use-before-set?
@@ -330,7 +329,7 @@ def _macro_conv(v) -> list:
330329
env_dict[key] = deque(defines.split())
331330
elif is_Tuple(defines):
332331
if len(defines) > 2:
333-
raise SCons.Errors.UserError(
332+
raise UserError(
334333
f"Invalid tuple in CPPDEFINES: {defines!r}, must be a two-tuple"
335334
)
336335
env_dict[key] = deque([defines])
@@ -379,14 +378,14 @@ def _macro_conv(v) -> list:
379378
# A tuple appended to anything should yield -Dkey=value
380379
elif is_Tuple(val):
381380
if len(val) > 2:
382-
raise SCons.Errors.UserError(
381+
raise UserError(
383382
f"Invalid tuple added to CPPDEFINES: {val!r}, "
384383
"must be a two-tuple"
385384
)
386385
if len(val) == 1:
387386
val = (val[0], None) # normalize
388387
if not is_Scalar(val[0]) or not is_Scalar(val[1]):
389-
raise SCons.Errors.UserError(
388+
raise UserError(
390389
f"Invalid tuple added to CPPDEFINES: {val!r}, "
391390
"values must be scalar"
392391
)
@@ -522,19 +521,19 @@ def __semi_deepcopy__(self):
522521
# object, and indeed just copying would modify the original builder
523522
raise TypeError( 'cannot semi_deepcopy a BuilderDict' )
524523

525-
def __setitem__(self, item: str, val) -> None:
524+
def __setitem__(self, key: str, item) -> None:
526525
try:
527-
method = getattr(self.env, item).method
526+
method = getattr(self.env, key).method
528527
except AttributeError:
529528
pass
530529
else:
531530
self.env.RemoveMethod(method)
532-
super().__setitem__(item, val)
533-
BuilderWrapper(self.env, val, item)
531+
super().__setitem__(key, item)
532+
BuilderWrapper(self.env, item, key)
534533

535-
def __delitem__(self, item: str) -> None:
536-
super().__delitem__(item)
537-
delattr(self.env, item)
534+
def __delitem__(self, key: str) -> None:
535+
super().__delitem__(key)
536+
delattr(self.env, key)
538537

539538
# MutableMapping.update() should be fine here with our own __setitem__.
540539
# def update(self, mapping: dict[str, Any]) -> None: # type: ignore[override]
@@ -693,7 +692,7 @@ def setdefault(self, key: str, default: Any | None = None) -> Any | None:
693692
def arg2nodes(
694693
self,
695694
args: str | Node | list[str | Node],
696-
node_factory: Callable[[str], Node | list[Node]] = _null, # type: ignore[assignment]
695+
node_factory: Callable[[str], Node | list[Node]] | None = _null, # type: ignore[assignment]
697696
lookup_list: list[Callable[[str], Node | None]] = _null, # type: ignore[assignment]
698697
**kw
699698
) -> list[Node]:
@@ -1260,7 +1259,7 @@ def MergeFlags(
12601259
# the cleanup loops at the end of the outer for loop,
12611260
# which implicitly gives us a new object.
12621261
if isinstance(orig, deque):
1263-
self[key] = self[key].copy()
1262+
self[key] = deque(orig)
12641263
cast(EnvironmentBase, self).AppendUnique(CPPDEFINES=value, delete_existing=True)
12651264
continue
12661265
try:
@@ -1465,21 +1464,21 @@ def get_builder(self, name: str) -> SCons.Builder.BuilderBase | None:
14651464
def validate_CacheDir_class(self, custom_class: type | None = None) -> type:
14661465
"""Return a validated custom CacheDir class.
14671466
1468-
Validate *custom_class*, which must be derived from
1467+
Validate that *custom_class*, is derived from
14691468
:class:`SCons.CacheDir.CacheDir`. If *custom_class* is not
14701469
supplied, use the ``CACHEDIR_CLASS`` entry from the environment.
14711470
Return the class if there was no error.
14721471
14731472
Raises:
1474-
UserError: if the class is not derived from ``CacheDir``.
1473+
UserError: if the class is not derived from :class:`~SConsCache.CacheDir`.
14751474
"""
14761475
if custom_class is None:
14771476
custom_class = self.get("CACHEDIR_CLASS", SCons.CacheDir.CacheDir)
14781477
if not issubclass(custom_class, SCons.CacheDir.CacheDir):
14791478
raise UserError("Custom CACHEDIR_CLASS %s not derived from CacheDir" % str(custom_class))
14801479
return custom_class
14811480

1482-
def get_CacheDir(self) -> CacheDir:
1481+
def get_CacheDir(self) -> SCons.CacheDir.CacheDir:
14831482
"""Return the CacheDir object for this environment, instantiating it if necessary."""
14841483
try:
14851484
path = self._CacheDir_path
@@ -1500,7 +1499,7 @@ def get_CacheDir(self) -> CacheDir:
15001499

15011500
cd = cachedir_class(path)
15021501
self._last_CacheDir_path: str | None = path
1503-
self._last_CacheDir: CacheDir = cd
1502+
self._last_CacheDir: SCons.CacheDir.CacheDir = cd
15041503
return cd
15051504

15061505
def get_factory(self, factory, default: str = 'File'):
@@ -1576,14 +1575,14 @@ def scanner_map_delete(self, kw=None) -> None:
15761575
except KeyError:
15771576
pass
15781577

1579-
def _update(self, other: EnvironmentBase) -> None:
1578+
def _update(self, other: dict[str, Any]) -> None:
15801579
"""Private method to update an environment's consvar dict directly.
15811580
15821581
Bypasses the normal checks that occur when users try to set items.
15831582
"""
15841583
self._dict.update(other)
15851584

1586-
def _update_onlynew(self, other: EnvironmentBase) -> None:
1585+
def _update_onlynew(self, other: dict[str, Any]) -> None:
15871586
"""Private method to add new items to an environment's consvar dict.
15881587
15891588
Only adds items from *other* whose keys do not already appear in
@@ -2703,7 +2702,7 @@ def Entry(self, name: str | Node | list[str | Node], *args, **kw) -> EntryNode |
27032702

27042703
def Environment(self, **kw) -> EnvironmentBase:
27052704
"""Create a construction environment object."""
2706-
return SCons.Environment.Environment(**self.subst_kw(kw))
2705+
return Environment(**self.subst_kw(kw))
27072706

27082707
def Execute(self, action, *args, **kw):
27092708
"""Directly execute *action* through an Environment."""
@@ -2822,10 +2821,10 @@ def Pseudo(self, *targets: str | Node | list[str | Node]) -> list[Node]:
28222821
t.set_pseudo()
28232822
return tlist
28242823

2825-
def Repository(self, *dirs: str | DirNode | list[str | DirNode], **kw) -> None:
2824+
def Repository(self, *dirs: str | DirNode | list[str | DirNode]) -> None:
28262825
"""Specify Repository directories to search."""
2827-
dirs = self.arg2nodes(list(dirs), self.fs.Dir)
2828-
self.fs.Repository(*dirs, **kw)
2826+
dirs = self.arg2nodes(dirs, self.fs.Dir)
2827+
self.fs.Repository(*dirs)
28292828

28302829
def Requires(
28312830
self,
@@ -2970,12 +2969,12 @@ def VariantDir(
29702969
29712970
This function creates a mapping from the source directory *src_dir* to the
29722971
variant directory *variant_dir*. If *duplicate* is true (the default),
2973-
the source files are duplicated into the variant directory; if false,
2972+
the source files are duplicated into the variant directory; otherwise
29742973
they are not.
29752974
"""
2976-
variant_dir = self.arg2nodes(variant_dir, self.fs.Dir)[0] # type: ignore[assignment]
2977-
src_dir = self.arg2nodes(src_dir, self.fs.Dir)[0] # type: ignore[assignment]
2978-
self.fs.VariantDir(variant_dir, src_dir, duplicate) # type: ignore[assignment]
2975+
variant_dirs = self.arg2nodes(variant_dir, self.fs.Dir)
2976+
src_dirs = self.arg2nodes(src_dir, self.fs.Dir)
2977+
self.fs.VariantDir(variant_dirs[0], src_dirs[0], duplicate)
29792978

29802979
def FindSourceFiles(self, node: str | Node = ".") -> list[EntryNode]:
29812980
"""Return the list of all source files under *node*."""

SCons/Node/FS.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1570,7 +1570,7 @@ def variant_dir_target_climb(self, orig: Base, dir: DirNode, tail: list[str]) ->
15701570
message = fmt % ' '.join(map(str, targets))
15711571
return targets, message
15721572

1573-
def Glob(self, pathname: str, ondisk: bool = True, source: bool = True, strings: bool = False, exclude: list[str] | None = None, cwd: DirNode | None = None) -> list[Node] | list[str]:
1573+
def Glob(self, pathname: str, ondisk: bool = True, source: bool = True, strings: bool = False, exclude: str | list[str] | None = None, cwd: DirNode | None = None) -> list[Node] | list[str]:
15741574
"""
15751575
Globs
15761576
@@ -2221,7 +2221,7 @@ def walk(self, func, arg) -> None:
22212221
for dirname in [n for n in names if isinstance(entries[n], Dir)]:
22222222
entries[dirname].walk(func, arg)
22232223

2224-
def glob(self, pathname: str, ondisk: bool = True, source: bool = False, strings: bool = False, exclude: list[str] | None = None) -> list[Node] | list[str]:
2224+
def glob(self, pathname: str, ondisk: bool = True, source: bool = False, strings: bool = False, exclude: str | list[str] | None = None) -> list[Node] | list[str]:
22252225
"""Returns a list of Nodes (or strings) matching a pathname pattern.
22262226
22272227
Pathname patterns follow POSIX shell syntax::

SCons/Tool/FortranCommon.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ def isfortran(env, source) -> bool:
5959
return False
6060

6161

62-
def _fortranEmitter(target, source, env) -> Tuple:
62+
def _fortranEmitter(target, source, env) -> tuple:
6363
"""Common code for Fortran emitter.
6464
6565
Called by both the static and shared object emitters,
@@ -86,13 +86,13 @@ def _fortranEmitter(target, source, env) -> Tuple:
8686
return target, source
8787

8888

89-
def FortranEmitter(target, source, env) -> Tuple:
89+
def FortranEmitter(target, source, env) -> tuple:
9090
"""Create emitter for static objects."""
9191
target, source = _fortranEmitter(target, source, env)
9292
return StaticObjectEmitter(target, source, env)
9393

9494

95-
def ShFortranEmitter(target, source, env) -> Tuple:
95+
def ShFortranEmitter(target, source, env) -> tuple:
9696
"""Create emitter for shared objects."""
9797
target, source = _fortranEmitter(target, source, env)
9898
return SharedObjectEmitter(target, source, env)

SCons/Util/__init__.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@
5959
import time
6060
from collections import UserDict, UserList, deque
6161
from contextlib import suppress
62-
from typing import Any
62+
from typing import Any, TypeVar
6363
from logging import Formatter
6464

6565
# Util split into a package. Make sure things that used to work
@@ -113,6 +113,8 @@
113113

114114
PYPY = hasattr(sys, 'pypy_translation_info')
115115

116+
T = TypeVar('T')
117+
116118
# this string will be hashed if a Node refers to a file that doesn't exist
117119
# in order to distinguish from a file that exists but is empty.
118120
NOFILE = "SCONS_MAGIC_MISSING_FILE_STRING"
@@ -539,7 +541,7 @@ def _semi_deepcopy_tuple(obj) -> tuple:
539541
}
540542

541543

542-
def semi_deepcopy(obj):
544+
def semi_deepcopy(obj: T) -> T:
543545
copier = _semi_deepcopy_dispatch.get(type(obj))
544546
if copier:
545547
return copier(obj)

0 commit comments

Comments
 (0)