Skip to content

Commit dca2620

Browse files
smitpatelsmitpatel
committed
ApiCheck: Remove concept of modification
Print out breaking changes to be added as exceptions Change format of exception files Use word breaking changes instead of exceptions/exclusions Upgrade to net46
1 parent b0ca852 commit dca2620

File tree

11 files changed

+373
-255
lines changed

11 files changed

+373
-255
lines changed

‎src/Internal.AspNetCore.Sdk/build/ApiCheck.targets‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
<_ApiListingFileSuffix Condition=" '$(TargetFrameworkIdentifier)' == '.NETFramework' ">net45.json</_ApiListingFileSuffix>
99
<_ApiListingFileSuffix Condition=" '$(_ApiListingFileSuffix)' == '' ">netcore.json</_ApiListingFileSuffix>
1010
<_ApiListingFilePath>$(MSBuildProjectDirectory)\baseline.$(_ApiListingFileSuffix)</_ApiListingFilePath>
11-
<_ApiExclusionsFilePath>$(MSBuildProjectDirectory)\exceptions.$(_ApiListingFileSuffix)</_ApiExclusionsFilePath>
11+
<_ApiExclusionsFilePath>$(MSBuildProjectDirectory)\breakingchanges.$(_ApiListingFileSuffix)</_ApiExclusionsFilePath>
1212
<_ApiListingFile Condition=" Exists('$(_ApiListingFilePath)') ">$(_ApiListingFilePath)</_ApiListingFile>
1313
<_ApiExclusionsFile Condition=" Exists('$(_ApiExclusionsFilePath)') ">$(_ApiExclusionsFilePath)</_ApiExclusionsFile>
1414
</PropertyGroup>

‎src/Microsoft.AspNetCore.BuildTools.ApiCheck/ApiChangeExclusion.cs‎

Lines changed: 0 additions & 27 deletions
This file was deleted.

‎src/Microsoft.AspNetCore.BuildTools.ApiCheck/ApiComparisonResult.cs‎

Lines changed: 0 additions & 19 deletions
This file was deleted.

‎src/Microsoft.AspNetCore.BuildTools.ApiCheck/ApiListingComparer.cs‎

Lines changed: 47 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -13,128 +13,108 @@ public class ApiListingComparer
1313
{
1414
private readonly ApiListing _newApiListing;
1515
private readonly ApiListing _oldApiListing;
16-
private readonly IList<ApiChangeExclusion> _exclusions;
1716

1817
public ApiListingComparer(
1918
ApiListing oldApiListing,
20-
ApiListing newApiListing,
21-
IList<ApiChangeExclusion> exclusions = null)
19+
ApiListing newApiListing)
2220
{
2321
_oldApiListing = oldApiListing;
2422
_newApiListing = newApiListing;
25-
_exclusions = exclusions ?? new List<ApiChangeExclusion>();
2623
}
2724

28-
public ApiComparisonResult GetDifferences()
25+
public IList<BreakingChange> GetDifferences()
2926
{
30-
var exclusions = _exclusions.ToList();
31-
3227
var breakingChanges = new List<BreakingChange>();
28+
var newTypes = _newApiListing.Types;
29+
3330
foreach (var type in _oldApiListing.Types)
3431
{
3532
var newType = _newApiListing.FindType(type.Name);
36-
if (newType == null || !string.Equals(type.Id, newType.Id, StringComparison.Ordinal))
33+
if (newType == null)
3734
{
38-
var isAcceptable = newType != null && IsAcceptableTypeChange(type, newType);
39-
if (!isAcceptable)
35+
breakingChanges.Add(new BreakingChange(type.Id, memberId: null, kind: ChangeKind.Removal));
36+
}
37+
else
38+
{
39+
newTypes.Remove(newType);
40+
41+
if (!string.Equals(type.Id, newType.Id, StringComparison.Ordinal)
42+
&& !IsAcceptableTypeChange(type, newType))
4043
{
41-
var typeChange = FilterExclusions(type, member: null, exclusions: exclusions);
42-
if (typeChange != null)
43-
{
44-
breakingChanges.Add(typeChange);
45-
}
44+
breakingChanges.Add(new BreakingChange(type.Id, memberId: null, kind: ChangeKind.Removal));
45+
continue;
4646
}
4747

48-
}
49-
50-
if (newType != null)
51-
{
52-
CompareMembers(type, newType, exclusions, breakingChanges);
48+
CompareMembers(type, newType, breakingChanges);
5349
}
5450
}
5551

56-
return new ApiComparisonResult(breakingChanges, exclusions);
52+
return breakingChanges;
5753
}
5854

