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

源代码封版推送 #368

Merged
merged 1 commit into from
Nov 13, 2024
Merged

源代码封版推送 #368

merged 1 commit into from
Nov 13, 2024

Conversation

NMSAzulX
Copy link
Collaborator

@NMSAzulX NMSAzulX commented Nov 13, 2024

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced new projects: NETCore3.0 and ConsoleSample.
    • Enhanced logging for hot recompilation processes in console applications.
  • Bug Fixes

    • Improved error handling in the HotExecute method for better reporting.
  • Documentation

    • Added a README file for the HotExecutor extension, detailing execution and debugging features.
  • Refactor

    • Simplified initialization logic in various classes, including HEProxy and NatashaSlimMethodBuilder.
  • Chores

    • Updated target frameworks to include .NET 9.0 across multiple projects.

Copy link

coderabbitai bot commented Nov 13, 2024

Walkthrough

The pull request includes extensive modifications to the Natasha.sln solution file, resulting in the removal of several projects and the addition of new ones, including NETCore3.0 and ConsoleSample. Project identifiers for MemAssembly and WebapiWIthController were updated to a new GUID format. The solution's structure was reorganized, with updates to project configurations and documentation. Additionally, various project files underwent changes such as target framework updates, method signature modifications, and the introduction of new properties and methods across multiple components.

Changes

File Path Change Summary
Natasha.sln Removed projects: ReferenceSample, HotReloadSample, HotReloadPlugin, WinFormsSample, WPFSample; added NETCore3.0, ConsoleSample; updated GUIDs for MemAssembly, WebapiWIthController.
samples/AllProxySample/Program.cs Changed instantiation of NatashaDomain to shorthand syntax.
samples/DebugSample/DebugSample.csproj Updated target framework from .NET 8.0 to .NET 9.0.
samples/DebugSample/Program.cs Updated Main method, changed debug compilation method, and modified class A with new overloaded methods.
samples/Directory.Build.props Added new properties: NoWarn, IsHEProject, IsHENETCore30.
samples/ExtensionSample/Program.cs Commented out process information querying code, reducing active codebase.
samples/HE/NET5.0/ConsoleSample/Program.cs Added method calls and nested lambda expressions in Main.
samples/HE/NET6.0/WebapiWIthController/Program.cs Removed .WithSlimMethodBuilder() from action definition.
samples/HE/NETCORE3.0/ConsoleSample/ConsoleSample.csproj Created new project file for console application targeting .NET Core 3.1.
samples/HE/NETCORE3.0/ConsoleSample/Program.cs Introduced hot execution environment setup and logging.
src/Natasha.CSharp/Component/Core/Natasha.Domain/Natasha.Domain.csproj Added property <LangVersion>preview</LangVersion>.
src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor.SG/Natasha.CSharp.HotExecutor.SG.csproj Updated TargetFramework to netstandard2.1, added new properties, and modified package details.
src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/HEProxy.cs Simplified initialization and improved error handling in HEProxy methods.
src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/Natasha.CSharp.HotExecutor.csproj Updated <PackageTags>, removed <LangVersion>.
src/Natasha.CSharp/Extension/HotExecutor/ReCompile/Component/SyntaxHandler/ToplevelHandler.cs Added XML documentation for Handle method.
src/Natasha.CSharp/Extension/HotExecutor/README.md Introduced new README outlining HotExecutor features.
src/Natasha.CSharp/Extension/Natasha.CSharp.Extension.Codecov/Natasha.CSharp.Extension.Codecov.csproj Removed <Nullable> property.
src/Natasha.CSharp/Extension/Natasha.CSharp.Extension.MethodCreator/Natasha.CSharp.Extension.MethodCreator.csproj Removed <Nullable> property.
src/Natasha.CSharp/Extension/Natasha.CSharp.Extension.MethodCreator/NatashaSlimMethodBuilder.cs Streamlined constructor and field initializations.
src/Natasha.CSharp/Natasha.CSharp.Compiler/CompileUnit/AssemblyCSharpBuilder.Compile.cs Added using directive for Microsoft.CodeAnalysis.CSharp and improved resource disposal.
src/Natasha.CSharp/Natasha.CSharp.Compiler/Component/Compiler/Model/CompilerBinderFlags.cs Added documentation comment referencing Roslyn source.
src/Natasha.CSharp/Natasha.CSharp.Compiler/Component/Metadata/NatashaLoadContext.cs Added new constructor for NatashaLoadContext.
src/Natasha.CSharp/Natasha.CSharp.Compiler/Extension/NatashaAssemblyBuilderExtension.cs Added new overload for UseExistLoadContext and updated parameter name for clarity.
src/Natasha.CSharp/Natasha.CSharp.Compiler/Extension/NatashaStringExtension.cs Added XML documentation for ToAccessPrivateTree and overloaded WithAllUsingCode.
src/Natasha.CSharp/Natasha.CSharp.Compiler/Natasha.CSharp.Compiler.csproj Updated package reference for Microsoft.Extensions.DependencyModel and removed LangVersion and Nullable.
src/Natasha.CSharp/Natasha.CSharp.Compiler/Utils/NatashaAccessHelper.cs Commented out code related to access checks, simplifying initialization.
test/ut/Compile/MemAssembly/Compile/Access/AccessModelTest.cs Changed PrivateName field to readonly.
test/ut/Compile/MemAssembly/Compile/Access/PrivateTest.cs Removed _privateAssemblyName field; enhanced access level testing.
test/ut/Compile/MemAssembly/MemAssembly.csproj Updated <TargetFrameworks> to include .NET 9.0 and removed IgnoresAccessChecksToAttribute.cs.
test/ut/Compile/RefAssembly/RefAssembly.csproj Updated <TargetFrameworks> to include .NET 9.0 and removed IgnoresAccessChecksToAttribute.cs.

Possibly related PRs

Suggested labels

pr_src

🐰 In the land of code, where changes abound,
Projects were shuffled, new ones were found.
With GUIDs refreshed and frameworks anew,
The structure is tidy, and the logic is true.
So hop on, dear coder, let’s celebrate this feat,
For in every change, there's a rhythm, a beat! 🎉


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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 using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@NMSAzulX
Copy link
Collaborator Author

@coderabbitai review

Copy link

coderabbitai bot commented Nov 13, 2024

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 12

🧹 Outside diff range and nitpick comments (22)
test/ut/Compile/MemAssembly/Compile/Access/AccessModelTest.cs (1)

5-5: Good improvement! Consider extending readonly to other fields.

Making PrivateName readonly is a good practice as it ensures immutability. For consistency, consider applying the same pattern to InternalName and PublicName since they appear to be constant values used for testing.

Here's how you could make all fields consistent:

-        private readonly string PrivateName = "Private";
-        internal string InternalName = "Internal";
-        public string PublicName = "Public";
+        private readonly string PrivateName = "Private";
+        internal readonly string InternalName = "Internal";
+        public readonly string PublicName = "Public";
src/Natasha.CSharp/Natasha.CSharp.Compiler/Natasha.CSharp.Compiler.csproj (1)

Inconsistent Compiler Settings Detected

