Address feedback and make some other improvements.

dotnet · carlossanlop · Dec 10, 2024 · Dec 10, 2024 · Dec 10, 2024 · Dec 11, 2024
commit a3a2c70c7e09d0157b10b0aa0dfcaa43e694bb9f
diff --git a/src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/AssemblyLoaderFactory.cs b/src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/AssemblyLoaderFactory.cs
@@ -8,8 +8,18 @@
 
 namespace Microsoft.DotNet.GenAPI;
 
+/// <summary>
+/// Class that facilitates the creation of an assembly symbol loader and its corresponding assembly symbols.
+/// </summary>
 public class AssemblyLoaderFactory
 {
+    /// <summary>
+    /// Creates an assembly symbol loader and its corresponding assembly symbols from the given DLL files in the filesystem.
+    /// </summary>
+    /// <param name="assembliesPaths">A collection of paths where the assembly DLLs should be searched.</param>
+    /// <param name="assemblyReferencesPaths">An optional collection of paths where the assembly references should be searched.</param>
+    /// <param name="respectInternals">Whether to include internal symbols or not.</param>
+    /// <returns>A tuple containing the assembly symbol loader and a dictionary of assembly symbols.</returns>
     public static (IAssemblySymbolLoader, Dictionary<string, IAssemblySymbol>) CreateFromFiles(string[] assembliesPaths, string[]? assemblyReferencesPaths, bool respectInternals = false)
    {
         AssemblySymbolLoader loader;
@@ -31,6 +41,11 @@ public static (IAssemblySymbolLoader, Dictionary<string, IAssemblySymbol>) Creat
         return (loader, assemblySymbols);
     }
 
+    /// <summary>
+    /// Creates a default assembly symbol loader and a dictionary of assembly symbols.
+    /// </summary>
+    /// <param name="respectInternals">Whether to include internal symbols or not.</param>
+    /// <returns>A tuple containing the assembly symbol loader and a dictionary of assembly symbols.</returns>
     public static (IAssemblySymbolLoader, Dictionary<string, IAssemblySymbol>) CreateWithNoAssemblies(bool respectInternals = false) =>
         (new AssemblySymbolLoader(resolveAssemblyReferences: true, includeInternalSymbols: respectInternals), new Dictionary<string, IAssemblySymbol>());
 }
diff --git a/src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/CSharpAssemblyVisitor.cs b/src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/CSharpAssemblyVisitor.cs
@@ -19,6 +19,9 @@
 
 namespace Microsoft.DotNet.GenAPI;
 
+/// <summary>
+/// A class that visits a collection of specified assemblies and generates the corresponding C# document and syntax trees.
+/// </summary>
 public class CSharpAssemblyVisitor : IAssemblyVisitor
 {
     private readonly ILog _logger;
@@ -30,14 +33,30 @@ public class CSharpAssemblyVisitor : IAssemblyVisitor
     private readonly AdhocWorkspace _adhocWorkspace;
     private readonly SyntaxGenerator _syntaxGenerator;
     private readonly IEnumerable<MetadataReference>? _metadataReferences;
-
+    private readonly bool _addPartialModifier;
+    private readonly bool _hideImplicitDefaultConstructors;
+
+    /// <summary>
+    /// Initializes a new instance of the <see cref="CSharpAssemblyVisitor"/> class.
+    /// </summary>
+    /// <param name="logger">The logger to use.</param>
+    /// <param name="loader">The assembly symbol loader to use.</param>
+    /// <param name="symbolFilter">The symbol filter to use.</param>
+    /// <param name="attributeDataSymbolFilter">The attribute data symbol filter to use.</param>
+    /// <param name="exceptionMessage">The optional exception message to use.</param>
+    /// <param name="includeAssemblyAttributes">Whether to include assembly attributes or not.</param>
+    /// <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,
                                  IAssemblySymbolLoader loader,
                                  ISymbolFilter symbolFilter,
                                  ISymbolFilter attributeDataSymbolFilter,
                                  string? exceptionMessage,
                                  bool includeAssemblyAttributes,
-                                 IEnumerable<MetadataReference>? metadataReferences = null)
+                                 IEnumerable<MetadataReference>? metadataReferences = null,
+                                 bool addPartialModifier = true,
+                                 bool hideImplicitDefaultConstructors = true)
     {
         _logger = logger;
         _loader = loader;
@@ -48,6 +67,8 @@ public CSharpAssemblyVisitor(ILog logger,
         _adhocWorkspace = new AdhocWorkspace();
         _syntaxGenerator = SyntaxGenerator.GetGenerator(_adhocWorkspace, LanguageNames.CSharp);
         _metadataReferences = metadataReferences;
+        _addPartialModifier = addPartialModifier;
+        _hideImplicitDefaultConstructors = hideImplicitDefaultConstructors;
     }
 
     /// <inheritdoc />
@@ -75,7 +96,7 @@ public Document GetDocumentForAssembly(IAssemblySymbol assemblySymbol)
 
         SyntaxNode compilationUnit = _syntaxGenerator.CompilationUnit(namespaceSyntaxNodes)
             .WithAdditionalAnnotations(Formatter.Annotation, Simplifier.Annotation)
-            .Rewrite(new TypeDeclarationCSharpSyntaxRewriter())
+            .Rewrite(new TypeDeclarationCSharpSyntaxRewriter(_addPartialModifier))
             .Rewrite(new BodyBlockCSharpSyntaxRewriter(_exceptionMessage));
 
         if (_includeAssemblyAttributes)
@@ -192,7 +213,7 @@ private SyntaxNode Visit(SyntaxNode namedTypeNode, INamedTypeSymbol namedType)
                 }
 
                 // Filter out default constructors since these will be added automatically
-                if (method.IsImplicitDefaultConstructor(_symbolFilter))
+                if (_hideImplicitDefaultConstructors && method.IsImplicitDefaultConstructor(_symbolFilter))
                 {
                     continue;
                 }

@@ -46,21 +46,8 @@ public static void Run(ILog logger,
                 writer.WriteAssembly(kvp.Value);
             }
 
-            if (loader.HasRoslynDiagnostics(out IReadOnlyList<Diagnostic> roslynDiagnostics))
-            {
-                foreach (Diagnostic warning in roslynDiagnostics)
-                {
-                    logger.LogWarning(warning.Id, warning.ToString());
-                }
-            }
-
-            if (loader.HasLoadWarnings(out IReadOnlyList<AssemblyLoadWarning> loadWarnings))
-            {
-                foreach (AssemblyLoadWarning warning in loadWarnings)
-                {
-                    logger.LogWarning(warning.DiagnosticId, warning.Message);
-                }
-            }
+            loader.LogAllDiagnostics(logger);
+            loader.LogAllWarnings(logger);
         }
 
         // Creates a TextWriter capable of writing into Console or a cs file.

diff --git a/src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/SymbolFilterFactory.cs b/src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/SymbolFilterFactory.cs
@@ -7,8 +7,19 @@
 
 namespace Microsoft.DotNet.GenAPI;
 
