From 0ac878f72cd50b2df02c023fcdbd718ea198eb26 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Tue, 10 Jul 2018 12:46:48 -0700 Subject: [PATCH] [Feature] Address Feedback: Fix style and xml doc issues. Update tests --- .../engine/Attributes.cs | 6 +-- .../ExperimentalFeature.cs | 20 ++++++---- .../GetExperimentalFeatureCommand.cs | 4 +- .../engine/Modules/ModuleCmdletBase.cs | 8 ++-- .../engine/parser/ast.cs | 5 ++- .../ExperimentalFeature.Basic.Tests.ps1 | 40 ++++++++++++++----- 6 files changed, 54 insertions(+), 29 deletions(-) diff --git a/src/System.Management.Automation/engine/Attributes.cs b/src/System.Management.Automation/engine/Attributes.cs index 75035dd4b9..309bee7a9b 100644 --- a/src/System.Management.Automation/engine/Attributes.cs +++ b/src/System.Management.Automation/engine/Attributes.cs @@ -639,12 +639,12 @@ namespace System.Management.Automation #region Experimental Feature Related Properties /// - /// Name of the experimental feature this attribute is associated with. + /// Get name of the experimental feature this attribute is associated with. /// public string ExperimentName { get; } /// - /// Action for engine to take when the experimental feature is enabled. + /// Get action for engine to take when the experimental feature is enabled. /// public ExperimentAction ExperimentAction { get; } @@ -652,7 +652,7 @@ namespace System.Management.Automation internal bool ToShow => EffectiveAction == ExperimentAction.Show; /// - /// Effective action to take at run time. + /// Get effective action to take at run time. /// private ExperimentAction EffectiveAction { diff --git a/src/System.Management.Automation/engine/ExperimentalFeature/ExperimentalFeature.cs b/src/System.Management.Automation/engine/ExperimentalFeature/ExperimentalFeature.cs index 8af881b3e0..ef2f6a7bd4 100644 --- a/src/System.Management.Automation/engine/ExperimentalFeature/ExperimentalFeature.cs +++ b/src/System.Management.Automation/engine/ExperimentalFeature/ExperimentalFeature.cs @@ -13,7 +13,7 @@ using System.Runtime.CompilerServices; namespace System.Management.Automation { /// - /// Support experimental features in PowerShell + /// Support experimental features in PowerShell. /// public class ExperimentalFeature { @@ -103,7 +103,11 @@ namespace System.Management.Automation // instead, it will be done when the type is used for the first time, which is always earlier than // any experimental features take effect. string[] enabledFeatures = Utils.EmptyArray(); - try { enabledFeatures = PowerShellConfig.Instance.GetExperimentalFeatures(); } catch (Exception e) when (LogException(e)) { } + try + { + enabledFeatures = PowerShellConfig.Instance.GetExperimentalFeatures(); + } + catch (Exception e) when (LogException(e)) { } EnabledExperimentalFeatureNames = ProcessEnabledFeatures(enabledFeatures); } @@ -165,7 +169,7 @@ namespace System.Management.Automation /// /// Check if the name follows the engine experimental feature name convention. - /// Convention: prefix 'PS' to the feature name -- PSFeatureName + /// Convention: prefix 'PS' to the feature name -- 'PSFeatureName'. /// internal static bool IsEngineFeatureName(string featureName) { @@ -174,10 +178,10 @@ namespace System.Management.Automation /// /// Check if the name follows the module experimental feature name convention. - /// Convention: ModuleName.FeatureName + /// Convention: prefix the module name to the feature name -- 'ModuleName.FeatureName'. /// /// The feature name to check. - /// When specified, we check if the feature name matches the module name + /// When specified, we check if the feature name matches the module name. internal static bool IsModuleFeatureName(string featureName, string moduleName = null) { // Feature names cannot start with a dot @@ -263,12 +267,12 @@ namespace System.Management.Automation public sealed class ExperimentalAttribute : ParsingBaseAttribute { /// - /// Specify the experimental feature this attribute is associated with. + /// Get name of the experimental feature this attribute is associated with. /// public string ExperimentName { get; } /// - /// Specify the action for engine to take when the experimental feature is enabled. + /// Get action for engine to take when the experimental feature is enabled. /// public ExperimentAction ExperimentAction { get; } @@ -316,7 +320,7 @@ namespace System.Management.Automation internal bool ToShow => EffectiveAction == ExperimentAction.Show; /// - /// Effective action to take at run time. + /// Get effective action to take at run time. /// private ExperimentAction EffectiveAction { diff --git a/src/System.Management.Automation/engine/ExperimentalFeature/GetExperimentalFeatureCommand.cs b/src/System.Management.Automation/engine/ExperimentalFeature/GetExperimentalFeatureCommand.cs index f526870288..a6cc9e7d7c 100644 --- a/src/System.Management.Automation/engine/ExperimentalFeature/GetExperimentalFeatureCommand.cs +++ b/src/System.Management.Automation/engine/ExperimentalFeature/GetExperimentalFeatureCommand.cs @@ -17,14 +17,14 @@ namespace Microsoft.PowerShell.Commands public class GetExperimentalFeatureCommand : PSCmdlet { /// - /// Specify the feature names. + /// Get and set the feature names. /// [Parameter(ValueFromPipeline = true, Position = 0)] [ValidateNotNullOrEmpty] public string[] Name { get; set; } /// - /// Search module paths to find all available experimental features. + /// Get and set the switch flag to search module paths to find all available experimental features. /// [Parameter] public SwitchParameter ListAvailable { get; set; } diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index 5539285cdf..f7d2fc73c2 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -2071,8 +2071,8 @@ namespace Microsoft.PowerShell.Commands if (writingErrors) { WriteError(new ErrorRecord(new ArgumentException(Modules.ExperimentalFeatureNameMissingOrEmpty), - "Modules_ExperimentalFeatureNameMissingOrEmpty", - ErrorCategory.InvalidData, null)); + "Modules_ExperimentalFeatureNameMissingOrEmpty", + ErrorCategory.InvalidData, null)); } containedErrors = true; @@ -2086,8 +2086,8 @@ namespace Microsoft.PowerShell.Commands string invalidNameStr = String.Join(", ", invalidNames); string errorMsg = StringUtil.Format(Modules.InvalidExperimentalFeatureName, invalidNameStr); WriteError(new ErrorRecord(new ArgumentException(errorMsg), - "Modules_InvalidExperimentalFeatureName", - ErrorCategory.InvalidData, null)); + "Modules_InvalidExperimentalFeatureName", + ErrorCategory.InvalidData, null)); } containedErrors = true; diff --git a/src/System.Management.Automation/engine/parser/ast.cs b/src/System.Management.Automation/engine/parser/ast.cs index 66722983b2..10221af3a8 100644 --- a/src/System.Management.Automation/engine/parser/ast.cs +++ b/src/System.Management.Automation/engine/parser/ast.cs @@ -1419,7 +1419,10 @@ namespace System.Management.Automation.Language { return Compiler.GetAttribute(potentialExpAttr) as ExperimentalAttribute; } - catch (Exception) { /* catch all and assume it's not a declaration of ExperimentalAttribute */ } + catch (Exception) + { + // catch all and assume it's not a declaration of ExperimentalAttribute + } } return null; diff --git a/test/powershell/engine/ExperimentalFeature/ExperimentalFeature.Basic.Tests.ps1 b/test/powershell/engine/ExperimentalFeature/ExperimentalFeature.Basic.Tests.ps1 index cb8869b265..84757e4909 100644 --- a/test/powershell/engine/ExperimentalFeature/ExperimentalFeature.Basic.Tests.ps1 +++ b/test/powershell/engine/ExperimentalFeature/ExperimentalFeature.Basic.Tests.ps1 @@ -381,18 +381,36 @@ Describe "Expected errors" -Tag "CI" { { [Experimental()]param() } | Should -Throw -ErrorId "MethodCountCouldNotFindBest" } - It "Argument validation for constructors of 'ExperimentalAttribute' and 'ParameterAttribute'" { - { [Experimental]::new("", "None") } | Should -Throw -ErrorId "PSArgumentNullException" - { [Experimental]::new([NullString]::Value, "None") } | Should -Throw -ErrorId "PSArgumentNullException" - { [Experimental]::new("feature", "None") } | Should -Throw -ErrorId "PSArgumentException" - { [Experimental]::new("feature", "Show") } | Should -Not -Throw - { [Experimental]::new("feature", "Hide") } | Should -Not -Throw + It "Argument validation for constructors of 'ExperimentalAttribute' - " -TestCases @( + @{ TestName = "Name is empty string"; FeatureName = ""; FeatureAction = "None"; ErrorId = "PSArgumentNullException" } + @{ TestName = "Name is null"; FeatureName = [NullString]::Value; FeatureAction = "None"; ErrorId = "PSArgumentNullException" } + @{ TestName = "Action is None"; FeatureName = "feature"; FeatureAction = "None"; ErrorId = "PSArgumentException" } + @{ TestName = "Action is Show"; FeatureName = "feature"; FeatureAction = "Show"; ErrorId = $null } + @{ TestName = "Action is Hide"; FeatureName = "feature"; FeatureAction = "Hide"; ErrorId = $null } + ) { + param($FeatureName, $FeatureAction, $ErrorId) - { [Parameter]::new("", "None") } | Should -Throw -ErrorId "PSArgumentNullException" - { [Parameter]::new([NullString]::Value, "None") } | Should -Throw -ErrorId "PSArgumentNullException" - { [Parameter]::new("feature", "None") } | Should -Throw -ErrorId "PSArgumentException" - { [Parameter]::new("feature", "Show") } | Should -Not -Throw - { [Parameter]::new("feature", "Hide") } | Should -Not -Throw + if ($ErrorId -ne $null) { + { [Experimental]::new($FeatureName, $FeatureAction) } | Should -Throw -ErrorId $ErrorId + } else { + { [Experimental]::new($FeatureName, $FeatureAction) } | Should -Not -Throw + } + } + + It "Argument validation for constructors of 'ParameterAttribute' - " -TestCases @( + @{ TestName = "Name is empty string"; FeatureName = ""; FeatureAction = "None"; ErrorId = "PSArgumentNullException" } + @{ TestName = "Name is null"; FeatureName = [NullString]::Value; FeatureAction = "None"; ErrorId = "PSArgumentNullException" } + @{ TestName = "Action is None"; FeatureName = "feature"; FeatureAction = "None"; ErrorId = "PSArgumentException" } + @{ TestName = "Action is Show"; FeatureName = "feature"; FeatureAction = "Show"; ErrorId = $null } + @{ TestName = "Action is Hide"; FeatureName = "feature"; FeatureAction = "Hide"; ErrorId = $null } + ) { + param($FeatureName, $FeatureAction, $ErrorId) + + if ($ErrorId -ne $null) { + { [Parameter]::new($FeatureName, $FeatureAction) } | Should -Throw -ErrorId $ErrorId + } else { + { [Parameter]::new($FeatureName, $FeatureAction) } | Should -Not -Throw + } } It "Feature name check" {