Skip to content

Extract MergeSourceSpans helper and deduplicate condition branches in tag helper resolution phase#13008

Merged
chsienki merged 3 commits intomainfrom
copilot/extract-merge-source-spans-helper
Apr 13, 2026
Merged

Extract MergeSourceSpans helper and deduplicate condition branches in tag helper resolution phase#13008
chsienki merged 3 commits intomainfrom
copilot/extract-merge-source-spans-helper

Conversation

Copy link
Copy Markdown

Copilot AI commented Apr 3, 2026

Why

DefaultTagHelperResolutionPhase and its partial-class companions (ComponentTagHelperResolver, LegacyTagHelperResolver) each had several places that manually constructed a merged SourceSpan by inlining the same arithmetic — computing an end absolute index, subtracting the start, etc. This duplication was:

  • Error-prone: two of the existing inline computations used a slightly different lineCount formula (last.LineIndex - first.LineIndex + 1 vs. last.LineIndex + last.LineCount - first.LineIndex), making it hard to know which was correct.
  • Hard to audit: every copy had to be read independently to verify correctness.
  • Missing a safety net: there was no enforcement of the documented ordering requirement that first must start at or before last.

Similarly, ConvertPureCSharpExpressionValue and ComputeValueSource each had duplicate condition arms calling the same UnwrapValueChildrenToTokens fallback, and GetAttributeSource had two identical loops over token children for HtmlAttributeValueIntermediateNode and CSharpExpressionAttributeValueIntermediateNode.

What

  • Extracted MergeSourceSpans(first, last): a single, documented internal static helper that merges two ordered SourceSpan values into one covering span, using the correct formula for length, lineCount, and endCharacterIndex. Replaces all six inline span-merge expressions across the three resolver files.
  • Added Debug.Assert: enforces the precondition that first.AbsoluteIndex <= last.AbsoluteIndex, catching ordering bugs immediately in debug builds.
  • Deduplicated condition branches:
    • Collapsed the two duplicate UnwrapValueChildrenToTokens fallback arms in ComputeValueSource (bound-string + dynamic vs. complex/non-dynamic) into a single else branch.
    • Removed the redundant inner else chains in ConvertPureCSharpExpressionValue, using an early return so the UnwrapValueChildrenToTokens fallback only appears once.
    • Merged the two identical child-token loops in GetAttributeSource for HtmlAttributeValueIntermediateNode and CSharpExpressionAttributeValueIntermediateNode into a single pattern-match branch.
  • Added unit tests (DefaultTagHelperResolutionPhaseTest.cs): 5 tests covering the MergeSourceSpans helper — same-line merge, multi-line merge, adjacent spans, identical span passed for both arguments, and null file path preservation.

Copilot AI changed the title [WIP] Extract shared helpers for source span merging and duplicated resolution logic Extract MergeSourceSpans helper and deduplicate condition branches in tag helper resolution phase Apr 3, 2026
Copilot AI requested a review from chsienki April 3, 2026 01:35
@chsienki chsienki marked this pull request as ready for review April 8, 2026 20:36
@chsienki chsienki requested a review from a team as a code owner April 8, 2026 20:36
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.

Extract shared helpers for source span merging and duplicated resolution logic

4 participants