Update formatting and CI (#21)

- Completely migrate to `pre-commit` and `pre-commit.ci`
- Add `ruff`, `codespell`, and some built-in pre-commit hooks
- Add Pipfile verification
- Deprecate `check.sh` and `format.sh`
- Apply new formatting to the entire codebase

Author: JasonGrace2282

.github/workflows/ci.yml

@@ -30,7 +30,10 @@ jobs:
         run: |
           set -e
           pip install pipenv
-          pipenv install --dev --deploy
 
-      - name: Check formatting with format.sh
-        run: pipenv run ./scripts/format.sh && test -z "$(git status --porcelain=v1 .)"
+      - name: Check Pipfile.lock
+        run: pipenv verify
+
+      # Since we don't install dependencies, we have to have this
+      - name: Make dir for virtualenv
+        run: mkdir -p $HOME/.local/share/virtualenvs

.pre-commit-config.yaml

@@ -1,9 +1,37 @@
+default_stages: [commit, push]
+fail_fast: false
+exclude: ^tin/(static/.*vendor|.*migrations)
+
 repos:
-- repo: local
-  hooks:
-    - id: format
-      name: format
-      entry: ./scripts/format.sh && test -z "$(git status --porcelain=v1 .)"
-      language: system
-      types: [python]
-      pass_filenames: false
+  - repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v4.4.0
+    hooks:
+      - id: check-ast
+        name: validate python
+      - id: trailing-whitespace
+      - id: mixed-line-ending
+      - id: check-toml
+      - id: detect-private-key
+      - id: check-yaml
+  - repo: https://github.com/codespell-project/codespell
+    rev: v2.2.5
+    hooks:
+      - id: codespell
+        files: ^.*\.(py|md|rst)$
+        # TODO: Remove after python version >= 3.11
+        additional_dependencies:
+          - tomli
+  - repo: https://github.com/astral-sh/ruff-pre-commit
+    rev: v0.3.5
+    hooks:
+      - id: ruff
+        args: [ "--fix", "--exit-non-zero-on-fix" ]
+        files: ^tin/.*
+        name: ruff lint
+      - id: ruff-format
+        files: ^tin/.*
+  - repo: https://github.com/pycqa/isort
+    rev: 5.12.0
+    hooks:
+      - id: isort
+        name: isort (python)

Pipfile

@@ -24,12 +24,6 @@ mosspy = "~=1.0"
 django-debug-toolbar = "~=4.3"
 
 [dev-packages]
-flake8 = "*"
-pylint = "*"
-isort = "*"
-black = "*"
-autopep8 = "*"
-pylint-django = "*"
 django-stubs = "*"
 pre-commit = "*"
 

Pipfile.lock

@@ -31,7 +31,7 @@
 # fmt: on
 
 for user_info in users:
-    print("Creating user {}".format(user_info[0]))
+    print(f"Creating user {user_info[0]}")
     if user_info[1] is None:
         user = User.objects.get_or_create(username=user_info[0])[0]
     else:

create_debug_users.py

@@ -18,3 +18,77 @@ exclude = '''
 )/
 '''
 
+[tool.ruff]
+exclude = [
+  ".eggs",
+  ".git",
+  ".github",
+  ".hg",
+  ".mypy_cache",
+  ".tox",
+  ".venv",
+  ".env",
+  "venv",
+  "env",
+  "secret",
+  "build",
+  "_build",
+  "buck-out",
+  "dist",
+  "media",
+  "migrations",
+]
+
+# show fixes made in stdout
+# show-fixes = true
+
+line-length = 100
+
+target-version = "py38"
+
+[tool.ruff.lint]
+select = [
+    # flake8-bugbear
+    "B",
+    # flake8-comprehensions
+    "C4",
+    # flake8-django
+    "DJ",
+    # pycodestyle
+    "E",
+    # Pyflakes
+    "F",
+    # flake8-no-pep420
+    "INP",
+    # Pylint
+    "PL",
+    # pygrep hooks
+    "PGH",
+    # ruff
+    "RUF",
+    # pyupgrade
+    "UP",
+]
+ignore = [
+  # null=True on charfields
+  "DJ001",
+  # branching
+  "PLR09",
+  # magic number comparison
+  "PLR2004",
+  # mutable class attrs annotated as typing.ClassVar
+  "RUF012",
+  # as recommended by https://docs.astral.sh/ruff/formatter/#conflicting-lint-rules
+  "E111",
+  "E114",
+  "E117",
+  "E501",
+]
+
+[tool.ruff.format]
+docstring-code-format = true
+line-ending = "lf"
+
+[tool.codespell]
+write-changes = true
+ignore-words-list = ["num", "ans"]

pyproject.toml

@@ -1,6 +1,9 @@
 #!/bin/bash
 cd "$(dirname -- "$(dirname -- "$(readlink -f "$0")")")"
 
+echo "This script is now deprecated, please use pre-commit instead."
+echo "To run pre-commit before commiting, do 'pre-commit install'"
+
 for cmd in flake8 isort pylint; do
     if [[ ! -x "$(which "$cmd")" ]]; then
         echo "Could not find $cmd. Please make sure that flake8, isort, and pylint are all installed."

scripts/check.sh

@@ -1,6 +1,9 @@
 #!/bin/bash
 cd "$(dirname -- "$(dirname -- "$(readlink -f "$0")")")"
 
+echo "This script is now deprecated, please use pre-commit instead"
+echo "To run pre-commit before commiting, do 'pre-commit install'"
+
 for cmd in black autopep8 isort; do
     if [[ ! -x "$(which "$cmd")" ]]; then
         echo "Could not find $cmd. Please make sure that black, autopep8, and isort are all installed."

scripts/format.sh

@@ -1,7 +1,7 @@
 from __future__ import annotations
 
 from logging import getLogger
-from typing import Dict, Iterable, Tuple
+from typing import Iterable
 
 from django import forms
 from django.conf import settings
@@ -33,20 +33,20 @@ def __init__(self, course, *args, **kwargs):
         # prevent description from getting too big
         self.fields["description"].widget.attrs.update({"id": "description"})
 
-    def get_sections(self) -> Iterable[Dict[str, str | Tuple[str, ...] | bool]]:
+    def get_sections(self) -> Iterable[dict[str, str | tuple[str, ...] | bool]]:
         for section in self.Meta.sections:
             if section["name"]:
                 # operate on copy so errors on refresh don't happen
-                section = section.copy()
-                section["fields"] = tuple(self[field] for field in section["fields"])
-                yield section
+                new_section = section.copy()
+                new_section["fields"] = tuple(self[field] for field in new_section["fields"])
+                yield new_section
 
-    def get_main_section(self) -> Dict[str, str | Tuple[str, ...]]:
+    def get_main_section(self) -> dict[str, str | tuple[str, ...]]:
         for section in self.Meta.sections:
             if section["name"] == "":
-                section = section.copy()
-                section["fields"] = tuple(self[field] for field in section["fields"])
-                return section
+                new_section = section.copy()
+                new_section["fields"] = tuple(self[field] for field in new_section["fields"])
+                return new_section
         logger.error(f"Could not find main section for assignment {self}")
         return {"fields": ()}
 

tin/apps/assignments/forms.py

@@ -28,12 +28,12 @@ class Folder(models.Model):
     def __str__(self):
         return self.name
 
-    def __repr__(self):
-        return self.name
-
     def get_absolute_url(self):
         return reverse("assignments:show_folder", args=[self.course.id, self.id])
 
+    def __repr__(self):
+        return self.name
+
 
 class AssignmentQuerySet(models.query.QuerySet):
     def filter_visible(self, user):
@@ -54,14 +54,12 @@ def filter_editable(self, user):
 def upload_grader_file_path(assignment, _):  # pylint: disable=unused-argument
     assert assignment.id is not None
     if assignment.language == "P":
-        return "assignment-{}/grader.py".format(assignment.id)
+        return f"assignment-{assignment.id}/grader.py"
     else:
-        return "assignment-{}/Grader.java".format(assignment.id)
+        return f"assignment-{assignment.id}/Grader.java"
 
 
 class Assignment(models.Model):
-    objects = AssignmentQuerySet.as_manager()
-
     name = models.CharField(max_length=50)
     folder = models.ForeignKey(
         Folder,
@@ -126,15 +124,17 @@ class Assignment(models.Model):
 
     last_action_output = models.CharField(max_length=16 * 1024, default="", null=False, blank=True)
 
-    def __str__(self):
-        return self.name
+    objects = AssignmentQuerySet.as_manager()
 
-    def __repr__(self):
+    def __str__(self):
         return self.name
 
     def get_absolute_url(self):
         return reverse("assignments:show", args=(self.id,))
 
+    def __repr__(self):
+        return self.name
+
     def make_assignment_dir(self) -> None:
         assignment_path = os.path.join(settings.MEDIA_ROOT, f"assignment-{self.id}")
         os.makedirs(assignment_path, exist_ok=True)
@@ -169,7 +169,7 @@ def save_grader_file(self, grader_text: str) -> None:
                 stdout=subprocess.DEVNULL,
                 stderr=subprocess.PIPE,
                 encoding="utf-8",
-                universal_newlines=True,
+                text=True,
                 check=True,
             )
         except FileNotFoundError as e:
@@ -203,7 +203,7 @@ def list_files(self) -> List[Tuple[int, str, str, int, datetime.datetime]]:
     def save_file(self, file_text: str, file_name: str) -> None:
         self.make_assignment_dir()
 
-        fpath = os.path.join(settings.MEDIA_ROOT, "assignment-{}".format(self.id), file_name)
+        fpath = os.path.join(settings.MEDIA_ROOT, f"assignment-{self.id}", file_name)
 
         os.makedirs(os.path.dirname(fpath), exist_ok=True)
 
@@ -220,7 +220,7 @@ def save_file(self, file_text: str, file_name: str) -> None:
                 stdout=subprocess.DEVNULL,
                 stderr=subprocess.PIPE,
                 encoding="utf-8",
-                universal_newlines=True,
+                text=True,
                 check=True,
             )
         except FileNotFoundError as e:
@@ -349,12 +349,12 @@ class Meta:
     def __str__(self):
         return f"Quiz for {self.assignment}"
 
-    def __repr__(self):
-        return f"Quiz for {self.assignment}"
-
     def get_absolute_url(self):
         return reverse("assignments:show", args=(self.assignment.id,))
 
+    def __repr__(self):
+        return f"Quiz for {self.assignment}"
+
     def issues_for_student(self, student):
         return (
             sum(lm.severity for lm in self.log_messages.filter(student=student))
@@ -387,14 +387,14 @@ class LogMessage(models.Model):
     def __str__(self):
         return f"{self.content} for {self.quiz}"
 
-    def __repr__(self):
-        return f"{self.content} for {self.quiz}"
-
     def get_absolute_url(self):
         return reverse(
             "assignments:student_submission", args=(self.quiz.assignment.id, self.student.id)
         )
 
+    def __repr__(self):
+        return f"{self.content} for {self.quiz}"
+
 
 def moss_base_file_path(obj, _):  # pylint: disable=unused-argument
     assert obj.assignment.id is not None
@@ -452,6 +452,9 @@ class MossResult(models.Model):
     url = models.URLField(max_length=200, null=True, blank=True)
     status = models.CharField(max_length=1024, default="", null=False, blank=True)
 
+    def __str__(self):
+        return f"Moss result for {self.assignment}"
+
     @property
     def extension(self):
         return "java" if self.language == "java" else "py"
@@ -460,9 +463,6 @@ def extension(self):
     def download_folder(self):
         return os.path.join(settings.MEDIA_ROOT, "moss-runs", f"moss-{self.id}")
 
-    def __str__(self):
-        return f"Moss result for {self.assignment}"
-
     def __repr__(self):
         return f"Moss result for {self.assignment}"
 
@@ -476,7 +476,7 @@ def run_action(command: List[str]) -> str:
             stdout=subprocess.PIPE,
             stderr=subprocess.STDOUT,
             encoding="utf-8",
-            universal_newlines=True,
+            text=True,
         )
     except FileNotFoundError as e:
         logger.error("File not found: %s", e)