The removal of LangVersion and Nullable properties in src/Natasha.CSharp/Natasha.CSharp.Compiler/Natasha.CSharp.Compiler.csproj is not uniformly managed across the repository. Project files with explicitly set LangVersion or Nullable:

  • samples/ReferenceSample/ReferenceSample.csproj
  • samples/AllProxySample/AllProxySample.csproj
  • samples/DebugSample/DebugSample.csproj
  • samples/ExtensionSample/ExtensionSample.csproj
  • samples/HE/NET6.0/WebapiSample/WebapiSample.csproj
  • samples/HESample/HESample.csproj
  • samples/WinFormsSample/WinFormsSample.csproj
  • samples/HE/NET6.0/WebapiWIthController/WebapiWIthController.csproj
  • samples/HE/NET5.0/WinFormsSample/WinFormsSample.csproj
  • samples/WPFSample/WPFSample.csproj
  • test/benchmark/NormalBenchmark/NormalBenchmark.csproj
  • src/Natasha.CSharp/Component/Core/Natasha.Domain/Natasha.Domain.csproj
  • src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/Natasha.CSharp.HotExecutor.csproj
  • src/Natasha.CSharp/Component/Core/Natasha.CSharp.Compiler.Domain/Natasha.CSharp.Compiler.Domain.csproj
  • test/benchmark/NatashaBenchmark/NatashaBenchmark.csproj
  • test/benchmark/BenchmarkProject/BenchmarkProject.csproj
  • test/ut/Plugin/Domain/Domain.csproj
  • samples/PluginSample/PluginA/PluginA.csproj
  • samples/PluginSample/PluginB/PluginB.csproj
  • samples/PluginSample/PluginBase/PluginBase.csproj
  • samples/PluginSample/PluginSample/PluginSample.csproj
  • test/ut/Plugin/Compile/Compile.csproj
  • samples/HotReloadSample/HotReloadSample.csproj
  • samples/HotReloadPlugin/HotReloadPlugin.csproj
  • test/ut/Natasha.CSharp.UnitTest.Base/Natasha.CSharp.UnitTest.Base.csproj
  • test/workflow/Workflow.Initialization/Workflow.Initialization.csproj
  • test/ut/Compile/MemAssembly/MemAssembly.csproj
  • test/workflow/Workflow.Nuget.Publish/Workflow.Nuget.Publish.csproj
  • test/ut/Extension/Template/NatashaUT.csproj
  • test/workflow/Solution.NET.Sdk/Solution.NET.Sdk.csproj
  • test/workflow/Workflow.Runner/Workflow.Runner.csproj
  • test/workflow/Github.NET.Sdk/Github.NET.Sdk.csproj
  • test/workflow/Github.NMSAction.NET.Sdk/Github.NMSAction.NET.Sdk.csproj
  • test/ut/Extension/Extension/NatashaFunctionUT.csproj
  • test/ut/Compile/RefAssembly/RefAssembly.csproj
🔗 Analysis chain

Line range hint 1-31: Review removed compiler settings

The removal of LangVersion and Nullable properties might affect the project's behavior. Consider:

  1. If these settings are now managed centrally (e.g., Directory.Build.props), ensure consistent application across all projects
  2. If not, consider documenting the expected C# language version and nullable reference behavior
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for centralized build properties

# Look for Directory.Build.props or similar files
echo "Checking for centralized build properties:"
fd "Directory.Build.props"
fd "*.props" | grep -v "Directory.Build.props"

# Check other project files for similar settings
echo "Checking other projects for language version and nullable settings:"
rg -l "LangVersion|Nullable" --type csproj

Length of output: 759


Script:

#!/bin/bash
# Description: Check for centralized build properties

# Look for Directory.Build.props or similar files
echo "Checking for centralized build properties:"
fd --glob 'Directory.Build.props'
fd --glob '*.props' | grep -v 'Directory.Build.props'

# Check other project files for language version and nullable settings
echo "Checking other projects for language version and nullable settings:"
rg -l "LangVersion|Nullable" --glob "*.csproj"

Length of output: 2355

src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/ReCompile/Component/SyntaxHandler/ToplevelHandler.cs (4)

14-18: Enhance XML documentation completeness

The XML documentation is incomplete and mixed language (Chinese/English). Consider:

  • Adding parameter description for root
  • Adding return value description
  • Using consistent language (preferably English for international collaboration)

Here's a suggested improvement:

/// <summary>
-/// 包裹顶级语句
+/// Wraps top-level statements into a Program class with an async Main method.
/// </summary>
-/// <param name="root"></param>
-/// <returns></returns>
+/// <param name="root">The compilation unit containing potential top-level statements</param>
+/// <returns>A transformed compilation unit with top-level statements wrapped in a Program class, or the original if no transformation needed</returns>

Line range hint 20-35: Consider adding validation and error handling

The method makes several assumptions about the input that could lead to runtime errors:

  • No null check for root
  • No validation for root.Members being non-empty
  • Assumes successful parsing of the constructed content

Consider adding proper validation:

 public static CompilationUnitSyntax Handle(CompilationUnitSyntax root)
 {
+    if (root == null) throw new ArgumentNullException(nameof(root));
+    if (root.Members.Count == 0) return root;
+
     var file = root.SyntaxTree.FilePath;
     var firstMember = root.Members[0];
     if (firstMember != null && firstMember.IsKind(SyntaxKind.GlobalStatement))
     {
#if DEBUG
         HEProxy.ShowMessage("检测到顶级语句!");
#endif
         var usings = root.Usings;
         root = root.RemoveNodes(usings, SyntaxRemoveOptions.KeepExteriorTrivia)!;
         var content = "public class Program{ async static Task Main(string[] args){" + root!.ToFullString() + "}}";
+        try {
             var tree = NatashaCSharpSyntax.ParseTree(content, file,(CSharpParseOptions)(root.SyntaxTree).Options);
             root = tree.GetCompilationUnitRoot();
             root = root.AddUsings([.. usings]);
+        } catch (Exception ex) {
+            throw new InvalidOperationException("Failed to transform top-level statements", ex);
+        }
     }
     return root;
 }

Line range hint 27-27: Localize debug messages

The debug message is in Chinese. Consider using English for consistency or implementing proper localization.

-HEProxy.ShowMessage("检测到顶级语句!");
+HEProxy.ShowMessage("Top-level statements detected!");

Line range hint 31-31: Consider making the Program class name configurable

The hardcoded "Program" class name might conflict with existing types. Consider making it configurable or generating a unique name.

Consider adding a parameter for the class name or generating a unique name:

+private static string GetUniqueClassName() => $"Program_{Guid.NewGuid():N}";
+
 public static CompilationUnitSyntax Handle(CompilationUnitSyntax root)
 {
     // ...
-    var content = "public class Program{ async static Task Main(string[] args){" + root!.ToFullString() + "}}";
+    var className = GetUniqueClassName();
+    var content = $"public class {className}{{ async static Task Main(string[] args){{{root!.ToFullString()}}}}}";
     // ...
 }
src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor.SG/Natasha.CSharp.HotExecutor.SG.csproj (1)

9-11: Consider adding English translations

The description and release notes are currently in Chinese only. Consider adding English translations to improve accessibility for international users:

-<Description>动态重载控制台代码结果.</Description>
-<PackageReleaseNotes>升级到最新版.</PackageReleaseNotes>
+<Description>动态重载控制台代码结果. (Dynamic reload console code results.)</Description>
+<PackageReleaseNotes>升级到最新版. (Upgraded to latest version.)</PackageReleaseNotes>
samples/Directory.Build.props (1)

