Azure / autorest.powershell

AutoRest PowerShell Generator
MIT License
112 stars 81 forks source link

Set the ConfirmImpact of New-Az*Object and Get-* to Low rather than using default value #1226

Closed BethanyZhou closed 1 year ago

BethanyZhou commented 1 year ago

Our static analyzer always reports New-Az*Object has signature issue and developer has to suppress them by configuring our tools.

To save engineer effort, please exempt New-Az*Object from "should implement ShouldProcess" issue.

"Az.NetworkCloud","New-AzNetworkCloudEndpointDependencyObject","New-AzNetworkCloudEndpointDependencyObject","1","8100","New-AzNetworkCloudEndpointDependencyObject Does not support ShouldProcess but the cmdlet verb New indicates that it should.","Determine if the cmdlet should implement ShouldProcess and if so determine if it should implement Force / ShouldContinue"

For Get-* cmdlet, its ConfirmImpact is empty. PowerShell will use "High" as its ConfirmImpact by default if not specify. And our static analyzer reports "the cmdlet should implement ShouldProcess" warning.

"Az.NetworkCloud","Get-AzNetworkCloudAgentPool","Get-AzNetworkCloudAgentPool","2","8010","Get-AzNetworkCloudAgentPool Changes the ConfirmImpact but does not set the SupportsShouldProcess property to true in the cmdlet attribute.","Determine if the cmdlet should implement ShouldProcess and if so determine if it should implement Force / ShouldContinue"
dolauli commented 1 year ago

New-AzObject is a client side cmdlet to create an object. So both new-Azobject and Get-* will not change any thing in service side, so adding shouldprocess with even confirmImpact set to low does not make sense here. I think we should fix the analyzer instead.

BethanyZhou commented 1 year ago

Hi @dolauli , thanks for your opinions. But if we don't set confirmImpact for them, powershell will regard them as high impact in fact, which may cause harm.

BethanyZhou commented 1 year ago

Close as add task to improve static analyzer for these two kinds of cmdlets.