Skip to content

Commit

Permalink
expose scoping and ElementLocation.Create
Browse files Browse the repository at this point in the history
  • Loading branch information
YuliiaKovalova committed Oct 7, 2024
1 parent 6d20978 commit d8c8b69
Show file tree
Hide file tree
Showing 13 changed files with 115 additions and 63 deletions.
1 change: 0 additions & 1 deletion src/Build/BuildCheck/API/IBuildCheckRegistrationContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using Microsoft.Build.BuildCheck.OM;

namespace Microsoft.Build.Experimental.BuildCheck;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ private void HandleProjectEvaluationStartedEvent(ProjectEvaluationStartedEventAr
BuildCheckDataSource.EventArgs,
checkContext,
eventArgs.ProjectFile!);

_buildCheckManager.ProcessProjectEvaluationStarted(
checkContext,
eventArgs.ProjectFile!);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.Collections.Generic;
using System.Linq;
using Microsoft.Build.BuildCheck.Infrastructure;
using Microsoft.Build.BuildCheck.OM;

namespace Microsoft.Build.Experimental.BuildCheck.Infrastructure;

Expand Down
65 changes: 57 additions & 8 deletions src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Threading;
using Microsoft.Build.BackEnd;
using Microsoft.Build.BackEnd.Logging;
using Microsoft.Build.BuildCheck.Infrastructure;
Expand Down Expand Up @@ -59,7 +58,7 @@ public void ShutdownComponent()
{
_instance?.Shutdown();
_instance = null;
}
}

internal sealed class BuildCheckManager : IBuildCheckManager, IBuildEngineDataRouter, IResultReporter
{
Expand Down Expand Up @@ -94,7 +93,7 @@ public void SetDataSource(BuildCheckDataSource buildCheckDataSource)
{
_enabledDataSources[(int)buildCheckDataSource] = true;
RegisterBuiltInChecks(buildCheckDataSource);
}
}
stopwatch.Stop();
_tracingReporter.AddSetDataSourceStats(stopwatch.Elapsed);
}
Expand Down Expand Up @@ -344,11 +343,11 @@ public void RemoveThrottledChecks(ICheckContext checkContext)
private void RemoveCheck(CheckFactoryContext checkToRemove)
{
_checkRegistry.Remove(checkToRemove);

if (checkToRemove.MaterializedCheck is not null)
{
_buildCheckCentralContext.DeregisterCheck(checkToRemove.MaterializedCheck);
_ruleTelemetryData.AddRange(checkToRemove.MaterializedCheck.GetRuleTelemetryData());
_ruleTelemetryData.AddRange(checkToRemove.MaterializedCheck.GetRuleTelemetryData());
checkToRemove.MaterializedCheck.Check.Dispose();
}
}
Expand All @@ -372,6 +371,18 @@ public void ProcessEvaluationFinishedEventArgs(
FileClassifier.Shared.RegisterKnownImmutableLocations(getPropertyValue);
}

// run it here to avoid the missed imports that can be reported before the checks registration
if (TryGetProjectFullPath(checkContext.BuildEventContext, out string projectPath))
{
if (_deferredProjectToImportedProjects.TryGetValue(projectPath, out HashSet<string>? importedProjects))
{
foreach (string importedProject in importedProjects)
{
_buildEventsProcessor.ProcessProjectImportedEventArgs(checkContext, projectPath, importedProject);
}
}
}

_buildEventsProcessor
.ProcessEvaluationFinishedEventArgs(checkContext, evaluationFinishedEventArgs, propertiesLookup);
}
Expand All @@ -394,10 +405,12 @@ public void ProcessEnvironmentVariableReadEventArgs(ICheckContext checkContext,

public void ProcessProjectImportedEventArgs(ICheckContext checkContext, ProjectImportedEventArgs projectImportedEventArgs)
{
if (TryGetProjectFullPath(checkContext.BuildEventContext, out string projectPath))
if (string.IsNullOrEmpty(projectImportedEventArgs.ImportedProjectFile))
{
_buildEventsProcessor.ProcessProjectImportedEventArgs(checkContext, projectPath);
return;
}

PropagateImport(projectImportedEventArgs.ProjectFile, projectImportedEventArgs.ImportedProjectFile);
}

public void ProcessTaskStartedEventArgs(
Expand All @@ -422,6 +435,7 @@ public void ProcessTaskParameterEventArgs(
.ProcessTaskParameterEventArgs(checkContext, taskParameterEventArgs);

private readonly List<BuildCheckRuleTelemetryData> _ruleTelemetryData = [];

public BuildCheckTracingData CreateCheckTracingStats()
{
foreach (CheckFactoryContext checkFactoryContext in _checkRegistry)
Expand Down Expand Up @@ -453,6 +467,8 @@ public void FinalizeProcessing(LoggingContext loggingContext)
private readonly ConcurrentDictionary<int, string> _projectsByInstanceId = new();
private readonly ConcurrentDictionary<int, string> _projectsByEvaluationId = new();

private readonly ConcurrentDictionary<string, HashSet<string>> _deferredProjectToImportedProjects = new();

/// <summary>
/// This method fetches the project full path from the context id.
/// This is needed because the full path is needed for configuration and later for fetching configured checks
Expand Down Expand Up @@ -523,6 +539,10 @@ public void ProcessProjectEvaluationStarted(
string projectFullPath)
{
_projectsByEvaluationId[checkContext.BuildEventContext.EvaluationId] = projectFullPath;
if (!_deferredProjectToImportedProjects.ContainsKey(projectFullPath))
{
_deferredProjectToImportedProjects.TryAdd(projectFullPath, new HashSet<string>() { projectFullPath });
}
}

/*
Expand All @@ -531,7 +551,6 @@ public void ProcessProjectEvaluationStarted(
*
*/


public void EndProjectEvaluation(BuildEventContext buildEventContext)
{
}
Expand All @@ -556,6 +575,36 @@ public void StartProjectRequest(ICheckContext checkContext, string projectFullPa
}

private readonly Dictionary<int, List<BuildEventArgs>> _deferredEvalDiagnostics = new();

/// <summary>
/// Propagates a newly imported project file to all projects that import the original project file.
/// This method ensures that if Project A imports Project B, and Project B now imports Project C,
/// then Project A will also show Project C as an import.
/// </summary>
/// <param name="originalProjectFile">The path of the project file that is importing a new project.</param>
/// <param name="newImportedProjectFile">The path of the newly imported project file.</param>
private void PropagateImport(string originalProjectFile, string newImportedProjectFile)
{
foreach (var entry in _deferredProjectToImportedProjects)
{
if (entry.Value.Contains(originalProjectFile))
{
_deferredProjectToImportedProjects.AddOrUpdate(
entry.Key,
_ => new HashSet<string> { newImportedProjectFile },
(_, existingSet) =>
{
lock (existingSet)
{
existingSet.Add(newImportedProjectFile);
}

return existingSet;
});
}
}
}

void IResultReporter.ReportResult(BuildEventArgs eventArgs, ICheckContext checkContext)
{
// If we do not need to decide on promotability/demotability of warnings or we are ready to decide on those
Expand Down
5 changes: 2 additions & 3 deletions src/Build/BuildCheck/Infrastructure/BuildEventsProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System;
using System.Collections.Generic;
using System.Linq;
using Microsoft.Build.BuildCheck.OM;
using Microsoft.Build.Construction;
using Microsoft.Build.Evaluation;
using Microsoft.Build.Framework;
Expand Down Expand Up @@ -100,9 +99,9 @@ internal void ProcessEnvironmentVariableReadEventArgs(ICheckContext checkContext
/// <summary>
/// The method handles events associated with the ProjectImportedEventArgs.
/// </summary>
internal void ProcessProjectImportedEventArgs(ICheckContext checkContext, string projectPath)
internal void ProcessProjectImportedEventArgs(ICheckContext checkContext, string projectPath, string importedProjectPath)
{
ProjectImportedCheckData checkData = new(projectPath, checkContext.BuildEventContext?.ProjectInstanceId);
ProjectImportedCheckData checkData = new(importedProjectPath, projectPath, checkContext.BuildEventContext?.ProjectInstanceId);

_buildCheckCentralContext.RunProjectImportedActions(checkData, checkContext, ReportResult);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using Microsoft.Build.BuildCheck.OM;
using Microsoft.Build.Experimental.BuildCheck.Checks;

namespace Microsoft.Build.Experimental.BuildCheck.Infrastructure;
Expand Down
35 changes: 12 additions & 23 deletions src/Build/BuildCheck/Infrastructure/CheckScopeClassifier.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,9 @@

namespace Microsoft.Build.BuildCheck.Infrastructure;

internal static class CheckScopeClassifier
public static class CheckScopeClassifier
{
static CheckScopeClassifier()
{
FileClassifier.Shared.OnImmutablePathsInitialized += SubscribeImmutablePathsInitialized;
}
static CheckScopeClassifier() => FileClassifier.Shared.OnImmutablePathsInitialized += SubscribeImmutablePathsInitialized;

internal static event Action? NotifyOnScopingReadiness;

Expand All @@ -32,7 +29,7 @@ static CheckScopeClassifier()
/// <param name="projectFileFullPath"></param>
/// <returns></returns>
/// <exception cref="ArgumentOutOfRangeException"></exception>
internal static bool IsActionInObservedScope(
public static bool IsActionInObservedScope(
EvaluationCheckScope scope,
IMSBuildElementLocation? location,
string projectFileFullPath)
Expand All @@ -46,26 +43,18 @@ internal static bool IsActionInObservedScope(
/// <param name="projectFileFullPath"></param>
/// <returns></returns>
/// <exception cref="ArgumentOutOfRangeException"></exception>
internal static bool IsActionInObservedScope(
public static bool IsActionInObservedScope(
EvaluationCheckScope scope,
string? filePathOfEvent,
string projectFileFullPath)
{
switch (scope)
string projectFileFullPath) => scope switch
{
case EvaluationCheckScope.ProjectFileOnly:
return filePathOfEvent == projectFileFullPath;
case EvaluationCheckScope.WorkTreeImports:
return
filePathOfEvent != null &&
!FileClassifier.Shared.IsNonModifiable(filePathOfEvent) &&
!IsGeneratedNugetImport(filePathOfEvent);
case EvaluationCheckScope.All:
return true;
default:
throw new ArgumentOutOfRangeException(nameof(scope), scope, null);
}
}
EvaluationCheckScope.ProjectFileOnly => filePathOfEvent == projectFileFullPath,
EvaluationCheckScope.WorkTreeImports => filePathOfEvent != null
&& !FileClassifier.Shared.IsNonModifiable(filePathOfEvent)
&& !IsGeneratedNugetImport(filePathOfEvent),
EvaluationCheckScope.All => true,
_ => throw new ArgumentOutOfRangeException(nameof(scope), scope, null),
};

private static bool IsGeneratedNugetImport(string file) =>
file.EndsWith("nuget.g.props", StringComparison.OrdinalIgnoreCase)
Expand Down
2 changes: 0 additions & 2 deletions src/Build/BuildCheck/Infrastructure/IBuildCheckManager.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// 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;
using Microsoft.Build.BackEnd.Logging;
using Microsoft.Build.Experimental.BuildCheck.Acquisition;
using Microsoft.Build.Framework;
Expand Down
10 changes: 5 additions & 5 deletions src/Build/BuildCheck/OM/ProjectImportedCheckData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@

using Microsoft.Build.Experimental.BuildCheck;

namespace Microsoft.Build.BuildCheck.OM;
namespace Microsoft.Build.Experimental.BuildCheck;

public class ProjectImportedCheckData : CheckData
{
public ProjectImportedCheckData(string projectFilePath, int? projectConfigurationId)
: base(projectFilePath, projectConfigurationId)
{
}
public ProjectImportedCheckData(string importedProjectFile, string projectFilePath, int? projectConfigurationId)
: base(projectFilePath, projectConfigurationId) => ImportedProjectFile = importedProjectFile;

public string ImportedProjectFile { get; }
}
25 changes: 23 additions & 2 deletions src/Build/CompatibilitySuppressions.xml
Original file line number Diff line number Diff line change
@@ -1,4 +1,25 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- https://learn.microsoft.com/en-us/dotnet/fundamentals/package-validation/diagnostic-ids -->
<!-- https://learn.microsoft.com/dotnet/fundamentals/package-validation/diagnostic-ids -->
<Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
</Suppressions>
<Suppression>
<DiagnosticId>CP0006</DiagnosticId>
<Target>M:Microsoft.Build.Experimental.BuildCheck.IBuildCheckRegistrationContext.RegisterProjectImportedAction(System.Action{Microsoft.Build.Experimental.BuildCheck.BuildCheckDataContext{Microsoft.Build.BuildCheck.OM.ProjectImportedCheckData}})</Target>
<Left>lib/net472/Microsoft.Build.dll</Left>
<Right>lib/net472/Microsoft.Build.dll</Right>
<IsBaselineSuppression>true</IsBaselineSuppression>
</Suppression>
<Suppression>
<DiagnosticId>CP0006</DiagnosticId>
<Target>M:Microsoft.Build.Experimental.BuildCheck.IBuildCheckRegistrationContext.RegisterProjectImportedAction(System.Action{Microsoft.Build.Experimental.BuildCheck.BuildCheckDataContext{Microsoft.Build.BuildCheck.OM.ProjectImportedCheckData}})</Target>
<Left>lib/net9.0/Microsoft.Build.dll</Left>
<Right>lib/net9.0/Microsoft.Build.dll</Right>
<IsBaselineSuppression>true</IsBaselineSuppression>
</Suppression>
<Suppression>
<DiagnosticId>CP0006</DiagnosticId>
<Target>M:Microsoft.Build.Experimental.BuildCheck.IBuildCheckRegistrationContext.RegisterProjectImportedAction(System.Action{Microsoft.Build.Experimental.BuildCheck.BuildCheckDataContext{Microsoft.Build.BuildCheck.OM.ProjectImportedCheckData}})</Target>
<Left>ref/net9.0/Microsoft.Build.dll</Left>
<Right>ref/net9.0/Microsoft.Build.dll</Right>
<IsBaselineSuppression>true</IsBaselineSuppression>
</Suppression>
</Suppressions>
11 changes: 4 additions & 7 deletions src/Build/ElementLocation/ElementLocation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -185,19 +185,16 @@ internal static ElementLocation Create(string file)
/// In AG there are 600 locations that have a file but zero line and column.
/// In theory yet another derived class could be made for these to save 4 bytes each.
/// </remarks>
internal static ElementLocation Create(string file, int line, int column)
public static ElementLocation Create(string file, int line, int column)
{
if (string.IsNullOrEmpty(file) && line == 0 && column == 0)
{
return EmptyLocation;
}

if (line <= 65535 && column <= 65535)
{
return new ElementLocation.SmallElementLocation(file, line, column);
}

return new ElementLocation.RegularElementLocation(file, line, column);
return line <= 65535 && column <= 65535
? new ElementLocation.SmallElementLocation(file, line, column)
: new ElementLocation.RegularElementLocation(file, line, column);
}

/// <summary>
Expand Down
3 changes: 3 additions & 0 deletions src/BuildCheck.UnitTests/MockBuildCheckRegistrationContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using Microsoft.Build.BuildCheck.OM;

Check failure on line 6 in src/BuildCheck.UnitTests/MockBuildCheckRegistrationContext.cs

View check run for this annotation

Azure Pipelines / msbuild-pr (Linux Core)

src/BuildCheck.UnitTests/MockBuildCheckRegistrationContext.cs#L6

src/BuildCheck.UnitTests/MockBuildCheckRegistrationContext.cs(6,34): error CS0234: (NETCORE_ENGINEERING_TELEMETRY=Build) The type or namespace name 'OM' does not exist in the namespace 'Microsoft.Build.BuildCheck' (are you missing an assembly reference?)

Check failure on line 6 in src/BuildCheck.UnitTests/MockBuildCheckRegistrationContext.cs

View check run for this annotation

Azure Pipelines / msbuild-pr (Linux Core)

src/BuildCheck.UnitTests/MockBuildCheckRegistrationContext.cs#L6

src/BuildCheck.UnitTests/MockBuildCheckRegistrationContext.cs(6,34): error CS0234: (NETCORE_ENGINEERING_TELEMETRY=Build) The type or namespace name 'OM' does not exist in the namespace 'Microsoft.Build.BuildCheck' (are you missing an assembly reference?)

Check failure on line 6 in src/BuildCheck.UnitTests/MockBuildCheckRegistrationContext.cs

View check run for this annotation

Azure Pipelines / msbuild-pr (macOS Core)

src/BuildCheck.UnitTests/MockBuildCheckRegistrationContext.cs#L6

src/BuildCheck.UnitTests/MockBuildCheckRegistrationContext.cs(6,34): error CS0234: (NETCORE_ENGINEERING_TELEMETRY=Build) The type or namespace name 'OM' does not exist in the namespace 'Microsoft.Build.BuildCheck' (are you missing an assembly reference?)

Check failure on line 6 in src/BuildCheck.UnitTests/MockBuildCheckRegistrationContext.cs

View check run for this annotation

Azure Pipelines / msbuild-pr (macOS Core)

src/BuildCheck.UnitTests/MockBuildCheckRegistrationContext.cs#L6

src/BuildCheck.UnitTests/MockBuildCheckRegistrationContext.cs(6,34): error CS0234: (NETCORE_ENGINEERING_TELEMETRY=Build) The type or namespace name 'OM' does not exist in the namespace 'Microsoft.Build.BuildCheck' (are you missing an assembly reference?)
using Microsoft.Build.Experimental.BuildCheck;
using Microsoft.Build.Experimental.BuildCheck.Infrastructure;

Expand Down Expand Up @@ -55,5 +56,7 @@ private void ResultHandler(CheckWrapper wrapper, ICheckContext context, CheckCon
=> Results.Add(result);

public void RegisterEnvironmentVariableReadAction(Action<BuildCheckDataContext<EnvironmentVariableCheckData>> environmentVariableAction) => throw new NotImplementedException();

public void RegisterProjectImportedAction(Action<BuildCheckDataContext<ProjectImportedCheckData>> projectImportedAction) => throw new NotImplementedException();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,26 +20,24 @@
</ItemGroup>

<Target Name="AddNuGetDlls" BeforeTargets="_GetPackageFiles">
<!-- Merge the collection of PackageReference and Assemblies using the NuGetPackageId key.
This produces a new list containing the DLL path and the "IncludeInPackage" metadata-->
<!-- Merge the collection of PackageReference and Assemblies using the NuGetPackageId key. This produces a new list containing the DLL path and the "IncludeInPackage" metadata-->
<JoinItems Left="@(ResolvedCompileFileDefinitions)" LeftKey="NuGetPackageId" LeftMetadata="*" Right="@(PackageReference)" RightKey="" RightMetadata="*" ItemSpecToUse="Left">
<Output TaskParameter="JoinResult" ItemName="_PackagesToPack" />
<Output TaskParameter="JoinResult" ItemName="_PackagesToPack" />
</JoinItems>

<ItemGroup>
<!-- Remove NETStandard DLLs -->
<_PackagesToPack Remove="@(_PackagesToPack)" Condition="%(NuGetPackageId) == 'NETStandard.Library'" />
<_PackagesToPack Remove="@(_PackagesToPack)" Condition="%(_PackagesToPack.IncludeInPackage) != 'true'" />
<!-- Remove packages, except those with IncludeInPackage=true -->
<_PackagesToPack Remove="@(_PackagesToPack)" Condition="'%(_PackagesToPack.IncludeInPackage)' != 'true'" />
</ItemGroup>

<Message Importance="High" Text="Adding DLLs from the following packages: @(_PackagesToPack->'%(NuGetPackageId)')" />

<ItemGroup>
<!-- Update the collection of items to pack with the DLLs from the NuGet packages -->
<None Include="@(_PackagesToPack)" Pack="true" PackagePath="build" Visible="false" />
<!-- Update the collection of items to pack with the DLLs from the NuGet packages -->
<None Include="@(_PackagesToPack)" Pack="true" PackagePath="build" Visible="false" />

<!-- Add the DLL produced by the current project to the NuGet package -->
<None Include="$(OutputPath)\$(AssemblyName).dll" Pack="true" PackagePath="build" Visible="false" />
<!-- Add the DLL produced by the current project to the NuGet package -->
<None Include="$(OutputPath)\$(AssemblyName).dll" Pack="true" PackagePath="build" Visible="false" />
</ItemGroup>
</Target>
</Project>

0 comments on commit d8c8b69

Please sign in to comment.