Fix crash on Node2code operation in case of conflicts with non-unique namespaces #9255
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Do we have any test coverage for this? Other than that LGTM. |
Contributor
|
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. |
Contributor
Author
|
@ColinDayOrg will try adding some tests and will wait for you to finish testing and/or adding further checks if needed. Thanks. |
Contributor
|
This change does appear to address the crash. |
Contributor
Author
|
@ColinDayOrg I've added a test. Please let me know if it's good to go. |
Contributor
|
LGTM |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.ListandA.B.C.List, whereA,B, andCare namespaces for 2 classes of the same nameList, the language cannot distinguish between them even if both classes are pre-qualified with their full namespaces, since in this caseB.C.Listcan be considered a partial namespace forA.B.C.List. In such a case node to code fails to find a (shortest) unique qualified name for example, aList.Countnode (as it cannot decide which namespaces to qualify the classListwith) and returns anullfor the unique name. The code following this step does not account for anullname 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.ToUppernode in the presence of another class with the same name belonging to namespaceX.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
*.resxfilesReviewers
@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