Skip to content

reason inside #[allow]#7049

Merged
youknowone merged 1 commit intoRustPython:mainfrom
youknowone:allow-reasons
Feb 8, 2026
Merged

reason inside #[allow]#7049
youknowone merged 1 commit intoRustPython:mainfrom
youknowone:allow-reasons

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Feb 8, 2026 •
edited by coderabbitai bot
Loading

Summary by CodeRabbit

  • Chores
    • Enhanced code quality with comprehensive lint attribute documentation and reasoning across the codebase.
    • Optimized internal sorting implementation for improved clarity.
    • Refined lint configuration with explicit justifications for all suppressions.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 8, 2026 •
edited
Loading

📝 Walkthrough

Walkthrough

This PR systematically adds reason annotations to existing Clippy lint suppressions across 19 crates, improving lint documentation with explicit justifications. Additionally, minor functional changes include adding a get_thread_id field to a lock primitive, switching a sort method, and removing a lint suppression.

Changes

Cohort / File(s) Summary
Lock and Synchronization Primitives
crates/common/src/lock/cell_lock.rs, crates/common/src/lock/thread_mutex.rs, crates/common/src/signal.rs		 Expanded clippy allow attributes with reasons on lock/sync primitives. Added get_thread_id: G field to RawThreadMutex and updated thread-ID checks to use it; no signature changes.
Clippy Lint Annotation Updates
crates/codegen/src/compile.rs, crates/common/src/str.rs, crates/sre_engine/src/engine.rs, crates/stdlib/src/json/machinery.rs, crates/stdlib/src/overlapped.rs, crates/stdlib/src/statistics.rs, crates/vm/src/builtins/module.rs, crates/vm/src/builtins/str.rs, crates/vm/src/exceptions.rs, crates/vm/src/signal.rs, crates/vm/src/stdlib/ctypes/base.rs, crates/vm/src/stdlib/winapi.rs		 Converted single-line clippy allow attributes to multi-line forms with explanatory reason parameters, improving lint documentation without behavioral changes.
Macro and Attribute Enhancements
crates/vm/src/macros.rs		 Added reason annotations to match_class and identifier_utf8 macro arms, clarifying lint suppressions in macro contexts.
Lint Suppression Refinement
crates/vm/src/object/core.rs		 Removed non_send_fields_in_send_ty suppression from PyWeak threading config; added reason to diverging_sub_expression allowance in partially_init macro.
Build Script and Sorting Updates
crates/stdlib/build.rs, crates/derive-impl/src/util.rs		 Added clippy reasons for byte-grouping suppressions in ossl_vers array and ssl-openssl config. Changed sort_by comparator to sort_by_key for ValidatedItemNursery items.
Lint Suppression Removal
crates/vm/src/builtins/function.rs		 Removed needless_match suppression from PyBoundMethod::repr_str, enabling lint checking.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • RustPython/RustPython#6961: Both PRs modify pattern handling in crates/codegen/src/compile.rs and touch the mapping-pattern area with lint attribute adjustments.

Suggested reviewers

  • ShaharNaveh
  • arihant2math

Poem

🐰 Lint reasons now rest where lint warnings once cried,
Each suppress adorned with a "why" by its side,
From locks to macros, from builds to the thread,
Clippy's confusions are carefully shed! ✨

🚥 Pre-merge checks | ✅ 3
✅ 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 'reason inside #[allow]' directly describes the main change across all modified files: adding reason annotations to Clippy lint allow attributes throughout the codebase.
Docstring Coverage ✅ Passed Docstring coverage is 92.31% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉

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.

ShaharNaveh
ShaharNaveh approved these changes Feb 8, 2026
View reviewed changes
Copy link
Contributor

@ShaharNaveh ShaharNaveh left a comment

Choose a reason for hiding this comment

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

This is good, I think a good next step will be to replace "allow" with "expect". this will let us remove any unused lines

@youknowone youknowone marked this pull request as ready for review February 8, 2026 12:44
@youknowone youknowone enabled auto-merge (squash) February 8, 2026 12:54
@youknowone youknowone merged commit 5dabad6 into RustPython:main Feb 8, 2026
13 checks passed
@youknowone youknowone deleted the allow-reasons branch February 8, 2026 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ShaharNaveh ShaharNaveh ShaharNaveh approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@youknowone @ShaharNaveh