Context actions and inspections for serialised fields

resharper/src/resharper-unity/Daemon/Errors/CSharpErrors.xml

@@ -43,7 +43,7 @@
       <Tag externalName="DuplicateEventFunctionWarning.HIGHLIGHTING_ID"
            default="WARNING">
         <Title>Event function with the same name is already declared</Title>
-        <Description>Event function with the same name is already declared</Description>
+        <Description>Event function with the same name is already declared.</Description>
       </Tag>
 
       <Tag externalName="ExplicitTagStringComparisonWarning.HIGHLIGHTING_ID"
@@ -54,7 +54,7 @@
 
       <Tag externalName="IncorrectMonoBehaviourInstantiationWarning.HIGHLIGHTING_ID" default="WARNING">
         <Title>MonoBehaviours must be instantiated with 'GameObject.AddComponent&lt;T&gt;()' instead of 'new'</Title>
-        <Description>Instantiating a MonoBehaviour based class with 'new' does not attach it to a GameObject, and Unity will not invoke any event functions. Create a new instance using 'GameObject.AddComponent&lt;T&gt;()</Description>
+        <Description>Instantiating a MonoBehaviour based class with 'new' does not attach it to a GameObject, and Unity will not invoke any event functions. Create a new instance using 'GameObject.AddComponent&lt;T&gt;().</Description>
       </Tag>
 
       <Tag externalName="IncorrectScriptableObjectInstantiationWarning.HIGHLIGHTING_ID"
@@ -72,6 +72,7 @@
            default="WARNING">
         <Title>Attribute is redundant when applied to this declaration type</Title>
         <Description>The attribute does not define any restrictions on valid targets, but is only useful when applied to specific declarations, e.g. field, class or method.</Description>
+        <CompoundItemName>Redundant attribute usage</CompoundItemName>
       </Tag>
 
       <Tag externalName="RedundantEventFunctionWarning.HIGHLIGHTING_ID"
@@ -84,12 +85,21 @@
            default="WARNING">
         <Title>Redundant 'InitializeOnLoad' attribute</Title>
         <Description>The 'InitializeOnLoad' attribute is redundant when static constructor is missing.</Description>
+        <CompoundItemName>Redundant attribute usage</CompoundItemName>
       </Tag>
 
       <Tag externalName="RedundantSerializeFieldAttributeWarning.HIGHLIGHTING_ID"
            default="WARNING">
         <Title>Redundant 'SerializeField' attribute</Title>
-        <Description>Unity will ignore the 'SerializeField' attribute if a field is also marked with the 'NonSerialized' attribute</Description>
+        <Description>Unity will ignore the 'SerializeField' attribute if a field is also marked with the 'NonSerialized' attribute.</Description>
+        <CompoundItemName>Redundant attribute usage</CompoundItemName>
+      </Tag>
+
+      <Tag externalName="RedundantHideInInspectorAttributeWarning.HIGHLIGHTING_ID"
+           default="WARNING">
+        <Title>Redundant 'HideInInspector' attribute</Title>
+        <Description>The 'HideInInspector' attribute only applies to serialised fields.</Description>
+        <CompoundItemName>Redundant attribute usage</CompoundItemName>
       </Tag>
 
       <Tag externalName="StringLiteralReferenceIncorrectSignatureWarning.HIGHLIGHTING_ID"
@@ -253,6 +263,13 @@
     <Behavour attributeID="DEADCODE" overlapResolvePolicy="DEADCODE" />
   </Warning>
 
+  <Warning name="RedundantHideInInspectorAttribute" configurableSeverity="Unity.RedundantHideInInspectorAttribute">
+    <Parameter type="IAttribute" name="attribute" />
+    <Message value="Redundant 'HideInInspector' attribute" />
+    <Range>Attribute.GetHighlightingRange()</Range>
+    <Behavour attributeID="DEADCODE" overlapResolvePolicy="DEADCODE" />
+  </Warning>
+
   <Error name="StringLiteralReferenceIncorrectSignature" staticGroup="UnityErrors">
     <Parameter type="SyncVarHookReference" name="reference" />
     <Message value="Expected a method with '{0}' signature">

resharper/src/resharper-unity/Daemon/Stages/Analysis/RedundantHideInInspectorAttributeProblemAnalyzer.cs

@@ -0,0 +1,37 @@
+using JetBrains.ReSharper.Feature.Services.Daemon;
+using JetBrains.ReSharper.Plugins.Unity.CSharp.Daemon.Errors;
+using JetBrains.ReSharper.Plugins.Unity.Daemon.Stages.Dispatcher;
+using JetBrains.ReSharper.Psi;
+using JetBrains.ReSharper.Psi.CSharp.Tree;
+
+namespace JetBrains.ReSharper.Plugins.Unity.Daemon.Stages.Analysis
+{
+    [ElementProblemAnalyzer(typeof(IAttribute),
+        HighlightingTypes = new[] { typeof(RedundantHideInInspectorAttributeWarning) })]
+    public class RedundantHideInInspectorAttributeProblemAnalyzer : UnityElementProblemAnalyzer<IAttribute>
+    {
+        public RedundantHideInInspectorAttributeProblemAnalyzer(UnityApi unityApi)
+            : base(unityApi)
+        {
+        }
+
+        protected override void Analyze(IAttribute attribute, ElementProblemAnalyzerData data, IHighlightingConsumer consumer)
+        {
+            if (!(attribute.TypeReference?.Resolve().DeclaredElement is ITypeElement attributeTypeElement))
+                return;
+
+            if (!Equals(attributeTypeElement.GetClrName(), KnownTypes.HideInInspector))
+                return;
+
+            var fieldDeclarations = FieldDeclarationNavigator.GetByAttribute(attribute);
+            foreach (var fieldDeclaration in fieldDeclarations)
+            {
+                if (!Api.IsUnityField(fieldDeclaration.DeclaredElement))
+                {
+                    consumer.AddHighlighting(new RedundantHideInInspectorAttributeWarning(attribute));
+                    return;
+                }
+            }
+        }
+    }
+}

resharper/src/resharper-unity/Daemon/Stages/Analysis/RedundantSerializeFieldAttributeProblemAnalyzer.cs

