postsharp / Metalama

Metalama is a Roslyn-based meta-programming framework. Use this repo to report bugs or ask questions.
164 stars 4 forks source link

Introducing Attributes can place xml comments in the wrong location #268

Closed jon-armen closed 3 months ago

jon-armen commented 4 months ago

Introducing an attribute may cause the xml comment to shift locations. This triggers warning CS1591: Missing XML comment for publicly visible type or member 'MyClass.PageSize'

Source

    /// <inheritdoc/>
    [Required]
    [Range(1, 1000)]
    public int? PageSize { get; init; }

Transformed

    [Range(1, 1000)]
[Required(ErrorMessage = "REQUIRED")]
    /// <inheritdoc/>

    public int? PageSize { get; init; }

Expected

    /// <inheritdoc/>
    [Range(1, 1000)]
[Required(ErrorMessage = "REQUIRED")]

    public int? PageSize { get; init; }

Aspect

        IReadOnlyCollection<IAttribute> requiredAttribute = builder.Target.Attributes.OfAttributeType(typeof(RequiredAttribute)).ToList();

        foreach (IAttribute attribute in requiredAttribute)
        {
            //builder.Advice.RemoveAttributes(builder.Target, attribute.Type);

            if (attribute.TryGetNamedArgument(nameof(RequiredAttribute.ErrorMessage), out _))
            {
                builder.Diagnostics.Report(DisallowedValidationErrorMessage.WithArguments(attribute));
            }
            else
            {
                builder.Advice.IntroduceAttribute(
                    builder.Target,
                    AttributeConstruction.Create(
                        attribute.Type.Constructors.Single(), attribute.ConstructorArguments, attribute.NamedArguments.Append(new KeyValuePair<string, TypedConstant>(nameof(RequiredAttribute.ErrorMessage), TypedConstant.Create("REQUIRED"))).ToList()),
                    OverrideStrategy.Override);
            }
        }
svick commented 4 months ago

Thanks for the report, I will work on fixing this. In the meantime, you can work around this by switching the order of attributes. I.e., this code:

    /// <inheritdoc/>
    [Range(1, 1000)]
    [Required]
    public int? PageSize { get; init; }

Is correctly transformed to:

    /// <inheritdoc/>
    [Range(1, 1000)]
[Required(ErrorMessage = "REQUIRED")]
    public int? PageSize { get; init; }
addabis commented 3 months ago

This bug has been fixed in Metalama 2024.0.10.