Skip to content

feat: add missing [StringSyntax(DateTimeFormat)] to DateTimeOffset methods in BindConverter#66532

Open
EduardF1 wants to merge 1 commit intodotnet:mainfrom
EduardF1:feat/stringsyntax-remaining-formats
Open

feat: add missing [StringSyntax(DateTimeFormat)] to DateTimeOffset methods in BindConverter#66532
EduardF1 wants to merge 1 commit intodotnet:mainfrom
EduardF1:feat/stringsyntax-remaining-formats

Conversation

@EduardF1
Copy link
Copy Markdown

Summary

This PR addresses issue #44535 by adding the missing [StringSyntax] attributes in BindConverter.cs.

Changes Made

A previous PR added [StringSyntax] annotations to the DateTime, DateOnly, and TimeOnly format overloads in BindConverter, but inadvertently omitted the equivalent DateTimeOffset overloads. This PR fixes that gap:

  • FormatValue(DateTimeOffset value, string format, ...) → [StringSyntax(StringSyntaxAttribute.DateTimeFormat)]
  • FormatValue(DateTimeOffset? value, string format, ...) → [StringSyntax(StringSyntaxAttribute.DateTimeFormat)]
  • TryConvertToDateTimeOffset(obj, culture, string format, ...) → [StringSyntax(StringSyntaxAttribute.DateTimeFormat)]
  • TryConvertToNullableDateTimeOffset(obj, culture, string format, ...) → [StringSyntax(StringSyntaxAttribute.DateTimeFormat)]

Investigation of Remaining Format Types

The issue also listed EnumFormat, GuidFormat, NumericFormat, and TimeSpanFormat as remaining items. After a thorough audit of the codebase:

  • GuidFormat: No public aspnetcore methods accept a Guid-specific format string (route constraints use Guid.TryParse without a format parameter).
  • NumericFormat: BindConverter numeric FormatValue methods do not accept format strings. IHtmlGenerator.FormatValue(object, string) accepts object (not a typed numeric value), so annotating it with NumericFormat would be misleading/incorrect — as confirmed by closed draft PR #62179 which attempted exactly this and was not merged.
  • TimeSpanFormat: No public aspnetcore methods take a TimeSpan-specific format string.
  • EnumFormat: Enum formatting is handled internally by private methods that call �alue.ToString() with no user-supplied format string.

The BCL polyfill at src/Shared/CodeAnalysis/StringSyntaxAttribute.cs already defines all four constants — they're simply not applicable to any current public API surface in aspnetcore.

Fixes #44535

@EduardF1 EduardF1 requested a review from a team as a code owner April 29, 2026 22:19
@github-actions github-actions Bot added the needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically label Apr 29, 2026
@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Apr 29, 2026
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Thanks for your PR, @EduardF1. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@Youssef1313
Copy link
Copy Markdown
Member

Related past PR: #44663

@JamesNK @MackinnonBuck for review.

… BindConverter

The previous PR (commit f036ca8) added StringSyntax annotations to DateTime,
DateOnly, and TimeOnly methods but missed the equivalent DateTimeOffset overloads.
This commit fixes that omission.

Fixes dotnet#44535 (partially)
@EduardF1 EduardF1 force-pushed the feat/stringsyntax-remaining-formats branch from 48687fc to a905e8f Compare April 30, 2026 18:02
@EduardF1
Copy link
Copy Markdown
Author

EduardF1 commented May 5, 2026

For visibility on the failing checks: I pulled the AzDO build timeline for build 1404094 and the two real failures are:

  1. Run build.sh (Ubuntu x64)error : Failed to convert '.../Microsoft.Extensions.Features.dll' to webcil: Could not find file '...Microsoft.Extensions.Features.dll' from Microsoft.NET.Sdk.WebAssembly.Browser.targets(364,5). Missing-artifact / build orchestration error, not produced by the source change.
  2. Helix x64 Subset 1InMemory.FunctionalTests--net11.0 and Microsoft.AspNetCore.Server.Kestrel.Tests--net11.0 work items failed under Microsoft.DotNet.Helix.Sdk.MultiQueue.targets(91,5). Both are Kestrel/Hosting tests, completely independent of the BindConverter.cs change in this PR (which is purely metadata — adding [StringSyntax(StringSyntaxAttribute.DateTimeFormat)] to four already-existing parameters).

Build Analysis is just rolling those two up. Could a maintainer trigger a rerun — or, if these are known-flaky on main right now, ignore the gates? Happy to rebase if that helps.

@Youssef1313
Copy link
Copy Markdown
Member

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 4 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-contribution Indicates that the PR has been added by a community member needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add StringSyntax formats throughout source code

2 participants