Switch parameter standardization#26150
Conversation
Marked SwitchParameter.IsPresent and SwitchParameter.ToBool() as [Obsolete] and [Browsable(false)]. Added a property IsPresent for the same purpose that is more semantically-accurate.
…d instead, standardizing the codebase.
logiclrd
requested review from
TravisEz13,
adityapatwardhan and
daxian-dbw
as code owners
|
The changes to public APIs require an RFC. I suggest we change the codebase to the implicit conversion internally: https://github.com/xtqqczze/PowerShell-PowerShell/tree/rm-SwitchParameterIsPresent |
|
Shucks, I thought I might get a Hacktoberfest contribution out of this 😛 |
|
This new parameter doesn't look any more convenient/clear than the old one. Considering that SwitchParameter has been around for many years, I would prefer to simply improve its description in the documentation. Perhaps ScriptAnalyzer can also help too. |
|
I would agree, good to standardise the internal codebase onto either |
|
What about marking |
|
The thought process I'm basing that on is the fact that in the context of PowerShell code, the implicit conversion is already documented in a way that implies it is the preferred way to interact with |
|
I think relying on the implicit conversion is fine but would personally rather just 1 official method or property to get the bool value. While the obsolete attribute might help from a binary module implementation to try and unify things on the PowerShell side would require a rule in PSScriptAnalyzer and even then that can only do so much due to the dynamic nature of PowerShell. |
There is discussion about the right way to implement deprecation in #11674. |
I have a PR to use the implicit conversion internally: #26152 |
|
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days. |
microsoft-github-policy-service
bot
added
the
Review - Needed
4 participants
PR Summary
Replace SwitchParameter.IsPresent and SwitchParameter.ToBool() with SwitchParameter.IsSpecified and standardize the codebase to it.
PR Context
In discussion on #26140, it has come up that there are in fact three ways to do the identical conversion of a
SwitchParameterto aboolvalue that indicates whether it is specified in the invocation:SwitchParametercan be implicitly converted tobool. This is documented as the recommended way to process switches from PowerShell code: TheSwitchParametertype implicitly converts to Boolean.IsPresent, which is used in 135 places across the current codebase..ToBool(), which is used in 11 places across the current codebase.The C# codebase does not use the implicit conversion anywhere.
The
.ToBool()method is descriptive only of the what but not the why; where it is used, it's not describing the meaning of the bool value.The
.IsPresentproperty is poorly-named, because it suggests that it is return a value indicating whether the switch is, in fact, present when a switch can be present but explicitly $false, in which case.IsPresent == false:It has been deemed a bug that these two commands return identical output, and PR #26140 fixes this. There is a similar issue with
Get-Uptime -Since, fixed by PR #26141.This PR has two aims:
.IsPresentand.ToBool()with a more semantically-accurate property.IsSpecified. (The old ones cannot be deleted, per se, since they're part of the public API, but they can be marked[Obsolete].)SwitchParameter.I searched for existing documentation of
.IsPresentand.ToBool(), and as far as I can tell, they are only explicitly documented in the XmlDoc for their members in the source code, and.IsSpecifiedis presented here with equivalent XmlDoc documentation. The implicit convention for the use of the new property.IsSpecifiedis internal to the PowerShell codebase. I have thus checkedNot Applicablefor the documentation section of the PR checklist. If this is in error, let me know and I'll amend as appropriate. 🙂Closes: #26149
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright header