Merge pull request #690 from PowerShell/kapilmb/CodeFormatting

Add code formatting rules

Author: Kapil Borle

Engine/Generic/ConfigurableRule.cs

@@ -0,0 +1,156 @@
+// Copyright (c) Microsoft Corporation.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+using System;
+using System.Collections.Generic;
+using System.Management.Automation.Language;
+using System.Reflection;
+
+namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic
+{
+    // This is still an experimental class. Use at your own risk!
+    public abstract class ConfigurableRule : IScriptRule
+    {
+        /// <summary>
+        /// Indicates if the rule is enabled or not.
+        ///
+        /// If the rule is enabled ScriptAnalyzer engine will run it
+        /// otherwise it will not.
+        ///
+        /// Configurable rule properties should define a default value
+        /// because if reading the configuration fails we use the
+        /// property's default value
+        /// </summary>
+        [ConfigurableRuleProperty(defaultValue: false)]
+        public bool Enable { get; protected set; }
+
+        /// <summary>
+        /// Initialize the configurable properties of a configurable rule.
+        /// </summary>
+        protected ConfigurableRule()
+        {
+            SetDefaultValues();
+        }
+
+        /// <summary>
+        /// Sets the configurable properties of the rule.
+        ///
+        /// Properties having ConfigurableRuleProperty attribute are called configurable properties.
+        /// </summary>
+        /// <param name="paramValueMap">A dictionary that maps parameter name to it value. Must be non-null</param>
+        public virtual void ConfigureRule(IDictionary<string, object> paramValueMap)
+        {
+            if (paramValueMap == null)
+            {
+                throw new ArgumentNullException(nameof(paramValueMap));
+            }
+
+            try
+            {
+                foreach (var property in GetConfigurableProperties())
+                {
+                    if (paramValueMap.ContainsKey(property.Name))
+                    {
+                        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 revert the property values to their default
+                SetDefaultValues();
+            }
+        }
+
+        /// <summary>
+        /// Analyzes the given abstract syntax tree (AST) and returns diagnostic records based on the analysis.
+        /// </summary>
+        /// <param name="ast">AST representing the file content</param>
+        /// <param name="fileName">Path of the file corresponding to the AST</param>
+        /// <returns>The results of the analysis</returns>
+        public abstract IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName);
+
+        /// <summary>
+        /// Retrieves the Common name of the rule.
+        /// </summary>
+        /// <returns>The name of the rule.</returns>
+        public abstract string GetCommonName();
+
+        /// <summary>
+        /// Retrieves the description of the rule.
+        /// </summary>
+        /// <returns>The description of the rule.</returns>
+        public abstract string GetDescription();
+
+        /// <summary>
+        /// Retrieves the name of the rule.
+        /// </summary>
+        /// <returns>The name of the rule.</returns>
+        public abstract string GetName();
+
+        /// <summary>
+        /// Retrieves severity of the rule.
+        /// </summary>
+        /// <returns>The severity of the rule.</returns>
+        public abstract RuleSeverity GetSeverity();
+
+        /// <summary>
+        /// Retrieves the source name of the rule.
+        /// </summary>
+        /// <returns>The source name of the rule.</returns>
+        public abstract string GetSourceName();
+
+        /// <summary>
+        /// Retrieves the source type of the rule.
+        /// </summary>
+        /// <returns>The source type of the rule.</returns>
+        public abstract SourceType GetSourceType();
+
+        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())
+            {
+                if (property.GetCustomAttribute(typeof(ConfigurableRulePropertyAttribute)) != null)
+                {
+                    yield return property;
+                }
+            }
+        }
+
+        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;
+        }
+    }
+}

Engine/Generic/ConfigurableRulePropertyAttribute.cs

@@ -0,0 +1,40 @@
+// Copyright (c) Microsoft Corporation.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+using System;
+
+namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic
+{
+    /// <summary>
+    /// The attribute class to designate if a property is configurable or not.
+    /// </summary>
+    [AttributeUsage(AttributeTargets.Property)]
+    public class ConfigurableRulePropertyAttribute : Attribute
+    {
+        /// <summary>
+        /// Default value of the property that the attribute decorates.
+        /// </summary>
+        public object DefaultValue { get; private set; }
+
+        /// <summary>
+        /// Initialize the attribute with the decorated property's default value.
+        /// </summary>
+        /// <param name="defaultValue"></param>
+        public ConfigurableRulePropertyAttribute(object defaultValue)
+        {
+            if (defaultValue == null)
+            {
+                throw new ArgumentNullException(nameof(defaultValue), Strings.ConfigurableScriptRuleNRE);
+            }
+
+            DefaultValue = defaultValue;
+        }
+    }
+}

Engine/Helper.cs

@@ -64,10 +64,7 @@ internal set
             {
                 lock (syncRoot)
                 {
-                    if (instance == null)
-                    {
-                        instance = value;
-                    }
+                    instance = value;
                 }
             }
         }

Engine/ScriptAnalyzer.cs

@@ -12,7 +12,6 @@
 //
 
 using System.Text.RegularExpressions;
-using Microsoft.Windows.PowerShell.ScriptAnalyzer.Commands;
 using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
 using System;
 using System.Collections.Generic;
