Fixed expression nodes created with custom param names incorrectly clearing during internal cleanup

[Arlodotexe]

Fixes #4595

This PR fixes an issue where expression nodes created with custom param names were incorrectly cleared during internal cleanup. The bug was introduced in #4183.

The fix has been tested against the original bugged code provided in #4183, and the repro provided in #4595.

PR Checklist

Please check if your PR fulfills the following requirements:

• Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
• Based off latest main branch of toolkit
• Tested code with current supported SDKs
• New component
  • Pull Request has been submitted to the documentation repository instructions. Link:
  • Added description of major feature to project description for NuGet package (4000 total character limit, so don't push entire description over that)
  • If control, added to Visual Studio Design project
• Sample in sample app has been added / updated (for bug fixes / features)
  • Icon has been created (if new sample) following the Thumbnail Style Guide and templates
• New major technical changes in the toolkit have or will be added to the Wiki e.g. build changes, source generators, testing infrastructure, sample creation changes, etc...
• Tests for the changes have been added (for bug fixes / features) (if applicable)
• Header has been added to all new source files (run build/UpdateHeaders.bat)
• Contains NO breaking changes

Other information

n/a

[ghost]

Thanks Arlodotexe for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

[arcadiogarcia]

LGTM!

[michael-hawker]

@Arlodotexe quick follow-up question here. Is this anything we can add a unit test for?

[Arlodotexe]

@michael-hawker I considered it, but we currently don't have any tests for ExpressionFunctions and ExpressionNode.

Even the minimum tests could have caught both these issues before they happened, so I'd rather have those before writing something more specific.

I can draw up a few of those, if that works. Otherwise we can save adding basic tests for the Labs port.

[michael-hawker]

Good point @Arlodotexe, mind opening a tracking issue and highlighting some thoughts on this for 8.0?

[Arlodotexe]

Good point @Arlodotexe, mind opening a tracking issue and highlighting some thoughts on this for 8.0?

As we move to the Labs-style architecture for 8.0, part of the migration process should be to add any missing essentials - samples, documentation, UI / unit tests, etc. This would be covered by that process.

We should meet and discuss this a bit more before we ship 7.1.3/7.2. We'll want a checklist of sorts for migrating to the new repo arch, then create tracking issues (or a project board) for each component or namespace.

[michael-hawker]

Marking as signed-off as @arcadiogarcia reviewed as well.