ASSERT-KTH / sorald

Automatic repair system for static analysis warnings from SonarQube's SonarJava, TDSC 2022 http://arxiv.org/pdf/2103.12033
MIT License
90 stars 27 forks source link

Bug in S1481 #1020

Closed Zuplyx closed 1 year ago

Zuplyx commented 1 year ago

The repair for rule S1481 creates compile errors when processing local variables in for loops or local variables with writes (see example below). A possible fix for this problem, would be to ignore unused variables in for-loops and to also remove all writes to the unused local variable.

Original Code:

import java.util.List;

public class UnunsedLocalVariables
{
  public void inForLoop(List<String> inputList)
  {
    for (String input : inputList)
    {
      doSomething();
    }
  }

  public int withWrite(int input)
  {
    int test = 0;
    if (input > 2)
    {
      test = input - 2;
    }

    return input;
  }

  public void doSomething()
  {

  }
}

Result after processing with sorald:

import java.util.List;

public class UnunsedLocalVariables
{
  public void inForLoop(List<String> inputList)
  {
    for ( : inputList)
    {
      doSomething();
    }
  }

  public int withWrite(int input)
  {
    if (input > 2)
    {
      test = input - 2;
    }

    return input;
  }

  public void doSomething()
  {

  }
}
algomaster99 commented 1 year ago

@Zuplyx thank you for the report! It is indeed a bug. Will fix it as soon as possible.

algomaster99 commented 1 year ago

Hi @Zuplyx ! I looked at the issue, and sonar-java does not suggest any fix for it. However, I do agree that is should not produce a non-compilable output.

@khaes-kth any idea what could we do here?

  1. Convert the enhanced for loop into the normal one with int as iterator.
  2. Mark the processor as incomplete.

Do we ever provide a fix that is not dictated in SonarQube? If yes, we could go forward with 1.

khaes-kth commented 1 year ago

Hi @algomaster99, I would say this is a problem with SonarSource documents and suggestions. I would choose option 2! If we cannot fix the issue using SonarSource suggestions, we don't do it (skip such issues) and mark the processor as incomplete.

algomaster99 commented 1 year ago

with SonarSource documents and suggestions

Opened a bug report here.

I prefer 1, but I will try to talk to SonarSource about this fix though. Otherwise I will go forward with 2.

algomaster99 commented 1 year ago

SonarSource updated the documentation for rule S1481.

However, it does not talk about enhanced for-loop at all. Let me ask them about this.

khaes-kth commented 1 year ago

Let me ask them about this.

Great! Let's do it!

algomaster99 commented 1 year ago

Done! See https://community.sonarsource.com/t/s1481-unused-local-variables-should-be-removed-is-wrongly-documented/89427/3?u=aman. Let's wait for their reply.

algomaster99 commented 1 year ago

Released in https://github.com/ASSERT-KTH/sorald/releases/tag/sorald-0.8.5.