Add [Obsolete] to deprecated members. #472
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds the [Obsolete] attribute to deprecated members and adjusts related test cases and code generation to account for obsolete behavior. Key changes include:
- Wrapping deprecated member usage in tests with pragma directives to suppress obsolete warnings.
- Emitting the [Obsolete] attribute in the generated code for deprecated fields.
- Filtering out deprecated fields from copy and default constructor routines.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Tests/FlatSharpEndToEndTests/TableMembers/TableMembersTests.cs | Added pragma directives to suppress obsolete warnings in tests. |
| src/Tests/FlatSharpEndToEndTests/RawData/RawDataTableTests.cs | Wrapped deprecated member access with pragma directives. |
| src/Tests/FlatSharpEndToEndTests/RawData/RawDataStringTests.cs | Added pragma directives to disable/restore warnings around deprecated usage. |
| src/Tests/FlatSharpEndToEndTests/MetadataAttributes/MetadataAttributes.fbs | Marked a field as deprecated in the schema file. |
| src/Tests/FlatSharpEndToEndTests/MetadataAttributes/MetadataAttributeTests.cs | Added tests to validate that the deprecated attribute is applied. |
| src/FlatSharp/Serialization/RoslynSerializerGenerator.cs | Filtered out diagnostics for obsolete warnings in code generation. |
| src/FlatSharp.Compiler/SchemaModel/PropertyFieldModel.cs | Emitted the [Obsolete] attribute for deprecated fields in code generation. |
| src/FlatSharp.Compiler/SchemaModel/BaseReferenceTypeSchemaModel.cs | Excluded deprecated fields from copy and default constructors. |
| if (this.Field.Deprecated) | ||
| { | ||
| writer.AppendLine("[Obsolete]"); | ||
| } |
There was a problem hiding this comment.
Consider including a descriptive message in the [Obsolete] attribute to help developers understand why this member is deprecated.
| if (this.Field.Deprecated) | |
| { | |
| writer.AppendLine("[Obsolete]"); | |
| } | |
| if (this.Field.Deprecated) | |
| { | |
| string deprecationMessage = string.IsNullOrEmpty(this.Field.Documentation) | |
| ? "This field is deprecated and should not be used." | |
| : $"This field is deprecated: {this.Field.Documentation}"; | |
| writer.AppendLine($"[Obsolete(\"{deprecationMessage}\")]"); | |
| } |
Copilot uses AI. Check for mistakes.
| @@ -84,6 +84,8 @@ public void Byte(FlatBufferDeserializationOption option) | |||
| [DynamicData(nameof(DynamicDataHelper.DeserializationModes), typeof(DynamicDataHelper))] | |||
| public void Double(FlatBufferDeserializationOption option) => this.RunTest<double, DoubleTable>(3.14, option); | |||
|
|
|||
There was a problem hiding this comment.
It would be helpful to add a comment explaining why obsolete warnings are suppressed here to aid future maintainers.
| // Suppressing CS0612 because the obsolete member is still required for testing backward compatibility. |
Copilot uses AI. Check for mistakes.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #472 +/- ##
=======================================
Coverage 96.04% 96.04%
=======================================
Files 124 124
Lines 8923 8928 +5
Branches 748 749 +1
=======================================
+ Hits 8570 8575 +5
Misses 261 261
Partials 92 92
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
No description provided.