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

264

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

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

Why was this include required?

397–400

(Probably have to re-run clang-format)

[vaibhav.y]

clang/include/clang/Basic/Sarif.h

157

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

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.

397–400

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

[aaron.ballman]

clang/include/clang/Basic/Sarif.h

157

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

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.

397–400

Oh yes, that's even better!

[vaibhav.y]

clang/include/clang/Basic/Sarif.h

157

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/include/clang/Basic/Sarif.h

SarifArtifact setMimeType(llvm::StringRef ArtifactMimeType) {
     return *this;
   }
 };
 
 } // namespace detail
 
 enum class ThreadFlowImportance { Important, Essential, Unimportant };
 
+/// The level of severity associated with a \ref SarifResult.
+///
+/// Of all the levels, \c None is the only one that is not associated with
+/// a failure.
+///
+/// A typical mapping for clang's DiagnosticKind to SarifResultLevel would look
+/// like:
+/// * \c None: \ref clang::DiagnosticsEngine::Level::Remark, \ref clang::DiagnosticsEngine::Level::Ignored
+/// * \c Note: \ref clang::DiagnosticsEngine::Level::Note
+/// * \c Warning: \ref clang::DiagnosticsEngine::Level::Warning
+/// * \c Error could be generated from one of:
+///   - \ref clang::DiagnosticsEngine::Level::Warning with \c -Werror
+///   - \ref clang::DiagnosticsEngine::Level::Error
+///   - \ref clang::DiagnosticsEngine::Level::Fatal when \ref clang::DiagnosticsEngine::ErrorsAsFatal is set.
+///
+/// Reference:
+/// 1. <a href="https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317648">level property</a>
+enum class SarifResultLevel { None, Note, Warning, Error };
+
 /// A thread flow is a sequence of code locations that specify a possible path
 /// through a single thread of execution.
 /// A thread flow in SARIF is related to a code flow which describes
 /// the progress of one or more programs through one or more thread flows.
 ///
 /// Reference:
 /// 1. <a href="https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317744">threadFlow object</a>
 /// 2. <a href="https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317740">codeFlow object</a>
public:
   }
 
   ThreadFlow setMessage(llvm::StringRef ItemMessage) {
     Message = ItemMessage.str();
     return *this;
   }
 };
 
+/// A SARIF Reporting Configuration (\c reportingConfiguration) object contains
+/// properties for a \ref SarifRule that can be configured at runtime before
+/// analysis begins.
+///
+/// Reference:
+/// 1. <a href="https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317852">reportingConfiguration object</a>
+class SarifReportingConfiguration {
+  friend class clang::SarifDocumentWriter;
+
+  bool Enabled = true;
+  SarifResultLevel Level = SarifResultLevel::Warning;
+  float Rank = -1.0f;
+
+  SarifReportingConfiguration() = default;
+
+public:
+  static SarifReportingConfiguration create() { return {}; };
+
+  SarifReportingConfiguration disable() {
+    Enabled = false;
+    return *this;
+  }
+
+  SarifReportingConfiguration enable() {
+    Enabled = true;
+    return *this;
+  }
+
+  SarifReportingConfiguration setLevel(SarifResultLevel TheLevel) {
+    Level = TheLevel;
+    return *this;
+  }
+
+  SarifReportingConfiguration setRank(float TheRank) {
+    assert(TheRank >= 0.0f && "Rule rank cannot be smaller than 0.0");
+    assert(TheRank <= 100.0f && "Rule rank cannot be larger than 100.0");
+    Rank = TheRank;
+    return *this;
+  }
+};
+
 /// A SARIF rule (\c reportingDescriptor object) contains information that
 /// describes a reporting item generated by a tool. A reporting item is
 /// either a result of analysis or notification of a condition encountered by
 /// the tool. Rules are arbitrary but are identifiable by a hierarchical
 /// rule-id.
 ///
 /// This builder provides an interface to create SARIF \c reportingDescriptor
 /// objects via the \ref SarifRule::create static method.
 ///
 /// Reference:
 /// 1. <a href="https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317836">reportingDescriptor object</a>
 class SarifRule {
   friend class clang::SarifDocumentWriter;
 
   std::string Name;
   std::string Id;
   std::string Description;
   std::string HelpURI;
+  SarifReportingConfiguration DefaultConfiguration;
 
-  SarifRule() = default;
+  SarifRule() : DefaultConfiguration(SarifReportingConfiguration::create()) {}
 
 public:
   static SarifRule create() { return {}; }
 
   SarifRule setName(llvm::StringRef RuleName) {
     Name = RuleName.str();
     return *this;
   }
 
   SarifRule setRuleId(llvm::StringRef RuleId) {
     Id = RuleId.str();
     return *this;
   }
 
   SarifRule setDescription(llvm::StringRef RuleDesc) {
     Description = RuleDesc.str();
     return *this;
   }
 
   SarifRule setHelpURI(llvm::StringRef RuleHelpURI) {
     HelpURI = RuleHelpURI.str();
     return *this;
   }
+
+  SarifRule
+  setDefaultConfiguration(const SarifReportingConfiguration &Configuration) {
+    DefaultConfiguration = Configuration;
+    return *this;
+  }
 };
 
 /// A SARIF result (also called a "reporting item") is a unit of output
 /// produced when one of the tool's \c reportingDescriptor encounters a match
 /// on the file being analysed by the tool.
 ///
 /// This builder provides a \ref SarifResult::create static method that can be
 /// used to create an empty shell onto which attributes can be added using the
