fix: throw on missing properties and use integer fallbacks

Author: robertcoltheart

BREAKING_CHANGES.md

@@ -1,6 +1,3 @@
-## v6.8.0
-* When using `{env:VAR_NAME}` or `{BranchName}` syntax in labels or version formatting, a missing environment variable or placeholder no longer throws and instead returns a blank value.
-
 ## v6.2.0
 
 * The configuration property `label-number-pattern` was removed. The functionality can be still used by changing the label and the branch name regular expression for pull-request branches.

docs/input/docs/reference/mdsource/configuration.source.md

@@ -504,7 +504,7 @@ of `alpha.foo` with `label: 'alpha.{BranchName}'` and `regex: '^features?[\/-](?
 
 Another example: branch `features/sc-12345/some-description` would become a pre-release label of `sc-12345` with `label: '{StoryNo}'` and `regex: '^features?[\/-](?<StoryNo>sc-\d+)[-/].+'`.
 
-You can also use environment variable placeholders with the `{env:VARIABLE_NAME}` syntax. Environment variable placeholders can also be combined with regex placeholders, for example `{BranchName}-{env:VARIABLE_NAME}`, and support fallback values using the `{env:VARIABLE_NAME ?? "fallback"}` syntax. These can all be combined with cascading fallbacks using environment variables and placeholders like this: `{env:VARIABLE_NAME ?? BranchName ?? "fallback"}`. If no value is found and no fallback is provided, the placeholder expands to an empty string.
+You can also use environment variable placeholders with the `{env:VARIABLE_NAME}` syntax. Environment variable placeholders can also be combined with regex placeholders, for example `{BranchName}-{env:VARIABLE_NAME}`, and support fallback values using the `{env:VARIABLE_NAME ?? "fallback"}` syntax. These can be combined with cascading fallbacks using environment variables and placeholders like this: `{env:VARIABLE_NAME ?? BranchName ?? "fallback"}`.
 
 **Note:** To clear a default use an empty string: `label: ''`
 

src/GitVersion.Configuration.Tests/Configuration/ConfigurationExtensionsTests.cs

@@ -127,7 +127,7 @@ public void EnsureGetBranchSpecificLabelWorksWithoutEnvironmentWhenNoEnvPlacehol
     }
 
     [Test]
-    public void EnsureGetBranchSpecificLabelReturnsEmptyWhenEnvVarMissing()
+    public void EnsureGetBranchSpecificLabelThrowsWhenEnvVarMissing()
     {
         var environment = new TestEnvironment();
         // Do not set MISSING_VAR
@@ -140,7 +140,7 @@ public void EnsureGetBranchSpecificLabelReturnsEmptyWhenEnvVarMissing()
             .Build();
 
         var effectiveConfiguration = configuration.GetEffectiveConfiguration(ReferenceName.FromBranchName(BranchName));
-        var actual = effectiveConfiguration.GetBranchSpecificLabel(ReferenceName.FromBranchName(BranchName), null, environment);
-        actual.ShouldBe("pr-");
+        Should.Throw<ArgumentException>(() =>
+            effectiveConfiguration.GetBranchSpecificLabel(ReferenceName.FromBranchName(BranchName), null, environment));
     }
 }

src/GitVersion.Core.Tests/Extensions/StringFormatWithExtensionTests.cs

@@ -56,7 +56,7 @@ public void FormatWithEnvVarTokenWithFallback()
     {
         this.environment.SetEnvironmentVariable("GIT_VERSION_TEST_VAR", "Env Var Value");
         var propertyObject = new { };
-        const string target = "{env:GIT_VERSION_TEST_VAR ?? fallback}";
+        const string target = "{env:GIT_VERSION_TEST_VAR ?? \"fallback\"}";
         const string expected = "Env Var Value";
         var actual = target.FormatWith(propertyObject, this.environment);
         Assert.That(actual, Is.EqualTo(expected));
@@ -67,7 +67,7 @@ public void FormatWithUnsetEnvVarToken_WithFallback()
     {
         this.environment.SetEnvironmentVariable("GIT_VERSION_UNSET_TEST_VAR", null);
         var propertyObject = new { };
-        const string target = "{env:GIT_VERSION_UNSET_TEST_VAR ?? fallback}";
+        const string target = "{env:GIT_VERSION_UNSET_TEST_VAR ?? \"fallback\"}";
         const string expected = "fallback";
         var actual = target.FormatWith(propertyObject, this.environment);
         Assert.That(actual, Is.EqualTo(expected));
@@ -79,9 +79,7 @@ public void FormatWithUnsetEnvVarToken_WithoutFallback()
         this.environment.SetEnvironmentVariable("GIT_VERSION_UNSET_TEST_VAR", null);
         var propertyObject = new { };
         const string target = "{env:GIT_VERSION_UNSET_TEST_VAR}";
-        const string expected = "";
-        var actual = target.FormatWith(propertyObject, this.environment);
-        Assert.That(actual, Is.EqualTo(expected));
+        Assert.Throws<ArgumentException>(() => target.FormatWith(propertyObject, this.environment));
     }
 
     [Test]
@@ -100,17 +98,15 @@ public void FormatWithMultipleEnvVars()
     public void FormatWithMultipleEnvChars()
     {
         var propertyObject = new { };
-        const string target = "{env:env:GIT_VERSION_TEST_VAR_1} and {env:DUMMY_VAR ?? fallback}";
-        const string expected = " and fallback";
-        var actual = target.FormatWith(propertyObject, this.environment);
-        Assert.That(actual, Is.EqualTo(expected));
+        const string target = "{env:env:GIT_VERSION_TEST_VAR_1} and {env:DUMMY_VAR ?? \"fallback\"}";
+        Assert.Throws<ArgumentException>(() => target.FormatWith(propertyObject, this.environment));
     }
 
     [Test]
     public void FormatWithMultipleFallbackChars()
     {
         var propertyObject = new { };
-        const string target = " and {env:DUMMY_VAR ??? fallback}";
+        const string target = " and {env:DUMMY_VAR ??? \"fallback\"}";
         Assert.Throws<FormatException>(() => target.FormatWith(propertyObject, this.environment));
     }
 
@@ -119,17 +115,15 @@ public void FormatWithSingleFallbackChar()
     {
         this.environment.SetEnvironmentVariable("DUMMY_ENV_VAR", "DummyVal");
         var propertyObject = new { };
-        const string target = "{en:DUMMY_ENV_VAR} and {env:DUMMY_ENV_VAR??fallback}";
-        const string expected = " and DummyVal";
-        var actual = target.FormatWith(propertyObject, this.environment);
-        Assert.That(actual, Is.EqualTo(expected));
+        const string target = "{en:DUMMY_ENV_VAR} and {env:DUMMY_ENV_VAR??\"fallback\"}";
+        Assert.Throws<ArgumentException>(() => target.FormatWith(propertyObject, this.environment));
     }
 
     [Test]
     public void FormatWithNullPropagationWithMultipleSpaces()
     {
         var propertyObject = new { SomeProperty = "Some Value" };
-        const string target = "{SomeProperty} and {env:DUMMY_ENV_VAR  ??  fallback}";
+        const string target = "{SomeProperty} and {env:DUMMY_ENV_VAR  ??  \"fallback\"}";
         const string expected = "Some Value and fallback";
         var actual = target.FormatWith(propertyObject, this.environment);
         Assert.That(actual, Is.EqualTo(expected));
@@ -157,13 +151,11 @@ public void FormatWithMissingEnvAndPropertyFallback()
     }
 
     [Test]
-    public void FormatWithMultiplePropertiesAndFallback()
+    public void FormatWithMultiplePropertiesAndNoFallback()
     {
         var propertyObject = new { };
-        const string target = "{SomeProperty ?? SomeOtherProperty ?? MissingPropTreatedAsLiteral}";
-        const string expected = "MissingPropTreatedAsLiteral";
-        var actual = target.FormatWith(propertyObject, this.environment);
-        Assert.That(actual, Is.EqualTo(expected));
+        const string target = "{SomeProperty ?? SomeOtherProperty ?? MissingProp}";
+        Assert.Throws<ArgumentException>(() => target.FormatWith(propertyObject, this.environment));
     }
 
     [Test]
@@ -180,12 +172,22 @@ public void FormatWithMultiplePropertiesAndQuotedFallback()
     public void FormatWithMultipleAvailablePropertiesAndFallback()
     {
         var propertyObject = new { SomeOtherProperty = "Some-Value" };
-        const string target = "{SomeProperty ?? SomeOtherProperty ?? fallback}";
+        const string target = "{SomeProperty ?? SomeOtherProperty ?? \"fallback\"}";
         const string expected = "Some-Value";
         var actual = target.FormatWith(propertyObject, this.environment);
         Assert.That(actual, Is.EqualTo(expected));
     }
 
+    [Test]
+    public void FormatWithMissingPropertiesAndIntegerFallback()
+    {
+        var propertyObject = new { };
+        const string target = "{SomeProperty ?? SomeOtherProperty ?? 47}";
+        const string expected = "47";
+        var actual = target.FormatWith(propertyObject, this.environment);
+        Assert.That(actual, Is.EqualTo(expected));
+    }
+
     [Test]
     public void FormatWithPropertyAndEnvAndFormatters()
     {
@@ -197,6 +199,38 @@ public void FormatWithPropertyAndEnvAndFormatters()
         Assert.That(actual, Is.EqualTo(expected));
     }
 
+    [TestCase("{env:VARIABLE ?? \"\"}", null, null, "")]
+    [TestCase("{env:MISSING ?? env:VARIABLE}", "Var", null, "Var")]
+    [TestCase("{Property ?? \"\"}", null, null, "")]
+    [TestCase("{Property ?? 47}", null, null, "47")]
+    [TestCase("{Property ?? env:VARIABLE}", null, "Branch", "Branch")]
+    [TestCase("{Property ?? env:VARIABLE ?? \"\"}", null, null, "")]
+    [TestCase("{Property ?? env:VARIABLE ?? 42}", null, null, "42")]
+    public void FormatWith_EnvVarAndPropertyAndFallback_DoesNotThrow(string input, string? envVar, string? property, string expected)
+    {
+        if (envVar != null)
+        {
+            this.environment.SetEnvironmentVariable("VARIABLE", envVar);
+        }
+
+        object propertyObject = property != null
+            ? new { Property = property }
+            : new { };
+
+        var actual = input.FormatWith(propertyObject, this.environment);
+        Assert.That(actual, Is.EqualTo(expected));
+    }
+
+    [TestCase("{env:VARIABLE}")]
+    [TestCase("{Property}")]
+    [TestCase("{Property ?? env:VARIABLE}")]
+    [TestCase("{Property ?? Property}")]
+    public void FormatWith_MissingEnvVarOrPropertyAndNoFallback_Throws(string input)
+    {
+        object propertyObject = new { };
+        Assert.Throws<ArgumentException>(() => input.FormatWith(propertyObject, this.environment));
+    }
+
     [Test]
     public void FormatEnvVar_WithFallback_QuotedAndEmpty()
     {
@@ -247,7 +281,7 @@ public void FormatProperty_NullInteger()
     public void FormatProperty_String_WithFallback()
     {
         var propertyObject = new { Property = "Value" };
-        const string target = "{Property ?? fallback}";
+        const string target = "{Property ?? \"fallback\"}";
         var actual = target.FormatWith(propertyObject, this.environment);
         Assert.That(actual, Is.EqualTo("Value"));
     }
@@ -256,7 +290,7 @@ public void FormatProperty_String_WithFallback()
     public void FormatProperty_Integer_WithFallback()
     {
         var propertyObject = new { Property = 42 };
-        const string target = "{Property ?? fallback}";
+        const string target = "{Property ?? \"fallback\"}";
         var actual = target.FormatWith(propertyObject, this.environment);
         Assert.That(actual, Is.EqualTo("42"));
     }
@@ -265,7 +299,7 @@ public void FormatProperty_Integer_WithFallback()
     public void FormatProperty_NullObject_WithFallback()
     {
         var propertyObject = new { Property = (object?)null };
-        const string target = "{Property ?? fallback}";
+        const string target = "{Property ?? \"fallback\"}";
         var actual = target.FormatWith(propertyObject, this.environment);
         Assert.That(actual, Is.EqualTo("fallback"));
     }
@@ -274,9 +308,9 @@ public void FormatProperty_NullObject_WithFallback()
     public void FormatProperty_NullInteger_WithFallback()
     {
         var propertyObject = new { Property = (int?)null };
-        const string target = "{Property ?? fallback}";
+        const string target = "{Property ?? 43}";
         var actual = target.FormatWith(propertyObject, this.environment);
-        Assert.That(actual, Is.EqualTo("fallback"));
+        Assert.That(actual, Is.EqualTo("43"));
     }
 
     [Test]

src/GitVersion.Core.Tests/Formatting/LabelTokenizerTests.cs

@@ -24,14 +24,17 @@ public class LabelTokenizerTests
     [TestCase("Pat\"tern")]
     public void ParseTokens_InvalidLiterals_Throws(string input) => AssertThrows(input);
 
-    [TestCase("Prop ?? literal", "Prop", "literal")]
-    [TestCase("Prop??literal", "Prop", "literal")]
-    [TestCase("Prop ?? literal ?? fallback", "Prop", "literal", "fallback")]
-    [TestCase("Prop ??literal?? fallback", "Prop", "literal", "fallback")]
-    [TestCase("Prop ?? \"literal\" ?? fallback", "Prop", "literal", "fallback")]
-    [TestCase("Prop:format ?? \"literal\" ?? fallback", "Prop", "literal", "fallback")]
-    [TestCase("env:Prop ?? \"literal\" ?? fallback", "Prop", "literal", "fallback")]
-    [TestCase("env:Prop:format ?? \"literal\" ?? fallback", "Prop", "literal", "fallback")]
+    [TestCase("Prop1 ?? Prop2", "Prop1", "Prop2")]
+    [TestCase("Prop1??Prop2", "Prop1", "Prop2")]
+    [TestCase("Prop1??Prop2??42", "Prop1", "Prop2", "42")]
+    [TestCase("Prop1??Prop2??\"42\"", "Prop1", "Prop2", "42")]
+    [TestCase("Prop1 ?? Prop2 ?? \"fallback\"", "Prop1", "Prop2", "fallback")]
+    [TestCase("Prop1 ??Prop2?? \"fallback\"", "Prop1", "Prop2", "fallback")]
+    [TestCase("Prop1 ?? Prop2 ?? 42", "Prop1", "Prop2", "42")]
+    [TestCase("Prop1:format ?? Prop2 ?? \"fallback\"", "Prop1", "Prop2", "fallback")]
+    [TestCase("env:Env1 ?? Prop2 ?? \"fallback\"", "Env1", "Prop2", "fallback")]
+    [TestCase("env:Env1:format ?? \"literal\" ?? \"fallback\"", "Env1", "literal", "fallback")]
+    [TestCase("env:Env1 ?? 42", "Env1", "42")]
     public void ParseTokens_ValidIdentifiers_ReturnsValid(string input, params string[] expected) => AssertTokens(input, expected);
 
     [TestCase("Prop ??? literal")]

src/GitVersion.Core/Formatting/LabelTokenizer.cs

@@ -52,6 +52,11 @@ private static LabelToken ParseToken(string identifier)
             return new LabelToken(identifier[1..^1], LabelTokenType.Literal);
         }
 
+        if (int.TryParse(identifier, out _))
+        {
+            return new LabelToken(identifier, LabelTokenType.Literal);
+        }
+
         var (propertyName, propertyFormat) = ParseKeyAndFormat(identifier);
 
         return new LabelToken(propertyName, LabelTokenType.Property, propertyFormat);

src/GitVersion.Core/Formatting/MemberResolver.cs

@@ -15,7 +15,7 @@ public MemberInfo[] ResolveMemberPath(Type type, string memberExpression)
             {
                 var recursivePath = FindMemberRecursive(type, memberName, []);
                 return recursivePath == null
-                    ? []
+                    ? throw new ArgumentException($"'{memberName}' is not a property or field on type '{type.Name}'")
                     : [.. recursivePath];
             }
 

src/GitVersion.Core/Formatting/StringFormatWithExtension.cs

@@ -84,38 +84,45 @@ private static string EvaluateMatch(string input, Func<string, string?> memberEv
         var tokenizer = new LabelTokenizer(input);
         var tokens = tokenizer.ParseTokens().ToArray();
 
+        Exception? lastException = null;
+
         foreach (var token in tokens)
         {
             if (token.Type == LabelTokenType.Literal)
             {
                 return token.Name;
             }
 
-            var value = token.Type == LabelTokenType.Environment
-                ? environment.GetEnvironmentVariable(InputSanitizer.SanitizeEnvVarName(token.Name))
-                : memberEvaluator(token.Name);
-
-            if (value is not null && !string.IsNullOrEmpty(token.Format))
+            try
             {
-                if (ValueFormatter.Default.TryFormat(value, InputSanitizer.SanitizeFormat(token.Format), out var formatted))
+                var value = token.Type == LabelTokenType.Environment
+                    ? EvaluateEnvVar(token.Name, environment)
+                    : memberEvaluator(token.Name);
+
+                if (value is not null && !string.IsNullOrEmpty(token.Format))
                 {
-                    return formatted;
+                    if (ValueFormatter.Default.TryFormat(value, InputSanitizer.SanitizeFormat(token.Format), out var formatted))
+                    {
+                        return formatted;
+                    }
+
+                    return value;
                 }
 
-                return value;
+                if (value is not null)
+                {
+                    return value;
+                }
             }
-
-            if (value is not null)
+            catch (Exception e)
             {
-                return value;
+                lastException = e;
             }
         }
 
-        var lastToken = tokens.LastOrDefault();
-
-        if (tokens.Length > 1 && lastToken?.Type == LabelTokenType.Property)
+        if (lastException != null)
         {
-            return lastToken.Name;
+            throw lastException;
         }
 
         return string.Empty;
@@ -125,12 +132,6 @@ private static string EvaluateMatch(string input, Func<string, string?> memberEv
     {
         var safeMember = InputSanitizer.SanitizeMemberName(member);
         var memberPath = MemberResolver.ResolveMemberPath(source.GetType(), safeMember);
-
-        if (memberPath.Length == 0)
-        {
-            return null;
-        }
-
         var getter = ExpressionCompiler.CompileGetter(source.GetType(), memberPath);
 
         var value = getter(source);
@@ -143,8 +144,15 @@ private static string EvaluateMatch(string input, Func<string, string?> memberEv
         var safeMember = InputSanitizer.SanitizeMemberName(member);
 
         if (!source.TryGetValue(safeMember, out var value))
-            return null;
+            throw new ArgumentException($"'{safeMember}' is not a valid placeholder");
 
         return value?.ToString();
     }
+
+    private static string EvaluateEnvVar(string name, IEnvironment environment)
+    {
+        var safeName = InputSanitizer.SanitizeEnvVarName(name);
+
+        return environment.GetEnvironmentVariable(name) ?? throw new ArgumentException($"Environment variable {safeName} not found and no fallback provided");
+    }
 }