Setup pylint, make misago.acl and misago.admin pass

.pylintrc

@@ -13,10 +13,12 @@ disable=
     bad-continuation,
     expression-not-assigned,
     fixme,
+    inconsistent-return-statements,
     invalid-name,
     missing-docstring,
     no-member,
     no-self-use,
+    redefined-outer-name,
     too-few-public-methods,
     too-many-ancestors,
     too-many-arguments,
@@ -24,6 +26,8 @@ disable=
     too-many-lines,  # FIXME
     too-many-public-methods,
     too-many-statements,  # FIXME
+    ungrouped-imports,
+    unsubscriptable-object,
     unused-argument
 
 

.travis.yml

@@ -20,7 +20,8 @@ jobs:
       python: 3.6
       install:
         - pip install -U pip setuptools
+        - python setup.py install
         - pip install black pylint pylint-django
       script:
         - black --check devproject misago
-        - pylint misago
+        - pylint misago/acl misago/admin

misago/acl/views.py

@@ -64,9 +64,7 @@ class EditRole(RoleFormMixin, RoleAdmin, generic.ModelFormView):
 
 
 class DeleteRole(RoleAdmin, generic.ButtonView):
-    def check_permissions(
-        self, request, target
-    ):  # pylint: disable=inconsistent-return-statements
+    def check_permissions(self, request, target):
         if target.special_role:
             message = _('Role "%(name)s" is special role and can\'t be deleted.')
             return message % {"name": target.name}

misago/admin/forms.py

@@ -32,8 +32,7 @@ class YesNoSwitchBase(TypedChoiceField):
         """normalize bools to binary 1/0 so field works on them too"""
         if value in (True, "True", "true", 1, "1"):
             return 1
-        else:
-            return 0
+        return 0
 
     def clean(self, value):
         return self.prepare_value(value)

misago/admin/hierarchy.py

@@ -9,15 +9,15 @@ class Node:
         self.link = link
         self._children = []
         self._children_dict = {}
+        self._resolved_namespace = None
 
     @property
     def namespace(self):
-        try:
+        if self._resolved_namespace:
             return self._resolved_namespace
-        except AttributeError:
-            bits = self.link.split(":")
-            self._resolved_namespace = ":".join(bits[:-1])
 
+        bits = self.link.split(":")
+        self._resolved_namespace = ":".join(bits[:-1])
         return self._resolved_namespace
 
     def children(self):
@@ -39,13 +39,12 @@ class Node:
     def add_node(self, node, after=None, before=None):
         if after:
             return self.add_node_after(node, after)
-        elif before:
+        if before:
             return self.add_node_before(node, before)
-        else:
-            node.parent = self
-            self._children.append(node)
-            self._children_dict[node.link] = node
-            return True
+        node.parent = self
+        self._children.append(node)
+        self._children_dict[node.link] = node
+        return True
 
     def add_node_after(self, node, after):
         success = False
@@ -103,7 +102,10 @@ class AdminHierarchyBuilder:
         while self.nodes_record:
             iterations += 1
             if iterations > 512:
-                message = "Misago Admin hierarchy is invalid or too complex to resolve. Nodes left: %s"
+                message = (
+                    "Misago Admin hierarchy is invalid or too complex to resolve. "
+                    "Nodes left: %s"
+                )
                 raise ValueError(message % self.nodes_record)
 
             for index, node in enumerate(self.nodes_record):
@@ -143,7 +145,8 @@ class AdminHierarchyBuilder:
     ):
         if self.nodes_dict:
             raise RuntimeError(
-                "Misago admin site has already been initialized. You can't add new nodes to it."
+                "Misago admin site has already been initialized. "
+                "You can't add new nodes to it."
             )
 
         if after and before:

misago/admin/middleware.py

@@ -9,13 +9,12 @@ from .views.auth import login
 class AdminAuthMiddleware(MiddlewareMixin):
     def process_view(self, request, view_func, view_args, view_kwargs):
         request.admin_namespace = get_protected_namespace(request)
-
         if request.admin_namespace:
             if not auth.is_admin_session(request):
                 auth.close_admin_session(request)
                 if request.resolver_match.url_name == "index":
                     return login(request)
-                else:
-                    return redirect("%s:index" % request.admin_namespace)
-            else:
-                auth.update_admin_session(request)
+                return redirect("%s:index" % request.admin_namespace)
+            auth.update_admin_session(request)
+
+        return None

misago/admin/tests/test_admin_form_templatetags.py

@@ -185,41 +185,36 @@ def test_attr_with_boolean_true_value_is_rendered():
     assert result == "bool"
 
 
-def test_attr_with_string_value_is_not_rendered():
+def test_bool_attr_with_string_value_is_not_rendered():
     result = render_bool_attrs({"name": "hello"})
     assert result == ""
 
 
-def test_attr_with_int_value_is_not_rendered():
+def test_bool_attr_with_int_value_is_not_rendered():
     result = render_bool_attrs({"col": 13})
     assert result == ""
 
 
-def test_attr_with_boolean_false_value_is_not_rendered():
+def test_bool_attr_with_boolean_false_value_is_not_rendered():
     result = render_bool_attrs({"selected": False})
     assert result == ""
 
 
-def test_attr_with_none_value_is_not_rendered():
+def test_bool_attr_with_none_value_is_not_rendered():
     result = render_bool_attrs({"selected": None})
     assert result == ""
 
 
-def test_attr_with_false_int_value_is_not_rendered():
+def test_bool_attr_with_false_int_value_is_not_rendered():
     result = render_bool_attrs({"selected": 0})
     assert result == ""
 
 
-def test_multiple_attrs_with_boolean_true_value_are_rendered():
+def test_multiple_bool_attrs_with_boolean_true_value_are_rendered():
     result = render_bool_attrs({"selected": True, "required": True})
     assert result == "selected required"
 
 
-def test_only_attrs_with_boolean_true_value_are_rendered():
+def test_only_bool_attrs_with_boolean_true_value_are_rendered():
     result = render_bool_attrs({"bool": True, "string": "hello", "int": 123})
     assert result == "bool"
-
-
-def test_empty_attr_dict_is_not_rendered():
-    result = render_bool_attrs({})
-    assert result == ""

misago/admin/urlpatterns.py

@@ -3,8 +3,10 @@ from django.conf.urls import include, url
 
 class URLPatterns:
     def __init__(self):
+        self.namespace_patterns = None
         self._namespaces = []
         self._patterns = []
+        self._urlpatterns = None
 
     def namespace(self, path, namespace, parent=None):
         self._namespaces.append(
@@ -57,13 +59,13 @@ class URLPatterns:
         return self.build_root_urlpatterns()
 
     def __call__(self):
-        try:
-            return self._urlpatterns
-        except AttributeError:
-            self._urlpatterns = self.build_urlpatterns()
-            self._namespaces = []
-            self._patterns = []
+        if self._urlpatterns:
             return self._urlpatterns
 
+        self._urlpatterns = self.build_urlpatterns()
+        self._namespaces = []
+        self._patterns = []
+        return self._urlpatterns
+
 
 urlpatterns = URLPatterns()

misago/admin/views/__init__.py

@@ -15,8 +15,7 @@ def get_protected_namespace(request):
                 return namespace
         except NoReverseMatch:
             pass
-    else:
-        return None
+    return None
 
 
 def render(request, template, context=None, error_page=False):
@@ -61,10 +60,8 @@ def protected_admin_view(f):
             if is_admin_session(request):
                 update_admin_session(request)
                 return f(request, *args, **kwargs)
-            else:
-                request.admin_namespace = protected_view
-                return login(request)
-        else:
-            return f(request, *args, **kwargs)
+            request.admin_namespace = protected_view
+            return login(request)
+        return f(request, *args, **kwargs)
 
     return decorator

misago/admin/views/auth.py

@@ -38,5 +38,4 @@ def logout(request):
         auth.close_admin_session(request)
         messages.info(request, _("Your admin session has been closed."))
         return redirect("misago:index")
-    else:
-        return redirect("misago:admin:index")
+    return redirect("misago:admin:index")

misago/admin/views/errorpages.py

@@ -7,28 +7,27 @@ from ..auth import is_admin_session, update_admin_session
 # Magic error page used by admin
 @protected_admin_view
 def _error_page(request, code, exception=None, default_message=None):
-    if is_admin_session(request):
-        template_pattern = "misago/admin/errorpages/%s.html" % code
-
-        response = render(
-            request,
-            template_pattern,
-            {"message": get_exception_message(exception, default_message)},
-            error_page=True,
-        )
-        response.status_code = code
-        return response
-    else:
+    if not is_admin_session(request):
         return redirect("misago:admin:index")
 
+    template_pattern = "misago/admin/errorpages/%s.html" % code
+
+    response = render(
+        request,
+        template_pattern,
+        {"message": get_exception_message(exception, default_message)},
+        error_page=True,
+    )
+    response.status_code = code
+    return response
+
 
 def admin_error_page(f):
     def decorator(request, *args, **kwargs):
         if get_protected_namespace(request):
             update_admin_session(request)
             return _error_page(request, *args, **kwargs)
-        else:
-            return f(request, *args, **kwargs)
+        return f(request, *args, **kwargs)
 
     return decorator
 
@@ -53,7 +52,6 @@ def admin_csrf_failure(f):
     def decorator(request, *args, **kwargs):
         if get_protected_namespace(request):
             return _csrf_failure(request, *args, **kwargs)
-        else:
-            return f(request, *args, **kwargs)
+        return f(request, *args, **kwargs)
 
     return decorator

misago/admin/views/generic/formsbuttons.py

@@ -12,17 +12,17 @@ class TargetedView(AdminView):
         pass
 
     def get_target(self, kwargs):
-        if len(kwargs) == 1:
-            select_for_update = self.get_model().objects
-            if self.is_atomic:
-                select_for_update = select_for_update.select_for_update()
-            # Does not work on Python 3:
-            # return select_for_update.get(pk=kwargs[kwargs.keys()[0]])
-            (pk,) = kwargs.values()
-            return select_for_update.get(pk=pk)
-        else:
+        if len(kwargs) != 1:
             return self.get_model()()
 
+        select_for_update = self.get_model().objects
+        if self.is_atomic:
+            select_for_update = select_for_update.select_for_update()
+        # Does not work on Python 3:
+        # return select_for_update.get(pk=kwargs[kwargs.keys()[0]])
+        (pk,) = kwargs.values()
+        return select_for_update.get(pk=pk)
+
     def get_target_or_none(self, request, kwargs):
         try:
             return self.get_target(kwargs)
@@ -33,8 +33,7 @@ class TargetedView(AdminView):
         if self.is_atomic:
             with transaction.atomic():
                 return self.wrapped_dispatch(request, *args, **kwargs)
-        else:
-            return self.wrapped_dispatch(request, *args, **kwargs)
+        return self.wrapped_dispatch(request, *args, **kwargs)
 
     def wrapped_dispatch(self, request, *args, **kwargs):
         target = self.get_target_or_none(request, kwargs)
@@ -42,7 +41,9 @@ class TargetedView(AdminView):
             messages.error(request, self.message_404)
             return redirect(self.root_link)
 
-        error = self.check_permissions(request, target)
+        error = self.check_permissions(  # pylint: disable=assignment-from-no-return
+            request, target
+        )
         if error:
             messages.error(request, error)
             return redirect(self.root_link)
@@ -63,8 +64,7 @@ class FormView(TargetedView):
     def initialize_form(self, form, request):
         if request.method == "POST":
             return form(request.POST, request.FILES)
-        else:
-            return form()
+        return form()
 
     def handle_form(self, form, request):
         raise NotImplementedError(
@@ -80,10 +80,9 @@ class FormView(TargetedView):
 
             if response:
                 return response
-            elif "stay" in request.POST:
+            if "stay" in request.POST:
                 return redirect(request.path)
-            else:
-                return redirect(self.root_link)
+            return redirect(self.root_link)
 
         return self.render(request, {"form": form})
 
@@ -97,8 +96,7 @@ class ModelFormView(FormView):
     def initialize_form(self, form, request, target):
         if request.method == "POST":
             return form(request.POST, request.FILES, instance=target)
-        else:
-            return form(instance=target)
+        return form(instance=target)
 
     def handle_form(self, form, request, target):
         form.instance.save()
@@ -110,14 +108,14 @@ class ModelFormView(FormView):
         form = self.initialize_form(FormType, request, target)
 
         if request.method == "POST" and form.is_valid():
-            response = self.handle_form(form, request, target)
-
+            response = self.handle_form(  # pylint: disable=assignment-from-no-return
+                form, request, target
+            )
             if response:
                 return response
-            elif "stay" in request.POST:
+            if "stay" in request.POST:
                 return redirect(request.path)
-            else:
-                return redirect(self.root_link)
+            return redirect(self.root_link)
 
         return self.render(request, {"form": form, "target": target})
 

misago/admin/views/generic/list.py

@@ -60,7 +60,9 @@ class ListView(AdminView):
     def get_queryset(self):
         return self.get_model().objects.all()
 
-    def dispatch(self, request, *args, **kwargs):
+    def dispatch(
+        self, request, *args, **kwargs
+    ):  # pylint: disable=too-many-branches, too-many-locals
         mass_actions_list = self.mass_actions or []
         extra_actions_list = self.extra_actions or []
 
@@ -90,8 +92,7 @@ class ListView(AdminView):
                 response = self.handle_mass_action(request, context)
                 if response:
                     return response
-                else:
-                    return redirect(request.path)
+                return redirect(request.path)
             except MassActionError as e:
                 messages.error(request, e.args[0])
 
@@ -214,8 +215,6 @@ class ListView(AdminView):
         for method in ("GET", "session"):
             if methods.get(method):
                 return methods.get(method)
-        else:
-            return {}
 
     def apply_filtering_on_context(self, context, active_filters, search_form):
         context["active_filters"] = active_filters
@@ -248,11 +247,10 @@ class ListView(AdminView):
         return self.clean_ordering(new_ordering)
 
     def clean_ordering(self, new_ordering):
-        for order_by, _ in self.ordering:
+        for order_by, _ in self.ordering:  # pylint: disable=not-an-iterable
             if order_by == new_ordering:
                 return order_by
-        else:
-            return None
+        return None
 
     def get_ordering_methods(self, request):
         return {
@@ -267,7 +265,7 @@ class ListView(AdminView):
                 return methods.get(method)
 
     def set_ordering_in_context(self, context, method):
-        for order_by, name in self.ordering:
+        for order_by, name in self.ordering:  # pylint: disable=not-an-iterable
             order_as_dict = {
                 "type": "desc" if order_by[0] == "-" else "asc",
                 "order_by": order_by,
@@ -306,11 +304,10 @@ class ListView(AdminView):
             return action_callable(request, action_queryset)
 
     def select_mass_action(self, action):
-        for definition in self.mass_actions:
+        for definition in self.mass_actions:  # pylint: disable=not-an-iterable
             if definition["action"] == action:
                 return definition
-        else:
-            raise MassActionError(_("Action is not allowed."))
+        raise MassActionError(_("Action is not allowed."))
 
     # Querystring builder
     def make_querystring(self, context):

misago/admin/views/index.py

@@ -77,7 +77,7 @@ def check_version(request):
                 }
 
             cache.set(VERSION_CHECK_CACHE_KEY, version, 180)
-        except (RequestException, IndexError, KeyError, ValueError) as e:
+        except (RequestException, IndexError, KeyError, ValueError):
             version = {
                 "is_error": True,
                 "message": _("Failed to connect to pypi.org API. Try again later."),