Skip to content

#[builder(with = Some / FromIterator::from_iter / <_>::from_iter)] attribute#157

Merged
Veetaha merged 10 commits intomasterfrom
feat/attr-with-well-known-functions
Oct 26, 2024
Merged

#[builder(with = Some / FromIterator::from_iter / <_>::from_iter)] attribute#157
Veetaha merged 10 commits intomasterfrom
feat/attr-with-well-known-functions

Conversation

@Veetaha
Copy link
Copy Markdown
Collaborator

@Veetaha Veetaha commented Oct 25, 2024

Closes #22.

Although.. the issue above was already closed by #145 where the attribute #[builder(with)] was introduced. However, this PR adds the convenience shortcut #[builder(with = FromIterator::from_iter)] or #[builder(with = <_>::from_iter)].

The part of #[builder(with = Some)] of this PR closes #35, or more specifically the remaining item described in #35 (comment)

Summary by CodeRabbit

  • New Features

    • Enhanced error reporting for builder attributes, providing clearer guidance on usage.
    • Support for overriding visibility for builder structs and functions.
    • Improved rustdoc output indicating member requirements and optionality.
    • Introduction of graceful internal panic handling for macro errors.
  • Bug Fixes

    • Stricter validation for attributes, including rejection of unnecessary empty attributes.
    • Resolved various lint issues to enhance developer experience.
  • Documentation

    • Updated changelog to reflect significant changes and improvements in version 2.3.0.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 25, 2024

Walkthrough

The changes in this pull request address multiple error messages related to the usage of builder attributes in Rust code, specifically for #[builder(transparent)] and #[builder(with = ...)]. The updates clarify the correct syntax and constraints for these attributes, enhancing error reporting. Additionally, the changelog has been updated to reflect significant changes in version 2.3.0, including stricter validation for builder attributes and the removal of deprecated macros.

Changes

File Change Summary
bon/tests/integration/ui/compile_fail/attr_transparent.stderr Introduces error messages for incorrect usage of #[builder(transparent)] attribute, including misuse of attributes and method availability issues.
bon/tests/integration/ui/compile_fail/attr_with.stderr Updates error messages related to #[builder(with = ...)], clarifying syntax, return types, and type mismatches.
website/changelog.md Updates changelog for version 2.3.0, detailing stricter validation, new features, and removal of deprecated attributes.

Assessment against linked issues

