- Removed 'goto' from 'LineFormatter', straighten case breaks

- Added TextEditCollectionAssertions
- Various miscellaneous code clean-ups

Author: AlexanderSher

src/Analysis/Engine/Impl/Infrastructure/Extensions/StringBuilderExtensions.cs

@@ -25,19 +25,19 @@ public static StringBuilder TrimEnd(this StringBuilder sb) {
             return sb;
         }
 
-        public static StringBuilder EnsureEndsWithSpace(this StringBuilder sb, int count = 1, bool allowLeading = false) {
-            if (sb.Length == 0 && !allowLeading) {
+        public static StringBuilder EnsureEndsWithWhiteSpace(this StringBuilder sb, int whiteSpaceCount = 1) {
+            if (sb.Length == 0) {
                 return sb;
             }
 
-            for (var i = sb.Length - 1; i >= 0 && char.IsWhiteSpace(sb[i]); i--) {
-                count--;
+            for (var i = sb.Length - 1; i >= 0 && whiteSpaceCount > 0 && char.IsWhiteSpace(sb[i]); i--) {
+                whiteSpaceCount--;
             }
 
-            if (count > 0) {
-                sb.Append(new string(' ', count));
+            if (whiteSpaceCount > 0) {
+                sb.Append(' ', whiteSpaceCount);
             }
-
+            
             return sb;
         }
     }

src/Analysis/Engine/Test/FluentAssertions/AssertionsFactory.cs

@@ -102,5 +102,8 @@ public static SignatureHelpAssertions Should(this SignatureHelp signatureHelp)
 
         public static SignatureInformationAssertions Should(this SignatureInformation signatureInformation)
             => new SignatureInformationAssertions(signatureInformation);
+
+        public static TextEditCollectionAssertions Should(this IEnumerable<TextEdit> textEdits)
+            => new TextEditCollectionAssertions(textEdits);
     }
 }

src/Analysis/Engine/Test/FluentAssertions/AssertionsUtilities.cs

@@ -186,6 +186,10 @@ public static string GetName(object value) {
                     return scope.Name;
                 case IAnalysisValue analysisValue:
                     return $"value {analysisValue.Name}";
+                case Range range:
+                    return $"({range.start.line}, {range.start.character}) - ({range.end.line}, {range.end.character})";
+                case Position position:
+                    return $"({position.line}, {position.character})";
                 case string str:
                     return str;
                 default:
@@ -205,6 +209,9 @@ public static IEnumerable<IAnalysisValue> FlattenAnalysisValues(IEnumerable<IAna
             }
         }
 
+        public static bool RangeEquals(Range r1, Range r2) => PositionEquals(r1.start, r2.start) && PositionEquals(r1.end, r2.end);
+        public static bool PositionEquals(Position p1, Position p2) => p1.line == p2.line && p1.character == p2.character;
+
         public static string DoubleEscape(string input) 
             => input.Replace("\r", "\\\u200Br").Replace("\n", "\\\u200Bn").Replace("\t", @"\t");
     }

src/Analysis/Engine/Test/FluentAssertions/ReferenceCollectionAssertions.cs

