Merge pull request #482 from wilzbach/phobo-master

Merge master into phobos
merged-on-behalf-of: Sebastian Wilzbach <sebi.wilzbach@gmail.com>
This commit is contained in:
The Dlang Bot 2017-06-28 04:07:12 +02:00 committed by GitHub
commit 18f0a0c1a3
17 changed files with 516 additions and 105 deletions

91
.dscanner.ini Normal file
View File

@ -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"

View File

@ -10,4 +10,5 @@ elif [[ $DC == ldc2 ]]; then
else
git submodule update --init --recursive
make test
make lint
fi

172
README.md
View File

@ -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
<module>
<declaration>
<importDeclaration>
<singleImport>
<identifierChain>
<identifier>std</identifier>
<identifier>stdio</identifier>
</identifierChain>
</singleImport>
</importDeclaration>
</declaration>
<declaration>
<functionDeclaration line="3">
<name>main</name>
<type pretty="void">
<type2>
void
</type2>
</type>
<parameters>
<parameter>
<name>args</name>
<type pretty="string[]">
<type2>
<symbol>
<identifierOrTemplateChain>
<identifierOrTemplateInstance>
<identifier>string</identifier>
</identifierOrTemplateInstance>
</identifierOrTemplateChain>
</symbol>
</type2>
<typeSuffix type="[]"/>
</type>
<identifier>args</identifier>
</parameter>
</parameters>
<functionBody>
<blockStatement>
<declarationsAndStatements>
<declarationOrStatement>
<statement>
<statementNoCaseNoDefault>
<expressionStatement>
<expression>
<assignExpression>
<functionCallExpression>
<unaryExpression>
<primaryExpression>
<identifierOrTemplateInstance>
<identifier>writeln</identifier>
</identifierOrTemplateInstance>
</primaryExpression>
</unaryExpression>
<arguments>
<argumentList>
<assignExpression>
<primaryExpression>
<stringLiteral>Hello World</stringLiteral>
</primaryExpression>
</assignExpression>
</argumentList>
</arguments>
</functionCallExpression>
</assignExpression>
</expression>
</expressionStatement>
</statementNoCaseNoDefault>
</statement>
</declarationOrStatement>
</declarationsAndStatements>
</blockStatement>
</functionBody>
</functionDeclaration>
</declaration>
</module>
```sh
$ dscanner --ast helloworld.d
```
```xml
<module>
<declaration>
<importDeclaration>
<singleImport>
<identifierChain>
<identifier>std</identifier>
<identifier>stdio</identifier>
</identifierChain>
</singleImport>
</importDeclaration>
</declaration>
<declaration>
<functionDeclaration line="3">
<name>main</name>
<type pretty="void">
<type2>
void
</type2>
</type>
<parameters>
<parameter>
<name>args</name>
<type pretty="string[]">
<type2>
<symbol>
<identifierOrTemplateChain>
<identifierOrTemplateInstance>
<identifier>string</identifier>
</identifierOrTemplateInstance>
</identifierOrTemplateChain>
</symbol>
</type2>
<typeSuffix type="[]"/>
</type>
<identifier>args</identifier>
</parameter>
</parameters>
<functionBody>
<blockStatement>
<declarationsAndStatements>
<declarationOrStatement>
<statement>
<statementNoCaseNoDefault>
<expressionStatement>
<expression>
<assignExpression>
<functionCallExpression>
<unaryExpression>
<primaryExpression>
<identifierOrTemplateInstance>
<identifier>writeln</identifier>
</identifierOrTemplateInstance>
</primaryExpression>
</unaryExpression>
<arguments>
<argumentList>
<assignExpression>
<primaryExpression>
<stringLiteral>Hello World</stringLiteral>
</primaryExpression>
</assignExpression>
</argumentList>
</arguments>
</functionCallExpression>
</assignExpression>
</expression>
</expressionStatement>
</statementNoCaseNoDefault>
</statement>
</declarationOrStatement>
</declarationsAndStatements>
</blockStatement>
</functionBody>
</functionDeclaration>
</declaration>
</module>
```
For more readable output, pipe the command through [xmllint](http://xmlsoft.org/xmllint.html)
using its formatting switch.

View File

@ -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

View File

@ -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

View File

@ -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;
}

View File

@ -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;
}

View File

@ -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;

View File

@ -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.");
}

View File

@ -66,7 +66,7 @@ class ImportSortednessCheck : BaseAnalyzer
private:
int level = 0;
int level;
string[][int] imports;
template ScopedVisit(NodeType)

View File

@ -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;

View File

@ -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)

View File

@ -71,7 +71,7 @@ private:
{
override void visit(const T thing)
{
auto b = interesting;
const b = interesting;
interesting = true;
thing.accept(this);
interesting = b;

View File

@ -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)
{

View File

@ -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";
}

View File

@ -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,

View File

@ -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;