Pass rule configuration to configure method

Author: Kapil Borle

Engine/Generic/ConfigurableScriptRule.cs

@@ -8,9 +8,7 @@
 
 namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic
 {
-    // This is still an experimental class and we do not want to expose it
-    // as a public API as of yet. So we place it in builtinrules project
-    // and keep it internal as it consumed only by a handful of rules.
+    // This is still an experimental class. Use at your own risk!
     public abstract class ConfigurableScriptRule : IScriptRule
     {
         // Configurable rules should define a default value
@@ -19,19 +17,24 @@ public abstract class ConfigurableScriptRule : IScriptRule
         [ConfigurableRuleProperty()]
         public bool Enable { get; protected set; } = false;
 
-        public virtual void ConfigureRule()
+        public virtual void ConfigureRule(IDictionary<string, object> paramValueMap)
         {
-            // TODO Do not use Helper. Pass this method a dictionary instead
-            var arguments = Helper.Instance.GetRuleArguments(this.GetName());
+            if (paramValueMap == null)
+            {
+                throw new ArgumentNullException(nameof(paramValueMap));
+            }
+
             try
             {
                 var properties = GetConfigurableProperties();
                 foreach (var property in properties)
                 {
-                    if (arguments.ContainsKey(property.Name))
+                    if (paramValueMap.ContainsKey(property.Name))
                     {
                         var type = property.PropertyType;
-                        var obj = arguments[property.Name];
+                        var obj = paramValueMap[property.Name];
+
+                        // TODO Check if type is convertible
                         property.SetValue(
                             this,
                             System.Convert.ChangeType(obj, type));
@@ -40,7 +43,7 @@ public virtual void ConfigureRule()
             }
             catch
             {
-                // we do not know how to handle an exception yet in this case yet!
+                // 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
             }

Engine/ScriptAnalyzer.cs

@@ -968,12 +968,26 @@ private void LoadRules(Dictionary<string, List<string>> result, CommandInvocatio
         // Configure rules derived from ConfigurableScriptRule class
         private void ConfigureScriptRules()
         {
-            if (ScriptRules != null)
+            if (ScriptRules == null)
             {
-                foreach (var scriptRule in ScriptRules)
+                return;
+            }
+
+            foreach (var scriptRule in ScriptRules)
+            {
+                var configurableScriptRule = scriptRule as ConfigurableScriptRule;
+                if (configurableScriptRule == null)
                 {
-                    (scriptRule as ConfigurableScriptRule)?.ConfigureRule();
+                    continue;
                 }
+
+                var paramValueMap = Helper.Instance.GetRuleArguments(scriptRule.GetName());
+                if (paramValueMap == null)
+                {
+                    continue;
+                }
+
+                configurableScriptRule.ConfigureRule(paramValueMap);
             }
         }
 

Rules/PlaceOpenBrace.cs

@@ -51,6 +51,13 @@ public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string file
                 throw new ArgumentNullException("ast");
             }
 
+            // TODO Should have the following option
+            // * no-empty-lines-after
+            // TODO handle open brace for a command parameter.
+            // * E.g. get-process | % { "blah }
+            // In the above case even if OnSameLine == false, we should not
+            // flag the open brace as it would move the brace to the next line
+            // and it will invalidate the command
             var diagnosticRecords = new List<DiagnosticRecord>();
             if (Enable)
             {
@@ -60,23 +67,18 @@ public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string file
                 }
             }
 
-            // TODO Should have the following options
-            // * new-line-after
-            // * no-empty-line-after
             return diagnosticRecords;
         }
 
-        public override void ConfigureRule()
+        public override void ConfigureRule(IDictionary<string, object> paramValueMap)
         {
-            base.ConfigureRule();
+            base.ConfigureRule(paramValueMap);
             if (OnSameLine)
             {
-                // findViolations = this.FindViolationsForBraceShouldBeOnSameLine;
                 violationFinders.Add(FindViolationsForBraceShouldBeOnSameLine);
             }
             else
             {
-                // findViolations = this.FindViolationsForBraceShouldNotBeOnSameLine;
                 violationFinders.Add(FindViolationsForBraceShouldNotBeOnSameLine);
             }