@@ -26,8 +26,8 @@ protected override void Analyze(IAttribute attribute, ElementProblemAnalyzerData
             var fieldDeclarations = FieldDeclarationNavigator.GetByAttribute(attribute);
             foreach (var fieldDeclaration in fieldDeclarations)
             {
-                if (fieldDeclaration.DeclaredElement.HasAttributeInstance(PredefinedType.NONSERIALIZED_ATTRIBUTE_CLASS,
-                    false))
+                if (fieldDeclaration.DeclaredElement?.HasAttributeInstance(PredefinedType.NONSERIALIZED_ATTRIBUTE_CLASS,
+                    false) == true || fieldDeclaration.IsReadonly)
                 {
                     consumer.AddHighlighting(new RedundantSerializeFieldAttributeWarning(attribute));
                     return;

resharper/src/resharper-unity/Feature/Services/ContextActions/AttributeUtil.cs

@@ -2,24 +2,83 @@
 using JetBrains.Metadata.Reader.API;
 using JetBrains.ReSharper.Psi;
 using JetBrains.ReSharper.Psi.CSharp;
+using JetBrains.ReSharper.Psi.CSharp.Impl;
 using JetBrains.ReSharper.Psi.CSharp.Tree;
 using JetBrains.ReSharper.Psi.Modules;
 
 namespace JetBrains.ReSharper.Plugins.Unity.Feature.Services.ContextActions
 {
     public static class AttributeUtil
     {
-        public static void AddAttribute([CanBeNull] IFieldDeclaration fieldDeclaration, IClrTypeName attributeTypeName,
-            IPsiModule psiModule, CSharpElementFactory elementFactory)
+        public static void AddAttributeToSingleDeclaration([CanBeNull] IFieldDeclaration fieldDeclaration,
+            IClrTypeName attributeTypeName, IPsiModule psiModule, CSharpElementFactory elementFactory)
         {
             if (fieldDeclaration == null) return;
 
-            var typeElement = TypeFactory.CreateTypeByCLRName(attributeTypeName, psiModule).GetTypeElement();
-            if (typeElement != null)
-            {
-                var attribute = elementFactory.CreateAttribute(typeElement);
+            var existingAttribute = GetAttribute(fieldDeclaration, attributeTypeName);
+            if (existingAttribute != null)
+                return;
+
+            var attribute = CreateAttribute(attributeTypeName, psiModule, elementFactory);
+            if (attribute != null)
                 fieldDeclaration.AddAttributeAfter(attribute, null);
+        }
+
+        public static void AddAttributeToAllDeclarations([CanBeNull] IFieldDeclaration fieldDeclaration,
+            IClrTypeName attributeTypeName, IPsiModule psiModule, CSharpElementFactory elementFactory)
+        {
+            if (fieldDeclaration == null) return;
+
+            var existingAttribute = GetAttribute(fieldDeclaration, attributeTypeName);
+            if (existingAttribute != null)
+                return;
+
+            var attribute = CreateAttribute(attributeTypeName, psiModule, elementFactory);
+            if (attribute != null)
+                CSharpSharedImplUtil.AddAttributeAfter(fieldDeclaration, attribute, null);
+        }
+
+        [CanBeNull]
+        private static IAttribute CreateAttribute(IClrTypeName attributeTypeName, IPsiModule module,
+            CSharpElementFactory elementFactory)
+        {
+            var typeElement = TypeFactory.CreateTypeByCLRName(attributeTypeName, module).GetTypeElement();
+            return typeElement != null ? elementFactory.CreateAttribute(typeElement) : null;
+        }
+
+        public static void RemoveAttributeFromSingleDeclaration([CanBeNull] IFieldDeclaration fieldDeclaration,
+            IClrTypeName attributeTypeName)
+        {
+            var attribute = GetAttribute(fieldDeclaration, attributeTypeName);
+            if (attribute != null)
+                fieldDeclaration.RemoveAttribute(attribute);
+        }
+
+        public static void RemoveAttributeFromAllDeclarations([CanBeNull] IFieldDeclaration fieldDeclaration,
+            IClrTypeName attributeTypeName)
+        {
+            var attribute = GetAttribute(fieldDeclaration, attributeTypeName);
+            if (attribute != null)
+                CSharpSharedImplUtil.RemoveAttribute(fieldDeclaration, attribute);
+        }
+
+        [CanBeNull, ContractAnnotation("attributesOwner:null => null")]
+        public static IAttribute GetAttribute([CanBeNull] IAttributesOwnerDeclaration attributesOwner,
+            IClrTypeName requiredAttributeTypeName)
+        {
+            if (attributesOwner == null) return null;
+
+            foreach (var attribute in attributesOwner.AttributesEnumerable)
+            {
+                if (attribute.TypeReference?.Resolve().DeclaredElement is ITypeElement typeElement)
+                {
+                    var attributeTypeName = typeElement.GetClrName();
+                    if (Equals(attributeTypeName, requiredAttributeTypeName))
+                        return attribute;
+                }
             }
+
+            return null;
         }
     }
 }

resharper/src/resharper-unity/Feature/Services/ContextActions/AutoPropertyToSerializedBackingFieldAction.cs

@@ -16,7 +16,7 @@ namespace JetBrains.ReSharper.Plugins.Unity.Feature.Services.ContextActions
 {
     [ContextAction(Group = UnityContextActions.GroupID,
         Name = "Replace auto-property with property and serialized backing field",
-        Description = "Replaces auto-property in a Unity type with a property that utilizes a backing field that is marked with the 'UnityEngine.SerializeField' attribute.",
+        Description = "Replaces an auto-property in a Unity type with a property that utilizes a backing field that is marked with the 'UnityEngine.SerializeField' attribute.",
         Priority = 2)]
     public class AutoPropertyToSerializedBackingFieldAction : ContextActionBase
     {
@@ -74,7 +74,7 @@ public static Action<ITextControl> Execute([CanBeNull] IPropertyDeclaration prop
                 return null;
 
             var fieldDeclaration = AutomaticToBackingFieldAction.Execute(propertyDeclaration);
-            AttributeUtil.AddAttribute(fieldDeclaration, KnownTypes.SerializeField, propertyDeclaration.GetPsiModule(), elementFactory);
+            AttributeUtil.AddAttributeToSingleDeclaration(fieldDeclaration, KnownTypes.SerializeField, propertyDeclaration.GetPsiModule(), elementFactory);
             return AutomaticToBackingFieldAction.PostExecute(propertyDeclaration, fieldDeclaration, solution);
         }
     }