Skip to content

Implement Display for nested shapes referenced in error messages #4081

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

drganjoo
Copy link
Contributor

@drganjoo drganjoo commented Apr 3, 2025

This PR addresses the issue where shapes annotated with @error can reference other complex shapes in their message fields, which previously lacked Display implementations.

Changes

  • Added a model transformation (AddSyntheticTraitForImplDisplay) to identify all shapes reachable from error shapes
  • Added a synthetic trait to mark shapes that need Display implementations
  • Modified code generation to implement Display for all shapes reachable from error shapes
  • Ensured consistent formatting for both optional and non-optional fields
  • Added tests to verify correct Display implementation for nested error structures

Implementation Details

  • The transformation uses a directed graph walker to find all shapes reachable from error shapes
  • Only adds implementations to shapes that don't already have the @error trait
  • Formats all fields consistently, including handling of Option<T> values
  • Maintains the standard "StructName { field=value }" format across all generated Display implementations

Testing

  • Added test cases for various error structures with different levels of nesting
  • Verified that both simple and complex error shapes correctly implement Display
  • Confirmed proper handling of optional fields

Fixes #4080

@drganjoo drganjoo force-pushed the fahadzub/impldisplay branch 3 times, most recently from e526dc3 to 1a8f04d Compare April 3, 2025 12:01
@drganjoo drganjoo force-pushed the fahadzub/impldisplay branch from 1a8f04d to 024d02a Compare April 3, 2025 12:11
Copy link

github-actions bot commented Apr 3, 2025

Copy link

github-actions bot commented Apr 3, 2025

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.

Error shapes with structure members don't implement Display for referenced structures
2 participants