Skip to content

Conversation

@trippwill
Copy link
Owner

@trippwill trippwill commented Feb 24, 2025

Introduces a SymbolVisitor to simply some of the type schema generation logic.

Introduces a new solution for handling abstract type schemas, and generic type schemas.

Copilot AI review requested due to automatic review settings February 24, 2025 00:32
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 65 out of 79 changed files in this pull request and generated 1 comment.

Files not reviewed (14)
  • .editorconfig: Language not supported
  • Directory.Packages.props: Language not supported
  • src/SharpSchema.Annotations/SharpSchema.Annotations.csproj: Language not supported
  • src/SharpSchema.Generator/GeneratorOptions.cs: Evaluated as low risk
  • src/SharpSchema.Generator/LeafDeclaredTypeSyntaxVisitor.cs: Evaluated as low risk
  • src/SharpSchema.Generator/RootSyntaxVisitor.cs: Evaluated as low risk
  • src/SharpSchema.Generator/Model/Metadata.cs: Evaluated as low risk
  • src/SharpSchema.Generator/Resolvers/EnumSymbolVisitor.cs: Evaluated as low risk
  • src/SharpSchema.Annotations/DictionaryKeyMode.cs: Evaluated as low risk
  • src/SharpSchema.Annotations/EnumMode.cs: Evaluated as low risk
  • src/SharpSchema.Annotations/TraversalMode.cs: Evaluated as low risk
  • src/SharpSchema.Annotations/AccessibilityMode.cs: Evaluated as low risk
  • src/SharpSchema.Annotations/SchemaAccessibilityModeAttribute.cs: Evaluated as low risk
  • src/SharpSchema.Annotations/GeneratorOptions.cs: Evaluated as low risk
Comments suppressed due to low confidence (3)

src/SharpSchema.Generator/LeafSyntaxVisitor.cs:188

  • Ensure that the elementSchema resolution logic is robust. If elementSchema is null, it should handle the scenario appropriately.
if (elementSymbol.FindDeclaringSyntax() is BaseTypeDeclarationSyntax elx)

src/SharpSchema.Annotations/SchemaEnumModeAttribute.cs:12

  • The constructor syntax used here might not be supported in all versions of C#. Consider using a standard constructor syntax: public SchemaEnumModeAttribute(EnumMode value) : base() { Value = value; }
public class SchemaEnumModeAttribute(EnumMode value) : SchemaAttribute

src/SharpSchema.Annotations/SchemaDictionaryKeyModeAttribute.cs:10

  • [nitpick] The AttributeUsage might be too broad. Consider specifying the exact members it supports.
[AttributeUsage(SchemaAttribute.SupportedMembers)]

Improved documentation for parameters in `GeneratorOptions.cs` and added a new `NumberMode` parameter with default value `StrictDefs`. Introduced `NumberMode.cs` with an enumeration defining `StrictDefs`, `StrictInline`, and `JsonNative`. Updated access modifiers from `public` to `internal` for various attributes in the `SharpSchema.Annotations` namespace, along with enhanced documentation for clarity.
- Added `.runsettings` and `nvim-diff.runsettings` for diff engine configurations.
- Refactored `GeneratorOptions` to use `init` properties for better immutability.
- Removed `SchemaNumberModeAttribute` class to simplify code.
- Introduced `Unsupported` class for centralized error messaging in schema generation.
- Improved `LeafSyntaxVisitor` for better caching and error handling.
- Streamlined schema creation in `CommonSchemas`.
- Updated `MemberMeta` for more user-friendly title generation.
- Renamed and refactored `TypeSchemaSymbolVisitor` to `NamedTypeSymbolVisitor`.
- Enhanced `VerifyTests` with new verification methods and improved organization.
- Updated test data files to reflect schema changes, including new records and properties.
- Added verification files for accessibility and number modes.
- Introduced new test cases for dictionary key modes and enum modes.
- Updated `vscode-diff.runsettings` for Visual Studio Code configuration.
Builder? builder = base.Visit(node);

Builder result = new Builder()
.Schema("http://json-schema.org/draft-07/schema#");

Check warning

Code scanning / devskim

An HTTP-based URL without TLS was detected. Warning

Insecure URL
- Removed `IsExternalInit` package version from `Directory.Packages.props`.
- Added `PackageReference` for `PolySharp` in `SharpSchema.Annotations.Source.csproj`.
- Replaced `IsExternalInit` with `PolySharp` in `SharpSchema.Annotations.csproj`, maintaining asset settings.
@codecov
Copy link

codecov bot commented Feb 25, 2025

Codecov Report

Attention: Patch coverage is 73.56495% with 175 lines in your changes missing coverage. Please review.

Project coverage is 75.52%. Comparing base (4971faa) to head (9857c02).

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...harpSchema.Generator/Utilities/SymbolExtensions.cs 39.34% 66 Missing and 8 partials ⚠️
src/SharpSchema.Generator/LeafSyntaxVisitor.cs 76.78% 18 Missing and 21 partials ⚠️
...rc/SharpSchema.Generator/Model/ObjectAttributes.cs 39.13% 14 Missing ⚠️
src/SharpSchema.Generator/NamedTypeResolver.cs 91.42% 6 Missing and 3 partials ⚠️
src/SharpSchema.Generator/Utilities/Tracer.cs 40.00% 4 Missing and 2 partials ⚠️
...rc/SharpSchema.Generator/Resolvers/EnumResolver.cs 72.22% 2 Missing and 3 partials ⚠️
...rpSchema.Generator/Resolvers/CollectionResolver.cs 89.65% 0 Missing and 3 partials ⚠️
src/SharpSchema.Annotations/GeneratorOptions.cs 87.50% 1 Missing and 1 partial ⚠️
...ma.Annotations/SchemaAccessibilityModeAttribute.cs 0.00% 2 Missing ⚠️
...ma.Annotations/SchemaDictionaryKeyModeAttribute.cs 0.00% 2 Missing ⚠️
... and 13 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #96      +/-   ##
==========================================
- Coverage   78.48%   75.52%   -2.97%     
==========================================
  Files          49       60      +11     
  Lines        1455     1679     +224     
  Branches      274      329      +55     
==========================================
+ Hits         1142     1268     +126     
- Misses        216      309      +93     
- Partials       97      102       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- Updated `NamedTypeSymbolVisitor` to improve `WasLastPropertyRequired` evaluation using `IsRequired`.
- Simplified `AnnotationExtensions` to rely solely on `SchemaIgnoreAttribute` for ignoring symbols.
- Expanded `VerifyTests` to cover new and modified classes and records.
Enhance the NamedTypeSymbolVisitor to support schema overrides for properties and constructors, refining the logic for required properties and default values. Expand VerifyTests with additional cases to validate the new functionality.
- Changed `GeneratorOptions` from a class to a record, adding properties for `AccessibilityMode`, `TraversalMode`, `DictionaryKeyMode`, `EnumMode`, and `NumberMode`.
- Introduced `ObjectAttributes` and `PropertyAttributes` records to handle various attribute properties.
- Updated `NamedTypeSymbolVisitor` and `EnumSymbolVisitor` to utilize new traversal modes.
- Added `GetAttributeHandler` methods in `SymbolExtensions` for easier attribute retrieval.
- Enhanced `JsonSchemaBuilderExtensions` with a method to merge properties.
- Modified `VerifyTests` to include new test cases and adjust existing ones.
- Updated test data classes to reflect changes in `SchemaTraversalMode`.
- Revised verification files to align with the new schema structure.
This commit introduces a `RootContext` class to encapsulate the context for schema generation, simplifying the `LeafSyntaxVisitor` and `NamedTypeResolver` classes. The `CommonSchemas` class has been enhanced for better handling of unsupported objects and schema references. Visitor classes have been documented for clarity, and new utility methods have been added to improve schema generation options. Additionally, property and parameter handling has been refined, and test data has been updated to ensure accuracy in generated schemas. These changes enhance maintainability, readability, and functionality of the code.
{
NullableAnnotation.NotAnnotated => false,
NullableAnnotation.Annotated => true,
NullableAnnotation.None => false, // TODO: Make Configurable

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
- Updated `GeneratorOptions` to include a constructor for copying values from another instance, improving flexibility.
- Changed attribute usage in `SchemaAccessibilityModeAttribute.cs` from `SupportedMembers` to `SupportedTypes`.
- Modified `NamedTypeResolver` to preserve original options when processing parameters and properties, enhancing schema generation accuracy.
- Added new test classes and methods to verify accessibility overrides in schema generation.
- Updated `TestDataFixture` to include a path to the new accessibility test data file.
- Added or updated JSON schema verification files to reflect new accessibility rules.
- Enhanced `VerifyTests` with new test cases for accessibility overrides.
Updated `ObjectAttributes` and `PropertyAttributes` to reintroduce `TraversalMode` and `ValueRange` attribute handlers. Modified `GeneratorOptionsExtensions` to adjust how `DictionaryKeyMode` is handled. Added new classes in `TestData.Attributes.cs` for various dictionary key scenarios and updated test references accordingly. Created new JSON schema files for these scenarios and added a test method in `VerifyTests.cs` to validate the schemas, improving test coverage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants