Skip to content

Commit

Permalink
Add the Search Filter Checker. (#107)
Browse files Browse the repository at this point in the history
  • Loading branch information
smk4664 authored Jan 10, 2025
1 parent 6a2c22e commit f69c28f
Show file tree
Hide file tree
Showing 9 changed files with 171 additions and 0 deletions.
14 changes: 14 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ jobs:
uses: "actions/checkout@v4"
- name: "Setup environment"
uses: "networktocode/gh-action-setup-poetry-environment@v6"
with:
poetry-version: "1.8.5"
- name: "Linting: ruff format"
run: "poetry run invoke ruff --action format"
ruff-lint:
Expand All @@ -36,6 +38,8 @@ jobs:
uses: "actions/checkout@v4"
- name: "Setup environment"
uses: "networktocode/gh-action-setup-poetry-environment@v6"
with:
poetry-version: "1.8.5"
- name: "Linting: ruff"
run: "poetry run invoke ruff"
check-docs-build:
Expand All @@ -47,6 +51,8 @@ jobs:
uses: "actions/checkout@v4"
- name: "Setup environment"
uses: "networktocode/gh-action-setup-poetry-environment@v6"
with:
poetry-version: "1.8.5"
- name: "Check Docs Build"
run: "poetry run invoke build-and-check-docs"
poetry:
Expand All @@ -58,6 +64,8 @@ jobs:
uses: "actions/checkout@v4"
- name: "Setup environment"
uses: "networktocode/gh-action-setup-poetry-environment@v6"
with:
poetry-version: "1.8.5"
- name: "Checking: poetry lock file"
run: "poetry run invoke lock --check"
yamllint:
Expand All @@ -69,6 +77,8 @@ jobs:
uses: "actions/checkout@v4"
- name: "Setup environment"
uses: "networktocode/gh-action-setup-poetry-environment@v6"
with:
poetry-version: "1.8.5"
- name: "Linting: yamllint"
run: "poetry run invoke yamllint"
pylint:
Expand All @@ -92,6 +102,8 @@ jobs:
uses: "actions/checkout@v4"
- name: "Setup environment"
uses: "networktocode/gh-action-setup-poetry-environment@v6"
with:
poetry-version: "1.8.5"
- name: "Set up Docker Buildx"
id: "buildx"
uses: "docker/setup-buildx-action@v3"
Expand Down Expand Up @@ -135,6 +147,8 @@ jobs:
uses: "actions/checkout@v4"
- name: "Setup environment"
uses: "networktocode/gh-action-setup-poetry-environment@v6"
with:
poetry-version: "1.8.5"
- name: "Set up Docker Buildx"
id: "buildx"
uses: "docker/setup-buildx-action@v3"
Expand Down
2 changes: 2 additions & 0 deletions pylint_nautobot/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from .deprecated_status_model import NautobotDeprecatedStatusModelChecker
from .incorrect_base_class import NautobotIncorrectBaseClassChecker
from .model_label import NautobotModelLabelChecker
from .q_search_filter import NautobotUseSearchFilterChecker
from .replaced_models import NautobotReplacedModelsImportChecker
from .string_field_blank_null import NautobotStringFieldBlankNull
from .sub_class_name import NautobotSubClassNameChecker
Expand All @@ -25,6 +26,7 @@
NautobotStringFieldBlankNull,
NautobotSubClassNameChecker,
NautobotUseFieldsAllChecker,
NautobotUseSearchFilterChecker,
]


Expand Down
52 changes: 52 additions & 0 deletions pylint_nautobot/q_search_filter.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
"""Check for use of SearchFilter on NautobotFilterSets instead of custom `q` search functions."""

from astroid import Assign, AssignName, ClassDef, FunctionDef
from pylint.checkers import BaseChecker


class NautobotUseSearchFilterChecker(BaseChecker):
"""Visit NautobotFilterSet subclasses and check for use of `q = SearchFilter`, instead of `q = django_filters.CharField`."""

version_specifier = ">=2,<3"

name = "nautobot-use-search-filter"
msgs = {
"C4272": (
"Use `q = SearchFilter` instead of django_filters.CharFilter and the custom `search` function.",
"nb-no-char-filter-q",
"Nautobot provides a `SearchFilter` class that uses MappedPredicates to provide a more flexible search experience. "
"This should be used in place of `django_filters.CharFilter` and no longer requires the custom `search` function.",
),
"C4273": (
"Use `q = SearchFilter` instead.",
"nb-use-search-filter",
"Nautobot provides a `SearchFilter` class that uses MappedPredicates to provide a more flexible search experience. "
"This can be disabled if you need to use a custom Filter.",
),
"C4274": (
"Don't use custom `search` function, use SearchFilter instead.",
"nb-no-search-function",
"Nautobot provides a `SearchFilter` class that uses MappedPredicates to provide a more flexible search experience. "
"This should be used in place of the custom `search` function.",
),
}

def visit_classdef(self, node: ClassDef):
"""Visit class definitions."""
for ancestor in node.ancestors():
if ancestor.qname() != "nautobot.extras.filters.NautobotFilterSet":
return

for child_node in node.get_children():
if isinstance(child_node, Assign) and any(
isinstance(target, AssignName) and target.name == "q" for target in child_node.targets
):
child_node_name = child_node.value.func.as_string().split(".")[-1]
# The `q` attribute should not be a CharFilter, instead it should be a SearchFilter
if child_node_name == "CharFilter":
self.add_message("nb-no-char-filter-q", node=child_node)
# Warn but allow override if the `q` attribute is not a SearchFilter as this could be inherited from the SearchFilter
elif child_node_name != "SearchFilter":
self.add_message("nb-use-search-filter", node=child_node)
if isinstance(child_node, FunctionDef) and child_node.name == "search":
self.add_message("nb-no-search-function", node=child_node)
11 changes: 11 additions & 0 deletions tests/inputs/q-search-filter/error_filter_set.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import django_filters
from nautobot.apps.filters import NautobotFilterSet


class IncorrectQFilterTypeFilterSet(NautobotFilterSet):
"""Filter for AddressObject."""

q = django_filters.CharFilter(
method="search",
label="Search",
)
16 changes: 16 additions & 0 deletions tests/inputs/q-search-filter/error_filter_set2.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
from nautobot.apps.filters import NautobotFilterSet, SearchFilter


class CustomSearchFilter(SearchFilter):
"""Custom search filter."""

filter_predicates = {
"name": ["name__icontains"],
"description": ["description__icontains"],
}


class MyAddressObjectFilterSet(NautobotFilterSet):
"""Filter for AddressObject."""

q = CustomSearchFilter()
10 changes: 10 additions & 0 deletions tests/inputs/q-search-filter/error_filter_set3.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
from django.db.models import Q
from nautobot.apps.filters import NautobotFilterSet


class MissingQFilterFilterSet(NautobotFilterSet):
"""Filter for AddressObject."""

def search(self, queryset, name, value):
"""Search for a string in multiple fields."""
return queryset.filter(Q(name__icontains=value) | Q(description__icontains=value))
12 changes: 12 additions & 0 deletions tests/inputs/q-search-filter/good_filter_set.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
from nautobot.apps.filters import NautobotFilterSet, SearchFilter


class ValidQFilterFilterSet(NautobotFilterSet):
"""Filter for AddressObject."""

q = SearchFilter(
filter_predicates={
"name": ["name__icontains"],
"description": ["description__icontains"],
}
)
12 changes: 12 additions & 0 deletions tests/inputs/q-search-filter/good_filter_set3.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
from nautobot.apps.filters import NautobotFilterSet, SearchFilter


class ValidQFilterWithCustomFilterPredicatesFilterSet(NautobotFilterSet):
"""Filter for AddressObject."""

q = SearchFilter(
filter_predicates={
"name": ["name__icontains"],
"description": ["description__icontains"],
}
)
42 changes: 42 additions & 0 deletions tests/test_q_search_filter.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
"""Tests for use of SearchFilter on NautobotFilterSets instead of custom `q` search functions.."""

from pylint.testutils import CheckerTestCase

from pylint_nautobot.q_search_filter import NautobotUseSearchFilterChecker

from .utils import assert_error_file, assert_good_file, parametrize_error_files, parametrize_good_files

_EXPECTED_ERRORS = {
"filter_set": {
"msg_id": "nb-no-char-filter-q",
"line": 8,
"col_offset": 4,
"node": lambda module_node: module_node.body[2].body[0],
},
"filter_set2": {
"msg_id": "nb-use-search-filter",
"line": 16,
"col_offset": 4,
"node": lambda module_node: module_node.body[2].body[0],
},
"filter_set3": {
"msg_id": "nb-no-search-function",
"line": 8,
"col_offset": 4,
"node": lambda module_node: module_node.body[2].body[0],
},
}


class TestNautobotUseSearchFilterChecker(CheckerTestCase):
"""Test blank and null StringField checker."""

CHECKER_CLASS = NautobotUseSearchFilterChecker

@parametrize_error_files(__file__, _EXPECTED_ERRORS)
def test_error(self, filename, expected_error):
assert_error_file(self, filename, expected_error)

@parametrize_good_files(__file__)
def test_good(self, filename):
assert_good_file(self, filename)

0 comments on commit f69c28f

Please sign in to comment.