From f77a768843ef492e6141bd1055a03b70252492dd Mon Sep 17 00:00:00 2001 From: Sebastian Wilzbach Date: Mon, 19 Jun 2017 12:55:12 +0200 Subject: [PATCH 1/5] Use Dscanner with selective checks --- posix.mak | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/posix.mak b/posix.mak index 653bb8e870e..0eabd8dd372 100644 --- a/posix.mak +++ b/posix.mak @@ -522,8 +522,8 @@ checkwhitespace: $(LIB) $(TOOLS_DIR)/checkwhitespace.d ############################# ../dscanner: - git clone https://github.com/Hackerpilot/Dscanner ../dscanner - git -C ../dscanner checkout tags/v0.4.0 + git clone https://github.com/dlang-community/Dscanner ../dscanner + git -C ../dscanner checkout phobos git -C ../dscanner submodule update --init --recursive ../dscanner/dsc: ../dscanner From 43b33c6b98b2a490d7e2f085b383735970b1c7f9 Mon Sep 17 00:00:00 2001 From: Sebastian Wilzbach Date: Mon, 19 Jun 2017 13:02:53 +0200 Subject: [PATCH 2/5] Enable selective module checks --- .dscanner.ini | 85 +++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 62 insertions(+), 23 deletions(-) diff --git a/.dscanner.ini b/.dscanner.ini index a2b0999c75a..0fe24530b48 100644 --- a/.dscanner.ini +++ b/.dscanner.ini @@ -2,20 +2,20 @@ [analysis.config.StaticAnalysisConfig] ; Check variable, class, struct, interface, union, and function names against ; the Phobos style guide -style_check="disabled" +style_check="enabled" ; Check for array literals that cause unnecessary allocation enum_array_literal_check="enabled" ; Check for poor exception handling practices -exception_check="disabled" ; FIXME +exception_check="enabled" ; Check for use of the deprecated 'delete' keyword delete_check="enabled" ; Check for use of the deprecated floating point operators float_operator_check="enabled" ; Check number literals for readability -number_style_check="skip-unittest"; +number_style_check="enabled" ; Checks that opEquals, opCmp, toHash, and toString are either const, immutable ; , or inout. -object_const_check="disabled" +object_const_check="enabled" ; Checks for .. expressions where the left side is larger than the right. backwards_range_check="enabled" ; Checks for if statements whose 'then' block is the same as the 'else' block @@ -23,43 +23,44 @@ if_else_same_check="enabled" ; Checks for some problems with constructors constructor_check="enabled" ; Checks for unused variables and function parameters -unused_variable_check="disabled" +unused_variable_check="enabled" ; Checks for unused labels -unused_label_check="disabled" ; FIXME +unused_label_check="enabled" ; Checks for duplicate attributes duplicate_attribute="enabled" ; Checks that opEquals and toHash are both defined or neither are defined -opequals_tohash_check="disabled" +opequals_tohash_check="enabled" ; Checks for subtraction from .length properties -length_subtraction_check="disabled" -; Checks for methods or properties whose names conflict with built-in properties -builtin_property_names_check="enabled"; FIXME +length_subtraction_check="enabled" +; Checks for methods or properties whose names conflict with built-in propertie +; s +builtin_property_names_check="enabled" ; Checks for confusing code in inline asm statements -asm_style_check="disabled"; FIXME +asm_style_check="enabled" ; Checks for confusing logical operator precedence -logical_precedence_check="disabled" +logical_precedence_check="enabled" ; Checks for undocumented public declarations -undocumented_declaration_check="disabled"; FIXME +undocumented_declaration_check="enabled" ; Checks for poor placement of function attributes -function_attribute_check="disabled" +function_attribute_check="enabled" ; Checks for use of the comma operator comma_expression_check="enabled" ; Checks for local imports that are too broad local_import_check="skip-unittest" ; Checks for variables that could be declared immutable -could_be_immutable_check="disabled" +could_be_immutable_check="enabled" ; Checks for redundant expressions in if statements redundant_if_check="enabled" ; Checks for redundant parenthesis redundant_parens_check="enabled" ; Checks for mismatched argument and parameter names -mismatched_args_check="disabled" +mismatched_args_check="enabled" ; Checks for labels with the same name as variables -label_var_same_name_check="disabled" +label_var_same_name_check="enabled" ; Checks for lines longer than 120 characters long_line_check="enabled" ; Checks for assignment to auto-ref function parameters -auto_ref_assignment_check="disabled" ; FIXME +auto_ref_assignment_check="enabled" ; Checks for incorrect infinite range definitions incorrect_infinite_range_check="enabled" ; Checks for asserts that are always true @@ -71,10 +72,48 @@ static_if_else_check="enabled" ; Check for unclear lambda syntax lambda_return_check="enabled" ; Check for auto function without return statement -auto_function_check = "enabled" -; Check for explicitly annotated unittests -explicitly_annotated_unittests = "enabled" +auto_function_check="enabled" ; Check for sortedness of imports -imports_sortedness = "disabled" +imports_sortedness="enabled" +; Check for explicitly annotated unittests +explicitly_annotated_unittests="enabled" +; Check for properly documented public functions (Returns, Params) +properly_documented_public_functions="enabled" ; Check for useless usage of the final attribute -final_attribute_check = "enabled" +final_attribute_check="enabled" +; Check for virtual calls in the class constructors +vcall_in_ctor="enabled" +; Check for useless user defined initializers +useless_initializer="enabled" +; Check allman brace style +allman_braces_check="disabled" +; Check for redundant attributes +redundant_attributes_check="enabled" + +; Configure which modules are checked with a specific checker +; Please help to extend these checks onto more Phobos modules +[analysis.config.ModuleFilters] +asm_style_check="+std.algorithm" +auto_ref_assignment_check="+std.algorithm.searching" +could_be_immutable_check="+std.algorithm.internal" +exception_check="+std.algorithm" +function_attribute_check="+std.algorithm.searching" +imports_sortedness = "+std.algorithm.disabled" ; currently disabled, see https://github.com/dlang/phobos/pull/5478 +label_var_same_name_check="+std.algorithm.searching" +length_subtraction_check="+std.algorithm.comparison" +logical_precedence_check="+std.algorithm.internal" +long_line_check="-std.regex.internal.thompson"; Dscanner bug: https://github.com/dlang-community/D-Scanner/pull/465 +mismatched_args_check="+std.algorithm" +mismatched_args_check="+std.algorithm" +number_style_check="+std.algorithm.searching" +object_const_check="+std.algorithm.internal" +opequals_tohash_check="+std.algorithm.comparison" +redundant_attributes_check = "+std.algorithm" +style_check="+std.algorithm.searching" +undocumented_declaration_check="+std.algorithm.searching" +unused_label_check="+std.algorithm" +unused_variable_check="+std.algorithm.internal" +useless_initializer = "+std.algorithm.comparison" +vcall_in_ctor="-std.socket,-std.xml" +properly_documented_public_functions="+std.algorithm.internal" +explicitly_annotated_unittests="+std.algorithm.searching" From 5facd9b6740640863ebdee1cc6f452fc17c31c59 Mon Sep 17 00:00:00 2001 From: Sebastian Wilzbach Date: Mon, 19 Jun 2017 13:03:27 +0200 Subject: [PATCH 3/5] Remove unused label --- std/algorithm/sorting.d | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/std/algorithm/sorting.d b/std/algorithm/sorting.d index 99ac551649e..e18f2d0d9b7 100644 --- a/std/algorithm/sorting.d +++ b/std/algorithm/sorting.d @@ -3402,7 +3402,7 @@ body r.swapAt(rite, pivot); } // Second loop: make left and pivot meet - outer: for (; rite > pivot; --rite) + for (; rite > pivot; --rite) { if (!lp(r[rite], r[oldPivot])) continue; while (rite > pivot) From ffc6ec5d6b5ca81cc4c97cbce4c7874d476898f6 Mon Sep 17 00:00:00 2001 From: Sebastian Wilzbach Date: Mon, 19 Jun 2017 13:13:42 +0200 Subject: [PATCH 4/5] Remove has_public_example in favor of Dscanner Previously one could only apply a check for an entire codebase and not selectively for specific modules. As a temporary solution, has_public_example has been introduced - a standalone checker with filtering capabilities. However, with selective filtering integrated in Dscanner, the analog Dscanner check `explicitly_annotated_unittests` can be used. --- .dscanner.ini | 2 +- circle.yml | 1 - circleci.sh | 7 ------- posix.mak | 8 +------- 4 files changed, 2 insertions(+), 16 deletions(-) diff --git a/.dscanner.ini b/.dscanner.ini index 0fe24530b48..8a092861bbd 100644 --- a/.dscanner.ini +++ b/.dscanner.ini @@ -116,4 +116,4 @@ unused_variable_check="+std.algorithm.internal" useless_initializer = "+std.algorithm.comparison" vcall_in_ctor="-std.socket,-std.xml" properly_documented_public_functions="+std.algorithm.internal" -explicitly_annotated_unittests="+std.algorithm.searching" +explicitly_annotated_unittests="-std.array,-std.allocator,-std.base64,-std.bitmanip,-std.concurrency,-std.conv,-std.csv,-std.datetime,-std.demangle,-std.digest.hmac,-std.digest.sha,-std.encoding,-std.exception,-std.file,-std.format,-std.getopt,-std.internal,-std.isemail,-std.json,-std.logger.core,-std.logger.nulllogger,-std.math,-std.mathspecial,-std.net.curl,-std.numeric,-std.parallelism,-std.path,-std.process,-std.random,-std.range,-std.regex,-std.socket,-std.stdio,-std.string,-std.traits,-std.typecons,-std.uni,-std.uri,-std.utf,-std.uuid,-std.xml,-std.zlib" diff --git a/circle.yml b/circle.yml index 72801310c99..aedcc77f9fb 100644 --- a/circle.yml +++ b/circle.yml @@ -8,7 +8,6 @@ test: override: - ./circleci.sh setup-repos - ./circleci.sh style_lint - - ./circleci.sh has_public_example - ./circleci.sh publictests - ./circleci.sh coverage: parallel: true diff --git a/circleci.sh b/circleci.sh index 80e528b5ac4..8865a3c7a0b 100755 --- a/circleci.sh +++ b/circleci.sh @@ -133,18 +133,11 @@ publictests() make -f posix.mak -j$N publictests DUB=$DUB } -# check modules for public unittests -has_public_example() -{ - make -f posix.mak -j$N has_public_example DUB=$DUB -} - case $1 in install-deps) install_deps ;; setup-repos) setup_repos ;; coverage) coverage ;; publictests) publictests ;; - has_public_example) has_public_example;; style_lint) style_lint ;; *) echo "Unknown command"; exit 1;; esac diff --git a/posix.mak b/posix.mak index 0eabd8dd372..582477ea58c 100644 --- a/posix.mak +++ b/posix.mak @@ -506,7 +506,6 @@ ${TOOLS_DIR}: git clone --depth=1 ${GIT_HOME}/$(@F) $@ $(TOOLS_DIR)/checkwhitespace.d: | $(TOOLS_DIR) $(TOOLS_DIR)/styles/tests_extractor.d: | $(TOOLS_DIR) -$(TOOLS_DIR)/styles/has_public_example.d: | $(TOOLS_DIR) #################### test for undesired white spaces ########################## CWS_TOCHECK = posix.mak win32.mak win64.mak osmodel.mak @@ -532,7 +531,7 @@ checkwhitespace: $(LIB) $(TOOLS_DIR)/checkwhitespace.d mv dscanner_makefile_tmp ../dscanner/makefile make -C ../dscanner githash debug -style: has_public_example publictests style_lint +style: publictests style_lint style_lint: ../dscanner/dsc $(LIB) @echo "Check for trailing whitespace" @@ -599,11 +598,6 @@ $(TEST_EXTRACTOR): $(TOOLS_DIR)/styles/tests_extractor.d $(LIB) @$(TEST_EXTRACTOR) --inputdir $< --outputdir $(PUBLICTESTS_DIR) @$(DMD) $(DFLAGS) -defaultlib= -debuglib= $(LIB) -main -unittest -run $(PUBLICTESTS_DIR)/$(subst /,_,$<) -has_public_example: $(LIB) - # checks whether public function have public examples (for now some modules are excluded) - rm -rf ./out - DFLAGS="$(DFLAGS) $(LIB) $(LINKDL)" $(DUB) -v --compiler=$${PWD}/$(DMD) --root=../tools/styles -c has_public_example -- --inputdir std --ignore "array.d,allocator,base64.d,bitmanip.d,concurrency.d,conv.d,csv.d,datetime/date.d,datetime/interval.d,datetime/package.d,datetime/stopwatch.d,datetime/systime.d,datetime/timezone.d,demangle.d,digest/hmac.d,digest/sha.d,encoding.d,exception.d,file.d,format.d,getopt.d,index.d,internal,isemail.d,json.d,logger/core.d,logger/nulllogger.d,math.d,mathspecial.d,net/curl.d,numeric.d,parallelism.d,path.d,process.d,random.d,range,regex/package.d,socket.d,stdio.d,string.d,traits.d,typecons.d,uni.d,unittest.d,uri.d,utf.d,uuid.d,xml.d,zlib.d" - .PHONY : auto-tester-build auto-tester-build: all checkwhitespace From 3a13b1212021769f9e417c641b4dbb2b6fbad48c Mon Sep 17 00:00:00 2001 From: Sebastian Wilzbach Date: Mon, 19 Jun 2017 14:06:49 +0200 Subject: [PATCH 5/5] Use current DMD to compile Dscanner --- posix.mak | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/posix.mak b/posix.mak index 582477ea58c..6b6c9bb0a3e 100644 --- a/posix.mak +++ b/posix.mak @@ -525,11 +525,11 @@ checkwhitespace: $(LIB) $(TOOLS_DIR)/checkwhitespace.d git -C ../dscanner checkout phobos git -C ../dscanner submodule update --init --recursive -../dscanner/dsc: ../dscanner +../dscanner/dsc: ../dscanner $(DMD) $(LIB) # debug build is faster, but disable 'missing import' messages (missing core from druntime) sed 's/dparse_verbose/StdLoggerDisableWarning/' ../dscanner/makefile > dscanner_makefile_tmp mv dscanner_makefile_tmp ../dscanner/makefile - make -C ../dscanner githash debug + DC=$(DMD) DFLAGS="$(DFLAGS) -defaultlib=$(LIB)" make -C ../dscanner githash debug style: publictests style_lint