Fixed code analysis warnings.

Author: RxDave

Build/Properties/CodeAnalysisDictionary.xml

@@ -13,6 +13,14 @@
       <Word>__</Word>
       <Word>___</Word>
       <Word>____</Word>
+      <Word>LabelTarget</Word>
+      <Word>SwitchCase</Word>
+      <Word>CatchBlock</Word>
+      <Word>ElementInit</Word>
+      <Word>MemberBinding</Word>
+      <Word>MemberAssignment</Word>
+      <Word>MemberMemberBinding</Word>
+      <Word>MemberListBinding</Word>
     </Recognized>
 
     <Compound>

Source/Qactive.Expressions/EqualityExpressionVisitor.cs

@@ -8,6 +8,7 @@
 
 namespace Qactive.Expressions
 {
+  [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Maintainability", "CA1506:AvoidExcessiveClassCoupling", Justification = "Visitor pattern.")]
   public class EqualityExpressionVisitor : ExpressionVisitor
   {
     private readonly Stack<Expression> others = new Stack<Expression>();
@@ -55,16 +56,12 @@ private void ObjectInvariant()
 
     internal bool ShallowEquals(Expression first, Expression second) => shallowEquals(first, second);
 
-    internal bool TypeEquals(Type first, Type second) => typeEquals(first, second);
-
-    internal bool MemberEquals(MemberInfo first, MemberInfo second) => memberEquals(first, second);
-
-    public void Break(IEnumerable<Expression> inequalityNodes, IEnumerable<Expression> inequalityOthers)
+    public void Break(IEnumerable<Expression> newInequalityNodes, IEnumerable<Expression> newInequalityOthers)
     {
       AreEqual = false;
 
-      this.inequalityNodes = inequalityNodes.ToList().AsReadOnly();
-      this.inequalityOthers = inequalityOthers.ToList().AsReadOnly();
+      this.inequalityNodes = newInequalityNodes.ToList().AsReadOnly();
+      this.inequalityOthers = newInequalityOthers.ToList().AsReadOnly();
     }
 
     internal void SetOtherRoot(Expression other)
@@ -149,18 +146,18 @@ private void Visit(Expression node, Expression other)
       }
     }
 
-    private void Visit<TExpression>(ICollection<TExpression> nodes, ICollection<TExpression> others)
+    private void Visit<TExpression>(ICollection<TExpression> nodes, ICollection<TExpression> otherNodes)
       where TExpression : Expression
     {
       if (AreEqual)
       {
-        if (!(ExpressionEqualityComparer.NullsOrEquals(nodes, others, (n, o) => n.Count == o.Count)))
+        if (!(ExpressionEqualityComparer.NullsOrEquals(nodes, otherNodes, (n, o) => n.Count == o.Count)))
         {
-          Break(nodes, others);
+          Break(nodes, otherNodes);
         }
-        else if (nodes != null && others != null)
+        else if (nodes != null && otherNodes != null)
         {
-          foreach (var pair in nodes.Zip(others, (node, other) => new { node, other }))
+          foreach (var pair in nodes.Zip(otherNodes, (node, other) => new { node, other }))
           {
             Visit(pair.node, pair.other);
           }
@@ -261,6 +258,7 @@ protected override CatchBlock VisitCatchBlock(CatchBlock node)
       throw new InvalidOperationException("Bug: CatchBlock must not be visited since it's compared within the expressions that contain it.");
     }
 
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Maintainability", "CA1502:AvoidExcessiveComplexity", Justification = "Seems unavoidable.")]
     protected override Expression VisitTry(TryExpression node)
       => VisitIfEqual(node, other => ExpressionEqualityComparer.NullsOrEquals(node.Handlers, other.Handlers, (n, o) => n.Select(h => h.Test).SequenceEqual(o.Select(h => h.Test))),
                             other => Visit((node.Handlers ?? Enumerable.Empty<CatchBlock>()).Select(n => n.Body).ToList(), (other.Handlers ?? Enumerable.Empty<CatchBlock>()).Select(n => n.Body).ToList()),
@@ -311,26 +309,26 @@ protected override MemberListBinding VisitMemberListBinding(MemberListBinding no
       throw new InvalidOperationException("Bug: MemberListBinding must not be visited since it's compared within the expressions that contain it.");
     }
 
-    private bool Equals(LabelTarget target, LabelTarget other)
+    private static bool Equals(LabelTarget target, LabelTarget other)
       => ExpressionEqualityComparer.NullsOrEquals(target, other, (n, o) => n.Name == o.Name && n.Type == o.Type);
 
-    private bool Equals(ICollection<MemberBinding> binding, ICollection<MemberBinding> other)
+    private static bool Equals(ICollection<MemberBinding> binding, ICollection<MemberBinding> other)
       => ExpressionEqualityComparer.NullsOrEquals(binding, other, (n, o) => n.Count == o.Count && n.Zip(o, Equals).All(b => b));
 
-    private bool Equals(MemberBinding binding, MemberBinding other)
+    private static bool Equals(MemberBinding binding, MemberBinding other)
       => ExpressionEqualityComparer.NullsOrEquals(binding, other, (n, o) => n.BindingType == o.BindingType
                                                                          && n.Member == o.Member
                                                                          && (EqualsIfType<MemberMemberBinding>(n, o, Equals)
                                                                            ?? EqualsIfType<MemberListBinding>(n, o, Equals)
                                                                            ?? false));
 
-    private bool Equals(MemberMemberBinding binding, MemberMemberBinding other)
+    private static bool Equals(MemberMemberBinding binding, MemberMemberBinding other)
       => Equals(binding.Bindings, other.Bindings);
 
-    private bool Equals(MemberListBinding binding, MemberListBinding other)
+    private static bool Equals(MemberListBinding binding, MemberListBinding other)
       => ExpressionEqualityComparer.NullsOrEquals(binding.Initializers, other.Initializers, (n, o) => n.Select(i => i.AddMethod).SequenceEqual(o.Select(i => i.AddMethod)));
 
-    private bool? EqualsIfType<T>(object first, object second, Func<T, T, bool> comparer)
+    private static bool? EqualsIfType<T>(object first, object second, Func<T, T, bool> comparer)
       where T : class
     {
       var x = first as T;
@@ -339,7 +337,7 @@ private bool Equals(MemberListBinding binding, MemberListBinding other)
       return x != null && y != null ? comparer(x, y) : (bool?)null;
     }
 
-    private ICollection<Expression> GetExpressions(ReadOnlyCollection<MemberBinding> bindings)
+    private static ICollection<Expression> GetExpressions(ReadOnlyCollection<MemberBinding> bindings)
       => bindings.OfType<MemberAssignment>().Select(assignment => assignment.Expression)
                  .Concat(
          bindings.OfType<MemberListBinding>().SelectMany(binding => binding.Initializers).SelectMany(init => init.Arguments))

Source/Qactive.Expressions/ExpressionEqualityComparer.cs

@@ -8,7 +8,10 @@ namespace Qactive.Expressions
 {
   public sealed class ExpressionEqualityComparer : IEqualityComparer<Expression>
   {
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Security", "CA2104:DoNotDeclareReadOnlyMutableReferenceTypes", Justification = "It's immutable.")]
     public static readonly ExpressionEqualityComparer Exact = new ExpressionEqualityComparer(reflectionNamesOnly: false);
+
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Security", "CA2104:DoNotDeclareReadOnlyMutableReferenceTypes", Justification = "It's immutable.")]
     public static readonly ExpressionEqualityComparer ReflectionNamesOnly = new ExpressionEqualityComparer(reflectionNamesOnly: true);
 
     private readonly bool reflectionNamesOnly;
@@ -21,11 +24,15 @@ private ExpressionEqualityComparer(bool reflectionNamesOnly)
     public bool Equals(Expression x, Expression y)
       => Equals(x, y, new EqualityExpressionVisitor(ShallowEquals, TypeEquals, MemberEquals));
 
-    public bool Equals(Expression x, Expression y, EqualityExpressionVisitor visitor)
-      => visitor.ShallowEquals(x, y) && DeepEquals(x, y, visitor);
+    public static bool Equals(Expression first, Expression second, EqualityExpressionVisitor visitor)
+    {
+      Contract.Requires(visitor != null);
+
+      return visitor.ShallowEquals(first, second) && DeepEquals(first, second, visitor);
+    }
 
-    public bool ShallowEquals(Expression x, Expression y)
-      => NullsOrEquals(x, y, (f, s) => f.NodeType == s.NodeType && TypeEquals(GetRepresentativeType(f), GetRepresentativeType(s)));
+    public bool ShallowEquals(Expression first, Expression second)
+      => NullsOrEquals(first, second, (f, s) => f.NodeType == s.NodeType && TypeEquals(GetRepresentativeType(f), GetRepresentativeType(s)));
 
     public static Type GetRepresentativeType(Expression expression)
     {
@@ -45,18 +52,18 @@ public static Type GetRepresentativeType(Expression expression)
       return type;
     }
 
-    private bool DeepEquals(Expression x, Expression y, EqualityExpressionVisitor visitor)
+    private static bool DeepEquals(Expression first, Expression second, EqualityExpressionVisitor visitor)
     {
-      Contract.Requires((x == null && y == null) || (x != null && y != null));
+      Contract.Requires((first == null && second == null) || (first != null && second != null));
       Contract.Requires(visitor != null);
 
-      if (x == null && y == null)
+      if (first == null)
       {
         return true;
       }
 
-      visitor.SetOtherRoot(y);
-      visitor.Visit(x);
+      visitor.SetOtherRoot(second);
+      visitor.Visit(first);
 
       Contract.Assume(visitor.StackCount == 1);
 

Source/Qactive.Providers.Tcp/TcpQactiveProvider.cs

@@ -129,6 +129,7 @@ public static TcpQactiveProvider Server(IPEndPoint endPoint, ITcpQactiveProvider
       return new TcpQactiveProvider(endPoint, transportInitializer);
     }
 
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Maintainability", "CA1506:AvoidExcessiveClassCoupling", Justification = "Seems as simple as it's going to get.")]
     [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Justification = "The SocketAsyncEventArgs instance is either disposed before returning or by the observable's Finally operator.")]
     public override IObservable<TResult> Connect<TResult>(Func<IQbservableProtocol, Expression> prepareExpression)
     {
@@ -181,7 +182,7 @@ public override IObservable<TResult> Connect<TResult>(Func<IQbservableProtocol,
 
                                     var s = Observable.Using(
                                       () => new NetworkStream(e2.ConnectSocket, ownsSocket: false),
-                                      stream => ReadObservable<TResult>(stream, prepareExpression, cancel.Token))
+                                      stream => GetObservable<TResult>(stream, prepareExpression, cancel.Token))
                                       .SubscribeSafe(innerObserver);
 
                                     return new CompositeDisposable(s, cancel);
@@ -205,7 +206,7 @@ public override IObservable<TResult> Connect<TResult>(Func<IQbservableProtocol,
       }
     }
 
-    private IObservable<TResult> ReadObservable<TResult>(Stream stream, Func<IQbservableProtocol, Expression> prepareExpression, CancellationToken cancel)
+    private IObservable<TResult> GetObservable<TResult>(Stream stream, Func<IQbservableProtocol, Expression> prepareExpression, CancellationToken cancel)
     {
       Contract.Requires(stream != null);
       Contract.Requires(prepareExpression != null);
@@ -218,6 +219,7 @@ from result in protocol
              select result;
     }
 
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Maintainability", "CA1506:AvoidExcessiveClassCoupling", Justification = "Seems as simple as it's going to get.")]
     public override IObservable<ClientTermination> Listen(QbservableServiceOptions options, Func<IQbservableProtocol, IParameterizedQbservableProvider> providerFactory)
     {
       return from listener in Observable.Return(new TcpListener(EndPoint))
@@ -251,84 +253,96 @@ from client in Observable.FromAsync(listener.AcceptTcpClientAsync).Repeat()
                Stopped();
              })
              let capturedId = new CapturedId(Id + " C" + Interlocked.Increment(ref lastServerClientNumber) + " " + client.Client.RemoteEndPoint)
-             from result in Observable.StartAsync(async cancel =>
-             {
-               ReceivingConnection(idOverride: capturedId.Value);
+             from termination in Observable.StartAsync(cancel => AcceptAsync(client, capturedId, options, providerFactory, cancel))
+             .Finally(() => Shutdown(client.Client, capturedId.Value))
+             select termination;
+    }
 
-               // These default settings enable a proper graceful shutdown. DisconnectAsync is used instead of Close on the server-side to request 
-               // that the client terminates the connection ASAP. This is important because it prevents the server-side socket from going into a 
-               // TIME_WAIT state rather than the client. The linger option is meant to ensure that any outgoing data, such as an exception, is 
-               // completely transmitted to the client before the socket terminates. The seconds specified is arbitrary, though chosen to be large 
-               // enough to transfer any remaining data successfully and small enough to cause a timely disconnection. A custom prepareSocket 
-               // implementation can always change it via SetSocketOption, if necessary.
-               //
-               // https://msdn.microsoft.com/en-us/library/system.net.sockets.socket.disconnect(v=vs.110).aspx
-               client.LingerState.Enabled = true;
-               client.LingerState.LingerTime = LingerTimeInSeconds;
+    private async Task<TcpClientTermination> AcceptAsync(
+      TcpClient client,
+      CapturedId capturedId,
+      QbservableServiceOptions options,
+      Func<IQbservableProtocol, IParameterizedQbservableProvider> providerFactory,
+      CancellationToken cancel)
+    {
+      Contract.Requires(client != null);
+      Contract.Requires(capturedId != null);
+      Contract.Requires(options != null);
+      Contract.Requires(providerFactory != null);
 
-               prepareSocket(client.Client);
+      ReceivingConnection(idOverride: capturedId.Value);
 
-               var watch = Stopwatch.StartNew();
+      // These default settings enable a proper graceful shutdown. DisconnectAsync is used instead of Close on the server-side to request 
+      // that the client terminates the connection ASAP. This is important because it prevents the server-side socket from going into a 
+      // TIME_WAIT state rather than the client. The linger option is meant to ensure that any outgoing data, such as an exception, is 
+      // completely transmitted to the client before the socket terminates. The seconds specified is arbitrary, though chosen to be large 
+      // enough to transfer any remaining data successfully and small enough to cause a timely disconnection. A custom prepareSocket 
+      // implementation can always change it via SetSocketOption, if necessary.
+      //
+      // https://msdn.microsoft.com/en-us/library/system.net.sockets.socket.disconnect(v=vs.110).aspx
+      client.LingerState.Enabled = true;
+      client.LingerState.LingerTime = LingerTimeInSeconds;
 
-               var localEndPoint = client.Client.LocalEndPoint;
-               var remoteEndPoint = client.Client.RemoteEndPoint;
+      prepareSocket(client.Client);
 
-               var exceptions = new List<ExceptionDispatchInfo>();
-               var shutdownReason = QbservableProtocolShutdownReason.None;
+      var watch = Stopwatch.StartNew();
 
-               try
-               {
-                 using (var stream = new NetworkStream(client.Client, ownsSocket: false))
-                 using (var protocol = await NegotiateServerAsync(capturedId.Value, stream, formatterFactory(), options, cancel).ConfigureAwait(false))
-                 {
-                   capturedId.Value = protocol.ClientId;
-
-                   var provider = providerFactory(protocol);
-
-                   ReceivedConnection(idOverride: capturedId.Value);
-
-                   try
-                   {
-                     await protocol.ExecuteServerAsync(provider).ConfigureAwait(false);
-                   }
-                   catch (OperationCanceledException)
-                   {
-                   }
-                   catch (Exception ex)
-                   {
-                     exceptions.Add(ExceptionDispatchInfo.Capture(ex));
-                   }
-                   finally
-                   {
-                     shutdownReason = protocol.ShutdownReason;
-                   }
-
-                   var protocolExceptions = protocol.Exceptions;
-
-                   if (protocolExceptions != null)
-                   {
-                     foreach (var exception in protocolExceptions)
-                     {
-                       exceptions.Add(exception);
-                     }
-                   }
-                 }
-               }
-               catch (OperationCanceledException)
-               {
-                 shutdownReason = QbservableProtocolShutdownReason.ProtocolNegotiationCanceled;
-               }
-               catch (Exception ex)
-               {
-                 shutdownReason = QbservableProtocolShutdownReason.ProtocolNegotiationError;
+      var localEndPoint = client.Client.LocalEndPoint;
+      var remoteEndPoint = client.Client.RemoteEndPoint;
 
-                 exceptions.Add(ExceptionDispatchInfo.Capture(ex));
-               }
+      var exceptions = new List<ExceptionDispatchInfo>();
+      var shutdownReason = QbservableProtocolShutdownReason.None;
 
-               return new TcpClientTermination(localEndPoint, remoteEndPoint, watch.Elapsed, shutdownReason, exceptions);
-             })
-             .Finally(() => Shutdown(client.Client, capturedId.Value))
-             select result;
+      try
+      {
+        using (var stream = new NetworkStream(client.Client, ownsSocket: false))
+        using (var protocol = await NegotiateServerAsync(capturedId.Value, stream, formatterFactory(), options, cancel).ConfigureAwait(false))
+        {
+          capturedId.Value = protocol.ClientId;
+
+          var provider = providerFactory(protocol);
+
+          ReceivedConnection(idOverride: capturedId.Value);
+
+          try
+          {
+            await protocol.ExecuteServerAsync(provider).ConfigureAwait(false);
+          }
+          catch (OperationCanceledException)
+          {
+          }
+          catch (Exception ex)
+          {
+            exceptions.Add(ExceptionDispatchInfo.Capture(ex));
+          }
+          finally
+          {
+            shutdownReason = protocol.ShutdownReason;
+          }
+
+          var protocolExceptions = protocol.Exceptions;
+
+          if (protocolExceptions != null)
+          {
+            foreach (var exception in protocolExceptions)
+            {
+              exceptions.Add(exception);
+            }
+          }
+        }
+      }
+      catch (OperationCanceledException)
+      {
+        shutdownReason = QbservableProtocolShutdownReason.ProtocolNegotiationCanceled;
+      }
+      catch (Exception ex)
+      {
+        shutdownReason = QbservableProtocolShutdownReason.ProtocolNegotiationError;
+
+        exceptions.Add(ExceptionDispatchInfo.Capture(ex));
+      }
+
+      return new TcpClientTermination(localEndPoint, remoteEndPoint, watch.Elapsed, shutdownReason, exceptions);
     }
 
     private async Task<IStreamQbservableProtocol> NegotiateClientAsync(Stream stream, IRemotingFormatter formatter, CancellationToken cancel)
@@ -464,7 +478,7 @@ private async void Shutdown(Socket socket, object id = null)
     }
 
     [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Justification = "The SocketAsyncEventArgs instance is either disposed before returning or by the observable's Finally operator.")]
-    private async Task DisconnectAsync(Socket socket)
+    private static async Task DisconnectAsync(Socket socket)
     {
       Contract.Requires(socket != null);