Skip to content

Commit be54cb4

Browse files
scarytomscarytom
committed
relax the constraints on factory methods to allow for builder-pattern matcher factories that do not initially return a type that extends Matcher.
1 parent 2f76c0f commit be54cb4

File tree

3 files changed

+19
-27
lines changed

3 files changed

+19
-27
lines changed

‎hamcrest-generator/src/main/java/org/hamcrest/generator/ReflectiveFactoryReader.java

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -86,26 +86,18 @@ private boolean outsideArrayBounds() {
8686
* <p>To use another set of rules, override this method.
8787
*/
8888
protected boolean isFactoryMethod(Method javaMethod) {
89-
// We dynamically load these classes, to avoid a compile time
90-
// dependency on org.hamcrest.{Factory,Matcher}. This gets around
91-
// a circular bootstrap issue (because generator is required to
92-
// compile core).
9389
return isStatic(javaMethod.getModifiers())
9490
&& isPublic(javaMethod.getModifiers())
9591
&& hasFactoryAnnotation(javaMethod)
96-
&& matcherClass().isAssignableFrom(javaMethod.getReturnType());
97-
}
98-
99-
private Class<?> matcherClass() {
100-
try {
101-
return classLoader.loadClass("org.hamcrest.Matcher");
102-
} catch (ClassNotFoundException e) {
103-
throw new RuntimeException("Cannot load hamcrest core", e);
104-
}
92+
&& !Void.TYPE.equals(javaMethod.getReturnType());
10593
}
10694

10795
@SuppressWarnings("unchecked")
10896
private boolean hasFactoryAnnotation(Method javaMethod) {
97+
// We dynamically load the Factory class, to avoid a compile time
98+
// dependency on org.hamcrest.Factory. This gets around
99+
// a circular bootstrap issue (because generator is required to
100+
// compile core).
109101
try {
110102
final Class<?> factoryClass = classLoader.loadClass("org.hamcrest.Factory");
111103
if (!Annotation.class.isAssignableFrom(factoryClass)) {
@@ -119,9 +111,9 @@ private boolean hasFactoryAnnotation(Method javaMethod) {
119111

120112
private static FactoryMethod buildFactoryMethod(Method javaMethod) {
121113
FactoryMethod result = new FactoryMethod(
122-
javaMethod.getDeclaringClass().getName(),
114+
classToString(javaMethod.getDeclaringClass()),
123115
javaMethod.getName(),
124-
javaMethod.getReturnType().getName());
116+
classToString(javaMethod.getReturnType()));
125117

126118
for (TypeVariable<Method> typeVariable : javaMethod.getTypeParameters()) {
127119
boolean hasBound = false;
@@ -175,7 +167,8 @@ private static String typeToString(Type type) {
175167
}
176168

177169
private static String classToString(Class<?> cls) {
178-
return cls.isArray() ? cls.getComponentType().getName() + "[]" : cls.getName();
170+
final String name = cls.isArray() ? cls.getComponentType().getName() + "[]" : cls.getName();
171+
return name.replace('$', '.');
179172
}
180173

181-
}
174+
}

‎hamcrest-unit-test/src/main/java/org/hamcrest/generator/ReflectiveFactoryReaderTest.java

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,12 @@ public void testIteratesOverFactoryMethods() {
3737
assertTrue("Expected first method", methods.hasNext());
3838
FactoryMethod firstMethod = methods.next();
3939
assertEquals("firstMethod", firstMethod.getName());
40-
assertEquals(SimpleSetOfMatchers.class.getName(), firstMethod.getMatcherClass());
40+
assertEquals(SimpleSetOfMatchers.class.getName().replace('$', '.'), firstMethod.getMatcherClass());
4141

4242
assertTrue("Expected second method", methods.hasNext());
4343
FactoryMethod secondMethod = methods.next();
4444
assertEquals("secondMethod", secondMethod.getName());
45-
assertEquals(SimpleSetOfMatchers.class.getName(), secondMethod.getMatcherClass());
45+
assertEquals(SimpleSetOfMatchers.class.getName().replace('$', '.'), secondMethod.getMatcherClass());
4646

4747
assertFalse("Expected no more methods", methods.hasNext());
4848
}
@@ -69,18 +69,17 @@ public static Matcher<String> goodMethod() {
6969
}
7070

7171
@Factory
72-
public static Matcher<String> anotherGoodMethod() {
72+
public static String anotherGoodMethod() {
7373
return null;
7474
}
7575

7676
@Factory
77-
public static String wrongReturnType() {
78-
return null;
77+
public static void wrongReturnType() {
7978
}
8079

8180
}
8281

83-
public void testOnlyReadsPublicStaticAnnotatedMethodsThatReturnMatcher() {
82+
public void testOnlyReadsPublicStaticAnnotatedMethodsThatReturnNonVoid() {
8483
Iterable<FactoryMethod> reader = new ReflectiveFactoryReader(MatchersWithDodgySignatures.class);
8584
Iterator<FactoryMethod> methods = reader.iterator();
8685

@@ -267,4 +266,4 @@ private static FactoryMethod readMethod(Class<?> cls, String methodName) {
267266
return null;
268267
}
269268

270-
}
269+
}

‎hamcrest-unit-test/src/main/java/org/hamcrest/generator/config/XmlConfiguratorTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,11 @@ public void testAddsMatcherFactoryMethodsToConfiguration() throws Exception {
3636
"</matchers>"));
3737

3838
assertThat(sugarConfiguration.factoryMethods(),
39-
hasItem(new FactoryMethod(SomeMatcher.class.getName(), "matcher1", "org.hamcrest.Matcher")));
39+
hasItem(new FactoryMethod(SomeMatcher.class.getName().replace('$', '.'), "matcher1", "org.hamcrest.Matcher")));
4040
assertThat(sugarConfiguration.factoryMethods(),
41-
hasItem(new FactoryMethod(SomeMatcher.class.getName(), "matcher2", "org.hamcrest.Matcher")));
41+
hasItem(new FactoryMethod(SomeMatcher.class.getName().replace('$', '.'), "matcher2", "org.hamcrest.Matcher")));
4242
assertThat(sugarConfiguration.factoryMethods(),
43-
hasItem(new FactoryMethod(AnotherMatcher.class.getName(), "matcher3", "org.hamcrest.CombinableMatcher")));
43+
hasItem(new FactoryMethod(AnotherMatcher.class.getName().replace('$', '.'), "matcher3", "org.hamcrest.CombinableMatcher")));
4444
}
4545

4646
private static InputSource createXml(String xml) {

0 commit comments

Comments
 (0)