Developers Club geek daily blog

1 year, 1 month ago
We look for errors in MonoDevelop

In life of the PVS-Studio analyzer the important event took place — in the latest version verifiability of the code written on C# was added. Being one of developers of this analyzer, I just could not pass by, without having checked some project. It is clear, that it will be interesting to very few people to read about verification of small and unknown projects therefore it was necessary to select something known, and the choice fell on MonoDevelop.

It is a little about the project


MonoDevelop — the free development environment intended for an application creation C#, Java, Boo, Nemerle, Visual Basic .NET, Vala, CIL, Xi C ++. Also support of Oxygeneco of the party of Embarcadero Technologies is planned.

We look for errors in MonoDevelop

Initially it was the SharpDevelop port on Mono/GTK+, but since then the project far left from the start state.

MonoDevelop is part of the Mono project. It is built in the Unity3Dkak distribution kit means of writing of scripts, but the outdated version (4.0.1).

Among opportunities of this development environment select syntax highlighting, turning of a code, autocompletion of a code, a class browser, support of plug-ins, the built-in debugger, a visual form designer, modular testing.

The source code of the project is available in the corresponding repository on GitHub, and assembly instructions are described on the official site of the project.

Than checked?


As it was already mentioned above, the analysis of the project was performed by means of the latest version of a static code analyzer of PVS-Studio in which the feature for the analysis C#-кода was added. This the first release C#-анализатора, and at the moment in it more than 40 diagnostic rules are implemented. It is clear, that the version is still developed far not so strongly as the C ++-analyzer, but using this tool can already be found rather interesting errors (some of which will are provided in this article). C#-анализатор is not a separate product, it is a part of the same PVS-Studio which just is able to analyze now the code written in one more programming language.

It is possible to download the latest version of the analyzer at this link.

Several words about result of the analysis


As a result of the analysis 8457 files as a part of 95 projects were checked.

The analyzer issued 118 warnings of the first, 128 warnings of the second and 475 warnings of the third of levels.

Someone can tell that it not so is a lot of for such number of files. But here it is worth taking into account the fact that the smaller number of diagnostics, than in With ++ the analyzer is at the moment implemented. Secondly, the analyzer is ineffective at one-time checks. Though it repeatedly repeated, but it is worth mentioning once again — for receipt of full-fledged advantage of use of instruments of static analysis, they have to be used regularly, but not one-time. It will save time for search and elimination of errors, and as a result — will make development of the project cheaper and easier.

Results of the analysis


In article some will be considered the most interesting of the found errors as the overview of all found errors would increase the amount of this article to the indecent sizes. Article is broken into the subsections comprising the description of these or those types of errors with code samples from the project. So you can pass to viewing of the most interesting to you of errors at once.

Identical expressions at the left and to the right of the operator


Descriptions of errors of a type 'by A || A' are provided in this subsection. Often such errors turn out as a result of typographical errors or unsuccessful 'copy-paste' and the programmer's carelessness. Often such errors are difficult to be found in large volumes of a code, especially if names of variables rather long and differ only with one character. As a rule, use of other variable is meant, but occasionally similar checks are just redundant code. In more detail about all this is slightly lower.
protected override SourceCodeLocation 
  GetSourceCodeLocation (string fixtureTypeNamespace, 
                         string fixtureTypeName, 
                         string methodName)
{
  if (string.IsNullOrEmpty (fixtureTypeName) || 
      string.IsNullOrEmpty (fixtureTypeName))
    return null;
  ....
}

Warning of the analyzer: V3001 There are identical sub-expressions 'string.IsNullOrEmpty (fixtureTypeName)' to the left and to the right of the '||' operator. MonoDevelop.NUnit NUnitProjectTestSuite.cs 84

Error it is visible to the naked eye — in a condition the same string variable is twice checked for equality of 'null' or for equivalence of 'String.Empty'. Below on a code (not all body is given here not to complicate perception so take the word) similar check is performed for the fixtureTypeNamespace variable so it is possible to assume that the second check of the given condition as argument of a method had to accept the methodName variable or at all be absent.

Other example of a similar error:
bool TryAddDocument (string fileName, 
     out OpenRazorDocument currentDocument)
{
  ....
  var guiDoc = IdeApp.Workbench.GetDocument (fileName);
  if (guiDoc != null && guiDoc.Editor != null)
  ....
  guiDoc.Closed += (sender, args) =>
  {
    var doc = sender as MonoDevelop.Ide.Gui.Document;
    if (doc.Editor != null && doc.Editor != null) 
    ....
  }
  ....
}

Warning of the analyzer: V3001 There are identical sub-expressions 'doc.Editor! = null' to the left and to the right of the '&&' operator. MonoDevelop.AspNet RazorCSharpParser.cs 180

Again 2 identical checks within one expression. Theoretically after reduction of the sender variable with use of the operator 'as' in the doc variable null value can be written. As a result, at check execution 'doc.Editor! = null' will be generated an exception like 'NullReferenceException'. The corrected option of a code could look so:
if (doc != null && doc.Editor != null)

One more fragment of a code with an error:
static MemberCore GetLaterDefinedMember (MemberSpec a, MemberSpec b)
{
  var mc_a = a.MemberDefinition as MemberCore;
  var mc_b = b.MemberDefinition as MemberCore;
  if (mc_a == null)
    return mc_b;

  if (mc_b == null)
    return mc_a;

  if (a.DeclaringType.MemberDefinition !=  
      b.DeclaringType.MemberDefinition)
    return mc_b;

  if (mc_a.Location.File != mc_a.Location.File)
    return mc_b;

  return mc_b.Location.Row > mc_a.Location.Row ? mc_b : mc_a;
} 

Warning of the analyzer: V3001 There are identical sub-expressions 'mc_a.Location.File' to the left and to the right of the'!=' operator. ICSharpCode.NRefactory.CSharp membercache.cs 1319

The similar error can not be evident, but the analyzer — not the person, and does not pass such things. From a code it is visible that File property of object of 'mc_a' is compared to itself, but it is obvious that it had to be compared to the corresponding object property of 'mc_b'.

Correct code:
if (mc_a.Location.File != mc_b.Location.File)

Redundant code:
public override AppResult Property (string propertyName, object value)
{
  if (resultIter != null && resultIter.HasValue) {
    var objectToCompare = TModel.GetValue (resultIter.Value, Column);
      return MatchProperty (propertyName, objectToCompare, value);
  }

  return MatchProperty (propertyName, ParentWidget, value);
}
TreeIter? resultIter;

Warning of the analyzer: V3001 There are identical sub-expressions 'resultIter! = null' to the left and to the right of the '&&' operator. MonoDevelop.Ide GtkTreeModelResult.cs 125

The resultIter variable has nullable-type, therefore, of check of a type 'resultIter! = null' and 'resultIter.HasValue' are identical and it was possible to be limited to one of them.

Just the same code met 1 more time. Corresponding warning of the analyzer:

V3001 There are identical sub-expressions 'resultIter! = null' to the left and to the right of the '&&' operator. MonoDevelop.Ide GtkTreeModelResult.cs 135

Let's consider the following fragment of a code:
Accessibility DeclaredAccessibility { get; }
bool IsStatic { get; }
private bool MembersMatch(ISymbol member1, ISymbol member2)
{
  if (member1.Kind != member2.Kind)
  {
    return false;
  }

  if (member1.DeclaredAccessibility != member1.DeclaredAccessibility 
   || member1.IsStatic != member1.IsStatic)
  {
    return false;
  }

  if (member1.ExplicitInterfaceImplementations().Any() ||  
      member2.ExplicitInterfaceImplementations().Any())
  {
    return false;
  }

  return SignatureComparer
          .HaveSameSignatureAndConstraintsAndReturnTypeAndAccessors(
             member1, member2, this.IsCaseSensitive);
}