class SarifResult {
   // This type cannot fit all possible indexes representable by JSON, but is
   // chosen because it is the largest unsigned type that can be safely
   // converted to an \c int64_t.
   uint32_t RuleIdx;
   std::string RuleId;
   std::string DiagnosticMessage;
   llvm::SmallVector<CharSourceRange, 8> Locations;
   llvm::SmallVector<ThreadFlow, 8> ThreadFlows;
+  llvm::Optional<SarifResultLevel> LevelOverride;
 
   SarifResult() = delete;
   explicit SarifResult(uint32_t RuleIdx) : RuleIdx(RuleIdx) {}
 
 public:
   static SarifResult create(uint32_t RuleIdx) { return SarifResult{RuleIdx}; }
 
   SarifResult setIndex(uint32_t Idx) {

 #endif
     Locations.assign(DiagLocs.begin(), DiagLocs.end());
     return *this;
   }
   SarifResult setThreadFlows(llvm::ArrayRef<ThreadFlow> ThreadFlowResults) {
     ThreadFlows.assign(ThreadFlowResults.begin(), ThreadFlowResults.end());
     return *this;
   }
+
+  SarifResult setDiagnosticLevel(const SarifResultLevel &TheLevel) {
+    LevelOverride = TheLevel;
+    return *this;
+  }
 };
 
 /// This class handles creating a valid SARIF document given various input
 /// attributes. However, it requires an ordering among certain method calls:
 ///
 /// 1. Because every SARIF document must contain at least 1 \c run, callers
 ///    must ensure that \ref SarifDocumentWriter::createRun is is called before
 ///    any other methods.

clang/lib/Basic/Sarif.cpp

 #include "clang/Basic/Sarif.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/Path.h"
 
 #include <string>
 #include <utility>
 
 using namespace clang;
 using namespace llvm;
 
static StringRef importanceToStr(ThreadFlowImportance I) {
   case ThreadFlowImportance::Essential:
     return "essential";
   case ThreadFlowImportance::Unimportant:
     return "unimportant";
   }
   llvm_unreachable("Fully covered switch is not so fully covered");
 }
 
+static StringRef resultLevelToStr(SarifResultLevel R) {
+  switch (R) {
+  case SarifResultLevel::None:
+    return "none";
+  case SarifResultLevel::Note:
+    return "note";
+  case SarifResultLevel::Warning:
+    return "warning";
+  case SarifResultLevel::Error:
+    return "error";
+  }
+  llvm_unreachable("Potentially un-handled SarifResultLevel. "
+                   "Is the switch not fully covered?");
+}
+
 static json::Object
 createThreadFlowLocation(json::Object &&Location,
                          const ThreadFlowImportance &Importance) {
   return json::Object{{"location", std::move(Location)},
                       {"importance", importanceToStr(Importance)}};
 }
 ///  @}
 
void SarifDocumentWriter::endRun() {
   // Since Closed = false here, expect there to be at least 1 Run, anything
   // else is an invalid state.
   assert(!Runs.empty() && "There are no runs associated with the document!");
 
   // Flush all the rules.
   json::Object &Tool = getCurrentTool();
   json::Array Rules;
   for (const SarifRule &R : CurrentRules) {
+    json::Object Config{
+        {"enabled", R.DefaultConfiguration.Enabled},
+        {"level", resultLevelToStr(R.DefaultConfiguration.Level)},
+        {"rank", R.DefaultConfiguration.Rank}};
     json::Object Rule{
         {"name", R.Name},
         {"id", R.Id},
-        {"fullDescription", json::Object{{"text", R.Description}}}};
+        {"fullDescription", json::Object{{"text", R.Description}}},
+        {"defaultConfiguration", std::move(Config)}};
     if (!R.HelpURI.empty())
       Rule["helpUri"] = R.HelpURI;
     Rules.emplace_back(std::move(Rule));
   }
   json::Object &Driver = *Tool.getObject("driver");
   Driver["rules"] = std::move(Rules);
 
   // Flush all the artifacts.
