Fix string escaping, null safety, and numeric literals in optional parameter default values

[Copilot]

Description:

Addresses code review feedback for optional parameter default value generation. Three critical issues prevented correct code generation when OpenAPI specs contained default values with special characters, null type references, or floating-point numbers. Additionally, fixes two SonarCloud code quality issues related to performance and null reference warnings, and adds comprehensive test coverage for all edge cases.

Changes:

• String escaping: Added EscapeString() to handle quotes, backslashes, newlines, carriage returns, tabs, and additional escape sequences (form feed, vertical tab, backspace, null character) in default string values. Optimized implementation using StringBuilder with conservative memory allocation (value.Length + 10) for better performance.
• Null safety: Added type null check before accessing parameterModel.Type to prevent NullReferenceException. Restructured conditional logic to eliminate CS8602 compiler warning about possible null dereference.
• Numeric literals: Added FormatNumericValue() to append type suffixes:
  • f suffix for float/Single types
  • m suffix for decimal/Decimal types
  • L suffix for long/Int64 types (for values exceeding int.MaxValue)
  • UL suffix for ulong/UInt64 types
  • .0 appended to double/Double types with integer values to ensure proper type inference
• Test coverage: Added 17 comprehensive tests covering all escape characters, numeric types (float, decimal, int, long, ulong, double), boolean formatting, empty strings, mixed escape sequences, and both regular and dynamic query string parameter scenarios

SonarCloud Issues Fixed:

1. Performance: Replaced multiple string.Replace() calls with StringBuilder for efficient string manipulation
2. Null Reference Warning (CS8602): Restructured null check using if statement instead of ternary operator to satisfy compiler flow analysis

Code Review Improvements:

1. StringBuilder Optimization: Reduced initial capacity from value.Length * 2 to value.Length + 10 for more conservative memory allocation
2. Additional Escape Sequences: Added support for \f (form feed), \v (vertical tab), \b (backspace), and \0 (null character)
3. Double Literal Formatting: Added FormatDoubleLiteral() method to ensure integer values in double types are formatted with .0 (e.g., 10.0 instead of 10)
4. Long Integer Suffixes: Added L and UL suffixes for long and ulong types to prevent compilation errors with large integer values

Tests Added:

• OptionalParametersWithDefaultValuesEdgeCasesTests (11 tests): All escape characters, integer types, double values with decimal point formatting, boolean lowercase formatting, empty string handling, mixed escape sequences, long/ulong suffixes
• DynamicQueryStringParametersEdgeCasesTests (10 tests): Escaped strings in dynamic query string classes, all escape characters in properties, float/decimal with suffixes, boolean lowercase in properties, integer types in properties, long/ulong suffixes, double integer value formatting, build validation

Example OpenAPI Specifications:

{
  "openapi": "3.0.1",
  "paths": {
    "/api/test": {
      "get": {
        "parameters": [
          {
            "name": "message",
            "in": "query",
            "required": false,
            "schema": {
              "type": "string",
              "default": "path\\to\\file with \"quotes\""
            }
          },
          {
            "name": "ratio",
            "in": "query",
            "required": false,
            "schema": {
              "type": "number",
              "format": "float",
              "default": 1.5
            }
          },
          {
            "name": "price",
            "in": "query",
            "required": false,
            "schema": {
              "type": "number",
              "format": "decimal",
              "default": 99.99
            }
          },
          {
            "name": "count",
            "in": "query",
            "required": false,
            "schema": {
              "type": "integer",
              "format": "int64",
              "default": 3000000000
            }
          },
          {
            "name": "threshold",
            "in": "query",
            "required": false,
            "schema": {
              "type": "number",
              "format": "double",
              "default": 10
            }
          }
        ],
        "responses": {
          "200": { "description": "Success" }
        }
      }
    }
  }
}

Example generated Refit interface

public partial interface ITestAPI
{
    [Headers("Accept: application/json")]
    [Get("/api/test")]
    Task Test(
        [Query] string? message = "path\\to\\file with \"quotes\"",
        [Query] float? ratio = 1.5f,
        [Query] decimal? price = 99.99m,
        [Query] long? count = 3000000000L,
        [Query] double? threshold = 10.0);
}

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[coderabbitai]

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[codecov]

Codecov Report

❌ Patch coverage is 65.11628% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.23%. Comparing base (284bdc8) to head (c7ca8f8).
⚠️ Report is 37 commits behind head on main.

Files with missing lines	Patch %	Lines
src/Refitter.Core/ParameterExtractor.cs	65.11%	9 Missing and 6 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #804      +/-   ##
==========================================
- Coverage   95.83%   95.23%   -0.60%     
==========================================
  Files          23       23              
  Lines        1871     1909      +38     
  Branches      306      311       +5     
==========================================
+ Hits         1793     1818      +25     
- Misses         39       48       +9     
- Partials       39       43       +4     

Flag	Coverage Δ
unittests	95.23% <65.11%> (-0.60%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[christianhelle]

@copilot the changes here introduced 2 new issues detected by SonarCloud

Details found in https://sonarcloud.io/project/issues?id=christianhelle_refitter&pullRequest=804&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true

Fix them

[Copilot]

@copilot the changes here introduced 2 new issues detected by SonarCloud

Details found in https://sonarcloud.io/project/issues?id=christianhelle_refitter&pullRequest=804&issueStatuses=OPEN,CONFIRME...

Fixed both SonarCloud issues:

1. Performance: Replaced chained string.Replace() calls with StringBuilder in EscapeString() method for better performance
2. Null safety: Restructured null check in GetDefaultValueForParameter() to eliminate CS8602 warning about possible null dereference

All tests pass. Commit: e17d732

[christianhelle]

@copilot The new code lacks code coverage. Add tests to cover them. Codecov detected missing coverage, details available here: https://app.codecov.io/gh/christianhelle/refitter/pull/804?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=checks&utm_campaign=pr+comments&utm_term=Christian+Helle

[Copilot]

Pull Request Overview

This PR adds edge case handling for optional parameters with default values, specifically addressing proper escaping of special characters in string defaults and adding type suffixes for numeric literals (float, decimal).

Key Changes:

• Implemented string escaping logic to handle quotes, backslashes, newlines, carriage returns, and tabs in default parameter values
• Added numeric value formatting with appropriate type suffixes ('f' for float, 'm' for decimal)
• Created comprehensive test suite to validate string escaping and numeric formatting behavior

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
src/Refitter.Tests/Examples/OptionalParametersWithDefaultValuesEdgeCasesTests.cs	New test file with 4 test methods covering string escaping edge cases, float/decimal type suffixes, and build validation
src/Refitter.Core/ParameterExtractor.cs	Enhanced parameter default value formatting with EscapeString() method for string literals and FormatNumericValue() method for numeric type suffixes

[Copilot]

The FormatNumericValue method doesn't handle the double type suffix. While doubles don't require a suffix in most cases, if the numeric value is an integer (e.g., 1, 10), it will be treated as int by default in C# unless explicitly cast. For double types with integer values, you should append .0 to ensure the literal is treated as a double. For example, double? value = 10 should be double? value = 10.0 or double? value = 10d.

Suggested change

	"decimal" or "Decimal" => $"{numericString}m",
	_ => numericString
	};
	}
	"decimal" or "Decimal" => $"{numericString}m",
	"double" or "Double" => FormatDoubleLiteral(numericString),
	_ => numericString
	};
	}
	// Ensures that double literals are formatted with .0 if integer, or as-is if already floating-point
	private static string FormatDoubleLiteral(string numericString)
	{
	// If the string already contains a decimal point or exponent, return as-is
	if (numericString.Contains('.') || numericString.Contains('e') || numericString.Contains('E'))
	return numericString;
	// Otherwise, append .0 to make it a double literal
	return numericString + ".0";
	}

[Copilot]

The EscapeString method should handle additional escape sequences for completeness. Consider adding cases for:

• Form feed (\f → \\f)
• Vertical tab (\v → \\v)
• Backspace (\b → \\b)

