Skip to content

Conversation

@goneall
Copy link
Member

@goneall goneall commented Apr 14, 2025

Checks simple license tokens to make sure they are either a listed license or a LicenseRef-

Fixes #227

Checks simple license tokens to make sure they are either a listed
license or a LicenseRef-

Fixes #227
@pmonks
Copy link
Collaborator

pmonks commented Apr 14, 2025

@goneall this is probably a separate issue, unrelated to this PR, but I'm not seeing any parsing of AdditionRefs in the 3.x parsing functions. I assume those code paths (if/when they're added) would benefit from similar message updates as the ones here?

Co-authored-by: Arthit Suriyawongkul <[email protected]>
Signed-off-by: Gary O'Neall <[email protected]>
@goneall
Copy link
Member Author

goneall commented Apr 14, 2025

@goneall this is probably a separate issue, unrelated to this PR, but I'm not seeing any parsing of AdditionRefs in the 3.x parsing functions. I assume those code paths (if/when they're added) would benefit from similar message updates as the ones here?

Good catch @pmonks - In looking at the license addition code, I realized I'm not taking into account external license refs which is yet another issue.

I'll add a check for the AdditionRef syntax and also implement the external license refs. I'll also add some unit tests to make sure these cases are covered.

@pmonks
Copy link
Collaborator

pmonks commented Apr 14, 2025

@goneall if it's helpful, here are the regexes I use in my own parser for LicenseRefs & AdditionRefs (the values inside the #" ... " syntax):

; LicenseRefs:
#"(DocumentRef-(?<DocumentRef>[\p{Alnum}-\.]+):)?LicenseRef-(?<LicenseRef>[\p{Alnum}-\.]+)"

; AdditionRefs:
#"(DocumentRef-(?<AdditionDocumentRef>[\p{Alnum}-\.]+):)?AdditionRef-(?<AdditionRef>[\p{Alnum}-\.]+)"

Clojure is hosted on the JVM, so these regexes will also work in Spdx-Java-Library (albeit with escaping as needed for a Java string literal that then gets compiled to a Pattern - Clojure has literal syntax for regexes, which obviates the need for escaping of anything but regex-specific characters).

Oh and note that these regexes make use of named capturing groups for extracting the custom sections of the DocumentRef, LicenseRef and AdditionRef components, but NCGs were only added in Java 7, so depending on the minimum Java version Spdx-Java-Library supports, these regexes might need a little tweaking to switch those to regular (non-named) capturing groups.

@goneall
Copy link
Member Author

goneall commented Apr 14, 2025

@goneall this is probably a separate issue, unrelated to this PR, but I'm not seeing any parsing of AdditionRefs in the 3.x parsing functions. I assume those code paths (if/when they're added) would benefit from similar message updates as the ones here?

Good catch @pmonks - In looking at the license addition code, I realized I'm not taking into account external license refs which is yet another issue.

I'll add a check for the AdditionRef syntax and also implement the external license refs. I'll also add some unit tests to make sure these cases are covered.

Turns out I was checking for external license refs (first line of the method), so I only need to add the additional check for the additionref syntax.

@pmonks - let me know if I missed anything

@pmonks
Copy link
Collaborator

pmonks commented Apr 14, 2025

@goneall
Copy link
Member Author

goneall commented Apr 15, 2025

@pmonks @bact - if you could give this another review - I think I addressed all the issues

@bact bact added the enhancement New feature or request label Apr 16, 2025
Copy link
Collaborator

@bact bact left a comment

Choose a reason for hiding this comment

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

Minor changes, adding 'a'.

@goneall goneall merged commit 95aba1f into master Apr 16, 2025
1 check passed
@goneall goneall deleted the issue227 branch April 16, 2025 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Verify error when license exceptions are used in an expression in LicenseConcluded but not in LicenseInfoInFile

4 participants