1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
|
//
// Gendarme.Rules.Correctness.ReviewDoubleAssignmentRule
//
// Authors:
// Sebastien Pouliot <sebastien@ximian.com>
//
// Copyright (C) 2008 Novell, Inc (http://www.novell.com)
//
// Permission is hereby granted, free of charge, to any person obtaining
// a copy of this software and associated documentation files (the
// "Software"), to deal in the Software without restriction, including
// without limitation the rights to use, copy, modify, merge, publish,
// distribute, sublicense, and/or sell copies of the Software, and to
// permit persons to whom the Software is furnished to do so, subject to
// the following conditions:
//
// The above copyright notice and this permission notice shall be
// included in all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
// LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
// WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
//
using System;
using System.Globalization;
using Mono.Cecil;
using Mono.Cecil.Cil;
using Gendarme.Framework;
using Gendarme.Framework.Engines;
using Gendarme.Framework.Helpers;
using Gendarme.Framework.Rocks;
namespace Gendarme.Rules.Correctness {
// rule idea credits to FindBug - http://findbugs.sourceforge.net/
// SA: Double assignment of field (SA_FIELD_DOUBLE_ASSIGNMENT)
// SA: Double assignment of local variable (SA_LOCAL_DOUBLE_ASSIGNMENT)
/// <summary>
/// This rule checks for variables or fields that are assigned multiple times
/// using the same value. This won't change the value of the variable (or fields)
/// but should be reviewed since it could be a typo that hides a real issue in
/// the code.
/// </summary>
/// <example>
/// Bad example:
/// <code>
/// public class Bad {
/// private int x, y;
///
/// public Bad (int value)
/// {
/// // x is assigned twice, but y is not assigned
/// x = x = value;
/// }
/// }
/// </code>
/// </example>
/// <example>
/// Good example:
/// <code>
/// public class Good {
/// private int x, y;
///
/// public Good (int value)
/// {
/// // x = y = value; was the original meaning but since it's confusing...
/// x = value;
/// y = value;
/// }
///}
/// </code>
/// </example>
/// <remarks>This rule is available since Gendarme 2.0</remarks>
[Problem ("This method assigns the same value twice to the same variable or field.")]
[Solution ("Verify the code logic. This is likely a typo where the second assignment is unneeded or should have been used with another variable/field.")]
[EngineDependency (typeof (OpCodeEngine))]
public class ReviewDoubleAssignmentRule : Rule, IMethodRule {
static string CheckDoubleAssignementOnInstanceFields (MethodDefinition method, Instruction ins, Instruction next)
{
// for an instance fiels the pattern is more complex because we must check that we're assigning to the same instance
// first we go forward: DUP, STLOC, STFLD, LDLOC, STFLD
Instruction load = next.Next;
if ((load == null) || !load.IsLoadLocal ())
return String.Empty;
// check that this is the same variable
VariableDefinition vd1 = ins.GetVariable (method);
VariableDefinition vd2 = load.GetVariable (method);
if (vd1.Index != vd2.Index)
return String.Empty;
Instruction stfld = load.Next;
if ((stfld == null) || (stfld.OpCode.Code != Code.Stfld))
return String.Empty;
// check that we're assigning the same field twice
FieldReference fd1 = (next.Operand as FieldReference);
FieldReference fd2 = (stfld.Operand as FieldReference);
if (fd1.MetadataToken.RID != fd2.MetadataToken.RID)
return String.Empty;
// backward: DUP, (possible CONV), LD (value to be duplicated), LD instance, LD instance
if (stfld.TraceBack (method).GetOperand (method) != next.TraceBack (method).GetOperand (method))
return String.Empty;
string vd1Name = String.Empty; // vd1.Name is not valid anymore since Cecil 0.10
return String.Format (CultureInfo.InvariantCulture, "Instance field '{0}' on same variable '{1}'.", fd1.Name, vd1Name);
}
static string CheckDoubleAssignement (MethodDefinition method, Instruction ins, Instruction next)
{
// for a static field the pattern is
// DUP, STSFLD, STSFLD
if (ins.OpCode.Code == Code.Stsfld) {
if (next.OpCode.Code != Code.Stsfld)
return String.Empty;
// check that we're assigning the same field twice
FieldDefinition fd1 = ins.GetField ();
FieldDefinition fd2 = next.GetField ();
if (fd1.MetadataToken.RID != fd2.MetadataToken.RID)
return String.Empty;
return String.Format (CultureInfo.InvariantCulture, "Static field '{0}'.", fd1.Name);
} else if (ins.IsStoreLocal ()) {
// for a local variable the pattern is
// DUP, STLOC, STLOC
VariableDefinition vd2 = next.GetVariable (method);
// check that we're assigning the same variable twice
if (vd2 != null) {
VariableDefinition vd1 = ins.GetVariable (method);
if (vd1.Index != vd2.Index)
return String.Empty;
string vd1Name = String.Empty; // vd1.Name is not valid since Cecil 0.10
return String.Format (CultureInfo.InvariantCulture, "Local variable '{0}'.", vd1Name);
} else if (next.OpCode.Code == Code.Stfld) {
// instance fields are a bit more complex...
return CheckDoubleAssignementOnInstanceFields (method, ins, next);
}
}
return String.Empty;
}
public RuleResult CheckMethod (MethodDefinition method)
{
if (!method.HasBody)
return RuleResult.DoesNotApply;
// exclude methods that don't have any DUP instruction
if (!OpCodeEngine.GetBitmask (method).Get (Code.Dup))
return RuleResult.DoesNotApply;
foreach (Instruction ins in method.Body.Instructions) {
if (ins.OpCode.Code != Code.Dup)
continue;
Instruction next = ins.Next;
if (next == null)
continue;
Instruction nn = next.Next;
if (nn == null)
continue;
string msg = CheckDoubleAssignement (method, next, nn);
if (msg.Length > 0)
Runner.Report (method, ins, Severity.Medium, Confidence.Normal, msg);
}
return Runner.CurrentRuleResult;
}
}
}
|
