Fix #885 (#886)

* [WIP] Started updating VS LSP client.
* A few more nullability fixes.
* Use explicit 0:0 range instead of relying on constructor.
* Propagate nullability metadata to F# callers.
* Avoid calling unsafe constructors.
* Avoid deprecated Assert.Equals.
* Set DocumentRangeFormattingProvider to false.
* Fix to AssertCapability.
* Assertions should work more reliabily when capabilities are null.
* Disable nullable reference checking when copying unsound types.
* Check if Range is null, since nullability metadata can be violated.

Co-authored-by: Ricardo Espinoza <[email protected]>

Author: Chris Granade

src/QsCompiler/CommandLineTool/Logging.cs

@@ -41,12 +41,14 @@ public ConsoleLogger(
         /// Prints the given message to the Console.
         /// Errors and Warnings are printed to the error stream.
         /// </summary>
-        private static void PrintToConsole(DiagnosticSeverity severity, string message)
+        private static void PrintToConsole(DiagnosticSeverity? severity, string message)
         {
-            var (stream, color) =
-                severity == DiagnosticSeverity.Error ? (Console.Error, ConsoleColor.Red) :
-                severity == DiagnosticSeverity.Warning ? (Console.Error, ConsoleColor.Yellow) :
-                (Console.Out, Console.ForegroundColor);
+            var (stream, color) = severity switch
+            {
+                DiagnosticSeverity.Error => (Console.Error, ConsoleColor.Red),
+                DiagnosticSeverity.Warning => (Console.Error, ConsoleColor.Yellow),
+                _ => (Console.Out, Console.ForegroundColor)
+            };
 
             var consoleColor = Console.ForegroundColor;
             Console.ForegroundColor = color;

src/QsCompiler/CompilationManager/CompilationManager.csproj

@@ -10,7 +10,7 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="Microsoft.VisualStudio.LanguageServer.Protocol" Version="16.3.57" />
+    <PackageReference Include="Microsoft.VisualStudio.LanguageServer.Protocol" Version="16.9.1180" />
     <PackageReference Include="Newtonsoft.Json.Bson" Version="1.0.2" />
   </ItemGroup>
 

src/QsCompiler/CompilationManager/DiagnosticTools.cs

@@ -37,28 +37,35 @@ public static Position SymbolPosition(QsLocation rootLocation, QsNullable<Positi
         [return: NotNullIfNotNull("message")]
         public static Diagnostic? Copy(this Diagnostic message)
         {
-            Lsp.Position? CopyPosition(Lsp.Position? position) =>
-                position is null ? null : new Lsp.Position(position.Line, position.Character);
-
-            Lsp.Range? CopyRange(Lsp.Range? range) =>
-                range is null
-                    ? null
-                    : new Lsp.Range
-                    {
-                        Start = CopyPosition(range.Start),
-                        End = CopyPosition(range.End)
-                    };
+            Lsp.Position CopyPosition(Lsp.Position position) =>
+                new Lsp.Position(position.Line, position.Character);
 
+            Lsp.Range CopyRange(Lsp.Range range) =>
+                new Lsp.Range
+                {
+                    Start = CopyPosition(range.Start),
+                    End = CopyPosition(range.End)
+                };
+
+            // NB: The nullability metadata on Diagnostic.Range is incorrect,
+            //     such that some Diagnostic values may have nullable ranges.
+            //     We cannot assign that to a new Diagnostic without
+            //     contradicting nullability metadata, however, so we need to
+            //     explicitly disable nullable references for the following
+            //     statement. Once the upstream bug in the LSP client package
+            //     is fixed, we can remove the nullable disable here.
+            #nullable disable
             return message is null
                 ? null
                 : new Diagnostic
                 {
-                    Range = CopyRange(message.Range),
+                    Range = message.Range == null ? null : CopyRange(message.Range),
                     Severity = message.Severity,
                     Code = message.Code,
                     Source = message.Source,
                     Message = message.Message
                 };
+            #nullable restore
         }
 
         /// <summary>
@@ -70,12 +77,17 @@ range is null
         public static Diagnostic WithLineNumOffset(this Diagnostic diagnostic, int offset)
         {
             var copy = diagnostic.Copy();
-            copy.Range.Start.Line += offset;
-            copy.Range.End.Line += offset;
-            if (copy.Range.Start.Line < 0 || copy.Range.End.Line < 0)
+            // NB: Despite the nullability metadata, Range may be null here.
+            //     We thus need to guard accordingly.
+            if (copy.Range != null)
             {
-                throw new ArgumentOutOfRangeException(
-                    nameof(offset), "Translated diagnostic has negative line numbers.");
+                copy.Range.Start.Line += offset;
+                copy.Range.End.Line += offset;
+                if (copy.Range.Start.Line < 0 || copy.Range.End.Line < 0)
+                {
+                    throw new ArgumentOutOfRangeException(
+                        nameof(offset), "Translated diagnostic has negative line numbers.");
+                }
             }
             return copy;
         }

src/QsCompiler/CompilationManager/Diagnostics.cs

@@ -121,7 +121,7 @@ public static Diagnostic LoadWarning(WarningCode code, IEnumerable<string> args,
                 Code = Code(code),
                 Source = source,
                 Message = DiagnosticItem.Message(code, args ?? Enumerable.Empty<string>()),
-                Range = null
+                Range = new Lsp.Range { Start = new Lsp.Position(0, 0), End = new Lsp.Position(0, 0) }
             };
 
         // warnings 20**
@@ -134,7 +134,7 @@ internal static Diagnostic EmptyStatementWarning(string filename, Position pos)
                 Code = WarningCode.ExcessSemicolon.Code(),
                 Source = filename,
                 Message = DiagnosticItem.Message(WarningCode.ExcessSemicolon, Enumerable.Empty<string>()),
-                Range = pos == null ? null : new Lsp.Range { Start = pos.ToLsp(), End = pos.ToLsp() }
+                Range = new Lsp.Range { Start = pos.ToLsp(), End = pos.ToLsp() }
             };
         }
     }
@@ -156,82 +156,112 @@ public static Diagnostic LoadError(ErrorCode code, IEnumerable<string> args, str
                 Code = Code(code),
                 Source = source,
                 Message = DiagnosticItem.Message(code, args ?? Enumerable.Empty<string>()),
-                Range = null
+                Range = new Lsp.Range { Start = new Lsp.Position(0, 0), End = new Lsp.Position(0, 0) }
             };
 
         // errors 20**
 
         internal static Diagnostic InvalidFragmentEnding(string filename, ErrorCode code, Position pos)
         {
+            if (pos == null)
+            {
+                throw new ArgumentNullException(nameof(pos));
+            }
+
             return new Diagnostic
             {
                 Severity = DiagnosticSeverity.Error,
                 Code = Code(code),
                 Source = filename,
                 Message = DiagnosticItem.Message(code, Enumerable.Empty<string>()),
-                Range = pos == null ? null : new Lsp.Range { Start = pos.ToLsp(), End = pos.ToLsp() }
+                Range = new Lsp.Range { Start = pos.ToLsp(), End = pos.ToLsp() }
             };
         }
 
         internal static Diagnostic MisplacedOpeningBracketError(string filename, Position pos)
         {
+            if (pos == null)
+            {
+                throw new ArgumentNullException(nameof(pos));
+            }
+
             return new Diagnostic
             {
                 Severity = DiagnosticSeverity.Error,
                 Code = ErrorCode.MisplacedOpeningBracket.Code(),
                 Source = filename,
                 Message = DiagnosticItem.Message(ErrorCode.MisplacedOpeningBracket, Enumerable.Empty<string>()),
-                Range = pos == null ? null : new Lsp.Range { Start = pos.ToLsp(), End = pos.ToLsp() }
+                Range = new Lsp.Range { Start = pos.ToLsp(), End = pos.ToLsp() }
             };
         }
 
         // errors 10**
 
         internal static Diagnostic ExcessBracketError(string filename, Position pos)
         {
+            if (pos == null)
+            {
+                throw new ArgumentNullException(nameof(pos));
+            }
+
             return new Diagnostic
             {
                 Severity = DiagnosticSeverity.Error,
                 Code = ErrorCode.ExcessBracketError.Code(),
                 Source = filename,
                 Message = DiagnosticItem.Message(ErrorCode.ExcessBracketError, Enumerable.Empty<string>()),
-                Range = pos == null ? null : new Lsp.Range { Start = pos.ToLsp(), End = pos.ToLsp() }
+                Range = new Lsp.Range { Start = pos.ToLsp(), End = pos.ToLsp() }
             };
         }
 
         internal static Diagnostic MissingClosingBracketError(string filename, Position pos)
         {
+            if (pos == null)
+            {
+                throw new ArgumentNullException(nameof(pos));
+            }
+
             return new Diagnostic
             {
                 Severity = DiagnosticSeverity.Error,
                 Code = ErrorCode.MissingBracketError.Code(),
                 Source = filename,
                 Message = DiagnosticItem.Message(ErrorCode.MissingBracketError, Enumerable.Empty<string>()),
-                Range = pos == null ? null : new Lsp.Range { Start = pos.ToLsp(), End = pos.ToLsp() }
+                Range = new Lsp.Range { Start = pos.ToLsp(), End = pos.ToLsp() }
             };
         }
 
         internal static Diagnostic MissingStringDelimiterError(string filename, Position pos)
         {
+            if (pos == null)
+            {
+                throw new ArgumentNullException(nameof(pos));
+            }
+
             return new Diagnostic
             {
                 Severity = DiagnosticSeverity.Error,
                 Code = ErrorCode.MissingStringDelimiterError.Code(),
                 Source = filename,
                 Message = DiagnosticItem.Message(ErrorCode.MissingStringDelimiterError, Enumerable.Empty<string>()),
-                Range = pos == null ? null : new Lsp.Range { Start = pos.ToLsp(), End = pos.ToLsp() }
+                Range = new Lsp.Range { Start = pos.ToLsp(), End = pos.ToLsp() }
             };
         }
 
         internal static Diagnostic InvalidCharacterInInterpolatedArgument(string filename, Position pos, char invalidCharacter)
         {
+            if (pos == null)
+            {
+                throw new ArgumentNullException(nameof(pos));
+            }
+
             return new Diagnostic
             {
                 Severity = DiagnosticSeverity.Error,
                 Code = ErrorCode.InvalidCharacterInInterpolatedArgument.Code(),
                 Source = filename,
                 Message = DiagnosticItem.Message(ErrorCode.InvalidCharacterInInterpolatedArgument, new[] { invalidCharacter.ToString() }),
-                Range = pos == null ? null : new Lsp.Range { Start = pos.ToLsp(), End = pos.ToLsp() }
+                Range = new Lsp.Range { Start = pos.ToLsp(), End = pos.ToLsp() }
             };
         }
     }

