Improve line formatter behavior with unmatched close brackets, log warning (#393)

Fixes #388.

Instead of throwing a generic exception, keep track of the unmatched token for later use, and just ignore it. When the line formatting is done, DocumentOnTypeFormatting can check to see if an unmatched token occurred before or in the line being formatted and report to the user a warning about the bad behavior.

I also fixed the comment strings that claimed that the line formatter worked with one-indexed lines, which is no longer true after Alex's changes to this code.

Author: jakebailey

src/Analysis/Engine/Test/LineFormatterTests.cs

@@ -396,6 +396,14 @@ public void CommentAfterOperator() {
             AssertSingleLineFormat("a+# comment\nb", "a +  # comment");
         }
 
+        [DataRow("def foo()):\n    a+b", "a + b", 1, 4, ")", 0)]
+        [DataRow("x = [1, 2]\nx += [3]]\na+b", "a + b", 2, 0, "]", 1)]
+        [DataRow("x = { foo: bar } } }\na+b", "a + b", 1, 0, "}", 0)]
+        [DataTestMethod, Priority(0)]
+        public void UnmatchedBracket(string code, string expected, int line, int editStart, string unmatched, int unmatchedLine) {
+            AssertSingleLineFormat(code, expected, line: line, editStart: editStart, unmatched: (unmatched, unmatchedLine));
+        }
+
         [DataRow("'a''b'", "'a' 'b'")]
         [DataRow("'a' 'b'", "'a' 'b'")]
         [DataRow("'''a''''''b'''", "'''a''' '''b'''")]
@@ -452,13 +460,17 @@ public void GrammarFile() {
         /// <param name="line">The line number to request to be formatted.</param>
         /// <param name="languageVersion">Python language version to format.</param>
         /// <param name="editStart">Where the edit should begin (i.e. when whitespace or a multi-line string begins a line).</param>
-        public static void AssertSingleLineFormat(string text, string expected, int line = 0, PythonLanguageVersion languageVersion = PythonLanguageVersion.V37, int editStart = 0) {
+        /// <param name="unmatched">A nullable tuple to check against the line formatter's UnmatchedToken.</param>
+        public static void AssertSingleLineFormat(string text, string expected, int line = 0, PythonLanguageVersion languageVersion = PythonLanguageVersion.V37, int editStart = 0, (string, int)? unmatched = null) {
             Check.ArgumentNotNull(nameof(text), text);
             Check.ArgumentNotNull(nameof(expected), expected);
 
             using (var reader = new StringReader(text)) {
-                var edits = new LineFormatter(reader, languageVersion).FormatLine(line);
+                var lineFormatter = new LineFormatter(reader, languageVersion);
+                var edits = lineFormatter.FormatLine(line);
+
                 edits.Should().OnlyHaveTextEdit(expected, (line, editStart, line, text.Split('\n')[line].Length));
+                lineFormatter.UnmatchedToken(line).Should().Be(unmatched);
             }
         }
 

src/LanguageServer/Impl/Implementation/LineFormatter.cs

@@ -19,7 +19,6 @@
 using System.IO;
 using System.Linq;
 using System.Text;
-using Microsoft.PythonTools;
 using Microsoft.PythonTools.Analysis;
 using Microsoft.PythonTools.Analysis.Infrastructure;
 using Microsoft.PythonTools.Parsing;
@@ -73,7 +72,7 @@ private void AddToken(TokenExt token) {
         /// next line to the first non-ignored token so that the formatter
         /// can look ahead.
         /// </summary>
-        /// <param name="line">One-indexed line number.</param>
+        /// <param name="line">Zero-indexed line number.</param>
         /// <param name="includeToken">A function which returns true if the token should be added to the final list. If null, all tokens will be added.</param>
         /// <returns>A non-null list of tokens on that line.</returns>
         private List<TokenExt> TokenizeLine(int line, Func<TokenExt, bool> includeToken = null) {
@@ -93,10 +92,25 @@ private List<TokenExt> TokenizeLine(int line, Func<TokenExt, bool> includeToken
             return _lineTokens.TryGetValue(line, out var tokens) ? tokens : new List<TokenExt>();
         }
 
+        /// <summary>
+        /// Returns a human-readable message describing the first unmatched
+        /// bracket found by the tokenizer at the time this function is called.
+        /// </summary>
+        /// <param name="line">Zero-indexed line number.</param>
+        /// <returns>A string representation of the unmatched token and its zero-indexed line number, or null if none exists or hasn't been scanned yet.</returns>
+        public (string token, int line)? UnmatchedToken(int line) {
+            var unmatched = _tokenizer.Unmatched;
+            if (unmatched == null || unmatched.Line > line) {
+                return null;
+            }
+
+            return (unmatched.ToString(), unmatched.Line);
+        }
+
         /// <summary>
         /// Formats a single line and returns TextEdits to replace the old text.
         /// </summary>
-        /// <param name="line">One-indexed line number.</param>
+        /// <param name="line">Zero-indexed line number.</param>
         /// <returns>A list of TextEdits needed to format the line.</returns>
         public TextEdit[] FormatLine(int line) {
             if (line < 0) {
@@ -122,7 +136,7 @@ public TextEdit[] FormatLine(int line) {
                 startIdx = 1;
                 builder.Append(' ');
             }
-            
+
             for (var i = startIdx; i < tokens.Count; i++) {
                 var token = tokens[i];
                 var prev = tokens.ElementAtOrDefault(i - 1);
@@ -239,7 +253,7 @@ public TextEdit[] FormatLine(int line) {
                             // Check unpacking case
                         } else if (token.Kind == TokenKind.Multiply && (actualPrev == null || actualPrev.Kind != TokenKind.Name && actualPrev.Kind != TokenKind.Constant && !actualPrev.IsClose)) {
                             builder.Append(token);
-                        } else { 
+                        } else {
                             AppendTokenEnsureWhiteSpacesAround(builder, token);
                         }
                         break;
@@ -360,16 +374,16 @@ public TextEdit[] FormatLine(int line) {
 
             var edit = new TextEdit {
                 range = new Range {
-                    start = new Position{ line = line, character = startIndex - lineStartIndex },
-                    end = new Position{ line = line, character = endIndex - lineStartIndex }
+                    start = new Position { line = line, character = startIndex - lineStartIndex },
+                    end = new Position { line = line, character = endIndex - lineStartIndex }
                 },
                 newText = newText
             };
 
             return new[] { edit };
         }
 
-        private static void AppendTokenEnsureWhiteSpacesAround(StringBuilder builder, TokenExt token) 
+        private static void AppendTokenEnsureWhiteSpacesAround(StringBuilder builder, TokenExt token)
             => builder.EnsureEndsWithWhiteSpace()
             .Append(token)
             .EnsureEndsWithWhiteSpace();
@@ -430,7 +444,7 @@ public bool MatchesClose(TokenExt other) {
             public bool IsKeyword => (Kind >= TokenKind.FirstKeyword && Kind <= TokenKind.LastKeyword) || Kind == TokenKind.KeywordAsync || Kind == TokenKind.KeywordAwait;
 
             public bool IsString => Kind == TokenKind.Constant && Token != Tokens.NoneToken && (Token.Value is string || Token.Value is AsciiString);
-            
+
             public bool IsSimpleSliceToLeft {
                 get {
                     if (Kind != TokenKind.Colon) {
@@ -535,6 +549,7 @@ private class TokenizerWrapper {
             private readonly Tokenizer _tokenizer;
             private readonly Stack<TokenExt> _insides = new Stack<TokenExt>();
             public TokenExt Current { get; private set; }
+            public TokenExt Unmatched { get; private set; }
 
             public TokenizerWrapper(Tokenizer tokenizer) {
                 _tokenizer = tokenizer;
@@ -570,10 +585,17 @@ public TokenExt Next() {
                 );
 
                 if (tokenExt.IsClose) {
-                    if (_insides.Count == 0 || !_insides.Peek().MatchesClose(tokenExt)) {
-                        throw new Exception($"Close bracket ({token.Kind}) has no matching open");
+                    if (_insides.Count > 0 && _insides.Peek().MatchesClose(tokenExt)) {
+                        _insides.Pop();
+                    } else {
+                        // This close doesn't match, so assume that the token is just a stray
+                        // and do nothing to the _insides stack.
+
+                        // Keep track of the first unmatched token to present back to the user.
+                        if (Unmatched == null) {
+                            Unmatched = tokenExt;
+                        }
                     }
-                    _insides.Pop();
                 } else if (tokenExt.Kind == TokenKind.Colon && _insides.Count != 0 && _insides.Peek().Kind == TokenKind.KeywordLambda) {
                     _insides.Pop();
                 }

src/LanguageServer/Impl/Implementation/Server.OnTypeFormatting.cs

@@ -18,6 +18,7 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.PythonTools.Analysis;
+using Microsoft.PythonTools.Analysis.Infrastructure;
 
 namespace Microsoft.Python.LanguageServer.Implementation {
     public sealed partial class Server {
@@ -49,7 +50,15 @@ public override async Task<TextEdit[]> DocumentOnTypeFormatting(DocumentOnTypeFo
                 }
 
                 var lineFormatter = new LineFormatter(reader, Analyzer.LanguageVersion);
-                return lineFormatter.FormatLine(targetLine);
+                var edits = lineFormatter.FormatLine(targetLine);
+                var unmatchedToken = lineFormatter.UnmatchedToken(targetLine);
+
+                if (unmatchedToken != null) {
+                    var message = Resources.LineFormatter_UnmatchedToken.FormatInvariant(unmatchedToken.Value.token, unmatchedToken.Value.line + 1);
+                    LogMessage(MessageType.Warning, message);
+                }
+
+                return edits;
             }
         }
     }

src/LanguageServer/Impl/Resources.Designer.cs

@@ -132,6 +132,9 @@
   <data name="LanguageServerVersion" xml:space="preserve">
     <value>Microsoft Python Language Server version {0}</value>
   </data>
+  <data name="LineFormatter_UnmatchedToken" xml:space="preserve">
+    <value>Unmatched token '{0}' on line {1}; line formatting may not be accurate.</value>
+  </data>
   <data name="ReloadingModules" xml:space="preserve">
     <value>Reloading modules... </value>
   </data>