False Positive CSharp - S3457 - when using a property with back field and initialize by a constant?

Rules and Languages Report False-positive / False-negative...
,
1

SonarQube Server: Community Build v25.5.0.107428
No information available about configuration, not maintained by us.

public const String TRACE_FORMAT = "{1:yyyy-MM-dd HH:mm:ss.fff}Z - {2} ({3}) : {4} ({5}) - {6} - {7}";

private static String __TraceFormat = TRACE_FORMAT;

public static String TraceFormat
{
  get => __TraceFormat;
  set
  {
    if(String.IsNullOrWhiteSpace(value))
    {
      value = TRACE_FORMAT;
    }

    __TraceFormat = value;
  }
}

private void WriteLine(Int32 depth,
                       TraceEventType traceEventType,
                       String message,
                       Int32 id)
{
  try
  {
    MethodBase methodBase = null;
    String fullName = null;

    traceEventType.TestEnumValueThrowArgumentOutOfRangeException(nameof(traceEventType));

    message = message ?? "NULL";

    methodBase = new StackTrace().GetFrame(depth)
                                 .GetMethod();

    fullName = $"{methodBase.ReflectedType.FullName.Replace("+", ".")}.{methodBase.Name}";

    _ = this.GetTraceSources(fullName)
            .Where(ts => ts.Switch
                           .Level
                           .HasFlag((SourceLevels)traceEventType))
            .ForEach(ts => ts.Listeners
                             .Cast<TraceListener>()
                             .ToList()
                             .ForEach(tl => tl.WriteLine(String.Format(__TraceFormat,
                                                                       DateTime.Now,
                                                                       DateTime.UtcNow,
                                                                       ProcessName,
                                                                       ProcessId,
                                                                       traceEventType,
                                                                       id,
                                                                       fullName,
                                                                       message,
                                                                       tl.Name,
                                                                       ts.Name))));
  }
  catch(Exception exception)
  {
    this.WriteException(exception);
  }
}

It reports the value of __TraceFormat in WriteLine. Yes there is no 0 indexed value in the initial string. __TraceFormat get initialized with TRACE_FORMAT, which don’t contain a placeholder for index 0. But the developer is able to change the __TraceFormat by static property TraceFormat and then can use the index 0 also.

3

I would argue that this code is hard to read. What you could do, both to improve readability, and bypass S3457 is the following:

internal class S3457
{
	public const string TRACE_FORMAT = "{0:yyyy-MM-dd HH:mm:ss.fff}Z - {1} ({2}) : {3} ({4}) - {5} - {6}";

	public static string TraceFormat { get; set; }

	private void WriteLine(int depth, TraceEventType traceEventType, string message, int id)
	{
		try
		{
			MethodBase methodBase = null;
			string fullName = null;

			traceEventType.TestEnumValueThrowArgumentOutOfRangeException(nameof(traceEventType));

			message = message ?? "NULL";

			methodBase = new StackTrace().GetFrame(depth).GetMethod();

			fullName = $"{methodBase.ReflectedType.FullName.Replace("+", ".")}.{methodBase.Name}";

			_ = this.GetTraceSources(fullName)
					.Where(ts => ts.Switch
								   .Level
								   .HasFlag((SourceLevels)traceEventType))
					.ForEach(ts => ts.Listeners
									 .Cast<TraceListener>()
									 .ToList()
									 .ForEach(tl => tl.WriteLine(Format()));
		}
		catch (Exception exception)
		{
			this.WriteException(exception);
		}

		string Format() => TraceFormat is { Length: > 0 }
		? string.Format(TraceFormat, DateTime.Now, DateTime.UtcNow, ProcessName, ProcessId, traceEventType, id, fullName, message, tl.Name, ts.Name)
		: string.Format(TRACE_FORMAT, DateTime.UtcNow, ProcessName, ProcessId, traceEventType, id, fullName, message, tl.Name, ts.Name);
	}
}

By doing so, you do not need an underlying value or setter logic. Is clear from the Format() method that once specified, you can switch between DateTime.Now and DateTime.UtcNow, but if not the latter is applied. You even could (and may be should) use string interpolation for the latter case.

I would not advise Sonar to change the behavior of this rule.

4

Thanks for sharing. On other hand your approach increases the complexity from your SonarQube measure point of view.