feat: nb-use-fields-all

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 .use_fields_all import NautobotUseFieldsAllChecker
 from .utils import MINIMUM_NAUTOBOT_VERSION
 
 __version__ = metadata.version(__name__)
@@ -21,6 +22,7 @@
     NautobotModelLabelChecker,
     NautobotReplacedModelsImportChecker,
     NautobotStringFieldBlankNull,
+    NautobotUseFieldsAllChecker,
 ]
 
 

pylint_nautobot/use_fields_all.py

@@ -0,0 +1,49 @@
+"""Check for CharField's or TextField's on models where null=True and blank=True."""
+from astroid import Assign
+from astroid import AssignName
+from astroid import ClassDef
+from astroid import Const
+from pylint.checkers import BaseChecker
+
+from .utils import is_version_compatible
+
+_META_CLASSES = {
+    "nautobot.core.api.serializers.NautobotModelSerializer.Meta": ">1",
+    "nautobot.core.tables.BaseTable.Meta": ">=2",
+    "nautobot.extras.filters.NautobotFilterSet.Meta": ">1",  # Capability should be added, but does not exist today
+    "nautobot.extras.forms.NautobotModelForm.Meta": ">1",
+    "nautobot.utilities.tables.BaseTable.Meta": ">1,<2",
+    # NautobotBulkEditForm - Unclear, in floor app I see it defined, but others I do not, perhaps it is an implied all, in which case I think better to be explicit and convert to respect the all param as best practices)
+    # NautobotFilterForm - Not needed (can investigate other options for field_order)
+}
+
+_CHECK_CLASSES = [key for key, specifier_set in _META_CLASSES.items() if is_version_compatible(specifier_set)]
+
+
+class NautobotUseFieldsAllChecker(BaseChecker):
+    """Visit Meta subclasses and check for use of `fields = "__all__"`, instead of `fields = ["field1", ...]`."""
+
+    version_specifier = ">=1,<3"
+
+    name = "nautobot-use-fields-all"
+    msgs = {
+        "E4271": (
+            "Use `fields = '__all__'` instead of specifying each field individually.",
+            "nb-use-fields-all",
+            "Defining `fields = '__all__'` in a model serializer's Meta class is a Django convention that automatically "
+            "includes all fields from the associated model. This approach is more maintainable because it avoids having "
+            "to explicitly list each field, reducing the risk of errors and inconsistencies when the model is updated.",
+        ),
+    }
+
+    def visit_classdef(self, node: ClassDef):
+        """Visit class definitions."""
+        if not any(ancestor.qname() in _CHECK_CLASSES for ancestor in node.ancestors()):
+            return
+
+        for child_node in node.get_children():
+            if isinstance(child_node, Assign):
+                if any(isinstance(target, AssignName) and target.name == "fields" for target in child_node.targets):
+                    value = child_node.value
+                    if not (isinstance(value, Const) and value.value == "__all__"):
+                        self.add_message("nb-use-fields-all", node=value)

tests/inputs/use-fields-all/error_table.py

@@ -0,0 +1,14 @@
+from nautobot.apps.tables import BaseTable
+from nautobot.apps.tables import ToggleColumn
+from nautobot.extras.tables import StatusTableMixin
+
+
+class MyTable(StatusTableMixin, BaseTable):
+    """Table for list view."""
+
+    pk = ToggleColumn()
+
+    class Meta(BaseTable.Meta):
+        """Meta attributes."""
+
+        fields = ("pk", "my_field1", "my_field2")

tests/inputs/use-fields-all/good_table.py

@@ -0,0 +1,14 @@
+from nautobot.apps.tables import BaseTable
+from nautobot.apps.tables import ToggleColumn
+from nautobot.extras.tables import StatusTableMixin
+
+
+class MyTable(StatusTableMixin, BaseTable):
+    """Table for list view."""
+
+    pk = ToggleColumn()
+
+    class Meta(BaseTable.Meta):
+        """Meta attributes."""
+
+        fields = "__all__"

tests/test_use_fields_all.py

@@ -0,0 +1,42 @@
+"""Tests for use fields all"""
+from pathlib import Path
+
+from pylint.testutils import CheckerTestCase
+from pytest import mark
+
+from pylint_nautobot.use_fields_all import NautobotUseFieldsAllChecker
+
+from .utils import assert_error_file
+from .utils import assert_good_file
+
+
+def _find_fields_node(module_node):
+    """Find the fields node in the class definition."""
+    class_node = module_node.body[3]
+    meta = list(class_node.get_children())[3]
+    return list(meta.get_children())[1].value
+
+
+_INPUTS_PATH = Path(__file__).parent / "inputs/use-fields-all/"
+_EXPECTED_ERRORS = {
+    "table": {
+        "msg_id": "nb-use-fields-all",
+        "line": 14,
+        "col_offset": 17,
+        "node": _find_fields_node,
+    },
+}
+
+
+class TestUseFieldsAllChecker(CheckerTestCase):
+    """Test use fields all checker"""
+
+    CHECKER_CLASS = NautobotUseFieldsAllChecker
+
+    @mark.parametrize("path", _INPUTS_PATH.glob("error_*.py"))
+    def test_use_fields_all(self, path):
+        assert_error_file(self, path, _EXPECTED_ERRORS)
+
+    @mark.parametrize("path", _INPUTS_PATH.glob("good_*.py"))
+    def test_no_issues(self, path):
+        assert_good_file(self, path)