Gendarme.Rules.Xamarin/lib/Gendarme.Rules.BadPractice....

910 строки
35 KiB
XML

<?xml version="1.0"?>
<doc>
<assembly>
<name>Gendarme.Rules.BadPractice</name>
</assembly>
<members>
<member name="T:Gendarme.Rules.BadPractice.AvoidAssemblyVersionMismatchRule">
<summary>
This rule checks that the <c>[AssemblyVersion]</c> matches the <c>[AssemblyFileVersion]</c>
when both are present inside an assembly. Having different version numbers in both
attributes can be confusing once the application is deployed.
</summary>
<example>
Bad example:
<code>
[assembly: AssemblyVersion ("2.2.0.0")]
[assembly: AssemblyFileVersion ("1.0.0.0")]
</code></example>
<example>
Good example:
<code>
[assembly: AssemblyVersion ("2.2.0.0")]
[assembly: AssemblyFileVersion ("2.2.0.0")]
</code></example>
<remarks>This rule is available since Gendarme 2.2</remarks>
</member>
<member name="T:Gendarme.Rules.BadPractice.AvoidCallingProblematicMethodsRule">
<summary>
This rule warns about methods that calls into potentially dangerous API of the .NET
framework. If possible try to avoid the API (there are generally safer ways to do the
same) or at least make sure your code can be safely called from others.
<list type="bullet"><item><description><c>System.GC::Collect()</c></description></item><item><description><c>System.Threading.Thread::Suspend()</c> and <c>Resume()</c></description></item><item><description><c>System.Runtime.InteropServices.SafeHandle::DangerousGetHandle()</c></description></item><item><description><c>System.Reflection.Assembly::LoadFrom()</c>, <c>LoadFile()</c> and
<c>LoadWithPartialName()</c></description></item><item><description><c>System.Type::InvokeMember()</c> when used with
<c>BindingFlags.NonPublic</c></description></item></list></summary>
<example>
Bad example:
<code>
public void Load (string filename)
{
Assembly a = Assembly.LoadFile (filename);
// ...
}
</code></example>
<example>
Good example:
<code>
public void Load (string filename)
{
AssemblyName aname = AssemblyName.GetAssemblyName (filename);
// ensure it's the assembly you expect (e.g. public key, version...)
Assembly a = Assembly.Load (aname);
// ...
}
</code></example>
<remarks>This rule is available since Gendarme 2.0</remarks>
</member>
<member name="T:Gendarme.Rules.BadPractice.AvoidNullCheckWithAsOperatorRule">
<summary>
The rule will detect if a null check is done before using the <c>as</c> operator.
This null check is not needed, a <c>null</c> instance will return <c>null</c>,
and the code will need to deal with <c>as</c> returning a null value anyway.
</summary>
<example>
Bad example:
<code>
public string AsString (object obj)
{
return (o == null) ? null : o as string;
}
</code></example>
<example>
Good example:
<code>
public string AsString (object obj)
{
return (o as string);
}
</code></example>
</member>
<member name="T:Gendarme.Rules.BadPractice.AvoidVisibleConstantFieldRule">
<summary>
This rule looks for constant fields which are visible outside the current assembly.
Such fields, if used outside the assemblies, will have their value (not the field
reference) copied into the other assembly. Changing the field's value requires that all
assemblies which use the field to be recompiled. Declaring the field
as <c>static readonly</c>, on the other hand, allows the value to be changed
without requiring that client assemblies be recompiled.
</summary>
<example>
Bad example:
<code>
// if this fields is used inside another assembly then
// the integer 42, not the field, will be baked into it
public const int MagicNumber = 42;
</code></example>
<example>
Good example:
<code>
// if this field is used inside another assembly then
// that assembly will reference the field instead of
// embedding the value
static public readonly int MagicNumber = 42;
</code></example>
<remarks>This rule is available since Gendarme 2.0</remarks>
</member>
<member name="T:Gendarme.Rules.BadPractice.CheckNewExceptionWithoutThrowingRule">
<summary>
This rule checks for exception objects which are created but not thrown, not returned,
and not passed to another method as an argument.
</summary>
<example>
Bad example:
<code>
void MissingThrow (object arg)
{
if (arg == null) {
new ArgumentNullException ("arg");
}
DoWork (arg);
}
</code></example>
<example>
Good examples:
<code>
void Throw (object arg)
{
if (arg == null) {
throw new ArgumentNullException ("arg");
}
DoWork (arg);
}
Exception CreateException ()
{
return new Exception ();
}
</code></example>
</member>
<member name="T:Gendarme.Rules.BadPractice.CheckNewThreadWithoutStartRule">
<summary>
This rule checks for threads which are created but not started, or returned or passed
to another method as an argument.
</summary>
<example>
Bad example:
<code>
void UnusedThread ()
{
Thread thread = new Thread (threadStart);
thread.Name = "Thread 1";
}
</code></example>
<example>
Good examples:
<code>
void Start ()
{
Thread thread = new Thread (threadStart);
thread.Name = "Thread 1";
thread.Start ();
}
Thread InitializeThread ()
{
Thread thread = new Thread (threadStart);
thread.Name = "Thread 1";
return thread;
}
</code></example>
</member>
<member name="T:Gendarme.Rules.BadPractice.CloneMethodShouldNotReturnNullRule">
<summary>
This rule checks for <c>Clone()</c> methods which return <c>null</c>.
</summary>
<example>
Bad example:
<code>
public class MyClass : ICloneable {
public object Clone ()
{
MyClass myClass = new MyClass ();
// set some internals
return null;
}
}
</code></example>
<example>
Good example:
<code>
public class MyClass : ICloneable {
public object Clone ()
{
MyClass myClass = new MyClass ();
// set some internals
return myClass;
}
}
</code></example>
</member>
<member name="T:Gendarme.Rules.BadPractice.ConstructorShouldNotCallVirtualMethodsRule">
<summary>
This rule warns the developer if any virtual methods are called in the constructor
of a non-sealed type. The problem is that if a derived class overrides the method
then that method will be called before the derived constructor has had a chance
to run. This makes the code quite fragile.
</summary>
<example>
Bad example:
<code>
class A {
public A ()
{
this.DoSomething ();
}
protected virtual void DoSomething ()
{
}
}
class B : A {
private int x;
public B ()
{
x = 10;
}
protected override void DoSomething ()
{
Console.WriteLine (x);
}
}
B b = new B (); // outputs 0 because B's constructor hasn't been called yet
</code></example>
<example>
Good example:
<code>
class A {
public void Run ()
{
this.DoSomething ();
}
protected virtual void DoSomething ()
{
}
}
class B : A {
private int x;
public B ()
{
x = 10;
}
protected override void DoSomething ()
{
Console.WriteLine (x);
}
}
B b = new B ();
b.Run (); // outputs 10 as intended
</code></example>
<remarks>This rule is available since Gendarme 2.0</remarks>
</member>
<member name="T:Gendarme.Rules.BadPractice.DisableDebuggingCodeRule">
<summary>
This rule checks for non-console applications which contain calls to <c>Console.WriteLine</c>.
These are often used as debugging aids but such code should be removed or disabled in
the released version. If you don't want to remove it altogether you can place it inside a method
decorated with <c>[Conditional ("DEBUG")]</c>, use <c>Debug.WriteLine</c>, use
<c>Trace.WriteLine</c>, or use the preprocessor. But note that TRACE is often enabled
in release builds so if you do use that you'll probably want to use a config file to remove
the default trace listener.
</summary>
<example>
Bad example:
<code>
private byte[] GenerateKey ()
{
byte[] key = new byte[16];
rng.GetBytes (key);
Console.WriteLine ("debug key = {0}", BitConverter.ToString (key));
return key;
}
</code></example>
<example>
Good example (removed):
<code>
private byte[] GenerateKey ()
{
byte[] key = new byte[16];
rng.GetBytes (key);
return key;
}
</code></example>
<example>
Good example (changed):
<code>
private byte[] GenerateKey ()
{
byte[] key = new byte[16];
rng.GetBytes (key);
Debug.WriteLine ("debug key = {0}", BitConverter.ToString (key));
return key;
}
</code></example>
<remarks>This rule is available since Gendarme 2.0</remarks>
</member>
<member name="T:Gendarme.Rules.BadPractice.DoNotDecreaseVisibilityRule">
<summary>
The rule detect when a method visibility is decreased in an inherited type.
Decreasing visibility does not prevent calling the base class method unless
the type is <c>sealed</c>. Note that some language (but not C#) will allow
you to seal, e.g. <c>final</c>, the method without an <c>override</c>.
</summary>
<example>
Bad example:
<code>
public class Base {
public void Public ()
{
}
}
public class BadInheritor : Base {
private new void Public ()
{
}
}
</code></example>
<example>
Good example (do not hide):
<code>
public class Inheritor : Base {
}
</code></example>
<example>
Good example (sealed type):
<code>
public sealed class Inheritor : Base {
private new void Public ()
{
}
}
</code></example>
</member>
<member name="T:Gendarme.Rules.BadPractice.DoNotForgetNotImplementedMethodsRule">
<summary>
This rule checks for short methods that throw a <c>System.NotImplementedException</c>
exception. It's likely a method that has not yet been implemented and should not be
forgotten by the developer before a release.
</summary>
<example>
Bad example:
<code>
private void Save ()
{
throw new NotImplementedException ("pending final format");
}
</code></example>
<remarks>This rule is available since Gendarme 2.0</remarks>
</member>
<member name="T:Gendarme.Rules.BadPractice.DoNotUseEnumIsAssignableFromRule">
<summary>
This rule checks for calls to <c>typeof (Enum).IsAssignableFrom (type)</c> that
can be simplified to <c>type.IsEnum</c>.
</summary>
<example>
Bad example:
<code>
if (typeof (Enum).IsAssignableFrom (type)) {
// then the type is an enum
}
</code></example>
<example>
Good example:
<code>
if (type.IsEnum) {
// then the type is an enum.
}
</code></example>
<remarks>This rule is available since Gendarme 2.6</remarks>
</member>
<member name="T:Gendarme.Rules.BadPractice.DoNotUseGetInterfaceToCheckAssignabilityRule">
<summary>
This rule checks for calls to <c>Type.GetInterface</c> that look like they query if
a type is supported, i.e. the result is only used to compare against <c>null</c>.
The problem is that only assembly qualified names uniquely identify a type so if
you just use the interface name or even just the name and namespace you may
get unexpected results.
</summary>
<example>
Bad example:
<code>
if (type.GetInterface ("IConvertible") != null) {
// then the type can be assigned to IConvertible
// but what if there is another IConvertible in there ?!?
}
</code></example>
<example>
Good example:
<code>
if (typeof (IConvertible).IsAssignableFrom (type)) {
// then the type can be assigned to IConvertible
// without a doubt!
}
</code></example>
<remarks>This rule is available since Gendarme 2.2</remarks>
</member>
<member name="T:Gendarme.Rules.BadPractice.EqualsShouldHandleNullArgRule">
<summary>
This rule ensures that <c>Equals(object)</c> methods return <c>false</c> when the
object parameter is <c>null</c>.
</summary>
<example>
Bad example:
<code>
public bool Equals (object obj)
{
// this would throw a NullReferenceException instead of returning false
return ToString ().Equals (obj.ToString ());
}
</code></example>
<example>
Good example:
<code>
public override bool Equals (object obj)
{
if (obj == null) {
return false;
}
return ToString ().Equals (obj.ToString ());
}
</code></example>
</member>
<member name="T:Gendarme.Rules.BadPractice.GetEntryAssemblyMayReturnNullRule">
<summary>
This rule warns when an assembly without an entry point (i.e. a dll or library) calls
<c>Assembly.GetEntryAssembly ()</c>. This call is problematic since it will always
return <c>null</c> when called from outside the root (main) application domain. This may
become a problem inside libraries that can be used, for example, inside ASP.NET
applications.
</summary>
<example>
Bad example:
<code>
// this will throw a NullReferenceException from an ASP.NET page
Response.WriteLine (Assembly.GetEntryAssembly ().CodeBase);
</code></example>
<example>
Good example:
<code>
public class MainClass {
static void Main ()
{
Console.WriteLine (Assembly.GetEntryAssembly ().CodeBase);
}
}
</code></example>
</member>
<member name="T:Gendarme.Rules.BadPractice.ObsoleteMessagesShouldNotBeEmptyRule">
<summary>
This rule warns if any type (including classes, structs, enums, interfaces and
delegates), field, property, events, method and constructor are decorated with
an empty <c>[Obsolete]</c> attribute because the attribute is much more helpful
if it includes advice on how to deal with the situation (e.g. the new recommended
API to use).
</summary>
<example>
Bad example:
<code>
[Obsolete]
public byte[] Key {
get {
return (byte[]) key.Clone ();
}
}
</code></example>
<example>
Good example:
<code>
[Obsolete ("Use the new GetKey() method since properties should not return arrays.")]
public byte[] Key {
get {
return (byte[]) key.Clone ();
}
}
</code></example>
</member>
<member name="T:Gendarme.Rules.BadPractice.OnlyUseDisposeForIDisposableTypesRule">
<summary>
To avoid confusing developers methods named Dispose should be
reserved for types that implement IDisposable.
</summary>
<example>
Bad example:
<code>
internal sealed class Worker
{
// This class uses one or more temporary files to do its work.
private List&lt;string&gt; files = new List&lt;string&gt; ();
// This is confusing: developers will think they can do things
// like use the instance with a using statement.
public void Dispose ()
{
foreach (string path in files) {
File.Delete (path);
}
files.Clear ();
}
}
</code></example>
<example>
Good example:
<code>
internal sealed class Worker
{
// This class uses one or more temporary files to do its work.
private List&lt;string&gt; files = new List&lt;string&gt; ();
public void Reset ()
{
foreach (string path in files) {
File.Delete (path);
}
files.Clear ();
}
}
</code></example>
<remarks>This rule is available since Gendarme 2.6</remarks>
</member>
<member name="T:Gendarme.Rules.BadPractice.PreferEmptyInstanceOverNullRule">
<summary>
This rule checks that all methods and properties which return a string, an array,
a collection, or an enumerable do not return <c>null</c>.
It is usually better to return an empty instance, as this allows
the caller to use the result without having to perform a null-check first.
</summary>
<example>
Bad example (string):
<code>
public string DisplayName {
get {
if (IsAnonymous) {
return null;
}
return name;
}
}
</code></example>
<example>
Good example (string):
<code>
public string DisplayName {
get {
if (IsAnonymous) {
return string.Empty;
}
return name;
}
}
</code></example>
<example>
Bad example (array):
<code>
public int [] GetOffsets ()
{
if (!store.HasOffsets) {
return null;
}
store.LoadOffsets ();
return store.Offsets;
}
</code></example>
<example>
Good example (array):
<code>
static const int [] Empty = new int [0];
public int [] GetOffsets ()
{
if (!store.HasOffsets) {
return Empty;
}
store.LoadOffsets ();
return store.Offsets.ToArray ();
}
</code></example>
<example>
Bad example (enumerable):
<code>
public IEnumerable&lt;int&gt; GetOffsets ()
{
if (!store.HasOffsets) {
return null;
}
store.LoadOffsets ();
return store.Offsets;
}
</code></example>
<example>
Good example (enumerable):
<code>
public IEnumerable&lt;int&gt; GetOffsets ()
{
if (!store.HasOffsets) {
yield break;
}
store.LoadOffsets ();
foreach (int offset in store.Offsets) {
yield return offset;
}
}
</code></example>
<remarks>This rule is available since Gendarme 2.4</remarks>
</member>
<member name="T:Gendarme.Rules.BadPractice.PreferParamsArrayForVariableArgumentsRule">
<summary>
The rule warns for any method that use the (semi-documented) <c>vararg</c>
calling convention (e.g. <c>__arglist</c> in C#) and that is not used for
interoperability (i.e. pinvoke to unmanaged code).
Using <c>params</c> (C#) can to achieve the same objective while <c>vararg</c>
is not CLS compliant. The later will limit the usability of the method to CLS
compliant language (e.g. Visual Basic does not support <c>vararg</c>.
</summary>
<example>
Bad example:
<code>
public void ShowItems_Bad (string header, __arglist)
{
Console.WriteLine (header);
ArgIterator args = new ArgIterator (__arglist);
for (int i = 0; i &lt; args.GetRemainingCount (); i++) {
Console.WriteLine (__refvalue (args.GetNextArg (), string));
}
}
</code></example>
<example>
Good example:
<code>
public void ShowItems (string header, params string [] items)
{
Console.WriteLine (header);
for (int i = 0; i &lt; items.Length; i++) {
Console.WriteLine (items [i]);
}
}
</code></example>
<example>
Good example (interoperability):
<code>
[DllImport ("libc.dll")]
static extern int printf (string format, __arglist);
</code></example>
<remarks>This rule is available since Gendarme 2.8</remarks>
</member>
<member name="T:Gendarme.Rules.BadPractice.PreferTryParseRule">
<summary>
This rule will warn you if a method use a <c>Parse</c> method when an
alternative <c>TryParse</c> method is available. A <c>Parser</c> method,
when using correctly, requires you to deal with multiple exceptions (a
complete list likely not easily available) or catching all exceptions (bad).
Also the throwing/catching of exceptions can kill performance.
The <c>TryParse</c> method allow simpler code without the performance penality.
</summary>
<example>
Bad example (no validation):
<code>
bool ParseLine (string line)
{
string values = line.Split (',');
if (values.Length == 3) {
id = Int32.Parse (values [0]);
timestamp = DateTime.Parse (values [1]);
msg = values [2];
return true;
} else {
return false;
}
}
</code></example>
<example>
Bad example (validation):
<code>
bool ParseLine (string line)
{
string values = line.Split (',');
if (values.Length == 3) {
try {
id = Int32.Parse (values [0]);
timestamp = DateTime.Parse (values [1]);
msg = values [2];
return true;
}
catch {
// catching all exception is bad
return false;
}
} else {
return false;
}
}
</code></example>
<example>
Good example:
<code>
bool ParseLine (string line)
{
string values = line.Split (',');
if (values.Length == 3) {
if (!Int32.TryParse (values [0], out id))
return false;
if (!DateTime.TryParse (values [1], out timestamp))
return false;
msg = values [2];
return true;
} else {
return false;
}
}
</code></example>
<remarks>This rule is available since Gendarme 2.8</remarks>
</member>
<member name="T:Gendarme.Rules.BadPractice.PreferSafeHandleRule">
<summary>
In general it is best to interop with native code using
<c>System.Runtime.InteropServices.SafeHandle</c> instead of
<c>System.IntPtr</c> or <c>System.UIntPtr</c> because:
<list type="bullet"><item><description>SafeHandles are type safe.</description></item><item><description>SafeHandles are guaranteed to be disposed of during
exceptional conditions like a thread aborting unexpectedly or a stack
overflow.</description></item><item><description>SafeHandles are not vulnerable to reycle attacks.</description></item><item><description>You don't need to write a finalizer which can be tricky
to do because they execute within their own thread, may execute on
partially constructed objects, and normally tear down the application
if you allow an exception to escape from them.</description></item></list></summary>
<example>
Bad example:
<code>
using System.Runtime.InteropServices;
using System.Security;
using System.Security.Permissions;
// If cleaning up the native resource in a timely manner is important you can
// implement IDisposable.
public sealed class Database {
~Database ()
{
// This will execute even if the ctor throws so it is important to check
// to see if the fields are initialized.
if (m_database != IntPtr.Zero) {
NativeMethods.sqlite3_close (m_database);
}
}
public Database (string path)
{
NativeMethods.OpenFlags flags = NativeMethods.OpenFlags.READWRITE | NativeMethods.OpenFlags.CREATE;
int err = NativeMethods.sqlite3_open_v2 (path, out m_database, flags, IntPtr.Zero);
// handle errors
}
// exec and query methods would go here
[SuppressUnmanagedCodeSecurity]
private static class NativeMethods {
[Flags]
public enum OpenFlags : int {
READONLY = 0x00000001,
READWRITE = 0x00000002,
CREATE = 0x00000004,
// ...
}
[DllImport ("sqlite3")]
public static extern int sqlite3_close (IntPtr db);
[DllImport ("sqlite3")]
public static extern int sqlite3_open_v2 (string fileName, out IntPtr db, OpenFlags flags, IntPtr module);
}
private IntPtr m_database;
}
</code></example>
<example>
Good example:
<code>
using System.Runtime.ConstrainedExecution;
using System.Runtime.InteropServices;
using System.Security;
using System.Security.Permissions;
// If cleaning up the native resource in a timely manner is important you can
// implement IDisposable, but you do not need to implement a finalizer because
// SafeHandle will take care of the cleanup.
internal sealed class Database {
public Database (string path)
{
NativeMethods.OpenFlags flags = NativeMethods.OpenFlags.READWRITE | NativeMethods.OpenFlags.CREATE;
m_database = new SqlitePtr (path, flags);
}
// exec and query methods would go here
// This corresponds to a native sqlite3*.
[SecurityPermission (SecurityAction.InheritanceDemand, UnmanagedCode = true)]
[SecurityPermission (SecurityAction.Demand, UnmanagedCode = true)]
private sealed class SqlitePtr : SafeHandle {
public SqlitePtr (string path, NativeMethods.OpenFlags flags) : base (IntPtr.Zero, true)
{
int err = NativeMethods.sqlite3_open_v2 (path, out handle, flags, IntPtr.Zero);
// handle errors
}
public override bool IsInvalid {
get {
return (handle == IntPtr.Zero);
}
}
// This will not be called if the handle is invalid. Note that this method should not throw.
[ReliabilityContract (Consistency.WillNotCorruptState, Cer.MayFail)]
protected override bool ReleaseHandle ()
{
NativeMethods.sqlite3_close (this);
return true;
}
}
[SuppressUnmanagedCodeSecurity]
private static class NativeMethods {
[Flags]
public enum OpenFlags : int {
READONLY = 0x00000001,
READWRITE = 0x00000002,
CREATE = 0x00000004,
// ...
}
[DllImport ("sqlite3")]
public static extern int sqlite3_close (SqlitePtr db);
// Open must take an IntPtr but all other methods take a type safe SqlitePtr.
[DllImport ("sqlite3")]
public static extern int sqlite3_open_v2 (string fileName, out IntPtr db, OpenFlags flags, IntPtr module);
}
private SqlitePtr m_database;
}
</code></example>
<remarks>This rule is available since Gendarme 2.6</remarks>
</member>
<member name="T:Gendarme.Rules.BadPractice.ReplaceIncompleteOddnessCheckRule">
<summary>
This rule checks for problematic oddness checks. Often this is done by comparing
a value modulo two (% 2) with one (1). However this will not work if the value is
negative because negative one will be returned. A better (and faster) approach is
to check the least significant bit of the integer.
</summary>
<example>
Bad example:
<code>
public bool IsOdd (int x)
{
// (x % 2) won't work for negative numbers (it returns -1)
return ((x % 2) == 1);
}
</code></example>
<example>
Good example:
<code>
public bool IsOdd (int x)
{
return ((x % 2) != 0);
}
</code></example>
<example>
Good example (faster):
<code>
public bool IsOdd (int x)
{
return ((x &amp; 1) == 1);
}
</code></example>
<remarks>This rule is available since Gendarme 2.0</remarks>
</member>
<member name="T:Gendarme.Rules.BadPractice.ToStringShouldNotReturnNullRule">
<summary>
This rule checks for overridden <c>ToString()</c> methods which return <c>null</c>.
An appropriately descriptive string, or <c>string.Empty</c>, should be returned
instead in order to make the value more useful (especially in debugging).
</summary>
<example>
Bad example:
<code>
public override string ToString ()
{
return (count == 0) ? null : count.ToString ();
}
</code></example>
<example>
Good example:
<code>
public override string ToString ()
{
return count.ToString ();
}
</code></example>
<remarks>Before Gendarme 2.4 this rule was named ToStringReturnsNull.</remarks>
</member>
<member name="T:Gendarme.Rules.BadPractice.UseFileOpenOnlyWithFileAccessRule">
<summary>
This rule checks that when file open method is called with FileMode parameter
it is also called with FileAccess (or FileSystemRights) parameter. It is needed
because default behaviour of file open methods when they are called only with
FileMode is to require read-write access while it is commonly expected that they
will require only read access.
</summary>
<example>
Bad example:
<code>
public void OpenFile ()
{
FileStream f = File.Open ("Filename.ext", FileMode.Open);
}
</code></example>
<example>
Good example:
<code>
public void OpenFile ()
{
FileStream f = File.Open ("Filename.ext", FileMode.Open, FileAccess.Read);
}
</code></example>
</member>
</members>
</doc>