diff --git a/docs/deprecations.md b/docs/deprecations.md index c416ad6d..ca277d7e 100644 --- a/docs/deprecations.md +++ b/docs/deprecations.md @@ -9,6 +9,35 @@ either of these is a breaking change, and thus will not happen until at least the next major release. +### 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 ### `CHECKED_IF` and `CHECKED_ELSE` diff --git a/src/catch2/catch_matchers.h b/src/catch2/catch_matchers.h index 78d9e84e..08b69584 100644 --- a/src/catch2/catch_matchers.h +++ b/src/catch2/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/tests/SelfTest/Baselines/automake.sw.approved.txt b/tests/SelfTest/Baselines/automake.sw.approved.txt index db7f4006..77e56433 100644 --- a/tests/SelfTest/Baselines/automake.sw.approved.txt +++ b/tests/SelfTest/Baselines/automake.sw.approved.txt @@ -99,6 +99,7 @@ Nor would this :test-result: PASS Comparisons between ints where one side is computed :test-result: PASS Comparisons between unsigned ints and negative signed ints match c++ standard behaviour :test-result: PASS Comparisons with int literals don't warn when mixing signed/ unsigned +:test-result: PASS Composed matchers are distinct :test-result: FAIL Contains string matcher :test-result: PASS Copy and then generate a range :test-result: FAIL Custom exceptions can be translated when testing for nothrow diff --git a/tests/SelfTest/Baselines/compact.sw.approved.txt b/tests/SelfTest/Baselines/compact.sw.approved.txt index 48007822..66c48408 100644 --- a/tests/SelfTest/Baselines/compact.sw.approved.txt +++ b/tests/SelfTest/Baselines/compact.sw.approved.txt @@ -344,6 +344,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/tests/SelfTest/Baselines/console.std.approved.txt b/tests/SelfTest/Baselines/console.std.approved.txt index 69ba4035..a0603ad8 100644 --- a/tests/SelfTest/Baselines/console.std.approved.txt +++ b/tests/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: 328 | 254 passed | 70 failed | 4 failed as expected -assertions: 1829 | 1677 passed | 131 failed | 21 failed as expected +test cases: 329 | 255 passed | 70 failed | 4 failed as expected +assertions: 1831 | 1679 passed | 131 failed | 21 failed as expected diff --git a/tests/SelfTest/Baselines/console.sw.approved.txt b/tests/SelfTest/Baselines/console.sw.approved.txt index d6285a81..10eada63 100644 --- a/tests/SelfTest/Baselines/console.sw.approved.txt +++ b/tests/SelfTest/Baselines/console.sw.approved.txt @@ -2610,6 +2610,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 ------------------------------------------------------------------------------- @@ -14300,6 +14318,6 @@ Misc.tests.cpp: Misc.tests.cpp:: PASSED: =============================================================================== -test cases: 328 | 238 passed | 86 failed | 4 failed as expected -assertions: 1846 | 1677 passed | 148 failed | 21 failed as expected +test cases: 329 | 239 passed | 86 failed | 4 failed as expected +assertions: 1848 | 1679 passed | 148 failed | 21 failed as expected diff --git a/tests/SelfTest/Baselines/junit.sw.approved.txt b/tests/SelfTest/Baselines/junit.sw.approved.txt index 0be5cc07..b10151f1 100644 --- a/tests/SelfTest/Baselines/junit.sw.approved.txt +++ b/tests/SelfTest/Baselines/junit.sw.approved.txt @@ -1,7 +1,7 @@ - + @@ -364,6 +364,7 @@ Exception.tests.cpp: + FAILED: diff --git a/tests/SelfTest/Baselines/sonarqube.sw.approved.txt b/tests/SelfTest/Baselines/sonarqube.sw.approved.txt index 07c512f0..1b446e04 100644 --- a/tests/SelfTest/Baselines/sonarqube.sw.approved.txt +++ b/tests/SelfTest/Baselines/sonarqube.sw.approved.txt @@ -923,6 +923,7 @@ Exception.tests.cpp: + FAILED: diff --git a/tests/SelfTest/Baselines/tap.sw.approved.txt b/tests/SelfTest/Baselines/tap.sw.approved.txt index b22d58c5..9e366479 100644 --- a/tests/SelfTest/Baselines/tap.sw.approved.txt +++ b/tests/SelfTest/Baselines/tap.sw.approved.txt @@ -686,6 +686,10 @@ ok {test-number} - 5 == c for: 5 == 5 ok {test-number} - 6 == uc for: 6 == 6 # Comparisons with int literals don't warn when mixing signed/ unsigned ok {test-number} - (std::numeric_limits::max)() > ul for: 4294967295 (0x) > 4 +# Composed matchers are distinct +ok {test-number} - testStringForMatching2(), !composed1 for: "some completely different text that contains one common word" not ( contains: "string" or contains: "random" ) +# Composed matchers are distinct +ok {test-number} - testStringForMatching2(), composed2 for: "some completely different text that contains one common word" ( contains: "string" or contains: "random" or contains: "different" ) # Contains string matcher not ok {test-number} - testStringForMatching(), Contains("not there", Catch::CaseSensitive::No) for: "this string contains 'abc' as a substring" contains: "not there" (case insensitive) # Contains string matcher @@ -3684,5 +3688,5 @@ ok {test-number} - q3 == 23. for: 23.0 == 23.0 ok {test-number} - # xmlentitycheck ok {test-number} - -1..1838 +1..1840 diff --git a/tests/SelfTest/Baselines/teamcity.sw.approved.txt b/tests/SelfTest/Baselines/teamcity.sw.approved.txt index dbfb9835..e3d1ead2 100644 --- a/tests/SelfTest/Baselines/teamcity.sw.approved.txt +++ b/tests/SelfTest/Baselines/teamcity.sw.approved.txt @@ -231,6 +231,8 @@ Exception.tests.cpp:|nunexpected exception with message:|n "unexpe ##teamcity[testFinished name='Comparisons between unsigned ints and negative signed ints match c++ standard behaviour' duration="{duration}"] ##teamcity[testStarted name='Comparisons with int literals don|'t warn when mixing signed/ unsigned'] ##teamcity[testFinished name='Comparisons with int literals don|'t warn when mixing signed/ unsigned' duration="{duration}"] +##teamcity[testStarted name='Composed matchers are distinct'] +##teamcity[testFinished name='Composed matchers are distinct' duration="{duration}"] ##teamcity[testStarted name='Contains string matcher'] Matchers.tests.cpp:|nexpression failed|n CHECK_THAT( testStringForMatching(), Contains("not there", Catch::CaseSensitive::No) )|nwith expansion:|n "this string contains |'abc|' as a substring" contains: "not there" (case insensitive)|n'] Matchers.tests.cpp:|nexpression failed|n CHECK_THAT( testStringForMatching(), Contains("STRING") )|nwith expansion:|n "this string contains |'abc|' as a substring" contains: "STRING"|n'] diff --git a/tests/SelfTest/Baselines/xml.sw.approved.txt b/tests/SelfTest/Baselines/xml.sw.approved.txt index d2d0ce54..63f6d760 100644 --- a/tests/SelfTest/Baselines/xml.sw.approved.txt +++ b/tests/SelfTest/Baselines/xml.sw.approved.txt @@ -3011,6 +3011,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" ) + + + + @@ -17139,7 +17158,7 @@ loose text artifact - + - + diff --git a/tests/SelfTest/UsageTests/Matchers.tests.cpp b/tests/SelfTest/UsageTests/Matchers.tests.cpp index e10e98ca..e5c28ef9 100644 --- a/tests/SelfTest/UsageTests/Matchers.tests.cpp +++ b/tests/SelfTest/UsageTests/Matchers.tests.cpp @@ -556,6 +556,16 @@ namespace { namespace MatchersTests { 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); + } + template struct EqualsRangeMatcher : Catch::MatcherGenericBase {