Merge pull request #1272 from OmniSharp/feature/csx-rsp-tests

construct scripting workspace information directly from the ProjectInfo
OmniSharp · Aug 28, 2018 · 9e76cbf · 9e76cbf
2 parents ed12fc8 + 37f1859
commit 9e76cbf
Show file tree

Hide file tree

Showing 10 changed files with 67 additions and 27 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,7 @@ All changes to the project will be documented in this file.
 
 ## [1.33.0] - Not yet released
 * Added support for files without a project. (PR: [#1252](https://github.com/OmniSharp/omnisharp-roslyn/pull/1252))
+* Fixed a bug where `*.rsp`-based scripting references where not exposed in the Workspace information endpoint (PR: [#1272](https://github.com/OmniSharp/omnisharp-roslyn/pull/1272))
 
 ## [1.32.2] - 2018-08-07
 * OmniSharp now targets **net461**, instead of **net46** (PR: [#1237](https://github.com/OmniSharp/omnisharp-roslyn/pull/1237))

diff --git a/src/OmniSharp.Script/ScriptContext.cs b/src/OmniSharp.Script/ScriptContext.cs
@@ -7,11 +7,10 @@ namespace OmniSharp.Script
 {
     public class ScriptContext
     {
-        public ScriptContext(ScriptProjectProvider scriptProjectProvider, HashSet<MetadataReference> metadataReferences, HashSet<string> assemblyReferences, CompilationDependency[] compilationDependencies, Type globalsType)
+        public ScriptContext(ScriptProjectProvider scriptProjectProvider, HashSet<MetadataReference> metadataReferences, CompilationDependency[] compilationDependencies, Type globalsType)
         {
             ScriptProjectProvider = scriptProjectProvider;
             MetadataReferences = metadataReferences;
-            AssemblyReferences = assemblyReferences;
             CompilationDependencies = compilationDependencies;
             GlobalsType = globalsType;
         }
@@ -20,8 +19,6 @@ public ScriptContext(ScriptProjectProvider scriptProjectProvider, HashSet<Metada
 
         public HashSet<MetadataReference> MetadataReferences { get; }
 
-        public HashSet<string> AssemblyReferences { get; }
-
         public CompilationDependency[] CompilationDependencies { get; }
 
         public Type GlobalsType { get; } 

diff --git a/src/OmniSharp.Script/ScriptContextModel.cs b/src/OmniSharp.Script/ScriptContextModel.cs
@@ -1,22 +1,32 @@
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
 
 namespace OmniSharp.Script
 {
     public class ScriptContextModel
     {
-        public ScriptContextModel(string csxPath, ProjectInfo project, HashSet<string> implicitAssemblyReferences)
+        public ScriptContextModel(string csxPath, ProjectInfo project)
         {
             Path = csxPath;
-            ImplicitAssemblyReferences = implicitAssemblyReferences;
-            CommonUsings = ScriptProjectProvider.DefaultNamespaces;
+            AssemblyReferences = project.MetadataReferences.Select(a =>
+            {
+                if (a is PortableExecutableReference portableExecutableReference)
+                {
+                    return portableExecutableReference.FilePath ?? portableExecutableReference.Display;
+                }
+
+                return a.Display;
+            });
+            CommonUsings = ((CSharpCompilationOptions)(project.CompilationOptions)).Usings;
             GlobalsType = project.HostObjectType;
         }
 
         public string Path { get; }
 
-        public HashSet<string> ImplicitAssemblyReferences { get; }
+        public IEnumerable<string> AssemblyReferences { get; }
 
         public Type GlobalsType { get; }
 

diff --git a/src/OmniSharp.Script/ScriptContextProvider.cs b/src/OmniSharp.Script/ScriptContextProvider.cs
@@ -76,7 +76,6 @@ public ScriptContext CreateScriptContext(ScriptOptions scriptOptions)
             }
 
             var metadataReferences = new HashSet<MetadataReference>(MetadataReferenceEqualityComparer.Instance);
-            var assemblyReferences = new HashSet<string>();
 
             var isDesktopClr = true;
             // if we have no compilation dependencies
@@ -86,7 +85,7 @@ public ScriptContext CreateScriptContext(ScriptOptions scriptOptions)
             if (!compilationDependencies.Any())
             {
                 _logger.LogInformation("Unable to find dependency context for CSX files. Will default to non-context usage (Desktop CLR scripts).");
-                AddDefaultClrMetadataReferences(metadataReferences, assemblyReferences);
+                AddDefaultClrMetadataReferences(metadataReferences);
             }
             else
             {
@@ -98,7 +97,7 @@ public ScriptContext CreateScriptContext(ScriptOptions scriptOptions)
                     if (loadedFiles.Add(Path.GetFileName(compilationAssembly)))
                     {
                         _logger.LogDebug("Discovered script compilation assembly reference: " + compilationAssembly);
-                        AddMetadataReference(metadataReferences, assemblyReferences, compilationAssembly);
+                        AddMetadataReference(metadataReferences, compilationAssembly);
                     }
                 }
             }
@@ -107,30 +106,26 @@ public ScriptContext CreateScriptContext(ScriptOptions scriptOptions)
             foreach (var inheritedCompileLib in inheritedCompileLibraries)
             {
                 _logger.LogDebug("Adding implicit reference: " + inheritedCompileLib);
-                AddMetadataReference(metadataReferences, assemblyReferences, inheritedCompileLib.Location);
+                AddMetadataReference(metadataReferences, inheritedCompileLib.Location);
             }
 
             var scriptProjectProvider = new ScriptProjectProvider(scriptOptions, _env, _loggerFactory, isDesktopClr);
 
-            return new ScriptContext(scriptProjectProvider, metadataReferences, assemblyReferences, compilationDependencies, _defaultGlobalsType);
+            return new ScriptContext(scriptProjectProvider, metadataReferences, compilationDependencies, _defaultGlobalsType);
         }
 
-        private void AddDefaultClrMetadataReferences(HashSet<MetadataReference> commonReferences, HashSet<string> assemblyReferences)
+        private void AddDefaultClrMetadataReferences(HashSet<MetadataReference> commonReferences)
         {
             var references = DefaultMetadataReferenceHelper.GetDefaultMetadataReferenceLocations()
-                .Select(l =>
-                {
-                    assemblyReferences.Add(l);
-                    return _metadataFileReferenceCache.GetMetadataReference(l);
-                });
+                .Select(l => _metadataFileReferenceCache.GetMetadataReference(l));
 
             foreach (var reference in references)
             {
                 commonReferences.Add(reference);
             }
         }
 
-        private void AddMetadataReference(ISet<MetadataReference> referenceCollection, HashSet<string> assemblyReferences, string fileReference)
+        private void AddMetadataReference(ISet<MetadataReference> referenceCollection, string fileReference)
         {
             if (!File.Exists(fileReference))
             {
@@ -146,7 +141,6 @@ private void AddMetadataReference(ISet<MetadataReference> referenceCollection, H
             }
 
             referenceCollection.Add(metadataReference);
-            assemblyReferences.Add(fileReference);
             _logger.LogDebug($"Added reference to '{fileReference}'");
         }
     }

diff --git a/src/OmniSharp.Script/ScriptProjectSystem.cs b/src/OmniSharp.Script/ScriptProjectSystem.cs
@@ -168,15 +168,15 @@ Task<object> IProjectSystem.GetProjectModelAsync(string filePath)
                 return Task.FromResult<object>(null);
             }
 
-            return Task.FromResult<object>(new ScriptContextModel(filePath, projectInfo, _scriptContext.Value.AssemblyReferences));
+            return Task.FromResult<object>(new ScriptContextModel(filePath, projectInfo));
         }
 
         Task<object> IProjectSystem.GetWorkspaceModelAsync(WorkspaceInformationRequest request)
         {
             var scriptContextModels = new List<ScriptContextModel>();
             foreach (var project in _projects)
             {
-                scriptContextModels.Add(new ScriptContextModel(project.Key, project.Value, _scriptContext.Value.AssemblyReferences));
+                scriptContextModels.Add(new ScriptContextModel(project.Key, project.Value));
             }
             return Task.FromResult<object>(new ScriptContextModelCollection(scriptContextModels));
         }

diff --git a/test-assets/test-scripts/EmptyScript/.gitignore b/test-assets/test-scripts/EmptyScript/.gitignore
@@ -0,0 +1,4 @@
+# Ignore everything
+*
+# Except gitignore file
+!.gitignore
diff --git a/test-assets/test-scripts/SingleCsiScriptWithCustomRsp/main.csx b/test-assets/test-scripts/SingleCsiScriptWithCustomRsp/main.csx
@@ -0,0 +1,5 @@
+#r "System.Net.Http"
+
+using System.Net.Http;
+var client = new HttpClient();
+Console.WriteLine(client.GetAsync("https://google.com"));
diff --git a/test-assets/test-scripts/SingleCsiScriptWithCustomRsp/test.rsp b/test-assets/test-scripts/SingleCsiScriptWithCustomRsp/test.rsp
@@ -0,0 +1,2 @@
+/u:System.Web
+/r:System.Web
diff --git a/tests/OmniSharp.Script.Tests/WorkspaceInformationTests.cs b/tests/OmniSharp.Script.Tests/WorkspaceInformationTests.cs
@@ -46,6 +46,33 @@ public async Task SingleCsiScript()
             }
         }
 
+        [Fact]
+        public async Task SingleCsiScriptWithCustomRspNamespacesAndReferences()
+        {
+            using (var testProject = TestAssets.Instance.GetTestScript("SingleCsiScriptWithCustomRsp"))
+            using (var host = CreateOmniSharpHost(testProject.Directory, new Dictionary<string, string>
+            {
+                ["script:rspFilePath"] = Path.Combine(testProject.Directory, "test.rsp")
+            }))
+            {
+                var workspaceInfo = await GetWorkspaceInfoAsync(host);
+                var project = Assert.Single(workspaceInfo.Projects);
+
+                Assert.Equal("main.csx", Path.GetFileName(project.Path));
+
+                // should have reference to mscorlib
+                VerifyCorLib(project);
+
+                // default globals object
+                Assert.Equal(typeof(CommandLineScriptGlobals), project.GlobalsType);
+
+                // should have RSP inherited settings
+                VerifyAssemblyReference(project, "system.web");
+                var commonUsingStatement = Assert.Single(project.CommonUsings);
+                Assert.Equal("System.Web", commonUsingStatement);
+            }
+        }
+
         [Fact]
         public async Task CsiScriptWithFileCreatedAfterStartingServer()
         {
@@ -163,12 +190,12 @@ public async Task DotnetCoreScriptWithNuget()
 
         private void VerifyCorLib(ScriptContextModel project, bool expected = true)
         {
-            var corLibFound = project.ImplicitAssemblyReferences.Any(r => r == GetMsCorlibPath());
+            var corLibFound = project.AssemblyReferences.Any(r => r == GetMsCorlibPath());
             Assert.True(corLibFound == expected, $"{(expected ? "Missing" : "Unnecessary")} reference to mscorlib");
         }
 
         private void VerifyAssemblyReference(ScriptContextModel project, string partialName) =>
-            Assert.True(project.ImplicitAssemblyReferences.Any(r => r.IndexOf(partialName, StringComparison.OrdinalIgnoreCase) > 0), $"Missing reference to {partialName}");
+            Assert.True(project.AssemblyReferences.Any(r => r.IndexOf(partialName, StringComparison.OrdinalIgnoreCase) > 0), $"Missing reference to {partialName}");
 
         private static async Task<ScriptContextModelCollection> GetWorkspaceInfoAsync(OmniSharpTestHost host)
         {

diff --git a/tests/TestUtility/TestAssets.TestProject.cs b/tests/TestUtility/TestAssets.TestProject.cs
@@ -32,7 +32,7 @@ public TestProject(string name, string baseDirectory, string directory, bool sha
 
             public string AddDisposableFile(string fileName, string contents = null)
             {
-                var filePath = Path.Combine(BaseDirectory, fileName);
+                var filePath = Path.Combine(Directory, fileName);
                 File.WriteAllText(filePath, contents ?? string.Empty);
                 _disposableFiles.Add(filePath);
 
@@ -59,7 +59,7 @@ public virtual void Dispose()
                     foreach (var filePath in _disposableFiles)
                     {
                         RunWithRetry(() => File.Delete(filePath));
-                        if (System.IO.Directory.Exists(this.BaseDirectory))
+                        if (System.IO.File.Exists(filePath))
                         {
                             throw new InvalidOperationException($"{nameof(ITestProject)} file still exists: '{filePath}'");
                         }