Skip to content

Commit 3bfd45b

Browse files
Annotatively research current state of exception handling surrounding settings file load invocation
1 parent e206b32 commit 3bfd45b

File tree

3 files changed

+38
-1
lines changed

3 files changed

+38
-1
lines changed

Engine/Commands/InvokeScriptAnalyzerCommand.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@ protected override void BeginProcessing()
268268
Helper.Instance = new Helper(
269269
SessionState.InvokeCommand,
270270
this);
271+
// NOTE Helper.Instance.Initialize() does *not* modify this.settings.
271272
Helper.Instance.Initialize();
272273

273274
var psVersionTable = this.SessionState.PSVariable.GetValue("PSVersionTable") as Hashtable;
@@ -276,6 +277,7 @@ protected override void BeginProcessing()
276277
Helper.Instance.SetPSVersionTable(psVersionTable);
277278
}
278279

280+
// NOTE Helper.ProcessCustomRulePaths does *not* modify this.settings.
279281
string[] rulePaths = Helper.ProcessCustomRulePaths(
280282
customRulePath,
281283
this.SessionState,
@@ -284,6 +286,7 @@ protected override void BeginProcessing()
284286
if (IsFileParameterSet() && Path != null)
285287
{
286288
// just used to obtain the directory to use to find settings below
289+
// NOTE ProcessPath() does *not* modify this.settings.
287290
ProcessPath();
288291
}
289292

@@ -292,17 +295,24 @@ protected override void BeginProcessing()
292295
var combIncludeDefaultRules = IncludeDefaultRules.IsPresent;
293296
try
294297
{
298+
// THROW PSSASettings.Create(object, string, IOutputWriter, GetResolvedProviderPathFromPSPath) throws if TODO
299+
// NOTE Microsoft.Windows.PowerShell.ScriptAnalyzer.Settings.Create(...) types the input, gets the input (if necessary), and parses it (if any).
295300
var settingsObj = PSSASettings.Create(
301+
// NOTHROW A null this.settings results in returning an "empty" (but not null) settingsObj without exception.
302+
// NOTE this.settings is unmodified since start of BeginProcessing(). Thus, it is exactly the raw argument value, if any.
296303
settings,
297304
processedPaths == null || processedPaths.Count == 0 ? CurrentProviderLocation("FileSystem").ProviderPath : processedPaths[0],
298305
this,
299306
GetResolvedProviderPathFromPSPath);
307+
// NOTE settingsObj cannot be null here since PSSASettings.Create(...) returns exactly `new Settings(settingsFound)`, which can never be null (but can throw).
300308
if (settingsObj != null)
301309
{
310+
// NOTHROW UpdateSettings can throw an ArgumentNullException, but that will never happen since settingsObj is tested for non-nullity immediately above.
302311
ScriptAnalyzer.Instance.UpdateSettings(settingsObj);
303312

304313
// For includeDefaultRules and RecurseCustomRulePath we override the value in the settings file by
305314
// command line argument.
315+
// NOTHROW InvokeScriptAnalyzerCommand.OverrideSwitchParam(bool, string)
306316
combRecurseCustomRulePath = OverrideSwitchParam(
307317
settingsObj.RecurseCustomRulePath,
308318
"RecurseCustomRulePath");
@@ -315,6 +325,7 @@ protected override void BeginProcessing()
315325
// simultaneously. But since, this was done before with IncludeRules, ExcludeRules and Severity,
316326
// we use the same strategy for CustomRulePath. So, we take the union of CustomRulePath provided in
317327
// the settings file and if provided on command line.
328+
// 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.
318329
var settingsCustomRulePath = Helper.ProcessCustomRulePaths(
319330
settingsObj?.CustomRulePath?.ToArray(),
320331
this.SessionState,
@@ -327,6 +338,7 @@ protected override void BeginProcessing()
327338
}
328339
catch
329340
{
341+
// NOTE Any exception in resolving, getting, parsing, updating, etc. the settings herein results in an contextless WriteWarning(Strings.SettingsNotParsable), regardless of provenance.
330342
this.WriteWarning(String.Format(CultureInfo.CurrentCulture, Strings.SettingsNotParsable));
331343
stopProcessing = true;
332344
return;

Engine/Helper.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1506,6 +1506,12 @@ public static string[] ProcessCustomRulePaths(string[] rulePaths, SessionState s
15061506
Collection<PathInfo> pathInfo = new Collection<PathInfo>();
15071507
foreach (string rulePath in rulePaths)
15081508
{
1509+
// THROW PathIntrinsics.GetResolvedPSPathFromPSPath(rulePath) throws [System.Management.Automation.ProviderNotFoundException] if path is a provider-qualified path and the specified provider does not exist.
1510+
// THROW PathIntrinsics.GetResolvedPSPathFromPSPath(rulePath) throws [System.Management.Automation.DriveNotFoundException] if path is a drive-qualified path and the specified drive does not exist.
1511+
// THROW PathIntrinsics.GetResolvedPSPathFromPSPath(rulePath) throws [System.Management.Automation.ProviderInvocationException] if the provider throws an exception when its MakePath gets called.
1512+
// THROW PathIntrinsics.GetResolvedPSPathFromPSPath(rulePath) throws [System.NotSupportedException] if the provider does not support multiple items.
1513+
// THROW PathIntrinsics.GetResolvedPSPathFromPSPath(rulePath) throws [System.InvalidOperationException] the home location for the provider is not set and path starts with a "~".
1514+
// THROW PathIntrinsics.GetResolvedPSPathFromPSPath(rulePath) throws [System.Management.Automation.ItemNotFoundException] if path does not contain wildcard characters and could not be found.
15091515
Collection<PathInfo> pathInfosForRulePath = sessionState.Path.GetResolvedPSPathFromPSPath(rulePath);
15101516
if (null != pathInfosForRulePath)
15111517
{

Engine/Settings.cs

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,12 @@ public Settings(object settings, Func<string, string> presetResolver)
7373
if (File.Exists(settingsFilePath))
7474
{
7575
filePath = settingsFilePath;
76+
// QUESTION When does parseSettingsFile(string) throw?
7677
parseSettingsFile(settingsFilePath);
7778
}
7879
else
7980
{
81+
// 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)
8082
throw new ArgumentException(
8183
String.Format(
8284
CultureInfo.CurrentCulture,
@@ -89,10 +91,12 @@ public Settings(object settings, Func<string, string> presetResolver)
8991
var settingsHashtable = settings as Hashtable;
9092
if (settingsHashtable != null)
9193
{
94+
// QUESTION When does parseSettingsHashtable(Hashtable) throw?
9295
parseSettingsHashtable(settingsHashtable);
9396
}
9497
else
9598
{
99+
// THROW ArgumentException(Strings.SettingsInvalidType) if settings is not convertible (`as` keyword) to Hashtable.
96100
throw new ArgumentException(Strings.SettingsInvalidType);
97101
}
98102
}
@@ -179,11 +183,12 @@ public static string GetSettingPresetFilePath(string settingPreset)
179183
/// <param name="outputWriter">An output writer.</param>
180184
/// <param name="getResolvedProviderPathFromPSPathDelegate">The GetResolvedProviderPathFromPSPath method from PSCmdlet to resolve relative path including wildcard support.</param>
181185
/// <returns>An object of Settings type.</returns>
186+
// THROW Settings.Create(object, string, IOutputWriter, GetResolvedProviderPathFromPSPath) throws only because of the contained invocation to the Settings constructor, which does throw.
182187
internal static Settings Create(object settingsObj, string cwd, IOutputWriter outputWriter,
183188
PathResolver.GetResolvedProviderPathFromPSPath<string, ProviderInfo, Collection<string>> getResolvedProviderPathFromPSPathDelegate)
184189
{
185190
object settingsFound;
186-
var settingsMode = FindSettingsMode(settingsObj, cwd, out settingsFound);
191+
var settingsMode = FindSettingsMode(settingsObj, cwd, out settingsFound); // NOTHROW
187192

188193
switch (settingsMode)
189194
{
@@ -205,6 +210,7 @@ internal static Settings Create(object settingsObj, string cwd, IOutputWriter ou
205210
var userProvidedSettingsString = settingsFound.ToString();
206211
try
207212
{
213+
// THROW ..Single() throws [System.InvalidOperationException] if the settings path does not resolve to exactly one item. (more or less)
208214
var resolvedPath = getResolvedProviderPathFromPSPathDelegate(userProvidedSettingsString, out ProviderInfo providerInfo).Single();
209215
settingsFound = resolvedPath;
210216
outputWriter?.WriteVerbose(
@@ -213,8 +219,11 @@ internal static Settings Create(object settingsObj, string cwd, IOutputWriter ou
213219
Strings.SettingsUsingFile,
214220
resolvedPath));
215221
}
222+
// 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.
216223
catch
217224
{
225+
// TODO Upgrade WriteVerbose(Strings.SettingsCannotFindFile) to WriteWarning(Strings.SettingsCannotFindFile).
226+
// TODO Perform a ShouldContinue (?) confirmation check in the event that an exception occurred while resolving the settings file path.
218227
outputWriter?.WriteVerbose(
219228
String.Format(
220229
CultureInfo.CurrentCulture,
@@ -238,6 +247,7 @@ internal static Settings Create(object settingsObj, string cwd, IOutputWriter ou
238247
return null;
239248
}
240249

250+
// THROW new Settings(object)
241251
return new Settings(settingsFound);
242252
}
243253

@@ -456,16 +466,19 @@ private void parseSettingsHashtable(Hashtable settingsHashtable)
456466
}
457467
}
458468

469+
// NOTE parseSettingsFile concludes by calling parseSettingsHashtable if it (parseSettingsFile) is successful.
459470
private void parseSettingsFile(string settingsFilePath)
460471
{
461472
Token[] parserTokens = null;
462473
ParseError[] parserErrors = null;
474+
// QUESTION Can Parser.ParseFile throw?
463475
Ast profileAst = Parser.ParseFile(settingsFilePath, out parserTokens, out parserErrors);
464476
IEnumerable<Ast> hashTableAsts = profileAst.FindAll(item => item is HashtableAst, false);
465477

466478
// no hashtable, raise warning
467479
if (hashTableAsts.Count() == 0)
468480
{
481+
// THROW parseSettingsFile throws ArgumentException(Strings.InvalidProfile) if no hashTableAst is detected in settings file.
469482
throw new ArgumentException(string.Format(CultureInfo.CurrentCulture, Strings.InvalidProfile, settingsFilePath));
470483
}
471484

@@ -475,22 +488,26 @@ private void parseSettingsFile(string settingsFilePath)
475488
{
476489
// ideally we should use HashtableAst.SafeGetValue() but since
477490
// it is not available on PSv3, we resort to our own narrow implementation.
491+
// QUESTION When does GetSafeValueFromHashtableAst(hashTableAst) throw?
478492
hashtable = GetSafeValueFromHashtableAst(hashTableAst);
479493
}
480494
catch (InvalidOperationException e)
481495
{
496+
// THROW parseSettingsFile throws ArgumentException(Strings.InvalidProfile) if GetSafeValueFromHashtableAst(hashTableAst) throws an InvalidOperationException.
482497
throw new ArgumentException(Strings.InvalidProfile, e);
483498
}
484499

485500
if (hashtable == null)
486501
{
502+
// THROW parseSettingsFile throws ArgumentException if GetSafeValueFromHashtableAst(hashTableAst) returns null.
487503
throw new ArgumentException(
488504
String.Format(
489505
CultureInfo.CurrentCulture,
490506
Strings.InvalidProfile,
491507
settingsFilePath));
492508
}
493509

510+
// QUESTION When does parseSettingsHashtable(Hashtable) throw?
494511
parseSettingsHashtable(hashtable);
495512
}
496513

@@ -686,6 +703,7 @@ private static bool IsBuiltinSettingPreset(object settingPreset)
686703
return false;
687704
}
688705

706+
// NOTHROW FindSettingsMode(object, string, out object)
689707
internal static SettingsMode FindSettingsMode(object settings, string path, out object settingsFound)
690708
{
691709
var settingsMode = SettingsMode.None;
@@ -706,6 +724,7 @@ internal static SettingsMode FindSettingsMode(object settings, string path, out
706724
{
707725
// if settings are not provided explicitly, look for it in the given path
708726
// check if pssasettings.psd1 exists
727+
// 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)
709728
var settingsFilename = "PSScriptAnalyzerSettings.psd1";
710729
var settingsFilePath = Path.Combine(directory, settingsFilename);
711730
settingsFound = settingsFilePath;

0 commit comments

Comments
 (0)