Add readability configuration setting 'allowBuiltInTypeAliases'

Author: sharwell

StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1121UnitTests.cs

@@ -23,6 +23,16 @@ namespace StyleCop.Analyzers.Test.ReadabilityRules
     /// </summary>
     public class SA1121UnitTests : CodeFixVerifier
     {
+        private const string AllowBuiltInTypeAliasesSettings = @"
+{
+  ""settings"": {
+    ""readabilityRules"": {
+      ""allowBuiltInTypeAliases"": true
+    }
+  }
+}
+";
+
         private static readonly Tuple<string, string>[] ReferenceTypesData = new Tuple<string, string>[]
         {
             new Tuple<string, string>("object", nameof(Object)),
@@ -60,6 +70,8 @@ public class SA1121UnitTests : CodeFixVerifier
 
         private static readonly Tuple<string, string>[] AllTypesData = ReferenceTypesData.Concat(ValueTypesData).ToArray();
 
+        private string currentTestSettings;
+
         public static IEnumerable<object[]> ReferenceTypes
         {
             get
@@ -831,6 +843,53 @@ class Bar
             await this.VerifyCSharpFixAsync(oldSource, newSource, allowNewCompilerDiagnostics: true).ConfigureAwait(false);
         }
 
+        [Fact]
+        public async Task TestMissleadingUsingAllowAliasesAsync()
+        {
+            string oldSource = @"namespace Foo
+{
+  using Int32 = System.UInt32;
+  class Bar
+  {
+    Int32 value = 3;
+  }
+}
+";
+            string newSource = @"namespace Foo
+{
+  using Int32 = System.UInt32;
+  class Bar
+  {
+    uint value = 3;
+  }
+}
+";
+
+            DiagnosticResult expected = this.CSharpDiagnostic().WithLocation(6, 5);
+
+            this.currentTestSettings = AllowBuiltInTypeAliasesSettings;
+            await this.VerifyCSharpDiagnosticAsync(oldSource, expected, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpDiagnosticAsync(newSource, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpFixAsync(oldSource, newSource, allowNewCompilerDiagnostics: true).ConfigureAwait(false);
+        }
+
+        [Fact]
+        public async Task TestUsingNameChangeAllowAliasesAsync()
+        {
+            string testSource = @"namespace Foo
+{
+  using MyInt = System.UInt32;
+  class Bar
+  {
+    MyInt value = 3;
+  }
+}
+";
+
+            this.currentTestSettings = AllowBuiltInTypeAliasesSettings;
+            await this.VerifyCSharpDiagnosticAsync(testSource, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
         [Fact]
         public async Task TestWrongTypeAsync()
         {
@@ -935,6 +994,11 @@ public void Bar()
             await this.VerifyCSharpFixAsync(string.Format(testCode, fullName), string.Format(testCode, predefined), cancellationToken: CancellationToken.None).ConfigureAwait(false);
         }
 
+        protected override string GetSettings()
+        {
+            return this.currentTestSettings ?? base.GetSettings();
+        }
+
         protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
         {
             yield return new SA1121UseBuiltInTypeAlias();

StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1121UseBuiltInTypeAlias.cs

@@ -11,6 +11,7 @@ namespace StyleCop.Analyzers.ReadabilityRules
     using Microsoft.CodeAnalysis.CSharp.Syntax;
     using Microsoft.CodeAnalysis.Diagnostics;
     using StyleCop.Analyzers.Helpers;
+    using StyleCop.Analyzers.Settings.ObjectModel;
 
     /// <summary>
     /// The code uses one of the basic C# types, but does not use the built-in alias for the type.
@@ -163,7 +164,7 @@ public Analyzer(ConcurrentDictionary<SyntaxTree, bool> usingAliasCache)
                 this.usingAliasCache = usingAliasCache;
             }
 
-            public void HandleIdentifierNameSyntax(SyntaxNodeAnalysisContext context)
+            public void HandleIdentifierNameSyntax(SyntaxNodeAnalysisContext context, StyleCopSettings settings)
             {
                 IdentifierNameSyntax identifierNameSyntax = (IdentifierNameSyntax)context.Node;
                 if (identifierNameSyntax.IsVar)
@@ -207,7 +208,8 @@ public void HandleIdentifierNameSyntax(SyntaxNodeAnalysisContext context)
 
                 // 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))
+                if (settings.ReadabilityRules.AllowBuiltInTypeAliases
+                    || !identifierNameSyntax.SyntaxTree.ContainsUsingAlias(this.usingAliasCache))
                 {
                     switch (identifierNameSyntax.Identifier.ValueText)
                     {
@@ -234,6 +236,15 @@ public void HandleIdentifierNameSyntax(SyntaxNodeAnalysisContext context)
                 }
 
                 SemanticModel semanticModel = context.SemanticModel;
+
+                // We go straight to the symbol here. We don't need to check alias information because aliases will fall
+                // into one of two categories:
+                //
+                // 1. The alias to a built-in type matches the name of a built-in type (e.g. using Int32 = Int32;). In
+                //    this case, a diagnostic is reported even if allowBuiltInTypeAliases=true.
+                // 2. The alias to a built-in type is a different name. In this case, if allowBuiltInTypeAliases is true
+                //    then the above code would have already returned due to the renamed symbol not being in the set of
+                //    strings checked by the analyzer above.
                 INamedTypeSymbol symbol = semanticModel.GetSymbolInfo(identifierNameSyntax, context.CancellationToken).Symbol as INamedTypeSymbol;
 
                 switch (symbol?.SpecialType)

StyleCop.Analyzers/StyleCop.Analyzers/Settings/ObjectModel/ReadabilitySettings.cs

@@ -7,11 +7,17 @@ namespace StyleCop.Analyzers.Settings.ObjectModel
 
     internal class ReadabilitySettings
     {
+        /// <summary>
+        /// This is the backing field for the <see cref="AllowBuiltInTypeAliases"/> property.
+        /// </summary>
+        private bool allowBuiltInTypeAliases;
+
         /// <summary>
         /// Initializes a new instance of the <see cref="ReadabilitySettings"/> class.
         /// </summary>
         protected internal ReadabilitySettings()
         {
+            this.allowBuiltInTypeAliases = false;
         }
 
         /// <summary>
@@ -21,6 +27,21 @@ protected internal ReadabilitySettings()
         protected internal ReadabilitySettings(JsonObject readabilitySettingsObject)
             : this()
         {
+            foreach (var kvp in readabilitySettingsObject)
+            {
+                switch (kvp.Key)
+                {
+                case "allowBuiltInTypeAliases":
+                    this.allowBuiltInTypeAliases = kvp.ToBooleanValue();
+                    break;
+
+                default:
+                    break;
+                }
+            }
         }
+
+        public bool AllowBuiltInTypeAliases =>
+            this.allowBuiltInTypeAliases;
     }
 }

StyleCop.Analyzers/StyleCop.Analyzers/Settings/stylecop.schema.json

@@ -46,6 +46,11 @@
           "description": "Configuration for readability rules (SA1100-)",
           "additionalProperties": false,
           "properties": {
+            "allowBuiltInTypeAliases": {
+              "type": "boolean",
+              "description": "When true, diagnostics will not be reported for using aliases of built-in types.",
+              "default": false
+            }
           }
         },
         "orderingRules": {

documentation/Configuration.md

@@ -100,7 +100,21 @@ This section describes the features of readability rules which can be configured
 }
 ```
 
-> Currently there are no configurable settings for readability rules.
+### Aliases for Built-In Types
+
+| Property | Default Value | Minimum Version | Summary |
+| --- | --- | --- | --- |
+| `allowBuiltInTypeAliases` | **false** | 1.1.0-beta007 | Specifies whether aliases are allowed for built-in types. |
+
+By default, SA1121 reports a diagnostic for the use of named aliases for built-in types:
+
+```csharp
+using HRESULT = System.Int32;
+
+HRESULT hr = SomeNativeOperation(); // SA1121
+```
+
+The `allowBuiltInTypeAliases` configuration property can be set to `true` to allow cases like this while continuing to report diagnostics for direct references to the metadata type name, `Int32`.
 
 ## Ordering Rules