Add ExcludeFromCodeCoverageAttribute.Justification property (dotnet#3…

…8520) * Add ExcludeFromCodeCoverage.Justification property Delete some ExcludeFromCodeCoverages that aren't warranted, and add justifications almost everywhere else (only not doing so where a project builds for targets without the member). * Update src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Semantics/Types/PointerType.cs Co-authored-by: Levi Broderick <[email protected]> Co-authored-by: Levi Broderick <[email protected]>
garora · Jul 1, 2020 · 237eb2f · 237eb2f
1 parent c5596cf
commit 237eb2f
Show file tree

Hide file tree

Showing 57 changed files with 112 additions and 176 deletions.
diff --git a/src/libraries/Common/src/System/Diagnostics/CodeAnalysis/ExcludeFromCodeCoverageAttribute.cs b/src/libraries/Common/src/System/Diagnostics/CodeAnalysis/ExcludeFromCodeCoverageAttribute.cs
@@ -12,7 +12,9 @@ namespace System.Diagnostics.CodeAnalysis
 #endif
     sealed class ExcludeFromCodeCoverageAttribute : Attribute
     {
-        public ExcludeFromCodeCoverageAttribute()
-        { }
+        public ExcludeFromCodeCoverageAttribute() { }
+
+        /// <summary>Gets or sets the justification for excluding the member from code coverage.</summary>
+        public string? Justification { get; set; }
     }
 }
