[issue-305, review] reformat, type hints, fix typos, error messages

Signed-off-by: Meret Behrens <meret.behrens@tngtech.com>
spdx · meretp · Dec 28, 2022 · Nov 25, 2022 · Dec 14, 2022 · Dec 14, 2022
commit 347051ace184acdb4362faeef466ca28e6688709
diff --git a/src/parser/json/annotation_parser.py b/src/parser/json/annotation_parser.py
@@ -34,11 +34,8 @@ def parse_all_annotations(self, input_doc_dict: Dict) -> List[Annotation]:
         self.parse_annotations_from_object(annotations, [input_doc_dict])
         reviews: List[Dict] = input_doc_dict.get("revieweds", [])
         for review in reviews:
-            annotations = append_parsed_field_or_log_error(self.logger, annotations, review,
-                                                                lambda x: self.parse_review(x,
-                                                                                            spdx_id=input_doc_dict.get(
-                                                                                                "SPDXID")))
-
+            annotations = append_parsed_field_or_log_error(
+                self.logger, annotations, review, lambda x: self.parse_review(x, spdx_id=input_doc_dict.get("SPDXID")))
         packages: List[Dict] = input_doc_dict.get("packages", [])
         self.parse_annotations_from_object(annotations, packages)
         files: List[Dict] = input_doc_dict.get("files", [])
@@ -54,8 +51,9 @@ def parse_annotations_from_object(self, annotations: List[Annotation], element_l
             element_spdx_id: Optional[str] = element.get("SPDXID")
             element_annotations: List[Dict] = element.get("annotations", [])
             annotations.extend(parse_field_or_log_error(self.logger, element_annotations,
-                                                        lambda y: self.parse_annotation(y, spdx_id=element_spdx_id),
-                                                        [], True))
+
+                                                            lambda y: self.parse_annotation(y, spdx_id=element_spdx_id),
+                                                            [], True))
 
     def parse_annotation(self, annotation_dict: Dict, spdx_id: Optional[str] = None) -> Annotation:
         logger = Logger()
@@ -75,8 +73,8 @@ def parse_annotation(self, annotation_dict: Dict, spdx_id: Optional[str] = None)
         raise_parsing_error_if_logger_has_messages(logger, "Annotation")
         annotation_dict = construct_or_raise_parsing_error(Annotation,
                                                            dict(spdx_id=spdx_id, annotation_type=annotation_type,
-                                                           annotator=annotator, annotation_date=annotation_date,
-                                                           annotation_comment=annotation_comment))
+                                                                annotator=annotator, annotation_date=annotation_date,
+                                                                annotation_comment=annotation_comment))
 
         return annotation_dict
 

diff --git a/src/parser/json/checksum_parser.py b/src/parser/json/checksum_parser.py
@@ -30,7 +30,7 @@ def parse_checksum(checksum_dict: Dict) -> Checksum:
         try:
             checksum_algorithm = ChecksumAlgorithm[algorithm]
         except KeyError:
-            logger.append(f"Invalid Algorithm for checksum: {algorithm}")
+            logger.append(f"Invalid ChecksumAlgorithm: {algorithm}")
             checksum_algorithm = None
         checksum_value: Optional[str] = checksum_dict.get("checksumValue")
         raise_parsing_error_if_logger_has_messages(logger, "Checksum")

diff --git a/src/parser/json/dict_parsing_functions.py b/src/parser/json/dict_parsing_functions.py
@@ -9,7 +9,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 from datetime import datetime
-from typing import Any, Callable, Dict, List, Union
+from typing import Any, Callable, Dict, List, Union, Optional
 
 from src.model.spdx_no_assertion import SpdxNoAssertion
 from src.model.spdx_none import SpdxNone
@@ -74,15 +74,17 @@ def raise_parsing_error_if_logger_has_messages(logger: Logger, parsed_object_nam
         else:
             raise SPDXParsingError(logger.get_messages())
 
-def parse_field_or_no_assertion_or_none(field: str, method_for_field: Callable=lambda x: x) -> Any:
+
+def parse_field_or_no_assertion_or_none(field: Optional[str], method_for_field: Callable = lambda x: x) -> Any:
     if field == SpdxNoAssertion().__str__():
         return SpdxNoAssertion()
     elif field == SpdxNone().__str__():
         return SpdxNone()
     else:
         return method_for_field(field)
 
-def parse_field_or_no_assertion(field: str, method_for_field: Callable = lambda x: x) -> Any:
+
+def parse_field_or_no_assertion(field: Optional[str], method_for_field: Callable = lambda x: x) -> Any:
     if field == SpdxNoAssertion().__str__():
         return SpdxNoAssertion()
     else:
@@ -94,6 +96,7 @@ def parse_list_of_elements(list_of_elements: List[Dict], method_to_parse_element
         logger = Logger()
     parsed_elements = []
     for element_dict in list_of_elements:
-        parsed_elements = append_parsed_field_or_log_error(logger, parsed_elements, element_dict, method_to_parse_element)
+        parsed_elements = append_parsed_field_or_log_error(logger, parsed_elements, element_dict,
+                                                           method_to_parse_element)
     raise_parsing_error_if_logger_has_messages(logger)
     return parsed_elements
diff --git a/src/parser/json/package_parser.py b/src/parser/json/package_parser.py
@@ -63,29 +63,23 @@ def parse_package(self, package_dict: Dict) -> Package:
                                                                   lambda x: x, True)
         homepage: Optional[str] = package_dict.get("homepage")
         license_comments: Optional[str] = package_dict.get("licenseComments")
-        license_concluded = parse_field_or_log_error(logger, package_dict.get("licenseConcluded"),
-                                                     lambda x: parse_field_or_no_assertion_or_none(x,
-                                                                                                   self.license_expression_parser.parse_license_expression),
-                                                     None)
+        license_concluded = parse_field_or_log_error(
+            logger, package_dict.get("licenseConcluded"),
+            lambda x: parse_field_or_no_assertion_or_none(x, self.license_expression_parser.parse_license_expression),
+            None)
 
         license_declared: Optional[Union[LicenseExpression, SpdxNoAssertion, SpdxNone]] = parse_field_or_log_error(
             logger, package_dict.get("licenseDeclared"),
             lambda x: parse_field_or_no_assertion_or_none(x, self.license_expression_parser.parse_license_expression))
 
-        license_info_from_file: Optional[
-            Union[List[LicenseExpression], SpdxNoAssertion, SpdxNone]] = parse_field_or_log_error(logger,
-                                                                                                  package_dict.get(
-                                                                                                      "licenseInfoFromFiles"),
-                                                                                                  lambda
-                                                                                                      x: parse_field_or_no_assertion_or_none(
-                                                                                                      x,
-                                                                                                      self.license_expression_parser.parse_license_expressions))
-        originator: Optional[Union[Actor, SpdxNoAssertion]] = parse_field_or_log_error(logger,
-                                                                                       package_dict.get("originator"),
-                                                                                       lambda
-                                                                                           x: parse_field_or_no_assertion(
-                                                                                           x,
-                                                                                           self.actor_parser.parse_actor))
+        license_info_from_file: Optional[Union[List[LicenseExpression], SpdxNoAssertion, SpdxNone]] = \
+            parse_field_or_log_error(
+                logger, package_dict.get("licenseInfoFromFiles"),
+                lambda x: parse_field_or_no_assertion_or_none(x,
+                                                              self.license_expression_parser.parse_license_expressions))
+        originator: Optional[Union[Actor, SpdxNoAssertion]] = parse_field_or_log_error(
+            logger, package_dict.get("originator"),
+            lambda x: parse_field_or_no_assertion(x, self.actor_parser.parse_actor))
         package_file_name: Optional[str] = package_dict.get("packageFileName")
 
         package_verification_code: Optional[
@@ -155,7 +149,7 @@ def parse_external_ref_category(external_ref_category_str: str) -> ExternalPacka
             external_ref_category = ExternalPackageRefCategory[
                 json_str_to_enum_name(external_ref_category_str)]
         except KeyError:
-            raise SPDXParsingError([f"Invalid Category for ExternalPackageRef {external_ref_category_str}"])
+            raise SPDXParsingError([f"Invalid ExternalPackageRefCategory: {external_ref_category_str}"])
 
         return external_ref_category
 

diff --git a/src/parser/json/relationship_parser.py b/src/parser/json/relationship_parser.py
@@ -41,10 +41,9 @@ def parse_all_relationships(self, input_doc_dict: Dict) -> List[Relationship]:
 
        package_dicts: List[Dict] = input_doc_dict.get("packages", [])
 
-        relationships.extend(
-            parse_field_or_log_error(self.logger, package_dicts, lambda x: self.parse_has_files(package_dicts=x,
-                                                                                                existing_relationships=relationships),
-                                     []))
+        relationships.extend(parse_field_or_log_error(
+            self.logger, package_dicts,
+            lambda x: self.parse_has_files(package_dicts=x, existing_relationships=relationships), []))
 
         file_dicts: List[Dict] = input_doc_dict.get("files", [])
 
@@ -144,13 +143,13 @@ def get_all_relationships_without_comments(existing_relationships: List[Relation
     def invert_relationship(self, relationship: Relationship) -> Relationship:
         return Relationship(related_spdx_element_id=relationship.spdx_element_id,
                             spdx_element_id=relationship.related_spdx_element_id,
-                            relationship_type=self.invvert_relationship_types[relationship.relationship_type],
+                            relationship_type=self.invert_relationship_types[relationship.relationship_type],
                             comment=relationship.comment)
 
-    invvert_relationship_types = {RelationshipType.DESCRIBES: RelationshipType.DESCRIBED_BY,
-                                  RelationshipType.DESCRIBED_BY: RelationshipType.DESCRIBES,
-                                  RelationshipType.CONTAINS: RelationshipType.CONTAINED_BY,
-                                  RelationshipType.CONTAINED_BY: RelationshipType.CONTAINS}
+    invert_relationship_types = {RelationshipType.DESCRIBES: RelationshipType.DESCRIBED_BY,
+                                 RelationshipType.DESCRIBED_BY: RelationshipType.DESCRIBES,
+                                 RelationshipType.CONTAINS: RelationshipType.CONTAINED_BY,
+                                 RelationshipType.CONTAINED_BY: RelationshipType.CONTAINS}
 
     @staticmethod
     def parse_file_dependencies(file_dicts: List[Dict]) -> List[

diff --git a/tests/parser/test_checksum_parser.py b/tests/parser/test_checksum_parser.py
@@ -41,7 +41,7 @@ def test_parse_invalid_checksum():
         checksum_parser.parse_checksum(checksum_dict)
 
     TestCase().assertCountEqual(err.value.get_messages(),
-                                ["Error while parsing Checksum: ['Invalid Algorithm for checksum: SHA']"])
+                                ["Error while parsing Checksum: ['Invalid ChecksumAlgorithm: SHA']"])
 
 
 def test_parse_incomplete_checksum():

diff --git a/tests/parser/test_creation_info_parser.py b/tests/parser/test_creation_info_parser.py
@@ -21,7 +21,7 @@
 from src.parser.json.creation_info_parser import CreationInfoParser
 
 
-def test_pares_creation_info():
+def test_parse_creation_info():
     creation_info_parser = CreationInfoParser()
     doc_dict = {
         "spdxVersion": "2.3",

diff --git a/tests/parser/test_file_parser.py b/tests/parser/test_file_parser.py
@@ -107,7 +107,7 @@ def test_parse_invalid_files():
     TestCase().assertCountEqual(err.value.get_messages(), [
         "Error while constructing File: ['SetterError File: type of argument " '"checksums" must be a list; got NoneType instead: None\']',
         'Error while constructing File: [\'SetterError File: type of argument "name" ' "must be str; got NoneType instead: None']",
-        'Error while parsing File: ["Error while parsing Checksum: [\'Invalid Algorithm for checksum: MD\']"]'])
+        'Error while parsing File: ["Error while parsing Checksum: [\'Invalid ChecksumAlgorithm: MD\']"]'])
 
 
 def test_parse_file_types():

diff --git a/tests/parser/test_package_parser.py b/tests/parser/test_package_parser.py
@@ -156,7 +156,7 @@ def test_parse_invalid_package():
         package_parser.parse_package(package_dict)
 
     TestCase().assertCountEqual(err.value.get_messages(), [
-        'Error while parsing Package: ["Error while parsing Checksum: [\'Invalid Algorithm for checksum: SHA\']"]'])
+        'Error while parsing Package: ["Error while parsing Checksum: [\'Invalid ChecksumAlgorithm: SHA\']"]'])
 
 
 def test_parse_packages():
@@ -184,7 +184,7 @@ def test_parse_packages():
 
     TestCase().assertCountEqual(err.value.get_messages(),
                                 ['Error while parsing Package: ["Error while parsing Checksum: '
-                                 '[\'Invalid Algorithm for checksum: SHA\']"]',
+                                 '[\'Invalid ChecksumAlgorithm: SHA\']"]',
                                  "Error while constructing Package: ['SetterError Package: type of argument "
                                  '"name" must be str; got int instead: 5\']'])
 
@@ -200,3 +200,12 @@ def test_parse_external_ref():
 
     TestCase().assertCountEqual(err.value.get_messages(), [
         "Error while constructing ExternalPackageRef: ['SetterError " 'ExternalPackageRef: type of argument "category" must be ' "src.model.package.ExternalPackageRefCategory; got NoneType instead: None', " '\'SetterError ExternalPackageRef: type of argument "locator" must be str; ' "got NoneType instead: None']"])
+
+def test_parse_invalid_external_package_ref_category():
+    package_parser = PackageParser()
+    external_package_ref_category = "TEST"
+
+    with pytest.raises(SPDXParsingError) as err:
+        package_parser.parse_external_ref_category(external_package_ref_category)
+
+    TestCase().assertCountEqual(err.value.get_messages(), ["Invalid ExternalPackageRefCategory: TEST"])
diff --git a/tests/parser/test_relationship_parser.py b/tests/parser/test_relationship_parser.py
@@ -129,13 +129,13 @@ def test_parse_has_files_without_duplicating_relationships():
                 "hasFiles": ["SPDXRef-File1", "SPDXRef-File2"]
             }]
     }
-    created_relationships = [
+    existing_relationships = [
         Relationship(spdx_element_id="SPDXRef-Package", relationship_type=RelationshipType.CONTAINS,
                      related_spdx_element_id="SPDXRef-File1", comment="This relationship has a comment."),
         Relationship(spdx_element_id="SPDXRef-File2", relationship_type=RelationshipType.CONTAINED_BY,
                      related_spdx_element_id="SPDXRef-Package")]
 
     relationships = relationship_parser.parse_has_files(document_dict.get("packages"),
-                                                        existing_relationships=created_relationships)
+                                                        existing_relationships=existing_relationships)
 
     assert len(relationships) == 0