diff --git a/.gitignore b/.gitignore index 1d29087..7ea0dc2 100755 --- a/.gitignore +++ b/.gitignore @@ -31,6 +31,7 @@ dsc # Git hash githash.txt +githash_.txt # GDB history .gdb_history diff --git a/build.bat b/build.bat index abe613a..c51e135 100644 --- a/build.bat +++ b/build.bat @@ -4,7 +4,19 @@ setlocal enabledelayedexpansion if "%DC%"=="" set DC="dmd" if "%DC%"=="ldc2" set DC="ldmd2" -set DFLAGS=-O -release -inline -version=StdLoggerDisableWarning +:: git might not be installed, so we provide 0.0.0 as a fallback or use +:: the existing githash file if existent +git describe --tags > githash_.txt +for /f %%i in ("githash_.txt") do set githashsize=%%~zi +if %githashsize% == 0 ( + if not exist "githash.txt" ( + echo v0.0.0 > githash.txt + ) +) else ( + move /y githash_.txt githash.txt +) + +set DFLAGS=-O -release -inline -version=StdLoggerDisableWarning -J. set TESTFLAGS=-g -w -version=StdLoggerDisableWarning set CORE= set LIBDPARSE= diff --git a/dsymbol b/dsymbol index e018446..3f7eaa1 160000 --- a/dsymbol +++ b/dsymbol @@ -1 +1 @@ -Subproject commit e018446b028b92c34398032e698c05c0919630ee +Subproject commit 3f7eaa1b1e6dd0c5e14d500d0b3d4ba1e1e41138 diff --git a/dub.json b/dub.json index 46296ad..2328140 100644 --- a/dub.json +++ b/dub.json @@ -12,8 +12,8 @@ "StdLoggerDisableWarning" ], "dependencies" : { - "libdparse" : "~>0.8.6", - "dsymbol" : "~>0.3.7", + "libdparse" : "~>0.8.7", + "dsymbol" : "~>0.3.10", "inifiled" : "~>1.3.1", "emsi_containers" : "~>0.8.0-alpha.7", "libddoc" : "~>0.3.0-beta.1", @@ -24,6 +24,6 @@ "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\\\"));\"" + "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.strip : \\\"v\\\" ~ dir.dirName.baseName.findSplitAfter(environment.get(\\\"DUB_ROOT_PACKAGE\\\")~\\\"-\\\")[1]).ifThrown(\\\"0.0.0\\\").chain(newline).toFile(dir.buildPath(\\\"bin\\\", \\\"dubhash.txt\\\"));\"" ] } diff --git a/libdparse b/libdparse index 41c5674..086cf06 160000 --- a/libdparse +++ b/libdparse @@ -1 +1 @@ -Subproject commit 41c567460dee3fab9db0c0f7775534b1d70f1f27 +Subproject commit 086cf06051bb1f33c94891ba6c39a57f164ee296 diff --git a/release-windows.sh b/release-windows.sh index cd493a9..a6e4c47 100755 --- a/release-windows.sh +++ b/release-windows.sh @@ -19,6 +19,7 @@ fi archiveName="dscanner-$VERSION-$OS-$ARCH_SUFFIX.zip" echo "Building $archiveName" mkdir -p bin +git describe --tags > githash.txt # no git installed under Wine DC="$DIR/dmd2/windows/bin/dmd.exe" wine cmd /C build.bat cd bin diff --git a/src/dscanner/analysis/auto_function.d b/src/dscanner/analysis/auto_function.d index 25bb1bb..53aeac0 100644 --- a/src/dscanner/analysis/auto_function.d +++ b/src/dscanner/analysis/auto_function.d @@ -49,7 +49,7 @@ public: _returns[$-1] = false; const bool autoFun = decl.storageClasses - .any!(a => a.token.type == tok!"auto"); + .any!(a => a.token.type == tok!"auto" || a.atAttribute !is null); decl.accept(this); @@ -216,6 +216,16 @@ unittest AutoFunctionChecker.MESSAGE, ), sac); + assertAnalyzerWarnings(q{ + @property doStuff(){} // [warn]: %s + @safe doStuff(){} // [warn]: %s + @disable doStuff(); + @safe void doStuff(); + }c.format( + AutoFunctionChecker.MESSAGE, + AutoFunctionChecker.MESSAGE, + ), sac); + assertAnalyzerWarnings(q{ enum _genSave = "return true;"; auto doStuff(){ mixin(_genSave);} diff --git a/src/dscanner/analysis/ifelsesame.d b/src/dscanner/analysis/ifelsesame.d index 57f26f9..0247816 100644 --- a/src/dscanner/analysis/ifelsesame.d +++ b/src/dscanner/analysis/ifelsesame.d @@ -31,7 +31,7 @@ final class IfElseSameCheck : BaseAnalyzer override void visit(const IfStatement ifStatement) { - if (ifStatement.thenStatement == ifStatement.elseStatement) + if (ifStatement.thenStatement && (ifStatement.thenStatement == ifStatement.elseStatement)) addErrorMessage(ifStatement.line, ifStatement.column, "dscanner.bugs.if_else_same", "'Else' branch is identical to 'Then' branch."); ifStatement.accept(this); @@ -95,5 +95,13 @@ unittest person = "bobby"; // not same } }c, sac); + + assertAnalyzerWarnings(q{ + void foo() + { + if (auto stuff = call()) + } + }c, sac); + stderr.writeln("Unittest for IfElseSameCheck passed."); } diff --git a/src/dscanner/analysis/incorrect_infinite_range.d b/src/dscanner/analysis/incorrect_infinite_range.d index 499af70..a92c1d2 100644 --- a/src/dscanner/analysis/incorrect_infinite_range.d +++ b/src/dscanner/analysis/incorrect_infinite_range.d @@ -62,7 +62,9 @@ final class IncorrectInfiniteRangeCheck : BaseAnalyzer override void visit(const ReturnStatement rs) { - if (rs.expression.items.length != 1) + if (inStruct == 0 || line == size_t.max) // not within a struct yet + return; + if (!rs.expression || rs.expression.items.length != 1) return; UnaryExpression unary = cast(UnaryExpression) rs.expression.items[0]; if (unary is null) @@ -82,7 +84,7 @@ private: uint inStruct; enum string KEY = "dscanner.suspicious.incorrect_infinite_range"; enum string MESSAGE = "Use `enum bool empty = false;` to define an infinite range."; - size_t line; + size_t line = size_t.max; size_t column; } @@ -123,6 +125,34 @@ class C { bool empty() { return false; } } // [warn]: %1$s }c .format(IncorrectInfiniteRangeCheck.MESSAGE), sac); +} +// test for https://github.com/dlang-community/D-Scanner/issues/656 +// unittests are skipped but D-Scanner sources are self checked +version(none) struct Foo +{ + void empty() + { + return; + } +} + +unittest +{ + import std.stdio : stderr; + import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig; + import std.format : format; + + StaticAnalysisConfig sac = disabledConfig(); + sac.incorrect_infinite_range_check = Check.enabled; + assertAnalyzerWarnings(q{ + enum isAllZeroBits = () + { + if (true) + return true; + else + return false; + }(); + }, sac); stderr.writeln("Unittest for IncorrectInfiniteRangeCheck passed."); } diff --git a/src/dscanner/analysis/objectconst.d b/src/dscanner/analysis/objectconst.d index c20bc5c..c089e76 100644 --- a/src/dscanner/analysis/objectconst.d +++ b/src/dscanner/analysis/objectconst.d @@ -51,13 +51,25 @@ final class ObjectConstCheck : BaseAnalyzer constBlock = true; } - if (inAggregate && d.functionDeclaration !is null && !constColon && !constBlock - && isInteresting(d.functionDeclaration.name.text) - && !hasConst(d.functionDeclaration.memberFunctionAttributes)) + bool containsDisable(A)(const A[] attribs) { - addErrorMessage(d.functionDeclaration.name.line, - d.functionDeclaration.name.column, "dscanner.suspicious.object_const", - "Methods 'opCmp', 'toHash', 'opEquals', 'opCast', and/or 'toString' are non-const."); + import std.algorithm.searching : canFind; + return attribs.canFind!(a => a.atAttribute !is null && + a.atAttribute.identifier.text == "disable"); + } + + if (const FunctionDeclaration fd = d.functionDeclaration) + { + const isDeclationDisabled = containsDisable(d.attributes) || + containsDisable(fd.memberFunctionAttributes); + + if (inAggregate && !constColon && !constBlock && !isDeclationDisabled + && isInteresting(fd.name.text) && !hasConst(fd.memberFunctionAttributes)) + { + addErrorMessage(d.functionDeclaration.name.line, + d.functionDeclaration.name.column, "dscanner.suspicious.object_const", + "Methods 'opCmp', 'toHash', 'opEquals', 'opCast', and/or 'toString' are non-const."); + } } d.accept(this); @@ -130,6 +142,16 @@ unittest const{ override string toString() { return "foo"; }} // ok } + class Rat + { + bool opEquals(Object a, Object b) @disable; // ok + } + + class Ant + { + @disable bool opEquals(Object a, Object b); // ok + } + // Will warn, because none are const class Dog { diff --git a/src/dscanner/analysis/opequals_without_tohash.d b/src/dscanner/analysis/opequals_without_tohash.d index a1b4e84..6517ccd 100644 --- a/src/dscanner/analysis/opequals_without_tohash.d +++ b/src/dscanner/analysis/opequals_without_tohash.d @@ -54,6 +54,19 @@ final class OpEqualsWithoutToHashCheck : BaseAnalyzer if (!declaration || !declaration.functionDeclaration) continue; + bool containsDisable(A)(const A[] attribs) + { + import std.algorithm.searching : canFind; + return attribs.canFind!(a => a.atAttribute !is null && + a.atAttribute.identifier.text == "disable"); + } + + const isDeclationDisabled = containsDisable(declaration.attributes) || + containsDisable(declaration.functionDeclaration.memberFunctionAttributes); + + if (isDeclationDisabled) + continue; + // Check if opEquals or toHash immutable string methodName = declaration.functionDeclaration.name.text; if (methodName == "opEquals") @@ -146,6 +159,13 @@ unittest return 0; } } + + // issue #659, do not warn if one miss and the other is not callable + struct Fox {const nothrow @safe hash_t toHash() @disable;} + struct Bat {@disable const nothrow @safe hash_t toHash();} + struct Rat {const bool opEquals(Object a, Object b) @disable;} + struct Cat {@disable const bool opEquals(Object a, Object b);} + }c, sac); stderr.writeln("Unittest for OpEqualsWithoutToHashCheck passed."); diff --git a/src/dscanner/analysis/trust_too_much.d b/src/dscanner/analysis/trust_too_much.d index 8d7d826..ee3850b 100644 --- a/src/dscanner/analysis/trust_too_much.d +++ b/src/dscanner/analysis/trust_too_much.d @@ -57,7 +57,11 @@ public: override void visit(const Declaration d) { const oldCheckAtAttribute = checkAtAttribute; - checkAtAttribute = d.functionDeclaration is null; + + checkAtAttribute = d.functionDeclaration is null && d.unittest_ is null && + d.constructor is null && d.destructor is null && + d.staticConstructor is null && d.staticDestructor is null && + d.sharedStaticConstructor is null && d.sharedStaticDestructor is null; d.accept(this); checkAtAttribute = oldCheckAtAttribute; } @@ -145,5 +149,10 @@ unittest alias nothrow @trusted uint F4(); }c , sac); + assertAnalyzerWarnings(q{ + @trusted ~this(); + @trusted this(); + }c , sac); + stderr.writeln("Unittest for TrustTooMuchCheck passed."); } diff --git a/src/dscanner/analysis/useless_initializer.d b/src/dscanner/analysis/useless_initializer.d index ccc9077..f3f2af1 100644 --- a/src/dscanner/analysis/useless_initializer.d +++ b/src/dscanner/analysis/useless_initializer.d @@ -39,7 +39,7 @@ private: } else { - enum msg = `Variable %s initializer is useless because it does not differ from the default value`; + enum msg = "Variable `%s` initializer is useless because it does not differ from the default value"; } static immutable intDefs = ["0", "0L", "0UL", "0uL", "0U", "0x0", "0b0"]; diff --git a/src/dscanner/dscanner_version.d b/src/dscanner/dscanner_version.d index eb7611a..0509eef 100644 --- a/src/dscanner/dscanner_version.d +++ b/src/dscanner/dscanner_version.d @@ -8,16 +8,11 @@ module dscanner.dscanner_version; /** * Human-readable version number */ -enum DEFAUULT_DSCANNER_VERSION = "v0.5.5"; version (built_with_dub) { enum DSCANNER_VERSION = import("dubhash.txt"); } -else version (Windows) -{ - enum DSCANNER_VERSION = DEFAUULT_DSCANNER_VERSION; -} else { /**