Ignore issue when coming from the same class #6
Conversation
|
@reviewbybees for more eyes on this, even if done on personal time. |
|
Ping @kohsuke as this will be required to merge Jenkins' jenkinsci/jenkins#2892 (if deemed correct). |
|
This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation. |
| @@ -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); | |||
There was a problem hiding this comment.
Maybe factor into a private helper method.
| public boolean isSameClass(Location location) { | ||
| String locationClassName = location.getClassName(); | ||
| if (locationClassName.contains("$")) { | ||
| locationClassName = locationClassName.split("\\$")[0]; |
There was a problem hiding this comment.
or
String locationClassName = location.getClassName().replaceFirst("[$].+$", "");| if (locationClassName.contains("$")) { | ||
| locationClassName = locationClassName.split("\\$")[0]; | ||
| } | ||
| if(keyToClassName(keyName).equals(locationClassName)) { |
There was a problem hiding this comment.
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();}
}
}
}?
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
| import static org.mockito.Mockito.mock; | ||
| import static org.mockito.Mockito.when; |
There was a problem hiding this comment.
Personally I do not trust mocks and would recommend a new IT.
|
@batmat any plans to finalize it? |
|
Finally simplified the upstream PR to use (Sorry Oleg, had missed that ping above :-() |
until jenkinsci/lib-access-modifier#6 is revived, if ever. LOL, @jglick was the one reminding me about that PR 🤦 :)
Trying to fix jenkinsci/jenkins#2892 I got blocked because
DoNotUsewould fail apparently onTimeUnit2enum instances. This fixes this behaviour and ignores issues when an usage is from the same class. (for instance, it's OK for a class marked DoNotUse to leave it as is, that it calls itself)