Merge pull request #412 from BBasile/issue-406

disable Phobos-specific checks by default, close #406
This commit is contained in:
Richard Andrew Cattermole 2017-05-12 16:39:01 +12:00 committed by GitHub
commit 67834d50df
36 changed files with 148 additions and 113 deletions

View File

@ -118,6 +118,9 @@ Note that the "--skipTests" option is the equivalent of changing each
* Some assertions that check conditions that will always be true.
* Auto functions without return statement. The compiler doesn't see an omission and it infers 'void' as return type.
* `final` attribute is used but in this context it's a noop.
* Check for properly documented public functions ("Returns" and "Params" sections). Initially implemented to lint Phobos. By default disabled.
* Check for explicitly annotated unittests (_@system_ or _@safe_). Initially implemented to lint Phobos. By default disabled.
* Check for that imports are sorted. Initially implemented to lint Phobos. By default disabled.
* Virtual calls inside classes constructors.
#### Wishlist

View File

@ -40,10 +40,10 @@ private:
unittest
{
import analysis.helpers : assertAnalyzerWarnings;
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import std.stdio : stderr;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.alias_syntax_check = Check.enabled;
assertAnalyzerWarnings(q{
alias int abcde; // [warn]: Prefer the new "'alias' identifier '=' type ';'" syntax to the old "'alias' type identifier ';'" syntax.

View File

@ -39,9 +39,9 @@ class AsmStyleCheck : BaseAnalyzer
unittest
{
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.asm_style_check = Check.enabled;
assertAnalyzerWarnings(q{
void testAsm()

View File

@ -157,10 +157,10 @@ unittest
{
import std.stdio : stderr;
import std.format : format;
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import analysis.helpers : assertAnalyzerWarnings;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.auto_function_check = Check.enabled;
assertAnalyzerWarnings(q{
auto ref doStuff(){} // [warn]: %s

View File

@ -113,10 +113,10 @@ unittest
{
import std.stdio : stderr;
import std.format : format;
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import analysis.helpers : assertAnalyzerWarnings;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.auto_ref_assignment_check = Check.enabled;
assertAnalyzerWarnings(q{
int doStuff(T)(auto ref int a)

View File

@ -102,9 +102,9 @@ private:
unittest
{
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.builtin_property_names_check = Check.enabled;
assertAnalyzerWarnings(q{
class SomeClass

View File

@ -7,146 +7,160 @@ module analysis.config;
import inifiled;
/// Returns: A default configuration.
StaticAnalysisConfig defaultStaticAnalysisConfig()
{
StaticAnalysisConfig config;
config.fillConfig!(Check.enabled);
return config;
}
/// Describes how a check is operated.
enum Check: string
{
/// Check is disabled.
disabled = "disabled",
/// Check is enabled.
enabled = "enabled",
/// Check is enabled but not operated in the unittests.
skipTests = "skip-unittest"
}
void fillConfig(string check)(ref StaticAnalysisConfig config)
/// Applies the --skipTests switch, allowing to call Dscanner without config
/// and less noise related to the unittests.
void enabled2SkipTests(ref StaticAnalysisConfig config)
{
foreach (mem; __traits(allMembers, StaticAnalysisConfig))
{
static if (is(typeof(__traits(getMember, StaticAnalysisConfig, mem))))
static if (is(typeof(__traits(getMember, config, mem)) == string))
__traits(getMember, config, mem) = check;
{
if (__traits(getMember, config, mem) == Check.enabled)
__traits(getMember, config, mem) = Check.skipTests;
}
}
}
unittest
/// Returns a config with all the checks disabled.
StaticAnalysisConfig disabledConfig()
{
StaticAnalysisConfig c;
c.fillConfig!(Check.enabled);
assert(c.enum_array_literal_check == Check.enabled);
fillConfig!(Check.skipTests)(c);
assert(c.alias_syntax_check == Check.skipTests);
StaticAnalysisConfig config;
foreach (mem; __traits(allMembers, StaticAnalysisConfig))
{
static if (is(typeof(__traits(getMember, StaticAnalysisConfig, mem))))
static if (is(typeof(__traits(getMember, config, mem)) == string))
__traits(getMember, config, mem) = Check.disabled;
}
return config;
}
@INI("Configure which static analysis checks are enabled")
struct StaticAnalysisConfig
{
@INI("Check variable, class, struct, interface, union, and function names against the Phobos style guide")
string style_check = Check.disabled;
string style_check = Check.enabled;
@INI("Check for array literals that cause unnecessary allocation")
string enum_array_literal_check = Check.disabled;
string enum_array_literal_check = Check.enabled;
@INI("Check for poor exception handling practices")
string exception_check = Check.disabled;
string exception_check = Check.enabled;
@INI("Check for use of the deprecated 'delete' keyword")
string delete_check = Check.disabled;
string delete_check = Check.enabled;
@INI("Check for use of the deprecated floating point operators")
string float_operator_check = Check.disabled;
string float_operator_check = Check.enabled;
@INI("Check number literals for readability")
string number_style_check = Check.disabled;
string number_style_check = Check.enabled;
@INI("Checks that opEquals, opCmp, toHash, and toString are either const, immutable, or inout.")
string object_const_check = Check.disabled;
string object_const_check = Check.enabled;
@INI("Checks for .. expressions where the left side is larger than the right.")
string backwards_range_check = Check.disabled;
string backwards_range_check = Check.enabled;
@INI("Checks for if statements whose 'then' block is the same as the 'else' block")
string if_else_same_check = Check.disabled;
string if_else_same_check = Check.enabled;
@INI("Checks for some problems with constructors")
string constructor_check = Check.disabled;
string constructor_check = Check.enabled;
@INI("Checks for unused variables and function parameters")
string unused_variable_check = Check.disabled;
string unused_variable_check = Check.enabled;
@INI("Checks for unused labels")
string unused_label_check = Check.disabled;
string unused_label_check = Check.enabled;
@INI("Checks for duplicate attributes")
string duplicate_attribute = Check.disabled;
string duplicate_attribute = Check.enabled;
@INI("Checks that opEquals and toHash are both defined or neither are defined")
string opequals_tohash_check = Check.disabled;
string opequals_tohash_check = Check.enabled;
@INI("Checks for subtraction from .length properties")
string length_subtraction_check = Check.disabled;
string length_subtraction_check = Check.enabled;
@INI("Checks for methods or properties whose names conflict with built-in properties")
string builtin_property_names_check = Check.disabled;
string builtin_property_names_check = Check.enabled;
@INI("Checks for confusing code in inline asm statements")
string asm_style_check = Check.disabled;
string asm_style_check = Check.enabled;
@INI("Checks for confusing logical operator precedence")
string logical_precedence_check = Check.disabled;
string logical_precedence_check = Check.enabled;
@INI("Checks for undocumented public declarations")
string undocumented_declaration_check = Check.disabled;
string undocumented_declaration_check = Check.enabled;
@INI("Checks for poor placement of function attributes")
string function_attribute_check = Check.disabled;
string function_attribute_check = Check.enabled;
@INI("Checks for use of the comma operator")
string comma_expression_check = Check.disabled;
string comma_expression_check = Check.enabled;
@INI("Checks for local imports that are too broad")
string local_import_check = Check.disabled;
string local_import_check = Check.enabled;
@INI("Checks for variables that could be declared immutable")
string could_be_immutable_check = Check.disabled; // disabled by default for now
string could_be_immutable_check = Check.enabled;
@INI("Checks for redundant expressions in if statements")
string redundant_if_check = Check.disabled;
string redundant_if_check = Check.enabled;
@INI("Checks for redundant parenthesis")
string redundant_parens_check = Check.disabled;
string redundant_parens_check = Check.enabled;
@INI("Checks for mismatched argument and parameter names")
string mismatched_args_check = Check.disabled;
string mismatched_args_check = Check.enabled;
@INI("Checks for labels with the same name as variables")
string label_var_same_name_check = Check.disabled;
string label_var_same_name_check = Check.enabled;
@INI("Checks for lines longer than 120 characters")
string long_line_check = Check.disabled;
string long_line_check = Check.enabled;
@INI("Checks for assignment to auto-ref function parameters")
string auto_ref_assignment_check = Check.disabled;
string auto_ref_assignment_check = Check.enabled;
@INI("Checks for incorrect infinite range definitions")
string incorrect_infinite_range_check = Check.disabled;
string incorrect_infinite_range_check = Check.enabled;
@INI("Checks for asserts that are always true")
string useless_assert_check = Check.disabled;
string useless_assert_check = Check.enabled;
@INI("Check for uses of the old-style alias syntax")
string alias_syntax_check = Check.disabled;
string alias_syntax_check = Check.enabled;
@INI("Checks for else if that should be else static if")
string static_if_else_check = Check.disabled;
string static_if_else_check = Check.enabled;
@INI("Check for unclear lambda syntax")
string lambda_return_check = Check.disabled;
string lambda_return_check = Check.enabled;
@INI("Check for auto function without return statement")
string auto_function_check = Check.disabled;
string auto_function_check = Check.enabled;
@INI("Check for sortedness of imports")
string imports_sortedness = Check.disabled;

View File

@ -90,9 +90,9 @@ private:
unittest
{
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.constructor_check = Check.enabled;
assertAnalyzerWarnings(q{
class Cat // [warn]: This class has a zero-argument constructor as well as a constructor with one default argument. This can be confusing.

View File

@ -33,10 +33,10 @@ class DeleteCheck : BaseAnalyzer
unittest
{
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import analysis.helpers : assertAnalyzerWarnings;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.delete_check = Check.enabled;
assertAnalyzerWarnings(q{
void testDelete()

View File

@ -153,9 +153,9 @@ class DuplicateAttributeCheck : BaseAnalyzer
unittest
{
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.duplicate_attribute = Check.enabled;
assertAnalyzerWarnings(q{
class ExampleAttributes

View File

@ -56,10 +56,10 @@ unittest
{
import std.stdio : stderr;
import std.format : format;
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import analysis.helpers : assertAnalyzerWarnings;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.explicitly_annotated_unittests = Check.enabled;
assertAnalyzerWarnings(q{

View File

@ -219,12 +219,12 @@ public:
@system unittest
{
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import analysis.helpers : assertAnalyzerWarnings;
import std.stdio : stderr;
import std.format : format;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.final_attribute_check = Check.enabled;
// pass

View File

@ -42,9 +42,9 @@ class FloatOperatorCheck : BaseAnalyzer
unittest
{
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.float_operator_check = Check.enabled;
assertAnalyzerWarnings(q{
void testFish()

View File

@ -76,9 +76,9 @@ class IfElseSameCheck : BaseAnalyzer
unittest
{
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.if_else_same_check = Check.enabled;
assertAnalyzerWarnings(q{
void testSizeT()

View File

@ -104,10 +104,10 @@ unittest
{
import std.stdio : stderr;
import std.format : format;
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import analysis.helpers : assertAnalyzerWarnings;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.imports_sortedness = Check.enabled;
assertAnalyzerWarnings(q{

View File

@ -88,10 +88,10 @@ private:
unittest
{
import std.stdio : stderr;
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import std.format : format;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.incorrect_infinite_range_check = Check.enabled;
assertAnalyzerWarnings(q{struct InfiniteRange
{

View File

@ -125,10 +125,10 @@ private:
unittest
{
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import std.stdio : stderr;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.label_var_same_name_check = Check.enabled;
assertAnalyzerWarnings(q{
unittest

View File

@ -47,10 +47,10 @@ private:
unittest
{
import analysis.helpers : assertAnalyzerWarnings;
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import std.stdio : stderr;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.lambda_return_check = Check.enabled;
auto code = `

View File

@ -58,9 +58,9 @@ class LengthSubtractionCheck : BaseAnalyzer
unittest
{
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.length_subtraction_check = Check.enabled;
assertAnalyzerWarnings(q{
void testSizeT()

View File

@ -85,11 +85,11 @@ private:
@system unittest
{
import analysis.config : Check, StaticAnalysisConfig;
import analysis.config : Check, StaticAnalysisConfig, disabledConfig;
import analysis.helpers : assertAnalyzerWarnings;
import std.stdio : stderr;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.long_line_check = Check.enabled;
assertAnalyzerWarnings(q{
Window window = Platform.instance.createWindow("Дистанционное управление сварочным оборудованием", null);

View File

@ -84,9 +84,9 @@ private:
unittest
{
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.local_import_check = Check.enabled;
assertAnalyzerWarnings(q{
void testLocalImport()

View File

@ -48,9 +48,9 @@ class LogicPrecedenceCheck : BaseAnalyzer
unittest
{
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.logical_precedence_check = Check.enabled;
assertAnalyzerWarnings(q{
void testFish()

View File

@ -49,9 +49,9 @@ private:
unittest
{
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.number_style_check = Check.enabled;
assertAnalyzerWarnings(q{
void testNumbers()

View File

@ -71,9 +71,9 @@ class ObjectConstCheck : BaseAnalyzer
unittest
{
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.object_const_check = Check.enabled;
assertAnalyzerWarnings(q{
void testConsts()

View File

@ -89,9 +89,9 @@ class OpEqualsWithoutToHashCheck : BaseAnalyzer
unittest
{
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.opequals_tohash_check = Check.enabled;
assertAnalyzerWarnings(q{
// Success because it has opEquals and toHash

View File

@ -85,9 +85,9 @@ class PokemonExceptionCheck : BaseAnalyzer
unittest
{
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.exception_check = Check.enabled;
assertAnalyzerWarnings(q{
void testCatch()

View File

@ -286,15 +286,16 @@ version(unittest)
{
import std.stdio : stderr;
import std.format : format;
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import analysis.helpers : assertAnalyzerWarnings;
StaticAnalysisConfig sac = {properly_documented_public_functions: Check.enabled};
}
// missing params
unittest
{
StaticAnalysisConfig sac = disabledConfig;
sac.properly_documented_public_functions = Check.enabled;
assertAnalyzerWarnings(q{
/**
Some text
@ -367,6 +368,9 @@ unittest
// missing returns (only functions)
unittest
{
StaticAnalysisConfig sac = disabledConfig;
sac.properly_documented_public_functions = Check.enabled;
assertAnalyzerWarnings(q{
/**
Some text
@ -389,6 +393,9 @@ unittest
// ignore private
unittest
{
StaticAnalysisConfig sac = disabledConfig;
sac.properly_documented_public_functions = Check.enabled;
assertAnalyzerWarnings(q{
/**
Some text
@ -442,10 +449,12 @@ unittest
), sac);
}
// test parameter names
unittest
{
StaticAnalysisConfig sac = disabledConfig;
sac.properly_documented_public_functions = Check.enabled;
assertAnalyzerWarnings(q{
/**
* Description.
@ -557,6 +566,9 @@ struct foo(int foo, int bar){}
// support ditto
unittest
{
StaticAnalysisConfig sac = disabledConfig;
sac.properly_documented_public_functions = Check.enabled;
assertAnalyzerWarnings(q{
/**
* Description.
@ -651,6 +663,9 @@ int bar(int bar){}
// check correct ddoc headers
unittest
{
StaticAnalysisConfig sac = disabledConfig;
sac.properly_documented_public_functions = Check.enabled;
assertAnalyzerWarnings(q{
/++
Counts elements in the given
@ -684,6 +699,9 @@ template bar(string val){}
unittest
{
StaticAnalysisConfig sac = disabledConfig;
sac.properly_documented_public_functions = Check.enabled;
assertAnalyzerWarnings(q{
/**
* Ddoc for the inner function appears here.

View File

@ -157,9 +157,9 @@ private:
unittest
{
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.backwards_range_check = Check.enabled;
assertAnalyzerWarnings(q{
void testRange()

View File

@ -66,10 +66,10 @@ class StaticIfElse : BaseAnalyzer
unittest
{
import analysis.helpers : assertAnalyzerWarnings;
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import std.stdio : stderr;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.static_if_else_check = Check.enabled;
assertAnalyzerWarnings(q{
void foo() {

View File

@ -166,9 +166,9 @@ final class StyleChecker : BaseAnalyzer
unittest
{
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.style_check = Check.enabled;
assertAnalyzerWarnings(q{

View File

@ -316,12 +316,12 @@ bool isValueTypeSimple(const Type type) pure nothrow @nogc
@system unittest
{
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import analysis.helpers : assertAnalyzerWarnings;
import std.stdio : stderr;
import std.format : format;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.could_be_immutable_check = Check.enabled;
// pass

View File

@ -435,10 +435,10 @@ private:
unittest
{
import std.stdio : stderr;
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import analysis.helpers : assertAnalyzerWarnings;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.unused_variable_check = Check.enabled;
assertAnalyzerWarnings(q{

View File

@ -165,10 +165,10 @@ private:
unittest
{
import analysis.config : Check, StaticAnalysisConfig;
import analysis.config : Check, StaticAnalysisConfig, disabledConfig;
import std.stdio : stderr;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.unused_label_check = Check.enabled;
assertAnalyzerWarnings(q{
int testUnusedLabel()

View File

@ -96,10 +96,10 @@ private:
unittest
{
import std.stdio : stderr;
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import std.format : format;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.useless_assert_check = Check.enabled;
assertAnalyzerWarnings(q{
unittest

View File

@ -276,12 +276,12 @@ public:
unittest
{
import analysis.config : StaticAnalysisConfig, Check;
import analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import analysis.helpers : assertAnalyzerWarnings;
import std.stdio : stderr;
import std.format : format;
StaticAnalysisConfig sac;
StaticAnalysisConfig sac = disabledConfig();
sac.vcall_in_ctor = Check.enabled;
// fails

View File

@ -229,7 +229,7 @@ else
if (s.exists())
readINIFile(config, s);
if (skipTests)
config.fillConfig!(Check.skipTests);
config.enabled2SkipTests;
if (report)
generateReport(expandArgs(args), config, cache, moduleCache);
else