Incorrect mapping leads to Invert Condition False Positive

[victorgveloso]

apache/hbase@29cf51d#diff-d827cf8ba45db10748de68b5b3d90987bb09b2bac680030080a940173f8f45b7L148-R203

The statements involved in the detected Invert Condition Refactoring are unrelated. Notably, while (!isUpdated) is in line 148 and if (isUpdated) is in line 202.

[tsantalis]

Based on the diff, I think
testUtil.waitFor(60000, 250, new ExplainingPredicate<Exception>() {

is now executing the same code in a loop, so the while loop is not needed any more.

So, there is a new if added at the end of the code with inverted condition from !isUpdated -> isUpdated

if (isUpdated) {
    return true;
}

RefactoringMiner allows the matching of different control statements, and in this case we have a control flow restructuring, where the original while is eliminated and at the end an if is added with an inverted condition.
So, these two control statements are somehow related.

Note how the statement quotaCache.triggerCacheRefresh();
which was the first statement inside the body of the deleted while is also moved right after the newly added if
This confirms that these two control statements are related.

[tsantalis]

@victorgveloso
Based on the analysis I did above, I think the Invert Condition is not a False positive.

[tsantalis]

@pouryafard75
What is your opinion about our diff for this commit?

[pouryafard75]

@tsantalis @victorgveloso
I prefer not to have a mapping for isUpdated.
However, there are more apparent inaccuracies in this diff such as:

• L166 -> R157 , R167, R179. isUpdated = false;
• L167 -> R158 , R168, R180. break;

Also, FP for:

• L154 -> R150 boolean isBypass = true; -> boolean isUserBypass = quotaCache.getUserLimiter(User.getCurrent().getUGI(), table).isBypass(); if (isUserBypass != bypass)
  These statements are irrelevant.

Probably after fixing these inaccuracies, we can discuss the main issue again which was isUpdated but at the moment, I prefer not to have them mapped.