From fd12071509282907f8902b134485ac18a737d54a Mon Sep 17 00:00:00 2001 From: Michael L Smith Date: Sun, 20 Mar 2016 20:38:27 -0700 Subject: [PATCH] Added NDepend project, ignored ndepend output --- .gitignore | 1 + OSCADSharp/OSCADSharp.ndproj | 8101 ++++++++++++++++++++++++++++++++++ 2 files changed, 8102 insertions(+) create mode 100644 OSCADSharp/OSCADSharp.ndproj diff --git a/.gitignore b/.gitignore index b06e864..0b4744b 100644 --- a/.gitignore +++ b/.gitignore @@ -210,3 +210,4 @@ FakesAssemblies/ GeneratedArtifacts/ _Pvt_Extensions/ ModelManifest.xml +OSCADSharp/NDependOut/ diff --git a/OSCADSharp/OSCADSharp.ndproj b/OSCADSharp/OSCADSharp.ndproj new file mode 100644 index 0000000..e0918c1 --- /dev/null +++ b/OSCADSharp/OSCADSharp.ndproj @@ -0,0 +1,8101 @@ + + + C:\src\OSCADSharp\OSCADSharp\NDependOut + + OSCADSharp + + + mscorlib + System.Core + System + + + C:\Windows\Microsoft.NET\Framework\v4.0.30319 + C:\Windows\Microsoft.NET\Framework\v4.0.30319\WPF + C:\src\OSCADSharp\OSCADSharp\OSCADSharp\bin\Debug + C:\src\OSCADSharp\OSCADSharp\OSCADSharp.UnitTests\bin\Debug + + True + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Types too big - critical +warnif count > 0 from t in JustMyCode.Types where + t.NbLinesOfCode > 500 + // We've commented # IL Instructions, because with LINQ syntax, a few lines of code can compile to hundreds of IL instructions. + // || t.NbILInstructions > 3000 + orderby t.NbLinesOfCode descending +select new { t, t.NbLinesOfCode, t.NbILInstructions, + t.Methods, t.Fields } + +// +// This rule matches types with more than 500 lines of code. +// +// Types where *NbLinesOfCode > 500* are extremely complex +// to develop and maintain. +// See the definition of the NbLinesOfCode metric here +// http://www.ndepend.com/docs/code-metrics#NbLinesOfCode +// +// Maybe you are facing the **God Class** phenomenon: +// A **God Class** is a class that controls way too many other classes +// in the system and has grown beyond all logic to become +// *The Class That Does Everything*. +// +// In average, a line of code is compiled to around +// 6 IL instructions. This is why the code metric +// *NbILInstructions* is used here, in case the +// code metric *NbLinesOfCode* is un-available because +// of missing assemblies corresponding PDB files. +// See the definition of the *NbILInstructions* metric here +// http://www.ndepend.com/docs/code-metrics#NbILInstructions +// + +// +// Types with many lines of code +// should be split in a group of smaller types. +// +// To refactor a *God Class* you'll need patience, +// and you might even need to recreate everything from scratch. +// Here are a few advices: +// +// • Think before pulling out methods: on what data does this method operate? +// What responsibility does it have? +// +// • Try to maintain the interface of the god class at first +// and delegate calls to the new extracted classes. +// In the end the god class should be a pure facade without own logic. +// Then you can keep it for convenience +// or throw it away and start to use the new classes only. +// +// • Unit Tests can help: write tests for each method before extracting it +// to ensure you don't break functionality. +//]]> + Methods too complex - critical +warnif count > 0 from m in JustMyCode.Methods where + m.CyclomaticComplexity > 30 || + m.ILCyclomaticComplexity > 60 || + m.ILNestingDepth > 6 + orderby m.CyclomaticComplexity descending, + m.ILCyclomaticComplexity descending, + m.ILNestingDepth descending +select new { m, m.CyclomaticComplexity, + m.ILCyclomaticComplexity, + m.ILNestingDepth } + +// +// This rule matches methods where *CyclomaticComplexity* > 30 +// or *ILCyclomaticComplexity* > 60 +// or *ILNestingDepth* > 6. +// Such method is typically hard to understand and maintain. +// +// Maybe you are facing the **God Method** phenomenon. +// A "God Method" is a method that does way too many processes in the system +// and has grown beyond all logic to become *The Method That Does Everything*. +// When need for new processes increases suddenly some programmers realize: +// why should I create a new method for each processe if I can only add an *if*. +// +// See the definition of the *CyclomaticComplexity* metric here: +// http://www.ndepend.com/docs/code-metrics#CC +// +// See the definition of the *ILCyclomaticComplexity* metric here: +// http://www.ndepend.com/docs/code-metrics#ILCC +// +// See the definition of the *ILNestingDepth* metric here: +// http://www.ndepend.com/docs/code-metrics#ILNestingDepth +// + +// +// A large and complex method should be split in smaller methods, +// or even one or several classes can be created for that. +// +// During this process it is important to question the scope of each +// variable local to the method. This can be an indication if +// such local variable will become an instance field of the newly created class(es). +// +// Large *switch…case* structures might be refactored through the help +// of a set of types that implement a common interface, the interface polymorphism +// playing the role of the *switch cases tests*. +// +// Unit Tests can help: write tests for each method before extracting it +// to ensure you don't break functionality. +//]]> + Methods with too many parameters - critical +warnif count > 0 from m in JustMyCode.Methods where + m.NbParameters > 8 + orderby m.NbParameters descending +select new { m, m.NbParameters } + +// +// This rule matches methods with more than 8 parameters. +// Such method is painful to call and might degrade performance. +// See the definition of the *NbParameters* metric here: +// http://www.ndepend.com/docs/code-metrics#NbParameters +// + +// +// More properties/fields can be added to the declaring type to +// handle numerous states. An alternative is to provide +// a class or a structure dedicated to handle arguments passing. +// For example see the class *System.Diagnostics.ProcessStartInfo* +// and the method *System.Diagnostics.Process.Start(ProcessStartInfo)*. +//]]> + Quick summary of methods to refactor +warnif count > 0 from m in JustMyCode.Methods where + // Code Metrics' definitions + m.NbLinesOfCode > 30 || // http://www.ndepend.com/docs/code-metrics#NbLinesOfCode + // We've commented # IL Instructions, because with LINQ syntax, a few lines of code can compile to hundreds of IL instructions. + // m.NbILInstructions > 200 || // http://www.ndepend.com/docs/code-metrics#NbILInstructions + m.CyclomaticComplexity > 20 || // http://www.ndepend.com/docs/code-metrics#CC + m.ILCyclomaticComplexity > 50 || // http://www.ndepend.com/docs/code-metrics#ILCC + m.ILNestingDepth > 5 || // http://www.ndepend.com/docs/code-metrics#ILNestingDepth + m.NbParameters > 5 || // http://www.ndepend.com/docs/code-metrics#NbParameters + m.NbVariables > 8 || // http://www.ndepend.com/docs/code-metrics#NbVariables + m.NbOverloads > 6 // http://www.ndepend.com/docs/code-metrics#NbOverloads + +select new { m, m.NbLinesOfCode, m.NbILInstructions, m.CyclomaticComplexity, + m.ILCyclomaticComplexity, m.ILNestingDepth, + m.NbParameters, m.NbVariables, m.NbOverloads } + +// +// Methods matched by this rule somehow violate +// one or several basic quality principles, +// whether it is too large (too many *lines of code*), +// too complex (too many *if*, *switch case*, loops…) +// has too many variables, too many parameters +// or has too many overloads. +// + +// +// To refactor such method and increase code quality and maintainability, +// certainly you'll have to split the method into several smaller methods +// or even create one or several classes to implement the logic. +// +// During this process it is important to question the scope of each +// variable local to the method. This can be an indication if +// such local variable will become an instance field of the newly created class(es). +// +// Large *switch…case* structures might be refactored through the help +// of a set of types that implement a common interface, the interface polymorphism +// playing the role of the *switch cases tests*. +// +// Unit Tests can help: write tests for each method before extracting it +// to ensure you don't break functionality. +//]]> + Methods too big +warnif count > 0 from m in JustMyCode.Methods where + m.NbLinesOfCode > 30 + // We've commented # IL Instructions, because with LINQ syntax, a few lines of code can compile to hundreds of IL instructions. + // || m.NbILInstructions > 200 + orderby m.NbLinesOfCode descending, + m.NbILInstructions descending +select new { m, m.NbLinesOfCode, m.NbILInstructions } + +// +// This rule matches methods where *NbLinesOfCode > 30* or +// (commented per default) *NbILInstructions > 200*. +// Such method can be hard to understand and maintain. +// +// However rules like *Methods too complex* or *Methods with too many variables* +// might be more relevant to detect *painful to maintain* methods, +// because complexity is more related to numbers of *if*, +// *switch case*, loops… than to just number of lines. +// +// See the definition of the *NbLinesOfCode* metric here +// http://www.ndepend.com/docs/code-metrics#NbLinesOfCode +// + +// +// Usually too big methods should be split in smaller methods. +// +// But long methods with no branch conditions, that typically initialize some data, +// are not necessarily a problem to maintain nor to test, and might not need +// refactoring. +//]]> + Methods too complex +warnif count > 0 from m in JustMyCode.Methods where + m.CyclomaticComplexity > 20 || + m.ILCyclomaticComplexity > 40 || + m.ILNestingDepth > 4 + orderby m.CyclomaticComplexity descending, + m.ILCyclomaticComplexity descending, + m.ILNestingDepth descending +select new { m, m.CyclomaticComplexity, + m.ILCyclomaticComplexity, + m.ILNestingDepth } + +// +// This rule matches methods where *CyclomaticComplexity > 20* +// or *ILCyclomaticComplexity > 40* +// or *ILNestingDepth > 4*. +// Such method is typically hard to understand and maintain. +// +// See the definition of the *CyclomaticComplexity* metric here: +// http://www.ndepend.com/docs/code-metrics#CC +// +// See the definition of the *ILCyclomaticComplexity* metric here: +// http://www.ndepend.com/docs/code-metrics#ILCC +// +// See the definition of the *ILNestingDepth* metric here: +// http://www.ndepend.com/docs/code-metrics#ILNestingDepth +// + +// +// A large and complex method should be split in smaller methods, +// or even one or several classes can be created for that. +// +// During this process it is important to question the scope of each +// variable local to the method. This can be an indication if +// such local variable will become an instance field of the newly created class(es). +// +// Large *switch…case* structures might be refactored through the help +// of a set of types that implement a common interface, the interface polymorphism +// playing the role of the *switch cases tests*. +// +// Unit Tests can help: write tests for each method before extracting it +// to ensure you don't break functionality. +//]]> + Methods potentially poorly commented +warnif count > 0 from m in JustMyCode.Methods where + m.PercentageComment < 20 && + m.NbLinesOfCode > 20 + orderby m.PercentageComment ascending +select new { m, m.PercentageComment, m.NbLinesOfCode, m.NbLinesOfComment } + +// +// This rule matches methods with less than 20% of comment lines and that have +// at least 20 lines of code. Such method might need to be more commented. +// +// See the definitions of the *Comments metric* here: +// http://www.ndepend.com/docs/code-metrics#PercentageComment +// http://www.ndepend.com/docs/code-metrics#NbLinesOfComment +// + +// +// Typically add more comment. But code commenting is subject to controversy. +// While poorly written and designed code would needs a lot of comment +// to be understood, clean code doesn't need that much comment, especially +// if variables and methods are properly named and convey enough information. +// Unit-Test code can also play the role of code commenting. +// +// However, even when writing clean and well-tested code, one will have +// to write **hacks** at a point, usually to circumvent some API limitations or bugs. +// A hack is a non-trivial piece of code, that doesn't make sense at first glance, +// and that took time and web research to be found. +// In such situation comments must absolutely be used to express the intention, +// the need for the hacks and the source where the solution has been found. +//]]> + Methods with too many parameters +warnif count > 0 from m in JustMyCode.Methods where + m.NbParameters > 5 + orderby m.NbParameters descending +select new { m, m.NbParameters } + +// +// This rule matches methods with more than 5 parameters. +// Such method might be painful to call and might degrade performance. +// See the definition of the *NbParameters* metric here: +// http://www.ndepend.com/docs/code-metrics#NbParameters +// + +// +// More properties/fields can be added to the declaring type to +// handle numerous states. An alternative is to provide +// a class or a structure dedicated to handle arguments passing. +// For example see the class *System.Diagnostics.ProcessStartInfo* +// and the method *System.Diagnostics.Process.Start(ProcessStartInfo))*. +//]]> + Methods with too many local variables +warnif count > 0 from m in JustMyCode.Methods where + m.NbVariables > 15 + orderby m.NbVariables descending +select new { m, m.NbVariables } + +// +// This rule matches methods with more than 15 variables. +// +// Methods where *NbVariables > 8* are hard to understand and maintain. +// Methods where *NbVariables > 15* are extremely complex and must be refactored. +// +// See the definition of the *Nbvariables* metric here: +// http://www.ndepend.com/docs/code-metrics#Nbvariables +// + +// +// To refactor such method and increase code quality and maintainability, +// certainly you'll have to split the method into several smaller methods +// or even create one or several classes to implement the logic. +// +// During this process it is important to question the scope of each +// variable local to the method. This can be an indication if +// such local variable will become an instance field of the newly created class(es). +//]]> + Methods with too many overloads +warnif count > 0 from m in JustMyCode.Methods where + m.NbOverloads > 6 && + !m.IsOperator // Don't report operator overload + orderby m.NbOverloads descending +let overloads = + m.IsConstructor ? m.ParentType.Constructors : + m.ParentType.Methods.Where(m1 => m1.SimpleName == m.SimpleName) +select new { m, overloads } + +// +// Method overloading is the ability to create multiple methods of the same name +// with different implementations, and various set of parameters. +// +// This rule matches sets of method with more than 6 overloads. +// +// Such method set might be a problem to maintain +// and provokes higher coupling than necessary. +// +// See the definition of the *NbOverloads* metric here +// http://www.ndepend.com/docs/code-metrics#NbOverloads +// + +// +// Typically the *too many overloads* phenomenon appears when an algorithm +// takes a various set of in-parameters. Each overload is presented as +// a facility to provide a various set of in-parameters. +// In such situation, the C# and VB.NET language feature named +// *Named and Optional arguments* should be used. +// +// The *too many overloads* phenomenon can also be a consequence of the usage +// of the **visitor design pattern** http://en.wikipedia.org/wiki/Visitor_pattern +// since a method named *Visit()* must be provided for each sub type. +// In such situation there is no need for fix. +//]]> + Types with too many methods +warnif count > 0 from t in JustMyCode.Types + + // Optimization: Fast discard of non-relevant types + where t.Methods.Count() > 20 + + // Don't match these methods + let methods = t.Methods.Where( + m => !(m.IsGeneratedByCompiler || + m.IsConstructor || m.IsClassConstructor || + m.IsPropertyGetter || m.IsPropertySetter || + m.IsEventAdder || m.IsEventRemover)) + + where methods.Count() > 20 + orderby methods.Count() descending +select new { t, + nbMethods = methods.Count(), + instanceMethods = methods.Where(m => !m.IsStatic), + staticMethods = methods.Where(m => m.IsStatic)} + +// +// This rule matches types with more than 20 methods. +// Such type might be hard to understand and maintain. +// +// Notice that methods like constructors or property +// and event accessors are not taken account. +// +// Having many methods for a type might be a symptom +// of too many responsibilities implemented. +// +// Maybe you are facing the **God Class** phenomenon: +// A **God Class** is a class that controls way too many other classes +// in the system and has grown beyond all logic to become +// *The Class That Does Everything*. +// + +// +// To refactor such type and increase code quality and maintainability, +// certainly you'll have to split the type into several smaller types +// that together, implement the same logic. +// +// To refactor a *God Class* you'll need patience, +// and you might even need to recreate everything from scratch. +// Here are a few advices: +// +// • Think before pulling out methods: +// What responsibility does it have? +// Can you isolate some subsets of methods that operate on the same subsets of fields? +// +// • Try to maintain the interface of the god class at first +// and delegate calls to the new extracted classes. +// In the end the god class should be a pure facade without own logic. +// Then you can keep it for convenience +// or throw it away and start to use the new classes only. +// +// • Unit Tests can help: write tests for each method before extracting it +// to ensure you don't break functionality. +//]]> + Types with too many fields +warnif count > 0 from t in JustMyCode.Types + + // Optimization: Fast discard of non-relevant types + where !t.IsEnumeration && + t.Fields.Count() > 20 + + // Count instance fields and non-constant static fields + let fields = t.Fields.Where(f => + !f.IsGeneratedByCompiler && + !f.IsLiteral && + !(f.IsStatic && f.IsInitOnly) && + JustMyCode.Contains(f) ) + + where fields.Count() > 20 + + orderby fields.Count() descending +select new { t, + instanceFields = fields.Where(f => !f.IsStatic), + staticFields = fields.Where(f => f.IsStatic), + + // See definition of Size of Instances metric here: + // http://www.ndepend.com/docs/code-metrics#SizeOfInst + t.SizeOfInst +} + +// +// This rule matches types with more than 20 fields. +// Such type might be hard to understand and maintain. +// +// Notice that constant fields and static-readonly fields are not counted. +// Enumerations types are not counted also. +// +// Having many fields for a type might be a symptom +// of too many responsibilities implemented. +// + +// +// To refactor such type and increase code quality and maintainability, +// certainly you'll have to group subsets of fields into smaller types +// and dispatch the logic implemented into the methods +// into these smaller types. +//]]> + Types with poor cohesion +warnif count > 0 from t in JustMyCode.Types where + (t.LCOM > 0.8 || t.LCOMHS > 0.95) && + t.NbFields > 10 && + t.NbMethods >10 + orderby t.LCOM descending, t.LCOMHS descending +select new { t, t.LCOM, t.LCOMHS, + t.NbMethods, t.NbFields } + +// +// This rule is based on the *LCOM code metric*, +// LCOM stands for **Lack Of Cohesion of Methods**. +// See the definition of the LCOM metric here +// http://www.ndepend.com/docs/code-metrics#LCOM +// +// The LCOM metric measures the fact that most methods are using most fields. +// A class is considered utterly cohesive (which is good) +// if all its methods use all its instance fields. +// +// Only types with enough methods and fields are taken account to avoid bias. +// The LCOM takes its values in the range [0-1]. +// +// This rule matches types with LCOM higher than 0.8. +// Such value generally pinpoints a **poorly cohesive class**. +// +// There are several LCOM metrics. +// The LCOM HS (HS stands for Henderson-Sellers) takes its values in the range [0-2]. +// A LCOM HS value higher than 1 should be considered alarming. +// + +// +// To refactor a poorly cohesive type and increase code quality and maintainability, +// certainly you'll have to split the type into several smaller and more cohesive types +// that together, implement the same logic. +//]]> + + + From now, all methods added or refactored should respect basic quality principles +warnif count > 0 from m in JustMyCode.Methods where + +// *** Only match new or refactored methods since Baseline for Comparison *** + (m.WasAdded() || m.CodeWasChanged()) && + +// Low Quality methods// Metrics' definitions +( m.NbLinesOfCode > 30 || // http://www.ndepend.com/docs/code-metrics#NbLinesOfCode + m.NbILInstructions > 200 || // http://www.ndepend.com/docs/code-metrics#NbILInstructions + m.CyclomaticComplexity > 20 || // http://www.ndepend.com/docs/code-metrics#CC + m.ILCyclomaticComplexity > 50 || // http://www.ndepend.com/docs/code-metrics#ILCC + m.ILNestingDepth > 4 || // http://www.ndepend.com/docs/code-metrics#ILNestingDepth + m.NbParameters > 5 || // http://www.ndepend.com/docs/code-metrics#NbParameters + m.NbVariables > 8 || // http://www.ndepend.com/docs/code-metrics#NbVariables + m.NbOverloads > 6 ) +select new { m, m.NbLinesOfCode, m.NbILInstructions, m.CyclomaticComplexity, + m.ILCyclomaticComplexity, m.ILNestingDepth, + m.NbParameters, m.NbVariables, m.NbOverloads } // http://www.ndepend.com/docs/code-metrics#NbOverloads + +// +// This rule is executed only if a *baseline for comparison* is defined (*diff mode*). +// This rule operates only on methods added or refactored since the baseline. +// The same effect can be obtained for any rules through: +// *Dashboard > Filter Recent Violations Only*. +// Doing so let's focus on code quality on recent changes only. +// +// Methods matched by this rule not only have been recently added or refactored, +// but also somehow violate one or several basic quality principles, +// whether it is too large (too many *lines of code*), +// too complex (too many *if*, *switch case*, loops…) +// has too many variables, too many parameters +// or has too many overloads. +// + +// +// To refactor such method and increase code quality and maintainability, +// certainly you'll have to split the method into several smaller methods +// or even create one or several classes to implement the logic. +// +// During this process it is important to question the scope of each +// variable local to the method. This can be an indication if +// such local variable will become an instance field of the newly created class(es). +// +// Large *switch…case* structures might be refactored through the help +// of a set of types that implement a common interface, the interface polymorphism +// playing the role of the *switch cases tests*. +// +// Unit Tests can help: write tests for each method before extracting it +// to ensure you don't break functionality. +//]]> + From now, all types added or refactored should respect basic quality principles +warnif count > 0 from t in JustMyCode.Types where + +// *** Only match new or refactored types since Baseline for Comparison *** +(t.WasAdded() || t.CodeWasChanged()) && + +// Eliminate interfaces, enumerations or types only with constant fields +// by making sure we are matching type with code. +t.NbLinesOfCode > 10 && + +// Optimization: Fast discard of non-relevant types +(t.Fields.Count() > 20 || t.Methods.Count() > 20) + +// Count instance fields and non-constant static fields +let fields = t.Fields.Where(f => + !f.IsLiteral && + !(f.IsStatic && f.IsInitOnly)) + +// Don't match these methods +let methods = t.Methods.Where( + m => !(m.IsConstructor || m.IsClassConstructor || + m.IsGeneratedByCompiler || + m.IsPropertyGetter || m.IsPropertySetter || + m.IsEventAdder || m.IsEventRemover)) + +where + +// Low Quality types Metrics' definitions are available here: +// http://www.ndepend.com/docs/code-metrics#MetricsOnTypes +( // Types with too many methods + fields.Count() > 20 || + + methods.Count() > 20 || + + // Complex Types that use more than 50 other types + t.NbTypesUsed > 50 +) +select new { t, t.NbLinesOfCode, + + instanceMethods = methods.Where(m => !m.IsStatic), + staticMethods = methods.Where(m => m.IsStatic), + + instanceFields = fields.Where(f => !f.IsStatic), + staticFields = fields.Where(f => f.IsStatic), + + t.TypesUsed } + +// +// This rule is executed only if a *baseline for comparison* is defined (*diff mode*). +// This rule operates only on types added or refactored since the baseline. +// The same effect can be obtained for any rules through: +// *Dashboard > Filter Recent Violations Only*. +// Doing so let's focus on code quality on recent changes only. +// +// Types matched by this rule not only have been recently added or refactored, +// but also somehow violate one or several basic quality principles, +// whether it has too many methods, +// it has too many fields, +// or is using too many types. +// Any of these criterions is often a symptom of a type with too many responsibilities. +// +// Notice that to count methods and fields, methods like constructors +// or property and event accessors are not taken account. +// Notice that constants fields and static-readonly fields are not counted. +// Enumerations types are not counted also. +// + +// +// To refactor such type and increase code quality and maintainability, +// certainly you'll have to split the type into several smaller types +// that together, implement the same logic. +//]]> + From now, all types added or refactored should be 100% covered by tests +warnif count > 0 from t in JustMyCode.Types where + + // *** Only match new or refactored types since Baseline for Comparison *** + (t.WasAdded() || t.CodeWasChanged()) && + + // …that are not 100% covered by tests + t.PercentageCoverage < 100 + + let methodsCulprit = t.Methods.Where(m => m.PercentageCoverage < 100) + +select new { t, t.PercentageCoverage, methodsCulprit } + +// +// This rule is executed only if a *baseline for comparison* is defined (*diff mode*). +// This rule operates only on types added or refactored since the baseline. +// The same effect can be obtained for any rules through: +// *Dashboard > Filter Recent Violations Only*. +// Doing so let's focus on code quality on recent changes only. +// +// This rule is executed only if some code coverage data is imported +// from some code coverage files. +// +// Often covering 10% of remaining uncovered code of a class, +// requires as much work as covering the first 90%. +// For this reason, typically teams estimate that 90% coverage is enough. +// However *untestable code* usually means *poorly written code* +// which usually leads to *error prone code*. +// So it might be worth refactoring and making sure to cover the 10% remaining code +// because **most tricky bugs might come from this small portion of hard-to-test code**. +// +// Not all classes should be 100% covered by tests (like UI code can be hard to test) +// but you should make sure that most of the logic of your application +// is defined in some *easy-to-test classes*, 100% covered by tests. +// +// In this context, this rule warns when a type added or refactored since the baseline, +// is not fully covered by tests. +// + +// +// Write more unit-tests dedicated to cover code not covered yet. +// If you find some *hard-to-test code*, it is certainly a sign that this code +// is not *well designed* and hence, needs refactoring. +// +// You'll find code impossible to cover by unit-tests, like calls to *MessageBox.Show()*. +// An infrastructure must be defined to be able to *mock* such code at test-time. +//]]> + Avoid decreasing code coverage by tests of types + +warnif count > 0 +from t in JustMyCode.Types where + t.IsPresentInBothBuilds() && + t.PercentageCoverage < t.OlderVersion().PercentageCoverage + +select new { t, + OldCov = t.OlderVersion().PercentageCoverage, + NewCov = t.PercentageCoverage, + OldLoc = t.OlderVersion().NbLinesOfCode, + NewLoc = t.NbLinesOfCode, +} + +// +// This rule is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This rule is executed only if some code coverage data is imported +// from some code coverage files. +// +// This rule warns when a type code coverage ratio +// decreased since the baseline. +// This can mean that some tests have been removed +// but more often, this means that the type has been modified, +// and that changes haven't been covered by tests. +// +// To visualize changes in code, right-click a matched type and select: +// +// • Compare older and newer versions of source file +// +// • or Compare older and newer versions disassembled with Reflector +// + +// +// Write more unit-tests dedicated to cover changes in matched types +// not covered yet. +// If you find some *hard-to-test code*, it is certainly a sign that this code +// is not *well designed* and hence, needs refactoring. +//]]> + Types that used to be 100% covered but not anymore + +warnif count > 0 +from t in JustMyCode.Types where + t.IsPresentInBothBuilds() && + t.OlderVersion().PercentageCoverage == 100 && + t.PercentageCoverage < 100 +let culpritMethods = t.Methods.Where(m => m.PercentageCoverage < 100) +select new {t, t.PercentageCoverage, culpritMethods } + +// +// This rule is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This rule is executed only if some code coverage data is imported +// from some code coverage files. +// +// Often covering 10% of remaining uncovered code of a class, +// requires as much work as covering the first 90%. +// For this reason, typically teams estimate that 90% coverage is enough. +// However *untestable code* usually means *poorly written code* +// which usually leads to *error prone code*. +// So it might be worth refactoring and making sure to cover the 10% remaining code +// **because most tricky bugs might come from this small portion of hard-to-test code**. +// +// Not all classes should be 100% covered by tests (like UI code can be hard to test) +// but you should make sure that most of the logic of your application +// is defined in some *easy-to-test classes*, 100% covered by tests. +// +// In this context, this rule warns when a type fully covered by tests is now only partially covered. +// + +// +// Write more unit-tests dedicated to cover code not covered anymore. +// If you find some *hard-to-test code*, it is certainly a sign that this code +// is not *well designed* and hence, needs refactoring. +// +// You'll find code impossible to cover by unit-tests, like calls to *MessageBox.Show()*. +// An infrastructure must be defined to be able to *mock* such code at test-time. +//]]> + Avoid making complex methods even more complex (Source CC) + +warnif count > 0 +from m in JustMyCode.Methods where + !m.IsAbstract && + m.IsPresentInBothBuilds() && + m.CodeWasChanged() + +let oldCC = m.OlderVersion().CyclomaticComplexity +where oldCC > 6 && m.CyclomaticComplexity > oldCC + +select new { m, + oldCC , + newCC = m.CyclomaticComplexity , + oldLoc = m.OlderVersion().NbLinesOfCode, + newLoc = m.NbLinesOfCode, +} + +// +// This rule is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// The method complexity is measured through the code metric +// *Cyclomatic Complexity* defined here: +// http://www.ndepend.com/docs/code-metrics#CC +// +// This rule warns when a method already complex +// (i.e with *Cyclomatic Complexity* higher than 6) +// become even more complex since the baseline. +// +// This rule needs assemblies PDB files and source code +// to be available at analysis time, because the *Cyclomatic Complexity* +// is inferred from the source code and source code location +// is inferred from PDB files. See: +// http://www.ndepend.com/docs/ndepend-analysis-inputs-explanation +// +// To visualize changes in code, right-click a matched method and select: +// +// • Compare older and newer versions of source file +// +// • or Compare older and newer versions disassembled with Reflector +// + +// +// A large and complex method should be split in smaller methods, +// or even one or several classes can be created for that. +// +// During this process it is important to question the scope of each +// variable local to the method. This can be an indication if +// such local variable will become an instance field of the newly created class(es). +// +// Large *switch…case* structures might be refactored through the help +// of a set of types that implement a common interface, the interface polymorphism +// playing the role of the *switch cases tests*. +// +// Unit Tests can help: write tests for each method before extracting it +// to ensure you don't break functionality. +//]]> + Avoid making complex methods even more complex (IL CC) + +warnif count > 0 +from m in JustMyCode.Methods where + !m.IsAbstract && + m.IsPresentInBothBuilds() && + m.CodeWasChanged() + +let oldCC = m.OlderVersion().ILCyclomaticComplexity +where oldCC > 10 && m.ILCyclomaticComplexity > oldCC + +select new { m, + oldCC , + newCC = m.ILCyclomaticComplexity , + oldLoc = m.OlderVersion().NbLinesOfCode, + newLoc = m.NbLinesOfCode, +} + +// +// This rule is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// The method complexity is measured through the code metric +// *IL Cyclomatic Complexity* defined here: +// http://www.ndepend.com/docs/code-metrics#ILCC +// +// This rule warns when a method already complex +// (i.e with *IL Cyclomatic Complexity* higher than 10) +// become even more complex since the baseline. +// +// If assemblies PDB files and source code +// are available at analysis time, +// the *Cyclomatic Complexity* can be inferred from source code, +// and this is more precise than inferring it from IL code. +// Hence, prefer use the rule +// *Avoid making complex methods even more complex (Source CC)* +// that offers more precise result. +// +// To visualize changes in code, right-click a matched method and select: +// +// • Compare older and newer versions of source file +// +// • or Compare older and newer versions disassembled with Reflector +// + +// +// A large and complex method should be split in smaller methods, +// or even one or several classes can be created for that. +// +// During this process it is important to question the scope of each +// variable local to the method. This can be an indication if +// such local variable will become an instance field of the newly created class(es). +// +// Large *switch…case* structures might be refactored through the help +// of a set of types that implement a common interface, the interface polymorphism +// playing the role of the *switch cases tests*. +// +// Unit Tests can help: write tests for each method before extracting it +// to ensure you don't break functionality. +//]]> + Avoid making large methods even larger + +warnif count > 0 +from m in JustMyCode.Methods where + !m.IsAbstract && + m.IsPresentInBothBuilds() && + m.CodeWasChanged() && + // Eliminate constructors from match, since they get larger + // as soons as some fields initialization are added. + !m.IsConstructor && + !m.IsClassConstructor + +let oldLoc = m.OlderVersion().NbLinesOfCode +where oldLoc > 15 && m.NbLinesOfCode > oldLoc + +select new { m, + oldLoc, + newLoc = m.NbLinesOfCode, +} + +// +// This rule is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This rule warns when a method already large +// (i.e with more than 15 lines of code) +// become even larger since the baseline. +// +// The method size is measured through the code metric +// *# Lines of Code* defined here: +// http://www.ndepend.com/docs/code-metrics#NbLinesOfCode +// +// This rule needs assemblies PDB files +// to be available at analysis time, because the *# Lines of Code* +// is inferred from PDB files. See: +// http://www.ndepend.com/docs/ndepend-analysis-inputs-explanation +// +// To visualize changes in code, right-click a matched method and select: +// +// • Compare older and newer versions of source file +// +// • or Compare older and newer versions disassembled with Reflector +// + +// +// Usually too big methods should be split in smaller methods. +// +// But long methods with no branch conditions, that typically initialize some data, +// are not necessarily a problem to maintain, and might not need refactoring. +//]]> + Avoid adding methods to a type that already had many methods + +warnif count > 0 + +// Don't count constructors and methods generated by the compiler! +let getMethodsProc = new Func>( + t => t.Methods.Where(m => + !m.IsConstructor && !m.IsClassConstructor && + !m.IsGeneratedByCompiler).ToArray()) + + +from t in JustMyCode.Types where + + t.IsPresentInBothBuilds() + + // Optimization: fast discard of non-relevant types + where t.OlderVersion().NbMethods > 15 + + let oldMethods = getMethodsProc(t.OlderVersion()) + where oldMethods.Count > 15 + + let newMethods = getMethodsProc(t) + where newMethods.Count > oldMethods.Count + + let addedMethods = newMethods.Where(m => m.WasAdded()) + let removedMethods = oldMethods.Where(m => m.WasRemoved()) + +select new { + t, + nbOldMethods = oldMethods.Count, + nbNewMethods = newMethods.Count, + addedMethods , + removedMethods +} + +// +// This rule is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// Types where number of methods is greater than 15 +// might be hard to understand and maintain. +// +// This rule lists types that already had more than 15 methods +// at the baseline time, and for which new methods have been added. +// +// Having many methods for a type might be a symptom +// of too many responsibilities implemented. +// +// Notice that constructors and methods generated by the compiler +// are not taken account. +// + +// +// To refactor such type and increase code quality and maintainability, +// certainly you'll have to split the type into several smaller types +// that together, implement the same logic. +//]]> + Avoid transforming an immutable type into a mutable one + +warnif count > 0 +from t in Application.Types where + t.IsPresentInBothBuilds() && + t.OlderVersion().IsImmutable && + !t.IsImmutable && + // Don't take account of immutable types transformed into static types (not deemed as immtable) + !t.IsStatic +let culpritFields = t.Fields.Where(f => f.IsImmutable) +select new { + t, + culpritFields +} + +// +// This rule is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// A type is considered as *immutable* if its instance fields +// cannot be modified once an instance has been built by a constructor. +// +// Being immutable has several fortunate consequences for a type. +// For example its instance objects can be used concurrently +// from several threads without the need to synchronize accesses. +// +// Hence users of such type often rely on the fact that the type is immutable. +// If an immutable type becomes mutable, there are chances that this will break +// users code. +// This is why this rule warns about such immutable type that become mutable. +// + +// +// If being immutable is an important property for a matched type, +// then the code must be refactored to preserve immutability. +//]]> + Avoid adding instance fields to a type that already had many instance fields + +warnif count > 0 + +// Count instance fields and non-readonly static fields +let getFieldsProc = new Func>( + t => t.Fields.Where(f => + !f.IsLiteral && + !f.IsGeneratedByCompiler && + !(f.IsStatic && f.IsInitOnly)).ToArray()) + + +from t in JustMyCode.Types where + + !t.IsEnumeration && + t.IsPresentInBothBuilds() + + // Optimization: fast discard of non-relevant types + where t.OlderVersion().NbFields > 15 + + let oldFields = getFieldsProc(t.OlderVersion()) + where oldFields.Count > 15 + + let newFields = getFieldsProc(t) + where newFields.Count > oldFields.Count + + let addedFields = newFields.Where(f => f.WasAdded()) + let removedFields = oldFields.Where(f => f.WasRemoved()) + +select new { + t, + nbOldFields = oldFields.Count, + nbNewFields = newFields.Count, + addedFields , + removedFields +} + +// +// This rule is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// Types where number of fields is greater than 15 +// might be hard to understand and maintain. +// +// This rule lists types that already had more than 15 fields +// at the baseline time, and for which new fields have been added. +// +// Having many fields for a type might be a symptom +// of too many responsibilities implemented. +// +// Notice that *constants* fields and *static-readonly* fields are not taken account. +// Enumerations types are not taken account also. +// + +// +// To refactor such type and increase code quality and maintainability, +// certainly you'll have to group subsets of fields into smaller types +// and dispatch the logic implemented into the methods +// into these smaller types. +//]]> + + + Base class should not use derivatives + +warnif count > 0 +from baseClass in JustMyCode.Types +where baseClass.IsClass && baseClass.NbChildren > 0 // <-- for optimization! +let derivedClassesUsed = baseClass.DerivedTypes.UsedBy(baseClass) +where derivedClassesUsed.Count() > 0 +select new { baseClass, derivedClassesUsed } + +// +// In *Object-Oriented Programming*, the **open/closed principle** states: +// *software entities (components, classes, methods, etc.) should be open +// for extension, but closed for modification*. +// http://en.wikipedia.org/wiki/Open/closed_principle +// +// Hence a base class should be designed properly to make it easy to derive from, +// this is *extension*. But creating a new derived class, or modifying an +// existing one, shouldn't provoke any *modification* in the base class. +// And if a base class is using some derivative classes somehow, there +// are good chances that such *modification* will be needed. +// +// Extending the base class is not anymore a simple operation, +// this is not good design. +// + +// +// Understand the need for using derivatives, +// then imagine a new design, and then refactor. +// +// Typically an algorithm in the base class needs to access something +// from derived classes. You can try to encapsulate this access behind +// an abstract or a virtual method. +// +// If you see in the base class some conditions on *typeof(DerivedClass)* +// not only *urgent refactoring* is needed. Such condition can easily +// be replaced through an abstract or a virtual method. +// +// Sometime you'll see a base class that creates instance of some derived classes. +// In such situation, certainly using the *factory method pattern* +// http://en.wikipedia.org/wiki/Factory_method_pattern +// or the *abstract factory pattern* +// http://en.wikipedia.org/wiki/Abstract_factory_pattern +// will improve the design. +//]]> + Class shouldn't be too deep in inheritance tree + +warnif count > 0 from t in JustMyCode.Types +where t.IsClass +let baseClasses = t.BaseClasses.ExceptThirdParty() + +where baseClasses.Count() >= 3 + +select new { t, baseClasses, + // The metric value DepthOfInheritance takes account + // of third-party base classessee its definition here: + // http://www.ndepend.com/docs/code-metrics#DIT + t.DepthOfInheritance } + +// +// This rule warns about classes having 3 or more base classes. +// Notice that third-party base classes are not counted +// because this rule is about your code design, not +// third-party libraries consumed design. +// +// *In theory*, there is nothing wrong having a *long inheritance chain*, +// if the modelization has been well thought out, +// if each base class is a well-designed refinement of the domain. +// +// *In practice*, modeling properly a domain demands a lot of effort +// and experience and more often than not, a *long inheritance chain* +// is a sign of confused design, that will be hard to work with and maintain. +// + +// +// In *Object-Oriented Programming*, a well-known motto is +// **Favor Composition over Inheritance**. +// +// This is because *inheritance* comes with pitfalls. +// In general, the implementation of a derived class is very bound up with +// the base class implementation. Also a base class exposes implementation +// details to its derived classes, that's why it's often said that +// inheritance breaks encapsulation. +// +// On the other hands, *Composition* favors binding with interfaces +// over binding with implementations. Hence, not only the encapsulation +// is preserved, but the design is clearer, because interfaces make it explicit +// and less coupled. +// +// Hence, to break a *long inheritance chain*, *Composition* is often +// a powerful way to enhance the design of the refactored underlying logic. +// +// You can also read: +// http://en.wikipedia.org/wiki/Composition_over_inheritance and +// http://stackoverflow.com/questions/49002/prefer-composition-over-inheritance +//]]> + Class with no descendant should be sealed if possible +warnif count > 0 from t in JustMyCode.Types where + t.IsClass && + t.NbChildren ==0 && + !t.IsSealed && + !t.IsStatic +// && !t.IsPublic //<-- You might want to add this condition + // if you are developing a framework + // with classes that are intended to be + // sub-classed by your clients. + orderby t.NbLinesOfCode descending +select new { t, t.NbLinesOfCode } + + +// +// If a *non-static* class isn't declared with the keyword *sealed*, +// it means that it can be subclassed everywhere the *non-sealed* +// class is visible. +// +// Making a class a *base class* requires significant design effort. +// Subclassing a *non-sealed* class, not initially designed +// to be subclassed, will lead to unanticipated design issue. +// +// Most classes are *non-sealed* because developers don't care about +// the keyword *sealed*, not because the primary intention was to write +// a class that can be subclassed. +// +// There are minor performance gain in declaring a class as *sealed*. +// But the real benefit of doing so, is actually to **express the +// intention**: *this class has not be designed to be a base class, +// hence it is not allowed to subclass it*. +// +// Notice that by default this rule matches also *public* class. +// If you are developing a framework with classes that are intended +// to be subclassed by your clients, just uncomment the line +// *&& !t.IsPublic*. +// + +// +// For each matched class, take the time to assess if it is really +// meant to be subclassed. Certainly most matched class will end up +// being declared as *sealed*. +//]]> + Overrides of Method() should call base.Method() +warnif count > 0 +from t in Types // Take account of third-party base classes also + +// Bother only classes with descendant +where t.IsClass && t.NbChildren > 0 + +from mBase in t.InstanceMethods +where mBase.IsVirtual && + !mBase.IsThirdParty && + !mBase.IsAbstract && + !mBase.IsExplicitInterfaceImpl +from mOverride in mBase.OverridesDirectDerived +where !mOverride.IsUsing(mBase) +select new { mOverride, shouldCall = mBase, definedInBaseClass = mBase.ParentType } + +// +// Typically overrides of a base method, should **refine** or **complete** +// the behavior of the base method. If the base method is not called, +// the base behavior is not refined but it is *replaced*. +// +// Violations of this rule are a sign of *design flaw*, +// especially if the actual design provides valid reasons +// that advocates that the base behavior must be replaced and not refined. +// + +// +// You should investigate if *inheritance* is the right choice +// to bind the base class implementation with the derived classes +// implementations. Does presenting the method with polymorphic +// behavior through an interface, would be a better design choice? +// +// In such situation, often using the design pattern **template method** +// http://en.wikipedia.org/wiki/Template_method_pattern might help +// improving the design. +//]]> + Do not hide base class methods +warnif count > 0 + +// Define a lookup table indexing methods by their name including parameters signature. +let lookup = Methods.Where(m => !m.IsConstructor && !m.IsStatic && !m.IsGeneratedByCompiler) + .ToLookup(m1 => m1.Name) + +from t in Application.Types +where !t.IsStatic && t.IsClass && + // Discard classes deriving directly from System.Object + t.DepthOfInheritance > 1 +where t.BaseClasses.Any() + +// For each methods not overriding any methods (new slot), +// let's check if it hides by name some methods defined in base classes. +from m in t.InstanceMethods +where m.IsNewSlot && !m.IsExplicitInterfaceImpl && !m.IsGeneratedByCompiler + +// Notice how lookup is used to quickly retrieve methods with same name as m. +// This makes the query 10 times faster than iterating each base methods to check their name. +let baseMethodsHidden = lookup[m.Name].Where(m1 => m1 != m && t.DeriveFrom(m1.ParentType)) + +where baseMethodsHidden.Count() > 0 +select new { m, baseMethodsHidden } + +// +// Method hiding is when a base class has a non-virtual method *M()*, +// and a derived class has also a method *M()* with the same signature. +// In such situation, calling *base.M()* does something different +// than calling *derived.M()*. +// +// Notice that this is not *polymorphic* behavior. With *polymorphic* +// behavior, calling both *base.M()* and *derived.M()* on an instance +// object of *derived*, invoke the same implementation. +// +// This situation should be avoided because it obviously leads to confusion. +// This rule warns about all method hiding cases in the code base. +// + +// +// To fix a violation of this rule, remove or rename the method, +// or change the parameter signature so that the method does +// not hide the base method. +// +// However *method hiding is for those times when you need to have two +// things to have the same name but different behavior*. This is a very +// rare situations, described here: +// http://blogs.msdn.com/b/ericlippert/archive/2008/05/21/method-hiding-apologia.aspx +//]]> + A stateless class or structure might be turned into a static type + +warnif count > 0 from t in JustMyCode.Types where + !t.IsStatic && + !t.IsGeneric && + t.InstanceFields.Count() == 0 && + + // Don't match: + // --> types that implement some interfaces. + t.NbInterfacesImplemented == 0 && + + // --> or classes that have sub-classes children. + t.NbChildren == 0 && + + // --> or classes that have a base class + ((t.IsClass && t.DepthOfDeriveFrom("System.Object".AllowNoMatch()) == 1) || + t.IsStructure) + +select t + +// +// This rule matches classes and structures that are not static, nor generic, +// that doesn't have any instance fields, that doesn't implement any interface +// nor has a base class (different than *System.Object*). +// +// Such class or structure is a *stateless* collection of *pure* functions, +// that doesn't act on any *this* object data. Such collection of *pure* functions +// is better hosted in a **static class**. Doing so simplifies the client code +// that doesn't have to create an object anymore to invoke the *pure* functions. +// + +// +// Declare all methods as *static* and transform the class or structure +// into a *static* class. +//]]> + Non-static classes should be instantiated or turned to static +warnif count > 0 +from t in JustMyCode.Types +where t.IsClass && + //!t.IsPublic && // if you are developing a framework, + // you might not want to match public classes + !t.IsStatic && + !t.IsAttributeClass && // Attributes class are never seen as instantiated + + // Don't suggest to turn to static, classes that implement interfaces + t.InterfacesImplemented.Count() == 0 && + + !t.DeriveFrom("System.MarshalByRefObject".AllowNoMatch()) && // Types instantiated through remoting infrastructure + + // XML serialized type might never be seen as instantiated. + !t.HasAttribute("System.Xml.Serialization.XmlRootAttribute".AllowNoMatch()) && + !t.IsUsing("System.Xml.Serialization.XmlElementAttribute".AllowNoMatch()) && + !t.IsUsing("System.Xml.Serialization.XmlAttributeAttribute".AllowNoMatch()) + +// find the first constructor of t called +let ctorCalled = t.Constructors.FirstOrDefault(ctor => ctor.NbMethodsCallingMe > 0) + +// match t if none of its constructors is called. +where ctorCalled == null +select new { t, t.Visibility } + +// Notice that classes only instantiated through reflection, like plug-in root classes +// are matched by this rules. + +// +// If the constructors of a class are never called, the class is +// never instantiated, and should be defined as a *static class*. +// +// However this rule doesn't match instantiation through reflection. +// As a consequence, plug-in root classes, instantiated through reflection +// via *IoC frameworks*, can be *false positives* for this rule. +// +// Notice that by default this rule matches also *public* class. +// If you are developing a framework with classes that are intended +// to be instantiated by your clients, just uncomment the line +// *!t.IsPublic*. +// + +// +// First it is important to investigate why the class is never instantiated. +// If the reason is *the class hosts only static methods* then the class +// can be safely declared as *static*. +// +// Others reasons like, *the class is meant to be instantiated via reflection*, +// or *is meant to be instantiated only by client code* should lead to +// adapt this rule code to avoid these matches. +//]]> + Methods should be declared static if possible +warnif count > 0 + +from t in JustMyCode.Types.Where(t => + !t.IsStatic && !t.IsInterface && + !t.IsEnumeration && !t.IsDelegate && + !t.IsGeneratedByCompiler) + +let methodsThatCanBeMadeStatic = + from m in t.InstanceMethods + + // An instance method can be turned to static if it is not virtual, + // not using the this reference and also, not using + // any of its class or base classes instance fields or instance methods. + where !m.IsAbstract && !m.IsVirtual && + !m.AccessThis && !m.IsExplicitInterfaceImpl && + + // Optimization: Using FirstOrDefault() avoid to check all members, + // as soon as one member is found + // we know the method m cannot be made static. + m.MembersUsed.FirstOrDefault( + mUsed => !mUsed.IsStatic && + (mUsed.ParentType == t || + t.DeriveFrom(mUsed.ParentType)) + ) == null + select m + +from m in methodsThatCanBeMadeStatic +let staticFieldsUsed = m.ParentType.StaticFields.UsedBy(m).Where(f => !f.IsGeneratedByCompiler) +select new { m, staticFieldsUsed } + +// +// When an instance method can be *safely* declared as static you should declare it as static. +// +// Whenever you write a method, you fulfill a contract in a given scope. +// The narrower the scope is, the smaller the chance is that you write a bug. +// +// When a method is static, you can't access non-static members; hence, your scope is +// narrower. So, if you don't need and will never need (even in subclasses) instance +// fields to fulfill your contract, why give access to these fields to your method? +// Declaring the method static in this case will let the compiler check that you +// don't use members that you do not intend to use. +// +// Declaring a method as static if possible is also good practice because clients can +// tell from the method signature that calling the method can't alter the object's state. +// +// Doing so, is also a micro performance optimization, since a static method is a +// bit cheaper to invoke than an instance method, because the *this* reference* +// doesn't need anymore to be passed. +// +// Notice that if a matched method is a handler, bound to an event through code +// generated by a designer, declaring it as static might break the designer +// generated code, if the generated code use the *this* invocation syntax, +// (like *this.Method()*). +// + +// +// Declare matched methods as static. +// +// Since such method doesn't use any instance fields and methods of its type and +// base-types, you should consider if it makes sense, to move such a method +// to a static utility class. +//]]> + Constructor should not call a virtual method +warnif count > 0 + +from t in Application.Types where + t.IsClass && + !t.IsGeneratedByCompiler && + !t.IsSealed + +from ctor in t.Constructors +let virtualMethodsCalled = + from mCalled in ctor.MethodsCalled + where mCalled.IsVirtual && !mCalled.IsFinal && + // Only take care of just-my-code virtual methods called + JustMyCode.Contains(mCalled) && + ( mCalled.ParentType == t || + (t.DeriveFrom(mCalled.ParentType) && + // Don't accept Object methods since they can be called + // from another reference than the 'this' reference. + mCalled.ParentType.FullName != "System.Object") + ) + select mCalled +where virtualMethodsCalled.Count() > 0 + +select new { ctor , + virtualMethodsCalled, + // If there is no derived type, it might be + // an opportunity to mark t as sealed. + t.DerivedTypes } + +// +// This rule matches constructors of a non-sealed class that call one or +// several virtual methods. +// +// When an object written in C# is constructed, what happens is that constructors +// run in order from the base class to the most derived class. +// +// Also objects do not change type as they are constructed, but start out as +// the most derived type, with the method table being for the most derived type. +// This means that virtual method calls always run on the most derived type, +// even when calls are made from the constructor. +// +// When you combine these two facts you are left with the problem that if you +// make a virtual method call in a constructor, and it is not the most derived +// type in its inheritance hierarchy, then it will be called on a class whose +// constructor has not been run, and therefore may not be in a suitable state +// to have that method called. +// +// Hence this situation makes the class *fragile to derive from*. +// + +// +// Violations reported can be solved by re-designing object initialisation +// or by declaring the parent class as *sealed*, if possible. +// +]]> + Avoid the Singleton pattern +warnif count > 0 +from t in Application.Types +where !t.IsStatic && !t.IsAbstract && (t.IsClass || t.IsStructure) + +// All ctors of a singleton are private +where t.Constructors.Where(ctor => !ctor.IsPrivate).Count() == 0 + +// A singleton contains one static field of its parent type, to reference the unique instance +let staticFieldInstances = t.StaticFields.WithFieldType(t) +where staticFieldInstances.Count() == 1 +select new { t, staticFieldInstance = staticFieldInstances.First() } + +// +// The *singleton pattern* consists in enforcing that a class has just +// a single instance: http://en.wikipedia.org/wiki/Singleton_pattern +// At first glance, this pattern looks appealing, it is simple to implement, +// it adresses a common situation, and as a consequence it is widely used. +// +// However, we discourage you from using singleton classes because experience +// shows that **singleton often results in less testable and less maintainable code**. +// Singleton is *by-design*, not testable. Each unit test should use their own objects +// while singleton forces multiple unit-tests to use the same instance object. +// +// Also the singleton static *GetInstance()* method allows *magic* access to that +// single object and its state from wherever developers want! This potentially +// attractive facility unfortunatly ends up into *unorganized*/*messy* code that +// will require effort to be refactored. +// +// More details available in these discussions: +// http://codebetter.com/patricksmacchia/2011/05/04/back-to-basics-usage-of-static-members/ +// http://adamschepis.com/blog/2011/05/02/im-adam-and-im-a-recovering-singleton-addict/ +// + +// +// This rule matches *the classic syntax of singletons*, where one +// static field hold the single instance of the parent class. We underline that +// *the problem is this particular syntax*, that plays against testability. +// The problem is not the fact that a single instance of the class lives +// at runtime. +// +// Hence to fix matches fo this rule, creates the single instance +// at the startup of the program, and pass it to all classes and methods +// that need to access it. +// +// If multiple singletons are identified, they actually form together a +// *program execution context*. Such context can be unified in a unique +// singleton context. Doing so will make it easier to propagate the +// context across the various program units. +//]]> + Don't assign static fields from instance methods + +warnif count > 0 +from f in Application.Fields where + f.IsStatic && + !f.IsLiteral && + !f.IsInitOnly && + !f.IsGeneratedByCompiler && + // Contract API define such a insideContractEvaluation static field + f.Name != "insideContractEvaluation" +let assignedBy = f.MethodsAssigningMe.Where(m => !m.IsStatic) +where assignedBy .Count() > 0 +select new { f, assignedBy } + +// +// Assigning static fields from instance methods leads to +// poorly maintainable and non-thread-safe code. +// +// More discussion on the topic can be found here: +// http://codebetter.com/patricksmacchia/2011/05/04/back-to-basics-usage-of-static-members/ +// + +// +// If the *static* field is just assigned once in the program +// lifetime, make sure to declare it as *readonly* and assign +// it inline, or from the static constructor. +// +// In *Object-Oriented-Programming* the natural artifact +// to hold states that can be modified is **instance fields**. +// +// Hence to fix violations of this rule, make sure to +// hold assignable states through *instance* fields, not +// through *static* fields. +//]]> + Avoid empty interfaces +warnif count > 0 from t in JustMyCode.Types where + t.IsInterface && + t.NbMethods == 0 +select new { t, t.TypesThatImplementMe } + +// +//Interfaces define members that provide +//a behavior or usage contract. +//The functionality that is described by the interface +//can be adopted by any type, regardless of where the type +//appears in the inheritance hierarchy. +//A type implements an interface by providing implementations +//for the members of the interface. +//An empty interface does not define any members. +//Therefore, it does not define a contract that can be implemented. +// +//If your design includes empty interfaces that types +//are expected to implement, you are probably using an interface +//as a marker or a way to identify a group of types. +//If this identification will occur at run time, +//the correct way to accomplish this is to use a custom attribute. +//Use the presence or absence of the attribute, +//or the properties of the attribute, to identify the target types. +//If the identification must occur at compile time, +//then it is acceptable to use an empty interface. +// + +// +//Remove the interface or add members to it. +//If the empty interface is being used to label a set of types, +//replace the interface with a custom attribute. +//]]> + Avoid types initialization cycles +warnif count > 0 + +// Types initialization cycle can only happen between types of an assembly. +from assembly in Application.Assemblies + +let cctorSuspects = assembly.ChildMethods.Where( + m => m.IsClassConstructor && + // Optimization: types involved in a type cycle necessarily don't have type level. + m.ParentType.Level == null) + +where cctorSuspects.Count() > 1 +let typesSuspects = cctorSuspects.ParentTypes().ToHashSet() + +// +// dicoTmp associates to each type suspect T, a set of types from typesSuspects +// that contains at least a method or a field used directly or indirectly by the cctor of T. +// +let dicoTmp = cctorSuspects.ToDictionary( + cctor => cctor.ParentType, + cctor => ((IMember)cctor).ToEnumerable().FillIterative( + members => from m in members + from mUsed in (m is IMethod) ? (m as IMethod).MembersUsed : new IMember[0] + where mUsed.ParentAssembly == assembly + select mUsed) + .DefinitionDomain + .Select(m => m.ParentType) // Don't need .Distinct() here, because of ToHashSet() below. + .Except(cctor.ParentType) + .Intersect(typesSuspects) + .ToHashSet() +) + +// +// dico associates to each type suspect T, the set of types initialized (directly or indirectly) +// by the initialization of T. This second step is needed, because if a cctor of a type T1 +// calls a member of a type T2, not only the cctor of T1 triggers the initialization of T2, +// but also it triggers the initialization of all types that are initialized by T2 initialization. +// +let dico = typesSuspects.Where(t => dicoTmp[t].Count() > 0).ToDictionary( + typeSuspect => typeSuspect, + typeSuspect => typeSuspect.ToEnumerable().FillIterative( + types => from t in types + from tUsed in dicoTmp[t] + select tUsed) + .DefinitionDomain + .Except(typeSuspect) + .ToHashSet() +) + + +// +// Now that dico is prepared, detect the cctor cycles +// +from t in dico.Keys + + // Thanks to the work done to build dico, it is now pretty easy + // to spot types involved in an initialization cyle with t! + let usersAndUseds = from tTmp in dico[t] + where dico.ContainsKey(tTmp) && dico[tTmp].Contains(t) + select tTmp + where usersAndUseds.Count() > 0 + + // Here we've found type(s) both using and used by the suspect type. + // A cycle involving the type t is found! + let typeInitCycle = usersAndUseds.Append(t) + + + // Compute methodsCalled and fieldsUsed, useful to explore + // how a cctor involved in a type initialization cycle, triggers other type initialization. + let methodsCalledDepth = assembly.ChildMethods.DepthOfIsUsedBy(t.ClassConstructor) + let fieldsUsedDepth = assembly.ChildFields.DepthOfIsUsedBy(t.ClassConstructor) + + let methodsCalled = methodsCalledDepth.DefinitionDomain.OrderBy(m => methodsCalledDepth[m]).ToArray() + let fieldsUsed = fieldsUsedDepth.DefinitionDomain.OrderBy(f => fieldsUsedDepth[f]).ToArray() + +// Use the tick box to: Group cctors methods By parent types +select new { t.ClassConstructor, + cctorsCycle= typeInitCycle.Select(tTmp => tTmp.ClassConstructor), + + // methodsCalled and fieldsUsed are members used directly and indirectly by the cctor. + // Export these members to the dependency graph (right click the cell Export/Append … to the Graph) + // and see how the cctor trigger the initialization of other types + methodsCalled, + fieldsUsed +} + +// +// The *class constructor* (also called *static constructor*, and named *cctor* in IL code) +// of a type, if any, is executed by the CLR at runtime, the first time the type is used. +// A *cctor* doesn't need to be explicitly declared in C# or VB.NET, to exist in compiled IL code. +// Having a static field inline initialization is enough to have +// the *cctor* implicitly declared in the parent class or structure. +// +// If the *cctor* of a type *t1* is using the type *t2* and if the *cctor* of *t2* is using *t1*, +// some type initialization unexpected and hard-to-diagnose buggy behavior can occur. +// Such a cyclic chain of initialization is not necessarily limited to two types +// and can embrace *N* types in the general case. +// More information on types initialization cycles can be found here: +// http://codeblog.jonskeet.uk/2012/04/07/type-initializer-circular-dependencies/ +// +// The present code rule enumerates types initialization cycles. +// Some *false positives* can appear if some lambda expressions are defined +// in *cctors* or in methods called by *cctors*. In such situation, this rule +// considers these lambda expressions as executed at type initialization time, +// while it is not necessarily the case. +// + +// +// Types initialization cycles create confusion and unexpected behaviors. +// If several states hold by several classes must be initialized during the first +// access of any of those classes, a better design option is to create a dedicated +// class whose responsibility is to initialize and hold all these states. +//]]> + + + Avoid custom delegates +warnif count > 0 +from t in Application.Types where t.IsDelegate + +let invokeMethod = (from m in t.Methods where m.SimpleName == "Invoke" select m).Single() +let signature1 = invokeMethod.Name.Substring( + invokeMethod.SimpleName.Length, + invokeMethod.Name.Length - invokeMethod.SimpleName.Length) + +// 'ref' and 'out' parameters cannot be supported +where !signature1.Contains("&") + +let signature2 = signature1.Replace("(","<").Replace(")",">") +let signature3 = signature2 == "<>" ? "" : signature2 +let resultTypeName = invokeMethod.ReturnType == null ? "????" : + invokeMethod.ReturnType.FullName == "System.Void" ? "" : + invokeMethod.ReturnType.Name +let replaceWith = + resultTypeName == "Boolean" && invokeMethod.NbParameters == 1 ? + "Predicate" + signature3 : resultTypeName == "" ? + "Action" + signature3 : invokeMethod.NbParameters ==0 ? + "Func<" + resultTypeName + ">" : + "Func" + signature3.Replace(">", "," + resultTypeName + ">") + +select new { t, replaceWith } + +// +// Generic delegates sould be preferred over custom delegates. +// Generic delegates are: +// +// • *Action<…>* to represent any method with *void* return type. +// +// • *Func<…>* to represent any method with a return type. The last +// generic argument is the return type of the prototyped methods. +// +// • *Predicate* to represent any method that takes an instance +// of *T* and that returns a *boolean*. +// +// • Expression<…> that represents function definitions that can be +// compiled and subsequently invoked at runtime but can also be +// serialized and passed to remote processes. +// +// Thanks to generic delegates, not only the code using these custom +// delegates will become clearer, but you'll be relieved from the +// maintenance of these delegate types. +// +// Notice that delegates that are consumed by *DllImport* extern methods +// must not be converted, else this could provoke marshalling issues. +// + +// +// Remove custom delegates and replace them with generic +// delegates shown in the **replaceWith** column. +//]]> + Types with disposable instance fields must be disposable + +warnif count > 0 + +// Several IDisposable types can be found if several .NET Fx are referenced. +let iDisposables = ThirdParty.Types.WithFullName("System.IDisposable") +where iDisposables.Any() // in case the code base doesn't use at all System.IDisposable + +from t in Application.Types.Except(Application.Types.ThatImplementAny(iDisposables)) +where !t.IsGeneratedByCompiler + +let instanceFieldsDisposable = + t.InstanceFields.Where(f => f.FieldType != null && + f.FieldType.InterfacesImplemented.Intersect(iDisposables).Any()) + +where instanceFieldsDisposable.Any() +select new { t, instanceFieldsDisposable } + +// +// This rule warns when a class declares and implements an instance field that +// is a *System.IDisposable* type and the class does not implement *IDisposable*. +// +// A class implements the *IDisposable* interface to dispose of unmanaged resources +// that it owns. An instance field that is an *IDisposable* type indicates that +// the field owns an unmanaged resource. A class that declares an *IDisposable* +// field indirectly owns an unmanaged resource and should implement the +// *IDisposable* interface. If the class does not directly own any unmanaged +// resources, it should not implement a finalizer. +// + +// +// To fix a violation of this rule, implement *IDisposable* and from the +// *IDisposable.Dispose()* method call the *Dispose()* method of the field(s). +//]]> + Disposable types with unmanaged resources should declare finalizer + +// This default rule is disabled by default, +// see in the rule description (below) why. +// warnif count > 0 + +// Several IDisposable type can be found if several .NET Fx are referenced. +let iDisposables = ThirdParty.Types.WithFullName("System.IDisposable") +where iDisposables.Any() // in case the code base doesn't use at all System.IDisposable + +let disposableTypes = Application.Types.ThatImplementAny(iDisposables) +let unmanagedResourcesFields = disposableTypes.ChildFields().Where(f => + !f.IsStatic && + f.FieldType != null && + f.FieldType.FullName.EqualsAny( + "System.IntPtr", + "System.UIntPtr", + "System.Runtime.InteropServices.HandleRef")).ToHashSet() +let disposableTypesWithUnmanagedResource = unmanagedResourcesFields.ParentTypes() + +from t in disposableTypesWithUnmanagedResource +where !t.HasFinalizer +let unmanagedResourcesTypeFields = unmanagedResourcesFields.Intersect(t.InstanceFields) +select new { t, unmanagedResourcesTypeFields } + +// +//A type that implements *System.IDisposable*, +//and has fields that suggest the use of unmanaged resources, +//does not implement a finalizer as described by *Object.Finalize()*. +//A violation of this rule is reported +//if the disposable type contains fields of the following types: +// +// • *System.IntPtr* +// +// • *System.UIntPtr* +// +// • *System.Runtime.InteropServices.HandleRef* +// +// Notice that this default rule is disabled by default, +// because it typically reports *false positive* for classes +// that just hold some references to managed resources, +// without the responsibility to dispose them. +// +// To enable this rule just uncomment *warnif count > 0*. +// + +// +//To fix a violation of this rule, +//implement a finalizer that calls your *Dispose()* method. +//]]> + Methods that create disposable object(s) and that don't call Dispose() + +// Uncomment this to transform this code query into a code rule. +// warnif count > 0 + +// Several IDisposable types can be found if several .NET Fx are referenced. +let iDisposables = ThirdParty.Types.WithFullName("System.IDisposable") +where iDisposables.Any() // in case the code base doesn't use at all System.IDisposable + +// Build sequences of disposableTypes and disposeMethods +let disposableTypes = Types.ThatImplementAny(iDisposables).Concat(iDisposables) +let disposeMethods = disposableTypes.ChildMethods().WithName("Dispose()").ToHashSet() + + +// -> You can refine this code query by assigning to disposableTypesToLookAfter something like: +// disposableTypes.WithFullNameIn("Namespace.TypeName1", "Namespace.TypeName2", ...) +let disposableTypesToLookAfter = disposableTypes + + +// -> You can refine this code query by assigning to methodsToLookAfter something like: +// Application.Assemblies.WithNameLike("Asm").ChildMethods() +let methodsToLookAfter = Application.Methods + + +// Enumerate methods that create any disposable type, without calling Dispose() +from m in methodsToLookAfter.ThatCreateAny(disposableTypesToLookAfter ) + +where !m.MethodsCalled.Intersect(disposeMethods).Any() +select new { + m, + disposableObjectsCreated = disposableTypes.Where(t => m.CreateA(t)), + m.MethodsCalled +} + +// +// This code query enumerates methods that create one or several disposable object(s), +// without calling any Dispose() method. +// +// This code query is not a code rule because it is acceptable to do so, +// as long as disposable objects are disposed somewhere else. +// +// This code query is designed to be be easily refactored +// to look after only specific disposable types, or specific caller methods. +// +// You can then refactor this code query to adapt it to your needs and transform it into a code rule. +//]]> + Classes that are candidate to be turned into structures + +warnif count > 0 from t in JustMyCode.Types where + t.IsClass && + !t.IsGeneratedByCompiler && + !t.IsStatic && + t.SizeOfInst > 0 && + t.SizeOfInst <= 16 && // Structure instance must not be too big, + // else it degrades performance. + + t.NbChildren == 0 && // Must not have children + + // Must not implement interfaces to avoid boxing mismatch + // when structures implements interfaces. + t.InterfacesImplemented.Count() == 0 && + + // Must derive directly from System.Object + t.DepthOfDeriveFrom("System.Object".AllowNoMatch()) == 1 + + // && t.IsSealed <-- You might want to add this condition + // to restraint the set. + // && t.IsImmutable <-- Structures should be immutable type. + // && t.!IsPublic <-- You might want to add this condition if + // you are developping a framework with classes + // that are intended to be sub-classed by + // your clients. + +select new { t, t.SizeOfInst, t.InstanceFields } + +// +// *Int32*, *Double*, *Char* or *Boolean* are structures and not classes. +// Structure are particularly suited to implement **lightweight value**s. +// Hence a class is candidate to be turned into a structure +// when its instances are *lightweight values*. +// +// This is a matter of *performance*. It is expected that a program +// works with plenty of *short lived lightweight values*. +// In such situation, the advantage of using *struct* instead of +// *class*, (in other words, the advantage of using *values* instead +// of *objects*), is that *values* are not managed by the garbage collector. +// This means that values are cheaper to deal with. +// +// This rule matches classes that looks like being *lightweight values*. +// The characterization of such class is: +// +// • Its instances have a small *memory footprint* (at most 16 bytes) +// +// • Its instances have a memory footprint greater than zero +// (i.e the class has at least one instance field). +// +// • It implements no interfaces. +// +// • It has no derived classes. +// +// • It derives directly from *System.Object*. +// +// This rule doesn't take account if instances of matched +// classes are numerous *short-lived* objects. +// These criterions are just indications. Only you can decide if it is +// *performance wise* to transform a class into a structure. +// +// A related case-study of using *class* or *struct* for *Tuple<…>* generic +// types can be found here: +// http://stackoverflow.com/questions/2410710/why-is-the-new-tuple-type-in-net-4-0-a-reference-type-class-and-not-a-value-t +// + +// +// Just use the keyword *struct* instead of the keyword *class*. +// +// **CAUTION:** Before applying this rule, make sure to understand +// the **deep implications** of transforming a class into a structure. +// http://msdn.microsoft.com/en-us/library/aa664471(v=vs.71).aspx +//]]> + Avoid namespaces with few types +warnif count > 0 from n in JustMyCode.Namespaces +where n.Name.Length > 0 // Don't match anonymous namespaces +let types = n.ChildTypes.Where(t => !t.IsGeneratedByCompiler) +where + types.Count() < 5 + orderby types.Count() ascending +select new { n, types } + +// +// This rule warns about namespaces other than the global namespace +// that contain less than five types. +// +// Make sure that each of your namespaces has a logical organization +// and that a valid reason exists to put types in a sparsely +// populated namespace. +// +// Namespaces should contain types that are used together in most +// scenarios. When their applications are mutually exclusive, +// types should be located in separate namespaces. For example, +// the *System.Web.UI* namespace contains types that are used +// in Web applications, and the *System.Windows.Forms* namespace +// contains types that are used in Windows-based applications. +// Even though both namespaces have types that control aspects +// of the user interface, these types are not designed for +// use in the same application. Therefore, they are located in +// separate namespaces. +// +// Careful namespace organization can also be helpful because +// it increases the discoverability of a feature. By examining the +// namespace hierarchy, library consumers should be able to locate +// the types that implement a feature. +// + +// +// To fix a violation of this rule, try to combine namespaces +//that contain just a few types into a single namespace. +//]]> + Nested types should not be visible +warnif count > 0 from t in JustMyCode.Types where + t.IsNested && + !t.IsGeneratedByCompiler && + !t.IsPrivate +select new { + t, + t.Visibility +} + +// +// This rule warns about nested types not declared as private. +// +// A nested type is a type declared within the scope of another +// type. Nested types are useful for encapsulating private +// implementation details of the containing type. Used +// for this purpose, nested types should not be externally visible. +// +// Do not use externally visible nested types for logical +// grouping or to avoid name collisions; instead use namespaces. +// +// Nested types include the notion of member accessibility, +// which some programmers do not understand clearly. +// +// Protected types can be used in subclasses and nested types +// in advanced customization scenarios. +// + +// +// If you do not intend the nested type to be externally visible, +// change the type's accessibility. +// +// Otherwise, remove the nested type from its parent and make it +// *non-nested*. +// +// If the purpose of the nesting is to group some nested types, +// use a namespace to create the hierarchy instead. +//]]> + Declare types in namespaces +warnif count > 0 from n in Application.Namespaces where + // If an anonymous namespace can be found, + // it means that it contains types outside of namespaces. + n.Name == "" + + // Eliminate anonymous namespaces that contains + // only generated types. + let childTypes = n.ChildTypes.Where(t => !t.IsGeneratedByCompiler) + where childTypes.Count() > 0 +select new { + n, + childTypes, + n.NbLinesOfCode } + +// +// Types are declared within namespaces to prevent name collisions, +// and as a way of organizing related types in an object hierarchy. +// +// Types outside any named namespace are in a *global +// namespace* that cannot be referenced in code. +// +// The *global namespace* has no name, hence it is qualified as +// being the *anonymous namespace*. +// +// This rule warns about *anonymous namespaces*. +// + +// +// To fix a violation of this rule, +// declare all types of all anonymous +// namespaces in some named namespaces. +//]]> + Empty static constructor can be discarded +warnif count > 0 from m in Application.Methods where + m.IsClassConstructor && + m.NbLinesOfCode == 0 +select m + +// +// The *class constructor* (also called *static constructor*, and named *cctor* in IL code) +// of a type, if any, is executed by the CLR at runtime, just before the first time the type is used. +// +// This rule warns about the declarations of *static constructors* +// that don't contain any lines of code. Such *cctors* are useless +// and can be safely removed. +// + +// +// Remove matched empty *static constructors*. +//]]> + Instances size shouldn't be too big +warnif count > 0 from t in JustMyCode.Types where + t.SizeOfInst > 64 + orderby t.SizeOfInst descending +select new { t, t.SizeOfInst, t.InstanceFields } + +// +// Types where *SizeOfInst > 64* might degrade performance +// if many instances are created at runtime. +// They can also be hard to maintain. +// +// Notice that a class with a large *SizeOfInst* value +// doesn't necessarily have a lot of instance fields. +// It might derive from a class with a large *SizeOfInst* value. +// +// See the definition of the *SizeOfInst* metric here +// http://www.ndepend.com/docs/code-metrics#SizeOfInst +// + +// +// A type with a large *SizeOInst* value hold *directly* +// a lot of data. Typically, you can group this data into +// smaller types that can then be composed. +//]]> + Boxing/unboxing should be avoided +warnif percentage > 5 from m in Application.Methods where + m.IsUsingBoxing || + m.IsUsingUnboxing +select new { m, m.NbLinesOfCode, m.IsUsingBoxing, m.IsUsingUnboxing } + +// +// *Boxing* is the process of converting a value type to the type +// *object* or to any interface type implemented by this value +// type. When the CLR boxes a value type, it wraps the value +// inside a *System.Object* and stores it on the managed heap. +// +// *Unboxing* extracts the value type from the object. Boxing +// is implicit; unboxing is explicit. +// +// The concept of boxing and unboxing underlies the C# unified +// view of the type system in which a value of any type can +// be treated as an object. More about *boxing* and *unboxing* +// here: https://msdn.microsoft.com/en-us/library/yz2be5wk.aspx +// +// This rule warns when more than 5% of methods in a code base +// are using *boxing* or *unboxing*. Because *boxing* and +// *unboxing* come with a performance penalty and can be often +// avoided. +// + +// +// Thanks to .NET generic, and especially thanks to +// generic collections, *boxing* and *unboxing* should +// be rarely used. Hence in most situations the code can +// be refactored to avoid relying on *boxing* and *unboxing*. +// See for example: +// http://stackoverflow.com/questions/4403055/boxing-unboxing-and-generics +// +// With a performance profiler, indentify methods that consume +// a lot of CPU time. If such method uses *boxing* or +// *unboxing*, especially in a **loop**, make sure to refactor it. +//]]> + Attribute classes should be sealed +warnif count > 0 from t in Application.Types where + t.IsAttributeClass && + !t.IsSealed && + !t.IsAbstract && + t.IsPublic +select new { t, t.NbLinesOfCode } + +// +// The .NET Framework class library provides methods +// for retrieving custom attributes. By default, +// these methods search the attribute inheritance +// hierarchy; for example +// *System.Attribute.GetCustomAttribute()* +// searches for the specified attribute type, or any +// attribute type that extends the specified attribute +// type. +// +// Sealing the attribute eliminates the search +// through the inheritance hierarchy, and can improve +// performance. +// + +// +// To fix a violation of this rule, seal the attribute +// type or make it abstract. +//]]> + Don't use obsolete types, methods or fields +warnif count > 0 +let obsoleteTypes = Types.Where(t => t.IsObsolete) +let obsoleteMethods = Methods.Where(m => m.IsObsolete).ToHashSet() +let obsoleteFields = Fields.Where(f => f.IsObsolete) + +from m in JustMyCode.Methods.UsingAny(obsoleteTypes).Union( + JustMyCode.Methods.UsingAny(obsoleteMethods)).Union( + JustMyCode.Methods.UsingAny(obsoleteFields)) +let obsoleteTypesUsed = obsoleteTypes.UsedBy(m) + +// Optimization: MethodsCalled + Intersect() is faster than using obsoleteMethods.UsedBy() +let obsoleteMethodsUsed = m.MethodsCalled.Intersect(obsoleteMethods) +let obsoleteFieldsUsed = obsoleteFields.UsedBy(m) +select new { m, obsoleteTypesUsed, obsoleteMethodsUsed, obsoleteFieldsUsed } + +// +// The attribute *System.ObsoleteAttribute* is used to tag +// types, methods or fields of an API that clients shouldn't +// use because these code elements will be removed sooner +// or later. +// +// This rule warns about methods that use a type, a method +// or a field, tagged with *System.ObsoleteAttribute*. +// + +// +// Typically when a code element is tagged with +// *System.ObsoleteAttribute*, a *workaround message* +// is provided to clients. +// +// This *workaround message* will tell you what to do +// to avoid using the obsolete code element. +//]]> + Don't forget to implement methods that throw NotImplementedException +warnif count > 0 +from m in Application.Methods +where m.CreateA("System.NotImplementedException".AllowNoMatch()) +select m + +// +// The exception *NotImplementedException* is used to declare +// a method *stub* that can be invoked, and defer the +// development of the method implementation. +// +// This exception is especially useful when doing **TDD** +// (*Test Driven Development*) when tests are written first. +// This way tests fail until the implementation is written. +// +// Hence using *NotImplementedException* is a *temporary* +// facility, and before releasing, will come a time when +// this exception shouldn't be used anywhere in code. +// +// This rule warns about method still using +// *NotImplementedException*. +// + +// +// Investigate first why *NotImplementedException* is still +// used. Either the method must be implemented, or +// the method stub is not used and can be safely removed. +//]]> + Override equals and operator equals on value types +warnif count > 0 +from t in Application.Types where + t.IsStructure +let equalsMethod = t.InstanceMethods.Where(m0 => m0.Name == "Equals(Object)").SingleOrDefault() +where equalsMethod == null +select t + +// +// For value types, the inherited implementation of *Equals()* uses +// the Reflection library, and compares the contents of all +// fields. Reflection is computationally expensive, and comparing +// every field for equality might be unnecessary. +// +// If you expect users to compare or sort instances, or use them +// as hash table keys, your value type should implement *Equals()*. +// In C# and VB.NET, you should also provide an implementation of +// the equality and inequality operators. +// + +// +// To fix a violation of this rule, provide an implementation +// of *Equals()* and implement the equality and inequality operators. +//]]> + + + Avoid namespaces mutually dependent +warnif count > 0 + +// Optimization: restraint application assemblies set +// If some namespaces are mutually dependent +// - They must be declared in the same assembly +// - The parent assembly must ContainsNamespaceDependencyCycle +from assembly in Application.Assemblies.Where(a => a.ContainsNamespaceDependencyCycle != null && a.ContainsNamespaceDependencyCycle.Value) + +// hashset is used to avoid reporting both A <-> B and B <-> A +let hashset = new HashSet() + +// Optimization: restraint namespaces set +// If a namespace doesn't have a Level value, it must be in a dependency cycle +// or it must be using directly or indirectly a dependency cycle. +let namespacesSuspect = assembly.ChildNamespaces.Where(n => n.Level == null) + +from nA in namespacesSuspect + +// Select namespaces mutually dependent with nA +let unused = hashset.Add(nA) // Populate hashset +let namespacesMutuallyDependentWith_nA = nA.NamespacesUsed.Using(nA) + .Except(hashset) // <-- avoid reporting both A <-> B and B <-> A +where namespacesMutuallyDependentWith_nA.Count() > 0 + +from nB in namespacesMutuallyDependentWith_nA + +// nA and nB are mutually dependent +// First select the one that shouldn't use the other. +// The first namespace is inferred from the fact that it is using less types of the second. +let typesOfBUsedByA = nB.ChildTypes.UsedBy(nA) +let typesOfAUsedByB = nA.ChildTypes.UsedBy(nB) +let first = (typesOfBUsedByA.Count() > typesOfAUsedByB.Count()) ? nB : nA +let second = (first == nA) ? nB : nA +let typesOfFirstUsedBySecond = (first == nA) ? typesOfAUsedByB : typesOfBUsedByA +let typesOfSecondUsedByFirst = (first == nA) ? typesOfBUsedByA : typesOfAUsedByB + +select new { + first, + shouldntUseNamespace = second, + OK_typesOfFirstUsedBySecond = typesOfFirstUsedBySecond, + KO_typesOfSecondUsedByFirst = typesOfSecondUsedByFirst } +// +// This rule lists pair of namespace mutually dependent. +// +// The pair *{ first, second }* is formatted to show +// **first namespace shouldn't use the second namespace**. +// The *first/second* order is inferred from the number of types +// used by each other. The first namespace is using fewer types of +// the second. In general this indicates that the first namespace +// should be at a *lower-level* in the architecture than +// the second. +// +// Following this rule is useful to avoid **namespaces dependency +// cycles**. This will get the code architecture close to a +// *layered architecture*, where *low-level* code is not allowed +// to use *high-level* code. +// +// In other words, abiding by this rule will help significantly +// getting rid of what is often called **spagetthi code: +// Entangled code that is not properly layered and structured**. +// +// More on this in our white books relative to partitioning code. +// http://www.ndepend.com/docs/white-books +// + +// +// Refactor the code to make sure that *the first namespace doesn't +// use the second namespace*. To get started, first explore the +// coupling between two namespaces mutually dependent: +// +// 1) from the *right-click menu* export the first namespace to +// the vertical header of the dependency matrix. +// +// 2) export the second namespace to the horizontal header of +// the dependency matrix. +// +// 3) double-click the black matrix cell (it is black because of +// mutual dependency). +// +// 4) in the matrix command bar, click the button: +// *Remove empty Row(s) and Column(s)*. +// +// At this point, the dependency matrix shows types involved +// into the coupling. +// +// Some refactoring that helps uncouple two namespaces +// mutually dependent include: +// +// • Moving one or several types from the *low-level* namespaces +// to the *high-level* one, or do the opposite. +// +// • Use *Inversion of Control (IoC)*: +// http://en.wikipedia.org/wiki/Inversion_of_control +// This can consist in creating new interfaces in the +// *low-level* namespace, implemented by classes +// in the *high-level* namespace. This way *low-level* +// code can consume *high-level* code through interfaces, +// without using directly *high-level* implementations. +// Interfaces can be passed to *low-level* code through +// the *high-level* namespace code, or through even +// higher-level code. In related documentations +// you can see these interfaces named as *callbacks*, +// and the overall pattern is also known as +// *Dependency Injection (DI)*: +// http://en.wikipedia.org/wiki/Dependency_injection +//P + ]]> + Avoid namespaces dependency cycles +warnif count > 0 + +// Optimization: restraint application assemblies set +// If some namespaces are mutually dependent +// - They must be declared in the same assembly +// - The parent assembly must ContainsNamespaceDependencyCycle +from assembly in Application.Assemblies + .Where(a => a.ContainsNamespaceDependencyCycle != null && + a.ContainsNamespaceDependencyCycle.Value) + +// Optimization: restraint namespaces set +// A namespace involved in a cycle necessarily have a null Level. +let namespacesSuspect = assembly.ChildNamespaces.Where(n => n.Level == null) + +// hashset is used to avoid iterating again on namespaces already caught in a cycle. +let hashset = new HashSet() + + +from suspect in namespacesSuspect + // By commenting in this line, the query matches all namespaces involved in a cycle. + where !hashset.Contains(suspect) + + // Define 2 code metrics + // - Namespaces depth of is using indirectly the suspect namespace. + // - Namespaces depth of is used by the suspect namespace indirectly. + // Note: for direct usage the depth is equal to 1. + let namespacesUserDepth = namespacesSuspect.DepthOfIsUsing(suspect) + let namespacesUsedDepth = namespacesSuspect.DepthOfIsUsedBy(suspect) + + // Select namespaces that are both using and used by namespaceSuspect + let usersAndUsed = from n in namespacesSuspect where + namespacesUserDepth[n] > 0 && + namespacesUsedDepth[n] > 0 + select n + + where usersAndUsed.Count() > 0 + + // Here we've found namespace(s) both using and used by the suspect namespace. + // A cycle involving the suspect namespace is found! + let cycle = usersAndUsed.Append(suspect) + + // Fill hashset with namespaces in the cycle. + // .ToArray() is needed to force the iterating process. + let unused1 = (from n in cycle let unused2 = hashset.Add(n) select n).ToArray() + +select new { suspect, cycle } + +// +// This rule lists all *application namespace dependency cycles*. +// Each row shows a different cycle, indexed with one of the namespace entangled +// in the cycle. +// +// To browse a cycle on the dependency graph or the dependency matrix, right click +// a cycle cell and export the matched namespaces to the dependency graph or matrix. +// +// In the matrix, dependency cycles are represented with red squares and black cells. +// To easily browse dependency cycles, the dependency matrix comes with an option: +// *Display Direct and Indirect Dependencies* +// +// Read our white books relative to partitioning code, +// to know more about namespaces dependency cycles, and why avoiding them +// is a *simple yet efficient* solution to clean the architecture of a code base. +// http://www.ndepend.com/docs/white-books +// + +// +// Removing first pairs of *mutually dependent namespaces* will eliminate +// most *namespaces dependency cycles*. This is why we suggest +// focusing on matches of the default rule +// **Avoid namespaces mutually dependent** before dealing +// with the present rule. +// +// Once solving all *mutually dependent namespaces*, remaining cycles +// matched by the present rule necessarily involve 3 or more namespaces +// like in: *A is using B is using C is using A*. +// Such cycle can be broken by identifying which namespace should +// be at the *lower-level*. For example if B should be at the +// *lower-level*, then it means C should be at the *higher-level* +// and to break the cycle, you just have to remove the dependency +// from B to C, with a pattern described in the *HowToFix* section +// of the rule *Avoid namespaces mutually dependent*. +//]]> + Avoid partitioning the code base through many small library Assemblies +warnif count > 10 +from a in Application.Assemblies where + ( a.NbLinesOfCode < 1000 || + a.NbILInstructions < 7000 ) && + a.FilePath.FileExtension.ToLower() == ".dll" +select new { a, a.NbLinesOfCode, a.NbILInstructions } + +// +// Each .NET Assembly compiled represents one or several physical file(s). +// Having too many library .NET Assemblies is a symptom of +// considering **physical** .NET Assemblies as **logical** components. +// +// We advise having less, and bigger, .NET Assemblies +// and using the concept of namespaces to define logical components. +// Benefits are: +// +// • Faster compilation time. +// +// • Faster startup time for your program. +// +// • Easier deployment thanks to less files to manage. +// +// • If you are developing a Framework, +// less .NET assemblies to reference and manage for your clients. +// + +// +// Consider using the *physical* concept of assemblies for physical needs +// only. +// +// Our white book about **Partitioning code base through .NET assemblies +// and Visual Studio projects** explains in details valid and invalid +// reasons to use assemblies. +// Download it here: +// http://www.ndepend.com/Res/NDependWhiteBook_Assembly.pdf +//]]> + UI layer shouldn't use directly DB types + +warnif count > 0 + +// UI layer is made of types in namespaces using a UI framework +let uiTypes = Application.Namespaces.UsingAny(Assemblies.WithNameIn("PresentationFramework", "System.Windows", "System.Windows.Forms", "System.Web")).ChildTypes() + +// You can easily customize this line to define what are DB types. +let dbTypes = ThirdParty.Assemblies.WithNameIn("System.Data", "EntityFramework", "NHibernate").ChildTypes() + // Ideally even DataSet and associated, usage should be forbidden from UI layer: + // http://stackoverflow.com/questions/1708690/is-list-better-than-dataset-for-ui-layer-in-asp-net + .Except(ThirdParty.Types.WithNameIn("DataSet", "DataTable", "DataRow")) + +from uiType in uiTypes.UsingAny(dbTypes) +let dbTypesUsed = dbTypes.Intersect(uiType.TypesUsed) +select new { uiType, dbTypesUsed } + + +// +// This rule is more a *sample rule to adapt to your need*, +// than a rigid rule you should abide by. It shows how to +// define code layers in a rule and how to be warned about +// layers dependencies violations. +// +// This rule first defines the UI layer and the DB framework +// layer. Second it checks if any UI layer type is using +// directly any DB framework layer type. +// +// • The **DB framework layer** is defined as the set of *third-party* +// types in the framework *ADO.NET*, *EntityFramework*, +// *NHibernate* types, that the application is consuming. +// It is easy to append and suppress any DB framework. +// +// • The UI layer (**User Interface Layer**) is defined as the +// set of types that use *WPF*, *Windows Form*, *ASP.NET*. +// +// *UI using directly DB frameworks* is generally considered +// as *poor design* because DB frameworks accesses should be +// a concept hidden to UI, encapsulated into a **dedicated +// Data Access Layer (DAL)**. +// + +// +// This rule lists precisely which UI type uses which +// DB framework type. Instead of fixing matches one by one, +// first imagine how DB framework accesses could be +// encapsulated into a dedicated layer. +//]]> + UI layer shouldn't use directly DAL layer + +warnif count > 0 + +// UI layer is made of types in namespaces using a UI framework +let uiTypes = Application.Namespaces.UsingAny(Assemblies.WithNameIn("PresentationFramework", "System.Windows", "System.Windows.Forms", "System.Web")).ChildTypes() + +// Exclude commonly used DataSet and associated, from ADO.Net types +// You can easily customize this line to define what are DB types. +let dbTypes = ThirdParty.Assemblies.WithNameIn("System.Data", "EntityFramework", "NHibernate").ChildTypes() + .Except(ThirdParty.Types.WithNameIn("DataSet", "DataTable", "DataRow")) + +// DAL layer is made of types in namespaces using a DB framework +// .ToHashSet() results to faster execution of dalTypes.Intersect(uiType.TypesIUse). +let dalTypes = Application.Namespaces.UsingAny(dbTypes).ChildTypes().ToHashSet() + +from uiType in uiTypes.UsingAny(dalTypes) +let dalTypesUsed = dalTypes.Intersect(uiType.TypesUsed) +select new { + uiType, + // if dalTypesUsed is empty, it means that the uiType is part of the DAL + dalTypesUsed +} + +// +// This rule is more a *sample rule to adapt to your need*, +// than a rigid rule you should abide by. It shows how to +// define code layers in a rule and how to be warned about +// layers dependencies violations. +// +// This rule first defines the UI layer and the DAL layer. +// Second it checks if any UI layer type is using directly +// any DAL layer type. +// +// • The DB layer (the DAL, **Data Access Layer**) is defined as +// the set of types of the application that use *ADO.NET*, +// *EntityFramework*, *NHibernate* types. It is easy to append +// and suppress any DB framework. +// +// • The UI layer (**User Interface Layer**) is defined as the +// set of types that use *WPF*, *Windows Form*, *ASP.NET*. +// +// *UI using directly DAL* is generally considered as *poor +// design* because DAL accesses should be a concept +// hidden to UI, encapsulated into an **intermediary domain +// logic**. +// + +// +// This rule lists precisely which UI type uses which DAL type. +// +// More about this particular design topic here: +// http://www.kenneth-truyers.net/2013/05/12/the-n-layer-myth-and-basic-dependency-injection/ +//]]> + Assemblies with poor cohesion (RelationalCohesion) +warnif count > 0 from a in Application.Assemblies + +// Build the types list on which we want to check cohesion +// This is the assembly 'a' type, minus enumeration +// and types generated by the compiler. +let types = a.ChildTypes.Where( + t => !t.IsGeneratedByCompiler && + !t.IsEnumeration) + // Absolutly need ToHashet() to have fast Intersect() calls below. + .ToHashSet() + +// Relational Cohesion metrics is relevant only if there are several types +where types.LongCount()> 20 + +// R is the total number of relationship between types of the assemblies. +let R = types.Sum(t => t.TypesUsed.Intersect(types).Count()) + +// Relational Cohesion formula +let relationalCohesion = (float)R / types.Count +where + + (relationalCohesion < 1.5 || + relationalCohesion > 4.0) +select new { + a, + a.ChildTypes, + relationalCohesion, + a.RelationalCohesion } + +// +// This rule computes the *Relational Cohesion* metric for +// the application assemblies, and warns about wrong values. +// +// The *Relational Cohesion* for an assembly, is the total number +// of relationship between types of the assemblies, divided +// by the number of types. In other words it is the average +// number of types in the assembly used by a type in the assembly. +// +// As classes inside an assembly should be strongly related, +// the cohesion should be high. On the other hand, a value +// which is too high may indicate over-coupling. A good range +// for *Relational Cohesion* is **1.5 to 4.0**. +// +// Notice that assemblies with less than 20 types are ignored. +// + +// +// Matches of this present rule might reveal either assemblies +// with specific coding constraints (like code generated that +// have particular structure) either issues in design. +// +// In the second case, large refactoring can be planned +// not to respect this rule in particular, but to increase +// the overall design and code maintainability. +//]]> + Namespaces with poor cohesion (RelationalCohesion) +warnif count > 0 from n in Application.Namespaces + +// Build the types list on which we want to check cohesion +// This is the namespace children types, minus enumerations +// and types generated by the compiler. +let types = n.ChildTypes.Where( + t => !t.IsGeneratedByCompiler && + !t.IsEnumeration) + // Absolutly need ToHashet() to have fast Intersect() calls below. + .ToHashSet() + +// Relational Cohesion metrics is relevant only if there are enough types +where types.LongCount() > 20 + +// R is the total number of relationship between types of the namespaces. +let R = types.Sum(t => t.TypesUsed.Intersect(types).Count()) + +// Relational Cohesion formula +let relationalCohesion = (float)R / types.Count +where + + (relationalCohesion < 1.5 || + relationalCohesion > 4.0) +select new { + n, + n.ChildTypes, + relationalCohesion +} + +// +// This rule computes the *Relational Cohesion* metric for +// the application namespaces, and warns about wrong values. +// +// The *Relational Cohesion* for a namespace, is the total number +// of relationship between types of the namespaces, divided +// by the number of types. In other words it is the average +// number of types in the namespace used by a type in the namespace. +// +// As classes inside an namespace should be strongly related, +// the cohesion should be high. On the other hand, a value +// which is too high may indicate over-coupling. A good range +// for *Relational Cohesion* is **1.5 to 4.0**. +// +// Notice that namespaces with less than 20 types are ignored. +// + +// +// Matches of this present rule might reveal either namespaces +// with specific coding constraints (like code generated that +// have particular structure) either issues in design. +// +// In the second case, refactoring sessions can be planned +// to increase the overall design and code maintainability. +//]]> + Assemblies that don't satisfy the Abstractness/Instability principle +warnif percentage > 15 from a in Application.Assemblies where + a.NormDistFromMainSeq > 0.7 + orderby a.NormDistFromMainSeq descending +select new { a, a.NormDistFromMainSeq } + +// +// The **Abstractness versus Instability Diagram** that is shown in the NDepend +// report helps to assess which assemblies are **potentially painful to maintain** +// (i.e concrete and stable) and which assemblies are **potentially useless** +// (i.e abstract and instable). +// +// • **Abstractness**: If an assembly contains many abstract types +// (i.e interfaces and abstract classes) and few concrete types, +// it is considered as abstract. +// +// • **Stability**: An assembly is considered stable if its types +// are used by a lot of types from other assemblies. In this context +// stable means *painful to modify*. +// +// From these metrics, we define the *perpendicular normalized distance of +// an assembly from the idealized line* **A + I = 1** (called *main sequence*). +// This metric is an indicator of the assembly's balance between abstractness +// and stability. We precise that the word *normalized* means that the range +// of values is [0.0 … 1.0]. +// +// This rule warns about assemblies with a *normalized distance* greater than +// than 0.7. +// +// This rules use the default code metric on assembly +// *Normalized Distance from the Main Sequence* explained here: +// http://www.ndepend.com/docs/code-metrics#DitFromMainSeq +// +// These concepts have been originally introduced by *Robert C. Martin* +// in 1994 in this paper: http://www.objectmentor.com/resources/articles/oodmetrc.pdf +// + +// +// Violations of this rule indicate assemblies with an improper +// *abstractness / stability* balance. +// +// • Either the assembly is *potentially painful to maintain* (i.e is massively +// used and contains mostly concrete types). This can be fixed by creating +// abstractions to avoid too high coupling with concrete implementations. +// +// • Either the assembly is *potentially useless* (i.e contains mostly +// abstractions and is not used enough). In such situation, the design +// must be reviewed to see if it can be enhanced. +//]]> + Example of custom rule to check for dependency +warnif count > 0 from a in Assemblies +where +a.IsUsing("Foo1.Foo2".AllowNoMatch().MatchNamespace()) && +(a.Name == @"Foo3") +select new { a, a.NbLinesOfCode } +// the assembly Foo3 +// shouldn't use directly +// the namespace Foo3.Foo4 +// because (TODO insert your reason) + +// +// This rule is a **sample rule** that shows how to +// check if a particular dependency exists or not, +// from a code element **A** to a code element **B**, +// **A** and **B** being an *Assembly*, a *Namespace*, a *Type*, +// a *Method* or a *Field*, **A** and **B** being not +// necessarily of same kind (i.e two Assemblies or +// two Namespaces…). +// +// Such rule can be generated: +// +// • by right clicking the cell in the *Dependency Matrix* +// with **B** in row and **A** in column, +// +// • or by right-clicking the concerned arrow in the *Dependency +// Graph* from **A** to **B**, +// +// and in both cases, click the menu +// **Generate a code rule that warns if this dependency exists** +// +// The generated rule will look like this one. +// It is now up to you to adapt this rule to check exactly +// your needs. +// + +// +// This is a *sample rule* there is nothing to fix *as is*. +//]]> + Higher cohesion - lower coupling + +let abstractNamespaces = JustMyCode.Namespaces.Where( + n => n.ChildTypes.Where(t => !t.IsInterface && !t.IsEnumeration && !t.IsDelegate).Count() == 0 +).ToHashSet() + +let concreteNamespaces = JustMyCode.Namespaces.Except(abstractNamespaces).ToHashSet() + +from n in concreteNamespaces +let namespacesUsed = n.NamespacesUsed.ExceptThirdParty() +let concreteNamespacesUsed = namespacesUsed.Except(abstractNamespaces) +let abstractNamespacesUsed = namespacesUsed.Except(concreteNamespaces) +select new { n, concreteNamespacesUsed , abstractNamespacesUsed } + +// +// It is deemed as a good software architecture practice to clearly separate +// *abstract* namespaces that contain only abstractions (interfaces, enumerations, delegates) +// from *concrete* namespaces, that contain classes and structures. +// +// Typically, the more concrete namespaces rely on abstract namespaces *only*, +// the more **Decoupled** is the architecture, and the more **Cohesive** are +// classes inside concrete namespaces. +// +// The present code query defines sets of abstract and concrete namespaces +// and show for each concrete namespaces, which concrete and abstract namespaces +// are used. +// + +// +// This query can be transformed into a code rule, depending if you wish to +// constraint your code structure *coupling / cohesion* ratio. +//]]> + + + API Breaking Changes: Types + +warnif count > 0 from t in codeBase.OlderVersion().Application.Types +where t.IsPubliclyVisible && + + // The type has been removed, it was not tagged as obsolete + // and its parent assembly hasn't been removed … + ( ( t.WasRemoved() && + !t.ParentAssembly.WasRemoved() && + !t.IsObsolete) || + + // … or the type is not publicly visible anymore + !t.WasRemoved() && !t.NewerVersion().IsPubliclyVisible) + +select new { + t, + NewVisibility = + (t.WasRemoved() ? " " : + t.NewerVersion().Visibility.ToString()) +} + +// +// This rule is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This rule warns if a type publicly visible in the *baseline*, +// is not publicly visible anymore or if it has been removed. +// Clients code using such type will be broken. +// + +// +// Make sure that public types that used to be presented to +// clients, still remain public now, and in the future. +// +// If a public type must really be removed, you can tag it +// with *System.ObsoleteAttribute* with a *workaround message* +// during a few public releases, until it gets removed definitely. +// Notice that this rule doesn't match types removed that were +// tagged as obsolete. +//]]> + API Breaking Changes: Methods +warnif count > 0 from m in codeBase.OlderVersion().Application.Methods +where m.IsPubliclyVisible && + + // The method has been removed, it was not tagged as obsolete + // and its parent type hasn't been removed … + ( ( m.WasRemoved() && + !m.ParentType.WasRemoved() && + !m.IsObsolete) || + + // … or the method is not publicly visible anymore + !m.WasRemoved() && !m.NewerVersion().IsPubliclyVisible) + +select new { + m, + NewVisibility = + (m.WasRemoved() ? " " : + m.NewerVersion().Visibility.ToString()) +} + +// +// This rule is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This rule warns if a method publicly visible in the *baseline*, +// is not publicly visible anymore or if it has been removed. +// Clients code using such method will be broken. +// + +// +// Make sure that public methods that used to be presented to +// clients, still remain public now, and in the future. +// +// If a public method must really be removed, you can tag it +// with *System.ObsoleteAttribute* with a *workaround message* +// during a few public releases, until it gets removed definitely. +// Notice that this rule doesn't match methods removed that were +// tagged as obsolete. +//]]> + API Breaking Changes: Fields +warnif count > 0 from f in codeBase.OlderVersion().Application.Fields +where f.IsPubliclyVisible && + + // The field has been removed, it was not tagged as obsolete + // and its parent type hasn't been removed … + ( ( f.WasRemoved() && + !f.ParentType.WasRemoved() && + !f.IsObsolete) || + + // … or the field is not publicly visible anymore + !f.WasRemoved() && !f.NewerVersion().IsPubliclyVisible) + +select new { + f, + NewVisibility = + (f.WasRemoved() ? " " : + f.NewerVersion().Visibility.ToString()) +} + +// +// This rule is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This rule warns if a field publicly visible in the *baseline*, +// is not publicly visible anymore or if it has been removed. +// Clients code using such field will be broken. +// + +// +// Make sure that public fields that used to be presented to +// clients, still remain public now, and in the future. +// +// If a public field must really be removed, you can tag it +// with *System.ObsoleteAttribute* with a *workaround message* +// during a few public releases, until it gets removed definitely. +// Notice that this rule doesn't match fields removed that were +// tagged as obsolete. +//]]> + API Breaking Changes: Interfaces and Abstract Classes +warnif count > 0 from tNewer in Application.Types where + (tNewer.IsInterface || tNewer.IsClass && tNewer.IsAbstract) && + tNewer.IsPubliclyVisible && + tNewer.IsPresentInBothBuilds() + +let tOlder = tNewer.OlderVersion() where tOlder.IsPubliclyVisible + +let methodsRemoved = tOlder.Methods.Where(m => m.IsAbstract && m.WasRemoved()) +let methodsAdded = tNewer.Methods.Where(m => m.IsAbstract && m.WasAdded()) + +where methodsAdded.Count() > 0 || methodsRemoved.Count() > 0 +select new { + tNewer, + methodsAdded, + methodsRemoved +} + +// +// This rule is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This rule warns if a publicly visible interface or abstract class +// has been changed and contains new abstract methods or +// if some abstract methods have been removed. +// +// Clients code that implement such interface or derive from +// such abstract class will be broken. +// + +// +// Make sure that the public contracts of interfaces and abstract classes +// that used to be presented to clients, remain stable now, and in the future. +// +// If a public contract must really be changed, you can tag +// abstract methods that will be removed with *System.ObsoleteAttribute* +// with a *workaround message* during a few public releases, until it gets +// removed definitely. +//]]> + Broken serializable types +warnif count > 0 + +from t in Application.Types where + + // Collect types tagged with SerializableAttribute + t.HasAttribute("System.SerializableAttribute".AllowNoMatch()) && + !t.IsDelegate && + t.IsPresentInBothBuilds() && + t.HasAttribute(t) + + // Find newer and older versions of NonSerializedAttribute + let newNonSerializedAttribute = ThirdParty.Types.WithFullName("System.NonSerializedAttribute").SingleOrDefault() + let oldNonSerializedAttribute = newNonSerializedAttribute == null ? null : newNonSerializedAttribute.OlderVersion() + + // Find added or removed fields not marked with NonSerializedAttribute + let addedInstanceField = from f in t.InstanceFields where + f.WasAdded() && + (newNonSerializedAttribute == null || !f.HasAttribute(newNonSerializedAttribute)) + select f + let removedInstanceField = from f in t.OlderVersion().InstanceFields where + f.WasRemoved() && + (oldNonSerializedAttribute == null || !f.HasAttribute(oldNonSerializedAttribute)) + select f + where addedInstanceField.Count() > 0 || removedInstanceField.Count() > 0 + +select new { + t, + addedInstanceField, + removedInstanceField +} + +// +// This rule is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This rule warns about breaking changes in types tagged with +// *SerializableAttribute*. +// +// To do so, this rule searches for serializable type with serializable +// instance fields added or removed. Notice that it doesn't take account +// of fields tagged with *NonSerializedAttribute*. +// +// From http://msdn.microsoft.com/library/system.serializableattribute.aspx : +// "All the public and private fields in a type that are marked by the +// *SerializableAttribute* are serialized by default, unless the type +// implements the *ISerializable* interface to override the serialization process. +// The default serialization process excludes fields that are marked +// with the *NonSerializedAttribute* attribute." +// + +// +// Make sure that the serialization process of serializable types remains +// stable now, and in the future. +// +// Else you'll have to deal with **Version Tolerant Serialization** +// that is explained here: +// https://msdn.microsoft.com/en-us/library/ms229752(v=vs.110).aspx +//]]> + Avoid changing enumerations Flags status +warnif count > 0 + +let oldFlags = codeBase.OlderVersion().ThirdParty.Types.WithFullName("System.FlagsAttribute").SingleOrDefault() +let newFlags = ThirdParty.Types.WithFullName("System.FlagsAttribute").SingleOrDefault() +where oldFlags != null && newFlags != null + +from t in Application.Types where + t.IsEnumeration && + t.IsPresentInBothBuilds() +let hasFlagsAttributeNow = t.HasAttribute(newFlags) +let usedToHaveFlagsAttribute = t.OlderVersion().HasAttribute(oldFlags) +where hasFlagsAttributeNow != usedToHaveFlagsAttribute +select new { + t, + hasFlagsAttributeNow, + usedToHaveFlagsAttribute +} + +// +// This rule is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This rule matches enumeration types that used to be tagged +// with *FlagsAttribute* in the *baseline*, and not anymore. +// It also matches the opposite, enumeration types that are now +// tagged with *FlagsAttribute*, and were not tagged in the *baseline*. +// +// Being tagged with *FlagsAttribute* is a strong property for an enumeration. +// Not so much in terms of *behavior* (only the *enum.ToString()* method +// behavior changes when an enumeration is tagged with *FlagsAttribute*) +// but in terms of *meaning*: is the enumeration a **range of values** +// or a **range of flags**? +// +// As a consequence, changing the *FlagsAttribute*s status of an enumeration can +// have significant impact for its clients. +// + +// +// Make sure the *FlagsAttribute* status of each enumeration remains stable +// now, and in the future. +//]]> + API: New publicly visible types +from t in Application.Types +where t.IsPubliclyVisible && + + // The type has been removed and its parent assembly hasn't been removed … + ( (t.WasAdded() && !t.ParentAssembly.WasAdded()) || + + // … or the type existed but was not publicly visible + !t.WasAdded() && !t.OlderVersion().IsPubliclyVisible) + +select new { + t, + OldVisibility = + (t.WasAdded() ? " " : + t.OlderVersion().Visibility.ToString()) +} + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists types that are new in the public +// surface of the analyzed assemblies. +//]]> + API: New publicly visible methods +from m in Application.Methods +where m.IsPubliclyVisible && + + // The method has been removed and its parent assembly hasn'm been removed … + ( (m.WasAdded() && !m.ParentType.WasAdded()) || + + // … or the t existed but was not publicly visible + !m.WasAdded() && !m.OlderVersion().IsPubliclyVisible) + +select new { + m, + OldVisibility = + (m.WasAdded() ? " " : + m.OlderVersion().Visibility.ToString()) +} + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists methods that are new in the public +// surface of the analyzed assemblies. +//]]> + API: New publicly visible fields +from f in Application.Fields +where f.IsPubliclyVisible && + + // The method has been removed and its parent assembly hasn'f been removed … + ( (f.WasAdded() && !f.ParentType.WasAdded()) || + + // … or the t existed but was not publicly visible + !f.WasAdded() && !f.OlderVersion().IsPubliclyVisible) + +select new { + f, + OldVisibility = + (f.WasAdded() ? " " : + f.OlderVersion().Visibility.ToString()) +} + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists fields that are new in the public +// surface of the analyzed assemblies. +//]]> + + + New assemblies +from a in Application.Assemblies where a.WasAdded() +select new { a, a.NbLinesOfCode } + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists *assemblies* that have been added since the *baseline*. +//]]> + Assemblies removed +from a in codeBase.OlderVersion().Application.Assemblies where a.WasRemoved() +select new { a, a.NbLinesOfCode } + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists *assemblies* that have been removed since the *baseline*. +//]]> + Assemblies where code was changed +from a in Application.Assemblies where a.CodeWasChanged() +select new { + a, + a.NbLinesOfCode, + oldNbLinesOfCode = a.OlderVersion().NbLinesOfCode.GetValueOrDefault() , + delta = (int) a.NbLinesOfCode.GetValueOrDefault() - a.OlderVersion().NbLinesOfCode.GetValueOrDefault() +} + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists *assemblies* in which, code has been changed since the *baseline*. +//]]> + New namespaces +from n in Application.Namespaces where + !n.ParentAssembly.WasAdded() && + n.WasAdded() +select new { n, n.NbLinesOfCode } + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists *namespaces* that have been added since the *baseline*. +//]]> + Namespaces removed +from n in codeBase.OlderVersion().Application.Namespaces where + !n.ParentAssembly.WasRemoved() && + n.WasRemoved() +select new { n, n.NbLinesOfCode } + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists *namespaces* that have been removed since the *baseline*. +//]]> + Namespaces where code was changed +from n in Application.Namespaces where n.CodeWasChanged() +select new { + n, + n.NbLinesOfCode, + oldNbLinesOfCode = n.OlderVersion().NbLinesOfCode.GetValueOrDefault() , + delta = (int) n.NbLinesOfCode.GetValueOrDefault() - n.OlderVersion().NbLinesOfCode.GetValueOrDefault() +} + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists *namespaces* in which, code has been changed since the *baseline*. +//]]> + New types +from t in Application.Types where + !t.ParentNamespace.WasAdded() && + t.WasAdded() && + !t.IsGeneratedByCompiler +select new { t, t.NbLinesOfCode } + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists *types* that have been added since the *baseline*. +//]]> + Types removed +from t in codeBase.OlderVersion().Application.Types where + !t.ParentNamespace.WasRemoved() && + t.WasRemoved() && + !t.IsGeneratedByCompiler +select new { t, t.NbLinesOfCode } + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists *types* that have been removed since the *baseline*. +//]]> + Types where code was changed + +from t in Application.Types where t.CodeWasChanged() +//select new { t, t.NbLinesOfCode } +select new { + t, + t.NbLinesOfCode, + oldNbLinesOfCode = t.OlderVersion().NbLinesOfCode , + delta = (int?) t.NbLinesOfCode - t.OlderVersion().NbLinesOfCode +} +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists *types* in which, code has been changed since the *baseline*. +// +// To visualize changes in code, right-click a matched type and select: +// +// • Compare older and newer versions of source file +// +// • Compare older and newer versions disassembled with Reflector +//]]> + Heuristic to find types moved from one namespace or assembly to another +let typesRemoved = codeBase.OlderVersion().Types.Where(t => t.WasRemoved()) +let typesAdded = Types.Where(t => t.WasAdded()) + +from tMoved in typesAdded.Join( + typesRemoved, + t => t.Name, + t => t.Name, + (tNewer, tOlder) => new { tNewer, + OlderParentNamespace = tOlder.ParentNamespace, + OlderParentAssembly = tOlder.ParentAssembly } ) +select tMoved + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists *types* moved from one namespace or assembly to another. +// The heuristic implemented consists in making a **join LINQ query** on +// type name (without namespace prefix), applied to the two sets of types *added* +// and types *removed*. +//]]> + Types directly using one or several types changed +let typesChanged = Application.Types.Where(t => t.CodeWasChanged()).ToHashSet() + +from t in JustMyCode.Types.UsingAny(typesChanged) where + !t.CodeWasChanged() && + !t.WasAdded() +let typesChangedUsed = t.TypesUsed.Intersect(typesChanged) +select new { t, typesChangedUsed } + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists types *unchanged* since the *baseline* +// but that use directly some *types* where code has been changed +// since the *baseline*. +// +// For such matched type, the code hasen't been changed, but still the overall +// behavior might have been changed. +// +// The query result includes types changed directly used, +//]]> + Types indirectly using one or several types changed +let typesChanged = Application.Types.Where(t => t.CodeWasChanged()).ToHashSet() + +// 'depth' represents a code metric defined on types using +// directly or indirectly any type where code was changed. +let depth = JustMyCode.Types.DepthOfIsUsingAny(typesChanged) + +from t in depth.DefinitionDomain where + !t.CodeWasChanged() && + !t.WasAdded() + +let typesChangedDirectlyUsed = t.TypesUsed.Intersect(typesChanged) +let depthOfUsingTypesChanged = depth[t] +orderby depthOfUsingTypesChanged + +select new { + t, + depthOfUsingTypesChanged, + typesChangedDirectlyUsed +} + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists types *unchanged* since the *baseline* +// but that **use directly or indirectly** some *types* where +// code has been changed since the *baseline*. +// +// For such matched type, the code hasen't been changed, but still the overall +// behavior might have been changed. +// +// The query result includes types changed directly used, and the **depth of usage** +// of types indirectly used, *depth of usage* as defined in the documentation of +// *DepthOfIsUsingAny()* NDepend API method: +// http://www.ndepend.com/api/webframe.html?NDepend.API~NDepend.CodeModel.ExtensionMethodsSequenceUsage~DepthOfIsUsingAny.html +//]]> + New methods +from m in Application.Methods where + !m.ParentType.WasAdded() && + m.WasAdded() && + !m.IsGeneratedByCompiler +select new { m, m.NbLinesOfCode } + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists *methods* that have been added since the *baseline*. +//]]> + Methods removed +from m in codeBase.OlderVersion().Application.Methods where + !m.ParentType.WasRemoved() && + m.WasRemoved() && + !m.IsGeneratedByCompiler +select new { m, m.NbLinesOfCode } + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists *methods* that have been removed since the *baseline*. +//]]> + Methods where code was changed +from m in Application.Methods where m.CodeWasChanged() +select new { + m, + m.NbLinesOfCode, + oldNbLinesOfCode = m.OlderVersion().NbLinesOfCode , + delta = (int?) m.NbLinesOfCode - m.OlderVersion().NbLinesOfCode +} + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists *methods* in which, code has been changed since the *baseline*. +// +// To visualize changes in code, right-click a matched method and select: +// +// • Compare older and newer versions of source file +// +// • Compare older and newer versions disassembled with Reflector +//]]> + Methods directly calling one or several methods changed +let methodsChanged = Application.Methods.Where(m => m.CodeWasChanged()).ToHashSet() + +from m in JustMyCode.Methods.UsingAny(methodsChanged ) where + !m.CodeWasChanged() && + !m.WasAdded() +let methodsChangedCalled = m.MethodsCalled.Intersect(methodsChanged) +select new { + m, + methodsChangedCalled +} + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists methods *unchanged* since the *baseline* +// but that call directly some *methods* where code has been changed +// since the *baseline*. +// +// For such matched method, the code hasen't been changed, but still the overall +// behavior might have been changed. +// +// The query result includes methods changed directly used, +//]]> + Methods indirectly calling one or several methods changed +let methodsChanged = Application.Methods.Where(m => m.CodeWasChanged()).ToHashSet() + +// 'depth' represents a code metric defined on methods using +// directly or indirectly any method where code was changed. +let depth = JustMyCode.Methods.DepthOfIsUsingAny(methodsChanged) + +from m in depth.DefinitionDomain where + !m.CodeWasChanged() && + !m.WasAdded() + +let methodsChangedDirectlyUsed = m.MethodsCalled.Intersect(methodsChanged) +let depthOfUsingMethodsChanged = depth[m] +orderby depthOfUsingMethodsChanged + +select new { + m, + depthOfUsingMethodsChanged, + methodsChangedDirectlyUsed +} + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists methods *unchanged* since the *baseline* +// but that **use directly or indirectly** some *methods* where +// code has been changed since the *baseline*. +// +// For such matched method, the code hasen't been changed, but still the overall +// behavior might have been changed. +// +// The query result includes methods changed directly used, and the **depth of usage** +// of methods indirectly used, *depth of usage* as defined in the documentation of +// *DepthOfIsUsingAny()* NDepend API method: +// http://www.ndepend.com/api/webframe.html?NDepend.API~NDepend.CodeModel.ExtensionMethodsSequenceUsage~DepthOfIsUsingAny.html +//]]> + New fields +from f in Application.Fields where + !f.ParentType.WasAdded() && + f.WasAdded() && + !f.IsGeneratedByCompiler +select f + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists *fields* that have been added since the *baseline*. +//]]> + Fields removed +from f in codeBase.OlderVersion().Application.Fields where + !f.ParentType.WasRemoved() && + f.WasRemoved() && + !f.IsGeneratedByCompiler +select f + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists *fields* that have been removed since the *baseline*. +//]]> + Third party types that were not used and that are now used +from t in ThirdParty.Types where t.IsUsedRecently() +select new { + t, + t.Methods, + t.Fields, + t.TypesUsingMe +} + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists *types* defined in **third-party assemblies**, that were not +// used at *baseline* time, and that are now used. +//]]> + Third party types that were used and that are not used anymore +from t in codeBase.OlderVersion().Types where t.IsNotUsedAnymore() +select new { + t, + t.Methods, + t.Fields, + TypesThatUsedMe = t.TypesUsingMe +} + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists *types* defined in **third-party assemblies**, that were +// used at *baseline* time, and that are not used anymore. +//]]> + Third party methods that were not used and that are now used +from m in ThirdParty.Methods where + m.IsUsedRecently() && + !m.ParentType.IsUsedRecently() +select new { + m, + m.MethodsCallingMe +} + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists *methods* defined in **third-party assemblies**, that were not +// used at *baseline* time, and that are now used. +//]]> + Third party methods that were used and that are not used anymore +from m in codeBase.OlderVersion().Methods where + m.IsNotUsedAnymore() && + !m.ParentType.IsNotUsedAnymore() +select new { + m, + MethodsThatCalledMe = m.MethodsCallingMe +} + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists *methods* defined in **third-party assemblies**, that were +// used at *baseline* time, and that are not used anymore. +//]]> + Third party fields that were not used and that are now used +from f in ThirdParty.Fields where + f.IsUsedRecently() && + !f.ParentType.IsUsedRecently() +select new { + f, + f.MethodsUsingMe +} + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists *fields* defined in **third-party assemblies**, that were not +// used at *baseline* time, and that are now used. +//]]> + Third party fields that were used and that are not used anymore +from f in codeBase.OlderVersion().Fields where + f.IsNotUsedAnymore() && + !f.ParentType.IsNotUsedAnymore() +select new { + f, + MethodsThatUsedMe = f.MethodsUsingMe +} + +// +// This query is executed only if a *baseline for comparison* is defined (*diff mode*). +// +// This code query lists *fields* defined in **third-party assemblies**, that were +// used at *baseline* time, and that are not used anymore. +//]]> + + + C.R.A.P method code metric + +warnif count > 0 +from m in JustMyCode.Methods + +// Don't match too short methods +where m.NbLinesOfCode > 10 + +let CC = m.CyclomaticComplexity +let uncov = (100 - m.PercentageCoverage) / 100f +let CRAP = (CC * CC * uncov * uncov * uncov) + CC +where CRAP != null && CRAP > 30 +orderby CRAP descending, m.NbLinesOfCode descending +select new { m, CRAP, CC, m.PercentageCoverage, m.NbLinesOfCode } + +// +// This rule is executed only if some code coverage data is imported +// from some code coverage files. +// +// **Change Risk Analyzer and Predictor** (i.e. CRAP) is a code metric +// that helps in pinpointing overly complex and untested code. +// Is has been first defined here: +// http://www.artima.com/weblogs/viewpost.jsp?thread=215899 +// +// The Formula is: **CRAP(m) = CC(m)^2 * (1 – cov(m)/100)^3 + CC(m)** +// +// • where *CC(m)* is the *cyclomatic complexity* of the method *m* +// +// • and *cov(m)* is the *percentage coverage* by tests of the method *m* +// +// Matched methods cumulates two highly *error prone* code smells: +// +// • A complex method, difficult to develop and maintain. +// +// • Non 100% covered code, difficult to refactor without introducing any regression bug. +// +// The highest the CRAP score, the more painful to maintain and error prone is the method. +// +// An arbitrary threshold of 30 is fixed for this code rule as suggested by inventors. +// +// Notice that no amount of testing will keep methods with a Cyclomatic Complexity +// highest than 30, out of CRAP territory. +// +// Notice that this rule doesn't match too short method +// with less than 10 lines of code. +// + +// +// In such situation, it is recommended to both refactor the complex method logic +// into several smaller and less complex methods +// (that might belong to some new types especially created), +// and also write unit-tests to full cover the refactored logic. +// +// You'll find code impossible to cover by unit-tests, like calls to *MessageBox.Show()*. +// An infrastructure must be defined to be able to *mock* such code at test-time. +//]]> + Complex methods partially covered by tests should be 100% covered + +warnif count > 0 from m in JustMyCode.Methods + where + // These metrics' definitions are available here: + // http://www.ndepend.com/docs/code-metrics#MetricsOnMethods + ( m.NbLinesOfCode > 30 || + m.ILCyclomaticComplexity > 50 || + m.ILNestingDepth > 4 || + m.NbVariables > 8) && + + // Take care only of complex methods + // already partially covered, but not completely covered. + m.PercentageCoverage > 0 && + m.PercentageCoverage < 100 + + orderby m.NbLinesOfCodeNotCovered ascending, + m.NbLinesOfCode descending +select new { m, m.PercentageCoverage, m.NbLinesOfCode, + m.NbLinesOfCodeCovered, m.NbLinesOfCodeNotCovered, + m.ILCyclomaticComplexity, m.ILNestingDepth, m.NbVariables } + +// +// This rule is executed only if some code coverage data is imported +// from some code coverage files. +// +// There are default rules that warn about complex methods to refactor. +// The present rule warns about complex methods that are already a bit covered by tests +// but not 100% covered by tests. +// +// Such situation cumulates two highly *error prone* code smells: +// +// • A complex method, difficult to develop and maintain. +// +// • Non 100% covered code, difficult to refactor without creating any regression bug. +// +// Because the complex method is already covered partially by tests, +// it might be not so costly to write more tests to full cover it. +// + +// +// In such situation, it is recommended to both: +// +// • refactor the complex method logic +// into several smaller and less complex methods +// (that might belong to some new types especially created), +// +// • and also write more unit-tests to full cover the refactored logic. +// +// You'll find code impossible to cover by unit-tests, like calls to *MessageBox.Show()*. +// An infrastructure must be defined to be able to *mock* such code at test-time. +//]]> + Method changed poorly covered +warnif count > 0 +from m in Application.Methods where + m.PercentageCoverage < 30 && + m.CodeWasChanged() + orderby m.NbLinesOfCode descending, + m.NbLinesOfCodeNotCovered , + m.PercentageCoverage +select new { m, m.PercentageCoverage, m.NbLinesOfCode, + m.NbLinesOfCodeNotCovered } + +// +// This rule is executed only if a *baseline for comparison* is defined (*diff mode*). +// This rule operates only on methods added or refactored since the baseline. +// +// This rule is executed only if some code coverage data is imported +// from some code coverage files. +// +// It is important to write code mostly covered by tests +// to achieve *maintainable* and *non-error-prone* code. +// +// In real-world, many code bases are poorly covered by tests. +// However it is not practicable to stop the development for months +// to refactor and write tests to achieve high code coverage ratio. +// +// Hence it is recommended that each time a method (or a type) gets refactored, +// the developer takes the time to write associated unit-tests to cover it. +// +// Doing so will help to increase significantly the maintainability of the code base. +// You'll notice that quickly, refactoring will also be driven by testability, +// and as a consequence, the overall code structure and design will increase as well. +// + +// +// Write unit-tests to cover the code of most methods and classes refactored. +//]]> + Method added poorly covered +warnif count > 0 +from m in Application.Methods where + m.NbLinesOfCode > 0 && + m.PercentageCoverage < 30 && + m.WasAdded() + orderby m.NbLinesOfCode descending, + m.NbLinesOfCodeNotCovered , + m.PercentageCoverage +select new { m, m.PercentageCoverage, m.NbLinesOfCode, + m.NbLinesOfCodeNotCovered } + +// +// This rule is executed only if a *baseline for comparison* is defined (*diff mode*). +// This rule operates only on methods added or refactored since the baseline. +// +// This rule is executed only if some code coverage data is imported +// from some code coverage files. +// +// It is important to write code mostly covered by tests +// to achieve *maintainable* and *non-error-prone* code. +// +// In real-world, many code bases are poorly covered by tests. +// However it is not practicable to stop the development for months +// to refactor and write tests to achieve high code coverage ratio. +// +// Hence it is recommended that each time a method (or a type) gets added, +// the developer takes the time to write associated unit-tests to cover it. +// +// Doing so will help to increase significantly the maintainability of the code base. +// You'll notice that quickly, refactoring will also be driven by testability, +// and as a consequence, the overall code structure and design will increase as well. +// + +// +// Write unit-tests to cover the code of most methods and classes added. +//]]> + Types 95% to 99% covered +warnif count > 0 +from t in Application.Types where + t.PercentageCoverage >= 95 && + t.PercentageCoverage <= 99 && + !t.IsGeneratedByCompiler + + let methodsCulprit = t.Methods.Where(m => m.PercentageCoverage < 100) + + orderby t.NbLinesOfCode descending , + t.NbLinesOfCodeNotCovered , + t.PercentageCoverage +select new { t, t.PercentageCoverage, t.NbLinesOfCode, + t.NbLinesOfCodeNotCovered, methodsCulprit } + +// +// This rule is executed only if some code coverage data is imported +// from some code coverage files. +// +// Often covering the few percents of remaining uncovered code of a class, +// requires as much work as covering the first 90%. +// For this reason, often teams estimate that 90% coverage is enough. +// However *untestable code* usually means *poorly written code* +// which usually leads to *error prone code*. +// So it might be worth refactoring and making sure to cover the few uncovered lines of code +// **because most tricky bugs might come from this small portion of hard-to-test code**. +// +// Not all classes should be 100% covered by tests (like UI code can be hard to test) +// but you should make sure that most of the logic of your application +// is defined in some *easy-to-test classes*, 100% covered by tests. +// + +// +// Write more unit-tests dedicated to cover code not covered yet. +// If you find some *hard-to-test code*, it is certainly a sign that this code +// is not *well designed* and hence, needs refactoring. +//]]> + Namespaces 95% to 99% covered +warnif count > 0 +from n in Application.Namespaces where + n.PercentageCoverage >= 95 && + n.PercentageCoverage <= 99 + + let methodsCulprit = n.ChildMethods.Where(m => m.PercentageCoverage < 100) + + orderby n.NbLinesOfCode descending , + n.NbLinesOfCodeNotCovered , + n.PercentageCoverage +select new { n, n.PercentageCoverage, n.NbLinesOfCode, + n.NbLinesOfCodeNotCovered, methodsCulprit } + +// +// This rule is executed only if some code coverage data is imported +// from some code coverage files. +// +// Often covering the few percents of remaining uncovered code of +// one or several classes in a namespace +// requires as much work as covering the first 90%. +// For this reason, often teams estimate that 90% coverage is enough. +// However *untestable code* usually means *poorly written code* +// which usually leads to *error prone code*. +// So it might be worth refactoring and making sure to cover the few uncovered lines of code +// **because most tricky bugs might come from this small portion of hard-to-test code**. +// +// Not all classes should be 100% covered by tests (like UI code can be hard to test) +// but you should make sure that most of the logic of your application +// is defined in some *easy-to-test classes*, 100% covered by tests. +// + +// +// Write more unit-tests dedicated to cover code not covered yet in the namespace. +// If you find some *hard-to-test code*, it is certainly a sign that this code +// is not *well designed* and hence, needs refactoring. +//]]> + Types tagged with FullCoveredAttribute should be 100% covered +warnif count > 0 +from t in Application.Types where + t.HasAttribute ("NDepend.Attributes.FullCoveredAttribute".AllowNoMatch()) && + t.PercentageCoverage < 100 + +let notFullCoveredMethods = t.Methods.Where( + m => m.NbLinesOfCode> 0 && + m.PercentageCoverage < 100 && + !m.HasAttribute("NDepend.Attributes.UncoverableByTestAttribute".AllowNoMatch())) + +orderby t.NbLinesOfCodeNotCovered descending + +select new { t, t.PercentageCoverage, t.NbLinesOfCodeNotCovered, notFullCoveredMethods, + t.NbLinesOfCode, t.NbLinesOfCodeCovered } + +// +// This rule is executed only if some code coverage data is imported +// from some code coverage files. +// +// By using a **FullCoveredAttribute**, you can express in source code the intention +// that a class is 100% covered by tests, and should remain 100% covered in the future. +// If you don't want to link *NDepend.API.dll*, +// you can use your own attribute and adapt the source code of this rule. +// +// Benefits of using a **FullCoveredAttribute** are twofold: +// Not only the intention is expressed in source code, +// but it is also continuously checked by the present rule. +// +// Often covering 10% of remaining uncovered code of a class, +// requires as much work as covering the first 90%. +// For this reason, often teams estimate that 90% coverage is enough. +// However *untestable code* usually means *poorly written code* which usually means *error prone code*. +// So it might be worth refactoring and making sure to cover the 10% remaining code +// **because most tricky bugs might come from this small portion of hard-to-test code**. +// +// Not all classes should be 100% covered by tests (like UI code can be hard to test) +// but you should make sure that most of the logic of your application +// is defined in some *easy-to-test classes*, 100% covered by tests. +// + +// +// Write more unit-tests dedicated to cover code of matched classes not covered yet. +// If you find some *hard-to-test code*, it is certainly a sign that this code +// is not *well designed* and hence, needs refactoring. +//]]> + Types 100% covered should be tagged with FullCoveredAttribute + +warnif count > 0 from t in JustMyCode.Types where + !t.HasAttribute ("NDepend.Attributes.FullCoveredAttribute".AllowNoMatch()) && + t.PercentageCoverage == 100 && + !t.IsGeneratedByCompiler +select new { t, t.NbLinesOfCode } + +// +// This rule is executed only if some code coverage data is imported +// from some code coverage files. +// +// By using a **FullCoveredAttribute**, you can express in source code the intention +// that a class is 100% covered by tests, and should remain 100% covered in the future. +// +// Benefits of using a **FullCoveredAttribute** are twofold: +// Not only the intention is expressed in source code, +// but it is also continuously checked by the present rule. +// + +// +// Just tag types 100% covered by tests with the **FullCoveredAttribute** +// that can be found in *NDepend.API.dll*, +// or by an attribute of yours defined in your own code +// (in which case this rule must be adapted). +//]]> + Types not covered at all +from t in Application.Types where + t.PercentageCoverage == 0 + orderby t.NbLinesOfCode descending +select new { t, t.NbLinesOfCode } + +// +// This query is executed only if some code coverage data is imported +// from some code coverage files. +// +// This code query lists types not covered at all by any test. +// +// Often, when code is not covered at all by test, one can reach +// decent coverage ratio (like 50% to 90%) just by writing a few tests. +// +// The idea is not to cover code for the sake of it. +// The idea is that thanks to a small effort of writing a few tests, +// one can continuously check, that a significant portion of code +// runs without a problem. +//]]> + Namespaces not covered at all +from n in Application.Namespaces where + n.PercentageCoverage == 0 + orderby n.NbLinesOfCode descending +select new { n, n.NbLinesOfCode} + +// +// This query is executed only if some code coverage data is imported +// from some code coverage files. +// +// This code query lists namespaces not covered at all by any test. +// +// Often, when code is not covered at all by test, one can reach +// decent coverage ratio (like 50% to 90%) just by writing a few tests. +// +// The idea is not to cover code for the sake of it. +// The idea is that thanks to a small effort of writing a few tests, +// one can continuously check, that a significant portion of code +// runs without a problem. +//]]> + Test Methods + +let testAttr = ThirdParty.Types.WithNameIn("TestAttribute", "TestCaseAttribute") +let testMethods = Methods.TaggedWithAnyAttributes(testAttr) +from m in testMethods +select m + +// +// We advise to not include test assemblies in code analyzed by NDepend. +// We estimate that it is acceptable and practical to lower the quality gate of test code, +// because the important measures for tests are: +// +// • The coverage ratio, +// +// • And the amount of logic results asserted: This includes both +// assertions in test code, and assertions in code covered by tests, +// like *Code Contract* assertions and *Debug.Assert(…)* assertions. +// +// But if you wish to enforce the quality of test code, you'll need to +// consider test assemblies in your list of application assemblies +// analyzed by NDepend. +// +// In such situation, this code query lists tests methods and you can +// reuse this code in custom rules. +//]]> + Methods directly called by test Methods + +let testAttr = ThirdParty.Types.WithNameIn("TestAttribute", "TestCaseAttribute") +let testMethods = Methods.TaggedWithAnyAttributes(testAttr).ToHashSet() + +// --- Uncomment this line if your test methods are in dedicated test assemblies --- +//let testAssemblies = testMethods.ParentAssemblies().ToHashSet() + +from m in Application.Methods.UsedByAny(testMethods) + +// --- Uncomment this line if your test methods are in dedicated test assemblies --- +//where !testAssemblies.Contains(m.ParentAssembly) + +select new { m , + calledByTests = m.MethodsCallingMe.Intersect(testMethods ), + // --- Uncomment this line if your project import some coverage data --- + // m.PercentageCoverage +} + + +// +// This query lists all methods directly called by tests methods. +// Overrides of virtual and abstract methods, called through polymorphism, are not listed. +// Methods solely invoked through a delegate are not listed. +// Methods solely invoked through reflection are not listed. +// +// We advise to not include test assemblies in code analyzed by NDepend. +// We estimate that it is acceptable and practical to lower the quality gate of test code, +// because the important measures for tests are: +// +// • The coverage ratio, +// +// • And the amount of logic results asserted: This includes both +// assertions in test code, and assertions in code covered by tests, +// like *Code Contract* assertions and *Debug.Assert(…)* assertions. +// +// But if you wish to run this code query, +// you'll need to consider test assemblies in your list of +// application assemblies analyzed by NDepend. +//]]> + Methods directly and indirectly called by test Methods + +let testAttr = from t in ThirdParty.Types.WithNameIn("TestAttribute", "TestCaseAttribute") select t +let testMethods = Methods.TaggedWithAnyAttributes(testAttr) + +// --- Uncomment this line if your test methods are in dedicated test assemblies --- +// let testAssemblies = testMethods.ParentAssemblies().ToHashSet() + +let depthOfCalledByTest = Application.Methods.DepthOfIsUsedByAny(testMethods) +from pair in depthOfCalledByTest +where pair.Value > 0 +orderby pair.Value ascending +// --- Uncomment this line if your test methods are in dedicated test assemblies --- +//&& !testAssemblies.Contains(pair.CodeElement.ParentAssembly) + +select new { + method = pair.CodeElement, + // (depthOfCalledByTests == 1) means that the method is directly called by tests + // (depthOfCalledByTests == 2) means that the method is directly called by a method directly called by tests + // … + depthOfCalledByTests = pair.Value, + nbLinesOfCode = pair.CodeElement.NbLinesOfCode, + // --- Uncomment this line if your project import some coverage data --- + // m.PercentageCoverage +} + +// +// This query lists all methods *directly or indirectly* called by tests methods. +// *Indirectly* called by a test means that a test method calls a method, that calls a method… +// From this recursion, a code metric named *depthOfCalledByTests* is inferred, +// The value *1* means directly called by test, +// the value *2* means called by a method that is called by a test… +// +// Overrides of virtual and abstract methods, called through polymorphism, are not listed. +// Methods solely invoked through a delegate are not listed. +// Methods solely invoked through reflection are not listed. +// +// We advise to not include test assemblies in code analyzed by NDepend. +// We estimate that it is acceptable and practical to lower the quality gate of test code, +// because the important measures for tests are: +// +// • The coverage ratio, +// +// • And the amount of logic results asserted: This includes both +// assertions in test code, and assertions in code covered by tests, +// like *Code Contract* assertions and *Debug.Assert(…)* assertions. +// +// But if you wish to run this code query, +// you'll need to consider test assemblies in your list of +// application assemblies analyzed by NDepend. +//]]> + + + Potentially dead Types + +warnif count > 0 +// Filter procedure for types that should'nt be considered as dead +let canTypeBeConsideredAsDeadProc = new Func( + t => !t.IsPublic && // Public types might be used by client applications of your assemblies. + t.Name != "Program" && + !t.IsGeneratedByCompiler && + + // If you don't want to link NDepend.API.dll, you can use your own IsNotDeadCodeAttribute + // and adapt the source code of this rule. + !t.HasAttribute("NDepend.Attributes.IsNotDeadCodeAttribute".AllowNoMatch()) && + + // Exclude static types that define only const fields + // because they cannot be seen as used in IL code. + !(t.IsStatic && t.NbMethods == 0 && !t.Fields.Where(f => !f.IsLiteral).Any())) + +// Select types unused +let typesUnused = + from t in JustMyCode.Types where + t.NbTypesUsingMe == 0 && canTypeBeConsideredAsDeadProc(t) + select t + +// Dead types = types used only by unused types (recursive) +let deadTypesMetric = typesUnused.FillIterative( +types => from t in codeBase.Application.Types.UsedByAny(types).Except(types) + where canTypeBeConsideredAsDeadProc(t) && + t.TypesUsingMe.Intersect(types).Count() == t.NbTypesUsingMe + select t) + +from t in deadTypesMetric.DefinitionDomain +select new { + t, + depth = deadTypesMetric[t], + t.TypesUsingMe +} + +// +// This rule lists *potentially* **dead types**. +// A dead type is a type that can be removed +// because it is never used by the program. +// +// This rule lists not only types not used anywhere in code, +// but also types used only by types not used anywhere in code. +// This is why this rule comes with a column *TypesusingMe* and +// this is why there is a code metric named *depth*: +// +// • A *depth* value of *0* means the type is not used. +// +// • A *depth* value of *1* means the type is used only by types not used. +// +// • etc… +// +// By reading the source code of this rule, you'll see that by default, +// *public* types are not matched, because such type might not be used +// by the analyzed code, but still be used by client code, not analyzed by NDepend. +// This default behavior can be easily changed. +// + +// +// *Static analysis* cannot provide an *exact* list of dead types, +// because there are several ways to use a type *dynamically* (like through reflection). +// +// For each type matched by this query, first investigate if the type is used somehow +// (like through reflection). +// If the type is really never used, it is important to remove it +// to avoid maintaining useless code. +// If you estimate the code of the type might be used in the future, +// at least comment it, and provide an explanatory comment about the future intentions. +// +// If a type is used somehow, +// but still is matched by this rule, you can tag it with the attribute +// **IsNotDeadCodeAttribute** found in *NDepend.API.dll* to avoid matching the type again. +// You can also provide your own attribute for this need, +// but then you'll need to adapt this code rule. +//]]> + Potentially dead Methods + +warnif count > 0 +// Filter procedure for methods that should'nt be considered as dead +let canMethodBeConsideredAsDeadProc = new Func( + m => !m.IsPubliclyVisible && // Public methods might be used by client applications of your assemblies. + !m.IsEntryPoint && // Main() method is not used by-design. + !m.IsExplicitInterfaceImpl && // The IL code never explicitly calls explicit interface methods implementation. + !m.IsClassConstructor && // The IL code never explicitly calls class constructors. + !m.IsFinalizer && // The IL code never explicitly calls finalizers. + !m.IsVirtual && // Only check for non virtual method that are not seen as used in IL. + !(m.IsConstructor && // Don't take account of protected ctor that might be call by a derived ctors. + m.IsProtected) && + !m.IsEventAdder && // The IL code never explicitly calls events adder/remover. + !m.IsEventRemover && + !m.IsGeneratedByCompiler && + !m.ParentType.IsDelegate && + + // Methods tagged with these two attributes are called by the serialization infrastructure. + !m.HasAttribute("System.Runtime.Serialization.OnSerializingAttribute".AllowNoMatch()) && + !m.HasAttribute("System.Runtime.Serialization.OnDeserializedAttribute".AllowNoMatch()) && + + // If you don't want to link NDepend.API.dll, you can use your own IsNotDeadCodeAttribute + // and adapt the source code of this rule. + !m.HasAttribute("NDepend.Attributes.IsNotDeadCodeAttribute".AllowNoMatch())) + +// Get methods unused +let methodsUnused = + from m in JustMyCode.Methods where + m.NbMethodsCallingMe == 0 && + canMethodBeConsideredAsDeadProc(m) + select m + +// Dead methods = methods used only by unused methods (recursive) +let deadMethodsMetric = methodsUnused.FillIterative( + methods => // Unique loop, just to let a chance to build the hashset. + from o in (new object()).ToEnumerable() + // Use a hashet to make Intersect calls much faster! + let hashset = methods.ToHashSet() + from m in codeBase.Application.Methods.UsedByAny(methods).Except(methods) + where canMethodBeConsideredAsDeadProc(m) && + // Select methods called only by methods already considered as dead + hashset.Intersect(m.MethodsCallingMe).Count() == m.NbMethodsCallingMe + select m) + +from m in JustMyCode.Methods.Intersect(deadMethodsMetric.DefinitionDomain) +select new { + m, + depth = deadMethodsMetric[m], + m.MethodsCallingMe +} + +// +// This rule lists *potentially* **dead methods**. +// A dead method is a method that can be removed +// because it is never called by the program. +// +// This rule lists not only methods not called anywhere in code, +// but also methods called only by methods not called anywhere in code. +// This is why this rule comes with a column *MethodsCallingMe* and +// this is why there is a code metric named *depth*: +// +// • A *depth* value of *0* means the method is not called. +// +// • A *depth* value of *1* means the method is called only by methods not called. +// +// • etc… +// +// By reading the source code of this rule, you'll see that by default, +// *public* methods are not matched, because such method might not be called +// by the analyzed code, but still be called by client code, not analyzed by NDepend. +// This default behavior can be easily changed. +// + +// +// *Static analysis* cannot provide an *exact* list of dead methods, +// because there are several ways to invoke a method *dynamically* (like through reflection). +// +// For each method matched by this query, first investigate if the method is invoked somehow +// (like through reflection). +// If the method is really never invoked, it is important to remove it +// to avoid maintaining useless code. +// If you estimate the code of the method might be used in the future, +// at least comment it, and provide an explanatory comment about the future intentions. +// +// If a method is invoked somehow, +// but still is matched by this rule, you can tag it with the attribute +// **IsNotDeadCodeAttribute** found in *NDepend.API.dll* to avoid matching the method again. +// You can also provide your own attribute for this need, +// but then you'll need to adapt this code rule. +//]]> + Potentially dead Fields +warnif count > 0 +from f in JustMyCode.Fields where + f.NbMethodsUsingMe == 0 && + !f.IsPublic && // Although not recommended, public fields might be used by client applications of your assemblies. + !f.IsLiteral && // The IL code never explicitly uses literal fields. + !f.IsEnumValue && // The IL code never explicitly uses enumeration value. + f.Name != "value__" && // Field named 'value__' are relative to enumerations and the IL code never explicitly uses them. + !f.HasAttribute("NDepend.Attributes.IsNotDeadCodeAttribute".AllowNoMatch()) && + !f.IsGeneratedByCompiler + // If you don't want to link NDepend.API.dll, you can use your own IsNotDeadCodeAttribute + // and adapt the source code of this rule. +select f + +// +// This rule lists *potentially* **dead fields**. +// A dead field is a field that can be removed +// because it is never used by the program. +// +// By reading the source code of this rule, you'll see that by default, +// *public* fields are not matched, because such field might not be used +// by the analyzed code, but still be used by client code, not analyzed by NDepend. +// This default behavior can be easily changed. +// Some others default rules in the *Visibility* group, warn about public fields. +// +// More restrictions are applied by this rule because of some *by-design* limitations. +// NDepend mostly analyzes compiled IL code, and the information that +// an enumeration value or a literal constant (which are fields) is used +// is lost in IL code. Hence by default this rule won't match such field. +// + +// +// *Static analysis* cannot provide an *exact* list of dead fields, +// because there are several ways to assign or read a field *dynamically* +// (like through reflection). +// +// For each field matched by this query, first investigate +// if the field is used somehow (like through reflection). +// If the field is really never used, it is important to remove it +// to avoid maintaining a useless code element. +// +// If a field is used somehow, +// but still is matched by this rule, you can tag it with the attribute +// **IsNotDeadCodeAttribute** found in *NDepend.API.dll* +// to avoid matching the field again. +// You can also provide your own attribute for this need, +// but then you'll need to adapt this code rule. +//]]> + Wrong usage of IsNotDeadCodeAttribute + +warnif count == 1 + +let tAttr = Types.WithFullName("NDepend.Attributes.IsNotDeadCodeAttribute").FirstOrDefault() +where tAttr != null + +// Get types that do a wrong usage of IsNotDeadCodeAttribute +let types = from t in Application.Types where + t.HasAttribute("NDepend.Attributes.IsNotDeadCodeAttribute".AllowNoMatch()) && + + ( // types used don't need to be tagged with IsNotDeadCodeAttribute! + t.TypesUsingMe.Count(t1 => + !t.NestedTypes.Contains(t1) && + !t1.HasAttribute("NDepend.Attributes.IsNotDeadCodeAttribute".AllowNoMatch()) ) > 0 || + + // Static types that define only const fields cannot be seen as used in IL code. + // They don't need to be tagged with IsNotDeadCodeAttribute. + (t.IsStatic && t.NbMethods == 0 && !t.Fields.Where(f => !f.IsLiteral).Any()) + ) + select t + +// Get methods that do a wrong usage of IsNotDeadCodeAttribute +let methods = from m in Application.Methods where + m.HasAttribute("NDepend.Attributes.IsNotDeadCodeAttribute".AllowNoMatch()) && + m.MethodsCallingMe.Count(m1 => !m1.HasAttribute("NDepend.Attributes.IsNotDeadCodeAttribute".AllowNoMatch())) > 0 + select m + +// Get fields that do a wrong usage of IsNotDeadCodeAttribute +let fields = from f in Application.Fields where + f.HasAttribute("NDepend.Attributes.IsNotDeadCodeAttribute".AllowNoMatch()) && + f.MethodsUsingMe.Count(m1 => !m1.HasAttribute("NDepend.Attributes.IsNotDeadCodeAttribute".AllowNoMatch())) > 0 + select f + +where types.Count() > 0 || methods.Count() > 0 || fields.Count() > 0 +select new { tAttr, types , methods, fields } + +// +// The attribute **NDepend.Attributes.IsNotDeadCodeAttribute** +// is defined in *NDepend.API.dll*. This attribute is used +// to mean that a code element is not used directly, but is used +// somehow, like through reflection. +// +// This attribute is used in the dead code rules, +// *Potentially dead Types*, *Potentially dead Methods* +// and *Potentially dead Fields*. +// If you don't want to link *NDepend.API.dll*, you can use +// your own *IsNotDeadCodeAttribute* and adapt the source code of +// this rule, and the source code of the *dead code* rules. +// +// In this context, this code rule matches code elements +// (types, methods, fields) that are tagged with this attribute, +// but still used directly somewhere in the code. +// + +// +// Just remove *IsNotDeadCodeAttribute* tagging of +// types, methods and fields matched by this rule +// because this tag is not useful anymore. +//]]> + + + Methods that could have a lower visibility +warnif count > 0 from m in JustMyCode.Methods where + m.Visibility != m.OptimalVisibility && + !m.HasAttribute("NDepend.Attributes.CannotDecreaseVisibilityAttribute".AllowNoMatch()) && + !m.HasAttribute("NDepend.Attributes.IsNotDeadCodeAttribute".AllowNoMatch()) && + // If you don't want to link NDepend.API.dll, you can use your own attributes + // and adapt the source code of this rule. + + // Eliminate default constructor from the result. + // Whatever the visibility of the declaring class, + // default constructors are public and introduce noise + // in the current rule. + !( m.IsConstructor && m.IsPublic && m.NbParameters == 0) && + + // Don't decrease the visibility of Main() methods. + !m.IsEntryPoint + +select new { + m, + m.Visibility , + CouldBeDeclared = m.OptimalVisibility, + m.MethodsCallingMe +} + +// +// This rule warns about methods that can be declared with a lower visibility +// without provoking any compilation error. +// For example *private* is a visibility lower than *internal* +// which is lower than *public*. +// +// **Narrowing visibility** is a good practice because doing so **promotes encapsulation**. +// The scope from which methods can be called is then reduced to a minimum. +// +// Notice that methods tagged with one of the attribute +// *NDepend.Attributes.CannotDecreaseVisibilityAttribute* or +// *NDepend.Attributes.IsNotDeadCodeAttribute*, found in *NDepend.API.dll* +// are not matched. If you don't want to link *NDepend.API.dll* but still +// wish to rely on this facility, you can declare these attributes in your code. +// + +// +// Declare each matched method with the specified *optimal visibility* +// in the *CouldBeDeclared* rule result column. +// +// By default, this rule matches *public methods*. If you are publishing an API +// many public methods matched should remain public. In such situation, +// you can opt for the *coarse solution* to this problem by adding in the +// rule source code *&& !m.IsPubliclyVisible* or you can prefer the +// *finer solution* by tagging each concerned method with +// *CannotDecreaseVisibilityAttribute*. +//]]> + Types that could have a lower visibility +warnif count > 0 from t in JustMyCode.Types where + + t.Visibility != t.OptimalVisibility && + + // If you don't want to link NDepend.API.dll, you can use your own attributes + // and adapt the source code of this rule. + !t.HasAttribute("NDepend.Attributes.CannotDecreaseVisibilityAttribute".AllowNoMatch()) && + !t.HasAttribute("NDepend.Attributes.IsNotDeadCodeAttribute".AllowNoMatch()) && + + // XML serialized type must remain public. + !t.HasAttribute("System.Xml.Serialization.XmlRootAttribute".AllowNoMatch()) && + + // Static types that define only const fields cannot be seen as used in IL code. + // They don't have to be tagged with CannotDecreaseVisibilityAttribute. + !( t.IsStatic && + !t.Methods.Any(m => !m.IsClassConstructor) && + !t.Fields.Any(f => !f.IsLiteral && !(f.IsStatic && f.IsInitOnly))) && + + // A type used by an interface that has the same visibility + // cannot have its visibility decreased, else a compilation error occurs! + !t.TypesUsingMe.Any(tUser => + tUser.IsInterface && + tUser.Visibility == t.Visibility) && + + // Don't change the visibility of a type that contain an entry point method. + !t.Methods.Any(m =>m.IsEntryPoint) + +select new { + t, + t.Visibility , + CouldBeDeclared = t.OptimalVisibility, + t.TypesUsingMe +} + +// +// This rule warns about types that can be declared with a lower visibility +// without provoking any compilation error. +// For example *private* is a visibility lower than *internal* +// which is lower than *public*. +// +// **Narrowing visibility** is a good practice because doing so **promotes encapsulation**. +// The scope from which types can be consumed is then reduced to a minimum. +// +// Notice that types tagged with one of the attribute +// *NDepend.Attributes.CannotDecreaseVisibilityAttribute* or +// *NDepend.Attributes.IsNotDeadCodeAttribute*, found in *NDepend.API.dll* +// are not matched. If you don't want to link *NDepend.API.dll* but still +// wish to rely on this facility, you can declare these attributes in your code. +// + +// +// Declare each matched type with the specified *optimal visibility* +// in the *CouldBeDeclared* rule result column. +// +// By default, this rule matches *public types*. If you are publishing an API +// many public types matched should remain public. In such situation, +// you can opt for the *coarse solution* to this problem by adding in the +// rule source code *&& !m.IsPubliclyVisible* or you can prefer the +// *finer solution* by tagging each concerned type with +// *CannotDecreaseVisibilityAttribute*. +//]]> + Fields that could have a lower visibility +warnif count > 0 from f in JustMyCode.Fields where + f.Visibility != f.OptimalVisibility && + !f.HasAttribute("NDepend.Attributes.CannotDecreaseVisibilityAttribute".AllowNoMatch()) && + !f.HasAttribute("NDepend.Attributes.IsNotDeadCodeAttribute".AllowNoMatch()) && + // If you don't want to link NDepend.API.dll, you can use your own attributes + // and adapt the source code of this rule. + + // XML serialized fields must remain public. + !f.HasAttribute("System.Xml.Serialization.XmlElementAttribute".AllowNoMatch()) && + !f.HasAttribute("System.Xml.Serialization.XmlAttributeAttribute".AllowNoMatch()) + +select new { + f, + f.Visibility , + CouldBeDeclared = f.OptimalVisibility, + f.MethodsUsingMe +} + +// +// This rule warns about fields that can be declared with a lower visibility +// without provoking any compilation error. +// For example *private* is a visibility lower than *internal* +// which is lower than *public*. +// +// **Narrowing visibility** is a good practice because doing so **promotes encapsulation**. +// The scope from which fields can be consumed is then reduced to a minimum. +// +// Notice that fields tagged with one of the attribute +// *NDepend.Attributes.CannotDecreaseVisibilityAttribute* or +// *NDepend.Attributes.IsNotDeadCodeAttribute*, found in *NDepend.API.dll* +// are not matched. If you don't want to link *NDepend.API.dll* but still +// wish to rely on this facility, you can declare these attributes in your code. +// + +// +// Declare each matched field with the specified *optimal visibility* +// in the *CouldBeDeclared* rule result column. +// +// By default, this rule matches *public fields*. If you are publishing an API +// some public fields matched should remain public. In such situation, +// you can opt for the *coarse solution* to this problem by adding in the +// rule source code *&& !m.IsPubliclyVisible* or you can prefer the +// *finer solution* by tagging eah concerned field with +// *CannotDecreaseVisibilityAttribute*. +//]]> + Types that could be declared as private, nested in a parent type + +warnif count > 0 +from t in Application.Types +where !t.IsGeneratedByCompiler && + !t.IsNested && + !t.IsPubliclyVisible && + !t.IsEnumeration && + // Only one type user… + t.TypesUsingMe.Count() == 1 +let couldBeNestedIn = t.TypesUsingMe.Single() +where !couldBeNestedIn.IsGeneratedByCompiler && + // …declared in the same namespace + couldBeNestedIn.ParentNamespace == t.ParentNamespace +select new { + t, + couldBeNestedIn +} + +// +// This rule matches types that can be potentially +// *nested* and declared *private* into another type. +// +// The conditions for a type to be potentially nested +// into a *parent type* are: +// +// • the *parent type* is the only type consuming it, +// +// • the type and the *parent type* are declared in the same namespace. +// +// Declaring a type as private into a parent type **promotes encapsulation**. +// The scope from which the type can be consumed is then reduced to a minimum. +// + +// +// Nest each matched *type* into the specified *parent type* and +// declare it as private. +// +// However *nested private types* are hardly testable. Hence this rule +// might not be applied to types consumed directly by tests. +//]]> + Avoid publicly visible constant fields +warnif count > 0 +from f in Application.Fields +where f.IsLiteral && + f.IsPubliclyVisible && + !f.IsEnumValue +select f + +// +// This rule warns about constant fields that are visible outside their +// parent assembly. Such field, when used from outside its parent assembly, +// has its constant value *hard-coded* into the client assembly. +// Hence, when changing the field's value, it is *mandatory* to recompile +// all assemblies that consume the field, else the program will run +// with different constant values in-memory. Certainly in such situation +// bugs are lurking. +// + +// +// Declare matched fields as **static readonly** instead of **constant**. +// This way, the field value is *safely changeable* without the need to +// recompile client assemblies. +// +// Notice that enumeration value fields suffer from the same *potential +// pitfall*. But enumeration values cannot be declared as +// *static readonly* hence the rule comes with the condition +// **&& !f.IsEnumValue** to avoid matching these. Unless you decide +// to banish public enumerations, just let the rule *as is*. +//]]> + Fields should be declared as private +warnif count > 0 from f in Application.Fields where + !f.IsPrivate && + + // These conditions filter cases where fields + // doesn't represent state that should be encapsulated. + !f.IsGeneratedByCompiler && + !f.IsSpecialName && + !f.IsInitOnly && + !f.IsLiteral && + !f.IsEnumValue + +// A non-private field assigned from outside its class, +// usually leads to complicated field state management. +let outsideMethodsAssigningMe = + f.MethodsAssigningMe.Where(m => m.ParentType != f.ParentType) + +select new { + f, + f.Visibility, + outsideMethodsAssigningMe +} + +// +// This rule matches **non-private and mutable fields**. +// *Mutable* means that the field value can be modified. +// Typically mutable fields are *non-constant*, +// *non-readonly* fields. +// +// Fields should be considered as **implementation details** +// and as a consequence they should be declared as private. +// +// If something goes wrong with a *non-private field*, +// the culprit can be anywhere, and so in order to track down +// the bug, you may have to look at quite a lot of code. +// +// A private field, by contrast, can only be assigned from +// inside the same class, so if something goes wrong with that, +// there is usually only one source file to look at. +// + +// +// Declare a matched mutable field as *private*, or declare it +// as *readonly*. +//]]> + Constructors of abstract classes should be declared as protected or private +warnif count > 0 +from t in Application.Types where + t.IsClass && + t.IsAbstract +let ctors = t.Constructors.Where(c => !c.IsProtected && !c.IsPrivate) +where ctors.Count() > 0 +select new { t, ctors } + +// +// Constructors of abstract classes can only be called from derived +// classes. +// +// Because a public constructor is creating instances of its class, +// and it is forbidden to create instances of an *abstract* type, +// as a consequence an abstract class with a public constructor is +// wrong design. +// + +// +// To fix a violation of this rule, +// either declare the constructor as *protected*, +// or do not declare the type as *abstract*. +//]]> + Avoid public methods not publicly visible + +warnif count > 0 +from m in JustMyCode.Methods where + !m.IsPubliclyVisible && m.IsPublic && + + // Eliminate virtual methods + !m.IsVirtual && + // Eliminate interface and delegate types + !m.ParentType.IsInterface && + !m.ParentType.IsDelegate && + // Eliminate default constructors + !(m.IsConstructor && m.NbParameters == 0) && + // Eliminate operators that must be declared public + !m.IsOperator && + // Eliminate methods generated by compiler + !m.IsGeneratedByCompiler + +let calledOutsideParentType = + m.MethodsCallingMe.FirstOrDefault(mCaller => mCaller.ParentType != m.ParentType) != null + +select new { + m, + parentTypeVisibility = m.ParentType.Visibility, + declareMethodAs = (Visibility) (calledOutsideParentType ? Visibility.Internal : Visibility.Private), + methodsCaller = m.MethodsCallingMe +} + +// +// This rule warns about methods declared as *public* +// whose parent type is not declared as *public*. +// +// In such situation *public* means, *can be accessed +// from anywhere my parent type is visible*. Some +// developers think this is an elegant language construct, +// some others find it misleading. +// +// This rule can be deactivated if you don't agree with it. +// Read the whole debate here: +// http://ericlippert.com/2014/09/15/internal-or-public/ +// + +// +// Declare the method as *internal* if it is used outside of +// its type, else declare it as *private*. +//]]> + Methods that should be declared as 'public' in C#, 'Public' in VB.NET +from m in Application.Methods where + m.ShouldBePublic +let usedInAssemblies = m.MethodsCallingMe.ParentAssemblies().Except(m.ParentAssembly) +select new { m, m.ParentAssembly, usedInAssemblies, m.MethodsCallingMe } + +// +// This code query lists methods that *should* be declared +// as *public*. Such method is actually declared as *internal* +// and is consumed from outside its parent assembly +// thanks to the attribute +// *System.Runtime.CompilerServices.InternalsVisibleToAttribute*. +// +// This query relies on the property +// *NDepend.CodeModel.IMember.ShouldBePublic* +// http://www.ndepend.com/api/webframe.html?NDepend.API~NDepend.CodeModel.IMember~ShouldBePublic.html +// +// This is just a code query, it is not intended to advise +// you to declare the method as *public*, but to inform you +// that the code actually relies on the peculiar behavior +// of the attribute *InternalsVisibleToAttribute*. +//]]> + Wrong usage of CannotDecreaseVisibilityAttribute +warnif count == 1 + +let tAttr = Types.WithFullName("NDepend.Attributes.CannotDecreaseVisibilityAttribute").FirstOrDefault() +where tAttr != null + +// Get types that do a wrong usage of CannotDecreaseVisibilityAttribute +let types = from t in Application.Types where + t.HasAttribute("NDepend.Attributes.CannotDecreaseVisibilityAttribute".AllowNoMatch()) && + ( t.Visibility == t.OptimalVisibility || + + // Static types that define only const fields cannot be seen as used in IL code. + // They don't need to be tagged with CannotDecreaseVisibilityAttribute. + (t.IsStatic && t.NbMethods == 0 && !t.Fields.Any(f => !f.IsLiteral)) + ) + select t + +// Get methods that do a wrong usage of CannotDecreaseVisibilityAttribute +let methods = from m in Application.Methods where + m.HasAttribute("NDepend.Attributes.CannotDecreaseVisibilityAttribute".AllowNoMatch()) && + m.Visibility == m.OptimalVisibility + select m + +// Get fields that do a wrong usage of CannotDecreaseVisibilityAttribute +let fields = from f in Application.Fields where + f.HasAttribute("NDepend.Attributes.CannotDecreaseVisibilityAttribute".AllowNoMatch()) && + f.Visibility == f.OptimalVisibility + select f + +where types.Count() > 0 || methods.Count() > 0 || fields.Count() > 0 +select new { + tAttr, + types , + methods, + fields +} + +// +// The attribute **NDepend.Attributes.CannotDecreaseVisibilityAttribute** +// is defined in *NDepend.API.dll*. If you don't want to reference +// *NDepend.API.dll* you can declare it in your code. +// +// Usage of this attribute means that a code element visibility is not +// optimal (it can be lowered like for example from *public* to *internal*) +// but shouldn’t be modified anyway. Typical reasons to do so include: +// +// • Public code elements consumed through reflection, through a mocking +// framework, through XML or binary serialization, through designer, +// COM interfaces… +// +// • Non-private code element invoked by test code, that would be difficult +// to reach from test if it was declared as *private*. +// +// In such situation *CannotDecreaseVisibilityAttribute* is used to avoid +// that default rules about not-optimal visibility warn. Using this +// attribute can be seen as an extra burden, but it can also be seen as +// an opportunity to express in code: **Don't change the visibility else +// something will be broken** +// +// In this context, this code rule matches code elements +// (types, methods, fields) that are tagged with this attribute, +// but still already have an optimal visibility. +// + +// +// Just remove *CannotDecreaseVisibilityAttribute* tagging of +// types, methods and fields matched by this rule +// because this tag is not useful anymore. +//]]> + Event handler methods should be declared private +warnif count > 0 +from m in Application.Methods where + !m.IsPrivate && + !m.IsGeneratedByCompiler && + + // A method is considered as an event handler if… + m.NbParameters == 2 && // … it has two parameters … + m.Name.Contains("Object") && // … of types Object … + m.Name.Contains("EventArgs") && // … and EventArgs + + // Discard special cases + !m.ParentType.IsDelegate && + !m.IsGeneratedByCompiler + +select new { m,m.Visibility } + +// +// Think of a event handler like for example *OnClickButton()*. +// Typically such method must be declared as private +// and shouldn't be invoked in other context than event firing. +// + +// +// If you have the need that event handler method should be called +// from another class, then find a code structure that more +// closely matches the concept of what you're trying to do. +// Certainly you don't want the other class to click a button; you +// want your other class to do something that clicking a button +// also do. +//]]> + + + Fields should be marked as ReadOnly when possible +warnif count > 0 +from f in JustMyCode.Fields where + f.IsImmutable && + !f.IsInitOnly && // The condition IsInitOnly matches fields that + // are marked with the C# readonly keyword + // (ReadOnly in VB.NET). + !f.IsGeneratedByCompiler && + !f.IsEventDelegateObject +select new { f, f.MethodsReadingMeButNotAssigningMe, f.MethodsAssigningMe } + +// +// This rule warns about instance and static fields that +// can be declared as **readonly**. +// +// This source code of this rule is based on the conditon +// *IField.IsImmutable*. +// http://www.ndepend.com/api/webframe.html?NDepend.API~NDepend.CodeModel.IField~IsImmutable.html +// +// A field that matches the condition *IsImmutable* +// is a field that is assigned only by constructors +// of its class. +// +// For an *instance field*, this means its value +// will remain constant through the lifetime +// of the object. +// +// For a *static field*, this means its value will +// remain constant through the lifetime of the +// program. +// + +// +// Declare the field with the C# *readonly* keyword +// (*ReadOnly* in VB.NET). This way the intention +// that the field value shouldn't change is made +// explicit. +//]]> + Structures should be immutable +warnif count > 0 from t in Application.Types where + t.IsStructure && + !t.IsImmutable + +let mutableFields = t.Fields.Where(f => !f.IsImmutable) + +select new { t, t.NbLinesOfCode, mutableFields } + +// +// An object is immutable if its state doesn’t change once the +// object has been created. Consequently, a structure or a class +// is immutable if its instances fields are only assigned inline +// or from constructor(s). +// +// But for structure it is a bit different. **Structures are value +// types** which means instances of structures are copied when they are +// passed around (like through a method argument). +// +// So if you change a copy you are changing only that copy, not the +// original and not any other copies which might be around. Such +// situation is very different than what happen with instances of +// classes. Hence developers are not used to work with modified values +// and doing so introduces *confusion* and is *error-prone*. +// + +// +// Make sure matched structures are immutable. This way, all +// automatic copies of an original instance, resulting from being +// *passed by value* will hold the same values and there will be +// no surprises. +// +// If your structure is immutable then if you want to change +// a value, you have to consciously do it by creating a new instance +// of the structure with the modified data. +//]]> + Property Getters should be immutable +warnif count > 0 from m in Application.Methods where + !m.IsGeneratedByCompiler && + m.IsPropertyGetter && + ( ( !m.IsStatic && m.ChangesObjectState) || + ( m.IsStatic && m.ChangesTypeState) ) + +let fieldsAssigned = m.FieldsAssigned + +select new { m, m.NbLinesOfCode, fieldsAssigned } + +// +// It is not expected that a state gets modified when +// accessing a property getter. Hence doing so create +// confusion and property getters should be pure methods, +// they shouldn't assign any field. +// + +// +// Make sure that matched property getters don't assign any +// field. +// +// Notice the case of an object *lazy-initialized* when +// the property getter is accessed the first time. In such +// situation the getter assigns a field, but it is only once +// and from the client point of view, lazy initialization +// is an invisible implementation detail. If you are using +// a lot lazy initalization but still want to use the +// present rule, an option is to create a +// *LazyInitializerAttribute* that tags concerned property +// getters, and modify the present rule to avoid matching them. +//]]> + Avoid static fields with a mutable field type +warnif count > 0 +from f in Application.Fields +where f.IsStatic && + !f.IsEnumValue && + !f.IsGeneratedByCompiler + && !f.IsLiteral +let fieldType = f.FieldType +where fieldType != null && + !fieldType.IsThirdParty && + !fieldType.IsInterface && + !fieldType.IsImmutable +select new { + f, + mutableFieldType = fieldType , + isFieldImmutable = f.IsImmutable, + isFieldIsReadOnly = f.IsInitOnly } + +// +// This rule warns about static fields that can be assigned +// (i.e they are not declared as *readonly*). This rule +// also warns about *readonly* static fields whose field type +// is mutable. In both cases, each matched static field is +// holding a state that can be modified. +// +// In *Object-Oriented-Programming* the natural artifact +// to hold states that can be modified is **instance fields**. +// Hence such static fields create *confusion* at runtime. +// +// More discussion on the topic can be found here: +// http://codebetter.com/patricksmacchia/2011/05/04/back-to-basics-usage-of-static-members/ +// + +// +// If the *static* field is just assigned once in the program +// lifetime, make sure to declare it as *readonly* and assign +// it inline, or from the static constructor. +// +// Else, to fix violations of this rule, make sure to +// hold mutable states through objects that are passed +// **explicitly** everywhere they need to be consumed, in +// opposition to mutable object hold by a static field that +// makes it modifiable from a bit everywhere in the program. +//]]> + A field must not be assigned from outside its parent hierarchy types + +warnif count > 0 +from f in JustMyCode.Fields.Where(f => + (f.IsInternal || f.IsPublic) && + !f.IsGeneratedByCompiler && + !f.IsImmutable && + !f.IsEnumValue) + +let methodsAssignerOutsideOfMyType = f.MethodsAssigningMe.Where( + m =>!m.IsGeneratedByCompiler && + m.ParentType != f.ParentType && + !m.ParentType.DeriveFrom(f.ParentType) ) + +where methodsAssignerOutsideOfMyType.Count() > 0 +select new { + f, + methodsAssignerOutsideOfMyType +} + +// +// This rule is related to the rule *Fields should be declared as +// private*. It matches any **public or internal, mutable field** +// that is assigned from outside its parent class and subclasses. +// +// Fields should be considered as **implementation details** +// and as a consequence they should be declared as private. +// +// If something goes wrong with a *non-private field*, +// the culprit can be anywhere, and so in order to track down +// the bug, you may have to look at quite a lot of code. +// +// A private field, by contrast, can only be assigned from +// inside the same class, so if something goes wrong with that, +// there is usually only one source file to look at. +// + +// +// Matched fields must be declared as *protected* and even better +// as *private*. Alternatively, if the field can reference +// immutable states, it can remain visible from the outside, +// but then must be declared as *readonly*. +//]]> + Don't assign a field from many methods + +warnif count > 0 +from f in JustMyCode.Fields where + !f.IsEnumValue && + !f.IsImmutable && + !f.IsInitOnly && + !f.IsGeneratedByCompiler && + !f.IsEventDelegateObject + +let methodsAssigningMe = f.MethodsAssigningMe.Where(m => !m.IsConstructor) + +// The threshold 4 is arbitrary and it should avoid matching too many fields. +// Threshold is even lower for static fields because this reveals situations even more complex. +where methodsAssigningMe.Count() >= (!f.IsStatic ? 4 : 2) +select new { + f, + methodsAssigningMe, + f.MethodsReadingMeButNotAssigningMe, + f.MethodsUsingMe +} + +// +// A field assigned from many methods is a symptom of **bug-prone code**. +// Notice that: +// +// • For an instance field, constructor(s) of its class that assign the field are not counted. +// +// • For a static field, the class constructor that assigns the field is not counted. +// +// The default threshold for *instance fields* is *assigned by more than 4 +// methods*. Such situation makes harder to anticipate the field state during +// runtime. The code is then complicated to read, hard to debug and hard to +// maintain. Hard-to-solve bugs due to corrupted state are often the +// consequence of fields *anarchically assigned*. +// +// The situation is even more complex if the field is *static*. +// Indeed, such situation potentially involves global random accesses from +// various parts of the application. This is why this rule provides a lower +// threshold equals to *assigned by more than 2 methods* for static fields. +// +// If the object containing such field is meant to be used from multiple threads, +// there are **alarming chances** that the code is unmaintainable and bugged. +// When multiple threads are involved, the rule of thumb is to use immutable objects. +// +// If the field type is a reference type (interfaces, classes, strings, delegates) +// corrupted state might result in a *NullReferenceException*. +// If the field type is a value type (number, boolean, structure) +// corrupted state might result in wrong result not even signaled by an exception +// thrown. +// + +// +// There is no straight advice to refactor the number of methods responsible +// for assigning a field. Solutions often involve rethinking and then rewriting +// a complex algorithm. Such field can sometime become just a variable accessed +// locally by a method or a *closure*. Sometime, just rethinking the life-time +// and the role of the parent object allows the field to become immutable +// (i.e assigned only by the constructor). +//]]> + Do not declare read only mutable reference types + +warnif count > 0 from f in JustMyCode.Fields where + f.IsInitOnly && + !f.ParentType.IsPrivate && + !f.IsPrivate && + f.FieldType != null && + f.FieldType.IsClass && + !f.FieldType.IsThirdParty && + !f.FieldType.IsImmutable +select new { + f, + f.FieldType, + FieldVisibility = f.Visibility +} + +// +// This rule is violated when a *public* or *internal* +// type contains a *public* or *internal* read-only field +// whose field type is a mutable reference type. +// +// This situation provides the wrong impression that the +// value can't change, when actually it's only the field +// value that can't change, but the object state +// can still change. +// + +// +// To fix a violation of this rule, +// replace the field type with an immutable type, +// or declare the field as *private*. +//]]> + Array fields should not be read only + +warnif count > 0 from f in Application.Fields where + f.IsInitOnly && + f.IsPubliclyVisible && + f.FieldType != null && + f.FieldType.FullName == "System.Array" +select new { + f, + FieldVisibility = f.Visibility +} + +// +// This rule is violated when a publicly visible field +// that holds an array, is declared read-only. +// +// This situation represents a *security vulnerability*. +// Because the field is read-only it cannot be changed to refer +// to a different array. However, the elements of the array +// that are stored in a read-only field can be changed. +// Code that makes decisions or performs operations that are +// based on the elements of a read-only array that can be publicly +// accessed might contain an exploitable security vulnerability. +// + +// +// To fix the security vulnerability that is identified by +// this rule do not rely on the contents of a read-only array +// that can be publicly accessed. It is strongly recommended +// that you use one of the following procedures: +// +// • Replace the array with a strongly typed collection +// that cannot be changed. See for example: +// *System.Collections.Generic.IReadOnlyList* ; +// *System.Collections.Generic.IReadOnlyCollection* ; +// *System.Collections.ReadOnlyCollectionBase* +// +// • Or replace the public field with a method that returns a clone +// of a private array. Because your code does not rely on +// the clone, there is no danger if the elements are modified. +//]]> + Types tagged with ImmutableAttribute must be immutable +warnif count > 0 +from t in Application.Types where + t.HasAttribute ("NDepend.Attributes.ImmutableAttribute".AllowNoMatch()) && + !t.IsImmutable +let culpritFields = t.Fields.Where(f => !f.IsStatic && !f.IsImmutable) +select new { + t, + culpritFields +} + +// +// An object is immutable if its state doesn’t change once the +// object has been created. Consequently, a structure or a class +// is immutable if its instances fields are only assigned inline +// or from constructor(s). +// +// An attribute **NDepend.Attributes.ImmutableAttribute** can be +// used to express in code that a type is immutable. In such +// situation, the present code rule checks continuously that the +// type remains immutable whatever the modification done. +// +// This rule warns when a type that is tagged with +// *ImmutableAttribute* is actually not immutable anymore. +// +// Notice that *FullCoveredAttribute* is defined in *NDepend.API.dll* +// and if you don't want to link this assembly, you can create your +// own *FullCoveredAttribute* and adapt the rule. +// + +// +// First understand which modification broke the type immutability. +// The list of *culpritFields* provided in this rule result can help. +// Then try to refactor the type to bring it back to immutability. +//]]> + Types immutable should be tagged with ImmutableAttribute +from t in Application.Types where + !t.HasAttribute ("NDepend.Attributes.ImmutableAttribute".AllowNoMatch()) && + t.IsImmutable +select new { t, t.NbLinesOfCode } + +// +// An object is immutable if its state doesn’t change once the +// object has been created. Consequently, a structure or a class +// is immutable if its instances fields are only assigned inline +// or from constructor(s). +// +// This code query lists immutable type that are not tagged with +// an **ImmutableAttribute**. By using such attribute, you can express +// in source code the intention that a class is immutable, and +// should remain immutable in the future. Benefits of using +// an **ImmutableAttribute** are twofold: +// +// • Not only the intention is expressed in source code, +// +// • but it is also continuously checked by the rule +// *Types tagged with ImmutableAttribute must be immutable*. +// + +// +// Just tag types matched by this code query with **ImmutableAttribute** +// that can be found in *NDepend.API.dll*, +// or by an attribute of yours defined in your own code +// (in which case this code query must be adapted). +//]]> + Methods tagged with PureAttribute must be pure + +warnif count > 0 from m in Application.Methods where + ( m.HasAttribute ("NDepend.Attributes.PureAttribute".AllowNoMatch()) || + m.HasAttribute ("System.Diagnostics.Contract.PureAttribute".AllowNoMatch()) ) && + ( m.ChangesObjectState || m.ChangesTypeState ) && + m.NbLinesOfCode > 0 + +let fieldsAssigned = m.FieldsAssigned + +select new { m, m.NbLinesOfCode, fieldsAssigned } + +// +// A method is pure if its execution doesn’t change +// the value of any instance or static field. +// Pure methods naturally simplify code by **limiting +// side-effects**. +// +// An attribute **PureAttribute** can be +// used to express in code that a method is pure. In such +// situation, the present code rule checks continuously that the +// method remains pure whatever the modification done. +// +// This rule warns when a method that is tagged with +// *PureAttribute* is actually not pure anymore. +// +// Notice that *NDepend.Attributes.PureAttribute* is defined +// in *NDepend.API.dll* and if you don't want to link this +// assembly, you can also use +// *System.Diagnostics.Contract.PureAttribute* +// or create your own *PureAttribute* and adapt the rule. +// +// Notice that *System.Diagnostics.Contract.PureAttribute* is +// taken account by the compiler only when the VS project has +// Microsoft Code Contract enabled. +// + +// +// First understand which modification broke the method purity. +// Then refactor the method to bring it back to purity. +//]]> + Pure methods should be tagged with PureAttribute +// warnif count > 0 <-- not a cod rule per default + +from m in Application.Methods where + !m.IsGeneratedByCompiler && + !m.HasAttribute ("NDepend.Attributes.PureAttribute".AllowNoMatch()) && + !m.HasAttribute ("System.Diagnostics.Contract.PureAttribute".AllowNoMatch()) && + !m.ChangesObjectState && !m.ChangesTypeState && + m.NbLinesOfCode > 0 +select new { m, m.NbLinesOfCode } + +// +// A method is pure if its execution doesn’t change +// the value of any instance or static field. +// Pure methods naturally simplify code by **limiting +// side-effects**. +// +// This code query lists pure methods that are not tagged with +// a **PureAttribute**. By using such attribute, you can express +// in source code the intention that a method is pure, and +// should remain pure in the future. Benefits of using +// a **PureAttribute** are twofold: +// +// • Not only the intention is expressed in source code, +// +// • but it is also continuously checked by the rule +// *Methods tagged with PureAttribute must be pure*. +// +// This code query is not by default defined as a code rule +// because certainly many of the methods of the code base +// are matched. Hence fixing all matches and then +// maintaining the rule unviolated might require a lot of +// work. This may *counter-balance* such rule benefits. +// + +// +// Just tag methods matched by this code query with +// *NDepend.Attributes.PureAttribute* +// that can be found in *NDepend.API.dll*, +// or with *System.Diagnostics.Contract.PureAttribute*, +// or with an attribute of yours defined in your own code +// (in which case this code query must be adapted). +// +// Notice that *System.Diagnostics.Contract.PureAttribute* is +// taken account by the compiler only when the VS project has +// Microsoft Code Contract enabled. +//]]> + + + Instance fields should be prefixed with a 'm_' +warnif count > 0 from f in Application.Fields where + !f.NameLike(@"^m_") && + !f.IsStatic && + !f.IsLiteral && + !f.IsGeneratedByCompiler && + !f.IsSpecialName && + !f.IsEventDelegateObject +select new { f, f.SizeOfInst } + +// +// In terms of behavior, a *static field* is something completely different +// than an *instance field*, so it is interesting to differentiate them at +// a glance through a naming convetion. +// +// The present rule enforces **m_** prefix for *instance fields* names. +// To do so, the rule relies on a **f.NameLike(@"^m_")** *regular expression*. +// You can easily adapt the *regular expression* to your own naming convention. +// +// Related discussion: +// http://codebetter.com/patricksmacchia/2013/09/04/on-hungarian-notation-for-instance-vs-static-fields-naming/ +// + +// +// Once the rule has been adapted to your own naming convention +// make sure to name all matched instance fields adequately. +//]]> + Static fields should be prefixed with a 's_' +warnif count > 0 from f in Application.Fields where + !f.NameLike (@"^s_") && + f.IsStatic && + !f.IsLiteral && + !f.IsGeneratedByCompiler && + !f.IsSpecialName && + !f.IsEventDelegateObject +select new { f, f.SizeOfInst } + +// +// In terms of behavior, a *static field* is something completely different +// than an *instance field*, so it is interesting to differentiate them at +// a glance through a naming convetion. +// +// The present rule enforces **s_** prefix for *static fields* names. +// To do so, the rule relies on a **f.NameLike(@"^s_")** *regular expression*. +// You can easily adapt the *regular expression* to your own naming convention. +// +// Related discussion: +// http://codebetter.com/patricksmacchia/2013/09/04/on-hungarian-notation-for-instance-vs-static-fields-naming/ +// + +// +// Once the rule has been adapted to your own naming convention +// make sure to name all matched static fields adequately. +//]]> + Interface name should begin with a 'I' + +warnif count > 0 from t in Application.Types where + t.IsInterface && + // Don't apply this rule for COM interfaces. + !t.HasAttribute("System.Runtime.InteropServices.ComVisibleAttribute".AllowNoMatch()) + +// Discard outter type(s) name prefix for nested types +let name = !t.IsNested ? + t.Name : + t.Name.Substring(t.Name.LastIndexOf('+') + 1, t.Name.Length - t.Name.LastIndexOf('+') - 1) + +where name[0] != 'I' +select t + +// +// In the .NET world, interfaces names are commonly prefixed +// with an upper case **I**. This rule warns about interfaces +// whose names don't follow this convention. Because this +// naming convention is widely used and accepted, we +// recommend abiding by this rule. +// +// Typically COM interfaces names don't follow this rule. +// Hence this code rule doesn't take care of interfaces tagged +// with *ComVisibleAttribute*. +// + +// +// Make sure that matched interfaces names are prefixed with +// an upper **I**. +//]]> + Abstract base class should be suffixed with 'Base' + +warnif count > 0 from t in Application.Types where + t.IsAbstract && + t.IsClass && + + t.BaseClass != null && + t.BaseClass.FullName == "System.Object" && + + ((!t.IsGeneric && !t.NameLike (@"Base$")) || + ( t.IsGeneric && !t.NameLike (@"Base<"))) +select t + +// +// This rule warns about *abstract classes* whose names are not +// suffixed with **Base**. It is a common practice in the .NET +// world to suffix base classes names with **Base**. +// +// Notice that this rule doesn't match +// abstract classes that are in a middle of a hierarchy chain. +// Only base classes that derive directly from *System.Object* +// are matched. +// + +// +// Suffix the names of matched base classes with **Base**. +//]]> + Exception class name should be suffixed with 'Exception' +warnif count > 0 from t in Application.Types where + t.IsExceptionClass && + + // We use SimpleName, because in case of generic Exception type + // SimpleName suppresses the generic suffix (like ). + !t.SimpleNameLike(@"Exception$") && + !t.SimpleNameLike(@"ExceptionBase$") // Allow the second suffix Base + // for base exception classes. +select t + +// +// This rule warns about *exception classes* whose names are not +// suffixed with **Exception**. It is a common practice in the .NET +// world to suffix exception classes names with **Exception**. +// +// For exception base classes, the suffix **ExceptionBase** +// is also accepted. +// + +// +// Suffix the names of matched exception classes with **Exception**. +//]]> + Attribute class name should be suffixed with 'Attribute' +warnif count > 0 from t in Application.Types where + t.IsAttributeClass && + !t.NameLike (@"Attribute$") +select t + +// +// This rule warns about *attribute classes* whose names are not +// suffixed with **Attribute**. It is a common practice in the .NET +// world to suffix attribute classes names with **Attribute**. +// + +// +// Suffix the names of matched attribute classes with **Attribute**. +//]]> + Types name should begin with an Upper character + +warnif count > 0 from t in JustMyCode.Types where + // The name of a type should begin with an Upper letter. + !t.SimpleNameLike (@"^[A-Z]") && + + // Except if it is generated by compiler. + !t.IsSpecialName && + !t.IsGeneratedByCompiler + +select new { + t, + // We show the type simple name + // that doesn't include the parent type name + // for nested types. + t.SimpleName } + +// +// This rule warns about *types* whose names don't start +// with an Upper character. It is a common practice in the .NET +// world to use **Pascal Casing Style** to name types. +// +// **Pascal Casing Style** : The first letter in the identifier +// and the first letter of each subsequent concatenated word +// are capitalized. For example: *BackColor* +// + +// +// *Pascal Case* the names of matched types. +//]]> + Methods name should begin with an Upper character + +warnif count > 0 from m in JustMyCode.Methods where + !m.NameLike (@"^[A-Z]") && + !m.IsSpecialName && + !m.IsGeneratedByCompiler +select m + +// +// This rule warns about *methods* whose names don't start +// with an Upper character. It is a common practice in the .NET +// world to use **Pascal Casing Style** to name methods. +// +// **Pascal Casing Style** : The first letter in the identifier +// and the first letter of each subsequent concatenated word +// are capitalized. For example: *ComputeSize* +// + +// +// *Pascal Case* the names of matched methods. +//]]> + Do not name enum values 'Reserved' + +warnif count > 0 from f in Application.Fields where + f.IsEnumValue && + f.NameLike (@"Reserved") +select f + +// +// This rule assumes that an enumeration member +// with a name that contains **"Reserved"** +// is not currently used but is a placeholder to +// be renamed or removed in a future version. +// Renaming or removing a member is a breaking +// change. You should not expect users to ignore +// a member just because its name contains +// **"Reserved"** nor can you rely on users to read or +// abide by documentation. Furthermore, because +// reserved members appear in object browsers +// and smart integrated development environments, +// they can cause confusion as to which members +// are actually being used. +// +// Instead of using a reserved member, add a +// new member to the enumeration in the future +// version. +// +// In most cases, the addition of the new +// member is not a breaking change, as long as the +// addition does not cause the values of the +// original members to change. +// + +// +// To fix a violation of this rule, remove or +// rename the member. +//]]> + Avoid types with name too long + +warnif count > 0 from t in Application.Types +where !t.IsGeneratedByCompiler + +where t.SimpleName.Length > 35 +select new { t, t.SimpleName } + +// +// Types with a name too long tend to decrease code readability. +// This might also be an indication that a type is doing too much. +// +// This rule matches types with names with more than 35 characters. +// + +// +// To fix a violation of this rule, rename the type with a shortest name +// or eventually split the type in several more fine-grained types. +//]]> + Avoid methods with name too long +warnif count > 0 from m in Application.Methods where + + // Explicit Interface Implementation methods are + // discarded because their names are prefixed + // with the interface name. + !m.IsExplicitInterfaceImpl && + !m.IsGeneratedByCompiler && + ((!m.IsSpecialName && m.SimpleName.Length > 35) || + // Property getter/setter are prefixed with "get_" "set_" of length 4. + ( m.IsSpecialName && m.SimpleName.Length - 4 > 35)) + +select new { m, m.SimpleName } + +// +// Methods with a name too long tend to decrease code readability. +// This might also be an indication that a method is doing too much. +// +// This rule matches methods with names with more than 35 characters. +// + +// +// To fix a violation of this rule, rename the method with a shortest name +// that equally conveys the behavior of the method. +// Or eventually split the method into several smaller methods. +//]]> + Avoid fields with name too long +warnif count > 0 from f in Application.Fields where + !f.IsGeneratedByCompiler && + f.Name.Length > 35 +select f + +// +// Fields with a name too long tend to decrease code readability. +// +// This rule matches fields with names with more than 35 characters. +// + +// +// To fix a violation of this rule, rename the field with a shortest name +// that equally conveys the same information. +//]]> + Avoid having different types with same name +warnif count > 0 + +// This rule matches also collisions between +// application and third-party types sharing a same name. +let groups = JustMyCode.Types.Union(ThirdParty.Types) + // Discard nested types, whose name is + // prefixed with the parent type name. + .Where(t => !t.IsNested) + + // Group types by name. + .GroupBy(t => t.Name) + +from @group in groups + where @group.Count() > 1 + + // Let's see if types with the same name are declared + // in different namespaces. + // (t.FullName is {namespaceName}.{typeName} ) + let groupsFullName = @group.GroupBy(t => t.FullName) + where groupsFullName.Count() > 1 + + // If several types with same name are declared in different namespaces + // eliminate the case where all types are declared in third-party assemblies. + let types= groupsFullName.SelectMany(g => g) + where types.Any(t => !t.IsThirdParty) + // Uncomment this line, to only gets naming collision involving + // both application and third-party types. + // && types.Any(t => t.IsThirdParty) + +orderby types.Count() descending + +select new { + t = types.First(), + + // In the 'types' column, make sure to group matched types + // by parent assemblies and parent namespaces, to get a result + // more readable. + types +} + +// +// This rule warns about multiple types with same name, +// that are defined in different *application* or +// *third-party* namespaces or assemblies. +// +// Such practice create confusion and also naming collision +// in source files that use different types with same name. +// + +// +// To fix a violation of this rule, rename concerned types. +//]]> + Avoid prefixing type name with parent namespace name +warnif count > 0 + +from n in Application.Namespaces +where n.Name.Length > 0 + +from t in n.ChildTypes +where + !t.IsGeneratedByCompiler && + !t.IsNested && + t.Name.IndexOf(n.SimpleName) == 0 +select new { + t, + namespaceName = n.SimpleName +} + +// +// This rule warns about situations where the parent namespace name +// is used as the prefix of a contained type. +// +// For example a type named "RuntimeEnvironment" +// declared in a namespace named "Foo.Runtime" +// should be named "Environment". +// +// Such situation creates naming redundancy with no readability gain. +// + +// +// To fix a violation of this rule, remove the prefix from the type name. +//]]> + Avoid naming types and namespaces with the same identifier +warnif count > 0 +let hashsetShortNames = Namespaces.Where(n => n.Name.Length > 0).Select(n => n.SimpleName).ToHashSet() + +from t in JustMyCode.Types +where hashsetShortNames.Contains(t.Name) +select new { + t, + namespaces = Namespaces.Where(n => n.SimpleName == t.Name) +} + +// +// This rule warns when a type and a namespace have the same name. +// +// For example when a type is named *Environment* +// and a namespace is named *Foo.Environment*. +// +// Such situation provokes tedious compiler resolution collision, +// and makes the code less readable because concepts are not +// concisely identified. +// + +// +// To fix a violation of this rule, renamed the concerned type or namespace. +//]]> + Don't call your method Dispose +warnif count > 0 +from m in JustMyCode.Methods.WithSimpleName("Dispose") +where !m.ParentType.Implement("System.IDisposable".AllowNoMatch()) +select m + +// +// In .NET programming, the identifier *Dispose()* should be kept +// only for implementations of *System.IDisposable*. +// +// This rule warns when a method is named *Dispose()*, +// but the parent type doesn't implement *System.IDisposable*. +// + +// +// To fix a violation of this rule, +// either make the parent type implements *System.IDisposable*, +// or rename the *Dispose()* method with another identifier: +// *Close() Terminate() Finish() Quit() Exit() Unlock() ShutDown()*… +//]]> + Methods prefixed with 'Try' should return a boolean +warnif count > 0 +from m in Application.Methods where + m.SimpleNameLike("^Try") && + m.ReturnType != null && + m.ReturnType.FullName != "System.Boolean" +select new { + m, + m.ReturnType +} + +// +// When a method has a name prefixed with **Try**, it is expected that +// it returns a *boolean*, that reflects the method execution status, +// *success* or *failure*. +// +// Such method usually returns a result through an *out parameter*. +// For example: *System.Int32.TryParse(int,out string):bool* +// + +// +// To fix a violation of this rule, +// Rename the method, or transform it into an operation that can fail. +//]]> + + + Avoid referencing source file out of Visual Studio project directory +warnif count > 0 + +from a in Application.Assemblies +where a.VisualStudioProjectFilePath != null +let vsProjDirPathLower = a.VisualStudioProjectFilePath.ParentDirectoryPath.ToString().ToLower() + +from t in a.ChildTypes +where JustMyCode.Contains(t) && t.SourceFileDeclAvailable + +from decl in t.SourceDecls +let sourceFilePathLower = decl.SourceFile.FilePath.ToString().ToLower() +where sourceFilePathLower.IndexOf(vsProjDirPathLower) != 0 +select new { + t, + sourceFilePathLower, + projectFilePath = a.VisualStudioProjectFilePath.ToString() +} + +// +// A source file located outside of the VS project directory can be added through: +// *> Add > Existing Items… > Add As Link* +// +// Doing so can be used to share types definitions across several assemblies. +// This provokes type duplication at binary level. +// Hence maintainability is degraded and subtle versioning bug can appear. +// +// This rule matches types whose source files are not declared under the +// directory that contains the related Visual Studio project file, or under +// any sub-directory of this directory. +// +// This practice can be tolerated for certain types shared across executable assemblies. +// Such type can be responsible for startup related concerns, +// such as registering custom assembly resolving handlers or +// checking the .NET Framework version before loading any custom library. +// + +// +// To fix a violation of this rule, prefer referencing from a VS project +// only source files defined in sub-directories of the VS project file location. +//]]> + Avoid duplicating a type definition across assemblies +warnif count > 0 + +let groups = Application.Types + .Where(t => !t.IsGeneratedByCompiler) + .GroupBy(t => t.FullName) +from @group in groups +where @group.Count() > 1 + +select new { + t = @group.First(), + // In the 'types' column, make sure to group matched types by parent assemblies. + typesDefs = @group.ToArray() +} + +// +// A source file located outside of the VS project directory can be added through: +// *> Add > Existing Items… > Add As Link* +// +// This rule warns about using this feature to share code across several assemblies. +// This provokes type duplication at binary level. +// Hence maintainability is degraded and subtle versioning bug can appear. +// +// This practice can be tolerated for certain types shared across executable assemblies. +// Such type can be responsible for startup related concerns, +// such as registering custom assembly resolving handlers or +// checking the .NET Framework version before loading any custom library. +// + +// +// To fix a violation of this rule, prefer sharing types through DLLs. +//]]> + Avoid defining multiple types in a source file +warnif count > 0 + +// Build a lookup indexed by source files, values being a sequence of types defined in the source file. +let lookup = Application.Types.Where(t => + t.SourceFileDeclAvailable && + // except nested types and types generated by compilers! + !t.IsGeneratedByCompiler && + !t.IsNested) + // It could make sense to not apply this rule for enumerations. + // && !t.IsEnumeration) + + // We use multi-key, since a type can be declared in multiple source files. + .ToMultiKeyLookup(t => t.SourceDecls.Select(d => d.SourceFile)) + +from @group in lookup where @group.Count() > 1 + let sourceFile = @group.Key + + // CQLinq doesn't let indexing result with sourceFile + // so we choose a typeIndex in types, + // preferably the type that has the file name. + let typeWithSourceFileName = @group.FirstOrDefault(t => t.SimpleName == sourceFile.FileNameWithoutExtension) + let typeIndex = typeWithSourceFileName ?? @group.First() + +select new { + typeIndex, + TypesInSourceFile = @group as IEnumerable, + SourceFilePathString = sourceFile.FilePathString +} + +// +// Defining multiple types in a single source file decreases code readability, +// because developers are used to see all types in a namespace, +// when expanding a folder in the *Visual Studio Solution Explorer*. +// Also doing so, leads to source files with too many lines. +// +// Each match of this rule is a source file that contains several types +// definitions, indexed by one of those types, preferably the one with +// the same name than the source file name without file extension, if any. +// + +// +// To fix a violation of this rule, create a source file for each type. +//]]> + Namespace name should correspond to file location + +warnif count > 0 +from n in Application.Namespaces + +// Replace dots by spaces in namespace name +let dirCorresponding = n.Name.Replace('.', ' ') + +// Look at source file decl of JustMyCode type's declared in n +from t in n.ChildTypes +where JustMyCode.Contains(t) && t.SourceFileDeclAvailable +from decl in t.SourceDecls +let sourceFilePath = decl.SourceFile.FilePath.ToString() + +// Replace dots and path separators by spaces in source files names +where !sourceFilePath.Replace('.',' ').Replace('\\',' ').Contains(dirCorresponding) + +select new { + t, + dirCorresponding , + sourceFilePath +} + +// +// For a solid code structure and organization, +// do mirror the namespaces hierarchy and the directories hierarchy containing source files. +// +// Doing so is a widely accepted convention, and not respecting this convention +// will lead to less maintainable and less browsable source code. +// +// This rule matches all types in such source file, whose location doesn't correspond +// to the type parent namespace. If a source file contains several such types (that +// are not necessarily in the same namespace) each type will result in a violation. +// + +// +// To fix a violation of this rule, make sure that the type parent namespace and +// the directory sub-paths that contains the type source file, are mirrored. +//]]> + Types with source files stored in the same directory, should be declared in the same namespace +warnif count > 0 + +// Group JustMyCode types in a lookup +// where groups are keyed with directories that contain the types' source file(s). +// Note that a type can be contained in several groups +// if it is declared in several source files stored in different directories. +let lookup = JustMyCode.Types.Where(t => t.SourceFileDeclAvailable) + .ToMultiKeyLookup( + t => t.SourceDecls.Select( + decl => decl.SourceFile.FilePath.ParentDirectoryPath).Distinct() + ) + +from groupOfTypes in lookup +let parentNamespaces = groupOfTypes.ParentNamespaces() + +// Select group of types (with source files stored in the same directory) … +// … but contained in several namespaces +where parentNamespaces.Count() > 1 + +// mainNamespaces is the namespace that contains many types +// declared in the directory groupOfTypes .key +let mainNamespace = groupOfTypes + .ToLookup(t => t.ParentNamespace) + .OrderByDescending(g => g.Count()).First().Key + +// Select types with source files stored in the same directory, +// but contained in namespaces different than mainNamespace. +let typesOutOfMainNamespace = groupOfTypes + .Where(t => t.ParentNamespace != mainNamespace && + t.ParentAssembly == mainNamespace.ParentAssembly) + + // Filter types declared on several source files that contain generated methods + // because typically such type contains one or several partial definitions generated. + // These partially generated types would be false positive for the present rule. + .Where(t => t.SourceDecls.Count() == 1 || + t.Methods.Count(m => JustMyCode.Contains(m)) == 0) +where typesOutOfMainNamespace.Count() > 0 + +let typesInMainNamespace = groupOfTypes.Where(t => t.ParentNamespace == mainNamespace) + +select new { + mainNamespace, + typesOutOfMainNamespace, + typesInMainNamespace } + +// +// For a solid code structure and organization, do mirror the namespaces +// hierarchy and the directories hierarchy containing source files. +// +// Doing so is a widely accepted convention, and not respecting this convention +// will lead to less maintainable and less browsable code. +// +// Respecting this convention means that types with source files stored in the same directory, +// should be declared in the same namespace. +// +// For each directory that contains several source files, where most types are declared +// in a namespace (what we call the **main namespace**) and a few types are declared +// out of the *main namespace*, this code rule matches: +// +// • The *main namespace* +// +// • **typesOutOfMainNamespace**: Types declared in source files in the *main namespace*'s directory +// but that are not in the *main namespace*. +// +// • *typesInMainNamespace*: And for informational purposes, types declared in source files in the +// *main namespace*'s directory, and that are in the *main namespace*. +// + +// +// Violations of this rule are types in the *typesOutOfMainNamespace* column. +// Typically such type … +// +// • … is contained in the wrong namespace but its source file is stored in the right directory. +// In such situation the type should be contained in *main namespace*. +// +// • … is contained in the right namespace but its source file is stored in the wrong directory +// In such situation the source file of the type must be moved to the proper parent namespace directory. +// +// • … is declared in multiple source files, stored in different directories. +// In such situation it is preferable that all source files are stored in a single directory. +//]]> + Types declared in the same namespace, should have their source files stored in the same directory + +warnif count > 0 +from @namespace in Application.Namespaces + +// Group types of @namespace in a lookup +// where groups are keyed with directories that contain the types' source file(s). +// Note that a type can be contained in several groups +// if it is declared in several source files stored in different directories. +let lookup = @namespace.ChildTypes.Where(t => t.SourceFileDeclAvailable && JustMyCode.Contains(t)) + .ToMultiKeyLookup( + t => t.SourceDecls.Select( + decl => decl.SourceFile.FilePath.ParentDirectoryPath).Distinct() + ) + +// Are types of @namespaces declared in more than one directory? +where lookup.Count > 1 + +// Infer the main folder, preferably the one that has the same name as the namespace. +let dirs = lookup.Select(types => types.Key) +let mainDirNullable = dirs.Where(d => d.DirectoryName == @namespace.SimpleName).FirstOrDefault() +let mainDir = mainDirNullable ?? dirs.First() + +// Types declared out of mainDir, are types in group of types declared in a directory different than mainDir! +let typesDeclaredOutOfMainDir = + lookup.Where(types => types.Key != mainDir) + .SelectMany(types => types) + + // Filter types declared on several source files that contain generated methods + // because typically such type contains one or several partial definitions generated. + // These partially generated types would be false positive for the present rule. + .Where(t => t.SourceDecls.Count() == 1 || + t.Methods.Count(m => JustMyCode.Contains(m)) == 0) + +where typesDeclaredOutOfMainDir.Count() > 0 + +let typesDeclaredInMainDir = + lookup.Where(types => types.Key == mainDir) + .SelectMany(types => types) + +select new { + @namespace, + typesDeclaredOutOfMainDir , + mainDir = mainDir.ToString(), + typesDeclaredInMainDir +} + +// +// For a solid code structure and organization, +// do mirror the namespaces hierarchy and the directories hierarchy containing source files. +// +// Doing so is a widely accepted convention, and not respecting this convention +// will lead to less maintainable and less browsable code. +// +// Respecting this convention means that types declared in the same namespace, +// should have their source files stored in the same directory. +// +// For each namespace that contains types whose source files +// are declared in several directories, infer the **main directory**, +// the directory that naturally hosts source files of types, +// preferably the directory whose name corresponds with the namespace +// name. In this context, this code rule matches: +// +// • The namespace +// +// • **typesDeclaredOutOfMainDir**: types in the namespace whose source files +// are stored out of the *main directory*. +// +// • The *main directory* +// +// • *typesDeclaredInMainDir*: for informational purposes, types declared +// in the namespace, whose source files are stored in the *main directory*. +// + +// +// Violations of this rule are types in the **typesDeclaredOutOfMainDir** column. +// Typically such type… +// +// • … is contained in the wrong namespace but its source file is stored in the right directory. +// In such situation the type should be contained in the namespace corresponding to +// the parent directory. +// +// • … is contained in the right namespace but its source file is stored in the wrong directory. +// In such situation the source file of the type must be moved to the *main directory*. +// +// • … is declared in multiple source files, stored in different directories. +// In such situation it is preferable that all source files are stored in a single directory. +//]]> + + + + Mark ISerializable types with SerializableAttribute +warnif count > 0 + +from t in Application.Types where + t.IsPublic && + !t.IsDelegate && + !t.IsExceptionClass && // Don't match exceptions, since the Exception class + // implements ISerializable, this would generate + // too many false positives. + t.Implement ("System.Runtime.Serialization.ISerializable".AllowNoMatch()) && + !t.HasAttribute ("System.SerializableAttribute".AllowNoMatch()) + +select new { t, t.NbLinesOfCode } + +// +// To be recognized by the CLR as serializable, +// types must be marked with the *SerializableAttribute* +// attribute even if the type uses a custom +// serialization routine through implementation of +// the *ISerializable* interface. +// +// This rule matches types that implement *ISerializable* and +// that are not tagged with *SerializableAttribute*. +// + +// +// To fix a violation of this rule, tag the matched type +// with *SerializableAttribute* . +//]]> + Mark assemblies with assembly version +warnif count > 0 from a in Application.Assemblies where + !a.HasAttribute ("System.Reflection.AssemblyVersionAttribute".AllowNoMatch()) +select a + +// +// The identity of an assembly is composed of the following information: +// +// • Assembly name +// +// • Version number +// +// • Culture +// +// • Public key (for strong-named assemblies). +// +// The .NET Framework uses the version number to uniquely identify an +// assembly, and to bind to types in strong-named assemblies. The +// version number is used together with version and publisher policy. +// By default, applications run only with the assembly version with +// which they were built. +// +// This rule matches assemblies that are not tagged with +// **System.Reflection.AssemblyVersionAttribute**. +// + +// +// To fix a violation of this rule, add a version number to the assembly +// by using the *System.Reflection.AssemblyVersionAttribute* attribute. +//]]> + Mark assemblies with CLSCompliant +warnif count > 0 from a in Application.Assemblies where + !a.HasAttribute ("System.CLSCompliantAttribute".AllowNoMatch()) +select a + +// +// The *Common Language Specification* (CLS) defines naming restrictions, +// data types, and rules to which assemblies must conform if they are to +// be used across programming languages. Good design dictates that all +// assemblies explicitly indicate CLS compliance with **CLSCompliantAttribute**. +// If the attribute is not present on an assembly, the assembly is not compliant. +// +// Notice that it is possible for a CLS-compliant assembly to contain types or +// type members that are not compliant. +// +// This rule matches assemblies that are not tagged with +// **System.CLSCompliantAttribute**. +// + +// +// To fix a violation of this rule, tag the assembly with *CLSCompliantAttribute*. +// +// Instead of marking the whole assembly as non-compliant, you should determine +// which type or type members are not compliant and mark these elements as such. +// If possible, you should provide a CLS-compliant alternative for non-compliant +// members so that the widest possible audience can access all the functionality +// of your assembly. +//]]> + Mark assemblies with ComVisible +warnif count > 0 from a in Application.Assemblies where + !a.HasAttribute ("System.Runtime.InteropServices.ComVisibleAttribute".AllowNoMatch()) +select a + +// +// The **ComVisibleAttribute** attribute determines how COM clients access +// managed code. Good design dictates that assemblies explicitly indicate +// COM visibility. COM visibility can be set for a whole assembly and then +// overridden for individual types and type members. If the attribute is not +// present, the contents of the assembly are visible to COM clients. +// +// This rule matches assemblies that are not tagged with +// **System.Runtime.InteropServices.ComVisibleAttribute**. +// + +// +// To fix a violation of this rule, tag the assembly with *ComVisibleAttribute*. +// +// If you do not want the assembly to be visible to COM clients, set the +// attribute value to **false**. +//]]> + Mark attributes with AttributeUsageAttribute +warnif count > 0 from t in Application.Types where + t.DeriveFrom ("System.Attribute".AllowNoMatch()) && + !t.HasAttribute ("System.AttributeUsageAttribute".AllowNoMatch()) +select t + +// +// When you define a custom attribute, mark it by using **AttributeUsageAttribute** +// to indicate where in the source code the custom attribute can be applied. The +// meaning and intended usage of an attribute will determine its valid locations +// in code. For example, you might define an attribute that identifies the person +// who is responsible for maintaining and enhancing each type in a library, and +// that responsibility is always assigned at the type level. In this case, compilers +// should enable the attribute on classes, enumerations, and interfaces, but should +// not enable it on methods, events, or properties. Organizational policies and +// procedures would dictate whether the attribute should be enabled on assemblies. +// +// The **System.AttributeTargets** enumeration defines the targets that you can +// specify for a custom attribute. If you omit *AttributeUsageAttribute*, your +// custom attribute will be valid for all targets, as defined by the **All** value of +// *AttributeTargets* enumeration. +// +// This rule matches attribute classes that are not tagged with +// **System.AttributeUsageAttribute**. +// + +// +// To fix a violation of this rule, specify targets for the attribute by using +// *AttributeUsageAttribute*. +//]]> + Remove calls to GC.Collect() +warnif count > 0 + +let gcCollectMethods = ThirdParty.Methods.WithFullNameWildcardMatch( + "System.GC.Collect(*)").ToHashSet() + +from m in Application.Methods.UsingAny(gcCollectMethods) +select new { + m, + gcCollectMethodCalled = m.MethodsCalled.Intersect(gcCollectMethods) +} + +// +// It is preferable to avoid calling **GC.Collect()** +// explicitly in order to avoid some performance pitfall. +// +// More in information on this here: +// http://blogs.msdn.com/ricom/archive/2004/11/29/271829.aspx +// +// This rule matches application methods that call an +// overload of the method *GC.Collect()*. +// + +// +// Remove matched calls to *GC.Collect()*. +//]]> + Don't call GC.Collect() without calling GC.WaitForPendingFinalizers() +warnif count > 0 + +let gcCollectMethods = ThirdParty.Methods.WithFullNameWildcardMatch( + "System.GC.Collect(*)").ToHashSet() + +from m in Application.Methods.UsingAny(gcCollectMethods) where + !m.IsUsing ("System.GC.WaitForPendingFinalizers()".AllowNoMatch()) +select new { + m, + gcCollectMethodCalled = m.MethodsCalled.Intersect(gcCollectMethods) +} + +// +// It is preferable to avoid calling **GC.Collect()** +// explicitly in order to avoid some performance +// pitfall. This situation is checked through the +// default rules: *Remove calls to GC.Collect()* +// +// But if you wish to call *GC.Collect()* anyway, +// you must do it this way: +// +// GC.Collect(); +// +// GC.WaitForPendingFinalizers(); +// +// GC.Collect(); +// +// To make sure that finalizer got executed, and +// object with finalizer got cleaned properly. +// +// This rule matches application methods that call an +// overload of the method *GC.Collect()*, without calling +// *GC.WaitForPendingFinalizers()*. +// + +// +// To fix a violation of this rule, if you really +// need to call *GC.Collect()*, make sure to call +// *GC.WaitForPendingFinalizers()* properly. +//]]> + Enum Storage should be Int32 +warnif count > 0 from f in Fields where + f.Name == @"value__" && + !f.FieldTypeIs ("System.Int32".AllowNoMatch()) && + !f.IsThirdParty +select new { f, f.SizeOfInst, f.FieldType } + +// +// An enumeration is a value type that defines a set of related named constants. +// By default, the **System.Int32** data type is used to store the constant value. +// +// Even though you can change this underlying type, it is not necessary or +// recommended for most scenarios. Note that *no significant performance gain* is +// achieved by using a data type that is smaller than *Int32*. If you cannot use +// the default data type, you should use one of the Common Language System +// (CLS)-compliant integral types, *Byte*, *Int16*, *Int32*, or *Int64* to make +// sure that all values of the enumeration can be represented in CLS-compliant +// programming languages. +// +// This rule matches enumerations whose underlying type used to store +// values is not *System.Int32*. +// + +// +// To fix a violation of this rule, unless size or compatibility issues exist, +// use *Int32*. For situations where *Int32* is not large enough to hold the values, +// use *Int64*. If backward compatibility requires a smaller data type, use +// *Byte* or *Int16*. +//]]> + Do not raise too general exception types + +warnif count > 0 from m in Application.Methods where + // Test for non-constructor, else this rule + // would warn on ctor of classes that derive + // from these exception types. + !m.IsConstructor && ( + + m.CreateA("System.Exception".AllowNoMatch()) || + m.CreateA("System.ApplicationException".AllowNoMatch()) || + m.CreateA("System.SystemException".AllowNoMatch()) ) +select m + +// +// The following exception types are too general +// to provide sufficient information to the user: +// +// • System.Exception +// +// • System.ApplicationException +// +// • System.SystemException +// +// If you throw such a general exception type in a library or framework, +// it forces consumers to catch all exceptions, +// including unknown exceptions that they do not know how to handle. +// +// This rule matches methods that create an instance of +// such general exception class. +// + +// +// To fix a violation of this rule, change the type of the thrown exception +// to either a more derived type that already exists in the framework, +// or create your own type that derives from *System.Exception*. +//]]> + Do not raise reserved exception types +warnif count > 0 + +let reservedExceptions = ThirdParty.Types.WithFullNameIn( + "System.ExecutionEngineException", + "System.IndexOutOfRangeException", + "System.NullReferenceException", + "System.OutOfMemoryException", + "System.StackOverflowException", + "System.InvalidProgramException", + "System.AccessViolationException", + "System.CannotUnloadAppDomainException", + "System.BadImageFormatException", + "System.DataMisalignedException") + +from m in Application.Methods.ThatCreateAny(reservedExceptions) +let reservedExceptionsCreated = reservedExceptions.Where(t => m.IsUsing(t)) +select new { m, reservedExceptionsCreated } + +// +// The following exception types are reserved +// and should be thrown only by the Common Language Runtime: +// +// • System.ExecutionEngineException +// +// • System.IndexOutOfRangeException +// +// • System.NullReferenceException +// +// • System.OutOfMemoryException +// +// • System.StackOverflowException +// +// • System.InvalidProgramException +// +// • System.AccessViolationException +// +// • System.CannotUnloadAppDomainException +// +// • System.BadImageFormatException +// +// • System.DataMisalignedException +// +// Do not throw an exception of such reserved type. +// +// This rule matches methods that create an instance of +// such reserved exception class. +// + +// +// To fix a violation of this rule, change the type of the +// thrown exception to a specific type that is not one of +// the reserved types. +//]]> + Use integral or string argument for indexers +warnif count > 0 +from m in Application.Methods where + m.IsIndexerGetter && + !( (m.Name == @"get_Item(String)") || + m.NameLike (@"get_Item\(Int") || + m.NameLike (@"get_Item\(Byte") || + m.NameLike (@"get_Item\(SByte") ) +select m + +// +// Indexers, that is, indexed properties, should use *integer* or *string* +// types for the index. These types are typically used for indexing data +// structures and increase the usability of the library. Use of the *Object* +// type should be restricted to those cases where the specific *integer* or +// *string* type cannot be specified at design time. If the design requires +// other types for the index, reconsider whether the type represents a +// logical data store. If it does not represent a logical data store, +// use a method. +// +// This rule matches indexer getter methods that whose index type +// is not *string*, *int*, *byte* or *sbyte*. +// + +// +// To fix a violation of this rule, change the index to an *integer* or *string* +// type, or use a method instead of the indexer. +//]]> + Uri fields should be of type System.Uri +warnif count > 0 from f in Application.Fields where + (f.NameLike (@"Uri$") || + f.NameLike (@"Url$")) && + !f.FieldTypeIs ("System.Uri".AllowNoMatch()) +select new { + f, + f.FieldType +} + +// +// A field with the name ending with *'Uri'* or *'Url'* is deemed +// to represent a *Uniform Resource Identifier or Locator*. +// Such field should be of type **System.Uri**. +// +// This rule matches fields with the name ending with *'Uri'* or +// *'Url'* that are not typed with *System.Uri*. +// + +// +// Rename the field, or change the field type to *System.Uri*. +//]]> + Types should not extend System.ApplicationException +warnif count > 0 from t in Application.Types where + t.DeriveFrom("System.ApplicationException".AllowNoMatch()) +select t + +// +// At .NET Framework version 1 time, it was +// recommended to derive new exceptions from +// *ApplicationException*. +// +// The recommendation has changed and new +// exceptions should derive from **System.Exception** +// or one of its subclasses in the *System* namespace. +// +// This rule matches application exception classes +// that derive from *ApplicationException*. +// + +// +// Make sure that matched exception types, +// derive from **System.Exception** or one of its +// subclasses in the *System* namespace. +//]]> + + + Collection properties should be read only +warnif count > 0 + +// First find collectionTypes +let collectionInterfaces = ThirdParty.Types.WithFullNameIn( + "System.Collections.ICollection", + "System.Collections.Generic.ICollection") +where collectionInterfaces.Count() > 0 +let collectionTypes = Types.ThatImplementAny(collectionInterfaces) + .Union(collectionInterfaces) + .ToHashSet() + +// Then find all property setters that have an associated +// getter that returns a collection type. +from propGetter in Application.Methods.Where( + m => m.IsPropertyGetter && + m.ReturnType != null && + collectionTypes.Contains(m.ReturnType)) + +let propSetter = propGetter.ParentType.Methods.WithSimpleName( + propGetter.SimpleName.Replace("get_","set_") + ).FirstOrDefault() + +where propSetter != null && + !propSetter.IsPrivate && + !propSetter.ParentType.IsPrivate // ignore setters of private types + +select new { + propSetter, + CollectionType = propGetter.ReturnType +} + +// +// A writable collection property allows a user to replace the collection with +// a completely different collection. A read-only property stops the collection +// from being replaced but still allows the individual members to be set. If +// replacing the collection is a goal, the preferred *design pattern* is to include +// a method to remove all the elements from the collection and a method to +// re-populate the collection. See the *Clear()* and *AddRange()* methods of the +// *System.Collections.Generic.List* class for an example of this pattern. +// +// Both binary and XML serialization support read-only properties that are +// collections. The *System.Xml.Serialization.XmlSerializer* class has specific +// requirements for types that implement *ICollection* and *System.Collections.IEnumerable* +// in order to be serializable. +// +// This rule matches property setter methods that assign a collection object. +// + +// +// To fix a violation of this rule, make the property read-only and, if +// the design requires it, add methods to clear and re-populate the collection. +//]]> + Don't use .NET 1.x HashTable and ArrayList +warnif count > 0 +let forbiddenTypes = ThirdParty.Types.WithFullNameIn( + "System.Collections.HashTable", + "System.Collections.ArrayList", + "System.Collections.Queue", + "System.Collections.Stack", + "System.Collections.SortedList") +where forbiddenTypes.Count() > 0 +from m in Application.Methods.ThatCreateAny(forbiddenTypes) +select m + +// +// This rule warns about application methods that use a non-generic +// collection class, including **ArrayList**, **HashTable**, **Queue**, +// **Stack** or **SortedList**. +// + +// +// **List** should be preferred over **ArrayList**. +// It is generic hence you get strongly typed elements. +// Also, it is faster with *T* as a value types since it avoids boxing. +// +// For the same reasons: +// +// • **Dictionary** should be prevered over **HashTable**. +// +// • **Queue** should be prevered over **Queue**. +// +// • **Stack** should be prevered over **Stack**. +// +// • **SortedDictionary** or **SortedList** should be prevered over **SortedList**. +// +// You can be forced to use *non generic* collections +// because you are using third party code that requires +// working with these classes or because you are +// coding with .NET 1.x, but nowadays this situation should +// question about using newer updates of .NET. +// .NET 1.x is an immature platform conpared to newer .NET +// updates. +//]]> + Caution with List.Contains() +let containsMethods = ThirdParty.Methods.WithFullNameIn( + "System.Collections.Generic.List.Contains(T)", + "System.Collections.Generic.IList.Contains(T)", + "System.Collections.ArrayList.Contains(Object)") + +from m in Application.Methods.UsingAny(containsMethods) +select m + +// +// This code query matches calls to *List.Contains()* method. +// +// The cost of checking if a list contains an object is proportional +// to the size of the list. In other words it is a *O(N)* operation. +// For large lists and/or frequent calls to *Contains()*, prefer using +// the *System.Collections.Generic.HashSet* class +// where calls to *Contains()* take a constant +// time (*O(0)* operation). +// +// This code query is not a code rule, because more often than not, +// calling *O(N) Contains()* is not a mistake. This code query +// aims at pointing out this potential performance pitfall. +//]]> + Prefer return collection abstraction instead of implementation +let implTypes = ThirdParty.Types.WithFullNameIn( + "System.Collections.Generic.List", + "System.Collections.Generic.HashSet", + "System.Collections.Generic.Dictionary") + +from m in Application.Methods.WithReturnTypeIn(implTypes) +select new { m, m.ReturnType } + +// +// This code query matches methods that return a +// collection implementation, such as *List* +// *HashSet* or *Dictionary*. +// +// Most often than not, clients of a method don't +// need to know the exact implementation of the +// collection returned. It is preferable to return +// a collection interface such as *IList*, +// *ICollection*, *IEnumerable* or +// *IDictionary*. +// +// Using the collection interface instead of the +// implementation shouldn't applies to all cases, +// hence this code query is not code rule. +//]]> + + + P/Invokes should be static and not be visible +warnif count > 0 from m in Application.Methods where + !m.IsThirdParty && + (m.HasAttribute ("System.Runtime.InteropServices.DllImportAttribute".AllowNoMatch())) && + ( m.IsPublic || + !m.IsStatic) +select new { m, m.Visibility, m.IsStatic } + +// +// Methods that are marked with the **DllImportAttribute** attribute +// (or methods that are defined by using the **Declare** keyword in Visual Basic) +// use **Platform Invocation Services** to access unmanaged code. +// +// Such methods should not be exposed. By keeping these methods *private* or *internal*, +// you make sure that your library cannot be used to breach security by allowing +// callers access to unmanaged APIs that they could not call otherwise. +// +// This rule matches methods tagged with *DllImportAttribute* attribute +// that are declared as *public* or declared as *non-static*. +// + +// +// To fix a violation of this rule, change the access level of the method +// and/or declare it as static. +//]]> + Move P/Invokes to NativeMethods class +warnif count > 0 from m in Application.Methods where + m.HasAttribute ("System.Runtime.InteropServices.DllImportAttribute".AllowNoMatch()) && + m.ParentType.Name != "NativeMethods" +select m + +// +// **Platform Invocation methods**, such as those that are marked by using the +// **System.Runtime.InteropServices.DllImportAttribute** attribute, or methods +// that are defined by using the **Declare** keyword in Visual Basic, access +// unmanaged code. These methods should be in one of the following classes: +// +// • **NativeMethods** - This class does not suppress stack walks for unmanaged +// code permission. (*System.Security.SuppressUnmanagedCodeSecurityAttribute* +// must not be applied to this class.) This class is for methods that can be +// used anywhere because a stack walk will be performed. +// +// • **SafeNativeMethods** - This class suppresses stack walks for unmanaged +// code permission. (*System.Security.SuppressUnmanagedCodeSecurityAttribute* +//is applied to this class.) This class is for methods that are safe for anyone +// to call. Callers of these methods are not required to perform a full security +// review to make sure that the usage is secure because the methods are harmless +// for any caller. +// +// • **UnsafeNativeMethods** - This class suppresses stack walks for unmanaged +// code permission. (*System.Security.SuppressUnmanagedCodeSecurityAttribute* +// is applied to this class.) This class is for methods that are potentially +// dangerous. Any caller of these methods must perform a full security review +// to make sure that the usage is secure because no stack walk will be performed. +// +// These classes are declared as *static internal*. The methods in these +// classes are *static* and *internal*. +// +// This rule matches *P/Invoke* methods not declared in such *NativeMethods* +// class. +// + +// +// To fix a violation of this rule, move the method to the appropriate +// **NativeMethods** class. For most applications, moving P/Invokes to a new +// class that is named **NativeMethods** is enough. +//]]> + NativeMethods class should be static and internal +warnif count > 0 from t in Application.Types.WithNameIn( + @"NativeMethods", "SafeNativeMethods", "UnsafeNativeMethods") where + t.IsPublic || !t.IsStatic +select new { t, t.Visibility, t.IsStatic } + +// +// In the description of the default rule *Move P/Invokes to NativeMethods class* +// it is explained that *NativeMethods* classes that host *P/Invoke* methods, +// should be declared as *static* and *internal*. +// +// This code rule warns about *NativeMethods* classes that are not declared +// *static* and *internal*. +// + +// +// Matched *NativeMethods* classes must be declared as *static* and *internal*. +//]]> + + + Method non-synchronized that read mutable states +from m in Application.Methods where + (m.ReadsMutableObjectState || m.ReadsMutableTypeState) && + !m.IsUsing ("System.Threading.Monitor".AllowNoMatch()) && + !m.IsUsing ("System.Threading.ReaderWriterLock".AllowNoMatch()) +select new { + m, + mutableFieldsUsed = m.FieldsUsed.Where(f => !f.IsImmutable) +} + +// +// Mutable object states are instance fields that +// can be modified through the lifetime of the object. +// +// Mutable type states are static fields that can be +// modified through the lifetime of the program. +// +// This query lists methods that read mutable state +// without synchronizing access. In the case of +// multi-threaded program, doing so can lead to +// state corruption. +// +// This code query is not a code rule because more often +// than not, a match of this query is not an issue. +//]]> + Don't create threads explicitly +warnif count > 0 from m in Application.Methods where + m.CreateA ("System.Threading.Thread".AllowNoMatch()) +select m + +// +// This code rule warns about methods that create *threads* explicitly +// by creating an instance of the class *System.Threading.Thread*. +// +// Prefer using the thread pool instead of creating manually your +// own threads. Threads are costly objects. They take approximately +// 200,000 cycles to create and about 100,000 cycles to destroy. +// By default they reserve 1 Mega Bytes of virtual memory for its +// stack and use 2,000-8,000 cycles for each context switch. +// +// As a consequence, it is preferable to let the thread pool +// recycle threads. +// + +// +// Instead of creating explicitly threads, use the **Task Parralel +// Library** *(TPL)* that relies on the CLR thread pool. +// +// Introduction to TPL: https://msdn.microsoft.com/en-us/library/dd460717(v=vs.110).aspx +// +// TPL and the CLR v4 thread pool: +// http://www.danielmoth.com/Blog/New-And-Improved-CLR-4-Thread-Pool-Engine.aspx +//]]> + Don't use dangerous threading methods +warnif count > 0 + +let wrongMethods = ThirdParty.Methods.WithFullNameIn( + + "System.Threading.Thread.Abort()", + "System.Threading.Thread.Abort(Object)", + + "System.Threading.Thread.Sleep(Int32)", + + "System.Threading.Thread.Suspend()", + "System.Threading.Thread.Resume()") + +from m in Application.Methods.UsingAny(wrongMethods) +select new { + m, + suppressCallsTo = m.MethodsCalled.Intersect(wrongMethods) +} + +// +// This rule warns about using the methods +// *Abort()*, *Sleep()*, *Suspend()* or *Resume()* +// declared by the *Thread* class. +// +// • Usage of *Thread.Abort()* is dangerous. +// More information on this here: +// http://www.interact-sw.co.uk/iangblog/2004/11/12/cancellation +// +// • Usage of *Thread.Sleep()* is a sign of +// flawed design. More information on this here: +// http://msmvps.com/blogs/peterritchie/archive/2007/04/26/thread-sleep-is-a-sign-of-a-poorly-designed-program.aspx +// +// • *Suspend()* and *Resume()* are dangerous threading methods, marked as obsolete. +// More information on workaround here: +// http://stackoverflow.com/questions/382173/what-are-alternative-ways-to-suspend-and-resume-a-thread +// + +// +// Suppress calls to *Thread* methods exposed in the +// *suppressCallsTo* column in the rule result. +// +// Use instead facilities offered by the **Task Parralel +// Library** *(TPL)* : +// https://msdn.microsoft.com/en-us/library/dd460717(v=vs.110).aspx +//]]> + Monitor TryEnter/Exit must be both called within the same method +warnif count > 0 + +let enterMethods = ThirdParty.Methods.WithFullNameWildcardMatchIn( + "System.Threading.Monitor.Enter(*", + "System.Threading.Monitor.TryEnter(*") + +from m in Application.Methods.UsingAny(enterMethods) where + !m.IsUsing ("System.Threading.Monitor.Exit(Object)".AllowNoMatch()) +select new { + m, + enterMethodsCalled = m.MethodsCalled.Intersect(enterMethods) +} + +// +// This rule warns when **System.Threading.Monitor** *Enter()* +// (or *TryEnter()*) and *Exit() methods are not called within +// the same method. +// +// Doing so makes the code *less readable*, because it gets harder +// to locate when **critical sections** begin and end. +// +// Also, you expose yourself to complex and error-prone scenarios. +// + +// +// Refactor matched methods to make sure that *Monitor critical +// sections* begin and end within the same method. Basics scenarios +// can be handled through the C# **lock** keyword. Using explicitly +// the class *Monitor* should be left for advanced situations, +// that require calls to methods like *Wait()* and *Pulse()*. +// +// More information on using the *Monitor* class can be found here: +// http://www.codeproject.com/Articles/13453/Practical-NET-and-C-Chapter +//]]> + ReaderWriterLock AcquireLock/ReleaseLock must be both called within the same method +warnif count > 0 + +let acquireLockMethods = ThirdParty.Methods.WithFullNameWildcardMatch( + "System.Threading.ReaderWriterLock.Acquire*Lock(*") + +let releaseLockMethods = ThirdParty.Methods.WithFullNameWildcardMatch( + "System.Threading.ReaderWriterLock.Release*Lock(*") + +from m in Application.Methods.UsingAny(acquireLockMethods) + .Except(Application.Methods.UsingAny(releaseLockMethods)) +select new { + m, + acquireLockMethods = m.MethodsCalled.Intersect(acquireLockMethods) +} + +// +// This rule warns when **System.Threading.ReaderWriterLock** +// acquire and release, reader or writer locks methods are not called +// within the same method. +// +// Doing so makes the code *less readable*, because it gets harder +// to locate when **critical sections** begin and end. +// +// Also, you expose yourself to complex and error-prone scenarios. +// + +// +// Refactor matched methods to make sure that *ReaderWriterLock +// read or write critical sections* begin and end within the +// same method. +//]]> + Don't tag instance fields with ThreadStaticAttribute +warnif count > 0 +from f in Application.Fields +where !f.IsStatic && + f.HasAttribute ("System.ThreadStaticAttribute".AllowNoMatch()) +select f + +// +// This rule warns when the attribute **System.ThreadStaticAttribute** +// is tagging *instance* fields. As explained in documentation, this attribute +// is designed to tag only *static* fields. +// https://msdn.microsoft.com/en-us/library/system.threadstaticattribute +// + +// +// Refactor the code to make sure that all fields tagged with +// *ThreadStaticAttribute* are *static*. +//]]> + + + Method should not return concrete XmlNode +warnif count > 0 + +let concreteXmlTypes = ThirdParty.Types.ThatDeriveFromAny( + ThirdParty.Types.WithFullName("System.Xml.XmlNode")) + +from m in Application.Methods.WithReturnTypeIn(concreteXmlTypes) +select new { m, m.ReturnType } + +// +// This rule warns about method whose return type is +// **System.Xml.XmlNode** or any type derived from *XmlNode*. +// +// *XmlNode* implements the interface **System.Xml.Xpath.IXPathNavigable**. +// In most situation, returning this interface instead of the concrete +// type is a better *design* choice that will abstract client code +// from implementation details. +// + +// +// To fix a violation of this rule, change the concrete returned type +// to the suggested interface *IXPathNavigable*. +//]]> + Types should not extend System.Xml.XmlDocument +warnif count > 0 from t in Application.Types where + t.DeriveFrom("System.Xml.XmlDocument".AllowNoMatch()) +select t + +// +// This rule warns aboud subclasses of **System.Xml.XmlDocument**. +// +// Do not create a subclass of *XmlDocument* if you want to +// create an XML view of an underlying object model or data source. +// + +// +// Instead of subclassing *XmlDocument*, you can use the interface +// **System.Xml.XPath.IXPathNavigable** implemented by the class +// *XmlDocument*. +// +// An alternative of using *XmlDocument*, is to use +// **System.Xml.Linq.XDocument**, aka **LINQ2XML**. +// More information on this can be found here: +// http://stackoverflow.com/questions/1542073/xdocument-or-xmldocument +//]]> + + + Float and Date Parsing must be culture aware +warnif count > 0 +from m in ThirdParty.Types.WithFullNameIn( + "System.DateTime", + "System.Single", + "System.Double", + "System.Decimal").ChildMethods() + where m.NbParameters > 0 && + (m.SimpleName.EqualsAny( + "Parse", "TryParse", "ToString")) && + !m.Name.Contains("IFormatProvider") +select new { + m, + m.MethodsCallingMe +} + +// +// Globalization is the design and development of applications that support +// localized user interfaces and regional data for users in multiple cultures. +// +// This rule warns about the usage of *non-globalized overloads* of +// the methods **Parse()**, **TryParse()** and **ToString()**, +// of the types **DateTime**, **float**, **double** and **decimal**. +// This is the symptom that your application is *at least partially* +// not globalized. +// +// *Non-globalized overloads* of these methods are the overloads +// that don't take a parameter of type **IFormatProvider**. +// + +// +// Globalize your applicaton and make sure to use the globalized overloads +// of these methods. In the column **MethodsCallingMe** of this rule result +// are listed the methods of your application that call the +// *non-globalized overloads*. +// +// More information on **Creating Globally Aware Applications** here: +// https://msdn.microsoft.com/en-us/library/cc853414(VS.95).aspx +//]]> + + + Public methods returning a reference needs a contract to ensure that a non-null reference is returned +warnif count > 0 +let ensureMethods = Application.Methods.WithFullName( + "System.Diagnostics.Contracts.__ContractsRuntime.Ensures(Boolean,String,String)") + +from ensureMethod in ensureMethods +from m in ensureMethod.ParentAssembly.ChildMethods where + m.IsPubliclyVisible && + !m.IsAbstract && + m.ReturnType != null && + // Identify that the return type is a reference type + (m.ReturnType.IsClass || m.ReturnType.IsInterface) && + !m.IsUsing(ensureMethod) && + + // Don't match method not implemented yet! + !m.CreateA("System.NotImplementedException".AllowNoMatch()) + +select new { + m, + ReturnTypeReference = m.ReturnType +} + +// +// **Code Contracts** are useful to decrease ambiguity between callers and callees. +// Not ensuring that a reference returned by a method is *non-null* leaves ambiguity +// for the caller. This rule matches methods returning an instance of a reference type +// (class or interface) that don't use a **Contract.Ensure()** method. +// +// *Contract.Ensure()* is defined in the **Microsoft Code Contracts for .NET** +// library, and is typically used to write a code contract on returned reference: +// *Contract.Ensures(Contract.Result() != null, "returned reference is not null");* +// https://visualstudiogallery.msdn.microsoft.com/1ec7db13-3363-46c9-851f-1ce455f66970 +// + +// +// Use *Microsoft Code Contracts for .NET* on the public surface of your API, +// to remove most ambiguity presented to your client. Most of such ambiguities +// are about *null* or *not null* references. +// +// Don't use *null* reference if you need to express that a method might not +// return a result. Use instead the **TryXXX()** pattern exposed for example +// in the *System.Int32.TryParse()* method. +//]]> + + + Third-Party Assemblies Used +from a in ThirdParty.Assemblies +select new { a, a.AssembliesUsingMe } + +// +// This code query lists *third-party assemblies* used +// by the application analyzed. +// +// For each third-party assembly matched, the column +// *AssembliesUsingMe* of the query result contains +// application assemblies using the assembly. +//]]> + Third-Party Namespaces Used +from n in ThirdParty.Namespaces +select new { n, n.NamespacesUsingMe } + +// +// This code query lists *third-party namespaces* used +// by the application analyzed. +// +// For each third-party namespace matched, the column +// *NamespacesUsingMe* of the query result contains +// application namespaces using the namespace. +//]]> + Third-Party Types Used +from t in ThirdParty.Types +select new { t, t.TypesUsingMe, t.DerivedTypes, t.TypesThatImplementMe } + +// +// This code query lists *third-party types* used +// by the application analyzed. +// +// For each third-party type matched: +// +// • The column *TypesUsingMe* of the query result contains +// application types using the third-party type. +// +// • The column *DerivedTypes* of the query result contains +// application sub-classes of the third-party class. +// +// • The column *TypesThatImplementMe* of the query result +// contains application classes that implement the third-party interface. +//]]> + Third-Party Methods Used +from m in ThirdParty.Methods +select new { m, m.MethodsCallingMe } + +// +// This code query lists *third-party methods* called +// by the application analyzed. +// +// For each third-party method matched, the column +// *MethodsCallingMe* of the query result contains +// application methods that call the method. +//]]> + Third-Party Fields Used +from f in ThirdParty.Fields +where !f.ParentType.IsEnumeration +select new { f, f.MethodsReadingMeButNotAssigningMe, f.MethodsAssigningMe } + +// +// This code query lists *third-party fields* called +// by the application analyzed. +// +// For each third-party field matched, the column +// *MethodsReadingMeButNotAssigningMe* and *MethodsAssigningMe* +// of the query result contains application methods that read +// or assign the field. +//]]> + + + + + +Application.Assemblies.Sum(a => a.NbLinesOfCode)]]> + +JustMyCode.Methods.Sum(m => m.NbLinesOfCode) + +// JustMyCode is defined by code queries prefixed with 'notmycode' +// in the group 'Defining JustMyCode'. +]]> + +Application.Methods.Where(m => !JustMyCode.Contains(m)) + .Sum(m => m.NbLinesOfCode) + +// JustMyCode is defined by code queries prefixed with 'notmycode' +// in the group 'Defining JustMyCode'. +]]> + +( from a in Application.Assemblies + let nbLocAdded = !a.IsPresentInBothBuilds() + ? a.NbLinesOfCode + : (a.NbLinesOfCode != null && a.OlderVersion().NbLinesOfCode != null) + ? a.NbLinesOfCode - (int)a.OlderVersion().NbLinesOfCode + : 0 + select nbLocAdded) +.Sum(loc => loc) + +// A value is computed by this Trend Metric query +// only if a Baseline for Comparison is provided. +// See Project Properties > Analysis > Baseline for Comparison +]]> + +Application.Assemblies.SelectMany( + a => a.SourceDecls.Select(sd => sd.SourceFile.FilePathString.ToLower())) +.Distinct() +.Count() + +// If a value 0 is obtained, it means that at analysis time, +// assemblies PDB files were not available. +// http://www.ndepend.com/docs/ndepend-analysis-inputs-explanation +]]> + +Application.Assemblies.Sum(a => a.NbILInstructions) +]]> + +Application.Methods.Where(m => !JustMyCode.Contains(m)) + .Sum(m => m.NbILInstructions) + +// JustMyCode is defined by code queries prefixed with 'notmycode' +// in the group 'Defining JustMyCode'. +]]> + +Application.Assemblies.Sum(a => a.NbLinesOfComment) + +// So far comments are only extracted from C# source code. +]]> + +Application.Assemblies.Count()]]> + +Application.Namespaces.Count()]]> + +Application.Types.Count(t => !t.IsGeneratedByCompiler)]]> + +Application.Types.Where(t => t.IsPubliclyVisible && !t.IsGeneratedByCompiler).Count()]]> + +Application.Types.Count(t => t.IsClass && !t.IsGeneratedByCompiler)]]> + +Application.Types.Count(t => t.IsClass && t.IsAbstract && !t.IsGeneratedByCompiler)]]> + +Application.Types.Count(t => t.IsInterface)]]> + +Application.Types.Count(t => t.IsStructure && !t.IsGeneratedByCompiler)]]> + +Application.Methods.Count(m => !m.IsGeneratedByCompiler)]]> + +Application.Methods.Count(m => m.IsAbstract)]]> + +Application.Methods.Count(m => !m.IsAbstract && !m.IsGeneratedByCompiler)]]> + +Application.Fields.Count(f => + !f.IsEnumValue && + !f.IsGeneratedByCompiler && + !f.IsLiteral && + !f.ParentType.IsEnumeration)]]> + + + +JustMyCode.Methods + .Max(m => m.NbLinesOfCode) + .ToEnumerable().Sum(loc => loc) + +// Here is the code query to get the (JustMyCode) method with largest # Lines of Code +// JustMyCode.Methods.OrderByDescending(m => m.NbLinesOfCode).Take(1).Select(m => new {m, m.NbLinesOfCode})]]> + +Application.Methods.Where(m => m.NbLinesOfCode > 0) + .Average(m => m.NbLinesOfCode)]]> + +Application.Methods.Where(m => m.NbLinesOfCode >= 3) + .Average(m => m.NbLinesOfCode)]]> + +JustMyCode.Types + .Max(t => t.NbLinesOfCode) + .ToEnumerable().Sum(loc => loc) + +// Here is the code query to get the (JustMyCode) type with largest # Lines of Code +// JustMyCode.Types.OrderByDescending(t => t.NbLinesOfCode).Take(1).Select(t => new {t, t.NbLinesOfCode})]]> + +Application.Types.Where(t => t.NbLinesOfCode > 0) + .Average(t => t.NbLinesOfCode)]]> + +Application.Methods + .Max(m => m.CyclomaticComplexity) + .ToEnumerable().Sum(loc => loc) + +// Here is the code query to get the most complex method, according to Cyclomatic Complexity +// Application.Methods.OrderByDescending(m => m.CyclomaticComplexity).Take(1).Select(m => new {m, m.CyclomaticComplexity})]]> + +Application.Methods.Where(m => m.NbLinesOfCode> 0) + .Average(m => m.CyclomaticComplexity)]]> + +Application.Methods + .Max(m => m.ILCyclomaticComplexity) + .ToEnumerable().Sum(loc => loc) + +// Here is the code query to get the most complex method, according to Cyclomatic Complexity computed from IL code. +// Application.Methods.OrderByDescending(m => m.ILCyclomaticComplexity).Take(1).Select(m => new {m, m.CyclomaticComplexity})]]> + +Application.Methods.Where(m => m.NbILInstructions> 0) + .Average(m => m.ILCyclomaticComplexity)]]> + +Application.Methods + .Max(m => m.ILNestingDepth) + .ToEnumerable().Sum(loc => loc) + +// Here is the code query to get the method with highest ILNestingDepth. +// Application.Methods.OrderByDescending(m => m.ILNestingDepth).Take(1).Select(m => new {m, m.ILNestingDepth})]]> + +Application.Methods.Where(m => m.NbILInstructions> 0) + .Average(m => m.ILNestingDepth)]]> + +Application.Types + .Max(t => t.NbMethods) + .ToEnumerable().Sum(loc => loc) + +// Here is the code query to get the (JustMyCode) type with largest # of Methods +// JustMyCode.Types.OrderByDescending(t => t.NbMethods).Take(1).Select(t => new {t, t.Methods})]]> + +Application.Types.Average(t => t.NbMethods)]]> + +Application.Types.Where(t => t.IsInterface) + .Max(t => t.NbMethods) + .ToEnumerable().Sum(loc => loc) + +// Here is the code query to get the (JustMyCode) type with largest # of Methods +// JustMyCode.Types.OrderByDescending(t => t.NbMethods).Take(1).Select(t => new {t, t.Methods})]]> + +JustMyCode.Types.Where(t => t.IsInterface) + .Average(t => t.NbMethods)]]> + + + +((float)Application.Assemblies.Sum(a => a.NbLinesOfCodeCovered) / + Application.Assemblies.Sum(a => a.NbLinesOfCode) + * 100f) +.ToEnumerable().Sum()]]> + +Application.Assemblies.Sum(a => a.NbLinesOfCodeCovered)]]> + +Application.Assemblies.Sum(a => a.NbLinesOfCodeNotCovered)]]> + +Application.Types.Where(t => t.PercentageCoverage == 100) + .Sum(t => t.NbLinesOfCodeCovered)]]> + +Application.Methods.Where(m => m.PercentageCoverage == 100) + .Sum(m => m.NbLinesOfCodeCovered)]]> + + +(from m in JustMyCode.Methods + +// Don't match too short methods +where m.NbLinesOfCode > 10 + +let CC = m.CyclomaticComplexity +let uncov = (100 - m.PercentageCoverage) / 100f +let CRAP = (CC * CC * uncov * uncov * uncov) + CC +where CRAP != null && CRAP > 30 select CRAP) +.Max(CRAP => CRAP) + +// +// **Change Risk Analyzer and Predictor** (i.e. CRAP) is a code metric +// that helps in pinpointing overly complex and untested code. +// Is has been first defined here: +// http://www.artima.com/weblogs/viewpost.jsp?thread=215899 +// +// The Formula is: **CRAP(m) = CC(m)^2 * (1 – cov(m)/100)^3 + CC(m)** +// +// • where *CC(m)* is the *cyclomatic complexity* of the method *m* +// +// • and *cov(m)* is the *percentage coverage* by tests of the method *m* +// +// Matched methods cumulates two highly *error prone* code smells: +// +// • A complex method, difficult to develop and maintain. +// +// • Non 100% covered code, difficult to refactor without any regression bug. +// +// The highest the CRAP score, the more painful to maintain and error prone is the method. +// +// An arbitrary threshold of 30 is fixed for this code rule as suggested by inventors. +// +// Notice that no amount of testing will keep methods with a Cyclomatic Complexity +// highest than 30, out of CRAP territory. +// +// Notice that CRAP score is not computed for too short methods +// with less than 10 lines of code. +// +// To list methods with highest C.R.A.P scores, please refer to the default rule: +// *Test and Code Coverage* > *C.R.A.P method code metric* +//]]> + + +(from m in JustMyCode.Methods + +// Don't match too short methods +where m.NbLinesOfCode > 10 + +let CC = m.CyclomaticComplexity +let uncov = (100 - m.PercentageCoverage) / 100f +let CRAP = (CC * CC * uncov * uncov * uncov) + CC +where CRAP != null && CRAP > 30 select CRAP) +.Average(CRAP => CRAP) + +// +// **Change Risk Analyzer and Predictor** (i.e. CRAP) is a code metric +// that helps in pinpointing overly complex and untested code. +// Is has been first defined here: +// http://www.artima.com/weblogs/viewpost.jsp?thread=215899 +// +// The Formula is: **CRAP(m) = CC(m)^2 * (1 – cov(m)/100)^3 + CC(m)** +// +// • where *CC(m)* is the *cyclomatic complexity* of the method *m* +// +// • and *cov(m)* is the *percentage coverage* by tests of the method *m* +// +// Matched methods cumulates two highly *error prone* code smells: +// +// • A complex method, difficult to develop and maintain. +// +// • Non 100% covered code, difficult to refactor without any regression bug. +// +// The highest the CRAP score, the more painful to maintain and error prone is the method. +// +// An arbitrary threshold of 30 is fixed for this code rule as suggested by inventors. +// +// Notice that no amount of testing will keep methods with a Cyclomatic Complexity +// highest than 30, out of CRAP territory. +// +// Notice that CRAP score is not computed for too short methods +// with less than 10 lines of code. +// +// To list methods with highest C.R.A.P scores, please refer to the default rule: +// *Test and Code Coverage* > *C.R.A.P method code metric* +//]]> + + + +ThirdParty.Assemblies.Count()]]> + +ThirdParty.Namespaces.Count()]]> + +ThirdParty.Types.Count()]]> + +ThirdParty.Methods.Count()]]> + +ThirdParty.Fields.Count()]]> + + + + Discard generated Assemblies from JustMyCode +notmycode +from a in Application.Assemblies where +// Assemblies generated for Xsl IL compilation for example are tagged with this attribute +a.HasAttribute ("System.CodeDom.Compiler.GeneratedCodeAttribute".AllowNoMatch()) +select a + +// +// This code query is prefixed with **notmycode**. +// This means that all application assemblies matched by this +// code query are removed from the *code base view* **JustMyCode.Assemblies**. +// It also means that all *namespaces*, *types*, *methods* and +// *fields* contained in a matched assembly are removed from +// the code base view *JustMyCode*. +// The code base view *JustMyCode* is used by most default code queries +// and rules. +// +// So far this query only matches application assemblies tagged +// with *System.CodeDom.Compiler.GeneratedCodeAttribute*. +// Make sure to make this query richer to discard your generated +// assemblies from the NDepend rules results. +// +// *notmycode* queries are executed before running others +// queries and rules. Also modifying a *notmycode* query +// provokes re-run of queries and rules that rely +// on the *JustMyCode* code base view. +// +// Several *notmycode* queries can be written to match *assemblies*, +// in which case this results in cumulative effect. +// +// Online documentation: +// http://www.ndepend.com/docs/cqlinq-syntax#NotMyCode +//]]> + Discard generated Types from JustMyCode +notmycode + +from t in Application.Types where + + // Resources, Settings, or typed DataSet generated types for example, are tagged with this attribute + t.HasAttribute ("System.CodeDom.Compiler.GeneratedCodeAttribute".AllowNoMatch()) || + + // This attributes identifies a type or member that is not part of the user code for an application. + t.HasAttribute ("System.Diagnostics.DebuggerNonUserCodeAttribute".AllowNoMatch()) || + + // Delegate types are always generated + t.IsDelegate || + + // Discard ASP.NET page types generated by aspnet_compiler.exe + // See: http://www.ndepend.com/FAQ.aspx#ASPNET + t.ParentNamespace.Name.EqualsAny("ASP", "__ASP") || + + // Discard types generated for code contract + t.FullName.StartsWith("System.Diagnostics.Contracts.__ContractsRuntime") || + t.FullName == "System.Diagnostics.Contracts.RuntimeContractsAttribute" || + + // Discard all types declared in a folder path containing the word "generated" + (t.SourceFileDeclAvailable && + t.SourceDecls.All(s => s.SourceFile.FilePath.ParentDirectoryPath.ToString().ToLower().Contains("generated"))) + +select t + +// +// This code query is prefixed with **notmycode**. +// This means that all application types matched by this +// code query are removed from the *code base view* **JustMyCode.Types**. +// It also means that all *methods* and *fields* contained in a +// matched type are removed from the code base view *JustMyCode*. +// The code base view *JustMyCode* is used by most default code queries +// and rules. +// +// So far this query matches several well-identified generated +// types, like the ones tagged with *System.CodeDom.Compiler.GeneratedCodeAttribute*. +// Make sure to make this query richer to discard your generated +// types from the NDepend rules results. +// +// *notmycode* queries are executed before running others +// queries and rules. Also modifying a *notmycode* query +// provokes re-run of queries and rules that rely +// on the *JustMyCode* code base view. +// +// Several *notmycode* queries can be written to match *types*, +// in which case this results in cumulative effect. +// +// Online documentation: +// http://www.ndepend.com/docs/cqlinq-syntax#NotMyCode +//]]> + Discard generated and designer Methods from JustMyCode +notmycode + +// +// First define source files paths to discard +// +from a in Application.Assemblies +where a.SourceFileDeclAvailable +let asmSourceFilesPaths = a.SourceDecls.Select(s => s.SourceFile.FilePath) + +let sourceFilesPathsToDiscard = ( + from filePath in asmSourceFilesPaths + let filePathLower= filePath.ToString().ToLower() + where + filePathLower.EndsWithAny( + ".g.cs", // Popular pattern to name generated files. + ".g.vb", + ".xaml", // notmycode WPF xaml code + ".designer.cs", // notmycode C# Windows Forms designer code + ".designer.vb") // notmycode VB.NET Windows Forms designer code + || + // notmycode methods in source files in a directory containing generated + filePathLower.Contains("generated") + select filePath +).ToHashSet() + +// +// Second: discard methods in sourceFilesPathsToDiscard +// +from m in a.ChildMethods +where (m.SourceFileDeclAvailable && + sourceFilesPathsToDiscard.Contains(m.SourceDecls.First().SourceFile.FilePath)) || + // Generated methods might be tagged with this attribute + m.HasAttribute ("System.CodeDom.Compiler.GeneratedCodeAttribute".AllowNoMatch()) || + + // This attributes identifies a type or member that is not part of the user code for an application. + m.HasAttribute ("System.Diagnostics.DebuggerNonUserCodeAttribute".AllowNoMatch()) + +select new { m, m.NbLinesOfCode } + +// +// This code query is prefixed with **notmycode**. +// This means that all application methods matched by this +// code query are removed from the *code base view* **JustMyCode.Methods**. +// The code base view *JustMyCode* is used by most default code queries +// and rules. +// +// So far this query matches several well-identified generated +// methods, like the ones tagged with *System.CodeDom.Compiler.GeneratedCodeAttribute*, +// or the ones declared in a source file suffixed with *.designer.cs*. +// Make sure to make this query richer to discard your generated +// methods from the NDepend rules results. +// +// *notmycode* queries are executed before running others +// queries and rules. Also modifying a *notmycode* query +// provokes re-run of queries and rules that rely +// on the *JustMyCode* code base view. +// +// Several *notmycode* queries can be written to match *methods*, +// in which case this results in cumulative effect. +// +// Online documentation: +// http://www.ndepend.com/docs/cqlinq-syntax#NotMyCode +//]]> + Discard generated Fields from JustMyCode +notmycode +from f in Application.Fields where + f.HasAttribute ("System.CodeDom.Compiler.GeneratedCodeAttribute".AllowNoMatch()) || + + // Eliminate "components" generated in Windows Form Control context + f.Name == "components" && f.ParentType.DeriveFrom("System.Windows.Forms.Control".AllowNoMatch()) +select f + +// +// This code query is prefixed with **notmycode**. +// This means that all application fields matched by this +// code query are removed from the *code base view* **JustMyCode.Fields**. +// The code base view *JustMyCode* is used by most default code queries +// and rules. +// +// So far this query only matches application fields tagged +// with *System.CodeDom.Compiler.GeneratedCodeAttribute*, and +// *Windows Form* generated fields named *components*. +// Make sure to make this query richer to discard your generated +// fields from the NDepend rules results. +// +// *notmycode* queries are executed before running others +// queries and rules. Also modifying a *notmycode* query +// provokes re-run of queries and rules that rely +// on the *JustMyCode* code base view. +// +// Several *notmycode* queries can be written to match *fields*, +// in which case this results in cumulative effect. +// +// Online documentation: +// http://www.ndepend.com/docs/cqlinq-syntax#NotMyCode +//]]> + + + Most used types (Rank) +(from t in Application.Types + where !t.IsGeneratedByCompiler + orderby t.Rank descending + select new { t, t.Rank, t.TypesUsingMe }).Take(100) + +// +// **TypeRank** values are computed by applying +// the **Google PageRank** algorithm on the +// graph of types' dependencies. Types with +// high *Rank* are the most used ones. Not necessarily +// the ones with the most users types, but the ones +// used by many types, themselves having a lot of +// types users. +// +// See the definition of the TypeRank metric here: +// http://www.ndepend.com/docs/code-metrics#TypeRank +// +// This code query lists the 100 application types +// with the highest rank. +// +// The main consequence of being used a lot for a +// type is that each change (both *syntax change* +// and *behavior change*) will result in potentially +// a lot of **pain** since most types clients will be +// **impacted**. +// +// Hence it is preferable that types with highest +// *TypeRank*, are **interfaces**, that are typically +// less subject changes. +// +// Also interfaces avoid clients relying on +// implementations details. Hence, when the behavior of +// classes implementing an interface changes, this +// shouldn't impact clients of the interface. +// This is *in essence* the +// **Liskov Substitution Principle**. +// http://en.wikipedia.org/wiki/Liskov_substitution_principle +//]]> + Most used methods (Rank) +(from m in Application.Methods + where !m.IsGeneratedByCompiler + orderby m.Rank descending + select new { m, m.Rank, m.MethodsCallingMe }).Take(100) + +// +// **MethodRank** values are computed by applying +// the **Google PageRank** algorithm on the +// graph of methods' dependencies. Methods with +// high *Rank* are the most used ones. Not necessarily +// the ones with the most callers methods, but the ones +// called by many methods, themselves having a lot +// of callers. +// +// See the definition of the MethodRank metric here: +// http://www.ndepend.com/docs/code-metrics#MethodRank +// +// This code query lists the 100 application methods +// with the highest rank. +// +// The main consequence of being used a lot for a +// method is that each change (both *signature change* +// and *behavior change*) will result in potentially +// a lot of **pain** since most methods callers will be +// **impacted**. +// +// Hence it is preferable that methods with highest +// *MethodRank*, are **abstract methods**, that are +// typically less subject to signature changes. +// +// Also abstract methods avoid callers relying on +// implementations details. Hence, when the code +// of a method implementing an abstract method changes, +// this shouldn't impact callers of the abstract method. +// This is *in essence* the +// **Liskov Substitution Principle**. +// http://en.wikipedia.org/wiki/Liskov_substitution_principle +//]]> + Most used assemblies (#AssembliesUsingMe) +(from a in Assemblies orderby a.AssembliesUsingMe.Count() descending + select new { a, a.AssembliesUsingMe }).Take(100) + +// +// This code query lists the 100 *application* and *third-party* +// assemblies, with the highest number of assemblies users. +//]]> + Most used namespaces (#NamespacesUsingMe ) +(from n in Namespaces orderby n.NbNamespacesUsingMe descending + select new { n, n.NamespacesUsingMe }).Take(100) + +// +// This code query lists the 100 *application* and *third-party* +// namespaces, with the highest number of namespaces users. +//]]> + Most used types (#TypesUsingMe ) +(from t in Types orderby t.NbTypesUsingMe descending + where !t.IsGeneratedByCompiler + select new { t, t.TypesUsingMe }).Take(100) + +// +// This code query lists the 100 *application* and *third-party* +// types, with the highest number of types users. +//]]> + Most used methods (#MethodsCallingMe ) +(from m in Methods orderby m.NbMethodsCallingMe + where !m.IsGeneratedByCompiler + select new { m, m.MethodsCallingMe }).Take(100) + +// +// This code query lists the 100 *application* and *third-party* +// methods, with the highest number of methods callers. +//]]> + Namespaces that use many other namespaces (#NamespacesUsed ) +(from n in Application.Namespaces orderby n.NbNamespacesUsed descending + select new { n, n.NamespacesUsed }).Take(100) + +// +// This code query lists the 100 *application* namespaces +// with the highest number of namespaces used. +//]]> + Types that use many other types (#TypesUsed ) +(from t in Application.Types orderby t.NbTypesUsed descending + select new { t, t.TypesUsed, isMyCode = JustMyCode.Contains(t) }).Take(100) + +// +// This code query lists the 100 *application* types +// with the highest number of types used. +//]]> + Methods that use many other methods (#MethodsCalled ) +(from m in Application.Methods orderby m.NbMethodsCalled descending + select new { m, m.MethodsCalled, isMyCode = JustMyCode.Contains(m) }).Take(100) + +// +// This code query lists the 100 *application* methods +// with the highest number of methods called. +//]]> + High-level to low-level assemblies (Level) +from a in Application.Assemblies orderby a.Level descending +select new { a, a.Level } + +// +// This code query lists assemblies ordered by **Level** values. +// See the definition of the *AssemblyLevel* metric here: +// http://www.ndepend.com/docs/code-metrics#Level +//]]> + High-level to low-level namespaces (Level) +from n in Application.Namespaces orderby n.Level descending +select new { n, n.Level } + +// +// This code query lists namespaces ordered by **Level** values. +// See the definition of the *NamespaceLevel* metric here: +// http://www.ndepend.com/docs/code-metrics#Level +//]]> + High-level to low-level types (Level) +from t in Application.Types orderby t.Level descending +select new { t, t.Level } + +// +// This code query lists types ordered by **Level** values. +// See the definition of the *TypeLevel* metric here: +// http://www.ndepend.com/docs/code-metrics#Level +//]]> + High-level to low-level methods (Level) +from m in Application.Methods orderby m.Level descending +select new { m, m.Level } + +// +// This code query lists methods ordered by **Level** values. +// See the definition of the *MethodLevel* metric here: +// http://www.ndepend.com/docs/code-metrics#Level +//]]> + + + Check that the assembly Asm1 is not using the assembly Asm2 +warnif count > 0 from a in Application.Assemblies where + a.IsUsing ("Asm2".AllowNoMatch().MatchAssembly()) && + (a.Name == @"Asm1") +select a + +// +// This rule is a *sample rule that can be adapted to your need*. +// +// It shows how to be warned if a particular assembly is using +// another particular assembly. +// +// Such rule can be generated for assemblies **A** and **B**: +// +// • by right clicking the cell in the *Dependency Matrix* +// with **B** in row and **A** in column, +// +// • or by right-clicking the concerned arrow in the *Dependency +// Graph* from **A** to **B**, +// +// and in both cases, click the menu +// **Generate a code rule that warns if this dependency exists** +// +// The generated rule will look like this one. +// It is now up to you to adapt this rule to check exactly +// your needs. +// + +// +// This is a *sample rule* there is nothing to fix *as is*. +//]]> + Check that the namespace N1.N2 is not using the namespace N3.N4.N5 +warnif count > 0 from n in Application.Namespaces where + n.IsUsing ("N3.N4.N5".AllowNoMatch().MatchNamespace()) && + (n.Name == @"N1.N2") +select n + +// +// This rule is a *sample rule that can be adapted to your need*. +// +// It shows how to be warned if a particular namespace is using +// another particular namespace. +// +// Such rule can be generated for namespaces **A** and **B**: +// +// • by right clicking the cell in the *Dependency Matrix* +// with **B** in row and **A** in column, +// +// • or by right-clicking the concerned arrow in the *Dependency +// Graph* from **A** to **B**, +// +// and in both cases, click the menu +// **Generate a code rule that warns if this dependency exists** +// +// The generated rule will look like this one. +// It is now up to you to adapt this rule to check exactly +// your needs. +// + +// +// This is a *sample rule* there is nothing to fix *as is*. +//]]> + Check that the assembly Asm1 is only using the assemblies Asm2, Asm3 or mscorlib +warnif count > 0 from a in Application.Assemblies where + ( !a.IsUsing ("Asm2".AllowNoMatch().MatchAssembly()) || + !a.IsUsing ("Asm3".AllowNoMatch().MatchAssembly()) || + !a.IsUsing ("mscorlib".MatchAssembly()) || + a.AssembliesUsed.Count() != 3) // Must not be used more than 3 assemblies +&& + (a.Name == @"Asm1") +select new { a, a.AssembliesUsed } + +// +// This rule is a *sample rule that can be adapted to your need*. +// +// It shows how to enforce that a particular assembly +// is only using a particular set of assemblies. +// + +// +// This is a *sample rule* there is nothing to fix *as is*. +//]]> + Check that the namespace N1.N2 is only using the namespaces N3.N4, N5 or System +warnif count > 0 from n in Application.Namespaces where + ( !n.IsUsing("N3.N4".AllowNoMatch().MatchNamespace()) || + !n.IsUsing("N5".AllowNoMatch().MatchNamespace()) || + !n.IsUsing("System".MatchNamespace()) || + n.NamespacesUsed.Count() != 3) // Must not be used more than 3 assemblies + // AsmCe = Efferent Coupling for assembly +&& + (n.Name == @"N1.N2") +select new { n, n.NamespacesUsed } + +// +// This rule is a *sample rule that can be adapted to your need*. +// +// It shows how to enforce that a particular namespace +// **is only using** a particular set of namespaces. +// + +// +// This is a *sample rule* there is nothing to fix *as is*. +//]]> + Check that AsmDrawing is the only assembly that is using System.Drawing +warnif count> 0 from a in Application.Assemblies where + a.IsUsing ("System.Drawing".AllowNoMatch().MatchAssembly()) && + !(a.Name == @"AsmDrawing") +select a + +// +// This rule is a *sample rule that can be adapted to your need*. +// +// It shows how to enforce that a particular assembly +// is **only used by** another particular assembly. +// + +// +// This is a *sample rule* there is nothing to fix *as is*. +//]]> + Check that only 3 assemblies are using System.Drawing +warnif count != 3 from a in Application.Assemblies where + a.IsUsing ("System.Drawing".AllowNoMatch().MatchAssembly()) +select a + +// +// This rule is a *sample rule that can be adapted to your need*. +// +// It shows how to enforce that a particular assembly +// is **only used by** 3 any others assemblies. +// + +// +// This is a *sample rule* there is nothing to fix *as is*. +//]]> + Check that all methods that call Foo.Fct1() also call Foo.Fct2(Int32) +warnif count > 0 from m in Application.Methods where + m.IsUsing ("Foo.Fct1()".AllowNoMatch()) && + !m.IsUsing ("Foo.Fct2(Int32)".AllowNoMatch()) +select m + +// +// This rule is a *sample rule that can be adapted to your need*. +// +// It shows how to enforce that if a method calls a particular method, +// it must call another particular method. +// + +// +// This is a *sample rule* there is nothing to fix *as is*. +//]]> + Check that all types that derive from Foo, also implement IFoo +warnif count > 0 from t in Application.Types where + t.DeriveFrom ("Foo".AllowNoMatch().MatchType()) && + !t.Implement ("IFoo".AllowNoMatch().MatchType()) +select t + +// +// This rule is a *sample rule that can be adapted to your need*. +// +// It shows how to enforce that all classes that derive from a particular base class, +// also implement a particular interface. +// + +// +// This is a *sample rule* there is nothing to fix *as is*. +//]]> + Check that all types that has the attribute FooAttribute are declared in the namespace N1.N2* +warnif count > 0 from t in + Application.Namespaces.WithNameWildcardMatchNotIn("N1.N2*").ChildTypes() + where + t.HasAttribute ("FooAttribute".AllowNoMatch()) +select t + +// +// This rule is a *sample rule that can be adapted to your need*. +// +// It shows how to enforce that all types that are tagged +// with a particular attribute, are declared in a +// particular namespace. +// + +// +// This is a *sample rule* there is nothing to fix *as is*. +//]]> + Check that all synchronization objects are only used from the namespaces under MyNamespace.Sync +warnif count > 0 from n in Application.Namespaces + where + (n.IsUsing ("System.Threading.Monitor".AllowNoMatch()) || + n.IsUsing ("System.Threading.ReaderWriterLock".AllowNoMatch()) || + n.IsUsing ("System.Threading.Mutex".AllowNoMatch()) || + n.IsUsing ("System.Threading.EventWaitHandle".AllowNoMatch()) || + n.IsUsing ("System.Threading.Semaphore".AllowNoMatch()) || + n.IsUsing ("System.Threading.Interlocked".AllowNoMatch())) + && !n.NameLike (@"^MyNamespace.Sync") +select n + +// +// This rule is a *sample rule that can be adapted to your need*. +// +// It shows how to enforce that all synchronization objects +// are used from a particular namespace. +// + +// +// This is a *sample rule* there is nothing to fix *as is*. +//]]> + + Check that the assembly Asm is 100% covered by tests +warnif count > 0 from a in Application.Assemblies where + (a.Name == @"Asm") && + a.PercentageCoverage < 100 +select new { a, a.PercentageCoverage } + +// +// This is a sample rule that shows how to check +// if a particular assembly is 100% covered by tests. +// Both the string **@"Asm"** and the threshold **100** can be adapted to your own needs. +// +// To execute this sample rule, coverage data must be imported. +// More info here: http://www.ndepend.com/docs/code-coverage +// + +// +// This is a *sample rule* there is nothing to fix *as is*. +//]]> + Check that the namespace N1.N2 is 100% covered by tests +warnif count > 0 from n in Application.Namespaces where + (n.Name == @"N1.N2") && + n.PercentageCoverage < 100 +select new { n, n.PercentageCoverage } + +// +// This is a sample rule that shows how to check +// if a particular namespace is 100% covered by tests. +// Both the string **@"N1.N2"** and the threshold **100** can be adapted to your own needs. +// +// To execute this sample rule, coverage data must be imported. +// More info here: http://www.ndepend.com/docs/code-coverage +// + +// +// This is a *sample rule* there is nothing to fix *as is*. +//]]> + Check that the class Namespace.Foo is 100% covered by tests +warnif count > 0 from t in Application.Types where + (t.FullName == @"Namespace.Foo") && + t.PercentageCoverage < 100 +select new { t, t.PercentageCoverage } + +// +// This is a sample rule that shows how to check +// if a particular class is 100% covered by tests. +// Both the string **@"Namespace.Foo"** and the threshold **100** +// can be adapted to your own needs. +// +// To execute this sample rule, coverage data must be imported. +// More info here: http://www.ndepend.com/docs/code-coverage +// + +// +// This is a *sample rule* there is nothing to fix *as is*. +//]]> + Check that the class Namespace.Foo.Method(Int32) is 100% covered by tests +warnif count > 0 from t in Application.Types where + (t.FullName == @"Namespace.Foo.Method(Int32)") && + t.PercentageCoverage < 100 +select new { t, t.PercentageCoverage } + + +// +// This is a sample rule that shows how to check +// if a particular method is 100% covered by tests. +// Both the string **@"Namespace.Foo.Method(Int32)"** and the threshold **100** +// can be adapted to your own needs. +// +// To execute this sample rule, coverage data must be imported. +// More info here: http://www.ndepend.com/docs/code-coverage +// + +// +// This is a *sample rule* there is nothing to fix *as is*. +//]]> + + + Check that all types that derive from Foo, has a name that ends up with Foo +warnif count > 0 from t in Application.Types where + t.DeriveFrom ("Foo".AllowNoMatch().MatchType()) && + !t.NameLike (@"Foo$") +select new { t, t.NbLinesOfCode } + +// +// This rule is a *sample rule that can be adapted to your need*. +// +// It shows how to enforce that all classes that derive from +// a particular class, are named with a particular suffix. +// + +// +// This is a *sample rule* there is nothing to fix *as is*. +//]]> + Check that all namespaces begins with CompanyName.ProductName +warnif count > 0 from n in Application.Namespaces where + !n.NameLike (@"^CompanyName.ProductName") +select new { n, n.NbLinesOfCode } + +// +// A practice widely adopted is that, in a product source code, +// all namespaces start with "CompanyName.ProductName". +// +// This rule must be adapted with your own **"CompanyName.ProductName"**. +// + +// +// Update all namespaces definitions in source code to satisfy this rule. +//]]> + + + + \ No newline at end of file