From cc87d0e3ec12e7749fe6cfc62834208f116c01bc Mon Sep 17 00:00:00 2001 From: Michael Dowling Date: Wed, 29 Jul 2020 10:18:38 -0700 Subject: [PATCH] Simplify conflict shape loading conflict handling There was a check in the loader visitor around duplicate members that is no longer necessary now that duplicate members are detected while loading aggregate shapes. Beyond the removal of that if statement, this change improves the readability of the conflict handling code and gives more context as to why some conflicts are allowed. --- .../smithy/model/loader/LoaderVisitor.java | 39 +++++++++---------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderVisitor.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderVisitor.java index b2340715f9c..aa9f7369eea 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderVisitor.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderVisitor.java @@ -238,30 +238,27 @@ private void onTraitDefinition(ShapeId target, TraitDefinition definition) { private boolean validateOnShape(ShapeId id, FromSourceLocation source) { if (!hasDefinedShape(id)) { return true; + } else if (Prelude.isPreludeShape(id)) { + // Ignore prelude shape conflicts since it's such a common case of + // passing an already built model into a ModelAssembler. + return false; } - // Ignore prelude shape conflicts since it's such a common case of - // passing an already built model into a ModelAssembler. - if (!Prelude.isPreludeShape(id)) { - // The shape has been duplicated, so get the previously defined pending shape or built shape. - SourceLocation previous = Optional.ofNullable(pendingShapes.get(id)) - .orElseGet(() -> builtShapes.get(id)).getSourceLocation(); - // Cannot ignore duplicate member definitions. - boolean canIgnore = !id.getMember().isPresent() - // Ignore duplicate shapes defined in the same file. - && previous != SourceLocation.NONE - && previous.equals(source.getSourceLocation()); - if (canIgnore) { - LOGGER.warning(() -> "Ignoring duplicate shape definition defined in the same file: " - + id + " defined at " + source.getSourceLocation()); - } else { - String message = String.format("Duplicate shape definition for `%s` found at `%s` and `%s`", - id, previous.getSourceLocation(), source.getSourceLocation()); - throw new SourceException(message, source); - } - } + // The shape has been duplicated, so get the previously defined pending shape or built shape. + SourceLocation previous = Optional.ofNullable(pendingShapes.get(id)) + .orElseGet(() -> builtShapes.get(id)).getSourceLocation(); - return false; + // Ignore duplicate shapes defined in the same file (this can happen + // when the same file is included multiple times in a model assembler). + if (previous != SourceLocation.NONE && previous.equals(source.getSourceLocation())) { + LOGGER.warning(() -> "Ignoring duplicate shape definition defined in the same file: " + + id + " defined at " + source.getSourceLocation()); + return false; + } else { + String message = String.format("Duplicate shape definition for `%s` found at `%s` and `%s`", + id, previous.getSourceLocation(), source.getSourceLocation()); + throw new SourceException(message, source); + } } /**