Skip to content

Transition ActionPreference.Suspend enumeration value into a non-supported, reserved state, and remove restriction on using ActionPreference.Ignore in preference variables #10317

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
iSazonov merged 11 commits into from
Sep 7, 2019
Merged

Transition ActionPreference.Suspend enumeration value into a non-supported, reserved state, and remove restriction on using ActionPreference.Ignore in preference variables #10317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
637ecbf
remove deprecated ActionPreference.Suspend value
KirkMunro Aug 7, 2019
cd4b42c
remove unused string
KirkMunro Aug 7, 2019
c53b9eb
Merge branch 'master' into deprecate-actionpreference-suspend
KirkMunro Aug 22, 2019
1d84d26
put enum value back, updated plumbing and tests
KirkMunro Aug 22, 2019
008d97b
CodeFactor changes
KirkMunro Aug 22, 2019
efbb687
change test to avoid failure (#8205 fixes this)
KirkMunro Aug 22, 2019
d9710a2
merge master
KirkMunro Aug 23, 2019
d68b07b
remove global qualifier added during testing
KirkMunro Aug 23, 2019
5eb9658
code review changes
KirkMunro Aug 29, 2019
1962ea8
Merge branch 'master' into deprecate-actionpreference-suspend
KirkMunro Sep 6, 2019
7b0c053
remove ActionPreference.Ignore restriction
KirkMunro Sep 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/System.Management.Automation/engine/ArgumentTypeConverterAttribute.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,15 +147,14 @@ internal object Transform(EngineIntrinsics engineIntrinsics, object inputData, b
// Note - this is duplicated in ExecutionContext.cs as parameter binding for script cmdlets can avoid this code path.
Comment thread
KirkMunro marked this conversation as resolved.
Comment thread
KirkMunro marked this conversation as resolved.
Comment thread
KirkMunro marked this conversation as resolved.
if ((!bindingScriptCmdlet) && (!bindingParameters))
{
// ActionPreference of Suspend is not supported as a preference variable. We can only block "Suspend"
// during variable assignment (here) - "Ignore" is blocked during variable retrieval.
// ActionPreference.Suspend is reserved for future use and is not supported as a preference variable.
if (_convertTypes[i] == typeof(ActionPreference))
{
ActionPreference resultPreference = (ActionPreference)result;

if (resultPreference == ActionPreference.Suspend)
{
throw new PSInvalidCastException("InvalidActionPreference", null, ErrorPackage.UnsupportedPreferenceVariable, resultPreference);
throw new PSInvalidCastException("InvalidActionPreference", null, ErrorPackage.ActionPreferenceReservedForFutureUseError, resultPreference);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/System.Management.Automation/engine/CommandBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ public enum ActionPreference
/// <summary>Ignore the event completely (not even logging it to the target stream)</summary>
Ignore = 4,

/// <summary>Suspend the command for further diagnosis. Supported only for workflows.</summary>
/// <summary>Reserved for future use.</summary>
Suspend = 5,

/// <summary>Enter the debugger.</summary>
Expand Down
30 changes: 19 additions & 11 deletions src/System.Management.Automation/engine/ExecutionContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -573,18 +573,9 @@ internal T GetEnumPreference<T>(VariablePath preferenceVariablePath, T defaultPr
object val = EngineSessionState.GetVariableValue(preferenceVariablePath, out _, out _);
if (val is T)
{
// We don't want to support "Ignore" as action preferences, as it leads to bad
// scripting habits. They are only supported as cmdlet overrides.
if (val is ActionPreference)
if (val is ActionPreference actionPreferenceValue)
{
ActionPreference preference = (ActionPreference)val;
if ((preference == ActionPreference.Ignore) || (preference == ActionPreference.Suspend))
{
// Reset the variable value
EngineSessionState.SetVariableValue(preferenceVariablePath.UserPath, defaultPref);
string message = StringUtil.Format(ErrorPackage.UnsupportedPreferenceError, preference);
throw new NotSupportedException(message);
}
CheckActionPreference(preferenceVariablePath, actionPreferenceValue, defaultPref);
}

T convertedResult = (T)val;
Expand All @@ -611,6 +602,11 @@ internal T GetEnumPreference<T>(VariablePath preferenceVariablePath, T defaultPr
result = (T)PSObject.Base(val);
defaultUsed = false;
}

if (result is ActionPreference actionPreferenceValue)
{
CheckActionPreference(preferenceVariablePath, actionPreferenceValue, defaultPref);
}
Comment thread
KirkMunro marked this conversation as resolved.
}
catch (InvalidCastException)
{
Expand All @@ -625,6 +621,18 @@ internal T GetEnumPreference<T>(VariablePath preferenceVariablePath, T defaultPr
return result;
}

private void CheckActionPreference(VariablePath preferenceVariablePath, ActionPreference preference, object defaultValue)
{
Comment thread
KirkMunro marked this conversation as resolved.
if (preference == ActionPreference.Suspend)
{
// ActionPreference.Suspend is reserved for future use. When it is used, reset
// the variable to its default.
string message = StringUtil.Format(ErrorPackage.ReservedActionPreferenceReplacedError, preference, preferenceVariablePath.UserPath, defaultValue);
EngineSessionState.SetVariable(preferenceVariablePath, defaultValue, true, CommandOrigin.Internal);
throw new NotSupportedException(message);
}
}

/// <summary>
/// Same as GetEnumPreference, but for boolean values.
/// </summary>
Expand Down
16 changes: 13 additions & 3 deletions src/System.Management.Automation/engine/MshCommandRuntime.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3009,6 +3009,11 @@ internal ActionPreference DebugPreference

set
{
if (value == ActionPreference.Suspend)
{
throw PSTraceSource.NewNotSupportedException(ErrorPackage.ActionPreferenceReservedForFutureUseError, value);
}

_debugPreference = value;
_isDebugPreferenceSet = true;
}
Expand Down Expand Up @@ -3098,7 +3103,7 @@ internal ActionPreference WarningPreference
{
if (value == ActionPreference.Suspend)
{
throw PSTraceSource.NewNotSupportedException(ErrorPackage.SuspendActionPreferenceErrorActionOnly);
throw PSTraceSource.NewNotSupportedException(ErrorPackage.ActionPreferenceReservedForFutureUseError, value);
}

_warningPreference = value;
Expand Down Expand Up @@ -3268,7 +3273,7 @@ internal ActionPreference ErrorAction
{
if (value == ActionPreference.Suspend)
{
throw PSTraceSource.NewNotSupportedException(ErrorPackage.SuspendActionPreferenceSupportedOnlyOnWorkflow);
throw PSTraceSource.NewNotSupportedException(ErrorPackage.ActionPreferenceReservedForFutureUseError, value);
}

_errorAction = value;
Expand Down Expand Up @@ -3301,6 +3306,11 @@ internal ActionPreference ProgressPreference

set
{
if (value == ActionPreference.Suspend)
{
throw PSTraceSource.NewNotSupportedException(ErrorPackage.ActionPreferenceReservedForFutureUseError, value);
}

_progressPreference = value;
_isProgressPreferenceSet = true;
}
Expand Down Expand Up @@ -3335,7 +3345,7 @@ internal ActionPreference InformationPreference
{
if (value == ActionPreference.Suspend)
{
throw PSTraceSource.NewNotSupportedException(ErrorPackage.SuspendActionPreferenceErrorActionOnly);
throw PSTraceSource.NewNotSupportedException(ErrorPackage.ActionPreferenceReservedForFutureUseError, value);
}

_informationPreference = value;
Expand Down
15 changes: 5 additions & 10 deletions src/System.Management.Automation/resources/ErrorPackage.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,18 +126,13 @@
<data name="RedirectedException" xml:space="preserve">
<value>Object "{0}" is reported as an error.</value>
</data>
<data name="SuspendActionPreferenceErrorActionOnly" xml:space="preserve">
<value>The action preference of "Suspend" is supported only for ErrorAction.</value>
<comment>"Suspend" and ErrorAction should not be localized</comment>
</data>
<data name="SuspendActionPreferenceSupportedOnlyOnWorkflow" xml:space="preserve">
<value>The error action preference of "Suspend" is supported only on workflows.</value>
<comment>"Suspend" should not be localized - it is a literal.</comment>
</data>
<data name="UnsupportedPreferenceError" xml:space="preserve">
<value>The value {0} is not supported for an ActionPreference variable. The provided value should be used only as a value for a preference parameter, and has been replaced by the default value. For more information, see the Help topic, "about_Preference_Variables."</value>
</data>
<data name="UnsupportedPreferenceVariable" xml:space="preserve">
<value>The value {0} is not supported for an ActionPreference variable. The provided value should be used only as a value for a preference parameter. For more information, see the Help topic, "about_Preference_Variables."</value>
<data name="ActionPreferenceReservedForFutureUseError" xml:space="preserve">
<value>The {0} ActionPreference value is reserved for future use and is not supported at this time. For more information about preference variables, see the Help topic, "about_Preference_Variables."</value>
</data>
<data name="ReservedActionPreferenceReplacedError" xml:space="preserve">
<value>The {0} ActionPreference value is reserved for future use and is not supported at this time. It has been replaced in your {1} variable by the default value of {2}. For more information about preference variables, see the Help topic, "about_Preference_Variables."</value>
</data>
</root>
Loading