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.

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

False positive in BED_BOGUS_EXCEPTION_DECLARATION with jcabi-aspects #706

Closed
mkordas opened this issue Feb 13, 2016 · 61 comments
Closed

False positive in BED_BOGUS_EXCEPTION_DECLARATION with jcabi-aspects #706

mkordas opened this issue Feb 13, 2016 · 61 comments
Assignees
Labels

Comments

@mkordas
Copy link
Contributor

mkordas commented Feb 13, 2016

The following code:

public final class Test implements Socket {
    ...
    @Override
    @RetryOnFailure
    public void connect() throws IOException {
        this.socket.connect();
    }
    ...
}

Causes [INFO] >> M C BED: Non derivable methodconnect()declares throwing an exception that isn't thrown.
However, both interface and this.socket.connect() declare `throws IOException', so I consider this as false positive.

Root cause here is annotation from jcabi-aspects on a method that throws correct exception. In that case fb-contrib can't findout that the exception is thrown. AspectJ must be doing some bytecode magic that makes it hard to detect.

Proposed solution is in #706 (comment)


@krzyk
Copy link
Collaborator

krzyk commented Feb 13, 2016

@mkordas could you post complete reproducer?

@krzyk
Copy link
Collaborator

krzyk commented Feb 13, 2016

@mkordas ping

@mkordas
Copy link
Contributor Author

mkordas commented Feb 14, 2016

@krzyk I'll try to work on isolated reproducer tomorrow

@mkordas
Copy link
Contributor Author

mkordas commented Feb 14, 2016

@krzyk cannot find isolated reproducer, closing for now

@mkordas mkordas closed this as completed Feb 14, 2016
@krzyk
Copy link
Collaborator

krzyk commented Feb 16, 2016

@mkordas I think I found when the problem exists. If we have any annotation from jcabi-aspects on a method that throws correct exception, fb-contrib can't findout that the exception is thrown.
AspectJ must be doing some bytecode magic that makes it hard to detect.
I'm wondering how to prevent it (and how to report bug to fb-contrib).
Maybe do another compilation with aspectj disabled.

@mkordas
Copy link
Contributor Author

mkordas commented Feb 16, 2016

@krzyk right, I was trying to reproduce in isolation without these annotations and I couldn't, so now it makes sense. Should I reopen the issue? I think that we should find workaround ASAP or check should be disabled, as it will cause too much noise for some projects (e.g retryable methods usually throw exceptions...)

@krzyk
Copy link
Collaborator

krzyk commented Feb 16, 2016

@mkordas yes, let's reopen it, we need to find a way to workaround it somehow.
By tuning the jcabi-maven-plugin to store the original classes in some directory and run findbugs on them.
I was thinking that we should:

  1. check that output directory has a directory named unwoven, if it does then run FindBugs on it, if it doesn't exist then run it on the output directory.
  2. document that behavior and document how to configure a project that uses jcabi-maven-plugin to support that
  3. create a invoker test that uses the 2

@mkordas mkordas changed the title False positive in BED_BOGUS_EXCEPTION_DECLARATION False positive in BED_BOGUS_EXCEPTION_DECLARATION with jcabi-aspects Feb 16, 2016
@mkordas mkordas reopened this Feb 16, 2016
@mkordas
Copy link
Contributor Author

mkordas commented Feb 16, 2016

@krzyk issue reopened, description updated

@krzyk
Copy link
Collaborator

krzyk commented Feb 16, 2016

@davvd valid bug

@krzyk
Copy link
Collaborator

krzyk commented Feb 16, 2016

@davvd this is urgent

@yegor256
Copy link
Owner

@krzyk +1

@krzyk
Copy link
Collaborator

krzyk commented Feb 19, 2016

@yegor256 it would be easier to fix it with jcabi/jcabi-maven-plugin#45

@davvd
Copy link

davvd commented Feb 24, 2016

@davvd valid bug

@krzyk tagged this issue with "bug"

@davvd davvd added the bug label Feb 24, 2016
@krzyk
Copy link
Collaborator

krzyk commented Feb 25, 2016

@davvd this is urgent

@davvd
Copy link

davvd commented Mar 1, 2016

@davvd this is urgent

@krzyk thanks, I added "urgent" label

@davvd
Copy link

davvd commented Mar 1, 2016

@davvd this is urgent

@krzyk "urgent" label already here

@davvd
Copy link

davvd commented Mar 1, 2016

@gumbelmj could you please pick this up? This article explains how we work. Any technical questions you may ask right here. The budget here is 30 mins, which is exactly how much time will be paid for, when the task is completed

@gumbelmj
Copy link
Contributor

gumbelmj commented Mar 4, 2016

@davvd please assign to someone else.

@davvd
Copy link

davvd commented Mar 4, 2016

@davvd please assign to someone else.

@gumbelmj -30 points to your rating :(

@krzyk
Copy link
Collaborator

krzyk commented May 30, 2016

@davvd this is urgent

@davvd davvd added the urgent label May 31, 2016
@davvd
Copy link

davvd commented May 31, 2016

@davvd this is urgent

@krzyk OK, I put "urgent" label here

@davvd
Copy link

davvd commented May 31, 2016

@davvd this is urgent

@krzyk OK, I un-labeled it as "postponed" tags

@davvd davvd removed the postponed label May 31, 2016
@davvd
Copy link

davvd commented Jun 8, 2016

@dskalenko please pick this up, and keep in mind these instructions. Any technical questions - ask right here

The budget here is 30 mins, which is exactly how much time will be paid for, when the task is completed

@dskalenko
Copy link
Contributor

@krzyk or @mkordas

I have implemented test to be able to fix the problem, now I am stuck , because I can't find elegant solution how I can get unwovenClassesDir parameter from jcabi-maven-plugin, I found something like this http://stackoverflow.com/questions/125389/best-way-to-access-the-runtime-configuration-of-a-maven-plugin-from-a-custom-mojo/130872#130872
Could you please give me advice how I can do it easily?

@krzyk
Copy link
Collaborator

krzyk commented Jun 14, 2016

@dskalenko I think we would need to wait for jcabi/jcabi-maven-plugin#48 to be fixed other option is to temporary disable BED_BOGUS_EXCEPTION_DECLARATION check and create a puzzle to implement it after jcabi/jcabi-maven-plugin#48 is done. Other options I think are too costly.

dskalenko added a commit to dskalenko/qulice that referenced this issue Jun 15, 2016
…cabi-aspects (add test and for the moment temporary disable BED_BOGUS_EXCEPTION_DECLARATION check)
@dskalenko
Copy link
Contributor

@krzyk
here my PR #781, please read description of PR before doing code review and let me know if I am done correctly for the moment, thanks

dskalenko added a commit to dskalenko/qulice that referenced this issue Jun 17, 2016
dskalenko added a commit to dskalenko/qulice that referenced this issue Jun 22, 2016
@davvd
Copy link

davvd commented Jun 23, 2016

@dskalenko check this "no obligations principle".. This task is on your name for at least 15 days. If you can't close it within the next 48 hours I will have to assign someone else to it. This article should help if you're stuck; -30 added to your rating, current score is: +0

dskalenko added a commit to dskalenko/qulice that referenced this issue Jun 23, 2016
dskalenko added a commit to dskalenko/qulice that referenced this issue Jun 24, 2016
dskalenko added a commit to dskalenko/qulice that referenced this issue Jun 24, 2016
@krzyk
Copy link
Collaborator

krzyk commented Jun 25, 2016

@dskalenko now you have to ask mkordas to close the issue (if it fixes his problrm)

@krzyk
Copy link
Collaborator

krzyk commented Jun 26, 2016

@dskalenko ping

@dskalenko
Copy link
Contributor

@mkordas Could you please check this issue and close if all good

@davvd
Copy link

davvd commented Jun 27, 2016

@mkordas once 706-72bdab5d/#787 puzzle is resolved (later, in another ticket), this ticket will be fully complete

@mkordas mkordas closed this as completed Jun 27, 2016
@krzyk
Copy link
Collaborator

krzyk commented Jun 28, 2016

@davvd this is not urgent

@davvd davvd removed the urgent label Jun 28, 2016
@davvd
Copy link

davvd commented Jun 28, 2016

@davvd this is not urgent

@krzyk right, I removed "urgent" tag

@davvd
Copy link

davvd commented Jun 29, 2016

@dskalenko Thanks so much! Your account was topped up for 30 mins (transaction ID is 91616835, the task took 482 hours and 3 mins)

added +30 to your rating, now it is equal to +30

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants