Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 25 additions & 2 deletions .github/workflows/api-review-baselines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,29 @@ jobs:
pull_number: prNumber
});
let baseSha = pullRequest.base.sha;
const targetSha = pullRequest.merge_commit_sha ?? pullRequest.head.sha;
// pull_request_target's `base.sha` is the original base from when the PR was opened, so it
// includes anything pushed to the target branch while the PR was open. The merge commit's
// first parent is the target tip immediately before the merge — that gives a diff that
// reflects only what the PR introduced.
if (pullRequest.merged && pullRequest.merge_commit_sha) {
try {
const { data: mergeCommit } = await github.rest.git.getCommit({
owner,
repo,
commit_sha: pullRequest.merge_commit_sha
});
if (mergeCommit.parents.length > 0) {
baseSha = mergeCommit.parents[0].sha;
console.log(`Using merge commit's first parent ${baseSha} as base.`);
}
} catch (err) {
console.log(`Failed to fetch merge commit; falling back to PR base.sha: ${err.message}`);
}
}
const files = await github.paginate(github.rest.pulls.listFiles, {
owner,
repo,
Expand All @@ -65,8 +88,8 @@ jobs:
}));
core.setOutput('pr_number', String(prNumber));
core.setOutput('base_sha', pullRequest.base.sha);
core.setOutput('target_sha', pullRequest.merge_commit_sha ?? pullRequest.head.sha);
core.setOutput('base_sha', baseSha);
core.setOutput('target_sha', targetSha);
core.setOutput('has_baselines', String(baselineFiles.length > 0));
core.setOutput('files_json', JSON.stringify(baselineFiles));
Expand Down
78 changes: 73 additions & 5 deletions eng/Tools/ApiChief/Commands/EmitDelta.cs
Original file line number Diff line number Diff line change
Expand Up @@ -163,27 +163,94 @@ private static string FormatDiffMarkdown(ApiType type)

var typeAdded = type.IsNew;
var typeRemoved = type.IsRemoved;
var stageTag = FormatStageTag(type.Stage);

if (typeRemoved)
{
lines.Add($"- {type.Type}");
lines.Add($"- {stageTag}{type.Type}");
}
else if (typeAdded)
{
lines.Add($"+ {type.Type}");
lines.Add($"+ {stageTag}{type.Type}");
}
else if (type.PreviousType != null && type.PreviousType != type.Type)
{
lines.Add($"- {stageTag}{type.PreviousType}");
lines.Add($"+ {stageTag}{type.Type}");
}
else
{
lines.Add($" {type.Type}");
lines.Add($" {stageTag}{type.Type}");
}

AppendStageDiffLine(lines, type.Removals, '-');
AppendStageDiffLine(lines, type.Additions, '+');
AppendGroupedDiffMembers(lines, type);

return $"```diff{Environment.NewLine}{string.Join(Environment.NewLine, lines)}{Environment.NewLine}```{Environment.NewLine}";
var wrapped = lines.SelectMany(WrapDiffLine);
return $"```diff{Environment.NewLine}{string.Join(Environment.NewLine, wrapped)}{Environment.NewLine}```{Environment.NewLine}";
}

private const int MaxDiffLineLength = 160;

private static IEnumerable<string> WrapDiffLine(string line)
{
if (line.Length <= MaxDiffLineLength)
{
yield return line;
yield break;
}

var prefix = line.Length >= 2 ? line[..2] : " ";
var content = line.Length >= 2 ? line[2..] : line;
const string continuationIndent = " ";
var firstMax = MaxDiffLineLength - prefix.Length;
var contMax = MaxDiffLineLength - prefix.Length - continuationIndent.Length;

var isFirst = true;
while (content.Length > (isFirst ? firstMax : contMax))
{
var max = isFirst ? firstMax : contMax;
var breakAt = FindWrapBreak(content, max);
var chunk = content[..breakAt].TrimEnd();
yield return isFirst ? prefix + chunk : prefix + continuationIndent + chunk;
content = content[breakAt..].TrimStart();
isFirst = false;
}

if (content.Length > 0)
{
yield return isFirst ? prefix + content : prefix + continuationIndent + content;
}
}

