From b7e76d1d171ef74adec2f3b5e79a5b586a7e7020 Mon Sep 17 00:00:00 2001 From: Jan Krivanek Date: Fri, 9 Aug 2024 17:39:06 +0200 Subject: [PATCH] Initial version of properties analyzer (#10457) * Initial version of properties analyzer * Improve the properties analyzers * Fix the property analyzers * Fix suppressions * Fix after merge * Fix test * Reflect PR comments --- documentation/specs/BuildCheck/Codes.md | 56 +++- .../BackEnd/MockLoggingService.cs | 3 + .../Logging/EvaluationLoggingContext.cs | 3 + .../Logging/LoggingServiceLogMethods.cs | 3 + .../RequestBuilder/RequestBuilder.cs | 9 +- src/Build/BuildCheck/API/BuildCheckResult.cs | 7 +- .../API/IInternalCheckRegistrationContext.cs | 12 + src/Build/BuildCheck/API/InternalCheck.cs | 28 ++ .../BuildCheck/Checks/PropertiesUsageCheck.cs | 242 ++++++++++++++++++ .../BuildCheckBuildEventHandler.cs | 13 +- .../BuildCheckCentralContext.cs | 18 +- .../BuildCheckConfigurationException.cs | 3 + .../BuildCheckManagerProvider.cs | 113 +++++--- .../Infrastructure/BuildEventsProcessor.cs | 2 +- .../CheckConfigurationEffective.cs | 4 + ...Context.cs => CheckRegistrationContext.cs} | 14 +- .../Infrastructure/IBuildCheckManager.cs | 14 +- .../InternalOM/IBuildEngineDataRouter.cs | 7 + .../Infrastructure/NullBuildCheckManager.cs | 27 +- .../OM/ProjectProcessingDoneData.cs | 8 - .../OM/ProjectRequestProcessingDoneData.cs | 15 ++ src/Build/CompatibilitySuppressions.xml | 28 ++ src/Build/Evaluation/Evaluator.cs | 5 +- src/BuildCheck.UnitTests/EndToEndTests.cs | 46 ++++ src/Shared/IElementLocation.cs | 2 +- 25 files changed, 586 insertions(+), 96 deletions(-) create mode 100644 src/Build/BuildCheck/API/IInternalCheckRegistrationContext.cs create mode 100644 src/Build/BuildCheck/API/InternalCheck.cs create mode 100644 src/Build/BuildCheck/Checks/PropertiesUsageCheck.cs rename src/Build/BuildCheck/Infrastructure/{BuildCheckContext.cs => CheckRegistrationContext.cs} (54%) delete mode 100644 src/Build/BuildCheck/OM/ProjectProcessingDoneData.cs create mode 100644 src/Build/BuildCheck/OM/ProjectRequestProcessingDoneData.cs diff --git a/documentation/specs/BuildCheck/Codes.md b/documentation/specs/BuildCheck/Codes.md index 4ffcca8427c..b208653baff 100644 --- a/documentation/specs/BuildCheck/Codes.md +++ b/documentation/specs/BuildCheck/Codes.md @@ -2,12 +2,14 @@ Report codes are chosen to conform to suggested guidelines. Those guidelines are currently in revew: https://github.com/dotnet/msbuild/pull/10088 -| Exit Code | Reason | -|:-----|----------| -| 0 | Success | -| [BC0101](#BC0101) | Shared output path. | -| [BC0102](#BC0102) | Double writes. | -| [BC0103](#BC0103) | Used environment variable. | +| Diagnostic Code | Default Severity | Reason | +|:-----|-------|----------| +| [BC0101](#BC0101) | Warning | Shared output path. | +| [BC0102](#BC0102) | Warning | Double writes. | +| [BC0103](#BC0103) | Suggestion | Used environment variable. | +| [BC0201](#BC0201) | Warning | Usage of undefined property. | +| [BC0202](#BC0202) | Warning | Property first declared after it was used. | +| [BC0203](#BC0203) | None | Property declared but never used. | 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) @@ -43,6 +45,48 @@ Relying on environment variables introduces variability and unpredictability, as This practice can result in inconsistent build outcomes and makes debugging difficult, since environment variables are external to project files and build scripts. To ensure consistent and reproducible builds, avoid using environment variables. Instead, explicitly pass properties using the /p option, which offers better control and traceability. +## BC0201 - Usage of undefined property. + +"A property that is accessed should be declared first." + +This check indicates that a property was acessed without being declared (the declaration might have happen later - see [BC0202](#BC0202) for such checking). Only accessing in the configured scope (by default it's the project file only) are checked. + +There are couple cases which are allowed by the check: + +* Selfreferencing declaration is allowed - e.g.: + `$(ChainProp)` + +* Checking the property for emptyness - e.g.: + `` + +* Any usage of property in condition. This can be opted out vie the configuration `AllowUninitializedPropertiesInConditions` - e.g.: + ```ini + [*.csproj] + build_check.BC0201.severity=error + build_check.BC0201.AllowUninitializedPropertiesInConditions=false + build_check.BC0202.AllowUninitializedPropertiesInConditions=false + ``` + + BC0201 and BC0202 must have same value for the optional switch - as both operate on top of same data and same filtering. + +## BC0202 - Property first declared after it was used. + +"A property should be declared before it is first used." + +This check indicates that a property was acessed before it was declared. The default scope of this rule is the project file only. The scope captures the read and write operations as well. So this rule reports: + * Uninitialized reads that happened anywhere during the build, while the uninitialized property was later defined within the scope of this check (e.g. project file). + * Uninitialized reads that happened within the scope of check (e.g. project file), while later defined anywhere in the build + +If `BC0202` and [BC0201](#BC0201) are both enabled - then `BC0201` reports only the undefined reads that are not reported by this rule (so those that do not have late definitions). + +## BC0203 - Property declared but never used. + +"A property that is not used should not be declared." + +This check indicates that a property was defined in the observed scope (by default it's the project file only) and it was then not used anywhere in the build. + +This is a runtime check, not a static analysis check - so it can have false positives (as property not used in particular build might be needed in a build with different conditions). For this reasons it's currently only suggestion. +


diff --git a/src/Build.UnitTests/BackEnd/MockLoggingService.cs b/src/Build.UnitTests/BackEnd/MockLoggingService.cs index 16adcc0eb2f..e829d9f4384 100644 --- a/src/Build.UnitTests/BackEnd/MockLoggingService.cs +++ b/src/Build.UnitTests/BackEnd/MockLoggingService.cs @@ -661,5 +661,8 @@ public void ProcessPropertyRead(PropertyReadInfo propertyReadInfo, CheckLoggingC public void ProcessPropertyWrite(PropertyWriteInfo propertyWriteInfo, CheckLoggingContext checkContext) { /* Ignore the data */ } + + public void ProcessProjectEvaluationStarted(ICheckContext analysisContext, string projectFullPath) + { /* Ignore the data */ } } } diff --git a/src/Build/BackEnd/Components/Logging/EvaluationLoggingContext.cs b/src/Build/BackEnd/Components/Logging/EvaluationLoggingContext.cs index d9cb65d4b93..15f4387218a 100644 --- a/src/Build/BackEnd/Components/Logging/EvaluationLoggingContext.cs +++ b/src/Build/BackEnd/Components/Logging/EvaluationLoggingContext.cs @@ -3,6 +3,7 @@ using System.Collections; using Microsoft.Build.BackEnd.Logging; +using Microsoft.Build.Experimental.BuildCheck; using Microsoft.Build.Framework; using Microsoft.Build.Framework.Profiler; using Microsoft.Build.Shared; @@ -30,6 +31,8 @@ public EvaluationLoggingContext(ILoggingService loggingService, BuildEventContex public void LogProjectEvaluationStarted() { LoggingService.LogProjectEvaluationStarted(BuildEventContext, _projectFile); + LoggingService.BuildEngineDataRouter.ProcessProjectEvaluationStarted( + new CheckLoggingContext(LoggingService, BuildEventContext), _projectFile); } /// diff --git a/src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs b/src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs index b118ca46d34..40762761917 100644 --- a/src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs +++ b/src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs @@ -803,5 +803,8 @@ public void ProcessPropertyRead(PropertyReadInfo propertyReadInfo, CheckLoggingC public void ProcessPropertyWrite(PropertyWriteInfo propertyWriteInfo, CheckLoggingContext checkContext) => BuildCheckManagerProvider.GlobalBuildEngineDataRouter?.ProcessPropertyWrite(propertyWriteInfo, checkContext); + + public void ProcessProjectEvaluationStarted(ICheckContext checkContext, string projectFullPath) + => BuildCheckManagerProvider.GlobalBuildEngineDataRouter?.ProcessProjectEvaluationStarted(checkContext, projectFullPath); } } diff --git a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs index cf4dc1d3cdf..27bc3fa75be 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs @@ -1125,7 +1125,7 @@ private async Task BuildProject() // Load the project if (!_requestEntry.RequestConfiguration.IsLoaded) { - buildCheckManager?.StartProjectEvaluation( + buildCheckManager?.ProjectFirstEncountered( BuildCheckDataSource.BuildExecution, new CheckLoggingContext(_nodeLoggingContext.LoggingService, _requestEntry.Request.BuildEventContext), _requestEntry.RequestConfiguration.ProjectFullPath); @@ -1151,14 +1151,12 @@ private async Task BuildProject() finally { buildCheckManager?.EndProjectEvaluation( - BuildCheckDataSource.BuildExecution, _requestEntry.Request.BuildEventContext); } _projectLoggingContext = _nodeLoggingContext.LogProjectStarted(_requestEntry); buildCheckManager?.StartProjectRequest( - BuildCheckDataSource.BuildExecution, - _requestEntry.Request.BuildEventContext, + _projectLoggingContext.BuildEventContext, _requestEntry.RequestConfiguration.ProjectFullPath); try @@ -1229,8 +1227,7 @@ private async Task BuildProject() finally { buildCheckManager?.EndProjectRequest( - BuildCheckDataSource.BuildExecution, - new CheckLoggingContext(_nodeLoggingContext.LoggingService, _requestEntry.Request.BuildEventContext), + new CheckLoggingContext(_nodeLoggingContext.LoggingService, _projectLoggingContext.BuildEventContext), _requestEntry.RequestConfiguration.ProjectFullPath); } diff --git a/src/Build/BuildCheck/API/BuildCheckResult.cs b/src/Build/BuildCheck/API/BuildCheckResult.cs index 1ff0e46715e..37203165c79 100644 --- a/src/Build/BuildCheck/API/BuildCheckResult.cs +++ b/src/Build/BuildCheck/API/BuildCheckResult.cs @@ -5,6 +5,7 @@ using System.IO; using Microsoft.Build.Construction; using Microsoft.Build.Framework; +using Microsoft.Build.Shared; namespace Microsoft.Build.Experimental.BuildCheck; @@ -16,12 +17,12 @@ namespace Microsoft.Build.Experimental.BuildCheck; /// public sealed class BuildCheckResult : IBuildCheckResult { - public static BuildCheckResult Create(CheckRule rule, ElementLocation location, params string[] messageArgs) + public static BuildCheckResult Create(CheckRule rule, IMsBuildElementLocation location, params string[] messageArgs) { return new BuildCheckResult(rule, location, messageArgs); } - public BuildCheckResult(CheckRule checkConfig, ElementLocation location, string[] messageArgs) + public BuildCheckResult(CheckRule checkConfig, IMsBuildElementLocation location, string[] messageArgs) { CheckRule = checkConfig; Location = location; @@ -42,7 +43,7 @@ internal BuildEventArgs ToEventArgs(CheckResultSeverity severity) /// /// Optional location of the finding (in near future we might need to support multiple locations). /// - public ElementLocation Location { get; } + public IMsBuildElementLocation Location { get; } public string LocationString => Location.LocationString; diff --git a/src/Build/BuildCheck/API/IInternalCheckRegistrationContext.cs b/src/Build/BuildCheck/API/IInternalCheckRegistrationContext.cs new file mode 100644 index 00000000000..2f8875fb62a --- /dev/null +++ b/src/Build/BuildCheck/API/IInternalCheckRegistrationContext.cs @@ -0,0 +1,12 @@ +using System; + +namespace Microsoft.Build.Experimental.BuildCheck.Checks; + +internal interface IInternalCheckRegistrationContext : IBuildCheckRegistrationContext +{ + void RegisterPropertyReadAction(Action> propertyReadAction); + + void RegisterPropertyWriteAction(Action> propertyWriteAction); + + void RegisterProjectRequestProcessingDoneAction(Action> propertyWriteAction); +} diff --git a/src/Build/BuildCheck/API/InternalCheck.cs b/src/Build/BuildCheck/API/InternalCheck.cs new file mode 100644 index 00000000000..58e71338e93 --- /dev/null +++ b/src/Build/BuildCheck/API/InternalCheck.cs @@ -0,0 +1,28 @@ +using System; +using Microsoft.Build.Experimental.BuildCheck; + +namespace Microsoft.Build.Experimental.BuildCheck.Checks; + +internal abstract class InternalCheck : Check +{ + /// + /// Used by the implementors to subscribe to data and events they are interested in. + /// This offers superset of registrations options to . + /// + /// + public abstract void RegisterInternalActions(IInternalCheckRegistrationContext registrationContext); + + /// + /// This is intentionally not implemented, as it is extended by . + /// + /// + public override void RegisterActions(IBuildCheckRegistrationContext registrationContext) + { + if (registrationContext is not IInternalCheckRegistrationContext internalRegistrationContext) + { + throw new ArgumentException("The registration context for InternalBuildAnalyzer must be of type IInternalBuildCheckRegistrationContext.", nameof(registrationContext)); + } + + this.RegisterInternalActions(internalRegistrationContext); + } +} diff --git a/src/Build/BuildCheck/Checks/PropertiesUsageCheck.cs b/src/Build/BuildCheck/Checks/PropertiesUsageCheck.cs new file mode 100644 index 00000000000..5af02c861ee --- /dev/null +++ b/src/Build/BuildCheck/Checks/PropertiesUsageCheck.cs @@ -0,0 +1,242 @@ +// 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 System.Diagnostics; +using System.Linq; +using Microsoft.Build.BuildCheck.Infrastructure; +using Microsoft.Build.Collections; +using Microsoft.Build.Evaluation; +using Microsoft.Build.Experimental.BuildCheck; +using Microsoft.Build.Experimental.BuildCheck.Infrastructure; +using Microsoft.Build.Shared; + +namespace Microsoft.Build.Experimental.BuildCheck.Checks; + +internal class PropertiesUsageCheck : InternalCheck +{ + private static readonly CheckRule _usedBeforeInitializedRule = new CheckRule("BC0201", "PropertyUsedBeforeDeclared", + "A property that is accessed should be declared first.", + "Property: [{0}] was accessed, but it was never initialized.", + new CheckConfiguration() { Severity = CheckResultSeverity.Warning, EvaluationCheckScope = EvaluationCheckScope.ProjectFileOnly }); + + private static readonly CheckRule _initializedAfterUsedRule = new CheckRule("BC0202", "PropertyDeclaredAfterUsed", + "A property should be declared before it is first used.", + "Property: [{0}] first declared/initialized at [{1}] used before it was initialized.", + new CheckConfiguration() { Severity = CheckResultSeverity.Warning, EvaluationCheckScope = EvaluationCheckScope.ProjectFileOnly }); + + private static readonly CheckRule _unusedPropertyRule = new CheckRule("BC0203", "UnusedPropertyDeclared", + "A property that is not used should not be declared.", + "Property: [{0}] was declared/initialized, but it was never used.", + new CheckConfiguration() { Severity = CheckResultSeverity.Suggestion, EvaluationCheckScope = EvaluationCheckScope.ProjectFileOnly }); + + internal static readonly IReadOnlyList SupportedRulesList = [_usedBeforeInitializedRule, _initializedAfterUsedRule, _unusedPropertyRule]; + + public override string FriendlyName => "MSBuild.PropertiesUsageAnalyzer"; + + public override IReadOnlyList SupportedRules => SupportedRulesList; + + private const string _allowUninitPropsInConditionsKey = "AllowUninitializedPropertiesInConditions"; + private bool _allowUninitPropsInConditions = false; + // Each check can have it's scope and enablement + private EvaluationCheckScope _uninitializedReadScope; + private EvaluationCheckScope _unusedPropertyScope; + private EvaluationCheckScope _initializedAfterUseScope; + private bool _uninitializedReadEnabled; + private bool _unusedPropertyEnabled; + private bool _initializedAfterUseEnabled; + public override void Initialize(ConfigurationContext configurationContext) + { + var config = configurationContext.CheckConfig.FirstOrDefault(c => c.RuleId == _usedBeforeInitializedRule.Id) + ?? CheckConfigurationEffective.Default; + + _uninitializedReadEnabled = config.IsEnabled; + _uninitializedReadScope = config.EvaluationCheckScope; + + config = configurationContext.CheckConfig.FirstOrDefault(c => c.RuleId == _unusedPropertyRule.Id) + ?? CheckConfigurationEffective.Default; + + _unusedPropertyEnabled = config.IsEnabled; + _unusedPropertyScope = config.EvaluationCheckScope; + + config = configurationContext.CheckConfig.FirstOrDefault(c => c.RuleId == _usedBeforeInitializedRule.Id) + ?? CheckConfigurationEffective.Default; + + _initializedAfterUseEnabled = config.IsEnabled; + _initializedAfterUseScope = config.EvaluationCheckScope; + + bool? allowUninitPropsInConditionsRule1 = null; + bool? allowUninitPropsInConditionsRule2 = null; + + foreach (CustomConfigurationData customConfigurationData in configurationContext.CustomConfigurationData) + { + allowUninitPropsInConditionsRule1 = + GetAllowUninitPropsInConditionsConfig(customConfigurationData, _usedBeforeInitializedRule.Id); + allowUninitPropsInConditionsRule2 = + GetAllowUninitPropsInConditionsConfig(customConfigurationData, _initializedAfterUsedRule.Id); + } + + if (allowUninitPropsInConditionsRule1.HasValue && + allowUninitPropsInConditionsRule2.HasValue && + allowUninitPropsInConditionsRule1 != allowUninitPropsInConditionsRule2) + { + throw new BuildCheckConfigurationException( + $"[{_usedBeforeInitializedRule.Id}] and [{_initializedAfterUsedRule.Id}] are not allowed to have differing configuration value for [{_allowUninitPropsInConditionsKey}]"); + } + + if (allowUninitPropsInConditionsRule1.HasValue || allowUninitPropsInConditionsRule2.HasValue) + { + _allowUninitPropsInConditions = allowUninitPropsInConditionsRule1 ?? allowUninitPropsInConditionsRule2 ?? false; + } + } + + private static bool? GetAllowUninitPropsInConditionsConfig(CustomConfigurationData customConfigurationData, + string ruleId) + { + if (customConfigurationData.RuleId.Equals(ruleId, StringComparison.InvariantCultureIgnoreCase) && + (customConfigurationData.ConfigurationData?.TryGetValue(_allowUninitPropsInConditionsKey, out string? configVal) ?? false)) + { + return bool.Parse(configVal); + } + + return null; + } + + public override void RegisterInternalActions(IInternalCheckRegistrationContext registrationContext) + { + registrationContext.RegisterPropertyReadAction(ProcessPropertyRead); + + if (_unusedPropertyEnabled || _initializedAfterUseEnabled) + { + registrationContext.RegisterPropertyWriteAction(ProcessPropertyWrite); + } + + if (_unusedPropertyEnabled || _uninitializedReadEnabled) + { + registrationContext.RegisterProjectRequestProcessingDoneAction(DoneWithProject); + } + } + + private Dictionary _writenProperties = new(MSBuildNameIgnoreCaseComparer.Default); + private HashSet _readProperties = new(MSBuildNameIgnoreCaseComparer.Default); + // For the 'Property Initialized after used' check - we are interested in cases where: + // 1. Property is read anywhere and then initialized in the checked scope. + // 2. Property is read in the checked scope and then initialized anywhere. + private Dictionary _uninitializedReadsInScope = new(MSBuildNameIgnoreCaseComparer.Default); + private Dictionary _uninitializedReadsOutOfScope = new(MSBuildNameIgnoreCaseComparer.Default); + + private void ProcessPropertyWrite(BuildCheckDataContext context) + { + PropertyWriteData writeData = context.Data; + + // If we want to track unused properties - store all definitions that are in scope. + if (_unusedPropertyEnabled && CheckScopeClassifier.IsActionInObservedScope(_unusedPropertyScope, + writeData.ElementLocation, writeData.ProjectFilePath)) + { + _writenProperties[writeData.PropertyName] = writeData.ElementLocation; + } + + if (_initializedAfterUseEnabled && !writeData.IsEmpty) + { + // For initialized after used check - we can remove the read from dictionary after hitting write - because + // once the property is written it should no more be uninitialized (so shouldn't be added again). + + if (_uninitializedReadsInScope.TryGetValue(writeData.PropertyName, out IMsBuildElementLocation? uninitInScopeReadLocation)) + { + _uninitializedReadsInScope.Remove(writeData.PropertyName); + + context.ReportResult(BuildCheckResult.Create( + _initializedAfterUsedRule, + uninitInScopeReadLocation, + writeData.PropertyName, writeData.ElementLocation?.LocationString ?? string.Empty)); + } + + if (CheckScopeClassifier.IsActionInObservedScope(_initializedAfterUseScope, + writeData.ElementLocation, writeData.ProjectFilePath) && + _uninitializedReadsOutOfScope.TryGetValue(writeData.PropertyName, out IMsBuildElementLocation? uninitOutScopeReadLocation)) + { + _uninitializedReadsOutOfScope.Remove(writeData.PropertyName); + + context.ReportResult(BuildCheckResult.Create( + _initializedAfterUsedRule, + uninitOutScopeReadLocation, + writeData.PropertyName, writeData.ElementLocation?.LocationString ?? string.Empty)); + } + } + } + + private void ProcessPropertyRead(BuildCheckDataContext context) + { + PropertyReadData readData = context.Data; + + // Self property initialization is not considered as a violation. + if (readData.PropertyReadContext != PropertyReadContext.PropertyEvaluationSelf && + // If we are interested in missing usage checking - let's store, regardless of location of read. + _unusedPropertyEnabled) + { + _readProperties.Add(readData.PropertyName); + } + + if (readData.IsUninitialized && + (_uninitializedReadEnabled || _initializedAfterUseEnabled) && + readData.PropertyReadContext != PropertyReadContext.PropertyEvaluationSelf && + readData.PropertyReadContext != PropertyReadContext.ConditionEvaluationWithOneSideEmpty && + (!_allowUninitPropsInConditions || + readData.PropertyReadContext != PropertyReadContext.ConditionEvaluation)) + { + // We want to wait with reporting uninitialized reads until we are sure there wasn't later attempts to initialize them. + if (_initializedAfterUseEnabled) + { + if (CheckScopeClassifier.IsActionInObservedScope(_initializedAfterUseScope, + readData.ElementLocation, readData.ProjectFilePath)) + { + _uninitializedReadsInScope[readData.PropertyName] = readData.ElementLocation; + } + // If uninitialized read happened in scope and out of scope - keep just that in scope. + else if (!_uninitializedReadsInScope.ContainsKey(readData.PropertyName)) + { + _uninitializedReadsOutOfScope[readData.PropertyName] = readData.ElementLocation; + } + } + else if (CheckScopeClassifier.IsActionInObservedScope(_uninitializedReadScope, + readData.ElementLocation, readData.ProjectFilePath)) + { + // report immediately + context.ReportResult(BuildCheckResult.Create( + _usedBeforeInitializedRule, + readData.ElementLocation, + readData.PropertyName)); + } + } + } + + + private void DoneWithProject(BuildCheckDataContext context) + { + foreach (var propWithLocation in _writenProperties) + { + if (propWithLocation.Value != null && !_readProperties.Contains(propWithLocation.Key)) + { + context.ReportResult(BuildCheckResult.Create( + _unusedPropertyRule, + propWithLocation.Value, + propWithLocation.Key)); + } + } + + // Report the remaining uninitialized reads - as if 'initialized after read' check was enabled - we cannot report + // uninitialized reads immediately (instead we wait if they are attempted to be initialized late). + foreach (var uninitializedRead in _uninitializedReadsInScope) + { + context.ReportResult(BuildCheckResult.Create( + _usedBeforeInitializedRule, + uninitializedRead.Value, + uninitializedRead.Key)); + } + + _readProperties = new HashSet(MSBuildNameIgnoreCaseComparer.Default); + _writenProperties = new Dictionary(MSBuildNameIgnoreCaseComparer.Default); + _uninitializedReadsInScope = new Dictionary(MSBuildNameIgnoreCaseComparer.Default); + } +} diff --git a/src/Build/BuildCheck/Infrastructure/BuildCheckBuildEventHandler.cs b/src/Build/BuildCheck/Infrastructure/BuildCheckBuildEventHandler.cs index 8e6bd5c24cc..419ca2c9f26 100644 --- a/src/Build/BuildCheck/Infrastructure/BuildCheckBuildEventHandler.cs +++ b/src/Build/BuildCheck/Infrastructure/BuildCheckBuildEventHandler.cs @@ -36,7 +36,7 @@ internal BuildCheckBuildEventHandler( { typeof(ProjectEvaluationFinishedEventArgs), (BuildEventArgs e) => HandleProjectEvaluationFinishedEvent((ProjectEvaluationFinishedEventArgs)e) }, { typeof(ProjectEvaluationStartedEventArgs), (BuildEventArgs e) => HandleProjectEvaluationStartedEvent((ProjectEvaluationStartedEventArgs)e) }, { typeof(EnvironmentVariableReadEventArgs), (BuildEventArgs e) => HandleEnvironmentVariableReadEvent((EnvironmentVariableReadEventArgs)e) }, - { typeof(ProjectStartedEventArgs), (BuildEventArgs e) => _buildCheckManager.StartProjectRequest(BuildCheckDataSource.EventArgs, e.BuildEventContext!, ((ProjectStartedEventArgs)e).ProjectFile!) }, + { typeof(ProjectStartedEventArgs), (BuildEventArgs e) => _buildCheckManager.StartProjectRequest(e.BuildEventContext!, ((ProjectStartedEventArgs)e).ProjectFile!) }, { typeof(ProjectFinishedEventArgs), (BuildEventArgs e) => HandleProjectFinishedRequest((ProjectFinishedEventArgs)e) }, { typeof(BuildCheckTracingEventArgs), (BuildEventArgs e) => HandleBuildCheckTracingEvent((BuildCheckTracingEventArgs)e) }, { typeof(BuildCheckAcquisitionEventArgs), (BuildEventArgs e) => HandleBuildCheckAcquisitionEvent((BuildCheckAcquisitionEventArgs)e) }, @@ -79,7 +79,7 @@ private void HandleProjectEvaluationFinishedEvent(ProjectEvaluationFinishedEvent _checkContextFactory.CreateCheckContext(eventArgs.BuildEventContext!), eventArgs); - _buildCheckManager.EndProjectEvaluation(BuildCheckDataSource.EventArgs, eventArgs.BuildEventContext!); + _buildCheckManager.EndProjectEvaluation(eventArgs.BuildEventContext!); } } @@ -87,16 +87,19 @@ private void HandleProjectEvaluationStartedEvent(ProjectEvaluationStartedEventAr { if (!IsMetaProjFile(eventArgs.ProjectFile)) { - _buildCheckManager.StartProjectEvaluation( + var checkContext = _checkContextFactory.CreateCheckContext(eventArgs.BuildEventContext!); + _buildCheckManager.ProjectFirstEncountered( BuildCheckDataSource.EventArgs, - _checkContextFactory.CreateCheckContext(eventArgs.BuildEventContext!), + checkContext, + eventArgs.ProjectFile!); + _buildCheckManager.ProcessProjectEvaluationStarted( + checkContext, eventArgs.ProjectFile!); } } private void HandleProjectFinishedRequest(ProjectFinishedEventArgs eventArgs) => _buildCheckManager.EndProjectRequest( - BuildCheckDataSource.EventArgs, _checkContextFactory.CreateCheckContext(eventArgs.BuildEventContext!), eventArgs!.ProjectFile!); diff --git a/src/Build/BuildCheck/Infrastructure/BuildCheckCentralContext.cs b/src/Build/BuildCheck/Infrastructure/BuildCheckCentralContext.cs index e1b1a96ceef..fd812421526 100644 --- a/src/Build/BuildCheck/Infrastructure/BuildCheckCentralContext.cs +++ b/src/Build/BuildCheck/Infrastructure/BuildCheckCentralContext.cs @@ -27,7 +27,7 @@ private record CallbackRegistry( List<(CheckWrapper, Action>)> TaskInvocationActions, List<(CheckWrapper, Action>)> PropertyReadActions, List<(CheckWrapper, Action>)> PropertyWriteActions, - List<(CheckWrapper, Action>)> ProjectProcessingDoneActions, + List<(CheckWrapper, Action>)> ProjectRequestProcessingDoneActions, List<(CheckWrapper, Action>)> BuildFinishedActions) { public CallbackRegistry() : this([], [], [], [], [], [], []) { } @@ -38,7 +38,7 @@ internal void DeregisterCheck(CheckWrapper check) ParsedItemsActions.RemoveAll(a => a.Item1 == check); PropertyReadActions.RemoveAll(a => a.Item1 == check); PropertyWriteActions.RemoveAll(a => a.Item1 == check); - ProjectProcessingDoneActions.RemoveAll(a => a.Item1 == check); + ProjectRequestProcessingDoneActions.RemoveAll(a => a.Item1 == check); BuildFinishedActions.RemoveAll(a => a.Item1 == check); } } @@ -71,14 +71,14 @@ internal void RegisterTaskInvocationAction(CheckWrapper check, Action> propertyReadAction) => RegisterAction(check, propertyReadAction, _globalCallbacks.PropertyReadActions); - internal void RegisterBuildFinishedAction(CheckWrapper check, Action> buildFinishedAction) - => RegisterAction(check, buildFinishedAction, _globalCallbacks.BuildFinishedActions); - internal void RegisterPropertyWriteAction(CheckWrapper check, Action> propertyWriteAction) => RegisterAction(check, propertyWriteAction, _globalCallbacks.PropertyWriteActions); - internal void RegisterProjectProcessingDoneAction(CheckWrapper check, Action> projectDoneAction) - => RegisterAction(check, projectDoneAction, _globalCallbacks.ProjectProcessingDoneActions); + internal void RegisterProjectRequestProcessingDoneAction(CheckWrapper check, Action> projectDoneAction) + => RegisterAction(check, projectDoneAction, _globalCallbacks.ProjectRequestProcessingDoneActions); + + internal void RegisterBuildFinishedAction(CheckWrapper check, Action> buildFinishedAction) + => RegisterAction(check, buildFinishedAction, _globalCallbacks.BuildFinishedActions); private void RegisterAction( CheckWrapper wrappedCheck, @@ -144,11 +144,11 @@ internal void RunPropertyWriteActions( checkContext, resultHandler); internal void RunProjectProcessingDoneActions( - ProjectProcessingDoneData projectProcessingDoneData, + ProjectRequestProcessingDoneData projectProcessingDoneData, ICheckContext checkContext, Action resultHandler) - => RunRegisteredActions(_globalCallbacks.ProjectProcessingDoneActions, projectProcessingDoneData, + => RunRegisteredActions(_globalCallbacks.ProjectRequestProcessingDoneActions, projectProcessingDoneData, checkContext, resultHandler); internal void RunBuildFinishedActions( diff --git a/src/Build/BuildCheck/Infrastructure/BuildCheckConfigurationException.cs b/src/Build/BuildCheck/Infrastructure/BuildCheckConfigurationException.cs index 00cb17cff70..98c04bb78f5 100644 --- a/src/Build/BuildCheck/Infrastructure/BuildCheckConfigurationException.cs +++ b/src/Build/BuildCheck/Infrastructure/BuildCheckConfigurationException.cs @@ -26,4 +26,7 @@ public BuildCheckConfigurationException(string message, BuildCheckConfigurationE { this.buildCheckConfigurationErrorScope = buildCheckConfigurationErrorScope; } + + public BuildCheckConfigurationException(string message, Exception innerException) : base(message, innerException) + { } } diff --git a/src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs b/src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs index a1a44743f0e..7f76541d134 100644 --- a/src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs +++ b/src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs @@ -147,7 +147,11 @@ private static readonly (string[] ruleIds, bool defaultEnablement, CheckFactory ], // BuildCheckDataSource.Execution - [] + [ + (PropertiesUsageCheck.SupportedRulesList.Select(r => r.Id).ToArray(), + PropertiesUsageCheck.SupportedRulesList.Any(r => r.DefaultConfiguration.IsEnabled ?? false), + Construct) + ] ]; /// @@ -256,7 +260,7 @@ private void SetupSingleCheck(CheckFactoryContext checkFactoryContext, string pr // Create the wrapper and register to central context wrapper.StartNewProject(projectFullPath, configurations); - var wrappedContext = new BuildCheckRegistrationContext(wrapper, _buildCheckCentralContext); + var wrappedContext = new CheckRegistrationContext(wrapper, _buildCheckCentralContext); check.RegisterActions(wrappedContext); } else @@ -409,34 +413,59 @@ public void FinalizeProcessing(LoggingContext loggingContext) loggingContext.LogBuildEvent(checkEventArg); } - private readonly ConcurrentDictionary _projectsByContextId = new(); + private readonly ConcurrentDictionary _projectsByInstnaceId = new(); + private readonly ConcurrentDictionary _projectsByEvaluationId = new(); + /// /// 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 /// (future version might optimize by using the ProjectContextId directly for fetching the checks). /// /// + /// /// - private string GetProjectFullPath(BuildEventContext buildEventContext) + private bool TryGetProjectFullPath(BuildEventContext buildEventContext, out string projectFullPath) { - const string defaultProjectFullPath = "Unknown_Project"; - - if (_projectsByContextId.TryGetValue(buildEventContext.ProjectContextId, out string? projectFullPath)) + if (buildEventContext.EvaluationId >= 0) { - return projectFullPath; + if (_projectsByEvaluationId.TryGetValue(buildEventContext.EvaluationId, out string? val)) + { + projectFullPath = val; + return true; + } + } + else if (buildEventContext.ProjectInstanceId >= 0) + { + if (_projectsByInstnaceId.TryGetValue(buildEventContext.ProjectInstanceId, out string? val)) + { + projectFullPath = val; + return true; + } } - else if (buildEventContext.ProjectContextId == BuildEventContext.InvalidProjectContextId && - _projectsByContextId.Count == 1) + else if (_projectsByInstnaceId.Count == 1) { - // The coalescing is for a rare possibility of a race where other thread removed the item (between the if check and fetch here). + projectFullPath = _projectsByInstnaceId.FirstOrDefault().Value; + // This is for a rare possibility of a race where other thread removed the item (between the if check and fetch here). // We currently do not support multiple projects in parallel in a single node anyway. - return _projectsByContextId.FirstOrDefault().Value ?? defaultProjectFullPath; + if (!string.IsNullOrEmpty(projectFullPath)) + { + return true; + } + } + else if (_projectsByEvaluationId.Count == 1) + { + projectFullPath = _projectsByEvaluationId.FirstOrDefault().Value; + if (!string.IsNullOrEmpty(projectFullPath)) + { + return true; + } } - return defaultProjectFullPath; + projectFullPath = string.Empty; + return false; } - public void StartProjectEvaluation( + public void ProjectFirstEncountered( BuildCheckDataSource buildCheckDataSource, ICheckContext checkContext, string projectFullPath) @@ -450,7 +479,13 @@ public void StartProjectEvaluation( } SetupChecksForNewProject(projectFullPath, checkContext); - _projectsByContextId[checkContext.BuildEventContext.ProjectContextId] = projectFullPath; + } + + public void ProcessProjectEvaluationStarted( + ICheckContext checkContext, + string projectFullPath) + { + _projectsByEvaluationId[checkContext.BuildEventContext.EvaluationId] = projectFullPath; } /* @@ -460,23 +495,21 @@ public void StartProjectEvaluation( */ - public void EndProjectEvaluation(BuildCheckDataSource buildCheckDataSource, BuildEventContext buildEventContext) + public void EndProjectEvaluation(BuildEventContext buildEventContext) { } - public void StartProjectRequest(BuildCheckDataSource buildCheckDataSource, BuildEventContext buildEventContext, string projectFullPath) + public void StartProjectRequest(BuildEventContext buildEventContext, string projectFullPath) { // There can be multiple ProjectStarted-ProjectFinished per single configuration project build (each request for different target) - _projectsByContextId[buildEventContext.ProjectContextId] = projectFullPath; + _projectsByInstnaceId[buildEventContext.ProjectInstanceId] = projectFullPath; } public void EndProjectRequest( - BuildCheckDataSource buildCheckDataSource, ICheckContext checkContext, string projectFullPath) { _buildEventsProcessor.ProcessProjectDone(checkContext, projectFullPath); - _projectsByContextId.TryRemove(checkContext.BuildEventContext.ProjectContextId, out _); } public void ProcessPropertyRead(PropertyReadInfo propertyReadInfo, CheckLoggingContext checkContext) @@ -486,11 +519,14 @@ public void ProcessPropertyRead(PropertyReadInfo propertyReadInfo, CheckLoggingC return; } - PropertyReadData propertyReadData = new( - GetProjectFullPath(checkContext.BuildEventContext), - checkContext.BuildEventContext.ProjectInstanceId, - propertyReadInfo); - _buildEventsProcessor.ProcessPropertyRead(propertyReadData, checkContext); + if (TryGetProjectFullPath(checkContext.BuildEventContext, out string projectFullPath)) + { + PropertyReadData propertyReadData = new( + projectFullPath, + checkContext.BuildEventContext.ProjectInstanceId, + propertyReadInfo); + _buildEventsProcessor.ProcessPropertyRead(propertyReadData, checkContext); + } } public void ProcessPropertyWrite(PropertyWriteInfo propertyWriteInfo, CheckLoggingContext checkContext) @@ -500,11 +536,14 @@ public void ProcessPropertyWrite(PropertyWriteInfo propertyWriteInfo, CheckLoggi return; } - PropertyWriteData propertyWriteData = new( - GetProjectFullPath(checkContext.BuildEventContext), - checkContext.BuildEventContext.ProjectInstanceId, - propertyWriteInfo); - _buildEventsProcessor.ProcessPropertyWrite(propertyWriteData, checkContext); + if (TryGetProjectFullPath(checkContext.BuildEventContext, out string projectFullPath)) + { + PropertyWriteData propertyWriteData = new( + projectFullPath, + checkContext.BuildEventContext.ProjectInstanceId, + propertyWriteInfo); + _buildEventsProcessor.ProcessPropertyWrite(propertyWriteData, checkContext); + } } public void Shutdown() @@ -523,7 +562,19 @@ public Check Factory() public CheckWrapper Initialize(Check ba, ConfigurationContext configContext) { - ba.Initialize(configContext); + try + { + ba.Initialize(configContext); + } + catch (BuildCheckConfigurationException) + { + throw; + } + catch (Exception e) + { + throw new BuildCheckConfigurationException( + $"The Check '{ba.FriendlyName}' failed to initialize: {e.Message}", e); + } return new CheckWrapper(ba); } diff --git a/src/Build/BuildCheck/Infrastructure/BuildEventsProcessor.cs b/src/Build/BuildCheck/Infrastructure/BuildEventsProcessor.cs index 7cf944f4ca9..d06190f3008 100644 --- a/src/Build/BuildCheck/Infrastructure/BuildEventsProcessor.cs +++ b/src/Build/BuildCheck/Infrastructure/BuildEventsProcessor.cs @@ -217,7 +217,7 @@ public void ProcessPropertyWrite(PropertyWriteData propertyWriteData, CheckLoggi public void ProcessProjectDone(ICheckContext checkContext, string projectFullPath) => _buildCheckCentralContext.RunProjectProcessingDoneActions( - new ProjectProcessingDoneData(projectFullPath, checkContext.BuildEventContext.ProjectInstanceId), + new ProjectRequestProcessingDoneData(projectFullPath, checkContext.BuildEventContext.ProjectInstanceId), checkContext, ReportResult); diff --git a/src/Build/BuildCheck/Infrastructure/CheckConfigurationEffective.cs b/src/Build/BuildCheck/Infrastructure/CheckConfigurationEffective.cs index 3ab5a72de72..0f857dad631 100644 --- a/src/Build/BuildCheck/Infrastructure/CheckConfigurationEffective.cs +++ b/src/Build/BuildCheck/Infrastructure/CheckConfigurationEffective.cs @@ -23,6 +23,10 @@ public CheckConfigurationEffective(string ruleId, EvaluationCheckScope evaluatio Severity = severity; } + internal static CheckConfigurationEffective Default { get; } = + new(string.Empty, CheckConfiguration.Default.EvaluationCheckScope!.Value, + CheckConfiguration.Default.Severity!.Value); + public string RuleId { get; } public EvaluationCheckScope EvaluationCheckScope { get; } diff --git a/src/Build/BuildCheck/Infrastructure/BuildCheckContext.cs b/src/Build/BuildCheck/Infrastructure/CheckRegistrationContext.cs similarity index 54% rename from src/Build/BuildCheck/Infrastructure/BuildCheckContext.cs rename to src/Build/BuildCheck/Infrastructure/CheckRegistrationContext.cs index 74174021b7c..33fb7e09ff3 100644 --- a/src/Build/BuildCheck/Infrastructure/BuildCheckContext.cs +++ b/src/Build/BuildCheck/Infrastructure/CheckRegistrationContext.cs @@ -2,10 +2,13 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Threading; +using Microsoft.Build.Experimental.BuildCheck; +using Microsoft.Build.Experimental.BuildCheck.Checks; namespace Microsoft.Build.Experimental.BuildCheck.Infrastructure; -internal sealed class BuildCheckRegistrationContext(CheckWrapper checkWrapper, BuildCheckCentralContext buildCheckCentralContext) : IBuildCheckRegistrationContext +internal sealed class CheckRegistrationContext(CheckWrapper checkWrapper, BuildCheckCentralContext buildCheckCentralContext) : IInternalCheckRegistrationContext { public void RegisterEvaluatedPropertiesAction(Action> evaluatedPropertiesAction) { @@ -22,6 +25,15 @@ public void RegisterTaskInvocationAction(Action> propertyReadAction) + => buildCheckCentralContext.RegisterPropertyReadAction(checkWrapper, propertyReadAction); + + public void RegisterPropertyWriteAction(Action> propertyWriteAction) + => buildCheckCentralContext.RegisterPropertyWriteAction(checkWrapper, propertyWriteAction); + + public void RegisterProjectRequestProcessingDoneAction(Action> projectDoneAction) + => buildCheckCentralContext.RegisterProjectRequestProcessingDoneAction(checkWrapper, projectDoneAction); + public void RegisterBuildFinishedAction(Action> buildFinishedAction) => buildCheckCentralContext.RegisterBuildFinishedAction(checkWrapper, buildFinishedAction); } diff --git a/src/Build/BuildCheck/Infrastructure/IBuildCheckManager.cs b/src/Build/BuildCheck/Infrastructure/IBuildCheckManager.cs index eb12b5de287..88c644954e7 100644 --- a/src/Build/BuildCheck/Infrastructure/IBuildCheckManager.cs +++ b/src/Build/BuildCheck/Infrastructure/IBuildCheckManager.cs @@ -69,13 +69,19 @@ void ProcessTaskParameterEventArgs( // but as well from the ConnectorLogger - as even if interleaved, it gives the info // to manager about what checks need to be materialized and configuration fetched. // No unloading of checks is yet considered - once loaded it stays for whole build. - void StartProjectEvaluation(BuildCheckDataSource buildCheckDataSource, ICheckContext checksContext, string projectFullPath); + + + // Project might be encountered first time in some node, but be already evaluated in another - so StartProjectEvaluation won't happen + // - but we still need to know about it, hence the dedicated event. + void ProjectFirstEncountered(BuildCheckDataSource buildCheckDataSource, ICheckContext analysisContext, string projectFullPath); - void EndProjectEvaluation(BuildCheckDataSource buildCheckDataSource, BuildEventContext buildEventContext); + void ProcessProjectEvaluationStarted(ICheckContext checksContext, string projectFullPath); - void StartProjectRequest(BuildCheckDataSource buildCheckDataSource, BuildEventContext buildEventContext, string projectFullPath); + void EndProjectEvaluation(BuildEventContext buildEventContext); - void EndProjectRequest(BuildCheckDataSource buildCheckDataSource, ICheckContext checksContext, string projectFullPath); + void StartProjectRequest(BuildEventContext buildEventContext, string projectFullPath); + + void EndProjectRequest(ICheckContext checksContext, string projectFullPath); void Shutdown(); } diff --git a/src/Build/BuildCheck/Infrastructure/InternalOM/IBuildEngineDataRouter.cs b/src/Build/BuildCheck/Infrastructure/InternalOM/IBuildEngineDataRouter.cs index b837b71f07e..6d94625052c 100644 --- a/src/Build/BuildCheck/Infrastructure/InternalOM/IBuildEngineDataRouter.cs +++ b/src/Build/BuildCheck/Infrastructure/InternalOM/IBuildEngineDataRouter.cs @@ -22,4 +22,11 @@ void ProcessPropertyRead( void ProcessPropertyWrite( PropertyWriteInfo propertyWriteInfo, CheckLoggingContext checkContext); + + /// + /// Signals that evaluation in current node is starting + /// + /// + /// + void ProcessProjectEvaluationStarted(ICheckContext checkContext, string projectFullPath); } diff --git a/src/Build/BuildCheck/Infrastructure/NullBuildCheckManager.cs b/src/Build/BuildCheck/Infrastructure/NullBuildCheckManager.cs index 35bb8b9ee37..a5bf0b968a8 100644 --- a/src/Build/BuildCheck/Infrastructure/NullBuildCheckManager.cs +++ b/src/Build/BuildCheck/Infrastructure/NullBuildCheckManager.cs @@ -58,48 +58,35 @@ public void FinalizeProcessing(LoggingContext loggingContext) { } - public void StartProjectEvaluation(BuildCheckDataSource buildCheckDataSource, ICheckContext checkContext, string fullPath) - { - } - - public void EndProjectEvaluation(BuildCheckDataSource buildCheckDataSource, BuildEventContext buildEventContext) + public void ProjectFirstEncountered(BuildCheckDataSource buildCheckDataSource, ICheckContext checkContext, + string projectFullPath) { } - public void StartProjectRequest(BuildCheckDataSource buildCheckDataSource, BuildEventContext buildEventContext, string projectFullPath) + public void ProcessProjectEvaluationStarted(ICheckContext checkContext, string projectFullPath) { } - public void EndProjectRequest(BuildCheckDataSource buildCheckDataSource, ICheckContext checkContext, - string projectFullPath) + public void EndProjectEvaluation(BuildEventContext buildEventContext) { } - public void YieldProject(BuildCheckDataSource buildCheckDataSource, BuildEventContext buildEventContext) + public void StartProjectRequest(BuildEventContext buildEventContext, string projectFullPath) { } - public void ResumeProject(BuildCheckDataSource buildCheckDataSource, BuildEventContext buildEventContext) + public void EndProjectRequest(ICheckContext checkContext, string projectFullPath) { } public Dictionary CreateCheckTracingStats() => new Dictionary(); - public void StartTaskInvocation(BuildCheckDataSource buildCheckDataSource, TaskStartedEventArgs eventArgs) - { } - - public void EndTaskInvocation(BuildCheckDataSource buildCheckDataSource, TaskFinishedEventArgs eventArgs) - { } - - public void ProcessTaskParameter(BuildCheckDataSource buildCheckDataSource, TaskParameterEventArgs eventArg) - { } - public void ProcessPropertyRead(PropertyReadInfo propertyReadInfo, CheckLoggingContext buildEventContext) { } public void ProcessPropertyWrite(PropertyWriteInfo propertyWriteInfo, CheckLoggingContext buildEventContext) { } - + public void ProcessEnvironmentVariableReadEventArgs(ICheckContext checkContext, EnvironmentVariableReadEventArgs projectEvaluationEventArgs) { } } diff --git a/src/Build/BuildCheck/OM/ProjectProcessingDoneData.cs b/src/Build/BuildCheck/OM/ProjectProcessingDoneData.cs deleted file mode 100644 index 634ee9026ce..00000000000 --- a/src/Build/BuildCheck/OM/ProjectProcessingDoneData.cs +++ /dev/null @@ -1,8 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using Microsoft.Build.Experimental.BuildCheck; - -namespace Microsoft.Build.Experimental.BuildCheck; - -internal class ProjectProcessingDoneData(string projectFilePath, int? projectConfigurationId) : CheckData(projectFilePath, projectConfigurationId); diff --git a/src/Build/BuildCheck/OM/ProjectRequestProcessingDoneData.cs b/src/Build/BuildCheck/OM/ProjectRequestProcessingDoneData.cs new file mode 100644 index 00000000000..3fa36d4cde5 --- /dev/null +++ b/src/Build/BuildCheck/OM/ProjectRequestProcessingDoneData.cs @@ -0,0 +1,15 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.Build.Experimental.BuildCheck; + +namespace Microsoft.Build.Experimental.BuildCheck; + +/// +/// This data captures end of single build request on a project. +/// There can be multiple build request on a single project within single build +/// (e.g. multiple targetting, or there can be explicit request for results of specific targets) +/// +/// +/// +internal class ProjectRequestProcessingDoneData(string projectFilePath, int? projectConfigurationId) : CheckData(projectFilePath, projectConfigurationId); diff --git a/src/Build/CompatibilitySuppressions.xml b/src/Build/CompatibilitySuppressions.xml index 2e51f01cc8c..17d4b54af0f 100644 --- a/src/Build/CompatibilitySuppressions.xml +++ b/src/Build/CompatibilitySuppressions.xml @@ -281,6 +281,13 @@ lib/net472/Microsoft.Build.dll true + + CP0002 + M:Microsoft.Build.Experimental.BuildCheck.BuildCheckResult.get_Location + lib/net472/Microsoft.Build.dll + lib/net472/Microsoft.Build.dll + true + CP0002 M:Microsoft.Build.Experimental.BuildCheck.IBuildCheckRegistrationContext.RegisterEvaluatedPropertiesAction(System.Action{Microsoft.Build.Experimental.BuildCheck.BuildCheckDataContext{Microsoft.Build.Experimental.BuildCheck.EvaluatedPropertiesAnalysisData}}) @@ -330,6 +337,13 @@ lib/net8.0/Microsoft.Build.dll true + + CP0002 + M:Microsoft.Build.Experimental.BuildCheck.BuildCheckResult.get_Location + lib/net8.0/Microsoft.Build.dll + lib/net8.0/Microsoft.Build.dll + true + CP0002 M:Microsoft.Build.Experimental.BuildCheck.IBuildCheckRegistrationContext.RegisterEvaluatedPropertiesAction(System.Action{Microsoft.Build.Experimental.BuildCheck.BuildCheckDataContext{Microsoft.Build.Experimental.BuildCheck.EvaluatedPropertiesAnalysisData}}) @@ -379,6 +393,13 @@ ref/net472/Microsoft.Build.dll true + + CP0002 + M:Microsoft.Build.Experimental.BuildCheck.BuildCheckResult.get_Location + ref/net472/Microsoft.Build.dll + ref/net472/Microsoft.Build.dll + true + CP0002 M:Microsoft.Build.Experimental.BuildCheck.IBuildCheckRegistrationContext.RegisterEvaluatedPropertiesAction(System.Action{Microsoft.Build.Experimental.BuildCheck.BuildCheckDataContext{Microsoft.Build.Experimental.BuildCheck.EvaluatedPropertiesAnalysisData}}) @@ -428,6 +449,13 @@ ref/net8.0/Microsoft.Build.dll true + + CP0002 + M:Microsoft.Build.Experimental.BuildCheck.BuildCheckResult.get_Location + ref/net8.0/Microsoft.Build.dll + ref/net8.0/Microsoft.Build.dll + true + CP0002 M:Microsoft.Build.Experimental.BuildCheck.IBuildCheckRegistrationContext.RegisterEvaluatedPropertiesAction(System.Action{Microsoft.Build.Experimental.BuildCheck.BuildCheckDataContext{Microsoft.Build.Experimental.BuildCheck.EvaluatedPropertiesAnalysisData}}) diff --git a/src/Build/Evaluation/Evaluator.cs b/src/Build/Evaluation/Evaluator.cs index 33513cc27ec..064ea77a49d 100644 --- a/src/Build/Evaluation/Evaluator.cs +++ b/src/Build/Evaluation/Evaluator.cs @@ -614,13 +614,16 @@ private static ProjectTargetInstance ReadNewTargetElement(ProjectTargetElement t private void Evaluate() { string projectFile = String.IsNullOrEmpty(_projectRootElement.ProjectFileLocation.File) ? "(null)" : _projectRootElement.ProjectFileLocation.File; - using (AssemblyLoadsTracker.StartTracking(_evaluationLoggingContext, AssemblyLoadingContext.Evaluation)) using (_evaluationProfiler.TrackPass(EvaluationPass.TotalEvaluation)) { ErrorUtilities.VerifyThrow(_data.EvaluationId == BuildEventContext.InvalidEvaluationId, "There is no prior evaluation ID. The evaluator data needs to be reset at this point"); _data.EvaluationId = _evaluationLoggingContext.BuildEventContext.EvaluationId; _evaluationLoggingContext.LogProjectEvaluationStarted(); + // Track loads only after start of evaluation was actually logged + using var assemblyLoadsTracker = + AssemblyLoadsTracker.StartTracking(_evaluationLoggingContext, AssemblyLoadingContext.Evaluation); + _logProjectImportedEvents = Traits.Instance.EscapeHatches.LogProjectImports; int globalPropertiesCount; diff --git a/src/BuildCheck.UnitTests/EndToEndTests.cs b/src/BuildCheck.UnitTests/EndToEndTests.cs index 69f56d71449..2ccc4a88c32 100644 --- a/src/BuildCheck.UnitTests/EndToEndTests.cs +++ b/src/BuildCheck.UnitTests/EndToEndTests.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Text.RegularExpressions; using System.IO.Ports; using System.Linq; using System.Xml; @@ -37,6 +38,51 @@ public EndToEndTests(ITestOutputHelper output) public void Dispose() => _env.Dispose(); + [Fact] + public void PropertiesUsageAnalyzerTest() + { + using TestEnvironment env = TestEnvironment.Create(); + string contents = """ + + + + + + + $(MyProp1) + + + + + + + + $(MyProp2);xxx + + + + + """; + TransientTestFolder logFolder = env.CreateFolder(createFolder: true); + TransientTestFile projectFile = env.CreateFile(logFolder, "myProj.proj", contents); + + string output = RunnerUtilities.ExecBootstrapedMSBuild($"{projectFile.Path} -check /v:detailed", out bool success); + _env.Output.WriteLine(output); + _env.Output.WriteLine("========================="); + success.ShouldBeTrue(output); + + output.ShouldMatch(@"BC0201: .* Property: \[MyProp1\]"); + output.ShouldMatch(@"BC0202: .* Property: \[MyProp2\]"); + // since it's just suggestion, it doesn't have a colon ':' + output.ShouldMatch(@"BC0203 .* Property: \[MyProp3\]"); + + // each finding should be found just once - but reported twice, due to summary + Regex.Matches(output, "BC0201: .* Property").Count.ShouldBe(2); + Regex.Matches(output, "BC0202: .* Property").Count.ShouldBe(2); + // since it's not an error - it's not in summary + Regex.Matches(output, "BC0203 .* Property").Count.ShouldBe(1); + } + [Theory] [InlineData(true, true)] [InlineData(false, true)] diff --git a/src/Shared/IElementLocation.cs b/src/Shared/IElementLocation.cs index 39f520475ba..1623d1582eb 100644 --- a/src/Shared/IElementLocation.cs +++ b/src/Shared/IElementLocation.cs @@ -20,7 +20,7 @@ internal interface IElementLocation : IMsBuildElementLocation, ITranslatable { } /// This is currently internal - but it is prepared to be made public once it will be needed by other public BuildCheck OM /// (e.g. by property read/write OM) /// - internal interface IMsBuildElementLocation + public interface IMsBuildElementLocation { /// /// The file from which this particular element originated. It may