From 1eb86bce020ef8dc2a9b90c421b997a2def82c41 Mon Sep 17 00:00:00 2001 From: Jon-Luke Biddle Date: Tue, 17 Feb 2026 16:03:34 -0700 Subject: [PATCH 1/6] Refactor to Roslyn for code formatting, remove JetBrains cleanup - Replace regex/string manipulation in CodePostProcessor with Roslyn syntax tree APIs - Add Microsoft.CodeAnalysis.CSharp package for Roslyn support - Use NormalizeWhitespace() for consistent code formatting - Remove JetBrainsCleanupRunner.cs (no longer needed) - Remove --no-cleanup CLI option - Remove JetBrains constants from Constants.cs Benefits: - More robust code transformations (semantic understanding vs regex) - Built-in formatting without external tool dependency - Cleaner, more maintainable code (~130 lines reduced) --- DotSchema/CodePostProcessor.cs | 407 ++++++++---------------- DotSchema/CommandLineOptions.cs | 11 - DotSchema/Constants.cs | 17 - DotSchema/DotSchema.csproj | 1 + DotSchema/Generators/SchemaGenerator.cs | 6 - DotSchema/JetBrainsCleanupRunner.cs | 127 -------- 6 files changed, 139 insertions(+), 430 deletions(-) delete mode 100644 DotSchema/JetBrainsCleanupRunner.cs diff --git a/DotSchema/CodePostProcessor.cs b/DotSchema/CodePostProcessor.cs index 17fd24c..1e48b79 100644 --- a/DotSchema/CodePostProcessor.cs +++ b/DotSchema/CodePostProcessor.cs @@ -1,29 +1,15 @@ -using System.Text; -using System.Text.RegularExpressions; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; namespace DotSchema; /// /// Post-processes generated C# code to clean up and transform type definitions. -/// Uses compiled regex patterns for better performance. +/// Uses Roslyn syntax tree APIs for robust code transformations. /// -public static partial class CodePostProcessor +public static class CodePostProcessor { - // Compiled regex patterns for performance - [GeneratedRegex( - @"//----------------------\s*\n// .*?\s*\n//----------------------\s*\n", - RegexOptions.Singleline)] - private static partial Regex AutoGeneratedCommentRegex(); - - [GeneratedRegex( - @"\s*private System\.Collections\.Generic\.IDictionary\? _additionalProperties;\s*" - + @"\[System\.Text\.Json\.Serialization\.JsonExtensionData\]\s*" - + @"public System\.Collections\.Generic\.IDictionary AdditionalProperties\s*" - + @"\{\s*get \{ return _additionalProperties \?\? \(_additionalProperties = new System\.Collections\.Generic\.Dictionary\(\)\); \}\s*" - + @"set \{ _additionalProperties = value; \}\s*\}", - RegexOptions.Singleline)] - private static partial Regex AdditionalPropertiesRegex(); - /// /// Processes generated code based on the generation mode. /// Note: Type renaming (RootType -> {Variant}RootType) is now handled by CleanTypeNameGenerator @@ -39,352 +25,235 @@ public static string Process( string rootTypeName, bool generateInterface = true) { - return mode switch + var tree = CSharpSyntaxTree.ParseText(code); + var root = tree.GetCompilationUnitRoot(); + + root = mode switch { - GenerationMode.Shared => CleanupSharedCode(code, variantTypes, conflictingTypes, rootTypeName), - GenerationMode.Variant => CleanupVariantCode(code, sharedTypes, variant, rootTypeName, generateInterface), - _ => CleanupAllCode(code) + GenerationMode.Shared => CleanupSharedCode(root, variantTypes, conflictingTypes, rootTypeName), + GenerationMode.Variant => CleanupVariantCode(root, sharedTypes, variant, rootTypeName, generateInterface), + _ => CleanupAllCode(root) }; + + return root.NormalizeWhitespace().ToFullString(); } - private static string CleanupSharedCode( - string code, + private static CompilationUnitSyntax CleanupSharedCode( + CompilationUnitSyntax root, IReadOnlySet variantSpecificTypes, IReadOnlySet conflictingTypes, string rootTypeName) { - code = RemoveAdditionalPropertiesBoilerplate(code); - - // Collect all types to remove in a single set for single-pass removal + // Collect all types to remove var typesToRemove = new HashSet(variantSpecificTypes); typesToRemove.UnionWith(conflictingTypes); typesToRemove.Add(rootTypeName); // Remove the root type class (it's variant-specific) - // Remove all types in a single pass - code = RemoveTypesBatch(code, typesToRemove); - - // Make all classes sealed - code = MakeClassesSealed(code); + root = RemoveAdditionalPropertiesBoilerplate(root); + root = RemoveTypes(root, typesToRemove); + root = MakeClassesSealed(root); - return code; + return root; } - private static string CleanupVariantCode( - string code, + private static CompilationUnitSyntax CleanupVariantCode( + CompilationUnitSyntax root, IReadOnlySet sharedTypes, string variant, string rootTypeName, bool generateInterface) { - code = RemoveAdditionalPropertiesBoilerplate(code); - - // Remove shared types from variant-specific code in a single pass - code = RemoveTypesBatch(code, sharedTypes); + root = RemoveAdditionalPropertiesBoilerplate(root); + root = RemoveTypes(root, sharedTypes); // Add I{RootType} interface to the root variant config class (if enabled) if (generateInterface && !string.IsNullOrEmpty(variant)) { - code = AddRootTypeInterface(code, variant, rootTypeName); + var configClassName = $"{variant}{rootTypeName}"; + var interfaceName = Constants.GetInterfaceName(rootTypeName); + root = AddInterfaceToClass(root, configClassName, interfaceName); } - // Make all classes sealed - code = MakeClassesSealed(code); + root = MakeClassesSealed(root); - return code; + return root; } - /// - /// Converts partial classes to sealed classes, except for base classes that are inherited from. - /// Generated DTO classes should not be inherited from unless they are base types. - /// - private static string MakeClassesSealed(string code) + private static CompilationUnitSyntax CleanupAllCode(CompilationUnitSyntax root) { - // Find all base classes (classes that are inherited from) - // Pattern matches: "class SomeClass : BaseClassName" or "class SomeClass : BaseClassName," - var baseClasses = InheritanceRegex() - .Matches(code) - .Select(m => m.Groups[1].Value) - .ToHashSet(); - - // Replace "public partial class ClassName" with "public sealed class ClassName" - // but only if ClassName is not a base class - return ClassDeclarationRegex() - .Replace( - code, - match => - { - var className = match.Groups[1].Value; + root = RemoveAdditionalPropertiesBoilerplate(root); + root = MakeClassesSealed(root); - return baseClasses.Contains(className) - ? match.Value // Keep as partial class (base class) - : $"public sealed class {className}"; - }); + return root; } - [GeneratedRegex(@"class\s+\w+\s*:\s*(\w+)")] - private static partial Regex InheritanceRegex(); - - [GeneratedRegex(@"public partial class (\w+)")] - private static partial Regex ClassDeclarationRegex(); - /// - /// Adds the I{RootType} interface to the root variant config class. - /// Transforms: "public partial class WindowsConfig" -> "public partial class WindowsConfig : IConfig" + /// Removes types (classes and enums) by name from the syntax tree. /// - private static string AddRootTypeInterface(string code, string variant, string rootTypeName) + private static CompilationUnitSyntax RemoveTypes(CompilationUnitSyntax root, IReadOnlySet typeNames) { - var configClassName = $"{variant}{rootTypeName}"; - var interfaceName = Constants.GetInterfaceName(rootTypeName); - - // NJsonSchema generates "public partial class {Name}", so check for that first - var classDeclaration = $"public partial class {configClassName}"; - var classIndex = code.IndexOf(classDeclaration, StringComparison.Ordinal); - - // Fall back to non-partial class if not found - if (classIndex == -1) - { - classDeclaration = $"public class {configClassName}"; - classIndex = code.IndexOf(classDeclaration, StringComparison.Ordinal); - } - - if (classIndex == -1) + if (typeNames.Count == 0) { - return code; + return root; } - // Check if it already has an inheritance (: something) - var afterDeclaration = classIndex + classDeclaration.Length; - var nextNonWhitespace = afterDeclaration; - - while (nextNonWhitespace < code.Length && char.IsWhiteSpace(code[nextNonWhitespace])) - { - nextNonWhitespace++; - } + var typesToRemove = root.DescendantNodes() + .OfType() + .Where(t => typeNames.Contains(t.Identifier.Text)) + .ToList(); - if (nextNonWhitespace < code.Length && code[nextNonWhitespace] == ':') - { - // Already has inheritance, just add interface to the list - // Find the position after the colon and any whitespace - var colonPos = nextNonWhitespace + 1; - - while (colonPos < code.Length && char.IsWhiteSpace(code[colonPos])) - { - colonPos++; - } - - // Insert "{InterfaceName}, " before the existing base type - return code[..colonPos] + $"{interfaceName}, " + code[colonPos..]; - } - - // No inheritance - add ": {InterfaceName}" after the class name - return code[..afterDeclaration] + $" : {interfaceName}" + code[afterDeclaration..]; + return root.RemoveNodes(typesToRemove, SyntaxRemoveOptions.KeepNoTrivia) + ?? root; } - private static string CleanupAllCode(string code) + /// + /// Removes the AdditionalProperties boilerplate (field and property) from all classes. + /// + private static CompilationUnitSyntax RemoveAdditionalPropertiesBoilerplate(CompilationUnitSyntax root) { - code = RemoveAdditionalPropertiesBoilerplate(code); - - // Make all classes sealed - code = MakeClassesSealed(code); + var rewriter = new RemoveAdditionalPropertiesRewriter(); - return code; + return (CompilationUnitSyntax) rewriter.Visit(root); } - private static string RemoveAdditionalPropertiesBoilerplate(string code) + /// + /// Converts partial classes to sealed classes, except for base classes that are inherited from. + /// + private static CompilationUnitSyntax MakeClassesSealed(CompilationUnitSyntax root) { - // Remove the comment block so JetBrains cleanup will process the file - code = AutoGeneratedCommentRegex().Replace(code, ""); - - return AdditionalPropertiesRegex().Replace(code, ""); + // Find all base class names (classes that are inherited from) + var baseClasses = root.DescendantNodes() + .OfType() + .Where(c => c.BaseList != null) + .SelectMany(c => c.BaseList!.Types) + .Select(t => t.Type) + .OfType() + .Select(i => i.Identifier.Text) + .ToHashSet(); + + var rewriter = new SealClassesRewriter(baseClasses); + + return (CompilationUnitSyntax) rewriter.Visit(root); } /// - /// Removes all specified types from the code in a single pass by finding all type boundaries - /// first, then building the result string without the removed sections. + /// Adds an interface to a specific class by name. /// - private static string RemoveTypesBatch(string code, IReadOnlySet typeNames) + private static CompilationUnitSyntax AddInterfaceToClass( + CompilationUnitSyntax root, + string className, + string interfaceName) { - if (typeNames.Count == 0) - { - return code; - } + var rewriter = new AddInterfaceRewriter(className, interfaceName); - // Find all type definition ranges to remove - var rangesToRemove = new List<(int Start, int End)>(); + return (CompilationUnitSyntax) rewriter.Visit(root); + } - foreach (var typeName in typeNames) + /// + /// Rewriter that removes AdditionalProperties field and property from classes. + /// + private sealed class RemoveAdditionalPropertiesRewriter : CSharpSyntaxRewriter + { + public override SyntaxNode? VisitFieldDeclaration(FieldDeclarationSyntax node) { - // Find as both class and enum - FindTypeRanges(code, typeName, "public partial class ", rangesToRemove); - FindTypeRanges(code, typeName, "public enum ", rangesToRemove); - } + // Remove _additionalProperties field + if (node.Declaration.Variables.Any(v => v.Identifier.Text == "_additionalProperties")) + { + return null; + } - if (rangesToRemove.Count == 0) - { - return code; + return base.VisitFieldDeclaration(node); } - // Sort ranges by start position and merge overlapping ranges - rangesToRemove.Sort((a, b) => a.Start.CompareTo(b.Start)); - var mergedRanges = MergeOverlappingRanges(rangesToRemove); - - // Build result string by copying non-removed sections - var result = new StringBuilder(code.Length); - var currentPos = 0; - - foreach (var (start, end) in mergedRanges) + public override SyntaxNode? VisitPropertyDeclaration(PropertyDeclarationSyntax node) { - if (start > currentPos) + // Remove AdditionalProperties property + if (node.Identifier.Text == "AdditionalProperties") { - result.Append(code, currentPos, start - currentPos); + return null; } - currentPos = end; - } - - // Append remaining code after last removal - if (currentPos < code.Length) - { - result.Append(code, currentPos, code.Length - currentPos); + return base.VisitPropertyDeclaration(node); } - - return result.ToString(); } /// - /// Finds all occurrences of a type definition and adds their ranges to the list. + /// Rewriter that converts partial classes to sealed classes (except base classes). /// - private static void FindTypeRanges( - string code, - string typeName, - string typeKeyword, - List<(int Start, int End)> ranges) + private sealed class SealClassesRewriter(IReadOnlySet baseClasses) : CSharpSyntaxRewriter { - var searchPattern = typeKeyword + typeName; - var searchStart = 0; - - while (searchStart < code.Length) + public override SyntaxNode? VisitClassDeclaration(ClassDeclarationSyntax node) { - var typeIndex = code.IndexOf(searchPattern, searchStart, StringComparison.Ordinal); + var visited = (ClassDeclarationSyntax) base.VisitClassDeclaration(node)!; - if (typeIndex == -1) + // Don't seal base classes + if (baseClasses.Contains(visited.Identifier.Text)) { - break; + return visited; } - // Make sure we're matching the exact type name (not a prefix) - var afterName = typeIndex + searchPattern.Length; - - if (afterName < code.Length) + // Check if already sealed + if (visited.Modifiers.Any(SyntaxKind.SealedKeyword)) { - var nextChar = code[afterName]; + return visited; + } - // Valid next characters: whitespace, newline, colon (for inheritance), open brace - if (char.IsLetterOrDigit(nextChar) || nextChar == '_') - { - // This is a prefix match (e.g., "Config" matching "ConfigMetadata"), skip it - searchStart = typeIndex + 1; + // Remove partial modifier and add sealed + var newModifiers = visited.Modifiers + .Where(m => !m.IsKind(SyntaxKind.PartialKeyword)) + .ToList(); - continue; + // Find position to insert sealed (after public/internal/etc.) + var insertIndex = 0; + + for (var i = 0; i < newModifiers.Count; i++) + { + if (newModifiers[i].IsKind(SyntaxKind.PublicKeyword) + || newModifiers[i].IsKind(SyntaxKind.InternalKeyword) + || newModifiers[i].IsKind(SyntaxKind.PrivateKeyword) + || newModifiers[i].IsKind(SyntaxKind.ProtectedKeyword)) + { + insertIndex = i + 1; } } - // Find the full range of this type definition - var range = GetTypeDefinitionRange(code, typeIndex); + newModifiers.Insert(insertIndex, SyntaxFactory.Token(SyntaxKind.SealedKeyword)); - if (range.HasValue) - { - ranges.Add(range.Value); - searchStart = range.Value.End; - } - else - { - searchStart = typeIndex + 1; - } + return visited.WithModifiers(SyntaxFactory.TokenList(newModifiers)); } } /// - /// Gets the full range of a type definition including doc comments and attributes. + /// Rewriter that adds an interface to a specific class. /// - private static (int Start, int End)? GetTypeDefinitionRange(string code, int typeIndex) + private sealed class AddInterfaceRewriter(string className, string interfaceName) : CSharpSyntaxRewriter { - // Find the start of the definition by looking backwards for preceding elements - var definitionStart = typeIndex; - - // Look backwards for [GeneratedCode...] attribute - but only within reasonable distance (200 chars) - var searchStart = Math.Max(0, typeIndex - 200); - var beforeType = code[searchStart..typeIndex]; - var attrIndex = beforeType.LastIndexOf("[System.CodeDom.Compiler.GeneratedCode", StringComparison.Ordinal); - - if (attrIndex != -1) + public override SyntaxNode? VisitClassDeclaration(ClassDeclarationSyntax node) { - definitionStart = searchStart + attrIndex; + var visited = (ClassDeclarationSyntax) base.VisitClassDeclaration(node)!; - // Look backwards for /// doc comment - var docSearchStart = Math.Max(0, definitionStart - 300); - var docCommentSearch = code[docSearchStart..definitionStart].TrimEnd(); - - if (docCommentSearch.EndsWith("", StringComparison.Ordinal)) + if (visited.Identifier.Text != className) { - var summaryIndex = docCommentSearch.LastIndexOf("/// ", StringComparison.Ordinal); - - if (summaryIndex != -1) - { - definitionStart = docSearchStart + summaryIndex; - } + return visited; } - } - - // Find the closing brace of the type - use brace counting - var braceStart = code.IndexOf('{', typeIndex); - - if (braceStart == -1) - { - return null; - } - var braceCount = 1; - var definitionEnd = braceStart + 1; + // Create the interface type + var interfaceType = SyntaxFactory.SimpleBaseType(SyntaxFactory.IdentifierName(interfaceName)); - while (definitionEnd < code.Length && braceCount > 0) - { - if (code[definitionEnd] == '{') - { - braceCount++; - } - else if (code[definitionEnd] == '}') + // Add to base list + if (visited.BaseList == null) { - braceCount--; - } - - definitionEnd++; - } + // No existing base list - create one + var baseList = SyntaxFactory.BaseList( + SyntaxFactory.SingletonSeparatedList(interfaceType)); - return (definitionStart, definitionEnd); - } + return visited.WithBaseList(baseList); + } - /// - /// Merges overlapping or adjacent ranges into a single list of non-overlapping ranges. - /// - private static List<(int Start, int End)> MergeOverlappingRanges(List<(int Start, int End)> sortedRanges) - { - var merged = new List<(int Start, int End)>(); + // Existing base list - prepend interface + var newTypes = visited.BaseList.Types.Insert(0, interfaceType); - foreach (var range in sortedRanges) - { - if (merged.Count == 0 || merged[^1].End < range.Start) - { - merged.Add(range); - } - else - { - // Extend the last range if overlapping - var last = merged[^1]; - merged[^1] = (last.Start, Math.Max(last.End, range.End)); - } + return visited.WithBaseList(visited.BaseList.WithTypes(newTypes)); } - - return merged; } } diff --git a/DotSchema/CommandLineOptions.cs b/DotSchema/CommandLineOptions.cs index 51e18d5..bf33c27 100644 --- a/DotSchema/CommandLineOptions.cs +++ b/DotSchema/CommandLineOptions.cs @@ -49,12 +49,6 @@ public sealed record GenerateOptions HelpText = "Skip generating the marker interface (e.g., IConfig) that all variant types implement.")] public bool NoInterface { get; init; } = false; - [Option( - "no-cleanup", - Default = false, - HelpText = "Skip running JetBrains code cleanup on generated files.")] - public bool NoCleanup { get; init; } = false; - [Option( "verbose", Default = false, @@ -79,11 +73,6 @@ public sealed record GenerateOptions /// public bool GenerateInterface => !NoInterface; - /// - /// Gets whether to run JetBrains cleanup. - /// - public bool RunCleanup => !NoCleanup; - /// /// Gets the schema file paths as a list. /// diff --git a/DotSchema/Constants.cs b/DotSchema/Constants.cs index af4546b..f054151 100644 --- a/DotSchema/Constants.cs +++ b/DotSchema/Constants.cs @@ -145,21 +145,4 @@ private static string ToPascalCase(string name) return char.ToUpperInvariant(name[0]) + name[1..].ToLowerInvariant(); } - - /// - /// Constants for JetBrains cleanup tool. - /// - public static class JetBrains - { - public const string DotnetExecutable = "dotnet"; - public const string CleanupProfile = "Built-in: Full Cleanup"; - } - - /// - /// Constants for file patterns. - /// - public static class FilePatterns - { - public const string SolutionPattern = "*.sln"; - } } diff --git a/DotSchema/DotSchema.csproj b/DotSchema/DotSchema.csproj index cfc3dc5..ced97bb 100644 --- a/DotSchema/DotSchema.csproj +++ b/DotSchema/DotSchema.csproj @@ -22,6 +22,7 @@ + diff --git a/DotSchema/Generators/SchemaGenerator.cs b/DotSchema/Generators/SchemaGenerator.cs index 8938498..b50c9c7 100644 --- a/DotSchema/Generators/SchemaGenerator.cs +++ b/DotSchema/Generators/SchemaGenerator.cs @@ -38,12 +38,6 @@ public static async Task RunAsync( return result; } - // Run JetBrains cleanup on all generated files at the end (if enabled) - if (options.RunCleanup) - { - await JetBrainsCleanupRunner.RunAsync(generatedFiles, logger, cancellationToken).ConfigureAwait(false); - } - logger.LogInformation("Done!"); return 0; diff --git a/DotSchema/JetBrainsCleanupRunner.cs b/DotSchema/JetBrainsCleanupRunner.cs deleted file mode 100644 index afc5917..0000000 --- a/DotSchema/JetBrainsCleanupRunner.cs +++ /dev/null @@ -1,127 +0,0 @@ -using System.Diagnostics; - -using Microsoft.Extensions.Logging; - -namespace DotSchema; - -/// -/// Runs JetBrains cleanup code tool on generated files. -/// -public static class JetBrainsCleanupRunner -{ - /// - /// Runs JetBrains cleanup on the specified files. - /// - public static async Task RunAsync( - IReadOnlyList filePaths, - ILogger logger, - CancellationToken cancellationToken = default) - { - if (filePaths.Count == 0) - { - return; - } - - var absolutePaths = filePaths.Select(Path.GetFullPath).ToList(); - - // Find solution directory by walking up from the first file - var (solutionDir, slnFile) = FindSolutionDirectory(absolutePaths[0]); - - if (solutionDir == null || slnFile == null) - { - logger.LogWarning("Could not find solution directory containing .sln file"); - - return; - } - - // jb cleanupcode needs relative paths for --include, semicolon-separated - var relativePaths = absolutePaths - .Select(p => Path.GetRelativePath(solutionDir, p)) - .ToList(); - - var includePattern = string.Join(";", relativePaths); - - var processInfo = new ProcessStartInfo - { - FileName = Constants.JetBrains.DotnetExecutable, - Arguments = - $"tool run jb cleanupcode --profile=\"{Constants.JetBrains.CleanupProfile}\" --include=\"{includePattern}\" \"{slnFile}\"", - RedirectStandardOutput = true, - RedirectStandardError = true, - UseShellExecute = false, - CreateNoWindow = true, - WorkingDirectory = solutionDir - }; - - try - { - logger.LogInformation("Running jb cleanupcode on {FileCount} file(s)...", filePaths.Count); - logger.LogDebug("Command: {FileName} {Arguments}", processInfo.FileName, processInfo.Arguments); - - using var process = Process.Start(processInfo); - - if (process != null) - { - process.OutputDataReceived += (_, e) => - { - if (e.Data != null) - { - logger.LogDebug("{Output}", e.Data); - } - }; - - process.ErrorDataReceived += (_, e) => - { - if (e.Data != null) - { - logger.LogDebug("{Error}", e.Data); - } - }; - - process.BeginOutputReadLine(); - process.BeginErrorReadLine(); - - await process.WaitForExitAsync(cancellationToken).ConfigureAwait(false); - - if (process.ExitCode != 0) - { - logger.LogWarning("jb cleanupcode exit code: {ExitCode}", process.ExitCode); - } - } - } - catch (OperationCanceledException) - { - logger.LogWarning("jb cleanupcode was cancelled"); - - throw; - } - catch (Exception ex) - { - // jb tool not available, skip formatting - logger.LogWarning("jb cleanupcode failed: {Message}", ex.Message); - } - } - - /// - /// Walks up the directory tree from the starting path to find a directory containing a .sln file. - /// - /// A tuple of (solution directory, solution file path) or (null, null) if not found. - private static (string? SolutionDir, string? SlnFile) FindSolutionDirectory(string startPath) - { - var dir = Path.GetDirectoryName(startPath); - - while (dir != null) - { - var slnFiles = Directory.GetFiles(dir, Constants.FilePatterns.SolutionPattern); - - if (slnFiles.Length > 0) - { - return (dir, slnFiles[0]); - } - - dir = Path.GetDirectoryName(dir); - } - - return (null, null); - } -} From 0637dc60ff0dde7a56bb039f43f611d853117ad7 Mon Sep 17 00:00:00 2001 From: Jon-Luke Biddle Date: Tue, 17 Feb 2026 16:10:53 -0700 Subject: [PATCH 2/6] Update-README-and-add-tests-for-Roslyn-refactor --- DotSchema.Tests/CodePostProcessorTests.cs | 99 +++++++++++++++++++++++ DotSchema/CodePostProcessor.cs | 11 ++- README.md | 37 +++++++-- 3 files changed, 140 insertions(+), 7 deletions(-) diff --git a/DotSchema.Tests/CodePostProcessorTests.cs b/DotSchema.Tests/CodePostProcessorTests.cs index f4f5bae..e7a870c 100644 --- a/DotSchema.Tests/CodePostProcessorTests.cs +++ b/DotSchema.Tests/CodePostProcessorTests.cs @@ -173,4 +173,103 @@ public partial class MyType Assert.Contains("public sealed class MyType", result); Assert.DoesNotContain("public partial class MyType", result); } + + [Fact] + public void Process_RemovesAdditionalPropertiesBoilerplate() + { + var code = """ + namespace Test; + + public partial class MyType + { + public string Name { get; set; } + + private System.Collections.Generic.IDictionary? _additionalProperties; + + [System.Text.Json.Serialization.JsonExtensionData] + public System.Collections.Generic.IDictionary AdditionalProperties + { + get { return _additionalProperties ?? (_additionalProperties = new System.Collections.Generic.Dictionary()); } + set { _additionalProperties = value; } + } + } + """; + + var result = CodePostProcessor.Process( + code, + GenerationMode.All, + "", + EmptySet, + EmptySet, + EmptySet, + "Config"); + + Assert.DoesNotContain("_additionalProperties", result); + Assert.DoesNotContain("AdditionalProperties", result); + Assert.DoesNotContain("JsonExtensionData", result); + Assert.Contains("Name", result); + } + + [Fact] + public void Process_PreservesBaseClassesAsNonSealed() + { + var code = """ + namespace Test; + + public partial class BaseType + { + public string Name { get; set; } + } + + public partial class DerivedType : BaseType + { + public int Value { get; set; } + } + """; + + var result = CodePostProcessor.Process( + code, + GenerationMode.All, + "", + EmptySet, + EmptySet, + EmptySet, + "Config"); + + // BaseType should NOT be sealed (it's inherited from) + Assert.DoesNotContain("public sealed class BaseType", result); + + // DerivedType should be sealed + Assert.Contains("public sealed class DerivedType", result); + } + + [Fact] + public void Process_SharedMode_RemovesRootType() + { + var code = """ + namespace Test; + + public partial class SharedType + { + public string Name { get; set; } + } + + public partial class Config + { + public int Value { get; set; } + } + """; + + var result = CodePostProcessor.Process( + code, + GenerationMode.Shared, + "", + EmptySet, + EmptySet, + EmptySet, + "Config"); + + Assert.Contains("SharedType", result); + Assert.DoesNotContain("public sealed class Config", result); + } } diff --git a/DotSchema/CodePostProcessor.cs b/DotSchema/CodePostProcessor.cs index 1e48b79..8347732 100644 --- a/DotSchema/CodePostProcessor.cs +++ b/DotSchema/CodePostProcessor.cs @@ -149,8 +149,11 @@ private static CompilationUnitSyntax AddInterfaceToClass( return (CompilationUnitSyntax) rewriter.Visit(root); } +#region Syntax Rewriters + /// /// Rewriter that removes AdditionalProperties field and property from classes. + /// NJsonSchema generates these by default, but they add unnecessary complexity to DTOs. /// private sealed class RemoveAdditionalPropertiesRewriter : CSharpSyntaxRewriter { @@ -178,7 +181,8 @@ private sealed class RemoveAdditionalPropertiesRewriter : CSharpSyntaxRewriter } /// - /// Rewriter that converts partial classes to sealed classes (except base classes). + /// Rewriter that converts partial classes to sealed classes. + /// Base classes (those inherited from) are preserved as non-sealed to allow inheritance. /// private sealed class SealClassesRewriter(IReadOnlySet baseClasses) : CSharpSyntaxRewriter { @@ -224,7 +228,8 @@ private sealed class SealClassesRewriter(IReadOnlySet baseClasses) : CSh } /// - /// Rewriter that adds an interface to a specific class. + /// Rewriter that adds an interface to a specific class by name. + /// Used to add marker interfaces (e.g., IConfig) to variant root types. /// private sealed class AddInterfaceRewriter(string className, string interfaceName) : CSharpSyntaxRewriter { @@ -256,4 +261,6 @@ private sealed class AddInterfaceRewriter(string className, string interfaceName return visited.WithBaseList(visited.BaseList.WithTypes(newTypes)); } } + +#endregion } diff --git a/README.md b/README.md index 10ae9a8..b1b75a5 100644 --- a/README.md +++ b/README.md @@ -12,11 +12,11 @@ across multiple schema files. - `Shared` - Generates only types that exist in all provided schemas - `Variant` - Generates only types unique to a specific variant - **Automatic interface generation** for variant types -- **JetBrains code cleanup integration** (optional) +- **Roslyn-based code formatting** for clean, consistent output ## Requirements -- .NET 8.0 SDK +- .NET 9.0 SDK or later ## Installation @@ -53,14 +53,16 @@ dotnet run -- [options] ### Options | Option | Short | Required | Description | -|------------------|-------|----------|-----------------------------------------------------------------| +| ---------------- | ----- | -------- | --------------------------------------------------------------- | | `--schemas` | `-s` | Yes | One or more JSON schema files to process | | `--output` | `-o` | Yes | Output file path (Shared/Variant) or directory (All mode) | | `--namespace` | `-n` | Yes | Namespace for generated types | | `--mode` | `-m` | No | Generation mode: `All`, `Shared`, or `Variant` (default: `All`) | | `--variant` | `-v` | No | Variant name for single-variant generation | | `--no-interface` | | No | Skip generating the marker interface | -| `--no-cleanup` | | No | Skip running JetBrains code cleanup | +| `--verbose` | | No | Enable verbose output (debug-level logging) | +| `--quiet` | `-q` | No | Suppress non-error output | +| `--dry-run` | | No | Preview what would be generated without writing files | ### Examples @@ -90,10 +92,35 @@ In `All` mode, the tool generates: - `Shared{RootType}.cs` - Types common to all schemas - `{Variant}{RootType}.cs` - Variant-specific types for each schema +## Architecture + +The codebase is organized into several key components: + +``` +DotSchema/ +├── Program.cs # Entry point, CLI parsing +├── CommandLineOptions.cs # CLI option definitions +├── Constants.cs # Shared constants and naming utilities +├── CodePostProcessor.cs # Roslyn-based code cleanup and transformation +├── Analyzers/ +│ └── SchemaAnalyzer.cs # Detects shared vs variant-specific types +└── Generators/ + ├── SchemaGenerator.cs # Orchestrates code generation + ├── CleanTypeNameGenerator.cs # Type name cleanup + └── PascalCasePropertyNameGenerator.cs # Property name conversion +``` + +**Flow:** + +1. `SchemaAnalyzer` parses all schemas and categorizes types as shared, variant-specific, or conflicting +2. `SchemaGenerator` uses NJsonSchema to generate C# code with custom name generators +3. `CodePostProcessor` uses Roslyn syntax trees to clean up the generated code (seal classes, remove + boilerplate, add interfaces) + ## Dependencies - [CommandLineParser](https://github.com/commandlineparser/commandline) - Command line argument parsing - [NJsonSchema.CodeGeneration.CSharp](https://github.com/RicoSuter/NJsonSchema) - JSON Schema to C# code generation +- [Microsoft.CodeAnalysis.CSharp](https://github.com/dotnet/roslyn) - Roslyn C# syntax tree APIs for code formatting - [Microsoft.Extensions.Logging](https://docs.microsoft.com/en-us/dotnet/core/extensions/logging) - Logging infrastructure - From da9dd5350bb713e194a3e2b6e1463bce4fa21740 Mon Sep 17 00:00:00 2001 From: Jon-Luke Biddle Date: Tue, 17 Feb 2026 16:18:19 -0700 Subject: [PATCH 3/6] Future-proof MakeClassesSealed to handle generic base types Extract GetBaseTypeName helper method that handles both IdentifierNameSyntax (simple base types like BaseClass) and GenericNameSyntax (generic base types like BaseClass) when determining which classes should not be sealed. --- DotSchema/CodePostProcessor.cs | 20 +++++++++++++++++--- README.md | 2 +- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/DotSchema/CodePostProcessor.cs b/DotSchema/CodePostProcessor.cs index 8347732..932072e 100644 --- a/DotSchema/CodePostProcessor.cs +++ b/DotSchema/CodePostProcessor.cs @@ -122,13 +122,13 @@ private static CompilationUnitSyntax RemoveAdditionalPropertiesBoilerplate(Compi private static CompilationUnitSyntax MakeClassesSealed(CompilationUnitSyntax root) { // Find all base class names (classes that are inherited from) + // Handles both simple names (BaseClass) and generic names (BaseClass) var baseClasses = root.DescendantNodes() .OfType() .Where(c => c.BaseList != null) .SelectMany(c => c.BaseList!.Types) - .Select(t => t.Type) - .OfType() - .Select(i => i.Identifier.Text) + .Select(t => GetBaseTypeName(t.Type)) + .OfType() .ToHashSet(); var rewriter = new SealClassesRewriter(baseClasses); @@ -136,6 +136,20 @@ private static CompilationUnitSyntax MakeClassesSealed(CompilationUnitSyntax roo return (CompilationUnitSyntax) rewriter.Visit(root); } + /// + /// Extracts the type name from a base type syntax node. + /// Handles both simple names (BaseClass) and generic names (BaseClass<T>). + /// + private static string? GetBaseTypeName(TypeSyntax type) + { + return type switch + { + IdentifierNameSyntax id => id.Identifier.Text, + GenericNameSyntax gen => gen.Identifier.Text, + _ => null + }; + } + /// /// Adds an interface to a specific class by name. /// diff --git a/README.md b/README.md index b1b75a5..52f5014 100644 --- a/README.md +++ b/README.md @@ -53,7 +53,7 @@ dotnet run -- [options] ### Options | Option | Short | Required | Description | -| ---------------- | ----- | -------- | --------------------------------------------------------------- | +|------------------|-------|----------|-----------------------------------------------------------------| | `--schemas` | `-s` | Yes | One or more JSON schema files to process | | `--output` | `-o` | Yes | Output file path (Shared/Variant) or directory (All mode) | | `--namespace` | `-n` | Yes | Namespace for generated types | From 0ee6b91757299b88034165becee19d205c67f4e4 Mon Sep 17 00:00:00 2001 From: Jon-Luke Biddle Date: Tue, 17 Feb 2026 16:24:03 -0700 Subject: [PATCH 4/6] Refactor: Extract Roslyn rewriters into separate files and fix issues - Extract rewriters into DotSchema/Rewriters/ folder: - AddInterfaceRewriter.cs - RemoveAdditionalPropertiesRewriter.cs - SealClassesRewriter.cs - SyntaxHelpers.cs - Fix trivia issue: sealed keyword now has proper trailing space - Fix QualifiedNameSyntax support in GetBaseTypeName for Namespace.BaseClass patterns - Add AliasQualifiedNameSyntax support for completeness - Reduce CodePostProcessor.cs from 281 to 145 lines All 57 tests pass. --- DotSchema/CodePostProcessor.cs | 141 +----------------- DotSchema/Rewriters/AddInterfaceRewriter.cs | 45 ++++++ .../RemoveAdditionalPropertiesRewriter.cs | 38 +++++ DotSchema/Rewriters/SealClassesRewriter.cs | 57 +++++++ DotSchema/Rewriters/SyntaxHelpers.cs | 42 ++++++ 5 files changed, 185 insertions(+), 138 deletions(-) create mode 100644 DotSchema/Rewriters/AddInterfaceRewriter.cs create mode 100644 DotSchema/Rewriters/RemoveAdditionalPropertiesRewriter.cs create mode 100644 DotSchema/Rewriters/SealClassesRewriter.cs create mode 100644 DotSchema/Rewriters/SyntaxHelpers.cs diff --git a/DotSchema/CodePostProcessor.cs b/DotSchema/CodePostProcessor.cs index 932072e..50cfd54 100644 --- a/DotSchema/CodePostProcessor.cs +++ b/DotSchema/CodePostProcessor.cs @@ -2,6 +2,8 @@ using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; +using DotSchema.Rewriters; + namespace DotSchema; /// @@ -122,34 +124,12 @@ private static CompilationUnitSyntax RemoveAdditionalPropertiesBoilerplate(Compi private static CompilationUnitSyntax MakeClassesSealed(CompilationUnitSyntax root) { // Find all base class names (classes that are inherited from) - // Handles both simple names (BaseClass) and generic names (BaseClass) - var baseClasses = root.DescendantNodes() - .OfType() - .Where(c => c.BaseList != null) - .SelectMany(c => c.BaseList!.Types) - .Select(t => GetBaseTypeName(t.Type)) - .OfType() - .ToHashSet(); - + var baseClasses = SyntaxHelpers.GetBaseClassNames(root); var rewriter = new SealClassesRewriter(baseClasses); return (CompilationUnitSyntax) rewriter.Visit(root); } - /// - /// Extracts the type name from a base type syntax node. - /// Handles both simple names (BaseClass) and generic names (BaseClass<T>). - /// - private static string? GetBaseTypeName(TypeSyntax type) - { - return type switch - { - IdentifierNameSyntax id => id.Identifier.Text, - GenericNameSyntax gen => gen.Identifier.Text, - _ => null - }; - } - /// /// Adds an interface to a specific class by name. /// @@ -162,119 +142,4 @@ private static CompilationUnitSyntax AddInterfaceToClass( return (CompilationUnitSyntax) rewriter.Visit(root); } - -#region Syntax Rewriters - - /// - /// Rewriter that removes AdditionalProperties field and property from classes. - /// NJsonSchema generates these by default, but they add unnecessary complexity to DTOs. - /// - private sealed class RemoveAdditionalPropertiesRewriter : CSharpSyntaxRewriter - { - public override SyntaxNode? VisitFieldDeclaration(FieldDeclarationSyntax node) - { - // Remove _additionalProperties field - if (node.Declaration.Variables.Any(v => v.Identifier.Text == "_additionalProperties")) - { - return null; - } - - return base.VisitFieldDeclaration(node); - } - - public override SyntaxNode? VisitPropertyDeclaration(PropertyDeclarationSyntax node) - { - // Remove AdditionalProperties property - if (node.Identifier.Text == "AdditionalProperties") - { - return null; - } - - return base.VisitPropertyDeclaration(node); - } - } - - /// - /// Rewriter that converts partial classes to sealed classes. - /// Base classes (those inherited from) are preserved as non-sealed to allow inheritance. - /// - private sealed class SealClassesRewriter(IReadOnlySet baseClasses) : CSharpSyntaxRewriter - { - public override SyntaxNode? VisitClassDeclaration(ClassDeclarationSyntax node) - { - var visited = (ClassDeclarationSyntax) base.VisitClassDeclaration(node)!; - - // Don't seal base classes - if (baseClasses.Contains(visited.Identifier.Text)) - { - return visited; - } - - // Check if already sealed - if (visited.Modifiers.Any(SyntaxKind.SealedKeyword)) - { - return visited; - } - - // Remove partial modifier and add sealed - var newModifiers = visited.Modifiers - .Where(m => !m.IsKind(SyntaxKind.PartialKeyword)) - .ToList(); - - // Find position to insert sealed (after public/internal/etc.) - var insertIndex = 0; - - for (var i = 0; i < newModifiers.Count; i++) - { - if (newModifiers[i].IsKind(SyntaxKind.PublicKeyword) - || newModifiers[i].IsKind(SyntaxKind.InternalKeyword) - || newModifiers[i].IsKind(SyntaxKind.PrivateKeyword) - || newModifiers[i].IsKind(SyntaxKind.ProtectedKeyword)) - { - insertIndex = i + 1; - } - } - - newModifiers.Insert(insertIndex, SyntaxFactory.Token(SyntaxKind.SealedKeyword)); - - return visited.WithModifiers(SyntaxFactory.TokenList(newModifiers)); - } - } - - /// - /// Rewriter that adds an interface to a specific class by name. - /// Used to add marker interfaces (e.g., IConfig) to variant root types. - /// - private sealed class AddInterfaceRewriter(string className, string interfaceName) : CSharpSyntaxRewriter - { - public override SyntaxNode? VisitClassDeclaration(ClassDeclarationSyntax node) - { - var visited = (ClassDeclarationSyntax) base.VisitClassDeclaration(node)!; - - if (visited.Identifier.Text != className) - { - return visited; - } - - // Create the interface type - var interfaceType = SyntaxFactory.SimpleBaseType(SyntaxFactory.IdentifierName(interfaceName)); - - // Add to base list - if (visited.BaseList == null) - { - // No existing base list - create one - var baseList = SyntaxFactory.BaseList( - SyntaxFactory.SingletonSeparatedList(interfaceType)); - - return visited.WithBaseList(baseList); - } - - // Existing base list - prepend interface - var newTypes = visited.BaseList.Types.Insert(0, interfaceType); - - return visited.WithBaseList(visited.BaseList.WithTypes(newTypes)); - } - } - -#endregion } diff --git a/DotSchema/Rewriters/AddInterfaceRewriter.cs b/DotSchema/Rewriters/AddInterfaceRewriter.cs new file mode 100644 index 0000000..bee3b6e --- /dev/null +++ b/DotSchema/Rewriters/AddInterfaceRewriter.cs @@ -0,0 +1,45 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace DotSchema.Rewriters; + +/// +/// Rewriter that adds an interface to a specific class by name. +/// Used to add marker interfaces (e.g., IConfig) to variant root types. +/// +internal sealed class AddInterfaceRewriter(string className, string interfaceName) : CSharpSyntaxRewriter +{ + public override SyntaxNode? VisitClassDeclaration(ClassDeclarationSyntax node) + { + var visited = (ClassDeclarationSyntax) base.VisitClassDeclaration(node)!; + + if (visited.Identifier.Text != className) + { + return visited; + } + + // Create the interface type + var interfaceType = SyntaxFactory.SimpleBaseType(SyntaxFactory.IdentifierName(interfaceName)); + + // Add to base list + if (visited.BaseList == null) + { + // No existing base list - create one with proper spacing + var baseList = SyntaxFactory.BaseList( + SyntaxFactory.SingletonSeparatedList(interfaceType)) + .WithColonToken( + SyntaxFactory.Token(SyntaxKind.ColonToken) + .WithLeadingTrivia(SyntaxFactory.Space) + .WithTrailingTrivia(SyntaxFactory.Space)); + + return visited.WithBaseList(baseList); + } + + // Existing base list - prepend interface + var newTypes = visited.BaseList.Types.Insert(0, interfaceType); + + return visited.WithBaseList(visited.BaseList.WithTypes(newTypes)); + } +} + diff --git a/DotSchema/Rewriters/RemoveAdditionalPropertiesRewriter.cs b/DotSchema/Rewriters/RemoveAdditionalPropertiesRewriter.cs new file mode 100644 index 0000000..d05404f --- /dev/null +++ b/DotSchema/Rewriters/RemoveAdditionalPropertiesRewriter.cs @@ -0,0 +1,38 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace DotSchema.Rewriters; + +/// +/// Rewriter that removes AdditionalProperties field and property from classes. +/// NJsonSchema generates these by default, but they add unnecessary complexity to DTOs. +/// +internal sealed class RemoveAdditionalPropertiesRewriter : CSharpSyntaxRewriter +{ + private const string AdditionalPropertiesFieldName = "_additionalProperties"; + private const string AdditionalPropertiesPropertyName = "AdditionalProperties"; + + public override SyntaxNode? VisitFieldDeclaration(FieldDeclarationSyntax node) + { + // Remove _additionalProperties field + if (node.Declaration.Variables.Any(v => v.Identifier.Text == AdditionalPropertiesFieldName)) + { + return null; + } + + return base.VisitFieldDeclaration(node); + } + + public override SyntaxNode? VisitPropertyDeclaration(PropertyDeclarationSyntax node) + { + // Remove AdditionalProperties property + if (node.Identifier.Text == AdditionalPropertiesPropertyName) + { + return null; + } + + return base.VisitPropertyDeclaration(node); + } +} + diff --git a/DotSchema/Rewriters/SealClassesRewriter.cs b/DotSchema/Rewriters/SealClassesRewriter.cs new file mode 100644 index 0000000..17483f2 --- /dev/null +++ b/DotSchema/Rewriters/SealClassesRewriter.cs @@ -0,0 +1,57 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace DotSchema.Rewriters; + +/// +/// Rewriter that converts partial classes to sealed classes. +/// Base classes (those inherited from) are preserved as non-sealed to allow inheritance. +/// +internal sealed class SealClassesRewriter(IReadOnlySet baseClasses) : CSharpSyntaxRewriter +{ + public override SyntaxNode? VisitClassDeclaration(ClassDeclarationSyntax node) + { + var visited = (ClassDeclarationSyntax) base.VisitClassDeclaration(node)!; + + // Don't seal base classes + if (baseClasses.Contains(visited.Identifier.Text)) + { + return visited; + } + + // Check if already sealed + if (visited.Modifiers.Any(SyntaxKind.SealedKeyword)) + { + return visited; + } + + // Remove partial modifier and add sealed + var newModifiers = visited.Modifiers + .Where(m => !m.IsKind(SyntaxKind.PartialKeyword)) + .ToList(); + + // Find position to insert sealed (after public/internal/etc.) + var insertIndex = 0; + + for (var i = 0; i < newModifiers.Count; i++) + { + if (newModifiers[i].IsKind(SyntaxKind.PublicKeyword) + || newModifiers[i].IsKind(SyntaxKind.InternalKeyword) + || newModifiers[i].IsKind(SyntaxKind.PrivateKeyword) + || newModifiers[i].IsKind(SyntaxKind.ProtectedKeyword)) + { + insertIndex = i + 1; + } + } + + // Create sealed keyword with proper trivia (space before the class keyword) + var sealedKeyword = SyntaxFactory.Token(SyntaxKind.SealedKeyword) + .WithTrailingTrivia(SyntaxFactory.Space); + + newModifiers.Insert(insertIndex, sealedKeyword); + + return visited.WithModifiers(SyntaxFactory.TokenList(newModifiers)); + } +} + diff --git a/DotSchema/Rewriters/SyntaxHelpers.cs b/DotSchema/Rewriters/SyntaxHelpers.cs new file mode 100644 index 0000000..411c18a --- /dev/null +++ b/DotSchema/Rewriters/SyntaxHelpers.cs @@ -0,0 +1,42 @@ +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace DotSchema.Rewriters; + +/// +/// Helper methods for working with Roslyn syntax nodes. +/// +internal static class SyntaxHelpers +{ + /// + /// Extracts the type name from a base type syntax node. + /// Handles simple names (BaseClass), generic names (BaseClass<T>), + /// and qualified names (Namespace.BaseClass). + /// + public static string? GetBaseTypeName(TypeSyntax type) + { + return type switch + { + IdentifierNameSyntax id => id.Identifier.Text, + GenericNameSyntax gen => gen.Identifier.Text, + QualifiedNameSyntax qualified => GetBaseTypeName(qualified.Right), + AliasQualifiedNameSyntax alias => GetBaseTypeName(alias.Name), + _ => null + }; + } + + /// + /// Gets all base class names from a compilation unit. + /// These are classes that other classes inherit from. + /// + public static HashSet GetBaseClassNames(CompilationUnitSyntax root) + { + return root.DescendantNodes() + .OfType() + .Where(c => c.BaseList != null) + .SelectMany(c => c.BaseList!.Types) + .Select(t => GetBaseTypeName(t.Type)) + .OfType() + .ToHashSet(); + } +} + From 8d3b63d586f702c6a30f6ce2844c200dc68b34c3 Mon Sep 17 00:00:00 2001 From: Jon-Luke Biddle Date: Tue, 17 Feb 2026 16:31:45 -0700 Subject: [PATCH 5/6] Address-code-review-feedback --- DotSchema/CodePostProcessor.cs | 4 ++-- DotSchema/Rewriters/AddInterfaceRewriter.cs | 4 ++-- DotSchema/Rewriters/RemoveAdditionalPropertiesRewriter.cs | 2 ++ DotSchema/Rewriters/SyntaxHelpers.cs | 6 +++--- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/DotSchema/CodePostProcessor.cs b/DotSchema/CodePostProcessor.cs index 50cfd54..60c959b 100644 --- a/DotSchema/CodePostProcessor.cs +++ b/DotSchema/CodePostProcessor.cs @@ -123,8 +123,8 @@ private static CompilationUnitSyntax RemoveAdditionalPropertiesBoilerplate(Compi /// private static CompilationUnitSyntax MakeClassesSealed(CompilationUnitSyntax root) { - // Find all base class names (classes that are inherited from) - var baseClasses = SyntaxHelpers.GetBaseClassNames(root); + // Find all base type names (classes/interfaces that are inherited from or implemented) + var baseClasses = SyntaxHelpers.GetBaseTypeNames(root); var rewriter = new SealClassesRewriter(baseClasses); return (CompilationUnitSyntax) rewriter.Visit(root); diff --git a/DotSchema/Rewriters/AddInterfaceRewriter.cs b/DotSchema/Rewriters/AddInterfaceRewriter.cs index bee3b6e..9f4b7db 100644 --- a/DotSchema/Rewriters/AddInterfaceRewriter.cs +++ b/DotSchema/Rewriters/AddInterfaceRewriter.cs @@ -36,8 +36,8 @@ internal sealed class AddInterfaceRewriter(string className, string interfaceNam return visited.WithBaseList(baseList); } - // Existing base list - prepend interface - var newTypes = visited.BaseList.Types.Insert(0, interfaceType); + // Existing base list - append interface (C# convention: base class first, then interfaces) + var newTypes = visited.BaseList.Types.Add(interfaceType); return visited.WithBaseList(visited.BaseList.WithTypes(newTypes)); } diff --git a/DotSchema/Rewriters/RemoveAdditionalPropertiesRewriter.cs b/DotSchema/Rewriters/RemoveAdditionalPropertiesRewriter.cs index d05404f..ef19410 100644 --- a/DotSchema/Rewriters/RemoveAdditionalPropertiesRewriter.cs +++ b/DotSchema/Rewriters/RemoveAdditionalPropertiesRewriter.cs @@ -7,6 +7,8 @@ namespace DotSchema.Rewriters; /// /// Rewriter that removes AdditionalProperties field and property from classes. /// NJsonSchema generates these by default, but they add unnecessary complexity to DTOs. +/// Note: Associated attributes (e.g., [JsonExtensionData]) are implicitly removed +/// when the property is removed, as they are part of the property's syntax node. /// internal sealed class RemoveAdditionalPropertiesRewriter : CSharpSyntaxRewriter { diff --git a/DotSchema/Rewriters/SyntaxHelpers.cs b/DotSchema/Rewriters/SyntaxHelpers.cs index 411c18a..8b39547 100644 --- a/DotSchema/Rewriters/SyntaxHelpers.cs +++ b/DotSchema/Rewriters/SyntaxHelpers.cs @@ -25,10 +25,10 @@ internal static class SyntaxHelpers } /// - /// Gets all base class names from a compilation unit. - /// These are classes that other classes inherit from. + /// Gets all base type names (classes and interfaces) from a compilation unit. + /// These are types that other classes inherit from or implement. /// - public static HashSet GetBaseClassNames(CompilationUnitSyntax root) + public static HashSet GetBaseTypeNames(CompilationUnitSyntax root) { return root.DescendantNodes() .OfType() From 912d6b8be52d498818f923e971f90061676f48c6 Mon Sep 17 00:00:00 2001 From: Jon-Luke Biddle Date: Tue, 17 Feb 2026 16:47:40 -0700 Subject: [PATCH 6/6] Refactor-rewriters-extract-SyntaxHelpers-improve-handling --- DotSchema.Tests/CodePostProcessorTests.cs | 70 +++++++++++++++++++ DotSchema/CodePostProcessor.cs | 7 +- DotSchema/Rewriters/AddInterfaceRewriter.cs | 12 ++-- .../RemoveAdditionalPropertiesRewriter.cs | 1 - DotSchema/Rewriters/SealClassesRewriter.cs | 53 +++++++------- DotSchema/Rewriters/SyntaxHelpers.cs | 6 +- 6 files changed, 109 insertions(+), 40 deletions(-) diff --git a/DotSchema.Tests/CodePostProcessorTests.cs b/DotSchema.Tests/CodePostProcessorTests.cs index e7a870c..26a79f4 100644 --- a/DotSchema.Tests/CodePostProcessorTests.cs +++ b/DotSchema.Tests/CodePostProcessorTests.cs @@ -272,4 +272,74 @@ public partial class Config Assert.Contains("SharedType", result); Assert.DoesNotContain("public sealed class Config", result); } + + [Fact] + public void Process_DoesNotSealAbstractClasses() + { + var code = """ + namespace Test; + + public abstract partial class AbstractBase + { + public abstract string Name { get; set; } + } + + public partial class ConcreteType : AbstractBase + { + public override string Name { get; set; } + } + """; + + var result = CodePostProcessor.Process( + code, + GenerationMode.All, + "", + EmptySet, + EmptySet, + EmptySet, + "Config"); + + // AbstractBase should remain abstract (not sealed) + Assert.Contains("public abstract class AbstractBase", result); + Assert.DoesNotContain("sealed abstract", result); + + // ConcreteType should NOT be sealed either (it's a base class for AbstractBase inheritance) + // Actually, ConcreteType inherits from AbstractBase, so AbstractBase is the base class + // ConcreteType should be sealed since it's not inherited from + Assert.Contains("public sealed class ConcreteType", result); + } + + [Fact] + public void Process_DoesNotSealStaticClasses() + { + var code = """ + namespace Test; + + public static partial class StaticHelper + { + public static string GetValue() => "test"; + } + + public partial class NormalType + { + public string Name { get; set; } + } + """; + + var result = CodePostProcessor.Process( + code, + GenerationMode.All, + "", + EmptySet, + EmptySet, + EmptySet, + "Config"); + + // StaticHelper should remain static (not sealed) + Assert.Contains("public static class StaticHelper", result); + Assert.DoesNotContain("sealed static", result); + + // NormalType should be sealed + Assert.Contains("public sealed class NormalType", result); + } } diff --git a/DotSchema/CodePostProcessor.cs b/DotSchema/CodePostProcessor.cs index 60c959b..3c16041 100644 --- a/DotSchema/CodePostProcessor.cs +++ b/DotSchema/CodePostProcessor.cs @@ -1,9 +1,9 @@ +using DotSchema.Rewriters; + using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; -using DotSchema.Rewriters; - namespace DotSchema; /// @@ -119,7 +119,8 @@ private static CompilationUnitSyntax RemoveAdditionalPropertiesBoilerplate(Compi } /// - /// Converts partial classes to sealed classes, except for base classes that are inherited from. + /// Converts partial classes to sealed classes. + /// Does not add sealed to: base classes (inherited from), abstract classes, or static classes. /// private static CompilationUnitSyntax MakeClassesSealed(CompilationUnitSyntax root) { diff --git a/DotSchema/Rewriters/AddInterfaceRewriter.cs b/DotSchema/Rewriters/AddInterfaceRewriter.cs index 9f4b7db..0fc37ca 100644 --- a/DotSchema/Rewriters/AddInterfaceRewriter.cs +++ b/DotSchema/Rewriters/AddInterfaceRewriter.cs @@ -26,12 +26,11 @@ internal sealed class AddInterfaceRewriter(string className, string interfaceNam if (visited.BaseList == null) { // No existing base list - create one with proper spacing - var baseList = SyntaxFactory.BaseList( - SyntaxFactory.SingletonSeparatedList(interfaceType)) - .WithColonToken( - SyntaxFactory.Token(SyntaxKind.ColonToken) - .WithLeadingTrivia(SyntaxFactory.Space) - .WithTrailingTrivia(SyntaxFactory.Space)); + var baseList = SyntaxFactory.BaseList(SyntaxFactory.SingletonSeparatedList(interfaceType)) + .WithColonToken( + SyntaxFactory.Token(SyntaxKind.ColonToken) + .WithLeadingTrivia(SyntaxFactory.Space) + .WithTrailingTrivia(SyntaxFactory.Space)); return visited.WithBaseList(baseList); } @@ -42,4 +41,3 @@ internal sealed class AddInterfaceRewriter(string className, string interfaceNam return visited.WithBaseList(visited.BaseList.WithTypes(newTypes)); } } - diff --git a/DotSchema/Rewriters/RemoveAdditionalPropertiesRewriter.cs b/DotSchema/Rewriters/RemoveAdditionalPropertiesRewriter.cs index ef19410..b36ed19 100644 --- a/DotSchema/Rewriters/RemoveAdditionalPropertiesRewriter.cs +++ b/DotSchema/Rewriters/RemoveAdditionalPropertiesRewriter.cs @@ -37,4 +37,3 @@ internal sealed class RemoveAdditionalPropertiesRewriter : CSharpSyntaxRewriter return base.VisitPropertyDeclaration(node); } } - diff --git a/DotSchema/Rewriters/SealClassesRewriter.cs b/DotSchema/Rewriters/SealClassesRewriter.cs index 17483f2..c3b6b04 100644 --- a/DotSchema/Rewriters/SealClassesRewriter.cs +++ b/DotSchema/Rewriters/SealClassesRewriter.cs @@ -6,7 +6,8 @@ namespace DotSchema.Rewriters; /// /// Rewriter that converts partial classes to sealed classes. -/// Base classes (those inherited from) are preserved as non-sealed to allow inheritance. +/// Always removes the partial modifier from all classes. +/// Does not add sealed to: base classes (inherited from), abstract classes, or static classes. /// internal sealed class SealClassesRewriter(IReadOnlySet baseClasses) : CSharpSyntaxRewriter { @@ -14,44 +15,40 @@ internal sealed class SealClassesRewriter(IReadOnlySet baseClasses) : CS { var visited = (ClassDeclarationSyntax) base.VisitClassDeclaration(node)!; - // Don't seal base classes - if (baseClasses.Contains(visited.Identifier.Text)) - { - return visited; - } - - // Check if already sealed - if (visited.Modifiers.Any(SyntaxKind.SealedKeyword)) - { - return visited; - } - - // Remove partial modifier and add sealed + // Always remove the partial modifier (generated code doesn't need it) var newModifiers = visited.Modifiers .Where(m => !m.IsKind(SyntaxKind.PartialKeyword)) .ToList(); - // Find position to insert sealed (after public/internal/etc.) - var insertIndex = 0; + // Determine if we should add sealed + var shouldSeal = !baseClasses.Contains(visited.Identifier.Text) // Don't seal base classes + && !visited.Modifiers.Any(SyntaxKind.AbstractKeyword) // sealed + abstract is invalid + && !visited.Modifiers.Any(SyntaxKind.StaticKeyword) // sealed + static is invalid + && !visited.Modifiers.Any(SyntaxKind.SealedKeyword); // Already sealed - for (var i = 0; i < newModifiers.Count; i++) + if (shouldSeal) { - if (newModifiers[i].IsKind(SyntaxKind.PublicKeyword) - || newModifiers[i].IsKind(SyntaxKind.InternalKeyword) - || newModifiers[i].IsKind(SyntaxKind.PrivateKeyword) - || newModifiers[i].IsKind(SyntaxKind.ProtectedKeyword)) + // Find position to insert sealed (after public/internal/etc.) + var insertIndex = 0; + + for (var i = 0; i < newModifiers.Count; i++) { - insertIndex = i + 1; + if (newModifiers[i].IsKind(SyntaxKind.PublicKeyword) + || newModifiers[i].IsKind(SyntaxKind.InternalKeyword) + || newModifiers[i].IsKind(SyntaxKind.PrivateKeyword) + || newModifiers[i].IsKind(SyntaxKind.ProtectedKeyword)) + { + insertIndex = i + 1; + } } - } - // Create sealed keyword with proper trivia (space before the class keyword) - var sealedKeyword = SyntaxFactory.Token(SyntaxKind.SealedKeyword) - .WithTrailingTrivia(SyntaxFactory.Space); + // Create sealed keyword with proper trivia (space before the class keyword) + var sealedKeyword = SyntaxFactory.Token(SyntaxKind.SealedKeyword) + .WithTrailingTrivia(SyntaxFactory.Space); - newModifiers.Insert(insertIndex, sealedKeyword); + newModifiers.Insert(insertIndex, sealedKeyword); + } return visited.WithModifiers(SyntaxFactory.TokenList(newModifiers)); } } - diff --git a/DotSchema/Rewriters/SyntaxHelpers.cs b/DotSchema/Rewriters/SyntaxHelpers.cs index 8b39547..30bc3c9 100644 --- a/DotSchema/Rewriters/SyntaxHelpers.cs +++ b/DotSchema/Rewriters/SyntaxHelpers.cs @@ -28,6 +28,11 @@ internal static class SyntaxHelpers /// Gets all base type names (classes and interfaces) from a compilation unit. /// These are types that other classes inherit from or implement. /// + /// + /// This method only detects base types within the same compilation unit. + /// Types inherited from external assemblies or other files are not detected. + /// This is acceptable for NJsonSchema output since all generated code is in a single file. + /// public static HashSet GetBaseTypeNames(CompilationUnitSyntax root) { return root.DescendantNodes() @@ -39,4 +44,3 @@ public static HashSet GetBaseTypeNames(CompilationUnitSyntax root) .ToHashSet(); } } -