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

Fix S1104 FP: Should be ignored on classes and structs marked as [Serializable] #8504

Closed
dukobpa3 opened this issue Dec 22, 2023 · 7 comments · Fixed by #8521
Closed

Fix S1104 FP: Should be ignored on classes and structs marked as [Serializable] #8504

dukobpa3 opened this issue Dec 22, 2023 · 7 comments · Fixed by #8521
Assignees
Labels
Area: C# C# rules related issues. Area: VB.NET VB.NET rules related issues. Type: False Positive Rule IS triggered when it shouldn't be.
Milestone

Comments

@dukobpa3
Copy link

dukobpa3 commented Dec 22, 2023

Description

S1104 FP: Should be ignored on classes and structs marked as [Serializable]

Repro steps

This kind of classes/structs always have an issue on each field.

        [Serializable]
        public class ConfigOptionDto
        {
            public string type;

            public string key;

            public string value;
        }

Expected behavior

If object have [Serializable] attribute then 99.9% it will have a lot of public fields, so this rule is not relevant in this case

Actual behavior

This kind of classes/structs always have an issue on each field.

Known workarounds

Something like this. But in this case we really don't need it since all compilers already understand [Serializable] objects and will not strip it, an even Rider/Resharper will not highlite this issue etc...

            [UsedImplicitly]
            public string type;

            [UsedImplicitly]
            public string key;

            [UsedImplicitly]
            public string value;

Related information

  • Visual Studio version – 2022
  • MSBuild / dotnet version – latest from VS 2022
  • SonarScanner for .NET version (if used) – 6.xx
  • Operating System – Windows 10
@dukobpa3 dukobpa3 changed the title Fix S1104 FP/FN: Should be ignored on classes and structs marked as [Serializable] Fix S1104, S1144 FP/FN: Should be ignored on classes and structs marked as [Serializable] Dec 22, 2023
@Tim-Pohlmann
Copy link
Contributor

Hello @dukobpa3, thanks for the report.
For clarification: Using properties instead of fields would be another workaround, correct? Is there a particular reason why you prefer fields over properties for serializable classes?

@Tim-Pohlmann Tim-Pohlmann changed the title Fix S1104, S1144 FP/FN: Should be ignored on classes and structs marked as [Serializable] Fix S1104, S1144 FP: Should be ignored on classes and structs marked as [Serializable] Jan 4, 2024
@Tim-Pohlmann Tim-Pohlmann added Type: False Positive Rule IS triggered when it shouldn't be. Area: VB.NET VB.NET rules related issues. Area: C# C# rules related issues. labels Jan 4, 2024
@dukobpa3
Copy link
Author

dukobpa3 commented Jan 4, 2024

Using properties instead of fields would be another workaround, correct?

Actually not because many serializers can't work with properties.

@Tim-Pohlmann
Copy link
Contributor

Good point. Then, this must be frustrating to deal with; I apologize. We will fix it in a future hardening sprint.

@Tim-Pohlmann Tim-Pohlmann changed the title Fix S1104, S1144 FP: Should be ignored on classes and structs marked as [Serializable] Fix S1104 FP: Should be ignored on classes and structs marked as [Serializable] Jan 5, 2024
@Tim-Pohlmann
Copy link
Contributor

I don't think making the fields private is a solution in most cases, so I clarified this to be an FP for S1104 but not S1144.

@Tim-Pohlmann
Copy link
Contributor

For transparency: I updated the code sample to use a public class, since the rule doesn't raise for private ones.

@dukobpa3
Copy link
Author

dukobpa3 commented Jan 6, 2024

S1144 I think relevant also

Sometimes I should add the fields just to copy serializing source structure, but not all of them will be used after that

Especially when working with some 3-d parties.
Or in case implementing network protocols with sending serialized data
I should implement some protocol DTO but not each field will be used after that

@Tim-Pohlmann
Copy link
Contributor

@dukobpa3 You are making some great points. I have created another issue. Thanks!

@Tim-Pohlmann Tim-Pohlmann added this to the 9.17 milestone Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: C# C# rules related issues. Area: VB.NET VB.NET rules related issues. Type: False Positive Rule IS triggered when it shouldn't be.
Projects
None yet
2 participants