THRIFT-5504 CA2254 Message template should be compile time constant

Client netstd Patch: Jens Geyer
paddyesch · Jan 27, 2022 · 561bc9a · 561bc9a
1 parent bf09675
commit 561bc9a
Show file tree

Hide file tree

Showing 17 changed files with 62 additions and 40 deletions.
diff --git a/lib/netstd/Thrift/GlobalSuppressions.cs b/lib/netstd/Thrift/GlobalSuppressions.cs
@@ -0,0 +1,31 @@
+// Licensed to the Apache Software Foundation(ASF) under one
+// or more contributor license agreements.See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+
+// This file is used by Code Analysis to maintain SuppressMessage
+// attributes that are applied to this project.
+// Project-level suppressions either have no target or are given
+// a specific target and scoped to a namespace, type, member, etc.
+
+using System.Diagnostics.CodeAnalysis;
+
+// suppress certain messages for compatibility reasons with older C# versions we want to support
+[assembly: SuppressMessage("Style", "IDE0057", Justification = "compatibility", Scope = "module")]
+[assembly: SuppressMessage("Style", "IDE0066", Justification = "compatibility", Scope = "module")]
+[assembly: SuppressMessage("Style", "IDE0090", Justification = "compatibility", Scope = "module")]
+[assembly: SuppressMessage("Style", "IDE0063", Justification = "compatibility", Scope = "module")]
+
diff --git a/lib/netstd/Thrift/Processor/ITAsyncProcessor.cs b/lib/netstd/Thrift/Processor/ITAsyncProcessor.cs
@@ -23,6 +23,6 @@ namespace Thrift.Processor
 {
     public interface ITAsyncProcessor
     {
-        Task<bool> ProcessAsync(TProtocol iprot, TProtocol oprot, CancellationToken cancellationToken = default(CancellationToken));
+        Task<bool> ProcessAsync(TProtocol iprot, TProtocol oprot, CancellationToken cancellationToken = default);
     }
 }
