Enable more ruff rules (#34)

This PR, as the name suggests, enables more ruff rules and fixes the
code accordingly.

---

* This PR closes #28

README.md

@@ -3,23 +3,24 @@
 The `secret-santa` package is a _Secret Santa_ 🎅🎄 game which randomly assigns and notifies people to whom they should give a gift using _Twilio_'s messaging API.
 
 [![Test Workflow on Main](https://github.com/FawziAS/secret-santa/actions/workflows/test.yml/badge.svg?branch=main)](https://github.com/FawziAS/secret-santa/actions/workflows/test.yml)
-[![Code style: black](https://img.shields.io/badge/code%20style-black-000000.svg)](https://github.com/psf/black)
+[![Ruff](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/astral-sh/ruff/main/assets/badge/format.json)](https://github.com/astral-sh/ruff)
 
 <details>
   <summary>Table of Contents</summary>
 
-  1. [Why?](#why)
-     1. [But WHY is it over-engineered?](#but-why-is-it-over-engineered)
-  2. [Usage](#usage)
-     1. [Prerequisites](#prerequisites)
-     2. [Configs and Environment Needed](#configs-and-environment-needed)
-     3. [Installing the Dependencies](#installing-the-dependencies)
-     4. [Running](#running)
-  3. [Future Plans](#future-plans)
-  4. [Contributing](#contributing)
-     1. [So... How can I contribute?](#so-how-can-i-contribute)
-  5. [License](#license)
-  6. [Who](#who)
+- [Secret Santa](#secret-santa)
+  - [Why?](#why)
+    - [But WHY is it over-engineered?](#but-why-is-it-over-engineered)
+  - [Usage](#usage)
+    - [Prerequisites](#prerequisites)
+    - [Configs and Environment Needed](#configs-and-environment-needed)
+    - [Installing the Dependencies](#installing-the-dependencies)
+    - [Running](#running)
+  - [Future Plans](#future-plans)
+  - [Contributing](#contributing)
+    - [So... How can I contribute?](#so-how-can-i-contribute)
+  - [License](#license)
+  - [Who](#who)
 </details>
 
 ## Why?

pyproject.toml

@@ -43,14 +43,34 @@ select = [
     "I",
     # pydocstyle
     "D",
+    # mccabe
+    "C90",
+    # pep8-naming
+    "N",
+    # pyupgrade
+    "UP",
+    # ruff
+    "RUF",
+    # Perflint
+    "PERF",
+    # tryceratops
+    "TRY",
+    # flynt
+    "FLY",
+    # pylint
+    "PL",
+    # pyflakes
+    "YTT", "ANN", "ASYNC", "BLE", "B", "A", "C4", "DTZ", "T10", "EM", "EXE", "ICN", "PTH", "ARG", "TCH", "TID", 
+    "SIM", "SLOT", "SLF", "RET", "RSE", "Q", "PT", "PYI", "T20", "PIE", "INP", "G", # TODO: "S", "FBT", "FA",
 ]
+ignore = ["ANN101", "G004"]
 # Allow fix for all enabled rules (when `--fix`) is provided.
 fixable = ["ALL"]
 unfixable = []
 
 
 [tool.ruff.lint.per-file-ignores]
-"**/{tests}/*.py" = ["D"]
+"**/{tests}/*.py" = ["D", "S"]
 
 
 [tool.ruff.lint.pydocstyle]

secret_santa/const.py

@@ -3,3 +3,5 @@
 TWILIO_ACCOUNT_SID = "TWILIO_ACCOUNT_SID"
 TWILIO_AUTH_TOKEN = "TWILIO_AUTH_TOKEN"
 TWILIO_NUMBER = "TWILIO_NUMBER"
+
+MINIMUM_NUMBER_OF_PARTICIPANTS = 3

secret_santa/model/participant.py

@@ -1,5 +1,7 @@
 """Participant model."""
 
+from typing import Self
+
 from attr import dataclass
 
 
@@ -19,7 +21,7 @@ class Participant:
     nickname: str | None = None
 
     # TODO: Replace `ignore` with Self from typing once Python 3.11 is supported (and the type checker supports it).
-    def __eq__(self, other) -> bool:  # type: ignore
+    def __eq__(self, other: Self) -> bool:  # type: ignore
         """Custom equals method for this data class to check if two participants are identical.
 
         Args:

secret_santa/secret_santa_module.py

@@ -13,7 +13,7 @@
 import pyfiglet
 from dotenv import load_dotenv
 
-from secret_santa.const import TWILIO_ACCOUNT_SID, TWILIO_AUTH_TOKEN, TWILIO_NUMBER
+from secret_santa.const import MINIMUM_NUMBER_OF_PARTICIPANTS, TWILIO_ACCOUNT_SID, TWILIO_AUTH_TOKEN, TWILIO_NUMBER
 from secret_santa.model.participant import Participant
 from secret_santa.twilio_messaging_service import TwilioMessagingService
 from secret_santa.util import arg_parser, file, misc, path
@@ -39,8 +39,8 @@ class SecretSanta:
     def __init__(
         self,
         participants_json_path: Optional[PathLike] = None,
-        show_arrangement: bool = False,
         *,
+        show_arrangement: bool = False,
         dry_run: bool,
     ) -> None:
         """Initialize the Secret Santa game class.
@@ -63,7 +63,7 @@ def __init__(
             participants_json_path = path.get_project_root() / "participants.json"
             self.logger.debug("Attempting to look for a participants file at root level")
         assert Path(
-            participants_json_path
+            participants_json_path,
         ).exists(), f"Could not find the participants JSON file @ {participants_json_path}"
 
         self.logger.debug("Loading the participants")
@@ -93,7 +93,7 @@ def load_participants(self, participants_json_path: PathLike) -> list[Participan
         participants_json: str = file.read_file(Path(participants_json_path))
         # Convert the JSON string to a list of dicts
         participants_dict_list = json.loads(participants_json)
-        assert len(participants_dict_list) > 2, (
+        assert len(participants_dict_list) >= MINIMUM_NUMBER_OF_PARTICIPANTS, (
             f"Secret Santa should have at least 3 participants. "
             f"Current number of participants: {len(participants_dict_list)}"
         )
@@ -151,9 +151,7 @@ def get_secret_santa_message(participant: Participant, recipient: Participant) -
         participant_msg_name = SecretSanta.get_participant_message_name(participant)
         # Recipient name to use
         recipient_msg_name = SecretSanta.get_participant_message_name(recipient)
-        # Construct message
-        message_body = f"Hello {participant_msg_name},\n" f"You'll be {recipient_msg_name}'s Secret Santa!"
-        return message_body
+        return f"Hello {participant_msg_name},\nYou'll be {recipient_msg_name}'s Secret Santa!"
 
     def run(self) -> int:
         """Find a recipient for each participant and send the participant a message.
@@ -174,14 +172,14 @@ def run(self) -> int:
             if self.show_arrangement:
                 self.logger.info(
                     f"{SecretSanta.get_participant_message_name(participant)} -> "
-                    f"{SecretSanta.get_participant_message_name(recipient)}"
+                    f"{SecretSanta.get_participant_message_name(recipient)}",
                 )
             response = self.messaging_client.send_message(
                 SecretSanta.get_secret_santa_message(participant, recipient),
                 participant.phone_number,
                 dry_run=self.dry_run,
             )
-            logger.info(f"Message sent to: {participant}, " f"Status: {response.status}")
+            logger.info(f"Message sent to: {participant}, Status: {response.status}")
         return 0
 
 
@@ -209,15 +207,17 @@ def load_env(dotenv_path: Optional[PathLike] = None, override_system: bool = Fal
             logger.warning(f"No .env file could be found at: {dotenv_path}")
             # load_dotenv won't fail in case the file does not exist and setting it to an empty
             # string would prevent it from looking for a `.env` file.
-            dotenv_path = Path("")
+            dotenv_path = Path()
 
     load_dotenv(dotenv_path=dotenv_path, override=override_system)
 
     # Make sure the needed Twilio configuration environment variables are provided
     logger.debug("Asserting Twilio configuration has been provided")
     required_env_vars = [TWILIO_ACCOUNT_SID, TWILIO_AUTH_TOKEN, TWILIO_NUMBER]
-    if not all([os.getenv(env) for env in required_env_vars]):
-        raise SystemExit(f"One or more of the environment variables needed ({required_env_vars}) has not been passed.")
+    if not all(os.getenv(env) for env in required_env_vars):
+        # TODO: Add custom errors, error strings, and exceptions
+        environment_err = f"One or more of the environment variables needed ({required_env_vars}) has not been passed."
+        raise SystemExit(environment_err)
     logger.info("Environment loaded successfully")
 
 
@@ -229,7 +229,7 @@ def main() -> int:
 
     """
     secret_santa_figlet = pyfiglet.figlet_format("Secret  Santa")
-    print(secret_santa_figlet)
+    print(secret_santa_figlet)  # noqa: T201
     time.sleep(0.5)
     # Parse the provided arguments
     parser = arg_parser.get_secret_santa_argument_parser()

secret_santa/twilio_messaging_service.py

@@ -66,7 +66,7 @@ def __init__(self, alphanumeric_id: Optional[str] = None) -> None:
         if alphanumeric_id:
             # Assert at least one character exists
             assert bool(
-                re.search("[a-zA-Z]", alphanumeric_id)
+                re.search("[a-zA-Z]", alphanumeric_id),
             ), "The alphanumeric sender Id needs to contain at least one character"
             # Assert the alphanumeric sender ID is up to Twilio's requirements
             assert bool(re.search(r"^[a-zA-Z0-9 ]{1,11}$", alphanumeric_id)), (

secret_santa/util/arg_parser.py

@@ -30,7 +30,7 @@ def _split_lines(self, text: str, width: int) -> list[str]:
                     lines.extend(textwrap.wrap(line, width))
             return lines
             # This is the RawTextHelpFormatter._split_lines
-        return argparse.HelpFormatter._split_lines(self, text, width)
+        return argparse.HelpFormatter._split_lines(self, text, width)  # noqa: SLF001
 
 
 def get_secret_santa_argument_parser() -> argparse.ArgumentParser:

secret_santa/util/file.py

@@ -1,6 +1,7 @@
 """File utilities."""
 
 from os import PathLike
+from pathlib import Path
 
 
 def create_file(full_file_name: PathLike, content: str) -> None:
@@ -11,8 +12,7 @@ def create_file(full_file_name: PathLike, content: str) -> None:
         content: The content to write to the file.
 
     """
-    with open(full_file_name, "w", encoding="utf8") as new_file:
-        new_file.write(content)
+    Path(full_file_name).write_text(content, encoding="utf8")
 
 
 def read_file(file_name: PathLike) -> str:
@@ -26,6 +26,4 @@ def read_file(file_name: PathLike) -> str:
             as a list of strings read line by line in case ``read_line_by_line`` is True.
 
     """
-    with open(file_name) as f:
-        content = f.read()
-    return content
+    return Path(file_name).read_text(encoding="utf8")

secret_santa/util/logging.py

@@ -22,7 +22,7 @@ def format(self, record: logging.LogRecord) -> str:
         location = f"{record.module}.{record.name}().{record.funcName}"
         msg = f'{f"[{record.levelname}]".ljust(10)} {location.ljust(39)} : {record.message}'
         record.msg = msg
-        return super(CustomLogFormatter, self).format(record)
+        return super().format(record)
 
 
 def get_common_handler() -> logging.Handler:

secret_santa/util/misc.py

@@ -21,19 +21,17 @@ def is_derangement(list_a: list[T], list_b: list[T]) -> bool:
 
     """
     # Assert both of the lists have at least one element
-    assert len(list_a) > 0 and len(list_b) > 0, (
-        f"The two lists must contain at least one element to qualify for a derangement check: "
-        f"len(list_a)={len(list_a)}, len(list_b)={len(list_b)}"
-    )
+    assert len(list_a) > 0, f"The list must not be empty to qualify for a derangement check: {len(list_a)=}"
+    assert len(list_b) > 0, f"The list must not be empty to qualify for a derangement check: {len(list_b)=}"
     # Assert the two lists are of the same length
-    assert len(list_a) == len(list_b), (
-        f"The two lists must be of the same size to qualify for a derangement check: " f"{len(list_a)} != {len(list_b)}"
-    )
+    assert len(list_a) == len(
+        list_b
+    ), f"The two lists must be of the same size to qualify for a derangement check: {len(list_a)} != {len(list_b)}"
     # Assert the two lists contain items of the same type
-    assert not any([True if type(x) != type(y) else False for x, y in zip(list_a, list_b)]), (  # noqa: E721
+    assert not any(type(x) != type(y) for x, y in zip(list_a, list_b)), (  # noqa: E721
         f"The two lists' elements must be of the same type to qualify for a derangement check: "
         f"{type(list_a[0])} != {type(list_b[0])}"
     )
 
     # Each two parallel items should be different
-    return all([True if x != y else False for x, y in zip(list_a, list_b)])
+    return all(x != y for x, y in zip(list_a, list_b))

tests/test_secret_santa.py

@@ -1,11 +1,10 @@
 import itertools
+import json
 import os
 from argparse import Namespace
 from pathlib import Path
-from typing import Generator, Iterator
 
 import pytest
-from _pytest.capture import CaptureFixture
 from _pytest.monkeypatch import MonkeyPatch
 from pytest_lazy_fixtures import lf
 from pytest_mock import MockerFixture
@@ -20,7 +19,7 @@
 
 # TODO: Remove in refactor and get rid of env load as this has some unintended side effects
 @pytest.fixture(autouse=True)
-def clear_environment(mocker: MockerFixture) -> Iterator:
+def _clear_environment(mocker: MockerFixture) -> None:
     def is_github_environment_variable(name: str) -> bool:
         return any(name.startswith(gh_env_prefix) for gh_env_prefix in ["GITHUB_", "RUNNER_"])
 
@@ -30,7 +29,6 @@ def is_github_environment_variable(name: str) -> bool:
     #   Until this is taken care of properly - the GitHub actions environment variables should not be cleared if exists.
     github_env_variables = {key: val for key, val in os.environ.items() if is_github_environment_variable(key)}
     mocker.patch.dict(os.environ, {**github_env_variables}, clear=True)
-    yield
 
 
 @pytest.fixture()
@@ -206,7 +204,6 @@ def test_load_env_without_env_file_with_predefined_env(
 def test_module_main(
     mocker: MockerFixture,
     monkeypatch: MonkeyPatch,
-    capsys: Generator[CaptureFixture[str], None, None],
     test_participants_file_path: Path,
     dry_run: bool,
     show_arrangement: bool,
@@ -226,6 +223,8 @@ def test_module_main(
             show_arrangement=show_arrangement,
         ),
     )
+
+    num_of_participants = len(json.loads(test_participants_file_path.read_text("utf-8")))
     # TODO: Capture the output and check the "->" of the show arrangement appears in the log
     #  in case it is True / if "Dry run" appears in the log in case it is a dry run.
     assert (
@@ -235,8 +234,8 @@ def test_module_main(
         create_message_mock.assert_not_called()
     else:
         assert (
-            create_message_mock.call_count == 3
-        ), "The `create` method of the Twilio API was not called the number of times expected 3."
+            create_message_mock.call_count == num_of_participants
+        ), f"The `create` method of the Twilio API was not called the number of times expected {num_of_participants}."
 
 
 @pytest.mark.parametrize(
@@ -335,9 +334,8 @@ def test_get_participant_message_name(
 
 @pytest.mark.parametrize("execution_number", range(9))
 def test_participants_derangement(
-    execution_number: int,
+    execution_number: int,  # noqa: ARG001
     default_secret_santa_instance: SecretSanta,
-    participants_in_participants_file: list[Participant],
 ) -> None:
     participants_derangement = default_secret_santa_instance.get_participants_derangement()
     assert misc.is_derangement(

tests/test_twilio_messaging_service.py

@@ -70,7 +70,7 @@ def test_invalid_config(
     with pytest.raises(AssertionError) as exception_info:
         TwilioMessagingService()
     assert "Required environment variables TWILIO_ACCOUNT_SID or TWILIO_AUTH_TOKEN or TWILIO_NUMBER missing." in str(
-        exception_info.value
+        exception_info.value,
     ), "The assertion raised does not match the assertion expected."
 
 
@@ -108,7 +108,7 @@ def test_invalid_twilio_messaging_service_alphanumeric_id(
     with pytest.raises(AssertionError) as exception_info:
         twilio_messaging_service_builder(alphanumeric_id)
     assert "The alphanumeric sender Id can only contain up to 11 characters from the following categories:" in str(
-        exception_info.value
+        exception_info.value,
     ), "The assertion raised does not match the assertion expected."
 
 
@@ -136,7 +136,7 @@ def test_send_message(
 
     if dry_run:
         assert response == MessageResponse(
-            status="Not executed (DRY RUN)"
+            status="Not executed (DRY RUN)",
         ), "The response returned does not match the response expected."
     else:
         create_message_mock.assert_called_once_with(

tests/util/test_file.py

@@ -1,4 +1,3 @@
-import os
 from pathlib import Path
 
 import pytest
@@ -27,11 +26,11 @@ def test_create_file(tests_temp_directory: Path) -> None:
     new_file_content: str = "This is a simple text file creation to test create_file()"
     try:
         file.create_file(full_file_name=new_file_path, content=new_file_content)
-        assert os.path.exists(new_file_path), f"A new txt file ({new_file_path}) should've been created"
+        assert new_file_path.exists(), f"A new txt file ({new_file_path}) should've been created"
         read_content: str = file.read_file(new_file_path)
         assert (
             read_content == new_file_content
         ), f"The file {new_file_path} created did not contain the intended content."
     finally:
-        if os.path.exists(new_file_path):
-            os.remove(new_file_path)
+        if new_file_path.exists():
+            new_file_path.unlink()