Refactor to Roslyn for code formatting, remove JetBrains cleanup

[lonecat13]

Summary

Refactors CodePostProcessor from regex/string manipulation to Roslyn syntax tree APIs, and removes the JetBrains cleanup dependency.

Changes

• CodePostProcessor.cs: Complete rewrite using Roslyn's CSharpSyntaxTree, CSharpSyntaxRewriter, and NormalizeWhitespace()
• DotSchema.csproj: Added Microsoft.CodeAnalysis.CSharp v5.0.0 package
• JetBrainsCleanupRunner.cs: Deleted (no longer needed)
• CommandLineOptions.cs: Removed --no-cleanup option
• Constants.cs: Removed JetBrains and FilePatterns constants
• SchemaGenerator.cs: Removed JetBrains cleanup call

Benefits

Before	After
~391 lines of regex/string manipulation	~261 lines of Roslyn APIs
Fragile brace-counting for type removal	Semantic RemoveNodes()
Regex for class modifications	Type-safe CSharpSyntaxRewriter
Required external jb cleanupcode tool	Built-in NormalizeWhitespace()

Testing

All 54 tests pass.

[lonecat13]

Code Review: Refactor to Roslyn for code formatting

Summary

This is a well-executed refactor that replaces fragile regex-based code manipulation with Roslyn's type-safe syntax tree APIs. The change removes an external dependency (JetBrains cleanup tool) and improves maintainability significantly.

✅ What's Good

1. Architecture & Design

• Clean separation of concerns with dedicated CSharpSyntaxRewriter classes in the Rewriters/ directory
• Each rewriter has a single responsibility (SRP)
• SyntaxHelpers provides reusable helper methods
• The CodePostProcessor orchestrates transformations cleanly

2. Code Quality

• Excellent use of Roslyn's RemoveNodes() for type removal instead of fragile brace-counting
• NormalizeWhitespace() provides consistent formatting without external tools
• Primary constructors used appropriately for the rewriters
• Good XML documentation on public APIs

3. Test Coverage

• Added 3 new tests covering key scenarios:
  • Process_RemovesAdditionalPropertiesBoilerplate
  • Process_PreservesBaseClassesAsNonSealed
  • Process_SharedMode_RemovesRootType
• All 54 tests pass

4. Dependency Management

• Microsoft.CodeAnalysis.CSharp v5.0.0 is a stable, well-maintained package
• Removes external runtime dependency on JetBrains CLI tool

5. README Updates

• Good documentation of the architecture and flow
• Updated requirements and removed obsolete options

---

🔍 Minor Suggestions

1. SyntaxHelpers.GetBaseClassNames could include interfaces incorrectly

The GetBaseClassNames method collects ALL items from a class's base list. This includes interfaces, not just classes. While this doesn't affect correctness (not sealing a class because it implements an interface is harmless), the naming could be misleading.

Suggestion: Consider renaming to GetBaseTypeNames or adding a comment clarifying this includes interfaces.

2. RemoveAdditionalPropertiesRewriter - Consider removing associated attributes

The rewriter removes the _additionalProperties field and AdditionalProperties property, but it doesn't explicitly remove the [JsonExtensionData] attribute. The tests show this works because the property removal takes the attribute with it, but worth noting.

3. SealClassesRewriter - trivia handling edge case

When removing partial and inserting sealed, the trivia (whitespace) handling looks correct. However, if a class has other modifiers like static, the insert position logic only checks for access modifiers. Since NJsonSchema generates public partial class, this should be fine in practice.

4. AddInterfaceRewriter - interface ordering consideration

When a class already has a base list, the interface is prepended (inserted at position 0). In C#, the base class should come first, followed by interfaces. If the class already inherits from another class, prepending would put the interface before the base class.

Looking at the current code flow:

var newTypes = visited.BaseList.Types.Insert(0, interfaceType);

This could result in class Foo : IConfig, BaseClass instead of class Foo : BaseClass, IConfig.

Recommendation: Consider appending instead of prepending, or checking if the first base type is a class vs interface.

---

📊 Metrics Improvement

Metric	Before	After
Lines of code	~391	~261
External dependencies	JetBrains CLI	None
Type safety	Regex/string	Roslyn AST

---

✅ Verdict

LOOKS GOOD - This is a solid refactor that improves code quality, removes an external dependency, and maintains test coverage. The minor suggestions above are non-blocking and can be addressed in future iterations if needed.

[lonecat13]

✅ Review Comments Addressed

All feedback from the code review has been implemented in commit 8d3b63d:

1. Interface ordering - Now using .Add() to append the interface to the end of the base list (maintains C# convention of base class first, then interfaces)

2. Naming - Renamed GetBaseClassNames → GetBaseTypeNames with updated documentation clarifying it collects both classes and interfaces

3. Documentation - Added clarifying comment to RemoveAdditionalPropertiesRewriter explaining that attributes are implicitly removed with the property

All 57 tests pass. Ready for merge! 🚀