Skip to content

style(lib): remove default_trait_access lint#4111

Open
nakaryo716 wants to merge 1 commit into
hyperium:masterfrom
nakaryo716:lint-default-trait-access
Open

style(lib): remove default_trait_access lint#4111
nakaryo716 wants to merge 1 commit into
hyperium:masterfrom
nakaryo716:lint-default-trait-access

Conversation

@nakaryo716

@nakaryo716 nakaryo716 commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Overview

This PR removes default_trait_access = "allow" from Cargo.toml and updates the code to explicitly call type-specific default methods (e.g., ParserConfig::default()) instead of Default::default().

Changes

  • Removed default_trait_access = "allow" from Cargo.toml.
  • Replaced Default::default() with explicit type-default calls in code.
  • Refactored the httparse import in src/server/conn/http1.rs to align with the client-side style.

Questions

Full path for h2 config

In src/client/conn/http2.rs and src/server/conn/http2.rs, I used fully qualified paths to avoid Default::default() while preventing name conflicts:

h2_builder: proto::h2::client::Config::default(),

Question:
Does this look good to you, or would you prefer introducing type aliases (e.g., use proto::h2::client::Config as H2ClientConfig) at the top of the file to keep it shorter?

Import refactoring in src/server/conn/http1.rs

I changed use httparse; to use httparse::ParserConfig;

If you prefer keeping the original import style to minimize the diff, please let me know. I will revert this specific part.

Untouched test codes

I noticed that there are still many Default::default() occurrences in the test codes (e.g., src/proto/h1/role.rs). Currently, the CI clippy job does not check test targets because it runs without --all-targets:

cargo clippy --features full -- -D warnings

As a result, the CI passes with this PR.

Question:
Should I also update the test codes, or should we keep this PR focused only on the library code?

Related issue

Part of a #4071

Sorry for the long description!
If there are any changes needed, please let me know. Otherwise, feel free to merge this PR if everything looks good to you.🙏

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