Author: spouliot
Date: 2008-02-15 14:56:22 -0500 (Fri, 15 Feb 2008)
New Revision: 95812
Modified:
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Portability/ChangeLog
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Portability/ExitCodeIsLimitedOnUnixRule.cs
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Portability/FeatureRequiresRootPrivilegeOnUnixRule.cs
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Portability/MonoCompatibilityReviewRule.cs
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Portability/NewLineLiteralRule.cs
Log:
2008-02-15 Sebastien Pouliot <[EMAIL PROTECTED]>
* ExitCodeIsLimitedOnUnixRule.cs
* FeatureRequiresRootPrivilegeOnUnixRule.cs
* MonoCompatibilityReviewRule.cs
* NewLineLiteralRule.cs:
Updated rules wrt framework changes.
Modified: trunk/mono-tools/gendarme/rules/Gendarme.Rules.Portability/ChangeLog
===================================================================
--- trunk/mono-tools/gendarme/rules/Gendarme.Rules.Portability/ChangeLog
2008-02-15 19:54:55 UTC (rev 95811)
+++ trunk/mono-tools/gendarme/rules/Gendarme.Rules.Portability/ChangeLog
2008-02-15 19:56:22 UTC (rev 95812)
@@ -1,3 +1,11 @@
+2008-02-15 Sebastien Pouliot <[EMAIL PROTECTED]>
+
+ * ExitCodeIsLimitedOnUnixRule.cs
+ * FeatureRequiresRootPrivilegeOnUnixRule.cs
+ * MonoCompatibilityReviewRule.cs
+ * NewLineLiteralRule.cs:
+ Updated rules wrt framework changes.
+
2008-02-12 Sebastien Pouliot <[EMAIL PROTECTED]>
* Gendarme.Rules.Portability.xml.in: Fix definition for
Modified:
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Portability/ExitCodeIsLimitedOnUnixRule.cs
===================================================================
---
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Portability/ExitCodeIsLimitedOnUnixRule.cs
2008-02-15 19:54:55 UTC (rev 95811)
+++
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Portability/ExitCodeIsLimitedOnUnixRule.cs
2008-02-15 19:56:22 UTC (rev 95812)
@@ -30,63 +30,94 @@
using Mono.Cecil.Cil;
using Gendarme.Framework;
+using Gendarme.Framework.Rocks;
namespace Gendarme.Rules.Portability {
- public class ExitCodeIsLimitedOnUnixRule : IAssemblyRule, IMethodRule {
+ [Problem ("The rule detected a value outside the 0-255 range or
couldn't be sure of the returned value.")]
+ [Solution ("Review that your return values are all between 0 and 255,
this will ensure them to works under both Unix and Windows OS.")]
+ public class ExitCodeIsLimitedOnUnixRule : Rule, IAssemblyRule,
IMethodRule {
+ private const string Message = "In Unix, unlike in Windows,
Main () method must return values between 0 and 255 inclusively. Change the
exit code or change method return type from 'int' to 'void'.";
+
private enum InspectionResult {
Good,
Bad,
Unsure
}
- public MessageCollection CheckAssembly (AssemblyDefinition
assemblyDefinition, Runner runner)
+ public override void Initialize (IRunner runner)
{
- MethodDefinition entryPoint =
assemblyDefinition.EntryPoint;
- // if no entry point, good bye
- if (entryPoint == null)
- return runner.RuleSuccess;
+ base.Initialize (runner);
- // if it returns void, we don't introspect it
- // FIXME: entryPoint.ReturnType.ReturnType should not
be null with void Main ()
- // either bad unit tests or bug in cecil
- if (entryPoint.ReturnType.ReturnType == null ||
entryPoint.ReturnType.ReturnType.FullName != "System.Int32")
- return runner.RuleSuccess;
+ // we always want to call CheckAssembly (single call on
each assembly)
+ Runner.AnalyzeAssembly += delegate (object o,
RunnerEventArgs e) {
+ Active = true;
+ };
- return CheckIntMainBody (entryPoint.Body, runner);
+ // but we want to avoid checking all methods if the
Environment type
+ // isn't referenced in a module (big performance
difference)
+ Runner.AnalyzeModule += delegate (object o,
RunnerEventArgs e) {
+ Active = false;
+ foreach (TypeReference type in
e.CurrentAssembly.MainModule.TypeReferences) {
+ if (type.FullName ==
"System.Environment") {
+ Active = true;
+ break;
+ }
+ }
+ };
}
+ private void Report (MethodDefinition method, Instruction ins,
InspectionResult result)
+ {
+ switch (result) {
+ case InspectionResult.Good:
+ // should never occur
+ break;
+ case InspectionResult.Bad:
+ Runner.Report (method, ins, Severity.Medium,
Confidence.High,
+ "Return value is outside the range of
valid values (0-255).");
+ break;
+ case InspectionResult.Unsure:
+ Runner.Report (method, ins, Severity.Medium,
Confidence.Low,
+ "Make sure not to return values that
are out of range (0-255).");
+ break;
+ }
+ }
- private static MessageCollection CheckIntMainBody (MethodBody
body, Runner runner)
+ public RuleResult CheckAssembly (AssemblyDefinition assembly)
{
- MessageCollection messages = runner.RuleSuccess;
+ MethodDefinition entry_point = assembly.EntryPoint;
+
+ // the rule does not apply if the assembly has no entry
point
+ // or if it's entry point has no IL
+ if ((entry_point == null) || !entry_point.HasBody)
+ return RuleResult.DoesNotApply;
+
+ // the rule does not apply of the entry point returns
void
+ // FIXME: entryPoint.ReturnType.ReturnType should not
be null with void Main ()
+ // either bad unit tests or bug in cecil
+ if (entry_point.ReturnType.ReturnType == null ||
entry_point.ReturnType.ReturnType.FullName != "System.Int32")
+ return RuleResult.DoesNotApply;
+
Instruction previous = null;
- foreach (Instruction current in body.Instructions) {
+ foreach (Instruction current in
entry_point.Body.Instructions) {
switch (current.OpCode.Code) {
case Code.Nop:
break;
case Code.Ret:
- InspectionResult results =
CheckInstruction (previous);
- if (results == InspectionResult.Good)
+ InspectionResult result =
CheckInstruction (previous);
+ if (result == InspectionResult.Good)
break;
- Location loc = new Location
(body.Method, current.Offset);
- Message message = null;
- if (results == InspectionResult.Bad)
- message = new Message ("In
Unix, unlike in Windows, Main () method must return values between 0 and 255
inclusively. Change the exit code or change method return type from 'int' to
'void'.", loc, MessageType.Error);
- else if (results ==
InspectionResult.Unsure)
- message = new Message ("In
Unix, unlike in Windows, Main () method must return values between 0 and 255
inclusively. Be sure not to return values that are out of range.", loc,
MessageType.Warning);
- if (messages == null)
- messages = new
MessageCollection ();
- messages.Add (message);
+ Report (entry_point, current, result);
break;
default:
previous = current;
break;
}
}
- return messages;
+ return Runner.CurrentRuleResult;
}
private static InspectionResult CheckInstruction (Instruction
instruction)
@@ -114,29 +145,17 @@
default:
return InspectionResult.Unsure;
}
-
}
- public MessageCollection CheckMethod (MethodDefinition method,
Runner runner)
+ public RuleResult CheckMethod (MethodDefinition method)
{
- // here we check for usage of Environment.ExitCode
property
- MethodDefinition entryPoint =
method.DeclaringType.Module.Assembly.EntryPoint;
+ // rule does not apply if method has no IL
+ if (!method.HasBody)
+ return RuleResult.DoesNotApply;
- // assembly must have an entry point
- if (entryPoint == null)
- return runner.RuleSuccess;
-
- // rule applies only to void Main-ish assemblies
- // int Main () anyways returns something so we
shouldn't care about ExitCode usage
- if (entryPoint.ReturnType.ReturnType != null &&
entryPoint.ReturnType.ReturnType.FullName == "System.Int32")
- return runner.RuleSuccess;
-
// go!
- MessageCollection messages = runner.RuleSuccess;
Instruction previous = null;
foreach (Instruction current in
method.Body.Instructions) {
- // rather useful for debugging
- // Console.WriteLine ("{0} {1}",
current.OpCode, current.Operand);
switch (current.OpCode.Code) {
case Code.Nop:
break;
@@ -149,25 +168,16 @@
if (calledMethod.DeclaringType.FullName
!= "System.Environment")
break;
- InspectionResult results =
CheckInstruction (previous);
- if (results == InspectionResult.Good)
+ InspectionResult result =
CheckInstruction (previous);
+ if (result == InspectionResult.Good)
break;
- if (messages == runner.RuleSuccess)
- messages = new
MessageCollection ();
-
- Message message = null;
- Location loc = new Location (method,
current.Offset);
- if (results == InspectionResult.Unsure)
- message = new Message ("In
Unix, unlike in Windows, process exit code can be a value between 0 and 255
inclusively. Be sure not to set it to values that are out of range.", loc,
MessageType.Warning);
- else // bad
- message = new Message ("In
Unix, unlike in Windows, process exit code can be a value between 0 and 255
inclusively. Do not set it to values that are out of range.", loc,
MessageType.Error);
- messages.Add (message);
+ Report (method, current, result);
break;
}
previous = current;
}
- return messages;
+ return Runner.CurrentRuleResult;
}
}
}
Modified:
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Portability/FeatureRequiresRootPrivilegeOnUnixRule.cs
===================================================================
---
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Portability/FeatureRequiresRootPrivilegeOnUnixRule.cs
2008-02-15 19:54:55 UTC (rev 95811)
+++
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Portability/FeatureRequiresRootPrivilegeOnUnixRule.cs
2008-02-15 19:56:22 UTC (rev 95812)
@@ -38,8 +38,15 @@
namespace Gendarme.Rules.Portability {
- public class FeatureRequiresRootPrivilegeOnUnixRule : IMethodRule {
+ [Problem ("The method use some features that requires 'root' priviledge
under Unix.")]
+ [Solution ("Make sure your code can work without requiring users to
have 'root' priviledge.")]
+ public class FeatureRequiresRootPrivilegeOnUnixRule : Rule, IMethodRule
{
+ // localizable
+ private const string ProcessMessage = "Setting
Process.PriorityClass to something else than ProcessPriorityClass.Normal
requires root privileges.";
+ private const string PingMessage = "Usage of
System.Net.NetworkInformation.Ping requires root privileges.";
+
+ // non-localizable
private const string Ping =
"System.Net.NetworkInformation.Ping";
//Check for usage of
System.Diagnostics.Process.set_PriorityClass
@@ -86,47 +93,36 @@
case Code.Calli:
case Code.Callvirt:
MethodReference method = (ins.Operand as
MethodReference);
- return (method.DeclaringType.FullName ==
"System.Net.NetworkInformation.Ping");
+ return (method.DeclaringType.FullName == Ping);
}
return false;
}
- public MessageCollection CheckMethod (MethodDefinition method,
Runner runner)
+ public RuleResult CheckMethod (MethodDefinition method)
{
+ // rule does not apply to methods without IL
if (!method.HasBody)
- return runner.RuleSuccess;
+ return RuleResult.DoesNotApply;
- // do not apply the rule to the Ping class itself (i.e.
when running Gendarme on System.dll 2.0+)
- bool is_ping = (method.DeclaringType.FullName == Ping ||
- (method.DeclaringType.IsNested &&
method.DeclaringType.DeclaringType.FullName == Ping));
+ // Ping only exists in fx 2.0 and later
+ bool fx20 =
(method.DeclaringType.Module.Assembly.Runtime >= TargetRuntime.NET_2_0);
- MessageCollection results = null;
-
foreach (Instruction ins in method.Body.Instructions) {
// Check for usage of
System.Diagnostics.Process.set_PriorityClass
if (CheckProcessSetPriorityClass (ins)) {
- if (results == null)
- results = new MessageCollection
();
-
- Location loc = new Location (method,
ins.Offset);
- Message msg = new Message ("Setting
Process.PriorityClass to something else than ProcessPriorityClass.Normal
requires root privileges.", loc, MessageType.Warning);
- results.Add (msg);
+ // code won't work with default
(non-root) users == High
+ Runner.Report (method, ins,
Severity.High, Confidence.High, ProcessMessage);
}
// short-circuit
- if (!is_ping && CheckPing (ins)) {
- if (results == null)
- results = new MessageCollection
();
-
- Location loc = new Location (method,
ins.Offset);
- string s = String.Format ("Usage of {0}
requires root privileges.", Ping);
- Message msg = new Message (s, loc,
MessageType.Warning);
- results.Add (msg);
+ if (fx20 && CheckPing (ins)) {
+ // code won't work with default
(non-root) users == High
+ Runner.Report (method, ins,
Severity.High, Confidence.High, PingMessage);
}
}
- return results;
+ return Runner.CurrentRuleResult;
}
}
}
Modified:
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Portability/MonoCompatibilityReviewRule.cs
===================================================================
---
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Portability/MonoCompatibilityReviewRule.cs
2008-02-15 19:54:55 UTC (rev 95811)
+++
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Portability/MonoCompatibilityReviewRule.cs
2008-02-15 19:56:22 UTC (rev 95812)
@@ -27,51 +27,48 @@
//
using System;
-using System.Text;
using System.Collections.Generic;
-using System.Diagnostics;
using System.IO;
+using System.Net;
using Mono.Cecil;
using Mono.Cecil.Cil;
using Gendarme.Framework;
-using ICSharpCode.SharpZipLib;
using ICSharpCode.SharpZipLib.Zip;
using MoMA.Analyzer.MoMAWebService;
namespace Gendarme.Rules.Portability {
- public class MonoCompatibilityReviewRule : IMethodRule {
+ [Problem ("The method has some known limitations when used with the
Mono:: runtime.")]
+ [Solution ("Check if this code is critical to your application. Also
make sure your definition files are up to date.")]
+ public class MonoCompatibilityReviewRule : Rule, IMethodRule {
- private System.Net.WebException DownloadExceptionInternal;
+ private const string NotImplementedMessage = "{0} is not
implemented.{1}";
+ private const string MissingMessage = "{0} is missing from
Mono.{1}";
+ private const string TodoMessage = "{0} is marked with a
[MonoTODO] attribute: {1}.";
+
private Dictionary<string, string> NotImplementedInternal;
//value is unused
private Dictionary<string, string> MissingInternal; //value is
unused
private Dictionary<string, string> TodoInternal; //value = TODO
Description
- public System.Net.WebException DownloadException {
- get { return DownloadExceptionInternal; }
- set { DownloadExceptionInternal = value; }
- }
-
public Dictionary<string, string> NotImplemented {
get { return NotImplementedInternal; }
- set { NotImplementedInternal = value; }
}
public Dictionary<string, string> Missing {
get { return MissingInternal; }
- set { MissingInternal = value; }
}
- public Dictionary<string, string> Todo {
+ public Dictionary<string, string> ToDo {
get { return TodoInternal; }
- set { TodoInternal = value; }
}
- public MonoCompatibilityReviewRule ()
+ public override void Initialize (IRunner runner)
{
+ base.Initialize (runner);
+
string localAppDataFolder = Environment.GetFolderPath
(Environment.SpecialFolder.LocalApplicationData);
string definitionsFolder = Path.Combine
(localAppDataFolder, "Gendarme");
string definitionsFile = Path.Combine
(definitionsFolder, "definitions.zip");
@@ -80,17 +77,8 @@
if (!Directory.Exists (definitionsFolder))
Directory.CreateDirectory
(definitionsFolder);
- try {
- //try to download files from the net
- MoMASubmit ws = new MoMASubmit ();
- string definitionsUri =
ws.GetLatestDefinitionsVersion ().Split ('|') [2];
- ws.Dispose ();
-
- System.Net.WebClient wc = new
System.Net.WebClient ();
- wc.DownloadFile (new Uri
(definitionsUri), definitionsFile);
- }
- catch (System.Net.WebException e) {
- DownloadException = e;
+ if (!Download (definitionsFile)) {
+ Active = false;
return;
}
}
@@ -100,20 +88,43 @@
while ((ze = zs.GetNextEntry ()) != null) {
switch (ze.Name) {
case "exception.txt":
- NotImplemented = Read (new StreamReader
(zs), false);
+ NotImplementedInternal = Read (new
StreamReader (zs), false);
break;
case "missing.txt":
- Missing = Read (new StreamReader (zs),
false);
+ MissingInternal = Read (new
StreamReader (zs), false);
break;
case "monotodo.txt":
- Todo = Read (new StreamReader (zs),
true);
+ TodoInternal = Read (new StreamReader
(zs), true);
break;
default:
break;
}
}
+
+ // rule is active only if we have, at least one of, the
MoMA files
+ Active = ((NotImplemented != null) || (Missing != null)
|| (ToDo != null));
}
+ private bool Download (string definitionsFile)
+ {
+ // try to download files from the net
+ try {
+ string definitionsUri;
+ using (MoMASubmit ws = new MoMASubmit ()) {
+ definitionsUri =
ws.GetLatestDefinitionsVersion ().Split ('|') [2];
+ }
+
+ WebClient wc = new WebClient ();
+ wc.DownloadFile (new Uri (definitionsUri),
definitionsFile);
+ }
+ catch (WebException e) {
+ if (Runner.VerbosityLevel > 0)
+ Console.WriteLine (e);
+ return false;
+ }
+ return true;
+ }
+
private static Dictionary<string, string> Read (StreamReader
reader, bool split)
{
Dictionary<string, string> dict = new
Dictionary<string, string> ();
@@ -129,43 +140,26 @@
return dict;
}
- private static void Check (Dictionary<string, string> dict,
string calledMethod,
- MethodDefinition method, Instruction ins,
- ref MessageCollection results, string error,
MessageType type)
+ private void Check (Dictionary<string, string> dict,
MethodDefinition method, Instruction ins, string error, Severity severity)
{
- if (dict == null)
+ string callee = ins.Operand.ToString ();
+ if (!dict.ContainsKey (callee))
return;
- if (!dict.ContainsKey (calledMethod))
- return;
- if (results == null)
- results = new MessageCollection ();
-
- error = string.Format (error, calledMethod, dict
[calledMethod]);
- Location loc = new Location (method, ins.Offset);
-
- Message msg = new Message (error, loc, type);
- results.Add (msg);
+ string message = String.Format (error, callee, dict
[callee]);
+ // confidence is Normal since we can't be sure if MoMA
data is up to date
+ Runner.Report (method, ins, severity,
Confidence.Normal, message);
}
- public MessageCollection CheckMethod (MethodDefinition method,
Runner runner)
+ public RuleResult CheckMethod (MethodDefinition method)
{
+ // rule doesn't apply if method has no IL
if (!method.HasBody)
- return runner.RuleSuccess;
+ return RuleResult.DoesNotApply;
- MessageCollection results = null;
+ // rule applies
- if (DownloadException != null) {
- results = new MessageCollection ();
- results.Add (new Message (string.Format
("Unable to read or download the definitions file: {0}.",
DownloadException.Message), null, MessageType.Warning));
- DownloadException = null;
- }
-
- if (NotImplemented == null && Missing == null && Todo
== null)
- return results;
-
foreach (Instruction ins in method.Body.Instructions) {
-
switch (ins.OpCode.Code) {
case Code.Call:
case Code.Calli:
@@ -174,23 +168,27 @@
case Code.Initobj:
case Code.Ldftn:
case Code.Ldvirtftn:
+ // calling not implemented method is
very likely not to work == High
+ if (NotImplemented != null) {
+ Check (NotImplemented, method,
ins, NotImplementedMessage, Severity.High);
+ }
- string calledMethodString =
ins.Operand.ToString ();
+ // calling missing methods can't work
== Critical
+ if (Missing != null) {
+ Check (Missing, method, ins,
MissingMessage, Severity.Critical);
+ }
- Check (NotImplemented,
calledMethodString, method, ins,
- ref results, "{0} is not
implemented.", MessageType.Warning);
- Check (Missing, calledMethodString,
method, ins,
- ref results, "{0} is missing.",
MessageType.Error);
- Check (Todo, calledMethodString,
method, ins,
- ref results, "{0} is marked with
the MonoTODOAttribute ({1}).", MessageType.Warning);
-
+ // calling todo methods migh work with
some limitations == Medium
+ if (ToDo != null) {
+ Check (ToDo, method, ins,
TodoMessage, Severity.Medium);
+ }
break;
default:
break;
}
}
- return results;
+ return Runner.CurrentRuleResult;
}
}
}
Modified:
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Portability/NewLineLiteralRule.cs
===================================================================
---
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Portability/NewLineLiteralRule.cs
2008-02-15 19:54:55 UTC (rev 95811)
+++
trunk/mono-tools/gendarme/rules/Gendarme.Rules.Portability/NewLineLiteralRule.cs
2008-02-15 19:56:22 UTC (rev 95812)
@@ -35,19 +35,20 @@
namespace Gendarme.Rules.Portability {
- public class NewLineLiteralRule: IMethodRule {
+ [Problem ("The method use some literal values for new lines (e.g.
\\r\\n) which aren't portable across operating systems.")]
+ [Solution ("Replace literals with Environment.NewLine.")]
+ public class NewLineLiteralRule : Rule, IMethodRule {
private static char[] InvalidChar = { '\r', '\n' };
- public MessageCollection CheckMethod (MethodDefinition method,
Runner runner)
+ public RuleResult CheckMethod (MethodDefinition method)
{
// methods can be empty (e.g. p/invoke declarations)
if (!method.HasBody)
- return runner.RuleSuccess;
+ return RuleResult.DoesNotApply;
// rule applies
- MessageCollection results = null;
foreach (Instruction ins in method.Body.Instructions) {
switch (ins.OpCode.Code) {
case Code.Ldstr:
@@ -57,17 +58,11 @@
continue;
if (s.IndexOfAny (InvalidChar) >= 0) {
- Location loc = new Location
(method, ins.Offset);
// make the invalid char
visible on output
s = s.Replace ("\n", "\\n");
s = s.Replace ("\r", "\\r");
- Message msg = new Message
(String.Format ("Found string: \"{0}\"", s),
- loc,
MessageType.Warning);
-
- if (results == null)
- results = new
MessageCollection (msg);
- else
- results.Add (msg);
+ s = String.Format ("Found
string: \"{0}\"", s);
+ Runner.Report (method, ins,
Severity.Low, Confidence.High, s);
}
break;
default:
@@ -75,7 +70,7 @@
}
}
- return results;
+ return Runner.CurrentRuleResult;
}
}
}
_______________________________________________
Mono-patches maillist - [email protected]
http://lists.ximian.com/mailman/listinfo/mono-patches
