Skip to content
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

feat: collect clippy's results children spans in related_locations #585

Merged
merged 2 commits into from
Jul 8, 2024

Conversation

fcasal
Copy link
Contributor

@fcasal fcasal commented Jan 23, 2024

This PR collects clippy's diagnostic children span locations into the SARIF related locations.

This prevents clippy-sarif from removing relevant data that is present in clippy's json output.

As an example, running cargo clippy with cargo clippy --message-format=json on the following file

fn main() {
    let i = 3;
    let x = format!("{}", i);
}

generates a help diagnostic

{
                "children": [],
                "code": null,
                "level": "help",
                "message": "if this is intentional, prefix it with an underscore",
                "rendered": null,
                "spans": [
                    {
                        "byte_end": 36,
                        "byte_start": 35,
                        "column_end": 10,
                        "column_start": 9,
                        "expansion": null,
                        "file_name": "sarif-rs-example/src/main.rs",
                        "is_primary": true,
                        "label": null,
                        "line_end": 3,
                        "line_start": 3,
                        "suggested_replacement": "_x",
                        "suggestion_applicability": "MachineApplicable",
                        "text": [
                            {
                                "highlight_end": 10,
                                "highlight_start": 9,
                                "text": "    let x = format!(\"{}\", i);"
                            }
                        ]
                    }
                ]
            }

However, neither the message nor the suggestion shows up in the SARIF file:

"results": [
        {
          "level": "warning",
          "locations": [
            {
              "physicalLocation": {
                "artifactLocation": {
                  "uri": "sarif-rs-example/src/main.rs"
                },
                "region": {
                  "byteLength": 1,
                  "byteOffset": 35,
                  "endColumn": 10,
                  "endLine": 3,
                  "startColumn": 9,
                  "startLine": 3
                }
              }
            }
          ],
          "message": {
            "text": "unused variable: `x`"
          },
          "ruleId": "unused_variables",
          "ruleIndex": 0
        }
      ],

This PR adds a related location entry with this information:

"relatedLocations": [
            {
              "message": {
                "text": "if this is intentional, prefix it with an underscore \"_x\""
              },
              "physicalLocation": {
                "artifactLocation": {
                  "uri": "sarif-rs-example/src/main.rs"
                },
                "region": {
                  "byteLength": 1,
                  "byteOffset": 35,
                  "endColumn": 10,
                  "endLine": 3,
                  "startColumn": 9,
                  "startLine": 3
                }
              }
            }
          ],
          "ruleId": "unused_variables",
          "ruleIndex": 0
        }

Copy link
Owner

@psastras psastras left a comment

Choose a reason for hiding this comment

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

thanks! this looks good, a test case would be nice but fine as is

@psastras psastras enabled auto-merge (rebase) July 8, 2024 01:06
@psastras psastras merged commit aa1fe89 into psastras:main Jul 8, 2024
7 checks passed
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.

2 participants