59-
private void CompareMembers(TypeDescriptor type, TypeDescriptor newType, List<ApiChangeExclusion> exclusions, List<BreakingChange> breakingChanges)
55+
private void CompareMembers(TypeDescriptor type, TypeDescriptor newType, List<BreakingChange> breakingChanges)
6056
{
61-
var removedOrChanged = 0;
57+
var newMembers = newType.Members.ToList();
58+
6259
foreach (var member in type.Members)
6360
{
64-
var newMember = newType.FindMember(member.Id);
65-
var isAcceptable = IsAcceptableMemberChange(_newApiListing, newType, member);
66-
if (isAcceptable)
61+
if (IsAcceptableMemberChange(newType, member, out var newMember))
6762
{
68-
if (newMember == null)
69-
{
70-
removedOrChanged++;
71-
}
72-
73-
continue;
63+
newMembers.Remove(newMember);
7464
}
75-
76-
if (newMember == null)
65+
else
7766
{
78-
removedOrChanged++;
79-
var memberChange = FilterExclusions(type, member, exclusions);
80-
if (memberChange != null)
81-
{
82-
breakingChanges.Add(memberChange);
83-
}
67+
breakingChanges.Add(new BreakingChange(type.Id, member.Id, ChangeKind.Removal));
8468
}
8569
}
8670

87-
if (type.Kind == TypeKind.Interface && type.Members.Count - removedOrChanged < newType.Members.Count)
71+
if (type.Kind == TypeKind.Interface && newMembers.Count > 0)
8872
{
89-
var members = newType.Members.ToList();
90-
foreach (var member in newType.Members)
91-
{
92-
var change = FilterExclusions(type, null, exclusions);
93-
if (change == null)
94-
{
95-
members.Remove(member);
96-
}
97-
}
98-
99-
if (type.Members.Count - removedOrChanged < members.Count)
100-
{
101-
breakingChanges.Add(new BreakingChange(type, "New members were added to the following interface"));
102-
}
73+
breakingChanges.AddRange(newMembers.Select(member => new BreakingChange(newType.Id, member.Id, ChangeKind.Addition)));
10374
}
10475
}
10576

106-
private bool IsAcceptableMemberChange(ApiListing newApiListing, TypeDescriptor newType, MemberDescriptor member)
77+
private bool IsAcceptableMemberChange(TypeDescriptor newType, MemberDescriptor member, out MemberDescriptor newMember)
10778
{
10879
var acceptable = false;
80+
newMember = null;
10981
var candidate = newType;
11082
while (candidate != null && !acceptable)
11183
{
112-
if (candidate.Members.Any(m => m.Id == member.Id))
84+
var matchingMembers = candidate.Members.Where(m => m.Id == member.Id).ToList();
85+
86+
if (matchingMembers.Count == 1)
11387
{
88+
newMember = matchingMembers.Single();
11489
acceptable = true;
11590
}
11691
else if (member.Kind == MemberKind.Method)
11792
{
118-
var newMember = newType.Members.FirstOrDefault(m => SameSignature(member, m));
119-
if (newMember != null)
93+
var matchingMember = newType.Members.FirstOrDefault(m => SameSignature(member, m));
94+
if (matchingMember != null)
12095
{
121-
acceptable = (!member.Sealed ? !newMember.Sealed : true) &&
122-
(member.Virtual ? newMember.Virtual || newMember.Override : true) &&
123-
(member.Static == newMember.Static) &&
124-
(!member.Abstract ? !newMember.Abstract : true);
96+
acceptable = (member.Sealed || !matchingMember.Sealed)
97+
&& (!member.Virtual || matchingMember.Virtual || matchingMember.Override)
98+
&& member.Static == matchingMember.Static
99+
&& (member.Abstract || !matchingMember.Abstract);
100+
101+
if (acceptable)
102+
{
103+
newMember = matchingMember;
104+
}
125105
}
126106
}
127107

128-
candidate = candidate.BaseType == null ? null : FindOrGenerateDescriptorForBaseType(newApiListing, candidate);
108+
candidate = candidate.BaseType == null ? null : FindOrGenerateDescriptorForBaseType(candidate);
129109
}
130110

131111
return acceptable;
132112
}
133113

134-
private static TypeDescriptor FindOrGenerateDescriptorForBaseType(ApiListing newApiListing, TypeDescriptor candidate)
114+
private TypeDescriptor FindOrGenerateDescriptorForBaseType(TypeDescriptor candidate)
135115
{
136-
return newApiListing.FindType(candidate.BaseType) ??
137-
ApiListingGenerator.GenerateTypeDescriptor(candidate.Source.BaseType.GetTypeInfo(), newApiListing.SourceFilters);
116+
return _newApiListing.FindType(candidate.BaseType) ??
117+
ApiListingGenerator.GenerateTypeDescriptor(candidate.Source.BaseType.GetTypeInfo(), _newApiListing.SourceFilters);
138118
}
139119

