Skip to content

fix LicenseRef handled incorrectly during satisfies check - #167

Merged
elrayle merged 3 commits into
mainfrom
elr/or-ref
Oct 6, 2026
Merged

elrayle merged 3 commits into
mainfrom
elr/or-ref

Conversation

@elrayle

@elrayle elrayle commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #165

Co-authored-by: MrBeldum danielbae@ucla.edu

Expanded investigation on other deep expressions involving licenseRefs with OR and AND. This resulted in multiple modifications to the Satisfies support functions including expandOrTerm and appendTerms. Adds tests in the satisfies_test.go file including checks of OR with two licenses, license OR licenseRef, and license OR documentRef along with deeper complex licenses with multiple ORs and ANDs.

Initial fix for the reported problem was introduced by @MrBeldum in PR #166.

Copilot AI balanced review requested due to automatic review settings October 5, 2026 21:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Nested LicenseRef alternatives can still be dropped when an OR operand contains an expanded AND expression.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Fixes LicenseRef alternatives being dropped during SPDX satisfaction checks.

Changes:

  • Retains LicenseRef nodes while expanding OR expressions.
  • Adds satisfaction tests for LicenseRef and DocumentRef combinations.
File Description
spdxexp/​satisfies.go Preserves LicenseRef OR operands.
spdxexp/​satisfies_test.go Adds regression cases for reference alternatives.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread spdxexp/satisfies.go
@elrayle
elrayle requested a balanced review from Copilot October 5, 2026 22:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The expansion logic now handles reference alternatives correctly at arbitrary nesting depths with comprehensive regression coverage.

Review effort: Balanced
Findings: None

Resolved since last review (1)

@claire153 claire153 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great fix!

@elrayle
elrayle merged commit 5d050a1 into main Oct 6, 2026
7 checks passed
@elrayle
elrayle deleted the elr/or-ref branch October 6, 2026 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Satisfies ignores a LicenseRef alternative of an OR

3 participants