From cba1f648b0354a0c7e66fa51ec13011d0633a0e4 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 27 Apr 2016 21:17:12 -0700 Subject: [PATCH 01/23] Add module dependency handler for missing modules Whenever Invoke-ScriptAnalyzer (isa) is run on a script having the dynamic keyword "Import-DSCResource -ModuleName ", if is not present in any of the PSModulePath, isa gives parse error. This error is caused by the powershell parser not being able to find the symbol for . --- Engine/Generic/ModuleDependencyHandler.cs | 232 ++++++++++++++++++++++ Engine/ScriptAnalyzer.cs | 53 ++++- Engine/ScriptAnalyzerEngine.csproj | 37 ++-- 3 files changed, 302 insertions(+), 20 deletions(-) create mode 100644 Engine/Generic/ModuleDependencyHandler.cs diff --git a/Engine/Generic/ModuleDependencyHandler.cs b/Engine/Generic/ModuleDependencyHandler.cs new file mode 100644 index 000000000..7c21f2a4e --- /dev/null +++ b/Engine/Generic/ModuleDependencyHandler.cs @@ -0,0 +1,232 @@ +using System; +using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.IO; +using System.Linq; +using System.Management.Automation; +using System.Management.Automation.Language; +using System.Management.Automation.Runspaces; +using System.Text; + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic +{ + // TODO Use runspace pool + // TODO Create a new process for the runspace + public class ModuleDependencyHandler : IDisposable + { + #region Private Variables + private Runspace runspace; + private readonly string moduleRepository; + private string tempDirPath; + Dictionary modulesFound; + HashSet modulesSaved; + private string oldPSModulePath; + private string currentModulePath; + + #endregion Private Variables + + #region Properties + public string ModulePath + { + get { return tempDirPath; } + } + + public Runspace Runspace + { + get { return runspace; } + } + + #endregion + + #region Private Methods + private static void ThrowIfNull(T obj, string name) + { + if (obj == null) + { + throw new ArgumentNullException(name); + } + } + private void SetupTempDir() + { + //var tempPath = Path.GetTempPath(); + //do + //{ + // tempDirPath = Path.Combine( + // tempPath, + // Path.GetFileNameWithoutExtension(Path.GetRandomFileName())); + //} while (Directory.Exists(tempDirPath)); + //Directory.CreateDirectory(tempDirPath); + tempDirPath = "C:\\Users\\kabawany\\tmp\\modules\\"; + } + + private void RemoveTempDir() + { + //Directory.Delete(tempDirPath, true); + } + + private void SetupPSModulePath() + { + oldPSModulePath = Environment.GetEnvironmentVariable("PSModulePath", EnvironmentVariableTarget.Process); + var sb = new StringBuilder(); + sb.Append(oldPSModulePath) + .Append(Path.DirectorySeparatorChar) + .Append(tempDirPath); + currentModulePath = sb.ToString(); + } + + private void CleanUp() + { + runspace.Dispose(); + RemoveTempDir(); + RestorePSModulePath(); + } + + private void RestorePSModulePath() + { + Environment.SetEnvironmentVariable("PSModulePath", oldPSModulePath, EnvironmentVariableTarget.Process); + } + + private void SaveModule(PSObject module) + { + ThrowIfNull(module, "module"); + // TODO validate module + var ps = System.Management.Automation.PowerShell.Create(); + ps.Runspace = runspace; + ps.AddCommand("Save-Module") + .AddParameter("Path", tempDirPath) + .AddParameter("InputObject", module); + ps.Invoke(); + } + + #endregion Private Methods + + #region Public Methods + + public ModuleDependencyHandler() + { + runspace = null; + moduleRepository = "PSGallery"; + modulesSaved = new HashSet(); + modulesFound = new Dictionary(); + SetupTempDir(); + //SetupPSModulePath(); + } + + public ModuleDependencyHandler(Runspace runspace) : this() + { + ThrowIfNull(runspace, "runspace"); + this.runspace = runspace; + this.runspace.Open(); + } + + + public void SetupDefaultRunspace() + { + runspace = RunspaceFactory.CreateRunspace(); + } + + public void SetupDefaultRunspace(Runspace runspace) + { + ThrowIfNull(runspace, "runspace"); + if (runspace != null) + { + this.runspace = runspace; + } + Runspace.DefaultRunspace = this.runspace; + } + + public PSObject FindModule(string moduleName) + { + ThrowIfNull(moduleName, "moduleName"); + if (modulesFound.ContainsKey(moduleName)) + { + return modulesFound[moduleName]; + } + var ps = System.Management.Automation.PowerShell.Create(); + Collection modules = null; + ps.Runspace = runspace; + ps.AddCommand("Find-Module", true) + .AddParameter("Name", moduleName) + .AddParameter("Repository", moduleRepository); + modules = ps.Invoke(); + + if (modules == null) + { + return null; + } + var module = modules.FirstOrDefault(); + if (module == null ) + { + return null; + } + modulesFound.Add(moduleName, module); + return module; + } + + public void SaveModule(string moduleName) + { + ThrowIfNull(moduleName, "moduleName"); + if (modulesSaved.Contains(moduleName)) + { + return; + } + var module = FindModule(moduleName); + if (module == null) + { + throw new ItemNotFoundException( + string.Format( + "Cannot find {0} in {1} repository.", + moduleName, + moduleRepository)); + } + SaveModule(module); + modulesSaved.Add(moduleName); + } + + public static string GetModuleNameFromErrorExtent(ParseError error, ScriptBlockAst ast) + { + ThrowIfNull(error, "error"); + ThrowIfNull(ast, "ast"); + var statement = ast.Find(x => x.Extent.Equals(error.Extent), true); + var dynamicKywdAst = statement as DynamicKeywordStatementAst; + if (dynamicKywdAst == null) + { + return null; + } + // check if the command name is import-dscmodule + // right now we handle only the following form + // Import-DSCResource -ModuleName xActiveDirectory + if (dynamicKywdAst.CommandElements.Count != 3) + { + return null; + } + + var dscKeywordAst = dynamicKywdAst.CommandElements[0] as StringConstantExpressionAst; + if (dscKeywordAst == null || !dscKeywordAst.Value.Equals("Import-DscResource", StringComparison.OrdinalIgnoreCase)) + { + return null; + } + + var paramAst = dynamicKywdAst.CommandElements[1] as CommandParameterAst; + if (paramAst == null || !paramAst.ParameterName.Equals("ModuleName", StringComparison.OrdinalIgnoreCase)) + { + return null; + } + + var paramValAst = dynamicKywdAst.CommandElements[2] as StringConstantExpressionAst; + if (paramValAst == null) + { + return null; + } + + return paramValAst.Value; + } + + public void Dispose() + { + CleanUp(); + } + + #endregion Public Methods + } +} diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 8dd1fb82b..60383d46f 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -45,6 +45,8 @@ public sealed class ScriptAnalyzer List includeRegexList; List excludeRegexList; bool suppressedOnly; + Runspace runspace; + ModuleDependencyHandler moduleHandler; #endregion @@ -165,7 +167,8 @@ public void CleanUp() severity = null; includeRegexList = null; excludeRegexList = null; - suppressedOnly = false; + suppressedOnly = false; + moduleHandler.Dispose(); } internal bool ParseProfile(object profileObject, PathIntrinsics path, IOutputWriter writer) @@ -476,6 +479,10 @@ private void Initialize( } this.outputWriter = outputWriter; + + // TODO Create a runspace pool + runspace = RunspaceFactory.CreateRunspace(); + moduleHandler = new ModuleDependencyHandler(runspace); #region Verifies rule extensions and loggers path @@ -1271,8 +1278,9 @@ private void BuildScriptPathList( ErrorCategory.InvalidArgument, this)); } - } + } + private IEnumerable AnalyzeFile(string filePath) { ScriptBlockAst scriptAst = null; @@ -1297,6 +1305,41 @@ private IEnumerable AnalyzeFile(string filePath) return null; } + + // TODO Handle Parse Errors causes by missing modules + // if errors are due to ModuleNotFoundDuringParse + // - EITHER + // - create a runspace and set the default runspace (make sure backup the default runspace before modifying it) + // - check if it is present in the gallery + // - if present on the gallery, save it to a temporary file, and load the module + // - now parse again + // - if parse is successful, proceed + // - else inform the user of the issue + // - OR + // - swallow the these errors + + + if (errors != null && errors.Length > 0) + { + foreach (ParseError error in errors) + { + if (IsModuleNotFoundError(error)) + { + var moduleName = ModuleDependencyHandler.GetModuleNameFromErrorExtent(error, scriptAst); + if (moduleName != null) + { + moduleHandler.SaveModule(moduleName); + } + } + } + } + + //try parsing again + //var oldDefault = Runspace.DefaultRunspace; + //Runspace.DefaultRunspace = moduleHandler.Runspace; + scriptAst = Parser.ParseFile(filePath, out scriptTokens, out errors); + //Runspace.DefaultRunspace = oldDefault; + if (errors != null && errors.Length > 0) { foreach (ParseError error in errors) @@ -1327,6 +1370,12 @@ private IEnumerable AnalyzeFile(string filePath) return this.AnalyzeSyntaxTree(scriptAst, scriptTokens, filePath); } + private bool IsModuleNotFoundError(ParseError error) + { + return error.ErrorId != null + && error.ErrorId.Equals("ModuleNotFoundDuringParse", StringComparison.OrdinalIgnoreCase); + } + private bool IsSeverityAllowed(IEnumerable allowedSeverities, IRule rule) { return severity == null diff --git a/Engine/ScriptAnalyzerEngine.csproj b/Engine/ScriptAnalyzerEngine.csproj index 368214b56..35d9ce7da 100644 --- a/Engine/ScriptAnalyzerEngine.csproj +++ b/Engine/ScriptAnalyzerEngine.csproj @@ -34,23 +34,23 @@ Microsoft.Windows.PowerShell.ScriptAnalyzer - true - bin\PSV3 Debug\ - TRACE;DEBUG;PSV3 - full - AnyCPU - prompt - MinimumRecommendedRules.ruleset - - - bin\PSV3 Release\ - TRACE;PSV3 - true - pdbonly - AnyCPU - prompt - MinimumRecommendedRules.ruleset - + true + bin\PSV3 Debug\ + TRACE;DEBUG;PSV3 + full + AnyCPU + prompt + MinimumRecommendedRules.ruleset + + + bin\PSV3 Release\ + TRACE;PSV3 + true + pdbonly + AnyCPU + prompt + MinimumRecommendedRules.ruleset + @@ -70,6 +70,7 @@ + @@ -101,7 +102,7 @@ - + From f3cde078d56ddc0ddb5a509df7ae0520fc4b805c Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 28 Apr 2016 17:40:30 -0700 Subject: [PATCH 02/23] Fix parse error caused by missing dsc module One solution to solve this is to download the missing modules to a temporary path and then add that temporary path to PSModulePath when isa is run and remove the temp path when isa is finished running. But at this iteration this approach doesn't seem to work and needs further investigation. The solution, that we pursue, is to download the missing module to a temp path. This temp path and its data is persistent in the sense that the temp path information is stored in $LOCALAPPDATA/PSScriptAnalyzer (pssaappdata). Whenver isa in invoked, it checks the temp path pointed in pssaappdata, checks if the modules are present in the temp directory and if so, copies them to the user psmodule path, which is typcially $HOME/Documents/WindowsPowerShell/Modules. And, when isa is finished running we remove those copied modues from the user psmodule path. --- Engine/Generic/ModuleDependencyHandler.cs | 182 ++++++++++++++++++---- 1 file changed, 148 insertions(+), 34 deletions(-) diff --git a/Engine/Generic/ModuleDependencyHandler.cs b/Engine/Generic/ModuleDependencyHandler.cs index 7c21f2a4e..1efcb971f 100644 --- a/Engine/Generic/ModuleDependencyHandler.cs +++ b/Engine/Generic/ModuleDependencyHandler.cs @@ -18,10 +18,15 @@ public class ModuleDependencyHandler : IDisposable private Runspace runspace; private readonly string moduleRepository; private string tempDirPath; + private string localPSModulePath; Dictionary modulesFound; - HashSet modulesSaved; - private string oldPSModulePath; - private string currentModulePath; + HashSet modulesSavedInModulePath; + HashSet modulesSavedInTempPath; + private string localAppdataPath; + private string pssaAppdataPath; + private const string symLinkName = "TempModuleDir"; + private const string tempPrefix = "PSSAModules-"; + private string symLinkPath; #endregion Private Variables @@ -36,7 +41,7 @@ public Runspace Runspace get { return runspace; } } - #endregion + #endregion Properties #region Private Methods private static void ThrowIfNull(T obj, string name) @@ -46,44 +51,96 @@ private static void ThrowIfNull(T obj, string name) throw new ArgumentNullException(name); } } + + private void SetupCache() + { + // check if pssa exists in local appdata + if (Directory.Exists(pssaAppdataPath)) + { + // check if there is a link + if (File.Exists(symLinkPath)) + { + tempDirPath = GetTempDirPath(symLinkPath); + + // check if the temp dir exists + if (tempDirPath != null + && Directory.Exists(tempDirPath)) + { + SetModulesInTempPath(); + return; + } + } + SetupTempDir(); + } + else + { + Directory.CreateDirectory(pssaAppdataPath); + SetupTempDir(); + } + } + + private void SetModulesInTempPath() + { + // we assume the modules have not been tampered with + foreach (var dir in Directory.EnumerateDirectories(tempDirPath)) + { + modulesSavedInTempPath.Add(Path.GetFileName(dir)); + } + } + private void SetupTempDir() { - //var tempPath = Path.GetTempPath(); - //do - //{ - // tempDirPath = Path.Combine( - // tempPath, - // Path.GetFileNameWithoutExtension(Path.GetRandomFileName())); - //} while (Directory.Exists(tempDirPath)); - //Directory.CreateDirectory(tempDirPath); - tempDirPath = "C:\\Users\\kabawany\\tmp\\modules\\"; + CreateTempDir(); + UpdateSymLinkFile(); } - private void RemoveTempDir() + private void UpdateSymLinkFile() { - //Directory.Delete(tempDirPath, true); + File.WriteAllLines(symLinkPath, new string[] { tempDirPath }); } - private void SetupPSModulePath() + private void CreateTempDir() { - oldPSModulePath = Environment.GetEnvironmentVariable("PSModulePath", EnvironmentVariableTarget.Process); - var sb = new StringBuilder(); - sb.Append(oldPSModulePath) - .Append(Path.DirectorySeparatorChar) - .Append(tempDirPath); - currentModulePath = sb.ToString(); + tempDirPath = GetTempDirPath(); + Directory.CreateDirectory(tempDirPath); + } + + private string GetTempDirPath() + { + var tempPathRoot = Path.GetTempPath(); + string tempPath; + do + { + tempPath = Path.Combine( + tempPathRoot, + tempPrefix + Path.GetFileNameWithoutExtension(Path.GetRandomFileName())); + } while (Directory.Exists(tempPath)); + return tempPath; + } + + // Return the first line of the file + private string GetTempDirPath(string symLinkPath) + { + var symLinkLines = File.ReadAllLines(symLinkPath); + if(symLinkLines.Length != 1) + { + return null; + } + return symLinkLines[0]; } private void CleanUp() { runspace.Dispose(); - RemoveTempDir(); - RestorePSModulePath(); - } - private void RestorePSModulePath() - { - Environment.SetEnvironmentVariable("PSModulePath", oldPSModulePath, EnvironmentVariableTarget.Process); + // remove the modules from local psmodule path + foreach (var dir in Directory.EnumerateDirectories(localPSModulePath)) + { + if (modulesSavedInModulePath.Contains(Path.GetFileName(dir))) + { + Directory.Delete(dir, true); + } + } } private void SaveModule(PSObject module) @@ -98,6 +155,33 @@ private void SaveModule(PSObject module) ps.Invoke(); } + // TODO Use powershell copy-item + private void CopyDir(string srcParentPath, + string srcName, + string dstParentPath, + string dstName = null, + bool recurse = false) + { + if (dstName == null) + { + dstName = srcName; + } + var srcPath = Path.Combine(srcParentPath, srcName); + var dstPath = Path.Combine(dstParentPath, dstName); + Directory.CreateDirectory(dstPath); + foreach (var file in Directory.EnumerateFiles(srcPath)) + { + File.Copy(file, Path.Combine(dstPath, Path.GetFileName(file))); + } + foreach (var dir in Directory.EnumerateDirectories(srcPath)) + { + CopyDir(srcPath, + Path.GetFileName(dir), + dstPath, + recurse: true); + } + } + #endregion Private Methods #region Public Methods @@ -106,10 +190,21 @@ public ModuleDependencyHandler() { runspace = null; moduleRepository = "PSGallery"; - modulesSaved = new HashSet(); + modulesSavedInModulePath = new HashSet(); + modulesSavedInTempPath = new HashSet(); modulesFound = new Dictionary(); - SetupTempDir(); - //SetupPSModulePath(); + + // TODO search it in the $psmodulepath instead of constructing it + localPSModulePath = Path.Combine( + Environment.GetEnvironmentVariable("USERPROFILE"), + "Documents\\WindowsPowerShell\\Modules"); + localAppdataPath = Environment.GetEnvironmentVariable("LOCALAPPDATA"); + + // TODO Add PSSA Version in the path + pssaAppdataPath = Path.Combine(localAppdataPath, "PSScriptAnalyzer"); + symLinkPath = Path.Combine(pssaAppdataPath, symLinkName); + + SetupCache(); } public ModuleDependencyHandler(Runspace runspace) : this() @@ -163,13 +258,28 @@ public PSObject FindModule(string moduleName) return module; } + + public bool ModuleExists(string moduleName) + { + throw new NotImplementedException(); + } + + public void SaveModule(string moduleName) { ThrowIfNull(moduleName, "moduleName"); - if (modulesSaved.Contains(moduleName)) + if (modulesSavedInModulePath.Contains(moduleName)) + { + return; + } + if (modulesSavedInTempPath.Contains(moduleName)) { + // copy to local ps module path + CopyDir(tempDirPath, moduleName, localPSModulePath, recurse: true); + modulesSavedInModulePath.Add(moduleName); return; } + var module = FindModule(moduleName); if (module == null) { @@ -180,7 +290,11 @@ public void SaveModule(string moduleName) moduleRepository)); } SaveModule(module); - modulesSaved.Add(moduleName); + modulesSavedInTempPath.Add(moduleName); + + // copy to local ps module path + CopyDir(tempDirPath, moduleName, localPSModulePath, recurse: true); + modulesSavedInModulePath.Add(moduleName); } public static string GetModuleNameFromErrorExtent(ParseError error, ScriptBlockAst ast) @@ -229,4 +343,4 @@ public void Dispose() #endregion Public Methods } -} +} \ No newline at end of file From ea3c0adfb7f3f4fc303eb3ce2c99185e0d5388ba Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 28 Apr 2016 22:42:07 -0700 Subject: [PATCH 03/23] Change copy directory implementation in module handler Use powershell copy-item to copy directories instead of writing a routine to copy directories recursively. --- Engine/Generic/ModuleDependencyHandler.cs | 46 +++++++++-------------- 1 file changed, 17 insertions(+), 29 deletions(-) diff --git a/Engine/Generic/ModuleDependencyHandler.cs b/Engine/Generic/ModuleDependencyHandler.cs index 1efcb971f..0a337e9c8 100644 --- a/Engine/Generic/ModuleDependencyHandler.cs +++ b/Engine/Generic/ModuleDependencyHandler.cs @@ -12,6 +12,8 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic { // TODO Use runspace pool // TODO Create a new process for the runspace + // TODO Support for verbose mode + // TODO Try changing the psmodulepath variable through powershell layer. This will save copying and removing the modules public class ModuleDependencyHandler : IDisposable { #region Private Variables @@ -31,7 +33,7 @@ public class ModuleDependencyHandler : IDisposable #endregion Private Variables #region Properties - public string ModulePath + public string TempModulePath { get { return tempDirPath; } } @@ -156,30 +158,15 @@ private void SaveModule(PSObject module) } // TODO Use powershell copy-item - private void CopyDir(string srcParentPath, - string srcName, - string dstParentPath, - string dstName = null, - bool recurse = false) + private void CopyDir(string srcPath, string dstPath) { - if (dstName == null) - { - dstName = srcName; - } - var srcPath = Path.Combine(srcParentPath, srcName); - var dstPath = Path.Combine(dstParentPath, dstName); - Directory.CreateDirectory(dstPath); - foreach (var file in Directory.EnumerateFiles(srcPath)) - { - File.Copy(file, Path.Combine(dstPath, Path.GetFileName(file))); - } - foreach (var dir in Directory.EnumerateDirectories(srcPath)) - { - CopyDir(srcPath, - Path.GetFileName(dir), - dstPath, - recurse: true); - } + var ps = System.Management.Automation.PowerShell.Create(); + ps.Runspace = runspace; + ps.AddCommand("Copy-Item") + .AddParameter("Recurse") + .AddParameter("Path", srcPath) + .AddParameter("Destination", dstPath); + ps.Invoke(); } #endregion Private Methods @@ -243,8 +230,7 @@ public PSObject FindModule(string moduleName) ps.AddCommand("Find-Module", true) .AddParameter("Name", moduleName) .AddParameter("Repository", moduleRepository); - modules = ps.Invoke(); - + modules = ps.Invoke(); if (modules == null) { return null; @@ -264,7 +250,9 @@ public bool ModuleExists(string moduleName) throw new NotImplementedException(); } - + // TODO Do not use find module because it leads to two queries to the server + // instead use save module and check if it couldn't find the module + // TODO Add a TrySaveModule method public void SaveModule(string moduleName) { ThrowIfNull(moduleName, "moduleName"); @@ -275,7 +263,7 @@ public void SaveModule(string moduleName) if (modulesSavedInTempPath.Contains(moduleName)) { // copy to local ps module path - CopyDir(tempDirPath, moduleName, localPSModulePath, recurse: true); + CopyDir(Path.Combine(tempDirPath, moduleName), localPSModulePath); modulesSavedInModulePath.Add(moduleName); return; } @@ -293,7 +281,7 @@ public void SaveModule(string moduleName) modulesSavedInTempPath.Add(moduleName); // copy to local ps module path - CopyDir(tempDirPath, moduleName, localPSModulePath, recurse: true); + CopyDir(Path.Combine(tempDirPath, moduleName), localPSModulePath); modulesSavedInModulePath.Add(moduleName); } From 4ad4dcad2280c2e8baf5edf7c94a355f92b2d34e Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Tue, 3 May 2016 16:16:42 -0700 Subject: [PATCH 04/23] Check for module presence before copying --- Engine/Generic/ModuleDependencyHandler.cs | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/Engine/Generic/ModuleDependencyHandler.cs b/Engine/Generic/ModuleDependencyHandler.cs index 0a337e9c8..7cd46dbc1 100644 --- a/Engine/Generic/ModuleDependencyHandler.cs +++ b/Engine/Generic/ModuleDependencyHandler.cs @@ -263,8 +263,7 @@ public void SaveModule(string moduleName) if (modulesSavedInTempPath.Contains(moduleName)) { // copy to local ps module path - CopyDir(Path.Combine(tempDirPath, moduleName), localPSModulePath); - modulesSavedInModulePath.Add(moduleName); + CopyToPSModulePath(moduleName); return; } @@ -278,9 +277,22 @@ public void SaveModule(string moduleName) moduleRepository)); } SaveModule(module); - modulesSavedInTempPath.Add(moduleName); + modulesSavedInTempPath.Add(moduleName); + CopyToPSModulePath(moduleName); + } - // copy to local ps module path + private void CopyToPSModulePath(string moduleName, bool checkModulePresence = false) + { + if (checkModulePresence) + { + foreach(var dir in Directory.EnumerateDirectories(localPSModulePath)) + { + if (Path.GetFileName(dir).Equals(moduleName, StringComparison.OrdinalIgnoreCase)) + { + return; + } + } + } CopyDir(Path.Combine(tempDirPath, moduleName), localPSModulePath); modulesSavedInModulePath.Add(moduleName); } From e81cc3b8a0729e578d22891a62357733f9a80fc3 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 4 May 2016 12:57:44 -0700 Subject: [PATCH 05/23] Improve module name comparison in module handler --- Engine/Generic/ModuleDependencyHandler.cs | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/Engine/Generic/ModuleDependencyHandler.cs b/Engine/Generic/ModuleDependencyHandler.cs index 7cd46dbc1..1b03fffee 100644 --- a/Engine/Generic/ModuleDependencyHandler.cs +++ b/Engine/Generic/ModuleDependencyHandler.cs @@ -86,10 +86,20 @@ private void SetModulesInTempPath() // we assume the modules have not been tampered with foreach (var dir in Directory.EnumerateDirectories(tempDirPath)) { - modulesSavedInTempPath.Add(Path.GetFileName(dir)); + AddModuleName(modulesSavedInTempPath, Path.GetFileName(dir)); } } + private void AddModuleName(HashSet hashSet, string moduleName) + { + hashSet.Add(moduleName.ToLower()); + } + + private bool IsModuleNamePresent(HashSet hashSet, string moduleName) + { + return hashSet.Contains(moduleName.ToLower()); + } + private void SetupTempDir() { CreateTempDir(); @@ -138,7 +148,7 @@ private void CleanUp() // remove the modules from local psmodule path foreach (var dir in Directory.EnumerateDirectories(localPSModulePath)) { - if (modulesSavedInModulePath.Contains(Path.GetFileName(dir))) + if (IsModuleNamePresent(modulesSavedInModulePath, Path.GetFileName(dir))) { Directory.Delete(dir, true); } @@ -220,6 +230,7 @@ public void SetupDefaultRunspace(Runspace runspace) public PSObject FindModule(string moduleName) { ThrowIfNull(moduleName, "moduleName"); + moduleName = moduleName.ToLower(); if (modulesFound.ContainsKey(moduleName)) { return modulesFound[moduleName]; @@ -256,11 +267,11 @@ public bool ModuleExists(string moduleName) public void SaveModule(string moduleName) { ThrowIfNull(moduleName, "moduleName"); - if (modulesSavedInModulePath.Contains(moduleName)) + if (IsModuleNamePresent(modulesSavedInModulePath, moduleName)) { return; } - if (modulesSavedInTempPath.Contains(moduleName)) + if (IsModuleNamePresent(modulesSavedInTempPath, moduleName)) { // copy to local ps module path CopyToPSModulePath(moduleName); @@ -277,7 +288,7 @@ public void SaveModule(string moduleName) moduleRepository)); } SaveModule(module); - modulesSavedInTempPath.Add(moduleName); + AddModuleName(modulesSavedInTempPath, moduleName); CopyToPSModulePath(moduleName); } @@ -294,7 +305,7 @@ private void CopyToPSModulePath(string moduleName, bool checkModulePresence = fa } } CopyDir(Path.Combine(tempDirPath, moduleName), localPSModulePath); - modulesSavedInModulePath.Add(moduleName); + AddModuleName(modulesSavedInModulePath, moduleName); } public static string GetModuleNameFromErrorExtent(ParseError error, ScriptBlockAst ast) From 6e8097f1aee7dd8800b6524eb9b3477aa50295a3 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 4 May 2016 13:44:01 -0700 Subject: [PATCH 06/23] Modify PSModulePath in module dependency handler Adds a temporary path to the PSModulePath that contains the downloaded modules. This prevents the powershell parser from throwing parse errors when it cannot find a symbol that refers to an external module. --- Engine/Generic/ModuleDependencyHandler.cs | 81 ++++++----------------- 1 file changed, 21 insertions(+), 60 deletions(-) diff --git a/Engine/Generic/ModuleDependencyHandler.cs b/Engine/Generic/ModuleDependencyHandler.cs index 1b03fffee..0f99b6740 100644 --- a/Engine/Generic/ModuleDependencyHandler.cs +++ b/Engine/Generic/ModuleDependencyHandler.cs @@ -6,29 +6,27 @@ using System.Management.Automation; using System.Management.Automation.Language; using System.Management.Automation.Runspaces; -using System.Text; namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic { // TODO Use runspace pool // TODO Create a new process for the runspace - // TODO Support for verbose mode - // TODO Try changing the psmodulepath variable through powershell layer. This will save copying and removing the modules + // TODO Support for verbose mode public class ModuleDependencyHandler : IDisposable { #region Private Variables private Runspace runspace; private readonly string moduleRepository; - private string tempDirPath; - private string localPSModulePath; + private string tempDirPath; Dictionary modulesFound; - HashSet modulesSavedInModulePath; HashSet modulesSavedInTempPath; private string localAppdataPath; private string pssaAppdataPath; private const string symLinkName = "TempModuleDir"; private const string tempPrefix = "PSSAModules-"; private string symLinkPath; + private string oldPSModulePath; + private string curPSModulePath; #endregion Private Variables @@ -144,20 +142,13 @@ private string GetTempDirPath(string symLinkPath) private void CleanUp() { runspace.Dispose(); - - // remove the modules from local psmodule path - foreach (var dir in Directory.EnumerateDirectories(localPSModulePath)) - { - if (IsModuleNamePresent(modulesSavedInModulePath, Path.GetFileName(dir))) - { - Directory.Delete(dir, true); - } - } + RestorePSModulePath(); } private void SaveModule(PSObject module) { ThrowIfNull(module, "module"); + // TODO validate module var ps = System.Management.Automation.PowerShell.Create(); ps.Runspace = runspace; @@ -166,19 +157,6 @@ private void SaveModule(PSObject module) .AddParameter("InputObject", module); ps.Invoke(); } - - // TODO Use powershell copy-item - private void CopyDir(string srcPath, string dstPath) - { - var ps = System.Management.Automation.PowerShell.Create(); - ps.Runspace = runspace; - ps.AddCommand("Copy-Item") - .AddParameter("Recurse") - .AddParameter("Path", srcPath) - .AddParameter("Destination", dstPath); - ps.Invoke(); - } - #endregion Private Methods #region Public Methods @@ -187,21 +165,27 @@ public ModuleDependencyHandler() { runspace = null; moduleRepository = "PSGallery"; - modulesSavedInModulePath = new HashSet(); modulesSavedInTempPath = new HashSet(); - modulesFound = new Dictionary(); - - // TODO search it in the $psmodulepath instead of constructing it - localPSModulePath = Path.Combine( - Environment.GetEnvironmentVariable("USERPROFILE"), - "Documents\\WindowsPowerShell\\Modules"); + modulesFound = new Dictionary(); localAppdataPath = Environment.GetEnvironmentVariable("LOCALAPPDATA"); // TODO Add PSSA Version in the path pssaAppdataPath = Path.Combine(localAppdataPath, "PSScriptAnalyzer"); symLinkPath = Path.Combine(pssaAppdataPath, symLinkName); - SetupCache(); + SetupPSModulePath(); + } + + private void SetupPSModulePath() + { + oldPSModulePath = Environment.GetEnvironmentVariable("PSModulePath"); + curPSModulePath = oldPSModulePath + ";" + tempDirPath; + Environment.SetEnvironmentVariable("PSModulePath", curPSModulePath, EnvironmentVariableTarget.Process); + } + + private void RestorePSModulePath() + { + Environment.SetEnvironmentVariable("PSModulePath", oldPSModulePath, EnvironmentVariableTarget.Process); } public ModuleDependencyHandler(Runspace runspace) : this() @@ -267,14 +251,8 @@ public bool ModuleExists(string moduleName) public void SaveModule(string moduleName) { ThrowIfNull(moduleName, "moduleName"); - if (IsModuleNamePresent(modulesSavedInModulePath, moduleName)) - { - return; - } if (IsModuleNamePresent(modulesSavedInTempPath, moduleName)) - { - // copy to local ps module path - CopyToPSModulePath(moduleName); + { return; } @@ -289,25 +267,8 @@ public void SaveModule(string moduleName) } SaveModule(module); AddModuleName(modulesSavedInTempPath, moduleName); - CopyToPSModulePath(moduleName); } - private void CopyToPSModulePath(string moduleName, bool checkModulePresence = false) - { - if (checkModulePresence) - { - foreach(var dir in Directory.EnumerateDirectories(localPSModulePath)) - { - if (Path.GetFileName(dir).Equals(moduleName, StringComparison.OrdinalIgnoreCase)) - { - return; - } - } - } - CopyDir(Path.Combine(tempDirPath, moduleName), localPSModulePath); - AddModuleName(modulesSavedInModulePath, moduleName); - } - public static string GetModuleNameFromErrorExtent(ParseError error, ScriptBlockAst ast) { ThrowIfNull(error, "error"); From ce31b436ad73f3a6518b072b2cf9ea47891eec5f Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 4 May 2016 15:23:33 -0700 Subject: [PATCH 07/23] Add a switch to resolve dsc module dependency Adds a switch named "ResolveDSCResourceDependency" to Invoke-ScriptAnalyzer (isa). If the switch is given, then isa with try to resolve the dependency, whenever parse error is thrown due to modulenotfound exception, through the ModuleDependencyHandler class. If the switch is not provided, isa will not try to resolve the dependency. --- .../Commands/InvokeScriptAnalyzerCommand.cs | 13 +++++- Engine/ScriptAnalyzer.cs | 41 ++++++++++++------- 2 files changed, 38 insertions(+), 16 deletions(-) diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index 9a097cb81..d93c89593 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -188,6 +188,16 @@ public object Settings private bool stopProcessing; + /// + /// Resolve DSC resoure dependency + /// + [Parameter(Mandatory = false)] + public SwitchParameter ResolveDSCResourceDependency + { + get { return resolveDSCResourceDependency; } + set { resolveDSCResourceDependency = value; } + } + private bool resolveDSCResourceDependency; #endregion Parameters #region Overrides @@ -213,7 +223,8 @@ protected override void BeginProcessing() this.excludeRule, this.severity, null == rulePaths ? true : this.includeDefaultRules, - this.suppressedOnly); + this.suppressedOnly, + resolveDSCResourceDependency: resolveDSCResourceDependency); } /// diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 60383d46f..4d0223414 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -105,7 +105,8 @@ internal void Initialize( string[] excludeRuleNames = null, string[] severity = null, bool includeDefaultRules = false, - bool suppressedOnly = false) + bool suppressedOnly = false, + bool resolveDSCResourceDependency = false) where TCmdlet : PSCmdlet, IOutputWriter { if (cmdlet == null) @@ -122,7 +123,8 @@ internal void Initialize( excludeRuleNames, severity, includeDefaultRules, - suppressedOnly); + suppressedOnly, + resolveDSCResourceDependency: resolveDSCResourceDependency); } /// @@ -167,8 +169,11 @@ public void CleanUp() severity = null; includeRegexList = null; excludeRegexList = null; - suppressedOnly = false; - moduleHandler.Dispose(); + suppressedOnly = false; + if (moduleHandler != null) + { + moduleHandler.Dispose(); + } } internal bool ParseProfile(object profileObject, PathIntrinsics path, IOutputWriter writer) @@ -462,16 +467,17 @@ private bool ParseProfileString(string profile, PathIntrinsics path, IOutputWrit } private void Initialize( - IOutputWriter outputWriter, - PathIntrinsics path, - CommandInvocationIntrinsics invokeCommand, - string[] customizedRulePath, + IOutputWriter outputWriter, + PathIntrinsics path, + CommandInvocationIntrinsics invokeCommand, + string[] customizedRulePath, string[] includeRuleNames, string[] excludeRuleNames, string[] severity, bool includeDefaultRules = false, bool suppressedOnly = false, - string profile = null) + string profile = null, + bool resolveDSCResourceDependency = false) { if (outputWriter == null) { @@ -481,8 +487,8 @@ private void Initialize( this.outputWriter = outputWriter; // TODO Create a runspace pool - runspace = RunspaceFactory.CreateRunspace(); - moduleHandler = new ModuleDependencyHandler(runspace); + runspace = RunspaceFactory.CreateRunspace(); + moduleHandler = resolveDSCResourceDependency ? new ModuleDependencyHandler(runspace) : null; #region Verifies rule extensions and loggers path @@ -1318,8 +1324,8 @@ private IEnumerable AnalyzeFile(string filePath) // - OR // - swallow the these errors - - if (errors != null && errors.Length > 0) + bool parseAgain = false; + if (moduleHandler != null && errors != null && errors.Length > 0) { foreach (ParseError error in errors) { @@ -1329,6 +1335,8 @@ private IEnumerable AnalyzeFile(string filePath) if (moduleName != null) { moduleHandler.SaveModule(moduleName); + // if successfully saved + parseAgain = true; } } } @@ -1337,9 +1345,12 @@ private IEnumerable AnalyzeFile(string filePath) //try parsing again //var oldDefault = Runspace.DefaultRunspace; //Runspace.DefaultRunspace = moduleHandler.Runspace; - scriptAst = Parser.ParseFile(filePath, out scriptTokens, out errors); - //Runspace.DefaultRunspace = oldDefault; + if (parseAgain) + { + scriptAst = Parser.ParseFile(filePath, out scriptTokens, out errors); + } + //Runspace.DefaultRunspace = oldDefault; if (errors != null && errors.Length > 0) { foreach (ParseError error in errors) From ddae10f7f0b7b89f862967f3689d94b74adf23f7 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 4 May 2016 15:46:51 -0700 Subject: [PATCH 08/23] Add TrySaveModule method to ModuleDependencyHandler class --- Engine/Generic/ModuleDependencyHandler.cs | 14 ++++++++++++++ Engine/ScriptAnalyzer.cs | 15 +++++---------- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/Engine/Generic/ModuleDependencyHandler.cs b/Engine/Generic/ModuleDependencyHandler.cs index 0f99b6740..7e3ac09d1 100644 --- a/Engine/Generic/ModuleDependencyHandler.cs +++ b/Engine/Generic/ModuleDependencyHandler.cs @@ -245,6 +245,20 @@ public bool ModuleExists(string moduleName) throw new NotImplementedException(); } + public bool TrySaveModule(string moduleName) + { + try + { + SaveModule(moduleName); + return true; + } + catch + { + // log exception to verbose + return false; + } + } + // TODO Do not use find module because it leads to two queries to the server // instead use save module and check if it couldn't find the module // TODO Add a TrySaveModule method diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 4d0223414..8b6e2e2b3 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -1327,17 +1327,13 @@ private IEnumerable AnalyzeFile(string filePath) bool parseAgain = false; if (moduleHandler != null && errors != null && errors.Length > 0) { - foreach (ParseError error in errors) + foreach (ParseError error in errors.Where(IsModuleNotFoundError)) { - if (IsModuleNotFoundError(error)) + var moduleName = ModuleDependencyHandler.GetModuleNameFromErrorExtent(error, scriptAst); + if (moduleName != null + && moduleHandler.TrySaveModule(moduleName)) { - var moduleName = ModuleDependencyHandler.GetModuleNameFromErrorExtent(error, scriptAst); - if (moduleName != null) - { - moduleHandler.SaveModule(moduleName); - // if successfully saved - parseAgain = true; - } + parseAgain = true; } } } @@ -1364,7 +1360,6 @@ private IEnumerable AnalyzeFile(string filePath) { string manyParseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParserErrorMessage, System.IO.Path.GetFileName(filePath)); this.outputWriter.WriteError(new ErrorRecord(new ParseException(manyParseErrorMessage), manyParseErrorMessage, ErrorCategory.ParserError, filePath)); - return new List(); } } From a5bd6fd7948851b63699883225b7864460dfbaad Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 4 May 2016 15:50:13 -0700 Subject: [PATCH 09/23] Change SaveModule implementation Previously, SaveModule method in ModuleDependency handler would invoke Find-Module and then Save-Module. This is inefficient as two queries are made the PSGallery server which are redundant. This commit removes Find-Module invocation. --- Engine/Generic/ModuleDependencyHandler.cs | 25 ++++++----------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/Engine/Generic/ModuleDependencyHandler.cs b/Engine/Generic/ModuleDependencyHandler.cs index 7e3ac09d1..666e43140 100644 --- a/Engine/Generic/ModuleDependencyHandler.cs +++ b/Engine/Generic/ModuleDependencyHandler.cs @@ -239,12 +239,6 @@ public PSObject FindModule(string moduleName) return module; } - - public bool ModuleExists(string moduleName) - { - throw new NotImplementedException(); - } - public bool TrySaveModule(string moduleName) { try @@ -259,9 +253,6 @@ public bool TrySaveModule(string moduleName) } } - // TODO Do not use find module because it leads to two queries to the server - // instead use save module and check if it couldn't find the module - // TODO Add a TrySaveModule method public void SaveModule(string moduleName) { ThrowIfNull(moduleName, "moduleName"); @@ -270,16 +261,12 @@ public void SaveModule(string moduleName) return; } - var module = FindModule(moduleName); - if (module == null) - { - throw new ItemNotFoundException( - string.Format( - "Cannot find {0} in {1} repository.", - moduleName, - moduleRepository)); - } - SaveModule(module); + var ps = System.Management.Automation.PowerShell.Create(); + ps.Runspace = runspace; + ps.AddCommand("Save-Module") + .AddParameter("Path", tempDirPath) + .AddParameter("Name", moduleName); + ps.Invoke(); AddModuleName(modulesSavedInTempPath, moduleName); } From 9411a821719a9ea0b76c92e40b3a2668b4228a39 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 9 May 2016 22:12:24 -0700 Subject: [PATCH 10/23] Clean up ModuleDependencyHandler implementation --- Engine/Generic/ModuleDependencyHandler.cs | 320 +++++++++++++++------- Engine/ScriptAnalyzer.cs | 13 - 2 files changed, 214 insertions(+), 119 deletions(-) diff --git a/Engine/Generic/ModuleDependencyHandler.cs b/Engine/Generic/ModuleDependencyHandler.cs index 666e43140..d8bcb96c2 100644 --- a/Engine/Generic/ModuleDependencyHandler.cs +++ b/Engine/Generic/ModuleDependencyHandler.cs @@ -10,37 +10,130 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic { // TODO Use runspace pool - // TODO Create a new process for the runspace // TODO Support for verbose mode public class ModuleDependencyHandler : IDisposable { #region Private Variables private Runspace runspace; - private readonly string moduleRepository; - private string tempDirPath; + private string moduleRepository; + private string tempPath; // path to the user temporary directory + private string tempModulePath; // path to temp directory containing modules Dictionary modulesFound; - HashSet modulesSavedInTempPath; private string localAppdataPath; - private string pssaAppdataPath; + private string pssaAppDataPath; private const string symLinkName = "TempModuleDir"; private const string tempPrefix = "PSSAModules-"; private string symLinkPath; private string oldPSModulePath; private string curPSModulePath; - #endregion Private Variables #region Properties + /// + /// Path where the object stores the modules + /// public string TempModulePath { - get { return tempDirPath; } + get { return tempModulePath; } + } + + /// + /// Temporary path of the current user scope + /// + public string TempPath + { + get + { + return tempPath; + } + // it must be set only during initialization + private set + { + tempPath + = (string.IsNullOrEmpty(value) + || string.IsNullOrWhiteSpace(value)) + ? Environment.GetEnvironmentVariable("TEMP") + : value; + } + + } + + /// + /// Local App Data path + /// + public string LocalAppDataPath + { + get + { + return localAppdataPath; + } + private set + { + localAppdataPath + = (string.IsNullOrEmpty(value) + || string.IsNullOrWhiteSpace(value)) + ? Environment.GetEnvironmentVariable("LOCALAPPDATA") + : value; + } } - public Runspace Runspace + /// + /// Module Respository + /// By default it is PSGallery + /// + public string ModuleRepository + { + get + { + return moduleRepository; + } + set + { + moduleRepository + = (string.IsNullOrEmpty(value) + || string.IsNullOrWhiteSpace(value)) + ? "PSGallery" + : value; + } + } + + /// + /// Local App data of PSSScriptAnalyzer + /// + public string PSSAAppDataPath + { + get + { + return pssaAppDataPath; + } + private set + { + var leaf + = (string.IsNullOrEmpty(value) || string.IsNullOrWhiteSpace(value)) + ? "PSScriptAnalyzer" + : value; + pssaAppDataPath = Path.Combine(LocalAppDataPath, leaf); + } + } + + /// + /// Module Paths + /// + public string PSModulePath + { + get { return curPSModulePath; } + } + + /// + /// Runspace in which the object invokes powershell cmdlets + /// + public Runspace Runspace { get { return runspace; } + set { runspace = value; } } + #endregion Properties #region Private Methods @@ -52,84 +145,64 @@ private static void ThrowIfNull(T obj, string name) } } - private void SetupCache() + private void SetupPSSAAppData() { // check if pssa exists in local appdata - if (Directory.Exists(pssaAppdataPath)) + if (Directory.Exists(pssaAppDataPath)) { // check if there is a link if (File.Exists(symLinkPath)) { - tempDirPath = GetTempDirPath(symLinkPath); + tempModulePath = GetTempModulePath(symLinkPath); // check if the temp dir exists - if (tempDirPath != null - && Directory.Exists(tempDirPath)) + if (tempModulePath != null + && Directory.Exists(tempModulePath)) { - SetModulesInTempPath(); return; } - } - SetupTempDir(); + } } else { - Directory.CreateDirectory(pssaAppdataPath); - SetupTempDir(); - } + Directory.CreateDirectory(pssaAppDataPath); + } + SetupTempDir(); } - private void SetModulesInTempPath() + private bool IsModulePresent(string moduleName) { - // we assume the modules have not been tampered with - foreach (var dir in Directory.EnumerateDirectories(tempDirPath)) + foreach (var dir in Directory.EnumerateDirectories(TempModulePath)) { - AddModuleName(modulesSavedInTempPath, Path.GetFileName(dir)); + if (moduleName.Equals(dir, StringComparison.OrdinalIgnoreCase)) + { + return true; + } } - } - - private void AddModuleName(HashSet hashSet, string moduleName) - { - hashSet.Add(moduleName.ToLower()); - } - - private bool IsModuleNamePresent(HashSet hashSet, string moduleName) - { - return hashSet.Contains(moduleName.ToLower()); + return false; } private void SetupTempDir() { - CreateTempDir(); - UpdateSymLinkFile(); + tempModulePath = GetPSSATempDirPath(); + Directory.CreateDirectory(tempModulePath); + File.WriteAllLines(symLinkPath, new string[] { tempModulePath }); } - private void UpdateSymLinkFile() + private string GetPSSATempDirPath() { - File.WriteAllLines(symLinkPath, new string[] { tempDirPath }); - } - - private void CreateTempDir() - { - tempDirPath = GetTempDirPath(); - Directory.CreateDirectory(tempDirPath); - } - - private string GetTempDirPath() - { - var tempPathRoot = Path.GetTempPath(); - string tempPath; + string path; do { - tempPath = Path.Combine( - tempPathRoot, + path = Path.Combine( + tempPath, tempPrefix + Path.GetFileNameWithoutExtension(Path.GetRandomFileName())); - } while (Directory.Exists(tempPath)); - return tempPath; + } while (Directory.Exists(path)); + return path; } // Return the first line of the file - private string GetTempDirPath(string symLinkPath) + private string GetTempModulePath(string symLinkPath) { var symLinkLines = File.ReadAllLines(symLinkPath); if(symLinkLines.Length != 1) @@ -138,12 +211,6 @@ private string GetTempDirPath(string symLinkPath) } return symLinkLines[0]; } - - private void CleanUp() - { - runspace.Dispose(); - RestorePSModulePath(); - } private void SaveModule(PSObject module) { @@ -153,33 +220,15 @@ private void SaveModule(PSObject module) var ps = System.Management.Automation.PowerShell.Create(); ps.Runspace = runspace; ps.AddCommand("Save-Module") - .AddParameter("Path", tempDirPath) + .AddParameter("Path", tempModulePath) .AddParameter("InputObject", module); ps.Invoke(); } - #endregion Private Methods - - #region Public Methods - - public ModuleDependencyHandler() - { - runspace = null; - moduleRepository = "PSGallery"; - modulesSavedInTempPath = new HashSet(); - modulesFound = new Dictionary(); - localAppdataPath = Environment.GetEnvironmentVariable("LOCALAPPDATA"); - - // TODO Add PSSA Version in the path - pssaAppdataPath = Path.Combine(localAppdataPath, "PSScriptAnalyzer"); - symLinkPath = Path.Combine(pssaAppdataPath, symLinkName); - SetupCache(); - SetupPSModulePath(); - } private void SetupPSModulePath() { oldPSModulePath = Environment.GetEnvironmentVariable("PSModulePath"); - curPSModulePath = oldPSModulePath + ";" + tempDirPath; + curPSModulePath = oldPSModulePath + ";" + tempModulePath; Environment.SetEnvironmentVariable("PSModulePath", curPSModulePath, EnvironmentVariableTarget.Process); } @@ -187,30 +236,65 @@ private void RestorePSModulePath() { Environment.SetEnvironmentVariable("PSModulePath", oldPSModulePath, EnvironmentVariableTarget.Process); } + #endregion Private Methods - public ModuleDependencyHandler(Runspace runspace) : this() - { - ThrowIfNull(runspace, "runspace"); - this.runspace = runspace; - this.runspace.Open(); - } - - - public void SetupDefaultRunspace() - { - runspace = RunspaceFactory.CreateRunspace(); - } + #region Public Methods - public void SetupDefaultRunspace(Runspace runspace) + /// + /// Creates an instance of the ModuleDependencyHandler class + /// + /// Runspace in which the instance runs powershell cmdlets to find and save modules + /// Name of the repository from where to download the modules. By default it is PSGallery. This should be a registered repository. + /// Path to the user scoped temporary directory + /// Path to the local app data directory + public ModuleDependencyHandler( + Runspace runspace = null, + string moduleRepository = null, + string tempPath = null, + string localAppDataPath = null) { - ThrowIfNull(runspace, "runspace"); - if (runspace != null) + if (runspace == null) + { + Runspace = RunspaceFactory.CreateRunspace(); + } + else + { + Runspace = runspace; + } + + if (Runspace.RunspaceStateInfo.State == RunspaceState.BeforeOpen) + { + Runspace.Open(); + } + else if (Runspace.RunspaceStateInfo.State != RunspaceState.Opened) { - this.runspace = runspace; + throw new ArgumentException(string.Format( + "Runspace state cannot be {0}", + runspace.RunspaceStateInfo.State.ToString())); } - Runspace.DefaultRunspace = this.runspace; + + // TODO should set PSSA environment variables outside this class + // Should be set in ScriptAnalyzer class + // and then passed into modulehandler + TempPath = tempPath; + LocalAppDataPath = localAppDataPath; + PSSAAppDataPath = pssaAppDataPath; + ModuleRepository = moduleRepository; + + modulesFound = new Dictionary(); + + // TODO Add PSSA Version in the path + symLinkPath = Path.Combine(pssaAppDataPath, symLinkName); + SetupPSSAAppData(); + SetupPSModulePath(); + } + /// + /// Encapsulates Find-Module + /// + /// Name of the module + /// A PSObject if it finds the modules otherwise returns null public PSObject FindModule(string moduleName) { ThrowIfNull(moduleName, "moduleName"); @@ -239,6 +323,11 @@ public PSObject FindModule(string moduleName) return module; } + /// + /// SaveModule version that doesn't throw + /// + /// Name of the module + /// True if it can save a module otherwise false. public bool TrySaveModule(string moduleName) { try @@ -248,35 +337,50 @@ public bool TrySaveModule(string moduleName) } catch { - // log exception to verbose + // log exception to verbose return false; } } + /// + /// Encapsulates Save-Module cmdlet + /// + /// Name of the module public void SaveModule(string moduleName) { ThrowIfNull(moduleName, "moduleName"); - if (IsModuleNamePresent(modulesSavedInTempPath, moduleName)) + if (IsModulePresent(moduleName)) { return; } - var ps = System.Management.Automation.PowerShell.Create(); ps.Runspace = runspace; ps.AddCommand("Save-Module") - .AddParameter("Path", tempDirPath) - .AddParameter("Name", moduleName); + .AddParameter("Path", tempModulePath) + .AddParameter("Name", moduleName) + .AddParameter("Force"); ps.Invoke(); - AddModuleName(modulesSavedInTempPath, moduleName); - } + } + /// + /// Get the module name from the error extent + /// + /// If a parser encounters Import-DSCResource -ModuleName SomeModule + /// and if SomeModule is not present in any of the PSModulePaths, the + /// parser throws ModuleNotFoundDuringParse Error. We correlate the + /// error message with extent to extract the module name as the error + /// record doesn't provide direct access to the missing module name. + /// + /// Parse error + /// AST of the script that contians the parse error + /// The name of the module that caused the parser to throw the error. Returns null if it cannot extract the module name. public static string GetModuleNameFromErrorExtent(ParseError error, ScriptBlockAst ast) { ThrowIfNull(error, "error"); ThrowIfNull(ast, "ast"); var statement = ast.Find(x => x.Extent.Equals(error.Extent), true); var dynamicKywdAst = statement as DynamicKeywordStatementAst; - if (dynamicKywdAst == null) + if (dynamicKywdAst == null) { return null; } @@ -309,9 +413,13 @@ public static string GetModuleNameFromErrorExtent(ParseError error, ScriptBlockA return paramValAst.Value; } + /// + /// Disposes the runspace and restores the PSModulePath. + /// public void Dispose() { - CleanUp(); + runspace.Dispose(); + RestorePSModulePath(); } #endregion Public Methods diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 8b6e2e2b3..443cc6597 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -1311,19 +1311,6 @@ private IEnumerable AnalyzeFile(string filePath) return null; } - - // TODO Handle Parse Errors causes by missing modules - // if errors are due to ModuleNotFoundDuringParse - // - EITHER - // - create a runspace and set the default runspace (make sure backup the default runspace before modifying it) - // - check if it is present in the gallery - // - if present on the gallery, save it to a temporary file, and load the module - // - now parse again - // - if parse is successful, proceed - // - else inform the user of the issue - // - OR - // - swallow the these errors - bool parseAgain = false; if (moduleHandler != null && errors != null && errors.Length > 0) { From d1293c1e7b6ff0ddc211cc715a639ff445d8b047 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 9 May 2016 22:21:12 -0700 Subject: [PATCH 11/23] Add tests for ModuleDependencyHandler --- Tests/Engine/MissingDSCResource.ps1 | 4 + .../Engine/ModuleDependencyHandler.tests.ps1 | 115 ++++++++++++++++++ 2 files changed, 119 insertions(+) create mode 100644 Tests/Engine/MissingDSCResource.ps1 create mode 100644 Tests/Engine/ModuleDependencyHandler.tests.ps1 diff --git a/Tests/Engine/MissingDSCResource.ps1 b/Tests/Engine/MissingDSCResource.ps1 new file mode 100644 index 000000000..c61a94cc5 --- /dev/null +++ b/Tests/Engine/MissingDSCResource.ps1 @@ -0,0 +1,4 @@ +Configuration SomeConfiguration +{ + Import-DscResource -ModuleName MyDSCResource +} \ No newline at end of file diff --git a/Tests/Engine/ModuleDependencyHandler.tests.ps1 b/Tests/Engine/ModuleDependencyHandler.tests.ps1 new file mode 100644 index 000000000..8cd074e18 --- /dev/null +++ b/Tests/Engine/ModuleDependencyHandler.tests.ps1 @@ -0,0 +1,115 @@ +if (!(Get-Module PSScriptAnalyzer) -and !$testingLibraryUsage) +{ + Import-Module PSScriptAnalyzer +} + +if ($testingLibraryUsage) +{ + return +} + +$directory = Split-Path -Parent $MyInvocation.MyCommand.Path +$violationFileName = 'MissingDSCResource.ps1' +$violationFilePath = Join-Path $directory $violationFileName + +Describe "Resolve DSC Resource Dependency" { + + Function Test-EnvironmentVariables($oldEnv) + { + $newEnv = Get-Item Env:\* | Sort-Object -Property Key + $newEnv.Count | Should Be $oldEnv.Count + foreach ($index in 1..$newEnv.Count) + { + $newEnv[$index].Key | Should Be $oldEnv[$index].Key + $newEnv[$index].Value | Should Be $oldEnv[$index].Value + } + } + + Context "Module handler class" { + $oldEnvVars = Get-Item Env:\* | Sort-Object -Property Key + $oldPSModulePath = $env:PSModulePath + It "Sets defaults correctly" { + $depHandler = [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.ModuleDependencyHandler]::new() + + $expectedPath = [System.Environment]::GetEnvironmentVariable("TEMP"); + $depHandler.TempPath | Should Be $expectedPath + + $expectedLocalAppDataPath = [System.Environment]::GetEnvironmentVariable("LOCALAPPDATA"); + $depHandler.LocalAppDataPath | Should Be $expectedLocalAppDataPath + + $expectedModuleRepository = "PSGallery" + $depHandler.ModuleRepository | Should Be $expectedModuleRepository + + $expectedPssaAppDataPath = Join-Path $depHandler.LocalAppDataPath "PSScriptAnalyzer" + $depHandler.PSSAAppDataPath | Should Be $expectedPssaAppDataPath + + $expectedPSModulePath = $oldPSModulePath + [System.IO.Path]::PathSeparator + $depHandler.TempModulePath + $env:PSModulePath | Should Be $expectedPSModulePath + + $depHandler.Dispose() + } + + It "Keeps the environment variables unchanged" { + Test-EnvironmentVariables($oldEnvVars) + } + } + + Context "Invoke-ScriptAnalyzer without switch" { + It "Has parse errors" { + $dr = Invoke-ScriptAnalyzer -Path $violationFilePath -ErrorVariable parseErrors -ErrorAction SilentlyContinue + $parseErrors.Count | Should Be 1 + } + } + + Context "Invoke-ScriptAnalyzer with switch" { + $oldEnvVars = Get-Item Env:\* | Sort-Object -Property Key + $moduleName = "MyDscResource" + $modulePath = Join-Path (Join-Path (Join-Path (Split-Path $directory) "Rules") "DSCResources") $moduleName + # Save the current environment variables + $oldLocalAppDataPath = $env:LOCALAPPDATA + $oldTempPath = $env:TEMP + $oldPSModulePath = $env:PSModulePath + + # set the environment variables + $tempPath = Join-Path $oldTempPath ([guid]::NewGUID()).ToString() + $newLocalAppDataPath = Join-Path $tempPath "LocalAppData" + $newTempPath = Join-Path $tempPath "Temp" + $env:LOCALAPPDATA = $newLocalAppDataPath + $env:TEMP = $newTempPath + + # create the temporary directories + New-Item -Type Directory -Path $newLocalAppDataPath + New-Item -Type Directory -Path $newTempPath + + # create and dispose module dependency handler object + # to setup the temporary module location + $depHandler = [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.ModuleDependencyHandler]::new() + $pssaAppDataPath = $depHandler.PSSAAppDataPath + $tempModulePath = $depHandler.TempModulePath + $depHandler.Dispose() + + # copy myresource module to the temporary location + # we could let the module dependency handler download it from psgallery + Copy-Item -Recurse -Path $modulePath -Destination $tempModulePath + + It "Doesn't have parse errors" { + # invoke script analyzer + $dr = Invoke-ScriptAnalyzer -Path $violationFilePath -ErrorVariable parseErrors -ErrorAction SilentlyContinue + $dr.Count | Should Be 0 + } + + It "Keeps PSModulePath unchanged before and after invocation" { + $dr = Invoke-ScriptAnalyzer -Path $violationFilePath -ErrorVariable parseErrors -ErrorAction SilentlyContinue + $env:PSModulePath | Should Be $oldPSModulePath + } + #restore environment variables and clean up temporary location + $env:LOCALAPPDATA = $oldLocalAppDataPath + $env:TEMP = $oldTempPath + Remove-Item -Recurse -Path $tempModulePath + Remove-Item -Recurse -Path $tempPath + + It "Keeps the environment variables unchanged" { + Test-EnvironmentVariables($oldEnvVars) + } + } +} \ No newline at end of file From a470e161da768f19525c8f04ae9fcd5b68e1f387 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Tue, 10 May 2016 14:26:57 -0700 Subject: [PATCH 12/23] Remove redundancies from ModuleDependencyHandler --- Engine/Generic/ModuleDependencyHandler.cs | 42 ++++++++----------- .../Engine/ModuleDependencyHandler.tests.ps1 | 6 +-- 2 files changed, 20 insertions(+), 28 deletions(-) diff --git a/Engine/Generic/ModuleDependencyHandler.cs b/Engine/Generic/ModuleDependencyHandler.cs index d8bcb96c2..5b5fa795d 100644 --- a/Engine/Generic/ModuleDependencyHandler.cs +++ b/Engine/Generic/ModuleDependencyHandler.cs @@ -50,9 +50,8 @@ public string TempPath private set { tempPath - = (string.IsNullOrEmpty(value) - || string.IsNullOrWhiteSpace(value)) - ? Environment.GetEnvironmentVariable("TEMP") + = string.IsNullOrWhiteSpace(value) + ? Path.GetTempPath() : value; } @@ -69,10 +68,9 @@ public string LocalAppDataPath } private set { - localAppdataPath - = (string.IsNullOrEmpty(value) - || string.IsNullOrWhiteSpace(value)) - ? Environment.GetEnvironmentVariable("LOCALAPPDATA") + localAppdataPath + = string.IsNullOrWhiteSpace(value) + ? Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData) : value; } } @@ -90,8 +88,7 @@ public string ModuleRepository set { moduleRepository - = (string.IsNullOrEmpty(value) - || string.IsNullOrWhiteSpace(value)) + = string.IsNullOrWhiteSpace(value) ? "PSGallery" : value; } @@ -106,14 +103,6 @@ public string PSSAAppDataPath { return pssaAppDataPath; } - private set - { - var leaf - = (string.IsNullOrEmpty(value) || string.IsNullOrWhiteSpace(value)) - ? "PSScriptAnalyzer" - : value; - pssaAppDataPath = Path.Combine(LocalAppDataPath, leaf); - } } /// @@ -156,8 +145,7 @@ private void SetupPSSAAppData() tempModulePath = GetTempModulePath(symLinkPath); // check if the temp dir exists - if (tempModulePath != null - && Directory.Exists(tempModulePath)) + if (Directory.Exists(tempModulePath)) { return; } @@ -204,12 +192,12 @@ private string GetPSSATempDirPath() // Return the first line of the file private string GetTempModulePath(string symLinkPath) { - var symLinkLines = File.ReadAllLines(symLinkPath); - if(symLinkLines.Length != 1) + string line; + using (var fileStream = new StreamReader(symLinkPath)) { - return null; + line = fileStream.ReadLine(); } - return symLinkLines[0]; + return line; } private void SaveModule(PSObject module) @@ -278,9 +266,13 @@ public ModuleDependencyHandler( // and then passed into modulehandler TempPath = tempPath; LocalAppDataPath = localAppDataPath; - PSSAAppDataPath = pssaAppDataPath; ModuleRepository = moduleRepository; - + pssaAppDataPath = Path.Combine( + LocalAppDataPath, + string.IsNullOrWhiteSpace(pssaAppDataPath) + ? "PSScriptAnalyzer" + : pssaAppDataPath); + modulesFound = new Dictionary(); // TODO Add PSSA Version in the path diff --git a/Tests/Engine/ModuleDependencyHandler.tests.ps1 b/Tests/Engine/ModuleDependencyHandler.tests.ps1 index 8cd074e18..122de1608 100644 --- a/Tests/Engine/ModuleDependencyHandler.tests.ps1 +++ b/Tests/Engine/ModuleDependencyHandler.tests.ps1 @@ -31,7 +31,7 @@ Describe "Resolve DSC Resource Dependency" { It "Sets defaults correctly" { $depHandler = [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.ModuleDependencyHandler]::new() - $expectedPath = [System.Environment]::GetEnvironmentVariable("TEMP"); + $expectedPath = [System.IO.Path]::GetTempPath() $depHandler.TempPath | Should Be $expectedPath $expectedLocalAppDataPath = [System.Environment]::GetEnvironmentVariable("LOCALAPPDATA"); @@ -105,8 +105,8 @@ Describe "Resolve DSC Resource Dependency" { #restore environment variables and clean up temporary location $env:LOCALAPPDATA = $oldLocalAppDataPath $env:TEMP = $oldTempPath - Remove-Item -Recurse -Path $tempModulePath - Remove-Item -Recurse -Path $tempPath + Remove-Item -Recurse -Path $tempModulePath -Force + Remove-Item -Recurse -Path $tempPath -Force It "Keeps the environment variables unchanged" { Test-EnvironmentVariables($oldEnvVars) From fd3a306e4ca7c7bf2c904d26bcf491125919477f Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 12 May 2016 12:00:28 -0700 Subject: [PATCH 13/23] Use IDisposable pattern while creating PowerShell objects --- Engine/Generic/ModuleDependencyHandler.cs | 46 +++++++++++++---------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/Engine/Generic/ModuleDependencyHandler.cs b/Engine/Generic/ModuleDependencyHandler.cs index 5b5fa795d..be6e24285 100644 --- a/Engine/Generic/ModuleDependencyHandler.cs +++ b/Engine/Generic/ModuleDependencyHandler.cs @@ -205,12 +205,14 @@ private void SaveModule(PSObject module) ThrowIfNull(module, "module"); // TODO validate module - var ps = System.Management.Automation.PowerShell.Create(); - ps.Runspace = runspace; - ps.AddCommand("Save-Module") - .AddParameter("Path", tempModulePath) - .AddParameter("InputObject", module); - ps.Invoke(); + using (var ps = System.Management.Automation.PowerShell.Create()) + { + ps.Runspace = runspace; + ps.AddCommand("Save-Module") + .AddParameter("Path", tempModulePath) + .AddParameter("InputObject", module); + ps.Invoke(); + } } private void SetupPSModulePath() @@ -295,13 +297,15 @@ public PSObject FindModule(string moduleName) { return modulesFound[moduleName]; } - var ps = System.Management.Automation.PowerShell.Create(); - Collection modules = null; - ps.Runspace = runspace; - ps.AddCommand("Find-Module", true) - .AddParameter("Name", moduleName) - .AddParameter("Repository", moduleRepository); - modules = ps.Invoke(); + Collection modules = null; + using (var ps = System.Management.Automation.PowerShell.Create()) + { + ps.Runspace = runspace; + ps.AddCommand("Find-Module", true) + .AddParameter("Name", moduleName) + .AddParameter("Repository", moduleRepository); + modules = ps.Invoke(); + } if (modules == null) { return null; @@ -345,13 +349,15 @@ public void SaveModule(string moduleName) { return; } - var ps = System.Management.Automation.PowerShell.Create(); - ps.Runspace = runspace; - ps.AddCommand("Save-Module") - .AddParameter("Path", tempModulePath) - .AddParameter("Name", moduleName) - .AddParameter("Force"); - ps.Invoke(); + using (var ps = System.Management.Automation.PowerShell.Create()) + { + ps.Runspace = runspace; + ps.AddCommand("Save-Module") + .AddParameter("Path", tempModulePath) + .AddParameter("Name", moduleName) + .AddParameter("Force"); + ps.Invoke(); + } } /// From ccdb0e05f486d1581f6beebc84d0a285f30ee8e4 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 12 May 2016 12:01:54 -0700 Subject: [PATCH 14/23] Set environment variable differently in module handler tests --- Tests/Engine/ModuleDependencyHandler.tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Engine/ModuleDependencyHandler.tests.ps1 b/Tests/Engine/ModuleDependencyHandler.tests.ps1 index 122de1608..03ed26c7b 100644 --- a/Tests/Engine/ModuleDependencyHandler.tests.ps1 +++ b/Tests/Engine/ModuleDependencyHandler.tests.ps1 @@ -34,7 +34,7 @@ Describe "Resolve DSC Resource Dependency" { $expectedPath = [System.IO.Path]::GetTempPath() $depHandler.TempPath | Should Be $expectedPath - $expectedLocalAppDataPath = [System.Environment]::GetEnvironmentVariable("LOCALAPPDATA"); + $expectedLocalAppDataPath = [System.Environment]::GetFolderPath([System.Environment+SpecialFolder]::LocalApplicationData) $depHandler.LocalAppDataPath | Should Be $expectedLocalAppDataPath $expectedModuleRepository = "PSGallery" From 73d63ee01920a0fdbcc82f04274273737cf322dd Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 12 May 2016 12:40:35 -0700 Subject: [PATCH 15/23] Use IDisposable pattern for ModuleDependencyHandler --- Engine/Generic/ModuleDependencyHandler.cs | 23 ++++-------- Engine/ScriptAnalyzer.cs | 35 +++++++++---------- .../Engine/ModuleDependencyHandler.tests.ps1 | 23 ++++++++++-- 3 files changed, 42 insertions(+), 39 deletions(-) diff --git a/Engine/Generic/ModuleDependencyHandler.cs b/Engine/Generic/ModuleDependencyHandler.cs index be6e24285..574a15dd5 100644 --- a/Engine/Generic/ModuleDependencyHandler.cs +++ b/Engine/Generic/ModuleDependencyHandler.cs @@ -238,30 +238,20 @@ private void RestorePSModulePath() /// Path to the user scoped temporary directory /// Path to the local app data directory public ModuleDependencyHandler( - Runspace runspace = null, + Runspace runspace, string moduleRepository = null, string tempPath = null, string localAppDataPath = null) { - if (runspace == null) - { - Runspace = RunspaceFactory.CreateRunspace(); - } - else - { - Runspace = runspace; - } - - if (Runspace.RunspaceStateInfo.State == RunspaceState.BeforeOpen) - { - Runspace.Open(); - } - else if (Runspace.RunspaceStateInfo.State != RunspaceState.Opened) + + ThrowIfNull(runspace, "runspace"); + if (runspace.RunspaceStateInfo.State != RunspaceState.Opened) { throw new ArgumentException(string.Format( - "Runspace state cannot be {0}", + "Runspace state cannot be in {0} state. It must be in Opened state", runspace.RunspaceStateInfo.State.ToString())); } + Runspace = runspace; // TODO should set PSSA environment variables outside this class // Should be set in ScriptAnalyzer class @@ -416,7 +406,6 @@ public static string GetModuleNameFromErrorExtent(ParseError error, ScriptBlockA /// public void Dispose() { - runspace.Dispose(); RestorePSModulePath(); } diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 443cc6597..d51503eaa 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -45,9 +45,6 @@ public sealed class ScriptAnalyzer List includeRegexList; List excludeRegexList; bool suppressedOnly; - Runspace runspace; - ModuleDependencyHandler moduleHandler; - #endregion #region Singleton @@ -170,10 +167,6 @@ public void CleanUp() includeRegexList = null; excludeRegexList = null; suppressedOnly = false; - if (moduleHandler != null) - { - moduleHandler.Dispose(); - } } internal bool ParseProfile(object profileObject, PathIntrinsics path, IOutputWriter writer) @@ -486,10 +479,6 @@ private void Initialize( this.outputWriter = outputWriter; - // TODO Create a runspace pool - runspace = RunspaceFactory.CreateRunspace(); - moduleHandler = resolveDSCResourceDependency ? new ModuleDependencyHandler(runspace) : null; - #region Verifies rule extensions and loggers path List paths = this.GetValidCustomRulePaths(customizedRulePath, path); @@ -1176,16 +1165,22 @@ public IEnumerable AnalyzePath(string path, bool searchRecursi // is an optimization over doing the whole operation at once // and calling .Concat on IEnumerables to join results. this.BuildScriptPathList(path, searchRecursively, scriptFilePaths); - - foreach (string scriptFilePath in scriptFilePaths) + using (var rsp = RunspaceFactory.CreateRunspace()) { - // Yield each record in the result so that the - // caller can pull them one at a time - foreach (var diagnosticRecord in this.AnalyzeFile(scriptFilePath)) + rsp.Open(); + using (var moduleHandler = new ModuleDependencyHandler(rsp)) { - yield return diagnosticRecord; + foreach (string scriptFilePath in scriptFilePaths) + { + // Yield each record in the result so that the + // caller can pull them one at a time + foreach (var diagnosticRecord in this.AnalyzeFile(scriptFilePath, moduleHandler)) + { + yield return diagnosticRecord; + } + } } - } + } // disposing the runspace also closes it if it not already closed } /// @@ -1287,7 +1282,9 @@ private void BuildScriptPathList( } - private IEnumerable AnalyzeFile(string filePath) + private IEnumerable AnalyzeFile( + string filePath, + ModuleDependencyHandler moduleHandler = null) { ScriptBlockAst scriptAst = null; Token[] scriptTokens = null; diff --git a/Tests/Engine/ModuleDependencyHandler.tests.ps1 b/Tests/Engine/ModuleDependencyHandler.tests.ps1 index 03ed26c7b..41cfecada 100644 --- a/Tests/Engine/ModuleDependencyHandler.tests.ps1 +++ b/Tests/Engine/ModuleDependencyHandler.tests.ps1 @@ -26,10 +26,13 @@ Describe "Resolve DSC Resource Dependency" { } Context "Module handler class" { + $moduleHandlerType = [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.ModuleDependencyHandler] $oldEnvVars = Get-Item Env:\* | Sort-Object -Property Key $oldPSModulePath = $env:PSModulePath It "Sets defaults correctly" { - $depHandler = [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.ModuleDependencyHandler]::new() + $rsp = [runspacefactory]::CreateRunspace() + $rsp.Open() + $depHandler = $moduleHandlerType::new($rsp) $expectedPath = [System.IO.Path]::GetTempPath() $depHandler.TempPath | Should Be $expectedPath @@ -47,11 +50,22 @@ Describe "Resolve DSC Resource Dependency" { $env:PSModulePath | Should Be $expectedPSModulePath $depHandler.Dispose() + $rsp.Dispose() } It "Keeps the environment variables unchanged" { Test-EnvironmentVariables($oldEnvVars) } + + It "Throws if runspace is null" { + {$moduleHandlerType::new($null)} | Should Throw + } + + It "Throws if runspace is not opened" { + $rsp = [runspacefactory]::CreateRunspace() + {$moduleHandlerType::new($rsp)} | Should Throw + $rsp.Dispose() + } } Context "Invoke-ScriptAnalyzer without switch" { @@ -82,10 +96,13 @@ Describe "Resolve DSC Resource Dependency" { New-Item -Type Directory -Path $newTempPath # create and dispose module dependency handler object - # to setup the temporary module location - $depHandler = [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.ModuleDependencyHandler]::new() + # to setup the temporary module + $rsp = [runspacefactory]::CreateRunspace() + $rsp.Open() + $depHandler = [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.ModuleDependencyHandler]::new($rsp) $pssaAppDataPath = $depHandler.PSSAAppDataPath $tempModulePath = $depHandler.TempModulePath + $rsp.Dispose() $depHandler.Dispose() # copy myresource module to the temporary location From 66eeb9d4502d8c11013bb4f09bffd0932d8f21d2 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 12 May 2016 16:33:17 -0700 Subject: [PATCH 16/23] Rename DSC in switch name to Dsc and change module handler usage ModuleDependencyHandler class usage is changed to utilize the IDisposable pattern. It is instantiated only before analyzing the files and disposed right after that. --- .../Commands/InvokeScriptAnalyzerCommand.cs | 10 ++-- Engine/ScriptAnalyzer.cs | 46 ++++++++++++------- 2 files changed, 35 insertions(+), 21 deletions(-) diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index d93c89593..76a3f700a 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -192,12 +192,12 @@ public object Settings /// Resolve DSC resoure dependency /// [Parameter(Mandatory = false)] - public SwitchParameter ResolveDSCResourceDependency + public SwitchParameter ResolveDscResourceDependency { - get { return resolveDSCResourceDependency; } - set { resolveDSCResourceDependency = value; } + get { return resolveDscResourceDependency; } + set { resolveDscResourceDependency = value; } } - private bool resolveDSCResourceDependency; + private bool resolveDscResourceDependency; #endregion Parameters #region Overrides @@ -224,7 +224,7 @@ protected override void BeginProcessing() this.severity, null == rulePaths ? true : this.includeDefaultRules, this.suppressedOnly, - resolveDSCResourceDependency: resolveDSCResourceDependency); + resolveDscResourceDependency: resolveDscResourceDependency); } /// diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index d51503eaa..43d11a3c5 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -45,13 +45,12 @@ public sealed class ScriptAnalyzer List includeRegexList; List excludeRegexList; bool suppressedOnly; + private bool resolveDscResourceDependency; #endregion #region Singleton private static object syncRoot = new Object(); - private static ScriptAnalyzer instance; - public static ScriptAnalyzer Instance { get @@ -103,7 +102,7 @@ internal void Initialize( string[] severity = null, bool includeDefaultRules = false, bool suppressedOnly = false, - bool resolveDSCResourceDependency = false) + bool resolveDscResourceDependency = false) where TCmdlet : PSCmdlet, IOutputWriter { if (cmdlet == null) @@ -121,7 +120,7 @@ internal void Initialize( severity, includeDefaultRules, suppressedOnly, - resolveDSCResourceDependency: resolveDSCResourceDependency); + resolveDscResourceDependency: resolveDscResourceDependency); } /// @@ -470,7 +469,7 @@ private void Initialize( bool includeDefaultRules = false, bool suppressedOnly = false, string profile = null, - bool resolveDSCResourceDependency = false) + bool resolveDscResourceDependency = false) { if (outputWriter == null) { @@ -478,6 +477,7 @@ private void Initialize( } this.outputWriter = outputWriter; + this.resolveDscResourceDependency = resolveDscResourceDependency; #region Verifies rule extensions and loggers path @@ -1165,22 +1165,36 @@ public IEnumerable AnalyzePath(string path, bool searchRecursi // is an optimization over doing the whole operation at once // and calling .Concat on IEnumerables to join results. this.BuildScriptPathList(path, searchRecursively, scriptFilePaths); - using (var rsp = RunspaceFactory.CreateRunspace()) + if (resolveDscResourceDependency) { - rsp.Open(); - using (var moduleHandler = new ModuleDependencyHandler(rsp)) + using (var rsp = RunspaceFactory.CreateRunspace()) { - foreach (string scriptFilePath in scriptFilePaths) + rsp.Open(); + using (var moduleHandler = new ModuleDependencyHandler(rsp)) { - // Yield each record in the result so that the - // caller can pull them one at a time - foreach (var diagnosticRecord in this.AnalyzeFile(scriptFilePath, moduleHandler)) - { - yield return diagnosticRecord; - } + return AnalyzePaths(scriptFilePaths, moduleHandler); } + } // disposing the runspace also closes it if it not already closed + } + else + { + return AnalyzePaths(scriptFilePaths, null); + } + } + + private IEnumerable AnalyzePaths( + IEnumerable scriptFilePaths, + ModuleDependencyHandler moduleHandler) + { + foreach (string scriptFilePath in scriptFilePaths) + { + // Yield each record in the result so that the + // caller can pull them one at a time + foreach (var diagnosticRecord in this.AnalyzeFile(scriptFilePath, moduleHandler)) + { + yield return diagnosticRecord; } - } // disposing the runspace also closes it if it not already closed + } } /// From 9b02e80788d919d3126e49423cd03a4170ce1a1f Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 16 May 2016 14:17:25 -0700 Subject: [PATCH 17/23] Parse an array of module names for import-dscresource --- Engine/Generic/ModuleDependencyHandler.cs | 118 ++++++++++++++++-- Engine/ScriptAnalyzer.cs | 22 ++-- .../Engine/ModuleDependencyHandler.tests.ps1 | 30 +++++ 3 files changed, 149 insertions(+), 21 deletions(-) diff --git a/Engine/Generic/ModuleDependencyHandler.cs b/Engine/Generic/ModuleDependencyHandler.cs index 574a15dd5..08d354a0d 100644 --- a/Engine/Generic/ModuleDependencyHandler.cs +++ b/Engine/Generic/ModuleDependencyHandler.cs @@ -158,7 +158,7 @@ private void SetupPSSAAppData() SetupTempDir(); } - private bool IsModulePresent(string moduleName) + private bool IsModulePresentInTempModulePath(string moduleName) { foreach (var dir in Directory.EnumerateDirectories(TempModulePath)) { @@ -335,7 +335,7 @@ public bool TrySaveModule(string moduleName) public void SaveModule(string moduleName) { ThrowIfNull(moduleName, "moduleName"); - if (IsModulePresent(moduleName)) + if (IsModulePresentInTempModulePath(moduleName)) { return; } @@ -350,6 +350,60 @@ public void SaveModule(string moduleName) } } + /// + /// Encapsulates Get-Module to check the availability of the module on the system + /// + /// + /// True indicating the presence of the module, otherwise false + public bool IsModuleAvailable(string moduleName) + { + ThrowIfNull(moduleName, "moduleName"); + IEnumerable availableModules; + using (var ps = System.Management.Automation.PowerShell.Create()) + { + ps.Runspace = runspace; + availableModules = ps.AddCommand("Get-Module") + .AddParameter("Name", moduleName) + .AddParameter("ListAvailable") + .Invoke(); + } + return availableModules != null ? availableModules.Any() : false; + } + + /// + /// Extracts out the module names from the error extent that are not available + /// + /// This handles the following case. + /// Import-DSCResourceModule -ModuleName ModulePresent,ModuleAbsent + /// + /// ModulePresent is present in PSModulePath whereas ModuleAbsent is not. + /// But the error exent coverts the entire extent and hence we need to check + /// which module is actually not present so as to be downloaded + /// + /// + /// + /// An enumeration over the module names that are not available + public IEnumerable GetUnavailableModuleNameFromErrorExtent(ParseError error, ScriptBlockAst ast) + { + ThrowIfNull(error, "error"); + ThrowIfNull(ast, "ast"); + var moduleNames = ModuleDependencyHandler.GetModuleNameFromErrorExtent(error, ast); + if (moduleNames == null) + { + return null; + } + var unavailableModules = new List(); + foreach (var moduleName in moduleNames) + { + if (!IsModuleAvailable(moduleName)) + { + unavailableModules.Add(moduleName); + } + } + //return moduleNames.Where(x => !IsModuleAvailable(x)); + return unavailableModules; + } + /// /// Get the module name from the error extent /// @@ -362,7 +416,7 @@ public void SaveModule(string moduleName) /// Parse error /// AST of the script that contians the parse error /// The name of the module that caused the parser to throw the error. Returns null if it cannot extract the module name. - public static string GetModuleNameFromErrorExtent(ParseError error, ScriptBlockAst ast) + public static IEnumerable GetModuleNameFromErrorExtent(ParseError error, ScriptBlockAst ast) { ThrowIfNull(error, "error"); ThrowIfNull(ast, "ast"); @@ -373,9 +427,10 @@ public static string GetModuleNameFromErrorExtent(ParseError error, ScriptBlockA return null; } // check if the command name is import-dscmodule - // right now we handle only the following form - // Import-DSCResource -ModuleName xActiveDirectory - if (dynamicKywdAst.CommandElements.Count != 3) + // right now we handle only the following forms + // 1. Import-DSCResourceModule -ModuleName somemodule + // 2. Import-DSCResourceModule -ModuleName somemodule1,somemodule2 + if (dynamicKywdAst.CommandElements.Count < 3) { return null; } @@ -386,19 +441,56 @@ public static string GetModuleNameFromErrorExtent(ParseError error, ScriptBlockA return null; } - var paramAst = dynamicKywdAst.CommandElements[1] as CommandParameterAst; - if (paramAst == null || !paramAst.ParameterName.Equals("ModuleName", StringComparison.OrdinalIgnoreCase)) + // find a parameter named modulename + int k; + for (k = 1; k < dynamicKywdAst.CommandElements.Count; k++) { - return null; + var paramAst = dynamicKywdAst.CommandElements[1] as CommandParameterAst; + // TODO match the initial letters only + if (paramAst == null || !paramAst.ParameterName.Equals("ModuleName", StringComparison.OrdinalIgnoreCase)) + { + continue; + } + break; } - - var paramValAst = dynamicKywdAst.CommandElements[2] as StringConstantExpressionAst; - if (paramValAst == null) + + if (k == dynamicKywdAst.CommandElements.Count) { + // cannot find modulename return null; } + var modules = new List(); + + // k < count - 1, because only -ModuleName throws parse error and hence not possible + var paramValAst = dynamicKywdAst.CommandElements[++k]; - return paramValAst.Value; + // import-dscresource -ModuleName module1 + var paramValStrConstExprAst = paramValAst as StringConstantExpressionAst; + if (paramValStrConstExprAst != null) + { + modules.Add(paramValStrConstExprAst.Value); + return modules; + } + + // import-dscresource -ModuleName module1,module2 + var paramValArrLtrlAst = paramValAst as ArrayLiteralAst; + if (paramValArrLtrlAst != null) + { + foreach (var elem in paramValArrLtrlAst.Elements) + { + var elemStrConstExprAst = elem as StringConstantExpressionAst; + if (elemStrConstExprAst != null) + { + modules.Add(elemStrConstExprAst.Value); + } + } + if (modules.Count == 0) + { + return null; + } + return modules; + } + return null; } /// diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 43d11a3c5..503b19461 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -1165,6 +1165,7 @@ public IEnumerable AnalyzePath(string path, bool searchRecursi // is an optimization over doing the whole operation at once // and calling .Concat on IEnumerables to join results. this.BuildScriptPathList(path, searchRecursively, scriptFilePaths); + IEnumerable diagnosticRecords; if (resolveDscResourceDependency) { using (var rsp = RunspaceFactory.CreateRunspace()) @@ -1172,29 +1173,35 @@ public IEnumerable AnalyzePath(string path, bool searchRecursi rsp.Open(); using (var moduleHandler = new ModuleDependencyHandler(rsp)) { - return AnalyzePaths(scriptFilePaths, moduleHandler); + // cannot use IEnumerable because the execution is deferred + // which causes the code to go out of the "using" scope which + // in turn closes the runspace + diagnosticRecords = AnalyzePaths(scriptFilePaths, moduleHandler); } } // disposing the runspace also closes it if it not already closed } else { - return AnalyzePaths(scriptFilePaths, null); + diagnosticRecords = AnalyzePaths(scriptFilePaths, null); } + return diagnosticRecords; } - private IEnumerable AnalyzePaths( + private List AnalyzePaths( IEnumerable scriptFilePaths, ModuleDependencyHandler moduleHandler) { + var diagnosticRecords = new List(); foreach (string scriptFilePath in scriptFilePaths) { // Yield each record in the result so that the // caller can pull them one at a time foreach (var diagnosticRecord in this.AnalyzeFile(scriptFilePath, moduleHandler)) { - yield return diagnosticRecord; + diagnosticRecords.Add(diagnosticRecord); } } + return diagnosticRecords; } /// @@ -1327,11 +1334,10 @@ private IEnumerable AnalyzeFile( { foreach (ParseError error in errors.Where(IsModuleNotFoundError)) { - var moduleName = ModuleDependencyHandler.GetModuleNameFromErrorExtent(error, scriptAst); - if (moduleName != null - && moduleHandler.TrySaveModule(moduleName)) + var moduleNames = moduleHandler.GetUnavailableModuleNameFromErrorExtent(error, scriptAst); + if (moduleNames != null) { - parseAgain = true; + parseAgain |= moduleNames.Any(x => moduleHandler.TrySaveModule(x)); } } } diff --git a/Tests/Engine/ModuleDependencyHandler.tests.ps1 b/Tests/Engine/ModuleDependencyHandler.tests.ps1 index 41cfecada..be1ebfc25 100644 --- a/Tests/Engine/ModuleDependencyHandler.tests.ps1 +++ b/Tests/Engine/ModuleDependencyHandler.tests.ps1 @@ -66,6 +66,36 @@ Describe "Resolve DSC Resource Dependency" { {$moduleHandlerType::new($rsp)} | Should Throw $rsp.Dispose() } + + It "Extracts 1 module name" { + $sb = @" +{Configuration SomeConfiguration +{ + Import-DscResource -ModuleName SomeDscModule1 +}} +"@ + $tokens = $null + $parseError = $null + $ast = [System.Management.Automation.Language.Parser]::ParseInput($sb, [ref]$tokens, [ref]$parseError) + $resultModuleNames = $moduleHandlerType::GetModuleNameFromErrorExtent($parseError[0], $ast).ToArray() + $resultModuleNames[0] | Should Be 'SomeDscModule1' + } + + It "Extracts more than 1 module names" { + $sb = @" +{Configuration SomeConfiguration +{ + Import-DscResource -ModuleName SomeDscModule1,SomeDscModule2,SomeDscModule3 +}} +"@ + $tokens = $null + $parseError = $null + $ast = [System.Management.Automation.Language.Parser]::ParseInput($sb, [ref]$tokens, [ref]$parseError) + $resultModuleNames = $moduleHandlerType::GetModuleNameFromErrorExtent($parseError[0], $ast).ToArray() + $resultModuleNames[0] | Should Be 'SomeDscModule1' + $resultModuleNames[1] | Should Be 'SomeDscModule2' + $resultModuleNames[2] | Should Be 'SomeDscModule3' + } } Context "Invoke-ScriptAnalyzer without switch" { From 750f8a4dd95d50057819c1dcc474fb4aca462a1e Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 16 May 2016 14:23:45 -0700 Subject: [PATCH 18/23] Add repository parameter to save module --- Engine/Generic/ModuleDependencyHandler.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/Engine/Generic/ModuleDependencyHandler.cs b/Engine/Generic/ModuleDependencyHandler.cs index 08d354a0d..1494816a1 100644 --- a/Engine/Generic/ModuleDependencyHandler.cs +++ b/Engine/Generic/ModuleDependencyHandler.cs @@ -345,6 +345,7 @@ public void SaveModule(string moduleName) ps.AddCommand("Save-Module") .AddParameter("Path", tempModulePath) .AddParameter("Name", moduleName) + .AddParameter("Repository", moduleRepository) .AddParameter("Force"); ps.Invoke(); } From 60fe6c92ef67bb21ef4af801245e03c499d43613 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 16 May 2016 17:14:27 -0700 Subject: [PATCH 19/23] Move module handler initialization outside scriptanalyzer class --- .../Commands/InvokeScriptAnalyzerCommand.cs | 86 ++++++--- Engine/ScriptAnalyzer.cs | 182 +++++++++--------- 2 files changed, 148 insertions(+), 120 deletions(-) diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index 76a3f700a..c54c6dcea 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -25,6 +25,7 @@ using System.Threading.Tasks; using System.Collections.Concurrent; using System.Threading; +using System.Management.Automation.Runspaces; namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Commands { @@ -44,7 +45,7 @@ public class InvokeScriptAnalyzerCommand : PSCmdlet, IOutputWriter [Parameter(Position = 0, ParameterSetName = "File", Mandatory = true, - ValueFromPipeline = true, + ValueFromPipeline = true, ValueFromPipelineByPropertyName = true)] [ValidateNotNull] [Alias("PSPath")] @@ -190,7 +191,7 @@ public object Settings /// /// Resolve DSC resoure dependency - /// + /// [Parameter(Mandatory = false)] public SwitchParameter ResolveDscResourceDependency { @@ -223,8 +224,7 @@ protected override void BeginProcessing() this.excludeRule, this.severity, null == rulePaths ? true : this.includeDefaultRules, - this.suppressedOnly, - resolveDscResourceDependency: resolveDscResourceDependency); + this.suppressedOnly); } /// @@ -238,18 +238,50 @@ protected override void ProcessRecord() return; } + // TODO Support dependency resolution for analyzing script definitions + if (resolveDscResourceDependency) + { + using (var rsp = RunspaceFactory.CreateRunspace()) + { + rsp.Open(); + using (var moduleHandler = new ModuleDependencyHandler(rsp)) + { + ScriptAnalyzer.Instance.ModuleHandler = moduleHandler; + ProcessInput(); + } + } + } + else + { + ProcessInput(); + } + } + + private void ProcessInput() + { + IEnumerable diagnosticsList = Enumerable.Empty(); if (String.Equals(this.ParameterSetName, "File", StringComparison.OrdinalIgnoreCase)) { - // throws Item Not Found Exception + // throws Item Not Found Exception Collection paths = this.SessionState.Path.GetResolvedPSPathFromPSPath(path); foreach (PathInfo p in paths) { - ProcessPathOrScriptDefinition(this.SessionState.Path.GetUnresolvedProviderPathFromPSPath(p.Path)); + diagnosticsList = ScriptAnalyzer.Instance.AnalyzePath( + this.SessionState.Path.GetUnresolvedProviderPathFromPSPath(p.Path), + this.recurse); } } else if (String.Equals(this.ParameterSetName, "ScriptDefinition", StringComparison.OrdinalIgnoreCase)) { - ProcessPathOrScriptDefinition(scriptDefinition); + diagnosticsList = ScriptAnalyzer.Instance.AnalyzeScriptDefinition(scriptDefinition); + } + + foreach (ILogger logger in ScriptAnalyzer.Instance.Loggers) + { + foreach (DiagnosticRecord diagnostic in diagnosticsList) + { + logger.LogObject(diagnostic, this); + } } } @@ -269,28 +301,28 @@ protected override void StopProcessing() #region Methods - private void ProcessPathOrScriptDefinition(string pathOrScriptDefinition) - { - IEnumerable diagnosticsList = Enumerable.Empty(); + //private void ProcessPathOrScriptDefinition(string pathOrScriptDefinition) + //{ + // IEnumerable diagnosticsList = Enumerable.Empty(); - if (String.Equals(this.ParameterSetName, "File", StringComparison.OrdinalIgnoreCase)) - { - diagnosticsList = ScriptAnalyzer.Instance.AnalyzePath(pathOrScriptDefinition, this.recurse); - } - else if (String.Equals(this.ParameterSetName, "ScriptDefinition", StringComparison.OrdinalIgnoreCase)) - { - diagnosticsList = ScriptAnalyzer.Instance.AnalyzeScriptDefinition(pathOrScriptDefinition); - } + // if (String.Equals(this.ParameterSetName, "File", StringComparison.OrdinalIgnoreCase)) + // { + // diagnosticsList = ScriptAnalyzer.Instance.AnalyzePath(pathOrScriptDefinition, this.recurse); + // } + // else if (String.Equals(this.ParameterSetName, "ScriptDefinition", StringComparison.OrdinalIgnoreCase)) + // { + // diagnosticsList = ScriptAnalyzer.Instance.AnalyzeScriptDefinition(pathOrScriptDefinition); + // } - //Output through loggers - foreach (ILogger logger in ScriptAnalyzer.Instance.Loggers) - { - foreach (DiagnosticRecord diagnostic in diagnosticsList) - { - logger.LogObject(diagnostic, this); - } - } - } + // //Output through loggers + // foreach (ILogger logger in ScriptAnalyzer.Instance.Loggers) + // { + // foreach (DiagnosticRecord diagnostic in diagnosticsList) + // { + // logger.LogObject(diagnostic, this); + // } + // } + //} #endregion } diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 503b19461..b6236c235 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -45,7 +45,7 @@ public sealed class ScriptAnalyzer List includeRegexList; List excludeRegexList; bool suppressedOnly; - private bool resolveDscResourceDependency; + ModuleDependencyHandler moduleHandler; #endregion #region Singleton @@ -86,6 +86,17 @@ public static ScriptAnalyzer Instance public IEnumerable DSCResourceRules { get; private set; } internal List ExternalRules { get; set; } + public ModuleDependencyHandler ModuleHandler { + get + { + return moduleHandler; + } + + internal set + { + moduleHandler = value; + } + } #endregion @@ -95,21 +106,20 @@ public static ScriptAnalyzer Instance /// Initialize : Initializes default rules, loggers and helper. /// internal void Initialize( - TCmdlet cmdlet, - string[] customizedRulePath = null, - string[] includeRuleNames = null, + TCmdlet cmdlet, + string[] customizedRulePath = null, + string[] includeRuleNames = null, string[] excludeRuleNames = null, string[] severity = null, bool includeDefaultRules = false, - bool suppressedOnly = false, - bool resolveDscResourceDependency = false) + bool suppressedOnly = false) where TCmdlet : PSCmdlet, IOutputWriter { if (cmdlet == null) { throw new ArgumentNullException("cmdlet"); } - + this.Initialize( cmdlet, cmdlet.SessionState.Path, @@ -119,18 +129,17 @@ internal void Initialize( excludeRuleNames, severity, includeDefaultRules, - suppressedOnly, - resolveDscResourceDependency: resolveDscResourceDependency); + suppressedOnly); } /// /// Initialize : Initializes default rules, loggers and helper. /// public void Initialize( - Runspace runspace, - IOutputWriter outputWriter, - string[] customizedRulePath = null, - string[] includeRuleNames = null, + Runspace runspace, + IOutputWriter outputWriter, + string[] customizedRulePath = null, + string[] includeRuleNames = null, string[] excludeRuleNames = null, string[] severity = null, bool includeDefaultRules = false, @@ -204,7 +213,7 @@ internal bool ParseProfile(object profileObject, PathIntrinsics path, IOutputWri hasError = ParseProfileString(profile, path, writer, severityList, includeRuleList, excludeRuleList); } } - + if (hasError) { return false; @@ -269,7 +278,7 @@ private bool ParseProfileHashtable(Hashtable profile, PathIntrinsics path, IOutp hasError = true; continue; } - + // checks whether it falls into list of valid keys if (!validKeys.Contains(key)) { @@ -325,7 +334,7 @@ private bool ParseProfileHashtable(Hashtable profile, PathIntrinsics path, IOutp } AddProfileItem(key, values, severityList, includeRuleList, excludeRuleList); - + } return hasError; @@ -468,17 +477,17 @@ private void Initialize( string[] severity, bool includeDefaultRules = false, bool suppressedOnly = false, - string profile = null, - bool resolveDscResourceDependency = false) + string profile = null) { if (outputWriter == null) { throw new ArgumentNullException("outputWriter"); } + this.moduleHandler = null; + this.outputWriter = outputWriter; - this.resolveDscResourceDependency = resolveDscResourceDependency; - + #region Verifies rule extensions and loggers path List paths = this.GetValidCustomRulePaths(customizedRulePath, path); @@ -578,7 +587,7 @@ private void Initialize( { this.outputWriter.ThrowTerminatingError( new ErrorRecord( - ex, + ex, ex.HResult.ToString("X", CultureInfo.CurrentCulture), ErrorCategory.NotSpecified, this)); } @@ -729,7 +738,7 @@ public IEnumerable GetRule(string[] moduleNames, string[] ruleNames) if (null != ScriptRules) { rules = ScriptRules.Union(TokenRules).Union(DSCResourceRules); - } + } // Gets PowerShell Rules. if (moduleNames != null) @@ -783,7 +792,7 @@ private List GetExternalRule(string[] moduleNames) { shortModuleName = loadedModules.First().Name; } - + // Invokes Get-Command and Get-Help for each functions in the module. posh.Commands.Clear(); posh.AddCommand("Get-Command").AddParameter("Module", shortModuleName); @@ -803,7 +812,7 @@ private List GetExternalRule(string[] moduleNames) item.Name.EndsWith("token", StringComparison.OrdinalIgnoreCase)); } catch - { + { } //Only add functions that are defined as rules. @@ -813,7 +822,7 @@ private List GetExternalRule(string[] moduleNames) // using Update-Help. This results in an interactive prompt - which we cannot handle // Workaround to prevent Update-Help from running is to set the following reg key // HKLM:\Software\Microsoft\PowerShell\DisablePromptToUpdateHelp - // OR execute Update-Help in an elevated admin mode before running ScriptAnalyzer + // OR execute Update-Help in an elevated admin mode before running ScriptAnalyzer Collection helpContent = null; try { @@ -833,11 +842,11 @@ private List GetExternalRule(string[] moduleNames) dynamic description = helpContent[0].Properties["Description"]; if (null != description && null != description.Value && description.Value.GetType().IsArray) - { + { desc = description.Value[0].Text; } } - + rules.Add(new ExternalRule(funcInfo.Name, funcInfo.Name, desc, param.Name, param.ParameterType.FullName, funcInfo.ModuleName, funcInfo.Module.Path)); } @@ -982,7 +991,7 @@ internal IEnumerable GetExternalRecord(Ast ast, Token[] token, // DiagnosticRecord may not be correctly returned from external rule. try - { + { severity = (DiagnosticSeverity)Enum.Parse(typeof(DiagnosticSeverity), psobject.Properties["Severity"].Value.ToString()); message = psobject.Properties["Message"].Value.ToString(); extent = (IScriptExtent)psobject.Properties["Extent"].Value; @@ -1030,11 +1039,11 @@ public Dictionary> CheckRuleExtension(string[] path, PathIn string resolvedPath = string.Empty; - // Users may provide a valid module path or name, + // Users may provide a valid module path or name, // We have to identify the childPath is really a directory or just a module name. // You can also consider following two commands. // Get-ScriptAnalyzerRule -RuleExtension "ContosoAnalyzerRules" - // Get-ScriptAnalyzerRule -RuleExtension "%USERPROFILE%\WindowsPowerShell\Modules\ContosoAnalyzerRules" + // Get-ScriptAnalyzerRule -RuleExtension "%USERPROFILE%\WindowsPowerShell\Modules\ContosoAnalyzerRules" if (Path.GetDirectoryName(childPath) == string.Empty) { resolvedPath = childPath; @@ -1043,21 +1052,21 @@ public Dictionary> CheckRuleExtension(string[] path, PathIn { resolvedPath = basePath .GetResolvedPSPathFromPSPath(childPath).First().ToString(); - } - + } + // Import the module - InitialSessionState state = InitialSessionState.CreateDefault2(); + InitialSessionState state = InitialSessionState.CreateDefault2(); using (System.Management.Automation.PowerShell posh = System.Management.Automation.PowerShell.Create(state)) - { + { posh.AddCommand("Import-Module").AddArgument(resolvedPath).AddParameter("PassThru"); Collection loadedModules = posh.Invoke(); - if (loadedModules != null + if (loadedModules != null && loadedModules.Count > 0 && loadedModules.First().ExportedFunctions.Count > 0) - { - validModPaths.Add(resolvedPath); - } + { + validModPaths.Add(resolvedPath); + } } } catch @@ -1136,14 +1145,14 @@ public Dictionary> CheckRuleExtension(string[] path, PathIn } #endregion - + /// /// Analyzes a script file or a directory containing script files. /// /// The path of the file or directory to analyze. /// - /// If true, recursively searches the given file path and analyzes any + /// If true, recursively searches the given file path and analyzes any /// script files that are found. /// /// An enumeration of DiagnosticRecords that were found by rules. @@ -1157,7 +1166,7 @@ public IEnumerable AnalyzePath(string path, bool searchRecursi new ErrorRecord( new FileNotFoundException(), string.Format(CultureInfo.CurrentCulture, Strings.FileNotFound, path), - ErrorCategory.InvalidArgument, + ErrorCategory.InvalidArgument, this)); } @@ -1165,45 +1174,34 @@ public IEnumerable AnalyzePath(string path, bool searchRecursi // is an optimization over doing the whole operation at once // and calling .Concat on IEnumerables to join results. this.BuildScriptPathList(path, searchRecursively, scriptFilePaths); - IEnumerable diagnosticRecords; - if (resolveDscResourceDependency) - { - using (var rsp = RunspaceFactory.CreateRunspace()) - { - rsp.Open(); - using (var moduleHandler = new ModuleDependencyHandler(rsp)) - { - // cannot use IEnumerable because the execution is deferred - // which causes the code to go out of the "using" scope which - // in turn closes the runspace - diagnosticRecords = AnalyzePaths(scriptFilePaths, moduleHandler); - } - } // disposing the runspace also closes it if it not already closed - } - else - { - diagnosticRecords = AnalyzePaths(scriptFilePaths, null); - } - return diagnosticRecords; - } - - private List AnalyzePaths( - IEnumerable scriptFilePaths, - ModuleDependencyHandler moduleHandler) - { - var diagnosticRecords = new List(); foreach (string scriptFilePath in scriptFilePaths) { // Yield each record in the result so that the // caller can pull them one at a time - foreach (var diagnosticRecord in this.AnalyzeFile(scriptFilePath, moduleHandler)) + foreach (var diagnosticRecord in this.AnalyzeFile(scriptFilePath)) { - diagnosticRecords.Add(diagnosticRecord); + yield return diagnosticRecord; } } - return diagnosticRecords; } + //private List AnalyzePaths( + // IEnumerable scriptFilePaths, + // ModuleDependencyHandler moduleHandler) + //{ + // var diagnosticRecords = new List(); + // foreach (string scriptFilePath in scriptFilePaths) + // { + // // Yield each record in the result so that the + // // caller can pull them one at a time + // foreach (var diagnosticRecord in this.AnalyzeFile(scriptFilePath, moduleHandler)) + // { + // diagnosticRecords.Add(diagnosticRecord); + // } + // } + // return diagnosticRecords; + //} + /// /// Analyzes a script definition in the form of a string input /// @@ -1248,8 +1246,8 @@ public IEnumerable AnalyzeScriptDefinition(string scriptDefini } private void BuildScriptPathList( - string path, - bool searchRecursively, + string path, + bool searchRecursively, IList scriptFilePaths) { const string ps1Suffix = ".ps1"; @@ -1295,17 +1293,15 @@ private void BuildScriptPathList( { this.outputWriter.WriteError( new ErrorRecord( - new FileNotFoundException(), - string.Format(CultureInfo.CurrentCulture, Strings.FileNotFound, path), - ErrorCategory.InvalidArgument, + new FileNotFoundException(), + string.Format(CultureInfo.CurrentCulture, Strings.FileNotFound, path), + ErrorCategory.InvalidArgument, this)); } - } + } - - private IEnumerable AnalyzeFile( - string filePath, - ModuleDependencyHandler moduleHandler = null) + + private IEnumerable AnalyzeFile(string filePath) { ScriptBlockAst scriptAst = null; Token[] scriptTokens = null; @@ -1334,7 +1330,7 @@ private IEnumerable AnalyzeFile( { foreach (ParseError error in errors.Where(IsModuleNotFoundError)) { - var moduleNames = moduleHandler.GetUnavailableModuleNameFromErrorExtent(error, scriptAst); + var moduleNames = moduleHandler.GetUnavailableModuleNameFromErrorExtent(error, scriptAst); if (moduleNames != null) { parseAgain |= moduleNames.Any(x => moduleHandler.TrySaveModule(x)); @@ -1388,17 +1384,17 @@ private bool IsModuleNotFoundError(ParseError error) private bool IsSeverityAllowed(IEnumerable allowedSeverities, IRule rule) { - return severity == null - || (allowedSeverities != null - && rule != null - && HasGetSeverity(rule) + return severity == null + || (allowedSeverities != null + && rule != null + && HasGetSeverity(rule) && allowedSeverities.Contains((uint)rule.GetSeverity())); } IEnumerable GetAllowedSeveritiesInInt() { - return severity != null - ? severity.Select(item => (uint)Enum.Parse(typeof(DiagnosticSeverity), item, true)) + return severity != null + ? severity.Select(item => (uint)Enum.Parse(typeof(DiagnosticSeverity), item, true)) : null; } @@ -1479,8 +1475,8 @@ private Tuple, List> SuppressRule( /// /// An enumeration of DiagnosticRecords that were found by rules. public IEnumerable AnalyzeSyntaxTree( - ScriptBlockAst scriptAst, - Token[] scriptTokens, + ScriptBlockAst scriptAst, + Token[] scriptTokens, string filePath) { Dictionary> ruleSuppressions = new Dictionary>(); @@ -1522,7 +1518,7 @@ public IEnumerable AnalyzeSyntaxTree( Helper.Instance.Tokens = scriptTokens; } - + #region Run ScriptRules //Trim down to the leaf element of the filePath and pass it to Diagnostic Record string fileName = filePathIsNullOrWhiteSpace ? String.Empty : System.IO.Path.GetFileName(filePath); @@ -1554,8 +1550,8 @@ public IEnumerable AnalyzeSyntaxTree( List suppressRuleErrors; var ruleRecords = scriptRule.AnalyzeScript(scriptAst, scriptAst.Extent.File).ToList(); var records = Helper.Instance.SuppressRule( - scriptRule.GetName(), - ruleSuppressions, + scriptRule.GetName(), + ruleSuppressions, ruleRecords, out suppressRuleErrors); result.AddRange(suppressRuleErrors); From 09588c2bff37232d15f5943542a5ea003ebd5705 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 16 May 2016 19:55:53 -0700 Subject: [PATCH 20/23] Fix failing tests --- .../Commands/InvokeScriptAnalyzerCommand.cs | 64 +++++++------------ Engine/ScriptAnalyzer.cs | 19 +----- 2 files changed, 23 insertions(+), 60 deletions(-) diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index c54c6dcea..d67c423e8 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -190,7 +190,7 @@ public object Settings private bool stopProcessing; /// - /// Resolve DSC resoure dependency + /// Resolve DSC resource dependency /// [Parameter(Mandatory = false)] public SwitchParameter ResolveDscResourceDependency @@ -257,6 +257,21 @@ protected override void ProcessRecord() } } + protected override void EndProcessing() + { + ScriptAnalyzer.Instance.CleanUp(); + base.EndProcessing(); + } + + protected override void StopProcessing() + { + ScriptAnalyzer.Instance.CleanUp(); + base.StopProcessing(); + } + + #endregion + + #region Methods private void ProcessInput() { IEnumerable diagnosticsList = Enumerable.Empty(); @@ -269,61 +284,26 @@ private void ProcessInput() diagnosticsList = ScriptAnalyzer.Instance.AnalyzePath( this.SessionState.Path.GetUnresolvedProviderPathFromPSPath(p.Path), this.recurse); + WriteToOutput(diagnosticsList); } } else if (String.Equals(this.ParameterSetName, "ScriptDefinition", StringComparison.OrdinalIgnoreCase)) { diagnosticsList = ScriptAnalyzer.Instance.AnalyzeScriptDefinition(scriptDefinition); + WriteToOutput(diagnosticsList); } + } + private void WriteToOutput(IEnumerable diagnosticRecords) + { foreach (ILogger logger in ScriptAnalyzer.Instance.Loggers) { - foreach (DiagnosticRecord diagnostic in diagnosticsList) + foreach (DiagnosticRecord diagnostic in diagnosticRecords) { logger.LogObject(diagnostic, this); } } } - - protected override void EndProcessing() - { - ScriptAnalyzer.Instance.CleanUp(); - base.EndProcessing(); - } - - protected override void StopProcessing() - { - ScriptAnalyzer.Instance.CleanUp(); - base.StopProcessing(); - } - - #endregion - - #region Methods - - //private void ProcessPathOrScriptDefinition(string pathOrScriptDefinition) - //{ - // IEnumerable diagnosticsList = Enumerable.Empty(); - - // if (String.Equals(this.ParameterSetName, "File", StringComparison.OrdinalIgnoreCase)) - // { - // diagnosticsList = ScriptAnalyzer.Instance.AnalyzePath(pathOrScriptDefinition, this.recurse); - // } - // else if (String.Equals(this.ParameterSetName, "ScriptDefinition", StringComparison.OrdinalIgnoreCase)) - // { - // diagnosticsList = ScriptAnalyzer.Instance.AnalyzeScriptDefinition(pathOrScriptDefinition); - // } - - // //Output through loggers - // foreach (ILogger logger in ScriptAnalyzer.Instance.Loggers) - // { - // foreach (DiagnosticRecord diagnostic in diagnosticsList) - // { - // logger.LogObject(diagnostic, this); - // } - // } - //} - #endregion } } \ No newline at end of file diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index b6236c235..1f8e239c1 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -1170,7 +1170,7 @@ public IEnumerable AnalyzePath(string path, bool searchRecursi this)); } - // Precreate the list of script file paths to analyze. This + // Create in advance the list of script file paths to analyze. This // is an optimization over doing the whole operation at once // and calling .Concat on IEnumerables to join results. this.BuildScriptPathList(path, searchRecursively, scriptFilePaths); @@ -1185,23 +1185,6 @@ public IEnumerable AnalyzePath(string path, bool searchRecursi } } - //private List AnalyzePaths( - // IEnumerable scriptFilePaths, - // ModuleDependencyHandler moduleHandler) - //{ - // var diagnosticRecords = new List(); - // foreach (string scriptFilePath in scriptFilePaths) - // { - // // Yield each record in the result so that the - // // caller can pull them one at a time - // foreach (var diagnosticRecord in this.AnalyzeFile(scriptFilePath, moduleHandler)) - // { - // diagnosticRecords.Add(diagnosticRecord); - // } - // } - // return diagnosticRecords; - //} - /// /// Analyzes a script definition in the form of a string input /// From 5081eda64a1e6b7b0efcf54cc168fe52af96018c Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Tue, 17 May 2016 09:53:36 -0700 Subject: [PATCH 21/23] Rename ResolveDscResourceDependency to SaveDscResourceDependency --- Engine/Commands/InvokeScriptAnalyzerCommand.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index d67c423e8..bbaf6fcab 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -193,12 +193,12 @@ public object Settings /// Resolve DSC resource dependency /// [Parameter(Mandatory = false)] - public SwitchParameter ResolveDscResourceDependency + public SwitchParameter SaveDscResourceDependency { - get { return resolveDscResourceDependency; } - set { resolveDscResourceDependency = value; } + get { return saveDscResourceDependency; } + set { saveDscResourceDependency = value; } } - private bool resolveDscResourceDependency; + private bool saveDscResourceDependency; #endregion Parameters #region Overrides @@ -239,7 +239,7 @@ protected override void ProcessRecord() } // TODO Support dependency resolution for analyzing script definitions - if (resolveDscResourceDependency) + if (saveDscResourceDependency) { using (var rsp = RunspaceFactory.CreateRunspace()) { From 38e3708a7049e1aa6bfd660f3c393dc852078779 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Tue, 17 May 2016 10:14:48 -0700 Subject: [PATCH 22/23] Add test to check SaveDscResourceDependency parameter --- Tests/Engine/InvokeScriptAnalyzer.tests.ps1 | 34 ++++++++++++------- .../Engine/ModuleDependencyHandler.tests.ps1 | 2 +- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 index 953a4f6c1..780f0e571 100644 --- a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 +++ b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 @@ -19,7 +19,7 @@ Describe "Test available parameters" { It "has a Path parameter" { $params.ContainsKey("Path") | Should Be $true } - + It "accepts string" { $params["Path"].ParameterType.FullName | Should Be "System.String" } @@ -29,8 +29,8 @@ Describe "Test available parameters" { It "has a ScriptDefinition parameter" { $params.ContainsKey("ScriptDefinition") | Should Be $true } - - It "accepts string" { + + It "accepts string" { $params["ScriptDefinition"].ParameterType.FullName | Should Be "System.String" } } @@ -69,6 +69,16 @@ Describe "Test available parameters" { } } + Context "SaveDSCResourceDependency parameter" { + It "has the parameter" { + $params.ContainsKey("SaveDscResourceDependency") | Should Be $true + } + + It "is a switch parameter" { + $params["SaveDscResourceDependency"].ParameterType.FullName | Should Be "System.Management.Automation.SwitchParameter" + } + } + Context "It has 2 parameter sets: File and ScriptDefinition" { It "Has 2 parameter sets" { $sa.ParameterSets.Count | Should Be 2 @@ -144,14 +154,14 @@ Describe "Test Path" { if (!$testingLibraryUsage) { #There is probably a more concise way to do this but for now we will settle for this! - Function GetFreeDrive ($freeDriveLen) { + Function GetFreeDrive ($freeDriveLen) { $ordA = 65 $ordZ = 90 $freeDrive = "" $freeDriveName = "" do{ $freeDriveName = (1..$freeDriveLen | %{[char](Get-Random -Maximum $ordZ -Minimum $ordA)}) -join '' - $freeDrive = $freeDriveName + ":" + $freeDrive = $freeDriveName + ":" }while(Test-Path $freeDrive) $freeDrive, $freeDriveName } @@ -180,7 +190,7 @@ Describe "Test Path" { Context "When given a directory" { $withoutPathWithDirectory = Invoke-ScriptAnalyzer -Recurse $directory\RecursionDirectoryTest $withPathWithDirectory = Invoke-ScriptAnalyzer -Recurse -Path $directory\RecursionDirectoryTest - + It "Has the same count as without Path parameter"{ $withoutPathWithDirectory.Count -eq $withPathWithDirectory.Count | Should Be $true } @@ -206,7 +216,7 @@ Describe "Test ExcludeRule" { It "excludes 3 rules" { $noViolations = Invoke-ScriptAnalyzer $directory\..\Rules\BadCmdlet.ps1 -ExcludeRule $rules | Where-Object {$rules -contains $_.RuleName} - $noViolations.Count | Should Be 0 + $noViolations.Count | Should Be 0 } } @@ -329,13 +339,13 @@ Describe "Test CustomizedRulePath" { It "When supplied with a collection of paths" { $customizedRulePath = Invoke-ScriptAnalyzer $directory\TestScript.ps1 -CustomRulePath ("$directory\CommunityAnalyzerRules", "$directory\SampleRule", "$directory\SampleRule\SampleRule2") $customizedRulePath.Count | Should Be 3 - } + } } Context "When used incorrectly" { - It "file cannot be found" { + It "file cannot be found" { try { Invoke-ScriptAnalyzer $directory\TestScript.ps1 -CustomRulePath "Invalid CustomRulePath" @@ -343,9 +353,9 @@ Describe "Test CustomizedRulePath" { catch { if (-not $testingLibraryUsage) - { - $Error[0].FullyQualifiedErrorId | should match "PathNotFound,Microsoft.Windows.PowerShell.ScriptAnalyzer.Commands.InvokeScriptAnalyzerCommand" - } + { + $Error[0].FullyQualifiedErrorId | should match "PathNotFound,Microsoft.Windows.PowerShell.ScriptAnalyzer.Commands.InvokeScriptAnalyzerCommand" + } } } } diff --git a/Tests/Engine/ModuleDependencyHandler.tests.ps1 b/Tests/Engine/ModuleDependencyHandler.tests.ps1 index be1ebfc25..225fe8082 100644 --- a/Tests/Engine/ModuleDependencyHandler.tests.ps1 +++ b/Tests/Engine/ModuleDependencyHandler.tests.ps1 @@ -105,7 +105,7 @@ Describe "Resolve DSC Resource Dependency" { } } - Context "Invoke-ScriptAnalyzer with switch" { + Context "Invoke-ScriptAnalyzer without switch but with module in temp path" { $oldEnvVars = Get-Item Env:\* | Sort-Object -Property Key $moduleName = "MyDscResource" $modulePath = Join-Path (Join-Path (Join-Path (Split-Path $directory) "Rules") "DSCResources") $moduleName From 2458eaea02bc9b340a41cd0da7c0c6364dfc2497 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Tue, 17 May 2016 10:24:28 -0700 Subject: [PATCH 23/23] Fix failing tests --- Tests/Engine/InvokeScriptAnalyzer.tests.ps1 | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 index 780f0e571..bbdf62dda 100644 --- a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 +++ b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 @@ -69,13 +69,16 @@ Describe "Test available parameters" { } } - Context "SaveDSCResourceDependency parameter" { - It "has the parameter" { - $params.ContainsKey("SaveDscResourceDependency") | Should Be $true - } + if (!$testingLibraryUsage) + { + Context "SaveDSCResourceDependency parameter" { + It "has the parameter" { + $params.ContainsKey("SaveDscResourceDependency") | Should Be $true + } - It "is a switch parameter" { - $params["SaveDscResourceDependency"].ParameterType.FullName | Should Be "System.Management.Automation.SwitchParameter" + It "is a switch parameter" { + $params["SaveDscResourceDependency"].ParameterType.FullName | Should Be "System.Management.Automation.SwitchParameter" + } } }