code-quality-check

Code Quality Checks for Breenix

Pre-commit code quality verification for Breenix kernel development.

Purpose

Breenix enforces strict code quality standards. This skill provides the checks and fixes required before committing code.

Core Quality Standards (from CLAUDE.md)

1. Fix ALL compiler warnings before committing
2. Fix ALL clippy warnings
3. Use proper patterns (e.g., Once) to avoid unsafe warnings
4. Only #[allow(dead_code)] for legitimate API functions

Pre-Commit Checklist

Before every commit:

# 1. Build kernel and check for warnings
cd kernel
cargo build --target x86_64-unknown-none 2>&1 | grep warning

# 2. Run clippy
cargo clippy --target x86_64-unknown-none

# 3. Run tests (if modifying core subsystems)
cd ..
cargo test

# 4. Check for log side-effects (manual)
grep -R "log::trace!.*(" kernel/src/ | grep -vE '\".*\"' | grep -vE '\.(as_|to_|into_|len|is_|get)'

Clippy Configuration

Project-Specific Clippy Flags

cd kernel
RUSTFLAGS="-Aclippy::redundant_closure_for_method_calls" \
cargo clippy --target x86_64-unknown-none \
  -- -Dclippy::debug_assert_with_mut_call \
     -Dclippy::print_stdout \
     -Wclippy::suspicious_operation_groupings

What These Check

• debug_assert_with_mut_call: Prevent side-effects in debug assertions
• print_stdout: No print!/println! in kernel (use log! macros)
• suspicious_operation_groupings: Catch likely logic errors

Common Issues and Fixes

Compiler Warnings

Unused imports:

// BAD
use x86_64::{VirtAddr, PageTable, PageTableFlags};  // PageTable unused

// GOOD
use x86_64::{VirtAddr, PageTableFlags};

Unused variables:

// BAD
let result = some_function();  // result unused

// GOOD
let _result = some_function();  // Explicitly unused
// OR
some_function();  // Don't capture if not needed

Dead code:

// BAD - function never called
fn helper_function() { ... }

// GOOD - remove it
// OR add #[allow(dead_code)] if it's part of a public API

// GOOD - legitimate API function
#[allow(dead_code)]  // Part of public allocator API
pub fn dealloc_stack(&mut self, stack_id: usize) { ... }

Clippy Warnings

Redundant closure:

// BAD
items.map(|x| x.to_string())

// GOOD (but we allow this via RUSTFLAGS)
items.map(ToString::to_string)

Debug assert with mutation:

// BAD - side effect in assertion
debug_assert!(list.pop().is_some());

// GOOD - separate the effect
let item = list.pop();
debug_assert!(item.is_some());

Log Side-Effects

Problem: Function calls in log statements execute even when logging disabled.

// BAD - get_state() called even if TRACE disabled
log::trace!("State: {:?}", get_state());

// GOOD - only format if needed
let state = get_state();
log::trace!("State: {:?}", state);

// BETTER - for expensive operations
if log::log_enabled!(log::Level::Trace) {
    let state = expensive_get_state();
    log::trace!("State: {:?}", state);
}

CI Code Quality Workflow

The .github/workflows/code-quality.yml runs these checks automatically:

1. Clippy checks with project-specific flags
2. Log side-effects scan for trace statements with function calls
3. Complex log expression check for multi-argument format strings
4. Log level regression guard - ensures feature flags control log level

Quick Reference

Before Commit

# Full quality check
cd kernel
cargo build --target x86_64-unknown-none 2>&1 | tee /tmp/build-warnings.txt
cargo clippy --target x86_64-unknown-none 2>&1 | tee /tmp/clippy-warnings.txt

# Review warnings
grep warning /tmp/build-warnings.txt
less /tmp/clippy-warnings.txt

# Fix all warnings before committing!

Common Warning Fixes

Integration with Git Workflow

# Before committing
git status  # See what you're about to commit
cd kernel
cargo clippy --target x86_64-unknown-none  # Fix all warnings

# Then commit
git add kernel/src/...
git commit -m "Fix: ..."

Best Practices

1. Fix warnings as you go: Don't accumulate them
2. Run clippy frequently: Catch issues early
3. Use proper logging: log! macros, not print!
4. Avoid side-effects in logs: Especially in trace/debug
5. Comment allowed dead code: Explain why it's part of the API
6. Use feature flags: Control debug vs release behavior
7. Test before committing: cargo test if touching core code

Summary

Code quality standards enforce:

• Zero compiler warnings
• Zero clippy warnings
• No side-effects in log statements
• Appropriate use of #[allow] attributes
• Proper logging practices

Run checks before every commit to maintain high code quality.