diff --git a/pylint_nautobot/__init__.py b/pylint_nautobot/__init__.py index 6899162..77ecb99 100644 --- a/pylint_nautobot/__init__.py +++ b/pylint_nautobot/__init__.py @@ -10,6 +10,7 @@ from .model_label import NautobotModelLabelChecker from .replaced_models import NautobotReplacedModelsImportChecker from .string_field_blank_null import NautobotStringFieldBlankNull +from .sub_class_name import NautobotSubClassNameChecker from .use_fields_all import NautobotUseFieldsAllChecker from .utils import is_version_compatible @@ -22,6 +23,7 @@ NautobotModelLabelChecker, NautobotReplacedModelsImportChecker, NautobotStringFieldBlankNull, + NautobotSubClassNameChecker, NautobotUseFieldsAllChecker, ] diff --git a/pylint_nautobot/sub_class_name.py b/pylint_nautobot/sub_class_name.py new file mode 100644 index 0000000..68c3ffc --- /dev/null +++ b/pylint_nautobot/sub_class_name.py @@ -0,0 +1,92 @@ +"""Check for imports whose paths have changed in 2.0.""" + +from typing import NamedTuple + +from astroid import ClassDef +from pylint.checkers import BaseChecker + +from .utils import find_ancestor +from .utils import get_model_name +from .utils import is_abstract_class +from .utils import is_version_compatible +from .utils import trim_first_pascal_word + +_ANCESTORS = ( + { + "versions": ">=2", + "ancestor": "nautobot.extras.filters.NautobotFilterSet", + }, + { + "versions": "<1", # Disabled, unable to find a model inside the class + "ancestor": "nautobot.extras.forms.base.BulkEditForm", + }, + { + "versions": ">=2", + "ancestor": "nautobot.extras.forms.base.NautobotFilterForm", + }, + { + "versions": ">=2", + "ancestor": "nautobot.extras.forms.base.NautobotModelForm", + "suffix": "Form", + }, + { + "versions": ">=2", + "ancestor": "nautobot.core.api.serializers.NautobotModelSerializer", + "suffix": "Serializer", + }, + { + "versions": ">=2", + "ancestor": "nautobot.core.views.viewsets.NautobotUIViewSet", + }, + { + "versions": ">=2", + "ancestor": "nautobot.core.tables.BaseTable", + }, +) + + +class _Ancestor(NamedTuple): + ancestor: str + suffix: str + + +def _get_ancestor(item: dict) -> _Ancestor: + ancestor = item["ancestor"] + return _Ancestor(ancestor, item.get("suffix", trim_first_pascal_word(ancestor.split(".")[-1]))) + + +class NautobotSubClassNameChecker(BaseChecker): + """Ensure subclass name is . + + This can typically be done via .replace("Nautobot", ) + """ + + version_specifier = ">=2,<3" + + name = "nautobot-sub-class-name" + msgs = { + "E4281": ( + "Sub-class name should be %s.", + "nb-sub-class-name", + "All classes should have a sub-class name that is .", + ) + } + + def __init__(self, *args, **kwargs): + """Initialize the checker.""" + super().__init__(*args, **kwargs) + + self.ancestors = tuple(_get_ancestor(item) for item in _ANCESTORS if is_version_compatible(item["versions"])) + + def visit_classdef(self, node: ClassDef): + """Visit class definitions.""" + if is_abstract_class(node): + return + + ancestor = find_ancestor(node, self.ancestors, lambda item: item.ancestor) + if not ancestor: + return + + expected_name = get_model_name(node) + ancestor.suffix + if expected_name != node.name: + self.add_message("nb-sub-class-name", node=node, args=(expected_name,)) diff --git a/pylint_nautobot/utils.py b/pylint_nautobot/utils.py index 2a4f911..be5e816 100644 --- a/pylint_nautobot/utils.py +++ b/pylint_nautobot/utils.py @@ -2,21 +2,27 @@ from importlib import metadata from pathlib import Path -from typing import List +from typing import Callable +from typing import Iterable from typing import Optional +from typing import TypeVar from typing import Union import toml from astroid import Assign from astroid import Attribute +from astroid import Call from astroid import ClassDef from astroid import Const from astroid import Name +from astroid import NodeNG from importlib_resources import files from packaging.specifiers import SpecifierSet from packaging.version import Version from yaml import safe_load +T = TypeVar("T") + def _read_poetry_lock() -> dict: for directory in (Path.cwd(), *Path.cwd().parents): @@ -42,6 +48,27 @@ def _read_locked_nautobot_version() -> Optional[str]: MINIMUM_NAUTOBOT_VERSION = Version(_read_locked_nautobot_version() or metadata.version("nautobot")) +def trim_first_pascal_word(pascal_case_string: str) -> str: + """Remove the first word from a pascal case string. + + Examples: + >>> trim_first_pascal_word("NautobotFilterSet") + 'FilterSet' + >>> trim_first_pascal_word("BaseTable") + 'Table' + >>> trim_first_pascal_word("FQDNModel") + 'Model' + """ + start_index = 0 + + for i in range(1, len(pascal_case_string)): + if pascal_case_string[i].isupper(): + start_index = i + break + + return pascal_case_string[start_index:] + + def is_abstract_class(node: ClassDef) -> bool: """Given a node, returns whether it is an abstract base model.""" for child_node in node.get_children(): @@ -63,32 +90,63 @@ def is_abstract_class(node: ClassDef) -> bool: return False -def get_model_name(ancestor: str, node: ClassDef) -> str: +def find_attr(node: ClassDef, attr_name: str) -> Optional[Assign]: + """Get the attribute from the class definition.""" + for attr in node.body: + if isinstance(attr, Assign): + for target in attr.targets: + if ( + isinstance(target, (Name, Attribute)) + and getattr(target, "attrname", None) == attr_name + or getattr(target, "name", None) == attr_name + ): + return attr + return None + + +def find_meta(node: ClassDef) -> Optional[ClassDef]: + """Find the Meta class from the class definition.""" + for child in node.body: + if isinstance(child, ClassDef) and child.name == "Meta": + return child + return None + + +def get_model_name(node: ClassDef) -> str: """Get the model name from the class definition.""" - if ancestor == "from nautobot.apps.views.NautobotUIViewSet": - raise NotImplementedError("This ancestor is not yet supported.") - - meta = next((n for n in node.body if isinstance(n, ClassDef) and n.name == "Meta"), None) - if not meta: - raise NotImplementedError("This class does not have a Meta class.") - - model_attr = next( - ( - attr - for attr in meta.body - if isinstance(attr, Assign) - and any( - isinstance(target, (Name, Attribute)) - and getattr(target, "attrname", None) == "model" - or getattr(target, "name", None) == "model" - for target in attr.targets - ) - ), - None, - ) + queryset = find_attr(node, "queryset") + if queryset: + return find_model_name_from_queryset(queryset.value) + + model_attr = find_attr(node, "model") if not model_attr: - raise NotImplementedError("The Meta class does not define a model attribute.") + meta = find_meta(node) + if not meta: + raise NotImplementedError("This class does not have a Meta class.") + model_attr = find_attr(meta, "model") + if not model_attr: + raise NotImplementedError("The Meta class does not define a model attribute.") + + return get_model_name_from_attr(model_attr) + +def find_model_name_from_queryset(node: NodeNG) -> str: + """Get the model name from the queryset assignment value.""" + while node: + if isinstance(node, Call): + node = node.func + elif isinstance(node, Attribute): + if node.attrname == "objects" and isinstance(node.expr, Name): + return node.expr.name + node = node.expr + else: + break + + raise NotImplementedError("Model was not found") + + +def get_model_name_from_attr(model_attr: Assign) -> str: + """Get the model name from the model attribute.""" if isinstance(model_attr.value, Name): return model_attr.value.name if not isinstance(model_attr.value, Attribute): @@ -105,14 +163,14 @@ def get_model_name(ancestor: str, node: ClassDef) -> str: return model_attr_chain[-1] -def find_ancestor(node: ClassDef, ancestors: List[str]) -> str: +def find_ancestor(node: ClassDef, ancestors: Iterable[T], get_value: Callable[[T], str]) -> Optional[T]: """Find the class ancestor from the list of ancestors.""" ancestor_class_types = [ancestor.qname() for ancestor in node.ancestors()] for checked_ancestor in ancestors: - if checked_ancestor in ancestor_class_types: + if get_value(checked_ancestor) in ancestor_class_types: return checked_ancestor - return "" + return None def is_version_compatible(specifier_set: Union[str, SpecifierSet]) -> bool: diff --git a/tests/inputs/sub-class-name/error_filter_form.py b/tests/inputs/sub-class-name/error_filter_form.py new file mode 100644 index 0000000..6038a51 --- /dev/null +++ b/tests/inputs/sub-class-name/error_filter_form.py @@ -0,0 +1,12 @@ +from nautobot.core.models.generics import PrimaryModel +from nautobot.extras.forms import NautobotFilterForm + + +class AddressObject(PrimaryModel): + pass + + +class MyAddressObjectFilterForm(NautobotFilterForm): + """Filter for AddressObject.""" + + model = AddressObject diff --git a/tests/inputs/sub-class-name/error_filter_set.py b/tests/inputs/sub-class-name/error_filter_set.py new file mode 100644 index 0000000..d1fd241 --- /dev/null +++ b/tests/inputs/sub-class-name/error_filter_set.py @@ -0,0 +1,15 @@ +from nautobot.apps.filters import NautobotFilterSet +from nautobot.core.models.generics import PrimaryModel + + +class AddressObject(PrimaryModel): + pass + + +class MyAddressObjectFilterSet(NautobotFilterSet): + """Filter for AddressObject.""" + + class Meta: + """Meta attributes for filter.""" + + model = AddressObject diff --git a/tests/inputs/sub-class-name/error_model_form.py b/tests/inputs/sub-class-name/error_model_form.py new file mode 100644 index 0000000..4d3fe33 --- /dev/null +++ b/tests/inputs/sub-class-name/error_model_form.py @@ -0,0 +1,15 @@ +from nautobot.core.models.generics import PrimaryModel +from nautobot.extras.forms import NautobotModelForm + + +class AddressObject(PrimaryModel): + pass + + +class MyAddressObjectForm(NautobotModelForm): + """Filter for AddressObject.""" + + class Meta: + """Meta attributes.""" + + model = AddressObject diff --git a/tests/inputs/sub-class-name/error_serializer.py b/tests/inputs/sub-class-name/error_serializer.py new file mode 100644 index 0000000..5acf5f0 --- /dev/null +++ b/tests/inputs/sub-class-name/error_serializer.py @@ -0,0 +1,15 @@ +from nautobot.apps.api import NautobotModelSerializer +from nautobot.core.models.generics import PrimaryModel + + +class AddressObject(PrimaryModel): + pass + + +class MyAddressObjectSerializer(NautobotModelSerializer): + """Serializer for AddressObject.""" + + class Meta: + """Meta attributes for filter.""" + + model = AddressObject diff --git a/tests/inputs/sub-class-name/error_table.py b/tests/inputs/sub-class-name/error_table.py new file mode 100644 index 0000000..3590b9e --- /dev/null +++ b/tests/inputs/sub-class-name/error_table.py @@ -0,0 +1,15 @@ +from nautobot.apps.tables import BaseTable +from nautobot.core.models.generics import PrimaryModel + + +class AddressObject(PrimaryModel): + pass + + +class MyAddressObjectTable(BaseTable): + """Filter for AddressObject.""" + + class Meta: + """Meta attributes for filter.""" + + model = AddressObject diff --git a/tests/inputs/sub-class-name/error_viewset.py b/tests/inputs/sub-class-name/error_viewset.py new file mode 100644 index 0000000..8035c84 --- /dev/null +++ b/tests/inputs/sub-class-name/error_viewset.py @@ -0,0 +1,12 @@ +from nautobot.apps.views import NautobotUIViewSet +from nautobot.core.models.generics import PrimaryModel + + +class AddressObject(PrimaryModel): + pass + + +class MyAddressObjectUIViewSet(NautobotUIViewSet): + """Filter for AddressObject.""" + + queryset = AddressObject.objects.all() diff --git a/tests/inputs/sub-class-name/good_filter_form.py b/tests/inputs/sub-class-name/good_filter_form.py new file mode 100644 index 0000000..6d2ff22 --- /dev/null +++ b/tests/inputs/sub-class-name/good_filter_form.py @@ -0,0 +1,12 @@ +from nautobot.core.models.generics import PrimaryModel +from nautobot.extras.forms import NautobotFilterForm + + +class AddressObject(PrimaryModel): + pass + + +class AddressObjectFilterForm(NautobotFilterForm): + """Filter for AddressObject.""" + + model = AddressObject diff --git a/tests/inputs/sub-class-name/good_filter_set.py b/tests/inputs/sub-class-name/good_filter_set.py new file mode 100644 index 0000000..0545356 --- /dev/null +++ b/tests/inputs/sub-class-name/good_filter_set.py @@ -0,0 +1,15 @@ +from nautobot.apps.filters import NautobotFilterSet +from nautobot.core.models.generics import PrimaryModel + + +class AddressObject(PrimaryModel): + pass + + +class AddressObjectFilterSet(NautobotFilterSet): + """Filter for AddressObject.""" + + class Meta: + """Meta attributes for filter.""" + + model = AddressObject diff --git a/tests/inputs/sub-class-name/good_model_form.py b/tests/inputs/sub-class-name/good_model_form.py new file mode 100644 index 0000000..77dcdb7 --- /dev/null +++ b/tests/inputs/sub-class-name/good_model_form.py @@ -0,0 +1,15 @@ +from nautobot.core.models.generics import PrimaryModel +from nautobot.extras.forms import NautobotModelForm + + +class AddressObject(PrimaryModel): + pass + + +class AddressObjectForm(NautobotModelForm): + """Filter for AddressObject.""" + + class Meta: + """Meta attributes.""" + + model = AddressObject diff --git a/tests/inputs/sub-class-name/good_serializer.py b/tests/inputs/sub-class-name/good_serializer.py new file mode 100644 index 0000000..0e6bf27 --- /dev/null +++ b/tests/inputs/sub-class-name/good_serializer.py @@ -0,0 +1,15 @@ +from nautobot.apps.api import NautobotModelSerializer +from nautobot.core.models.generics import PrimaryModel + + +class AddressObject(PrimaryModel): + pass + + +class AddressObjectSerializer(NautobotModelSerializer): + """Serializer for AddressObject.""" + + class Meta: + """Meta attributes for filter.""" + + model = AddressObject diff --git a/tests/inputs/sub-class-name/good_table.py b/tests/inputs/sub-class-name/good_table.py new file mode 100644 index 0000000..928b817 --- /dev/null +++ b/tests/inputs/sub-class-name/good_table.py @@ -0,0 +1,15 @@ +from nautobot.apps.tables import BaseTable +from nautobot.core.models.generics import PrimaryModel + + +class AddressObject(PrimaryModel): + pass + + +class AddressObjectTable(BaseTable): + """Filter for AddressObject.""" + + class Meta: + """Meta attributes for filter.""" + + model = AddressObject diff --git a/tests/inputs/sub-class-name/good_viewset.py b/tests/inputs/sub-class-name/good_viewset.py new file mode 100644 index 0000000..ea9f39f --- /dev/null +++ b/tests/inputs/sub-class-name/good_viewset.py @@ -0,0 +1,12 @@ +from nautobot.apps.views import NautobotUIViewSet +from nautobot.core.models.generics import PrimaryModel + + +class AddressObject(PrimaryModel): + pass + + +class AddressObjectUIViewSet(NautobotUIViewSet): + """Filter for AddressObject.""" + + queryset = AddressObject.objects.all() diff --git a/tests/test_sub_class_name.py b/tests/test_sub_class_name.py new file mode 100644 index 0000000..3de290b --- /dev/null +++ b/tests/test_sub_class_name.py @@ -0,0 +1,80 @@ +"""Tests for sub class name checker.""" + +from pylint.testutils import CheckerTestCase + +from pylint_nautobot.sub_class_name import NautobotSubClassNameChecker + +from .utils import assert_error_file +from .utils import assert_good_file +from .utils import parametrize_error_files +from .utils import parametrize_good_files + + +def _find_failing_node(module_node): + return module_node.body[3] + + +_EXPECTED_ERRORS = { + "filter_form": { + "versions": ">=2", + "msg_id": "nb-sub-class-name", + "line": 9, + "col_offset": 0, + "args": ("AddressObjectFilterForm",), + "node": _find_failing_node, + }, + "filter_set": { + "versions": ">=2", + "msg_id": "nb-sub-class-name", + "line": 9, + "col_offset": 0, + "args": ("AddressObjectFilterSet",), + "node": _find_failing_node, + }, + "model_form": { + "versions": ">=2", + "msg_id": "nb-sub-class-name", + "line": 9, + "col_offset": 0, + "args": ("AddressObjectForm",), + "node": _find_failing_node, + }, + "serializer": { + "versions": ">=2", + "msg_id": "nb-sub-class-name", + "line": 9, + "col_offset": 0, + "args": ("AddressObjectSerializer",), + "node": _find_failing_node, + }, + "viewset": { + "versions": ">=2", + "msg_id": "nb-sub-class-name", + "line": 9, + "col_offset": 0, + "args": ("AddressObjectUIViewSet",), + "node": _find_failing_node, + }, + "table": { + "versions": ">=2", + "msg_id": "nb-sub-class-name", + "line": 9, + "col_offset": 0, + "args": ("AddressObjectTable",), + "node": _find_failing_node, + }, +} + + +class TestSubClassNameChecker(CheckerTestCase): + """Test sub class name checker""" + + CHECKER_CLASS = NautobotSubClassNameChecker + + @parametrize_error_files(__file__, _EXPECTED_ERRORS) + def test_sub_class_name(self, filename, expected_error): + assert_error_file(self, filename, expected_error) + + @parametrize_good_files(__file__) + def test_no_issues(self, filename): + assert_good_file(self, filename)