Improve variable type inference

[MartinGC94]

PR Summary

Improves the type inference of variables in the following ways:

1. Infer type from "ref" assignments, like: [System.Management.Automation.Language.Parser]::ParseFile("ls", [ref] $Tokens, [ref] $null)
2. Infer type from the *Variable common parameters, like: New-Guid -WarningVariable MyVar
3. Infer type for variables set by New/Set-Variable
4. Infer type of variables set with the Data statement
5. More accurate inference of variables assigned with the PipelineVariable common parameter (Now it correctly limits the type inference to the pipeline scope)
6. Fix issue where the type of parameters and foreach statement variables were inferred based on the first assignment, rather than the closest
7. Fix type inference crash when the inferred variable had a scope assigned, like: [AstTypeInference]::InferTypeOf({$Global:true}.Ast)
8. Infer type of variables assigned with an attribute: [ValidateNotNull()]$Var1 = "Hello"
9. Infer type of variables assigned with multiple type constraints: [int] [string] $Var1 = "10"
10. Infers type of variables assigned with redirection: New-Guid 1>variable:RedirectedVar; $RedirectedVar

PR Context

Fixes#18759
Fixes#24205

PR Checklist

• PR has a meaningful title
  • Use the present tense and imperative mood when describing your changes
• Summarized changes
• Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
• This PR is ready to merge and is not Work in Progress.
  • If the PR is work in progress, please add the prefix WIP: or [ WIP ] to the beginning of the title (the WIP bot will keep its status check at Pending while the prefix is present) and remove the prefix when the PR is ready.
• Breaking changes
  • None
  • OR
  • Experimental feature(s) needed
    • Experimental feature name(s):
• User-facing changes
  • Not Applicable
  • OR
  • Documentation needed
    • Issue filed:
• Testing - New and feature
  • N/A or can only be tested interactively
  • OR
  • Make sure you've added a new test if existing tests do not effectively test the code changed
• Tooling
  • I have considered the user experience from a tooling perspective and don't believe tooling will be impacted.
  • OR
  • I have considered the user experience from a tooling perspective and opened an issue in the relevant tool repository. This may include:
    • Impact on PowerShell Editor Services which is used in the PowerShell extension for VSCode
      (which runs in a different PS Host).
      • Issue filed:
    • Impact on Completions (both in the console and in editors) - one of PowerShell's most powerful features.
      • Issue filed:
    • Impact on PSScriptAnalyzer (which provides linting & formatting in the editor extensions).
      • Issue filed:
    • Impact on EditorSyntax (which provides syntax highlighting with in VSCode, GitHub, and many other editors).
      • Issue filed:

[ghost]

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

[pull-request-quantifier-deprecated]

This PR has 405 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!

---

Quantification details

Label : Extra Large Size : +289 -116 Percentile : 80.17% Total files changed: 3 Change summary by file extension: .cs : +229 -116 .ps1 : +60 -0 

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

• Fast and predictable releases to production:
  • Optimal size changes are more likely to be reviewed faster with fewer
    iterations.
  • Similarity in low PR complexity drives similar review times.
• Review quality is likely higher as complexity is lower:
  • Bugs are more likely to be detected.
  • Code inconsistencies are more likely to be detected.
• Knowledge sharing is improved within the participants:
  • Small portions can be assimilated better.
• Better engineering practices are exercised:
  • Solving big problems by dividing them in well contained, smaller problems.
  • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

• Use the PullRequestQuantifier to quantify your PR accurately
  • Create a context profile for your repo using the context generator
  • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
  • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
  • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
• Change your engineering behaviors
  • For PRs that fall outside of the desired spectrum, review the details and check if:
    • Your PR could be split in smaller, self-contained PRs instead
    • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

• One line was added: +1 -0
• One line was deleted: +0 -1
• One line was modified: +1 -1 (git diff doesn't know about modified, it will
  interpret that line like one addition plus one deletion)
• Change percentiles: Change characteristics (addition, deletion, modification)
  of this PR in relation to all other PRs within the repository.

---

Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

