Fix crash on Node2code operation in case of conflicts with non-unique namespaces

[aparajit-pratap]

Purpose

JIRA link: https://jira.autodesk.com/browse/QNTM-5426
GH Issue: #8981

This addresses a crash on performing node to code on class with non-unique namespace with another class: E.g. If there are 2 classes, B.C.List and A.B.C.List, where A, B, and C are namespaces for 2 classes of the same name List, the language cannot distinguish between them even if both classes are pre-qualified with their full namespaces, since in this case B.C.List can be considered a partial namespace for A.B.C.List. In such a case node to code fails to find a (shortest) unique qualified name for example, a List.Count node (as it cannot decide which namespaces to qualify the class List with) and returns a null for the unique name. The code following this step does not account for a null name and thus results in a crash.

The fix is to return the fully qualified name no matter whether it is unique or not so as to prevent the crash and let the code block node throw an error indicating to the user of the namespace conflict as follows:

The above graph is a result of a node-to-code command on DSCore.String.ToUpper node in the presence of another class with the same name belonging to namespace X.DSCore.String.

Note: The above error message appears only at compile-time (manual mode). When the graph is executed directly (Auto mode), the runtime error message is not so useful:

We can consider displaying the above compilation error (which is more useful to the user) as a future improvement.

Declarations

Check these if you believe they are true

• The code base is in a better state after this PR
• Is documented according to the standards
• The level of testing this PR includes is appropriate
• User facing strings, if any, are extracted into *.resx files
• All tests pass using the self-service CI.
• Snapshot of UI changes, if any.
• Changes to the API follow Semantic Versioning, and are documented in the API Changes document.

Reviewers

@ColinDayOrg

Colin, this is my take on the issue that you've been investigating. I've added some background describing the current behavior, reasons for the crash, and fix. Please take a look.

FYIs

@mjkkirschner

[ColinDayOrg]

Do we have any test coverage for this? Other than that LGTM.

[ColinDayOrg]

This may possibly address the crash I am looking at, but in the call to CreateNodeFromString from CreateNodeFromShortName there are definitely a couple of places where it is possible to try to dereference a null value. It may still be needed or wanted to add some guard checks. John added some information to the issue that I was going to take a look at today.

[aparajit-pratap]

@ColinDayOrg will try adding some tests and will wait for you to finish testing and/or adding further checks if needed. Thanks.

[ColinDayOrg]

This change does appear to address the crash.

[aparajit-pratap]

@ColinDayOrg I've added a test. Please let me know if it's good to go.

[ColinDayOrg]

LGTM