From 9b036df587d5d9104123434752ec4e3c61413896 Mon Sep 17 00:00:00 2001 From: Nils Hartmann Date: Wed, 2 Jan 2019 16:45:09 +0100 Subject: [PATCH] #7 Find more kinds of references (Annotations, Type Parameters) WIP --- .../internal/DeptectiveTreeVisitor.java | 110 +++++++++++++--- .../plugintest/basic/BasicPluginTest.java | 124 ++++++++++++++---- .../basic/barclazzan/BarClazzAnnotation.java | 24 ++++ .../basic/barfieldan/BarFieldAnnotation.java | 24 ++++ .../plugintest/basic/bargen/BarGeneric.java | 5 + .../basic/bargentype/BarGenType.java | 5 + .../deptective/plugintest/basic/foo/Foo.java | 13 ++ .../plugintest/basic/foo/FooAnnotation.java | 24 ++++ .../plugintest/basic/foo/FooContainer.java | 5 + 9 files changed, 293 insertions(+), 41 deletions(-) create mode 100644 javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/barclazzan/BarClazzAnnotation.java create mode 100644 javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/barfieldan/BarFieldAnnotation.java create mode 100644 javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/bargen/BarGeneric.java create mode 100644 javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/bargentype/BarGenType.java create mode 100644 javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/foo/FooAnnotation.java create mode 100644 javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/foo/FooContainer.java diff --git a/javac-plugin/src/main/java/org/moditect/deptective/internal/DeptectiveTreeVisitor.java b/javac-plugin/src/main/java/org/moditect/deptective/internal/DeptectiveTreeVisitor.java index 46a61a2..96fcb22 100644 --- a/javac-plugin/src/main/java/org/moditect/deptective/internal/DeptectiveTreeVisitor.java +++ b/javac-plugin/src/main/java/org/moditect/deptective/internal/DeptectiveTreeVisitor.java @@ -21,12 +21,18 @@ import org.moditect.deptective.internal.model.Package; import org.moditect.deptective.internal.model.PackageDependencies; +import com.sun.source.tree.AnnotationTree; +import com.sun.source.tree.AssignmentTree; import com.sun.source.tree.CompilationUnitTree; import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.ParameterizedTypeTree; +import com.sun.source.tree.Tree; +import com.sun.source.tree.TypeParameterTree; import com.sun.source.tree.VariableTree; import com.sun.source.util.JavacTask; import com.sun.source.util.TreePathScanner; import com.sun.tools.javac.api.BasicJavacTask; +import com.sun.tools.javac.code.Type; import com.sun.tools.javac.util.Context; import com.sun.tools.javac.util.Log; @@ -59,31 +65,105 @@ public Void visitCompilationUnit(CompilationUnitTree tree, Void p) { } if (packageOfCurrentCompilationUnit == null) { - throw new IllegalArgumentException("Package " + packageName + " is not configured."); + throw new IllegalArgumentException("Package " +packageName + " is not configured."); } return super.visitCompilationUnit(tree, p); } - @Override + + +// @Override +// public Void visitImport(ImportTree node, Void p) { + // TODO: Deal with "on-demand-imports" (com.foo.*) +// node.getQualifiedIdentifier().accept(new TreeScanner() { +// @Override +// public Void visitMemberSelect(MemberSelectTree n, Void p) { +// return super.visitMemberSelect(n, p); +// } +// }, null); +// +// checkPackageAccess(node, getQualifiedName(node.getQualifiedIdentifier())); +// return super.visitImport(node, p); +// } + + @Override public Void visitVariable(VariableTree node, Void p) { - com.sun.tools.javac.tree.JCTree jcTree = (com.sun.tools.javac.tree.JCTree)node; + com.sun.tools.javac.tree.JCTree jcTree = (com.sun.tools.javac.tree.JCTree)node; PackageElement pakkage = elements.getPackageOf(jcTree.type.asElement()); String qualifiedName = pakkage.getQualifiedName().toString(); - - if (!packageOfCurrentCompilationUnit.getName().equals(qualifiedName) && - !packageDependencies.isWhitelisted(qualifiedName)) { - if (!qualifiedName.isEmpty() && !packageOfCurrentCompilationUnit.reads(qualifiedName)) { - if (reportingPolicy == ReportingPolicy.ERROR) { - log.error(jcTree.pos, DeptectiveMessages.ILLEGAL_PACKAGE_DEPENDENCY, packageOfCurrentCompilationUnit, qualifiedName); - } - else { - log.strictWarning(jcTree, DeptectiveMessages.ILLEGAL_PACKAGE_DEPENDENCY, packageOfCurrentCompilationUnit, qualifiedName); - } - } - } + checkPackageAccess(node, qualifiedName); return super.visitVariable(node, p); } + + @Override + public Void visitTypeParameter(TypeParameterTree node, Void p) { + node.getBounds().forEach(s -> { + checkPackageAccess(s, getQualifiedName(s)); + }); + + return super.visitTypeParameter(node, p); + } + + @Override + public Void visitParameterizedType(ParameterizedTypeTree node, Void p) { + node.getTypeArguments().forEach(s -> { + checkPackageAccess(s, getQualifiedName(s)); + }); + return super.visitParameterizedType(node, p); + } + + @Override + public Void visitAnnotation(AnnotationTree node, Void p) { + checkPackageAccess(node.getAnnotationType(), getQualifiedName(node)); + + // TODO: find Types that are references from Annotation Arguments +// node.getArguments().forEach(expr -> { +// +// System.out.println("expr" + expr + "(" + expr.getClass().getName() + ")"); +// if (expr instanceof AssignmentTree) { +// AssignmentTree assignmentTree = (AssignmentTree)expr; +// System.out.println("expr" + expr); +// System.out.println("qn => " + getQualifiedName(assignmentTree.getExpression())); +// checkPackageAccess(assignmentTree.getExpression(), getQualifiedName(assignmentTree.getExpression())); +// } +// }); + return super.visitAnnotation(node, p); + } + + protected String getQualifiedName(Tree tree) { + com.sun.tools.javac.tree.JCTree jcTree = (com.sun.tools.javac.tree.JCTree)tree; + Type type = jcTree.type; + if (type == null) { + throw new IllegalArgumentException("Could not determine type for tree object " + tree + " (" + tree.getClass()+")"); + } + PackageElement pakkage = elements.getPackageOf(type.asElement()); + return pakkage.getQualifiedName().toString(); + } + + protected void checkPackageAccess(Tree node, String qualifiedName) { + com.sun.tools.javac.tree.JCTree jcTree = (com.sun.tools.javac.tree.JCTree)node; + + if (packageDependencies.isWhitelisted(qualifiedName)) { + return; + } + + if (packageOfCurrentCompilationUnit.getName().equals(qualifiedName)) { + return; + } + + if (qualifiedName.isEmpty() || packageOfCurrentCompilationUnit.reads(qualifiedName)) { + return; + } + + + if (reportingPolicy == ReportingPolicy.ERROR) { + log.error(jcTree.pos, DeptectiveMessages.ILLEGAL_PACKAGE_DEPENDENCY, packageOfCurrentCompilationUnit, qualifiedName); + } + else { + log.strictWarning(jcTree, DeptectiveMessages.ILLEGAL_PACKAGE_DEPENDENCY, packageOfCurrentCompilationUnit, qualifiedName); + } + } } diff --git a/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/BasicPluginTest.java b/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/BasicPluginTest.java index c24a79a..02c18dc 100644 --- a/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/BasicPluginTest.java +++ b/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/BasicPluginTest.java @@ -33,50 +33,122 @@ public class BasicPluginTest extends PluginTestBase { - @Test - public void shouldDetectDisallowedPackageDependence() { - Compilation compilation = Compiler.javac() - .withOptions( - "-Xplugin:Deptective", - getConfigFileOption() - ) - .compile( - forTestClass(BarCtorCall.class), - forTestClass(BarField.class), - forTestClass(BarLocalVar.class), - forTestClass(BarLoopVar.class), - forTestClass(BarParameter.class), - forTestClass(BarRetVal.class), - forTestClass(BarTypeArg.class), - forTestClass(Foo.class) - ); + private Compilation compile() { + Compilation compilation = Compiler.javac() + .withOptions( + "-Xplugin:Deptective", + getConfigFileOption() + ) + .compile( + forTestClass(BarCtorCall.class), + forTestClass(BarField.class), + forTestClass(BarLocalVar.class), + forTestClass(BarLoopVar.class), + forTestClass(BarParameter.class), + forTestClass(BarRetVal.class), + forTestClass(BarTypeArg.class), + forTestClass(Foo.class) + ); + - assertThat(compilation).failed(); + return compilation; + } - // TODO https://github.com/moditect/deptective/issues/7 +// @Test +// public void shouldDetectInvalidConstructorParameters() { +// Compilation compilation = compile(); +// assertThat(compilation).failed(); +// +// // TODO https://github.com/moditect/deptective/issues/7 // assertThat(compilation).hadErrorContaining( // "package org.moditect.deptective.plugintest.basic.foo does not read org.moditect.deptective.plugintest.basic.barctorcall" // ); +// } + + @Test + public void shouldDetectInvalidFieldReferences() { + Compilation compilation = compile(); + assertThat(compilation).failed(); + assertThat(compilation).hadErrorContaining( "package org.moditect.deptective.plugintest.basic.foo does not read org.moditect.deptective.plugintest.basic.barfield" ); + } + + @Test + public void shouldDetectInvalidLocalVariableReferences() { + Compilation compilation = compile(); + assertThat(compilation).failed(); + assertThat(compilation).hadErrorContaining( "package org.moditect.deptective.plugintest.basic.foo does not read org.moditect.deptective.plugintest.basic.barlocalvar" ); + } + + @Test + public void shouldDetectInvalidLoopVariableReferences() { + Compilation compilation = compile(); + assertThat(compilation).failed(); + assertThat(compilation).hadErrorContaining( "package org.moditect.deptective.plugintest.basic.foo does not read org.moditect.deptective.plugintest.basic.barloopvar" ); + } + + @Test + public void shouldDetectInvalidMethodParameterReferences() { + Compilation compilation = compile(); + assertThat(compilation).failed(); + assertThat(compilation).hadErrorContaining( "package org.moditect.deptective.plugintest.basic.foo does not read org.moditect.deptective.plugintest.basic.barparameter" ); + } - // TODO https://github.com/moditect/deptective/issues/7 -// assertThat(compilation).hadErrorContaining( -// "package org.moditect.deptective.plugintest.basic.foo does not read org.moditect.deptective.plugintest.basic.barretval" -// ); -// assertThat(compilation).hadErrorContaining( -// "package org.moditect.deptective.plugintest.basic.foo does not read org.moditect.deptective.plugintest.basic.bartypearg" -// ); + @Test + public void shouldDetectInvalidAnnotationReferences() { + Compilation compilation = compile(); + assertThat(compilation).failed(); + + assertThat(compilation).hadErrorContaining( + "package org.moditect.deptective.plugintest.basic.foo does not read org.moditect.deptective.plugintest.basic.barclazzan" + ); + assertThat(compilation).hadErrorContaining( + "package org.moditect.deptective.plugintest.basic.foo does not read org.moditect.deptective.plugintest.basic.barfieldan" + ); + } + +// @Test +// public void shouldDetectInvalidReturnValueReferences() { +// Compilation compilation = compile(); +// assertThat(compilation).failed(); +// +// // TODO https://github.com/moditect/deptective/issues/7 +// assertThat(compilation).hadErrorContaining( +// "package org.moditect.deptective.plugintest.basic.foo does not read org.moditect.deptective.plugintest.basic.barretval" +// ); +// +// } + + @Test + public void shouldDetectInvalidTypeArguments() { + Compilation compilation = compile(); + assertThat(compilation).failed(); + + // in type argument + assertThat(compilation).hadErrorContaining( + "package org.moditect.deptective.plugintest.basic.foo does not read org.moditect.deptective.plugintest.basic.bartypearg" + ); + + // in class definition type argument bound + assertThat(compilation).hadErrorContaining( + "package org.moditect.deptective.plugintest.basic.foo does not read org.moditect.deptective.plugintest.basic.bargen" + ); + + // in 'extends' class definition type argument + assertThat(compilation).hadErrorContaining( + "package org.moditect.deptective.plugintest.basic.foo does not read org.moditect.deptective.plugintest.basic.bargentype" + ); } @Test diff --git a/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/barclazzan/BarClazzAnnotation.java b/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/barclazzan/BarClazzAnnotation.java new file mode 100644 index 0000000..a2c6244 --- /dev/null +++ b/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/barclazzan/BarClazzAnnotation.java @@ -0,0 +1,24 @@ +package org.moditect.deptective.plugintest.basic.barclazzan; + +import static java.lang.annotation.ElementType.ANNOTATION_TYPE; +import static java.lang.annotation.ElementType.CONSTRUCTOR; +import static java.lang.annotation.ElementType.FIELD; +import static java.lang.annotation.ElementType.LOCAL_VARIABLE; +import static java.lang.annotation.ElementType.METHOD; +import static java.lang.annotation.ElementType.MODULE; +import static java.lang.annotation.ElementType.PACKAGE; +import static java.lang.annotation.ElementType.PARAMETER; +import static java.lang.annotation.ElementType.TYPE; +import static java.lang.annotation.ElementType.TYPE_PARAMETER; +import static java.lang.annotation.ElementType.TYPE_USE; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +@Retention(RUNTIME) +@Target({ TYPE, FIELD, METHOD, PARAMETER, CONSTRUCTOR, LOCAL_VARIABLE, ANNOTATION_TYPE, PACKAGE, TYPE_PARAMETER, + TYPE_USE, MODULE }) +public @interface BarClazzAnnotation { + Class classParameter() default String.class; +} diff --git a/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/barfieldan/BarFieldAnnotation.java b/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/barfieldan/BarFieldAnnotation.java new file mode 100644 index 0000000..b959247 --- /dev/null +++ b/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/barfieldan/BarFieldAnnotation.java @@ -0,0 +1,24 @@ +package org.moditect.deptective.plugintest.basic.barfieldan; + +import static java.lang.annotation.ElementType.ANNOTATION_TYPE; +import static java.lang.annotation.ElementType.CONSTRUCTOR; +import static java.lang.annotation.ElementType.FIELD; +import static java.lang.annotation.ElementType.LOCAL_VARIABLE; +import static java.lang.annotation.ElementType.METHOD; +import static java.lang.annotation.ElementType.MODULE; +import static java.lang.annotation.ElementType.PACKAGE; +import static java.lang.annotation.ElementType.PARAMETER; +import static java.lang.annotation.ElementType.TYPE; +import static java.lang.annotation.ElementType.TYPE_PARAMETER; +import static java.lang.annotation.ElementType.TYPE_USE; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +@Retention(RUNTIME) +@Target({ TYPE, FIELD, METHOD, PARAMETER, CONSTRUCTOR, LOCAL_VARIABLE, ANNOTATION_TYPE, PACKAGE, TYPE_PARAMETER, + TYPE_USE, MODULE }) +public @interface BarFieldAnnotation { + Class classParameter() default String.class; +} diff --git a/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/bargen/BarGeneric.java b/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/bargen/BarGeneric.java new file mode 100644 index 0000000..16de4ba --- /dev/null +++ b/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/bargen/BarGeneric.java @@ -0,0 +1,5 @@ +package org.moditect.deptective.plugintest.basic.bargen; + +public class BarGeneric { + +} diff --git a/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/bargentype/BarGenType.java b/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/bargentype/BarGenType.java new file mode 100644 index 0000000..3d6a48c --- /dev/null +++ b/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/bargentype/BarGenType.java @@ -0,0 +1,5 @@ +package org.moditect.deptective.plugintest.basic.bargentype; + +public class BarGenType { + +} diff --git a/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/foo/Foo.java b/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/foo/Foo.java index cf49bf6..eb516dd 100644 --- a/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/foo/Foo.java +++ b/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/foo/Foo.java @@ -18,16 +18,23 @@ import java.util.ArrayList; import java.util.List; +import org.moditect.deptective.plugintest.basic.barclazzan.BarClazzAnnotation; import org.moditect.deptective.plugintest.basic.barctorcall.BarCtorCall; import org.moditect.deptective.plugintest.basic.barfield.BarField; +import org.moditect.deptective.plugintest.basic.barfieldan.BarFieldAnnotation; +import org.moditect.deptective.plugintest.basic.bargen.BarGeneric; +import org.moditect.deptective.plugintest.basic.bargentype.BarGenType; import org.moditect.deptective.plugintest.basic.barlocalvar.BarLocalVar; import org.moditect.deptective.plugintest.basic.barloopvar.BarLoopVar; import org.moditect.deptective.plugintest.basic.barparameter.BarParameter; import org.moditect.deptective.plugintest.basic.barretval.BarRetVal; import org.moditect.deptective.plugintest.basic.bartypearg.BarTypeArg; +@FooAnnotation +@BarClazzAnnotation public class Foo { + @BarFieldAnnotation private String s; private final BarField bar = new BarField(); @@ -47,5 +54,11 @@ public BarRetVal doSomething(BarParameter bar) { return null; } + static class InvalidFooGeneric {} + + static class InvalidFooImplementation extends FooContainer {} + + + } diff --git a/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/foo/FooAnnotation.java b/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/foo/FooAnnotation.java new file mode 100644 index 0000000..4977ec4 --- /dev/null +++ b/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/foo/FooAnnotation.java @@ -0,0 +1,24 @@ +package org.moditect.deptective.plugintest.basic.foo; + +import static java.lang.annotation.ElementType.ANNOTATION_TYPE; +import static java.lang.annotation.ElementType.CONSTRUCTOR; +import static java.lang.annotation.ElementType.FIELD; +import static java.lang.annotation.ElementType.LOCAL_VARIABLE; +import static java.lang.annotation.ElementType.METHOD; +import static java.lang.annotation.ElementType.MODULE; +import static java.lang.annotation.ElementType.PACKAGE; +import static java.lang.annotation.ElementType.PARAMETER; +import static java.lang.annotation.ElementType.TYPE; +import static java.lang.annotation.ElementType.TYPE_PARAMETER; +import static java.lang.annotation.ElementType.TYPE_USE; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +@Retention(RUNTIME) +@Target({ TYPE, FIELD, METHOD, PARAMETER, CONSTRUCTOR, LOCAL_VARIABLE, ANNOTATION_TYPE, PACKAGE, TYPE_PARAMETER, + TYPE_USE, MODULE }) +public @interface FooAnnotation { + Class classParameter() default String.class; +} diff --git a/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/foo/FooContainer.java b/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/foo/FooContainer.java new file mode 100644 index 0000000..bcd037b --- /dev/null +++ b/javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/foo/FooContainer.java @@ -0,0 +1,5 @@ +package org.moditect.deptective.plugintest.basic.foo; + +public class FooContainer { + +}