Add support for specifying the severity of a SARIF Result.

[vaibhav.y]

Summary

• Extend SarifResult with level property, and allow rule configuration
• Create SarifReportingConfiguration which allow configuring rules with a default priority, severity and an enable-toggle
• Support for setting the level property[1] of a result.

If unset, it defaults to "warning", which is the result of an empty default configuration on rules[2]

[1]: https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317648
[2]: https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317855

[vaibhav.y]

Update diff

[vaibhav.y]

Rebase on main, set result level to warning by default.

[vaibhav.y]

• Add differential revision to individual commits
• Remove unused header

[vaibhav.y]

Fix typo in comment

[vaibhav.y]

Submitting for review:

Some notes:

There are a couple of ways I think we can acheive this, per the spec:

1. The reportingDescriptor (rule) objects can be given a default configuration property, which can set the default warning level and other data such as rule parameters etc.
2. The reportingDescriptor objects can omit the default configuration (which then allows operating with warning as default), and the level is then set when the result is reported.

The first approach would be "more correct", what are your thoughts on this? Would we benefit from having per-diagnostic configuration?

There is also the question about the "kind" of results in clang. From my reading of the spec, it seems that "fail" is the only case that applies to us because:

• "pass": Implies no issue was found.
• "open": This value is used by proof-based tools. It could also mean additional assertions required
• "informational": The specified rule was evaluated and produced a purely informational result that does not indicate the presence of a problem
• "notApplicable": The rule specified by ruleId was not evaluated, because it does not apply to the analysis target.

Of these "open" and "notApplicable" seem to be ones that *could* come to use but I'm not sure where / what kind of diagnostics would use these. Probably clang-tidy's bugprone-* suite?

Let me know what you think is a good way to approach this wrt clang's diagnostics system.

[cjdb]

In D131084#3697211, @vaibhav.y wrote:

Submitting for review:

Some notes:

There are a couple of ways I think we can acheive this, per the spec:

1. The reportingDescriptor (rule) objects can be given a default configuration property, which can set the default warning level and other data such as rule parameters etc.
2. The reportingDescriptor objects can omit the default configuration (which then allows operating with warning as default), and the level is then set when the result is reported.

The first approach would be "more correct", what are your thoughts on this? Would we benefit from having per-diagnostic configuration?

There is also the question about the "kind" of results in clang. From my reading of the spec, it seems that "fail" is the only case that applies to us because:

• "pass": Implies no issue was found.
• "open": This value is used by proof-based tools. It could also mean additional assertions required
• "informational": The specified rule was evaluated and produced a purely informational result that does not indicate the presence of a problem
• "notApplicable": The rule specified by ruleId was not evaluated, because it does not apply to the analysis target.

Of these "open" and "notApplicable" seem to be ones that *could* come to use but I'm not sure where / what kind of diagnostics would use these. Probably clang-tidy's bugprone-* suite?

Let me know what you think is a good way to approach this wrt clang's diagnostics system.

Hmm, we can probably use "informational" for notes, warnings, and remarks, but I'm kinda partial to proposing the latter two upstream.

[denik]

In D131084#3697256, @cjdb wrote:

In D131084#3697211, @vaibhav.y wrote:

Submitting for review:

Some notes:

There are a couple of ways I think we can acheive this, per the spec:

1. The reportingDescriptor (rule) objects can be given a default configuration property, which can set the default warning level and other data such as rule parameters etc.
2. The reportingDescriptor objects can omit the default configuration (which then allows operating with warning as default), and the level is then set when the result is reported.

The first approach would be "more correct", what are your thoughts on this? Would we benefit from having per-diagnostic configuration?

There is also the question about the "kind" of results in clang. From my reading of the spec, it seems that "fail" is the only case that applies to us because:

• "pass": Implies no issue was found.
• "open": This value is used by proof-based tools. It could also mean additional assertions required
• "informational": The specified rule was evaluated and produced a purely informational result that does not indicate the presence of a problem
• "notApplicable": The rule specified by ruleId was not evaluated, because it does not apply to the analysis target.

Of these "open" and "notApplicable" seem to be ones that *could* come to use but I'm not sure where / what kind of diagnostics would use these. Probably clang-tidy's bugprone-* suite?

Let me know what you think is a good way to approach this wrt clang's diagnostics system.

Hmm, we can probably use "informational" for notes, warnings, and remarks, but I'm kinda partial to proposing the latter two upstream.

I think we can skip kind in the clang diagnostic. I found this:

If kind is absent, it SHALL default to "fail".
If level has any value other than "none" and kind is present, then kind SHALL have the value "fail".

[vaibhav.y]

Hmm, we can probably use "informational" for notes, warnings, and remarks, but I'm kinda partial to proposing the latter two upstream.

Interesting, I agree that remarks could fit under "informational".

As for notes: As I understand it they are always child diagnostics of warnings/errors, so seems it would be okay to associate these with the "fail" kind.

[cjdb]

It seems I got confused and conflated the two this morning.

In D131084#3697525, @vaibhav.y wrote:

Hmm, we can probably use "informational" for notes, warnings, and remarks, but I'm kinda partial to proposing the latter two upstream.

Interesting, I agree that remarks could fit under "informational".

As for notes: As I understand it they are always child diagnostics of warnings/errors, so seems it would be okay to associate these with the "fail" kind.

I don't think classifying warnings as "fail" is right unless -Werror is being used. For notes, I think there may be two options, if we don't want to use informational:

• inherit from the parent diagnostic
• use notApplicable, since they don't have any influence on their own

In D131084#3697308, @denik wrote:

In D131084#3697256, @cjdb wrote:

In D131084#3697211, @vaibhav.y wrote:

Submitting for review:

Some notes:

There are a couple of ways I think we can acheive this, per the spec:

1. The reportingDescriptor (rule) objects can be given a default configuration property, which can set the default warning level and other data such as rule parameters etc.
2. The reportingDescriptor objects can omit the default configuration (which then allows operating with warning as default), and the level is then set when the result is reported.

The first approach would be "more correct", what are your thoughts on this? Would we benefit from having per-diagnostic configuration?

There is also the question about the "kind" of results in clang. From my reading of the spec, it seems that "fail" is the only case that applies to us because:

• "pass": Implies no issue was found.
• "open": This value is used by proof-based tools. It could also mean additional assertions required
• "informational": The specified rule was evaluated and produced a purely informational result that does not indicate the presence of a problem
• "notApplicable": The rule specified by ruleId was not evaluated, because it does not apply to the analysis target.

Of these "open" and "notApplicable" seem to be ones that *could* come to use but I'm not sure where / what kind of diagnostics would use these. Probably clang-tidy's bugprone-* suite?

Let me know what you think is a good way to approach this wrt clang's diagnostics system.

Hmm, we can probably use "informational" for notes, warnings, and remarks, but I'm kinda partial to proposing the latter two upstream.

I think we can skip kind in the clang diagnostic. I found this:

If kind is absent, it SHALL default to "fail".
If level has any value other than "none" and kind is present, then kind SHALL have the value "fail".

If skipping the kind defaults to fail, then we shouldn't skip it, because getting a warning or remark doesn't necessarily mean that the build has failed.

[aaron.ballman]

In D131084#3697678, @cjdb wrote:

It seems I got confused and conflated the two this morning.

In D131084#3697525, @vaibhav.y wrote:

Hmm, we can probably use "informational" for notes, warnings, and remarks, but I'm kinda partial to proposing the latter two upstream.

Interesting, I agree that remarks could fit under "informational".

As for notes: As I understand it they are always child diagnostics of warnings/errors, so seems it would be okay to associate these with the "fail" kind.

I don't think classifying warnings as "fail" is right unless -Werror is being used. For notes, I think there may be two options, if we don't want to use informational:

