Skip to content

Commit 599016b

Browse files
committed
Extend dead-chain pattern to Matcher group/groups and Iterator.toIterable
`AbstractMatcherAssert#group(int)`, `group(String)`, and `groups()` called `matches()` (which wraps its own `isNotNull()` in `executeAssertion`) and then dereferenced `actual.group(...)` / `actual.groupCount()`; on a null matcher in soft mode the null-guard was collected but the subsequent dereference leaked an NPE. `AbstractIteratorAssert#toIterable()` had no guard at all and NPE'd on a null iterator. Add `if (actual == null) return markAsDeadChain(new StringAssert(null))` (or `ListAssert`/`IterableAssert` for the other return types) after the existing guard. Also propagate `withAssertionState(myself)` on the normal-mode return for Matcher navigations so downstream chained assertions inherit the soft handler.
1 parent ba3f0f3 commit 599016b

3 files changed

Lines changed: 55 additions & 7 deletions

File tree

assertj-core/src/main/java/org/assertj/core/api/AbstractIteratorAssert.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,9 @@ public SELF isExhausted() {
9393
* @since 3.12.0
9494
*/
9595
public IterableAssert<ELEMENT> toIterable() {
96-
return new IterableAssert<>(IterableAssert.toIterable(actual));
96+
isNotNull();
97+
if (actual == null) return markAsDeadChain(new IterableAssert<ELEMENT>((Iterable<? extends ELEMENT>) null));
98+
return new IterableAssert<ELEMENT>(IterableAssert.toIterable(actual)).withAssertionState(myself);
9799
}
98100

99101
/**

assertj-core/src/main/java/org/assertj/core/api/AbstractMatcherAssert.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import static org.assertj.core.error.MatcherShouldHaveGroup.shouldHaveGroup;
2121
import static org.assertj.core.error.MatcherShouldMatch.shouldMatch;
2222

23+
import java.util.List;
2324
import java.util.function.Supplier;
2425
import java.util.regex.Matcher;
2526

@@ -84,9 +85,9 @@ public SELF matches() {
8485
* @since 4.0.0
8586
*/
8687
public AbstractStringAssert<?> group(int groupIndex) {
87-
isNotNull();
8888
matches();
89-
return assertThat(extractGroup(() -> actual.group(groupIndex), groupIndex));
89+
if (actual == null) return markAsDeadChain(new StringAssert(null));
90+
return assertThat(extractGroup(() -> actual.group(groupIndex), groupIndex)).withAssertionState(myself);
9091
}
9192

9293
/**
@@ -108,9 +109,9 @@ public AbstractStringAssert<?> group(int groupIndex) {
108109
* @since 4.0.0
109110
*/
110111
public AbstractStringAssert<?> group(String groupName) {
111-
isNotNull();
112112
matches();
113-
return assertThat(extractGroup(() -> actual.group(groupName), groupName));
113+
if (actual == null) return markAsDeadChain(new StringAssert(null));
114+
return assertThat(extractGroup(() -> actual.group(groupName), groupName)).withAssertionState(myself);
114115
}
115116

116117
/**
@@ -130,9 +131,9 @@ public AbstractStringAssert<?> group(String groupName) {
130131
* @since 4.0.0
131132
*/
132133
public ListAssert<String> groups() {
133-
isNotNull();
134134
matches();
135-
return assertThat(rangeClosed(1, actual.groupCount()).mapToObj(actual::group).toList());
135+
if (actual == null) return markAsDeadChain(new ListAssert<>((List<String>) null));
136+
return assertThat(rangeClosed(1, actual.groupCount()).mapToObj(actual::group).toList()).withAssertionState(myself);
136137
}
137138

138139
private String extractGroup(Supplier<String> groupExtractor, Object groupIdentifier) {

assertj-core/src/test/java/org/assertj/core/api/SoftAssertions_navigations_on_null_actual_Test.java

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -843,6 +843,51 @@ void should_not_throw_when_calling_asString_with_charset_on_null_input_stream()
843843

844844
}
845845

846+
@Nested
847+
class MatcherAndIterator {
848+
849+
@Test
850+
void should_not_throw_when_calling_group_by_index_on_null_matcher() {
851+
// GIVEN / WHEN
852+
softly.assertThat((java.util.regex.Matcher) null).group(1).isEqualTo("foo");
853+
// THEN
854+
List<Throwable> errors = softly.errorsCollected();
855+
then(errors).hasSize(1);
856+
then(errors.get(0)).hasMessageContaining("Expecting actual not to be null");
857+
}
858+
859+
@Test
860+
void should_not_throw_when_calling_group_by_name_on_null_matcher() {
861+
// GIVEN / WHEN
862+
softly.assertThat((java.util.regex.Matcher) null).group("g1").isEqualTo("foo");
863+
// THEN
864+
List<Throwable> errors = softly.errorsCollected();
865+
then(errors).hasSize(1);
866+
then(errors.get(0)).hasMessageContaining("Expecting actual not to be null");
867+
}
868+
869+
@Test
870+
void should_not_throw_when_calling_groups_on_null_matcher() {
871+
// GIVEN / WHEN
872+
softly.assertThat((java.util.regex.Matcher) null).groups().contains("foo");
873+
// THEN
874+
List<Throwable> errors = softly.errorsCollected();
875+
then(errors).hasSize(1);
876+
then(errors.get(0)).hasMessageContaining("Expecting actual not to be null");
877+
}
878+
879+
@Test
880+
void should_not_throw_when_calling_toIterable_on_null_iterator() {
881+
// GIVEN / WHEN
882+
softly.assertThat((java.util.Iterator<String>) null).toIterable().contains("foo");
883+
// THEN
884+
List<Throwable> errors = softly.errorsCollected();
885+
then(errors).hasSize(1);
886+
then(errors.get(0)).hasMessageContaining("Expecting actual not to be null");
887+
}
888+
889+
}
890+
846891
@Nested
847892
class StringNavigations {
848893

0 commit comments

Comments
 (0)