Skip to content

null check logging mechanism before logging glob failure #11537

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Mar 5, 2025
Merged

null check logging mechanism before logging glob failure #11537

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
91012cb
null check logging mechanism before logging glob failure
JanProvaznik Mar 4, 2025
9b51108
turn on nullable annotation and resolve issues
JanProvaznik Mar 5, 2025
40f7bc8
more annotations
JanProvaznik Mar 5, 2025
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
76 changes: 37 additions & 39 deletions src/Build/Utilities/EngineFileUtilities.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
using Microsoft.Build.Framework;
using Microsoft.Build.Shared;

#nullable disable

namespace Microsoft.Build.Internal
{
internal static class EngineFileUtilities
Expand All @@ -22,7 +20,7 @@ internal static class EngineFileUtilities

// Regexes for wildcard filespecs that should not get expanded
// By default all wildcards are expanded.
private static List<Regex> s_lazyWildCardExpansionRegexes;
private static List<Regex>? s_lazyWildCardExpansionRegexes;

static EngineFileUtilities()
{
Expand Down Expand Up @@ -59,8 +57,8 @@ internal static void CaptureLazyWildcardRegexes()
internal static string[] GetFileListUnescaped(
string directoryEscaped,
string filespecEscaped,
object loggingMechanism = null,
IElementLocation excludeLocation = null)
object? loggingMechanism = null,
IElementLocation? excludeLocation = null)
{
return GetFileList(
directoryEscaped,
Expand Down Expand Up @@ -100,17 +98,17 @@ internal static string[] GetFileListUnescaped(
/// for the Exclude attribute after detecting a drive enumerating wildcard.</param>
/// <returns>Array of file paths, escaped.</returns>
internal static string[] GetFileListEscaped(
string directoryEscaped,
string? directoryEscaped,
string filespecEscaped,
IEnumerable<string> excludeSpecsEscaped = null,
IEnumerable<string>? excludeSpecsEscaped = null,
bool forceEvaluate = false,
FileMatcher fileMatcher = null,
object loggingMechanism = null,
IElementLocation includeLocation = null,
IElementLocation excludeLocation = null,
IElementLocation importLocation = null,
BuildEventContext buildEventContext = null,
string buildEventFileInfoFullPath = null,
FileMatcher? fileMatcher = null,
object? loggingMechanism = null,
IElementLocation? includeLocation = null,
IElementLocation? excludeLocation = null,
IElementLocation? importLocation = null,
BuildEventContext? buildEventContext = null,
string? buildEventFileInfoFullPath = null,
bool disableExcludeDriveEnumerationWarning = false)
{
return GetFileList(
Expand Down Expand Up @@ -171,18 +169,18 @@ internal static bool FilespecHasWildcards(string filespecEscaped)
/// for the Exclude attribute after detecting a drive enumerating wildcard.</param>
/// <returns>Array of file paths.</returns>
private static string[] GetFileList(
string directoryEscaped,
string filespecEscaped,
string? directoryEscaped,
string? filespecEscaped,
bool returnEscaped,
bool forceEvaluateWildCards,
IEnumerable<string> excludeSpecsEscaped,
IEnumerable<string>? excludeSpecsEscaped,
FileMatcher fileMatcher,
object loggingMechanism = null,
IElementLocation includeLocation = null,
IElementLocation excludeLocation = null,
IElementLocation importLocation = null,
BuildEventContext buildEventContext = null,
string buildEventFileInfoFullPath = null,
object? loggingMechanism = null,
IElementLocation? includeLocation = null,
IElementLocation? excludeLocation = null,
IElementLocation? importLocation = null,
BuildEventContext? buildEventContext = null,
string? buildEventFileInfoFullPath = null,
bool disableExcludeDriveEnumerationWarning = false)
{
ErrorUtilities.VerifyThrowInternalLength(filespecEscaped, nameof(filespecEscaped));
Expand Down Expand Up @@ -286,7 +284,7 @@ private static string[] GetFileList(
default:
throw new InternalErrorException(ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword(
"UnknownLoggingType",
loggingMechanism.GetType(),
loggingMechanism?.GetType(),
nameof(GetFileList)));
}
}
Expand Down Expand Up @@ -327,7 +325,7 @@ private static string[] GetFileList(
default:
throw new InternalErrorException(ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword(
"UnknownLoggingType",
loggingMechanism.GetType(),
loggingMechanism?.GetType(),
nameof(GetFileList)));
}
}
Expand All @@ -338,10 +336,10 @@ private static string[] GetFileList(
// as a relative path, we will get back a bunch of relative paths.
// If the filespec started out as an absolute path, we will get
// back a bunch of absolute paths
(fileList, _, _, string globFailure) = fileMatcher.GetFiles(directoryUnescaped, filespecUnescaped, excludeSpecsUnescaped);
(fileList, _, _, string? globFailure) = fileMatcher.GetFiles(directoryUnescaped, filespecUnescaped, excludeSpecsUnescaped);

// log globing failure with the present logging mechanism
if (globFailure != null)
// log globing failure with the present logging mechanism, skip if there is no logging mechanism
if (globFailure != null && loggingMechanism != null)
{
switch (loggingMechanism)
{
Expand Down Expand Up @@ -388,7 +386,7 @@ private static string[] GetFileList(
return fileList;
}

private static void LogDriveEnumerationWarningWithTargetLoggingContext(TargetLoggingContext targetLoggingContext, IElementLocation includeLocation, IElementLocation excludeLocation, bool excludeFileSpecIsEmpty, bool disableExcludeDriveEnumerationWarning, string fileSpec)
private static void LogDriveEnumerationWarningWithTargetLoggingContext(TargetLoggingContext targetLoggingContext, IElementLocation? includeLocation, IElementLocation? excludeLocation, bool excludeFileSpecIsEmpty, bool disableExcludeDriveEnumerationWarning, string fileSpec)
{
// Both condition lines are necessary to skip for the first GetFileListEscaped call
// and reach for the GetFileListUnescaped call when the wildcarded Exclude attribute results
Expand All @@ -404,7 +402,7 @@ private static void LogDriveEnumerationWarningWithTargetLoggingContext(TargetLog
fileSpec,
XMakeAttributes.exclude,
XMakeElements.itemGroup,
excludeLocation.LocationString);
excludeLocation?.LocationString ?? "");
}

// Both conditions are necessary to reach for both GetFileListEscaped calls
Expand All @@ -421,7 +419,7 @@ private static void LogDriveEnumerationWarningWithTargetLoggingContext(TargetLog
}
}

private static void LogDriveEnumerationWarningWithLoggingService(ILoggingService loggingService, IElementLocation includeLocation, BuildEventContext buildEventContext, string buildEventFileInfoFullPath, string filespecUnescaped)
private static void LogDriveEnumerationWarningWithLoggingService(ILoggingService loggingService, IElementLocation? includeLocation, BuildEventContext? buildEventContext, string? buildEventFileInfoFullPath, string filespecUnescaped)
{
if (buildEventContext != null && includeLocation != null)
{
Expand All @@ -437,7 +435,7 @@ private static void LogDriveEnumerationWarningWithLoggingService(ILoggingService
}
}

private static void LogDriveEnumerationWarningWithEvaluationLoggingContext(EvaluationLoggingContext evaluationLoggingContext, IElementLocation importLocation, IElementLocation includeLocation, IElementLocation excludeLocation, bool excludeFileSpecIsEmpty, string filespecUnescaped, string fileSpec)
private static void LogDriveEnumerationWarningWithEvaluationLoggingContext(EvaluationLoggingContext evaluationLoggingContext, IElementLocation? importLocation, IElementLocation? includeLocation, IElementLocation? excludeLocation, bool excludeFileSpecIsEmpty, string filespecUnescaped, string fileSpec)
{
if (importLocation != null)
{
Expand Down Expand Up @@ -468,7 +466,7 @@ private static void LogDriveEnumerationWarningWithEvaluationLoggingContext(Evalu
}
}

private static void ThrowDriveEnumerationExceptionWithTargetLoggingContext(IElementLocation includeLocation, IElementLocation excludeLocation, bool excludeFileSpecIsEmpty, string filespecUnescaped, string fileSpec)
private static void ThrowDriveEnumerationExceptionWithTargetLoggingContext(IElementLocation? includeLocation, IElementLocation? excludeLocation, bool excludeFileSpecIsEmpty, string filespecUnescaped, string fileSpec)
{
// The first condition is necessary to reach for both GetFileListEscaped calls
// whenever the wildcarded Include attribute results in drive enumeration, and
Expand Down Expand Up @@ -501,18 +499,18 @@ private static void ThrowDriveEnumerationExceptionWithTargetLoggingContext(IElem
}
}

private static void ThrowDriveEnumerationExceptionWithLoggingService(IElementLocation includeLocation, string filespecUnescaped)
private static void ThrowDriveEnumerationExceptionWithLoggingService(IElementLocation? includeLocation, string filespecUnescaped)
{
ProjectErrorUtilities.ThrowInvalidProject(
includeLocation,
DriveEnumeratingWildcardMessageResourceName,
filespecUnescaped,
XMakeAttributes.include,
XMakeElements.itemGroup,
includeLocation.LocationString);
includeLocation?.LocationString ?? "");
}

private static void ThrowDriveEnumerationExceptionWithEvaluationLoggingContext(IElementLocation importLocation, IElementLocation includeLocation, IElementLocation excludeLocation, string filespecUnescaped, string fileSpec, bool excludeFileSpecIsEmpty)
private static void ThrowDriveEnumerationExceptionWithEvaluationLoggingContext(IElementLocation? importLocation, IElementLocation? includeLocation, IElementLocation? excludeLocation, string filespecUnescaped, string fileSpec, bool excludeFileSpecIsEmpty)
{
if (importLocation != null)
{
Expand Down Expand Up @@ -565,7 +563,7 @@ private static bool IsValidExclude(string exclude)

private static List<Regex> PopulateRegexFromEnvironment()
{
string wildCards = Environment.GetEnvironmentVariable("MsBuildSkipEagerWildCardEvaluationRegexes");
string? wildCards = Environment.GetEnvironmentVariable("MsBuildSkipEagerWildCardEvaluationRegexes");
if (string.IsNullOrEmpty(wildCards))
{
return new List<Regex>(0);
Expand All @@ -590,7 +588,7 @@ private static List<Regex> PopulateRegexFromEnvironment()

private static bool MatchesLazyWildcard(string fileSpec)
{
return _regexMatchCache.Value.GetOrAdd(fileSpec, file => s_lazyWildCardExpansionRegexes.Any(regex => regex.IsMatch(fileSpec)));
return _regexMatchCache.Value.GetOrAdd(fileSpec, file => s_lazyWildCardExpansionRegexes!.Any(regex => regex.IsMatch(fileSpec)));
}

/// <summary>
Expand All @@ -601,7 +599,7 @@ private static bool MatchesLazyWildcard(string fileSpec)
/// <param name="filespecsEscaped"></param>
/// <param name="currentDirectory"></param>
/// <returns>A Func that will return true IFF its argument matches any of the specified filespecs.</returns>
internal static Func<string, bool> GetFileSpecMatchTester(IList<string> filespecsEscaped, string currentDirectory)
internal static Func<string, bool> GetFileSpecMatchTester(IList<string> filespecsEscaped, string? currentDirectory)
{
var matchers = filespecsEscaped
.Select(fs => new Lazy<FileSpecMatcherTester>(() => FileSpecMatcherTester.Parse(currentDirectory, fs)))
Expand Down