• inherit from the parent diagnostic
• use notApplicable, since they don't have any influence on their own

In D131084#3697308, @denik wrote:

In D131084#3697256, @cjdb wrote:

In D131084#3697211, @vaibhav.y wrote:

Submitting for review:

Some notes:

There are a couple of ways I think we can acheive this, per the spec:

1. The reportingDescriptor (rule) objects can be given a default configuration property, which can set the default warning level and other data such as rule parameters etc.
2. The reportingDescriptor objects can omit the default configuration (which then allows operating with warning as default), and the level is then set when the result is reported.

The first approach would be "more correct", what are your thoughts on this? Would we benefit from having per-diagnostic configuration?

There is also the question about the "kind" of results in clang. From my reading of the spec, it seems that "fail" is the only case that applies to us because:

• "pass": Implies no issue was found.
• "open": This value is used by proof-based tools. It could also mean additional assertions required
• "informational": The specified rule was evaluated and produced a purely informational result that does not indicate the presence of a problem
• "notApplicable": The rule specified by ruleId was not evaluated, because it does not apply to the analysis target.

Of these "open" and "notApplicable" seem to be ones that *could* come to use but I'm not sure where / what kind of diagnostics would use these. Probably clang-tidy's bugprone-* suite?

Let me know what you think is a good way to approach this wrt clang's diagnostics system.

Hmm, we can probably use "informational" for notes, warnings, and remarks, but I'm kinda partial to proposing the latter two upstream.

I think we can skip kind in the clang diagnostic. I found this:

If kind is absent, it SHALL default to "fail".
If level has any value other than "none" and kind is present, then kind SHALL have the value "fail".

If skipping the kind defaults to fail, then we shouldn't skip it, because getting a warning or remark doesn't necessarily mean that the build has failed.

My suggestion is that we only need to worry about two kinds: fail for warnings and errors and informational for remarks. When the kind if fail, I'd expect we'd use the level warning or error.

