diff --git a/docs/deprecations.md b/docs/deprecations.md index c2b178e2..39194ed7 100644 --- a/docs/deprecations.md +++ b/docs/deprecations.md @@ -45,6 +45,34 @@ There should be no reason to ever have an empty `SourceLineInfo`, so the method will be removed. +### Composing lvalues of already composed matchers + +Because a significant bug in this use case has persisted for 2+ years +without a bug report, and to simplify the implementation, code that +composes lvalues of composed matchers will not compile. That is, +this code will no longer work: + +```cpp + auto m1 = Contains("string"); + auto m2 = Contains("random"); + auto composed1 = m1 || m2; + auto m3 = Contains("different"); + auto composed2 = composed1 || m3; + REQUIRE_THAT(foo(), !composed1); + REQUIRE_THAT(foo(), composed2); +``` + +Instead you will have to write this: + +```cpp + auto m1 = Contains("string"); + auto m2 = Contains("random"); + auto m3 = Contains("different"); + REQUIRE_THAT(foo(), !(m1 || m2)); + REQUIRE_THAT(foo(), m1 || m2 || m3); +``` + + ## Planned changes diff --git a/include/internal/catch_matchers.h b/include/internal/catch_matchers.h index 5b69c4a8..518a6e0d 100644 --- a/include/internal/catch_matchers.h +++ b/include/internal/catch_matchers.h @@ -91,9 +91,10 @@ namespace Matchers { return description; } - MatchAllOf& operator && ( MatcherBase const& other ) { - m_matchers.push_back( &other ); - return *this; + MatchAllOf operator && ( MatcherBase const& other ) { + auto copy(*this); + copy.m_matchers.push_back( &other ); + return copy; } std::vector const*> m_matchers; @@ -124,9 +125,10 @@ namespace Matchers { return description; } - MatchAnyOf& operator || ( MatcherBase const& other ) { - m_matchers.push_back( &other ); - return *this; + MatchAnyOf operator || ( MatcherBase const& other ) { + auto copy(*this); + copy.m_matchers.push_back( &other ); + return copy; } std::vector const*> m_matchers; diff --git a/projects/SelfTest/Baselines/compact.sw.approved.txt b/projects/SelfTest/Baselines/compact.sw.approved.txt index 628c6bdf..c12ce081 100644 --- a/projects/SelfTest/Baselines/compact.sw.approved.txt +++ b/projects/SelfTest/Baselines/compact.sw.approved.txt @@ -312,6 +312,8 @@ Condition.tests.cpp:: passed: 4 == ul for: 4 == 4 Condition.tests.cpp:: passed: 5 == c for: 5 == 5 Condition.tests.cpp:: passed: 6 == uc for: 6 == 6 Condition.tests.cpp:: passed: (std::numeric_limits::max)() > ul for: 4294967295 (0x) > 4 +Matchers.tests.cpp:: passed: testStringForMatching2(), !composed1 for: "some completely different text that contains one common word" not ( contains: "string" or contains: "random" ) +Matchers.tests.cpp:: passed: testStringForMatching2(), composed2 for: "some completely different text that contains one common word" ( contains: "string" or contains: "random" or contains: "different" ) Matchers.tests.cpp:: failed: testStringForMatching(), Contains("not there", Catch::CaseSensitive::No) for: "this string contains 'abc' as a substring" contains: "not there" (case insensitive) Matchers.tests.cpp:: failed: testStringForMatching(), Contains("STRING") for: "this string contains 'abc' as a substring" contains: "STRING" Generators.tests.cpp:: passed: elem % 2 == 1 for: 1 == 1 diff --git a/projects/SelfTest/Baselines/console.std.approved.txt b/projects/SelfTest/Baselines/console.std.approved.txt index 74e576a9..f62b2e48 100644 --- a/projects/SelfTest/Baselines/console.std.approved.txt +++ b/projects/SelfTest/Baselines/console.std.approved.txt @@ -1380,6 +1380,6 @@ due to unexpected exception with message: Why would you throw a std::string? =============================================================================== -test cases: 305 | 231 passed | 70 failed | 4 failed as expected -assertions: 1664 | 1512 passed | 131 failed | 21 failed as expected +test cases: 306 | 232 passed | 70 failed | 4 failed as expected +assertions: 1666 | 1514 passed | 131 failed | 21 failed as expected diff --git a/projects/SelfTest/Baselines/console.sw.approved.txt b/projects/SelfTest/Baselines/console.sw.approved.txt index 25a92320..68357ef0 100644 --- a/projects/SelfTest/Baselines/console.sw.approved.txt +++ b/projects/SelfTest/Baselines/console.sw.approved.txt @@ -2390,6 +2390,24 @@ Condition.tests.cpp:: PASSED: with expansion: 4294967295 (0x) > 4 +------------------------------------------------------------------------------- +Composed matchers are distinct +------------------------------------------------------------------------------- +Matchers.tests.cpp: +............................................................................... + +Matchers.tests.cpp:: PASSED: + REQUIRE_THAT( testStringForMatching2(), !composed1 ) +with expansion: + "some completely different text that contains one common word" not ( + contains: "string" or contains: "random" ) + +Matchers.tests.cpp:: PASSED: + REQUIRE_THAT( testStringForMatching2(), composed2 ) +with expansion: + "some completely different text that contains one common word" ( contains: + "string" or contains: "random" or contains: "different" ) + ------------------------------------------------------------------------------- Contains string matcher ------------------------------------------------------------------------------- @@ -13302,6 +13320,6 @@ Misc.tests.cpp: Misc.tests.cpp:: PASSED: =============================================================================== -test cases: 305 | 215 passed | 86 failed | 4 failed as expected -assertions: 1681 | 1512 passed | 148 failed | 21 failed as expected +test cases: 306 | 216 passed | 86 failed | 4 failed as expected +assertions: 1683 | 1514 passed | 148 failed | 21 failed as expected diff --git a/projects/SelfTest/Baselines/junit.sw.approved.txt b/projects/SelfTest/Baselines/junit.sw.approved.txt index e0b7c1b8..59119a25 100644 --- a/projects/SelfTest/Baselines/junit.sw.approved.txt +++ b/projects/SelfTest/Baselines/junit.sw.approved.txt @@ -1,7 +1,7 @@ - + @@ -356,6 +356,7 @@ Exception.tests.cpp: + FAILED: diff --git a/projects/SelfTest/Baselines/sonarqube.sw.approved.txt b/projects/SelfTest/Baselines/sonarqube.sw.approved.txt index d6f6b090..1d7e92bf 100644 --- a/projects/SelfTest/Baselines/sonarqube.sw.approved.txt +++ b/projects/SelfTest/Baselines/sonarqube.sw.approved.txt @@ -892,6 +892,7 @@ Exception.tests.cpp: + FAILED: diff --git a/projects/SelfTest/Baselines/xml.sw.approved.txt b/projects/SelfTest/Baselines/xml.sw.approved.txt index 6d06368e..1021e168 100644 --- a/projects/SelfTest/Baselines/xml.sw.approved.txt +++ b/projects/SelfTest/Baselines/xml.sw.approved.txt @@ -2832,6 +2832,25 @@ Nor would this + + + + testStringForMatching2(), !composed1 + + + "some completely different text that contains one common word" not ( contains: "string" or contains: "random" ) + + + + + testStringForMatching2(), composed2 + + + "some completely different text that contains one common word" ( contains: "string" or contains: "random" or contains: "different" ) + + + + @@ -15893,7 +15912,7 @@ loose text artifact - + - + diff --git a/projects/SelfTest/UsageTests/Matchers.tests.cpp b/projects/SelfTest/UsageTests/Matchers.tests.cpp index 80e0420f..52f2912d 100644 --- a/projects/SelfTest/UsageTests/Matchers.tests.cpp +++ b/projects/SelfTest/UsageTests/Matchers.tests.cpp @@ -552,6 +552,16 @@ namespace { namespace MatchersTests { REQUIRE_THROWS_MATCHES(throwsSpecialException(2), SpecialException, !Message("DerivedException::what")); REQUIRE_THROWS_MATCHES(throwsSpecialException(2), SpecialException, Message("SpecialException::what")); } + + TEST_CASE("Composed matchers are distinct", "[matchers][composed]") { + auto m1 = Contains("string"); + auto m2 = Contains("random"); + auto composed1 = m1 || m2; + auto m3 = Contains("different"); + auto composed2 = composed1 || m3; + REQUIRE_THAT(testStringForMatching2(), !composed1); + REQUIRE_THAT(testStringForMatching2(), composed2); + } } } // namespace MatchersTests