Revert "Annotatively research current state of exception handling surrounding settings file load invocation"

This reverts commit 3bfd45b.

Author: travisclagrone

Engine/Commands/InvokeScriptAnalyzerCommand.cs

@@ -269,7 +269,6 @@ protected override void BeginProcessing()
             Helper.Instance = new Helper(
                 SessionState.InvokeCommand,
                 this);
-            // NOTE Helper.Instance.Initialize() does *not* modify this.settings.
             Helper.Instance.Initialize();
 
             var psVersionTable = this.SessionState.PSVariable.GetValue("PSVersionTable") as Hashtable;
@@ -278,7 +277,6 @@ protected override void BeginProcessing()
                 Helper.Instance.SetPSVersionTable(psVersionTable);
             }
 
-            // NOTE Helper.ProcessCustomRulePaths does *not* modify this.settings.
             string[] rulePaths = Helper.ProcessCustomRulePaths(
                 customRulePath,
                 this.SessionState,
@@ -287,7 +285,6 @@ protected override void BeginProcessing()
             if (IsFileParameterSet() && Path != null)
             {
                 // just used to obtain the directory to use to find settings below
-                // NOTE ProcessPath() does *not* modify this.settings.
                 ProcessPath();
             }
 
@@ -296,24 +293,17 @@ protected override void BeginProcessing()
             var combIncludeDefaultRules = IncludeDefaultRules.IsPresent;
             try
             {
-                // THROW PSSASettings.Create(object, string, IOutputWriter, GetResolvedProviderPathFromPSPath) throws if ...
-                // NOTE Microsoft.Windows.PowerShell.ScriptAnalyzer.Settings.Create(...) types the input, gets the input (if necessary), and parses it (if any).
                 var settingsObj = PSSASettings.Create(
-                    // NOTHROW A null this.settings results in returning an "empty" (but not null) settingsObj without exception.
-                    // NOTE this.settings is unmodified since start of BeginProcessing(). Thus, it is exactly the raw argument value, if any.
                     settings,
                     processedPaths == null || processedPaths.Count == 0 ? CurrentProviderLocation("FileSystem").ProviderPath : processedPaths[0],
                     this,
                     GetResolvedProviderPathFromPSPath);
-                // NOTE settingsObj cannot be null here since PSSASettings.Create(...) returns exactly `new Settings(settingsFound)`, which can never be null (but can throw).
                 if (settingsObj != null)
                 {
-                    // NOTHROW UpdateSettings(object) can throw an ArgumentNullException, but that will never happen since settingsObj is tested for non-nullity immediately above.
                     ScriptAnalyzer.Instance.UpdateSettings(settingsObj);
 
                     // For includeDefaultRules and RecurseCustomRulePath we override the value in the settings file by
                     // command line argument.
-                    // NOTHROW InvokeScriptAnalyzerCommand.OverrideSwitchParam(bool, string)
                     combRecurseCustomRulePath = OverrideSwitchParam(
                         settingsObj.RecurseCustomRulePath,
                         "RecurseCustomRulePath");
@@ -326,7 +316,6 @@ protected override void BeginProcessing()
                 // simultaneously. But since, this was done before with IncludeRules, ExcludeRules and Severity,
                 // we use the same strategy for CustomRulePath. So, we take the union of CustomRulePath provided in
                 // the settings file and if provided on command line.
-                // THROW Helper.ProcessCustomRulePaths(string[], SessionState, bool) throws one of six different exceptions if a settings' custom rule path is invalid somehow (e.g. drive doesn't exit; no wildcards but item doesn't exist; provider throws a lower-level exception; etc.). See the implementation of Helper.ProcessCustomRulePaths(string[], SessionState, bool) for details.
                 var settingsCustomRulePath = Helper.ProcessCustomRulePaths(
                         settingsObj?.CustomRulePath?.ToArray(),
                         this.SessionState,
@@ -339,7 +328,6 @@ protected override void BeginProcessing()
             }
             catch (Exception e)
             {
-                // NOTE Any exception in resolving, getting, parsing, updating, etc. the settings herein results in an contextless WriteWarning(Strings.SettingsNotParsable), regardless of provenance.
                 string errorId;
                 ErrorCategory errorCategory;
                 switch (e)

Engine/Helper.cs

@@ -1506,12 +1506,6 @@ public static string[] ProcessCustomRulePaths(string[] rulePaths, SessionState s
             Collection<PathInfo> pathInfo = new Collection<PathInfo>();
             foreach (string rulePath in rulePaths)
             {
-                // THROW PathIntrinsics.GetResolvedPSPathFromPSPath(rulePath) throws [System.Management.Automation.ProviderNotFoundException] if path is a provider-qualified path and the specified provider does not exist.
-                // THROW PathIntrinsics.GetResolvedPSPathFromPSPath(rulePath) throws [System.Management.Automation.DriveNotFoundException] if path is a drive-qualified path and the specified drive does not exist.
-                // THROW PathIntrinsics.GetResolvedPSPathFromPSPath(rulePath) throws [System.Management.Automation.ProviderInvocationException] if the provider throws an exception when its MakePath gets called.
-                // THROW PathIntrinsics.GetResolvedPSPathFromPSPath(rulePath) throws [System.NotSupportedException] if the provider does not support multiple items.
-                // THROW PathIntrinsics.GetResolvedPSPathFromPSPath(rulePath) throws [System.InvalidOperationException] the home location for the provider is not set and path starts with a "~".
-                // THROW PathIntrinsics.GetResolvedPSPathFromPSPath(rulePath) throws [System.Management.Automation.ItemNotFoundException] if path does not contain wildcard characters and could not be found.
                 Collection<PathInfo> pathInfosForRulePath = sessionState.Path.GetResolvedPSPathFromPSPath(rulePath);
                 if (null != pathInfosForRulePath)
                 {

Engine/Settings.cs

@@ -73,12 +73,10 @@ public Settings(object settings, Func<string, string> presetResolver)
                 if (File.Exists(settingsFilePath))
                 {
                     filePath = settingsFilePath;
-                    // QUESTION When does parseSettingsFile(string) throw?
                     parseSettingsFile(settingsFilePath);
                 }
                 else
                 {
-                    // THROW ArgumentException(Strings.InvalidPath) if the resolution of the settings argument via GetResolvedProviderPathFromPSPathDelegate is non-null but not an existing file. (e.g. it may be a directory)
                     throw new ArgumentException(
                         String.Format(
                             CultureInfo.CurrentCulture,
@@ -91,12 +89,10 @@ public Settings(object settings, Func<string, string> presetResolver)
                 var settingsHashtable = settings as Hashtable;
                 if (settingsHashtable != null)
                 {
-                    // QUESTION When does parseSettingsHashtable(Hashtable) throw?
                     parseSettingsHashtable(settingsHashtable);
                 }
                 else
                 {
-                    // THROW ArgumentException(Strings.SettingsInvalidType) if settings is not convertible (`as` keyword) to Hashtable.
                     throw new ArgumentException(Strings.SettingsInvalidType);
                 }
             }
@@ -183,12 +179,11 @@ public static string GetSettingPresetFilePath(string settingPreset)
         /// <param name="outputWriter">An output writer.</param>
         /// <param name="getResolvedProviderPathFromPSPathDelegate">The GetResolvedProviderPathFromPSPath method from PSCmdlet to resolve relative path including wildcard support.</param>
         /// <returns>An object of Settings type.</returns>
-        // THROW Settings.Create(object, string, IOutputWriter, GetResolvedProviderPathFromPSPath) throws only because of the contained invocation to the Settings constructor, which does throw.
         internal static Settings Create(object settingsObj, string cwd, IOutputWriter outputWriter,
             PathResolver.GetResolvedProviderPathFromPSPath<string, ProviderInfo, Collection<string>> getResolvedProviderPathFromPSPathDelegate)
         {
             object settingsFound;
-            var settingsMode = FindSettingsMode(settingsObj, cwd, out settingsFound);  // NOTHROW
+            var settingsMode = FindSettingsMode(settingsObj, cwd, out settingsFound);
 
             switch (settingsMode)
             {
@@ -210,7 +205,6 @@ internal static Settings Create(object settingsObj, string cwd, IOutputWriter ou
                     var userProvidedSettingsString = settingsFound.ToString();
                     try
                     {
-                        // THROW ..Single() throws [System.InvalidOperationException] if the settings path does not resolve to exactly one item. (more or less)
                         var resolvedPath = getResolvedProviderPathFromPSPathDelegate(userProvidedSettingsString, out ProviderInfo providerInfo).Single();
                         settingsFound = resolvedPath;
                         outputWriter?.WriteVerbose(
@@ -219,11 +213,8 @@ internal static Settings Create(object settingsObj, string cwd, IOutputWriter ou
                                 Strings.SettingsUsingFile,
                                 resolvedPath));
                     }
-                    // NOTE This catch block effectively makes *any* exception resulting from resolving the settings file path reduce to the case of no settings (by way of null settingsFound). Only a WriteVerbose(Strings.SettingsCannotFindFile) identifies what happened.
                     catch
                     {
-                        // TODO Upgrade WriteVerbose(Strings.SettingsCannotFindFile) to WriteWarning(Strings.SettingsCannotFindFile).
-                        // TODO Perform a ShouldContinue (?) confirmation check in the event that an exception occurred while resolving the settings file path.
                         outputWriter?.WriteVerbose(
                             String.Format(
                                 CultureInfo.CurrentCulture,
@@ -247,7 +238,6 @@ internal static Settings Create(object settingsObj, string cwd, IOutputWriter ou
                     return null;
             }
 
-            // THROW new Settings(object)
             return new Settings(settingsFound);
         }
 
@@ -466,19 +456,16 @@ private void parseSettingsHashtable(Hashtable settingsHashtable)
             }
         }
 
-        // NOTE parseSettingsFile concludes by calling parseSettingsHashtable if it (parseSettingsFile) is successful.
         private void parseSettingsFile(string settingsFilePath)
         {
             Token[] parserTokens = null;
             ParseError[] parserErrors = null;
-            // QUESTION Can Parser.ParseFile throw?
             Ast profileAst = Parser.ParseFile(settingsFilePath, out parserTokens, out parserErrors);
             IEnumerable<Ast> hashTableAsts = profileAst.FindAll(item => item is HashtableAst, false);
 
             // no hashtable, raise warning
             if (hashTableAsts.Count() == 0)
             {
-                // THROW parseSettingsFile throws ArgumentException(Strings.InvalidProfile) if no hashTableAst is detected in settings file.
                 throw new ArgumentException(string.Format(CultureInfo.CurrentCulture, Strings.InvalidProfile, settingsFilePath));
             }
 
@@ -488,26 +475,22 @@ private void parseSettingsFile(string settingsFilePath)
             {
                 // ideally we should use HashtableAst.SafeGetValue() but since
                 // it is not available on PSv3, we resort to our own narrow implementation.
-                // QUESTION When does GetSafeValueFromHashtableAst(hashTableAst) throw?
                 hashtable = GetSafeValueFromHashtableAst(hashTableAst);
             }
             catch (InvalidOperationException e)
             {
-                // THROW parseSettingsFile throws ArgumentException(Strings.InvalidProfile) if GetSafeValueFromHashtableAst(hashTableAst) throws an InvalidOperationException.
                 throw new ArgumentException(Strings.InvalidProfile, e);
             }
 
             if (hashtable == null)
             {
-                // THROW parseSettingsFile throws ArgumentException if GetSafeValueFromHashtableAst(hashTableAst) returns null.
                 throw new ArgumentException(
                     String.Format(
                         CultureInfo.CurrentCulture,
                         Strings.InvalidProfile,
                         settingsFilePath));
             }
 
-            // QUESTION When does parseSettingsHashtable(Hashtable) throw?
             parseSettingsHashtable(hashtable);
         }
 
@@ -703,7 +686,6 @@ private static bool IsBuiltinSettingPreset(object settingPreset)
             return false;
         }
 
-        // NOTHROW FindSettingsMode(object, string, out object)
         internal static SettingsMode FindSettingsMode(object settings, string path, out object settingsFound)
         {
             var settingsMode = SettingsMode.None;
@@ -724,7 +706,6 @@ internal static SettingsMode FindSettingsMode(object settings, string path, out
                     {
                         // if settings are not provided explicitly, look for it in the given path
                         // check if pssasettings.psd1 exists
-                        // COMBAK Refactor the magic string literal "PSScriptAnalyzerSettings.psd1" to a public const field on the class. (bare minimum... although will also help open possibility for user-configurable default name)
                         var settingsFilename = "PSScriptAnalyzerSettings.psd1";
                         var settingsFilePath = Path.Combine(directory, settingsFilename);
                         settingsFound = settingsFilePath;