Skip to content
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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Dec 14, 2021
Merged

Nullable enable GetTypeByMetadataName #58317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dcd5abb
Nullable enable GetTypeByMetadataName APIs.
333fred Dec 14, 2021
3c69b1f
Fix other nullable warning.
333fred Dec 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

@AlekseyTs AlekseyTs Dec 14, 2021 •
edited by 333fred
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (result is null)

Is this a "style-only" change? Please revert if so. #WontFix

Copy link
Member

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.

Copy link
Member Author

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.

{
var errorType = new ExtendedErrorTypeSymbol(this, type.Name, 0, CreateReflectionTypeNotFoundError(type));
diagnostics.Add(errorType.ErrorInfo, NoLocation.Singleton);
Expand Down Expand Up @@ -1590,9 +1590,9 @@ protected override ITypeSymbol? CommonScriptGlobalsType
{
if (HostObjectType != null && _lazyHostObjectTypeSymbol is null)
{
TypeSymbol symbol = Assembly.GetTypeByReflectionType(HostObjectType, includeReferences: true);
TypeSymbol? symbol = Assembly.GetTypeByReflectionType(HostObjectType, includeReferences: true);

if ((object)symbol == null)
if (symbol is null)
Copy link
Contributor

@AlekseyTs AlekseyTs Dec 14, 2021 •
edited by 333fred
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

symbol is null

Is this a "style-only" change? #WontFix

{
MetadataTypeName mdName = MetadataTypeName.FromNamespaceAndTypeName(HostObjectType.Namespace ?? String.Empty,
HostObjectType.Name,
Expand Down
65 changes: 34 additions & 31 deletions src/Compilers/CSharp/Portable/Symbols/AssemblySymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Reflection;
using System.Reflection.PortableExecutable;
Expand Down Expand Up @@ -565,12 +566,13 @@ internal NamedTypeSymbol GetPrimitiveType(Microsoft.Cci.PrimitiveTypeCode type)
return GetSpecialType(SpecialTypes.GetTypeFromMetadataName(type));
}

#nullable enable
/// <summary>
/// Lookup a type within the assembly using the canonical CLR metadata name of the type.
/// </summary>
/// <param name="fullyQualifiedMetadataName">Type name.</param>
/// <returns>Symbol for the type or null if type cannot be found or is ambiguous. </returns>
public NamedTypeSymbol GetTypeByMetadataName(string fullyQualifiedMetadataName)
public NamedTypeSymbol? GetTypeByMetadataName(string fullyQualifiedMetadataName)
{
if (fullyQualifiedMetadataName == null)
{
Expand Down Expand Up @@ -606,16 +608,16 @@ public NamedTypeSymbol GetTypeByMetadataName(string fullyQualifiedMetadataName)
/// In cases a type could not be found because of ambiguity, we return two of the candidates that caused the ambiguity.
/// </param>
/// <returns>Null if the type can't be found.</returns>
internal NamedTypeSymbol GetTypeByMetadataName(
internal NamedTypeSymbol? GetTypeByMetadataName(
string metadataName,
bool includeReferences,
bool isWellKnownType,
out (AssemblySymbol, AssemblySymbol) conflicts,
bool useCLSCompliantNameArityEncoding = false,
DiagnosticBag warnings = null,
DiagnosticBag? warnings = null,
bool ignoreCorLibraryDuplicatedTypes = false)
{
NamedTypeSymbol type;
NamedTypeSymbol? type;
MetadataTypeName mdName;

if (metadataName.IndexOf('+') >= 0)
Expand All @@ -626,7 +628,7 @@ internal NamedTypeSymbol GetTypeByMetadataName(
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++)
Copy link
Contributor

@AlekseyTs AlekseyTs Dec 14, 2021 •
edited by 333fred
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type is object

Is this a "style-only" change? #WontFix

{
mdName = MetadataTypeName.FromTypeName(parts[i]);
NamedTypeSymbol temp = type.LookupMetadataType(ref mdName);
Expand All @@ -640,7 +642,7 @@ internal NamedTypeSymbol GetTypeByMetadataName(
conflicts: out conflicts, warnings: warnings, ignoreCorLibraryDuplicatedTypes: ignoreCorLibraryDuplicatedTypes);
}

return ((object)type == null || type.IsErrorType()) ? null : type;
return (type is null || type.IsErrorType()) ? null : type;
Copy link
Contributor

@AlekseyTs AlekseyTs Dec 14, 2021 •
edited by 333fred
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type is null

Is this a "style-only" change? #WontFix

}

private static readonly char[] s_nestedTypeNameSeparators = new char[] { '+' };
Expand All @@ -652,7 +654,7 @@ internal NamedTypeSymbol GetTypeByMetadataName(
/// <param name="type">The type to resolve.</param>
/// <param name="includeReferences">Use referenced assemblies for resolution.</param>
/// <returns>The resolved symbol if successful or null on failure.</returns>
internal TypeSymbol GetTypeByReflectionType(Type type, bool includeReferences)
internal TypeSymbol? GetTypeByReflectionType(Type type, bool includeReferences)
{
System.Reflection.TypeInfo typeInfo = type.GetTypeInfo();

Expand All @@ -663,8 +665,8 @@ internal TypeSymbol GetTypeByReflectionType(Type type, bool includeReferences)

if (typeInfo.IsArray)
{
TypeSymbol symbol = GetTypeByReflectionType(typeInfo.GetElementType(), includeReferences);
if ((object)symbol == null)
TypeSymbol? symbol = GetTypeByReflectionType(typeInfo.GetElementType()!, includeReferences);
if (symbol is null)
{
return null;
}
Expand All @@ -675,8 +677,8 @@ internal TypeSymbol GetTypeByReflectionType(Type type, bool includeReferences)
}
else if (typeInfo.IsPointer)
{
TypeSymbol symbol = GetTypeByReflectionType(typeInfo.GetElementType(), includeReferences);
if ((object)symbol == null)
TypeSymbol? symbol = GetTypeByReflectionType(typeInfo.GetElementType()!, includeReferences);
if (symbol is null)
{
return null;
}
Expand Down Expand Up @@ -707,8 +709,8 @@ internal TypeSymbol GetTypeByReflectionType(Type type, bool includeReferences)
}

int i = nestedTypes.Count - 1;
var symbol = (NamedTypeSymbol)GetTypeByReflectionType(nestedTypes[i].AsType(), includeReferences);
if ((object)symbol == null)
var symbol = (NamedTypeSymbol?)GetTypeByReflectionType(nestedTypes[i].AsType(), includeReferences);
if (symbol is null)
{
return null;
}
Expand All @@ -719,13 +721,13 @@ internal TypeSymbol GetTypeByReflectionType(Type type, bool includeReferences)
MetadataTypeName mdName = MetadataTypeName.FromTypeName(nestedTypes[i].Name, forcedArity: forcedArity);

symbol = symbol.LookupMetadataType(ref mdName);
if ((object)symbol == null || symbol.IsErrorType())
if (symbol is null || symbol.IsErrorType())
{
return null;
}

symbol = ApplyGenericArguments(symbol, genericArguments, ref typeArgumentIndex, includeReferences);
if ((object)symbol == null)
if (symbol is null)
{
return null;
}
Expand All @@ -744,9 +746,9 @@ internal TypeSymbol GetTypeByReflectionType(Type type, bool includeReferences)
typeInfo.Name,
forcedArity: typeInfo.GenericTypeArguments.Length);

NamedTypeSymbol symbol = GetTopLevelTypeByMetadataName(ref mdName, assemblyId, includeReferences, isWellKnownType: false, conflicts: out var _);
NamedTypeSymbol? symbol = GetTopLevelTypeByMetadataName(ref mdName, assemblyId, includeReferences, isWellKnownType: false, conflicts: out var _);

if ((object)symbol == null || symbol.IsErrorType())
if (symbol is null || symbol.IsErrorType())
{
return null;
}
Expand All @@ -759,7 +761,7 @@ internal TypeSymbol GetTypeByReflectionType(Type type, bool includeReferences)
}
}

private NamedTypeSymbol ApplyGenericArguments(NamedTypeSymbol symbol, Type[] typeArguments, ref int currentTypeArgument, bool includeReferences)
private NamedTypeSymbol? ApplyGenericArguments(NamedTypeSymbol symbol, Type[] typeArguments, ref int currentTypeArgument, bool includeReferences)
{
int remainingTypeArguments = typeArguments.Length - currentTypeArgument;

Expand All @@ -776,7 +778,7 @@ private NamedTypeSymbol ApplyGenericArguments(NamedTypeSymbol symbol, Type[] typ
for (int i = 0; i < length; i++)
{
var argSymbol = GetTypeByReflectionType(typeArguments[currentTypeArgument++], includeReferences);
if ((object)argSymbol == null)
if (argSymbol is null)
{
return null;
}
Expand All @@ -786,13 +788,13 @@ private NamedTypeSymbol ApplyGenericArguments(NamedTypeSymbol symbol, Type[] typ
return symbol.ConstructIfGeneric(typeArgumentSymbols.ToImmutableAndFree());
}

internal NamedTypeSymbol GetTopLevelTypeByMetadataName(
internal NamedTypeSymbol? GetTopLevelTypeByMetadataName(
ref MetadataTypeName metadataName,
AssemblyIdentity assemblyOpt,
AssemblyIdentity? assemblyOpt,
bool includeReferences,
bool isWellKnownType,
out (AssemblySymbol, AssemblySymbol) conflicts,
DiagnosticBag warnings = null, // this is set to collect ambiguity warning for well-known types before C# 7
DiagnosticBag? warnings = null, // this is set to collect ambiguity warning for well-known types before C# 7
bool ignoreCorLibraryDuplicatedTypes = false)
{
// Type from this assembly always wins.
Expand All @@ -805,7 +807,7 @@ internal NamedTypeSymbol GetTopLevelTypeByMetadataName(
Debug.Assert(warnings is null || isWellKnownType);

conflicts = default;
NamedTypeSymbol result;
NamedTypeSymbol? result;

// First try this assembly
result = GetTopLevelTypeByMetadataName(this, ref metadataName, assemblyOpt);
Expand All @@ -816,7 +818,7 @@ internal NamedTypeSymbol GetTopLevelTypeByMetadataName(
}

// ignore any types of the same name that might be in referenced assemblies (prefer the current assembly):
if ((object)result != null || !includeReferences)
if (result is object || !includeReferences)
{
return result;
}
Expand All @@ -829,7 +831,7 @@ internal NamedTypeSymbol GetTopLevelTypeByMetadataName(
!CorLibrary.IsMissing &&
!isWellKnownTypeBeforeCSharp7 && !ignoreCorLibraryDuplicatedTypes)
{
NamedTypeSymbol corLibCandidate = GetTopLevelTypeByMetadataName(CorLibrary, ref metadataName, assemblyOpt);
NamedTypeSymbol? corLibCandidate = GetTopLevelTypeByMetadataName(CorLibrary, ref metadataName, assemblyOpt);
skipCorLibrary = true;

if (isValidCandidate(corLibCandidate, isWellKnownType))
Expand Down Expand Up @@ -863,7 +865,7 @@ internal NamedTypeSymbol GetTopLevelTypeByMetadataName(
continue;
}

NamedTypeSymbol candidate = GetTopLevelTypeByMetadataName(assembly, ref metadataName, assemblyOpt);
NamedTypeSymbol? candidate = GetTopLevelTypeByMetadataName(assembly, ref metadataName, assemblyOpt);

if (!isValidCandidate(candidate, isWellKnownType))
{
Expand All @@ -872,7 +874,7 @@ internal NamedTypeSymbol GetTopLevelTypeByMetadataName(

Debug.Assert(!TypeSymbol.Equals(candidate, result, TypeCompareKind.ConsiderEverything));

if ((object)result != null)
if (result is object)
{
// duplicate
if (ignoreCorLibraryDuplicatedTypes)
Expand Down Expand Up @@ -910,7 +912,7 @@ internal NamedTypeSymbol GetTopLevelTypeByMetadataName(
assemblies.Free();
return result;

bool isValidCandidate(NamedTypeSymbol candidate, bool isWellKnownType)
bool isValidCandidate([NotNullWhen(true)] NamedTypeSymbol? candidate, bool isWellKnownType)
{
return candidate is not null
&& (!isWellKnownType || IsValidWellKnownType(candidate))
Expand All @@ -923,9 +925,9 @@ private bool IsInCorLib(NamedTypeSymbol type)
return (object)type.ContainingAssembly == CorLibrary;
}

private bool IsValidWellKnownType(NamedTypeSymbol result)
private bool IsValidWellKnownType(NamedTypeSymbol? result)
{
if ((object)result == null || result.TypeKind == TypeKind.Error)
if (result is null || result.TypeKind == TypeKind.Error)
{
return false;
}
Expand All @@ -936,7 +938,7 @@ private bool IsValidWellKnownType(NamedTypeSymbol result)
return result.DeclaredAccessibility == Accessibility.Public || IsSymbolAccessible(result, this);
}

private static NamedTypeSymbol GetTopLevelTypeByMetadataName(AssemblySymbol assembly, ref MetadataTypeName metadataName, AssemblyIdentity assemblyOpt)
private static NamedTypeSymbol? GetTopLevelTypeByMetadataName(AssemblySymbol assembly, ref MetadataTypeName metadataName, AssemblyIdentity? assemblyOpt)
{
var result = assembly.LookupTopLevelMetadataType(ref metadataName, digThroughForwardedTypes: false);
if (!IsAcceptableMatchForGetTypeByMetadataName(result))
Expand All @@ -956,6 +958,7 @@ private static bool IsAcceptableMatchForGetTypeByMetadataName(NamedTypeSymbol ca
{
return candidate.Kind != SymbolKind.ErrorType || !(candidate is MissingMetadataTypeSymbol);
}
#nullable disable

/// <summary>
/// Lookup member declaration in predefined CorLib type in this Assembly. Only valid if this
Expand Down
4 changes: 3 additions & 1 deletion src/Compilers/CSharp/Portable/Symbols/PublicModel/AssemblySymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,12 @@ bool IAssemblySymbol.GivesAccessTo(IAssemblySymbol assemblyWantingAccess)
return false;
}

INamedTypeSymbol IAssemblySymbol.GetTypeByMetadataName(string metadataName)
#nullable enable
INamedTypeSymbol? IAssemblySymbol.GetTypeByMetadataName(string metadataName)
{
return UnderlyingAssemblySymbol.GetTypeByMetadataName(metadataName).GetPublicSymbol();
}
#nullable disable

#region ISymbol Members

Expand Down