Skip to content

Fix SA1509 getting reported for consecutive blocks#702

Merged
sharwell merged 4 commits intoDotNetAnalyzers:masterfrom
sharwell:fix-689
Nov 26, 2015
Merged

Fix SA1509 getting reported for consecutive blocks#702
sharwell merged 4 commits intoDotNetAnalyzers:masterfrom
sharwell:fix-689

Conversation

@sharwell
Copy link
Copy Markdown
Member

Fixes #689

@sharwell sharwell added this to the 1.0.0 Alpha 5 milestone Apr 23, 2015
@vweijsters
Copy link
Copy Markdown
Contributor

What should happen with the code below? I think it will report SA1509, but I think it should not report any diagnostic.

class Foo
{
    void Bar()
    {
        /* test comment */

        {
        }

        {
        }
    }
}";

@Przemyslaw-W
Copy link
Copy Markdown

I still claim it should raise SA1509 for first nested block in both cases:

class Foo
{
    void Bar()
    {
        /* test comment */

        {
        }

        {
        }
    }
}";

and

class Foo
{
    void Bar()
    {
        // test comment

        {
        }

        {
        }
    }
}";

@Przemyslaw-W
Copy link
Copy Markdown

And also for:

class Foo
{
    void Bar()
    {
        #region Test region

        {
        }

        {
        }
        #endregion
    }
}";

@sharwell
Copy link
Copy Markdown
Member Author

@vweijsters The current code reports SA1509 in that case, and this pull request does not change that. Since that line is not between blocks, it's also outside the scope of this particular issue; you can file a new issue if you'd like to see the behavior in that case changed.

@Przemyslaw-W I added unit tests to verify that the behavior of this pull request adheres to your expectations.

@codecov-io
Copy link
Copy Markdown

Current coverage is 79.50%

Merging #702 into master will not affect coverage as of bd34329

@@            master    #702   diff @@
======================================
  Files          555     556     +1
  Stmts        33508   33734   +226
  Branches      9243    9320    +77
  Methods                          
======================================
+ Hit          26640   26821   +181
- Partial       5369    5424    +55
+ Missed        1499    1489    -10

Review entire Coverage Diff as of bd34329

Powered by Codecov. Updated on successful CI builds.

@pdelvo
Copy link
Copy Markdown
Member

pdelvo commented Nov 25, 2015

The docs should reflect the behavior.

At the same time, the implementation of the new behavior was greatly
simplified.
@oatkins
Copy link
Copy Markdown
Contributor

oatkins commented Nov 26, 2015

👍

@sharwell sharwell merged commit 81250fe into DotNetAnalyzers:master Nov 26, 2015
sharwell added a commit that referenced this pull request Nov 26, 2015
Fix SA1509 getting reported for consecutive blocks
@sharwell sharwell deleted the fix-689 branch November 26, 2015 02:20
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 this pull request may close these issues.

SA1509 (OpeningCurlyBracketsMustNotBePrecededByBlankLine) reported for consecutive blocks

6 participants