chore: resolve all pyright diagnostics and add a pyright CI job (#95)

.github/workflows/python-tests.yml

@@ -36,3 +36,23 @@ jobs:
 
       - name: Run unit tests
         run: poetry run pytest
+
+  pyright:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out repository
+        uses: actions/checkout@v4
+
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: "3.12"
+
+      - name: Install Poetry
+        run: pip install poetry
+
+      - name: Install dependencies
+        run: poetry install
+
+      - name: Run Pyright
+        run: poetry run pyright

campus_python/__init__.py

@@ -163,7 +163,7 @@ def audit(self) -> AuditRoot:
                 mode="device",
             )
             env.require("AUDIT_API_KEY")
-            client.set_bearer_authorization(env.get("AUDIT_API_KEY"))
+            client.set_bearer_authorization(env.get("AUDIT_API_KEY") or "")
             self._audit = AuditRoot(json_client=client)
         return self._audit
 
@@ -190,7 +190,12 @@ def _get_token_from_session(
         """Get the token from flask session."""
         # flask session stores a session id
         login_session = self.auth.logins.from_session()
-        creds_resource = self.auth.credentials["campus"][login_session.user_id]
+        user_id = login_session.user_id
+        if user_id is None:
+            raise errors.AuthenticationError(
+                error_description="Login session has no user id."
+            )
+        creds_resource = self.auth.credentials["campus"][user_id]
         user_creds = creds_resource.get()
         if (
                 force_refresh
@@ -200,9 +205,7 @@ def _get_token_from_session(
                 grant_type="refresh_token",
                 refresh_token=user_creds.token.refresh_token
             )
-            self.auth.credentials["campus"][login_session.user_id].update(
-                token=token
-            )
+            self.auth.credentials["campus"][user_id].update(token=token)
             # Return the refreshed token — user_creds still holds the
             # pre-refresh (now rotated) access token.
             return token
@@ -307,8 +310,11 @@ def refresh_bearer() -> str | None:
                 return refreshed.access_token
 
             for client in (self.api.client, self.auth.client):
-                client.set_unauthorized_hook(refresh_bearer)
-                hooked_clients.append(client)
+                # The 401 hook is an optional JsonClient capability
+                # (see json_client.interface) — feature-detect it.
+                if isinstance(client, CampusRequest):
+                    client.set_unauthorized_hook(refresh_bearer)
+                    hooked_clients.append(client)
 
         try:
             yield self

campus_python/api/v1/assignments.py

@@ -45,7 +45,7 @@ def new(
             questions: "list[dict] | None" = None,
             classroom_links: "list[dict] | None" = None,
     ) -> campus.model.Assignment:
-        payload = {"title": title}
+        payload: "dict[str, object]" = {"title": title}
         if description is not None:
             payload["description"] = description
         if questions is not None:

campus_python/audit/ratelimit.py

@@ -200,10 +200,12 @@ def _bucket_from_body(body: typing.Any) -> str | None:
 def _retry_after_seconds(headers: typing.Mapping[str, str] | None) -> float:
     """Parse Retry-After from response headers, defaulting to the floor."""
     if headers:
-        try:
-            return float(headers.get("Retry-After"))
-        except (TypeError, ValueError):
-            pass
+        retry_after = headers.get("Retry-After")
+        if retry_after is not None:
+            try:
+                return float(retry_after)
+            except ValueError:
+                pass
     return BACKOFF_FLOOR_SECONDS
 
 

campus_python/auth/v1/__init__.py

@@ -332,6 +332,10 @@ def get_token(self) -> campus.model.OAuthToken:
             )
         login_session = self.logins.from_session()
         user_id = login_session.user_id
+        if user_id is None:
+            raise errors.AuthenticationError(
+                error_description="Login session has no user id."
+            )
 
         # Retrieve credentials for the user
         credentials = self.credentials["campus"][user_id].get()
@@ -432,15 +436,16 @@ def push_context(self) -> None:
             try:
                 login_session = self.logins.from_session()
                 # Embedded login user (#879) with the same fallback.
-                user = (
-                    getattr(login_session, "user", None)
-                    or self.users[login_session.user_id].get()
-                )
+                login_user_id = login_session.user_id
+                user = getattr(login_session, "user", None)
+                if user is None and login_user_id is not None:
+                    user = self.users[login_user_id].get()
                 flask.g.user = user
                 flask.g.device = login_session.device_id
-                self._user_cache_put(
-                    "login", login_id, user, login_session.device_id
-                )
+                if user is not None:
+                    self._user_cache_put(
+                        "login", login_id, user, login_session.device_id
+                    )
             except errors.NotFoundError:
                 # Login session no longer exists on server
                 self._user_cache_pop("login", login_id)

campus_python/auth/v1/oauth.py

@@ -169,7 +169,7 @@ def poll_for_token(
             raise errors.AuthenticationError(
                 status_code=400,
                 error_description=(
-                    descriptions.get(oauth_error)
+                    descriptions.get(oauth_error or "")
                     or message
                     or "Unknown error"
                 ),

campus_python/json_client/__init__.py

@@ -15,14 +15,7 @@
 
 import requests
 from campus.common import env
-
-try:
-    from campus.webauth.models import HttpHeader
-except ImportError:
-    # campus-suite predating campus#857 keeps header models in
-    # campus.model; this fallback lets this client work against both
-    # the old and the new campus-suite until locks are re-resolved.
-    from campus.model import HttpHeader
+from campus.webauth.models import HttpHeader
 
 from .. import errors
 from .interface import JsonClient, JsonDict, JsonResponse

pyproject.toml

@@ -15,6 +15,7 @@ campus-suite = {git = "https://github.com/nyjc-computing/campus.git", branch = "
 
 [tool.poetry.group.dev.dependencies]
 pytest = "^8.4.2"
+pyright = "^1.1.400"
 
 [[tool.poetry.packages]]
 include = "campus_python"

tests/unit/test_audit.py

@@ -11,10 +11,12 @@
 
 import os
 import unittest
+from typing import cast
 from unittest.mock import Mock, patch
 
 from campus_python import Campus
 from campus_python.audit.v1 import AuditRoot
+from campus_python.json_client import CampusRequest
 
 API_KEY_RESOURCE = {
     "id": "akm123",
@@ -187,7 +189,7 @@ def test_audit_root_uses_audit_url_and_key(self):
             audit = campus.audit
         self.assertEqual(audit.base_url, "https://audit.example.test")
         self.assertEqual(
-            audit.client._session.headers["Authorization"],
+            cast(CampusRequest, audit.client)._session.headers["Authorization"],
             "Bearer audit_v1_testkey",
         )
         self.assertEqual(audit.url_prefix, "/audit/v1")

tests/unit/test_campus_request.py

@@ -65,6 +65,7 @@ def test_server_mode_with_credentials(self):
         # Verify Basic auth header was set
         auth_header = client._session.headers.get('Authorization')
         self.assertIsNotNone(auth_header)
+        assert auth_header is not None
         self.assertTrue(auth_header.startswith('Basic '))
 
     def test_default_mode_is_server(self):
@@ -101,6 +102,7 @@ def test_bearer_authorization_overrides_basic(self):
 
         # Verify Basic auth was set initially
         auth_header = client._session.headers.get('Authorization')
+        assert auth_header is not None
         self.assertTrue(auth_header.startswith('Basic '))
 
         # Set Bearer authorization (should override)

tests/unit/test_embedded_session_user.py

@@ -10,6 +10,7 @@
 import os
 import unittest
 from types import SimpleNamespace
+from typing import Any
 from unittest.mock import MagicMock, Mock, patch
 
 import campus.model
@@ -68,7 +69,7 @@ class TestFinalizeUsesEmbed(unittest.TestCase):
     """finalize() answers step 4 from the embed when present."""
 
     def setUp(self):
-        self.auth, _ = make_auth()
+        self.auth, self.client_mock = make_auth()
 
     def run_finalize(self, finalize_return: tuple) -> None:
         sessions = MagicMock()
@@ -83,11 +84,12 @@ def run_finalize(self, finalize_return: tuple) -> None:
         users = MagicMock()
         users["user-1"].get.return_value = EMBEDDED_USER
         self.auth._users = users
+        self.users_mock = users
         logins = MagicMock()
         self.auth._logins = logins
         token_response = Mock()
         token_response.json.return_value = TOKEN_RESOURCE
-        self.auth.client.post.return_value = token_response
+        self.client_mock.post.return_value = token_response
 
         app = flask.Flask(__name__)
         app.secret_key = "test-secret"
@@ -101,12 +103,12 @@ def run_finalize(self, finalize_return: tuple) -> None:
     def test_embedded_user_skips_users_route(self):
         """The embed answers the user-exists check without users.get."""
         self.run_finalize(("https://app.example.org/landing", EMBEDDED_USER))
-        self.assertFalse(self.auth._users["user-1"].get.called)
+        self.assertFalse(self.users_mock["user-1"].get.called)
 
     def test_no_embed_falls_back_to_users_route(self):
         """Pre-#879 services return no embed; users.get stays the check."""
         self.run_finalize(("https://app.example.org/landing", None))
-        self.assertTrue(self.auth._users["user-1"].get.called)
+        self.assertTrue(self.users_mock["user-1"].get.called)
 
 
 class TestPushContextUsesEmbed(unittest.TestCase):
@@ -115,7 +117,7 @@ class TestPushContextUsesEmbed(unittest.TestCase):
     def setUp(self):
         self.auth, _ = make_auth()
 
-    def push_session(self, session_obj) -> object:
+    def push_session(self, session_obj) -> Any:
         sessions = MagicMock()
         sessions._session_key = "campus_session_id"
         sessions.has_session.return_value = True
@@ -124,12 +126,13 @@ def push_session(self, session_obj) -> object:
         users = MagicMock()
         users["user-1"].get.return_value = EMBEDDED_USER
         self.auth._users = users
+        self.users_mock = users
         with session_context():
             flask.session["campus_session_id"] = SESSION_ID
             self.auth.push_context()
             return flask.g.user
 
-    def push_login(self, login_obj) -> object:
+    def push_login(self, login_obj) -> Any:
         logins = MagicMock()
         logins._session_key = "logins_login_id"
         logins.has_session.return_value = True
@@ -138,6 +141,7 @@ def push_login(self, login_obj) -> object:
         users = MagicMock()
         users["user-1"].get.return_value = EMBEDDED_USER
         self.auth._users = users
+        self.users_mock = users
         with session_context():
             flask.session["logins_login_id"] = "login-1"
             self.auth.push_context()
@@ -147,13 +151,13 @@ def test_session_embed_skips_users_route(self):
         session = SimpleNamespace(user_id="user-1", user=EMBEDDED_USER)
         user = self.push_session(session)
         self.assertEqual(user.email, USER_RESOURCE["email"])
-        self.assertFalse(self.auth._users["user-1"].get.called)
+        self.assertFalse(self.users_mock["user-1"].get.called)
 
     def test_session_without_embed_falls_back(self):
         session = SimpleNamespace(user_id="user-1", user=None)
         user = self.push_session(session)
         self.assertEqual(user.email, USER_RESOURCE["email"])
-        self.assertTrue(self.auth._users["user-1"].get.called)
+        self.assertTrue(self.users_mock["user-1"].get.called)
 
     def test_model_without_user_field_falls_back(self):
         """An installed campus-suite predating the field still resolves."""
@@ -166,21 +170,21 @@ def test_model_without_user_field_falls_back(self):
         )
         user = self.push_session(session)
         self.assertEqual(user.email, USER_RESOURCE["email"])
-        self.assertTrue(self.auth._users["user-1"].get.called)
+        self.assertTrue(self.users_mock["user-1"].get.called)
 
     def test_login_embed_skips_users_route(self):
         login = SimpleNamespace(
             user_id="user-1", user=EMBEDDED_USER, device_id="device-1"
         )
         user = self.push_login(login)
         self.assertEqual(user.email, USER_RESOURCE["email"])
-        self.assertFalse(self.auth._users["user-1"].get.called)
+        self.assertFalse(self.users_mock["user-1"].get.called)
 
     def test_login_without_embed_falls_back(self):
         login = SimpleNamespace(user_id="user-1", user=None, device_id=None)
         user = self.push_login(login)
         self.assertEqual(user.email, USER_RESOURCE["email"])
-        self.assertTrue(self.auth._users["user-1"].get.called)
+        self.assertTrue(self.users_mock["user-1"].get.called)
 
 
 if __name__ == "__main__":