-
Notifications
You must be signed in to change notification settings - Fork 420
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added support for referencing NuGet packages in C# scripts #813
Changes from 2 commits
02c7124
ad90c6f
f1d6dd7
6252c0d
c3bbfcc
0155e96
63db355
d58e538
a8683af
0e902d1
4607fbd
05756ec
5499e1e
1511099
d8eef4e
a7f7daf
7e52b3c
8316f1c
a603bf5
c9e9de7
e5a341d
ac8974a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,27 +1,32 @@ | ||
using System.Collections.Generic; | ||
using System.Collections.Immutable; | ||
using Microsoft.CodeAnalysis; | ||
using Microsoft.CodeAnalysis.Scripting; | ||
|
||
namespace OmniSharp.Script | ||
{ | ||
public class CachingScriptMetadataResolver : MetadataReferenceResolver | ||
{ | ||
private readonly MetadataReferenceResolver defaultReferenceResolver; | ||
private static Dictionary<string, ImmutableArray<PortableExecutableReference>> DirectReferenceCache = new Dictionary<string, ImmutableArray<PortableExecutableReference>>(); | ||
private static Dictionary<string, PortableExecutableReference> MissingReferenceCache = new Dictionary<string, PortableExecutableReference>(); | ||
private static MetadataReferenceResolver _defaultRuntimeResolver = ScriptMetadataResolver.Default; | ||
|
||
|
||
public CachingScriptMetadataResolver(MetadataReferenceResolver defaultReferenceResolver) | ||
{ | ||
this.defaultReferenceResolver = defaultReferenceResolver; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use underscore for private field name. |
||
} | ||
|
||
public override bool Equals(object other) | ||
{ | ||
return _defaultRuntimeResolver.Equals(other); | ||
return defaultReferenceResolver.Equals(other); | ||
} | ||
|
||
public override int GetHashCode() | ||
{ | ||
return _defaultRuntimeResolver.GetHashCode(); | ||
return defaultReferenceResolver.GetHashCode(); | ||
} | ||
|
||
public override bool ResolveMissingAssemblies => _defaultRuntimeResolver.ResolveMissingAssemblies; | ||
public override bool ResolveMissingAssemblies => defaultReferenceResolver.ResolveMissingAssemblies; | ||
|
||
public override PortableExecutableReference ResolveMissingAssembly(MetadataReference definition, AssemblyIdentity referenceIdentity) | ||
{ | ||
|
@@ -30,7 +35,7 @@ public override PortableExecutableReference ResolveMissingAssembly(MetadataRefer | |
return MissingReferenceCache[referenceIdentity.Name]; | ||
} | ||
|
||
var result = _defaultRuntimeResolver.ResolveMissingAssembly(definition, referenceIdentity); | ||
var result = defaultReferenceResolver.ResolveMissingAssembly(definition, referenceIdentity); | ||
if (result != null) | ||
{ | ||
MissingReferenceCache[referenceIdentity.Name] = result; | ||
|
@@ -47,7 +52,7 @@ public override ImmutableArray<PortableExecutableReference> ResolveReference(str | |
return DirectReferenceCache[key]; | ||
} | ||
|
||
var result = _defaultRuntimeResolver.ResolveReference(reference, baseFilePath, properties); | ||
var result = defaultReferenceResolver.ResolveReference(reference, baseFilePath, properties); | ||
if (result.Length > 0) | ||
{ | ||
DirectReferenceCache[key] = result; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,9 @@ | |
|
||
namespace OmniSharp.Script | ||
{ | ||
using Dotnet.Script.NuGetMetadataResolver; | ||
using Microsoft.DotNet.InternalAbstractions; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please don't nest usings inside of namespaces. Put them with the rest of the namespaces at the top of the file. |
||
|
||
[Export(typeof(IProjectSystem)), Shared] | ||
public class ScriptProjectSystem : IProjectSystem | ||
{ | ||
|
@@ -39,38 +42,45 @@ public class ScriptProjectSystem : IProjectSystem | |
|
||
private const string CsxExtension = ".csx"; | ||
private static readonly CSharpParseOptions ParseOptions = new CSharpParseOptions(LanguageVersion.Default, DocumentationMode.Parse, SourceCodeKind.Script); | ||
|
||
private static readonly Lazy<CSharpCompilationOptions> CompilationOptions = new Lazy<CSharpCompilationOptions>(() => | ||
private readonly Lazy<CSharpCompilationOptions> _compilationOptions; | ||
private CSharpCompilationOptions CreateCompilation() | ||
{ | ||
var resolver = NuGetMetadataReferenceResolver.Create(ScriptMetadataResolver.Default, | ||
NugetFrameworkProvider.GetFrameworkNameFromAssembly(),loggerFactory, _env.Path); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. missing space between There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove extra blank line. |
||
var compilationOptions = new CSharpCompilationOptions( | ||
OutputKind.DynamicallyLinkedLibrary, | ||
usings: DefaultNamespaces, | ||
allowUnsafe: true, | ||
metadataReferenceResolver: new CachingScriptMetadataResolver(), | ||
sourceReferenceResolver: ScriptSourceResolver.Default, | ||
assemblyIdentityComparer: DesktopAssemblyIdentityComparer.Default). | ||
OutputKind.DynamicallyLinkedLibrary, | ||
usings: DefaultNamespaces, | ||
allowUnsafe: true, | ||
metadataReferenceResolver: | ||
new CachingScriptMetadataResolver(resolver), | ||
sourceReferenceResolver: ScriptSourceResolver.Default, | ||
assemblyIdentityComparer: DesktopAssemblyIdentityComparer.Default). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you like, maybe move the |
||
WithSpecificDiagnosticOptions(new Dictionary<string, ReportDiagnostic> | ||
{ | ||
// ensure that specific warnings about assembly references are always suppressed | ||
// https://github.com/dotnet/roslyn/issues/5501 | ||
{ "CS1701", ReportDiagnostic.Suppress }, | ||
{ "CS1702", ReportDiagnostic.Suppress }, | ||
{ "CS1705", ReportDiagnostic.Suppress } | ||
}); | ||
|
||
var topLevelBinderFlagsProperty = typeof(CSharpCompilationOptions).GetProperty("TopLevelBinderFlags", BindingFlags.Instance | BindingFlags.NonPublic); | ||
var binderFlagsType = typeof(CSharpCompilationOptions).GetTypeInfo().Assembly.GetType("Microsoft.CodeAnalysis.CSharp.BinderFlags"); | ||
|
||
var ignoreCorLibraryDuplicatedTypesMember = binderFlagsType?.GetField("IgnoreCorLibraryDuplicatedTypes", BindingFlags.Static | BindingFlags.Public); | ||
{"CS1701", ReportDiagnostic.Suppress}, | ||
{"CS1702", ReportDiagnostic.Suppress}, | ||
{"CS1705", ReportDiagnostic.Suppress} | ||
}); | ||
|
||
var topLevelBinderFlagsProperty = typeof(CSharpCompilationOptions).GetProperty("TopLevelBinderFlags", | ||
BindingFlags.Instance | BindingFlags.NonPublic); | ||
var binderFlagsType = | ||
typeof(CSharpCompilationOptions).GetTypeInfo().Assembly.GetType("Microsoft.CodeAnalysis.CSharp.BinderFlags"); | ||
|
||
var ignoreCorLibraryDuplicatedTypesMember = binderFlagsType?.GetField("IgnoreCorLibraryDuplicatedTypes", | ||
BindingFlags.Static | BindingFlags.Public); | ||
var ignoreCorLibraryDuplicatedTypesValue = ignoreCorLibraryDuplicatedTypesMember?.GetValue(null); | ||
if (ignoreCorLibraryDuplicatedTypesValue != null) | ||
{ | ||
topLevelBinderFlagsProperty?.SetValue(compilationOptions, ignoreCorLibraryDuplicatedTypesValue); | ||
} | ||
|
||
return compilationOptions; | ||
}); | ||
|
||
} | ||
private readonly IMetadataFileReferenceCache _metadataFileReferenceCache; | ||
|
||
// used for tracking purposes only | ||
|
@@ -79,16 +89,29 @@ public class ScriptProjectSystem : IProjectSystem | |
private readonly Dictionary<string, ProjectInfo> _projects; | ||
private readonly OmniSharpWorkspace _workspace; | ||
private readonly IOmniSharpEnvironment _env; | ||
private readonly ILoggerFactory loggerFactory; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add an underscore to match the other fields in this file. |
||
private readonly ILogger _logger; | ||
private static readonly Lazy<string> _targetFrameWork = new Lazy<string>(ResolveTargetFramework); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rename to |
||
|
||
private static string ResolveTargetFramework() | ||
{ | ||
return Assembly.GetEntryAssembly().GetCustomAttributes() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what problem did you run into that you added this code? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No problem. I was just making sure that I did not change the way metadata references are being added when running on the full desktop framework. |
||
.OfType<System.Runtime.Versioning.TargetFrameworkAttribute>() | ||
.Select(x => x.FrameworkName) | ||
.FirstOrDefault(); | ||
} | ||
|
||
|
||
[ImportingConstructor] | ||
public ScriptProjectSystem(OmniSharpWorkspace workspace, IOmniSharpEnvironment env, ILoggerFactory loggerFactory, IMetadataFileReferenceCache metadataFileReferenceCache) | ||
{ | ||
_metadataFileReferenceCache = metadataFileReferenceCache; | ||
_workspace = workspace; | ||
_env = env; | ||
this.loggerFactory = loggerFactory; | ||
_logger = loggerFactory.CreateLogger<ScriptProjectSystem>(); | ||
_projects = new Dictionary<string, ProjectInfo>(); | ||
_compilationOptions = new Lazy<CSharpCompilationOptions>(CreateCompilation); | ||
} | ||
|
||
public string Key => "Script"; | ||
|
@@ -121,18 +144,38 @@ public void Initalize(IConfiguration configuration) | |
|
||
var commonReferences = new HashSet<MetadataReference>(); | ||
|
||
// if we have no context, then we also have no dependencies | ||
// we can assume desktop framework | ||
// and add mscorlib | ||
|
||
if (runtimeContexts == null || runtimeContexts.Any() == false) | ||
{ | ||
_logger.LogInformation("Unable to find project context for CSX files. Will default to non-context usage."); | ||
_logger.LogInformation($"Unable to find project context for CSX files. Will default to non-context usage for target framework {_targetFrameWork.Value}"); | ||
|
||
// We are running in the context of a .Net Core app. | ||
if (_targetFrameWork.Value.StartsWith(".NETCoreApp")) | ||
{ | ||
var runtimeId = RuntimeEnvironment.GetRuntimeIdentifier(); | ||
var inheritedAssemblyNames = DependencyContext.Default.GetRuntimeAssemblyNames(runtimeId).Where(x => | ||
x.FullName.StartsWith("system.", StringComparison.OrdinalIgnoreCase) || | ||
x.FullName.StartsWith("microsoft.codeanalysis", StringComparison.OrdinalIgnoreCase) || | ||
x.FullName.StartsWith("mscorlib", StringComparison.OrdinalIgnoreCase)); | ||
|
||
foreach (var inheritedAssemblyName in inheritedAssemblyNames) | ||
{ | ||
var assembly = Assembly.Load(inheritedAssemblyName); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We generally use an |
||
AddMetadataReference(commonReferences, assembly.Location); | ||
} | ||
} | ||
else | ||
{ | ||
|
||
AddMetadataReference(commonReferences, typeof(object).GetTypeInfo().Assembly.Location); | ||
AddMetadataReference(commonReferences, typeof(Enumerable).GetTypeInfo().Assembly.Location); | ||
// Assume desktop framework. | ||
AddMetadataReference(commonReferences, typeof(object).GetTypeInfo().Assembly.Location); | ||
AddMetadataReference(commonReferences, typeof(Enumerable).GetTypeInfo().Assembly.Location); | ||
|
||
inheritedCompileLibraries.AddRange(DependencyContext.Default.CompileLibraries.Where(x => | ||
x.Name.ToLowerInvariant().StartsWith("system.runtime"))); | ||
} | ||
|
||
|
||
inheritedCompileLibraries.AddRange(DependencyContext.Default.CompileLibraries.Where(x => | ||
x.Name.ToLowerInvariant().StartsWith("system.runtime"))); | ||
} | ||
// otherwise we will grab dependencies for the script from the runtime context | ||
else | ||
|
@@ -181,7 +224,7 @@ public void Initalize(IConfiguration configuration) | |
name: csxFileName, | ||
assemblyName: $"{csxFileName}.dll", | ||
language: LanguageNames.CSharp, | ||
compilationOptions: CompilationOptions.Value, | ||
compilationOptions: _compilationOptions.Value, | ||
metadataReferences: commonReferences, | ||
parseOptions: ParseOptions, | ||
isSubmission: true, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove extra blank line