@@ -490,7 +489,10 @@ private bool ParseProfileHashtable(Hashtable profile, PathIntrinsics path, IOutp
             {
                 return hasError;
             }
-            foreach (var settingKey in settings.Keys)
+
+            var settingsKeys = new string[settings.Keys.Count];
+            settings.Keys.CopyTo(settingsKeys, 0);
+            foreach (var settingKey in settingsKeys)
             {
                 var key = settingKey.ToLower();
                 object value = settings[key];
@@ -599,6 +601,7 @@ private bool ParseProfileString(string profile, PathIntrinsics path, IOutputWrit
                 else
                 {
                     HashtableAst hashTableAst = hashTableAsts.First() as HashtableAst;
+#if PSV3
                     settings = GetDictionaryFromHashTableAst(
                         hashTableAst,
                         writer,
@@ -627,6 +630,24 @@ private bool ParseProfileString(string profile, PathIntrinsics path, IOutputWrit
                             hasError = true;
                         }
                     }
+#else
+
+                    try
+                    {
+                        var hashtable = hashTableAst.SafeGetValue() as Hashtable;
+                        hasError = ParseProfileHashtable(hashtable, path, writer, severityList, includeRuleList, excludeRuleList);
+                    }
+                    catch
+                    {
+                        writer.WriteError(
+                            new ErrorRecord(
+                                new ArgumentException(string.Format(CultureInfo.CurrentCulture, Strings.InvalidProfile, profile)),
+                                Strings.ConfigurationFileHasInvalidHashtable,
+                                ErrorCategory.ResourceUnavailable,
+                                profile));
+                        hasError = true;
+                    }
+#endif // PSV3
                 }
             }
 
@@ -754,6 +775,7 @@ private void Initialize(
             try
             {
                 this.LoadRules(this.validationResults, invokeCommand, includeDefaultRules);
+                this.ConfigureScriptRules();
             }
             catch (Exception ex)
             {
@@ -942,6 +964,32 @@ private void LoadRules(Dictionary<string, List<string>> result, CommandInvocatio
             }
         }
 
+        // Configure rules derived from ConfigurableScriptRule class
+        private void ConfigureScriptRules()
+        {
+            if (ScriptRules == null)
+            {
+                return;
+            }
+
+            foreach (var scriptRule in ScriptRules)
+            {
+                var configurableScriptRule = scriptRule as ConfigurableRule;
+                if (configurableScriptRule == null)
+                {
+                    continue;
+                }
+
+                var paramValueMap = Helper.Instance.GetRuleArguments(scriptRule.GetName());
+                if (paramValueMap == null)
+                {
+                    continue;
+                }
+
+                configurableScriptRule.ConfigureRule(paramValueMap);
+            }
+        }
+
         internal string[] GetValidModulePaths()
         {
             List<string> validModulePaths = null;
@@ -1621,6 +1669,12 @@ private bool IsSeverityAllowed(IEnumerable<uint> allowedSeverities, IRule rule)
                     && allowedSeverities.Contains((uint)rule.GetSeverity()));
         }
 
+        private static bool IsRuleEnabled(IRule rule)
+        {
+            var configurableRule = rule as ConfigurableRule;
+            return configurableRule == null || configurableRule.Enable;
+        }
+
         IEnumerable<uint> GetAllowedSeveritiesInInt()
         {
             return severity != null
@@ -1667,7 +1721,8 @@ bool IsRuleAllowed(IRule rule)
 
             return (includeRule == null || includeRegexMatch)
                     && (excludeRule == null || !excludeRegexMatch)
-                    && IsSeverityAllowed(allowedSeverities, rule);
+                    && IsSeverityAllowed(allowedSeverities, rule)
+                    && IsRuleEnabled(rule);
         }
 
         /// <summary>

Engine/ScriptAnalyzerEngine.csproj

@@ -70,6 +70,8 @@
     <Compile Include="Commands\InvokeScriptAnalyzerCommand.cs" />
     <Compile Include="Generic\AvoidCmdletGeneric.cs" />
     <Compile Include="Generic\AvoidParameterGeneric.cs" />
+    <Compile Include="Generic\ConfigurableRulePropertyAttribute.cs" />
+    <Compile Include="Generic\ConfigurableRule.cs" />
     <Compile Include="Generic\ModuleDependencyHandler.cs" />
     <Compile Include="Generic\CorrectionExtent.cs" />
     <Compile Include="Generic\SuppressedRecord.cs" />
@@ -99,6 +101,7 @@
       <DesignTime>True</DesignTime>
       <DependentUpon>Strings.resx</DependentUpon>
     </Compile>
+    <Compile Include="TokenOperations.cs" />
     <Compile Include="VariableAnalysis.cs" />
     <Compile Include="VariableAnalysisBase.cs" />
   </ItemGroup>

Engine/Settings/CodeFormatting.psd1

@@ -0,0 +1,24 @@
+@{
+    IncludeRules = @(
+        'PSPlaceOpenBrace',
+        'PSPlaceCloseBrace',
+        'PSUseConsistentIndentation'
+    )
+
+    Rules = @{
+        PSPlaceOpenBrace = @{
+            Enable = $true
+            OnSameLine = $true
+            NewLineAfter = $true
+        }
+
+        PSPlaceCloseBrace = @{
+            Enable = $true
+        }
+
+        PSUseConsistentIndentation = @{
+            Enable = $true
+            IndentationSize = 4
+        }
+    }
+}

Engine/Strings.resx

@@ -216,6 +216,9 @@
   <data name="VerboseScriptDefinitionMessage" xml:space="preserve">
     <value>Analyzing Script Definition.</value>
   </data>
+  <data name="ConfigurationFileHasInvalidHashtable" xml:space="preserve">
+    <value>SettingsFileHasInvalidHashtable</value>
+  </data>
   <data name="ConfigurationFileHasNoHashTable" xml:space="preserve">
     <value>SettingsFileHasNoHashTable</value>
   </data>
@@ -252,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>