Is there a way to disable specific quick fixes without disabling the rule itself?

Clean Code Report False-positive / False-negative...
, ,
1

Some rules, such as java:S1168, have quick fix actions that just aren’t always correct. While I think that rule is a good guideline, I’d prefer to not have a quick fix that can introduce a bug.

2

Hi,

Welcome to the community!

Would you mind providing some details? Specifically:

  • The code the issue is raised on
  • The incorrect quick fix
  • Why you think it’s incorrect

As well as the information requested in the category template:

  • Operating system:
  • SonarLint plugin version:
  • Programming language you’re coding in:
  • Is connected mode used:
    • Connected to SonarCloud or SonarQube (and which version):

 
Thx,
Ann

3

This in java on Windows (and mac and linux) with sonarlint 8.4.0.73538 (and others) connected to sonarqube 9.9.0.65466

 public class Demo{
 	class Foo{}
 	class Bar{}
 			
 	/**
 	 * @return all the Foos or null if Foo-getting is not currently permitted 
 	 */
 	List<Foo> getFoos() {
 		return null;
 	}
 	
 	/**
 	 * @return all the Bars or null if Bar-getting is not currently permitted 
 	 */
 	Bar[] getBars() {
 		return null;
 	}
 }

The quick fix is just to replace return null with return Collections.emptyList() and return new Bar[0]. For any existing method with specified behavior regarding null return, that will tend to introduce a bug.

4

Hi,

I’m confused. I don’t see anything on either of these methods that specifies behavior regarding null return…?

 
Ann

5

Maybe a slightly less trivial case would help:

/**
* @return all the Foos or null if Foo-getting is not currently permitted 
*/
List<Foo> getFoos() {
   if(canGetFoos()) {
      return List.of(new Foo(), new Foo());
   }
   return null;
}

A caller may rightfully handle a null return and an empty return differently and so might break when naively applying a quick fix to replace the null return with an empty list:

/**
*@throws FoosNotAvialableException if foos cannot be retrieved but not if they can be and there are none
*/
public void checkFoos() throws FoosNotAvailableException {
   if(getFoos() == null) {
      throw new FoosNotAvailableException();
   }
}

It’s sort of a tristate bool problem, and it is awkward and probably better avoided, but when the null is actually being used to represent a different state than an empty List or array, a proper fix would be more involved than just changing the return statement.

6

Hi,

Thanks for the clarification. I’ve flagged this for the language experts.

 
Ann

9

Hey @Connor_Balin,

Unfortunately, the only solution documented for SonarLint seems to be disabling the rule whole together.

However, and this may not be clear from the description, the rule will not raise any issue if your method is marked as nullable (eg: @CheckForNull, Nullable, …).

Is adding nullability annotations something possible on your code base?

This would also bring the benefit of helping other nullability-savvy rules to raise better issues on other parts of your code.

Cheers,

Dorian

11

Interesting. It does respect JSR305 and checker nullity annotations. I hadn’t noticed since we use custom nullity annotations.