Implement Null Coalescing and Null Coalescing assignment operators

[adityapatwardhan]

PR Summary

Implement the Null Coalescing ?? and Null Coalescing Assignment ??= operators.

PR Context

The operators are discussed in the issue #3240
This PR addresses part of RFC PowerShell/PowerShell-RFC#223

The PR is marked as a Breaking Change due to changes in the TokenFlags enum. The changes were made to include the new TokenFlag - BinaryPrecedenceCoalesce. While making this change, it was also decided to create more space in the BinaryPrecedence section of the enum for future binary operators. The BinaryPrecedenceMask was also changed from 0x07 to 0x0f. The order of precedence is not changed. We expected it can cause a breaking change for binary modules as C# treats enums as constants. Though, it is a breaking change we expect the impact to be pretty low as the usage of precedence token flags should be fairly low.

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: Add documentation for ?? ??= ?. and ?[] operators MicrosoftDocs/PowerShell-Docs#4925
• 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 enumerated concerns in the summary. This may include:
    • Impact on PowerShell Editor Services which is used in the PowerShell extension for VSCode (which runs in a different PS Host).
    • Impact on Completions (both in the console and in editors) - one of PowerShell's most powerful features.
    • Impact on PSScriptAnalyzer (which provides linting & formatting in the editor extensions).
    • Impact on EditorSyntax (which provides syntax highlighting with in VSCode, GitHub, and many other editors).
      • Issue opened Possible update to syntax highlighting for ?? ??= ?. ?[] operators EditorSyntax#185

[kilasuit]

FYI this will clash with Posh-Git that implements an alias of ?? as below

CommandType     Name                                               Version    Source
-----------     ----                                               -------    ------
Alias           ?? -> Invoke-NullCoalescing                        0.7.3      Posh-git

[rkeithhill]

We've already moved the use of ?? to internal-only in v1 and will happily get rid of it altogether for PS v7.

[rjmholt]

Really nice that this didn't require any changes to the parser and generally quite contained elsewhere -- feels like a vindication of good code for assignment processing.

Left a few comments.

[daxian-dbw]

Talked to @adityapatwardhan offline.
After more thoughts, supporting the coalesce operation in binder doesn't really make sense. The operation is so simple that the restriction generated from binder will basically be the same checks we will do if the support is done in compiler directly. So after talking with @adityapatwardhan, we decided to move the implementation back to compiler.

The extended binary operation approach could be useful for a binary operation that is more complex, in case we need another binary operator in future.

[SeeminglyScience]

Since enum's are treated like constants, this would be a breaking change for compiled projects right?

[adityapatwardhan]

Yes, this will be a breaking change. We are expecting the impact to very small though. I have updated the PR Context in description with more details.

[TylerLeonhardt]

This issue has changes that could potentially change syntax. Please consider adding this feature to EditorSyntax which is used for syntax highlighting in GitHub, Visual Studio Code, Atom, Sublime Text, and many more locations.

Consistent syntax highlighting is very important for the language and a feature isn't "complete" until syntax highlighting is what is expected.

If you can't contribute to EditorSyntax, at least open an issue to track the work - however, please note, that no one is actively working on the repo and so the work will likely not get done in a timely manner.

We hope that you consider contributing to EditorSyntax.

(note this is copy/pasted text for any change that looks like it could impact EditorSyntax - and will be a bot in the future)

[TylerLeonhardt]

(thanks for opening the issue already!)

[daxian-dbw]

LGTM! Now waiting on the breaking change review from the committee.

[daxian-dbw]

It just occurred to me that none of the existing ICustomAstVisitor2.VisitBinaryExpression implementation is changed.
@adityapatwardhan did you review all existing VisitBinaryExpression and VisitAssignmentStatement to see if anything needs to be changed? For example, IsConstantValueVisitor might need to be updated for the ?? binary operation, as this operation can be potentially constant value.

[rjmholt]

IsConstantValueVisitor might need to be updated for the ?? binary operation, as this operation can be potentially constant value.

It looks like that's just a question of the token flag:

PowerShell/src/System.Management.Automation/engine/parser/ConstantValues.cs

Lines 196 to 201 in beb8b44

	public object VisitBinaryExpression(BinaryExpressionAst binaryExpressionAst)
	{
	return binaryExpressionAst.Operator.HasTrait(TokenFlags.CanConstantFold) &&
	(bool)binaryExpressionAst.Left.Accept(this) && (bool)binaryExpressionAst.Right.Accept(this)
	&& !IsNullDivisor(binaryExpressionAst.Right);
	}

[adityapatwardhan]

@daxian-dbw I will review all the existing VisitBinaryExpression and VisitAssignmentStatement to see if anything needs to change. I will submit those changes as part of a different PR.

[daxian-dbw]

@PowerShell/powershell-committee reviewed the changes to TokenFlags and agreed to accept this breaking change. We can revisit this decision if there is new feedback coming in during the use of 7.0.0-preview.5 and 7.0.0-preview.6 of pwsh.

[ghost]

🎉v7.0.0-preview.5 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes