Enable nullable: System.Management.Automation.Language.TypeName by xtqqczze · Pull Request #14232 · PowerShell/PowerShell

xtqqczze · 2020-11-23T09:50:03Z

Follow-up to #14088, #14181.

xtqqczze · 2020-11-23T09:52:13Z

src/System.Management.Automation/engine/parser/ast.cs

        {
-            string fullTypeName = type.FullName;
+            string? fullTypeName = type.FullName;
+            Debug.Assert(fullTypeName is not null);


Maybe we should check for null?

if (fullTypeName is null) return false;

The Debug.Assert is necessary here to tell the compiler fullTypeName is not null.

In my opinion there should an actual null check here, but in a separate PR,

~~I don't think it can ever be null. Fullname is the qualified name of the type, including the namespace. I don't see how that could ever be null.~~

The fully qualified name of the type, including its namespace but not its assembly; or null if the current instance represents a generic type parameter, an array type, pointer type, or byref type based on a type parameter, or a generic type that is not a generic type definition but contains unresolved type parameters.

Looking at the summary of the type, it says

A simple type that is not an array or does not have generic arguments.

So basically, we are meeting the requirement that it has a typename.
I think you can bang it, maybe with a comment.

i.e.

string fullTypeName = type.FullName!;

string fullTypeName = type.FullName!;

What about the possible NullReferenceException in the next line?

type always has a fullname in the cases this class handles.

Currently, the only reference to the TypeName.IsType method is from FunctionMemberAst.IsReturnTypeVoid. But the method is internal, it is accessible to any file in System.Management.Automation, so I think we should try to avoid the NullReferenceException.

Formally fullTypeName is nullable. So I am ok with adding the Debug.Assert() here. We should avoid changing a code at annotation time.

ghost · 2020-11-30T14:00:07Z

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

xtqqczze · 2020-11-30T14:20:27Z

@powercode Can you review?

powercode · 2020-12-01T00:44:15Z

src/System.Management.Automation/engine/parser/ast.cs

        {
-            string fullTypeName = type.FullName;
+            string? fullTypeName = type.FullName;
+            Debug.Assert(fullTypeName is not null);


~~I don't think it can ever be null. Fullname is the qualified name of the type, including the namespace. I don't see how that could ever be null.~~

The fully qualified name of the type, including its namespace but not its assembly; or null if the current instance represents a generic type parameter, an array type, pointer type, or byref type based on a type parameter, or a generic type that is not a generic type definition but contains unresolved type parameters.

powercode · 2020-12-01T00:48:24Z

src/System.Management.Automation/engine/parser/ast.cs

        {
-            string fullTypeName = type.FullName;
+            string? fullTypeName = type.FullName;
+            Debug.Assert(fullTypeName is not null);


Looking at the summary of the type, it says

A simple type that is not an array or does not have generic arguments.

So basically, we are meeting the requirement that it has a typename.
I think you can bang it, maybe with a comment.

i.e.

string fullTypeName = type.FullName!;

powercode · 2020-12-01T01:03:09Z

src/System.Management.Automation/engine/parser/ast.cs

        /// The name of the assembly.
        /// </summary>
-        public string AssemblyName { get { return _type.Assembly.FullName; } }
+        public string? AssemblyName { get { return _type.Assembly.FullName; } }


I wonder if this will ever be null. I know that it's annotated as such on Assembly, but I haven't been able to produce an instance of it.

@daxian-dbw, do you know?

AssemblyName is public property in public class System.Management.Automation.Language.ReflectionTypeName, I don't think can should assume it can never return null.

Since we use .Net API without wrapping we should follow .Net.
I suggest to keep this as is and open new tracking issue in PowerShell repo and new issue in .Net Runtime repo.
Also I guess we could suddenly get null if single file packaging is used.

iSazonov · 2025-10-16T12:04:21Z

Nice to have but too old.

microsoft-github-policy-service · 2025-10-16T12:04:42Z

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

🔗 https://aka.ms/PSRepoFeedback

Microsoft Forms

Enable nullable: System.Management.Automation.Language.TypeName

92b6099

xtqqczze requested a review from daxian-dbw as a code owner November 23, 2020 09:50

ghost assigned daxian-dbw Nov 23, 2020

xtqqczze commented Nov 23, 2020

View reviewed changes

ghost added the Review - Needed The PR is being reviewed label Nov 30, 2020

powercode reviewed Dec 1, 2020

View reviewed changes

SteveL-MSFT added the CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log label Dec 15, 2020

xtqqczze closed this Jun 25, 2021

xtqqczze reopened this Jun 25, 2021

iSazonov closed this Oct 16, 2025

microsoft-github-policy-service bot removed the Review - Needed The PR is being reviewed label Oct 16, 2025

Conversation

xtqqczze commented Nov 23, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Nov 30, 2020

Uh oh!

xtqqczze commented Nov 30, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iSazonov commented Oct 16, 2025

Uh oh!

microsoft-github-policy-service bot commented Oct 16, 2025 • edited by unfurl-links bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

microsoft-github-policy-service bot commented Oct 16, 2025 •

edited by unfurl-links bot

Loading