Merge pull request #6609 from KevinH-MS/stabilization

[AskMode] Fix issue with #load of files that can't be read as text...

(fixes #6589)

Author: KevinH-MS

src/Compilers/CSharp/Portable/Compilation/SyntaxAndDeclarationManager.cs

@@ -383,8 +383,8 @@ private static void GetRemoveSet(
                         Debug.Assert(!loadDirectives.IsEmpty);
                         foreach (var directive in loadDirectives)
                         {
-                            var loadedTree = loadedSyntaxTreeMap[directive.ResolvedPath];
-                            if (removeSet.Contains(loadedTree))
+                            SyntaxTree loadedTree;
+                            if (TryGetLoadedSyntaxTree(loadedSyntaxTreeMap, directive, out loadedTree))
                             {
                                 removeSet.Remove(loadedTree);
                             }
@@ -404,15 +404,18 @@ private static void GetRemoveSetForLoadedTrees(
             {
                 if (directive.ResolvedPath != null)
                 {
-                    var loadedTree = loadedSyntaxTreeMap[directive.ResolvedPath];
-                    ImmutableArray<LoadDirective> nestedLoadDirectives;
-                    if (loadDirectiveMap.TryGetValue(loadedTree, out nestedLoadDirectives))
+                    SyntaxTree loadedTree;
+                    if (TryGetLoadedSyntaxTree(loadedSyntaxTreeMap, directive, out loadedTree))
                     {
-                        Debug.Assert(!nestedLoadDirectives.IsEmpty);
-                        GetRemoveSetForLoadedTrees(nestedLoadDirectives, loadDirectiveMap, loadedSyntaxTreeMap, removeSet);
-                    }
+                        ImmutableArray<LoadDirective> nestedLoadDirectives;
+                        if (loadDirectiveMap.TryGetValue(loadedTree, out nestedLoadDirectives))
+                        {
+                            Debug.Assert(!nestedLoadDirectives.IsEmpty);
+                            GetRemoveSetForLoadedTrees(nestedLoadDirectives, loadDirectiveMap, loadedSyntaxTreeMap, removeSet);
+                        }
 
-                    removeSet.Add(loadedTree);
+                        removeSet.Add(loadedTree);
+                    }
                 }
             }
         }
@@ -586,20 +589,21 @@ private static void UpdateSyntaxTreesAndOrdinalMapOnly(
                     {
                         var resolvedPath = directive.ResolvedPath;
                         Debug.Assert((resolvedPath != null) || !directive.Diagnostics.IsEmpty);
-                        if (resolvedPath != null)
+                        if (resolvedPath == null)
+                        {
+                            continue;
+                        }
+
+                        SyntaxTree loadedTree;
+                        if (TryGetLoadedSyntaxTree(loadedSyntaxTreeMap, directive, out loadedTree))
                         {
-                            var loadedTree = loadedSyntaxTreeMap[directive.ResolvedPath];
                             UpdateSyntaxTreesAndOrdinalMapOnly(
                                 treesBuilder,
                                 loadedTree,
                                 ordinalMapBuilder,
                                 loadDirectiveMap,
                                 loadedSyntaxTreeMap);
                         }
-
-                        treesBuilder.Add(tree);
-
-                        ordinalMapBuilder.Add(tree, ordinalMapBuilder.Count);
                     }
                 }
             }
@@ -620,5 +624,18 @@ internal bool MayHaveReferenceDirectives()
 
             return state.DeclarationTable.ReferenceDirectives.Any();
         }
+
+        private static bool TryGetLoadedSyntaxTree(ImmutableDictionary<string, SyntaxTree> loadedSyntaxTreeMap, LoadDirective directive, out SyntaxTree loadedTree)
+        {
+            if (loadedSyntaxTreeMap.TryGetValue(directive.ResolvedPath, out loadedTree))
+            {
+                return true;
+            }
+
+            // If we don't have a tree for this directive, there should be errors.
+            Debug.Assert(directive.Diagnostics.HasAnyErrors());
+
+            return false;
+        }
     }
 }

src/Compilers/CSharp/Test/Symbol/Compilation/LoadDirectiveTests.cs

@@ -1,5 +1,6 @@
 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
+using System.Linq;
 using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
 using Roslyn.Test.Utilities;
 using Roslyn.Utilities;
@@ -60,6 +61,60 @@ public void FileWithErrors()
                 Diagnostic(ErrorCode.ERR_NameNotInContext, "asdf").WithArguments("asdf").WithLocation(3, 21));
         }
 
