From 86b7b93acbef44c70c1ef34ecb5be93796f741f9 Mon Sep 17 00:00:00 2001 From: Santiago Fernandez Madero Date: Fri, 9 Apr 2021 09:42:08 -0700 Subject: [PATCH 1/6] Add initial version of ApiCompatibility --- .gitignore | 3 +- CODEOWNERS | 4 + sdk.sln | 17 + .../Abstractions/CompatDifference.cs | 39 ++ .../Abstractions/DifferenceType.cs | 9 + .../Filtering/AccessibilityFilter.cs | 20 + .../Abstractions/Filtering/IDiffingFilter.cs | 12 + .../Abstractions/IDiagnostic.cs | 8 + .../Abstractions/MapperVisitor.cs | 68 ++++ .../Abstractions/Mappers/AssemblyMapper.cs | 84 +++++ .../Abstractions/Mappers/AssemblySetMapper.cs | 47 +++ .../Abstractions/Mappers/ElementMapper.cs | 42 +++ .../Abstractions/Mappers/MemberMapper.cs | 12 + .../Abstractions/Mappers/NamespaceMapper.cs | 74 ++++ .../Abstractions/Mappers/TypeMapper.cs | 87 +++++ .../Abstractions/Rule.cs | 20 + .../ApiDiffer.cs | 49 +++ .../AssemblySymbolLoader.cs | 296 +++++++++++++++ .../DefaultSymbolsEqualityComparer.cs | 26 ++ .../DiagnosticBag.cs | 56 +++ .../DiagnosticIds.cs | 8 + .../DiferenceVisitor.cs | 39 ++ .../DiffingSettings.cs | 21 ++ .../Extensions/TypeSymbolExtensions.cs | 29 ++ .../Microsoft.DotNet.ApiCompatibility.csproj | 18 + .../Rules/MembersMustExist.cs | 88 +++++ .../Rules/RuleDriver.cs | 90 +++++ .../AssemblySymbolLoaderTests.cs | 356 ++++++++++++++++++ ...osoft.DotNet.ApiCompatibility.Tests.csproj | 18 + .../Rules/MembersMustExistTests.cs | 233 ++++++++++++ .../Rules/TypeMustExistTests.cs | 277 ++++++++++++++ .../SymbolFactory.cs | 52 +++ 32 files changed, 2201 insertions(+), 1 deletion(-) create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/CompatDifference.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/DifferenceType.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/AccessibilityFilter.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/IDiffingFilter.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/IDiagnostic.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/MapperVisitor.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblyMapper.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblySetMapper.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/ElementMapper.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/MemberMapper.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/NamespaceMapper.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/TypeMapper.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Rule.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/ApiDiffer.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/AssemblySymbolLoader.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/DefaultSymbolsEqualityComparer.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticBag.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticIds.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiferenceVisitor.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiffingSettings.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/Extensions/TypeSymbolExtensions.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/Microsoft.DotNet.ApiCompatibility.csproj create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/MembersMustExist.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleDriver.cs create mode 100644 src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/AssemblySymbolLoaderTests.cs create mode 100644 src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Microsoft.DotNet.ApiCompatibility.Tests.csproj create mode 100644 src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/MembersMustExistTests.cs create mode 100644 src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/TypeMustExistTests.cs create mode 100644 src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/SymbolFactory.cs diff --git a/.gitignore b/.gitignore index e2cd5ecd2684..84a4e7bf3425 100644 --- a/.gitignore +++ b/.gitignore @@ -19,7 +19,8 @@ obj/ *.binlog -sdk.sln.DotSettings.user +# User specific files +*.user # Debian and python stuff *.dsc diff --git a/CODEOWNERS b/CODEOWNERS index 767ae68495a7..584e649fd8ba 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -53,3 +53,7 @@ /src/Tests/dotnet-watch.Tests/ @captainsafia, @pranavkm, @mkArtakMSFT /src/Tests/Microsoft.AspNetCore.Watch.BrowserRefresh.Tests/ @captainsafia, @pranavkm, @mkArtakMSFT /src/BuiltInTools/ @captainsafia, @pranavkm, @mkArtakMSFT + +# Compatibility tools owned by runtime team +/src/Compatibility/ @Anipik, @safern, @ericstj +/src/Tests/Microsoft.DotNet.ApiCompatibility/ @Anipik, @safern, @ericstj diff --git a/sdk.sln b/sdk.sln index 17d60ed4398b..efa1e86700e5 100644 --- a/sdk.sln +++ b/sdk.sln @@ -341,6 +341,12 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.DotNet.NativeWrap EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.NET.Sdk.Razor.SourceGenerators.Tests", "src\Tests\Microsoft.NET.Sdk.Razor.SourceGenerators.Tests\Microsoft.NET.Sdk.Razor.SourceGenerators.Tests.csproj", "{A71FC21D-D90A-49B5-9B5A-AD4776287B55}" EndProject +Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Compatibility", "Compatibility", "{AF683E5C-421E-4DE0-ADD7-9841E5D12BFA}" +EndProject +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.DotNet.ApiCompatibility", "src\Compatibility\Microsoft.DotNet.ApiCompatibility\Microsoft.DotNet.ApiCompatibility.csproj", "{3F5A028C-C51B-434A-8C10-37680CD2635C}" +EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Microsoft.DotNet.ApiCompatibility.Tests", "src\Tests\Microsoft.DotNet.ApiCompatibility.Tests\Microsoft.DotNet.ApiCompatibility.Tests.csproj", "{24F084ED-35BB-401E-89F5-63E5E22C3B3B}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -615,6 +621,14 @@ Global {A71FC21D-D90A-49B5-9B5A-AD4776287B55}.Debug|Any CPU.Build.0 = Debug|Any CPU {A71FC21D-D90A-49B5-9B5A-AD4776287B55}.Release|Any CPU.ActiveCfg = Release|Any CPU {A71FC21D-D90A-49B5-9B5A-AD4776287B55}.Release|Any CPU.Build.0 = Release|Any CPU + {3F5A028C-C51B-434A-8C10-37680CD2635C}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {3F5A028C-C51B-434A-8C10-37680CD2635C}.Debug|Any CPU.Build.0 = Debug|Any CPU + {3F5A028C-C51B-434A-8C10-37680CD2635C}.Release|Any CPU.ActiveCfg = Release|Any CPU + {3F5A028C-C51B-434A-8C10-37680CD2635C}.Release|Any CPU.Build.0 = Release|Any CPU + {24F084ED-35BB-401E-89F5-63E5E22C3B3B}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {24F084ED-35BB-401E-89F5-63E5E22C3B3B}.Debug|Any CPU.Build.0 = Debug|Any CPU + {24F084ED-35BB-401E-89F5-63E5E22C3B3B}.Release|Any CPU.ActiveCfg = Release|Any CPU + {24F084ED-35BB-401E-89F5-63E5E22C3B3B}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -728,6 +742,9 @@ Global {1BBFA19C-03F0-4D27-9D0D-0F8172642107} = {71A9F549-0EB6-41F9-BC16-4A6C5007FC91} {E97E9E7F-11B4-42F7-8B55-D0451F5E82A0} = {8F22FBD6-BDC8-431E-8402-B7460D3A9724} {A71FC21D-D90A-49B5-9B5A-AD4776287B55} = {580D1AE7-AA8F-4912-8B76-105594E00B3B} + {AF683E5C-421E-4DE0-ADD7-9841E5D12BFA} = {22AB674F-ED91-4FBC-BFEE-8A1E82F9F05E} + {3F5A028C-C51B-434A-8C10-37680CD2635C} = {AF683E5C-421E-4DE0-ADD7-9841E5D12BFA} + {24F084ED-35BB-401E-89F5-63E5E22C3B3B} = {580D1AE7-AA8F-4912-8B76-105594E00B3B} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {FB8F26CE-4DE6-433F-B32A-79183020BBD6} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/CompatDifference.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/CompatDifference.cs new file mode 100644 index 000000000000..cf7e31425d97 --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/CompatDifference.cs @@ -0,0 +1,39 @@ +using Microsoft.CodeAnalysis; +using System; + +namespace Microsoft.DotNet.ApiCompatibility.Abstractions +{ + public class CompatDifference : IDiagnostic, IEquatable + { + public string DiagnosticId { get; } + public DifferenceType Type { get; } + public virtual string Message { get; } + public string ReferenceId { get; } + + private CompatDifference() { } + + public CompatDifference(string id, string message, DifferenceType type, ISymbol member) + : this(id, message, type, member.GetDocumentationCommentId()) + { + } + + public CompatDifference(string id, string message, DifferenceType type, string memberId) + { + DiagnosticId = id; + Message = message; + Type = type; + ReferenceId = memberId; + } + + public bool Equals(CompatDifference other) => + Type == other.Type && + DiagnosticId.Equals(other.DiagnosticId, StringComparison.OrdinalIgnoreCase) && + ReferenceId.Equals(other.ReferenceId, StringComparison.OrdinalIgnoreCase) && + Message.Equals(other.Message, StringComparison.OrdinalIgnoreCase); + + public override int GetHashCode() => + HashCode.Combine(ReferenceId, DiagnosticId, Message, Type); + + public override string ToString() => $"{DiagnosticId} : {Message}"; + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/DifferenceType.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/DifferenceType.cs new file mode 100644 index 000000000000..3666614545e2 --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/DifferenceType.cs @@ -0,0 +1,9 @@ +namespace Microsoft.DotNet.ApiCompatibility.Abstractions +{ + public enum DifferenceType + { + Changed, + Added, + Removed + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/AccessibilityFilter.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/AccessibilityFilter.cs new file mode 100644 index 000000000000..6cc726926130 --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/AccessibilityFilter.cs @@ -0,0 +1,20 @@ +using Microsoft.CodeAnalysis; + +namespace Microsoft.DotNet.ApiCompatibility.Abstractions +{ + internal class AccessibilityFilter : IDiffingFilter + { + private readonly bool _includeInternalSymbols; + + internal AccessibilityFilter(bool includeInternalSymbols) + { + _includeInternalSymbols = includeInternalSymbols; + } + + public bool Include(ISymbol symbol) => + symbol.DeclaredAccessibility == Accessibility.Public || + symbol.DeclaredAccessibility == Accessibility.Protected || + symbol.DeclaredAccessibility == Accessibility.ProtectedOrInternal || + (_includeInternalSymbols && symbol.DeclaredAccessibility != Accessibility.Private); + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/IDiffingFilter.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/IDiffingFilter.cs new file mode 100644 index 000000000000..7d8bf5acadce --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/IDiffingFilter.cs @@ -0,0 +1,12 @@ +using Microsoft.CodeAnalysis; +using System; +using System.Collections.Generic; +using System.Text; + +namespace Microsoft.DotNet.ApiCompatibility.Abstractions +{ + public interface IDiffingFilter + { + bool Include(ISymbol symbol); + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/IDiagnostic.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/IDiagnostic.cs new file mode 100644 index 000000000000..6aa79584c1f5 --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/IDiagnostic.cs @@ -0,0 +1,8 @@ +namespace Microsoft.DotNet.ApiCompatibility.Abstractions +{ + public interface IDiagnostic + { + string DiagnosticId { get; } + string ReferenceId { get; } + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/MapperVisitor.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/MapperVisitor.cs new file mode 100644 index 000000000000..683a7e0207ae --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/MapperVisitor.cs @@ -0,0 +1,68 @@ +namespace Microsoft.DotNet.ApiCompatibility.Abstractions +{ + public class MapperVisitor + { + public void Visit(ElementMapper mapper) + { + if (mapper is AssemblySetMapper assemblySetMapper) + { + Visit(assemblySetMapper); + } + else if (mapper is AssemblyMapper assemblyMapper) + { + Visit(assemblyMapper); + } + else if (mapper is NamespaceMapper nsMapper) + { + Visit(nsMapper); + } + else if (mapper is TypeMapper typeMapper) + { + Visit(typeMapper); + } + else if (mapper is MemberMapper memberMapper) + { + Visit(memberMapper); + } + } + + public virtual void Visit(AssemblySetMapper mapper) + { + foreach (AssemblyMapper assembly in mapper.GetAssemblies()) + { + Visit(assembly); + } + } + + public virtual void Visit(AssemblyMapper mapper) + { + foreach (NamespaceMapper nsMapper in mapper.GetNamespaces()) + { + Visit(nsMapper); + } + } + + public virtual void Visit(NamespaceMapper mapper) + { + foreach (TypeMapper type in mapper.GetTypes()) + { + Visit(type); + } + } + + public virtual void Visit(TypeMapper mapper) + { + foreach (TypeMapper type in mapper.GetNestedTypes()) + { + Visit(type); + } + + foreach (MemberMapper member in mapper.GetMembers()) + { + Visit(member); + } + } + + public virtual void Visit(MemberMapper mapper) { } + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblyMapper.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblyMapper.cs new file mode 100644 index 000000000000..d9b7240d8534 --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblyMapper.cs @@ -0,0 +1,84 @@ +using Microsoft.CodeAnalysis; +using System; +using System.Collections.Generic; +using System.Text; + +namespace Microsoft.DotNet.ApiCompatibility.Abstractions +{ + public class AssemblyMapper : ElementMapper + { + private Dictionary _namespaces; + + public AssemblyMapper(DiffingSettings settings) : base(settings) { } + + public IEnumerable GetNamespaces() + { + if (_namespaces == null) + { + _namespaces = new Dictionary(Settings.EqualityComparer); + Dictionary> typeForwards; + if (Left != null) + { + typeForwards = ResolveTypeForwards(Left, Settings.EqualityComparer); + AddOrCreateMappers(Left.GlobalNamespace, 0); + } + + if (Right != null) + { + typeForwards = ResolveTypeForwards(Right, Settings.EqualityComparer); + AddOrCreateMappers(Right.GlobalNamespace, 1); + } + + void AddOrCreateMappers(INamespaceSymbol ns, int index) + { + Stack stack = new(); + stack.Push(ns); + while (stack.Count > 0) + { + INamespaceSymbol symbol = stack.Pop(); + if (typeForwards.TryGetValue(symbol, out List forwardedTypes) || symbol.GetTypeMembers().Length > 0) + { + if (!_namespaces.TryGetValue(symbol, out NamespaceMapper mapper)) + { + mapper = new NamespaceMapper(Settings); + _namespaces.Add(symbol, mapper); + } + + mapper.AddElement(symbol, index); + mapper.AddForwardedTypes(forwardedTypes ?? new List(), index); + } + + foreach (INamespaceSymbol child in symbol.GetNamespaceMembers()) + stack.Push(child); + } + } + + static Dictionary> ResolveTypeForwards(IAssemblySymbol assembly, IEqualityComparer comparer) + { + Dictionary> typeForwards = new(comparer); + foreach (INamedTypeSymbol symbol in assembly.GetForwardedTypes()) + { + if (symbol.TypeKind != TypeKind.Error) + { + if (!typeForwards.TryGetValue(symbol.ContainingNamespace, out List types)) + { + types = new List(); + typeForwards.Add(symbol.ContainingNamespace, types); + } + + types.Add(symbol); + } + else + { + // TODO: Log Warning; + } + } + + return typeForwards; + } + } + + return _namespaces.Values; + } + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblySetMapper.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblySetMapper.cs new file mode 100644 index 000000000000..ab97947bf547 --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblySetMapper.cs @@ -0,0 +1,47 @@ +using Microsoft.CodeAnalysis; +using System; +using System.Collections.Generic; +using System.Text; + +namespace Microsoft.DotNet.ApiCompatibility.Abstractions +{ + public class AssemblySetMapper : ElementMapper> + { + private Dictionary _assemblies; + + public AssemblySetMapper(DiffingSettings settings) : base(settings) { } + + public IEnumerable GetAssemblies() + { + if (_assemblies == null) + { + _assemblies = new Dictionary(Settings.EqualityComparer); + + if (Left != null) + { + AddOrCreateMappers(Left, 0); + } + + if (Right != null) + { + AddOrCreateMappers(Right, 1); + } + + void AddOrCreateMappers(IEnumerable elements, int index) + { + foreach (IAssemblySymbol assembly in elements) + { + if (!_assemblies.TryGetValue(assembly, out AssemblyMapper mapper)) + { + mapper = new AssemblyMapper(Settings); + _assemblies.Add(assembly, mapper); + } + mapper.AddElement(assembly, index); + } + } + } + + return _assemblies.Values; + } + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/ElementMapper.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/ElementMapper.cs new file mode 100644 index 000000000000..727af20ee6c0 --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/ElementMapper.cs @@ -0,0 +1,42 @@ +using Microsoft.DotNet.ApiCompatibility.Rules; +using System; +using System.Collections.Generic; +using System.Text; + +namespace Microsoft.DotNet.ApiCompatibility.Abstractions +{ + public class ElementMapper + { + private IEnumerable _differences; + + public T Left { get; private set; } + public T Right { get; private set; } + + public DiffingSettings Settings { get; } + + public ElementMapper(DiffingSettings settings) + { + Settings = settings; + } + + public virtual void AddElement(T element, int index) + { + if (index < 0 || index > 1) + throw new ArgumentOutOfRangeException(nameof(index), "index should either be 0 or 1"); + + if (index == 0) + { + Left = element; + } + else + { + Right = element; + } + } + + public IEnumerable GetDifferences() + { + return _differences ??= Settings.RuleDriverFactory.GetRuleDriver().Run(this); + } + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/MemberMapper.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/MemberMapper.cs new file mode 100644 index 000000000000..4e16e78e6966 --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/MemberMapper.cs @@ -0,0 +1,12 @@ +using Microsoft.CodeAnalysis; +using System; +using System.Collections.Generic; +using System.Text; + +namespace Microsoft.DotNet.ApiCompatibility.Abstractions +{ + public class MemberMapper : ElementMapper + { + public MemberMapper(DiffingSettings settings) : base(settings) { } + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/NamespaceMapper.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/NamespaceMapper.cs new file mode 100644 index 000000000000..feb218d8eea8 --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/NamespaceMapper.cs @@ -0,0 +1,74 @@ +using Microsoft.CodeAnalysis; +using System; +using System.Collections.Generic; +using System.Linq; + +namespace Microsoft.DotNet.ApiCompatibility.Abstractions +{ + public class NamespaceMapper : ElementMapper + { + private Dictionary _types; + private IEnumerable _leftForwardedTypes; + private IEnumerable _rightForwardedTypes; + + public NamespaceMapper(DiffingSettings settings) : base(settings) { } + + public IEnumerable GetTypes() + { + if (_types == null) + { + _types = new Dictionary(Settings.EqualityComparer); + IEnumerable types; + + if (Left != null) + { + types = Left.GetTypeMembers().AddRange(_leftForwardedTypes); + AddOrCreateMappers(0); + } + + if (Right != null) + { + types = Right.GetTypeMembers().AddRange(_rightForwardedTypes); + AddOrCreateMappers(1); + } + + void AddOrCreateMappers(int index) + { + if (types == null) + return; + + foreach (var type in types) + { + if (Settings.Filter.Include(type)) + { + if (!_types.TryGetValue(type, out TypeMapper mapper)) + { + mapper = new TypeMapper(Settings); + _types.Add(type, mapper); + } + + mapper.AddElement(type, index); + } + } + } + } + + return _types.Values; + } + + public void AddForwardedTypes(IEnumerable forwardedTypes, int index) + { + if (index < 0 || index > 1) + throw new ArgumentOutOfRangeException(nameof(index), $"Value must be 0 or 1"); + + if (index == 0) + { + _leftForwardedTypes = forwardedTypes; + } + else + { + _rightForwardedTypes = forwardedTypes; + } + } + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/TypeMapper.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/TypeMapper.cs new file mode 100644 index 000000000000..8f02e504fdcd --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/TypeMapper.cs @@ -0,0 +1,87 @@ +using Microsoft.CodeAnalysis; +using System.Collections.Generic; + +namespace Microsoft.DotNet.ApiCompatibility.Abstractions +{ + public class TypeMapper : ElementMapper + { + private Dictionary _nestedTypes; + private Dictionary _members; + + public TypeMapper(DiffingSettings settings) : base(settings) { } + + public bool ShouldDiffMembers => Left != null && Right != null; + + public IEnumerable GetNestedTypes() + { + if (_nestedTypes == null) + { + _nestedTypes = new Dictionary(Settings.EqualityComparer); + + if (Left != null) + { + AddOrCreateMappers(Left.GetTypeMembers(), 0); + } + + if (Right != null) + { + AddOrCreateMappers(Right.GetTypeMembers(), 1); + } + + void AddOrCreateMappers(IEnumerable symbols, int index) + { + foreach (var nestedType in symbols) + { + if (Settings.Filter.Include(nestedType)) + { + if (!_nestedTypes.TryGetValue(nestedType, out TypeMapper mapper)) + { + mapper = new TypeMapper(Settings); + _nestedTypes.Add(nestedType, mapper); + } + mapper.AddElement(nestedType, index); + } + } + } + } + + return _nestedTypes.Values; + } + + public IEnumerable GetMembers() + { + if (_members == null) + { + _members = new Dictionary(Settings.EqualityComparer); + + if (Left != null) + { + AddOrCreateMappers(Left.GetMembers(), 0); + } + + if (Right != null) + { + AddOrCreateMappers(Right.GetMembers(), 1); + } + + void AddOrCreateMappers(IEnumerable symbols, int index) + { + foreach (var member in symbols) + { + if (Settings.Filter.Include(member) && member is not ITypeSymbol) + { + if (!_members.TryGetValue(member, out MemberMapper mapper)) + { + mapper = new MemberMapper(Settings); + _members.Add(member, mapper); + } + mapper.AddElement(member, index); + } + } + } + } + + return _members.Values; + } + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Rule.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Rule.cs new file mode 100644 index 000000000000..817cee2962ff --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Rule.cs @@ -0,0 +1,20 @@ +using Microsoft.CodeAnalysis; +using System; +using System.Collections.Generic; +using System.Text; + +namespace Microsoft.DotNet.ApiCompatibility.Abstractions +{ + public abstract class Rule + { + public virtual void Run(AssemblyMapper mapper, List differences) + { + } + public virtual void Run(TypeMapper mapper, List differences) + { + } + public virtual void Run(MemberMapper mapper, List differences) + { + } + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ApiDiffer.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ApiDiffer.cs new file mode 100644 index 000000000000..cdbde94b4e01 --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ApiDiffer.cs @@ -0,0 +1,49 @@ +using Microsoft.CodeAnalysis; +using Microsoft.DotNet.ApiCompatibility.Abstractions; +using System; +using System.Collections.Generic; + +namespace Microsoft.DotNet.ApiCompatibility +{ + public class ApiDiffer + { + private readonly DiffingSettings _settings; + + public ApiDiffer() : this(new DiffingSettings()) { } + + public ApiDiffer(DiffingSettings settings) + { + _settings = settings; + } + + public ApiDiffer(bool includeInternalSymbols) + { + _settings = new DiffingSettings(filter: new AccessibilityFilter(includeInternalSymbols)); + } + + public string NoWarn { get; set; } = string.Empty; + public (string diagnosticId, string memberId)[] IgnoredDifferences { get; set; } + + public IEnumerable GetDifferences(IEnumerable left, IEnumerable right) + { + AssemblySetMapper mapper = new(_settings); + mapper.AddElement(left, 0); + mapper.AddElement(right, 1); + + DiferenceVisitor visitor = new(noWarn: NoWarn, ignoredDifferences: IgnoredDifferences); + visitor.Visit(mapper); + return visitor.Differences; + } + + public IEnumerable GetDifferences(IAssemblySymbol left, IAssemblySymbol right) + { + AssemblyMapper mapper = new(_settings); + mapper.AddElement(left, 0); + mapper.AddElement(right, 1); + + DiferenceVisitor visitor = new(noWarn: NoWarn, ignoredDifferences: IgnoredDifferences); + visitor.Visit(mapper); + return visitor.Differences; + } + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/AssemblySymbolLoader.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/AssemblySymbolLoader.cs new file mode 100644 index 000000000000..8e8c31e6ee0b --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/AssemblySymbolLoader.cs @@ -0,0 +1,296 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Reflection.Metadata; +using System.Reflection.PortableExecutable; + +namespace Microsoft.DotNet.ApiCompatibility +{ + public class AssemblySymbolLoader + { + private readonly HashSet _referenceSearchPaths = new(); + private readonly List _warnings = new(); + private readonly Dictionary _loadedAssemblies; + private readonly bool _resolveReferences; + private CSharpCompilation _cSharpCompilation; + + public AssemblySymbolLoader(string assemblyName = "", bool resolveAssemblyReferences = false) + { + if (string.IsNullOrEmpty(assemblyName)) + { + assemblyName = $"AssemblyLoader_{DateTime.Now:MM_dd_yy_HH_mm_ss_FFF}"; + } + + _loadedAssemblies = new Dictionary(); + var compilationOptions = new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary, nullableContextOptions: NullableContextOptions.Enable); + _cSharpCompilation = CSharpCompilation.Create(assemblyName, options: compilationOptions); + _resolveReferences = resolveAssemblyReferences; + } + + public void AddReferenceSearchDirectories(string paths) => AddReferenceSearchDirectories(SplitPaths(paths)); + + public void AddReferenceSearchDirectories(IEnumerable paths) + { + if (paths == null) + { + throw new ArgumentNullException(nameof(paths)); + } + + foreach (string path in paths) + _referenceSearchPaths.Add(path); + } + + public bool HasRoslynDiagnostics(out IEnumerable diagnostics) + { + diagnostics = _cSharpCompilation.GetDiagnostics(); + return diagnostics.Any(); + } + + public bool HasLoadWarnings(out IEnumerable warnings) + { + warnings = _warnings; + return _warnings.Count > 0; + } + + private static string[] SplitPaths(string paths) => + paths == null ? null : paths.Split(new char[] { ',', ';' }, StringSplitOptions.RemoveEmptyEntries); + + public IEnumerable LoadAssemblies(string paths) => LoadAssemblies(SplitPaths(paths)); + + public IEnumerable LoadAssemblies(IEnumerable paths) + { + if (paths == null) + { + throw new ArgumentNullException(nameof(paths)); + } + + IEnumerable assembliesToReturn = LoadFromPaths(paths); + + List result = new List(); + foreach (MetadataReference metadataReference in assembliesToReturn) + { + ISymbol symbol = _cSharpCompilation.GetAssemblyOrModuleSymbol(metadataReference); + if (symbol is IAssemblySymbol assemblySymbol) + { + result.Add(assemblySymbol); + } + } + + return result; + } + + public IAssemblySymbol LoadAssembly(string path) + { + if (path == null) + { + throw new ArgumentNullException(nameof(path)); + } + + MetadataReference metadataReference = CreateOrGetMetadataReferenceFromPath(path); + return (IAssemblySymbol)_cSharpCompilation.GetAssemblyOrModuleSymbol(metadataReference); + } + + public IAssemblySymbol LoadAssembly(string name, Stream stream) + { + if (name == null) + { + throw new ArgumentNullException(nameof(name)); + } + + if (stream == null) + { + throw new ArgumentNullException(nameof(stream)); + } + + if (!_loadedAssemblies.TryGetValue(name, out MetadataReference metadataReference)) + { + metadataReference = CreateAndAddReferenceToCompilation(name, stream); + } + + return (IAssemblySymbol)_cSharpCompilation.GetAssemblyOrModuleSymbol(metadataReference); + } + + public IAssemblySymbol LoadAssemblyFromSourceFiles(IEnumerable filePaths, IEnumerable referencePaths) + { + if (filePaths == null || filePaths.Count() == 0) + { + throw new ArgumentNullException(nameof(filePaths), $"Should not be null and contain at least one element"); + } + + List syntaxTrees = new(); + foreach (string filePath in filePaths) + { + if (!File.Exists(filePath)) + { + throw new FileNotFoundException($"File '{filePath}' does not exist."); + } + + syntaxTrees.Add(CSharpSyntaxTree.ParseText(File.ReadAllText(filePath))); + } + + _cSharpCompilation = _cSharpCompilation.AddSyntaxTrees(syntaxTrees); + + LoadFromPaths(referencePaths); + return _cSharpCompilation.Assembly; + } + + public IEnumerable LoadMatchingAssemblies(IEnumerable fromAssemblies, IEnumerable searchPaths, bool validateMatchingIdentity = true, bool errorOnMissingAssemblies = true) + { + if (fromAssemblies == null) + { + throw new ArgumentNullException(nameof(fromAssemblies)); + } + + if (searchPaths == null) + { + throw new ArgumentNullException(nameof(searchPaths)); + } + + List matchingAssemblies = new List(); + foreach (IAssemblySymbol assembly in fromAssemblies) + { + bool found = false; + string name = $"{assembly.Name}.dll"; + foreach (string directory in searchPaths) + { + if (!Directory.Exists(directory)) + { + throw new FileNotFoundException($"Matching assembly search directory '{directory}' does not exist", nameof(searchPaths)); + } + + string possiblePath = Path.Combine(directory, name); + if (File.Exists(possiblePath)) + { + MetadataReference reference = CreateOrGetMetadataReferenceFromPath(possiblePath); + ISymbol symbol = _cSharpCompilation.GetAssemblyOrModuleSymbol(reference); + if (symbol is IAssemblySymbol matchingAssembly) + { + if (validateMatchingIdentity && !matchingAssembly.Identity.Equals(assembly.Identity)) + { + _cSharpCompilation = _cSharpCompilation.RemoveReferences(new[] { reference }); + _loadedAssemblies.Remove(name); + continue; + } + + matchingAssemblies.Add(matchingAssembly); + found = true; + break; + } + } + } + + if (errorOnMissingAssemblies && !found) + { + string assemblyInfo = validateMatchingIdentity ? assembly.Identity.GetDisplayName() : assembly.Name; + _warnings.Add($"Could not find matching assembly: '{assemblyInfo}' in any of the search directories."); + } + } + + return matchingAssemblies; + } + + private IEnumerable LoadFromPaths(IEnumerable paths) + { + List result = new List(); + foreach (string path in paths) + { + string resolvedPath = Environment.ExpandEnvironmentVariables(path); + string directory = null; + if (Directory.Exists(resolvedPath)) + { + directory = resolvedPath; + result.AddRange(LoadAssembliesFromDirectory(resolvedPath)); + } + else if (File.Exists(resolvedPath)) + { + directory = Path.GetDirectoryName(resolvedPath); + result.Add(CreateOrGetMetadataReferenceFromPath(resolvedPath)); + } + else + { + _warnings.Add($"Could not find the provided path '{resolvedPath}' to load binaries from."); + } + + if (_resolveReferences && !string.IsNullOrEmpty(directory)) + _referenceSearchPaths.Add(directory); + } + + return result; + } + + private IEnumerable LoadAssembliesFromDirectory(string directory) + { + foreach (string assembly in Directory.EnumerateFiles(directory, "*.dll")) + { + yield return CreateOrGetMetadataReferenceFromPath(assembly); + } + } + + private MetadataReference CreateOrGetMetadataReferenceFromPath(string path) + { + // Roslyn doesn't support having two assemblies as references with the same identity and then getting the symbol for it. + string name = Path.GetFileName(path); + if (!_loadedAssemblies.TryGetValue(name, out MetadataReference metadataReference)) + { + using FileStream stream = File.OpenRead(path); + metadataReference = CreateAndAddReferenceToCompilation(name, stream); + } + + return metadataReference; + } + + private MetadataReference CreateAndAddReferenceToCompilation(string name, Stream fileStream) + { + // If we need to resolve references we can't reuse the same stream after creating the metadata + // reference from it as Roslyn closes it. So instead we use PEReader and get the bytes + // and create the metadata reference from that. + using PEReader reader = new(fileStream); + PEMemoryBlock image = reader.GetEntireImage(); + MetadataReference metadataReference = MetadataReference.CreateFromImage(image.GetContent()); + _loadedAssemblies.Add(name, metadataReference); + _cSharpCompilation = _cSharpCompilation.AddReferences(new MetadataReference[] { metadataReference }); + + if (_resolveReferences) + { + ResolveReferences(reader); + } + + + return metadataReference; + } + + private void ResolveReferences(PEReader peReader) + { + MetadataReader reader = peReader.GetMetadataReader(); + foreach (AssemblyReferenceHandle handle in reader.AssemblyReferences) + { + AssemblyReference reference = reader.GetAssemblyReference(handle); + string name = $"{reader.GetString(reference.Name)}.dll"; + bool found = _loadedAssemblies.TryGetValue(name, out MetadataReference _); + if (!found) + { + foreach (string directory in _referenceSearchPaths) + { + string potentialPath = Path.Combine(directory, name); + if (File.Exists(potentialPath)) + { + // TODO: add version check and add a warning if it doesn't match? + using FileStream resolvedStream = File.OpenRead(potentialPath); + CreateAndAddReferenceToCompilation(name, resolvedStream); + found = true; + break; + } + } + } + + if (!found) + { + _warnings.Add($"Could not resolve reference '{name}' in any of the provided search directories."); + } + } + } + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DefaultSymbolsEqualityComparer.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DefaultSymbolsEqualityComparer.cs new file mode 100644 index 000000000000..8fb3168fc39e --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DefaultSymbolsEqualityComparer.cs @@ -0,0 +1,26 @@ +using Microsoft.CodeAnalysis; +using System; +using System.Collections.Generic; + +namespace Microsoft.DotNet.ApiCompatibility +{ + public class DefaultSymbolsEqualityComparer : IEqualityComparer + { + public bool Equals(ISymbol x, ISymbol y) => + string.Equals(GetKey(x), GetKey(y), StringComparison.OrdinalIgnoreCase); + + public int GetHashCode(ISymbol obj) => + GetKey(obj).GetHashCode(); + + private static string GetKey(ISymbol symbol) => + symbol switch + { + IMethodSymbol => symbol.ToDisplayString(), + IFieldSymbol => symbol.ToDisplayString(), + IPropertySymbol => symbol.ToDisplayString(), + IEventSymbol => symbol.ToDisplayString(), + ITypeSymbol => symbol.ToDisplayString(), + _ => symbol.Name, + }; + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticBag.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticBag.cs new file mode 100644 index 000000000000..9c94996d04db --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticBag.cs @@ -0,0 +1,56 @@ +using Microsoft.CodeAnalysis; +using Microsoft.DotNet.ApiCompatibility.Abstractions; +using System; +using System.Collections.Generic; + +namespace Microsoft.DotNet.ApiCompatibility +{ + public class DiagnosticBag where T : IDiagnostic + { + private readonly Dictionary> _ignore; + private readonly HashSet _noWarn; + + private readonly List _differences = new(); + + public DiagnosticBag(string noWarn, (string diagnosticId, string referenceId)[] ignoredDifferences) + { + _noWarn = new HashSet(noWarn?.Split(';')); + _ignore = new Dictionary>(); + + foreach (var ignored in ignoredDifferences) + { + if (!_ignore.TryGetValue(ignored.diagnosticId, out HashSet members)) + { + members = new HashSet(); + _ignore.Add(ignored.diagnosticId, members); + } + + members.Add(ignored.referenceId); + } + } + + public void AddRange(IEnumerable differences) + { + foreach (T difference in differences) + Add(difference); + } + + public void Add(T difference) + { + if (_noWarn.Contains(difference.DiagnosticId)) + return; + + if (_ignore.TryGetValue(difference.DiagnosticId, out HashSet members)) + { + if (members.Contains(difference.ReferenceId)) + { + return; + } + } + + _differences.Add(difference); + } + + public IEnumerable Differences => _differences; + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticIds.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticIds.cs new file mode 100644 index 000000000000..48b47d720aeb --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticIds.cs @@ -0,0 +1,8 @@ +namespace Microsoft.DotNet.ApiCompatibility +{ + public static class DiagnosticIds + { + public const string TypeMustExist = "CP0001"; + public const string MemberMustExist = "CP0002"; + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiferenceVisitor.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiferenceVisitor.cs new file mode 100644 index 000000000000..48d905e61474 --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiferenceVisitor.cs @@ -0,0 +1,39 @@ +using Microsoft.DotNet.ApiCompatibility.Abstractions; +using System; +using System.Collections.Generic; + +namespace Microsoft.DotNet.ApiCompatibility +{ + public class DiferenceVisitor : MapperVisitor + { + private readonly DiagnosticBag _differenceBag; + + public DiferenceVisitor(string noWarn = null, (string diagnosticId, string memberId)[] ignoredDifferences = null) + { + _differenceBag = new DiagnosticBag(noWarn ?? string.Empty, ignoredDifferences ?? Array.Empty<(string, string)>()); + } + + public override void Visit(AssemblyMapper assembly) + { + _differenceBag.AddRange(assembly.GetDifferences()); + base.Visit(assembly); + } + + public override void Visit(TypeMapper type) + { + _differenceBag.AddRange(type.GetDifferences()); + + if (type.ShouldDiffMembers) + { + base.Visit(type); + } + } + + public override void Visit(MemberMapper member) + { + _differenceBag.AddRange(member.GetDifferences()); + } + + public IEnumerable Differences => _differenceBag.Differences; + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiffingSettings.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiffingSettings.cs new file mode 100644 index 000000000000..cf7285fa8414 --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiffingSettings.cs @@ -0,0 +1,21 @@ +using Microsoft.CodeAnalysis; +using Microsoft.DotNet.ApiCompatibility.Abstractions; +using Microsoft.DotNet.ApiCompatibility.Rules; +using System.Collections.Generic; + +namespace Microsoft.DotNet.ApiCompatibility +{ + public class DiffingSettings + { + public IRuleDriverFactory RuleDriverFactory { get; } + public IDiffingFilter Filter { get; } + public IEqualityComparer EqualityComparer { get; } + + public DiffingSettings(IRuleDriverFactory ruleDriverFactory = null, IDiffingFilter filter = null, IEqualityComparer equalityComparer = null) + { + RuleDriverFactory = ruleDriverFactory ?? new RuleDriverFactory(); + Filter = filter ?? new AccessibilityFilter(includeInternalSymbols: false); + EqualityComparer = equalityComparer ?? new DefaultSymbolsEqualityComparer(); + } + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Extensions/TypeSymbolExtensions.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Extensions/TypeSymbolExtensions.cs new file mode 100644 index 000000000000..43da17157526 --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Extensions/TypeSymbolExtensions.cs @@ -0,0 +1,29 @@ +using Microsoft.CodeAnalysis; +using System; +using System.Collections.Generic; +using System.Text; + +namespace Microsoft.DotNet.ApiCompatibility.Extensions +{ + internal static class TypeSymbolExtensions + { + internal static IEnumerable GetAllBaseTypes(this ITypeSymbol type) + { + if (type.TypeKind == TypeKind.Interface) + { + foreach (ITypeSymbol @interface in type.Interfaces) + { + yield return @interface; + foreach (ITypeSymbol baseInterface in @interface.GetAllBaseTypes()) + yield return baseInterface; + } + } + else if (type.BaseType != null) + { + yield return type.BaseType; + foreach (ITypeSymbol baseType in type.BaseType.GetAllBaseTypes()) + yield return baseType; + } + } + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Microsoft.DotNet.ApiCompatibility.csproj b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Microsoft.DotNet.ApiCompatibility.csproj new file mode 100644 index 000000000000..ce1ea9aa3968 --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Microsoft.DotNet.ApiCompatibility.csproj @@ -0,0 +1,18 @@ + + + + netstandard2.0 + true + true + Open + 0.1.0 + + RS1024;$(NoWarn) + + + + + + + + diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/MembersMustExist.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/MembersMustExist.cs new file mode 100644 index 000000000000..4bcfdb8c4ed5 --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/MembersMustExist.cs @@ -0,0 +1,88 @@ +using Microsoft.CodeAnalysis; +using Microsoft.DotNet.ApiCompatibility.Abstractions; +using Microsoft.DotNet.ApiCompatibility.Extensions; +using System; +using System.Collections.Generic; +using System.Text; + +namespace Microsoft.DotNet.ApiCompatibility.Rules +{ + public class MembersMustExist : Rule + { + public override void Run(TypeMapper mapper, List differences) + { + ITypeSymbol left = mapper.Left; + if (left != null && mapper.Right == null) + differences.Add(new CompatDifference(DiagnosticIds.TypeMustExist, $"Type '{left.ToDisplayString()}' exists on the contract but not on the implementation", DifferenceType.Removed, left)); + } + + public override void Run(MemberMapper mapper, List differences) + { + ISymbol left = mapper.Left; + if (left != null && mapper.Right == null) + { + // Events and properties are handled via their accessors. + if (left.Kind == SymbolKind.Property || left.Kind == SymbolKind.Event) + return; + + if (left is IMethodSymbol method) + { + // Will be handled by a different rule + if (method.MethodKind == MethodKind.ExplicitInterfaceImplementation) + return; + + // If method is an override or hides a base type definition removing it from right is compatible. + if (method.IsOverride || FindMatchingOnBaseType(method)) + return; + } + + differences.Add(new CompatDifference(DiagnosticIds.MemberMustExist, $"Member '{left.ToDisplayString()}' exists on the contract but not on the implementation", DifferenceType.Removed, left)); + } + } + + private bool FindMatchingOnBaseType(IMethodSymbol method) + { + foreach (ITypeSymbol type in method.ContainingType.GetAllBaseTypes()) + { + foreach (ISymbol symbol in type.GetMembers()) + { + if (symbol is IMethodSymbol candidate && IsMatchingMethod(method, candidate)) + return true; + } + } + + return false; + } + + private bool IsMatchingMethod(IMethodSymbol method, IMethodSymbol candidate) + { + if (method.Name == candidate.Name) + { + if (ParametersMatch(method, candidate) && ReturnTypesMatch(method, candidate)) + { + if (method.TypeParameters.Length == candidate.TypeParameters.Length) + return true; + } + } + + return false; + } + + private bool ReturnTypesMatch(IMethodSymbol method, IMethodSymbol candidate) => + method.ReturnType.ToDisplayString() == candidate.ReturnType.ToDisplayString(); + + private bool ParametersMatch(IMethodSymbol method, IMethodSymbol candidate) + { + if (method.Parameters.Length != candidate.Parameters.Length) + return false; + + for (int i = 0; i < method.Parameters.Length; i++) + { + if (method.Parameters[i].Type.ToDisplayString() != method.Parameters[i].Type.ToDisplayString()) + return false; + } + + return true; + } + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleDriver.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleDriver.cs new file mode 100644 index 000000000000..812df73294ce --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleDriver.cs @@ -0,0 +1,90 @@ +using Microsoft.CodeAnalysis; +using Microsoft.DotNet.ApiCompatibility.Abstractions; +using System; +using System.Collections.Generic; +using System.Linq; + +namespace Microsoft.DotNet.ApiCompatibility.Rules +{ + public interface IRuleDriverFactory + { + IRuleDriver GetRuleDriver(); + } + + public class RuleDriverFactory : IRuleDriverFactory + { + private RuleDriver _driver; + public IRuleDriver GetRuleDriver() + { + if (_driver == null) + _driver = new RuleDriver(); + + return _driver; + } + } + + public interface IRuleDriver + { + IEnumerable Run(ElementMapper mapper); + } + + internal class RuleDriver : IRuleDriver + { + private readonly IEnumerable _rules; + + internal RuleDriver() + { + _rules = GetRules(); + } + + public IEnumerable Run(ElementMapper mapper) + { + List differences = new(); + if (mapper is AssemblyMapper am) + { + Run(am, differences); + } + if (mapper is TypeMapper tm) + { + Run(tm, differences); + } + if (mapper is MemberMapper mm) + { + Run(mm, differences); + } + + return differences; + } + + private void Run(AssemblyMapper mapper, List differences) + { + foreach (Rule rule in _rules) + { + rule.Run(mapper, differences); + } + } + + private void Run(TypeMapper mapper, List differences) + { + foreach (Rule rule in _rules) + { + rule.Run(mapper, differences); + } + } + + private void Run(MemberMapper mapper, List differences) + { + foreach (Rule rule in _rules) + { + rule.Run(mapper, differences); + } + } + + private IEnumerable GetRules() + { + return this.GetType().Assembly.GetTypes() + .Where(t => !t.IsAbstract && typeof(Rule).IsAssignableFrom(t)) + .Select(t => (Rule)Activator.CreateInstance(t)); + } + } +} diff --git a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/AssemblySymbolLoaderTests.cs b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/AssemblySymbolLoaderTests.cs new file mode 100644 index 000000000000..6736975d46a0 --- /dev/null +++ b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/AssemblySymbolLoaderTests.cs @@ -0,0 +1,356 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. +// + +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Threading; +using System.Xml.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.DotNet.Cli.Utils; +using Microsoft.NET.TestFramework; +using Microsoft.NET.TestFramework.Assertions; +using Microsoft.NET.TestFramework.Commands; +using Microsoft.NET.TestFramework.ProjectConstruction; +using Xunit; +using Xunit.Abstractions; + +namespace Microsoft.DotNet.ApiCompatibility.Tests +{ + public class AssemblySymbolLoaderTests : SdkTest + { + public AssemblySymbolLoaderTests(ITestOutputHelper log) : base(log) { } + + private const string SimpleAssemblySourceContents = @" +namespace MyNamespace +{ + public class MyClass + { + } +} +"; + + // Since we uset typeof(string).Assembly.Location to resolve references + // We need to target a framework compatible with what the test is being + // built for so that we resolve the references correctly. +#if NETCOREAPP + private const string TargetFrameworks = "net5.0"; +#else + private const string TargetFrameworks = "net471"; +#endif + + private class TestAssetInfo + { + public TestAsset TestAsset { get; set; } + public string OutputDirectory { get; set; } + } + + // We use the same asset in multiple tests. + // Creating a TestAsset and building it for each test + // creates a lot of overhead, using the cache + // speeds up test execution ~3x in this test assembly. + // Tests within the same class run serially in xunit, so + // it is fine to reuse the same asset. + private class TestAssetCache + { + public static TestAssetCache Instance = new TestAssetCache(); + + private ConcurrentDictionary Dictionary = new(); + + private TestAssetInfo _asset = null; + + public TestAssetInfo GetSimpleAsset(TestAssetsManager manager) + => _asset ?? InitAsset(manager); + + private TestAssetInfo InitAsset(TestAssetsManager manager) + { + Interlocked.CompareExchange(ref _asset, GetAsset(manager), null); + return _asset; + } + + private TestAssetInfo GetAsset(TestAssetsManager manager) + { + TestProject project = new("SimpleAsset") + { + TargetFrameworks = TargetFrameworks, + IsExe = false + }; + + project.SourceFiles.Add("MyClass.cs", SimpleAssemblySourceContents); + + TestAsset testAsset = manager.CreateTestProject(project); + BuildTestAsset(testAsset, out string outDir) + .Should() + .Pass(); + + return new TestAssetInfo() + { + TestAsset = testAsset, + OutputDirectory = outDir, + }; + } + } + + private TestAssetInfo GetSimpleTestAsset() => TestAssetCache.Instance.GetSimpleAsset(_testAssetsManager); + + [Fact] + public void LoadAssembly_Throws() + { + AssemblySymbolLoader loader = new(); + Assert.Throws(() => loader.LoadAssembly(Guid.NewGuid().ToString("N").Substring(0, 8))); + Assert.Throws("path", () => loader.LoadAssembly(null)); + Assert.Throws("stream", () => loader.LoadAssembly("Assembly", null)); + Assert.Throws("name", () => loader.LoadAssembly(null, new MemoryStream())); + } + + [Fact] + public void LoadAssemblies_Throws() + { + AssemblySymbolLoader loader = new(); + Assert.Throws("paths", () => loader.LoadAssemblies((string)null)); + Assert.Throws("paths", () => loader.LoadAssemblies((IEnumerable)null)); + } + + [Fact] + public void LoadAssemblyFromSourceFiles_Throws() + { + AssemblySymbolLoader loader = new(); + IEnumerable paths = new[] { Guid.NewGuid().ToString("N") }; + Assert.Throws(() => loader.LoadAssemblyFromSourceFiles(paths, Array.Empty())); + Assert.Throws("filePaths", () => loader.LoadAssemblyFromSourceFiles(null, Array.Empty())); + Assert.Throws("filePaths", () => loader.LoadAssemblyFromSourceFiles(Array.Empty(), Array.Empty())); + } + + [Fact] + public void LoadMatchingAssemblies_Throws() + { + AssemblySymbolLoader loader = new(); + IEnumerable paths = new[] { Guid.NewGuid().ToString("N") }; + IAssemblySymbol assembly = SymbolFactory.GetAssemblyFromSyntax("namespace MyNamespace { class Foo { } }"); + + Assert.Throws(() => loader.LoadMatchingAssemblies(new[] { assembly }, paths)); + Assert.Throws("fromAssemblies", () => loader.LoadMatchingAssemblies(null, paths)); + Assert.Throws("searchPaths", () => loader.LoadMatchingAssemblies(Array.Empty(), null)); + } + + [Fact] + public void LoadMatchingAssembliesWarns() + { + IAssemblySymbol assembly = SymbolFactory.GetAssemblyFromSyntax("namespace MyNamespace { class Foo { } }"); + IEnumerable paths = new[] { AppContext.BaseDirectory }; + + AssemblySymbolLoader loader = new(); + IEnumerable symbols = loader.LoadMatchingAssemblies(new[] { assembly }, paths); + Assert.Empty(symbols); + Assert.True(loader.HasLoadWarnings(out IEnumerable warnings)); + + IEnumerable expected = new[] + { + $"Could not find matching assembly: '{assembly.Identity.GetDisplayName()}' in any of the search directories." + }; + + Assert.Equal(expected, warnings, StringComparer.Ordinal); + } + + [Fact] + public void LoadMatchingAssembliesSameIdentitySucceeds() + { + string assemblyName = nameof(LoadMatchingAssembliesSameIdentitySucceeds); + IAssemblySymbol fromAssembly = SymbolFactory.GetAssemblyFromSyntax(SimpleAssemblySourceContents, assemblyName: assemblyName); + + TestProject testProject = new(assemblyName) + { + TargetFrameworks = "net5.0", + IsExe = false, + }; + + testProject.SourceFiles.Add("MyClass.cs", SimpleAssemblySourceContents); + testProject.AdditionalProperties.Add("AssemblyVersion", "0.0.0.0"); + TestAsset testAsset = _testAssetsManager.CreateTestProject(testProject); + + BuildTestAsset(testAsset, out string outputDirectory) + .Should() + .Pass(); + + AssemblySymbolLoader loader = new(); + IEnumerable matchingAssemblies = loader.LoadMatchingAssemblies(new[] { fromAssembly }, new[] { outputDirectory }); + + Assert.Single(matchingAssemblies); + Assert.False(loader.HasLoadWarnings(out var _)); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void LoadMatchingAssemblies_DifferentIdentity(bool validateIdentities) + { + var assetInfo = GetSimpleTestAsset(); + IAssemblySymbol fromAssembly = SymbolFactory.GetAssemblyFromSyntax(SimpleAssemblySourceContents, assemblyName: assetInfo.TestAsset.TestProject.Name); + + AssemblySymbolLoader loader = new(); + IEnumerable matchingAssemblies = loader.LoadMatchingAssemblies(new[] { fromAssembly }, new[] { assetInfo.OutputDirectory }, validateMatchingIdentity: validateIdentities); + + if (validateIdentities) + { + Assert.Empty(matchingAssemblies); + Assert.True(loader.HasLoadWarnings(out IEnumerable warnings)); + + IEnumerable expected = new[] + { + $"Could not find matching assembly: '{fromAssembly.Identity.GetDisplayName()}' in any of the search directories." + }; + + Assert.Equal(expected, warnings, StringComparer.Ordinal); + } + else + { + Assert.Single(matchingAssemblies); + Assert.False(loader.HasLoadWarnings(out var _)); + Assert.NotEqual(fromAssembly.Identity, matchingAssemblies.FirstOrDefault().Identity); + } + } + + [Fact] + public void LoadsSimpleAssemblyFromDirectory() + { + var assetInfo = GetSimpleTestAsset(); + AssemblySymbolLoader loader = new(); + IEnumerable symbols = loader.LoadAssemblies(assetInfo.OutputDirectory); + Assert.Single(symbols); + + IEnumerable types = symbols.FirstOrDefault() + .GlobalNamespace + .GetNamespaceMembers() + .FirstOrDefault() + .GetTypeMembers(); + + Assert.Single(types); + Assert.Equal("MyNamespace.MyClass", types.FirstOrDefault().ToDisplayString()); + } + + [Fact] + public void LoadSimpleAssemblyFullPath() + { + var assetInfo = GetSimpleTestAsset(); + AssemblySymbolLoader loader = new(); + IAssemblySymbol symbol = loader.LoadAssembly(Path.Combine(assetInfo.OutputDirectory, assetInfo.TestAsset.TestProject.Name + ".dll")); + + IEnumerable types = symbol.GlobalNamespace + .GetNamespaceMembers() + .FirstOrDefault() + .GetTypeMembers(); + + Assert.Single(types); + Assert.Equal("MyNamespace.MyClass", types.FirstOrDefault().ToDisplayString()); + } + + [Fact] + public void LoadsMultipleAssembliesFromDirectory() + { + TestProject first = new("LoadsMultipleAssembliesFromDirectory_First") + { + TargetFrameworks = TargetFrameworks, + IsExe = false + }; + + TestProject second = new("LoadsMultipleAssembliesFromDirectory_Second") + { + TargetFrameworks = TargetFrameworks, + IsExe = false + }; + + first.ReferencedProjects.Add(second); + TestAsset testAsset = _testAssetsManager.CreateTestProject(first); + + BuildTestAsset(testAsset, out string outputDirectory) + .Should() + .Pass(); + + AssemblySymbolLoader loader = new(); + IEnumerable symbols = loader.LoadAssemblies(outputDirectory); + + Assert.Equal(2, symbols.Count()); + + IEnumerable expected = new[] { "LoadsMultipleAssembliesFromDirectory_First", "LoadsMultipleAssembliesFromDirectory_Second" }; + IEnumerable actual = symbols.Select(a => a.Name); + + Assert.Equal(expected, actual, StringComparer.Ordinal); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void LoadAssemblyResolveReferences_WarnsWhenEnabled(bool resolveReferences) + { + var assetInfo = GetSimpleTestAsset(); + AssemblySymbolLoader loader = new(resolveAssemblyReferences: resolveReferences); + loader.LoadAssembly(Path.Combine(assetInfo.OutputDirectory, assetInfo.TestAsset.TestProject.Name + ".dll")); + + if (resolveReferences) + { + Assert.True(loader.HasLoadWarnings(out IEnumerable warnings)); + + string expectedReference = "System.Runtime.dll"; + + if (TargetFrameworks.StartsWith("net4", StringComparison.OrdinalIgnoreCase)) + { + expectedReference = "mscorlib.dll"; + } + + IEnumerable expected = new[] + { + $"Could not resolve reference '{expectedReference}' in any of the provided search directories." + }; + + Assert.Equal(expected, warnings, StringComparer.Ordinal); + } + else + { + Assert.False(loader.HasLoadWarnings(out IEnumerable warnings)); + Assert.Empty(warnings); + } + } + + [Fact] + public void LoadAssembliesShouldResolveReferencesNoWarnings() + { + var assetInfo = GetSimpleTestAsset(); + AssemblySymbolLoader loader = new(resolveAssemblyReferences: true); + loader.AddReferenceSearchDirectories(Path.GetDirectoryName(typeof(string).Assembly.Location)); + loader.LoadAssembly(Path.Combine(assetInfo.OutputDirectory, assetInfo.TestAsset.TestProject.Name + ".dll")); + + Assert.False(loader.HasLoadWarnings(out IEnumerable warnings)); + Assert.Empty(warnings); + } + + [Fact] + public void LoadAssemblyFromStreamNoWarns() + { + var assetInfo = GetSimpleTestAsset(); + TestProject testProject = assetInfo.TestAsset.TestProject; + AssemblySymbolLoader loader = new(); + using FileStream stream = File.OpenRead(Path.Combine(assetInfo.OutputDirectory, testProject.Name + ".dll")); + IAssemblySymbol symbol = loader.LoadAssembly(testProject.Name, stream); + + Assert.Equal(testProject.Name, symbol.Name, StringComparer.Ordinal); + + IEnumerable types = symbol.GlobalNamespace + .GetNamespaceMembers() + .FirstOrDefault() + .GetTypeMembers(); + + Assert.Single(types); + Assert.Equal("MyNamespace.MyClass", types.FirstOrDefault().ToDisplayString()); + } + + private static CommandResult BuildTestAsset(TestAsset testAsset, out string outputDirectory) + { + BuildCommand buildCommand = new(testAsset); + outputDirectory = buildCommand.GetOutputDirectory(testAsset.TestProject.TargetFrameworks).FullName; + return buildCommand.Execute(); + } + } +} diff --git a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Microsoft.DotNet.ApiCompatibility.Tests.csproj b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Microsoft.DotNet.ApiCompatibility.Tests.csproj new file mode 100644 index 000000000000..dbf5425a530e --- /dev/null +++ b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Microsoft.DotNet.ApiCompatibility.Tests.csproj @@ -0,0 +1,18 @@ + + + + false + net472;$(ToolsetTargetFramework) + $(ToolsetTargetFramework) + Exe + + true + true + + + + + + + + diff --git a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/MembersMustExistTests.cs b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/MembersMustExistTests.cs new file mode 100644 index 000000000000..facaa0a84266 --- /dev/null +++ b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/MembersMustExistTests.cs @@ -0,0 +1,233 @@ +using Microsoft.CodeAnalysis; +using Microsoft.DotNet.ApiCompatibility.Abstractions; +using System.Collections.Generic; +using Xunit; + +namespace Microsoft.DotNet.ApiCompatibility.Tests.Rules +{ + public class MembersMustExistTests + { + [Fact] + public static void MissingMembersAreReported() + { + string leftSyntax = @" +namespace CompatTests +{ + public class First + { + public string Parameterless() { } + public void ShouldReportMethod(string a, string b) { } + public string ShouldReportMissingProperty { get; } + public string this[int index] { get; } + public event EventHandler ShouldReportMissingEvent; + public int ReportMissingField = 0; + } + + public delegate void EventHandler(object sender EventArgs e); +} +"; + + string rightSyntax = @" +namespace CompatTests +{ + public class First + { + public string Parameterless() { } + } + public delegate void EventHandler(object sender EventArgs e); +} +"; + + IAssemblySymbol left = SymbolFactory.GetAssemblyFromSyntax(leftSyntax); + IAssemblySymbol right = SymbolFactory.GetAssemblyFromSyntax(rightSyntax); + ApiDiffer differ = new(); + IEnumerable differences = differ.GetDifferences(new[] { left }, new[] { right }); + + CompatDifference[] expected = new[] + { + new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.ShouldReportMethod(string, string)' exists on the contract but not on the implementation", DifferenceType.Removed, "M:CompatTests.First.ShouldReportMethod(System.String,System.String)"), + new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.ShouldReportMissingProperty.get' exists on the contract but not on the implementation", DifferenceType.Removed, "M:CompatTests.First.get_ShouldReportMissingProperty"), + new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.this[int].get' exists on the contract but not on the implementation", DifferenceType.Removed, "M:CompatTests.First.get_Item(System.Int32)"), + new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.ShouldReportMissingEvent.add' exists on the contract but not on the implementation", DifferenceType.Removed, "M:CompatTests.First.add_ShouldReportMissingEvent(CompatTests.EventHandler)"), + new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.ShouldReportMissingEvent.remove' exists on the contract but not on the implementation", DifferenceType.Removed, "M:CompatTests.First.remove_ShouldReportMissingEvent(CompatTests.EventHandler)"), + new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.ReportMissingField' exists on the contract but not on the implementation", DifferenceType.Removed, "F:CompatTests.First.ReportMissingField"), + }; + + Assert.Equal(expected, differences); + } + + [Fact] + public static void HiddenMemberInLeftIsNotReported() + { + string leftSyntax = @" +namespace CompatTests +{ + public class FirstBase + { + public void MyMethod() { } + public string MyMethodWithParams(string a, int b, FirstBase c) { } + public T MyGenericMethod(string name, T2 a, T3 b) { } + public virtual string MyVirtualMethod() { } + } + public class Second : FirstBase + { + public new void MyMethod() { } + public new string MyMethodWithParams(string a, int b, FirstBase c) { } + public new T MyGenericMethod(string name, T2 a, T3 b) { } + public override string MyVirtualMethod() { } + } +} +"; + + string rightSyntax = @" +namespace CompatTests +{ + public class FirstBase + { + public void MyMethod() { } + public string MyMethodWithParams(string a, int b, FirstBase c) { } + public T MyGenericMethod(string name, T2 a, T3 b) { } + public virtual string MyVirtualMethod() { } + } + public class Second : FirstBase { } +} +"; + + IAssemblySymbol left = SymbolFactory.GetAssemblyFromSyntax(leftSyntax); + IAssemblySymbol right = SymbolFactory.GetAssemblyFromSyntax(rightSyntax); + ApiDiffer differ = new(); + IEnumerable differences = differ.GetDifferences(new[] { left }, new[] { right }); + Assert.Empty(differences); + } + + [Fact] + public static void NoDifferencesWithNoWarn() + { + string leftSyntax = @" +namespace CompatTests +{ + public class First + { + public void MissingMember() { } + public int MissingProperty { get; } + public int MissingField; + } +} +"; + + string rightSyntax = @" +namespace CompatTests +{ + public class First + { + } +} +"; + + IAssemblySymbol left = SymbolFactory.GetAssemblyFromSyntax(leftSyntax); + IAssemblySymbol right = SymbolFactory.GetAssemblyFromSyntax(rightSyntax); + ApiDiffer differ = new(); + differ.NoWarn = DiagnosticIds.MemberMustExist; + IEnumerable differences = differ.GetDifferences(new[] { left }, new[] { right }); + Assert.Empty(differences); + } + + [Fact] + public static void MultipleOverridesAreReported() + { + string leftSyntax = @" +namespace CompatTests +{ + public class First + { + public string MultipleOverrides() { } + public string MultipleOverrides(string a) { } + public string MultipleOverrides(string a, string b) { } + public string MultipleOverrides(string a, int b, string c) { } + public string MultipleOverrides(string a, int b, int c) { } + } +} +"; + + string rightSyntax = @" +namespace CompatTests +{ + public class First + { + public string MultipleOverrides() { } + public string MultipleOverrides(string a) { } + public string MultipleOverrides(string a, int b, int c) { } + } +} +"; + + IAssemblySymbol left = SymbolFactory.GetAssemblyFromSyntax(leftSyntax); + IAssemblySymbol right = SymbolFactory.GetAssemblyFromSyntax(rightSyntax); + ApiDiffer differ = new(); + IEnumerable differences = differ.GetDifferences(new[] { left }, new[] { right }); + + CompatDifference[] expected = new[] + { + new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.MultipleOverrides(string, string)' exists on the contract but not on the implementation", DifferenceType.Removed, "M:CompatTests.First.MultipleOverrides(System.String,System.String)"), + new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.MultipleOverrides(string, int, string)' exists on the contract but not on the implementation", DifferenceType.Removed, "M:CompatTests.First.MultipleOverrides(System.String,System.Int32,System.String)"), + }; + + Assert.Equal(expected, differences); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public static void IncludeInternalsIsRespectedForMembers_IndividualAssemblies(bool includeInternals) + { + string leftSyntax = @" +namespace CompatTests +{ + public class First + { + public string MultipleOverrides() { } + public string MultipleOverrides(string a) { } + public string MultipleOverrides(string a, string b) { } + public string MultipleOverrides(string a, int b, string c) { } + internal string MultipleOverrides(string a, int b, int c) { } + internal int InternalProperty { get; set; } + } +} +"; + + string rightSyntax = @" +namespace CompatTests +{ + public class First + { + public string MultipleOverrides() { } + public string MultipleOverrides(string a) { } + public string MultipleOverrides(string a, string b) { } + public string MultipleOverrides(string a, int b, string c) { } + internal int InternalProperty { get; } + } +} +"; + + IAssemblySymbol left = SymbolFactory.GetAssemblyFromSyntax(leftSyntax); + IAssemblySymbol right = SymbolFactory.GetAssemblyFromSyntax(rightSyntax, assemblyName: "DifferentName"); + ApiDiffer differ = new(includeInternalSymbols: includeInternals); + IEnumerable differences = differ.GetDifferences(left, right); + + if (includeInternals) + { + CompatDifference[] expected = new[] + { + new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.MultipleOverrides(string, int, int)' exists on the contract but not on the implementation", DifferenceType.Removed, "M:CompatTests.First.MultipleOverrides(System.String,System.Int32,System.Int32)"), + new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.InternalProperty.set' exists on the contract but not on the implementation", DifferenceType.Removed, "M:CompatTests.First.set_InternalProperty(System.Int32)"), + }; + + Assert.Equal(expected, differences); + } + else + { + Assert.Empty(differences); + } + } + } +} diff --git a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/TypeMustExistTests.cs b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/TypeMustExistTests.cs new file mode 100644 index 000000000000..8ff6a2693c20 --- /dev/null +++ b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/TypeMustExistTests.cs @@ -0,0 +1,277 @@ +using Microsoft.CodeAnalysis; +using Microsoft.DotNet.ApiCompatibility.Abstractions; +using System.Collections.Generic; +using Xunit; + +namespace Microsoft.DotNet.ApiCompatibility.Tests +{ + public class TypeMustExistTests + { + [Theory] + [InlineData("")] + [InlineData("CP002")] + public void MissingPublicTypesInRightAreReported(string noWarn) + { + string leftSyntax = @" + +namespace CompatTests +{ + public class First { } + public class Second { } + public record MyRecord(string a, string b); + public struct MyStruct { } + public delegate void MyDelegate(object a); + public enum MyEnum { } +} +"; + + string rightSyntax = @" +namespace CompatTests +{ + public class First { } +} +"; + + ApiDiffer differ = new(); + differ.NoWarn = noWarn; + bool enableNullable = false; + IAssemblySymbol left = SymbolFactory.GetAssemblyFromSyntax(leftSyntax, enableNullable); + IAssemblySymbol right = SymbolFactory.GetAssemblyFromSyntax(rightSyntax, enableNullable); + IEnumerable differences = differ.GetDifferences(new[] { left }, new[] { right }); + + CompatDifference[] expected = new[] + { + new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.Second' exists on the contract but not on the implementation", DifferenceType.Removed, "T:CompatTests.Second"), + new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.MyRecord' exists on the contract but not on the implementation", DifferenceType.Removed, "T:CompatTests.MyRecord"), + new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.MyStruct' exists on the contract but not on the implementation", DifferenceType.Removed, "T:CompatTests.MyStruct"), + new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.MyDelegate' exists on the contract but not on the implementation", DifferenceType.Removed, "T:CompatTests.MyDelegate"), + new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.MyEnum' exists on the contract but not on the implementation", DifferenceType.Removed, "T:CompatTests.MyEnum"), + }; + + Assert.Equal(expected, differences); + } + + [Fact] + public void MissingTypeFromTypeForwardIsReported() + { + string forwardedTypeSyntax = @" +namespace CompatTests +{ + public class ForwardedTestType { } +} +"; + string leftSyntax = @" +[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(CompatTests.ForwardedTestType))] +namespace CompatTests +{ + public class First { } +} +"; + + string rightSyntax = @" +namespace CompatTests +{ + public class First { } +} +"; + IAssemblySymbol left = SymbolFactory.GetAssemblyFromSyntaxWithReferences(leftSyntax, new[] { forwardedTypeSyntax }, includeDefaultReferences: true); + IAssemblySymbol right = SymbolFactory.GetAssemblyFromSyntax(rightSyntax); + ApiDiffer differ = new(); + IEnumerable differences = differ.GetDifferences(new[] { left }, new[] { right }); + + CompatDifference[] expected = new[] + { + new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.ForwardedTestType' exists on the contract but not on the implementation", DifferenceType.Removed, "T:CompatTests.ForwardedTestType") + }; + + Assert.Equal(expected, differences); + } + + [Fact] + public void TypeForwardExistsOnBoth() + { + string forwardedTypeSyntax = @" +namespace CompatTests +{ + public class ForwardedTestType { } +} +"; + string syntax = @" +[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(CompatTests.ForwardedTestType))] +namespace CompatTests +{ + public class First { } +} +"; + IEnumerable references = new[] { forwardedTypeSyntax }; + IAssemblySymbol left = SymbolFactory.GetAssemblyFromSyntaxWithReferences(syntax, references, includeDefaultReferences: true); + IAssemblySymbol right = SymbolFactory.GetAssemblyFromSyntaxWithReferences(syntax, references, includeDefaultReferences: true); + ApiDiffer differ = new(); + Assert.Empty(differ.GetDifferences(new[] { left }, new[] { right })); + } + + [Fact] + public void NoDifferencesReportedWithNoWarn() + { + string leftSyntax = @" + +namespace CompatTests +{ + public class First { } + public class Second { } +} +"; + + string rightSyntax = @" +namespace CompatTests +{ + public class First { } +} +"; + + ApiDiffer differ = new(); + differ.NoWarn = DiagnosticIds.TypeMustExist; + IAssemblySymbol left = SymbolFactory.GetAssemblyFromSyntax(leftSyntax); + IAssemblySymbol right = SymbolFactory.GetAssemblyFromSyntax(rightSyntax); + Assert.Empty(differ.GetDifferences(new[] { left }, new[] { right })); + } + + [Fact] + public void DifferenceIsIgnoredForMember() + { + string leftSyntax = @" + +namespace CompatTests +{ + public record First(string a, string b); + public class Second { } + public class Third { } + public class Fourth { } + public enum MyEnum { } +} +"; + + string rightSyntax = @" +namespace CompatTests +{ + public record First(string a, string b); +} +"; + + (string, string)[] ignoredDifferences = new[] + { + (DiagnosticIds.TypeMustExist, "T:CompatTests.Second"), + (DiagnosticIds.TypeMustExist, "T:CompatTests.MyEnum"), + }; + + ApiDiffer differ = new(); + differ.IgnoredDifferences = ignoredDifferences; + + IAssemblySymbol left = SymbolFactory.GetAssemblyFromSyntax(leftSyntax); + IAssemblySymbol right = SymbolFactory.GetAssemblyFromSyntax(rightSyntax); + IEnumerable differences = differ.GetDifferences(new[] { left }, new[] { right }); + + CompatDifference[] expected = new[] + { + new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.Third' exists on the contract but not on the implementation", DifferenceType.Removed, "T:CompatTests.Third"), + new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.Fourth' exists on the contract but not on the implementation", DifferenceType.Removed, "T:CompatTests.Fourth") + }; + + Assert.Equal(expected, differences); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void InternalTypesAreIgnoredWhenSpecified(bool includeInternalSymbols) + { + string leftSyntax = @" + +namespace CompatTests +{ + public class First { } + internal class InternalType { } +} +"; + + string rightSyntax = @" +namespace CompatTests +{ + public class First { } +} +"; + + ApiDiffer differ = new(includeInternalSymbols); + IAssemblySymbol left = SymbolFactory.GetAssemblyFromSyntax(leftSyntax); + IAssemblySymbol right = SymbolFactory.GetAssemblyFromSyntax(rightSyntax); + IEnumerable differences = differ.GetDifferences(new[] { left }, new[] { right }); + + if (!includeInternalSymbols) + { + Assert.Empty(differences); + } + else + { + CompatDifference[] expected = new[] + { + new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.InternalType' exists on the contract but not on the implementation", DifferenceType.Removed, "T:CompatTests.InternalType") + }; + + Assert.Equal(expected, differences); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public static void MissingNestedTypeIsReported(bool includeInternalSymbols) + { + string leftSyntax = @" + +namespace CompatTests +{ + public class First + { + public class FirstNested + { + public class SecondNested { } + } + internal class InternalNested + { + internal class DoubleNested { } + } + } +} +"; + + string rightSyntax = @" +namespace CompatTests +{ + public class First + { + internal class InternalNested { } + } +} +"; + + ApiDiffer differ = new(includeInternalSymbols); + IAssemblySymbol left = SymbolFactory.GetAssemblyFromSyntax(leftSyntax); + IAssemblySymbol right = SymbolFactory.GetAssemblyFromSyntax(rightSyntax); + IEnumerable differences = differ.GetDifferences(new[] { left }, new[] { right }); + + List expected = new() + { + new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.First.FirstNested' exists on the contract but not on the implementation", DifferenceType.Removed, "T:CompatTests.First.FirstNested"), + }; + + if (includeInternalSymbols) + { + expected.Add( + new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.First.InternalNested.DoubleNested' exists on the contract but not on the implementation", DifferenceType.Removed, "T:CompatTests.First.InternalNested.DoubleNested") + ); + } + + Assert.Equal(expected, differences); + } + } +} diff --git a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/SymbolFactory.cs b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/SymbolFactory.cs new file mode 100644 index 000000000000..44e622dea436 --- /dev/null +++ b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/SymbolFactory.cs @@ -0,0 +1,52 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using System.Collections.Generic; +using System.Linq; +using System.Runtime.CompilerServices; + +namespace Microsoft.DotNet.ApiCompatibility.Tests +{ + internal static class SymbolFactory + { + internal static IAssemblySymbol GetAssemblyFromSyntax(string syntax, bool enableNullable = false, bool includeDefaultReferences = false, [CallerMemberName] string assemblyName = "") + { + CSharpCompilation compilation = CreateCSharpCompilationFromSyntax(syntax, assemblyName, enableNullable, includeDefaultReferences); + return compilation.Assembly; + } + + internal static IAssemblySymbol GetAssemblyFromSyntaxWithReferences(string syntax, IEnumerable referencesSyntax, bool enableNullable = false, bool includeDefaultReferences = false, [CallerMemberName] string assemblyName = "") + { + CSharpCompilation compilation = CreateCSharpCompilationFromSyntax(syntax, assemblyName, enableNullable, includeDefaultReferences); + CSharpCompilation compilationWithReferences = CreateCSharpCompilationFromSyntax(referencesSyntax, $"{assemblyName}_reference", enableNullable, includeDefaultReferences); + + compilation = compilation.AddReferences(compilationWithReferences.ToMetadataReference()); + return compilation.Assembly; + } + + private static CSharpCompilation CreateCSharpCompilationFromSyntax(string syntax, string name, bool enableNullable, bool includeDefaultReferences) + { + CSharpCompilation compilation = CreateCSharpCompilation(name, enableNullable, includeDefaultReferences); + return compilation.AddSyntaxTrees(CSharpSyntaxTree.ParseText(syntax)); + } + + private static CSharpCompilation CreateCSharpCompilationFromSyntax(IEnumerable syntax, string name, bool enableNullable, bool includeDefaultReferences) + { + CSharpCompilation compilation = CreateCSharpCompilation(name, enableNullable, includeDefaultReferences); + IEnumerable syntaxTrees = syntax.Select(s => CSharpSyntaxTree.ParseText(s)); + return compilation.AddSyntaxTrees(syntaxTrees); + } + + private static CSharpCompilation CreateCSharpCompilation(string name, bool enableNullable, bool includeDefaultReferences) + { + var compilationOptions = new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary, + nullableContextOptions: enableNullable ? NullableContextOptions.Enable : NullableContextOptions.Disable); + + return CSharpCompilation.Create(name, options: compilationOptions, references: includeDefaultReferences ? DefaultReferences : null); + } + + private static IEnumerable DefaultReferences { get; } = new[] + { + MetadataReference.CreateFromFile(typeof(object).Assembly.Location), + }; + } +} From c54dac87665c4c0d6eb4245e08dd418ecefc6e98 Mon Sep 17 00:00:00 2001 From: Santiago Fernandez Madero Date: Fri, 9 Apr 2021 10:38:29 -0700 Subject: [PATCH 2/6] Add headers and cleanup usings --- .../Abstractions/CompatDifference.cs | 5 ++++- .../Abstractions/DifferenceType.cs | 5 ++++- .../Abstractions/Filtering/AccessibilityFilter.cs | 5 ++++- .../Abstractions/Filtering/IDiffingFilter.cs | 8 ++++---- .../Abstractions/IDiagnostic.cs | 5 ++++- .../Abstractions/MapperVisitor.cs | 5 ++++- .../Abstractions/Mappers/AssemblyMapper.cs | 7 ++++--- .../Abstractions/Mappers/AssemblySetMapper.cs | 4 ++-- .../Abstractions/Mappers/ElementMapper.cs | 5 +++-- .../Abstractions/Mappers/MemberMapper.cs | 8 ++++---- .../Abstractions/Mappers/NamespaceMapper.cs | 6 ++++-- .../Abstractions/Mappers/TypeMapper.cs | 5 ++++- .../Abstractions/Rule.cs | 6 +++--- .../Microsoft.DotNet.ApiCompatibility/ApiDiffer.cs | 6 ++++-- .../AssemblySymbolLoader.cs | 5 ++++- .../DefaultSymbolsEqualityComparer.cs | 5 ++++- .../Microsoft.DotNet.ApiCompatibility/DiagnosticBag.cs | 5 +++-- .../Microsoft.DotNet.ApiCompatibility/DiagnosticIds.cs | 5 ++++- .../Microsoft.DotNet.ApiCompatibility/DiferenceVisitor.cs | 5 ++++- .../Microsoft.DotNet.ApiCompatibility/DiffingSettings.cs | 5 ++++- .../Extensions/TypeSymbolExtensions.cs | 7 ++++--- .../Rules/MembersMustExist.cs | 7 ++++--- .../Microsoft.DotNet.ApiCompatibility/Rules/RuleDriver.cs | 4 +++- .../AssemblySymbolLoaderTests.cs | 2 -- .../Rules/MembersMustExistTests.cs | 5 ++++- .../Rules/TypeMustExistTests.cs | 3 +++ .../SymbolFactory.cs | 5 ++++- 27 files changed, 97 insertions(+), 46 deletions(-) diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/CompatDifference.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/CompatDifference.cs index cf7e31425d97..9527dc5f96a8 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/CompatDifference.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/CompatDifference.cs @@ -1,4 +1,7 @@ -using Microsoft.CodeAnalysis; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.CodeAnalysis; using System; namespace Microsoft.DotNet.ApiCompatibility.Abstractions diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/DifferenceType.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/DifferenceType.cs index 3666614545e2..152f3e18e397 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/DifferenceType.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/DifferenceType.cs @@ -1,4 +1,7 @@ -namespace Microsoft.DotNet.ApiCompatibility.Abstractions +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +namespace Microsoft.DotNet.ApiCompatibility.Abstractions { public enum DifferenceType { diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/AccessibilityFilter.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/AccessibilityFilter.cs index 6cc726926130..a354bb99e964 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/AccessibilityFilter.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/AccessibilityFilter.cs @@ -1,4 +1,7 @@ -using Microsoft.CodeAnalysis; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.CodeAnalysis; namespace Microsoft.DotNet.ApiCompatibility.Abstractions { diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/IDiffingFilter.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/IDiffingFilter.cs index 7d8bf5acadce..8a78f983ad36 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/IDiffingFilter.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/IDiffingFilter.cs @@ -1,7 +1,7 @@ -using Microsoft.CodeAnalysis; -using System; -using System.Collections.Generic; -using System.Text; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.CodeAnalysis; namespace Microsoft.DotNet.ApiCompatibility.Abstractions { diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/IDiagnostic.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/IDiagnostic.cs index 6aa79584c1f5..fa54fa0cd1d1 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/IDiagnostic.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/IDiagnostic.cs @@ -1,4 +1,7 @@ -namespace Microsoft.DotNet.ApiCompatibility.Abstractions +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +namespace Microsoft.DotNet.ApiCompatibility.Abstractions { public interface IDiagnostic { diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/MapperVisitor.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/MapperVisitor.cs index 683a7e0207ae..c0dd99d67c4e 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/MapperVisitor.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/MapperVisitor.cs @@ -1,4 +1,7 @@ -namespace Microsoft.DotNet.ApiCompatibility.Abstractions +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +namespace Microsoft.DotNet.ApiCompatibility.Abstractions { public class MapperVisitor { diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblyMapper.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblyMapper.cs index d9b7240d8534..f53ab3ff90c0 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblyMapper.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblyMapper.cs @@ -1,7 +1,8 @@ -using Microsoft.CodeAnalysis; -using System; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.CodeAnalysis; using System.Collections.Generic; -using System.Text; namespace Microsoft.DotNet.ApiCompatibility.Abstractions { diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblySetMapper.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblySetMapper.cs index ab97947bf547..0874dc619af4 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblySetMapper.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblySetMapper.cs @@ -1,7 +1,7 @@ using Microsoft.CodeAnalysis; -using System; using System.Collections.Generic; -using System.Text; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. namespace Microsoft.DotNet.ApiCompatibility.Abstractions { diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/ElementMapper.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/ElementMapper.cs index 727af20ee6c0..ec6fd43ef415 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/ElementMapper.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/ElementMapper.cs @@ -1,7 +1,8 @@ -using Microsoft.DotNet.ApiCompatibility.Rules; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + using System; using System.Collections.Generic; -using System.Text; namespace Microsoft.DotNet.ApiCompatibility.Abstractions { diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/MemberMapper.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/MemberMapper.cs index 4e16e78e6966..c9df6b3f8ca4 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/MemberMapper.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/MemberMapper.cs @@ -1,7 +1,7 @@ -using Microsoft.CodeAnalysis; -using System; -using System.Collections.Generic; -using System.Text; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.CodeAnalysis; namespace Microsoft.DotNet.ApiCompatibility.Abstractions { diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/NamespaceMapper.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/NamespaceMapper.cs index feb218d8eea8..2c4f4ae79373 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/NamespaceMapper.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/NamespaceMapper.cs @@ -1,7 +1,9 @@ -using Microsoft.CodeAnalysis; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.CodeAnalysis; using System; using System.Collections.Generic; -using System.Linq; namespace Microsoft.DotNet.ApiCompatibility.Abstractions { diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/TypeMapper.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/TypeMapper.cs index 8f02e504fdcd..1be705ab9240 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/TypeMapper.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/TypeMapper.cs @@ -1,4 +1,7 @@ -using Microsoft.CodeAnalysis; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.CodeAnalysis; using System.Collections.Generic; namespace Microsoft.DotNet.ApiCompatibility.Abstractions diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Rule.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Rule.cs index 817cee2962ff..841c36f1b881 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Rule.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Rule.cs @@ -1,7 +1,7 @@ -using Microsoft.CodeAnalysis; -using System; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + using System.Collections.Generic; -using System.Text; namespace Microsoft.DotNet.ApiCompatibility.Abstractions { diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ApiDiffer.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ApiDiffer.cs index cdbde94b4e01..dc65a3641361 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ApiDiffer.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ApiDiffer.cs @@ -1,6 +1,8 @@ -using Microsoft.CodeAnalysis; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.CodeAnalysis; using Microsoft.DotNet.ApiCompatibility.Abstractions; -using System; using System.Collections.Generic; namespace Microsoft.DotNet.ApiCompatibility diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/AssemblySymbolLoader.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/AssemblySymbolLoader.cs index 8e8c31e6ee0b..27e5cc497aad 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/AssemblySymbolLoader.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/AssemblySymbolLoader.cs @@ -1,4 +1,7 @@ -using Microsoft.CodeAnalysis; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using System; using System.Collections.Generic; diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DefaultSymbolsEqualityComparer.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DefaultSymbolsEqualityComparer.cs index 8fb3168fc39e..1901edb08e9f 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DefaultSymbolsEqualityComparer.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DefaultSymbolsEqualityComparer.cs @@ -1,4 +1,7 @@ -using Microsoft.CodeAnalysis; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.CodeAnalysis; using System; using System.Collections.Generic; diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticBag.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticBag.cs index 9c94996d04db..588806603073 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticBag.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticBag.cs @@ -1,6 +1,7 @@ -using Microsoft.CodeAnalysis; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + using Microsoft.DotNet.ApiCompatibility.Abstractions; -using System; using System.Collections.Generic; namespace Microsoft.DotNet.ApiCompatibility diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticIds.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticIds.cs index 48b47d720aeb..47c1f7218f71 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticIds.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticIds.cs @@ -1,4 +1,7 @@ -namespace Microsoft.DotNet.ApiCompatibility +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +namespace Microsoft.DotNet.ApiCompatibility { public static class DiagnosticIds { diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiferenceVisitor.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiferenceVisitor.cs index 48d905e61474..a8c2edd03e98 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiferenceVisitor.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiferenceVisitor.cs @@ -1,4 +1,7 @@ -using Microsoft.DotNet.ApiCompatibility.Abstractions; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.DotNet.ApiCompatibility.Abstractions; using System; using System.Collections.Generic; diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiffingSettings.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiffingSettings.cs index cf7285fa8414..5fde21d3fb3c 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiffingSettings.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiffingSettings.cs @@ -1,4 +1,7 @@ -using Microsoft.CodeAnalysis; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.CodeAnalysis; using Microsoft.DotNet.ApiCompatibility.Abstractions; using Microsoft.DotNet.ApiCompatibility.Rules; using System.Collections.Generic; diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Extensions/TypeSymbolExtensions.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Extensions/TypeSymbolExtensions.cs index 43da17157526..095c0e4432a6 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Extensions/TypeSymbolExtensions.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Extensions/TypeSymbolExtensions.cs @@ -1,7 +1,8 @@ -using Microsoft.CodeAnalysis; -using System; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.CodeAnalysis; using System.Collections.Generic; -using System.Text; namespace Microsoft.DotNet.ApiCompatibility.Extensions { diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/MembersMustExist.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/MembersMustExist.cs index 4bcfdb8c4ed5..6c9e073695ba 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/MembersMustExist.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/MembersMustExist.cs @@ -1,9 +1,10 @@ -using Microsoft.CodeAnalysis; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.CodeAnalysis; using Microsoft.DotNet.ApiCompatibility.Abstractions; using Microsoft.DotNet.ApiCompatibility.Extensions; -using System; using System.Collections.Generic; -using System.Text; namespace Microsoft.DotNet.ApiCompatibility.Rules { diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleDriver.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleDriver.cs index 812df73294ce..12e98eb4f902 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleDriver.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleDriver.cs @@ -1,4 +1,6 @@ -using Microsoft.CodeAnalysis; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information.s + using Microsoft.DotNet.ApiCompatibility.Abstractions; using System; using System.Collections.Generic; diff --git a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/AssemblySymbolLoaderTests.cs b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/AssemblySymbolLoaderTests.cs index 6736975d46a0..6923fb387322 100644 --- a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/AssemblySymbolLoaderTests.cs +++ b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/AssemblySymbolLoaderTests.cs @@ -1,6 +1,5 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. -// using System; using System.Collections.Concurrent; @@ -8,7 +7,6 @@ using System.IO; using System.Linq; using System.Threading; -using System.Xml.Linq; using Microsoft.CodeAnalysis; using Microsoft.DotNet.Cli.Utils; using Microsoft.NET.TestFramework; diff --git a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/MembersMustExistTests.cs b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/MembersMustExistTests.cs index facaa0a84266..1ad1d91b35e2 100644 --- a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/MembersMustExistTests.cs +++ b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/MembersMustExistTests.cs @@ -1,4 +1,7 @@ -using Microsoft.CodeAnalysis; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.CodeAnalysis; using Microsoft.DotNet.ApiCompatibility.Abstractions; using System.Collections.Generic; using Xunit; diff --git a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/TypeMustExistTests.cs b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/TypeMustExistTests.cs index 8ff6a2693c20..71b8a365dee0 100644 --- a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/TypeMustExistTests.cs +++ b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/TypeMustExistTests.cs @@ -1,3 +1,6 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + using Microsoft.CodeAnalysis; using Microsoft.DotNet.ApiCompatibility.Abstractions; using System.Collections.Generic; diff --git a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/SymbolFactory.cs b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/SymbolFactory.cs index 44e622dea436..513efd53661c 100644 --- a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/SymbolFactory.cs +++ b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/SymbolFactory.cs @@ -1,4 +1,7 @@ -using Microsoft.CodeAnalysis; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using System.Collections.Generic; using System.Linq; From 244e6035ec61a300e5ecedcf98cfa8767aa16a9d Mon Sep 17 00:00:00 2001 From: Santiago Fernandez Madero Date: Fri, 9 Apr 2021 12:38:55 -0700 Subject: [PATCH 3/6] Fix tests on non-Windows --- .../AssemblySymbolLoaderTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/AssemblySymbolLoaderTests.cs b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/AssemblySymbolLoaderTests.cs index 6923fb387322..d96661a532e1 100644 --- a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/AssemblySymbolLoaderTests.cs +++ b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/AssemblySymbolLoaderTests.cs @@ -273,7 +273,7 @@ public void LoadsMultipleAssembliesFromDirectory() Assert.Equal(2, symbols.Count()); IEnumerable expected = new[] { "LoadsMultipleAssembliesFromDirectory_First", "LoadsMultipleAssembliesFromDirectory_Second" }; - IEnumerable actual = symbols.Select(a => a.Name); + IEnumerable actual = symbols.Select(a => a.Name).OrderBy(a => a, StringComparer.Ordinal); Assert.Equal(expected, actual, StringComparer.Ordinal); } From 59bdddda37a4a720f50e5b9c8696abd25b44a3e0 Mon Sep 17 00:00:00 2001 From: Santiago Fernandez Madero Date: Mon, 12 Apr 2021 19:17:03 -0700 Subject: [PATCH 4/6] Add docs, small fixes and cleanup --- .../Abstractions/CompatDifference.cs | 48 ++++++++- .../Abstractions/DifferenceType.cs | 3 + .../Filtering/AccessibilityFilter.cs | 2 +- .../Abstractions/Filtering/IDiffingFilter.cs | 12 --- .../Abstractions/Filtering/ISymbolFilter.cs | 20 ++++ .../Abstractions/IDiagnostic.cs | 10 ++ .../Abstractions/IRuleRunner.cs | 21 ++++ .../Abstractions/IRuleRunnerFactory.cs | 13 +++ .../Abstractions/MapperVisitor.cs | 28 ++++++ .../Abstractions/Mappers/AssemblyMapper.cs | 15 ++- .../Abstractions/Mappers/AssemblySetMapper.cs | 12 ++- .../Abstractions/Mappers/ElementMapper.cs | 34 ++++++- .../Abstractions/Mappers/MemberMapper.cs | 9 +- .../Abstractions/Mappers/NamespaceMapper.cs | 22 ++++- .../Abstractions/Mappers/TypeMapper.cs | 22 ++++- .../Abstractions/Rule.cs | 20 ++++ .../ApiComparer.cs | 86 ++++++++++++++++ .../ApiDiffer.cs | 51 ---------- .../AssemblySymbolLoader.cs | 98 +++++++++++++++++-- .../ComparingSettings.cs | 44 +++++++++ .../DefaultSymbolsEqualityComparer.cs | 2 +- .../DiagnosticBag.cs | 29 +++++- .../DiagnosticIds.cs | 3 + .../DiferenceVisitor.cs | 25 ++++- .../DiffingSettings.cs | 24 ----- .../Rules/MembersMustExist.cs | 14 +++ .../Rules/RuleDriverFactory.cs | 19 ++++ .../Rules/{RuleDriver.cs => RuleRunner.cs} | 28 +----- .../AssemblySymbolLoaderTests.cs | 8 +- .../Rules/MembersMustExistTests.cs | 10 +- .../Rules/TypeMustExistTests.cs | 14 +-- 31 files changed, 591 insertions(+), 155 deletions(-) delete mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/IDiffingFilter.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/ISymbolFilter.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/IRuleRunner.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/IRuleRunnerFactory.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/ApiComparer.cs delete mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/ApiDiffer.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/ComparingSettings.cs delete mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiffingSettings.cs create mode 100644 src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleDriverFactory.cs rename src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/{RuleDriver.cs => RuleRunner.cs} (75%) diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/CompatDifference.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/CompatDifference.cs index 9527dc5f96a8..f1148965e318 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/CompatDifference.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/CompatDifference.cs @@ -6,20 +6,52 @@ namespace Microsoft.DotNet.ApiCompatibility.Abstractions { + /// + /// Class representing a difference of compatibility, containing detailed information about it. + /// public class CompatDifference : IDiagnostic, IEquatable { + /// + /// The Diagnostic ID for this difference. + /// public string DiagnosticId { get; } + + /// + /// The . + /// public DifferenceType Type { get; } + + /// + /// A diagnostic message for the difference. + /// public virtual string Message { get; } + + /// + /// A unique ID in order to identify the API that the difference was raised for. + /// public string ReferenceId { get; } private CompatDifference() { } + /// + /// Instanciates a new object representing the compatibility difference. + /// + /// representing the diagnostic ID. + /// message describing the difference. + /// to describe the type of the difference. + /// for which the difference is associated to. public CompatDifference(string id, string message, DifferenceType type, ISymbol member) : this(id, message, type, member.GetDocumentationCommentId()) { } + /// + /// Instanciates a new object representing the compatibility difference. + /// + /// representing the diagnostic ID. + /// message describing the difference. + /// to describe the type of the difference. + /// containing the member ID for which the difference is associated to. public CompatDifference(string id, string message, DifferenceType type, string memberId) { DiagnosticId = id; @@ -28,15 +60,29 @@ public CompatDifference(string id, string message, DifferenceType type, string m ReferenceId = memberId; } - public bool Equals(CompatDifference other) => + /// + /// Evaluates whether the current object is equal to another . + /// + /// to compare against. + /// True if equals, False if different. + public bool Equals(CompatDifference other) => + other != null && Type == other.Type && DiagnosticId.Equals(other.DiagnosticId, StringComparison.OrdinalIgnoreCase) && ReferenceId.Equals(other.ReferenceId, StringComparison.OrdinalIgnoreCase) && Message.Equals(other.Message, StringComparison.OrdinalIgnoreCase); + /// + /// Gets the hashcode that reperesents this instance. + /// + /// Unique based on the properties' values of the instance. public override int GetHashCode() => HashCode.Combine(ReferenceId, DiagnosticId, Message, Type); + /// + /// Gets a representation of the difference. + /// + /// describing the difference. public override string ToString() => $"{DiagnosticId} : {Message}"; } } diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/DifferenceType.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/DifferenceType.cs index 152f3e18e397..30042793526f 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/DifferenceType.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/DifferenceType.cs @@ -3,6 +3,9 @@ namespace Microsoft.DotNet.ApiCompatibility.Abstractions { + /// + /// Enum representing the different type of differences available. + /// public enum DifferenceType { Changed, diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/AccessibilityFilter.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/AccessibilityFilter.cs index a354bb99e964..201bb7816e03 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/AccessibilityFilter.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/AccessibilityFilter.cs @@ -5,7 +5,7 @@ namespace Microsoft.DotNet.ApiCompatibility.Abstractions { - internal class AccessibilityFilter : IDiffingFilter + internal class AccessibilityFilter : ISymbolFilter { private readonly bool _includeInternalSymbols; diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/IDiffingFilter.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/IDiffingFilter.cs deleted file mode 100644 index 8a78f983ad36..000000000000 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/IDiffingFilter.cs +++ /dev/null @@ -1,12 +0,0 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using Microsoft.CodeAnalysis; - -namespace Microsoft.DotNet.ApiCompatibility.Abstractions -{ - public interface IDiffingFilter - { - bool Include(ISymbol symbol); - } -} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/ISymbolFilter.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/ISymbolFilter.cs new file mode 100644 index 000000000000..6b06a0d05ff9 --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/ISymbolFilter.cs @@ -0,0 +1,20 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.CodeAnalysis; + +namespace Microsoft.DotNet.ApiCompatibility.Abstractions +{ + /// + /// Provides a mechanism to filter when building the . + /// + public interface ISymbolFilter + { + /// + /// Determines whether the should be included. + /// + /// to evaluate. + /// True to include the or false to filter it out. + bool Include(ISymbol symbol); + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/IDiagnostic.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/IDiagnostic.cs index fa54fa0cd1d1..c8e3540f1c00 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/IDiagnostic.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/IDiagnostic.cs @@ -3,9 +3,19 @@ namespace Microsoft.DotNet.ApiCompatibility.Abstractions { + /// + /// Interface that describes a diagnostic. + /// public interface IDiagnostic { + /// + /// String representing the diagnostic ID. + /// string DiagnosticId { get; } + + /// + /// String representing the ID for the object that the diagnostic was created for. + /// string ReferenceId { get; } } } diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/IRuleRunner.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/IRuleRunner.cs new file mode 100644 index 000000000000..4203622d2829 --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/IRuleRunner.cs @@ -0,0 +1,21 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information.s + +using System.Collections.Generic; + +namespace Microsoft.DotNet.ApiCompatibility.Abstractions +{ + /// + /// Interface for rule drivers to implement in order to be used returned by the + /// + public interface IRuleRunner + { + /// + /// Runs the registered rules on the mapper. + /// + /// The underlying type on the mapper. + /// The mapper to run the rules on. + /// + IEnumerable Run(ElementMapper mapper); + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/IRuleRunnerFactory.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/IRuleRunnerFactory.cs new file mode 100644 index 000000000000..eca973e74494 --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/IRuleRunnerFactory.cs @@ -0,0 +1,13 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information.s + +namespace Microsoft.DotNet.ApiCompatibility.Abstractions +{ + /// + /// The factory to create the driver that the differ should use based on the . + /// + public interface IRuleRunnerFactory + { + IRuleRunner GetRuleRunner(); + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/MapperVisitor.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/MapperVisitor.cs index c0dd99d67c4e..f046666faa1e 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/MapperVisitor.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/MapperVisitor.cs @@ -3,8 +3,16 @@ namespace Microsoft.DotNet.ApiCompatibility.Abstractions { + /// + /// Class that implements a visitor pattern to visit the tree for a given mapper. + /// public class MapperVisitor { + /// + /// Visits the tree for the given . + /// + /// Underlying type for the objects that the mapper holds. + /// to visit. public void Visit(ElementMapper mapper) { if (mapper is AssemblySetMapper assemblySetMapper) @@ -29,6 +37,10 @@ public void Visit(ElementMapper mapper) } } + /// + /// Visits the and visits each in the mapper. + /// + /// The to visit. public virtual void Visit(AssemblySetMapper mapper) { foreach (AssemblyMapper assembly in mapper.GetAssemblies()) @@ -37,6 +49,10 @@ public virtual void Visit(AssemblySetMapper mapper) } } + /// + /// Visits the and visits each in the mapper. + /// + /// The to visit. public virtual void Visit(AssemblyMapper mapper) { foreach (NamespaceMapper nsMapper in mapper.GetNamespaces()) @@ -45,6 +61,10 @@ public virtual void Visit(AssemblyMapper mapper) } } + /// + /// Visits the and visits each in the mapper. + /// + /// The to visit. public virtual void Visit(NamespaceMapper mapper) { foreach (TypeMapper type in mapper.GetTypes()) @@ -53,6 +73,10 @@ public virtual void Visit(NamespaceMapper mapper) } } + /// + /// Visits the and visits the nested types and members in the mapper. + /// + /// The to visit. public virtual void Visit(TypeMapper mapper) { foreach (TypeMapper type in mapper.GetNestedTypes()) @@ -66,6 +90,10 @@ public virtual void Visit(TypeMapper mapper) } } + /// + /// Visits the . + /// + /// The to visit. public virtual void Visit(MemberMapper mapper) { } } } diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblyMapper.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblyMapper.cs index f53ab3ff90c0..c547c96902a0 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblyMapper.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblyMapper.cs @@ -6,12 +6,25 @@ namespace Microsoft.DotNet.ApiCompatibility.Abstractions { + /// + /// Object that represents a mapping between two objects. + /// This also holds a list of to represent the mapping of namespaces in between + /// and . + /// public class AssemblyMapper : ElementMapper { private Dictionary _namespaces; - public AssemblyMapper(DiffingSettings settings) : base(settings) { } + /// + /// Instantiates an object with the provided . + /// + /// The settings used to diff the elements in the mapper. + public AssemblyMapper(ComparingSettings settings) : base(settings) { } + /// + /// Gets the mappers for the namespaces contained in and + /// + /// The list of . public IEnumerable GetNamespaces() { if (_namespaces == null) diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblySetMapper.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblySetMapper.cs index 0874dc619af4..be728395ba25 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblySetMapper.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblySetMapper.cs @@ -5,12 +5,22 @@ namespace Microsoft.DotNet.ApiCompatibility.Abstractions { + /// + /// Object that represents a mapping between two lists of . + /// public class AssemblySetMapper : ElementMapper> { private Dictionary _assemblies; - public AssemblySetMapper(DiffingSettings settings) : base(settings) { } + /// + /// Instantiates an object with the provided . + /// + /// The settings used to diff the elements in the mapper. + public AssemblySetMapper(ComparingSettings settings) : base(settings) { } + /// + /// Gets the assembly mappers built from the provided lists of . + /// The list of representing the underlying assemblies. public IEnumerable GetAssemblies() { if (_assemblies == null) diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/ElementMapper.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/ElementMapper.cs index ec6fd43ef415..d64251109ea8 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/ElementMapper.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/ElementMapper.cs @@ -6,23 +6,45 @@ namespace Microsoft.DotNet.ApiCompatibility.Abstractions { + /// + /// Class that represents a mapping in between two objects of type . + /// public class ElementMapper { private IEnumerable _differences; + /// + /// Property representing the Left hand side of the mapping. + /// public T Left { get; private set; } + + /// + /// Property representing the Right hand side of the mapping. + /// public T Right { get; private set; } - public DiffingSettings Settings { get; } + /// + /// The used to diff and . + /// + public ComparingSettings Settings { get; } - public ElementMapper(DiffingSettings settings) + /// + /// Instantiates an object with the provided . + /// + /// The settings used to diff the elements in the mapper. + public ElementMapper(ComparingSettings settings) { Settings = settings; } + /// + /// Adds an element to the mapping given the index, 0 (Left) or 1 (Right). + /// + /// The element to add to the mapping. + /// Index representing the side of the mapping, 0 (Left) or 1 (Right). public virtual void AddElement(T element, int index) { - if (index < 0 || index > 1) + if ((uint)index > 1) throw new ArgumentOutOfRangeException(nameof(index), "index should either be 0 or 1"); if (index == 0) @@ -35,9 +57,13 @@ public virtual void AddElement(T element, int index) } } + /// + /// Runs the rules found by the rule driver on the element mapper and returns a list of differences. + /// + /// The list of . public IEnumerable GetDifferences() { - return _differences ??= Settings.RuleDriverFactory.GetRuleDriver().Run(this); + return _differences ??= Settings.RuleRunnerFactory.GetRuleRunner().Run(this); } } } diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/MemberMapper.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/MemberMapper.cs index c9df6b3f8ca4..92fc0f79fc46 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/MemberMapper.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/MemberMapper.cs @@ -5,8 +5,15 @@ namespace Microsoft.DotNet.ApiCompatibility.Abstractions { + /// + /// Object that represents a mapping between two objects. + /// public class MemberMapper : ElementMapper { - public MemberMapper(DiffingSettings settings) : base(settings) { } + /// + /// Instantiates an object with the provided . + /// + /// The settings used to diff the elements in the mapper. + public MemberMapper(ComparingSettings settings) : base(settings) { } } } diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/NamespaceMapper.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/NamespaceMapper.cs index 2c4f4ae79373..6ff25603e0be 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/NamespaceMapper.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/NamespaceMapper.cs @@ -7,14 +7,27 @@ namespace Microsoft.DotNet.ApiCompatibility.Abstractions { + /// + /// Object that represents a mapping between two objects. + /// This also holds a list of to represent the mapping of types in between + /// and . + /// public class NamespaceMapper : ElementMapper { private Dictionary _types; private IEnumerable _leftForwardedTypes; private IEnumerable _rightForwardedTypes; - public NamespaceMapper(DiffingSettings settings) : base(settings) { } + /// + /// Instantiates an object with the provided . + /// + /// The settings used to diff the elements in the mapper. + public NamespaceMapper(ComparingSettings settings) : base(settings) { } + /// + /// Gets all the representing the types defined in the namespace including the typeforwards. + /// + /// The mapper representing the types in the namespace public IEnumerable GetTypes() { if (_types == null) @@ -58,9 +71,14 @@ void AddOrCreateMappers(int index) return _types.Values; } + /// + /// Adds forwarded types to the mapper to the index specified in the mapper. + /// + /// List containing the that represents the forwarded types. + /// Index to add the forwarded types into, 0 or 1. public void AddForwardedTypes(IEnumerable forwardedTypes, int index) { - if (index < 0 || index > 1) + if ((uint)index > 1) throw new ArgumentOutOfRangeException(nameof(index), $"Value must be 0 or 1"); if (index == 0) diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/TypeMapper.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/TypeMapper.cs index 1be705ab9240..634052ff029b 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/TypeMapper.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/TypeMapper.cs @@ -6,15 +6,31 @@ namespace Microsoft.DotNet.ApiCompatibility.Abstractions { + /// + /// Object that represents a mapping between two objects. + /// This also holds the nested types as a list of and the members defined within the type + /// as a list of + /// public class TypeMapper : ElementMapper { private Dictionary _nestedTypes; private Dictionary _members; - public TypeMapper(DiffingSettings settings) : base(settings) { } + /// + /// Instantiates an object with the provided . + /// + /// The settings used to diff the elements in the mapper. + public TypeMapper(ComparingSettings settings) : base(settings) { } + /// + /// Indicates whether we have a complete mapper and if the members should be diffed. + /// public bool ShouldDiffMembers => Left != null && Right != null; + /// + /// Gets the nested types within the mapped types. + /// + /// The list of representing the nested types. public IEnumerable GetNestedTypes() { if (_nestedTypes == null) @@ -51,6 +67,10 @@ void AddOrCreateMappers(IEnumerable symbols, int index) return _nestedTypes.Values; } + /// + /// Gets the members defined in this type. + /// + /// The list of representing the members. public IEnumerable GetMembers() { if (_members == null) diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Rule.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Rule.cs index 841c36f1b881..b25e53add4ee 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Rule.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Rule.cs @@ -5,14 +5,34 @@ namespace Microsoft.DotNet.ApiCompatibility.Abstractions { + /// + /// Base class for Rules to use in order to be discovered and invoked by the + /// public abstract class Rule { + /// + /// Entrypoint to evaluate an + /// + /// The to evaluate. + /// The list of to add any differences to. public virtual void Run(AssemblyMapper mapper, List differences) { } + + /// + /// Entrypoint to evaluate an + /// + /// The to evaluate. + /// The list of to add any differences to. public virtual void Run(TypeMapper mapper, List differences) { } + + /// + /// Entrypoint to evaluate an + /// + /// The to evaluate. + /// The list of to add any differences to. public virtual void Run(MemberMapper mapper, List differences) { } diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ApiComparer.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ApiComparer.cs new file mode 100644 index 000000000000..5af6b4f74274 --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ApiComparer.cs @@ -0,0 +1,86 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.CodeAnalysis; +using Microsoft.DotNet.ApiCompatibility.Abstractions; +using System.Collections.Generic; + +namespace Microsoft.DotNet.ApiCompatibility +{ + /// + /// Class that performs api comparison based on ISymbol inputs. + /// + public class ApiComparer + { + private readonly ComparingSettings _settings; + + /// + /// Instanciates an object with the default . + /// + public ApiComparer() : this(new ComparingSettings()) { } + + /// + /// Instanciates an object with the provided + /// + /// Settings to use for comparison. + public ApiComparer(ComparingSettings settings) + { + _settings = settings; + } + + /// + /// Instanciates an object that includes comparing internal symbols. + /// + /// Indicates whether internal symbols should be included or not. + public ApiComparer(bool includeInternalSymbols) + { + _settings = new ComparingSettings(filter: new AccessibilityFilter(includeInternalSymbols)); + } + + /// + /// Comma separated list to ignore diagnostic IDs. + /// + public string NoWarn { get; set; } = string.Empty; + + /// + /// Array of diagnosticId and memberId to ignore those differences. + /// + public (string diagnosticId, string memberId)[] IgnoredDifferences { get; set; } + + /// + /// Get's the differences when comparing Left vs Right based on the settings when instanciating the object. + /// It compares two lists of symbols. + /// + /// Left symbols to compare against. + /// Right symbols to compare against. + /// List of found differences. + public IEnumerable GetDifferences(IEnumerable left, IEnumerable right) + { + AssemblySetMapper mapper = new(_settings); + mapper.AddElement(left, 0); + mapper.AddElement(right, 1); + + DiferenceVisitor visitor = new(noWarn: NoWarn, ignoredDifferences: IgnoredDifferences); + visitor.Visit(mapper); + return visitor.Differences; + } + + /// + /// Get's the differences when comparing Left vs Right based on the settings when instanciating the object. + /// It compares two symbols. + /// + /// Left symbol to compare against. + /// Right symbol to compare against. + /// List of found differences. + public IEnumerable GetDifferences(IAssemblySymbol left, IAssemblySymbol right) + { + AssemblyMapper mapper = new(_settings); + mapper.AddElement(left, 0); + mapper.AddElement(right, 1); + + DiferenceVisitor visitor = new(noWarn: NoWarn, ignoredDifferences: IgnoredDifferences); + visitor.Visit(mapper); + return visitor.Differences; + } + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ApiDiffer.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ApiDiffer.cs deleted file mode 100644 index dc65a3641361..000000000000 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ApiDiffer.cs +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using Microsoft.CodeAnalysis; -using Microsoft.DotNet.ApiCompatibility.Abstractions; -using System.Collections.Generic; - -namespace Microsoft.DotNet.ApiCompatibility -{ - public class ApiDiffer - { - private readonly DiffingSettings _settings; - - public ApiDiffer() : this(new DiffingSettings()) { } - - public ApiDiffer(DiffingSettings settings) - { - _settings = settings; - } - - public ApiDiffer(bool includeInternalSymbols) - { - _settings = new DiffingSettings(filter: new AccessibilityFilter(includeInternalSymbols)); - } - - public string NoWarn { get; set; } = string.Empty; - public (string diagnosticId, string memberId)[] IgnoredDifferences { get; set; } - - public IEnumerable GetDifferences(IEnumerable left, IEnumerable right) - { - AssemblySetMapper mapper = new(_settings); - mapper.AddElement(left, 0); - mapper.AddElement(right, 1); - - DiferenceVisitor visitor = new(noWarn: NoWarn, ignoredDifferences: IgnoredDifferences); - visitor.Visit(mapper); - return visitor.Differences; - } - - public IEnumerable GetDifferences(IAssemblySymbol left, IAssemblySymbol right) - { - AssemblyMapper mapper = new(_settings); - mapper.AddElement(left, 0); - mapper.AddElement(right, 1); - - DiferenceVisitor visitor = new(noWarn: NoWarn, ignoredDifferences: IgnoredDifferences); - visitor.Visit(mapper); - return visitor.Differences; - } - } -} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/AssemblySymbolLoader.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/AssemblySymbolLoader.cs index 27e5cc497aad..a5bbd2927082 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/AssemblySymbolLoader.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/AssemblySymbolLoader.cs @@ -12,6 +12,9 @@ namespace Microsoft.DotNet.ApiCompatibility { + /// + /// Class that loads objects from source files, binaries or directories containing binaries. + /// public class AssemblySymbolLoader { private readonly HashSet _referenceSearchPaths = new(); @@ -20,21 +23,30 @@ public class AssemblySymbolLoader private readonly bool _resolveReferences; private CSharpCompilation _cSharpCompilation; - public AssemblySymbolLoader(string assemblyName = "", bool resolveAssemblyReferences = false) + /// + /// Instanciate an object with the desired setting to resolve assembly references or not. + /// + /// Indicates whether it should try to resolve assembly references when loading or not. + public AssemblySymbolLoader(bool resolveAssemblyReferences = false) { - if (string.IsNullOrEmpty(assemblyName)) - { - assemblyName = $"AssemblyLoader_{DateTime.Now:MM_dd_yy_HH_mm_ss_FFF}"; - } - _loadedAssemblies = new Dictionary(); var compilationOptions = new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary, nullableContextOptions: NullableContextOptions.Enable); - _cSharpCompilation = CSharpCompilation.Create(assemblyName, options: compilationOptions); + _cSharpCompilation = CSharpCompilation.Create($"AssemblyLoader_{DateTime.Now:MM_dd_yy_HH_mm_ss_FFF}", options: compilationOptions); _resolveReferences = resolveAssemblyReferences; } + /// + /// Adds a set of paths to the search directories to resolve references from. + /// This is only used when the setting to resolve assembly references is set to true. + /// + /// Comma separated list of paths to register as search directories. public void AddReferenceSearchDirectories(string paths) => AddReferenceSearchDirectories(SplitPaths(paths)); + /// + /// Adds a set of paths to the search directories to resolve references from. + /// This is only used when the setting to resolve assembly references is set to true. + /// + /// The list of paths to register as search directories. public void AddReferenceSearchDirectories(IEnumerable paths) { if (paths == null) @@ -46,12 +58,23 @@ public void AddReferenceSearchDirectories(IEnumerable paths) _referenceSearchPaths.Add(path); } + /// + /// Indicates if the 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. public bool HasRoslynDiagnostics(out IEnumerable diagnostics) { diagnostics = _cSharpCompilation.GetDiagnostics(); return diagnostics.Any(); } + /// + /// Indicates if the loader emitted any warnings that might affect the assembly resolution. + /// + /// List of warnings. + /// True if there are any warnings, false otherwise. public bool HasLoadWarnings(out IEnumerable warnings) { warnings = _warnings; @@ -61,8 +84,18 @@ public bool HasLoadWarnings(out IEnumerable warnings) private static string[] SplitPaths(string paths) => paths == null ? null : paths.Split(new char[] { ',', ';' }, StringSplitOptions.RemoveEmptyEntries); + /// + /// Loads a list of assemblies and gets its corresponding from the specified paths. + /// + /// Comma separated list of paths to load binaries from. Can be full paths to binaries or a directory. + /// The list of resolved . public IEnumerable LoadAssemblies(string paths) => LoadAssemblies(SplitPaths(paths)); + /// + /// Loads a list of assemblies and gets its corresponding from the specified paths. + /// + /// List of paths to load binaries from. Can be full paths to binaries or a directory. + /// The list of resolved . public IEnumerable LoadAssemblies(IEnumerable paths) { if (paths == null) @@ -85,6 +118,11 @@ public IEnumerable LoadAssemblies(IEnumerable paths) return result; } + /// + /// Loads an assembly from the provided path. + /// + /// The full path to the assembly. + /// representing the loaded assembly. public IAssemblySymbol LoadAssembly(string path) { if (path == null) @@ -96,6 +134,13 @@ public IAssemblySymbol LoadAssembly(string path) return (IAssemblySymbol)_cSharpCompilation.GetAssemblyOrModuleSymbol(metadataReference); } + /// + /// Loads an assembly using the provided name from a given . + /// + /// The name to use to resolve the assembly. + /// The stream to read the metadata from. + /// respresenting the given . If an + /// assembly with the same was already loaded, the previously loaded assembly is returned. public IAssemblySymbol LoadAssembly(string name, Stream stream) { if (name == null) @@ -108,6 +153,11 @@ public IAssemblySymbol LoadAssembly(string name, Stream stream) throw new ArgumentNullException(nameof(stream)); } + if (stream.Position >= stream.Length) + { + throw new ArgumentException("Stream position is greater than it's length, so there are no contents available to read", nameof(stream)); + } + if (!_loadedAssemblies.TryGetValue(name, out MetadataReference metadataReference)) { metadataReference = CreateAndAddReferenceToCompilation(name, stream); @@ -116,13 +166,27 @@ public IAssemblySymbol LoadAssembly(string name, Stream stream) return (IAssemblySymbol)_cSharpCompilation.GetAssemblyOrModuleSymbol(metadataReference); } - public IAssemblySymbol LoadAssemblyFromSourceFiles(IEnumerable filePaths, IEnumerable referencePaths) + /// + /// Loads an containing the metadata from the provided source files and given assembly name. + /// + /// The file paths to use as syntax trees to create the . + /// The name of the . + /// Paths to use as references if we want all the references to be included in the metadata. + /// The containing the metadata from the provided source files. + public IAssemblySymbol LoadAssemblyFromSourceFiles(IEnumerable filePaths, string assemblyName, IEnumerable referencePaths) { if (filePaths == null || filePaths.Count() == 0) { throw new ArgumentNullException(nameof(filePaths), $"Should not be null and contain at least one element"); } + if (string.IsNullOrEmpty(assemblyName)) + { + throw new ArgumentNullException(nameof(assemblyName), $"Should provide a valid assembly name"); + } + + _cSharpCompilation = _cSharpCompilation.WithAssemblyName(assemblyName); + List syntaxTrees = new(); foreach (string filePath in filePaths) { @@ -140,7 +204,15 @@ public IAssemblySymbol LoadAssemblyFromSourceFiles(IEnumerable filePaths return _cSharpCompilation.Assembly; } - public IEnumerable LoadMatchingAssemblies(IEnumerable fromAssemblies, IEnumerable searchPaths, bool validateMatchingIdentity = true, bool errorOnMissingAssemblies = true) + /// + /// Loads a list of matching assemblies given the "from" list that we should try and load it's matching assembly from the given paths. + /// + /// List of to search for. + /// List of search paths. + /// Indicates if we should validate that the identity of the resolved assembly is the same. + /// Indicates if a warning should be added to the warning list when a matching assembly is not found. + /// The list of matching assemblies represented as . + public IEnumerable LoadMatchingAssemblies(IEnumerable fromAssemblies, IEnumerable searchPaths, bool validateMatchingIdentity = true, bool warnOnMissingAssemblies = true) { if (fromAssemblies == null) { @@ -185,7 +257,7 @@ public IEnumerable LoadMatchingAssemblies(IEnumerable + /// Class that contains all the settings used to filter metadata, compare symbols and run rules. + /// + public class ComparingSettings + { + /// + /// The factory to get the . + /// + public IRuleRunnerFactory RuleRunnerFactory { get; } + + /// + /// The metadata filter to use when creating the . + /// + public ISymbolFilter Filter { get; } + + /// + /// The comparer to map metadata. + /// + public IEqualityComparer EqualityComparer { get; } + + /// + /// Instanciates an object with the desired settings. + /// + /// The factory to create a + /// The symbol filter. + /// The comparer to map metadata. + public ComparingSettings(IRuleRunnerFactory ruleRunnerFactory = null, ISymbolFilter filter = null, IEqualityComparer equalityComparer = null) + { + RuleRunnerFactory = ruleRunnerFactory ?? new RuleDriverFactory(); + Filter = filter ?? new AccessibilityFilter(includeInternalSymbols: false); + EqualityComparer = equalityComparer ?? new DefaultSymbolsEqualityComparer(); + } + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DefaultSymbolsEqualityComparer.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DefaultSymbolsEqualityComparer.cs index 1901edb08e9f..3d6af65d7507 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DefaultSymbolsEqualityComparer.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DefaultSymbolsEqualityComparer.cs @@ -7,7 +7,7 @@ namespace Microsoft.DotNet.ApiCompatibility { - public class DefaultSymbolsEqualityComparer : IEqualityComparer + internal class DefaultSymbolsEqualityComparer : IEqualityComparer { public bool Equals(ISymbol x, ISymbol y) => string.Equals(GetKey(x), GetKey(y), StringComparison.OrdinalIgnoreCase); diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticBag.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticBag.cs index 588806603073..b473e7e74119 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticBag.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticBag.cs @@ -6,6 +6,11 @@ namespace Microsoft.DotNet.ApiCompatibility { + /// + /// This is a bag that contains a list of and filters them out based on the + /// noWarn and ignoredDifferences settings when they are added to the bag. + /// + /// Type to represent the diagnostics. public class DiagnosticBag where T : IDiagnostic { private readonly Dictionary> _ignore; @@ -13,29 +18,42 @@ public class DiagnosticBag where T : IDiagnostic private readonly List _differences = new(); + /// + /// Instanciates an diagnostic bag with the provided settings to ignore diagnostics. + /// + /// Comma separated list of diagnostic IDs to ignore. + /// An array of differences to ignore based on diagnostic ID and reference ID. public DiagnosticBag(string noWarn, (string diagnosticId, string referenceId)[] ignoredDifferences) { _noWarn = new HashSet(noWarn?.Split(';')); _ignore = new Dictionary>(); - foreach (var ignored in ignoredDifferences) + foreach ((string diagnosticId, string referenceId) in ignoredDifferences) { - if (!_ignore.TryGetValue(ignored.diagnosticId, out HashSet members)) + if (!_ignore.TryGetValue(diagnosticId, out HashSet members)) { members = new HashSet(); - _ignore.Add(ignored.diagnosticId, members); + _ignore.Add(diagnosticId, members); } - members.Add(ignored.referenceId); + members.Add(referenceId); } } + /// + /// Adds the differences to the diagnostic bag if they are not found in the exclusion settings. + /// + /// The differences to add. public void AddRange(IEnumerable differences) { foreach (T difference in differences) Add(difference); } + /// + /// Adds a difference to the diagnostic bag if they are not found in the exclusion settings. + /// + /// The difference to add. public void Add(T difference) { if (_noWarn.Contains(difference.DiagnosticId)) @@ -52,6 +70,9 @@ public void Add(T difference) _differences.Add(difference); } + /// + /// A list of differences contained in the diagnostic bag. + /// public IEnumerable Differences => _differences; } } diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticIds.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticIds.cs index 47c1f7218f71..9114576645f4 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticIds.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticIds.cs @@ -3,6 +3,9 @@ namespace Microsoft.DotNet.ApiCompatibility { + /// + /// Class containing the strings representing the Diagnostic IDs that can be returned in the compatibility differences. + /// public static class DiagnosticIds { public const string TypeMustExist = "CP0001"; diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiferenceVisitor.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiferenceVisitor.cs index a8c2edd03e98..1e79fc1f5798 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiferenceVisitor.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiferenceVisitor.cs @@ -7,21 +7,37 @@ namespace Microsoft.DotNet.ApiCompatibility { + /// + /// The visitor that traverses the mappers' tree and gets it's differences in a . + /// public class DiferenceVisitor : MapperVisitor { private readonly DiagnosticBag _differenceBag; - public DiferenceVisitor(string noWarn = null, (string diagnosticId, string memberId)[] ignoredDifferences = null) + /// + /// Instantiates the visitor with the desired settings. + /// + /// A comma separated list of diagnostic IDs to ignore. + /// A list of tuples to ignore diagnostic IDs by symbol. + public DiferenceVisitor(string noWarn = null, (string diagnosticId, string symbolId)[] ignoredDifferences = null) { _differenceBag = new DiagnosticBag(noWarn ?? string.Empty, ignoredDifferences ?? Array.Empty<(string, string)>()); } + /// + /// Visits an and adds it's differences to the . + /// + /// The mapper to visit. public override void Visit(AssemblyMapper assembly) { _differenceBag.AddRange(assembly.GetDifferences()); base.Visit(assembly); } + /// + /// Visits an and adds it's differences to the . + /// + /// The mapper to visit. public override void Visit(TypeMapper type) { _differenceBag.AddRange(type.GetDifferences()); @@ -32,11 +48,18 @@ public override void Visit(TypeMapper type) } } + /// + /// Visits an and adds it's differences to the . + /// + /// The mapper to visit. public override void Visit(MemberMapper member) { _differenceBag.AddRange(member.GetDifferences()); } + /// + /// The differences that the has at this point. + /// public IEnumerable Differences => _differenceBag.Differences; } } diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiffingSettings.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiffingSettings.cs deleted file mode 100644 index 5fde21d3fb3c..000000000000 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiffingSettings.cs +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using Microsoft.CodeAnalysis; -using Microsoft.DotNet.ApiCompatibility.Abstractions; -using Microsoft.DotNet.ApiCompatibility.Rules; -using System.Collections.Generic; - -namespace Microsoft.DotNet.ApiCompatibility -{ - public class DiffingSettings - { - public IRuleDriverFactory RuleDriverFactory { get; } - public IDiffingFilter Filter { get; } - public IEqualityComparer EqualityComparer { get; } - - public DiffingSettings(IRuleDriverFactory ruleDriverFactory = null, IDiffingFilter filter = null, IEqualityComparer equalityComparer = null) - { - RuleDriverFactory = ruleDriverFactory ?? new RuleDriverFactory(); - Filter = filter ?? new AccessibilityFilter(includeInternalSymbols: false); - EqualityComparer = equalityComparer ?? new DefaultSymbolsEqualityComparer(); - } - } -} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/MembersMustExist.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/MembersMustExist.cs index 6c9e073695ba..57059ab2b87f 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/MembersMustExist.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/MembersMustExist.cs @@ -8,8 +8,17 @@ namespace Microsoft.DotNet.ApiCompatibility.Rules { + /// + /// Rule that evaluates whether a member exists on Left and Right. + /// If the member doesn't exist on Right but it does on Left, it adds a to the list of differences. + /// public class MembersMustExist : Rule { + /// + /// Evaluates whether a type exists on both sides of the . + /// + /// The to evaluate. + /// The list of to add differences to. public override void Run(TypeMapper mapper, List differences) { ITypeSymbol left = mapper.Left; @@ -17,6 +26,11 @@ public override void Run(TypeMapper mapper, List differences) differences.Add(new CompatDifference(DiagnosticIds.TypeMustExist, $"Type '{left.ToDisplayString()}' exists on the contract but not on the implementation", DifferenceType.Removed, left)); } + /// + /// Evaluates whether member (Field, Property, Method, Constructor, Event) exists on both sides of the . + /// + /// The to evaluate. + /// The list of to add differences to. public override void Run(MemberMapper mapper, List differences) { ISymbol left = mapper.Left; diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleDriverFactory.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleDriverFactory.cs new file mode 100644 index 000000000000..bda6b2040f26 --- /dev/null +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleDriverFactory.cs @@ -0,0 +1,19 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information.s + +using Microsoft.DotNet.ApiCompatibility.Abstractions; + +namespace Microsoft.DotNet.ApiCompatibility.Rules +{ + public class RuleDriverFactory : IRuleRunnerFactory + { + private RuleRunner _driver; + public IRuleRunner GetRuleRunner() + { + if (_driver == null) + _driver = new RuleRunner(); + + return _driver; + } + } +} diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleDriver.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleRunner.cs similarity index 75% rename from src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleDriver.cs rename to src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleRunner.cs index 12e98eb4f902..9ec1581883ad 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleDriver.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleRunner.cs @@ -8,33 +8,11 @@ namespace Microsoft.DotNet.ApiCompatibility.Rules { - public interface IRuleDriverFactory - { - IRuleDriver GetRuleDriver(); - } - - public class RuleDriverFactory : IRuleDriverFactory - { - private RuleDriver _driver; - public IRuleDriver GetRuleDriver() - { - if (_driver == null) - _driver = new RuleDriver(); - - return _driver; - } - } - - public interface IRuleDriver - { - IEnumerable Run(ElementMapper mapper); - } - - internal class RuleDriver : IRuleDriver + internal class RuleRunner : IRuleRunner { private readonly IEnumerable _rules; - internal RuleDriver() + internal RuleRunner() { _rules = GetRules(); } @@ -84,7 +62,7 @@ private void Run(MemberMapper mapper, List differences) private IEnumerable GetRules() { - return this.GetType().Assembly.GetTypes() + return GetType().Assembly.GetTypes() .Where(t => !t.IsAbstract && typeof(Rule).IsAssignableFrom(t)) .Select(t => (Rule)Activator.CreateInstance(t)); } diff --git a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/AssemblySymbolLoaderTests.cs b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/AssemblySymbolLoaderTests.cs index d96661a532e1..82d497a7d1c7 100644 --- a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/AssemblySymbolLoaderTests.cs +++ b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/AssemblySymbolLoaderTests.cs @@ -117,9 +117,10 @@ public void LoadAssemblyFromSourceFiles_Throws() { AssemblySymbolLoader loader = new(); IEnumerable paths = new[] { Guid.NewGuid().ToString("N") }; - Assert.Throws(() => loader.LoadAssemblyFromSourceFiles(paths, Array.Empty())); - Assert.Throws("filePaths", () => loader.LoadAssemblyFromSourceFiles(null, Array.Empty())); - Assert.Throws("filePaths", () => loader.LoadAssemblyFromSourceFiles(Array.Empty(), Array.Empty())); + Assert.Throws(() => loader.LoadAssemblyFromSourceFiles(paths, "assembly1", Array.Empty())); + Assert.Throws("filePaths", () => loader.LoadAssemblyFromSourceFiles(null, "assembly1", Array.Empty())); + Assert.Throws("filePaths", () => loader.LoadAssemblyFromSourceFiles(Array.Empty(), "assembly1", Array.Empty())); + Assert.Throws("assemblyName", () => loader.LoadAssemblyFromSourceFiles(paths, null, Array.Empty())); } [Fact] @@ -333,6 +334,7 @@ public void LoadAssemblyFromStreamNoWarns() 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.Equal(testProject.Name, symbol.Name, StringComparer.Ordinal); IEnumerable types = symbol.GlobalNamespace diff --git a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/MembersMustExistTests.cs b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/MembersMustExistTests.cs index 1ad1d91b35e2..aec94e6161f8 100644 --- a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/MembersMustExistTests.cs +++ b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/MembersMustExistTests.cs @@ -43,7 +43,7 @@ public string Parameterless() { } IAssemblySymbol left = SymbolFactory.GetAssemblyFromSyntax(leftSyntax); IAssemblySymbol right = SymbolFactory.GetAssemblyFromSyntax(rightSyntax); - ApiDiffer differ = new(); + ApiComparer differ = new(); IEnumerable differences = differ.GetDifferences(new[] { left }, new[] { right }); CompatDifference[] expected = new[] @@ -98,7 +98,7 @@ public class Second : FirstBase { } IAssemblySymbol left = SymbolFactory.GetAssemblyFromSyntax(leftSyntax); IAssemblySymbol right = SymbolFactory.GetAssemblyFromSyntax(rightSyntax); - ApiDiffer differ = new(); + ApiComparer differ = new(); IEnumerable differences = differ.GetDifferences(new[] { left }, new[] { right }); Assert.Empty(differences); } @@ -129,7 +129,7 @@ public class First IAssemblySymbol left = SymbolFactory.GetAssemblyFromSyntax(leftSyntax); IAssemblySymbol right = SymbolFactory.GetAssemblyFromSyntax(rightSyntax); - ApiDiffer differ = new(); + ApiComparer differ = new(); differ.NoWarn = DiagnosticIds.MemberMustExist; IEnumerable differences = differ.GetDifferences(new[] { left }, new[] { right }); Assert.Empty(differences); @@ -166,7 +166,7 @@ public string MultipleOverrides(string a, int b, int c) { } IAssemblySymbol left = SymbolFactory.GetAssemblyFromSyntax(leftSyntax); IAssemblySymbol right = SymbolFactory.GetAssemblyFromSyntax(rightSyntax); - ApiDiffer differ = new(); + ApiComparer differ = new(); IEnumerable differences = differ.GetDifferences(new[] { left }, new[] { right }); CompatDifference[] expected = new[] @@ -214,7 +214,7 @@ public string MultipleOverrides(string a, int b, string c) { } IAssemblySymbol left = SymbolFactory.GetAssemblyFromSyntax(leftSyntax); IAssemblySymbol right = SymbolFactory.GetAssemblyFromSyntax(rightSyntax, assemblyName: "DifferentName"); - ApiDiffer differ = new(includeInternalSymbols: includeInternals); + ApiComparer differ = new(includeInternalSymbols: includeInternals); IEnumerable differences = differ.GetDifferences(left, right); if (includeInternals) diff --git a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/TypeMustExistTests.cs b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/TypeMustExistTests.cs index 71b8a365dee0..781d4c648dba 100644 --- a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/TypeMustExistTests.cs +++ b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/TypeMustExistTests.cs @@ -35,7 +35,7 @@ public class First { } } "; - ApiDiffer differ = new(); + ApiComparer differ = new(); differ.NoWarn = noWarn; bool enableNullable = false; IAssemblySymbol left = SymbolFactory.GetAssemblyFromSyntax(leftSyntax, enableNullable); @@ -79,7 +79,7 @@ public class First { } "; IAssemblySymbol left = SymbolFactory.GetAssemblyFromSyntaxWithReferences(leftSyntax, new[] { forwardedTypeSyntax }, includeDefaultReferences: true); IAssemblySymbol right = SymbolFactory.GetAssemblyFromSyntax(rightSyntax); - ApiDiffer differ = new(); + ApiComparer differ = new(); IEnumerable differences = differ.GetDifferences(new[] { left }, new[] { right }); CompatDifference[] expected = new[] @@ -109,7 +109,7 @@ public class First { } IEnumerable references = new[] { forwardedTypeSyntax }; IAssemblySymbol left = SymbolFactory.GetAssemblyFromSyntaxWithReferences(syntax, references, includeDefaultReferences: true); IAssemblySymbol right = SymbolFactory.GetAssemblyFromSyntaxWithReferences(syntax, references, includeDefaultReferences: true); - ApiDiffer differ = new(); + ApiComparer differ = new(); Assert.Empty(differ.GetDifferences(new[] { left }, new[] { right })); } @@ -132,7 +132,7 @@ public class First { } } "; - ApiDiffer differ = new(); + ApiComparer differ = new(); differ.NoWarn = DiagnosticIds.TypeMustExist; IAssemblySymbol left = SymbolFactory.GetAssemblyFromSyntax(leftSyntax); IAssemblySymbol right = SymbolFactory.GetAssemblyFromSyntax(rightSyntax); @@ -167,7 +167,7 @@ public record First(string a, string b); (DiagnosticIds.TypeMustExist, "T:CompatTests.MyEnum"), }; - ApiDiffer differ = new(); + ApiComparer differ = new(); differ.IgnoredDifferences = ignoredDifferences; IAssemblySymbol left = SymbolFactory.GetAssemblyFromSyntax(leftSyntax); @@ -204,7 +204,7 @@ public class First { } } "; - ApiDiffer differ = new(includeInternalSymbols); + ApiComparer differ = new(includeInternalSymbols); IAssemblySymbol left = SymbolFactory.GetAssemblyFromSyntax(leftSyntax); IAssemblySymbol right = SymbolFactory.GetAssemblyFromSyntax(rightSyntax); IEnumerable differences = differ.GetDifferences(new[] { left }, new[] { right }); @@ -257,7 +257,7 @@ internal class InternalNested { } } "; - ApiDiffer differ = new(includeInternalSymbols); + ApiComparer differ = new(includeInternalSymbols); IAssemblySymbol left = SymbolFactory.GetAssemblyFromSyntax(leftSyntax); IAssemblySymbol right = SymbolFactory.GetAssemblyFromSyntax(rightSyntax); IEnumerable differences = differ.GetDifferences(new[] { left }, new[] { right }); From a70fd12bb850744ccf1e55a43cb3514d1a0cb8e2 Mon Sep 17 00:00:00 2001 From: Santiago Fernandez Madero Date: Tue, 13 Apr 2021 18:17:00 -0700 Subject: [PATCH 5/6] More PR Feedback --- .../Abstractions/Rule.cs | 6 +++--- .../AssemblySymbolLoader.cs | 18 +++++------------- .../ComparingSettings.cs | 2 +- .../Rules/MembersMustExist.cs | 4 ++-- .../Rules/RuleRunner.cs | 15 ++++++++++----- ...leDriverFactory.cs => RuleRunnerFactory.cs} | 2 +- 6 files changed, 22 insertions(+), 25 deletions(-) rename src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/{RuleDriverFactory.cs => RuleRunnerFactory.cs} (90%) diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Rule.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Rule.cs index b25e53add4ee..579ce07b12a4 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Rule.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Rule.cs @@ -15,7 +15,7 @@ public abstract class Rule /// /// The to evaluate. /// The list of to add any differences to. - public virtual void Run(AssemblyMapper mapper, List differences) + public virtual void Run(AssemblyMapper mapper, IList differences) { } @@ -24,7 +24,7 @@ public virtual void Run(AssemblyMapper mapper, List difference /// /// The to evaluate. /// The list of to add any differences to. - public virtual void Run(TypeMapper mapper, List differences) + public virtual void Run(TypeMapper mapper, IList differences) { } @@ -33,7 +33,7 @@ public virtual void Run(TypeMapper mapper, List differences) /// /// The to evaluate. /// The list of to add any differences to. - public virtual void Run(MemberMapper mapper, List differences) + public virtual void Run(MemberMapper mapper, IList differences) { } } diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/AssemblySymbolLoader.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/AssemblySymbolLoader.cs index a5bbd2927082..ce75a1b92772 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/AssemblySymbolLoader.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/AssemblySymbolLoader.cs @@ -105,17 +105,14 @@ public IEnumerable LoadAssemblies(IEnumerable paths) IEnumerable assembliesToReturn = LoadFromPaths(paths); - List result = new List(); foreach (MetadataReference metadataReference in assembliesToReturn) { ISymbol symbol = _cSharpCompilation.GetAssemblyOrModuleSymbol(metadataReference); if (symbol is IAssemblySymbol assemblySymbol) { - result.Add(assemblySymbol); + yield return symbol; } } - - return result; } /// @@ -224,7 +221,6 @@ public IEnumerable LoadMatchingAssemblies(IEnumerable matchingAssemblies = new List(); foreach (IAssemblySymbol assembly in fromAssemblies) { bool found = false; @@ -250,7 +246,7 @@ public IEnumerable LoadMatchingAssemblies(IEnumerable LoadMatchingAssemblies(IEnumerable LoadFromPaths(IEnumerable paths) { - List result = new List(); foreach (string path in paths) { string resolvedPath = Environment.ExpandEnvironmentVariables(path); @@ -277,12 +270,13 @@ private IEnumerable LoadFromPaths(IEnumerable paths) if (Directory.Exists(resolvedPath)) { directory = resolvedPath; - result.AddRange(LoadAssembliesFromDirectory(resolvedPath)); + foreach (MetadataReference reference in LoadAssembliesFromDirectory(resolvedPath)) + yield return reference; } else if (File.Exists(resolvedPath)) { directory = Path.GetDirectoryName(resolvedPath); - result.Add(CreateOrGetMetadataReferenceFromPath(resolvedPath)); + yield return CreateOrGetMetadataReferenceFromPath(resolvedPath); } else { @@ -292,8 +286,6 @@ private IEnumerable LoadFromPaths(IEnumerable paths) if (_resolveReferences && !string.IsNullOrEmpty(directory)) _referenceSearchPaths.Add(directory); } - - return result; } private IEnumerable LoadAssembliesFromDirectory(string directory) diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ComparingSettings.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ComparingSettings.cs index 232cfb5813f3..9b0168142527 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ComparingSettings.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ComparingSettings.cs @@ -36,7 +36,7 @@ public class ComparingSettings /// The comparer to map metadata. public ComparingSettings(IRuleRunnerFactory ruleRunnerFactory = null, ISymbolFilter filter = null, IEqualityComparer equalityComparer = null) { - RuleRunnerFactory = ruleRunnerFactory ?? new RuleDriverFactory(); + RuleRunnerFactory = ruleRunnerFactory ?? new RuleRunnerFactory(); Filter = filter ?? new AccessibilityFilter(includeInternalSymbols: false); EqualityComparer = equalityComparer ?? new DefaultSymbolsEqualityComparer(); } diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/MembersMustExist.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/MembersMustExist.cs index 57059ab2b87f..39b7fe821adc 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/MembersMustExist.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/MembersMustExist.cs @@ -19,7 +19,7 @@ public class MembersMustExist : Rule /// /// The to evaluate. /// The list of to add differences to. - public override void Run(TypeMapper mapper, List differences) + public override void Run(TypeMapper mapper, IList differences) { ITypeSymbol left = mapper.Left; if (left != null && mapper.Right == null) @@ -31,7 +31,7 @@ public override void Run(TypeMapper mapper, List differences) /// /// The to evaluate. /// The list of to add differences to. - public override void Run(MemberMapper mapper, List differences) + public override void Run(MemberMapper mapper, IList differences) { ISymbol left = mapper.Left; if (left != null && mapper.Right == null) diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleRunner.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleRunner.cs index 9ec1581883ad..c354949516c2 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleRunner.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleRunner.cs @@ -10,7 +10,7 @@ namespace Microsoft.DotNet.ApiCompatibility.Rules { internal class RuleRunner : IRuleRunner { - private readonly IEnumerable _rules; + private readonly Rule[] _rules; internal RuleRunner() { @@ -60,11 +60,16 @@ private void Run(MemberMapper mapper, List differences) } } - private IEnumerable GetRules() + private Rule[] GetRules() { - return GetType().Assembly.GetTypes() - .Where(t => !t.IsAbstract && typeof(Rule).IsAssignableFrom(t)) - .Select(t => (Rule)Activator.CreateInstance(t)); + List rules = new(); + foreach (Type type in GetType().Assembly.GetTypes()) + { + if (!type.IsAbstract && typeof(Rule).IsAssignableFrom(type)) + rules.Add((Rule)Activator.CreateInstance(type)); + } + + return rules.ToArray(); } } } diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleDriverFactory.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleRunnerFactory.cs similarity index 90% rename from src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleDriverFactory.cs rename to src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleRunnerFactory.cs index bda6b2040f26..1c3b87008252 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleDriverFactory.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/RuleRunnerFactory.cs @@ -5,7 +5,7 @@ namespace Microsoft.DotNet.ApiCompatibility.Rules { - public class RuleDriverFactory : IRuleRunnerFactory + public class RuleRunnerFactory : IRuleRunnerFactory { private RuleRunner _driver; public IRuleRunner GetRuleRunner() From c88e924e5e3560e24493dc7566cdee903f204e28 Mon Sep 17 00:00:00 2001 From: Santiago Fernandez Madero Date: Wed, 14 Apr 2021 11:28:16 -0700 Subject: [PATCH 6/6] PR Feedback --- .../Abstractions/CompatDifference.cs | 16 +++++++------- ...r.cs => SymbolAccessibilityBasedFilter.cs} | 4 ++-- .../Abstractions/MapperVisitor.cs | 22 +++++++++++++++++++ .../Abstractions/Mappers/AssemblySetMapper.cs | 7 +++--- .../ApiComparer.cs | 8 +++---- .../AssemblySymbolLoader.cs | 18 ++++++++++----- .../ComparingSettings.cs | 4 ++-- .../DiagnosticBag.cs | 2 +- .../DiferenceVisitor.cs | 15 +++++++++++++ .../Rules/MembersMustExist.cs | 4 ++-- .../AssemblySymbolLoaderTests.cs | 2 +- .../Rules/MembersMustExistTests.cs | 20 ++++++++--------- .../Rules/TypeMustExistTests.cs | 22 +++++++++---------- 13 files changed, 95 insertions(+), 49 deletions(-) rename src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/{AccessibilityFilter.cs => SymbolAccessibilityBasedFilter.cs} (84%) diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/CompatDifference.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/CompatDifference.cs index f1148965e318..c693dc74339f 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/CompatDifference.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/CompatDifference.cs @@ -34,30 +34,30 @@ public class CompatDifference : IDiagnostic, IEquatable private CompatDifference() { } /// - /// Instanciates a new object representing the compatibility difference. + /// Instantiate a new object representing the compatibility difference. /// /// representing the diagnostic ID. /// message describing the difference. /// to describe the type of the difference. /// for which the difference is associated to. - public CompatDifference(string id, string message, DifferenceType type, ISymbol member) - : this(id, message, type, member.GetDocumentationCommentId()) + public CompatDifference(string diagnosticId, string message, DifferenceType type, ISymbol member) + : this(diagnosticId, message, type, member?.GetDocumentationCommentId()) { } /// - /// Instanciates a new object representing the compatibility difference. + /// Instantiate a new object representing the compatibility difference. /// /// representing the diagnostic ID. /// message describing the difference. /// to describe the type of the difference. /// containing the member ID for which the difference is associated to. - public CompatDifference(string id, string message, DifferenceType type, string memberId) + public CompatDifference(string diagnosticId, string message, DifferenceType type, string memberId) { - DiagnosticId = id; - Message = message; + DiagnosticId = diagnosticId ?? throw new ArgumentNullException(nameof(diagnosticId)); + Message = message ?? throw new ArgumentNullException(nameof(message)); Type = type; - ReferenceId = memberId; + ReferenceId = memberId ?? throw new ArgumentNullException(nameof(memberId)); } /// diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/AccessibilityFilter.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/SymbolAccessibilityBasedFilter.cs similarity index 84% rename from src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/AccessibilityFilter.cs rename to src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/SymbolAccessibilityBasedFilter.cs index 201bb7816e03..983e49593038 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/AccessibilityFilter.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Filtering/SymbolAccessibilityBasedFilter.cs @@ -5,11 +5,11 @@ namespace Microsoft.DotNet.ApiCompatibility.Abstractions { - internal class AccessibilityFilter : ISymbolFilter + internal class SymbolAccessibilityBasedFilter : ISymbolFilter { private readonly bool _includeInternalSymbols; - internal AccessibilityFilter(bool includeInternalSymbols) + internal SymbolAccessibilityBasedFilter(bool includeInternalSymbols) { _includeInternalSymbols = includeInternalSymbols; } diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/MapperVisitor.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/MapperVisitor.cs index f046666faa1e..f6305f08aa6e 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/MapperVisitor.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/MapperVisitor.cs @@ -1,6 +1,8 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System; + namespace Microsoft.DotNet.ApiCompatibility.Abstractions { /// @@ -43,6 +45,11 @@ public void Visit(ElementMapper mapper) /// The to visit. public virtual void Visit(AssemblySetMapper mapper) { + if (mapper == null) + { + throw new ArgumentNullException(nameof(mapper)); + } + foreach (AssemblyMapper assembly in mapper.GetAssemblies()) { Visit(assembly); @@ -55,6 +62,11 @@ public virtual void Visit(AssemblySetMapper mapper) /// The to visit. public virtual void Visit(AssemblyMapper mapper) { + if (mapper == null) + { + throw new ArgumentNullException(nameof(mapper)); + } + foreach (NamespaceMapper nsMapper in mapper.GetNamespaces()) { Visit(nsMapper); @@ -67,6 +79,11 @@ public virtual void Visit(AssemblyMapper mapper) /// The to visit. public virtual void Visit(NamespaceMapper mapper) { + if (mapper == null) + { + throw new ArgumentNullException(nameof(mapper)); + } + foreach (TypeMapper type in mapper.GetTypes()) { Visit(type); @@ -79,6 +96,11 @@ public virtual void Visit(NamespaceMapper mapper) /// The to visit. public virtual void Visit(TypeMapper mapper) { + if (mapper == null) + { + throw new ArgumentNullException(nameof(mapper)); + } + foreach (TypeMapper type in mapper.GetNestedTypes()) { Visit(type); diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblySetMapper.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblySetMapper.cs index be728395ba25..e34a877506ab 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblySetMapper.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Abstractions/Mappers/AssemblySetMapper.cs @@ -1,8 +1,9 @@ -using Microsoft.CodeAnalysis; -using System.Collections.Generic; -// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using Microsoft.CodeAnalysis; +using System.Collections.Generic; + namespace Microsoft.DotNet.ApiCompatibility.Abstractions { /// diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ApiComparer.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ApiComparer.cs index 5af6b4f74274..88f20828d31a 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ApiComparer.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ApiComparer.cs @@ -15,12 +15,12 @@ public class ApiComparer private readonly ComparingSettings _settings; /// - /// Instanciates an object with the default . + /// Instantiate an object with the default . /// public ApiComparer() : this(new ComparingSettings()) { } /// - /// Instanciates an object with the provided + /// Instantiate an object with the provided /// /// Settings to use for comparison. public ApiComparer(ComparingSettings settings) @@ -29,12 +29,12 @@ public ApiComparer(ComparingSettings settings) } /// - /// Instanciates an object that includes comparing internal symbols. + /// Instantiate an object that includes comparing internal symbols. /// /// Indicates whether internal symbols should be included or not. public ApiComparer(bool includeInternalSymbols) { - _settings = new ComparingSettings(filter: new AccessibilityFilter(includeInternalSymbols)); + _settings = new ComparingSettings(filter: new SymbolAccessibilityBasedFilter(includeInternalSymbols)); } /// diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/AssemblySymbolLoader.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/AssemblySymbolLoader.cs index ce75a1b92772..1b78c1aaab7f 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/AssemblySymbolLoader.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/AssemblySymbolLoader.cs @@ -105,14 +105,17 @@ public IEnumerable LoadAssemblies(IEnumerable paths) IEnumerable assembliesToReturn = LoadFromPaths(paths); + List result = new(); foreach (MetadataReference metadataReference in assembliesToReturn) { ISymbol symbol = _cSharpCompilation.GetAssemblyOrModuleSymbol(metadataReference); if (symbol is IAssemblySymbol assemblySymbol) { - yield return symbol; + result.Add(assemblySymbol); } } + + return result; } /// @@ -221,6 +224,7 @@ public IEnumerable LoadMatchingAssemblies(IEnumerable matchingAssemblies = new(); foreach (IAssemblySymbol assembly in fromAssemblies) { bool found = false; @@ -246,7 +250,7 @@ public IEnumerable LoadMatchingAssemblies(IEnumerable LoadMatchingAssemblies(IEnumerable LoadFromPaths(IEnumerable paths) { + List result = new(); foreach (string path in paths) { string resolvedPath = Environment.ExpandEnvironmentVariables(path); @@ -270,13 +277,12 @@ private IEnumerable LoadFromPaths(IEnumerable paths) if (Directory.Exists(resolvedPath)) { directory = resolvedPath; - foreach (MetadataReference reference in LoadAssembliesFromDirectory(resolvedPath)) - yield return reference; + result.AddRange(LoadAssembliesFromDirectory(resolvedPath)); } else if (File.Exists(resolvedPath)) { directory = Path.GetDirectoryName(resolvedPath); - yield return CreateOrGetMetadataReferenceFromPath(resolvedPath); + result.Add(CreateOrGetMetadataReferenceFromPath(resolvedPath)); } else { @@ -286,6 +292,8 @@ private IEnumerable LoadFromPaths(IEnumerable paths) if (_resolveReferences && !string.IsNullOrEmpty(directory)) _referenceSearchPaths.Add(directory); } + + return result; } private IEnumerable LoadAssembliesFromDirectory(string directory) diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ComparingSettings.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ComparingSettings.cs index 9b0168142527..d218ec6e4f43 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ComparingSettings.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/ComparingSettings.cs @@ -29,7 +29,7 @@ public class ComparingSettings public IEqualityComparer EqualityComparer { get; } /// - /// Instanciates an object with the desired settings. + /// Instantiate an object with the desired settings. /// /// The factory to create a /// The symbol filter. @@ -37,7 +37,7 @@ public class ComparingSettings public ComparingSettings(IRuleRunnerFactory ruleRunnerFactory = null, ISymbolFilter filter = null, IEqualityComparer equalityComparer = null) { RuleRunnerFactory = ruleRunnerFactory ?? new RuleRunnerFactory(); - Filter = filter ?? new AccessibilityFilter(includeInternalSymbols: false); + Filter = filter ?? new SymbolAccessibilityBasedFilter(includeInternalSymbols: false); EqualityComparer = equalityComparer ?? new DefaultSymbolsEqualityComparer(); } } diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticBag.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticBag.cs index b473e7e74119..9450fafcff26 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticBag.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiagnosticBag.cs @@ -19,7 +19,7 @@ public class DiagnosticBag where T : IDiagnostic private readonly List _differences = new(); /// - /// Instanciates an diagnostic bag with the provided settings to ignore diagnostics. + /// Instantiate an diagnostic bag with the provided settings to ignore diagnostics. /// /// Comma separated list of diagnostic IDs to ignore. /// An array of differences to ignore based on diagnostic ID and reference ID. diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiferenceVisitor.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiferenceVisitor.cs index 1e79fc1f5798..c725ecd24df1 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiferenceVisitor.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/DiferenceVisitor.cs @@ -30,6 +30,11 @@ public DiferenceVisitor(string noWarn = null, (string diagnosticId, string symbo /// The mapper to visit. public override void Visit(AssemblyMapper assembly) { + if (assembly == null) + { + throw new ArgumentNullException(nameof(assembly)); + } + _differenceBag.AddRange(assembly.GetDifferences()); base.Visit(assembly); } @@ -40,6 +45,11 @@ public override void Visit(AssemblyMapper assembly) /// The mapper to visit. public override void Visit(TypeMapper type) { + if (type == null) + { + throw new ArgumentNullException(nameof(type)); + } + _differenceBag.AddRange(type.GetDifferences()); if (type.ShouldDiffMembers) @@ -54,6 +64,11 @@ public override void Visit(TypeMapper type) /// The mapper to visit. public override void Visit(MemberMapper member) { + if (member == null) + { + throw new ArgumentNullException(nameof(member)); + } + _differenceBag.AddRange(member.GetDifferences()); } diff --git a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/MembersMustExist.cs b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/MembersMustExist.cs index 39b7fe821adc..ad310a7b9fe4 100644 --- a/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/MembersMustExist.cs +++ b/src/Compatibility/Microsoft.DotNet.ApiCompatibility/Rules/MembersMustExist.cs @@ -23,7 +23,7 @@ public override void Run(TypeMapper mapper, IList differences) { ITypeSymbol left = mapper.Left; if (left != null && mapper.Right == null) - differences.Add(new CompatDifference(DiagnosticIds.TypeMustExist, $"Type '{left.ToDisplayString()}' exists on the contract but not on the implementation", DifferenceType.Removed, left)); + differences.Add(new CompatDifference(DiagnosticIds.TypeMustExist, $"Type '{left.ToDisplayString()}' exists on the left but not on the right", DifferenceType.Removed, left)); } /// @@ -51,7 +51,7 @@ public override void Run(MemberMapper mapper, IList difference return; } - differences.Add(new CompatDifference(DiagnosticIds.MemberMustExist, $"Member '{left.ToDisplayString()}' exists on the contract but not on the implementation", DifferenceType.Removed, left)); + differences.Add(new CompatDifference(DiagnosticIds.MemberMustExist, $"Member '{left.ToDisplayString()}' exists on the left but not on the right", DifferenceType.Removed, left)); } } diff --git a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/AssemblySymbolLoaderTests.cs b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/AssemblySymbolLoaderTests.cs index 82d497a7d1c7..7e062ee58b7b 100644 --- a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/AssemblySymbolLoaderTests.cs +++ b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/AssemblySymbolLoaderTests.cs @@ -31,7 +31,7 @@ public class MyClass } "; - // Since we uset typeof(string).Assembly.Location to resolve references + // Since we use typeof(string).Assembly.Location to resolve references // We need to target a framework compatible with what the test is being // built for so that we resolve the references correctly. #if NETCOREAPP diff --git a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/MembersMustExistTests.cs b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/MembersMustExistTests.cs index aec94e6161f8..245daef6f1e9 100644 --- a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/MembersMustExistTests.cs +++ b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/MembersMustExistTests.cs @@ -48,12 +48,12 @@ public string Parameterless() { } CompatDifference[] expected = new[] { - new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.ShouldReportMethod(string, string)' exists on the contract but not on the implementation", DifferenceType.Removed, "M:CompatTests.First.ShouldReportMethod(System.String,System.String)"), - new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.ShouldReportMissingProperty.get' exists on the contract but not on the implementation", DifferenceType.Removed, "M:CompatTests.First.get_ShouldReportMissingProperty"), - new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.this[int].get' exists on the contract but not on the implementation", DifferenceType.Removed, "M:CompatTests.First.get_Item(System.Int32)"), - new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.ShouldReportMissingEvent.add' exists on the contract but not on the implementation", DifferenceType.Removed, "M:CompatTests.First.add_ShouldReportMissingEvent(CompatTests.EventHandler)"), - new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.ShouldReportMissingEvent.remove' exists on the contract but not on the implementation", DifferenceType.Removed, "M:CompatTests.First.remove_ShouldReportMissingEvent(CompatTests.EventHandler)"), - new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.ReportMissingField' exists on the contract but not on the implementation", DifferenceType.Removed, "F:CompatTests.First.ReportMissingField"), + new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.ShouldReportMethod(string, string)' exists on the left but not on the right", DifferenceType.Removed, "M:CompatTests.First.ShouldReportMethod(System.String,System.String)"), + new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.ShouldReportMissingProperty.get' exists on the left but not on the right", DifferenceType.Removed, "M:CompatTests.First.get_ShouldReportMissingProperty"), + new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.this[int].get' exists on the left but not on the right", DifferenceType.Removed, "M:CompatTests.First.get_Item(System.Int32)"), + new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.ShouldReportMissingEvent.add' exists on the left but not on the right", DifferenceType.Removed, "M:CompatTests.First.add_ShouldReportMissingEvent(CompatTests.EventHandler)"), + new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.ShouldReportMissingEvent.remove' exists on the left but not on the right", DifferenceType.Removed, "M:CompatTests.First.remove_ShouldReportMissingEvent(CompatTests.EventHandler)"), + new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.ReportMissingField' exists on the left but not on the right", DifferenceType.Removed, "F:CompatTests.First.ReportMissingField"), }; Assert.Equal(expected, differences); @@ -171,8 +171,8 @@ public string MultipleOverrides(string a, int b, int c) { } CompatDifference[] expected = new[] { - new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.MultipleOverrides(string, string)' exists on the contract but not on the implementation", DifferenceType.Removed, "M:CompatTests.First.MultipleOverrides(System.String,System.String)"), - new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.MultipleOverrides(string, int, string)' exists on the contract but not on the implementation", DifferenceType.Removed, "M:CompatTests.First.MultipleOverrides(System.String,System.Int32,System.String)"), + new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.MultipleOverrides(string, string)' exists on the left but not on the right", DifferenceType.Removed, "M:CompatTests.First.MultipleOverrides(System.String,System.String)"), + new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.MultipleOverrides(string, int, string)' exists on the left but not on the right", DifferenceType.Removed, "M:CompatTests.First.MultipleOverrides(System.String,System.Int32,System.String)"), }; Assert.Equal(expected, differences); @@ -221,8 +221,8 @@ public string MultipleOverrides(string a, int b, string c) { } { CompatDifference[] expected = new[] { - new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.MultipleOverrides(string, int, int)' exists on the contract but not on the implementation", DifferenceType.Removed, "M:CompatTests.First.MultipleOverrides(System.String,System.Int32,System.Int32)"), - new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.InternalProperty.set' exists on the contract but not on the implementation", DifferenceType.Removed, "M:CompatTests.First.set_InternalProperty(System.Int32)"), + new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.MultipleOverrides(string, int, int)' exists on the left but not on the right", DifferenceType.Removed, "M:CompatTests.First.MultipleOverrides(System.String,System.Int32,System.Int32)"), + new CompatDifference(DiagnosticIds.MemberMustExist, "Member 'CompatTests.First.InternalProperty.set' exists on the left but not on the right", DifferenceType.Removed, "M:CompatTests.First.set_InternalProperty(System.Int32)"), }; Assert.Equal(expected, differences); diff --git a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/TypeMustExistTests.cs b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/TypeMustExistTests.cs index 781d4c648dba..4092071fab57 100644 --- a/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/TypeMustExistTests.cs +++ b/src/Tests/Microsoft.DotNet.ApiCompatibility.Tests/Rules/TypeMustExistTests.cs @@ -44,11 +44,11 @@ public class First { } CompatDifference[] expected = new[] { - new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.Second' exists on the contract but not on the implementation", DifferenceType.Removed, "T:CompatTests.Second"), - new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.MyRecord' exists on the contract but not on the implementation", DifferenceType.Removed, "T:CompatTests.MyRecord"), - new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.MyStruct' exists on the contract but not on the implementation", DifferenceType.Removed, "T:CompatTests.MyStruct"), - new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.MyDelegate' exists on the contract but not on the implementation", DifferenceType.Removed, "T:CompatTests.MyDelegate"), - new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.MyEnum' exists on the contract but not on the implementation", DifferenceType.Removed, "T:CompatTests.MyEnum"), + new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.Second' exists on the left but not on the right", DifferenceType.Removed, "T:CompatTests.Second"), + new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.MyRecord' exists on the left but not on the right", DifferenceType.Removed, "T:CompatTests.MyRecord"), + new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.MyStruct' exists on the left but not on the right", DifferenceType.Removed, "T:CompatTests.MyStruct"), + new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.MyDelegate' exists on the left but not on the right", DifferenceType.Removed, "T:CompatTests.MyDelegate"), + new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.MyEnum' exists on the left but not on the right", DifferenceType.Removed, "T:CompatTests.MyEnum"), }; Assert.Equal(expected, differences); @@ -84,7 +84,7 @@ public class First { } CompatDifference[] expected = new[] { - new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.ForwardedTestType' exists on the contract but not on the implementation", DifferenceType.Removed, "T:CompatTests.ForwardedTestType") + new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.ForwardedTestType' exists on the left but not on the right", DifferenceType.Removed, "T:CompatTests.ForwardedTestType") }; Assert.Equal(expected, differences); @@ -176,8 +176,8 @@ public record First(string a, string b); CompatDifference[] expected = new[] { - new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.Third' exists on the contract but not on the implementation", DifferenceType.Removed, "T:CompatTests.Third"), - new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.Fourth' exists on the contract but not on the implementation", DifferenceType.Removed, "T:CompatTests.Fourth") + new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.Third' exists on the left but not on the right", DifferenceType.Removed, "T:CompatTests.Third"), + new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.Fourth' exists on the left but not on the right", DifferenceType.Removed, "T:CompatTests.Fourth") }; Assert.Equal(expected, differences); @@ -217,7 +217,7 @@ public class First { } { CompatDifference[] expected = new[] { - new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.InternalType' exists on the contract but not on the implementation", DifferenceType.Removed, "T:CompatTests.InternalType") + new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.InternalType' exists on the left but not on the right", DifferenceType.Removed, "T:CompatTests.InternalType") }; Assert.Equal(expected, differences); @@ -264,13 +264,13 @@ internal class InternalNested { } List expected = new() { - new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.First.FirstNested' exists on the contract but not on the implementation", DifferenceType.Removed, "T:CompatTests.First.FirstNested"), + new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.First.FirstNested' exists on the left but not on the right", DifferenceType.Removed, "T:CompatTests.First.FirstNested"), }; if (includeInternalSymbols) { expected.Add( - new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.First.InternalNested.DoubleNested' exists on the contract but not on the implementation", DifferenceType.Removed, "T:CompatTests.First.InternalNested.DoubleNested") + new CompatDifference(DiagnosticIds.TypeMustExist, $"Type 'CompatTests.First.InternalNested.DoubleNested' exists on the left but not on the right", DifferenceType.Removed, "T:CompatTests.First.InternalNested.DoubleNested") ); }