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

JSON source generator emits CS0618 warnings if serialized types use [Obsolete] #62076

Closed
martincostello opened this issue Nov 26, 2021 · 12 comments · Fixed by #63496
Closed

JSON source generator emits CS0618 warnings if serialized types use [Obsolete] #62076

martincostello opened this issue Nov 26, 2021 · 12 comments · Fixed by #63496
Assignees
Milestone

Comments

@martincostello
Copy link
Member

Description

If an application uses the .NET 6 source generator for JSON with type(s) that have one or more properties decorated with the [Obsolete] property, three CS0618 warnings will be emitted by the compiler for each such property.

If the application does not control the code of all of the types it (de)serializes these warnings cannot be worked around by removing the properties' obsolete attributes (for example when using shared models distributed via NuGet).

If the application uses TreatWarningsAsErrors or any obsolete properties are marked as an error to use, the application fails to compile.

Reproduction Steps

Compile the following application using the .NET 6.0.100 SDK with dotnet build.

using System.Text;
using System.Text.Json;
using System.Text.Json.Serialization;

var value = new MyObject { NewValue = "Hello World" };
var bytes = JsonSerializer.SerializeToUtf8Bytes(value, typeof(MyObject), new MyJsonContext(new (JsonSerializerDefaults.Web)));
var json = Encoding.UTF8.GetString(bytes);

Console.WriteLine(json);

public class MyObject
{
    public string? NewValue {get; set; }

    [Obsolete("Use NewValue instead")]
    public string? OldValue {get; set; }
}

[JsonSerializable(typeof(MyObject))]
public partial class MyJsonContext : JsonSerializerContext
{
}
<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <ImplicitUsings>enable</ImplicitUsings>
    <NoWarn>$(NoWarn);CA1050</NoWarn>
    <Nullable>enable</Nullable>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0</TargetFramework>
  </PropertyGroup>
</Project>

Expected behavior

The application compiles with zero warnings/errors due to the obsolete properties not being used by user code.

Actual behavior

The application emits three CS0618 warnings per obsolete property from usages in the generated code.

> dotnet build
Microsoft (R) Build Engine version 17.0.0+c9eb9dd64 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

  Determining projects to restore...
  All projects are up-to-date for restore.
C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyJsonContext.MyObject.g.cs(72,42): warning CS0618: 'MyObject.OldValue' is obsolete: 'Use NewValue instead' [C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\JsonSourceGeneratorObsoleteProperties.csproj]
C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyJsonContext.MyObject.g.cs(73,49): warning CS0618: 'MyObject.OldValue' is obsolete: 'Use NewValue instead' [C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\JsonSourceGeneratorObsoleteProperties.csproj]
C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyJsonContext.MyObject.g.cs(97,51): warning CS0618: 'MyObject.OldValue' is obsolete: 'Use NewValue instead' [C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\JsonSourceGeneratorObsoleteProperties.csproj]
  JsonSourceGeneratorObsoleteProperties -> C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\bin\Debug\net6.0\JsonSourceGeneratorObsoleteProperties.dll

Build succeeded.

C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyJsonContext.MyObject.g.cs(72,42): warning CS0618: 'MyObject.OldValue' is obsolete: 'Use NewValue instead' [C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\JsonSourceGeneratorObsoleteProperties.csproj]
C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyJsonContext.MyObject.g.cs(73,49): warning CS0618: 'MyObject.OldValue' is obsolete: 'Use NewValue instead' [C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\JsonSourceGeneratorObsoleteProperties.csproj]
C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyJsonContext.MyObject.g.cs(97,51): warning CS0618: 'MyObject.OldValue' is obsolete: 'Use NewValue instead' [C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\JsonSourceGeneratorObsoleteProperties.csproj]
    3 Warning(s)
    0 Error(s)

Time Elapsed 00:00:01.99

Regression?

No.

Known Workarounds

No response

Configuration

Partial output from dotnet --info:

> dotnet --info
.NET SDK (reflecting any global.json):
 Version:   6.0.100
 Commit:    9e8b04bbff

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.19043
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\6.0.100\

Host (useful for support):
  Version: 6.0.0
  Commit:  4822e3c3aa

Other information

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Text.Json untriaged New issue has not been triaged by the area owner labels Nov 26, 2021
@ghost
Copy link

ghost commented Nov 26, 2021

Tagging subscribers to this area: @dotnet/area-system-text-json
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

If an application uses the .NET 6 source generator for JSON with type(s) that have one or more properties decorated with the [Obsolete] property, three CS0618 warnings will be emitted by the compiler for each such property.

If the application does not control the code of all of the types it (de)serializes these warnings cannot be worked around by removing the properties' obsolete attributes (for example when using shared models distributed via NuGet).

If the application uses TreatWarningsAsErrors or any obsolete properties are marked as an error to use, the application fails to compile.

Reproduction Steps

Compile the following application using the .NET 6.0.100 SDK with dotnet build.

using System.Text;
using System.Text.Json;
using System.Text.Json.Serialization;

var value = new MyObject { NewValue = "Hello World" };
var bytes = JsonSerializer.SerializeToUtf8Bytes(value, typeof(MyObject), new MyJsonContext(new (JsonSerializerDefaults.Web)));
var json = Encoding.UTF8.GetString(bytes);

Console.WriteLine(json);

public class MyObject
{
    public string? NewValue {get; set; }

    [Obsolete("Use NewValue instead")]
    public string? OldValue {get; set; }
}

[JsonSerializable(typeof(MyObject))]
public partial class MyJsonContext : JsonSerializerContext
{
}
<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <ImplicitUsings>enable</ImplicitUsings>
    <NoWarn>$(NoWarn);CA1050</NoWarn>
    <Nullable>enable</Nullable>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0</TargetFramework>
  </PropertyGroup>
</Project>

Expected behavior

The application compiles with zero warnings/errors due to the obsolete properties not being used by user code.

Actual behavior

The application emits three CS0618 warnings per obsolete property from usages in the generated code.

> dotnet build
Microsoft (R) Build Engine version 17.0.0+c9eb9dd64 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

  Determining projects to restore...
  All projects are up-to-date for restore.
C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyJsonContext.MyObject.g.cs(72,42): warning CS0618: 'MyObject.OldValue' is obsolete: 'Use NewValue instead' [C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\JsonSourceGeneratorObsoleteProperties.csproj]
C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyJsonContext.MyObject.g.cs(73,49): warning CS0618: 'MyObject.OldValue' is obsolete: 'Use NewValue instead' [C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\JsonSourceGeneratorObsoleteProperties.csproj]
C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyJsonContext.MyObject.g.cs(97,51): warning CS0618: 'MyObject.OldValue' is obsolete: 'Use NewValue instead' [C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\JsonSourceGeneratorObsoleteProperties.csproj]
  JsonSourceGeneratorObsoleteProperties -> C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\bin\Debug\net6.0\JsonSourceGeneratorObsoleteProperties.dll

Build succeeded.

C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyJsonContext.MyObject.g.cs(72,42): warning CS0618: 'MyObject.OldValue' is obsolete: 'Use NewValue instead' [C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\JsonSourceGeneratorObsoleteProperties.csproj]
C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyJsonContext.MyObject.g.cs(73,49): warning CS0618: 'MyObject.OldValue' is obsolete: 'Use NewValue instead' [C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\JsonSourceGeneratorObsoleteProperties.csproj]
C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyJsonContext.MyObject.g.cs(97,51): warning CS0618: 'MyObject.OldValue' is obsolete: 'Use NewValue instead' [C:\Coding\martincostello\JsonSourceGeneratorObsoleteProperties\JsonSourceGeneratorObsoleteProperties.csproj]
    3 Warning(s)
    0 Error(s)

Time Elapsed 00:00:01.99

Regression?

No.

Known Workarounds

No response

Configuration

Partial output from dotnet --info:

> dotnet --info
.NET SDK (reflecting any global.json):
 Version:   6.0.100
 Commit:    9e8b04bbff

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.19043
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\6.0.100\

Host (useful for support):
  Version: 6.0.0
  Commit:  4822e3c3aa

Other information

No response

Author: martincostello
Assignees: -
Labels:

area-System.Text.Json, untriaged

Milestone: -

@eiriktsarpalis
Copy link
Member

The application emits three CS0618 warnings per obsolete property from usages in the generated code.

Curious, why would you consider this to be an issue? Marking a property obsolete does not prevent it from being serialized, this holds for both the reflection and source gen serializers. The warning simply reflects the fact that source gen accesses the property at compile time.

If you need to prevent the property from being serialized, I would recommend adding a JsonIgnore attribute next to the obsolete attribute.

