docs: Add code style guide

CONTRIBUTING.md

@@ -1,27 +1,15 @@
 # Contributing guide
 
 This page is about contributing to Continuwuity. The
-[development](./development.md) page may be of interest for you as well.
+[development](./development.md) and [code style guide](./development/code_style.md) pages may be of interest for you as well.
 
 If you would like to work on an [issue][issues] that is not assigned, preferably
 ask in the Matrix room first at [#continuwuity:continuwuity.org][continuwuity-matrix],
 and comment on it.
 
-### Linting and Formatting
+### Code Style
 
-It is mandatory all your changes satisfy the lints (clippy, rustc, rustdoc, etc)
+Please review and follow the [code style guide](./development/code_style.md) for formatting, linting, naming conventions, and other code standards.
-and your code is formatted via the **nightly** rustfmt (`cargo +nightly fmt`). A lot of the
-`rustfmt.toml` features depend on nightly toolchain. It would be ideal if they
-weren't nightly-exclusive features, but they currently still are. CI's rustfmt
-uses nightly.
-
-If you need to allow a lint, please make sure it's either obvious as to why
-(e.g. clippy saying redundant clone but it's actually required) or it has a
-comment saying why. Do not write inefficient code for the sake of satisfying
-lints. If a lint is wrong and provides a more inefficient solution or
-suggestion, allow the lint and mention that in a comment.
-
-If there is a large formatting change across unrelated files, make a separate commit so that it can be added to the `.git-blame-ignore-revs` file.
 
 ### Pre-commit Checks
 
@@ -66,7 +54,7 @@ pre-commit install
 pre-commit run --all-files
 ```
 
-These same checks are run in CI via the prefligit-checks workflow to ensure consistency.
+These same checks are run in CI via the prefligit-checks workflow to ensure consistency. These must pass before the PR is merged.
 
 ### Running tests locally
 
@@ -115,37 +103,13 @@ To build the documentation locally:
 
 The output of the mdbook generation is in `public/`. You can open the HTML files directly in your browser without needing a web server.
 
-### Inclusivity and Diversity
-
-All **MUST** code and write with inclusivity and diversity in mind. See the
-[following page by Google on writing inclusive code and
-documentation](https://developers.google.com/style/inclusive-documentation).
-
-This **EXPLICITLY** forbids usage of terms like "blacklist"/"whitelist" and
-"master"/"slave", [forbids gender-specific words and
-phrases](https://developers.google.com/style/pronouns#gender-neutral-pronouns),
-forbids ableist language like "sanity-check", "cripple", or "insane", and
-forbids culture-specific language (e.g. US-only holidays or cultures).
-
-No exceptions are allowed. Dependencies that may use these terms are allowed but
-[do not replicate the name in your functions or
-variables](https://developers.google.com/style/inclusive-documentation#write-around).
-
-In addition to language, write and code with the user experience in mind. This
-is software that intends to be used by everyone, so make it easy and comfortable
-for everyone to use. 🏳️‍⚧️
-
-### Variable, comment, function, etc standards
-
-Rust's default style and standards with regards to [function names, variable
-names, comments](https://rust-lang.github.io/api-guidelines/naming.html), etc
-applies here.
 
 ### Commit Messages
 
 Continuwuity follows the [Conventional Commits](https://www.conventionalcommits.org/) specification for commit messages. This provides a standardized format that makes the commit history more readable and enables automated tools to generate changelogs.
 
 The basic structure is:
+
 ```
 <type>[(optional scope)]: <description>
 
@@ -186,11 +150,10 @@ of it, especially when the CI completed successfully and everything so it
 
 Before submitting a pull request, please ensure:
 1. Your code passes all CI checks (formatting, linting, typo detection, etc.)
-2. Your commit messages follow the conventional commits format
+2. Your code follows the [code style guide](./development/code_style.md)
-3. Tests are added for new functionality
+3. Your commit messages follow the conventional commits format
-4. Documentation is updated if needed
+4. Tests are added for new functionality
-
+5. Documentation is updated if needed
-
 
 Direct all PRs/MRs to the `main` branch.
 

docs/SUMMARY.md

@@ -17,6 +17,7 @@
 - [Troubleshooting](troubleshooting.md)
 - [Development](development.md)
   - [Contributing](contributing.md)
+  - [Code Style Guide](development/code_style.md)
   - [Testing](development/testing.md)
   - [Hot Reloading ("Live" Development)](development/hot_reload.md)
 - [Community (and Guidelines)](community.md)

docs/development.md

@@ -2,7 +2,7 @@
 
 Information about developing the project. If you are only interested in using
 it, you can safely ignore this page. If you plan on contributing, see the
-[contributor's guide](./contributing.md).
+[contributor's guide](./contributing.md) and [code style guide](./development/code_style.md).
 
 ## Continuwuity project layout
 

docs/development/code_style.md

@@ -0,0 +1,331 @@
+# Code Style Guide
+
+This guide outlines the coding standards and best practices for Continuwuity development. These guidelines help avoid bugs and maintain code consistency, readability, and quality across the project.
+
+These guidelines apply to new code on a best-effort basis. When modifying existing code, follow existing patterns in the immediate area you're changing and then gradually improve code style when making substantial changes.
+
+## General Principles
+
+- **Clarity over cleverness**: Write code that is easy to understand and maintain
+- **Consistency**: Pragmatically follow existing patterns in the codebase, rather than adding new dependencies.
+- **Safety**: Prefer safe, explicit code over unsafe code with implicit requirements
+- **Performance**: Consider performance implications, but not at the expense of correctness or maintainability
+
+## Formatting and Linting
+
+All code must satisfy lints (clippy, rustc, rustdoc, etc) and be formatted using **nightly** rustfmt (`cargo +nightly fmt`). Many of the `rustfmt.toml` features depend on the nightly toolchain.
+
+If you need to allow a lint, ensure it's either obvious why (e.g. clippy saying redundant clone but it's actually required) or add a comment explaining the reason. Do not write inefficient code just to satisfy lints. If a lint is wrong and provides a less efficient solution, allow the lint and mention that in a comment.
+
+If making large formatting changes across unrelated files, create a separate commit so it can be added to the `.git-blame-ignore-revs` file.
+
+## Rust-Specific Guidelines
+
+### Naming Conventions
+
+Follow standard Rust naming conventions as outlined in the [Rust API Guidelines](https://rust-lang.github.io/api-guidelines/naming.html):
+
+- Use `snake_case` for functions, variables, and modules
+- Use `PascalCase` for types, traits, and enum variants
+- Use `SCREAMING_SNAKE_CASE` for constants and statics
+- Use descriptive names that clearly indicate purpose
+
+```rs
+// Good
+fn process_user_request(user_id: &UserId) -> Result<Response, Error> { ... }
+
+const MAX_RETRY_ATTEMPTS: usize = 3;
+
+struct UserSession {
+    session_id: String,
+    created_at: SystemTime,
+}
+
+// Avoid
+fn proc_reqw(id: &str) -> Result<Resp, Err> { ... }
+```
+
+### Error Handling
+
+- Use `Result<T, E>` for operations that can fail
+- Prefer specific error types over generic ones
+- Use `?` operator for error propagation
+- Provide meaningful error messages
+- If needed, create or use an error enum.
+
+```rs
+// Good
+fn parse_server_name(input: &str) -> Result<ServerName, InvalidServerNameError> {
+    ServerName::parse(input)
+        .map_err(|_| InvalidServerNameError::new(input))
+}
+
+// Avoid
+fn parse_server_name(input: &str) -> Result<ServerName, Box<dyn Error>> {
+    Ok(ServerName::parse(input).unwrap())
+}
+```
+
+### Option Handling
+
+- Prefer explicit `Option` handling over unwrapping
+- Use combinators like `map`, `and_then`, `unwrap_or_else` when appropriate
+
+```rs
+// Good
+let display_name = user.display_name
+    .as_ref()
+    .map(|name| name.trim())
+    .filter(|name| !name.is_empty())
+    .unwrap_or(&user.localpart);
+
+// Avoid
+let display_name = if user.display_name.is_some() {
+    user.display_name.as_ref().unwrap()
+} else {
+    &user.localpart
+};
+```
+
+## Logging Guidelines
+
+### Structured Logging
+
+**Always use structured logging instead of string interpolation.** This improves log parsing, filtering, and observability.
+
+```rs
+// Good - structured parameters
+debug!(
+    room_id = %room_id,
+    user_id = %user_id,
+    event_type = ?event.event_type(),
+    "Processing room event"
+);
+
+info!(
+    server_name = %server_name,
+    response_time_ms = response_time.as_millis(),
+    "Federation request completed successfully"
+);
+
+// Avoid - string interpolation
+debug!("Processing room event for {room_id} from {user_id}");
+info!("Federation request to {server_name} took {response_time:?}");
+```
+
+### Log Levels
+
+Use appropriate log levels:
+
+- `error!`: Unrecoverable errors that affect functionality
+- `warn!`: Potentially problematic situations that don't stop execution
+- `info!`: General information about application flow
+- `debug!`: Detailed information for debugging
+- `trace!`: Very detailed information, typically only useful during development
+
+Keep in mind the frequency that the log will be reached, and the relevancy to a server operator.
+
+```rs
+// Good
+error!(
+    error = %err,
+    room_id = %room_id,
+    "Failed to send event to room"
+);
+
+warn!(
+    server_name = %server_name,
+    attempt = retry_count,
+    "Federation request failed, retrying"
+);
+
+info!(
+    user_id = %user_id,
+    "User registered successfully"
+);
+
+debug!(
+    event_id = %event_id,
+    auth_events = ?auth_event_ids,
+    "Validating event authorization"
+);
+```
+
+### Sensitive Information
+
+Never log sensitive information such as:
+- Access tokens
+- Passwords
+- Private keys
+- Personal user data (unless specifically needed for debugging)
+
+```rs
+// Good
+debug!(
+    user_id = %user_id,
+    session_id = %session_id,
+    "Processing authenticated request"
+);
+
+// Avoid
+debug!(
+    user_id = %user_id,
+    access_token = %access_token,
+    "Processing authenticated request"
+);
+```
+
+## Lock Management
+
+### Explicit Lock Scopes
+
+**Always use closure guards instead of implicitly dropped guards.** This makes lock scopes explicit and helps prevent deadlocks.
+
+Use the `WithLock` trait from `continuwuity-core::utils::with_lock`:
+
+```rs
+use continuwuity_core::utils::with_lock::WithLock;
+
+// Good - explicit closure guard
+shared_data.with_lock(|data| {
+    data.counter += 1;
+    data.last_updated = SystemTime::now();
+    // Lock is explicitly released here
+});
+
+// Avoid - implicit guard
+{
+    let mut data = shared_data.lock().unwrap();
+    data.counter += 1;
+    data.last_updated = SystemTime::now();
+    // Lock released when guard goes out of scope - less explicit
+}
+```
+
+For async contexts, use the async variant:
+
+```rs
+use continuwuity_core::utils::with_lock::WithLockAsync;
+
+// Good - async closure guard
+async_shared_data.with_lock(|data| {
+    data.process_async_update();
+}).await;
+```
+
+### Lock Ordering
+
+When acquiring multiple locks, always acquire them in a consistent order to prevent deadlocks:
+
+```rs
+// Good - consistent ordering (e.g., by memory address or logical hierarchy)
+let locks = [&lock_a, &lock_b, &lock_c];
+locks.sort_by_key(|lock| lock as *const _ as usize);
+
+for lock in locks {
+    lock.with_lock(|data| {
+        // Process data
+    });
+}
+
+// Avoid - inconsistent ordering that can cause deadlocks
+lock_b.with_lock(|data_b| {
+    lock_a.with_lock(|data_a| {
+        // Deadlock risk if another thread acquires in A->B order
+    });
+});
+```
+
+## Documentation
+
+### Code Comments
+
+- Reference related documentation or parts of the specification
+- When a task has multiple ways of being acheved, explain your reasoning for your decision
+- Update comments when code changes
+
+```rs
+/// Processes a federation request with automatic retries and backoff.
+///
+/// Implements exponential backoff to handle temporary
+/// network issues and server overload gracefully.
+pub async fn send_federation_request(
+    destination: &ServerName,
+    request: FederationRequest,
+) -> Result<FederationResponse, FederationError> {
+    // Retry with exponential backoff because federation can be flaky
+    // due to network issues or temporary server overload
+    let mut retry_delay = Duration::from_millis(100);
+
+    for attempt in 1..=MAX_RETRIES {
+        match try_send_request(destination, &request).await {
+            Ok(response) => return Ok(response),
+            Err(err) if err.is_retriable() && attempt < MAX_RETRIES => {
+                warn!(
+                    destination = %destination,
+                    attempt = attempt,
+                    error = %err,
+                    retry_delay_ms = retry_delay.as_millis(),
+                    "Federation request failed, retrying"
+                );
+
+                tokio::time::sleep(retry_delay).await;
+                retry_delay *= 2; // Exponential backoff
+            }
+            Err(err) => return Err(err),
+        }
+    }
+
+    unreachable!("Loop should have returned or failed by now")
+}
+```
+
+### Async Patterns
+
+- Use `async`/`await` appropriately
+- Avoid blocking operations in async contexts
+- Consider using `tokio::task::spawn_blocking` for CPU-intensive work
+
+```rs
+// Good - non-blocking async operation
+pub async fn fetch_user_profile(
+    &self,
+    user_id: &UserId,
+) -> Result<UserProfile, Error> {
+    let profile = self.db
+        .get_user_profile(user_id)
+        .await?;
+
+    Ok(profile)
+}
+
+// Good - CPU-intensive work moved to blocking thread
+pub async fn generate_thumbnail(
+    &self,
+    image_data: Vec<u8>,
+) -> Result<Vec<u8>, Error> {
+    tokio::task::spawn_blocking(move || {
+        image::generate_thumbnail(image_data)
+    })
+    .await
+    .map_err(|_| Error::TaskJoinError)?
+}
+```
+
+## Inclusivity and Diversity Guidelines
+
+All code and documentation must be written with inclusivity and diversity in mind. This ensures our software is welcoming and accessible to all users and contributors. Follow the [Google guide on writing inclusive code and documentation](https://developers.google.com/style/inclusive-documentation) for comprehensive guidance.
+
+The following types of language are explicitly forbidden in all code, comments, documentation, and commit messages:
+
+**Ableist language:** Avoid terms like "sanity check", "crazy", "insane", "cripple", or "blind to". Use alternatives like "validation", "unexpected", "disable", or "unaware of".
+
+**Socially-charged technical terms:** Replace overly divisive terminology with neutral alternatives:
+- "whitelist/blacklist" → "allowlist/denylist" or "permitted/blocked"
+- "master/slave" → "primary/replica", "controller/worker", or "parent/child"
+
+When working with external dependencies that use non-inclusive terminology, avoid propagating them in your own APIs and variable names.
+
+Use diverse examples in documentation that avoid culturally-specific references, assumptions about user demographics, or unnecessarily gendered language. Design with accessibility and inclusivity in mind by providing clear error messages and considering diverse user needs.
+
+This software is intended to be used by everyone regardless of background, identity, or ability. Write code and documentation that reflects this commitment to inclusivity.