Skip to content

Conversation

@samueltardieu
Copy link
Member

@samueltardieu samueltardieu commented Oct 9, 2025

Summary by CodeRabbit

  • New Features

    • No user-facing changes in this release.
  • Style

    • Tightened static analysis by activating stricter lint checks and converting some suppressions into explicit expectations.
    • Adjusted lint handling related to documentation and attribute usage during static analysis.
  • Tests

    • Simplified a test to avoid an unnecessary clone, improving ownership semantics.
  • Chores

    • Refined lint configuration for improved code consistency; no runtime or public API changes.

@coderabbitai
Copy link

coderabbitai bot commented Oct 9, 2025

Walkthrough

Enable two Clippy lints in Cargo.toml, change several clippy attributes from allow to expect, and replace one test clone with a move; no runtime or API behavior changes.

Changes

Cohort / File(s) Summary
Clippy lint configuration
Cargo.toml
Activate Clippy settings: redundant_clone = "deny" and allow_attributes = "deny" (comment context retained).
BFS lint attribute update
src/directed/bfs.rs
Replace allow(clippy::missing_panics_doc) with expect(clippy::missing_panics_doc) on bfs_bidirectional.
Example lint attribute update
examples/sliding-puzzle.rs
Replace allow(clippy::derived_hash_with_manual_eq) with expect(clippy::derived_hash_with_manual_eq) on Game.
Test ownership adjustment
tests/matrix.rs
Use m1 (moved) instead of m1.clone() when preparing matrix for rotate_ccw(3), removing a redundant clone.
Test lint attribute updates
tests/pathfinding.rs
Replace two allow(...) attributes on static SUCCESSORS with expect(...) for the corresponding Clippy lints.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I hop through code with careful paws,
Turning nags to notes and fixing flaws.
Deny the clones, expect the quirks,
Tests still pass with tidy perks.
A carrot for CI—cheers and applause! 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately and concisely summarizes the primary change—enabling new Clippy lints—using the conventional commit format with a clear scope and action. It directly reflects the changeset without extraneous detail.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch push-tywmvorkmvqz

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2b52d6e and becfe6b.

📒 Files selected for processing (5)
  • Cargo.toml (1 hunks)
  • examples/sliding-puzzle.rs (1 hunks)
  • src/directed/bfs.rs (1 hunks)
  • tests/matrix.rs (1 hunks)
  • tests/pathfinding.rs (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/directed/bfs.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: benchmarks
🔇 Additional comments (4)
tests/pathfinding.rs (1)

4-4: LGTM! Appropriate use of #[expect] attributes.

Changing from #[allow] to #[expect] is a best practice for intentional lint violations. The expect attribute will warn if the code changes such that the lint no longer triggers, helping to keep the codebase clean.

Also applies to: 338-338, 423-423

examples/sliding-puzzle.rs (1)

15-15: LGTM! Correct use of #[expect] for intentional design.

The derived_hash_with_manual_eq lint is expected here because the Game struct intentionally derives Hash while providing a manual PartialEq implementation that includes ordering checks for performance optimization.

tests/matrix.rs (1)

172-172: LGTM! Eliminates redundant clone.

Good change—m1 is no longer used after this point, so moving ownership instead of cloning is more efficient and aligns with the newly enabled redundant_clone = "deny" lint.

Cargo.toml (1)

51-52: LGTM! Enables stricter lint enforcement.

Activating these lints enforces better code quality practices:

  • redundant_clone = "deny": Catches unnecessary clones
  • allow_attributes = "deny": Encourages use of #[expect] over #[allow] for intentional lint suppressions

The changes throughout the codebase properly address violations of these newly enabled lints.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/directed/bfs.rs (1)

175-175: Good switch to #[expect]; consider adding a reason or documenting panics

Using #[expect(clippy::missing_panics_doc)] aligns with denying allow attributes. Optionally add a reason for future readers, e.g., #[expect(clippy::missing_panics_doc, reason = "Internal unwraps rely on BFS invariants")], or add a brief “Panics” section to the docstring to remove the need for the attribute later.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2024b16 and 2b52d6e.

📒 Files selected for processing (2)
  • Cargo.toml (1 hunks)
  • src/directed/bfs.rs (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Extra tests in release mode
  • GitHub Check: Test with minimal versions
  • GitHub Check: benchmarks
🔇 Additional comments (1)
Cargo.toml (1)

51-52: Verify absence of Clippy allow/expect attributes

Run a simple ripgrep scan to catch any remaining attributes:

#!/usr/bin/env bash
set -euo pipefail

echo "Scanning for residual #[allow(clippy::…)] attributes:"
rg -n 'allow\(clippy::' -C2

echo
echo "Scanning for residual #[expect(clippy::…)] attributes:"
rg -n 'expect\(clippy::' -n

@samueltardieu samueltardieu added this pull request to the merge queue Oct 9, 2025
Merged via the queue into main with commit 66cc6e0 Oct 9, 2025
14 checks passed
@samueltardieu samueltardieu deleted the push-tywmvorkmvqz branch October 9, 2025 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants