This is an automated email from the ASF dual-hosted git repository.
xiazcy pushed a commit to branch steps-taking-traversal
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
The following commit(s) were added to refs/heads/steps-taking-traversal by this
push:
new b8a6f57291 Eliminate leftover null guards and dead code from
HasContainer unification
b8a6f57291 is described below
commit b8a6f57291f09a76240ae5c52f839964f38b66aa
Author: Yang Xia <[email protected]>
AuthorDate: Fri Jun 12 10:41:32 2026 -0700
Eliminate leftover null guards and dead code from HasContainer unification
Assisted-by: Kiro:claude-opus-4-6
---
.../process/traversal/step/HasContainerHolder.java | 2 +-
.../process/traversal/step/filter/HasStep.java | 4 +-
.../traversal/step/sideEffect/AddPropertyStep.java | 11 +-----
.../process/traversal/step/util/HasContainer.java | 6 +--
.../GremlinLangTraversalRoundTripTest.java | 7 +---
.../gremlin/process/traversal/PTraversalTest.java | 44 ++++++++++------------
.../step/CloneIndependenceTraversalTest.java | 2 -
.../traversal/step/util/HasContainerTest.java | 1 -
.../util/ChildTraversalValidatorTest.java | 2 +-
9 files changed, 29 insertions(+), 50 deletions(-)
diff --git
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/HasContainerHolder.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/HasContainerHolder.java
index 47343d3ea6..f016aa03f4 100644
---
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/HasContainerHolder.java
+++
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/HasContainerHolder.java
@@ -53,7 +53,7 @@ public interface HasContainerHolder<S, E> extends
GValueHolder<S, E> {
}
public default Collection<P<?>> getPredicatesGValueSafe() {
- return getHasContainers().stream().map(p -> p.getPredicate()).filter(p
-> p != null).collect(Collectors.toList());
+ return getHasContainers().stream().map(p ->
p.getPredicate()).collect(Collectors.toList());
}
public default HasContainerHolder<S, E> asConcreteStep() {
diff --git
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/HasStep.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/HasStep.java
index 1b56dbdfeb..a69e8f7871 100644
---
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/HasStep.java
+++
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/HasStep.java
@@ -131,7 +131,7 @@ public class HasStep<S extends Element> extends
FilterStep<S> implements HasCont
* {@link #setTraversal(Traversal.Admin)} which is the single integration
point.
*/
private void collectChildTraversals(final HasContainer hc) {
- if (hc.getPredicate() != null && hc.getPredicate().hasTraversal()) {
+ if (hc.getPredicate().hasTraversal()) {
P.collectTraversals(hc.getPredicate(), this.childTraversals);
}
}
@@ -182,7 +182,7 @@ public class HasStep<S extends Element> extends
FilterStep<S> implements HasCont
for (final HasContainer hasContainer : this.hasContainers) {
final HasContainer clonedHc = hasContainer.clone();
clone.hasContainers.add(clonedHc);
- if (clonedHc.getPredicate() != null &&
clonedHc.getPredicate().hasTraversal()) {
+ if (clonedHc.getPredicate().hasTraversal()) {
P.collectTraversals(clonedHc.getPredicate(),
clone.childTraversals);
}
}
diff --git
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java
index 7b5acbf44a..0901a67196 100644
---
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java
+++
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java
@@ -49,21 +49,12 @@ import java.util.Set;
*/
public class AddPropertyStep<S extends Element> extends SideEffectStep<S>
implements AddPropertyStepContract<S> {
- /**
- * Internal placeholder key used for the single-argument {@code
property(traversal)} (Map-producing) form,
- * where the property keys are supplied by the traversal's resulting Map
rather than by a caller-provided key.
- * This value is never used for dispatch (see {@link #mapForm}); it exists
only to give the parameter a
- * stable, non-null key within {@link
org.apache.tinkerpop.gremlin.process.traversal.step.util.Parameters}.
- */
- static final String MAP_VALUE_TRAVERSAL_KEY = "~traversalMap";
-
private Parameters internalParameters = new Parameters();
private Parameters withConfiguration = new Parameters();
private final VertexProperty.Cardinality cardinality;
/**
* Marks the single-argument {@code property(traversal)} form whose child
traversal must produce a Map of
- * property key/value pairs. Dispatch is driven by this flag rather than
by inspecting the property key, so a
- * caller-supplied key that happens to collide with {@link
#MAP_VALUE_TRAVERSAL_KEY} is handled normally.
+ * property key/value pairs. Dispatch is driven by this flag rather than
by inspecting the property key.
*/
private final boolean mapForm;
private CallbackRegistry<Event.ElementPropertyChangedEvent>
callbackRegistry;
diff --git
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java
index c6d0d1fe29..82204d49f2 100644
---
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java
+++
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java
@@ -110,7 +110,7 @@ public class HasContainer implements Serializable,
Cloneable, Predicate<Element>
public HasContainer clone() {
try {
final HasContainer clone = (HasContainer) super.clone();
- clone.predicate = this.predicate != null ? this.predicate.clone()
: null;
+ clone.predicate = this.predicate.clone();
return clone;
} catch (final CloneNotSupportedException e) {
throw new IllegalStateException(e.getMessage(), e);
@@ -119,7 +119,7 @@ public class HasContainer implements Serializable,
Cloneable, Predicate<Element>
@Override
public int hashCode() {
- return (this.key != null ? this.key.hashCode() : 0) ^ (this.predicate
!= null ? this.predicate.hashCode() : 0);
+ return (this.key != null ? this.key.hashCode() : 0) ^
this.predicate.hashCode();
}
public final String getKey() {
@@ -147,7 +147,7 @@ public class HasContainer implements Serializable,
Cloneable, Predicate<Element>
* at runtime (e.g. {@code P.eq(traversal)} or {@code
P.within(traversal)}).
*/
public boolean hasTraversal() {
- return this.predicate != null && this.predicate.hasTraversal();
+ return this.predicate.hasTraversal();
}
////////////
diff --git
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/GremlinLangTraversalRoundTripTest.java
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/GremlinLangTraversalRoundTripTest.java
index 391938b9ca..ff1cc9a6a7 100644
---
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/GremlinLangTraversalRoundTripTest.java
+++
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/GremlinLangTraversalRoundTripTest.java
@@ -29,12 +29,9 @@ import static
org.apache.tinkerpop.gremlin.process.traversal.AnonymousTraversalS
import static org.junit.Assert.assertEquals;
/**
- * Property 7: GremlinLang serialization round-trip preserves traversal
arguments.
- * <p>
+ * Tests that GremlinLang serialization round-trips preserve traversal
arguments.
* For any step containing a child traversal argument, serializing to
GremlinLang and parsing back
- * SHALL produce a structurally equivalent traversal.
- * <p>
- * <b>Validates: Requirements 1.3, 3.6, 4.5, 6.5, 7.3</b>
+ * produces a structurally equivalent traversal.
*/
public class GremlinLangTraversalRoundTripTest {
diff --git
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/PTraversalTest.java
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/PTraversalTest.java
index d52b45ae49..b4e32ede83 100644
---
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/PTraversalTest.java
+++
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/PTraversalTest.java
@@ -26,6 +26,9 @@ import org.junit.experimental.runners.Enclosed;
import org.junit.runner.RunWith;
import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.CoreMatchers.nullValue;
+import static org.hamcrest.CoreMatchers.sameInstance;
import static org.hamcrest.MatcherAssert.assertThat;
/**
@@ -36,13 +39,8 @@ import static org.hamcrest.MatcherAssert.assertThat;
public class PTraversalTest {
/**
- * Property 10: Traversal detection in predicates is accurate.
- * <p>
- * For any P instance containing a child traversal, {@code
P.hasTraversal()} SHALL return true.
- * For any P instance containing only literal values or GValue variables,
- * {@code P.hasTraversal()} SHALL return false.
- * <p>
- * <b>Validates: Requirements 9.4</b>
+ * Tests that traversal detection in predicates is accurate: {@code
P.hasTraversal()} returns
+ * true for traversal-bearing predicates and false for literal/GValue
predicates.
*/
public static class TraversalDetectionTest {
@@ -74,25 +72,20 @@ public class PTraversalTest {
public void shouldReturnTraversalValueWhenPresent() {
final Traversal.Admin<?, ?> traversal = __.inject(42).asAdmin();
final P<Object> p = P.eq(traversal);
- assertThat(p.getTraversalValue() == traversal, is(true));
+ assertThat(p.getTraversalValue(), is(sameInstance(traversal)));
}
@Test
public void shouldReturnNullTraversalValueForLiteral() {
final P<String> p = P.eq("value");
- assertThat(p.getTraversalValue() == null, is(true));
+ assertThat(p.getTraversalValue(), is(nullValue()));
}
}
/**
- * Property 3: Single-value predicate rejects multiple traversal results.
- * <p>
- * For any single-value predicate (eq, neq, gt, lt, gte, lte) and any
child traversal that produces
- * more than one result, the predicate SHALL throw an
IllegalArgumentException.
- * For any collection predicate (within, without) and any child traversal
producing multiple results,
- * the predicate SHALL accept all results as the collection value.
- * <p>
- * <b>Validates: Requirements 2.5</b>
+ * Tests first-result semantics: single-value predicates (eq, neq, gt, lt,
gte, lte) take the
+ * first result from a multi-result traversal. Collection predicates
(within, without) accept
+ * all results as the collection value.
*/
public static class SingleValuePredicateRejectionTest {
@@ -100,8 +93,6 @@ public class PTraversalTest {
return new B_O_Traverser<>(value, 1L);
}
- // --- Single-value predicates should throw on multiple results ---
-
// --- Single-value predicates take first result, ignore extras
(consistent with by()) ---
@SuppressWarnings("unchecked")
@@ -267,7 +258,7 @@ public class PTraversalTest {
@Test
public void shouldReturnTraversalValuesListForMultiTraversal() {
final P<Object> p = P.within(__.constant(1).asAdmin(),
__.constant(2).asAdmin());
- assertThat(p.getTraversalValues() != null, is(true));
+ assertThat(p.getTraversalValues(), is(notNullValue()));
assertThat(p.getTraversalValues().size(), is(2));
}
@@ -275,7 +266,7 @@ public class PTraversalTest {
public void shouldReturnNullTraversalValueForMultiTraversal() {
// Single traversalValue should be null when using multi-traversal
form
final P<Object> p = P.within(__.constant(1).asAdmin(),
__.constant(2).asAdmin());
- assertThat(p.getTraversalValue() == null, is(true));
+ assertThat(p.getTraversalValue(), is(nullValue()));
}
// --- Resolution and testing ---
@@ -361,14 +352,17 @@ public class PTraversalTest {
// Clone should have independent traversal values
assertThat(clone.hasTraversal(), is(true));
- assertThat(clone.getTraversalValues() != null, is(true));
+ assertThat(clone.getTraversalValues(), is(notNullValue()));
assertThat(clone.getTraversalValues().size(), is(2));
- // Modifying clone should not affect original
+ // Resolve the clone — this mutates the clone's internal state
clone.resolve(createTraverser("start"));
assertThat(clone.test(1), is(true));
- // Original should still be unresolved (literals empty)
- assertThat(original.getTraversalValues().size(), is(2));
+
+ // Original should be unaffected: resolving it independently
should still work correctly
+ original.resolve(createTraverser("other"));
+ assertThat(original.test(1), is(true));
+ assertThat(original.test(2), is(true));
}
// --- Varargs detection ---
diff --git
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/CloneIndependenceTraversalTest.java
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/CloneIndependenceTraversalTest.java
index 09af6da179..4c8d6ff850 100644
---
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/CloneIndependenceTraversalTest.java
+++
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/CloneIndependenceTraversalTest.java
@@ -42,8 +42,6 @@ import static org.hamcrest.core.IsSame.sameInstance;
* For any step containing child traversal arguments, cloning the step SHALL
produce a deep copy
* where modifying the clone's child traversal state does NOT affect the
original step's child
* traversal state.
- * <p>
- * <b>Validates: Requirements 6.2</b>
*/
public class CloneIndependenceTraversalTest {
diff --git
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainerTest.java
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainerTest.java
index 4049b20eb6..efe7072ad0 100644
---
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainerTest.java
+++
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainerTest.java
@@ -56,7 +56,6 @@ public class HasContainerTest {
final HasContainer hc = new HasContainer("age", P.eq(traversal));
assertEquals("age", hc.getKey());
- assertThat(hc.getPredicate(), is(notNullValue()));
assertThat(hc.getPredicate().getTraversalValue(),
is(sameInstance(traversal)));
}
diff --git
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/util/ChildTraversalValidatorTest.java
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/util/ChildTraversalValidatorTest.java
index 68b99296ec..fc97b89638 100644
---
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/util/ChildTraversalValidatorTest.java
+++
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/util/ChildTraversalValidatorTest.java
@@ -216,7 +216,7 @@ public class ChildTraversalValidatorTest {
@Test
public void shouldRejectAddVInMutationContext() {
- // addV is now blocked in property(traversal) - all mutating steps are
blocked in all contexts
+ // addV is blocked in property(traversal) — mutating steps are blocked
in child traversals
assertThrows(IllegalArgumentException.class, () ->
g.V().property(__.V().addV("temp").project("k").by("name")));
}
