DaveAKing / guava-libraries

Automatically exported from code.google.com/p/guava-libraries
Apache License 2.0
0 stars 0 forks source link

let @VisibleForTesting tells original visibility of annotated element #1640

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Hello, I am trying to implement static code analysis tool which detects illegal 
method call like below:

  // an implementation which has a method annotated by @VisibleForTesting
  class MyImplementation {
    @VisibleForTesting
    /* private */ void method() { ... }
  }

  // another implementation
  class AnotherImplementation {
    void method() {
      new MyImplementation().method(); // illegal method call, because this class is not test code
    }
  }

To find illegal method calling, we should know the original visibility of 
annotated element (method in this case). In some case it is difficult to guess 
original visibility, so I want to enhance @VisibleForTesting to explain it like 
below:

    @VisibleForTesting(original = Visibility.PRIVATE)
    void method() { ... }

I think all static analysis tool which uses this annotation needs method to 
judge illegality of method call, so I have added methods into Visibility enum 
in attached diff. It is also good to make this enum simple like 
com.android.internal.annotations.VisibleForTesting annotation. See 
https://github.com/android/platform_frameworks_base/blob/8b2c3a14603d163d7564e6f
60286995079687690/core/java/com/android/internal/annotations/VisibleForTesting.j
ava

Original issue reported on code.google.com by skypencil@gmail.com on 22 Jan 2014 at 2:13

Attachments:

GoogleCodeExporter commented 9 years ago
If we were to do this, and I am not suggesting we should, we should probably 
make it value() not original(), so we can just write 
@VisibleForTesting(PRIVATE).  I think it's really obvious what's going on - the 
extra verbiage seems superfluous.

Original comment by christia...@gmail.com on 23 Jan 2014 at 11:45

GoogleCodeExporter commented 9 years ago
Any progress?

Original comment by skypencil@gmail.com on 4 Apr 2014 at 12:20

GoogleCodeExporter commented 9 years ago
FWIW, we've had this internally for about 6 years (the Android version is 
nearly a copy of the Guava version), but until we have tooling to support this, 
it doesn't seem particularly useful.

Do you have some special static analysis tools that make it useful?

Original comment by kak@google.com on 4 Apr 2014 at 3:18

GoogleCodeExporter commented 9 years ago
Hello, thanks for your reply.

> Do you have some special static analysis tools that make it useful?

I have a plan to implement new FindBugs detector which uses on this annotation. 
It might be a part of my FindBugs 
plugin(https://github.com/eller86/findbugs-plugin), or it might be merged into 
FindBugs itself (I'll try to send a patch).

I wish this post will help you to judge. Thanks.

Original comment by skypencil@gmail.com on 5 Apr 2014 at 3:14

GoogleCodeExporter commented 9 years ago
FYI: simple implementation is here.

https://github.com/eller86/findbugs-plugin/commit/e53e0e168d37101f2e7daeecdc53b1
48d84125b7

Original comment by skypencil@gmail.com on 8 Apr 2014 at 8:55

GoogleCodeExporter commented 9 years ago
This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<issue id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:10

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:17

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:07