squashed review commits with name fixes and comment improvement

meretp · meretp · commit af58dfa644a0 · 2023-03-09T12:12:06.000+01:00
Signed-off-by: Meret Behrens &lt;meret.behrens@tngtech.com&gt;
diff --git a/src/spdx/parser/tagvalue/helper_methods.py b/src/spdx/parser/tagvalue/helper_methods.py
@@ -57,7 +57,7 @@ def parse_checksum(checksum_str: str) -> Checksum:
 def set_value(parsed_value: YaccProduction, dict_to_fill: Dict[str, Any], argument_name: Optional[str] = None,
               method_to_apply: Callable = lambda x: x):
     if not argument_name:
-        argument_name = get_property(parsed_value[1])
+        argument_name = get_property_name(parsed_value[1])
     if argument_name in dict_to_fill:
         dict_to_fill["logger"].append(
             f"Multiple values for {parsed_value[1]} found. Line: {parsed_value.lineno(1)}")
@@ -72,7 +72,7 @@ def set_value(parsed_value: YaccProduction, dict_to_fill: Dict[str, Any], argume
         dict_to_fill["logger"].append(f"Invalid {parsed_value[1]}: {parsed_value[2]}. Line: {parsed_value.lineno(1)}")
 
 
-def get_property(tag: str):
+def get_property_name(tag: str):
     if tag not in TAG_DATA_MODEL_FIELD.keys():
         return camel_case_to_snake_case(tag)
     return TAG_DATA_MODEL_FIELD[tag][1]
diff --git a/src/spdx/parser/tagvalue/lexer.py b/src/spdx/parser/tagvalue/lexer.py
@@ -41,7 +41,7 @@ class SPDXLexer(object):
         # Package fields
         "PackageName": "PKG_NAME",
         "PackageVersion": "PKG_VERSION",
-        "PackageDownloadLocation": "PKG_DOWWNLOAD_LOCATION",
+        "PackageDownloadLocation": "PKG_DOWNLOAD_LOCATION",
         "FilesAnalyzed": "PKG_FILES_ANALYZED",
         "PackageSummary": "PKG_SUMMARY",
         "PackageSourceInfo": "PKG_SOURCE_INFO",
diff --git a/src/spdx/parser/tagvalue/parser.py b/src/spdx/parser/tagvalue/parser.py
@@ -119,7 +119,7 @@ def p_attrib(self, p):
                   "pkg_license_concluded : PKG_LICENSE_CONCLUDED error\n source_info : PKG_SOURCE_INFO error\n "
                   "homepage : PKG_HOMEPAGE error\n pkg_checksum : PKG_CHECKSUM error\n "
                   "verification_code : PKG_VERIFICATION_CODE error\n originator : PKG_ORIGINATOR error\n "
-                  "download_location : PKG_DOWWNLOAD_LOCATION error\n files_analyzed : PKG_FILES_ANALYZED error\n "
+                  "download_location : PKG_DOWNLOAD_LOCATION error\n files_analyzed : PKG_FILES_ANALYZED error\n "
                   "supplier : PKG_SUPPLIER error\n pkg_file_name : PKG_FILE_NAME error\n "
                   "package_version : PKG_VERSION error\n primary_package_purpose : PRIMARY_PACKAGE_PURPOSE error\n "
                   "built_date : BUILT_DATE error\n release_date : RELEASE_DATE error\n "
@@ -150,7 +150,7 @@ def p_current_element_error(self, p):
                   "package_name : PKG_NAME LINE\n description : PKG_DESCRIPTION text_or_line\n "
                   "summary : PKG_SUMMARY text_or_line\n source_info : PKG_SOURCE_INFO text_or_line\n "
                   "homepage : PKG_HOMEPAGE line_or_no_assertion_or_none\n "
-                  "download_location : PKG_DOWWNLOAD_LOCATION line_or_no_assertion_or_none\n "
+                  "download_location : PKG_DOWNLOAD_LOCATION line_or_no_assertion_or_none\n "
                   "originator : PKG_ORIGINATOR actor_or_no_assertion\n supplier : PKG_SUPPLIER actor_or_no_assertion\n "
                   "pkg_comment : PKG_COMMENT text_or_line\n "
                   "pkg_copyright_text : PKG_COPYRIGHT_TEXT line_or_no_assertion_or_none\n "
@@ -165,9 +165,7 @@ def p_current_element_error(self, p):
                   "file_spdx_id : SNIPPET_FILE_SPDXID LINE\n "
                   "snippet_license_concluded : SNIPPET_LICENSE_CONCLUDED license_or_no_assertion_or_none\n "
                   "annotation_spdx_id : ANNOTATION_SPDX_ID LINE\n "
-                  "annotation_comment : ANNOTATION_COMMENT text_or_line\n "
-
-                  )
+                  "annotation_comment : ANNOTATION_COMMENT text_or_line")
     def p_generic_value(self, p):
         if p[1] in EXPECTED_START_TAG_ELEMENT.keys():
             self.initialize_new_current_element(EXPECTED_START_TAG_ELEMENT[p[1]])
@@ -206,8 +204,9 @@ def p_actor_values(self, p):
 
     @grammar_rule("spdx_id : SPDX_ID LINE")
     def p_spdx_id(self, p):
-        # We assume that the documents spdx_id is defined first in the SPDXDocument, before any package or file
-        # information. If this is not the case the parser will behave unexpectedly as the spdx_ids are assigned falsy.
+        # As all SPDX Ids share the same tag, there is no knowing which spdx_id belongs to the document.
+        # We assume that to be the first spdx_id we encounter. As the specification does not explicitly require this,
+        # our approach might lead to unwanted behavior when the document's SPDX Id is defined later in the document.
         if "spdx_id" in self.creation_info:
             self.current_element["spdx_id"] = p[2]
         else:
@@ -302,7 +301,8 @@ def p_pkg_attribution_text(self, p):
         self.check_that_current_element_matches_class_for_value(Package, p.lineno(1))
         self.current_element.setdefault("attribution_texts", []).append(p[2])
 
-    @grammar_rule("pkg_external_ref : PKG_EXTERNAL_REF LINE PKG_EXTERNAL_REF_COMMENT text_or_line\n | PKG_EXTERNAL_REF LINE")
+    @grammar_rule(
+        "pkg_external_ref : PKG_EXTERNAL_REF LINE PKG_EXTERNAL_REF_COMMENT text_or_line\n | PKG_EXTERNAL_REF LINE")
     def p_pkg_external_refs(self, p):
         if not self.check_that_current_element_matches_class_for_value(Package, p.lineno(1)):
             return
@@ -516,9 +516,8 @@ def check_that_current_element_matches_class_for_value(self, expected_class, lin
 
     def construct_current_element(self):
         if "class" not in self.current_element:
-            # When the first element of the document is instantiated we don't have a current element in scope
-            # and the key "class" doesn't exist. Additionally, if the first element doesn't have the expected start
-            # value the key "class" wouldn't exist. To prevent a KeyError we use early return.
+            # This happens when the first element is initialized via initialize_new_current_element() or if the first
+            # element is missing its expected starting tag. In both cases we are unable to construct an element.
             return
 
         clazz = self.current_element.pop("class")
diff --git a/tests/spdx/parser/tagvalue/test_tag_value_lexer.py b/tests/spdx/parser/tagvalue/test_tag_value_lexer.py
@@ -159,7 +159,7 @@ def test_tokenization_of_package(lexer):
     token_assert_helper(lexer.token(), "LINE", "SPDXRef-Package", 2)
     token_assert_helper(lexer.token(), "PKG_VERSION", "PackageVersion", 3)
     token_assert_helper(lexer.token(), "LINE", "Version 0.9.2", 3)
-    token_assert_helper(lexer.token(), "PKG_DOWWNLOAD_LOCATION", "PackageDownloadLocation", 4)
+    token_assert_helper(lexer.token(), "PKG_DOWNLOAD_LOCATION", "PackageDownloadLocation", 4)
     token_assert_helper(lexer.token(), "LINE", "http://example.com/test", 4)
     token_assert_helper(lexer.token(), "PKG_FILES_ANALYZED", "FilesAnalyzed", 5)
     token_assert_helper(lexer.token(), "LINE", "True", 5)
