Skip to content

WIP: Improve the performance of the formatter #1280

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

Closed
wants to merge 3 commits into from
Closed

WIP: Improve the performance of the formatter #1280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
384c2a3
Move format logic to new data structure
rjmholt Jul 1, 2019
1653212
Fix string logic
rjmholt Jul 2, 2019
0693d9c
Fix issues with cross-plat newlines
Jul 3, 2019
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
2 changes: 1 addition & 1 deletion Engine/EditableText.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public EditableText ApplyEdit(TextEdit textEdit)
{
ValidateTextEdit(textEdit);

var editLines = textEdit.Lines;
string[] editLines = textEdit.Lines;

// Get the first fragment of the first line
string firstLineFragment =
Expand Down
89 changes: 56 additions & 33 deletions Engine/Formatter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@

using System;
using System.Collections;
using System.Collections.Generic;
using System.Management.Automation;
using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;

namespace Microsoft.Windows.PowerShell.ScriptAnalyzer
{
Expand All @@ -12,6 +14,16 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer
/// </summary>
public class Formatter
{
private static readonly IEnumerable<string> s_formattingRulesInOrder = new []
{
"PSPlaceCloseBrace",
"PSPlaceOpenBrace",
"PSUseConsistentWhitespace",
"PSUseConsistentIndentation",
"PSAlignAssignmentStatement",
"PSUseCorrectCasing"
};

/// <summary>
/// Format a powershell script.
/// </summary>
Expand All @@ -27,59 +39,70 @@ public static string Format<TCmdlet>(
TCmdlet cmdlet) where TCmdlet : PSCmdlet, IOutputWriter
{
// todo implement notnull attribute for such a check
ValidateNotNull(scriptDefinition, "scriptDefinition");
ValidateNotNull(settings, "settings");
ValidateNotNull(cmdlet, "cmdlet");
ValidateNotNull(scriptDefinition, nameof(scriptDefinition));
ValidateNotNull(settings, nameof(settings));
ValidateNotNull(cmdlet, nameof(cmdlet));

Helper.Instance = new Helper(cmdlet.SessionState.InvokeCommand, cmdlet);
Helper.Instance.Initialize();

var ruleOrder = new string[]
{
"PSPlaceCloseBrace",
"PSPlaceOpenBrace",
"PSUseConsistentWhitespace",
"PSUseConsistentIndentation",
"PSAlignAssignmentStatement",
"PSUseCorrectCasing"
};
Settings currentSettings = GetCurrentSettings(settings);
ScriptAnalyzer.Instance.UpdateSettings(currentSettings);
ScriptAnalyzer.Instance.Initialize(cmdlet, includeDefaultRules: true);

var text = new EditableText(scriptDefinition);
foreach (var rule in ruleOrder)
try
{
if (!settings.RuleArguments.ContainsKey(rule))
{
continue;
}

var currentSettings = GetCurrentSettings(settings, rule);
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);
range = updatedRange;
return ScriptAnalyzer.Instance.Fix(scriptDefinition, range);
}
catch (FormattingException e)
{
cmdlet.ThrowTerminatingError(
new ErrorRecord(
e,
"FIX_ERROR",
ErrorCategory.InvalidOperation,
e.Corrections));
return null;
}

return text.ToString();
}

private static void ValidateNotNull<T>(T obj, string name)
private static void ValidateNotNull(object obj, string name)
{
if (obj == null)
{
throw new ArgumentNullException(name);
}
}

private static Settings GetCurrentSettings(Settings settings, string rule)
private static Settings GetCurrentSettings(Settings settings)
{
var ruleSettings = new Hashtable();
foreach (string rule in s_formattingRulesInOrder)
{
if (settings.RuleArguments.TryGetValue(rule, out Dictionary<string, object> ruleConfiguration))
{
ruleSettings[rule] = ruleConfiguration;
}
}

return new Settings(new Hashtable()
{
{"IncludeRules", new string[] {rule}},
{"Rules", new Hashtable() { { rule, new Hashtable(settings.RuleArguments[rule]) } } }
{ "IncludeRules", s_formattingRulesInOrder },
{ "Rules", ruleSettings }
});
}
}

public class FormattingException : Exception
{
public FormattingException(
string message,
IReadOnlyList<CorrectionExtent> corrections)
: base(message)
{
Corrections = corrections;
}

public IReadOnlyList<CorrectionExtent> Corrections { get; }
}
}
50 changes: 50 additions & 0 deletions Engine/Generic/CorrectionExtent.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,4 +105,54 @@ public CorrectionExtent(

}
}

internal struct CorrectionComparer : IComparer<CorrectionExtent>, IEqualityComparer<CorrectionExtent>
{
public int Compare(CorrectionExtent x, CorrectionExtent y)
{
if (x.StartLineNumber > y.StartLineNumber)
{
return 1;
}

if (x.StartLineNumber < y.StartLineNumber)
{
return -1;
}

if (x.StartColumnNumber > y.StartColumnNumber)
{
return 1;
}

if (x.StartColumnNumber < y.StartColumnNumber)
{
return -1;
}

if (x.Text.Length > y.Text.Length)
{
return 1;
}

if (x.Text.Length < y.Text.Length)
{
return -1;
}

return 0;
}

public bool Equals(CorrectionExtent x, CorrectionExtent y)
{
return Compare(x, y) == 0;
}

public int GetHashCode(CorrectionExtent obj)
{
return obj != null
? obj.GetHashCode()
: 0;
}
}
}
129 changes: 117 additions & 12 deletions Engine/ScriptAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ public sealed class ScriptAnalyzer
{
#region Private members

private readonly CorrectionComparer s_correctionComparer = new CorrectionComparer();

private IOutputWriter outputWriter;
private Dictionary<string, object> settings;
private readonly Regex s_aboutHelpRegex = new Regex("^about_.*help\\.txt$", RegexOptions.IgnoreCase | RegexOptions.Compiled);
Expand Down Expand Up @@ -1574,22 +1576,24 @@ public EditableText Fix(EditableText text, Range range, out Range updatedRange,
throw new ArgumentNullException(nameof(text));
}

var isRangeNull = range == null;
bool isRangeNull = range == null;
if (!isRangeNull && !text.IsValidRange(range))
{
this.outputWriter.ThrowTerminatingError(new ErrorRecord(
new ArgumentException("Invalid Range", nameof(range)),
"FIX_ERROR",
ErrorCategory.InvalidArgument,
range));
new ArgumentException(
"Invalid Range",
nameof(range)),
"FIX_ERROR",
ErrorCategory.InvalidArgument,
range));
}

