From 8231a0d1b84e9a0b2b44340d35d09dcee013d0c4 Mon Sep 17 00:00:00 2001 From: Matthew Brennan Jones Date: Fri, 16 May 2014 18:47:13 -0700 Subject: [PATCH 1/2] Added basic unit tests to some analyzers. --- analysis/constructors.d | 23 +++++++ analysis/del.d | 19 ++++++ analysis/fish.d | 24 +++++++ analysis/helpers.d | 141 ++++++++++++++++++++++++++++++++++++++++ analysis/ifelsesame.d | 32 +++++++++ analysis/linespan.d | 5 +- analysis/numbers.d | 21 ++++++ analysis/objectconst.d | 61 +++++++++++++++++ analysis/pokemon.d | 34 ++++++++++ analysis/range.d | 21 ++++++ analysis/run.d | 132 ++++++++++++++++++++++--------------- analysis/style.d | 31 +++++++-- main.d | 14 +++- makefile | 9 ++- std/allocator.d | 15 +++-- std/d/parser.d | 14 +++- test.sh | 22 +++++++ 17 files changed, 551 insertions(+), 67 deletions(-) create mode 100644 analysis/helpers.d create mode 100755 test.sh diff --git a/analysis/constructors.d b/analysis/constructors.d index ed4dd76..39c5125 100644 --- a/analysis/constructors.d +++ b/analysis/constructors.d @@ -2,7 +2,10 @@ module analysis.constructors; import std.d.ast; import std.d.lexer; +import std.stdio; import analysis.base; +import analysis.helpers; + class ConstructorCheck : BaseAnalyzer { @@ -84,3 +87,23 @@ private: bool hasNoArgConstructor; bool hasDefaultArgConstructor; } + +unittest +{ + shouldWarn(q{ + class Cat // [warn]: This class has a zero-argument constructor as well as a constructor with one default argument. This can be confusing + { + this() {} + this(string name = "kittie") {} + } + + struct Dog + { + this() {} + this(string name = "doggie") {} // [warn]: This struct constructor can never be called with its default argument. + } + }c, analysis.run.AnalyzerCheck.constructor_check); + + stderr.writeln("Unittest for ConstructorCheck passed."); +} + diff --git a/analysis/del.d b/analysis/del.d index 5ef4472..5d48152 100644 --- a/analysis/del.d +++ b/analysis/del.d @@ -5,9 +5,11 @@ module analysis.del; +import std.stdio; import std.d.ast; import std.d.lexer; import analysis.base; +import analysis.helpers; /** * Checks for use of the deprecated "delete" keyword @@ -27,3 +29,20 @@ class DeleteCheck : BaseAnalyzer d.accept(this); } } + +unittest +{ + shouldWarn(q{ + void testDelete() + { + int[int] data = [1 : 2]; + delete data[1]; // [warn]: Avoid using the delete keyword + + auto a = new Class(); + delete a; // [warn]: Avoid using the delete keyword + } + }c, analysis.run.AnalyzerCheck.delete_check); + + stderr.writeln("Unittest for DeleteCheck passed."); +} + diff --git a/analysis/fish.d b/analysis/fish.d index 71309d7..d46d54f 100644 --- a/analysis/fish.d +++ b/analysis/fish.d @@ -5,9 +5,11 @@ module analysis.fish; +import std.stdio; import std.d.ast; import std.d.lexer; import analysis.base; +import analysis.helpers; /** * Checks for use of the deprecated floating point comparison operators. @@ -37,3 +39,25 @@ class FloatOperatorCheck : BaseAnalyzer r.accept(this); } } + +unittest +{ + shouldWarn(q{ + void testFish() + { + float z = 1.5f; + bool a; + a = z !<>= z; // [warn]: Avoid using the deprecated floating-point operators + a = z !<> z; // [warn]: Avoid using the deprecated floating-point operators + a = z <> z; // [warn]: Avoid using the deprecated floating-point operators + a = z <>= z; // [warn]: Avoid using the deprecated floating-point operators + a = z !> z; // [warn]: Avoid using the deprecated floating-point operators + a = z !>= z; // [warn]: Avoid using the deprecated floating-point operators + a = z !< z; // [warn]: Avoid using the deprecated floating-point operators + a = z !<= z; // [warn]: Avoid using the deprecated floating-point operators + } + }c, analysis.run.AnalyzerCheck.float_operator_check); + + stderr.writeln("Unittest for FloatOperatorCheck passed."); +} + diff --git a/analysis/helpers.d b/analysis/helpers.d new file mode 100644 index 0000000..58bdb44 --- /dev/null +++ b/analysis/helpers.d @@ -0,0 +1,141 @@ +// Copyright (c) 2014, Matthew Brennan Jones +// 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.helpers; + +import std.string; +import std.traits; + +import std.d.ast; + + +S between(S)(S value, S before, S after) +if (isSomeString!S) { + return value.after(before).before(after); +} + +S before(S)(S value, S separator) +if (isSomeString!S) { + auto i = indexOf(value, separator); + + if (i == -1) + return value; + + return value[0 .. i]; +} + +S after(S)(S value, S separator) +if(isSomeString!S) { + auto i = indexOf(value, separator); + + if( i == -1) + return ""; + + size_t start = i + separator.length; + + return value[start .. $]; +} + +S afterLast(S)(S value, S separator) +if (isSomeString!S) { + size_t i = rindex(value, separator); + + if (i == value.length) + return ""; + + size_t start = i + separator.length; + + return value[start .. $]; +} + +void shouldWarn(string code, analysis.run.AnalyzerCheck analyzers, string file=__FILE__, size_t line=__LINE__) +{ + import analysis.run; + + // Run the code and get any warnings + string[] rawWarnings = analyze("test", cast(ubyte[]) code, analyzers); + string[] codeLines = code.split("\n"); + + // Get the warnings ordered by line + string[size_t] warnings; + for (size_t i=0; i Date: Sat, 17 May 2014 12:17:44 -0700 Subject: [PATCH 2/2] Cleanup of test functions. --- analysis/constructors.d | 2 +- analysis/del.d | 2 +- analysis/fish.d | 2 +- analysis/helpers.d | 41 ++++++++++++++++------------------------- analysis/ifelsesame.d | 2 +- analysis/numbers.d | 2 +- analysis/objectconst.d | 2 +- analysis/pokemon.d | 2 +- analysis/range.d | 2 +- analysis/style.d | 2 +- std/allocator.d | 5 ++--- 11 files changed, 27 insertions(+), 37 deletions(-) diff --git a/analysis/constructors.d b/analysis/constructors.d index 39c5125..2d38e82 100644 --- a/analysis/constructors.d +++ b/analysis/constructors.d @@ -90,7 +90,7 @@ private: unittest { - shouldWarn(q{ + assertAnalyzerWarnings(q{ class Cat // [warn]: This class has a zero-argument constructor as well as a constructor with one default argument. This can be confusing { this() {} diff --git a/analysis/del.d b/analysis/del.d index 5d48152..d6120f9 100644 --- a/analysis/del.d +++ b/analysis/del.d @@ -32,7 +32,7 @@ class DeleteCheck : BaseAnalyzer unittest { - shouldWarn(q{ + assertAnalyzerWarnings(q{ void testDelete() { int[int] data = [1 : 2]; diff --git a/analysis/fish.d b/analysis/fish.d index d46d54f..c81fc9e 100644 --- a/analysis/fish.d +++ b/analysis/fish.d @@ -42,7 +42,7 @@ class FloatOperatorCheck : BaseAnalyzer unittest { - shouldWarn(q{ + assertAnalyzerWarnings(q{ void testFish() { float z = 1.5f; diff --git a/analysis/helpers.d b/analysis/helpers.d index 58bdb44..e4bbc0a 100644 --- a/analysis/helpers.d +++ b/analysis/helpers.d @@ -7,17 +7,18 @@ module analysis.helpers; import std.string; import std.traits; - import std.d.ast; S between(S)(S value, S before, S after) -if (isSomeString!S) { + if (isSomeString!S) +{ return value.after(before).before(after); } -S before(S)(S value, S separator) -if (isSomeString!S) { +S before(S)(S value, S separator) + if (isSomeString!S) +{ auto i = indexOf(value, separator); if (i == -1) @@ -26,11 +27,12 @@ if (isSomeString!S) { return value[0 .. i]; } -S after(S)(S value, S separator) -if(isSomeString!S) { +S after(S)(S value, S separator) + if (isSomeString!S) +{ auto i = indexOf(value, separator); - if( i == -1) + if (i == -1) return ""; size_t start = i + separator.length; @@ -38,19 +40,12 @@ if(isSomeString!S) { return value[start .. $]; } -S afterLast(S)(S value, S separator) -if (isSomeString!S) { - size_t i = rindex(value, separator); - - if (i == value.length) - return ""; - - size_t start = i + separator.length; - - return value[start .. $]; -} - -void shouldWarn(string code, analysis.run.AnalyzerCheck analyzers, string file=__FILE__, size_t line=__LINE__) +/** + * This assert function will analyze the passed in code, get the warnings, + * and make sure they match the warnings in the comments. Warnings are + * marked like so: // [warn]: Failed to do somethings. + */ +void assertAnalyzerWarnings(string code, analysis.run.AnalyzerCheck analyzers, string file=__FILE__, size_t line=__LINE__) { import analysis.run; @@ -64,7 +59,6 @@ void shouldWarn(string code, analysis.run.AnalyzerCheck analyzers, string file=_ { size_t warnLine = line - 1 + std.conv.to!size_t(rawWarnings[i].between("test(", ":")); warnings[warnLine] = rawWarnings[i].after(")"); -// stderr.writefln("!!! warnings[%d] = \"%s\"", warnLine, warnings[warnLine]); } // Get all the messages from the comments in the code @@ -85,14 +79,12 @@ void shouldWarn(string code, analysis.run.AnalyzerCheck analyzers, string file=_ size_t lineNo = i + line; // Get the message -// stderr.writefln("!!! message[%d] = \"%s\"", lineNo, commentPart); messages[lineNo] = commentPart; } // Throw an assert error if any messages are not listed in the warnings foreach (lineNo, message; messages) { -// stderr.writefln("!!!!!! messages[%d] : %s", lineNo, messages[lineNo]); // No warning if (lineNo !in warnings) { @@ -102,8 +94,8 @@ void shouldWarn(string code, analysis.run.AnalyzerCheck analyzers, string file=_ codeLines[lineNo - line] ); throw new core.exception.AssertError(errors, file, lineNo); - // Different warning } + // Different warning else if (warnings[lineNo] != messages[lineNo]) { string errors = "Expected warning:\n%s\nBut was:\n%s\nFrom source code at (%s:?):\n%s".format( @@ -120,7 +112,6 @@ void shouldWarn(string code, analysis.run.AnalyzerCheck analyzers, string file=_ string[] unexpectedWarnings; foreach (lineNo, warning; warnings) { -// stderr.writefln("!!!!!! warnings[%d] : %s", lineNo, warning); // Unexpected warning if (lineNo !in messages) { diff --git a/analysis/ifelsesame.d b/analysis/ifelsesame.d index 6736bd5..7a02d13 100644 --- a/analysis/ifelsesame.d +++ b/analysis/ifelsesame.d @@ -48,7 +48,7 @@ class IfElseSameCheck : BaseAnalyzer unittest { - shouldWarn(q{ + assertAnalyzerWarnings(q{ void testSizeT() { string person = "unknown"; diff --git a/analysis/numbers.d b/analysis/numbers.d index 24c7136..e9935a7 100644 --- a/analysis/numbers.d +++ b/analysis/numbers.d @@ -42,7 +42,7 @@ class NumberStyleCheck : BaseAnalyzer unittest { - shouldWarn(q{ + assertAnalyzerWarnings(q{ void testNumbers() { int a; diff --git a/analysis/objectconst.d b/analysis/objectconst.d index 62f979b..f8fc5d8 100644 --- a/analysis/objectconst.d +++ b/analysis/objectconst.d @@ -71,7 +71,7 @@ class ObjectConstCheck : BaseAnalyzer unittest { - shouldWarn(q{ + assertAnalyzerWarnings(q{ void testConsts() { // Will be ok because all are declared const/immutable diff --git a/analysis/pokemon.d b/analysis/pokemon.d index 1d8bd73..0937bfe 100644 --- a/analysis/pokemon.d +++ b/analysis/pokemon.d @@ -58,7 +58,7 @@ class PokemonExceptionCheck : BaseAnalyzer unittest { - shouldWarn(q{ + assertAnalyzerWarnings(q{ void testCatch() { try diff --git a/analysis/range.d b/analysis/range.d index 617fef3..9985159 100644 --- a/analysis/range.d +++ b/analysis/range.d @@ -123,7 +123,7 @@ class BackwardsRangeCheck : BaseAnalyzer unittest { - shouldWarn(q{ + assertAnalyzerWarnings(q{ void testRange() { int[] data = [1, 2, 3, 4, 5]; diff --git a/analysis/style.d b/analysis/style.d index 92a4192..24fd1af 100644 --- a/analysis/style.d +++ b/analysis/style.d @@ -93,7 +93,7 @@ class StyleChecker : BaseAnalyzer unittest { - shouldWarn(q{ + assertAnalyzerWarnings(q{ module AMODULE; // [warn]: Module/package name 'AMODULE' does not match style guidelines bool A_VARIABLE; // FIXME: diff --git a/std/allocator.d b/std/allocator.d index d2a0aa6..be494e9 100644 --- a/std/allocator.d +++ b/std/allocator.d @@ -3056,8 +3056,7 @@ unittest } // FIXME: Disabling this test because it is machine dependent -/* -unittest +version (none) unittest { InSituRegion!(4096) r1; auto a = r1.allocate(2001); @@ -3069,7 +3068,7 @@ unittest a = r2.allocate(2001); assert(a.length == 2001); } -*/ + /** _Options for $(D AllocatorWithStats) defined below. Each enables during compilation one specific counter, statistic, or other piece of information.