Skip to content

Commit 585dff2

Browse files
authored
fix: correct expression hierarchy, cycle detection, and AST capture (#6)
1 parent 77f9bd3 commit 585dff2

21 files changed

Lines changed: 564 additions & 75 deletions

File tree

codemodel-foundation/src/main/java/build/codemodel/foundation/usage/WildcardTypeUsage.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import build.codemodel.foundation.naming.TypeName;
3434

3535
import java.lang.invoke.MethodHandles;
36+
import java.util.Objects;
3637
import java.util.Optional;
3738
import java.util.stream.Stream;
3839

@@ -135,7 +136,7 @@ public boolean equals(final Object object) {
135136

136137
@Override
137138
public int hashCode() {
138-
return java.util.Objects.hash(lowerBound(), upperBound());
139+
return Objects.hash(lowerBound(), upperBound());
139140
}
140141

141142
/**

codemodel-foundation/src/test/java/build/codemodel/foundation/usage/GenericTypeUsageTests.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import org.junit.jupiter.api.BeforeEach;
1111
import org.junit.jupiter.api.Test;
1212

13+
import java.util.List;
1314
import java.util.Optional;
1415
import java.util.Spliterator;
1516

@@ -68,7 +69,7 @@ void shouldCompareSameTypes() {
6869
void shouldNotBeEqualWhenParametersAreDifferent() {
6970
final var codeModel = new ConceptualCodeModel(this.naming);
7071

71-
final var listTypeName = this.naming.getTypeName(java.util.List.class);
72+
final var listTypeName = this.naming.getTypeName(List.class);
7273
final var stringTypeName = this.naming.getTypeName(String.class);
7374
final var integerTypeName = this.naming.getTypeName(Integer.class);
7475

@@ -88,7 +89,7 @@ void shouldNotBeEqualWhenParametersAreDifferent() {
8889
void shouldBeEqualWhenBothHaveNoParameters() {
8990
final var codeModel = new ConceptualCodeModel(this.naming);
9091

91-
final var listTypeName = this.naming.getTypeName(java.util.List.class);
92+
final var listTypeName = this.naming.getTypeName(List.class);
9293

9394
final var rawList1 = GenericTypeUsage.of(codeModel, listTypeName);
9495
final var rawList2 = GenericTypeUsage.of(codeModel, listTypeName);

dependency-injection/src/test/java/build/codemodel/injection/DefaultOptionResolverTests.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66
import jakarta.inject.Inject;
77
import org.junit.jupiter.api.Test;
88

9+
import java.util.Optional;
10+
import java.util.stream.Stream;
11+
912
import static org.assertj.core.api.Assertions.assertThat;
1013

1114
/**
@@ -74,7 +77,7 @@ void shouldReturnEmptyWhenNoDefaultAnnotationPresent() {
7477
final var resolver = DefaultOptionResolver.of(framework);
7578

7679
final var typeUsage = framework.codeModel().getTypeUsage(OptionWithNoDefault.class);
77-
final var dependency = IndependentDependency.of(typeUsage, _ -> java.util.stream.Stream.empty());
80+
final var dependency = IndependentDependency.of(typeUsage, _ -> Stream.empty());
7881

7982
assertThat(resolver.resolve(dependency)).isEmpty();
8083
}
@@ -89,7 +92,7 @@ void shouldNotResolveNonOptionTypes() {
8992
// add a fallback for String so creation doesn't fail
9093
final var context = framework.newContext(
9194
DefaultOptionResolver.of(framework),
92-
dep -> java.util.Optional.of(new ValueBinding<Object>() {
95+
dep -> Optional.of(new ValueBinding<Object>() {
9396
@Override public Object value() { return "fallback"; }
9497
@Override public Dependency dependency() { return dep; }
9598
}));

dependency-injection/src/test/java/build/codemodel/injection/ProvidesResolverTests.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import org.junit.jupiter.api.Test;
55

66
import java.util.Optional;
7+
import java.util.stream.Stream;
78

89
import static org.assertj.core.api.Assertions.assertThat;
910

@@ -57,7 +58,7 @@ void shouldReturnEmptyWhenNoProvidesMethodMatchesDependency() {
5758
// EmptyProvider has no @Provides for String
5859
final var dependency = IndependentDependency.of(
5960
framework.codeModel().getTypeUsage(String.class),
60-
_ -> java.util.stream.Stream.empty());
61+
_ -> Stream.empty());
6162

6263
final Optional<?> result = resolver.resolve(dependency);
6364
assertThat(result).isEmpty();
@@ -77,7 +78,7 @@ void shouldIgnoreVoidProvidesMethod() {
7778
// nothing should be registered, so any dependency comes back empty
7879
final var dependency = IndependentDependency.of(
7980
framework.codeModel().getTypeUsage(String.class),
80-
_ -> java.util.stream.Stream.empty());
81+
_ -> Stream.empty());
8182

8283
assertThat(resolver.resolve(dependency)).isEmpty();
8384
}

expression-codemodel/src/main/java/build/codemodel/expression/BinaryArithmeticExpression.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
* @since Sep-2024
2828
*/
2929
public interface BinaryArithmeticExpression
30-
extends LogicalExpression {
30+
extends ArithmeticExpression {
3131

3232
/**
3333
* Obtains the left-hand-side of the {@link Expression} for the <i>Arithmetic Operand</i>.

expression-codemodel/src/main/java/build/codemodel/expression/UnaryArithmeticExpression.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
* @since Sep-2024
2828
*/
2929
public interface UnaryArithmeticExpression
30-
extends LogicalExpression {
30+
extends ArithmeticExpression {
3131

3232
/**
3333
* Obtains the {@link Expression} for the <i>ArithmeticOperand</i>.
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
package build.codemodel.expression;
2+
3+
/*-
4+
* #%L
5+
* Expression Code Model
6+
* %%
7+
* Copyright (C) 2026 Workday, Inc.
8+
* %%
9+
* Licensed under the Apache License, Version 2.0 (the "License");
10+
* you may not use this file except in compliance with the License.
11+
* You may obtain a copy of the License at
12+
*
13+
* http://www.apache.org/licenses/LICENSE-2.0
14+
*
15+
* Unless required by applicable law or agreed to in writing, software
16+
* distributed under the License is distributed on an "AS IS" BASIS,
17+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
18+
* See the License for the specific language governing permissions and
19+
* limitations under the License.
20+
* #L%
21+
*/
22+
23+
import build.codemodel.foundation.ConceptualCodeModel;
24+
import build.codemodel.foundation.naming.NonCachingNameProvider;
25+
import org.junit.jupiter.api.Test;
26+
27+
import static org.junit.jupiter.api.Assertions.assertFalse;
28+
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
29+
30+
/**
31+
* Tests for the {@link Expression} type hierarchy.
32+
*
33+
* @author reed.vonredwitz
34+
* @since Apr-2026
35+
*/
36+
class ExpressionHierarchyTests {
37+
38+
@Test
39+
void unaryArithmeticExpressionShouldBeArithmeticNotLogical() {
40+
final var codeModel = new ConceptualCodeModel(new NonCachingNameProvider());
41+
final var operand = NumericLiteral.of(codeModel, 1);
42+
final var negative = Negative.of(operand);
43+
44+
assertInstanceOf(UnaryArithmeticExpression.class, negative);
45+
assertInstanceOf(ArithmeticExpression.class, negative);
46+
assertFalse(negative instanceof LogicalExpression, "UnaryArithmeticExpression must not be a LogicalExpression");
47+
}
48+
49+
@Test
50+
void binaryArithmeticExpressionShouldBeArithmeticNotLogical() {
51+
final var codeModel = new ConceptualCodeModel(new NonCachingNameProvider());
52+
final var left = NumericLiteral.of(codeModel, 1);
53+
final var right = NumericLiteral.of(codeModel, 2);
54+
final var addition = Addition.of(left, right);
55+
56+
assertInstanceOf(BinaryArithmeticExpression.class, addition);
57+
assertInstanceOf(ArithmeticExpression.class, addition);
58+
assertFalse(addition instanceof LogicalExpression, "BinaryArithmeticExpression must not be a LogicalExpression");
59+
}
60+
}

hierarchical-codemodel/src/main/java/build/codemodel/hierarchical/descriptor/HierarchicalTypeDescriptor.java

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,10 @@
2626
import build.codemodel.foundation.usage.NamedTypeUsage;
2727
import build.codemodel.hierarchical.HierarchicalCodeModel;
2828

29+
import java.util.HashSet;
2930
import java.util.LinkedHashSet;
3031
import java.util.Optional;
32+
import java.util.Set;
3133
import java.util.function.Predicate;
3234
import java.util.stream.Collectors;
3335
import java.util.stream.Stream;
@@ -129,11 +131,22 @@ default <T> Stream<T> parents(final Class<T> typeDescriptorClass) {
129131
* @return the level of the {@link HierarchicalTypeDescriptor}
130132
*/
131133
default int level() {
132-
return parents()
133-
.map(HierarchicalTypeDescriptor::level)
134-
.max(Integer::compareTo)
135-
.map(level -> level + 1)
136-
.orElse(0);
134+
return level(new HashSet<>());
135+
}
136+
137+
private int level(final Set<TypeName> visited) {
138+
if (!visited.add(typeName())) {
139+
throw new IllegalStateException("Cycle detected in type hierarchy at: " + typeName());
140+
}
141+
try {
142+
return parents()
143+
.map(p -> p.level(visited))
144+
.max(Integer::compareTo)
145+
.map(level -> level + 1)
146+
.orElse(0);
147+
} finally {
148+
visited.remove(typeName());
149+
}
137150
}
138151

139152
/**
@@ -200,26 +213,37 @@ default <T> Stream<T> ancestors(final Class<T> typeDescriptorClass) {
200213
* @return the {@link Optional}ly found <i>first</i> <i>ancestor</i>, otherwise {@link Optional#empty()}
201214
*/
202215
default Optional<HierarchicalTypeDescriptor> getAncestor(final Predicate<? super HierarchicalTypeDescriptor> predicate) {
216+
return getAncestor(predicate, new HashSet<>());
217+
}
203218

219+
private Optional<HierarchicalTypeDescriptor> getAncestor(final Predicate<? super HierarchicalTypeDescriptor> predicate,
220+
final Set<TypeName> visited) {
204221
if (!hasParents()) {
205222
return Optional.empty();
206223
}
207224

208-
// first attempt to locate a parent that satisfies the Predicate
209-
final var optional = parents()
210-
.filter(predicate)
211-
.findFirst();
212-
213-
if (optional.isPresent()) {
214-
return optional;
225+
if (!visited.add(typeName())) {
226+
throw new IllegalStateException("Cycle detected in type hierarchy at: " + typeName());
227+
}
228+
try {
229+
// first attempt to locate a parent that satisfies the Predicate
230+
final var optional = parents()
231+
.filter(predicate)
232+
.findFirst();
233+
234+
if (optional.isPresent()) {
235+
return optional;
236+
}
237+
238+
// now attempt to locate a parent of the parent that satisfies the Predicate
239+
return parents()
240+
.map(parent -> parent.getAncestor(predicate, visited))
241+
.filter(Optional::isPresent)
242+
.map(Optional::orElseThrow)
243+
.findFirst();
244+
} finally {
245+
visited.remove(typeName());
215246
}
216-
217-
// now attempt to locate a parent of the parent that satisfies the Predicate
218-
return parents()
219-
.map(parent -> parent.getAncestor(predicate))
220-
.filter(Optional::isPresent)
221-
.map(Optional::orElseThrow)
222-
.findFirst();
223247
}
224248

225249
/**

hierarchical-codemodel/src/test/java/build/codemodel/hierarchical/HierarchicalCodeModelTests.java

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import java.util.stream.Stream;
1818

1919
import static org.assertj.core.api.Assertions.assertThat;
20+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2021

2122
/**
2223
* Tests for {@link HierarchicalCodeModel}s.
@@ -696,4 +697,57 @@ private void assertIsOnlyAssignableTo(final HierarchicalCodeModel codeModel,
696697
() -> "Type: " + fromEncodedTypeName + " is assignable to type: " + toTypeName)
697698
.isFalse());
698699
}
700+
701+
/**
702+
* Ensure that {@link HierarchicalTypeDescriptor#level()} throws {@link IllegalStateException}
703+
* when a cycle exists in the type hierarchy instead of causing a {@link StackOverflowError}.
704+
*/
705+
@Test
706+
void levelShouldThrowOnCyclicHierarchy() {
707+
final var codeModel = createEmptyCodeModel();
708+
final var nameProvider = codeModel.getNameProvider();
709+
710+
final var nameA = nameProvider.getTypeName("CycleA");
711+
final var nameB = nameProvider.getTypeName("CycleB");
712+
713+
final var usageA = SpecificTypeUsage.of(codeModel, nameA);
714+
final var usageB = SpecificTypeUsage.of(codeModel, nameB);
715+
716+
final var descriptorA = codeModel.createTypeDescriptor(nameA, (_, n) -> new ClassTypeDescriptor(codeModel, n));
717+
final var descriptorB = codeModel.createTypeDescriptor(nameB, (_, n) -> new ClassTypeDescriptor(codeModel, n));
718+
719+
// A extends B and B extends A — a direct cycle
720+
descriptorA.addTrait(Extends.of(usageB));
721+
descriptorB.addTrait(Extends.of(usageA));
722+
723+
assertThatThrownBy(descriptorA::level)
724+
.isInstanceOf(IllegalStateException.class)
725+
.hasMessageContaining("Cycle");
726+
}
727+
728+
/**
729+
* Ensure that {@link HierarchicalTypeDescriptor#getAncestor} throws {@link IllegalStateException}
730+
* when a cycle exists in the type hierarchy instead of causing a {@link StackOverflowError}.
731+
*/
732+
@Test
733+
void getAncestorShouldThrowOnCyclicHierarchy() {
734+
final var codeModel = createEmptyCodeModel();
735+
final var nameProvider = codeModel.getNameProvider();
736+
737+
final var nameA = nameProvider.getTypeName("AncCycleA");
738+
final var nameB = nameProvider.getTypeName("AncCycleB");
739+
740+
final var usageA = SpecificTypeUsage.of(codeModel, nameA);
741+
final var usageB = SpecificTypeUsage.of(codeModel, nameB);
742+
743+
final var descriptorA = codeModel.createTypeDescriptor(nameA, (_, n) -> new ClassTypeDescriptor(codeModel, n));
744+
final var descriptorB = codeModel.createTypeDescriptor(nameB, (_, n) -> new ClassTypeDescriptor(codeModel, n));
745+
746+
descriptorA.addTrait(Extends.of(usageB));
747+
descriptorB.addTrait(Extends.of(usageA));
748+
749+
assertThatThrownBy(() -> descriptorA.getAncestor(_ -> false))
750+
.isInstanceOf(IllegalStateException.class)
751+
.hasMessageContaining("Cycle");
752+
}
699753
}

jdk-codemodel/src/main/java/build/codemodel/jdk/JDKCodeModel.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -250,10 +250,19 @@ else if (type instanceof TypeVariable<?> typeVariable) {
250250
typeUsage = TypeVariableUsage.of(this, typeName, Optional.empty(), upperBound);
251251
}
252252
else if (type instanceof WildcardType wildcardType) {
253+
final java.lang.reflect.Type[] lowers = wildcardType.getLowerBounds();
254+
final java.lang.reflect.Type[] uppers = wildcardType.getUpperBounds();
253255

254-
// TODO: determine lower and upper bounds from the WildcardType?
256+
final Optional<Lazy<TypeUsage>> optLower = lowers.length > 0
257+
? Optional.of(Lazy.of(getTypeUsage(lowers[0])))
258+
: Optional.empty();
255259

256-
typeUsage = WildcardTypeUsage.create(this);
260+
// getUpperBounds() returns [Object] for unbounded `?` — treat that as no explicit upper bound
261+
final Optional<Lazy<TypeUsage>> optUpper = uppers.length > 0 && !uppers[0].equals(Object.class)
262+
? Optional.of(Lazy.of(getTypeUsage(uppers[0])))
263+
: Optional.empty();
264+
265+
typeUsage = WildcardTypeUsage.of(this, optLower, optUpper);
257266
}
258267
else if (type instanceof GenericArrayType genericArrayType) {
259268
typeUsage = ArrayTypeUsage.of(

0 commit comments

Comments
 (0)