chore: logger enforce "CA2254: Template should be a static expression" (

microsoft#564)

* chore: logger enforce "CA2254: Template should be a static expression"

.editorconfig

@@ -35,6 +35,10 @@ dotnet_diagnostic.CS1591.severity = suggestion
 # CA5359: Do not disable certificate validation
 dotnet_diagnostic.CA5359.severity = suggestion
 
+# Usage rules
+# CA2254: Template should be a static expression
+dotnet_diagnostic.CA2254.severity = warning
+
 # Miscellaneous style rules
 dotnet_separate_import_directive_groups = false
 file_header_template = Copyright (c) Microsoft Corporation.\nLicensed under the MIT license.

benchmark/Resp.benchmark/RespOnlineBench.cs

@@ -323,7 +323,7 @@ public void Run()
                             $"{"total_ops;",pad}" +
                             $"{"iter_tops;",pad}" +
                             $"{"tpt (Kops/sec)",pad}";
-                        logger?.LogInformation(msg);
+                        logger?.LogInformation("{msg}", msg);
                     }
                 }
 
@@ -379,12 +379,12 @@ public void Run()
                         $"{summary.TotalCount,pad}" +
                         $"{curr_iter_ops,pad}" +
                         $"{Math.Round(BatchSize * curr_iter_ops / elapsedSecs, 2),pad}";
-                        logger.Log(LogLevel.Information, histogramOutput);
+                        logger.Log(LogLevel.Information, "{msg}", histogramOutput);
                     }
                     else
                     {
                         var histogramOutput = $"{0,pad}" + $"{0,pad}" + $"{0,pad}" + $"{0,pad}" + $"{0,pad}" + $"{0,pad}" + $"{0,pad}" + $"{0,pad}" + $"{0,pad}" + $"{0,pad}";
-                        logger.Log(LogLevel.Information, histogramOutput);
+                        logger.Log(LogLevel.Information, "{msg}", histogramOutput);
                     }
                 }
                 last_iter_ops = summary.TotalCount;

libs/client/Utility.cs

@@ -55,7 +55,7 @@ internal static int NumBitsPreviousPowerOf2(long v, ILogger logger = null)
         {
             long adjustedSize = PreviousPowerOf2(v);
             if (v != adjustedSize)
-                logger?.LogInformation($"Warning: using lower value {adjustedSize} instead of specified value {v}");
+                logger?.LogInformation("Warning: using lower value {adjustedSize} instead of specified value {specifiedValue}", adjustedSize, v);
             return (int)Math.Log(adjustedSize, 2);
         }
 

libs/cluster/Server/Migration/MigrationDriver.cs

@@ -80,7 +80,7 @@ private void BeginAsyncMigrationTask()
                 //3. Migrate actual data
                 if (!MigrateSlotsDriver())
                 {
-                    logger?.LogError($"MigrateSlotsDriver failed");
+                    logger?.LogError("MigrateSlotsDriver failed");
                     TryRecoverFromFailure();
                     Status = MigrateState.FAIL;
                     return;
@@ -91,7 +91,7 @@ private void BeginAsyncMigrationTask()
                 //5. Clear local migration set.
                 if (!RelinquishOwnership())
                 {
-                    logger?.LogError($"Failed to relinquish ownerhsip to target node");
+                    logger?.LogError("Failed to relinquish ownerhsip to target node");
                     TryRecoverFromFailure();
                     Status = MigrateState.FAIL;
                     return;
@@ -100,7 +100,7 @@ private void BeginAsyncMigrationTask()
                 //6. Change ownership of slots to target node.
                 if (!TrySetSlotRanges(GetTargetNodeId, MigrateState.NODE))
                 {
-                    logger?.LogError($"Failed to assign ownerhsip to target node");
+                    logger?.LogError("Failed to assign ownerhsip to target node");
                     TryRecoverFromFailure();
                     Status = MigrateState.FAIL;
                     return;

libs/cluster/Server/Replication/ReplicationManager.cs

@@ -202,7 +202,7 @@ public void Recover()
                     // ReplicaRecover();
                     break;
                 default:
-                    logger?.LogError($"Not valid role for node {nodeRole}");
+                    logger?.LogError("Not valid role for node {nodeRole}", nodeRole);
                     throw new Exception($"Not valid role for node {nodeRole}");
             }
         }
@@ -270,7 +270,7 @@ public void Start()
                     if (clusterProvider.replicationManager.TryAddReplicationTask(replicaId, 0, out var aofSyncTaskInfo))
                     {
                         if (!TryConnectToReplica(replicaId, 0, aofSyncTaskInfo, out var errorMessage))
-                            logger?.LogError($"{{errorMessage}}", Encoding.ASCII.GetString(errorMessage));
+                            logger?.LogError("{errorMessage}", Encoding.ASCII.GetString(errorMessage));
                     }
                 }
             }

libs/cluster/Session/MigrateCommand.cs

@@ -184,7 +184,7 @@ private bool TryMIGRATE(int count, byte* ptr)
                         // Add pointer of current parsed key
                         if (!keys.TryAdd(new ArgSlice(keyPtr, ksize), KeyMigrationStatus.QUEUED))
                         {
-                            logger?.LogWarning($"Failed to add {{key}}", Encoding.ASCII.GetString(keyPtr, ksize));
+                            logger?.LogWarning("Failed to add {key}", Encoding.ASCII.GetString(keyPtr, ksize));
                             pstate = MigrateCmdParseState.FAILEDTOADDKEY;
                             continue;
                         }

libs/cluster/Session/ReplicaOfCommand.cs

@@ -47,7 +47,7 @@ private bool TryREPLICAOF(int count, byte* ptr)
             {
                 if (!int.TryParse(portStr, out var port))
                 {
-                    logger?.LogWarning("TryREPLICAOF failed to parse port {port}", portStr);
+                    logger?.LogWarning($"{nameof(TryREPLICAOF)} failed to parse port {{port}}", portStr);
                     while (!RespWriteUtils.WriteError($"ERR REPLICAOF failed to parse port '{portStr}'", ref dcurr, dend))
                         SendAndReset();
                     return true;

libs/common/Memory/LimitedFixedBufferPool.cs

@@ -76,7 +76,7 @@ public unsafe PoolEntry Get(int size)
             if (Interlocked.Increment(ref totalAllocations) < 0)
             {
                 Interlocked.Decrement(ref totalAllocations);
-                logger?.LogError($"Invalid Get on disposed pool");
+                logger?.LogError("Invalid Get on disposed pool");
                 return null;
             }
 

libs/common/Networking/NetworkHandler.cs

@@ -569,7 +569,7 @@ public override void ReturnResponseObject() { }
         public override unsafe bool SendResponse(int offset, int size)
         {
 #if MESSAGETRAGE
-            logger?.LogInformation($"Sending response of size {size} bytes");
+            logger?.LogInformation("Sending response of size {size} bytes", size);
             logger?.LogTrace("SEND: [{send}]", System.Text.Encoding.UTF8.GetString(
                 new Span<byte>(transportSendBuffer).Slice(offset, size)).Replace("\n", "|").Replace("\r", ""));
 #endif
@@ -582,7 +582,7 @@ public override unsafe bool SendResponse(int offset, int size)
         public override void SendResponse(byte[] buffer, int offset, int count, object context)
         {
 #if MESSAGETRAGE
-            logger?.LogInformation($"Sending response of size {count} bytes");
+            logger?.LogInformation("Sending response of size {count} bytes", count);
             logger?.LogTrace("SEND: [{send}]", System.Text.Encoding.UTF8.GetString(
                 new Span<byte>(buffer).Slice(offset, count)).Replace("\n", "|").Replace("\r", ""));
 #endif

libs/host/Configuration/ConfigProviders.cs

@@ -91,16 +91,15 @@ public bool TryImportOptions(string path, IStreamProvider streamProvider, Option
                     MissingMemberHandling = MissingMemberHandling.Error,
                     Error = delegate (object _, ErrorEventArgs args)
                     {
-                        logger?.LogWarning(
-                            $"Encountered an issue when deserializing config file  (Path: {path}): {args.ErrorContext.Error.Message}");
+                        logger?.LogWarning("Encountered an issue when deserializing config file  (Path: {path}): {ErrorMessage}", path, args.ErrorContext.Error.Message);
                         args.ErrorContext.Handled = true;
                     }
                 };
                 JsonConvert.PopulateObject(streamReader.ReadToEnd(), options, settings);
             }
             catch (Newtonsoft.Json.JsonException je)
             {
-                logger?.LogError(je, $"An error occurred while parsing config file (Path: {path}).");
+                logger?.LogError(je, "An error occurred while parsing config file (Path: {path}).", path);
                 return false;
             }
 
@@ -116,7 +115,7 @@ public bool TryExportOptions(string path, IStreamProvider streamProvider, Option
             }
             catch (NotSupportedException e)
             {
-                logger?.LogError(e, $"An error occurred while serializing config file (Path: {path}).");
+                logger?.LogError(e, "An error occurred while serializing config file (Path: {path}).", path);
                 return false;
             }
 

libs/host/Configuration/Options.cs

@@ -474,7 +474,7 @@ public bool IsValid(out List<string> invalidOptions, ILogger logger)
                 foreach (var validationResult in validationResults)
                 {
                     invalidOptions.AddRange(validationResult.MemberNames);
-                    logger?.LogError(validationResult.ErrorMessage);
+                    logger?.LogError("{errorMessage}", validationResult.ErrorMessage);
                 }
             }
 

libs/host/Configuration/Redis/RedisConfigSerializer.cs

@@ -81,7 +81,7 @@ public static RedisOptions Deserialize(StreamReader reader, ILogger logger)
                 var key = line.Substring(0, sepIdx);
                 if (!KeyToProperty.Value.ContainsKey(key))
                 {
-                    logger?.LogWarning($"Redis configuration option not supported: {key}.");
+                    logger?.LogWarning("Redis configuration option not supported: {key}.", key);
                     continue;
                 }
 
@@ -136,7 +136,7 @@ public static RedisOptions Deserialize(StreamReader reader, ILogger logger)
                 var redisOptionAttr = (RedisOptionAttribute)prop.GetCustomAttributes(typeof(RedisOptionAttribute), false).First();
                 if (!string.IsNullOrEmpty(redisOptionAttr.UsageWarning))
                 {
-                    logger?.LogWarning($"Redis configuration option usage warning ({key}): {redisOptionAttr.UsageWarning}");
+                    logger?.LogWarning("Redis configuration option usage warning ({key}): {UsageWarning}", key, redisOptionAttr.UsageWarning);
                 }
             }
 

libs/host/MemoryLogger.cs

@@ -38,7 +38,9 @@ public void FlushLogger(ILogger dstLogger)
         {
             foreach (var entry in this._memoryLog)
             {
+#pragma warning disable CA2254 // Template should be a static expression
                 dstLogger.Log(entry.Item1, entry.Item2, entry.Item3);
+#pragma warning restore CA2254 // Template should be a static expression
             }
             this._memoryLog.Clear();
         }

libs/host/ServerSettingsManager.cs

@@ -86,8 +86,8 @@ internal static bool TryParseCommandLineArguments(string[] args, out Options opt
                 // If any unparsed arguments remain, display a warning to the user
                 if (unparsedArguments.Count > 0)
                 {
-                    logger?.LogWarning(@$"The following command line arguments were not parsed: {string.Join(',', unparsedArguments)}. 
-Please check the syntax of your command. For detailed usage information run with --help.");
+                    logger?.LogWarning(@"The following command line arguments were not parsed: {unparsedArguments}. 
+Please check the syntax of your command. For detailed usage information run with --help.", string.Join(',', unparsedArguments));
                 }
             }
 
@@ -254,8 +254,8 @@ private static bool TryImportServerOptions(string path, ConfigFileType configFil
                 _ => throw new NotImplementedException()
             };
 
-            logger?.Log(importSucceeded ? LogLevel.Information : LogLevel.Error,
-                $"Configuration import from {fileLocation} {(importSucceeded ? "succeeded" : "failed")}. Path: {path}.");
+            logger?.Log(importSucceeded ? LogLevel.Information : LogLevel.Error, "Configuration import from {fileLocation} {importSucceeded}. Path: {path}.",
+                fileLocation, importSucceeded ? "succeeded" : "failed", path);
 
             return importSucceeded;
         }
@@ -287,8 +287,8 @@ private static bool TryExportServerOptions(string path, ConfigFileType configFil
                 _ => throw new NotImplementedException()
             };
 
-            logger?.Log(exportSucceeded ? LogLevel.Information : LogLevel.Error,
-                $"Configuration export to {fileLocation} {(exportSucceeded ? "succeeded" : "failed")}. File path: {path}.");
+            logger?.Log(exportSucceeded ? LogLevel.Information : LogLevel.Error, "Configuration export to {fileLocation} {exportSucceeded}. File path: {path}.",
+                fileLocation, exportSucceeded ? "succeeded" : "failed", path);
             return exportSucceeded;
         }
 

libs/server/Auth/GarnetAadAuthenticator.cs

@@ -75,7 +75,7 @@ public bool Authenticate(ReadOnlySpan<byte> password, ReadOnlySpan<byte> usernam
                 _validateTo = token.ValidTo;
 
                 _authorized = IsIdentityAuthorized(identity, username);
-                _logger?.LogInformation($"Authentication successful. Token valid from {_validFrom} to {_validateTo}");
+                _logger?.LogInformation("Authentication successful. Token valid from {validFrom} to {validateTo}", _validFrom, _validateTo);
 
                 return IsAuthorized();
             }

libs/server/Metrics/GarnetServerMonitor.cs

@@ -213,7 +213,7 @@ private void ResetLatencyMetrics()
                 {
                     if (resetLatencyMetrics[eventType])
                     {
-                        logger?.LogInformation($"Resetting server-side stats {eventType}");
+                        logger?.LogInformation("Resetting server-side stats {eventType}", eventType);
 
                         var sessions = ((GarnetServerBase)server).ActiveConsumers();
                         foreach (var entry in sessions)

libs/server/Resp/BasicCommands.cs

@@ -1508,8 +1508,7 @@ void FlushDb(RespCommand cmd)
             else
                 ExecuteFlushDb(unsafeTruncateLog);
 
-            logger?.LogInformation(
-                $"Running {nameof(cmd)} {(async ? "async" : "sync")} {(unsafeTruncateLog ? " with unsafetruncatelog." : string.Empty)}");
+            logger?.LogInformation($"Running {nameof(cmd)} {{async}} {{mode}}", async ? "async" : "sync", unsafeTruncateLog ? " with unsafetruncatelog." : string.Empty);
             while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_OK, ref dcurr, dend))
                 SendAndReset();
         }

libs/server/Resp/RespCommandsInfoProvider.cs

@@ -107,7 +107,7 @@ public bool TryImportRespCommandsInfo(string path, IStreamProvider streamProvide
             }
             catch (JsonException je)
             {
-                logger?.LogError(je, $"An error occurred while parsing resp commands info file (Path: {path}).");
+                logger?.LogError(je, "An error occurred while parsing resp commands info file (Path: {path}).", path);
                 return false;
             }
 
@@ -125,7 +125,7 @@ public bool TryExportRespCommandsInfo(string path, IStreamProvider streamProvide
             }
             catch (NotSupportedException e)
             {
-                logger?.LogError(e, $"An error occurred while serializing resp commands info file (Path: {path}).");
+                logger?.LogError(e, "An error occurred while serializing resp commands info file (Path: {path}).", path);
                 return false;
             }