Skip to content

Commit 6f4dc9c

Browse files
christothesCopilot
andauthored
Fix additional LineId dupe scenarios (#12361)
* Fix more dupe LineId scenarios * cleanup * cleanup * cleanup * Update tools/apiview/parsers/csharp-api-parser/CSharpAPIParser/TreeToken/CodeFileBuilder.cs Co-authored-by: Copilot <[email protected]> * fail fast with dupes --------- Co-authored-by: Copilot <[email protected]>
1 parent 58e64f9 commit 6f4dc9c

File tree

2 files changed

+119
-4
lines changed

2 files changed

+119
-4
lines changed

tools/apiview/parsers/csharp-api-parser/CSharpAPIParser/TreeToken/CodeFileBuilder.cs

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
using System.Collections.Immutable;
1010
using System.ComponentModel;
1111
using ApiView;
12-
using System.Diagnostics.CodeAnalysis;
1312

1413
namespace CSharpAPIParser.TreeToken
1514
{
@@ -107,6 +106,7 @@ public CodeFile Build(IAssemblySymbol assemblySymbol, bool runAnalysis, List<Dep
107106
}
108107

109108
codeFile.Diagnostics = analyzer.Results.ToArray();
109+
VerifyCodeFile(codeFile);
110110
return codeFile;
111111
}
112112

@@ -140,7 +140,7 @@ public static void BuildInternalsVisibleToAttributes(List<ReviewLine> reviewLine
140140
param = firstComma > 0 ? param?[..(int)firstComma] : param;
141141
reviewLines.Add(new ReviewLine()
142142
{
143-
LineId = attribute.AttributeClass?.Name,
143+
LineId = attribute.AttributeClass?.Name + "_" + param,
144144
Tokens = [
145145
ReviewToken.CreateStringLiteralToken(param)
146146
]
@@ -174,7 +174,7 @@ public static void BuildDependencies(List<ReviewLine> reviewLines, List<Dependen
174174
versionToken.SkipDiff = true;
175175
var dependencyLine = new ReviewLine()
176176
{
177-
LineId = dependency.Name,
177+
LineId = dependency.Name + versionToken.Value,
178178
Tokens = [
179179
ReviewToken.CreateStringLiteralToken(dependency.Name, false),
180180
versionToken
@@ -871,5 +871,43 @@ string GetLineId(ISymbol member)
871871
};
872872
return value;
873873
}
874+
875+
/// <summary>
876+
/// Verifies that all LineId values in the codeFile's ReviewLines collection are unique.
877+
/// </summary>
878+
/// <param name="codeFile">The CodeFile to verify.</param>
879+
/// <exception cref="InvalidOperationException">Thrown when duplicate LineId values are found.</exception>
880+
private static void VerifyCodeFile(CodeFile codeFile)
881+
{
882+
var lineIds = new HashSet<string>();
883+
var duplicates = new List<string>();
884+
885+
void CheckLineIds(IEnumerable<ReviewLine> lines)
886+
{
887+
foreach (var line in lines)
888+
{
889+
if (!string.IsNullOrEmpty(line.LineId))
890+
{
891+
if (!lineIds.Add(line.LineId))
892+
{
893+
duplicates.Add(line.LineId);
894+
}
895+
}
896+
897+
if (line.Children != null && line.Children.Count > 0)
898+
{
899+
CheckLineIds(line.Children);
900+
}
901+
}
902+
}
903+
904+
CheckLineIds(codeFile.ReviewLines);
905+
906+
if (duplicates.Count > 0)
907+
{
908+
throw new InvalidOperationException(
909+
$"Duplicate LineId values found: {string.Join(", ", duplicates.Distinct())}");
910+
}
911+
}
874912
}
875913
}

tools/apiview/parsers/csharp-api-parser/CSharpAPIParserTests/CodeFileTests.cs

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Text.Json.Serialization;
77
using APIView.Model.V2;
88
using Microsoft.CodeAnalysis;
9+
using System.CommandLine.Parsing;
910

1011

1112
namespace CSharpAPIParserTests
@@ -310,7 +311,7 @@ private int CountAttributeRelatedToProperty(List<ReviewLine> lines)
310311

311312
foreach (var line in lines)
312313
{
313-
if (line.LineId != null && line.LineId.StartsWith("System.FlagsAttribute.") && !string.IsNullOrEmpty(line.RelatedToLine))
314+
if (line.LineId != null && line.LineId.StartsWith("System.FlagsAttribute().") && !string.IsNullOrEmpty(line.RelatedToLine))
314315
{
315316
count++;
316317
}
@@ -406,5 +407,81 @@ public void VerifySkippedAttributes()
406407
Assert.False(isRequiresUnreferencedCodePresent);
407408
Assert.False(isRequiresDynamicCode);
408409
}
410+
411+
[Fact]
412+
public void VerifyCodeFileBuilderThrowsOnDuplicateLineIdsInBuildProcess()
413+
{
414+
// This test verifies that CodeFileBuilder.Build() properly throws InvalidOperationException
415+
// when duplicate LineIds are generated during the build process. We simulate this by
416+
// creating a custom SymbolOrderProvider that returns duplicate members.
417+
418+
var sourceCode = @"
419+
using System;
420+
421+
namespace TestNamespace
422+
{
423+
public class TestClass
424+
{
425+
public void Method1() { }
426+
public void Method2() { }
427+
public int Property1 { get; set; }
428+
}
429+
}";
430+
431+
// Create a syntax tree from the source
432+
var syntaxTree = Microsoft.CodeAnalysis.CSharp.CSharpSyntaxTree.ParseText(sourceCode);
433+
434+
// Create a compilation with the syntax tree
435+
var compilation = Microsoft.CodeAnalysis.CSharp.CSharpCompilation.Create(
436+
"TestAssembly",
437+
new[] { syntaxTree },
438+
new[] {
439+
MetadataReference.CreateFromFile(typeof(object).Assembly.Location),
440+
MetadataReference.CreateFromFile(typeof(Console).Assembly.Location)
441+
},
442+
new Microsoft.CodeAnalysis.CSharp.CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary));
443+
444+
// Get the assembly symbol
445+
var assemblySymbol = compilation.Assembly;
446+
447+
// Create the builder with a custom SymbolOrderProvider that duplicates members
448+
var builder = new CSharpAPIParser.TreeToken.CodeFileBuilder();
449+
builder.SymbolOrderProvider = new DuplicatingSymbolOrderProvider();
450+
451+
// The Build method should throw InvalidOperationException when it calls VerifyCodeFile
452+
// because the DuplicatingSymbolOrderProvider causes duplicate LineIds to be generated
453+
var exception = Assert.Throws<InvalidOperationException>(() =>
454+
builder.Build(assemblySymbol, false, null));
455+
456+
Assert.Contains("Duplicate LineId values found", exception.Message);
457+
}
458+
459+
// Custom SymbolOrderProvider that returns members twice to simulate duplicate LineIds
460+
private class DuplicatingSymbolOrderProvider : ICodeFileBuilderSymbolOrderProvider
461+
{
462+
public IEnumerable<INamespaceSymbol> OrderNamespaces(IEnumerable<INamespaceSymbol> namespaces)
463+
{
464+
return namespaces;
465+
}
466+
467+
public IEnumerable<T> OrderTypes<T>(IEnumerable<T> symbols) where T : ITypeSymbol
468+
{
469+
return symbols;
470+
}
471+
472+
public IEnumerable<ISymbol> OrderMembers(IEnumerable<ISymbol> members)
473+
{
474+
// Return each member twice to create duplicate LineIds
475+
var membersList = members.ToList();
476+
foreach (var member in membersList)
477+
{
478+
yield return member;
479+
}
480+
foreach (var member in membersList)
481+
{
482+
yield return member;
483+
}
484+
}
485+
}
409486
}
410487
}

0 commit comments

Comments
 (0)