From 61a579e0105cdce47587350dcbc746ff3798ef6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20S=C3=A1nchez=20L=C3=B3pez?= <1175054+carlossanlop@users.noreply.github.com> Date: Wed, 8 Jan 2025 16:23:08 -0800 Subject: [PATCH 1/9] Pass an ILog instance to the AssemblySymbolLoader constructor. --- .../ApiCompatServiceProvider.cs | 2 +- .../Microsoft.DotNet.GenAPI/GenAPIApp.cs | 19 ++------- .../AssemblySymbolLoader.cs | 42 ++++++++++++++++++- .../AssemblySymbolLoaderFactory.cs | 7 +++- .../IAssemblySymbolLoader.cs | 14 ++++++- ...eworkInPackageValidatorIntegrationTests.cs | 2 +- .../ValidatePackageTargetIntegrationTests.cs | 2 +- .../Rules/AssemblyIdentityMustMatchTests.cs | 8 ++-- .../AssemblySymbolLoaderTests.cs | 27 ++++++------ .../CSharpFileBuilderTests.cs | 6 ++- 10 files changed, 88 insertions(+), 41 deletions(-) diff --git a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Shared/ApiCompatServiceProvider.cs b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Shared/ApiCompatServiceProvider.cs index b5d95dd8debd..4edb495c0e5d 100644 --- a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Shared/ApiCompatServiceProvider.cs +++ b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Shared/ApiCompatServiceProvider.cs @@ -49,7 +49,7 @@ public ApiCompatServiceProvider(Func logFa return new ApiCompatRunner(SuppressibleLog, SuppressionEngine, new ApiComparerFactory(ruleFactory(SuppressibleLog), apiComparerSettings), - new AssemblySymbolLoaderFactory(respectInternals)); + new AssemblySymbolLoaderFactory(SuppressibleLog, respectInternals)); }); } diff --git a/src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/GenAPIApp.cs b/src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/GenAPIApp.cs index 5fbdd6c574c3..f7e751f42b29 100644 --- a/src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/GenAPIApp.cs +++ b/src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/GenAPIApp.cs @@ -35,7 +35,7 @@ public static void Run(ILog logger, bool resolveAssemblyReferences = assemblyReferences?.Length > 0; // Create, configure and execute the assembly loader. - AssemblySymbolLoader loader = new(resolveAssemblyReferences, respectInternals); + AssemblySymbolLoader loader = new(logger, resolveAssemblyReferences, respectInternals); if (assemblyReferences is not null) { loader.AddReferenceSearchPaths(assemblyReferences); @@ -87,21 +87,8 @@ public static void Run(ILog logger, fileBuilder.WriteAssembly(assemblySymbol); } - if (loader.HasRoslynDiagnostics(out IReadOnlyList roslynDiagnostics)) - { - foreach (Diagnostic warning in roslynDiagnostics) - { - logger.LogWarning(warning.Id, warning.ToString()); - } - } - - if (loader.HasLoadWarnings(out IReadOnlyList loadWarnings)) - { - foreach (AssemblyLoadWarning warning in loadWarnings) - { - logger.LogWarning(warning.DiagnosticId, warning.Message); - } - } + loader.LogAllDiagnostics(); + loader.LogAllWarnings(); } // Creates a TextWriter capable of writing into Console or a cs file. diff --git a/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblySymbolLoader.cs b/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblySymbolLoader.cs index 6d7dea1bc8ae..5d886334fd6b 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblySymbolLoader.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblySymbolLoader.cs @@ -7,6 +7,7 @@ using System.Reflection.PortableExecutable; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; +using Microsoft.DotNet.ApiSymbolExtensions.Logging; namespace Microsoft.DotNet.ApiSymbolExtensions { @@ -15,7 +16,8 @@ namespace Microsoft.DotNet.ApiSymbolExtensions /// public class AssemblySymbolLoader : IAssemblySymbolLoader { - // Dictionary that holds the paths to help loading dependencies. Keys will be assembly name and + private readonly ILog _logger; + // Dictionary that holds the paths to help loading dependencies. Keys will be assembly name and // value are the containing folder. private readonly Dictionary _referencePathFiles = new(StringComparer.OrdinalIgnoreCase); private readonly HashSet _referencePathDirectories = new(StringComparer.OrdinalIgnoreCase); @@ -37,10 +39,12 @@ public class AssemblySymbolLoader : IAssemblySymbolLoader /// /// Creates a new instance of the class. /// + /// A logger instance for logging message. /// True to attempt to load references for loaded assemblies from the locations specified with . Default is false. /// True to include all internal metadata for assemblies loaded. Default is false which only includes public and some internal metadata. - public AssemblySymbolLoader(bool resolveAssemblyReferences = false, bool includeInternalSymbols = false) + public AssemblySymbolLoader(ILog logger, bool resolveAssemblyReferences = false, bool includeInternalSymbols = false) { + _logger = logger; _loadedAssemblies = []; CSharpCompilationOptions compilationOptions = new(OutputKind.DynamicallyLinkedLibrary, nullableContextOptions: NullableContextOptions.Enable, metadataImportOptions: includeInternalSymbols ? MetadataImportOptions.Internal : MetadataImportOptions.Public); @@ -386,5 +390,39 @@ private void ResolveReferences(PEReader peReader, ImmutableHashSet? refe } } } + + /// + public void LogAllDiagnostics(string? headerMessage = null) + { + if (HasRoslynDiagnostics(out IReadOnlyList roslynDiagnostics)) + { + if (!string.IsNullOrEmpty(headerMessage)) + { + _logger.LogWarning(headerMessage!); + } + + foreach (Diagnostic warning in roslynDiagnostics) + { + _logger.LogWarning(warning.Id, warning.ToString()); + } + } + } + + /// + public void LogAllWarnings(string? headerMessage = null) + { + if (HasLoadWarnings(out IReadOnlyList loadWarnings)) + { + if (!string.IsNullOrEmpty(headerMessage)) + { + _logger.LogWarning(headerMessage!); + } + + foreach (AssemblyLoadWarning warning in loadWarnings) + { + _logger.LogWarning(warning.DiagnosticId, warning.Message); + } + } + } } } diff --git a/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblySymbolLoaderFactory.cs b/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblySymbolLoaderFactory.cs index 2df44a418f9f..c0b1c0260778 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblySymbolLoaderFactory.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblySymbolLoaderFactory.cs @@ -1,16 +1,19 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.DotNet.ApiSymbolExtensions.Logging; + namespace Microsoft.DotNet.ApiSymbolExtensions { /// /// Factory to create an AssemblySymbolLoader /// + /// A logger instance used for logging messages. /// True to include internal API when reading assemblies from the created. - public sealed class AssemblySymbolLoaderFactory(bool includeInternalSymbols = false) : IAssemblySymbolLoaderFactory + public sealed class AssemblySymbolLoaderFactory(ILog logger, bool includeInternalSymbols = false) : IAssemblySymbolLoaderFactory { /// public IAssemblySymbolLoader Create(bool shouldResolveReferences) => - new AssemblySymbolLoader(shouldResolveReferences, includeInternalSymbols); + new AssemblySymbolLoader(logger, shouldResolveReferences, includeInternalSymbols); } } diff --git a/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/IAssemblySymbolLoader.cs b/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/IAssemblySymbolLoader.cs index cfb78f0a13d1..06e628bf9ef5 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/IAssemblySymbolLoader.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/IAssemblySymbolLoader.cs @@ -60,7 +60,7 @@ public interface IAssemblySymbolLoader /// /// The name to use to resolve the assembly. /// The stream to read the metadata from. - /// representing the given . If an + /// representing the given . If an /// assembly with the same was already loaded, the previously loaded assembly is returned. IAssemblySymbol? LoadAssembly(string name, Stream stream); @@ -83,6 +83,18 @@ public interface IAssemblySymbolLoader /// The list of matching assemblies represented as . IEnumerable LoadMatchingAssemblies(IEnumerable fromAssemblies, IEnumerable searchPaths, bool validateMatchingIdentity = true, bool warnOnMissingAssemblies = true); + /// + /// Logs all diagnostics that were emitted during the loading of the assemblies. + /// + /// Optional custom message to prepend to the diagnostics. + public void LogAllDiagnostics(string? headerMessage = null); + + /// + /// Logs all warnings that were emitted during the loading of the assemblies. + /// + /// Optional custom message to prepend to the warnings. + public void LogAllWarnings(string? headerMessage = null); + /// /// The list of metadata references represented as . /// diff --git a/test/Microsoft.DotNet.ApiCompat.IntegrationTests/CompatibleFrameworkInPackageValidatorIntegrationTests.cs b/test/Microsoft.DotNet.ApiCompat.IntegrationTests/CompatibleFrameworkInPackageValidatorIntegrationTests.cs index 91a896625fd5..f2c1acde2421 100644 --- a/test/Microsoft.DotNet.ApiCompat.IntegrationTests/CompatibleFrameworkInPackageValidatorIntegrationTests.cs +++ b/test/Microsoft.DotNet.ApiCompat.IntegrationTests/CompatibleFrameworkInPackageValidatorIntegrationTests.cs @@ -27,7 +27,7 @@ public CompatibleFrameworkInPackageValidatorIntegrationTests(ITestOutputHelper l new ApiCompatRunner(log, new SuppressionEngine(), new ApiComparerFactory(new RuleFactory(log)), - new AssemblySymbolLoaderFactory())); + new AssemblySymbolLoaderFactory(log))); return (log, validator); } diff --git a/test/Microsoft.DotNet.ApiCompat.IntegrationTests/Task/ValidatePackageTargetIntegrationTests.cs b/test/Microsoft.DotNet.ApiCompat.IntegrationTests/Task/ValidatePackageTargetIntegrationTests.cs index 6bbb8beb8362..028fddd8d262 100644 --- a/test/Microsoft.DotNet.ApiCompat.IntegrationTests/Task/ValidatePackageTargetIntegrationTests.cs +++ b/test/Microsoft.DotNet.ApiCompat.IntegrationTests/Task/ValidatePackageTargetIntegrationTests.cs @@ -29,7 +29,7 @@ public ValidatePackageTargetIntegrationTests(ITestOutputHelper log) : base(log) new ApiCompatRunner(log, new SuppressionEngine(), new ApiComparerFactory(new RuleFactory(log)), - new AssemblySymbolLoaderFactory())); + new AssemblySymbolLoaderFactory(log))); return (log, validator); } diff --git a/test/Microsoft.DotNet.ApiCompatibility.Tests/Rules/AssemblyIdentityMustMatchTests.cs b/test/Microsoft.DotNet.ApiCompatibility.Tests/Rules/AssemblyIdentityMustMatchTests.cs index ca9fe2ca9a06..ee833c9ba768 100644 --- a/test/Microsoft.DotNet.ApiCompatibility.Tests/Rules/AssemblyIdentityMustMatchTests.cs +++ b/test/Microsoft.DotNet.ApiCompatibility.Tests/Rules/AssemblyIdentityMustMatchTests.cs @@ -7,6 +7,7 @@ using Microsoft.CodeAnalysis.CSharp; using Microsoft.DotNet.ApiCompatibility.Tests; using Microsoft.DotNet.ApiSymbolExtensions; +using Microsoft.DotNet.ApiSymbolExtensions.Logging; using Microsoft.DotNet.ApiSymbolExtensions.Tests; namespace Microsoft.DotNet.ApiCompatibility.Rules.Tests @@ -196,8 +197,10 @@ public void RetargetableFlagSet(bool strictMode) string leftAssembly = SymbolFactory.EmitAssemblyFromSyntax(syntax, publicKey: _publicKey); string rightAssembly = SymbolFactory.EmitAssemblyFromSyntax(syntax); - IAssemblySymbol leftSymbol = new AssemblySymbolLoader().LoadAssembly(leftAssembly); - IAssemblySymbol rightSymbol = new AssemblySymbolLoader().LoadAssembly(rightAssembly); + ILog logger = new ConsoleLog(MessageImportance.High); + + IAssemblySymbol leftSymbol = new AssemblySymbolLoader(logger).LoadAssembly(leftAssembly); + IAssemblySymbol rightSymbol = new AssemblySymbolLoader(logger).LoadAssembly(rightAssembly); Assert.True(leftSymbol.Identity.IsRetargetable); Assert.True(rightSymbol.Identity.IsRetargetable); @@ -210,4 +213,3 @@ public void RetargetableFlagSet(bool strictMode) } } } - diff --git a/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/AssemblySymbolLoaderTests.cs b/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/AssemblySymbolLoaderTests.cs index bb31747720b9..764363f1ec03 100644 --- a/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/AssemblySymbolLoaderTests.cs +++ b/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/AssemblySymbolLoaderTests.cs @@ -6,12 +6,15 @@ using System.Collections.Concurrent; using System.Reflection; using Microsoft.CodeAnalysis; +using Microsoft.DotNet.ApiSymbolExtensions.Logging; using Microsoft.DotNet.Cli.Utils; namespace Microsoft.DotNet.ApiSymbolExtensions.Tests { public class AssemblySymbolLoaderTests : SdkTest { + private readonly ILog _logger = new ConsoleLog(MessageImportance.High); + public AssemblySymbolLoaderTests(ITestOutputHelper log) : base(log) { } private const string SimpleAssemblySourceContents = @" @@ -89,14 +92,14 @@ private TestAssetInfo GetAsset(TestAssetsManager manager) [Fact] public void LoadAssembly_Throws() { - AssemblySymbolLoader loader = new(); + AssemblySymbolLoader loader = new(_logger); Assert.Throws(() => loader.LoadAssembly(Guid.NewGuid().ToString("N").Substring(0, 8))); } [Fact] public void LoadAssemblyFromSourceFiles_Throws() { - AssemblySymbolLoader loader = new(); + AssemblySymbolLoader loader = new(_logger); IEnumerable paths = new[] { Guid.NewGuid().ToString("N") }; Assert.Throws(() => loader.LoadAssemblyFromSourceFiles(paths, "assembly1", Array.Empty())); Assert.Throws("filePaths", () => loader.LoadAssemblyFromSourceFiles(Array.Empty(), "assembly1", Array.Empty())); @@ -106,7 +109,7 @@ public void LoadAssemblyFromSourceFiles_Throws() [Fact] public void LoadMatchingAssemblies_Throws() { - AssemblySymbolLoader loader = new(); + AssemblySymbolLoader loader = new(_logger); IEnumerable paths = new[] { Guid.NewGuid().ToString("N") }; IAssemblySymbol assembly = SymbolFactory.GetAssemblyFromSyntax("namespace MyNamespace { class Foo { } }"); @@ -119,7 +122,7 @@ public void LoadMatchingAssembliesWarns() IAssemblySymbol assembly = SymbolFactory.GetAssemblyFromSyntax("namespace MyNamespace { class Foo { } }"); IEnumerable paths = new[] { AppContext.BaseDirectory }; - AssemblySymbolLoader loader = new(); + AssemblySymbolLoader loader = new(_logger); IEnumerable symbols = loader.LoadMatchingAssemblies(new[] { assembly }, paths); Assert.Empty(symbols); Assert.True(loader.HasLoadWarnings(out IReadOnlyList warnings)); @@ -152,7 +155,7 @@ public void LoadMatchingAssembliesSameIdentitySucceeds() .Should() .Pass(); - AssemblySymbolLoader loader = new(); + AssemblySymbolLoader loader = new(_logger); IEnumerable matchingAssemblies = loader.LoadMatchingAssemblies(new[] { fromAssembly }, new[] { outputDirectory }); Assert.Single(matchingAssemblies); @@ -167,7 +170,7 @@ public void LoadMatchingAssemblies_DifferentIdentity(bool validateIdentities) var assetInfo = GetSimpleTestAsset(); IAssemblySymbol fromAssembly = SymbolFactory.GetAssemblyFromSyntax(SimpleAssemblySourceContents, assemblyName: assetInfo.TestAsset.TestProject.Name); - AssemblySymbolLoader loader = new(); + AssemblySymbolLoader loader = new(_logger); IEnumerable matchingAssemblies = loader.LoadMatchingAssemblies(new[] { fromAssembly }, new[] { assetInfo.OutputDirectory }, validateMatchingIdentity: validateIdentities); if (validateIdentities) @@ -194,7 +197,7 @@ public void LoadMatchingAssemblies_DifferentIdentity(bool validateIdentities) public void LoadsSimpleAssemblyFromDirectory() { var assetInfo = GetSimpleTestAsset(); - AssemblySymbolLoader loader = new(); + AssemblySymbolLoader loader = new(_logger); IEnumerable symbols = loader.LoadAssemblies(assetInfo.OutputDirectory); Assert.Single(symbols); @@ -212,7 +215,7 @@ public void LoadsSimpleAssemblyFromDirectory() public void LoadSimpleAssemblyFullPath() { var assetInfo = GetSimpleTestAsset(); - AssemblySymbolLoader loader = new(); + AssemblySymbolLoader loader = new(_logger); IAssemblySymbol symbol = loader.LoadAssembly(Path.Combine(assetInfo.OutputDirectory, assetInfo.TestAsset.TestProject.Name + ".dll")); IEnumerable types = symbol.GlobalNamespace @@ -246,7 +249,7 @@ public void LoadsMultipleAssembliesFromDirectory() .Should() .Pass(); - AssemblySymbolLoader loader = new(); + AssemblySymbolLoader loader = new(_logger); IEnumerable symbols = loader.LoadAssemblies(outputDirectory); Assert.Equal(2, symbols.Count()); @@ -263,7 +266,7 @@ public void LoadsMultipleAssembliesFromDirectory() public void LoadAssemblyResolveReferences_WarnsWhenEnabled(bool resolveReferences) { var assetInfo = GetSimpleTestAsset(); - AssemblySymbolLoader loader = new(resolveAssemblyReferences: resolveReferences); + AssemblySymbolLoader loader = new(_logger, resolveAssemblyReferences: resolveReferences); loader.LoadAssembly(Path.Combine(assetInfo.OutputDirectory, assetInfo.TestAsset.TestProject.Name + ".dll")); if (resolveReferences) @@ -295,7 +298,7 @@ public void LoadAssemblyResolveReferences_WarnsWhenEnabled(bool resolveReference public void LoadAssembliesShouldResolveReferencesNoWarnings() { var assetInfo = GetSimpleTestAsset(); - AssemblySymbolLoader loader = new(resolveAssemblyReferences: true); + AssemblySymbolLoader loader = new(_logger, resolveAssemblyReferences: true); // AddReferenceSearchDirectories should be able to handle directories as well as full path to assemblies. loader.AddReferenceSearchPaths(Path.GetDirectoryName(typeof(string).Assembly.Location)); loader.AddReferenceSearchPaths(Path.GetFullPath(typeof(string).Assembly.Location)); @@ -314,7 +317,7 @@ public void LoadAssemblyFromStreamNoWarns() { var assetInfo = GetSimpleTestAsset(); TestProject testProject = assetInfo.TestAsset.TestProject; - AssemblySymbolLoader loader = new(); + AssemblySymbolLoader loader = new(_logger); using FileStream stream = File.OpenRead(Path.Combine(assetInfo.OutputDirectory, testProject.Name + ".dll")); IAssemblySymbol symbol = loader.LoadAssembly(testProject.Name, stream); diff --git a/test/Microsoft.DotNet.GenAPI.Tests/CSharpFileBuilderTests.cs b/test/Microsoft.DotNet.GenAPI.Tests/CSharpFileBuilderTests.cs index 55ad81c0c048..ea643d155593 100644 --- a/test/Microsoft.DotNet.GenAPI.Tests/CSharpFileBuilderTests.cs +++ b/test/Microsoft.DotNet.GenAPI.Tests/CSharpFileBuilderTests.cs @@ -54,8 +54,10 @@ private void RunTest(string original, } attributeDataSymbolFilter.Add(accessibilitySymbolFilter); + ILog logger = new ConsoleLog(MessageImportance.Low); + IAssemblySymbolWriter csharpFileBuilder = new CSharpFileBuilder( - new ConsoleLog(MessageImportance.Low), + logger, symbolFilter, attributeDataSymbolFilter, stringWriter, @@ -65,7 +67,7 @@ private void RunTest(string original, addPartialModifier: true); using Stream assemblyStream = SymbolFactory.EmitAssemblyStreamFromSyntax(original, enableNullable: true, allowUnsafe: allowUnsafe, assemblyName: assemblyName); - AssemblySymbolLoader assemblySymbolLoader = new(resolveAssemblyReferences: true, includeInternalSymbols: includeInternalSymbols); + AssemblySymbolLoader assemblySymbolLoader = new(logger, resolveAssemblyReferences: true, includeInternalSymbols: includeInternalSymbols); assemblySymbolLoader.AddReferenceSearchPaths(typeof(object).Assembly!.Location!); assemblySymbolLoader.AddReferenceSearchPaths(typeof(DynamicAttribute).Assembly!.Location!); IAssemblySymbol assemblySymbol = assemblySymbolLoader.LoadAssembly(assemblyName, assemblyStream); From 4117998f38d3ab49906cb3fe16541480276d2363 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20S=C3=A1nchez=20L=C3=B3pez?= <1175054+carlossanlop@users.noreply.github.com> Date: Thu, 9 Jan 2025 11:26:52 -0800 Subject: [PATCH 2/9] Rename logger to log, use mock logger where needed. --- .../Microsoft.DotNet.GenAPI/CSharpFileBuilder.cs | 8 ++++---- .../GenAPI/Microsoft.DotNet.GenAPI/GenAPIApp.cs | 6 +++--- .../AssemblySymbolLoader.cs | 16 ++++++++-------- .../AssemblySymbolLoaderFactory.cs | 6 +++--- .../Rules/AssemblyIdentityMustMatchTests.cs | 10 ++++------ .../SuppressibleTestLog.cs | 2 +- .../AssemblySymbolLoaderTests.cs | 4 ++-- ...osoft.DotNet.ApiSymbolExtensions.Tests.csproj | 1 + .../CSharpFileBuilderTests.cs | 8 ++++---- .../Microsoft.DotNet.GenAPI.Tests.csproj | 2 +- 10 files changed, 31 insertions(+), 32 deletions(-) diff --git a/src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/CSharpFileBuilder.cs b/src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/CSharpFileBuilder.cs index 10cf790e827a..0301672ffe3e 100644 --- a/src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/CSharpFileBuilder.cs +++ b/src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/CSharpFileBuilder.cs @@ -25,7 +25,7 @@ namespace Microsoft.DotNet.GenAPI /// public sealed class CSharpFileBuilder : IAssemblySymbolWriter, IDisposable { - private readonly ILog _logger; + private readonly ILog _log; private readonly TextWriter _textWriter; private readonly ISymbolFilter _symbolFilter; private readonly ISymbolFilter _attributeDataSymbolFilter; @@ -36,7 +36,7 @@ public sealed class CSharpFileBuilder : IAssemblySymbolWriter, IDisposable private readonly IEnumerable _metadataReferences; private readonly bool _addPartialModifier; - public CSharpFileBuilder(ILog logger, + public CSharpFileBuilder(ILog log, ISymbolFilter symbolFilter, ISymbolFilter attributeDataSymbolFilter, TextWriter textWriter, @@ -45,7 +45,7 @@ public CSharpFileBuilder(ILog logger, IEnumerable metadataReferences, bool addPartialModifier) { - _logger = logger; + _log = log; _textWriter = textWriter; _symbolFilter = symbolFilter; _attributeDataSymbolFilter = attributeDataSymbolFilter; @@ -296,7 +296,7 @@ private SyntaxNode GenerateForwardedTypeAssemblyAttributes(IAssemblySymbol assem } else { - _logger.LogWarning(string.Format( + _log.LogWarning(string.Format( Resources.ResolveTypeForwardFailed, symbol.ToDisplayString(), $"{symbol.ContainingAssembly.Name}.dll")); diff --git a/src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/GenAPIApp.cs b/src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/GenAPIApp.cs index f7e751f42b29..bc929232fd2a 100644 --- a/src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/GenAPIApp.cs +++ b/src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/GenAPIApp.cs @@ -21,7 +21,7 @@ public static class GenAPIApp /// /// Initialize and run Roslyn-based GenAPI tool. /// - public static void Run(ILog logger, + public static void Run(ILog log, string[] assemblies, string[]? assemblyReferences, string? outputPath, @@ -35,7 +35,7 @@ public static void Run(ILog logger, bool resolveAssemblyReferences = assemblyReferences?.Length > 0; // Create, configure and execute the assembly loader. - AssemblySymbolLoader loader = new(logger, resolveAssemblyReferences, respectInternals); + AssemblySymbolLoader loader = new(log, resolveAssemblyReferences, respectInternals); if (assemblyReferences is not null) { loader.AddReferenceSearchPaths(assemblyReferences); @@ -75,7 +75,7 @@ public static void Run(ILog logger, using TextWriter textWriter = GetTextWriter(outputPath, assemblySymbol.Name); textWriter.Write(headerFileText); - using CSharpFileBuilder fileBuilder = new(logger, + using CSharpFileBuilder fileBuilder = new(log, symbolFilter, attributeDataSymbolFilter, textWriter, diff --git a/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblySymbolLoader.cs b/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblySymbolLoader.cs index 5d886334fd6b..b3d876778be5 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblySymbolLoader.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblySymbolLoader.cs @@ -16,7 +16,7 @@ namespace Microsoft.DotNet.ApiSymbolExtensions /// public class AssemblySymbolLoader : IAssemblySymbolLoader { - private readonly ILog _logger; + private readonly ILog _log; // Dictionary that holds the paths to help loading dependencies. Keys will be assembly name and // value are the containing folder. private readonly Dictionary _referencePathFiles = new(StringComparer.OrdinalIgnoreCase); @@ -39,12 +39,12 @@ public class AssemblySymbolLoader : IAssemblySymbolLoader /// /// Creates a new instance of the class. /// - /// A logger instance for logging message. + /// A logger instance for logging message. /// True to attempt to load references for loaded assemblies from the locations specified with . Default is false. /// True to include all internal metadata for assemblies loaded. Default is false which only includes public and some internal metadata. - public AssemblySymbolLoader(ILog logger, bool resolveAssemblyReferences = false, bool includeInternalSymbols = false) + public AssemblySymbolLoader(ILog log, bool resolveAssemblyReferences = false, bool includeInternalSymbols = false) { - _logger = logger; + _log = log; _loadedAssemblies = []; CSharpCompilationOptions compilationOptions = new(OutputKind.DynamicallyLinkedLibrary, nullableContextOptions: NullableContextOptions.Enable, metadataImportOptions: includeInternalSymbols ? MetadataImportOptions.Internal : MetadataImportOptions.Public); @@ -398,12 +398,12 @@ public void LogAllDiagnostics(string? headerMessage = null) { if (!string.IsNullOrEmpty(headerMessage)) { - _logger.LogWarning(headerMessage!); + _log.LogWarning(headerMessage!); } foreach (Diagnostic warning in roslynDiagnostics) { - _logger.LogWarning(warning.Id, warning.ToString()); + _log.LogWarning(warning.Id, warning.ToString()); } } } @@ -415,12 +415,12 @@ public void LogAllWarnings(string? headerMessage = null) { if (!string.IsNullOrEmpty(headerMessage)) { - _logger.LogWarning(headerMessage!); + _log.LogWarning(headerMessage!); } foreach (AssemblyLoadWarning warning in loadWarnings) { - _logger.LogWarning(warning.DiagnosticId, warning.Message); + _log.LogWarning(warning.DiagnosticId, warning.Message); } } } diff --git a/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblySymbolLoaderFactory.cs b/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblySymbolLoaderFactory.cs index c0b1c0260778..5a2b74600cc0 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblySymbolLoaderFactory.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblySymbolLoaderFactory.cs @@ -8,12 +8,12 @@ namespace Microsoft.DotNet.ApiSymbolExtensions /// /// Factory to create an AssemblySymbolLoader /// - /// A logger instance used for logging messages. + /// A logger instance used for logging messages. /// True to include internal API when reading assemblies from the created. - public sealed class AssemblySymbolLoaderFactory(ILog logger, bool includeInternalSymbols = false) : IAssemblySymbolLoaderFactory + public sealed class AssemblySymbolLoaderFactory(ILog log, bool includeInternalSymbols = false) : IAssemblySymbolLoaderFactory { /// public IAssemblySymbolLoader Create(bool shouldResolveReferences) => - new AssemblySymbolLoader(logger, shouldResolveReferences, includeInternalSymbols); + new AssemblySymbolLoader(log, shouldResolveReferences, includeInternalSymbols); } } diff --git a/test/Microsoft.DotNet.ApiCompatibility.Tests/Rules/AssemblyIdentityMustMatchTests.cs b/test/Microsoft.DotNet.ApiCompatibility.Tests/Rules/AssemblyIdentityMustMatchTests.cs index ee833c9ba768..4f9e53e5e75c 100644 --- a/test/Microsoft.DotNet.ApiCompatibility.Tests/Rules/AssemblyIdentityMustMatchTests.cs +++ b/test/Microsoft.DotNet.ApiCompatibility.Tests/Rules/AssemblyIdentityMustMatchTests.cs @@ -7,14 +7,14 @@ using Microsoft.CodeAnalysis.CSharp; using Microsoft.DotNet.ApiCompatibility.Tests; using Microsoft.DotNet.ApiSymbolExtensions; -using Microsoft.DotNet.ApiSymbolExtensions.Logging; using Microsoft.DotNet.ApiSymbolExtensions.Tests; namespace Microsoft.DotNet.ApiCompatibility.Rules.Tests { public class AssemblyIdentityMustMatchTests { - private static readonly TestRuleFactory s_ruleFactory = new((settings, context) => new AssemblyIdentityMustMatch(new SuppressibleTestLog(), settings, context)); + private static readonly SuppressibleTestLog s_log = new(); + private static readonly TestRuleFactory s_ruleFactory = new((settings, context) => new AssemblyIdentityMustMatch(s_log, settings, context)); private static readonly byte[] _publicKey = new byte[] { @@ -197,10 +197,8 @@ public void RetargetableFlagSet(bool strictMode) string leftAssembly = SymbolFactory.EmitAssemblyFromSyntax(syntax, publicKey: _publicKey); string rightAssembly = SymbolFactory.EmitAssemblyFromSyntax(syntax); - ILog logger = new ConsoleLog(MessageImportance.High); - - IAssemblySymbol leftSymbol = new AssemblySymbolLoader(logger).LoadAssembly(leftAssembly); - IAssemblySymbol rightSymbol = new AssemblySymbolLoader(logger).LoadAssembly(rightAssembly); + IAssemblySymbol leftSymbol = new AssemblySymbolLoader(s_log).LoadAssembly(leftAssembly); + IAssemblySymbol rightSymbol = new AssemblySymbolLoader(s_log).LoadAssembly(rightAssembly); Assert.True(leftSymbol.Identity.IsRetargetable); Assert.True(rightSymbol.Identity.IsRetargetable); diff --git a/test/Microsoft.DotNet.ApiCompatibility.Tests/SuppressibleTestLog.cs b/test/Microsoft.DotNet.ApiCompatibility.Tests/SuppressibleTestLog.cs index 798c8c32adda..bafa1eac7044 100644 --- a/test/Microsoft.DotNet.ApiCompatibility.Tests/SuppressibleTestLog.cs +++ b/test/Microsoft.DotNet.ApiCompatibility.Tests/SuppressibleTestLog.cs @@ -6,7 +6,7 @@ namespace Microsoft.DotNet.ApiCompatibility.Tests { - internal class SuppressibleTestLog : ISuppressibleLog + public class SuppressibleTestLog : ISuppressibleLog { public List info = []; public List errors = []; diff --git a/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/AssemblySymbolLoaderTests.cs b/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/AssemblySymbolLoaderTests.cs index 764363f1ec03..95de07287ff2 100644 --- a/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/AssemblySymbolLoaderTests.cs +++ b/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/AssemblySymbolLoaderTests.cs @@ -6,14 +6,14 @@ using System.Collections.Concurrent; using System.Reflection; using Microsoft.CodeAnalysis; -using Microsoft.DotNet.ApiSymbolExtensions.Logging; +using Microsoft.DotNet.ApiCompatibility.Tests; using Microsoft.DotNet.Cli.Utils; namespace Microsoft.DotNet.ApiSymbolExtensions.Tests { public class AssemblySymbolLoaderTests : SdkTest { - private readonly ILog _logger = new ConsoleLog(MessageImportance.High); + private readonly SuppressibleTestLog _logger = new(); public AssemblySymbolLoaderTests(ITestOutputHelper log) : base(log) { } diff --git a/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/Microsoft.DotNet.ApiSymbolExtensions.Tests.csproj b/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/Microsoft.DotNet.ApiSymbolExtensions.Tests.csproj index 9cb2cadbe2ed..5602a7825845 100644 --- a/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/Microsoft.DotNet.ApiSymbolExtensions.Tests.csproj +++ b/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/Microsoft.DotNet.ApiSymbolExtensions.Tests.csproj @@ -7,6 +7,7 @@ + diff --git a/test/Microsoft.DotNet.GenAPI.Tests/CSharpFileBuilderTests.cs b/test/Microsoft.DotNet.GenAPI.Tests/CSharpFileBuilderTests.cs index ea643d155593..c7b61312fe11 100644 --- a/test/Microsoft.DotNet.GenAPI.Tests/CSharpFileBuilderTests.cs +++ b/test/Microsoft.DotNet.GenAPI.Tests/CSharpFileBuilderTests.cs @@ -8,7 +8,7 @@ using Microsoft.CodeAnalysis.CSharp; using Microsoft.DotNet.ApiSymbolExtensions; using Microsoft.DotNet.ApiSymbolExtensions.Filtering; -using Microsoft.DotNet.ApiSymbolExtensions.Logging; +using Microsoft.DotNet.ApiCompatibility.Tests; using Microsoft.DotNet.ApiSymbolExtensions.Tests; using Microsoft.DotNet.GenAPI.Filtering; @@ -54,10 +54,10 @@ private void RunTest(string original, } attributeDataSymbolFilter.Add(accessibilitySymbolFilter); - ILog logger = new ConsoleLog(MessageImportance.Low); + SuppressibleTestLog log = new(); IAssemblySymbolWriter csharpFileBuilder = new CSharpFileBuilder( - logger, + log, symbolFilter, attributeDataSymbolFilter, stringWriter, @@ -67,7 +67,7 @@ private void RunTest(string original, addPartialModifier: true); using Stream assemblyStream = SymbolFactory.EmitAssemblyStreamFromSyntax(original, enableNullable: true, allowUnsafe: allowUnsafe, assemblyName: assemblyName); - AssemblySymbolLoader assemblySymbolLoader = new(logger, resolveAssemblyReferences: true, includeInternalSymbols: includeInternalSymbols); + AssemblySymbolLoader assemblySymbolLoader = new(log, resolveAssemblyReferences: true, includeInternalSymbols: includeInternalSymbols); assemblySymbolLoader.AddReferenceSearchPaths(typeof(object).Assembly!.Location!); assemblySymbolLoader.AddReferenceSearchPaths(typeof(DynamicAttribute).Assembly!.Location!); IAssemblySymbol assemblySymbol = assemblySymbolLoader.LoadAssembly(assemblyName, assemblyStream); diff --git a/test/Microsoft.DotNet.GenAPI.Tests/Microsoft.DotNet.GenAPI.Tests.csproj b/test/Microsoft.DotNet.GenAPI.Tests/Microsoft.DotNet.GenAPI.Tests.csproj index 82da898e0bb7..e553c0c19ea1 100644 --- a/test/Microsoft.DotNet.GenAPI.Tests/Microsoft.DotNet.GenAPI.Tests.csproj +++ b/test/Microsoft.DotNet.GenAPI.Tests/Microsoft.DotNet.GenAPI.Tests.csproj @@ -12,10 +12,10 @@ - + From 5449e93573e547df1fea72ae805d2ce2169626a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20S=C3=A1nchez=20L=C3=B3pez?= <1175054+carlossanlop@users.noreply.github.com> Date: Thu, 9 Jan 2025 12:02:56 -0800 Subject: [PATCH 3/9] Mock it all --- .../SuppressibleTestLog.cs | 2 +- .../AssemblySymbolLoaderTests.cs | 29 ++++++++++--------- .../CSharpFileBuilderTests.cs | 9 +++--- .../Microsoft.DotNet.GenAPI.Tests.csproj | 2 +- 4 files changed, 22 insertions(+), 20 deletions(-) diff --git a/test/Microsoft.DotNet.ApiCompatibility.Tests/SuppressibleTestLog.cs b/test/Microsoft.DotNet.ApiCompatibility.Tests/SuppressibleTestLog.cs index bafa1eac7044..798c8c32adda 100644 --- a/test/Microsoft.DotNet.ApiCompatibility.Tests/SuppressibleTestLog.cs +++ b/test/Microsoft.DotNet.ApiCompatibility.Tests/SuppressibleTestLog.cs @@ -6,7 +6,7 @@ namespace Microsoft.DotNet.ApiCompatibility.Tests { - public class SuppressibleTestLog : ISuppressibleLog + internal class SuppressibleTestLog : ISuppressibleLog { public List info = []; public List errors = []; diff --git a/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/AssemblySymbolLoaderTests.cs b/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/AssemblySymbolLoaderTests.cs index 95de07287ff2..4a6929790d45 100644 --- a/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/AssemblySymbolLoaderTests.cs +++ b/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/AssemblySymbolLoaderTests.cs @@ -6,14 +6,15 @@ using System.Collections.Concurrent; using System.Reflection; using Microsoft.CodeAnalysis; -using Microsoft.DotNet.ApiCompatibility.Tests; +using Microsoft.DotNet.ApiSymbolExtensions.Logging; using Microsoft.DotNet.Cli.Utils; +using Moq; namespace Microsoft.DotNet.ApiSymbolExtensions.Tests { public class AssemblySymbolLoaderTests : SdkTest { - private readonly SuppressibleTestLog _logger = new(); + private readonly Mock _log = new(); public AssemblySymbolLoaderTests(ITestOutputHelper log) : base(log) { } @@ -92,14 +93,14 @@ private TestAssetInfo GetAsset(TestAssetsManager manager) [Fact] public void LoadAssembly_Throws() { - AssemblySymbolLoader loader = new(_logger); + AssemblySymbolLoader loader = new(_log.Object); Assert.Throws(() => loader.LoadAssembly(Guid.NewGuid().ToString("N").Substring(0, 8))); } [Fact] public void LoadAssemblyFromSourceFiles_Throws() { - AssemblySymbolLoader loader = new(_logger); + AssemblySymbolLoader loader = new(_log.Object); IEnumerable paths = new[] { Guid.NewGuid().ToString("N") }; Assert.Throws(() => loader.LoadAssemblyFromSourceFiles(paths, "assembly1", Array.Empty())); Assert.Throws("filePaths", () => loader.LoadAssemblyFromSourceFiles(Array.Empty(), "assembly1", Array.Empty())); @@ -109,7 +110,7 @@ public void LoadAssemblyFromSourceFiles_Throws() [Fact] public void LoadMatchingAssemblies_Throws() { - AssemblySymbolLoader loader = new(_logger); + AssemblySymbolLoader loader = new(_log.Object); IEnumerable paths = new[] { Guid.NewGuid().ToString("N") }; IAssemblySymbol assembly = SymbolFactory.GetAssemblyFromSyntax("namespace MyNamespace { class Foo { } }"); @@ -122,7 +123,7 @@ public void LoadMatchingAssembliesWarns() IAssemblySymbol assembly = SymbolFactory.GetAssemblyFromSyntax("namespace MyNamespace { class Foo { } }"); IEnumerable paths = new[] { AppContext.BaseDirectory }; - AssemblySymbolLoader loader = new(_logger); + AssemblySymbolLoader loader = new(_log.Object); IEnumerable symbols = loader.LoadMatchingAssemblies(new[] { assembly }, paths); Assert.Empty(symbols); Assert.True(loader.HasLoadWarnings(out IReadOnlyList warnings)); @@ -155,7 +156,7 @@ public void LoadMatchingAssembliesSameIdentitySucceeds() .Should() .Pass(); - AssemblySymbolLoader loader = new(_logger); + AssemblySymbolLoader loader = new(_log.Object); IEnumerable matchingAssemblies = loader.LoadMatchingAssemblies(new[] { fromAssembly }, new[] { outputDirectory }); Assert.Single(matchingAssemblies); @@ -170,7 +171,7 @@ public void LoadMatchingAssemblies_DifferentIdentity(bool validateIdentities) var assetInfo = GetSimpleTestAsset(); IAssemblySymbol fromAssembly = SymbolFactory.GetAssemblyFromSyntax(SimpleAssemblySourceContents, assemblyName: assetInfo.TestAsset.TestProject.Name); - AssemblySymbolLoader loader = new(_logger); + AssemblySymbolLoader loader = new(_log.Object); IEnumerable matchingAssemblies = loader.LoadMatchingAssemblies(new[] { fromAssembly }, new[] { assetInfo.OutputDirectory }, validateMatchingIdentity: validateIdentities); if (validateIdentities) @@ -197,7 +198,7 @@ public void LoadMatchingAssemblies_DifferentIdentity(bool validateIdentities) public void LoadsSimpleAssemblyFromDirectory() { var assetInfo = GetSimpleTestAsset(); - AssemblySymbolLoader loader = new(_logger); + AssemblySymbolLoader loader = new(_log.Object); IEnumerable symbols = loader.LoadAssemblies(assetInfo.OutputDirectory); Assert.Single(symbols); @@ -215,7 +216,7 @@ public void LoadsSimpleAssemblyFromDirectory() public void LoadSimpleAssemblyFullPath() { var assetInfo = GetSimpleTestAsset(); - AssemblySymbolLoader loader = new(_logger); + AssemblySymbolLoader loader = new(_log.Object); IAssemblySymbol symbol = loader.LoadAssembly(Path.Combine(assetInfo.OutputDirectory, assetInfo.TestAsset.TestProject.Name + ".dll")); IEnumerable types = symbol.GlobalNamespace @@ -249,7 +250,7 @@ public void LoadsMultipleAssembliesFromDirectory() .Should() .Pass(); - AssemblySymbolLoader loader = new(_logger); + AssemblySymbolLoader loader = new(_log.Object); IEnumerable symbols = loader.LoadAssemblies(outputDirectory); Assert.Equal(2, symbols.Count()); @@ -266,7 +267,7 @@ public void LoadsMultipleAssembliesFromDirectory() public void LoadAssemblyResolveReferences_WarnsWhenEnabled(bool resolveReferences) { var assetInfo = GetSimpleTestAsset(); - AssemblySymbolLoader loader = new(_logger, resolveAssemblyReferences: resolveReferences); + AssemblySymbolLoader loader = new(_log.Object, resolveAssemblyReferences: resolveReferences); loader.LoadAssembly(Path.Combine(assetInfo.OutputDirectory, assetInfo.TestAsset.TestProject.Name + ".dll")); if (resolveReferences) @@ -298,7 +299,7 @@ public void LoadAssemblyResolveReferences_WarnsWhenEnabled(bool resolveReference public void LoadAssembliesShouldResolveReferencesNoWarnings() { var assetInfo = GetSimpleTestAsset(); - AssemblySymbolLoader loader = new(_logger, resolveAssemblyReferences: true); + AssemblySymbolLoader loader = new(_log.Object, resolveAssemblyReferences: true); // AddReferenceSearchDirectories should be able to handle directories as well as full path to assemblies. loader.AddReferenceSearchPaths(Path.GetDirectoryName(typeof(string).Assembly.Location)); loader.AddReferenceSearchPaths(Path.GetFullPath(typeof(string).Assembly.Location)); @@ -317,7 +318,7 @@ public void LoadAssemblyFromStreamNoWarns() { var assetInfo = GetSimpleTestAsset(); TestProject testProject = assetInfo.TestAsset.TestProject; - AssemblySymbolLoader loader = new(_logger); + AssemblySymbolLoader loader = new(_log.Object); using FileStream stream = File.OpenRead(Path.Combine(assetInfo.OutputDirectory, testProject.Name + ".dll")); IAssemblySymbol symbol = loader.LoadAssembly(testProject.Name, stream); diff --git a/test/Microsoft.DotNet.GenAPI.Tests/CSharpFileBuilderTests.cs b/test/Microsoft.DotNet.GenAPI.Tests/CSharpFileBuilderTests.cs index c7b61312fe11..42603c25a1c9 100644 --- a/test/Microsoft.DotNet.GenAPI.Tests/CSharpFileBuilderTests.cs +++ b/test/Microsoft.DotNet.GenAPI.Tests/CSharpFileBuilderTests.cs @@ -8,9 +8,10 @@ using Microsoft.CodeAnalysis.CSharp; using Microsoft.DotNet.ApiSymbolExtensions; using Microsoft.DotNet.ApiSymbolExtensions.Filtering; -using Microsoft.DotNet.ApiCompatibility.Tests; +using Microsoft.DotNet.ApiSymbolExtensions.Logging; using Microsoft.DotNet.ApiSymbolExtensions.Tests; using Microsoft.DotNet.GenAPI.Filtering; +using Moq; namespace Microsoft.DotNet.GenAPI.Tests { @@ -54,10 +55,10 @@ private void RunTest(string original, } attributeDataSymbolFilter.Add(accessibilitySymbolFilter); - SuppressibleTestLog log = new(); + Mock log = new(); IAssemblySymbolWriter csharpFileBuilder = new CSharpFileBuilder( - log, + log.Object, symbolFilter, attributeDataSymbolFilter, stringWriter, @@ -67,7 +68,7 @@ private void RunTest(string original, addPartialModifier: true); using Stream assemblyStream = SymbolFactory.EmitAssemblyStreamFromSyntax(original, enableNullable: true, allowUnsafe: allowUnsafe, assemblyName: assemblyName); - AssemblySymbolLoader assemblySymbolLoader = new(log, resolveAssemblyReferences: true, includeInternalSymbols: includeInternalSymbols); + AssemblySymbolLoader assemblySymbolLoader = new(log.Object, resolveAssemblyReferences: true, includeInternalSymbols: includeInternalSymbols); assemblySymbolLoader.AddReferenceSearchPaths(typeof(object).Assembly!.Location!); assemblySymbolLoader.AddReferenceSearchPaths(typeof(DynamicAttribute).Assembly!.Location!); IAssemblySymbol assemblySymbol = assemblySymbolLoader.LoadAssembly(assemblyName, assemblyStream); diff --git a/test/Microsoft.DotNet.GenAPI.Tests/Microsoft.DotNet.GenAPI.Tests.csproj b/test/Microsoft.DotNet.GenAPI.Tests/Microsoft.DotNet.GenAPI.Tests.csproj index e553c0c19ea1..82da898e0bb7 100644 --- a/test/Microsoft.DotNet.GenAPI.Tests/Microsoft.DotNet.GenAPI.Tests.csproj +++ b/test/Microsoft.DotNet.GenAPI.Tests/Microsoft.DotNet.GenAPI.Tests.csproj @@ -12,10 +12,10 @@ + - From fc939dffbbb29d19166138254a8b2c5aa3a7e652 Mon Sep 17 00:00:00 2001 From: Viktor Hofer Date: Thu, 9 Jan 2025 20:24:25 +0100 Subject: [PATCH 4/9] AssemblySymbolLoader changes for warnings and diagnostics. Author: Viktor Hofer --- .../Microsoft.DotNet.GenAPI/GenAPIApp.cs | 3 - .../AssemblyLoadWarning.cs | 49 ------------ .../AssemblySymbolLoader.cs | 75 ++++++------------- .../IAssemblySymbolLoader.cs | 27 ------- 4 files changed, 22 insertions(+), 132 deletions(-) delete mode 100644 src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblyLoadWarning.cs diff --git a/src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/GenAPIApp.cs b/src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/GenAPIApp.cs index bc929232fd2a..1e1bb55400dd 100644 --- a/src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/GenAPIApp.cs +++ b/src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/GenAPIApp.cs @@ -86,9 +86,6 @@ public static void Run(ILog log, fileBuilder.WriteAssembly(assemblySymbol); } - - loader.LogAllDiagnostics(); - loader.LogAllWarnings(); } // Creates a TextWriter capable of writing into Console or a cs file. diff --git a/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblyLoadWarning.cs b/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblyLoadWarning.cs deleted file mode 100644 index 4a8d187b8923..000000000000 --- a/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblyLoadWarning.cs +++ /dev/null @@ -1,49 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace Microsoft.DotNet.ApiSymbolExtensions -{ - /// - /// Class that represents a warning that occurred while trying to load a specific assembly. - /// - /// String representing the diagnostic ID. - /// String representing the ID for the object that the diagnostic was created for. - /// String describing the diagnostic. - public class AssemblyLoadWarning(string diagnosticId, string referenceId, string message) : IDiagnostic, IEquatable - { - private readonly StringComparer _ordinalComparer = StringComparer.Ordinal; - - /// - public string DiagnosticId { get; } = diagnosticId; - - /// - public string ReferenceId { get; } = referenceId; - - /// - public string Message { get; } = message; - - /// - public bool Equals(AssemblyLoadWarning? other) => other != null && - _ordinalComparer.Equals(DiagnosticId, other.DiagnosticId) && - _ordinalComparer.Equals(ReferenceId, other.ReferenceId) && - _ordinalComparer.Equals(Message, other.Message); - - /// - public override bool Equals(object? obj) => - obj is AssemblyLoadWarning assemblyLoadWarning && Equals(assemblyLoadWarning); - - /// - public override int GetHashCode() - { -#if NET - return HashCode.Combine(DiagnosticId, ReferenceId, Message); -#else - int hashCode = 1447485498; - hashCode = hashCode * -1521134295 + EqualityComparer.Default.GetHashCode(DiagnosticId); - hashCode = hashCode * -1521134295 + EqualityComparer.Default.GetHashCode(ReferenceId); - hashCode = hashCode * -1521134295 + EqualityComparer.Default.GetHashCode(Message); - return hashCode; -#endif - } - } -} diff --git a/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblySymbolLoader.cs b/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblySymbolLoader.cs index b3d876778be5..1d6921dd1f92 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblySymbolLoader.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/AssemblySymbolLoader.cs @@ -21,7 +21,6 @@ public class AssemblySymbolLoader : IAssemblySymbolLoader // value are the containing folder. private readonly Dictionary _referencePathFiles = new(StringComparer.OrdinalIgnoreCase); private readonly HashSet _referencePathDirectories = new(StringComparer.OrdinalIgnoreCase); - private readonly List _warnings = []; private readonly Dictionary _loadedAssemblies; private readonly bool _resolveReferences; private CSharpCompilation _cSharpCompilation; @@ -77,20 +76,6 @@ public void AddReferenceSearchPaths(params string[] paths) } } - /// - public bool HasRoslynDiagnostics(out IReadOnlyList diagnostics) - { - diagnostics = _cSharpCompilation.GetDiagnostics(); - return diagnostics.Count > 0; - } - - /// - public bool HasLoadWarnings(out IReadOnlyList warnings) - { - warnings = _warnings; - return _warnings.Count > 0; - } - /// public IReadOnlyList LoadAssemblies(params string[] paths) { @@ -110,6 +95,8 @@ public bool HasLoadWarnings(out IReadOnlyList warnings) assemblySymbols[i] = symbol as IAssemblySymbol; } + LogCompilationDiagnostics(); + return assemblySymbols; } @@ -159,6 +146,8 @@ public bool HasLoadWarnings(out IReadOnlyList warnings) null; } + LogCompilationDiagnostics(); + return assemblySymbols; } @@ -166,7 +155,10 @@ public bool HasLoadWarnings(out IReadOnlyList warnings) public IAssemblySymbol? LoadAssembly(string path) { MetadataReference metadataReference = CreateOrGetMetadataReferenceFromPath(path); - return _cSharpCompilation.GetAssemblyOrModuleSymbol(metadataReference) as IAssemblySymbol; + IAssemblySymbol? assemblySymbol = _cSharpCompilation.GetAssemblyOrModuleSymbol(metadataReference) as IAssemblySymbol; + LogCompilationDiagnostics(); + + return assemblySymbol; } /// @@ -182,7 +174,10 @@ public bool HasLoadWarnings(out IReadOnlyList warnings) metadataReference = CreateAndAddReferenceToCompilation(name, stream); } - return _cSharpCompilation.GetAssemblyOrModuleSymbol(metadataReference) as IAssemblySymbol; + IAssemblySymbol? assemblySymbol = _cSharpCompilation.GetAssemblyOrModuleSymbol(metadataReference) as IAssemblySymbol; + LogCompilationDiagnostics(); + + return assemblySymbol; } /// @@ -209,6 +204,8 @@ public IAssemblySymbol LoadAssemblyFromSourceFiles(IEnumerable filePaths _cSharpCompilation = _cSharpCompilation.AddSyntaxTrees(syntaxTrees); LoadFromPaths(referencePaths); + LogCompilationDiagnostics(); + return _cSharpCompilation.Assembly; } @@ -251,12 +248,12 @@ public IEnumerable LoadMatchingAssemblies(IEnumerable? refe if (!found) { - _warnings.Add(new AssemblyLoadWarning( - AssemblyReferenceNotFoundErrorCode, - name, - string.Format(Resources.CouldNotResolveReference, name))); + _log.LogWarning(AssemblyReferenceNotFoundErrorCode, string.Format(Resources.CouldNotResolveReference, name)); } } } } - /// - public void LogAllDiagnostics(string? headerMessage = null) + private void LogCompilationDiagnostics() { - if (HasRoslynDiagnostics(out IReadOnlyList roslynDiagnostics)) + var diagnostics = _cSharpCompilation.GetDiagnostics(); + foreach (Diagnostic warning in diagnostics) { - if (!string.IsNullOrEmpty(headerMessage)) - { - _log.LogWarning(headerMessage!); - } - - foreach (Diagnostic warning in roslynDiagnostics) - { - _log.LogWarning(warning.Id, warning.ToString()); - } - } - } - - /// - public void LogAllWarnings(string? headerMessage = null) - { - if (HasLoadWarnings(out IReadOnlyList loadWarnings)) - { - if (!string.IsNullOrEmpty(headerMessage)) - { - _log.LogWarning(headerMessage!); - } - - foreach (AssemblyLoadWarning warning in loadWarnings) - { - _log.LogWarning(warning.DiagnosticId, warning.Message); - } + _log.LogMessage(MessageImportance.Normal, warning.ToString()); } } } diff --git a/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/IAssemblySymbolLoader.cs b/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/IAssemblySymbolLoader.cs index 06e628bf9ef5..161ead0aea87 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/IAssemblySymbolLoader.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiSymbolExtensions/IAssemblySymbolLoader.cs @@ -18,21 +18,6 @@ public interface IAssemblySymbolLoader /// The list of paths to register as search directories. void AddReferenceSearchPaths(params string[] paths); - /// - /// Indicates if the compilation used to resolve binaries has any roslyn diagnostics. - /// Might be useful when loading an assembly from source files. - /// - /// List of diagnostics. - /// True if there are any diagnostics, false otherwise. - bool HasRoslynDiagnostics(out IReadOnlyList diagnostics); - - /// - /// Indicates if the loader emitted any warnings that might affect the assembly resolution. - /// - /// List of warnings. - /// True if there are any warnings, false otherwise. - bool HasLoadWarnings(out IReadOnlyList warnings); - /// /// Loads a list of assemblies and gets its corresponding from the specified paths. /// @@ -83,18 +68,6 @@ public interface IAssemblySymbolLoader /// The list of matching assemblies represented as . IEnumerable LoadMatchingAssemblies(IEnumerable fromAssemblies, IEnumerable searchPaths, bool validateMatchingIdentity = true, bool warnOnMissingAssemblies = true); - /// - /// Logs all diagnostics that were emitted during the loading of the assemblies. - /// - /// Optional custom message to prepend to the diagnostics. - public void LogAllDiagnostics(string? headerMessage = null); - - /// - /// Logs all warnings that were emitted during the loading of the assemblies. - /// - /// Optional custom message to prepend to the warnings. - public void LogAllWarnings(string? headerMessage = null); - /// /// The list of metadata references represented as . /// From 0998075596c547635561f1a7a520a9aa523b26cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20S=C3=A1nchez=20L=C3=B3pez?= <1175054+carlossanlop@users.noreply.github.com> Date: Thu, 9 Jan 2025 13:29:41 -0800 Subject: [PATCH 5/9] Add TestLog class and use it to verify warnings in AssemblySymbolLoaderTests. --- .../AssemblySymbolLoaderTests.cs | 83 +++++++++---------- .../TestLog.cs | 25 ++++++ 2 files changed, 63 insertions(+), 45 deletions(-) create mode 100644 test/Microsoft.DotNet.ApiSymbolExtensions.Tests/TestLog.cs diff --git a/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/AssemblySymbolLoaderTests.cs b/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/AssemblySymbolLoaderTests.cs index 4a6929790d45..45c4deca7e0b 100644 --- a/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/AssemblySymbolLoaderTests.cs +++ b/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/AssemblySymbolLoaderTests.cs @@ -6,15 +6,12 @@ using System.Collections.Concurrent; using System.Reflection; using Microsoft.CodeAnalysis; -using Microsoft.DotNet.ApiSymbolExtensions.Logging; using Microsoft.DotNet.Cli.Utils; -using Moq; namespace Microsoft.DotNet.ApiSymbolExtensions.Tests { public class AssemblySymbolLoaderTests : SdkTest { - private readonly Mock _log = new(); public AssemblySymbolLoaderTests(ITestOutputHelper log) : base(log) { } @@ -93,14 +90,16 @@ private TestAssetInfo GetAsset(TestAssetsManager manager) [Fact] public void LoadAssembly_Throws() { - AssemblySymbolLoader loader = new(_log.Object); + TestLog log = new(); + AssemblySymbolLoader loader = new(log); Assert.Throws(() => loader.LoadAssembly(Guid.NewGuid().ToString("N").Substring(0, 8))); } [Fact] public void LoadAssemblyFromSourceFiles_Throws() { - AssemblySymbolLoader loader = new(_log.Object); + TestLog log = new(); + AssemblySymbolLoader loader = new(log); IEnumerable paths = new[] { Guid.NewGuid().ToString("N") }; Assert.Throws(() => loader.LoadAssemblyFromSourceFiles(paths, "assembly1", Array.Empty())); Assert.Throws("filePaths", () => loader.LoadAssemblyFromSourceFiles(Array.Empty(), "assembly1", Array.Empty())); @@ -110,7 +109,8 @@ public void LoadAssemblyFromSourceFiles_Throws() [Fact] public void LoadMatchingAssemblies_Throws() { - AssemblySymbolLoader loader = new(_log.Object); + TestLog log = new(); + AssemblySymbolLoader loader = new(log); IEnumerable paths = new[] { Guid.NewGuid().ToString("N") }; IAssemblySymbol assembly = SymbolFactory.GetAssemblyFromSyntax("namespace MyNamespace { class Foo { } }"); @@ -123,17 +123,13 @@ public void LoadMatchingAssembliesWarns() IAssemblySymbol assembly = SymbolFactory.GetAssemblyFromSyntax("namespace MyNamespace { class Foo { } }"); IEnumerable paths = new[] { AppContext.BaseDirectory }; - AssemblySymbolLoader loader = new(_log.Object); + TestLog log = new(); + AssemblySymbolLoader loader = new(log); IEnumerable symbols = loader.LoadMatchingAssemblies(new[] { assembly }, paths); Assert.Empty(symbols); - Assert.True(loader.HasLoadWarnings(out IReadOnlyList warnings)); - - IEnumerable expected = new[] - { - new AssemblyLoadWarning(AssemblySymbolLoader.AssemblyNotFoundErrorCode, assembly.Identity.GetDisplayName(), $"Could not find matching assembly: '{assembly.Identity.GetDisplayName()}' in any of the search directories.") - }; - - Assert.Equal(expected, warnings); + Assert.True(log.HasLoggedWarnings); + List expected = [ $"{AssemblySymbolLoader.AssemblyNotFoundErrorCode} Could not find matching assembly: '{assembly.Identity.GetDisplayName()}' in any of the search directories." ]; + Assert.Equal(expected, log.Warnings, StringComparer.CurrentCultureIgnoreCase); } [Fact] @@ -156,11 +152,12 @@ public void LoadMatchingAssembliesSameIdentitySucceeds() .Should() .Pass(); - AssemblySymbolLoader loader = new(_log.Object); + TestLog log = new(); + AssemblySymbolLoader loader = new(log); IEnumerable matchingAssemblies = loader.LoadMatchingAssemblies(new[] { fromAssembly }, new[] { outputDirectory }); Assert.Single(matchingAssemblies); - Assert.False(loader.HasLoadWarnings(out var _)); + Assert.False(log.HasLoggedWarnings); } [Theory] @@ -171,25 +168,21 @@ public void LoadMatchingAssemblies_DifferentIdentity(bool validateIdentities) var assetInfo = GetSimpleTestAsset(); IAssemblySymbol fromAssembly = SymbolFactory.GetAssemblyFromSyntax(SimpleAssemblySourceContents, assemblyName: assetInfo.TestAsset.TestProject.Name); - AssemblySymbolLoader loader = new(_log.Object); + TestLog log = new(); + AssemblySymbolLoader loader = new(log); IEnumerable matchingAssemblies = loader.LoadMatchingAssemblies(new[] { fromAssembly }, new[] { assetInfo.OutputDirectory }, validateMatchingIdentity: validateIdentities); if (validateIdentities) { Assert.Empty(matchingAssemblies); - Assert.True(loader.HasLoadWarnings(out IReadOnlyList warnings)); - - IEnumerable expected = new[] - { - new AssemblyLoadWarning(AssemblySymbolLoader.AssemblyNotFoundErrorCode, fromAssembly.Identity.GetDisplayName(), $"Could not find matching assembly: '{fromAssembly.Identity.GetDisplayName()}' in any of the search directories.") - }; - - Assert.Equal(expected, warnings); + Assert.True(log.HasLoggedWarnings); + List expected = [$"{AssemblySymbolLoader.AssemblyNotFoundErrorCode} Could not find matching assembly: '{fromAssembly.Identity.GetDisplayName()}' in any of the search directories."]; + Assert.Equal(expected, log.Warnings, StringComparer.CurrentCultureIgnoreCase); } else { Assert.Single(matchingAssemblies); - Assert.False(loader.HasLoadWarnings(out var _)); + Assert.False(log.HasLoggedWarnings); Assert.NotEqual(fromAssembly.Identity, matchingAssemblies.FirstOrDefault().Identity); } } @@ -198,7 +191,8 @@ public void LoadMatchingAssemblies_DifferentIdentity(bool validateIdentities) public void LoadsSimpleAssemblyFromDirectory() { var assetInfo = GetSimpleTestAsset(); - AssemblySymbolLoader loader = new(_log.Object); + TestLog log = new(); + AssemblySymbolLoader loader = new(log); IEnumerable symbols = loader.LoadAssemblies(assetInfo.OutputDirectory); Assert.Single(symbols); @@ -216,7 +210,8 @@ public void LoadsSimpleAssemblyFromDirectory() public void LoadSimpleAssemblyFullPath() { var assetInfo = GetSimpleTestAsset(); - AssemblySymbolLoader loader = new(_log.Object); + TestLog log = new(); + AssemblySymbolLoader loader = new(log); IAssemblySymbol symbol = loader.LoadAssembly(Path.Combine(assetInfo.OutputDirectory, assetInfo.TestAsset.TestProject.Name + ".dll")); IEnumerable types = symbol.GlobalNamespace @@ -250,7 +245,8 @@ public void LoadsMultipleAssembliesFromDirectory() .Should() .Pass(); - AssemblySymbolLoader loader = new(_log.Object); + TestLog log = new(); + AssemblySymbolLoader loader = new(log); IEnumerable symbols = loader.LoadAssemblies(outputDirectory); Assert.Equal(2, symbols.Count()); @@ -267,12 +263,13 @@ public void LoadsMultipleAssembliesFromDirectory() public void LoadAssemblyResolveReferences_WarnsWhenEnabled(bool resolveReferences) { var assetInfo = GetSimpleTestAsset(); - AssemblySymbolLoader loader = new(_log.Object, resolveAssemblyReferences: resolveReferences); + TestLog log = new(); + AssemblySymbolLoader loader = new(log, resolveAssemblyReferences: resolveReferences); loader.LoadAssembly(Path.Combine(assetInfo.OutputDirectory, assetInfo.TestAsset.TestProject.Name + ".dll")); if (resolveReferences) { - Assert.True(loader.HasLoadWarnings(out IReadOnlyList warnings)); + Assert.True(log.HasLoggedWarnings); string expectedReference = "System.Runtime.dll"; @@ -281,17 +278,12 @@ public void LoadAssemblyResolveReferences_WarnsWhenEnabled(bool resolveReference expectedReference = "mscorlib.dll"; } - IEnumerable expected = new List - { - new AssemblyLoadWarning(AssemblySymbolLoader.AssemblyReferenceNotFoundErrorCode, expectedReference, $"Could not resolve reference '{expectedReference}' in any of the provided search directories.") - }; - - Assert.Equal(expected, warnings); + List expected = [$"{AssemblySymbolLoader.AssemblyReferenceNotFoundErrorCode} Could not resolve reference '{expectedReference}' in any of the provided search directories."]; + Assert.Equal(expected, log.Warnings, StringComparer.CurrentCultureIgnoreCase); } else { - Assert.False(loader.HasLoadWarnings(out IReadOnlyList warnings)); - Assert.Empty(warnings); + Assert.Empty(log.Warnings); } } @@ -299,14 +291,14 @@ public void LoadAssemblyResolveReferences_WarnsWhenEnabled(bool resolveReference public void LoadAssembliesShouldResolveReferencesNoWarnings() { var assetInfo = GetSimpleTestAsset(); - AssemblySymbolLoader loader = new(_log.Object, resolveAssemblyReferences: true); + TestLog log = new(); + AssemblySymbolLoader loader = new(log, resolveAssemblyReferences: true); // AddReferenceSearchDirectories should be able to handle directories as well as full path to assemblies. loader.AddReferenceSearchPaths(Path.GetDirectoryName(typeof(string).Assembly.Location)); loader.AddReferenceSearchPaths(Path.GetFullPath(typeof(string).Assembly.Location)); loader.LoadAssembly(Path.Combine(assetInfo.OutputDirectory, assetInfo.TestAsset.TestProject.Name + ".dll")); - Assert.False(loader.HasLoadWarnings(out IReadOnlyList warnings)); - Assert.Empty(warnings); + Assert.Empty(log.Warnings); // Ensure we loaded more than one assembly since resolveReferences was set to true. Dictionary loadedAssemblies = (Dictionary)typeof(AssemblySymbolLoader)?.GetField("_loadedAssemblies", BindingFlags.NonPublic | BindingFlags.Instance)?.GetValue(loader); @@ -318,11 +310,12 @@ public void LoadAssemblyFromStreamNoWarns() { var assetInfo = GetSimpleTestAsset(); TestProject testProject = assetInfo.TestAsset.TestProject; - AssemblySymbolLoader loader = new(_log.Object); + TestLog log = new(); + AssemblySymbolLoader loader = new(log); using FileStream stream = File.OpenRead(Path.Combine(assetInfo.OutputDirectory, testProject.Name + ".dll")); IAssemblySymbol symbol = loader.LoadAssembly(testProject.Name, stream); - Assert.False(loader.HasLoadWarnings(out var _)); + Assert.False(log.HasLoggedWarnings); Assert.Equal(testProject.Name, symbol.Name, StringComparer.Ordinal); IEnumerable types = symbol.GlobalNamespace diff --git a/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/TestLog.cs b/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/TestLog.cs new file mode 100644 index 000000000000..1d915e915793 --- /dev/null +++ b/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/TestLog.cs @@ -0,0 +1,25 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.DotNet.ApiSymbolExtensions.Logging; + +namespace Microsoft.DotNet.ApiSymbolExtensions.Tests; + +internal class TestLog : ILog +{ + public List Info { get; } = []; + public List Errors { get; } = []; + public List Warnings { get; } = []; + + public bool HasLoggedErrors => Errors.Count != 0; + public bool HasLoggedWarnings => Warnings.Count != 0; + + public void LogError(string message) => Errors.Add(message); + public void LogError(string code, string message) => Errors.Add($"{code} {message}"); + + public void LogWarning(string message) => Warnings.Add(message); + public void LogWarning(string code, string message) => Warnings.Add($"{code} {message}"); + + public void LogMessage(string message) => Info.Add(message); + public void LogMessage(MessageImportance importance, string message) => Info.Add(message); +} From 50e61824deddf835bd0d0628b4c00434714347fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20S=C3=A1nchez=20L=C3=B3pez?= <1175054+carlossanlop@users.noreply.github.com> Date: Thu, 9 Jan 2025 13:44:08 -0800 Subject: [PATCH 6/9] Apply suggestions from code review Co-authored-by: Viktor Hofer --- .../AssemblySymbolLoaderTests.cs | 1 - .../Microsoft.DotNet.ApiSymbolExtensions.Tests.csproj | 1 - 2 files changed, 2 deletions(-) diff --git a/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/AssemblySymbolLoaderTests.cs b/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/AssemblySymbolLoaderTests.cs index 45c4deca7e0b..35886f8cbf05 100644 --- a/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/AssemblySymbolLoaderTests.cs +++ b/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/AssemblySymbolLoaderTests.cs @@ -12,7 +12,6 @@ namespace Microsoft.DotNet.ApiSymbolExtensions.Tests { public class AssemblySymbolLoaderTests : SdkTest { - public AssemblySymbolLoaderTests(ITestOutputHelper log) : base(log) { } private const string SimpleAssemblySourceContents = @" diff --git a/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/Microsoft.DotNet.ApiSymbolExtensions.Tests.csproj b/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/Microsoft.DotNet.ApiSymbolExtensions.Tests.csproj index 5602a7825845..9cb2cadbe2ed 100644 --- a/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/Microsoft.DotNet.ApiSymbolExtensions.Tests.csproj +++ b/test/Microsoft.DotNet.ApiSymbolExtensions.Tests/Microsoft.DotNet.ApiSymbolExtensions.Tests.csproj @@ -7,7 +7,6 @@ - From 14309ebe53779ab827dd82b976cab1a23820ccde Mon Sep 17 00:00:00 2001 From: Viktor Hofer Date: Fri, 10 Jan 2025 21:31:28 +0100 Subject: [PATCH 7/9] Remove AssemblyReferenceNotFound compatibility differences in APICompat --- .../DiagnosticIds.cs | 3 -- .../Mapping/AssemblyMapper.cs | 14 -------- .../Resources.resx | 3 -- .../Rules/CannotRemoveBaseTypeOrInterface.cs | 34 ------------------- .../Rules/RuleSettings.cs | 5 --- .../xlf/Resources.cs.xlf | 5 --- .../xlf/Resources.de.xlf | 5 --- .../xlf/Resources.es.xlf | 5 --- .../xlf/Resources.fr.xlf | 5 --- .../xlf/Resources.it.xlf | 5 --- .../xlf/Resources.ja.xlf | 5 --- .../xlf/Resources.ko.xlf | 5 --- .../xlf/Resources.pl.xlf | 5 --- .../xlf/Resources.pt-BR.xlf | 5 --- .../xlf/Resources.ru.xlf | 5 --- .../xlf/Resources.tr.xlf | 5 --- .../xlf/Resources.zh-Hans.xlf | 5 --- .../xlf/Resources.zh-Hant.xlf | 5 --- 18 files changed, 124 deletions(-) diff --git a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/DiagnosticIds.cs b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/DiagnosticIds.cs index 5d8d4fca4284..cb1f09738d0d 100644 --- a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/DiagnosticIds.cs +++ b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/DiagnosticIds.cs @@ -29,8 +29,5 @@ public static class DiagnosticIds public const string CannotReduceVisibility = "CP0019"; public const string CannotExpandVisibility = "CP0020"; public const string CannotChangeGenericConstraint = "CP0021"; - - // Assembly loading ids - public const string AssemblyReferenceNotFound = "CP1002"; } } diff --git a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/Mapping/AssemblyMapper.cs b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/Mapping/AssemblyMapper.cs index c3347c3a9433..7646c1628ad0 100644 --- a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/Mapping/AssemblyMapper.cs +++ b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/Mapping/AssemblyMapper.cs @@ -121,20 +121,6 @@ Dictionary> ResolveTypeForwards(Element types.Add(symbol); } - else - { - // If we should warn on missing references and we are unable to resolve the type forward, then we should log a diagnostic - if (Settings.WithReferences) - { - _assemblyLoadErrors.Add(new CompatDifference( - side == ElementSide.Left ? assembly.MetadataInformation : MetadataInformation.DefaultLeft, - side == ElementSide.Right ? assembly.MetadataInformation : MetadataInformation.DefaultRight, - DiagnosticIds.AssemblyReferenceNotFound, - string.Format(Resources.MatchingAssemblyNotFound, $"{symbol.ContainingAssembly.Name}.dll"), - DifferenceType.Changed, - symbol.ContainingAssembly.Identity.GetDisplayName())); - } - } } return typeForwards; diff --git a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/Resources.resx b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/Resources.resx index d3f005e0cb1c..781c94f35366 100644 --- a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/Resources.resx +++ b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/Resources.resx @@ -147,9 +147,6 @@ The {0} index should be in the range zero through {1} inclusive. - - Could not find matching assembly: '{0}' in any of the search directories. - Member '{0}' exists on {1} but not on {2} diff --git a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/Rules/CannotRemoveBaseTypeOrInterface.cs b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/Rules/CannotRemoveBaseTypeOrInterface.cs index 09068721b5df..956356df27d1 100644 --- a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/Rules/CannotRemoveBaseTypeOrInterface.cs +++ b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/Rules/CannotRemoveBaseTypeOrInterface.cs @@ -48,11 +48,6 @@ private void ValidateBaseTypeNotRemoved(ITypeSymbol left, ITypeSymbol right, str if (leftBaseType == null) return; - if (leftBaseType.TypeKind == TypeKind.Error && _settings.WithReferences) - { - AddAssemblyLoadError(leftMetadata, rightMetadata, differences, leftBaseType); - } - while (rightBaseType != null) { // If we found the immediate left base type on right we can assume @@ -61,11 +56,6 @@ private void ValidateBaseTypeNotRemoved(ITypeSymbol left, ITypeSymbol right, str if (_settings.SymbolEqualityComparer.Equals(leftBaseType, rightBaseType)) return; - if (rightBaseType.TypeKind == TypeKind.Error && _settings.WithReferences) - { - AddAssemblyLoadError(leftMetadata, rightMetadata, differences, rightBaseType); - } - rightBaseType = rightBaseType.BaseType; } @@ -84,11 +74,6 @@ private void ValidateInterfaceNotRemoved(ITypeSymbol left, ITypeSymbol right, st foreach (ITypeSymbol leftInterface in left.GetAllBaseInterfaces()) { - if (leftInterface.TypeKind == TypeKind.Error && _settings.WithReferences) - { - AddAssemblyLoadError(leftMetadata, rightMetadata, differences, leftInterface); - } - // Ignore non visible interfaces based on the run Settings // If TypeKind == Error it means the Roslyn couldn't resolve it, // so we are running with a missing assembly reference to where that type ef is defined. @@ -108,25 +93,6 @@ private void ValidateInterfaceNotRemoved(ITypeSymbol left, ITypeSymbol right, st return; } } - - foreach (ITypeSymbol rightInterface in rightInterfaces) - { - if (rightInterface.TypeKind == TypeKind.Error && _settings.WithReferences) - { - AddAssemblyLoadError(leftMetadata, rightMetadata, differences, rightInterface); - } - } - } - - private static void AddAssemblyLoadError(MetadataInformation leftMetadata, MetadataInformation rightMetadata, IList differences, ITypeSymbol type) - { - differences.Add(new CompatDifference( - leftMetadata, - rightMetadata, - DiagnosticIds.AssemblyReferenceNotFound, - string.Format(Resources.MatchingAssemblyNotFound, $"{type.ContainingAssembly.Name}.dll"), - DifferenceType.Changed, - type.ContainingAssembly.Identity.GetDisplayName())); } } } diff --git a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/Rules/RuleSettings.cs b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/Rules/RuleSettings.cs index d0d7fa955489..2587ad98f582 100644 --- a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/Rules/RuleSettings.cs +++ b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/Rules/RuleSettings.cs @@ -43,10 +43,5 @@ public interface IRuleSettings /// which are compared, should not differ. /// bool StrictMode { get; } - - /// - /// If true, references are available. Necessary to know for following type forwards. - /// - bool WithReferences { get; } } } diff --git a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.cs.xlf b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.cs.xlf index 01d041181abb..5872df4520eb 100644 --- a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.cs.xlf +++ b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.cs.xlf @@ -147,11 +147,6 @@ Index {0} by měl být v rozsahu nula až {1} včetně. - - Could not find matching assembly: '{0}' in any of the search directories. - V žádném z hledaných adresářů se nepovedlo najít odpovídající sestavení: {0}. - - Member '{0}' exists on {1} but not on {2} Člen {0} existuje v {1}, ale ne v {2} diff --git a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.de.xlf b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.de.xlf index 2cc94a6dfdbe..a1c7eb6bc0ff 100644 --- a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.de.xlf +++ b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.de.xlf @@ -147,11 +147,6 @@ Der {0} Index sollte zwischen null und einschließlich {1} liegen. - - Could not find matching assembly: '{0}' in any of the search directories. - Die übereinstimmende Assembly "{0}" wurde in keinem der Suchverzeichnisse gefunden. - - Member '{0}' exists on {1} but not on {2} Das Element „{0}“ ist auf {1} vorhanden, aber nicht auf {2}. diff --git a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.es.xlf b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.es.xlf index 904725d93069..4588e5e99dc8 100644 --- a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.es.xlf +++ b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.es.xlf @@ -147,11 +147,6 @@ El índice de {0} debe estar comprendido entre cero y {1} inclusive. - - Could not find matching assembly: '{0}' in any of the search directories. - No se han encontrado coincidencias de ensamblado: "{0}" en ninguno de los directorios de búsqueda. - - Member '{0}' exists on {1} but not on {2} El miembro "{0}" existe en {1} pero no en {2} diff --git a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.fr.xlf b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.fr.xlf index 9233f13d40c0..2f1d893ea7a8 100644 --- a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.fr.xlf +++ b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.fr.xlf @@ -147,11 +147,6 @@ L’index {0} doit être compris entre zéro et {1} inclus. - - Could not find matching assembly: '{0}' in any of the search directories. - Assembly correspondant introuvable : «{0}» dans les répertoires de recherche. - - Member '{0}' exists on {1} but not on {2} Le membre '{0}' existe sur {1}, mais pas sur {2}. diff --git a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.it.xlf b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.it.xlf index 22c388a9b01e..dd4468415175 100644 --- a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.it.xlf +++ b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.it.xlf @@ -147,11 +147,6 @@ L'indice {0} deve essere compreso nell'intervallo compreso tra zero e {1} incluso. - - Could not find matching assembly: '{0}' in any of the search directories. - Non è stato possibile trovare un assembly corrispondente: '{0}' in nessuna delle directory di ricerca. - - Member '{0}' exists on {1} but not on {2} Il membro ' {0}' esiste in {1} ma non in {2} diff --git a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.ja.xlf b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.ja.xlf index 8c284153fed8..41eec2342b28 100644 --- a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.ja.xlf +++ b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.ja.xlf @@ -147,11 +147,6 @@ {0} インデックスは、0 から {1} の範囲内である必要があります。 - - Could not find matching assembly: '{0}' in any of the search directories. - 任意の検索ディレクトリ '{0}' に一致するアセンブリが見つかりませんでした。 - - Member '{0}' exists on {1} but not on {2} メンバー '{0}' は {1} に存在していますが、{2} には存在しません diff --git a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.ko.xlf b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.ko.xlf index bb4ed60391a0..b18330f14056 100644 --- a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.ko.xlf +++ b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.ko.xlf @@ -147,11 +147,6 @@ {0} 인덱스는 0 이상 {1} 이하의 범위 안에 있어야 합니다. - - Could not find matching assembly: '{0}' in any of the search directories. - 검색 디렉터리에서 일치하는 어셈블리 '{0}'(을)를 찾을 수 없습니다. - - Member '{0}' exists on {1} but not on {2} 멤버 '{0}'이(가) {1}에는 있지만 {2}에는 없습니다 diff --git a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.pl.xlf b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.pl.xlf index 100782f99b1a..6aefc58b20a5 100644 --- a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.pl.xlf +++ b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.pl.xlf @@ -147,11 +147,6 @@ Indeks {0} powinien należeć do zakresu od zera do {1} włącznie. - - Could not find matching assembly: '{0}' in any of the search directories. - Nie można znaleźć pasującego zestawu: "{0}" w żadnym z katalogów wyszukiwania. - - Member '{0}' exists on {1} but not on {2} Członek „{0}” istnieje w {1}, ale nie w {2} diff --git a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.pt-BR.xlf b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.pt-BR.xlf index 17a61a7bef92..c52b5f528fb2 100644 --- a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.pt-BR.xlf +++ b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.pt-BR.xlf @@ -147,11 +147,6 @@ O índice {0} deve estar no intervalo de zero até {1} inclusivo. - - Could not find matching assembly: '{0}' in any of the search directories. - Não foi possível encontrar o assembly correspondente: '{0}' em qualquer um dos diretórios de pesquisa. - - Member '{0}' exists on {1} but not on {2} O membro '{0}' existe em {1} mas não em {2} diff --git a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.ru.xlf b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.ru.xlf index 889779190047..7254edc7e52a 100644 --- a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.ru.xlf +++ b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.ru.xlf @@ -147,11 +147,6 @@ Индекс {0} должен находиться в диапазоне от нуля до {1} включительно. - - Could not find matching assembly: '{0}' in any of the search directories. - Не удалось найти соответствующую сборку "{0}" ни в одном из каталогов поиска. - - Member '{0}' exists on {1} but not on {2} Элемент "{0}" существует в {1}, но не в {2} diff --git a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.tr.xlf b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.tr.xlf index 202835edaf0e..8f5b2a486612 100644 --- a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.tr.xlf +++ b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.tr.xlf @@ -147,11 +147,6 @@ {0} dizini sıfırdan başlayarak {1} dahil olmak üzere bu aralıkta olmalıdır. - - Could not find matching assembly: '{0}' in any of the search directories. - Arama dizinlerinin hiçbirinde eşleşen {0} bütünleştirilmiş kodu bulunamadı. - - Member '{0}' exists on {1} but not on {2} '{0}' üyesi {1} üzerinde var ancak {2} üzerinde yok diff --git a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.zh-Hans.xlf b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.zh-Hans.xlf index 5dfe74a0df94..cb556448476d 100644 --- a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.zh-Hans.xlf +++ b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.zh-Hans.xlf @@ -147,11 +147,6 @@ {0} 索引应在 0 到 {1} (含)范围内。 - - Could not find matching assembly: '{0}' in any of the search directories. - 在任何搜索目录中都找不到匹配的程序集“{0}”。 - - Member '{0}' exists on {1} but not on {2} 成员“{0}”在 {1} 上存在，但在 {2} 上不存在 diff --git a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.zh-Hant.xlf b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.zh-Hant.xlf index 0e8fad65bba1..bcd1eb5ac9e6 100644 --- a/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.zh-Hant.xlf +++ b/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/xlf/Resources.zh-Hant.xlf @@ -147,11 +147,6 @@ {0} 索引應在零到 {1} (含) 之間的範圍。 - - Could not find matching assembly: '{0}' in any of the search directories. - 在任何搜尋目錄中都找不到符合的元件: '{0}'。 - - Member '{0}' exists on {1} but not on {2} 成員 '{0}' 存在於 {1}，但不在 {2} 上 From a82b10db2ac01280c71f42a9b981953e0b43b3cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20S=C3=A1nchez=20L=C3=B3pez?= <1175054+carlossanlop@users.noreply.github.com> Date: Fri, 10 Jan 2025 12:51:38 -0800 Subject: [PATCH 8/9] Fix merge conflict error --- test/Microsoft.DotNet.GenAPI.Tests/CSharpFileBuilderTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/test/Microsoft.DotNet.GenAPI.Tests/CSharpFileBuilderTests.cs b/test/Microsoft.DotNet.GenAPI.Tests/CSharpFileBuilderTests.cs index 42603c25a1c9..81856d2842ce 100644 --- a/test/Microsoft.DotNet.GenAPI.Tests/CSharpFileBuilderTests.cs +++ b/test/Microsoft.DotNet.GenAPI.Tests/CSharpFileBuilderTests.cs @@ -10,7 +10,6 @@ using Microsoft.DotNet.ApiSymbolExtensions.Filtering; using Microsoft.DotNet.ApiSymbolExtensions.Logging; using Microsoft.DotNet.ApiSymbolExtensions.Tests; -using Microsoft.DotNet.GenAPI.Filtering; using Moq; namespace Microsoft.DotNet.GenAPI.Tests From 903b8edfda51128721ad87f7ac69473ed632b7da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20S=C3=A1nchez=20L=C3=B3pez?= <1175054+carlossanlop@users.noreply.github.com> Date: Fri, 10 Jan 2025 15:57:11 -0800 Subject: [PATCH 9/9] Delete/modify tests expecting CP1002 --- .../ValidatePackageTargetIntegrationTests.cs | 65 +++---------------- 1 file changed, 9 insertions(+), 56 deletions(-) diff --git a/test/Microsoft.DotNet.ApiCompat.IntegrationTests/Task/ValidatePackageTargetIntegrationTests.cs b/test/Microsoft.DotNet.ApiCompat.IntegrationTests/Task/ValidatePackageTargetIntegrationTests.cs index 028fddd8d262..c7e8afadb6ba 100644 --- a/test/Microsoft.DotNet.ApiCompat.IntegrationTests/Task/ValidatePackageTargetIntegrationTests.cs +++ b/test/Microsoft.DotNet.ApiCompat.IntegrationTests/Task/ValidatePackageTargetIntegrationTests.cs @@ -187,11 +187,11 @@ public void ValidatePackageWithReferences() } [RequiresMSBuildVersionTheory("17.12")] - [InlineData(false, true, false)] - [InlineData(false, false, false)] - [InlineData(true, false, false)] - [InlineData(true, true, true)] - public void ValidateOnlyErrorWhenAReferenceIsRequired(bool createDependencyToDummy, bool useReferences, bool shouldLogError) + [InlineData(false, true)] + [InlineData(false, false)] + [InlineData(true, false)] + [InlineData(true, true)] + public void ValidateOnlyErrorWhenAReferenceIsRequired(bool createDependencyToDummy, bool useReferences) { string testDependencyCode = createDependencyToDummy ? @"namespace PackageValidationTests{public class SomeBaseClass : IDummyInterface { }public class SomeDummyClass : IDummyInterface { }}" : @@ -220,17 +220,13 @@ public void ValidateOnlyErrorWhenAReferenceIsRequired(bool createDependencyToDum // removed an interface due to it's base class removing that implementation. We validate that APICompat doesn't // log errors when not using references. validator.Validate(new PackageValidatorOption(package)); - if (shouldLogError) - Assert.Contains($"CP1002 Could not find matching assembly: '{testDummyDependency.Name}.dll' in any of the search directories.", log.errors); - else - Assert.DoesNotContain($"CP1002 Could not find matching assembly: '{testDummyDependency.Name}.dll' in any of the search directories.", log.errors); } [RequiresMSBuildVersionTheory("17.12")] - [InlineData(false, true, false, false)] - [InlineData(true, false, false, false)] - [InlineData(true, true, true, true)] - public void ValidateErrorWhenTypeForwardingReferences(bool useReferences, bool expectCP0001, bool deleteFile, bool expectCP1002) + [InlineData(false, true, false)] + [InlineData(true, false, false)] + [InlineData(true, true, true)] + public void ValidateErrorWhenTypeForwardingReferences(bool useReferences, bool expectCP0001, bool deleteFile) { string dependencySourceCode = @"namespace PackageValidationTests { public interface ISomeInterface { } #if !NETSTANDARD2_0 @@ -267,49 +263,6 @@ namespace PackageValidationTests { public class MyForwardedType : ISomeInterface if (expectCP0001) Assert.Contains($"CP0001 Type 'PackageValidationTests.MyForwardedType' exists on lib/netstandard2.0/{testProject.Name}.dll but not on lib/{ToolsetInfo.CurrentTargetFramework}/{testProject.Name}.dll", log.errors); - - if (expectCP1002) - Assert.Contains($"CP1002 Could not find matching assembly: '{dependency.Name}.dll' in any of the search directories.", log.errors); - } - - [RequiresMSBuildVersionFact("17.12", Reason = "Needs System.Text.Json 8.0.5")] - public void EnsureOnlyOneAssemblyLoadErrorIsLoggedPerMissingAssembly() - { - string dependencySourceCode = @"namespace PackageValidationTests { public interface ISomeInterface { } -#if !NETSTANDARD2_0 -public class MyForwardedType : ISomeInterface { } -public class MySecondForwardedType : ISomeInterface { } -#endif -}"; - string testSourceCode = @" -#if NETSTANDARD2_0 -namespace PackageValidationTests { public class MyForwardedType : ISomeInterface { } public class MySecondForwardedType : ISomeInterface { } } -#else -[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(PackageValidationTests.MyForwardedType))] -[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(PackageValidationTests.MySecondForwardedType))] -#endif"; - - TestProject dependency = CreateTestProject(dependencySourceCode, $"netstandard2.0;{ToolsetInfo.CurrentTargetFramework}"); - TestProject testProject = CreateTestProject(testSourceCode, $"netstandard2.0;{ToolsetInfo.CurrentTargetFramework}", new[] { dependency }); - - TestAsset asset = _testAssetsManager.CreateTestProject(testProject, testProject.Name); - PackCommand packCommand = new(Log, Path.Combine(asset.TestRoot, testProject.Name)); - var result = packCommand.Execute(); - Assert.Equal(string.Empty, result.StdErr); - - Dictionary> references = new() - { - { NuGetFramework.ParseFolder("netstandard2.0"), new string[] { Path.Combine(asset.TestRoot, asset.TestProject.Name, "bin", "Debug", "netstandard2.0") } }, - { NuGetFramework.ParseFolder(ToolsetInfo.CurrentTargetFramework), new string[] { Path.Combine(asset.TestRoot, asset.TestProject.Name, "bin", "Debug", ToolsetInfo.CurrentTargetFramework) } } - }; - Package package = Package.Create(packCommand.GetNuGetPackage(), references); - - File.Delete(Path.Combine(asset.TestRoot, asset.TestProject.Name, "bin", "Debug", ToolsetInfo.CurrentTargetFramework, $"{dependency.Name}.dll")); - (SuppressibleTestLog log, CompatibleFrameworkInPackageValidator validator) = CreateLoggerAndValidator(); - - validator.Validate(new PackageValidatorOption(package)); - - Assert.Single(log.errors, e => e.Contains("CP1002")); } [RequiresMSBuildVersionTheory("17.12")]