Add performance and code analysis fixes

Author: Chris Martinez

src/Common/CollectionExtensions.cs

@@ -18,10 +18,10 @@ namespace Microsoft.AspNetCore.Mvc
 
     static partial class CollectionExtensions
     {
-#if NETAPPCORE3_1
-        internal static bool TryGetValue<TKey, TValue>( this IDictionary<TKey, object?> dictionary, TKey key, [NotNullWhen(true)] out TValue value ) where TKey : notnull
-#else
+#if WEBAPI
         internal static bool TryGetValue<TKey, TValue>( this IDictionary<TKey, object?> dictionary, TKey key, out TValue value ) where TKey : notnull
+#else
+        internal static bool TryGetValue<TKey, TValue>( this IDictionary<TKey, object?> dictionary, TKey key, [NotNullWhen( true )] out TValue value ) where TKey : notnull
 #endif
         {
             if ( dictionary.TryGetValue( key, out var val ) && val is TValue v )
@@ -34,7 +34,7 @@ internal static bool TryGetValue<TKey, TValue>( this IDictionary<TKey, object?>
             return false;
         }
 
-        internal static List<T> AsList<T>( this IEnumerable<T> sequence ) => ( sequence as List<T> ) ?? new List<T>( sequence );
+        internal static List<T> AsList<T>( this IEnumerable<T> sequence ) => ( sequence as List<T> ) ?? sequence.ToList();
 
         internal static IReadOnlyList<T> ToSortedReadOnlyList<T>( this IEnumerable<T> sequence ) where T : IComparable<T>
         {
@@ -59,15 +59,18 @@ internal static void AddRange<T>( this ICollection<T> collection, IEnumerable<T>
 
         internal static string? EnsureZeroOrOneApiVersions( this ICollection<string> apiVersions )
         {
-            if ( apiVersions.Count < 2 )
+            switch ( apiVersions.Count )
             {
-                return apiVersions.SingleOrDefault();
+                case 0:
+                    return default;
+                case 1:
+                    var values = new string[1];
+                    apiVersions.CopyTo( values, 0 );
+                    return values[0];
+                default:
+                    var message = Format( InvariantCulture, SR.MultipleDifferentApiVersionsRequested, Join( ", ", apiVersions ) );
+                    throw new AmbiguousApiVersionException( message, apiVersions );
             }
-
-            var requestedVersions = Join( ", ", apiVersions.OrderBy( v => v ) );
-            var message = Format( InvariantCulture, SR.MultipleDifferentApiVersionsRequested, requestedVersions );
-
-            throw new AmbiguousApiVersionException( message, apiVersions.OrderBy( v => v ) );
         }
 
         internal static void UnionWith<T>( this ICollection<T> collection, IEnumerable<T> other )

src/Common/Versioning/AmbiguousApiVersionException.cs

@@ -4,27 +4,40 @@ namespace Microsoft.Web.Http.Versioning
 namespace Microsoft.AspNetCore.Mvc.Versioning
 #endif
 {
-    // disable warnings for false positives targeting netstandard2.0
-#pragma warning disable CA1032 // Implement standard exception constructors
-#pragma warning disable CA2235 // Mark all non-serializable fields
-
     using System;
     using System.Collections.Generic;
     using System.Linq;
-#if NET451 || WEBAPI
+    using System.Runtime.CompilerServices;
     using System.Runtime.Serialization;
-#endif
+    using static System.Runtime.CompilerServices.MethodImplOptions;
 
     /// <summary>
     /// Represents the exception thrown when multiple, different API versions specified in a single request.
     /// </summary>
-#if NET451 || WEBAPI
     [Serializable]
-#endif
     public class AmbiguousApiVersionException : Exception
     {
         readonly string[] apiVersions;
 
+        /// <summary>
+        /// Initializes a new instance of the <see cref="AmbiguousApiVersionException"/> class.
+        /// </summary>
+        public AmbiguousApiVersionException() => apiVersions = EmptyArray();
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="AmbiguousApiVersionException"/> class.
+        /// </summary>
+        /// <param name="message">The associated error message.</param>
+        public AmbiguousApiVersionException( string message ) : base( message ) => apiVersions = EmptyArray();
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="AmbiguousApiVersionException"/> class.
+        /// </summary>
+        /// <param name="message">The associated error message.</param>
+        /// <param name="innerException">The inner <see cref="Exception">exception</see> that caused the current exception, if any.</param>
+        public AmbiguousApiVersionException( string message, Exception innerException )
+            : base( message, innerException ) => apiVersions = EmptyArray();
+
         /// <summary>
         /// Initializes a new instance of the <see cref="AmbiguousApiVersionException"/> class.
         /// </summary>
@@ -47,14 +60,14 @@ public AmbiguousApiVersionException( string message, IEnumerable<string> apiVers
         /// </summary>
         /// <value>A <see cref="IReadOnlyList{T}">read-only list</see> of unparsed, ambiguous API versions.</value>
         public IReadOnlyList<string> ApiVersions => apiVersions;
-#if NET451 || WEBAPI
+
         /// <summary>
         /// Initializes a new instance of the <see cref="AmbiguousApiVersionException"/> class.
         /// </summary>
         /// <param name="info">The <see cref="SerializationInfo">serialization info</see> the exception is being deserialized with.</param>
         /// <param name="context">The <see cref="StreamingContext">streaming context</see> the exception is being deserialized from.</param>
         protected AmbiguousApiVersionException( SerializationInfo info, StreamingContext context )
-            : base( info, context ) => apiVersions = (string[]) info.GetValue( nameof( apiVersions ), typeof( string[] ) );
+            : base( info, context ) => apiVersions = (string[]) info.GetValue( nameof( apiVersions ), typeof( string[] ) )!;
 
         /// <summary>
         /// Gets information about the exception being serialized.
@@ -66,6 +79,12 @@ public override void GetObjectData( SerializationInfo info, StreamingContext con
             base.GetObjectData( info, context );
             info.AddValue( nameof( apiVersions ), apiVersions );
         }
+
+        [MethodImpl( AggressiveInlining )]
+#if WEBAPI
+        static string[] EmptyArray() => new string[0];
+#else
+        static string[] EmptyArray() => Array.Empty<string>();
 #endif
     }
 }

src/Common/Versioning/Conventions/ActionApiVersionConventionBuilderCollection.cs

@@ -59,10 +59,10 @@ protected internal virtual ActionApiVersionConventionBuilder GetOrAdd( MethodInf
         /// <param name="actionMethod">The controller action method to get the convention builder for.</param>
         /// <param name="actionBuilder">The <see cref="ActionApiVersionConventionBuilder">controller action convention builder</see> or <c>null</c>.</param>
         /// <returns>True if the <paramref name="actionBuilder">action builder</paramref> is successfully retrieved; otherwise, false.</returns>
-#if NETCOREAPP3_1
-        public virtual bool TryGetValue( MethodInfo? actionMethod, [NotNullWhen( true )] out ActionApiVersionConventionBuilder? actionBuilder )
-#else
+#if WEBAPI
         public virtual bool TryGetValue( MethodInfo? actionMethod, out ActionApiVersionConventionBuilder? actionBuilder )
+#else
+        public virtual bool TryGetValue( MethodInfo? actionMethod, [NotNullWhen( true )] out ActionApiVersionConventionBuilder? actionBuilder )
 #endif
         {
             if ( actionMethod == null )

src/Common/Versioning/Conventions/ActionApiVersionConventionBuilderCollection{T}.cs

@@ -58,10 +58,10 @@ protected internal virtual ActionApiVersionConventionBuilder<T> GetOrAdd( Method
         /// <param name="actionMethod">The controller action method to get the convention builder for.</param>
         /// <param name="actionBuilder">The <see cref="ActionApiVersionConventionBuilder{T}">controller action convention builder</see> or <c>null</c>.</param>
         /// <returns>True if the <paramref name="actionBuilder">action builder</paramref> is successfully retrieved; otherwise, false.</returns>
-#if NETCOREAPP3_1
-        public virtual bool TryGetValue( MethodInfo? actionMethod, [NotNullWhen( true )] out ActionApiVersionConventionBuilder<T>? actionBuilder )
-#else
+#if WEBAPI
         public virtual bool TryGetValue( MethodInfo? actionMethod, out ActionApiVersionConventionBuilder<T>? actionBuilder )
+#else
+        public virtual bool TryGetValue( MethodInfo? actionMethod, [NotNullWhen( true )] out ActionApiVersionConventionBuilder<T>? actionBuilder )
 #endif
         {
             if ( actionMethod == null )

src/Common/Versioning/HeaderApiVersionReader.cs

@@ -50,9 +50,14 @@ public virtual void AddParameters( IApiVersionParameterDescriptionContext contex
                 throw new ArgumentNullException( nameof( context ) );
             }
 
-            foreach ( var name in HeaderNames )
+            var count = HeaderNames.Count;
+            var names = new string[count];
+
+            HeaderNames.CopyTo( names, 0 );
+
+            for ( var i = 0; i < count; i++ )
             {
-                context.AddParameter( name, Header );
+                context.AddParameter( names[i], Header );
             }
         }
     }

src/Common/Versioning/MediaTypeApiVersionReader.cs

@@ -5,6 +5,7 @@ namespace Microsoft.AspNetCore.Mvc.Versioning
 #endif
 {
 #if WEBAPI
+    using Microsoft.Web.Http;
     using Microsoft.Web.Http.Routing;
 #else
     using Microsoft.AspNetCore.Mvc.Routing;
@@ -20,6 +21,7 @@ namespace Microsoft.AspNetCore.Mvc.Versioning
     using MediaTypeWithQualityHeaderValue = Microsoft.Net.Http.Headers.MediaTypeHeaderValue;
 #endif
     using static ApiVersionParameterLocation;
+    using static System.StringComparison;
 
     /// <summary>
     /// Represents a service API version reader that reads the value from a media type HTTP header in the request.
@@ -47,34 +49,48 @@ public partial class MediaTypeApiVersionReader : IApiVersionReader
         /// <summary>
         /// Reads the requested API version from the HTTP Accept header.
         /// </summary>
-        /// <param name="accept">The <see cref="IEnumerable{T}">sequence</see> of Accept
+        /// <param name="accept">The <see cref="ICollection{T}">collection</see> of Accept
         /// <see cref="MediaTypeWithQualityHeaderValue">headers</see> to read from.</param>
         /// <returns>The API version read or <c>null</c>.</returns>
         /// <remarks>The default implementation will return the first defined API version ranked by the media type
         /// quality parameter.</remarks>
-        protected virtual string? ReadAcceptHeader( IEnumerable<MediaTypeWithQualityHeaderValue> accept )
+        protected virtual string? ReadAcceptHeader( ICollection<MediaTypeWithQualityHeaderValue> accept )
         {
-            var comparer = StringComparer.OrdinalIgnoreCase;
-            var contentTypes = from entry in accept
-                               orderby entry.Quality descending
-                               group entry by entry.MediaType;
+            if ( accept == null )
+            {
+                throw new ArgumentNullException( nameof( accept ) );
+            }
 
-            foreach ( var contentType in contentTypes )
+            var count = accept.Count;
+
+            if ( count == 0 )
             {
-                foreach ( var entry in contentType )
+                return default;
+            }
+
+            var mediaTypes = accept.ToArray();
+
+            Array.Sort( mediaTypes, ByQualityDescending );
+
+            for ( var i = 0; i < count; i++ )
+            {
+#if WEBAPI
+                var parameters = mediaTypes[i].Parameters.ToArray();
+                var paramCount = parameters.Length;
+#else
+                var parameters = mediaTypes[i].Parameters;
+                var paramCount = parameters.Count;
+#endif
+                for ( var j = 0; j < paramCount; j++ )
                 {
-                    foreach ( var parameter in entry.Parameters )
+                    var parameter = parameters[j];
+
+                    if ( parameter.Name.Equals( ParameterName, OrdinalIgnoreCase ) )
                     {
 #if WEBAPI
-                        if ( comparer.Equals( parameter.Name, ParameterName ) )
-                        {
-                            return parameter.Value;
-                        }
+                        return parameter.Value;
 #else
-                        if ( comparer.Equals( parameter.Name.Value, ParameterName ) )
-                        {
-                            return parameter.Value.Value;
-                        }
+                        return parameter.Value.Value;
 #endif
                     }
                 }
@@ -94,22 +110,25 @@ orderby entry.Quality descending
             {
                 throw new ArgumentNullException( nameof( contentType ) );
             }
-
-            var comparer = StringComparer.OrdinalIgnoreCase;
-
-            foreach ( var parameter in contentType.Parameters )
-            {
 #if WEBAPI
-                if ( comparer.Equals( parameter.Name, ParameterName ) )
+            var parameters = contentType.Parameters.ToArray();
+            var count = parameters.Length;
+#else
+            var parameters = contentType.Parameters;
+            var count = parameters.Count;
+#endif
+            for ( var i = 0; i < count; i++ )
+            {
+                var parameter = parameters[i];
+
+                if ( parameter.Name.Equals( ParameterName, OrdinalIgnoreCase ) )
                 {
+#if WEBAPI
                     return parameter.Value;
-                }
 #else
-                if ( comparer.Equals( parameter.Name.Value, ParameterName ) )
-                {
                     return parameter.Value.Value;
-                }
 #endif
+                }
             }
 
             return null;
@@ -128,5 +147,8 @@ public virtual void AddParameters( IApiVersionParameterDescriptionContext contex
 
             context.AddParameter( ParameterName, MediaTypeParameter );
         }
+
+        static int ByQualityDescending( MediaTypeWithQualityHeaderValue? left, MediaTypeWithQualityHeaderValue? right ) =>
+            -Nullable.Compare( left?.Quality, right?.Quality );
     }
 }

src/Common/Versioning/QueryStringApiVersionReader.cs

@@ -75,9 +75,14 @@ public virtual void AddParameters( IApiVersionParameterDescriptionContext contex
                 throw new ArgumentNullException( nameof( context ) );
             }
 
-            foreach ( var name in ParameterNames )
+            var count = ParameterNames.Count;
+            var names = new string[count];
+
+            ParameterNames.CopyTo( names, 0 );
+
+            for ( var i = 0; i < count; i++ )
             {
-                context.AddParameter( name, Query );
+                context.AddParameter( names[i], Query );
             }
         }
     }

src/Microsoft.AspNet.WebApi.Versioning.ApiExplorer/Description/VersionedApiExplorer.cs

@@ -949,7 +949,7 @@ static void AddUndeclaredRouteParameters( IParsedRoute parsedRoute, IDictionary<
         {
             for ( var i = 0; i < parsedRoute.PathSegments.Count; i++ )
             {
-                if ( !( parsedRoute.PathSegments[i] is IPathContentSegment content ) )
+                if ( parsedRoute.PathSegments[i] is not IPathContentSegment content )
                 {
                     continue;
                 }
@@ -1055,7 +1055,7 @@ static bool MatchRegexConstraint( IHttpRoute route, string parameterName, string
             }
 
             // note that we don't support custom constraint (IHttpRouteConstraint) because it might rely on the request and some runtime states
-            if ( !( constraint is string constraintsRule ) )
+            if ( constraint is not string constraintsRule )
             {
                 return true;
             }

src/Microsoft.AspNet.WebApi.Versioning.ApiExplorer/System.Web.Http/Description/ApiDescriptionExtensions.cs

@@ -103,7 +103,7 @@ public static bool TryUpdateRelativePathAndRemoveApiVersionParameter( this ApiDe
                 throw new ArgumentNullException( nameof( options ) );
             }
 
-            if ( !options.SubstituteApiVersionInUrl || !( apiDescription is VersionedApiDescription versionedApiDescription ) )
+            if ( !options.SubstituteApiVersionInUrl || apiDescription is not VersionedApiDescription versionedApiDescription )
             {
                 return false;
             }

src/Microsoft.AspNet.WebApi.Versioning/System.Web.Http/HttpParameterBindingExtensions.cs

@@ -9,7 +9,7 @@ static class HttpParameterBindingExtensions
     {
         internal static bool WillReadUri( this HttpParameterBinding parameterBinding )
         {
-            if ( !( parameterBinding is IValueProviderParameterBinding valueProviderParameterBinding ) )
+            if ( parameterBinding is not IValueProviderParameterBinding valueProviderParameterBinding )
             {
                 return false;
             }