Skip to content

Commit 08355d3

Browse files
committed
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.
1 parent a79b5be commit 08355d3

File tree

1 file changed

+18
-21
lines changed

1 file changed

+18
-21
lines changed

smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderVisitor.java

+18-21
Original file line numberDiff line numberDiff line change
@@ -238,30 +238,27 @@ private void onTraitDefinition(ShapeId target, TraitDefinition definition) {
238238
private boolean validateOnShape(ShapeId id, FromSourceLocation source) {
239239
if (!hasDefinedShape(id)) {
240240
return true;
241+
} else if (Prelude.isPreludeShape(id)) {
242+
// Ignore prelude shape conflicts since it's such a common case of
243+
// passing an already built model into a ModelAssembler.
244+
return false;
241245
}
242246

243-
// Ignore prelude shape conflicts since it's such a common case of
244-
// passing an already built model into a ModelAssembler.
245-
if (!Prelude.isPreludeShape(id)) {
246-
// The shape has been duplicated, so get the previously defined pending shape or built shape.
247-
SourceLocation previous = Optional.<FromSourceLocation>ofNullable(pendingShapes.get(id))
248-
.orElseGet(() -> builtShapes.get(id)).getSourceLocation();
249-
// Cannot ignore duplicate member definitions.
250-
boolean canIgnore = !id.getMember().isPresent()
251-
// Ignore duplicate shapes defined in the same file.
252-
&& previous != SourceLocation.NONE
253-
&& previous.equals(source.getSourceLocation());
254-
if (canIgnore) {
255-
LOGGER.warning(() -> "Ignoring duplicate shape definition defined in the same file: "
256-
+ id + " defined at " + source.getSourceLocation());
257-
} else {
258-
String message = String.format("Duplicate shape definition for `%s` found at `%s` and `%s`",
259-
id, previous.getSourceLocation(), source.getSourceLocation());
260-
throw new SourceException(message, source);
261-
}
262-
}
247+
// The shape has been duplicated, so get the previously defined pending shape or built shape.
248+
SourceLocation previous = Optional.<FromSourceLocation>ofNullable(pendingShapes.get(id))
249+
.orElseGet(() -> builtShapes.get(id)).getSourceLocation();
263250

264-
return false;
251+
// Ignore duplicate shapes defined in the same file (this can happen
252+
// when the same file is included multiple times in a model assembler).
253+
if (previous != SourceLocation.NONE && previous.equals(source.getSourceLocation())) {
254+
LOGGER.warning(() -> "Ignoring duplicate shape definition defined in the same file: "
255+
+ id + " defined at " + source.getSourceLocation());
256+
return false;
257+
} else {
258+
String message = String.format("Duplicate shape definition for `%s` found at `%s` and `%s`",
259+
id, previous.getSourceLocation(), source.getSourceLocation());
260+
throw new SourceException(message, source);
261+
}
265262
}
266263

267264
/**

0 commit comments

Comments
 (0)