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

ImmutableArray light-up support missing from several builds #652

Closed
sharwell opened this issue Sep 18, 2015 · 6 comments
Closed

ImmutableArray light-up support missing from several builds #652

sharwell opened this issue Sep 18, 2015 · 6 comments

Comments

@sharwell
Copy link

Currently ImmutableCollectionsUtils.cs is excluded from several builds (e.g. .NET 2.0, 3.5, 4.0, and 4.0 PCL). This is causing two problems:

  1. When two extensions to the same application bundle JSON.net, where one of them bundles the 4.0 PCL build and the other uses an immutable, the application could load an assembly which is incapable of providing the functionality required by the .NET 4.5 extension. This appeared in the wild in NotSupportedException thrown by SA1602EnumerationItemsMustBeDocumented DotNetAnalyzers/StyleCopAnalyzers#1359.
  2. The Immutable Collections library is available going back at least to .NET 3.5 using an identical API via the Rackspace.Collections.Immutable package on NuGet.

If ImmutableCollectionsUtils.cs does not itself depend on functionality only available in .NET 4.5+, it should be included in other target builds as well to ensure proper light-up functionality for clients.

sharwell added a commit to sharwell/StyleCopAnalyzers that referenced this issue Sep 19, 2015
This works around JamesNK/Newtonsoft.Json#652 by avoiding the direct use
of immutable types in the serialization process. The result remains
efficient because the builder class caches the last returned immutable
instance, and will continue returning the same instance as long as no
changes are made to the builder.

Fixes DotNetAnalyzers#1359
@JamesNK
Copy link
Owner

JamesNK commented Sep 30, 2015

I'll include it with the .NET 4.0 PCL since it is likely it could be used with up level frameworks. Also I see it is the one you are using. Unfortunately since my test project for the .NET 4.0 PCL are .NET 4.0 I won't be able to give it test coverage. Hopefully the same code in .NET 4.0 doesn't cause any issues.

I think it is unlikely applications are using json.net 2.0, 3.5, 4.0 and are also using immutable types so I won't change them.

@JamesNK JamesNK closed this as completed Sep 30, 2015
@JamesNK
Copy link
Owner

JamesNK commented Sep 30, 2015

239cde8

@sharwell
Copy link
Author

I think it is unlikely applications are using json.net 2.0, 3.5, 4.0 and are also using immutable types so I won't change them.

We are. I originally found and filed this bug due to a report in a .NET 3.5 application.

@sharwell
Copy link
Author

sharwell commented Aug 9, 2017

@JamesNK This issue is showing up as the cause of crashes in Visual Studio. In the event any application places the .NET 4.0 assembly in the GAC, all assemblies using that version of Json.NET will start to fail if they try to deserialize immutable collections. I believe this issue should be reopened, as it is currently impossible to reliably deserialize immutable collection types regardless of the build of Json.NET being used.

If you are interested, I would be willing to implement a light-up based fallback which is slower but at least functional to handle cases where applications load an "early" target of Json.NET and subsequently try to deserialize one of these collections.

@JamesNK JamesNK reopened this Aug 9, 2017
@JamesNK
Copy link
Owner

JamesNK commented Aug 10, 2017

Ok, I've made immutable support universal to all assemblies 72b98ae

@JamesNK JamesNK closed this as completed Aug 10, 2017
@sharwell
Copy link
Author

🙌

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

No branches or pull requests

2 participants