handzlikchris/FastScriptReload

Fix record short syntax rewriting issue

Closed this issue · 4 comments

Related to discussion in #111
I believe we could have a small patch that takes care of renaming records under the shortcut syntax with the missing _patched suffix.

Actually the issue is witch the record keyword itself not specifically on the shortcut syntax of records.

Also :

  • Tried to add a script override, for instance adding suffix public record LeapAction__Patched_
  • FSR still rewrites it public record LeapAction
  • The constructor gets rewritten with the suffix though : public LeapAction__Patched_().
  • We end up with the compiler throwing on the ctor error CS1520: Method must have a return type' as the ctor is recognised as a regular function.

Alright, I think I got it, I post the workaround here and either I'll do a PR or someone else after further testing, for now it looks okay with the regular and the shotcut syntax.
Add in DynamicCompilationBase just before user Scripts overrides :
root = new RecordeRewriter(DebugWriteRewriteReasonAsComment).Visit(root);
Rewriter :

using FastScriptReload.Runtime;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;

namespace FastScriptReload.Editor.Compilation.CodeRewriting
{
	class RecordeRewriter : FastScriptReloadCodeRewriterBase
        {
	        public RecordeRewriter(bool writeRewriteReasonAsComment)
				: base(writeRewriteReasonAsComment)
	        {
		        }
	        
	        public override SyntaxNode VisitRecordDeclaration(RecordDeclarationSyntax node)
	        {
		        return AdjustRecordName(node, node.Identifier);
	        }

	        private SyntaxNode AdjustRecordName(RecordDeclarationSyntax node, SyntaxToken nodeIdentifier)
	        {
		        var typeName = nodeIdentifier.ToString(); //Not ToFullString() as it may include spaces and break.
		        
		        if (!typeName.EndsWith(AssemblyChangesLoader.ClassnamePatchedPostfix))
		        {
			        typeName += AssemblyChangesLoader.ClassnamePatchedPostfix;
		        }

		        return AddRewriteCommentIfNeeded(
			        node.ReplaceToken(nodeIdentifier, SyntaxFactory.Identifier(typeName)), 
			        $"{nameof(RecordeRewriter)}:{nameof(AdjustRecordName)}"
			    );
	        }
        }
}

Thanks - that looks good, and targetting just VisitRecordDeclaration makes it quite isolated. It'd be great if you can create a PR and I'll merge that in