Skip to content

Feature Request: Warnings about breaking changes #9138

Closed

Description

@iSazonov

Summary of the new feature/enhancement

In critical environment before update to new PowerShell Core version consumers should test their scripts.

Warnings about breaking changes would help them to locate places where errors is possible.

Proposed technical implementation details (optional)

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.

Activity

  1. SteveL-MSFT commented on Mar 14, 2019

    @SteveL-MSFT
    Member

    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.

  2. iSazonov commented on Mar 14, 2019

    @iSazonov
    CollaboratorAuthor

    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).

  3. iSazonov commented on Mar 14, 2019

    @iSazonov
    CollaboratorAuthor

    We could place the database in external module and inject in PowerShell hooks to use the module if it is loaded.

  4. iSazonov commented on Mar 19, 2019

    @iSazonov
    CollaboratorAuthor

    Christoph Bergmeister (@bergmeister) Rob Holt (@rjmholt) Have you thoughts how can we make friends with PSSA?

  5. bergmeister commented on Mar 19, 2019

    @bergmeister
    Contributor

    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.

  6. iSazonov commented on Mar 19, 2019

    @iSazonov
    CollaboratorAuthor

    My question is more about what do we can add in PowerShell repo to help users and PSSA?

  7. rjmholt commented on Mar 26, 2019

    @rjmholt
    Collaborator

    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 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.

  8. iSazonov commented on Mar 26, 2019

    @iSazonov
    CollaboratorAuthor

    Nothing in the PowerShell repo.

    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.

  9. rjmholt commented on Mar 26, 2019

    @rjmholt
    Collaborator

    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)

  10. iSazonov commented on Mar 26, 2019

    @iSazonov
    CollaboratorAuthor

    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.

  11. rjmholt commented on Mar 26, 2019

    @rjmholt
    Collaborator

    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Issue-Enhancementthe issue is more of a feature request than a bugResolution-AnsweredThe question is answered.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions