Meta class to handle isinstance checks for BaseItem

pytest.ini

@@ -204,7 +204,7 @@ flake8-ignore =
     tests/test_http_headers.py E501
     tests/test_http_request.py E402 E501 E127 E128 E128 E126 E123
     tests/test_http_response.py E501 E128
-    tests/test_item.py E128 F841
+    tests/test_item.py E128 F841 E501
     tests/test_link.py E501
     tests/test_linkextractors.py E501 E128 E124
     tests/test_loader.py E501 E741 E128 E117

scrapy/commands/parse.py

@@ -5,7 +5,7 @@ from w3lib.url import is_url
 
 from scrapy.commands import ScrapyCommand
 from scrapy.http import Request
-from scrapy.item import BaseItem
+from scrapy.item import _BaseItem
 from scrapy.utils import display
 from scrapy.utils.conf import arglist_to_dict
 from scrapy.utils.spider import iterate_spider_output, spidercls_for_request
@@ -117,7 +117,7 @@ class Command(ScrapyCommand):
         items, requests = [], []
 
         for x in iterate_spider_output(callback(response, **cb_kwargs)):
-            if isinstance(x, (BaseItem, dict)):
+            if isinstance(x, (_BaseItem, dict)):
                 items.append(x)
             elif isinstance(x, Request):
                 requests.append(x)

scrapy/contracts/default.py

@@ -1,6 +1,6 @@
 import json
 
-from scrapy.item import BaseItem
+from scrapy.item import _BaseItem
 from scrapy.http import Request
 from scrapy.exceptions import ContractFail
 
@@ -51,8 +51,8 @@ class ReturnsContract(Contract):
     objects = {
         'request': Request,
         'requests': Request,
-        'item': (BaseItem, dict),
-        'items': (BaseItem, dict),
+        'item': (_BaseItem, dict),
+        'items': (_BaseItem, dict),
     }
 
     def __init__(self, *args, **kwargs):
@@ -103,7 +103,7 @@ class ScrapesContract(Contract):
 
     def post_process(self, output):
         for x in output:
