Skip to content

Add report generation and optimize snippet findings for SCANOSS #10287

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.

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

isasmendiagus
Copy link
Contributor

This PR implements reporting capabilities for SCANOSS scans:

  • Adding new snippet report generation functionality
  • Including release dates in snippet findings and report columns
  • Optimizing the ScanOssResultParser to:
    • Generate one Snippet per detected line range
    • Remove duplicate licenses

This builds on top of #10270 implementing SCANOSS SDK migration and filtering capabilities.

Copy link

codecov bot commented May 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 56.43%. Comparing base (737038d) to head (425cc72).

Additional details and impacted files
@@            Coverage Diff            @@
##               main   #10287   +/-   ##
=========================================
  Coverage     56.43%   56.43%           
  Complexity     1604     1604           
=========================================
  Files           331      331           
  Lines         12243    12243           
  Branches       1135     1135           
=========================================
  Hits           6909     6909           
  Misses         4887     4887           
  Partials        447      447           
Flag Coverage Δ
funTest-docker 70.59% <ø> (ø)
funTest-non-docker 33.42% <ø> (ø)
test-ubuntu-24.04 40.45% <ø> (ø)
test-windows-2022 40.43% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Implement functionality to generate snippet findings reports from
SCANOSS scan results.

Signed-off-by: Agustin Isasmendi <[email protected]>
Include releaseDate in snippetFindings additionalData and add a new
column to display this information in generated reports.

Signed-off-by: Agustin Isasmendi <[email protected]>
* Generate one Snippet for each detected line range
* Remove duplicate licenses to optimize results
* Remove identified snippets from the summary

Signed-off-by: Agustin Isasmendi <[email protected]>
@isasmendiagus isasmendiagus force-pushed the feat/scanoss/reporting-implementation branch from c29efef to 425cc72 Compare May 8, 2025 11:17
@sschuberth
Copy link
Member

sschuberth commented May 8, 2025

I'd like to comment on this draft already, to avoid going it into an undesired direction eventually:

While I agree with @nnobelis saying here that we should not spread conditionals all over the place in reporter templates, I still envision having a single snippet report for any snippet scanner.

Instead of having conditionals in the reporter templates, we should introduce variables for any information that can change among snippet scanners, make these available to the reporter, and use them in the template without conditionals. Ideally, such a generic snippet scanner report template should also work for the case when multiple snippet scanners were enabled in a single ort scan run.

So my "wish" for an approach would be to:

  1. generalize the existing FossIdSnippetReporter (which is completely separate / different from FossIdReporter) into a SnippetScannerReporter.
  2. Nothing more to do 😄 Because ideally, that generic SnippetScannerReporter "just works" with scan results that include SCANOSS data.

@isasmendiagus
Copy link
Contributor Author

I'm wondering who should work on creating this generic SnippetScannerReporter? Should I try to adapt the existing FossIdSnippetReporter into a more general solution as part of my current PR?

If this makes sense, maybe we could take a iterative approach:

  • First iteration: Create a generic reporter that works with one scanner at a time (not supporting multiple scanners simultaneously). This means we could use it with SCANOSS scan output and then separately with FossID scan output.

  • Future iteration: Enhance the reporter to support multiple scanners in a single ort scan run.

@isasmendiagus
Copy link
Contributor Author

I believe that commit 425cc72 refactor(scanoss): ScanOssResultParser to improve snippets findings should probably be in its own PR since it's really about fixing and improving the ScanOssResultParser rather than report generation.

If you agree, I'll remove it from the current PR and create a separate one

@sschuberth
Copy link
Member

If this makes sense, maybe we could take a iterative approach:

Yes, that makes sense to me. We should start with a separate PR that generalizes the existing FossIdSnippetReporter into a SnippetScannerReporter, only considering the case where a single snippet scanner was used to create the ORT result (because I believe also the current FossIdSnippetReporter assumes that only a single snippet scanner, namely FossID, has been used).

But I would also like @nnobelis to agree (and help with) this approach.

@sschuberth
Copy link
Member

If you agree, I'll remove it from the current PR and create a separate one

Yes, I was about to suggest that any changes to plugins/scanners/scanoss/src/test/kotlin/*.kt files in this PR could / should already be submitted, independently of having a reporter ready.

@nnobelis
Copy link
Member

nnobelis commented May 9, 2025

@isasmendiagus
@sschuberth

Feel free to replace the FossIdReporter by a SnipperScannerReporter. I can then review the PR.

we should introduce variables for any information that can change among snippet scanners, make these available to the reporter, and use them in the template without conditionals.

I am not sure what you mean by "variables" though.

Let's take a concrete example: The FossId report contains a column snippet.provenance.sourceArtifact.url that is not available in SCANOSS.

How would you prevent an if in the template and still do some conditional rendering ?
Bear in mind that it also has some consequence in the template columns definitions:

@sschuberth
Copy link
Member

How would you prevent an if in the template and still do some conditional rendering ?

Ok, that's a different case than what I had in mind. This is not strictly scanner-specific, but you conditionally want to display a generic property depending on whether it's set or not. I also don't see how to avoid conditionals here.

What I was referring to is template logic like (pseudo-code ahead)

if (scanner == fossid) {
    "$fossid-specific-property"
} else if (scanner == scanoss) {
    "$scanoss-specific-property"
}

Instead of that, we should do

"$generic-property"

in the template, and on the reporter side / host code in ORT ensure that generic-property is set correctly depending on the snippet scanner in use. That does not avoid the conditional completely, but it moves it from the template to the reporter logic which is easier to maintain.

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.

3 participants