PVS-Studio
Now one of the important questions is: "Why do regular code analysis tool for C #».
Let's try to answer as the potential users, and to ourselves, to understand clearly why and where we are heading.
We have successfully created and continue to develop the PVS-Studio analyzer for C / C ++ language. This analyzer is realized many interesting and unique ideas to identify different types of errors. Over time, it became clear that many of the implemented diagnostics are not related to a specific programming language. It does not matter what language you use. There will always be typos, errors due to carelessness or failure Copy-Paste.
And then we decided to try to apply their experience to another programming language to C #. How this will be a successful venture, time will tell. We ourselves believe that we can gradually create a very interesting tool, the benefits of which can receive a large number of C #-developers.
Our challenge now - to start as soon as receive feedback from our potential customers as possible. Full version of PVS-Studio analyzer is not ready yet. Now there's little diagnostics (as of this writing, there were 36). But this version, you can install and try. And we are grateful to all those who would do it. It is important to make sure that we are all moving in the right direction and that the analyzer is generally functional. And so to add more and more diagnostic we will be very fast.
So, all interested are welcome to download the current version of the pilot version of PVS-Studio at this link: http://files.viva64.com/beta/PVS-Studio_setup.exe.
Note. Over time, the above link will become invalid. Therefore, if you are reading this article for the coming months or more from the time of publication, we suggest you install the current version of the distribution: http://www.viva64.com/ru/pvs-studio-download/
If the reader is not never tried PVS-Studio, I suggest to read the article "PVS-Studio for Visual C ++". As you can see, it is focused on C ++, but in fact there is no difference. In terms of the interface in general is almost no difference, you work with C ++ or C # projects.
To send your comments and suggestions, you can use the feedback page.
Checking the project SharpDevelop
For programmers ordinary advertising does not work. However, I know how to attract the attention of serious and very busy creators. We check various open source projects and write articles about it. There is no better advertising than to show that our tool can.
I see no reason to reinvent the wheel. Now the same method I will fascinate C # programmers. And that's before you check another article about the open source project SharpDevelop.
SharpDevelop - free IDE for C #, Visual Basic .NET, Boo, IronPython, IronRuby, F #, C ++. Usually used as an alternative to Visual Studio .NET. There is also a fork on a Mono / GTK + - MonoDevelop.
For us it is important that the project is written entirely in C #. And it means we can check the experimental version of PVS-Studio. In 8522 the project file with the extension «cs», which is equal to the total size of 45 megabytes.
The most suspicious code fragments
Fragment N1
public override string ToString ()
{
return String.Format ( "Thread Name = {1} Suspended = {2}"
ID, Name, Suspended);
}
Warning PVS-Studio: 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. Perhaps this is no error here. However, this is clearly a place worth checking out. Perhaps here 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);
}
Warning PVS-Studio: V3025 Incorrect format. A different number of actual arguments is expected while calling 'Format' function. Expected: 4. Present: 6. MonoSymbolTable.cs 235
A more interesting case. What is it like to make a programmer, I do not understand. Perhaps he wanted to create a message that's like the following:
[Line file.cs: 10,20-30,40: 7]
But apparently he missed some braces. So it turns out that the "2" and "4" specifies the alignment of the field, and not output the values of variables and EndRow EndColumn.
I would venture to suggest that the right will be the next format string:
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;
}
Warning PVS-Studio: V3001 There are identical sub-expressions 'mc_a.Location.File' to the left and to the right of the '! =' Operator. membercache.cs 1306
Here we are dealing with a typo. I think the best option would be the following comparison:
if (mc_a.Location.File! = mc_b.Location.File)
Fragment N4
public WhitespaceNode (string whiteSpaceText,
TextLocation startLocation)
{
this.WhiteSpaceText = WhiteSpaceText;
this.startLocation = startLocation;
}
Warning PVS-Studio: V3005 The 'this.WhiteSpaceText' variable is assigned to itself. WhitespaceNode.cs 65
Beautiful mistake. There static analyzer showed its main essence. He is attentive and not get tired, unlike humans. Therefore, he noticed a typo. Did you see it? Agree to find the error is not easy.
Thus, a typo in one letter. We had to write "= whiteSpaceText". A written "= WhiteSpaceText". As a result, the value of 'WhiteSpaceText' in the class remains unchanged.
Generally, this is a good example of how not to name the variables. Bad idea to make a difference in the names of only a lowercase / uppercase. However, on the coding style of reasoning goes beyond the subject of the article. Moreover, it smacks of holy war 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;
}
}
}
Warning PVS-Studio: V3004 The 'then' statement is equivalent to the 'else' statement. Editor.cs 225
Very suspicious that regardless of the value 'this.InvokeRequired' are made are the same steps. I have a strong suspicion that the string «base.Enabled = .....» has been copied. And then there is something forgotten to change.
Fragment N6, N7, N8, N9
public override void Run ()
{
....
ISolutionFolderNode solutionFolderNode =
node as ISolutionFolderNode;V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'geometry', 'g'. PathHandlerExtension.cs 578
if (node! = null)
{
ISolutionFolder newSolutionFolder =
solutionFolderNode.Folder.CreateFolder (....);
solutionFolderNode.Solution.Save ();
....
}
Warning PVS-Studio: 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 take some action if 'node' is inherited from interface 'ISolutionFolderNode'. But checking the variable. The correct version:
ISolutionFolderNode solutionFolderNode =
node as ISolutionFolderNode;
if (solutionFolderNode! = null)
{
By the way, it is quite common pattern in C # error-program. For example, the project encountered SharpDevelop 3 more of these errors:
V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'oldTransform', 'tg'. ModelTools.cs 420
V3019 Possibly an incorrect variable is 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;
....
}
Warning PVS-Studio: V3008 The 'foundInvocations' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 211, 209. RedundantAssignmentIssue.cs 211
Very suspicious re-assignment. Perhaps the second assignment written during debugging of 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;
}
....
}
Warning PVS-Studio: V3008 The 'pos' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 151, 148. CodeSnippet.cs 151
Another suspicious re-assignment. Here, or error, or assignment «pos = m.Index;» is superfluous.
Fragment N12
....
public string Text {get; set; }
....
protected override void OnKeyUp (KeyEventArgs e)
{
....
editor.Text.Insert (editor.CaretIndex, Environment.NewLine);
....
}
Warning PVS-Studio: V3010 The return value of function 'Insert' is required to be utilized. InPlaceEditor.cs 166
Strings in C # are immutable. Therefore, if we do something with string, the results need to be stored somewhere. However, it is easy to forget about, such as happened here. The developer decided that calling the method Insert () he had something to add to the line. But this is not the case. The correct version of the code:
editor.Text =
editor.Text.Insert (editor.CaretIndex, Environment.NewLine);
Detail of 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;
}
Warning PVS-Studio: V3010 The return value of function 'Union' is required to be utilized. MappingBase.cs 274
Actually, I have the feeling that in C # projects, I will meet a lot of errors due to the fact that the programmer expects the changes to the object, and it does not happen.
Method extension 'Union', defined for collections that implement the IEnumerable interface, allows you to get the intersection of two sets. However, the container 'value' is not changed. The correct version:
value = value.Union (baseMapping.GetMappingForTable (table));
More one such situation can be found here: V3010 The return value of function 'OrderBy' is required to be utilized. CodeCoverageMethodElement.cs 124
Fragment N15
PVS-Studio analyzer tries to identify situations where the programmer could forget something to do in the switch (). The logic of the decision, warning or not, is quite complicated. Sometimes it turns out false positives, sometimes there are obvious mistakes. Consider one of these alarms.
So, the code takes place here is a listing:
public enum TargetArchitecture {
I386,
AMD64,
IA64,
ARMv7,
}
Mostly used by all versions of the listing:
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 suspicious places. For example, the analyzer drew 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 ();
}
Warning PVS-Studio: V3002 The switch statement does not cover all values of the 'TargetArchitecture' enum: ARMv7. ImageWriter.cs 209
As you can see, does not consider the case if the architecture is ARMv7. I do not know this is a bug or not. But it seems to me that this is a mistake. ARMv7 name is at the end of transfer, which means added last. As a result, a programmer could forget the correct GetMachine () function and consider this architecture.
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;
}
}
Warning PVS-Studio: V3005 The 'linearGradientBrush.GradientStops' variable is assigned to itself. BrushEditor.cs 120
Suffice it hard to read the code snippet. And probably why it made a mistake. Most likely the code was written by Copy-Paste in one place has been incorrectly modified.
By all appearances, instead of:
linearGradientBrush.GradientStops =
linearGradientBrush.GradientStops;
It should have been written:
linearGradientBrush.GradientStops =
radialGradientBrush.GradientStops;
Scents
Many of the fragments, the analyzer, are unlikely to be the real error. On the other hand, messages issued on such a code, called false positives, too, is impossible. Usually about such code it says it smells.
Earlier I reviewed a lot of code that appear to contain errors. Now here are a few examples of odors. All situations I will not consider, it is not interesting. Limited to 3 examples. With other scents, developers can familiarize yourself by checking the SharpDevelop project.
The code snippet with the smell of N1
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;
}
}
....
}
Warning PVS-Studio: V3003 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability 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 it is not a mistake. I think that just randomly duplicated block, and it is superfluous. Nevertheless, this place is worth a look and correct.
The code snippet with the smell of N2
void PropertyExpandButton_Click (object sender, RoutedEventArgs e)
{
....
ContentPropertyNode clickedNode =
clickedButton.DataContext as ContentPropertyNode;
clickedNode = clickedButton.DataContext as ContentPropertyNode;
if (clickedNode == null)
....
}
Warning PVS-Studio: V3008 The 'clickedNode' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 105, 104. PositionedGraphNodeControl.xaml.cs 105
No comments:
Post a Comment