From d5c355c3bb9fee81708bb83f2ccc52dbefba0ba2 Mon Sep 17 00:00:00 2001
From: David Almeida <58078834+dc-almeida@users.noreply.github.com>
Date: Thu, 30 Jan 2025 16:31:55 +0100
Subject: [PATCH] Specify illegal character(s) found and respective
 attribute(s) (#456)

* Specify illegal character(s) found and respective attribute(s)

* Fix error message

* Refactor f-string for <3.12

* Rewrite error message

Co-authored-by: Daniel Huppmann <dh@dergelbesalon.at>

* Update tests

* Shorten error message

* Use Code from external repo attribute

---------

Co-authored-by: Daniel Huppmann <dh@dergelbesalon.at>
Co-authored-by: David Almeida <almeida@iiasa.ac.at>
---
 nomenclature/codelist.py | 23 +++++++++++------------
 tests/test_codelist.py   | 17 +++++++++++++----
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/nomenclature/codelist.py b/nomenclature/codelist.py
index 0b7f4343..af4ff77b 100644
--- a/nomenclature/codelist.py
+++ b/nomenclature/codelist.py
@@ -339,28 +339,27 @@ def check_illegal_characters(self, config: NomenclatureConfig) -> dict[str, Code
         illegal = ["{", "}"] + config.illegal_characters
         errors = ErrorCollector()
 
-        def _check_string(value):
+        def _check_string(attr, value):
             if isinstance(value, str):
-                if any(char in value for char in illegal):
+                if found := set(illegal).intersection(value):
+                    found = "', '".join(sorted(found))
                     errors.append(
                         ValueError(
-                            f"Unexpected character in {self.name}: '{code.name}'."
-                            " Check for illegal characters and/or if tags were spelled correctly."
+                            f"Illegal character(s) '{found}' in {attr} of {self.name} '{code.name}'."
                         )
                     )
             elif isinstance(value, dict):
-                for k in value.keys():
-                    _check_string(k)
-                for v in value.values():
-                    _check_string(v)
+                for k, v in value.items():
+                    _check_string(k, k)
+                    _check_string(k, v)
             elif isinstance(value, list):
                 for item in value:
-                    _check_string(item)
+                    _check_string(attr, item)
 
         for code in self.mapping.values():
-            if not code.repository:
-                for value in code.model_dump(exclude="file").values():
-                    _check_string(value)
+            if not code.from_external_repository:
+                for attr, value in code.model_dump(exclude="file").items():
+                    _check_string(attr, value)
         if errors:
             raise ValueError(errors)
 
diff --git a/tests/test_codelist.py b/tests/test_codelist.py
index 552161f2..f9106b80 100644
--- a/tests/test_codelist.py
+++ b/tests/test_codelist.py
@@ -271,9 +271,18 @@ def test_to_csv():
 @pytest.mark.parametrize(
     "subfolder, match",
     [
-        ("tag_in_str", r"Unexpected character in variable: 'Primary Energy\|{Feul}'"),
-        ("tag_in_list", r"Unexpected character in variable: 'Share\|Coal'"),
-        ("tag_in_dict", r"Unexpected character in variable: 'Primary Energy'"),
+        (
+            "tag_in_str",
+            r"Illegal character\(s\) '{', '}' in name of variable 'Primary Energy\|{Feul}'",
+        ),
+        (
+            "tag_in_list",
+            r"Illegal character\(s\) '{' in info of variable 'Share\|Coal'",
+        ),
+        (
+            "tag_in_dict",
+            r"Illegal character\(s\) '}' in invalid of variable 'Primary Energy'",
+        ),
     ],
 )
 def test_stray_tag_fails(subfolder, match):
@@ -287,7 +296,7 @@ def test_stray_tag_fails(subfolder, match):
 
 def test_illegal_char_fails():
     """Check that illegal character raises expected error."""
-    match = r"Unexpected character in variable: 'Primary Energy\|Coal'"
+    match = r"Illegal character\(s\) '\"' in info of variable 'Primary Energy\|Coal'"
     with raises(ValueError, match=match):
         DataStructureDefinition(
             MODULE_TEST_DATA_DIR / "illegal_chars" / "char_in_str" / "definitions"