diff --git a/scrapy/tests/test_utils_deprecate.py b/scrapy/tests/test_utils_deprecate.py index 35aae8335..07b9a3dd5 100644 --- a/scrapy/tests/test_utils_deprecate.py +++ b/scrapy/tests/test_utils_deprecate.py @@ -17,51 +17,109 @@ class NewName(SomeBaseClass): class WarnWhenSubclassedTest(unittest.TestCase): + def _mywarnings(self, w, category=MyWarning): + return [x for x in w if x.category is MyWarning] + def test_no_warning_on_definition(self): with warnings.catch_warnings(record=True) as w: Deprecated = create_deprecated_class('Deprecated', NewName) + w = self._mywarnings(w) self.assertEqual(w, []) - def test_warning_on_subclassing(self): - with warnings.catch_warnings(record=True) as w: - Deprecated = create_deprecated_class('Deprecated', NewName, - warn_category=MyWarning) + def test_subclassing_warning_message(self): + Deprecated = create_deprecated_class('Deprecated', NewName, + warn_category=MyWarning) + with warnings.catch_warnings(record=True) as w: class UserClass(Deprecated): pass + w = self._mywarnings(w) self.assertEqual(len(w), 1) - msg = w[0] - assert issubclass(msg.category, MyWarning) self.assertEqual( - str(msg.message), + str(w[0].message), "scrapy.tests.test_utils_deprecate.UserClass inherits from " "deprecated class scrapy.tests.test_utils_deprecate.Deprecated, " "please inherit from scrapy.tests.test_utils_deprecate.NewName." + " (warning only on first subclass, there may be others)" ) - self.assertEqual(msg.lineno, inspect.getsourcelines(UserClass)[1]) + self.assertEqual(w[0].lineno, inspect.getsourcelines(UserClass)[1]) + + def test_custom_class_paths(self): + Deprecated = create_deprecated_class('Deprecated', NewName, + new_class_path='foo.NewClass', + old_class_path='bar.OldClass', + warn_category=MyWarning) - def test_warning_on_instance(self): with warnings.catch_warnings(record=True) as w: - Deprecated = create_deprecated_class('Deprecated', NewName, - warn_category=MyWarning) - class UserClass(Deprecated): pass - _, lineno = Deprecated(), inspect.getlineno(inspect.currentframe()) - _ = UserClass() + _ = Deprecated() + w = self._mywarnings(w) self.assertEqual(len(w), 2) - msg = w[1] - assert issubclass(msg.category, MyWarning) + self.assertIn('foo.NewClass', str(w[0].message)) + self.assertIn('bar.OldClass', str(w[0].message)) + self.assertIn('foo.NewClass', str(w[1].message)) + self.assertIn('bar.OldClass', str(w[1].message)) + + def test_subclassing_warns_only_on_direct_childs(self): + Deprecated = create_deprecated_class('Deprecated', NewName, + warn_once=False, + warn_category=MyWarning) + + with warnings.catch_warnings(record=True) as w: + class UserClass(Deprecated): + pass + + class NoWarnOnMe(UserClass): + pass + + w = self._mywarnings(w) + self.assertEqual(len(w), 1) + self.assertIn('UserClass', str(w[0].message)) + + def test_subclassing_warns_once_by_default(self): + Deprecated = create_deprecated_class('Deprecated', NewName, + warn_category=MyWarning) + + with warnings.catch_warnings(record=True) as w: + class UserClass(Deprecated): + pass + + class FooClass(Deprecated): + pass + + class BarClass(Deprecated): + pass + + w = self._mywarnings(w) + self.assertEqual(len(w), 1) + self.assertIn('UserClass', str(w[0].message)) + + def test_warning_on_instance(self): + Deprecated = create_deprecated_class('Deprecated', NewName, + warn_category=MyWarning) + + # ignore subclassing warnings + with warnings.catch_warnings(record=True): + class UserClass(Deprecated): + pass + + with warnings.catch_warnings(record=True) as w: + _, lineno = Deprecated(), inspect.getlineno(inspect.currentframe()) + _ = UserClass() # subclass instances don't warn + + w = self._mywarnings(w) + self.assertEqual(len(w), 1) self.assertEqual( - str(msg.message), + str(w[0].message), "scrapy.tests.test_utils_deprecate.Deprecated is deprecated, " "instantiate scrapy.tests.test_utils_deprecate.NewName instead." ) - self.assertEqual(msg.lineno, lineno) + self.assertEqual(w[0].lineno, lineno) def test_warning_auto_message(self): with warnings.catch_warnings(record=True) as w: @@ -136,3 +194,32 @@ class WarnWhenSubclassedTest(unittest.TestCase): Deprecated = create_deprecated_class('Deprecated', NewName, {'foo': 'bar'}) self.assertEqual(Deprecated.foo, 'bar') + + def test_deprecate_a_class_with_custom_metaclass(self): + Meta1 = type('Meta1', (type,), {}) + New = Meta1('New', (), {}) + Deprecated = create_deprecated_class('Deprecated', New) + + def test_deprecate_subclass_of_deprecated_class(self): + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter('always') + Deprecated = create_deprecated_class('Deprecated', NewName, + warn_category=MyWarning) + AlsoDeprecated = create_deprecated_class('AlsoDeprecated', Deprecated, + new_class_path='foo.Bar', + warn_category=MyWarning) + + w = self._mywarnings(w) + self.assertEqual(len(w), 0, str(map(str, w))) + + with warnings.catch_warnings(record=True) as w: + AlsoDeprecated() + class UserClass(AlsoDeprecated): + pass + + w = self._mywarnings(w) + self.assertEqual(len(w), 2) + self.assertIn('AlsoDeprecated', str(w[0].message)) + self.assertIn('foo.Bar', str(w[0].message)) + self.assertIn('AlsoDeprecated', str(w[1].message)) + self.assertIn('foo.Bar', str(w[1].message)) diff --git a/scrapy/utils/deprecate.py b/scrapy/utils/deprecate.py index 20f4f1796..edaecc3d3 100644 --- a/scrapy/utils/deprecate.py +++ b/scrapy/utils/deprecate.py @@ -14,6 +14,9 @@ def attribute(obj, oldattr, newattr, version='0.12'): def create_deprecated_class(name, new_class, clsdict=None, warn_category=ScrapyDeprecationWarning, + warn_once=True, + old_class_path=None, + new_class_path=None, subclass_warn_message="{cls} inherits from "\ "deprecated class {old}, please inherit "\ "from {new}.", @@ -44,9 +47,10 @@ def create_deprecated_class(name, new_class, clsdict=None, OldName. """ - class DeprecatedClass(type): + class DeprecatedClass(new_class.__class__): deprecated_class = None + warned_on_subclass = False def __new__(metacls, name, bases, clsdict_): cls = super(DeprecatedClass, metacls).__new__(metacls, name, bases, clsdict_) @@ -55,11 +59,15 @@ def create_deprecated_class(name, new_class, clsdict=None, return cls def __init__(cls, name, bases, clsdict_): - old = cls.__class__.deprecated_class - if cls is not old: + meta = cls.__class__ + old = meta.deprecated_class + if old in bases and not (warn_once and meta.warned_on_subclass): + meta.warned_on_subclass = True msg = subclass_warn_message.format(cls=_clspath(cls), - old=_clspath(old), - new=_clspath(new_class)) + old=_clspath(old, old_class_path), + new=_clspath(new_class, new_class_path)) + if warn_once: + msg += ' (warning only on first subclass, there may be others)' warnings.warn(msg, warn_category, stacklevel=2) super(DeprecatedClass, cls).__init__(name, bases, clsdict_) @@ -79,9 +87,10 @@ def create_deprecated_class(name, new_class, clsdict=None, return any(c in candidates for c in mro) def __call__(cls, *args, **kwargs): - if cls is cls.__class__.deprecated_class: - msg = instance_warn_message.format(cls=_clspath(cls), - new=_clspath(new_class)) + old = DeprecatedClass.deprecated_class + if cls is old: + msg = instance_warn_message.format(cls=_clspath(cls, old_class_path), + new=_clspath(new_class, new_class_path)) warnings.warn(msg, warn_category, stacklevel=2) return super(DeprecatedClass, cls).__call__(*args, **kwargs) @@ -94,5 +103,7 @@ def create_deprecated_class(name, new_class, clsdict=None, return deprecated_cls -def _clspath(cls): +def _clspath(cls, forced=None): + if forced is not None: + return forced return '{}.{}'.format(cls.__module__, cls.__name__)