From b70443f2d06b1b0ad8c474fc5e8a424e363bdd81 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Thu, 2 Jan 2025 00:31:26 +0500 Subject: [PATCH] Split ruff and pylint ignores into two categories, some pylint cleanup. --- docs/_ext/scrapydocs.py | 1 + docs/conf.py | 2 + pyproject.toml | 81 +++++++++++++++++---------------- scrapy/contracts/__init__.py | 2 +- scrapy/extensions/feedexport.py | 2 + scrapy/extensions/telnet.py | 4 +- scrapy/interfaces.py | 2 + scrapy/shell.py | 17 ++++--- scrapy/utils/console.py | 2 +- scrapy/utils/deprecate.py | 1 + scrapy/utils/display.py | 1 + scrapy/utils/engine.py | 2 +- tests/test_contracts.py | 4 +- tests/test_exporters.py | 4 +- tests/test_item.py | 2 +- tests/test_link.py | 2 +- tests/test_scrapy__getattr__.py | 2 +- tests/test_selector.py | 2 +- tests/test_squeues_request.py | 8 ++-- 19 files changed, 77 insertions(+), 64 deletions(-) diff --git a/docs/_ext/scrapydocs.py b/docs/_ext/scrapydocs.py index c23a89089..9b63f39f6 100644 --- a/docs/_ext/scrapydocs.py +++ b/docs/_ext/scrapydocs.py @@ -1,3 +1,4 @@ +# pylint: disable=import-error from operator import itemgetter from docutils import nodes diff --git a/docs/conf.py b/docs/conf.py index 7a5166053..d06828bcc 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -8,6 +8,8 @@ # # All configuration values have a default; values that are commented out # serve to show the default. + +# pylint: disable=import-error import os import sys from pathlib import Path diff --git a/pyproject.toml b/pyproject.toml index a75f3b6db..88005ec4a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -125,43 +125,30 @@ extension-pkg-allow-list=[ [tool.pylint."MESSAGES CONTROL"] disable = [ - "abstract-method", - "arguments-differ", - "arguments-renamed", + # Ones we want to ignore "attribute-defined-outside-init", "broad-exception-caught", "consider-using-with", "cyclic-import", - "dangerous-default-value", "disallowed-name", - "duplicate-code", # https://github.com/PyCQA/pylint/issues/214 - "eval-used", + "duplicate-code", # https://github.com/pylint-dev/pylint/issues/214 "fixme", - "import-error", "import-outside-toplevel", - "inherit-non-class", + "inherit-non-class", # false positives with create_deprecated_class() "invalid-name", "invalid-overridden-method", - "isinstance-second-argument-not-valid-type", - "keyword-arg-before-vararg", + "isinstance-second-argument-not-valid-type", # false positives with create_deprecated_class() "line-too-long", "logging-format-interpolation", "logging-fstring-interpolation", "logging-not-lazy", "missing-docstring", "no-member", - "no-method-argument", - "no-name-in-module", - "no-self-argument", - "no-value-for-parameter", # https://github.com/pylint-dev/pylint/issues/3268 + "no-value-for-parameter", # https://github.com/pylint-dev/pylint/issues/3268 "not-callable", - "pointless-statement", - "pointless-string-statement", "protected-access", - "raise-missing-from", "redefined-builtin", "redefined-outer-name", - "signature-differs", "too-few-public-methods", "too-many-ancestors", "too-many-arguments", @@ -173,14 +160,23 @@ disable = [ "too-many-positional-arguments", "too-many-public-methods", "too-many-return-statements", - "unbalanced-tuple-unpacking", - "unnecessary-dunder-call", "unused-argument", "unused-import", "unused-variable", - "used-before-assignment", - "useless-return", + "useless-return", # https://github.com/pylint-dev/pylint/issues/6530 "wrong-import-position", + + # Ones that we may want to address (fix, ignore per-line or move to "don't want to fix") + "abstract-method", + "arguments-differ", + "arguments-renamed", + "dangerous-default-value", + "keyword-arg-before-vararg", + "pointless-statement", + "raise-missing-from", + "unbalanced-tuple-unpacking", + "unnecessary-dunder-call", + "used-before-assignment", ] [tool.pytest.ini_options] @@ -270,22 +266,8 @@ extend-select = [ "YTT", ] ignore = [ - # Assigning to `os.environ` doesn't clear the environment. - "B003", - # Do not use mutable data structures for argument defaults. - "B006", - # Loop control variable not used within the loop body. - "B007", - # Do not perform function calls in argument defaults. - "B008", - # Star-arg unpacking after a keyword argument is strongly discouraged. - "B026", - # Found useless expression. - "B018", - # No explicit stacklevel argument found. - "B028", - # Within an `except` clause, raise exceptions with `raise ... from` - "B904", + # Ones we want to ignore + # Missing docstring in public module "D100", # Missing docstring in public class @@ -346,12 +328,31 @@ ignore = [ "S321", # Argument default set to insecure SSL protocol "S503", - # Use capitalized environment variable - "SIM112", # Use a context manager for opening files "SIM115", # Yoda condition detected "SIM300", + + # Ones that we may want to address (fix, ignore per-line or move to "don't want to fix") + + # Assigning to `os.environ` doesn't clear the environment. + "B003", + # Do not use mutable data structures for argument defaults. + "B006", + # Loop control variable not used within the loop body. + "B007", + # Do not perform function calls in argument defaults. + "B008", + # Found useless expression. + "B018", + # Star-arg unpacking after a keyword argument is strongly discouraged. + "B026", + # No explicit stacklevel argument found. + "B028", + # Within an `except` clause, raise exceptions with `raise ... from` + "B904", + # Use capitalized environment variable + "SIM112", ] [tool.ruff.lint.per-file-ignores] diff --git a/scrapy/contracts/__init__.py b/scrapy/contracts/__init__.py index 3b4f932a0..bdb68c4ad 100644 --- a/scrapy/contracts/__init__.py +++ b/scrapy/contracts/__init__.py @@ -199,7 +199,7 @@ def _create_testcase(method: Callable, desc: str) -> TestCase: spider = method.__self__.name # type: ignore[attr-defined] class ContractTestCase(TestCase): - def __str__(_self) -> str: + def __str__(_self) -> str: # pylint: disable=no-self-argument return f"[{spider}] {method.__name__} ({desc})" name = f"{spider}_{method.__name__}" diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index 8a3d607b0..c6e2aa0dd 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -110,6 +110,8 @@ class ItemFilter: class IFeedStorage(Interface): """Interface that all Feed Storages must implement""" + # pylint: disable=no-self-argument + def __init__(uri, *, feed_options=None): # pylint: disable=super-init-not-called """Initialize the storage with the parameters given in the URI and the feed-specific options (see :setting:`FEEDS`)""" diff --git a/scrapy/extensions/telnet.py b/scrapy/extensions/telnet.py index ee28d86ba..189b1953b 100644 --- a/scrapy/extensions/telnet.py +++ b/scrapy/extensions/telnet.py @@ -84,7 +84,9 @@ class TelnetConsole(protocol.ServerFactory): """An implementation of IPortal""" @defers - def login(self_, credentials, mind, *interfaces): + def login( + self_, credentials, mind, *interfaces + ): # pylint: disable=no-self-argument if not ( credentials.username == self.username.encode("utf8") and credentials.checkPassword(self.password.encode("utf8")) diff --git a/scrapy/interfaces.py b/scrapy/interfaces.py index 9a2c5f170..13a4d822d 100644 --- a/scrapy/interfaces.py +++ b/scrapy/interfaces.py @@ -1,3 +1,5 @@ +# pylint:disable=no-method-argument,no-self-argument + from zope.interface import Interface diff --git a/scrapy/shell.py b/scrapy/shell.py index 4a5b9e9cf..5e5e57a9a 100644 --- a/scrapy/shell.py +++ b/scrapy/shell.py @@ -71,17 +71,16 @@ class Shell: else: self.populate_vars() if self.code: + # pylint: disable-next=eval-used print(eval(self.code, globals(), self.vars)) # noqa: S307 else: - """ - Detect interactive shell setting in scrapy.cfg - e.g.: ~/.config/scrapy.cfg or ~/.scrapy.cfg - [settings] - # shell can be one of ipython, bpython or python; - # to be used as the interactive python console, if available. - # (default is ipython, fallbacks in the order listed above) - shell = python - """ + # Detect interactive shell setting in scrapy.cfg + # e.g.: ~/.config/scrapy.cfg or ~/.scrapy.cfg + # [settings] + # # shell can be one of ipython, bpython or python; + # # to be used as the interactive python console, if available. + # # (default is ipython, fallbacks in the order listed above) + # shell = python cfg = get_config() section, option = "settings", "shell" env = os.environ.get("SCRAPY_PYTHON_SHELL") diff --git a/scrapy/utils/console.py b/scrapy/utils/console.py index 95844a48c..7425543ff 100644 --- a/scrapy/utils/console.py +++ b/scrapy/utils/console.py @@ -59,7 +59,7 @@ def _embed_ptpython_shell( namespace: dict[str, Any] = {}, banner: str = "" ) -> EmbedFuncT: """Start a ptpython shell""" - import ptpython.repl + import ptpython.repl # pylint: disable=import-error @wraps(_embed_ptpython_shell) def wrapper(namespace: dict[str, Any] = namespace, banner: str = "") -> None: diff --git a/scrapy/utils/deprecate.py b/scrapy/utils/deprecate.py index 0a0acc742..20d03cae6 100644 --- a/scrapy/utils/deprecate.py +++ b/scrapy/utils/deprecate.py @@ -57,6 +57,7 @@ def create_deprecated_class( # https://github.com/python/mypy/issues/4177 class DeprecatedClass(new_class.__class__): # type: ignore[misc, name-defined] + # pylint: disable=no-self-argument deprecated_class: type | None = None warned_on_subclass: bool = False diff --git a/scrapy/utils/display.py b/scrapy/utils/display.py index 39f46270b..20744a604 100644 --- a/scrapy/utils/display.py +++ b/scrapy/utils/display.py @@ -30,6 +30,7 @@ def _tty_supports_color() -> bool: def _colorize(text: str, colorize: bool = True) -> str: + # pylint: disable=no-name-in-module if not colorize or not sys.stdout.isatty() or not _tty_supports_color(): return text try: diff --git a/scrapy/utils/engine.py b/scrapy/utils/engine.py index 1948009e8..52f29e22c 100644 --- a/scrapy/utils/engine.py +++ b/scrapy/utils/engine.py @@ -32,7 +32,7 @@ def get_engine_status(engine: ExecutionEngine) -> list[tuple[str, Any]]: checks: list[tuple[str, Any]] = [] for test in tests: try: - checks += [(test, eval(test))] # noqa: S307 + checks += [(test, eval(test))] # noqa: S307 # pylint: disable=eval-used except Exception as e: checks += [(test, f"{type(e).__name__} (exception)")] diff --git a/tests/test_contracts.py b/tests/test_contracts.py index 743889234..f7581707b 100644 --- a/tests/test_contracts.py +++ b/tests/test_contracts.py @@ -517,8 +517,8 @@ class ContractsManagerTest(unittest.TestCase): super().__init__(*args, **kwargs) self.visited = 0 - def start_requests(s): - return self.conman.from_spider(s, self.results) + def start_requests(self_): # pylint: disable=no-self-argument + return self.conman.from_spider(self_, self.results) def parse_first(self, response): self.visited += 1 diff --git a/tests/test_exporters.py b/tests/test_exporters.py index 522c6638d..970f8d2f5 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -216,7 +216,9 @@ class PprintItemExporterTest(BaseItemExporterTest): return PprintItemExporter(self.output, **kwargs) def _check_output(self): - self._assert_expected_item(eval(self.output.getvalue())) + self._assert_expected_item( + eval(self.output.getvalue()) # pylint: disable=eval-used + ) class PprintItemExporterDataclassTest(PprintItemExporterTest): diff --git a/tests/test_item.py b/tests/test_item.py index 13243b67f..35212c153 100644 --- a/tests/test_item.py +++ b/tests/test_item.py @@ -54,7 +54,7 @@ class ItemTest(unittest.TestCase): self.assertEqual(itemrepr, "{'name': 'John Doe', 'number': 123}") - i2 = eval(itemrepr) + i2 = eval(itemrepr) # pylint: disable=eval-used self.assertEqual(i2["name"], "John Doe") self.assertEqual(i2["number"], 123) diff --git a/tests/test_link.py b/tests/test_link.py index 7ba0851ae..35723bbd6 100644 --- a/tests/test_link.py +++ b/tests/test_link.py @@ -49,7 +49,7 @@ class LinkTest(unittest.TestCase): l1 = Link( "http://www.example.com", text="test", fragment="something", nofollow=True ) - l2 = eval(repr(l1)) + l2 = eval(repr(l1)) # pylint: disable=eval-used self._assert_same_links(l1, l2) def test_bytes_url(self): diff --git a/tests/test_scrapy__getattr__.py b/tests/test_scrapy__getattr__.py index 979c42267..443e26a3c 100644 --- a/tests/test_scrapy__getattr__.py +++ b/tests/test_scrapy__getattr__.py @@ -3,7 +3,7 @@ import warnings def test_deprecated_twisted_version(): with warnings.catch_warnings(record=True) as warns: - from scrapy import twisted_version + from scrapy import twisted_version # pylint: disable=no-name-in-module assert twisted_version is not None assert isinstance(twisted_version, tuple) diff --git a/tests/test_selector.py b/tests/test_selector.py index 1b5f3f018..857c7d626 100644 --- a/tests/test_selector.py +++ b/tests/test_selector.py @@ -264,7 +264,7 @@ class JMESPathTestCase(unittest.TestCase): ) @pytest.mark.skipif(PARSEL_18_PLUS, reason="parsel >= 1.8 supports jmespath") - def test_jmespath_not_available(my_json_page) -> None: + def test_jmespath_not_available(self) -> None: body = """ { "website": {"name": "Example"} diff --git a/tests/test_squeues_request.py b/tests/test_squeues_request.py index 02ea8027f..04eeae4dc 100644 --- a/tests/test_squeues_request.py +++ b/tests/test_squeues_request.py @@ -1,3 +1,7 @@ +""" +Queues that handle requests +""" + import shutil import tempfile import unittest @@ -16,10 +20,6 @@ from scrapy.squeues import ( ) from scrapy.utils.test import get_crawler -""" -Queues that handle requests -""" - class BaseQueueTestCase(unittest.TestCase): def setUp(self):