@@ -75,10 +75,10 @@ public AndConstraint<ReferenceCollectionAssertions> OnlyHaveReferences(IEnumerab
             if (excess.Length > 0) {
                 var excessString = string.Join(", ", excess.Select(Format));
                 var errorMessage = expected.Length > 1
-                    ? $"Expected {GetName()} to have only {expected.Length} references{{reason}}, but it also has references: {excessString}."
+                    ? $"Expected {GetSubjectName()} to have only {expected.Length} references{{reason}}, but it also has references: {excessString}."
                     : expected.Length > 0
-                        ? $"Expected {GetName()} to have only one reference{{reason}}, but it also has references: {excessString}."
-                        : $"Expected {GetName()} to have no references{{reason}}, but it has references: {excessString}.";
+                        ? $"Expected {GetSubjectName()} to have only one reference{{reason}}, but it also has references: {excessString}."
+                        : $"Expected {GetSubjectName()} to have no references{{reason}}, but it has references: {excessString}.";
 
                 Execute.Assertion.BecauseOf(because, reasonArgs).FailWith(errorMessage);
             }
@@ -87,10 +87,7 @@ public AndConstraint<ReferenceCollectionAssertions> OnlyHaveReferences(IEnumerab
         }
 
         private static string Format((Uri uri, (int, int, int, int) range, ReferenceKind? kind) reference) 
-            => $"({TestData.GetTestRelativePath(reference.uri)}, {Format(reference.range)}, {reference.kind})";
-
-        private static string Format((int startLine, int startCharacter, int endLine, int endCharacter) range) 
-            => $"({range.startLine}, {range.startCharacter}) - ({range.endLine}, {range.endCharacter})";
+            => $"({TestData.GetTestRelativePath(reference.uri)}, {GetName(reference.range)}, {reference.kind})";
 
         [CustomAssertion]
         public AndConstraint<ReferenceCollectionAssertions> HaveReferenceAt(Uri documentUri, int startLine, int startCharacter, int endLine, int endCharacter, ReferenceKind? referenceKind = null, string because = "", params object[] reasonArgs) {
@@ -117,38 +114,29 @@ public AndConstraint<ReferenceCollectionAssertions> HaveReferenceAt(Uri document
         private string FindReference(Uri documentUri, string moduleName, Range range, ReferenceKind? referenceKind = null) {
             var candidates = Subject.Where(av => Equals(av.uri, documentUri)).ToArray();
             if (candidates.Length == 0) {
-                return $"Expected {GetName()} to have reference in the module '{moduleName}'{{reason}}, but no references has been found.";
+                return $"Expected {GetSubjectName()} to have reference in the module '{moduleName}'{{reason}}, but no references has been found.";
             }
 
             foreach (var candidate in candidates.Where(c => RangeEquals(c.range, range))) {
                 return referenceKind.HasValue && candidate._kind != referenceKind
-                    ? $"Expected {GetName()} to have reference of type '{referenceKind}'{{reason}}, but reference in module '{moduleName}' at {RangeToString(range)} has type '{candidate._kind}'"
+                    ? $"Expected {GetSubjectName()} to have reference of type '{referenceKind}'{{reason}}, but reference in module '{moduleName}' at {GetName(range)} has type '{candidate._kind}'"
                     : string.Empty;
             }
 
-            var errorMessage = $"Expected {GetName()} to have reference at {RangeToString(range)}{{reason}}, but module '{moduleName}' has no references at that range.";
+            var errorMessage = $"Expected {GetSubjectName()} to have reference at {GetName(range)}{{reason}}, but module '{moduleName}' has no references at that range.";
             if (!referenceKind.HasValue) {
                 return errorMessage;
             }
 
             var matchingTypes = candidates.Where(av => av._kind == referenceKind).ToArray();
             var matchingTypesString = matchingTypes.Length > 0
-                ? $"References that match type '{referenceKind}' have spans {string.Join(" ,", matchingTypes.Select(av => RangeToString(av.range)))}"
+                ? $"References that match type '{referenceKind}' have spans {string.Join(" ,", matchingTypes.Select(av => GetName(av.range)))}"
                 : $"There are no references with type '{referenceKind}' either";
 
             return $"{errorMessage} {matchingTypesString}";
         }
-
-        private static string RangeToString(Range range)
-            => $"({range.start.line}, {range.start.character}) - ({range.end.line}, {range.end.character})";
-
-        private static bool RangeEquals(Range r1, Range r2)
-            => r1.start.line == r2.start.line
-               && r1.start.character == r2.start.character
-               && r1.end.line == r2.end.line
-               && r1.end.character == r2.end.character;
-
+        
         [CustomAssertion]
-        private static string GetName() => CallerIdentifier.DetermineCallerIdentity() ?? "collection";
+        private static string GetSubjectName() => CallerIdentifier.DetermineCallerIdentity() ?? "collection";
     }
 }

src/Analysis/Engine/Test/FluentAssertions/TextEditCollectionAssertions.cs

@@ -0,0 +1,109 @@
+// Python Tools for Visual Studio
+// Copyright(c) Microsoft Corporation
+// All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the License); you may not use
+// this file except in compliance with the License. You may obtain a copy of the
+// License at http://www.apache.org/licenses/LICENSE-2.0
+//
+// THIS CODE IS PROVIDED ON AN  *AS IS* BASIS, WITHOUT WARRANTIES OR CONDITIONS
+// OF ANY KIND, EITHER EXPRESS OR IMPLIED, INCLUDING WITHOUT LIMITATION ANY
+// IMPLIED WARRANTIES OR CONDITIONS OF TITLE, FITNESS FOR A PARTICULAR PURPOSE,
+// MERCHANTABLITY OR NON-INFRINGEMENT.
+//
+// See the Apache Version 2.0 License for specific language governing
+// permissions and limitations under the License.
+
+using System;
+using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
+using System.Linq;
+using FluentAssertions;
+using FluentAssertions.Collections;
+using FluentAssertions.Execution;
+using Microsoft.Python.LanguageServer;
+using static Microsoft.PythonTools.Analysis.FluentAssertions.AssertionsUtilities;
+
+namespace Microsoft.PythonTools.Analysis.FluentAssertions {
+    [ExcludeFromCodeCoverage]
+    internal sealed class TextEditCollectionAssertions : SelfReferencingCollectionAssertions<TextEdit, TextEditCollectionAssertions> {
+        public TextEditCollectionAssertions(IEnumerable<TextEdit> references) : base(references) {}
+
+        protected override string Identifier => nameof(TextEdit) + "Collection";
+
+        
+        [CustomAssertion]
+        public AndConstraint<TextEditCollectionAssertions> OnlyHaveTextEdit(string expectedText, (int startLine, int startCharacter, int endLine, int endCharacter) expectedRange, string because = "", params object[] reasonArgs)
+            => OnlyHaveTextEdits(new []{(expectedText, expectedRange)}, because, reasonArgs);
+
+        [CustomAssertion]
+        public AndConstraint<TextEditCollectionAssertions> OnlyHaveTextEdits(params (string expectedText, (int startLine, int startCharacter, int endLine, int endCharacter) expectedRange)[] textEdits)
+            => OnlyHaveTextEdits(textEdits, string.Empty);
+
+        [CustomAssertion]
+        public AndConstraint<TextEditCollectionAssertions> OnlyHaveTextEdits(IEnumerable<(string expectedText, (int startLine, int startCharacter, int endLine, int endCharacter) expectedRange)> textEdits, string because = "", params object[] reasonArgs) {
+            var expected = textEdits.ToArray();
+            foreach (var (expectedText, (startLine, startCharacter, endLine, endCharacter)) in expected) {
+                HaveTextEditAt(expectedText, (startLine, startCharacter, endLine, endCharacter), because, reasonArgs);
+            }
+
+            var excess = Subject.Select(r => (r.newText, (r.range.start.line, r.range.start.character, r.range.end.line, r.range.end.character)))
+                .Except(expected)
+                .ToArray();
+
+            if (excess.Length > 0) {
+                var excessString = string.Join(", ", excess.Select(((string text, (int, int, int, int) range) te) => $"({te.text}, {GetName(te.range)})"));
+                var errorMessage = expected.Length > 1
+                    ? $"Expected {GetSubjectName()} to have only {expected.Length} textEdits{{reason}}, but it also has textEdits: {excessString}."
+                    : expected.Length > 0
+                        ? $"Expected {GetSubjectName()} to have only one reference{{reason}}, but it also has textEdits: {excessString}."
+                        : $"Expected {GetSubjectName()} to have no textEdits{{reason}}, but it has textEdits: {excessString}.";
+
+                Execute.Assertion.BecauseOf(because, reasonArgs).FailWith(errorMessage);
+            }
+
+            return new AndConstraint<TextEditCollectionAssertions>(this);
+        }
+        
+        [CustomAssertion]
+        public AndConstraint<TextEditCollectionAssertions> HaveTextEditAt(string expectedText, (int startLine, int startCharacter, int endLine, int endCharacter) expectedRange, string because = "", params object[] reasonArgs) {
+            var range = new Range {
+                start = new Position { line = expectedRange.startLine, character = expectedRange.startCharacter },
+                end = new Position { line = expectedRange.endLine, character = expectedRange.endCharacter }
+            };
+            
+            var errorMessage = GetHaveTextEditErrorMessage(expectedText, range);
+            Execute.Assertion.ForCondition(errorMessage == string.Empty)
+                .BecauseOf(because, reasonArgs)
+                .FailWith(errorMessage);
+
+            return new AndConstraint<TextEditCollectionAssertions>(this);
+        }
+
+        [CustomAssertion]
+        private string GetHaveTextEditErrorMessage(string expectedText, Range expectedRange) {
+            var candidates = Subject.Where(av => string.Equals(av.newText, expectedText, StringComparison.Ordinal)).ToArray();
+            if (candidates.Length == 0) {
+                return $"Expected {GetSubjectName()} to have text edit with newText '{expectedText}'{{reason}}, but "
+                    + (Subject.Any() ? "it is empty" : $"it has {GetQuotedNames(Subject.Select(te => te.newText))}");
+            }
+
+            var candidatesWithRange = candidates.Where(c => RangeEquals(c.range, expectedRange)).ToArray();
+            if (candidatesWithRange.Length > 1) {
+                return $"Expected {GetSubjectName()} to have only one text edit with newText '{expectedText}' and range {GetName(expectedRange)}{{reason}}, but there are {candidatesWithRange.Length}";
+            }
+
+            if (candidatesWithRange.Length == 0) {
+                return $"Expected {GetSubjectName()} to have text edit with newText '{expectedText}' in range {GetName(expectedRange)} {{reason}}, but "
+                    + (candidatesWithRange.Length == 1 
+                        ? $"it has range {GetName(candidatesWithRange[0].range)}" 
+                        : $"they are in ranges {string.Join(", ", candidatesWithRange.Select(te => GetName(te.range)))}");
+            }
+
+            return string.Empty;
+        }
+
+        [CustomAssertion]
+        private static string GetSubjectName() => CallerIdentifier.DetermineCallerIdentity() ?? "collection";
+    }
+}

src/Analysis/Engine/Test/LanguageServerTests.cs

@@ -1010,31 +1010,13 @@ public async Task OnTypeFormatting() {
                 // Extended tests for line formatting are in LineFormatterTests.
                 // These just verify that the language server formats and returns something correct.
                 var edits = await s.SendDocumentOnTypeFormatting(uri, new SourceLocation(2, 1), "\n");
-                edits.Should().OnlyContain(new TextEdit {
-                    newText = "def foo():",
-                    range = new Range {
-                        start = new SourceLocation(1, 1),
-                        end = new SourceLocation(1, 15)
-                    }
-                });
+                edits.Should().OnlyHaveTextEdit("def foo():", (0, 0, 0, 14));
 
                 edits = await s.SendDocumentOnTypeFormatting(uri, new SourceLocation(3, 1), "\n");
-                edits.Should().OnlyContain(new TextEdit {
-                    newText = "x = a + b",
-                    range = new Range {
-                        start = new SourceLocation(2, 5),
-                        end = new SourceLocation(2, 14)
-                    }
-                });
+                edits.Should().OnlyHaveTextEdit("x = a + b", (1, 4, 1, 13));
 
                 edits = await s.SendDocumentOnTypeFormatting(uri, new SourceLocation(4, 1), "\n");
-                edits.Should().OnlyContain(new TextEdit {
-                    newText = "x += 1",
-                    range = new Range {
-                        start = new SourceLocation(3, 5),
-                        end = new SourceLocation(3, 10)
-                    }
-                });
+                edits.Should().OnlyHaveTextEdit("x += 1", (2, 4, 2, 9));
             }
         }