mirror of https://github.com/scrapy/scrapy.git
Syntax Error Fixed (#6738)
* Syntax error fix issue #6731 * test case added * extra logic removed * mock spider fixture * Update scrapy/utils/misc.py Co-authored-by: Adrián Chaves <adrian@chaves.gal> * settings.rst updated * settings.rst updated * settings.rst updated --------- Co-authored-by: Adrián Chaves <adrian@chaves.gal>
This commit is contained in:
parent
2ee01efe49
commit
3ca882fba8
|
|
@ -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:
|
||||
------------------------------
|
||||
|
|
|
|||
|
|
@ -351,3 +351,5 @@ SPIDER_CONTRACTS_BASE = {
|
|||
"scrapy.contracts.default.ReturnsContract": 2,
|
||||
"scrapy.contracts.default.ScrapesContract": 3,
|
||||
}
|
||||
|
||||
WARN_ON_GENERATOR_RETURN_VALUE = True
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Reference in New Issue