dotnet / arcade

Tools that provide common build infrastructure for multiple .NET Foundation projects.
MIT License
672 stars 347 forks source link

ApiCompat: enum values are converted to integers #6652

Open scottdallamura opened 3 years ago

scottdallamura commented 3 years ago

it's entirely possible i'm doing something wrong so please correct me if so :)

i'm integrating ApiCheck with one of our projects. we generate our clients via NSwag. ApiCheck is getting mad because the generated JsonPropertyAttributes use values like Required.Default ... but in the reference (an older version of my library) these enums are (according to ApiCheck) converted to their integer values, e.g. "0".

C:\Users\scdallam\.nuget\packages\microsoft.dotnet.apicompat\6.0.0-beta.20604.3\build\Microsoft.DotNet.ApiCompat.targets(82,5): error : CannotChangeAttribute : Attribute 'Newtonsoft.Json.JsonPropertyAttribute' on 'MyNamespace.MyClass.MyProperty' changed from '[JsonPropertyAttribute("myProperty", Required=0, NullValueHandling=1)]' in the contract to '[JsonPropertyAttribute("myProperty", Required=Required.Default, NullValueHandling=NullValueHandling.Ignore)]' in the implementation. [C:\src\MyProject\MyProject.csproj]

looking at both assemblies with ilspy i see

[JsonProperty("myProperty", Required = Required.Default, NullValueHandling = NullValueHandling.Ignore)]

... but maybe is ApiCheck having trouble loading up newtonsoft.json or something? i don't want to suppress these attributes as they're kinda relevant to compatibility...

scottdallamura commented 3 years ago

more info: looking at the apicompat.rsp i see it's including C:\Users\scdallam\.nuget\packages\newtonsoft.json\12.0.3\lib\netstandard2.0\ in the --impl-dirs (i guess because it's part of _DependencyDirectories which makes sense i think). taking that out and running from the command line gives me the results i expect. both the contract and the implementation reference 12.0.3, so i don't expect any difference here...

also, adding the nuget lib folder to --contract-depends also gives the results i expect. but i don't know what the correct solution is :)

scottdallamura commented 3 years ago

yeah i was on the right track (well, a track that didn't dead-end anyway). so i got it working with

<PackageReference Include="Newtonsoft.Json" Version="12.0.3" GeneratePathProperty="true" />

and adding DependencyPaths="$(PkgNewtonsoft_Json)\lib\netstandard2.0\" to my ResolvedMatchingContract. i'm unblocked, but this feels like it shouldn't be necessary... and what happens if a new version of my library uses a newer version of newtonsoft.json - then the DependencyPaths would be inaccurate and i suppose it would potentially miss compat breaks. i see some PRs mentioning some stuff about netstandard dependencies (is my newtonsoft reference wrong?) ... could my project be arranged differently to make this work more smoothly?

nils-a commented 2 years ago

I'm seeing the same problem here, but without any references. My Test is the following:

using System;

namespace ApiCompatExample
{
    [My]
    public class MyClass
    {
        public void SomethingIsRequiredInTheClass()
        {
        }

        /*
        public void AddThisAsANonBreakingChange()
        {
        }
        */
    }

    [AttributeUsage(AttributeTargets.Class)]
    public class MyAttribute : Attribute
    {
    }
}

The project file is quite "empty":

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>netstandard2.0</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <ResolvedMatchingContract Include=".\LastMajorVersionBinary\ApiCompatExample.dll" />
  </ItemGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.DotNet.ApiCompat" Version="5.0.0-beta.19557.20" PrivateAssets="All" />
  </ItemGroup>

</Project>

Running the build multiple times does not yield a problem. However, after adding a non-breaking change the following "breaking change" is detected:

C:\Users\***\build\Microsoft.DotNet.ApiCompat.targets(82,5): error : CannotChangeAttribute : Attribute 'System.AttributeUsageAttribute' on 'ApiCompatExample.MyAttribute' changed from '[AttributeUsageAttribute(4)]' in the contract to '[AttributeUsageAttribute(AttributeTargets.Class)]' in the implementation. [C:\_dev\ApiCompatExample\ApiCompatExample.csproj]
C:\Users\***\build\Microsoft.DotNet.ApiCompat.targets(96,5): error : ApiCompat failed for 'C:\_dev\ApiCompatExample\bin\Debug\netstandard2.0\ApiCompatExample.dll' [C:\_dev\ApiCompatExample\ApiCompatExample.csproj]
    0 Warning(s)
    2 Error(s)

I prepared a test repo in https://github.com/nils-a/ApiCompatExample