S1120: SonarQube tab size calculation

Hello,

After running sonar on my projects (maven plugin), I see an issue under SonarQube that I could not explain / fix.
I am facing " Make this line start after 6 spaces instead of 12 in order to indent the code consistently. (Indentation level is at 4.)" error (Rule java:S1120) on the following code :

public static void myMethod(List<SR> roles) { 
	Collections.sort(roles, new Comparator<SR>() {
		@Override
		public int compare(SR o1, SR o2) {  <----- HERE

The count seems to be ok on my IDE (3 tabs, tab size is equals to 4).
How could I fix the sonar issue ? How does sonar count ?

Thanks for your help !
Estelle

  • SonarQube Developer 10.2.1.78527 (zip)
  • maven-sonar-plugin 3.10.0.2594

Hello, @stl543, and welcome to the Sonar Community!

Unfortunately, I’m unable to reproduce the issue from your code snippet. The undesired behavior may be due to the anonymous class defined for implementing the comparator.

May I suggest you use a lambda instead? You can extract the implementation of your compare into a method and use it as an argument for Collections.sort(); this way, the code is more readable and clean, and the reported issue will disappear.

Let me know if this helps you. Alternatively, I need a small reproducer to investigate the issue further: a small project, as a zip file or public GitHub project, having a class that reproduces the issue you incur.

Cheers,
Angelo

Hello Angelo !

First of all, thanks for your answer !! I’m going to make a reproducer :slight_smile:
I agree that this specific code can be modified to be more readable and probably the sonar indentation error will be fixed too, but it is just a sample. The problem appears too on some lambdas, try/catch…
However, I do not understand how sonarqube can find 6 spaces when the indentation level is 4. Here there are 3 tabs, and the tab is supposed to be “4 spaces” length. How is it possible to get 6 spaces ?
I think that I misunderstand the sonar error.

Thanks for help !
Estelle

Hello,

Here’s a simple reproducer project.
reproducerSonar.zip (2.6 KB)

Thanks for help,
Estelle

1 Like

Thank you, @stl543. Can you tell me which IDE you use and what your configuration is for tabs and indents?

For example, in IntelliJ, you can find it in settings → editor → code style → java:

The rule is more reliable when the indentation uses spaces instead of characters. In IntelliJ, this can be enabled by unticking the Use tab character box.

I will investigate further the issue and come back to you.

Hi Angelo,

I use intelliJ with the eclipse code formatter plugin (= based on a xml configuration file)
This configuration file is also used by the IDE and maven (formatter-maven-plugin).
Here’s the file, if can help.
Estelle

formatter.zip (3.7 KB)

Thank you @stl543 for sharing your configuration. The file specifies the tab character for indentation:

<setting id="org.eclipse.jdt.core.formatter.tabulation.char" value="tab"/>

Instead of using the space character:

<setting id="org.eclipse.jdt.core.formatter.tabulation.char" value="space"/>

Unfortunately, using the tab char leads to undesired behavior, and the recommendation is to use the space char. As mentioned in the previous answer, you can untick the Use tab character box or update the formatter.xml configuration to use space. Visually, there will be no difference.

The root problem is related to how the OS represents the tab character (see this discussion). The Java Analyzer uses the AST generated by the ECJ Parser, so we don’t have any control on how the position of tokens is computed. Mapping the tabulation to spaces instead of tabs avoids inconsistencies.

Hi Angelo,

I understand your response, but the problem is that I can’t change this: the formatter configuration file is a file common to all our projects. It is a communalized configuration. I will try to propose the modification, but I think it will be refused, and sonar rule will simply be disabled :frowning:
So this is not really a solution for me…

However, the Sonar message is still confusing for me:

  • if the indentation level is 4 (dixit sonar, and it is compliant with my configuration), how is it possible to force 6 spaces ?
  • if tabs are not supported by sonar (due to OS or system limitations), does that means that all the sonar rules dealing with tabulation/indentation are inconsistent?

Thanks for help :slight_smile:
Estelle

Hi @stl543,

I think proposing that change for your projects is a good idea. It is a configuration for the IDE that allows a cross-platform, consistent indentation.

Rule S1120 is not enabled by default from the Sonar Way profile, so it is up to your company to decide whether to keep it. Ensuring that the codebase is uniformly indented is generally recommended.

Answering your questions:

  1. The Java Analyzer doesn’t have control over how the token positions are computed; it relies on ECJ.
    Regarding the message “Make this line start after 6 spaces instead of 12 in order to indent the code consistently. (Indentation level is at 4.)” reported on the code below: the six comes from adding 4 (the indentation level) to the position of the anonymous class closing bracket, which is a tab char. It seems that the tab char is mapped to 2 spaces. In the case of anonymous classes and lambdas, the rule doesn’t check the initial indentation, assuming it is valid.
public static void myMethod(List<SR> roles) { 
	Collections.sort(roles, new Comparator<SR>() {
		@Override
		public int compare(SR o1, SR o2) { // <- anonymous class indentation + 4
                }
       } // <- anonymous class offset using tab
}
  1. The rules about tabulation/indentation rely on spaces. Using tabs means depending on the OS to replace the tab character with a default number of spaces. This is not consistent. Thus, the inconsistency is not in the rule but in choosing the tab character as tabulation.

Cheers,
Angelo

Hi Angelo,

Sorry but this is still confusing for me, because when I read sonar message, the detection and the configuration seem to be ok.
I really would like to understand why some indentations are badly detect and some are ok.

Let’s take again a look to the first sample I show.
In the line where Sonar detects the bad indentation, there is a 3 indentation level (1 for method, 1 for Collections, and 1 for inner).
Sonar annonces 12 spaces. Thats right to me: 3 x 4. One tab = 4 spaces is not really read through the code but forced through Sonar configuration. Thats ok
Now, Sonar annonces that 6 spaces is expected.
If the indentation size is 4, that is simply impossible (4 or 8 or 12). Sonar should respect the indentation size set in the rules…

I try to run from different OS and I get the same result (Through linux, 1 tab = 8 spaces). But there is no impact on sonar result.

Where am I wrong ?
Thanks for help !
Estelle

Hello @stl543,

It was also confusing for me initially, and the only way to understand it was by looking at the code implementing this rule.

I will point you to the code so you can understand better what I described in the previous answer. When visiting an anonymous class or a lambda expression, the initial indentation level is computed from the column offset of the closing bracket. When the tab character is used for tabulation, the value of this column offset may not correspond to the indentation level: in your case, it seems to be 2 instead of 4.

For this reason, the rule may report a confusing message when anonymous classes and lambda expressions are combined with an IDE that uses the tab character for tabulation. I created [SONARJAVA-4894] - Jira to improve the reported message in those cases and create less confusion.

I wish you a wonderful day.

Cheers,
Angelo

Hi Angelo !
Thanks a lot for your explanation, your patience, the JRA ticket and the links to the code !
It is confusing but really interesting !
If columnOffset is the indentation level (=number of “separators” = 2), don’t you think the indentation size (= nb of spaces to make one separator) should be taken into account there?
As a sonar newbie, I realize that my reasoning may seem very simplistic, but I think that the total indentation size for a anonymous class should be equals to columnOffSet * indentationSize + indentationSize, so as to convert every calculus into the same universal unit (space).
Does this thinking make sense, or am I totally wrong ?

It is unfortunate that the rule can’t be applied if tabs are used. :frowning:

Cheers,
Estelle

1 Like

Thank you, @stl543.

Now that [SONARJAVA-4894] - Jira has been created it will be up to the developer working on it to come up with a solution and your input will be taken into account.

Cheers,
Angelo

1 Like