Use OptionStore as the key rather than a string.optionrefactor2

2 files changed, 109 insertions, 85 deletions

diff --git a/mesonbuild/options.py b/mesonbuild/options.py
index f2cc63c..7a83b92 100644
--- a/mesonbuild/options.py
+++ b/mesonbuild/options.py
@@ -680,6 +680,8 @@ OPTNAME_AND_VALUE_REGEX = OPTNAME_REGEX + r'=(?P<value>.*)'
 OPTNAME_SPLITTER = re.compile(OPTNAME_REGEX)
 OPTNAME_AND_VALUE_SPLITTER = re.compile(OPTNAME_AND_VALUE_REGEX)
 
+BAD_VALUE = 'Qwert Zuiopü'
+
 class OptionParts:
     def __init__(self, name, subproject=None, for_build=False):
         self.name = name
@@ -693,6 +695,11 @@ class OptionParts:
         if isinstance(other, OptionParts):
             return self.name == other.name and self.subproject == other.subproject and self.for_build == other.for_build
 
+    def copy_with(self, *, name=BAD_VALUE, subproject=BAD_VALUE, for_build=BAD_VALUE):
+        return OptionParts(name if name != BAD_VALUE else self.name,
+                           subproject if subproject != BAD_VALUE else self.subproject, # None is a valid value so it can'the default value in method declaration.
+                           for_build if for_build != BAD_VALUE else self.for_build)
+
 class OptionStore:
     def __init__(self):
         self.d: T.Dict['OptionKey', 'UserOption[T.Any]'] = {}
@@ -711,26 +718,24 @@ class OptionStore:
         build = len(self.build_options) if self.build_options else 0
         return basic + build
 
-    def has_option(self, name, subproject, for_build=False):
-        cname = self.form_canonical_keystring(name, subproject, for_build)
-        return cname in self.options
+    def has_option(self, optparts):
+        assert isinstance(optparts, OptionParts)
+        return optparts in self.options
 
-    def set_option(self, name, subproject, new_value):
-        cname = self.form_canonical_keystring(name)
-        return self.set_option_from_string(cname, new_value)
+    def set_option(self, optparts, new_value):
+        assert isinstance(optparts, OptionParts)
+        return self.options[optparts].set_value(new_value)
 
     def set_option_from_string(self, keystr, new_value):
-        m = re.fullmatch(OPTNAME_REGEX, keystr) # Merely for validation
-        return self.options[keystr].set_value(new_value)
-
-    def parts_to_canonical_keystring(self, parts):
-        return self.form_canonical_keystring(parts.name, parts.subproject, parts.for_build)
-
-    def form_canonical_keystring(self, name, subproject=None, for_build=None):
-        strname = name
-        if subproject is not None:
-            strname = f'{subproject}:{strname}'
-        if for_build:
+        o = self.split_keystring(keystr)
+        return self.options[o].set_value(new_value)
+
+    def form_canonical_keystring(self, optparts):
+        assert isinstance(optparts, OptionParts)
+        strname = optparts.name
+        if optparts.subproject is not None:
+            strname = f'{optparts.subproject}:{strname}'
+        if optparts.for_build:
             strname = 'build.' + strname
         return strname
 
@@ -746,29 +751,30 @@ class OptionStore:
 
     def canonicalize_keystring(self, keystr):
         parts = self.split_keystring(keystr)
-        return self.form_canonical_keystring(parts.name, parts.subproject, parts.for_build)
+        return self.form_canonical_keystring(parts)
 
     def add_system_option(self, name, value_object):
         assert isinstance(name, str)
-        cname = self.form_canonical_keystring(name)
+        k = OptionParts(name)
         # FIXME; transfer the old value for combos etc.
-        if cname not in self.options:
-            self.options[cname] = value_object
+        if k not in self.options:
+            self.options[k] = value_object
 
     def add_project_option(self, name, subproject, value_object):
-        cname = self.form_canonical_keystring(name, subproject)
-        self.options[cname] = value_object
-        self.project_options.add(cname)
+        assert isinstance(name, str)
+        k = OptionParts(name, subproject)
+        self.options[k] = value_object
+        self.project_options.add(k)
 
-    def get_value_object_for(self, name, subproject=None):
-        cname = self.form_canonical_keystring(name, subproject)
-        potential = self.options.get(cname, None)
+    def get_value_object_for(self, optioninfo, subproject=None):
+        assert isinstance(optioninfo, OptionParts)
+        potential = self.options.get(optioninfo, None)
         if potential is None:
-            top_cname = self.form_canonical_keystring(name)
-            return self.options[top_cname]
+            top_option = optioninfo.copy_with(subproject=None)
+            return self.options[top_option]
         if potential.yielding:
-            top_cname = self.form_canonical_keystring(name, '')
-            return self.options.get(top_cname, potential)
+            top_option = optioninfo.copy_with(subproject='')
+            return self.options.get(top_option, potential)
         return potential
 
     def add_system_option(self, key: T.Union[OptionKey, str], valobj: 'UserOption[T.Any]'):
@@ -885,34 +891,37 @@ class OptionStore:
     def is_module_option(self, key: OptionKey) -> bool:
         return key in self.module_options
 
-    def get_value_for(self, name, subproject=None):
-        vobject = self.get_value_object_for(name, subproject)
-        cname = self.form_canonical_keystring(name, subproject)
-        if cname in self.augments:
-            return vobject.validate_value(self.augments[cname])
+    def get_value_for(self, optioninfo):
+        assert isinstance(optioninfo, OptionParts)
+        vobject = self.get_value_object_for(optioninfo)
+        if optioninfo in self.augments:
+            return vobject.validate_value(self.augments[optioninfo])
         return vobject.value
 
     def set_from_configure_command(self, D, A, U):
         for setval in D:
             keystr, valstr = setval.split('=', 1)
-            if keystr in self.augments:
-                self.augments[keystr] = valstr
+            key = self.split_keystring(keystr)
+            if key in self.augments:
+                self.augments[key] = valstr
             else:
                 self.set_option_from_string(keystr, valstr)
         for add in A:
             keystr, valstr = add.split('=', 1)
-            keystr = self.canonicalize_keystring(keystr)
-            if keystr in self.augments:
+            key = self.split_keystring(keystr)
+            if key in self.augments:
                 raise MesonException(f'Tried to add augment to option {keystr}, which already has an augment. Set it with -D instead.')
-            self.augments[keystr] = valstr
+            self.augments[key] = valstr
         for delete in U:
-            delete = self.canonicalize_keystring(delete)
+            delete = self.split_keystring(delete)
             if delete in self.augments:
                 del self.augments[delete]
 
     def set_subproject_options(self, subproject, spcall_default_options, project_default_options):
         for o in itertools.chain(spcall_default_options, project_default_options):
             keystr, valstr = o.split('=', 1)
-            keystr = f'{subproject}:{keystr}'
-            if keystr not in self.augments:
-                self.augments[keystr] = valstr
+            key = self.split_keystring(keystr)
+            assert key.subproject is None
+            key.subproject = subproject
+            if key not in self.augments:
+                self.augments[key] = valstr
diff --git a/unittests/optiontests.py b/unittests/optiontests.py
index 98c7054..04739a0 100644
--- a/unittests/optiontests.py
+++ b/unittests/optiontests.py
@@ -13,50 +13,54 @@ class OptionTests(unittest.TestCase):
         name = 'someoption'
         default_value = 'somevalue'
         new_value = 'new_value'
-        vo = UserStringOption(name, 'An option of some sort', default_value)
-        optstore.add_system_option(name, vo)
-        self.assertEqual(optstore.get_value_for(name), default_value)
-        optstore.set_option(name, '', new_value)
-        self.assertEqual(optstore.get_value_for(name), new_value)
+        k = OptionParts(name)
+        vo = UserStringOption(k, 'An option of some sort', default_value)
+        optstore.add_system_option(k.name, vo)
+        self.assertEqual(optstore.get_value_for(k), default_value)
+        optstore.set_option(k, new_value)
+        self.assertEqual(optstore.get_value_for(k), new_value)
 
     def test_parsing(self):
         optstore = OptionStore()
         s1 = optstore.split_keystring('sub:optname')
         s1_expected = OptionParts('optname', 'sub', False)
         self.assertEqual(s1, s1_expected)
-        self.assertEqual(optstore.parts_to_canonical_keystring(s1), 'sub:optname')
+        self.assertEqual(optstore.form_canonical_keystring(s1), 'sub:optname')
 
         s2 = optstore.split_keystring('optname')
         s2_expected = OptionParts('optname', None, False)
         self.assertEqual(s2, s2_expected)
 
-        self.assertEqual(optstore.parts_to_canonical_keystring(s2), 'optname')
+        self.assertEqual(optstore.form_canonical_keystring(s2), 'optname')
 
         s3 = optstore.split_keystring(':optname')
         s3_expected = OptionParts('optname', '', False)
         self.assertEqual(s3, s3_expected)
-        self.assertEqual(optstore.parts_to_canonical_keystring(s3), ':optname')
+        self.assertEqual(optstore.form_canonical_keystring(s3), ':optname')
 
     def test_subproject_for_system(self):
         optstore = OptionStore()
         name = 'someoption'
+        key = OptionParts(name)
+        subkey = key.copy_with(subproject='somesubproject')
         default_value = 'somevalue'
         vo = UserStringOption(name, 'An option of some sort', default_value)
         optstore.add_system_option(name, vo)
-        self.assertEqual(optstore.get_value_for(name, 'somesubproject'), default_value)
+        self.assertEqual(optstore.get_value_for(subkey), default_value)
 
     def test_reset(self):
         optstore = OptionStore()
         name = 'someoption'
         original_value = 'original'
         reset_value = 'reset'
+        k = OptionParts(name)
         vo = UserStringOption(name, 'An option set twice', original_value)
         optstore.add_system_option(name, vo)
-        self.assertEqual(optstore.get_value_for(name), original_value)
+        self.assertEqual(optstore.get_value_for(k), original_value)
         self.assertEqual(optstore.num_options(), 1)
         vo2 = UserStringOption(name, 'An option set twice', reset_value)
         optstore.add_system_option(name, vo2)
-        self.assertEqual(optstore.get_value_for(name), original_value)
+        self.assertEqual(optstore.get_value_for(k), original_value)
         self.assertEqual(optstore.num_options(), 1)
 
     def test_project_nonyielding(self):
@@ -64,14 +68,16 @@ class OptionTests(unittest.TestCase):
         name = 'someoption'
         top_value = 'top'
         sub_value = 'sub'
+        top_key = OptionParts(name, '')
+        sub_key = top_key.copy_with(subproject='sub')
         vo = UserStringOption(name, 'A top level option', top_value, False)
         optstore.add_project_option(name, '', vo)
-        self.assertEqual(optstore.get_value_for(name, ''), top_value, False)
+        self.assertEqual(optstore.get_value_for(top_key), top_value, False)
         self.assertEqual(optstore.num_options(), 1)
         vo2 = UserStringOption(name, 'A subproject option', sub_value)
         optstore.add_project_option(name, 'sub', vo2)
-        self.assertEqual(optstore.get_value_for(name, ''), top_value)
-        self.assertEqual(optstore.get_value_for(name, 'sub'), sub_value)
+        self.assertEqual(optstore.get_value_for(top_key), top_value)
+        self.assertEqual(optstore.get_value_for(sub_key), sub_value)
         self.assertEqual(optstore.num_options(), 2)
 
     def test_project_yielding(self):
@@ -80,13 +86,15 @@ class OptionTests(unittest.TestCase):
         top_value = 'top'
         sub_value = 'sub'
         vo = UserStringOption(name, 'A top level option', top_value)
+        top_key = OptionParts(name, '')
+        sub_key = top_key.copy_with(subproject='sub')
         optstore.add_project_option(name, '', vo)
-        self.assertEqual(optstore.get_value_for(name, ''), top_value)
+        self.assertEqual(optstore.get_value_for(top_key), top_value)
         self.assertEqual(optstore.num_options(), 1)
         vo2 = UserStringOption(name, 'A subproject option', sub_value, True)
         optstore.add_project_option(name, 'sub', vo2)
-        self.assertEqual(optstore.get_value_for(name, ''), top_value)
-        self.assertEqual(optstore.get_value_for(name, 'sub'), top_value)
+        self.assertEqual(optstore.get_value_for(top_key), top_value)
+        self.assertEqual(optstore.get_value_for(sub_key), top_value)
         self.assertEqual(optstore.num_options(), 2)
 
     def test_augments(self):
@@ -96,45 +104,50 @@ class OptionTests(unittest.TestCase):
         sub2_name = 'sub2'
         top_value = 'c++11'
         aug_value = 'c++23'
+        top_key = OptionParts(name)
+        topsub_key = top_key.copy_with(subproject='')
+        sub_key = top_key.copy_with(subproject=sub_name)
+        sub2_key = top_key.copy_with(subproject=sub2_name)
 
         co = UserComboOption(name,
                              'C++ language standard to use',
                              ['c++98', 'c++11', 'c++14', 'c++17', 'c++20', 'c++23'],
                              top_value)
         optstore.add_system_option(name, co)
-        self.assertEqual(optstore.get_value_for(name), top_value)
-        self.assertEqual(optstore.get_value_for(name, sub_name), top_value)
-        self.assertEqual(optstore.get_value_for(name, sub2_name), top_value)
+        self.assertEqual(optstore.get_value_for(top_key), top_value)
+        self.assertEqual(optstore.get_value_for(sub_key), top_value)
+        self.assertEqual(optstore.get_value_for(sub2_key), top_value)
 
         # First augment a subproject
         optstore.set_from_configure_command([], [f'{sub_name}:{name}={aug_value}'], [])
