NonStaticMethodCheck complains about methods that just throw an exception

[HDouss]

NonStaticMethodCheck complains about methods that only throws an exception. This could be fine in cases where a class implementation have a "not yet" supported feature or a serializable object that does not support readObject/writeObject where the method have just to throw an appropriate exception. Example:

private void writeObject(final ObjectOutputStream stream)
    throws NotSerializableException {
    throw new NotSerializableException(AggregateException.class.getName());
}

[mkordas]

@HDouss but then method has '@OverRide' and is excluded from check, right? Please provide an example.

[HDouss]

@mkordas You are right, I update description to give another example.

[krzyk]

@HDouss If you gave an example please show it, if not please close this issue

[HDouss]

@krzyk The examples are in the description:

This could be fine in cases where a class implementation have a "not yet" supported feature or a serializable object that does not support readObject/writeObject where the method have just to throw an appropriate exception

[krzyk]

@HDouss but such methods are not correct ones, so the appropriate way of dealing with this is adding an exclude. readObject/writeObject shouldn't be implemented if there is no state.

[HDouss]

@krzyk But NonStaticMethodCheck should alert the developer about methods that are not declared as static but are in fact static. For example:

private void writeObject(final ObjectOutputStream stream)
        throws NotSerializableException {
        throw new NotSerializableException(AggregateException.class.getName());
    }

Why such a method should be static ?

[krzyk]

@HDouss OK, so you are saying that we should exclude the readObject() and writeObject() methods or some other general rule?

[HDouss]

@krzyk I would say that in general, if a method just throws an exception shouldn't be considered as static.

[krzyk]

@HDouss please update the description with that clarification

[krzyk]

@HDouss method that only throws exception

[HDouss]

@krzyk Done.

[krzyk]

@HDouss also add an example code, it will be easier to understand and implement

[HDouss]

@krzyk First sentence:

NonStaticMethodCheck complains about methods that only throws an exception

[krzyk]

@HDouss yes, I noticed that later :), but example would make it even better.
Could you add an example method that shouldn't cause NonStaticMethodCheck to complain?

[krzyk]

@davvd valid bug

[krzyk]

@davvd this is postponed

[davvd]

@davvd valid bug

@krzyk tagged this issue with "bug"

[davvd]

@davvd this is postponed

@krzyk OK, I put "postponed" label here

[davvd]

@davvd this is postponed

@krzyk right, I'll try to find someone else for this task

[davvd]

@HDouss thanks for reporting! I topped your account for 15 mins, transaction 75228386

[krzyk]

@davvd this is not postponed

[davvd]

@davvd this is not postponed

@krzyk got it, "postponed" tag removed from here

[krzyk]

@davvd assign @kitsook

[kitsook]

@krzyk @davvd i am not taking new tasks at the moment. please assign to someone else

[krzyk]

@kitsook OK, I'll reassign

[krzyk]

@davvd please assign @gumbelmj

[davvd]

@davvd assign @kitsook

@krzyk done. @kitsook the task is yours, please go ahead

[davvd]

@davvd please assign @gumbelmj

@krzyk sure! @gumbelmj the task is yours, please go ahead

[gumbelmj]

@davvd the pull request has been submitted.

[gumbelmj]

@HDouss can you close this issue as it has been fixed? Thanks.

[HDouss]

@gumbelmj it seems that the implementation also excludes such cases:

    private void data(final String input) {
        try {
            Class.someStaticMethod(input);
        } catch (final IOException ex) {
            throw new IllegalStateException(ex);
        }
    }

The example above is a true positive (i.e it should raise a complaint by NonStaticMethodCheck), but your implementation is discarding it.

[krzyk]

@HDouss I don't think this is a good case, if there is such method it should be definitely static (unless it is an @Override method).

[HDouss]

@krzyk This is what I said

The example above is a true positive (i.e it should raise a complaint by NonStaticMethodCheck)

But the solution provided by the PR will exclude such case.

[HDouss]

@krzyk It will exclude it from the check, but it shouldn't. Such case should make qulice raise an issue.

[krzyk]

@HDouss Sorry, you are right ( I must learn not to read on the phone :) ).

[gumbelmj]

@krzyk I saw you deployed the changes. Should I continue to address @HDouss concerns?

[krzyk]

@gumbelmj yes you should, deploy is just to finish the PR, you will need to create a new one.

[davvd]

@davvd the pull request has been submitted.

@gumbelmj OK

[krzyk]

@gumbelmj please add reference to the pull requests here

[gumbelmj]

@krzyk I updated pull request #685 to count the number of semi-colons instead of the number of lines. I also added another test case.

[gumbelmj]

@HDouss this has been merged again. Are you satisfied with the code now?

[HDouss]

@gumbelmj Thanks

[davvd]

@gumbelmj Thank you, I have added 30 mins to your account in payment/transaction "AP-61V51661VF813222P", time consumed: 261 hours and 29 mins... +30 added to your rating, at the moment it is: -30