private static int FindWrapBreak(string value, int maxLen)
{
if (value.Length <= maxLen)
{
return value.Length;
}

// Prefer breaking right after a comma to keep type lists readable.
var commaIdx = value.LastIndexOf(',', maxLen - 1);
if (commaIdx > 0 && commaIdx + 1 < value.Length && value[commaIdx + 1] == ' ')
{
return commaIdx + 2;
}

var spaceIdx = value.LastIndexOf(' ', maxLen - 1);
if (spaceIdx > 0)
{
return spaceIdx + 1;
}

// No good break point — break at the limit so we still wrap.
return maxLen;
}

private static string FormatStageTag(ApiStage stage)
=> stage != ApiStage.Stable ? $"[{stage}] " : string.Empty;

private static void AppendStageDiffLine(List<string> lines, ApiType? changeSet, char prefix)
{
if (changeSet?.Stage != null && changeSet.Stage != ApiStage.Stable)
Expand Down Expand Up @@ -257,7 +324,8 @@ private static void AddDiffMembers(List<(string Name, string Line)> entries, ISe

foreach (var member in members.OrderBy(m => GetMemberName(m.Member), StringComparer.Ordinal).ThenBy(m => m.Member, StringComparer.Ordinal))
{
entries.Add((GetMemberName(member.Member), $"{prefix} {member.Member}"));
var stageTag = FormatStageTag(member.Stage);
entries.Add((GetMemberName(member.Member), $"{prefix} {stageTag}{member.Member}"));
}
}

Expand Down
6 changes: 5 additions & 1 deletion eng/Tools/ApiChief/Model/ApiModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,13 @@ private static ISet<ApiType> FindChanges(ApiModel baseline, ApiModel current, bo
var additions = CreateChangeSet(addedStage, addedMethods, addedFields, addedProperties);
var removals = CreateChangeSet(removedStage, removedMethods, removedFields, removedProperties);

var headerChanged = currentType != null && baselineType != null && currentType.Type != baselineType.Type;

if (addedStage == ApiStage.Stable
&& removedStage == ApiStage.Stable
&& additions == null
&& removals == null)
&& removals == null
&& !headerChanged)
{
return null;
}
Expand All @@ -124,6 +127,7 @@ private static ISet<ApiType> FindChanges(ApiModel baseline, ApiModel current, bo
{
Type = outputType.Type,
Stage = currentType?.Stage ?? baselineType?.Stage ?? ApiStage.Stable,
PreviousType = headerChanged ? baselineType!.Type : null,
Methods = ToDisplayMembers(sharedMethods),
Fields = ToDisplayMembers(sharedFields),
Properties = ToDisplayMembers(sharedProperties),
Expand Down
42 changes: 40 additions & 2 deletions eng/Tools/ApiChief/Model/ApiType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,45 @@ public sealed class ApiType : IEquatable<ApiType>
[JsonIgnore]
public bool IsRemoved { get; set; }

public bool Equals(ApiType? other) => other != null && Type == other.Type;
/// <summary>
/// The baseline declaration when only the type header changed (e.g. an interface was added or removed).
/// Not serialized; only used by the in-memory delta to surface header-only modifications.
/// </summary>
[JsonIgnore]
public string? PreviousType { get; set; }

/// <summary>
/// Stable identity for matching baseline and current types. Strips the base/interface list and generic
/// constraints so that changes to those are reported as modifications instead of a removed-and-re-added type.
/// </summary>
[JsonIgnore]
public string Identity => GetIdentity(Type);

private static string GetIdentity(string type)
{
if (string.IsNullOrEmpty(type))
{
return string.Empty;
}

var endIdx = type.Length;

var whereIdx = type.IndexOf(" where ", StringComparison.Ordinal);
if (whereIdx >= 0)
{
endIdx = whereIdx;
}

var colonIdx = type.IndexOf(" : ", StringComparison.Ordinal);
if (colonIdx >= 0 && colonIdx < endIdx)
{
endIdx = colonIdx;
}

return type[..endIdx].TrimEnd();
}

public bool Equals(ApiType? other) => other != null && Identity == other.Identity;
public override bool Equals(object? obj) => Equals(obj as ApiType);
public override int GetHashCode() => Type.GetHashCode();
public override int GetHashCode() => Identity.GetHashCode();
}
Loading