Add mypy type checker to GitHub Actions (#1056)

* Fix yaml formatting

* Bump actions versions to latest

* Bump pipenv version to latest

* Add mypy lint action

* Fix mypy lint errors

* Suppress warnings about unchecked function bodies for now

* Add deprecation comment

* Log OperationalError exception traces

Author: Askaholic

.github/workflows/doc.yml

@@ -9,10 +9,10 @@ jobs:
   deploy:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
 
       - name: Setup Python
-        uses: actions/setup-python@v4
+        uses: actions/setup-python@v5
         with:
           python-version: '3.10'
 

.github/workflows/lint.yml

@@ -13,10 +13,10 @@ jobs:
   isort:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
 
       - name: Setup Python
-        uses: actions/setup-python@v4
+        uses: actions/setup-python@v5
         with:
           python-version: '3.10'
 
@@ -28,10 +28,10 @@ jobs:
   flake8:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
 
       - name: Setup Python
-        uses: actions/setup-python@v4
+        uses: actions/setup-python@v5
         with:
           python-version: '3.10'
 
@@ -40,13 +40,40 @@ jobs:
           flake8_version: 6.0.0
           plugins: flake8-quotes~=3.3
 
+  mypy:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Create requirements file
+        run: |
+          pip install pipenv==2024.4.1
+          pipenv requirements --dev > requirements.txt
+
+      # Mypy-check runs inside its own docker image
+      - uses: jpetrucciani/mypy-check@master
+        with:
+          python_version: '3.10'
+          requirements: |
+            types-PyYAML
+            types-PyMySQL
+            types-cachetools
+          requirements_file: requirements.txt
+          mypy_flags: |
+            --disable-error-code=annotation-unchecked
+            --follow-untyped-imports
+            --strict-equality
+            --warn-redundant-casts
+            --warn-unused-ignores
+          path: server/ main.py
+
   pipenv-verify:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
 
       - name: Setup Python
-        uses: actions/setup-python@v4
+        uses: actions/setup-python@v5
         with:
           python-version: '3.10'
 

.github/workflows/test.yml

@@ -51,16 +51,16 @@ jobs:
           RABBITMQ_DEFAULT_VHOST: /
 
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
 
       - name: Cache hypothesis examples
-        uses: actions/cache@v3
+        uses: actions/cache@v4
         with:
           path: .hypothesis
           key: .hypothesis
 
       - name: Setup Python
-        uses: actions/setup-python@v4
+        uses: actions/setup-python@v5
         with:
           python-version: '3.10'
 
@@ -81,7 +81,7 @@ jobs:
 
       - name: Install dependencies with pipenv
         run: |
-          pip install pipenv==2023.4.20
+          pip install pipenv==2024.4.1
           pipenv sync --dev
           pipenv run pip install pytest-github-actions-annotate-failures
 
@@ -105,14 +105,14 @@ jobs:
         run: PYTHONWARNINGS='error::UserWarning' pipenv run pdoc3 server >/dev/null
 
   docker-build:
-      runs-on: ubuntu-latest
-      steps:
-        - uses: actions/checkout@v3
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
 
-        - name: Build docker image
-          run: docker build --build-arg GITHUB_REF -t test_image .
+      - name: Build docker image
+        run: docker build --build-arg GITHUB_REF -t test_image .
 
-        - name: Test image
-          run: |
-            docker run --rm -d -p 8001:8001 test_image
-            nc -z localhost 8001
+      - name: Test image
+        run: |
+          docker run --rm -d -p 8001:8001 test_image
+          nc -z localhost 8001

main.py

@@ -216,7 +216,7 @@ def drain_handler(sig: int, frame):
     stop_time = time.perf_counter()
     logger.info(
         "Total server uptime: %s",
-        humanize.precisedelta(stop_time - startup_time)
+        humanize.precisedelta(int(stop_time - startup_time))
     )
 
     if shutdown_time is not None:

server/__init__.py

@@ -112,7 +112,7 @@
 import asyncio
 import logging
 import time
-from typing import Optional
+from typing import Optional, cast
 
 import server.metrics as metrics
 
@@ -176,7 +176,7 @@ def __init__(
         self,
         name: str,
         database: FAFDatabase,
-        loop: asyncio.BaseEventLoop,
+        loop: asyncio.AbstractEventLoop,
         # For testing
         _override_services: Optional[dict[str, Service]] = None
     ):
@@ -320,8 +320,8 @@ async def drain(self):
         """
         Wait for all games to end.
         """
-        game_service: GameService = self.services["game_service"]
-        broadcast_service: BroadcastService = self.services["broadcast_service"]
+        game_service = cast(GameService, self.services["game_service"])
+        broadcast_service = cast(BroadcastService, self.services["broadcast_service"])
         try:
             await asyncio.wait_for(
                 game_service.drain_games(),

server/asyncio_extensions.py

@@ -16,7 +16,6 @@
     Optional,
     Protocol,
     TypeVar,
-    cast,
     overload
 )
 
@@ -27,7 +26,7 @@
 T = TypeVar("T")
 
 
-class AsyncLock(Protocol, AsyncContextManager["AsyncLock"]):
+class AsyncLock(Protocol, AsyncContextManager[None]):
     def locked(self) -> bool: ...
     async def acquire(self) -> bool: ...
     def release(self) -> None: ...
@@ -153,7 +152,7 @@ async def wrapped(*args, **kwargs):
         nonlocal lock
 
         if lock is None:
-            lock = lock or cast(AsyncLock, asyncio.Lock())
+            lock = lock or asyncio.Lock()
 
         async with lock:
             return await function(*args, **kwargs)

server/broadcast_service.py

@@ -1,4 +1,6 @@
 import asyncio
+import logging
+from typing import TYPE_CHECKING, ClassVar, Optional
 
 import humanize
 from aio_pika import DeliveryMode
@@ -12,13 +14,18 @@
 from .player_service import PlayerService
 from .timing import LazyIntervalTimer
 
+if TYPE_CHECKING:
+    from server import ServerInstance
+
 
 @with_logger
 class BroadcastService(Service):
     """
     Broadcast updates about changed entities.
     """
 
+    _logger: ClassVar[logging.Logger]
+
     def __init__(
         self,
         server: "ServerInstance",
@@ -30,7 +37,7 @@ def __init__(
         self.message_queue_service = message_queue_service
         self.game_service = game_service
         self.player_service = player_service
-        self._report_dirties_event = None
+        self._report_dirties_event: Optional[asyncio.Event] = None
 
     async def initialize(self):
         # Using a lazy interval timer so that the intervals can be changed

server/config.py

@@ -6,7 +6,7 @@
 import logging
 import os
 import statistics
-from typing import Callable
+from typing import Callable, ClassVar, Iterable
 
 import trueskill
 import yaml
@@ -27,7 +27,7 @@
 # see: http://forums.faforever.com/viewtopic.php?f=45&t=11698#p119599
 # Optimum values for ladder here, using them for global as well.
 trueskill.setup(mu=1500, sigma=500, beta=240, tau=10, draw_probability=0.10)
-MAP_POOL_RATING_SELECTION_FUNCTIONS = {
+MAP_POOL_RATING_SELECTION_FUNCTIONS: dict[str, Callable[[Iterable[float]], float]] = {
     "mean": statistics.mean,
     "min": min,
     "max": max,
@@ -36,6 +36,8 @@
 
 @with_logger
 class ConfigurationStore:
+    _logger: ClassVar[logging.Logger]
+
     def __init__(self):
         """
         Change default values here.

server/configuration_service.py

@@ -3,6 +3,8 @@
 """
 
 import asyncio
+import logging
+from typing import ClassVar, Optional
 
 from .config import config
 from .core import Service
@@ -11,9 +13,11 @@
 
 @with_logger
 class ConfigurationService(Service):
+    _logger: ClassVar[logging.Logger]
+
     def __init__(self) -> None:
         self._store = config
-        self._task = None
+        self._task: Optional[asyncio.Task] = None
 
     async def initialize(self) -> None:
         self._task = asyncio.create_task(self._worker_loop())

server/control.py

@@ -2,25 +2,34 @@
 Tiny http server for introspecting state
 """
 
+import logging
 import socket
+from typing import TYPE_CHECKING, ClassVar, Optional, cast
 
 from aiohttp import web
 
 from .config import config
 from .decorators import with_logger
+from .game_service import GameService
+from .player_service import PlayerService
+
+if TYPE_CHECKING:
+    from server import ServerInstance
 
 
 @with_logger
 class ControlServer:
+    _logger: ClassVar[logging.Logger]
+
     def __init__(
         self,
         lobby_server: "ServerInstance",
     ):
         self.lobby_server = lobby_server
-        self.game_service = lobby_server.services["game_service"]
-        self.player_service = lobby_server.services["player_service"]
-        self.host = None
-        self.port = None
+        self.game_service = cast(GameService, lobby_server.services["game_service"])
+        self.player_service = cast(PlayerService, lobby_server.services["player_service"])
+        self.host: Optional[str] = None
+        self.port: Optional[int] = None
 
         self.app = web.Application()
         self.runner = web.AppRunner(self.app)
@@ -55,13 +64,13 @@ async def shutdown(self) -> None:
         self.host = None
         self.port = None
 
-    async def games(self, request) -> web.Response:
+    async def games(self, request: web.Request) -> web.Response:
         return web.json_response([
             game.to_dict()
             for game in self.game_service.all_games
         ])
 
-    async def players(self, request) -> web.Response:
+    async def players(self, request: web.Request) -> web.Response:
         return web.json_response([
             player.to_dict()
             for player in self.player_service.all_players