Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed and improved serializability analyzers #1244

Merged
merged 10 commits into from
Dec 22, 2021

Conversation

acizmarik
Copy link
Member

@acizmarik acizmarik commented Dec 20, 2021

This PR introduces a couple of fixes and improvements to serializability analyzers.

  • Fixed bug: nullables were sometimes incorrectly marked as not serializable.
  • Fixed bug: certain supported types were incorrectly marked as not serializable (GridViewDataSet<T>, UploadedFilesCollection and BusinessPackDataSet<T>). They do not follow our general rules, however their serialization is well-defined and should not issue warnings.
  • Fixed bug: that caused that some user-defined generic reference types were incorrectly marked as not serializable
  • New improvement: symbols obtained from a specific Compilation within a specific SemanticModel are no longer cached indefinitely. Instead, we now construct cache for each Compilation. These caches are held using weak references.
  • New improvement: sanity check. Serializability analyzers perform a simple sanity check to ensure that everything works before issuing warnings. (This might not be necessary anymore but I decided to leave it there).
  • New improvement: serializability warnings now issue property paths. This is more straight-forward when looking for the root of the issue - especially when working with multiple levels of nested classes.

Additionally, DateTimeOffset was added to supported types. Analyzers no longer generate serializability warnings for properties that implement IEnumerable (however element type needs to be still serializable).

@acizmarik acizmarik added this to the Version 4.0 milestone Dec 20, 2021
@acizmarik
Copy link
Member Author

acizmarik commented Dec 20, 2021

It seems like Analyzer tests are not run on CI currently

@acizmarik acizmarik marked this pull request as ready for review December 22, 2021 23:02
@acizmarik acizmarik merged commit 7876247 into main Dec 22, 2021
@acizmarik acizmarik deleted the fix/serializability-analyzers branch December 22, 2021 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants