added missing_debug_implementations lint

[g4titanx]

Description:
added the missing_debug_implementations lint which detects missing implementations of #[deive(Debug)] for public types.

Related issue: #1841

[glihm]

Hey @g4titanx, thanks for the great work here!

In the issue comment you mention that:

I was having trouble making some pub types impl the debug trait

Do you still have some public types missing, or you managed to do all the public types? Would you mind pointing to the types you had difficulties to?

Thank you!

[g4titanx]

Hey @g4titanx, thanks for the great work here!

In the issue comment you mention that:

I was having trouble making some pub types impl the debug trait

Do you still have some public types missing, or you managed to do all the public types? Would you mind pointing to the types you had difficulties to?

Thank you!

yeah, check:
line 21 in crates/torii/core/src/engine.rs
line 27 in crates/katana/storage/provider/src/providers/in_memory/state.rs

and think I covered the rest

[g4titanx]

@glihm

[glihm]

Thank you @g4titanx for the effort here!
So, for the missing types, it is related to the traits bounds.

Let's take this trait:

pub trait EventProcessor<P>
where
    P: Provider + Sync,
{...}

There's nothing that bounds P to the Debug trait. As there are some dyn used, the compiler must also know that the types implement Debug.

pub trait EventProcessor<P>
where
    P: Provider + Sync + std::fmt::Debug,
{...}

This is what's missing.

So you may now have to track the traits and ensure the std::fmt::Debug bound is always present.

This is why for instance in Processor type in engine.rs you couldn't have #[derive(Debug)] working.

If you have any question please don't hesitate.

[g4titanx]

Thank you @g4titanx for the effort here! So, for the missing types, it is related to the traits bounds.

Let's take this trait:

pub trait EventProcessor<P>
where
    P: Provider + Sync,
{...}

There's nothing that bounds P to the Debug trait. As there are some dyn used, the compiler must also know that the types implement Debug.

pub trait EventProcessor<P>
where
    P: Provider + Sync + std::fmt::Debug,
{...}

This is what's missing.

So you may now have to track the traits and ensure the std::fmt::Debug bound is always present.

This is why for instance in Processor type in engine.rs you couldn't have #[derive(Debug)] working.

If you have any question please don't hesitate.

oh great! i would make the changes

[g4titanx]

hi @glihm , in the recent commit i made, i added the Debug traits to the trait bounds and i am still getting a compiler error that says the types that implement dyn in the Processors type in engine.rs are missing a Debug implementation.
but that's wrong right? cause i just fixed it.

also, i am getting a conflicting implementation of traits std::fmt::Debug for type BlockProducerMetrics error in line 9 of crates/katana/core/src/service/metrics.rs. i would love an explanation on that

[glihm]

also, i am getting a conflicting implementation of traits std::fmt::Debug for type BlockProducerMetrics error in line 9 of crates/katana/core/src/service/metrics.rs. i would love an explanation on that

Metrics is already deriving Debug I guess, you can try to remove Debug as it may already be present then. 👍

hi @glihm , in this recent commit i made, i added the Debug traits to the trait bounds and i am still getting a compiler error that says the types that implement dyn in the Processors type in engine.rs are missing a Debug implementation.
but that's wrong right? cause i just fixed it.

Will check, but did you add the trait bound to all other possible related traits?

[g4titanx]

Will check, but did you add the trait bound to all other possible related traits?

yeah, I added it to all related traits. you can check the latest commit

Metrics is already deriving Debug I guess, you can try to remove Debug as it may already be present then. 👍

okay, I am gonna try this now

[g4titanx]

hi @glihm , in the recent commit i made, i added the Debug traits to the trait bounds and i am still getting a compiler error that says the types that implement dyn in the Processors type in engine.rs are missing a Debug implementation. but that's wrong right? cause i just fixed it.

i think the issue here stems from how Rust treats dynamic dispatch trait objects. for example, dyn TransactionProcessor<P> being used as a field in Processors struct, the compiler expects that the trait object itself implements all the methods defined by its trait, including those required by Debug. but, the Debug implementation is tied to the specific types that implement TransactionProcessor, not the trait object itself.
therefore, a solution would be to ensure that the dyn trait object specifies that it implements Debug.
one way to do that would be to create a wrapper structs around each trait objects declared in the Processors struct, then implement Debug for each wrapper.

what do you think? that's about the only error i am getting right now tho. no error with Metrics so far.

[glihm]

Thank you for the feedback and researches on that!

what do you think? that's about the only error i am getting right now tho. no error with Metrics so far.

@tcoratger may have other experience with that? On my end, it seems the only way with dyn is as you mentioned having the bound that enforce the type to implement debug, which can be verbose for sure, but can be ok for a first pass.

Could you update the PR by only enforcing the trait bound and see how it looks? To avoid creating a wrapper in a first time and only having the bound check passing. WDYT?

[g4titanx]

Thank you for the feedback and researches on that!

what do you think? that's about the only error i am getting right now tho. no error with Metrics so far.

you are welcome!

@tcoratger may have other experience with that? On my end, it seems the only way with dyn is as you mentioned having the bound that enforce the type to implement debug, which can be verbose for sure, but can be ok for a first pass.

Could you update the PR by only enforcing the trait bound and see how it looks? To avoid creating a wrapper in a first time and only having the bound check passing. WDYT?

i did that in my local repo, the error was persistent. i will enforce on the trait bound and push, so you can see it

[coderabbitai]

Important

Review skipped

More than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review.

50 files out of 106 files are above the max files limit of 50. Please upgrade to Pro plan to get higher limits.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[glihm]

Thanks for the work here @g4titanx and your time on that. 🙏

I've merged main and added missing Debug. I've also tried to solution at trait level where Debug is required, but this may require a bigger change as we didn't build with this in mind.

So currently, there's several types with #[allow(missing_debug_implementation)] when it comes to futures or external types not having Debug.

@kariy @lambda-0x we may refactor in the future to gate Debug under a feature to not generate too much code when not required.

For now, I suppose the PR in it's state is ok to be reviewed and possibly merged to at least ensure most of the types derive Debug.

[codecov]

Codecov Report

Attention: Patch coverage is 58.33333% with 5 lines in your changes missing coverage. Please review.

Project coverage is 68.00%. Comparing base (7c09e4a) to head (3d07b9e).

Files	Patch %	Lines
crates/sozo/ops/src/account.rs	50.00%	3 Missing ⚠️
crates/torii/core/src/lib.rs	0.00%	1 Missing ⚠️
crates/torii/core/src/types.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2016   +/-   ##
=======================================
  Coverage   67.99%   68.00%           
=======================================
  Files         331      331           
  Lines       42679    42679           
=======================================
+ Hits        29021    29024    +3     
+ Misses      13658    13655    -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kariy]

lgtm

[g4titanx]

Thanks for the work here @g4titanx and your time on that. 🙏

I've merged main and added missing Debug. I've also tried to solution at trait level where Debug is required, but this may require a bigger change as we didn't build with this in mind.

So currently, there's several types with #[allow(missing_debug_implementation)] when it comes to futures or external types not having Debug.

@kariy @lambda-0x we may refactor in the future to gate Debug under a feature to not generate too much code when not required.

For now, I suppose the PR in it's state is ok to be reviewed and possibly merged to at least ensure most of the types derive Debug.

great!
thank you for your support @glihm
i look forward to working on more issues