From 809f0e4cc1513133145a66cafb12ad767d051b49 Mon Sep 17 00:00:00 2001 From: Bernie White Date: Sat, 13 Jul 2024 21:49:45 +1000 Subject: [PATCH] Refactoring for nullable (#1870) --- src/PSRule.Types/Definitions/ISourceExtent.cs | 25 +++ .../Definitions/InfoString.cs | 12 +- src/PSRule/Definitions/SourceExtent.cs | 21 --- src/PSRule/Pipeline/Dependencies/LockFile.cs | 20 +-- src/PSRule/Runtime/RuleConditionHelper.cs | 2 +- src/PSRule/Runtime/RunspaceContext.cs | 160 ++++++++++-------- 6 files changed, 128 insertions(+), 112 deletions(-) create mode 100644 src/PSRule.Types/Definitions/ISourceExtent.cs rename src/{PSRule => PSRule.Types}/Definitions/InfoString.cs (84%) diff --git a/src/PSRule.Types/Definitions/ISourceExtent.cs b/src/PSRule.Types/Definitions/ISourceExtent.cs new file mode 100644 index 0000000000..db91e639d7 --- /dev/null +++ b/src/PSRule.Types/Definitions/ISourceExtent.cs @@ -0,0 +1,25 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace PSRule.Definitions; + +/// +/// A source location for a PSRule expression. +/// +public interface ISourceExtent +{ + /// + /// The source file path. + /// + string File { get; } + + /// + /// The first line of the expression. + /// + int? Line { get; } + + /// + /// The first position of the expression. + /// + int? Position { get; } +} diff --git a/src/PSRule/Definitions/InfoString.cs b/src/PSRule.Types/Definitions/InfoString.cs similarity index 84% rename from src/PSRule/Definitions/InfoString.cs rename to src/PSRule.Types/Definitions/InfoString.cs index 3e939a7fab..072079c17f 100644 --- a/src/PSRule/Definitions/InfoString.cs +++ b/src/PSRule.Types/Definitions/InfoString.cs @@ -11,12 +11,12 @@ namespace PSRule.Definitions; [DebuggerDisplay("{Text}")] public sealed class InfoString { - private string _Text; - private string _Markdown; + private string? _Text; + private string? _Markdown; internal InfoString() { } - internal InfoString(string text, string markdown = null) + internal InfoString(string text, string? markdown = null) { Text = text; Markdown = markdown ?? text; @@ -33,7 +33,7 @@ public bool HasValue /// /// A plain text representation. /// - public string Text + public string? Text { get { return _Text; } set @@ -46,7 +46,7 @@ public string Text /// /// A markdown formatted representation if set. Otherwise this is the same as . /// - public string Markdown + public string? Markdown { get { return _Markdown; } set @@ -59,7 +59,7 @@ public string Markdown /// /// Create an info string when not null or empty. /// - internal static InfoString Create(string text, string markdown = null) + internal static InfoString? Create(string text, string? markdown = null) { return string.IsNullOrEmpty(text) && string.IsNullOrEmpty(markdown) ? null : new InfoString(text, markdown); } diff --git a/src/PSRule/Definitions/SourceExtent.cs b/src/PSRule/Definitions/SourceExtent.cs index 4ec73631a7..e48adadcc8 100644 --- a/src/PSRule/Definitions/SourceExtent.cs +++ b/src/PSRule/Definitions/SourceExtent.cs @@ -3,27 +3,6 @@ namespace PSRule.Definitions; -/// -/// A source location for a PSRule expression. -/// -public interface ISourceExtent -{ - /// - /// The source file path. - /// - string File { get; } - - /// - /// The first line of the expression. - /// - int? Line { get; } - - /// - /// The first position of the expression. - /// - int? Position { get; } -} - internal sealed class SourceExtent : ISourceExtent { internal SourceExtent(string file, int? line) diff --git a/src/PSRule/Pipeline/Dependencies/LockFile.cs b/src/PSRule/Pipeline/Dependencies/LockFile.cs index 1a4639ef4c..12464c59f1 100644 --- a/src/PSRule/Pipeline/Dependencies/LockFile.cs +++ b/src/PSRule/Pipeline/Dependencies/LockFile.cs @@ -31,22 +31,22 @@ public sealed class LockFile /// /// An alternative path to the lock file. /// Returns an instance of the lock file or a default instance if the file does not exist. - public static LockFile Read(string path) + public static LockFile Read(string? path) { path = Environment.GetRootedPath(path); path = Path.GetExtension(path) == ".json" ? path : Path.Combine(path, DEFAULT_FILE); - LockFile result = null; + LockFile? result = null; if (File.Exists(path)) { var json = File.ReadAllText(path, Encoding.UTF8); result = JsonConvert.DeserializeObject(json, new JsonSerializerSettings { - Converters = new List - { + Converters = + [ new SemanticVersionConverter() - }, + ], }); - return result; + return result ?? new LockFile(); } result ??= new LockFile(); result.Modules ??= new Dictionary(StringComparer.OrdinalIgnoreCase); @@ -57,7 +57,7 @@ public static LockFile Read(string path) /// Write the lock file to disk. /// /// An alternative path to the lock file. - public void Write(string path) + public void Write(string? path) { Version = 1; @@ -65,10 +65,10 @@ public void Write(string path) path = Path.GetExtension(path) == "json" ? path : Path.Combine(path, DEFAULT_FILE); var json = JsonConvert.SerializeObject(this, Formatting.Indented, new JsonSerializerSettings { - Converters = new List - { + Converters = + [ new SemanticVersionConverter() - } + ] }); File.WriteAllText(path, json, Encoding.UTF8); } diff --git a/src/PSRule/Runtime/RuleConditionHelper.cs b/src/PSRule/Runtime/RuleConditionHelper.cs index 949b5c290c..714dc748b0 100644 --- a/src/PSRule/Runtime/RuleConditionHelper.cs +++ b/src/PSRule/Runtime/RuleConditionHelper.cs @@ -24,7 +24,7 @@ internal static RuleConditionResult Create(IEnumerable value) var baseObject = ExpressionHelpers.GetBaseObject(v); if (!(TryAssertResult(baseObject, out var result) || TryBoolean(baseObject, out result))) { - RunspaceContext.CurrentThread.ErrorInvaildRuleResult(); + RunspaceContext.CurrentThread.ErrorInvalidRuleResult(); hasErrors = true; } else if (result) diff --git a/src/PSRule/Runtime/RunspaceContext.cs b/src/PSRule/Runtime/RunspaceContext.cs index 44e3203e76..3786412118 100644 --- a/src/PSRule/Runtime/RunspaceContext.cs +++ b/src/PSRule/Runtime/RunspaceContext.cs @@ -14,6 +14,8 @@ namespace PSRule.Runtime; +#nullable enable + /// /// A context for a PSRule runspace. /// @@ -25,15 +27,15 @@ internal sealed class RunspaceContext : IDisposable, ILogger private const string WARN_KEY_SEPARATOR = "_"; [ThreadStatic] - internal static RunspaceContext CurrentThread; + internal static RunspaceContext? CurrentThread; internal readonly PipelineContext Pipeline; internal readonly IPipelineWriter Writer; // Fields exposed to engine - internal RuleRecord RuleRecord; - internal RuleBlock RuleBlock; - internal ITargetBindingResult Binding; + internal RuleRecord? RuleRecord; + internal RuleBlock? RuleBlock; + internal ITargetBindingResult? Binding; private readonly ExecutionActionPreference _RuleInconclusive; private readonly ExecutionActionPreference _UnprocessedObject; @@ -55,7 +57,7 @@ internal sealed class RunspaceContext : IDisposable, ILogger private bool _RaisedUsingInvariantCulture; // Pipeline logging - private string _LogPrefix; + private string? _LogPrefix; private int _ObjectNumber; private int _RuleErrors; @@ -77,32 +79,32 @@ internal RunspaceContext(PipelineContext pipeline, IPipelineWriter writer) CurrentThread = this; Pipeline = pipeline; - _RuleInconclusive = Pipeline.Option.Execution.RuleInconclusive.GetValueOrDefault(ExecutionOption.Default.RuleInconclusive.Value); - _UnprocessedObject = Pipeline.Option.Execution.UnprocessedObject.GetValueOrDefault(ExecutionOption.Default.UnprocessedObject.Value); - _RuleSuppressed = Pipeline.Option.Execution.RuleSuppressed.GetValueOrDefault(ExecutionOption.Default.RuleSuppressed.Value); - _InvariantCulture = Pipeline.Option.Execution.InvariantCulture.GetValueOrDefault(ExecutionOption.Default.InvariantCulture.Value); + _RuleInconclusive = Pipeline.Option.Execution.RuleInconclusive.GetValueOrDefault(ExecutionOption.Default.RuleInconclusive!.Value); + _UnprocessedObject = Pipeline.Option.Execution.UnprocessedObject.GetValueOrDefault(ExecutionOption.Default.UnprocessedObject!.Value); + _RuleSuppressed = Pipeline.Option.Execution.RuleSuppressed.GetValueOrDefault(ExecutionOption.Default.RuleSuppressed!.Value); + _InvariantCulture = Pipeline.Option.Execution.InvariantCulture.GetValueOrDefault(ExecutionOption.Default.InvariantCulture!.Value); - _FailStream = Pipeline.Option.Logging.RuleFail ?? LoggingOption.Default.RuleFail.Value; - _PassStream = Pipeline.Option.Logging.RulePass ?? LoggingOption.Default.RulePass.Value; - _WarnOnce = new HashSet(); + _FailStream = Pipeline.Option.Logging.RuleFail ?? LoggingOption.Default.RuleFail!.Value; + _PassStream = Pipeline.Option.Logging.RulePass ?? LoggingOption.Default.RulePass!.Value; + _WarnOnce = []; _ObjectNumber = -1; _RuleTimer = new Stopwatch(); - _Reason = new List(); - _Conventions = new List(); + _Reason = []; + _Conventions = []; _LanguageScopes = new LanguageScopeSet(this); _Scope = new Stack(); } internal bool HadErrors => _RuleErrors > 0; - internal IEnumerable Output { get; private set; } + internal IEnumerable? Output { get; private set; } - internal TargetObject TargetObject { get; private set; } + internal TargetObject? TargetObject { get; private set; } - internal ITargetBinder TargetBinder { get; private set; } + internal ITargetBinder? TargetBinder { get; private set; } - internal SourceScope Source { get; private set; } + internal SourceScope? Source { get; private set; } internal ILanguageScope LanguageScope { @@ -141,11 +143,11 @@ internal void PopScope(RunspaceScope scope) public void Pass() { - if (Writer == null || _PassStream == OutcomeLogStream.None) + if (Writer == null || _PassStream == OutcomeLogStream.None || RuleRecord == null) return; if (_PassStream == OutcomeLogStream.Warning && Writer.ShouldWriteWarning()) - Writer.WriteWarning(PSRuleResources.OutcomeRulePass, RuleRecord.RuleName, Binding.TargetName); + Writer.WriteWarning(PSRuleResources.OutcomeRulePass, RuleRecord.RuleName, Binding?.TargetName); if (_PassStream == OutcomeLogStream.Error && Writer.ShouldWriteError()) Writer.WriteError(new ErrorRecord( @@ -153,7 +155,7 @@ public void Pass() Thread.CurrentThread.CurrentCulture, PSRuleResources.OutcomeRulePass, RuleRecord.RuleName, - Binding.TargetName)), + Binding?.TargetName)), SOURCE_OUTCOME_PASS, ErrorCategory.InvalidData, null)); @@ -164,17 +166,17 @@ public void Pass() Thread.CurrentThread.CurrentCulture, PSRuleResources.OutcomeRulePass, RuleRecord.RuleName, - Binding.TargetName), + Binding?.TargetName), source: SOURCE_OUTCOME_PASS)); } public void Fail() { - if (Writer == null || _FailStream == OutcomeLogStream.None) + if (Writer == null || _FailStream == OutcomeLogStream.None || RuleRecord == null) return; if (_FailStream == OutcomeLogStream.Warning && Writer.ShouldWriteWarning()) - Writer.WriteWarning(PSRuleResources.OutcomeRuleFail, RuleRecord.RuleName, Binding.TargetName); + Writer.WriteWarning(PSRuleResources.OutcomeRuleFail, RuleRecord.RuleName, Binding?.TargetName); if (_FailStream == OutcomeLogStream.Error && Writer.ShouldWriteError()) Writer.WriteError(new ErrorRecord( @@ -182,7 +184,7 @@ public void Fail() Thread.CurrentThread.CurrentCulture, PSRuleResources.OutcomeRuleFail, RuleRecord.RuleName, - Binding.TargetName)), + Binding?.TargetName)), SOURCE_OUTCOME_FAIL, ErrorCategory.InvalidData, null)); @@ -193,28 +195,28 @@ public void Fail() Thread.CurrentThread.CurrentCulture, PSRuleResources.OutcomeRuleFail, RuleRecord.RuleName, - Binding.TargetName), + Binding?.TargetName), source: SOURCE_OUTCOME_FAIL)); } public void WarnRuleInconclusive(string ruleId) { - this.Throw(_RuleInconclusive, PSRuleResources.RuleInconclusive, ruleId, Binding.TargetName); + this.Throw(_RuleInconclusive, PSRuleResources.RuleInconclusive, ruleId, Binding?.TargetName); } public void WarnObjectNotProcessed() { - this.Throw(_UnprocessedObject, PSRuleResources.ObjectNotProcessed, Binding.TargetName); + this.Throw(_UnprocessedObject, PSRuleResources.ObjectNotProcessed, Binding?.TargetName); } public void RuleSuppressed(string ruleId) { - this.Throw(_RuleSuppressed, PSRuleResources.RuleSuppressed, ruleId, Binding.TargetName); + this.Throw(_RuleSuppressed, PSRuleResources.RuleSuppressed, ruleId, Binding?.TargetName); } public void WarnRuleCountSuppressed(int ruleCount) { - this.Throw(_RuleSuppressed, PSRuleResources.RuleCountSuppressed, ruleCount, Binding.TargetName); + this.Throw(_RuleSuppressed, PSRuleResources.RuleCountSuppressed, ruleCount, Binding?.TargetName); } public void RuleSuppressionGroup(string ruleId, ISuppressionInfo suppression) @@ -223,9 +225,9 @@ public void RuleSuppressionGroup(string ruleId, ISuppressionInfo suppression) return; if (suppression.Synopsis != null && suppression.Synopsis.HasValue) - this.Throw(_RuleSuppressed, PSRuleResources.RuleSuppressionGroupExtended, ruleId, suppression.Id, Binding.TargetName, suppression.Synopsis.Text); + this.Throw(_RuleSuppressed, PSRuleResources.RuleSuppressionGroupExtended, ruleId, suppression.Id, Binding?.TargetName, suppression.Synopsis.Text); else - this.Throw(_RuleSuppressed, PSRuleResources.RuleSuppressionGroup, ruleId, suppression.Id, Binding.TargetName); + this.Throw(_RuleSuppressed, PSRuleResources.RuleSuppressionGroup, ruleId, suppression.Id, Binding?.TargetName); } public void RuleSuppressionGroupCount(ISuppressionInfo suppression, int count) @@ -234,12 +236,12 @@ public void RuleSuppressionGroupCount(ISuppressionInfo suppression, int count) return; if (suppression.Synopsis != null && suppression.Synopsis.HasValue) - this.Throw(_RuleSuppressed, PSRuleResources.RuleSuppressionGroupExtendedCount, count, suppression.Id, Binding.TargetName, suppression.Synopsis.Text); + this.Throw(_RuleSuppressed, PSRuleResources.RuleSuppressionGroupExtendedCount, count, suppression.Id, Binding?.TargetName, suppression.Synopsis.Text); else - this.Throw(_RuleSuppressed, PSRuleResources.RuleSuppressionGroupCount, count, suppression.Id, Binding.TargetName); + this.Throw(_RuleSuppressed, PSRuleResources.RuleSuppressionGroupCount, count, suppression.Id, Binding?.TargetName); } - public void ErrorInvaildRuleResult() + public void ErrorInvalidRuleResult() { if (Writer == null || !Writer.ShouldWriteError()) return; @@ -248,7 +250,7 @@ public void ErrorInvaildRuleResult() exception: new RuleException(message: string.Format( Thread.CurrentThread.CurrentCulture, PSRuleResources.InvalidRuleResult, - RuleBlock.Id + RuleBlock?.Id )), errorId: ERRORID_INVALIDRULERESULT, errorCategory: ErrorCategory.InvalidResult, @@ -278,7 +280,7 @@ public void VerboseObjectStart() if (Writer == null || !Writer.ShouldWriteVerbose()) return; - Writer.WriteVerbose(string.Concat(GetLogPrefix(), " :: ", Binding.TargetName)); + Writer.WriteVerbose(string.Concat(GetLogPrefix(), " :: ", Binding?.TargetName)); } public void VerboseConditionMessage(string condition, string message, params object[] args) @@ -360,51 +362,64 @@ private static void EnableLogging(PowerShell ps) private static void Debug_DataAdded(object sender, DataAddedEventArgs e) { - if (CurrentThread.Writer == null) + if (CurrentThread?.Writer == null) + return; + + if (sender is not PSDataCollection collection) return; - var collection = sender as PSDataCollection; var record = collection[e.Index]; CurrentThread.Writer.WriteDebug(debugRecord: record); } private static void Information_DataAdded(object sender, DataAddedEventArgs e) { - if (CurrentThread.Writer == null) + if (CurrentThread?.Writer == null) + return; + + if (sender is not PSDataCollection collection) return; - var collection = sender as PSDataCollection; var record = collection[e.Index]; CurrentThread.Writer.WriteInformation(informationRecord: record); } private static void Verbose_DataAdded(object sender, DataAddedEventArgs e) { - if (CurrentThread.Writer == null) + if (CurrentThread?.Writer == null) + return; + + if (sender is not PSDataCollection collection) return; - var collection = sender as PSDataCollection; var record = collection[e.Index]; CurrentThread.Writer.WriteVerbose(record.Message); } private static void Warning_DataAdded(object sender, DataAddedEventArgs e) { - if (CurrentThread.Writer == null) + if (CurrentThread?.Writer == null) + return; + + if (sender is not PSDataCollection collection) return; - var collection = sender as PSDataCollection; var record = collection[e.Index]; CurrentThread.Writer.WriteWarning(message: record.Message); } private static void Error_DataAdded(object sender, DataAddedEventArgs e) { + if (CurrentThread == null) + return; + CurrentThread._RuleErrors++; if (CurrentThread.Writer == null) return; - var collection = sender as PSDataCollection; + if (sender is not PSDataCollection collection) + return; + var record = collection[e.Index]; CurrentThread.Error(record); } @@ -489,12 +504,12 @@ private string GetErrorId(ErrorRecord record) ); } - private static string GetPositionMessage(ErrorRecord errorRecord) + private static string? GetPositionMessage(ErrorRecord? errorRecord) { return errorRecord?.InvocationInfo?.PositionMessage; } - private static IScriptExtent GetErrorScriptExtent(ErrorRecord errorRecord) + private static ScriptExtent? GetErrorScriptExtent(ErrorRecord? errorRecord) { if (errorRecord == null) return null; @@ -560,7 +575,7 @@ internal void EnterTargetObject(TargetObject targetObject) { _ObjectNumber++; TargetObject = targetObject; - TargetBinder.Bind(TargetObject); + TargetBinder?.Bind(TargetObject); if (Pipeline.ContentCache.Count > 0) Pipeline.ContentCache.Clear(); @@ -576,7 +591,7 @@ public void ExitTargetObject() public bool TrySelector(string name) { - return TrySelector(ResourceHelper.GetRuleId(Source.File.Module, name, ResourceIdKind.Unknown)); + return TrySelector(ResourceHelper.GetRuleId(Source?.File?.Module, name, ResourceIdKind.Unknown)); } public bool TrySelector(ResourceId id) @@ -598,7 +613,7 @@ public bool TrySelector(ResourceId id) /// public RuleRecord EnterRuleBlock(RuleBlock ruleBlock) { - Binding = TargetBinder.Result(ruleBlock.Info.ModuleName); + Binding = TargetBinder?.Result(ruleBlock.Info.ModuleName); _RuleErrors = 0; RuleBlock = ruleBlock; @@ -607,11 +622,11 @@ public RuleRecord EnterRuleBlock(RuleBlock ruleBlock) ruleId: ruleBlock.Id, @ref: ruleBlock.Ref.GetValueOrDefault().Name, targetObject: TargetObject, - targetName: Binding.TargetName, - targetType: Binding.TargetType, + targetName: Binding?.TargetName, + targetType: Binding?.TargetType, tag: ruleBlock.Tag, info: ruleBlock.Info, - field: Binding.Field, + field: Binding?.Field, level: ruleBlock.Level, extent: ruleBlock.Extent ); @@ -630,11 +645,16 @@ public void ExitRuleBlock() { // Stop rule execution time _RuleTimer.Stop(); - RuleRecord.Time = _RuleTimer.ElapsedMilliseconds; - if (!RuleRecord.IsSuccess()) - for (var i = 0; i < _Reason.Count; i++) - RuleRecord._Detail.Add(_Reason[i]); + if (RuleRecord != null) + { + RuleRecord.Time = _RuleTimer.ElapsedMilliseconds; + if (!RuleRecord.IsSuccess()) + { + for (var i = 0; i < _Reason.Count; i++) + RuleRecord._Detail.Add(_Reason[i]); + } + } Writer?.ExitScope(); @@ -659,7 +679,7 @@ internal void AddService(string id, object service) LanguageScope.AddService(name, service); } - internal object GetService(string id) + internal object? GetService(string id) { ResourceHelper.ParseIdString(LanguageScope.Name, id, out var scopeName, out var name); return !_LanguageScopes.TryScope(scopeName, out var scope) ? null : scope.GetService(name); @@ -769,7 +789,7 @@ public void Begin() Pipeline.BindField, Pipeline.Option.Input.TargetType); - HashSet _TypeFilter = null; + HashSet? _TypeFilter = null; if (Pipeline.Option.Input.TargetType != null && Pipeline.Option.Input.TargetType.Length > 0) _TypeFilter = new HashSet(Pipeline.Option.Input.TargetType, StringComparer.OrdinalIgnoreCase); @@ -787,10 +807,10 @@ public void End(IEnumerable output) RunConventionEnd(); } - public string GetLocalizedPath(string file, out string culture) + public string? GetLocalizedPath(string file, out string? culture) { culture = null; - if (string.IsNullOrEmpty(Source.File.HelpPath)) + if (string.IsNullOrEmpty(Source?.File.HelpPath)) return null; var cultures = LanguageScope.Culture; @@ -803,7 +823,7 @@ public string GetLocalizedPath(string file, out string culture) for (var i = 0; cultures != null && i < cultures.Length; i++) { - var path = Path.Combine(Source.File.HelpPath, cultures[i], file); + var path = Path.Combine(Source?.File.HelpPath, cultures[i], file); if (File.Exists(path)) { culture = cultures[i]; @@ -825,7 +845,7 @@ internal bool ShouldWarnOnce(params string[] key) #region Configuration - internal bool TryGetConfigurationValue(string name, out object value) + internal bool TryGetConfigurationValue(string name, out object? value) { value = null; if (string.IsNullOrEmpty(name)) @@ -863,8 +883,6 @@ bool ILogger.IsEnabled(LogLevel logLevel) ); } -#nullable enable - /// void ILogger.Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) { @@ -892,14 +910,6 @@ void ILogger.Log(LogLevel logLevel, EventId eventId, TState state, Excep } } - ///// - //IDisposable? ILogger.BeginScope(TState state) //where TState : notnull - //{ - // throw new NotImplementedException(); - //} - -#nullable restore - #endregion ILogger #region IDisposable @@ -930,3 +940,5 @@ private void Dispose(bool disposing) #endregion IDisposable } + +#nullable restore