From 90a716b0908fcbeb9cd0ebe3ff5b4436ef9159ce Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 4 Apr 2023 14:44:27 -0700 Subject: [PATCH 1/5] Improve performance of spell checking analyzer --- .../Core/CodeAnalysisDictionary.cs | 44 +++++++------- .../IdentifiersShouldBeSpelledCorrectly.cs | 59 ++++++++++++++----- 2 files changed, 66 insertions(+), 37 deletions(-) diff --git a/src/Text.Analyzers/Core/CodeAnalysisDictionary.cs b/src/Text.Analyzers/Core/CodeAnalysisDictionary.cs index a030ac4c2e..a1b522b9c9 100644 --- a/src/Text.Analyzers/Core/CodeAnalysisDictionary.cs +++ b/src/Text.Analyzers/Core/CodeAnalysisDictionary.cs @@ -2,7 +2,6 @@ using System; using System.Collections.Generic; -using System.Collections.Immutable; using System.IO; using System.Linq; using System.Xml.Linq; @@ -10,25 +9,14 @@ namespace Text.Analyzers { /// - /// Source for "recognized" misspellings and "unrecognized" spellings obtained by parsing either - /// XML or DIC code analysis dictionaries. + /// Source for "recognized" misspellings and "unrecognized" spellings obtained by parsing either XML or DIC code + /// analysis dictionaries. /// /// /// /// internal sealed class CodeAnalysisDictionary { - /// - /// Initialize a new instance of . - /// - /// Misspelled words that the spell checker will now ignore. - /// Correctly spelled words that the spell checker will now report. - private CodeAnalysisDictionary(IEnumerable recognizedWords, IEnumerable unrecognizedWords) - { - RecognizedWords = ImmutableHashSet.Create(StringComparer.OrdinalIgnoreCase, recognizedWords.ToArray()); - UnrecognizedWords = ImmutableHashSet.Create(StringComparer.OrdinalIgnoreCase, unrecognizedWords.ToArray()); - } - /// /// A list of misspelled words that the spell checker will now ignore. /// @@ -39,7 +27,7 @@ private CodeAnalysisDictionary(IEnumerable recognizedWords, IEnumerable< /// /// /// - public ImmutableHashSet RecognizedWords { get; } + private readonly HashSet _recognizedWords; /// /// A list of correctly spelled words that the spell checker will now report. @@ -51,7 +39,18 @@ private CodeAnalysisDictionary(IEnumerable recognizedWords, IEnumerable< /// /// /// - public ImmutableHashSet UnrecognizedWords { get; } + private readonly HashSet _unrecognizedWords; + + /// + /// Initialize a new instance of . + /// + /// Misspelled words that the spell checker will now ignore. + /// Correctly spelled words that the spell checker will now report. + private CodeAnalysisDictionary(IEnumerable recognizedWords, IEnumerable unrecognizedWords) + { + _recognizedWords = new HashSet(recognizedWords, StringComparer.OrdinalIgnoreCase); + _unrecognizedWords = new HashSet(unrecognizedWords, StringComparer.OrdinalIgnoreCase); + } /// /// Creates a new instance of this class with recognized and unrecognized words (if specified) loaded @@ -97,14 +96,13 @@ public static CodeAnalysisDictionary CreateFromDic(StreamReader streamReader) return new CodeAnalysisDictionary(recognizedWords, Enumerable.Empty()); } - public static CodeAnalysisDictionary CreateFromDictionaries(IEnumerable dictionaries) - { - var recognizedWords = dictionaries.Select(x => x.RecognizedWords).Aggregate((x, y) => x.Union(y)); - var unrecognizedWords = dictionaries.Select(x => x.UnrecognizedWords).Aggregate((x, y) => x.Union(y)); - return new CodeAnalysisDictionary(recognizedWords, unrecognizedWords); - } - private static IEnumerable GetSectionWords(XDocument document, string section, string property) => document.Descendants(section).SelectMany(section => section.Elements(property)).Select(element => element.Value.Trim()); + + public bool ContainsUnrecognizedWord(string word) + => _unrecognizedWords.Contains(word); + + public bool ContainsRecognizedWord(string word) + => _recognizedWords.Contains(word); } } diff --git a/src/Text.Analyzers/Core/IdentifiersShouldBeSpelledCorrectly.cs b/src/Text.Analyzers/Core/IdentifiersShouldBeSpelledCorrectly.cs index 9ddef30efc..400a8f52fd 100644 --- a/src/Text.Analyzers/Core/IdentifiersShouldBeSpelledCorrectly.cs +++ b/src/Text.Analyzers/Core/IdentifiersShouldBeSpelledCorrectly.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; +using System.Runtime.CompilerServices; using Analyzer.Utilities; using Analyzer.Utilities.Extensions; using Microsoft.CodeAnalysis; @@ -210,6 +211,12 @@ public sealed class IdentifiersShouldBeSpelledCorrectlyAnalyzer : DiagnosticAnal isPortedFxCopRule: true, isDataflowRule: false); + /// + /// Todo, use an actual AdditionalTextValueProvider once it is available: + /// https://github.com/dotnet/roslyn/issues/67611 + /// + private readonly ConditionalWeakTable _textToDictionary = new(); + public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create( FileParseRule, AssemblyRule, @@ -238,14 +245,15 @@ public override void Initialize(AnalysisContext context) context.RegisterCompilationStartAction(OnCompilationStart); } - private static void OnCompilationStart(CompilationStartAnalysisContext compilationStartContext) + private void OnCompilationStart(CompilationStartAnalysisContext context) { - var dictionaries = ReadDictionaries(); - var projectDictionary = CodeAnalysisDictionary.CreateFromDictionaries(dictionaries.Concat(s_mainDictionary)); + var cancellationToken = context.CancellationToken; - compilationStartContext.RegisterOperationAction(AnalyzeVariable, OperationKind.VariableDeclarator); - compilationStartContext.RegisterCompilationEndAction(AnalyzeAssembly); - compilationStartContext.RegisterSymbolAction( + var dictionaries = ReadDictionaries().Add(s_mainDictionary); + + context.RegisterOperationAction(AnalyzeVariable, OperationKind.VariableDeclarator); + context.RegisterCompilationEndAction(AnalyzeAssembly); + context.RegisterSymbolAction( AnalyzeSymbol, SymbolKind.Namespace, SymbolKind.NamedType, @@ -255,21 +263,39 @@ private static void OnCompilationStart(CompilationStartAnalysisContext compilati SymbolKind.Field, SymbolKind.Parameter); - IEnumerable ReadDictionaries() + ImmutableArray ReadDictionaries() { - var fileProvider = AdditionalFileProvider.FromOptions(compilationStartContext.Options); + var fileProvider = AdditionalFileProvider.FromOptions(context.Options); return fileProvider.GetMatchingFiles(@"(?:dictionary|custom).*?\.(?:xml|dic)$") - .Select(CreateDictionaryFromAdditionalText) + .Select(GetOrCreateDictionaryFromAdditionalText) .Where(x => x != null) - .ToList(); + .ToImmutableArray(); + + CodeAnalysisDictionary GetOrCreateDictionaryFromAdditionalText(AdditionalText additionalText) + { + if (!_textToDictionary.TryGetValue(additionalText, out var dictionary)) + { + dictionary = CreateAndAddDictionary(additionalText); + } + + return dictionary; + + // Local function to avoid unnecessary lambda alloc in mainline case. + CodeAnalysisDictionary CreateAndAddDictionary(AdditionalText additionalText) + { + var dictionary = CreateDictionaryFromAdditionalText(additionalText); + dictionary = _textToDictionary.GetValue(additionalText, _ => dictionary); + return dictionary; + } + } CodeAnalysisDictionary CreateDictionaryFromAdditionalText(AdditionalText additionalFile) { - var text = additionalFile.GetText(compilationStartContext.CancellationToken); + var text = additionalFile.GetText(context.CancellationToken); var isXml = additionalFile.Path.EndsWith(".xml", StringComparison.OrdinalIgnoreCase); var provider = isXml ? s_xmlDictionaryProvider : s_dicDictionaryProvider; - if (!compilationStartContext.TryGetValue(text, provider, out var dictionary)) + if (!context.TryGetValue(text, provider, out var dictionary)) { try { @@ -292,7 +318,7 @@ CodeAnalysisDictionary CreateDictionaryFromAdditionalText(AdditionalText additio void ReportFileParseDiagnostic(string filePath, string message) { var diagnostic = Diagnostic.Create(FileParseRule, Location.None, filePath, message); - compilationStartContext.RegisterCompilationEndAction(x => x.ReportDiagnostic(diagnostic)); + context.RegisterCompilationEndAction(x => x.ReportDiagnostic(diagnostic)); } } @@ -409,7 +435,12 @@ IEnumerable GetMisspelledWords(string symbolName) static bool IsWordNumeric(string word) => char.IsDigit(word[0]); bool IsWordSpelledCorrectly(string word) - => !projectDictionary.UnrecognizedWords.Contains(word) && projectDictionary.RecognizedWords.Contains(word); + { + var unrecognizedWordsContains = dictionaries.Any(static (d, word) => d.ContainsUnrecognizedWord(word), word); + var recognizedWordsContains = dictionaries.Any(static (d, word) => d.ContainsRecognizedWord(word), word); + + return !unrecognizedWordsContains && recognizedWordsContains; + } } /// From 54c14f3cdeb2da5695234d62248138bb63ba8ccd Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Tue, 4 Apr 2023 15:12:11 -0700 Subject: [PATCH 2/5] Update src/Text.Analyzers/Core/IdentifiersShouldBeSpelledCorrectly.cs --- .../Core/IdentifiersShouldBeSpelledCorrectly.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Text.Analyzers/Core/IdentifiersShouldBeSpelledCorrectly.cs b/src/Text.Analyzers/Core/IdentifiersShouldBeSpelledCorrectly.cs index 400a8f52fd..450d8bb1c9 100644 --- a/src/Text.Analyzers/Core/IdentifiersShouldBeSpelledCorrectly.cs +++ b/src/Text.Analyzers/Core/IdentifiersShouldBeSpelledCorrectly.cs @@ -436,10 +436,7 @@ IEnumerable GetMisspelledWords(string symbolName) bool IsWordSpelledCorrectly(string word) { - var unrecognizedWordsContains = dictionaries.Any(static (d, word) => d.ContainsUnrecognizedWord(word), word); - var recognizedWordsContains = dictionaries.Any(static (d, word) => d.ContainsRecognizedWord(word), word); - - return !unrecognizedWordsContains && recognizedWordsContains; + return !dictionaries.Any(static (d, word) => d.ContainsUnrecognizedWord(word), word) && dictionaries.Any(static (d, word) => d.ContainsRecognizedWord(word), word); } } From 52019935407d6098a0bec367c2f578d34f9bdd81 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 4 Apr 2023 15:27:00 -0700 Subject: [PATCH 3/5] Switch to provider --- .../IdentifiersShouldBeSpelledCorrectly.cs | 75 +++++++------------ 1 file changed, 25 insertions(+), 50 deletions(-) diff --git a/src/Text.Analyzers/Core/IdentifiersShouldBeSpelledCorrectly.cs b/src/Text.Analyzers/Core/IdentifiersShouldBeSpelledCorrectly.cs index 400a8f52fd..9de4a9aa35 100644 --- a/src/Text.Analyzers/Core/IdentifiersShouldBeSpelledCorrectly.cs +++ b/src/Text.Analyzers/Core/IdentifiersShouldBeSpelledCorrectly.cs @@ -4,7 +4,6 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; -using System.Runtime.CompilerServices; using Analyzer.Utilities; using Analyzer.Utilities.Extensions; using Microsoft.CodeAnalysis; @@ -27,8 +26,8 @@ public sealed class IdentifiersShouldBeSpelledCorrectlyAnalyzer : DiagnosticAnal private static readonly LocalizableString s_localizableTitle = CreateLocalizableResourceString(nameof(IdentifiersShouldBeSpelledCorrectlyTitle)); private static readonly LocalizableString s_localizableDescription = CreateLocalizableResourceString(nameof(IdentifiersShouldBeSpelledCorrectlyDescription)); - private static readonly SourceTextValueProvider s_xmlDictionaryProvider = new(ParseXmlDictionary); - private static readonly SourceTextValueProvider s_dicDictionaryProvider = new(ParseDicDictionary); + private static readonly SourceTextValueProvider<(CodeAnalysisDictionary dictionary, Exception? exception)> s_xmlDictionaryProvider = new(static text => ParseDictionary(text, isXml: false)); + private static readonly SourceTextValueProvider<(CodeAnalysisDictionary dictionary, Exception? exception)> s_dicDictionaryProvider = new(static text => ParseDictionary(text, isXml: false)); private static readonly CodeAnalysisDictionary s_mainDictionary = GetMainDictionary(); internal static readonly DiagnosticDescriptor FileParseRule = DiagnosticDescriptorHelper.Create( @@ -211,12 +210,6 @@ public sealed class IdentifiersShouldBeSpelledCorrectlyAnalyzer : DiagnosticAnal isPortedFxCopRule: true, isDataflowRule: false); - /// - /// Todo, use an actual AdditionalTextValueProvider once it is available: - /// https://github.com/dotnet/roslyn/issues/67611 - /// - private readonly ConditionalWeakTable _textToDictionary = new(); - public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create( FileParseRule, AssemblyRule, @@ -273,52 +266,20 @@ ImmutableArray ReadDictionaries() CodeAnalysisDictionary GetOrCreateDictionaryFromAdditionalText(AdditionalText additionalText) { - if (!_textToDictionary.TryGetValue(additionalText, out var dictionary)) - { - dictionary = CreateAndAddDictionary(additionalText); - } + var isXml = additionalText.Path.EndsWith(".xml", StringComparison.OrdinalIgnoreCase); + var provider = isXml ? s_xmlDictionaryProvider : s_dicDictionaryProvider; - return dictionary; + var (dictionary, exception) = context.TryGetValue(additionalText.GetText(cancellationToken), provider, out var result) + ? result + : default; - // Local function to avoid unnecessary lambda alloc in mainline case. - CodeAnalysisDictionary CreateAndAddDictionary(AdditionalText additionalText) + if (exception != null) { - var dictionary = CreateDictionaryFromAdditionalText(additionalText); - dictionary = _textToDictionary.GetValue(additionalText, _ => dictionary); - return dictionary; + var diagnostic = Diagnostic.Create(FileParseRule, Location.None, additionalText.Path, exception.Message); + context.RegisterCompilationEndAction(x => x.ReportDiagnostic(diagnostic)); } - } - - CodeAnalysisDictionary CreateDictionaryFromAdditionalText(AdditionalText additionalFile) - { - var text = additionalFile.GetText(context.CancellationToken); - var isXml = additionalFile.Path.EndsWith(".xml", StringComparison.OrdinalIgnoreCase); - var provider = isXml ? s_xmlDictionaryProvider : s_dicDictionaryProvider; - if (!context.TryGetValue(text, provider, out var dictionary)) - { - try - { - // Annoyingly (and expectedly), TryGetValue swallows the parsing exception, - // so we have to parse again to get it. - var unused = isXml ? ParseXmlDictionary(text) : ParseDicDictionary(text); - ReportFileParseDiagnostic(additionalFile.Path, "Unknown error"); - } -#pragma warning disable CA1031 // Do not catch general exception types - catch (Exception ex) -#pragma warning restore CA1031 // Do not catch general exception types - { - ReportFileParseDiagnostic(additionalFile.Path, ex.Message); - } - } - - return dictionary; - } - - void ReportFileParseDiagnostic(string filePath, string message) - { - var diagnostic = Diagnostic.Create(FileParseRule, Location.None, filePath, message); - context.RegisterCompilationEndAction(x => x.ReportDiagnostic(diagnostic)); + return dictionary!; } } @@ -499,6 +460,20 @@ private static CodeAnalysisDictionary GetMainDictionary() return ParseDicDictionary(text); } + private static (CodeAnalysisDictionary dictionary, Exception? exception) ParseDictionary(SourceText text, bool isXml) + { + try + { + return (isXml ? ParseXmlDictionary(text) : ParseDicDictionary(text), exception: null); + } +#pragma warning disable CA1031 // Do not catch general exception types + catch (Exception ex) +#pragma warning restore CA1031 // Do not catch general exception types + { + return (null!, ex); + } + } + private static CodeAnalysisDictionary ParseXmlDictionary(SourceText text) => text.Parse(CodeAnalysisDictionary.CreateFromXml); From 14cfa984bad018e8319c675103edbb42b42dfe14 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 4 Apr 2023 15:28:30 -0700 Subject: [PATCH 4/5] true --- src/Text.Analyzers/Core/IdentifiersShouldBeSpelledCorrectly.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Text.Analyzers/Core/IdentifiersShouldBeSpelledCorrectly.cs b/src/Text.Analyzers/Core/IdentifiersShouldBeSpelledCorrectly.cs index da88bb0337..689c82a012 100644 --- a/src/Text.Analyzers/Core/IdentifiersShouldBeSpelledCorrectly.cs +++ b/src/Text.Analyzers/Core/IdentifiersShouldBeSpelledCorrectly.cs @@ -26,7 +26,7 @@ public sealed class IdentifiersShouldBeSpelledCorrectlyAnalyzer : DiagnosticAnal private static readonly LocalizableString s_localizableTitle = CreateLocalizableResourceString(nameof(IdentifiersShouldBeSpelledCorrectlyTitle)); private static readonly LocalizableString s_localizableDescription = CreateLocalizableResourceString(nameof(IdentifiersShouldBeSpelledCorrectlyDescription)); - private static readonly SourceTextValueProvider<(CodeAnalysisDictionary dictionary, Exception? exception)> s_xmlDictionaryProvider = new(static text => ParseDictionary(text, isXml: false)); + private static readonly SourceTextValueProvider<(CodeAnalysisDictionary dictionary, Exception? exception)> s_xmlDictionaryProvider = new(static text => ParseDictionary(text, isXml: true)); private static readonly SourceTextValueProvider<(CodeAnalysisDictionary dictionary, Exception? exception)> s_dicDictionaryProvider = new(static text => ParseDictionary(text, isXml: false)); private static readonly CodeAnalysisDictionary s_mainDictionary = GetMainDictionary(); From bac0aeef523770c8ca917c8b12cb7937f9f871c4 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 4 Apr 2023 15:29:11 -0700 Subject: [PATCH 5/5] Simplify --- .../IdentifiersShouldBeSpelledCorrectly.cs | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Text.Analyzers/Core/IdentifiersShouldBeSpelledCorrectly.cs b/src/Text.Analyzers/Core/IdentifiersShouldBeSpelledCorrectly.cs index 689c82a012..029a6350f9 100644 --- a/src/Text.Analyzers/Core/IdentifiersShouldBeSpelledCorrectly.cs +++ b/src/Text.Analyzers/Core/IdentifiersShouldBeSpelledCorrectly.cs @@ -263,24 +263,24 @@ ImmutableArray ReadDictionaries() .Select(GetOrCreateDictionaryFromAdditionalText) .Where(x => x != null) .ToImmutableArray(); + } - CodeAnalysisDictionary GetOrCreateDictionaryFromAdditionalText(AdditionalText additionalText) - { - var isXml = additionalText.Path.EndsWith(".xml", StringComparison.OrdinalIgnoreCase); - var provider = isXml ? s_xmlDictionaryProvider : s_dicDictionaryProvider; - - var (dictionary, exception) = context.TryGetValue(additionalText.GetText(cancellationToken), provider, out var result) - ? result - : default; + CodeAnalysisDictionary GetOrCreateDictionaryFromAdditionalText(AdditionalText additionalText) + { + var isXml = additionalText.Path.EndsWith(".xml", StringComparison.OrdinalIgnoreCase); + var provider = isXml ? s_xmlDictionaryProvider : s_dicDictionaryProvider; - if (exception != null) - { - var diagnostic = Diagnostic.Create(FileParseRule, Location.None, additionalText.Path, exception.Message); - context.RegisterCompilationEndAction(x => x.ReportDiagnostic(diagnostic)); - } + var (dictionary, exception) = context.TryGetValue(additionalText.GetText(cancellationToken), provider, out var result) + ? result + : default; - return dictionary!; + if (exception != null) + { + var diagnostic = Diagnostic.Create(FileParseRule, Location.None, additionalText.Path, exception.Message); + context.RegisterCompilationEndAction(x => x.ReportDiagnostic(diagnostic)); } + + return dictionary!; } void AnalyzeVariable(OperationAnalysisContext operationContext)