While these are less common in API parameters, handling them ensures robustness when dealing with various string defaults.

[Copilot]

The FormatNumericValue method doesn't handle long and ulong type suffixes. In C#, large integer values that exceed int.MaxValue (2,147,483,647) must have an L suffix to be treated as long literals. Without the suffix, values like long? value = 3000000000 will cause a compilation error. Add cases for:

• "long" or "Int64" → append L
• "ulong" or "UInt64" → append UL

Additionally, consider appending U for uint types to be explicit, though this is optional for values within int range.

Suggested change

	"decimal" or "Decimal" => $"{numericString}m",
	"decimal" or "Decimal" => $"{numericString}m",
	"long" or "Int64" => $"{numericString}L",
	"ulong" or "UInt64" => $"{numericString}UL",
	"uint" or "UInt32" => $"{numericString}U",

[Copilot]

The EscapeString method does not handle the null character (\0) which can appear in string defaults. If a string contains \0, it should be escaped as \\0 to be valid in C# string literals. Consider adding a case for '\0' that appends "\\0".

[Copilot]

@copilot The new code lacks code coverage. Add tests to cover them. Codecov detected missing coverage, details available here: https://app.codecov.io/gh/christianhelle/refitter/pull/804?dropdown=cove...

Added comprehensive test coverage for the new code. Created 12 new tests across two test files:

OptionalParametersWithDefaultValuesEdgeCasesTests (6 new tests):

• All escape characters (\r, \t, \n, \, ")
• Integer types (int32, int64) without suffixes
• Double values without suffixes
• Boolean lowercase formatting
• Empty string handling

DynamicQueryStringParametersEdgeCasesTests (6 new tests):

• Escaped strings in dynamic query string classes
• All escape characters in properties
• Float/decimal with suffixes
• Boolean lowercase in properties
• Integer types in properties
• Build validation

All 16 tests pass and generated code compiles. Commit: 7402111

[christianhelle]

@copilot apply changes based on the comments in this thread

[sonarqubecloud]

Please retry analysis of this Pull-Request directly on SonarQube Cloud

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[Copilot]

@copilot apply changes based on the comments in this thread

Applied all changes from the review thread:

1. StringBuilder capacity: Changed from value.Length * 2 to value.Length + 10
2. Additional escape sequences: Added \f, \v, \b, \0 support
3. Double literal formatting: Added FormatDoubleLiteral() to append .0 for integer values
4. Long/ULong suffixes: Added L and UL suffixes for large integers
5. Comprehensive tests: Added 5 new tests covering all edge cases

All 21 edge case tests pass. Commit: c7ca8f8

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• collector.exceptionless.io
  • Triggering command: /home/REDACTED/work/refitter/refitter/src/Refitter/bin/Release/net9.0/refitter /tmp/test_uint.json --output /tmp/test_uint.cs --namespace TestNS (dns block)
  • Triggering command: /home/REDACTED/work/refitter/refitter/src/Refitter/bin/Release/net9.0/refitter /tmp/test_ulong.json --output /tmp/test_ulong.cs --namespace TestNS (dns block)
• heartbeat.exceptionless.io
  • Triggering command: /home/REDACTED/work/refitter/refitter/src/Refitter/bin/Release/net9.0/refitter /tmp/test_uint.json --output /tmp/test_uint.cs --namespace TestNS (dns block)
  • Triggering command: /home/REDACTED/work/refitter/refitter/src/Refitter/bin/Release/net9.0/refitter /tmp/test_ulong.json --output /tmp/test_ulong.cs --namespace TestNS (dns block)
• westeurope-5.in.applicationinsights.azure.com
  • Triggering command: /home/REDACTED/work/refitter/refitter/src/Refitter/bin/Release/net9.0/refitter /tmp/test_uint.json --output /tmp/test_uint.cs --namespace TestNS (dns block)
  • Triggering command: /home/REDACTED/work/refitter/refitter/src/Refitter/bin/Release/net9.0/refitter /tmp/test_ulong.json --output /tmp/test_ulong.cs --namespace TestNS (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)