From cbe072e2cfcf13e136a06ab63c62871486981876 Mon Sep 17 00:00:00 2001
From: Justin King <jcking@google.com>
Date: Fri, 26 Apr 2024 13:51:47 -0700
Subject: [PATCH] Delete dangling source info from macro expansion (#934)

* Delete dangling source info from macro expansion

Signed-off-by: Justin King <jcking@google.com>

* Fix go doc

Signed-off-by: Justin King <jcking@google.com>

* Fix test

Signed-off-by: Justin King <jcking@google.com>

---------

Signed-off-by: Justin King <jcking@google.com>
---
 common/ast/ast.go             | 7 +++++++
 common/ast/conversion_test.go | 8 --------
 parser/helper.go              | 7 +++++++
 parser/parser.go              | 9 ++++++---
 4 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/common/ast/ast.go b/common/ast/ast.go
index 355ddd49..27cbbde1 100644
--- a/common/ast/ast.go
+++ b/common/ast/ast.go
@@ -310,6 +310,13 @@ func (s *SourceInfo) SetOffsetRange(id int64, o OffsetRange) {
 	s.offsetRanges[id] = o
 }
 
+// ClearOffsetRange removes the OffsetRange for the given expression id.
+func (s *SourceInfo) ClearOffsetRange(id int64) {
+	if s != nil {
+		delete(s.offsetRanges, id)
+	}
+}
+
 // GetStartLocation calculates the human-readable 1-based line and 0-based column of the first character
 // of the expression node at the id.
 func (s *SourceInfo) GetStartLocation(id int64) common.Location {
diff --git a/common/ast/conversion_test.go b/common/ast/conversion_test.go
index 32ad13c3..972e3218 100644
--- a/common/ast/conversion_test.go
+++ b/common/ast/conversion_test.go
@@ -284,18 +284,10 @@ func TestSourceInfoToProto(t *testing.T) {
           key: 6
           value: 15
         }
-        positions: {
-          key: 7
-          value: 28
-        }
         positions: {
           key: 8
           value: 29
         }
-        positions: {
-          key: 9
-          value: 35
-        }
         positions: {
           key: 10
           value: 36
diff --git a/parser/helper.go b/parser/helper.go
index 182ff034..176af695 100644
--- a/parser/helper.go
+++ b/parser/helper.go
@@ -164,6 +164,13 @@ func (p *parserHelper) id(ctx any) int64 {
 	return id
 }
 
+func (p *parserHelper) deleteId(id int64) {
+	p.sourceInfo.ClearOffsetRange(id)
+	if id == p.nextID-1 {
+		p.nextID--
+	}
+}
+
 func (p *parserHelper) getLocation(id int64) common.Location {
 	return p.sourceInfo.GetStartLocation(id)
 }
diff --git a/parser/parser.go b/parser/parser.go
index cb753df7..b72b3354 100644
--- a/parser/parser.go
+++ b/parser/parser.go
@@ -916,10 +916,12 @@ func (p *parser) expandMacro(exprID int64, function string, target ast.Expr, arg
 	expr, err := macro.Expander()(eh, target, args)
 	// An error indicates that the macro was matched, but the arguments were not well-formed.
 	if err != nil {
-		if err.Location != nil {
-			return p.reportError(err.Location, err.Message), true
+		loc := err.Location
+		if loc == nil {
+			loc = p.helper.getLocation(exprID)
 		}
-		return p.reportError(p.helper.getLocation(exprID), err.Message), true
+		p.helper.deleteId(exprID)
+		return p.reportError(loc, err.Message), true
 	}
 	// A nil value from the macro indicates that the macro implementation decided that
 	// an expansion should not be performed.
@@ -929,6 +931,7 @@ func (p *parser) expandMacro(exprID int64, function string, target ast.Expr, arg
 	if p.populateMacroCalls {
 		p.helper.addMacroCall(expr.ID(), function, target, args...)
 	}
+	p.helper.deleteId(exprID)
 	return expr, true
 }