src/QsCompiler/CompilationManager/EditorSupport/CodeCompletion.cs

@@ -430,7 +430,7 @@ private static IEnumerable<CompletionItem> GetNamespaceAliasCompletions(
         /// completion item data is missing properties.
         /// </summary>
         private static string? TryGetDocumentation(
-            CompilationUnit compilation, CompletionItemData data, CompletionItemKind kind, bool useMarkdown)
+            CompilationUnit compilation, CompletionItemData data, CompletionItemKind? kind, bool useMarkdown)
         {
             if (data.QualifiedName == null
                 || data.SourceFile == null
@@ -619,7 +619,7 @@ private static CompletionList ToCompletionList(this IEnumerable<CompletionItem>
             new CompletionList
             {
                 IsIncomplete = isIncomplete,
-                Items = items?.ToArray()
+                Items = items?.ToArray() ?? new CompletionItem[] { }
             };
 
         /// <summary>

src/QsCompiler/CompilationManager/EditorSupport/EditorCommands.cs

@@ -203,7 +203,7 @@ DocumentHighlight AsHighlight(Lsp.Range range) =>
             Hover? GetHover(string? info) => info == null ? null : new Hover
             {
                 Contents = new MarkupContent { Kind = format, Value = info },
-                Range = new Lsp.Range { Start = position?.ToLsp(), End = position?.ToLsp() }
+                Range = new Lsp.Range { Start = position.ToLsp(), End = position.ToLsp() }
             };
 
             var markdown = format == MarkupKind.Markdown;
@@ -393,7 +393,7 @@ List<QsFunctor> FunctorApplications(ref QsExpression ex)
             MarkupContent AsMarkupContent(string str) => new MarkupContent { Kind = format, Value = str };
             ParameterInformation AsParameterInfo(string? paramName) => new ParameterInformation
             {
-                Label = paramName,
+                Label = paramName ?? "<unknown parameter>",
                 Documentation = AsMarkupContent(documentation.ParameterDescription(paramName))
             };
 

src/QsCompiler/CompilationManager/EditorSupport/SymbolInformation.cs

@@ -28,7 +28,7 @@ internal static class SymbolInfo
         internal static Location AsLocation(string source, Position offset, Range relRange) =>
             new Location
             {
-                Uri = CompilationUnitManager.TryGetUri(source, out var uri) ? uri : null,
+                Uri = CompilationUnitManager.TryGetUri(source, out var uri) ? uri : throw new Exception($"Source location {source} could not be converted to a valid URI."),
                 Range = (offset + relRange).ToLsp()
             };
 

src/QsCompiler/CompilationManager/ProjectManager.cs

@@ -929,10 +929,30 @@ public Task ManagerTaskAsync(Uri file, Action<CompilationUnitManager, bool> exec
 
             try
             {
+                // NB: As of version 16.9.1180 of the LSP client, document
+                //     changes are presented as the sum type
+                //     TextDocumentEdit[] | (TextDocumentEdit | CreateFile | RenameFile | DeleteFile)[].
+                //     Thus, to collect them with a SelectMany call, we need
+                //     to ensure that the first case (TextDocumentEdit[]) is
+                //     first wrapped in a cast to the sum type
+                //     TextDocumentEdit | CreateFile | RenameFile | DeleteFile.
+                //     Note that the SumType struct is defined in the LSP client,
+                //     and works by defining explicit cast operators for each case.
+                IEnumerable<SumType<TextDocumentEdit, CreateFile, RenameFile, DeleteFile>> CastToSumType(SumType<TextDocumentEdit[], SumType<TextDocumentEdit, CreateFile, RenameFile, DeleteFile>[]>? editCollection) =>
+                    editCollection switch
+                    {
+                        { } edits => edits.Match(
+                            simpleEdits => simpleEdits.Cast<SumType<TextDocumentEdit, CreateFile, RenameFile, DeleteFile>>(),
+                            complexEdits => complexEdits),
+                        null => ImmutableList<SumType<TextDocumentEdit, CreateFile, RenameFile, DeleteFile>>.Empty
+                    };
+
                 // if a file belongs to several compilation units, then this will fail
                 var changes = edits.SelectMany(edit => edit.Changes)
                     .ToDictionary(pair => pair.Key, pair => pair.Value);
-                var documentChanges = edits.SelectMany(edit => edit.DocumentChanges).ToArray();
+                var documentChanges = edits
+                    .SelectMany(edits => CastToSumType(edits.DocumentChanges).ToArray())
+                    .ToArray();
                 return new WorkspaceEdit { Changes = changes, DocumentChanges = documentChanges };
             }
             catch

src/QsCompiler/CompilationManager/Utils.cs

@@ -168,8 +168,10 @@ public static Position ToQSharp(this Lsp.Position position) =>
         /// <summary>
         /// Converts the Q# compiler position into a language server protocol position.
         /// </summary>
-        public static Lsp.Position ToLsp(this Position position) =>
-            new Lsp.Position(position.Line, position.Column);
+        public static Lsp.Position ToLsp(this Position? position) =>
+            position == null
+            ? new Lsp.Position()
+            : new Lsp.Position(position.Line, position.Column);
 
         /// <summary>
         /// Converts the language server protocol range into a Q# compiler range.

src/QsCompiler/Compiler/LoadedStep.cs

@@ -123,7 +123,9 @@ internal static Diagnostic ConvertDiagnostic(IRewriteStep.Diagnostic diagnostic,
                 Severity = severity,
                 Message = $"{stageAnnotation}{diagnostic.Message}",
                 Source = diagnostic.Source,
-                Range = diagnostic.Source is null ? null : diagnostic.Range?.ToLsp()
+                Range = diagnostic.Source is null || diagnostic.Range is null
+                        ? new VisualStudio.LanguageServer.Protocol.Range()
+                        : diagnostic.Range.ToLsp()
             };
         }