From 8cbd179805673d7ccf0e6eaac7ca364db39f5a4d Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 25 Apr 2020 21:33:15 +0100 Subject: [PATCH 1/5] Recycle Parse result for formatter, if a rule does not return a DiagnosticRecord and therefore does not require re-parsing --- .../Commands/InvokeScriptAnalyzerCommand.cs | 2 +- Engine/Formatter.cs | 9 +++-- Engine/ScriptAnalyzer.cs | 40 +++++++++++++------ 3 files changed, 34 insertions(+), 17 deletions(-) diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index e7c800f54..f5598d5bd 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -422,7 +422,7 @@ private void ProcessInput() } else if (String.Equals(this.ParameterSetName, "ScriptDefinition", StringComparison.OrdinalIgnoreCase)) { - diagnosticsList = ScriptAnalyzer.Instance.AnalyzeScriptDefinition(scriptDefinition); + diagnosticsList = ScriptAnalyzer.Instance.AnalyzeScriptDefinition(scriptDefinition, out _, out _); WriteToOutput(diagnosticsList); } } diff --git a/Engine/Formatter.cs b/Engine/Formatter.cs index 76557856d..15db1b39d 100644 --- a/Engine/Formatter.cs +++ b/Engine/Formatter.cs @@ -4,6 +4,7 @@ using System; using System.Collections; using System.Management.Automation; +using System.Management.Automation.Language; namespace Microsoft.Windows.PowerShell.ScriptAnalyzer { @@ -46,6 +47,9 @@ public static string Format( }; var text = new EditableText(scriptDefinition); + ScriptBlockAst scriptAst = null; + Token[] scriptTokens = null; + bool skipParsing = false; foreach (var rule in ruleOrder) { if (!settings.RuleArguments.ContainsKey(rule)) @@ -57,9 +61,8 @@ public static string Format( ScriptAnalyzer.Instance.UpdateSettings(currentSettings); ScriptAnalyzer.Instance.Initialize(cmdlet, null, null, null, null, true, false); - Range updatedRange; - bool fixesWereApplied; - text = ScriptAnalyzer.Instance.Fix(text, range, out updatedRange, out fixesWereApplied, skipVariableAnalysis: true); + text = ScriptAnalyzer.Instance.Fix(text, range, skipParsing, out Range updatedRange, out bool fixesWereApplied, ref scriptAst, ref scriptTokens, skipVariableAnalysis: true); + skipParsing = !fixesWereApplied; range = updatedRange; } diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 811c265fd..6f3cd8f5a 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -1478,8 +1478,7 @@ public IEnumerable AnalyzeAndFixPath(string path, Func AnalyzeAndFixPath(string path, Func /// Analyzes a script definition in the form of a string input /// - /// The script to be analyzed + /// The script to be analyzed. + /// Parsed AST of . + /// Parsed tokens of + /// Whether variable analysis can be skipped (applicable if rules do not use variable analysis APIs). /// - public IEnumerable AnalyzeScriptDefinition(string scriptDefinition, bool skipVariableAnalysis = false) + public IEnumerable AnalyzeScriptDefinition(string scriptDefinition, out ScriptBlockAst scriptAst, out Token[] scriptTokens, bool skipVariableAnalysis = false) { - ScriptBlockAst scriptAst = null; - Token[] scriptTokens = null; + scriptAst = null; + scriptTokens = null; ParseError[] errors = null; this.outputWriter.WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseScriptDefinitionMessage)); @@ -1546,7 +1548,7 @@ public IEnumerable AnalyzeScriptDefinition(string scriptDefini /// Fix the violations in the given script text. /// /// The script text to be fixed. - /// Whether any warnings were fixed. + /// Whether any warnings were fixed. /// The fixed script text. public string Fix(string scriptDefinition, out bool fixesWereApplied) { @@ -1555,20 +1557,24 @@ public string Fix(string scriptDefinition, out bool fixesWereApplied) throw new ArgumentNullException(nameof(scriptDefinition)); } - Range updatedRange; - return Fix(new EditableText(scriptDefinition), null, out updatedRange, out fixesWereApplied).ToString(); + ScriptBlockAst scriptAst = null; + Token[] scriptTokens = null; + return Fix(new EditableText(scriptDefinition), range: null, skipParsing: false, out _, out fixesWereApplied, ref scriptAst, ref scriptTokens).ToString(); } /// /// Fix the violations in the given script text. /// - /// An object of type `EditableText` that encapsulates the script text to be fixed. + /// An object of type that encapsulates the script text to be fixed. /// The range in which the fixes are allowed. + /// Whether to use the and parameters instead of parsing the parameter. /// The updated range after the fixes have been applied. - /// Whether any warnings were fixed. + /// Whether any warnings were fixed. + /// Optionally pre-parsed AST. + /// Optionally pre-parsed tokens. /// Whether to skip variable analysis. /// The same instance of `EditableText` that was passed to the method, but the instance encapsulates the fixed script text. This helps in chaining the Fix method. - public EditableText Fix(EditableText text, Range range, out Range updatedRange, out bool fixesWereApplied, bool skipVariableAnalysis = false) + public EditableText Fix(EditableText text, Range range, bool skipParsing, out Range updatedRange, out bool fixesWereApplied, ref ScriptBlockAst scriptAst, ref Token[] scriptTokens, bool skipVariableAnalysis = false) { if (text == null) { @@ -1590,7 +1596,15 @@ public EditableText Fix(EditableText text, Range range, out Range updatedRange, var previousUnusedCorrections = 0; do { - var records = AnalyzeScriptDefinition(text.ToString(), skipVariableAnalysis); + IEnumerable records; + if (skipParsing) + { + records = AnalyzeSyntaxTree(scriptAst, scriptTokens, String.Empty, skipVariableAnalysis); + } + else + { + records = AnalyzeScriptDefinition(text.ToString(), out scriptAst, out scriptTokens, skipVariableAnalysis); + } var corrections = records .Select(r => r.SuggestedCorrections) .Where(sc => sc != null && sc.Any()) From 5932cfa087e8a568d58d5d98658e253e48e5ad06 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 25 Apr 2020 23:22:30 +0100 Subject: [PATCH 2/5] fix issue where the parsing stage can only be skipped in the first loop iteration of the Fix API --- Engine/ScriptAnalyzer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 6f3cd8f5a..d209a39cb 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -1597,7 +1597,7 @@ public EditableText Fix(EditableText text, Range range, bool skipParsing, out Ra do { IEnumerable records; - if (skipParsing) + if (skipParsing && previousUnusedCorrections == 0) { records = AnalyzeSyntaxTree(scriptAst, scriptTokens, String.Empty, skipVariableAnalysis); } From 12a6f550cc7fde05c7bca9b1da973780095ab4d7 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sun, 26 Apr 2020 00:36:23 +0100 Subject: [PATCH 3/5] Adapt LibraryUsage.tests.ps1 --- Tests/Engine/LibraryUsage.tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Engine/LibraryUsage.tests.ps1 b/Tests/Engine/LibraryUsage.tests.ps1 index 008483908..279f48238 100644 --- a/Tests/Engine/LibraryUsage.tests.ps1 +++ b/Tests/Engine/LibraryUsage.tests.ps1 @@ -94,7 +94,7 @@ function Invoke-ScriptAnalyzer { } else { - $results = $scriptAnalyzer.AnalyzeScriptDefinition($ScriptDefinition); + $results = $scriptAnalyzer.AnalyzeScriptDefinition($ScriptDefinition, [ref] $null, [ref] $null) } $results From e6ae0ff7222e37b725b61dcd4bdab6c87e340441 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sun, 26 Apr 2020 09:17:35 +0100 Subject: [PATCH 4/5] Remove unnecessary call to ToList() and Count() where object is already a list --- Engine/ScriptAnalyzer.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index d209a39cb..ffe277375 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -1698,22 +1698,21 @@ private static Range SnapToEdges(EditableText text, Range range) } private static IEnumerable GetCorrectionExtentsForFix( - IEnumerable correctionExtents) + List correctionExtents) { - var ceList = correctionExtents.ToList(); - ceList.Sort((x, y) => + correctionExtents.Sort((x, y) => { return x.StartLineNumber < y.StartLineNumber ? 1 : (x.StartLineNumber == y.StartLineNumber ? 0 : -1); }); - return ceList.GroupBy(ce => ce.StartLineNumber).Select(g => g.First()); + return correctionExtents.GroupBy(ce => ce.StartLineNumber).Select(g => g.First()); } private static EditableText Fix( EditableText text, - IEnumerable correctionExtents, + List correctionExtents, out int unusedCorrections) { var correctionsToUse = GetCorrectionExtentsForFix(correctionExtents); @@ -1724,7 +1723,7 @@ private static EditableText Fix( text.ApplyEdit(correction); } - unusedCorrections = correctionExtents.Count() - count; + unusedCorrections = correctionExtents.Count - count; return text; } From 0d01812b379e515197b80f4ab6c3b9473b2638ac Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Mon, 27 Apr 2020 22:27:31 +0100 Subject: [PATCH 5/5] re-trigger ci