[API Compat] add rule to make sure that parameter default value don't change

[andriipatsula]

Currently, the ApiCompat does not detect the change of default value for parameters.

void Execute(int i) == void Execute(int i = 0) == void Execute(int i = 2)

@dotnet/area-infrastructure-libraries a new issue has been filed in the ApiCompat area, please triage

[KalleOlaviNiemitalo]

Make sure that the rule also works with void M(decimal d = 1.999m), which is encoded using DecimalConstantAttribute rather than the Constant table. Maybe the Roslyn API handles this automatically, but it's still worth testing.

[ViktorHofer]

@KalleOlaviNiemitalo or @andriipatsula would you like to help implementing the rule? The work for that is very much self-contained:

1. Add a new file similar to https://github.com/dotnet/sdk/blob/main/src/ApiCompat/Microsoft.DotNet.ApiCompatibility/Rules/CannotChangeParameterName.cs
2. Add it to the rule factory:

   sdk/src/ApiCompat/Microsoft.DotNet.ApiCompatibility/Rules/RuleFactory.cs

   Line 31 in 80c0e64

   public IRule[] CreateRules(RuleSettings settings, IRuleRegistrationContext context)

3. Add some tests to verify the behavior

[KalleOlaviNiemitalo]

I don't think I'll be implementing this.

[ViktorHofer]

@andriipatsula based on https://learn.microsoft.com/en-us/dotnet/core/compatibility/library-change-rules#properties-fields-parameters-and-return-values, changing a parameter's default value isn't a binary breaking change as a parameter's default value is not part of the public contract.
In certain cases, such a modification could be considered a behavioral change though but which isn't what ApiCompat validates.

Strict mode which essentially is a 1:1 equality check should flag these changes though. Can you please verify that's the case?

[danmoseley]

@ViktorHofer you might consider adding 'help wanted' to this and other apicompat issues that community folks might do.

@andriipatsula do you have any interest in this one? (or any of the other apicompat issues)

[andriipatsula]

Hello @danmoseley

do you have any interest in this one? (or any of the other apicompat issues)

I cannot invest much effort into it right now. I am monitoring several apicompat issues, and eventually, I will start working on one.

[ViktorHofer]

@danmoseley I'm not yet sure if this is actually something that we want to implement in APICompat at all based on the above:

changing a parameter's default value isn't a binary breaking change as a parameter's default value is not part of the public contract.
In certain cases, such a modification could be considered a behavioral change though but which isn't what ApiCompat validates.

@ericstj thoughts?

[ericstj]

I'd put this is the same boat as parameter name changes. These are also not binary breaking but are "source breaking" and potentially functionally breaking.

In that case it's an optional rule that can be enabled.

sdk/src/ApiCompat/Microsoft.DotNet.ApiCompat.Tool/Program.cs

Lines 73 to 77 in 8089630

	CliOption<bool> enableRuleCannotChangeParameterNameOption = new("--enable-rule-cannot-change-parameter-name")
	{
	Description = "If true, enables rule to check that the parameter names between public methods do not change.",
	Recursive = true
	};

Perhaps we might consider a more extensible mechanism for describing optional rules or even a rule level system 🤔

[ShreyasJejurkar]

Can I take this one to implement based on @ericstj's comment, if we don't have any major plans on changing how the optional mechanism works as of now in ApiCompact?

I hope I can refer to CannotChangeParameterName` rule when implementing this one.