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

6 files changed, 134 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):
diff --git a/test/Depends.py b/test/Depends.py
index 530c3c9..a7d1bdf 100644
--- a/test/Depends.py
+++ b/test/Depends.py
@@ -140,4 +140,18 @@ test.fail_test(test.read(['subdir', 'f4.out']) !=
 test.fail_test(test.read('f5.out') != "f5.in\nsubdir/foo.dep 3\n")
 test.fail_test(test.read(['sub2', 'f6.out']) != "f6.in 3\nsubdir/bar.dep 3\n")
 
+#
+test.write('SConstruct', """\
+env = Environment()
+file1 = File('file1')
+file2 = File('file2')
+env.Depends(file1, [[file2, 'file3']])
+""")
+
+test.run(status = 2, stderr = """
+scons: *** attempted to add a non-Node dependency to file1:
+\t['file2', 'file3'] is a <type 'list'>, not a Node
+File "SConstruct", line 4, in ?
+""")
+
 test.pass_test()
diff --git a/test/Ignore.py b/test/Ignore.py
index 5e9650e..06db727 100644
--- a/test/Ignore.py
+++ b/test/Ignore.py
@@ -108,4 +108,18 @@ test.fail_test(test.read(['subdir', 'f3.out']) !=
 
 test.up_to_date(arguments = '.')
 
+#
+test.write('SConstruct', """\
+env = Environment()
+file1 = File('file1')
+file2 = File('file2')
+env.Ignore(file1, [[file2, 'file3']])
+""")
+
+test.run(status = 2, stderr = """
+scons: *** attempted to ignore a non-Node dependency of file1:
+\t['file2', 'file3'] is a <type 'list'>, not a Node
+File "SConstruct", line 4, in ?
+""")
+
 test.pass_test()
diff --git a/test/Program.py b/test/Program.py
index f231ac1..7c0eb46 100644
--- a/test/Program.py
+++ b/test/Program.py
@@ -327,4 +327,17 @@ test.fail_test(not (oldtime3 == os.path.getmtime(foo3)))
 test.fail_test(not (oldtime4 == os.path.getmtime(foo4)))
 test.fail_test(not (oldtime5 == os.path.getmtime(foo5)))
 
+# 
+test.write('SConstruct', """\
+file1 = File('file1.c')
+file2 = File('file2.c')
+Program('foo', [file1, [file2, 'file3.c']])
+""")
+
+test.run(status = 2, stderr = """
+scons: *** attempted to add a non-Node as source of foo:
+\t['file2.c', 'file3.c'] is a <type 'list'>, not a Node
+File "SConstruct", line 3, in ?
+""")
+
 test.pass_test()