Bug 353394 - "Method can potentially be static" warning on JUnit test methods (annotated with org.junit.Test)
Summary: "Method can potentially be static" warning on JUnit test methods (annotated w...
Status: VERIFIED WORKSFORME
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.7   Edit
Hardware: PC Windows 7
: P3 minor with 1 vote (vote)
Target Milestone: 3.8 M1   Edit
Assignee: Ayushman Jain CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-29 07:06 EDT by Ben Davis CLA
Modified: 2022-04-08 04:18 EDT (History)
7 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ben Davis CLA 2011-07-29 07:06:50 EDT
Build Identifier: 20110615-0604

A "method can potentially be static" warning is reported for JUnit test methods, i.e. those that have the annotation @org.junit.Test. If I make such a method static, I get "java.lang.Exception: Method blah() should not be static" when I try to run the test.

Reproducible: Always

Steps to Reproduce:
1. Make sure 'method can potentially be static' warning is disabled.
2. Paste the example below into a Java editor.

import org.junit.Test;
public class StaticMethodWarningTest {
  @Test public void theTest() {}
}
Comment 1 Ben Davis CLA 2011-07-29 13:50:51 EDT
In the first step to reproduce, I obviously meant 'enabled'!
Comment 2 Ayushman Jain CLA 2011-08-01 10:15:07 EDT
I think the method should not be static is a JUnit framework limitation. There's nothing wrong from a language point of view in having this method static. Even the compiler does not complain if you change it to static. And thats what all the static analysis warnings/errors are for - they cannot predict runtime issues because of non-language limitations. 
You can use the quick fix on the warning to add an @SuppressWarnings("static-method") annotation to the method.

Btw, I can only reproduce your case if there's atleast one line of code inside theTest() method.
Comment 3 Ben Davis CLA 2011-08-01 12:26:10 EDT
I thought it seemed such a common use case that it would be worth the compiler knowing about it.

Otherwise, perhaps a solution in collaboration with the JUnit developers would work?

- org.junit.Test itself could have @SuppressWarnings("static-method") as a meta-annotation - or @SuppressWarnings("annotated-static-method") or something like that.
- The compiler could check meta-annotations when deciding what to warn about.

That seems less clean to me though.
Comment 4 Ayushman Jain CLA 2011-08-01 13:03:24 EDT
(In reply to comment #3)
> - org.junit.Test itself could have @SuppressWarnings("static-method") as a
> meta-annotation - or @SuppressWarnings("annotated-static-method") or something
> like that.
This might not make too much sense for those using junit outside of Eclipse as such, so I don't think they would go with this.

> - The compiler could check meta-annotations when deciding what to warn about.
That would mean hardcoding a comparison with the @Test or such annotations. Doesn't look good.

I do understand your concern, but I think it is best to leave the warning as just a guideline and to exercise your own discretion when adding "static". This is also true for other compiler warnings such as "Potential null pointer access". In many cases you don't need a null check, but the compiler is just making you aware there's a chance.
Comment 5 Ayushman Jain CLA 2011-08-02 14:18:42 EDT
Closing as WORKSFORME.
Comment 6 Satyam Kandula CLA 2011-09-13 07:13:29 EDT
Verified for 3.8M2
Comment 7 Radim Kolar CLA 2012-11-01 07:35:03 EDT
I also would like to get @Test annotations excluded from this warning. It will fix major portion of misswarnings.

Hard coding into eclipse might not be cleanest solution but junit is most used framework for unit testing in java, it will definitively help people to write better code.
Comment 8 Ayushman Jain CLA 2012-11-07 04:40:15 EST
(In reply to comment #7)
> I also would like to get @Test annotations excluded from this warning. It
> will fix major portion of misswarnings.
> 
> Hard coding into eclipse might not be cleanest solution but junit is most
> used framework for unit testing in java, it will definitively help people to
> write better code.

Please see discussions in comment 2 and comment 4. If you still think you have a concern thats not addressed, please mention it here. As I said earlier, the warning is just a guideline, which in this case is quite correct from a java language standpoint.
Comment 9 Ben Davis CLA 2012-11-07 21:12:50 EST
I think you are allowing yourself to lose sight of real use cases...

Nevertheless, I think I have a compromise that should make both sides reasonably happy. How about having an extra checkbox in the settings:

Method can potentially be static  [ Warning [v] ]
[ ] Include annotated methods

People who care about the JUnit methods can then keep the checkbox disabled. The only people left cold are those who use their own annotations too and wanted the warning in those cases, but that's probably not many people. :)
Comment 10 Ayushman Jain CLA 2012-11-08 03:40:46 EST
(In reply to comment #9)
> The only people left cold are those who use their own annotations too and
> wanted the warning in those cases, but that's probably not many people. :)

This may be correct for now. Note, however, that once java 8 comes out you will see annotated methods very commonly, and having this warning be ignored for annotated methods will virtually hide it at all relevant places. The best solution would be to special case @Test annotation, but that path is full of new annotations demand similar custom treatment. I'd rather just leave things as they are for now.
Comment 11 Joshua Ventura CLA 2016-04-26 11:22:05 EDT
I just wanted to second this issue. The "static-method" warning is quite useful, but suppressing it is ugly, and I don't like doing so before every method I'm already annotating @Test. It's really ugly, and hard to get through code review. The warning isn't very popular here, so I don't get much sympathy suppressing it in unit tests, which just makes it noise.