1-27: Add XML documentation for build properties and conditions

The build configuration is well-structured, but would benefit from documentation explaining:

  • The purpose of each suppressed warning
  • The significance of the HE and NETCORE3.0 conditions
  • The role of the source generator in the build process

Add XML comments above each major section, for example:

 <Project>
+  <!-- 
+    Warning suppressions:
+    - CS8602: Suppressed due to [reason]
+    - CS8032: Suppressed due to [reason]
+    ...
+  -->
   <PropertyGroup>
test/ut/Compile/MemAssembly/MemAssembly.csproj (2)

Line range hint 19-24: Consider updating xunit.runner.visualstudio package

The xunit.runner.visualstudio package (2.4.5) is not aligned with the xunit package version (2.5.3). Consider updating to maintain consistency and avoid potential compatibility issues.

 <PackageReference Include="coverlet.collector" Version="6.0.2" />
 <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.9.0" />
 <PackageReference Include="xunit" Version="2.5.3" />
-<PackageReference Include="xunit.runner.visualstudio" Version="2.4.5" />
+<PackageReference Include="xunit.runner.visualstudio" Version="2.5.3" />

Removal of IgnoresAccessChecksToAttribute.cs affects multiple files. Please address or remove dependent references to avoid potential issues.

🔗 Analysis chain

Line range hint 15-17: Verify removal of IgnoresAccessChecksToAttribute.cs

The removal of this file might affect assembly access checks. Please ensure:

  1. The functionality is either moved elsewhere or no longer needed
  2. No breaking changes for existing tests that might depend on this attribute
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any remaining references to IgnoresAccessChecksToAttribute
echo "Checking for references to IgnoresAccessChecksToAttribute..."
rg -l "IgnoresAccessChecksToAttribute"

# Check if the attribute is defined elsewhere
ast-grep --pattern 'class IgnoresAccessChecksToAttribute'

Length of output: 92783

samples/DebugSample/Program.cs (1)

26-29: Add assertions to verify delegate execution results

The delegate execution with floating-point values could lead to precision issues. Consider adding assertions to verify the expected behavior.

 var action = builder
     .GetAssembly()
     .GetDelegateFromShortName<Func<int, short, double, long>>("A", "Show");
-var a = action(1,2,1.2);
+var result = action(1, 2, 1.2);
+Debug.Assert(result == 14, $"Expected 14 but got {result}"); // 1 + 2 + 1.2 + 10 ≈ 14
src/Natasha.CSharp/Natasha.CSharp.Compiler/Extension/NatashaStringExtension.cs (2)

9-14: Consider enhancing the XML documentation

The documentation could be improved in several ways:

  1. Consider providing English translations alongside Chinese text for better international collaboration
  2. Add <returns> documentation to describe the return value
  3. Expand the objects parameter description to clearly specify the expected format and examples for each accepted type (private member instance, namespace string, private type)
    /// <summary>
    /// 给当前类脚本增加私有注解
+   /// Add private access annotations to the current class script
    /// </summary>
    /// <param name="script"></param>
    /// <param name="objects">可以是私有成员实例/命名空间字符串"interal.aa"/私有类型</param>
+   /// <param name="objects">Can be:
+   /// - Private member instances
+   /// - Namespace strings (e.g., "internal.aa")
+   /// - Private types</param>
+   /// <returns>A CompilationUnitSyntax with private access annotations applied</returns>

Line range hint 43-46: Fix infinite recursion in method implementation

The current implementation creates an infinite recursion by calling itself instead of the other overload. This will cause a StackOverflowException at runtime.

    public static string WithAllUsingCode(this string script, params string[] exceptUsings)
    {
-       return WithAllUsingCode(script, exceptUsings);
+       return WithAllUsingCode(script, (IEnumerable<string>)exceptUsings);
    }
src/Natasha.CSharp/Natasha.CSharp.Compiler/Component/Metadata/NatashaLoadContext.cs (1)

49-61: Design approach looks good.

The new constructor provides a flexible way to inject existing domain contexts while maintaining consistency with the class's responsibilities. It aligns well with dependency injection principles and could be particularly useful for testing scenarios.

Consider documenting the intended use cases for this constructor, particularly how it differs from the Creator pattern used by other constructors.

src/Natasha.CSharp/Extension/Natasha.CSharp.Extension.MethodCreator/NatashaSlimMethodBuilder.cs (1)

7-9: Consider documenting default Builder configuration.

The inline initialization of Builder with new() uses default configuration. Consider documenting the default behavior or adding a method to reset to default configuration if needed.

The collection expression syntax ([]) for Usings initialization requires C# 12.

Consider adding XML documentation:

+    /// <summary>
+    /// The assembly builder with default configuration.
+    /// </summary>
     public readonly AssemblyCSharpBuilder Builder = new();
+    /// <summary>
+    /// The script to be compiled.
+    /// </summary>
     public readonly string Script = script;
+    /// <summary>
+    /// Collection of using directives to be included.
+    /// </summary>
     public readonly HashSet<string> Usings = [];
src/Natasha.CSharp/Natasha.CSharp.Compiler/CompileUnit/AssemblyCSharpBuilder.Compile.cs (1)

33-33: Consider adding runtime validation for assembly name

The documentation change emphasizes the importance of specifying a new assembly name. Consider adding runtime validation to ensure the assembly name is always specified.

public AssemblyCSharpBuilder Reset()
{
    WithPreCompilationOptions();
    WithPreCompilationReferences();
+   if (string.IsNullOrEmpty(AssemblyName))
+   {
+       throw new InvalidOperationException("Assembly name must be specified after Reset()");
+   }
    return this;
}
src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/HEProxy.cs (3)

Line range hint 205-270: Consider breaking down the HandleTree method for better maintainability

The method handles multiple responsibilities including:

  • File parsing
  • Syntax tree manipulation
  • CS0104 handling
  • Debug output

Consider splitting this into smaller, focused methods following the Single Responsibility Principle.

Suggested refactor structure:

private async static Task<SyntaxTree?> HandleTree(string file)
{
    var tree = await ParseSourceFile(file);
    if (tree == null || !HasMembers(tree)) return tree;
    
    var processedRoot = await ProcessSyntaxRoot(tree.GetCompilationUnitRoot());
    return CreateFinalTree(processedRoot, file);
}

private static async Task<SyntaxTree?> ParseSourceFile(string file) { ... }
private static bool HasMembers(SyntaxTree tree) { ... }
private static async Task<CompilationUnitSyntax> ProcessSyntaxRoot(CompilationUnitSyntax root) { ... }
private static SyntaxTree CreateFinalTree(CompilationUnitSyntax root, string file) { ... }

Line range hint 273-419: Improve error handling and method organization in HotExecute

The method is quite long and handles multiple concerns. Consider:

  1. Extracting the cleanup logic into a separate method
  2. Using a more structured approach for error handling
  3. Adding logging for better diagnostics

Suggested improvements:

private static async Task HotExecute()
{
+    await using var _ = new AsyncScope(CleanupResources);
     try
     {
-        CleanErrorFiles();
+        await CleanupAndInitialize();
+        await UpdateCompilationOptions();
+        await CompileAndExecute();
     }
     catch (NatashaException nex)
     {
+        await HandleNatashaException(nex);
     }
     catch (Exception ex)
     {
+        await HandleGeneralException(ex);
     }
     finally
     {
         _isHotCompiling = false;
     }
}

