Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

esql:NavigatingTreeCouldBeReference - Rule Behavior Issue #119

Open
TuneableSloth opened this issue Jun 30, 2020 · 6 comments
Open

esql:NavigatingTreeCouldBeReference - Rule Behavior Issue #119

TuneableSloth opened this issue Jun 30, 2020 · 6 comments
Assignees

Comments

@TuneableSloth
Copy link

Hi,

We recently upgraded to Sonar Server 7.9 and along with that upgraded the sonar-esql-plugin from 2.3.5 to 3.0.1. However, after this upgrade, projects for which we had clean reports started showing major issues after re-run with the "Navigating message tree could be replaced by a reference" Rule. (We have classified this as a major rule and we have build breakers in place with a threshold of 0 for this).

The behavior seems off as it reports major issue if the occurrence of two level tree structure is more than 3. For the following case, it seems to be not applicable -

image

There is no other way in our knowledge that we can create a reference for XMLNSC.Attribute.

Also, in case of the compliant solution, if reqRef.age occurs more than 3 times, it will be reported as an issue.

image

This default threshold however seems to be unchanged compared to the earlier version. So, the behavior of rule has changed it seems.

Please suggest if this is working as expected or needs an enhancement.

Thanks.

@ThomasPohl ThomasPohl self-assigned this Jul 22, 2020
@ThomasPohl
Copy link
Member

Hi @TuneableSloth,

the NavigatingTreeCouldBeReferenceCheck has been refactored completely between 2.3.5 and 3.0.1. The former implementation had a lot false-negatives. To make the check useable for you again I could add a second configuration property to specify the minimum depth/size of the field reference.

Just to be sure we are talking about the same thing. You would linke to have this snippet to not create an issue:

IF reqRef.age = 2 THEN
   SET reqRef.age = 4;
ELSEIF reqRef.age = 3 THEN
   SET reqRef.age = 5;
ELSE
   SET reqRef.age = 77;
END IF;

But this one should create an issue:

IF reqRef.person.age = 2 THEN
   SET reqRef.person.age = 4;
ELSEIF reqRef.person.age = 3 THEN
   SET reqRef.person.age = 5;
ELSE
   SET reqRef.person.age = 77;
END IF;

@TuneableSloth
Copy link
Author

Hi @ThomasPohl ,

Your understanding is correct. The second snippet should create an issue. Another configuration property to specify the minimum depth/size of the field reference would be really great. This would avoid lot of false positives at our end.

I would be looking forward to the updated release for the these changes.

Thanks.

@ThomasPohl
Copy link
Member

H @ThomasPohl ,

I added the changes to a new release candidate: https://github.com/EXXETA/sonar-esql-plugin/releases/tag/3.1.0-RC1

Does it fit for your needs?

@TuneableSloth
Copy link
Author

Hi @ThomasPohl,

Thank you for implementing the enhancement. The changes are working as expected. We can now see major issues being reported if e.g- reqRef.person.age is used more than 3 times. However, during the testing I observed that reading the usertrace file renders a warning -

image

The code coverage is reported properly after this, but this didn't occur in the previous versions. Any clue what might be the reason for this? I am still analyzing if this would have any further impact.

Thanks

@ThomasPohl
Copy link
Member

Hi @TuneableSloth ,
can you provide a trace file that produces these warnings? I would like to check if we are missing something here.

@TuneableSloth
Copy link
Author

Hi @ThomasPohl ,

Unfortunately, due to organization policies it would not be possible for me to share the trace file here. Can you please let me know if you would have some bandwidth for a short meeting. Then I can show you the trace file and the issue over call. I am seeking an exception to check if I can share the file over email with you. Please let me know what suits you.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants