From df5ef402be3cf61f4a16e54e490e37e9cf276c05 Mon Sep 17 00:00:00 2001
From: James Owen <jamesowen@outlook.com>
Date: Fri, 26 Jul 2024 16:41:16 +0100
Subject: [PATCH] Respect the `with_spans` parser option for annotations (#201)

* Add versions of the syntax tests without spans

Prior to this change, all of the syntax tests covered the behaviour of
the parser when with_spans is set to True.

This change updates the test generation to create a version of each test
which tests the parser when with_spans is set to False. To achieve this,
we strip the span information from the expected file (rather than
needing to maintain two files).

* Respect the with_spans setting for Annotations

Prior to this change, Annotations would always have span information,
irrespective of the value of the with_spans argument to the parser.

This change changes the behaviour to only include span information if
with_spans if set to True.
---
 fluent.syntax/fluent/syntax/parser.py        |  3 +-
 fluent.syntax/tests/syntax/test_entry.py     |  4 +--
 fluent.syntax/tests/syntax/test_structure.py | 36 ++++++++++++++++----
 3 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/fluent.syntax/fluent/syntax/parser.py b/fluent.syntax/fluent/syntax/parser.py
index 7b41f314..5999f97c 100644
--- a/fluent.syntax/fluent/syntax/parser.py
+++ b/fluent.syntax/fluent/syntax/parser.py
@@ -133,7 +133,8 @@ def get_entry_or_junk(self, ps: FluentParserStream) -> ast.EntryType:
             annot = ast.Annotation(
                 err.code, list(err.args) if err.args else None, err.message
             )
-            annot.add_span(error_index, error_index)
+            if self.with_spans:
+                annot.add_span(error_index, error_index)
             junk.add_annotation(annot)
             return junk
 
diff --git a/fluent.syntax/tests/syntax/test_entry.py b/fluent.syntax/tests/syntax/test_entry.py
index 060c5240..250f8488 100644
--- a/fluent.syntax/tests/syntax/test_entry.py
+++ b/fluent.syntax/tests/syntax/test_entry.py
@@ -65,7 +65,7 @@ def test_return_junk(self):
                     "arguments": ["="],
                     "code": "E0003",
                     "message": 'Expected token: "="',
-                    "span": {"end": 23, "start": 23, "type": "Span"},
+                    "span": None,
                     "type": "Annotation",
                 }
             ],
@@ -111,7 +111,7 @@ def test_do_not_ignore_invalid_comments(self):
                     "arguments": [" "],
                     "code": "E0003",
                     "message": 'Expected token: " "',
-                    "span": {"end": 21, "start": 21, "type": "Span"},
+                    "span": None,
                     "type": "Annotation",
                 }
             ],
diff --git a/fluent.syntax/tests/syntax/test_structure.py b/fluent.syntax/tests/syntax/test_structure.py
index 9cccb541..067187fb 100644
--- a/fluent.syntax/tests/syntax/test_structure.py
+++ b/fluent.syntax/tests/syntax/test_structure.py
@@ -12,23 +12,47 @@ def read_file(path):
     return text
 
 
+def without_spans(expected):
+    """
+    Given an expected JSON fragment with span information, recursively replace all of the spans
+    with None.
+    """
+    if isinstance(expected, dict):
+        result = {}
+        for key, value in expected.items():
+            if key == "span":
+                result[key] = None
+            else:
+                result[key] = without_spans(value)
+
+        return result
+    elif isinstance(expected, list):
+        return [without_spans(item) for item in expected]
+    else:
+        # We have been passed something which would not have span information in it
+        return expected
+
+
 fixtures = os.path.join(os.path.dirname(__file__), "fixtures_structure")
 
 
 class TestStructureMeta(type):
     def __new__(mcs, name, bases, attrs):
 
-        def gen_test(file_name):
+        def gen_test(file_name, with_spans):
             def test(self):
                 ftl_path = os.path.join(fixtures, file_name + ".ftl")
                 ast_path = os.path.join(fixtures, file_name + ".json")
 
                 source = read_file(ftl_path)
-                expected = read_file(ast_path)
+                expected = json.loads(read_file(ast_path))
+
+                if not with_spans:
+                    expected = without_spans(expected)
 
-                ast = parse(source)
+                ast = parse(source, with_spans=with_spans)
 
-                self.assertEqual(ast.to_json(), json.loads(expected))
+                self.assertEqual(ast.to_json(), expected)
 
             return test
 
@@ -38,8 +62,8 @@ def test(self):
             if ext != ".ftl":
                 continue
 
-            test_name = f"test_{file_name}"
-            attrs[test_name] = gen_test(file_name)
+            attrs[f"test_{file_name}_with_spans"] = gen_test(file_name, with_spans=True)
+            attrs[f"test_{file_name}_without_spans"] = gen_test(file_name, with_spans=False)
 
         return type.__new__(mcs, name, bases, attrs)