size_t SarifDocumentWriter::createRule(const SarifRule &Rule) {
   CurrentRules.emplace_back(Rule);
   return Ret;
 }
 
 void SarifDocumentWriter::appendResult(const SarifResult &Result) {
   size_t RuleIdx = Result.RuleIdx;
   assert(RuleIdx < CurrentRules.size() &&
          "Trying to reference a rule that doesn't exist");
+  const SarifRule &Rule = CurrentRules[RuleIdx];
+  assert(Rule.DefaultConfiguration.Enabled &&
+         "Cannot add a result referencing a disabled Rule");
   json::Object Ret{{"message", createMessage(Result.DiagnosticMessage)},
                    {"ruleIndex", static_cast<int64_t>(RuleIdx)},
-                   {"ruleId", CurrentRules[RuleIdx].Id}};
+                   {"ruleId", Rule.Id}};
   if (!Result.Locations.empty()) {
     json::Array Locs;
     for (auto &Range : Result.Locations) {
       Locs.emplace_back(createLocation(createPhysicalLocation(Range)));
     }
     Ret["locations"] = std::move(Locs);
   }
   if (!Result.ThreadFlows.empty())
     Ret["codeFlows"] = json::Array{createCodeFlow(Result.ThreadFlows)};
+
+  Ret["level"] = resultLevelToStr(
+      Result.LevelOverride.value_or(Rule.DefaultConfiguration.Level));
+
   json::Object &Run = getCurrentRun();
   json::Array *Results = Run.getArray("results");
   Results->emplace_back(std::move(Ret));
 }
 
 json::Object SarifDocumentWriter::createDocument() {
   // Flush all temporaries to their destinations if needed.
   endRun();
 
   json::Object Doc{
       {"$schema", SchemaURI},
       {"version", SchemaVersion},
   };
   if (!Runs.empty())
     Doc["runs"] = json::Array(Runs);
   return Doc;
 }

clang/unittests/Basic/SarifTest.cpp

 //===- unittests/Basic/SarifTest.cpp - Test writing SARIF documents -------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Basic/Sarif.h"
