-
Notifications
You must be signed in to change notification settings - Fork 216
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
update HE 重构 支持语法块递归处理 HE 命令 #335
Conversation
WalkthroughThe recent updates encompass the addition of logging functionality specific to WinForms and WPF within the Natasha.CSharp framework. Notable enhancements include delegate creation facilities, asynchronous file handling, syntax manipulation tools, and hot recompilation improvements. New static classes and methods have been introduced for better file logging, syntax analysis, and execution monitoring, enhancing the robustness and readability of code. Changes
Poem
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
@coderabbitai review |
Actions performedReview triggered.
|
未检测到合适的 ISSUE 推荐给您。感谢您的反馈!
|
UT Test - Ubuntu1 tests 1 ✅ 0s ⏱️ Results for commit 696b352. |
UT Test - Windows1 tests 1 ✅ 0s ⏱️ Results for commit 696b352. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 25
Outside diff range and nitpick comments (1)
src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/Component/HECompiler.cs (1)
Line range hint
38-53
: Ensure proper asynchronous execution and error handling.The current implementation returns a completed task with the result of
GetAssembly()
, which might not be truly asynchronous. Ensure proper asynchronous execution and include error handling.public static async Task<Assembly> ReCompile(IEnumerable<SyntaxTree> trees, bool isRelease) { _builderCache.WithRandomAssenblyName(); _builderCache.UseRandomLoadContext(); _builderCache.SyntaxTrees.Clear(); _builderCache.SyntaxTrees.AddRange(trees); if (isRelease) { _builderCache.WithReleasePlusCompile(); } else { _builderCache.WithDebugPlusCompile(debugger => debugger.ForAssembly()); } try { return await Task.Run(() => _builderCache.GetAssembly()); } catch (Exception ex) { Debug.WriteLine($"Compilation failed: {ex.Message}"); throw; } }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (17)
- samples/WinFormsSample/HEDebug.txt (1 hunks)
- src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor.SG/HotExecutorGenerator.cs (4 hunks)
- src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/Component/DelegateHelper.cs (1 hunks)
- src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/Component/HECompiler.cs (3 hunks)
- src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/Component/HESpinLock.cs (1 hunks)
- src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/Component/IOUtils/HEFileHelper.cs (1 hunks)
- src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/Component/IOUtils/HEFileLogger.cs (1 hunks)
- src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/Component/SyntaxUtils/CS0104Analyser.cs (1 hunks)
- src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/Component/SyntaxUtils/ConsoleWriter.cs (1 hunks)
- src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/Component/SyntaxUtils/MethodTriviaRewriter.cs (1 hunks)
- src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/Component/SyntaxUtils/OptimizationAnalyser.cs (1 hunks)
- src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/Component/SyntaxUtils/ToplevelRewriter.cs (1 hunks)
- src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/Component/SyntaxUtils/UsingsRewriter.cs (1 hunks)
- src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/Component/SyntaxUtils/WinformRewriter.cs (1 hunks)
- src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/Component/SyntaxUtils/WpfWriter.cs (1 hunks)
- src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/Component/WatcherUtils/VSCSharpProjectFileWatcher.cs (2 hunks)
- src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/HEProxy.cs (15 hunks)
Additional context used
LanguageTool
samples/WinFormsSample/HEDebug.txt
[typographical] ~3-~3: Nepárový symbol: zdá sa, že chýba ')'
Context: 重新扫描文件! new ApplicationContext(new Form1()) 检测到空成员文件 G:\Project\OpenSo...(UNPAIRED_BRACKETS)
[typographical] ~11-
11: Dve po sebe idúce bodkyRF5efaba5.TMP 准备执行热编译... new ApplicationContext(new Form1()) ...
Context: ...Sample\Program.cs(DOUBLE_PUNCTUATION)
[typographical] ~13-~13: Nepárový symbol: zdá sa, že chýba ')'
Context: ....TMP 准备执行热编译... new ApplicationContext(new Form1()) 执行主入口前导方法.... A GC GC ...(UNPAIRED_BRACKETS)
Additional comments not posted (20)
samples/WinFormsSample/HEDebug.txt (3)
3-3
: Unpaired bracket detected.The line
new ApplicationContext(new Form1())
is missing a closing parenthesis.- new ApplicationContext(new Form1()) + new ApplicationContext(new Form1())Likely invalid or redundant comment.
Tools
LanguageTool
[typographical] ~3-~3: Nepárový symbol: zdá sa, že chýba ')'
Context: 重新扫描文件! new ApplicationContext(new Form1()) 检测到空成员文件 G:\Project\OpenSo...(UNPAIRED_BRACKETS)
13-13
: Unpaired bracket detected.The line
new ApplicationContext(new Form1())
is missing a closing parenthesis.- new ApplicationContext(new Form1()) + new ApplicationContext(new Form1())Likely invalid or redundant comment.
Tools
LanguageTool
[typographical] ~13-~13: Nepárový symbol: zdá sa, že chýba ')'
Context: ....TMP 准备执行热编译... new ApplicationContext(new Form1()) 执行主入口前导方法.... A GC GC ...(UNPAIRED_BRACKETS)
11-11
: Double punctuation detected.The line
准备执行热编译...
has double punctuation.- 准备执行热编译... + 准备执行热编译。Likely invalid or redundant comment.
Tools
LanguageTool
[typographical] ~11-
11: Dve po sebe idúce bodkyRF5efaba5.TMP 准备执行热编译... new ApplicationContext(new Form1()) ...
Context: ...Sample\Program.cs(DOUBLE_PUNCTUATION)
src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/Component/HESpinLock.cs (1)
25-28
: LGTM!The
IsLocking
method is simple and correct. It checks if_lockCount
is 1 to determine if the lock is active.src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/Component/SyntaxUtils/ConsoleWriter.cs (1)
8-21
: LGTM!The
Handle
method inConsoleWriter
is correctly implemented. It removes the last statement if it isConsole.ReadKey();
.src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/Component/SyntaxUtils/UsingsRewriter.cs (3)
22-25
: LGTM!
26-30
: LGTM!
32-35
: LGTM!src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/Component/WatcherUtils/VSCSharpProjectFileWatcher.cs (2)
24-27
: LGTM!
Line range hint
29-35
: LGTM!src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/HEProxy.cs (10)
7-7
: Approved: New using directiveThe addition of the using directive for
Natasha.CSharp.HotExecutor.Component.SyntaxUtils
is appropriate for the new functionality.
17-18
: Approved: Field visibility and value transformationMaking
_proxyCommentDebugShow
and_proxyCommentReleaseShow
public and converting their values to lowercase enhances accessibility and ensures consistent handling.
39-39
: Approved: New public static fieldThe addition of
public static readonly HashSet<string> NatashaExtGlobalUsing
is appropriate for managing global using directives.
44-46
: Approved: New fieldsThe additions of
_triviaHandle
,_projectKind
, and_isHotCompiling
are appropriate for handling code comments, managing project types, and tracking hot compilation status.
51-86
: Approved: Static constructor and comment handling logicThe static constructor and its logic for handling comments based on different tags and conditions are well-implemented.
Line range hint
97-150
: Approved: New methodDeployCSProjWatcher
The addition of the
DeployCSProjWatcher
method for setting up a file watcher and handling project rebuilds and recompilations is appropriate.
Line range hint
165-181
: Approved: File change actions inDeployMainWatcher
The additions of actions for handling tree parsing and caching for changed, deleted, and created files in the
DeployMainWatcher
method are appropriate.
199-208
: Approved: Watcher deployment and monitoring inRun
methodThe additions in the
Run
method to deploy the file watchers and start monitoring during the first run are appropriate.
Line range hint
220-255
: Approved: New async methodHandleTree
The addition of the
HandleTree
method for parsing files, processing using directives, handling specific comments, and rewriting the main method is appropriate.
Line range hint
257-351
: Approved: New async methodHotExecute
The addition of the
HotExecute
method for managing the hot execution process, including recompilation, method invocation, and cleanup, is appropriate.
public static class DelegateHelper<T,R> | ||
{ | ||
public static Action<T,R>? Execute; | ||
public static void GetDelegate(string methodName, BindingFlags flags) | ||
{ | ||
var methodInfo = typeof(T).GetMethod(methodName, flags); | ||
Execute = (Action<T, R>)Delegate.CreateDelegate(typeof(Action<T, R>), methodInfo); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential NullReferenceException.
The GetDelegate
method does not handle the case where methodInfo
could be null.
- Execute = (Action<T, R>)Delegate.CreateDelegate(typeof(Action<T, R>), methodInfo);
+ if (methodInfo != null)
+ {
+ Execute = (Action<T, R>)Delegate.CreateDelegate(typeof(Action<T, R>), methodInfo);
+ }
+ else
+ {
+ throw new InvalidOperationException($"Method {methodName} not found in type {typeof(T)}");
+ }
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
public static class DelegateHelper<T,R> | |
{ | |
public static Action<T,R>? Execute; | |
public static void GetDelegate(string methodName, BindingFlags flags) | |
{ | |
var methodInfo = typeof(T).GetMethod(methodName, flags); | |
Execute = (Action<T, R>)Delegate.CreateDelegate(typeof(Action<T, R>), methodInfo); | |
public static class DelegateHelper<T,R> | |
{ | |
public static Action<T,R>? Execute; | |
public static void GetDelegate(string methodName, BindingFlags flags) | |
{ | |
var methodInfo = typeof(T).GetMethod(methodName, flags); | |
if (methodInfo != null) | |
{ | |
Execute = (Action<T, R>)Delegate.CreateDelegate(typeof(Action<T, R>), methodInfo); | |
} | |
else | |
{ | |
throw new InvalidOperationException($"Method {methodName} not found in type {typeof(T)}"); | |
} |
public static class DelegateHelper<T> | ||
{ | ||
public static Action<T>? Execute; | ||
public static void GetDelegate(string methodName,BindingFlags flags) | ||
{ | ||
var methodInfo = typeof(T).GetMethod(methodName, flags); | ||
Execute = (Action<T>)Delegate.CreateDelegate(typeof(Action<T>), methodInfo); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential NullReferenceException.
The GetDelegate
method does not handle the case where methodInfo
could be null.
- Execute = (Action<T>)Delegate.CreateDelegate(typeof(Action<T>), methodInfo);
+ if (methodInfo != null)
+ {
+ Execute = (Action<T>)Delegate.CreateDelegate(typeof(Action<T>), methodInfo);
+ }
+ else
+ {
+ throw new InvalidOperationException($"Method {methodName} not found in type {typeof(T)}");
+ }
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
public static class DelegateHelper<T> | |
{ | |
public static Action<T>? Execute; | |
public static void GetDelegate(string methodName,BindingFlags flags) | |
{ | |
var methodInfo = typeof(T).GetMethod(methodName, flags); | |
Execute = (Action<T>)Delegate.CreateDelegate(typeof(Action<T>), methodInfo); | |
} | |
public static class DelegateHelper<T> | |
{ | |
public static Action<T>? Execute; | |
public static void GetDelegate(string methodName,BindingFlags flags) | |
{ | |
var methodInfo = typeof(T).GetMethod(methodName, flags); | |
if (methodInfo != null) | |
{ | |
Execute = (Action<T>)Delegate.CreateDelegate(typeof(Action<T>), methodInfo); | |
} | |
else | |
{ | |
throw new InvalidOperationException($"Method {methodName} not found in type {typeof(T)}"); | |
} | |
} |
var content = "public class Program{ async static Task Main(string[] args){" + root!.ToFullString() + "}}"; | ||
var tree = NatashaCSharpSyntax.ParseTree(content, file,(CSharpParseOptions)(root.SyntaxTree).Options); | ||
root = tree.GetCompilationUnitRoot(); | ||
root = root.AddUsings([.. usings]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verify the syntax for adding usings.
The [.. usings]
syntax for adding usings is non-standard and might cause issues. Consider using the AddUsings
method correctly.
- root = root.AddUsings([.. usings]);
+ root = root.AddUsings(usings.ToArray());
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
root = root.AddUsings([.. usings]); | |
root = root.AddUsings(usings.ToArray()); |
static bool DiposeWindows() | ||
{{ | ||
try{{ | ||
for (int i = 0; i < Application.Current.Windows.Count; i++) | ||
{{ | ||
var window = Application.Current.Windows[i]; | ||
try{{ | ||
window.Close(); | ||
}}catch{{ | ||
|
||
}} | ||
}} | ||
}}catch{{ | ||
return false; | ||
}} | ||
return true; | ||
}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix typo and add proper exception handling.
The nested method DiposeWindows
has a typo and lacks proper exception handling for the Close
method.
- static bool DiposeWindows()
+ static bool DisposeWindows()
{
try{
for (int i = 0; i < Application.Current.Windows.Count; i++)
{
var window = Application.Current.Windows[i];
try{
window.Close();
}catch(Exception ex){
// Log exception
Debug.WriteLine($"Exception while closing window: {ex.Message}");
}
}
}catch{
return false;
}
return true;
}
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
static bool DiposeWindows() | |
{{ | |
try{{ | |
for (int i = 0; i < Application.Current.Windows.Count; i++) | |
{{ | |
var window = Application.Current.Windows[i]; | |
try{{ | |
window.Close(); | |
}}catch{{ | |
}} | |
}} | |
}}catch{{ | |
return false; | |
}} | |
return true; | |
}} | |
static bool DisposeWindows() | |
{ | |
try{ | |
for (int i = 0; i < Application.Current.Windows.Count; i++) | |
{ | |
var window = Application.Current.Windows[i]; | |
try{ | |
window.Close(); | |
}catch(Exception ex){ | |
// Log exception | |
Debug.WriteLine($"Exception while closing window: {ex.Message}"); | |
} | |
} | |
}catch{ | |
return false; | |
} | |
return true; | |
} |
foreach (SyntaxNode node in body.DescendantNodesAndSelf()) | ||
{ | ||
foreach (SyntaxTrivia trivia in node.GetLeadingTrivia()) | ||
{ | ||
if (trivia.IsKind(SyntaxKind.SingleLineCommentTrivia)) | ||
{ | ||
var commentText = trivia.ToString().Trim().ToLower(); | ||
if (commentText.StartsWith(_proxyCommentOPLDebug)) | ||
{ | ||
return false; | ||
} | ||
else if (commentText.StartsWith(_proxyCommentOPLRelease)) | ||
{ | ||
return true; | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optimize the function to use FirstOrDefault
.
The function can be optimized by using FirstOrDefault
to find the comment.
- foreach (SyntaxNode node in body.DescendantNodesAndSelf())
- {
- foreach (SyntaxTrivia trivia in node.GetLeadingTrivia())
- {
- if (trivia.IsKind(SyntaxKind.SingleLineCommentTrivia))
- {
- var commentText = trivia.ToString().Trim().ToLower();
- if (commentText.StartsWith(_proxyCommentOPLDebug))
- {
- return false;
- }
- else if (commentText.StartsWith(_proxyCommentOPLRelease))
- {
- return true;
- }
- }
- }
- }
+ var comment = body.DescendantNodesAndSelf()
+ .SelectMany(node => node.GetLeadingTrivia())
+ .FirstOrDefault(trivia => trivia.IsKind(SyntaxKind.SingleLineCommentTrivia));
+ if (comment != default)
+ {
+ var commentText = comment.ToString().Trim().ToLower();
+ if (commentText.StartsWith(_proxyCommentOPLDebug))
+ {
+ return false;
+ }
+ else if (commentText.StartsWith(_proxyCommentOPLRelease))
+ {
+ return true;
+ }
+ }
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
foreach (SyntaxNode node in body.DescendantNodesAndSelf()) | |
{ | |
foreach (SyntaxTrivia trivia in node.GetLeadingTrivia()) | |
{ | |
if (trivia.IsKind(SyntaxKind.SingleLineCommentTrivia)) | |
{ | |
var commentText = trivia.ToString().Trim().ToLower(); | |
if (commentText.StartsWith(_proxyCommentOPLDebug)) | |
{ | |
return false; | |
} | |
else if (commentText.StartsWith(_proxyCommentOPLRelease)) | |
{ | |
return true; | |
} | |
} | |
} | |
} | |
var comment = body.DescendantNodesAndSelf() | |
.SelectMany(node => node.GetLeadingTrivia()) | |
.FirstOrDefault(trivia => trivia.IsKind(SyntaxKind.SingleLineCommentTrivia)); | |
if (comment != default) | |
{ | |
var commentText = comment.ToString().Trim().ToLower(); | |
if (commentText.StartsWith(_proxyCommentOPLDebug)) | |
{ | |
return false; | |
} | |
else if (commentText.StartsWith(_proxyCommentOPLRelease)) | |
{ | |
return true; | |
} | |
} |
public static (bool needDelete, List<StatementSyntax> newStatements) HandleTriviaComment(SyntaxTriviaList trivias, bool needDeleted, Func<string, string?> replaceFunc) | ||
{ | ||
bool del = false; | ||
List<StatementSyntax> syntaxList = []; | ||
foreach (var trivia in trivias) | ||
{ | ||
|
||
if (trivia.IsKind(SyntaxKind.SingleLineCommentTrivia)) | ||
{ | ||
var commentText = trivia.ToString(); | ||
if (commentText.Length > 2) | ||
{ | ||
var result = replaceFunc(commentText); | ||
if (result != null) | ||
{ | ||
if (needDeleted && result == string.Empty) | ||
{ | ||
del = true; | ||
} | ||
else | ||
{ | ||
var statementNode = SyntaxFactory.ParseStatement(result); | ||
syntaxList.Add(statementNode); | ||
} | ||
} | ||
} | ||
} | ||
} | ||
return (del, syntaxList); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle potential null values in trivia
.
Ensure that trivia
is not null to avoid potential NullReferenceException
.
-if (trivia.IsKind(SyntaxKind.SingleLineCommentTrivia))
+if (trivia != null && trivia.IsKind(SyntaxKind.SingleLineCommentTrivia))
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
public static (bool needDelete, List<StatementSyntax> newStatements) HandleTriviaComment(SyntaxTriviaList trivias, bool needDeleted, Func<string, string?> replaceFunc) | |
{ | |
bool del = false; | |
List<StatementSyntax> syntaxList = []; | |
foreach (var trivia in trivias) | |
{ | |
if (trivia.IsKind(SyntaxKind.SingleLineCommentTrivia)) | |
{ | |
var commentText = trivia.ToString(); | |
if (commentText.Length > 2) | |
{ | |
var result = replaceFunc(commentText); | |
if (result != null) | |
{ | |
if (needDeleted && result == string.Empty) | |
{ | |
del = true; | |
} | |
else | |
{ | |
var statementNode = SyntaxFactory.ParseStatement(result); | |
syntaxList.Add(statementNode); | |
} | |
} | |
} | |
} | |
} | |
return (del, syntaxList); | |
} | |
if (trivia != null && trivia.IsKind(SyntaxKind.SingleLineCommentTrivia)) |
public static CompilationUnitSyntax Handle(CompilationUnitSyntax root, Func<string,string?> replaceFunc) | ||
{ | ||
var guid = System.Guid.NewGuid().ToString(); | ||
var methodDeclarations = root.DescendantNodes().OfType<MethodDeclarationSyntax>(); | ||
if (methodDeclarations == null) | ||
{ | ||
return root; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid unnecessary null checks.
The methodDeclarations
variable will never be null. The null check can be removed.
-if (methodDeclarations == null)
-{
- return root;
-}
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
public static CompilationUnitSyntax Handle(CompilationUnitSyntax root, Func<string,string?> replaceFunc) | |
{ | |
var guid = System.Guid.NewGuid().ToString(); | |
var methodDeclarations = root.DescendantNodes().OfType<MethodDeclarationSyntax>(); | |
if (methodDeclarations == null) | |
{ | |
return root; | |
} | |
public static CompilationUnitSyntax Handle(CompilationUnitSyntax root, Func<string,string?> replaceFunc) | |
{ | |
var guid = System.Guid.NewGuid().ToString(); | |
var methodDeclarations = root.DescendantNodes().OfType<MethodDeclarationSyntax>(); |
Dictionary<BlockSyntax, BlockSyntax> replaceBodyCache = []; | ||
foreach (var methodDeclaration in methodDeclarations) | ||
{ | ||
|
||
var bodyNode = methodDeclaration.Body; | ||
if (bodyNode != null) | ||
{ | ||
ConcurrentDictionary<SyntaxNode, BlockSyntax>? blockCache = null; | ||
var tempDict = bodyNode | ||
.DescendantNodesAndSelf() | ||
.OfType<BlockSyntax>() | ||
.Where(item => item != null && item.Parent != null) | ||
.Where(item => item.Parent != null); | ||
if (tempDict.Any()) | ||
{ | ||
blockCache = new(tempDict.ToDictionary(item => item.Parent!, item => item)); | ||
} | ||
|
||
var newBody = GetNewBlockSyntax(bodyNode, replaceFunc, blockCache); | ||
|
||
if (newBody != null) | ||
{ | ||
replaceBodyCache[bodyNode] = newBody; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using a more efficient data structure for replaceBodyCache
.
Using a Dictionary
for replaceBodyCache
might not be the most efficient choice. Consider using a ConcurrentDictionary
for thread safety and performance.
-Dictionary<BlockSyntax, BlockSyntax> replaceBodyCache = [];
+ConcurrentDictionary<BlockSyntax, BlockSyntax> replaceBodyCache = new();
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Dictionary<BlockSyntax, BlockSyntax> replaceBodyCache = []; | |
foreach (var methodDeclaration in methodDeclarations) | |
{ | |
var bodyNode = methodDeclaration.Body; | |
if (bodyNode != null) | |
{ | |
ConcurrentDictionary<SyntaxNode, BlockSyntax>? blockCache = null; | |
var tempDict = bodyNode | |
.DescendantNodesAndSelf() | |
.OfType<BlockSyntax>() | |
.Where(item => item != null && item.Parent != null) | |
.Where(item => item.Parent != null); | |
if (tempDict.Any()) | |
{ | |
blockCache = new(tempDict.ToDictionary(item => item.Parent!, item => item)); | |
} | |
var newBody = GetNewBlockSyntax(bodyNode, replaceFunc, blockCache); | |
if (newBody != null) | |
{ | |
replaceBodyCache[bodyNode] = newBody; | |
} | |
ConcurrentDictionary<BlockSyntax, BlockSyntax> replaceBodyCache = new(); | |
foreach (var methodDeclaration in methodDeclarations) | |
{ | |
var bodyNode = methodDeclaration.Body; | |
if (bodyNode != null) | |
{ | |
ConcurrentDictionary<SyntaxNode, BlockSyntax>? blockCache = null; | |
var tempDict = bodyNode | |
.DescendantNodesAndSelf() | |
.OfType<BlockSyntax>() | |
.Where(item => item != null && item.Parent != null) | |
.Where(item => item.Parent != null); | |
if (tempDict.Any()) | |
{ | |
blockCache = new(tempDict.ToDictionary(item => item.Parent!, item => item)); | |
} | |
var newBody = GetNewBlockSyntax(bodyNode, replaceFunc, blockCache); | |
if (newBody != null) | |
{ | |
replaceBodyCache[bodyNode] = newBody; | |
} |
private static (bool needDelete,List<StatementSyntax> newStatements) GetNewStatmentSyntax( | ||
StatementSyntax statement, | ||
Func<string, string?> replaceFunc, | ||
ConcurrentDictionary<SyntaxNode, BlockSyntax>? blockCache) | ||
{ | ||
bool del = false; | ||
List<StatementSyntax> statementList = []; | ||
if (statement.HasLeadingTrivia) | ||
{ | ||
var trivias = statement.GetLeadingTrivia(); | ||
(del, statementList) = HandleTriviaComment(trivias, true, replaceFunc); | ||
} | ||
if (!del) | ||
{ | ||
if (blockCache != null && blockCache.TryGetValue(statement, out var subBlock)) | ||
{ | ||
var newBlock = GetNewBlockSyntax(subBlock, replaceFunc, blockCache); | ||
if (newBlock != null) | ||
{ | ||
statementList.Add(statement.ReplaceNode(subBlock, newBlock)); | ||
del = true; | ||
} | ||
} | ||
} | ||
|
||
if (statement.HasTrailingTrivia) | ||
{ | ||
var trivias = statement.GetTrailingTrivia(); | ||
var tailResult = HandleTriviaComment(trivias, true, replaceFunc); | ||
if (!del && tailResult.needDelete) | ||
{ | ||
del = true; | ||
} | ||
statementList.AddRange(tailResult.newStatements); | ||
} | ||
|
||
return (del, statementList); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improve variable naming for clarity.
Using more descriptive variable names can improve readability and maintainability of the code.
-bool del = false;
+bool shouldDelete = false;
-List<StatementSyntax> statementList = [];
+List<StatementSyntax> newStatements = [];
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
private static (bool needDelete,List<StatementSyntax> newStatements) GetNewStatmentSyntax( | |
StatementSyntax statement, | |
Func<string, string?> replaceFunc, | |
ConcurrentDictionary<SyntaxNode, BlockSyntax>? blockCache) | |
{ | |
bool del = false; | |
List<StatementSyntax> statementList = []; | |
if (statement.HasLeadingTrivia) | |
{ | |
var trivias = statement.GetLeadingTrivia(); | |
(del, statementList) = HandleTriviaComment(trivias, true, replaceFunc); | |
} | |
if (!del) | |
{ | |
if (blockCache != null && blockCache.TryGetValue(statement, out var subBlock)) | |
{ | |
var newBlock = GetNewBlockSyntax(subBlock, replaceFunc, blockCache); | |
if (newBlock != null) | |
{ | |
statementList.Add(statement.ReplaceNode(subBlock, newBlock)); | |
del = true; | |
} | |
} | |
} | |
if (statement.HasTrailingTrivia) | |
{ | |
var trivias = statement.GetTrailingTrivia(); | |
var tailResult = HandleTriviaComment(trivias, true, replaceFunc); | |
if (!del && tailResult.needDelete) | |
{ | |
del = true; | |
} | |
statementList.AddRange(tailResult.newStatements); | |
} | |
return (del, statementList); | |
} | |
private static (bool needDelete,List<StatementSyntax> newStatements) GetNewStatmentSyntax( | |
StatementSyntax statement, | |
Func<string, string?> replaceFunc, | |
ConcurrentDictionary<SyntaxNode, BlockSyntax>? blockCache) | |
{ | |
bool shouldDelete = false; | |
List<StatementSyntax> newStatements = []; | |
if (statement.HasLeadingTrivia) | |
{ | |
var trivias = statement.GetLeadingTrivia(); | |
(shouldDelete, newStatements) = HandleTriviaComment(trivias, true, replaceFunc); | |
} | |
if (!shouldDelete) | |
{ | |
if (blockCache != null && blockCache.TryGetValue(statement, out var subBlock)) | |
{ | |
var newBlock = GetNewBlockSyntax(subBlock, replaceFunc, blockCache); | |
if (newBlock != null) | |
{ | |
newStatements.Add(statement.ReplaceNode(subBlock, newBlock)); | |
shouldDelete = true; | |
} | |
} | |
} | |
if (statement.HasTrailingTrivia) | |
{ | |
var trivias = statement.GetTrailingTrivia(); | |
var tailResult = HandleTriviaComment(trivias, true, replaceFunc); | |
if (!shouldDelete && tailResult.needDelete) | |
{ | |
shouldDelete = true; | |
} | |
newStatements.AddRange(tailResult.newStatements); | |
} | |
return (shouldDelete, newStatements); | |
} |
private static BlockSyntax? GetNewBlockSyntax( | ||
BlockSyntax methodBody, | ||
Func<string, string?> replaceFunc, | ||
ConcurrentDictionary<SyntaxNode, BlockSyntax>? blockCache | ||
) | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using a more descriptive variable name for methodBody
.
Using a more descriptive variable name can improve readability and maintainability of the code.
-BlockSyntax methodBody,
+BlockSyntax methodBodySyntax,
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
private static BlockSyntax? GetNewBlockSyntax( | |
BlockSyntax methodBody, | |
Func<string, string?> replaceFunc, | |
ConcurrentDictionary<SyntaxNode, BlockSyntax>? blockCache | |
) | |
{ | |
private static BlockSyntax? GetNewBlockSyntax( | |
BlockSyntax methodBodySyntax, | |
Func<string, string?> replaceFunc, | |
ConcurrentDictionary<SyntaxNode, BlockSyntax>? blockCache | |
) | |
{ |
Summary by CodeRabbit
New Features
Improvements
Refactor
Bug Fixes