You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
SA1514 falsely reported when method comment immediately follows region directive. #1280
💭 I'd like to keep this by design. The region is a higher-level block indicator than a single method and it's much easier to identify when the additional lines are included.
The current implementation is incompatible with the old StyleCop. Maybe causing a lot of code changes.
Technically the old StyleCop doesn't allow regions. I don't anticipate this difference would require many changes for existing code, plus code fixes are provided so any necessary changes are trivial. The inclusion of blank lines around #region and #endregion is also the more common style I've seen in code that does use regions.
Adding additional lines causes too much unnecessary whitespace.
IMO your second code example is vastly superior to the first regarding spacing around the region markers. It is exactly what I would expect to see.
@sharwell I generally agree with your sentiments, but the current rules will allow this:
public class TestClass
{
#region TestRegion
#endregion
#region TestRegion
#endregion
#region TestRegion
/// <summary>
/// Test summary.
/// </summary>
public void TestMethod()
{
}
#endregion
}
With that in mind, this rule doesn't seem like the right place to enforce #region spacing, but I'm not sold on an exception either.
@EdwinEngelen Why are you still using regions in the year 2015? If your classes are so big that the standard code-collapse options aren't sufficient, you should break your classes into partial files or into separate classes altogether.
@otac0n We are still using regions because we have 10 year old code with large classes. I won't write it that way today indeed, but I can't ignore the legacy.
This is unlikely to ever be checked. The spacing around these directives is a side effect of other rules and not enforced on the directives themselves. The only rules which deal directly with these directives say to remove them. 😄
}
#endregion
This would need to be handled by SA1513, a change which I would support.
I would like to see also no blank lines after a #region and before an #endregion statement. Is it possible to have some extra rules like SX1514? The sample code from EdwinEngelen looks cluttered, so many empty lines. We use #region/#endregion here, for example around the usings and the codeing rules inhouse say no empty line after a #region and no blank line before an #endregion.
Is it possible to set the behaviour in the settings?
In our code, I generally use regions only to encompass the constructors in a class and on rare occasions, an internal private class. Old StyleCop rules may exclude regions, but that means that there is no finding for not having an extra line after the region. I can follow you thinking about regions and the extra line, however, I think there are many different reasons why someone, including myself, would prefer not to have the extra line. Like the post above, I'm asking if there's a way that an option could be provided?
Seems clear enough that style choice of #region can't succinctly be yes/no on having spaces. I too prefer not having extra space in my projects, though at work we use extra spaces. So it would seem it's a worthy addition. I don't know about stylecop.json's very minimal list of adjustable rules, but it would be nice to configure this rule there.
Our source code is comprised of several Gb in hundreds of thousands of .cs files (yes you read that right) and now that we are finally porting to Roslyn Analyzers this inconsistency with old SC is hitting pretty much every file.
So in my biased point of view, I believe consistency should be maintained, especially in cases like this where the correct behavior is up for discussion.
Meanwhile this should at least be listed on the Known Differences documentation.
Splitting every behavior change into separate rules is also a great solution as we can disable those.
Activity
sharwell commentedon Aug 25, 2015
📝 This is currently by design.
sharwell commentedon Aug 25, 2015
💭 I'd like to keep this by design. The region is a higher-level block indicator than a single method and it's much easier to identify when the additional lines are included.
EdwinEngelen commentedon Aug 25, 2015
sharwell commentedon Aug 25, 2015
Technically the old StyleCop doesn't allow regions. I don't anticipate this difference would require many changes for existing code, plus code fixes are provided so any necessary changes are trivial. The inclusion of blank lines around
#regionand#endregionis also the more common style I've seen in code that does use regions.IMO your second code example is vastly superior to the first regarding spacing around the region markers. It is exactly what I would expect to see.
otac0n commentedon Aug 25, 2015
@sharwell I generally agree with your sentiments, but the current rules will allow this:
With that in mind, this rule doesn't seem like the right place to enforce #region spacing, but I'm not sold on an exception either.
@EdwinEngelen Why are you still using regions in the year 2015? If your classes are so big that the standard code-collapse options aren't sufficient, you should break your classes into
partialfiles or into separate classes altogether.EdwinEngelen commentedon Aug 25, 2015
@otac0n We are still using regions because we have 10 year old code with large classes. I won't write it that way today indeed, but I can't ignore the legacy.
sharwell commentedon Aug 25, 2015
This is unlikely to ever be checked. The spacing around these directives is a side effect of other rules and not enforced on the directives themselves. The only rules which deal directly with these directives say to remove them. 😄
This would need to be handled by SA1513, a change which I would support.
mreu commentedon Aug 26, 2015
I would like to see also no blank lines after a #region and before an #endregion statement. Is it possible to have some extra rules like SX1514? The sample code from EdwinEngelen looks cluttered, so many empty lines. We use #region/#endregion here, for example around the usings and the codeing rules inhouse say no empty line after a #region and no blank line before an #endregion.
Is it possible to set the behaviour in the settings?
jaydinslage commentedon Sep 29, 2015
In our code, I generally use regions only to encompass the constructors in a class and on rare occasions, an internal private class. Old StyleCop rules may exclude regions, but that means that there is no finding for not having an extra line after the region. I can follow you thinking about regions and the extra line, however, I think there are many different reasons why someone, including myself, would prefer not to have the extra line. Like the post above, I'm asking if there's a way that an option could be provided?
appel1 commentedon Oct 6, 2015
We wrap WPF dependency properties in regions which I think increases readability and would prefer not having the extra new lines.
#region Foo property /// <summary> /// Identifies the <see cref="Foo"/> dependency property. /// </summary> public static readonly DependencyProperty FooProperty = DependencyProperty.Register("Foo", typeof(bool), typeof(Bar), new FrameworkPropertyMetadata(false)); /// <summary> /// Gets or sets a value indicating whether to foo. /// </summary> /// <value><see langword="true"/> if foo; otherwise, <see langword="false"/>. </value> public bool Foo { get { return (bool)GetValue(FooProperty); } set { SetValue(FooProperty, value); } } #endregionNinthDesertDude commentedon Sep 2, 2019
Seems clear enough that style choice of #region can't succinctly be yes/no on having spaces. I too prefer not having extra space in my projects, though at work we use extra spaces. So it would seem it's a worthy addition. I don't know about stylecop.json's very minimal list of adjustable rules, but it would be nice to configure this rule there.
Fabi0San commentedon Oct 17, 2019
Our source code is comprised of several Gb in hundreds of thousands of .cs files (yes you read that right) and now that we are finally porting to Roslyn Analyzers this inconsistency with old SC is hitting pretty much every file.
So in my biased point of view, I believe consistency should be maintained, especially in cases like this where the correct behavior is up for discussion.
Meanwhile this should at least be listed on the Known Differences documentation.
Splitting every behavior change into separate rules is also a great solution as we can disable those.
sharwell commentedon Oct 17, 2019
Yes, it should be listed in KnownChanges.md if it changed between StyleCop Classic and StyleCop Analyzers.
Document known behavior change in SA1514