-#include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/FileSystemOptions.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FormatVariadic.h"

 using namespace clang;
 
 namespace {
 
 using LineCol = std::pair<unsigned int, unsigned int>;
 
 static std::string serializeSarifDocument(llvm::json::Object &&Doc) {
   std::string Output;
-  llvm::json::Value value(std::move(Doc));
+  llvm::json::Value Value(std::move(Doc));
   llvm::raw_string_ostream OS{Output};
-  OS << llvm::formatv("{0}", value);
+  OS << llvm::formatv("{0}", Value);
   OS.flush();
   return Output;
 }
 
 class SarifDocumentWriterTest : public ::testing::Test {
 protected:
   SarifDocumentWriterTest()
       : InMemoryFileSystem(new llvm::vfs::InMemoryFileSystem),

 TEST_F(SarifDocumentWriterTest, canCreateEmptyDocument) {
   // GIVEN:
   SarifDocumentWriter Writer{SourceMgr};
 
   // WHEN:
   const llvm::json::Object &EmptyDoc = Writer.createDocument();
   std::vector<StringRef> Keys(EmptyDoc.size());
   std::transform(EmptyDoc.begin(), EmptyDoc.end(), Keys.begin(),
-                 [](auto item) { return item.getFirst(); });
+                 [](auto Item) { return Item.getFirst(); });
 
   // THEN:
   ASSERT_THAT(Keys, testing::UnorderedElementsAre("$schema", "version"));
 }
 
 // Test that a newly inserted run will associate correct tool names
 TEST_F(SarifDocumentWriterTest, canCreateDocumentWithOneRun) {
   // GIVEN:
TEST_F(SarifDocumentWriterTest, canCreateDocumentWithOneRun) {
   // THEN:
   // A run was created
   ASSERT_THAT(Runs, testing::NotNull());
 
   // It is the only run
   ASSERT_EQ(Runs->size(), 1UL);
 
   // The tool associated with the run was the tool
-  const llvm::json::Object *driver =
+  const llvm::json::Object *Driver =
       Runs->begin()->getAsObject()->getObject("tool")->getObject("driver");
-  ASSERT_THAT(driver, testing::NotNull());
+  ASSERT_THAT(Driver, testing::NotNull());
+
+  ASSERT_TRUE(Driver->getString("name").has_value());
+  ASSERT_TRUE(Driver->getString("fullName").has_value());
+  ASSERT_TRUE(Driver->getString("language").has_value());
 
-  ASSERT_TRUE(driver->getString("name").has_value());
-  ASSERT_TRUE(driver->getString("fullName").has_value());
-  ASSERT_TRUE(driver->getString("language").has_value());
-
-  EXPECT_EQ(driver->getString("name").value(), ShortName);
-  EXPECT_EQ(driver->getString("fullName").value(), LongName);
-  EXPECT_EQ(driver->getString("language").value(), "en-US");
+  EXPECT_EQ(Driver->getString("name").value(), ShortName);
+  EXPECT_EQ(Driver->getString("fullName").value(), LongName);
+  EXPECT_EQ(Driver->getString("language").value(), "en-US");
 }
 
 TEST_F(SarifDocumentWriterTest, addingResultsWillCrashIfThereIsNoRun) {
 #if defined(NDEBUG) || !GTEST_HAS_DEATH_TEST
   GTEST_SKIP() << "This death test is only available for debug builds.";
 #endif
   // GIVEN:
   SarifDocumentWriter Writer{SourceMgr};
 
   // WHEN:
   //  A SarifDocumentWriter::createRun(...) was not called prior to
   //  SarifDocumentWriter::appendResult(...)
   // But a rule exists
   auto RuleIdx = Writer.createRule(SarifRule::create());
   const SarifResult &EmptyResult = SarifResult::create(RuleIdx);
 
   // THEN:
   auto Matcher = ::testing::AnyOf(
       ::testing::HasSubstr("create a run first"),
       ::testing::HasSubstr("no runs associated with the document"));
   ASSERT_DEATH(Writer.appendResult(EmptyResult), Matcher);
 }
 
+TEST_F(SarifDocumentWriterTest, settingInvalidRankWillCrash) {
+#if defined(NDEBUG) || !GTEST_HAS_DEATH_TEST
+  GTEST_SKIP() << "This death test is only available for debug builds.";
+#endif
+  // GIVEN:
+  SarifDocumentWriter Writer{SourceMgr};
+
+  // WHEN:
+  // A SarifReportingConfiguration is created with an invalid "rank"
+  // * Ranks below 0.0 are invalid
+  // * Ranks above 100.0 are invalid
+
+  // THEN: The builder will crash in either case
+  EXPECT_DEATH(SarifReportingConfiguration::create().setRank(-1.0),
+               ::testing::HasSubstr("Rule rank cannot be smaller than 0.0"));
+  EXPECT_DEATH(SarifReportingConfiguration::create().setRank(101.0),
+               ::testing::HasSubstr("Rule rank cannot be larger than 100.0"));
+}
+
+TEST_F(SarifDocumentWriterTest, creatingResultWithDisabledRuleWillCrash) {
+#if defined(NDEBUG) || !GTEST_HAS_DEATH_TEST
+  GTEST_SKIP() << "This death test is only available for debug builds.";
+#endif
+
+  // GIVEN:
+  SarifDocumentWriter Writer{SourceMgr};
+
+  // WHEN:
+  // A disabled Rule is created, and a result is create referencing this rule
+  const auto &Config = SarifReportingConfiguration::create().disable();
+  auto RuleIdx =
+      Writer.createRule(SarifRule::create().setDefaultConfiguration(Config));
+  const SarifResult &Result = SarifResult::create(RuleIdx);
+
+  // THEN:
+  // SarifResult::create(...) will produce a crash
+  ASSERT_DEATH(
+      Writer.appendResult(Result),
+      ::testing::HasSubstr("Cannot add a result referencing a disabled Rule"));
+}
+
 // Test adding rule and result shows up in the final document
 TEST_F(SarifDocumentWriterTest, addingResultWithValidRuleAndRunIsOk) {
   // GIVEN:
   SarifDocumentWriter Writer{SourceMgr};
   const SarifRule &Rule =
       SarifRule::create()
           .setRuleId("clang.unittest")
           .setDescription("Example rule created during unit tests")
           .setName("clang unit test");
 
   // WHEN:
   Writer.createRun("sarif test", "sarif test runner");
   unsigned RuleIdx = Writer.createRule(Rule);
-  const SarifResult &result = SarifResult::create(RuleIdx);
+  const SarifResult &Result = SarifResult::create(RuleIdx);
 
-  Writer.appendResult(result);
+  Writer.appendResult(Result);
   const llvm::json::Object &Doc = Writer.createDocument();
 
   // THEN:
   // A document with a valid schema and version exists
   ASSERT_THAT(Doc.get("$schema"), ::testing::NotNull());
   ASSERT_THAT(Doc.get("version"), ::testing::NotNull());
   const llvm::json::Array *Runs = Doc.getArray("runs");
 
TEST_F(SarifDocumentWriterTest, addingResultWithValidRuleAndRunIsOk) {
 
   // The results are as expected
   EXPECT_EQ(Results->size(), 1UL);
 
   // The artifacts are as expected
   EXPECT_TRUE(Artifacts->empty());
 }
 
-TEST_F(SarifDocumentWriterTest, checkSerializingResults) {
+TEST_F(SarifDocumentWriterTest, checkSerializingResultsWithDefaultRuleConfig) {
   // 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":[],"columnKind":"unicodeCodePoints","results":[{"message":{"text":""},"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":[],"columnKind":"unicodeCodePoints","results":[{"level":"warning","message":{"text":""},"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":[{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"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");
 
   // WHEN: A run contains a result
   Writer.createRun("sarif test", "sarif test runner", "1.0.0");
-  unsigned ruleIdx = Writer.createRule(Rule);
-  const SarifResult &Result = SarifResult::create(ruleIdx);
+  unsigned RuleIdx = Writer.createRule(Rule);
+  const SarifResult &Result = SarifResult::create(RuleIdx);
+  Writer.appendResult(Result);
+  std::string Output = serializeSarifDocument(Writer.createDocument());
+
+  // THEN:
+  ASSERT_THAT(Output, ::testing::StrEq(ExpectedOutput));
+}
+
+TEST_F(SarifDocumentWriterTest, checkSerializingResultsWithCustomRuleConfig) {
+  // 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":[],"columnKind":"unicodeCodePoints","results":[{"level":"error","message":{"text":""},"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":[{"defaultConfiguration":{"enabled":true,"level":"error","rank":35.5},"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")
+          .setDefaultConfiguration(SarifReportingConfiguration::create()
+                                       .setLevel(SarifResultLevel::Error)
+                                       .setRank(35.5));
+
+  // WHEN: A run contains a result
+  Writer.createRun("sarif test", "sarif test runner", "1.0.0");
+  unsigned RuleIdx = Writer.createRule(Rule);
+  const SarifResult &Result = SarifResult::create(RuleIdx);
   Writer.appendResult(Result);
   std::string Output = serializeSarifDocument(Writer.createDocument());
 
   // 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":[{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"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":[{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"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})};
 
   SarifDocumentWriter Writer{SourceMgr};
   const SarifRule &Rule =
       SarifRule::create()
           .setRuleId("clang.unittest")
           .setDescription("Example rule created during unit tests")
           .setName("clang unit test");
 
-  constexpr unsigned int NUM_CASES = 3;
-  llvm::SmallVector<ThreadFlow, NUM_CASES> Threadflows;
-  const char *HeaderTexts[NUM_CASES]{("#pragma once\n"
-                                      "#include <foo>"),
-                                     ("#ifndef FOO\n"
-                                      "#define FOO\n"
-                                      "#endif"),
-                                     ("#ifdef FOO\n"
-                                      "#undef FOO\n"
-                                      "#endif")};
-  const char *HeaderNames[NUM_CASES]{"/test-header-1.h", "/test-header-2.h",
-                                     "/test-header-3.h"};
-  ThreadFlowImportance Importances[NUM_CASES]{
-      ThreadFlowImportance::Essential, ThreadFlowImportance::Important,
+  constexpr unsigned int NumCases = 3;
+  llvm::SmallVector<ThreadFlow, NumCases> Threadflows;
+  const char *HeaderTexts[NumCases]{("#pragma once\n"
+  const char *HeaderNames[NumCases]{"/test-header-1.h", "/test-header-2.h",
+  ThreadFlowImportance Importances[NumCases]{ThreadFlowImportance::Essential,
+                                             ThreadFlowImportance::Important,
                                              ThreadFlowImportance::Unimportant};
-  for (size_t Idx = 0; Idx != NUM_CASES; ++Idx) {
+  for (size_t Idx = 0; Idx != NumCases; ++Idx) {
     FileID FID = registerSource(HeaderNames[Idx], HeaderTexts[Idx]);
     CharSourceRange &&CSR = getFakeCharSourceRange(FID, {1, 1}, {2, 8});
     std::string Message = llvm::formatv("Message #{0}", Idx + 1);
     ThreadFlow Item = ThreadFlow::create()
                           .setRange(CSR)
                           .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