-            if isinstance(x, (BaseItem, dict)):
+            if isinstance(x, (_BaseItem, dict)):
                 missing = [arg for arg in self.args if arg not in x]
                 if missing:
                     raise ContractFail(

scrapy/core/scraper.py

@@ -14,7 +14,7 @@ from scrapy.utils.log import logformatter_adapter, failure_to_exc_info
 from scrapy.exceptions import CloseSpider, DropItem, IgnoreRequest
 from scrapy import signals
 from scrapy.http import Request, Response
-from scrapy.item import BaseItem
+from scrapy.item import _BaseItem
 from scrapy.core.spidermw import SpiderMiddlewareManager
 
 
@@ -191,7 +191,7 @@ class Scraper:
         """
         if isinstance(output, Request):
             self.crawler.engine.crawl(request=output, spider=spider)
-        elif isinstance(output, (BaseItem, dict)):
+        elif isinstance(output, (_BaseItem, dict)):
             self.slot.itemproc_size += 1
             dfd = self.itemproc.process_item(output, spider)
             dfd.addBoth(self._itemproc_finished, output, response, spider)

scrapy/exporters.py

@@ -12,7 +12,7 @@ from xml.sax.saxutils import XMLGenerator
 
 from scrapy.utils.serialize import ScrapyJSONEncoder
 from scrapy.utils.python import to_bytes, to_unicode, is_listlike
-from scrapy.item import BaseItem
+from scrapy.item import _BaseItem
 from scrapy.exceptions import ScrapyDeprecationWarning
 
 
@@ -312,7 +312,7 @@ class PythonItemExporter(BaseItemExporter):
         return serializer(value)
 
     def _serialize_value(self, value):
-        if isinstance(value, BaseItem):
+        if isinstance(value, _BaseItem):
             return self.export_item(value)
         if isinstance(value, dict):
             return dict(self._serialize_dict(value))

scrapy/item.py

@@ -14,7 +14,23 @@ from scrapy.utils.deprecate import ScrapyDeprecationWarning
 from scrapy.utils.trackref import object_ref
 
 
-class BaseItem(object_ref):
+class _BaseItem(object_ref):
+    """
+    Temporary class used internally to avoid the deprecation
+    warning raised by isinstance checks using BaseItem.
+    """
+    pass
+
+
+class _BaseItemMeta(ABCMeta):
+    def __instancecheck__(cls, instance):
+        if cls is BaseItem:
+            warn('scrapy.item.BaseItem is deprecated, please use scrapy.item.Item instead',
+                 ScrapyDeprecationWarning, stacklevel=2)
+        return super().__instancecheck__(instance)
+
+
+class BaseItem(_BaseItem, metaclass=_BaseItemMeta):
     """
     Deprecated, please use :class:`scrapy.item.Item` instead
     """
@@ -30,7 +46,7 @@ class Field(dict):
     """Container of field metadata"""
 
 
-class ItemMeta(ABCMeta):
+class ItemMeta(_BaseItemMeta):
     """Metaclass_ of :class:`Item` that handles field definitions.
 
     .. _metaclass: https://realpython.com/python-metaclasses

scrapy/shell.py

@@ -13,7 +13,7 @@ from w3lib.url import any_to_uri
 from scrapy.crawler import Crawler
 from scrapy.exceptions import IgnoreRequest
 from scrapy.http import Request, Response
-from scrapy.item import BaseItem
+from scrapy.item import _BaseItem
 from scrapy.settings import Settings
 from scrapy.spiders import Spider
 from scrapy.utils.console import start_python_console
@@ -26,8 +26,7 @@ from scrapy.utils.console import DEFAULT_PYTHON_SHELLS
 
 class Shell:
 
-    relevant_classes = (Crawler, Spider, Request, Response, BaseItem,
-                        Settings)
+    relevant_classes = (Crawler, Spider, Request, Response, _BaseItem, Settings)
 
     def __init__(self, crawler, update_vars=None, code=None):
         self.crawler = crawler

scrapy/utils/misc.py

@@ -14,10 +14,10 @@ from w3lib.html import replace_entities
 
 from scrapy.utils.datatypes import LocalWeakReferencedCache
 from scrapy.utils.python import flatten, to_unicode
-from scrapy.item import BaseItem
+from scrapy.item import _BaseItem
 
 
-_ITERABLE_SINGLE_VALUES = dict, BaseItem, str, bytes
+_ITERABLE_SINGLE_VALUES = dict, _BaseItem, str, bytes
 
 
 def arg_to_iter(arg):

scrapy/utils/serialize.py

@@ -5,7 +5,7 @@ import decimal
 from twisted.internet import defer
 
 from scrapy.http import Request, Response
-from scrapy.item import BaseItem
+from scrapy.item import _BaseItem
 
 
 class ScrapyJSONEncoder(json.JSONEncoder):
@@ -26,7 +26,7 @@ class ScrapyJSONEncoder(json.JSONEncoder):
             return str(o)
         elif isinstance(o, defer.Deferred):
             return str(o)
-        elif isinstance(o, BaseItem):
+        elif isinstance(o, _BaseItem):
             return dict(o)
         elif isinstance(o, Request):
             return "<%s %s %s>" % (type(o).__name__, o.method, o.url)

tests/test_item.py

@@ -4,7 +4,7 @@ from unittest import mock
 from warnings import catch_warnings
 
 from scrapy.exceptions import ScrapyDeprecationWarning
-from scrapy.item import ABCMeta, BaseItem, DictItem, Field, Item, ItemMeta
+from scrapy.item import ABCMeta, _BaseItem, BaseItem, DictItem, Field, Item, ItemMeta
 
 
 PY36_PLUS = (sys.version_info.major >= 3) and (sys.version_info.minor >= 6)
@@ -334,29 +334,73 @@ class DictItemTest(unittest.TestCase):
 
 class BaseItemTest(unittest.TestCase):
 
+    def test_isinstance_check(self):
+
+        class SubclassedBaseItem(BaseItem):
+            pass
+
+        class SubclassedItem(Item):
+            pass
+
+        self.assertTrue(isinstance(BaseItem(), BaseItem))
+        self.assertTrue(isinstance(SubclassedBaseItem(), BaseItem))
+        self.assertTrue(isinstance(Item(), BaseItem))
+        self.assertTrue(isinstance(SubclassedItem(), BaseItem))
+
+        # make sure internal checks using private _BaseItem class succeed
+        self.assertTrue(isinstance(BaseItem(), _BaseItem))
+        self.assertTrue(isinstance(SubclassedBaseItem(), _BaseItem))
+        self.assertTrue(isinstance(Item(), _BaseItem))
+        self.assertTrue(isinstance(SubclassedItem(), _BaseItem))
+
     def test_deprecation_warning(self):
+        """
+        Make sure deprecation warnings are logged whenever BaseItem is used,
+        either instantiated or in an isinstance check
+        """
         with catch_warnings(record=True) as warnings:
             BaseItem()
             self.assertEqual(len(warnings), 1)
             self.assertEqual(warnings[0].category, ScrapyDeprecationWarning)
+
         with catch_warnings(record=True) as warnings:
+
             class SubclassedBaseItem(BaseItem):
                 pass
+
             SubclassedBaseItem()
             self.assertEqual(len(warnings), 1)
             self.assertEqual(warnings[0].category, ScrapyDeprecationWarning)
 
+        with catch_warnings(record=True) as warnings:
+            self.assertFalse(isinstance("foo", BaseItem))
+            self.assertEqual(len(warnings), 1)
+            self.assertEqual(warnings[0].category, ScrapyDeprecationWarning)
+
+        with catch_warnings(record=True) as warnings:
+            self.assertTrue(isinstance(BaseItem(), BaseItem))
+            self.assertEqual(len(warnings), 1)
+            self.assertEqual(warnings[0].category, ScrapyDeprecationWarning)
+
 
 class ItemNoDeprecationWarningTest(unittest.TestCase):
-
     def test_no_deprecation_warning(self):
+        """
+        Make sure deprecation warnings are NOT logged whenever BaseItem subclasses are used.
+        """
+        class SubclassedItem(Item):
+            pass
+
         with catch_warnings(record=True) as warnings:
             Item()
-            self.assertEqual(len(warnings), 0)
-        with catch_warnings(record=True) as warnings:
-            class SubclassedItem(Item):
-                pass
             SubclassedItem()
+            _BaseItem()
+            self.assertFalse(isinstance("foo", _BaseItem))
+            self.assertFalse(isinstance("foo", Item))
+            self.assertFalse(isinstance("foo", SubclassedItem))
+            self.assertTrue(isinstance(_BaseItem(), _BaseItem))
+            self.assertTrue(isinstance(Item(), Item))
+            self.assertTrue(isinstance(SubclassedItem(), SubclassedItem))
             self.assertEqual(len(warnings), 0)