Skip to content

Sarif reporting#1660

Open
matejdro wants to merge 2 commits intoautonomousapps:mainfrom
matejdro:sarif-reporting
Open

Sarif reporting#1660
matejdro wants to merge 2 commits intoautonomousapps:mainfrom
matejdro:sarif-reporting

Conversation

@matejdro
Copy link
Copy Markdown

@matejdro matejdro commented Mar 27, 2026

PR adds an ability for the plugin to also export the report in the standard .sarif format, in addition to the txt.

Would appreciate feedback on some things:

  • For the best experience, issues in sarif file should have line numbers attached, but no such information exists in the DAGP. I have attempted to infer those line numbers by reading through the build.gradle(.kts) file and just string matching of the advice's coordinates. Is that okay? Could we come up with a better way to do this?
  • I have only enabled this in the buildHealth task, but not the projectHealth. The issue is that individual projects do not have access to the reporting config and thus could not enable this. If necessary, I could add public fun reporting(action: Action<ReportingHandler>) to the DependencyAnalysisSubExtension and add sarif output to the projectHealth
  • Any hints on how can I add tests for this? From what I saw, there are no existing tests checking report output, so I don't have any template to follow.

(This PR is based on the 3.5.1 due to #1653, as it makes testing easier for me. When the code is final, I can resolve conflicts and merge latest version in)

this fixes #1094

Copy link
Copy Markdown
Owner

@autonomousapps autonomousapps left a comment

Choose a reason for hiding this comment

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

Thanks for this PR. I have not done a thorough review because I don't have time at the moment, but I'm very open to seeing this progress and eventually get merged.

We ought to be able to resolve the issue with projectHealth not having access to the reporting handler. I think for this to be considered feature-complete, it would have to work for both buildHealth and projectHealth.

The build file scanning may have some issues, but I haven't thoroughly inspected it. As you know, the plugin's analysis uses Gradle's APIs to find dependencies—it doesn't actually scan build scripts unless it's trying to rewrite them. So that'll have to be done on a best-effort basis (which is what I think you're doing).


private val SARIF_TOOL = Tool(
driver = ToolComponent(
guid = "f9137358-f4fb-44f2-8300-39ca0b85fb77",
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

What is the guid?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

From what I understand, it's just a static unique identifiable ID for every tool. I just generated a random UUID and plopped it there.

fromConfiguration = theRemove.fromConfiguration!!,
toConfiguration = theAdd.toConfiguration!!
toConfiguration = theAdd.toConfiguration!!,
declarationLineNumber = theRemove.buildFileDeclarationLineNumber ?: theAdd.buildFileDeclarationLineNumber
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

theAdd won't have a current declaration. The only things that would have a current declaration relate to remove advice and change advice.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ah, you are right. Do I remove this? I have added it to all types just for consistency's sake.

@autonomousapps autonomousapps added the enhancement New feature or request label Apr 2, 2026
@autonomousapps
Copy link
Copy Markdown
Owner

I had a thought about this and I think I do not want to modify the Advice class. It's already quite complicated and I think that adding more properties to it is the wrong direction. In particular, it means you have to have the nullable line number available at construction time, which really complicates things. Instead, I propose that the report-generating task in the project (not buildHealth, to be clear) optionally* iterate over the advice and write out a new output with a new model that includes the advice and the line number. That new class might look like this:

*this should be opt-in, based on user preference.

public data class SarifAdvice( // name TBD
  advice: Advice,
  lineNumber: Int?,
)

And then we could emit a sorted set of SarifAdvice in json form to disk. That could then be further processed by projectHealth and buildHealth.

There are already examples in the project of configuring something in the root and having that value picked up safely in subprojects. If you need pointers, please let me know.

@matejdro
Copy link
Copy Markdown
Author

So you would want to generate yet another json next to the regular advice json just for an extra property and then consume that json in addition to the regular json? Sounds a pretty complicated thing to do.

Or did I miss the intention here?

@autonomousapps
Copy link
Copy Markdown
Owner

So you would want to generate yet another json next to the regular advice json just for an extra property and then consume that json in addition to the regular json? Sounds a pretty complicated thing to do.

Or did I miss the intention here?

Yes, that's correct. And also that feature should be opt-in, based on user preference (as expressed in the reporting DSL).

In my opinion, this is actually easier than what you've spiked here. json is cheap.

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.

Generate output for project/build health in machine-readable format

2 participants