Skip to content

Commit 126b861

Browse files
committed
Move the validation into the for loop body in method to avoid two loops.
1 parent 5f7860d commit 126b861

File tree

1 file changed

+20
-22
lines changed

1 file changed

+20
-22
lines changed

smithy-model/src/main/java/software/amazon/smithy/model/transform/RemoveShapes.java

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import java.util.Collection;
1010
import java.util.HashSet;
1111
import java.util.List;
12-
import java.util.Optional;
1312
import java.util.Set;
1413
import java.util.logging.Logger;
1514
import software.amazon.smithy.model.Model;
@@ -30,11 +29,11 @@
3029
* containing shape rather than the member.
3130
*/
3231
final class RemoveShapes {
32+
private static final Logger LOGGER = Logger.getLogger(RemoveShapes.class.getName());
33+
3334
private final Collection<? extends Shape> toRemove;
3435
private final List<ModelTransformerPlugin> plugins;
3536

36-
private static final Logger LOGGER = Logger.getLogger(RemoveShapes.class.getName());
37-
3837
RemoveShapes(Collection<? extends Shape> toRemove, List<ModelTransformerPlugin> plugins) {
3938
this.toRemove = toRemove;
4039
this.plugins = plugins;
@@ -45,8 +44,8 @@ Model transform(ModelTransformer transformer, Model model) {
4544

4645
// Iteratively add each shape that needs to be removed from the index using multiple rounds.
4746
Set<Shape> removed = new HashSet<>(toRemove);
48-
validateMembersFromMixin(model, removed);
4947
for (Shape removedShape : toRemove) {
48+
validateShapeCopiedFromMixin(model, removedShape);
5049
builder.removeShape(removedShape.getId());
5150
// We don't need to remove members from the builder since
5251
// members are automatically removed with the container.
@@ -62,25 +61,24 @@ Model transform(ModelTransformer transformer, Model model) {
6261
return result;
6362
}
6463

65-
private void validateMembersFromMixin(Model model, Collection<? extends Shape> shapesToExclude) {
66-
for (Shape shape : shapesToExclude) {
67-
// Only validate MemberShapes
68-
if (!shape.isMemberShape()) {
69-
continue;
70-
}
71-
MemberShape memberShape = shape.asMemberShape().get();
72-
Optional<Shape> container = model.getShape(memberShape.getContainer());
73-
if (container.isPresent()) {
74-
for (ShapeId mixinId : container.get().getMixins()) {
75-
Shape mixinShape = model.expectShape(mixinId);
76-
if (mixinShape.getMemberNames().contains(shape.getId().getMember().get())) {
77-
LOGGER.warning(format("Removing mixed in member `%s` from mixin shape `%s` "
64+
private void validateShapeCopiedFromMixin(Model model, Shape shape) {
65+
// Only validate MemberShapes
66+
if (!shape.isMemberShape()) {
67+
return;
68+
}
69+
MemberShape memberShape = shape.asMemberShape().get();
70+
Shape container = model.getShape(memberShape.getContainer())
71+
.orElseThrow(() -> new ModelTransformException(
72+
format("Cannot find the container shape for member `%s`.",
73+
memberShape.getMemberName())));
74+
for (ShapeId mixinId : container.getMixins()) {
75+
Shape mixinShape = model.expectShape(mixinId);
76+
if (mixinShape.getMemberNames().contains(shape.getId().getMember().get())) {
77+
LOGGER.warning(format("Removing mixed in member `%s` from mixin shape `%s` "
7878
+ "in `%s` will result in an inconsistent model.",
79-
memberShape.getMemberName(),
80-
mixinShape.getId(),
81-
container.get().getId().getName()));
82-
}
83-
}
79+
memberShape.getMemberName(),
80+
mixinShape.getId(),
81+
container.getId().getName()));
8482
}
8583
}
8684
}

0 commit comments

Comments
 (0)