feat: add ruff (#2865)

* feat: add ruff

@lint: ignore EM101 Exception must not use a string literal, assign to variable first

* feat: update poetry.lock

* lint: fix all linting errors

* lint: fmt

* update readme, remove pylint comments and ignore ISC001

* lint: remove unused noqa

* lint: lint fixes

* lint: fix after rebase

* fix: fix more lint issues

* fix: disable D401

* refactor: fix linting errors

.github/workflows/ci.yml

@@ -61,12 +61,11 @@ jobs:
       - name: Install dependencies
         run: poetry install --no-interaction
 
-# TODO: use ruff or upgrade pylint, pylint-django, and asteroid to support String based model references.
-#      - name: Lint
-#        run: poetry run pylint ./**/*.py
+      - name: Lint (Ruff)
+        run: poetry run ruff check .
 
-      - name: Code Formatting (Black)
-        run: poetry run black --check .
+      - name: Code Formatting (Ruff)
+        run: poetry run ruff format --check .
 
       # Configurations required for elasticsearch.
       - name: Configure sysctl limits

README.md

@@ -72,13 +72,13 @@ for running the app
     # Run Python test cases in a single file that match some function/class name
     docker-compose run --rm web pytest /path/to/test.py -k test_some_logic
     # Run Python linter
-    docker-compose run --rm web pylint
+    docker-compose run --rm web ruff check .
 
     ### PYTHON FORMATTING
     # Format all python files
-    docker-compose run --rm web black .
+    docker-compose run --rm web ruff format --check .
     # Format a specific file
-    docker-compose run --rm web black /path/to/file.py
+    docker-compose run --rm ruff format --check /path/to/file.py
 
     ### JS/CSS TESTS/LINTING
     # We also include a helper script to execute JS tests in most of our projects 

affiliate/admin.py

@@ -31,7 +31,7 @@ class AffiliateReferralActionAdmin(TimestampedModelAdmin):
     list_filter = ["affiliate__name"]
     ordering = ["-created_on"]
 
-    def get_queryset(self, request):
+    def get_queryset(self, request):  # noqa: ARG002
         """Overrides base method"""
         return self.model.objects.select_related("affiliate")
 

affiliate/api.py

@@ -14,10 +14,7 @@ def get_affiliate_code_from_qstring(request):
     Returns:
         Optional[str]: The affiliate code (or None)
     """
-    if request.method != "GET":
-        return None
-    affiliate_code = request.GET.get(AFFILIATE_QS_PARAM)
-    return affiliate_code
+    return request.GET.get(AFFILIATE_QS_PARAM) if request.method == "GET" else None
 
 
 def get_affiliate_code_from_request(request):

affiliate/api_test.py

@@ -36,7 +36,7 @@ def test_get_affiliate_code_from_request():
     request = RequestFactory().get("/")
     code = get_affiliate_code_from_request(request)
     assert code is None
-    setattr(request, "affiliate_code", affiliate_code)
+    setattr(request, "affiliate_code", affiliate_code)  # noqa: B010
     code = get_affiliate_code_from_request(request)
     assert code == affiliate_code
 
@@ -62,7 +62,7 @@ def test_get_affiliate_id_from_request():
     """
     affiliate_code = "abc"
     request = RequestFactory().get("/")
-    setattr(request, "affiliate_code", affiliate_code)
+    setattr(request, "affiliate_code", affiliate_code)  # noqa: B010
     affiliate_id = get_affiliate_id_from_request(request)
     assert affiliate_id is None
     affiliate = AffiliateFactory.create(code=affiliate_code)

affiliate/middleware.py

@@ -9,9 +9,9 @@ class AffiliateMiddleware:
     def __init__(self, get_response):
         self.get_response = get_response
 
-    def __call__(self, request):
+    def __call__(self, request):  # noqa: D102
         request.affiliate_code = None
-        session = getattr(request, "session")
+        session = getattr(request, "session")  # noqa: B009
         if session is None:
             return self.get_response(request)
         qs_affiliate_code = get_affiliate_code_from_qstring(request)

affiliate/migrations/0001_affiliate_initial_models.py

@@ -1,12 +1,11 @@
 # Generated by Django 2.2.10 on 2020-10-22 16:42
 
+import django.db.models.deletion
 from django.conf import settings
 from django.db import migrations, models
-import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
-
     initial = True
 
     dependencies = [

affiliate/models.py

@@ -12,9 +12,7 @@ class Affiliate(TimestampedModel):
     name = models.CharField(max_length=50, unique=True)
 
     def __str__(self):
-        return "Affiliate: id={}, code={}, name={}".format(
-            self.id, self.code, self.name
-        )
+        return f"Affiliate: id={self.id}, code={self.code}, name={self.name}"
 
 
 class AffiliateReferralAction(TimestampedModel):

authentication/api.py

@@ -2,12 +2,11 @@
 from importlib import import_module
 
 from django.conf import settings
-from django.contrib.auth import SESSION_KEY, BACKEND_SESSION_KEY, HASH_SESSION_KEY
+from django.contrib.auth import BACKEND_SESSION_KEY, HASH_SESSION_KEY, SESSION_KEY
 from django.db import IntegrityError
 
-from users.utils import is_duplicate_username_error
 from users.api import find_available_username
-
+from users.utils import is_duplicate_username_error
 
 USERNAME_COLLISION_ATTEMPTS = 10
 
@@ -26,7 +25,7 @@ def create_user_session(user):
 
     session = SessionStore()
 
-    session[SESSION_KEY] = user._meta.pk.value_to_string(user)
+    session[SESSION_KEY] = user._meta.pk.value_to_string(user)  # noqa: SLF001
     session[BACKEND_SESSION_KEY] = "django.contrib.auth.backends.ModelBackend"
     session[HASH_SESSION_KEY] = user.get_session_auth_hash()
     session.save()
@@ -51,13 +50,13 @@ def create_user_with_generated_username(serializer, initial_username):
     username = initial_username
     attempts = 0
 
-    if len(username) < 2:
+    if len(username) < 2:  # noqa: PLR2004
         username = username + "11"
 
     while created_user is None and attempts < USERNAME_COLLISION_ATTEMPTS:
         try:
             created_user = serializer.save(username=username)
-        except IntegrityError as exc:
+        except IntegrityError as exc:  # noqa: PERF203
             if not is_duplicate_username_error(exc):
                 raise
             username = find_available_username(initial_username)

authentication/api_test.py

@@ -110,6 +110,6 @@ def test_create_user_exception(mocker):
     patched_save = mocker.patch.object(
         UserSerializer, "save", side_effect=ValueError("idk")
     )
-    with pytest.raises(ValueError):
+    with pytest.raises(ValueError):  # noqa: PT011
         api.create_user_with_generated_username(UserSerializer(data={}), "testuser")
     patched_save.assert_called_once()

authentication/exceptions.py

@@ -92,5 +92,5 @@ class UserTryAgainLaterException(AuthException):
     """The user should try to register again later"""
 
 
-class UserMissingSocialAuthException(Exception):
+class UserMissingSocialAuthException(Exception):  # noqa: N818
     """Raised if the user doesn't have a social auth"""

authentication/middleware.py

@@ -1,7 +1,7 @@
 """Authentication middleware"""
-from django.shortcuts import redirect
 from urllib.parse import quote
 
+from django.shortcuts import redirect
 from social_core.exceptions import SocialAuthBaseException
 from social_django.middleware import SocialAuthExceptionMiddleware
 
@@ -19,17 +19,17 @@ def process_exception(self, request, exception):
         """
         strategy = getattr(request, "social_strategy", None)
         if strategy is None or self.raise_exception(request, exception):
-            return
+            return  # noqa: RET502
 
-        if isinstance(exception, SocialAuthBaseException):
+        if isinstance(exception, SocialAuthBaseException):  # noqa: RET503
             backend = getattr(request, "backend", None)
             backend_name = getattr(backend, "name", "unknown-backend")
 
             message = self.get_message(request, exception)
             url = self.get_redirect_uri(request, exception)
 
-            if url:
-                url += ("?" in url and "&" or "?") + "message={0}&backend={1}".format(
+            if url:  # noqa: RET503
+                url += ("?" in url and "&" or "?") + "message={0}&backend={1}".format(  # noqa: UP030
                     quote(message), backend_name
                 )
                 return redirect(url)

authentication/middleware_test.py

@@ -1,7 +1,8 @@
 """Tests for auth middleware"""
+from urllib.parse import quote
+
 from django.contrib.sessions.middleware import SessionMiddleware
 from django.shortcuts import reverse
-from urllib.parse import quote
 from rest_framework import status
 from social_core.exceptions import AuthAlreadyAssociated
 from social_django.utils import load_backend, load_strategy

authentication/pipeline/compliance.py

@@ -12,13 +12,15 @@
 )
 from compliance import api
 
-
 log = logging.getLogger()
 
 
 def verify_exports_compliance(
-    strategy, backend, user=None, **kwargs
-):  # pylint: disable=unused-argument
+    strategy,  # noqa: ARG001
+    backend,
+    user=None,
+    **kwargs,  # noqa: ARG001
+):
     """
     Verify that the user is allowed by exports compliance
 
@@ -37,7 +39,7 @@ def verify_exports_compliance(
 
     try:
         export_inquiry = api.verify_user_with_exports(user)
-    except Exception as exc:  # pylint: disable=broad-except
+    except Exception as exc:
         # hard failure to request the exports API, log an error but don't let the user proceed
         log.exception("Unable to verify exports compliance")
         raise UserTryAgainLaterException(backend) from exc
@@ -78,12 +80,12 @@ def verify_exports_compliance(
                     [settings.ADMIN_EMAIL],
                     connection=connection,
                 )
-        except Exception:  # pylint: disable=broad-except
+        except Exception:
             log.exception(
                 "Exception sending email to support regarding export compliance check failure"
             )
         raise UserExportBlockedException(backend, export_inquiry.reason_code)
     if export_inquiry.is_unknown:
-        raise AuthException("Unable to authenticate, please contact support")
+        raise AuthException("Unable to authenticate, please contact support")  # noqa: EM101
 
     return {}

authentication/pipeline/compliance_test.py

@@ -9,7 +9,6 @@
 from authentication.pipeline import compliance
 from compliance.factories import ExportsInquiryLogFactory
 
-
 pytestmark = pytest.mark.django_db
 
 
@@ -22,17 +21,17 @@ def test_verify_exports_compliance_disabled(mocker):
 
 
 @pytest.mark.parametrize(
-    "is_active, inquiry_exists, should_verify",
+    "is_active, inquiry_exists, should_verify",  # noqa: PT006
     [
-        [True, True, False],
-        [True, False, True],
-        [False, True, True],
-        [False, False, True],
+        [True, True, False],  # noqa: PT007
+        [True, False, True],  # noqa: PT007
+        [False, True, True],  # noqa: PT007
+        [False, False, True],  # noqa: PT007
     ],
 )
-def test_verify_exports_compliance_user_active(
+def test_verify_exports_compliance_user_active(  # noqa: PLR0913
     mailoutbox, mocker, user, is_active, inquiry_exists, should_verify
-):  # pylint: disable=too-many-arguments
+):
     """Assert that the user is verified only if they already haven't been"""
     user.is_active = is_active
     if inquiry_exists: