Detect subclassing in glue code and report to the user that it's illegal. Closes #301

Author: aslakhellesoy

History.md

@@ -1,5 +1,6 @@
 ## [Git master](https://github.com/cucumber/cucumber-jvm/compare/v1.0.3...master)
 
+* [Java] Detect subclassing in glue code and report to the user that it's illegal. ([#301](https://github.com/cucumber/cucumber-jvm/issues/301) Aslak Hellesøy)
 * [Core] Friendlier error message when XStream fails to assign null to primitive fields ([#296](https://github.com/cucumber/cucumber-jvm/issues/296) Aslak Hellesøy)
 
 ## [1.0.3](https://github.com/cucumber/cucumber-jvm/compare/v1.0.2...1.0.3)

java/src/main/java/cucumber/runtime/java/ClasspathMethodScanner.java

@@ -41,7 +41,7 @@ public void scan(JavaBackend javaBackend, List<String> gluePaths) {
                 }
                 if (glueCodeClass != null) {
                     for (Method method : glueCodeClass.getMethods()) {
-                        scan(javaBackend, method);
+                        scan(javaBackend, method, glueCodeClass);
                     }
                 }
             }
@@ -53,11 +53,18 @@ public void scan(JavaBackend javaBackend, List<String> gluePaths) {
      *
      * @param javaBackend the backend where stepdefs and hooks will be registered
      * @param method      a candidate for being a stepdef or hook
+     * @param glueCodeClass
      */
-    public void scan(JavaBackend javaBackend, Method method) {
+    public void scan(JavaBackend javaBackend, Method method, Class<?> glueCodeClass) {
         for (Class<? extends Annotation> cucumberAnnotationClass : cucumberAnnotationClasses) {
             Annotation annotation = method.getAnnotation(cucumberAnnotationClass);
             if (annotation != null && !annotation.annotationType().equals(Order.class)) {
+                if(!method.getDeclaringClass().isAssignableFrom(glueCodeClass)) {
+                    throw new CucumberException(String.format("%s isn't assignable from %s", method.getDeclaringClass(), glueCodeClass));
+                }
+                if(!glueCodeClass.equals(method.getDeclaringClass())) {
+                    throw new CucumberException(String.format("You're not allowed to extend classes that define Step Definitions or hooks. %s extends %s", glueCodeClass, method.getDeclaringClass()));
+                }
                 if (isHookAnnotation(annotation)) {
                     javaBackend.addHook(annotation, method);
                 } else if (isStepdefAnnotation(annotation)) {

java/src/main/java/cucumber/runtime/java/JavaBackend.java

@@ -64,10 +64,11 @@ public void loadGlue(Glue glue, List<String> gluePaths) {
      *
      * @param glue   where stepdefs and hooks will be added.
      * @param method a candidate method.
+     * @param glueCodeClass the class implementing the method. Must not be a subclass of the class implementing the method.
      */
-    public void loadGlue(Glue glue, Method method) {
+    public void loadGlue(Glue glue, Method method, Class<?> glueCodeClass) {
         this.glue = glue;
-        classpathMethodScanner.scan(this, method);
+        classpathMethodScanner.scan(this, method, glueCodeClass);
     }
 
     @Override

java/src/test/java/cucumber/runtime/java/ClasspathMethodScannerTest.java

@@ -2,11 +2,14 @@
 
 import cucumber.annotation.Before;
 import cucumber.io.ClasspathResourceLoader;
+import cucumber.runtime.CucumberException;
 import cucumber.runtime.Glue;
 import org.junit.Test;
 import org.mockito.Mockito;
 import org.mockito.internal.util.reflection.Whitebox;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
@@ -15,7 +18,6 @@ public class ClasspathMethodScannerTest {
 
     @Test
     public void loadGlue_registers_the_methods_declaring_class_in_the_object_factory() throws NoSuchMethodException {
-
         ClasspathResourceLoader resourceLoader = new ClasspathResourceLoader(Thread.currentThread().getContextClassLoader());
         ClasspathMethodScanner classpathMethodScanner = new ClasspathMethodScanner(resourceLoader);
 
@@ -25,12 +27,34 @@ public void loadGlue_registers_the_methods_declaring_class_in_the_object_factory
         Whitebox.setInternalState(backend, "glue", world);
 
         // this delegates to classpathMethodScanner.scan which we test
-        classpathMethodScanner.scan(backend, BaseStepDefs.class.getMethod("m"));
+        classpathMethodScanner.scan(backend, BaseStepDefs.class.getMethod("m"), BaseStepDefs.class);
 
         verify(factory, times(1)).addClass(BaseStepDefs.class);
         verifyNoMoreInteractions(factory);
     }
 
+    @Test
+    public void loadGlue_fails_when_class_is_not_method_declaring_class() throws NoSuchMethodException {
+        JavaBackend backend = new JavaBackend((ObjectFactory) null);
+        try {
+            backend.loadGlue(null, BaseStepDefs.class.getMethod("m"), Stepdefs2.class);
+            fail();
+        } catch (CucumberException e) {
+            assertEquals("You're not allowed to extend classes that define Step Definitions or hooks. class cucumber.runtime.java.ClasspathMethodScannerTest$Stepdefs2 extends class cucumber.runtime.java.ClasspathMethodScannerTest$BaseStepDefs", e.getMessage());
+        }
+    }
+
+    @Test
+    public void loadGlue_fails_when_class_is_not_subclass_of_declaring_class() throws NoSuchMethodException {
+        JavaBackend backend = new JavaBackend((ObjectFactory) null);
+        try {
+            backend.loadGlue(null, BaseStepDefs.class.getMethod("m"), String.class);
+            fail();
+        } catch (CucumberException e) {
+            assertEquals("class cucumber.runtime.java.ClasspathMethodScannerTest$BaseStepDefs isn't assignable from class java.lang.String", e.getMessage());
+        }
+    }
+
     public static class Stepdefs2 extends BaseStepDefs {
         public interface Interface1 {
         }

java/src/test/java/cucumber/runtime/java/JavaBackendTest.java

@@ -6,7 +6,8 @@
 import cucumber.runtime.HookDefinition;
 import cucumber.runtime.StepDefinition;
 import cucumber.runtime.StepDefinitionMatch;
-import cucumber.runtime.java.test.Stepdefs;
+import cucumber.runtime.java.incorrectlysubclassedstepdefs.SubclassesStepdefs;
+import cucumber.runtime.java.stepdefs.Stepdefs;
 import gherkin.I18n;
 import gherkin.formatter.model.Step;
 import org.junit.Test;
@@ -25,19 +26,27 @@ public void doesnt_like_path_like_glue() {
         ObjectFactory factory = new DefaultJavaObjectFactory();
         JavaBackend backend = new JavaBackend(factory);
         GlueStub world = new GlueStub();
-        backend.loadGlue(world, asList("cucumber/runtime/java/test"));
+        backend.loadGlue(world, asList("cucumber/runtime/java/stepdefs"));
     }
 
     @Test
     public void finds_step_definitions_by_scanning_for_annotations() {
         ObjectFactory factory = new DefaultJavaObjectFactory();
         JavaBackend backend = new JavaBackend(factory);
         GlueStub world = new GlueStub();
-        backend.loadGlue(world, asList("cucumber.runtime.java.test"));
+        backend.loadGlue(world, asList("cucumber.runtime.java.stepdefs"));
         backend.buildWorld();
         assertEquals(Stepdefs.class, factory.getInstance(Stepdefs.class).getClass());
     }
 
+    @Test(expected = CucumberException.class)
+    public void detects_subclassed_glue_and_throws_exception() {
+        ObjectFactory factory = new DefaultJavaObjectFactory();
+        JavaBackend backend = new JavaBackend(factory);
+        GlueStub world = new GlueStub();
+        backend.loadGlue(world, asList("cucumber.runtime.java.stepdefs", "cucumber.runtime.java.incorrectlysubclassedstepdefs"));
+    }
+
     private class GlueStub implements Glue {
         public final List<StepDefinition> stepDefinitions = new ArrayList<StepDefinition>();
 

java/src/test/java/cucumber/runtime/java/JavaStepDefinitionTest.java

@@ -31,7 +31,6 @@
 
 public class JavaStepDefinitionTest {
     private static final List<Comment> NO_COMMENTS = Collections.emptyList();
-    private static final List<String> NO_PATHS = Collections.emptyList();
     private static final Method THREE_DISABLED_MICE;
     private static final Method THREE_BLIND_ANIMALS;
     private static final I18n ENGLISH = new I18n("en");

java/src/test/java/cucumber/runtime/java/incorrectlysubclassedstepdefs/SubclassesStepdefs.java

@@ -0,0 +1,6 @@
+package cucumber.runtime.java.incorrectlysubclassedstepdefs;
+
+import cucumber.runtime.java.stepdefs.Stepdefs;
+
+public class SubclassesStepdefs extends Stepdefs {
+}

java/src/test/java/cucumber/runtime/java/test/Stepdefs.java renamed to java/src/test/java/cucumber/runtime/java/stepdefs/Stepdefs.java

@@ -1,4 +1,4 @@
-package cucumber.runtime.java.test;
+package cucumber.runtime.java.stepdefs;
 
 import cucumber.annotation.en.Given;