Add Roslyn analyzer to build to check nothing has [Serializable] that's not on the approved list

[ViktorHofer]

Relates to changes introduced in https://github.com/dotnet/corefx/issues/19119 Add Roslyn analyzer to build to check nothing has [Serializable] that's not on the approved list

cc @danmosemsft @morganbr

[Vertygo]

Is "approved list" available on repository or it should be created? If we need to create list where we can find it? Where should we put analyzer source code?

[danmoseley]

Hi @Vertygo the list is here: https://docs.microsoft.com/en-us/dotnet/standard/serialization/binary-serialization#binary-serialization-in-net-core

For where, we hvae https://github.com/dotnet/corefx-tools but it seems not very used. There is also https://github.com/dotnet/buildtools. I would say the latter. But we can decide when we have it working.

Would you like me to assign you this project?

[Vertygo]

Yes, I would like to give a shot. Is it ok to hardcode list of approved seriazable classes and later on add it to some kind of config or such?

[ViktorHofer]

You could either hardcode it for now and we change it later when it's working or you already put the list of types (I can send you the actual one, this is only the public one for the docs without internal types) into a textfile and accept is as an argument in the analyzer. We will most likely include the type file in corefx and then invoke your analyzer with a msbuild target.

[ViktorHofer]

Please use this file: https://gist.github.com/ViktorHofer/2da00443c8284c93f0aea5402c11238e

It contains all serializables types with the reflectable layout.

[Vertygo]

Great, I will go straight for input file instead of hardcoding it.

[ViktorHofer]

And an example of what I meant with msbuild target which then invokes something from the build tools: https://github.com/dotnet/corefx/blob/master/src/shims/ApiCompat.proj

In your case you don't need most of it. Just one target with an Exec inside, and properties for the location of the serializables.txt file. These two files would go into corefx and the analyzer into buildtools.

For the right location in corefx I would ask @weshaggard.

[ViktorHofer]

Thanks for your help 👍

[weshaggard]

Assuming this anaylyzer would run on each library independently I would expected any exception list to be in the library directory. Similar to what we do for our PInvoke checker lists.

[ViktorHofer]

@Vertygo if you need any more help just ask here or email me.

[Vertygo]

@weshaggard PInvoke analyzer is really great example. I suppose that I should add new analyzer in same namespace as PInvoke. Should we open up new ticket on buildtools repo for this?

[weshaggard]

I suppose that I should add new analyzer in same namespace as PInvoke.

Yes

Should we open up new ticket on buildtools repo for this?

No real need we can use this issue to track it.

[ngbrown]

@Vertygo still working on this? Do you have a preview version that can be tested?

[ViktorHofer]

Unassigning and putting up-for-grabs label on it as we haven't heart back from @Vertygo for a while.

Since I created the issue we expanded the serialization set and now support exception types again. ping me for an updated list.

[joperezr]

This doesn't seem to belong on System.Runtime area. Moving it to infrastructure so that it gets triaged there in case we want to add any analyzer as part of our build infra to cover this.

[ghost]

Tagging subscribers to this area: @safern, @viktorhofer Notify danmosemsft if you want to be subscribed.

[safern]

@ericstj @ViktorHofer is this something we still need/want? It seems we have lived without it for 3 years already.

[ViktorHofer]

Closing as we don't plan to add the SerializableAttribute to any new types and changes on existing types should be easy to spot in PR reviews.

[ericstj]

cc @GrabYourPitchforks