From f3b718c9aaa16b8bbf77465b6a69623bdbbb7e82 Mon Sep 17 00:00:00 2001 From: farhan Date: Thu, 10 Sep 2026 19:49:12 +0500 Subject: [PATCH] build: restore pylint to the quality checks Restores pylint to the quality gate alongside ruff. Ruff is not a drop-in replacement for pylint -- it does not run the edx_lint plugins or catch issues such as undefined attributes and type mismatches. Ruff owns style, imports, formatting, unused-import, and line-length; pylint provides the deeper static analysis. Ownership is split so no check runs in both tools: - unused-import (F401) -> ruff. It is auto-fixable, so pylint disables unused-import. Package __init__ re-exports use the `import X as X` idiom so ruff treats them as intentional re-exports (no F401) without Sphinx re-documenting them under -W; pylint's useless-import-alias is disabled as a false positive on this PEP 484 idiom. - line-length (E501 / C0301) -> ruff, so `ruff format --fix` can auto-fix long lines; pylint's C0301 is disabled. Also renames the Makefile `lint` target to `quality` and removes stale pylint useless-suppression comments. Co-Authored-By: Claude Opus 4.8 (1M context) --- Makefile | 4 +- pylintrc | 396 ++++++++++++++++++ pylintrc_tweaks | 13 + pyproject.toml | 8 +- tox.ini | 1 + xblocks_contrib/annotatable/annotatable.py | 3 +- xblocks_contrib/legacy_utils/xml_utils.py | 2 +- xblocks_contrib/lti/lti.py | 4 +- xblocks_contrib/lti/lti_2_util.py | 6 +- xblocks_contrib/lti/tests/helpers.py | 6 +- xblocks_contrib/problem/capa/inputtypes.py | 4 +- .../problem/capa/tests/test_responsetypes.py | 4 +- .../problem/capa/xqueue_interface.py | 2 +- xblocks_contrib/problem/tests/__init__.py | 2 +- xblocks_contrib/video/validation.py | 4 +- 15 files changed, 430 insertions(+), 29 deletions(-) create mode 100644 pylintrc create mode 100644 pylintrc_tweaks diff --git a/Makefile b/Makefile index 1e478c4e..b08fde22 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ .DEFAULT_GOAL := help -.PHONY: upgrade help requirements lint format test docs +.PHONY: upgrade help requirements quality format test docs .PHONY: extract_translations compile_translations .PHONY: detect_changed_source_translations dummy_translations build_dummy_translations .PHONY: validate_translations pull_translations push_translations install_transifex_clients @@ -13,7 +13,7 @@ JS_TARGET := $(PACKAGE_NAME)/public/js/translations help: @perl -nle'print $& if m{^[\.a-zA-Z_-]+:.*?## .*$$}' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m %-25s\033[0m %s\n", $$1, $$2}' -lint: ## run linting checks +quality: ## run quality checks tox -e quality format: ## auto-fix ruff lint and formatting issues diff --git a/pylintrc b/pylintrc new file mode 100644 index 00000000..b321f717 --- /dev/null +++ b/pylintrc @@ -0,0 +1,396 @@ +# *************************** +# ** DO NOT EDIT THIS FILE ** +# *************************** +# +# This file was generated by edx-lint: https://github.com/openedx/edx-lint +# +# If you want to change this file, you have two choices, depending on whether +# you want to make a local change that applies only to this repo, or whether +# you want to make a central change that applies to all repos using edx-lint. +# +# Note: If your pylintrc file is simply out-of-date relative to the latest +# pylintrc in edx-lint, ensure you have the latest edx-lint installed +# and then follow the steps for a "LOCAL CHANGE". +# +# LOCAL CHANGE: +# +# 1. Edit the local pylintrc_tweaks file to add changes just to this +# repo's file. +# +# 2. Run: +# +# $ edx_lint write pylintrc +# +# 3. This will modify the local file. Submit a pull request to get it +# checked in so that others will benefit. +# +# +# CENTRAL CHANGE: +# +# 1. Edit the pylintrc file in the edx-lint repo at +# https://github.com/openedx/edx-lint/blob/master/edx_lint/files/pylintrc +# +# 2. install the updated version of edx-lint (in edx-lint): +# +# $ pip install . +# +# 3. Run (in edx-lint): +# +# $ edx_lint write pylintrc +# +# 4. Make a new version of edx_lint, submit and review a pull request with the +# pylintrc update, and after merging, update the edx-lint version and +# publish the new version. +# +# 5. In your local repo, install the newer version of edx-lint. +# +# 6. Run: +# +# $ edx_lint write pylintrc +# +# 7. This will modify the local file. Submit a pull request to get it +# checked in so that others will benefit. +# +# +# +# +# +# STAY AWAY FROM THIS FILE! +# +# +# +# +# +# SERIOUSLY. +# +# ------------------------------ +# Generated by edx-lint version: 6.2.0 +# ------------------------------ +[MASTER] +ignore = migrations +persistent = yes +load-plugins = edx_lint.pylint + +[MESSAGES CONTROL] +enable = + blacklisted-name, + line-too-long, + + abstract-class-instantiated, + abstract-method, + access-member-before-definition, + anomalous-backslash-in-string, + anomalous-unicode-escape-in-string, + arguments-differ, + assert-on-tuple, + assigning-non-slot, + assignment-from-no-return, + assignment-from-none, + attribute-defined-outside-init, + bad-except-order, + bad-format-character, + bad-format-string-key, + bad-format-string, + bad-open-mode, + bad-reversed-sequence, + bad-staticmethod-argument, + bad-str-strip-call, + bad-super-call, + binary-op-exception, + boolean-datetime, + catching-non-exception, + cell-var-from-loop, + confusing-with-statement, + continue-in-finally, + dangerous-default-value, + duplicate-argument-name, + duplicate-bases, + duplicate-except, + duplicate-key, + expression-not-assigned, + format-combined-specification, + format-needs-mapping, + function-redefined, + global-variable-undefined, + import-error, + import-self, + inconsistent-mro, + inherit-non-class, + init-is-generator, + invalid-all-object, + invalid-format-index, + invalid-length-returned, + invalid-sequence-index, + invalid-slice-index, + invalid-slots-object, + invalid-slots, + invalid-unary-operand-type, + logging-too-few-args, + logging-too-many-args, + logging-unsupported-format, + lost-exception, + method-hidden, + misplaced-bare-raise, + misplaced-future, + missing-format-argument-key, + missing-format-attribute, + missing-format-string-key, + no-member, + no-method-argument, + no-name-in-module, + no-self-argument, + no-value-for-parameter, + non-iterator-returned, + non-parent-method-called, + nonexistent-operator, + not-a-mapping, + not-an-iterable, + not-callable, + not-context-manager, + not-in-loop, + pointless-statement, + pointless-string-statement, + raising-bad-type, + raising-non-exception, + redefined-builtin, + redefined-outer-name, + redundant-keyword-arg, + repeated-keyword, + return-arg-in-generator, + return-in-init, + return-outside-function, + signature-differs, + super-init-not-called, + super-method-not-called, + syntax-error, + test-inherits-tests, + too-few-format-args, + too-many-format-args, + too-many-function-args, + translation-of-non-string, + truncated-format-string, + undefined-all-variable, + undefined-loop-variable, + undefined-variable, + unexpected-keyword-arg, + unexpected-special-method-signature, + unpacking-non-sequence, + unreachable, + unsubscriptable-object, + unsupported-binary-operation, + unsupported-membership-test, + unused-format-string-argument, + unused-format-string-key, + used-before-assignment, + using-constant-test, + yield-outside-function, + + astroid-error, + fatal, + method-check-failed, + parse-error, + raw-checker-failed, + + empty-docstring, + invalid-characters-in-docstring, + missing-docstring, + wrong-spelling-in-comment, + wrong-spelling-in-docstring, + + unused-argument, + unused-import, + unused-variable, + + eval-used, + exec-used, + + bad-classmethod-argument, + bad-mcs-classmethod-argument, + bad-mcs-method-argument, + bare-except, + broad-except, + consider-iterating-dictionary, + consider-using-enumerate, + global-at-module-level, + global-variable-not-assigned, + literal-used-as-attribute, + logging-format-interpolation, + logging-not-lazy, + multiple-imports, + multiple-statements, + no-classmethod-decorator, + no-staticmethod-decorator, + protected-access, + redundant-unittest-assert, + reimported, + simplifiable-if-statement, + simplifiable-range, + singleton-comparison, + superfluous-parens, + unidiomatic-typecheck, + unnecessary-lambda, + unnecessary-pass, + unnecessary-semicolon, + unneeded-not, + useless-else-on-loop, + wrong-assert-type, + + deprecated-method, + deprecated-module, + + too-many-boolean-expressions, + too-many-nested-blocks, + too-many-statements, + + wildcard-import, + wrong-import-order, + wrong-import-position, + + missing-final-newline, + mixed-line-endings, + trailing-newlines, + trailing-whitespace, + unexpected-line-ending-format, + + bad-inline-option, + bad-option-value, + deprecated-pragma, + unrecognized-inline-option, + useless-suppression, + + pii-invalid-no-pii-annotation, +disable = + bad-indentation, + broad-exception-raised, + consider-using-f-string, + duplicate-code, + file-ignored, + fixme, + global-statement, + invalid-name, + locally-disabled, + no-else-return, + suppressed-message, + too-few-public-methods, + too-many-ancestors, + too-many-arguments, + too-many-branches, + too-many-instance-attributes, + too-many-lines, + too-many-locals, + too-many-public-methods, + too-many-return-statements, + ungrouped-imports, + unspecified-encoding, + unused-wildcard-import, + use-maxsplit-arg, + + feature-toggle-needs-doc, + illegal-waffle-usage, + + logging-fstring-interpolation, + unused-import, + useless-import-alias, + line-too-long, + +[REPORTS] +output-format = text +reports = no +score = no + +[BASIC] +module-rgx = (([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$ +const-rgx = (([A-Z_][A-Z0-9_]*)|(__.*__)|log|urlpatterns)$ +class-rgx = [A-Z_][a-zA-Z0-9]+$ +function-rgx = ([a-z_][a-z0-9_]{2,40}|test_[a-z0-9_]+)$ +method-rgx = ([a-z_][a-z0-9_]{2,40}|setUp|set[Uu]pClass|tearDown|tear[Dd]ownClass|assert[A-Z]\w*|maxDiff|test_[a-z0-9_]+)$ +attr-rgx = [a-z_][a-z0-9_]{2,30}$ +argument-rgx = [a-z_][a-z0-9_]{2,30}$ +variable-rgx = [a-z_][a-z0-9_]{2,30}$ +class-attribute-rgx = ([A-Za-z_][A-Za-z0-9_]{2,30}|(__.*__))$ +inlinevar-rgx = [A-Za-z_][A-Za-z0-9_]*$ +good-names = f,i,j,k,db,ex,Run,_,__ +bad-names = foo,bar,baz,toto,tutu,tata +no-docstring-rgx = __.*__$|test_.+|setUp$|setUpClass$|tearDown$|tearDownClass$|Meta$ +docstring-min-length = 5 + +[FORMAT] +max-line-length = 120 +ignore-long-lines = ^\s*(# )?((?)|(\.\. \w+: .*))$ +single-line-if-stmt = no +max-module-lines = 1000 +indent-string = ' ' + +[MISCELLANEOUS] +notes = FIXME,XXX,TODO + +[SIMILARITIES] +min-similarity-lines = 4 +ignore-comments = yes +ignore-docstrings = yes +ignore-imports = no + +[TYPECHECK] +ignore-mixin-members = yes +ignored-classes = SQLObject +unsafe-load-any-extension = yes +generated-members = + REQUEST, + acl_users, + aq_parent, + objects, + DoesNotExist, + can_read, + can_write, + get_url, + size, + content, + status_code, + create, + build, + fields, + tag, + org, + course, + category, + name, + revision, + _meta, + +[VARIABLES] +init-import = no +dummy-variables-rgx = _|dummy|unused|.*_unused +additional-builtins = + +[CLASSES] +defining-attr-methods = __init__,__new__,setUp +valid-classmethod-first-arg = cls +valid-metaclass-classmethod-first-arg = mcs + +[DESIGN] +max-args = 5 +ignored-argument-names = _.* +max-locals = 15 +max-returns = 6 +max-branches = 12 +max-statements = 50 +max-parents = 7 +max-attributes = 7 +min-public-methods = 2 +max-public-methods = 20 + +[IMPORTS] +deprecated-modules = regsub,TERMIOS,Bastion,rexec +import-graph = +ext-import-graph = +int-import-graph = + +[EXCEPTIONS] +overgeneral-exceptions = builtins.Exception + +[PII] +pii-terms = + email, + username + +# 3fb04bcbcc6a1c289a937aae0a663bb118dedac6 diff --git a/pylintrc_tweaks b/pylintrc_tweaks new file mode 100644 index 00000000..8273f457 --- /dev/null +++ b/pylintrc_tweaks @@ -0,0 +1,13 @@ +# pylintrc tweaks for use with edx_lint. +[MASTER] +ignore = migrations +load-plugins = edx_lint.pylint + +[MESSAGES CONTROL] +# unused-import: ruff owns F401; it can auto-fix, pylint cannot. +# useless-import-alias: `import X as X` is the explicit re-export idiom (PEP 484). +# line-too-long: ruff owns E501; ruff format --fix can auto-fix, pylint cannot. +disable+ = + unused-import, + useless-import-alias, + line-too-long, diff --git a/pyproject.toml b/pyproject.toml index ecd35c0e..069529b2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -101,6 +101,7 @@ django42 = [ quality = [ { include-group = "test" }, "ruff", + "edx-lint", ] doc = [ { include-group = "test" }, @@ -161,9 +162,7 @@ select = [ "UP", # pyupgrade "DJ", # flake8-django ] -ignore = [ - "E501", # line too long (handled by formatter) -] +ignore = [] [tool.ruff.lint.isort] known-third-party = ["django", "xblock"] @@ -225,9 +224,6 @@ conflicts = [ constraint-dependencies = [ "Django<6.0", "elasticsearch<7.14.0", - "social-auth-app-django<6.0.0", - "social-auth-core<5.0.0", - "pip<26.2.1", "setuptools<=80.9.0", "pytest-django<4.13.0", ] diff --git a/tox.ini b/tox.ini index 0b945438..45cee062 100644 --- a/tox.ini +++ b/tox.ini @@ -50,4 +50,5 @@ dependency_groups = quality commands = ruff check . ruff format --check . + pylint xblocks_contrib xblock_pdf make selfcheck diff --git a/xblocks_contrib/annotatable/annotatable.py b/xblocks_contrib/annotatable/annotatable.py index d7db7610..d1257f9c 100644 --- a/xblocks_contrib/annotatable/annotatable.py +++ b/xblocks_contrib/annotatable/annotatable.py @@ -302,5 +302,6 @@ def definition_to_xml(self, resource_fs): # re-raise lines = self.data.split("\n") line, offset = err.position # lint-amnesty, pylint: disable=unpacking-non-sequence - msg = f"Unable to create xml for block {self.usage_key}. Context: '{lines[line - 1][offset - 40 : offset + 40]}'" + context = lines[line - 1][offset - 40 : offset + 40] + msg = f"Unable to create xml for block {self.usage_key}. Context: '{context}'" raise SerializationError(self.usage_key, msg) from err diff --git a/xblocks_contrib/legacy_utils/xml_utils.py b/xblocks_contrib/legacy_utils/xml_utils.py index 9d23ad7e..ab6afb39 100644 --- a/xblocks_contrib/legacy_utils/xml_utils.py +++ b/xblocks_contrib/legacy_utils/xml_utils.py @@ -154,7 +154,7 @@ def own_metadata(block: XBlock) -> dict[str, Any]: result[field.name] = field.read_json(block) except TypeError as exception: exception_message = f"{str(exception)}, Block-location:{str(block.usage_key)}, Field-name:{field.name}" - raise TypeError(exception_message) from None # lint-amnesty, pylint: disable=raise-missing-from + raise TypeError(exception_message) from None return result diff --git a/xblocks_contrib/lti/lti.py b/xblocks_contrib/lti/lti.py index 968956b7..e32c1b78 100644 --- a/xblocks_contrib/lti/lti.py +++ b/xblocks_contrib/lti/lti.py @@ -443,7 +443,7 @@ def get_input_fields(self): # lint-amnesty, pylint: disable=missing-function-do msg = _('Could not parse custom parameter: {custom_parameter}. Should be "x=y" string.').format( custom_parameter=f"{custom_parameter!r}" ) - raise LTIError(msg) from None # lint-amnesty, pylint: disable=raise-missing-from + raise LTIError(msg) from None # LTI specs: 'custom_' should be prepended before each custom parameter, as pointed in link above. if param_name not in PARAMETERS: @@ -989,7 +989,7 @@ def get_client_key_secret(self): msg = _('Could not parse LTI passport: {lti_passport}. Should be "id:key:secret" string.').format( lti_passport=f"{lti_passport!r}" ) - raise LTIError(msg) from None # lint-amnesty, pylint: disable=raise-missing-from + raise LTIError(msg) from None if lti_id == self.lti_id.strip(): return key, secret diff --git a/xblocks_contrib/lti/lti_2_util.py b/xblocks_contrib/lti/lti_2_util.py index b1e912dd..0861d486 100644 --- a/xblocks_contrib/lti/lti_2_util.py +++ b/xblocks_contrib/lti/lti_2_util.py @@ -313,7 +313,7 @@ def verify_lti_2_0_result_rest_headers(self, request, verify_content_type=True): self.verify_oauth_body_sign(request, content_type=LTI_2_0_JSON_CONTENT_TYPE) except (ValueError, LTIError) as err: log.info(f"[LTI]: v2.0 result service -- OAuth body verification failed: {str(err)}") - raise LTIError(str(err)) from None # lint-amnesty, pylint: disable=raise-missing-from + raise LTIError(str(err)) from None def parse_lti_2_0_result_json(self, json_str): """ @@ -340,7 +340,7 @@ def parse_lti_2_0_result_json(self, json_str): except (ValueError, TypeError): msg = f"Supplied JSON string in request body could not be decoded: {json_str}" log.info(f"[LTI] {msg}") - raise LTIError(msg) from None # lint-amnesty, pylint: disable=raise-missing-from + raise LTIError(msg) from None # the standard supports a list of objects, who knows why. It must contain at least 1 element, and the # first element must be a dict @@ -383,6 +383,6 @@ def parse_lti_2_0_result_json(self, json_str): except (TypeError, ValueError) as err: msg = f"Could not convert resultScore to float: {str(err)}" log.info(f"[LTI] {msg}") - raise LTIError(msg) from None # lint-amnesty, pylint: disable=raise-missing-from + raise LTIError(msg) from None return score, json_obj.get("comment", "") diff --git a/xblocks_contrib/lti/tests/helpers.py b/xblocks_contrib/lti/tests/helpers.py index d6e0a396..5d1bc792 100644 --- a/xblocks_contrib/lti/tests/helpers.py +++ b/xblocks_contrib/lti/tests/helpers.py @@ -76,8 +76,8 @@ class StubUserService(UserService): Stub UserService for testing the sequence block. """ - def __init__( - self, # pylint: disable=too-many-positional-arguments + def __init__( # pylint: disable=too-many-positional-arguments + self, user=None, user_is_staff=False, user_role=None, @@ -129,7 +129,7 @@ def __init__(self, anonymous_student_id, services=None): super().__init__(id_reader=lambda: None, id_generator=lambda: None, services=services) self.anonymous_student_id = anonymous_student_id - def handler_url(self, block, handler_name, suffix="", query="", thirdparty=False): # pylint: disable=too-many-positional-arguments + def handler_url(self, block, handler_name, suffix="", query="", thirdparty=False): return f"/mock_url/{handler_name}" def local_resource_url(self, block, resource): # pylint: disable=arguments-renamed diff --git a/xblocks_contrib/problem/capa/inputtypes.py b/xblocks_contrib/problem/capa/inputtypes.py index c87017d0..5ce6b3a7 100644 --- a/xblocks_contrib/problem/capa/inputtypes.py +++ b/xblocks_contrib/problem/capa/inputtypes.py @@ -128,9 +128,7 @@ class Attribute: # want to allow default to be None, but also allow required objects _sentinel = object() - def __init__( # pylint: disable=too-many-positional-arguments,too-many-arguments - self, name, default=_sentinel, transform=None, validate=None, render=True - ): + def __init__(self, name, default=_sentinel, transform=None, validate=None, render=True): """ Define an attribute diff --git a/xblocks_contrib/problem/capa/tests/test_responsetypes.py b/xblocks_contrib/problem/capa/tests/test_responsetypes.py index 5f0ee1cb..58a4b287 100644 --- a/xblocks_contrib/problem/capa/tests/test_responsetypes.py +++ b/xblocks_contrib/problem/capa/tests/test_responsetypes.py @@ -378,9 +378,7 @@ def test_multiple_inputs_exception(self): with pytest.raises(Exception): # noqa: B017 self.build_problem(math_display=True, expect="2*x+3*y", num_inputs=3) - def _assert_symbolic_grade( # pylint: disable=too-many-arguments,too-many-positional-arguments - self, problem, student_input, dynamath_input, expected_correctness, snuggletex_resp="" - ): + def _assert_symbolic_grade(self, problem, student_input, dynamath_input, expected_correctness, snuggletex_resp=""): """ Assert that the symbolic response has a certain grade. diff --git a/xblocks_contrib/problem/capa/xqueue_interface.py b/xblocks_contrib/problem/capa/xqueue_interface.py index c4d5ca71..e13ba1e4 100644 --- a/xblocks_contrib/problem/capa/xqueue_interface.py +++ b/xblocks_contrib/problem/capa/xqueue_interface.py @@ -72,7 +72,7 @@ def parse_xreply(xreply): class XQueueInterface: """Initializes the XQueue interface.""" - def __init__( # pylint: disable=too-many-positional-arguments + def __init__( self, url: str, django_auth: dict[str, str], diff --git a/xblocks_contrib/problem/tests/__init__.py b/xblocks_contrib/problem/tests/__init__.py index a2bbe977..b624cba5 100644 --- a/xblocks_contrib/problem/tests/__init__.py +++ b/xblocks_contrib/problem/tests/__init__.py @@ -168,7 +168,7 @@ def resource_url(self, *args, **kwargs): """Satisfy abstract method.""" return "" - def handler_url( # pylint: disable=arguments-differ,too-many-positional-arguments,unused-argument + def handler_url( # pylint: disable=arguments-differ,unused-argument self, block, handler_name, suffix="", query="", thirdparty=False ): """Mock handler URL generation to look like edx-platform URLs.""" diff --git a/xblocks_contrib/video/validation.py b/xblocks_contrib/video/validation.py index 11379164..6d26c1ff 100644 --- a/xblocks_contrib/video/validation.py +++ b/xblocks_contrib/video/validation.py @@ -16,9 +16,7 @@ class StudioValidationMessage(ValidationMessage): TYPES = [ValidationMessage.WARNING, ValidationMessage.ERROR, NOT_CONFIGURED] - def __init__( # pylint: disable=too-many-positional-arguments - self, message_type, message_text, action_label=None, action_class=None, action_runtime_event=None - ): + def __init__(self, message_type, message_text, action_label=None, action_class=None, action_runtime_event=None): """ Create a new message.