Notes are interesting though. Sometimes they're extra information about the diagnostic on the given line, so I was kind of expecting they'd be specified via attachments in that case. But they can also sometimes be used to represent code flow (like in some CFG based warnings where we're showing where something was called from), and they can also sometimes be a related location (like "declared here" notes). Perhaps we may want to expose those to SARIF in different ways depending on the situation?

[cjdb]

In D131084#3699223, @aaron.ballman wrote:

In D131084#3697678, @cjdb wrote:

It seems I got confused and conflated the two this morning.

In D131084#3697525, @vaibhav.y wrote:

Hmm, we can probably use "informational" for notes, warnings, and remarks, but I'm kinda partial to proposing the latter two upstream.

Interesting, I agree that remarks could fit under "informational".

As for notes: As I understand it they are always child diagnostics of warnings/errors, so seems it would be okay to associate these with the "fail" kind.

I don't think classifying warnings as "fail" is right unless -Werror is being used. For notes, I think there may be two options, if we don't want to use informational:

• inherit from the parent diagnostic
• use notApplicable, since they don't have any influence on their own

In D131084#3697308, @denik wrote:

In D131084#3697256, @cjdb wrote:

In D131084#3697211, @vaibhav.y wrote:

Submitting for review:

Some notes:

There are a couple of ways I think we can acheive this, per the spec:

1. The reportingDescriptor (rule) objects can be given a default configuration property, which can set the default warning level and other data such as rule parameters etc.
2. The reportingDescriptor objects can omit the default configuration (which then allows operating with warning as default), and the level is then set when the result is reported.

The first approach would be "more correct", what are your thoughts on this? Would we benefit from having per-diagnostic configuration?

There is also the question about the "kind" of results in clang. From my reading of the spec, it seems that "fail" is the only case that applies to us because:

• "pass": Implies no issue was found.
• "open": This value is used by proof-based tools. It could also mean additional assertions required
• "informational": The specified rule was evaluated and produced a purely informational result that does not indicate the presence of a problem
• "notApplicable": The rule specified by ruleId was not evaluated, because it does not apply to the analysis target.

Of these "open" and "notApplicable" seem to be ones that *could* come to use but I'm not sure where / what kind of diagnostics would use these. Probably clang-tidy's bugprone-* suite?

Let me know what you think is a good way to approach this wrt clang's diagnostics system.

Hmm, we can probably use "informational" for notes, warnings, and remarks, but I'm kinda partial to proposing the latter two upstream.

I think we can skip kind in the clang diagnostic. I found this:

If kind is absent, it SHALL default to "fail".
If level has any value other than "none" and kind is present, then kind SHALL have the value "fail".

If skipping the kind defaults to fail, then we shouldn't skip it, because getting a warning or remark doesn't necessarily mean that the build has failed.

My suggestion is that we only need to worry about two kinds: fail for warnings and errors and informational for remarks. When the kind if fail, I'd expect we'd use the level warning or error.

Notes are interesting though. Sometimes they're extra information about the diagnostic on the given line, so I was kind of expecting they'd be specified via attachments in that case. But they can also sometimes be used to represent code flow (like in some CFG based warnings where we're showing where something was called from), and they can also sometimes be a related location (like "declared here" notes). Perhaps we may want to expose those to SARIF in different ways depending on the situation?

I proposed relaxing the kind upstream last night., since I think that it might be good to discern when a warning isn't an error with warning/informational, because then consumers could trivially treat them differently. I think review is the best one for remarks, because some of them do communicate potential problems (remark_fe_backend_optimization_remark_analysis_aliasing is the only public one I can see that's like this, but I do recall a few from ComputeCpp too (cc @Naghasan). Perhaps these ones should instead be promoted to warnings?)

Notes that are exposed as SARIF could be none/informational, while remarks are none/review? A part of my endgame is to see notes be incorporated into their parents, but that's a long way off methinks.

[vaibhav.y]

A part of my endgame is to see notes be incorporated into their parents, but that's a long way off methinks.

Regarding this, the current best approach the spec provides is using the "locationRelationShip", but the relationships that exist dont' seem to cover cases needed by us. One example is macro-expansion (example from an older proposal for SARIF in clang).

Seems that for locationRelationShip the spec allows producer defined strings:

A locationRelationship object MAY contain a property named kinds whose value is an array of one or more unique (§3.7.3) strings each of which specifies a relationship between theSource and theTarget (see §3.34.1). If kinds is absent, it SHALL default to [ "relevant" ] (see below for the meaning of "relevant").

When possible, SARIF producers SHOULD use the following values, with the specified meanings.

· "includes": The artifact identified by theSource includes the artifact identified by theTarget.

· "isIncludedBy": The artifact identified by theSource is included by the artifact identified by theTarget.

· "relevant": theTarget is relevant to theSource in a way not covered by other relationship kinds.

If none of these values are appropriate, a SARIF producer MAY use any value.

NOTE: Although "relevant" is a catch-all for any relationship not described by the other values, a producer might still wish to define its own more specific values.

In particular, the values defined for logicalLocation.kind (§3.33.7) and threadFlowLocation.kinds (§3.38.8) might prove useful.

[abrahamcd]

In D131084#3697211, @vaibhav.y wrote:

Submitting for review:

Some notes:

There are a couple of ways I think we can acheive this, per the spec:

1. The reportingDescriptor (rule) objects can be given a default configuration property, which can set the default warning level and other data such as rule parameters etc.
2. The reportingDescriptor objects can omit the default configuration (which then allows operating with warning as default), and the level is then set when the result is reported.

The first approach would be "more correct", what are your thoughts on this? Would we benefit from having per-diagnostic configuration?

There is also the question about the "kind" of results in clang. From my reading of the spec, it seems that "fail" is the only case that applies to us because:

• "pass": Implies no issue was found.
• "open": This value is used by proof-based tools. It could also mean additional assertions required
• "informational": The specified rule was evaluated and produced a purely informational result that does not indicate the presence of a problem
• "notApplicable": The rule specified by ruleId was not evaluated, because it does not apply to the analysis target.

Of these "open" and "notApplicable" seem to be ones that *could* come to use but I'm not sure where / what kind of diagnostics would use these. Probably clang-tidy's bugprone-* suite?

Let me know what you think is a good way to approach this wrt clang's diagnostics system.

@cjdb, @denik, and I discussed the two approaches yesterday, and we thought the first would be the better option (add setDefaultConfig to SarifRule instead of setDiagnosticLevel in SarifResult). We could make use of the defaultConfiguration's "level", "enabled", and "rank" properties to define the different diagnostic types that are currently present in Clang, and it would consolidate that information in a single rule rather than copying it across every result related to that rule. Then we could just have a small set of defaultConfigurations that correspond to the current Clang diagnostic categories that we can choose from when making new rules.

[vaibhav.y]

Update diff:

• Create class SarifReportingConfiguration:

This should allow rules to specify the defaultConfiguration property

Add tests for:

• Creating rules with custom reporting configuration
• Death test for invalid reporting configuration rank
• Death test for creating a result that references a disabled rule

[vaibhav.y]

clang/include/clang/Basic/Sarif.h

256 ↗

(On Diff #451585)

Is this form preferred for expressing a statically known default value?

[vaibhav.y]

Autofix clang-tidy readability warnings

[vaibhav.y]

Run git-clang-format

Rebase on upstream

[aaron.ballman]

clang/include/clang/Basic/Sarif.h

157 ↗

(On Diff #451635)

Should this include Remark for diagnostics like: https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/DiagnosticFrontendKinds.td#L55

If not, I think the comments should explain how to map remarks to one of these levels.

clang/lib/Basic/Sarif.cpp

25 ↗

(On Diff #451635)

Why was this include required?

396–399 ↗

(On Diff #451635)

(Probably have to re-run clang-format)

[vaibhav.y]

clang/include/clang/Basic/Sarif.h

157 ↗

(On Diff #451635)

Ack, if we're adding remark, should I also add support the "informational"/"fail" result kind that was previously discussed by you? Currently there's no kind, so we are defaulting to "fail".

clang/lib/Basic/Sarif.cpp

25 ↗

(On Diff #451635)

It provides llvm_unreachable. so far it was available transitively, must have been added when I implemented resultLevelToStr(...).

I don't feel strongly about keeping this though.

396–399 ↗

(On Diff #451635)

Good catch! I notice Optional<T> has value_or, is that okay as well here?

[aaron.ballman]

clang/include/clang/Basic/Sarif.h

157 ↗

(On Diff #451635)

I think we should, but it could be done in a follow-up commit. Remarks aren't the most common of diagnostics (they're infrequent unless you're asking for them), but they certainly exist and we should have some plan for them. Doing it as a separate patch means we don't have to think about things like "What does it mean to have an optimization report in SARIF? What should that look like?" quite yet.

clang/lib/Basic/Sarif.cpp

25 ↗

(On Diff #451635)

Yeah, that's one I'd drop as it's transitively included through many different interfaces already. But I don't feel strongly because IWYU is a nice approach too.

396–399 ↗

(On Diff #451635)

Oh yes, that's even better!

[vaibhav.y]

clang/include/clang/Basic/Sarif.h

157 ↗

(On Diff #451635)

Gotcha, the best way forward for now would be to include the None Level. I'll add support for alternative result kinds as a follow-up task.

[vaibhav.y]

Address review comments:

• Drop include line that added llvm/Support/ErrorHandling.h
• Use llvm::Optional::value_or(...) instead of bare conditional
• Add SarifResultLevel::None type for encoding Remark-like diagnostics.
  • Support for these is not yet complete since it requires a custom result kind (this currently defaults to "fail", thus omitted)
  • Add a comment explaining how diagnostic levels are typically mapped to sarif result levels

[aaron.ballman]

LGTM!

[vaibhav.y]

Thank you! I don't have merge access, will need your help for that :)

For the commit details, I'd like to use:

• Name: Vaibhav Yenamandra
• email address: [e-mail]

clang/unittests/Basic/SarifTest.cpp

TEST_F(SarifDocumentWriterTest, checkSerializingResults) {
   // THEN:
   ASSERT_THAT(Output, ::testing::StrEq(ExpectedOutput));
 }
 
 // Check that serializing artifacts from results produces valid SARIF
 TEST_F(SarifDocumentWriterTest, checkSerializingArtifacts) {
   // GIVEN:
   const std::string ExpectedOutput =
-      R"({"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":40,"location":{"index":0,"uri":"file:///main.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":14,"startColumn":14,"startLine":3}}}],"message":{"text":"expected ';' after top level declarator"},"ruleId":"clang.unittest","ruleIndex":0}],"tool":{"driver":{"fullName":"sarif test runner","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"sarif test","rules":[{"fullDescription":{"text":"Example rule created during unit tests"},"id":"clang.unittest","name":"clang unit test"}],"version":"1.0.0"}}}],"version":"2.1.0"})";
+      R"({"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":40,"location":{"index":0,"uri":"file:///main.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":14,"startColumn":14,"startLine":3}}}],"message":{"text":"expected ';' after top level declarator"},"ruleId":"clang.unittest","ruleIndex":0}],"tool":{"driver":{"fullName":"sarif test runner","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"sarif test","rules":[{"fullDescription":{"text":"Example rule created during unit tests"},"id":"clang.unittest","name":"clang unit test"}],"version":"1.0.0"}}}],"version":"2.1.0"})";
 
   SarifDocumentWriter Writer{SourceMgr};
   const SarifRule &Rule =
       SarifRule::create()
           .setRuleId("clang.unittest")
           .setDescription("Example rule created during unit tests")
           .setName("clang unit test");
 
TEST_F(SarifDocumentWriterTest, checkSerializingArtifacts) {
   FileID MainFileID =
       registerSource("/main.cpp", SourceText, /* IsMainFile = */ true);
   CharSourceRange SourceCSR =
       getFakeCharSourceRange(MainFileID, {3, 14}, {3, 14});
 
   DiagLocs.push_back(SourceCSR);
 
   const SarifResult &Result =
-      SarifResult::create(RuleIdx).setLocations(DiagLocs).setDiagnosticMessage(
-          "expected ';' after top level declarator");
+      SarifResult::create(RuleIdx)
+          .setLocations(DiagLocs)
+          .setDiagnosticMessage("expected ';' after top level declarator")
+          .setDiagnosticLevel(SarifResultLevel::Error);
   Writer.appendResult(Result);
   std::string Output = serializeSarifDocument(Writer.createDocument());
 
   // THEN: Assert that the serialized SARIF is as expected
   ASSERT_THAT(Output, ::testing::StrEq(ExpectedOutput));
 }
 
 TEST_F(SarifDocumentWriterTest, checkSerializingCodeflows) {
   // GIVEN:
   const std::string ExpectedOutput =
-      R"({"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":27,"location":{"index":1,"uri":"file:///test-header-1.h"},"mimeType":"text/plain","roles":["resultFile"]},{"length":30,"location":{"index":2,"uri":"file:///test-header-2.h"},"mimeType":"text/plain","roles":["resultFile"]},{"length":28,"location":{"index":3,"uri":"file:///test-header-3.h"},"mimeType":"text/plain","roles":["resultFile"]},{"length":41,"location":{"index":0,"uri":"file:///main.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"codeFlows":[{"threadFlows":[{"locations":[{"importance":"essential","location":{"message":{"text":"Message #1"},"physicalLocation":{"artifactLocation":{"index":1},"region":{"endColumn":8,"endLine":2,"startColumn":1,"startLine":1}}}},{"importance":"important","location":{"message":{"text":"Message #2"},"physicalLocation":{"artifactLocation":{"index":2},"region":{"endColumn":8,"endLine":2,"startColumn":1,"startLine":1}}}},{"importance":"unimportant","location":{"message":{"text":"Message #3"},"physicalLocation":{"artifactLocation":{"index":3},"region":{"endColumn":8,"endLine":2,"startColumn":1,"startLine":1}}}}]}]}],"locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":8,"endLine":2,"startColumn":5,"startLine":2}}}],"message":{"text":"Redefinition of 'foo'"},"ruleId":"clang.unittest","ruleIndex":0}],"tool":{"driver":{"fullName":"sarif test runner","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"sarif test","rules":[{"fullDescription":{"text":"Example rule created during unit tests"},"id":"clang.unittest","name":"clang unit test"}],"version":"1.0.0"}}}],"version":"2.1.0"})";
+      R"({"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":27,"location":{"index":1,"uri":"file:///test-header-1.h"},"mimeType":"text/plain","roles":["resultFile"]},{"length":30,"location":{"index":2,"uri":"file:///test-header-2.h"},"mimeType":"text/plain","roles":["resultFile"]},{"length":28,"location":{"index":3,"uri":"file:///test-header-3.h"},"mimeType":"text/plain","roles":["resultFile"]},{"length":41,"location":{"index":0,"uri":"file:///main.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"codeFlows":[{"threadFlows":[{"locations":[{"importance":"essential","location":{"message":{"text":"Message #1"},"physicalLocation":{"artifactLocation":{"index":1},"region":{"endColumn":8,"endLine":2,"startColumn":1,"startLine":1}}}},{"importance":"important","location":{"message":{"text":"Message #2"},"physicalLocation":{"artifactLocation":{"index":2},"region":{"endColumn":8,"endLine":2,"startColumn":1,"startLine":1}}}},{"importance":"unimportant","location":{"message":{"text":"Message #3"},"physicalLocation":{"artifactLocation":{"index":3},"region":{"endColumn":8,"endLine":2,"startColumn":1,"startLine":1}}}}]}]}],"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":8,"endLine":2,"startColumn":5,"startLine":2}}}],"message":{"text":"Redefinition of 'foo'"},"ruleId":"clang.unittest","ruleIndex":0}],"tool":{"driver":{"fullName":"sarif test runner","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"sarif test","rules":[{"fullDescription":{"text":"Example rule created during unit tests"},"id":"clang.unittest","name":"clang unit test"}],"version":"1.0.0"}}}],"version":"2.1.0"})";
 
   const char *SourceText = "int foo = 0;\n"
                            "int foo = 1;\n"
                            "float x = 0.0;\n";
   FileID MainFileID =
       registerSource("/main.cpp", SourceText, /* IsMainFile = */ true);
   CharSourceRange DiagLoc{getFakeCharSourceRange(MainFileID, {2, 5}, {2, 8})};
 
ThreadFlow Item = ThreadFlow::create()
                           .setImportance(Importances[Idx])
                           .setMessage(Message);
     Threadflows.push_back(Item);
   }
 
   // WHEN: A result containing code flows and diagnostic locations is added
   Writer.createRun("sarif test", "sarif test runner", "1.0.0");
   unsigned RuleIdx = Writer.createRule(Rule);
-  const SarifResult &Result = SarifResult::create(RuleIdx)
-                                  .setLocations({DiagLoc})
-                                  .setDiagnosticMessage("Redefinition of 'foo'")
-                                  .setThreadFlows(Threadflows);
+  const SarifResult &Result =
+      SarifResult::create(RuleIdx)
+          .setThreadFlows(Threadflows)
+          .setDiagnosticLevel(SarifResultLevel::Warning);
   Writer.appendResult(Result);
   std::string Output = serializeSarifDocument(Writer.createDocument());
 
   // THEN: Assert that the serialized SARIF is as expected
   ASSERT_THAT(Output, ::testing::StrEq(ExpectedOutput));
 }
 
 } // namespace