+ private static async Task CleanupAndInitialize() { ... }
+ private static async Task UpdateCompilationOptions() { ... }
+ private static async Task CompileAndExecute() { ... }

Line range hint 421-432: Improve file cleanup logic

The current implementation using string operations could be enhanced using pattern matching and LINQ for better readability and maintainability.

private static void CleanErrorFiles()
{
-    var files = Directory.GetFiles(VSCSProjectInfoHelper.HEOutputPath);
-    foreach (var file in files)
-    {
-        if (Path.GetFileName(file).StartsWith("error."))
-        {
-            File.Delete(file);
-        }
-    }
+    Directory.EnumerateFiles(VSCSProjectInfoHelper.HEOutputPath, "error.*")
+        .ToList()
+        .ForEach(File.Delete);
}
Natasha.sln (1)

Removed projects are still referenced in the solution and project files.

  • WinFormsSample is referenced in Natasha.sln.
  • WPFSample is present in multiple files within the samples/ directory.
  • ReferenceSample and HotReloadSample still have project references and related code.

Ensure all references to these projects are deleted to fully clean up the solution.

🔗 Analysis chain

Line range hint 1-590: Verify removed projects are fully cleaned up.

According to the AI summary, several projects were removed (ReferenceSample, HotReloadSample, HotReloadPlugin, WinFormsSample, WPFSample). Ensure that:

  1. All references to these projects are removed from the solution
  2. No orphaned project dependencies remain
  3. Build configurations are properly cleaned up
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for remnants of removed projects
# Test: Search for any remaining references to removed projects
rg -i "ReferenceSample|HotReloadSample|HotReloadPlugin|WinFormsSample|WPFSample"

# Test: Check for any orphaned GUIDs from removed projects in other project files
fd -e csproj | xargs grep -l "ProjectReference"

Length of output: 7138

samples/HE/NETCORE3.0/ConsoleSample/Program.cs (2)

99-102: Potential Missing Argument: '参数4'

In the method ProxyMainArguments(), you append arguments "参数1" through "参数3" and then "参数5". It appears that "参数4" is missing. Please verify if "参数4" should be included for sequence consistency.

Apply this diff to include "参数4":

             HEProxy.AppendArgs("参数3");
+            HEProxy.AppendArgs("参数4");
             HEProxy.AppendArgs("参数5");

14-95: Consider Using Consistent Language in Comments

The code contains comments in both Chinese and English. For consistency and maintainability, consider using a single language for comments throughout the file.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between f1f2e1f and 339b1ee.

📒 Files selected for processing (31)
  • Natasha.sln (5 hunks)
  • samples/AllProxySample/Program.cs (1 hunks)
  • samples/DebugSample/DebugSample.csproj (1 hunks)
  • samples/DebugSample/Program.cs (1 hunks)
  • samples/Directory.Build.props (1 hunks)
  • samples/ExtensionSample/Program.cs (1 hunks)
  • samples/HE/NET5.0/ConsoleSample/Program.cs (0 hunks)
  • samples/HE/NET6.0/WebapiWIthController/Program.cs (0 hunks)
  • samples/HE/NETCORE3.0/ConsoleSample/ConsoleSample.csproj (1 hunks)
  • samples/HE/NETCORE3.0/ConsoleSample/HEOutput/Debug.txt (1 hunks)
  • samples/HE/NETCORE3.0/ConsoleSample/Program.cs (1 hunks)
  • src/Natasha.CSharp/Component/Core/Natasha.Domain/Natasha.Domain.csproj (1 hunks)
  • src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor.SG/Natasha.CSharp.HotExecutor.SG.csproj (1 hunks)
  • src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/HEProxy.cs (1 hunks)
  • src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/Natasha.CSharp.HotExecutor.csproj (1 hunks)
  • src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/ReCompile/Component/SyntaxHandler/ToplevelHandler.cs (1 hunks)
  • src/Natasha.CSharp/Extension/HotExecutor/README.md (1 hunks)
  • src/Natasha.CSharp/Extension/Natasha.CSharp.Extension.Codecov/Natasha.CSharp.Extension.Codecov.csproj (0 hunks)
  • src/Natasha.CSharp/Extension/Natasha.CSharp.Extension.MethodCreator/Natasha.CSharp.Extension.MethodCreator.csproj (0 hunks)
  • src/Natasha.CSharp/Extension/Natasha.CSharp.Extension.MethodCreator/NatashaSlimMethodBuilder.cs (1 hunks)
  • src/Natasha.CSharp/Natasha.CSharp.Compiler/CompileUnit/AssemblyCSharpBuilder.Compile.cs (4 hunks)
  • src/Natasha.CSharp/Natasha.CSharp.Compiler/Component/Compiler/Model/CompilerBinderFlags.cs (1 hunks)
  • src/Natasha.CSharp/Natasha.CSharp.Compiler/Component/Metadata/NatashaLoadContext.cs (1 hunks)
  • src/Natasha.CSharp/Natasha.CSharp.Compiler/Extension/NatashaAssemblyBuilderExtension.cs (2 hunks)
  • src/Natasha.CSharp/Natasha.CSharp.Compiler/Extension/NatashaStringExtension.cs (1 hunks)
  • src/Natasha.CSharp/Natasha.CSharp.Compiler/Natasha.CSharp.Compiler.csproj (1 hunks)
  • src/Natasha.CSharp/Natasha.CSharp.Compiler/Utils/NatashaAccessHelper.cs (1 hunks)
  • test/ut/Compile/MemAssembly/Compile/Access/AccessModelTest.cs (1 hunks)
  • test/ut/Compile/MemAssembly/Compile/Access/PrivateTest.cs (0 hunks)
  • test/ut/Compile/MemAssembly/MemAssembly.csproj (1 hunks)
  • test/ut/Compile/RefAssembly/RefAssembly.csproj (1 hunks)
💤 Files with no reviewable changes (5)
  • samples/HE/NET5.0/ConsoleSample/Program.cs
  • samples/HE/NET6.0/WebapiWIthController/Program.cs
  • src/Natasha.CSharp/Extension/Natasha.CSharp.Extension.Codecov/Natasha.CSharp.Extension.Codecov.csproj
  • src/Natasha.CSharp/Extension/Natasha.CSharp.Extension.MethodCreator/Natasha.CSharp.Extension.MethodCreator.csproj
  • test/ut/Compile/MemAssembly/Compile/Access/PrivateTest.cs
✅ Files skipped from review due to trivial changes (6)
  • samples/DebugSample/DebugSample.csproj
  • samples/ExtensionSample/Program.cs
  • samples/HE/NETCORE3.0/ConsoleSample/ConsoleSample.csproj
  • src/Natasha.CSharp/Extension/HotExecutor/README.md
  • src/Natasha.CSharp/Natasha.CSharp.Compiler/Component/Compiler/Model/CompilerBinderFlags.cs
  • src/Natasha.CSharp/Natasha.CSharp.Compiler/Utils/NatashaAccessHelper.cs
🧰 Additional context used
🪛 LanguageTool
samples/HE/NETCORE3.0/ConsoleSample/HEOutput/Debug.txt

