Skip to content

Apply Dscanner checks only to specific modules #5495

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 21, 2017
Merged

Apply Dscanner checks only to specific modules #5495

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f77a768
Use Dscanner with selective checks
wilzbach Jun 19, 2017
43b33c6
Enable selective module checks
wilzbach Jun 19, 2017
5facd9b
Remove unused label
wilzbach Jun 19, 2017
ffc6ec5
Remove has_public_example in favor of Dscanner
wilzbach Jun 19, 2017
3a13b12
Use current DMD to compile Dscanner
wilzbach Jun 19, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 62 additions & 23 deletions .dscanner.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,64 +2,65 @@
[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
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
Expand All @@ -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"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand + denotes a whitelist? Why not a blacklist?

Copy link
Member Author

@wilzbach wilzbach Jun 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - (blacklisting) is supported as well. The documentation isn't public yet, but it can be found e.g. here:
https://github.com/dlang-community/D-Scanner/blob/d9df33a53f03dd0647316c64c848a8c214bb4a0d/README.md#selecting-modules-for-a-specific-check

I am a huge fan of using blacklists as well, I was just too lazy to generate a specific module blacklist list for every non-enabled check. However the explicitly_annotated_unittests check is an example of using blacklists

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be doable with Dustmite :)

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.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"
1 change: 0 additions & 1 deletion circle.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 0 additions & 7 deletions circleci.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
16 changes: 5 additions & 11 deletions posix.mak
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -522,17 +521,17 @@ 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a feature branch may interfere with editor/IDE Dscanner integrations, is this temporary?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, my hope is that with by using this feature at Phobos, the approval of the [respective PR] at Dscanner gets sped up. However, considering that in the past I was often blocked due to slow response times, having our own branch to add unapproved features (and workarounds) might not be a bad idea.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have enabled force-push protection and a passing CI status requirement for phobos branch at dlang-community, so we should be safe from accidentally breaking the Phobos setup.

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: has_public_example publictests style_lint
style: publictests style_lint

style_lint: ../dscanner/dsc $(LIB)
@echo "Check for trailing whitespace"
Expand Down Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion std/algorithm/sorting.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down