Make the parameter 'args' no-nullable in the public `ConsoleHost` APIs by daxian-dbw · Pull Request #13429

Make the parameter 'args' no-nullable in the public `ConsoleHost` APIs by daxian-dbw · Pull Request #13429 · PowerShell/PowerShell

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

PR Summary

This is a follow-up on #11482
We actually don't expect args to contain null value in it. For example, the use of args in GetSwitchKey at here indicates we are not expecting an element of args is null, and many other places like ParseFile.

/cc @iSazonov Maybe we should validate if args contains any null elements, maybe that's not necessary given it has been working fine so far, but either way, I'm deferring that to a separate PR.

PR Context

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

Maybe we should validate if args contains any null elements, maybe that's not necessary given it has been working fine so far, but either way, I'm deferring that to a separate PR.

.Net team confirmed that it is not null for Main() - right signature Main(string[] args).
For UnmanagedEntry users could send null. We do not check this previously so we can (1) document this explicitly that it is Main() signature, (2) add the check.

For UnmanagedEntry users could send null.

Why is that? Yes, there is a null assignment in Program.cs but that's the arg for exec, which requires a null element to indicate the end of args. I don't think that null will be passed to UnmanagedEntry.Start.

ConsoleShell.Start is public API, so a user potentially can pass in a string array with null elements. If we want to validate args, then I guess that should be done in CommandLineParameterParser.Parse.