Neolution Code Analysis rulesets

We maintain two general rulesets for all our projects and distribute them including the Analyzers (StyleCop, FxCop and SonarSource) in two respective NuGet packages.

• For Libraries, Tools, Web, UI and other projects
  • Neolution.CodeAnalysis
• For projects with automated testing
  • Neolution.CodeAnalysis.TestsRuleset

General decisions

When compiling a project with one of the packages installed, expect them to behave differently in different build configurations.

The following applies in general:

• In Release configuration (default configuration for build pipelines) all the rules are set to Error. The build will fail if at least one Analyzer raises a Warning or an Error.
• In Debug configuration all the rules are set to None. This will allow for much faster compilation time, but the build will never fail due to an Analyzer rule. Also, raised Warning's will never fail the build, only Error's do.

However, there are a few exceptions to the statements above.

• Rules that match the ID *WithoutSuggestion must always be set to None. They are redundant to the rule number without the "WithoutSuggestion" suffix.
• Rules that match the ID IDE* must never raise Error. Change to Warning instead. They are never executed on build servers.
• Rules that match the ID IDE* must never be set to Hidden. Change it to Information or track the change in this document if you want to disable it.

Versioning

Given a version number MAJOR.MINOR.PATCH, an incremented number for:

• MAJOR version means there are incompatible changes with the previous version,
• MINOR version means functionality and rules were added, and
• PATCH version means that the changes are all completely backwards compatible. Usually when rules were removed or lowered in severity.

What this means in practice

1. You can (and should) always update the package to the latest PATCH version whenever you have the chance to do it. Applying a PATCH update should never break the build.
2. Updating to the latest MINOR version can break the build. But you can expect to have Roslyn code fixes available for the changes that are needed to fix it again. Thus it requires minor efforts, relative to the projects size.
3. For MAJOR updates, detailed migration paths will be provided.

Exceptions for Release configuration

As stated in General decisions, in Release configuration, all rules will throw Error. With the following exceptions:

Rule ID	Action	Description/Title	Reason/Remarks
S4055 CA1303	None	Literals should not be passed as localized parameters	Localization is not needed in every project and for every message. This rule disallows string literals everywhere e.g. for log messages, content that does not need to be translated etc.
S1451 SA1633	None	Each source file should start with a header stating file ownership and the license which must be used to distribute the application	Header is not needed for each file. If the source code of a project will be distributed, the text can be added to each file individually at a later point in time to satisfy this rule.
SX1309 SX1309S	None	A field name in C# does not begin with an underscore.	Field names must not begin with an underscore. The `this.` qualifier is used in favor of the underscore
SX1101 IDE0003	None	Do not prefix local calls with `this.`.	Always prefix local calls with "this.". The "this." qualifier is used in favor of the underscore.
IDE0008	None	Use explicit type	Do not force explicit type usage. Prefer using the implicit type (var) but you can use the explicit type to improve reading flow and comprehension.
S1309	Info	Track uses of in-source issue suppressions	Suppressions should be allowed, but still tracked.
CA1014 S3992 S3990	None	Mark assemblies with CLSCompliant	It is not needed to mark assemblies with CLSCompliant. We actively use libraries that are not compliant. If we mark our assemblies as CLS compliant we cannot use these anymore.
S2221 CA1031	None	"Exception" should not be caught when not required by called methods	This rule probably originates from Java where you have to declare the Exception that will be throwed by a method and will subsequently being checked by the compiler.
CA2210	None	Assemblies should have valid strong names	Strong names are usually not needed for our typical deployments.
CA1020	None	Avoid namespaces with few types	It happens often that a namespace contains only a few classes, but is extended later during development. Merging it with some parent namespace at first and extracting to separate namespace later (when there are enough classes), is just a pain and unnecessary waste of time.
CA1024	None	Use properties where appropriate	Developers can make a correct decision when the property is a better option than a method.
CA1054 CA1056 S3996 S3994	None	URI properties or parameters should not be strings	The team decided to drop this rule due to inconveniences resulting from System.Uri not having a parameterless constructor.
CA2234	None	Pass System.Uri objects instead of strings	Dropped in favor of S4005
S3235	None	Redundant parentheses should not be used	The team decided to drop this rule because most of the team members are used to write redundant parentheses.
CA2227 S4004	None	Collection properties should be read only	The team decided to drop this rule due to the additional effort that is needed to write methods that add items for these type of properties.
IDE0045 IDE0046	None	Convert to conditional expression	Conflicts with rule S3358.
CA1506	None	Avoid excessive class coupling	Even if this rule might sound useful, in practice it was generating too many false-positives. Also, EF LINQ Queries are very prone to this. Unit and Integration tests must setup and configure numerous dependencies.
S4018	None	Generic methods should provide type parameters	Using the generic argument as return type does not lead to bugs. Only when using the method, you need to write a little more explicit code.
CA1822	None	Mark members as static	Dropped in favor of S2325.
SA1609	None	PropertyDocumentationMustHaveValue	The documentation of the value is doomed to become a hollow echo of the name and the summary.
SA1133	None	DoNotCombineAttributes	The team voted to deactivate this rule on 28.02.2019.
CA1308 S4040	None	Normalize strings to uppercase	Too many false positives for paths, URLs etc.
S1135	Info	Track uses of "TODO" tags	Still show it to the developer in his IDE.
CS1591	None	Missing XML comment for publicly visible type or member 'Type_or_Member'	Can be deactivated because SA1600 already checks for the XML comment.
IDE0058	None	C# Expression value is never used	Is reporting way too many false negatives.
S1541 CA1502	None	Methods and properties should not be too complex	Deactivated in favor of the more thoughtful rule S3776 (Cognitive Complexity of methods should not be too high).
SA1629	None	Documentation text should end with a period	Does not add too much quality and developers find it too annoying to comply with this.
IDE0065	None	Misplaced using directive	Placing using directives outside of the namespace conflicts with our code style.
CA1001	None	Types that own disposable fields should be disposable	Dropped in favor of S2931.
CA1307 S4058	None	Overloads with a "StringComparison" parameter should be used.	Generated too many false positives for EF queries.
CA2201	None	Do not raise reserved exception types	Deactivated in favor of S112
IDE0001	None	Name can be simplified	Can lead to conflicting scenarios in the using directives with SA1135.
S103	None	Lines should not be too long	Developers should be in charge to decide when the line is too long. Horizontal scrolling is not a big issue anymore with todays display sizes and resolutions.
S109	Info	Magic numbers should not be used	Developers are advised to add a comment to demystify the meaning of such a number.
S2228	None	Magic numbers should not be used	This rule has been superseded by S106. (Specification)[https://jira.sonarsource.com/browse/RSPEC-2228]
SA1005	None	Single line comments should begin with single space	Dropped in favor of S125.
CS8019 IDE0005	None	Unnecessary using directive	Dropped in favor of S1128.
CA1062	None	Validate arguments of public methods	Dropped in favor of S3900.
CA1305	None	Specify IFormatProvider	Dropped in favor of S4056.
IDE0052	None	Unread "private" fields should be removed	Dropped in favor of S4487
S1301	None	"switch" statements should have at least 3 "case" clauses	Removed based on team vote. They prefer switch even if it has only one case clause.
S4142	None	Duplicate values should not be passed as arguments	This rule is marked as deprecated by SonarSource, and will eventually be removed.
S1227	None	break statements should not be used except for switch cases	We enforce multiple rules to force developers to write simple functions. In simple functions, break statements should not be too problematic concerning readability.
S1075	Info	URIs should not be hardcoded	The suggestion is helpful, but should not be enforced in all cases.
CA2200	None	Rethrow to preserve stack details	Dropped in favor of S3445.
CA1002	None	Do not expose generic lists	Dropped in favor of S3956

Exceptions for Debug configuration

As stated in General decisions, in Debug configuration, all rules are set to None. Exceptions defined for Release configurations also apply here. Additionally, in order to support early refactoring of your code, the follwoing rules will be always be executed in this configuration:

Rule ID	Action	Description/Title
S107	Warning	Methods should not have too many parameters
S134	Warning	Control flow statements "if", "switch", "for", "foreach", "while", "do" and "try" should not be nested too deeply
S138	Warning	Functions should not have too many lines of code
S1067	Warning	Expressions should not be too complex
S1151	Warning	"switch case" clauses should not have too many lines of code
S1200	Warning	Classes should not be coupled to too many other classes (Single Responsibility Principle)
S1244	Warning	Floating point numbers should not be tested for equality
S2360	Warning	Optional parameters should not be used
S3776	Warning	Cognitive Complexity of methods should not be too high
S4017	Warning	Method signatures should not contain nested generic types
S4049	Warning	Properties should be preferred
CA5351	Warning	Do Not Use Broken Cryptographic Algorithms
CA5386	Warning	Avoid hardcoding SecurityProtocolType value
CA5394	Warning	Do not use insecure randomness
SA1200	Error	Using directives should be placed correctly
CS8602	Warning	Dereference of a possibly null reference.
CS8603	Warning	Possible null reference return.
CS8604	Warning	Possible null reference argument.
CS8605	Warning	Unboxing a possibly null value.
CS8607	Warning	A possible null value may not be used for a type marked with [NotNull] or [DisallowNull]
CS8608	Warning	Nullability of reference types in type doesn’t match overridden member.
CS8609	Warning	Nullability of reference types in return type doesn’t match overridden member.
CS8610	Warning	Nullability of reference types in type of parameter doesn’t match overridden member.
CS8611	Warning	Nullability of reference types in type of parameter doesn’t match partial method declaration.
CS8612	Warning	Nullability of reference types in type doesn’t match implicitly implemented member.
CS8613	Warning	Nullability of reference types in return type doesn’t match implicitly implemented member.
CS8614	Warning	Nullability of reference types in type of parameter doesn’t match implicitly implemented member.
CS8615	Warning	Nullability of reference types in type doesn’t match implemented member.
CS8616	Warning	Nullability of reference types in return type doesn’t match implemented member.
CS8617	Warning	Nullability of reference types in type of parameter doesn’t match implemented member.
CS8618	Warning	Non-nullable field is uninitialized. Consider declaring as nullable.
CS8619	Warning	Nullability of reference types in value doesn’t match target type.
CS8620	Warning	Argument cannot be used for parameter due to differences in the nullability of reference types.
CS8621	Warning	Nullability of reference types in return type doesn’t match the target delegate.
CS8622	Warning	Nullability of reference types in type of parameter doesn’t match the target delegate.
CS8624	Warning	Argument cannot be used as an output for parameter due to differences in the nullability of reference types.
CS8625	Warning	Cannot convert null literal to non-nullable reference type.
CS8629	Warning	Nullable value type may be null.
CS8631	Warning	The type cannot be used as type parameter in the generic type or method. Nullability of type argument doesn’t match constraint type.
CS8632	Warning	The annotation for nullable reference types should only be used in code within a ‘#nullable’ annotations context.
CS8633	Warning	Nullability in constraints for type parameter doesn’t match the constraints for type parameter in implicitly implemented interface method'.
CS8634	Warning	The type cannot be used as type parameter in the generic type or method. Nullability of type argument doesn’t match ‘class’ constraint.
CS8643	Warning	Nullability of reference types in explicit interface specifier doesn’t match interface implemented by the type.
CS8644	Warning	Type does not implement interface member. Nullability of reference types in interface implemented by the base type doesn’t match.
CS8645	Warning	Interface is already listed in the interface list with different nullability of reference types.
CS8655	Warning	The switch expression does not handle some null inputs.
CS8656	Warning	Call to non-readonly member.
CS8667	Warning	Partial method declarations have inconsistent nullability in constraints for type parameter.
CS8714	Warning	The type cannot be used as type parameter in the generic type or method. Nullability of type argument doesn’t match ‘notnull’ constraint.

Tests Ruleset

Tests are used mostly internally and allow for much laxer rules. The *.TestsRuleset package includes a modified ruleset for both Debug and Release. The following rules are different to their respective configuration.

Rule ID	Action	Description/Title	Reason/Remarks.
CA1707	None	Identifiers should not contain underscores	We use underscores in test method names to make them easier to read.
S112	None	Do not raise reserved exception types	In Tests we should be able to raise any Exception we like.
CA1031	None	Do not catch general exception types	Since we can raise any Exception we like In Tests we should also be able to catch any Exception we like.
CA2000	None	Dispose objects before losing scope	Tests are never run continuously in a production environment. Resources are freed as soon as the Test run ends, so there is no need to worry about the pitfalls of not freeing resources.
S131	None	"switch/Select" statements should contain a "default/Case Else" clauses	There is no need for defensive programming in Tests
S126	None	"if ... else if" constructs should end with "else" clauses	There is no need for defensive programming in Tests
CA2007	None	Do not directly await a Task	This warning is intended for libraries, where the code may be executed in arbitrary environments and where code shouldn't make assumptions about the environment or how the caller of the method may be invoking or waiting on it