Use Version.TryParse to avoid exceptions when calling [System.Version]::Parse

[Copilot]

Problem

MSBuild was experiencing performance regressions in Visual Studio scenarios due to first-chance ArgumentExceptions being thrown when calling [System.Version]::Parse() on invalid version strings. This was observed during .NET SDK 10.0.100-rc.2 insertions and appears to be related to changes in dotnet/sdk#50264.

The stack trace showed exceptions originating from:

Microsoft.Build.dll!Microsoft.Build.Evaluation.Expander.WellKnownFunctions.TryExecuteWellKnownFunction(...)
    at Version.Parse(arg0)  // Line 862

While these exceptions were caught by the error handling mechanism in Expander.cs, they still caused performance degradation due to first-chance exception handling overhead.

Solution

Changed WellKnownFunctions.TryExecuteWellKnownFunction to use Version.TryParse() instead of Version.Parse(). This eliminates the exception being thrown for invalid input while maintaining the same external behavior.

Before:

if (ParseArgs.TryGetArg(args, out string? arg0) && arg0 != null)
{
    returnVal = Version.Parse(arg0);  // Throws ArgumentException on invalid input
    return true;
}

After:

if (ParseArgs.TryGetArg(args, out string? arg0) && arg0 != null)
{
    if (Version.TryParse(arg0, out Version? version))
    {
        returnVal = version;
        return true;
    }
    // Returns false on invalid input, no exception thrown
}

Impact

• Performance: Eliminates first-chance exceptions in VS scenarios, improving performance
• Compatibility: Maintains existing behavior - invalid version strings still result in appropriate error messages (either InvalidProjectFileException or unexpanded expressions with LeavePropertiesUnexpandedOnError)
• Code Quality: Uses the recommended TryParse pattern for parsing operations that may fail

Testing

Added two new tests to verify the behavior:

• PropertyFunctionVersionParseInvalidWithLeavePropertiesUnexpandedOnError - Verifies that invalid version strings with the LeavePropertiesUnexpandedOnError option return the unexpanded expression without throwing
• PropertyFunctionVersionParseInvalidThrows - Verifies that invalid version strings without the option throw InvalidProjectFileException as expected

All existing tests continue to pass, including the original PropertyFunctionVersionParse test.

Fixes #12590

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Avoid exceptions when calling [System.Version]::Parse</issue_title>
<issue_description>We're observing a regression in exception count in some VS scenarios when inserting builds of .NET SDK 10.0.100-rc.2. Specifically, there's a System.ArgumentException thrown with a stack like this:

 	mscorlib.dll!System.Version.VersionResult.SetFailure(System.Version.ParseFailureKind failure, string argument)	Unknown
 	mscorlib.dll!System.Version.TryParseVersion(string version, ref System.Version.VersionResult result)	Unknown
 	mscorlib.dll!System.Version.Parse(string input)	Unknown
 	Microsoft.Build.dll!Microsoft.Build.Evaluation.Expander.WellKnownFunctions.TryExecuteWellKnownFunction(string methodName, System.Type receiverType, Microsoft.Build.Shared.FileSystem.IFileSystem fileSystem, out object returnVal, object objectInstance, object[] args) Line 862	C#
 	Microsoft.Build.dll!Microsoft.Build.Evaluation.Expander<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance>.Function<Microsoft.Build.Execution.ProjectPropertyInstance>.Execute(object objectInstance, Microsoft.Build.Evaluation.IPropertyProvider<Microsoft.Build.Execution.ProjectPropertyInstance> properties, Microsoft.Build.Evaluation.ExpanderOptions options, Microsoft.Build.Shared.IElementLocation elementLocation) Line 4061	C#
 	Microsoft.Build.dll!Microsoft.Build.Evaluation.Expander<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance>.PropertyExpander<Microsoft.Build.Execution.ProjectPropertyInstance>.ExpandPropertyBody(string propertyBody, object propertyValue, Microsoft.Build.Evaluation.IPropertyProvider<Microsoft.Build.Execution.ProjectPropertyInstance> properties, Microsoft.Build.Evaluation.ExpanderOptions options, Microsoft.Build.Shared.IElementLocation elementLocation, Microsoft.Build.Evaluation.PropertiesUseTracker propertiesUseTracker, Microsoft.Build.Shared.FileSystem.IFileSystem fileSystem) Line 1529	C#
 	Microsoft.Build.dll!Microsoft.Build.Evaluation.Expander<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance>.PropertyExpander<Microsoft.Build.Execution.ProjectPropertyInstance>.ExpandPropertiesLeaveTypedAndEscaped(string expression, Microsoft.Build.Evaluation.IPropertyProvider<Microsoft.Build.Execution.ProjectPropertyInstance> properties, Microsoft.Build.Evaluation.ExpanderOptions options, Microsoft.Build.Shared.IElementLocation elementLocation, Microsoft.Build.Evaluation.PropertiesUseTracker propertiesUseTracker, Microsoft.Build.Shared.FileSystem.IFileSystem fileSystem) Line 1373	C#
 	Microsoft.Build.dll!Microsoft.Build.Evaluation.Expander<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance>.PropertyExpander<Microsoft.Build.Execution.ProjectPropertyInstance>.ExpandPropertiesLeaveEscaped(string expression, Microsoft.Build.Evaluation.IPropertyProvider<Microsoft.Build.Execution.ProjectPropertyInstance> properties, Microsoft.Build.Evaluation.ExpanderOptions options, Microsoft.Build.Shared.IElementLocation elementLocation, Microsoft.Build.Evaluation.PropertiesUseTracker propertiesUseTracker, Microsoft.Build.Shared.FileSystem.IFileSystem fileSystem) Line 1239	C#
 	Microsoft.Build.dll!Microsoft.Build.Evaluation.Expander<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance>.ExpandIntoStringLeaveEscaped(string expression, Microsoft.Build.Evaluation.ExpanderOptions options, Microsoft.Build.Shared.IElementLocation elementLocation) Line 497	C#
 	Microsoft.Build.dll!Microsoft.Build.Evaluation.LazyItemEvaluator<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance, Microsoft.Build.Execution.ProjectMetadataInstance, Microsoft.Build.Execution.ProjectItemDefinitionInstance>.ProcessMetadataElements(Microsoft.Build.Construction.ProjectItemElement itemElement, Microsoft.Build.Evaluation.LazyItemEvaluator<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance, Microsoft.Build.Execution.ProjectMetadataInstance, Microsoft.Build.Execution.ProjectItemDefinitionInstance>.OperationBuilderWithMetadata operationBuilder) Line 642	C#
 	Microsoft.Build.dll!Microsoft.Build.Evaluation.LazyItemEvaluator<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance, Microsoft.Build.Execution.ProjectMetadataInstance, Microsoft.Build.Execution.ProjectItemDefinitionInstance>.BuildIncludeOperation(string rootDirectory, Microsoft.Build.Construction.ProjectItemElement itemElement, bool conditionResult) Line 576	C#
 	Microsoft.Build.dll!Microsoft.Build.Evaluation.LazyItemEvaluator<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance, Microsoft.Build.Execution.ProjectMetadataInstance, Microsoft.Build.Execution.ProjectItemDef...

