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

Added support for brackets in boolean expressions #165

Merged
merged 12 commits into from
Sep 21, 2018
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
- Added support for resolving superclass properties for not-NSObject subclasses
- The `{% for %}` tag can now iterate over tuples, structures and classes via
their stored properties.
- You can now use brackets in boolean expressions to change operators precedence
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you update the entry format to match the rest? (., 2 spaces, credit yourself and ref. this PR)


### Bug Fixes

Expand Down
80 changes: 66 additions & 14 deletions Sources/IfTag.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,11 @@ func findOperator(name: String) -> Operator? {
}


enum IfToken {
indirect enum IfToken {
case infix(name: String, bindingPower: Int, op: InfixOperator.Type)
case prefix(name: String, bindingPower: Int, op: PrefixOperator.Type)
case variable(Resolvable)
case subExpression(Expression)
case end

var bindingPower: Int {
Expand All @@ -52,6 +53,8 @@ enum IfToken {
return bindingPower
case .variable(_):
return 0
case .subExpression(_):
return 0
case .end:
return 0
}
Expand All @@ -66,6 +69,8 @@ enum IfToken {
return op.init(expression: expression)
case .variable(let variable):
return VariableExpression(variable: variable)
case .subExpression(let expression):
return expression
case .end:
throw TemplateSyntaxError("'if' expression error: end")
}
Expand All @@ -80,6 +85,8 @@ enum IfToken {
throw TemplateSyntaxError("'if' expression error: prefix operator '\(name)' was called with a left hand side")
case .variable(let variable):
throw TemplateSyntaxError("'if' expression error: variable '\(variable)' was called with a left hand side")
case .subExpression(_):
throw TemplateSyntaxError("'if' expression error: sub expression was called with a left hand side")
case .end:
throw TemplateSyntaxError("'if' expression error: end")
}
Expand All @@ -100,21 +107,68 @@ final class IfExpressionParser {
let tokens: [IfToken]
var position: Int = 0

init(components: [String], tokenParser: TokenParser) throws {
self.tokens = try components.map { component in
if let op = findOperator(name: component) {
switch op {
case .infix(let name, let bindingPower, let cls):
return .infix(name: name, bindingPower: bindingPower, op: cls)
case .prefix(let name, let bindingPower, let cls):
return .prefix(name: name, bindingPower: bindingPower, op: cls)
private init(tokens: [IfToken]) {
self.tokens = tokens
}

static func parser(components: [String], tokenParser: TokenParser) throws -> IfExpressionParser {
return try IfExpressionParser(components: ArraySlice(components), tokenParser: tokenParser)
}

private init(components: ArraySlice<String>, tokenParser: TokenParser) throws {
Copy link
Collaborator

Choose a reason for hiding this comment

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

👌

var parsedComponents = Set<Int>()
var bracketsBalance = 0
self.tokens = try zip(components.indices, components).flatMap { (index, component) in
guard !parsedComponents.contains(index) else { return nil }

if component == "(" {
bracketsBalance += 1
let (expression, parsedCount) = try IfExpressionParser.subExpression(
from: components.suffix(from: index + 1),
tokenParser: tokenParser
)
parsedComponents.formUnion(Set(index...(index + parsedCount)))
return .subExpression(expression)
}
else if component == ")" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why isn't this on the same line as the }?

bracketsBalance -= 1
if bracketsBalance < 0 {
throw TemplateSyntaxError("'if' expression error: missing opening bracket")
}
parsedComponents.insert(index)
return nil
} else {
parsedComponents.insert(index)
if let op = findOperator(name: component) {
switch op {
case .infix(let name, let bindingPower, let cls):
Copy link
Contributor

Choose a reason for hiding this comment

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

I had to look up what the cls value was in the Operator enum. Maybe ..., let operator woud be clearer?

return .infix(name: name, bindingPower: bindingPower, op: cls)
case .prefix(let name, let bindingPower, let cls):
return .prefix(name: name, bindingPower: bindingPower, op: cls)
}
}
return .variable(try tokenParser.compileFilter(component))
}

return .variable(try tokenParser.compileFilter(component))
}
}

private static func subExpression(from components: ArraySlice<String>, tokenParser: TokenParser) throws -> (Expression, Int) {
var bracketsBalance = 1
let subComponents = components
.prefix(while: {
if $0 == "(" { bracketsBalance += 1 }
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a big fan of having the content of the if on the same line, and pretty sure swiftlint'll complain about this too.

else if $0 == ")" { bracketsBalance -= 1 }
return bracketsBalance != 0
})
if bracketsBalance > 0 {
throw TemplateSyntaxError("'if' expression error: missing closing bracket")
}

let expressionParser = try IfExpressionParser(components: subComponents, tokenParser: tokenParser)
let expression = try expressionParser.parse()
return (expression, subComponents.count)
}

var currentToken: IfToken {
if tokens.count > position {
return tokens[position]
Expand Down Expand Up @@ -154,13 +208,11 @@ final class IfExpressionParser {
}
}


func parseExpression(components: [String], tokenParser: TokenParser) throws -> Expression {
let parser = try IfExpressionParser(components: components, tokenParser: tokenParser)
let parser = try IfExpressionParser.parser(components: components, tokenParser: tokenParser)
return try parser.parse()
}


/// Represents an if condition and the associated nodes when the condition
/// evaluates
final class IfCondition {
Expand Down
36 changes: 36 additions & 0 deletions Tests/StencilTests/ExpressionSpec.swift
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,42 @@ func testExpressions() {
try expect(expression.evaluate(context: Context(dictionary: ["lhs": "a", "rhs": "bcd"]))).to.beFalse()
}
}

$0.describe("sub expression") {
$0.it("evaluates correctly") {
let context = Context(dictionary: ["one": false, "two": false, "three": true, "four": true])

let expression = try! parseExpression(components: ["one", "and", "two", "or", "three", "and", "four"], tokenParser: parser)
let expressionWithBrackets = try! parseExpression(components: ["one", "and", "(", "(", "two", ")", "or", "(", "three", "and", "four", ")", ")"], tokenParser: parser)

try expect(expression.evaluate(context: context)).to.beTrue()
try expect(expressionWithBrackets.evaluate(context: context)).to.beFalse()

let notExpression = try! parseExpression(components: ["not", "one", "or", "three"], tokenParser: parser)
let notExpressionWithBrackets = try! parseExpression(components: ["not", "(", "one", "or", "three", ")"], tokenParser: parser)

try expect(notExpression.evaluate(context: context)).to.beTrue()
try expect(notExpressionWithBrackets.evaluate(context: context)).to.beFalse()
}

$0.it("fails when brackets are not balanced") {
try expect(parseExpression(components: ["(", "lhs", "and", "rhs"], tokenParser: parser))
.toThrow(TemplateSyntaxError("'if' expression error: missing closing bracket"))
try expect(parseExpression(components: [")", "lhs", "and", "rhs"], tokenParser: parser))
.toThrow(TemplateSyntaxError("'if' expression error: missing opening bracket"))
try expect(parseExpression(components: ["lhs", "and", "rhs", ")"], tokenParser: parser))
.toThrow(TemplateSyntaxError("'if' expression error: missing opening bracket"))
try expect(parseExpression(components: ["(", "lhs", "and", "rhs", ")", "("], tokenParser: parser))
.toThrow(TemplateSyntaxError("'if' expression error: missing closing bracket"))
try expect(parseExpression(components: ["(", "lhs", "and", "rhs", ")", ")"], tokenParser: parser))
.toThrow(TemplateSyntaxError("'if' expression error: missing opening bracket"))
try expect(parseExpression(components: ["(", "lhs", "and", ")"], tokenParser: parser))
.toThrow(TemplateSyntaxError("'if' expression error: end"))
try expect(parseExpression(components: ["(", "and", "rhs", ")"], tokenParser: parser))
.toThrow(TemplateSyntaxError("'if' expression error: infix operator 'and' doesn't have a left hand side"))
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

unneeded empty new line

}
}
}