Experimental version of PVS-Studio supporting C #

We have an experimental version of the PVS-Studio analyzer, which is able to analyze C # projects and which can be shown to the world. This is not Release, and not even Beta. This is just the current build of PVS-Studio. We want to start getting feedback from our users or potential users regarding C # support as early as possible. Therefore, we suggest enthusiasts to try the new version of PVS-Studio on their C # projects and tell us about the results, shortcomings and express their wishes. Oh yeah, and of course the article will describe the results of checking the next project - SharpDevelop.

PVS-Studio

Now one of the important questions is: “Why make another code analysis tool for C #?”.

Let's try to answer both potential users and ourselves in order to clearly understand where and why we are moving.

We have successfully created and continue to develop the PVS-Studio analyzer for the C / C ++ language. This analyzer implements many interesting and unique ideas for identifying various types of errors. Over time, it became clear that many of the implemented diagnostics have nothing to do with a specific programming language. It doesn't matter what language you use. There will always be typos, mistakes due to carelessness or unsuccessful Copy-Paste.
')
And then we decided to try to apply our experience to another programming language, to C #. How far this will be a successful undertaking, time will tell. We ourselves believe that we will gradually be able to create a very interesting tool that can benefit from a large number of C # developers.

Now our task is to start getting feedback from our potential users as early as possible. The full version of the PVS-Studio analyzer is not ready yet. Now there are few diagnostics in it (at the time of writing this article there were 36 of them). But this version can be installed and tried. And we will be grateful to everyone who does it. It is important for us to make sure that we are generally moving in the right direction and that the analyzer as a whole is operational. And we will add new and new diagnostics very quickly.

So, I invite everyone to download the current version of the PVS-Studio experimental version from this link: http://files.viva64.com/beta/PVS-Studio_setup.exe .

Note. Over time, the above link will become invalid. Therefore, if you read this article after a month or more from the moment of publication, we suggest you install the current version of the distribution: http://www.viva64.com/ru/pvs-studio-download/

If the reader has never tried PVS-Studio, I suggest reading the article " PVS-Studio for Visual C ++ ". As you can see, it is focused on C ++, but in fact there is no difference. From the point of view of the interface, there is almost no difference at all whether you work with C ++ or with C # projects.

To send your feedback and suggestions, you can use the feedback page .

Check SharpDevelop project

For programmers, ordinary advertising does not work. However, I know how to attract the attention of these serious and very busy creators. We check various open projects and write articles about it. There is no better advertising than to show what our tool can do.

I see no reason to reinvent the wheel. Now by the same method I will fascinate C # programmers. And here is another article about checking open SharpDevelop project.

SharpDevelop is a free development environment for C #, Visual Basic .NET, Boo, IronPython, IronRuby, F #, C ++. Typically used as an alternative to Visual Studio .NET. There is also a fork on Mono / GTK + - MonoDevelop.

For us, it is important that the project is written entirely in C #. So, we can check it with an experimental version of PVS-Studio. The project has 8522 files with the extension “cs”, the total size of which is 45 megabytes.

Most suspicious code snippets

Fragment N1

public override string ToString() { return String.Format("Thread Name = {1} Suspended = {2}", ID, Name, Suspended); } 

PVS-Studio warning: V3025 Incorrect format. A different number of actual arguments is expected while calling 'Format' function. Expected: 2. Present: 3. Thread.cs 235

The variable ID is not used at all. Perhaps there is no real error here. However, this place is clearly worth checking out. Perhaps it was planned to form a completely different line.

Fragment N2

 public override string ToString () { return String.Format ("[Line {0}:{1,2}-{3,4}:{5}]", File, Row, Column, EndRow, EndColumn, Offset); } 

PVS-Studio warning: V3025 Incorrect format. A different number of actual arguments is expected while calling 'Format' function. Expected: 4. Present: 6. MonoSymbolTable.cs 235

More interesting case. What exactly the programmer wanted to do was not clear to me. Perhaps he wanted to form a message like this:

[Line file.cs: 10.20-30.40: 7]

But apparently he missed some braces. Therefore, it turns out that ", 2" and ", 4" sets the alignment of the fields, and does not display the values ​​of the EndRow and EndColumn variables at all.

I would venture to suggest that the following formatting string will be correct:

 String.Format ("[Line {0}:{1},{2}-{3},{4}:{5}]", File, Row, Column, EndRow, EndColumn, Offset); 

Fragment N3

 static MemberCore GetLaterDefinedMember(MemberSpec a, MemberSpec b) { var mc_a = a.MemberDefinition as MemberCore; var mc_b = b.MemberDefinition as MemberCore; .... 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; } 

PVS-Studio warning: V3001 There are identical sub-expressions 'mc_a.Location.File' to the left! membercache.cs 1306

Here we are dealing with a typo. I think the following comparison would be the right one:

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

Fragment N4

 public WhitespaceNode(string whiteSpaceText, TextLocation startLocation) { this.WhiteSpaceText = WhiteSpaceText; this.startLocation = startLocation; } 

PVS-Studio warning: V3005 The 'this. WhiteSpaceText' variable is assigned to itself. WhitespaceNode.cs 65

Beautiful mistake. Here the static analyzer showed its main essence. He is attentive and, unlike a person, does not get tired. So he noticed a typo. Do you see her? Agree to find the error is not easy.

So a typo in one letter. It was necessary to write "= whiteSpaceText". And it says "= WhiteSpaceText". As a result, the value of 'WhiteSpaceText' in the class remains unchanged.

In general, this is a good example of how not to name variables. A bad idea is to make a difference in the names of only one lowercase / uppercase letter. However, reasoning about coding style is beyond the scope of the article. Moreover, it smacks of a holy war of discussion.

Fragment N5

 new public bool Enabled { get { return base.Enabled; } set { if (this.InvokeRequired) { base.Enabled = this.VScrollBar.Enabled = this.hexView.Enabled =this.textView.Enabled = this.side.Enabled = this.header.Enabled = value; } else { base.Enabled = this.VScrollBar.Enabled = this.hexView.Enabled = this.textView.Enabled = this.side.Enabled = this.header.Enabled = value; } } } 

PVS-Studio warning: V3004 The 'then' statement is equivalent to the 'else' statement. Editor.cs 225

It is very suspicious that the same actions will be performed regardless of the value of 'this. InvokeRequired'. I have a strong suspicion that the string "base.Enabled = ....." was copied. And then they forgot to change something in it.

Fragment N6, N7, N8, N9

 public override void Run() { .... ISolutionFolderNode solutionFolderNode = node as ISolutionFolderNode; if (node != null) { ISolutionFolder newSolutionFolder = solutionFolderNode.Folder.CreateFolder(....); solutionFolderNode.Solution.Save(); .... } 

PVS-Studio warning: V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'node', 'solutionFolderNode'. SolutionNodeCommands.cs 127

We wanted to perform some actions if the 'node' is inherited from the 'ISolutionFolderNode' interface. But we checked the wrong variable. The correct option is:

 ISolutionFolderNode solutionFolderNode = node as ISolutionFolderNode; if (solutionFolderNode != null) { 

By the way, this is a fairly common pattern of error in C # programs. For example, in the SharpDevelop project, there were 3 more such errors:

• V3019 Possibly incorrectly variable conversion compared to null after type conversion using 'as' keyword. Check variables 'geometry', 'g'. PathHandlerExtension.cs 578
• V3019 Possibly incorrectly variable conversion compared to null after type conversion using 'as' keyword. Check variables 'oldTransform', 'tg'. ModelTools.cs 420
• V3019 Possibly incorrectly variable conversion compared to null after type conversion using 'as' keyword. Check variables 'node', 'solutionFolderNode'. SolutionNodeCommands.cs 104

Fragment N10

 public override void VisitInvocationExpression(....) { .... foundInvocations = (idExpression.Identifier == _varName); foundInvocations = true; .... } 

PVS-Studio warning: V3008 The 'foundInvocations' variable is assigned values ​​twice successively. Perhaps this is a mistake. Check lines: 211, 209. RedundantAssignmentIssue.cs 211

Very suspicious reassignment. Perhaps the second assignment was written in the process of debugging the code, and then forgot about it.

Error n11

 public static Snippet CreateAvalonEditSnippet(....) { .... int pos = 0; foreach (Match m in pattern.Matches(snippetText)) { if (pos < m.Index) { snippet.Elements.Add(....); pos = m.Index; } snippet.Elements.Add(....); pos = m.Index + m.Length; } .... } 

PVS-Studio warning: V3008 The 'pos' variable is assigned values ​​twice successively. Perhaps this is a mistake. Check lines: 151, 148. CodeSnippet.cs 151

Another suspicious reassignment. Here either an error or the assignment “pos = m.Index;” is redundant.

Fragment N12

 .... public string Text { get; set; } .... protected override void OnKeyUp(KeyEventArgs e) { .... editor.Text.Insert(editor.CaretIndex, Environment.NewLine); .... } 

PVS-Studio warning: V3010 The return value of the function 'Insert' is required to be utilized. InPlaceEditor.cs 166

C # strings are immutable. Therefore, if we do something with a string, the result must be saved somewhere. However, it is easy to forget about it, as for example it happened here. The developer decided that by calling the Insert () method he would add something to the string. But it is not. The correct code is:

 editor.Text = editor.Text.Insert(editor.CaretIndex, Environment.NewLine); 

Fragment N13, N14

 public IEnumerable<PropertyMapping> GetMappingForTable(SSDL.EntityType.EntityType table) { var value = GetSpecificMappingForTable(table); var baseMapping = BaseMapping; if (baseMapping != null) value.Union(baseMapping.GetMappingForTable(table)); return value; } 

PVS-Studio warning: V3010 The return value of the function 'Union' is required to be utilized. MappingBase.cs 274

In general, I have a premonition that in C # projects I will encounter quite a few errors related to the fact that the programmer is waiting for the object to change, but this does not happen.

The extension method 'Union', defined for collections implementing the IEnumerable interface, allows you to get the intersection of two sets. However, the container 'value' does not change. The correct option is:

 value = value.Union(baseMapping.GetMappingForTable(table)); 

Another such situation can be found here: V3010 The return value of the function 'OrderBy' is required to be utilized. CodeCoverageMethodElement.cs 124

Fragment N15

The PVS-Studio analyzer is trying to identify situations where the programmer could forget to do something in switch (). The logic of deciding whether to warn or not is quite complicated. Sometimes false alarms are obtained, sometimes there are obvious errors. Consider one of these triggers.

So, in the code there is such an enumeration:

 public enum TargetArchitecture { I386, AMD64, IA64, ARMv7, } 

In places, all variants of this listing are used:

 TargetArchitecture ReadArchitecture () { var machine = ReadUInt16 (); switch (machine) { case 0x014c: return TargetArchitecture.I386; case 0x8664: return TargetArchitecture.AMD64; case 0x0200: return TargetArchitecture.IA64; case 0x01c4: return TargetArchitecture.ARMv7; } throw new NotSupportedException (); } 

However, there are also suspicious places. For example, the analyzer called my attention to the following code:

 ushort GetMachine () { switch (module.Architecture) { case TargetArchitecture.I386: return 0x014c; case TargetArchitecture.AMD64: return 0x8664; case TargetArchitecture.IA64: return 0x0200; } throw new NotSupportedException (); } 

PVS-Studio warning: V3002 The switch statement doesn’t cover all of the TargetArchitecture 'enum: ARMv7. ImageWriter.cs 209

As you can see, the case is not considered if the architecture is ARMv7. I do not know if it is a mistake or not. But it seems to me that this is a mistake. The name ARMv7 is at the end of the listing, which means it was added last. As a result, the programmer could forget to fix the GetMachine () function and take this architecture into account.

Fragment N15

 void DetermineCurrentKind() { ..... else if (Brush is LinearGradientBrush) { linearGradientBrush = Brush as LinearGradientBrush; radialGradientBrush.GradientStops = linearGradientBrush.GradientStops; CurrentKind = BrushEditorKind.Linear; } else if (Brush is RadialGradientBrush) { radialGradientBrush = Brush as RadialGradientBrush; linearGradientBrush.GradientStops = linearGradientBrush.GradientStops; CurrentKind = BrushEditorKind.Radial; } } 

PVS-Studio warning: V3005 The 'linearGradientBrush.GradientStops' variable is assigned to itself. BrushEditor.cs 120

A rather heavy piece of code to read. And apparently so it made a mistake. Most likely the code was written by the Copy-Paste method and was incorrectly changed in one place.

Apparently, instead of:

 linearGradientBrush.GradientStops = linearGradientBrush.GradientStops; 

It should have been written:

 linearGradientBrush.GradientStops = radialGradientBrush.GradientStops; 

Smells

Many of the fragments pointed to by the analyzer are hardly real errors. On the other hand, messages issued on such a code cannot be called false alarms either. Usually about such code they say that it smells.

Above, I looked at a lot of code that seems to contain errors. Now I will give a few examples of smells. I will not consider all situations, it is not interesting. I will confine myself to 3 examples. Developers can familiarize themselves with the remaining odors by checking out the SharpDevelop project.

N1 code snippet snippet

 protected override bool CanExecuteCommand(ICommand command) { .... } else if (command == DockableContentCommands.ShowAsDocument) { if (State == DockableContentState.Document) { return false; } } .... else if (command == DockableContentCommands.ShowAsDocument) { if (State == DockableContentState.Document) { return false; } } .... } 

PVS-Studio warning: V3003 The use of if (A) {...} else if (A) {...} 'pattern was detected. There is a possibility of logical error presence. Check lines: 773, 798. DockableContent.cs 773

As you can see, the program contains two identical blocks. The condition of the lower block 'if' will never be fulfilled. But in my opinion this is not a mistake. It seems to me that they simply accidentally duplicated the block, and it is superfluous. Nevertheless, this is a place that is worth a look and fix.

N2 code snippet snippet

 void PropertyExpandButton_Click(object sender, RoutedEventArgs e) { .... ContentPropertyNode clickedNode = clickedButton.DataContext as ContentPropertyNode; clickedNode = clickedButton.DataContext as ContentPropertyNode; if (clickedNode == null) .... } 

PVS-Studio warning: V3008 The 'clickedNode' variable is assigned values ​​twice successively. Perhaps this is a mistake. Check lines: 105, 104. PositionedGraphNodeControl.xaml.cs 105

The code is redundant and can be simplified to:

 ContentPropertyNode clickedNode = clickedButton.DataContext as ContentPropertyNode; if (clickedNode == null) 

N3 code snippet snippet

 IEnumerable<ICompletionData> CreateConstructorCompletionData(IType hintType) { .... if (!(hintType.Kind == TypeKind.Interface && hintType.Kind != TypeKind.Array)) { .... } 

PVS-Studio warning: V3023 Consider inspecting this expression. The expression is misprint. CSharpCompletionEngine.cs 2392

Redundant code. The expression can be simplified:

 if (hintType.Kind != TypeKind.Interface) { 

I can continue, but perhaps enough. All other "smells" are quite monotonous and similar to those already listed.

Conclusion

Well, as you can see, C # itself does not protect against stupid errors. Therefore, I can give this picture here with a clear conscience.



Long live the unicorn, which has now learned to find errors in C # programs!

But seriously, then:

• When programming, we all make not only complex, but also simple mistakes. A lot of time is spent on finding simple errors. And sometimes a lot .
• A large number of simple errors can be identified at the stage of writing code, if you use the tools of static code analysis. The use of such tools significantly saves time, which can be spent on searching and debugging many errors.
• The most important thing in static analysis is regular use. There is no point in one-time checks static analysis. Its whole point is to find the error immediately after it appeared in the code. Rare checks take a lot of time and do little good. After all, those errors that could be found quickly and easily, already by this moment corrected by sweat and blood.

If you want to share this article with an English-speaking audience, then please use the link to the translation: Andrey Karpov. Experimental version of PVS-Studio with C # support .