From d31a8c2a20a62dcfed6b71665bd0a1c5a069bb1d Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Wed, 21 Oct 2015 13:01:45 -0500 Subject: [PATCH 1/4] Access settings once per compilation instead of once per file --- .../DocumentationRules/FileHeaderAnalyzers.cs | 315 +++++++++--------- 1 file changed, 163 insertions(+), 152 deletions(-) diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/FileHeaderAnalyzers.cs b/StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/FileHeaderAnalyzers.cs index 69e8182fa..0f2c41390 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/FileHeaderAnalyzers.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/FileHeaderAnalyzers.cs @@ -179,220 +179,231 @@ private static void HandleCompilationStart(CompilationStartAnalysisContext conte // Disabling SA1633 will disable all other header related diagnostics. if (!compilation.IsAnalyzerSuppressed(SA1633Identifier)) { - context.RegisterSyntaxTreeActionHonorExclusions(ctx => HandleSyntaxTreeAction(ctx, compilation)); + Analyzer analyzer = new Analyzer(context.Options); + context.RegisterSyntaxTreeActionHonorExclusions(ctx => analyzer.HandleSyntaxTreeAction(ctx, compilation)); } } - private static void HandleSyntaxTreeAction(SyntaxTreeAnalysisContext context, Compilation compilation) + private sealed class Analyzer { - var root = context.Tree.GetRoot(context.CancellationToken); - var settings = context.GetStyleCopSettings(); + private readonly DocumentationSettings documentationSettings; - // don't process empty files - if (root.FullSpan.IsEmpty) + public Analyzer(AnalyzerOptions options) { - return; + StyleCopSettings settings = options.GetStyleCopSettings(); + this.documentationSettings = settings.DocumentationRules; } - if (settings.DocumentationRules.XmlHeader) + public void HandleSyntaxTreeAction(SyntaxTreeAnalysisContext context, Compilation compilation) { - var fileHeader = FileHeaderHelpers.ParseXmlFileHeader(root); - if (fileHeader.IsMissing) - { - context.ReportDiagnostic(Diagnostic.Create(SA1633DescriptorMissing, fileHeader.GetLocation(context.Tree))); - return; - } + var root = context.Tree.GetRoot(context.CancellationToken); - if (fileHeader.IsMalformed) + // don't process empty files + if (root.FullSpan.IsEmpty) { - context.ReportDiagnostic(Diagnostic.Create(SA1633DescriptorMalformed, fileHeader.GetLocation(context.Tree))); return; } - if (!compilation.IsAnalyzerSuppressed(SA1634Identifier)) - { - CheckCopyrightHeader(context, compilation, settings, fileHeader); - } - - if (!compilation.IsAnalyzerSuppressed(SA1639Identifier)) - { - CheckSummaryHeader(context, compilation, fileHeader); - } - } - else - { - var fileHeader = FileHeaderHelpers.ParseFileHeader(root); - if (fileHeader.IsMissing) - { - context.ReportDiagnostic(Diagnostic.Create(SA1633DescriptorMissing, fileHeader.GetLocation(context.Tree))); - return; - } - - if (!compilation.IsAnalyzerSuppressed(SA1635Identifier)) + if (this.documentationSettings.XmlHeader) { - if (string.IsNullOrWhiteSpace(fileHeader.CopyrightText)) + var fileHeader = FileHeaderHelpers.ParseXmlFileHeader(root); + if (fileHeader.IsMissing) { - context.ReportDiagnostic(Diagnostic.Create(SA1635Descriptor, fileHeader.GetLocation(context.Tree))); + context.ReportDiagnostic(Diagnostic.Create(SA1633DescriptorMissing, fileHeader.GetLocation(context.Tree))); return; } - if (compilation.IsAnalyzerSuppressed(SA1636Identifier)) + if (fileHeader.IsMalformed) { + context.ReportDiagnostic(Diagnostic.Create(SA1633DescriptorMalformed, fileHeader.GetLocation(context.Tree))); return; } - if (!CompareCopyrightText(fileHeader.CopyrightText, settings)) + if (!compilation.IsAnalyzerSuppressed(SA1634Identifier)) { - context.ReportDiagnostic(Diagnostic.Create(SA1636Descriptor, fileHeader.GetLocation(context.Tree))); + this.CheckCopyrightHeader(context, compilation, fileHeader); + } + + if (!compilation.IsAnalyzerSuppressed(SA1639Identifier)) + { + this.CheckSummaryHeader(context, compilation, fileHeader); + } + } + else + { + var fileHeader = FileHeaderHelpers.ParseFileHeader(root); + if (fileHeader.IsMissing) + { + context.ReportDiagnostic(Diagnostic.Create(SA1633DescriptorMissing, fileHeader.GetLocation(context.Tree))); return; } + + if (!compilation.IsAnalyzerSuppressed(SA1635Identifier)) + { + if (string.IsNullOrWhiteSpace(fileHeader.CopyrightText)) + { + context.ReportDiagnostic(Diagnostic.Create(SA1635Descriptor, fileHeader.GetLocation(context.Tree))); + return; + } + + if (compilation.IsAnalyzerSuppressed(SA1636Identifier)) + { + return; + } + + if (!this.CompareCopyrightText(fileHeader.CopyrightText)) + { + context.ReportDiagnostic(Diagnostic.Create(SA1636Descriptor, fileHeader.GetLocation(context.Tree))); + return; + } + } } } - } - private static void CheckCopyrightHeader(SyntaxTreeAnalysisContext context, Compilation compilation, StyleCopSettings settings, XmlFileHeader fileHeader) - { - var copyrightElement = fileHeader.GetElement("copyright"); - if (copyrightElement == null) + private void CheckCopyrightHeader(SyntaxTreeAnalysisContext context, Compilation compilation, XmlFileHeader fileHeader) { - context.ReportDiagnostic(Diagnostic.Create(SA1634Descriptor, fileHeader.GetLocation(context.Tree))); - return; - } + var copyrightElement = fileHeader.GetElement("copyright"); + if (copyrightElement == null) + { + context.ReportDiagnostic(Diagnostic.Create(SA1634Descriptor, fileHeader.GetLocation(context.Tree))); + return; + } - if (!compilation.IsAnalyzerSuppressed(SA1637Identifier)) - { - CheckFile(context, compilation, fileHeader, copyrightElement, settings); - } + if (!compilation.IsAnalyzerSuppressed(SA1637Identifier)) + { + this.CheckFile(context, compilation, fileHeader, copyrightElement); + } - if (!compilation.IsAnalyzerSuppressed(SA1640Identifier)) - { - CheckCompanyName(context, compilation, fileHeader, copyrightElement, settings); - } + if (!compilation.IsAnalyzerSuppressed(SA1640Identifier)) + { + this.CheckCompanyName(context, compilation, fileHeader, copyrightElement); + } - if (!compilation.IsAnalyzerSuppressed(SA1635Identifier)) - { - CheckCopyrightText(context, compilation, fileHeader, copyrightElement, settings); + if (!compilation.IsAnalyzerSuppressed(SA1635Identifier)) + { + this.CheckCopyrightText(context, compilation, fileHeader, copyrightElement); + } } - } - private static void CheckFile(SyntaxTreeAnalysisContext context, Compilation compilation, XmlFileHeader fileHeader, XElement copyrightElement, StyleCopSettings settings) - { - var fileAttribute = copyrightElement.Attribute("file"); - if (fileAttribute == null) + private void CheckFile(SyntaxTreeAnalysisContext context, Compilation compilation, XmlFileHeader fileHeader, XElement copyrightElement) { - var location = fileHeader.GetElementLocation(context.Tree, copyrightElement); - context.ReportDiagnostic(Diagnostic.Create(SA1637Descriptor, location)); - return; - } + var fileAttribute = copyrightElement.Attribute("file"); + if (fileAttribute == null) + { + var location = fileHeader.GetElementLocation(context.Tree, copyrightElement); + context.ReportDiagnostic(Diagnostic.Create(SA1637Descriptor, location)); + return; + } - if (compilation.IsAnalyzerSuppressed(SA1638Identifier)) - { - return; - } + if (compilation.IsAnalyzerSuppressed(SA1638Identifier)) + { + return; + } - var fileName = Path.GetFileName(context.Tree.FilePath); - if (string.CompareOrdinal(fileAttribute.Value, fileName) != 0) - { - var location = fileHeader.GetElementLocation(context.Tree, copyrightElement); - context.ReportDiagnostic(Diagnostic.Create(SA1638Descriptor, location)); + var fileName = Path.GetFileName(context.Tree.FilePath); + if (string.CompareOrdinal(fileAttribute.Value, fileName) != 0) + { + var location = fileHeader.GetElementLocation(context.Tree, copyrightElement); + context.ReportDiagnostic(Diagnostic.Create(SA1638Descriptor, location)); + } } - } - private static void CheckCopyrightText(SyntaxTreeAnalysisContext context, Compilation compilation, XmlFileHeader fileHeader, XElement copyrightElement, StyleCopSettings settings) - { - var copyrightText = copyrightElement.Value; - if (string.IsNullOrWhiteSpace(copyrightText)) + private void CheckCopyrightText(SyntaxTreeAnalysisContext context, Compilation compilation, XmlFileHeader fileHeader, XElement copyrightElement) { - var location = fileHeader.GetElementLocation(context.Tree, copyrightElement); - context.ReportDiagnostic(Diagnostic.Create(SA1635Descriptor, location)); - return; - } + var copyrightText = copyrightElement.Value; + if (string.IsNullOrWhiteSpace(copyrightText)) + { + var location = fileHeader.GetElementLocation(context.Tree, copyrightElement); + context.ReportDiagnostic(Diagnostic.Create(SA1635Descriptor, location)); + return; + } - if (compilation.IsAnalyzerSuppressed(SA1636Identifier)) - { - return; - } + if (compilation.IsAnalyzerSuppressed(SA1636Identifier)) + { + return; + } - var settingsCopyrightText = settings.DocumentationRules.CopyrightText; - if (string.Equals(settingsCopyrightText, DocumentationSettings.DefaultCopyrightText, StringComparison.OrdinalIgnoreCase)) - { - // The copyright text is meaningless until the company name is configured by the user. - return; - } + var settingsCopyrightText = this.documentationSettings.CopyrightText; + if (string.Equals(settingsCopyrightText, DocumentationSettings.DefaultCopyrightText, StringComparison.OrdinalIgnoreCase)) + { + // The copyright text is meaningless until the company name is configured by the user. + return; + } - // trim any leading / trailing new line or whitespace characters (those are a result of the XML formatting) - if (!CompareCopyrightText(copyrightText.Trim('\r', '\n', ' ', '\t'), settings)) - { - var location = fileHeader.GetElementLocation(context.Tree, copyrightElement); - context.ReportDiagnostic(Diagnostic.Create(SA1636Descriptor, location)); + // trim any leading / trailing new line or whitespace characters (those are a result of the XML formatting) + if (!this.CompareCopyrightText(copyrightText.Trim('\r', '\n', ' ', '\t'))) + { + var location = fileHeader.GetElementLocation(context.Tree, copyrightElement); + context.ReportDiagnostic(Diagnostic.Create(SA1636Descriptor, location)); + } } - } - private static void CheckCompanyName(SyntaxTreeAnalysisContext context, Compilation compilation, XmlFileHeader fileHeader, XElement copyrightElement, StyleCopSettings settings) - { - var companyName = copyrightElement.Attribute("company")?.Value; - if (string.IsNullOrWhiteSpace(companyName)) + private void CheckCompanyName(SyntaxTreeAnalysisContext context, Compilation compilation, XmlFileHeader fileHeader, XElement copyrightElement) { - var location = fileHeader.GetElementLocation(context.Tree, copyrightElement); - context.ReportDiagnostic(Diagnostic.Create(SA1640Descriptor, location)); - return; - } + var companyName = copyrightElement.Attribute("company")?.Value; + if (string.IsNullOrWhiteSpace(companyName)) + { + var location = fileHeader.GetElementLocation(context.Tree, copyrightElement); + context.ReportDiagnostic(Diagnostic.Create(SA1640Descriptor, location)); + return; + } - if (compilation.IsAnalyzerSuppressed(SA1641Identifier)) - { - return; - } + if (compilation.IsAnalyzerSuppressed(SA1641Identifier)) + { + return; + } - if (string.Equals(settings.DocumentationRules.CompanyName, DocumentationSettings.DefaultCompanyName, StringComparison.OrdinalIgnoreCase)) - { - // The company name is meaningless until configured by the user. - return; - } + if (string.Equals(this.documentationSettings.CompanyName, DocumentationSettings.DefaultCompanyName, StringComparison.OrdinalIgnoreCase)) + { + // The company name is meaningless until configured by the user. + return; + } - if (string.CompareOrdinal(companyName, settings.DocumentationRules.CompanyName) != 0) - { - var location = fileHeader.GetElementLocation(context.Tree, copyrightElement); - context.ReportDiagnostic(Diagnostic.Create(SA1641Descriptor, location)); + if (string.CompareOrdinal(companyName, this.documentationSettings.CompanyName) != 0) + { + var location = fileHeader.GetElementLocation(context.Tree, copyrightElement); + context.ReportDiagnostic(Diagnostic.Create(SA1641Descriptor, location)); + } } - } - private static void CheckSummaryHeader(SyntaxTreeAnalysisContext context, Compilation compilation, XmlFileHeader fileHeader) - { - var summaryElement = fileHeader.GetElement("summary"); - if (summaryElement == null) + private void CheckSummaryHeader(SyntaxTreeAnalysisContext context, Compilation compilation, XmlFileHeader fileHeader) { - context.ReportDiagnostic(Diagnostic.Create(SA1639Descriptor, fileHeader.GetLocation(context.Tree))); - return; - } + var summaryElement = fileHeader.GetElement("summary"); + if (summaryElement == null) + { + context.ReportDiagnostic(Diagnostic.Create(SA1639Descriptor, fileHeader.GetLocation(context.Tree))); + return; + } - if (string.IsNullOrWhiteSpace(summaryElement.Value)) - { - var location = fileHeader.GetElementLocation(context.Tree, summaryElement); - context.ReportDiagnostic(Diagnostic.Create(SA1639Descriptor, location)); + if (string.IsNullOrWhiteSpace(summaryElement.Value)) + { + var location = fileHeader.GetElementLocation(context.Tree, summaryElement); + context.ReportDiagnostic(Diagnostic.Create(SA1639Descriptor, location)); + } } - } - private static bool CompareCopyrightText(string copyrightText, StyleCopSettings settings) - { - // make sure that both \n and \r\n are accepted from the settings. - var reformattedCopyrightTextParts = settings.DocumentationRules.CopyrightText.Replace("\r\n", "\n").Split('\n'); - var fileHeaderCopyrightTextParts = copyrightText.Replace("\r\n", "\n").Split('\n'); - - if (reformattedCopyrightTextParts.Length != fileHeaderCopyrightTextParts.Length) + private bool CompareCopyrightText(string copyrightText) { - return false; - } + // make sure that both \n and \r\n are accepted from the settings. + var reformattedCopyrightTextParts = this.documentationSettings.CopyrightText.Replace("\r\n", "\n").Split('\n'); + var fileHeaderCopyrightTextParts = copyrightText.Replace("\r\n", "\n").Split('\n'); - // compare line by line, ignoring leading and trailing whitespace on each line. - for (var i = 0; i < reformattedCopyrightTextParts.Length; i++) - { - if (string.CompareOrdinal(reformattedCopyrightTextParts[i].Trim(), fileHeaderCopyrightTextParts[i].Trim()) != 0) + if (reformattedCopyrightTextParts.Length != fileHeaderCopyrightTextParts.Length) { return false; } - } - return true; + // compare line by line, ignoring leading and trailing whitespace on each line. + for (var i = 0; i < reformattedCopyrightTextParts.Length; i++) + { + if (string.CompareOrdinal(reformattedCopyrightTextParts[i].Trim(), fileHeaderCopyrightTextParts[i].Trim()) != 0) + { + return false; + } + } + + return true; + } } } } From b4ede523f5852dae5cba942603a3635bd784081e Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Thu, 22 Oct 2015 10:34:12 -0500 Subject: [PATCH 2/4] Store state as part of an analyzer instance instead of in a static field --- ...05FieldNamesMustNotUseHungarianNotation.cs | 93 ++++++++++--------- 1 file changed, 51 insertions(+), 42 deletions(-) diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/NamingRules/SA1305FieldNamesMustNotUseHungarianNotation.cs b/StyleCop.Analyzers/StyleCop.Analyzers/NamingRules/SA1305FieldNamesMustNotUseHungarianNotation.cs index 2053be064..6d918bb6f 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/NamingRules/SA1305FieldNamesMustNotUseHungarianNotation.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/NamingRules/SA1305FieldNamesMustNotUseHungarianNotation.cs @@ -65,8 +65,6 @@ internal class SA1305FieldNamesMustNotUseHungarianNotation : DiagnosticAnalyzer private static readonly Regex HungarianRegex = new Regex(@"^(?[a-z]{1,2})[A-Z]"); - private static NamingSettings namingSettings; - /// public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(Descriptor); @@ -79,63 +77,74 @@ public override void Initialize(AnalysisContext context) private static void HandleCompilationStart(CompilationStartAnalysisContext context) { - namingSettings = context.Options.GetStyleCopSettings().NamingRules; - context.RegisterSyntaxNodeActionHonorExclusions(HandleVariableDeclarationSyntax, SyntaxKind.VariableDeclaration); + Analyzer analyzer = new Analyzer(context.Options); + context.RegisterSyntaxNodeActionHonorExclusions(analyzer.HandleVariableDeclarationSyntax, SyntaxKind.VariableDeclaration); } - private static void HandleVariableDeclarationSyntax(SyntaxNodeAnalysisContext context) + private sealed class Analyzer { - var syntax = (VariableDeclarationSyntax)context.Node; + private readonly NamingSettings namingSettings; - if (syntax.Parent.IsKind(SyntaxKind.EventFieldDeclaration)) + public Analyzer(AnalyzerOptions options) { - return; + StyleCopSettings settings = options.GetStyleCopSettings(); + this.namingSettings = settings.NamingRules; } - if (NamedTypeHelpers.IsContainedInNativeMethodsClass(syntax)) + public void HandleVariableDeclarationSyntax(SyntaxNodeAnalysisContext context) { - return; - } + var syntax = (VariableDeclarationSyntax)context.Node; - var fieldDeclaration = syntax.Parent.IsKind(SyntaxKind.FieldDeclaration); - foreach (var variableDeclarator in syntax.Variables) - { - if (variableDeclarator == null) + if (syntax.Parent.IsKind(SyntaxKind.EventFieldDeclaration)) { - continue; + return; } - var identifier = variableDeclarator.Identifier; - if (identifier.IsMissing) + if (NamedTypeHelpers.IsContainedInNativeMethodsClass(syntax)) { - continue; + return; } - string name = identifier.ValueText; - if (string.IsNullOrEmpty(name)) + var fieldDeclaration = syntax.Parent.IsKind(SyntaxKind.FieldDeclaration); + foreach (var variableDeclarator in syntax.Variables) { - continue; + if (variableDeclarator == null) + { + continue; + } + + var identifier = variableDeclarator.Identifier; + if (identifier.IsMissing) + { + continue; + } + + string name = identifier.ValueText; + if (string.IsNullOrEmpty(name)) + { + continue; + } + + var match = HungarianRegex.Match(name); + if (!match.Success) + { + continue; + } + + var notationValue = match.Groups["notation"].Value; + if (this.namingSettings.AllowCommonHungarianPrefixes && CommonPrefixes.Contains(notationValue)) + { + continue; + } + + if (this.namingSettings.AllowedHungarianPrefixes.Contains(notationValue)) + { + continue; + } + + // Variable names must begin with lower-case letter + context.ReportDiagnostic(Diagnostic.Create(Descriptor, identifier.GetLocation(), fieldDeclaration ? "field" : "variable", name)); } - - var match = HungarianRegex.Match(name); - if (!match.Success) - { - continue; - } - - var notationValue = match.Groups["notation"].Value; - if (namingSettings.AllowCommonHungarianPrefixes && CommonPrefixes.Contains(notationValue)) - { - continue; - } - - if (namingSettings.AllowedHungarianPrefixes.Contains(notationValue)) - { - continue; - } - - // Variable names must begin with lower-case letter - context.ReportDiagnostic(Diagnostic.Create(Descriptor, identifier.GetLocation(), fieldDeclaration ? "field" : "variable", name)); } } } From c15c50e32681dda25583100a69cdfe2b93e244a7 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Thu, 22 Oct 2015 11:29:31 -0500 Subject: [PATCH 3/4] Update generated code detection to avoid ConditionalWeakTable --- .../StyleCop.Analyzers/AnalyzerExtensions.cs | 74 +++++++++-------- .../SA1401FieldsMustBePrivate.cs | 83 +++++++++++-------- ...tFieldNamesMustBeginWithUpperCaseLetter.cs | 77 ++++++++++------- 3 files changed, 135 insertions(+), 99 deletions(-) diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/AnalyzerExtensions.cs b/StyleCop.Analyzers/StyleCop.Analyzers/AnalyzerExtensions.cs index 49c4f93a2..dd71c00f8 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/AnalyzerExtensions.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/AnalyzerExtensions.cs @@ -5,7 +5,6 @@ namespace StyleCop.Analyzers { using System; using System.Collections.Concurrent; - using System.Runtime.CompilerServices; using System.Threading; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; @@ -22,8 +21,8 @@ internal static class AnalyzerExtensions /// This allows many analyzers that run on every token in the file to avoid checking /// the same state in the document repeatedly. /// - private static readonly ConditionalWeakTable> GeneratedHeaderCache - = new ConditionalWeakTable>(); + private static Tuple, ConcurrentDictionary> generatedHeaderCache + = Tuple.Create(new WeakReference(null), default(ConcurrentDictionary)); /// /// Register an action to be executed at completion of parsing of a code document. A syntax tree action reports @@ -35,7 +34,7 @@ private static readonly ConditionalWeakTable action) { Compilation compilation = context.Compilation; - ConcurrentDictionary cache = GeneratedHeaderCache.GetOrCreateValue(compilation); + ConcurrentDictionary cache = GetOrCreateGeneratedDocumentCache(compilation); context.RegisterSyntaxTreeAction( c => @@ -53,6 +52,40 @@ public static void RegisterSyntaxTreeActionHonorExclusions(this CompilationStart }); } + /// + /// Gets or creates a cache which can be used with methods to + /// efficiently determine whether or not a source file is considered generated. + /// + /// The compilation which the cache applies to. + /// A cache which tracks the syntax trees in a compilation which are considered generated. + public static ConcurrentDictionary GetOrCreateGeneratedDocumentCache(this Compilation compilation) + { + var headerCache = generatedHeaderCache; + + Compilation cachedCompilation; + if (!headerCache.Item1.TryGetTarget(out cachedCompilation) || cachedCompilation != compilation) + { + var replacementCache = Tuple.Create(new WeakReference(compilation), new ConcurrentDictionary()); + while (true) + { + var prior = Interlocked.CompareExchange(ref generatedHeaderCache, replacementCache, headerCache); + if (prior == headerCache) + { + headerCache = replacementCache; + break; + } + + headerCache = prior; + if (headerCache.Item1.TryGetTarget(out cachedCompilation) && cachedCompilation == compilation) + { + break; + } + } + } + + return headerCache.Item2; + } + /// /// Register an action to be executed at completion of semantic analysis of a with an /// appropriate kind. A syntax node action can report diagnostics about a , and can also @@ -70,7 +103,7 @@ public static void RegisterSyntaxNodeActionHonorExclusions(th where TLanguageKindEnum : struct { Compilation compilation = context.Compilation; - ConcurrentDictionary cache = GeneratedHeaderCache.GetOrCreateValue(compilation); + ConcurrentDictionary cache = GetOrCreateGeneratedDocumentCache(compilation); context.RegisterSyntaxNodeAction( c => @@ -88,36 +121,5 @@ public static void RegisterSyntaxNodeActionHonorExclusions(th }, syntaxKinds); } - - /// - /// Checks whether the given document is auto generated by a tool (based on filename or comment header). - /// - /// - /// The exact conditions used to identify generated code are subject to change in future releases. The - /// current algorithm uses the following checks. - /// Code is considered generated if it meets any of the following conditions. - /// - /// The code is contained in a file which starts with a comment containing the text - /// <auto-generated. - /// The code is contained in a file with a name matching certain patterns (case-insensitive): - /// - /// *.designer.cs - /// - /// - /// - /// - /// The syntax tree to examine. - /// The containing the specified . - /// The that the task will observe. - /// - /// if is located in generated code; otherwise, - /// . If is , this method returns - /// . - /// - public static bool IsGeneratedDocument(this SyntaxTree tree, Compilation compilation, CancellationToken cancellationToken) - { - ConcurrentDictionary cache = GeneratedHeaderCache.GetOrCreateValue(compilation); - return tree.IsGeneratedDocument(cache, cancellationToken); - } } } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/MaintainabilityRules/SA1401FieldsMustBePrivate.cs b/StyleCop.Analyzers/StyleCop.Analyzers/MaintainabilityRules/SA1401FieldsMustBePrivate.cs index 300445357..b464d35cc 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/MaintainabilityRules/SA1401FieldsMustBePrivate.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/MaintainabilityRules/SA1401FieldsMustBePrivate.cs @@ -3,6 +3,7 @@ namespace StyleCop.Analyzers.MaintainabilityRules { + using System.Collections.Concurrent; using System.Collections.Immutable; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; @@ -41,54 +42,70 @@ internal class SA1401FieldsMustBePrivate : DiagnosticAnalyzer /// public override void Initialize(AnalysisContext context) { - context.RegisterSymbolAction(AnalyzeField, SymbolKind.Field); + context.RegisterCompilationStartAction(HandleCompilationStart); } - private static void AnalyzeField(SymbolAnalysisContext symbolAnalysisContext) + private static void HandleCompilationStart(CompilationStartAnalysisContext context) { - var fieldDeclarationSyntax = (IFieldSymbol)symbolAnalysisContext.Symbol; - if (!IsFieldPrivate(fieldDeclarationSyntax) && - !IsStaticReadonly(fieldDeclarationSyntax) && - IsParentAClass(fieldDeclarationSyntax) && - !fieldDeclarationSyntax.IsConst) + Analyzer analyzer = new Analyzer(context.Compilation.GetOrCreateGeneratedDocumentCache()); + context.RegisterSymbolAction(analyzer.AnalyzeField, SymbolKind.Field); + } + + private sealed class Analyzer + { + private readonly ConcurrentDictionary generatedHeaderCache; + + public Analyzer(ConcurrentDictionary generatedHeaderCache) + { + this.generatedHeaderCache = generatedHeaderCache; + } + + public void AnalyzeField(SymbolAnalysisContext symbolAnalysisContext) { - foreach (var location in symbolAnalysisContext.Symbol.Locations) + var fieldDeclarationSyntax = (IFieldSymbol)symbolAnalysisContext.Symbol; + if (!IsFieldPrivate(fieldDeclarationSyntax) && + !IsStaticReadonly(fieldDeclarationSyntax) && + IsParentAClass(fieldDeclarationSyntax) && + !fieldDeclarationSyntax.IsConst) { - if (!location.IsInSource) + foreach (var location in symbolAnalysisContext.Symbol.Locations) { - // assume symbols not defined in a source document are "out of reach" - return; - } + if (!location.IsInSource) + { + // assume symbols not defined in a source document are "out of reach" + return; + } - if (location.SourceTree.IsGeneratedDocument(symbolAnalysisContext.Compilation, symbolAnalysisContext.CancellationToken)) - { - return; + if (location.SourceTree.IsGeneratedDocument(this.generatedHeaderCache, symbolAnalysisContext.CancellationToken)) + { + return; + } } - } - symbolAnalysisContext.ReportDiagnostic(Diagnostic.Create(Descriptor, fieldDeclarationSyntax.Locations[0])); + symbolAnalysisContext.ReportDiagnostic(Diagnostic.Create(Descriptor, fieldDeclarationSyntax.Locations[0])); + } } - } - private static bool IsFieldPrivate(IFieldSymbol fieldDeclarationSyntax) - { - return fieldDeclarationSyntax.DeclaredAccessibility == Accessibility.Private; - } - - private static bool IsStaticReadonly(IFieldSymbol fieldDeclarationSyntax) - { - return fieldDeclarationSyntax.IsStatic && fieldDeclarationSyntax.IsReadOnly; - } + private static bool IsFieldPrivate(IFieldSymbol fieldDeclarationSyntax) + { + return fieldDeclarationSyntax.DeclaredAccessibility == Accessibility.Private; + } - private static bool IsParentAClass(IFieldSymbol fieldDeclarationSyntax) - { - if (fieldDeclarationSyntax.ContainingSymbol != null && - fieldDeclarationSyntax.ContainingSymbol.Kind == SymbolKind.NamedType) + private static bool IsStaticReadonly(IFieldSymbol fieldDeclarationSyntax) { - return ((ITypeSymbol)fieldDeclarationSyntax.ContainingSymbol).TypeKind == TypeKind.Class; + return fieldDeclarationSyntax.IsStatic && fieldDeclarationSyntax.IsReadOnly; } - return false; + private static bool IsParentAClass(IFieldSymbol fieldDeclarationSyntax) + { + if (fieldDeclarationSyntax.ContainingSymbol != null && + fieldDeclarationSyntax.ContainingSymbol.Kind == SymbolKind.NamedType) + { + return ((ITypeSymbol)fieldDeclarationSyntax.ContainingSymbol).TypeKind == TypeKind.Class; + } + + return false; + } } } } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/NamingRules/SA1303ConstFieldNamesMustBeginWithUpperCaseLetter.cs b/StyleCop.Analyzers/StyleCop.Analyzers/NamingRules/SA1303ConstFieldNamesMustBeginWithUpperCaseLetter.cs index 06f65cc62..33d4cf967 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/NamingRules/SA1303ConstFieldNamesMustBeginWithUpperCaseLetter.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/NamingRules/SA1303ConstFieldNamesMustBeginWithUpperCaseLetter.cs @@ -3,6 +3,7 @@ namespace StyleCop.Analyzers.NamingRules { + using System.Collections.Concurrent; using System.Collections.Immutable; using System.Linq; using Microsoft.CodeAnalysis; @@ -45,50 +46,66 @@ internal class SA1303ConstFieldNamesMustBeginWithUpperCaseLetter : DiagnosticAna /// public override void Initialize(AnalysisContext context) { - context.RegisterSymbolAction(this.HandleFieldDeclaration, SymbolKind.Field); + context.RegisterCompilationStartAction(HandleCompilationStart); } - private void HandleFieldDeclaration(SymbolAnalysisContext context) + private static void HandleCompilationStart(CompilationStartAnalysisContext context) { - var symbol = context.Symbol as IFieldSymbol; + Analyzer analyzer = new Analyzer(context.Compilation.GetOrCreateGeneratedDocumentCache()); + context.RegisterSymbolAction(analyzer.HandleFieldDeclaration, SymbolKind.Field); + } - if (symbol == null || !symbol.IsConst) - { - return; - } + private sealed class Analyzer + { + private readonly ConcurrentDictionary generatedHeaderCache; - if (NamedTypeHelpers.IsContainedInNativeMethodsClass(symbol.ContainingType)) + public Analyzer(ConcurrentDictionary generatedHeaderCache) { - return; + this.generatedHeaderCache = generatedHeaderCache; } - /* This code uses char.IsLower(...) instead of !char.IsUpper(...) for all of the following reasons: - * 1. Fields starting with `_` should be reported as SA1309 instead of this diagnostic - * 2. Foreign languages may not have upper case variants for certain characters - * 3. This diagnostic appears targeted for "English" identifiers. - * - * See DotNetAnalyzers/StyleCopAnalyzers#369 for additional information: - * https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/369 - */ - if (!string.IsNullOrEmpty(symbol.Name) && - char.IsLower(symbol.Name[0]) && - symbol.Locations.Any()) + public void HandleFieldDeclaration(SymbolAnalysisContext context) { - foreach (var location in context.Symbol.Locations) + var symbol = context.Symbol as IFieldSymbol; + + if (symbol == null || !symbol.IsConst) { - if (!location.IsInSource) - { - // assume symbols not defined in a source document are "out of reach" - return; - } + return; + } - if (location.SourceTree.IsGeneratedDocument(context.Compilation, context.CancellationToken)) + if (NamedTypeHelpers.IsContainedInNativeMethodsClass(symbol.ContainingType)) + { + return; + } + + /* This code uses char.IsLower(...) instead of !char.IsUpper(...) for all of the following reasons: + * 1. Fields starting with `_` should be reported as SA1309 instead of this diagnostic + * 2. Foreign languages may not have upper case variants for certain characters + * 3. This diagnostic appears targeted for "English" identifiers. + * + * See DotNetAnalyzers/StyleCopAnalyzers#369 for additional information: + * https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/369 + */ + if (!string.IsNullOrEmpty(symbol.Name) && + char.IsLower(symbol.Name[0]) && + symbol.Locations.Any()) + { + foreach (var location in context.Symbol.Locations) { - return; + if (!location.IsInSource) + { + // assume symbols not defined in a source document are "out of reach" + return; + } + + if (location.SourceTree.IsGeneratedDocument(this.generatedHeaderCache, context.CancellationToken)) + { + return; + } } - } - context.ReportDiagnostic(Diagnostic.Create(Descriptor, symbol.Locations[0])); + context.ReportDiagnostic(Diagnostic.Create(Descriptor, symbol.Locations[0])); + } } } } From 919d61ac2b7e2dc537d576ec844fa44528a4c52b Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Thu, 22 Oct 2015 11:30:30 -0500 Subject: [PATCH 4/4] Update using alias detection to avoid the use of ConditionalWeakTable --- .../Helpers/SyntaxTreeHelpers.cs | 58 ++-- ...nalysisSuppressionMustHaveJustification.cs | 12 +- .../SA1121UseBuiltInTypeAlias.cs | 250 +++++++++--------- 3 files changed, 184 insertions(+), 136 deletions(-) diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/Helpers/SyntaxTreeHelpers.cs b/StyleCop.Analyzers/StyleCop.Analyzers/Helpers/SyntaxTreeHelpers.cs index 692e36433..c9e509682 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/Helpers/SyntaxTreeHelpers.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/Helpers/SyntaxTreeHelpers.cs @@ -3,8 +3,10 @@ namespace StyleCop.Analyzers.Helpers { + using System; + using System.Collections.Concurrent; using System.Linq; - using System.Runtime.CompilerServices; + using System.Threading; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -18,27 +20,53 @@ internal static class SyntaxTreeHelpers /// This allows many analyzers that run on every token in the file to avoid checking /// the same state in the document repeatedly. /// - private static readonly ConditionalWeakTable> UsingAliasPresentCheck - = new ConditionalWeakTable>(); + private static Tuple, ConcurrentDictionary> usingAliasCache + = Tuple.Create(new WeakReference(null), default(ConcurrentDictionary)); - internal static bool ContainsUsingAlias(this SyntaxTree tree) + public static ConcurrentDictionary GetOrCreateUsingAliasCache(this Compilation compilation) { - StrongBox cachedResult = UsingAliasPresentCheck.GetOrCreateValue(tree); - if (cachedResult.Value.HasValue) + var cache = usingAliasCache; + + Compilation cachedCompilation; + if (!cache.Item1.TryGetTarget(out cachedCompilation) || cachedCompilation != compilation) { - return cachedResult.Value.Value; + var replacementCache = Tuple.Create(new WeakReference(compilation), new ConcurrentDictionary()); + while (true) + { + var prior = Interlocked.CompareExchange(ref usingAliasCache, replacementCache, cache); + if (prior == cache) + { + cache = replacementCache; + break; + } + + cache = prior; + if (cache.Item1.TryGetTarget(out cachedCompilation) && cachedCompilation == compilation) + { + break; + } + } } - bool hasUsingAlias = ContainsUsingAliasNoCache(tree); + return cache.Item2; + } - // Update the strongbox's value with our computed result. - // This doesn't change the strongbox reference, and its presence in the - // ConditionalWeakTable is already assured, so we're updating in-place. - // In the event of a race condition with another thread that set the value, - // we'll just be re-setting it to the same value. - cachedResult.Value = hasUsingAlias; + internal static bool ContainsUsingAlias(this SyntaxTree tree, ConcurrentDictionary cache) + { + if (tree == null) + { + return false; + } + + bool result; + if (cache.TryGetValue(tree, out result)) + { + return result; + } - return hasUsingAlias; + bool generated = ContainsUsingAliasNoCache(tree); + cache.TryAdd(tree, generated); + return generated; } private static bool ContainsUsingAliasNoCache(SyntaxTree tree) diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/MaintainabilityRules/SA1404CodeAnalysisSuppressionMustHaveJustification.cs b/StyleCop.Analyzers/StyleCop.Analyzers/MaintainabilityRules/SA1404CodeAnalysisSuppressionMustHaveJustification.cs index 9aa3f0c84..24a271583 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/MaintainabilityRules/SA1404CodeAnalysisSuppressionMustHaveJustification.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/MaintainabilityRules/SA1404CodeAnalysisSuppressionMustHaveJustification.cs @@ -3,6 +3,7 @@ namespace StyleCop.Analyzers.MaintainabilityRules { + using System.Collections.Concurrent; using System.Collections.Immutable; using System.Diagnostics.CodeAnalysis; using Helpers; @@ -60,7 +61,7 @@ public override void Initialize(AnalysisContext context) private static void HandleCompilationStart(CompilationStartAnalysisContext context) { - AnalyzerInstance instance = new AnalyzerInstance(); + AnalyzerInstance instance = new AnalyzerInstance(context.Compilation.GetOrCreateUsingAliasCache()); context.RegisterSyntaxNodeActionHonorExclusions(instance.HandleAttributeNode, SyntaxKind.Attribute); } @@ -69,18 +70,25 @@ private static void HandleCompilationStart(CompilationStartAnalysisContext conte /// private sealed class AnalyzerInstance { + private readonly ConcurrentDictionary usingAliasCache; + /// /// A lazily-initialized reference to within the context of a /// particular . /// private INamedTypeSymbol suppressMessageAttribute; + public AnalyzerInstance(ConcurrentDictionary usingAliasCache) + { + this.usingAliasCache = usingAliasCache; + } + public void HandleAttributeNode(SyntaxNodeAnalysisContext context) { var attribute = (AttributeSyntax)context.Node; // Return fast if the name doesn't match and the file doesn't contain any using alias directives - if (!attribute.SyntaxTree.ContainsUsingAlias()) + if (!attribute.SyntaxTree.ContainsUsingAlias(this.usingAliasCache)) { SimpleNameSyntax simpleNameSyntax = attribute.Name as SimpleNameSyntax; if (simpleNameSyntax == null) diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1121UseBuiltInTypeAlias.cs b/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1121UseBuiltInTypeAlias.cs index 405363ca1..2f5a7f699 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1121UseBuiltInTypeAlias.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1121UseBuiltInTypeAlias.cs @@ -4,6 +4,7 @@ namespace StyleCop.Analyzers.ReadabilityRules { using System; + using System.Collections.Concurrent; using System.Collections.Immutable; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; @@ -144,153 +145,164 @@ public override void Initialize(AnalysisContext context) private static void HandleCompilationStart(CompilationStartAnalysisContext context) { - context.RegisterSyntaxNodeActionHonorExclusions(HandleIdentifierNameSyntax, SyntaxKind.IdentifierName); + Analyzer analyzer = new Analyzer(context.Compilation.GetOrCreateUsingAliasCache()); + context.RegisterSyntaxNodeActionHonorExclusions(analyzer.HandleIdentifierNameSyntax, SyntaxKind.IdentifierName); } - private static void HandleIdentifierNameSyntax(SyntaxNodeAnalysisContext context) + private sealed class Analyzer { - IdentifierNameSyntax identifierNameSyntax = (IdentifierNameSyntax)context.Node; - if (identifierNameSyntax.IsVar) - { - return; - } + private readonly ConcurrentDictionary usingAliasCache; - if (identifierNameSyntax.Identifier.IsMissing) + public Analyzer(ConcurrentDictionary usingAliasCache) { - return; + this.usingAliasCache = usingAliasCache; } - switch (identifierNameSyntax.Identifier.ValueText) + public void HandleIdentifierNameSyntax(SyntaxNodeAnalysisContext context) { - case "bool": - case "byte": - case "char": - case "decimal": - case "double": - case "short": - case "int": - case "long": - case "object": - case "sbyte": - case "float": - case "string": - case "ushort": - case "uint": - case "ulong": - return; - - default: - break; - } + IdentifierNameSyntax identifierNameSyntax = (IdentifierNameSyntax)context.Node; + if (identifierNameSyntax.IsVar) + { + return; + } - if (identifierNameSyntax.FirstAncestorOrSelf() != null - && identifierNameSyntax.FirstAncestorOrSelf() == null) - { - return; - } + if (identifierNameSyntax.Identifier.IsMissing) + { + return; + } - // Most source files will not have any using alias directives. Then we don't have to use semantics - // if the identifier name doesn't match the name of a special type - if (!identifierNameSyntax.SyntaxTree.ContainsUsingAlias()) - { switch (identifierNameSyntax.Identifier.ValueText) { - case nameof(Boolean): - case nameof(Byte): - case nameof(Char): - case nameof(Decimal): - case nameof(Double): - case nameof(Int16): - case nameof(Int32): - case nameof(Int64): - case nameof(Object): - case nameof(SByte): - case nameof(Single): - case nameof(String): - case nameof(UInt16): - case nameof(UInt32): - case nameof(UInt64): - break; + case "bool": + case "byte": + case "char": + case "decimal": + case "double": + case "short": + case "int": + case "long": + case "object": + case "sbyte": + case "float": + case "string": + case "ushort": + case "uint": + case "ulong": + return; default: + break; + } + + if (identifierNameSyntax.FirstAncestorOrSelf() != null + && identifierNameSyntax.FirstAncestorOrSelf() == null) + { return; } - } - SemanticModel semanticModel = context.SemanticModel; - INamedTypeSymbol symbol = semanticModel.GetSymbolInfo(identifierNameSyntax, context.CancellationToken).Symbol as INamedTypeSymbol; + // Most source files will not have any using alias directives. Then we don't have to use semantics + // if the identifier name doesn't match the name of a special type + if (!identifierNameSyntax.SyntaxTree.ContainsUsingAlias(this.usingAliasCache)) + { + switch (identifierNameSyntax.Identifier.ValueText) + { + case nameof(Boolean): + case nameof(Byte): + case nameof(Char): + case nameof(Decimal): + case nameof(Double): + case nameof(Int16): + case nameof(Int32): + case nameof(Int64): + case nameof(Object): + case nameof(SByte): + case nameof(Single): + case nameof(String): + case nameof(UInt16): + case nameof(UInt32): + case nameof(UInt64): + break; - switch (symbol?.SpecialType) - { - case SpecialType.System_Boolean: - case SpecialType.System_Byte: - case SpecialType.System_Char: - case SpecialType.System_Decimal: - case SpecialType.System_Double: - case SpecialType.System_Int16: - case SpecialType.System_Int32: - case SpecialType.System_Int64: - case SpecialType.System_Object: - case SpecialType.System_SByte: - case SpecialType.System_Single: - case SpecialType.System_String: - case SpecialType.System_UInt16: - case SpecialType.System_UInt32: - case SpecialType.System_UInt64: - break; + default: + return; + } + } - default: - return; - } + SemanticModel semanticModel = context.SemanticModel; + INamedTypeSymbol symbol = semanticModel.GetSymbolInfo(identifierNameSyntax, context.CancellationToken).Symbol as INamedTypeSymbol; - SyntaxNode locationNode = identifierNameSyntax; - if (identifierNameSyntax.Parent is QualifiedNameSyntax) - { - locationNode = identifierNameSyntax.Parent; - } - else if ((identifierNameSyntax.Parent as MemberAccessExpressionSyntax)?.Name == identifierNameSyntax) - { - // this "weird" syntax appears for qualified references within a nameof expression - locationNode = identifierNameSyntax.Parent; - } - else if (identifierNameSyntax.Parent is NameMemberCrefSyntax && identifierNameSyntax.Parent.Parent is QualifiedCrefSyntax) - { - locationNode = identifierNameSyntax.Parent.Parent; - } + switch (symbol?.SpecialType) + { + case SpecialType.System_Boolean: + case SpecialType.System_Byte: + case SpecialType.System_Char: + case SpecialType.System_Decimal: + case SpecialType.System_Double: + case SpecialType.System_Int16: + case SpecialType.System_Int32: + case SpecialType.System_Int64: + case SpecialType.System_Object: + case SpecialType.System_SByte: + case SpecialType.System_Single: + case SpecialType.System_String: + case SpecialType.System_UInt16: + case SpecialType.System_UInt32: + case SpecialType.System_UInt64: + break; - // Allow nameof - if (IsNameInNameOfExpression(identifierNameSyntax)) - { - return; - } + default: + return; + } - // Use built-in type alias - context.ReportDiagnostic(Diagnostic.Create(Descriptor, locationNode.GetLocation())); - } + SyntaxNode locationNode = identifierNameSyntax; + if (identifierNameSyntax.Parent is QualifiedNameSyntax) + { + locationNode = identifierNameSyntax.Parent; + } + else if ((identifierNameSyntax.Parent as MemberAccessExpressionSyntax)?.Name == identifierNameSyntax) + { + // this "weird" syntax appears for qualified references within a nameof expression + locationNode = identifierNameSyntax.Parent; + } + else if (identifierNameSyntax.Parent is NameMemberCrefSyntax && identifierNameSyntax.Parent.Parent is QualifiedCrefSyntax) + { + locationNode = identifierNameSyntax.Parent.Parent; + } - private static bool IsNameInNameOfExpression(IdentifierNameSyntax identifierNameSyntax) - { - // The only time a type name can appear as an argument is for the invocation expression created for the - // nameof keyword. This assumption is the foundation of the following simple analysis algorithm. + // Allow nameof + if (IsNameInNameOfExpression(identifierNameSyntax)) + { + return; + } - // This covers the case nameof(Int32) - if (identifierNameSyntax.Parent is ArgumentSyntax) - { - return true; + // Use built-in type alias + context.ReportDiagnostic(Diagnostic.Create(Descriptor, locationNode.GetLocation())); } - MemberAccessExpressionSyntax simpleMemberAccess = identifierNameSyntax.Parent as MemberAccessExpressionSyntax; - - // This covers the case nameof(System.Int32) - if (simpleMemberAccess != null) + private static bool IsNameInNameOfExpression(IdentifierNameSyntax identifierNameSyntax) { - // This final check ensures that we don't consider nameof(System.Int32.ToString) the same as - // nameof(System.Int32) - return identifierNameSyntax.Parent.Parent.IsKind(SyntaxKind.Argument) - && simpleMemberAccess.Name == identifierNameSyntax; - } + // The only time a type name can appear as an argument is for the invocation expression created for the + // nameof keyword. This assumption is the foundation of the following simple analysis algorithm. + + // This covers the case nameof(Int32) + if (identifierNameSyntax.Parent is ArgumentSyntax) + { + return true; + } - return false; + MemberAccessExpressionSyntax simpleMemberAccess = identifierNameSyntax.Parent as MemberAccessExpressionSyntax; + + // This covers the case nameof(System.Int32) + if (simpleMemberAccess != null) + { + // This final check ensures that we don't consider nameof(System.Int32.ToString) the same as + // nameof(System.Int32) + return identifierNameSyntax.Parent.Parent.IsKind(SyntaxKind.Argument) + && simpleMemberAccess.Name == identifierNameSyntax; + } + + return false; + } } } }