From ef2abbf6c5e83885a60beb03174e6a70ede18269 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Mon, 13 Jan 2025 09:45:46 +0100 Subject: [PATCH 1/6] feat: add linter for openedx-filters classes definitions --- edx_lint/pylint/filters_docstring/__init__.py | 10 ++ .../filters_docstring_check.py | 106 ++++++++++++++++++ 2 files changed, 116 insertions(+) create mode 100644 edx_lint/pylint/filters_docstring/__init__.py create mode 100644 edx_lint/pylint/filters_docstring/filters_docstring_check.py diff --git a/edx_lint/pylint/filters_docstring/__init__.py b/edx_lint/pylint/filters_docstring/__init__.py new file mode 100644 index 0000000..588ee6e --- /dev/null +++ b/edx_lint/pylint/filters_docstring/__init__.py @@ -0,0 +1,10 @@ +""" +edx_lint filters_docstring module (optional plugin for filters docstrings). + +Add this to your pylintrc:: + load-plugins=edx_lint.pylint.filters_docstring +""" + +from .filters_docstring_check import register_checkers + +register = register_checkers diff --git a/edx_lint/pylint/filters_docstring/filters_docstring_check.py b/edx_lint/pylint/filters_docstring/filters_docstring_check.py new file mode 100644 index 0000000..2c2dfad --- /dev/null +++ b/edx_lint/pylint/filters_docstring/filters_docstring_check.py @@ -0,0 +1,106 @@ +""" +Pylint checker for the format of the docstrings of filters. + +A filter's docstring should have the following structure: + +1. Description: Any non-empty text followed by a blank line. +2. Filter Type: A line that starts with "Filter Type:". +3. Trigger: A line that starts with "Trigger:". +""" + +from pylint.checkers import BaseChecker, utils +import re +from ..common import BASE_ID + + +def register_checkers(linter): + """ + Register checkers. + """ + linter.register_checker(FiltersDocstringFormatChecker(linter)) + + +class FiltersDocstringFormatChecker(BaseChecker): + """Pylint checker for the format of the docstrings of filters.""" + + name = "docstring-format-checker" + + FILTER_DOCSTRING_MISSING_DESCRIPTION = "filter-docstring-missing-description" + FILTER_DOCSTRING_MISSING_TYPE = "filter-docstring-missing-type" + FILTER_DOCSTRING_MISSING_TRIGGER = "filter-docstring-missing-trigger" + + msgs = { + ("E%d90" % BASE_ID): ( + "Filter's (%s) docstring is missing the required description section", + FILTER_DOCSTRING_MISSING_DESCRIPTION, + "filters docstring is missing the required description section", + ), + ("E%d91" % BASE_ID): ( + "Filter's (%s) docstring is missing the required filter type section", + FILTER_DOCSTRING_MISSING_TYPE, + "filters docstring is missing the required filter type section", + ), + ("E%d92" % BASE_ID): ( + "Filter's (%s) docstring is missing the required trigger section", + FILTER_DOCSTRING_MISSING_TRIGGER, + "filters docstring is missing the required trigger section", + ), + } + + options = () + + @utils.only_required_for_messages( + FILTER_DOCSTRING_MISSING_DESCRIPTION, + FILTER_DOCSTRING_MISSING_TYPE, + FILTER_DOCSTRING_MISSING_TRIGGER, + ) + def visit_classdef(self, node): + """Visit a class definition and check its docstring.""" + if not node.is_subtype_of("openedx_filters.tooling.OpenEdxPublicFilter"): + return + + docstring = node.doc_node.value if node.doc_node else "" + if not (error_messages := self._check_docstring_format(docstring)): + return + for error_message in error_messages: + self.add_message(error_message, node=node, args=(node.name,)) + + def _check_docstring_format(self, docstring): + """ + Check the format of the docstring for errors and return a list of error messages. + + The docstring should have the following structure: + 1. Description: Any non-empty text followed by a blank line. + 2. Filter Type: A line that starts with "Filter Type:". + 3. Trigger: A line that starts with "Trigger:". + + For example: + + ``` + Description: + Filter used to modify the certificate rendering process. + + ... (more description) + + Filter Type: + org.openedx.learning.certificate.render.started.v1 + + Trigger: + - Repository: openedx/edx-platform + - Path: lms/djangoapps/certificates/views/webview.py + - Function or Method: render_html_view + ``` + """ + required_sections = [ + (r"Description:\s*.*\n", self.FILTER_DOCSTRING_MISSING_DESCRIPTION), + (r"Filter Type:\s*.*\n", self.FILTER_DOCSTRING_MISSING_TYPE), + ( + r"Trigger:\s*(NA|-\s*Repository:\s*[^\n]+\s*-\s*Path:\s*[^\n]+\s*-\s*Function\s*or\s*Method:\s*[^\n]+)", + self.FILTER_DOCSTRING_MISSING_TRIGGER, + ), + ] + error_messages = [] + for pattern, error_message in required_sections: + if not re.search(pattern, docstring, re.MULTILINE): + error_messages.append(error_message) + return error_messages From d17b9345e8cfa53a362ad506f2f3c0422c7a1f6d Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Mon, 13 Jan 2025 12:11:35 +0100 Subject: [PATCH 2/6] refactor: address quality errors --- .../pylint/filters_docstring/filters_docstring_check.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/edx_lint/pylint/filters_docstring/filters_docstring_check.py b/edx_lint/pylint/filters_docstring/filters_docstring_check.py index 2c2dfad..20c283a 100644 --- a/edx_lint/pylint/filters_docstring/filters_docstring_check.py +++ b/edx_lint/pylint/filters_docstring/filters_docstring_check.py @@ -8,9 +8,11 @@ 3. Trigger: A line that starts with "Trigger:". """ -from pylint.checkers import BaseChecker, utils import re -from ..common import BASE_ID + +from pylint.checkers import BaseChecker, utils + +from edx_lint.pylint.common import BASE_ID def register_checkers(linter): From 9d4fc47aea6ce353c4432a24fdf055c0b841620a Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Mon, 13 Jan 2025 12:22:56 +0100 Subject: [PATCH 3/6] refactor: drop filter prefix from variables names --- .../filters_docstring_check.py | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/edx_lint/pylint/filters_docstring/filters_docstring_check.py b/edx_lint/pylint/filters_docstring/filters_docstring_check.py index 20c283a..9aaf50c 100644 --- a/edx_lint/pylint/filters_docstring/filters_docstring_check.py +++ b/edx_lint/pylint/filters_docstring/filters_docstring_check.py @@ -27,24 +27,24 @@ class FiltersDocstringFormatChecker(BaseChecker): name = "docstring-format-checker" - FILTER_DOCSTRING_MISSING_DESCRIPTION = "filter-docstring-missing-description" - FILTER_DOCSTRING_MISSING_TYPE = "filter-docstring-missing-type" - FILTER_DOCSTRING_MISSING_TRIGGER = "filter-docstring-missing-trigger" + DOCSTRING_MISSING_DESCRIPTION = "filter-docstring-missing-description" + DOCSTRING_MISSING_TYPE = "filter-docstring-missing-type" + DOCSTRING_MISSING_TRIGGER = "filter-docstring-missing-trigger" msgs = { ("E%d90" % BASE_ID): ( "Filter's (%s) docstring is missing the required description section", - FILTER_DOCSTRING_MISSING_DESCRIPTION, + DOCSTRING_MISSING_DESCRIPTION, "filters docstring is missing the required description section", ), ("E%d91" % BASE_ID): ( "Filter's (%s) docstring is missing the required filter type section", - FILTER_DOCSTRING_MISSING_TYPE, + DOCSTRING_MISSING_TYPE, "filters docstring is missing the required filter type section", ), ("E%d92" % BASE_ID): ( "Filter's (%s) docstring is missing the required trigger section", - FILTER_DOCSTRING_MISSING_TRIGGER, + DOCSTRING_MISSING_TRIGGER, "filters docstring is missing the required trigger section", ), } @@ -52,9 +52,9 @@ class FiltersDocstringFormatChecker(BaseChecker): options = () @utils.only_required_for_messages( - FILTER_DOCSTRING_MISSING_DESCRIPTION, - FILTER_DOCSTRING_MISSING_TYPE, - FILTER_DOCSTRING_MISSING_TRIGGER, + DOCSTRING_MISSING_DESCRIPTION, + DOCSTRING_MISSING_TYPE, + DOCSTRING_MISSING_TRIGGER, ) def visit_classdef(self, node): """Visit a class definition and check its docstring.""" @@ -94,11 +94,11 @@ def _check_docstring_format(self, docstring): ``` """ required_sections = [ - (r"Description:\s*.*\n", self.FILTER_DOCSTRING_MISSING_DESCRIPTION), - (r"Filter Type:\s*.*\n", self.FILTER_DOCSTRING_MISSING_TYPE), + (r"Description:\s*.*\n", self.DOCSTRING_MISSING_DESCRIPTION), + (r"Filter Type:\s*.*\n", self.DOCSTRING_MISSING_TYPE), ( r"Trigger:\s*(NA|-\s*Repository:\s*[^\n]+\s*-\s*Path:\s*[^\n]+\s*-\s*Function\s*or\s*Method:\s*[^\n]+)", - self.FILTER_DOCSTRING_MISSING_TRIGGER, + self.DOCSTRING_MISSING_TRIGGER, ), ] error_messages = [] From bca02a2d1f85d7f3ed4f10ff1686e60786379547 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Mon, 13 Jan 2025 12:27:14 +0100 Subject: [PATCH 4/6] refactor: add additional details for error message --- .../filters_docstring/filters_docstring_check.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/edx_lint/pylint/filters_docstring/filters_docstring_check.py b/edx_lint/pylint/filters_docstring/filters_docstring_check.py index 9aaf50c..f8a16f4 100644 --- a/edx_lint/pylint/filters_docstring/filters_docstring_check.py +++ b/edx_lint/pylint/filters_docstring/filters_docstring_check.py @@ -33,19 +33,19 @@ class FiltersDocstringFormatChecker(BaseChecker): msgs = { ("E%d90" % BASE_ID): ( - "Filter's (%s) docstring is missing the required description section", + "Filter's (%s) docstring is missing the required description section or is badly formatted", DOCSTRING_MISSING_DESCRIPTION, - "filters docstring is missing the required description section", + "filters docstring is missing the required description section or is badly formatted", ), ("E%d91" % BASE_ID): ( - "Filter's (%s) docstring is missing the required filter type section", + "Filter's (%s) docstring is missing the required filter type section or is badly formatted", DOCSTRING_MISSING_TYPE, - "filters docstring is missing the required filter type section", + "filters docstring is missing the required filter type section or is badly formatted", ), ("E%d92" % BASE_ID): ( - "Filter's (%s) docstring is missing the required trigger section", + "Filter's (%s) docstring is missing the required trigger section or is badly formatted", DOCSTRING_MISSING_TRIGGER, - "filters docstring is missing the required trigger section", + "filters docstring is missing the required trigger section or is badly formatted", ), } From 3a43c7a3ae976ab5f826c52676551df5ab3b2603 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Mon, 13 Jan 2025 12:55:17 +0100 Subject: [PATCH 5/6] refactor: skip OpenEdxPublicFilter base class --- .../filters_docstring/filters_docstring_check.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/edx_lint/pylint/filters_docstring/filters_docstring_check.py b/edx_lint/pylint/filters_docstring/filters_docstring_check.py index f8a16f4..fdaf62f 100644 --- a/edx_lint/pylint/filters_docstring/filters_docstring_check.py +++ b/edx_lint/pylint/filters_docstring/filters_docstring_check.py @@ -57,10 +57,19 @@ class FiltersDocstringFormatChecker(BaseChecker): DOCSTRING_MISSING_TRIGGER, ) def visit_classdef(self, node): - """Visit a class definition and check its docstring.""" + """ + Visit a class definition and check its docstring. + + If the class is a subclass of OpenEdxPublicFilter, check the format of its docstring. Skip the + OpenEdxPublicFilter class itself. + + """ if not node.is_subtype_of("openedx_filters.tooling.OpenEdxPublicFilter"): return + if node.name == "OpenEdxPublicFilter": + return + docstring = node.doc_node.value if node.doc_node else "" if not (error_messages := self._check_docstring_format(docstring)): return From e86e635c3ed549a990fdde12724a5039608e8655 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Mon, 13 Jan 2025 13:35:03 +0100 Subject: [PATCH 6/6] refactor: use purpose only instead of description --- .../filters_docstring_check.py | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/edx_lint/pylint/filters_docstring/filters_docstring_check.py b/edx_lint/pylint/filters_docstring/filters_docstring_check.py index fdaf62f..4c3e192 100644 --- a/edx_lint/pylint/filters_docstring/filters_docstring_check.py +++ b/edx_lint/pylint/filters_docstring/filters_docstring_check.py @@ -27,22 +27,22 @@ class FiltersDocstringFormatChecker(BaseChecker): name = "docstring-format-checker" - DOCSTRING_MISSING_DESCRIPTION = "filter-docstring-missing-description" + DOCSTRING_MISSING_PURPOSE = "filter-docstring-missing-purpose" DOCSTRING_MISSING_TYPE = "filter-docstring-missing-type" DOCSTRING_MISSING_TRIGGER = "filter-docstring-missing-trigger" msgs = { - ("E%d90" % BASE_ID): ( - "Filter's (%s) docstring is missing the required description section or is badly formatted", - DOCSTRING_MISSING_DESCRIPTION, - "filters docstring is missing the required description section or is badly formatted", - ), ("E%d91" % BASE_ID): ( - "Filter's (%s) docstring is missing the required filter type section or is badly formatted", - DOCSTRING_MISSING_TYPE, - "filters docstring is missing the required filter type section or is badly formatted", + "Filter's (%s) docstring is missing the required purpose section or is badly formatted", + DOCSTRING_MISSING_PURPOSE, + "filters docstring is missing the required purpose section or is badly formatted", ), ("E%d92" % BASE_ID): ( + "Filter's (%s) docstring is missing the required type section or is badly formatted", + DOCSTRING_MISSING_TYPE, + "filters docstring is missing the required type section or is badly formatted", + ), + ("E%d93" % BASE_ID): ( "Filter's (%s) docstring is missing the required trigger section or is badly formatted", DOCSTRING_MISSING_TRIGGER, "filters docstring is missing the required trigger section or is badly formatted", @@ -52,7 +52,7 @@ class FiltersDocstringFormatChecker(BaseChecker): options = () @utils.only_required_for_messages( - DOCSTRING_MISSING_DESCRIPTION, + DOCSTRING_MISSING_PURPOSE, DOCSTRING_MISSING_TYPE, DOCSTRING_MISSING_TRIGGER, ) @@ -103,7 +103,7 @@ def _check_docstring_format(self, docstring): ``` """ required_sections = [ - (r"Description:\s*.*\n", self.DOCSTRING_MISSING_DESCRIPTION), + (r"Purpose:\s*.*\n", self.DOCSTRING_MISSING_PURPOSE), (r"Filter Type:\s*.*\n", self.DOCSTRING_MISSING_TYPE), ( r"Trigger:\s*(NA|-\s*Repository:\s*[^\n]+\s*-\s*Path:\s*[^\n]+\s*-\s*Function\s*or\s*Method:\s*[^\n]+)",