Skip to content

Commit

Permalink
Re-enable CA1802 (use consts instead of readonly statics) (dotnet#39782)
Browse files Browse the repository at this point in the history 
* Re-enable CA1802 (use consts instead of readonly statics)

I enabled it only for privates and internals, as we've previously violated this in public surface area, and as everything goes through API review moving forward, didn't seem worthwhile fighting it and adding suppressions in various places.

* Address PR feedback
  • Loading branch information
@stephentoub
stephentoub authored Jul 26, 2019
1 parent 97d5957 commit 318b594
Show file tree
Hide file tree
Showing 55 changed files with 423 additions and 431 deletions.
3 changes: 3 additions & 0 deletions .editorconfig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,3 +158,6 @@ indent_size = 2
end_of_line = lf
[*.{cmd, bat}]
end_of_line = crlf

# Analyzers
dotnet_code_quality.ca1802.api_surface = private, internal
1 change: 0 additions & 1 deletion src/CodeAnalysis.ruleset
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
<Rule Id="CA1721" Action="None" /> <!-- Property names should not match get methods -->
<Rule Id="CA1724" Action="None" /> <!-- Type names should not match namespaces -->
<Rule Id="CA1801" Action="None" /> <!-- Review unused parameters -->
<Rule Id="CA1802" Action="None" /> <!-- Use literals where appropriate -->
<Rule Id="CA1806" Action="None" /> <!-- Do not ignore method results -->
<Rule Id="CA1810" Action="None" /> <!-- Initialize reference type static fields inline -->
<Rule Id="CA1812" Action="None" /> <!-- Avoid uninstantiated internal classes -->
Expand Down
2 changes: 1 addition & 1 deletion src/Common/src/CoreLib/System/Diagnostics/Tracing/EventSource.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2518,7 +2518,7 @@ private Type GetDataType(EventMetadata eventData, int parameterId)
return eventData.Parameters[parameterId].ParameterType;
}

private static readonly bool m_EventSourcePreventRecursion = false;
private const bool m_EventSourcePreventRecursion = false;
#else
private int GetParameterCount(EventMetadata eventData)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Common/src/System/Data/ProviderBase/TimeoutTimer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ internal void SetTimeoutSeconds(int seconds)
//-------------------

// Indicator for infinite timeout when starting a timer
internal static readonly long InfiniteTimeout = 0;
internal const long InfiniteTimeout = 0;

// Is this timer in an expired state?
internal bool IsExpired
Expand Down
38 changes: 17 additions & 21 deletions src/Common/src/System/Net/Mail/MailBnfHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,8 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Text;
using System.Net.Mail;
using System.Globalization;
using System.Collections.Generic;
using System.Diagnostics;
using System.Text;

namespace System.Net.Mime
{
Expand All @@ -31,22 +27,22 @@ internal static class MailBnfHelper
// characters allowed inside of comments
internal static readonly bool[] Ctext = CreateCharactersAllowedInComments();

internal static readonly int Ascii7bitMaxValue = 127;
internal static readonly char Quote = '\"';
internal static readonly char Space = ' ';
internal static readonly char Tab = '\t';
internal static readonly char CR = '\r';
internal static readonly char LF = '\n';
internal static readonly char StartComment = '(';
internal static readonly char EndComment = ')';
internal static readonly char Backslash = '\\';
internal static readonly char At = '@';
internal static readonly char EndAngleBracket = '>';
internal static readonly char StartAngleBracket = '<';
internal static readonly char StartSquareBracket = '[';
internal static readonly char EndSquareBracket = ']';
internal static readonly char Comma = ',';
internal static readonly char Dot = '.';
internal const int Ascii7bitMaxValue = 127;
internal const char Quote = '\"';
internal const char Space = ' ';
internal const char Tab = '\t';
internal const char CR = '\r';
internal const char LF = '\n';
internal const char StartComment = '(';
internal const char EndComment = ')';
internal const char Backslash = '\\';
internal const char At = '@';
internal const char EndAngleBracket = '>';
internal const char StartAngleBracket = '<';
internal const char StartSquareBracket = '[';
internal const char EndSquareBracket = ']';
internal const char Comma = ',';
internal const char Dot = '.';

private static readonly char[] s_colonSeparator = new char[] { ':' };

Expand Down
2 changes: 2 additions & 0 deletions src/Common/src/System/Net/SocketAddress.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ namespace System.Net.Internals
#endif
class SocketAddress
{
#pragma warning disable CA1802 // these could be const on Windows but need to be static readonly for Unix
internal static readonly int IPv6AddressSize = SocketAddressPal.IPv6AddressSize;
internal static readonly int IPv4AddressSize = SocketAddressPal.IPv4AddressSize;
#pragma warning restore CA1802

internal int InternalSize;
internal byte[] Buffer;
Expand Down
2 changes: 1 addition & 1 deletion ...ystem.Configuration.ConfigurationManager/src/System/Configuration/ConfigurationElement.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1163,7 +1163,7 @@ protected internal virtual bool SerializeElement(XmlWriter writer, bool serializ

ConfigurationElement elem = (ConfigurationElement)value;

if (prop.Name != ConfigurationProperty.s_defaultCollectionPropertyName)
if (prop.Name != ConfigurationProperty.DefaultCollectionPropertyName)
{
dataToWrite |= elem.SerializeToXmlElement(writer, prop.Name);
}
Expand Down
4 changes: 2 additions & 2 deletions ...stem.Configuration.ConfigurationManager/src/System/Configuration/ConfigurationProperty.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public sealed class ConfigurationProperty
{
internal static readonly ConfigurationValidatorBase s_nonEmptyStringValidator = new StringValidator(1);
private static readonly ConfigurationValidatorBase s_defaultValidatorInstance = new DefaultValidator();
internal static readonly string s_defaultCollectionPropertyName = "";
internal const string DefaultCollectionPropertyName = "";
private TypeConverter _converter;
private volatile bool _isConfigurationElementType;
private volatile bool _isTypeInited;
Expand Down Expand Up @@ -229,7 +229,7 @@ private void ConstructorInit(

if (((options & ConfigurationPropertyOptions.IsDefaultCollection) != 0) && string.IsNullOrEmpty(name))
{
name = s_defaultCollectionPropertyName;
name = DefaultCollectionPropertyName;
}
else
{
Expand Down
7 changes: 2 additions & 5 deletions ...iguration.ConfigurationManager/src/System/Configuration/ConfigurationPropertyAttribute.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,7 @@ namespace System.Configuration
[AttributeUsage(AttributeTargets.Property)]
public sealed class ConfigurationPropertyAttribute : Attribute
{
// disable csharp compiler warning #0414: field assigned unused value
#pragma warning disable 0414
internal static readonly string s_defaultCollectionPropertyName = "";
#pragma warning restore 0414
internal const string DefaultCollectionPropertyName = "";

public ConfigurationPropertyAttribute(string name)
{
Expand Down Expand Up @@ -54,4 +51,4 @@ public bool IsKey
}
}
}
}
}
2 changes: 1 addition & 1 deletion ...guration.ConfigurationManager/src/System/Configuration/ConfigurationPropertyCollection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public class ConfigurationPropertyCollection : ICollection
private readonly ArrayList _items = new ArrayList();

internal ConfigurationProperty DefaultCollectionProperty
=> this[ConfigurationProperty.s_defaultCollectionPropertyName];
=> this[ConfigurationProperty.DefaultCollectionPropertyName];

public ConfigurationProperty this[string name]
{
Expand Down
2 changes: 1 addition & 1 deletion ...figuration.ConfigurationManager/src/System/Configuration/PropertyInformationCollection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public PropertyInformation this[string propertyName]
if (result == null)
{
PropertyInformation defaultColl =
(PropertyInformation)BaseGet(ConfigurationProperty.s_defaultCollectionPropertyName);
(PropertyInformation)BaseGet(ConfigurationProperty.DefaultCollectionPropertyName);

if ((defaultColl != null) && (defaultColl.ProvidedName == propertyName)) result = defaultColl;
}
Expand Down
22 changes: 11 additions & 11 deletions src/System.Data.Common/src/System/Data/Common/DecimalStorage.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ namespace System.Data.Common
{
internal sealed class DecimalStorage : DataStorage
{
private static readonly decimal s_defaultValue = decimal.Zero;
private const decimal DefaultValue = decimal.Zero;

private decimal[] _values;

internal DecimalStorage(DataColumn column)
: base(column, typeof(decimal), s_defaultValue, StorageType.Decimal)
: base(column, typeof(decimal), DefaultValue, StorageType.Decimal)
{
}

Expand All @@ -26,7 +26,7 @@ public override object Aggregate(int[] records, AggregateType kind)
switch (kind)
{
case AggregateType.Sum:
decimal sum = s_defaultValue;
decimal sum = DefaultValue;
foreach (int record in records)
{
if (HasValue(record))
Expand All @@ -42,7 +42,7 @@ public override object Aggregate(int[] records, AggregateType kind)
return _nullValue;

case AggregateType.Mean:
decimal meanSum = s_defaultValue;
decimal meanSum = DefaultValue;
int meanCount = 0;
foreach (int record in records)
{
Expand All @@ -64,10 +64,10 @@ public override object Aggregate(int[] records, AggregateType kind)
case AggregateType.Var:
case AggregateType.StDev:
int count = 0;
double var = (double)s_defaultValue;
double prec = (double)s_defaultValue;
double dsum = (double)s_defaultValue;
double sqrsum = (double)s_defaultValue;
double var = (double)DefaultValue;
double prec = (double)DefaultValue;
double dsum = (double)DefaultValue;
double sqrsum = (double)DefaultValue;

foreach (int record in records)
{
Expand Down Expand Up @@ -158,7 +158,7 @@ public override int Compare(int recordNo1, int recordNo2)
decimal valueNo1 = _values[recordNo1];
decimal valueNo2 = _values[recordNo2];

if (valueNo1 == s_defaultValue || valueNo2 == s_defaultValue)
if (valueNo1 == DefaultValue || valueNo2 == DefaultValue)
{
int bitCheck = CompareBits(recordNo1, recordNo2);
if (0 != bitCheck)
Expand All @@ -178,7 +178,7 @@ public override int CompareValueTo(int recordNo, object value)
}

decimal valueNo1 = _values[recordNo];
if ((s_defaultValue == valueNo1) && !HasValue(recordNo))
if ((DefaultValue == valueNo1) && !HasValue(recordNo))
{
return -1;
}
Expand Down Expand Up @@ -217,7 +217,7 @@ public override void Set(int record, object value)
System.Diagnostics.Debug.Assert(null != value, "null value");
if (_nullValue == value)
{
_values[record] = s_defaultValue;
_values[record] = DefaultValue;
SetNullBit(record, true);
}
else
Expand Down
16 changes: 8 additions & 8 deletions src/System.Data.Common/src/System/Data/Common/UInt16Storage.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ namespace System.Data.Common
{
internal sealed class UInt16Storage : DataStorage
{
private static readonly ushort s_defaultValue = ushort.MinValue;
private const ushort DefaultValue = ushort.MinValue;

private ushort[] _values;

public UInt16Storage(DataColumn column)
: base(column, typeof(ushort), s_defaultValue, StorageType.UInt16)
: base(column, typeof(ushort), DefaultValue, StorageType.UInt16)
{
}

Expand All @@ -26,7 +26,7 @@ public override object Aggregate(int[] records, AggregateType kind)
switch (kind)
{
case AggregateType.Sum:
ulong sum = s_defaultValue;
ulong sum = DefaultValue;
foreach (int record in records)
{
if (HasValue(record))
Expand All @@ -42,7 +42,7 @@ public override object Aggregate(int[] records, AggregateType kind)
return _nullValue;

case AggregateType.Mean:
long meanSum = s_defaultValue;
long meanSum = DefaultValue;
int meanCount = 0;
foreach (int record in records)
{
Expand Down Expand Up @@ -166,7 +166,7 @@ public override int Compare(int recordNo1, int recordNo2)
ushort valueNo1 = _values[recordNo1];
ushort valueNo2 = _values[recordNo2];

if (valueNo1 == s_defaultValue || valueNo2 == s_defaultValue)
if (valueNo1 == DefaultValue || valueNo2 == DefaultValue)
{
int bitCheck = CompareBits(recordNo1, recordNo2);
if (0 != bitCheck)
Expand All @@ -189,7 +189,7 @@ public override int CompareValueTo(int recordNo, object value)
}

ushort valueNo1 = _values[recordNo];
if ((s_defaultValue == valueNo1) && !HasValue(recordNo))
if ((DefaultValue == valueNo1) && !HasValue(recordNo))
{
return -1;
}
Expand Down Expand Up @@ -222,7 +222,7 @@ public override void Copy(int recordNo1, int recordNo2)
public override object Get(int record)
{
ushort value = _values[record];
if (!value.Equals(s_defaultValue))
if (!value.Equals(DefaultValue))
{
return value;
}
Expand All @@ -234,7 +234,7 @@ public override void Set(int record, object value)
System.Diagnostics.Debug.Assert(null != value, "null value");
if (_nullValue == value)
{
_values[record] = s_defaultValue;
_values[record] = DefaultValue;
SetNullBit(record, true);
}
else
Expand Down
16 changes: 8 additions & 8 deletions src/System.Data.Common/src/System/Data/Common/UInt32Storage.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ namespace System.Data.Common
{
internal sealed class UInt32Storage : DataStorage
{
private static readonly uint s_defaultValue = uint.MinValue;
private const uint DefaultValue = uint.MinValue;

private uint[] _values;

public UInt32Storage(DataColumn column)
: base(column, typeof(uint), s_defaultValue, StorageType.UInt32)
: base(column, typeof(uint), DefaultValue, StorageType.UInt32)
{
}

Expand All @@ -26,7 +26,7 @@ public override object Aggregate(int[] records, AggregateType kind)
switch (kind)
{
case AggregateType.Sum:
ulong sum = s_defaultValue;
ulong sum = DefaultValue;
foreach (int record in records)
{
if (HasValue(record))
Expand All @@ -42,7 +42,7 @@ public override object Aggregate(int[] records, AggregateType kind)
return _nullValue;

case AggregateType.Mean:
long meanSum = s_defaultValue;
long meanSum = DefaultValue;
int meanCount = 0;
foreach (int record in records)
{
Expand Down Expand Up @@ -165,7 +165,7 @@ public override int Compare(int recordNo1, int recordNo2)
uint valueNo1 = _values[recordNo1];
uint valueNo2 = _values[recordNo2];

if (valueNo1 == s_defaultValue || valueNo2 == s_defaultValue)
if (valueNo1 == DefaultValue || valueNo2 == DefaultValue)
{
int bitCheck = CompareBits(recordNo1, recordNo2);
if (0 != bitCheck)
Expand All @@ -188,7 +188,7 @@ public override int CompareValueTo(int recordNo, object value)
}

uint valueNo1 = _values[recordNo];
if ((s_defaultValue == valueNo1) && !HasValue(recordNo))
if ((DefaultValue == valueNo1) && !HasValue(recordNo))
{
return -1;
}
Expand Down Expand Up @@ -221,7 +221,7 @@ public override void Copy(int recordNo1, int recordNo2)
public override object Get(int record)
{
uint value = _values[record];
if (!value.Equals(s_defaultValue))
if (!value.Equals(DefaultValue))
{
return value;
}
Expand All @@ -233,7 +233,7 @@ public override void Set(int record, object value)
System.Diagnostics.Debug.Assert(null != value, "null value");
if (_nullValue == value)
{
_values[record] = s_defaultValue;
_values[record] = DefaultValue;
SetNullBit(record, true);
}
else
Expand Down
Loading

0 comments on commit 318b594

Please sign in to comment.