Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 24, 2025

  • Understand the issue: Version.Parse with trailing dots shows empty string in error message
  • Analyze the code to identify where the fix should be applied
  • Design a minimal fix to pass original input for error messages
  • Add comprehensive tests for the fix
  • Build the changes successfully
  • Run tests to verify the fix works
  • Verify all tests pass - 69,511 tests passed, 0 failed
  • Address code review: refactor to use non-throwing path
  • Move throwOnFailure block to [DoesNotReturn] static local function
  • Add tests for invalid UTF8 byte sequences
  • Deduplicate FormatException throw statement
  • Remove unnecessary if condition before ThrowIfNegative

Summary

Fixed Version.Parse to include the original input string in FormatException messages when parsing fails due to trailing dots or empty components.

Changes:

  1. Modified TryParseComponent to accept original input parameter
  2. Refactored to use non-throwing Number.TryParseBinaryIntegerStyle path for better performance
  3. Moved exception throwing logic to a [DoesNotReturn] static local function
  4. Deduplicated FormatException throw by extracting string conversion to a local variable
  5. Removed unnecessary if condition - ThrowIfNegative already performs the check
  6. Added tests for invalid UTF8 byte sequences to ensure robust error handling

Verification:

  • All 69,511 System.Runtime tests pass with no failures
  • Error messages now show original input (e.g., "The input string '1.' was not in a correct format")
  • Properly handles overflow, format errors, and invalid UTF8 sequences
Original prompt

This section details on the original issue you should resolve

<issue_title>Empty parameter value in FormatException thrown by Version.Parse when input has trailing .</issue_title>
<issue_description>### Description

Version.Parse(".") throws FormatException with the message The input string '' was not in a correct format.

Reproduction Steps

var v = Version.Parse(".");
var v = Version.Parse("1.");
var v = Version.Parse("1.0.");

Expected behavior

The input string '1.0.' was not in a correct format

Actual behavior

The input string '' was not in a correct format

Regression?

No response

Known Workarounds

No response

Configuration

10.0.0-preview.4.25258.110

Other information

No response</issue_description>

Comments on the Issue (you are @copilot in this section)

@jeffhandley Marked as https://github.com/dotnet/runtime/labels/help%20wanted. A fix would need to come with thorough tests, ideally improving the existing test coverage not only related to this specific behavior change, but any other potentially affected behaviors.

Fixes #115612


💡 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 in the docs.

@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Copilot AI changed the title [WIP] Fix empty parameter value in FormatException for Version.Parse Fix Version.Parse to show original input in FormatException message when input has trailing dots Oct 24, 2025
Copilot finished work on behalf of stephentoub October 24, 2025 01:55
Copilot AI requested a review from stephentoub October 24, 2025 01:55
Copilot AI requested a review from stephentoub October 24, 2025 03:39
Copilot finished work on behalf of stephentoub October 24, 2025 03:39
…add UTF8 invalid byte tests

Co-authored-by: stephentoub <[email protected]>
Copilot finished work on behalf of stephentoub October 25, 2025 02:56
Copilot AI requested a review from stephentoub October 25, 2025 02:56
@stephentoub stephentoub marked this pull request as ready for review October 26, 2025 01:58
@Copilot Copilot AI review requested due to automatic review settings October 26, 2025 01:58
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a bug where Version.Parse displayed an empty string in error messages when parsing failed due to trailing dots (e.g., "1.0."). The fix ensures the original input string is included in FormatException messages for better diagnostics.

Key Changes:

  • Modified TryParseComponent to accept and use the original input string for error messages
  • Refactored error handling to use non-throwing TryParseBinaryIntegerStyle for better performance
  • Added comprehensive test coverage for trailing dot scenarios and invalid UTF8 sequences

Reviewed Changes

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

File Description
src/libraries/System.Private.CoreLib/src/System/Version.cs Updated TryParseComponent to accept original input parameter and refactored exception throwing to a [DoesNotReturn] static local function
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/VersionTests.cs Added tests for trailing dots with string/span/UTF8 inputs and invalid UTF8 byte sequences

@stephentoub
Copy link
Member

@EgorBot -amd -intel -arm

using System;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

BenchmarkSwitcher.FromAssembly(typeof(Benchmarks).Assembly).Run(args);

public class Benchmarks
{
    [Benchmark]
    [Arguments("1.2")]
    [Arguments("123.123.123.123")]
    public Version Parse(string s) => Version.Parse(s);
}

@stephentoub
Copy link
Member

@EgorBo, looks like your bot might need to bump some version numbers?

Copilot AI requested a review from stephentoub October 26, 2025 02:33
Copilot finished work on behalf of stephentoub October 26, 2025 02:33
Comment on lines 450 to 453
if (parsedComponent < 0)
{
ArgumentOutOfRangeException.ThrowIfNegative(parsedComponent, componentName);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot, this if condition should not be necessary, as it's going to be checked by ThrowIfNegative

Suggested change
if (parsedComponent < 0)
{
ArgumentOutOfRangeException.ThrowIfNegative(parsedComponent, componentName);
}
ArgumentOutOfRangeException.ThrowIfNegative(parsedComponent, componentName);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in commit 2fcaa84. Removed the unnecessary if (parsedComponent < 0) condition since ThrowIfNegative already performs this check.

@EgorBo
Copy link
Member

EgorBo commented Oct 26, 2025

@EgorBo, looks like your bot might need to bump some version numbers?

Yep, seems to be fixed now

auto-merge was automatically disabled October 26, 2025 03:21

Head branch was pushed to by a user without write access

Copilot AI requested a review from stephentoub October 26, 2025 03:22
Copilot finished work on behalf of stephentoub October 26, 2025 03:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Empty parameter value in FormatException thrown by Version.Parse when input has trailing .

4 participants