ASP.NET エンジニアリングガイドライン

Engineering guidelines · aspnet/Home Wiki.

ASP.NETがOSS化され、一般からのコントリビュートも受け入れるようになりましたが、それに伴いコーディングや設計の為のガイドラインが公開されたようです。

それでは気になったところを見て行きます。私の感想的な事は引用で書きます。

Basics/Copyright header and license notice

テストかどうかを問わず*.csファイルにはコピーライトの表記が必要で、以下の内容書式で変更してはいけません。 *.designer.cs は除外してもかまいません。

// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

すべてのリポジトリはルートにApache 2.0ライセンスの内容が書かれた LICENSE.txtというファイルをルートディレクトリに置く必要があります。

著作権はMS Open Tech.が持ち、Apache 2.0ライセンスと言うことになります。

Basics/External dependencies

ASP.NETレポジトリ以外の依存物（NuGetパッケージなど)は以下の物に限定。それ以外のの物に依存する場合には@elionの許可が必要。

現在依存が許可されている物：

• Core CLR
• Roslyn
• Json.NET
• Re-linq
• Lx-Asinc

Basics/Code reviews and checkins

変更はプルリクにして、プルリクにはすべてのコードを含むこと。これは、ランタイム コード、単体テストの更新に対する変更や更新公式のサンプル （Music Store やBug Tracker） が含まれます。

一般的にプルリクはSubject Matter Expert (SME) によってコードがサインオフされる必要がある。

SME!すごくなじみが無いと思いますがSixSigmaの用語で問題領域専門家のことです。要は関係ないヤツにサインオフさせるな、何を変更したかわかるヤツにサインオフさせろと言うことです。こんな用語が出てくるあたりがMS。。。

プルリクのコミットはGitHubのでっかい緑ボタンでしちゃだだからな！

やりたい気持ちもわかるけどやっちゃダメ。

Basics/Branch strategy

総則：

• master NuGet.orgにある最終リリースのコード
• dev リリース前の変更中コード
• release 次リリースに向けてステージされ安定化をされているコード

releaseブランチはdevブランチからリリースに向けた安定化作業に入るときに分岐され、次期リリースに向けての作業はdevブランチで続けられる。リリースが行われるときに、releaseブランチからmasterブランチにpushされる。

普通だった。

Source code management/Solution and project folder structure and naming

いろいろ書かれているけど、VS 2015 での ASP.NET 5のソリューションのスタイル。

ソリューションのルートにソリューションファイル(.sln)とsrcディレクトリ、testディレクトリが配置され、srcディレクトリにはコードのkproject、testディレクトリにはテストコードのkprojectが配置される。

詳しくはページのサンプルを見てください。

Source code management/Assembly naming pattern

総則：

 Microsoft.AspNet.. 

例外：

• EntityFrameworkはASP.NETではないので除外
• ASP.NETではないたとえばDIのようなプロジェクトでは以下の命名規則とする。

 Microsoft.Framework..  

Source code management/Build system

KoreBuildと読んでいるsakeビルドツールを使用した、新しいビルドシステムを使用する。

sakeビルドツール： https://github.com/sakeproject/sake

Source code management/Unit tests

xUnitを使用する。

へー

Coding guidelines/Coding style guidelines – general

基本的にVS標準のままにしておけ。

1. タブは使わないインデントは4つのスペース
2. プライベートメンバーは_camelCase とアンスコ付きcamelCase
3. 絶対に必要で無い限りthis.は付けない
4. メンバーアクセス修飾子は省略しない

アンダースコア文化の復活

Coding guidelines/Usage of the var keyword

varキーワードはコンパイルが許す限り使用する。

当たり前だね。var禁止とか本当に何がしたいかよくわからないよね。コード例は元のページ見てください。

Coding guidelines/Use C# type keywords in favor of .NET type names

C#の型名が有る場合には、,NETの型名より優先して使用する。たとえば.NETのStringではなく、C#のstringを、.NetのIn32でなく、C#のintを使用する。

VBで書いてはいけないのか

Coding guidelines/When to use internals vs. public and when to use InternalsVisibleTo

現代的なフレームワークのセットでは、internalな型とメンバーの使用が許されます。しかしおすすめは出来ません。

InternalsVisibleTo 属性はinternalな型をユニットテストしたいときのみに使用する。我々は二つのランタイムアセンブリ間で InternalsVisibleTo 属性は使用しない。

二つのアセンブリが共通のヘルパークラスを利用したい場合に同じソースコードが分散してしまう。

二つのアセンブリが同じAPIを呼べるように、APIはpublicである必要がある。

InternalsVisibleTo 属性をそのような使い方で使っている例は見たことがないけど、やっている人たちがイルとしたら怖いわー。

Coding guidelines/Argument null checking

引数がnullでないことを確認するNullチェックは必要。(大きな驚き！)Null チェック追加するには、何れかの名前空間に次のコードを追加し(JetBrains.Annotations名前空間を使えばReSharperが仕事をしてくれる)、あなたのアセンブリで属性を宣言します。

using System;

namespace JetBrains.Annotations
{
    [AttributeUsage(
        AttributeTargets.Method | AttributeTargets.Parameter |
        AttributeTargets.Property | AttributeTargets.Delegate |
        AttributeTargets.Field, AllowMultiple = false, Inherited = true)]
    internal sealed class NotNullAttribute : Attribute
    {
    }
}

属性の宣言の仕方

public void GetBanana([NotNull] string variety)
{
    // do not do explicit null check in the method body!
    ...
}

public string Variety
{
    get;
    [param: NotNull]
    set;
}

これはぜひ日常的にも使っていきたい。

Coding guidelines/Argument null checking in interface member definitions and abstract/virtual methods

インターフェイスのメンバー、もしくはabstract/virtualなメンバーに対しては、引数のnullチェックをするように[NotNull]でアノテーションを付ける。それらを実装(オーバーライド)したメソッドでは、コードジェネレータが自動的に引き継ぐので明示的に[NotNull]でアノテーションを付ける必要性はありません。

Coding guidelines/Async method patterns

asyncメソッドにはAsyncサフィックスを付ける。

キャンセルトークンをdefault(CancellationToken)という値の省略可能な引数として渡し、この値は CancellationToken.Noneに相当します。 このWebシナリオでの主な例外はHttpContext周り渡されるが、この場合コンテキストは必要に応じて自身のキャンセルトークンを使います。

リソースを閉じる処理が必要な場合な場合ここで書かれているようにCancelTokenを引数として必ず取るようにしないとだめだね。

Coding guidelines/Extension method patterns

一般的なルール： 標準的なstaticなメソッドで十分なはず。拡張メソッドは避ける。

本当に拡張メソッドが必要か自問せよと。。

Coding guidelines/Doc comments

公開されたAPIには必要。公開されていない無い物には必要ない。

「公開された」なので、publicだけでなく、protectedも含まれるらしい。

Coding guidelines/Assertions

Debug.Assert()を使い、Code Contract(Contract.Assertなど)は使用しない。

Note:基本的にデバッグ時にだけ使用し、リリースするコードには含めないようにする。

Coding guidelines/Unit tests and functional tests/Assembly naming

テスト対象のアセンブリ名に.testのサフィックスを付けた物とする。

Coding guidelines/Unit tests and functional tests/Unit test class naming

テスト対象のクラス名の最後にTestを追加する。 Microsoft.Fruit.BananaクラスのテストクラスはMicrosoft.Fruit.BananaTestという名前になる。

Coding guidelines/Unit tests and functional tests/Unit test method naming

単体テスト メソッドの名前は何がテストされるについて、どのような条件と、期待の下でテストを行うか記述する必要があります。パスカルケースとアンダースコアの使用は読みやすさを向上させます。

以下は正しいメソッド名です。

PublicApiArgumentsShouldHaveNotNullAnnotation
Public_api_arguments_should_have_not_null_annotation

以下は正しくない。

Test1
Constructor
FormatString
GetData

Coding guidelines/Unit tests and functional tests/Unit test structure

Arrange, Act, Assertの順序で書き、コメントで分ける。

// Arrange
P1 p1 = GetComplexParam1();
P2 p2 = GetComplexParam2();
P3 p3 = GetComplexParam3();

// Act
int result = myObj.CallSomeMethod(p1, p2, p3);

// Assert
Assert.AreEqual(1234, result);

全体感想

あまり厳しくないので、ここから社内コーディング規約等考えてみるのも良いかもしれません。また、MS TestでなくxUnit、MS BuildではなくOSSなビルドシステムといったあたりが、OSS化された.NETの標準なのかなと言う気がしています。