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

Implement ExecCliBuild build check to warn if the Exec task is used to build a project #11523

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions documentation/specs/BuildCheck/Codes.md
Original file line number Diff line number Diff line change
@@ -12,6 +12,7 @@ Report codes are chosen to conform to suggested guidelines. Those guidelines are
| [BC0106](#bc0106---copytooutputdirectoryalways-should-be-avoided) | Warning | N/A | 9.0.200 | CopyToOutputDirectory='Always' should be avoided. |
| [BC0107](#bc0107---targetframework-and-targetframeworks-specified-together) | Warning | N/A | 9.0.200 | TargetFramework and TargetFrameworks specified together. |
| [BC0108](#bc0108---targetframework-or-targetframeworks-specified-in-non-sdk-style-project) | Warning | N/A | 9.0.300 | TargetFramework or TargetFrameworks specified in non-SDK style project. |
| [BC0109](#bc0109---building-using-the-exec-task) | Warning | N/A | 9.0.300 | Building using the Exec task. |
| [BC0201](#bc0201---usage-of-undefined-property) | Warning | Project | 9.0.100 | Usage of undefined property. |
| [BC0202](#bc0202---property-first-declared-after-it-was-used) | Warning | Project | 9.0.100 | Property first declared after it was used. |
| [BC0203](#bc0203----property-declared-but-never-used) | None | Project | 9.0.100 | Property declared but never used. |
@@ -137,6 +138,13 @@ dotnet build my-multi-target.csproj /p:TargetFramework=net9.0

Make sure the Target Framework is specified appropriately for your project.

<a name="BC0109"></a>
## BC0109 - Building using the Exec task.

"The 'Exec' task should not be used to build projects."

Building projects using the dotnet/msbuild/nuget CLI in the `Exec` task is not recommended, as it spawns a separate build process that the MSBuild engine cannot track. Please use the [MSBuild task](https://learn.microsoft.com/visualstudio/msbuild/msbuild-task) instead.


<a name="BC0201"></a>
## BC0201 - Usage of undefined property.
245 changes: 245 additions & 0 deletions src/Build/BuildCheck/Checks/ExecCliBuildCheck.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,245 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections.Generic;
#if !FEATURE_MSIOREDIST
using System.IO;
#endif
using Microsoft.Build.Shared;

#if FEATURE_MSIOREDIST
using Path = Microsoft.IO.Path;
#endif

namespace Microsoft.Build.Experimental.BuildCheck.Checks;

internal sealed class ExecCliBuildCheck : Check
{
public static CheckRule SupportedRule = new CheckRule(
"BC0109",
"ExecCliBuild",
ResourceUtilities.GetResourceString("BuildCheck_BC0109_Title")!,
ResourceUtilities.GetResourceString("BuildCheck_BC0109_MessageFmt")!,
new CheckConfiguration() { Severity = CheckResultSeverity.Warning });

private const string ExecTaskName = "Exec";
private const string CommandParameterName = "Command";

private static readonly char[] s_knownCommandSeparators = ['&', ';', '|'];

private static readonly KnownBuildCommand[] s_knownBuildCommands =
[
new KnownBuildCommand("dotnet build"),
new KnownBuildCommand("dotnet clean"),
new KnownBuildCommand("dotnet msbuild"),
new KnownBuildCommand("dotnet restore"),
new KnownBuildCommand("dotnet publish"),
new KnownBuildCommand("dotnet pack"),
new KnownBuildCommand("dotnet vstest"),
new KnownBuildCommand("nuget restore"),
new KnownBuildCommand("msbuild", excludedSwitches: ["version", "ver", "help", "h", "?"]),
new KnownBuildCommand("dotnet test"),
new KnownBuildCommand("dotnet run"),
];

public override string FriendlyName => "MSBuild.ExecCliBuildCheck";

internal override bool IsBuiltIn => true;

public override IReadOnlyList<CheckRule> SupportedRules { get; } = [SupportedRule];

public override void Initialize(ConfigurationContext configurationContext)
{
/* This is it - no custom configuration */
}

public override void RegisterActions(IBuildCheckRegistrationContext registrationContext)
{
registrationContext.RegisterTaskInvocationAction(TaskInvocationAction);
}

private static void TaskInvocationAction(BuildCheckDataContext<TaskInvocationCheckData> context)
{
if (context.Data.TaskName == ExecTaskName
&& context.Data.Parameters.TryGetValue(CommandParameterName, out TaskInvocationCheckData.TaskParameter? commandArgument))
{
var execCommandValue = commandArgument.Value?.ToString() ?? string.Empty;

var commandSpan = execCommandValue.AsSpan();
int start = 0;

while (start < commandSpan.Length)
{
var nextSeparatorIndex = commandSpan.Slice(start, commandSpan.Length - start).IndexOfAny(s_knownCommandSeparators);

if (nextSeparatorIndex == -1)
{
if (TryGetMatchingKnownBuildCommand(commandSpan, out var knownBuildCommand))
{
context.ReportResult(BuildCheckResult.CreateBuiltIn(
SupportedRule,
context.Data.TaskInvocationLocation,
context.Data.TaskName,
Path.GetFileName(context.Data.ProjectFilePath),
knownBuildCommand.ToolName));
}

break;
}
else
{
var command = commandSpan.Slice(start, nextSeparatorIndex);

if (TryGetMatchingKnownBuildCommand(command, out var knownBuildCommand))
{
context.ReportResult(BuildCheckResult.CreateBuiltIn(
SupportedRule,
context.Data.TaskInvocationLocation,
context.Data.TaskName,
Path.GetFileName(context.Data.ProjectFilePath),
knownBuildCommand.ToolName));

break;
}

start += nextSeparatorIndex + 1;
}
}
}
}

private static bool TryGetMatchingKnownBuildCommand(ReadOnlySpan<char> command, out KnownBuildCommand knownBuildCommand)
{
Span<char> normalizedCommand = stackalloc char[command.Length];
int normalizedCommandIndex = 0;

foreach (var c in command)
{
if (char.IsWhiteSpace(c) && (normalizedCommandIndex == 0 || char.IsWhiteSpace(normalizedCommand[normalizedCommandIndex - 1])))
{
continue;
}

normalizedCommand[normalizedCommandIndex++] = c;
}

foreach (var buildCommand in s_knownBuildCommands)
{
if (buildCommand.IsMatch(normalizedCommand))
{
knownBuildCommand = buildCommand;
return true;
}
}

knownBuildCommand = default;
return false;
}

private readonly record struct KnownBuildCommand
{
private static readonly string[] s_knownSwitchPrefixes = ["/", "--", "-"];

private readonly string _knownBuildCommand;
private readonly string[] _excludedSwitches = [];

public KnownBuildCommand(string knownBuildCommand)
{
if (string.IsNullOrEmpty(knownBuildCommand))
{
throw new ArgumentNullException(nameof(knownBuildCommand));
}

_knownBuildCommand = knownBuildCommand;
}

public KnownBuildCommand(string knownBuildCommand, string[] excludedSwitches)
: this(knownBuildCommand)
{
_excludedSwitches = excludedSwitches;
}

public string ToolName
{
get
{
int nextSpaceIndex = _knownBuildCommand.IndexOf(' ');

return nextSpaceIndex == -1
? _knownBuildCommand
: _knownBuildCommand.AsSpan().Slice(0, nextSpaceIndex).ToString();
}
}

public bool IsMatch(ReadOnlySpan<char> execCommand)
{
if (!execCommand.StartsWith(_knownBuildCommand.AsSpan(), StringComparison.OrdinalIgnoreCase))
{
return false;
}

if (_excludedSwitches.Length == 0 || execCommand.Length == _knownBuildCommand.Length)
{
return true;
}

return !ContainsExcludedArguments(execCommand);
}

private bool ContainsExcludedArguments(ReadOnlySpan<char> execCommand)
{
int start = _knownBuildCommand.Length + 1;

while (start < execCommand.Length)
{
int nextSpaceIndex = execCommand.Slice(start).IndexOf(' ');

if (nextSpaceIndex == -1)
{
var argument = execCommand.Slice(start);

if (EqualsToAnyExcludedArguments(argument))
{
return true;
}

break;
}
else
{
var argument = execCommand.Slice(start, nextSpaceIndex);

if (EqualsToAnyExcludedArguments(argument))
{
return true;
}

start += nextSpaceIndex + 1;
}
}

return false;
}

private bool EqualsToAnyExcludedArguments(ReadOnlySpan<char> argument)
{
foreach (var knownSwitch in s_knownSwitchPrefixes)
{
if (argument.StartsWith(knownSwitch.AsSpan()))
{
foreach (var excludedSwitch in _excludedSwitches)
{
if (argument.EndsWith(excludedSwitch.AsSpan(), StringComparison.OrdinalIgnoreCase)
&& argument.Length == knownSwitch.Length + excludedSwitch.Length)
{
return true;
}
}
}
}

return false;
}
}
}
Original file line number Diff line number Diff line change
@@ -149,6 +149,7 @@ internal readonly record struct BuiltInCheckFactory(
new BuiltInCheckFactory([PreferProjectReferenceCheck.SupportedRule.Id], PreferProjectReferenceCheck.SupportedRule.DefaultConfiguration.IsEnabled ?? false, Construct<PreferProjectReferenceCheck>),
new BuiltInCheckFactory([CopyAlwaysCheck.SupportedRule.Id], CopyAlwaysCheck.SupportedRule.DefaultConfiguration.IsEnabled ?? false, Construct<CopyAlwaysCheck>),
new BuiltInCheckFactory([DoubleWritesCheck.SupportedRule.Id], DoubleWritesCheck.SupportedRule.DefaultConfiguration.IsEnabled ?? false, Construct<DoubleWritesCheck>),
new BuiltInCheckFactory([ExecCliBuildCheck.SupportedRule.Id], ExecCliBuildCheck.SupportedRule.DefaultConfiguration.IsEnabled ?? false, Construct<ExecCliBuildCheck>),
new BuiltInCheckFactory([NoEnvironmentVariablePropertyCheck.SupportedRule.Id], NoEnvironmentVariablePropertyCheck.SupportedRule.DefaultConfiguration.IsEnabled ?? false, Construct<NoEnvironmentVariablePropertyCheck>),
new BuiltInCheckFactory([EmbeddedResourceCheck.SupportedRule.Id], EmbeddedResourceCheck.SupportedRule.DefaultConfiguration.IsEnabled ?? false, Construct<EmbeddedResourceCheck>),
new BuiltInCheckFactory([TargetFrameworkConfusionCheck.SupportedRule.Id], TargetFrameworkConfusionCheck.SupportedRule.DefaultConfiguration.IsEnabled ?? false, Construct<TargetFrameworkConfusionCheck>),
6 changes: 6 additions & 0 deletions src/Build/Resources/Strings.resx
Original file line number Diff line number Diff line change
@@ -2206,6 +2206,12 @@ Utilization: {0} Average Utilization: {1:###.0}</value>
<value>Project {0} specifies 'TargetFramework(s)' property '{1}', which does not use the .NET SDK. Those properties are not understood by projects that import C# targets directly.</value>
<comment>Terms in quotes are not to be translated.</comment>
</data>
<data name="BuildCheck_BC0109_Title" xml:space="preserve">
<value>The 'Exec' task should not be used to build a project.</value>
</data>
<data name="BuildCheck_BC0109_MessageFmt" xml:space="preserve">
<value>Task {0} from project {1} builds a project using the {2} CLI. The MSBuild task should be used instead.</value>
</data>
<data name="BuildCheck_BC0201_Title" xml:space="preserve">
<value>A property that is accessed should be declared first.</value>
</data>
10 changes: 10 additions & 0 deletions src/Build/Resources/xlf/Strings.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Build/Resources/xlf/Strings.de.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Build/Resources/xlf/Strings.es.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Build/Resources/xlf/Strings.fr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Build/Resources/xlf/Strings.it.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Build/Resources/xlf/Strings.ja.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading