From 6baf4d424499b275e14a8b4cda1b1b0844b10a77 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Sat, 29 Jun 2024 09:29:08 -0600 Subject: [PATCH] Minor cleanups in Variables Continuing the maintenance pass on Variables, these are some minor tweaks for a few checker niggles and fixing up a couple of docstrings. There is no functional change. Signed-off-by: Mats Wichmann --- SCons/Variables/BoolVariable.py | 4 +-- SCons/Variables/EnumVariable.py | 5 ---- SCons/Variables/EnumVariableTests.py | 34 ++++++++++++------------- SCons/Variables/ListVariable.py | 2 +- SCons/Variables/ListVariableTests.py | 7 +++-- SCons/Variables/PackageVariableTests.py | 2 +- SCons/Variables/PathVariable.py | 2 +- SCons/Variables/PathVariableTests.py | 4 +-- SCons/Variables/VariablesTests.py | 7 ++--- SCons/Variables/__init__.py | 10 ++++---- pyproject.toml | 5 +++- test/Variables/EnumVariable.py | 3 ++- test/Variables/ListVariable.py | 7 +++-- test/Variables/PathVariable.py | 6 ++--- test/Variables/Variables.py | 16 ++++++------ test/Variables/chdir.py | 4 +-- test/Variables/help.py | 2 +- test/Variables/import.py | 4 +-- 18 files changed, 60 insertions(+), 64 deletions(-) diff --git a/SCons/Variables/BoolVariable.py b/SCons/Variables/BoolVariable.py index 4d998e4d9e..e1fe62b905 100644 --- a/SCons/Variables/BoolVariable.py +++ b/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. diff --git a/SCons/Variables/EnumVariable.py b/SCons/Variables/EnumVariable.py index d13e9a9fdb..3698e470dc 100644 --- a/SCons/Variables/EnumVariable.py +++ b/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. diff --git a/SCons/Variables/EnumVariableTests.py b/SCons/Variables/EnumVariableTests.py index c4f32782d6..03848f2bef 100644 --- a/SCons/Variables/EnumVariableTests.py +++ b/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__": diff --git a/SCons/Variables/ListVariable.py b/SCons/Variables/ListVariable.py index bbecbaf948..f795307424 100644 --- a/SCons/Variables/ListVariable.py +++ b/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*. diff --git a/SCons/Variables/ListVariableTests.py b/SCons/Variables/ListVariableTests.py index 62ce879b75..9424509d14 100644 --- a/SCons/Variables/ListVariableTests.py +++ b/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() diff --git a/SCons/Variables/PackageVariableTests.py b/SCons/Variables/PackageVariableTests.py index ed8ec30db4..0d8aa6bc81 100644 --- a/SCons/Variables/PackageVariableTests.py +++ b/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) diff --git a/SCons/Variables/PathVariable.py b/SCons/Variables/PathVariable.py index 4a827c5e12..d5988ac47d 100644 --- a/SCons/Variables/PathVariable.py +++ b/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() diff --git a/SCons/Variables/PathVariableTests.py b/SCons/Variables/PathVariableTests.py index efc75f1878..7fa8da18da 100644 --- a/SCons/Variables/PathVariableTests.py +++ b/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__": diff --git a/SCons/Variables/VariablesTests.py b/SCons/Variables/VariablesTests.py index 7c6eaab171..2c9fe580eb 100644 --- a/SCons/Variables/VariablesTests.py +++ b/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 diff --git a/SCons/Variables/__init__.py b/SCons/Variables/__init__.py index 1337d14f6e..d9df2a26c7 100644 --- a/SCons/Variables/__init__.py +++ b/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", diff --git a/pyproject.toml b/pyproject.toml index 0cdbc7b600..60bc9e607d 100644 --- a/pyproject.toml +++ b/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] diff --git a/test/Variables/EnumVariable.py b/test/Variables/EnumVariable.py index 066df741be..a81e8060be 100644 --- a/test/Variables/EnumVariable.py +++ b/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']) diff --git a/test/Variables/ListVariable.py b/test/Variables/ListVariable.py index 52f1bc56ef..a322f9b8a0 100644 --- a/test/Variables/ListVariable.py +++ b/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, """\ diff --git a/test/Variables/PathVariable.py b/test/Variables/PathVariable.py index 61b21b3573..effbd49dc2 100644 --- a/test/Variables/PathVariable.py +++ b/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') diff --git a/test/Variables/Variables.py b/test/Variables/Variables.py index d585b57a42..e8a1873705 100644 --- a/test/Variables/Variables.py +++ b/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 diff --git a/test/Variables/chdir.py b/test/Variables/chdir.py index ed7cf2be3e..04e10dea54 100644 --- a/test/Variables/chdir.py +++ b/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() diff --git a/test/Variables/help.py b/test/Variables/help.py index 84a405a9d3..e0addb19d2 100644 --- a/test/Variables/help.py +++ b/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') diff --git a/test/Variables/import.py b/test/Variables/import.py index 833b299c9f..8e0cd2595c 100644 --- a/test/Variables/import.py +++ b/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()