From 07c600ec9d0ca49edf22e860224186ea31039785 Mon Sep 17 00:00:00 2001 From: Matt Kotsenas Date: Thu, 19 Dec 2024 18:10:05 -0800 Subject: [PATCH] Add reference to Microsoft.CodeAnalysis.AnalyzerUtilities and delete forked methods (#290) I didn't know that the analyzer utilities in roslyn-analyzers produces a NuGet package. Reference that package and remove our forked copies of code. --- Directory.Packages.props | 1 + src/Analyzers/Moq.Analyzers.csproj | 1 + src/Analyzers/SquiggleCop.Baseline.yaml | 2 +- src/CodeFixes/Moq.CodeFixes.csproj | 1 + src/CodeFixes/SquiggleCop.Baseline.yaml | 4 +- src/Common/Caching/BoundedCacheWithFactory.cs | 78 ----- src/Common/Common.csproj | 1 + src/Common/ISymbolExtensions.cs | 47 --- src/Common/ITypeSymbolExtensions.cs | 2 +- src/Common/SymbolVisibility.cs | 24 -- src/Common/WellKnown/KnownSymbols.cs | 4 +- src/Common/WellKnown/MoqKnownSymbols.cs | 1 + src/Common/WellKnown/WellKnownTypeProvider.cs | 290 ------------------ .../Moq.Analyzers.Benchmarks.csproj | 1 + .../Moq.Analyzers.Test.csproj | 1 + 15 files changed, 14 insertions(+), 444 deletions(-) delete mode 100644 src/Common/Caching/BoundedCacheWithFactory.cs delete mode 100644 src/Common/SymbolVisibility.cs delete mode 100644 src/Common/WellKnown/WellKnownTypeProvider.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index 96b5061..889471f 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -20,6 +20,7 @@ + diff --git a/src/Analyzers/Moq.Analyzers.csproj b/src/Analyzers/Moq.Analyzers.csproj index 342bfff..e796e4a 100644 --- a/src/Analyzers/Moq.Analyzers.csproj +++ b/src/Analyzers/Moq.Analyzers.csproj @@ -25,6 +25,7 @@ + diff --git a/src/Analyzers/SquiggleCop.Baseline.yaml b/src/Analyzers/SquiggleCop.Baseline.yaml index f282437..82cdb0c 100644 --- a/src/Analyzers/SquiggleCop.Baseline.yaml +++ b/src/Analyzers/SquiggleCop.Baseline.yaml @@ -324,7 +324,7 @@ - {Id: ECS0700, Title: Express callbacks with delegates, Category: Design, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false} - {Id: ECS0800, Title: Use the Null Conditional Operator for Event Invocations, Category: Usage, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false} - {Id: ECS0900, Title: Minimize boxing and unboxing, Category: Performance, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: true} -- {Id: ECS1200, Title: Prefer member initializers to assignment statements, Category: Maintainability, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: true} +- {Id: ECS1200, Title: Prefer member initializers to assignment statements, Category: Maintainability, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false} - {Id: ECS1300, Title: Use proper initialization for static class members, Category: Initialization, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false} - {Id: ECS1400, Title: Minimize duplicate initialization logic, Category: Initialization, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false} - {Id: EM0001, Title: Switch on Enum Not Exhaustive, Category: Logic, DefaultSeverity: Error, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false} diff --git a/src/CodeFixes/Moq.CodeFixes.csproj b/src/CodeFixes/Moq.CodeFixes.csproj index 8121da5..d05ca7c 100644 --- a/src/CodeFixes/Moq.CodeFixes.csproj +++ b/src/CodeFixes/Moq.CodeFixes.csproj @@ -8,6 +8,7 @@ + diff --git a/src/CodeFixes/SquiggleCop.Baseline.yaml b/src/CodeFixes/SquiggleCop.Baseline.yaml index f709a50..20ddc36 100644 --- a/src/CodeFixes/SquiggleCop.Baseline.yaml +++ b/src/CodeFixes/SquiggleCop.Baseline.yaml @@ -324,7 +324,7 @@ - {Id: ECS0700, Title: Express callbacks with delegates, Category: Design, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false} - {Id: ECS0800, Title: Use the Null Conditional Operator for Event Invocations, Category: Usage, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false} - {Id: ECS0900, Title: Minimize boxing and unboxing, Category: Performance, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: true} -- {Id: ECS1200, Title: Prefer member initializers to assignment statements, Category: Maintainability, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: true} +- {Id: ECS1200, Title: Prefer member initializers to assignment statements, Category: Maintainability, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false} - {Id: ECS1300, Title: Use proper initialization for static class members, Category: Initialization, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false} - {Id: ECS1400, Title: Minimize duplicate initialization logic, Category: Initialization, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false} - {Id: EM0001, Title: Switch on Enum Not Exhaustive, Category: Logic, DefaultSeverity: Error, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false} @@ -516,7 +516,7 @@ - {Id: MA0048, Title: File name must match type name, Category: Design, DefaultSeverity: Warning, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false} - {Id: MA0049, Title: Type name should not match containing namespace, Category: Design, DefaultSeverity: Error, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false} - {Id: MA0050, Title: Validate arguments correctly in iterator methods, Category: Design, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: false} -- {Id: MA0051, Title: Method is too long, Category: Design, DefaultSeverity: Warning, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: true} +- {Id: MA0051, Title: Method is too long, Category: Design, DefaultSeverity: Warning, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false} - {Id: MA0052, Title: Replace constant Enum.ToString with nameof, Category: Performance, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: false} - {Id: MA0053, Title: Make class sealed, Category: Design, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: false} - {Id: MA0054, Title: Embed the caught exception as innerException, Category: Design, DefaultSeverity: Warning, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false} diff --git a/src/Common/Caching/BoundedCacheWithFactory.cs b/src/Common/Caching/BoundedCacheWithFactory.cs deleted file mode 100644 index db39f7d..0000000 --- a/src/Common/Caching/BoundedCacheWithFactory.cs +++ /dev/null @@ -1,78 +0,0 @@ -// Forked from https://github.com/dotnet/roslyn-analyzers/blob/5dd0dd5db8fa79932517f153854c0f2c24ac98a3/src/Utilities/Compiler/BoundedCacheWithFactory.cs - -namespace Moq.Analyzers.Common.Caching; - -/// -/// Provides bounded cache for analyzers. -/// Acts as a good alternative to -/// when the cached value has a cyclic reference to the key preventing early garbage collection of entries. -/// -/// The type to use as a cache key. -/// The type to use as the cached value. -internal class BoundedCacheWithFactory - where TKey : class -{ - // Bounded weak reference cache. - // Size 5 is an arbitrarily chosen bound, which can be tuned in future as required. - private readonly List> _weakReferencedEntries = new() - { - new WeakReference(null), - new WeakReference(null), - new WeakReference(null), - new WeakReference(null), - new WeakReference(null), - }; - - public TValue GetOrCreateValue(TKey key, Func valueFactory) - { - lock (_weakReferencedEntries) - { - int indexToSetTarget = -1; - for (int i = 0; i < _weakReferencedEntries.Count; i++) - { - WeakReference weakReferencedEntry = _weakReferencedEntries[i]; - if (!weakReferencedEntry.TryGetTarget(out Entry? cachedEntry) || - cachedEntry == null) - { - if (indexToSetTarget == -1) - { - indexToSetTarget = i; - } - - continue; - } - - if (Equals(cachedEntry.Key, key)) - { - // Move the cache hit item to the end of the list - // so it would be least likely to be evicted on next cache miss. - _weakReferencedEntries.RemoveAt(i); - _weakReferencedEntries.Add(weakReferencedEntry); - return cachedEntry.Value; - } - } - - if (indexToSetTarget == -1) - { - indexToSetTarget = 0; - } - - Entry newEntry = new Entry(key, valueFactory(key)); - _weakReferencedEntries[indexToSetTarget].SetTarget(newEntry); - return newEntry.Value; - } - } - - private sealed class Entry - { - public Entry(TKey key, TValue value) - { - Key = key; - Value = value; - } - - public TKey Key { get; } - - public TValue Value { get; } - } -} diff --git a/src/Common/Common.csproj b/src/Common/Common.csproj index 5a1087c..02cc6be 100644 --- a/src/Common/Common.csproj +++ b/src/Common/Common.csproj @@ -30,6 +30,7 @@ + diff --git a/src/Common/ISymbolExtensions.cs b/src/Common/ISymbolExtensions.cs index 77bb4e1..6cc992e 100644 --- a/src/Common/ISymbolExtensions.cs +++ b/src/Common/ISymbolExtensions.cs @@ -79,51 +79,4 @@ public static bool IsOverridable(this ISymbol symbol) { return !symbol.IsSealed && (symbol.IsVirtual || symbol.IsAbstract || symbol.IsOverride); } - - public static SymbolVisibility GetResultantVisibility(this ISymbol symbol) - { - // Start by assuming it's visible. - SymbolVisibility visibility = SymbolVisibility.Public; - - switch (symbol.Kind) - { - case SymbolKind.Alias: - // Aliases are uber private. They're only visible in the same file that they - // were declared in. - return SymbolVisibility.Private; - - case SymbolKind.Parameter: - // Parameters are only as visible as their containing symbol - return GetResultantVisibility(symbol.ContainingSymbol); - - case SymbolKind.TypeParameter: - // Type Parameters are private. - return SymbolVisibility.Private; - } - - while (symbol != null && symbol.Kind != SymbolKind.Namespace) - { - switch (symbol.DeclaredAccessibility) - { - // If we see anything private, then the symbol is private. - case Accessibility.NotApplicable: - case Accessibility.Private: - return SymbolVisibility.Private; - - // If we see anything internal, then knock it down from public to - // internal. - case Accessibility.Internal: - case Accessibility.ProtectedAndInternal: - visibility = SymbolVisibility.Internal; - break; - - // For anything else (Public, Protected, ProtectedOrInternal), the - // symbol stays at the level we've gotten so far. - } - - symbol = symbol.ContainingSymbol; - } - - return visibility; - } } diff --git a/src/Common/ITypeSymbolExtensions.cs b/src/Common/ITypeSymbolExtensions.cs index 23134f5..307a8af 100644 --- a/src/Common/ITypeSymbolExtensions.cs +++ b/src/Common/ITypeSymbolExtensions.cs @@ -12,7 +12,7 @@ internal static class ITypeSymbolExtensions /// The type and any inherited types. public static IEnumerable GetBaseTypesAndThis(this ITypeSymbol type) { - var current = type; + ITypeSymbol? current = type; while (current is not null) { yield return current; diff --git a/src/Common/SymbolVisibility.cs b/src/Common/SymbolVisibility.cs deleted file mode 100644 index 78f0136..0000000 --- a/src/Common/SymbolVisibility.cs +++ /dev/null @@ -1,24 +0,0 @@ -namespace Moq.Analyzers.Common; - -internal enum SymbolVisibility -{ - /// - /// Public symbol visibility. - /// - Public = 0, - - /// - /// Internal symbol visibility. - /// - Internal = 1, - - /// - /// Private symbol visibility. - /// - Private = 2, - - /// - /// Internal symbol visibility. - /// - Friend = Internal, -} diff --git a/src/Common/WellKnown/KnownSymbols.cs b/src/Common/WellKnown/KnownSymbols.cs index cc522bb..7f8e466 100644 --- a/src/Common/WellKnown/KnownSymbols.cs +++ b/src/Common/WellKnown/KnownSymbols.cs @@ -1,4 +1,6 @@ -namespace Moq.Analyzers.Common.WellKnown; +using Analyzer.Utilities; + +namespace Moq.Analyzers.Common.WellKnown; /// /// Main entrypoint to access well-known symbols for the analyzer. diff --git a/src/Common/WellKnown/MoqKnownSymbols.cs b/src/Common/WellKnown/MoqKnownSymbols.cs index 045ba0f..d245236 100644 --- a/src/Common/WellKnown/MoqKnownSymbols.cs +++ b/src/Common/WellKnown/MoqKnownSymbols.cs @@ -1,4 +1,5 @@ using System.Diagnostics.CodeAnalysis; +using Analyzer.Utilities; namespace Moq.Analyzers.Common.WellKnown; diff --git a/src/Common/WellKnown/WellKnownTypeProvider.cs b/src/Common/WellKnown/WellKnownTypeProvider.cs deleted file mode 100644 index a8a3e33..0000000 --- a/src/Common/WellKnown/WellKnownTypeProvider.cs +++ /dev/null @@ -1,290 +0,0 @@ -// Forked from https://github.com/dotnet/roslyn-analyzers/blob/5dd0dd5db8fa79932517f153854c0f2c24ac98a3/src/Utilities/Compiler/WellKnownTypeProvider.cs - -using System.Collections.Concurrent; -using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; -using System.Globalization; -using Moq.Analyzers.Common.Caching; -using Roslyn.Utilities; - -namespace Moq.Analyzers.Common.WellKnown; - -/// -/// Provides and caches well known types in a compilation. -/// -internal class WellKnownTypeProvider -{ - private static readonly BoundedCacheWithFactory ProviderCache = new(); - - /// - /// Static cache of full type names (with namespaces) to namespace name parts, - /// so we can query . - /// - /// - /// Example: "System.Collections.Generic.List`1" => [ "System", "Collections", "Generic" ] - /// - /// https://github.com/dotnet/roslyn/blob/9e786147b8cb884af454db081bb747a5bd36a086/src/Compilers/CSharp/Portable/Symbols/AssemblySymbol.cs#L455 - /// suggests the TypeNames collection can be checked to avoid expensive operations. But realizing TypeNames seems to be - /// as memory intensive as unnecessary calls GetTypeByMetadataName() in some cases. So we'll go with namespace names. - /// - private static readonly ConcurrentDictionary> _fullTypeNameToNamespaceNames = - new(StringComparer.Ordinal); - - /// - /// All the referenced assembly symbols. - /// - /// - /// Seems to be less memory intensive than: - /// foreach (Compilation.Assembly.Modules) - /// foreach (Module.ReferencedAssemblySymbols). - /// - private readonly Lazy> _referencedAssemblies; - - /// - /// Mapping of full name to . - /// - private readonly ConcurrentDictionary _fullNameToTypeMap = new(StringComparer.Ordinal); - - private WellKnownTypeProvider(Compilation compilation) - { - Compilation = compilation; -#pragma warning disable ECS1200 - _referencedAssemblies = new Lazy>( - () => - { - return Compilation.Assembly.Modules -#pragma warning disable ECS0900 - .SelectMany(m => m.ReferencedAssemblySymbols) -#pragma warning restore ECS0900 - .Distinct(SymbolEqualityComparer.Default) - .ToImmutableArray(); - }, - LazyThreadSafetyMode.ExecutionAndPublication); -#pragma warning restore ECS1200 - } - - /// - /// Gets the associated with this type provider. - /// - public Compilation Compilation { get; } - - /// - /// Get an existing or create a new for the given . - /// - /// The to create the type provider from. - /// A associated with the given . - public static WellKnownTypeProvider GetOrCreate(Compilation compilation) - { - return ProviderCache.GetOrCreateValue(compilation, CreateWellKnownTypeProvider); - - // Local functions - static WellKnownTypeProvider CreateWellKnownTypeProvider(Compilation compilation) => new(compilation); - } - - /// - /// Attempts to get the type by the full type name. - /// - /// Namespace + type name, e.g. "System.Exception". - /// Named type symbol, if any. - /// True if found in the compilation, false otherwise. - [PerformanceSensitive("https://github.com/dotnet/roslyn-analyzers/issues/4893", AllowCaptures = false)] - public bool TryGetOrCreateTypeByMetadataName( - string fullTypeName, - [NotNullWhen(returnValue: true)] out INamedTypeSymbol? namedTypeSymbol) - { - if (_fullNameToTypeMap.TryGetValue(fullTypeName, out namedTypeSymbol)) - { - return namedTypeSymbol is not null; - } - - return TryGetOrCreateTypeByMetadataNameSlow(fullTypeName, out namedTypeSymbol); - } - - /// - /// Gets a type by its full type name. - /// - /// Namespace + type name, e.g. "System.Exception". - /// The if found, null otherwise. - public INamedTypeSymbol? GetOrCreateTypeByMetadataName(string fullTypeName) - { - TryGetOrCreateTypeByMetadataName(fullTypeName, out INamedTypeSymbol? namedTypeSymbol); - return namedTypeSymbol; - } - - private static ImmutableArray GetNamespaceNamesFromFullTypeName(string fullTypeName) - { - ImmutableArray.Builder namespaceNamesBuilder = ImmutableArray.CreateBuilder(); - - int prevStartIndex = 0; - for (int i = 0; i < fullTypeName.Length; i++) - { - if (fullTypeName[i] == '.') - { - namespaceNamesBuilder.Add(fullTypeName[prevStartIndex..i]); - prevStartIndex = i + 1; - } - else if (!IsIdentifierPartCharacter(fullTypeName[i])) - { - break; - } - } - - return namespaceNamesBuilder.ToImmutable(); - } - - /// - /// Returns true if the Unicode character can be a part of an identifier. - /// - /// The Unicode character. - private static bool IsIdentifierPartCharacter(char ch) - { - // identifier-part-character: - // letter-character - // decimal-digit-character - // connecting-character - // combining-character - // formatting-character - - // '\u0061' - if (ch < 'a') - { - // '\u0041' - if (ch < 'A') - { - // '\u0030' and '\u0039' - return ch is >= '0' - and <= '9'; - } - - // '\u005A' or '\u005F' - return ch is <= 'Z' - or '_'; - } - - // '\u007A' - if (ch <= 'z') - { - return true; - } - - // max ASCII - if (ch <= '\u007F') - { - return false; - } - - UnicodeCategory cat = CharUnicodeInfo.GetUnicodeCategory(ch); - - return cat switch - { - // Letter - UnicodeCategory.UppercaseLetter - or UnicodeCategory.LowercaseLetter - or UnicodeCategory.TitlecaseLetter - or UnicodeCategory.ModifierLetter - or UnicodeCategory.OtherLetter - or UnicodeCategory.LetterNumber - or UnicodeCategory.DecimalDigitNumber - or UnicodeCategory.ConnectorPunctuation - or UnicodeCategory.NonSpacingMark - or UnicodeCategory.SpacingCombiningMark - or UnicodeCategory.Format => true, - _ => false, - }; - } - - private static bool IsSubsetOfCollection(ImmutableArray set1, ICollection set2) - { - if (set1.Length > set2.Count) - { - return false; - } - - for (int i = 0; i < set1.Length; i++) - { - if (!set2.Contains(set1[i])) - { - return false; - } - } - - return true; - } - - [SuppressMessage("Design", "MA0051:Method is too long", Justification = "Forked from upstream. Avoiding refactoring to reduce divergence.")] - private bool TryGetOrCreateTypeByMetadataNameSlow( - string fullTypeName, - [NotNullWhen(returnValue: true)] out INamedTypeSymbol? namedTypeSymbol) - { - namedTypeSymbol = _fullNameToTypeMap.GetOrAdd(fullTypeName, ValueFactory, fullTypeName); - - return namedTypeSymbol != null; - - [SuppressMessage("Design", "MA0051:Method is too long", Justification = "Forked from upstream. Avoiding refactoring to reduce divergence.")] - INamedTypeSymbol? ValueFactory(string fullyQualifiedMetadataName, string fullTypeName) - { - // Caching null results is intended. - - // sharwell says: Suppose you reference assembly A with public API X.Y, and you reference assembly B with - // internal API X.Y. Even though you can use X.Y from assembly A, compilation.GetTypeByMetadataName will - // fail outright because it finds two types with the same name. - INamedTypeSymbol? type = null; - - ImmutableArray namespaceNames; - if (string.IsInterned(fullTypeName) != null) - { - namespaceNames = _fullTypeNameToNamespaceNames.GetOrAdd( - fullTypeName, - GetNamespaceNamesFromFullTypeName); - } - else - { - namespaceNames = GetNamespaceNamesFromFullTypeName(fullTypeName); - } - - if (IsSubsetOfCollection(namespaceNames, Compilation.Assembly.NamespaceNames)) - { - type = Compilation.Assembly.GetTypeByMetadataName(fullyQualifiedMetadataName); - } - - if (type is null) - { - Debug.Assert(namespaceNames != null, $"{nameof(namespaceNames)} should not be null"); - - foreach (IAssemblySymbol? referencedAssembly in _referencedAssemblies.Value) - { - if (!IsSubsetOfCollection(namespaceNames, referencedAssembly.NamespaceNames)) - { - continue; - } - - INamedTypeSymbol? currentType = referencedAssembly.GetTypeByMetadataName(fullyQualifiedMetadataName); - if (currentType is null) - { - continue; - } - - switch (currentType.GetResultantVisibility()) - { - case SymbolVisibility.Public: - case SymbolVisibility.Internal when referencedAssembly.GivesAccessTo(Compilation.Assembly): - break; - - default: - continue; - } - - if (type is object) - { - // Multiple visible types with the same metadata name are present. - return null; - } - - type = currentType; - } - } - - return type; - } - } -} diff --git a/tests/Moq.Analyzers.Benchmarks/Moq.Analyzers.Benchmarks.csproj b/tests/Moq.Analyzers.Benchmarks/Moq.Analyzers.Benchmarks.csproj index 5e9bd27..9c81fdf 100644 --- a/tests/Moq.Analyzers.Benchmarks/Moq.Analyzers.Benchmarks.csproj +++ b/tests/Moq.Analyzers.Benchmarks/Moq.Analyzers.Benchmarks.csproj @@ -11,6 +11,7 @@ + diff --git a/tests/Moq.Analyzers.Test/Moq.Analyzers.Test.csproj b/tests/Moq.Analyzers.Test/Moq.Analyzers.Test.csproj index 8a0a9e4..2e46677 100644 --- a/tests/Moq.Analyzers.Test/Moq.Analyzers.Test.csproj +++ b/tests/Moq.Analyzers.Test/Moq.Analyzers.Test.csproj @@ -22,6 +22,7 @@ +