Objective Addressed Explanation
Option to completely customize input type of builder methods (#22) The changes do not implement customizable input types for builder methods.
Allow specifying mandatory Option argument (#35) The changes do not address the requirement for mandatory Option arguments.

Possibly related PRs

Suggested reviewers

  • anelson

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 1f11639 and c469cf8.

⛔ Files ignored due to path filters (6)
  • bon-macros/src/builder/builder_gen/member/config/mod.rs is excluded by !**/*.rs
  • bon-macros/src/builder/builder_gen/setters/mod.rs is excluded by !**/*.rs
  • bon/tests/integration/builder/attr_with/from_iter.rs is excluded by !**/*.rs
  • bon/tests/integration/builder/attr_with/mod.rs is excluded by !**/*.rs
  • bon/tests/integration/builder/attr_with/some.rs is excluded by !**/*.rs
  • bon/tests/integration/ui/compile_fail/attr_with.rs is excluded by !**/*.rs
📒 Files selected for processing (2)
  • bon/tests/integration/ui/compile_fail/attr_with.stderr (2 hunks)
  • website/changelog.md (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • bon/tests/integration/ui/compile_fail/attr_with.stderr
🔇 Additional comments (1)
website/changelog.md (1)

26-26: LGTM! The changelog entry accurately documents the changes.

The entry clearly documents the new convenience shortcuts added in PR #157, which address:


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
🪧 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>, please review it.
    • 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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @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 using 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.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration 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.

Copy link
Copy Markdown
Contributor

@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: 1

🧹 Outside diff range and nitpick comments (1)
bon/tests/integration/ui/compile_fail/attr_with.stderr (1)

Line range hint 1-61: Consider adding examples for Result usage

While the error messages are comprehensive, consider enhancing them with a concrete example of correct Result usage to complement the existing examples.

Example addition to the first error message:

 error: expected a closure e.g. `#[builder(with = |param: T| expression)]` or a well-known function path which could be one of:
        - #[builder(with = Some)]
        - #[builder(with = FromIterator::from_iter)]
        - #[builder(with = <_>::from_iter)] (same as above, but shorter)
+       - #[builder(with = |x: T| -> MyResult<_> { Ok(x) })] (for fallible conversions)
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 04cda8f and 1f11639.

⛔ Files ignored due to path filters (8)
  • bon-macros/src/builder/builder_gen/member/config/mod.rs is excluded by !**/*.rs
  • bon-macros/src/builder/builder_gen/member/config/with/closure.rs is excluded by !**/*.rs
  • bon-macros/src/builder/builder_gen/member/config/with/mod.rs is excluded by !**/*.rs
  • bon-macros/src/builder/builder_gen/mod.rs is excluded by !**/*.rs
  • bon-macros/src/builder/builder_gen/setters/mod.rs is excluded by !**/*.rs
  • bon/tests/integration/builder/attr_with/mod.rs is excluded by !**/*.rs
  • bon/tests/integration/builder/attr_with/some.rs is excluded by !**/*.rs
  • bon/tests/integration/ui/compile_fail/attr_transparent.rs is excluded by !**/*.rs
📒 Files selected for processing (3)
  • bon/tests/integration/ui/compile_fail/attr_transparent.stderr (1 hunks)
  • bon/tests/integration/ui/compile_fail/attr_with.stderr (1 hunks)
  • website/changelog.md (1 hunks)
🔇 Additional comments (8)
bon/tests/integration/ui/compile_fail/attr_transparent.stderr (4)

Line range hint 1-24: LGTM! Clear and helpful error messages for transparent attribute.

The error messages effectively communicate the constraints for #[builder(transparent)]:

  • Restricted to Option<T> types
  • Incompatible with start_fn, finish_fn, and skip attributes

26-52: LGTM! Helpful error messages with suggestions for method names.

The error messages effectively guide users by:

  • Clearly indicating which methods don't exist
  • Providing helpful suggestions for the correct method names

54-71: LGTM! Comprehensive error message for unset builder member.

The error message effectively communicates the build failure by:

  • Clearly stating which member is unset
  • Explaining the trait implementation requirement
  • Providing proper macro context

Line range hint 1-71: Verify error message testing coverage.

While the error messages look good, let's ensure we have test coverage for all possible error scenarios with the new builder attribute features.

✅ Verification successful

Error message testing coverage is comprehensive

The test coverage for builder attribute error cases is thorough and well-structured:

  • attr_transparent.rs covers all invalid combinations:
    • Using transparent on required (non-Option) members
    • Conflicts with start_fn
    • Conflicts with finish_fn
    • Conflicts with skip attribute
    • Missing setter method errors

Additional error cases are covered in separate compile_fail tests:

  • attr_with.rs for with-attribute errors
  • attr_skip.rs for skip-attribute errors
  • attr_top_level_start_finish_fn.rs for start/finish function errors

The positive test cases in builder/attr_transparent.rs complement these by verifying correct usage scenarios.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for comprehensive error message testing
echo "Checking for all error test cases..."

# Look for other compile_fail tests related to builder attributes
fd -e rs . tests/integration/ui/compile_fail/ --exec grep -l "builder.*with.*Some\|builder.*with.*FromIterator\|builder.*transparent"

# Look for test cases in the source
rg "#\[test\].*builder.*with" tests/

Length of output: 426


Script:

#!/bin/bash
# Let's first find all compile_fail test files
fd -e rs . -t f

# Then look for builder-related test files
rg -l "derive.*Builder" 

# And specifically look for transparent attribute tests
rg -l "#\[builder.*transparent\]"

Length of output: 9393


Script:

#!/bin/bash
# Let's examine the test coverage more specifically

# Check compile_fail tests for transparent attribute
rg -C 5 "transparent" bon/tests/integration/ui/compile_fail/attr_transparent.rs

# Check positive test cases for transparent attribute
rg -C 5 "transparent" bon/tests/integration/builder/attr_transparent.rs

# Look for other builder attribute combinations in compile_fail tests
rg -l "builder.*with|builder.*skip|builder.*start_fn|builder.*finish_fn" bon/tests/integration/ui/compile_fail/

Length of output: 5773

bon/tests/integration/ui/compile_fail/attr_with.stderr (4)

1-5: Excellent error message improvements for basic usage!

The error message now clearly documents the three supported patterns for the with attribute:

  1. #[builder(with = Some)]
  2. #[builder(with = FromIterator::from_iter)]
  3. #[builder(with = <_>::from_iter)]

This aligns perfectly with the PR objectives and provides clear guidance to users.


Line range hint 11-41: Clear restrictions on attribute combinations and keywords

The error messages effectively prevent:

  1. Combining with and into attributes
  2. Using for<...> lifetime syntax
  3. Using restricted keywords (const, static, async, move)

This helps maintain clean and predictable builder patterns.


Line range hint 47-79: Comprehensive guidance for return type annotations

The error messages provide excellent documentation about return type options:

  1. No annotation for direct type returns
  2. -> *Result<_, ErrorType> for fallible setters
  3. Support for custom Result types (e.g., ApiResult<_>)

The messages also clearly explain the underlying type concept with respect to Option<T> and #[builder(transparent)].


Line range hint 54-61: Type mismatch errors maintain Rust's high standard

The error messages preserve Rust's excellent type error reporting:

  1. Clear indication of expected vs found types
  2. Helpful notes about type parameter influence
  3. Precise source locations for type mismatches

Comment thread website/changelog.md
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.

Allow specifying mandatory Option argument option to completely customise input type of builder methods

2 participants