[Tech] Coding Style Guidelines

Engineering guidelines part 1

Photo by Chris Ried on Unsplash

General

1. The most general guideline is that we use all the VS default settings in terms of code formatting
2. Use four spaces of indentation (no tabs)
3. Use _camelCase for private fields
4. Avoid this. unless absolutely necessary
5. Always specify member visibility, even if it’s the default
   (i.e. private string _foo; not string _foo;)
6. Open-braces ({) go on a new line
7. Use any language features available to you (expression-bodied members, throw expressions, tuples, etc.) as long as they make for readable, manageable code.

This is pretty bad:

public (int, string) GetData(string filter) => (Data.Status, Data.GetWithFilter(filter ?? throw new ArgumentNullException(nameof(filter))));

Usage of the var keyword

The var keyword is to be used as much as the compiler will allow. For example, these are correct:

var fruit = "Lychee";
var fruits = new List<Fruit>();
var flavor = fruit.GetFlavor();
string fruit = null; // can't use "var" because the type isn't known (though you could do (string)null, don't!)
const string expectedName = "name"; // can't use "var" with const

Use C# type keywords in favor of .NET type names

When using a type that has a C# keyword the keyword is used in favor of the .NET type name. For example, these are correct:

public string TrimString(string s)
{
    return string.IsNullOrEmpty(s) ? null : s.Trim();
}
var intTypeName = nameof(Int32); // can't use C# type keywords with nameof

Cross-platform coding

Our frameworks should work on CoreCLR, which supports multiple operating systems. Don’t assume we only run (and develop) on Windows. Code should be sensitive to the differences between OS’s. Here are some specifics to consider.

Line breaks

Windows uses \r\n, OS X and Linux uses \n. When it is important, use Environment.NewLine instead of hard-coding the line break.

NOTE — Be aware that these line-endings may cause problems in code when using @"" text blocks with line breaks.

Environment Variables

OS’s use different variable names to represent similar settings. Code should consider these differences.

For example, when looking for the user’s home directory, on Windows the variable is USERPROFILE but on most Linux systems it is HOME.

var homeDir = Environment.GetEnvironmentVariable("USERPROFILE") 
                  ?? Environment.GetEnvironmentVariable("HOME");

File path separators

Windows uses \ and OS X and Linux use / to separate directories. Instead of hard-coding either type of slash, use Path.Combine() or Path.DirectorySeparatorChar.

NOTE — If this is not possible (such as in scripting), use a forward slash. Windows is more forgiving than Linux in this regard.

When to use internals vs. public and when to use InternalsVisibleTo

• Use InternalsVisibleTo when sharing code between types in the same assembly, same feature area, or to unit test internal types and members.
• If two runtime assemblies need to share common helpers then we will use a “shared source” solution with build-time only packages.

NOTE — If two runtime assemblies need to call each other’s APIs, consider making the APIs public or if there’s enough extensibility for a customer to perform something similar. If we need it, it is likely that our customers need it.

Async method patterns

By default all async methods must have the Async suffix. There are some exceptional circumstances where a method name from a previous framework will be grandfathered in.

Passing cancellation tokens is done with an optional parameter with a value of default(CancellationToken), which is equivalent to CancellationToken.None

public Task GetDataAsync(
    QueryParams query,
    int maxData,
    CancellationToken cancellationToken = default(CancellationToken))
{
    ...
}

Extension method patterns

• If a regular static method would suffice, avoid extension methods.
• Internal extension methods are allowed
• When writing extension methods for an interface the sponsor type name must not start with an I.
• The class name of an extension method container (also known as a “sponsor type”) should generally follow the pattern of <Feature>Extensions, <Target><Feature>Extensions, or <Feature><Target>Extensions. For example:

namespace Food 
{
    class Fruit { ... }
}
namespace Fruit.Eating 
{
    class FruitExtensions 
    { 
        public static void Eat(this Fruit fruit); 
    }
  OR
    class FruitEatingExtensions 
    { 
        public static void Eat(this Fruit fruit); 
    }
  OR
    class EatingFruitExtensions 
    { 
        public static void Eat(this Fruit fruit); 
    }
}

Doc comments

The person writing the code will write the doc comments. Public APIs only. No need for doc comments on non-public types.

NOTE— Public means callable by a customer, so it includes protected APIs. However, some public APIs might still be “for internal use only” but need to be public for technical reasons. We will still have doc comments for these APIs but they will be documented as appropriate.

Breaking changes

In general, breaking changes can be made only in a new major product version, e.g. moving from 1.x.x to 2.0.0. Even still, we generally try to avoid breaking changes because they can incur large costs for anyone using these products. All breaking changes must be approved as part of the API review process.

For the normal case of breaking changes in major versions, this is the ideal process:

1. Provide some new alternative API (if necessary)
2. Mark the old type/member as [Obsolete] to alert users (see below), and to point them at the new alternative API (if applicable)

• If the old API really doesn’t/can’t work at all, please discuss with engineering team

1. Update the XML doc comments to indicate the type/member is obsolete, plus what the alternative is. This is typically exactly the same as the obsolete attribute message.
2. File a bug in the next major milestone (e.g. 2.0.0) to remove the type/member

• Mark this bug with a red [breaking-change] label (use exact casing, hyphenation, etc.). Create the label in the repo if it's not there.

Example of obsoleted API:

/// <summary>
/// <para>
/// This method/property/type is obsolete and will be removed in a 
/// future version.
/// The recommended alternative is 
/// Microsoft.SomethingCore.SomeType.SomeNewMethod.
/// </para>
/// <para>
/// ... old docs...
/// </para>
/// </summary>
[Obsolete("This method/property/type is obsolete and will be removed in a future version. The recommended alternative is Microsoft.SomethingCore.SomeType.SomeNewMethod.")]
public void SomeOldMethod(...)
{
 ...
}

Source code analysis

.NET Compiler Platform (Roslyn) Analyzers inspect your C# or Visual Basic code for style, quality, maintainability, design, and other issues. This inspection or analysis is done during design time in all open files.

If rule violations are found by an analyzer, they’re reported in the code editor (as a squiggle under the offending code) and in the Error List window.

The analyzer violations reported in the error list match the severity level setting of the rule. Analyzer violations also show up in the code editor as squiggles under the offending code. The following image shows three violations — one error (red squiggle), one warning (green squiggle), and one suggestion (three grey dots):

Many analyzer rules, or diagnostics, have one or more associated code fixes that you can apply to correct the rule violation. Code fixes are shown in the light bulb icon menu along with other types of Quick Actions. For information about these code fixes, see Common Quick Actions.

Summary

1. Use default settings in terms of code formatting
2. Use four spaces of indentation (no tabs)
3. Use _camelCase for private fields
4. Avoid this. unless absolutely necessary
5. Always specify member visibility
6. Open-braces ({) go on a new line
7. Use any language features available to you as long as they make for readable
8. Usage of the var keyword
9. Use C# type keywords in favor of .NET type names
10. Use C# type keywords in favor of .NET type names
11. Line breaks — Environment.NewLine
12. Environment Variables — Windows vs Linux, Android vs iOS
13. File path separators — Path.Combine() & Path.DirectorySeparatorChar
14. InternalsVisibleTo
15. Shared source
16. Async method patterns ???
17. Extension method patterns — <Feature>Extensions
18. The person writing the code will write the doc comments
19. Avoid breaking changes — [Obsolete] & point to alternative API & [breaking-change] label
20. Source code analysis

References

• https://github.com/dotnet/aspnetcore/wiki/Engineering-guidelines
• https://docs.microsoft.com/en-us/visualstudio/code-quality/roslyn-analyzers-overview?view=vs-2019