From 510cbde3e4656d62aaa44894fc90b5dae5bcbfc6 Mon Sep 17 00:00:00 2001 From: pkurchatov Date: Wed, 13 Mar 2024 15:32:23 +0300 Subject: [PATCH] Fixed variable names parsing 44dfe07b4a3f02d1138dd7540b2f5365efe42e24 --- .../CalcExpression/AnyCalcExpression.swift | 148 ++++-------------- .../CalcExpression/CalcExpression.swift | 75 +++------ .../Expressions/ExpressionTests.swift | 2 +- .../expression_test_data/variables_names.json | 26 +++ 4 files changed, 80 insertions(+), 171 deletions(-) diff --git a/client/ios/DivKit/Expressions/CalcExpression/AnyCalcExpression.swift b/client/ios/DivKit/Expressions/CalcExpression/AnyCalcExpression.swift index 0ba81c365..875e6dbf7 100644 --- a/client/ios/DivKit/Expressions/CalcExpression/AnyCalcExpression.swift +++ b/client/ios/DivKit/Expressions/CalcExpression/AnyCalcExpression.swift @@ -61,6 +61,10 @@ struct AnyCalcExpression { expression, impureSymbols: { symbol in switch symbol { + case .variable("true"): + { _ in true } + case .variable("false"): + { _ in false } case .function, .infix, .prefix: functions[symbol]?.symbolEvaluator default: @@ -71,8 +75,6 @@ struct AnyCalcExpression { switch symbol { case let .variable(name): variables(name).map { value in { _ in value } } - case .function, .infix, .prefix: - functions[symbol]?.symbolEvaluator default: nil } @@ -97,39 +99,26 @@ struct AnyCalcExpression { } return String(name.dropFirst().dropLast()) } - func funcEvaluator(for _: Symbol, _ value: Any) -> CalcExpression.SymbolEvaluator? { - // TODO: should funcEvaluator call the `.infix("()")` implementation? - switch value { - case let fn as SymbolEvaluator: - { args in - try box.store(fn(args.map(box.load))) + + func defaultEvaluator(_ symbol: Symbol) throws -> CalcExpression.SymbolEvaluator? { + switch symbol { + case let .variable(name): + guard let string = unwrapString(name) else { + return { _ in throw Error.missingVariable(symbol) } } - case let fn as CalcExpression.SymbolEvaluator: - { args in - try fn(args) + let stringRef = try box.store(string) + return { _ in stringRef } + case .infix("!:"): + return { args in + switch args[0] { + case .error: + args[1] + default: + args[0] + } } default: - nil - } - } - - // Evaluators - func defaultEvaluator(for symbol: Symbol) throws -> CalcExpression.SymbolEvaluator? { - if let fn = AnyCalcExpression.standardSymbols[symbol] { - return fn - } else { - switch symbol { - case .function("[]", _): - return { try box.store($0.map(box.load)) } - case let .variable(name): - guard let string = unwrapString(name) else { - return { _ in throw Error.missingVariable(symbol) } - } - let stringRef = try box.store(string) - return { _ in stringRef } - default: - return nil - } + return nil } } @@ -140,9 +129,10 @@ struct AnyCalcExpression { impureSymbols: { symbol in if let fn = impureSymbols(symbol) { return { try box.store(fn($0.map(box.load))) } - } else if let fn = pureSymbols(symbol) { + } + if let fn = pureSymbols(symbol) { switch symbol { - case .variable, .function(_, arity: 0): + case .variable: do { let value = try box.store(fn([])) _pureSymbols[symbol] = { _ in value } @@ -152,69 +142,23 @@ struct AnyCalcExpression { default: _pureSymbols[symbol] = { try box.store(fn($0.map(box.load))) } } - } else if case .infix("()") = symbol { - // TODO: check for pure `.infix("()")` implementation, and use as - // fallback if the lhs isn't a SymbolEvaluator? - return { args in - switch try box.load(args[0]) { - case let fn as SymbolEvaluator: - return try box.store(fn(args.dropFirst().map(box.load))) - case let fn as CalcExpression.SymbolEvaluator: - return try fn(Array(args.dropFirst())) - default: - throw try Error.typeMismatch(symbol, args.map(box.load)) - } - } - } else if case let .function(name, _) = symbol { - if let fn = try defaultEvaluator(for: symbol) { - _pureSymbols[symbol] = fn - } else if let fn = impureSymbols(.variable(name)) { - return { args in - let value = try fn([]) - if let fn = funcEvaluator(for: symbol, value) { - return try fn(args) - } - throw try Error.typeMismatch( - .infix("()"), [value] + [args.map(box.load)] - ) - } - } else if let fn = pureSymbols(.variable(name)) { - do { - if let fn = try funcEvaluator(for: symbol, fn([])) { - return fn - } - } catch { - return { _ in throw error } - } - } } return nil }, pureSymbols: { symbol in - guard let fn = try (_pureSymbols[symbol] ?? defaultEvaluator(for: symbol)) else { + guard let fn = try (_pureSymbols[symbol] ?? defaultEvaluator(symbol)) else { if case let .function(name, actualArity) = symbol { - // TODO: check for pure `.infix("()")` implementation? for i in 0...10 { let symbol = Symbol.function(name, arity: .exactly(i)) if impureSymbols(symbol) ?? pureSymbols(symbol) != nil { if actualArity == .exactly(0) { - return { _ in throw - Error - .shortMessage("Non empty argument list is required for function '\(name)'.") + return { _ in + throw Error.shortMessage("Non empty argument list is required for function '\(name)'.") } } return { _ in throw Error.arityMismatch(symbol) } } } - if let fn = pureSymbols(.variable(name)) { - return { args in - let value = try fn([]) - throw try Error.typeMismatch( - .infix("()"), - [value] + [args.map(box.load)] - ) - } - } } return CalcExpression.errorEvaluator(for: symbol) } @@ -252,17 +196,8 @@ struct AnyCalcExpression { case is _String.Type, is NSString?.Type, is String?.Type, is Substring?.Type: // TODO: should we stringify any type like this? return (AnyCalcExpression.cast(AnyCalcExpression.stringify(anyValue)!) as T?)! - case is Bool.Type, is Bool?.Type: - // TODO: should we boolify numeric types like this? - if let value = AnyCalcExpression.cast(anyValue) as Double? { - return (value != 0) as! T - } default: - // TODO: should we numberify Bool values like this? - if let boolValue = anyValue as? Bool, - let value: T = AnyCalcExpression.cast(boolValue ? 1 : 0) { - return value - } + break } throw Error.resultTypeMismatch(T.self, anyValue) } @@ -277,8 +212,7 @@ extension AnyCalcExpression.Error { fileprivate static func typeMismatch( _ symbol: AnyCalcExpression.Symbol, _ args: [Any] - ) -> AnyCalcExpression - .Error { + ) -> AnyCalcExpression.Error { let types = args.map { AnyCalcExpression.stringifyOrNil(AnyCalcExpression.isNil($0) ? $0 : type(of: $0)) } @@ -344,8 +278,10 @@ extension AnyCalcExpression.Error { } /// Standard error message for invalid range - fileprivate static func invalidRange(_ lhs: T, _ rhs: T) -> AnyCalcExpression - .Error { + fileprivate static func invalidRange( + _ lhs: T, + _ rhs: T + ) -> AnyCalcExpression.Error { if lhs > rhs { return .message("Cannot form range with lower bound > upper bound") } @@ -354,8 +290,7 @@ extension AnyCalcExpression.Error { /// Standard error message for mismatched return type @usableFromInline - static func resultTypeMismatch(_ type: Any.Type, _ value: Any) -> AnyCalcExpression - .Error { + static func resultTypeMismatch(_ type: Any.Type, _ value: Any) -> AnyCalcExpression.Error { let valueType = AnyCalcExpression .stringifyOrNil(AnyCalcExpression.unwrap(value).map { Swift.type(of: $0) } as Any) return .message( @@ -556,21 +491,6 @@ extension AnyCalcExpression { } } } - - // Standard symbols - fileprivate static let standardSymbols: [Symbol: CalcExpression.SymbolEvaluator] = [ - // Boolean symbols - .variable("true"): { _ in .boolean(true) }, - .variable("false"): { _ in .boolean(false) }, - .infix("!:"): { args in - switch args[0] { - case .error: - args[1] - default: - args[0] - } - }, - ] } // Used for casting numeric values diff --git a/client/ios/DivKit/Expressions/CalcExpression/CalcExpression.swift b/client/ios/DivKit/Expressions/CalcExpression/CalcExpression.swift index d6c0d32a4..45f37baf2 100644 --- a/client/ios/DivKit/Expressions/CalcExpression/CalcExpression.swift +++ b/client/ios/DivKit/Expressions/CalcExpression/CalcExpression.swift @@ -164,24 +164,11 @@ final class CalcExpression: CustomStringConvertible { init( _ expression: ParsedCalcExpression, impureSymbols: (Symbol) throws -> SymbolEvaluator?, - pureSymbols: (Symbol) throws -> SymbolEvaluator? = { _ in nil } + pureSymbols: (Symbol) throws -> SymbolEvaluator ) throws { root = try expression.root.optimized( - withImpureSymbols: impureSymbols, - pureSymbols: { - if let fn = try pureSymbols($0) { - return fn - } - if case let .function(name, _) = $0 { - for i in 0...10 { - let symbol = Symbol.function(name, arity: .exactly(i)) - if try (impureSymbols(symbol) ?? pureSymbols(symbol)) != nil { - return { _ in throw Error.arityMismatch(symbol) } - } - } - } - return CalcExpression.errorEvaluator(for: $0) - } + impureSymbols: impureSymbols, + pureSymbols: pureSymbols ) } @@ -372,7 +359,8 @@ extension CalcExpression { fileprivate static func isIdentifier(_ c: UnicodeScalar) -> Bool { switch c.value { - case 0x30...0x39, // 0-9 + case 0x2E, // . + 0x30...0x39, // 0-9 0x03_00...0x03_6F, 0x1D_C0...0x1D_FF, 0x20_D0...0x20_FF, @@ -467,7 +455,7 @@ private enum Subexpression: CustomStringConvertible { } func needsSeparation(_ lhs: String, _ rhs: String) -> Bool { let lhs = lhs.unicodeScalars.last!, rhs = rhs.unicodeScalars.first! - return lhs == "." || (CalcExpression.isOperator(lhs) || lhs == "-") + return (CalcExpression.isOperator(lhs) || lhs == "-") == (CalcExpression.isOperator(rhs) || rhs == "-") } switch symbol { @@ -541,15 +529,14 @@ private enum Subexpression: CustomStringConvertible { } func optimized( - withImpureSymbols impureSymbols: (CalcExpression.Symbol) throws -> CalcExpression - .SymbolEvaluator?, + impureSymbols: (CalcExpression.Symbol) throws -> CalcExpression.SymbolEvaluator?, pureSymbols: (CalcExpression.Symbol) throws -> CalcExpression.SymbolEvaluator ) throws -> Subexpression { guard case .symbol(let symbol, var args, _) = self else { return self } args = try args.map { - try $0.optimized(withImpureSymbols: impureSymbols, pureSymbols: pureSymbols) + try $0.optimized(impureSymbols: impureSymbols, pureSymbols: pureSymbols) } if let fn = try impureSymbols(symbol) { return .symbol(symbol, args, fn) @@ -821,7 +808,7 @@ extension UnicodeScalarView { } fileprivate mutating func parseOperator() -> Subexpression? { - if var op = scanCharacters({ $0 == "." }) ?? scanCharacters({ $0 == "-" }) { + if var op = scanCharacters({ $0 == "-" }) { if let tail = scanCharacters(CalcExpression.isOperator) { op += tail } @@ -835,42 +822,18 @@ extension UnicodeScalarView { } fileprivate mutating func parseIdentifier() -> Subexpression? { - func scanIdentifier() -> String? { - var start = self - var identifier = "" - if scanCharacter(".") { - identifier = "." - } else if let head = scanCharacter(CalcExpression.isIdentifierHead) { - identifier = head - start = self - if scanCharacter(".") { - identifier.append(".") - } - } else { - return nil - } - while let tail = scanCharacters(CalcExpression.isIdentifier) { - identifier += tail - start = self - if scanCharacter(".") { - identifier.append(".") - } - } - if identifier.hasSuffix(".") { - self = start - if identifier == "." { - return nil - } - identifier = String(identifier.unicodeScalars.dropLast()) - } else if scanCharacter("'") { - identifier.append("'") - } - return identifier - } - - guard let identifier = scanIdentifier() else { + var identifier = "" + if let head = scanCharacter(CalcExpression.isIdentifierHead) { + identifier = head + } else { return nil } + while let tail = scanCharacters(CalcExpression.isIdentifier) { + identifier += tail + } + if scanCharacter("'") { + identifier.append("'") + } return .symbol(.variable(identifier), [], nil) } diff --git a/client/ios/DivKitTests/Expressions/ExpressionTests.swift b/client/ios/DivKitTests/Expressions/ExpressionTests.swift index 3865c3a15..d6768bea5 100644 --- a/client/ios/DivKitTests/Expressions/ExpressionTests.swift +++ b/client/ios/DivKitTests/Expressions/ExpressionTests.swift @@ -47,7 +47,7 @@ private func runTest(_ testCase: ExpressionTestCase) { _ = testCase.resolveValue(errorTracker: { errorMessage = $0.message }) if expectedMessage.isEmpty { // Can throw any message - XCTAssertFalse(errorMessage.isEmpty) + XCTAssertFalse(errorMessage.isEmpty, "Expected error") } else { XCTAssertEqual(errorMessage, expectedMessage) } diff --git a/test_data/expression_test_data/variables_names.json b/test_data/expression_test_data/variables_names.json index 23d155fdb..127cf5660 100644 --- a/test_data/expression_test_data/variables_names.json +++ b/test_data/expression_test_data/variables_names.json @@ -216,6 +216,7 @@ ], "platforms": [ "android", + "ios", "web" ] }, @@ -235,6 +236,7 @@ ], "platforms": [ "android", + "ios", "web" ] }, @@ -254,6 +256,7 @@ ], "platforms": [ "android", + "ios", "web" ] }, @@ -293,6 +296,7 @@ ], "platforms": [ "android", + "ios", "web" ] }, @@ -332,6 +336,7 @@ ], "platforms": [ "android", + "ios", "web" ] }, @@ -351,6 +356,27 @@ ], "platforms": [ "android", + "ios", + "web" + ] + }, + { + "name": "variable name same as function name", + "expression": "@{maxInteger}", + "expected": { + "type": "string", + "value": "ok" + }, + "variables": [ + { + "name": "maxInteger", + "type": "string", + "value": "ok" + } + ], + "platforms": [ + "android", + "ios", "web" ] }