Merge pull request #350 from sebhub/item-document-2

Remove superfluous item.document checks

doorstop/core/base.py

@@ -21,7 +21,7 @@ def wrapped(self, *args, **kwargs):
         if settings.ADDREMOVE_FILES and item.tree:
             item.tree.vcs.add(item.path)
         # pylint: disable=W0212
-        if item.document and item not in item.document._items:
+        if item not in item.document._items:
             item.document._items.append(item)
         if settings.CACHE_ITEMS and item.tree:
             item.tree._item_cache[item.uid] = item
@@ -49,7 +49,7 @@ def wrapped(self, *args, **kwargs):
         if settings.ADDREMOVE_FILES and item.tree:
             item.tree.vcs.delete(item.path)
         # pylint: disable=W0212
-        if item.document and item in item.document._items:
+        if item in item.document._items:
             item.document._items.remove(item)
         if settings.CACHE_ITEMS and item.tree:
             item.tree._item_cache[item.uid] = None

doorstop/core/item.py

@@ -576,17 +576,14 @@ def get_issues(self, skip=None, document_hook=None, item_hook=None):  # pylint:
             yield DoorstopWarning("non-normative, but has links")
 
         # Check links against the document
-        if self.document:
-            yield from self._get_issues_document(self.document, skip)
+        yield from self._get_issues_document(self.document, skip)
 
-        # Check links against the tree
         if self.tree:
+            # Check links against the tree
             yield from self._get_issues_tree(self.tree)
 
-        # Check links against both document and tree
-        if self.document and self.tree:
-            yield from self._get_issues_both(self.document, self.tree,
-                                             skip)
+            # Check links against both document and tree
+            yield from self._get_issues_both(self.document, self.tree, skip)
 
         # Check review status
         if not self.reviewed:

doorstop/core/tests/__init__.py

@@ -69,11 +69,27 @@ def __bool__(self):  # override __len__ behavior, pylint: disable=R0201
 class MockItem(MockFileObject, Item):  # pylint: disable=W0223,R0902
     """Mock Item class with stubbed file IO."""
 
+    def _no_get_issues_document(self, document, skip):  # pylint: disable=W0613,R0201
+        return
+        yield  # pylint: disable=W0101
+
+    def disable_get_issues_document(self):
+        self._get_issues_document = self._no_get_issues_document
+
 
 class MockDocument(MockFileObject, Document):  # pylint: disable=W0223,R0902
     """Mock Document class with stubbed file IO."""
 
 
+class MockSimpleDocument:
+    """Mock Document class with basic default members."""
+
+    def __init__(self):
+        self.parent = None
+        self.prefix = 'RQ'
+        self._items = []
+
+
 class MockDocumentSkip(MockDocument):  # pylint: disable=W0223,R0902
     """Mock Document class that is always skipped in tree placement."""
 

doorstop/core/tests/test_item.py

@@ -12,7 +12,7 @@
 from doorstop.core.vcs.mockvcs import WorkingCopy
 
 from doorstop.core.tests import FILES, EMPTY, EXTERNAL
-from doorstop.core.tests import MockItem
+from doorstop.core.tests import MockItem, MockSimpleDocument
 
 
 YAML_DEFAULT = """
@@ -34,15 +34,14 @@ class TestItem(unittest.TestCase):
 
     def setUp(self):
         path = os.path.join('path', 'to', 'RQ001.yml')
-        self.item = MockItem(None, path)
+        self.item = MockItem(MockSimpleDocument(), path)
 
     def test_init_invalid(self):
         """Verify an item cannot be initialized from an invalid path."""
         self.assertRaises(DoorstopError, Item, None, 'not/a/path')
 
-    def test_object_references(self):
-        """Verify a standalone item does not have object references."""
-        self.assertIs(None, self.item.document)
+    def test_no_tree_references(self):
+        """Verify a standalone item has no tree reference."""
         self.assertIs(None, self.item.tree)
 
     def test_load_empty(self):
@@ -535,7 +534,7 @@ def test_invalid_file_ext(self):
     def test_new(self):
         """Verify items can be created."""
         MockItem._create.reset_mock()
-        item = MockItem.new(None, None,
+        item = MockItem.new(None, MockSimpleDocument(),
                             EMPTY, FILES, 'TEST00042',
                             level=(1, 2, 3))
         path = os.path.join(EMPTY, 'TEST00042.yml')
@@ -548,7 +547,7 @@ def test_new_cache(self):
         """Verify new items are cached."""
         mock_tree = Mock()
         mock_tree._item_cache = {}
-        item = MockItem.new(mock_tree, None,
+        item = MockItem.new(mock_tree, MockSimpleDocument(),
                             EMPTY, FILES, 'TEST00042',
                             level=(1, 2, 3))
         self.assertEqual(item, mock_tree._item_cache[item.uid])
@@ -558,7 +557,7 @@ def test_new_cache(self):
     def test_new_special(self):
         """Verify items can be created with a specially named prefix."""
         MockItem._create.reset_mock()
-        item = MockItem.new(None, None,
+        item = MockItem.new(None, MockSimpleDocument(),
                             EMPTY, FILES, 'VSM.HLR_01-002-042',
                             level=(1, 0))
         path = os.path.join(EMPTY, 'VSM.HLR_01-002-042.yml')
@@ -614,6 +613,7 @@ def test_validate_cleared(self):
         mock_tree.find_item = Mock(return_value=mock_item)
         self.item.tree = mock_tree
         self.item.links = [{'mock_uid': True}]
+        self.item.disable_get_issues_document()
         self.assertTrue(self.item.validate())
         self.assertEqual('abc123', self.item.links[0].stamp)
 
@@ -625,13 +625,15 @@ def test_validate_cleared_new(self):
         mock_tree.find_item = Mock(return_value=mock_item)
         self.item.tree = mock_tree
         self.item.links = [{'mock_uid': None}]
+        self.item.disable_get_issues_document()
         self.assertTrue(self.item.validate())
         self.assertEqual('abc123', self.item.links[0].stamp)
 
     def test_validate_nonnormative_with_links(self):
         """Verify a non-normative item with links can be checked."""
         self.item.normative = False
         self.item.links = ['a']
+        self.item.disable_get_issues_document()
         self.assertTrue(self.item.validate())
 
     @patch('doorstop.settings.STAMP_NEW_LINKS', False)
@@ -643,6 +645,7 @@ def test_validate_link_to_inactive(self):
         mock_tree.find_item = Mock(return_value=mock_item)
         self.item.links = ['a']
         self.item.tree = mock_tree
+        self.item.disable_get_issues_document()
         self.assertTrue(self.item.validate())
 
     @patch('doorstop.settings.STAMP_NEW_LINKS', False)
@@ -654,6 +657,7 @@ def test_validate_link_to_nonnormative(self):
         mock_tree.find_item = Mock(return_value=mock_item)
         self.item.links = ['a']
         self.item.tree = mock_tree
+        self.item.disable_get_issues_document()
         self.assertTrue(self.item.validate())
 
     def test_validate_document(self):