@martincostello
Copy link
Member Author

The problems this caused in the application I found this in are:

  1. We use TreatWarningsAsErrors, causing the application to fail to compile. This could be disabled to work around it, but we'd prefer not to have warnings creep into our codebase over time that get missed/ignored.
  2. The properties are present in the JSON models returned by our application for backwards compatibility with older clients, which is why they aren't ignored. We cannot remove them as that would break these clients, but we can discourage their use when shared via NuGet packages, which is why we've added [Obsolete] to some of the properties.
  3. The application re-uses models via NuGet from other applications that also use [Obsolete] in a similar way to discourage their use. Our application doesn't use these properties, or uses #pragmas to disable them on a case-by-case basis where we need to pass them on for backwards compatibility reasons. We would have to systematically remove [Obsolete] from every model in every code base to work around these warnings as they can't be suppressed locally in the application as they are not compiled there.

I've been investigating plugging the new source generator into Refit for our HttpClient usage to see what sort of performance benefits we can get from the source generator. I turned off TreatWarningsAsErrors for now as this is just a spike to get some perf numbers, but I've run into further issues where the source generator is failing to output code at all for some of our types.

I'll file separate issue(s) for those problems if/when I can come up with a minimal repro (the error is error CS0103: The name 'GetConverterFromFactory' does not exist in the current context).

@eiriktsarpalis
Copy link
Member

Related to #61734. There should be a way for us to suppress compiler warnings for generated files. I assumed that the auto-generated tag would help in that regard but I couldn't find any definitive documentation on that matter. cc @ericstj @layomia

@stephentoub
Copy link
Member

There should be a way for us to suppress compiler warnings for generated files.

We can just emit #pragma warning disable at the top of the generated file for any warnings we want suppressed for the generated code in that file.

That would be separate from any global setting.

@martincostello
Copy link
Member Author

Yeah, a "quick fix" for this could be to just emit a #pragma warning disable CS0618 in the generated code.

@eiriktsarpalis
Copy link
Member

One concern is that we can only suppress warnings for codes we can predict are going to be an issue in source generated code.

@layomia
Copy link
Contributor

layomia commented Dec 10, 2021

One concern is that we can only suppress warnings for codes we can predict are going to be an issue in source generated code.

Since this issue is manifest, it seems reasonable to always suppress obsolete-usage warnings in the generated code. I can't think of a user scenario where it is desirable to surface them. Other codes could be considered as needed.

I think it's reasonable to service a fix here given the current behavior can cause a non-trivial nuisance in user apps.

@layomia layomia removed the untriaged New issue has not been triaged by the area owner label Dec 10, 2021
@layomia layomia self-assigned this Dec 10, 2021
@fbrosseau
Copy link

fbrosseau commented Dec 31, 2021

@layomia

I just encountered this issue, and one thing that makes it a lot worse, is that [JsonIgnore] does not help.

For instance, the following code:

    public class ClassWithObsolete
    {
        [Obsolete("This is a test")]
        public bool Test { get; set; }
    }

    public class MyJson
    {
        [JsonIgnore]
        public ClassWithObsolete O { get; set; }
    }

The code still emits the obsolete warnings, even though the code generator should not have touched the type at all. Worse, if the Obsolete attribute has warning-as-error, the build breaks.

I think this may be a bigger bug than suppressing warnings in generated code? Since there shouldn't have been generated code in the first place for this example?

Right now the only workaround I see is to avoid using properties altogether and to use methods for those "members" that shouldn't be touched by serialization, and type-erase the backing field into object.

Edit: the JsonIgnore part probably belongs to bug #62354

@layomia
Copy link
Contributor

layomia commented Jan 7, 2022

Edit: the JsonIgnore part probably belongs to bug #62354

@fbrosseau as I suggested in #62354 (comment), it would be great if you could create a new dedicated issue for [JsonIgnore] + source generation concerns. I'll also mention here that we still need to generate metadata for ignored props/types since some of the information might be needed at runtime.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jan 7, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jan 7, 2022
@layomia
Copy link
Contributor

layomia commented Jan 7, 2022

Re-opening for 6.0 consideration - #63501.

@layomia
Copy link
Contributor

layomia commented Jan 8, 2022

Fixed for 6.0 in #63501.

@layomia layomia closed this as completed Jan 8, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Feb 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants