Globals and return types for script compilations should be surfaced as ITypeSymbol, not System.Type #20920
Milestone
Comments
sandyarmstrong
added a commit
to microsoft/workbooks
that referenced
this issue
Nov 2, 2017
Additionally, the Xamarin.Interactive.Shared project that was consumed by each agent assembly is gone. All those types are now part of the actual Xamarin.Interactive.dll. Same with Xamarin.Interactive.Bootstrap. InspectorSupport is no longer a partial class, because the shared projects are gone now. But we need to keep the same API for Inspector extension compatibility, and because that API is based on static methods there's no helpful way to abstract any of it. So there is a little duplication now. Xamarin.Interactive.Facades is no longer necessary, and neither is the PCL loading code in `UnifiedInspectorSupport`. Xamarin.Forms has been bumped from 2.3.4.231 to 2.4.0.18342 for .NET Standard 2.0 support. `CompilationWorkspaceFactory` has to do some manual loading of `Xamarin.Interactive.dll` and `netstandard.dll` now, but this will go away when dotnet/roslyn#20920 gets fixed.
21 tasks
sandyarmstrong
added a commit
to microsoft/workbooks
that referenced
this issue
Nov 3, 2017
Additionally, the Xamarin.Interactive.Shared project that was consumed by each agent assembly is gone. All those types are now part of the actual Xamarin.Interactive.dll. Same with Xamarin.Interactive.Bootstrap. InspectorSupport is no longer a partial class, because the shared projects are gone now. But we need to keep the same API for Inspector extension compatibility, and because that API is based on static methods there's no helpful way to abstract any of it. So there is a little duplication now. Xamarin.Interactive.Facades is no longer necessary, and neither is the PCL loading code in `UnifiedInspectorSupport`. Xamarin.Forms has been bumped from 2.3.4.231 to 2.4.0.18342 for .NET Standard 2.0 support. `CompilationWorkspaceFactory` has to do some manual loading of `Xamarin.Interactive.dll` and `netstandard.dll` now, but this will go away when dotnet/roslyn#20920 gets fixed.
sandyarmstrong
added a commit
to microsoft/workbooks
that referenced
this issue
Nov 3, 2017
Additionally, the Xamarin.Interactive.Shared project that was consumed by each agent assembly is gone. All those types are now part of the actual Xamarin.Interactive.dll. Same with Xamarin.Interactive.Bootstrap. InspectorSupport is no longer a partial class, because the shared projects are gone now. But we need to keep the same API for Inspector extension compatibility, and because that API is based on static methods there's no helpful way to abstract any of it. So there is a little duplication now. Xamarin.Interactive.Facades is no longer necessary, and neither is the PCL loading code in `UnifiedInspectorSupport`. Xamarin.Forms has been bumped from 2.3.4.231 to 2.4.0.18342 for .NET Standard 2.0 support. `CompilationWorkspaceFactory` has to do some manual loading of `Xamarin.Interactive.dll` and `netstandard.dll` now, but this will go away when dotnet/roslyn#20920 gets fixed.
sandyarmstrong
added a commit
to microsoft/workbooks
that referenced
this issue
Nov 8, 2017
Additionally, the Xamarin.Interactive.Shared project that was consumed by each agent assembly is gone. All those types are now part of the actual Xamarin.Interactive.dll. Same with Xamarin.Interactive.Bootstrap. InspectorSupport is no longer a partial class, because the shared projects are gone now. But we need to keep the same API for Inspector extension compatibility, and because that API is based on static methods there's no helpful way to abstract any of it. So there is a little duplication now. Xamarin.Interactive.Facades is no longer necessary, and neither is the PCL loading code in `UnifiedInspectorSupport`. Xamarin.Forms has been bumped from 2.3.4.231 to 2.4.0.18342 for .NET Standard 2.0 support. `CompilationWorkspaceFactory` has to do some manual loading of `Xamarin.Interactive.dll` and `netstandard.dll` now, but this will go away when dotnet/roslyn#20920 gets fixed.
sandyarmstrong
added a commit
to microsoft/workbooks
that referenced
this issue
Nov 8, 2017
Additionally, the Xamarin.Interactive.Shared project that was consumed by each agent assembly is gone. All those types are now part of the actual Xamarin.Interactive.dll. Same with Xamarin.Interactive.Bootstrap. InspectorSupport is no longer a partial class, because the shared projects are gone now. But we need to keep the same API for Inspector extension compatibility, and because that API is based on static methods there's no helpful way to abstract any of it. So there is a little duplication now. Xamarin.Interactive.Facades is no longer necessary, and neither is the PCL loading code in `UnifiedInspectorSupport`. Xamarin.Forms has been bumped from 2.3.4.231 to 2.4.0.18342 for .NET Standard 2.0 support. `CompilationWorkspaceFactory` has to do some manual loading of `Xamarin.Interactive.dll` and `netstandard.dll` now, but this will go away when dotnet/roslyn#20920 gets fixed.
sandyarmstrong
added a commit
to microsoft/workbooks
that referenced
this issue
Nov 10, 2017
Additionally, the Xamarin.Interactive.Shared project that was consumed by each agent assembly is gone. All those types are now part of the actual Xamarin.Interactive.dll. Same with Xamarin.Interactive.Bootstrap. InspectorSupport is no longer a partial class, because the shared projects are gone now. But we need to keep the same API for Inspector extension compatibility, and because that API is based on static methods there's no helpful way to abstract any of it. So there is a little duplication now. Xamarin.Interactive.Facades is no longer necessary, and neither is the PCL loading code in `UnifiedInspectorSupport`. Xamarin.Forms has been bumped from 2.3.4.231 to 2.4.0.18342 for .NET Standard 2.0 support. `CompilationWorkspaceFactory` has to do some manual loading of `Xamarin.Interactive.dll` and `netstandard.dll` now, but this will go away when dotnet/roslyn#20920 gets fixed.
sandyarmstrong
added a commit
to microsoft/workbooks
that referenced
this issue
Nov 10, 2017
Additionally, the Xamarin.Interactive.Shared project that was consumed by each agent assembly is gone. All those types are now part of the actual Xamarin.Interactive.dll. Same with Xamarin.Interactive.Bootstrap. InspectorSupport is no longer a partial class, because the shared projects are gone now. But we need to keep the same API for Inspector extension compatibility, and because that API is based on static methods there's no helpful way to abstract any of it. So there is a little duplication now. Xamarin.Interactive.Facades is no longer necessary, and neither is the PCL loading code in `UnifiedInspectorSupport`. Xamarin.Forms has been bumped from 2.3.4.231 to 2.4.0.18342 for .NET Standard 2.0 support. `CompilationWorkspaceFactory` has to do some manual loading of `Xamarin.Interactive.dll` and `netstandard.dll` now, but this will go away when dotnet/roslyn#20920 gets fixed.
sandyarmstrong
added a commit
to microsoft/workbooks
that referenced
this issue
Nov 10, 2017
Additionally, the Xamarin.Interactive.Shared project that was consumed by each agent assembly is gone. All those types are now part of the actual Xamarin.Interactive.dll. Same with Xamarin.Interactive.Bootstrap. InspectorSupport is no longer a partial class, because the shared projects are gone now. But we need to keep the same API for Inspector extension compatibility, and because that API is based on static methods there's no helpful way to abstract any of it. So there is a little duplication now. Xamarin.Interactive.Facades is no longer necessary, and neither is the PCL loading code in `UnifiedInspectorSupport`. Xamarin.Forms has been bumped from 2.3.4.231 to 2.4.0.18342 for .NET Standard 2.0 support. `CompilationWorkspaceFactory` has to do some manual loading of `Xamarin.Interactive.dll` and `netstandard.dll` now, but this will go away when dotnet/roslyn#20920 gets fixed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Version Used: 2.3.0
Overview
Script compilations provide two facilities that currently tie the compilations to specific runtime host targets: return type and globals type (also called "host object type" in places).
In all public API necessary to create script compilations (e.g.
Microsoft.CodeAnalysis.CSharp.CSharpCompilation.CreateScriptCompilationMicrosoft.CodeAnalysis.ProjectInfo), return type and globals object type are provided by the consumer of the API as aSystem.Type. In some cases (e.g. Xamarin Workbooks), these types must be reflection loaded in order to satisfy the Roslyn APIs.Unfortunately, the reflection loading workaround no longer works when the assembly containing the type is a .NET core assembly (and Roslyn is running on .NET Framework / Desktop). This is currently preventing the Xamarin Workbooks team from cleanly providing support for .NET Core workbooks.
Fix Proposal
Roslyn needs an API audit to identify cases where
System.Typeis surfaced to the consumer. These APIs should be obsoleted and new ones provided that provide equivalent functionality throughITypeSymbol(or similar). This would allow the consumer of Roslyn to decide how to load the metadata (Cci, System.Reflection.Metadata, Cecil, etc.) and take no dependency on the runtime hosting Roslyn.Further, a single name should be chosen for APIs representing the globals type. Currently this concept is called "host object type" and "globals type" in different APIs. I'd vote for "globals type" since it is more clear as to what the type is used for (surfacing "global" APIs to scripts).
M.CA.ScriptCompilationInfoSystem.Type GlobalsTypeITypeSymbol GlobalsTypeSymbolM.CA.ScriptCompilationInfoSystem.Type ReturnTypeITypeSymbol ReturnTypeSymbolM.CA.Scripting.ScriptSystem.Type GlobalsTypeITypeSymbol GlobalsTypeSymbolM.CA.Scripting.ScriptSystem.Type ReturnTypeITypeSymbol ReturnTypeSymbolM.CA.ProjectInfoSystem.Type HostObjectTypeITypeSymbol GlobalsTypeSymbolNote: this table is not a complete audit, nor does it cover method parameters, but changing public properties should allow everything else to become obvious.
Addendum
Independent of this issue, #20919 - the
ReturnTypeAPI is not surfaced at all onMicrosoft.CodeAnalysis.ProjectInfo. Fixing that issue should wait until this issue is addressed.This is also somewhat related to / will affect similar code paths as #8506, which was fixed by #8507.
The text was updated successfully, but these errors were encountered: