Skip to content
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

Comment between attributes violates SA1515 #1252

Closed
maurobringolf opened this issue May 6, 2024 · 1 comment · Fixed by #1277
Closed

Comment between attributes violates SA1515 #1252

maurobringolf opened this issue May 6, 2024 · 1 comment · Fixed by #1277

Comments

@maurobringolf
Copy link

Input:

public class C {

    [A]
    
    // moves up
    [B]
    public void A()
    {
        var z = 1;
        
        // stays here
        var y = 2;
    }
}

Output:

public class C
{
    [A]
    // moves up
    [B]
    public void A()
    {
        var z = 1;

        // stays here
        var y = 2;
    }
}

Expected behavior:
The autoformatting causes the code not to comply with SA1515.
According to the docs this rule is not in conflict with the CSharpier format.
I expect the output to build without formatting warnings when configured as described.


As far as I can tell this can be resolved in two ways:

  1. Keep whitespace before comments in all cases (if that's possible).
  2. Add SA1515 to the to-be-disabled rules

I would favor the second option.

@belav belav modified the milestones: Planned, 0.29.0 May 7, 2024
@belav
Copy link
Owner

belav commented Jun 2, 2024

There is already logic for forcing a new line before the first attribute and removing blank lines between attributes. After looking into it - getting all of that to play nice with keeping/adding new lines before comments between attributes won't be straightforward. I'm good with adding SA1515 to the to-be-disabled list.

belav added a commit that referenced this issue Jun 2, 2024
belav added a commit that referenced this issue Jun 2, 2024
@belav belav removed this from the 0.29.0 milestone Jun 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants