Skip to content

Commit 8be0de0

Browse files
KevinH-MSKevinH-MS
committed
Fix issue with #load of files that can't be read as text...
If a #load'ed file can be resolved, but cannot be read (or cannot be read as text), it doesn't get added to the map of resolved file paths -> SyntaxTrees (because there is no SyntaxTree to add). This wasn't handled in the remove and replace helpers that rebuild the various syntax maps. In addition, we were attempting to add #load'ed trees to the ordinal map twice in the case that their ordinal position needed to be shifted after a remove or replace.
1 parent 15b927e commit 8be0de0

File tree

3 files changed

+98
-20
lines changed

3 files changed

+98
-20
lines changed

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

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -383,8 +383,8 @@ private static void GetRemoveSet(
383383
Debug.Assert(!loadDirectives.IsEmpty);
384384
foreach (var directive in loadDirectives)
385385
{
386-
var loadedTree = loadedSyntaxTreeMap[directive.ResolvedPath];
387-
if (removeSet.Contains(loadedTree))
386+
SyntaxTree loadedTree;
387+
if (TryGetLoadedSyntaxTree(loadedSyntaxTreeMap, directive, out loadedTree))
388388
{
389389
removeSet.Remove(loadedTree);
390390
}
@@ -404,15 +404,18 @@ private static void GetRemoveSetForLoadedTrees(
404404
{
405405
if (directive.ResolvedPath != null)
406406
{
407-
var loadedTree = loadedSyntaxTreeMap[directive.ResolvedPath];
408-
ImmutableArray<LoadDirective> nestedLoadDirectives;
409-
if (loadDirectiveMap.TryGetValue(loadedTree, out nestedLoadDirectives))
407+
SyntaxTree loadedTree;
408+
if (TryGetLoadedSyntaxTree(loadedSyntaxTreeMap, directive, out loadedTree))
410409
{
411-
Debug.Assert(!nestedLoadDirectives.IsEmpty);
412-
GetRemoveSetForLoadedTrees(nestedLoadDirectives, loadDirectiveMap, loadedSyntaxTreeMap, removeSet);
413-
}
410+
ImmutableArray<LoadDirective> nestedLoadDirectives;
411+
if (loadDirectiveMap.TryGetValue(loadedTree, out nestedLoadDirectives))
412+
{
413+
Debug.Assert(!nestedLoadDirectives.IsEmpty);
414+
GetRemoveSetForLoadedTrees(nestedLoadDirectives, loadDirectiveMap, loadedSyntaxTreeMap, removeSet);
415+
}
414416

415-
removeSet.Add(loadedTree);
417+
removeSet.Add(loadedTree);
418+
}
416419
}
417420
}
418421
}
@@ -586,20 +589,21 @@ private static void UpdateSyntaxTreesAndOrdinalMapOnly(
586589
{
587590
var resolvedPath = directive.ResolvedPath;
588591
Debug.Assert((resolvedPath != null) || !directive.Diagnostics.IsEmpty);
589-
if (resolvedPath != null)
592+
if (resolvedPath == null)
593+
{
594+
continue;
595+
}
596+
597+
SyntaxTree loadedTree;
598+
if (TryGetLoadedSyntaxTree(loadedSyntaxTreeMap, directive, out loadedTree))
590599
{
591-
var loadedTree = loadedSyntaxTreeMap[directive.ResolvedPath];
592600
UpdateSyntaxTreesAndOrdinalMapOnly(
593601
treesBuilder,
594602
loadedTree,
595603
ordinalMapBuilder,
596604
loadDirectiveMap,
597605
loadedSyntaxTreeMap);
598606
}
599-
600-
treesBuilder.Add(tree);
601-
602-
ordinalMapBuilder.Add(tree, ordinalMapBuilder.Count);
603607
}
604608
}
605609
}
@@ -620,5 +624,18 @@ internal bool MayHaveReferenceDirectives()
620624

621625
return state.DeclarationTable.ReferenceDirectives.Any();
622626
}
627+
628+
private static bool TryGetLoadedSyntaxTree(ImmutableDictionary<string, SyntaxTree> loadedSyntaxTreeMap, LoadDirective directive, out SyntaxTree loadedTree)
629+
{
630+
if (loadedSyntaxTreeMap.TryGetValue(directive.ResolvedPath, out loadedTree))
631+
{
632+
return true;
633+
}
634+
635+
// If we don't have a tree for this directive, there should be errors.
636+
Debug.Assert(directive.Diagnostics.HasAnyErrors());
637+
638+
return false;
639+
}
623640
}
624641
}

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

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
22

3+
using System.Linq;
34
using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
45
using Roslyn.Test.Utilities;
56
using Roslyn.Utilities;
@@ -60,6 +61,60 @@ public void FileWithErrors()
6061
Diagnostic(ErrorCode.ERR_NameNotInContext, "asdf").WithArguments("asdf").WithLocation(3, 21));
6162
}
6263

