[issue-721] update Actor regex and parsing

Signed-off-by: Armin Tänzer <armin.taenzer@tngtech.com>

src/spdx_tools/spdx/parser/actor_parser.py

@@ -3,7 +3,7 @@
 # SPDX-License-Identifier: Apache-2.0
 import re
 
-from beartype.typing import Match, Optional, Pattern
+from beartype.typing import Match, Pattern
 
 from spdx_tools.spdx.model import Actor, ActorType
 from spdx_tools.spdx.parser.error import SPDXParsingError
@@ -14,8 +14,8 @@ class ActorParser:
     @staticmethod
     def parse_actor(actor: str) -> Actor:
         tool_re: Pattern = re.compile(r"^Tool:\s*(.+)", re.UNICODE)
-        person_re: Pattern = re.compile(r"^Person:\s*(([^(])+)(\((.*)\))?", re.UNICODE)
-        org_re: Pattern = re.compile(r"^Organization:\s*(([^(])+)(\((.*)\))?", re.UNICODE)
+        person_re: Pattern = re.compile(r"^Person:\s*(?:(.*)\((.*)\)|(.*))$", re.UNICODE)
+        org_re: Pattern = re.compile(r"^Organization:\s*(?:(.*)\((.*)\)|(.*))$", re.UNICODE)
         tool_match: Match = tool_re.match(actor)
         person_match: Match = person_re.match(actor)
         org_match: Match = org_re.match(actor)
@@ -24,34 +24,30 @@ def parse_actor(actor: str) -> Actor:
             name: str = tool_match.group(1).strip()
             if not name:
                 raise SPDXParsingError([f"No name for Tool provided: {actor}."])
-            creator = construct_or_raise_parsing_error(Actor, dict(actor_type=ActorType.TOOL, name=name))
+            return construct_or_raise_parsing_error(Actor, dict(actor_type=ActorType.TOOL, name=name))
 
-        elif person_match:
-            name: str = person_match.group(1).strip()
-            if not name:
-                raise SPDXParsingError([f"No name for Person provided: {actor}."])
-            email: Optional[str] = ActorParser.get_email_or_none(person_match)
-            creator = construct_or_raise_parsing_error(
-                Actor, dict(actor_type=ActorType.PERSON, name=name, email=email)
-            )
+        if person_match:
+            actor_type = ActorType.PERSON
+            match = person_match
         elif org_match:
-            name: str = org_match.group(1).strip()
-            if not name:
-                raise SPDXParsingError([f"No name for Organization provided: {actor}."])
-            email: Optional[str] = ActorParser.get_email_or_none(org_match)
-            creator = construct_or_raise_parsing_error(
-                Actor, dict(actor_type=ActorType.ORGANIZATION, name=name, email=email)
-            )
+            actor_type = ActorType.ORGANIZATION
+            match = org_match
         else:
             raise SPDXParsingError([f"Actor {actor} doesn't match any of person, organization or tool."])
 
-        return creator
-
-    @staticmethod
-    def get_email_or_none(match: Match) -> Optional[str]:
-        email_match = match.group(4)
-        if email_match and email_match.strip():
-            email = email_match.strip()
+        if match.group(3):
+            return construct_or_raise_parsing_error(
+                Actor, dict(actor_type=actor_type, name=match.group(3).strip(), email=None)
+            )
         else:
-            email = None
-        return email
+            name = match.group(1)
+            if not name:
+                raise SPDXParsingError([f"No name for Actor provided: {actor}."])
+            else:
+                name = name.strip()
+
+            email = match.group(2).strip()
+
+            return construct_or_raise_parsing_error(
+                Actor, dict(actor_type=actor_type, name=name, email=email if email else None)
+            )

tests/spdx/parser/tagvalue/test_annotation_parser.py

@@ -57,7 +57,7 @@ def test_parse_annotation():
             "not match specified grammar rule. Line: 1', 'Error while parsing "
             "AnnotationDate: Token did not match specified grammar rule. Line: 2']",
         ),
-        ("Annotator: Person: ()", "Error while parsing Annotation: [['No name for Person provided: Person: ().']]"),
+        ("Annotator: Person: ()", "Error while parsing Annotation: [['No name for Actor provided: Person: ().']]"),
         (
             "AnnotationType: REVIEW",
             "Element Annotation is not the current element in scope, probably the "

tests/spdx/test_actor_parser.py

@@ -21,7 +21,16 @@
             "organization@example.com",
         ),
         ("Organization: Example organization ( )", ActorType.ORGANIZATION, "Example organization", None),
+        ("Person: Example person ()", ActorType.PERSON, "Example person", None),
+        ("Person: Example person ", ActorType.PERSON, "Example person", None),
         ("Tool: Example tool ", ActorType.TOOL, "Example tool", None),
+        ("Tool: Example tool (email@mail.com)", ActorType.TOOL, "Example tool (email@mail.com)", None),
+        (
+            "Organization: (c) Chris Sainty (chris@sainty.com)",
+            ActorType.ORGANIZATION,
+            "(c) Chris Sainty",
+            "chris@sainty.com",
+        ),
     ],
 )
 def test_parse_actor(actor_string, expected_type, expected_name, expected_mail):
@@ -42,6 +51,8 @@ def test_parse_actor(actor_string, expected_type, expected_name, expected_mail):
             ["Actor Perso: Jane Doe (jane.doe@example.com) doesn't match any of person, organization or tool."],
         ),
         ("Toole Example Tool ()", ["Actor Toole Example Tool () doesn't match any of person, organization or tool."]),
+        ("Organization:", ["No name for Actor provided: Organization:."]),
+        ("Person: ( )", ["No name for Actor provided: Person: ( )."]),
     ],
 )
 def test_parse_invalid_actor(actor_string, expected_message):