You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Feature Request: Warnings about breaking changes #9138
We could use (1) warning stream or (2) trace to write these warnings. It could be done in cmdlet Beginning() methods.
We will need to have a breaking change database and update it every new release.
We could have a global config option to set a PowerShell version to compare with.
I wonder if this should be done as part of PSScriptAnalyzer Compatibility rules. We would still need to keep track of the changes (I believe we've been pretty good at using the Breaking Change label.
we've been pretty good at using the Breaking Change label.
Yes, it is very helpful. Only we have some breaking changes before we introduce the label. :-(
I wonder if this should be done as part of PSScriptAnalyzer Compatibility rules.
It seems we have a tracking issue there for the feature (without progress).
I think it is different things. Both is useful. Injecting in engine could expose more in-depth information. Some things can also be caught only during script execution (at runtime) (and others only in PSScriptAnalyzer).
I suggest to write a (custom) rule for every upgrade (i.e. 6.1->6.2, 6.2->6.3, etc.). The diagnostic record can not only tell about the breaking change but it can also offer a correction, thereby enabling an auto-fix scenario (I added a -Fix switch to PSSA for that). When people upgrade their PowerShell version, then they can run the rule to check for compatibility and/or upgrade their script. To keep things simple, people should upgrade only one minor version at a time or apply the custom rule in this order.
I think the runtime overhead of checking for breaking change usages on the fly in PowerShell itself is probably not justifiable. By itself it's not huge, but as the expectations of what we do at runtime pile up we get bogged down with all the "magic" PowerShell does. And after you've seen the warning, you probably want to suppress it without suppressing other warnings. So we'd then need a new API (like a preference variable) to govern that.
We could theoretically do something like add a BreakingChangeAttribute to all the APIs that get changed, but I think that would get out of hand quickly and if we changed our minds we'd be left with them strewn throughout the codebase just like all the Microsoft linter exceptions currently in there. Plus a number of breaking changes wouldn't be easily isolated to a single method call or cmdlet.
So I think PSScriptAnalyzer is the right place for it -- we already have a lot of infrastructure for this there.
Using existing PSScriptAnalyzer rules
The compatibility rule work should already pick up most cmdlet changes automatically - we just need to generate a profile for each version. And it wouldn't be hard to invest in a little bit more work there over time to pick up other things.
The PSUseCompatibleSyntax rule can be updated by hand fairly easily for other things like operators that have been introduced later.
Getting PSScriptAnalyzer to warn about purely semantic breaking changes
What would remain is calls/uses that look the same but act differently -- semantic rather than syntactic changes. For example, if we were to allow negative indices for splitting, that isn't always statically checkable (you can use a variable there), but no matter what approach we take, we can't capture all breaking changes from an analysis standpoint. The simple solution is to just report on any syntax that could be using a semantically breaking change.
Then with each release, we would just need an AstVisitor that finds each syntax that might represent a breaking change and emits an informational diagnostic record saying "the way this worked changed in the versions you're targeting" and possibly a suggested fix if appropriate.
Tracking the breaking changes themselves would be nice to automate in general, but without significant investment I don't think that can be done. Instead we need an architecture like:
A single rule (PSAvoidBreakingChanges or something)
Per-version-diff AstVisitor class for breaking changes (e.g. IBreakingChangeCheckVisitor)
Code written by hand for each release
Logic to compose visitors for multi-version differences (6.0 -> 6.2 => 6.0 -> 6.1 -> 6.2)
This is still a lot of work, but avoids the need for some breakable serialisation format to describe:
What syntax has changed in meaning due to breaking changes
What the old and new meaning is
How to correct it
As much as I like declarative formats, imperative is easiest there.
Conclusion
Anyway, I don't know how feasible that is, who's signing up to do that work or how high priority we think it is, but that's how I would approach it:
PSUseCompatibleCommands for cmdlet availability/parameter/input set changes
PSUseCompatibleSyntax for changes in the language like operators, using statements, interfaces
A new rule like PSAvoidBreakingChanges for semantic changes, with a hand-written AstVisitor each release
My question is more about what do we can add in PowerShell repo to help users and PSSA?
Nothing in the PowerShell repo. PowerShell has evolved to become a full ecosystem, and we've made the investment in PSSA. The contribution to be made lies in PSSA.
With current workflow we create new issues in docs repo is needed. It is great for keeping docs being up to date.
For PSSA we have nothing in our workflow. And I still don't understand how we could keep PSSA rules being up to date with the "nothing in the PowerShell repo". So my thoughts is that we could add something like doc tags, attributes and so on.
Well we already have the breaking change tag. It feels like the next step is to say "when we do a release, use the breaking change tags to write a new visitor for PSSA".
I think we're essentially in agreement -- my proposal is that there's just a new workflow for adding to a checker in PSSA when you make a new breaking change, same as for documenting things. The only real changes I see there for the PowerShell repo though are adding a checkbox in PRs maybe and writing up the new workflow in a doc (since it's a bit complicated)
We could add new check box "write a new breaking change visitor for PSSA" in our release template.
If we decide that's the way to go, we should first do the work to create the rule and make creating a visitor a straightforward, mechanical task. Creating the base stuff in PSSA shouldn't be too difficult but requires some investment.
Activity
SteveL-MSFT commentedon Mar 14, 2019
I wonder if this should be done as part of PSScriptAnalyzer Compatibility rules. We would still need to keep track of the changes (I believe we've been pretty good at using the
Breaking Changelabel.iSazonov commentedon Mar 14, 2019
Yes, it is very helpful. Only we have some breaking changes before we introduce the label. :-(
It seems we have a tracking issue there for the feature (without progress).
I think it is different things. Both is useful. Injecting in engine could expose more in-depth information. Some things can also be caught only during script execution (at runtime) (and others only in PSScriptAnalyzer).
iSazonov commentedon Mar 14, 2019
We could place the database in external module and inject in PowerShell hooks to use the module if it is loaded.
iSazonov commentedon Mar 19, 2019
Christoph Bergmeister (@bergmeister) Rob Holt (@rjmholt) Have you thoughts how can we make friends with PSSA?
bergmeister commentedon Mar 19, 2019
I suggest to write a (custom) rule for every upgrade (i.e. 6.1->6.2, 6.2->6.3, etc.). The diagnostic record can not only tell about the breaking change but it can also offer a correction, thereby enabling an auto-fix scenario (I added a
-Fixswitch to PSSA for that). When people upgrade their PowerShell version, then they can run the rule to check for compatibility and/or upgrade their script. To keep things simple, people should upgrade only one minor version at a time or apply the custom rule in this order.iSazonov commentedon Mar 19, 2019
My question is more about what do we can add in PowerShell repo to help users and PSSA?
rjmholt commentedon Mar 26, 2019
Warning at runtime in PowerShell
I think the runtime overhead of checking for breaking change usages on the fly in PowerShell itself is probably not justifiable. By itself it's not huge, but as the expectations of what we do at runtime pile up we get bogged down with all the "magic" PowerShell does. And after you've seen the warning, you probably want to suppress it without suppressing other warnings. So we'd then need a new API (like a preference variable) to govern that.
We could theoretically do something like add a
BreakingChangeAttributeto all the APIs that get changed, but I think that would get out of hand quickly and if we changed our minds we'd be left with them strewn throughout the codebase just like all the Microsoft linter exceptions currently in there. Plus a number of breaking changes wouldn't be easily isolated to a single method call or cmdlet.So I think PSScriptAnalyzer is the right place for it -- we already have a lot of infrastructure for this there.
Using existing PSScriptAnalyzer rules
The compatibility rule work should already pick up most cmdlet changes automatically - we just need to generate a profile for each version. And it wouldn't be hard to invest in a little bit more work there over time to pick up other things.
The
PSUseCompatibleSyntaxrule can be updated by hand fairly easily for other things like operators that have been introduced later.Getting PSScriptAnalyzer to warn about purely semantic breaking changes
What would remain is calls/uses that look the same but act differently -- semantic rather than syntactic changes. For example, if we were to allow negative indices for splitting, that isn't always statically checkable (you can use a variable there), but no matter what approach we take, we can't capture all breaking changes from an analysis standpoint. The simple solution is to just report on any syntax that could be using a semantically breaking change.
Then with each release, we would just need an AstVisitor that finds each syntax that might represent a breaking change and emits an informational diagnostic record saying "the way this worked changed in the versions you're targeting" and possibly a suggested fix if appropriate.
Tracking the breaking changes themselves would be nice to automate in general, but without significant investment I don't think that can be done. Instead we need an architecture like:
PSAvoidBreakingChangesor something)IBreakingChangeCheckVisitor)This is still a lot of work, but avoids the need for some breakable serialisation format to describe:
As much as I like declarative formats, imperative is easiest there.
Conclusion
Anyway, I don't know how feasible that is, who's signing up to do that work or how high priority we think it is, but that's how I would approach it:
PSUseCompatibleCommandsfor cmdlet availability/parameter/input set changesPSUseCompatibleSyntaxfor changes in the language like operators,usingstatements, interfacesPSAvoidBreakingChangesfor semantic changes, with a hand-written AstVisitor each releaseNothing in the PowerShell repo. PowerShell has evolved to become a full ecosystem, and we've made the investment in PSSA. The contribution to be made lies in PSSA.
iSazonov commentedon Mar 26, 2019
With current workflow we create new issues in docs repo is needed. It is great for keeping docs being up to date.
For PSSA we have nothing in our workflow. And I still don't understand how we could keep PSSA rules being up to date with the "nothing in the PowerShell repo". So my thoughts is that we could add something like doc tags, attributes and so on.
rjmholt commentedon Mar 26, 2019
Well we already have the breaking change tag. It feels like the next step is to say "when we do a release, use the breaking change tags to write a new visitor for PSSA".
I think we're essentially in agreement -- my proposal is that there's just a new workflow for adding to a checker in PSSA when you make a new breaking change, same as for documenting things. The only real changes I see there for the PowerShell repo though are adding a checkbox in PRs maybe and writing up the new workflow in a doc (since it's a bit complicated)
iSazonov commentedon Mar 26, 2019
Rob Holt (@rjmholt) Thanks for great feedback!
Steve Lee (@SteveL-MSFT) We could add new check box "write a new breaking change visitor for PSSA" in our release template.
rjmholt commentedon Mar 26, 2019
If we decide that's the way to go, we should first do the work to create the rule and make creating a visitor a straightforward, mechanical task. Creating the base stuff in PSSA shouldn't be too difficult but requires some investment.