Skip to content
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

Add support for validating different IDL versions #917

Merged
merged 1 commit into from
Sep 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
*/
abstract class AbstractMutableModelFile implements ModelFile {

protected final TraitContainer traitContainer;
protected TraitContainer.VersionAwareTraitContainer traitContainer;

private final Set<ShapeId> allShapeIds = new HashSet<>();
private final Map<ShapeId, AbstractShapeBuilder<?, ?>> shapes = new LinkedHashMap<>();
Expand All @@ -59,7 +59,8 @@ abstract class AbstractMutableModelFile implements ModelFile {
*/
AbstractMutableModelFile(TraitFactory traitFactory) {
this.traitFactory = Objects.requireNonNull(traitFactory, "traitFactory must not be null");
traitContainer = new TraitContainer.TraitHashMap(traitFactory, events);
TraitContainer traitStore = new TraitContainer.TraitHashMap(traitFactory, events);
traitContainer = new TraitContainer.VersionAwareTraitContainer(traitStore);
}

/**
Expand Down Expand Up @@ -128,6 +129,24 @@ final void onTrait(ShapeId target, Trait trait) {
traitContainer.onTrait(target, trait);
}

/**
* Sets the version of the model file being loaded.
*
* @param version Version to set.
*/
final void setVersion(Version version) {
traitContainer.setVersion(version);
}

/**
* Gets the currently defined version.
*
* @return Returns the defined version.
*/
final Version getVersion() {
return traitContainer.getVersion();
}

@Override
public final List<ValidationEvent> events() {
return events;
Expand Down Expand Up @@ -248,7 +267,7 @@ private <S extends Shape, B extends AbstractShapeBuilder<? extends B, S>> Option
}
return Optional.of(builder.build());
} catch (SourceException e) {
events.add(ValidationEvent.fromSourceException(e).toBuilder().shapeId(builder.getId()).build());
events.add(ValidationEvent.fromSourceException(e, "", builder.getId()));
resolvedTraits.clearTraitsForShape(builder.getId());
return Optional.empty();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,9 @@ enum AstModelLoader {
private static final Set<String> SERVICE_PROPERTIES = SetUtils.of(
TYPE, "version", "operations", "resources", "rename", TRAITS);

ModelFile load(TraitFactory traitFactory, ObjectNode model) {
ModelFile load(Version modelVersion, TraitFactory traitFactory, ObjectNode model) {
FullyResolvedModelFile modelFile = new FullyResolvedModelFile(traitFactory);
modelFile.setVersion(modelVersion);
LoaderUtils.checkForAdditionalProperties(model, null, TOP_LEVEL_PROPERTIES, modelFile.events());
loadMetadata(model, modelFile);
loadShapes(model, modelFile);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ final class IdlModelParser extends SimpleParser {

final ForwardReferenceModelFile modelFile;
private final String filename;
private String definedVersion;
private TraitEntry pendingDocumentationComment;

// A pending trait that also doesn't yet have a resolved trait shape ID.
Expand Down Expand Up @@ -168,10 +167,16 @@ public void ws() {

@Override
public ModelSyntaxException syntax(String message) {
String formatted = format(
"Parse error at line %d, column %d near `%s`: %s",
line(), column(), peekDebugMessage(), message);
return new ModelSyntaxException(formatted, filename, line(), column());
return syntax(null, message);
}

ModelSyntaxException syntax(ShapeId shapeId, String message) {
return ModelSyntaxException.builder()
.message(format("Parse error at line %d, column %d near `%s`: %s",
line(), column(), peekDebugMessage(), message))
.sourceLocation(filename, line(), column())
.shapeId(shapeId)
.build();
}

private void parseControlSection() {
Expand All @@ -184,7 +189,7 @@ private void parseControlSection() {
ws();

// Validation here for better error location.
if (key.equals("version") && definedVersion != null) {
if (key.equals("version") && modelFile.getVersion() != Version.UNKNOWN) {
throw syntax("Cannot define multiple versions in the same file");
}

Expand Down Expand Up @@ -212,11 +217,13 @@ private void onVersion(Node value) {
}

String parsedVersion = value.expectStringNode().getValue();
if (!LoaderUtils.isVersionSupported(parsedVersion)) {
Version resolvedVersion = Version.fromString(parsedVersion);

if (resolvedVersion == null) {
throw syntax("Unsupported Smithy version number: " + parsedVersion);
}

definedVersion = parsedVersion;
modelFile.setVersion(resolvedVersion);
}

private void parseMetadataSection() {
Expand Down Expand Up @@ -422,7 +429,7 @@ private void parseShape(List<TraitEntry> traits) {
break;
default:
// Unreachable.
throw syntax("Unexpected shape type: " + shapeType);
throw syntax(id, "Unexpected shape type: " + shapeType);
}

addTraits(id, traits);
Expand Down Expand Up @@ -477,7 +484,7 @@ private void parseMembers(ShapeId id, Set<String> requiredMembers, boolean struc
}

if (!remaining.isEmpty()) {
throw syntax("Missing required members of shape `" + id + "`: ["
throw syntax(id, "Missing required members of shape `" + id + "`: ["
+ ValidationUtils.tickedList(remaining) + ']');
}

Expand All @@ -498,15 +505,15 @@ private void parseMember(

if (defined.contains(memberName)) {
// This is a duplicate member name.
throw syntax("Duplicate member of " + parent + ": '" + memberName + '\'');
throw syntax(parent, "Duplicate member of " + parent + ": '" + memberName + '\'');
}

defined.add(memberName);
remaining.remove(memberName);

// Only enforce "allowedMembers" if it isn't empty.
if (!required.isEmpty() && !required.contains(memberName)) {
throw syntax("Unexpected member of " + parent + ": '" + memberName + '\'');
throw syntax(parent, "Unexpected member of " + parent + ": '" + memberName + '\'');
}

ws();
Expand All @@ -517,6 +524,12 @@ private void parseMember(
String target = ParserUtils.parseShapeId(this);

if (structureMember && peek() == '!') {
if (!modelFile.getVersion().supportsRequiredSugar()) {
throw syntax(memberId, String.format(
"The '!' suffix can only be used on structure members when using Smithy 2.0 or later, but "
+ "you're using version `%s`. Make `$version: \"2\"` the first line of this file.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Make `$version: "2"` the first line of this file to update" or something to indicate they're updating (pedantic i know but still)

modelFile.getVersion()));
}
// Create a synthetic Node to specify the location.
Node requiredTrait = new ObjectNode(Collections.emptyMap(), currentLocation());
expect('!');
Expand Down Expand Up @@ -553,13 +566,20 @@ private void parseStructuredShape(
// "Member `foo.baz#Foo$Baz` cannot be added to software.amazon.smithy.model.shapes.OperationShape$Builder"
modelFile.onShape(builder.id(id).source(location));

// Parse optional "with" statements to add mixins.
ws();

// Parse optional "with" statements to add mixins, but only if it's supported by the version.
if (peek() == 'w') {
expect('w');
expect('i');
expect('t');
expect('h');

if (!modelFile.getVersion().supportsMixins()) {
throw syntax(id, "Mixins can only be used with Smithy version 2 or later. "
+ "Attempted to use mixins with version `" + modelFile.getVersion() + "`.");
}

ws();
do {
String target = ParserUtils.parseShapeId(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,6 @@ static void checkForAdditionalProperties(
}
}

/**
* Checks if the given version string is supported.
*
* @param versionString Version string to check (e.g., 1, 1.0).
* @return Returns true if this is a supported model version.
*/
static boolean isVersionSupported(String versionString) {
return versionString.equals("1")
|| versionString.equals("1.0")
|| versionString.equals("1.1");
}

/**
* Create a {@link ValidationEvent} for a shape conflict.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,14 @@ static ModelFile load(
// This loader supports version 1.0 and 1.1. Support for 0.5 and 0.4 was removed in 0.10.
static ModelFile loadParsedNode(TraitFactory traitFactory, Node node) {
ObjectNode model = node.expectObjectNode("Smithy documents must be an object. Found {type}.");
StringNode version = model.expectStringMember(SMITHY);
StringNode versionNode = model.expectStringMember(SMITHY);
Version version = Version.fromString(versionNode.getValue());

if (LoaderUtils.isVersionSupported(version.getValue())) {
return AstModelLoader.INSTANCE.load(traitFactory, model);
} else {
throw new ModelSyntaxException("Unsupported Smithy version number: " + version.getValue(), version);
if (version != null) {
return AstModelLoader.INSTANCE.load(version, traitFactory, model);
}

throw new ModelSyntaxException("Unsupported Smithy version number: " + versionNode.getValue(), versionNode);
}

// Allows importing JAR files by discovering models inside of a JAR file.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
* Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
Expand All @@ -18,20 +18,76 @@
import software.amazon.smithy.model.FromSourceLocation;
import software.amazon.smithy.model.SourceException;
import software.amazon.smithy.model.SourceLocation;
import software.amazon.smithy.model.shapes.ShapeId;
import software.amazon.smithy.model.shapes.ToShapeId;
import software.amazon.smithy.utils.SmithyBuilder;

/**
* Thrown when the syntax of the IDL is invalid.
*/
public class ModelSyntaxException extends SourceException {
public class ModelSyntaxException extends SourceException implements ToShapeId {
private final ShapeId shapeId;

public ModelSyntaxException(String message, int line, int column) {
this(message, SourceLocation.NONE.getFilename(), line, column);
this(builder().message(message).sourceLocation(line, column));
}

public ModelSyntaxException(String message, String filename, int line, int column) {
this(message, new SourceLocation(filename, line, column));
this(builder().message(message).sourceLocation(filename, line, column));
}

public ModelSyntaxException(String message, FromSourceLocation sourceLocation) {
super(message, sourceLocation);
this(builder().message(message).sourceLocation(sourceLocation.getSourceLocation()));
}

private ModelSyntaxException(Builder builder) {
super(builder.message, builder.sourceLocation);
this.shapeId = builder.shapeId;
}

static Builder builder() {
return new Builder();
}

@Override
public ShapeId toShapeId() {
return shapeId;
}

static final class Builder implements SmithyBuilder<ModelSyntaxException> {
private SourceLocation sourceLocation = SourceLocation.NONE;
private ShapeId shapeId = null;
private String message;

private Builder() {}

@Override
public ModelSyntaxException build() {
SmithyBuilder.requiredState("message", message);
return new ModelSyntaxException(this);
}

Builder shapeId(ShapeId shapeId) {
this.shapeId = shapeId;
return this;
}

Builder message(String message) {
this.message = message;
return this;
}

Builder sourceLocation(FromSourceLocation sourceLocation) {
this.sourceLocation = sourceLocation.getSourceLocation();
return this;
}

Builder sourceLocation(String filename, int line, int column) {
return sourceLocation(new SourceLocation(filename, line, column));
}

Builder sourceLocation(int line, int column) {
return sourceLocation(SourceLocation.NONE.getFilename(), line, column);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
* Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
Expand Down Expand Up @@ -246,4 +246,69 @@ private Trait createTrait(ShapeId target, ShapeId traitId, Node traitValue) {
}
}
}

/**
* Performs version-specific validation on traits as they are added.
*
* <p>For example, this class will throw a {@link ModelSyntaxException} if
* the mixin trait is used in Smithy IDL 1.0.
*/
final class VersionAwareTraitContainer implements TraitContainer {
private final TraitContainer delegate;
private Version version = Version.UNKNOWN;

VersionAwareTraitContainer(TraitContainer delegate) {
this.delegate = delegate;
}

/**
* Sets the version being tracked.
*
* @param version Version to set.
*/
void setVersion(Version version) {
this.version = version;
}

/**
* Gets the currently configured version.
*
* @return Returns the configured version.
*/
Version getVersion() {
return version;
}

@Override
public Map<ShapeId, Map<ShapeId, Trait>> traits() {
return delegate.traits();
}

@Override
public Map<ShapeId, Trait> getTraitsForShape(ShapeId shape) {
return delegate.getTraitsForShape(shape);
}

@Override
public void clearTraitsForShape(ShapeId shape) {
delegate.clearTraitsForShape(shape);
}

@Override
public Map<ShapeId, Map<ShapeId, Trait>> getTraitsAppliedToPrelude() {
return delegate.getTraitsAppliedToPrelude();
}

@Override
public void onTrait(ShapeId target, Trait value) {
version.validateVersionedTrait(target, value.toShapeId(), value.toNode());
delegate.onTrait(target, value);
}

@Override
public void onTrait(ShapeId target, ShapeId traitId, Node value) {
version.validateVersionedTrait(target, traitId, value);
delegate.onTrait(target, traitId, value);
}
}
}
Loading