Consider unsealing JsonSourceGenerationOptionsAttribute

[jasond-s]

Cannot subclass JsonSerializerOptionsAttribute for use with JsonSerializerContext source generators

Configuration

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

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net5.0</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="System.Text.Json" Version="6.0.0-preview.6.21352.12" />
  </ItemGroup>

</Project>

and

<?xml version="1.0" encoding="utf-8"?>
<configuration>
  <fallbackPackageFolders>
    <clear />
  </fallbackPackageFolders>
  <packageSources>
    <clear />
    <add key="NuGet.org" value="https://api.nuget.org/v3/index.json" />

    <!-- TEST_RESTORE_SOURCES_INSERTION_LINE -->
    <add key="dotnet-public" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-public/nuget/v3/index.json" />
    <add key="dotnet-tools" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-tools/nuget/v3/index.json" />
    <add key="dotnet-eng" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-eng/nuget/v3/index.json" />
    <add key="dotnet6" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet6/nuget/v3/index.json" />
    <add key="dotnet6-transport" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet6-transport/nuget/v3/index.json" />
    <!-- Used for dotnet pack task -->
    <add key="nuget-build" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/nuget-build/nuget/v3/index.json" />
    <!-- Used for the Rich Navigation indexing task -->
    <add key="richnav" value="https://pkgs.dev.azure.com/azure-public/vside/_packaging/vs-buildservices/nuget/v3/index.json" />
  </packageSources>
</configuration>

Desired Behaviour

As this is an attribute we can't pass around instances of the options object, but it would be really handy if you could subclass it.

This would mean that you could share a set of strict options that you might want to use between services or for public APIs. For instance everything being non indented and camel case. We have a lot of separate solutions around the org and we want to make sure that the data interchange formats can be standardised.

Possibly this is the 'offending' line?

runtime/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs

Line 134 in 779b794

if (jsonSerializableAttributeSymbol.Equals(attributeContainingTypeSymbol, SymbolEqualityComparer.Default))

[ghost]

Tagging subscribers to this area: @eiriktsarpalis, @layomia
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Cannot subclass JsonSerializerOptionsAttribute for use with JsonSerializerContext source generators

Configuration

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

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net5.0</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="System.Text.Json" Version="6.0.0-preview.6.21352.12" />
  </ItemGroup>

</Project>

and

<?xml version="1.0" encoding="utf-8"?>
<configuration>
  <fallbackPackageFolders>
    <clear />
  </fallbackPackageFolders>
  <packageSources>
    <clear />
    <add key="NuGet.org" value="https://api.nuget.org/v3/index.json" />

    <!-- TEST_RESTORE_SOURCES_INSERTION_LINE -->
    <add key="dotnet-public" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-public/nuget/v3/index.json" />
    <add key="dotnet-tools" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-tools/nuget/v3/index.json" />
    <add key="dotnet-eng" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-eng/nuget/v3/index.json" />
    <add key="dotnet6" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet6/nuget/v3/index.json" />
    <add key="dotnet6-transport" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet6-transport/nuget/v3/index.json" />
    <!-- Used for dotnet pack task -->
    <add key="nuget-build" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/nuget-build/nuget/v3/index.json" />
    <!-- Used for the Rich Navigation indexing task -->
    <add key="richnav" value="https://pkgs.dev.azure.com/azure-public/vside/_packaging/vs-buildservices/nuget/v3/index.json" />
  </packageSources>
</configuration>

Desired Behaviour

As this is an attribute we can't pass around instances of the options object, but it would be really handy if you could subclass it.

This would mean that you could share a set of strict options that you might want to use between services or for public APIs. For instance everything being non indented and camel case. We have a lot of separate solutions around the org and we want to make sure that the data interchange formats can be standardised.

Possibly this is the 'offending' line?

runtime/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs

Line 134 in 779b794

if (jsonSerializableAttributeSymbol.Equals(attributeContainingTypeSymbol, SymbolEqualityComparer.Default))

Author:	jasond-s
Assignees:	-
Labels:	area-System.Text.Json, untriaged
Milestone:	-

[layomia]

It looks like you're using a preview 6 build of System.Text.Json. Some of the APIs of the source generation feature have changed since then - I recommend picking up the latest build.

The intention with the JsonSourceGenerationOptionsAttribute is for it to be sealed, just like the analogous JsonSerializerOptions type used for runtime configuration. I'll put up a PR sealing the type shortly.

I think it is a pretty reasonable request to unseal the type as shown in your example, but that would need to go through API review so that we can properly consider benefits/downsides

[jasond-s]

The intention with the JsonSourceGenerationOptionsAttribute is for it to be sealed, just like the analogous JsonSerializerOptions type used for runtime configuration.

Making JsonSerializerOptions sealed makes a lot of sense, if you want to share them you can use an instance. This isn't easy here at all.

I think I would be happy for the attribute to be sealed as well, if you could pass a static instance of the options class into it's constructor. I guess making JsonSerializerOptions a valid attribute type is probably a bigger change than is likely to happen by NET6 though 😅

[KalleOlaviNiemitalo]

#54527 renamed JsonSerializerOptionsAttribute to JsonSourceGenerationOptionsAttribute.

In "Desired behaviour":

public class MyJsonSerializerOptions : JsonSerializerOptionsAttribute
{
    public MyJsonSerializerOptions()
    {
        WriteIndented = true;
    }
}

For this to work, the code generator would have to understand at compile time that the MyJsonSerializerOptions constructor sets WriteIndented, and preferably also what the value will be. That would be more difficult than recognising the named attribute argument here:

runtime/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs

Lines 390 to 392 in 770d630

	case "WriteIndented":
	options.WriteIndented = bool.Parse(propertyValueStr);
	break;

[KalleOlaviNiemitalo]

Perhaps it would be easier to make the source generator support a hacky thing like this:

[JsonSourceGenerationOptions(WriteIndented = true)]
public class MyJsonSourceGenerationOptionsAttribute : JsonSourceGenerationOptionsAttribute
{
}

i.e. move the property initialisation from the constructor to an attribute on the attribute. That way, the source generator would be able to find the value of WriteIndented without having to execute the attribute constructor at compile time.

Alternatively, define an attribute class that can target an assembly and provide default settings for JSON serialisation of all types in the specified namespace:

[assembly: JsonSourceGenerationDefaultOptions(
    Namespace = "MyApplication.MyNamespace",
    WriteIndented = true)]

[jasond-s]

Perhaps it would be easier to make the source generator support a hacky thing like this:

I'm down. While source generators start to be used more I am sure that there will be various patterns that emerge in their usage. This feels like an innovation that isn't going to have a large blast radius and move the code into a corner it can't get out of.

I don't feel, personally, the assembly version for the API is the same. This feels quite implicit and potentially dangerous/astonishing.

[layomia]

I've opened the PR to seal the attribute for now in 6.0 - #56317. There's a good set of ideas here on how to enable this sort of subclassing, but I think at this stage it's better to wait and address it in 7.0. For now, I hope it's not too inconvenient to repeat the desired annotations for multiple context types.

[eiriktsarpalis]

We won't have time to work on this for .NET 7, moving to Future.

[eiriktsarpalis]

Taking a closer look at this request -- I don't believe unsealing either JsonSourceGenerationOptionsAttribute or JsonSerializableAttribute is something that we could reasonably support since both are annotations are read at compile time (i.e. we can only access constructor expressions and not materialized attribute instances). This means that a source generator can never anticipate arbitrary logic from derived types, as the following absurd example might demonstrate:

public class MyJsonSourceGenOptionsAttribute : JsonSourceGenerationOptionsAttribute
{
    public MyJsonSourceGenOptionsAttribute(JsonSourceGenerationMode? sourceGenMode)
    {
        GenerationMode = sourceGenMode ?? Enum.Parse<JsonSourceGenerationMode>(Environment.GetEnvironmentVariable("SOURCE_GEN_MODE"));
    }
}