From 7f38c87f5d7220e7bcb31b042d82ee0ccf18b599 Mon Sep 17 00:00:00 2001 From: Sebastian Wilzbach Date: Thu, 15 Dec 2016 17:29:20 +0100 Subject: [PATCH 1/2] Add check for explictly annotated unittests --- src/analysis/config.d | 3 + src/analysis/explicitly_annotated_unittests.d | 116 ++++++++++++++++++ src/analysis/run.d | 5 + 3 files changed, 124 insertions(+) create mode 100644 src/analysis/explicitly_annotated_unittests.d diff --git a/src/analysis/config.d b/src/analysis/config.d index 8fd1eee..bc86450 100644 --- a/src/analysis/config.d +++ b/src/analysis/config.d @@ -150,4 +150,7 @@ struct StaticAnalysisConfig @INI("Check for sortedness of imports") string imports_sortedness = Check.disabled; + + @INI("Check for explicitly annotated unittests") + string explicitly_annotated_unittests = Check.disabled; } diff --git a/src/analysis/explicitly_annotated_unittests.d b/src/analysis/explicitly_annotated_unittests.d new file mode 100644 index 0000000..2911e26 --- /dev/null +++ b/src/analysis/explicitly_annotated_unittests.d @@ -0,0 +1,116 @@ +// 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.explicitly_annotated_unittests; + +import dparse.lexer; +import dparse.ast; +import analysis.base : BaseAnalyzer; + +import std.stdio; + +/** + * Requires unittests to be explicitly annotated with either @safe or @system + */ +class ExplicitlyAnnotatedUnittestCheck : BaseAnalyzer +{ + enum string KEY = "dscanner.style.explicitly_annotated_unittest"; + enum string MESSAGE = "A unittest should be annotated with at least @safe or @system"; + + /// + this(string fileName, bool skipTests = false) + { + super(fileName, null, skipTests); + } + + override void visit(const Declaration decl) + { + if (decl.unittest_ !is null) + { + bool isSafeOrSystem = false; + if (decl.attributes !is null) + foreach (attribute; decl.attributes) + { + if (attribute.atAttribute !is null) + { + auto token = attribute.atAttribute.identifier.text; + if (token == "safe" || token == "system") + { + isSafeOrSystem = true; + break; + } + } + } + if (!isSafeOrSystem) + addErrorMessage(decl.unittest_.line, decl.unittest_.column, KEY, MESSAGE); + } + decl.accept(this); + } + + alias visit = BaseAnalyzer.visit; + +} + +unittest +{ + import std.stdio : stderr; + import std.format : format; + import analysis.config : StaticAnalysisConfig, Check; + import analysis.helpers : assertAnalyzerWarnings; + + StaticAnalysisConfig sac; + sac.explicitly_annotated_unittests = Check.enabled; + + assertAnalyzerWarnings(q{ + @safe unittest { + + } + + @system unittest { + + } + + pure nothrow @system @nogc unittest { + + } + + unittest // [warn]: %s + { + + } + pure nothrow @nogc unittest // [warn]: %s + { + } + }c.format( + ExplicitlyAnnotatedUnittestCheck.MESSAGE, + ExplicitlyAnnotatedUnittestCheck.MESSAGE, + ), sac); + + // nested + assertAnalyzerWarnings(q{ + struct Foo + { + @safe unittest { + + } + + @system unittest { + + } + + unittest // [warn]: %s + { + + } + pure nothrow @nogc unittest // [warn]: %s + { + } + } + }c.format( + ExplicitlyAnnotatedUnittestCheck.MESSAGE, + ExplicitlyAnnotatedUnittestCheck.MESSAGE, + ), sac); + + stderr.writeln("Unittest for ExplicitlyAnnotatedUnittestCheck passed."); +} diff --git a/src/analysis/run.d b/src/analysis/run.d index 3222f05..965ea3b 100644 --- a/src/analysis/run.d +++ b/src/analysis/run.d @@ -61,6 +61,7 @@ import analysis.static_if_else; import analysis.lambda_return_check; import analysis.auto_function; import analysis.imports_sortedness; +import analysis.explicitly_annotated_unittests; import dsymbol.string_interning : internString; import dsymbol.scope_; @@ -364,6 +365,10 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a checks ~= new ImportSortednessCheck(fileName, analysisConfig.imports_sortedness == Check.skipTests && !ut); + if (analysisConfig.explicitly_annotated_unittests != Check.disabled) + checks ~= new ExplicitlyAnnotatedUnittestCheck(fileName, + analysisConfig.explicitly_annotated_unittests == Check.skipTests && !ut); + version (none) if (analysisConfig.redundant_if_check != Check.disabled) checks ~= new IfStatementCheck(fileName, moduleScope, From 1c89a01f33ebea40697b955a64c43c99f80b9c47 Mon Sep 17 00:00:00 2001 From: Sebastian Wilzbach Date: Fri, 16 Dec 2016 00:52:17 +0100 Subject: [PATCH 2/2] Address @BBasile's review --- src/analysis/explicitly_annotated_unittests.d | 45 +++++-------------- 1 file changed, 11 insertions(+), 34 deletions(-) diff --git a/src/analysis/explicitly_annotated_unittests.d b/src/analysis/explicitly_annotated_unittests.d index 2911e26..1fde833 100644 --- a/src/analysis/explicitly_annotated_unittests.d +++ b/src/analysis/explicitly_annotated_unittests.d @@ -28,13 +28,13 @@ class ExplicitlyAnnotatedUnittestCheck : BaseAnalyzer { if (decl.unittest_ !is null) { - bool isSafeOrSystem = false; + bool isSafeOrSystem; if (decl.attributes !is null) foreach (attribute; decl.attributes) { if (attribute.atAttribute !is null) { - auto token = attribute.atAttribute.identifier.text; + const token = attribute.atAttribute.identifier.text; if (token == "safe" || token == "system") { isSafeOrSystem = true; @@ -63,25 +63,12 @@ unittest sac.explicitly_annotated_unittests = Check.enabled; assertAnalyzerWarnings(q{ - @safe unittest { + @safe unittest {} + @system unittest {} + pure nothrow @system @nogc unittest {} - } - - @system unittest { - - } - - pure nothrow @system @nogc unittest { - - } - - unittest // [warn]: %s - { - - } - pure nothrow @nogc unittest // [warn]: %s - { - } + unittest {} // [warn]: %s + pure nothrow @nogc unittest {} // [warn]: %s }c.format( ExplicitlyAnnotatedUnittestCheck.MESSAGE, ExplicitlyAnnotatedUnittestCheck.MESSAGE, @@ -91,21 +78,11 @@ unittest assertAnalyzerWarnings(q{ struct Foo { - @safe unittest { + @safe unittest {} + @system unittest {} - } - - @system unittest { - - } - - unittest // [warn]: %s - { - - } - pure nothrow @nogc unittest // [warn]: %s - { - } + unittest {} // [warn]: %s + pure nothrow @nogc unittest {} // [warn]: %s } }c.format( ExplicitlyAnnotatedUnittestCheck.MESSAGE,