From cc3c1e2050ad37fb60f5c57ae9c05d7a6ec6e010 Mon Sep 17 00:00:00 2001 From: toofar Date: Tue, 15 Oct 2024 18:28:35 +1300 Subject: [PATCH] Enable pylint Too many positional arguments warning This re-enables the pylint too-many-positional-arguments for the main application code. It's still disabled for tests because that's how you pull in pytlint fixtures, and I don't think we want to push people into being creative with fixtures just to get around that. When functions are called with many positional arguments the reader has to do a bit of heavy lifting to figure out in what position a value is being passed, and it's easier to make mistakes. So I would like to encourage using keyword arguments for long argument lists. I've set the `max-positional-arguments` to a completely arbitrary 7, from a completely arbitrary 5, because there were many more violations under 7. If like 99% of our functions fit under 7 it's probably fine. Regarding the exceptions: * objreg.register: I grepped it and it looks like everything is only passing the first two args as positional already, lucky! * `_get_color_percentage`: only one usage of it, but I was in "add directive comment" mode * update_3rdparty.py: only one usage, already using kwargs * pyqtProperty: idk * commands.py: "its complicated". Many methods in this file map to commands used in qutebrowser's command mode. In that case it's usual for them to be called as flags, rather than positional. But it could be complicated to wade into that, and having one file excluded isn't so bad. --- .pylintrc | 4 ++-- qutebrowser/browser/commands.py | 2 ++ qutebrowser/qt/_core_pyqtproperty.py | 2 +- qutebrowser/utils/objreg.py | 1 + qutebrowser/utils/qtutils.py | 8 +++++--- scripts/dev/run_pylint_on_tests.py | 1 + scripts/dev/update_3rdparty.py | 2 +- 7 files changed, 13 insertions(+), 7 deletions(-) diff --git a/.pylintrc b/.pylintrc index 3830f8d44..1cb4e0f1b 100644 --- a/.pylintrc +++ b/.pylintrc @@ -45,7 +45,6 @@ disable=locally-disabled, too-many-locals, too-many-branches, too-many-statements, - too-many-positional-arguments, too-few-public-methods, import-outside-toplevel, consider-using-f-string, @@ -72,7 +71,8 @@ argument-rgx=[a-z_][a-z0-9_]{0,30}$ variable-rgx=[a-z_][a-z0-9_]{0,30}$ docstring-min-length=3 no-docstring-rgx=(^_|^main$) -class-const-naming-style = snake_case +class-const-naming-style=snake_case +max-positional-arguments=7 [FORMAT] # FIXME:v4 (lint) down to 88 again once we use black diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index f686f522c..65d6635a3 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -2,6 +2,8 @@ # # SPDX-License-Identifier: GPL-3.0-or-later +# pylint: disable=too-many-positional-arguments + """Command dispatcher for TabbedBrowser.""" import os.path diff --git a/qutebrowser/qt/_core_pyqtproperty.py b/qutebrowser/qt/_core_pyqtproperty.py index 50752de41..c2b034df3 100644 --- a/qutebrowser/qt/_core_pyqtproperty.py +++ b/qutebrowser/qt/_core_pyqtproperty.py @@ -33,7 +33,7 @@ if typing.TYPE_CHECKING: ) class pyqtProperty: - def __init__( + def __init__( # pylint: disable=too-many-positional-arguments self, type: typing.Union[type, str], fget: typing.Optional[typing.Callable[[QObjectT], TPropertyTypeVal]] = None, diff --git a/qutebrowser/utils/objreg.py b/qutebrowser/utils/objreg.py index aa5bfc7f9..c027b3cf6 100644 --- a/qutebrowser/utils/objreg.py +++ b/qutebrowser/utils/objreg.py @@ -241,6 +241,7 @@ def get(name: str, def register(name: str, obj: Any, + *, update: bool = False, scope: str = None, registry: ObjectRegistry = None, diff --git a/qutebrowser/utils/qtutils.py b/qutebrowser/utils/qtutils.py index 765eddd05..a027db74a 100644 --- a/qutebrowser/utils/qtutils.py +++ b/qutebrowser/utils/qtutils.py @@ -529,9 +529,11 @@ class EventLoop(QEventLoop): return status -def _get_color_percentage(x1: int, y1: int, z1: int, a1: int, - x2: int, y2: int, z2: int, a2: int, - percent: int) -> tuple[int, int, int, int]: +def _get_color_percentage( # pylint: disable=too-many-positional-arguments + x1: int, y1: int, z1: int, a1: int, + x2: int, y2: int, z2: int, a2: int, + percent: int +) -> tuple[int, int, int, int]: """Get a color which is percent% interpolated between start and end. Args: diff --git a/scripts/dev/run_pylint_on_tests.py b/scripts/dev/run_pylint_on_tests.py index 580ef988f..d55caaf36 100644 --- a/scripts/dev/run_pylint_on_tests.py +++ b/scripts/dev/run_pylint_on_tests.py @@ -40,6 +40,7 @@ def main(): 'redefined-outer-name', 'unused-argument', 'too-many-arguments', + 'too-many-positional-arguments', # things which are okay in tests 'missing-docstring', 'protected-access', diff --git a/scripts/dev/update_3rdparty.py b/scripts/dev/update_3rdparty.py index 1258be106..7ad27c915 100755 --- a/scripts/dev/update_3rdparty.py +++ b/scripts/dev/update_3rdparty.py @@ -171,7 +171,7 @@ def test_dicts(): print('ERROR: {}'.format(response.status)) -def run(nsis=False, ace=False, pdfjs=True, legacy_pdfjs=False, fancy_dmg=False, +def run(*, nsis=False, ace=False, pdfjs=True, legacy_pdfjs=False, fancy_dmg=False, pdfjs_version=None, dicts=False, gh_token=None): """Update components based on the given arguments.""" if nsis: