#7: Detect more invalid references (#16)

* #7 Finding more invalid references; reworking getQualifiedName()

javac-plugin/src/main/java/org/moditect/deptective/internal/DeptectiveTreeVisitor.java

@@ -15,12 +15,17 @@
  */
 package org.moditect.deptective.internal;
 
+import javax.lang.model.element.Element;
 import javax.lang.model.element.PackageElement;
+import javax.lang.model.type.TypeKind;
+import javax.lang.model.type.TypeMirror;
 import javax.lang.model.util.Elements;
+import javax.lang.model.util.Types;
 
 import com.sun.source.tree.AnnotationTree;
 import com.sun.source.tree.ClassTree;
 import com.sun.source.tree.CompilationUnitTree;
+import com.sun.source.tree.MemberSelectTree;
 import com.sun.source.tree.MethodTree;
 import com.sun.source.tree.NewClassTree;
 import com.sun.source.tree.ParameterizedTypeTree;
@@ -29,8 +34,8 @@
 import com.sun.source.tree.VariableTree;
 import com.sun.source.util.JavacTask;
 import com.sun.source.util.TreePathScanner;
+import com.sun.source.util.Trees;
 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;
 
@@ -39,11 +44,14 @@ public class DeptectiveTreeVisitor extends TreePathScanner<Void, Void> {
     private final Log log;
     private final Elements elements;
     private final PackageReferenceHandler packageReferenceHandler;
+    private final Trees trees;
+    private final Types types;
 
     public DeptectiveTreeVisitor(DeptectiveOptions options, JavacTask task,
             PackageReferenceHandler packageReferenceHandler) {
         elements = task.getElements();
-
+        types = task.getTypes();
+        trees = Trees.instance(task);
         Context context = ((BasicJavacTask) task).getContext();
         this.log = Log.instance(context);
         this.packageReferenceHandler = packageReferenceHandler;
@@ -57,47 +65,34 @@ public Void visitCompilationUnit(CompilationUnitTree tree, Void p) {
         return super.visitCompilationUnit(tree, p);
     }
 
-    //    @Override
-    //    public Void visitImport(ImportTree node, Void p) {
-    // TODO: Deal with "on-demand-imports" (com.foo.*)
-    //        node.getQualifiedIdentifier().accept(new TreeScanner<Void, Void>() {
-    //            @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 visitClass(ClassTree node, Void p) {
         Tree extendsClause = node.getExtendsClause();
         if (extendsClause != null) {
-            checkPackageAccess(extendsClause, getQualifiedName(extendsClause));
+            checkPackageAccess(extendsClause, getQualifiedPackageName(extendsClause));
         }
 
-        node.getImplementsClause().forEach(implementsClause -> checkPackageAccess(implementsClause, getQualifiedName(implementsClause)));
+        node.getImplementsClause().forEach(implementsClause -> checkPackageAccess(implementsClause, getQualifiedPackageName(implementsClause)));
 
         return super.visitClass(node, p);
     }
 
     @Override
-    public Void visitVariable(VariableTree node, Void p) {
-        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();
-        checkPackageAccess(node, qualifiedName);
+    public Void visitMemberSelect(MemberSelectTree node, Void p) {
+        checkPackageAccess(node, getQualifiedPackageName(node));
+        return super.visitMemberSelect(node, p);
+    }
 
+    @Override
+    public Void visitVariable(VariableTree node, Void p) {
+        checkPackageAccess(node, getQualifiedPackageName(node));
         return super.visitVariable(node, p);
     }
 
     @Override
     public Void visitTypeParameter(TypeParameterTree node, Void p) {
         node.getBounds().forEach(s -> {
-            checkPackageAccess(s, getQualifiedName(s));
+            checkPackageAccess(s, getQualifiedPackageName(s));
         });
 
         return super.visitTypeParameter(node, p);
@@ -106,55 +101,57 @@ public Void visitTypeParameter(TypeParameterTree node, Void p) {
     @Override
     public Void visitParameterizedType(ParameterizedTypeTree node, Void p) {
         node.getTypeArguments().forEach(s -> {
-            checkPackageAccess(s, getQualifiedName(s));
+            checkPackageAccess(s, getQualifiedPackageName(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()));
-        //            }
-        //        });
+        checkPackageAccess(node.getAnnotationType(), getQualifiedPackageName(node));
         return super.visitAnnotation(node, p);
     }
 
     @Override
     public Void visitNewClass(NewClassTree node, Void p) {
-        checkPackageAccess(node, getQualifiedName(node));
+        checkPackageAccess(node, getQualifiedPackageName(node));
         return super.visitNewClass(node, p);
     }
 
     @Override
     public Void visitMethod(MethodTree node, Void p) {
         Tree returnType = node.getReturnType();
         if (returnType != null) {
-            checkPackageAccess(returnType, getQualifiedName(returnType));
+            checkPackageAccess(returnType, getQualifiedPackageName(returnType));
         }
         return super.visitMethod(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()+")");
+    /**
+     * Returns the qualified Package Name of the given Tree object or null if the
+     * package could not be determined
+     */
+    protected String getQualifiedPackageName(Tree tree) {
+        TypeMirror typeMirror = trees.getTypeMirror(getCurrentPath());
+        if (typeMirror == null) {
+            return null;
+        }
+
+        if (typeMirror.getKind() != TypeKind.DECLARED && typeMirror.getKind() != TypeKind.TYPEVAR) {
+            return null;
         }
-        PackageElement pakkage = elements.getPackageOf(type.asElement());
+
+        Element typeMirrorElement = types.asElement(typeMirror);
+        if (typeMirrorElement == null) {
+            throw new IllegalStateException("Could not get Element for type '" + typeMirror + "'");
+        }
+        PackageElement pakkage = elements.getPackageOf(typeMirrorElement);
         return pakkage.getQualifiedName().toString();
     }
 
     protected void checkPackageAccess(Tree node, String qualifiedName) {
-        packageReferenceHandler.onPackageReference(node, qualifiedName);
+        if (qualifiedName != null) {
+            packageReferenceHandler.onPackageReference(node, qualifiedName);
+        }
     }
 }

javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/BasicPluginTest.java

@@ -16,7 +16,16 @@
 package org.moditect.deptective.plugintest.basic;
 
 import static com.google.testing.compile.CompilationSubject.assertThat;
+import static org.junit.Assert.assertThat;
 
+import java.util.List;
+import java.util.stream.Collectors;
+
+import javax.tools.Diagnostic;
+import javax.tools.JavaFileObject;
+
+import org.assertj.core.util.Lists;
+import org.hamcrest.core.Is;
 import org.junit.Test;
 import org.moditect.deptective.plugintest.PluginTestBase;
 import org.moditect.deptective.plugintest.basic.barctorcall.BarCtorCall;
@@ -63,6 +72,32 @@ public void shouldAllowAccessToJavaLangAutomatically() {
         assertThat(compilation).succeeded();
     }
 
+    @Test
+    public void shouldDetectInvalidClassAssignment() {
+        Compilation compilation = compile();
+        assertThat(compilation).failed();
+
+        assertThat(compilation).hadErrorContaining(
+                packageFooMustNotAccess("org.moditect.deptective.plugintest.basic.barclass"));
+    }
+
+    @Test
+    public void shouldNotReportValidReferences() {
+        // in our Testclass all "wanted" illegal references point to "org.moditect.deptective.plugintest.basic.bar*"
+        // so Deptective should only return those
+
+        Compilation compilation = compile();
+        assertThat(compilation).failed();
+
+        List<Diagnostic<? extends JavaFileObject>> collect = compilation.errors().stream()
+                .filter(e -> e.getMessage(null).indexOf("must not access org.moditect.deptective.plugintest.basic.bar")==-1)
+                .collect(Collectors.toList());
+
+        assertThat("Test-Class 'Foo' should not have any invalid dependencies beside those to 'org.moditect.deptective.plugintest.basic.bar*'",
+                collect, Is.is(Lists.emptyList()));
+    }
+
+
     @Test
     public void shouldDetectInvalidSuperClass() {
         Compilation compilation = compile();
@@ -153,6 +188,18 @@ public void shouldDetectInvalidAnnotationReferences() {
                 packageFooMustNotAccess("org.moditect.deptective.plugintest.basic.barfieldan"));
     }
 
+    @Test
+    public void shouldDetectInvalidAnnotationParameterReferences() {
+        Compilation compilation = compile();
+        assertThat(compilation).failed();
+
+        assertThat(compilation).hadErrorContaining(
+                packageFooMustNotAccess("org.moditect.deptective.plugintest.basic.barclass"));
+
+        assertThat(compilation).hadErrorContaining(
+                packageFooMustNotAccess("org.moditect.deptective.plugintest.basic.baranparam"));
+    }
+
     @Test
     public void shouldDetectInvalidReturnValueReferences() {
     	Compilation compilation = compile();

javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/baranparam/BarAnnotationParameter.java

@@ -0,0 +1,20 @@
+/**
+ *  Copyright 2019 The ModiTect authors
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.moditect.deptective.plugintest.basic.baranparam;
+
+public class BarAnnotationParameter {
+
+}

javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/barclass/BarClass.java

@@ -0,0 +1,20 @@
+/**
+ *  Copyright 2019 The ModiTect authors
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.moditect.deptective.plugintest.basic.barclass;
+
+public class BarClass {
+
+}

javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/foo/Foo.java

@@ -18,6 +18,8 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import org.moditect.deptective.plugintest.basic.baranparam.BarAnnotationParameter;
+import org.moditect.deptective.plugintest.basic.barclass.BarClass;
 import org.moditect.deptective.plugintest.basic.barclazzan.BarClazzAnnotation;
 import org.moditect.deptective.plugintest.basic.barctorcall.BarCtorCall;
 import org.moditect.deptective.plugintest.basic.barctorparam.BarCtorParam;
@@ -36,13 +38,14 @@
 import org.moditect.deptective.plugintest.basic.barsuper.BarSuper;
 import org.moditect.deptective.plugintest.basic.bartypearg.BarTypeArg;
 
-@FooAnnotation
+@FooAnnotation(classParameter = BarAnnotationParameter.class)
 @BarClazzAnnotation
 public class Foo extends BarSuper implements BarInterface, /* allowed: */ IFoo {
 
-	@BarFieldAnnotation
+    @BarFieldAnnotation
     private String s;
     private final BarField bar = new BarField();
+    private Class<?> clazz = BarClass.class;
 
     public Foo(BarCtorParam bar) {
     }
@@ -71,10 +74,8 @@ static class InvalidFooGeneric<T extends BarGeneric> {}
     static class InvalidFooImplementation extends FooContainer<BarGenType> {}
 
     static interface InnerFoo extends BarInnerInterface {
-
     }
 
     static class InnerFooClass extends BarInnerSuperClass {
-
     }
 }

javac-plugin/src/test/java/org/moditect/deptective/plugintest/basic/foo/SimpleFoo.java

@@ -0,0 +1,24 @@
+/**
+ *  Copyright 2019 The ModiTect authors
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.moditect.deptective.plugintest.basic.foo;
+
+import org.moditect.deptective.plugintest.basic.barclass.BarClass;
+
+public class SimpleFoo {
+
+    public Class<?> x = BarClass.class;
+
+}