[style] ~24-~24: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...2 参数3 参数4 In while loop! In while loop! In while loop! In while loop! In while loo...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)


[style] ~25-~25: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ile loop! In while loop! In while loop! In while loop! In while loop! In while loo...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)


[style] ~26-~26: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ile loop! In while loop! In while loop! In while loop! In while loop! Renamed: G:...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)


[style] ~27-~27: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ile loop! In while loop! In while loop! In while loop! Renamed: G:\Project\OpenSou...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)


[style] ~52-~52: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ile loop! In while loop! In while loop! In while loop! In while loop!

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)


[style] ~53-~53: Using many exclamation marks might seem excessive (in this case: 10 exclamation marks for a text that’s 1324 characters long)
Context: ...while loop! In while loop! In while loop! In while loop!

(EN_EXCESSIVE_EXCLAMATION)


[style] ~53-~53: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ile loop! In while loop! In while loop! In while loop!

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)

🔇 Additional comments (23)
src/Natasha.CSharp/Component/Core/Natasha.Domain/Natasha.Domain.csproj (2)

Line range hint 9-11: LGTM! Version numbers are properly aligned

The Version, FileVersion, and AssemblyVersion are all aligned to 8.0.0.0, following semantic versioning best practices.


13-13: Verify implications of using preview language version

Setting LangVersion to preview enables latest C# language features that are not yet stable. This carries several risks:

  • Preview features may change or be removed in future releases
  • Different developers may have incompatible tooling versions
  • CI/CD environments need to support preview features

Run the following script to check for preview language feature usage:

Consider adding a comment in the project file explaining why preview features are needed, and document any preview features being used.

src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/Natasha.CSharp.HotExecutor.csproj (1)

11-11: LGTM! Package tags updated to better reflect the extension's nature.

The updated tags better categorize this package as a Natasha extension, which is more accurate than the previous tags.

samples/AllProxySample/Program.cs (2)

11-11: LGTM: Simplified constructor syntax

The change to use target-typed new expression is a good modernization of the code.


Line range hint 14-21: Security: Add validation for plugin loading

The current implementation loads and executes assemblies without validation, which could pose security risks. Consider implementing:

  1. Assembly signature verification
  2. Allowlist of permitted assemblies
  3. Error handling for assembly loading failures
  4. Sandbox execution environment

Additionally, document the purpose of CurrentHEExeFilePath AppContext key.

Let's check for any security measures in the codebase:

src/Natasha.CSharp/Natasha.CSharp.Compiler/Natasha.CSharp.Compiler.csproj (1)

16-16: Verify compatibility with netstandard2.0

The major version upgrade of Microsoft.Extensions.DependencyModel from 8.0.2 to 9.0.0 requires careful verification:

  1. Ensure compatibility with netstandard2.0
  2. Review breaking changes between v8.x and v9.0
  3. Verify impact on dependent projects
src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor.SG/Natasha.CSharp.HotExecutor.SG.csproj (3)

12-14: Verify semantic versioning compliance

The version has been set to 8.0.0.0. Given this appears to be a major version:

  1. Ensure breaking changes are documented
  2. Verify that this version aligns with the project's versioning strategy
#!/bin/bash
# Check for previous version and changelog
rg -l "Version|CHANGELOG|BREAKING" --type md
cat README.md 2>/dev/null || true

8-8: Review necessity of unsafe code

The AllowUnsafeBlocks setting enables potentially dangerous low-level operations. Please verify if unsafe code is actually needed in this Source Generator.

#!/bin/bash
# Search for unsafe code usage
rg -l "unsafe" --type cs

4-4: Verify compatibility with existing consumers

The upgrade from netstandard2.0 to netstandard2.1 is a breaking change as it drops support for .NET Framework. Ensure this aligns with the project's compatibility requirements.

samples/Directory.Build.props (2)

20-26: Verify source generator configuration and compatibility

The source generator configuration looks correct with proper analyzer settings. However, let's verify the compatibility with different target frameworks.

✅ Verification successful

Source generator configuration is correctly set up and compatible

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify source generator project and its target framework compatibility

echo "Checking source generator project..."
if [ -f "src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor.SG/Natasha.CSharp.HotExecutor.SG.csproj" ]; then
    echo "✓ Source generator project found"
    # Check target framework
    grep -A 3 "TargetFramework" "src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor.SG/Natasha.CSharp.HotExecutor.SG.csproj"
else
    echo "✗ Source generator project not found"
fi

# Check for Generator attribute usage
echo "Checking for Generator attribute usage..."
rg "GeneratorAttribute|ISourceGenerator" --type cs

Length of output: 29507


8-19: Verify project reference paths and dependencies

The project references look well-structured, but let's verify their existence and relationships.

✅ Verification successful

Project references are valid and no circular dependencies found

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the existence of referenced projects and their relationships

# Check if the referenced projects exist
echo "Verifying project references..."
for project in \
    "src/Natasha.CSharp/Extension/HotExecutor/Natasha.CSharp.HotExecutor/Natasha.CSharp.HotExecutor.csproj" \
    "src/Natasha.CSharp/Component/Core/Natasha.CSharp.Compiler.Domain/Natasha.CSharp.Compiler.Domain.csproj" \
    "src/Natasha.CSharp/Extension/Natasha.CSharp.Extension.MethodCreator/Natasha.CSharp.Extension.MethodCreator.csproj"
do
    if [ -f "$project" ]; then
        echo "✓ Found: $project"
    else
        echo "✗ Missing: $project"
    fi
done

# Check for circular dependencies
echo "Checking for project references in the found files..."
for project in $(fd -e csproj); do
    echo "References in $project:"
    grep -A 2 "ProjectReference" "$project" || echo "No project references found"
done

Length of output: 29507

test/ut/Compile/MemAssembly/MemAssembly.csproj (1)

4-4: Verify .NET 9.0 preview compatibility

Adding .NET 9.0 target framework is forward-looking but requires careful consideration:

  1. Ensure all dependencies support .NET 9.0 preview
  2. Consider documenting any preview-specific behaviors or limitations
#!/bin/bash
# Check package compatibility with .NET 9.0
echo "Checking package references for .NET 9.0 compatibility..."
rg -l --type xml "<PackageReference" | while read -r file; do
  echo "=== $file ==="
  cat "$file" | grep -A 1 "<PackageReference"
done
test/ut/Compile/RefAssembly/RefAssembly.csproj (2)

Line range hint 15-17: Verify impact of removing IgnoresAccessChecksToAttribute

The removal of IgnoresAccessChecksToAttribute.cs might affect tests involving assembly access checks. Please ensure this doesn't break any existing tests.

#!/bin/bash
# Search for tests that might depend on IgnoresAccessChecksToAttribute
rg -l "IgnoresAccessChecksTo" --glob "**/*Test*.cs"

# Look for usage of the attribute in test files
ast-grep --pattern 'class $_ { $$$ [IgnoresAccessChecksTo$_] $$$ }'

4-4: Verify .NET 9 SDK availability and package compatibility

Adding net9.0 target framework is forward-looking, but please ensure:

  1. The CI/CD pipeline has the .NET 9 SDK preview installed
  2. All package dependencies support .NET 9 or are framework-independent
