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

Formatting may conflict with StyleCopAnalayzers #13

Closed
belav opened this issue Feb 27, 2021 · 8 comments
Closed

Formatting may conflict with StyleCopAnalayzers #13

belav opened this issue Feb 27, 2021 · 8 comments

Comments

@belav
Copy link
Owner

belav commented Feb 27, 2021

We need to investigate which style cop rules need to be disabled to avoid conflicts with csharpier

@belav belav added this to the Beta milestone Feb 27, 2021
@belav belav added the type:task label Mar 1, 2021
@belav belav modified the milestone: Beta Sep 18, 2021
@belav
Copy link
Owner Author

belav commented Sep 18, 2021

We should wait on this until after we decide if we are moving away from SpaceBrace - See #423

@belav
Copy link
Owner Author

belav commented Sep 20, 2021

Also .net 5 has moved to Microsoft.CodeAnalysis.NetAnalyzers, although the rule names may be the same.
See https://github.com/dotnet/roslyn-analyzers

@belav belav self-assigned this Oct 25, 2021
@belav
Copy link
Owner Author

belav commented Oct 25, 2021

This is the first pass of rules that conflict, some issues were discovered, there are open questions on some of the rules.
This was with StlyeCop.Analyzers 1.1.118

    <Rule Id="SA1009" Action="None" />
    <Rule Id="SA1111" Action="None" />
    <Rule Id="SA1118" Action="None" />
    <Rule Id="SA1501" Action="None" />
    <Rule Id="SA1502" Action="None" />
    <Rule Id="SA1504" Action="None" />
    <Rule Id="SA1137" Action="None" />
    <Rule Id="SA1500" Action="None" />

    <!-- wants spaces between multiline properties accessors, which does not seem to be a standard. #285 should be implemented though -->
    <Rule Id="SA1516" Action="None" />

   <!-- see #526 -->
    <Rule Id="SA1128" Action="None" />
   
   <!-- see #527 -->
   <Rule Id="SA1127" Action="None" />

<!-- probably leave, but add note -->
    <!-- forces new line before a comment on a catch of a try/catch
the comment can be moved to the brace above
-->
    <Rule Id="SA1513" Action="Warning" />

    <!-- conflicts with new(), See https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3214 will be fixed in 1.2 --> 
    <Rule Id="SA1000" Action="Warning" />

@belav belav modified the milestones: Beta, 0.11.0 Oct 25, 2021
@belav
Copy link
Owner Author

belav commented Oct 25, 2021

SA1500 conflicts with

do
{
    // something
} while (true);

~60% of do statements in https://github.com/belav/csharpier-repos use that syntax
Official doc uses that syntax
Some discussion about allowing it in stylecop
We will keep that syntax

@belav belav modified the milestones: 0.11.0, 0.12.0 Nov 15, 2021
@lara-ec
Copy link

lara-ec commented Nov 22, 2021

Hi,
SA1516 (ElementsMustBeSeparatedByBlankLine) conflicts with csharpier, when you have a get/set that does a line break. stylecop wants a newline between get and set, csharpier removes the newline.

private string a;

public string A
{
    get
    {
        return a + "this is a very long string that is this long so it can force a line break";
    }
    set { a = value; }
}

@belav belav modified the milestones: 0.12.0, 0.13.0 Nov 30, 2021
@belav
Copy link
Owner Author

belav commented Dec 20, 2021

@lara-ec I am adding SA1516 to the list of rules that should be disabled. I am not sure if your intention when reporting it was that csharpier should add the extra line before the set.

When I looking into multiline accessors for properties (using https://github.com/belav/csharpier-repos), only 16% of them included a line between them if one of the two spanned multiple lines. I think it makes sense to have csharpier stick to that and remove the empty line.

SA1516 is also used to ensure there is a new line between method definitions, which csharpier doesn't currently have an opinion on. See #285. I think it makes sense for csharpier to starting adding the blank lines between methods. 87% of the methods in classes from csharpier-repos have a blank line before them.

@lara-ec
Copy link

lara-ec commented Dec 21, 2021

@belav Yes, my intention was to have it on the list. :)

I don't have a strong opinion on it, but FWIW I agree with what you say.

@belav belav mentioned this issue Dec 21, 2021
7 tasks
@belav
Copy link
Owner Author

belav commented Dec 21, 2021

When 0.13.0 is released, I will update the docs with which stylecop rules conflict with csharpier.

@belav belav closed this as completed Dec 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants