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.

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

Fix missing duplicate diagnostic for enum declarations #68967

Merged
merged 2 commits into from
Jul 18, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -847,18 +847,18 @@ internal override ManagedKind GetManagedKind(ref CompoundUseSiteInfo<AssemblySym

internal bool IsUnsafe => HasFlag(DeclarationModifiers.Unsafe);

internal SyntaxTree AssociatedSyntaxTree => declaration.Declarations[0].Location.SourceTree;
private SyntaxTree? AssociatedSyntaxTree => IsFileLocal ? declaration.Declarations[0].Location.SourceTree : null;
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved

internal sealed override FileIdentifier? AssociatedFileIdentifier
{
get
{
if (!IsFileLocal)
if (AssociatedSyntaxTree is not SyntaxTree syntaxTree)
{
return null;
}

return FileIdentifier.Create(AssociatedSyntaxTree);
return FileIdentifier.Create(syntaxTree);
}
}

Expand Down Expand Up @@ -1309,6 +1309,11 @@ private Dictionary<ReadOnlyMemory<char>, ImmutableArray<NamedTypeSymbol>> MakeTy
var conflictDict = new Dictionary<(string name, int arity, SyntaxTree? syntaxTree), SourceNamedTypeSymbol>();
try
{
// Declarations which can be merged into a single type symbol have already been merged at this phase.
// Merging behaves the same in either presence or absence of 'partial' modifiers.
// However, type declarations which can never be partial won't merge, e.g. 'enum',
// and type declarations with different kinds, e.g. 'class' and 'struct' will never merge.
// Now we want to figure out if declarations which didn't merge have name conflicts.
foreach (var childDeclaration in declaration.Children)
{
var t = new SourceNamedTypeSymbol(this, childDeclaration, diagnostics);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,69 @@ void symbolValidator(ModuleSymbol symbol)
}
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67258")]
public void NonFileLocalClass_Duplicate()
{
var source = @"
public class D { }

public partial class C
{
public class D { }
}
";
var comp = CreateCompilation(new[] { source, source });
comp.VerifyEmitDiagnostics(
// 1.cs(2,14): error CS0101: The namespace '<global namespace>' already contains a definition for 'D'
// public class D { }
Diagnostic(ErrorCode.ERR_DuplicateNameInNS, "D").WithArguments("D", "<global namespace>").WithLocation(2, 14),
// 1.cs(6,18): error CS0102: The type 'C' already contains a definition for 'D'
// public class D { }
Diagnostic(ErrorCode.ERR_DuplicateNameInClass, "D").WithArguments("C", "D").WithLocation(6, 18));
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67258")]
public void NonFileLocalEnum_Duplicate()
{
var source = @"
public enum E { }

public partial class C
{
public enum E { }
}
";
var comp = CreateCompilation(new[] { source, source });
comp.VerifyEmitDiagnostics(
// 1.cs(2,13): error CS0101: The namespace '<global namespace>' already contains a definition for 'E'
// public enum E { }
Diagnostic(ErrorCode.ERR_DuplicateNameInNS, "E").WithArguments("E", "<global namespace>").WithLocation(2, 13),
// 1.cs(6,17): error CS0102: The type 'C' already contains a definition for 'E'
// public enum E { }
Diagnostic(ErrorCode.ERR_DuplicateNameInClass, "E").WithArguments("C", "E").WithLocation(6, 17));
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67258")]
public void NonFileLocalDelegate_Duplicate()
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved
{
var source = @"
public delegate void D();

public partial class C
{
public delegate void D();
}
";
var comp = CreateCompilation(new[] { source, source });
comp.VerifyEmitDiagnostics(
// 1.cs(2,22): error CS0101: The namespace '<global namespace>' already contains a definition for 'D'
// public delegate void D();
Diagnostic(ErrorCode.ERR_DuplicateNameInNS, "D").WithArguments("D", "<global namespace>").WithLocation(2, 22),
// 1.cs(6,26): error CS0102: The type 'C' already contains a definition for 'D'
// public delegate void D();
Diagnostic(ErrorCode.ERR_DuplicateNameInClass, "D").WithArguments("C", "D").WithLocation(6, 26));
}
Copy link
Member

@jcouv jcouv Jul 11, 2023

Choose a reason for hiding this comment

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

Please include the tests I documented in the issue: #67258 (comment) #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I included the one marked TODO2_2, did you also want the scenario marked TODO2 included?

Copy link
Member

Choose a reason for hiding this comment

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

That's good. Thanks

Copy link
Member

Choose a reason for hiding this comment

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

Which test did that end up being? I can't find that scenario (just a partial containing an enum, without any file-local modifier, included twice in the compilation)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From #67258 (comment), TODO2_2 is represented by NonFileLocalEnum_Duplicate. TODO2 is not included.


[Fact]
public void OtherFileUse()
{
Expand Down