src/Natasha.CSharp/Extension/Natasha.CSharp.Extension.MethodCreator/NatashaSlimMethodBuilder.cs (2)

13-13: LGTM!

The empty line addition improves code organization.


5-5: Verify .NET version compatibility and update documentation for breaking change.

The switch to primary constructor syntax requires C# 12/.NET 8. This is a breaking change as it now requires the script parameter during instantiation.

src/Natasha.CSharp/Natasha.CSharp.Compiler/Extension/NatashaAssemblyBuilderExtension.cs (2)

119-125: LGTM! Parameter name change improves clarity.

The rename from domain to loadContext better aligns with the method's purpose and maintains consistency with the method name.


126-132: ⚠️ Potential issue

Method implementation may lead to unexpected behavior.

Several concerns with the new overload:

  1. The method name UseExistLoadContext suggests using an existing context, but DomainManagement.Create might create a new one. Consider using a more appropriate name or implementing actual context reuse.
  2. Missing XML documentation for the new overload.
  3. The error message could be more descriptive about the expected format/requirements for the name.
  4. No validation that the created context matches the provided domain's context.

Consider this implementation instead:

+/// <summary>
+/// Uses an existing load context from the provided dynamic load context base.
+/// </summary>
+/// <param name="builder">The assembly builder instance.</param>
+/// <param name="domain">The dynamic load context to use.</param>
+/// <returns>The builder instance for method chaining.</returns>
+/// <exception cref="ArgumentNullException">Thrown when domain or domain.Name is null.</exception>
+/// <exception cref="InvalidOperationException">Thrown when the context cannot be found.</exception>
 public static AssemblyCSharpBuilder UseExistLoadContext(this AssemblyCSharpBuilder builder, INatashaDynamicLoadContextBase domain)
 {
+    if (domain == null)
+        throw new ArgumentNullException(nameof(domain));
+        
     if (domain.Name == null)
-        throw new NullReferenceException("domain.Name 为空, 无法从缓存中找到该值!");
+        throw new ArgumentNullException($"{nameof(domain)}.{nameof(domain.Name)}", 
+            "The context name cannot be null. Ensure a valid name is provided.");

-    builder.LoadContext = DomainManagement.Create(domain.Name);
+    // Get existing context instead of potentially creating a new one
+    var context = DomainManagement.Get(domain.Name);
+    if (context == null)
+        throw new InvalidOperationException(
+            $"No existing context found with name '{domain.Name}'.");
+
+    builder.LoadContext = context;
     return builder;
 }

Let's verify the usage of DomainManagement.Create vs Get:

src/Natasha.CSharp/Natasha.CSharp.Compiler/CompileUnit/AssemblyCSharpBuilder.Compile.cs (1)

2-2: LGTM: Required import for C# compilation functionality

The addition of Microsoft.CodeAnalysis.CSharp import is appropriate for the C# assembly builder.

Natasha.sln (2)

233-235: Documentation addition looks good.

The addition of README.md to the HotExecutor project section improves project documentation.


264-266: Verify NETCore3.0 project configuration.

The addition of the NETCore3.0 section with ConsoleSample project looks well-structured with proper build configurations and nested project relationships. However, ensure that:

  1. The target framework in the project file matches the folder structure
  2. All necessary dependencies are properly configured

Also applies to: 486-489, 585-586

samples/HE/NETCORE3.0/ConsoleSample/Program.cs (2)

32-32: Verify the Method Name 'RegistDomainCreator'

The method name RegistDomainCreator may be a typo. Please verify if it should be RegisterDomainCreator. An incorrect method name could lead to a compilation error.

Run the following script to search for the correct method name in the codebase:


58-58: Verify the Method 'ToHotExecutor()' on 'CancellationTokenSource'

Please ensure that the method ToHotExecutor() exists for CancellationTokenSource. If this is an extension method, confirm that the appropriate using directive or namespace is included to avoid runtime errors.

Run the following script to check for the definition of ToHotExecutor() extension method on CancellationTokenSource:

Comment on lines 12 to 13
string file = "G:\\Project\\OpenSource\\Jester\\src\\backend\\Jester.Api\\bin\\Debug\\net8.0\\Jester.Api.dll";
string dependencyDirectory = Path.GetDirectoryName(file)!;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider making the plugin path configurable

The hardcoded path G:\\Project\\... reduces portability and makes deployment challenging. Consider:

  1. Moving this to configuration
  2. Using relative paths or environment variables

Example improvement:

-            string file = "G:\\Project\\OpenSource\\Jester\\src\\backend\\Jester.Api\\bin\\Debug\\net8.0\\Jester.Api.dll";
-            string dependencyDirectory = Path.GetDirectoryName(file)!;
+            string file = Configuration.GetPluginPath() ?? throw new InvalidOperationException("Plugin path not configured");
+            string dependencyDirectory = Path.GetDirectoryName(file) ?? 
+                throw new InvalidOperationException("Invalid plugin path");

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines +3 to +7
<PropertyGroup>
<NoWarn>CS0169;CS8602;CS8002;CS8032;1701;1702;0168;NETSDK1138;IDE0060;xUnit2000;CS0067;CS8321;CS0649;CA1822;CS1998;RS1014;CS1591;CA1050;</NoWarn>
<IsHEProject Condition="$(MSBuildProjectFullPath.Contains('$([MSBuild]::NormalizeDirectory('$(SolutionDir)', 'samples','HE'))'))">true</IsHEProject>
<IsHENETCore30 Condition="$(MSBuildProjectFullPath.Contains('$([MSBuild]::NormalizeDirectory('$(SolutionDir)', 'samples','HE','NETCORE3.0'))'))">true</IsHENETCore30>
</PropertyGroup>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Review Suppressed Warnings:

  • CS0169: Multiple unused private fields detected. Evaluate the necessity of suppressing these warnings.

  • CA1050: No issues found; types are correctly declared within namespaces.

🔗 Analysis chain

Review and document warning suppressions

The extensive list of suppressed warnings includes some that could indicate potential issues:

  • CS8602: Possible null reference dereference
  • CS8032: Invalid analyzer configuration
  • IDE0060: Remove unused parameter
  • CS0649: Field never assigned

Let's check if these warnings are actually occurring in the codebase:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for occurrences of the suppressed warning patterns
# to verify if they are actually needed

echo "Searching for warning patterns in the codebase..."
rg -l "CS8602|CS8032|IDE0060|CS0649" --type cs

echo "Checking for compiler warning comments..."
rg "#pragma warning|// CS\d{4}|// IDE\d{4}" --type cs

Length of output: 2238


Script:

#!/bin/bash
# Description: Search for potential occurrences of suppressed warnings not identified in the initial scan.

echo "Searching for CS0169 (field never used)..."
rg "private\s+\w+\s+\w+;" --type cs

echo "Searching for CS1701 and CS1702 (Assembly version conflicts)..."
rg "assembly:\s+AssemblyVersion\(" --type cs

echo "Searching for CS8321 (unused local functions)..."
rg "\blocal\s+function\b" --type cs

echo "Searching for CA1822 (mark members as static)..."
rg "\bpublic\s+.*\{\s+get;\s+set;\s+\}" --type cs

echo "Searching for RS1014 (unused private members)..."
rg "private\s+\w+\s+\w+\s*\(" --type cs

echo "Searching for CS1591 (missing XML comments)..."
rg "public\s+.*;" --type cs

