Further investigate regression in primary constructor decompilation

[siegfriedpammer]

After the recent fixes the issue with PCFieldInitTest becomes the same as the C3 test in #3452. The primary constructor parameter wrongly gets referenced outside of field initialization and generates an extra backing field when recompiled. For PCFieldInitTest it now looks like this:

internal abstract class PCFieldInitTest(StringComparison value)
{
	private StringComparison _value = value;

	public bool Func()
	{
		// Should be `_value` not `value`
		return value == StringComparison.Ordinal;
	}
}

Sorry if the naming difference was too subtle, I could have made it clearer.

Originally posted by @mmusu3 in #3610

[mmusu3]

I don't like generating extra work for people but I figure it's better to get this all done now while it's still fresh in mind. I spent the day going through my diffs from the changes in commits 9c8d1e4 through 7a5a2a0 and there are now many improvements but I found some extra test cases that still need work. Some are still recent regressions but others are older cases that existed before.

Input

class C4(string value)
{
    public object Obj { get; } = new object();
    public string Value { get; } = value;
}

class C5(C5.ValueArray array)
{
    public struct ValueArray
    {
        bool b;
        public bool[] ToArray() => null;
    }

    public bool[] Values = array.ToArray();
}

class BaseClass
{
    protected BaseClass(int value) { }
}

class C6(C6.Data2 data) : BaseClass(data.Value)
{
    public struct Data2 { public int Value { get; set; } }

    public Data2 Data => data;
}

struct S3<T>(T v)
{
    public T Value => v;
}

interface I1
{
    int Number { get; }
}

// May be the same issue as S3
struct S4<T>(int number)
    : IComparable<T> where T : I1
{
    public int CompareTo(T other) => number.CompareTo(other.Number);
}

Output

private class C4
{
    public object Obj { get; } = new object();

    public string Value { get; }

    public C4(string value)
    {
        Value = value;
        base..ctor();
    }
}

private class C5
{
	public struct ValueArray
	{
		private bool b;

		public bool[] ToArray()
		{
			return null;
		}
	}

	public bool[] Values;

	public C5(ValueArray array)
	{
		Values = array.ToArray();
		base..ctor();
	}
}

private class BaseClass
{
	protected BaseClass(int value)
	{
	}
}

private class C6(Data2 data) : BaseClass(<data>P.Value)
{
	public struct Data2
	{
		public int Value { get; set; }
	}

	public Data2 Data => data;
}

private struct S3<T>(T v)
{
    public T Value => <v>P;
}

private interface I1
{
    int Number { get; }
}

private struct S4<T>(int number) : IComparable<T> where T : I1
{
    public int CompareTo(T other)
    {
        return <number>P.CompareTo(other.Number);
    }
}

The primary constructor detection seems too eager and puts them in many places where they aren't needed even though PreferPrimaryConstructorIfPossible is false. This also causes issues with XML docs.

Input

// Requires <GenerateDocumentationFile>true</GenerateDocumentationFile>
class ConstructorXMLTest
{
    class BaseClass
    {
        protected BaseClass(object arg) { }
    }

    /// <summary>
    /// This is a information about the class type
    /// </summary>
    class Class1 : BaseClass
    {
        /// <summary>
        /// This is a different info about the constructor
        /// </summary>
        public Class1(int arg1, object arg2) // The issue does not happen with only one parameter
            : base(arg2)
        {
        }
    }
}

Output

internal class ConstructorXMLTest
{
	private class BaseClass
	{
		protected BaseClass(object arg)
		{
		}
	}

	/// <summary>
	/// This is a information about the class type
	/// </summary>
	private class Class1(int arg1, object arg2) : BaseClass(arg2)
	{
	}
}

[sonyps5201314]

In my original PR, C4, C5, and ConstructorXMLTest are all fine.

Moreover, if the solution in my PR were used, it would be completely unnecessary to introduce the PreferPrimaryConstructorIfPossible switch. Adding one setting increases the complexity of the software.
Also, I had already noticed that with the latest official code, many XML documentation comments in RestSharp are missing, whereas this rarely happens with my PR.
#3598

[mmusu3]

As of latest master 5545614 the decompiler produces the correct output for these cases. However, the test cases as they are in ILSpy now are not equivalent to the ones in this issue, being based on the old incorrect output which does not reflect the original problem:

ILSpy/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3452.cs

Line 30 in 5545614

return comparison;

ILSpy/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3610.cs

Line 48 in 5545614

return value == StringComparison.Ordinal;

Once the test cases have been updated this issue can be closed.