Correctly Visit types & Add DI wiring to scraper (#998)

dotnet · Aug 5, 2022 · b3fc926 · b3fc926
1 parent 7739ef6
commit b3fc926
Show file tree

Hide file tree

Showing 14 changed files with 186 additions and 39 deletions.
diff --git a/Directory.Packages.props b/Directory.Packages.props
@@ -4,6 +4,7 @@
       For more information what this is see https://github.com/NuGet/Home/wiki/Centrally-managing-NuGet-package-versions
       -->
     <PropertyGroup>
+        <MsExtensionNugetVersion>6.0.0</MsExtensionNugetVersion>
         <RoslynPackageVersion>4.2.0</RoslynPackageVersion>
     </PropertyGroup>
     <ItemGroup>
@@ -12,7 +13,7 @@
         <PackageVersion Include="System.Text.Json" Version="6.0.5"/>
         <PackageVersion Include="ClangSharp.PInvokeGenerator" Version="14.0.0-beta2"/>
         <PackageVersion Include="Microsoft.Build.Locator" Version="1.4.1"/>
-        <PackageVersion Include="Microsoft.Extensions.FileSystemGlobbing" Version="6.0.0-preview.6.21352.12"/>
+        <PackageVersion Include="Microsoft.Extensions.FileSystemGlobbing" Version="$(MsExtensionNugetVersion)"/>
         <PackageVersion Include="Microsoft.Build.Framework" Version="16.10.0"/>
         <PackageVersion Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="$(RoslynPackageVersion)"/>
         <PackageVersion Include="Microsoft.CodeAnalysis.Workspaces.MSBuild" Version="$(RoslynPackageVersion)"/>
@@ -25,10 +26,10 @@
         <PackageVersion Include="Microsoft.Bcl.HashCode" Version="1.1.0"/>
         <PackageVersion Include="Microsoft.CodeAnalysis.PublicApiAnalyzers" Version="3.3.1"/>
         <PackageVersion Include="Microsoft.Extensions.Hosting" Version="6.0.0"/>
-        <PackageVersion Include="Microsoft.Extensions.Hosting.Abstractions" Version="6.0.0"/>
-        <PackageVersion Include="Microsoft.Extensions.Logging.Abstractions" Version="6.0.0"/>
-        <PackageVersion Include="Microsoft.Extensions.Configuration.Abstractions" Version="6.0.0"/>
-        <PackageVersion Include="Microsoft.Extensions.Options" Version="6.0.0"/>
+        <PackageVersion Include="Microsoft.Extensions.Hosting.Abstractions" Version="$(MsExtensionNugetVersion)"/>
+        <PackageVersion Include="Microsoft.Extensions.Logging.Abstractions" Version="$(MsExtensionNugetVersion)"/>
+        <PackageVersion Include="Microsoft.Extensions.Configuration.Abstractions" Version="$(MsExtensionNugetVersion)"/>
+        <PackageVersion Include="Microsoft.Extensions.Options" Version="$(MsExtensionNugetVersion)"/>
         <PackageVersion Include="Microsoft.NET.Test.Sdk" Version="16.8.0"/>
         <PackageVersion Include="xunit" Version="2.4.1"/>
         <PackageVersion Include="xunit.runner.visualstudio" Version="2.4.3"/>
@@ -46,5 +47,8 @@
         <PackageVersion Include="libclang" Version="14.0.0"/>
         <PackageVersion Include="libClangSharp" Version="14.0.0-beta1"/>
         <PackageVersion Include="Verify.Xunit" Version="16.9.0" />
+        <PackageVersion Include="Microsoft.Extensions.Logging.Console" Version="$(MsExtensionNugetVersion)" />
+        <PackageVersion Include="Microsoft.Extensions.DependencyInjection" Version="$(MsExtensionNugetVersion)" />
+        <PackageVersion Include="Microsoft.Extensions.Logging" Version="$(MsExtensionNugetVersion)" />
     </ItemGroup>
 </Project>
diff --git a/src/generators/Silk.NET.SilkTouch.Scraper/ClangScraper.cs b/src/generators/Silk.NET.SilkTouch.Scraper/ClangScraper.cs
@@ -9,6 +9,7 @@
 using System.Xml;
 using ClangSharp;
 using ClangSharp.Interop;
+using Microsoft.Extensions.Logging;
 using Silk.NET.SilkTouch.Symbols;
 
 namespace Silk.NET.SilkTouch.Scraper;
@@ -18,6 +19,7 @@ namespace Silk.NET.SilkTouch.Scraper;
 /// </summary>
 public sealed class ClangScraper
 {
+    private readonly ILoggerFactory _loggerFactory;
     /// <summary>
     /// Placeholder used in place of library paths
     /// </summary>
@@ -28,6 +30,15 @@ public sealed class ClangScraper
     /// </summary>
     public static readonly string LibraryNamespacePlaceholder = "LIBRARY_NAMESPACE";
 
+    /// <summary>
+    /// Creates a ClangScraper given it's dependencies
+    /// </summary>
+    /// <param name="loggerFactory">A logger factory to create loggers from</param>
+    public ClangScraper(ILoggerFactory loggerFactory)
+    {
+        _loggerFactory = loggerFactory;
+    }
+
     /// <summary>
     /// Scrapes the given XML document for symbols
     /// </summary>
@@ -42,7 +53,7 @@ public IEnumerable<Symbol> ScrapeXML(XmlDocument document)
             return Enumerable.Empty<Symbol>();
         }
 
-        var visitor = new XmlVisitor();
+        var visitor = new XmlVisitor(_loggerFactory.CreateLogger<XmlVisitor>());
         return visitor.Visit(bindings);
     }
 
