Skip to content

Enable Cargo Clippy pedantic linting rules#57

Merged
vtvz merged 2 commits intomasterfrom
cargo-enable-pedantic
Dec 27, 2025
Merged

Enable Cargo Clippy pedantic linting rules#57
vtvz merged 2 commits intomasterfrom
cargo-enable-pedantic

Conversation

@vtvz
Copy link
Owner

@vtvz vtvz commented Dec 22, 2025

Enables Clippy's pedantic lint level with carefully selected exceptions for rules that don't align with project style. Applies all resulting lint fixes across the codebase to improve code quality and consistency.

@coderabbitai
Copy link

coderabbitai bot commented Dec 22, 2025

📝 Walkthrough

Walkthrough

This PR modernizes code style and lints across the Rust codebase: adds Clippy lint configuration, converts several async logger/init functions to synchronous, adds many #[must_use] annotations, simplifies control-flow patterns (let-else, map_or), makes small API/derive changes (Copy on enum, IntoIterator impl, private LyricsResponse), and minor formatting/test adjustments.

Changes

Cohort / File(s) Summary
Clippy / Manifest
Cargo.toml, src/main.rs
Added a new [lints.clippy] configuration under release profile; removed a local unwrap_used lint in src/main.rs.
Logger: async→sync
src/infrastructure/logger.rs, src/app.rs, src/cli/users.rs, src/workers/*, src/workers/{bot.rs,queues.rs,server.rs,track_check.rs}
Converted logger/loki init functions from async to sync and updated call sites to remove .await.
Worker callsite updates
src/workers/bot.rs, src/workers/queues.rs, src/workers/server.rs, src/workers/track_check.rs
Replaced awaited init calls with direct sync calls; adjusted small select!/match patterns (e.g., Ok(_)Ok(()), _ =() =).
Entities / Generated files
src/entity/track_status.rs, src/entity/user.rs, src/entity/spotify_auth.rs, src/entity/mod.rs
Added Copy derive to Status; added #[must_use] to several user/locale/role accessors and added Locale::locale_codes(); removed generator file comments.
Profanity module
src/profanity.rs, src/queue/profanity_check.rs
Removed public Checker; implemented IntoIterator for &CheckResult; made Manager::check public + #[must_use]; added several #[must_use]s; small API/impl changes (TypeWrapper::name by-value).
Error handler
src/infrastructure/error_handler.rs
Added #[must_use] to constructors (unhandled, handled, handled_notified); simplified spotify_client_error logic with if let and included user_id in logs.
Lyrics modules
src/lyrics/genius.rs, src/lyrics/lrclib.rs, src/lyrics/musixmatch.rs, src/lyrics/utils.rs
Made LyricsResponse local/private in get_lyrics; replaced some raw-string delimiters; modernized iterator usage and Option handling; added #[must_use] to several util methods.
Metrics
src/metrics/influx.rs, src/metrics/mod.rs
Replaced some map+unwrap_or chains with map_or_else/is_some_and; adjusted tokio::select! binding patterns.
Spotify module refactor
src/spotify/auth.rs, src/spotify/mod.rs
Introduced Arc usage for token storage, converted some pattern matches to let-else, changed Deref impl to return &self.spotify, added many #[must_use] accessors, and adjusted string/ID accessor return types.
Notification & User services
src/services/notification.rs, src/services/user.rs
Replaced map+unwrap with map_or/map_or_else for optional formatting; added #[must_use] to UserService query methods; minor lint suppression and numeric conversion tweaks.
Telegram actions & handlers
src/telegram/actions/*, src/telegram/handlers/*, src/telegram/inline_buttons*.rs, src/telegram/keyboards.rs, src/telegram/commands.rs, src/telegram/utils.rs
Widespread small refactors: removed extraneous semicolons, changed raw-string delimiters, swapped map/unwrap → map_or, inverted some condition branches, added many #[must_use] annotations (inline buttons, keyboards, commands), and adjusted label formatting.
Recommendation & word-definition
src/telegram/actions/recommendasion.rs, src/telegram/actions/word_definition.rs
Added #[must_use] to formatted methods; replaced "" with String::new() and inverted keyboard-construction condition.
Misc utils & serde
src/utils/general.rs, src/utils/serde.rs, src/user.rs
Added #[must_use] to Clock::now(), switched formatting to named parameters, used u8::from(bool) for serialization; added clippy allow on nested Option and small refactors.
Minor formatting / small changes
src/tick/mod.rs, src/metrics/*, assorted small files
Various small control-flow/formatting fixes (brace/semicolon removal), changing Ok(_) to Ok(()) in some matches, and test-format tweaks.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45–60 minutes

  • Areas needing focused review:
    • logger async→sync conversions: ensure no synchronous blocking occurs in async contexts and all call sites updated.
    • Spotify token management: Arc introduction, Deref change, and expiry checks (security-sensitive).
    • Profanity API changes: removal of Checker, IntoIterator for &CheckResult, and TypeWrapper::name(self) — ensure callers compile and semantics preserved.
    • Copy derive on Status — validate usages where move vs copy matters.
    • Widespread #[must_use] additions — verify no intended discard sites relying on previous behavior.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 65.91% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Enable Cargo Clippy pedantic linting rules' directly and clearly describes the main change: enabling Clippy's pedantic lint level across the codebase.
Description check ✅ Passed The description accurately explains that the PR enables Clippy's pedantic lint level with selected exceptions and applies resulting lint fixes across the codebase to improve code quality.
✨ 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 cargo-enable-pedantic

📜 Recent review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7d417eb and ae259ca.

📒 Files selected for processing (1)
  • src/spotify/mod.rs
🔇 Additional comments (7)
src/spotify/mod.rs (7)

190-206: Good security improvement: HTML escaping added.

Adding html::escape() for user-controlled strings (name_with_artists() and album_name()) prevents potential XSS vulnerabilities when these values are rendered in Telegram HTML messages.


365-379: Token isolation pattern is correct.

The pattern of cloning the AuthCodeSpotify instance and then resetting its token with Arc::default() (line 367) ensures that each user's Spotify instance has independent token storage, preventing token sharing between users.


445-451: Verify MSRV compatibility with let-else syntax.

The let-else pattern was stabilized in Rust 1.65. Ensure the project's MSRV is at least 1.65.

The MSRV check from the previous comment will also verify this requirement.


463-465: Good simplification of Deref implementation.

Changing from self.spotify.deref() to &self.spotify is cleaner and idiomatic. The reference will automatically dereference as needed, so the behavior is equivalent.


40-40: Excellent addition of #[must_use] annotations.

The #[must_use] attributes have been appropriately added to constructors, getters, and methods that return computed values. This improves API safety by making it explicit that ignoring these return values would likely be a programming error.

Also applies to: 45-45, 86-86, 125-125, 130-130, 135-135, 140-140, 145-145, 152-152, 157-157, 162-162, 167-167, 172-172, 180-180, 185-185, 190-190, 199-199, 224-224, 260-260


316-316: No action needed. The project's minimum supported Rust version (1.85) is well above the 1.70 requirement for is_some_and, so there are no MSRV compatibility issues with this usage.


158-165: API change: Return type changed from owned to borrowed strings.

The return types of artist_names() and artist_ids() have changed from Vec<String> to Vec<&str>. This is a breaking change for external API consumers. However, the internal usage at src/lyrics/musixmatch.rs:147 (track.artist_names().iter().join(",")) is compatible with the new return type due to immediate consumption and deref coercion in join().

If this codebase has external consumers, document this as a breaking change in release notes.


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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/spotify/mod.rs (1)

199-206: Missing HTML escaping creates potential XSS vulnerability.

track_tg_link correctly escapes the name using html::escape(), but album_tg_link outputs album_name() raw. Album names from Spotify can contain special characters (&, <, >, quotes) that will break HTML rendering or potentially allow injection in Telegram messages.

🔎 Proposed fix
 #[must_use]
 pub fn album_tg_link(&self) -> String {
     format!(
         r#"<a href="{link}">{name}</a>"#,
-        name = self.album_name(),
+        name = html::escape(self.album_name()),
         link = self.album_url()
     )
 }
📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e1095df and 7d417eb.

📒 Files selected for processing (50)
  • Cargo.toml
  • src/app.rs
  • src/cli/users.rs
  • src/entity/mod.rs
  • src/entity/spotify_auth.rs
  • src/entity/track_status.rs
  • src/entity/user.rs
  • src/infrastructure/error_handler.rs
  • src/infrastructure/logger.rs
  • src/lyrics/genius.rs
  • src/lyrics/lrclib.rs
  • src/lyrics/musixmatch.rs
  • src/lyrics/utils.rs
  • src/main.rs
  • src/metrics/influx.rs
  • src/metrics/mod.rs
  • src/profanity.rs
  • src/queue/profanity_check.rs
  • src/services/notification.rs
  • src/services/user.rs
  • src/spotify/auth.rs
  • src/spotify/mod.rs
  • src/telegram/actions/admin_users/details.rs
  • src/telegram/actions/admin_users/list.rs
  • src/telegram/actions/analyze.rs
  • src/telegram/actions/broadcast.rs
  • src/telegram/actions/details.rs
  • src/telegram/actions/dislike.rs
  • src/telegram/actions/global_stats.rs
  • src/telegram/actions/language.rs
  • src/telegram/actions/like.rs
  • src/telegram/actions/magic.rs
  • src/telegram/actions/recommendasion.rs
  • src/telegram/actions/start.rs
  • src/telegram/actions/word_definition.rs
  • src/telegram/commands.rs
  • src/telegram/handlers/keyboards.rs
  • src/telegram/handlers/url.rs
  • src/telegram/inline_buttons.rs
  • src/telegram/inline_buttons_admin.rs
  • src/telegram/keyboards.rs
  • src/telegram/utils.rs
  • src/tick/mod.rs
  • src/user.rs
  • src/utils/general.rs
  • src/utils/serde.rs
  • src/workers/bot.rs
  • src/workers/queues.rs
  • src/workers/server.rs
  • src/workers/track_check.rs
💤 Files with no reviewable changes (3)
  • src/main.rs
  • src/entity/spotify_auth.rs
  • src/entity/mod.rs
🧰 Additional context used
🧬 Code graph analysis (22)
src/telegram/actions/admin_users/details.rs (1)
src/utils/general.rs (2)
  • pretty_format (164-164)
  • pretty_format (168-189)
src/telegram/actions/language.rs (2)
src/telegram/keyboards.rs (2)
  • markup (25-36)
  • markup (78-86)
src/telegram/actions/login.rs (1)
  • send_login_invite (16-39)
src/cli/users.rs (1)
src/infrastructure/logger.rs (1)
  • init (37-61)
src/entity/track_status.rs (2)
src/services/track_status.rs (2)
  • TrackStatusService (45-45)
  • TrackStatusService (47-165)
src/entity/mod.rs (1)
  • track_status (6-6)
src/lyrics/musixmatch.rs (2)
src/lyrics/mod.rs (1)
  • musixmatch (18-18)
src/main.rs (1)
  • lyrics (22-22)
src/workers/track_check.rs (1)
src/infrastructure/logger.rs (1)
  • init (37-61)
src/services/user.rs (1)
src/services/track_status.rs (1)
  • status (32-38)
src/infrastructure/error_handler.rs (1)
src/spotify/errors.rs (1)
  • SpotifyError (42-84)
src/tick/mod.rs (2)
src/infrastructure/error_handler.rs (2)
  • err (54-54)
  • err (178-178)
src/spotify/errors.rs (1)
  • err (44-44)
src/telegram/actions/recommendasion.rs (1)
src/spotify/mod.rs (2)
  • track_tg_link (191-197)
  • name_with_artists (146-150)
src/lyrics/lrclib.rs (1)
src/lyrics/mod.rs (1)
  • SearchResult (30-41)
src/services/notification.rs (4)
src/user.rs (1)
  • user (31-33)
src/telegram/inline_buttons.rs (1)
  • into (81-83)
src/telegram/inline_buttons_admin.rs (1)
  • into (187-189)
src/services/mod.rs (1)
  • notification (3-3)
src/metrics/mod.rs (1)
src/utils/general.rs (1)
  • ctrl_c (127-133)
src/telegram/actions/word_definition.rs (1)
src/telegram/handlers/mod.rs (1)
  • keyboards (4-4)
src/entity/user.rs (1)
src/user.rs (1)
  • language (43-45)
src/workers/queues.rs (2)
src/infrastructure/logger.rs (1)
  • init (37-61)
src/utils/general.rs (1)
  • ctrl_c (127-133)
src/workers/server.rs (1)
src/infrastructure/logger.rs (1)
  • init (37-61)
src/telegram/actions/admin_users/list.rs (4)
src/services/track_status.rs (1)
  • status (32-38)
src/telegram/inline_buttons.rs (1)
  • into (81-83)
src/telegram/inline_buttons_admin.rs (1)
  • into (187-189)
build.rs (2)
  • main (15-20)
  • emit_git (3-13)
src/telegram/actions/global_stats.rs (1)
src/user.rs (1)
  • user_id (57-59)
src/telegram/actions/details.rs (1)
src/profanity.rs (1)
  • highlighted (161-178)
src/telegram/handlers/keyboards.rs (1)
src/telegram/handlers/mod.rs (1)
  • keyboards (4-4)
src/workers/bot.rs (1)
src/infrastructure/logger.rs (1)
  • init (37-61)
🔇 Additional comments (99)
src/tick/mod.rs (1)

161-163: LGTM! Formatting aligns with Clippy pedantic rules.

The error-handling block formatting is now compliant with Clippy pedantic style expectations: proper semicolon placement and brace positioning. The error logging remains correct—using tracing::error! with debug-formatted error output is idiomatic for structured logging in async contexts.

src/telegram/handlers/url.rs (1)

15-17: LGTM!

The removal of the trailing semicolon after the if let block is a correct style fix aligned with Clippy's pedantic linting rules. No behavioral change.

src/utils/serde.rs (1)

15-15: LGTM! Idiomatic refactoring using standard library conversion.

Replacing the explicit conditional with u8::from(*b) is the idiomatic Rust approach for bool-to-u8 conversion. This produces the same output (1 for true, 0 for false) and aligns with Clippy's pedantic recommendations.

src/entity/track_status.rs (1)

90-90: LGTM: Adding Copy to a simple enum improves ergonomics.

The Status enum contains only simple discriminants with no associated data, making it an ideal candidate for the Copy trait. This change improves performance and ergonomics when passing status values around, as seen in the TrackStatusService methods where Status is frequently used by-value.

src/telegram/actions/magic.rs (2)

98-98: LGTM! Clean removal of unnecessary semicolon.

The semicolon after the closing brace was extraneous. This cleanup aligns with Clippy's pedantic linting rules and improves code consistency without any functional impact.


161-161: LGTM! Proper statement termination.

The semicolon addition ensures explicit statement termination, which aligns with Clippy's pedantic linting preferences for code clarity and consistency.

src/telegram/actions/global_stats.rs (3)

13-16: LGTM! Parameter rename improves clarity.

The change from _state to state and the corresponding tracing instrumentation update correctly address a clippy pedantic lint. Since the parameter is actually used in the tracing instrumentation, removing the underscore prefix eliminates potential confusion and makes the code more idiomatic.


61-61: LGTM! Semicolon addition improves consistency.

Adding the explicit semicolon after the push() statement aligns with Rust idioms for statement expressions, likely addressing a clippy pedantic lint.


66-93: LGTM! Raw string literal simplification.

The change from r#"..."# to r"..." correctly simplifies the raw string literal syntax. Since the string content doesn't contain sequences that require hash delimiters, using the simpler form improves readability without affecting functionality.

src/telegram/actions/recommendasion.rs (6)

66-72: Good addition of #[must_use] attribute.

The #[must_use] attribute is appropriate for this pure formatting method that computes and returns data without side effects. This helps catch potential bugs where the return value might be accidentally ignored.


74-91: Good addition of #[must_use] attribute.

Consistent with the other formatting method, this #[must_use] attribute appropriately signals that the return value should not be ignored.


202-202: Good style fix removing unnecessary semicolon.

The semicolon after the closing brace of an if let statement block is unnecessary in Rust. This Clippy pedantic fix improves code style consistency.


427-427: Good style fix removing unnecessary semicolon.

Consistent with the other semicolon removals, this improves code style without changing behavior.


435-435: Good style fix removing unnecessary semicolon.

Another appropriate semicolon removal that aligns with Rust idioms.


448-452: Excellent refactoring to method reference syntax.

The change from .map(|track| track.name_with_artists()) to .map(ShortTrack::name_with_artists) is a more idiomatic Rust pattern that eliminates the redundant closure. This is functionally equivalent and more concise.

src/spotify/mod.rs (6)

40-48: LGTM!

Adding #[must_use] to pure accessor methods is good practice and aligns with Clippy's pedantic recommendations.


158-165: LGTM!

Using method references (String::as_str, Id::id) instead of closures is more idiomatic and aligns with Clippy's pedantic recommendations.


310-316: LGTM!

The is_some_and(Token::is_expired) pattern is more concise and idiomatic than the previous explicit check, and correctly evaluates to false when the token is None.


367-367: LGTM!

Using Arc::default() is explicit and clearer than relying on type inference with Default::default().


445-451: LGTM!

The let ... else pattern is more readable for early returns compared to explicit match blocks, and is the idiomatic Rust approach for this pattern.


460-465: LGTM!

This change relies on Rust's deref coercion: &self.spotify (where self.spotify: T and T: Deref<Target = AuthCodeSpotify>) automatically coerces to &AuthCodeSpotify. This is equivalent to the explicit &*self.spotify but more idiomatic.

src/telegram/actions/like.rs (1)

45-45: LGTM: Formatting cleanup

Removed unnecessary semicolon after the closing brace. This aligns with Clippy's pedantic formatting rules without changing behavior.

src/telegram/actions/dislike.rs (1)

48-48: LGTM: Formatting cleanup

Removed unnecessary semicolon after the closing brace, consistent with Clippy pedantic linting rules.

src/telegram/actions/start.rs (1)

56-56: LGTM: Formatting cleanup

Removed unnecessary semicolon after the closing brace, consistent with the broader formatting standardization in this PR.

src/user.rs (3)

12-13: LGTM: Appropriate lint suppression

The #[allow(clippy::option_option)] attribute correctly acknowledges the intentional use of nested Option. This pattern is appropriate here for tri-state caching: None (not yet checked), Some(None) (checked but no Spotify user), Some(Some(user)) (Spotify user present).


21-29: LGTM: Good API hygiene improvements

The #[must_use] attribute on the constructor prevents accidental disposal of the UserState instance. The use of Mutex::default() is more idiomatic than Mutex::new(None) while maintaining equivalent behavior.


89-97: LGTM: More concise premium check

The refactor to is_some_and improves readability by chaining the option checks. The logic correctly verifies both that the Spotify user exists and that their product tier is Premium.

src/metrics/influx.rs (2)

43-44: LGTM: More explicit error handling

Changing the closure parameter from _ to () makes it explicit that the error type is the unit type, improving code clarity.


49-49: LGTM: Idiomatic default handling

Refactoring to map_or_else is the idiomatic Rust pattern for providing a default value when Option is None. This maintains the same behavior while being more concise.

src/metrics/mod.rs (1)

221-221: LGTM: Correct pattern matching for unit type

The change from Ok(_) to () correctly reflects that ctrl_c() returns the unit type () rather than a Result. This makes the pattern match more accurate and aligns with the function signature shown in the relevant code snippets.

src/spotify/auth.rs (1)

55-57: LGTM: Modern Rust idiom

The let-else refactor improves readability by using Rust's modern pattern for early returns. This is semantically equivalent to the previous match expression but more concise.

src/services/user.rs (4)

49-50: LGTM: Appropriate lint suppression

The #[allow(clippy::needless_pass_by_value)] is reasonable here. While Clippy suggests passing &bool, bool is a Copy type with minimal cost, and passing by value is idiomatic for small Copy types in Rust.


59-59: LGTM: Cleaner boolean-to-integer conversion

Using i32::from(profane) is more idiomatic than a conditional expression for converting bool to i32. This maintains the same semantics (false→0, true→1) with improved clarity.


107-120: LGTM: Query builder improvements

The #[must_use] attribute is appropriate for query builders, ensuring developers don't accidentally construct a query without executing it. The formatting cleanups (removing semicolons after closing braces) align with Clippy's pedantic rules.


362-365: LGTM: Builder pattern safety

Adding #[must_use] to the stats query builder constructor prevents accidental creation of builders that are never executed, which is important for ensuring database updates actually occur.

Cargo.toml (1)

82-96: LGTM! Well-balanced Clippy configuration.

The pedantic lint configuration with targeted exceptions is appropriate. The allowed lints (cast operations, excessive bools, too many lines) are pragmatic choices that prevent overly restrictive linting while still gaining the benefits of pedantic-level checks.

src/cli/users.rs (4)

53-53: LGTM! Improved error message clarity.

Using named interpolation {user_id} makes the error message more explicit and maintainable.


59-59: LGTM! Improved output formatting.

The named interpolation pattern {user_id} and {role:?} is more explicit and easier to maintain.


82-82: LGTM! Simplified error formatting.

Direct interpolation {e} is cleaner than the previous formatting approach.


65-65: Logger initialization successfully converted to synchronous.

The logger initialization has been converted from async to sync. All five call sites across the codebase (src/workers/server.rs, src/workers/track_check.rs, src/workers/bot.rs, src/workers/queues.rs, and src/cli/users.rs) have been updated to call logger::init().expect(...) without .await. The function signature in src/infrastructure/logger.rs confirms the synchronous implementation with no remaining async patterns.

src/app.rs (3)

38-46: LGTM! Appropriate use of #[must_use] for accessor methods.

Adding #[must_use] to these accessor methods ensures callers don't accidentally discard the returned references, which is a good practice for getters.


231-253: LGTM! Correctly removed unnecessary async.

The function doesn't perform any async operations (no .await calls), so removing async is appropriate and improves performance by avoiding unnecessary async machinery.


268-268: LGTM! Consistent with synchronous init_ai signature.

The removal of .await correctly matches the now-synchronous init_ai function signature.

src/telegram/keyboards.rs (2)

13-49: LGTM! Appropriate #[must_use] attributes on StartKeyboard methods.

The #[must_use] attributes on into_button, markup, and from_str are appropriate since these methods construct values that should be used rather than discarded.


59-97: LGTM! Appropriate #[must_use] attributes on LanguageKeyboard methods.

The #[must_use] attributes on into_button, into_locale, markup, and parse correctly ensure the constructed values are not accidentally ignored.

src/telegram/actions/language.rs (1)

25-39: LGTM! Logic inversion improves UX flow.

The inverted conditional now correctly shows:

  • Authenticated users: Get the StartKeyboard markup to interact with bot features
  • Unauthenticated users: Get keyboard removed and receive a login invite

This is more intuitive than the previous logic.

src/utils/general.rs (2)

138-141: LGTM! Appropriate #[must_use] for Clock::now().

Adding #[must_use] ensures callers don't accidentally discard the timestamp, which would be a logic error.


177-188: LGTM! Improved formatting with named parameters.

Using named format parameters ({centiseconds:02}, {days:02}, etc.) improves code clarity and makes the formatting intent explicit.

src/lyrics/musixmatch.rs (2)

48-48: LGTM! Eliminated redundant closure.

Using String::as_str directly instead of a closure |lyrics| lyrics.as_str() is more idiomatic and eliminates the Clippy redundant_closure warning.


204-206: LGTM! Modern let-else pattern.

The let-else pattern is more concise and idiomatic for early returns on pattern match failures, improving readability.

src/telegram/inline_buttons_admin.rs (3)

125-173: LGTM! Appropriate #[must_use] on label() method.

Adding #[must_use] ensures the constructed label isn't accidentally discarded, which would be a logic error since constructing labels has a purpose.


160-166: LGTM! Improved string formatting with named parameters.

Using named interpolation ({arrow}, {status:?}) makes the format strings more explicit and maintainable.


177-182: LGTM! Appropriate #[must_use] on button constructor.

Adding #[must_use] to into_inline_keyboard_button() correctly prevents accidentally discarding the constructed button.

src/entity/user.rs (3)

47-50: LGTM! Good use of #[must_use] attribute.

The #[must_use] attribute on is_admin() ensures callers don't accidentally discard the boolean result, improving API safety.


204-216: LGTM! Appropriate #[must_use] attributes.

Both language() and the new locale_codes() helper correctly use #[must_use] to prevent accidental discarding of their return values.


276-280: LGTM! Good use of #[must_use] attribute.

The #[must_use] attribute ensures the boolean result isn't ignored, consistent with the same attribute on Model::is_admin().

src/telegram/actions/analyze.rs (1)

195-195: LGTM! More idiomatic empty string initialization.

Using String::new() instead of "".to_string() is the idiomatic way to create empty strings in Rust and is more efficient.

src/telegram/inline_buttons.rs (3)

20-36: LGTM! Appropriate #[must_use] attribute.

The #[must_use] attribute ensures the computed label isn't discarded, which is correct for a pure function that returns a translated string.


40-67: LGTM! Good use of #[must_use] attribute.

The #[must_use] attribute ensures the constructed keyboard button vector isn't discarded. The locale parameter is correctly passed through to the button creation methods.


71-77: LGTM! Appropriate #[must_use] attribute.

The #[must_use] attribute ensures the created inline keyboard button isn't discarded, which is correct for this builder-style method.

src/telegram/actions/word_definition.rs (2)

227-251: LGTM! More idiomatic empty string initialization.

Using String::new() instead of "".to_string() is the standard Rust idiom for creating empty strings and avoids unnecessary string conversion.


279-283: LGTM! Clearer conditional flow.

The inverted conditional (checking is_empty() first) makes the early-return pattern more explicit and improves readability.

src/lyrics/utils.rs (2)

32-43: LGTM! Appropriate #[must_use] attribute.

The #[must_use] attribute ensures the computed track name variations aren't discarded, which is correct for this pure function.


52-66: LGTM! Good use of #[must_use] attributes.

All three methods (new(), confident(), and avg()) correctly use #[must_use] since they return computed values that should be consumed by the caller.

src/lyrics/lrclib.rs (2)

49-49: LGTM! More concise method reference.

Using String::as_str as a method reference instead of a closure is more idiomatic and concise.


173-176: LGTM! More idiomatic option handling.

Using map_or_else directly is more idiomatic than chaining map with unwrap_or_else and avoids an intermediate Option allocation.

src/services/notification.rs (1)

39-58: LGTM! More idiomatic option handling.

Using map_or instead of map + unwrap_or is more idiomatic and avoids unnecessary intermediate allocations. The direct variable interpolation in the format string is also cleaner.

src/infrastructure/logger.rs (1)

9-38: All call sites have been correctly updated to the synchronous API.

Verification confirms that no remaining .await calls exist on logger::init() or logger::loki(). All five call sites across the codebase (in src/workers/bot.rs, src/workers/track_check.rs, src/workers/server.rs, src/workers/queues.rs, and src/cli/users.rs) correctly use the synchronous API.

src/infrastructure/error_handler.rs (2)

23-23: LGTM: Excellent use of #[must_use] on result constructors.

Adding #[must_use] to these constructor methods ensures that callers don't accidentally ignore error handling results, improving code safety.

Also applies to: 31-31, 39-39


182-203: LGTM: Clean refactoring from match to if-let.

The refactoring simplifies the control flow while preserving the same error-handling logic. The specific InvalidToken case is handled first with user notification, then falls through to generic client error logging.

src/lyrics/genius.rs (2)

220-221: LGTM: Explicit initialization improves clarity.

Using vec![] and Language::default() is more explicit and readable than the generic Default::default(), making the intent clearer.


239-242: LGTM: Good API hygiene by making LyricsResponse private.

Moving LyricsResponse to a local struct within get_lyrics appropriately reduces the public API surface since this type is only used internally for deserialization.

src/telegram/handlers/keyboards.rs (1)

23-23: LGTM: Formatting cleanup.

Removing the redundant semicolons after the if-let blocks improves code style consistency.

Also applies to: 42-42

src/queue/profanity_check.rs (1)

89-89: LGTM: Explicit unit type improves clarity.

Changing from Ok(_) to Ok(()) makes it explicit that a unit type is expected, improving code readability.

src/workers/track_check.rs (1)

7-7: LGTM: Logger initialization updated to synchronous API.

The change from logger::init().await to logger::init() aligns with the updated logger API that now returns a synchronous anyhow::Result<()> instead of a future.

src/telegram/actions/broadcast.rs (1)

54-54: LGTM: Style consistency improvements.

Making the unit type explicit with Ok(()) and adding the trailing semicolon improves code clarity and consistency.

Also applies to: 57-57

src/telegram/utils.rs (1)

3-3: LGTM: Appropriate #[must_use] annotation.

Adding #[must_use] to extract_url_from_message is good API design, ensuring callers don't accidentally ignore the extraction result.

src/telegram/actions/admin_users/details.rs (1)

159-159: LGTM: Idiomatic Option handling with map_or.

Replacing .map().unwrap_or() with .map_or() is more concise and efficient, avoiding intermediate Option allocations. This is the idiomatic Rust pattern for transforming Option values with a default.

Also applies to: 169-169

src/telegram/actions/details.rs (3)

16-16: LGTM: Import supports method reference usage.

The new import enables the idiomatic method reference on Line 280, where LineResult::highlighted is used instead of a closure.


55-55: LGTM: Unnecessary semicolons removed.

These semicolons after closing braces are unnecessary and their removal aligns with Clippy pedantic linting rules. No behavioral change.

Also applies to: 110-110


280-280: LGTM: Idiomatic method reference.

Using LineResult::highlighted as a method reference is more idiomatic than the closure form, making the code cleaner while maintaining identical behavior.

src/workers/queues.rs (2)

10-10: LGTM: Logger initialization now synchronous.

The removal of .await aligns with the updated infrastructure::logger::init() function signature, which is now synchronous across the codebase.


27-27: LGTM: Explicit unit pattern.

Changing from _ = to () = makes the intent clearer that we're explicitly waiting for and discarding a unit value from ctrl_c().

src/workers/bot.rs (2)

60-60: LGTM: Logger initialization now synchronous.

Consistent with the synchronous logger API update applied across all worker files.


107-107: LGTM: Unnecessary semicolon removed.

Removing the semicolon after the closing brace aligns with Clippy pedantic rules and has no behavioral impact.

src/telegram/actions/admin_users/list.rs (2)

101-106: LGTM: Simplified raw string delimiter.

The # delimiter is unnecessary for this raw string since it doesn't contain quotation marks. This simplification improves readability.


108-108: LGTM: Idiomatic map_or usage.

Refactoring from .map(...).unwrap_or(...) to .map_or(...) is more idiomatic and efficient, avoiding an intermediate Option allocation.

src/telegram/commands.rs (2)

67-67: LGTM: Appropriate #[must_use] attributes.

These attributes are well-placed on functions returning computed values (Vec<BotCommand> and CommandDescriptions) that callers should consume. This helps prevent accidental misuse where the return value is ignored.

Also applies to: 80-80


88-114: LGTM: Clearer control flow with if let.

The refactoring from a match expression to if let Some(...) else is more idiomatic for this cache-hit pattern and improves readability without changing behavior.

src/workers/server.rs (3)

34-34: LGTM: Explicit unit type in pattern match.

Changing from Ok(_) to Ok(()) makes it explicit that we're matching on the unit type, improving code clarity.


39-39: LGTM: Modern format syntax.

Using format!("{err}") instead of format!("{}", err) is the modern Rust idiom, making the code more concise while maintaining identical behavior.


113-113: LGTM: Logger initialization now synchronous.

Consistent with the synchronous logger API update applied across all entry points.

src/profanity.rs (7)

29-29: LGTM: Well-placed #[must_use] attributes.

These attributes appropriately mark functions that perform computations and return meaningful values that shouldn't be ignored:

  • check() returns profanity check results
  • should_trigger() returns a boolean decision
  • get_profine_words() returns extracted profane words
  • highlighted() returns formatted output

This improves API safety across the profanity checking module.

Also applies to: 54-54, 139-139, 160-160, 180-180


40-47: LGTM: Ergonomic IntoIterator implementation.

Implementing IntoIterator for &CheckResult enables idiomatic for-loop iteration over check results while maintaining the existing iter() method. This is a standard pattern for collection-like types.


64-68: LGTM: Inverted condition logic.

The condition has been inverted from != with swapped branches to == with flipped branches, maintaining identical behavior. This likely aligns with a Clippy pedantic lint preference for positive conditions.


111-111: LGTM: Explicit empty vector initialization.

Using vec![] is more explicit and idiomatic than Default::default() for creating an empty vector, improving code readability.


195-195: LGTM: Idiomatic collect() usage.

Using iter.collect() instead of HashSet::from_iter(iter) is more idiomatic, leveraging type inference from the function's return type signature.


316-316: LGTM: Trait method reference.

Using ToString::to_string as a method reference is more idiomatic than the closure form, consistent with Rust best practices.


331-331: LGTM: Takes ownership for Copy type.

Changing from &self to self is appropriate for TypeWrapper since it implements Copy (line 327). For Copy types, taking ownership is idiomatic and has no performance penalty since the value is duplicated rather than moved.

@vtvz vtvz merged commit bc5ce21 into master Dec 27, 2025
4 checks passed
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.

1 participant