fix(license): treat UNKNOWN-category dependencies as incompatible#513
Open
a-oren wants to merge 1 commit into
Open
fix(license): treat UNKNOWN-category dependencies as incompatible#513a-oren wants to merge 1 commit into
a-oren wants to merge 1 commit into
Conversation
Split the UNKNOWN category check in getCompatibility() so that dependencies with unrecognized licenses are flagged as incompatible when the project has a known license category, while keeping UNKNOWN project category returning UNKNOWN (no false positives). Add a distinct reason message for UNKNOWN-category dependencies to guide manual review. Implements TC-4707 Assisted-by: Claude Code
Contributor
Reviewer's GuideAdjusts license compatibility logic so UNKNOWN dependency categories are treated as incompatible when the project license category is known, adds a specific user-facing reason message for these cases, and introduces parameterized tests to lock in behavior for UNKNOWN and known category combinations. Sequence diagram for handling UNKNOWN dependency license categorysequenceDiagram
participant LicenseCheck
participant LicenseUtils
participant IncompatibleDependency
LicenseCheck->>LicenseUtils: getCompatibility(projectCategory, entry.category)
LicenseUtils-->>LicenseCheck: Compatibility.INCOMPATIBLE
alt entry.category == LicenseCategory.UNKNOWN
LicenseCheck->>IncompatibleDependency: new IncompatibleDependency(purl, entry.licenses, entry.category, reason)
Note right of IncompatibleDependency: License not recognized as a standard SPDX identifier. Manual review recommended to verify compatibility.
else entry.category != LicenseCategory.UNKNOWN
LicenseCheck->>IncompatibleDependency: new IncompatibleDependency(purl, entry.licenses, entry.category, reason)
Note right of IncompatibleDependency: Dependency license(s) are incompatible with the project license.
end
Flow diagram for updated license compatibility evaluationflowchart TD
A[Start getCompatibility
projectCategory, dependencyCategory] --> B{projectCategory == null
or dependencyCategory == null}
B -->|yes| C[Return Compatibility.UNKNOWN]
B -->|no| D{projectCategory == LicenseCategory.UNKNOWN}
D -->|yes| C
D -->|no| E{dependencyCategory == LicenseCategory.UNKNOWN}
E -->|yes| F[Return Compatibility.INCOMPATIBLE]
E -->|no| G[Compute restrictiveness
for projectCategory and dependencyCategory]
G --> H[Return Compatibility based on
restrictiveness comparison]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider centralizing the construction of the incompatibility reason message (e.g., in
LicenseUtilsor a small helper) so that future callers ofgetCompatibilitydon’t have to re‑encode the UNKNOWN vs non‑UNKNOWN branching logic and texts independently. - If
LicenseCategory.UNKNOWNcan be produced for reasons other than a non‑SPDX identifier, you may want to either adjust the wording of the new reason string or have the code receive a more specific cause from the classification layer to avoid over‑specifying the root cause here.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider centralizing the construction of the incompatibility reason message (e.g., in `LicenseUtils` or a small helper) so that future callers of `getCompatibility` don’t have to re‑encode the UNKNOWN vs non‑UNKNOWN branching logic and texts independently.
- If `LicenseCategory.UNKNOWN` can be produced for reasons other than a non‑SPDX identifier, you may want to either adjust the wording of the new reason string or have the code receive a more specific cause from the classification layer to avoid over‑specifying the root cause here.
## Individual Comments
### Comment 1
<location path="src/test/java/io/github/guacsec/trustifyda/license/License_Compatibility_Test.java" line_range="67-64" />
<code_context>
+ assertThat(result).isEqualTo(Compatibility.UNKNOWN);
+ }
+
+ /** Null project category should return UNKNOWN. */
+ @ParameterizedTest
+ @EnumSource(LicenseCategory.class)
+ void null_project_returns_unknown(LicenseCategory dependencyCategory) {
+ // When
+ Compatibility result = LicenseUtils.getCompatibility(null, dependencyCategory);
+
+ // Then
+ assertThat(result).isEqualTo(Compatibility.UNKNOWN);
+ }
+ }
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for null dependency category to fully cover the null-handling branch
The implementation returns `Compatibility.UNKNOWN` when either `projectCategory` or `dependencyCategory` is `null`, but tests only cover the `null` project case. Please also add parameterized tests for:
- `getCompatibility(projectCategory, null)` for each non-null project category (including `UNKNOWN`).
- `getCompatibility(null, null)` explicitly.
This will fully cover the null-handling branch and protect against regressions.
Suggested implementation:
```java
/** Null project category should return UNKNOWN. */
@ParameterizedTest
@EnumSource(LicenseCategory.class)
void null_project_returns_unknown(LicenseCategory dependencyCategory) {
// When
Compatibility result = LicenseUtils.getCompatibility(null, dependencyCategory);
// Then
assertThat(result).isEqualTo(Compatibility.UNKNOWN);
}
/** Null dependency category should return UNKNOWN. */
@ParameterizedTest
@EnumSource(LicenseCategory.class)
void null_dependency_returns_unknown(LicenseCategory projectCategory) {
// When
Compatibility result = LicenseUtils.getCompatibility(projectCategory, null);
// Then
assertThat(result).isEqualTo(Compatibility.UNKNOWN);
}
/** Null project and dependency categories should return UNKNOWN. */
@Test
void null_project_and_dependency_return_unknown() {
// When
Compatibility result = LicenseUtils.getCompatibility(null, null);
// Then
assertThat(result).isEqualTo(Compatibility.UNKNOWN);
}
}
```
If `@Test` is not already imported in this test class, add:
`import org.junit.jupiter.api.Test;`
near the other JUnit imports.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| LicenseUtils.getCompatibility(LicenseCategory.UNKNOWN, dependencyCategory); | ||
|
|
||
| // Then | ||
| assertThat(result).isEqualTo(Compatibility.UNKNOWN); |
Contributor
There was a problem hiding this comment.
suggestion (testing): Add tests for null dependency category to fully cover the null-handling branch
The implementation returns Compatibility.UNKNOWN when either projectCategory or dependencyCategory is null, but tests only cover the null project case. Please also add parameterized tests for:
getCompatibility(projectCategory, null)for each non-null project category (includingUNKNOWN).getCompatibility(null, null)explicitly.
This will fully cover the null-handling branch and protect against regressions.
Suggested implementation:
/** Null project category should return UNKNOWN. */
@ParameterizedTest
@EnumSource(LicenseCategory.class)
void null_project_returns_unknown(LicenseCategory dependencyCategory) {
// When
Compatibility result = LicenseUtils.getCompatibility(null, dependencyCategory);
// Then
assertThat(result).isEqualTo(Compatibility.UNKNOWN);
}
/** Null dependency category should return UNKNOWN. */
@ParameterizedTest
@EnumSource(LicenseCategory.class)
void null_dependency_returns_unknown(LicenseCategory projectCategory) {
// When
Compatibility result = LicenseUtils.getCompatibility(projectCategory, null);
// Then
assertThat(result).isEqualTo(Compatibility.UNKNOWN);
}
/** Null project and dependency categories should return UNKNOWN. */
@Test
void null_project_and_dependency_return_unknown() {
// When
Compatibility result = LicenseUtils.getCompatibility(null, null);
// Then
assertThat(result).isEqualTo(Compatibility.UNKNOWN);
}
}If @Test is not already imported in this test class, add:
import org.junit.jupiter.api.Test;
near the other JUnit imports.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
getCompatibility()so dependencies with unrecognized licenses are flagged as incompatible when the project has a known license categoryImplements TC-4707
Test plan
getCompatibility(PERMISSIVE|WEAK_COPYLEFT|STRONG_COPYLEFT, UNKNOWN)returnsINCOMPATIBLEgetCompatibility(UNKNOWN, *)returnsUNKNOWN(no false positives)getCompatibility(null, *)returnsUNKNOWNSummary by Sourcery
Treat dependencies with UNKNOWN license category as incompatible when the project license category is known, and clarify reporting for such cases.
Bug Fixes:
Enhancements:
Tests: