Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Analyzers prototyping - Opting in/out #9723

Closed
Tracked by #9627
JanKrivanek opened this issue Feb 9, 2024 · 2 comments · Fixed by #10033
Closed
Tracked by #9627

Analyzers prototyping - Opting in/out #9723

JanKrivanek opened this issue Feb 9, 2024 · 2 comments · Fixed by #10033
Assignees
Labels
Area: BuildCheck Epic Groups multiple user stories. Can be grouped under a theme. triaged

Comments

@JanKrivanek
Copy link
Member

JanKrivanek commented Feb 9, 2024

Context

#9627

Users should be able to turn off or on the anlyzers run globally (in order to speedup or troubleshoot problematci runs), regardless of what will be the default behavior. At the same time MSBuild team should have a default behavior (likely it's opt-in-only in initial versions, then opt-out-only later on), which might as well be influenced by concrete run scenario (sdk run, msbuild.exe run, API invoking, binlog replay, etc.)

As part of this item one should propose and find consensus for opting in and out of anlyzing behavior. An inspiration might be a TerminalLogger opt -in/-out behavior (but hopefully we should start more simple).
Sample initial behavior might be as follows:

  • Off by default in all scenarios
  • Env var for overriding the default behavior
  • Cmd switch for overriding default or env

Reference

  • TerminalLogger opt-in/out code:

    msbuild/src/MSBuild/XMake.cs

    Lines 2715 to 2910 in e71eb7a

    private static bool ProcessTerminalLoggerConfiguration(CommandLineSwitches commandLineSwitches, out string aggregatedParameters)
    {
    aggregatedParameters = AggregateParameters(commandLineSwitches);
    string defaultValue = FindDefaultValue(aggregatedParameters);
    string terminalLoggerArg = null;
    if (!TryFromCommandLine(commandLineSwitches) && !TryFromEnvironmentVariables())
    {
    ApplyDefault();
    }
    terminalLoggerArg = NormalizeIntoBooleanValues();
    bool useTerminalLogger = false;
    if (!TrueOrFalse())
    {
    ItMustBeAuto();
    }
    return KnownTelemetry.LoggingConfigurationTelemetry.TerminalLogger = useTerminalLogger;
    static bool CheckIfTerminalIsSupportedAndTryEnableAnsiColorCodes()
    {
    (var acceptAnsiColorCodes, var outputIsScreen, s_originalConsoleMode) = NativeMethodsShared.QueryIsScreenAndTryEnableAnsiColorCodes();
    if (!outputIsScreen)
    {
    s_globalMessagesToLogInBuildLoggers.Add(
    new BuildManager.DeferredBuildMessage(ResourceUtilities.GetResourceString("TerminalLoggerNotUsedRedirected"), MessageImportance.Low));
    return false;
    }
    // TerminalLogger is not used if the terminal does not support ANSI/VT100 escape sequences.
    if (!acceptAnsiColorCodes)
    {
    s_globalMessagesToLogInBuildLoggers.Add(
    new BuildManager.DeferredBuildMessage(ResourceUtilities.GetResourceString("TerminalLoggerNotUsedNotSupported"), MessageImportance.Low));
    return false;
    }
    if (Traits.Instance.EscapeHatches.EnsureStdOutForChildNodesIsPrimaryStdout)
    {
    s_globalMessagesToLogInBuildLoggers.Add(
    new BuildManager.DeferredBuildMessage(ResourceUtilities.GetResourceString("TerminalLoggerNotUsedDisabled"), MessageImportance.Low));
    return false;
    }
    return true;
    }
    string FindDefaultValue(string s)
    {
    // Find default configuration so it is part of telemetry even when default is not used.
    // Default can be stored in /tlp:default=true|false|on|off|auto
    string terminalLoggerDefault = null;
    foreach (string parameter in s.Split(MSBuildConstants.SemicolonChar))
    {
    if (string.IsNullOrWhiteSpace(parameter))
    {
    continue;
    }
    string[] parameterAndValue = parameter.Split(MSBuildConstants.EqualsChar);
    if (parameterAndValue[0].Equals("default", StringComparison.InvariantCultureIgnoreCase) && parameterAndValue.Length > 1)
    {
    terminalLoggerDefault = parameterAndValue[1];
    }
    }
    if (terminalLoggerDefault == null)
    {
    terminalLoggerDefault = bool.FalseString;
    KnownTelemetry.LoggingConfigurationTelemetry.TerminalLoggerDefault = bool.FalseString;
    KnownTelemetry.LoggingConfigurationTelemetry.TerminalLoggerDefaultSource = "msbuild";
    }
    else
    {
    // Lets check DOTNET CLI env var
    string dotnetCliEnvVar = Environment.GetEnvironmentVariable("DOTNET_CLI_CONFIGURE_MSBUILD_TERMINAL_LOGGER");
    KnownTelemetry.LoggingConfigurationTelemetry.TerminalLoggerDefault = terminalLoggerDefault;
    KnownTelemetry.LoggingConfigurationTelemetry.TerminalLoggerDefaultSource = string.IsNullOrWhiteSpace(dotnetCliEnvVar) ? "sdk" : "DOTNET_CLI_CONFIGURE_MSBUILD_TERMINAL_LOGGER";
    }
    return terminalLoggerDefault;
    }
    bool TryFromCommandLine(CommandLineSwitches commandLineSwitches1)
    {
    if (!commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.TerminalLogger))
    {
    return false;
    }
    // There's a switch set, but there might be more than one
    string[] switches = commandLineSwitches1[CommandLineSwitches.ParameterizedSwitch.TerminalLogger];
    terminalLoggerArg = switches[switches.Length - 1];
    // if the switch was set but not to an explicit value, the value is "auto"
    if (string.IsNullOrEmpty(terminalLoggerArg))
    {
    terminalLoggerArg = "auto";
    }
    KnownTelemetry.LoggingConfigurationTelemetry.TerminalLoggerUserIntent = terminalLoggerArg ?? string.Empty;
    KnownTelemetry.LoggingConfigurationTelemetry.TerminalLoggerUserIntentSource = "arg";
    return true;
    }
    bool TryFromEnvironmentVariables()
    {
    // Keep MSBUILDLIVELOGGER supporitng existing use. But MSBUILDTERMINALLOGGER takes precedence.
    string liveLoggerArg = Environment.GetEnvironmentVariable("MSBUILDLIVELOGGER");
    terminalLoggerArg = Environment.GetEnvironmentVariable("MSBUILDTERMINALLOGGER");
    if (!string.IsNullOrEmpty(terminalLoggerArg))
    {
    s_globalMessagesToLogInBuildLoggers.Add(
    new BuildManager.DeferredBuildMessage($"The environment variable MSBUILDTERMINALLOGGER was set to {terminalLoggerArg}.", MessageImportance.Low));
    KnownTelemetry.LoggingConfigurationTelemetry.TerminalLoggerUserIntent = terminalLoggerArg;
    KnownTelemetry.LoggingConfigurationTelemetry.TerminalLoggerUserIntentSource = "MSBUILDTERMINALLOGGER";
    }
    else if (!string.IsNullOrEmpty(liveLoggerArg))
    {
    terminalLoggerArg = liveLoggerArg;
    s_globalMessagesToLogInBuildLoggers.Add(
    new BuildManager.DeferredBuildMessage($"The environment variable MSBUILDLIVELOGGER was set to {liveLoggerArg}.", MessageImportance.Low));
    KnownTelemetry.LoggingConfigurationTelemetry.TerminalLoggerUserIntent = terminalLoggerArg;
    KnownTelemetry.LoggingConfigurationTelemetry.TerminalLoggerUserIntentSource = "MSBUILDLIVELOGGER";
    }
    else
    {
    return false;
    }
    return true;
    }
    string NormalizeIntoBooleanValues()
    {
    // We now have a string`. It can be "true" or "false" which means just that:
    if (terminalLoggerArg.Equals("on", StringComparison.InvariantCultureIgnoreCase))
    {
    terminalLoggerArg = bool.TrueString;
    }
    else if (terminalLoggerArg.Equals("off", StringComparison.InvariantCultureIgnoreCase))
    {
    terminalLoggerArg = bool.FalseString;
    }
    return terminalLoggerArg;
    }
    void ApplyDefault()
    {
    terminalLoggerArg = defaultValue;
    }
    string AggregateParameters(CommandLineSwitches switches)
    {
    string[] terminalLoggerParameters = switches[CommandLineSwitches.ParameterizedSwitch.TerminalLoggerParameters];
    return terminalLoggerParameters?.Length > 0 ? MSBuildApp.AggregateParameters(string.Empty, terminalLoggerParameters) : string.Empty;
    }
    bool TrueOrFalse()
    {
    if (bool.TryParse(terminalLoggerArg, out bool result))
    {
    useTerminalLogger = result;
    // Try Enable Ansi Color Codes when terminal logger is enabled/enforced.
    if (result)
    {
    // This needs to be called so Ansi Color Codes are enabled for the terminal logger.
    (_, _, s_originalConsoleMode) = NativeMethodsShared.QueryIsScreenAndTryEnableAnsiColorCodes();
    }
    return true;
    }
    return false;
    }
    void ItMustBeAuto()
    {
    // or it can be "auto", meaning "enable if we can"
    if (!terminalLoggerArg.Equals("auto", StringComparison.OrdinalIgnoreCase))
    {
    CommandLineSwitchException.Throw("InvalidTerminalLoggerValue", terminalLoggerArg);
    }
    useTerminalLogger = CheckIfTerminalIsSupportedAndTryEnableAnsiColorCodes();
    }
    }

FYI @baronfel to express opinions on desired behavior

@JanKrivanek JanKrivanek mentioned this issue Feb 9, 2024
14 tasks
@ladipro ladipro self-assigned this Feb 14, 2024
@baronfel
Copy link
Member

Roslyn analyzers have two opt-in modes:

  • for NuGet-delivered Analyzers, the act of adding the PackageReference adds the Analyzer as well
  • for .NET SDK-delivered Analyzers, there are boolean MSBuild properties that control if the analyzer is used

in both cases, as long as there are any analyzers requested, analysis occurs. This model is ideally what I would like us to use, as users are already trained in this mode of use.

@JanKrivanek
Copy link
Member Author

@baronfel - can you elaborate more on the boolean MSBuild properties for the built-in analyzers?

Inline with Roslyn - Analyzers have default state (enabled/disabled) hardcoded, plus it is overridable via .editorconfig.
Is the msbuild prop an override of both of those (so that one can tune the behavior in different environments - CI/dev - just via commandline, without need to touch any code)?

@ladipro ladipro closed this as completed in a8e224f May 9, 2024
@baronfel baronfel added the Epic Groups multiple user stories. Can be grouped under a theme. label Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: BuildCheck Epic Groups multiple user stories. Can be grouped under a theme. triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants