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 S3925 FP: Classes not having extra properties should not have to extend ISerializable interface #3945

Closed
Corniel opened this issue Jan 15, 2021 · 3 comments · Fixed by #7673
Assignees
Labels
Area: C# C# rules related issues. Type: False Positive Rule IS triggered when it shouldn't be.
Milestone

Comments

@Corniel
Copy link
Contributor

Corniel commented Jan 15, 2021

Description

Rule S3925 demands that classes should always override ISerializable methods. However, if the class has no extra properties (or fields) this is not necessary, like in this scenario:

public CustomLookup : Dictionary<string, object> // Compliant, no extra fields/properties to serialize
{
}

The rule should not be applied in cases like these.

@costin-zaharia-sonarsource costin-zaharia-sonarsource added Area: C# C# rules related issues. Type: False Positive Rule IS triggered when it shouldn't be. labels Feb 1, 2021
@mary-georgiou-sonarsource mary-georgiou-sonarsource changed the title FP 3925: Classes not having extra properties should not have to extend ISerializable interface S3925 FP: Classes not having extra properties should not have to extend ISerializable interface Oct 14, 2022
@martin-strecker-sonarsource
Copy link
Contributor

According to the RSpec, this is indeed a false positive:

  • A derived type has serializable fields but the ISerializable.GetObjectData method is not overridden.

@andrei-epure-sonarsource andrei-epure-sonarsource modified the milestones: Onboarding hardening, 8.52 Jan 5, 2023
@pavel-mikula-sonarsource pavel-mikula-sonarsource modified the milestones: 8.52, 8.53 Jan 27, 2023
@zsolt-kolbay-sonarsource zsolt-kolbay-sonarsource removed this from the 8.53 milestone Feb 2, 2023
@zsolt-kolbay-sonarsource zsolt-kolbay-sonarsource removed their assignment Feb 2, 2023
@cristian-ambrosini-sonarsource cristian-ambrosini-sonarsource added this to the 9.7 milestone Jul 26, 2023
@cristian-ambrosini-sonarsource cristian-ambrosini-sonarsource changed the title S3925 FP: Classes not having extra properties should not have to extend ISerializable interface Fix S3925 FP: Classes not having extra properties should not have to extend ISerializable interface Jul 26, 2023
@martin-strecker-sonarsource
Copy link
Contributor

@Corniel This issue was closed as part of our work at #7721. This issue is slightly different, but I don't think it is needed anymore as it should cover the changes we made. If you still think that there are FPs, feel free to re-open or raise a new issue.
Thank you anyway for bringing this up and for your work on #5709 It helped a lot to improve the rule 👍 .

@Corniel
Copy link
Contributor Author

Corniel commented Aug 4, 2023

I think this solution should also work.

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. Type: False Positive Rule IS triggered when it shouldn't be.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants