SONAR complaining about Make the enclosing method "static" or remove this set

Question

I have the following piece of code in my program and I am running SonarQube 5 for code quality check on it after integrating it with Maven.

However, Sonar is asking to Make the enclosing method "static" or remove this set. the method is setApplicationContext.

How to remove this error? Why this error is coming?

public class SharedContext implements ApplicationContextAware {
public static final String REPORT_ENGINE_FACTORY = "reportEngineFactory";
private static ApplicationContext applicationContext;

public void setApplicationContext(ApplicationContext applicationContext)
        throws BeansException {
    SharedContext.applicationContext = applicationContext;
}

public static ApplicationContext getApplicationContext() {
    return applicationContext;
}

public Object getBean(String name) {
    return applicationContext.getBean(name);
} }

G. Ann - SonarSource Team · Accepted Answer · 2019-11-14T16:08:23.047

15

To be specific, you appear to be asking about rule S2696, 'Instance methods should not write to "static" fields'

As the rule description details:

Correctly updating a static field from a non-static method is tricky to get right and could easily lead to bugs if there are multiple class instances and/or multiple threads in play. Ideally, static fields are only updated from synchronized static methods.

Thus, the issue is telling you to make the method on which it was raised (presumably setApplicationContext) static, so that across all class instances there is only one copy of that method making updates to the static (i.e. shared across all class instances) field applicationContext. It additionally recommends making the method synchronized so that only one instance at a time can call the method.

Late edit: To see the rule description click either the "See Rule" or the "..." (depending on your version of SonarQube) shown after the issue message.

edited Nov 14 '19 at 16:08

answered Sep 09 '16 at 09:13

G. Ann - SonarSource Team

22,346
4
40
76

1

To add some context, any developer who sees that you have a non-static `setApplicationContext` method will think that it will change the application context only for that instance, not for all of the instance of the application. – SJuan76 Sep 09 '16 at 09:15
9

you cannot make it static. because you will violate the contract for ApplicationContextAware – anonymouse Aug 26 '17 at 18:46
Where do you find this rule description? – Nick Nov 12 '19 at 18:53
See my expansion @Nick. – G. Ann - SonarSource Team Nov 14 '19 at 16:08
1

I don't think this actually answers the question. The OP is indicating a situation that is currently an accepted practice within the Spring Framework world when it is necessary to grab the Application Context (such as in the case of a custom shutdown manager). Making the method static breaks the contract. Maybe the problem is with Spring, but this does not resolve the issue beyond offering an explanation of what is happening. – ccellist Mar 03 '20 at 15:49
1

@ccellist, OP asks "How to remove this error? Why this error is coming?" I think I've answered both fairly thoroughly. – G. Ann - SonarSource Team Apr 09 '20 at 15:07
Does not answer the question, as making this method static would break the contract of the overriden function. – Zmur Jan 06 '23 at 11:46

score 12 · Answer 2 · answered Sep 11 '20 at 01:17

For what it's worth, and I'm probably going to get blacklisted by the Sonar community specifically and the Java Universe in general for saying this, adding @SuppressWarnings("squid:S2696") to the top of the offending method causes Sonar to ignore that warning completely.

score 2 · Answer 3 · answered Jun 02 '21 at 04:34

2

This worked for me.

@Setter
private static volatile ApplicationContext context;

@Override
public void setApplicationContext(ApplicationContext ac) throws BeansException {
    setContext(ac);
}

answered Jun 02 '21 at 04:34

user16101992

21
1

Non-primitive fields should not be "volatile" – Sivaram Rasathurai Jun 15 '22 at 06:12

score 0 · Answer 4 · answered Jun 15 '22 at 06:34

I faced this issue today. @G.Ann explained why this sonar issue happens and how it can be removed. The way G.Ann suggested will not be worked in this particular scenario.

When someone wants to override a method, the method signature can not be changed hence we can't add static or synchronised in the method signature.

@Override
public void setApplicationContext(ApplicationContext ac) throws BeansException {
    //implmentation
}

So we can try @ccelis way to suppress the sonar warnings. but I don't like to do that.

@user16101992 said we can use volatile in the applicationContext declaration but this is not a good approach since it will violate the Non-primitive fields should not be "volatile" sonar violation.

Hence we can't change the method signature, we can remove the static from the applicationContext variable declaration.

private ApplicationContext applicationContext;

With this one, we can remove the violation and we don't need to worry about the object creation for the applicationContext since the spring framework follows the singleton factory pattern to create the applicationContext.

SONAR complaining about Make the enclosing method "static" or remove this set

4 Answers4

Linked