Clean Up Compiler and Linter Warnings

Objective

Eliminate all compiler warnings, clippy lints, and formatting issues across the pg_durable codebase without taking shortcuts.

Scope

  • Main library (src/)
  • All modules (src/types.rs, src/dsl.rs, src/runtime.rs, src/monitoring.rs, src/explain.rs)
  • Test code in src/lib.rs

Tools to Run

1. Cargo Build

cargo build --features pg17

Look for: - Unused imports - Unused variables - Unused functions/types - Dead code - Deprecated API usage

2. Cargo Clippy

cargo clippy --features pg17

Common clippy warnings to address: - needless_lifetimes - Remove unnecessary explicit lifetimes - derivable_impls - Replace manual impl Default with #[derive(Default)] - question_mark - Use ? operator instead of manual error checking - redundant_pattern_matching - Simplify match expressions - manual_map - Replace match with .map() - useless_conversion - Remove .into() when type already matches

3. Cargo Format

cargo fmt

Ensures consistent formatting across all code.

4. PGRX Tests

cargo pgrx test --features pg17

Ensures all pgrx tests compile and pass.

Handling Unused Code

❌ DO NOT

  • Add #[allow(unused)] or #[allow(dead_code)] without understanding why
  • Prefix variables with _ to silence warnings unless they’re truly meant to be ignored
  • Remove code that’s part of public API or used in feature-gated code
  • Remove error handling just to simplify code

✅ DO

  1. Investigate first: Understand why the code is unused
  2. Check feature gates: Code might be used under #[cfg(feature = "...")]
  3. Check tests: Code might only be used in test scenarios
  4. Remove genuinely unused code: If it’s truly not needed, delete it
  5. For intentionally unused parameters: Use _name pattern when the parameter is required by a trait but not used in a specific implementation

Example Workflow

# 1. Build and capture warnings
cargo build --features pg17 2>&1 | tee build-warnings.txt

# 2. Run clippy
cargo clippy --features pg17 2>&1 | tee clippy-warnings.txt

# 3. Review warnings
cat build-warnings.txt | grep "warning:"
cat clippy-warnings.txt | grep "warning:"

# 4. Fix warnings iteratively
# ... make fixes ...

# 5. Verify fixes
cargo build --features pg17
cargo clippy --features pg17

# 6. Format
cargo fmt

# 7. Test everything still works
cargo pgrx test --features pg17

Common Warning Fixes

1. Unused Import

// ❌ Before
use std::collections::HashMap;  // warning: unused import

// ✅ After - Remove if truly unused
// (import removed)

2. Unused Variable

// ❌ Wrong fix
let _result = expensive_operation();  // Misleading - operation still runs

// ✅ Correct - If value is genuinely not needed, remove the binding
expensive_operation();

// ✅ Correct - If required by trait but unused in this impl
fn process(&self, _ctx: Context) { }  // Trait requires ctx parameter

3. Dead Code

// If function is truly unused:
// ❌ Don't suppress
#[allow(dead_code)]
fn unused_helper() { }

// ✅ Remove it
// (function removed)

// If used in tests:
// ✅ Add appropriate cfg
#[cfg(test)]
fn test_helper() { }

4. Clippy: Question Mark

// ❌ Before
if result.is_err() {
    return result;
}

// ✅ After
result?;

PGRX-Specific Considerations

Extension SQL

The extension_sql! macro generates SQL that PostgreSQL executes. Warnings about unused items inside these blocks may be false positives.

Background Workers

Code in the background worker (src/runtime.rs) runs in a separate PostgreSQL process. Ensure you test with: bash ./scripts/test-e2e-local.sh

Schema Functions

Functions decorated with #[pg_extern(schema = "df")] are called from SQL, not Rust. They may appear unused to the compiler but are essential.

Validation Checklist

Before considering the cleanup complete:

  • [ ] cargo build --features pg17 produces zero warnings
  • [ ] cargo clippy --features pg17 produces zero warnings
  • [ ] cargo fmt --check produces no diff
  • [ ] cargo pgrx test --features pg17 passes completely
  • [ ] ./scripts/test-e2e-local.sh passes all tests
  • [ ] Spot-check: Run a few E2E tests to ensure they work

When to Ask

Stop and ask the user if: - You need to remove a large amount of code (>100 lines) - Warning fix requires changing public API (SQL functions) - You’re unsure if code is used in production scenarios - Fix would require significant refactoring

Anti-Patterns

Don’t do these: - Blindly adding #[allow(dead_code)] everywhere - Prefixing everything with _ to silence warnings - Removing error handling to eliminate unused Result - Deleting code you don’t understand - Changing SQL APIs just to reduce warnings