-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Nullable enable GetTypeByMetadataName #58317
Nullable enable GetTypeByMetadataName #58317
Conversation
@dotnet/roslyn-compiler for a fairly mechanical review. No code semantics should be changed by this PR. |
acbff46
to
dcd5abb
Compare
@dotnet/roslyn-compiler for a second review. |
Looks like some usages might need updating? |
@dotnet/roslyn-compiler for a second review. |
@@ -1562,7 +1562,7 @@ internal override ISymbolInternal CommonGetSpecialTypeMember(SpecialMember speci | |||
internal TypeSymbol GetTypeByReflectionType(Type type, BindingDiagnosticBag diagnostics) | |||
{ | |||
var result = Assembly.GetTypeByReflectionType(type, includeReferences: true); | |||
if ((object)result == null) | |||
if (result is null) |
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.
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.
A change of some sort is necessary to (object?)
or use is
. Personally, I'm happy with the latter.
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.
As Julien said, these lines need to change in some way to avoid the nullable warning for casting to object
. Given that, I used the style of null check I prefer.
|
||
if ((object)symbol == null) | ||
if (symbol is null) |
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.
@@ -626,7 +628,7 @@ public NamedTypeSymbol GetTypeByMetadataName(string fullyQualifiedMetadataName) | |||
type = GetTopLevelTypeByMetadataName(ref mdName, assemblyOpt: null, includeReferences: includeReferences, isWellKnownType: isWellKnownType, | |||
conflicts: out conflicts, warnings: warnings, ignoreCorLibraryDuplicatedTypes: ignoreCorLibraryDuplicatedTypes); | |||
|
|||
for (int i = 1; (object)type != null && !type.IsErrorType() && i < parts.Length; i++) | |||
for (int i = 1; type is object && !type.IsErrorType() && i < parts.Length; i++) |
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.
@@ -640,7 +642,7 @@ public NamedTypeSymbol GetTypeByMetadataName(string fullyQualifiedMetadataName) | |||
conflicts: out conflicts, warnings: warnings, ignoreCorLibraryDuplicatedTypes: ignoreCorLibraryDuplicatedTypes); | |||
} | |||
|
|||
return ((object)type == null || type.IsErrorType()) ? null : type; | |||
return (type is null || type.IsErrorType()) ? null : type; |
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.
@333fred Please revert style-only changes. |
…rations * upstream/main: (87 commits) Add support for nullable analysis in interpolated string handler constructors (dotnet#57780) Record list-patterns and newlines in interpolations as done (dotnet#58250) Swithc to acquiring the component model on a BG thread. Fix failure to propagate cancellation token [main] Update dependencies from dotnet/arcade (dotnet#58327) Change PROTOTYPE to issue reference (dotnet#58336) Resolve PROTOTYPE comments in param null checking (dotnet#58324) Address various minor param-nullchecking issues (dotnet#58321) Nullable enable GetTypeByMetadataName (dotnet#58317) Support CodeClass2.Parts returning parts in source generated files Allow the FileCodeModel.Parent to be null Ensure the CodeModel tests are using VisualStudioWorkspaceImpl Fix the bad words in TestDeepAlternation Fix regression in Equals/GetHashCode of LambdaSymbol. (dotnet#58247) Support "Enable nullable reference types" from disable keyword Support "Enable nullable reference types" from restore keyword Support "Enable nullable reference types" on the entire directive Add comment Remove descriptor Update src/Workspaces/Remote/ServiceHub/Services/SemanticClassification/RemoteSemanticClassificationService.cs ...
In preparation for working on #57802, I nullable-enabled the implementation of GetTypeByMetadataName in C# as I was reading through it to understand how it currently works.