diff --git a/...raries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/CSharpBinaryOperationBinder.cs b/...raries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/CSharpBinaryOperationBinder.cs
@@ -19,7 +19,7 @@ namespace Microsoft.CSharp.RuntimeBinder
     /// </summary>
     internal sealed class CSharpBinaryOperationBinder : BinaryOperationBinder, ICSharpBinder
     {
-        [ExcludeFromCodeCoverage]
+        [ExcludeFromCodeCoverage(Justification = "Name should not be called for this binder")]
         public string Name
         {
             get

diff --git a/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/CSharpConvertBinder.cs b/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/CSharpConvertBinder.cs
@@ -18,7 +18,7 @@ namespace Microsoft.CSharp.RuntimeBinder
     /// </summary>
     internal sealed class CSharpConvertBinder : ConvertBinder, ICSharpBinder
     {
-        [ExcludeFromCodeCoverage]
+        [ExcludeFromCodeCoverage(Justification = "Name should not be called for this binder")]
         public string Name
         {
             get

diff --git a/...braries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/CSharpUnaryOperationBinder.cs b/...braries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/CSharpUnaryOperationBinder.cs
@@ -19,7 +19,7 @@ namespace Microsoft.CSharp.RuntimeBinder
     /// </summary>
     internal sealed class CSharpUnaryOperationBinder : UnaryOperationBinder, ICSharpBinder
     {
-        [ExcludeFromCodeCoverage]
+        [ExcludeFromCodeCoverage(Justification = "Name should not be called for this binder")]
         public string Name
         {
             get

diff --git a/...ries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Semantics/Symbols/SymbolStore.cs b/...ries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Semantics/Symbols/SymbolStore.cs
@@ -90,7 +90,7 @@ public Key(Name name, ParentSymbol parent)
             public bool Equals(Key other) => _name == other._name && _parent == other._parent;
 
 #if  DEBUG
-            [ExcludeFromCodeCoverage] // Typed overload should always be the method called.
+            [ExcludeFromCodeCoverage(Justification = "Typed overload should always be the method called")]
 #endif
             public override bool Equals(object obj)
             {

diff --git a/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Semantics/Tree/EXPR.cs b/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Semantics/Tree/EXPR.cs
@@ -39,7 +39,7 @@ public CType Type
             protected set { _type = value; }
         }
 
-        [ExcludeFromCodeCoverage] // Should only be called through override.
+        [ExcludeFromCodeCoverage(Justification = "Should only be called through override")]
         public virtual object Object
         {
             get

diff --git a/...aries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Semantics/Types/NullableType.cs b/...aries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Semantics/Types/NullableType.cs
@@ -51,7 +51,7 @@ public override CType StripNubs(out bool wasNullable)
 
         public override FUNDTYPE FundamentalType => FUNDTYPE.FT_STRUCT;
 
-        [ExcludeFromCodeCoverage] // Should be unreachable. Overload exists just to catch it being hit during debug.
+        [ExcludeFromCodeCoverage(Justification = "Should be unreachable. Overload exists just to catch it being hit during debug.")]
         public override ConstValKind ConstValKind
         {
             get

diff --git a/...raries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Semantics/Types/PointerType.cs b/...raries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Semantics/Types/PointerType.cs
@@ -26,7 +26,7 @@ public PointerType(CType referentType)
 
         public override FUNDTYPE FundamentalType => FUNDTYPE.FT_PTR;
 
-        [ExcludeFromCodeCoverage] // Technically correct, but we can't have constant pointers in dynamically dispatched code.
+        [ExcludeFromCodeCoverage(Justification = "Dynamic code can't contain constant pointers")]
         public override ConstValKind ConstValKind
         {
             get

diff --git a/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Semantics/Types/Type.cs b/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Semantics/Types/Type.cs
@@ -17,7 +17,7 @@ private protected CType(TypeKind kind)
             TypeKind = kind;
         }
 
-        [ExcludeFromCodeCoverage] // Should only be called through override.
+        [ExcludeFromCodeCoverage(Justification = "Should only be called through override")]
         public virtual Type AssociatedSystemType => throw Error.InternalCompilerError();
 
         public TypeKind TypeKind { get; }
@@ -99,7 +99,7 @@ public virtual CType StripNubs(out bool wasNullable)
 
         public virtual bool IsClassType => false;
 
-        [ExcludeFromCodeCoverage] // Should only be called through override.
+        [ExcludeFromCodeCoverage(Justification = "Should only be called through override")]
         public virtual AggregateType UnderlyingEnumType => throw Error.InternalCompilerError();
 
         // Pointer types (or arrays of them) are the only unsafe types.
@@ -110,7 +110,7 @@ public virtual CType StripNubs(out bool wasNullable)
 
         public virtual bool IsPredefined => false;
 
-        [ExcludeFromCodeCoverage] // Should only be called through override.
+        [ExcludeFromCodeCoverage(Justification = "Should only be called through override")]
         public virtual PredefinedType PredefinedType => throw Error.InternalCompilerError();
 
         public virtual bool IsStaticClass => false;
@@ -122,7 +122,7 @@ public virtual CType StripNubs(out bool wasNullable)
 
         public virtual bool IsReferenceType => false;
 
-        [ExcludeFromCodeCoverage] // Should only be called through override.
+        [ExcludeFromCodeCoverage(Justification = "Should only be called through override")]
         public virtual AggregateType GetAts()
         {
             Debug.Fail("Bad type for AsAggregateType");

diff --git a/...ibraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Semantics/Types/TypeArray.cs b/...ibraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Semantics/Types/TypeArray.cs
@@ -68,7 +68,7 @@ public bool Equals(TypeArrayKey other)
                 return true;
             }
 
-            [ExcludeFromCodeCoverage] // Typed overload should always be the method called.
+            [ExcludeFromCodeCoverage(Justification = "Typed overload should always be the method called")]
             public override bool Equals(object obj)
             {
                 Debug.Fail("Sub-optimal overload called. Check if this can be avoided.");

diff --git a/...ibraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Semantics/Types/TypeTable.cs b/...ibraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Semantics/Types/TypeTable.cs
@@ -27,7 +27,7 @@ public bool Equals(KeyPair<TKey1, TKey2> other) =>
                 && EqualityComparer<TKey2>.Default.Equals(_pKey2, other._pKey2);
 
 #if DEBUG
-            [ExcludeFromCodeCoverage] // Typed overload should always be the method called.
+            [ExcludeFromCodeCoverage(Justification = "Typed overload should always be the method called")]
 #endif
             public override bool Equals(object obj)
             {

diff --git a/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Semantics/WithType.cs b/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Semantics/WithType.cs
@@ -77,7 +77,7 @@ public Symbol Sym
 
         public static bool operator !=(SymWithType swt1, SymWithType swt2) => !(swt1 == swt2);
 
-        [ExcludeFromCodeCoverage] // == overload should always be the method called.
+        [ExcludeFromCodeCoverage(Justification = "== overload should always be the method called")]
         public override bool Equals(object obj)
         {
             Debug.Fail("Sub-optimal equality called. Check if this is correct.");
@@ -86,7 +86,7 @@ public override bool Equals(object obj)
             return Sym == other.Sym && Ats == other.Ats;
         }
 
-        [ExcludeFromCodeCoverage] // Never used as a key.
+        [ExcludeFromCodeCoverage(Justification = "Never used as a key")]
         public override int GetHashCode()
         {
             Debug.Fail("If using this as a key, implement IEquatable<SymWithType>");

diff --git a/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/SymbolTable.cs b/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/SymbolTable.cs
@@ -35,7 +35,7 @@ public NameHashKey(Type type, string name)
             public bool Equals(NameHashKey other) => Type.Equals(other.Type) && Name.Equals(other.Name);
 
 #if  DEBUG
-            [ExcludeFromCodeCoverage] // Typed overload should always be the method called.
+            [ExcludeFromCodeCoverage(Justification = "Typed overload should always be the method called")]
 #endif
             public override bool Equals(object obj)
             {

diff --git a/...braries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableArray_1.cs b/...braries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableArray_1.cs
@@ -823,7 +823,6 @@ bool ICollection<T>.Remove(T item)
         /// <summary>
         /// See <see cref="IImmutableList{T}"/>
         /// </summary>
-        [ExcludeFromCodeCoverage]
         IImmutableList<T> IImmutableList<T>.Clear()
         {
             var self = this;
@@ -834,7 +833,6 @@ IImmutableList<T> IImmutableList<T>.Clear()
         /// <summary>
         /// See <see cref="IImmutableList{T}"/>
         /// </summary>
-        [ExcludeFromCodeCoverage]
         IImmutableList<T> IImmutableList<T>.Add(T value)
         {
             var self = this;
@@ -845,7 +843,6 @@ IImmutableList<T> IImmutableList<T>.Add(T value)
         /// <summary>
         /// See <see cref="IImmutableList{T}"/>
         /// </summary>
-        [ExcludeFromCodeCoverage]
         IImmutableList<T> IImmutableList<T>.AddRange(IEnumerable<T> items)
         {
             var self = this;
@@ -856,7 +853,6 @@ IImmutableList<T> IImmutableList<T>.AddRange(IEnumerable<T> items)
         /// <summary>
         /// See <see cref="IImmutableList{T}"/>
         /// </summary>
-        [ExcludeFromCodeCoverage]
         IImmutableList<T> IImmutableList<T>.Insert(int index, T element)
         {
             var self = this;
@@ -867,7 +863,6 @@ IImmutableList<T> IImmutableList<T>.Insert(int index, T element)
         /// <summary>
         /// See <see cref="IImmutableList{T}"/>
         /// </summary>
-        [ExcludeFromCodeCoverage]
         IImmutableList<T> IImmutableList<T>.InsertRange(int index, IEnumerable<T> items)
         {
             var self = this;
@@ -878,7 +873,6 @@ IImmutableList<T> IImmutableList<T>.InsertRange(int index, IEnumerable<T> items)
         /// <summary>
         /// See <see cref="IImmutableList{T}"/>
         /// </summary>
-        [ExcludeFromCodeCoverage]
         IImmutableList<T> IImmutableList<T>.Remove(T value, IEqualityComparer<T>? equalityComparer)
         {
             var self = this;
@@ -889,7 +883,6 @@ IImmutableList<T> IImmutableList<T>.Remove(T value, IEqualityComparer<T>? equali
         /// <summary>
         /// See <see cref="IImmutableList{T}"/>
         /// </summary>
-        [ExcludeFromCodeCoverage]
         IImmutableList<T> IImmutableList<T>.RemoveAll(Predicate<T> match)
         {
             var self = this;
@@ -900,7 +893,6 @@ IImmutableList<T> IImmutableList<T>.RemoveAll(Predicate<T> match)
         /// <summary>
         /// See <see cref="IImmutableList{T}"/>
         /// </summary>
-        [ExcludeFromCodeCoverage]
         IImmutableList<T> IImmutableList<T>.RemoveRange(IEnumerable<T> items, IEqualityComparer<T>? equalityComparer)
         {
             var self = this;
@@ -911,7 +903,6 @@ IImmutableList<T> IImmutableList<T>.RemoveRange(IEnumerable<T> items, IEqualityC
         /// <summary>
         /// See <see cref="IImmutableList{T}"/>
         /// </summary>
-        [ExcludeFromCodeCoverage]
         IImmutableList<T> IImmutableList<T>.RemoveRange(int index, int count)
         {
             var self = this;
@@ -922,7 +913,6 @@ IImmutableList<T> IImmutableList<T>.RemoveRange(int index, int count)
         /// <summary>
         /// See <see cref="IImmutableList{T}"/>
         /// </summary>
-        [ExcludeFromCodeCoverage]
         IImmutableList<T> IImmutableList<T>.RemoveAt(int index)
         {
             var self = this;
@@ -933,7 +923,6 @@ IImmutableList<T> IImmutableList<T>.RemoveAt(int index)
         /// <summary>
         /// See <see cref="IImmutableList{T}"/>
         /// </summary>
-        [ExcludeFromCodeCoverage]
         IImmutableList<T> IImmutableList<T>.SetItem(int index, T value)
         {
             var self = this;
@@ -959,7 +948,6 @@ IImmutableList<T> IImmutableList<T>.Replace(T oldValue, T newValue, IEqualityCom
         /// The position into which the new element was inserted, or -1 to indicate that the item was not inserted into the collection,
         /// </returns>
         /// <exception cref="System.NotSupportedException"></exception>
-        [ExcludeFromCodeCoverage]
         int IList.Add(object? value)
         {
             throw new NotSupportedException();
@@ -969,7 +957,6 @@ int IList.Add(object? value)
         /// Removes all items from the <see cref="ICollection{T}"/>.
         /// </summary>
         /// <exception cref="System.NotSupportedException"></exception>
-        [ExcludeFromCodeCoverage]
         void IList.Clear()
         {
             throw new NotSupportedException();
@@ -982,7 +969,6 @@ void IList.Clear()
         /// <returns>
         /// true if the <see cref="object"/> is found in the <see cref="IList"/>; otherwise, false.
         /// </returns>
-        [ExcludeFromCodeCoverage]
         bool IList.Contains(object? value)
         {
             var self = this;
@@ -997,7 +983,6 @@ bool IList.Contains(object? value)
         /// <returns>
         /// The index of <paramref name="value"/> if found in the list; otherwise, -1.
         /// </returns>
-        [ExcludeFromCodeCoverage]
         int IList.IndexOf(object? value)
         {
             var self = this;
@@ -1011,7 +996,6 @@ int IList.IndexOf(object? value)
         /// <param name="index">The zero-based index at which <paramref name="value"/> should be inserted.</param>
         /// <param name="value">The object to insert into the <see cref="IList"/>.</param>
         /// <exception cref="System.NotSupportedException"></exception>
-        [ExcludeFromCodeCoverage]
         void IList.Insert(int index, object? value)
         {
             throw new NotSupportedException();
@@ -1023,7 +1007,6 @@ void IList.Insert(int index, object? value)
         /// <value>
         /// <c>true</c> if this instance is fixed size; otherwise, <c>false</c>.
         /// </value>
-        [ExcludeFromCodeCoverage]
         [DebuggerBrowsable(DebuggerBrowsableState.Never)]
         bool IList.IsFixedSize
         {
@@ -1036,7 +1019,6 @@ bool IList.IsFixedSize
         /// <value>
         /// <c>true</c> if this instance is read only; otherwise, <c>false</c>.
         /// </value>
-        [ExcludeFromCodeCoverage]
         [DebuggerBrowsable(DebuggerBrowsableState.Never)]
         bool IList.IsReadOnly
         {
@@ -1047,7 +1029,6 @@ bool IList.IsReadOnly
         /// Gets the size of the array.
         /// </summary>
         /// <exception cref="InvalidOperationException">Thrown if the <see cref="IsDefault"/> property returns true.</exception>
-        [ExcludeFromCodeCoverage]
         [DebuggerBrowsable(DebuggerBrowsableState.Never)]
         int ICollection.Count
         {
@@ -1062,7 +1043,6 @@ int ICollection.Count
         /// <summary>
         /// See the <see cref="ICollection"/> interface.
         /// </summary>
-        [ExcludeFromCodeCoverage]
         [DebuggerBrowsable(DebuggerBrowsableState.Never)]
         bool ICollection.IsSynchronized
         {
@@ -1076,7 +1056,6 @@ bool ICollection.IsSynchronized
         /// <summary>
         /// Gets the sync root.
         /// </summary>
-        [ExcludeFromCodeCoverage]
         [DebuggerBrowsable(DebuggerBrowsableState.Never)]
         object ICollection.SyncRoot
         {
@@ -1088,7 +1067,6 @@ object ICollection.SyncRoot
         /// </summary>
         /// <param name="value">The object to remove from the <see cref="IList"/>.</param>
         /// <exception cref="System.NotSupportedException"></exception>
-        [ExcludeFromCodeCoverage]
         void IList.Remove(object? value)
         {
             throw new NotSupportedException();
@@ -1099,7 +1077,6 @@ void IList.Remove(object? value)
         /// </summary>
         /// <param name="index">The zero-based index of the item to remove.</param>
         /// <exception cref="System.NotSupportedException"></exception>
-        [ExcludeFromCodeCoverage]
         void IList.RemoveAt(int index)
         {
             throw new NotSupportedException();
@@ -1115,7 +1092,6 @@ void IList.RemoveAt(int index)
         /// <returns></returns>
         /// <exception cref="NotSupportedException">Always thrown from the setter.</exception>
         /// <exception cref="InvalidOperationException">Thrown if the <see cref="IsDefault"/> property returns true.</exception>
-        [ExcludeFromCodeCoverage]
         object? IList.this[int index]
         {
             get
@@ -1132,7 +1108,6 @@ void IList.RemoveAt(int index)
         /// </summary>
         /// <param name="array">The one-dimensional <see cref="Array"/> that is the destination of the elements copied from <see cref="ICollection"/>. The <see cref="Array"/> must have zero-based indexing.</param>
         /// <param name="index">The zero-based index in <paramref name="array"/> at which copying begins.</param>
-        [ExcludeFromCodeCoverage]
         void ICollection.CopyTo(Array array, int index)
         {
             var self = this;