echo "Searching for CA1050 (Declare types in namespaces)..."
rg "namespace\s+\w+" --type cs

Length of output: 226391

Comment on lines +37 to +41
AssemblyCSharpBuilder builder = new();
builder.UseSmartMode();
builder.AppendExceptUsings("System.IO");
builder.WithDebugPlusCompile(debugger => debugger.ForStandard());
builder.Add("public static class A{ public static void Show(){ Console.WriteLine(File.Exists(\"1\")); } }");
builder.Add("public static class A{ public static void Show(int i,int j){ return i+j; } }");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix critical type mismatch in CS0104Test

There are several issues in this method:

  1. The Show method is declared as void but contains a return i+j statement
  2. The delegate type Action doesn't match the method's signature which returns a value

Apply these fixes:

-builder.Add("public static class A{ public static void Show(int i,int j){ return i+j; } }");
-var action = builder.GetAssembly().GetDelegateFromShortName<Action>("A", "Show");
+builder.Add("public static class A{ public static int Show(int i,int j){ return i+j; } }");
+var action = builder.GetAssembly().GetDelegateFromShortName<Func<int,int,int>>("A", "Show");

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines +20 to +25
builder.Add(@"public static class A{
public static long Show(int i,short j,double z){ return (long)(i + j + z + 10); }
public static long Show2(int i,short j,double z){ return (long)(i + j + z + 10); }
public static long Show3(int i,short j,double z){ return (long)(i + j + z + 10); }
public static long Show4(int i,short j,double z){ return (long)(i + j + z + 10); }
}");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Reduce code duplication in class A