range = isRangeNull ? null : SnapToEdges(text, range);
var previousLineCount = text.LineCount;
var previousUnusedCorrections = 0;
int previousLineCount = text.LineCount;
int previousUnusedCorrections = 0;
do
{
var records = AnalyzeScriptDefinition(text.ToString());
IEnumerable<DiagnosticRecord> records = AnalyzeScriptDefinition(text.ToString());
var corrections = records
.Select(r => r.SuggestedCorrections)
.Where(sc => sc != null && sc.Any())
Expand All @@ -1598,9 +1602,8 @@ public EditableText Fix(EditableText text, Range range, out Range updatedRange,
.ToList();

this.outputWriter.WriteVerbose($"Found {corrections.Count} violations.");
int unusedCorrections;
Fix(text, corrections, out unusedCorrections);
var numberOfFixedViolatons = corrections.Count - unusedCorrections;
Fix(text, corrections, out int unusedCorrections);
int numberOfFixedViolatons = corrections.Count - unusedCorrections;
fixesWereApplied = numberOfFixedViolatons > 0;
this.outputWriter.WriteVerbose($"Fixed {numberOfFixedViolatons} violations.");

Expand All @@ -1616,7 +1619,7 @@ public EditableText Fix(EditableText text, Range range, out Range updatedRange,
}

previousUnusedCorrections = unusedCorrections;
var lineCount = text.LineCount;
int lineCount = text.LineCount;
if (!isRangeNull && lineCount != previousLineCount)
{
range = new Range(
Expand All @@ -1632,6 +1635,108 @@ public EditableText Fix(EditableText text, Range range, out Range updatedRange,
return text;
}

internal string Fix(string scriptContent, Range fixRange)
{
if (scriptContent == null)
{
throw new ArgumentNullException(nameof(scriptContent));
}

var scriptText = new TextDocumentBuilder(scriptContent);

if (fixRange != null)
{
var fixTextRange = new TextRange(
new TextPosition(fixRange.Start.Line - 1, fixRange.Start.Column - 1),
new TextPosition(fixRange.End.Line - 1, fixRange.End.Column - 1));

if (!scriptText.IsValidRange(fixTextRange))
{
this.outputWriter.ThrowTerminatingError(new ErrorRecord(
new ArgumentException(
"Invalid Range",
nameof(fixRange)),
"FIX_ERROR",
ErrorCategory.InvalidArgument,
fixRange));
}
}

int unappliedCorrectionCount = -1;
int previousUnappliedCorrections = -1;
int fixCount = 0;
do
{
previousUnappliedCorrections = unappliedCorrectionCount;
unappliedCorrectionCount = 0;

// First filter out diagnostics with no corrections,
// or ones not applied in the valid range
var possiblyOverlappingCorrections = new List<CorrectionExtent>();
foreach (DiagnosticRecord record in AnalyzeScriptDefinition(scriptText.ToString()))
{
if (record.SuggestedCorrections == null
|| !record.SuggestedCorrections.Any())
{
continue;
}

CorrectionExtent correction = record.SuggestedCorrections.First();

if (fixRange != null
&& !(correction.Start >= fixRange.Start && correction.End <= fixRange.End))
{
continue;
}

possiblyOverlappingCorrections.Add(correction);
}

// We now need the list to be sorted for a second pass
possiblyOverlappingCorrections.Sort(s_correctionComparer);

// Remove corrections that lie within the range of a predecessor.
// The sorting function we use
CorrectionExtent previousCorrection = null;
var corrections = new List<CorrectionExtent>(possiblyOverlappingCorrections.Count);
var unappliedCorrections = new List<CorrectionExtent>();
foreach (CorrectionExtent correction in possiblyOverlappingCorrections)
{
if (previousCorrection != null
&& (correction.Start >= previousCorrection.Start && correction.Start <= previousCorrection.End
|| correction.End >= previousCorrection.Start && correction.End <= previousCorrection.End))
{
unappliedCorrectionCount++;
continue;
}

corrections.Add(correction);
previousCorrection = correction;
}

if (unappliedCorrectionCount > 0
&& unappliedCorrectionCount == previousUnappliedCorrections)
{
throw new FormattingException(
"Unable to apply all fixes to script",
unappliedCorrections);
}

this.outputWriter.WriteVerbose($"Found {corrections.Count} violations.");

if (corrections.Count > 0)
{
fixCount += corrections.Count;
scriptText.ApplyCorrections(corrections);
}

} while (unappliedCorrectionCount > 0);

this.outputWriter.WriteVerbose($"Fixed {fixCount} violations.");

return scriptText.ToString();
}

private static Encoding GetFileEncoding(string path)
{
using (var stream = new FileStream(path, FileMode.Open))
Expand Down
Loading