Add API Compilation.ClassifyCommonConverstion#27093
Conversation
Also add - CommonConversion.IsImplicit - Compilation.HasImplicitConversion Fixes dotnet#9461
| /// either <paramref name="fromType"/> or <paramref name="toType"/> is null, or | ||
| /// if no such conversion exists. | ||
| /// </summary> | ||
| public bool HasImplicitConversion(ITypeSymbol fromType, ITypeSymbol toType) |
There was a problem hiding this comment.
We're not super consistent, but the common pattern in Compilation appears to be throwing if parameters are null. Why not here?
There was a problem hiding this comment.
Because this is what is most useful for clients of this API. I submit that bool-returning methods can be resilient against null (no conversion exists to or from no type):
public new bool ContainsSyntaxTree(SyntaxTree syntaxTree)
{
return syntaxTree != null && _syntaxAndDeclarations.GetLazyState().RootNamespaces.ContainsKey(syntaxTree);
}Unfortunately, C# and VB do not always agree (e.g. on this API).
There was a problem hiding this comment.
I feel like this method should either go away entirely, or just be an extension method... doesn't seem like it really needs to be in Compilation. Your call :)
There was a problem hiding this comment.
It was moved into Compilation during the API review. There are no extension methods on Compilation in the CodeAnalysis assembly, because such methods can just be placed in the API.
|
Done review pass (iteration 1) |
|
@333fred Do you have any further comments? (Your API question is one that was discussed and agreed at the API design review yesterday). |
|
I like this appraoch a lot :) |
|
test windows_release_unit32_prtest please |
|
@dotnet/roslyn-compiler May I please have a couple of reviews of the implementation of this new compilation API? |
|
@dotnet/roslyn-ide Can I please have a review of the IDE changes in this PR? |
Also add
Fixes Expose IsAssignableTo from SemanticModel #9461
Note that a preliminary review of drafts of this API were in #26719
This is the version of the API approved by CodeAnalysis review group 2018-05-23.
@dotnet/roslyn-compiler Please review.
/cc @dotnet/roslyn-ide for removal of no-longer-needed APIs.