The dynamically generated class contains four identical methods that only differ in name. This violates the DRY (Don't Repeat Yourself) principle.

Consider refactoring to a single method or using a more maintainable approach:

-public static class A{ 
-public static long Show(int i,short j,double z){  return (long)(i + j + z + 10); } 
-public static long Show2(int i,short j,double z){  return (long)(i + j + z + 10); }
-public static long Show3(int i,short j,double z){  return (long)(i + j + z + 10); }
-public static long Show4(int i,short j,double z){  return (long)(i + j + z + 10); }
-}
+public static class A {
+    private const long ADJUSTMENT = 10;
+    public static long Calculate(int i, short j, double z) => (long)(i + j + z + ADJUSTMENT);
+}

Also, consider documenting why the constant value 10 is added to the result.

📝 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.

Suggested change
builder.Add(@"public static class A{
public static long Show(int i,short j,double z){ return (long)(i + j + z + 10); }
public static long Show2(int i,short j,double z){ return (long)(i + j + z + 10); }
public static long Show3(int i,short j,double z){ return (long)(i + j + z + 10); }
public static long Show4(int i,short j,double z){ return (long)(i + j + z + 10); }
}");
builder.Add(@"public static class A {
private const long ADJUSTMENT = 10;
public static long Calculate(int i, short j, double z) => (long)(i + j + z + ADJUSTMENT);
}");

Comment on lines +13 to +19
AssemblyCSharpBuilder builder = new();
builder.GetException();
builder.UseRandomLoadContext();
builder.UseSmartMode();
builder.WithDebugPlusCompile(debugger=>debugger.ForStandard());
builder.Add("public static class A{ public static void Show(){ Console.WriteLine(1); } }");
var action = builder.GetAssembly().GetDelegateFromShortName<Action>("A", "Show");
action();
//builder.WithFileOutput();
builder.WithDebugCompile(debugger => debugger.ForCore());
//builder.WithDebugCompile(debugger=>debugger.ForStandard());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Update all instances of deprecated API methods to the new configuration

  • Remaining usages found:
    • samples/DebugSample/Program.cs
    • src/Natasha.CSharp/Compiler/CompileUnit/AssemblyCSharpBuilder.CompileOption.cs
    • src/Natasha.CSharp/Compiler/CompileUnit/Configuration/DebugConfiguration.cs
    • src/Natasha.CSharp/HotExecutor/ReCompile/HECompiler.cs

Ensure all deprecated methods are replaced with their updated counterparts and review CHANGELOG.md for migration guidelines.

🔗 Analysis chain

Verify API changes and consider documenting configuration choices

The builder configuration has undergone significant changes:

  1. New method calls (GetException, UseRandomLoadContext)
  2. Changed from WithDebugPlusCompile to WithDebugCompile
  3. Switched debugger configuration from ForStandard() to ForCore()

Let's verify the API changes and their compatibility:

Consider adding comments explaining:

  • Why ForCore() is preferred over ForStandard()
  • The purpose of GetException() and UseRandomLoadContext()
  • When to use WithFileOutput() (currently commented out)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other usages of the old API to ensure all instances are updated
rg "WithDebugPlusCompile|ForStandard\(\)" --type cs

# Look for any migration guides or breaking changes documentation
fd "BREAKING|CHANGELOG|MIGRATION" -e md

Length of output: 826

Comment on lines +5 to +9
public class NatashaSlimMethodBuilder(string script)
{
public readonly AssemblyCSharpBuilder Builder;
public readonly string Script;
public readonly HashSet<string> Usings;
private HashSet<string>? _exceptUsings;
public readonly AssemblyCSharpBuilder Builder = new();
public readonly string Script = script;
public readonly HashSet<string> Usings = [];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add input validation for the script parameter.

The class performs dynamic code generation without validating the input script. This could pose security risks if malicious code is provided.

Consider adding validation:

     public class NatashaSlimMethodBuilder(string script)
     {
+        static void ValidateScript(string script)
+        {
+            if (string.IsNullOrEmpty(script))
+                throw new ArgumentException("Script cannot be null or empty", nameof(script));
+            // Add additional validation rules as needed
+        }
+
         public readonly AssemblyCSharpBuilder Builder = new();
-        public readonly string Script = script;
+        public readonly string Script = ValidateScript(script);
         public readonly HashSet<string> Usings = [];
📝 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.

Suggested change
public class NatashaSlimMethodBuilder(string script)
{
public readonly AssemblyCSharpBuilder Builder;
public readonly string Script;
public readonly HashSet<string> Usings;
private HashSet<string>? _exceptUsings;
public readonly AssemblyCSharpBuilder Builder = new();
public readonly string Script = script;
public readonly HashSet<string> Usings = [];
public class NatashaSlimMethodBuilder
{
private static void ValidateScript(string script)
{
if (string.IsNullOrEmpty(script))
throw new ArgumentException("Script cannot be null or empty", nameof(script));
// Add additional validation rules as needed
}
public readonly AssemblyCSharpBuilder Builder = new();
public readonly string Script;
public readonly HashSet<string> Usings = [];
public NatashaSlimMethodBuilder(string script)
{
ValidateScript(script);
Script = script;
}

Comment on lines +159 to +161
dllStream.Dispose();
pdbStream?.Dispose();
xmlStream?.Dispose();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve resource management with using statements

While moving stream disposal right after compilation is good, consider using using statements to ensure proper resource cleanup even in case of exceptions.

-Stream dllStream;
-Stream? pdbStream = null;
-Stream? xmlStream = null;
+using Stream dllStream = DllFilePath != string.Empty 
+    ? File.Create(FileHandle(DllFilePath)) 
+    : new MemoryStream();
+using Stream? pdbStream = null;
+using Stream? xmlStream = null;

if (DllFilePath != string.Empty)
{
-   dllStream = File.Create(FileHandle(DllFilePath));
}
-else
-{
-   dllStream = new MemoryStream();
-}

if (CommentFilePath != string.Empty)
{
-   xmlStream = File.Create(FileHandle(CommentFilePath));
+   xmlStream = File.Create(FileHandle(CommentFilePath));
}

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines +300 to +307
dllStream.Dispose();
xmlStream?.Dispose();
}
else
{
dllStream.Dispose();
pdbStream?.Dispose();
xmlStream?.Dispose();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix potential use of disposed stream and improve resource management

There are several issues with the current stream disposal implementation:

  1. In the success path, dllStream is disposed before LoadAssemblyFromStream uses it
  2. Stream disposal code is duplicated between success and failure paths
  3. Resource cleanup might not happen if an exception occurs

Consider restructuring the code to use using statements and ensure proper stream lifecycle:

-Stream dllStream;
-Stream? pdbStream = null;
-Stream? xmlStream = null;
+using Stream dllStream = DllFilePath != string.Empty 
+    ? File.Create(FileHandle(DllFilePath)) 
+    : new MemoryStream();
+using Stream? pdbStream = null;
+using Stream? xmlStream = null;

// ... compilation code ...

if (compileResult.Success)
{
    dllStream.Position = 0;
-   pdbStream?.Dispose();
    assembly = Domain.LoadAssemblyFromStream(dllStream, null);
    LoadContext!.LoadMetadataWithAssembly(assembly);
    CompileSucceedEvent?.Invoke(_compilation, assembly!);
-   dllStream.Dispose();
-   xmlStream?.Dispose();
}
else
{
-   dllStream.Dispose();
-   pdbStream?.Dispose();
-   xmlStream?.Dispose();
    CompileFailedEvent?.Invoke(_compilation, compileResult.Diagnostics);
    _exception = NatashaExceptionAnalyzer.GetCompileException(_compilation, compileResult.Diagnostics);
    throw _exception;
}
📝 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.

Suggested change
dllStream.Dispose();
xmlStream?.Dispose();
}
else
{
dllStream.Dispose();
pdbStream?.Dispose();
xmlStream?.Dispose();
using Stream dllStream = DllFilePath != string.Empty
? File.Create(FileHandle(DllFilePath))
: new MemoryStream();
using Stream? pdbStream = null;
using Stream? xmlStream = null;
// ... compilation code ...
if (compileResult.Success)
{
dllStream.Position = 0;
assembly = Domain.LoadAssemblyFromStream(dllStream, null);
LoadContext!.LoadMetadataWithAssembly(assembly);
CompileSucceedEvent?.Invoke(_compilation, assembly!);
}
else
{
CompileFailedEvent?.Invoke(_compilation, compileResult.Diagnostics);
_exception = NatashaExceptionAnalyzer.GetCompileException(_compilation, compileResult.Diagnostics);
throw _exception;
}

@@ -19,7 +19,7 @@ public static class HEProxy
private static readonly ConcurrentDictionary<string, SyntaxTree> _fileSyntaxTreeCache = new();
private static readonly ConcurrentDictionary<string, HashSet<string>> _cs0104UsingCache = new();
//private static readonly ConcurrentDictionary<string, CSharpParseOptions?> _mixOPLCommentCache = new();
private static readonly object _runLock = new object();
private static readonly object _runLock = new();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

C# language version set to preview

All projects are using LangVersion set to preview. It's recommended to specify a stable version like 12.0 to ensure compatibility and stability.

🔗 Analysis chain

Verify C# language version compatibility

The code uses C# 12.0 features like collection expressions [] and simplified object initialization new(). While this makes the code more concise, ensure that:

  1. The project's language version is set to support C# 12.0
  2. All developers and build environments are compatible with this version

Also applies to: 23-23, 35-35, 36-36, 37-37

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check project language version settings
rg -l "LangVersion" --glob "**/*.csproj"

Length of output: 749


Script:

#!/bin/bash
# Extract LangVersion values from csproj files
rg -H "LangVersion" --glob "**/*.csproj" | awk -F '>' '{print $2}' | awk -F '<' '{print $1}'

Length of output: 196

@@ -225,7 +219,7 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Natasha.CSharp.UnitTest.Bas
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "RefAssembly", "test\ut\Compile\RefAssembly\RefAssembly.csproj", "{67BBDAD5-AFE1-4371-A5CA-C05242A28542}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "MemAssembly", "test\ut\Compile\MemAssembly\MemAssembly.csproj", "{AC2E37E8-A930-4092-BA9D-2CCC2E895E5D}"
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "MemAssembly", "test\ut\Compile\MemAssembly\MemAssembly.csproj", "{AC2E37E8-A930-4092-BA9D-2CCC2E895E5D}"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Duplicate GUIDs detected in Natasha.sln. Ensure each project has a unique GUID and verify consistency across all project files.

🔗 Analysis chain

Verify GUID consistency across solution.

The GUIDs for MemAssembly and WebapiWIthController projects have been updated. While the changes look correct, it's important to ensure these GUIDs are unique across the solution and match with their respective project files.

Also applies to: 262-263

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for GUID uniqueness and consistency
# Test: Extract all GUIDs and check for duplicates
rg -o "[A-F0-9]{8}-[A-F0-9]{4}-[A-F0-9]{4}-[A-F0-9]{4}-[A-F0-9]{12}" | sort | uniq -d

# Test: Verify GUIDs in project files match solution
fd -e csproj | xargs grep -l "ProjectGuid"

Length of output: 4932


Script:

#!/bin/bash
# Description: Count total and unique GUIDs in Natasha.sln
# Test: Compare total GUIDs with unique GUIDs to identify duplicates
rg -o "[A-F0-9]{8}-[A-F0-9]{4}-[A-F0-9]{4}-[A-F0-9]{4}-[A-F0-9]{12}" Natasha.sln | wc -l
rg -o "[A-F0-9]{8}-[A-F0-9]{4}-[A-F0-9]{4}-[A-F0-9]{4}-[A-F0-9]{12}" Natasha.sln | sort | uniq | wc -l

Length of output: 202


Script:

#!/bin/bash
# Description: Verify GUIDs in project files match those in Natasha.sln
# Test: Extract GUIDs from .csproj files and compare with solution GUIDs
solution_guids=$(rg -o "[A-F0-9]{8}-[A-F0-9]{4}-[A-F0-9]{4}-[A-F0-9]{4}-[A-F0-9]{12}" Natasha.sln)
proj_guids=$(fd -e csproj | xargs grep -o "ProjectGuid\s*=\s*\"[A-F0-9\-]*\"" | grep -o "[A-F0-9\-]*")
missing_in_proj=$(comm -23 <(echo "$solution_guids" | sort) <(echo "$proj_guids" | sort))
missing_in_sol=$(comm -13 <(echo "$solution_guids" | sort) <(echo "$proj_guids" | sort))

echo "GUIDs in solution but not in projects:"
echo "$missing_in_proj"

echo "GUIDs in projects but not in solution:"
echo "$missing_in_sol"

Length of output: 66558

@NMSAzulX NMSAzulX merged commit 70aa426 into dev.next Nov 13, 2024
3 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

1 participant