diff --git a/.dscanner.ini b/.dscanner.ini new file mode 100644 index 0000000..5f3682c --- /dev/null +++ b/.dscanner.ini @@ -0,0 +1,91 @@ +; Configure which static analysis checks are enabled +[analysis.config.StaticAnalysisConfig] +; Check variable, class, struct, interface, union, and function names against t +; he Phobos style guide +style_check="disabled" +; Check for array literals that cause unnecessary allocation +enum_array_literal_check="enabled" +; Check for poor exception handling practices +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="enabled" +; Checks that opEquals, opCmp, toHash, and toString are either const, immutable +; , or inout. +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" +; Checks for unused labels +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" +; Checks for subtraction from .length properties +length_subtraction_check="disabled" +; 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="enabled" +; Checks for confusing logical operator precedence +logical_precedence_check="enabled" +; Checks for undocumented public declarations +undocumented_declaration_check="disabled" +; Checks for poor placement of function attributes +function_attribute_check="disabled" +; Checks for use of the comma operator +comma_expression_check="enabled" +; Checks for local imports that are too broad +local_import_check="enabled" +; Checks for variables that could be declared immutable +could_be_immutable_check="disabled" +; 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="enabled" +; Checks for labels with the same name as variables +label_var_same_name_check="enabled" +; Checks for lines longer than 120 characters +long_line_check="disabled" +; Checks for assignment to auto-ref function parameters +auto_ref_assignment_check="enabled" +; Checks for incorrect infinite range definitions +incorrect_infinite_range_check="enabled" +; Checks for asserts that are always true +useless_assert_check="enabled" +; Check for uses of the old-style alias syntax +alias_syntax_check="enabled" +; Checks for else if that should be else static if +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 sortedness of imports +imports_sortedness="disabled" +; Check for explicitly annotated unittests +explicitly_annotated_unittests="disabled" +; Check for properly documented public functions (Returns, Params) +properly_documented_public_functions="disabled" +; Check for useless usage of the final attribute +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" diff --git a/.travis.sh b/.travis.sh index daa5661..d1c14b1 100755 --- a/.travis.sh +++ b/.travis.sh @@ -10,4 +10,5 @@ elif [[ $DC == ldc2 ]]; then else git submodule update --init --recursive make test + make lint fi diff --git a/README.md b/README.md index f5a7062..34a9b75 100644 --- a/README.md +++ b/README.md @@ -21,11 +21,13 @@ compilers instead of DMD. To install, simply place the generated binary (in the # Usage The following examples assume that we are analyzing a simple file called helloworld.d - import std.stdio; - void main(string[] args) - { - writeln("Hello World"); - } +```d +import std.stdio; +void main(string[] args) +{ + writeln("Hello World"); +} +``` ### Token Count The "--tokenCount" or "-t" option prints the number of tokens in the given file @@ -131,6 +133,7 @@ Note that the "--skipTests" option is the equivalent of changing each * Useless initializers. * Allman brace style * Redundant visibility attributes +* Public declarations without a documented unittest. By default disabled. #### Wishlist @@ -214,83 +217,88 @@ If a `dscanner.ini` file is locate in the working directory or any of it's paren The "--ast" or "--xml" options will dump the complete abstract syntax tree of the given source file to standard output in XML format. - $ dscanner --ast helloworld.d - - - - - - std - stdio - - - - - - - main - - - void - - - - - args - - - - - - string - - - - - - - args - - - - - - - - - - - - - - - - writeln - - - - - - - - Hello World - - - - - - - - - - - - - - - - - +```sh +$ dscanner --ast helloworld.d +``` + +```xml + + + + + +std +stdio + + + + + + +main + + +void + + + + +args + + + + + +string + + + + + + +args + + + + + + + + + + + + + + + +writeln + + + + + + + +Hello World + + + + + + + + + + + + + + + + + +``` For more readable output, pipe the command through [xmllint](http://xmlsoft.org/xmllint.html) using its formatting switch. diff --git a/makefile b/makefile index bf0a71c..d98e9f6 100644 --- a/makefile +++ b/makefile @@ -51,6 +51,9 @@ test: githash ./bin/dscanner-unittest rm -f bin/dscanner-unittest +lint: dmdbuild + ./bin/dscanner --config .dscanner.ini --styleCheck src + clean: rm -rf dsc rm -rf bin diff --git a/src/analysis/allman.d b/src/analysis/allman.d index c7b03e9..fb9c9ec 100644 --- a/src/analysis/allman.d +++ b/src/analysis/allman.d @@ -33,8 +33,8 @@ class AllManCheck : BaseAnalyzer super(fileName, null, skipTests); foreach (i; 1 .. tokens.length - 1) { - auto curLine = tokens[i].line; - auto prevTokenLine = tokens[i-1].line; + const curLine = tokens[i].line; + const prevTokenLine = tokens[i-1].line; if (tokens[i].type == tok!"{" && curLine == prevTokenLine) { // ignore struct initialization diff --git a/src/analysis/builtin_property_names.d b/src/analysis/builtin_property_names.d index f359bd8..ad72a7f 100644 --- a/src/analysis/builtin_property_names.d +++ b/src/analysis/builtin_property_names.d @@ -93,10 +93,10 @@ private: { import std.algorithm : canFind; - return builtinProperties.canFind(name); + return BuiltinProperties.canFind(name); } - enum string[] builtinProperties = ["init", "sizeof", "mangleof", "alignof", "stringof"]; + enum string[] BuiltinProperties = ["init", "sizeof", "mangleof", "alignof", "stringof"]; int depth; } diff --git a/src/analysis/config.d b/src/analysis/config.d index 7c13f55..1966e98 100644 --- a/src/analysis/config.d +++ b/src/analysis/config.d @@ -186,6 +186,9 @@ struct StaticAnalysisConfig @INI("Check for redundant attributes") string redundant_attributes_check = Check.enabled; + @INI("Check public declarations without a documented unittest") + string has_public_example = Check.disabled; + @INI("Module-specific filters") ModuleFilters filters; } diff --git a/src/analysis/function_attributes.d b/src/analysis/function_attributes.d index f060823..5dde5f2 100644 --- a/src/analysis/function_attributes.d +++ b/src/analysis/function_attributes.d @@ -32,7 +32,7 @@ class FunctionAttributeCheck : BaseAnalyzer override void visit(const InterfaceDeclaration dec) { - auto t = inInterface; + const t = inInterface; inInterface = true; dec.accept(this); inInterface = t; @@ -40,7 +40,7 @@ class FunctionAttributeCheck : BaseAnalyzer override void visit(const ClassDeclaration dec) { - auto t = inInterface; + const t = inInterface; inInterface = false; dec.accept(this); inInterface = t; diff --git a/src/analysis/has_public_example.d b/src/analysis/has_public_example.d new file mode 100644 index 0000000..9a0bba8 --- /dev/null +++ b/src/analysis/has_public_example.d @@ -0,0 +1,302 @@ +// Distributed under the Boost Software License, Version 1.0. +// (See accompanying file LICENSE_1_0.txt or copy at +// http://www.boost.org/LICENSE_1_0.txt) + +module analysis.has_public_example; + +import analysis.base; +import dsymbol.scope_ : Scope; +import dparse.ast; +import dparse.lexer; + +import std.algorithm; +import std.stdio; + +/** + * Checks for public declarations without a documented unittests. + * For now, variable and enum declarations aren't checked. + */ +class HasPublicExampleCheck : BaseAnalyzer +{ + alias visit = BaseAnalyzer.visit; + + this(string fileName, const(Scope)* sc, bool skipTests = false) + { + super(fileName, sc, skipTests); + } + + override void visit(const Module mod) + { + // the last seen declaration is memorized + Declaration lastDecl; + + // keep track of ddoced unittests after visiting lastDecl + bool hasNoDdocUnittest; + + // on lastDecl reset we check for seen ddoced unittests since lastDecl was observed + void checkLastDecl() + { + if (lastDecl !is null && hasNoDdocUnittest) + triggerError(lastDecl); + lastDecl = null; + } + + // check all public top-level declarations + foreach (decl; mod.declarations.filter!(decl => isPublic(decl.attributes))) + { + const bool hasDdocHeader = hasDdocHeader(decl); + + // check the documentation of a unittest declaration + if (decl.unittest_ !is null) + { + if (hasDdocHeader) + hasNoDdocUnittest = false; + } + // add all declarations that could be publicly documented to the lastDecl "stack" + else if (hasDittableDecl(decl)) + { + // ignore dittoed declarations + if (hasDittos(decl)) + continue; + + // new public symbol -> check the previous decl + checkLastDecl; + + lastDecl = hasDdocHeader ? cast(Declaration) decl : null; + hasNoDdocUnittest = true; + } + else + // ran into variableDeclaration or something else -> reset & validate current lastDecl "stack" + checkLastDecl; + } + checkLastDecl; + } + +private: + + bool hasDitto(Decl)(const Decl decl) + { + import ddoc.comments : parseComment; + if (decl is null || decl.comment is null) + return false; + + return parseComment(decl.comment, null).isDitto; + } + + bool hasDittos(Decl)(const Decl decl) + { + foreach (property; possibleDeclarations) + if (mixin("hasDitto(decl." ~ property ~ ")")) + return true; + return false; + } + + bool hasDittableDecl(Decl)(const Decl decl) + { + foreach (property; possibleDeclarations) + if (mixin("decl." ~ property ~ " !is null")) + return true; + return false; + } + + import std.meta : AliasSeq; + alias possibleDeclarations = AliasSeq!( + "classDeclaration", + "enumDeclaration", + "functionDeclaration", + "interfaceDeclaration", + "structDeclaration", + "templateDeclaration", + "unionDeclaration", + //"variableDeclaration", + ); + + bool hasDdocHeader(const Declaration decl) + { + if (decl.declarations !is null) + return false; + + // unittest can have ddoc headers as well, but don't have a name + if (decl.unittest_ !is null && decl.unittest_.comment.ptr !is null) + return true; + + foreach (property; possibleDeclarations) + if (mixin("decl." ~ property ~ " !is null && decl." ~ property ~ ".comment.ptr !is null")) + return true; + + return false; + } + + bool isPublic(const Attribute[] attrs) + { + import dparse.lexer : tok; + + enum tokPrivate = tok!"private", tokProtected = tok!"protected", tokPackage = tok!"package"; + + if (attrs.map!`a.attribute`.any!(x => x == tokPrivate || x == tokProtected || x == tokPackage)) + return false; + + return true; + } + + void triggerError(const Declaration decl) + { + foreach (property; possibleDeclarations) + if (auto fn = mixin("decl." ~ property)) + addMessage(fn.name.line, fn.name.column, fn.name.text); + } + + void addMessage(size_t line, size_t column, string name) + { + import std.string : format; + + addErrorMessage(line, column, "dscanner.style.has_public_example", name is null + ? "Public declaration has no documented example." + : format("Public declaration '%s' has no documented example.", name)); + } +} + +unittest +{ + import std.stdio : stderr; + import std.format : format; + import analysis.config : StaticAnalysisConfig, Check, disabledConfig; + import analysis.helpers : assertAnalyzerWarnings; + + StaticAnalysisConfig sac = disabledConfig(); + sac.has_public_example = Check.enabled; + + assertAnalyzerWarnings(q{ + /// C + class C{} + /// + unittest {} + + /// I + interface I{} + /// + unittest {} + + /// e + enum e = 0; + /// + unittest {} + + /// f + void f(){} + /// + unittest {} + + /// S + struct S{} + /// + unittest {} + + /// T + template T(){} + /// + unittest {} + + /// U + union U{} + /// + unittest {} + }, sac); + + // enums or variables don't need to have public unittest + assertAnalyzerWarnings(q{ + /// C + class C{} // [warn]: Public declaration 'C' has no documented example. + unittest {} + + /// I + interface I{} // [warn]: Public declaration 'I' has no documented example. + unittest {} + + /// f + void f(){} // [warn]: Public declaration 'f' has no documented example. + unittest {} + + /// S + struct S{} // [warn]: Public declaration 'S' has no documented example. + unittest {} + + /// T + template T(){} // [warn]: Public declaration 'T' has no documented example. + unittest {} + + /// U + union U{} // [warn]: Public declaration 'U' has no documented example. + unittest {} + }, sac); + + // test module header unittest + assertAnalyzerWarnings(q{ + unittest {} + /// C + class C{} // [warn]: Public declaration 'C' has no documented example. + }, sac); + + // test documented module header unittest + assertAnalyzerWarnings(q{ + /// + unittest {} + /// C + class C{} // [warn]: Public declaration 'C' has no documented example. + }, sac); + + // test multiple unittest blocks + assertAnalyzerWarnings(q{ + /// C + class C{} // [warn]: Public declaration 'C' has no documented example. + unittest {} + unittest {} + unittest {} + + /// U + union U{} + unittest {} + /// + unittest {} + unittest {} + }, sac); + + /// check private + assertAnalyzerWarnings(q{ + /// C + private class C{} + + /// I + protected interface I{} + + /// e + package enum e = 0; + + /// f + package(std) void f(){} + + /// S + extern(C) struct S{} + /// + unittest {} + }, sac); + + /// check intermediate private declarations and ditto-ed declarations + assertAnalyzerWarnings(q{ + /// C + class C{} + private void foo(){} + /// + unittest {} + + /// I + interface I{} + /// ditto + void f(){} + /// + unittest {} + }, sac); + + stderr.writeln("Unittest for HasPublicExampleCheck passed."); +} + diff --git a/src/analysis/imports_sortedness.d b/src/analysis/imports_sortedness.d index ff35d9e..0628f06 100644 --- a/src/analysis/imports_sortedness.d +++ b/src/analysis/imports_sortedness.d @@ -66,7 +66,7 @@ class ImportSortednessCheck : BaseAnalyzer private: - int level = 0; + int level; string[][int] imports; template ScopedVisit(NodeType) diff --git a/src/analysis/label_var_same_name_check.d b/src/analysis/label_var_same_name_check.d index b3cd80e..eb4172b 100644 --- a/src/analysis/label_var_same_name_check.d +++ b/src/analysis/label_var_same_name_check.d @@ -78,7 +78,7 @@ private: import std.conv : to; import std.range : retro; - size_t i = 0; + size_t i; foreach (s; retro(stack)) { const(Thing)* thing = name.text in s; diff --git a/src/analysis/line_length.d b/src/analysis/line_length.d index 2cf86ce..542f330 100644 --- a/src/analysis/line_length.d +++ b/src/analysis/line_length.d @@ -26,7 +26,7 @@ class LineLengthCheck : BaseAnalyzer override void visit(const Module) { - size_t endColumn = 0; + size_t endColumn; lastErrorLine = ulong.max; foreach (i, token; tokens) { @@ -89,7 +89,6 @@ private: unittest { - import std.stdio; assert(new LineLengthCheck(null, null).checkMultiLineToken(Token(tok!"stringLiteral", " ", 0, 0, 0)) == 8); assert(new LineLengthCheck(null, null).checkMultiLineToken(Token(tok!"stringLiteral", " \na", 0, 0, 0)) == 2); assert(new LineLengthCheck(null, null).checkMultiLineToken(Token(tok!"stringLiteral", " \n ", 0, 0, 0)) == 5); @@ -123,8 +122,8 @@ private: { import std.utf : byDchar; - size_t length = 0; - bool newLine = tok.line > prevLine; + size_t length; + const newLine = tok.line > prevLine; bool multiLine; if (tok.text is null) diff --git a/src/analysis/local_imports.d b/src/analysis/local_imports.d index 13a2214..52bece3 100644 --- a/src/analysis/local_imports.d +++ b/src/analysis/local_imports.d @@ -71,7 +71,7 @@ private: { override void visit(const T thing) { - auto b = interesting; + const b = interesting; interesting = true; thing.accept(this); interesting = b; diff --git a/src/analysis/pokemon.d b/src/analysis/pokemon.d index fd58d77..9bfa303 100644 --- a/src/analysis/pokemon.d +++ b/src/analysis/pokemon.d @@ -67,7 +67,7 @@ class PokemonExceptionCheck : BaseAnalyzer { return; } - auto identOrTemplate = type2.symbol.identifierOrTemplateChain + const identOrTemplate = type2.symbol.identifierOrTemplateChain .identifiersOrTemplateInstances[0]; if (identOrTemplate.templateInstance !is null) { diff --git a/src/analysis/redundant_attributes.d b/src/analysis/redundant_attributes.d index 3ceaaad..bc3bf2b 100644 --- a/src/analysis/redundant_attributes.d +++ b/src/analysis/redundant_attributes.d @@ -43,7 +43,7 @@ class RedundantAttributesCheck : BaseAnalyzer // new scope: private { } if (decl.declarations.length > 0) { - auto prev = currentAttributes[]; + const prev = currentAttributes[]; // append to current scope and reset once block is left foreach (attr; attributes) addAttribute(attr); @@ -113,7 +113,7 @@ private: void removeOverwrite(const Attribute attr) { import std.array : array; - auto group = getAttributeGroup(attr); + const group = getAttributeGroup(attr); if (currentAttributes.filter!(a => getAttributeGroup(a) == group && !isIdenticalAttribute(a, attr)).walkLength > 0) { @@ -172,7 +172,6 @@ private: stack.length--; } -private: enum string KEY = "dscanner.suspicious.redundant_attributes"; } diff --git a/src/analysis/run.d b/src/analysis/run.d index 9822cb4..9f8801e 100644 --- a/src/analysis/run.d +++ b/src/analysis/run.d @@ -68,6 +68,7 @@ import analysis.vcall_in_ctor; import analysis.useless_initializer; import analysis.allman; import analysis.redundant_attributes; +import analysis.has_public_example; import dsymbol.string_interning : internString; import dsymbol.scope_; @@ -168,8 +169,8 @@ bool analyze(string[] fileNames, const StaticAnalysisConfig config, if (code.length == 0) continue; RollbackAllocator r; - uint errorCount = 0; - uint warningCount = 0; + uint errorCount; + uint warningCount; const(Token)[] tokens; const Module m = parseModule(fileName, code, &r, cache, false, tokens, null, &errorCount, &warningCount); @@ -476,6 +477,10 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a checks ~= new RedundantAttributesCheck(fileName, moduleScope, analysisConfig.redundant_attributes_check == Check.skipTests && !ut); + if (analysisConfig.has_public_example!= Check.disabled) + checks ~= new HasPublicExampleCheck(fileName, moduleScope, + analysisConfig.has_public_example == Check.skipTests && !ut); + version (none) if (moduleName.shouldRun!"redundant_if_check"(analysisConfig)) checks ~= new IfStatementCheck(fileName, moduleScope, diff --git a/src/etags.d b/src/etags.d index c331abf..a89ce6c 100644 --- a/src/etags.d +++ b/src/etags.d @@ -97,7 +97,7 @@ final class EtagsPrinter : ASTVisitor override void visit(const ModuleDeclaration dec) { auto tok0 = dec.moduleName.identifiers[0]; - auto was = context; + const was = context; context = ""; maketag(moduleName, tok0.index, tok0.line); context = was; @@ -115,7 +115,7 @@ final class EtagsPrinter : ASTVisitor // visibility needs to be restored to what it was when changed by // attribute. - auto was = visibility; + const was = visibility; foreach (attr; dec.attributes) { updateVisibility(attr.attribute.type); @@ -217,7 +217,7 @@ final class EtagsPrinter : ASTVisitor override void visit(const Unittest dec) { - bool was = inUnittest; + const was = inUnittest; inUnittest = true; dec.accept(this); inUnittest = was; @@ -287,7 +287,7 @@ private: void acceptInContext(const ASTNode dec, string name) { // nest context before journeying on - auto c = context; + const c = context; context ~= name ~ "."; dec.accept(this); context = c;