Required trait removal doesn't emit any events - should it?

[kubukoz]

Hi!

I bumped into this: if you merge two models with a member that's @required in one but not in the other, the merged model will always make the field required.

While this behavior is consistent with the usual handling of traits in model merging, shouldn't this emit some sort of event, whether it be a NOTE or WARN? I imagine some code might misbehave if you merge things like these:

structure Greeting {
  from: String,
  @required to: String
}

structure Greeting {
  @required from: String,
  to: String
}

[mtdowling]

The title of the issue seems off here right? The required trait isn't removed, it's merged into the resulting member and kept. There's a warning logged for the Greeting shape, but nothing about the member. This warning is only logged when the conflict is allowed:

WARNING: Ignoring duplicate but equivalent shape definition: example#Greeting defined at b.smithy [3, 1] and a.smithy [3, 1]

I'm not sure that we can detect trait deltas between two merged members. When model files are loaded, shapes are essentially broken into pieces and later reassembled with the merging process. In a way, all traits applied to shapes/members basically become apply statements. When Greeting from model B is loaded, there's no notion of a fully formed shape or members with traits from model A's Greeting shape to compare against.

Maybe we could turn that logged warning into a validation event, to bring more attention to this.

[kubukoz]

The required trait isn't removed, it's merged into the resulting member and kept.

Right, what I mean is that I removed it between model versions and it still ends up in the final model once the old and new version are merged :)