Differences with classic StyleCop

[GregReddick]

I have a test suite for classic StyleCop (and Code Analysis) that I created for trying to trigger each of the warnings. I just ran it on StyleCop Analyzers. There are some differences. These should probably be triaged and bugs created in cases where it is warranted. In some cases, I think they should just be documented.

Issue D1: Related to SA1642.

If there is a class with a private empty constructor, the documentation text in classic StyleCop requires it to begin with "Prevents a default instance of the class from being constructed." The current rule requires "Initializes a new instance...".

    public class RuleCA1052Fail
    {
        private RuleCA1052Fail()
        {
        }
    }

Issue D2: Related to SA1402.

In classic StyleCop, a suppression of SA1402 only needs to occur on the first class within a file and all other classes in the file do not produce SA1402. The current analyzer requires that every class in the file has the suppression.

Issue D3: Related to SA1000 and SA1008

The following code would only trigger SA1000 in classic StyleCop. The current analyzer triggers both SA1000 and SA1008:

for(int i = 1; i < 10; i++)
{
}

Issue D4: Related to SA1003 and SA1020

The following code would only trigger SA1020 in classic StyleCop. The current analyzer triggers both SA1020 and SA1003.

int value = 1;
value ++;

Issue D5: Related to SA1003 and SA1023

The following code would only trigger SA1023 in classic StyleCop. The current analyzer triggers both SA1023 and SA1003. (This is unsafe code.)

int* value = null;
int blah = * value;

Issue D6: Related to SA1106 and SA1002

The following code would only trigger SA1106 in classic StyleCop. The current analyzer triggers both SA1106 and SA1002.

Console.WriteLine();;

Issue D7: Related to SA1109 and SA1123

The following code would trigger SA1109 in classic StyleCop. The current analyzer triggers SA1123.

        [SuppressMessage("StyleCop.CSharp.ReadabilityRules", "SA1124:DoNotUseRegions", Justification = "For example")]
        public static void SomeMethod()
        {
            if (true)
            #region Region
            {
            }
            #endregion
        }

Issue D8: Related to SA1101 and SA1126

The following code would trigger SA1126 in classic StyleCop. The current analyzer triggers SA1101.

namespace RuleViolations
{
    using System;
    using System.Diagnostics.CodeAnalysis;

    /// <summary>A rule sa 1126 fail.</summary>
    public class RuleSA1126Fail
    {
        /// <summary>Some method.</summary>
        public void SomeMethod()
        {
            Console.WriteLine(this);
        }
    }

    /// <summary>A rule sa 1126 fail derived.</summary>
    /// <seealso cref="T:RuleViolations.RuleSA1126Fail"/>
    [SuppressMessage(
        "Microsoft.StyleCop.CSharp.MaintainabilityRules",
        "SA1402:FileMayOnlyContainASingleClass",
        Justification = "For example")]
    public class RuleSA1126FailDerived : RuleSA1126Fail
    {
        /// <summary>Some other method.</summary>
        public void SomeOtherMethod()
        {
            SomeMethod();
        }
    }
}

Issue D9: Related to SA1202 and SA1205

The following code would trigger only SA1205 in classic StyleCop. The current analyzer triggers both SA1202 and SA1205.

namespace RuleViolations
{
    using System;
    using System.Diagnostics.CodeAnalysis;

    /// <summary>A rule sa 1205 fail.</summary>
    public class RuleSA1205Fail
    {
        /// <summary>Some method.</summary>
        public void SomeMethod()
        {
            Part1 part1 = new Part1();
            Console.WriteLine(this);
            Console.WriteLine(part1);
        }

        /// <summary>A part 1 class.</summary>
        [SuppressMessage("StyleCop.CSharp.MaintainabilityRules", "SA1400:AccessModifierMustBeDeclared", Justification = "For example")]
        partial class Part1
        {
        }

        /// <summary>A part 2 class.</summary>
        [SuppressMessage("Microsoft.Design", "CA1034:NestedTypesShouldNotBeVisible", Justification = "For example")]
        public class Part2
        {
        }
    }
}

Issue D10: Related to SA1304 and SA1401

The following code would trigger SA1304 in classic StyleCop. The current analyzer triggers SA1401.

namespace RuleViolations
{
    using System.Diagnostics.CodeAnalysis;

    /// <summary>A rule sa 1304 fail.</summary>
    public class RuleSA1304Fail
    {
        /// <summary>The value.</summary>
        [SuppressMessage("Microsoft.Design", "CA1051:DoNotDeclareVisibleInstanceFields", Justification = "For example")]
        [SuppressMessage("StyleCopPlus.StyleCopPlusRules", "SP0100:AdvancedNamingRules", Justification = "For example")]
        protected readonly int value = 1;
    }
}

Issue D11: Related to SA1307 and SA1311

The following code would trigger SA1311 in classic StyleCop. In the current analyzer, it triggers both SA1307 and SA1311.

namespace RuleViolations
{
    using System.Diagnostics.CodeAnalysis;

    /// <summary>A rule sa 1311 fail.</summary>
    public static class RuleSA1311Fail
    {
        /// <summary>The value.</summary>
        [SuppressMessage(
            "Microsoft.Naming",
            "CA1709:IdentifiersShouldBeCasedCorrectly",
            MessageId = "value",
            Justification = "For example")]
        [SuppressMessage("StyleCopPlus.StyleCopPlusRules", "SP0100:AdvancedNamingRules", Justification = "For example")]
        public static readonly int value = 1;
    }
}

Issue D12: Related to SA1130 and SA1410

The following code would trigger only SA1410 in classic StyleCop. In the current analyzer, it triggers both SA1410 and SA1130.

namespace RuleViolations
{
    using System;

    /// <summary>A rule sa 1410 fail.</summary>
    public class RuleSA1410Fail
    {
        /// <summary>Some method.</summary>
        public void SomeMethod()
        {
            this.SomeOtherMethod(delegate()
            {
                return 1;
            });
        }

        /// <summary>Some other method.</summary>
        /// <param name="value">The value.</param>
        public void SomeOtherMethod(Func<int> value)
        {
            Console.WriteLine(this);
            if (value != null)
            {
                Console.WriteLine(value());
            }
        }
    }
}

Issue D14: Related to SA1517 and SA1633

A blank line before the file header would only trigger SA1517 in classic StyleCop. In the current analyzer, it triggers both SA1517 and SA1633.

Issue D15: Related to SA1591 and SA1601

The following code would trigger SA1601 in classic StyleCop. In the current analyzer, it triggers both SA1591 and SA1601.

namespace RuleViolations
{
    public partial class RuleSA1601Fail
    {
    }
}

Issue D16: Related to SA1591 and SA 1602

The following code would trigger SA1602 in classic StyleCop. In the current analyzer, it triggers both SA1591 and SA1602.

namespace RuleViolations
{
    /// <summary>A rule sa 1602 fail.</summary>
    public enum RuleSA1602Fail
    {
        None
    }
}

Issue D17: Related to SA1570 and SA1603

The following code would trigger SA1603 in classic StyleCop. In the current analyzer, it triggers SA1570 only.

namespace RuleViolations
{
    /// <summary>A rule sa 1603 fail.
    public class RuleSA1603Fail
    {
    }
}

Issue D18: Related to SA1611 and SA1612

The following code would trigger SA1612 in classic StyleCop. In the current analyzer, it triggers SA1611.

namespace RuleViolations
{
    using System;

    /// <summary>A rule sa 1612 fail.</summary>
    public static class RuleSA1612Fail
    {
        /// <summary>Some method.</summary>
        /// <param name="someOtherValue">The value.</param>
        public static void SomeMethod(int value)
        {
            Console.WriteLine(value);
        }
    }
}

There are others, but probably if these get fixed, it will fix many of the others. I can provide my test suite if you want.

[sharwell]

Thanks for taking the time to do this. 👍

Issue D1: Related to SA1642.

The behavior is by-design, see #413. The documentation does not accurately reflect this, so a documentation bug should be created.

Issue D2: Related to SA1402.

Suppressions are completely handled by Roslyn, not by our code. You can disable the warning throughout a file by using the following syntax:

#pragma warning suppress SA1402 // File may only contain a single class

Issue D7: Related to SA1109 and SA1123

This is intentional (see #998).

Issue D8: Related to SA1101 and SA1126

This is a side effect of an intentional change (see #59, #997).

Issue D12: Related to SA1130 and SA1410

SA1130 is a new rule for StyleCop Analyzers. I think it's fine to report both and let the user decide what the best course of action is. If you disagree, feel free to open an issue for a proposed change.

Issue D15: Related to SA1591 and SA1601
Issue D16: Related to SA1591 and SA 1602

There is no SA1591; I believe you are referring to CS1591. The difference here is StyleCop Analyzers runs with the compiler, so any compiler warnings are produced alongside the StyleCop results, as opposed to StyleCop Classic which ran separately and only reported StyleCop results. See the documentation of SA1652 for additional information about CS1591.

Issue D17: Related to SA1570 and SA1603

There is no SA1570; I believe you are referring to CS1570. This behavior is intentional; see #1291.

Issue D3: Related to SA1000 and SA1008
Issue D4: Related to SA1003 and SA1020
Issue D5: Related to SA1003 and SA1023
Issue D6: Related to SA1106 and SA1002
Issue D9: Related to SA1202 and SA1205
Issue D10: Related to SA1304 and SA1401
Issue D11: Related to SA1307 and SA1311
Issue D14: Related to SA1517 and SA1633
Issue D18: Related to SA1611 and SA1612

Each of these should be moved to a new bug (one per issue) for further investigation and potentially fixes.

[sharwell]

I'll reopen this with the action item of filing issue reports for the items mentioned at the end of my comment.

[GregReddick]

Do you want me to created the related issues?

[sharwell]

You get "dibs" on them since you spotted them - I'll let you take credit for all those issues. If you don't want to let me know and I'll file them tonight.

[GregReddick]

I'll take care of it later today. I'll try to strip them down a bit, too, as I just copied and pasted from my test suite.

[GregReddick]

Sorry, had some internet connectivity issues with Comcast this weekend. Think they are all sorted out. Making the move to new issues now.

[GregReddick]

I think all the issues needed have been created. Closing this issue.

[sharwell]

👍