Warnings of the analyzer:
Next typographical error. And not one, and at once two. Again comparison of properties of the same object ('member1') is among themselves executed. As there is no property primitive and any additional logic in them, and similar checks lose meaning too. And from a code it is visible that object properties of 'member1' and 'member2' had to be compared. Correct option of a code:
if (member1.DeclaredAccessibility != member2.DeclaredAccessibility   
 || member1.IsStatic != member2.IsStatic)


Assignment of a variable to


Not such often found type of errors, as previous, but not less interesting. Often situations when some member of a class in a method needs to appropriate value of one of the transferred arguments are wrong, and these names often differ only in the register of the first character. At the same time it is easy to make a mistake. Also simple cases of assignment of a variable to themselves meet and if it is properties, the compiler will not issue any warnings. Such actions are clear if on getter / a setter of property the difficult logic, but if it is absent is hung up — assignment looks at least strange. But we will not be unfounded, we will better look at examples of such errors.
public ViMacro (char macroCharacter) {
  MacroCharacter = MacroCharacter;
}
public char MacroCharacter {get; set;}

Warning of the analyzer: V3005 The 'MacroCharacter' variable is assigned to itself. Mono.TextEditor ViMacro.cs 57

What it was told above about — because names of property and argument of the designer differ only with the register of the first character, value of property registers in itself instead of in it the value transferred in quality of argument registered. Having looked at determination of property it is possible to be convinced that it does not contain any additional logic.
public ViMark (char markCharacter) {
  MarkCharacter = MarkCharacter;
} 
public char MarkCharacter {get; set;}

Warning of the analyzer: V3005 The 'MarkCharacter' variable is assigned to itself. Mono.TextEditor ViMark.cs 45

The error is exactly similar to previous. Again the first character in the name variable because of what the designer works not as it was expected is mixed.
public WhitespaceNode(string whiteSpaceText, 
                      TextLocation startLocation)
{
  this.WhiteSpaceText = WhiteSpaceText;
  this.startLocation = startLocation;
}
public string WhiteSpaceText { get; set; }

Warning of the analyzer: V3005 The 'this.WhiteSpaceText' variable is assigned to itself. ICSharpCode.NRefactory.CSharp WhitespaceNode.cs 65

The error is again similar previous, but this time the code is more interesting that in one of two prisvaivaniye the programmer was not sealed up. During fast text typing it is easy to pass a similar error if means of automatic substitution of a code are used. However, it could be avoided, regularly checking a new code by means of the static analyzer. For example, in PVS-Studio there is a possibility of automatic check of a new code after compilation (see the mode of the incremental analysis).
void OptionsChanged (object sender, EventArgs e)
{
  gutterMargin.IsVisible = Options.ShowLineNumberMargin;
  iconMargin.IsVisible = iconMargin.IsVisible;
  ....
}
public bool IsVisible { get; set; }

Warning of the analyzer: V3005 The 'iconMargin.IsVisible' variable is assigned to itself. MonoDevelop.HexEditor HexEditor.cs 241

It is the second type of an error described by me at the beginning of subsection. Value of property is appropriated itself, however there are no local variables, the name similar to this property. At the same time on property, besides, no additional logic is tied. Perhaps, the correct code would have to look so though here it is already impossible to tell for certain:
iconMargin.IsVisible = gutterMargin.IsVisible;


Choice illusion


Interesting subtitle, isn't that so? However it, perhaps, most precisely describes some types of errors, for example such which are found by means of diagnostic messages of V3004 or V3012. The essence of this type of errors is that regardless of whether there will be a checked condition (V3004 for the operator 'if' and V3012 for ternary) true or false, the same operations will be always performed, or the same result is returned. Unfortunately, the errors diagnosed by warning of V3004 were not in the project, on but there was a couple of warnings of V3012 which will be considered below.
public enum WindowCommands
{
  NextDocument,
  PrevDocument,
  OpenDocumentList,
  OpenWindowList,
  SplitWindowVertically,
  SplitWindowHorizontally,
  UnsplitWindow,
  SwitchSplitWindow,
  SwitchNextDocument,
  SwitchPreviousDocument
}

protected static void Switch (bool next)
{
  if (!IdeApp.Preferences.EnableDocumentSwitchDialog) {
       IdeApp.CommandService.DispatchCommand (
         next ? WindowCommands.NextDocument : 
                WindowCommands.NextDocument);
       return;
  }

  var toplevel = Window.ListToplevels ()
                       .FirstOrDefault (w => w.HasToplevelFocus)
                       ?? IdeApp.Workbench.RootWindow;
  var sw = new DocumentSwitcher (toplevel, next);
  sw.Present ();
}

Warning of the analyzer: V3012 The'?:' operator, regardless of its conditional expression, always returns one and the same value: WindowCommands.NextDocument. MonoDevelop.Ide WindowCommands.cs 254

The ternary operator will always return the same element of transfer ('WindowCommands.NextDocument'). I will assume that if the next variable matters 'false', the WindowCommands.PrevDocument element had to return.

Besides, I have a suspicion that similar mistakes are made because of use of means of autosubstitution of a code. And during the fast work it is possible not to notice at all that the tool which has to help with writing of a code helped with writing of an error. However it only assumptions and reasonings on this subject are beyond this article.

One more similar example met:
private void StartTestElement(ITestResult result)
{
  ITest test = result.Test;
  TestSuite suite = test as TestSuite;

  if (suite != null)
  {
    xmlWriter.WriteStartElement("test-suite");
    xmlWriter.WriteAttributeString("type", suite.TestType);
    xmlWriter.WriteAttributeString("name", 
      suite.TestType == "Assembly" ? result.Test.FullName
                                   : result.Test.FullName);
  }
  ....
}

Warning of the analyzer: V3012 The'?:' operator, regardless of its conditional expression, always returns one and the same value: result.Test.FullName. GuiUnit_NET_4_5 NUnit2XmlOutputWriter.cs 207

Apparently from a code fragment, expression 'suite.TestType == "Assembly"' or false will be true, value of FullName property will be result of execution of ternary operator.

Check not of that variable on equality of 'null' after reduction by the operator 'as'


And it is already the situation specific for C#. And, judging by the checked projects, it is a certain pattern of errors, but not isolated cases. As all of us know if it was not succeeded to execute reduction, using the operator 'as', null value (in difference from explicit reduction with syntax use of' (type_name) of arg' when the exception like 'InvalidCastException' is generated) will be result. Often after such reduction check is executed to be convinced that it took place successfully. However quite often make a mistake, checking not result of reduction, but the given variable by a carelessness. Several similar cases will be considered below.
public override bool Equals (object o)
{
  SolutionItemReference sr = o as SolutionItemReference;
  if (o == null)
    return false;
  return (path == sr.path) && (id == sr.id);
}

Warning of the analyzer: V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'o', 'sr'. MonoDevelop.Core SolutionItemReference.cs 81

In this code reduction of the variable 'o' of the object type to the SolutionItemReference type is executed. If such reduction will not manage to be executed, in the sr variable null value will be written. As a result check '== null' will take place o successfully (naturally if 'o' — not 'null'), and when checking '== sr.path' is generated by path an exception like 'NullReferenceException'. All this could be avoided, having checked the correct variable in the appropriate place:
 if (sr == null)
    return false;

One more example of a similar code:
void OnTokenSelectionChanged (object sender, EventArgs args)
{
  TreeSelection selection = sender as TreeSelection;
  if (sender != null)
  {
    TreeIter iter;
    TreeModel model = (TreeModel)tokensStore;
    if (selection.GetSelected (out model, out iter)) {
        entryToken.Text = (string)tokensStore.GetValue (iter, 0);
        comboPriority.Active = (int)tokensStore.GetValue (iter, 1);
    } else
    {
      entryToken.Text = String.Empty;
      comboPriority.Active = (int)TaskPriority.Normal;
    }
  }
}

Warning of the analyzer: V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'sender', 'selection'. MonoDevelop.Ide TasksOptionsPanel.cs 123

The situation is exactly similar to previous. After reduction of 'sender' to 'TreeSelection' not that variable because of what there is a danger to receive 'NullReferenceException' is checked for 'null'.

Similar code samples met the same patterns of errors 2 more times:

The repeating checks of similar conditions


Cases when the same condition is twice checked meet, at the same time the variables used in these expressions do not change between them in any way. This error can cause much more serious effects, than it seems at first sight. At what — it is better to look on real examples.
public override void VisitIndexerExpression(
                      IndexerExpression indexerExpression)
{
  ....
  var localResolveResult = context.Resolve(indexerExpression.Target)  
                           as LocalResolveResult;
  if (localResolveResult == null)
    return;
  var resolveResult = context.Resolve(indexerExpression);
  if (localResolveResult == null)
    return;
  ....
} 

Warning of the analyzer: V3021 There are two 'if' statements with identical conditional expressions. The first 'if' statement contains method return. This means that the second 'if' statement is senseless ICSharpCode.NRefactory.CSharp.Refactoring ParameterCanBeDeclaredWithBaseTypeIssue.cs 356

From this fragment of a code clearly it is visible that instead of check 'resolveResult == null' is twice executed check 'by localResolveResult == null'. It is well visible from the cut code fragment. Whether it was possible also easy to find this error, browsing the code including in addition a fragment and the main logic to a method (here it is not brought not to stretch an example) — a big question. Anyway, instead of leaving a method if 'resolveResult' is equal to 'null', we successfully continue work, so that all subsequent logic using 'resolveResult' flies to depths of hell.

Here one more example of a similar oversight:
bool TryRemoveTransparentIdentifier(....)
{
  ....
  string nae1Name = ExtractExpressionName(ref nae1);
  if (nae1Name == null)
    return false;

  ....
  string nae2Name = ExtractExpressionName(ref nae2);
  if (nae1Name == null)
    return false;

  ....
}

Warning of the analyzer: V3021 There are two 'if' statements with identical conditional expressions. The first 'if' statement contains method return. This means that the second 'if' statement is senseless ICSharpCode.NRefactory.CSharp CombineQueryExpressions.cs 114

Again because mixed a variable for check, loop termination will not be performed and the correct value is returned, and the further logic of work of a method will be broken.

And here more interesting example, however, containing the same error:
public static SW.FontWeight ToWpfFontWeight (this FontWeight value)
{
  if (value == FontWeight.Thin)       
    return SW.FontWeights.Thin;
  if (value == FontWeight.Ultralight) 
    return SW.FontWeights.UltraLight;
  if (value == FontWeight.Light)      
    return SW.FontWeights.Light;
  if (value == FontWeight.Semilight)  
    return SW.FontWeights.Light;
  if (value == FontWeight.Book)       
    return SW.FontWeights.Normal;
  if (value == FontWeight.Medium)     
    return SW.FontWeights.Medium;
  if (value == FontWeight.Semibold)   
    return SW.FontWeights.SemiBold;
  if (value == FontWeight.Bold)       
    return SW.FontWeights.Bold;
  if (value == FontWeight.Ultrabold)  
    return SW.FontWeights.UltraBold;
  if (value == FontWeight.Heavy)      
    return SW.FontWeights.Black;
  if (value == FontWeight.Ultraheavy) 
    return SW.FontWeights.UltraBlack;

  return SW.FontWeights.Normal;
}

Well, found? I joke, here it is only possible to stick bad shot. And here for the analyzer there is no problem, and it quietly coped with a task.

Warning of the analyzer: V3021 There are two 'if' statements with identical conditional expressions. The first 'if' statement contains method return. This means that the second 'if' statement is senseless Xwt.WPF DataConverter.cs 217

In order that it is better to understand in what a problem, it is necessary to look at transfer of FontWeight.
public enum FontWeight
{
  /// The thin weight (100)
  Thin = 100,
  /// The ultra light weight (200)
  Ultralight = 200,
  /// The light weight (300)
  Light = 300,
  /// The semi light weight (350)
  Semilight = 350,
  /// The book weight (380)
  Book = 350,
  ....
}

The constants 'Semilight' and 'Book' have identical values though from comments it is accurately visible that the constant 'Book' has to matter 380.

What is more interesting — if value value is equal 380 — this method will work all the same truly! In this case any of the listed conditions will not be executed and consequently — just that value which would return at execution of a condition 'value == FontWeight.Book' will return. "Not a bug, but feature"

Well and in end of this subsection:
public override object GetData (TransferDataType type)
{
  if (type == TransferDataType.Text)
    return clipboard.WaitForText ();
  if (type == TransferDataType.Text)
    return clipboard.WaitForImage ();
  ....
}

Warning of the analyzer: V3021 There are two 'if' statements with identical conditional expressions. The first 'if' statement contains method return. This means that the second 'if' statement is senseless Xwt.Gtk ClipboardBackend.cs 86

In this fragment of a code it is easy to notice a typographical error. Instead of check repetition 'type == TransferDataType.Text' needed to execute check 'type == TransferDataType.Image'.

Check of the contradicting conditions


The code when within one expression the same variable is checked for an equality/inequality to any value meets. Such checks are at least excessive, and it is possible — even contain the error connected with the fact that the second time is checked value not of that variable. Such errors were in the project too.
IEnumerable<ICompletionData> 
  CreateConstructorCompletionData(IType hintType)
{
  ....
  if (!(hintType.Kind == TypeKind.Interface && 
        hintType.Kind != TypeKind.Array))
  ....
}

Warning of the analyzer: V3023 Consider inspecting this expression. The expression is excessive or contains a misprint. ICSharpCode.NRefactory.CSharp CSharpCompletionEngine.cs 2397

Judging by a code environment, here just pereuslozhnit with expression check. It is unclear, to what such complication as all this condition could be simplified to a code of the following type:
if (hintType.Kind != TypeKind.Interface)

Similar case:
void OnUpdateClicked (object s, StatusBarIconClickedEventArgs args)
{
  if (args.Button != Xwt.PointerButton.Right && 
      args.Button == Xwt.PointerButton.Left) {
    HideAlert ();
    AddinManagerWindow.Run (IdeApp.Workbench.RootWindow);
  }
}

Warning of the analyzer: V3023 Consider inspecting this expression. The expression is excessive or contains a misprint. MonoDevelop.Ide AddinsUpdateHandler.cs 97

By this fragment of a code it is visible that here use of other variables for comparison was not meant, but, nevertheless, excess comparison takes place to be. On Button property no additional logic is hung up, therefore, there are no "reefs" at its reading. Besides the code easily becomes simpler:
if (args.Button == Xwt.PointerButton.Left)


Incorrectly made lines of formatting


Quite often the code containing errors in lines of formatting meets. As a rule, such errors happen 2 types:
  • The amount of the expected arguments are less than quantity of actual. In that case not used arguments will be just ignored. The similar error can be a sign that the line is formed incorrectly, differently — why in it not used argument? It is possible that it remained as a result of refactoring.
  • The amount of the expected arguments are more than quantity of actual. More unpleasant case as the exception like 'FormatException' will be generated.

Only errors of the first type occurred in this project. Example of one of them:
ConditionExpression ParseReferenceExpression (string prefix)
{
  StringBuilder sb = new StringBuilder ();

  string ref_type = prefix [0] == '$' ? "a property" : "an item list";
  int token_pos = tokenizer.Token.Position;
  IsAtToken (TokenType.LeftParen, String.Format ( 
             "Expected {0} at position {1} in condition \"{2}\". 
             Missing opening parantheses after the '{3}'.",
             ref_type, token_pos, conditionStr, prefix));
  ....

  IsAtToken (TokenType.RightParen, String.Format (
             "Expected {0} at position {1} in condition \"{2}\". 
              Missing closing parantheses'.",
              ref_type, token_pos, conditionStr, prefix));
  ....
}

Warning of the analyzer: V3025 Incorrect format. A different number of format items is expected while calling 'Format' function. Expected: 3. Present: 4. MonoDevelop.Core ConditionParser.cs 254

Most likely, this error turned out as a result of unsuccessful 'copy-paste' as the second method call of 'IsAtToken' is similar with the first, distinctions only that it concerns a closing parenthesis. However the argument of 'prefix' in it is not used in any way. Not crucially, but also to sense from it is not present.

Similar warnings:
  • V3025 Incorrect format. A different number of format items is expected while calling 'Format' function. Expected: 1. Present: 2. MonoDevelop.Xml XmlFormatterWriter.cs 1131;
  • V3025 Incorrect format. A different number of format items is expected while calling 'Format' function. Expected: 4. Present: 6. ICSharpCode.NRefactory.CSharp MonoSymbolTable.cs 235
  • V3025 Incorrect format. A different number of format items is expected while calling 'Format' function. Expected: 1. Present: 2. MonoDevelop.Ide HelpOperations.cs 212
  • V3025 Incorrect format. A different number of format items is expected while calling 'Format' function. Expected: 4. Present: 6. Mono.Cecil.Mdb MonoSymbolTable.cs 235
  • V3025 Incorrect format. A different number of format items is expected while calling 'Format' function. Expected: 2. Present: 3. MonoDevelop.TextEditor.Tests ViTests.cs 255

Potential razymenovyvaniye of the zero link


Often it is necessary to check variables for equality of 'null', especially if this variable — argument of a method, result of its work, was received by means of the operator 'as'. Before its use it is necessary to make sure that the variable does not comprise null value as differently if, for example, call attempt of one of members of object is performed, the exception like 'NullReferenceException' will be generated.

But there are cases when the programmer because of a carelessness performs such check after a link razymenovyvaniye. Such cases met and here.
void Replace (RedBlackTreeNode oldNode, RedBlackTreeNode newNode)
{
  ....
  if (oldNode.parent.left == oldNode || 
      oldNode == null && oldNode.parent.left == null)
  ....
}

Warning of the analyzer: V3027 The variable 'oldNode' was utilized in the logical expression before it was verified against null in the same logical expression. MonoDevelop.HexEditor RedBlackTree.cs 167

Apparently from a code, at first some field of object 'oldNode.parent.left' is compared with object 'oldNode', and then this object and field is checked for equality of 'null'. However if 'oldNode' nevertheless matters 'null', already at the first check the exception like 'NullReferenceException' will be generated. The right decision would be to execute first of all check of object on equality of 'null'.

Conclusion


Personally I was satisfied with result of check as it was succeeded to find rather interesting errors. Not all from them were considered in this article. Many diagnostic messages were considered superficially as almost at once it became understood that more than it is enough material for article.

Someone can tell that for the project of such amount there were not so many errors. But it is necessary to remember that many errors come to light at a testing stage while with use of the static analyzer they could come to light and improve at a development stage that facilitates as process of writing and debugging of a code, and reduces a total cost of the final product.

Another checked C#-проекты


Perhaps, it will be interesting to you to esteem about check of others C#-проектов open source. But consider that some of these projects were checked during development of the analyzer so now at their check it could show the best results.

We look for errors in MonoDevelop

If you want to share this article with English-speaking audience, then I ask to use the reference to transfer: Sergey Vasiliev. Looking for Bugs in MonoDevelop.

Read article and there is a question?
Often to our articles ask the same questions. We collected answers to them here: Answers to questions of readers of articles about PVS-Studio, version 2015. Please, study the list.

This article is a translation of the original post at habrahabr.ru/post/274321/
If you have any questions regarding the material covered in the article above, please, contact the original author of the post.
If you have any complaints about this article or you want this article to be deleted, please, drop an email here: sysmagazine.com@gmail.com.

We believe that the knowledge, which is available at the most popular Russian IT blog habrahabr.ru, should be accessed by everyone, even though it is poorly translated.
Shared knowledge makes the world better.
Best wishes.

comments powered by Disqus