-        self.assertEqual(optstore.get_value_for(name), top_value)
-        self.assertEqual(optstore.get_value_for(name, sub_name), aug_value)
-        self.assertEqual(optstore.get_value_for(name, sub2_name), top_value)
+        self.assertEqual(optstore.get_value_for(top_key), top_value)
+        self.assertEqual(optstore.get_value_for(sub_key), aug_value)
+        self.assertEqual(optstore.get_value_for(sub2_key), top_value)
         optstore.set_from_configure_command([], [], [f'{sub_name}:{name}'])
-        self.assertEqual(optstore.get_value_for(name), top_value)
-        self.assertEqual(optstore.get_value_for(name, sub_name), top_value)
-        self.assertEqual(optstore.get_value_for(name, sub2_name), top_value)
+        self.assertEqual(optstore.get_value_for(top_key), top_value)
+        self.assertEqual(optstore.get_value_for(sub_key), top_value)
+        self.assertEqual(optstore.get_value_for(sub2_key), top_value)
 
         # And now augment the top level option
         optstore.set_from_configure_command([], [f':{name}={aug_value}'], [])
-        self.assertEqual(optstore.get_value_for(name, None), top_value)
-        self.assertEqual(optstore.get_value_for(name, ''), aug_value)
-        self.assertEqual(optstore.get_value_for(name, sub_name), top_value)
-        self.assertEqual(optstore.get_value_for(name, sub2_name), top_value)
+        self.assertEqual(optstore.get_value_for(top_key), top_value)
+        self.assertEqual(optstore.get_value_for(topsub_key), aug_value)
+        self.assertEqual(optstore.get_value_for(sub_key), top_value)
+        self.assertEqual(optstore.get_value_for(sub2_key), top_value)
         optstore.set_from_configure_command([], [], [f':{name}'])
-        self.assertEqual(optstore.get_value_for(name), top_value)
-        self.assertEqual(optstore.get_value_for(name, sub_name), top_value)
-        self.assertEqual(optstore.get_value_for(name, sub2_name), top_value)
+        self.assertEqual(optstore.get_value_for(top_key), top_value)
+        self.assertEqual(optstore.get_value_for(sub_key), top_value)
+        self.assertEqual(optstore.get_value_for(sub2_key), top_value)
 
     def test_augment_set_sub(self):
         optstore = OptionStore()
         name = 'cpp_std'
         sub_name = 'sub'
-        sub2_name = 'sub2'
         top_value = 'c++11'
         aug_value = 'c++23'
         set_value = 'c++20'
+        top_key = OptionParts(name=name)
+        sub_key = top_key.copy_with(subproject=sub_name)
 
         co = UserComboOption(name,
                              'C++ language standard to use',
@@ -143,8 +156,8 @@ class OptionTests(unittest.TestCase):
         optstore.add_system_option(name, co)
         optstore.set_from_configure_command([], [f'{sub_name}:{name}={aug_value}'], [])
         optstore.set_from_configure_command([f'{sub_name}:{name}={set_value}'], [], [])
-        self.assertEqual(optstore.get_value_for(name), top_value)
-        self.assertEqual(optstore.get_value_for(name, sub_name), set_value)
+        self.assertEqual(optstore.get_value_for(top_key), top_value)
+        self.assertEqual(optstore.get_value_for(sub_key), set_value)
 
     def test_subproject_call_options(self):
         optstore = OptionStore()
@@ -160,10 +173,12 @@ class OptionTests(unittest.TestCase):
                              default_value)
         optstore.add_system_option(name, co)
         optstore.set_subproject_options(subproject, [f'cpp_std={override_value}'], [f'cpp_std={unused_value}'])
-        self.assertEqual(optstore.get_value_for(name), default_value)
-        self.assertEqual(optstore.get_value_for(name, subproject), override_value)
+        k = OptionParts(name)
+        sub_k = k.copy_with(subproject=subproject)
+        self.assertEqual(optstore.get_value_for(k), default_value)
+        self.assertEqual(optstore.get_value_for(sub_k), override_value)
 
         # Trying again should change nothing
         optstore.set_subproject_options(subproject, [f'cpp_std={unused_value}'], [f'cpp_std={unused_value}'])
-        self.assertEqual(optstore.get_value_for(name), default_value)
-        self.assertEqual(optstore.get_value_for(name, subproject), override_value)
+        self.assertEqual(optstore.get_value_for(k), default_value)
+        self.assertEqual(optstore.get_value_for(sub_k), override_value)