diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index defd5fd14..3bcfe3a20 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -2046,13 +2046,13 @@ Default: ``True`` By default, when loading spiders, Scrapy only loads :class:`~scrapy.spiders.Spider` subclasses that have a non-empty -:class:`~scrapy.spiders.Spider.name` unless they are decorated with +:attr:`~scrapy.Spider.name` unless they are decorated with :func:`~scrapy.spiders.ignore_spider`. If :setting:`SPIDER_LOADER_REQUIRE_NAME` is ``False``, Scrapy loads all :class:`~scrapy.spiders.Spider` subclasses unless they are decorated with :func:`~scrapy.spiders.ignore_spider`. If they do not have a non-empty -:class:`~scrapy.spiders.Spider.name`, their fully-qualified class name is used +:attr:`~scrapy.Spider.name`, their fully-qualified class name is used as a name. .. note:: This is a :ref:`pre-crawler setting `. diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index dea0cc793..68c384881 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -45,8 +45,7 @@ scrapy.Spider If :setting:`SPIDER_LOADER_REQUIRE_NAME` is ``True`` and you use the default Scrapy spider loader (see :setting:`SPIDER_LOADER_CLASS`), a non-empty name is required for the spider to be discoverable by the - Scrapy commands :command:`crawl`, :command:`list`, and - :command:`runspider`. + Scrapy commands :command:`crawl` and :command:`list`. The spider name must be unique to one spider class. If two or more spiders have the same name, Scrapy commands :command:`crawl` and diff --git a/scrapy/commands/runspider.py b/scrapy/commands/runspider.py index 5d82a2b61..d24b85918 100644 --- a/scrapy/commands/runspider.py +++ b/scrapy/commands/runspider.py @@ -53,12 +53,14 @@ class Command(BaseRunSpiderCommand): module = _import_file(filename) except (ImportError, ValueError) as e: raise UsageError(f"Unable to load {str(filename)!r}: {e}\n") from e - assert self.settings is not None - require_name = self.settings.getbool("SPIDER_LOADER_REQUIRE_NAME") - spclasses = list(iter_spider_classes(module, require_name=require_name)) + # The spider is looked up by file name, so it does not need a name of + # its own. Named spiders still win over nameless ones, which in a file + # with both are usually base spiders. + spclasses = list(iter_spider_classes(module, require_name=False)) if not spclasses: raise UsageError(f"No spider found in file: {filename}\n") - spidercls = spclasses.pop() + named = [spcls for spcls in spclasses if getattr(spcls, "name", None)] + spidercls = (named or spclasses).pop() assert self.crawler_process self.crawler_process.crawl(spidercls, **opts.spargs) diff --git a/scrapy/spiderloader.py b/scrapy/spiderloader.py index c10179a3d..199f2d859 100644 --- a/scrapy/spiderloader.py +++ b/scrapy/spiderloader.py @@ -8,7 +8,6 @@ from typing import TYPE_CHECKING, Protocol, cast # working around https://github.com/sphinx-doc/sphinx/issues/10400 from scrapy import Request, Spider # noqa: TC001 from scrapy.utils.misc import load_object, walk_modules_iter -from scrapy.utils.python import global_object_name from scrapy.utils.spider import iter_spider_classes if TYPE_CHECKING: @@ -84,10 +83,8 @@ class SpiderLoader: ) def _load_spiders(self, module: ModuleType) -> None: - classes = iter_spider_classes(module, require_name=self.require_name) - for spcls in classes: - qualname = global_object_name(spcls) - name = getattr(spcls, "name", None) or qualname + for spcls in iter_spider_classes(module, require_name=self.require_name): + name = spcls._default_name() self._found[name].append((module.__name__, spcls.__name__)) self._spiders[name] = spcls diff --git a/scrapy/spiders/__init__.py b/scrapy/spiders/__init__.py index adb1b0eaa..cc5f9e487 100644 --- a/scrapy/spiders/__init__.py +++ b/scrapy/spiders/__init__.py @@ -33,21 +33,19 @@ if TYPE_CHECKING: _SpiderT = TypeVar("_SpiderT", bound="type[Spider]") -# Only the classes themselves are ignored, never their subclasses. -_ignored_spiders: set[type[Spider]] = set() - def ignore_spider(cls: _SpiderT) -> _SpiderT: """Mark a :class:`~scrapy.spiders.Spider` subclass to be ignored. - The default spider loader (see :setting:`SPIDER_LOADER_CLASS`) does not - make marked spider classes available for the :command:`crawl`, - :command:`list`, and :command:`runspider` commands. + Marked spider classes are not available to the :command:`crawl`, + :command:`list` and :command:`runspider` commands. Only the decorated + class is marked; its subclasses are unaffected. """ - _ignored_spiders.add(cls) + cls._ignore_spider = True return cls +@ignore_spider class Spider(object_ref): """Base class that any spider must subclass. @@ -58,6 +56,7 @@ class Spider(object_ref): name: str custom_settings: dict[str, Any] | None = None + _ignore_spider: bool #: Start URLs. See :meth:`start`. start_urls: list[str] @@ -66,14 +65,22 @@ class Spider(object_ref): if name is not None: self.name: str = name elif not getattr(self, "name", None): - self.name = global_object_name(self.__class__) + self.name = type(self)._default_name() self.__dict__.update(kwargs) if not hasattr(self, "start_urls"): self.start_urls: list[str] = [] + @classmethod + def _default_name(cls) -> str: + """Return the name under which spider loaders and commands know this + spider class, which falls back to its import path.""" + return getattr(cls, "name", None) or global_object_name(cls) + @classmethod def _is_ignored(cls) -> bool: - return cls in _ignored_spiders + # The mark set by ignore_spider() is read from the class __dict__ so + # that subclasses do not inherit it. + return "_ignore_spider" in cls.__dict__ @property def logger(self) -> SpiderLoggerAdapter: @@ -204,9 +211,6 @@ class Spider(object_ref): return f"<{type(self).__name__} {self.name!r} at 0x{id(self):0x}>" -ignore_spider(Spider) - - # Top-level imports from scrapy.spiders.crawl import CrawlSpider, Rule from scrapy.spiders.feed import CSVFeedSpider, XMLFeedSpider diff --git a/scrapy/utils/spider.py b/scrapy/utils/spider.py index 323c86576..f1904e8f0 100644 --- a/scrapy/utils/spider.py +++ b/scrapy/utils/spider.py @@ -47,15 +47,6 @@ def iterate_spider_output( return arg_to_iter(d) -def _is_ignored(obj: Any, *, require_name: bool) -> bool: - return ( - not inspect.isclass(obj) - or not issubclass(obj, Spider) - or obj._is_ignored() - or (require_name and not getattr(obj, "name", None)) - ) - - def iter_spider_classes( module: ModuleType, *, @@ -67,12 +58,15 @@ def iter_spider_classes( If `require_name` is ``True`` (default), any :class:`~scrapy.spiders.Spider` subclass without a non-empty - :class:`~scrapy.spiders.Spider.name` is also excluded. + :attr:`~scrapy.Spider.name` is also excluded. """ for obj in vars(module).values(): if ( - not _is_ignored(obj, require_name=require_name) + inspect.isclass(obj) + and issubclass(obj, Spider) and obj.__module__ == module.__name__ + and not obj._is_ignored() + and (not require_name or getattr(obj, "name", None)) ): yield obj diff --git a/tests/test_command_check.py b/tests/test_command_check.py index 240f44584..a8ccf92c1 100644 --- a/tests/test_command_check.py +++ b/tests/test_command_check.py @@ -122,6 +122,25 @@ class CheckSpider(scrapy.Spider): """ self._test_contract(proj_path, contracts, parse_def) + def test_check_list_nameless_spider(self, proj_path: Path) -> None: + spider = proj_path / self.project_name / "spiders" / "namelessspider.py" + spider.write_text( + ''' +import scrapy + +class NamelessSpider(scrapy.Spider): + def parse(self, response): + """ + @url data:, + """ +''', + encoding="utf-8", + ) + name = f"{self.project_name}.spiders.namelessspider.NamelessSpider" + ret, out, err = proc("check", "-l", name, cwd=proj_path) + assert ret == 0, err + assert out == f"{name}\n * parse\n" + def test_SCRAPY_CHECK_set(self, proj_path: Path) -> None: parse_def = """ import os diff --git a/tests/test_command_crawl.py b/tests/test_command_crawl.py index 5306e3bf8..94b1cc21a 100644 --- a/tests/test_command_crawl.py +++ b/tests/test_command_crawl.py @@ -35,6 +35,24 @@ class TestCrawlCommand(TestProjectBase): "running 'scrapy crawl' with more than one spider is not supported" in err ) + def test_nameless_spider(self, proj_path: Path) -> None: + spider_code = """ +import scrapy + +class MySpider(scrapy.Spider): + async def start(self): + self.logger.debug('It works!') + return + yield +""" + (proj_path / self.project_name / "spiders" / "myspider.py").write_text( + spider_code, encoding="utf-8" + ) + name = f"{self.project_name}.spiders.myspider.MySpider" + _, _, log = proc("crawl", name, cwd=proj_path) + assert f"[{name}] DEBUG: It works!" in log + assert "Spider closed (finished)" in log + def test_no_output(self, proj_path: Path) -> None: spider_code = """ import scrapy diff --git a/tests/test_command_runspider.py b/tests/test_command_runspider.py index 11036eaeb..6e8213b67 100644 --- a/tests/test_command_runspider.py +++ b/tests/test_command_runspider.py @@ -132,6 +132,39 @@ class MySpider(scrapy.Spider): assert ("[scrapy]" in log1) is value assert ("[scrapy.core.engine]" in log1) is not value + def test_runspider_nameless_spider(self, tmp_path: Path) -> None: + nameless_spider = """ +import scrapy + +class MySpider(scrapy.Spider): + async def start(self): + self.logger.debug("It Works!") + return + yield +""" + log = self.get_log(tmp_path, nameless_spider) + assert "[myspider.MySpider] DEBUG: It Works!" in log + assert "INFO: Spider closed (finished)" in log + + def test_runspider_prefers_named_spider(self, tmp_path: Path) -> None: + """A base spider defined after the spider itself does not shadow it.""" + base_last_spider = """ +import scrapy + +class MySpider(scrapy.Spider): + name = 'myspider' + + async def start(self): + self.logger.debug("It Works!") + return + yield + +class MyBaseSpider(scrapy.Spider): + pass +""" + log = self.get_log(tmp_path, base_last_spider) + assert "[myspider] DEBUG: It Works!" in log + def test_runspider_no_spider_found(self, tmp_path: Path) -> None: log = self.get_log(tmp_path, "from scrapy.spiders import Spider\n") assert "No spider found in file" in log diff --git a/tests/test_commands.py b/tests/test_commands.py index f20ecc153..943a9c43b 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -432,6 +432,31 @@ class TestMiscCommands(TestProjectBase): subdir.mkdir(exist_ok=True) assert call("list", cwd=subdir) == 0 + @pytest.mark.parametrize("require_name", [False, True]) + def test_list_nameless(self, proj_path: Path, require_name: bool) -> None: + (proj_path / self.project_name / "spiders" / "nameless.py").write_text( + "from scrapy import Spider\n" + "\n" + "\n" + "class NamelessSpider(Spider):\n" + " pass\n" + "\n" + "\n" + "class NamedSpider(Spider):\n" + ' name = "named"\n', + encoding="utf-8", + ) + returncode, out, err = proc( + "list", + "-s", + f"SPIDER_LOADER_REQUIRE_NAME={require_name}", + cwd=proj_path, + ) + assert returncode == 0, err + nameless = f"{self.project_name}.spiders.nameless.NamelessSpider" + expected = ["named"] if require_name else ["named", nameless] + assert out.split() == expected + class TestCommandListing(TestProjectBase): """Tests for the command list that ``scrapy`` prints when called without a diff --git a/tests/test_spiderloader/__init__.py b/tests/test_spiderloader/__init__.py index 81840f7e0..951d4fa76 100644 --- a/tests/test_spiderloader/__init__.py +++ b/tests/test_spiderloader/__init__.py @@ -1,6 +1,7 @@ import contextlib import shutil import sys +import warnings from pathlib import Path from unittest import mock @@ -13,6 +14,7 @@ from scrapy.crawler import CrawlerRunner from scrapy.http import Request from scrapy.settings import Settings from scrapy.spiderloader import DummySpiderLoader, SpiderLoader, get_spider_loader +from tests.test_spiderloader.nameless_spiders.nameless1 import NamelessSpider module_dir = Path(__file__).resolve().parent @@ -165,6 +167,69 @@ class TestSpiderLoader: assert not spiders +class TestNamelessSpiderLoader: + module = "tests.test_spiderloader.nameless_spiders" + nameless1 = f"{module}.nameless1.NamelessSpider" + nameless2 = f"{module}.nameless2.NamelessSpider" + + @pytest.fixture + def spider_loader(self): + settings = Settings( + { + "SPIDER_MODULES": [self.module], + "SPIDER_LOADER_REQUIRE_NAME": False, + } + ) + return SpiderLoader.from_settings(settings) + + def test_list(self, spider_loader): + assert set(spider_loader.list()) == { + "subclass", + self.nameless1, + self.nameless2, + } + + def test_list_require_name(self): + settings = Settings({"SPIDER_MODULES": [self.module]}) + spider_loader = SpiderLoader.from_settings(settings) + assert set(spider_loader.list()) == {"subclass"} + + def test_load(self, spider_loader): + assert spider_loader.load(self.nameless1) is NamelessSpider + + def test_instance_name(self, spider_loader): + """Spiders are instantiated with the name that the loader knows them + by.""" + for name in spider_loader.list(): + assert spider_loader.load(name)().name == name + + def test_find_by_request(self, spider_loader): + assert spider_loader.find_by_request( + Request("https://nameless.example.com") + ) == [self.nameless1] + + def test_no_dupename_warning(self): + settings = Settings( + { + "SPIDER_MODULES": [self.module], + "SPIDER_LOADER_REQUIRE_NAME": False, + } + ) + with warnings.catch_warnings(): + warnings.simplefilter("error", UserWarning) + SpiderLoader.from_settings(settings) + + def test_crawler_runner_loading(self, spider_loader): + runner = CrawlerRunner( + { + "SPIDER_MODULES": [self.module], + "SPIDER_LOADER_REQUIRE_NAME": False, + } + ) + crawler = runner.create_crawler(self.nameless1) + assert crawler.spidercls is NamelessSpider + + class TestDuplicateSpiderNameLoader: def test_dupename_warning(self, spider_loader_env): settings, spiders_dir = spider_loader_env diff --git a/tests/test_spiderloader/nameless_spiders/__init__.py b/tests/test_spiderloader/nameless_spiders/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/test_spiderloader/nameless_spiders/ignored.py b/tests/test_spiderloader/nameless_spiders/ignored.py new file mode 100644 index 000000000..fd9113eae --- /dev/null +++ b/tests/test_spiderloader/nameless_spiders/ignored.py @@ -0,0 +1,10 @@ +from scrapy.spiders import Spider, ignore_spider + + +@ignore_spider +class IgnoredSpider(Spider): + name = "ignored" + + +class SubclassSpider(IgnoredSpider): + name = "subclass" diff --git a/tests/test_spiderloader/nameless_spiders/nameless1.py b/tests/test_spiderloader/nameless_spiders/nameless1.py new file mode 100644 index 000000000..dc910cdd4 --- /dev/null +++ b/tests/test_spiderloader/nameless_spiders/nameless1.py @@ -0,0 +1,5 @@ +from scrapy.spiders import Spider + + +class NamelessSpider(Spider): + allowed_domains = ["nameless.example.com"] diff --git a/tests/test_spiderloader/nameless_spiders/nameless2.py b/tests/test_spiderloader/nameless_spiders/nameless2.py new file mode 100644 index 000000000..83c0364d8 --- /dev/null +++ b/tests/test_spiderloader/nameless_spiders/nameless2.py @@ -0,0 +1,7 @@ +from scrapy.spiders import Spider + + +# Same class name as in the nameless1 module, to check that nameless spiders +# are told apart by their full import path. +class NamelessSpider(Spider): + pass diff --git a/tests/utils/bases/spider.py b/tests/utils/bases/spider.py index df7420130..db3b0033e 100644 --- a/tests/utils/bases/spider.py +++ b/tests/utils/bases/spider.py @@ -41,6 +41,16 @@ class TestSpiderBase(ABC): spider = self.spider_class() assert spider.name == global_object_name(self.spider_class) + def test_ignored(self): + """Base spiders shipped by Scrapy are ignored, their subclasses are + not.""" + + class Subclass(self.spider_class): + pass + + assert self.spider_class._is_ignored() + assert not Subclass._is_ignored() + def test_from_crawler_crawler_and_settings_population(self): crawler = get_crawler() spider = self.spider_class.from_crawler(crawler, "example.com")