feat: add support for environment variables in branch labels

[robertcoltheart]

Description

Supports environment variable substitution in the label configuration for a branch using the established format used elsewhere. Fallbacks are also supported if the environment variable is not found. The changes are backwards compatible and should not fail for existing configuration.

Documentation detailing the changes has been added to configuration.md.

Related Issue

Resolves #4745

Motivation and Context

Allow labels to include environment variables as well as regex-captured variables. This is useful for CICD environments like GitHub where the pull request head_ref can be provided instead of the merge commit target.

How Has This Been Tested?

Unit tests covering the new functionality have been added.

Screenshots (if appropriate):

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

[robertcoltheart]

1 check is failing because the Nuke website is down.

[robertcoltheart]

Nuke is abandoned now and the website is unlikely to come back online. This was failing the build.

[robertcoltheart]

Throws if environment variable is not found and has no fallback. This is deliberate to stay consistent with other usages of environment variables, for example in assembly file version.

[robertcoltheart]

@HHobeck @arturcic Fixed a couple of issues that I missed. This is ready for review and merge now, thanks!

[HHobeck]

Here are a few suggestions regarding this implementation:

1. Use object instead of generics: Change the generic type T to object. It makes more sense here since a generic type parameter provides no extra benefit for this use case.

2. Clarify supported types: The method currently supports instances of type object and IDictionary<string, object>, but this intent is not clearly expressed in the method body. I suggest splitting this into explicit overloads for better readability, like this:

internal static class StringFormatWithExtension
{
    extension(string template)
    {
        public string FormatWith(object? source, IEnvironment environment)
        {
            object? GetFromSource(string value)
            {
                // ...
            }

            object? GetFromEnvironment(string value) => environment.GetEnvironmentVariable(value);
            
            return template.FormatWith(GetFromSource, GetFromEnvironment);
        }

        public string FormatWith(IDictionary<string, object> source, IEnvironment environment)
        {
            object? GetFromSource(string value)
            {
                source.TryGetValue(value, out var result);
                return result;
            }

            object? GetFromEnvironment(string value) => environment.GetEnvironmentVariable(value);

            return template.FormatWith(GetFromSource, GetFromEnvironment);
        }

        private string FormatWith(
            Func<string, object?> getFromSource, Func<string, object?> getFromEnvironment)
        {
            // ...
        }
    }
}

3. Allow flexible placeholder combinations: It does not make sense to restrict how placeholders can occur. Why shouldn't all of the following combinations be valid?

• {JustAProperty}
• {JustAProperty ?? "JustAFallback"}
• {JustAProperty} ?? {env:JustAVariable}
• {JustAProperty} ?? {env:JustAVariable} ?? "JustAFallback"
• {env:JustAVariable}
• {env:JustAVariable} ?? "JustAFallback"
• {env:JustAVariable} ?? {JustAProperty}
• {env:JustAVariable} ?? {JustAProperty} ?? "JustAFallback"

[robertcoltheart]

1 and 2, fair enough, will make these changes.

For 3, it was written this way (and I'm guessing the original implementor in #4746 did it this way for the same reason) to remain compatible/easy to understand as the existing configuration options for label and for assembly-file-versioning-format, where the {env:} syntax is derived from. The understanding I have is that {Name} syntax is reserved for regex captures, whereas {env:} is used for environment variables. I think it would be more confusing given the language that the configuration has cultivated to conflate these 2 concepts.

If the longer-term intention is to further enhance the regex/env var to support this conflation, I'd suggest doing this in a follow up change rather than here. Let me know your thoughts on this.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.9% Duplication on New Code

See analysis details on SonarQube Cloud