Merge pull request SCons#4561 from mwichmann/variables-niggles

Minor cleanups in Variables

SCons/Variables/BoolVariable.py

@@ -38,7 +38,7 @@
 
 __all__ = ['BoolVariable',]
 
-TRUE_STRINGS = ('y', 'yes', 'true', 't', '1', 'on' , 'all')
+TRUE_STRINGS = ('y', 'yes', 'true', 't', '1', 'on', 'all')
 FALSE_STRINGS = ('n', 'no', 'false', 'f', '0', 'off', 'none')
 
 
@@ -66,7 +66,7 @@ def _text2bool(val: str) -> bool:
 def _validator(key, val, env) -> None:
     """Validate that the value of *key* in *env* is a boolean.
 
-    Parmaeter *val* is not used in the check.
+    Parameter *val* is not used in the check.
 
     Usable as a validator function for SCons Variables.
 

SCons/Variables/EnumVariable.py

@@ -91,11 +91,6 @@ def EnumVariable(
        map: optional dictionary which may be used for converting the
           input value into canonical values (e.g. for aliases).
        ignorecase: defines the behavior of the validator and converter.
-       validator: callback function to test whether the value is in the
-          list of allowed values.
-       converter: callback function to convert input values according to
-          the given *map*-dictionary. Unmapped input values are returned
-          unchanged.
 
     Returns:
        A tuple including an appropriate converter and validator.

SCons/Variables/EnumVariableTests.py

@@ -97,9 +97,9 @@ def test_converter(self) -> None:
                                            'c' : 'three'},
                                           ignorecase=2))
 
-        o0 = opts.options[0]
-        o1 = opts.options[1]
-        o2 = opts.options[2]
+        opt0 = opts.options[0]
+        opt1 = opts.options[1]
+        opt2 = opts.options[2]
 
         table = {
             'one'   : ['one',   'one',   'one'],
@@ -119,13 +119,13 @@ def test_converter(self) -> None:
             'C'     : ['C',     'three', 'three'],
         }
 
-        for k, l in table.items():
-            x = o0.converter(k)
-            assert x == l[0], f"o0 got {x}, expected {l[0]}"
-            x = o1.converter(k)
-            assert x == l[1], f"o1 got {x}, expected {l[1]}"
-            x = o2.converter(k)
-            assert x == l[2], f"o2 got {x}, expected {l[2]}"
+        for k, expected in table.items():
+            x = opt0.converter(k)
+            assert x == expected[0], f"opt0 got {x}, expected {expected[0]}"
+            x = opt1.converter(k)
+            assert x == expected[1], f"opt1 got {x}, expected {expected[1]}"
+            x = opt2.converter(k)
+            assert x == expected[2], f"opt2 got {x}, expected {expected[2]}"
 
     def test_validator(self) -> None:
         """Test the EnumVariable validator"""
@@ -149,9 +149,9 @@ def test_validator(self) -> None:
                                            'c' : 'three'},
                                           ignorecase=2))
 
-        o0 = opts.options[0]
-        o1 = opts.options[1]
-        o2 = opts.options[2]
+        opt0 = opts.options[0]
+        opt1 = opts.options[1]
+        opt2 = opts.options[2]
 
         def valid(o, v) -> None:
             o.validator('X', v, {})
@@ -181,10 +181,10 @@ def invalid(o, v) -> None:
             'no_v'  : [invalid, invalid, invalid],
         }
 
-        for v, l in table.items():
-            l[0](o0, v)
-            l[1](o1, v)
-            l[2](o2, v)
+        for v, expected in table.items():
+            expected[0](opt0, v)
+            expected[1](opt1, v)
+            expected[2](opt2, v)
 
 
 if __name__ == "__main__":

SCons/Variables/ListVariable.py

@@ -140,7 +140,7 @@ def _validator(key, val, env) -> None:
     so we need to fish the allowed elements list out of the environment
     to complete the validation.
 
-    Note that since 18b45e456, whether or not ``subst`` has been
+    Note that since 18b45e456, whether ``subst`` has been
     called is conditional on the value of the *subst* argument to
     :meth:`~SCons.Variables.Variables.Add`, so we have to account for
     possible different types of *val*.

SCons/Variables/ListVariableTests.py

@@ -112,7 +112,7 @@ def test_converter(self) -> None:
         assert str(x) == 'no_match', x
         # ... and fail to validate
         with self.assertRaises(SCons.Errors.UserError):
-            z = o.validator('test', 'no_match', {"test": x})
+            o.validator('test', 'no_match', {"test": x})
 
     def test_copy(self) -> None:
         """Test copying a ListVariable like an Environment would"""
@@ -121,9 +121,8 @@ def test_copy(self) -> None:
                                           ['one', 'two', 'three']))
 
         o = opts.options[0]
-
-        l = o.converter('all')
-        n = l.__class__(copy.copy(l))
+        res = o.converter('all')
+        _ = res.__class__(copy.copy(res))
 
 if __name__ == "__main__":
     unittest.main()

SCons/Variables/PackageVariableTests.py

@@ -101,7 +101,7 @@ def test_validator(self) -> None:
         o.validator('T', '/path', env)
         o.validator('X', exists, env)
 
-        with self.assertRaises(SCons.Errors.UserError) as cm:
+        with self.assertRaises(SCons.Errors.UserError):
             o.validator('X', does_not_exist, env)
 
 

SCons/Variables/PathVariable.py

@@ -161,7 +161,7 @@ def __call__(
             helpmsg = f'{help} ( /path/to/{key[0]} )'
         else:
             helpmsg = f'{help} ( /path/to/{key} )'
-        return (key, helpmsg, default, validator, None)
+        return key, helpmsg, default, validator, None
 
 
 PathVariable = _PathVariableClass()

SCons/Variables/PathVariableTests.py

@@ -196,7 +196,7 @@ class ValidatorError(Exception):
             pass
 
         def my_validator(key, val, env):
-            raise ValidatorError(f"my_validator() got called for {key!r}, {val}!")
+            raise ValidatorError(f"my_validator() got called for {key!r}, {val!r}!")
 
         opts = SCons.Variables.Variables()
         opts.Add(SCons.Variables.PathVariable('test2',
@@ -207,7 +207,7 @@ def my_validator(key, val, env):
         with self.assertRaises(ValidatorError) as cm:
             o.validator('Y', 'value', {})
         e = cm.exception
-        self.assertEqual(str(e), f"my_validator() got called for 'Y', value!")
+        self.assertEqual(str(e), "my_validator() got called for 'Y', 'value'!")
 
 
 if __name__ == "__main__":

SCons/Variables/VariablesTests.py

@@ -206,7 +206,6 @@ def conv_subst(value) -> None:
                  lambda x: int(x) + 12)
 
         env = Environment()
-        exc_caught = None
         with self.assertRaises(AssertionError):
             opts.Update(env)
 
@@ -375,8 +374,10 @@ def test_Save(self) -> None:
         opts = SCons.Variables.Variables()
 
         def bool_converter(val):
-            if val in [1, 'y']: val = 1
-            if val in [0, 'n']: val = 0
+            if val in [1, 'y']:
+                val = 1
+            if val in [0, 'n']:
+                val = 0
             return val
 
         # test saving out empty file

SCons/Variables/__init__.py

@@ -34,11 +34,11 @@
 
 # Note: imports are for the benefit of SCons.Main (and tests); since they
 #   are not used here, the "as Foo" form is for checkers.
-from .BoolVariable import BoolVariable as BoolVariable
-from .EnumVariable import EnumVariable as EnumVariable
-from .ListVariable import ListVariable as ListVariable
-from .PackageVariable import PackageVariable as PackageVariable
-from .PathVariable import PathVariable as PathVariable
+from .BoolVariable import BoolVariable
+from .EnumVariable import EnumVariable
+from .ListVariable import ListVariable
+from .PackageVariable import PackageVariable
+from .PathVariable import PathVariable
 
 __all__ = [
     "Variable",

pyproject.toml

@@ -88,7 +88,10 @@ quote-style = "preserve" # Equivalent to black's "skip-string-normalization"
 
 [tool.ruff.lint.per-file-ignores]
 "SCons/Util/__init__.py" = [
-    "F401", # Module imported but unused
+    "F401",  # Module imported but unused
+]
+"SCons/Variables/__init__.py" = [
+    "F401",  # Symbol imported but unused
 ]
 
 [tool.mypy]

test/Variables/EnumVariable.py

@@ -67,7 +67,8 @@ def check(expect):
 Default(env.Alias('dummy', None))
 """)
 
-test.run(); check(['no', 'gtk', 'xaver'])
+test.run()
+check(['no', 'gtk', 'xaver'])
 
 test.run(arguments='debug=yes guilib=Motif some=xAVER')
 check(['yes', 'Motif', 'xaver'])

test/Variables/ListVariable.py

@@ -35,11 +35,10 @@
 
 SConstruct_path = test.workpath('SConstruct')
 
-def check(expect):
+def check(expected):
     result = test.stdout().split('\n')
-    r = result[1:len(expect)+1]
-    assert r == expect, (r, expect)
-
+    r = result[1:len(expected)+1]
+    assert r == expected, (r, expected)
 
 
 test.write(SConstruct_path, """\

test/Variables/PathVariable.py

@@ -40,11 +40,9 @@ def check(expect):
     result = test.stdout().split('\n')
     assert result[1:len(expect)+1] == expect, (result[1:len(expect)+1], expect)
 
-#### test PathVariable ####
 
-test.subdir('lib', 'qt', ['qt', 'lib'], 'nolib' )
+test.subdir('lib', 'qt', ['qt', 'lib'], 'nolib')
 workpath = test.workpath()
-libpath = os.path.join(workpath, 'lib')
 
 test.write(SConstruct_path, """\
 from SCons.Variables.PathVariable import PathVariable as PV
@@ -67,7 +65,7 @@ def check(expect):
 print(env.subst('$qt_libraries'))
 
 Default(env.Alias('dummy', None))
-""" % (workpath, os.path.join('$qtdir', 'lib') ))
+""" % (workpath, os.path.join('$qtdir', 'lib')))
 
 qtpath = workpath
 libpath = os.path.join(qtpath, 'lib')

test/Variables/Variables.py

@@ -165,7 +165,7 @@ def check(expect):
 test.run(arguments='DEBUG_BUILD=1')
 check(['1', '1', cc, (ccflags + ' -O -g').strip(), 'v', 'v'])
 
-test.run(arguments='-h', stdout = """\
+test.run(arguments='-h', stdout="""\
 scons: Reading SConscript files ...
 1
 0
@@ -241,7 +241,7 @@ def checkSave(file, expected):
     gdict = {}
     ldict = {}
     with open(file, 'r') as f:
-       contents = f.read()
+        contents = f.read()
     exec(contents, gdict, ldict)
     assert expected == ldict, "%s\n...not equal to...\n%s" % (expected, ldict)
 
@@ -297,18 +297,18 @@ def checkSave(file, expected):
 
 # First check for empty output file when nothing is passed on command line
 test.run()
-check(['0','1'])
+check(['0', '1'])
 checkSave('variables.saved', {})
 
 # Now specify one option the same as default and make sure it doesn't write out
 test.run(arguments='DEBUG_BUILD=1')
-check(['0','1'])
+check(['0', '1'])
 checkSave('variables.saved', {})
 
 # Now specify same option non-default and make sure only it is written out
 test.run(arguments='DEBUG_BUILD=0 LISTOPTION_TEST=a,b')
-check(['0','0'])
-checkSave('variables.saved',{'DEBUG_BUILD':0, 'LISTOPTION_TEST':'a,b'})
+check(['0', '0'])
+checkSave('variables.saved', {'DEBUG_BUILD': 0, 'LISTOPTION_TEST': 'a,b'})
 
 test.write('SConstruct', """
 opts = Variables('custom.py')
@@ -342,7 +342,7 @@ def compare(a, b):
 )
 """)
 
-test.run(arguments='-h', stdout = """\
+test.run(arguments='-h', stdout="""\
 scons: Reading SConscript files ...
 scons: done reading SConscript files.
 Variables settable in custom.py or on the command line:
@@ -364,7 +364,7 @@ def compare(a, b):
     actual: None
 
 Use scons -H for help about SCons built-in command-line options.
-"""%cc)
+""" % cc)
 
 test.write('SConstruct', """
 import SCons.Variables

test/Variables/chdir.py

@@ -24,7 +24,7 @@
 # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 """
-Verify that we can chdir() to the directory in which an Variables
+Verify that we can chdir() to the directory in which a Variables
 file lives by using the __name__ value.
 """
 
@@ -67,7 +67,7 @@
 VARIABLE = 'opts2.cfg value'
 """
 
-test.run(arguments = '-q -Q .', stdout=expect)
+test.run(arguments='-q -Q .', stdout=expect)
 
 test.pass_test()
 

test/Variables/help.py

@@ -37,7 +37,7 @@
 test = TestSCons.TestSCons()
 
 workpath = test.workpath()
-qtpath  = os.path.join(workpath, 'qt')
+qtpath = os.path.join(workpath, 'qt')
 libpath = os.path.join(qtpath, 'lib')
 libdirvar = os.path.join('$qtdir', 'lib')
 

test/Variables/import.py

@@ -24,7 +24,7 @@
 # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 """
-Verify that an Variables file in a different directory can import
+Verify that a Variables file in a different directory can import
 a module in that directory.
 """
 
@@ -63,7 +63,7 @@
 
 expect = "VARIABLE = bin/local_options.py\n"
 
-test.run(arguments = '-q -Q .', stdout = expect)
+test.run(arguments='-q -Q .', stdout=expect)
 
 test.pass_test()