</details>
Fixes dotnet/msbuild#12590

<!-- START COPILOT CODING AGENT TIPS -->
---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more [Copilot coding agent tips](https://gh.io/copilot-coding-agent-tips) in the docs.

[dotnet-policy-service]

Hello @@copilot, I noticed that you’re changing an .swr file or any file under src/Package/MSBuild.VSSetup.. Please make sure to validate this change by an experimental VS insertion. This is accomplished by pushing to an exp/* branch, which requires write permissions to this repo.

[Copilot]

Pull Request Overview

This PR fixes a performance regression in Visual Studio scenarios by eliminating first-chance exceptions when parsing invalid version strings in MSBuild property functions. The change replaces Version.Parse() with Version.TryParse() to avoid throwing exceptions while maintaining the same external behavior.

• Replaces exception-throwing Version.Parse() with Version.TryParse() pattern for better performance
• Maintains existing error handling behavior for invalid version strings
• Adds comprehensive test coverage for both success and failure scenarios

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/Build/Evaluation/Expander/WellKnownFunctions.cs	Replaces Version.Parse() with Version.TryParse() to avoid throwing exceptions on invalid input
src/Build.UnitTests/Evaluation/Expander_Tests.cs	Adds two new test cases to verify behavior with invalid version strings

[Copilot]

The change breaks the established pattern where the method returns false only when arguments cannot be parsed. Now it also returns false for invalid version strings, which changes the method's behavior contract. Consider adding a comment explaining this behavioral change or ensuring all callers handle this new failure case appropriately.

[JanProvaznik]

The change breaks the established pattern where the method returns false only when arguments cannot be parsed. Now it also returns false for invalid version strings, which changes the method's behavior contract. Consider adding a comment explaining this behavioral change or ensuring all callers handle this new failure case appropriately.

@copilot add a concise comment addressing this

[JanProvaznik]

you received this comment @copilot

The change breaks the established pattern where the method returns false only when arguments cannot be parsed. Now it also returns false for invalid version strings, which changes the method's behavior contract. Consider adding a comment explaining this behavioral change or ensuring all callers handle this new failure case appropriately.
on these lines:

•                        if (Version.TryParse(arg0, out Version? version))
  

•                        {
  

•                            returnVal = version;
  

•                            return true;
  

•                        }
  

please add to the code a concise comment addressing this

[JanProvaznik]

@copilot stop refusing to add a commit with the comment in code! you're not done here

just to repeat your instruction:

you received this comment

The change breaks the established pattern where the method returns false only when arguments cannot be parsed. Now it also returns false for invalid version strings, which changes the method's behavior contract. Consider adding a comment explaining this behavioral change or ensuring all callers handle this new failure case appropriately.
on these lines:

                   if (Version.TryParse(arg0, out Version? version))
                   {
                       returnVal = version;
                       return true;
                   }

please add to the code a concise comment addressing this

[JanProvaznik]

you received this comment @copilot

The change breaks the established pattern where the method returns false only when arguments cannot be parsed. Now it also returns false for invalid version strings, which changes the method's behavior contract. Consider adding a comment explaining this behavioral change or ensuring all callers handle this new failure case appropriately.
on these lines:

                   if (Version.TryParse(arg0, out Version? version))
                   {
                       returnVal = version;
                       return true;
                   }

please add to the code a concise comment addressing this

[JanProvaznik]

@copilot !

[JanProvaznik]

I'm probably misunderstanding gh agent, I thought it gets the comments where it's tagged as context 🤔

[rainersigwald]

This doesn't validate that it doesn't throw an exception internally, which is the Perf DDRIT regression that brought this up. It already didn't expose the exception.

[rainersigwald]

Expanded on the bug and I think it'll require a big enough redesign (+ we have a workaround in dotnet/sdk#51134) that I'm going to close this.