Classes should be able to access their own fields and methods (#17)

Co-authored-by: Tim Jacomb <[email protected]>
jenkinsci · Dec 15, 2020 · 32f416c · 32f416c
1 parent 805dc3b
commit 32f416c
Show file tree

Hide file tree

Showing 5 changed files with 95 additions and 3 deletions.
diff --git a/access-modifier-checker/src/it/own-members/invoker.properties b/access-modifier-checker/src/it/own-members/invoker.properties
@@ -0,0 +1 @@
+invoker.goals=clean package
diff --git a/access-modifier-checker/src/it/own-members/pom.xml b/access-modifier-checker/src/it/own-members/pom.xml
@@ -0,0 +1,36 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+    <groupId>test</groupId>
+    <artifactId>this-instance-impl</artifactId>
+    <version>1.0-SNAPSHOT</version>
+    <packaging>jar</packaging>
+    <properties>
+        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+        <maven.compiler.source>1.8</maven.compiler.source>
+        <maven.compiler.target>1.8</maven.compiler.target>
+    </properties>
+    <dependencies>
+        <dependency>
+            <groupId>org.kohsuke</groupId>
+            <artifactId>access-modifier-annotation</artifactId>
+            <version>@project.version@</version>
+        </dependency>
+    </dependencies>
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.kohsuke</groupId>
+                <artifactId>access-modifier-checker</artifactId>
+                <version>@project.version@</version>
+                <executions>
+                    <execution>
+                        <goals>
+                            <goal>enforce</goal>
+                        </goals>
+                    </execution>
+                </executions>
+           </plugin>
+        </plugins>
+    </build>
+</project>
diff --git a/access-modifier-checker/src/it/own-members/src/main/java/Outer.java b/access-modifier-checker/src/it/own-members/src/main/java/Outer.java
@@ -0,0 +1,11 @@
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.DoNotUse;
+
+class Outer {
+  @Restricted(DoNotUse.class)
+  static class Middle {
+    static class Inner {
+      static {new Middle();}
+    }
+  }
+}
diff --git a/access-modifier-checker/src/it/own-members/src/main/java/SomeClass.java b/access-modifier-checker/src/it/own-members/src/main/java/SomeClass.java
@@ -0,0 +1,18 @@
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.DoNotUse;
+
+@Restricted(DoNotUse.class)
+public class SomeClass {
+  private int foo;
+
+  public SomeClass() {
+    foo = 12;
+  }
+
+  public int getFoo() {
+    doSomething();
+    return foo;
+  }
+
+  private void doSomething() {}
+}
diff --git a/access-modifier-checker/src/main/java/org/kohsuke/accmod/impl/Checker.java b/access-modifier-checker/src/main/java/org/kohsuke/accmod/impl/Checker.java
@@ -344,7 +344,7 @@ public MethodVisitor visitMethod(int access, String name, String desc, String si
                 return null;
             }
 
-            return new RestrictedMethodVisitor(currentLocation, annotationVisitor.getSkippedTypes());
+            return new RestrictedMethodVisitor(currentLocation, className, annotationVisitor.getSkippedTypes());
         }
 
         @Override
@@ -388,24 +388,38 @@ public String getProperty(String key) {
         };
     }
 
+    private static String topLevelClass(String a) {
+      int i = a.indexOf('$');
+      if (i == -1) {
+          return a;
+      }
+      return a.substring(0, i);
+    }
+
+    private static boolean sameClassFile(String currentClass, String owner) {
+        return topLevelClass(currentClass).equals(topLevelClass(owner));
+    }
+
     private class RestrictedMethodVisitor extends MethodVisitor {
 
         private final Set<Type> skippedTypesFromParent;
         private Location currentLocation;
         private RestrictedAnnotationVisitor annotationVisitor = new RestrictedAnnotationVisitor();
+        private final String currentClass;
 
         private Set<Type> getSkippedTypes() {
             Set<Type> allSkippedTypes = new HashSet<>(skippedTypesFromParent);
             allSkippedTypes.addAll(annotationVisitor.getSkippedTypes());
             return allSkippedTypes;
         }
 
-        public RestrictedMethodVisitor(Location currentLocation, Set<Type> skippedTypes) {
+        public RestrictedMethodVisitor(Location currentLocation, String currentClass, Set<Type> skippedTypes) {
             super(Opcodes.ASM9);
             log.debug(String.format("New method visitor at %s#%s",
                     currentLocation.getClassName(), currentLocation.getMethodName()));
             this.currentLocation = currentLocation;
             this.skippedTypesFromParent = skippedTypes;
+            this.currentClass = currentClass;
         }
 
         @Override
@@ -416,6 +430,10 @@ public void visitLineNumber(int _line, Label start) {
         public void visitTypeInsn(int opcode, String type) {
             switch (opcode) {
             case Opcodes.NEW:
+                if (sameClassFile(currentClass, type)) {
+                    return;
+                }
+
                 for (Restrictions r : getRestrictions(type, getSkippedTypes())) {
                     r.instantiated(currentLocation, errorListener);
                 }
@@ -425,6 +443,11 @@ public void visitTypeInsn(int opcode, String type) {
         @Override
         public void visitMethodInsn(int opcode, String owner, String name, String desc, boolean itf) {
             log.debug(String.format("Visiting method %s#%s", owner, name));
+
+            if (sameClassFile(currentClass, owner)) {
+                return;
+            }
+
             for (Restrictions r : getRestrictions(owner + '.' + name + desc, getSkippedTypes())) {
                 r.invoked(currentLocation, errorListener);
             }
@@ -434,6 +457,10 @@ public void visitMethodInsn(int opcode, String owner, String name, String desc,
         public void visitFieldInsn(int opcode, String owner, String name, String desc) {
             log.debug(String.format("Visiting field '%s %s' in type %s", desc, name, owner));
 
+            if (sameClassFile(currentClass, owner)) {
+                return;
+            }
+
             Iterable<Restrictions> rs = getRestrictions(owner + '.' + name, getSkippedTypes());
             switch (opcode) {
                 case Opcodes.GETSTATIC:
@@ -449,7 +476,6 @@ public void visitFieldInsn(int opcode, String owner, String name, String desc) {
                     }
                     break;
             }
-            super.visitFieldInsn(opcode, owner, name, desc);
         }
 
         @Override