Skip to content

Commit

Permalink
Do not return file-local type requested by unmangled metadata name (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
DoctorKrolic authored Apr 26, 2023
1 parent dc47e84 commit bc609ca
Show file tree
Hide file tree
Showing 7 changed files with 195 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@

using System.Linq;
using Microsoft.CodeAnalysis.CodeStyle;
using Microsoft.CodeAnalysis.CSharp.Extensions;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Text;
using Roslyn.Utilities;
using Microsoft.CodeAnalysis.CSharp.Extensions;

namespace Microsoft.CodeAnalysis.CSharp.Analyzers.ConvertProgram
{
Expand All @@ -34,13 +34,8 @@ public static bool CanOfferUseProgramMain(
if (!CanOfferUseProgramMain(option, forAnalyzer))
return false;

// resiliency check for later on. This shouldn't happen but we don't want to crash if we are in a weird
// state where we have top level statements but no 'Program' type.
var programType = compilation.GetBestTypeByMetadataName(WellKnownMemberNames.TopLevelStatementsEntryPointTypeName);
if (programType == null)
return false;

if (programType.GetMembers(WellKnownMemberNames.TopLevelStatementsEntryPointMethodName).FirstOrDefault() is not IMethodSymbol)
// Ensure that top-level method actually exists.
if (compilation.GetTopLevelStatementsMethod() is null)
return false;

return true;
Expand All @@ -52,7 +47,7 @@ private static bool CanOfferUseProgramMain(CodeStyleOption2<bool> option, bool f
var analyzerDisabled = option.Notification.Severity == ReportDiagnostic.Suppress;
var forRefactoring = !forAnalyzer;

// If the user likes Program.Main, then we offer to conver to Program.Main from the diagnostic analyzer.
// If the user likes Program.Main, then we offer to convert to Program.Main from the diagnostic analyzer.
// If the user prefers Top-level-statements then we offer to use Program.Main from the refactoring provider.
// If the analyzer is disabled completely, the refactoring is enabled in both directions.
var canOffer = userPrefersProgramMain == forAnalyzer || (forRefactoring && analyzerDisabled);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@
using System.Diagnostics;
using System.Globalization;
using System.Linq;
using System.Runtime.InteropServices;
using System.Reflection;
using System.Reflection.Metadata;
using System.Reflection.Metadata.Ecma335;
using System.Runtime.InteropServices;
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.DocumentationComments;
using Microsoft.CodeAnalysis.CSharp.Emit;
using Microsoft.CodeAnalysis.PooledObjects;
using Roslyn.Utilities;
using System.Reflection.Metadata.Ecma335;

namespace Microsoft.CodeAnalysis.CSharp.Symbols.Metadata.PE
{
Expand Down Expand Up @@ -394,10 +394,18 @@ internal abstract override bool MangleName
get;
}

internal override FileIdentifier? AssociatedFileIdentifier =>
GetUncommonProperties() is { lazyFilePathChecksum: { IsDefault: false } checksum, lazyDisplayFileName: { } displayFileName }
? new FileIdentifier { FilePathChecksumOpt = checksum, DisplayFilePath = displayFileName }
: null;
internal override FileIdentifier? AssociatedFileIdentifier
{
get
{
// `lazyFilePathChecksum` and `lazyDisplayFileName` of `_lazyUncommonProperties` are initialized in the constructor, not on demand.
// Therefore we can use `_lazyUncommonProperties` directly to avoid additional computations.
// Also important, that computing full uncommon properties here may lead to stack overflow if there is a circular dependency between types in the metadata.
return _lazyUncommonProperties is { lazyFilePathChecksum: { IsDefault: false } checksum, lazyDisplayFileName: { } displayFileName }
? new FileIdentifier { FilePathChecksumOpt = checksum, DisplayFilePath = displayFileName }
: null;
}
}

internal abstract int MetadataArity
{
Expand Down
14 changes: 8 additions & 6 deletions src/Compilers/CSharp/Portable/Symbols/NamespaceOrTypeSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@
using System.Collections.Immutable;
using System.Diagnostics;
using System.Linq;
using Microsoft.CodeAnalysis.CSharp.Symbols.Metadata.PE;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Symbols;
using Roslyn.Utilities;

Expand Down Expand Up @@ -271,9 +269,11 @@ public virtual ImmutableArray<NamedTypeSymbol> GetTypeMembers(string name, int a

foreach (var named in namespaceOrTypeMembers)
{
if (emittedTypeName.InferredArity == named.Arity && named.MangleName)
if (emittedTypeName.InferredArity == named.Arity &&
named.MangleName &&
named.MetadataName == emittedTypeName.TypeName)
{
if ((object?)namedType != null)
if (namedType is not null)
{
namedType = null;
break;
Expand Down Expand Up @@ -317,9 +317,11 @@ public virtual ImmutableArray<NamedTypeSymbol> GetTypeMembers(string name, int a

foreach (var named in namespaceOrTypeMembers)
{
if (!named.MangleName && (forcedArity == -1 || forcedArity == named.Arity))
if (!named.MangleName &&
(forcedArity == -1 || forcedArity == named.Arity) &&
named.MetadataName == emittedTypeName.TypeName)
{
if ((object?)namedType != null)
if (namedType is not null)
{
namedType = null;
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Microsoft.CodeAnalysis.CSharp.Symbols.Retargeting;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
using Microsoft.CodeAnalysis.Emit;
using Microsoft.CodeAnalysis.Test.Utilities;
using Roslyn.Test.Utilities;
using Xunit;
Expand Down Expand Up @@ -4762,5 +4763,67 @@ static void verify(CSharpCompilation comp, string expectedAssemblyName)
Assert.Equal(expectedAssemblyName, comp.GetTypeByMetadataName("System.Runtime.CompilerServices.IsExternalInit").ContainingAssembly.Name);
}
}

[Theory, WorkItem(67079, "https://github.com/dotnet/roslyn/issues/67079")]
[CombinatorialData]
public void DoNotPickTypeFromSourceWithFileModifier(bool useCompilationReference)
{
var corlib_cs = """
namespace System
{
public class Object { }
public struct Int32 { }
public struct Boolean { }
public class String { }
public class ValueType { }
public struct Void { }
public class Attribute { }
public class AttributeUsageAttribute : Attribute
{
public AttributeUsageAttribute(AttributeTargets t) { }
public bool AllowMultiple { get; set; }
public bool Inherited { get; set; }
}
public struct Enum { }
public enum AttributeTargets { }
}
""";

var source = """
namespace System.Runtime.CompilerServices
{
file class IsExternalInit {}
}
public class C
{
public string Property { get; init; }
}
""";

var corlibWithoutIsExternalInitRef = AsReference(CreateEmptyCompilation(corlib_cs), useCompilationReference);
var corlibWithIsExternalInitRef = AsReference(CreateEmptyCompilation(corlib_cs + IsExternalInitTypeDefinition), useCompilationReference);
var emitOptions = EmitOptions.Default.WithRuntimeMetadataVersion("0.0.0.0");

{
// proper type in corlib and file type in source
var comp = CreateEmptyCompilation(source, references: new[] { corlibWithIsExternalInitRef });
comp.VerifyEmitDiagnostics(emitOptions);
var modifier = ((SourcePropertySymbol)comp.GlobalNamespace.GetMember("C.Property")).SetMethod.ReturnTypeWithAnnotations.CustomModifiers.Single();
Assert.False(modifier.Modifier.IsFileLocal);
}

{
// no type in corlib and file type in source
var comp = CreateEmptyCompilation(source, references: new[] { corlibWithoutIsExternalInitRef });
comp.VerifyEmitDiagnostics(emitOptions,
// (8,35): error CS018: Predefined type 'System.Runtime.CompilerServices.IsExternalInit' is not defined or imported
// public int Property { get; init; }
Diagnostic(ErrorCode.ERR_PredefinedTypeNotFound, "init").WithArguments("System.Runtime.CompilerServices.IsExternalInit").WithLocation(8, 35)
);
var modifier = ((SourcePropertySymbol)comp.GlobalNamespace.GetMember("C.Property")).SetMethod.ReturnTypeWithAnnotations.CustomModifiers.Single();
Assert.False(modifier.Modifier.IsFileLocal);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable disable

using System;
using System.Collections.Generic;
using System.Linq;
Expand Down Expand Up @@ -3576,6 +3578,8 @@ file class C { }
Assert.Null(comp.GetTypeByMetadataName("<>F1__C"));
Assert.Null(comp.GetTypeByMetadataName("F0__C"));
Assert.Null(comp.GetTypeByMetadataName("<file>F0__C"));
Assert.Null(comp.GetTypeByMetadataName("C"));
Assert.Null(comp.GetTypeByMetadataName("C`1"));

// from metadata
var comp2 = CreateCompilation("", references: new[] { comp.EmitToImageReference() });
Expand All @@ -3585,6 +3589,8 @@ file class C { }

var metadataType = comp2.GetTypeByMetadataName("<>FE3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855__C");
Assert.Equal(metadataMember, metadataType);

Assert.Null(comp2.GetTypeByMetadataName("C"));
}

[Fact]
Expand All @@ -3603,6 +3609,8 @@ file class C<T> { }
var sourceType = comp.GetTypeByMetadataName("<>FE3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855__C`1");
Assert.Equal(sourceMember, sourceType);
Assert.Null(comp.GetTypeByMetadataName("<>FE3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855__C"));
Assert.Null(comp.GetTypeByMetadataName("C"));
Assert.Null(comp.GetTypeByMetadataName("C`1"));

// from metadata
var comp2 = CreateCompilation("", references: new[] { comp.EmitToImageReference() });
Expand All @@ -3613,6 +3621,8 @@ file class C<T> { }

var metadataType = comp2.GetTypeByMetadataName("<>FE3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855__C`1");
Assert.Equal(metadataMember, metadataType);

Assert.Null(comp2.GetTypeByMetadataName("C`1"));
}

[Fact]
Expand All @@ -3639,6 +3649,7 @@ file class C { } // 1
// However, since we don't actually support nested file types, we don't think we need the API to do the additional lookup
// when the requested type is nested, and so we end up giving a null here.
Assert.Null(sourceType);
Assert.Null(comp.GetTypeByMetadataName("Outer.C"));
}

[Fact]
Expand All @@ -3661,6 +3672,11 @@ class C { }
var sourceType = comp.GetTypeByMetadataName("<file1>F96B1D9CB33A43D51528FE81EDAFE5AE31358FE749929AC76B76C64B60DEF129D__C");
Assert.Equal(sourceMember, sourceType);

var sourceTypeCByMetadataName = comp.GetTypeByMetadataName("C");
Assert.NotNull(sourceTypeCByMetadataName);
Assert.Equal("C", sourceTypeCByMetadataName.MetadataName);
Assert.False(sourceTypeCByMetadataName is SourceMemberContainerTypeSymbol { IsFileLocal: true });

// from metadata
var comp2 = CreateCompilation("", references: new[] { comp.EmitToImageReference() });
comp2.VerifyDiagnostics();
Expand All @@ -3670,6 +3686,10 @@ class C { }

var metadataType = comp2.GetTypeByMetadataName("<file1>F96B1D9CB33A43D51528FE81EDAFE5AE31358FE749929AC76B76C64B60DEF129D__C");
Assert.Equal(metadataMember, metadataType);

var metadataTypeCByMetadataName = comp2.GetTypeByMetadataName("C");
Assert.NotNull(metadataTypeCByMetadataName);
Assert.Equal("C", metadataTypeCByMetadataName.MetadataName);
}

[CombinatorialData]
Expand All @@ -3694,12 +3714,15 @@ file class C { }
const string metadataName = "<>FE3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855__C";
var sourceType = comp.GetTypeByMetadataName(metadataName);
Assert.Null(sourceType);
Assert.Null(comp.GetTypeByMetadataName("C"));

var types = comp.GetTypesByMetadataName(metadataName);
Assert.Equal(2, types.Length);
Assert.Equal(firstIsMetadataReference ? "C@<tree 0>" : "C@<unknown>", types[0].ToTestDisplayString());
Assert.Equal(secondIsMetadataReference ? "C@<tree 0>" : "C@<unknown>", types[1].ToTestDisplayString());
Assert.NotEqual(types[0], types[1]);

Assert.Empty(comp.GetTypesByMetadataName("C"));
}

[Fact]
Expand All @@ -3720,9 +3743,13 @@ file class C { }
var sourceType = ((Compilation)comp).GetTypeByMetadataName(metadataName);
Assert.Equal("C@<tree 0>", sourceType.ToTestDisplayString());

Assert.Null(((Compilation)comp).GetTypeByMetadataName("C"));

var types = comp.GetTypesByMetadataName(metadataName);
Assert.Equal(1, types.Length);
Assert.Same(sourceType, types[0]);

Assert.Empty(comp.GetTypesByMetadataName("C"));
}

[Fact]
Expand Down Expand Up @@ -3750,6 +3777,51 @@ file class C { }
Assert.Same(sourceType, types[0]);
}

[Fact]
public void GetTypeByMetadataName_08()
{
var source1 = """
file class C { public static void M() { } }
""";

var comp = CreateCompilation(source1, targetFramework: TargetFramework.Mscorlib40);
comp.VerifyDiagnostics();

const string metadataName = "<>FE3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855__C";

var member = comp.GetMember<NamedTypeSymbol>("C");
Assert.Equal(metadataName, member.MetadataName);

Assert.Null(comp.GetTypeByMetadataName("C"));
Assert.Equal(member, comp.GetTypeByMetadataName(metadataName));

var source2 = """
class C2
{
void M()
{
C.M();
}
}
""";

var comp2 = CreateCompilation(source2, references: new[] { comp.ToMetadataReference() }, targetFramework: TargetFramework.Mscorlib45);
comp2.VerifyDiagnostics(
// (5,9): error CS0103: The name 'C' does not exist in the current context
// C.M();
Diagnostic(ErrorCode.ERR_NameNotInContext, "C").WithArguments("C").WithLocation(5, 9)
);

Assert.NotEqual(comp.Assembly.CorLibrary, comp2.Assembly.CorLibrary);

var retargeted = comp2.GetMember<NamedTypeSymbol>("C");
Assert.IsType<RetargetingNamedTypeSymbol>(retargeted);
Assert.Equal(metadataName, retargeted.MetadataName);

Assert.Null(comp2.GetTypeByMetadataName("C"));
Assert.Equal(retargeted, comp2.GetTypeByMetadataName(metadataName));
}

[Fact]
public void AssociatedSyntaxTree_01()
{
Expand Down
Loading

0 comments on commit bc609ca

Please sign in to comment.