Add option 'roslynator_unity_code_analysis.enabled' (#1245)

josefpihrt · web-flow · commit e260e3df8047 · 2023-11-10T23:33:00.000+01:00
diff --git a/ChangeLog.md b/ChangeLog.md
@@ -17,6 +17,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fix refactoring [Inline method](https://josefpihrt.github.io/docs/roslynator/refactorings/RR0062) ([PR](https://github.com/dotnet/roslynator/pull/1234))
 - [CLI] Fix globbing ([PR](https://github.com/dotnet/roslynator/pull/1238))
 - [CLI] Remove assembly resolving ([PR](https://github.com/dotnet/roslynator/pull/1237))
+- Detect false positive from Unity code ([RCS1169](https://josefpihrt.github.io/docs/roslynator/analyzers/RCS1169)) ([PR](https://github.com/dotnet/roslynator/pull/1245))
+  - Introduce config option `roslynator_unity_code_analysis.enabled = true|false`
+  - Make option `roslynator_suppress_unity_script_methods` obsolete
 
 ## [4.6.1] - 2023-10-23
 
diff --git a/src/Analyzers.xml b/src/Analyzers.xml
@@ -5178,6 +5178,9 @@ abstract class Foo : IFoo
     <Title>Make field read-only.</Title>
     <DefaultSeverity>Info</DefaultSeverity>
     <IsEnabledByDefault>true</IsEnabledByDefault>
+    <ConfigOptions>
+      <Option Key="unity_code_analysis.enabled" />
+    </ConfigOptions>
     <Samples>
       <Sample>
         <Before><![CDATA[public class Foo
@@ -6117,6 +6120,7 @@ x = "";]]></Before>
     <IsEnabledByDefault>true</IsEnabledByDefault>
     <SupportsFadeOut>true</SupportsFadeOut>
     <ConfigOptions>
+      <Option Key="unity_code_analysis.enabled" />
       <Option Key="suppress_unity_script_methods" />
     </ConfigOptions>
     <Options>
diff --git a/src/Analyzers/CSharp/Analysis/AnalyzerOptionIsObsoleteAnalyzer.cs b/src/Analyzers/CSharp/Analysis/AnalyzerOptionIsObsoleteAnalyzer.cs
@@ -52,7 +52,9 @@ public override void Initialize(AnalysisContext context)
                 Validate(ref context, compilationOptions, options, Flags.RemoveEmptyLineBetweenClosingBraceAndSwitchSection, ref flags, DiagnosticRules.RemoveUnnecessaryBlankLine, ConfigOptions.BlankLineBetweenClosingBraceAndSwitchSection, LegacyConfigOptions.RemoveEmptyLineBetweenClosingBraceAndSwitchSection, "false");
                 Validate(ref context, compilationOptions, options, Flags.RemoveParenthesesFromConditionOfConditionalExpressionWhenExpressionIsSingleToken, ref flags, DiagnosticRules.AddOrRemoveParenthesesFromConditionInConditionalOperator, ConfigOptions.ConditionalOperatorConditionParenthesesStyle, LegacyConfigOptions.RemoveParenthesesFromConditionOfConditionalExpressionWhenExpressionIsSingleToken, ConfigOptionValues.ConditionalOperatorConditionParenthesesStyle_OmitWhenConditionIsSingleToken);
                 Validate(ref context, compilationOptions, options, Flags.RemoveParenthesesWhenCreatingNewObject, ref flags, DiagnosticRules.UseImplicitOrExplicitObjectCreation, ConfigOptions.ObjectCreationParenthesesStyle, LegacyConfigOptions.RemoveParenthesesWhenCreatingNewObject, ConfigOptionValues.ObjectCreationParenthesesStyle_Omit);
+#pragma warning disable CS0618 // Type or member is obsolete
                 Validate(ref context, compilationOptions, options, Flags.SuppressUnityScriptMethods, ref flags, DiagnosticRules.RemoveUnusedMemberDeclaration, ConfigOptions.SuppressUnityScriptMethods, LegacyConfigOptions.SuppressUnityScriptMethods, "true");
+#pragma warning restore CS0618 // Type or member is obsolete
                 Validate(ref context, compilationOptions, options, Flags.UseComparisonInsteadPatternMatchingToCheckForNull, ref flags, DiagnosticRules.NormalizeNullCheck, ConfigOptions.NullCheckStyle, LegacyConfigOptions.UseComparisonInsteadPatternMatchingToCheckForNull, ConfigOptionValues.NullCheckStyle_EqualityOperator);
                 Validate(ref context, compilationOptions, options, Flags.UseImplicitlyTypedArray, ref flags, DiagnosticRules.UseExplicitlyOrImplicitlyTypedArray, ConfigOptions.ArrayCreationTypeStyle, LegacyConfigOptions.UseImplicitlyTypedArray, ConfigOptionValues.ArrayCreationTypeStyle_Implicit);
                 Validate(ref context, compilationOptions, options, Flags.UseImplicitlyTypedArrayWhenTypeIsObvious, ref flags, DiagnosticRules.UseExplicitlyOrImplicitlyTypedArray, ConfigOptions.ArrayCreationTypeStyle, LegacyConfigOptions.UseImplicitlyTypedArrayWhenTypeIsObvious, ConfigOptionValues.ArrayCreationTypeStyle_ImplicitWhenTypeIsObvious);
diff --git a/src/Analyzers/CSharp/Analysis/EnumSymbolAnalyzer.cs b/src/Analyzers/CSharp/Analysis/EnumSymbolAnalyzer.cs
@@ -198,7 +198,9 @@ private static void AnalyzeNamedType(SymbolAnalysisContext context)
 
                     if (CSharpUtility.IsSymbolObsolete(symbolInfo1.Symbol)
                         || CSharpUtility.IsSymbolObsolete(symbolInfo2.Symbol))
+                    {
                         continue;
+                    }
 
                     var enumMember1 = (EnumMemberDeclarationSyntax)symbolInfo1.Symbol.GetSyntax(context.CancellationToken);
 
diff --git a/src/Analyzers/CSharp/Analysis/MakeMemberReadOnly/MakeMemberReadOnlyAnalyzer.cs b/src/Analyzers/CSharp/Analysis/MakeMemberReadOnly/MakeMemberReadOnlyAnalyzer.cs
@@ -18,6 +18,7 @@ public sealed class MakeMemberReadOnlyAnalyzer : BaseDiagnosticAnalyzer
     private static readonly MetadataName Microsoft_AspNetCore_Components_CascadingParameterAttribute = MetadataName.Parse("Microsoft.AspNetCore.Components.CascadingParameterAttribute");
     private static readonly MetadataName Microsoft_AspNetCore_Components_InjectAttribute = MetadataName.Parse("Microsoft.AspNetCore.Components.InjectAttribute");
     private static readonly MetadataName Newtonsoft_Json_JsonPropertyAttribute = MetadataName.Parse("Newtonsoft.Json.JsonPropertyAttribute");
+    private static readonly MetadataName UnityEngine_SerializeFieldAttribute = MetadataName.Parse("UnityEngine.SerializeFieldAttribute");
 
     private static ImmutableArray<DiagnosticDescriptor> _supportedDiagnostics;
 
@@ -135,7 +136,9 @@ private static void AnalyzeTypeDeclaration(
                                 && fieldSymbol.DeclaredAccessibility == Accessibility.Private
                                 && !fieldSymbol.IsReadOnly
                                 && !fieldSymbol.IsVolatile
-                                && ValidateType(fieldSymbol.Type))
+                                && ValidateType(fieldSymbol.Type)
+                                && (context.IsUnityCodeAnalysisEnabled() != true
+                                    || !fieldSymbol.HasAttribute(UnityEngine_SerializeFieldAttribute)))
                             {
                                 symbols[fieldSymbol.Name] = (declarator, fieldSymbol);
                             }
diff --git a/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberAnalyzer.cs b/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberAnalyzer.cs
@@ -159,7 +159,7 @@ private static void AnalyzeTypeDeclaration(SyntaxNodeAnalysisContext context)
                             break;
 
                         if (declaration.ReturnsVoid()
-                            && context.GetSuppressUnityScriptMethods() == true)
+                            && context.IsUnityCodeAnalysisEnabled() == true)
                         {
                             if (canContainUnityScriptMethods is null)
                             {
diff --git a/src/Common/CSharp/Extensions/CodeStyleExtensions.cs b/src/Common/CSharp/Extensions/CodeStyleExtensions.cs
@@ -565,12 +565,15 @@ public static BlankLineStyle GetBlankLineAfterFileScopedNamespaceDeclaration(thi
         return BlankLineStyle.None;
     }
 
-    public static bool? GetSuppressUnityScriptMethods(this SyntaxNodeAnalysisContext context)
+    public static bool? IsUnityCodeAnalysisEnabled(this SyntaxNodeAnalysisContext context)
     {
-        if (ConfigOptions.TryGetValueAsBool(context.GetConfigOptions(), ConfigOptions.SuppressUnityScriptMethods, out bool value))
-        {
+        if (ConfigOptions.TryGetValueAsBool(context.GetConfigOptions(), ConfigOptions.UnityCodeAnalysisEnabled, out bool value))
             return value;
-        }
+
+#pragma warning disable CS0618 // Type or member is obsolete
+        if (ConfigOptions.TryGetValueAsBool(context.GetConfigOptions(), ConfigOptions.SuppressUnityScriptMethods, out value))
+            return value;
+#pragma warning restore CS0618 // Type or member is obsolete
 
         if (context.TryGetOptionAsBool(LegacyConfigOptions.SuppressUnityScriptMethods, out value))
             return value;
diff --git a/src/Common/ConfigOptionKeys.Generated.cs b/src/Common/ConfigOptionKeys.Generated.cs
@@ -37,6 +37,7 @@ internal static partial class ConfigOptionKeys
         public const string SuppressUnityScriptMethods                        = "roslynator_suppress_unity_script_methods";
         public const string TabLength                                         = "roslynator_tab_length";
         public const string TrailingCommaStyle                                = "roslynator_trailing_comma_style";
+        public const string UnityCodeAnalysisEnabled                          = "roslynator_unity_code_analysis.enabled";
         public const string UseAnonymousFunctionOrMethodGroup                 = "roslynator_use_anonymous_function_or_method_group";
         public const string UseBlockBodyWhenDeclarationSpansOverMultipleLines = "roslynator_use_block_body_when_declaration_spans_over_multiple_lines";
         public const string UseBlockBodyWhenExpressionSpansOverMultipleLines  = "roslynator_use_block_body_when_expression_spans_over_multiple_lines";
diff --git a/src/Common/ConfigOptions.Generated.cs b/src/Common/ConfigOptions.Generated.cs
@@ -2,6 +2,7 @@
 
 // <auto-generated>
 
+using System;
 using System.Collections.Generic;
 
 namespace Roslynator
@@ -176,11 +177,12 @@ public static partial class ConfigOptions
             defaultValuePlaceholder: "true|false", 
             description:             "Prefix field identifier with underscore");
 
+        [Obsolete("", error: false)]
         public static readonly ConfigOptionDescriptor SuppressUnityScriptMethods = new(
             key:                     ConfigOptionKeys.SuppressUnityScriptMethods, 
             defaultValue:            null, 
             defaultValuePlaceholder: "true|false", 
-            description:             "Suppress Unity script methods");
+            description:             "[deprecated] This option is obsolete, use option 'roslynator_unity_code_analysis.enabled' instead.");
 
         public static readonly ConfigOptionDescriptor TabLength = new(
             key:                     ConfigOptionKeys.TabLength, 
@@ -194,6 +196,12 @@ public static partial class ConfigOptions
             defaultValuePlaceholder: "include|omit|omit_when_single_line", 
             description:             "Include/omit trailing comma in initializer or enum");
 
+        public static readonly ConfigOptionDescriptor UnityCodeAnalysisEnabled = new(
+            key:                     ConfigOptionKeys.UnityCodeAnalysisEnabled, 
+            defaultValue:            null, 
+            defaultValuePlaceholder: "true|false", 
+            description:             "Enable code analysis to detect Unity-specific code");
+
         public static readonly ConfigOptionDescriptor UseAnonymousFunctionOrMethodGroup = new(
             key:                     ConfigOptionKeys.UseAnonymousFunctionOrMethodGroup, 
             defaultValue:            null, 
diff --git a/src/ConfigOptions.xml b/src/ConfigOptions.xml
@@ -170,9 +170,14 @@
     <ValuePlaceholder>true|false</ValuePlaceholder>
     <Description>Prefix field identifier with underscore</Description>
   </Option>
-  <Option Id="SuppressUnityScriptMethods">
+  <Option Id="SuppressUnityScriptMethods" IsObsolete="true">
     <ValuePlaceholder>true|false</ValuePlaceholder>
-    <Description>Suppress Unity script methods</Description>
+    <Description>[deprecated] This option is obsolete, use option 'roslynator_unity_code_analysis.enabled' instead.</Description>
+  </Option>
+  <Option Id="UnityCodeAnalysisEnabled">
+    <Key>unity_code_analysis.enabled</Key>
+    <ValuePlaceholder>true|false</ValuePlaceholder> 
+    <Description>Enable code analysis to detect Unity-specific code</Description>
   </Option>
   <Option Id="UseAnonymousFunctionOrMethodGroup">
     <Description>Use anonymous function or method group</Description>
diff --git a/src/Tests/Analyzers.Tests/RCS1169MakeFieldReadOnlyTests.cs b/src/Tests/Analyzers.Tests/RCS1169MakeFieldReadOnlyTests.cs
@@ -377,4 +377,26 @@ class C
 }
 ");
     }
+
+    [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.MakeFieldReadOnly)]
+    public async Task TestNoDiagnostic_UnitySerializeAttribute()
+    {
+        await VerifyNoDiagnosticAsync(@"
+using System;
+using UnityEngine;
+
+class C
+{
+    [SerializeField]
+    private string f;
+}
+
+namespace UnityEngine
+{
+    class SerializeFieldAttribute : Attribute
+    {
+    }
+}
+", options: Options.AddConfigOption(ConfigOptionKeys.UnityCodeAnalysisEnabled, true));
+    }
 }
diff --git a/src/Tests/Analyzers.Tests/RCS1213RemoveUnusedMemberDeclarationTests.cs b/src/Tests/Analyzers.Tests/RCS1213RemoveUnusedMemberDeclarationTests.cs
@@ -366,4 +366,26 @@ class MonoBehaviour
 }
 ", options: Options.AddConfigOption(ConfigOptionKeys.SuppressUnityScriptMethods, true));
     }
+
+    [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.RemoveUnusedMemberDeclaration)]
+    public async Task TestNoDiagnostic_UnityScriptMethods2()
+    {
+        await VerifyNoDiagnosticAsync(@"
+using UnityEngine;
+
+class C : MonoBehaviour
+{
+    private void Awake()
+    {
+    }
+}
+
+namespace UnityEngine
+{
+    class MonoBehaviour
+    {
+    }
+}
+", options: Options.AddConfigOption(ConfigOptionKeys.UnityCodeAnalysisEnabled, true));
+    }
 }
diff --git a/src/Tools/CodeGeneration/CSharp/CodeGenerator.cs b/src/Tools/CodeGeneration/CSharp/CodeGenerator.cs
@@ -18,7 +18,7 @@ public static class CodeGenerator
     public static CompilationUnitSyntax GenerateConfigOptions(IEnumerable<AnalyzerOptionMetadata> options, IEnumerable<AnalyzerMetadata> analyzers)
     {
         CompilationUnitSyntax compilationUnit = CompilationUnit(
-            UsingDirectives("System.Collections.Generic"),
+            UsingDirectives("System", "System.Collections.Generic"),
             NamespaceDeclaration(
                 "Roslynator",
                 ClassDeclaration(
@@ -38,7 +38,8 @@ public static CompilationUnitSyntax GenerateConfigOptions(IEnumerable<AnalyzerOp
                                         Argument(NameColon("defaultValue"), (f.DefaultValue is not null) ? StringLiteralExpression(f.DefaultValue) : NullLiteralExpression()),
                                         Argument(NameColon("defaultValuePlaceholder"), StringLiteralExpression(f.DefaultValuePlaceholder)),
                                         Argument(NameColon("description"), StringLiteralExpression(f.Description))),
-                                    default(InitializerExpressionSyntax)));
+                                    default(InitializerExpressionSyntax)))
+                                .AddObsoleteAttributeIf(f.IsObsolete);
                         })
                         .Concat(new MemberDeclarationSyntax[]
                             {
diff --git a/src/Tools/CodeGeneration/EditorConfig/EditorConfigGenerator.cs b/src/Tools/CodeGeneration/EditorConfig/EditorConfigGenerator.cs
@@ -36,7 +36,9 @@ public static string GenerateEditorConfig(RoslynatorMetadata metadata, bool comm
 
             var isSeparatedWithNewLine = true;
 
-            foreach (AnalyzerOptionMetadata option in metadata.ConfigOptions.OrderBy(f => f.Key))
+            foreach (AnalyzerOptionMetadata option in metadata.ConfigOptions
+                .Where(f => !f.IsObsolete)
+                .OrderBy(f => f.Key))
             {
                 if (optionMap.TryGetValue(option.Key, out HashSet<AnalyzerMetadata> analyzers)
                     && !isSeparatedWithNewLine)
@@ -86,6 +88,7 @@ public static string GenerateEditorConfig(RoslynatorMetadata metadata, bool comm
                         + string.Join(
                             ", ",
                             analyzer.ConfigOptions
+                                .Where(f => metadata.ConfigOptions.FirstOrDefault(ff => ff.Key == f.Key)?.IsObsolete != true)
                                 .OrderBy(f => f.Key)
                                 .Select(f2 => metadata.ConfigOptions.First(f => f.Key == f2.Key).Key)));
                 }
diff --git a/src/Tools/ConfigurationFileGenerator/Properties/launchSettings.json b/src/Tools/ConfigurationFileGenerator/Properties/launchSettings.json
@@ -2,7 +2,7 @@
   "profiles": {
     "CodeGenerator": {
       "commandName": "Project",
-      "commandLineArgs": "\"../../../../..\""
+      "commandLineArgs": "\"../../../../..\" \"../../../../../Tools/ConfigurationFileGenerator/configuration.md\" \"../../../../../../tools/build/configuration.md\""
     }
   }
 }
diff --git a/src/Tools/Metadata/AnalyzerOptionMetadata.cs b/src/Tools/Metadata/AnalyzerOptionMetadata.cs
@@ -4,7 +4,7 @@
 
 namespace Roslynator.Metadata;
 
-public record AnalyzerOptionMetadata(string Id, string Key, string DefaultValue, string DefaultValuePlaceholder, string Description)
+public record AnalyzerOptionMetadata(string Id, string Key, string DefaultValue, string DefaultValuePlaceholder, string Description, bool IsObsolete)
 {
     public List<AnalyzerOptionValueMetadata> Values { get; } = new();
 }
diff --git a/src/Tools/Metadata/MetadataFile.cs b/src/Tools/Metadata/MetadataFile.cs
@@ -290,7 +290,8 @@ public static IEnumerable<AnalyzerOptionMetadata> ReadOptions(string filePath)
                 Key: "roslynator_" + key,
                 DefaultValue: defaultValue,
                 DefaultValuePlaceholder: defaultValuePlaceholder,
-                Description: element.Element("Description").Value
+                Description: element.Element("Description").Value,
+                IsObsolete: element.AttributeValueAsBooleanOrDefault("IsObsolete")
             );
 
             analyzerOption.Values.AddRange(values ?? Enumerable.Empty<AnalyzerOptionValueMetadata>());
diff --git a/src/VisualStudioCode/package/src/configurationFiles.generated.ts b/src/VisualStudioCode/package/src/configurationFiles.generated.ts
@@ -117,16 +117,16 @@ roslynator_analyzers.enabled_by_default = true|false
 
 #roslynator_prefix_field_identifier_with_underscore = true|false
 
-#roslynator_suppress_unity_script_methods = true|false
-# Applicable to: rcs1213
-
 #roslynator_tab_length = <NUM>
 # Default: 4
 # Applicable to: rcs0056
 
 #roslynator_trailing_comma_style = include|omit|omit_when_single_line
 # Applicable to: rcs1260
 
+#roslynator_unity_code_analysis.enabled = true|false
+# Applicable to: rcs1169, rcs1213
+
 #roslynator_use_anonymous_function_or_method_group = anonymous_function|method_group
 # Applicable to: rcs1207
 
@@ -641,6 +641,7 @@ roslynator_analyzers.enabled_by_default = true|false
 
 # Make field read-only
 #dotnet_diagnostic.rcs1169.severity = suggestion
+# Options: roslynator_unity_code_analysis.enabled
 
 # Use read-only auto-implemented property
 #dotnet_diagnostic.rcs1170.severity = suggestion
@@ -762,7 +763,7 @@ roslynator_analyzers.enabled_by_default = true|false
 
 # Remove unused member declaration
 #dotnet_diagnostic.rcs1213.severity = suggestion
-# Options: roslynator_suppress_unity_script_methods
+# Options: roslynator_unity_code_analysis.enabled
 
 # Unnecessary interpolated string
 #dotnet_diagnostic.rcs1214.severity = suggestion

Original file line number	Diff line number	Diff line change
`@@ -159,7 +159,7 @@ private static void AnalyzeTypeDeclaration(SyntaxNodeAnalysisContext context)`
`159`	`159`	`break;`
`160`	`160`
`161`	`161`	`if (declaration.ReturnsVoid()`
`162`		`- && context.GetSuppressUnityScriptMethods() == true)`
	`162`	`+ && context.IsUnityCodeAnalysisEnabled() == true)`
`163`	`163`	`{`
`164`	`164`	`if (canContainUnityScriptMethods is null)`
`165`	`165`	`{`