Allow nested $( $) sections

3 files changed, 49 insertions, 11 deletions

diff --git a/src/engine/SCons/ActionTests.py b/src/engine/SCons/ActionTests.py
index b790ccc..f0c48ea 100644
--- a/src/engine/SCons/ActionTests.py
+++ b/src/engine/SCons/ActionTests.py
@@ -1241,8 +1241,8 @@ class CommandActionTestCase(unittest.TestCase):
                    (env["foo"], env["bar"])
 
         # The number 1 is there to make sure all args get converted to strings.
-        a = SCons.Action.CommandAction(["|", "$(", "$foo", "|", "$bar",
-                                        "$)", "|", "$baz", 1])
+        a = SCons.Action.CommandAction(["|", "$(", "$foo", "|", "$(", "$bar",
+                                        "$)", "stuff",  "$)", "|", "$baz", 1])
         c = a.get_contents(target=[], source=[],
                            env=Environment(foo = 'FFF', bar = 'BBB',
                                            baz = CmdGen))
@@ -1257,7 +1257,7 @@ class CommandActionTestCase(unittest.TestCase):
         c = a.get_contents(target=DummyNode('ttt'), source = DummyNode('sss'),
                            env=SpecialEnvironment(foo = 'GGG', bar = 'CCC',
                                                   baz = 'ZZZ'))
-        assert c == b'subst_target_source: | $( $foo | $bar $) | $baz 1', c
+        assert c == b'subst_target_source: | $( $foo | $( $bar $) stuff $) | $baz 1', c
 
         # We've discussed using the real target and source names in a
         # CommandAction's signature contents.  This would have have the
diff --git a/src/engine/SCons/Subst.py b/src/engine/SCons/Subst.py
index 3c9b390..b81d79c 100644
--- a/src/engine/SCons/Subst.py
+++ b/src/engine/SCons/Subst.py
@@ -338,24 +338,28 @@ SUBST_RAW = 1
 SUBST_SIG = 2
 
 _rm = re.compile(r'\$[()]')
-_remove = re.compile(r'\$\([^\$]*(\$[^\)][^\$]*)*\$\)')
+_rm_split = re.compile(r'(\$[()])')
 
 # Indexed by the SUBST_* constants above.
-_regex_remove = [ _rm, None, _remove ]
+_regex_remove = [ _rm, None, _rm_split ]
 
 def _rm_list(list):
     return [l for l in list if not l in ('$(', '$)')]
 
 def _remove_list(list):
     result = []
-    do_append = result.append
+    depth = 0
     for l in list:
         if l == '$(':
-            do_append = lambda x: None
+            depth += 1
         elif l == '$)':
-            do_append = result.append
-        else:
-            do_append(l)
+            depth -= 1
+            if depth < 0:
+                break
+        elif depth == 0:
+            result.append(l)
+    if depth != 0:
+        return None
     return result
 
 # Indexed by the SUBST_* constants above.
@@ -562,12 +566,19 @@ def scons_subst(strSubst, env, mode=SUBST_RAW, target=None, source=None, gvars={
     except KeyError:
         pass
 
+    res = result
     if is_String(result):
         # Remove $(-$) pairs and any stuff in between,
         # if that's appropriate.
         remove = _regex_remove[mode]
         if remove:
-            result = remove.sub('', result)
+            if mode == SUBST_SIG:
+                result = _list_remove[mode](remove.split(result))
+                if result is None:
+                    raise SCons.Errors.UserError("Unbalanced $(/$) in: " + res)
+                result = ' '.join(result)
+            else:
+                result = remove.sub('', result)
         if mode != SUBST_RAW:
             # Compress strings of white space characters into
             # a single space.
@@ -576,6 +587,8 @@ def scons_subst(strSubst, env, mode=SUBST_RAW, target=None, source=None, gvars={
         remove = _list_remove[mode]
         if remove:
             result = remove(result)
+            if result is None:
+                raise SCons.Errors.UserError("Unbalanced $(/$) in: " + str(res))
 
     return result
 
diff --git a/src/engine/SCons/SubstTests.py b/src/engine/SCons/SubstTests.py
index 8eda845..f2cbc3f 100644
--- a/src/engine/SCons/SubstTests.py
+++ b/src/engine/SCons/SubstTests.py
@@ -183,6 +183,9 @@ class SubstTestCase(unittest.TestCase):
         'HHH'       : 'III',
         'FFFIII'    : 'BADNEWS',
 
+        'THING1'    : "$(STUFF$)",
+        'THING2'    : "$THING1",
+
         'LITERAL'   : TestLiteral("$XXX"),
 
         # Test that we can expand to and return a function.
@@ -405,6 +408,11 @@ class scons_subst_TestCase(SubstTestCase):
             "test",
             "test",
 
+        "test $( $THING2 $)",
+            "test $( $(STUFF$) $)",
+            "test STUFF",
+            "test",
+
         "$AAA ${AAA}A $BBBB $BBB",
             "a aA  b",
             "a aA b",
@@ -544,6 +552,23 @@ class scons_subst_TestCase(SubstTestCase):
         else:
             raise AssertionError("did not catch expected UserError")
 
+    def test_subst_balance_errors(self):
+        """Test scons_subst():  handling syntax errors"""
+        env = DummyEnv(self.loc)
+        try:
+            scons_subst('$(', env, mode=SUBST_SIG)
+        except SCons.Errors.UserError as e:
+            assert str(e) == "Unbalanced $(/$) in: $(", str(e)
+        else:
+            raise AssertionError("did not catch expected UserError")
+
+        try:
+            scons_subst('$)', env, mode=SUBST_SIG)
+        except SCons.Errors.UserError as e:
+            assert str(e) == "Unbalanced $(/$) in: $)", str(e)
+        else:
+            raise AssertionError("did not catch expected UserError")
+
     def test_subst_type_errors(self):
         """Test scons_subst():  handling type errors"""
         env = DummyEnv(self.loc)