diff --git a/src/Microsoft.AspNetCore.BuildTools.ApiCheck/ApiListingComparer.cs b/src/Microsoft.AspNetCore.BuildTools.ApiCheck/ApiListingComparer.cs
index bc215093e..813404de3 100644
--- a/src/Microsoft.AspNetCore.BuildTools.ApiCheck/ApiListingComparer.cs
+++ b/src/Microsoft.AspNetCore.BuildTools.ApiCheck/ApiListingComparer.cs
@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
@@ -68,9 +68,20 @@ private void CompareMembers(TypeDescriptor type, TypeDescriptor newType, List
0)
+ if (newMembers.Count > 0)
{
- breakingChanges.AddRange(newMembers.Select(member => new BreakingChange(newType.Id, member.Id, ChangeKind.Addition)));
+ if (type.Kind == TypeKind.Interface)
+ {
+ breakingChanges.AddRange(newMembers.Select(member => new BreakingChange(newType.Id, member.Id, ChangeKind.Addition)));
+ }
+ else
+ {
+ var disallowedNewMembers = newMembers.Where(member => member.Abstract).ToArray();
+ if (disallowedNewMembers.Length > 0)
+ {
+ breakingChanges.AddRange(disallowedNewMembers.Select(member => new BreakingChange(newType.Id, member.Id, ChangeKind.Addition)));
+ }
+ }
}
}
diff --git a/src/Microsoft.AspNetCore.BuildTools.ApiCheck/ApiListingGenerator.cs b/src/Microsoft.AspNetCore.BuildTools.ApiCheck/ApiListingGenerator.cs
index 95ea46890..6077bc9df 100644
--- a/src/Microsoft.AspNetCore.BuildTools.ApiCheck/ApiListingGenerator.cs
+++ b/src/Microsoft.AspNetCore.BuildTools.ApiCheck/ApiListingGenerator.cs
@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
@@ -220,6 +220,7 @@ public static MemberDescriptor GenerateMemberApiListing(TypeInfo type, MemberInf
}
return constructorDescriptor;
+
case MemberTypes.Method:
var method = (MethodInfo)member;
if (!method.IsPublic && !method.IsFamily && !method.IsFamilyOrAssembly)
@@ -274,6 +275,7 @@ public static MemberDescriptor GenerateMemberApiListing(TypeInfo type, MemberInf
methodDescriptor.ReturnType = TypeDescriptor.GetTypeNameFor(method.ReturnType.GetTypeInfo());
return methodDescriptor;
+
case MemberTypes.Field:
var field = (FieldInfo)member;
if (!field.IsPublic && !field.IsFamily && !field.IsFamilyOrAssembly)
@@ -318,12 +320,14 @@ public static MemberDescriptor GenerateMemberApiListing(TypeInfo type, MemberInf
// All these cases are covered by the methods they implicitly define on the class
// (Properties and Events) and when we enumerate all the types in an assembly (Nested types).
return null;
+
case MemberTypes.TypeInfo:
- // There should not be any member passsed into this method that is not a top level type.
+ // There should not be any member passed into this method that is not a top level type.
case MemberTypes.Custom:
- // We don't know about custom member types, so better throw if we find something we don't understand.
+ // We don't know about custom member types, so better throw if we find something we don't understand.
case MemberTypes.All:
throw new InvalidOperationException($"'{type.MemberType}' [{member}] is not supported.");
+
default:
return null;
}
@@ -457,4 +461,4 @@ rawDefaultValue is float ||
throw new InvalidOperationException("Unsupported default value type");
}
}
-}
\ No newline at end of file
+}
diff --git a/test/ApiCheck.Test/ApiListingComparerTests.cs b/test/ApiCheck.Test/ApiListingComparerTests.cs
index db501ab98..49fe7dc8e 100644
--- a/test/ApiCheck.Test/ApiListingComparerTests.cs
+++ b/test/ApiCheck.Test/ApiListingComparerTests.cs
@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
@@ -17,11 +17,10 @@ public class ApiListingComparerTests
public Assembly V1Assembly => typeof(ApiCheckApiListingV1).GetTypeInfo().Assembly;
public Assembly V2Assembly => typeof(ApiCheckApiListingV2).GetTypeInfo().Assembly;
- public IEnumerable> TestFilters
- => new Func []
- {
- ti => (ti as TypeInfo)?.Namespace?.StartsWith("ComparisonScenarios") == false
- };
+ public IEnumerable> TestFilters => new Func []
+ {
+ ti => (ti as TypeInfo)?.Namespace?.StartsWith("ComparisonScenarios") == false
+ };
[Fact]
public void Compare_Detects_ChangesInTypeVisibility_as_removal()
@@ -80,6 +79,68 @@ public void Compare_Detects_TypeGenericAritybreakingChanges_as_removal()
Assert.Contains(expected, breakingChanges);
}
+ [Theory]
+ [InlineData("public class ComparisonScenarios.ClassToRemoveFieldsFrom")]
+ [InlineData("public struct ComparisonScenarios.StructToRemoveFieldsFrom")]
+ public void Compare_DetectsAllFieldRemovals(string typeToCheck)
+ {
+ // Arrange
+ var v1ApiListing = CreateApiListingDocument(V1Assembly);
+ var v2ApiListing = CreateApiListingDocument(V2Assembly);
+ var comparer = new ApiListingComparer(v1ApiListing, v2ApiListing);
+
+ // Oops. The NewInternalProperty addition is a breaking change; makes it impossible to subclass type in
+ // another assembly.
+ var expected = new List
+ {
+ // Changing a const's value doesn't cause a binary incompatibility but often causes problems.
+ new BreakingChange(
+ typeToCheck,
+ "public const System.Int32 ConstToChangeValue = 1",
+ ChangeKind.Removal),
+ // Removing a const doesn't cause a binary incompatibilty but often causes problems.
+ new BreakingChange(
+ typeToCheck,
+ "public const System.Int32 ConstToMakeField = 2",
+ ChangeKind.Removal),
+ // Oops. Making a field writable is not technically a breaking change.
+ new BreakingChange(
+ typeToCheck,
+ "public readonly System.Int32 FieldToMakeWritable",
+ ChangeKind.Removal),
+ new BreakingChange(
+ typeToCheck,
+ "public static readonly System.Int32 StaticFieldToMakeConst",
+ ChangeKind.Removal),
+ // Oops. Making a field writable is not technically a breaking change.
+ new BreakingChange(
+ typeToCheck,
+ "public static readonly System.Int32 StaticFieldToMakeWritable",
+ ChangeKind.Removal),
+ new BreakingChange(
+ typeToCheck,
+ "public static System.Int32 StaticFieldToMakeReadonly",
+ ChangeKind.Removal),
+ new BreakingChange(
+ typeToCheck,
+ "public System.Int32 FieldToMakeReadonly",
+ ChangeKind.Removal),
+ new BreakingChange(
+ typeToCheck,
+ "public System.Int32 FieldToRemove",
+ ChangeKind.Removal),
+ };
+
+ // Act
+ var breakingChanges = comparer.GetDifferences();
+
+ // Assert
+ var breakingChanginesInType = breakingChanges
+ .Where(change => string.Equals(change.TypeId, typeToCheck, StringComparison.Ordinal))
+ .OrderBy(change => change.MemberId);
+ Assert.Equal(expected, breakingChanginesInType);
+ }
+
[Fact]
public void Compare_Detects_NamespacebreakingChanges_as_removal()
{
@@ -191,6 +252,56 @@ public void Compare_DoesNotFailForTypeAddingAnInterface()
bc => bc.TypeId == "public ComparisonScenarios.TypeWithExtraInterface"));
}
+ [Fact]
+ public void Compare_DetectsAbstractMethodAdditions()
+ {
+ // Arrange
+ var v1ApiListing = CreateApiListingDocument(V1Assembly);
+ var v2ApiListing = CreateApiListingDocument(V2Assembly);
+ var comparer = new ApiListingComparer(v1ApiListing, v2ApiListing);
+ var typeToCheck = "public abstract class ComparisonScenarios.AbstractClassToAddMethodsTo";
+
+ // Act
+ var breakingChanges = comparer.GetDifferences();
+
+ // Assert
+ var breakingChangesInType = breakingChanges
+ .Where(change => string.Equals(change.TypeId, typeToCheck, StringComparison.Ordinal));
+ var breakingChange = Assert.Single(breakingChangesInType);
+ Assert.Equal(ChangeKind.Addition, breakingChange.Kind);
+ Assert.Equal("public abstract System.Void NewAbstractMethod()", breakingChange.MemberId);
+ }
+
+ [Fact]
+ public void Compare_DetectsAbstractPropertyAdditions()
+ {
+ // Arrange
+ var v1ApiListing = CreateApiListingDocument(V1Assembly);
+ var v2ApiListing = CreateApiListingDocument(V2Assembly);
+ var comparer = new ApiListingComparer(v1ApiListing, v2ApiListing);
+ var typeToCheck = "public abstract class ComparisonScenarios.AbstractClassToAddPropertiesTo";
+ var expected = new List
+ {
+ new BreakingChange(
+ typeToCheck,
+ "public abstract System.Int32 get_NewAbstractProperty()",
+ ChangeKind.Addition),
+ new BreakingChange(
+ typeToCheck,
+ "public abstract System.Void set_PropertyToAddSetterTo(System.Int32 value)",
+ ChangeKind.Addition),
+ };
+
+ // Act
+ var breakingChanges = comparer.GetDifferences();
+
+ // Assert
+ var breakingChangesInType = breakingChanges
+ .Where(change => string.Equals(change.TypeId, typeToCheck, StringComparison.Ordinal))
+ .OrderBy(change => change.MemberId);
+ Assert.Equal(expected, breakingChangesInType);
+ }
+
[Fact]
public void Compare_DetectsNewMembersBeingAddedToAnInterface_as_addition()
{
@@ -203,9 +314,8 @@ public void Compare_DetectsNewMembersBeingAddedToAnInterface_as_addition()
var breakingChanges = comparer.GetDifferences();
// Assert
- var interfaceBreakingChanges = breakingChanges.Where(
- b => b.TypeId ==
- "public interface ComparisonScenarios.IInterfaceToAddMembersTo")
+ var interfaceBreakingChanges = breakingChanges
+ .Where( b => b.TypeId == "public interface ComparisonScenarios.IInterfaceToAddMembersTo")
.ToList();
Assert.Single(interfaceBreakingChanges,
b => b.MemberId == "System.Int32 get_NewMember()" && b.Kind == ChangeKind.Addition);
@@ -222,23 +332,24 @@ public void Compare_AllowsExclusionsOnNewInterfaceMembers()
var comparer = new ApiListingComparer(v1ApiListing, v2ApiListing);
var knownBreakingChanges = new List
- {
- new BreakingChange(
- "public interface ComparisonScenarios.IInterfaceToAddMembersTo",
- "System.Int32 get_NewMember()",
- ChangeKind.Addition),
- new BreakingChange(
- "public interface ComparisonScenarios.IInterfaceToAddMembersTo",
- "System.Void set_NewMember(System.Int32 value)",
- ChangeKind.Addition)
- };
+ {
+ new BreakingChange(
+ "public interface ComparisonScenarios.IInterfaceToAddMembersTo",
+ "System.Int32 get_NewMember()",
+ ChangeKind.Addition),
+ new BreakingChange(
+ "public interface ComparisonScenarios.IInterfaceToAddMembersTo",
+ "System.Void set_NewMember(System.Int32 value)",
+ ChangeKind.Addition)
+ };
// Act
var breakingChanges = comparer.GetDifferences().Except(knownBreakingChanges);
// Assert
- Assert.Null(breakingChanges.FirstOrDefault(
- bc => bc.TypeId == "public interface ComparisonScenarios.IInterfaceToAddMembersTo"));
+ Assert.DoesNotContain(
+ breakingChanges,
+ bc => bc.TypeId == "public interface ComparisonScenarios.IInterfaceToAddMembersTo");
}
[Fact]
@@ -253,9 +364,8 @@ public void Compare_DetectsNewMembersInThePresenceOfRenamedAndRemovedMembers()
var breakingChanges = comparer.GetDifferences();
// Assert
- var interfaceBreakingChanges = breakingChanges.Where(
- b => b.TypeId ==
- "public interface ComparisonScenarios.IInterfaceWithMembersThatWillGetRenamedRemovedAndAdded")
+ var interfaceBreakingChanges = breakingChanges
+ .Where( b => b.TypeId == "public interface ComparisonScenarios.IInterfaceWithMembersThatWillGetRenamedRemovedAndAdded")
.ToList();
Assert.Single(interfaceBreakingChanges,
b => b.MemberId == "System.Void MemberToBeRenamed()" && b.Kind == ChangeKind.Removal);
@@ -279,9 +389,8 @@ public void Compare_DetectsNewMembersInThePresenceOfTheSameNumberOfRemovedAndAdd
var breakingChanges = comparer.GetDifferences();
// Assert
- var interfaceBreakingChanges = breakingChanges.Where(
- b => b.TypeId ==
- "public interface ComparisonScenarios.IInterfaceWithSameNumberOfRemovedAndAddedMembers")
+ var interfaceBreakingChanges = breakingChanges
+ .Where(b => b.TypeId == "public interface ComparisonScenarios.IInterfaceWithSameNumberOfRemovedAndAddedMembers")
.ToList();
Assert.Single(interfaceBreakingChanges,
b => b.MemberId == "System.Void FirstMemberToRemove()" && b.Kind == ChangeKind.Removal);
@@ -297,11 +406,9 @@ public void Compare_DetectsNewMembersInThePresenceOfTheSameNumberOfRemovedAndAdd
b => b.MemberId == "System.Void ThirdAddedMember()" && b.Kind == ChangeKind.Addition);
}
- private ApiListing CreateApiListingDocument(Assembly assembly,
- IEnumerable> additionalFilters = null)
+ private ApiListing CreateApiListingDocument(Assembly assembly)
{
- additionalFilters = additionalFilters ?? Enumerable.Empty>();
- var generator = new ApiListingGenerator(assembly, TestFilters.Concat(additionalFilters));
+ var generator = new ApiListingGenerator(assembly, TestFilters);
return generator.GenerateApiListing();
}
diff --git a/test/ApiCheckBaseline.V1/ComparisonScenarios.cs b/test/ApiCheckBaseline.V1/ComparisonScenarios.cs
index 3c087ee79..0f25861ed 100644
--- a/test/ApiCheckBaseline.V1/ComparisonScenarios.cs
+++ b/test/ApiCheckBaseline.V1/ComparisonScenarios.cs
@@ -1,8 +1,6 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
-
-
// V1
namespace ComparisonScenarios
{
@@ -18,6 +16,56 @@ public struct StructToMakeGeneric
{
}
+ public struct StructToRemoveFieldsFrom
+ {
+ public StructToRemoveFieldsFrom(int fieldToIgnore)
+ {
+ FieldToIgnore = 0;
+ FieldToMakeReadonly = 3;
+ FieldToRemove = 4;
+ FieldToMakeWritable = 5;
+ }
+
+ internal int FieldToIgnore;
+
+ public const int ConstToChangeValue = 1;
+
+ public const int ConstToMakeField = 2;
+
+ public int FieldToMakeReadonly;
+
+ public int FieldToRemove;
+
+ public readonly int FieldToMakeWritable;
+
+ public static int StaticFieldToMakeReadonly = 6;
+
+ public static readonly int StaticFieldToMakeConst = 7;
+
+ public static readonly int StaticFieldToMakeWritable = 8;
+ }
+
+ public class ClassToRemoveFieldsFrom
+ {
+ internal int FieldToIgnore = 0;
+
+ public const int ConstToChangeValue = 1;
+
+ public const int ConstToMakeField = 2;
+
+ public int FieldToMakeReadonly = 3;
+
+ public int FieldToRemove = 4;
+
+ public readonly int FieldToMakeWritable = 5;
+
+ public static int StaticFieldToMakeReadonly = 6;
+
+ public static readonly int StaticFieldToMakeConst = 7;
+
+ public static readonly int StaticFieldToMakeWritable = 8;
+ }
+
public class ClassToChangeNamespaces
{
}
@@ -50,6 +98,15 @@ public class TypeWithExtraInterface
{
}
+ public abstract class AbstractClassToAddMethodsTo
+ {
+ }
+
+ public abstract class AbstractClassToAddPropertiesTo
+ {
+ public abstract int PropertyToAddSetterTo { get; }
+ }
+
public interface IInterfaceToAddMembersTo
{
bool ExistingMember { get; set; }
diff --git a/test/ApiCheckBaseline.V2/ComparisonScenarios.cs b/test/ApiCheckBaseline.V2/ComparisonScenarios.cs
index 6693debcc..2cbb6c98e 100644
--- a/test/ApiCheckBaseline.V2/ComparisonScenarios.cs
+++ b/test/ApiCheckBaseline.V2/ComparisonScenarios.cs
@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System.Collections.Generic;
@@ -18,6 +18,46 @@ public struct StructToMakeGeneric
{
}
+ public struct StructToRemoveFieldsFrom
+ {
+ public StructToRemoveFieldsFrom(int fieldToIgnore)
+ {
+ FieldToMakeReadonly = 3;
+ FieldToMakeWritable = 5;
+ }
+
+ public const int ConstToChangeValue = 0;
+
+ public static readonly int ConstToMakeField = 2;
+
+ public readonly int FieldToMakeReadonly;
+
+ public int FieldToMakeWritable;
+
+ public static readonly int StaticFieldToMakeReadonly = 6;
+
+ public const int StaticFieldToMakeConst = 7;
+
+ public static int StaticFieldToMakeWritable = 8;
+ }
+
+ public class ClassToRemoveFieldsFrom
+ {
+ public const int ConstToChangeValue = 0;
+
+ public static readonly int ConstToMakeField = 2;
+
+ public readonly int FieldToMakeReadonly = 3;
+
+ public int FieldToMakeWritable = 5;
+
+ public static readonly int StaticFieldToMakeReadonly = 6;
+
+ public const int StaticFieldToMakeConst = 7;
+
+ public static int StaticFieldToMakeWritable = 8;
+ }
+
public class ClassToNestContainer
{
public class ClassToNest
@@ -52,6 +92,30 @@ public interface IExtraInterface
int Property { get; set; }
}
+ public abstract class AbstractClassToAddMethodsTo
+ {
+ public abstract void NewAbstractMethod();
+
+ public virtual void NewVirtualMethod() { }
+
+ public void NewMethod() { }
+
+ internal abstract void NewInternalMethod();
+ }
+
+ public abstract class AbstractClassToAddPropertiesTo
+ {
+ public abstract int NewAbstractProperty { get; }
+
+ public abstract int PropertyToAddSetterTo { get; set; }
+
+ public int NewProperty => 0;
+
+ public virtual int NewVirtualProperty => 0;
+
+ internal abstract int NewInternalProperty { get; }
+ }
+
public interface IInterfaceToAddMembersTo
{
bool ExistingMember { get; set; }
@@ -79,4 +143,4 @@ namespace ComparisonScenarios.ChangedNamespace
public class ClassToChangeNamespaces
{
}
-}
\ No newline at end of file
+}