From f0da73496d970718e1b542b053611ac570ca3531 Mon Sep 17 00:00:00 2001 From: Staffan Gustafsson Date: Fri, 31 Aug 2018 01:26:06 +0200 Subject: [PATCH 1/3] Improve performance of Select-String by not doing unneccessary work. Don't check if fileinfos are directories (they never are). Don't track context when context size is 0. --- .../commands/utility/MatchString.cs | 61 +++++++++++++------ 1 file changed, 44 insertions(+), 17 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/MatchString.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/MatchString.cs index 2c57c95910c..d886a3dc223 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/MatchString.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/MatchString.cs @@ -954,6 +954,19 @@ private void UpdateQueue() } } + /// + /// ContextTracker that does not work for the case when pre- and post context is 0. + /// + private class NoContextTracker : IContextTracker + { + private readonly IList _matches = new List(1); + + IList IContextTracker.EmitQueue => _matches; + void IContextTracker.TrackLine(string line){} + void IContextTracker.TrackMatch(MatchInfo match) => _matches.Add(match); + void IContextTracker.TrackEOF() {} + } + /// /// This parameter specifies the current pipeline object. /// @@ -1244,6 +1257,8 @@ public SwitchParameter AllMatches private int _preContext = 0; private int _postContext = 0; + private IContextTracker GetContextTracker() => (_preContext == 0 && _postContext == 0) ? _noContextTracker : new ContextTracker(_preContext, _postContext); + // This context tracker is only used for strings which are piped // directly into the cmdlet. File processing doesn't need // to track state between calls to ProcessRecord, and so @@ -1251,8 +1266,8 @@ public SwitchParameter AllMatches // use a single global tracker for both is that in the case of // a mixed list of strings and FileInfo, the context tracker // would get reset after each file. - private ContextTracker _globalContextTracker = null; - + private IContextTracker _globalContextTracker = null; + private IContextTracker _noContextTracker; /// /// This is used to handle the case were we're done processing input objects. /// If true, process record will just return. @@ -1261,6 +1276,8 @@ public SwitchParameter AllMatches private int _inputRecordNumber; + + /// /// Read command line parameters. /// @@ -1283,10 +1300,12 @@ protected override void BeginProcessing() } } } - - _globalContextTracker = new ContextTracker(_preContext, _postContext); + _noContextTracker = new NoContextTracker(); + _globalContextTracker = GetContextTracker(); } + private readonly List _inputObjectFileList = new List(1){string.Empty}; + /// /// Process the input. /// @@ -1297,46 +1316,54 @@ protected override void BeginProcessing() protected override void ProcessRecord() { if (_doneProcessing) + { return; + } + // We may only have directories when we have resolved wildcards + var expandedPathsMaybeDirectory = false; List expandedPaths = null; if (Path != null) { expandedPaths = ResolveFilePaths(Path, _isLiteralPath); if (expandedPaths == null) + { return; + } + + expandedPathsMaybeDirectory = true; } else { - FileInfo fileInfo = _inputObject.BaseObject as FileInfo; - if (fileInfo != null) + if (_inputObject.BaseObject is FileInfo fileInfo) { - expandedPaths = new List(); - expandedPaths.Add(fileInfo.FullName); + _inputObjectFileList[0] = fileInfo.FullName; + expandedPaths = _inputObjectFileList; } } if (expandedPaths != null) { - foreach (string filename in expandedPaths) + foreach (var filename in expandedPaths) { - if (Directory.Exists(filename)) + if (expandedPathsMaybeDirectory && Directory.Exists(filename)) { continue; } - bool foundMatch = ProcessFile(filename); + var foundMatch = ProcessFile(filename); if (_quiet && foundMatch) + { return; + } } // No results in any files. if (_quiet) { - if (_list) - WriteObject(null); - else - WriteObject(false); + var res = _list ? null : Boxed.False; + WriteObject(res); + } } else @@ -1393,7 +1420,7 @@ protected override void ProcessRecord() /// True if a match was found; otherwise false. private bool ProcessFile(string filename) { - ContextTracker contextTracker = new ContextTracker(_preContext, _postContext); + var contextTracker = GetContextTracker(); bool foundMatch = false; @@ -1487,7 +1514,7 @@ private bool ProcessFile(string filename) /// /// The context tracker to operate on. /// Whether or not any objects were emitted. - private bool FlushTrackerQueue(ContextTracker contextTracker) + private bool FlushTrackerQueue(IContextTracker contextTracker) { // Do we even have any matches to emit? if (contextTracker.EmitQueue.Count < 1) From 439728d7a9f877742b069caa8e11272bae70126a Mon Sep 17 00:00:00 2001 From: Staffan Gustafsson Date: Thu, 4 Oct 2018 20:08:48 +0200 Subject: [PATCH 2/3] Fixing CodeFactor issues. --- .../commands/utility/MatchString.cs | 592 ++++++++---------- 1 file changed, 260 insertions(+), 332 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/MatchString.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/MatchString.cs index d886a3dc223..f3210d4f7d5 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/MatchString.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/MatchString.cs @@ -2,16 +2,16 @@ // Licensed under the MIT License. using System; -using System.Text; -using System.Text.RegularExpressions; -using System.IO; using System.Collections; using System.Collections.Generic; using System.Collections.ObjectModel; +using System.Diagnostics.CodeAnalysis; +using System.Globalization; +using System.IO; using System.Management.Automation; using System.Management.Automation.Internal; -using System.Globalization; -using System.Diagnostics.CodeAnalysis; +using System.Text; +using System.Text.RegularExpressions; namespace Microsoft.PowerShell.Commands { @@ -25,40 +25,37 @@ internal MatchInfoContext() } /// - /// Lines found before a match. + /// Gets or sets the lines found before a match. /// - [SuppressMessage("Microsoft.Performance", "CA1819:PropertiesShouldNotReturnArrays")] public string[] PreContext { get; set; } /// - /// Lines found after a match. + /// Gets or sets the lines found after a match. /// - [SuppressMessage("Microsoft.Performance", "CA1819:PropertiesShouldNotReturnArrays")] public string[] PostContext { get; set; } /// - /// Lines found before a match. Does not include + /// Gets or sets the lines found before a match. Does not include /// overlapping context and thus can be used to /// display contiguous match regions. /// - [SuppressMessage("Microsoft.Performance", "CA1819:PropertiesShouldNotReturnArrays")] public string[] DisplayPreContext { get; set; } /// - /// Lines found after a match. Does not include + /// Gets or sets the lines found after a match. Does not include /// overlapping context and thus can be used to /// display contiguous match regions. /// - [SuppressMessage("Microsoft.Performance", "CA1819:PropertiesShouldNotReturnArrays")] public string[] DisplayPostContext { get; set; } /// /// Produce a deep copy of this object. /// + /// A new object that is a copy of this instance. public object Clone() { return new MatchInfoContext() @@ -76,77 +73,78 @@ public object Clone() /// public class MatchInfo { - private static string s_inputStream = "InputStream"; + private static readonly string s_inputStream = "InputStream"; /// - /// Indicates if the match was done ignoring case. + /// Gets or sets a value indicating whether the match was done ignoring case. /// /// True if case was ignored. public bool IgnoreCase { get; set; } /// - /// Returns the number of the matching line. + /// Gets or sets the number of the matching line. /// /// The number of the matching line. public int LineNumber { get; set; } /// - /// Returns the text of the matching line. + /// Gets or sets the text of the matching line. /// /// The text of the matching line. public string Line { get; set; } = string.Empty; /// - /// Returns the base name of the file containing the matching line. + /// Gets the base name of the file containing the matching line. /// /// It will be the string "InputStream" if the object came from the input stream. - /// This is a readonly property calculated from the path. + /// This is a readonly property calculated from the path . /// /// - /// The file name + /// The file name. public string Filename { get { if (!_pathSet) + { return s_inputStream; + } + return _filename ?? (_filename = System.IO.Path.GetFileName(_path)); } } + private string _filename; /// - /// The full path of the file containing the matching line. + /// Gets or sets the full path of the file containing the matching line. /// /// It will be "InputStream" if the object came from the input stream. /// /// - /// The path name + /// The path name. public string Path { - get - { - if (!_pathSet) - return s_inputStream; - return _path; - } + get => !_pathSet ? s_inputStream : _path; set { _path = value; _pathSet = true; } } + private string _path = s_inputStream; + private bool _pathSet; /// - /// Returns the pattern that was used in the match. + /// Gets or sets the pattern that was used in the match. /// - /// The pattern string + /// The pattern string. public string Pattern { get; set; } /// - /// The context for the match, or null if -context was not specified. + /// Gets or sets context for the match, or null if -context was not specified. /// public MatchInfoContext Context { get; set; } @@ -162,10 +160,12 @@ public string Path public string RelativePath(string directory) { if (!_pathSet) + { return this.Path; + } string relPath = _path; - if (!String.IsNullOrEmpty(directory)) + if (!string.IsNullOrEmpty(directory)) { if (relPath.StartsWith(directory, StringComparison.OrdinalIgnoreCase)) { @@ -173,12 +173,17 @@ public string RelativePath(string directory) if (offset < relPath.Length) { if (directory[offset - 1] == '\\' || directory[offset - 1] == '/') + { relPath = relPath.Substring(offset); + } else if (relPath[offset] == '\\' || relPath[offset] == '/') + { relPath = relPath.Substring(offset + 1); + } } } } + return relPath; } @@ -201,7 +206,7 @@ public string RelativePath(string directory) /// If path is not set, then just the line text is presented. /// /// - /// The string representation of the match object + /// The string representation of the match object. public override string ToString() { return ToString(null); @@ -211,8 +216,8 @@ public override string ToString() /// Returns the string representation of the match object same format as ToString() /// but trims the path to be relative to the argument. /// - /// Directory to use as the root when calculating the relative path - /// The string representation of the match object + /// Directory to use as the root when calculating the relative path. + /// The string representation of the match object. public string ToString(string directory) { string displayPath = (directory != null) ? RelativePath(directory) : _path; @@ -240,7 +245,7 @@ public string ToString(string directory) lines.Add(FormatLine(contextLine, displayLineNumber++, displayPath, ContextPrefix)); } - return String.Join(System.Environment.NewLine, lines.ToArray()); + return string.Join(System.Environment.NewLine, lines.ToArray()); } /// @@ -253,14 +258,13 @@ public string ToString(string directory) /// The formatted line as a string. private string FormatLine(string lineStr, int displayLineNumber, string displayPath, string prefix) { - if (_pathSet) - return StringUtil.Format(MatchFormat, prefix, displayPath, displayLineNumber, lineStr); - else - return StringUtil.Format(SimpleFormat, prefix, lineStr); + return _pathSet + ? StringUtil.Format(MatchFormat, prefix, displayPath, displayLineNumber, lineStr) + : StringUtil.Format(SimpleFormat, prefix, lineStr); } /// - /// A list of all Regex matches on the matching line. + /// Gets or sets a list of all Regex matches on the matching line. /// [SuppressMessage("Microsoft.Performance", "CA1819:PropertiesShouldNotReturnArrays")] public Match[] Matches { get; set; } = new Match[] { }; @@ -268,6 +272,7 @@ private string FormatLine(string lineStr, int displayLineNumber, string displayP /// /// Create a deep copy of this MatchInfo instance. /// + /// A new object that is a copy of this instance. internal MatchInfo Clone() { // Just do a shallow copy and then deep-copy the @@ -297,10 +302,12 @@ public sealed class SelectStringCommand : PSCmdlet /// /// A generic circular buffer. /// + /// The type of items that are buffered. private class CircularBuffer : ICollection { // Ring of items - private T[] _items; + private readonly T[] _items; + // Current length, as opposed to the total capacity // Current start of the list. Starts at 0, but may // move forwards or wrap around back to 0 due to @@ -308,43 +315,33 @@ private class CircularBuffer : ICollection private int _firstIndex; /// - /// Construct a new buffer of the specified capacity. + /// Initializes a new instance of the class. /// /// The maximum capacity of the buffer. /// If is negative. public CircularBuffer(int capacity) { if (capacity < 0) - throw new ArgumentOutOfRangeException("capacity"); + { + throw new ArgumentOutOfRangeException(nameof(capacity)); + } _items = new T[capacity]; Clear(); } /// - /// The maximum capacity of the buffer. If more items + /// Gets the maximum capacity of the buffer. If more items /// are added than the buffer has capacity for, then /// older items will be removed from the buffer with /// a first-in, first-out policy. /// - public int Capacity - { - get - { - return _items.Length; - } - } + public int Capacity => _items.Length; /// /// Whether or not the buffer is at capacity. /// - public bool IsFull - { - get - { - return Count == Capacity; - } - } + public bool IsFull => Count == Capacity; /// /// Convert from a 0-based index to a buffer index which @@ -360,7 +357,7 @@ private int WrapIndex(int zeroBasedIndex) { if (Capacity == 0 || zeroBasedIndex < 0) { - throw new ArgumentOutOfRangeException("zeroBasedIndex"); + throw new ArgumentOutOfRangeException(nameof(zeroBasedIndex)); } return (zeroBasedIndex + _firstIndex) % Capacity; @@ -384,13 +381,7 @@ IEnumerator IEnumerable.GetEnumerator() #region ICollection implementation public int Count { get; private set; } - public bool IsReadOnly - { - get - { - return false; - } - } + public bool IsReadOnly => false; /// /// Adds an item to the buffer. If the buffer is already @@ -436,13 +427,19 @@ public bool Contains(T item) public void CopyTo(T[] array, int arrayIndex) { if (array == null) - throw new ArgumentNullException("array"); + { + throw new ArgumentNullException(nameof(array)); + } if (arrayIndex < 0) - throw new ArgumentOutOfRangeException("arrayIndex"); + { + throw new ArgumentOutOfRangeException(nameof(arrayIndex)); + } if (Count > (array.Length - arrayIndex)) + { throw new ArgumentException("arrayIndex"); + } // Iterate through the buffer in correct order. foreach (T item in this) @@ -475,13 +472,14 @@ public T[] ToArray() /// internal ordering the buffer may be maintaining. /// /// The index of the item to access. + /// The buffered item at index . public T this[int index] { get { if (!(index >= 0 && index < Count)) { - throw new ArgumentOutOfRangeException("index"); + throw new ArgumentOutOfRangeException(nameof(index)); } return _items[WrapIndex(index)]; @@ -495,7 +493,7 @@ public T this[int index] private interface IContextTracker { /// - /// Matches with completed context information + /// Gets matches with completed context information /// that are ready to be emitted into the pipeline. /// IList EmitQueue { get; } @@ -533,14 +531,14 @@ private enum ContextState } private ContextState _contextState = ContextState.InitialState; - private int _preContext = 0; - private int _postContext = 0; + private readonly int _preContext; + private readonly int _postContext; // The context leading up to the match. - private CircularBuffer _collectedPreContext = null; + private readonly CircularBuffer _collectedPreContext; // The context after the match. - private List _collectedPostContext = null; + private readonly List _collectedPostContext; // Current match info we are tracking postcontext for. // At any given time, if set, this value will not be @@ -548,10 +546,10 @@ private enum ContextState private MatchInfo _matchInfo = null; /// - /// Constructor for DisplayContextTracker. + /// Initializes a new instance of the class. /// - /// How much precontext to collect at most. - /// How much precontext to collect at most. + /// How much preContext to collect at most. + /// How much postContext to collect at most. public DisplayContextTracker(int preContext, int postContext) { _preContext = preContext; @@ -564,14 +562,9 @@ public DisplayContextTracker(int preContext, int postContext) } #region IContextTracker implementation - public IList EmitQueue - { - get - { - return _emitQueue; - } - } - private List _emitQueue = null; + public IList EmitQueue => _emitQueue; + + private readonly List _emitQueue; // Track non-matching line public void TrackLine(string line) @@ -592,6 +585,7 @@ public void TrackLine(string line) // Now we're done. UpdateQueue(); } + break; } } @@ -602,7 +596,9 @@ public void TrackMatch(MatchInfo match) // Update the queue in case we were in the middle // of collecting postcontext for an older match... if (_contextState == ContextState.CollectPost) + { UpdateQueue(); + } // Update the current matchInfo. _matchInfo = match; @@ -612,9 +608,13 @@ public void TrackMatch(MatchInfo match) // Otherwise, immediately move the match onto the queue // and let UpdateQueue update our state instead. if (_postContext > 0) + { _contextState = ContextState.CollectPost; + } else + { UpdateQueue(); + } } // Track having reached the end of the file. @@ -625,7 +625,9 @@ public void TrackEOF() // early since there are no more lines to track context // for. if (_contextState == ContextState.CollectPost) + { UpdateQueue(); + } } #endregion @@ -644,6 +646,7 @@ private void UpdateQueue() _matchInfo.Context.DisplayPreContext = _collectedPreContext.ToArray(); _matchInfo.Context.DisplayPostContext = _collectedPostContext.ToArray(); } + Reset(); } } @@ -682,8 +685,8 @@ private class LogicalContextTracker : IContextTracker // or non-matching lines. private class ContextEntry { - public string Line = null; - public MatchInfo Match = null; + public readonly string Line; + public readonly MatchInfo Match; public ContextEntry(string line) { @@ -695,20 +698,18 @@ public ContextEntry(MatchInfo match) Match = match; } - public override string ToString() - { - return (Match != null) ? Match.Line : Line; - } + public override string ToString() => Match?.Line ?? Line; } // Whether or not early entries found // while still filling up the context buffer // have been added to the emit queue. // Used by UpdateQueue. - private bool _hasProcessedPreEntries = false; + private bool _hasProcessedPreEntries; + + private readonly int _preContext; + private readonly int _postContext; - private int _preContext; - private int _postContext; // A circular buffer tracking both precontext and postcontext. // // Essentially, the buffer is separated into regions: @@ -722,13 +723,13 @@ public override string ToString() // enough context to populate the Context properties of the // match. At that point, we will add the match object // to the emit queue. - private CircularBuffer _collectedContext = null; + private readonly CircularBuffer _collectedContext; /// - /// Constructor for LogicalContextTracker. + /// Initializes a new instance of the class. /// - /// How much precontext to collect at most. - /// How much postcontext to collect at most. + /// How much preContext to collect at most. + /// How much postContext to collect at most. public LogicalContextTracker(int preContext, int postContext) { _preContext = preContext; @@ -738,14 +739,9 @@ public LogicalContextTracker(int preContext, int postContext) } #region IContextTracker implementation - public IList EmitQueue - { - get - { - return _emitQueue; - } - } - private List _emitQueue = null; + public IList EmitQueue => _emitQueue; + + private readonly List _emitQueue; public void TrackLine(string line) { @@ -773,8 +769,7 @@ public void TrackEOF() // If the buffer isn't full, then nothing will have // ever been emitted and everything is still waiting // on postcontext. So process the whole buffer. - - int startIndex = (_collectedContext.IsFull) ? _preContext + 1 : 0; + int startIndex = _collectedContext.IsFull ? _preContext + 1 : 0; EmitAllInRange(startIndex, _collectedContext.Count - 1); } #endregion @@ -782,7 +777,7 @@ public void TrackEOF() /// /// Add all matches found in the specified range /// to the emit queue, collecting as much context - /// as possible up to the limits specified in the ctor. + /// as possible up to the limits specified in the constructor. /// /// /// The range is inclusive; the entries at @@ -848,10 +843,10 @@ private void UpdateQueue() /// Context ranges must be within the bounds of the context buffer. /// /// The match to operate on. - /// The start index of the precontext range. - /// The length of the precontext range. - /// The start index of the postcontext range. - /// The length of the precontext range. + /// The start index of the preContext range. + /// The length of the preContext range. + /// The start index of the postContext range. + /// The length of the postContext range. private void Emit(MatchInfo match, int preStartIndex, int preLength, int postStartIndex, int postLength) { if (match.Context != null) @@ -871,6 +866,7 @@ private void Emit(MatchInfo match, int preStartIndex, int preLength, int postSta /// /// The index to start at. /// The length of the range. + /// String representation of the collected context at the specified range. private string[] CopyContext(int startIndex, int length) { string[] result = new string[length]; @@ -889,14 +885,14 @@ private string[] CopyContext(int startIndex, int length) /// private class ContextTracker : IContextTracker { - private IContextTracker _displayTracker; - private IContextTracker _logicalTracker; + private readonly IContextTracker _displayTracker; + private readonly IContextTracker _logicalTracker; /// - /// Constructor for LogicalContextTracker. + /// Initializes a new instance of the class. /// - /// How much precontext to collect at most. - /// How much postcontext to collect at most. + /// How much preContext to collect at most. + /// How much postContext to collect at most. public ContextTracker(int preContext, int postContext) { _displayTracker = new DisplayContextTracker(preContext, postContext); @@ -943,7 +939,6 @@ private void UpdateQueue() // time as the logical tracker, so we can // be sure the matches will have both logical // and display context already populated. - foreach (MatchInfo match in _logicalTracker.EmitQueue) { EmitQueue.Add(match); @@ -962,33 +957,34 @@ private class NoContextTracker : IContextTracker private readonly IList _matches = new List(1); IList IContextTracker.EmitQueue => _matches; - void IContextTracker.TrackLine(string line){} + + void IContextTracker.TrackLine(string line) + { + } + void IContextTracker.TrackMatch(MatchInfo match) => _matches.Add(match); - void IContextTracker.TrackEOF() {} + + void IContextTracker.TrackEOF() + { + } } /// - /// This parameter specifies the current pipeline object. + /// Gets or sets the current pipeline object. /// [Parameter(ValueFromPipeline = true, Mandatory = true, ParameterSetName = "Object")] [AllowNull] [AllowEmptyString] public PSObject InputObject { - get - { - return _inputObject; - } - set - { - _inputObject = LanguagePrimitives.IsNull(value) ? PSObject.AsPSObject(string.Empty) : value; - } + get => _inputObject; + set => _inputObject = LanguagePrimitives.IsNull(value) ? PSObject.AsPSObject(string.Empty) : value; } + private PSObject _inputObject = AutomationNull.Value; /// - /// String index to start from the beginning. - /// If the value is negative, the length is counted from the end of the string. + /// Gets or sets the patterns to find. /// [Parameter(Mandatory = true, Position = 0)] public string[] Pattern { get; set; } @@ -996,7 +992,7 @@ public PSObject InputObject private Regex[] _regexPattern; /// - /// File to read from. + /// Gets or sets files to read from. /// Globbing is done on these. /// [Parameter(Position = 1, Mandatory = true, ValueFromPipelineByPropertyName = true, ParameterSetName = "File")] @@ -1004,7 +1000,7 @@ public PSObject InputObject public string[] Path { get; set; } /// - /// Literal file to read from. + /// Gets or sets literal files to read from. /// Globbing is not done on these. /// [Parameter(Mandatory = true, ValueFromPipelineByPropertyName = true, ParameterSetName = "LiteralFile")] @@ -1013,198 +1009,118 @@ public PSObject InputObject [SuppressMessage("Microsoft.Performance", "CA1819:PropertiesShouldNotReturnArrays")] public string[] LiteralPath { - get - { - return Path; - } + get => Path; set { Path = value; _isLiteralPath = true; } } - private bool _isLiteralPath = false; + + private bool _isLiteralPath; /// - /// If set, match pattern string literally. + /// Gets or sets a value indicating if a pattern string should be matched literally. /// If not (default) search using pattern as a Regular Expression. /// [Parameter] - public SwitchParameter SimpleMatch - { - get - { - return _simpleMatch; - } - set - { - _simpleMatch = value; - } - } - private bool _simpleMatch; + public SwitchParameter SimpleMatch { get; set; } - /// - /// If true, then do case-sensitive searches... + /// + /// Gets or sets a value indicating if the search is case sensitive.If true, then do case-sensitive searches. /// [Parameter] - public SwitchParameter CaseSensitive - { - get - { - return _caseSensitive; - } - set - { - _caseSensitive = value; - } - } - private bool _caseSensitive; + public SwitchParameter CaseSensitive { get; set; } /// - /// If true the cmdlet will stop processing at the first successful match and + /// Gets or sets a value indicating if the cmdlet will stop processing at the first successful match and /// return true. If both List and Quiet parameters are given, an exception is thrown. /// [Parameter] - public SwitchParameter Quiet - { - get - { - return _quiet; - } - set - { - _quiet = value; - } - } - private bool _quiet; + public SwitchParameter Quiet { get; set; } /// - /// list files where a match is found + /// Gets or sets a value indicating if matching files should be listed. /// This is the Unix functionality this switch is intended to mimic; /// the actual action of this option is to stop after the first match /// is found and returned from any particular file. /// [Parameter] - public SwitchParameter List - { - get - { - return _list; - } - set - { - _list = value; - } - } - private bool _list; + public SwitchParameter List { get; set; } /// - /// Lets you include particular files. Files not matching - /// one of these (if specified) are excluded. + /// Gets or sets files to include. Files matching + /// one of these (if specified) are included. /// /// Invalid wildcard pattern was specified. [Parameter] [ValidateNotNullOrEmpty] public string[] Include { - get - { - return includeStrings; - } + get => _includeStrings; set { // null check is not needed (because of ValidateNotNullOrEmpty), // but we have to include it to silence OACR - if (value == null) - { - throw PSTraceSource.NewArgumentNullException("value"); - } - - includeStrings = value; + _includeStrings = value ?? throw PSTraceSource.NewArgumentNullException("value"); - this.include = new WildcardPattern[includeStrings.Length]; - for (int i = 0; i < includeStrings.Length; i++) + this._include = new WildcardPattern[_includeStrings.Length]; + for (int i = 0; i < _includeStrings.Length; i++) { - this.include[i] = WildcardPattern.Get(includeStrings[i], WildcardOptions.IgnoreCase); + this._include[i] = WildcardPattern.Get(_includeStrings[i], WildcardOptions.IgnoreCase); } } } - internal string[] includeStrings = null; - internal WildcardPattern[] include = null; + + private string[] _includeStrings; + + private WildcardPattern[] _include; /// - /// Lets you exclude particular files. Files matching + /// Gets or sets files to exclude. Files matching /// one of these (if specified) are excluded. /// [Parameter] [ValidateNotNullOrEmpty] public string[] Exclude { - get - { - return excludeStrings; - } + get => _excludeStrings; set { // null check is not needed (because of ValidateNotNullOrEmpty), // but we have to include it to silence OACR - if (value == null) - { - throw PSTraceSource.NewArgumentNullException("value"); - } + _excludeStrings = value ?? throw PSTraceSource.NewArgumentNullException("value"); - excludeStrings = value; - - this.exclude = new WildcardPattern[excludeStrings.Length]; - for (int i = 0; i < excludeStrings.Length; i++) + this._exclude = new WildcardPattern[_excludeStrings.Length]; + for (int i = 0; i < _excludeStrings.Length; i++) { - this.exclude[i] = WildcardPattern.Get(excludeStrings[i], WildcardOptions.IgnoreCase); + this._exclude[i] = WildcardPattern.Get(_excludeStrings[i], WildcardOptions.IgnoreCase); } } } - internal string[] excludeStrings; - internal WildcardPattern[] exclude; + + private string[] _excludeStrings; + + private WildcardPattern[] _exclude; /// - /// Only show lines which do not match. + /// Gets or sets a value indicating whether to only show lines which do not match. /// Equivalent to grep -v/findstr -v. /// [Parameter] - public SwitchParameter NotMatch - { - get - { - return _notMatch; - } - set - { - _notMatch = value; - } - } - private bool _notMatch; + public SwitchParameter NotMatch { get; set; } /// - /// If set, sets the Matches property of MatchInfo to the result - /// of calling System.Text.RegularExpressions.Regex.Matches() on + /// Gets or sets a value indicating whether the Matches property of MatchInfo should be set + /// to the result of calling System.Text.RegularExpressions.Regex.Matches() on /// the corresponding line. /// Has no effect if -SimpleMatch is also specified. /// [Parameter] - public SwitchParameter AllMatches - { - get - { - return _allMatches; - } - set - { - _allMatches = value; - } - } - private bool _allMatches; + public SwitchParameter AllMatches { get; set; } /// - /// The text encoding to process each file as. + /// Gets or sets the text encoding to process each file as. /// [Parameter] [ArgumentToEncodingTransformationAttribute()] @@ -1213,7 +1129,7 @@ public SwitchParameter AllMatches public Encoding Encoding { get; set; } = ClrFacade.GetDefaultEncoding(); /// - /// The number of context lines to collect. If set to a + /// Gets or sets the number of context lines to collect. If set to a /// single integer value N, collects N lines each of pre- /// and post- context. If set to a 2-tuple B,A, collects B /// lines of pre- and A lines of post- context. @@ -1226,20 +1142,12 @@ public SwitchParameter AllMatches [ValidateRange(0, Int32.MaxValue)] public new int[] Context { - get - { - return _context; - } + get => _context; set { // null check is not needed (because of ValidateNotNullOrEmpty), // but we have to include it to silence OACR - if (value == null) - { - throw PSTraceSource.NewArgumentNullException("value"); - } - - _context = value; + _context = value ?? throw PSTraceSource.NewArgumentNullException("value"); if (_context.Length == 1) { @@ -1253,8 +1161,11 @@ public SwitchParameter AllMatches } } } + private int[] _context; + private int _preContext = 0; + private int _postContext = 0; private IContextTracker GetContextTracker() => (_preContext == 0 && _postContext == 0) ? _noContextTracker : new ContextTracker(_preContext, _postContext); @@ -1266,8 +1177,10 @@ public SwitchParameter AllMatches // use a single global tracker for both is that in the case of // a mixed list of strings and FileInfo, the context tracker // would get reset after each file. - private IContextTracker _globalContextTracker = null; + private IContextTracker _globalContextTracker; + private IContextTracker _noContextTracker; + /// /// This is used to handle the case were we're done processing input objects. /// If true, process record will just return. @@ -1276,16 +1189,14 @@ public SwitchParameter AllMatches private int _inputRecordNumber; - - /// /// Read command line parameters. /// protected override void BeginProcessing() { - if (!_simpleMatch) + if (!SimpleMatch) { - RegexOptions regexOptions = (_caseSensitive) ? RegexOptions.None : RegexOptions.IgnoreCase; + RegexOptions regexOptions = CaseSensitive ? RegexOptions.None : RegexOptions.IgnoreCase; _regexPattern = new Regex[Pattern.Length]; for (int i = 0; i < Pattern.Length; i++) { @@ -1300,17 +1211,17 @@ protected override void BeginProcessing() } } } + _noContextTracker = new NoContextTracker(); _globalContextTracker = GetContextTracker(); } - private readonly List _inputObjectFileList = new List(1){string.Empty}; + private readonly List _inputObjectFileList = new List(1) { string.Empty }; /// /// Process the input. /// - /// Does not return a value - /// Regular expression parsing error, path error + /// Regular expression parsing error, path error. /// A file cannot be found. /// A file cannot be found. protected override void ProcessRecord() @@ -1352,18 +1263,17 @@ protected override void ProcessRecord() } var foundMatch = ProcessFile(filename); - if (_quiet && foundMatch) + if (Quiet && foundMatch) { return; } } // No results in any files. - if (_quiet) + if (Quiet) { - var res = _list ? null : Boxed.False; + var res = List ? null : Boxed.False; WriteObject(res); - } } else @@ -1374,16 +1284,15 @@ protected override void ProcessRecord() bool matched; MatchInfo result; MatchInfo matchInfo = null; - var line = _inputObject.BaseObject as string; - if (line != null) + if (_inputObject.BaseObject is string line) { - matched = doMatch(line, out result); + matched = DoMatch(line, out result); } else { matchInfo = _inputObject.BaseObject as MatchInfo; object objectToCheck = matchInfo ?? (object)_inputObject; - matched = doMatch(objectToCheck, out result, out line); + matched = DoMatch(objectToCheck, out result, out line); } if (matched) @@ -1393,6 +1302,7 @@ protected override void ProcessRecord() { result.LineNumber = _inputRecordNumber; } + // doMatch will have already set the pattern and line text... _globalContextTracker.TrackMatch(result); } @@ -1406,8 +1316,10 @@ protected override void ProcessRecord() { // If we're in quiet mode, go ahead and stop processing // now. - if (_quiet) + if (Quiet) + { _doneProcessing = true; + } } } } @@ -1428,14 +1340,16 @@ private bool ProcessFile(string filename) try { // see if the file is one the include exclude list... - if (!meetsIncludeExcludeCriteria(filename)) + if (!MeetsIncludeExcludeCriteria(filename)) + { return false; + } using (FileStream fs = new FileStream(filename, FileMode.Open, FileAccess.Read, FileShare.ReadWrite)) { using (StreamReader sr = new StreamReader(fs, Encoding)) { - String line; + string line; int lineNo = 0; // Read and display lines from the file until the end of @@ -1444,9 +1358,7 @@ private bool ProcessFile(string filename) { lineNo++; - MatchInfo result; - - if (doMatch(line, out result)) + if (DoMatch(line, out MatchInfo result)) { result.Path = filename; result.LineNumber = lineNo; @@ -1467,15 +1379,12 @@ private bool ProcessFile(string filename) // this file. It's done this way so the file is closed before emitting // the result so the downstream cmdlet can actually manipulate the file // that was found. - - if (_quiet || _list) + if (Quiet || List) { break; } - else - { - FlushTrackerQueue(contextTracker); - } + + FlushTrackerQueue(contextTracker); } } } @@ -1487,7 +1396,9 @@ private bool ProcessFile(string filename) // our postcontext. contextTracker.TrackEOF(); if (FlushTrackerQueue(contextTracker)) + { foundMatch = true; + } } catch (System.NotSupportedException nse) { @@ -1518,14 +1429,16 @@ private bool FlushTrackerQueue(IContextTracker contextTracker) { // Do we even have any matches to emit? if (contextTracker.EmitQueue.Count < 1) + { return false; + } // If -quiet is specified but not -list return true on first match - if (_quiet && !_list) + if (Quiet && !List) { WriteObject(true); } - else if (_list) + else if (List) { WriteObject(contextTracker.EmitQueue[0]); } @@ -1550,15 +1463,17 @@ protected override void EndProcessing() // Check for a leftover match that was still tracking context. _globalContextTracker.TrackEOF(); if (!_doneProcessing) + { FlushTrackerQueue(_globalContextTracker); + } } - private bool doMatch(string operandString, out MatchInfo matchResult) + private bool DoMatch(string operandString, out MatchInfo matchResult) { - return doMatchWorker(operandString, null, out matchResult); + return DoMatchWorker(operandString, null, out matchResult); } - private bool doMatch(object operand, out MatchInfo matchResult, out string operandString) + private bool DoMatch(object operand, out MatchInfo matchResult, out string operandString) { MatchInfo matchInfo = operand as MatchInfo; if (matchInfo != null) @@ -1585,27 +1500,25 @@ private bool doMatch(object operand, out MatchInfo matchResult, out string opera operandString = (string)LanguagePrimitives.ConvertTo(operand, typeof(string), CultureInfo.InvariantCulture); } - return doMatchWorker(operandString, matchInfo, out matchResult); + return DoMatchWorker(operandString, matchInfo, out matchResult); } /// /// Check the operand and see if it matches, if this.quiet is not set, then /// return a partially populated MatchInfo object with Line, Pattern, IgnoreCase set. /// - /// - /// the match info object - this will be - /// null if this.quiet is set. - /// the result of converting operand to - /// a string. - /// true if the input object matched - private bool doMatchWorker(string operandString, MatchInfo matchInfo, out MatchInfo matchResult) + /// The result of converting operand to a string. + /// The input object in filter mode. + /// The match info object - this will be null if this.quiet is set. + /// true if the input object matched. + private bool DoMatchWorker(string operandString, MatchInfo matchInfo, out MatchInfo matchResult) { bool gotMatch = false; Match[] matches = null; int patternIndex = 0; matchResult = null; - if (!_simpleMatch) + if (!SimpleMatch) { while (patternIndex < Pattern.Length) { @@ -1614,7 +1527,7 @@ private bool doMatchWorker(string operandString, MatchInfo matchInfo, out MatchI // Only honor allMatches if notMatch is not set, // since it's a fairly expensive operation and // notMatch takes precedent over allMatch. - if (_allMatches && !_notMatch) + if (AllMatches && !NotMatch) { MatchCollection mc = r.Matches(operandString); if (mc.Count > 0) @@ -1636,14 +1549,16 @@ private bool doMatchWorker(string operandString, MatchInfo matchInfo, out MatchI } if (gotMatch) + { break; + } patternIndex++; } } else { - StringComparison compareOption = _caseSensitive ? + StringComparison compareOption = CaseSensitive ? StringComparison.CurrentCulture : StringComparison.CurrentCultureIgnoreCase; while (patternIndex < Pattern.Length) { @@ -1659,9 +1574,10 @@ private bool doMatchWorker(string operandString, MatchInfo matchInfo, out MatchI } } - if (_notMatch) + if (NotMatch) { gotMatch = !gotMatch; + // If notMatch was specified with multiple // patterns, then *none* of the patterns // matched and any pattern could be picked @@ -1698,10 +1614,12 @@ private bool doMatchWorker(string operandString, MatchInfo matchInfo, out MatchI } // otherwise construct and populate a new MatchInfo object - matchResult = new MatchInfo(); - matchResult.IgnoreCase = !_caseSensitive; - matchResult.Line = operandString; - matchResult.Pattern = Pattern[patternIndex]; + matchResult = new MatchInfo + { + IgnoreCase = !CaseSensitive, + Line = operandString, + Pattern = Pattern[patternIndex] + }; if (_preContext > 0 || _postContext > 0) { @@ -1714,23 +1632,28 @@ private bool doMatchWorker(string operandString, MatchInfo matchInfo, out MatchI return true; } + return false; } + /// /// Get a list or resolved file paths. + /// + /// The filePaths to resolve. + /// True if the wildcard resolution should not be attempted. + /// The resolved (absolute) paths. private List ResolveFilePaths(string[] filePaths, bool isLiteralPath) { - ProviderInfo provider; List allPaths = new List(); foreach (string path in filePaths) { Collection resolvedPaths; + ProviderInfo provider; if (isLiteralPath) { resolvedPaths = new Collection(); - PSDriveInfo drive; - string resolvedPath = SessionState.Path.GetUnresolvedProviderPathFromPSPath(path, out provider, out drive); + string resolvedPath = SessionState.Path.GetUnresolvedProviderPathFromPSPath(path, out provider, out _); resolvedPaths.Add(resolvedPath); } else @@ -1738,12 +1661,13 @@ private List ResolveFilePaths(string[] filePaths, bool isLiteralPath) resolvedPaths = GetResolvedProviderPathFromPSPath(path, out provider); } - if (!provider.NameEquals(((PSCmdlet)this).Context.ProviderNames.FileSystem)) + if (!provider.NameEquals(base.Context.ProviderNames.FileSystem)) { // "The current provider ({0}) cannot open a file" WriteError(BuildErrorRecord(MatchStringStrings.FileOpenError, provider.FullName, "ProcessingFile", null)); continue; } + allPaths.AddRange(resolvedPaths); } @@ -1782,15 +1706,15 @@ public override object Transform(EngineIntrinsics engineIntrinsics, object input { object result = inputData; - PSObject mso = result as PSObject; - if (mso != null) + if (result is PSObject mso) + { result = mso.BaseObject; + } - IList argList = result as IList; FileInfo fileInfo; // Handle an array of elements... - if (argList != null) + if (result is IList argList) { object[] resultList = new object[argList.Count]; @@ -1800,21 +1724,23 @@ public override object Transform(EngineIntrinsics engineIntrinsics, object input mso = element as PSObject; if (mso != null) + { element = mso.BaseObject; + } fileInfo = element as FileInfo; - - if (fileInfo != null) - resultList[i] = fileInfo.FullName; - else resultList[i] = element; + resultList[i] = fileInfo?.FullName ?? element; } + return resultList; } // Handle the singleton case... fileInfo = result as FileInfo; if (fileInfo != null) + { return fileInfo.FullName; + } return inputData; } @@ -1825,16 +1751,16 @@ public override object Transform(EngineIntrinsics engineIntrinsics, object input /// That is - it's on the include list if there is one and not on /// the exclude list if there was one of those. /// - /// + /// The filename to test. /// True if the filename is acceptable. - private bool meetsIncludeExcludeCriteria(string filename) + private bool MeetsIncludeExcludeCriteria(string filename) { bool ok = false; // see if the file is on the include list... - if (this.include != null) + if (this._include != null) { - foreach (WildcardPattern patternItem in this.include) + foreach (WildcardPattern patternItem in this._include) { if (patternItem.IsMatch(filename)) { @@ -1849,12 +1775,14 @@ private bool meetsIncludeExcludeCriteria(string filename) } if (!ok) + { return false; + } // now see if it's on the exclude list... - if (this.exclude != null) + if (this._exclude != null) { - foreach (WildcardPattern patternItem in this.exclude) + foreach (WildcardPattern patternItem in this._exclude) { if (patternItem.IsMatch(filename)) { From fe15a92657793f7c4b80908e4e12c5ec53af617a Mon Sep 17 00:00:00 2001 From: Staffan Gustafsson Date: Mon, 15 Oct 2018 23:33:23 +0200 Subject: [PATCH 3/3] Addressing Ilyas comments. Also removing the non-referenced property FileName. --- .../commands/utility/MatchString.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/MatchString.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/MatchString.cs index f3210d4f7d5..c6ffc953586 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/MatchString.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/MatchString.cs @@ -125,7 +125,7 @@ public string Filename /// The path name. public string Path { - get => !_pathSet ? s_inputStream : _path; + get => _pathSet ? _path : s_inputStream; set { _path = value; @@ -1062,7 +1062,7 @@ public string[] Include { // null check is not needed (because of ValidateNotNullOrEmpty), // but we have to include it to silence OACR - _includeStrings = value ?? throw PSTraceSource.NewArgumentNullException("value"); + _includeStrings = value ?? throw PSTraceSource.NewArgumentNullException(nameof(value)); this._include = new WildcardPattern[_includeStrings.Length]; for (int i = 0; i < _includeStrings.Length; i++)