Conversation
…ration Indexers have IPropertySymbol.Name == 'this' and IsIndexer == true. Including them causes the generator to emit invalid C# like 'case this[]: v.this[] = ...'. Filter out indexers by checking IsIndexer before adding them to PropertySymbols.
Init-only properties have SetMethod.IsInitOnly == true and cannot be set via IObjectAccessor.Set. Mark them as read-only in StaticTypeInspector and exclude them from setter generation in ObjectAccessorFileGenerator.
… Create Classes without a parameterless constructor cannot be instantiated with new ClassName(). Add HasParameterlessConstructor property to ClassObject and filter them out in StaticObjectFactory.Create to avoid generating invalid new expressions.
Add the auto-generated comment block and disable CS1591 warning for the generated source file so analyzers properly skip analysis.
There was a problem hiding this comment.
🟡 Changes recommended
The new factory filtering can unintentionally drop collection interface overrides and the parameterless-constructor check can still allow types that will generate uncompilable new T() calls.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates YamlDotNet.Analyzers.StaticGenerator to avoid generating invalid/unsupported C# for certain members and types (indexers, init-only properties, and types that can’t be instantiated), and to mark generated output as auto-generated to reduce analyzer noise.
Changes:
- Excludes indexer properties from the serializable member set and skips init-only setters in generated object accessors.
- Treats init-only properties as read-only in the generated type inspector metadata.
- Filters
StaticObjectFactory.Createregistrations based on a newHasParameterlessConstructorflag and adds an auto-generated header + CS1591 suppression to generated sources.
File summaries
| File | Description |
|---|---|
| YamlDotNet.Analyzers.StaticGenerator/TypeFactoryGenerator.cs | Adds auto-generated header and disables CS1591 in generated output. |
| YamlDotNet.Analyzers.StaticGenerator/StaticTypeInspectorFile.cs | Marks init-only properties as read-only in generated property descriptors. |
| YamlDotNet.Analyzers.StaticGenerator/StaticObjectFactoryFile.cs | Filters factory Create registrations based on parameterless-constructor availability. |
| YamlDotNet.Analyzers.StaticGenerator/SerializableSyntaxReceiver.cs | Skips indexer properties when collecting serializable members. |
| YamlDotNet.Analyzers.StaticGenerator/ObjectAccessorFileGenerator.cs | Avoids generating setters for init-only properties. |
| YamlDotNet.Analyzers.StaticGenerator/ClassObject.cs | Introduces HasParameterlessConstructor used to decide whether types can be instantiated. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| bool isListOverride = false, | ||
| bool isDictionaryOverride = false) | ||
| { | ||
| HasParameterlessConstructor = moduleSymbol is INamedTypeSymbol namedTypeSymbol && namedTypeSymbol.InstanceConstructors.Any(c => c.Parameters.Length == 0); |
| Write("public override object Create(Type type)"); | ||
| Write("{"); Indent(); | ||
| foreach (var o in syntaxReceiver.Classes.Where(c => !c.Value.IsArray)) | ||
| foreach (var o in syntaxReceiver.Classes.Where(c => !c.Value.IsArray && c.Value.HasParameterlessConstructor)) |
| foreach (var property in classObject.PropertySymbols) | ||
| { | ||
| if (property.SetMethod != null) | ||
| if (property.SetMethod != null && !property.SetMethod.IsInitOnly) |
Comment on lines
+99
to
102
| write("#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member", true); | ||
| write("#pragma warning disable CS8767 // Nullability of reference types", true); | ||
| write("#pragma warning disable CS8767 // Nullability of reference types", true); | ||
| write("#pragma warning disable CS8603 // Possible null reference return", true); |
…tFactory Create" This reverts commit 960982a.
frg2089
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
YamlDotNet.Analyzers.StaticGenerator was generating invalid C# code when encountering certain class members:
this[]) were included in generatedIObjectAccessorswitch statements, producing invalid code likecase "this": v.this[] = ...Setmethod generation, butinitaccessors cannot be set after constructionSolution
IPropertySymbol.IsIndexerproperties inSerializableSyntaxReceiverSetMethod.IsInitOnlyis true inObjectAccessorFileGeneratorandStaticTypeInspectorFile<auto-generated>header and#pragma warning disable CS1591to the generated source file so analyzers properly skip itChanges
SerializableSyntaxReceiver.cs- Skip indexer propertiesObjectAccessorFileGenerator.cs- Skip init-only settersStaticTypeInspectorFile.cs- Mark init-only properties as read-onlyTypeFactoryGenerator.cs- Add auto-generated header and CS1591 pragma