Merge pull request #4658 from mwichmann/maint/Variable-dataclass

Turn `SCons.Variables.Variable` into a dataclass

CHANGES.txt

@@ -167,6 +167,13 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
       their values taken from the default in the variable description
       (if a variable was set to the same value as the default in one
       of the input sources, it is not included in this list).
+    - If a build Variable is created with no aliases, the name of the
+      Variable is no longer listed in its aliases. Internally, the name
+      and aliases are considered together anyway so this should not have
+      any effect except for being visible to custom help text formatters.
+    - A build Variable is now a dataclass, with initialization moving to
+      the automatically provided method; the Variables class no longer
+      writes directly to a Variable (makes static checkers happier).
     - The (optional) C Conditional Scanner now does limited macro
       replacement on the contents of CPPDEFINES, to improve finding deps
       that are conditionally included.  Previously replacement was only

RELEASE.txt

@@ -82,6 +82,11 @@ CHANGED/ENHANCED EXISTING FUNCTIONALITY
   (if a variable was set to the same value as the default in one
   of the input sources, it is not included in this list).
 
+- If a build Variable is created with no aliases, the name of the
+  Variable is no longer listed in its aliases. Internally, the name
+  and aliases are considered together anyway so this should not have
+  any effect except for being visible to custom help text formatters.
+
 FIXES
 -----
 

SCons/Variables/VariablesTests.py

@@ -550,7 +550,7 @@ def my_format(env, opt, help, default, actual, aliases) -> str:
                  check,
                  lambda x: int(x) + 12)
 
-        opts.Add('B',
+        opts.Add(['B', 'BOPTION'],
                  'b - alpha test',
                  "42",
                  check,
@@ -566,19 +566,19 @@ def my_format(env, opt, help, default, actual, aliases) -> str:
         opts.Update(env, {})
 
         expect = """\
-ANSWER 42 54 THE answer to THE question ['ANSWER']
-B 42 54 b - alpha test ['B']
-A 42 54 a - alpha test ['A']
+ANSWER 42 54 THE answer to THE question []
+B 42 54 b - alpha test ['BOPTION']
+A 42 54 a - alpha test []
 """
 
         text = opts.GenerateHelpText(env)
         with self.subTest():
             self.assertEqual(expect, text)
 
         expectAlpha = """\
-A 42 54 a - alpha test ['A']
-ANSWER 42 54 THE answer to THE question ['ANSWER']
-B 42 54 b - alpha test ['B']
+A 42 54 a - alpha test []
+ANSWER 42 54 THE answer to THE question []
+B 42 54 b - alpha test ['BOPTION']
 """
         text = opts.GenerateHelpText(env, sort=cmp)
         with self.subTest():

SCons/Variables/__init__.py

@@ -28,8 +28,9 @@
 import os.path
 import sys
 from contextlib import suppress
+from dataclasses import dataclass
 from functools import cmp_to_key
-from typing import Callable, Sequence
+from typing import Any, Callable, Sequence
 
 import SCons.Errors
 import SCons.Util
@@ -53,22 +54,18 @@
     "PathVariable",
 ]
 
+
+@dataclass(order=True)
 class Variable:
     """A Build Variable."""
-
     __slots__ = ('key', 'aliases', 'help', 'default', 'validator', 'converter', 'do_subst')
-
-    def __lt__(self, other):
-        """Comparison fuction so :class:`Variable` instances sort."""
-        return self.key < other.key
-
-    def __str__(self) -> str:
-        """Provide a way to "print" a :class:`Variable` object."""
-        return (
-            f"({self.key!r}, {self.aliases}, "
-            f"help={self.help!r}, default={self.default!r}, "
-            f"validator={self.validator}, converter={self.converter})"
-        )
+    key: str
+    aliases: list[str]
+    help: str
+    default: Any
+    validator: Callable | None
+    converter: Callable | None
+    do_subst: bool
 
 
 class Variables:
@@ -131,7 +128,7 @@ def __str__(self) -> str:
     # lint: W0622: Redefining built-in 'help'
     def _do_add(
         self,
-        key: str | list[str],
+        key: str | Sequence[str],
         help: str = "",
         default=None,
         validator: Callable | None = None,
@@ -146,30 +143,19 @@ def _do_add(
         .. versionadded:: 4.8.0
               *subst* keyword argument is now recognized.
         """
-        option = Variable()
-
-        # If we get a list or a tuple, we take the first element as the
-        # option key and store the remaining in aliases.
+        # aliases needs to be a list for later concatenation operations
         if SCons.Util.is_Sequence(key):
-            option.key = key[0]
-            option.aliases = list(key[1:])
+            name, aliases = key[0], list(key[1:])
         else:
-            option.key = key
-            # TODO: normalize to not include key in aliases. Currently breaks tests.
-            option.aliases = [key,]
-        if not option.key.isidentifier():
-            raise SCons.Errors.UserError(f"Illegal Variables key {option.key!r}")
-        option.help = help
-        option.default = default
-        option.validator = validator
-        option.converter = converter
-        option.do_subst = kwargs.pop("subst", True)
-        # TODO should any remaining kwargs be saved in the Variable?
-
+            name, aliases = key, []
+        if not name.isidentifier():
+            raise SCons.Errors.UserError(f"Illegal Variables key {name!r}")
+        do_subst = kwargs.pop("subst", True)
+        option = Variable(name, aliases, help, default, validator, converter, do_subst)
         self.options.append(option)
 
-        # options might be added after the 'unknown' dict has been set up,
-        # so we remove the key and all its aliases from that dict
+        # options might be added after the 'unknown' dict has been set up:
+        # look for and remove the key and all its aliases from that dict
         for alias in option.aliases + [option.key,]:
             if alias in self.unknown:
                 del self.unknown[alias]