When contributing to the project, and if otherwise not mentioned in this document, our coding conventions follow the Microsoft C# Coding Conventions and standard Naming Guidelines.
Members and types should always have the lowest possible visibility.
Ordering of class members should be the following:
- Constants
- Nested enum declarations
- Fields
- Abstract members
- Properties
- Constructors
- Methods
- Nested types
Furthermore, each of these categories should be ordered from higher to lower accessibility level (public, internal, protected, private).
Static fields and properties should be placed before instance ones.
Static methods are preferred to be after instance methods.
Once grouped as specified above, methods which are called by other methods in the same group should be placed below the callers.
public int PublicMethod() => 42;
int CallerOne() => Leaf();
int CallerTwo() => Leaf() + PublicMethod();
int Leaf() => 42;
There are no strict rules on when to use local functions. It should be decided on a case-by-case basis.
By default, you should prefer methods over local functions. Use local functions if it makes the code significantly easier to understand. For example:
- Accessing the method's local state directly, instead of using parameters, reduces noise.
- The name of the function would not make sense at the class level.
Local functions should always be placed at the end of a method.
public int MethodWithLocalFunction(int x)
{
return LocalFunction(x);
int LocalFunction(int x) => x;
}
Individual members must be separated by empty line, except sequence of constants, fields, single-line properties and abstract members. These members should not be separated by empty lines.
private const int ValueA = 42;
private const int ValueB = 24;
private int valueA;
private int valueB;
protected abstract int AbstractA { get; }
protected abstract void AbstractB();
public SemanticModel Model { get; }
public SyntaxNode Node { get; }
public int ComplexProperty
{
get => 42;
set
{
// ...
}
}
public Constructor() { }
public void MethodA() =>
MethodB();
public void MethodB()
{
// ...
}
Generic words in class names that don't convey meaning (e.g. Helper
) should be avoided. Overwordy and complex names should be avoided as well.
Single variable lambdas should use x
as the variable name (based on lambda calculus λx). Multi variable lambdas should use descriptive names, where x
can be used for the main iterated item like (x, index) => ...
. Name c
can be used for context of Roslyn callback.
Short names can be used as parameter and variable names, namely SyntaxTree tree
, SemanticModel model
, SyntaxNode node
and CancellationToken cancel
.
Unit tests for common C# and VB.NET rules should use two aliases using CS = SonarAnalyzer.Rules.CSharp
and using VB = SonarAnalyzer.Rules.VisualBasic
. Test method names should have _CS
and _VB
suffixes.
Unit tests for single language rule should not use alias nor language method suffix.
Variable name sut
(System Under Test) is recommended in unit tests that really tests a single unit (contrary to our usual rule integration unit tests).
- Operators (
&&
,||
,and
,or
,+
,:
,?
,??
and others) are placed at the beginning of a line.- Indented at the same level if the syntax at the beginning of the previous line is a sibling.
void Foo() => A && B; // A and B are siblings => we don't indent
- Indented one level further otherwise.
return A && B; // "return" is the parent of A and B => we indent
- Indented at the same level if the syntax at the beginning of the previous line is a sibling.
- Dot before an invocation
.Method()
is placed at the beginning of a line. - The comma separating arguments is placed at the end of a line.
- Method declaration parameters should be on the same line. If S103 is violated, parameters should be placed each on a separate line; the first parameter should be on the same line with the declaration; the other parameters should be aligned with the first parameter.
public void MethodWithManyParameters(int firstParameter, string secondParameter, Function<int, string, string> complexParameter);
- Long ternary operator statements should have
?
and:
on separate lines, aligned with a left-most single indendation.object.Property is SomeType something && something.AnotherProperty is OtherType other && other.Value == 42 ? object.Parent.Value : object;
- Chained invocations and member accesses violating S103 can have a chain of properties on the first line. Every other
.Invocation()
or.Member
should be on a separate line, aligned with a left-most single indendation.object.Property.Children .Select(x => x.Something) .Where(x => x != null) .OrderBy(x => x.Rank) .ToArray() .Length;
- Exception from this rule: Chains of assertions can have supporting properties,
.Should()
and assertion on the same line.values.Should().HaveCount(2) .And.ContainSingle(x => x.HasConstraint(BoolConstraint.True)) .And.ContainSingle(x => x.HasConstraint(BoolConstraint.False));
- Exception from this rule: Chains of assertions can have supporting properties,
- Method invocation arguments should be placed on the same line only when they are few and simple. Otherwise, they should be placed on separate lines. The first argument should be on a separate line, aligned with a left-most single indendation.
object.MethodName( firstArgument, x => x.Bar(), thirdArgument.Property);
- Exception from this rule: chained LINQ queries where the alignment of parameter expressions should be right-most.
someEnumerable.Where(x => x.Condition1 && x.Condition2);
- Exception from this rule: Lambda parameter name and arrow token should be on the same line as the invocation.
context.RegisterSyntaxNodeAction(c => { // Action }
- Exception from this rule: chained LINQ queries where the alignment of parameter expressions should be right-most.
- When using an arrow property or an arrow method, the
=>
token must be on the same line as the declaration. Regarding the expression body:- for properties: it should be on the same line as the property declaration. It should be on the following line only when it is too long and would trigger S103.
- for methods: it should be on the same line only for trivial cases: literal or simple identifier. Member access, indexer, invocation, and other complex structures should be on the following line.
- Field and property initializations are done directly in the member declaration instead of in a constructor.
if
/else if
and explicitelse
is used- when it helps to understand overall structure of the method,
- especially when each branch ends with a
return
statement.
- Explicit
else
is not used after input validation. - For multiple conditions before the core method logic:
- chain conditions in the same
if
statement together with positive logic for best readability (i.e.if (first && second) { DoSomething(); }
) - when chained conditions cannot be used, use early returns
- otherwise, use nested conditions
- chain conditions in the same
- Use positive logic.
- Use
is {}
andis not null
as null-checks (instead of!= null
). - Var pattern
is var o
can be used only where variable declarations would require additional nesting. - Var pattern
o is { P: var p }
can be used only whereo
can benull
andp
is used at least 3 times. - Do not use
nullable
.
- Code should contain as few comments as necessary in favor of well-named members and variables.
- Comments should generally be on separate lines.
- Comments on the same line with code are acceptable for short lines of code and short comments.
- Documentation comments for abstract methods and their implementations should be placed only on the abstract method, to avoid duplication. When reading the implementation, the IDE offers the tooling to peek in the base class and read the method comment.
-
FIXME
should only be used in the code during development as a temporary reminder that there is still work to be done here. AFIXME
should never appear in a PR or be merged into master. -
ToDo
can be used to mark part of the code that will need to be updated at a later time. It can be used to track updates that should be done at some point, but that either cannot be done at that moment, or can be fixed later. Ideally, aToDo
comment should be followed by an issue number (what needs to be done should be in the github issues).
Generally, as we do not want to have classes that are too long, regions are not necessary and should be avoided.
It can still be used when and where it makes sense. For instance, when a class having a specific purpose is
implementing generic interfaces (such as IComparable
, IDisposable
), it can make sense to have regions
for the implementation of these interfaces.
Do not use ValueTuples
in production code. The usage in test projects is fine. ValueTuples
are not supported in MsBuild 14 and while MsBuild 14 is not officially supported anymore, we still don't want to break it, if we can avoid it.
Empty lines should be used between blocks, Namespace
/End Namespace
statements, Class
/End Class
statements
and regions to improve readability.