Change type of property job.commit-message-options.include-scope to bool

[rhyskoedijk]

Given this dependabot.yml:

version: 2
updates:
  - package-ecosystem: 'nuget'
    directory: '/'
    commit-message:
      include: 'scope'

Dependabot-CLI will generate this job.json:

{
    "job": {
        "package-manager": "nuget",
        "source": { ...omitted for brevity },
        "commit-message-options": {
            "include-scope": "true"
        }
    }
}

After dependabot/dependabot-core#11249 was merged in v0.292.0, the above config causes JSON deserialisation errors in the NuGet updater because include-scope is expected to be bool, not string.

Example:

updater | System.Text.Json.JsonException: The JSON value could not be converted to System.Nullable`1[System.Boolean]. Path: $.job.commit-message-options.include-scope | LineNumber: 0 | BytePositionInLine: 1505.
updater |  ---> System.InvalidOperationException: Cannot get the value of a token type 'String' as a boolean.
updater |    at System.Text.Json.ThrowHelper.ThrowInvalidOperationException_ExpectedBoolean(JsonTokenType tokenType)
updater |    at System.Text.Json.Utf8JsonReader.GetBoolean()
updater |    at System.Text.Json.Serialization.Converters.NullableConverter`1.Read(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options)
updater |    at System.Text.Json.Serialization.Metadata.JsonPropertyInfo`1.ReadJsonAndSetMember(Object obj, ReadStack& state, Utf8JsonReader& reader)
updater |    at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
updater |    at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value, Boolean& isPopulatedValue)
updater |    at System.Text.Json.Serialization.Metadata.JsonPropertyInfo`1.ReadJsonAndSetMember(Object obj, ReadStack& state, Utf8JsonReader& reader)
updater |    at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
updater |    at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value, Boolean& isPopulatedValue)
updater |    at System.Text.Json.Serialization.Metadata.JsonPropertyInfo`1.ReadJsonAndSetMember(Object obj, ReadStack& state, Utf8JsonReader& reader)
updater |    at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
updater |    at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value, Boolean& isPopulatedValue)
updater |    at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, T& value, JsonSerializerOptions options, ReadStack& state)
updater |    --- End of inner exception stack trace ---
updater |    at System.Text.Json.ThrowHelper.ReThrowWithPath(ReadStack& state, Utf8JsonReader& reader, Exception ex)
updater |    at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, T& value, JsonSerializerOptions options, ReadStack& state)
updater |    at System.Text.Json.Serialization.Metadata.JsonTypeInfo`1.Deserialize(Utf8JsonReader& reader, ReadStack& state)
updater |    at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan`1 utf8Json, JsonTypeInfo`1 jsonTypeInfo, Nullable`1 actualByteCount)
updater |    at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan`1 json, JsonTypeInfo`1 jsonTypeInfo)
updater |    at NuGetUpdater.Core.Run.RunWorker.Deserialize(String json) in /opt/nuget/lib/NuGetUpdater/NuGetUpdater.Core/Run/RunWorker.cs:line 721
updater |    at NuGetUpdater.Core.ExperimentsManager.FromJobFileAsync(String jobId, String jobFilePath) in /opt/nuget/lib/NuGetUpdater/NuGetUpdater.Core/ExperimentsManager.cs:line 46

This change fixes the error by converting the the property to bool so that it matches the dependabot-core model:

{
    "job": {
        "package-manager": "nuget",
        "source": { ...omitted for brevity },
        "commit-message-options": {
            "include-scope": true
        }
    }
}

[rhyskoedijk]

@brettfo tagging for visibility, is it possible to get your change updated in CLI too? It is currently causing errors when doing NuGet updates in tinglesoftware/dependabot-azure-devops, see #1548 (comment)

[brettfo]

@rhyskoedijk Is this what you had in mind? Allow the NuGet updater to parse true as well as "true" and null? dependabot/dependabot-core#11778

[rhyskoedijk]

Allow the NuGet updater to parse true as well as "true" and null? dependabot/dependabot-core#11778

@brettfo that looks like a better fix.

I'm still not really sure what is the correct type and value of this include-scope property. Was it intended to be a string all along, or should it really be bool? Should I abandon this PR, or is there value in enforcing a strict true/false here?

[brettfo]

Allow the NuGet updater to parse true as well as "true" and null? dependabot/dependabot-core#11778

@brettfo that looks like a better fix.

I'm still not really sure what is the correct type and value of this include-scope property. Was it intended to be a string all along, or should it really be bool? Should I abandon this PR, or is there value in enforcing a strict true/false here?

The Ruby code in dependabot-core always treats this as a bool so I think this change here should still happen and I'll try to get my C# change merged, too, just to make it extra resilient, especially if anybody is using an older CLI with a newer updater image.

[sachin-sandhu]

approving based on comments