# RavenDB and PVS\-Studio: win\-win collaboration

This is a brief story of how PVS\-Studio collaborated with RavenDB\. PVS\-Studio is a static code analyzer\. RavenDB is an open\-source database\. How can searching for errors in one project benefit both? Let's find out by going over the bug fixes and RavenDB developers' comments\.

![1052_Collaboration_of_RavenDB_and_PVSStudio/image1.png](https://import.viva64.com/docx/blog/1052_Collaboration_of_RavenDB_and_PVSStudio/image1.png)

## A start has been made

Before we get started, let me briefly introduce [RavenDB](https://ravendb.net/) and [PVS\-Studio](https://pvs-studio.com/en/pvs-studio/)\. RavenDB is an open\-source [ACID](https://en.wikipedia.org/wiki/ACID) NoSQL document\-oriented database\. PVS\-Studio is a static code analyser for safety, security \(SAST\) and code quality control of programs written in C, C\+\+, C\# and Java\.

The RavenDB team chose PVS\-Studio for checking the project's source code\. That sparked our collaboration idea: we agreed to analyze the project first, then examine a few of the most interesting bugs we found, and share them with our readers\. So that happened\.

We downloaded the project's [source code](https://github.com/ravendb/ravendb/tree/e1f32a6b234bcd9e9ea27b1c431dcd3156044aed) and analyzed it\. It's worth noting that the program's source code is clear and well\-written – this is the merit of the development team, my hats off to them\. After checking the code, we sent RavenDB the list of 10 warnings issued for the most suspicious code fragments\.

We got a reply very soon\. [Paweł Pekról](https://github.com/ppekrol), the Head of Development at RavenDB, sent us feedback on PVS\-Studio warnings and made a [pull request](https://github.com/ravendb/ravendb/pull/16234) to fix the described bugs\. 

RavenDB developers immediately responded to the suspicious code snippets and fixed the detected errors\. Let's take a look at the most intriguing flaws and see how the developers fixed the code\. 

## Errors and their fixes

**Error 1**

```cpp
[Flags]
public enum TypeAttributes
{
  ....
  NotPublic = 0,
  ....
 }

private static bool CheckIfAnonymousType(Type type)
{
  // hack: the only way to detect anonymous types right now
  return type.IsDefined(typeof(CompilerGeneratedAttribute), false)
      && type.IsGenericType && type.Name.Contains("AnonymousType")
      && (type.Name.StartsWith("<>") || type.Name.StartsWith("VB$"))
      && type.Attributes.HasFlag(TypeAttributes.NotPublic);        // <=
 }
```

[V3180](https://pvs-studio.com/en/docs/warnings/v3180/) The 'HasFlag' method always returns 'true' because the value of 'TypeAttributes\.NotPublic' is '0', and it is passed as the method's argument\. ExpressionStringBuilder\.cs 2117

The _HasFlag_ method always returns _true_ for a flag with a zero value \(see more details in the [documentation](https://learn.microsoft.com/en-us/dotnet/api/system.enum.hasflag?view=net-8.0#remarks)\): So, the part of the expression is always _true_ and makes no sense\.

The code fragment has been fixed by removing the check with the_ HasFlag_ method\.

Here you can find the [commit](https://github.com/ravendb/ravendb/commit/d01ba0c60e51f8733ae5cdf1fb8a608b52056b1e)\.

**Error 2**

```cpp
private JsValue Spatial_Distance(JsValue self, JsValue[] args)
{
  if (args.Length < 4 && args.Length > 5) // <=
    throw new ArgumentException("....");
 }
```

[V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression 'args\.Length < 4 && args\.Length \> 5' is always false\. Probably the '\|\|' operator should be used here\. ScriptRunner\.cs 930

The value of the _args\.Length_ expression cannot be both less than 4 and greater than 5, therefore this condition is always false\. This error is easily fixed by replacing '&&' with '\|\|'\. 

However, RavenDB developers were on the lookout and spotted another error in this code fragment\. As it turned out, the upper bound for the number of arguments should be 6, not 5\. 

The correct code:

```cpp
private static JsValue Spatial_Distance(JsValue self, JsValue[] args)
{
  if (args.Length is < 4 or > 6)
    throw new ArgumentException("....");
 }
```

Here are the commits: for the [first](https://github.com/ravendb/ravendb/commit/bd487885bce7c3e1a1a441533c2cd106d7bce304) error and for [the second](https://github.com/ravendb/ravendb/commit/0e5185dfbf367ee14ad4c647aa85f557b899f90d)\.

**Error 3**

```cpp
public override DynamicJsonValue ToJson()fcg 
{
  var json = base.ToJson();
  json[nameof(DataDirectory)] = DataDirectory;
  json[nameof(JournalStoragePath)] = DataDirectory;      // <=
  json[nameof(SnapshotRestore)] = SnapshotRestore.ToJson();
  return json;
 }
```

[V3127](https://pvs-studio.com/en/docs/warnings/v3127/) Two similar code fragments were found\. Perhaps, this is a typo and 'JournalStoragePath' variable should be used instead of 'DataDirectory' RestoreResult\.cs 24

Since the value of the _DataDirectory_ property is written to _json\[nameof\(DataDirectory\)\]_, then the value of the _JournalStoragePath_ property should be written to _json\[nameof\(JournalStoragePath\)\]_\. That's exactly the way RavenDB developers fixed this bug:

```cpp
json[nameof(JournalStoragePath)] = JournalStoragePath;
```

The [commit](https://github.com/ravendb/ravendb/commit/c6c14145b97b74db353e7f00480d75a168839634)\.

**Error 4**

```cpp
public override int GetHashCode()
{
  unchecked
  {
    var hashCode = MinimumRevisionsToKeep?.GetHashCode() ?? 0;
    hashCode = (hashCode * 397) ^ 
      MinimumRevisionAgeToKeep?.GetHashCode() ?? 0; // <=
    hashCode = (hashCode * 397) ^ 
      Disabled.GetHashCode();
    hashCode = (hashCode * 397) ^
      PurgeOnDelete.GetHashCode();
    return hashCode;
   }
 }
```

[V3123](https://pvs-studio.com/en/docs/warnings/v3123/) Perhaps the '??' operator works in a different way than it was expected\. Its priority is lower than priority of other operators in its left part\. RevisionsCollectionConfiguration\.cs 40

Here PVS\-Studio issues a warning for the '??' operator's priority\. And that's true, since the priority of the '??' operator is lower than the priority of the '^' operator\.  If _MinimumRevisionAgeToKeep_ is _null_, the result of _hashCode_ in the specified line will always be 0\. Parenthesizing the '??' expression fixes the error\. 

By the way, RavenDB developers found another bug here: The _MaximumRevisionsToDeleteUponDocumentUpdate_ property was missed when calculating _hashCode_\.

The correct code:

```cpp
public override int GetHashCode()
{
  unchecked
  {
    var hashCode = MinimumRevisionsToKeep?.GetHashCode() ?? 0;
    hashCode = (hashCode * 397) ^ 
      (MinimumRevisionAgeToKeep?.GetHashCode() ?? 0);
    hashCode = (hashCode * 397) ^ 
      (MaximumRevisionsToDeleteUponDocumentUpdate?.GetHashCode() ?? 0);
    hashCode = (hashCode * 397) ^ 
      Disabled.GetHashCode();
    hashCode = (hashCode * 397) ^ 
      PurgeOnDelete.GetHashCode();
    return hashCode;
   }
 }
```

The [commit](https://github.com/ravendb/ravendb/commit/78e57c9d293bbd1812031ea7e761dc5b981b5684)\.

Head of Development Paweł Pekról commented on this error:

> Point 4 \- gold :\)

**Error 5**

```cpp
public override void VisitMethod(MethodExpression expr)
{
  if (expr.Name.Value == "id" && expr.Arguments.Count == 0)
  {
    if (expr.Arguments.Count != 1)   // <=
    {
      throw new InvalidOperationException("....");
     }
    _sb.Append("this");
   }
 }
```

[V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression 'expr\.Arguments\.Count \!\= 1' is always true\. JavascriptCodeQueryVisitor\.cs 188

The _expr\.Arguments\.Count \!\= 1_ check is performed immediately after _expr\.Arguments\.Count \=\= 0 is checked_\. Obviously, if _expr\.Arguments\.Count_ is 0, it is not 1\. If the execution flow enters the upper _if_ statement, an exception will be thrown\. As a result, _\_sb\.Append\("this"\)_ will never be executed\.

It seems that the code was just "lucky" during the testing: it wasn't executed, otherwise an exception would have been thrown\. However, such bugs may go unnoticed during testing, but users will eventually run into them\.

This has been fixed by removing the _if_ statement that throws an exception\.

Here you can find the [commit](https://github.com/ravendb/ravendb/commit/f91d886ab9eb931d97a71ec66ac6453ff3099f03)\.

## Conclusion

The title states that it was a win\-win collaboration\. What does RavenDB benefit from? Every product needs to keep enhancing\. This time the project enhanced not by adding new features, but by improving the quality of the source code\. What does it mean for PVS\-Studio? When studying the analysis report, we found several false positives\. Our developers have already fixed some of them, other false positives are waiting their turn\.

Head of Development RavenDB Paweł Pekról:

> Going over the list made me realize that devs are only humans and no matter how many tests you have there will always be bugs\. The tool is very helpful in finding obvious and non\-obvious bugs\.

I cannot but totally agree\. There's no way for full code coverage\. If we do care about the quality of the source code, it would be wise to use multiple tools that make the development process much easier\.

Get rid of bugs and make your code safer with [PVS\-Studio](https://pvs-studio.com/en/pvs-studio/)\! Store and process your data easily with [RavenDB](https://ravendb.net/)\!

Enjoy using RavenDB and PVS\-Studio\!