Old issues are marked new - wrong new code rating

Hi,

We use SQ 7.9 LTS and have the following problem:

When we edit code (marked yellow), the existing issues below the edited line are reopened as new. The figure below illustrates this effect very good. The “new” issue is in old code. Apparently only the change of the line number is checked for the decision. Also the display is very confusing: When I want to see new issues, they are shown in a non-yellow code.

In our context the developers question the whole Leak Period method, if old issues are displayed as new every time a change is made.

Regards,
Günter

Hi Guenter,

Is this widespread, or do you have only isolated (this one?) examples?

Can you give some more details on the issue you’re almost showing? You’ve obfuscated everything that might help me understand which rule is at issue, as well as the issue date. If you’re telling me the issue is wrongly marked new, you could at least show me the date. :smiley:

To be clear, this kind of behavior is expected with some rules.

 
Ann

Hi Ann,

In a larger project such effects are always very difficult to comprehend. We are therefore in the process of setting up an SQ demo server with a small demo project to be able to reproduce this exactly. We also plan to check this with different programming languages (C++ / Java) and internal and external issues.

But what I can already say for sure:

  • We are using SVN, the code with the new issue is definitely old, which we can see in the version history and also SQ mark it like this (is not yellow).
  • The issue was already in the code before, but was moved down a few lines by the change.

What confuses the users most: “New Issues” is shown in areas that are not marked yellow (not “New Code”).

Regards,
Günter

Hi Guenter,

What rule are we talking about?

 
Ann

Hi Ann,

we are using this code:

  • creating the repository with: NewRepository repository = context.createRepository(getRepositoryKey(language), this.language.getKey()).setName(NAME);
  • adding the issue with: NewIssue newIssue = sensorContext.newIssue().forRule(RuleKey.of(getRuleRepositoryKey(), issue.getRuleId())); NewIssueLocation newIssueLocation = newIssue.newLocation().on(inputFile).at(inputFile.selectLine(lineNr)).message(location.getInfo());

Regards,
Günter

Hi Guenter,

Are you saying this issue is raised by a custom rule?

 
Ann

Hi Ann,

what do you mean with “custom rule”?

Looking to this conversation with Julien the way the repository and rule is created is a “normal issue”:

So for my understanding it should work?

Regards,
Günter

Hi Guenter,

There are cases where it is perfectly normal and expected for new issues to be raised on old code. I keep asking about the rule that’s raising the issue in question so I can evaluate whether this is one of those cases. If it is, I’ll explain why & we’re probably done. If not, then we’ll need to go further.

And then you give me the code you’re adding the issue with… Which implies the issue is being raised by a rule that’s not written/supported by SonarSource. Which is fine. The same new/old determination should apply regardless of who’s raising the issue. But I’m trying here to understand context.

So… What can you tell me specifically about the logic of the rule in question? What does it raise an issue on? Does it have 2ndary locations? [Anything else that might be useful about it]?

 
Ann

Hi Ann,

Unfortunately I still have no answer from the colleagues who wanted to reproduce this again.

Here in advance:

  • We have a long existing software, in a SVN repository, let’s call it SVN revision 1.
  • Revision 1 has an issue in line 1412 that was added via the API NewRepository / NewIssue / NewIssueLocation. The issue has no 2ndary locations.
  • The leak period on the SQ 7.9.4 server is set to 90 days.
  • After one year 4 new lines are added to the code (SVN revision 2). The code is rescanned.
  • The existing issue moves from line 1412 to line 1416 and is passed again to SonarQube with API NewRepository / NewIssue / NewIssueLocation.
  • Lines 1410 to 1413 are now correctly identified as new in the display.
  • The issue behind the inserted code is now recognized as “new”. It is now in the “New Code Smells” list. The line 1416 with the issue is not marked new.

Hope this helps. As soon as I have more information I will add it here.

Regards,
Günter

Hi Guenter,

I’m looking at the docs on the “new” algorithm

Based on this, it seems that the issue shouldn’t be new:

  • detect block move inside file, then if the issue is on the same (moved) line and on the same rule (but not necessarily with the same message) > MATCH

However, looking at the initial screenshot, I see that the block of added lines includes a conditional… which presumably added indentation to the issue line…? Which I guess could have changed the hash. (I don’t know whether or not we trim before the hash.)

What blame date does SonarQube have for the line in question?

 
Ann

Hi Ann,

We have made first experiments with a single file. Each change was committed individually (own revision number) and passed to SonarQube.

  1. Added comments at the beginning of the file.
  2. Comment inserted in block.
  3. Statement inserted
  4. New block added around existing issues.

New code is always recognized correctly. None of the existing issues was recognized as new. Everything works as expected.

Two questions to your last answer:

  1. What is the definition of ‘block’ in the context of C++?
  2. “…This algorithm relies on content hashes (excluding whitespace) for the line the issue is reported on…” Is the content hash in C++ before or after preprocessing?

What blame date does SonarQube have for the line in question?

Will clarify this with the team asap.

Regards,
Günter

Hi Ann,

below a sample from our productive system with blame data and issue date:

Regards,
Günter

Hi Guenter,

Thanks for the blame data.

I can’t answer the questions you posed in your previous reply. I’m referring this internally. And it would still be nice to know what rule we’re talking about. Since I understand now that we’re talking about C++ and I have the impression that this might be a rule you developed in-house, I need to ask whether we’re dealing with the Developer Edition($) C++ analysis or the community plugin.

 
Ann

Hi Ann,

we are using the Enterprise Edition ($$$) with different programming languages. For C++ we are using the SonarCFamily and community plugin. The sample above is from the community plugin.

But why I think that makes no difference:

  • the SCM and blame handling is part of the SonarQube core
  • the ‘New Code’ detection is part of the SonarQube core
  • ‘New Issue’ detection is part of SonarQube core
  • plugins using all same API to forward issues

Regards,
Günter

Hi Ann,

we think we have a first finding:

In case the text of an issue is changing, the issue is marked ‘new’.

  • This could happen if the issue string contains additional context information like class name, method name, variable name, types, values, … .
  • Or after an update the text of an rule is different: changed, extended, fixed typo, …

It seems that the ‘New Code’ algorithm takes also the text of the issue into account and not only the rule key? Can you confirm this?

We will continue our search in this direction…

Regards,
Günter

Hi Guenter,

Yes, I can confirm this. It’s right there in the docs where it talks about “message”

 
HTH,
Ann

Hello @ganncamp,

on the identical server edition and version we are facing also problems for C#

First malicious example: rename a method, do not change it’s body => cyclomatic complexity is raised as new/open issue. No immediate coherence with the changed lines. Don’t mind if it’s obsolete, I could have reproduced the issue with any other method:

Second malicious example (changed lines marked red): only method body was changed, but SonarQube raised a new issue that the method should me made static. No immediate coherence with the changed lines:

Hi @acook,

Although I understand it may seem related, what you are describing is a different problem. Off the top of my head, it could seem like an analyzer update. Perhaps this code was originally scanned with an older version of our C# analyzer, and changing these few lines triggered a new analysis with an up-to-date version.

If that’s not the case, could you open a different thread for this issue, and tag it with csharp? This will attract the attention of the right people, so they can help you.

1 Like