From a108db202ccbfc28a34578553b90a01b720d77ca Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Fri, 9 Feb 2018 18:16:23 +0100 Subject: [PATCH 01/14] feat: add metho CtMethod#getTopDefinitions --- .../spoon/reflect/declaration/CtMethod.java | 9 ++++ .../reflect/declaration/CtMethodImpl.java | 31 +++++++++++++ .../java/spoon/test/filters/FilterTest.java | 46 +++++++++++++++++++ .../filters/testclasses/AbstractTostada.java | 4 ++ .../test/filters/testclasses/ITostada.java | 4 +- .../test/filters/testclasses/Tostada.java | 17 ++++++- 6 files changed, 109 insertions(+), 2 deletions(-) diff --git a/src/main/java/spoon/reflect/declaration/CtMethod.java b/src/main/java/spoon/reflect/declaration/CtMethod.java index c1978276845..ee039ec2466 100644 --- a/src/main/java/spoon/reflect/declaration/CtMethod.java +++ b/src/main/java/spoon/reflect/declaration/CtMethod.java @@ -19,6 +19,8 @@ import spoon.reflect.annotations.PropertyGetter; import spoon.reflect.annotations.PropertySetter; +import java.util.Collection; + import static spoon.reflect.path.CtRole.IS_DEFAULT; @@ -50,4 +52,11 @@ public interface CtMethod extends CtExecutable, CtTypeMember, CtFormalType @Override CtMethod clone(); + + /** + * Returns the top-most methods in the hierarchy defining this method + * (in super class and super interfaces). + * Returns the empty collection if defined here for the first time. + */ + Collection> getTopDefinitions(); } diff --git a/src/main/java/spoon/support/reflect/declaration/CtMethodImpl.java b/src/main/java/spoon/support/reflect/declaration/CtMethodImpl.java index b4e5d1a9672..a46f8011891 100644 --- a/src/main/java/spoon/support/reflect/declaration/CtMethodImpl.java +++ b/src/main/java/spoon/support/reflect/declaration/CtMethodImpl.java @@ -27,11 +27,13 @@ import spoon.reflect.path.CtRole; import spoon.reflect.reference.CtTypeReference; import spoon.reflect.visitor.CtVisitor; +import spoon.reflect.visitor.filter.AllTypeMembersFunction; import spoon.support.reflect.CtExtendedModifier; import spoon.support.reflect.CtModifierHandler; import spoon.support.visitor.ClassTypingContext; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.Set; @@ -217,6 +219,35 @@ public CtMethod clone() { return (CtMethod) super.clone(); } + @Override + public Collection> getTopDefinitions() { + List> s = new ArrayList<>(); + + // first collect potential declarations of this method in the type hierarchy + ClassTypingContext context = new ClassTypingContext(this.getDeclaringType()); + for (Object m : getDeclaringType().map(new AllTypeMembersFunction(CtMethod.class)).list()) { + if (m != this && context.isOverriding(this, (CtMethod) m)) { + s.add((CtMethod) m); + } + } + + // now removing the intermediate methods for which there exists a definition upper in the hierarchy + List> finalMeths = new ArrayList<>(s); + for (CtMethod m1 : s) { + ClassTypingContext context2 = new ClassTypingContext(m1.getDeclaringType()); + boolean m1IsIntermediate = false; + for (CtMethod m2 : s) { + if (context2.isOverriding(m1, m2)) { + m1IsIntermediate = true; + } + } + if (!m1IsIntermediate) { + finalMeths.add(m1); + } + } + return finalMeths; + } + @Override public boolean isPublic() { return this.modifierHandler.isPublic(); diff --git a/src/test/java/spoon/test/filters/FilterTest.java b/src/test/java/spoon/test/filters/FilterTest.java index de6d05474fe..c9e526526ca 100644 --- a/src/test/java/spoon/test/filters/FilterTest.java +++ b/src/test/java/spoon/test/filters/FilterTest.java @@ -12,10 +12,13 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; +import java.util.Comparator; import java.util.Iterator; import java.util.List; import java.util.TreeSet; import java.util.function.Predicate; +import java.util.stream.Collectors; import org.junit.Before; import org.junit.Test; @@ -74,6 +77,7 @@ import spoon.reflect.visitor.filter.ReturnOrThrowFilter; import spoon.reflect.visitor.filter.TypeFilter; import spoon.support.comparator.DeepRepresentationComparator; +import spoon.support.comparator.QualifiedNameComparator; import spoon.support.reflect.declaration.CtMethodImpl; import spoon.support.visitor.SubInheritanceHierarchyResolver; import spoon.test.filters.testclasses.AbstractTostada; @@ -382,6 +386,48 @@ public void testOverriddenMethodsFromSubClassOfAbstractClass() throws Exception assertEquals(Tostada.class, overridenMethodsFromSub.get(1).getParent(CtClass.class).getActualClass()); } + @Test + public void testgetTopDefinitions() throws Exception { + // contract: getTopDefinitions returns the correct answer + final Launcher launcher = new Launcher(); + launcher.setArgs(new String[] {"--output-type", "nooutput" }); + launcher.addInputResource("./src/test/java/spoon/test/filters/testclasses"); + launcher.run(); + + // start with ToStatda + final CtClass aTostada = launcher.getFactory().Class().get(Tostada.class); + List> methods; + + methods = orderByName(aTostada.getMethodsByName("make").get(0).getTopDefinitions()); + assertEquals(2, methods.size()); + assertEquals("AbstractTostada", methods.get(0).getDeclaringType().getSimpleName()); + assertEquals("ITostada", methods.get(1).getDeclaringType().getSimpleName()); + + methods = orderByName(aTostada.getMethodsByName("prepare").get(0).getTopDefinitions()); + assertEquals(1, methods.size()); + assertEquals("AbstractTostada", methods.get(0).getDeclaringType().getSimpleName()); + + methods = orderByName(aTostada.getMethodsByName("toString").get(0).getTopDefinitions()); + assertEquals(1, methods.size()); + assertEquals("Object", methods.get(0).getDeclaringType().getSimpleName()); + + methods = orderByName(aTostada.getMethodsByName("honey").get(0).getTopDefinitions()); + assertEquals(2, methods.size()); + assertEquals("AbstractTostada", methods.get(0).getDeclaringType().getSimpleName()); + assertEquals("Honey", methods.get(1).getDeclaringType().getSimpleName()); + } + + private List> orderByName(Collection> meths) { + List> ordered = new ArrayList<>(meths); + ordered.sort(new Comparator>() { + @Override + public int compare(CtMethod o1, CtMethod o2) { + return o1.getParent(CtType.class).getQualifiedName().compareTo(o2.getParent(CtType.class).getQualifiedName()); + } + }); + return ordered; + } + @Test public void testOverriddenMethodFromInterface() throws Exception { // contract: When we declare a method in an interface, we must return an empty list diff --git a/src/test/java/spoon/test/filters/testclasses/AbstractTostada.java b/src/test/java/spoon/test/filters/testclasses/AbstractTostada.java index b1745ce600e..9b608326a9c 100644 --- a/src/test/java/spoon/test/filters/testclasses/AbstractTostada.java +++ b/src/test/java/spoon/test/filters/testclasses/AbstractTostada.java @@ -33,4 +33,8 @@ public ITostada make() { } public abstract void prepare(); + + public void honey() { + + } } diff --git a/src/test/java/spoon/test/filters/testclasses/ITostada.java b/src/test/java/spoon/test/filters/testclasses/ITostada.java index d5ea1eedb1f..c220ba28048 100644 --- a/src/test/java/spoon/test/filters/testclasses/ITostada.java +++ b/src/test/java/spoon/test/filters/testclasses/ITostada.java @@ -16,6 +16,8 @@ */ package spoon.test.filters.testclasses; -public interface ITostada { +public interface ITostada extends Foo { ITostada make(); } +interface Foo { +} \ No newline at end of file diff --git a/src/test/java/spoon/test/filters/testclasses/Tostada.java b/src/test/java/spoon/test/filters/testclasses/Tostada.java index e95107f5625..3a2c6a04781 100644 --- a/src/test/java/spoon/test/filters/testclasses/Tostada.java +++ b/src/test/java/spoon/test/filters/testclasses/Tostada.java @@ -17,7 +17,8 @@ package spoon.test.filters.testclasses; -public class Tostada extends AbstractTostada { + +public class Tostada extends AbstractTostada implements Honey { @Override public ITostada make() { return new Tostada() { @@ -32,4 +33,18 @@ public void prepare() { @Override public void prepare() { } + + @Override + public String toString() { + return ""; + } + + @Override + public void honey() { + + } +} + +interface Honey { + void honey(); } From 35cd6f119c3df0a4fbc815fa235cc594c2ae8c94 Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Wed, 14 Feb 2018 22:13:13 +0100 Subject: [PATCH 02/14] up --- .../java/spoon/support/reflect/declaration/CtMethodImpl.java | 3 +-- src/test/java/spoon/test/filters/FilterTest.java | 5 ++++- src/test/java/spoon/test/filters/testclasses/Tostada.java | 2 ++ 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/main/java/spoon/support/reflect/declaration/CtMethodImpl.java b/src/main/java/spoon/support/reflect/declaration/CtMethodImpl.java index a46f8011891..4fccb2f7cc5 100644 --- a/src/main/java/spoon/support/reflect/declaration/CtMethodImpl.java +++ b/src/main/java/spoon/support/reflect/declaration/CtMethodImpl.java @@ -234,10 +234,9 @@ public Collection> getTopDefinitions() { // now removing the intermediate methods for which there exists a definition upper in the hierarchy List> finalMeths = new ArrayList<>(s); for (CtMethod m1 : s) { - ClassTypingContext context2 = new ClassTypingContext(m1.getDeclaringType()); boolean m1IsIntermediate = false; for (CtMethod m2 : s) { - if (context2.isOverriding(m1, m2)) { + if (context.isOverriding(m1, m2)) { m1IsIntermediate = true; } } diff --git a/src/test/java/spoon/test/filters/FilterTest.java b/src/test/java/spoon/test/filters/FilterTest.java index c9e526526ca..66a5926e388 100644 --- a/src/test/java/spoon/test/filters/FilterTest.java +++ b/src/test/java/spoon/test/filters/FilterTest.java @@ -388,7 +388,7 @@ public void testOverriddenMethodsFromSubClassOfAbstractClass() throws Exception @Test public void testgetTopDefinitions() throws Exception { - // contract: getTopDefinitions returns the correct answer + // contract: getTopDefinitions returns the correct number of definitions final Launcher launcher = new Launcher(); launcher.setArgs(new String[] {"--output-type", "nooutput" }); launcher.addInputResource("./src/test/java/spoon/test/filters/testclasses"); @@ -415,6 +415,9 @@ public void testgetTopDefinitions() throws Exception { assertEquals(2, methods.size()); assertEquals("AbstractTostada", methods.get(0).getDeclaringType().getSimpleName()); assertEquals("Honey", methods.get(1).getDeclaringType().getSimpleName()); + + methods = orderByName(aTostada.getMethodsByName("foo").get(0).getTopDefinitions()); + assertEquals(0, methods.size()); } private List> orderByName(Collection> meths) { diff --git a/src/test/java/spoon/test/filters/testclasses/Tostada.java b/src/test/java/spoon/test/filters/testclasses/Tostada.java index 3a2c6a04781..7dea5b28712 100644 --- a/src/test/java/spoon/test/filters/testclasses/Tostada.java +++ b/src/test/java/spoon/test/filters/testclasses/Tostada.java @@ -43,6 +43,8 @@ public String toString() { public void honey() { } + + public void foo() {} } interface Honey { From 08c2eefa2e35f959e56c540429aaa240655c0612 Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Fri, 12 Jan 2018 15:53:45 +0100 Subject: [PATCH 03/14] documentedTest --- .../SpoonArchitectureEnforcerTest.java | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java b/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java index ae083e71318..9e4a7e7cca1 100644 --- a/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java +++ b/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java @@ -23,7 +23,6 @@ import spoon.test.metamodel.SpoonMetaModel; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.HashSet; import java.util.List; @@ -62,6 +61,23 @@ public boolean matches(CtType element) { } + @Test + public void documentedTest() throws Exception { + // the public methods should be documented + Launcher spoon = new Launcher(); + spoon.getEnvironment().setCommentEnabled(true); + spoon.addInputResource("src/main/java/"); + spoon.buildModel(); + + for (CtMethod t : spoon.getFactory().Package().getRootPackage().filterChildren( x -> x instanceof CtMethod).list(CtMethod.class)) { + if (t.hasModifier(ModifierKind.PUBLIC) && t.getDocComment() != null && t.getDocComment().length()<10) { + fail("no documentation for public methods " + t.getSimpleName()); + } + } + + } + + @Test public void testFactorySubFactory() throws Exception { // contract:: all subfactory methods must also be in the main factory From 135ef607828c6016e06ee319710a20964137e2a1 Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Fri, 12 Jan 2018 15:55:22 +0100 Subject: [PATCH 04/14] Commit by Martin Monperrus on 12 January 2018 --- .../spoon/test/architecture/SpoonArchitectureEnforcerTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java b/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java index 9e4a7e7cca1..dfef573bf4c 100644 --- a/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java +++ b/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java @@ -70,6 +70,7 @@ public void documentedTest() throws Exception { spoon.buildModel(); for (CtMethod t : spoon.getFactory().Package().getRootPackage().filterChildren( x -> x instanceof CtMethod).list(CtMethod.class)) { + // add check if overridden, check if @inheritdoc if (t.hasModifier(ModifierKind.PUBLIC) && t.getDocComment() != null && t.getDocComment().length()<10) { fail("no documentation for public methods " + t.getSimpleName()); } From 7196f6e1391bc74175d6bdc91debb47564e3a85f Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Fri, 12 Jan 2018 22:48:16 +0100 Subject: [PATCH 05/14] up --- .../SpoonArchitectureEnforcerTest.java | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java b/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java index dfef573bf4c..af67b4c17cb 100644 --- a/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java +++ b/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java @@ -6,6 +6,7 @@ import spoon.SpoonAPI; import spoon.processing.AbstractManualProcessor; import spoon.processing.AbstractProcessor; +import spoon.reflect.code.CtCodeElement; import spoon.reflect.code.CtConstructorCall; import spoon.reflect.declaration.CtClass; import spoon.reflect.declaration.CtConstructor; @@ -63,19 +64,39 @@ public boolean matches(CtType element) { @Test public void documentedTest() throws Exception { - // the public methods should be documented + // the public methods should be documented with an API Javadoc comment Launcher spoon = new Launcher(); spoon.getEnvironment().setCommentEnabled(true); spoon.addInputResource("src/main/java/"); spoon.buildModel(); - + List notDocumented = new ArrayList<>(); for (CtMethod t : spoon.getFactory().Package().getRootPackage().filterChildren( x -> x instanceof CtMethod).list(CtMethod.class)) { - // add check if overridden, check if @inheritdoc - if (t.hasModifier(ModifierKind.PUBLIC) && t.getDocComment() != null && t.getDocComment().length()<10) { - fail("no documentation for public methods " + t.getSimpleName()); + if (isCorrectlyDocumented(t) + ) { + notDocumented.add(t.getParent(CtType.class).getQualifiedName()+"#"+t.getSimpleName()); } } + if (!notDocumented.isEmpty()) { + fail("this method should be documented ("+notDocumented.size()+"): "+notDocumented.toString()); + } + + } + private boolean isCorrectlyDocumented(CtMethod t) { + // pb: need getOverriddenMethod in CtMethod + + // pb: need getAllSuperClasses + // need getAllSuperInterfaces + // need getAllSuperTypes + + // pb: bug in getOverridingExecutable + return t.hasModifier(ModifierKind.PUBLIC) + && t.getDocComment().length()<10 // less than 10 characters is indeed small + && !t.getSimpleName().startsWith("get") + && !t.getSimpleName().startsWith("set") + && !t.getSimpleName().startsWith("is") + && t.getReference().getOverridingExecutable() == null // only the top declaration should be documented + && t.filterChildren(new TypeFilter<>(CtCodeElement.class)).list().size()>50; // not a trivial method } From 6e1cbf77b5c2d6e79146d7eccb2cd1801465b4dd Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Wed, 7 Feb 2018 15:54:00 +0100 Subject: [PATCH 06/14] up --- .../test/architecture/SpoonArchitectureEnforcerTest.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java b/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java index af67b4c17cb..0d2bc7cce82 100644 --- a/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java +++ b/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java @@ -95,8 +95,11 @@ private boolean isCorrectlyDocumented(CtMethod t) { && !t.getSimpleName().startsWith("get") && !t.getSimpleName().startsWith("set") && !t.getSimpleName().startsWith("is") - && t.getReference().getOverridingExecutable() == null // only the top declaration should be documented - && t.filterChildren(new TypeFilter<>(CtCodeElement.class)).list().size()>50; // not a trivial method + && t.getTopDefinitions().size() == 0 // only the top declaration should be documented + && (t.hasModifier(ModifierKind.ABSTRACT) + //|| t.filterChildren(new TypeFilter<>(CtCodeElement.class)).list().size()>50 + ) // not a trivial method + ; } From fb921b61f614cba6a74b273d89c28a567468c349 Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Wed, 7 Feb 2018 22:21:27 +0100 Subject: [PATCH 07/14] up --- .../test/architecture/SpoonArchitectureEnforcerTest.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java b/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java index 0d2bc7cce82..c619a9e3e1d 100644 --- a/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java +++ b/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java @@ -95,9 +95,11 @@ private boolean isCorrectlyDocumented(CtMethod t) { && !t.getSimpleName().startsWith("get") && !t.getSimpleName().startsWith("set") && !t.getSimpleName().startsWith("is") - && t.getTopDefinitions().size() == 0 // only the top declaration should be documented + && !t.getSimpleName().startsWith("add") + && !t.getSimpleName().startsWith("remove") + && t.getTopDefinitions().size() == 0 // only the top declarations should be documented && (t.hasModifier(ModifierKind.ABSTRACT) - //|| t.filterChildren(new TypeFilter<>(CtCodeElement.class)).list().size()>50 + || t.filterChildren(new TypeFilter<>(CtCodeElement.class)).list().size()>30 ) // not a trivial method ; } From a9fe48ca43766d321ce52710e0de33e126aa12e2 Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Fri, 16 Feb 2018 17:45:03 +0100 Subject: [PATCH 08/14] up --- .../reflect/visitor/CtAbstractBiScanner.java | 1 + .../SpoonArchitectureEnforcerTest.java | 15 +++++++++------ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/main/java/spoon/reflect/visitor/CtAbstractBiScanner.java b/src/main/java/spoon/reflect/visitor/CtAbstractBiScanner.java index b310f47968b..ea65f7f8019 100644 --- a/src/main/java/spoon/reflect/visitor/CtAbstractBiScanner.java +++ b/src/main/java/spoon/reflect/visitor/CtAbstractBiScanner.java @@ -41,6 +41,7 @@ protected void exit(CtElement e) { public boolean biScan(Collection elements, Collection others) { return biScan(null, elements, others); } + public boolean biScan(CtRole role, Collection elements, Collection others) { if (isNotEqual) { return isNotEqual; diff --git a/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java b/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java index c619a9e3e1d..864d3923093 100644 --- a/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java +++ b/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java @@ -77,7 +77,10 @@ public void documentedTest() throws Exception { } } if (!notDocumented.isEmpty()) { - fail("this method should be documented ("+notDocumented.size()+"): "+notDocumented.toString()); + for (String m : notDocumented) { + System.err.println(m); + } + fail("some methods should be documented ("+notDocumented.size()+")"); } } @@ -90,17 +93,17 @@ private boolean isCorrectlyDocumented(CtMethod t) { // need getAllSuperTypes // pb: bug in getOverridingExecutable - return t.hasModifier(ModifierKind.PUBLIC) - && t.getDocComment().length()<10 // less than 10 characters is indeed small + return t.hasModifier(ModifierKind.PUBLIC) // public methods should be documented + && t.getDocComment().length()<10 // less than 10 characters && !t.getSimpleName().startsWith("get") && !t.getSimpleName().startsWith("set") && !t.getSimpleName().startsWith("is") && !t.getSimpleName().startsWith("add") && !t.getSimpleName().startsWith("remove") && t.getTopDefinitions().size() == 0 // only the top declarations should be documented - && (t.hasModifier(ModifierKind.ABSTRACT) - || t.filterChildren(new TypeFilter<>(CtCodeElement.class)).list().size()>30 - ) // not a trivial method + && (t.hasModifier(ModifierKind.ABSTRACT) // all interface methods should be documented + || t.filterChildren(new TypeFilter<>(CtCodeElement.class)).list().size()>40 // trivial methods can be skipped + ) ; } From 9b3a64d1b64fad825e7209f2422d7c8eafcd25c1 Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Fri, 16 Feb 2018 19:16:23 +0100 Subject: [PATCH 09/14] up --- .../SpoonArchitectureEnforcerTest.java | 28 ++++++++----------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java b/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java index 864d3923093..421a459bcd8 100644 --- a/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java +++ b/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java @@ -64,16 +64,16 @@ public boolean matches(CtType element) { @Test public void documentedTest() throws Exception { - // the public methods should be documented with an API Javadoc comment + // contract: all non-trivial public methods should be documented with API Javadoc Launcher spoon = new Launcher(); spoon.getEnvironment().setCommentEnabled(true); spoon.addInputResource("src/main/java/"); spoon.buildModel(); List notDocumented = new ArrayList<>(); - for (CtMethod t : spoon.getFactory().Package().getRootPackage().filterChildren( x -> x instanceof CtMethod).list(CtMethod.class)) { - if (isCorrectlyDocumented(t) + for (CtMethod method : spoon.getFactory().Package().getRootPackage().filterChildren( x -> x instanceof CtMethod).list(CtMethod.class)) { + if (isNotDocumentedWhildShouldBe(method) ) { - notDocumented.add(t.getParent(CtType.class).getQualifiedName()+"#"+t.getSimpleName()); + notDocumented.add(method.getParent(CtType.class).getQualifiedName()+"#"+method.getSimpleName()); } } if (!notDocumented.isEmpty()) { @@ -85,26 +85,20 @@ public void documentedTest() throws Exception { } - private boolean isCorrectlyDocumented(CtMethod t) { - // pb: need getOverriddenMethod in CtMethod - - // pb: need getAllSuperClasses - // need getAllSuperInterfaces - // need getAllSuperTypes - - // pb: bug in getOverridingExecutable + private boolean isNotDocumentedWhildShouldBe(CtMethod t) { return t.hasModifier(ModifierKind.PUBLIC) // public methods should be documented - && t.getDocComment().length()<10 // less than 10 characters - && !t.getSimpleName().startsWith("get") + && t.getDocComment().length()<10 // less than 10 characters of Javadoc API, that's too short, sorry! + && !t.getSimpleName().startsWith("get") // setters can be undocumented && !t.getSimpleName().startsWith("set") && !t.getSimpleName().startsWith("is") && !t.getSimpleName().startsWith("add") && !t.getSimpleName().startsWith("remove") && t.getTopDefinitions().size() == 0 // only the top declarations should be documented - && (t.hasModifier(ModifierKind.ABSTRACT) // all interface methods should be documented - || t.filterChildren(new TypeFilter<>(CtCodeElement.class)).list().size()>40 // trivial methods can be skipped + && ( + t.hasModifier(ModifierKind.ABSTRACT) // all interface methods should be documented + || t.filterChildren(new TypeFilter<>(CtCodeElement.class)).list().size()>40 // only large methods should be documented, trivial methods can be skipped ) - ; + ; } From b1364624a46add00773c60def442240fb69fc466 Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Sat, 17 Feb 2018 09:56:36 +0100 Subject: [PATCH 10/14] up --- .../architecture/SpoonArchitectureEnforcerTest.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java b/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java index 421a459bcd8..43c62669338 100644 --- a/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java +++ b/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java @@ -71,7 +71,7 @@ public void documentedTest() throws Exception { spoon.buildModel(); List notDocumented = new ArrayList<>(); for (CtMethod method : spoon.getFactory().Package().getRootPackage().filterChildren( x -> x instanceof CtMethod).list(CtMethod.class)) { - if (isNotDocumentedWhildShouldBe(method) + if (isNotDocumentedWhileItShouldBe(method) ) { notDocumented.add(method.getParent(CtType.class).getQualifiedName()+"#"+method.getSimpleName()); } @@ -80,15 +80,15 @@ public void documentedTest() throws Exception { for (String m : notDocumented) { System.err.println(m); } - fail("some methods should be documented ("+notDocumented.size()+")"); + fail(notDocumented.size()+" public methods should be documented with proper API documentation"); } } - private boolean isNotDocumentedWhildShouldBe(CtMethod t) { + private boolean isNotDocumentedWhileItShouldBe(CtMethod t) { return t.hasModifier(ModifierKind.PUBLIC) // public methods should be documented - && t.getDocComment().length()<10 // less than 10 characters of Javadoc API, that's too short, sorry! - && !t.getSimpleName().startsWith("get") // setters can be undocumented + && t.getDocComment().length()<5 // the Javadoc is too short, sorry! + && !t.getSimpleName().startsWith("get") // all kinds of setters can be undocumented && !t.getSimpleName().startsWith("set") && !t.getSimpleName().startsWith("is") && !t.getSimpleName().startsWith("add") @@ -96,7 +96,7 @@ private boolean isNotDocumentedWhildShouldBe(CtMethod t) { && t.getTopDefinitions().size() == 0 // only the top declarations should be documented && ( t.hasModifier(ModifierKind.ABSTRACT) // all interface methods should be documented - || t.filterChildren(new TypeFilter<>(CtCodeElement.class)).list().size()>40 // only large methods should be documented, trivial methods can be skipped + || t.filterChildren(new TypeFilter<>(CtCodeElement.class)).list().size()>35 // only large methods should be documented, trivial methods can be skipped ) ; } From 8a121a09517d89e2393db26b5d67ac658ea710c9 Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Sat, 17 Feb 2018 10:20:01 +0100 Subject: [PATCH 11/14] up --- .../SpoonArchitectureEnforcerTest.java | 66 ++++++++++--------- 1 file changed, 36 insertions(+), 30 deletions(-) diff --git a/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java b/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java index 43c62669338..c5e83deb1a2 100644 --- a/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java +++ b/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java @@ -62,41 +62,29 @@ public boolean matches(CtType element) { } - @Test - public void documentedTest() throws Exception { - // contract: all non-trivial public methods should be documented with API Javadoc - Launcher spoon = new Launcher(); - spoon.getEnvironment().setCommentEnabled(true); - spoon.addInputResource("src/main/java/"); - spoon.buildModel(); - List notDocumented = new ArrayList<>(); - for (CtMethod method : spoon.getFactory().Package().getRootPackage().filterChildren( x -> x instanceof CtMethod).list(CtMethod.class)) { - if (isNotDocumentedWhileItShouldBe(method) - ) { - notDocumented.add(method.getParent(CtType.class).getQualifiedName()+"#"+method.getSimpleName()); - } - } - if (!notDocumented.isEmpty()) { - for (String m : notDocumented) { - System.err.println(m); - } - fail(notDocumented.size()+" public methods should be documented with proper API documentation"); - } - - } - private boolean isNotDocumentedWhileItShouldBe(CtMethod t) { return t.hasModifier(ModifierKind.PUBLIC) // public methods should be documented - && t.getDocComment().length()<5 // the Javadoc is too short, sorry! + && t.getDocComment().length()<1 // the Javadoc is too short, sorry! && !t.getSimpleName().startsWith("get") // all kinds of setters can be undocumented && !t.getSimpleName().startsWith("set") && !t.getSimpleName().startsWith("is") && !t.getSimpleName().startsWith("add") && !t.getSimpleName().startsWith("remove") - && t.getTopDefinitions().size() == 0 // only the top declarations should be documented + && t.getTopDefinitions().size() == 0 // only the top declarations should be documented (not the overriding methods which are lower in the hierarchy) */ && ( - t.hasModifier(ModifierKind.ABSTRACT) // all interface methods should be documented - || t.filterChildren(new TypeFilter<>(CtCodeElement.class)).list().size()>35 // only large methods should be documented, trivial methods can be skipped + t.hasModifier(ModifierKind.ABSTRACT) // all interface methods and abstract class methods must be documented + + // GOOD FIRST ISSUE + // ideally we want that **all** public methods are documented + // so far, we have this arbitrary limit in the condition below (35) + // because it's a huge task to document everything at once + // so to contribute to Spoon, what you can do is + // 1) you lower the threshold (eg 33) + // 2) you run test `documentedTest`, it will output a list on undocumented methods + // 3) you document those methods + // 4) you run the test again to check that it passes + // 4) you commit your changes and create the corresponding pull requests + || t.filterChildren(new TypeFilter<>(CtCodeElement.class)).list().size()>35 // means that only large methods must be documented ) ; } @@ -153,13 +141,31 @@ public void process() { assertTrue(sanityCheck.val > 100); } + // this test contains all the architectural rules that are valid for the whole src/main/java + // we put them in the same test in order to only build the full moedl once @Test - public void noTreeSetInSpoon() throws Exception { - // we don't use TreeSet, because they implicitly depend on Comparable (no static check, only dynamic checks) - SpoonAPI spoon = new Launcher(); + public void testSrcMainJava() throws Exception { + Launcher spoon = new Launcher(); + spoon.getEnvironment().setCommentEnabled(true); spoon.addInputResource("src/main/java/"); + + // contract: all non-trivial public methods should be documented with proper API Javadoc spoon.buildModel(); + List notDocumented = new ArrayList<>(); + for (CtMethod method : spoon.getFactory().Package().getRootPackage().filterChildren( x -> x instanceof CtMethod).list(CtMethod.class)) { + if (isNotDocumentedWhileItShouldBe(method)) { + notDocumented.add(method.getParent(CtType.class).getQualifiedName()+"#"+method.getSignature()); + } + } + if (notDocumented.size() > 0) { + for (String m : notDocumented) { + System.err.println(m); + } + fail(notDocumented.size()+" public methods should be documented with proper API documentation"); + } + + // contract: Spoon's code never uses TreeSet constructor, because they implicitly depend on Comparable (no static check, only dynamic checks) List treeSetWithoutComparators = spoon.getFactory().Package().getRootPackage().filterChildren(new AbstractFilter() { @Override public boolean matches(CtConstructorCall element) { From 3d8f78b5b0b456f40db742c4427c7deb6600f5c5 Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Sat, 17 Feb 2018 13:58:46 +0100 Subject: [PATCH 12/14] up --- .../test/architecture/SpoonArchitectureEnforcerTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java b/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java index c5e83deb1a2..6f7be49b797 100644 --- a/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java +++ b/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java @@ -142,7 +142,7 @@ public void process() { } // this test contains all the architectural rules that are valid for the whole src/main/java - // we put them in the same test in order to only build the full moedl once + // we put them in the same test in order to only build the full model once @Test public void testSrcMainJava() throws Exception { Launcher spoon = new Launcher(); @@ -152,7 +152,7 @@ public void testSrcMainJava() throws Exception { // contract: all non-trivial public methods should be documented with proper API Javadoc spoon.buildModel(); List notDocumented = new ArrayList<>(); - for (CtMethod method : spoon.getFactory().Package().getRootPackage().filterChildren( x -> x instanceof CtMethod).list(CtMethod.class)) { + for (CtMethod method : spoon.getModel().getElements(new TypeFilter<>(CtMethod.class))) { if (isNotDocumentedWhileItShouldBe(method)) { notDocumented.add(method.getParent(CtType.class).getQualifiedName()+"#"+method.getSignature()); } From 4d836e12bfc3fd390eb8fb38a06027ea024b8b52 Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Sun, 18 Feb 2018 13:51:55 +0100 Subject: [PATCH 13/14] Commit by Martin Monperrus on 18 February 2018 --- doc/architecture_test.md | 29 +++++++++ .../SpoonArchitectureEnforcerTest.java | 65 +++++++++---------- 2 files changed, 61 insertions(+), 33 deletions(-) diff --git a/doc/architecture_test.md b/doc/architecture_test.md index ace30ce14a1..1edcf1527ba 100644 --- a/doc/architecture_test.md +++ b/doc/architecture_test.md @@ -57,3 +57,32 @@ public void testGoodTestClassNames() throws Exception { } } ``` + +Example rule: all public methods must be documented +---------------------------- + +How to check that all public methods of the API contain proper Javadoc? One can also use Spoon to check documentation rules like this one. + +```java +@Test +public void testDocumentation() throws Exception { + SpoonAPI spoon = new Launcher(); + spoon.addInputResource("src/main/java/"); + spoon.buildModel(); + List notDocumented = new ArrayList<>(); + for (CtMethod method : spoon.getModel().getElements(new TypeFilter<>(CtMethod.class))) { + // now we see whether this should be documented + if (method.hasModifier(ModifierKind.PUBLIC) + && method.getTopDefinitions().size() == 0 // optional: only the top declarations should be documented (not the overriding methods which are lower in the hierarchy) + )) { + // is it really well documented? + if (method.getDocComment().length() < 1) { + notDocumented.add(method.getParent(CtType.class).getQualifiedName() + "#" + method.getSignature()); + } + } + } + if (notDocumented.size() > 0) { + fail(notDocumented.size()+" public methods should be documented with proper API documentation: \n"+StringUtils.join(notDocumented, "\n")); + } +} +``` diff --git a/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java b/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java index 6f7be49b797..d690e94df0d 100644 --- a/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java +++ b/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java @@ -62,33 +62,6 @@ public boolean matches(CtType element) { } - private boolean isNotDocumentedWhileItShouldBe(CtMethod t) { - return t.hasModifier(ModifierKind.PUBLIC) // public methods should be documented - && t.getDocComment().length()<1 // the Javadoc is too short, sorry! - && !t.getSimpleName().startsWith("get") // all kinds of setters can be undocumented - && !t.getSimpleName().startsWith("set") - && !t.getSimpleName().startsWith("is") - && !t.getSimpleName().startsWith("add") - && !t.getSimpleName().startsWith("remove") - && t.getTopDefinitions().size() == 0 // only the top declarations should be documented (not the overriding methods which are lower in the hierarchy) */ - && ( - t.hasModifier(ModifierKind.ABSTRACT) // all interface methods and abstract class methods must be documented - - // GOOD FIRST ISSUE - // ideally we want that **all** public methods are documented - // so far, we have this arbitrary limit in the condition below (35) - // because it's a huge task to document everything at once - // so to contribute to Spoon, what you can do is - // 1) you lower the threshold (eg 33) - // 2) you run test `documentedTest`, it will output a list on undocumented methods - // 3) you document those methods - // 4) you run the test again to check that it passes - // 4) you commit your changes and create the corresponding pull requests - || t.filterChildren(new TypeFilter<>(CtCodeElement.class)).list().size()>35 // means that only large methods must be documented - ) - ; - } - @Test public void testFactorySubFactory() throws Exception { @@ -153,15 +126,41 @@ public void testSrcMainJava() throws Exception { spoon.buildModel(); List notDocumented = new ArrayList<>(); for (CtMethod method : spoon.getModel().getElements(new TypeFilter<>(CtMethod.class))) { - if (isNotDocumentedWhileItShouldBe(method)) { - notDocumented.add(method.getParent(CtType.class).getQualifiedName()+"#"+method.getSignature()); + + // now we see whether this should be documented + if (method.hasModifier(ModifierKind.PUBLIC) // public methods should be documented + && !method.getSimpleName().startsWith("get") // all kinds of setters can be undocumented + && !method.getSimpleName().startsWith("set") + && !method.getSimpleName().startsWith("is") + && !method.getSimpleName().startsWith("add") + && !method.getSimpleName().startsWith("remove") + && method.getTopDefinitions().size() == 0 // only the top declarations should be documented (not the overriding methods which are lower in the hierarchy) + && ( + method.hasModifier(ModifierKind.ABSTRACT) // all interface methods and abstract class methods must be documented + + // GOOD FIRST ISSUE + // ideally we want that **all** public methods are documented + // so far, we have this arbitrary limit in the condition below (35) + // because it's a huge task to document everything at once + // so to contribute to Spoon, what you can do is + // 1) you lower the threshold (eg 33) + // 2) you run test `documentedTest`, it will output a list on undocumented methods + // 3) you document those methods + // 4) you run the test again to check that it passes + // 4) you commit your changes and create the corresponding pull requests + || method.filterChildren(new TypeFilter<>(CtCodeElement.class)).list().size() > 35 // means that only large methods must be documented + )) { + + // OK it should be properly documented + + // is it really well documented? + if (method.getDocComment().length() < 1) { // the Javadoc is too short, sorry! + notDocumented.add(method.getParent(CtType.class).getQualifiedName() + "#" + method.getSignature()); + } } } if (notDocumented.size() > 0) { - for (String m : notDocumented) { - System.err.println(m); - } - fail(notDocumented.size()+" public methods should be documented with proper API documentation"); + fail(notDocumented.size()+" public methods should be documented with proper API documentation: \n"+StringUtils.join(notDocumented, "\n")); } From 9d80578de9125655bba270c7cbdbc3451f22ae05 Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Sun, 18 Feb 2018 14:05:00 +0100 Subject: [PATCH 14/14] Commit by Martin Monperrus on 18 February 2018 --- doc/architecture_test.md | 2 +- .../spoon/test/architecture/SpoonArchitectureEnforcerTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/architecture_test.md b/doc/architecture_test.md index 1edcf1527ba..5f5ed6af942 100644 --- a/doc/architecture_test.md +++ b/doc/architecture_test.md @@ -76,7 +76,7 @@ public void testDocumentation() throws Exception { && method.getTopDefinitions().size() == 0 // optional: only the top declarations should be documented (not the overriding methods which are lower in the hierarchy) )) { // is it really well documented? - if (method.getDocComment().length() < 1) { + if (method.getDocComment().length() < 20) { // at least 20 characters notDocumented.add(method.getParent(CtType.class).getQualifiedName() + "#" + method.getSignature()); } } diff --git a/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java b/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java index d690e94df0d..804b4f42ed1 100644 --- a/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java +++ b/src/test/java/spoon/test/architecture/SpoonArchitectureEnforcerTest.java @@ -154,7 +154,7 @@ public void testSrcMainJava() throws Exception { // OK it should be properly documented // is it really well documented? - if (method.getDocComment().length() < 1) { // the Javadoc is too short, sorry! + if (method.getDocComment().length() <= 15) { // the Javadoc must be at least at least 15 characters (still pretty short...) notDocumented.add(method.getParent(CtType.class).getQualifiedName() + "#" + method.getSignature()); } }