Address latest batch of feedback: Rename the wrongly named CSharpAssemblyVisitor class, remove its interface. Move the static methods from SymbolFilterFactory to CompositeSymbolFilter, and delete SymbolFilterFactory. Move TryGetRecordConstructor static extension method to a higher assembly. Move the NullableAttributes.cs dependency to the same higher assembly. Move ImplicitSymbolFilter to a higher assembly. Adjust the tests accordingly.

Author: carlossanlop

src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/CSharpAssemblyVisitor.cs renamed to src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/CSharpAssemblyDocumentGenerator.cs

@@ -20,9 +20,9 @@
 namespace Microsoft.DotNet.GenAPI;
 
 /// <summary>
-/// A class that visits a collection of specified assemblies and generates the corresponding C# document and syntax trees.
+/// A class that generates the C# document and syntax trees of a specified collection of assemblies.
 /// </summary>
-public class CSharpAssemblyVisitor : IAssemblyVisitor
+public sealed class CSharpAssemblyDocumentGenerator
 {
     private readonly ILog _logger;
     private readonly IAssemblySymbolLoader _loader;
@@ -37,7 +37,7 @@ public class CSharpAssemblyVisitor : IAssemblyVisitor
     private readonly bool _hideImplicitDefaultConstructors;
 
     /// <summary>
-    /// Initializes a new instance of the <see cref="CSharpAssemblyVisitor"/> class.
+    /// Initializes a new instance of the <see cref="CSharpAssemblyDocumentGenerator"/> class.
     /// </summary>
     /// <param name="logger">The logger to use.</param>
     /// <param name="loader">The assembly symbol loader to use.</param>
@@ -48,7 +48,7 @@ public class CSharpAssemblyVisitor : IAssemblyVisitor
     /// <param name="metadataReferences">The metadata references to use. The default value is <see langword="null"/>.</param>
     /// <param name="addPartialModifier">Whether to add the partial modifier or not. The default value is <see langword="true"/>.</param>
     /// <param name="hideImplicitDefaultConstructors">Whether to hide implicit default constructors or not. The default value is <see langword="true"/>.</param>
-    public CSharpAssemblyVisitor(ILog logger,
+    public CSharpAssemblyDocumentGenerator(ILog logger,
                                  IAssemblySymbolLoader loader,
                                  ISymbolFilter symbolFilter,
                                  ISymbolFilter attributeDataSymbolFilter,
@@ -71,7 +71,11 @@ public CSharpAssemblyVisitor(ILog logger,
         _hideImplicitDefaultConstructors = hideImplicitDefaultConstructors;
     }
 
-    /// <inheritdoc />
+    /// <summary>
+    /// Returns the configured source code document for the specified assembly symbol.
+    /// </summary>
+    /// <param name="assemblySymbol">The assembly symbol that represents the loaded assembly.</param>
+    /// <returns>The source code document instance of the specified assembly symbol.</returns>
     public Document GetDocumentForAssembly(IAssemblySymbol assemblySymbol)
     {
         CSharpCompilationOptions compilationOptions = new(OutputKind.DynamicallyLinkedLibrary,
@@ -114,7 +118,11 @@ public Document GetDocumentForAssembly(IAssemblySymbol assemblySymbol)
         return document;
     }
 
-    /// <inheritdoc />
+    /// <summary>
+    /// Returns the formatted root syntax node for the specified document.
+    /// </summary>
+    /// <param name="document">A source code document instance.</param>
+    /// <returns>The root syntax node of the specified document.</returns>
     public SyntaxNode GetFormattedRootNodeForDocument(Document document) => document.GetSyntaxRootAsync().Result!.Rewrite(new SingleLineStatementCSharpSyntaxRewriter());
 
     private SyntaxNode? Visit(INamespaceSymbol namespaceSymbol)

src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/CSharpFileBuilder.cs

@@ -18,7 +18,7 @@ public sealed class CSharpFileBuilder : IAssemblySymbolWriter
     {
         private readonly TextWriter _textWriter;
         private readonly string? _header;
-        private readonly IAssemblyVisitor _assemblyVisitor;
+        private readonly CSharpAssemblyDocumentGenerator _docGenerator;
 
         public CSharpFileBuilder(ILog logger,
                                  TextWriter textWriter,
@@ -32,15 +32,15 @@ public CSharpFileBuilder(ILog logger,
         {
             _textWriter = textWriter;
             _header = header;
-            _assemblyVisitor = new CSharpAssemblyVisitor(logger, loader, symbolFilter, attributeDataSymbolFilter, exceptionMessage, includeAssemblyAttributes, metadataReferences);
+            _docGenerator = new CSharpAssemblyDocumentGenerator(logger, loader, symbolFilter, attributeDataSymbolFilter, exceptionMessage, includeAssemblyAttributes, metadataReferences);
         }
 
         /// <inheritdoc />
         public void WriteAssembly(IAssemblySymbol assemblySymbol)
         {
             _textWriter.Write(GetFormattedHeader(_header));
-            Document document = _assemblyVisitor.GetDocumentForAssembly(assemblySymbol);
-            _assemblyVisitor.GetFormattedRootNodeForDocument(document).WriteTo(_textWriter);
+            Document document = _docGenerator.GetDocumentForAssembly(assemblySymbol);
+            _docGenerator.GetFormattedRootNodeForDocument(document).WriteTo(_textWriter);
         }
 
         private static string GetFormattedHeader(string? customHeader)

src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/Filtering/ImplicitSymbolFilter.cs

@@ -6,6 +6,7 @@
 #endif
 using Microsoft.CodeAnalysis;
 using Microsoft.DotNet.ApiSymbolExtensions;
+using Microsoft.DotNet.ApiSymbolExtensions.Filtering;
 using Microsoft.DotNet.ApiSymbolExtensions.Logging;
 
 namespace Microsoft.DotNet.GenAPI
@@ -38,8 +39,8 @@ public static void Run(ILog logger,
                 CSharpFileBuilder writer = new(logger,
                                                textWriter,
                                                loader,
-                                               SymbolFilterFactory.GetSymbolFilterFromFiles(excludeApiFiles, respectInternals),
-                                               SymbolFilterFactory.GetAttributeFilterFromPaths(excludeAttributesFiles, respectInternals),
+                                               CompositeSymbolFilter.GetSymbolFilterFromFiles(excludeApiFiles, respectInternals),
+                                               CompositeSymbolFilter.GetAttributeFilterFromPaths(excludeAttributesFiles, respectInternals),
                                                headerFile,
                                                exceptionMessage,
                                                includeAssemblyAttributes);

src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/GenAPIApp.cs

@@ -1,7 +1,6 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.Diagnostics.CodeAnalysis;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -70,14 +69,14 @@ private static SyntaxList<AttributeListSyntax> FromAttributeData(IEnumerable<Att
         }
 
         // Build dummy field from a type, field name, and attribute list.
-        private static SyntaxNode CreateDummyField(string typ, string fieldName, SyntaxList<AttributeListSyntax> attrs, bool isReadonly)
+        private static SyntaxNode CreateDummyField(string type, string fieldName, SyntaxList<AttributeListSyntax> attrs, bool isReadonly)
         {
             List<SyntaxToken> modifiers = new() { SyntaxFactory.Token(SyntaxKind.PrivateKeyword) };
             if (isReadonly)
                 modifiers.Add(SyntaxFactory.Token(SyntaxKind.ReadOnlyKeyword));
             SyntaxNode declaration = SyntaxFactory.FieldDeclaration(
                 SyntaxFactory.VariableDeclaration(
-                    SyntaxFactory.ParseTypeName(typ))
+                    SyntaxFactory.ParseTypeName(type))
                 .WithVariables(
                     SyntaxFactory.SingletonSeparatedList<VariableDeclaratorSyntax>(
                         SyntaxFactory.VariableDeclarator(
@@ -224,7 +223,7 @@ static bool IncludeInternalSymbols(ISymbolFilter filter) =>
             yield return constructor;
         }
 
-        // Synthesize a base class initializer. 
+        // Synthesize a base class initializer.
         public static ConstructorInitializerSyntax GenerateBaseConstructorInitializer(this IMethodSymbol baseTypeConstructor)
         {
             return SyntaxFactory.ConstructorInitializer(SyntaxKind.BaseConstructorInitializer, baseTypeConstructor.CreateDefaultArgumentList());
@@ -248,33 +247,5 @@ public static ArgumentListSyntax CreateDefaultArgumentList(this IMethodSymbol me
 
             return argumentList;
         }
-
-        // Locates constructor generated by the compiler for `record Foo(...)` syntax
-        // If the type is a record and the compiler generated constructor is found it will be returned, otherwise null.
-        // The compiler will not generate a constructor in the case where the user defined it themself without using an argument list
-        // in the record declaration, or if the record has no parameters.
-        public static bool TryGetRecordConstructor(this INamedTypeSymbol type, [NotNullWhen(true)] out IMethodSymbol? recordConstructor)
-        {
-            if (!type.IsRecord)
-            {
-                recordConstructor = null;
-                return false;
-            }
-
-            // Locate the compiler generated Deconstruct method.
-            var deconstructMethod = (IMethodSymbol?)type.GetMembers("Deconstruct")
-                .FirstOrDefault(m => m is IMethodSymbol && m.IsCompilerGenerated());
-
-            // Locate the compiler generated constructor by matching parameters to Deconstruct - since we cannot locate it with an attribute.
-            recordConstructor = (IMethodSymbol?)type.GetMembers(".ctor")
-                .FirstOrDefault(m => m is IMethodSymbol method &&
-                    method.MethodKind == MethodKind.Constructor &&
-                    (deconstructMethod == null ?
-                        method.Parameters.IsEmpty :
-                        method.Parameters.Select(p => p.Type).SequenceEqual(
-                            deconstructMethod.Parameters.Select(p => p.Type), SymbolEqualityComparer.Default)));
-
-            return recordConstructor != null;
-        }
     }
 }

src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/IAssemblyVisitor.cs

@@ -4,10 +4,6 @@
     <TargetFrameworks>$(NetToolMinimum);$(NetFrameworkToolCurrent)</TargetFrameworks>
   </PropertyGroup>
 
-  <ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework'">
-    <Compile Include="$(RepoRoot)src\Common\NullableAttributes.cs" LinkBase="Common" />
-  </ItemGroup>
-
   <ItemGroup>
     <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" />
     <ProjectReference Include="..\..\Microsoft.DotNet.ApiSymbolExtensions\Microsoft.DotNet.ApiSymbolExtensions.csproj" />