◐ Shell
reader mode source ↗
Skip to content

Fix #9972 (Add support for SARIF output format)#6863

Merged
danmar merged 10 commits into
cppcheck-opensource:mainfrom
cppchecksolutions:fix-9972-2
Oct 10, 2024
Merged

Fix #9972 (Add support for SARIF output format)#6863
danmar merged 10 commits into
cppcheck-opensource:mainfrom
cppchecksolutions:fix-9972-2

Conversation

@danmar

@danmar danmar commented Oct 5, 2024

Copy link
Copy Markdown
Collaborator

No description provided.

@danmar

danmar commented Oct 5, 2024

Copy link
Copy Markdown
Collaborator Author

There is no testing yet. I will see if I can integrate it into github.

@danmar

danmar commented Oct 5, 2024

Copy link
Copy Markdown
Collaborator Author

@mario-campos would you be interested to test/review this Cppcheck SARIF output?

@firewave

firewave commented Oct 5, 2024

Copy link
Copy Markdown
Collaborator

See #4651 and #3365.

@danmar

danmar commented Oct 5, 2024

Copy link
Copy Markdown
Collaborator Author

See 4651 and 3365.

4651: very related but this is a simpler approach.

3365: I don't think this is related to the uniq/append output.

@danmar

danmar commented Oct 5, 2024

Copy link
Copy Markdown
Collaborator Author

I believe that we can output findings directly btw. We don't have to construct the complete report at the end. However I am not sure if that will be very useful anyway, the report is so verbose I don't think anybody would like to read the sarif output "live".

I believe a minimal SARIF header would be:

{
  "runs": [
    {
      "results": [

@danmar danmar force-pushed the fix-9972-2 branch 2 times, most recently from a9065ff to a798d28 Compare October 6, 2024 14:36
@github-advanced-security

github-advanced-security AI commented Oct 6, 2024

Copy link
Copy Markdown

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@danmar danmar mentioned this pull request Oct 6, 2024
Closed
@danmar

danmar commented Oct 6, 2024 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

There is an experimental SARIF upload here:
#6866

@mario-campos

mario-campos commented Oct 7, 2024

Copy link
Copy Markdown
Contributor

@danmar, sure. I can take a look at it later today.

mario-campos
mario-campos reviewed Oct 9, 2024
View reviewed changes

@mario-campos mario-campos left a comment •
edited
Loading

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.

Hide comment

Looks great!! Can't wait to see this merged!

I left a few comments about some missing SARIF properties, which GitHub requires. I think it would be a good idea to implement them, because I think someone will eventually try to upload the SARIF file to GitHub expecting it to work.

@danmar

danmar commented Oct 9, 2024

Copy link
Copy Markdown
Collaborator Author

@mario-campos thanks for the comments. Can you please check again?

Example output:

{
  "$schema": "https:\/\/docs.oasis-open.org\/sarif\/sarif\/v2.1.0\/errata01\/os\/schemas\/sarif-schema-2.1.0.json",
  "runs": [
    {
      "results": [
        {
          "level": "warning",
          "locations": [
            {
              "physicalLocation": {
                "artifactLocation": {
                  "uri": "1.c"
                },
                "region": {
                  "endColumn": 4,
                  "endLine": 1,
                  "startColumn": 4,
                  "startLine": 1
                }
              }
            }
          ],
          "message": {
            "text": "Division by zero."
          },
          "partialFingerprints": {
            "hash": "4df90040e1961f0f"
          },
          "ruleId": "zerodiv"
        }
      ],
      "tool": {
        "driver": {
          "informationUri": "https:\/\/cppcheck.sourceforge.io",
          "name": "Cppcheck Premium",
          "rules": [
            {
              "fullDescription": {
                "text": "Division by zero."
              },
              "help": {
                "text": "Division by zero."
              },
              "id": "zerodiv",
              "properties": {
                "precision": "high",
                "problem": {
                  "severity": "warning"
                }
              },
              "shortDescription": {
                "text": "Division by zero."
              }
            }
          ],
          "version": "24.9.0"
        }
      }
    }
  ],
  "version": "2.1.0"
}

@danmar

danmar commented Oct 9, 2024

Copy link
Copy Markdown
Collaborator Author

@mario-campos please take an extra careful look on the partialFingerprints , I am very unsure what that should be.

mario-campos
mario-campos reviewed Oct 9, 2024
View reviewed changes

@mario-campos mario-campos 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.

Hide comment

Nice job! A few more finishing touches.

Also, you can validate the SARIF file at https://sarifweb.azurewebsites.net/. I've already submitted it and there's a few "issues" but I wouldn't worry about them. According to the docs, they're not required.

danmar and others added 4 commits October 9, 2024 21:42
@danmar @mario-campos
Update cli/cppcheckexecutor.cpp …
9e1db25 
Co-authored-by: Mario Campos <mario-campos@github.com>
@danmar @mario-campos
Update cli/cppcheckexecutor.cpp …
5d76a94 
Co-authored-by: Mario Campos <mario-campos@github.com>
@danmar
review comments
fb9ca69
@danmar
fix
df6d289
danmar and others added 2 commits October 10, 2024 20:22
@danmar @mario-campos
Update cli/cppcheckexecutor.cpp …
816542c 
Co-authored-by: Mario Campos <mario-campos@github.com>
@danmar
test
26ca959
@danmar danmar merged commit c49b941 into cppcheck-opensource:main Oct 10, 2024
@danmar

danmar commented Oct 10, 2024

Copy link
Copy Markdown
Collaborator Author

@mario-campos I merged it now. but please feel free to provide additional advice if there is something we can fix..

@danmar danmar deleted the fix-9972-2 branch October 10, 2024 20:12
ludviggunne pushed a commit to ludviggunne/cppcheck that referenced this pull request Oct 19, 2024
@danmar @mario-campos
Fix #9972 (Add support for SARIF output format) (cppcheck-opensource#… …
f6e77ba 
…6863)

Co-authored-by: Mario Campos <mario-campos@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@firewave firewave firewave left review comments

+1 more reviewer

@mario-campos mario-campos mario-campos left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@danmar @firewave @github-advanced-security @mario-campos