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

fix: Add dummy location data to conform to SARIF spec #988

Merged
merged 6 commits into from
May 22, 2023

Conversation

DaveTryon
Copy link
Contributor

@DaveTryon DaveTryon commented May 5, 2023

Details

As called out in #961, our SARIF doesn't have a fully formed location. Since we don't have meaningful data for this, the "least bad" solution was to insert dummy data for the startLine property. This PR includes 2 code changes, plus a bunch of refreshed SARIF files generated via the CLI tool. I noticed that the refreshed version of basic-axe-3.2.2.sarif is pretty-printed differently than what we had before. I'm guessing that the tool shifted at some point and that this is the first time the file has been regenerated with the current tool.

Motivation

Address #961 for better tool compatibility

Context

Pull request checklist

  • PR title respects Conventional Commits (starts with fix:, feat:, etc, and is suitable for user-facing release notes)
  • PR contains no breaking changes, OR description of both PR and final merge commit starts with BREAKING CHANGE:
  • (if applicable) Addresses issue: Generated SARIF is incompatible with codeql schema #961
  • [n/a] Added relevant unit tests for your changes
  • Ran yarn precheckin
  • Verified code coverage for the changes made

@DaveTryon DaveTryon requested a review from a team as a code owner May 5, 2023 17:59
@@ -15,7 +15,7 @@ import { EnvironmentData } from './environment-data';
import { getInvocations } from './invocation-provider';
import { ResultToRuleConverter } from './result-to-rule-converter';
import { formatSarifResultMessage } from './sarif-result-message-formatter';
import { axeTagsToWcagLinkData, WCAGLinkData } from './wcag-link-data';
import { WCAGLinkData, axeTagsToWcagLinkData } from './wcag-link-data';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Something seems to have changed in prettier

@@ -15,7 +15,7 @@ import { getEnvironmentDataFromResults } from './environment-data-provider';
import { getInvocations } from './invocation-provider';
import { ResultToRuleConverter } from './result-to-rule-converter';
import { formatSarifResultMessage } from './sarif-result-message-formatter';
import { axeTagsToWcagLinkData, WCAGLinkData } from './wcag-link-data';
import { WCAGLinkData, axeTagsToWcagLinkData } from './wcag-link-data';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Something seems to have changed in prettier

@DaveTryon DaveTryon merged commit 5117010 into microsoft:main May 22, 2023
@ada-cat
Copy link
Collaborator

ada-cat commented Jun 5, 2023

🎉 This PR is included in version 2.10.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@DaveTryon DaveTryon deleted the add-dummy-location branch September 26, 2023 22:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants