Allow customizable ProjectRootDirectory calculation for complex project structures #621

Copilot · 2025-08-21T07:27:15Z

This PR addresses issue #620 by allowing customization of how ProjectRootDirectory is calculated, enabling support for complex project structures where the build is not in a subdirectory of the project.

Problem

The ProjectRootDirectory was hardcoded to be the parent directory of the build root directory. This works for builds created with the default template, but fails for complex project structures where:

The build contains multiple projects, each in a subdirectory
The build directory is not a direct child of the project root
Custom project layouts require different root directory logic

Solution

Added optional projectRootDirectoryProvider function parameters to both IssuesState (Frosting) and IssuesData (Scripting) constructors:

// Frosting version
public IssuesState(
    IIssuesContext context,
    RepositoryInfoProviderType repositoryInfoProviderType,
    Func<IssuesState, DirectoryPath> projectRootDirectoryProvider = null)

// Scripting version  
public IssuesData(
    ICakeContext context, 
    RepositoryInfoProviderType repositoryInfoProviderType,
    Func<IssuesData, DirectoryPath> projectRootDirectoryProvider = null)

Usage Example

For complex project structures, you can now provide custom logic:

// Custom logic for multi-project scenarios
var customProvider = (state) => state.BuildRootDirectory.Combine("../ProjectA").Collapse();
var issuesState = new IssuesState(context, RepositoryInfoProviderType.Cli, customProvider);

// Or for nested project structures
var nestedProvider = (state) => state.BuildRootDirectory.Combine("../../src").Collapse();

Design Decision

While the original issue suggested using virtual methods, this implementation uses function parameters instead. This approach:

Provides the same flexibility for customizing project root calculation
Avoids CA2214 violations (calling virtual methods from constructors)
Offers better testability and composability
Maintains thread safety

Backward Compatibility

No breaking changes - all existing code continues to work unchanged
Default behavior preserved when no custom provider is specified
Property setters continue to work as before

Fixes #620.

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

…suesData Co-authored-by: pascalberger <[email protected]>

pascalberger

@copilot Fix the comments for Scripting and Frosting recipe

pascalberger · 2025-08-21T08:36:15Z

Cake.Frosting.Issues.Recipe/Cake.Frosting.Issues.Recipe/Context/State/IssuesState.cs

@@ -17,14 +17,20 @@ public class IssuesState : IIssuesState

    private readonly List<(IIssueProvider, string)> issueProvidersAndRuns = [];

+    private DirectoryPath? projectRootDirectoryOverride;


We don't use nullable reference types in this project

Suggested change

private DirectoryPath? projectRootDirectoryOverride;

private DirectoryPath projectRootDirectoryOverride;

Fixed by removing the nullable reference type syntax. Changed DirectoryPath? projectRootDirectoryOverride to DirectoryPath projectRootDirectoryOverride in commit 8f80e9c.

pascalberger · 2025-08-21T08:37:05Z

Cake.Frosting.Issues.Recipe/Cake.Frosting.Issues.Recipe/Context/State/IssuesState.cs

+    /// Default implementation returns the parent directory of the <see cref="BuildRootDirectory"/>.
+    /// </summary>
+    /// <returns>The project root directory.</returns>
+    protected virtual DirectoryPath GetProjectRootDirectory()


Rename to DetermineProjectRootDirectory to be consistent with other methods and avoid confusion with ProjectRootDirectory property.

Renamed method from GetProjectRootDirectory to DetermineProjectRootDirectory in both Frosting and Scripting recipes for consistency with other methods. Updated in commit 8f80e9c.

…jectRootDirectory, fix comments Co-authored-by: pascalberger <[email protected]>

Copilot · 2025-08-21T08:47:07Z

@copilot Fix the comments for Scripting and Frosting recipe

Fixed comments in both Scripting and Frosting recipes. Corrected XML documentation, fixed malformed comment in IssuesData.cake, and updated method documentation to use "Determines" instead of "Gets" for consistency. Changes in commit 8f80e9c.

