Skip to content

Commit

Permalink
Merge branch 'main' into darc-main-4216ece9-8b23-41ca-96aa-6e814fc098c2
Browse files Browse the repository at this point in the history
  • Loading branch information
surayya-MS authored Nov 27, 2024
2 parents 0e545ad + bc2ad7f commit 7dcd892
Show file tree
Hide file tree
Showing 27 changed files with 720 additions and 34 deletions.
26 changes: 23 additions & 3 deletions documentation/specs/BuildCheck/Codes.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,18 @@ Report codes are chosen to conform to suggested guidelines. Those guidelines are

| Diagnostic Code | Default Severity | Default Scope | Available from SDK | Reason |
|:-----|-------|-------|-------|----------|
| [BC0101](#bc0101---shared-output-path) | Warning | Project | 9.0.100 | Shared output path. |
| [BC0102](#bc0102---double-writes) | Warning | Project | 9.0.100 | Double writes. |
| [BC0101](#bc0101---shared-output-path) | Warning | N/A | 9.0.100 | Shared output path. |
| [BC0102](#bc0102---double-writes) | Warning | N/A | 9.0.100 | Double writes. |
| [BC0103](#bc0103---used-environment-variable) | Suggestion | Project | 9.0.100 | Used environment variable. |
| [BC0104](#bc0104---projectreference-is-preferred-to-reference) | Warning | Project | 9.0.200 | ProjectReference is preferred to Reference. |
| [BC0104](#bc0104---projectreference-is-preferred-to-reference) | Warning | N/A | 9.0.200 | ProjectReference is preferred to Reference. |
| [BC0105](#bc0105---embeddedresource-should-specify-culture-metadata) | Warning | N/A | 9.0.200 | Culture specific EmbeddedResource should specify Culture metadata. |
| [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) | Suggestion | Project | 9.0.100 | Property declared but never used. |


Note: What does the 'N/A' scope mean? The scope of checks are only applicable and configurable in cases where evaluation-time data are being used and the source of the data is determinable and available. Otherwise the scope of whole build is always checked.

To enable verbose logging in order to troubleshoot issue(s), enable [binary logging](https://github.com/dotnet/msbuild/blob/main/documentation/wiki/Binary-Log.md#msbuild-binary-log-overview)

_Cmd:_
Expand Down Expand Up @@ -58,6 +61,23 @@ It is not recommended to reference project outputs. Such practice leads to losin

If you need to achieve more advanced dependency behavior - check [Controlling Dependencies Behavior](https://github.com/dotnet/msbuild/blob/main/documentation/wiki/Controlling-Dependencies-Behavior.md) document. If neither suits your needs - then you might need to disable this check for your build or for particular projects.

<a name="BC0105"></a>
## BC0105 - EmbeddedResource should specify Culture metadata.

"It is recommended to specify explicit 'Culture' metadata, or 'WithCulture=false' metadata with 'EmbeddedResource' item in order to avoid wrong or nondeterministic culture estimation."

[`EmbeddedResource` item](https://learn.microsoft.com/en-us/visualstudio/msbuild/common-msbuild-project-items#embeddedresource) has a `Culture` and `WithCulture` metadata that are strongly recommended to be used - to prevent MSBuild to need to 'guess' the culture from the file extension - which may be dependent on the current OS/Runtime available cultures and hence it can lead to nondeterministic build.

Examples:
* `<EmbeddedResource Update = "Resource1.xyz.resx" Culture="xyz" />` This indicates the culture to the MSBuild engine and the culture will be respected. No diagnostic (warning) is issued ([see below for exceptions](#RespectAlreadyAssignedItemCulture)).
* `<EmbeddedResource Update = "Resource1.xyz.resx" WithCulture="false" />` This indicates to the MSBuild engine that the file is culture neutral and the extension should not be treated as culture indicator. No diagnostic (warning) is issued.
* `<EmbeddedResource Update = "Resource1.xyz.resx" />` MSBuild infers the culture from the extra extension ('xyz') and if it is known to [`System.Globalization.CultureInfo`](https://learn.microsoft.com/en-us/dotnet/api/system.globalization.cultureinfo) it is being used as the resource culture. The `BC0105` diagnostic is emitted (if BuildCheck is enabled and BC0105 is not disabled)
* `<EmbeddedResource Update = "Resource1.resx" />` MSBuild infers that the resource is culture neutral. No diagnostic (warning) is issued.

<a name="RespectAlreadyAssignedItemCulture"></a>
**Note:** In Full Framework version of MSBuild (msbuild.exe, Visual Studio) and in .NET SDK prior 9.0 a global or project specific property `RespectAlreadyAssignedItemCulture` needs to be set to `'true'` in order for the explicit `Culture` metadata to be respected. Otherwise the explicit culture will be overwritten by MSBuild engine and if different from the extension - a `MSB3002` warning is emitted (`"MSB3002: Explicitly set culture "{0}" for item "{1}" was overwritten with inferred culture "{2}", because 'RespectAlreadyAssignedItemCulture' property was not set."`)


<a name="BC0201"></a>
## BC0201 - Usage of undefined property.

Expand Down
4 changes: 2 additions & 2 deletions eng/Version.Details.xml
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,9 @@
<Uri>https://github.com/dotnet/arcade</Uri>
<Sha>1c7e09a8d9c9c9b15ba574cd6a496553505559de</Sha>
</Dependency>
<Dependency Name="NuGet.Build.Tasks" Version="6.13.0-preview.1.62">
<Dependency Name="NuGet.Build.Tasks" Version="6.13.0-preview.1.71">
<Uri>https://github.com/nuget/nuget.client</Uri>
<Sha>ce95a567627472f8abd9d155047392e22142ff72</Sha>
<Sha>c0d3837b40a353b5178cd02953db2924aacb8712</Sha>
</Dependency>
<Dependency Name="Microsoft.Net.Compilers.Toolset" Version="4.13.0-3.24575.2">
<Uri>https://github.com/dotnet/roslyn</Uri>
Expand Down
2 changes: 1 addition & 1 deletion eng/Versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
<MicrosoftCodeAnalysisCollectionsVersion>4.2.0-1.22102.8</MicrosoftCodeAnalysisCollectionsVersion>
<MicrosoftDotNetXUnitExtensionsVersion>9.0.0-beta.24562.13</MicrosoftDotNetXUnitExtensionsVersion>
<MicrosoftNetCompilersToolsetVersion>4.13.0-3.24575.2</MicrosoftNetCompilersToolsetVersion>
<NuGetBuildTasksVersion>6.13.0-preview.1.62</NuGetBuildTasksVersion>
<NuGetBuildTasksVersion>6.13.0-preview.1.71</NuGetBuildTasksVersion>
</PropertyGroup>
<PropertyGroup Condition="!$(TargetFramework.StartsWith('net4'))">
<BootstrapSdkVersion>9.0.200-preview.0.24523.19</BootstrapSdkVersion>
Expand Down
115 changes: 115 additions & 0 deletions src/Build/BuildCheck/Checks/EmbeddedResourceCheck.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.IO;
using System.Collections.Generic;
using Microsoft.Build.Collections;
using Microsoft.Build.Construction;
using Microsoft.Build.Framework;
using Microsoft.Build.Shared;

namespace Microsoft.Build.Experimental.BuildCheck.Checks;
internal class EmbeddedResourceCheck : Check
{
private const string RuleId = "BC0105";
public static CheckRule SupportedRule = new CheckRule(RuleId, "EmbeddedResourceCulture",
ResourceUtilities.GetResourceString("BuildCheck_BC0105_Title")!,
ResourceUtilities.GetResourceString("BuildCheck_BC0105_MessageFmt")!,
new CheckConfiguration() { RuleId = RuleId, Severity = CheckResultSeverity.Warning });

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

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.RegisterEvaluatedItemsAction(EvaluatedItemsAction);
}

internal override bool IsBuiltIn => true;

private readonly HashSet<string> _projects = new(MSBuildNameIgnoreCaseComparer.Default);

private void EvaluatedItemsAction(BuildCheckDataContext<EvaluatedItemsCheckData> context)
{
// Deduplication
if (!_projects.Add(context.Data.ProjectFilePath))
{
return;
}

foreach (ItemData itemData in context.Data.EnumerateItemsOfType("EmbeddedResource"))
{
string evaluatedEmbedItem = itemData.EvaluatedInclude;
bool hasDoubleExtension = HasDoubleExtension(evaluatedEmbedItem);

if (!hasDoubleExtension)
{
continue;
}

bool hasNeededMetadata = false;
foreach (KeyValuePair<string, string> keyValuePair in itemData.EnumerateMetadata())
{
if (MSBuildNameIgnoreCaseComparer.Default.Equals(keyValuePair.Key, ItemMetadataNames.culture))
{
hasNeededMetadata = true;
break;
}

if (MSBuildNameIgnoreCaseComparer.Default.Equals(keyValuePair.Key, ItemMetadataNames.withCulture) &&
keyValuePair.Value.IsMSBuildFalseString())
{
hasNeededMetadata = true;
break;
}
}

if (!hasNeededMetadata)
{
context.ReportResult(BuildCheckResult.Create(
SupportedRule,
// Populating precise location tracked via https://github.com/orgs/dotnet/projects/373/views/1?pane=issue&itemId=58661732
ElementLocation.EmptyLocation,
Path.GetFileName(context.Data.ProjectFilePath),
evaluatedEmbedItem,
GetSupposedCultureExtension(evaluatedEmbedItem)));
}
}
}

private static bool HasDoubleExtension(string s)
{
const char extensionSeparator = '.';
int firstIndex;
return
!string.IsNullOrEmpty(s) &&
(firstIndex = s.IndexOf(extensionSeparator)) > -1 &&
// We need at least 2 chars for this extension - separator and one char of extension,
// so next extension can start closest 2 chars from this one
// (this is to grace handle double dot - which is not double extension)
firstIndex + 2 <= s.Length &&
s.IndexOf(extensionSeparator, firstIndex + 2) > -1;
}

/// <summary>
/// Returns the extension that is supposed to implicitly denote the culture.
/// This is mimicking the behavior of Microsoft.Build.Tasks.Culture.GetItemCultureInfo
/// </summary>
private string GetSupposedCultureExtension(string s)
{
// If the item is defined as "Strings.en-US.resx", then we want to arrive to 'en-US'

string extension = Path.GetExtension(Path.GetFileNameWithoutExtension(s));
if (extension.Length > 1)
{
extension = extension.Substring(1);
}
return extension;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,8 @@ internal readonly record struct BuiltInCheckFactory(
new BuiltInCheckFactory([SharedOutputPathCheck.SupportedRule.Id], SharedOutputPathCheck.SupportedRule.DefaultConfiguration.IsEnabled ?? false, Construct<SharedOutputPathCheck>),
new BuiltInCheckFactory([PreferProjectReferenceCheck.SupportedRule.Id], PreferProjectReferenceCheck.SupportedRule.DefaultConfiguration.IsEnabled ?? false, Construct<PreferProjectReferenceCheck>),
new BuiltInCheckFactory([DoubleWritesCheck.SupportedRule.Id], DoubleWritesCheck.SupportedRule.DefaultConfiguration.IsEnabled ?? false, Construct<DoubleWritesCheck>),
new BuiltInCheckFactory([NoEnvironmentVariablePropertyCheck.SupportedRule.Id], NoEnvironmentVariablePropertyCheck.SupportedRule.DefaultConfiguration.IsEnabled ?? false, Construct<NoEnvironmentVariablePropertyCheck>)
new BuiltInCheckFactory([NoEnvironmentVariablePropertyCheck.SupportedRule.Id], NoEnvironmentVariablePropertyCheck.SupportedRule.DefaultConfiguration.IsEnabled ?? false, Construct<NoEnvironmentVariablePropertyCheck>),
new BuiltInCheckFactory([EmbeddedResourceCheck.SupportedRule.Id], EmbeddedResourceCheck.SupportedRule.DefaultConfiguration.IsEnabled ?? false, Construct<EmbeddedResourceCheck>),
],

// BuildCheckDataSource.Execution
Expand Down
8 changes: 8 additions & 0 deletions src/Build/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -2174,6 +2174,14 @@ Utilization: {0} Average Utilization: {1:###.0}</value>
<data name="BuildCheck_BC0104_MessageFmt" xml:space="preserve">
<value>Project {0} references output of a project {1}. Referenced path: {2}. ProjectReference should be used instead.</value>
</data>
<data name="BuildCheck_BC0105_Title" xml:space="preserve">
<value>It is recommended to specify explicit 'Culture' metadata, or 'WithCulture=false' metadata with 'EmbeddedResource' item in order to avoid wrong or nondeterministic culture estimation.</value>
<comment>Terms in quotes are not to be translated.</comment>
</data>
<data name="BuildCheck_BC0105_MessageFmt" xml:space="preserve">
<value>Project {0} specifies 'EmbeddedResource' item '{1}', that has possibly a culture denoting extension ('{2}'), but explicit 'Culture' nor 'WithCulture=false' metadata are not specified.</value>
<comment>Terms in quotes are not to be translated.</comment>
</data>
<data name="BuildCheck_BC0201_Title" xml:space="preserve">
<value>A property that is accessed should be declared first.</value>
</data>
Expand Down
14 changes: 12 additions & 2 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.

14 changes: 12 additions & 2 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.

Loading

0 comments on commit 7dcd892

Please sign in to comment.