Repository: scrapy/scrapy
Pull request #6738 (2025-03-18 19:44:57)

Syntax Error Fixed 
Issue #6731 

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/scrapy/scrapy/pull/6738?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy) Report
Attention: Patch coverage is `33.33333%` with `2 lines` in your changes missing coverage. Please review.
> Project coverage is 33.49%. Comparing base [(`8729247`)](https://app.codecov.io/gh/scrapy/scrapy/commit/872924721344502cca92a72ce68d5f78a9b9e5bc?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy) to head [(`0ab0281`)](https://app.codecov.io/gh/scrapy/scrapy/commit/0ab028103890369c43663369c55055ec61b3f38b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy).

| [Files with missing lines](https://app.codecov.io/gh/scrapy/scrapy/pull/6738?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy) | Patch % | Lines |
|---|---|---|
| [scrapy/utils/misc.py](https://app.codecov.io/gh/scrapy/scrapy/pull/6738?src=pr&el=tree&filepath=scrapy%2Futils%2Fmisc.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy#diff-c2NyYXB5L3V0aWxzL21pc2MucHk=) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/scrapy/scrapy/pull/6738?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy) |

<details><summary>Additional details and impacted files</summary>


```diff
@@             Coverage Diff             @@
##           master    #6738       +/-   ##
===========================================
- Coverage   90.67%   33.49%   -57.18%     
===========================================
  Files         162      162               
  Lines       12129    12124        -5     
  Branches     1567     1568        +1     
===========================================
- Hits        10998     4061     -6937     
- Misses        829     7925     +7096     
+ Partials      302      138      -164     
```

| [Files with missing lines](https://app.codecov.io/gh/scrapy/scrapy/pull/6738?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy) | Coverage Δ | |
|---|---|---|
| [scrapy/settings/default\_settings.py](https://app.codecov.io/gh/scrapy/scrapy/pull/6738?src=pr&el=tree&filepath=scrapy%2Fsettings%2Fdefault_settings.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy#diff-c2NyYXB5L3NldHRpbmdzL2RlZmF1bHRfc2V0dGluZ3MucHk=) | `98.83% <100.00%> (-1.17%)` | :arrow_down: |
| [scrapy/utils/misc.py](https://app.codecov.io/gh/scrapy/scrapy/pull/6738?src=pr&el=tree&filepath=scrapy%2Futils%2Fmisc.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy#diff-c2NyYXB5L3V0aWxzL21pc2MucHk=) | `32.65% <0.00%> (-67.35%)` | :arrow_down: |

... and [144 files with indirect coverage changes](https://app.codecov.io/gh/scrapy/scrapy/pull/6738/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy)

</details>
<details><summary>🚀 New features to boost your workflow: </summary>

- ❄ [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
MehrazRumman: I will add the test too !! 
Gallaecio: I think the test failures may be due to some shortcuts taken in the current tests for the warning, i.e. the spider not being set up properly. Let me know if you need any help getting the existing tests to pass, your code looks fine.
MehrazRumman: @Gallaecio  Can you help me to review this PR ?
Gallaecio: Could you add an entry about this new setting to  `settings.rst`?

Patch:
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/core/scraper.py b/scrapy/core/scraper.py
index b664b61f6..496adb500 100644
--- a/scrapy/core/scraper.py
+++ b/scrapy/core/scraper.py
@@ -246,6 +246,7 @@ class Scraper:
             spider=spider,
         )
         assert self.crawler.stats
+        self.crawler.stats.inc_value("spider_exceptions/count", spider=spider)
         self.crawler.stats.inc_value(
             f"spider_exceptions/{_failure.value.__class__.__name__}", spider=spider
         )
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(


Test patch:
diff --git a/tests/test_closespider.py b/tests/test_closespider.py
index 476662789..4a17b254b 100644
--- a/tests/test_closespider.py
+++ b/tests/test_closespider.py
@@ -88,6 +88,7 @@ class TestCloseSpider(TestCase):
         assert reason == "closespider_errorcount"
         key = f"spider_exceptions/{crawler.spider.exception_cls.__name__}"
         errorcount = crawler.stats.get_value(key)
+        assert crawler.stats.get_value("spider_exceptions/count") >= close_on
         assert errorcount >= close_on
 
     @defer.inlineCallbacks
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)