140120
private bool SameSignature(MemberDescriptor original, MemberDescriptor candidate)
@@ -299,25 +279,5 @@ private bool HasCompatibleVisibility(TypeDescriptor oldType, TypeDescriptor newT
299279
throw new InvalidOperationException("Unrecognized visibility");
300280
}
301281
}
302-
303-
private BreakingChange FilterExclusions(TypeDescriptor type, MemberDescriptor member, List<ApiChangeExclusion> exclusions)
304-
{
305-
var exclusion = exclusions
306-
.FirstOrDefault(e => e.IsExclusionFor(type.Id, member?.Id));
307-
308-
if (exclusion != null)
309-
{
310-
var element = _newApiListing.FindElement(exclusion.NewTypeId, exclusion.NewMemberId);
311-
if (exclusion.Kind == ChangeKind.Removal && element == null ||
312-
exclusion.Kind == ChangeKind.Modification && element != null ||
313-
exclusion.Kind == ChangeKind.Addition && element != null)
314-
{
315-
exclusions.Remove(exclusion);
316-
return null;
317-
}
318-
}
319-
320-
return member == null ? new BreakingChange(type) : new BreakingChange(member, type.Id);
321-
}
322282
}
323283
}

‎src/Microsoft.AspNetCore.BuildTools.ApiCheck/BreakingChange.cs‎

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,51 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4-
using ApiCheck.Description;
4+
using Microsoft.DotNet.PlatformAbstractions;
55

66
namespace ApiCheck
77
{
88
public class BreakingChange
99
{
10-
public BreakingChange(ApiElement oldItem, string context = null)
10+
public BreakingChange(string typeId, string memberId, ChangeKind kind)
1111
{
12-
Context = context;
13-
Item = oldItem;
12+
TypeId = typeId;
13+
MemberId = memberId;
14+
Kind = kind;
1415
}
15-
public string Context { get; }
1616

17-
public ApiElement Item { get; }
17+
public string TypeId { get; }
18+
public string MemberId { get; }
19+
public ChangeKind Kind { get; }
1820

19-
public override string ToString() => Context == null ? Item.Id : $"{Context} => {Item.Id}";
21+
public override bool Equals(object obj)
22+
{
23+
if (ReferenceEquals(null, obj))
24+
{
25+
return false;
26+
}
27+
28+
if (ReferenceEquals(this, obj))
29+
{
30+
return true;
31+
}
32+
33+
return obj.GetType() == GetType() && Equals((BreakingChange)obj);
34+
}
35+
36+
private bool Equals(BreakingChange other)
37+
{
38+
return string.Equals(TypeId, other.TypeId) && string.Equals(MemberId, other.MemberId) && Kind == other.Kind;
39+
}
40+
41+
public override int GetHashCode()
42+
{
43+
var hashCodeCombiner = HashCodeCombiner.Start();
44+
hashCodeCombiner.Add(TypeId);
45+
hashCodeCombiner.Add(MemberId);
46+
hashCodeCombiner.Add(Kind);
47+
48+
return hashCodeCombiner.CombinedHash;
49+
}
2050
}
2151
}

‎src/Microsoft.AspNetCore.BuildTools.ApiCheck/ChangeKind.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ namespace ApiCheck
1010
public enum ChangeKind
1111
{
1212
Removal,
13-
Modification,
1413
Addition,
1514
}
1615
}

‎src/Microsoft.AspNetCore.BuildTools.ApiCheck/Loader/AssemblyLoader.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4-
#if NET452
4+
#if NET46
55
using System.IO;
66
#endif
77
using System.Reflection;

‎src/Microsoft.AspNetCore.BuildTools.ApiCheck/Microsoft.AspNetCore.BuildTools.ApiCheck.csproj‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@
22
<Import Project="..\..\build\common.props" />
33

44
<PropertyGroup>
5-
<VersionPrefix>1.0.2</VersionPrefix>
5+
<VersionPrefix>1.0.3</VersionPrefix>
66

77
<!--
88
The netstandard1.0 TFM doesn't actually compile. It's just there so Internal.AspNetCore.Sdk can take a dependency
99
on this project.
1010
-->
11-
<TargetFrameworks>netcoreapp1.1;net452;netstandard1.0</TargetFrameworks>
11+
<TargetFrameworks>netcoreapp1.1;net46;netstandard1.0</TargetFrameworks>
1212
</PropertyGroup>
1313

1414
<PropertyGroup Condition=" '$(TargetFramework)' == 'netstandard1.0' ">

0 commit comments

Comments
 (0)