-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Fixed the issue where IsPrimaryConstructor's judgment was too conservative, which resulted in the inability to generate correct code in many scenarios. #3598
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
The current judgment is still not rigorous enough. I will refer to the algorithm in dotPeek to improve it tomorrow. The official can take their time to merge it. |
Shouldn't these be unit tests (rather than listing them in the pull request description)? |
…ative, which resulted in the inability to generate correct code in many scenarios.
d1fdf7e to
e8f32ec
Compare
…otPeek's judgment rules and the definition and characteristics of the primary constructor. However, it is found that dotPeek's judgment logic is also problematic. For example, it does not support the following scenario: ``` class WebPair(string name, string? value, ref readonly object encode) { public string Name { get; } = name; public string? Value { get; } = value; string? WebValue { get; } = encode is null ? "111" : value; string? WebValue2 { get; } = encode.ToString(); } ```
…m/restsharp/RestSharp/blob/dev/src/RestSharp/Parameters/DefaultParameters.cs was incorrectly judged as to whether the primary constructor was used due to incorrect initialization logic of isStruct and isInheritedRecord in the constructor of the RecordDecompiler class.
…e copy constructor first, otherwise it will be judged incorrectly whether the NamedParameter type in https://github.com/restsharp/RestSharp/blob/dev/src/RestSharp/Parameters/Parameter.cs uses the primary constructor.
…, it would cause the translation of the `DeserializationException` type in `https://github.com/restsharp/RestSharp/blob/dev/src/RestSharp/Serializers/DeseralizationException.cs` to fail, as it is a primary constructor declaration that calls a base constructor with two arguments. Additionally, dotPeek's `IsPrimaryConstructorFast` function has been introduced. This is because, after the aforementioned modification to `IsBaseCtorCall`, the `NamedParameter` type in RestSharp was being incorrectly identified as a primary constructor. Without this correction, it would lead to many compilation errors.
… not passed correctly for inherited classes declared using the primary constructor form (e.g., the `DeserializationException` type in RestSharp), and also fix the subsequent issue where an extra parenthesis '()' is output if the base class is an interface (e.g., the `XmlRestSerializer` type in RestSharp).
Now that I’ve fixed the other issues in the |
I'm not familiar enough with the Here are some new test cases, which will be supported after applying this pull request. However, using System.Diagnostics;
namespace TestClassPropertyInit
{
class WebPair1(string name)
{
public string Name { get; } = name;
}
class WebPair2(string name, string? value, ref readonly object encode)
{
public string Name { get; } = name;
}
class WebPair3(string name, string? value, bool encode = false)
{
public string Name { get; } = name;
public string? Value { get; } = value;
string? WebValue { get; } = encode ? "111" : value;
//string? WebValue { get; } = "111" + value;
}
class WebPair4(string name, string? value, ref readonly object encode)
{
public string Name { get; } = name;
public string? Value { get; } = value;
string? WebValue { get; } = encode is null ? "111" : value;
string? WebValue2 { get; } = encode.ToString();
}
class WebPair5(string name, string? value)
{
public string Name { get; } = name;
}
class WebPair6(string name, string? value, ref readonly object encode)
{
public string? Value { get; } = name;
public string Name { get; } = value;
string? WebValue { get; } = name != null ? "111" : value;
string? WebValue2 { get; } = value != null ? name : "222";
}
public class PersonPrimary_CaptureParams(string name, int age)
{
public string GetDetails() => $"{name}, {age}";
}
public class PersonPrimary(string name, int age)
{
private readonly string _name = name;
}
public class PersonRegular1
{
private readonly string _name = "name";
private readonly int _age = 23;
public PersonRegular1(string name, int age)
{
Thread.Sleep(1000);
_age = name.Length;
}
}
public class PersonRegular2
{
private readonly string _name = "name" + Environment.GetEnvironmentVariable("Path");
private readonly int _age = Environment.GetEnvironmentVariable("Path")?.Length ?? -1;
public PersonRegular2(string name, int age)
{
//Thread.Sleep(1000);
//_age = name.Length;
}
}
public class Person(string name, int age)
{
private readonly string _name = name;
private readonly int _age = age;
// 这个构造函数会调用主构造函数
public Person(string name, int age, string email) : this(name, age)
{
// 这里可以有函数体
if (string.IsNullOrEmpty(email))
throw new ArgumentException("Email cannot be empty");
Email = email;
Console.WriteLine($"Created person: {name}");
}
public string Email { get; init; }
}
static class Ensure
{
public static T NotNull<T>(T? value, string name) => value ?? throw new ArgumentNullException(name);
public static string NotEmptyString(object? value, string name)
{
var s = value as string ?? value?.ToString();
if (s == null) throw new ArgumentNullException(name);
return string.IsNullOrWhiteSpace(s) ? throw new ArgumentException("Parameter cannot be an empty string", name) : s;
}
}
/// <summary>
/// Parameter container for REST requests
/// </summary>
[DebuggerDisplay($"{{{nameof(DebuggerDisplay)}()}}")]
public abstract record Parameter
{
/// <summary>
/// Parameter container for REST requests
/// </summary>
protected Parameter(string? name, object? value, bool encode)
{
Name = name;
Value = value;
Encode = encode;
}
/// <summary>
/// Parameter name
/// </summary>
public string? Name { get; }
/// <summary>
/// Parameter value
/// </summary>
public object? Value { get; }
/// <summary>
/// Indicates if the parameter value should be encoded or not.
/// </summary>
public bool Encode { get; }
/// <summary>
/// Return a human-readable representation of this parameter
/// </summary>
/// <returns>String</returns>
public sealed override string ToString() => Value == null ? $"{Name}" : $"{Name}={ValueString}";
protected virtual string ValueString => Value?.ToString() ?? "null";
/// <summary>
/// Assists with debugging by displaying in the debugger output
/// </summary>
/// <returns></returns>
protected string DebuggerDisplay() => $"{GetType().Name.Replace("Parameter", "")} {ToString()}";
}
public record NamedParameter(string name, object? value, bool encode = true) : Parameter(Ensure.NotEmptyString(name, nameof(name)), value, encode)
{
}
public record QueryParameter(string name, object? value, bool encode = true) : NamedParameter(name, value, encode);
public class DeserializationException(string response, Exception innerException)
: Exception("Error occured while deserializing the response", innerException)
{
public string Response { get; } = response;
}
internal class Program
{
static void Main(string[] args)
{
Console.WriteLine("Hello, World!");
}
}
} |
some bad test cases:
In short:
You cannot assume that the number of constructor parameters and properties is equal, nor can you assume the relationship between the number of instructions and the number of parameters. You cannot assume that
xxxin an assignment statement:var property = xxx;is necessarily a value or object. It could be a complex expression like a conditional expression or a concatenation call. You only need to ensure that the value is ultimately assigned to the member field.