FailureLevels commandline option does not fully override XML config if both exists

[shaopeng-gh]

Using:

commandline option

... -o C:\temp.sarif --log ForceOverwrite --level Error;Warning;Note -c "C:\config.xml"

XML config config.xml contains

<Property Key="FailureLevels" Type="FailureLevelSet">
			<Item>Warning</Item>
			<Item>Error</Item>
		</Property>

The result

should be effective Error;Warning;Note , but Error;Warning
The issue is at \src\sarif-sdk\src\Sarif.Driver\Sdk\MultithreadedAnalyzeCommandBase.cs

// Now that our context if fully initialized, we can create
            // the actual loggers used to complete analysis.
            context.Logger = savedLogger;
            context.Logger ??= InitializeLogger(context);

is after
context.FailureLevels = options.Level != null ? options.FailureLevels : context.FailureLevels;

so InitializeLogger(context) will reset to the XML one and the options.Level is not applied.

[michaelcfanning]

?. In this case, context.FailureLevels should be set properly and used in the Initialize method, correct? I thought you'd found a bug in our context override logic and wrote the beginning of a test for this scenario, but it doesn't fail.

microsoft/sarif-sdk#2707

[shaopeng-gh]

@michaelcfanning

See comment with "Shaopeng" below:

// First, we initialize data values that impact loggers, so that we can
            // pass accurate values to the console logger.
            context.Quiet = options.Quiet != null ? options.Quiet.Value : context.Quiet;
            context.ResultKinds = options.Kind != null ? options.ResultKinds : context.ResultKinds;
            context.FailureLevels = options.Level != null ? options.FailureLevels : context.FailureLevels;
// Shaopeng: here we override with command line, we now will have Note, but it is too early.

            // We initialize a temporary console logger that's used strictly to emit
            // diagnostics output while we load/initialize various configurations settings.
            IAnalysisLogger savedLogger = context.Logger;
            context.Logger = new ConsoleLogger(quietConsole: true,
                                               levels: BaseLogger.ErrorWarningNote,
                                               kinds: BaseLogger.Fail,
                                               toolName: Tool.Driver.Name);

            // Next, we initialize ourselves from disk-based configuration, 
            // if specified. This allows users to operate against configuration
            // XML but to override specific settings within it via options.
            context = InitializeConfiguration(options.ConfigurationFilePath, context);
// Shaopeng: This re-creates context, from disk setting which is not including Note.


            // Now that our context if fully initialized, we can create
            // the actual loggers used to complete analysis.
            context.Logger = savedLogger;
            context.Logger ??= InitializeLogger(context);
// Shaopeng: the savedLogger is NULL, so we re-create, and the context is not including Note, from previous comment. Then it leads to the screenshot.

The full config file is

<?xml version="1.0" encoding="utf-8"?>
<Properties Type="PropertiesDictionary">
	<Properties Key="CoreSettings.Options" Type="PropertiesDictionary">
		<!--One or more failure levels to persist to loggers. Valid values include Error, Warning, and Informational. Defaults to 'Error' and 'Warning'.-->
		<Property Key="FailureLevels" Type="FailureLevelSet">
			<Item>Warning</Item>
			<Item>Error</Item>
		</Property>
	</Properties>
</Properties>

[michaelcfanning]

Oh, I see. Will take a look.