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.

Sign up for GitHub

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

Fix JMESPath and evaluation #1053

Merged
merged 1 commit into from
Jan 13, 2022
Merged

Fix JMESPath and evaluation #1053

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 3 additions & 10 deletions smithy-jmespath/src/main/java/software/amazon/smithy/jmespath/TypeChecker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,17 +217,10 @@ public LiteralExpression visitMultiSelectHash(MultiSelectHashExpression expressi
@Override
public LiteralExpression visitAnd(AndExpression expression) {
LiteralExpression leftResult = expression.getLeft().accept(this);

// Visit right side regardless of the evaluation of the left side to validate the result.
TypeChecker checker = new TypeChecker(leftResult, problems);
LiteralExpression rightResult = expression.getRight().accept(checker);

// Return a proper result based on the evaluation.
if (leftResult.isTruthy() && rightResult.isTruthy()) {
return rightResult;
} else {
return NULL;
}
LiteralExpression rightResult = expression.getRight().accept(this);
// If LHS is falsey, return LHS. Otherwise, return RHS.
return leftResult.isTruthy() ? rightResult : leftResult;
}

@Override
Expand Down
20 changes: 17 additions & 3 deletions smithy-jmespath/src/test/java/software/amazon/smithy/jmespath/TypeCheckerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ public void comparesNulls() {
assertThat(check("length(`null` == `null` && 'hi')"), empty());
assertThat(check("length(`null` != `null` || 'hi')"), empty());
assertThat(check("length(`null` != `null` && 'hi')"), contains(
"[ERROR] length function argument 0 error: Expected one of [string, array, object], but found null (1:25)"));
"[ERROR] length function argument 0 error: Expected one of [string, array, object], but found boolean (1:25)"));
assertThat(check("length(`null` > `null` && 'hi')"), containsInAnyOrder(
"[WARNING] Invalid comparator '>' for null (1:17)",
"[ERROR] length function argument 0 error: Expected one of [string, array, object], but found null (1:24)"));
Expand All @@ -336,7 +336,7 @@ public void comparesArrays() {
assertThat(check("length(`[1,2]` == `[1,2]` && 'hi')"), empty());
assertThat(check("length(`[1]` != `[1,2]` && 'hi')"), empty());
assertThat(check("length(`[1]` != `[1]` && 'hi')"), contains(
"[ERROR] length function argument 0 error: Expected one of [string, array, object], but found null (1:23)"));
"[ERROR] length function argument 0 error: Expected one of [string, array, object], but found boolean (1:23)"));
assertThat(check("length(`[1]` > `[2]` && 'hi')"), containsInAnyOrder(
"[WARNING] Invalid comparator '>' for array (1:16)",
"[ERROR] length function argument 0 error: Expected one of [string, array, object], but found null (1:22)"));
Expand All @@ -356,7 +356,7 @@ public void comparesObjects() {
assertThat(check("length(`{}` == `{}` && 'hi')"), empty());
assertThat(check("length(`{\"foo\":true}` != `{}` && 'hi')"), empty());
assertThat(check("length(`[1]` != `[1]` && 'hi')"), contains(
"[ERROR] length function argument 0 error: Expected one of [string, array, object], but found null (1:23)"));
"[ERROR] length function argument 0 error: Expected one of [string, array, object], but found boolean (1:23)"));
assertThat(check("length(`{\"foo\":true}` > `{}` && 'hi')"), containsInAnyOrder(
"[WARNING] Invalid comparator '>' for object (1:25)",
"[ERROR] length function argument 0 error: Expected one of [string, array, object], but found null (1:30)"));
Expand All @@ -370,4 +370,18 @@ public void comparesObjects() {
"[WARNING] Invalid comparator '<=' for object (1:26)",
"[ERROR] length function argument 0 error: Expected one of [string, array, object], but found null (1:31)"));
}

@Test
public void falseyLhsIsReturnedFromAnd() {
assertThat(check("ceil(`[]` && `0.9`)"), contains(
"[ERROR] ceil function argument 0 error: Expected argument to be number, but found array (1:11)"));
assertThat(check("ceil(`{}` && `0.9`)"), contains(
"[ERROR] ceil function argument 0 error: Expected argument to be number, but found object (1:11)"));
assertThat(check("ceil(`\"\"` && `0.9`)"), contains(
"[ERROR] ceil function argument 0 error: Expected argument to be number, but found string (1:11)"));
assertThat(check("ceil(`false` && `0.9`)"), contains(
"[ERROR] ceil function argument 0 error: Expected argument to be number, but found boolean (1:14)"));
assertThat(check("ceil(`null` && `0.9`)"), contains(
"[ERROR] ceil function argument 0 error: Expected argument to be number, but found null (1:13)"));
}
}
Empty file added ...test/resources/software/amazon/smithy/waiters/errorfiles/input-output-and-bug-test.errors
View file
Open in desktop
Empty file.
29 changes: 29 additions & 0 deletions ...test/resources/software/amazon/smithy/waiters/errorfiles/input-output-and-bug-test.smithy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
namespace smithy.example

use smithy.waiters#waitable

@waitable(
A: {
documentation: "A",
acceptors: [
{
state: "success",
matcher: {
inputOutput: {
path: "(input.Status == 'failed') && (output.Status == 'failed')",
expected: "true",
comparator: "booleanEquals"
}
}
}
]
}
)
operation WaitersTest {
input: WaitersTestInputOutput,
output: WaitersTestInputOutput
}

structure WaitersTestInputOutput {
Status: String
}