diff --git a/addin/lambda-boss.Tests/EditLambdaCommandTests.cs b/addin/lambda-boss.Tests/EditLambdaCommandTests.cs index d239c80..becabe8 100644 --- a/addin/lambda-boss.Tests/EditLambdaCommandTests.cs +++ b/addin/lambda-boss.Tests/EditLambdaCommandTests.cs @@ -137,25 +137,145 @@ public void BuildExpandedLet_ZeroParamsZeroArgs_ReturnsBareBody() } [Fact] - public void BuildExpandedLet_FewerArgsThanParams_LeavesTrailingUnbound() + public void BuildExpandedLet_FewerArgsThanParams_BindsTrailingToPlaceholder() { + // Without an IF(ISOMITTED(p), …, p) pattern in the body, the unbound + // params can't have their defaults inferred — bind to "" so the LET + // is at least valid and the holes are obvious. var sig = new LambdaSignature(["x", "y", "z"], "x + y + z"); var result = EditLambdaCommand.BuildExpandedLet(sig, ["A1"]); Assert.Equal(Lines( "=LET(", " x, A1,", + " y, \"\",", + " z, \"\",", " x + y + z", ")"), result); } [Fact] - public void BuildExpandedLet_ZeroArgsWithParams_OmitsLetWrapper() + public void BuildExpandedLet_ZeroArgsWithParams_BindsAllToPlaceholders() { var sig = new LambdaSignature(["x"], "x + 1"); var result = EditLambdaCommand.BuildExpandedLet(sig, []); - Assert.Equal("=x + 1", result); + Assert.Equal(Lines( + "=LET(", + " x, \"\",", + " x + 1", + ")"), result); + } + + [Fact] + public void BuildExpandedLet_OmittedOptionalWithIsomittedDefault_BindsDefaultAndStripsIf() + { + // Caller omits `y`. Body has the canonical optional pattern + // `IF(ISOMITTED(y), 1, y)`. The pattern's default (1) becomes the + // bound value and the IF wrapper is stripped to a bare `y` so the + // LET round-trips cleanly through LET to LAMBDA. + var sig = LambdaSignatureParser.Parse("=LAMBDA(x, [y], x + IF(ISOMITTED(y), 1, y))"); + var result = EditLambdaCommand.BuildExpandedLet(sig, ["A1"]); + + Assert.Equal(Lines( + "=LET(", + " x, A1,", + " y, 1,", + " x + y", + ")"), result); + } + + [Fact] + public void BuildExpandedLet_OmittedOptionalInsideInnerLet_FoldsAndExtracts() + { + // Body is a LET; the IF(ISOMITTED) pattern is inside one of its + // binding RHS values. The inner LET still folds into the outer LET + // and the default is hoisted to the unbound-param binding. + var sig = LambdaSignatureParser.Parse( + "=LAMBDA(text, [chunk_size], LET(_size, IF(ISOMITTED(chunk_size), 1, chunk_size), LEN(text) / _size))"); + var result = EditLambdaCommand.BuildExpandedLet(sig, ["A1"]); + + Assert.Equal(Lines( + "=LET(", + " text, A1,", + " chunk_size, 1,", + " _size, chunk_size,", + " LEN(text) / _size", + ")"), result); + } + + [Fact] + public void BuildExpandedLet_MultipleOmittedOptionals_ExtractsAllDefaults() + { + var sig = LambdaSignatureParser.Parse( + "=LAMBDA(text, [chunk_size], [horizontal], " + + "LET(_size, IF(ISOMITTED(chunk_size), 1, chunk_size), " + + "_horiz, IF(ISOMITTED(horizontal), FALSE, horizontal), " + + "text & _size & _horiz))"); + var result = EditLambdaCommand.BuildExpandedLet(sig, ["A1"]); + + Assert.Equal(Lines( + "=LET(", + " text, A1,", + " chunk_size, 1,", + " horizontal, FALSE,", + " _size, chunk_size,", + " _horiz, horizontal,", + " text & _size & _horiz", + ")"), result); + } + + [Fact] + public void BuildExpandedLet_OmittedOptionalWithExpressionDefault_PreservesDefault() + { + // The default expression may itself be a function call with commas — + // the extractor must split the IF args at top-level commas. + var sig = LambdaSignatureParser.Parse( + "=LAMBDA([stop], LET(realStop, IF(ISOMITTED(stop), LAMBDA(a, i, FALSE), stop), realStop))"); + var result = EditLambdaCommand.BuildExpandedLet(sig, []); + + Assert.Equal(Lines( + "=LET(", + " stop, LAMBDA(a, i, FALSE),", + " realStop, stop,", + " realStop", + ")"), result); + } + + [Fact] + public void BuildExpandedLet_StandaloneIsomittedOnProvidedParam_LeftAlone() + { + // `ISOMITTED(text)` on its own (not wrapped as `IF(ISOMITTED(text), …, text)`) + // is not the canonical default pattern. When text is provided, the + // call evaluates to FALSE in LET context — that's the right semantics + // and we should not rewrite it. + var sig = LambdaSignatureParser.Parse( + "=LAMBDA(text, LET(Help?, ISOMITTED(text), IF(Help?, \"help\", text)))"); + var result = EditLambdaCommand.BuildExpandedLet(sig, ["A1"]); + + Assert.Equal(Lines( + "=LET(", + " text, A1,", + " Help?, ISOMITTED(text),", + " IF(Help?, \"help\", text)", + ")"), result); + } + + [Fact] + public void BuildExpandedLet_IsomittedInStringLiteral_NotRewritten() + { + // An IF(ISOMITTED(y), …, y) substring appearing inside a string + // literal in the body must not be detected as the optional pattern. + var sig = LambdaSignatureParser.Parse( + "=LAMBDA(x, [y], CONCAT(\"IF(ISOMITTED(y), 0, y)\", x))"); + var result = EditLambdaCommand.BuildExpandedLet(sig, ["A1"]); + + Assert.Equal(Lines( + "=LET(", + " x, A1,", + " y, \"\",", + " CONCAT(\"IF(ISOMITTED(y), 0, y)\", x)", + ")"), result); } [Fact] @@ -262,6 +382,38 @@ public void EndToEnd_SpecExample_ProducesExpectedLet() ")"), letFormula); } + [Fact] + public void RoundTrip_OptionalLetToLambdaThenOmittedEditLambda_RecoversCleanLet() + { + // Spec-0002 path: author marks `y` as optional in LetToLambda. The + // generated LAMBDA wraps `y` with `IF(ISOMITTED(y), 10, y)` as an + // inner LET binding named after the param. When the caller invokes + // it with only `x` (omitting `y`), Edit Lambda should extract the + // default into the outer binding and drop the now-redundant + // `y, y` shadow. + var parsed = LetParser.Parse("=LET(x, 5, y, 10, x + y)"); + var request = new LambdaGenerationRequest( + "MyCalc", + parsed, + [ + new InputChoice("x", "x", true), + new InputChoice("y", "y", true, IsOptional: true), + ]); + var refersTo = LetToLambdaBuilder.Build(request); + + var sig = LambdaSignatureParser.Parse(refersTo); + var call = EditLambdaCommand.TryParseLambdaCall("=MyCalc(A1)"); + Assert.NotNull(call); + var expanded = EditLambdaCommand.BuildExpandedLet(sig, call.Arguments); + + Assert.Equal(Lines( + "=LET(", + " x, A1,", + " y, 10,", + " x + y", + ")"), expanded); + } + [Fact] public void RoundTrip_LetToLambdaThenEditLambda_FlattensToOriginalShape() { diff --git a/addin/lambda-boss/Commands/EditLambdaCommand.cs b/addin/lambda-boss/Commands/EditLambdaCommand.cs index 79142e3..9e9b995 100644 --- a/addin/lambda-boss/Commands/EditLambdaCommand.cs +++ b/addin/lambda-boss/Commands/EditLambdaCommand.cs @@ -131,12 +131,25 @@ public static void Run() /// /// Builds a =LET(param1, arg1, ..., body) formula that binds - /// as many of the LAMBDA's parameters as call-site arguments were - /// provided. When the LAMBDA body is itself a LET, its bindings are - /// folded into the outer LET so the result is a single flat LET - /// rather than a LET-inside-LET. Output is formatted with newlines so - /// it renders legibly in Excel's formula bar. Throws when the caller - /// passed more arguments than the LAMBDA declares. + /// each of the LAMBDA's parameters to its call-site argument. When the + /// LAMBDA body is itself a LET, its bindings are folded into the outer + /// LET so the result is a single flat LET rather than a LET-inside-LET. + /// Output is formatted with newlines so it renders legibly in Excel's + /// formula bar. + /// + /// When the caller passed fewer arguments than the LAMBDA declares, + /// the unbound parameters would leave references in the body free, + /// producing an invalid LET. Each unbound parameter is bound at the + /// top of the LET to a "default" value: if the body contains the + /// canonical optional-handling pattern + /// IF(ISOMITTED(p), defaultExpr, p) the default expression + /// is extracted and the IF wrapper is stripped (replaced with a + /// bare reference to p) so the resulting LET round-trips + /// cleanly through LET to LAMBDA. Otherwise the parameter is + /// bound to "" as a placeholder the author can fill in. + /// Throws when the caller passed more arguments than the LAMBDA + /// declares. + /// /// internal static string BuildExpandedLet(LambdaSignature signature, IReadOnlyList arguments) { @@ -150,18 +163,50 @@ internal static string BuildExpandedLet(LambdaSignature signature, IReadOnlyList + $"but {arguments.Count} were provided."); } + // Pull defaults out of `IF(ISOMITTED(p), default, p)` patterns for any + // parameter the caller omitted, and strip the IF wrapper so it's not + // re-added when the LET is round-tripped back through LET to LAMBDA. + var unboundParams = signature.Parameters + .Skip(arguments.Count) + .ToList(); + var (workingBody, extractedDefaults) = + ExtractIsomittedDefaults(signature.Body, unboundParams); + var pairs = new List<(string Name, string Value)>(); for (var i = 0; i < arguments.Count; i++) pairs.Add((signature.Parameters[i], arguments[i])); + foreach (var name in unboundParams) + { + // Empty string is a deliberate placeholder for the unusual case + // where no default could be inferred — it keeps the LET valid and + // makes the missing default obvious to the author. + var value = extractedDefaults.TryGetValue(name, out var d) ? d : "\"\""; + pairs.Add((name, value)); + } string body; - if (TryParseBodyAsLet(signature.Body, out var innerBindings, out var innerBody)) + if (TryParseBodyAsLet(workingBody, out var innerBindings, out var innerBody)) { - pairs.AddRange(innerBindings); + var alreadyBound = new HashSet( + pairs.Select(p => p.Name), StringComparer.OrdinalIgnoreCase); + foreach (var (name, value) in innerBindings) + { + // After the IF(ISOMITTED(p), …, p) wrapper is stripped, an + // inner-LET binding of the form `p, IF(ISOMITTED(p), d, p)` + // collapses to `p, p` — a no-op shadow of the outer binding + // we just made. spec-0002's LetToLambdaBuilder emits exactly + // that wrapper, so dropping these keeps the round-tripped + // LET clean. + if (string.Equals(value.Trim(), name, StringComparison.OrdinalIgnoreCase) + && alreadyBound.Contains(name)) + continue; + pairs.Add((name, value)); + alreadyBound.Add(name); + } body = innerBody; } else - body = signature.Body; + body = workingBody; if (pairs.Count == 0) return "=" + body; @@ -172,6 +217,142 @@ internal static string BuildExpandedLet(LambdaSignature signature, IReadOnlyList return sb.ToString(); } + /// + /// Scans for top-level + /// IF(ISOMITTED(p), defaultExpr, p) expressions where + /// p is in , records the default + /// expression for each matched parameter, and returns the body with + /// every such IF expression replaced by a bare reference to + /// p. The first match wins when a parameter has multiple + /// occurrences; subsequent matches for the same parameter are still + /// stripped to p (so the LET stays clean) but the recorded + /// default is the first one. String literals are skipped so an IF + /// embedded in a help string is left untouched. + /// + internal static (string RewrittenBody, IReadOnlyDictionary Defaults) + ExtractIsomittedDefaults(string body, IReadOnlyList paramNames) + { + var defaults = new Dictionary(StringComparer.OrdinalIgnoreCase); + if (paramNames.Count == 0) + return (body, defaults); + + var paramSet = new HashSet(paramNames, StringComparer.OrdinalIgnoreCase); + var substitutions = new List<(int Start, int Length, string Replacement)>(); + + var i = 0; + while (i < body.Length) + { + var c = body[i]; + if (c == '"') + { + i = SkipString(body, i); + continue; + } + + if (!StartsWithIfOpen(body, i, out var afterOpenParen)) + { + i++; + continue; + } + + var openParen = afterOpenParen - 1; + var closeParen = LetParser.FindMatchingClose(body, openParen); + if (closeParen < 0) + { + i++; + continue; + } + + var inner = body[(openParen + 1)..closeParen]; + var args = LetParser.SplitTopLevelCommas(inner).Select(a => a.Trim()).ToList(); + if (args.Count != 3) + { + i = afterOpenParen; + continue; + } + + var paramName = TryParseIsomittedParam(args[0], paramSet); + if (paramName != null + && string.Equals(args[2], paramName, StringComparison.OrdinalIgnoreCase)) + { + defaults.TryAdd(paramName, args[1]); + substitutions.Add((i, closeParen - i + 1, paramName)); + i = closeParen + 1; + continue; + } + + i = afterOpenParen; + } + + if (substitutions.Count == 0) + return (body, defaults); + + var sb = new StringBuilder(body); + foreach (var (start, length, replacement) in substitutions.OrderByDescending(s => s.Start)) + { + sb.Remove(start, length); + sb.Insert(start, replacement); + } + return (sb.ToString(), defaults); + } + + private static readonly Regex IsomittedCallPattern = new( + @"^ISOMITTED\s*\(\s*([A-Za-z_][A-Za-z0-9_.?]*)\s*\)$", + RegexOptions.IgnoreCase | RegexOptions.CultureInvariant); + + private static string? TryParseIsomittedParam(string text, ISet paramNames) + { + var match = IsomittedCallPattern.Match(text); + if (!match.Success) return null; + var name = match.Groups[1].Value; + return paramNames.Contains(name) ? name : null; + } + + /// + /// Returns true when the text at starts + /// a top-level IF( token: identifier boundary on the left, + /// case-insensitive "IF", optional whitespace, then "(". + /// is set to the index immediately + /// after the open paren on success. + /// + private static bool StartsWithIfOpen(string body, int position, out int afterOpenParen) + { + afterOpenParen = -1; + if (position + 1 >= body.Length) return false; + if ((body[position] | 0x20) != 'i') return false; + if ((body[position + 1] | 0x20) != 'f') return false; + if (position > 0 && IsIdentifierChar(body[position - 1])) return false; + + var j = position + 2; + while (j < body.Length && char.IsWhiteSpace(body[j])) j++; + if (j >= body.Length || body[j] != '(') return false; + + afterOpenParen = j + 1; + return true; + } + + private static bool IsIdentifierChar(char c) + => char.IsLetterOrDigit(c) || c == '_' || c == '.' || c == '?'; + + private static int SkipString(string text, int openQuoteIndex) + { + var i = openQuoteIndex + 1; + while (i < text.Length) + { + if (text[i] == '"') + { + if (i + 1 < text.Length && text[i + 1] == '"') + { + i += 2; + continue; + } + return i + 1; + } + i++; + } + return text.Length; + } + /// /// Detects whether is exactly a single /// LET(...) expression (no leading or trailing content) and