Skip to content

Commit

Permalink
Parse & Serialize Bug Fixes (#75)
Browse files Browse the repository at this point in the history
- Properly respect `invariant` flag on entities during parsing to avoid pre-resolving
- Correct behavior on nested inline variable/define resolution
- Correct behavior of declaring variables referencing previous stack definition
- Improved error handling on `define` style mismatches in resolution
  • Loading branch information
tdotclare authored Nov 1, 2020
1 parent 61d6928 commit 8b965e6
Show file tree
Hide file tree
Showing 14 changed files with 134 additions and 47 deletions.
2 changes: 1 addition & 1 deletion Sources/LeafKit/LeafEntities/LeafCallParameter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ internal extension LeafCallParameter {
/// If only one type, return coerced value as long as it doesn't coerce to .trueNil (and for .bool always true)
if types.count == 1 {
let coerced = value.coerce(to: types.first!)
return coerced != .trueNil ? coerced : types.first! == .bool ? .bool(true) : .none
return !coerced.isTrueNil ? coerced : types.first! == .bool ? .bool(true) : .none
}
/// Otherwise assume function will handle coercion itself as long as one potential match exists
return types.first(where: {value.isCoercible(to: $0)}) != nil ? value : .none
Expand Down
7 changes: 7 additions & 0 deletions Sources/LeafKit/LeafError.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ public struct LeafError: LocalizedError, CustomStringConvertible {
/// Attempt to render an AST with cyclical external references
/// - Provide template name & ordered array of template names that causes the cycle path
case cyclicalReference(String, [String])

case defineMismatch(a: String, b: String, define: String)

// MARK: Wrapped Errors related to Lexing or Parsing
/// Errors due to malformed template syntax or grammar
Expand Down Expand Up @@ -102,6 +104,11 @@ public struct LeafError: LocalizedError, CustomStringConvertible {
case .noValueForKey(let k) : m += "No cache entry exists for `\(k)`"
case .noTemplateExists(let k) : m += "No template found for `\(k)`"
case .unresolvedAST(let k, let d) : m += "\(k) has unresolved dependencies: \(d)"
case .defineMismatch(let a, let b, let d)
: m += """
Resolution failure:
`\(a)` defines `\(d)()` as a block, but `\(b)` requires parameter semantics for `\(d)()` in usage.
"""
case .timeout(let d) : m += "Exceeded timeout at \(d.formatSeconds())"
case .configurationError(let d) : m += "Configuration error: `\(d)`"
case .missingRaw(let f) : m += "Missing raw inline file ``\(f)``"
Expand Down
4 changes: 3 additions & 1 deletion Sources/LeafKit/LeafParser/LKParameter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ internal struct LKParameter: LKSymbol {
invariant = v.container.isLazy ? v.invariant : true
isLiteral = invariant && !v.errored
case .variable(let v):
symbols = [v]
symbols = v.symbols
resolved = false
invariant = true
case .expression(let e):
Expand Down Expand Up @@ -245,6 +245,8 @@ internal struct LKParameter: LKSymbol {
case .tuple(let t) where t.collection
: return "\(t.labels.isEmpty ? "array" : "dictionary")\(short)"
case .tuple : return "tuple\(short)"
case .function(_,.some(let f as Evaluate),_,_,_):
return "`\(f.identifier)()"
case .function(let f,_,let p,_,_) : return "\(f)\(p?.description ?? "()")"
}
}
Expand Down
3 changes: 2 additions & 1 deletion Sources/LeafKit/LeafParser/LKParser.swift
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ internal struct LKParser {
if let match = createdVars.match(v) {
if match.1 == false {
return `nil`(.unknownError("#\(f)() explicitly does not exist and cannot be provided")) }
if match.0.state.contains(.blockDefine) && nested {
if match.0.isBlockDefine && nested {
return `nil`(.unknownError("#\(f)() is a block define - cannot use as parameter")) }
} else {
/// Block eval usage can take both forms unconditionally but nested use requires non-block definition
Expand Down Expand Up @@ -597,6 +597,7 @@ internal struct LKParser {
var a = a
/// If an invariant function with all literal params, and not a mutating or unsafe object, evaluate immediately
if case .function(_, .some(let f), let t, _, _) = a.container,
f.invariant,
f as? LKMetaBlock == nil,
f as? LeafMutatingMethod == nil,
f as? LeafUnsafeEntity == nil,
Expand Down
6 changes: 1 addition & 5 deletions Sources/LeafKit/LeafRenderer/LeafRenderer+Context.swift
Original file line number Diff line number Diff line change
Expand Up @@ -258,11 +258,7 @@ internal extension LeafRenderer.Context {
contexts[scope]![variable] = newValue
}
}

/// All scope & scoped atomic variables defined by the context
var allVariables: Set<LKVariable> {
contexts.values.reduce(into: []) {$0.formUnion($1.allVariables)} }


/// Return a filtered version of the context that holds only literal values for parse stage
var literalsOnly: Self {
guard isRootContext else { return .init(isRootContext: false) }
Expand Down
19 changes: 9 additions & 10 deletions Sources/LeafKit/LeafRenderer/LeafRenderer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ private extension LeafRenderer {
/// Call with any state of ASTBox - will fork to various behaviors as required until finally returning a
/// cached and serializable AST, if a failure hasn't bubbled out
func arbitrate(_ ast: LeafAST, _ context: Context, via chain: [String] = []) -> ELF<LeafAST> {
if let error = ast.error { return fail(error, on: eL) }
if ast.info.requiredASTs.isEmpty && ast.info.requiredRaws.isEmpty {
/// Succeed immediately if the ast is cached and doesn't need any kind of resolution
if ast.cached || context.caching.intersection([.store, .autoUpdate]).isEmpty {
Expand Down Expand Up @@ -356,21 +357,19 @@ private extension LeafRenderer {
self.fetch(.searchKey($0), context)
.flatMap { self.arbitrate($0, context, via: chain) } }

return ELF.reduce(into: ast, fetches, on: eL) { $0.inline(ast: $1) }
return ELF.reduce(into: ast, fetches, on: eL) { if !$0.errored { $0.inline(ast: $1) } }
.flatMap { self.arbitrate($0, context) }
}

/// Given a `LeafAST` and context data, serialize the AST with provided data into a final render
func syncSerialize(_ ast: LeafAST, _ context: Context) -> ELF<ByteBuffer> {
var needed = Set<LKVariable>(ast.info._requiredVars
.map {$0.isDefine ? $0 : !$0.isScoped ? $0.contextualized : $0})
needed.subtract(context.allVariables)
needed.subtract(needed.compactMap {$0.isCoalesced ? $0 : nil})
needed.subtract(needed.compactMap {context.allVariables.contains($0.contextualized) ? $0 : nil})

let shouldThrow = needed.isEmpty ? false : context.missingVariableThrows

if shouldThrow { return fail(err("[\(needed.map {$0.terse}.joined(separator: ", "))] variable(s) missing"), on: eL) }
if var needed = ast.info._requiredVars.unsatisfied(by: context) {
/// If missing variables don't throw, remove them, but leave any define references
if !context.missingVariableThrows { needed = needed.filter { $0.isDefine } }
if !needed.isEmpty {
return fail(err("[\(needed.map {$0.terse}.joined(separator: ", "))] variable(s) missing"), on: eL)
}
}

var block = LKConf.entities.raw.instantiate(size: ast.info.underestimatedSize,
encoding: context.encoding)
Expand Down
2 changes: 1 addition & 1 deletion Sources/LeafKit/LeafSerialize/LKSerializer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ internal final class LKSerializer {
case .failure(let err): return .failure(err)
}
case .passthrough(.expression(let exp)):
if let x = exp.declaresVariable {
if let x = exp.declaresVariable, x.set == nil {
buffer.pointee.voidAction()
if !allocated {
allocated = true
Expand Down
29 changes: 16 additions & 13 deletions Sources/LeafKit/LeafSyntax/LKExpression.swift
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,17 @@ internal struct LKExpression: LKSymbol {
/// Generate a `LKExpression` if possible. Guards for expressibility unless "custom" is true
private init?(_ params: LKParams) {
// .assignment/.calculation is failable, .custom does not check
guard let form = Self.expressible(params) else { return nil }
let storage = params
guard var form = Self.expressible(params) else { return nil }
var params = params
// Rewrite prefix minus special case into rhs * -1
if let unary = form.op, unary == .unaryPrefix,
let op = params[0].operator, op == .minus {
self = .init(.init(arrayLiteral: params[1], .operator(.multiply), .value(.int(-1))), (.calculation, .infix))
} else { self = .init(.init(storage), form) }
if let unary = form.op, unary == .unaryPrefix, params[0].operator == .minus {
form = (.calculation, .infix)
params = [params[1], .operator(.multiply), .value(.int(-1))]
} else if params[1].operator == .nilCoalesce, case .variable(var v) = params[0].container {
v.state.formUnion(.coalesced)
params[0] = .variable(v)
}
self = .init(.init(params), form)
}

/// Generate a custom `LKExpression` if possible.
Expand Down Expand Up @@ -145,12 +149,9 @@ internal struct LKExpression: LKSymbol {
private mutating func setStates() {
resolved = storage.allSatisfy { $0.resolved }
invariant = storage.allSatisfy {$0.invariant}
// Restate variable as coalesced if operator is ??
if storage[1].operator == .nilCoalesce {
symbols.formUnion(rhs?.symbols ?? [])
symbols.formUnion(lhs?.symbols.map { x in var x = x; x.state.formUnion(.coalesced); return x } ?? [])
//} else if let value = declaresVariable?.set { symbols.formUnion(value.symbols) }
} else { storage.forEach { symbols.formUnion($0.symbols) } }
if storage[1].operator == .nilCoalesce { symbols = rhs!.symbols }
else if let declaredValue = declaresVariable?.set { symbols = declaredValue.symbols }
else { storage.forEach { symbols.formUnion($0.symbols) } }

}

Expand All @@ -170,7 +171,9 @@ internal struct LKExpression: LKSymbol {
}
// Ignore special case of prefix minus here
if op.mathematical || op.logical { return (.calculation, opForm) }
else if [.subScript, .nilCoalesce].contains(op) { return (.calculation, .infix) }
else if [.subScript, .nilCoalesce].contains(op) {
return (.calculation, .infix)
}
else { return (.assignment, opForm) }
}

Expand Down
45 changes: 42 additions & 3 deletions Sources/LeafKit/LeafSyntax/LKVariable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ internal struct LKVariable: LKSymbol, Hashable, Equatable {
func hash(into hasher: inout Hasher) {
hasher.combine(flat)
hasher.combine(isDefine)
hasher.combine(isCoalesced)
}

static func ==(lhs: Self, rhs: Self) -> Bool { lhs.hashValue == rhs.hashValue }
Expand All @@ -22,6 +21,7 @@ internal struct LKVariable: LKSymbol, Hashable, Equatable {
/// Atomic, implicit scope variable - `x` - not `$context.x` or `x.pathed`
var isAtomic: Bool { !(isScoped || isPathed) }
var isDefine: Bool { state.contains(.defined) }
var isBlockDefine: Bool { state.contains(.blockDefine) }
var isDictionary: Bool { state.contains(.dictionary) }
var isArray: Bool { state.contains(.array) }
var isCollection: Bool { isArray || isDictionary }
Expand All @@ -45,15 +45,15 @@ internal struct LKVariable: LKSymbol, Hashable, Equatable {
// MARK: - LKSymbol
var resolved: Bool { false }
var invariant: Bool { state.contains(.constant) }
var symbols: Set<LKVariable> { [self] }
var symbols: Set<LKVariable> { isCoalesced ? [] : [self] }
func resolve(_ symbols: inout LKVarStack) -> Self { self }
func evaluate(_ symbols: inout LKVarStack) -> LeafData { symbols.match(self) }

// MARK: - LKPrintable
var description: String { flat }
var short: String { flat }
var terse: String {
isDefine ? state.contains(.blockDefine) ? "define(\(member!))" : "\(member!)()"
isDefine ? isBlockDefine ? "define(\(member!))" : "\(member!)()"
: !isScoped ? String(flat.dropFirst(2))
: isSelfScoped ? "self\(!isScope ? ".\(member!)" : "")"
: flat.replacingOccurrences(of: ":", with: ".") }
Expand Down Expand Up @@ -181,3 +181,42 @@ internal struct LKVarState: OptionSet {
scope == LKVariable.selfScope ? [scoped, constant, selfScoped] : incoming
}
}

extension Set where Element == LKVariable {
func unsatisfied(by provided: Self) -> Self? {
if isEmpty { return nil }
if provided.isEmpty { return self }
let needed = filter { this in
if this.isCoalesced { return false }
if this.isDefine {
if let other = provided.first(where: {$0 == this}) {
return other.isBlockDefine ? !this.isBlockDefine : false
} else { return true }
}
if this.isScoped { return !provided.contains(this) }
return provided.contains(this) ? false : !provided.contains(this.contextualized)
}
return needed.isEmpty ? nil : needed
}

func unsatisfied(by ctx: LeafContext) -> Self? {
unsatisfied(by: ctx.contexts.isEmpty ? []
: ctx.contexts.values.reduce(into: []) { $0.formUnion($1.allVariables) })
}

/// Defines in the provided set that match, but are block defines and not param defines
func badDefineMatches(in provided: Self) -> Self? {
let mismatches = paramDefines.intersection(provided.filter({$0.isBlockDefine}))
return mismatches.isEmpty ? nil : mismatches
}

///
var variables: Self { filter {!$0.isDefine} }
/// All defines are inherently block and param defines
var blockDefines: Self { filter {$0.isDefine} }
/// Param defines are any non-block define
var paramDefines: Self { filter {$0.isDefine && !$0.isBlockDefine} }

var coalesced: Self { filter {$0.isCoalesced} }
var unCoalesced: Self { filter {!$0.isCoalesced} }
}
30 changes: 21 additions & 9 deletions Sources/LeafKit/LeafSyntax/LeafAST.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ public struct LeafAST: Hashable {
/// List of any external unprocessed raw inlines needed to fully resolve the document
let requiredRaws: Set<String>
let stackDepths: (overallMax: UInt16, inlineMax: UInt16)

var error: LeafError? = nil

// MARK: - Computed Properties And Methods

Expand Down Expand Up @@ -151,6 +153,7 @@ internal extension LeafAST {
.reduce(into: .init(), { $0.insert($1.inline.identifier) })

let now = Date()

// Info properties (start same as actual AST, may modify from resolving
self.info = .init(parsed: now,
defines: defines.sorted(),
Expand All @@ -161,6 +164,8 @@ internal extension LeafAST {
stackDepths: stackDepths,
_requiredVars: requiredVars,
pollTime: now)

assert(requiredVars.coalesced.isEmpty, "AST includes coalesced variable state")
}

/// Any required files, whether template or raw, required to fully resolve
Expand Down Expand Up @@ -207,13 +212,19 @@ internal extension LeafAST {
: inAST.scopes[0][0]
info.underestimatedSize += nonAtomic ? inAST.underestimatedSize
: inAST.scopes[0][0].underestimatedSize
/// Non-block form defines required (eg x where`define(x = something)`, not `define(x):`)
let nonBlockDefines = inAST.requiredVars.filter { $0.isDefine && !$0.state.contains(.blockDefine) }
/// Matches for the define identifier provided that *are* block defines, not value defines
let badMatches = meta.availableVars?.intersection(nonBlockDefines).filter { $0.state.contains(.blockDefine) } ?? []
let satisfied = (meta.availableVars ?? []).subtracting(badMatches)
/// Update required vars with any new needed ones that aren't explicitly available at this inline point, and are good.
info._requiredVars.formUnion(inAST.requiredVars.subtracting(satisfied))
/// Get all required vars/defines the in AST needs that the inline point can't satisfy
var inNeeded = inAST.info._requiredVars
if let vars = meta.availableVars {
if let stillNeeded = inNeeded.unsatisfied(by: vars) {
inNeeded = stillNeeded }
if let mismatches = inNeeded.badDefineMatches(in: vars) {
error = err(.defineMismatch(a: key._name, b: inAST.key._name, define: mismatches.first!.member!))
return
}
inNeeded = []
}

info._requiredVars.formUnion(inNeeded)
}
}

Expand Down Expand Up @@ -291,8 +302,9 @@ internal extension LeafAST {

/// Never autoUpdate if pollTime isn't set yet
func autoUpdate(_ context: LKRContext) -> Bool {
info.pollTime +-> Date() >= context.pollingFrequency
}
info.pollTime +-> Date() >= context.pollingFrequency }

var errored: Bool { error != nil }
}

internal extension LeafAST.Touch {
Expand Down
2 changes: 1 addition & 1 deletion Sources/XCTLeafKit/Exports.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ public func LKXCAssertErrors<T>(_ expression: @autoclosure () throws -> T,
line: UInt = #line) {
do { _ = try expression(); XCTFail("Expression did not throw an error", file: file, line: line) }
catch {
let x = "Actual Error: `\(error.localizedDescription)`"
let x = "Actual Error:\n\(error.localizedDescription)"
let y = message()
let z = contains()
XCTAssert(!z.isEmpty, "Empty substring will catch all errors", file: file, line: line)
Expand Down
2 changes: 1 addition & 1 deletion Tests/LeafKitTests/LeafKitTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ final class LeafKitTests: MemoryRendererTestCase {
Results!
"""

try LKXCAssertErrors(render("template", aContext), contains: "[self.override] variable(s) missing")
try LKXCAssertErrors(render("template", aContext), contains: "[override] variable(s) missing")
aContext["override"] = true
try XCTAssertEqual(render("template", aContext), expected)
myAPI.version.major = 1
Expand Down
2 changes: 1 addition & 1 deletion Tests/LeafKitTests/LeafMiscTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ final class LeafMiscTests: MemoryRendererTestCase {
["nilVariable": .string(nil)],
options: [.missingVariableThrows(true)])) {
XCTAssert(($0 as! LeafError).description
.contains("[self.nonExistantVariable] variable(s) missing"),
.contains("[nonExistantVariable] variable(s) missing"),
$0.localizedDescription)
}
}
Expand Down
28 changes: 28 additions & 0 deletions Tests/LeafKitTests/LeafParserTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -680,5 +680,33 @@ final class LeafParserTests: MemoryRendererTestCase {

try LKXCAssertErrors(render("template"), contains: "Assignment via subscripted access not yet supported")
}

func testInvariantFunc() throws {
try XCTAssertEqual(parse(raw: "#(Timestamp())").terse,
"0: Timestamp(string(now), string(referenceDate))")
}

func testIndirectEvalWarning() throws {
files["A"] = "A: #(content()))"
files["B"] = "B: #inline(\"A\")"
files["C"] = "C: #inline(\"B\")"
files["D"] = """
#define(content): Block can't be param define #enddefine
#inline("A")
"""

try LKXCAssertErrors(render("A"), contains: "[content()] variable(s) missing")
try LKXCAssertErrors(render("B"), contains: "[content()] variable(s) missing")
try LKXCAssertErrors(render("C"), contains: "[content()] variable(s) missing")
try LKXCAssertErrors(render("D"), contains: "`A` requires parameter semantics for `content()`")
}

func testNestedLet() throws {
files["A1"] = "#let(x = 1)#inline(\"B\")"
files["A2"] = "#let(x = nil)#inline(\"B\")"
files["B"] = "#let(x = x ?? 2)#(x)"
try XCTAssertEqual(render("A1"), "1")
try XCTAssertEqual(render("A2"), "2")
}
}

0 comments on commit 8b965e6

Please sign in to comment.