Brightspace / D2L.CodeStyle

Annotations and analysis tools for D2L C# code style
Apache License 2.0
10 stars 22 forks source link

Add SerializationFrameworkAnalyzer #922

Closed Matthew-nop closed 12 months ago

Matthew-nop commented 1 year ago

This analyzer restricts the use of these interfaces and their implementations:

Invocation, property, and field references are only permitted in classes annotated with the [SerializationFramework] attribute

There is one small problem/catch to this analyzer though - implementing types may still be referenced through another safe interface. Given some safe interface a and a dangerous interface d, a serialiser s such that s : a, d may invoke dangerous behaviour through a. Although, I don't think this can be solved without a noticeable/significant performance hit.

I built the LMS with this analyzer to roughly gauge the performance. It seems to be a bit worse than the OBSL analyser which was the basis for a lot of this code. I figure this is a because for each type t we sometimes check t.AllInterfaces.Any( dangerousInterface.Contains ) in addition to the dangerousInterfaces.Contains( t ) that the OBSL analyser checks image

Before merging, the annotations should be added in the LMS:

ref: https://github.com/Brightspace/lms-dotnet-migration-issues/issues/135

Matthew-nop commented 1 year ago

Putting this on hold for now - investigating potentially adding a safe alternative interface to ISerializationWriter that serialisers can call

Matthew-nop commented 12 months ago

Closing this for now. This may be used as a plan B, but we've opted to go with https://github.com/Brightspace/lms/pull/42621 to enforce pinned type safety within serialization internals.