Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix record short syntax rewriting issue #115

Open
builder-main opened this issue Jan 7, 2024 · 4 comments
Open

Fix record short syntax rewriting issue #115

builder-main opened this issue Jan 7, 2024 · 4 comments

Comments

@builder-main
Copy link

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.

@builder-main
Copy link
Author

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

@builder-main
Copy link
Author

builder-main commented Jan 17, 2024

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.

@builder-main
Copy link
Author

builder-main commented Jan 17, 2024

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)}"
			    );
	        }
        }
}

@handzlikchris
Copy link
Owner

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

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

No branches or pull requests

2 participants