8000 [issue-788] fix tag-value parser to allow NONE and NOASSERTION for pa… by meretp · Pull Request #816 · spdx/tools-python · GitHub
[go: up one dir, main page]
Skip to content

[issue-788] fix tag-value parser to allow NONE and NOASSERTION for pa… #816

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 23, 2024
Merged

[issue-788] fix tag-value parser to allow NONE and NOASSERTION for pa… #816

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a78f9d0
[issue-788] fix tag-value parser to allow NONE and NOASSERTION for pa…
meretp Aug 2, 2024
937b788
[issue-815] Add legacy os macos-13 to CI
meretp Aug 2, 2024
215bc38
[issue-788] fix tag-value parser to allow NONE and NOASSERTION for pa…
meretp Aug 2, 2024
cd676e3
[issue-788] Improve test
meretp Aug 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
[issue-788] fix tag-value parser to allow NONE and NOASSERTION for pa… 
…ckage source info as they are valid strings

Signed-off-by: Meret Behrens <meret.behrens@tngtech.com>

# fixes 788
  • Loading branch information
@meretp
meretp committed Aug 16, 2024
commit a78f9d049a50abff73455a022e69ace9df167ecc
8 changes: 6 additions & 2 deletions src/spdx_tools/spdx/parser/tagvalue/parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ def p_current_element_error(self, p):
"file_comment : FILE_COMMENT text_or_line\n "
"file_license_concluded : FILE_LICENSE_CONCLUDED license_or_no_assertion_or_none\n "
"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 "
"summary : PKG_SUMMARY text_or_line\n source_info : PKG_SOURCE_INFO text_or_line_including_no_assertion\n "
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem at hand can also occur for other text_or_line fields apart from source_info. Basically all properties that use text_or_line. So I would believe that the whole text_or_line 8000 property has to be adapted, not only for the source information case.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the same problem will arise when somebody uses NONE. So best solve this one here, too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I changed the behaviour such that text_or_line parses NOASSERTION and NONE as regular strings and for fields that allow NOASSERTION or NONE, like the license fields these strings will be serialized as the corresponding SpdxNoAssertion and SpdxNone objects

"homepage : PKG_HOMEPAGE 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 "
Expand Down Expand Up @@ -216,7 +216,11 @@ def p_unknown_tag(self, p):
def p_text(self, p):
p[0] = str_from_text(p[1])

@grammar_rule("text_or_line : LINE\n line_or_no_assertion : LINE\nline_or_no_assertion_or_none : text_or_line")
@grammar_rule(
"text_or_line : LINE\n line_or_no_assertion : LINE\nline_or_no_assertion_or_none : text_or_line\n"
"text_or_line_including_no_assertion : text_or_line\ntext_or_line_including_no_assertion : NO_ASSERTION\n"
"text_or_line_including_no_assertion : NONE"
)
def p_line(self, p):
p[0] = p[1]

Expand Down
20 changes: 20 additions & 0 deletions tests/spdx/parser/tagvalue/test_package_parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,26 @@ def test_parse_package():
assert package.valid_until_date == datetime(2022, 1, 1, 12)


def test_parse_package_with_no_assertion_as_source_info():
Copy link
Collaborator
@armintaenzertng armintaenzertng Aug 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, this test should include all fields that can sport text only, all filled with either NONE or NOASSERTION. Might be bit overkill, though.
However, at least a field filled with NONE should be present, I think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding all cases would be overkill imho but I added a field that contains NONE and is parsed as text.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to rename this to test_parse_none_and_no_assertion_as_text or something. Otherwise we emphasize the source info despite it not being a special case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, I also moved the test to the test_tag_valu_parser.pymodule as it is not specific to packages.

parser = Parser()
package_str = "\n".join(
[
"PackageName: Test",
"SPDXID: SPDXRef-Package",
"PackageDownloadLocation: http://example.com/test",
"FilesAnalyzed: true",
"PackageSummary: <text>Test package</text>",
"PackageSourceInfo: NOASSERTION",
]
)
document = parser.parse("\n".join([DOCUMENT_STR, package_str]))
assert document is not None
package = document.packages[0]
assert package.name == "Test"
assert package.spdx_id == "SPDXRef-Package"
assert package.source_info == "NOASSERTION"


@pytest.mark.parametrize(
"package_str, expected_message",
[
Expand Down
0