+        [Fact]
+        public void FileThatCannotBeDecoded()
+        {
+            var code = "#load \"b.csx\"";
+            var resolver = TestSourceReferenceResolver.Create(
+                KeyValuePair.Create<string, object>("a.csx", new byte[] { 0xd8, 0x00, 0x00 }),
+                KeyValuePair.Create<string, object>("b.csx", "#load \"a.csx\""));
+            var options = TestOptions.DebugDll.WithSourceReferenceResolver(resolver);
+            var compilation = CreateCompilationWithMscorlib45(code, sourceFileName: "external1.csx", options: options, parseOptions: TestOptions.Script);
+            var external1 = compilation.SyntaxTrees.Last();
+            var external2 = Parse(code, filename: "external2.csx", options: TestOptions.Script);
+            compilation = compilation.AddSyntaxTrees(external2);
+
+            Assert.Equal(3, compilation.SyntaxTrees.Length);
+            compilation.GetParseDiagnostics().Verify(
+                // (1,7): error CS2015: 'a.csx' is a binary file instead of a text file
+                // #load "a.csx"
+                Diagnostic(ErrorCode.ERR_BinaryFile, @"""a.csx""").WithArguments("a.csx").WithLocation(1, 7));
+
+            var external3 = Parse(@"
+                #load ""b.csx""
+                #load ""a.csx""", filename: "external3.csx", options: TestOptions.Script);
+            compilation = compilation.ReplaceSyntaxTree(external1, external3);
+
+            Assert.Equal(3, compilation.SyntaxTrees.Length);
+            compilation.GetParseDiagnostics().Verify(
+                // external3.csx(3,23): error CS2015: 'a.csx' is a binary file instead of a text file
+                //                 #load "a.csx"
+                Diagnostic(ErrorCode.ERR_BinaryFile, @"""a.csx""").WithArguments("a.csx").WithLocation(3, 23),
+                // b.csx(1,7): error CS2015: 'a.csx' is a binary file instead of a text file
+                // #load "a.csx"
+                Diagnostic(ErrorCode.ERR_BinaryFile, @"""a.csx""").WithArguments("a.csx").WithLocation(1, 7));
+
+            var external4 = Parse("#load \"a.csx\"", filename: "external4.csx", options: TestOptions.Script);
+            compilation = compilation.ReplaceSyntaxTree(external3, external4);
+
+            Assert.Equal(3, compilation.SyntaxTrees.Length);
+            compilation.GetParseDiagnostics().Verify(
+                // external4.csx(1,7): error CS2015: 'a.csx' is a binary file instead of a text file
+                // #load "a.csx"
+                Diagnostic(ErrorCode.ERR_BinaryFile, @"""a.csx""").WithArguments("a.csx").WithLocation(1, 7),
+                // b.csx(1,7): error CS2015: 'a.csx' is a binary file instead of a text file
+                // #load "a.csx"
+                Diagnostic(ErrorCode.ERR_BinaryFile, @"""a.csx""").WithArguments("a.csx").WithLocation(1, 7));
+
+            compilation = compilation.RemoveSyntaxTrees(external2);
+
+            Assert.Equal(external4, compilation.SyntaxTrees.Single());
+            compilation.GetParseDiagnostics().Verify(
+                // external4.csx(1,7): error CS2015: 'a.csx' is a binary file instead of a text file
+                // #load "a.csx"
+                Diagnostic(ErrorCode.ERR_BinaryFile, @"""a.csx""").WithArguments("a.csx").WithLocation(1, 7));
+        }
+
         [Fact]
         public void NoSourceReferenceResolver()
         {

src/Test/Utilities/Shared/Mocks/TestSourceReferenceResolver.cs

@@ -15,17 +15,22 @@ public sealed class TestSourceReferenceResolver : SourceReferenceResolver
 
         public static SourceReferenceResolver Create(params KeyValuePair<string, string>[] sources)
         {
-            return TestSourceReferenceResolver.Create(sources.ToDictionary(p => p.Key, p => p.Value));
+            return new TestSourceReferenceResolver(sources.ToDictionary(p => p.Key, p => (object)p.Value));
+        }
+
+        public static SourceReferenceResolver Create(params KeyValuePair<string, object>[] sources)
+        {
+            return new TestSourceReferenceResolver(sources.ToDictionary(p => p.Key, p => p.Value));
         }
 
         public static SourceReferenceResolver Create(Dictionary<string, string> sources = null)
         {
-            return (sources == null || sources.Count == 0) ? Default : new TestSourceReferenceResolver(sources);
+            return new TestSourceReferenceResolver(sources.ToDictionary(p => p.Key, p => (object)p.Value));
         }
 
-        private readonly Dictionary<string, string> _sources;
+        private readonly Dictionary<string, object> _sources;
 
-        private TestSourceReferenceResolver(Dictionary<string, string> sources)
+        private TestSourceReferenceResolver(Dictionary<string, object> sources)
         {
             _sources = sources;
         }
@@ -39,7 +44,8 @@ public override Stream OpenRead(string resolvedPath)
         {
             if (_sources != null && resolvedPath != null)
             {
-                return new MemoryStream(Encoding.UTF8.GetBytes(_sources[resolvedPath]));
+                var data = _sources[resolvedPath];
+                return new MemoryStream((data is string) ? Encoding.UTF8.GetBytes((string)data) : (byte[])data);
             }
             else
             {