Better error handling if someone tries to add a non-Node as a source, dependency or ignored dependency of a Node.

3 files changed, 93 insertions, 41 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 474514e..d567560 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -119,6 +119,9 @@ RELEASE 0.95 - XXX
     SharedLibrary() are shared work even if the object was built in a
     previous run.
 
+  - Supply meaningful error messages, not stack traces, if we try to add
+    a non-Node as a source, dependency, or ignored dependency of a Node.
+
   From Vincent Risi:
 
   - Add support for the bcc32, ilink32 and tlib Borland tools.
diff --git a/src/engine/SCons/Node/NodeTests.py b/src/engine/SCons/Node/NodeTests.py
index fa0b6f7..08e4260 100644
--- a/src/engine/SCons/Node/NodeTests.py
+++ b/src/engine/SCons/Node/NodeTests.py
@@ -470,31 +470,39 @@ class NodeTestCase(unittest.TestCase):
         assert node.depends == []
 
         zero = SCons.Node.Node()
-        try:
-            node.add_dependency(zero)
-        except TypeError:
-            pass
-        else:
-            assert 0
 
         one = SCons.Node.Node()
         two = SCons.Node.Node()
         three = SCons.Node.Node()
         four = SCons.Node.Node()
+        five = SCons.Node.Node()
+        six = SCons.Node.Node()
 
+        node.add_dependency(zero)
+        assert node.depends == [zero]
         node.add_dependency([one])
-        assert node.depends == [one]
+        assert node.depends == [zero, one]
         node.add_dependency([two, three])
-        assert node.depends == [one, two, three]
+        assert node.depends == [zero, one, two, three]
         node.add_dependency([three, four, one])
-        assert node.depends == [one, two, three, four]
+        assert node.depends == [zero, one, two, three, four]
 
-        assert zero.get_parents() == []
+        assert zero.get_parents() == [node]
         assert one.get_parents() == [node]
         assert two.get_parents() == [node]
         assert three.get_parents() == [node]
         assert four.get_parents() == [node]
 
+        try:
+            node.add_depends([[five, six]])
+        except:
+            pass
+        else:
+            raise "did not catch expected exception"
+        assert node.depends == [zero, one, two, three, four]
+        assert five.get_parents() == []
+        assert six.get_parents() == []
+
 
     def test_add_source(self):
         """Test adding sources to a Node's list.
@@ -503,31 +511,38 @@ class NodeTestCase(unittest.TestCase):
         assert node.sources == []
 
         zero = SCons.Node.Node()
-        try:
-            node.add_source(zero)
-        except TypeError:
-            pass
-        else:
-            assert 0
-
         one = SCons.Node.Node()
         two = SCons.Node.Node()
         three = SCons.Node.Node()
         four = SCons.Node.Node()
+        five = SCons.Node.Node()
+        six = SCons.Node.Node()
 
+        node.add_source(zero)
+        assert node.sources == [zero]
         node.add_source([one])
-        assert node.sources == [one]
+        assert node.sources == [zero, one]
         node.add_source([two, three])
-        assert node.sources == [one, two, three]
+        assert node.sources == [zero, one, two, three]
         node.add_source([three, four, one])
-        assert node.sources == [one, two, three, four]
+        assert node.sources == [zero, one, two, three, four]
 
-        assert zero.get_parents() == []
+        assert zero.get_parents() == [node]
         assert one.get_parents() == [node]
         assert two.get_parents() == [node]
         assert three.get_parents() == [node]
         assert four.get_parents() == [node]
 
+        try:
+            node.add_source([[five, six]])
+        except:
+            pass
+        else:
+            raise "did not catch expected exception"
+        assert node.sources == [zero, one, two, three, four]
+        assert five.get_parents() == []
+        assert six.get_parents() == []
+
     def test_add_ignore(self):
         """Test adding files whose dependencies should be ignored.
         """
@@ -535,31 +550,38 @@ class NodeTestCase(unittest.TestCase):
         assert node.ignore == []
 
         zero = SCons.Node.Node()
-        try:
-            node.add_ignore(zero)
-        except TypeError:
-            pass
-        else:
-            assert 0
-
         one = SCons.Node.Node()
         two = SCons.Node.Node()
         three = SCons.Node.Node()
         four = SCons.Node.Node()
+        five = SCons.Node.Node()
+        six = SCons.Node.Node()
 
+        node.add_ignore(zero)
+        assert node.ignore == [zero]
         node.add_ignore([one])
-        assert node.ignore == [one]
+        assert node.ignore == [zero, one]
         node.add_ignore([two, three])
-        assert node.ignore == [one, two, three]
+        assert node.ignore == [zero, one, two, three]
         node.add_ignore([three, four, one])
-        assert node.ignore == [one, two, three, four]
+        assert node.ignore == [zero, one, two, three, four]
 
-        assert zero.get_parents() == []
+        assert zero.get_parents() == [node]
         assert one.get_parents() == [node]
         assert two.get_parents() == [node]
         assert three.get_parents() == [node]
         assert four.get_parents() == [node]
 
+        try:
+            node.add_ignore([[five, six]])
+        except:
+            pass
+        else:
+            raise "did not catch expected exception"
+        assert node.ignore == [zero, one, two, three, four]
+        assert five.get_parents() == []
+        assert six.get_parents() == []
+
     def test_get_found_includes(self):
         """Test the default get_found_includes() method
         """
diff --git a/src/engine/SCons/Node/__init__.py b/src/engine/SCons/Node/__init__.py
index 51bfe30..28264ce 100644
--- a/src/engine/SCons/Node/__init__.py
+++ b/src/engine/SCons/Node/__init__.py
@@ -583,22 +583,49 @@ class Node:
         return None
 
     def add_dependency(self, depend):
-        """Adds dependencies. The depend argument must be a list."""
-        self._add_child(self.depends, self.depends_dict, depend)
+        """Adds dependencies."""
+        try:
+            self._add_child(self.depends, self.depends_dict, depend)
+        except TypeError, e:
+            e = e.args[0]
+            if SCons.Util.is_List(e):
+                s = map(str, e)
+            else:
+                s = str(e)
+            raise SCons.Errors.UserError("attempted to add a non-Node dependency to %s:\n\t%s is a %s, not a Node" % (str(self), s, type(e)))
 
     def add_ignore(self, depend):
-        """Adds dependencies to ignore. The depend argument must be a list."""
-        self._add_child(self.ignore, self.ignore_dict, depend)
+        """Adds dependencies to ignore."""
+        try:
+            self._add_child(self.ignore, self.ignore_dict, depend)
+        except TypeError, e:
+            e = e.args[0]
+            if SCons.Util.is_List(e):
+                s = map(str, e)
+            else:
+                s = str(e)
+            raise SCons.Errors.UserError("attempted to ignore a non-Node dependency of %s:\n\t%s is a %s, not a Node" % (str(self), s, type(e)))
 
     def add_source(self, source):
-        """Adds sources. The source argument must be a list."""
-        self._add_child(self.sources, self.sources_dict, source)
+        """Adds sources."""
+        try:
+            self._add_child(self.sources, self.sources_dict, source)
+        except TypeError, e:
+            e = e.args[0]
+            if SCons.Util.is_List(e):
+                s = map(str, e)
+            else:
+                s = str(e)
+            raise SCons.Errors.UserError("attempted to add a non-Node as source of %s:\n\t%s is a %s, not a Node" % (str(self), s, type(e)))
 
     def _add_child(self, collection, dict, child):
-        """Adds 'child' to 'collection', first checking 'dict' to see if
-        it's already present. The 'child' argument must be a list"""
+        """Adds 'child' to 'collection', first checking 'dict' to see
+        if it's already present."""
         if type(child) is not type([]):
-            raise TypeError("child must be a list")
+            child = [child]
+        for c in child:
+            if not isinstance(c, Node):
+                raise TypeError, c
         added = None
         for c in child:
             if not dict.has_key(c):