Add sarif support to Invert using sarif4k #60

mannydelgado · 2025-05-19T19:59:35Z

This pull request introduces support for generating SARIF (Static Analysis Results Interchange Format) reports in the invert-gradle-plugin. The changes include adding a new InvertSarifReportWriter class, updating existing components to integrate SARIF report generation, and modifying configurations to support SARIF output.

SARIF Report Generation

Added the InvertSarifReportWriter class to handle the creation of SARIF reports, including methods for converting statistics data into SARIF-compliant rules and results, and writing these reports to files. (invert-gradle-plugin/src/main/kotlin/com/squareup/invert/internal/report/sarif/InvertSarifReportWriter.kt)
Extended Stat and StatMetadata with helper methods to convert statistics and metadata into SARIF-compatible formats. (invert-gradle-plugin/src/main/kotlin/com/squareup/invert/internal/report/sarif/InvertSarifReportWriter.kt)

Integration with Existing Reporting

Updated CollectedStatAggregator to generate SARIF reports for code references using the new InvertSarifReportWriter.writeToSarifReport method. (invert-gradle-plugin/src/main/kotlin/com/squareup/invert/internal/CollectedStatAggregator.kt)
Modified InvertReportWriter to include all project statistics in a single SARIF report by calling InvertSarifReportWriter.createInvertSarifReport. (invert-gradle-plugin/src/main/kotlin/com/squareup/invert/internal/report/InvertReportWriter.kt)

Configuration Updates

Added a new dependency on the sarif4k library for SARIF serialization. (invert-gradle-plugin/build.gradle.kts)
Introduced a new constant SARIF_FOLDER_NAME in InvertFileUtils for organizing SARIF report files. (invert-gradle-plugin/src/main/kotlin/com/squareup/invert/internal/InvertFileUtils.kt)
Added a new file key STATS_SARIF to InvertPluginFileKey for identifying SARIF report files. (invert-gradle-plugin/src/main/kotlin/com/squareup/invert/internal/models/InvertPluginFileKey.kt)

Attached is an example of the output from the Sarif writer.
sarif.zip

Copilot

Pull Request Overview

This pull request adds SARIF report support to Invert using the sarif4k library. Key changes include the introduction of InvertSarifReportWriter to generate SARIF files, integration of SARIF reporting in existing report writers and stat aggregation, and updating related file utils and build files to support SARIF output.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
invert-gradle-plugin/src/main/kotlin/com/squareup/invert/internal/report/sarif/InvertSarifReportWriter.kt	New writer for SARIF reports using sarif4k
invert-gradle-plugin/src/main/kotlin/com/squareup/invert/internal/report/InvertReportWriter.kt	Integration of SARIF report generation alongside JS/HTML reports
invert-gradle-plugin/src/main/kotlin/com/squareup/invert/internal/models/InvertPluginFileKey.kt	Addition of a new STATS_SARIF file key
invert-gradle-plugin/src/main/kotlin/com/squareup/invert/internal/InvertFileUtils.kt	New constant for SARIF folder */
invert-gradle-plugin/src/main/kotlin/com/squareup/invert/internal/CollectedStatAggregator.kt	Added static SARIF report generation for stat data
invert-gradle-plugin/build.gradle.kts	Added dependency on sarif4k

...-plugin/src/main/kotlin/com/squareup/invert/internal/report/sarif/InvertSarifReportWriter.kt

Copilot

Pull Request Overview

This PR adds SARIF report support to the invert-gradle-plugin by integrating the sarif4k library and providing utilities and writers to generate SARIF-compliant output alongside existing reports.

Introduces InvertSarifReportWriter and InvertSarifUtils to convert statistics into SARIF format.
Integrates SARIF generation in InvertReportWriter and CollectedStatAggregator.
Updates build and file utilities to support SARIF files.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
InvertSarifReportWriterTest.kt	Adds tests for SARIF report writer functionality
InvertSarifUtils.kt	Defines extension functions to map stats to SARIF structures
InvertSarifReportWriter.kt	Implements writing SARIF files using `sarif4k` serializer
InvertReportWriter.kt	Calls SARIF writer as part of overall report generation
InvertPluginFileKey.kt	Adds new enum key for SARIF report files
InvertFileUtils.kt	Introduces folder constant for SARIF reports
CollectedStatAggregator.kt	Emits individual SARIF files for code references
build.gradle.kts	Adds `sarif4k` dependency

Comments suppressed due to low confidence (5)

invert-gradle-plugin/src/main/kotlin/com/squareup/invert/internal/models/InvertPluginFileKey.kt:18

[nitpick] The display name for STATS_SARIF is generic ("Stats"); consider renaming it to something like "Stats SARIF" for clarity.

STATS_SARIF("stats.sarif", "Stats"),

invert-gradle-plugin/src/main/kotlin/com/squareup/invert/internal/report/sarif/InvertSarifUtils.kt:30

There are no unit tests covering the asSarifResult and determineSourceLanguage extension functions. Consider adding tests for these to ensure correct mapping for various file extensions and stat types.

fun Stat.asSarifResult(

invert-gradle-plugin/src/main/kotlin/com/squareup/invert/internal/CollectedStatAggregator.kt:104

The call to InvertFileUtils.outputFile below references InvertFileUtils, but there is no import for com.squareup.invert.internal.InvertFileUtils in this file. Add the import to resolve the reference.

InvertSarifReportWriter.writeToSarifReport(

invert-gradle-plugin/src/main/kotlin/com/squareup/invert/internal/report/sarif/InvertSarifReportWriter.kt:69

The constant SARIF_FILE_NAME is declared but never used; consider removing it or using it in writeToSarifReport to avoid dead code.

private const val SARIF_FILE_NAME = "invert-report.sarif"

invert-gradle-plugin/src/test/kotlin/com/squareup/invert/internal/report/sarif/InvertSarifReportWriterTest.kt:10

[nitpick] Mixing TestNG assertions with Kotlin test assertions may confuse test consistency. Consider using kotlin.test.assertEquals instead of importing TestNG.

import org.gradle.internal.impldep.org.testng.Assert.assertEquals

handstandsam

Please consider the comments. Nothing is a blocker from merging and doing follow ups, but if it is straightforward enough please update some in PR. Leaving this task to acknowledge but approving

invert-gradle-plugin/build.gradle.kts

handstandsam · 2025-05-23T15:08:12Z

invert-gradle-plugin/src/main/kotlin/com/squareup/invert/internal/InvertFileUtils.kt

  const val INVERT_FOLDER_NAME = "invert"
  const val JS_FOLDER_NAME = "js"
  const val JSON_FOLDER_NAME = "json"
+  const val SARIF_FOLDER_NAME = "sarif"


handstandsam · 2025-05-23T15:10:50Z

invert-gradle-plugin/src/main/kotlin/com/squareup/invert/internal/models/InvertPluginFileKey.kt

  OWNERS("owners.json", "Owners"),
  METADATA("metadata.json", "Metadata"),
  STATS("stats.json", "Stats"),
+  STATS_SARIF("stats.sarif", "Stats"),


Hmmm, i thought i deleted stats.json, but maybe that was only stats.js. This is fine but the concern is that the single file holds all data and that doesn't scale well. For the js/web we were forced to do that to collect things like all kotlin files. This can merge, but we should discuss how to get rid of those mega files eventually

Yeah we are splitting it up as well, but I saw that json was doing one whole file so I followed suit

if we don’t need this then I can remove for sure

...-plugin/src/main/kotlin/com/squareup/invert/internal/report/sarif/InvertSarifReportWriter.kt

handstandsam · 2025-05-23T15:17:24Z

...-gradle-plugin/src/main/kotlin/com/squareup/invert/internal/report/sarif/InvertSarifUtils.kt

+                ),
+                properties = PropertyBag(
+                    extras + mapOf(
+                        "fileType" to filePath.split(".").last()


Is fileType required?

I thought this was a nice to have but not required no we can remove

...-gradle-plugin/src/main/kotlin/com/squareup/invert/internal/report/sarif/InvertSarifUtils.kt

handstandsam · 2025-05-23T15:20:22Z

...-gradle-plugin/src/main/kotlin/com/squareup/invert/internal/report/sarif/InvertSarifUtils.kt

+/**
+ * Simple function to determine the source language based on the file path.
+ */
+private fun determineSourceLanguage(filePath: String): String {


Is this a sarif standard? Is that part of the library or do we need our own manual table?

this is another one of those nice to haves but can be removed, sourceLanguage isnt something thats necessary to build the sarif schema but it is an optional item.

as far as if this is part of the Library, its not unfortunately it would have been so nice not having to manually maintain this part.

but can be removed if we deem its not necessary

handstandsam · 2025-05-23T15:20:55Z

...gin/src/test/kotlin/com/squareup/invert/internal/report/sarif/InvertSarifReportWriterTest.kt

+import kotlin.test.assertTrue
+import java.io.File
+
+class InvertSarifReportWriterTest {


mannydelgado added 2 commits May 19, 2025 15:45

Add sarif support to Invert using sarif4k

df93951

Cleanup Sarif report writer

ee87848

mannydelgado requested review from Copilot and handstandsam May 19, 2025 19:59

Copilot AI reviewed May 19, 2025

View reviewed changes

...-plugin/src/main/kotlin/com/squareup/invert/internal/report/sarif/InvertSarifReportWriter.kt Outdated Show resolved Hide resolved

mannydelgado added 3 commits May 20, 2025 10:37

Add determine source language private function

cee77a3

Add tests for InvertSarifRerportWriter

606dd7e

Restucture into InvertSarifUtils class

c92b8d8

mannydelgado requested a review from Copilot May 20, 2025 15:43

Copilot AI reviewed May 20, 2025

View reviewed changes

handstandsam approved these changes May 23, 2025

View reviewed changes

mannydelgado added 3 commits May 27, 2025 09:25

Cleanup formatting and adjust key names

9e31943

Lowercase invert for test case.

9330b64

Use invert as the default rule id for all

3f0835f

mannydelgado merged commit d1babd2 into square:main May 27, 2025
1 check passed

Add sarif support to Invert using sarif4k #60

Add sarif support to Invert using sarif4k #60

Uh oh!

Conversation

mannydelgado commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SARIF Report Generation

Integration with Existing Reporting

Configuration Updates

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

handstandsam left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mannydelgado commented May 19, 2025 •

edited

Loading