From 37cedcb3e807ac03aeab279408a0081259e6ea3f Mon Sep 17 00:00:00 2001 From: Sebastian Wilzbach Date: Thu, 15 Jun 2017 21:02:04 +0200 Subject: [PATCH 1/3] Fix #457 - Allow to apply checks only for specific modules --- README.md | 28 +++++++ src/analysis/config.d | 21 ++++++ src/analysis/run.d | 165 +++++++++++++++++++++++++++++++----------- 3 files changed, 170 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index 90b05462..34a9b75c 100644 --- a/README.md +++ b/README.md @@ -304,3 +304,31 @@ For more readable output, pipe the command through [xmllint](http://xmlsoft.org/ using its formatting switch. $ dscanner --ast helloworld.d | xmllint --format - + +Selecting modules for a specific check +-------------------------------------- + +It is possible to create a new section `analysis.config.ModuleFilters` in the `.dscanner.ini`. +In this optional section a comma-separated list of inclusion and exclusion selectors can +be specified for every check on which selective filtering should be applied. +These given selectors match on the module name and partial matches (`std.` or `.foo.`) are possible. +Morover, every selectors must begin with either `+` (inclusion) or `-` (exclusion). +Exclusion selectors take precedence over all inclusion operators. +Of course, for every check a different selector set can given: + +```ini +[analysis.config.ModuleFilters] +final_attribute_check = "+std.foo,+std.bar" +useless_initializer = "-std." +;pseudo variable (workaround against an inifiled bug) +foo="" +``` + +A few examples: + +- `+std.`: Includes all modules matching `std.` +- `+std.bitmanip,+std.json`: Applies the check only for these two modules +- `-std.bitmanip,-std.json`: Applies the check for all modules, but these two +- `+.bar`: Includes all modules matching `.bar` (e.g. `foo.bar`, `a.b.c.barros`) +- `-etc.`: Excludes all modules from `.etc` +- `+std,-std.internal`: Includes entire `std`, except for the internal modules diff --git a/src/analysis/config.d b/src/analysis/config.d index 77c57fec..1966e985 100644 --- a/src/analysis/config.d +++ b/src/analysis/config.d @@ -188,4 +188,25 @@ struct StaticAnalysisConfig @INI("Check public declarations without a documented unittest") string has_public_example = Check.disabled; + + @INI("Module-specific filters") + ModuleFilters filters; +} + +private template ModuleFiltersMixin(A) +{ + const string ModuleFiltersMixin = () { + string s; + foreach (mem; __traits(allMembers, StaticAnalysisConfig)) + static if (is(typeof(__traits(getMember, StaticAnalysisConfig, mem)) == string)) + s ~= `@INI("Exclude/Import modules") string[] ` ~ mem ~ ";\n"; + + return s; + }(); +} + +@INI("ModuleFilters. +std.,-std.internal") +struct ModuleFilters +{ + mixin(ModuleFiltersMixin!int); } diff --git a/src/analysis/run.d b/src/analysis/run.d index a5e63aa2..9f8801e4 100644 --- a/src/analysis/run.d +++ b/src/analysis/run.d @@ -206,6 +206,77 @@ const(Module) parseModule(string fileName, ubyte[] code, RollbackAllocator* p, ? &messageFunctionJSON : &messageFunction, errorCount, warningCount); } +/** +Checks whether a module is part of a user-specified include/exclude list. +The user can specify a comma-separated list of filters, everyone needs to start with +either a '+' (inclusion) or '-' (exclusion). +If no includes are specified, all modules are included. +*/ +bool shouldRun(string a)(string moduleName, const ref StaticAnalysisConfig config) +{ + if (mixin("config." ~ a) == Check.disabled) + return false; + + // By default, run the check + if (!moduleName.length) + return true; + + auto filters = mixin("config.filters." ~ a); + + // Check if there are filters are defined + // filters starting with a comma are invalid + if (filters.length == 0 || filters[0].length == 0) + return true; + + auto includers = filters.filter!(f => f[0] == '+').map!(f => f[1..$]); + auto excluders = filters.filter!(f => f[0] == '-').map!(f => f[1..$]); + + // exclusion has preference over inclusion + if (!excluders.empty && excluders.any!(s => moduleName.canFind(s))) + return false; + + if (!includers.empty) + return includers.any!(s => moduleName.canFind(s)); + + // by default: include all modules + return true; +} + +/// +unittest +{ + bool test(string moduleName, string filters) + { + StaticAnalysisConfig config; + // it doesn't matter which check we test here + config.asm_style_check = Check.enabled; + // this is done automatically by inifiled + config.filters.asm_style_check = filters.split(","); + return shouldRun!"asm_style_check"(moduleName, config); + } + + // test inclusion + assert(test("std.foo", "+std.")); + // partial matches are ok + assert(test("std.foo", "+bar,+foo")); + // full as well + assert(test("std.foo", "+bar,+std.foo,+foo")); + // mismatch + assert(!test("std.foo", "+bar,+banana")); + + // test exclusion + assert(!test("std.foo", "-std.")); + assert(!test("std.foo", "-bar,-std.foo")); + assert(!test("std.foo", "-bar,-foo")); + // mismatch + assert(test("std.foo", "-bar,-banana")); + + // test combination (exclusion has precedence) + assert(!test("std.foo", "+foo,-foo")); + assert(test("std.foo", "+foo,-bar")); + assert(test("std.bar.foo", "-barr,+bar")); +} + MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig analysisConfig, ref ModuleCache moduleCache, const(Token)[] tokens, bool staticAnalyze = true) { @@ -220,6 +291,11 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a else enum ut = false; + string moduleName; + if (m !is null && m.moduleDeclaration !is null && + m.moduleDeclaration.moduleName !is null && + m.moduleDeclaration.moduleName.identifiers !is null) + moduleName = m.moduleDeclaration.moduleName.identifiers.map!(e => e.text).join("."); auto first = scoped!FirstPass(m, internString(fileName), symbolAllocator, symbolAllocator, true, &moduleCache, null); @@ -232,171 +308,172 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a scope(exit) typeid(Scope).destroy(first.moduleScope); BaseAnalyzer[] checks; - if (analysisConfig.asm_style_check != Check.disabled) + with(analysisConfig) + if (moduleName.shouldRun!"asm_style_check"(analysisConfig)) checks ~= new AsmStyleCheck(fileName, moduleScope, - analysisConfig.asm_style_check == Check.skipTests && !ut); + asm_style_check == Check.skipTests && !ut); - if (analysisConfig.backwards_range_check != Check.disabled) + if (moduleName.shouldRun!"backwards_range_check"(analysisConfig)) checks ~= new BackwardsRangeCheck(fileName, moduleScope, analysisConfig.backwards_range_check == Check.skipTests && !ut); - if (analysisConfig.builtin_property_names_check != Check.disabled) + if (moduleName.shouldRun!"builtin_property_names_check"(analysisConfig)) checks ~= new BuiltinPropertyNameCheck(fileName, moduleScope, analysisConfig.builtin_property_names_check == Check.skipTests && !ut); - if (analysisConfig.comma_expression_check != Check.disabled) + if (moduleName.shouldRun!"comma_expression_check"(analysisConfig)) checks ~= new CommaExpressionCheck(fileName, moduleScope, analysisConfig.comma_expression_check == Check.skipTests && !ut); - if (analysisConfig.constructor_check != Check.disabled) + if (moduleName.shouldRun!"constructor_check"(analysisConfig)) checks ~= new ConstructorCheck(fileName, moduleScope, analysisConfig.constructor_check == Check.skipTests && !ut); - if (analysisConfig.could_be_immutable_check != Check.disabled) + if (moduleName.shouldRun!"could_be_immutable_check"(analysisConfig)) checks ~= new UnmodifiedFinder(fileName, moduleScope, analysisConfig.could_be_immutable_check == Check.skipTests && !ut); - if (analysisConfig.delete_check != Check.disabled) + if (moduleName.shouldRun!"delete_check"(analysisConfig)) checks ~= new DeleteCheck(fileName, moduleScope, analysisConfig.delete_check == Check.skipTests && !ut); - if (analysisConfig.duplicate_attribute != Check.disabled) + if (moduleName.shouldRun!"duplicate_attribute"(analysisConfig)) checks ~= new DuplicateAttributeCheck(fileName, moduleScope, analysisConfig.duplicate_attribute == Check.skipTests && !ut); - if (analysisConfig.enum_array_literal_check != Check.disabled) + if (moduleName.shouldRun!"enum_array_literal_check"(analysisConfig)) checks ~= new EnumArrayLiteralCheck(fileName, moduleScope, analysisConfig.enum_array_literal_check == Check.skipTests && !ut); - if (analysisConfig.exception_check != Check.disabled) + if (moduleName.shouldRun!"exception_check"(analysisConfig)) checks ~= new PokemonExceptionCheck(fileName, moduleScope, analysisConfig.exception_check == Check.skipTests && !ut); - if (analysisConfig.float_operator_check != Check.disabled) + if (moduleName.shouldRun!"float_operator_check"(analysisConfig)) checks ~= new FloatOperatorCheck(fileName, moduleScope, analysisConfig.float_operator_check == Check.skipTests && !ut); - if (analysisConfig.function_attribute_check != Check.disabled) + if (moduleName.shouldRun!"function_attribute_check"(analysisConfig)) checks ~= new FunctionAttributeCheck(fileName, moduleScope, analysisConfig.function_attribute_check == Check.skipTests && !ut); - if (analysisConfig.if_else_same_check != Check.disabled) + if (moduleName.shouldRun!"if_else_same_check"(analysisConfig)) checks ~= new IfElseSameCheck(fileName, moduleScope, analysisConfig.if_else_same_check == Check.skipTests&& !ut); - if (analysisConfig.label_var_same_name_check != Check.disabled) + if (moduleName.shouldRun!"label_var_same_name_check"(analysisConfig)) checks ~= new LabelVarNameCheck(fileName, moduleScope, analysisConfig.label_var_same_name_check == Check.skipTests && !ut); - if (analysisConfig.length_subtraction_check != Check.disabled) + if (moduleName.shouldRun!"length_subtraction_check"(analysisConfig)) checks ~= new LengthSubtractionCheck(fileName, moduleScope, analysisConfig.length_subtraction_check == Check.skipTests && !ut); - if (analysisConfig.local_import_check != Check.disabled) + if (moduleName.shouldRun!"local_import_check"(analysisConfig)) checks ~= new LocalImportCheck(fileName, moduleScope, analysisConfig.local_import_check == Check.skipTests && !ut); - if (analysisConfig.logical_precedence_check != Check.disabled) + if (moduleName.shouldRun!"logical_precedence_check"(analysisConfig)) checks ~= new LogicPrecedenceCheck(fileName, moduleScope, analysisConfig.logical_precedence_check == Check.skipTests && !ut); - if (analysisConfig.mismatched_args_check != Check.disabled) + if (moduleName.shouldRun!"mismatched_args_check"(analysisConfig)) checks ~= new MismatchedArgumentCheck(fileName, moduleScope, analysisConfig.mismatched_args_check == Check.skipTests && !ut); - if (analysisConfig.number_style_check != Check.disabled) + if (moduleName.shouldRun!"number_style_check"(analysisConfig)) checks ~= new NumberStyleCheck(fileName, moduleScope, analysisConfig.number_style_check == Check.skipTests && !ut); - if (analysisConfig.object_const_check != Check.disabled) + if (moduleName.shouldRun!"object_const_check"(analysisConfig)) checks ~= new ObjectConstCheck(fileName, moduleScope, analysisConfig.object_const_check == Check.skipTests && !ut); - if (analysisConfig.opequals_tohash_check != Check.disabled) + if (moduleName.shouldRun!"opequals_tohash_check"(analysisConfig)) checks ~= new OpEqualsWithoutToHashCheck(fileName, moduleScope, analysisConfig.opequals_tohash_check == Check.skipTests && !ut); - if (analysisConfig.redundant_parens_check != Check.disabled) + if (moduleName.shouldRun!"redundant_parens_check"(analysisConfig)) checks ~= new RedundantParenCheck(fileName, moduleScope, analysisConfig.redundant_parens_check == Check.skipTests && !ut); - if (analysisConfig.style_check != Check.disabled) + if (moduleName.shouldRun!"style_check"(analysisConfig)) checks ~= new StyleChecker(fileName, moduleScope, analysisConfig.style_check == Check.skipTests && !ut); - if (analysisConfig.undocumented_declaration_check != Check.disabled) + if (moduleName.shouldRun!"undocumented_declaration_check"(analysisConfig)) checks ~= new UndocumentedDeclarationCheck(fileName, moduleScope, analysisConfig.undocumented_declaration_check == Check.skipTests && !ut); - if (analysisConfig.unused_label_check != Check.disabled) + if (moduleName.shouldRun!"unused_label_check"(analysisConfig)) checks ~= new UnusedLabelCheck(fileName, moduleScope, analysisConfig.unused_label_check == Check.skipTests && !ut); - if (analysisConfig.unused_variable_check != Check.disabled) + if (moduleName.shouldRun!"unused_variable_check"(analysisConfig)) checks ~= new UnusedVariableCheck(fileName, moduleScope, analysisConfig.unused_variable_check == Check.skipTests && !ut); - if (analysisConfig.long_line_check != Check.disabled) + if (moduleName.shouldRun!"long_line_check"(analysisConfig)) checks ~= new LineLengthCheck(fileName, tokens, analysisConfig.long_line_check == Check.skipTests && !ut); - if (analysisConfig.auto_ref_assignment_check != Check.disabled) + if (moduleName.shouldRun!"auto_ref_assignment_check"(analysisConfig)) checks ~= new AutoRefAssignmentCheck(fileName, analysisConfig.auto_ref_assignment_check == Check.skipTests && !ut); - if (analysisConfig.incorrect_infinite_range_check != Check.disabled) + if (moduleName.shouldRun!"incorrect_infinite_range_check"(analysisConfig)) checks ~= new IncorrectInfiniteRangeCheck(fileName, analysisConfig.incorrect_infinite_range_check == Check.skipTests && !ut); - if (analysisConfig.useless_assert_check != Check.disabled) + if (moduleName.shouldRun!"useless_assert_check"(analysisConfig)) checks ~= new UselessAssertCheck(fileName, analysisConfig.useless_assert_check == Check.skipTests && !ut); - if (analysisConfig.alias_syntax_check != Check.disabled) + if (moduleName.shouldRun!"alias_syntax_check"(analysisConfig)) checks ~= new AliasSyntaxCheck(fileName, analysisConfig.alias_syntax_check == Check.skipTests && !ut); - if (analysisConfig.static_if_else_check != Check.disabled) + if (moduleName.shouldRun!"static_if_else_check"(analysisConfig)) checks ~= new StaticIfElse(fileName, analysisConfig.static_if_else_check == Check.skipTests && !ut); - if (analysisConfig.lambda_return_check != Check.disabled) + if (moduleName.shouldRun!"lambda_return_check"(analysisConfig)) checks ~= new LambdaReturnCheck(fileName, analysisConfig.lambda_return_check == Check.skipTests && !ut); - if (analysisConfig.auto_function_check != Check.disabled) + if (moduleName.shouldRun!"auto_function_check"(analysisConfig)) checks ~= new AutoFunctionChecker(fileName, analysisConfig.auto_function_check == Check.skipTests && !ut); - if (analysisConfig.imports_sortedness != Check.disabled) + if (moduleName.shouldRun!"imports_sortedness"(analysisConfig)) checks ~= new ImportSortednessCheck(fileName, analysisConfig.imports_sortedness == Check.skipTests && !ut); - if (analysisConfig.explicitly_annotated_unittests != Check.disabled) + if (moduleName.shouldRun!"explicitly_annotated_unittests"(analysisConfig)) checks ~= new ExplicitlyAnnotatedUnittestCheck(fileName, analysisConfig.explicitly_annotated_unittests == Check.skipTests && !ut); - if (analysisConfig.properly_documented_public_functions != Check.disabled) + if (moduleName.shouldRun!"properly_documented_public_functions"(analysisConfig)) checks ~= new ProperlyDocumentedPublicFunctions(fileName, analysisConfig.properly_documented_public_functions == Check.skipTests && !ut); - if (analysisConfig.final_attribute_check != Check.disabled) + if (moduleName.shouldRun!"final_attribute_check"(analysisConfig)) checks ~= new FinalAttributeChecker(fileName, analysisConfig.final_attribute_check == Check.skipTests && !ut); - if (analysisConfig.vcall_in_ctor != Check.disabled) + if (moduleName.shouldRun!"vcall_in_ctor"(analysisConfig)) checks ~= new VcallCtorChecker(fileName, analysisConfig.vcall_in_ctor == Check.skipTests && !ut); - if (analysisConfig.useless_initializer != Check.disabled) + if (moduleName.shouldRun!"useless_initializer"(analysisConfig)) checks ~= new UselessInitializerChecker(fileName, analysisConfig.useless_initializer == Check.skipTests && !ut); - if (analysisConfig.allman_braces_check != Check.disabled) + if (moduleName.shouldRun!"allman_braces_check"(analysisConfig)) checks ~= new AllManCheck(fileName, tokens, analysisConfig.allman_braces_check == Check.skipTests && !ut); - if (analysisConfig.redundant_attributes_check != Check.disabled) + if (moduleName.shouldRun!"redundant_attributes_check"(analysisConfig)) checks ~= new RedundantAttributesCheck(fileName, moduleScope, analysisConfig.redundant_attributes_check == Check.skipTests && !ut); @@ -405,7 +482,7 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a analysisConfig.has_public_example == Check.skipTests && !ut); version (none) - if (analysisConfig.redundant_if_check != Check.disabled) + if (moduleName.shouldRun!"redundant_if_check"(analysisConfig)) checks ~= new IfStatementCheck(fileName, moduleScope, analysisConfig.redundant_if_check == Check.skipTests && !ut); From 9defcd086893f0ed98d754e4b6a02c4a77cfb65e Mon Sep 17 00:00:00 2001 From: Sebastian Wilzbach Date: Tue, 20 Jun 2017 10:13:28 +0200 Subject: [PATCH 2/3] update inifiled to 1.0.2 --- README.md | 4 +--- dub.json | 2 +- inifiled | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 34a9b75c..ac69b987 100644 --- a/README.md +++ b/README.md @@ -312,7 +312,7 @@ It is possible to create a new section `analysis.config.ModuleFilters` in the `. In this optional section a comma-separated list of inclusion and exclusion selectors can be specified for every check on which selective filtering should be applied. These given selectors match on the module name and partial matches (`std.` or `.foo.`) are possible. -Morover, every selectors must begin with either `+` (inclusion) or `-` (exclusion). +Moreover, every selectors must begin with either `+` (inclusion) or `-` (exclusion). Exclusion selectors take precedence over all inclusion operators. Of course, for every check a different selector set can given: @@ -320,8 +320,6 @@ Of course, for every check a different selector set can given: [analysis.config.ModuleFilters] final_attribute_check = "+std.foo,+std.bar" useless_initializer = "-std." -;pseudo variable (workaround against an inifiled bug) -foo="" ``` A few examples: diff --git a/dub.json b/dub.json index 3d790fb4..b91ddf7e 100644 --- a/dub.json +++ b/dub.json @@ -12,7 +12,7 @@ "dependencies": { "libdparse": "~>0.7.1-beta.4", "dsymbol": "~>0.2.0", - "inifiled": ">=0.0.6", + "inifiled": ">=1.0.2", "emsi_containers": "~>0.5.3", "libddoc": "~>0.2.0" }, diff --git a/inifiled b/inifiled index e4f63f12..35f8d2d9 160000 --- a/inifiled +++ b/inifiled @@ -1 +1 @@ -Subproject commit e4f63f126ddddb3e496574fec0f76b24e61b1d51 +Subproject commit 35f8d2d914560f8c73cf5e6b80b8e0f47f498d64 From b711890469549cc30229bfbbdcec808009d28b11 Mon Sep 17 00:00:00 2001 From: Sebastian Wilzbach Date: Thu, 22 Jun 2017 22:06:04 +0200 Subject: [PATCH 3/3] Compile dependencies separately, s.t. their unittests don't get executed --- makefile | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/makefile b/makefile index d98e9f6f..2780216a 100644 --- a/makefile +++ b/makefile @@ -5,14 +5,15 @@ DMD := $(DC) GDC := gdc LDC := ldc2 OBJ_DIR := obj -SRC := \ +LIB_SRC := \ $(shell find containers/src -name "*.d")\ $(shell find dsymbol/src -name "*.d")\ $(shell find inifiled/source/ -name "*.d")\ $(shell find libdparse/src/std/experimental/ -name "*.d")\ $(shell find libdparse/src/dparse/ -name "*.d")\ - $(shell find libddoc/src -name "*.d")\ - $(shell find src/ -name "*.d") + $(shell find libddoc/src -name "*.d") +PROJECT_SRC := $(shell find src/ -name "*.d") +SRC := $(LIB_SRC) $(PROJECT_SRC) INCLUDE_PATHS = \ -Iinifiled/source -Isrc\ -Ilibdparse/src\ @@ -21,7 +22,7 @@ INCLUDE_PATHS = \ VERSIONS = DEBUG_VERSIONS = -version=dparse_verbose DMD_FLAGS = -w -inline -release -O -J. -od${OBJ_DIR} -version=StdLoggerDisableWarning -DMD_TEST_FLAGS = -w -g -unittest -J. +DMD_TEST_FLAGS = -w -g -J. -version=StdLoggerDisableWarning all: dmdbuild ldc: ldcbuild @@ -46,8 +47,12 @@ ldcbuild: githash mkdir -p bin ${LDC} -O5 -release -oq -of=bin/dscanner ${VERSIONS} ${INCLUDE_PATHS} ${SRC} -J. -test: githash - ${DC} -w -g -J. -unittest ${INCLUDE_PATHS} ${SRC} -ofbin/dscanner-unittest -version=StdLoggerDisableWarning +# compile the dependencies separately, s.t. their unittests don't get executed +bin/dscanner-unittest-lib.a: ${LIB_SRC} + ${DC} ${DMD_TEST_FLAGS} -c ${INCLUDE_PATHS} ${LIB_SRC} -of$@ + +test: bin/dscanner-unittest-lib.a githash + ${DC} ${DMD_TEST_FLAGS} -unittest ${INCLUDE_PATHS} bin/dscanner-unittest-lib.a ${PROJECT_SRC} -ofbin/dscanner-unittest ./bin/dscanner-unittest rm -f bin/dscanner-unittest