diff --git a/src/CodeNav.OutOfProc/Languages/CSharp/Mappers/ClassMapper.cs b/src/CodeNav.OutOfProc/Languages/CSharp/Mappers/ClassMapper.cs index 9da7c84..2461f0b 100644 --- a/src/CodeNav.OutOfProc/Languages/CSharp/Mappers/ClassMapper.cs +++ b/src/CodeNav.OutOfProc/Languages/CSharp/Mappers/ClassMapper.cs @@ -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; } diff --git a/src/CodeNav.OutOfProc/Languages/CSharp/Mappers/ExtensionBlockMapper.cs b/src/CodeNav.OutOfProc/Languages/CSharp/Mappers/ExtensionBlockMapper.cs index 9a02302..43ed101 100644 --- a/src/CodeNav.OutOfProc/Languages/CSharp/Mappers/ExtensionBlockMapper.cs +++ b/src/CodeNav.OutOfProc/Languages/CSharp/Mappers/ExtensionBlockMapper.cs @@ -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; } diff --git a/src/CodeNav.OutOfProc/Languages/CSharp/Mappers/InterfaceMapper.cs b/src/CodeNav.OutOfProc/Languages/CSharp/Mappers/InterfaceMapper.cs index 438f96d..4fa1a2c 100644 --- a/src/CodeNav.OutOfProc/Languages/CSharp/Mappers/InterfaceMapper.cs +++ b/src/CodeNav.OutOfProc/Languages/CSharp/Mappers/InterfaceMapper.cs @@ -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; } diff --git a/src/CodeNav.OutOfProc/Languages/CSharp/Mappers/NamespaceMapper.cs b/src/CodeNav.OutOfProc/Languages/CSharp/Mappers/NamespaceMapper.cs index 6d3127f..0e4d9f9 100644 --- a/src/CodeNav.OutOfProc/Languages/CSharp/Mappers/NamespaceMapper.cs +++ b/src/CodeNav.OutOfProc/Languages/CSharp/Mappers/NamespaceMapper.cs @@ -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; } diff --git a/src/CodeNav.OutOfProc/Languages/CSharp/Mappers/RegionMapper.cs b/src/CodeNav.OutOfProc/Languages/CSharp/Mappers/RegionMapper.cs index 686c4fc..9fa8ffa 100644 --- a/src/CodeNav.OutOfProc/Languages/CSharp/Mappers/RegionMapper.cs +++ b/src/CodeNav.OutOfProc/Languages/CSharp/Mappers/RegionMapper.cs @@ -1,4 +1,5 @@ using CodeNav.OutOfProc.Constants; +using CodeNav.OutOfProc.Extensions; using CodeNav.OutOfProc.Helpers; using CodeNav.OutOfProc.Interfaces; using CodeNav.OutOfProc.Mappers; @@ -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; @@ -288,4 +290,37 @@ public static bool IsPartOfRegion(IEnumerable regions, CodeItem return null; } + + /// + /// Add regions to the given list of members, but only if a region with the same + /// is not already present somewhere within those members. + /// + /// + /// 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. + /// + /// The list of members regions should be added to + /// The regions found for this scope + public static void AddRegionsIfNotPresent(List members, List regions) + { + foreach (var region in regions) + { + var alreadyPresent = members + .Flatten() + .FilterNull() + .Any(item => item.Id == region?.Id); + + if (!alreadyPresent) + { + members.AddIfNotNull(region); + } + } + } } diff --git a/test/Files/ExtensionBlock/TestExtensionBlockRegion.cs b/test/Files/ExtensionBlock/TestExtensionBlockRegion.cs new file mode 100644 index 0000000..08172e1 --- /dev/null +++ b/test/Files/ExtensionBlock/TestExtensionBlockRegion.cs @@ -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 + } +} diff --git a/test/MapperTests/MapperTestExtensionBlock.cs b/test/MapperTests/MapperTestExtensionBlock.cs index 64119da..83fc06b 100644 --- a/test/MapperTests/MapperTestExtensionBlock.cs +++ b/test/MapperTests/MapperTestExtensionBlock.cs @@ -4,7 +4,7 @@ internal class MapperTestExtensionBlock : BaseTest { [Test] - public async Task TestInlineBaseClassShouldBeOk() + public async Task ExtensionBlockShouldBeOk() { var codeItems = await MapToCodeItems("ExtensionBlock/TestExtensionBlock.cs"); @@ -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(regionItem, "IsNull"); + } + } }