@@ -58,6 +69,7 @@ public IEnumerable<Symbol> ScrapeXML(XmlDocument document)
     /// <exception cref="InvalidOperationException">Will be thrown when errors during parsing are encountered</exception>
     public XmlDocument? GenerateXML(string headerFile, string[] includedNames, string[] excludedNames, string[] includeDirectories, string[] definedMacros)
     {
+        var logger = _loggerFactory.CreateLogger("ClangScraper.ScrapeXML");
         var opts = PInvokeGeneratorConfigurationOptions.None;
         opts |= PInvokeGeneratorConfigurationOptions.NoDefaultRemappings;
 
@@ -105,11 +117,11 @@ Stream OutputStreamFactory(string fileName)
         try
         {
             using (var pinvokeGenerator = new PInvokeGenerator(config, OutputStreamFactory))
-                GenerateBindings(pinvokeGenerator, headerFile, commandLineArgs.ToArray(), translationFlags);
+                GenerateBindings(pinvokeGenerator, headerFile, commandLineArgs.ToArray(), translationFlags, logger);
 
             foreach (var (name, stream) in files)
             {
-                Console.WriteLine(name);
+                logger.LogTrace("Outputting File \"{name}\"", name);
                 var doc = new XmlDocument();
                 stream.Position = 0;
                 doc.Load(stream);
@@ -132,7 +144,8 @@ private static void GenerateBindings
         PInvokeGenerator pinvokeGenerator,
         string headerFile,
         string[] commandLineArgs,
-        CXTranslationUnit_Flags translationFlags
+        CXTranslationUnit_Flags translationFlags,
+        ILogger logger
     )
     {
         var result = CXTranslationUnit.TryParse
@@ -168,10 +181,20 @@ CXTranslationUnit_Flags translationFlags
 
             foreach (var diagnostic in pinvokeGenerator.Diagnostics)
             {
-                if (diagnostic.Level > DiagnosticLevel.Warning)
-                {
-                    Console.WriteLine(diagnostic.Message);
-                }
+                logger.Log
+                (
+                    diagnostic.Level switch
+                    {
+                        DiagnosticLevel.Info    => LogLevel.Debug,
+                        DiagnosticLevel.Warning => LogLevel.Information,
+                        DiagnosticLevel.Error   => LogLevel.Warning,
+                        _                       => LogLevel.Debug
+                    },
+                    "Clang Diagnostic: {level} at: {location} \"{message}\"",
+                    diagnostic.Level,
+                    diagnostic.Level,
+                    diagnostic.Message
+                );
             }
         }
         finally

diff --git a/src/generators/Silk.NET.SilkTouch.Scraper/XmlVisitor.cs b/src/generators/Silk.NET.SilkTouch.Scraper/XmlVisitor.cs
@@ -4,15 +4,23 @@
 using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
+using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using System.Reflection.Metadata;
 using System.Xml;
+using Microsoft.Extensions.Logging;
 using Silk.NET.SilkTouch.Symbols;
+using TypeReference=Silk.NET.SilkTouch.Symbols.TypeReference;
 
 namespace Silk.NET.SilkTouch.Scraper;
 
 internal sealed class XmlVisitor
 {
+    private readonly ILogger _logger;
+    public XmlVisitor(ILogger logger) {
+        _logger = logger;
+    }
+
     public IEnumerable<Symbol> Visit(XmlNode node)
     {
         switch (node)
@@ -40,22 +48,77 @@ private IEnumerable<Symbol> VisitField(XmlElement field)
             throw new InvalidOperationException("Field requires a name");
         }
 
-        // TODO: Appropriately Visit Type.
-        var type = new ExternalTypeReference
-        (
-            null,
-            new IdentifierSymbol
-            (
-                field.ChildNodes.Cast<XmlNode>().SingleOrDefault(x => x.Name == "type")?.InnerText ??
-                throw new InvalidOperationException("Could not decode Field Type")
-            )
-        );
+        var typeNode = field.ChildNodes.Cast<XmlNode>().SingleOrDefault(x => x.Name == "type") as XmlElement;
+        if (typeNode is null)
+            throw new InvalidOperationException("Field type cannot be null");
+
+        TypeReference? finalType = null;
+        foreach (var type in VisitType(typeNode))
+        {
+            if (type is null)
+            {
+                throw new InvalidOperationException
+                (
+                    $"{nameof(XmlVisitor)}.{nameof(VisitType)} has returned null"
+                );
+            }
+
+            if (type is not TypeReference tr)
+            {
+                throw new InvalidOperationException
+                (
+                    $"{nameof(XmlVisitor)}.{nameof(VisitType)} has returned something other then a type reference"
+                );
+            }
+
+            if (finalType is not null)
+            {
+                throw new InvalidOperationException
+                    ($"{nameof(XmlVisitor)}.{nameof(VisitType)} has returned more then one result");
+            }
+
+            finalType = tr;
+        }
+
+        if (finalType is null)
+        {
+            throw new InvalidOperationException
+                ($"No type could be resolved from {typeNode.OuterXml}");
+        }
 
         return new[]
         {
-            new FieldSymbol(type, new IdentifierSymbol(name))
+            new FieldSymbol(finalType, new IdentifierSymbol(name))
         };
     }
+
+    // TODO: Configurable Type maps
+    private static readonly Dictionary<string, TypeReference> _typeMap = new()
+    {
+        ["int"] = new ExternalTypeReference(null, new IdentifierSymbol("int"))
+    };
+
+    private bool TryResolveTypeRef(string text, [NotNullWhen(true)] out TypeReference? reference)
+    {
+        if (_typeMap.TryGetValue(text, out reference))
+        {
+            return true;
+        }
+        else
+        {
+            _logger.LogDebug("Failed to resolve type reference from \"{text}\"", text);
+            return false;
+        }
+    }
+
+    // NOTE: This does not visit types as in class/struct, but visits *references* to types. Like from methods or fields.
+    private IEnumerable<Symbol> VisitType(XmlElement type)
+    {
+        return
+            TryResolveTypeRef(type.InnerText, out var r)
+                ? new[] { r }
+                : Array.Empty<Symbol>();
+    }
 
     private IEnumerable<Symbol> VisitStruct(XmlElement @struct)
     {

diff --git a/tests/Silk.NET.SilkTouch.IntegrationTests/Silk.NET.SilkTouch.IntegrationTests.csproj b/tests/Silk.NET.SilkTouch.IntegrationTests/Silk.NET.SilkTouch.IntegrationTests.csproj
@@ -20,6 +20,7 @@
         <PackageReference Include="libclang" />
         <PackageReference Include="libClangSharp" />
         <PackageReference Include="Verify.Xunit" />
+        <PackageReference Include="Microsoft.Extensions.Logging.Console" />
     </ItemGroup>
 
     <ItemGroup>

diff --git a/tests/Silk.NET.SilkTouch.IntegrationTests/StructIntegrationTests.cs b/tests/Silk.NET.SilkTouch.IntegrationTests/StructIntegrationTests.cs
@@ -152,6 +152,23 @@ struct S
         int i;
         double d;
     };
+};");
+        return Verifier.Verify(result);
+    }
+
+    [Fact,
+     Trait("Category",        "Integration"),
+     Trait("Source Language", "C++"),
+     Trait("Target Language", "C#"),
+     Trait("Feature",         "Structs"),
+     Trait("Feature",         "Fields")]
+    public Task Test8()
+    {
+        var result = TestHelper.GetCSharpOutputFromCpp(@"
+struct a {
+    struct x { struct a *p; /* ... */ };
+
+    struct x *p;
 };");
         return Verifier.Verify(result);
     }

diff --git a/tests/Silk.NET.SilkTouch.IntegrationTests/TestHelper.cs b/tests/Silk.NET.SilkTouch.IntegrationTests/TestHelper.cs
@@ -2,9 +2,11 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
-using System.Diagnostics;
+using System.Data;
 using System.IO;
 using System.Linq;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
 using Silk.NET.SilkTouch.Emitter;
 using Silk.NET.SilkTouch.Scraper;
 using Xunit;
@@ -16,10 +18,13 @@ public static class TestHelper
     public static string GetCSharpOutputFromCpp(string cpp)
     {
         var tempFile = Path.GetTempFileName();
+        var serviceProvider = new ServiceCollection()
+            .AddLogging(builder => builder.AddConsole())
+            .BuildServiceProvider();
 
         File.WriteAllText(tempFile, "/* THIS IS A GENERATED FILE, PIPED TO CLANG FOR TESTING BY SILK.NET */" + cpp);
 
-        var scraper = new ClangScraper();
+        var scraper = new ClangScraper(serviceProvider.GetRequiredService<ILoggerFactory>());
         var xml = scraper.GenerateXML
             (tempFile, Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>());
 

diff --git a/tests/Silk.NET.SilkTouch.Scraper.Tests/BasicXMLTests.cs b/tests/Silk.NET.SilkTouch.Scraper.Tests/BasicXMLTests.cs
@@ -44,7 +44,7 @@ public void BasicStructScrapingTest()
     int32_t f2;
 } Test;");
 
-        var scraper = new ClangScraper();
+        var scraper = Helpers.CreateScraper();
         var xml = scraper.GenerateXML
             (tempFile, Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>());
 

diff --git a/tests/Silk.NET.SilkTouch.Scraper.Tests/FieldScrapingTests.cs b/tests/Silk.NET.SilkTouch.Scraper.Tests/FieldScrapingTests.cs
@@ -17,7 +17,7 @@ public void FieldSymbol()
         var doc = new XmlDocument();
         doc.LoadXml(@"<field name=""f2""><type native=""int32_t"">int</type></field>");
 
-        var symbols = new ClangScraper().ScrapeXML(doc);
+        var symbols = Helpers.CreateScraper().ScrapeXML(doc);
 
         var symbol = Assert.Single(symbols);
         var field = Assert.IsType<FieldSymbol>(symbol);
@@ -31,7 +31,7 @@ public void CorrectIdentifier()
         var doc = new XmlDocument();
         doc.LoadXml(@"<field name=""f2""><type native=""int32_t"">int</type></field>");
 
-        var symbols = new ClangScraper().ScrapeXML(doc);
+        var symbols = Helpers.CreateScraper().ScrapeXML(doc);
 
         var symbol = Assert.Single(symbols);
         var field = Assert.IsType<FieldSymbol>(symbol);
@@ -46,7 +46,7 @@ public void CorrectType()
         var doc = new XmlDocument();
         doc.LoadXml(@"<field name=""f2""><type native=""int32_t"">int</type></field>");
 
-        var symbols = new ClangScraper().ScrapeXML(doc);
+        var symbols = Helpers.CreateScraper().ScrapeXML(doc);
 
         var symbol = Assert.Single(symbols);
         var field = Assert.IsType<FieldSymbol>(symbol);

diff --git a/tests/Silk.NET.SilkTouch.Scraper.Tests/Helpers.cs b/tests/Silk.NET.SilkTouch.Scraper.Tests/Helpers.cs
@@ -0,0 +1,33 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Collections.Generic;
+using System.Xml;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+using Silk.NET.SilkTouch.Symbols;
+
+namespace Silk.NET.SilkTouch.Scraper.Tests;
+
+public static class Helpers
+{
+    private static IServiceProvider CreateServiceProvider()
+    {
+        return new ServiceCollection()
+            .AddLogging(builder =>
+                {
+                    builder.AddConsole();
+                    builder.SetMinimumLevel(LogLevel.Trace);
+                }
+            )
+            .BuildServiceProvider();
+    }
+
+    public static ClangScraper CreateScraper()
+    {
+        var serviceProvider = CreateServiceProvider();
+
+        return new ClangScraper(serviceProvider.GetRequiredService<ILoggerFactory>());
+    }
+}
diff --git a/tests/Silk.NET.SilkTouch.Scraper.Tests/IdentityScrapingTests.cs b/tests/Silk.NET.SilkTouch.Scraper.Tests/IdentityScrapingTests.cs
@@ -15,7 +15,7 @@ public void GeneratesNoSymbols()
     {
         var doc = new XmlDocument();
 
-        var symbols = new ClangScraper().ScrapeXML(doc);
+        var symbols = Helpers.CreateScraper().ScrapeXML(doc);
 
         Assert.Empty(symbols);
     }
@@ -29,7 +29,7 @@ public void BindingsGeneratesNoSymbols()
 </bindings>
 ");
 
-        var symbols = new ClangScraper().ScrapeXML(doc);
+        var symbols = Helpers.CreateScraper().ScrapeXML(doc);
 
         Assert.Empty(symbols);
     }