diff --git a/README.md b/README.md index ea81ef8..f4d04fc 100644 --- a/README.md +++ b/README.md @@ -43,6 +43,12 @@ analysis and it does not compile the code. The "--styleCheck" or "-S" option runs some basic static analysis checks against the given source files. +#### Skip style checks in the tests +Static checks in the unit tests can produce irrelevant warnings. For example, +it's legit to declare a variable that's not used if the goal is to verify that +a templatized function can be instantiated by inference of the type of this variable. +To avoid these cases, it's possible to pass the "--skipTests" option. + #### Configuration By default all checks are enabled. Individual checks can be enabled or disabled by using a configuration file. Running ```dscanner --defaultConfig``` will @@ -50,6 +56,16 @@ generate a default configuration file and print the file's location. The "--config" option will allow you to specify the path to a configuration file if you do not want to use the one created by the "--defaultConfig" option. +For each check, three values are possible: +* `"disabled"`: the check is not performed. +* `"enabled"`: the check is performed. +* `"skip-unittest"`: the check is performed but not in the unit tests. + +Any other value deactivates a check. + +Note that the "--skipTests" option is the equivalent of changing each +`"enabled"` check by a `"skip-unittest"` check. + #### Implemented checks * Old alias syntax (i.e "alias a b;" should be replaced with "alias b = a;"). * Implicit concatenation of string literals. @@ -85,7 +101,7 @@ you do not want to use the one created by the "--defaultConfig" option. * Unused labels. * Lines longer than 120 characters. * Incorrect infinite range definitions. -* Some assertions that check conditions that will always be true. +* Some assertions that check conditions that will always be true. This check can't be skipped in the tests. #### Wishlist diff --git a/src/analysis/alias_syntax_check.d b/src/analysis/alias_syntax_check.d index 939f19c..d6f35fb 100644 --- a/src/analysis/alias_syntax_check.d +++ b/src/analysis/alias_syntax_check.d @@ -16,9 +16,9 @@ class AliasSyntaxCheck : BaseAnalyzer { alias visit = BaseAnalyzer.visit; - this(string fileName) + this(string fileName, bool skipTests = false) { - super(fileName, null); + super(fileName, null, skipTests); } override void visit(const AliasDeclaration ad) @@ -40,11 +40,11 @@ private: unittest { import analysis.helpers : assertAnalyzerWarnings; - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; import std.stdio : stderr; StaticAnalysisConfig sac; - sac.alias_syntax_check = true; + 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. alias abcde = int; diff --git a/src/analysis/asm_style.d b/src/analysis/asm_style.d index df0e06e..e2c2b08 100644 --- a/src/analysis/asm_style.d +++ b/src/analysis/asm_style.d @@ -20,9 +20,9 @@ class AsmStyleCheck : BaseAnalyzer { alias visit = BaseAnalyzer.visit; - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } override void visit(const AsmBrExp brExp) @@ -39,10 +39,10 @@ class AsmStyleCheck : BaseAnalyzer unittest { - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; StaticAnalysisConfig sac; - sac.asm_style_check = true; + sac.asm_style_check = Check.enabled; assertAnalyzerWarnings(q{ void testAsm() { diff --git a/src/analysis/auto_ref_assignment.d b/src/analysis/auto_ref_assignment.d index 4676e0d..1cb7697 100644 --- a/src/analysis/auto_ref_assignment.d +++ b/src/analysis/auto_ref_assignment.d @@ -15,9 +15,9 @@ import analysis.base; class AutoRefAssignmentCheck : BaseAnalyzer { /// - this(string fileName) + this(string fileName, bool skipTests = false) { - super(fileName, null); + super(fileName, null, skipTests); } override void visit(const Module m) @@ -113,11 +113,11 @@ unittest { import std.stdio : stderr; import std.format : format; - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; import analysis.helpers : assertAnalyzerWarnings; StaticAnalysisConfig sac; - sac.auto_ref_assignment_check = true; + sac.auto_ref_assignment_check = Check.enabled; assertAnalyzerWarnings(q{ int doStuff(T)(auto ref int a) { diff --git a/src/analysis/base.d b/src/analysis/base.d index 18752fa..193b971 100644 --- a/src/analysis/base.d +++ b/src/analysis/base.d @@ -27,10 +27,11 @@ alias MessageSet = RedBlackTree!(Message, comparitor, true); abstract class BaseAnalyzer : ASTVisitor { public: - this(string fileName, const Scope* sc) + this(string fileName, const Scope* sc, bool skipTests = false) { this.sc = sc; this.fileName = fileName; + this.skipTests = skipTests; _messages = new MessageSet; } @@ -39,9 +40,24 @@ public: return _messages[].array; } + alias visit = ASTVisitor.visit; + + /** + * Visits a unittest. + * + * When overriden, the protected bool "skipTests" should be handled + * so that the content of the test is not analyzed. + */ + override void visit(const Unittest unittest_) + { + if (!skipTests) + unittest_.accept(this); + } + protected: bool inAggregate = false; + bool skipTests; template visitTemplate(T) { diff --git a/src/analysis/builtin_property_names.d b/src/analysis/builtin_property_names.d index 51cbe08..792f07d 100644 --- a/src/analysis/builtin_property_names.d +++ b/src/analysis/builtin_property_names.d @@ -30,9 +30,9 @@ class BuiltinPropertyNameCheck : BaseAnalyzer { alias visit = BaseAnalyzer.visit; - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } override void visit(const FunctionDeclaration fd) @@ -101,10 +101,10 @@ private: unittest { - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; StaticAnalysisConfig sac; - sac.builtin_property_names_check = true; + sac.builtin_property_names_check = Check.enabled; assertAnalyzerWarnings(q{ class SomeClass { diff --git a/src/analysis/comma_expression.d b/src/analysis/comma_expression.d index 570af3a..fae7eba 100644 --- a/src/analysis/comma_expression.d +++ b/src/analysis/comma_expression.d @@ -17,9 +17,9 @@ class CommaExpressionCheck : BaseAnalyzer { alias visit = BaseAnalyzer.visit; - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } override void visit(const Expression ex) diff --git a/src/analysis/config.d b/src/analysis/config.d index 7dc2625..bd178b0 100644 --- a/src/analysis/config.d +++ b/src/analysis/config.d @@ -10,113 +10,138 @@ import inifiled; StaticAnalysisConfig defaultStaticAnalysisConfig() { StaticAnalysisConfig config; - foreach (mem; __traits(allMembers, StaticAnalysisConfig)) - mixin("config." ~ mem ~ " = true;"); + config.fillConfig!(Check.enabled); return config; } +enum Check: string +{ + disabled = "disabled", + enabled = "enabled", + skipTests = "skip-unittest" +} + +void fillConfig(string check)(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; + } +} + +unittest +{ + 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); +} + @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") - bool style_check; + string style_check = Check.disabled; @INI("Check for array literals that cause unnecessary allocation") - bool enum_array_literal_check; + string enum_array_literal_check = Check.disabled; @INI("Check for poor exception handling practices") - bool exception_check; + string exception_check = Check.disabled; @INI("Check for use of the deprecated 'delete' keyword") - bool delete_check; + string delete_check = Check.disabled; @INI("Check for use of the deprecated floating point operators") - bool float_operator_check; + string float_operator_check = Check.disabled; @INI("Check number literals for readability") - bool number_style_check; + string number_style_check = Check.disabled; @INI("Checks that opEquals, opCmp, toHash, and toString are either const, immutable, or inout.") - bool object_const_check; + string object_const_check = Check.disabled; @INI("Checks for .. expressions where the left side is larger than the right.") - bool backwards_range_check; + string backwards_range_check = Check.disabled; @INI("Checks for if statements whose 'then' block is the same as the 'else' block") - bool if_else_same_check; + string if_else_same_check = Check.disabled; @INI("Checks for some problems with constructors") - bool constructor_check; + string constructor_check = Check.disabled; @INI("Checks for unused variables and function parameters") - bool unused_variable_check; + string unused_variable_check = Check.disabled; @INI("Checks for unused labels") - bool unused_label_check; + string unused_label_check = Check.disabled; @INI("Checks for duplicate attributes") - bool duplicate_attribute; + string duplicate_attribute = Check.disabled; @INI("Checks that opEquals and toHash are both defined or neither are defined") - bool opequals_tohash_check; + string opequals_tohash_check = Check.disabled; @INI("Checks for subtraction from .length properties") - bool length_subtraction_check; + string length_subtraction_check = Check.disabled; @INI("Checks for methods or properties whose names conflict with built-in properties") - bool builtin_property_names_check; + string builtin_property_names_check = Check.disabled; @INI("Checks for confusing code in inline asm statements") - bool asm_style_check; + string asm_style_check = Check.disabled; @INI("Checks for confusing logical operator precedence") - bool logical_precedence_check; + string logical_precedence_check = Check.disabled; @INI("Checks for undocumented public declarations") - bool undocumented_declaration_check; + string undocumented_declaration_check = Check.disabled; @INI("Checks for poor placement of function attributes") - bool function_attribute_check; + string function_attribute_check = Check.disabled; @INI("Checks for use of the comma operator") - bool comma_expression_check; + string comma_expression_check = Check.disabled; @INI("Checks for local imports that are too broad") - bool local_import_check; + string local_import_check = Check.disabled; @INI("Checks for variables that could be declared immutable") - bool could_be_immutable_check = false; // disabled by default for now + string could_be_immutable_check = Check.disabled; // disabled by default for now @INI("Checks for redundant expressions in if statements") - bool redundant_if_check; + string redundant_if_check = Check.disabled; @INI("Checks for redundant parenthesis") - bool redundant_parens_check; + string redundant_parens_check = Check.disabled; @INI("Checks for mismatched argument and parameter names") - bool mismatched_args_check; + string mismatched_args_check = Check.disabled; @INI("Checks for labels with the same name as variables") - bool label_var_same_name_check; + string label_var_same_name_check = Check.disabled; @INI("Checks for lines longer than 120 characters") - bool long_line_check; + string long_line_check = Check.disabled; @INI("Checks for assignment to auto-ref function parameters") - bool auto_ref_assignment_check; + string auto_ref_assignment_check = Check.disabled; @INI("Checks for incorrect infinite range definitions") - bool incorrect_infinite_range_check; + string incorrect_infinite_range_check = Check.disabled; @INI("Checks for asserts that are always true") - bool useless_assert_check; + string useless_assert_check = Check.disabled; @INI("Check for uses of the old-style alias syntax") - bool alias_syntax_check; + string alias_syntax_check = Check.disabled; @INI("Checks for else if that should be else static if") - bool static_if_else_check; + string static_if_else_check = Check.disabled; @INI("Check for unclear lambda syntax") - bool lambda_return_check; + string lambda_return_check = Check.disabled; } diff --git a/src/analysis/constructors.d b/src/analysis/constructors.d index 6fec08a..ac3853b 100644 --- a/src/analysis/constructors.d +++ b/src/analysis/constructors.d @@ -11,9 +11,9 @@ class ConstructorCheck : BaseAnalyzer { alias visit = BaseAnalyzer.visit; - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } override void visit(const ClassDeclaration classDeclaration) @@ -90,10 +90,10 @@ private: unittest { - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; StaticAnalysisConfig sac; - sac.constructor_check = true; + 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. { diff --git a/src/analysis/del.d b/src/analysis/del.d index b61206f..25f0a47 100644 --- a/src/analysis/del.d +++ b/src/analysis/del.d @@ -18,9 +18,9 @@ class DeleteCheck : BaseAnalyzer { alias visit = BaseAnalyzer.visit; - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } override void visit(const DeleteExpression d) @@ -33,11 +33,11 @@ class DeleteCheck : BaseAnalyzer unittest { - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; import analysis.helpers : assertAnalyzerWarnings; StaticAnalysisConfig sac; - sac.delete_check = true; + sac.delete_check = Check.enabled; assertAnalyzerWarnings(q{ void testDelete() { diff --git a/src/analysis/duplicate_attribute.d b/src/analysis/duplicate_attribute.d index 90ab6b0..d77c78f 100644 --- a/src/analysis/duplicate_attribute.d +++ b/src/analysis/duplicate_attribute.d @@ -21,9 +21,9 @@ class DuplicateAttributeCheck : BaseAnalyzer { alias visit = BaseAnalyzer.visit; - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } override void visit(const Declaration node) @@ -153,10 +153,10 @@ class DuplicateAttributeCheck : BaseAnalyzer unittest { - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; StaticAnalysisConfig sac; - sac.duplicate_attribute = true; + sac.duplicate_attribute = Check.enabled; assertAnalyzerWarnings(q{ class ExampleAttributes { diff --git a/src/analysis/enumarrayliteral.d b/src/analysis/enumarrayliteral.d index 108d59a..87dc13f 100644 --- a/src/analysis/enumarrayliteral.d +++ b/src/analysis/enumarrayliteral.d @@ -19,9 +19,9 @@ class EnumArrayLiteralCheck : BaseAnalyzer { alias visit = BaseAnalyzer.visit; - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } bool looking = false; diff --git a/src/analysis/fish.d b/src/analysis/fish.d index 139c1cb..698ccb8 100644 --- a/src/analysis/fish.d +++ b/src/analysis/fish.d @@ -21,9 +21,9 @@ class FloatOperatorCheck : BaseAnalyzer enum string KEY = "dscanner.deprecated.floating_point_operators"; - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } override void visit(const RelExpression r) @@ -42,10 +42,10 @@ class FloatOperatorCheck : BaseAnalyzer unittest { - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; StaticAnalysisConfig sac; - sac.float_operator_check = true; + sac.float_operator_check = Check.enabled; assertAnalyzerWarnings(q{ void testFish() { diff --git a/src/analysis/function_attributes.d b/src/analysis/function_attributes.d index 28687da..f060823 100644 --- a/src/analysis/function_attributes.d +++ b/src/analysis/function_attributes.d @@ -25,9 +25,9 @@ class FunctionAttributeCheck : BaseAnalyzer { alias visit = BaseAnalyzer.visit; - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } override void visit(const InterfaceDeclaration dec) diff --git a/src/analysis/if_statements.d b/src/analysis/if_statements.d index 09a7b09..b1ad99f 100644 --- a/src/analysis/if_statements.d +++ b/src/analysis/if_statements.d @@ -13,9 +13,9 @@ import dsymbol.scope_ : Scope; class IfStatementCheck : BaseAnalyzer { alias visit = BaseAnalyzer.visit; - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } override void visit(const IfStatement ifStatement) diff --git a/src/analysis/ifelsesame.d b/src/analysis/ifelsesame.d index 0dc9f78..b3e1c58 100644 --- a/src/analysis/ifelsesame.d +++ b/src/analysis/ifelsesame.d @@ -24,9 +24,9 @@ class IfElseSameCheck : BaseAnalyzer { alias visit = BaseAnalyzer.visit; - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } override void visit(const IfStatement ifStatement) @@ -77,10 +77,10 @@ class IfElseSameCheck : BaseAnalyzer unittest { - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; StaticAnalysisConfig sac; - sac.if_else_same_check = true; + sac.if_else_same_check = Check.enabled; assertAnalyzerWarnings(q{ void testSizeT() { diff --git a/src/analysis/incorrect_infinite_range.d b/src/analysis/incorrect_infinite_range.d index 9de856d..0cdcd20 100644 --- a/src/analysis/incorrect_infinite_range.d +++ b/src/analysis/incorrect_infinite_range.d @@ -18,9 +18,9 @@ class IncorrectInfiniteRangeCheck : BaseAnalyzer alias visit = BaseAnalyzer.visit; /// - this(string fileName) + this(string fileName, bool skipTests = false) { - super(fileName, null); + super(fileName, null, skipTests); } override void visit(const StructBody structBody) @@ -88,11 +88,11 @@ private: unittest { import std.stdio : stderr; - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; import std.format : format; StaticAnalysisConfig sac; - sac.incorrect_infinite_range_check = true; + sac.incorrect_infinite_range_check = Check.enabled; assertAnalyzerWarnings(q{struct InfiniteRange { bool empty() // [warn]: %1$s diff --git a/src/analysis/label_var_same_name_check.d b/src/analysis/label_var_same_name_check.d index 480b5e7..6588c73 100644 --- a/src/analysis/label_var_same_name_check.d +++ b/src/analysis/label_var_same_name_check.d @@ -15,9 +15,9 @@ import analysis.helpers; */ class LabelVarNameCheck : BaseAnalyzer { - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } mixin ScopedVisit!Module; @@ -118,11 +118,11 @@ private: unittest { - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; import std.stdio : stderr; StaticAnalysisConfig sac; - sac.label_var_same_name_check = true; + sac.label_var_same_name_check = Check.enabled; assertAnalyzerWarnings(q{ unittest { diff --git a/src/analysis/lambda_return_check.d b/src/analysis/lambda_return_check.d index e16cea8..47750f0 100644 --- a/src/analysis/lambda_return_check.d +++ b/src/analysis/lambda_return_check.d @@ -13,10 +13,10 @@ class LambdaReturnCheck : BaseAnalyzer { alias visit = BaseAnalyzer.visit; - this(string fileName) - { - super(fileName, null); - } + this(string fileName, bool skipTests = false) + { + super(fileName, null, skipTests); + } override void visit(const FunctionLiteralExpression fLit) { @@ -47,11 +47,11 @@ private: unittest { import analysis.helpers : assertAnalyzerWarnings; - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; import std.stdio : stderr; StaticAnalysisConfig sac; - sac.lambda_return_check = true; + sac.lambda_return_check = Check.enabled; auto code = ` void main() diff --git a/src/analysis/length_subtraction.d b/src/analysis/length_subtraction.d index cf6736b..ddb5273 100644 --- a/src/analysis/length_subtraction.d +++ b/src/analysis/length_subtraction.d @@ -20,9 +20,9 @@ class LengthSubtractionCheck : BaseAnalyzer { alias visit = BaseAnalyzer.visit; - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } override void visit(const AddExpression addExpression) @@ -58,10 +58,10 @@ class LengthSubtractionCheck : BaseAnalyzer unittest { - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; StaticAnalysisConfig sac; - sac.length_subtraction_check = true; + sac.length_subtraction_check = Check.enabled; assertAnalyzerWarnings(q{ void testSizeT() { diff --git a/src/analysis/line_length.d b/src/analysis/line_length.d index 58f48ee..94ac173 100644 --- a/src/analysis/line_length.d +++ b/src/analysis/line_length.d @@ -15,9 +15,9 @@ import analysis.base : BaseAnalyzer; class LineLengthCheck : BaseAnalyzer { /// - this(string fileName, const(Token)[] tokens) + this(string fileName, const(Token)[] tokens, bool skipTests = false) { - super(fileName, null); + super(fileName, null, skipTests); this.tokens = tokens; } diff --git a/src/analysis/local_imports.d b/src/analysis/local_imports.d index 18c2235..a87b08b 100644 --- a/src/analysis/local_imports.d +++ b/src/analysis/local_imports.d @@ -23,9 +23,9 @@ class LocalImportCheck : BaseAnalyzer /** * Construct with the given file name. */ - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } mixin visitThing!StructBody; @@ -84,10 +84,10 @@ private: unittest { - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; StaticAnalysisConfig sac; - sac.local_import_check = true; + sac.local_import_check = Check.enabled; assertAnalyzerWarnings(q{ void testLocalImport() { diff --git a/src/analysis/logic_precedence.d b/src/analysis/logic_precedence.d index 7058516..7dc5e3e 100644 --- a/src/analysis/logic_precedence.d +++ b/src/analysis/logic_precedence.d @@ -25,9 +25,9 @@ class LogicPrecedenceCheck : BaseAnalyzer enum string KEY = "dscanner.confusing.logical_precedence"; - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } override void visit(const OrOrExpression orOr) @@ -48,10 +48,10 @@ class LogicPrecedenceCheck : BaseAnalyzer unittest { - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; StaticAnalysisConfig sac; - sac.logical_precedence_check = true; + sac.logical_precedence_check = Check.enabled; assertAnalyzerWarnings(q{ void testFish() { diff --git a/src/analysis/mismatched_args.d b/src/analysis/mismatched_args.d index 8411d9d..e66f6af 100644 --- a/src/analysis/mismatched_args.d +++ b/src/analysis/mismatched_args.d @@ -11,9 +11,9 @@ import dsymbol.builtin.names; final class MismatchedArgumentCheck : BaseAnalyzer { /// - this(string fileName, const Scope* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } override void visit(const FunctionCallExpression fce) diff --git a/src/analysis/numbers.d b/src/analysis/numbers.d index 3b07662..15f2f59 100644 --- a/src/analysis/numbers.d +++ b/src/analysis/numbers.d @@ -24,9 +24,9 @@ public: /** * Constructs the style checker with the given file name. */ - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } override void visit(const Token t) @@ -49,10 +49,10 @@ private: unittest { - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; StaticAnalysisConfig sac; - sac.number_style_check = true; + sac.number_style_check = Check.enabled; assertAnalyzerWarnings(q{ void testNumbers() { diff --git a/src/analysis/objectconst.d b/src/analysis/objectconst.d index db8997b..a398109 100644 --- a/src/analysis/objectconst.d +++ b/src/analysis/objectconst.d @@ -21,9 +21,9 @@ class ObjectConstCheck : BaseAnalyzer { alias visit = BaseAnalyzer.visit; - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } mixin visitTemplate!ClassDeclaration; @@ -71,10 +71,10 @@ class ObjectConstCheck : BaseAnalyzer unittest { - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; StaticAnalysisConfig sac; - sac.object_const_check = true; + sac.object_const_check = Check.enabled; assertAnalyzerWarnings(q{ void testConsts() { diff --git a/src/analysis/opequals_without_tohash.d b/src/analysis/opequals_without_tohash.d index f1e08e8..75ded4b 100644 --- a/src/analysis/opequals_without_tohash.d +++ b/src/analysis/opequals_without_tohash.d @@ -20,9 +20,9 @@ class OpEqualsWithoutToHashCheck : BaseAnalyzer { alias visit = BaseAnalyzer.visit; - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } override void visit(const ClassDeclaration node) @@ -89,10 +89,10 @@ class OpEqualsWithoutToHashCheck : BaseAnalyzer unittest { - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; StaticAnalysisConfig sac; - sac.opequals_tohash_check = true; + sac.opequals_tohash_check = Check.enabled; assertAnalyzerWarnings(q{ // Success because it has opEquals and toHash class Chimp diff --git a/src/analysis/pokemon.d b/src/analysis/pokemon.d index f6488da..773647b 100644 --- a/src/analysis/pokemon.d +++ b/src/analysis/pokemon.d @@ -30,9 +30,9 @@ class PokemonExceptionCheck : BaseAnalyzer alias visit = BaseAnalyzer.visit; - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } override void visit(const LastCatch lc) @@ -85,10 +85,10 @@ class PokemonExceptionCheck : BaseAnalyzer unittest { - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; StaticAnalysisConfig sac; - sac.exception_check = true; + sac.exception_check = Check.enabled; assertAnalyzerWarnings(q{ void testCatch() { diff --git a/src/analysis/range.d b/src/analysis/range.d index cccd124..b4547bb 100644 --- a/src/analysis/range.d +++ b/src/analysis/range.d @@ -27,9 +27,9 @@ class BackwardsRangeCheck : BaseAnalyzer * Params: * fileName = the name of the file being analyzed */ - this(string fileName, const(Scope)* sc) + this(string fileName, const Scope* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } override void visit(const ForeachStatement foreachStatement) @@ -157,10 +157,10 @@ private: unittest { - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; StaticAnalysisConfig sac; - sac.backwards_range_check = true; + sac.backwards_range_check = Check.enabled; assertAnalyzerWarnings(q{ void testRange() { diff --git a/src/analysis/redundant_parens.d b/src/analysis/redundant_parens.d index 8298ff5..871842e 100644 --- a/src/analysis/redundant_parens.d +++ b/src/analysis/redundant_parens.d @@ -18,9 +18,9 @@ class RedundantParenCheck : BaseAnalyzer alias visit = BaseAnalyzer.visit; /// - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } override void visit(const IfStatement statement) diff --git a/src/analysis/run.d b/src/analysis/run.d index 291c8e2..2cb2260 100644 --- a/src/analysis/run.d +++ b/src/analysis/run.d @@ -205,8 +205,10 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a return null; auto symbolAllocator = scoped!ASTAllocator(); + version (unittest) enum ut = true; else enum ut = false; + auto first = scoped!FirstPass(m, internString(fileName), symbolAllocator, - symbolAllocator, true, &moduleCache, null); + symbolAllocator, true, &moduleCache, null); first.run(); secondPass(first.rootSymbol, first.moduleScope, moduleCache); @@ -216,75 +218,142 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a scope(exit) typeid(Scope).destroy(first.moduleScope); BaseAnalyzer[] checks; - if (analysisConfig.asm_style_check) - checks ~= new AsmStyleCheck(fileName, moduleScope); - if (analysisConfig.backwards_range_check) - checks ~= new BackwardsRangeCheck(fileName, moduleScope); - if (analysisConfig.builtin_property_names_check) - checks ~= new BuiltinPropertyNameCheck(fileName, moduleScope); - if (analysisConfig.comma_expression_check) - checks ~= new CommaExpressionCheck(fileName, moduleScope); - if (analysisConfig.constructor_check) - checks ~= new ConstructorCheck(fileName, moduleScope); - if (analysisConfig.could_be_immutable_check) - checks ~= new UnmodifiedFinder(fileName, moduleScope); - if (analysisConfig.delete_check) - checks ~= new DeleteCheck(fileName, moduleScope); - if (analysisConfig.duplicate_attribute) - checks ~= new DuplicateAttributeCheck(fileName, moduleScope); - if (analysisConfig.enum_array_literal_check) - checks ~= new EnumArrayLiteralCheck(fileName, moduleScope); - if (analysisConfig.exception_check) - checks ~= new PokemonExceptionCheck(fileName, moduleScope); - if (analysisConfig.float_operator_check) - checks ~= new FloatOperatorCheck(fileName, moduleScope); - if (analysisConfig.function_attribute_check) - checks ~= new FunctionAttributeCheck(fileName, moduleScope); - if (analysisConfig.if_else_same_check) - checks ~= new IfElseSameCheck(fileName, moduleScope); - if (analysisConfig.label_var_same_name_check) - checks ~= new LabelVarNameCheck(fileName, moduleScope); - if (analysisConfig.length_subtraction_check) - checks ~= new LengthSubtractionCheck(fileName, moduleScope); - if (analysisConfig.local_import_check) - checks ~= new LocalImportCheck(fileName, moduleScope); - if (analysisConfig.logical_precedence_check) - checks ~= new LogicPrecedenceCheck(fileName, moduleScope); - if (analysisConfig.mismatched_args_check) - checks ~= new MismatchedArgumentCheck(fileName, moduleScope); - if (analysisConfig.number_style_check) - checks ~= new NumberStyleCheck(fileName, moduleScope); - if (analysisConfig.object_const_check) - checks ~= new ObjectConstCheck(fileName, moduleScope); - if (analysisConfig.opequals_tohash_check) - checks ~= new OpEqualsWithoutToHashCheck(fileName, moduleScope); - if (analysisConfig.redundant_parens_check) - checks ~= new RedundantParenCheck(fileName, moduleScope); - if (analysisConfig.style_check) - checks ~= new StyleChecker(fileName, moduleScope); - if (analysisConfig.undocumented_declaration_check) - checks ~= new UndocumentedDeclarationCheck(fileName, moduleScope); - if (analysisConfig.unused_label_check) - checks ~= new UnusedLabelCheck(fileName, moduleScope); - if (analysisConfig.unused_variable_check) - checks ~= new UnusedVariableCheck(fileName, moduleScope); - if (analysisConfig.long_line_check) - checks ~= new LineLengthCheck(fileName, tokens); - if (analysisConfig.auto_ref_assignment_check) - checks ~= new AutoRefAssignmentCheck(fileName); - if (analysisConfig.incorrect_infinite_range_check) - checks ~= new IncorrectInfiniteRangeCheck(fileName); - if (analysisConfig.useless_assert_check) - checks ~= new UselessAssertCheck(fileName); - if (analysisConfig.alias_syntax_check) - checks ~= new AliasSyntaxCheck(fileName); - if (analysisConfig.static_if_else_check) - checks ~= new StaticIfElse(fileName); - if (analysisConfig.lambda_return_check) - checks ~= new LambdaReturnCheck(fileName); + if (analysisConfig.asm_style_check != Check.disabled) + checks ~= new AsmStyleCheck(fileName, moduleScope, + analysisConfig.asm_style_check == Check.skipTests && !ut); + + if (analysisConfig.backwards_range_check != Check.disabled) + checks ~= new BackwardsRangeCheck(fileName, moduleScope, + analysisConfig.backwards_range_check == Check.skipTests && !ut); + + if (analysisConfig.builtin_property_names_check != Check.disabled) + checks ~= new BuiltinPropertyNameCheck(fileName, moduleScope, + analysisConfig.builtin_property_names_check == Check.skipTests && !ut); + + if (analysisConfig.comma_expression_check != Check.disabled) + checks ~= new CommaExpressionCheck(fileName, moduleScope, + analysisConfig.comma_expression_check == Check.skipTests && !ut); + + if (analysisConfig.constructor_check != Check.disabled) + checks ~= new ConstructorCheck(fileName, moduleScope, + analysisConfig.constructor_check == Check.skipTests && !ut); + + if (analysisConfig.could_be_immutable_check != Check.disabled) + checks ~= new UnmodifiedFinder(fileName, moduleScope, + analysisConfig.could_be_immutable_check == Check.skipTests && !ut); + + if (analysisConfig.delete_check != Check.disabled) + checks ~= new DeleteCheck(fileName, moduleScope, + analysisConfig.delete_check == Check.skipTests && !ut); + + if (analysisConfig.duplicate_attribute != Check.disabled) + checks ~= new DuplicateAttributeCheck(fileName, moduleScope, + analysisConfig.duplicate_attribute == Check.skipTests && !ut); + + if (analysisConfig.enum_array_literal_check != Check.disabled) + checks ~= new EnumArrayLiteralCheck(fileName, moduleScope, + analysisConfig.enum_array_literal_check == Check.skipTests && !ut); + + if (analysisConfig.exception_check != Check.disabled) + checks ~= new PokemonExceptionCheck(fileName, moduleScope, + analysisConfig.exception_check == Check.skipTests && !ut); + + if (analysisConfig.float_operator_check != Check.disabled) + checks ~= new FloatOperatorCheck(fileName, moduleScope, + analysisConfig.float_operator_check == Check.skipTests && !ut); + + if (analysisConfig.function_attribute_check != Check.disabled) + checks ~= new FunctionAttributeCheck(fileName, moduleScope, + analysisConfig.function_attribute_check == Check.skipTests && !ut); + + if (analysisConfig.if_else_same_check != Check.disabled) + checks ~= new IfElseSameCheck(fileName, moduleScope, + analysisConfig.if_else_same_check == Check.skipTests&& !ut); + + if (analysisConfig.label_var_same_name_check != Check.disabled) + checks ~= new LabelVarNameCheck(fileName, moduleScope, + analysisConfig.label_var_same_name_check == Check.skipTests && !ut); + + if (analysisConfig.length_subtraction_check != Check.disabled) + checks ~= new LengthSubtractionCheck(fileName, moduleScope, + analysisConfig.length_subtraction_check == Check.skipTests && !ut); + + if (analysisConfig.local_import_check != Check.disabled) + checks ~= new LocalImportCheck(fileName, moduleScope, + analysisConfig.local_import_check == Check.skipTests && !ut); + + if (analysisConfig.logical_precedence_check != Check.disabled) + checks ~= new LogicPrecedenceCheck(fileName, moduleScope, + analysisConfig.logical_precedence_check == Check.skipTests && !ut); + + if (analysisConfig.mismatched_args_check != Check.disabled) + checks ~= new MismatchedArgumentCheck(fileName, moduleScope, + analysisConfig.mismatched_args_check == Check.skipTests && !ut); + + if (analysisConfig.number_style_check != Check.disabled) + checks ~= new NumberStyleCheck(fileName, moduleScope, + analysisConfig.number_style_check == Check.skipTests && !ut); + + if (analysisConfig.object_const_check != Check.disabled) + checks ~= new ObjectConstCheck(fileName, moduleScope, + analysisConfig.object_const_check == Check.skipTests && !ut); + + if (analysisConfig.opequals_tohash_check != Check.disabled) + checks ~= new OpEqualsWithoutToHashCheck(fileName, moduleScope, + analysisConfig.opequals_tohash_check == Check.skipTests && !ut); + + if (analysisConfig.redundant_parens_check != Check.disabled) + checks ~= new RedundantParenCheck(fileName, moduleScope, + analysisConfig.redundant_parens_check == Check.skipTests && !ut); + + if (analysisConfig.style_check != Check.disabled) + checks ~= new StyleChecker(fileName, moduleScope, + analysisConfig.style_check == Check.skipTests && !ut); + + if (analysisConfig.undocumented_declaration_check != Check.disabled) + checks ~= new UndocumentedDeclarationCheck(fileName, moduleScope, + analysisConfig.undocumented_declaration_check == Check.skipTests && !ut); + + if (analysisConfig.unused_label_check != Check.disabled) + checks ~= new UnusedLabelCheck(fileName, moduleScope, + analysisConfig.unused_label_check == Check.skipTests && !ut); + + if (analysisConfig.unused_variable_check != Check.disabled) + checks ~= new UnusedVariableCheck(fileName, moduleScope, + analysisConfig.unused_variable_check == Check.skipTests && !ut); + + if (analysisConfig.long_line_check != Check.disabled) + checks ~= new LineLengthCheck(fileName, tokens, + analysisConfig.long_line_check == Check.skipTests && !ut); + + if (analysisConfig.auto_ref_assignment_check != Check.disabled) + checks ~= new AutoRefAssignmentCheck(fileName, + analysisConfig.auto_ref_assignment_check == Check.skipTests && !ut); + + if (analysisConfig.incorrect_infinite_range_check != Check.disabled) + checks ~= new IncorrectInfiniteRangeCheck(fileName, + analysisConfig.incorrect_infinite_range_check == Check.skipTests && !ut); + + if (analysisConfig.useless_assert_check != Check.disabled) + checks ~= new UselessAssertCheck(fileName, + analysisConfig.useless_assert_check == Check.skipTests && !ut); + + if (analysisConfig.alias_syntax_check != Check.disabled) + checks ~= new AliasSyntaxCheck(fileName, + analysisConfig.alias_syntax_check == Check.skipTests && !ut); + + if (analysisConfig.static_if_else_check != Check.disabled) + checks ~= new StaticIfElse(fileName, + analysisConfig.static_if_else_check == Check.skipTests && !ut); + + if (analysisConfig.lambda_return_check != Check.disabled) + checks ~= new LambdaReturnCheck(fileName, + analysisConfig.lambda_return_check == Check.skipTests && !ut); + version (none) - if (analysisConfig.redundant_if_check) - checks ~= new IfStatementCheck(fileName, moduleScope); + if (analysisConfig.redundant_if_check != Check.disabled) + checks ~= new IfStatementCheck(fileName, moduleScope, + analysisConfig.redundant_if_check == Check.skipTests && !ut); foreach (check; checks) { @@ -297,3 +366,4 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a set.insert(message); return set; } + diff --git a/src/analysis/static_if_else.d b/src/analysis/static_if_else.d index 00d5880..0cd9242 100644 --- a/src/analysis/static_if_else.d +++ b/src/analysis/static_if_else.d @@ -25,9 +25,9 @@ class StaticIfElse : BaseAnalyzer { alias visit = BaseAnalyzer.visit; - this(string filename) + this(string fileName, bool skipTests = false) { - super(filename, null); + super(fileName, null, skipTests); } override void visit(const ConditionalStatement cc) @@ -66,11 +66,11 @@ class StaticIfElse : BaseAnalyzer unittest { import analysis.helpers : assertAnalyzerWarnings; - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; import std.stdio : stderr; StaticAnalysisConfig sac; - sac.static_if_else_check = true; + sac.static_if_else_check = Check.enabled; assertAnalyzerWarnings(q{ void foo() { static if (false) diff --git a/src/analysis/style.d b/src/analysis/style.d index 0b3d5e1..255d7f3 100644 --- a/src/analysis/style.d +++ b/src/analysis/style.d @@ -25,9 +25,9 @@ class StyleChecker : BaseAnalyzer enum string moduleNameRegex = `^[\p{Ll}_\d]+$`; enum string KEY = "dscanner.style.phobos_naming_convention"; - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } override void visit(const ModuleDeclaration dec) @@ -93,10 +93,10 @@ class StyleChecker : BaseAnalyzer unittest { - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; StaticAnalysisConfig sac; - sac.style_check = true; + sac.style_check = Check.enabled; assertAnalyzerWarnings(q{ module AMODULE; // [warn]: Module/package name 'AMODULE' does not match style guidelines. diff --git a/src/analysis/undocumented.d b/src/analysis/undocumented.d index 0ae00ff..237e1a6 100644 --- a/src/analysis/undocumented.d +++ b/src/analysis/undocumented.d @@ -21,9 +21,9 @@ class UndocumentedDeclarationCheck : BaseAnalyzer { alias visit = BaseAnalyzer.visit; - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } override void visit(const Module mod) diff --git a/src/analysis/unmodified.d b/src/analysis/unmodified.d index 86ef31e..aa36144 100644 --- a/src/analysis/unmodified.d +++ b/src/analysis/unmodified.d @@ -18,9 +18,9 @@ class UnmodifiedFinder : BaseAnalyzer alias visit = BaseAnalyzer.visit; /// - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } override void visit(const Module mod) diff --git a/src/analysis/unused.d b/src/analysis/unused.d index 99d31cf..d170dc4 100644 --- a/src/analysis/unused.d +++ b/src/analysis/unused.d @@ -22,9 +22,9 @@ class UnusedVariableCheck : BaseAnalyzer * Params: * fileName = the name of the file being analyzed */ - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); re = regex("[\\p{Alphabetic}_][\\w_]*"); } @@ -410,11 +410,11 @@ private: unittest { import std.stdio : stderr; - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; import analysis.helpers : assertAnalyzerWarnings; StaticAnalysisConfig sac; - sac.unused_variable_check = true; + sac.unused_variable_check = Check.enabled; assertAnalyzerWarnings(q{ // Issue 274 diff --git a/src/analysis/unused_label.d b/src/analysis/unused_label.d index 2ed7bcc..cc19779 100644 --- a/src/analysis/unused_label.d +++ b/src/analysis/unused_label.d @@ -14,9 +14,9 @@ class UnusedLabelCheck : BaseAnalyzer { alias visit = BaseAnalyzer.visit; - this(string fileName, const(Scope)* sc) + this(string fileName, const(Scope)* sc, bool skipTests = false) { - super(fileName, sc); + super(fileName, sc, skipTests); } override void visit(const Module mod) @@ -137,11 +137,11 @@ private: unittest { - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; import std.stdio : stderr; StaticAnalysisConfig sac; - sac.unused_label_check = true; + sac.unused_label_check = Check.enabled; assertAnalyzerWarnings(q{ int testUnusedLabel() { diff --git a/src/analysis/useless_assert.d b/src/analysis/useless_assert.d index aeb53e0..8594020 100644 --- a/src/analysis/useless_assert.d +++ b/src/analysis/useless_assert.d @@ -19,11 +19,12 @@ class UselessAssertCheck : BaseAnalyzer { alias visit = BaseAnalyzer.visit; - /// - this(string fileName) - { - super(fileName, null); - } + /// + this(string fileName, bool skipTests = false) + { + // assertions likely to be in unittest so never skip + super(fileName, null, false); + } override void visit(const AssertExpression ae) { @@ -96,11 +97,11 @@ private: unittest { import std.stdio : stderr; - import analysis.config : StaticAnalysisConfig; + import analysis.config : StaticAnalysisConfig, Check; import std.format : format; StaticAnalysisConfig sac; - sac.useless_assert_check = true; + sac.useless_assert_check = Check.enabled; assertAnalyzerWarnings(q{ unittest { diff --git a/src/main.d b/src/main.d index c418ec6..f2d080d 100644 --- a/src/main.d +++ b/src/main.d @@ -59,6 +59,7 @@ else bool styleCheck; bool defaultConfig; bool report; + bool skipTests; string symbolName; string configLocation; string[] importPaths; @@ -89,7 +90,8 @@ else "I", &importPaths, "version", &printVersion, "muffinButton", &muffin, - "explore", &explore); + "explore", &explore, + "skipTests", &skipTests); //dfmt on } catch (ConvException e) @@ -218,6 +220,8 @@ else string s = configLocation is null ? getConfigurationLocation() : configLocation; if (s.exists()) readINIFile(config, s); + if (skipTests) + config.fillConfig!(Check.skipTests); if (report) generateReport(expandArgs(args), config, cache, moduleCache); else @@ -368,7 +372,7 @@ Options: If no files are specified, input is read from stdin. %1$s will exit with a status code of zero if no errors are found, 1 otherwise. - --styleCheck ..., ... + --styleCheck|S ..., ... Lexes and parses sourceFiles, printing the line and column number of any static analysis check failures stdout. %1$s will exit with a status code of zero if no warnings or errors are found, 1 otherwise. @@ -406,8 +410,12 @@ Options: $HOME/.config/dscanner/dscanner.ini --defaultConfig - Generates a default configuration file for the static analysis checks`, - programName); + Generates a default configuration file for the static analysis checks, + + --skipTests + Does not analyze in the unittests. Only works if --styleCheck.`, + + programName); } private void doNothing(string, size_t, size_t, string, bool)