Merge pull request #190 from jglick/DataBoundConstructor-dupe

Improve message regarding duplicated annotated constructors (II)
jenkinsci · Jun 12, 2020 · 57a9352 · 57a9352
2 parents 678b588 + b89eea1
commit 57a9352
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 4 deletions.
diff --git a/core/src/main/java/org/kohsuke/stapler/jsr269/ConstructorProcessor.java b/core/src/main/java/org/kohsuke/stapler/jsr269/ConstructorProcessor.java
@@ -14,6 +14,7 @@
 import javax.lang.model.element.VariableElement;
 import javax.lang.model.util.ElementScanner6;
 import java.io.IOException;
+import java.util.HashSet;
 import java.util.Properties;
 import java.util.Set;
 import javax.lang.model.element.Modifier;
@@ -26,18 +27,23 @@
 @SupportedAnnotationTypes("*")
 @MetaInfServices(Processor.class)
 public class ConstructorProcessor extends AbstractProcessorImpl {
+   /* private */ final static String MESSAGE = "Only one annotated constructor (@DataBoundConstructor annotation or @stapler-constructor javadoc) is permitted per class";
+
     @Override
     public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment roundEnv) {
         try {
             ElementScanner6<Void, Void> scanner = new ElementScanner6<Void, Void>() {
+                Set<Element> enclosingElementsWritten = new HashSet<>();
+                boolean messagePrinted;
+
                 @Override
                 public Void visitExecutable(ExecutableElement e, Void aVoid) {
                     if(e.getAnnotation(DataBoundConstructor.class)!=null) {
-                        write(e);
+                        writeOrAddOnlyOneMessage(e);
                     } else {
                         String javadoc = getJavadoc(e);
                         if(javadoc!=null && javadoc.contains("@stapler-constructor")) {
-                            write(e);
+                            writeOrAddOnlyOneMessage(e);
                         }
                     }
 
@@ -48,6 +54,15 @@ public Void visitExecutable(ExecutableElement e, Void aVoid) {
                 public Void visitUnknown(Element e, Void aVoid) {
                     return DEFAULT_VALUE;
                 }
+
+                private void writeOrAddOnlyOneMessage(ExecutableElement e) {
+                    if (enclosingElementsWritten.add(e.getEnclosingElement())) {
+                        write(e);
+                    } else if (!messagePrinted){
+                        processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, MESSAGE, e);
+                        messagePrinted = true;
+                    }
+                }
             };
 
             for (Element e : roundEnv.getRootElements()) {

diff --git a/core/src/test/java/org/kohsuke/stapler/jsr269/ConstructorProcessorTest.java b/core/src/test/java/org/kohsuke/stapler/jsr269/ConstructorProcessorTest.java
@@ -81,7 +81,56 @@ public class ConstructorProcessorTest {
         assertTrue(msg, msg.contains("abstract"));
     }
 
-    // TODO nested classes use qualified rather than binary name
-    // TODO behavior when multiple @DataBoundConstructor's specified on a single class - error?
+    //issue-179
+    @Test public void duplicatedConstructor1() {
+        Compilation compilation = new Compilation();
+        compilation.addSource("some.pkg.Stuff").
+                addLine("package some.pkg;").
+                addLine("import org.kohsuke.stapler.DataBoundConstructor;").
+                addLine("public class Stuff {").
+                addLine("  @DataBoundConstructor public Stuff() {}").
+                addLine("  @DataBoundConstructor public Stuff(int i) {}").
+                addLine("}");
+        compilation.doCompile(null, "-source", "6");
+        List<Diagnostic<? extends JavaFileObject>> diagnostics = compilation.getDiagnostics();
+        assertEquals(1, diagnostics.size());
+        String msg = diagnostics.get(0).getMessage(Locale.ENGLISH);
+        assertTrue(msg, msg.contains(ConstructorProcessor.MESSAGE));
+    }
 
+    //issue-179
+    @Test public void duplicatedConstructor2() {
+        Compilation compilation = new Compilation();
+        compilation.addSource("some.pkg.Stuff").
+                addLine("package some.pkg;").
+                addLine("import org.kohsuke.stapler.DataBoundConstructor;").
+                addLine("public class Stuff {").
+                addLine("  @DataBoundConstructor public Stuff() {}").
+                addLine("  /**").
+                addLine("    @stapler-constructor Another constructor").
+                addLine("   **/").
+                addLine("  public Stuff(int i) {}").
+                addLine("}");
+        compilation.doCompile(null, "-source", "6");
+        List<Diagnostic<? extends JavaFileObject>> diagnostics = compilation.getDiagnostics();
+        assertEquals(1, diagnostics.size());
+        String msg = diagnostics.get(0).getMessage(Locale.ENGLISH);
+        assertTrue(msg, msg.contains(ConstructorProcessor.MESSAGE));
+    }
+
+    //issue-179
+    @Test public void duplicatedButNotAnnotatedConstructor() {
+        Compilation compilation = new Compilation();
+        compilation.addSource("some.pkg.Stuff").
+                addLine("package some.pkg;").
+                addLine("import org.kohsuke.stapler.DataBoundConstructor;").
+                addLine("public class Stuff {").
+                addLine("  @DataBoundConstructor public Stuff() {}").
+                addLine("  public Stuff(int i) {}").
+                addLine("}");
+        compilation.doCompile(null, "-source", "6");
+        List<Diagnostic<? extends JavaFileObject>> diagnostics = compilation.getDiagnostics();
+        assertEquals(0, diagnostics.size());
+    }
+    // TODO nested classes use qualified rather than binary name
 }