+/// <summary>
+/// Factory class that creates composite filters based on the given exclusion lists.
+/// </summary>
 public static class SymbolFilterFactory
 {
+    /// <summary>
+    /// Creates a symbol filter based on the given exclusion file paths.
+    /// </summary>
+    /// <param name="apiExclusionFilePaths">A collection of paths where the exclusion files should be searched.</param>
+    /// <param name="respectInternals">Whether to include internal symbols or not.</param>
+    /// <param name="includeEffectivelyPrivateSymbols">Whether to include effectively private symbols or not.</param>
+    /// <param name="includeExplicitInterfaceImplementationSymbols">Whether to include explicit interface implementation symbols or not.</param>
+    /// <returns>An instance of the symbol filter.</returns>
     public static ISymbolFilter GetSymbolFilterFromFiles(string[]? apiExclusionFilePaths,
                                                                  bool respectInternals = false,
                                                                  bool includeEffectivelyPrivateSymbols = true,
@@ -21,6 +32,14 @@ public static ISymbolFilter GetSymbolFilterFromFiles(string[]? apiExclusionFileP
         return GetCompositeSymbolFilter(docIdSymbolFilter, respectInternals, includeEffectivelyPrivateSymbols, includeExplicitInterfaceImplementationSymbols, withImplicitSymbolFilter: true);
     }
 
+    /// <summary>
+    /// Creates a symbol filter based on the given exclusion list.
+    /// </summary>
+    /// <param name="apiExclusionList">A collection of exclusion list.</param>
+    /// <param name="respectInternals">Whether to include internal symbols or not.</param>
+    /// <param name="includeEffectivelyPrivateSymbols">Whether to include effectively private symbols or not.</param>
+    /// <param name="includeExplicitInterfaceImplementationSymbols">Whether to include explicit interface implementation symbols or not.</param>
+    /// <returns>An instance of the symbol filter.</returns>
     public static ISymbolFilter GetSymbolFilterFromList(string[]? apiExclusionList,
                                                                 bool respectInternals = false,
                                                                 bool includeEffectivelyPrivateSymbols = true,
@@ -33,6 +52,14 @@ public static ISymbolFilter GetSymbolFilterFromList(string[]? apiExclusionList,
         return GetCompositeSymbolFilter(docIdSymbolFilter, respectInternals, includeEffectivelyPrivateSymbols, includeExplicitInterfaceImplementationSymbols, withImplicitSymbolFilter: true);
     }
 
+    /// <summary>
+    /// Creates an attribute filter based on the given exclusion file paths.
+    /// </summary>
+    /// <param name="attributeExclusionFilePaths">A collection of paths where the exclusion files should be searched.</param>
+    /// <param name="respectInternals">Whether to include internal symbols or not.</param>
+    /// <param name="includeEffectivelyPrivateSymbols">Whether to include effectively private symbols or not.</param>
+    /// <param name="includeExplicitInterfaceImplementationSymbols">Whether to include explicit interface implementation symbols or not.</param>
+    /// <returns>An instance of the attribute filter.</returns>
     public static ISymbolFilter GetAttributeFilterFromPaths(string[]? attributeExclusionFilePaths,
                                                                     bool respectInternals = false,
                                                                     bool includeEffectivelyPrivateSymbols = true,
@@ -45,6 +72,14 @@ public static ISymbolFilter GetAttributeFilterFromPaths(string[]? attributeExclu
         return GetCompositeSymbolFilter(docIdSymbolFilter, respectInternals, includeEffectivelyPrivateSymbols, includeExplicitInterfaceImplementationSymbols, withImplicitSymbolFilter: false);
     }
 
+    /// <summary>
+    /// Creates an attribute filter based on the given exclusion list.
+    /// </summary>
+    /// <param name="attributeExclusionList">A collection of exclusion list.</param>
+    /// <param name="respectInternals">Whether to include internal symbols or not.</param>
+    /// <param name="includeEffectivelyPrivateSymbols">Whether to include effectively private symbols or not.</param>
+    /// <param name="includeExplicitInterfaceImplementationSymbols">Whether to include explicit interface implementation symbols or not.</param>
+    /// <returns>An instance of the attribute filter.</returns>
     public static ISymbolFilter GetAttributeFilterFromList(string[]? attributeExclusionList,
                                                                    bool respectInternals = false,
                                                                    bool includeEffectivelyPrivateSymbols = true,

@@ -17,6 +17,14 @@ namespace Microsoft.DotNet.GenAPI.SyntaxRewriter
     /// </summary>
     public class TypeDeclarationCSharpSyntaxRewriter : CSharpSyntaxRewriter
     {
+        private readonly bool _addPartialModifier;
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="TypeDeclarationCSharpSyntaxRewriter"/> class, and optionally allows deciding whether to insert the partial modifier for types or not.
+        /// </summary>
+        /// <param name="addPartialModifier">Determines whether to insert the partial modifier for types or not.</param>
+        public TypeDeclarationCSharpSyntaxRewriter(bool addPartialModifier = true) => _addPartialModifier = addPartialModifier;
+
         /// <inheritdoc />
         public override SyntaxNode? VisitInterfaceDeclaration(InterfaceDeclarationSyntax node)
         {
@@ -83,15 +91,15 @@ public class TypeDeclarationCSharpSyntaxRewriter : CSharpSyntaxRewriter
             }
         }
 
-        private static T? VisitCommonTypeDeclaration<T>(T? node) where T : TypeDeclarationSyntax
+        private T? VisitCommonTypeDeclaration<T>(T? node) where T : TypeDeclarationSyntax
         {
             if (node == null)
             {
                 return null;
             }
 
             node = RemoveBaseType(node, "global::System.Object");
-            return AddPartialModifier(node);
+            return _addPartialModifier ?  AddPartialModifier(node) : node;
         }
 
         private static T? AddPartialModifier<T>(T? node) where T : TypeDeclarationSyntax =>

@@ -7,6 +7,7 @@
 using System.Reflection.PortableExecutable;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.DotNet.ApiSymbolExtensions.Logging;
 
 namespace Microsoft.DotNet.ApiSymbolExtensions
 {
@@ -279,6 +280,40 @@ public IEnumerable<IAssemblySymbol> LoadMatchingAssemblies(IEnumerable<IAssembly
             return matchingAssemblies;
         }
 
+        /// <inheritdoc />
+        public void LogAllDiagnostics(ILog logger, string? customMessage = null)
+        {
+            if (HasRoslynDiagnostics(out IReadOnlyList<Diagnostic> roslynDiagnostics))
+            {
+                if (!string.IsNullOrEmpty(customMessage))
+                {
+                    logger.LogWarning(customMessage!);
+                }
+
+                foreach (Diagnostic warning in roslynDiagnostics)
+                {
+                    logger.LogWarning(warning.Id, warning.ToString());
+                }
+            }
+        }
+
+        /// <inheritdoc />
+        public void LogAllWarnings(ILog logger, string? customMessage = null)
+        {
+            if (HasLoadWarnings(out IReadOnlyList<AssemblyLoadWarning> loadWarnings))
+            {
+                if (!string.IsNullOrEmpty(customMessage))
+                {
+                    logger.LogWarning(customMessage!);
+                }
+
+                foreach (AssemblyLoadWarning warning in loadWarnings)
+                {
+                    logger.LogWarning(warning.DiagnosticId, warning.Message);
+                }
+            }
+        }
+
         /// <inheritdoc />
         public IEnumerable<MetadataReference> MetadataReferences => _cSharpCompilation.References;
 

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using Microsoft.CodeAnalysis;
+using Microsoft.DotNet.ApiSymbolExtensions.Logging;
 
 namespace Microsoft.DotNet.ApiSymbolExtensions
 {
@@ -90,6 +91,20 @@ public interface IAssemblySymbolLoader
         /// <returns>The list of matching assemblies represented as <see cref="IAssemblySymbol"/>.</returns>
         IEnumerable<IAssemblySymbol> LoadMatchingAssemblies(IEnumerable<IAssemblySymbol> fromAssemblies, IEnumerable<string> searchPaths, bool validateMatchingIdentity = true, bool warnOnMissingAssemblies = true);
 
+        /// <summary>
+        ///
+        /// </summary>
+        /// <param name="logger"></param>
+        /// <param name="customMessage"></param>
+        void LogAllDiagnostics(ILog logger, string? customMessage = null);
+
+        /// <summary>
+        ///
+        /// </summary>
+        /// <param name="logger"></param>
+        /// <param name="customMessage"></param>
+        void LogAllWarnings(ILog logger, string? customMessage = null);
+
         /// <summary>
         /// The list of metadata references represented as <see cref="MetadataReference" />.
         /// </summary>