From 2c3962a3de585a0e250539ceef23b68ef64fa929 Mon Sep 17 00:00:00 2001 From: Ilya Puchka Date: Fri, 21 Sep 2018 22:07:28 +0300 Subject: [PATCH] Added support for brackets in boolean expressions (#165) * added support for brackets in boolean expressions * more descriptive error messages * use array slices * added test for nested expressions * removed brackets validation step * address code review comments * added doc comment * simplify expression spec * fixed docs --- CHANGELOG.md | 3 + Sources/IfTag.swift | 87 ++++++++++++++++++++----- Tests/StencilTests/ExpressionSpec.swift | 64 ++++++++++++++---- docs/builtins.rst | 13 ++++ 4 files changed, 139 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 86927278..86d4ec9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,9 @@ - Now requires Swift 4.1 or newer. [Yonas Kolb](https://github.com/yonaskolb) [#228](https://github.com/stencilproject/Stencil/pull/228) +- You can now use parentheses in boolean expressions to change operator precedence. + [Ilya Puchka](https://github.com/ilyapuchka) + [#165](https://github.com/stencilproject/Stencil/pull/165) ### New Features diff --git a/Sources/IfTag.swift b/Sources/IfTag.swift index e0148121..997f4fb0 100644 --- a/Sources/IfTag.swift +++ b/Sources/IfTag.swift @@ -38,10 +38,11 @@ func findOperator(name: String) -> Operator? { } -enum IfToken { - case infix(name: String, bindingPower: Int, op: InfixOperator.Type) - case prefix(name: String, bindingPower: Int, op: PrefixOperator.Type) +indirect enum IfToken { + case infix(name: String, bindingPower: Int, operatorType: InfixOperator.Type) + case prefix(name: String, bindingPower: Int, operatorType: PrefixOperator.Type) case variable(Resolvable) + case subExpression(Expression) case end var bindingPower: Int { @@ -52,6 +53,8 @@ enum IfToken { return bindingPower case .variable(_): return 0 + case .subExpression(_): + return 0 case .end: return 0 } @@ -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") } @@ -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") } @@ -100,21 +107,71 @@ final class IfExpressionParser { let tokens: [IfToken] var position: Int = 0 - init(components: [String], tokenParser: TokenParser, token: Token) 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, token: Token) throws -> IfExpressionParser { + return try IfExpressionParser(components: ArraySlice(components), tokenParser: tokenParser, token: token) + } + + private init(components: ArraySlice, tokenParser: TokenParser, token: Token) throws { + var parsedComponents = Set() + 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, + token: token + ) + parsedComponents.formUnion(Set(index...(index + parsedCount))) + return .subExpression(expression) + } else if component == ")" { + 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 operatorType): + return .infix(name: name, bindingPower: bindingPower, operatorType: operatorType) + case .prefix(let name, let bindingPower, let operatorType): + return .prefix(name: name, bindingPower: bindingPower, operatorType: operatorType) + } + } + return .variable(try tokenParser.compileResolvable(component, containedIn: token)) } - - return .variable(try tokenParser.compileResolvable(component, containedIn: token)) } } + private static func subExpression(from components: ArraySlice, tokenParser: TokenParser, token: Token) throws -> (Expression, Int) { + var bracketsBalance = 1 + let subComponents = components + .prefix(while: { + if $0 == "(" { + bracketsBalance += 1 + } 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, token: token) + let expression = try expressionParser.parse() + return (expression, subComponents.count) + } + var currentToken: IfToken { if tokens.count > position { return tokens[position] @@ -154,13 +211,11 @@ final class IfExpressionParser { } } - func parseExpression(components: [String], tokenParser: TokenParser, token: Token) throws -> Expression { - let parser = try IfExpressionParser(components: components, tokenParser: tokenParser, token: token) + let parser = try IfExpressionParser.parser(components: components, tokenParser: tokenParser, token: token) return try parser.parse() } - /// Represents an if condition and the associated nodes when the condition /// evaluates final class IfCondition { diff --git a/Tests/StencilTests/ExpressionSpec.swift b/Tests/StencilTests/ExpressionSpec.swift index e53ffab0..e04408ba 100644 --- a/Tests/StencilTests/ExpressionSpec.swift +++ b/Tests/StencilTests/ExpressionSpec.swift @@ -6,6 +6,11 @@ func testExpressions() { describe("Expression") { let parser = TokenParser(tokens: [], environment: Environment()) + func parseExpression(components: [String]) throws -> Expression { + let parser = try IfExpressionParser.parser(components: components, tokenParser: parser, token: .text(value: "", at: .unknown)) + return try parser.parse() + } + $0.describe("VariableExpression") { let expression = VariableExpression(variable: Variable("value")) @@ -105,19 +110,19 @@ func testExpressions() { $0.describe("expression parsing") { $0.it("can parse a variable expression") { - let expression = try parseExpression(components: ["value"], tokenParser: parser, token: .text(value: "", at: .unknown)) + let expression = try parseExpression(components: ["value"]) try expect(expression.evaluate(context: Context())).to.beFalse() try expect(expression.evaluate(context: Context(dictionary: ["value": true]))).to.beTrue() } $0.it("can parse a not expression") { - let expression = try parseExpression(components: ["not", "value"], tokenParser: parser, token: .text(value: "", at: .unknown)) + let expression = try parseExpression(components: ["not", "value"]) try expect(expression.evaluate(context: Context())).to.beTrue() try expect(expression.evaluate(context: Context(dictionary: ["value": true]))).to.beFalse() } $0.describe("and expression") { - let expression = try! parseExpression(components: ["lhs", "and", "rhs"], tokenParser: parser, token: .text(value: "", at: .unknown)) + let expression = try! parseExpression(components: ["lhs", "and", "rhs"]) $0.it("evaluates to false with lhs false") { try expect(expression.evaluate(context: Context(dictionary: ["lhs": false, "rhs": true]))).to.beFalse() @@ -137,7 +142,7 @@ func testExpressions() { } $0.describe("or expression") { - let expression = try! parseExpression(components: ["lhs", "or", "rhs"], tokenParser: parser, token: .text(value: "", at: .unknown)) + let expression = try! parseExpression(components: ["lhs", "or", "rhs"]) $0.it("evaluates to true with lhs true") { try expect(expression.evaluate(context: Context(dictionary: ["lhs": true, "rhs": false]))).to.beTrue() @@ -157,7 +162,7 @@ func testExpressions() { } $0.describe("equality expression") { - let expression = try! parseExpression(components: ["lhs", "==", "rhs"], tokenParser: parser, token: .text(value: "", at: .unknown)) + let expression = try! parseExpression(components: ["lhs", "==", "rhs"]) $0.it("evaluates to true with equal lhs/rhs") { try expect(expression.evaluate(context: Context(dictionary: ["lhs": "a", "rhs": "a"]))).to.beTrue() @@ -193,7 +198,7 @@ func testExpressions() { } $0.describe("inequality expression") { - let expression = try! parseExpression(components: ["lhs", "!=", "rhs"], tokenParser: parser, token: .text(value: "", at: .unknown)) + let expression = try! parseExpression(components: ["lhs", "!=", "rhs"]) $0.it("evaluates to true with inequal lhs/rhs") { try expect(expression.evaluate(context: Context(dictionary: ["lhs": "a", "rhs": "b"]))).to.beTrue() @@ -205,7 +210,7 @@ func testExpressions() { } $0.describe("more than expression") { - let expression = try! parseExpression(components: ["lhs", ">", "rhs"], tokenParser: parser, token: .text(value: "", at: .unknown)) + let expression = try! parseExpression(components: ["lhs", ">", "rhs"]) $0.it("evaluates to true with lhs > rhs") { try expect(expression.evaluate(context: Context(dictionary: ["lhs": 5.0, "rhs": 4]))).to.beTrue() @@ -217,7 +222,7 @@ func testExpressions() { } $0.describe("more than equal expression") { - let expression = try! parseExpression(components: ["lhs", ">=", "rhs"], tokenParser: parser, token: .text(value: "", at: .unknown)) + let expression = try! parseExpression(components: ["lhs", ">=", "rhs"]) $0.it("evaluates to true with lhs == rhs") { try expect(expression.evaluate(context: Context(dictionary: ["lhs": 5.0, "rhs": 5]))).to.beTrue() @@ -229,7 +234,7 @@ func testExpressions() { } $0.describe("less than expression") { - let expression = try! parseExpression(components: ["lhs", "<", "rhs"], tokenParser: parser, token: .text(value: "", at: .unknown)) + let expression = try! parseExpression(components: ["lhs", "<", "rhs"]) $0.it("evaluates to true with lhs < rhs") { try expect(expression.evaluate(context: Context(dictionary: ["lhs": 4, "rhs": 4.5]))).to.beTrue() @@ -241,7 +246,7 @@ func testExpressions() { } $0.describe("less than equal expression") { - let expression = try! parseExpression(components: ["lhs", "<=", "rhs"], tokenParser: parser, token: .text(value: "", at: .unknown)) + let expression = try! parseExpression(components: ["lhs", "<=", "rhs"]) $0.it("evaluates to true with lhs == rhs") { try expect(expression.evaluate(context: Context(dictionary: ["lhs": 5.0, "rhs": 5]))).to.beTrue() @@ -253,7 +258,7 @@ func testExpressions() { } $0.describe("multiple expression") { - let expression = try! parseExpression(components: ["one", "or", "two", "and", "not", "three"], tokenParser: parser, token: .text(value: "", at: .unknown)) + let expression = try! parseExpression(components: ["one", "or", "two", "and", "not", "three"]) $0.it("evaluates to true with one") { try expect(expression.evaluate(context: Context(dictionary: ["one": true]))).to.beTrue() @@ -281,7 +286,7 @@ func testExpressions() { } $0.describe("in expression") { - let expression = try! parseExpression(components: ["lhs", "in", "rhs"], tokenParser: parser, token: .text(value: "", at: .unknown)) + let expression = try! parseExpression(components: ["lhs", "in", "rhs"]) $0.it("evaluates to true when rhs contains lhs") { try expect(expression.evaluate(context: Context(dictionary: ["lhs": 1, "rhs": [1, 2, 3]]))).to.beTrue() @@ -299,6 +304,41 @@ func testExpressions() { try expect(expression.evaluate(context: Context(dictionary: ["lhs": 3, "rhs": 1..<3]))).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"]) + let expressionWithBrackets = try! parseExpression(components: ["one", "and", "(", "(", "two", ")", "or", "(", "three", "and", "four", ")", ")"]) + + 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"]) + let notExpressionWithBrackets = try! parseExpression(components: ["not", "(", "one", "or", "three", ")"]) + + 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"])) + .toThrow(TemplateSyntaxError("'if' expression error: missing closing bracket")) + try expect(parseExpression(components: [")", "lhs", "and", "rhs"])) + .toThrow(TemplateSyntaxError("'if' expression error: missing opening bracket")) + try expect(parseExpression(components: ["lhs", "and", "rhs", ")"])) + .toThrow(TemplateSyntaxError("'if' expression error: missing opening bracket")) + try expect(parseExpression(components: ["(", "lhs", "and", "rhs", ")", "("])) + .toThrow(TemplateSyntaxError("'if' expression error: missing closing bracket")) + try expect(parseExpression(components: ["(", "lhs", "and", "rhs", ")", ")"])) + .toThrow(TemplateSyntaxError("'if' expression error: missing opening bracket")) + try expect(parseExpression(components: ["(", "lhs", "and", ")"])) + .toThrow(TemplateSyntaxError("'if' expression error: end")) + try expect(parseExpression(components: ["(", "and", "rhs", ")"])) + .toThrow(TemplateSyntaxError("'if' expression error: infix operator 'and' doesn't have a left hand side")) + } + } } } } diff --git a/docs/builtins.rst b/docs/builtins.rst index d4cc99a3..0300783b 100644 --- a/docs/builtins.rst +++ b/docs/builtins.rst @@ -149,6 +149,19 @@ Will be treated as: one or (two and three) +You can use parentheses to change operator precedence. For example: + +.. code-block:: html+django + + {% if (one or two) and three %} + +Will be treated as: + +.. code-block:: text + + (one or two) and three + + ``==`` operator """""""""""""""