Skip to content

feat(source-generators): opt-in base class property mapping#68

Merged
ncipollina merged 1 commit intomainfrom
feat/include-base-class-properties
Feb 9, 2026
Merged

feat(source-generators): opt-in base class property mapping#68
ncipollina merged 1 commit intomainfrom
feat/include-base-class-properties

Conversation

@ncipollina
Copy link
Contributor


🚀 Pull Request

📋 Summary

Briefly describe what this PR does and why it's needed.


✅ Checklist

  • My changes build cleanly
  • I’ve added/updated relevant tests
  • I’ve added/updated documentation or README
  • I’ve followed the coding style for this project
  • I’ve tested the changes locally (if applicable)

🧪 Related Issues or PRs

Closes #...


💬 Notes for Reviewers

Any specific areas to look at, known issues, or follow-up work.


Add IncludeBaseClassProperties to DynamoMapperAttribute and generator options to include inherited properties for root models, nested inline objects, and dot-notation validation. Update docs and add an inheritance example and generator snapshot tests. Bump package versions.
@ncipollina ncipollina requested a review from j-d-ha February 9, 2026 19:21
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 660faf96f3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +65 to +68
var property = current
.GetMembers()
.OfType<IPropertySymbol>()
.FirstOrDefault(p => p.Name == name);

Choose a reason for hiding this comment

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

P2 Badge Ignore non-mappable derived properties in base lookup

With IncludeBaseClassProperties = true, dot-notation validation calls FindPropertyByName and will return the first property name match even if it’s non-mappable (e.g., static/indexer). This lets a derived non-mappable property shadow a base instance property, causing valid paths like Address.Line1 to fail validation even though the base Address property is the one actually mapped. This regression only appears with base-property opt-in plus a same-named non-mappable member on the derived type; consider applying the same mappability predicate (or at least skipping static/indexers) when resolving the path so base properties remain reachable.

Useful? React with 👍 / 👎.

Copy link
Collaborator

@j-d-ha j-d-ha left a comment

Choose a reason for hiding this comment

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

🚀 Love this!

@ncipollina ncipollina merged commit 9d80a58 into main Feb 9, 2026
3 checks passed
@ncipollina ncipollina deleted the feat/include-base-class-properties branch February 9, 2026 19:39
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.

2 participants

Comments