64+
[Fact]
65+
public void FileThatCannotBeDecoded()
66+
{
67+
var code = "#load \"b.csx\"";
68+
var resolver = TestSourceReferenceResolver.Create(
69+
KeyValuePair.Create<string, object>("a.csx", new byte[] { 0xd8, 0x00, 0x00 }),
70+
KeyValuePair.Create<string, object>("b.csx", "#load \"a.csx\""));
71+
var options = TestOptions.DebugDll.WithSourceReferenceResolver(resolver);
72+
var compilation = CreateCompilationWithMscorlib45(code, sourceFileName: "external1.csx", options: options, parseOptions: TestOptions.Script);
73+
var external1 = compilation.SyntaxTrees.Last();
74+
var external2 = Parse(code, filename: "external2.csx", options: TestOptions.Script);
75+
compilation = compilation.AddSyntaxTrees(external2);
76+
77+
Assert.Equal(3, compilation.SyntaxTrees.Length);
78+
compilation.GetParseDiagnostics().Verify(
79+
// (1,7): error CS2015: 'a.csx' is a binary file instead of a text file
80+
// #load "a.csx"
81+
Diagnostic(ErrorCode.ERR_BinaryFile, @"""a.csx""").WithArguments("a.csx").WithLocation(1, 7));
82+
83+
var external3 = Parse(@"
84+
#load ""b.csx""
85+
#load ""a.csx""", filename: "external3.csx", options: TestOptions.Script);
86+
compilation = compilation.ReplaceSyntaxTree(external1, external3);
87+
88+
Assert.Equal(3, compilation.SyntaxTrees.Length);
89+
compilation.GetParseDiagnostics().Verify(
90+
// external3.csx(3,23): error CS2015: 'a.csx' is a binary file instead of a text file
91+
// #load "a.csx"
92+
Diagnostic(ErrorCode.ERR_BinaryFile, @"""a.csx""").WithArguments("a.csx").WithLocation(3, 23),
93+
// b.csx(1,7): error CS2015: 'a.csx' is a binary file instead of a text file
94+
// #load "a.csx"
95+
Diagnostic(ErrorCode.ERR_BinaryFile, @"""a.csx""").WithArguments("a.csx").WithLocation(1, 7));
96+
97+
var external4 = Parse("#load \"a.csx\"", filename: "external4.csx", options: TestOptions.Script);
98+
compilation = compilation.ReplaceSyntaxTree(external3, external4);
99+
100+
Assert.Equal(3, compilation.SyntaxTrees.Length);
101+
compilation.GetParseDiagnostics().Verify(
102+
// external4.csx(1,7): error CS2015: 'a.csx' is a binary file instead of a text file
103+
// #load "a.csx"
104+
Diagnostic(ErrorCode.ERR_BinaryFile, @"""a.csx""").WithArguments("a.csx").WithLocation(1, 7),
105+
// b.csx(1,7): error CS2015: 'a.csx' is a binary file instead of a text file
106+
// #load "a.csx"
107+
Diagnostic(ErrorCode.ERR_BinaryFile, @"""a.csx""").WithArguments("a.csx").WithLocation(1, 7));
108+
109+
compilation = compilation.RemoveSyntaxTrees(external2);
110+
111+
Assert.Equal(external4, compilation.SyntaxTrees.Single());
112+
compilation.GetParseDiagnostics().Verify(
113+
// external4.csx(1,7): error CS2015: 'a.csx' is a binary file instead of a text file
114+
// #load "a.csx"
115+
Diagnostic(ErrorCode.ERR_BinaryFile, @"""a.csx""").WithArguments("a.csx").WithLocation(1, 7));
116+
}
117+
63118
[Fact]
64119
public void NoSourceReferenceResolver()
65120
{

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,22 @@ public sealed class TestSourceReferenceResolver : SourceReferenceResolver
1515

1616
public static SourceReferenceResolver Create(params KeyValuePair<string, string>[] sources)
1717
{
18-
return TestSourceReferenceResolver.Create(sources.ToDictionary(p => p.Key, p => p.Value));
18+
return new TestSourceReferenceResolver(sources.ToDictionary(p => p.Key, p => (object)p.Value));
19+
}
20+
21+
public static SourceReferenceResolver Create(params KeyValuePair<string, object>[] sources)
22+
{
23+
return new TestSourceReferenceResolver(sources.ToDictionary(p => p.Key, p => p.Value));
1924
}
2025

2126
public static SourceReferenceResolver Create(Dictionary<string, string> sources = null)
2227
{
23-
return (sources == null || sources.Count == 0) ? Default : new TestSourceReferenceResolver(sources);
28+
return new TestSourceReferenceResolver(sources.ToDictionary(p => p.Key, p => (object)p.Value));
2429
}
2530

26-
private readonly Dictionary<string, string> _sources;
31+
private readonly Dictionary<string, object> _sources;
2732

28-
private TestSourceReferenceResolver(Dictionary<string, string> sources)
33+
private TestSourceReferenceResolver(Dictionary<string, object> sources)
2934
{
3035
_sources = sources;
3136
}
@@ -39,7 +44,8 @@ public override Stream OpenRead(string resolvedPath)
3944
{
4045
if (_sources != null && resolvedPath != null)
4146
{
42-
return new MemoryStream(Encoding.UTF8.GetBytes(_sources[resolvedPath]));
47+
var data = _sources[resolvedPath];
48+
return new MemoryStream((data is string) ? Encoding.UTF8.GetBytes((string)data) : (byte[])data);
4349
}
4450
else
4551
{

0 commit comments

Comments
 (0)