Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 3 additions & 2 deletions src/CodeNav.OutOfProc/Languages/CSharp/Mappers/ClassMapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,9 @@ public static CodeClassItem MapClass(ClassDeclarationSyntax member,
}
}

// Add regions to class
codeItem.Members.AddRange(regions);
// Add regions to class, but skip any that are already nested inside
// a member that maps its own regions (e.g. a nested extension block)
RegionMapper.AddRegionsIfNotPresent(codeItem.Members, regions);

return codeItem;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,10 @@ public static CodeClassItem MapExtensionBlock(ExtensionBlockDeclarationSyntax me
codeItem.Members.Add(memberItem);
}

// Add regions to block
codeItem.Members.AddRange(regions);
// Add regions to block, but skip any that are already nested inside
// a member that maps its own regions
RegionMapper.AddRegionsIfNotPresent(codeItem.Members, regions);


return codeItem;
}
Expand Down
14 changes: 3 additions & 11 deletions src/CodeNav.OutOfProc/Languages/CSharp/Mappers/InterfaceMapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -174,17 +174,9 @@ private static CodeImplementedInterfaceItem MapImplementedInterface(string name,
}
}

// Add regions to interface if they have a region member inside them
if (regions.Any())
{
foreach (var region in regions)
{
if (region?.Members.Any() == true)
{
codeItem.Members.Add(region);
}
}
}
// Add regions to interface, but skip any that are already nested inside
// a member that maps its own regions
RegionMapper.AddRegionsIfNotPresent(codeItem.Members, regions);

return codeItem;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,7 @@ public static CodeNamespaceItem MapNamespace(BaseNamespaceDeclarationSyntax memb
}

// Add regions to namespace if they are not present in any children of the namespace
if (regions.Any())
{
foreach (var region in regions)
{
if (codeItem.Members.Flatten().FilterNull().Any(i => i.Id == region?.Id) == false)
{
codeItem.Members.AddIfNotNull(region);
}
}
}
RegionMapper.AddRegionsIfNotPresent(codeItem.Members, regions);

return codeItem;
}
Expand Down
35 changes: 35 additions & 0 deletions src/CodeNav.OutOfProc/Languages/CSharp/Mappers/RegionMapper.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using CodeNav.OutOfProc.Constants;
using CodeNav.OutOfProc.Extensions;
using CodeNav.OutOfProc.Helpers;
using CodeNav.OutOfProc.Interfaces;
using CodeNav.OutOfProc.Mappers;
Expand All @@ -7,6 +8,7 @@
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Text;
using System.Drawing;

namespace CodeNav.OutOfProc.Languages.CSharp.Mappers;

Expand Down Expand Up @@ -288,4 +290,37 @@ public static bool IsPartOfRegion(IEnumerable<CodeRegionItem> regions, CodeItem

return null;
}

/// <summary>
/// Add regions to the given list of members, but only if a region with the same
/// <see cref="CodeItem.Id"/> is not already present somewhere within those members.
/// </summary>
/// <remarks>
/// A region can be discovered more than once: a container (class, interface,
/// namespace, extension block) scans its entire span for regions, which also
/// picks up regions that live inside a nested member (e.g. a nested class or an
/// extension block) that maps its own regions independently. That nested member
/// already nests its region correctly, so the outer scan's copy is a duplicate
/// (and ends up empty, since none of its members could be matched to it) and
/// should not be added again. Regions that are genuinely empty and not nested
/// inside another member (e.g. an intentionally empty `#region` directly in a
/// class) are still added, since no item with their Id exists elsewhere.
/// </remarks>
/// <param name="members">The list of members regions should be added to</param>
/// <param name="regions">The regions found for this scope</param>
public static void AddRegionsIfNotPresent(List<CodeItem> members, List<CodeRegionItem> regions)
{
foreach (var region in regions)
{
var alreadyPresent = members
.Flatten()
.FilterNull()
.Any(item => item.Id == region?.Id);

if (!alreadyPresent)
{
members.AddIfNotNull(region);
}
}
}
}
14 changes: 14 additions & 0 deletions test/Files/ExtensionBlock/TestExtensionBlockRegion.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
namespace CodeNav.Test.Files.ExtensionBlock;

internal static class TestExtensionBlockRegion
{
extension(object obj)
{
#region Foo
public bool IsNull()
{
return obj is null;
}
#endregion
}
}
31 changes: 30 additions & 1 deletion test/MapperTests/MapperTestExtensionBlock.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
internal class MapperTestExtensionBlock : BaseTest
{
[Test]
public async Task TestInlineBaseClassShouldBeOk()
public async Task ExtensionBlockShouldBeOk()
{
var codeItems = await MapToCodeItems("ExtensionBlock/TestExtensionBlock.cs");

Expand All @@ -18,4 +18,33 @@ public async Task TestInlineBaseClassShouldBeOk()
// Extension block item should have 2 members
Assert.That(extensionBlockItem!.Members, Has.Count.EqualTo(2));
}

[Test]
public async Task RegionInsideExtensionBlockShouldNotBeDuplicated()
{
// Regression test for https://github.com/sboulema/CodeNav/issues/180
var codeItems = await MapToCodeItems("ExtensionBlock/TestExtensionBlockRegion.cs");

var namespaceItem = GetNamespace(codeItems);
var classItem = GetFirstClass(namespaceItem);

// The class should only have the extension block as a member, the region
// inside the extension block should not also show up as a sibling of it
Assert.That(classItem.Members, Has.Count.EqualTo(1));

var extensionBlockItem = GetMemberAtIndex(classItem, 0) as CodeClassItem;

// The extension block should have a single member: the "Foo" region
Assert.That(extensionBlockItem!.Members, Has.Count.EqualTo(1));

var regionItem = GetFirstRegion(extensionBlockItem);

using (Assert.EnterMultipleScope())
{
Assert.That(regionItem.Name, Is.EqualTo("Foo"));

// The region should contain the IsNull method
GetMemberByName<CodeItem>(regionItem, "IsNull");
}
}
}
Loading