Refactor/improve code quality #5

HandyS11 · 2026-01-19T15:13:56Z

No description provided.

…roved entity analysis

Copilot

Pull request overview

This pull request focuses on refactoring and improving code quality across the ProjGraph solution. The main purpose is to reorganize the Entity Framework analysis service into a more maintainable structure with better separation of concerns, add comprehensive XML documentation, and introduce interface-based design for better testability.

Changes:

Refactored EfAnalysisService by splitting it into multiple focused classes within a new EfAnalysis namespace
Added comprehensive XML documentation comments across all public APIs
Introduced IGraphService interface for dependency injection
Removed the includePackages parameter from GetProjectGraph API
Updated SDK version in global.json
Added SonarQube workflow for code quality analysis

Reviewed changes

Copilot reviewed 36 out of 36 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
src/ProjGraph.Lib/Services/EfAnalysisService.cs	Deleted - code refactored into multiple files in EfAnalysis folder
src/ProjGraph.Lib/Services/EfAnalysis/*.cs	New files - decomposed EfAnalysisService into focused components (EntityAnalyzer, RelationshipAnalyzer, NavigationPropertyAnalyzer, etc.)
src/ProjGraph.Lib/Services/GraphService.cs	Added IGraphService interface implementation and XML documentation
src/ProjGraph.Lib/Interfaces/IGraphService.cs	New interface for graph service to enable dependency injection
src/ProjGraph.Mcp/Program.cs	Updated to use IGraphService interface and removed includePackages parameter
src/ProjGraph.Lib/Rendering/*.cs	Added XML documentation comments
src/ProjGraph.Lib/Parsers/*.cs	Added XML documentation comments
src/ProjGraph.Core/Models/*.cs	Added XML documentation and explicit enum values
src/ProjGraph.Cli/Commands/*.cs	Added XML documentation comments
tests/ProjGraph.Tests./*.cs	Updated imports to use new EfAnalysis namespace
global.json	Updated SDK version from 10.0.101 to 10.0.102
.github/workflows/sonarqube.yml	Added new workflow for SonarQube code quality analysis

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-19T15:17:22Z

tests/ProjGraph.Tests.Contract/McpContractTests.cs


        // Assert parameters match spec
        var parameters = method.GetParameters();
        parameters.Should().Contain(p => p.Name == "path" && p.ParameterType == typeof(string));


The removal of the includePackages parameter test assertion appears to be intentional as part of removing this feature from the GetProjectGraph tool. However, this represents a breaking change to the API. Consider adding a comment or documentation to explain why this parameter was removed, or verify that this change is documented in release notes.

Copilot · 2026-01-19T15:17:22Z

src/ProjGraph.Mcp/Program.cs

    public string GetProjectGraph(
-        [Description("Absolute path to the project or solution file.")] string path,
-        [Description("Include NuGet packages?")] bool includePackages = false)
+        [Description("Absolute path to the project or solution file.")]
+        string path)


The removal of the includePackages parameter from GetProjectGraph represents a breaking change to the public API. Users who were calling this method with this parameter will experience compilation errors. If this is intentional, ensure proper versioning (e.g., major version bump) and migration documentation are in place.

Copilot · 2026-01-19T15:17:23Z

src/ProjGraph.Lib/Services/GraphService.cs

+    /// It ensures that each project is only processed once by maintaining a set of discovered project paths.
+    /// If a project fails to parse, it is skipped, and the error is logged for debugging purposes.
+    /// </remarks>
+    private static HashSet<string> DiscoverProjectsRecursively(string rootProjectPath)


The return type of DiscoverProjectsRecursively has been changed from IEnumerable<string> to HashSet<string>. While this is more accurate to the implementation, it changes the public signature. If this method is used by external code, this represents a breaking change. Consider whether this method should remain private or if the return type change is acceptable for the API contract.

Copilot · 2026-01-19T15:17:23Z

src/ProjGraph.Core/Models/Project.cs

+    Library = 0,
+
+    /// <summary>
+    /// Represents an executable project.
+    /// </summary>
+    Executable = 1,
+
+    /// <summary>
+    /// Represents a test project.
+    /// </summary>
+    Test = 2,
+
+    /// <summary>
+    /// Represents a project of other types.
+    /// </summary>
+    Other = 3


While adding explicit values to enum members (0, 1, 2, 3) is generally a good practice for stability, this changes the binary representation if the previous implicit values were different. Verify that this doesn't affect serialization, database storage, or any external systems that depend on these enum values.

Copilot · 2026-01-19T15:17:23Z

src/ProjGraph.Lib/Services/EfAnalysis/EntityAnalyzer.cs

+            foreach (var arg in currentType.GetAttributes()
+                         .Where(attribute => attribute.AttributeClass?.Name == "PrimaryKeyAttribute")
+                         .SelectMany(attribute => attribute.ConstructorArguments))
+            {
+                if (arg.Value is string pkName)
+                {
+                    primaryKeyNames.Add(pkName);
+                }


This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.

Suggested change

foreach (var arg in currentType.GetAttributes()

.Where(attribute => attribute.AttributeClass?.Name == "PrimaryKeyAttribute")

.SelectMany(attribute => attribute.ConstructorArguments))

{

if (arg.Value is string pkName)

{

primaryKeyNames.Add(pkName);

}

foreach (var pkName in currentType.GetAttributes()

.Where(attribute => attribute.AttributeClass?.Name == "PrimaryKeyAttribute")

.SelectMany(attribute => attribute.ConstructorArguments)

.Select(arg => arg.Value)

.OfType<string>())

{

primaryKeyNames.Add(pkName);

Copilot · 2026-01-19T15:17:24Z

src/ProjGraph.Lib/Services/EfAnalysis/EfAnalysisService.cs

+        foreach (var entityFile in entityFiles.Values.Distinct())
+        {
+            var entityCode = await File.ReadAllTextAsync(entityFile);
+            syntaxTrees.Add(CSharpSyntaxTree.ParseText(entityCode));
+        }
+


This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.

Suggested change

foreach (var entityFile in entityFiles.Values.Distinct())

{

var entityCode = await File.ReadAllTextAsync(entityFile);

syntaxTrees.Add(CSharpSyntaxTree.ParseText(entityCode));

}

var entitySyntaxTreesTasks = entityFiles.Values

.Distinct()

.Select(async entityFile =>

{

var entityCode = await File.ReadAllTextAsync(entityFile);

return CSharpSyntaxTree.ParseText(entityCode);

});

var entitySyntaxTrees = await Task.WhenAll(entitySyntaxTreesTasks);

syntaxTrees.AddRange(entitySyntaxTrees);

Copilot · 2026-01-19T15:17:24Z

src/ProjGraph.Lib/Services/EfAnalysis/EntityFileDiscovery.cs

+        foreach (var entityFile in entityFiles.Values.Distinct())
+        {
+            var entityCode = await File.ReadAllTextAsync(entityFile);
+            var entityTree = CSharpSyntaxTree.ParseText(entityCode);
+            var entityRoot = await entityTree.GetRootAsync();
+


This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.

Suggested change

foreach (var entityFile in entityFiles.Values.Distinct())

{

var entityCode = await File.ReadAllTextAsync(entityFile);

var entityTree = CSharpSyntaxTree.ParseText(entityCode);

var entityRoot = await entityTree.GetRootAsync();

var entityRoots = await Task.WhenAll(

entityFiles.Values

.Distinct()

.Select(async entityFile =>

{

var entityCode = await File.ReadAllTextAsync(entityFile);

var entityTree = CSharpSyntaxTree.ParseText(entityCode);

return await entityTree.GetRootAsync();

}));

foreach (var entityRoot in entityRoots)

{

HandyS11 added 4 commits January 19, 2026 11:37

refactor: update SDK version and improve GraphService implementation

635ffe0

refactor: improve code quality and add SonarQube analysis configuration

c091b10

refactor: enhance code documentation and improve structure for clarity

d851e48

refactor: introduce EfAnalysis service and related extensions for imp…

f8fe624

…roved entity analysis

Copilot AI review requested due to automatic review settings January 19, 2026 15:13

Copilot started reviewing on behalf of HandyS11 January 19, 2026 15:14 View session

HandyS11 merged commit 8c012b4 into develop Jan 19, 2026
5 checks passed

Copilot AI reviewed Jan 19, 2026

View reviewed changes

HandyS11 deleted the refactor/improveCodeQuality branch January 19, 2026 15:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Refactor/improve code quality #5

Refactor/improve code quality #5

Uh oh!

HandyS11 commented Jan 19, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Refactor/improve code quality #5

Refactor/improve code quality #5

Uh oh!

Conversation

HandyS11 commented Jan 19, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants