Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore issue when coming from the same class #6

Closed
wants to merge 1 commit into from
Closed

Ignore issue when coming from the same class #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions access-modifier-annotation/src/main/java/org/kohsuke/accmod/impl/RestrictedElement.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,13 @@ public interface RestrictedElement {
*/
boolean isInTheInspectedModule();

/**
* Returns true if the given {@link Location} is actually in <code>this</code>.
*
* @param location the location to be tested.
* @return true if the given {@link Location} is actually in <code>this</code>.
*/
boolean isSameClass(Location location);

String toString();
}
18 changes: 18 additions & 0 deletions access-modifier-annotation/src/main/java/org/kohsuke/accmod/restrictions/DoNotUse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,26 +35,44 @@
*/
public class DoNotUse extends AccessRestriction {
public void written(Location loc, RestrictedElement target, ErrorListener errorListener) {
if (target.isSameClass(loc)) {
return;
}
error(loc,target,errorListener);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe factor into a private helper method.

}

public void usedAsSuperType(Location loc, RestrictedElement target, ErrorListener errorListener) {
if (target.isSameClass(loc)) {
return;
}
error(loc,target,errorListener);
}

public void usedAsInterface(Location loc, RestrictedElement target, ErrorListener errorListener) {
if (target.isSameClass(loc)) {
return;
}
error(loc,target,errorListener);
}

public void instantiated(Location loc, RestrictedElement target, ErrorListener errorListener) {
if (target.isSameClass(loc)) {
return;
}
error(loc,target,errorListener);
}

public void invoked(Location loc, RestrictedElement target, ErrorListener errorListener) {
if (target.isSameClass(loc)) {
return;
}
error(loc,target,errorListener);
}

public void read(Location loc, RestrictedElement target, ErrorListener errorListener) {
if (target.isSameClass(loc)) {
return;
}
error(loc,target,errorListener);
}

Expand Down
14 changes: 13 additions & 1 deletion access-modifier-checker/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,19 @@
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>3.8.1</version>
<version>4.12</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<version>2.2.15</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
<version>3.4.1</version>
<scope>test</scope>
</dependency>
<dependency>
Expand Down
8 changes: 1 addition & 7 deletions access-modifier-checker/src/main/java/org/kohsuke/accmod/impl/Checker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,13 +176,7 @@ public AnnotationVisitor visitAnnotation(String desc, boolean visible) {
*/
private AnnotationVisitor onAnnotationFor(final String keyName, String desc) {
if (RESTRICTED_DESCRIPTOR.equals(desc)) {
RestrictedElement target = new RestrictedElement() {
public boolean isInTheInspectedModule() {
return isInTheInspectedModule;
}

public String toString() { return keyName; }
};
RestrictedElement target = new RestrictedElementImpl(isInTheInspectedModule, keyName);
return new Parser(target) {
@Override
public void visitEnd() {
Expand Down
39 changes: 39 additions & 0 deletions access-modifier-checker/src/main/java/org/kohsuke/accmod/impl/RestrictedElementImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package org.kohsuke.accmod.impl;

public class RestrictedElementImpl implements RestrictedElement {
private final boolean isInTheInspectedModule;
private final String keyName;

public RestrictedElementImpl(boolean isInTheInspectedModule, String keyName) {
this.isInTheInspectedModule = isInTheInspectedModule;
this.keyName = keyName;
}

public boolean isInTheInspectedModule() {
return isInTheInspectedModule;
}

@Override
public boolean isSameClass(Location location) {
String locationClassName = location.getClassName();
if (locationClassName.contains("$")) {
locationClassName = locationClassName.split("\\$")[0];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or

String locationClassName = location.getClassName().replaceFirst("[$].+$", "");

}
if(keyToClassName(keyName).equals(locationClassName)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this going to work with all cases of nested classes? Like

class Outer
  @Restricted(DoNotUse.class)
  static class Middle {
    static class Inner {
      static {new Middle();}
    }
  }
}

?

return true;
}
return false;
}

private static String keyToClassName(String key) {
if (key.contains(".")) {
key = key.split("\\.")[0];
}
return key.replace('/', '.');
}


public String toString() {
return keyName;
}
}
6 changes: 6 additions & 0 deletions access-modifier-checker/src/main/java/org/kohsuke/accmod/impl/Restrictions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,12 @@ public void written(Location location, ErrorListener errorListener) {

public static final Restrictions NONE = new Restrictions(new RestrictedElement() {
public boolean isInTheInspectedModule() { return false; }

@Override
public boolean isSameClass(Location location) {
return false;
}

public String toString() { return "NONE"; }
});

Expand Down
32 changes: 32 additions & 0 deletions access-modifier-checker/src/test/java/org.kohsuke.accmod.impl/RestrictedElementImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package org.kohsuke.accmod.impl;

import org.junit.Test;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I do not trust mocks and would recommend a new IT.


public class RestrictedElementImplTest {

@Test
public void basic() throws Exception {

assertThat(new RestrictedElementImpl(false, "hudson/util/TimeUnit2")
.isSameClass(createLocation("hudson.util.TimeUnit2$4"))).isTrue();

assertThat(new RestrictedElementImpl(false, "hudson/util/TimeUnit2.someMethod();")
.isSameClass(createLocation("hudson.util.TimeUnit2$4"))).isTrue();

assertThat(new RestrictedElementImpl(false, "hudson/util/XStream2.addCriticalField(Ljava/lang/Class;Ljava/lang/String;)V")
.isSameClass(createLocation("jenkins.model.Jenkins"))).isFalse();

assertThat(new RestrictedElementImpl(false, "jenkins/model/Jenkins.expandVariablesForDirectory(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String; ")
.isSameClass(createLocation("jenkins.model.Jenkins$DescriptorImpl"))).isTrue();
}

private Location createLocation(String value) {
Location location = mock(Location.class);
when(location.getClassName()).thenReturn(value);
return location;
}
}
4 changes: 4 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
<packaging>pom</packaging>
<description>Extensible application-specific access modifiers for Java</description>

<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
</properties>

<modules>
<module>access-modifier-annotation</module>
<module>access-modifier-checker</module>
Expand Down