diff --git a/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs b/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs index 4406b3058..b28f24bde 100644 --- a/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs +++ b/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs @@ -375,10 +375,11 @@ namespace Microsoft.PowerShell.Commands var moduleSpecTable = new Dictionary(StringComparer.OrdinalIgnoreCase); if (FullyQualifiedName != null) { - // TODO: - // FullyQualifiedName.Name could be a path, in which case it will not match module.Name. - // This is potentially a bug (since version checks are ignored). - // We should normalize FullyQualifiedName.Name here with ModuleIntrinsics.NormalizeModuleName(). + for (int modSpecIndex = 0; modSpecIndex < FullyQualifiedName.Length; modSpecIndex++) + { + FullyQualifiedName[modSpecIndex] = FullyQualifiedName[modSpecIndex].WithNormalizedName(Context, SessionState.Path.CurrentLocation.Path); + } + moduleSpecTable = FullyQualifiedName.ToDictionary(moduleSpecification => moduleSpecification.Name, StringComparer.OrdinalIgnoreCase); strNames.AddRange(FullyQualifiedName.Select(spec => spec.Name)); } @@ -545,22 +546,36 @@ namespace Microsoft.PowerShell.Commands foreach (PSModuleInfo module in modules) { - // TODO: - // moduleSpecification.Name may be a path and will not match module.Name when they refer to the same module. - // This actually causes the module to be returned always, so other specification checks are skipped erroneously. - // Instead we need to be able to look up or match modules by path as well (e.g. a new comparer for PSModuleInfo). - - // No table entry means we return the module - if (!moduleSpecificationTable.TryGetValue(module.Name, out ModuleSpecification moduleSpecification)) - { - yield return module; - continue; - } + IEnumerable candidateModuleSpecs = GetCandidateModuleSpecs(moduleSpecificationTable, module); // Modules with table entries only get returned if they match them - if (ModuleIntrinsics.IsModuleMatchingModuleSpec(module, moduleSpecification)) + // We skip the name check since modules have already been prefiltered base on the moduleSpec path/name + foreach (ModuleSpecification moduleSpec in candidateModuleSpecs) { - yield return module; + if (ModuleIntrinsics.IsModuleMatchingModuleSpec(module, moduleSpec, skipNameCheck: true)) + { + yield return module; + } + } + } + } + + /// + /// Take a dictionary of module specifications and return those that potentially match the module + /// passed in as a parameter (checks on names and paths). + /// + /// The module specifications to filter candidates from. + /// The module to find candidates for from the module specification table. + /// The module specifications matching the module based on name, path and subpath. + private static IEnumerable GetCandidateModuleSpecs( + IDictionary moduleSpecTable, + PSModuleInfo module) + { + foreach (ModuleSpecification moduleSpec in moduleSpecTable.Values) + { + if (moduleSpec.Name == module.Name || moduleSpec.Name == module.Path || module.Path.Contains(moduleSpec.Name)) + { + yield return moduleSpec; } } } diff --git a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs index a36b69053..8be74965c 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -423,10 +423,14 @@ namespace System.Management.Automation /// /// The module info object to check. /// The module specification to match the module info object against. + /// True if we should skip the name check on the module specification. /// True if the module info object meets all the constraints on the module specification, false otherwise. - internal static bool IsModuleMatchingModuleSpec(PSModuleInfo moduleInfo, ModuleSpecification moduleSpec) + internal static bool IsModuleMatchingModuleSpec( + PSModuleInfo moduleInfo, + ModuleSpecification moduleSpec, + bool skipNameCheck = false) { - return IsModuleMatchingModuleSpec(out ModuleMatchFailure matchFailureReason, moduleInfo, moduleSpec); + return IsModuleMatchingModuleSpec(out ModuleMatchFailure matchFailureReason, moduleInfo, moduleSpec, skipNameCheck); } /// @@ -435,8 +439,13 @@ namespace System.Management.Automation /// The constraint that caused the match failure, if any. /// The module info object to check. /// The module specification to match the module info object against. + /// True if we should skip the name check on the module specification. /// True if the module info object meets all the constraints on the module specification, false otherwise. - internal static bool IsModuleMatchingModuleSpec(out ModuleMatchFailure matchFailureReason, PSModuleInfo moduleInfo, ModuleSpecification moduleSpec) + internal static bool IsModuleMatchingModuleSpec( + out ModuleMatchFailure matchFailureReason, + PSModuleInfo moduleInfo, + ModuleSpecification moduleSpec, + bool skipNameCheck = false) { if (moduleSpec == null) { @@ -447,7 +456,7 @@ namespace System.Management.Automation return IsModuleMatchingConstraints( out matchFailureReason, moduleInfo, - moduleSpec.Name, + skipNameCheck ? null : moduleSpec.Name, moduleSpec.Guid, moduleSpec.RequiredVersion, moduleSpec.Version, diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/Get-Module.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/Get-Module.Tests.ps1 index bbff5b8d9..8d8516437 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/Get-Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/Get-Module.Tests.ps1 @@ -27,8 +27,7 @@ Describe "Get-Module -ListAvailable" -Tags "CI" { New-Item -ItemType File -Path "$testdrive\Modules\Az\Az.psm1" > $null $fullyQualifiedPathTestCases = @( - # The current behaviour in PowerShell is that version gets ignored when using Get-Module -FullyQualifiedName with a path - @{ ModPath = "$TestDrive/Modules\Foo"; Name = 'Foo'; Version = '2.0'; Count = 2 } + @{ ModPath = "$TestDrive/Modules\Foo"; Name = 'Foo'; Version = '2.0'; Count = 1 } @{ ModPath = "$TestDrive\Modules/Foo\1.1/Foo.psd1"; Name = 'Foo'; Version = '1.1'; Count = 1 } @{ ModPath = "$TestDrive\Modules/Bar.psd1"; Name = 'Bar'; Version = '0.0'; Count = 1 } @{ ModPath = "$TestDrive\Modules\Zoo\Too\Zoo.psm1"; Name = 'Zoo'; Version = '0.0'; Count = 1 } @@ -225,3 +224,101 @@ Describe "Get-Module -ListAvailable" -Tags "CI" { } } } + +Describe 'Get-Module -ListAvailable with path' -Tags "CI" { + BeforeAll { + $moduleName = 'Banana' + $modulePath = Join-Path $TestDrive $moduleName + $v1 = '1.2.3' + $v2 = '4.8.3' + $v1DirPath = Join-Path $modulePath $v1 + $v2DirPath = Join-Path $modulePath $v2 + $manifestV1Path = Join-Path $v1DirPath "$moduleName.psd1" + $manifestV2Path = Join-Path $v2DirPath "$moduleName.psd1" + + New-Item -ItemType Directory $modulePath + New-Item -ItemType Directory -Path $v1DirPath + New-Item -ItemType Directory -Path $v2DirPath + New-ModuleManifest -Path $manifestV1Path -ModuleVersion $v1 + New-ModuleManifest -Path $manifestV2Path -ModuleVersion $v2 + } + + It "Gets all versions by path" { + $modules = Get-Module -ListAvailable $modulePath | Sort-Object -Property Version + + $modules | Should -HaveCount 2 + $modules[0].Name | Should -BeExactly $moduleName + $modules[0].Path | Should -BeExactly $manifestV1Path + $modules[0].Version | Should -Be $v1 + $modules[1].Name | Should -BeExactly $moduleName + $modules[1].Path | Should -BeExactly $manifestV2Path + $modules[1].Version | Should -Be $v2 + } + + It "Gets all versions by FullyQualifiedName with path with lower version" { + $modules = Get-Module -ListAvailable -FullyQualifiedName @{ ModuleName = $modulePath; ModuleVersion = '0.0' } | Sort-Object -Property Version + + $modules | Should -HaveCount 2 + $modules[0].Name | Should -BeExactly $moduleName + $modules[0].Path | Should -BeExactly $manifestV1Path + $modules[0].Version | Should -Be $v1 + $modules[1].Name | Should -BeExactly $moduleName + $modules[1].Path | Should -BeExactly $manifestV2Path + $modules[1].Version | Should -Be $v2 + } + + It "Gets high version by FullyQualifiedName with path with high version" { + $modules = Get-Module -ListAvailable -FullyQualifiedName @{ ModuleName = $modulePath; ModuleVersion = '2.0' } | Sort-Object -Property Version + + $modules | Should -HaveCount 1 + $modules[0].Name | Should -BeExactly $moduleName + $modules[0].Path | Should -BeExactly $manifestV2Path + $modules[0].Version | Should -Be $v2 + } + + It "Gets low version by FullyQualifiedName with path with low maximum version" { + $modules = Get-Module -ListAvailable -FullyQualifiedName @{ ModuleName = $modulePath; MaximumVersion = '2.0' } | Sort-Object -Property Version + + $modules | Should -HaveCount 1 + $modules[0].Name | Should -BeExactly $moduleName + $modules[0].Path | Should -BeExactly $manifestV1Path + $modules[0].Version | Should -Be $v1 + } + + It "Gets low version by FullyQualifiedName with path with low maximum version and version" { + $modules = Get-Module -ListAvailable -FullyQualifiedName @{ ModuleName = $modulePath; MaximumVersion = '2.0'; ModuleVersion = '1.0' } | Sort-Object -Property Version + + $modules | Should -HaveCount 1 + $modules[0].Name | Should -BeExactly $moduleName + $modules[0].Path | Should -BeExactly $manifestV1Path + $modules[0].Version | Should -Be $v1 + } + + It "Gets correct version by FullyQualifiedName with path with required version" -TestCases @( + @{ Version = $v1 } + @{ Version = $v2 } + ) { + param([version]$Version) + + switch ($Version) + { + $v1 + { + $expectedPath = $manifestV1Path + break + } + + $v2 + { + $expectedPath = $manifestV2Path + } + } + + $modules = Get-Module -ListAvailable -FullyQualifiedName @{ ModuleName = $modulePath; RequiredVersion = $Version } + + $modules | Should -HaveCount 1 + $modules[0].Name | Should -BeExactly $moduleName + $modules[0].Path | Should -BeExactly $expectedPath + $modules[0].Version | Should -Be $Version + } +}