[pull-request-quantifier-deprecated]

This PR has 405 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!

---

Quantification details

Label : Extra Large Size : +289 -116 Percentile : 80.17% Total files changed: 3 Change summary by file extension: .cs : +229 -116 .ps1 : +60 -0 

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

• Fast and predictable releases to production:
  • Optimal size changes are more likely to be reviewed faster with fewer
    iterations.
  • Similarity in low PR complexity drives similar review times.
• Review quality is likely higher as complexity is lower:
  • Bugs are more likely to be detected.
  • Code inconsistencies are more likely to be detected.
• Knowledge sharing is improved within the participants:
  • Small portions can be assimilated better.
• Better engineering practices are exercised:
  • Solving big problems by dividing them in well contained, smaller problems.
  • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

• Use the PullRequestQuantifier to quantify your PR accurately
  • Create a context profile for your repo using the context generator
  • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
  • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
  • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
• Change your engineering behaviors
  • For PRs that fall outside of the desired spectrum, review the details and check if:
    • Your PR could be split in smaller, self-contained PRs instead
    • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

• One line was added: +1 -0
• One line was deleted: +0 -1
• One line was modified: +1 -1 (git diff doesn't know about modified, it will
  interpret that line like one addition plus one deletion)
• Change percentiles: Change characteristics (addition, deletion, modification)
  of this PR in relation to all other PRs within the repository.

---

Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

[iSazonov]

@MartinGC94 Is it possible to split the PR to some small ones? This would significantly speed up the review.

[MartinGC94]

Not in a meaningful way. It's a new Ast visitor so we kinda need the whole thing for it to make sense.

[iSazonov]

I wonder why not use VisitDoWhileStatement/VisitDoUntilStatement method directly? It would be more clear.

If SkipChildren is for condition - it is clear that we skip it, what is StopVisit for?

[MartinGC94]

It's not possible to do it from those methods because we can only tell it to continue, skip children, or stop completely. We want to skip the condition part of the loop (if it's after the variable being inferred) but we still want to process the body.

[iSazonov]

My idea is that if we come in the methods we can return StopVisit but before directly call VisitStatementBlock for StatementBlockAst from Body.

[MartinGC94]

That doesn't work either. When calling VisitStatementBlock it will hit DefaultVisit which would return "Continue". Then it'd move on to the condition and hit the DefaultVisit again, and this time it will return "StopVisit" because it's after the cursor.

[iSazonov]

Then it'd move on to the condition

If we run for Body it cannot move to Condition.

[MartinGC94]

I tried it on a local build and it behaved the way I described.

[iSazonov]

After further thought, I believe I can take this as a step forward. Although I am inclined to believe that a direct bypass would be more convenient, simple and extensible. I'm even inclined to think that this was originally intended.

[iSazonov]

Will the inference work after follow swap?

It 'Ignores type constraint defined outside of scope' { $res =[AstTypeInference]::InferTypeOf(({function Outer { function Inner { $Test =2 $Test }[string] $Test="Hello" Inner } ... $res.Name|Should-Be 'System.Int32' }

[MartinGC94]

Yes, $Test inside the Inner function will also be inferred as an int32 in this scenario because there's an assignment inside Inner so outer scope type constraints/assignments don't matter.

[iSazonov]

And if we remove $Test = 2 the inference will stop?

[MartinGC94]

Yes. It would stop the search before it would reach the assignment because it's after the start of the Inner scope. This is not accurate to how PowerShell works, but trying to keep track of the variable values before each command invocation is quite difficult.

[iSazonov]

Thanks! For idea, we would need a safe interpreter.

[MartinGC94]

@iSazonov Thanks for the approval. Is the plan to wait for MS to review it as well?

[iSazonov]

@MartinGC94 I need a time to check all before merge. (Remind me in a couple of days if you don't see my activity.)

[microsoft-github-policy-service]

📣 Hey @MartinGC94, how did we do? We would love to hear your feedback with the link below! 🗣️

🔗 https://aka.ms/PSRepoFeedback

Microsoft Forms