amadan

oficina / sheets Public

Fix round 1: pin unary/^ precedence, disambiguate LOG10-shaped calls, reject A0

- parse.go: correct the comment claiming unary-vs-^ precedence was already
  test-pinned (it wasn't); add TestParseUnaryBindsTighterThanPower to
  actually pin it (-2^2 = 4, matching Excel).
- token.go: lexWord no longer commits to tRef purely on shape match. A word
  like LOG10 matches the reference pattern (<=3 letters + digits) but is
  immediately followed by '(', so it must lex as a function name. Peek past
  whitespace for a call paren before choosing tRef over tIdent. Added
  TestParseFunctionNameShapedLikeReference and
  TestParseRefStillLexesWhenNotFollowedByCall.
- token.go: decodeRef rejects row 0 (e.g. "A0") instead of silently
  producing RowPos -1 — A1 notation is one-based, so a zero row is
  malformed input, not a valid negative position. Added
  TestParseRejectsZeroRow.
Paul Campbell 883442dc76b6694983300d04ea5cebaa3604a081 parent 575eddc
3 files changed, +89 −3
  • internal/sheets/calc/parse.go +1 −1
  • internal/sheets/calc/parse_test.go +63 −0
  • internal/sheets/calc/token.go +25 −2

Intent

full ledger: core

diff --git a/internal/sheets/calc/parse.go b/internal/sheets/calc/parse.go
index 9018d08..e0237e9 100644
--- a/internal/sheets/calc/parse.go
+++ b/internal/sheets/calc/parse.go
@@ -14,7 +14,7 @@ import (
// unary minus, then primary. Unary minus binding tighter than ^ matches
// spreadsheet convention (Excel: -2^2 = 4, not -4), which is the reverse
// of most programming languages and is easy to get wrong later, so it is
-// pinned here with a test.
+// pinned by TestParseUnaryBindsTighterThanPower.
func Parse(src string) (Node, error) {
toks, err := lex(src)
if err != nil {
diff --git a/internal/sheets/calc/parse_test.go b/internal/sheets/calc/parse_test.go
index 524be85..8dd41f5 100644
--- a/internal/sheets/calc/parse_test.go
+++ b/internal/sheets/calc/parse_test.go
@@ -87,3 +87,66 @@ func TestParseFunctionNameCaseInsensitive(t *testing.T) {
t.Fatalf("root = %T %v, want Call SUM (normalised to upper case)", n, n)
}
}
+
+func TestParseUnaryBindsTighterThanPower(t *testing.T) {
+ // -2^2 must parse as (-2)^2 = 4, not -(2^2) = -4: unary minus binds
+ // tighter than ^, matching Excel rather than most programming
+ // languages.
+ n, err := Parse("-2^2")
+ if err != nil {
+ t.Fatal(err)
+ }
+ b, ok := n.(*Binary)
+ if !ok || b.Op != "^" {
+ t.Fatalf("root = %T %v, want Binary ^", n, n)
+ }
+ u, ok := b.Left.(*Unary)
+ if !ok || u.Op != "-" {
+ t.Fatalf("left = %T, want Unary - (unary binds tighter than ^)", b.Left)
+ }
+ if _, ok := u.X.(*Num); !ok {
+ t.Fatalf("unary operand = %T, want Num", u.X)
+ }
+ if _, ok := b.Right.(*Num); !ok {
+ t.Fatalf("right = %T, want Num", b.Right)
+ }
+}
+
+func TestParseFunctionNameShapedLikeReference(t *testing.T) {
+ // LOG10 matches the reference shape (<=3 letters followed by digits)
+ // but is followed immediately by '(', so it must lex as a function
+ // call, not a reference.
+ n, err := Parse("LOG10(100)")
+ if err != nil {
+ t.Fatal(err)
+ }
+ c, ok := n.(*Call)
+ if !ok || c.Name != "LOG10" {
+ t.Fatalf("root = %T %v, want Call LOG10", n, n)
+ }
+ if len(c.Args) != 1 {
+ t.Fatalf("args = %d, want 1", len(c.Args))
+ }
+ if _, ok := c.Args[0].(*Num); !ok {
+ t.Fatalf("arg 0 = %T, want Num", c.Args[0])
+ }
+}
+
+func TestParseRefStillLexesWhenNotFollowedByCall(t *testing.T) {
+ // A1 is not followed by '(', so it must still lex as a reference even
+ // though the call-parenthesis lookahead exists for words like LOG10.
+ n, err := Parse("A1")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if _, ok := n.(*Ref); !ok {
+ t.Fatalf("root = %T, want Ref", n)
+ }
+}
+
+func TestParseRejectsZeroRow(t *testing.T) {
+ // A1 notation is one-based; "A0" is malformed input, not row -1.
+ if _, err := Parse("A0"); err == nil {
+ t.Error(`Parse("A0") succeeded, want error`)
+ }
+}
diff --git a/internal/sheets/calc/token.go b/internal/sheets/calc/token.go
index 6eefb2c..6415563 100644
--- a/internal/sheets/calc/token.go
+++ b/internal/sheets/calc/token.go
@@ -153,6 +153,13 @@ func lexString(src string, start int) (token, int, error) {
return token{kind: tStr, pos: start, str: sb.String()}, i, nil
}
+// lexWord scans a maximal run of letters/digits/'$' and decides whether it
+// is a reference or an identifier (a bare word, a boolean literal, or a
+// function name). A word can match the reference shape and still be a
+// function name — e.g. LOG10 is three letters followed by digits, exactly
+// refPattern's shape — so before committing to tRef we peek past any
+// whitespace for an opening '(': a word immediately followed by a call
+// parenthesis is always a function name, never a reference.
func lexWord(src string, start int) (token, int, error) {
i := start
n := len(src)
@@ -161,7 +168,7 @@ func lexWord(src string, start int) (token, int, error) {
}
w := src[start:i]
- if refPattern.MatchString(w) {
+ if refPattern.MatchString(w) && !followedByCallParen(src, i) {
ref, err := decodeRef(w)
if err != nil {
return token{}, 0, fmt.Errorf("calc: malformed reference at position %d", start)
@@ -174,8 +181,21 @@ func lexWord(src string, start int) (token, int, error) {
return token{kind: tIdent, pos: start, ident: w}, i, nil
}
+// followedByCallParen reports whether the next non-whitespace character
+// starting at i is '(', which disambiguates a word like LOG10 as a
+// function name rather than a reference.
+func followedByCallParen(src string, i int) bool {
+ for i < len(src) && (src[i] == ' ' || src[i] == '\t' || src[i] == '\n' || src[i] == '\r') {
+ i++
+ }
+ return i < len(src) && src[i] == '('
+}
+
// decodeRef splits a word already known to match refPattern into a
-// zero-based, absolute-flagged reference.
+// zero-based, absolute-flagged reference. A1 notation is one-based, so a
+// row literal of 0 (e.g. "A0") is malformed input, not row -1 — it is
+// rejected here rather than allowed to flow downstream as a negative
+// position.
func decodeRef(w string) (refToken, error) {
idx := 0
var r refToken
@@ -200,6 +220,9 @@ func decodeRef(w string) (refToken, error) {
if err != nil {
return refToken{}, err
}
+ if row < 1 {
+ return refToken{}, fmt.Errorf("calc: row must be positive")
+ }
r.rowPos = row - 1
return r, nil
}