diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index ca0af569f..a59a61050 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -2047,6 +2047,21 @@ also used by :class:`~scrapy.downloadermiddlewares.robotstxt.RobotsTxtMiddleware if :setting:`ROBOTSTXT_USER_AGENT` setting is ``None`` and there is no overriding User-Agent header specified for the request. +.. setting:: WARN_ON_GENERATOR_RETURN_VALUE + +WARN_ON_GENERATOR_RETURN_VALUE +------------------------------ + +Default: ``True`` + +When enabled, Scrapy will warn if generator-based callback methods (like +``parse``) contain return statements with non-``None`` values. This helps detect +potential mistakes in spider development. + +Disable this setting to prevent syntax errors that may occur when dynamically +modifying generator function source code during runtime, skip AST parsing of +callback functions, or improve performance in auto-reloading development +environments. Settings documented elsewhere: ------------------------------ diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index 645e50301..680fded7a 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -351,3 +351,5 @@ SPIDER_CONTRACTS_BASE = { "scrapy.contracts.default.ReturnsContract": 2, "scrapy.contracts.default.ScrapesContract": 3, } + +WARN_ON_GENERATOR_RETURN_VALUE = True diff --git a/scrapy/utils/misc.py b/scrapy/utils/misc.py index d319e7950..b7b436260 100644 --- a/scrapy/utils/misc.py +++ b/scrapy/utils/misc.py @@ -286,6 +286,8 @@ def warn_on_generator_with_return_value( Logs a warning if a callable is a generator function and includes a 'return' statement with a value different than None """ + if not spider.settings.getbool("WARN_ON_GENERATOR_RETURN_VALUE"): + return try: if is_generator_with_return_value(callable): warnings.warn( diff --git a/tests/test_utils_misc/test_return_with_argument_inside_generator.py b/tests/test_utils_misc/test_return_with_argument_inside_generator.py index 81a83c3d7..ad31e5185 100644 --- a/tests/test_utils_misc/test_return_with_argument_inside_generator.py +++ b/tests/test_utils_misc/test_return_with_argument_inside_generator.py @@ -2,6 +2,8 @@ import warnings from functools import partial from unittest import mock +import pytest + from scrapy.utils.misc import ( is_generator_with_return_value, warn_on_generator_with_return_value, @@ -40,7 +42,24 @@ def generator_that_returns_stuff(): class TestUtilsMisc: - def test_generators_return_something(self): + @pytest.fixture + def mock_spider(self): + class MockSettings: + def __init__(self, settings_dict=None): + self.settings_dict = settings_dict or { + "WARN_ON_GENERATOR_RETURN_VALUE": True + } + + def getbool(self, name, default=False): + return self.settings_dict.get(name, default) + + class MockSpider: + def __init__(self): + self.settings = MockSettings() + + return MockSpider() + + def test_generators_return_something(self, mock_spider): def f1(): yield 1 return 2 @@ -75,30 +94,30 @@ https://example.org assert is_generator_with_return_value(i1) with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, top_level_return_something) + warn_on_generator_with_return_value(mock_spider, top_level_return_something) assert len(w) == 1 assert ( - 'The "NoneType.top_level_return_something" method is a generator' + 'The "MockSpider.top_level_return_something" method is a generator' in str(w[0].message) ) with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, f1) + warn_on_generator_with_return_value(mock_spider, f1) assert len(w) == 1 - assert 'The "NoneType.f1" method is a generator' in str(w[0].message) + assert 'The "MockSpider.f1" method is a generator' in str(w[0].message) with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, g1) + warn_on_generator_with_return_value(mock_spider, g1) assert len(w) == 1 - assert 'The "NoneType.g1" method is a generator' in str(w[0].message) + assert 'The "MockSpider.g1" method is a generator' in str(w[0].message) with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, h1) + warn_on_generator_with_return_value(mock_spider, h1) assert len(w) == 1 - assert 'The "NoneType.h1" method is a generator' in str(w[0].message) + assert 'The "MockSpider.h1" method is a generator' in str(w[0].message) with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, i1) + warn_on_generator_with_return_value(mock_spider, i1) assert len(w) == 1 - assert 'The "NoneType.i1" method is a generator' in str(w[0].message) + assert 'The "MockSpider.i1" method is a generator' in str(w[0].message) - def test_generators_return_none(self): + def test_generators_return_none(self, mock_spider): def f2(): yield 1 @@ -142,31 +161,31 @@ https://example.org assert not is_generator_with_return_value(l2) with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, top_level_return_none) + warn_on_generator_with_return_value(mock_spider, top_level_return_none) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, f2) + warn_on_generator_with_return_value(mock_spider, f2) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, g2) + warn_on_generator_with_return_value(mock_spider, g2) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, h2) + warn_on_generator_with_return_value(mock_spider, h2) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, i2) + warn_on_generator_with_return_value(mock_spider, i2) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, j2) + warn_on_generator_with_return_value(mock_spider, j2) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, k2) + warn_on_generator_with_return_value(mock_spider, k2) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, l2) + warn_on_generator_with_return_value(mock_spider, l2) assert len(w) == 0 - def test_generators_return_none_with_decorator(self): + def test_generators_return_none_with_decorator(self, mock_spider): def decorator(func): def inner_func(): func() @@ -223,36 +242,36 @@ https://example.org assert not is_generator_with_return_value(l3) with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, top_level_return_none) + warn_on_generator_with_return_value(mock_spider, top_level_return_none) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, f3) + warn_on_generator_with_return_value(mock_spider, f3) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, g3) + warn_on_generator_with_return_value(mock_spider, g3) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, h3) + warn_on_generator_with_return_value(mock_spider, h3) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, i3) + warn_on_generator_with_return_value(mock_spider, i3) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, j3) + warn_on_generator_with_return_value(mock_spider, j3) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, k3) + warn_on_generator_with_return_value(mock_spider, k3) assert len(w) == 0 with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, l3) + warn_on_generator_with_return_value(mock_spider, l3) assert len(w) == 0 @mock.patch( "scrapy.utils.misc.is_generator_with_return_value", new=_indentation_error ) - def test_indentation_error(self): + def test_indentation_error(self, mock_spider): with warnings.catch_warnings(record=True) as w: - warn_on_generator_with_return_value(None, top_level_return_none) + warn_on_generator_with_return_value(mock_spider, top_level_return_none) assert len(w) == 1 assert "Unable to determine" in str(w[0].message) @@ -262,3 +281,32 @@ https://example.org partial_cb = partial(cb, arg1=42) assert not is_generator_with_return_value(partial_cb) + + def test_warn_on_generator_with_return_value_settings_disabled(self): + class MockSettings: + def __init__(self, settings_dict=None): + self.settings_dict = settings_dict or {} + + def getbool(self, name, default=False): + return self.settings_dict.get(name, default) + + class MockSpider: + def __init__(self): + self.settings = MockSettings({"WARN_ON_GENERATOR_RETURN_VALUE": False}) + + spider = MockSpider() + + def gen_with_return(): + yield 1 + return "value" + + with warnings.catch_warnings(record=True) as w: + warn_on_generator_with_return_value(spider, gen_with_return) + assert len(w) == 0 + + spider.settings.settings_dict["WARN_ON_GENERATOR_RETURN_VALUE"] = True + + with warnings.catch_warnings(record=True) as w: + warn_on_generator_with_return_value(spider, gen_with_return) + assert len(w) == 1 + assert "is a generator" in str(w[0].message)