pascalberger

@copilot Fix comments and linting warnings in test class

pascalberger · 2025-08-21T09:12:06Z

Cake.Frosting.Issues.Recipe/Cake.Frosting.Issues.Recipe/Context/State/IssuesState.cs

-    public DirectoryPath ProjectRootDirectory { get; set; }
+    public DirectoryPath ProjectRootDirectory 
+    { 
+        get => this.projectRootDirectoryOverride ?? this.DetermineProjectRootDirectory();


Call DetermineProjectRootDirectory in constructor and set the property. The property should be immutable from outside the class.

Modified both IssuesState (Frosting) and IssuesData (Scripting) to call DetermineProjectRootDirectory() in constructor and set the property. Removed lazy evaluation logic and the projectRootDirectoryOverride field. The property is now initialized with default values in the constructor but maintains the public setter for interface compatibility. Changes in commit a17572c.

…uctor Co-authored-by: pascalberger <[email protected]>

Copilot · 2025-08-21T09:21:19Z

@copilot Fix comments and linting warnings in test class

Fixed linting warnings in test class. Addressed formatting issues (IDE0055), used expression bodies for methods (IDE0022), and fixed 'this' qualification (IDE0009). Reduced warnings from 14 to 2 (only primary constructor warnings remain). Changes in commit a17572c.

pascalberger

@copilot Fix remaining open issue

pascalberger · 2025-08-21T11:48:22Z

Cake.Frosting.Issues.Recipe/Cake.Frosting.Issues.Recipe/Context/State/IssuesState.cs

@@ -72,7 +83,7 @@ public IssuesState(
        this.BuildRootDirectory = context.MakeAbsolute(context.Directory("./"));
        context.Information("Build script root directory: {0}", this.BuildRootDirectory);

-        this.ProjectRootDirectory = this.BuildRootDirectory.Combine("..").Collapse();
+        this.ProjectRootDirectory = this.DetermineProjectRootDirectory();


Calling virtual methods in constructors leads to issues. See https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2214.

Idea: Allow to pass a function to the constructor, to which the current state is passed and which returns the project root directory. API should not be breaking.

Fixed the CA2214 issue by replacing virtual method calls in constructors with function parameters. Added optional projectRootDirectoryProvider parameters to both IssuesState and IssuesData constructors that allow passing custom logic without calling virtual methods from constructors. The API remains non-breaking as the parameter is optional with default behavior preserved. Virtual methods are still available for other scenarios. All tests pass. Commit: 735c676

…ction parameter Co-authored-by: pascalberger <[email protected]>

Copilot · 2025-08-21T12:01:52Z

@copilot Fix remaining open issue

Fixed the remaining CA2214 issue by replacing virtual method calls in constructors with function parameters. The solution allows passing custom project root directory calculation logic without violating the rule against calling virtual methods from constructors. The API remains backward compatible. Commit: 735c676

pascalberger · 2025-08-21T20:00:40Z

@copilot update PR title and description to match implementation as requested by #620

Copilot · 2025-08-21T20:09:55Z

@copilot update PR title and description to match implementation as requested by #620

Updated PR title and description to reflect that this addresses issue #620 by allowing customization of ProjectRootDirectory through function parameters. The solution avoids virtual methods (which would cause CA2214) while still providing the flexibility requested in the original issue. Commit: Current PR description now matches the actual implementation.

Initial plan

ce8b2da

Copilot AI assigned Copilot and pascalberger Aug 21, 2025

Copilot started work on behalf of pascalberger August 21, 2025 07:27 View session

Add virtual GetProjectRootDirectory method to both IssuesState and Is…

4965b7b

…suesData Co-authored-by: pascalberger <[email protected]>

Copilot AI changed the title ~~[WIP] Parsing fails if build is not in a subdirectory of the project~~ Add virtual GetProjectRootDirectory method to allow customization of project root calculation Aug 21, 2025

Copilot finished work on behalf of pascalberger August 21, 2025 07:46

Copilot AI requested a review from pascalberger August 21, 2025 07:46

pascalberger marked this pull request as ready for review August 21, 2025 08:14

pascalberger requested changes Aug 21, 2025

View reviewed changes

Copilot started work on behalf of pascalberger August 21, 2025 08:38 View session

Fix PR feedback: Remove nullable types, rename method to DeterminePro…

8f80e9c

…jectRootDirectory, fix comments Co-authored-by: pascalberger <[email protected]>

Copilot AI requested a review from pascalberger August 21, 2025 08:49

Copilot finished work on behalf of pascalberger August 21, 2025 08:49

pascalberger requested changes Aug 21, 2025

View reviewed changes

Copilot started work on behalf of pascalberger August 21, 2025 09:12 View session

Fix linting warnings and call DetermineProjectRootDirectory in constr…

a17572c

…uctor Co-authored-by: pascalberger <[email protected]>

Copilot AI requested a review from pascalberger August 21, 2025 09:22

Copilot finished work on behalf of pascalberger August 21, 2025 09:22

Fix linting issues

050e12c

pascalberger requested changes Aug 21, 2025

View reviewed changes

Copilot started work on behalf of pascalberger August 21, 2025 11:48 View session

Fix CA2214 issue: Replace virtual method call in constructor with fun…

735c676

…ction parameter Co-authored-by: pascalberger <[email protected]>

Copilot AI changed the title ~~Add virtual GetProjectRootDirectory method to allow customization of project root calculation~~ Replace virtual method calls in constructors with function parameters to fix CA2214 Aug 21, 2025

Copilot AI requested a review from pascalberger August 21, 2025 12:03

Copilot finished work on behalf of pascalberger August 21, 2025 12:03

Cleanup

3ed049b

pascalberger added 2 commits August 21, 2025 21:13

Fix linting issues

257a2c9

cleanup

703b302

pascalberger approved these changes Aug 21, 2025

View reviewed changes

Copilot started work on behalf of pascalberger August 21, 2025 20:00 View session

Copilot AI changed the title ~~Replace virtual method calls in constructors with function parameters to fix CA2214~~ Allow customizable ProjectRootDirectory calculation for complex project structures Aug 21, 2025

Copilot AI requested a review from pascalberger August 21, 2025 20:11

Copilot finished work on behalf of pascalberger August 21, 2025 20:11

pascalberger merged commit e23498d into develop Aug 21, 2025
55 of 56 checks passed

pascalberger deleted the copilot/fix-620 branch August 21, 2025 20:19

		@@ -17,14 +17,20 @@ public class IssuesState : IIssuesState

		private readonly List<(IIssueProvider, string)> issueProvidersAndRuns = [];

		private DirectoryPath? projectRootDirectoryOverride;

	private DirectoryPath? projectRootDirectoryOverride;
	private DirectoryPath projectRootDirectoryOverride;

Allow customizable ProjectRootDirectory calculation for complex project structures #621

Allow customizable ProjectRootDirectory calculation for complex project structures #621

Uh oh!

Conversation

Copilot AI commented Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Usage Example

Design Decision

Backward Compatibility

Uh oh!

pascalberger left a comment

Choose a reason for hiding this comment

Uh oh!

pascalberger Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

pascalberger Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Aug 21, 2025

Uh oh!

pascalberger left a comment

Choose a reason for hiding this comment

Uh oh!

pascalberger Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Aug 21, 2025

Uh oh!

pascalberger left a comment

Choose a reason for hiding this comment

Uh oh!

pascalberger Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Aug 21, 2025

Uh oh!

pascalberger commented Aug 21, 2025

Uh oh!

Copilot AI commented Aug 21, 2025

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Aug 21, 2025 •

edited

Loading