Skip to content

Node null check fix #2669

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

lewisjkl
Copy link
Contributor

Background

  • What do these changes do?
    Updates such that the node validator will not raise a false-positive related to a Document value being null.
  • Why are they important?
    Currently causing an error to flag in one of our projects where a trait has a Map shape with a target type of Document.

Testing

  • How did you test these changes?
    See unit test included.

Links

  • Links to additional context, if necessary
  • Issue #, if applicable (see here for a list of keywords to use for linking issues)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@haydenbaker
Copy link
Contributor

haydenbaker commented Jun 19, 2025

While this seems like a simple change, it would have some unintended consequences (i.e. type signature changes) in some smithy-based code-generators. Though I understand the intent, I also don't think we should allow for nulls in a list/map that isn't marked with sparse. Which leads me to your PR adding sparse trait to your shape, which is the right approach here.

@haydenbaker haydenbaker added the closing-soon This issue will automatically close in 7 days unless further comments are made. label Jun 19, 2025
@lewisjkl
Copy link
Contributor Author

I can understand that position, I think maybe the documentation for Document could be updated to remove mention that "null" is a valid document value. As far as I can tell, "null" for Documents is not treated any differently than it is for a String/Int or any other primitive.

"Document types are represented by a JSON-like data model and can contain UTF-8 strings, arbitrary precision numbers, booleans, nulls, a list of these values, and a map of UTF-8 strings to these values."

That's the passage that made me infer document had a special handling of Null compared to other primitives (and the fact that this used to work, we've had this same test case for years now)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closing-soon This issue will automatically close in 7 days unless further comments are made.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants