Set default value from ConfigurableScriptRule constructor

Author: Kapil Borle

Engine/Generic/ConfigurableScriptRule.cs

@@ -8,11 +8,16 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic
     // This is still an experimental class. Use at your own risk!
     public abstract class ConfigurableScriptRule : IScriptRule
     {
-        // Configurable rules should define a default value
+        // Configurable rule properties should define a default value
         // because if reading the configuration fails
         // we use the property's default value
-        [ConfigurableRuleProperty()]
-        public bool Enable { get; protected set; } = false;
+        [ConfigurableRuleProperty(defaultValue: false)]
+        public bool Enable { get; protected set; }
+
+        protected ConfigurableScriptRule()
+        {
+            SetDefaultValues();
+        }
 
         public virtual void ConfigureRule(IDictionary<string, object> paramValueMap)
         {
@@ -23,29 +28,37 @@ public virtual void ConfigureRule(IDictionary<string, object> paramValueMap)
 
             try
             {
-                var properties = GetConfigurableProperties();
-                foreach (var property in properties)
+                foreach (var property in GetConfigurableProperties())
                 {
                     if (paramValueMap.ContainsKey(property.Name))
                     {
-                        var type = property.PropertyType;
-                        var obj = paramValueMap[property.Name];
-
-                        // TODO Check if type is convertible
-                        property.SetValue(
-                            this,
-                            System.Convert.ChangeType(obj, type));
+                        SetValue(property, paramValueMap[property.Name]);
                     }
                 }
             }
             catch
             {
                 // we do not know how to handle an exception in this case yet!
-                // but we know that this should not crash the program hence we
-                // have this empty catch block
+                // but we know that this should not crash the program
+                // hence we revert the property values to their default
+                SetDefaultValues();
+            }
+        }
+
+        private void SetDefaultValues()
+        {
+            foreach (var property in GetConfigurableProperties())
+            {
+                SetValue(property, GetDefaultValue(property));
             }
         }
 
+        private void SetValue(PropertyInfo property, object value)
+        {
+            // TODO Check if type is convertible
+            property.SetValue(this, Convert.ChangeType(value, property.PropertyType));
+        }
+
         private IEnumerable<PropertyInfo> GetConfigurableProperties()
         {
             foreach (var property in this.GetType().GetProperties())
@@ -57,6 +70,19 @@ private IEnumerable<PropertyInfo> GetConfigurableProperties()
             }
         }
 
+        private Object GetDefaultValue(PropertyInfo property)
+        {
+            var attr = property.GetCustomAttribute(typeof(ConfigurableRulePropertyAttribute));
+            if (attr == null)
+            {
+                throw new ArgumentException(
+                    String.Format(Strings.ConfigurableScriptRulePropertyHasNotAttribute, property.Name),
+                    nameof(property));
+            }
+
+            return ((ConfigurableRulePropertyAttribute)attr).DefaultValue;
+        }
+
         public abstract IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName);
         public abstract string GetCommonName();
         public abstract string GetDescription();
@@ -69,6 +95,16 @@ private IEnumerable<PropertyInfo> GetConfigurableProperties()
     [AttributeUsage(AttributeTargets.Property, AllowMultiple = false)]
     public class ConfigurableRulePropertyAttribute : Attribute
     {
+        public object DefaultValue { get; private set; }
 
+        public ConfigurableRulePropertyAttribute(object defaultValue)
+        {
+            if (defaultValue == null)
+            {
+                throw new ArgumentNullException(nameof(defaultValue), Strings.ConfigurableScriptRuleNRE);
+            }
+
+            DefaultValue = defaultValue;
+        }
     }
 }

Engine/Strings.resx

@@ -255,4 +255,10 @@
   <data name="DigraphVertexDoesNotExists" xml:space="preserve">
     <value>Vertex {0} does not exist in the digraph.</value>
   </data>
+  <data name="ConfigurableScriptRulePropertyHasNotAttribute" xml:space="preserve">
+    <value>"Cannot find a ConfigurableRuleProperty attribute on property {0}".</value>
+  </data>
+  <data name="ConfigurableScriptRuleNRE" xml:space="preserve">
+    <value>"Argument should not be null.".</value>
+  </data>
 </root>

Rules/PlaceCloseBrace.cs

@@ -28,8 +28,8 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
 #endif
     public class PlaceCloseBrace : ConfigurableScriptRule
     {
-        [ConfigurableRuleProperty()]
-        public bool NoEmptyLineBefore { get; protected set; } = false;
+        [ConfigurableRuleProperty(defaultValue:false)]
+        public bool NoEmptyLineBefore { get; protected set; }
 
         /// <summary>
         /// Analyzes the given ast to find the [violation]

Rules/PlaceOpenBrace.cs

@@ -32,11 +32,11 @@ public class PlaceOpenBrace : ConfigurableScriptRule
         private List<Func<Token[], Ast, string, IEnumerable<DiagnosticRecord>>> violationFinders
             = new List<Func<Token[], Ast, string, IEnumerable<DiagnosticRecord>>>();
 
-        [ConfigurableRuleProperty()]
-        public bool OnSameLine { get; protected set; } = true;
+        [ConfigurableRuleProperty(defaultValue:true)]
+        public bool OnSameLine { get; protected set; }
 
-        [ConfigurableRuleProperty()]
-        public bool NewLineAfter { get; protected set; } = true;
+        [ConfigurableRuleProperty(defaultValue:true)]
+        public bool NewLineAfter { get; protected set; }
 
         /// <summary>
         /// Analyzes the given ast to find the [violation]

Rules/UseConsistentIndentation.cs

@@ -33,8 +33,8 @@ private enum IndentationKind { Space, Tab };
         // TODO make this configurable
         private readonly IndentationKind indentationKind = IndentationKind.Space;
 
-        [ConfigurableRuleProperty()]
-        public int IndentationSize { get; protected set; } = 4;
+        [ConfigurableRuleProperty(defaultValue:4)]
+        public int IndentationSize { get; protected set; }
 
         /// <summary>
         /// Analyzes the given ast to find the [violation]