diff --git a/lib/netstd/Thrift/Processor/TMultiplexedProcessor.cs b/lib/netstd/Thrift/Processor/TMultiplexedProcessor.cs
@@ -65,8 +65,7 @@ await FailAsync(oprot, message, TApplicationException.ExceptionType.InvalidProto
 
                 // Create a new TMessage, something that can be consumed by any TProtocol
                 var serviceName = message.Name.Substring(0, index);
-                ITAsyncProcessor actualProcessor;
-                if (!_serviceProcessorMap.TryGetValue(serviceName, out actualProcessor))
+                if (!_serviceProcessorMap.TryGetValue(serviceName, out ITAsyncProcessor actualProcessor))
                 {
                     await FailAsync(oprot, message, TApplicationException.ExceptionType.InternalError,
                         $"Service name not found: {serviceName}. Did you forget to call RegisterProcessor()?",
@@ -103,7 +102,7 @@ public void RegisterProcessor(string serviceName, ITAsyncProcessor processor)
             _serviceProcessorMap.Add(serviceName, processor);
         }
 
-        private async Task FailAsync(TProtocol oprot, TMessage message, TApplicationException.ExceptionType extype,
+        private static async Task FailAsync(TProtocol oprot, TMessage message, TApplicationException.ExceptionType extype,
             string etxt, CancellationToken cancellationToken)
         {
             var appex = new TApplicationException(extype, etxt);

diff --git a/lib/netstd/Thrift/Protocol/TBinaryProtocol.cs b/lib/netstd/Thrift/Protocol/TBinaryProtocol.cs
@@ -23,8 +23,6 @@
 using Thrift.Protocol.Entities;
 using Thrift.Transport;
 
-#pragma warning disable IDE0079 // unnecessary suppression
-#pragma warning disable IDE0066 // use switch expression
 
 namespace Thrift.Protocol
 {

diff --git a/lib/netstd/Thrift/Protocol/TCompactProtocol.cs b/lib/netstd/Thrift/Protocol/TCompactProtocol.cs
@@ -26,8 +26,6 @@
 using Thrift.Protocol.Entities;
 using Thrift.Transport;
 
-#pragma warning disable IDE0079 // unnecessary suppression
-#pragma warning disable IDE0066 // use switch expression
 
 namespace Thrift.Protocol
 {

diff --git a/lib/netstd/Thrift/Protocol/TJSONProtocol.cs b/lib/netstd/Thrift/Protocol/TJSONProtocol.cs
@@ -27,9 +27,6 @@
 using Thrift.Protocol.Utilities;
 using Thrift.Transport;
 
-#pragma warning disable IDE0079 // unnecessary suppression
-#pragma warning disable IDE0063 // simplify using
-#pragma warning disable IDE0066 // use switch expression
 
 namespace Thrift.Protocol
 {

diff --git a/lib/netstd/Thrift/Protocol/TProtocol.cs b/lib/netstd/Thrift/Protocol/TProtocol.cs
@@ -50,6 +50,7 @@ protected TProtocol(TTransport trans)
         public void Dispose()
         {
             Dispose(true);
+            GC.SuppressFinalize(this);
         }
 
         public void IncrementRecursionDepth()

diff --git a/lib/netstd/Thrift/Protocol/ToString.cs b/lib/netstd/Thrift/Protocol/ToString.cs
@@ -47,15 +47,15 @@ public static void ToString(this object self, StringBuilder sb, bool first = tru
                     if (first_child)
                         first_child = false;
                     else
-                        sb.Append(",");
+                        sb.Append(',');
 
                     sb.Append("{ ");
                     pair.Key.ToString(sb);
                     sb.Append(", ");
                     pair.Value.ToString(sb);
-                    sb.Append("}");
+                    sb.Append('}');
                 }
-                sb.Append("}");
+                sb.Append('}');
             }
             else if (self is IEnumerable)
             {
@@ -65,7 +65,7 @@ public static void ToString(this object self, StringBuilder sb, bool first = tru
                     elm.ToString(sb, first_child);
                     first_child = false;
                 }
-                sb.Append("}");
+                sb.Append('}');
             }
             else if (self is TBase)
             {

diff --git a/lib/netstd/Thrift/Server/TServer.cs b/lib/netstd/Thrift/Server/TServer.cs
@@ -35,7 +35,7 @@ public abstract class TServer
         protected TProtocolFactory OutputProtocolFactory;
         protected TTransportFactory OutputTransportFactory;
 
-        protected TServerEventHandler ServerEventHandler;
+        protected ITServerEventHandler ServerEventHandler;
         protected TServerTransport ServerTransport;
 
         protected TServer(ITProcessorFactory processorFactory, TServerTransport serverTransport,
@@ -52,12 +52,12 @@ protected TServer(ITProcessorFactory processorFactory, TServerTransport serverTr
             Logger = logger; // null is absolutely legal
         }
 
-        public void SetEventHandler(TServerEventHandler seh)
+        public void SetEventHandler(ITServerEventHandler seh)
         {
             ServerEventHandler = seh;
         }
 
-        public TServerEventHandler GetEventHandler()
+        public ITServerEventHandler GetEventHandler()
         {
             return ServerEventHandler;
         }
@@ -66,7 +66,7 @@ public TServerEventHandler GetEventHandler()
         protected void LogError( string msg)
         {
             if (Logger != null)
-                Logger.LogError(msg);
+                Logger.LogError("{Msg}",msg);  // NOTE: Log message template, not string interpolation!
         }
 
         public abstract void Stop();

diff --git a/lib/netstd/Thrift/Server/TServerEventHandler.cs b/lib/netstd/Thrift/Server/TServerEventHandler.cs
@@ -25,10 +25,18 @@ namespace Thrift.Server
     //TODO: replacement by event?
 
     /// <summary>
-    ///     Interface implemented by server users to handle events from the server
+    /// Interface implemented by server users to handle events from the server
     /// </summary>
+    /// <remarks>Replaced by ITServerEventHandler</remarks>
     // ReSharper disable once InconsistentNaming
-    public interface TServerEventHandler
+    #pragma warning disable IDE1006
+    public interface TServerEventHandler : ITServerEventHandler { }
+    #pragma warning restore IDE1006
+
+    /// <summary>
+    /// Interface implemented by server users to handle events from the server
+    /// </summary>
+    public interface ITServerEventHandler
     {
         /// <summary>
         ///     Called before the server begins */

diff --git a/lib/netstd/Thrift/Server/TSimpleAsyncServer.cs b/lib/netstd/Thrift/Server/TSimpleAsyncServer.cs
@@ -23,8 +23,6 @@
 using System.Threading.Tasks;
 using Microsoft.Extensions.Logging;
 
-#pragma warning disable IDE0079  // remove unnecessary pragmas
-#pragma warning disable IDE0063  // using can be simplified, we don't
 
 namespace Thrift.Server
 {

diff --git a/lib/netstd/Thrift/Server/TThreadPoolAsyncServer.cs b/lib/netstd/Thrift/Server/TThreadPoolAsyncServer.cs
@@ -29,8 +29,6 @@
 using System.Threading.Tasks;
 using Microsoft.Extensions.Logging;
 
-#pragma warning disable IDE0079  // remove unnecessary pragmas
-#pragma warning disable IDE0063  // using can be simplified, we don't
 
 namespace Thrift.Server
 {

diff --git a/lib/netstd/Thrift/Transport/Client/THttpTransport.cs b/lib/netstd/Thrift/Transport/Client/THttpTransport.cs
@@ -25,8 +25,6 @@
 using System.Threading;
 using System.Threading.Tasks;
 
-#pragma warning disable IDE0079 // unnecessary suppression
-#pragma warning disable IDE0063 // simplify using
 
 namespace Thrift.Transport.Client
 {

diff --git a/lib/netstd/Thrift/Transport/Client/TMemoryBufferTransport.cs b/lib/netstd/Thrift/Transport/Client/TMemoryBufferTransport.cs
@@ -21,8 +21,6 @@
 using System.Threading;
 using System.Threading.Tasks;
 
-#pragma warning disable IDE0079  // unused suppression
-#pragma warning disable IDE0066  // requires C# 8 
 
 namespace Thrift.Transport.Client
 {

diff --git a/lib/netstd/Thrift/Transport/Layered/TBufferedTransport.cs b/lib/netstd/Thrift/Transport/Layered/TBufferedTransport.cs
@@ -110,8 +110,7 @@ public override async ValueTask<int> ReadAsync(byte[] buffer, int offset, int le
 
             // buffer a new chunk of bytes from the underlying transport
             ReadBuffer.Length = ReadBuffer.Capacity;
-            ArraySegment<byte> bufSegment;
-            ReadBuffer.TryGetBuffer(out bufSegment);
+            ReadBuffer.TryGetBuffer(out ArraySegment<byte> bufSegment);
             ReadBuffer.Length = await InnerTransport.ReadAsync(bufSegment.Array, 0, bufSegment.Count, cancellationToken);
             ReadBuffer.Position = 0;
 
@@ -134,8 +133,7 @@ public override async Task WriteAsync(byte[] buffer, int offset, int length, Can
             var free = WriteBuffer.Capacity - WriteBuffer.Length;
             if (length > free)
             {
-                ArraySegment<byte> bufSegment;
-                WriteBuffer.TryGetBuffer(out bufSegment);
+                WriteBuffer.TryGetBuffer(out ArraySegment<byte> bufSegment);
                 await InnerTransport.WriteAsync(bufSegment.Array, 0, bufSegment.Count, cancellationToken);
                 WriteBuffer.SetLength(0);
             }
@@ -163,8 +161,7 @@ public override async Task FlushAsync(CancellationToken cancellationToken)
 
             if (WriteBuffer.Length > 0)
             {
-                ArraySegment<byte> bufSegment;
-                WriteBuffer.TryGetBuffer(out bufSegment);
+                WriteBuffer.TryGetBuffer(out ArraySegment<byte> bufSegment);
                 await InnerTransport.WriteAsync(bufSegment.Array, 0, bufSegment.Count, cancellationToken);
                 WriteBuffer.SetLength(0);
             }

diff --git a/lib/netstd/Thrift/Transport/Layered/TFramedTransport.cs b/lib/netstd/Thrift/Transport/Layered/TFramedTransport.cs
@@ -97,8 +97,7 @@ private async ValueTask ReadFrameAsync(CancellationToken cancellationToken)
             ReadBuffer.SetLength(size);
             ReadBuffer.Seek(0, SeekOrigin.Begin);
 
-            ArraySegment<byte> bufSegment;
-            ReadBuffer.TryGetBuffer(out bufSegment);
+            ReadBuffer.TryGetBuffer(out ArraySegment<byte> bufSegment);
             await InnerTransport.ReadAllAsync(bufSegment.Array, 0, size, cancellationToken);
         }
 
@@ -129,8 +128,7 @@ public override async Task FlushAsync(CancellationToken cancellationToken)
                 throw new TTransportException(TTransportException.ExceptionType.NotOpen);
             }
 
-            ArraySegment<byte> bufSegment;
-            WriteBuffer.TryGetBuffer(out bufSegment);
+            WriteBuffer.TryGetBuffer(out ArraySegment<byte> bufSegment);
 
             int dataLen = bufSegment.Count - HeaderSize;
             if (dataLen < 0)

diff --git a/lib/netstd/Thrift/Transport/Server/TServerSocketTransport.cs b/lib/netstd/Thrift/Transport/Server/TServerSocketTransport.cs
@@ -113,8 +113,11 @@ protected override async ValueTask<TTransport> AcceptImplementationAsync(Cancell
             try
             {
                 TTransport tSocketTransport = null;
+                #if NET6_0_OR_GREATER
+                var tcpClient = await _server.AcceptTcpClientAsync(cancellationToken);
+                #else
                 var tcpClient = await _server.AcceptTcpClientAsync();
-
+                #endif
                 try
                 {
                     tSocketTransport = new TSocketTransport(tcpClient, Configuration)