Merge branch 'master' into phobos

This commit is contained in:
Sebastian Wilzbach 2018-05-30 12:57:42 +02:00
commit 1b88b41f6b
59 changed files with 998 additions and 565 deletions

View File

@ -1,8 +1,6 @@
sudo: false sudo: false
language: d language: d
d: d:
- dmd-nightly
- dmd-beta
- dmd - dmd
- ldc-beta - ldc-beta
- ldc - ldc
@ -43,6 +41,27 @@ jobs:
on: on:
repo: dlang-community/D-Scanner repo: dlang-community/D-Scanner
tags: true tags: true
- stage: GitHub Release
#if: tag IS present
d: dmd
os: linux
language: generic
sudo: yes
script: echo "Deploying to GitHub releases ..." && ./release-windows.sh
addons:
apt:
packages:
- p7zip-full
- wine
deploy:
provider: releases
api_key: $GH_REPO_TOKEN
file_glob: true
file: bin/dscanner-*.zip
skip_cleanup: true
on:
repo: dlang-community/D-Scanner
tags: true
stages: stages:
- name: test - name: test
if: type = pull_request or (type = push and branch = master) if: type = pull_request or (type = push and branch = master)

View File

@ -1,12 +1,6 @@
platform: x64 platform: x64
environment: environment:
matrix: matrix:
#- DC: dmd
#DVersion: beta
#arch: x64
#- DC: dmd
#DVersion: beta
#arch: x86
- DC: dmd - DC: dmd
DVersion: stable DVersion: stable
arch: x64 arch: x64

@ -1 +1 @@
Subproject commit 6c5504cc80b75192b24cebe93209521c03f806d8 Subproject commit c261fa119072ce788ef81b8d8fee9a2adddca5d1

@ -1 +1 @@
Subproject commit 239b137b280c06864b73fcc1d00b75e06568d4c2 Subproject commit e018446b028b92c34398032e698c05c0919630ee

View File

@ -12,12 +12,18 @@
"StdLoggerDisableWarning" "StdLoggerDisableWarning"
], ],
"dependencies" : { "dependencies" : {
"libdparse": "~>0.8.0", "libdparse" : "~>0.8.6",
"dsymbol" : "~>0.3.0", "dsymbol" : "~>0.3.7",
"inifiled" : "~>1.3.0", "inifiled" : "~>1.3.1",
"emsi_containers" : "~>0.6.0", "emsi_containers" : "~>0.8.0-alpha.7",
"libddoc" : "~>0.3.0-beta.1", "libddoc" : "~>0.3.0-beta.1",
"stdx-allocator" : "~>2.77.0" "stdx-allocator" : "~>2.77.2"
}, },
"targetPath" : "bin" "targetPath" : "bin",
"stringImportPaths" : [
"bin"
],
"preGenerateCommands" : [
"rdmd --eval=\"auto dir=environment.get(\\\"DUB_PACKAGE_DIR\\\"); dir.buildPath(\\\"bin\\\").mkdirRecurse; auto gitVer = (\\\"git -C \\\"~dir~\\\" describe --tags\\\").executeShell; (gitVer.status == 0 ? gitVer.output : dir.dirName.baseName.findSplitAfter(environment.get(\\\"DUB_ROOT_PACKAGE\\\")~\\\"-\\\")[1]).ifThrown(\\\"0.0.0\\\").toFile(dir.buildPath(\\\"bin\\\", \\\"dubhash.txt\\\"));\""
]
} }

@ -1 +1 @@
Subproject commit 9d7333ec17f116a05712a24df139ff2f212a9867 Subproject commit cecaff8037a60db2a51c9bded4802c87d938a44e

@ -1 +1 @@
Subproject commit 970efe34e66fc7b3cb93a6ec59984099908070c5 Subproject commit cf102ff8e848fb18d2ce7056ae61dafb5333012d

View File

@ -36,7 +36,7 @@ ldc: ldcbuild
gdc: gdcbuild gdc: gdcbuild
githash: githash:
git log -1 --format="%H" > githash.txt git describe --tags > githash.txt
debug: debug:
${DC} -fPIC -w -g -J. -ofdsc ${VERSIONS} ${DEBUG_VERSIONS} ${INCLUDE_PATHS} ${SRC} ${DC} -fPIC -w -g -J. -ofdsc ${VERSIONS} ${DEBUG_VERSIONS} ${INCLUDE_PATHS} ${SRC}

25
release-windows.sh Executable file
View File

@ -0,0 +1,25 @@
#!/usr/bin/env bash
# Build the Windows binaries under Linux (requires wine)
set -eux -o pipefail
VERSION=$(git describe --abbrev=0 --tags)
OS=windows
ARCH_SUFFIX="x86"
# Allow the script to be run from anywhere
DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
cd $DIR
# Step 1: download the DMD binaries
if [ ! -d dmd2 ] ; then
wget http://downloads.dlang.org/releases/2.x/2.079.0/dmd.2.079.0.windows.7z
7z x dmd.2.079.0.windows.7z > /dev/null
fi
# Step 2: Run DMD via wineconsole
archiveName="dscanner-$VERSION-$OS-$ARCH_SUFFIX.zip"
echo "Building $archiveName"
mkdir -p bin
DC="$DIR/dmd2/windows/bin/dmd.exe" wine cmd /C build.bat
cd bin
zip "$archiveName" dscanner.exe

View File

@ -12,7 +12,7 @@ import dscanner.analysis.base;
/** /**
* Checks for uses of the old alias syntax. * Checks for uses of the old alias syntax.
*/ */
class AliasSyntaxCheck : BaseAnalyzer final class AliasSyntaxCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -25,7 +25,7 @@ if (param < 0)
} }
------------ ------------
*/ */
class AllManCheck : BaseAnalyzer final class AllManCheck : BaseAnalyzer
{ {
/// ///
this(string fileName, const(Token)[] tokens, bool skipTests = false) this(string fileName, const(Token)[] tokens, bool skipTests = false)

View File

@ -16,7 +16,7 @@ import dsymbol.scope_ : Scope;
* Checks for confusing asm expressions. * Checks for confusing asm expressions.
* See_also: $(LINK https://issues.dlang.org/show_bug.cgi?id=9738) * See_also: $(LINK https://issues.dlang.org/show_bug.cgi?id=9738)
*/ */
class AsmStyleCheck : BaseAnalyzer final class AsmStyleCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -16,7 +16,7 @@ import std.algorithm;
/** /**
* Check that all asserts have an explanatory message. * Check that all asserts have an explanatory message.
*/ */
class AssertWithoutMessageCheck : BaseAnalyzer final class AssertWithoutMessageCheck : BaseAnalyzer
{ {
enum string KEY = "dscanner.style.assert_without_msg"; enum string KEY = "dscanner.style.assert_without_msg";
enum string MESSAGE = "An assert should have an explanatory message"; enum string MESSAGE = "An assert should have an explanatory message";

View File

@ -12,7 +12,7 @@ import dscanner.analysis.base;
/** /**
* Checks for assignment to auto-ref function parameters. * Checks for assignment to auto-ref function parameters.
*/ */
class AutoRefAssignmentCheck : BaseAnalyzer final class AutoRefAssignmentCheck : BaseAnalyzer
{ {
/// ///
this(string fileName, bool skipTests = false) this(string fileName, bool skipTests = false)

View File

@ -27,7 +27,7 @@ import std.algorithm : map;
* } * }
* --- * ---
*/ */
class BuiltinPropertyNameCheck : BaseAnalyzer final class BuiltinPropertyNameCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -13,7 +13,7 @@ import dsymbol.scope_;
/** /**
* Check for uses of the comma expression. * Check for uses of the comma expression.
*/ */
class CommaExpressionCheck : BaseAnalyzer final class CommaExpressionCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -7,7 +7,7 @@ import dscanner.analysis.base;
import dscanner.analysis.helpers; import dscanner.analysis.helpers;
import dsymbol.scope_ : Scope; import dsymbol.scope_ : Scope;
class ConstructorCheck : BaseAnalyzer final class ConstructorCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -14,7 +14,7 @@ import dsymbol.scope_;
/** /**
* Checks for use of the deprecated 'delete' keyword * Checks for use of the deprecated 'delete' keyword
*/ */
class DeleteCheck : BaseAnalyzer final class DeleteCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -17,7 +17,7 @@ import dsymbol.scope_ : Scope;
* Checks for duplicate attributes such as @property, @safe, * Checks for duplicate attributes such as @property, @safe,
* @trusted, @system, pure, and nothrow * @trusted, @system, pure, and nothrow
*/ */
class DuplicateAttributeCheck : BaseAnalyzer final class DuplicateAttributeCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -15,7 +15,7 @@ void doNothing(string, size_t, size_t, string, bool)
{ {
} }
class EnumArrayLiteralCheck : BaseAnalyzer final class EnumArrayLiteralCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -13,7 +13,7 @@ import std.stdio;
/** /**
* Requires unittests to be explicitly annotated with either @safe or @system * Requires unittests to be explicitly annotated with either @safe or @system
*/ */
class ExplicitlyAnnotatedUnittestCheck : BaseAnalyzer final class ExplicitlyAnnotatedUnittestCheck : BaseAnalyzer
{ {
enum string KEY = "dscanner.style.explicitly_annotated_unittest"; enum string KEY = "dscanner.style.explicitly_annotated_unittest";
enum string MESSAGE = "A unittest should be annotated with at least @safe or @system"; enum string MESSAGE = "A unittest should be annotated with at least @safe or @system";

View File

@ -15,7 +15,7 @@ import dsymbol.scope_ : Scope;
/** /**
* Checks for use of the deprecated floating point comparison operators. * Checks for use of the deprecated floating point comparison operators.
*/ */
class FloatOperatorCheck : BaseAnalyzer final class FloatOperatorCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -21,7 +21,7 @@ import dsymbol.scope_;
* const int getStuff() {} * const int getStuff() {}
* --- * ---
*/ */
class FunctionAttributeCheck : BaseAnalyzer final class FunctionAttributeCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -16,7 +16,7 @@ import std.stdio;
* Checks for public declarations without a documented unittests. * Checks for public declarations without a documented unittests.
* For now, variable and enum declarations aren't checked. * For now, variable and enum declarations aren't checked.
*/ */
class HasPublicExampleCheck : BaseAnalyzer final class HasPublicExampleCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -15,7 +15,7 @@ import std.range;
/** /**
Checks whether all if constraints have the same indention as their declaration. Checks whether all if constraints have the same indention as their declaration.
*/ */
class IfConstraintsIndentCheck : BaseAnalyzer final class IfConstraintsIndentCheck : BaseAnalyzer
{ {
/// ///
this(string fileName, const(Token)[] tokens, bool skipTests = false) this(string fileName, const(Token)[] tokens, bool skipTests = false)

View File

@ -10,7 +10,7 @@ import dparse.formatter;
import dscanner.analysis.base; import dscanner.analysis.base;
import dsymbol.scope_ : Scope; import dsymbol.scope_ : Scope;
class IfStatementCheck : BaseAnalyzer final class IfStatementCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;
this(string fileName, const(Scope)* sc, bool skipTests = false) this(string fileName, const(Scope)* sc, bool skipTests = false)

View File

@ -20,7 +20,7 @@ import dsymbol.scope_ : Scope;
* $(LI == expressions where the left and right are the same) * $(LI == expressions where the left and right are the same)
* ) * )
*/ */
class IfElseSameCheck : BaseAnalyzer final class IfElseSameCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -13,7 +13,7 @@ import std.stdio;
/** /**
* Checks the sortedness of module imports * Checks the sortedness of module imports
*/ */
class ImportSortednessCheck : BaseAnalyzer final class ImportSortednessCheck : BaseAnalyzer
{ {
enum string KEY = "dscanner.style.imports_sortedness"; enum string KEY = "dscanner.style.imports_sortedness";
enum string MESSAGE = "The imports are not sorted in alphabetical order"; enum string MESSAGE = "The imports are not sorted in alphabetical order";

View File

@ -13,7 +13,7 @@ import dparse.lexer;
/** /**
* Checks for incorrect infinite range definitions * Checks for incorrect infinite range definitions
*/ */
class IncorrectInfiniteRangeCheck : BaseAnalyzer final class IncorrectInfiniteRangeCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -13,7 +13,7 @@ import dscanner.analysis.helpers;
/** /**
* Checks for labels and variables that have the same name. * Checks for labels and variables that have the same name.
*/ */
class LabelVarNameCheck : BaseAnalyzer final class LabelVarNameCheck : BaseAnalyzer
{ {
this(string fileName, const(Scope)* sc, bool skipTests = false) this(string fileName, const(Scope)* sc, bool skipTests = false)
{ {

View File

@ -9,7 +9,7 @@ import dparse.ast;
import dparse.lexer; import dparse.lexer;
import dscanner.analysis.base; import dscanner.analysis.base;
class LambdaReturnCheck : BaseAnalyzer final class LambdaReturnCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -16,7 +16,7 @@ import dsymbol.scope_;
/** /**
* Checks for subtraction from a .length property. This is usually a bug. * Checks for subtraction from a .length property. This is usually a bug.
*/ */
class LengthSubtractionCheck : BaseAnalyzer final class LengthSubtractionCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -15,7 +15,7 @@ import std.typecons : tuple, Tuple;
/** /**
* Checks for lines longer than 120 characters * Checks for lines longer than 120 characters
*/ */
class LineLengthCheck : BaseAnalyzer final class LineLengthCheck : BaseAnalyzer
{ {
/// ///
this(string fileName, const(Token)[] tokens, bool skipTests = false) this(string fileName, const(Token)[] tokens, bool skipTests = false)

View File

@ -16,7 +16,7 @@ import dsymbol.scope_;
* Checks for local imports that import all symbols. * Checks for local imports that import all symbols.
* See_also: $(LINK https://issues.dlang.org/show_bug.cgi?id=10378) * See_also: $(LINK https://issues.dlang.org/show_bug.cgi?id=10378)
*/ */
class LocalImportCheck : BaseAnalyzer final class LocalImportCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -19,7 +19,7 @@ import dsymbol.scope_;
* if (a && (b || c)) // good * if (a && (b || c)) // good
* --- * ---
*/ */
class LogicPrecedenceCheck : BaseAnalyzer final class LogicPrecedenceCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -16,7 +16,7 @@ import dsymbol.scope_ : Scope;
/** /**
* Checks for long and hard-to-read number literals * Checks for long and hard-to-read number literals
*/ */
class NumberStyleCheck : BaseAnalyzer final class NumberStyleCheck : BaseAnalyzer
{ {
public: public:
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -17,7 +17,7 @@ import dsymbol.scope_ : Scope;
* Checks that opEquals, opCmp, toHash, 'opCast', and toString are either const, * Checks that opEquals, opCmp, toHash, 'opCast', and toString are either const,
* immutable, or inout. * immutable, or inout.
*/ */
class ObjectConstCheck : BaseAnalyzer final class ObjectConstCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -16,7 +16,7 @@ import dsymbol.scope_ : Scope;
* Checks for when a class/struct has the method opEquals without toHash, or * Checks for when a class/struct has the method opEquals without toHash, or
* toHash without opEquals. * toHash without opEquals.
*/ */
class OpEqualsWithoutToHashCheck : BaseAnalyzer final class OpEqualsWithoutToHashCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -23,7 +23,7 @@ import dsymbol.scope_ : Scope;
* } * }
* --- * ---
*/ */
class PokemonExceptionCheck : BaseAnalyzer final class PokemonExceptionCheck : BaseAnalyzer
{ {
enum MESSAGE = "Catching Error or Throwable is almost always a bad idea."; enum MESSAGE = "Catching Error or Throwable is almost always a bad idea.";
enum string KEY = "dscanner.suspicious.catch_em_all"; enum string KEY = "dscanner.suspicious.catch_em_all";

View File

@ -6,7 +6,9 @@ module dscanner.analysis.properly_documented_public_functions;
import dparse.lexer; import dparse.lexer;
import dparse.ast; import dparse.ast;
import dparse.formatter : astFmt = format;
import dscanner.analysis.base : BaseAnalyzer; import dscanner.analysis.base : BaseAnalyzer;
import dscanner.utils : safeAccess;
import std.format : format; import std.format : format;
import std.range.primitives; import std.range.primitives;
@ -20,7 +22,7 @@ import std.stdio;
- Ddoc params entries without a parameter trigger warnings as well - Ddoc params entries without a parameter trigger warnings as well
- RETURNS: (except if it's void, only functions) - RETURNS: (except if it's void, only functions)
*/ */
class ProperlyDocumentedPublicFunctions : BaseAnalyzer final class ProperlyDocumentedPublicFunctions : BaseAnalyzer
{ {
enum string MISSING_PARAMS_KEY = "dscanner.style.doc_missing_params"; enum string MISSING_PARAMS_KEY = "dscanner.style.doc_missing_params";
enum string MISSING_PARAMS_MESSAGE = "Parameter %s isn't documented in the `Params` section."; enum string MISSING_PARAMS_MESSAGE = "Parameter %s isn't documented in the `Params` section.";
@ -33,6 +35,9 @@ class ProperlyDocumentedPublicFunctions : BaseAnalyzer
enum string MISSING_RETURNS_KEY = "dscanner.style.doc_missing_returns"; enum string MISSING_RETURNS_KEY = "dscanner.style.doc_missing_returns";
enum string MISSING_RETURNS_MESSAGE = "A public function needs to contain a `Returns` section."; enum string MISSING_RETURNS_MESSAGE = "A public function needs to contain a `Returns` section.";
enum string MISSING_THROW_KEY = "dscanner.style.doc_missing_throw";
enum string MISSING_THROW_MESSAGE = "An instance of `%s` is thrown but not documented in the `Throws` section";
/// ///
this(string fileName, bool skipTests = false) this(string fileName, bool skipTests = false)
{ {
@ -46,6 +51,46 @@ class ProperlyDocumentedPublicFunctions : BaseAnalyzer
postCheckSeenDdocParams(); postCheckSeenDdocParams();
} }
override void visit(const UnaryExpression decl)
{
const IdentifierOrTemplateInstance iot = safeAccess(decl)
.functionCallExpression.unaryExpression.primaryExpression
.identifierOrTemplateInstance;
Type newNamedType(N)(N name)
{
Type t = new Type;
t.type2 = new Type2;
t.type2.typeIdentifierPart = new TypeIdentifierPart;
t.type2.typeIdentifierPart.identifierOrTemplateInstance = new IdentifierOrTemplateInstance;
t.type2.typeIdentifierPart.identifierOrTemplateInstance.identifier = name;
return t;
}
// enforce(condition);
if (iot && iot.identifier.text == "enforce")
{
thrown ~= newNamedType(Token(tok!"identifier", "Exception", 0, 0, 0));
}
else if (iot && iot.templateInstance && iot.templateInstance.identifier.text == "enforce")
{
// enforce!Type(condition);
if (const TemplateSingleArgument tsa = safeAccess(iot.templateInstance)
.templateArguments.templateSingleArgument)
{
thrown ~= newNamedType(tsa.token);
}
// enforce!(Type)(condition);
else if (const TemplateArgumentList tal = safeAccess(iot.templateInstance)
.templateArguments.templateArgumentList)
{
if (tal.items.length && tal.items[0].type)
thrown ~= tal.items[0].type;
}
}
decl.accept(this);
}
override void visit(const Declaration decl) override void visit(const Declaration decl)
{ {
import std.algorithm.searching : any; import std.algorithm.searching : any;
@ -57,6 +102,24 @@ class ProperlyDocumentedPublicFunctions : BaseAnalyzer
tokPackage = tok!"package", tokPackage = tok!"package",
tokPublic = tok!"public"; tokPublic = tok!"public";
// Nested funcs for `Throws`
bool decNestedFunc;
if (decl.functionDeclaration)
{
nestedFuncs++;
decNestedFunc = true;
}
scope(exit)
{
if (decNestedFunc)
nestedFuncs--;
}
if (nestedFuncs > 1)
{
decl.accept(this);
return;
}
if (decl.attributes.length > 0) if (decl.attributes.length > 0)
{ {
const bool isPublic = !decl.attributes.map!`a.attribute`.any!(x => x == tokPrivate || const bool isPublic = !decl.attributes.map!`a.attribute`.any!(x => x == tokPrivate ||
@ -114,20 +177,69 @@ class ProperlyDocumentedPublicFunctions : BaseAnalyzer
override void visit(const FunctionDeclaration decl) override void visit(const FunctionDeclaration decl)
{ {
import std.algorithm.searching : all, any; import std.algorithm.searching : all, any;
import std.array : Appender;
// ignore header declaration for now // ignore header declaration for now
if (decl.functionBody is null) if (decl.functionBody is null)
return; return;
if (nestedFuncs == 1)
thrown.length = 0;
// detect ThrowStatement only if not nothrow
if (!decl.attributes.any!(a => a.attribute.text == "nothrow"))
{
decl.accept(this);
if (nestedFuncs == 1 && !hasThrowSection(decl.comment))
foreach(t; thrown)
{
Appender!(char[]) app;
astFmt(&app, t);
addErrorMessage(decl.name.line, decl.name.column, MISSING_THROW_KEY,
MISSING_THROW_MESSAGE.format(app.data));
}
}
if (nestedFuncs == 1)
{
auto comment = setLastDdocParams(decl.name.line, decl.name.column, decl.comment); auto comment = setLastDdocParams(decl.name.line, decl.name.column, decl.comment);
checkDdocParams(decl.name.line, decl.name.column, decl.parameters, decl.templateParameters); checkDdocParams(decl.name.line, decl.name.column, decl.parameters, decl.templateParameters);
enum voidType = tok!"void"; enum voidType = tok!"void";
if (decl.returnType is null || decl.returnType.type2.builtinType != voidType) if (decl.returnType is null || decl.returnType.type2.builtinType != voidType)
if (!(comment.isDitto || withinTemplate || comment.sections.any!(s => s.name == "Returns"))) if (!(comment.isDitto || withinTemplate || comment.sections.any!(s => s.name == "Returns")))
addErrorMessage(decl.name.line, decl.name.column, MISSING_RETURNS_KEY, MISSING_RETURNS_MESSAGE); addErrorMessage(decl.name.line, decl.name.column,
MISSING_RETURNS_KEY, MISSING_RETURNS_MESSAGE);
}
}
// remove thrown Type that are caught
override void visit(const TryStatement ts)
{
import std.algorithm.iteration : filter;
import std.algorithm.searching : canFind;
import std.array : array;
ts.accept(this);
if (ts.catches)
thrown = thrown.filter!(a => !ts.catches.catches
.canFind!(b => b.type == a))
.array;
}
override void visit(const ThrowStatement ts)
{
import std.algorithm.searching : canFind;
ts.accept(this);
if (ts.expression && ts.expression.items.length == 1)
if (const UnaryExpression ue = cast(UnaryExpression) ts.expression.items[0])
{
if (ue.newExpression && ue.newExpression.type &&
!thrown.canFind!(a => a == ue.newExpression.type))
{
thrown ~= ue.newExpression.type;
}
}
} }
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;
@ -135,6 +247,7 @@ class ProperlyDocumentedPublicFunctions : BaseAnalyzer
private: private:
bool islastSeenVisibilityLabelPublic; bool islastSeenVisibilityLabelPublic;
bool withinTemplate; bool withinTemplate;
size_t nestedFuncs;
static struct Function static struct Function
{ {
@ -145,6 +258,8 @@ private:
} }
Function lastSeenFun; Function lastSeenFun;
const(Type)[] thrown;
// find invalid ddoc parameters (i.e. they don't occur in a function declaration) // find invalid ddoc parameters (i.e. they don't occur in a function declaration)
void postCheckSeenDdocParams() void postCheckSeenDdocParams()
{ {
@ -159,6 +274,15 @@ private:
lastSeenFun.active = false; lastSeenFun.active = false;
} }
bool hasThrowSection(string commentText)
{
import std.algorithm.searching : canFind;
import ddoc.comments : parseComment;
const comment = parseComment(commentText, null);
return comment.isDitto || comment.sections.canFind!(s => s.name == "Throws");
}
auto setLastDdocParams(size_t line, size_t column, string commentText) auto setLastDdocParams(size_t line, size_t column, string commentText)
{ {
import ddoc.comments : parseComment; import ddoc.comments : parseComment;
@ -167,11 +291,14 @@ private:
import std.array : array; import std.array : array;
const comment = parseComment(commentText, null); const comment = parseComment(commentText, null);
if (withinTemplate) { if (withinTemplate)
{
const paramSection = comment.sections.find!(s => s.name == "Params"); const paramSection = comment.sections.find!(s => s.name == "Params");
if (!paramSection.empty) if (!paramSection.empty)
lastSeenFun.ddocParams ~= paramSection[0].mapping.map!(a => a[0]).array; lastSeenFun.ddocParams ~= paramSection[0].mapping.map!(a => a[0]).array;
} else if (!comment.isDitto) { }
else if (!comment.isDitto)
{
// check old function for invalid ddoc params // check old function for invalid ddoc params
if (lastSeenFun.active) if (lastSeenFun.active)
postCheckSeenDdocParams(); postCheckSeenDdocParams();
@ -214,11 +341,17 @@ private:
foreach (p; params.parameters) foreach (p; params.parameters)
{ {
string templateName; string templateName;
if (const t = p.type)
if (const t2 = t.type2) if (auto iot = safeAccess(p).type.type2
if (const tip = t2.typeIdentifierPart) .typeIdentifierPart.identifierOrTemplateInstance.unwrap)
if (const iot = tip.identifierOrTemplateInstance) {
templateName = iot.identifier.text; templateName = iot.identifier.text;
}
else if (auto iot = safeAccess(p).type.type2.type.type2
.typeIdentifierPart.identifierOrTemplateInstance.unwrap)
{
templateName = iot.identifier.text;
}
const idx = tlList.countUntil(templateName); const idx = tlList.countUntil(templateName);
if (idx >= 0) if (idx >= 0)
@ -243,8 +376,8 @@ private:
void checkDdocParams(size_t line, size_t column, const TemplateParameters templateParams) void checkDdocParams(size_t line, size_t column, const TemplateParameters templateParams)
{ {
if (lastSeenFun.active && templateParams !is null &&
if (lastSeenFun.active && templateParams !is null && templateParams.templateParameterList !is null) templateParams.templateParameterList !is null)
checkDdocParams(line, column, templateParams.templateParameterList.items); checkDdocParams(line, column, templateParams.templateParameterList.items);
} }
@ -824,6 +957,233 @@ unittest
}, sac); }, sac);
} }
unittest
{
StaticAnalysisConfig sac = disabledConfig;
sac.properly_documented_public_functions = Check.enabled;
assertAnalyzerWarnings(q{
/++
An awesome description.
Params:
items = things to put.
Returns: Awesome values.
+/
void put(Range)(const(Range) items) if (canPutConstRange!Range)
{}
}, sac);
}
unittest
{
StaticAnalysisConfig sac = disabledConfig;
sac.properly_documented_public_functions = Check.enabled;
assertAnalyzerWarnings(q{
/++
Throw but likely catched.
+/
void bar(){
try{throw new Exception("bla");throw new Error("bla");}
catch(Exception){} catch(Error){}}
}c, sac);
}
unittest
{
StaticAnalysisConfig sac = disabledConfig;
sac.properly_documented_public_functions = Check.enabled;
assertAnalyzerWarnings(q{
/++
Simple case
+/
void bar(){throw new Exception("bla");} // [warn]: %s
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_THROW_MESSAGE.format("Exception")
), sac);
}
unittest
{
StaticAnalysisConfig sac = disabledConfig;
sac.properly_documented_public_functions = Check.enabled;
assertAnalyzerWarnings(q{
/++
Supposed to be documented
Throws: Exception if...
+/
void bar(){throw new Exception("bla");}
}c.format(
), sac);
}
unittest
{
StaticAnalysisConfig sac = disabledConfig;
sac.properly_documented_public_functions = Check.enabled;
assertAnalyzerWarnings(q{
/++
rethrow
+/
void bar(){try throw new Exception("bla"); catch(Exception) throw new Error();} // [warn]: %s
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_THROW_MESSAGE.format("Error")
), sac);
}
unittest
{
StaticAnalysisConfig sac = disabledConfig;
sac.properly_documented_public_functions = Check.enabled;
assertAnalyzerWarnings(q{
/++
trust nothrow before everything
+/
void bar() nothrow {try throw new Exception("bla"); catch(Exception) assert(0);}
}c, sac);
}
unittest
{
StaticAnalysisConfig sac = disabledConfig;
sac.properly_documented_public_functions = Check.enabled;
assertAnalyzerWarnings(q{
/++
case of throw in nested func
+/
void bar() // [warn]: %s
{
void foo(){throw new AssertError("bla");}
foo();
}
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_THROW_MESSAGE.format("AssertError")
), sac);
}
unittest
{
StaticAnalysisConfig sac = disabledConfig;
sac.properly_documented_public_functions = Check.enabled;
assertAnalyzerWarnings(q{
/++
case of throw in nested func but caught
+/
void bar()
{
void foo(){throw new AssertError("bla");}
try foo();
catch (AssertError){}
}
}c, sac);
}
unittest
{
StaticAnalysisConfig sac = disabledConfig;
sac.properly_documented_public_functions = Check.enabled;
assertAnalyzerWarnings(q{
/++
case of double throw in nested func but only 1 caught
+/
void bar() // [warn]: %s
{
void foo(){throw new AssertError("bla");throw new Error("bla");}
try foo();
catch (Error){}
}
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_THROW_MESSAGE.format("AssertError")
), sac);
}
unittest
{
StaticAnalysisConfig sac = disabledConfig;
sac.properly_documented_public_functions = Check.enabled;
assertAnalyzerWarnings(q{
/++
enforce
+/
void bar() // [warn]: %s
{
enforce(condition);
}
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_THROW_MESSAGE.format("Exception")
), sac);
}
unittest
{
StaticAnalysisConfig sac = disabledConfig;
sac.properly_documented_public_functions = Check.enabled;
assertAnalyzerWarnings(q{
/++
enforce
+/
void bar() // [warn]: %s
{
enforce!AssertError(condition);
}
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_THROW_MESSAGE.format("AssertError")
), sac);
}
unittest
{
StaticAnalysisConfig sac = disabledConfig;
sac.properly_documented_public_functions = Check.enabled;
assertAnalyzerWarnings(q{
/++
enforce
+/
void bar() // [warn]: %s
{
enforce!(AssertError)(condition);
}
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_THROW_MESSAGE.format("AssertError")
), sac);
}
unittest
{
StaticAnalysisConfig sac = disabledConfig;
sac.properly_documented_public_functions = Check.enabled;
assertAnalyzerWarnings(q{
/++
enforce
+/
void foo() // [warn]: %s
{
void bar()
{
enforce!AssertError(condition);
}
bar();
}
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_THROW_MESSAGE.format("AssertError")
), sac);
}
// https://github.com/dlang-community/D-Scanner/issues/583 // https://github.com/dlang-community/D-Scanner/issues/583
unittest unittest
{ {

View File

@ -16,7 +16,7 @@ import dsymbol.scope_ : Scope;
* Checks for .. expressions where the left side is larger than the right. This * Checks for .. expressions where the left side is larger than the right. This
* is almost always a mistake. * is almost always a mistake.
*/ */
class BackwardsRangeCheck : BaseAnalyzer final class BackwardsRangeCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -17,7 +17,7 @@ import std.range : empty, front, walkLength;
/** /**
* Checks for redundant attributes. At the moment only visibility attributes. * Checks for redundant attributes. At the moment only visibility attributes.
*/ */
class RedundantAttributesCheck : BaseAnalyzer final class RedundantAttributesCheck : BaseAnalyzer
{ {
this(string fileName, const(Scope)* sc, bool skipTests = false) this(string fileName, const(Scope)* sc, bool skipTests = false)
{ {

View File

@ -13,7 +13,7 @@ import dsymbol.scope_ : Scope;
/** /**
* Checks for redundant parenthesis * Checks for redundant parenthesis
*/ */
class RedundantParenCheck : BaseAnalyzer final class RedundantParenCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -16,7 +16,7 @@ import dsymbol.scope_ : Scope;
/** /**
* Checks for redundant storage classes such immutable and __gshared, static and __gshared * Checks for redundant storage classes such immutable and __gshared, static and __gshared
*/ */
class RedundantStorageClassCheck : BaseAnalyzer final class RedundantStorageClassCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;
enum string REDUNDANT_VARIABLE_ATTRIBUTES = "Variable declaration for `%s` has redundant attributes (%-(`%s`%|, %))."; enum string REDUNDANT_VARIABLE_ATTRIBUTES = "Variable declaration for `%s` has redundant attributes (%-(`%s`%|, %)).";

View File

@ -22,7 +22,7 @@ import dscanner.utils : safeAccess;
* *
* However, it's more likely that this is a mistake. * However, it's more likely that this is a mistake.
*/ */
class StaticIfElse : BaseAnalyzer final class StaticIfElse : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -9,7 +9,7 @@ import dparse.ast;
import dparse.lexer; import dparse.lexer;
import dscanner.analysis.base; import dscanner.analysis.base;
class StatsCollector : BaseAnalyzer final class StatsCollector : BaseAnalyzer
{ {
alias visit = ASTVisitor.visit; alias visit = ASTVisitor.visit;

View File

@ -14,12 +14,12 @@ import dsymbol.scope_;
/** /**
* Checks that `@trusted` is only applied to a a single function * Checks that `@trusted` is only applied to a a single function
*/ */
class TrustTooMuchCheck : BaseAnalyzer final class TrustTooMuchCheck : BaseAnalyzer
{ {
private: private:
static immutable MESSAGE = "Trusting a whole scope is a bad idea, " ~ static immutable MESSAGE = "Trusting a whole scope is a bad idea, " ~
"`@trusted` should only be attached to a single function"; "`@trusted` should only be attached to the functions individually";
static immutable string KEY = "dscanner.trust_too_much"; static immutable string KEY = "dscanner.trust_too_much";
bool checkAtAttribute = true; bool checkAtAttribute = true;

View File

@ -17,7 +17,7 @@ import std.stdio;
* Checks for undocumented public declarations. Ignores some operator overloads, * Checks for undocumented public declarations. Ignores some operator overloads,
* main functions, and functions whose name starts with "get" or "set". * main functions, and functions whose name starts with "get" or "set".
*/ */
class UndocumentedDeclarationCheck : BaseAnalyzer final class UndocumentedDeclarationCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -14,7 +14,7 @@ import dparse.lexer;
/** /**
* Checks for variables that could have been declared const or immutable * Checks for variables that could have been declared const or immutable
*/ */
class UnmodifiedFinder : BaseAnalyzer final class UnmodifiedFinder : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;
@ -138,6 +138,7 @@ class UnmodifiedFinder : BaseAnalyzer
mixin PartsMightModify!AsmPrimaryExp; mixin PartsMightModify!AsmPrimaryExp;
mixin PartsMightModify!IndexExpression; mixin PartsMightModify!IndexExpression;
mixin PartsMightModify!FunctionCallExpression; mixin PartsMightModify!FunctionCallExpression;
mixin PartsMightModify!NewExpression;
mixin PartsMightModify!IdentifierOrTemplateChain; mixin PartsMightModify!IdentifierOrTemplateChain;
mixin PartsMightModify!ReturnStatement; mixin PartsMightModify!ReturnStatement;
@ -368,4 +369,15 @@ bool isValueTypeSimple(const Type type) pure nothrow @nogc
void foo(){auto e = new E;} void foo(){auto e = new E;}
}, sac); }, sac);
assertAnalyzerWarnings(q{
void issue640()
{
size_t i1;
new Foo(i1);
size_t i2;
foo(i2);
} }
}, sac);
}

View File

@ -16,7 +16,7 @@ import std.algorithm : all;
/** /**
* Checks for unused variables. * Checks for unused variables.
*/ */
class UnusedVariableCheck : BaseAnalyzer final class UnusedVariableCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;
@ -311,7 +311,7 @@ class UnusedVariableCheck : BaseAnalyzer
interestDepth++; interestDepth++;
withStatetement.expression.accept(this); withStatetement.expression.accept(this);
interestDepth--; interestDepth--;
withStatetement.statementNoCaseNoDefault.accept(this); withStatetement.declarationOrStatement.accept(this);
} }
override void visit(const Parameter parameter) override void visit(const Parameter parameter)

View File

@ -23,7 +23,7 @@ auto filterChars(string chars, S)(S str)
/** /**
* Checks for asserts that always succeed * Checks for asserts that always succeed
*/ */
class UselessAssertCheck : BaseAnalyzer final class UselessAssertCheck : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;

View File

@ -7,10 +7,7 @@ module dscanner.analysis.vcall_in_ctor;
import dscanner.analysis.base; import dscanner.analysis.base;
import dscanner.utils; import dscanner.utils;
import dparse.ast, dparse.lexer; import dparse.ast, dparse.lexer;
import std.algorithm: among; import std.algorithm.searching : canFind;
import std.algorithm.iteration : filter;
import std.algorithm.searching : find;
import std.range.primitives : empty;
import std.range: retro; import std.range: retro;
/** /**
@ -19,7 +16,7 @@ import std.range: retro;
* When not used carefully, virtual calls from constructors can lead to a call * When not used carefully, virtual calls from constructors can lead to a call
* in a derived instance that's not yet constructed. * in a derived instance that's not yet constructed.
*/ */
class VcallCtorChecker : BaseAnalyzer final class VcallCtorChecker : BaseAnalyzer
{ {
alias visit = BaseAnalyzer.visit; alias visit = BaseAnalyzer.visit;
@ -183,16 +180,21 @@ public:
bool pop; bool pop;
scope(exit) if (pop) scope(exit) if (pop)
popVirtual; popVirtual;
const bool hasAttribs = d.attributes !is null;
const bool hasStatic = hasAttribs ? d.attributes.canFind!(a => a.attribute.type == tok!"static") : false;
const bool hasFinal = hasAttribs ? d.attributes.canFind!(a => a.attribute.type == tok!"final") : false;
if (d.attributes) foreach (attr; d.attributes.retro) if (d.attributes) foreach (attr; d.attributes.retro)
{ {
if (attr.attribute == tok!"public" || attr.attribute == tok!"protected" || if (!hasStatic &&
attr.attribute == tok!"package") (attr.attribute == tok!"public" || attr.attribute == tok!"protected"))
{ {
pushVirtual(true); pushVirtual(true);
pop = true; pop = true;
break; break;
} }
else if (attr.attribute == tok!"private") else if (hasStatic || attr.attribute == tok!"private" || attr.attribute == tok!"package")
{ {
pushVirtual(false); pushVirtual(false);
pop = true; pop = true;
@ -201,13 +203,12 @@ public:
} }
// final class... final function // final class... final function
const bool pf = !d.attributes.find!(a => a.attribute.type == tok!"final").empty; if ((d.classDeclaration || d.functionDeclaration) && hasFinal)
if ((d.classDeclaration || d.functionDeclaration) && pf)
pushIsFinal(true); pushIsFinal(true);
d.accept(this); d.accept(this);
if ((d.classDeclaration || d.functionDeclaration) && pf) if ((d.classDeclaration || d.functionDeclaration) && hasFinal)
popIsFinal; popIsFinal;
} }
@ -242,11 +243,14 @@ public:
bool virtualOnce; bool virtualOnce;
bool notVirtualOnce; bool notVirtualOnce;
const bool hasAttribs = d.attributes !is null;
const bool hasStatic = hasAttribs ? d.attributes.canFind!(a => a.attribute.type == tok!"static") : false;
// handle "private", "public"... for this declaration // handle "private", "public"... for this declaration
if (d.attributes) foreach (attr; d.attributes.retro) if (d.attributes) foreach (attr; d.attributes.retro)
{ {
if (attr.attribute == tok!"public" || attr.attribute == tok!"protected" || if (!hasStatic &&
attr.attribute == tok!"package") (attr.attribute == tok!"public" || attr.attribute == tok!"protected"))
{ {
if (!isVirtual) if (!isVirtual)
{ {
@ -254,7 +258,7 @@ public:
break; break;
} }
} }
else if (attr.attribute == tok!"private") else if (hasStatic || attr.attribute == tok!"private" || attr.attribute == tok!"package")
{ {
if (isVirtual) if (isVirtual)
{ {
@ -382,6 +386,22 @@ unittest
} }
}, sac); }, sac);
assertAnalyzerWarnings(q{
class Foo
{
static void nonVirtual();
this(){nonVirtual();}
}
}, sac);
assertAnalyzerWarnings(q{
class Foo
{
package void nonVirtual();
this(){nonVirtual();}
}
}, sac);
import std.stdio: writeln; import std.stdio: writeln;
writeln("Unittest for VcallCtorChecker passed"); writeln("Unittest for VcallCtorChecker passed");
} }

View File

@ -8,18 +8,20 @@ module dscanner.dscanner_version;
/** /**
* Human-readable version number * Human-readable version number
*/ */
enum DSCANNER_VERSION = "v0.4.0"; enum DEFAUULT_DSCANNER_VERSION = "v0.5.5";
version (Windows) version (built_with_dub)
{ {
enum DSCANNER_VERSION = import("dubhash.txt");
} }
else version (built_with_dub) else version (Windows)
{ {
enum DSCANNER_VERSION = DEFAUULT_DSCANNER_VERSION;
} }
else else
{ {
/** /**
* Current build's Git commit hash * Current build's Git commit hash
*/ */
enum GIT_HASH = import("githash.txt"); enum DSCANNER_VERSION = import("githash.txt");
} }

View File

@ -139,12 +139,7 @@ else
if (printVersion) if (printVersion)
{ {
version (Windows) write(DSCANNER_VERSION);
writeln(DSCANNER_VERSION);
else version (built_with_dub)
writeln(DSCANNER_VERSION);
else
write(DSCANNER_VERSION, " ", GIT_HASH);
return 0; return 0;
} }

@ -1 +1 @@
Subproject commit 7487970b58f4a2c0d495679329a8a2857111f3fd Subproject commit b7778fd6bf5f9aaaa87dd27f989cefbf9b3b365f